Support Babeld without VLAN on ethernet interfaces inside br-lan #600

ilario · 2019-09-20T16:44:45Z

Fix #596
Part of the code has been copied and pasted from BMX6 (which run on br-lan by default).

We want to support running Babeld without VLAN (list protocols babeld:0) because:

it is not actually needed
seems that switch on some Mediatek routers does not like it

But Babeld cannot run directly on an interface (e.g. eth0 or eth0.1) which is included into a bridge (e.g. br-lan) because it cannot have an IPv6 Link-Local.
What have that is the bridge itself, so Babeld has to run on the bridge.
But the bridge includes bat0 (Batman-adv stuff) which assures that all the packets get through, distorting the way that Babeld has for determining the metric of a route (sending UDP packets and checking how many get lost).

Maybe the configurable option babeld_over_batman is not useful and we can consider that it is always false?

I am still testing this PR, please review but wait for merging :)

G10h4ck · 2019-09-21T09:20:31Z

packages/lime-proto-anygw/Makefile

@@ -23,7 +23,7 @@ define Package/$(PKG_NAME)
 	URL:=http://libremesh.org
 	DEPENDS:=+dnsmasq-dhcpv6 @(!PACKAGE_dnsmasq) +ebtables +libuci-lua \
 		+lime-system +lua +kmod-ebtables +kmod-macvlan \
-		+shared-state +shared-state-dnsmasq_leases
+		+shared-state +shared-state-dnsmasq_leases +kmod-ebtables-ipv6


Didn't you fixed this in another PR? here it seems unrelated

That kernel module is needed both by anygw (see #599) and Babeld in case of no VLAN and presence of Batman.

Gosh, now I noticed that this file was actually not pertaining to this PR, sorry.

G10h4ck · 2019-09-21T09:22:11Z

packages/lime-proto-babeld/files/usr/lib/lua/lime/proto/babeld.lua

+	--! the bridge interface br-lan
+	--! If Babeld's Hello packets run over Batman-adv (whose bat0 is also 
+	--! included in br-lan), the links will have a wrong quality metric,
+	--! so these hello on bat0 have to be filtered


It is not that they have a wrong quality metric, it's that babeld will see all nodes as direct neighbours through batman

mh... Isn't this the same? But ok, I'll change the text :)

no it is not the same

G10h4ck · 2019-09-21T09:23:29Z

packages/lime-proto-babeld/files/usr/lib/lua/lime/proto/babeld.lua

+			elseif(proto == "batadv") then hasBatman = true end
+		end
+
+		if hasLan and ifname:match("^eth%d") then


What happens with wan port? Or port manually configured with ground routing?

Also consider to move this to configure method as it don't seems to need to be run for each interface, run it once seems enough

What happens with wan port?

Woops... With this PR, WAN port is not anymore used by Babeld.
This was unintentional, sorry.
Anyway WAN port is used for connecting to non-LibreMesh networks, so Babeld is not needed there.
I am ok with fixing it if we want Babeld on WAN.
Rather than checking for lan protocol, I can check for the presence of the interface into the

config interface 'lan' list ifname 'bat0' list ifname 'eth0.1'

table.

Or port manually configured with ground routing?

Ports configured with specific configuration are not treated by the whole section.
Do you mean that even if ground routing ports have a specific configuration, they should trigger this fix anyway? Are them included into br-lan bridge?

Also consider to move this to configure method as it don't seems to need to be run for each interface, run it once seems enough

Sounds good!
Currently it runs more than once even if running once would be ok.
Running more than once is not detrimental but I agree that it is a waste.
If I put this into the configure, it would be executed even if no ethernet lan port is present, but I suppose this is ok, right?

Also consider to move this to configure method as it don't seems to need to be run for each interface, run it once seems enough

If I put this into the configure, it would be executed even if no ethernet lan port is present, but I suppose this is ok, right?

Wait, the ebtables rule can be in the configuration part, but the ifname = "br-lan" stuff has to stay into the setup interface one...
This is a mess.

Ports configured with specific configuration are not treated by the whole section.
Do you mean that even if ground routing ports have a specific configuration, they should trigger this fix anyway? Are them included into br-lan bridge?

It all depends on what kind of configuration they got, if they have babled specified as proto they get there too

This is a mess.

Networking is a complex problem, every apparently dumb change usually have unexpectedly complex implications to handle

Ports configured with specific configuration are not treated by the whole section.

I was wrong, interfaces with specific configuration are treated by this section.

Do you mean that even if ground routing ports have a specific configuration, they should trigger this fix anyway? Are them included into br-lan bridge?

The answer to my question was: no, interfaces configured via ground routing should not be touched by this fix and they will never be included in br-lan.

It all depends on what kind of configuration they got, if they have babled specified as proto they get there too

Interfaces managed by ground routing will never have VLAN ID 0, right? If this is correct, they will never trigger the fix and the problem is solved. Otherwise, how can I check if the interface is also configured for ground routing?

ilario · 2019-10-11T23:59:58Z

I tested these modifications and it works. I think I addressed all the comments.
Now the complete list of protocols of an interface gets passed to each of the protocols scripts, so that the Babeld script can check the presence of the lan proto on the same interface.
This way, the WAN interface will not get touched and neither will the interfaces managed by ground routing.
The code grew to an exaggerated length, I am sorry.
One awful thing is the check for the kind of interface in the last part of the code, where I add a new IPv4 to the interface. Is there a reason and a better way to do it or is it just an OpenWrt bug?
Please review again @G10h4ck

ilario · 2019-10-12T00:02:28Z

@spiccinini could you advise about where to insert the MTU fix from #601?
I mean, if Babeld is running on br-lan and inside of it there are some ethernet interfaces, the br-lan MTU needs to be set or is it already ok?

spiccinini · 2019-10-17T22:53:21Z

@spiccinini could you advise about where to insert the MTU fix from #601?
I mean, if Babeld is running on br-lan and inside of it there are some ethernet interfaces, the br-lan MTU needs to be set or is it already ok?

The fix from #601 should be enough, it lowers the MTU only when using vlans (vid !=0)

spiccinini · 2019-10-17T22:56:43Z

packages/lime-docs/files/lime-example

@@ -51,6 +51,7 @@ config lime network
 	option bmx7_over_batman false
 	option bmx7_pref_gw none			# Force bmx7 to use a specific gateway to Internet (hostname must be used as identifier)
 	option bmx7_wifi_rate_max 'auto'
+	option babeld_over_batman false			# When Babeld is run without VLAN (babeld:0), it runs on the bridge which includes Batman-adv's bat0, keeping this false avoids to have Babeld metrics distorted by Batman-adv


@ilario When someone would want to change from false to true ? If there is not known useful use case I prefer not to add more options.

I agree with you, and I'll remove this.
I added this originally just because the same was present for BMX6 over Batman-adv

ilario · 2019-10-20T21:59:54Z

Replaced by #631

ilario requested review from G10h4ck, aparcar, altergui, nicopace, p4u and spiccinini September 20, 2019 16:45

ilario force-pushed the babeld-no-vlan branch from b6bd1e0 to fd4aaec Compare September 20, 2019 19:26

ilario added 5 commits September 21, 2019 02:16

anygw needs ebtable-ipv6 kernel module

3d23da3

babeld without VLAN runs on br-lan, drop Babeld packets on bat0

80caad6

babeld needs ebtable-ipv6 if batman-adv is installed

3420b51

by default, disable Babeld over Batman

bc78e61

document babeld_over_batman

fbe0bc8

ilario force-pushed the babeld-no-vlan branch from fd4aaec to fbe0bc8 Compare September 21, 2019 00:17

G10h4ck requested changes Sep 21, 2019

View reviewed changes

ilario mentioned this pull request Sep 25, 2019

Babeld without VLAN does not run on LAN ethernet ports #596

Open

gmarcos87 added the in progress label Oct 2, 2019

ilario added 4 commits October 11, 2019 22:59

network.lua pass all deviceProtos to protocols

1e34f93

Split fix in two, check if interface is in br-lan

4b98f6d

added text output to Babeld VLAN 0 fix

070ddce

babeld: use different ifname for different cases

ed74e98

spiccinini reviewed Oct 17, 2019

View reviewed changes

ilario mentioned this pull request Oct 20, 2019

Support Babeld without VLAN on ethernet interfaces inside br-lan, replaces #600 #631

Open

ilario closed this Oct 20, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Babeld without VLAN on ethernet interfaces inside br-lan #600

Support Babeld without VLAN on ethernet interfaces inside br-lan #600

ilario commented Sep 20, 2019 •

edited

Loading

G10h4ck Sep 21, 2019

ilario Sep 21, 2019

ilario Oct 20, 2019

G10h4ck Sep 21, 2019

ilario Sep 21, 2019

G10h4ck Sep 21, 2019

G10h4ck Sep 21, 2019

G10h4ck Sep 21, 2019

ilario Sep 21, 2019

ilario Sep 21, 2019

G10h4ck Sep 21, 2019

G10h4ck Sep 21, 2019 •

edited

Loading

ilario Oct 11, 2019

ilario commented Oct 11, 2019

ilario commented Oct 12, 2019

spiccinini commented Oct 17, 2019

spiccinini Oct 17, 2019

ilario Oct 20, 2019

ilario commented Oct 20, 2019

Support Babeld without VLAN on ethernet interfaces inside br-lan #600

Support Babeld without VLAN on ethernet interfaces inside br-lan #600

Conversation

ilario commented Sep 20, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

G10h4ck Sep 21, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ilario commented Oct 11, 2019

ilario commented Oct 12, 2019

spiccinini commented Oct 17, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ilario commented Oct 20, 2019

ilario commented Sep 20, 2019 •

edited

Loading

G10h4ck Sep 21, 2019 •

edited

Loading