[PATCH 0/6] realtek: fix management of mdb entries

[Vladimir Oltean]

On Sat, Mar 04, 2023 at 01:52:32PM +0300, Arınç ÜNAL wrote:
> On 4.03.2023 00:48, Jan Hoffmann wrote:
> > This series fixes multiple issues related to the L2 table and multicast
> > table. That includes an issue that causes corruption of the port mask
> > for unknown multicast forwarding, which can occur even when multicast
> > snooping is disabled.
> > 
> > With these patches, multicast snooping should be mostly working.
> > However, one important missing piece is forwarding of all multicast
> > traffic to multicast router ports (as specified in section 2.1.2-1 of
> > RFC4541). As far as I can see, this is a general issue that affects all
> > DSA switches, and cannot be fixed without changes to the DSA subsystem.
> 
> Do you plan to discuss this on the netdev mailing list with Vladimir?
> 
> Arınç

I searched for the patches and found them at
https://patchwork.ozlabs.org/project/openwrt/cover/20230303214846.410414-1-jan@3e8.eu/

I guess that what should be discussed is the topic of switch ports
attached to multicast routers, yes?

DSA does not (and has never) listen(ed) to the switchdev notifications
emitted for bridge ports regarding multicast routers (SWITCHDEV_ATTR_ID_PORT_MROUTER).
It has only listened for a while to the switchdev notifications for the
bridge itself as a multicast router (SWITCHDEV_ATTR_ID_BRIDGE_MROUTER),
and even that was done for a fairly strange reason and eventually got
reverted for breaking something - see commits 08cc83cc7fd8 and c73c57081b3d.

I personally don't have use cases for IP multicast routing / snooping,
so I would need some guidance regarding what is needed for things to
work smoothly. Also, to make sure that they keep working in the future,
one of the tests from tools/testing/selftests/net/forwarding/ which
exercises flooding towards multicast ports (if it exists) should be
symlinked to tools/testing/selftests/drivers/net/dsa/ and adapted until
it works. That's a pretty good way to get maintainers' attention on a
feature that they don't normally test.

It's not the first time I'm reading RFC4541, but due to a lack of any
practical applications surrounding me (and therefore also partial lack
of understanding), I keep forgetting what it says :)

Section 2.1.1.  IGMP Forwarding Rules (for the control path) says

   1) A snooping switch should forward IGMP Membership Reports only to
      those ports where multicast routers are attached.

how? I guess IGMP/MLD packets should reach the CPU via packet traps
(which cause skb->offload_fwd_mark to be unset), and from there, the
bridge software data path identifies the mrouter ports and forwards
control packets only there? What happens if the particular switch
hardware doesn't support IGMP/MLD packet identification and trapping?
Should the driver install a normal multicast forwarding rule for
all 224.0.0.X traffic (translated to MAC), and patch the tagging
protocol driver to set skb->offload_fwd_mark = 0 based on
eth_hdr(skb)->h_dest?

Then for the data path we have:

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.

      This is the main IGMP snooping functionality for the data path.
      One approach that an implementation could take would be to
      maintain separate membership and multicast router tables in
      software and then "merge" these tables into a forwarding cache.

For my clarity, this means that *all* IP multicast packets must be
forwarded to the multicast router ports, be their addresses known
(bridge mdb entries exist for them) or unknown (flooded)?

What does the software bridge implementation do? Does it perform this
"merging" of tables for us? (for known MDB entries, does it also notify
them through switchdev on the mrouter ports?) Looking superficially at
the first-order callers of br_mdb_notify(), I don't get the impression
that the bridge has this logic?

Or am I completely off with my reading of RFC4541?

It's not obvious to me, after looking at the few implementations of
SWITCHDEV_ATTR_ID_PORT_MROUTER handlers in drivers, that this merging of
forwarding tables would be done anywhere within sight.

If someone could explain to me what are the expectations (we don't seem
to have a lot of good documentation) and how do the existing drivers work,
we can start discussing more concretely how the layering and API within
DSA should look like.

As a way to fix a bug quickly and get correct behavior, I guess there's
also the option of stopping to process multicast packets in hardware,
and configure the switch to always send any multicast to the CPU port
only. As long as the tagger knows to leave skb->offload_fwd_mark unset,
the bridge driver should know how to deal with those packets in
software, and forward them only to whom is interested. But the drawback
is that there is no forwarding acceleration involved. Maybe DSA should
have done that from the get go for drivers which didn't care about
multicast in particular, instead of ending up with this current situation
which appears to be slightly chaotic.