Re: [PATCH net-next 4/4] bridge: update vlan dev state when port added to or deleted from vlan

[Nikolay Aleksandrov <nikolay@cumulusnetworks.com>]

On 02/04/2019 18:35, Mike Manning wrote:
> If vlan bridge binding is enabled, then the link state of a vlan device
> that is an upper device of the bridge should track the state of bridge
> ports that are members of that vlan. So if a bridge port becomes or
> stops being a member of a vlan, then update the link state of the
> vlan device if necessary.
> 
> Signed-off-by: Mike Manning <mmanning@vyatta.att-mail.com>
> ---
>  net/bridge/br_vlan.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
> index 642373231386..7c11607cf1f4 100644
> --- a/net/bridge/br_vlan.c
> +++ b/net/bridge/br_vlan.c
> @@ -7,6 +7,9 @@
>  #include "br_private.h"
>  #include "br_private_tunnel.h"
>  
> +static void nbp_vlan_set_vlan_dev_state(struct net_bridge_port *p,
> +					struct net_bridge *br, u16 vid);
> +
>  static inline int br_vlan_cmp(struct rhashtable_compare_arg *arg,
>  			      const void *ptr)
>  {
> @@ -294,6 +297,9 @@ static int __vlan_add(struct net_bridge_vlan *v, u16 flags,
>  
>  	__vlan_add_list(v);
>  	__vlan_add_flags(v, flags);
> +
> +	if (p)
> +		nbp_vlan_set_vlan_dev_state(p, br, v->vid);

since you need this as a last action after vlan_add and acts only on ports
why not move it to nbp_vlan_add() ?

>  out:
>  	return err;
>  
> @@ -358,6 +364,8 @@ static int __vlan_del(struct net_bridge_vlan *v)
>  		rhashtable_remove_fast(&vg->vlan_hash, &v->vnode,
>  				       br_vlan_rht_params);
>  		__vlan_del_list(v);
> +		if (p)
> +			nbp_vlan_set_vlan_dev_state(p, p->br, v->vid);

p is guaranteed to be set here, but also there's an if (p) earlier in the function.
this can probably also move to nbp_vlan_delete if the other set_state moves to 
nbp_vlan_add

>  		call_rcu(&v->rcu, nbp_vlan_rcu_free);
>  	}
>  
> @@ -1357,6 +1365,21 @@ static void br_vlan_set_vlan_dev_state(struct net_bridge *br,
>  	}
>  }
>  
> +static void nbp_vlan_set_vlan_dev_state(struct net_bridge_port *p,
> +					struct net_bridge *br, u16 vid)
> +{
> +	struct net_device *vlan_dev;
> +
> +	if (!br->vlan_bridge_binding)
> +		return;
> +
> +	vlan_dev = br_vlan_get_upper_bind_vlan_dev(br->dev, vid);
> +	if (vlan_dev) {
> +		br_vlan_set_vlan_dev_state(br, vlan_dev);
> +		dev_put(vlan_dev);

I think this is running under rtnl.

> +	}
> +}
> +
>  static void br_vlan_set_all_vlan_dev_state(struct net_bridge_port *p,
>  					   struct net_bridge *br)
>  {
>