On Fri, 2 Aug 2019 13:57:36 +0300 Nikolay Aleksandrov <nikolay@xxxxxxxxxxxxxxxxxxx> wrote: > +int br_vlan_bridge_event(struct net_device *dev, unsigned long event, void *ptr) > { > struct netdev_notifier_changeupper_info *info; > - struct net_bridge *br; > + struct net_bridge *br = netdev_priv(dev); > + bool changed; > + int ret = 0; > > switch (event) { > + case NETDEV_REGISTER: > + ret = br_vlan_add(br, br->default_pvid, > + BRIDGE_VLAN_INFO_PVID | > + BRIDGE_VLAN_INFO_UNTAGGED | > + BRIDGE_VLAN_INFO_BRENTRY, &changed, NULL); > + break; Looks good. As minor optimization br_vlan_add could ignore changed pointer if NULL. This would save places where you don't care. Something like: diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c index 021cc9f66804..bacd3543b215 100644 --- a/net/bridge/br_vlan.c +++ b/net/bridge/br_vlan.c @@ -626,10 +626,11 @@ static int br_vlan_add_existing(struct net_bridge *br, refcount_inc(&vlan->refcnt); vlan->flags |= BRIDGE_VLAN_INFO_BRENTRY; vg->num_vlans++; - *changed = true; + if (changed) + *changed = true; } - if (__vlan_add_flags(vlan, flags)) + if (__vlan_add_flags(vlan, flags) && changed) *changed = true; return 0; @@ -653,7 +654,8 @@ int br_vlan_add(struct net_bridge *br, u16 vid, u16 flags, bool *changed, ASSERT_RTNL(); - *changed = false; + if (changed) + *changed = false; vg = br_vlan_group(br); vlan = br_vlan_find(vg, vid); if (vlan) @@ -679,7 +681,7 @@ int br_vlan_add(struct net_bridge *br, u16 vid, u16 flags, bool *changed, if (ret) { free_percpu(vlan->stats); kfree(vlan); - } else { + } else if (changed) { *changed = true; }