gluon icon indicating copy to clipboard operation
gluon copied to clipboard

gluon-core: enable bridge port isolation for br-mesh_other interfaces

Open herbetom opened this issue 2 years ago • 5 comments

This is an alternative approach to #2484.

It sets the isolate option for all mesh devices via uci and let's netifd handle the low level stuff.

The script is run on every gluon-reconfigure.

Where it should and shouldn't work:

Isolation should be enabled when there are multiple (non-uplink) mesh Interfaces.

For devices which were already ported to DSA isolation is enabled for all indiviual Ports on that device.

For devices which are still configured via swconfig isolation between indivial physical ports isn't possible.

On all devices isolation should be enabled between mesh VLANs and of course between those VLANs and phyiscal mesh Ports.

Commands

Show whether it's activated:

grep ^ /sys/devices/virtual/net/br-mesh*/brif/*/isolated

1 means it's enabled fo that interface, 0 that it's diabled.

herbetom avatar Aug 07 '22 19:08 herbetom

Thanks for picking this this up. I haven't got my DSA test device working again. I also don't know how I was able to miss that it is possible to set the flag via netifd. The last time I looked through the sources I could find anything but it is there since 2018...

belzebub40k avatar Aug 08 '22 06:08 belzebub40k

I've marked it as ready for now.

What i don't like about the current implementation is that it's not checked whether there already is a device section with the corresponding ifname as name. It shouldn't be a problem since the only possible conflict could come from within our network config generation scripts. It's just not "nice". My "problem" with implementing that is that I don't see a simple-uci function for doing such a lookup and it's (for now) just not necessary and only a source for bugs.

Currently it's still unclear if isolation will work on all platforms it theoretically should work on (bugs). Hoewer, from my perspective merging this is fine as long as it doesn't break networking on some device. If isolation just doesn't work that's in my perspective not a deal breaker.

herbetom avatar Aug 09 '22 10:08 herbetom

As a data point we can see that an Edgerouter-X at Freifunk Hannover has bridge port isolation enable on all bridge ports with no effect at all

root@UFU-FWH-E107-Helmke-Technik2:~# grep ^ /sys/devices/virtual/net/br-mesh*/brif/*/isolated
/sys/devices/virtual/net/br-mesh_other/brif/eth1/isolated:1
/sys/devices/virtual/net/br-mesh_other/brif/eth2/isolated:1
/sys/devices/virtual/net/br-mesh_other/brif/eth3/isolated:1
/sys/devices/virtual/net/br-mesh_other/brif/eth4/isolated:1

https://hannover.freifunk.net/karte/#/en/map/74acb96c84fe

One suspicion is that on the ER-X we're seeing the effects of hardware-offloaded bridge port forwarding

mweinelt avatar Sep 12 '22 21:09 mweinelt

As already mentioned on IRC I experienced the same behavior with my ER-X-SFP after I upgraded my node to a firmware based on OpenWrt 22.03. It was working fine when the master branch was still on OpenWrt 21.02. I wasn't able to downgrade my node as saving the config did not work after flashing an older firmware but I also thought that some hardware offloading features were introduced in the switch driver between 21.02 and 22.03.

belzebub40k avatar Sep 13 '22 07:09 belzebub40k

Ideally we'd need someone to confirm by going through the offloading capabilities (ethtool -k) and trying to disable them (ethtool -K) to wait and see if one of them affects the bridge port forwarding.

But this would then also just mean that we set up a known issue and leave it at that, unless someone wants to dig deep here.

mweinelt avatar Sep 13 '22 10:09 mweinelt

I don't think blocking on feedback for the er-x is a good idea, we can create an issue for that. Otherwise this is an obviously good idea that has been in production in Darmstadt and Hannover for a while now, so I'm going for the merge.

mweinelt avatar Oct 18 '22 08:10 mweinelt

No objection, just wanted to add; in hanover we haven't seen a working isolation yet; but tested whether we'd encounter the named issue with the er-x: we did.

AiyionPrime avatar Oct 18 '22 09:10 AiyionPrime

Successfully created backport PR #2680 for v2022.1.x.

github-actions[bot] avatar Oct 18 '22 10:10 github-actions[bot]