Re: [RFC PATCH v2 net-next 06/17] net: dsa: add addresses obtained from RX filtering to host addresses

[Tobias Waldekranz <tobias@waldekranz.com>]

On Wed, Feb 24, 2021 at 13:43, Vladimir Oltean <olteanv@gmail.com> wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
>
> In case we have ptp4l running on a bridged DSA switch interface, the PTP
> traffic is classified as link-local (in the default profile, the MAC
> addresses are 01:1b:19:00:00:00 and 01:80:c2:00:00:0e), which means it
> isn't the responsibility of the bridge to make sure it gets trapped to
> the CPU.
>
> The solution is to implement the standard callbacks for dev_uc_add and
> dev_mc_add, and behave just like any other network interface: ensure
> that the user space program can see those packets.

So presumably the application would use PACKET_ADD_MEMBERSHIP to set
this up?

This is a really elegant way of solving this problem I think!

One problem I see is that this will not result in packets getting
trapped to the CPU, rather they will simply be forwarded.  I.e. with
this patch applied, once ptp4l adds the groups it is interested in, my
HW FDB will look like this:

ADDR                VID  DST   TYPE
01:1b:19:00:00:00     0  cpu0  static
01:80:c2:00:00:0e     0  cpu0  static

But this will not allow these groups to ingress on (STP) blocked
ports. AFAIK, PTP (certainly LLDP which also uses the latter group)
should be able to do that.

For mv88e6xxx (but I think this applies to most switches), there are
roughly three ways a given multicast group can reach the CPU:

1. Trap: Packet is unconditionally redirected to the CPU, independent
   of things like 802.1X or STP state on the ingressing port.
2. Mirror: Send a copy of packets that pass all other ingress policy to
   the CPU.
3. Forward: Forward packets that pass all other ingress policy to the
   CPU.

Entries are now added as "Forward", which means that the group will no
longer reach the other local ports. But the command from the application
is "I want to see these packets", it says nothing about preventing the
group from being forwarded. So I think the default ought to be
"Mirror". Additionally, we probably need some way of specifying "Trap"
to those applications that need it. E.g. ptp4l could specify
PACKET_MR_MULTICAST_TRAP in mr_action or something if it does not want
the bridge (or the switch) to forward it.

If "Forward" is desired, the existing "bridge mdb" interface seems like
the proper one, since it also affects other ports.