fff-layer3-config: add rules for router_ip #178

jkimmel · 2021-11-28T16:30:27+01:00

jkimmel commented

2021-11-28 16:30:27 +01:00

This forces routes for packets originating from a router_ip to be looked
up in the fff table. If the router_ips don't happen to be included in
the client network's subnet, the decision defaults to a main table
lookup. This causes packets to choose the wrong interface.

This patch forces packets from a router_ip to be routed via the fff table.

Fixes #175

Signed-off-by: Johannes Kimmel <fff@bareminimum.eu

This forces routes for packets originating from a router_ip to be looked up in the fff table. If the router_ips don't happen to be included in the client network's subnet, the decision defaults to a main table lookup. This causes packets to choose the wrong interface. This patch forces packets from a router_ip to be routed via the fff table. Fixes #175 Signed-off-by: `Johannes Kimmel <fff@bareminimum.eu`

fbl approved these changes 2021-11-29 20:11:52 +01:00

fbl left a comment

Ein paar Sachen inline, ansonsten müsste es passen.

src/packages/fff/fff-layer3-config/files/etc/layer3.d/30-network-routerip Outdated

						
				@ -12,0 +16,4 @@

						local name="$1"

						# check if filter was added by configuregateway

						if ! [ "$(uci -q get network.$name.addedbyautoconfig)" = 'true' ]; then

fbl commented

2021-11-29 20:09:36 +01:00

'addedbyautoconfig' ist für so etwas generisches wie ip rules vielleicht etwas wenig. Wenn in Zukunft noch andere Komponenten rules hinzufügen sollen, kollidiert das.

Entweder machen wir diesen Tag spezifischer oder wir verschieben das entfernen der alten rules in ein eigenes layer3.d Skript, welches vor allem läuft, was Netzwerkdinge tut.

'addedbyautoconfig' ist für so etwas generisches wie ip rules vielleicht etwas wenig. Wenn in Zukunft noch andere Komponenten rules hinzufügen sollen, kollidiert das. Entweder machen wir diesen Tag spezifischer oder wir verschieben das entfernen der alten rules in ein eigenes layer3.d Skript, welches vor allem läuft, was Netzwerkdinge tut.

jkimmel marked this conversation as resolved

src/packages/fff/fff-layer3-config/files/etc/layer3.d/30-network-routerip Outdated

						
				@ -15,1 +33,4 @@

						uci -q add_list network.loopback.ipaddr="$ip"

						config=$(uci add network rule)

						uci -q set network.$config.src="$ip"

fbl commented

2021-11-29 20:11:35 +01:00

Was passiert, wenn man die Router IP ohne Subnetzgröße angibt? Funktioniert das trotzdem?

jkimmel commented

2021-11-29 20:51:50 +01:00

Guter Fund. ip kommt damit klar, OpenWRT leider nicht. Das will die IP unbedingt in CIDR Notation.

Da ich mir nicht sicher bin, ob in router_ip nur reine IPs stecken darf, oder auch dinge mit Subnetz dran, hänge ich nur an den IPs, die kein Subnetz definieren ein /32 bzw. /128 an.

Guter Fund. `ip` kommt damit klar, OpenWRT leider nicht. Das will die IP unbedingt in CIDR Notation. Da ich mir nicht sicher bin, ob in router_ip nur reine IPs stecken darf, oder auch dinge mit Subnetz dran, hänge ich nur an den IPs, die kein Subnetz definieren ein `/32` bzw. `/128` an.

src/packages/fff/fff-layer3-config/files/etc/layer3.d/30-network-routerip Outdated

						
				@ -16,0 +35,4 @@

						config=$(uci add network rule)

						uci -q set network.$config.src="$ip"

						uci -q set network.$config.lookup='fff'

						uci -q set network.$config.priority='10000'

fbl commented

2021-11-29 20:10:14 +01:00

Hier sollten wir dazu kommentieren, warum 10000. Scheinbar ist das an den bestehenden Regeln orientiert, die OpenWrt aber selbst erzeugt. Die Prio könnte sich daher in Zukunft ändern.

jkimmel marked this conversation as resolved

jkimmel force-pushed master from 1c727e9bc6 to 0104373444

2021-11-29 20:31:28 +01:00

Compare

jkimmel force-pushed master from 0104373444 to 9a189f541d

2021-11-29 20:49:05 +01:00

Compare

fbl approved these changes 2021-11-30 20:22:24 +01:00

fbl left a comment

Sieht jetzt gut aus, danke.

Für die router_ip haben wir bisher nicht definiert, was da drin sein darf. In so fern würde ich einfach mal das annehmen, was OpenWrt für ipaddr lists kann, und das ist sowohl mit als auch ohne Netzgröße. Passt also so.

Reviewed-by: Fabian Bläse <fabian@blaese.de>

Sieht jetzt gut aus, danke. Für die router_ip haben wir bisher nicht definiert, was da drin sein darf. In so fern würde ich einfach mal das annehmen, was OpenWrt für ipaddr lists kann, und das ist sowohl mit als auch ohne Netzgröße. Passt also so. `Reviewed-by: Fabian Bläse <fabian@blaese.de>`

fbl commented

2021-12-01 13:37:49 +01:00

Danke!

Commit Message noch etwas angepasst, PKG_RELEASE bump ergänzt und applied.

Danke! Commit Message noch etwas angepasst, PKG_RELEASE bump ergänzt und applied.

fbl closed this pull request

2021-12-01 13:37:49 +01:00