Date:   Thu, 14 May 2020 18:27:13 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 0/4] DSA: promisc on master, generic flow
 dissector code

Hi Florian,

On Tue, 12 May 2020 at 03:03, Florian Fainelli <f.fainelli@...il.com> wrote:
>
>
>
> On 5/11/2020 4:52 PM, Vladimir Oltean wrote:
> > On Tue, 12 May 2020 at 02:28, Florian Fainelli <f.fainelli@...il.com> wrote:
> >>
> >>
> >>
> >> On 5/11/2020 1:20 PM, Vladimir Oltean wrote:
> >>> From: Vladimir Oltean <vladimir.oltean@....com>
> >>>
> >>> The initial purpose of this series was to implement the .flow_dissect
> >>> method for sja1105 and for ocelot/felix. But on Felix this posed a
> >>> problem, as the DSA headers were of different lengths on RX and on TX.
> >>> A better solution than to just increase the smaller one was to also try
> >>> to shrink the larger one, but in turn that required the DSA master to be
> >>> put in promiscuous mode (which sja1105 also needed, for other reasons).
> >>>
> >>> Finally, we can add the missing .flow_dissect methods to ocelot and
> >>> sja1105 (as well as generalize the formula to other taggers as well).
> >>
> >> On a separate note, do you have any systems for which it would be
> >> desirable that the DSA standalone port implemented receive filtering? On
> >> BCM7278 devices, the Ethernet MAC connected to the switch is always in
> >> promiscuous mode, and so we rely on the switch not to flood the CPU port
> >> unnecessarily with MC traffic (if nothing else), this is currently
> >> implemented in our downstream kernel, but has not made it upstream yet,
> >> previous attempt was here:
> >>
> >> https://www.spinics.net/lists/netdev/msg544361.html
> >>
> >> I would like to revisit that at some point.
> >> --
> >> Florian
> >
> > Yes, CPU filtering of traffic (not just multicast) is one of the
> > problems we're facing. I'll take a look at your patches and maybe I'll
> > pick them up.
>
> The part that modifies DSA to program the known MC addresses should
> still be largely applicable, there were essentially two problems that I
> was facing, which could be tackled separately.
>
> 1) flooding of unknown MC traffic on DSA standalone ports is not very
> intuitive if you come from NICs that did filtering before. We should
> leverage a DSA switch driver's ability to support port_egress_floods and
> support port_mdb_add and combine them to avoid flooding the CPU port.
>

Could you clarify one thing for me:
- SWITCHDEV_ATTR_ID_BRIDGE_MC_DISABLED is supposed to sync/unsync all
known multicast {mac, vid} addresses to the hardware filtering table
- SWITCHDEV_ATTR_ID_BRIDGE_MROUTER is supposed to toggle flooding of
unknown multicast addresses to the CPU
- What is BR_MCAST_FLOOD from SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS
supposed to do? The same thing?

> 2) Programming of known multicast addresses for VLAN devices on top of
> DSA standalone ports while the switch implements global VLAN filtering.
> In that case when we get to the DSA slave device's ndo_set_rx_mode() we
> have lost all information about which VID the MAC address is coming from
> so we cannot insert the MAC address with the correct VID to support
> proper filtering. TI's cpsw driver implements a super complicated scheme
> to solve that problem and this was worked on by Ivan in a more generic
> and usable form: https://lwn.net/Articles/780783/

So you're thinking of pulling his vlan_dev_get_addr_vid API and
syncing {DMAC, VID} addresses to the DSA slave ports by installing mdb
rules (or fdb in case of unicast filtering) on the CPU port?

But I think the only thing that would make global VLAN filtering more
complicated than the general case is that we would have to:
- deny switch ports from being enslaved to a VLAN-unaware bridge if
there is at least one other DSA slave in this switch that has
addresses with a non-pvid VLAN in its RX filter
- deny changes to the VLAN filtering state of any bridge that spans a
switch which has at least a port with a non-pvid VLAN in its RX filter
Am I missing something?

Also, for unicast filtering, I believe you would agree to leverage
port_egress_floods(unicast=false), and have the DSA core install one
fdb entry on the CPU port per each slave netdevice MAC address. Sadly,
for that to work, we'd have to keep our own parallel reference
counting in dsa_slave_sync_unsync_fdb_addr (because the refcount in
__hw_addr_sync_dev is per slave device and not per cpu port).

Last but not least, how do you see the CPU membership of VLANs problem
being approached? Using Ivan's vlan_dev_get_addr_vid API, only let the
CPU see traffic from a particular VLAN if there is any upper 8021q net
device installed on any DSA slave? What should happen when we bridge a
DSA slave port? Let the flood gates open? If the DSA slave is bridged
only with other DSA slaves from the same switch, hopefully the answer
is no. Hopefully we can open the flood gates only when bridging with a
foreign interface.

> --
> Florian

Thanks,
-Vladimir

Powered by blists - more mailing lists