Make vpn-select modular #8

Closed
rohammer wants to merge 1 commits from rohammer:vxlan-node into master
Member

vpn-select is an old relic and did not reflect the opportunities of our hoodfile.
This rewrite makes vpn-select modular to easely add new vpn-protocols.

The stuff dependent on the vpn-protocol is outsourced to files in /usr/lib/vpn-select.d/ and comes in with the respective vpn package.

vpn-stop is removed to use the protocol independent start/stop mechanism of vpn-select. Instead, a symlink is used.

We should avoid paralell vpns to the same gateway and not mix the protocol per hood. It is therefore necessary that all gateways offer the same protocols in one hood.
The most prefered protocol supported and avaylable in the hood is selected. The order for this is defined in "uci fff.vpnselect.protocol_order" eg. "vxlan l2tp fastd"

Signed-off-by: Robert Langhammer rlanghammer@web.de

vpn-select is an old relic and did not reflect the opportunities of our hoodfile. This rewrite makes vpn-select modular to easely add new vpn-protocols. The stuff dependent on the vpn-protocol is outsourced to files in /usr/lib/vpn-select.d/ and comes in with the respective vpn package. vpn-stop is removed to use the protocol independent start/stop mechanism of vpn-select. Instead, a symlink is used. We should avoid paralell vpns to the same gateway and not mix the protocol per hood. It is therefore necessary that all gateways offer the same protocols in one hood. The most prefered protocol supported and avaylable in the hood is selected. The order for this is defined in "uci fff.vpnselect.protocol_order" eg. "vxlan l2tp fastd" Signed-off-by: Robert Langhammer <rlanghammer@web.de>
rohammer force-pushed vxlan-node from c3d19ed4b7 to 9cc338534f 2020-12-13 10:17:07 +01:00 Compare
adschm added the
packages/fff
node
labels 2020-12-13 14:01:27 +01:00
Owner

Hallo, ein prefix für den Commit-Titel wäre schön, also "fff-vpn-select: make vpn-select modular" oder so.

Hallo, ein prefix für den Commit-Titel wäre schön, also "fff-vpn-select: make vpn-select modular" oder so.
adschm reviewed 2020-12-13 14:47:23 +01:00
@ -0,0 +1,5 @@
uci set fff.vpnselect=fff
uci set fff.vpnselect.protocol_order="vxlan fastd"
Owner

Eigenlich unterstützen wir noch kein vxlan, insofern würde ich das hier rausnehmen, bis es offiziell unterstützt wird.

Eigenlich unterstützen wir noch kein vxlan, insofern würde ich das hier rausnehmen, bis es offiziell unterstützt wird.
Author
Member

jo, kann ich raus nehmen. Kommt dann mit dem vxlan Patch wieder rein.

jo, kann ich raus nehmen. Kommt dann mit dem vxlan Patch wieder rein.
rohammer marked this conversation as resolved
adschm reviewed 2020-12-14 12:34:18 +01:00
@ -12,0 +27,4 @@
# compare $protocol_order with $supported_protocols
for proto in $protocol_order; do
echo "$supported_protocols" | grep $proto >&/dev/null || protocol_order=${protocol_order/$proto/}
Owner

Hier würde ggf. auch z.B. "fastd" mit "fastd2" gematcht. Ggf. sollte man word-boundaries in den grep mit einführen.

Hier würde ggf. auch z.B. "fastd" mit "fastd2" gematcht. Ggf. sollte man word-boundaries in den grep mit einführen.
Owner

eventuell lohnt sich ein blick auf man 1 comm oder aehnliches

eventuell lohnt sich ein blick auf `man 1 comm` oder aehnliches
rohammer marked this conversation as resolved
adschm reviewed 2020-12-14 12:43:17 +01:00
@ -12,0 +28,4 @@
# compare $protocol_order with $supported_protocols
for proto in $protocol_order; do
echo "$supported_protocols" | grep $proto >&/dev/null || protocol_order=${protocol_order/$proto/}
done
Owner

Wenn man voraussetzt, das jedes Protokoll in protocol_order eingetragen werden muss, könnte man das etwas vereinfachen:

# load protocol_order
protocol_order="$(uci get fff.vpnselect.protocol_order)"

# source functions
for p in $protocol_order; do
	# only select files that are in protocol_order
	file="/usr/lib/vpn-select.d/$p"
	[ -f $file ] && . "$file" || continue
	# add to supported_protocols if file is found
	supported_protocols="$supported_protocols $p"
	# clear old config
	"${protocol}_clear"
done

Wenn man voraussetzt, das jedes Protokoll in protocol_order eingetragen werden muss, könnte man das etwas vereinfachen: ``` # load protocol_order protocol_order="$(uci get fff.vpnselect.protocol_order)" # source functions for p in $protocol_order; do # only select files that are in protocol_order file="/usr/lib/vpn-select.d/$p" [ -f $file ] && . "$file" || continue # add to supported_protocols if file is found supported_protocols="$supported_protocols $p" # clear old config "${protocol}_clear" done ```
Author
Member

Gefaellt mir gut. Tut genau was ich moechte. Das grep ist weg und es wird ev. nur gesourced, was gebraucht wird.
Vielen Dank!

Gefaellt mir gut. Tut genau was ich moechte. Das grep ist weg und es wird ev. nur gesourced, was gebraucht wird. Vielen Dank!
Author
Member

Ach bloed, geht doch nicht. Ich brauche alle supported_protocols zum clear. Sonst können configs uebrig bleiben, wenn man ein protocol aus der Liste protocol_order loescht.

Ach bloed, geht doch nicht. Ich brauche alle supported_protocols zum clear. Sonst können configs uebrig bleiben, wenn man ein protocol aus der Liste protocol_order loescht.
Owner

Wie wichtig ist es denn in der Praxis, dass man die Reihenfolge per uci ändern kann?

Könnte man nicht einfach die Reihenfolge per prefix vorgeben, wie bei uci-default auch?

Also 10-fastd und 20-vxlan?

Wir haben ja ohnehin schon zwei Orte (vpn-select.d und hood file), wo wir die Protokolle behandeln müssen. Ich weiß nicht, ob das Ganze besser wird, wenn man jetzt drei hat.

Wie wichtig ist es denn in der Praxis, dass man die Reihenfolge per uci ändern kann? Könnte man nicht einfach die Reihenfolge per prefix vorgeben, wie bei uci-default auch? Also 10-fastd und 20-vxlan? Wir haben ja ohnehin schon zwei Orte (vpn-select.d und hood file), wo wir die Protokolle behandeln müssen. Ich weiß nicht, ob das Ganze besser wird, wenn man jetzt drei hat.
Owner

Alternativ könnte man sagen, das man das uci-Setting zum an-/abschalten behält, aber zumindest die Reihenfolge von den Dateien nimmt. Das wäre immer noch deutlich einfacher zu implementieren als mit einer echten protocol order.

Alternativ könnte man sagen, das man das uci-Setting zum an-/abschalten behält, aber zumindest die Reihenfolge von den Dateien nimmt. Das wäre immer noch deutlich einfacher zu implementieren als mit einer echten protocol order.
Author
Member

Ich hab das ganze umgebaut um den vxlan Client zu machen. Darum habe ich darauf geachtet, dass es voellig unabhaengig voneinander ist. Zum Um/Ein/Ausschalten reicht eine uci Zeile. So kann ich zum testen auch einfach einen tarball auf den Router werfen. Die protocol_order koennte man auch als Hoodparameter vom KeyX beziehen. Wenn man das statischer macht, wird es nur auf den ersten Blick einfacher. Es war echt schick mit diesem Unterbau das vxlan Ding zu bauen. Man muss in keine bestehende Datei rein. Nur die protocol_order. Und die ist konfigurierbar.
Ich wuerde es gerne so offen wie moeglich lassen.

Ich hab das ganze umgebaut um den vxlan Client zu machen. Darum habe ich darauf geachtet, dass es voellig unabhaengig voneinander ist. Zum Um/Ein/Ausschalten reicht eine uci Zeile. So kann ich zum testen auch einfach einen tarball auf den Router werfen. Die protocol_order koennte man auch als Hoodparameter vom KeyX beziehen. Wenn man das statischer macht, wird es nur auf den ersten Blick einfacher. Es war echt schick mit diesem Unterbau das vxlan Ding zu bauen. Man muss in keine bestehende Datei rein. Nur die protocol_order. Und die ist konfigurierbar. Ich wuerde es gerne so offen wie moeglich lassen.
Owner

Ich kuck später noch mal, ob man da einen Kompromiss finden kann. Die Entropie ist ja hier recht hoch ...

Ich kuck später noch mal, ob man da einen Kompromiss finden kann. Die Entropie ist ja hier recht hoch ...
Owner

Also in jedem Fall kann man die for-Schleife "#clear old config" wegmachen und vorziehen:

# source functions and reset all available protocols
for file in /usr/lib/vpn-select.d/*; do
	[ -f $file ] && . "$file" || break
	supported_protocols="$supported_protocols $protocol"
	"${protocol}_clear"
done

Den Rest muss ich mir noch überlegen, mir ist das insgesamt einfach eine Nummer zu kompliziert ...

Also in jedem Fall kann man die for-Schleife "#clear old config" wegmachen und vorziehen: ``` # source functions and reset all available protocols for file in /usr/lib/vpn-select.d/*; do [ -f $file ] && . "$file" || break supported_protocols="$supported_protocols $protocol" "${protocol}_clear" done ``` Den Rest muss ich mir noch überlegen, mir ist das insgesamt einfach eine Nummer zu kompliziert ...
adschm reviewed 2020-12-14 12:45:46 +01:00
@ -0,0 +32,4 @@
([ -s "$pidfile" ] && [ -d "/proc/$(cat "$pidfile")" ]) && /etc/init.d/fastd stop
fi
}
Owner

Unneeded empty line at EOF.

Unneeded empty line at EOF.
rohammer marked this conversation as resolved
adschm reviewed 2020-12-14 12:46:38 +01:00
@ -4,0 +8,4 @@
# The function ${protocol}_addpeer() is called for every selected peer in hoodfile.
# The function ${protocol}_start_stop() is called at the end once per installed protocol.
#
# The variable $protocol_order defines the order of preference. The most preffered protocol of the avaylable and offered protocols is selected.
Owner

Typo: "available"
Hier könnte man auch dann zur Klarstellung darauf hinweisen, dass ein Protokoll hier genannt sein muss, damit es funktioniert.

Typo: "available" Hier könnte man auch dann zur Klarstellung darauf hinweisen, dass ein Protokoll hier genannt sein _muss_, damit es funktioniert.
rohammer marked this conversation as resolved
adschm reviewed 2020-12-14 12:49:55 +01:00
@ -0,0 +5,4 @@
}
fastd_addpeer() {
[ -d /tmp/fastd_fff_peers ] || mkdir /tmp/fastd_fff_peers
Owner

Hmm, gibts es da theoretische Vor- oder Nachteile im Vergleich zu mkdir -p /tmp/fastd_fff_peers ohne test?

Hmm, gibts es da theoretische Vor- oder Nachteile im Vergleich zu `mkdir -p /tmp/fastd_fff_peers ohne test?`
Owner

mkdir -p ist definitiv besser hier.

`mkdir -p` ist definitiv besser hier.
Author
Member

Das fastd fasse ich in diesem Patch nicht an. Braucht einen eigenen.

Das fastd fasse ich in diesem Patch nicht an. Braucht einen eigenen.
rohammer marked this conversation as resolved
adschm reviewed 2020-12-14 13:03:53 +01:00
@ -0,0 +17,4 @@
json_get_var port port
echo "remote \"${address}\" port ${port};" >> "$filename"
echo "" >> "$filename"
echo "float yes;" >> "$filename"
Owner

Ich frage mich gerade, ob es klug ist, hier zeilenweise zu schreiben. Da die Dateien aber auf /tmp im RAM liegen, ist es an der Stelle wahrscheinlich wurscht. Keine Ahnung ob die Buffer da Effekte haben...

Im Prinzip würde das ja genauso funktionieren?
echo "#name \"${servername}\";\nkey \"${key}\";\nremote \"${address}\" port ${port};\n\nfloat yes;" > "$filename"

Ich frage mich gerade, ob es klug ist, hier zeilenweise zu schreiben. Da die Dateien aber auf /tmp im RAM liegen, ist es an der Stelle wahrscheinlich wurscht. Keine Ahnung ob die Buffer da Effekte haben... Im Prinzip würde das ja genauso funktionieren? ` echo "#name \"${servername}\";\nkey \"${key}\";\nremote \"${address}\" port ${port};\n\nfloat yes;" > "$filename"`
Owner

Bitte aber nur als Diskussionsanregung verstehen. Sowas würde ich wenn überhaupt nur in einem separaten Patch anfassen wollen ...

Bitte aber nur als Diskussionsanregung verstehen. Sowas würde ich wenn überhaupt nur in einem separaten Patch anfassen wollen ...
Owner

Sowas kann man auch mal mit einem here-document loesen. Dann hat man nur ein cat und kann es trotzdem lesen, im Gegensatz zu diesem Einzeiler.

Sowas kann man auch mal mit einem here-document loesen. Dann hat man nur ein cat und kann es trotzdem lesen, im Gegensatz zu diesem Einzeiler.
Author
Member

Der fastd Kram ist mehr oder weniger eine Kopie von der alten version. Das wollte ich nicht anfassen. Gehoert in einen extra Patch.

Der fastd Kram ist mehr oder weniger eine Kopie von der alten version. Das wollte ich nicht anfassen. Gehoert in einen extra Patch.
rohammer marked this conversation as resolved
adschm reviewed 2020-12-14 13:13:45 +01:00
@ -50,2 +57,2 @@
make_config
/etc/init.d/fastd reload
# select most preferred protocol for this hood
selected_protocol=$(for proto in $protocol_order; do echo $offered_protocols | grep $proto >&/dev/null && echo $proto && break; done)
Owner

Statt prüfen, Ausgabe unterdrücken, und dann echo könnte man auch einfach den grep direkt das richtige ausgeben lassen (-o). Den Fehler kann man dann auch gleich mit (-q) unterdrücken:

selected_protocol=$(for proto in $protocol_order; do echo $offered_protocols | grep -o -q $proto && break; done)

Ansonsten gäbe es noch die klassische Version, hier mal primär als Diskussionsbeitrag:

for proto in $protocol_order; do
	echo $offered_protocols | grep -q $proto >/dev/null && {
    	selected_protocol=$proto
    	break
	}
done
Statt prüfen, Ausgabe unterdrücken, und dann echo könnte man auch einfach den grep direkt das richtige ausgeben lassen (-o). Den Fehler kann man dann auch gleich mit (-q) unterdrücken: `selected_protocol=$(for proto in $protocol_order; do echo $offered_protocols | grep -o -q $proto && break; done)` Ansonsten gäbe es noch die klassische Version, hier mal primär als Diskussionsbeitrag: ``` for proto in $protocol_order; do echo $offered_protocols | grep -q $proto >/dev/null && { selected_protocol=$proto break } done ```
Author
Member

grep -o -q , sehr schoen. Danke!

grep -o -q , sehr schoen. Danke!
Author
Member

-o und -q kann man leider nicht kombinieren. -q gewinnt. Es kommt nix raus.
Ich hab es jetzt nur mit -o gemacht

-o und -q kann man leider nicht kombinieren. -q gewinnt. Es kommt nix raus. Ich hab es jetzt nur mit -o gemacht
Owner

Interessant, ich dachte -q wäre für stderr. Offenbar ist es für stdout ... Dann muss man ggf. noch den stderr auch abfangen (bzw. das mal testen).

Interessant, ich dachte -q wäre für stderr. Offenbar ist es für stdout ... Dann muss man ggf. noch den stderr auch abfangen (bzw. das mal testen).
Author
Member

Ich habe es getestet. stderr muss man nicht abfangen. Ein "no match" ist kein Fehler.
echo blob | grep foo liefert nie eine Ausgabe auf stderr.

Ich habe es getestet. stderr muss man nicht abfangen. Ein "no match" ist kein Fehler. echo blob | grep foo liefert nie eine Ausgabe auf stderr.
Owner

I see, ich hatte das mit uci -q verwechselt...

I see, ich hatte das mit uci -q verwechselt...
Author
Member

Auch das -o muss ich wieder zurueck nehmen.
Bei 2 peers mit z.B. fastd.

echo "fastd vxlan fastd" | grep -o fastd 
fastd
fastd

und das matcht natuerlich nicht mit "fastd"

Auch das -o muss ich wieder zurueck nehmen. Bei 2 peers mit z.B. fastd. ``` echo "fastd vxlan fastd" | grep -o fastd fastd fastd ``` und das matcht natuerlich nicht mit "fastd"
Owner

geht da -m1?

geht da -m1?
Owner

geht da -m1?

Nö, schade. Aber ich finde sowieso, dass das Ganze irgendwie auch eleganter gehen muss.

> geht da -m1? Nö, schade. Aber ich finde sowieso, dass das Ganze irgendwie auch eleganter gehen muss.
adschm reviewed 2020-12-14 13:17:24 +01:00
@ -65,0 +63,4 @@
json_get_var protocol protocol
[ "$protocol" = "$selected_protocol" ] && "${protocol}_addpeer"
json_select ".." # back to vpn
done
Owner

Der Ordnung halber könnte man hier noch das json_select ".." # back to root wieder einfügen, funktional dürfte es egal sein.

Der Ordnung halber könnte man hier noch das `json_select ".." # back to root` wieder einfügen, funktional dürfte es egal sein.
Author
Member

Kurz danach ist das Skript zu ende. Spielt keine Rolle. Ich lass es so.

Kurz danach ist das Skript zu ende. Spielt keine Rolle. Ich lass es so.
rohammer marked this conversation as resolved
adschm reviewed 2020-12-14 13:21:46 +01:00
@ -0,0 +30,4 @@
([ -s "$pidfile" ] && [ -d "/proc/$(cat "$pidfile")" ]) || /etc/init.d/fastd start
else
([ -s "$pidfile" ] && [ -d "/proc/$(cat "$pidfile")" ]) && /etc/init.d/fastd stop
fi
Owner

Hmm, subshells. Ob das wieder ich war? Das sollte man auch mal ordentlich machen, aber auch hier separat.

Hmm, subshells. Ob das wieder ich war? Das sollte man auch mal ordentlich machen, aber auch hier separat.
Owner

PR #13

PR #13
rohammer marked this conversation as resolved
adschm reviewed 2020-12-14 13:24:47 +01:00
Owner

Also persönlich würde ich vpn-stop töten und das in configurehood anpassen.

Also persönlich würde ich vpn-stop töten und das in configurehood anpassen.
adschm reviewed 2020-12-14 13:36:25 +01:00
@ -3,1 +3,4 @@
# Usage: vpn-select <path-to-hood-file>
# To add a new protocol, put a file with three functions to /usr/lib/vpn-select.d/ .
# The file must start with protocol=name. It is most important to use the same name here and in hoodfile.
# Some parameters of the hood are avaylable via $hood_id, $hood_name and $hood_protocol.
Owner

Typo: available

Typo: available
rohammer marked this conversation as resolved
adschm reviewed 2020-12-14 13:37:03 +01:00
@ -16,0 +43,4 @@
json_select hood
json_get_var hood_id id
json_get_var hood_name name
json_get_var hood_protocol protocol
Owner

Benutzen wir die für irgendwas?

Benutzen wir die für irgendwas?
Author
Member

noch nicht. Die id braucht dann vxlan.

noch nicht. Die id braucht dann vxlan.
Owner

Ist zwar kleinkariert, aber würde ich dann auch erst dann einfügen. Macht auch die Änderungen in diesem Patch einfacher nachzuvollziehen.

Ist zwar kleinkariert, aber würde ich dann auch erst dann einfügen. Macht auch die Änderungen in diesem Patch einfacher nachzuvollziehen.
Author
Member

Die Idee war, eine Basis zu schaffen, auf der jemand ein neues Protokoll hinzufuegen kann, ohne das package vpn-select anfassen zu muessen. Aber wenn es dann besser durchs Karo passt, kommt es weg.

Die Idee war, eine Basis zu schaffen, auf der jemand ein neues Protokoll hinzufuegen kann, ohne das package vpn-select anfassen zu muessen. Aber wenn es dann besser durchs Karo passt, kommt es weg.
Author
Member

Ich wuerde das gerne lassen, dann muss der vxlan Patch nicht in die vpn-select rein greifen.

Ich wuerde das gerne lassen, dann muss der vxlan Patch nicht in die vpn-select rein greifen.
Owner

Aber wenn der vxlan Patch diese Parameter plötzlich nutzt, dann ist doch der vxlan-Patch der Grund, warum die überhaupt da sind. Dann gehören die auch in den vxlan-Patch rein, damit man sie dort findet. D.h. du fasst ja doch im vxlan-Patch vpn-select an, weil du neue Abhängigkeiten erzeugt, du würdest es nur nicht hinschreiben.

Aber wenn der vxlan Patch diese Parameter plötzlich nutzt, dann ist doch der vxlan-Patch der Grund, warum die überhaupt da sind. Dann gehören die auch in den vxlan-Patch rein, damit man sie dort findet. D.h. du fasst ja doch im vxlan-Patch vpn-select an, weil du neue Abhängigkeiten erzeugt, du würdest es nur nicht hinschreiben.
Owner

Alternativ könnte man höchstens noch nur diese drei Zeilen in einen Extra-Patch tun, als separate "Vorbereitung" für vxlan.

Alternativ könnte man höchstens noch nur diese drei Zeilen in einen Extra-Patch tun, als separate "Vorbereitung" für vxlan.
Author
Member

Man koennte auch sagen, die Daten werden angeboten und koennen bei Bedaf genutzt werden. So ne Art Schnittstelle. Die VPNs koennen das nutzen was sie brauchen und sich konfigurieren.

Man koennte auch sagen, die Daten werden angeboten und koennen bei Bedaf genutzt werden. So ne Art Schnittstelle. Die VPNs koennen das nutzen was sie brauchen und sich konfigurieren.
Owner

Sieht gut aus. Die Remarks sind ja im Wesentlichen Gestaltungsfragen, grundsätzlich erscheint mir das erstrebenswert und sollte auch funktionieren.

Sieht gut aus. Die Remarks sind ja im Wesentlichen Gestaltungsfragen, grundsätzlich erscheint mir das erstrebenswert und sollte auch funktionieren.
adschm reviewed 2020-12-15 12:27:15 +01:00
@ -12,0 +25,4 @@
supported_protocols="$supported_protocols $protocol"
done
# compare $protocol_order with $supported_protocols
Owner

Also zumindest das würde ich mir gerne sparen:
Erstens würde ich erwarten, das man die Protokolle in protocol_order richtig eintragen kann. Wenn nicht, sollte das nicht silently entfernt werden, sondern dann später im Skript tatsächlich einen Fehler erzeugen, weil die Funktion/Datei nicht gefunden werden kann. So merkt man dann ggf. auch, das man Quatsch eingetragen hat (was in meinen Augen gut ist).

Damit haben wir schon mal eine for-Schleife weniger, die bei jedem Run von vpn-select läuft. Als Nebeneffekt können wir dann im Prinzip sogar die ganze supported_protocols Variable entfernen, und einfach am Anfang (clear protocols) und Schluss (apply protocols) jeweils einmal über /usr/lib/vpn-select.d/* iterieren. Das sollte dann auch insgesamt deutlich einfacher zu lesen und verstehen sein. Funktional ändert sich dadurch erstmal nichts, außer dass eben "falsche" Einträge in protocol_order dann Fehler erzeugen, und nicht einfach weggelöscht werden.

Also zumindest das würde ich mir gerne sparen: Erstens würde ich erwarten, das man die Protokolle in protocol_order richtig eintragen kann. Wenn nicht, sollte das nicht silently entfernt werden, sondern dann später im Skript tatsächlich einen Fehler erzeugen, weil die Funktion/Datei nicht gefunden werden kann. So merkt man dann ggf. auch, das man Quatsch eingetragen hat (was in meinen Augen gut ist). Damit haben wir schon mal eine for-Schleife weniger, die bei jedem Run von vpn-select läuft. Als Nebeneffekt können wir dann im Prinzip sogar die ganze supported_protocols Variable entfernen, und einfach am Anfang (clear protocols) und Schluss (apply protocols) jeweils einmal über /usr/lib/vpn-select.d/* iterieren. Das sollte dann auch insgesamt deutlich einfacher zu lesen und verstehen sein. Funktional ändert sich dadurch erstmal nichts, außer dass eben "falsche" Einträge in protocol_order dann Fehler erzeugen, und nicht einfach weggelöscht werden.
Author
Member

Ich versuch das mal genauer zu erklaeren, wie ich zu dieser Variante kam.
Der Mechanismus mit der protocol_order kam spaeter rein, als mir bewusst wurde, dass ein protokoll, in dem Fall vxlan, protokollspezifische Voraussetzungen pruefen muss. Sind diese nicht erfuellt, ist es noetig, dass es sich aus den moeglichen Protokollen entfernt.
In protocol_order steht also drin, was wir uns wuenschen. Wir wissen nicht, was in der Hood angeboten wird, und ob die technichen Voraussetzungen am Anschluss erfuellt sind. (bei vxlan ist es ipv6) Darum habe ich diese Variable auch als Schalter konzepiert, der durch uci aber auch von den Protokollen bedient werden kann und die Priorisierung wiederspiegelt.

Wenn man das einfacher hin bekommt waere das klasse.

Natuerlich kann man jedes mal die liste der supported_protocols mit ls * holen. Ich mag das gerne einmal in einer Variablen. Aber das ist Geschmacksache.

Ich versuch das mal genauer zu erklaeren, wie ich zu dieser Variante kam. Der Mechanismus mit der protocol_order kam spaeter rein, als mir bewusst wurde, dass ein protokoll, in dem Fall vxlan, protokollspezifische Voraussetzungen pruefen muss. Sind diese nicht erfuellt, ist es noetig, dass es sich aus den moeglichen Protokollen entfernt. In protocol_order steht also drin, was wir uns wuenschen. Wir wissen nicht, was in der Hood angeboten wird, und ob die technichen Voraussetzungen am Anschluss erfuellt sind. (bei vxlan ist es ipv6) Darum habe ich diese Variable auch als Schalter konzepiert, der durch uci aber auch von den Protokollen bedient werden kann und die Priorisierung wiederspiegelt. Wenn man das einfacher hin bekommt waere das klasse. Natuerlich kann man jedes mal die liste der supported_protocols mit ls * holen. Ich mag das gerne einmal in einer Variablen. Aber das ist Geschmacksache.
Owner

Aber welche Protokolle in vpn-select.d liegen wird doch an gleicher Stelle kontrolliert wie der uci Eintrag. Insofern hat der Eintragende ja immer über beides Kontrolle. Entsprechend finde ich das automatische Löschen immer noch unnötig und würde hier einen Fehler erwarten.

Aber welche Protokolle in vpn-select.d liegen wird doch an gleicher Stelle kontrolliert wie der uci Eintrag. Insofern hat der Eintragende ja immer über beides Kontrolle. Entsprechend finde ich das automatische Löschen immer noch unnötig und würde hier einen Fehler erwarten.
Author
Member

Ich wollte es auch so haben, dass man bei der Firmware einfach fastd oder vxlan raus werfen kann, ohne irgendwas an der Konfiguration aendern zu muessen. Also ist protocol_order durchaus ungleich supported_protocols. Und an der Stelle gleiche ich das ab.
Vielleicht kann man das auch ohne den ganzen foo machen. Ich sehe es aber nicht.

Ich wollte es auch so haben, dass man bei der Firmware einfach fastd oder vxlan raus werfen kann, ohne irgendwas an der Konfiguration aendern zu muessen. Also ist protocol_order durchaus ungleich supported_protocols. Und an der Stelle gleiche ich das ab. Vielleicht kann man das auch ohne den ganzen foo machen. Ich sehe es aber nicht.
Owner

Aber das Löschen ist doch nur dafür da, wenn du zusätzliche Eintrage in protocol_order hast. In der Praxis löscht doch niemand ein Skript aus vpn-select.d und lässt den Eintrag in protocol_order; wenn das so wäre, bräuchten wir protocol_order ja nicht. Insofern muss davon ausgegangen werden, das jeder zusätzliche Eintrag in protocol_order ohne passenden vpn-select.d File einen Fehler darstellt.

Aber das Löschen ist doch nur dafür da, wenn du _zusätzliche_ Eintrage in protocol_order hast. In der Praxis löscht doch niemand ein Skript aus vpn-select.d und lässt den Eintrag in protocol_order; wenn das so wäre, bräuchten wir protocol_order ja nicht. Insofern muss davon ausgegangen werden, das jeder zusätzliche Eintrag in protocol_order ohne passenden vpn-select.d File einen Fehler darstellt.
Author
Member

Wir haben doch jedes protocol in einem package, das an- oder abgewaehlt werden kann. Wie soll man dann eine Reihenfolge festlegen. Ich muss doch irgendwo sagen, falls fastd da ist, bevorzuge fastd. Und falls es vxlan gibt, setze es an 2. Stelle. Und wenn l2tp vohanden, nimm das vor fastd. usw. Auch wenn man mal die Reihenfolg vom KeyX bezieht, kann man nicht voraussagen, welche protocols auf dem Router vorhanden sind. Meine Loesung gleicht das ab und reagiert auf alle moeglichen Varianten.
Das hab ich auch getestet und funzt.
Und falls das gewuenschte protocol zwar da ist, aber nicht von den Gateways angeboten wird, muss man auch darauf reagieren und das naechste aus der Liste nehmen.
Das ist schon alles ziemlich verzwickt.
Ausserdem muss man auch testen ob ein protocol funktionieren kann, also ob z.B ipv6 da ist. Falls nicht, das naechste aus der Liste nehmen. Das kann aber vpn-select nicht wissen. Das muss das protocol mitbringen. Und ueber das manipulieren der protocol_order habe ich das geloest.

Ich versuche das vxlan Ding fertig zu machen. Dann sieht man vielleicht noch etwas mehr.

Wir haben doch jedes protocol in einem package, das an- oder abgewaehlt werden kann. Wie soll man dann eine Reihenfolge festlegen. Ich muss doch irgendwo sagen, falls fastd da ist, bevorzuge fastd. Und falls es vxlan gibt, setze es an 2. Stelle. Und wenn l2tp vohanden, nimm das vor fastd. usw. Auch wenn man mal die Reihenfolg vom KeyX bezieht, kann man nicht voraussagen, welche protocols auf dem Router vorhanden sind. Meine Loesung gleicht das ab und reagiert auf alle moeglichen Varianten. Das hab ich auch getestet und funzt. Und falls das gewuenschte protocol zwar da ist, aber nicht von den Gateways angeboten wird, muss man auch darauf reagieren und das naechste aus der Liste nehmen. Das ist schon alles ziemlich verzwickt. Ausserdem muss man auch testen ob ein protocol funktionieren kann, also ob z.B ipv6 da ist. Falls nicht, das naechste aus der Liste nehmen. Das kann aber vpn-select nicht wissen. Das muss das protocol mitbringen. Und ueber das manipulieren der protocol_order habe ich das geloest. Ich versuche das vxlan Ding fertig zu machen. Dann sieht man vielleicht noch etwas mehr.
Owner

Verstehe ich das eigentlich richtig: Mit der neuen Implementierung gibt es ein "preferred protocol", und nur das wird dann mit allen Gateways gesprochen? Man kann also nicht mehr wie früher z.B. mit einem Gateway fastd und dem anderen l2tp sprechen, je nachdem was diese anbieten?

Wenn ja, gibt es dafür einen technischen Grund oder ist das nur wegen der Implementierung im Code so gewählt (in letzterem Fall könnte man ggf. überlegen, ob man das nicht einfach besser im json lösen kann)?

Verstehe ich das eigentlich richtig: Mit der neuen Implementierung gibt es ein "preferred protocol", und _nur_ das wird dann mit allen Gateways gesprochen? Man kann also nicht mehr wie früher z.B. mit einem Gateway fastd und dem anderen l2tp sprechen, je nachdem was diese anbieten? Wenn ja, gibt es dafür einen technischen Grund oder ist das nur wegen der Implementierung im Code so gewählt (in letzterem Fall könnte man ggf. überlegen, ob man das nicht einfach besser im json lösen kann)?
Author
Member

Wir hatten im Meehaeusle mal besprochen, das pro Gateway zu machen und z.B vxlan zu bevorzugen und fastd als fallback. Das ist aber so kompliziert geworden, dass es mein kleiner Geist nicht erfassen konnte. Darum habe ich beschlossen, eine Variante zu bauen, bei der pro Hood nur ein Protokoll benutzt wird. Das setzt voraus, dass alle Gateways einer Hood die gleichen Protokolle anbieten. Das sollte aber moeglich sein.
Ich wuerde immer noch eine Variante pro Gateway toll finden. Wenn das jeman hin bekommt waere das super.

Wir hatten im Meehaeusle mal besprochen, das pro Gateway zu machen und z.B vxlan zu bevorzugen und fastd als fallback. Das ist aber so kompliziert geworden, dass es mein kleiner Geist nicht erfassen konnte. Darum habe ich beschlossen, eine Variante zu bauen, bei der pro Hood nur ein Protokoll benutzt wird. Das setzt voraus, dass alle Gateways einer Hood die gleichen Protokolle anbieten. Das sollte aber moeglich sein. Ich wuerde immer noch eine Variante pro Gateway toll finden. Wenn das jeman hin bekommt waere das super.
rohammer force-pushed vxlan-node from 9cc338534f to 2eccadc3ce 2020-12-15 16:06:41 +01:00 Compare
adschm reviewed 2020-12-15 18:11:20 +01:00
@ -4,0 +8,4 @@
# The function ${protocol}_addpeer() is called for every selected peer in hoodfile.
# The function ${protocol}_start_stop() is called at the end once per installed protocol.
#
# The variable $protocol_order defines the order of preference. The most preffered protocol of the available and offered protocols is selected.
Owner

Typo: preferred

Typo: preferred
Owner

Noch ein anderes Thema: Ich finde für diesen Fall die Includes weniger zweckdienlich als bei gateway.d. Ich wundere mich, ob es nicht vielleicht besser wäre, daraus ausführbare Dateien zu machen:

/etc/vpn-select.d/fastd clear
/etc/vpn-select.d/fastd start-stop
/etc/vpn-select.d/vxlan configure

Dadurch würde man diese Redundanz bei den Funktionsnamen los, und könnte die im vpn-select.d Skript einfach start-stop, clear etc. nennen, ohne das Prefix. Nebenbei würde das auch das Testen/Debuggen vereinfachen, wenn man mit den einzelnen Skripten direkt arbeiten kann. Und im Prinzip sind die einzelnen "Protokolle" ja schon so was wie separate Dienste ...

Mittelfristig könnte man dann sogar überlegen, ob man noch irgendwelches status-Gedöns oder so anflanscht ...

Noch ein anderes Thema: Ich finde für diesen Fall die Includes weniger zweckdienlich als bei gateway.d. Ich wundere mich, ob es nicht vielleicht besser wäre, daraus ausführbare Dateien zu machen: /etc/vpn-select.d/fastd clear /etc/vpn-select.d/fastd start-stop /etc/vpn-select.d/vxlan configure Dadurch würde man diese Redundanz bei den Funktionsnamen los, und könnte die im vpn-select.d Skript einfach start-stop, clear etc. nennen, ohne das Prefix. Nebenbei würde das auch das Testen/Debuggen vereinfachen, wenn man mit den einzelnen Skripten direkt arbeiten kann. Und im Prinzip sind die einzelnen "Protokolle" ja schon so was wie separate Dienste ... Mittelfristig könnte man dann sogar überlegen, ob man noch irgendwelches status-Gedöns oder so anflanscht ...
Member

/etc/vpn-select.d/fastd clear
/etc/vpn-select.d/fastd start-stop
/etc/vpn-select.d/vxlan configure

Dadurch würde man diese Redundanz bei den Funktionsnamen los, und könnte die im vpn-select.d Skript einfach start-stop, clear etc. nennen, ohne das Prefix. Nebenbei würde das auch das Testen/Debuggen vereinfachen, wenn man mit den einzelnen Skripten direkt arbeiten kann. Und im Prinzip sind die einzelnen "Protokolle" ja schon so was wie separate Dienste ...

ich hab den Patch jetzt nicht komplett verfolgt aber wären das nicht Kanditaten für /etc/init.d? Irgendwie klingt das so als würde man das gerne dort haben wollen.

> /etc/vpn-select.d/fastd clear > /etc/vpn-select.d/fastd start-stop > /etc/vpn-select.d/vxlan configure > > Dadurch würde man diese Redundanz bei den Funktionsnamen los, und könnte die im vpn-select.d Skript einfach start-stop, clear etc. nennen, ohne das Prefix. Nebenbei würde das auch das Testen/Debuggen vereinfachen, wenn man mit den einzelnen Skripten direkt arbeiten kann. Und im Prinzip sind die einzelnen "Protokolle" ja schon so was wie separate Dienste ... ich hab den Patch jetzt nicht komplett verfolgt aber wären das nicht Kanditaten für /etc/init.d? Irgendwie klingt das so als würde man das gerne dort haben wollen.
rohammer closed this pull request 2021-01-11 17:32:12 +01:00
Author
Member

Es wird eine neue, simplere Version geben. PR geschlossen

Es wird eine neue, simplere Version geben. PR geschlossen

Pull request closed

Sign in to join this conversation.
No description provided.