• Vladimir Oltean's avatar
    net: bridge: switchdev: replay all VLAN groups · b28d580e
    Vladimir Oltean authored
    The major user of replayed switchdev objects is DSA, and so far it
    hasn't needed information about anything other than bridge port VLANs,
    so this is all that br_switchdev_vlan_replay() knows to handle.
    
    DSA has managed to get by through replicating every VLAN addition on a
    user port such that the same VLAN is also added on all DSA and CPU
    ports, but there is a corner case where this does not work.
    
    The mv88e6xxx DSA driver currently prints this error message as soon as
    the first port of a switch joins a bridge:
    
    mv88e6085 0x0000000008b96000:00: port 0 failed to add a6:ef:77:c8:5f:3d vid 1 to fdb: -95
    
    where a6:ef:77:c8:5f:3d vid 1 is a local FDB entry corresponding to the
    bridge MAC address in the default_pvid.
    
    The -EOPNOTSUPP is returned by mv88e6xxx_port_db_load_purge() because it
    tries to map VID 1 to a FID (the ATU is indexed by FID not VID), but
    fails to do so. This is because ->port_fdb_add() is called before
    ->port_vlan_add() for VID 1.
    
    The abridged timeline of the calls is:
    
    br_add_if
    -> netdev_master_upper_dev_link
       -> dsa_port_bridge_join
          -> switchdev_bridge_port_offload
             -> br_switchdev_vlan_replay (*)
             -> br_switchdev_fdb_replay
                -> mv88e6xxx_port_fdb_add
    -> nbp_vlan_init
       -> nbp_vlan_add
          -> mv88e6xxx_port_vlan_add
    
    and the issue is that at the time of (*), the bridge port isn't in VID 1
    (nbp_vlan_init hasn't been called), therefore br_switchdev_vlan_replay()
    won't have anything to replay, therefore VID 1 won't be in the VTU by
    the time mv88e6xxx_port_fdb_add() is called.
    
    This happens only when the first port of a switch joins. For further
    ports, the initial mv88e6xxx_port_vlan_add() is sufficient for VID 1 to
    be loaded in the VTU (which is switch-wide, not per port).
    
    The problem is somewhat unique to mv88e6xxx by chance, because most
    other drivers offload an FDB entry by VID, so FDBs and VLANs can be
    added asynchronously with respect to each other, but addressing the
    issue at the bridge layer makes sense, since what mv88e6xxx requires
    isn't absurd.
    
    To fix this problem, we need to recognize that it isn't the VLAN group
    of the port that we're interested in, but the VLAN group of the bridge
    itself (so it isn't a timing issue, but rather insufficient information
    being passed from switchdev to drivers).
    
    As mentioned, currently nbp_switchdev_sync_objs() only calls
    br_switchdev_vlan_replay() for VLANs corresponding to the port, but the
    VLANs corresponding to the bridge itself, for local termination, also
    need to be replayed. In this case, VID 1 is not (yet) present in the
    port's VLAN group but is present in the bridge's VLAN group.
    
    So to fix this bug, DSA is now obligated to explicitly handle VLANs
    pointing towards the bridge in order to "close this race" (which isn't
    really a race). As Tobias Waldekranz notices, this also implies that it
    must explicitly handle port VLANs on foreign interfaces, something that
    worked implicitly before:
    https://patchwork.kernel.org/project/netdevbpf/patch/20220209213044.2353153-6-vladimir.oltean@nxp.com/#24735260
    
    So in the end, br_switchdev_vlan_replay() must replay all VLANs from all
    VLAN groups: all the ports, and the bridge itself.
    Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    b28d580e
br_switchdev.c 18.1 KB