• Nikolay Aleksandrov's avatar
    net: bridge: move default pvid init/deinit to NETDEV_REGISTER/UNREGISTER · 091adf9b
    Nikolay Aleksandrov authored
    Most of the bridge device's vlan init bugs come from the fact that its
    default pvid is created at the wrong time, way too early in ndo_init()
    before the device is even assigned an ifindex. It introduces a bug when the
    bridge's dev_addr is added as fdb during the initial default pvid creation
    the notification has ifindex/NDA_MASTER both equal to 0 (see example below)
    which really makes no sense for user-space[0] and is wrong.
    Usually user-space software would ignore such entries, but they are
    actually valid and will eventually have all necessary attributes.
    It makes much more sense to send a notification *after* the device has
    registered and has a proper ifindex allocated rather than before when
    there's a chance that the registration might still fail or to receive
    it with ifindex/NDA_MASTER == 0. Note that we can remove the fdb flush
    from br_vlan_flush() since that case can no longer happen. At
    NETDEV_REGISTER br->default_pvid is always == 1 as it's initialized by
    br_vlan_init() before that and at NETDEV_UNREGISTER it can be anything
    depending why it was called (if called due to NETDEV_REGISTER error
    it'll still be == 1, otherwise it could be any value changed during the
    device life time).
    
    For the demonstration below a small change to iproute2 for printing all fdb
    notifications is added, because it contained a workaround not to show
    entries with ifindex == 0.
    Command executed while monitoring: $ ip l add br0 type bridge
    Before (both ifindex and master == 0):
    $ bridge monitor fdb
    36:7e:8a:b3:56:ba dev * vlan 1 master * permanent
    
    After (proper br0 ifindex):
    $ bridge monitor fdb
    e6:2a:ae:7a:b7:48 dev br0 vlan 1 master br0 permanent
    
    v4: move only the default pvid init/deinit to NETDEV_REGISTER/UNREGISTER
    v3: send the correct v2 patch with all changes (stub should return 0)
    v2: on error in br_vlan_init set br->vlgrp to NULL and return 0 in
        the br_vlan_bridge_event stub when bridge vlans are disabled
    
    [0] https://bugzilla.kernel.org/show_bug.cgi?id=204389Reported-by: default avatarmichael-dev <michael-dev@fami-braun.de>
    Fixes: 5be5a2df ("bridge: Add filtering support for default_pvid")
    Signed-off-by: default avatarNikolay Aleksandrov <nikolay@cumulusnetworks.com>
    Acked-by: default avatarRoopa Prabhu <roopa@cumulusnetworks.com>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    091adf9b
br_vlan.c 33.9 KB