On 02/08/2019 18:35, Stephen Hemminger wrote:
> 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;
> }
>
>
sure, this is ok for net-next
