On 02/08/2019 18:35, Stephen Hemminger wrote: > On Fri, 2 Aug 2019 13:57:36 +0300 > Nikolay Aleksandrov <[email protected]> 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; > } > >
sure, this is ok for net-next
