On 02/08/2019 17:07, Allan W. Nielsen wrote: > The 08/01/2019 17:07, Nikolay Aleksandrov wrote: >>> To create a group for two of the front ports the following entries can >>> be added: >>> bridge mdb add dev br0 port eth0 grp 01:00:00:00:00:04 permanent vid 1 >>> bridge mdb add dev br0 port eth1 grp 01:00:00:00:00:04 permanent vid 1 >>> >>> Now the entries will be display as following: >>> dev br0 port eth0 grp 01:00:00:00:00:04 permanent offload vid 1 >>> dev br0 port eth1 grp 01:00:00:00:00:04 permanent offload vid 1 >>> >>> This requires changes to iproute2 as well, see the follogin patch for that. >>> >>> Now if frame with dmac '01:00:00:00:00:04' will arrive at one of the front >>> ports. If we have HW offload support, then the frame will be forwarded by >>> the switch, and need not to go to the CPU. In a pure SW world, the frame is >>> forwarded by the SW bridge, which will flooded it only the ports which are >>> part of the group. >>> >>> So far so good. This is an important part of the problem we wanted to solve. >>> >>> But, there is one drawback of this approach. If you want to add two of the >>> front ports and br0 to receive the frame then I can't see a way of doing it >>> with the bridge mdb command. To do that it requireds many more changes to >>> the existing code. >>> >>> Example: >>> bridge mdb add dev br0 port eth0 grp 01:00:00:00:00:04 permanent vid 1 >>> bridge mdb add dev br0 port eth1 grp 01:00:00:00:00:04 permanent vid 1 >>> bridge mdb add dev br0 port br0 grp 01:00:00:00:00:04 permanent vid 1 // This looks wrong. >>> >>> We believe we come a long way by re-using the facilities in MDB (thanks for >>> convincing us in doing this), but we are still not completely happy with >>> the result. >> Just add self argument for the bridge mdb command, no need to specify it twice. > Like this: > bridge mdb add dev br0 port eth1 grp 01:00:00:00:00:04 permanent vid self What ?! No, that is not what I meant. bridge mdb add dev br0 grp 01:00:00:00:00:04 permanent vid self bridge mdb del dev br0 grp 01:00:00:00:00:04 permanent vid self Similar to fdb. You don't need no-self.. I don't mind a different approach, this was just a suggestion. But please without "no-self" :) > > Then if I want to remove br0 rom the group, should I then have a no-self, and > then it becomes even more strange what to write in the port. > > bridge mdb add dev br0 port ?? grp 01:00:00:00:00:04 permanent vid no-self > ^^ > And, what if it is a group with only br0 (the traffic should go to br0 and > not any of the slave interfaces)? > > Also, the 'self' keyword has different meanings in the 'bridge vlan' and the > 'bridge fdb' commands where it refres to if the offload rule should be install > in HW - or only in the SW bridge. No, it shouldn't. Self means act on the device, in this case act on the bridge, otherwise master is assumed. > > The proposed does not look pretty bad, but at least it will be possible to > configured the different scenarios: > > bridge mdb add dev br0 port br0 grp 01:00:00:00:00:04 permanent vid 1 > bridge mdb del dev br0 port br0 grp 01:00:00:00:00:04 permanent vid 1 > That works too, but the "port" part is redundant. > The more I look at the "bridge mdb { add | del } dev DEV port PORT" command, the > less I understand why do we have both 'dev' and 'port'? The implementation will > only allow this if 'port' has become enslaved to the switch represented by > 'dev'. Anyway, what is done is done, and we need to stay backwards compatible, > but we could make it optional, and then it looks a bit less strange to allow the > port to specify a br0. > > Like this: > > bridge mdb { add | del } [dev DEV] port PORT grp GROUP [ permanent | temp ] [ vid VID ] > > bridge mdb add port eth0 grp 01:00:00:00:00:04 permanent vid 1 > bridge mdb add port eth1 grp 01:00:00:00:00:04 permanent vid 1 > bridge mdb add port br0 grp 01:00:00:00:00:04 permanent vid 1 // Add br0 to the gruop > bridge mdb del port br0 grp 01:00:00:00:00:04 permanent vid 1 // Delete it again > br0 is not a port, thus the "self" or just dev or whatever you choose.. > Alternative we could also make the port optional: > > bridge mdb { add | del } dev DEV [port PORT] grp GROUP [ permanent | temp ] [ vid VID ] > > bridge mdb add dev br0 port eth0 grp 01:00:00:00:00:04 permanent vid 1 > bridge mdb add dev br0 port eth1 grp 01:00:00:00:00:04 permanent vid 1 > bridge mdb add dev br0 grp 01:00:00:00:00:04 permanent vid 1 // Add br0 to the gruop > bridge mdb del dev br0 grp 01:00:00:00:00:04 permanent vid 1 // Delete it again > Right. I read this one later. :) > Any preferences? > Not really, up to you. Any of the above seem fine to me. > /Allan > >