On 8/1/19 1:49 AM, Nikolay Aleksandrov wrote: [snip] > [0] https://bugzilla.kernel.org/show_bug.cgi?id=204389 > > Reported-by: michael-dev <michael-dev@xxxxxxxxxxxxx> > Fixes: 5be5a2df40f0 ("bridge: Add filtering support for default_pvid") > Signed-off-by: Nikolay Aleksandrov <nikolay@xxxxxxxxxxxxxxxxxxx> > --- > I tried a few different approaches to resolve this but they were all > unsuitable for some kernels, this approach can go to stables easily > and IMO is the way this had to be done from the start. Alternatively > we could move only the br_vlan_add and pair it with a br_vlan_del of > default_pvid on the same events, but I don't think it hurts to move > the whole init/deinit there as it'd help older stable releases as well. > > I also tested the br_vlan_init error handling after the move by always > returning errors from all over it. Since errors at NETDEV_REGISTER cause > NETDEV_UNREGISTER we can deinit vlans properly for all cases regardless > why it happened (e.g. device destruction or init error). > > net/bridge/br.c | 5 ++++- > net/bridge/br_device.c | 10 ---------- > net/bridge/br_private.h | 20 +++++--------------- > net/bridge/br_vlan.c | 25 ++++++++++++++++++------- > 4 files changed, 27 insertions(+), 33 deletions(-) > Self-NAK, after thinking more about how to best handle this and running more tests I believe it'll be better to go with the alternative I suggested above - to move out only the default pvid add out of br_vlan_init to NETDEV_REGSITER and pair it with br_vlan_delete in NETDEV_UNREGISTER. That way we'll split the init/deinit in 2 steps, but we'll keep the current order and will reduce the churn for this fix, functionally it should be equivalent as that is the problematic part of the init which has to be done after the netdev has been registered. I'll spin v4 tomorrow after running more tests with it.
