Re: [Bridge] [PATCH 6/7] bridge: Manage promisc mode when vlans are configured on top of a bridge

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Feb 26, 2014 at 10:18:24AM -0500, Vlad Yasevich wrote:
> If the user configures vlan interfaces on top of the bridge and the bridge
> doesn't have vlan filtering enabled, we have to place all the ports in
> promsic mode so that we can correctly receive tagged frames.
> When vlan filtering is enabled, the vlan configuration will be provided
> via filtering interface.
> When the vlan filtering is toggled, we also have mange promiscuity.

have to manage?

> 
> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>


I wonder if it matters that we scan all ports
on vlan add/del now.

If yes, we could optimize some cases by using
a counter of promisc ports.


> ---
>  net/bridge/br_device.c  | 14 ++++++++++++++
>  net/bridge/br_if.c      | 17 +++++++++++++----
>  net/bridge/br_private.h |  9 +++++++++
>  net/bridge/br_vlan.c    |  1 +
>  4 files changed, 37 insertions(+), 4 deletions(-)
> 
> diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
> index 0af9d6c..967abb3 100644
> --- a/net/bridge/br_device.c
> +++ b/net/bridge/br_device.c
> @@ -297,6 +297,18 @@ void br_netpoll_disable(struct net_bridge_port *p)
>  
>  #endif
>  
> +static int br_dev_rx_add_vid(struct net_device *br_dev, __be16 proto, u16 vid)
> +{
> +	br_manage_promisc(netdev_priv(br_dev));
> +	return 0;
> +}
> +
> +static int br_dev_rx_kill_vid(struct net_device *br_dev, __be16 proto, u16 vid)
> +{
> +	br_manage_promisc(netdev_priv(br_dev));
> +	return 0;
> +}
> +
>  static int br_add_slave(struct net_device *dev, struct net_device *slave_dev)
>  
>  {
> @@ -328,6 +340,8 @@ static const struct net_device_ops br_netdev_ops = {
>  	.ndo_change_rx_flags	 = br_dev_change_rx_flags,
>  	.ndo_change_mtu		 = br_change_mtu,
>  	.ndo_do_ioctl		 = br_dev_ioctl,
> +	.ndo_vlan_rx_add_vid	 = br_dev_rx_add_vid,
> +	.ndo_vlan_rx_kill_vid	 = br_dev_rx_kill_vid,
>  #ifdef CONFIG_NET_POLL_CONTROLLER
>  	.ndo_netpoll_setup	 = br_netpoll_setup,
>  	.ndo_netpoll_cleanup	 = br_netpoll_cleanup,
> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
> index 7e92bd0..55e4e28 100644
> --- a/net/bridge/br_if.c
> +++ b/net/bridge/br_if.c
> @@ -497,12 +497,21 @@ static void br_port_clear_promisc(struct net_bridge_port *p)
>  void br_manage_promisc(struct net_bridge *br)
>  {
>  	struct net_bridge_port *p;
> +	int set_all = false;
> +
> +	if (br->dev->flags & IFF_PROMISC)
> +		set_all = true;
> +
> +	/* If vlan filtering is disabled and there are any VLANs
> +	 * configured on top of the bridge, set promisc on all
> +	 * ports.
> +	 */
> +	if (!br_vlan_enabled(br) && vlan_uses_dev(br->dev))
> +		set_all = true;
>  
>  	list_for_each_entry(p, &br->port_list, list) {
> -		if (br->dev->flags & IFF_PROMISC) {
> -			/* PROMISC flag has been turned on for the bridge
> -			 * itself.  Turn on promisc on all ports.
> -			 */
> +		if (set_all) {
> +			/* Set all the ports to promisc mode.  */
>  			br_port_set_promisc(p);
>  
>  		} else {
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 4042f86..87dcc09 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -641,6 +641,10 @@ static inline u16 br_get_pvid(const struct net_port_vlans *v)
>  	return v->pvid ?: VLAN_N_VID;
>  }
>  
> +static inline int br_vlan_enabled(struct net_bridge *br)
> +{
> +	return br->vlan_enabled;
> +}
>  #else
>  static inline bool br_allowed_ingress(struct net_bridge *br,
>  				      struct net_port_vlans *v,
> @@ -721,6 +725,11 @@ static inline u16 br_get_pvid(const struct net_port_vlans *v)
>  {
>  	return VLAN_N_VID;	/* Returns invalid vid */
>  }
> +
> +static inline int br_vlan_enabled(struct net_bridge *br);
> +{
> +	return 0;
> +}
>  #endif
>  
>  /* br_netfilter.c */
> diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
> index 8249ca7..eddc2f6 100644
> --- a/net/bridge/br_vlan.c
> +++ b/net/bridge/br_vlan.c
> @@ -321,6 +321,7 @@ int br_vlan_filter_toggle(struct net_bridge *br, unsigned long val)
>  		goto unlock;
>  
>  	br->vlan_enabled = val;
> +	br_manage_promisc(br);
>  
>  unlock:
>  	rtnl_unlock();
> -- 
> 1.8.5.3