On Sat, Nov 28, 2020 at 00:27, Vladimir Oltean <vladimir.olt...@nxp.com> wrote: > On Sat, Nov 28, 2020 at 12:58:10AM +0100, Tobias Waldekranz wrote: >> That sounds like a good idea. We have run into another issue with the >> MDB that maybe could be worked into this changeset. This is what we have >> observed on 4.19, but from looking at the source it does not look like >> anything has changed with respect to this issue. >> >> The DSA driver handles the addition/removal of router ports by >> enabling/disabling multicast flooding to the port in question. On >> mv88e6xxx at least, this is only part of the solution. It only takes >> care of the unregistered multicast. You also have to iterate through all >> _registered_ groups and add the port to the destination vector. > > And this observation is based on what? Based on this paragraph from RFC4541?
Well in all honesty, it is mostly based on information from a colleague of mine who knows the ins and outs of all these RFCs. > 2.1.2. Data Forwarding Rules > > 1) Packets with a destination IP address outside 224.0.0.X which are > not IGMP should be forwarded according to group-based port > membership tables and must also be forwarded on router ports. ...but yes, that paragraph does not leave a lot of wiggle room :) > Let me ask you a different question. Why would DSA be in charge of > updating the MDB records, and not the bridge? Or why DSA and not the end > driver? Ignore my patch. I'm just trying to understand what you're > saying. Why precisely DSA, the mid layer? I don't know, this is new > information to me, I'm still digesting it. The bridge has all the necessary information for sure. But it has a different model with a separate list of router ports. Then in br_multicast_flood you simply forward to the union of the group entry and the router ports. It is not the bridge's fault that our hardware does not have a separate bitmask for router ports. Some hardware may very well have it. I guess we could create internal APIs to the bridge to retrieve the information though. There is already `bool br_multicast_router(dev)`, so we should only need to add `bool br_multicast_member(dev, group, vid)`. Assuming that those were available we should be able to solve it either at the DSA or the driver layer. I seem to recall some issue that forced us to place the cache at the dst level, but I would have to go through the implementation to figure out what that issue was.
