diff options
author | Nikolay Aleksandrov <nikolay@cumulusnetworks.com> | 2019-08-02 13:57:36 +0300 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2019-08-05 23:32:53 +0300 |
commit | 091adf9ba6cdb432cbcc217b47e4ffb8aa0d8865 (patch) | |
tree | ce8a98f894226b07ece01e485e505d4d22b1f6fa /net/bridge/br.c | |
parent | d934423ac26ed373dfe089734d505dca5ff679b6 (diff) | |
download | linux-091adf9ba6cdb432cbcc217b47e4ffb8aa0d8865.tar.xz |
net: bridge: move default pvid init/deinit to NETDEV_REGISTER/UNREGISTER
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=204389
Reported-by: michael-dev <michael-dev@fami-braun.de>
Fixes: 5be5a2df40f0 ("bridge: Add filtering support for default_pvid")
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'net/bridge/br.c')
-rw-r--r-- | net/bridge/br.c | 5 |
1 files changed, 4 insertions, 1 deletions
diff --git a/net/bridge/br.c b/net/bridge/br.c index d164f63a4345..8a8f9e5f264f 100644 --- a/net/bridge/br.c +++ b/net/bridge/br.c @@ -37,12 +37,15 @@ static int br_device_event(struct notifier_block *unused, unsigned long event, v int err; if (dev->priv_flags & IFF_EBRIDGE) { + err = br_vlan_bridge_event(dev, event, ptr); + if (err) + return notifier_from_errno(err); + if (event == NETDEV_REGISTER) { /* register of bridge completed, add sysfs entries */ br_sysfs_addbr(dev); return NOTIFY_DONE; } - br_vlan_bridge_event(dev, event, ptr); } /* not a port of a bridge */ |