netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] net: dsa: Don't add vlans when vlan filtering is disabled
@ 2017-11-06 23:04 Andrew Lunn
  2017-11-06 23:10 ` Andrew Lunn
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Andrew Lunn @ 2017-11-06 23:04 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Vivien Didelot, Florian Fainelli, Andrew Lunn

The software bridge can be build with vlan filtering support
included. However, by default it is turned off. In its turned off
state, it still passes VLANs via switchev, even though they are not to
be used. Don't pass these VLANs to the hardware. Only do so when vlan
filtering is enabled.

This fixes at least one corner case. There are still issues in other
corners, such as when vlan_filtering is later enabled.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 net/dsa/port.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/dsa/port.c b/net/dsa/port.c
index bb30b1a7de3a..00d691765b9d 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -252,7 +252,10 @@ int dsa_port_vlan_add(struct dsa_port *dp,
 		.vlan = vlan,
 	};
 
-	return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_ADD, &info);
+	if (br_vlan_enabled(dp->bridge_dev))
+		return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_ADD, &info);
+
+	return 0;
 }
 
 int dsa_port_vlan_del(struct dsa_port *dp,
@@ -264,7 +267,10 @@ int dsa_port_vlan_del(struct dsa_port *dp,
 		.vlan = vlan,
 	};
 
-	return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_DEL, &info);
+	if (br_vlan_enabled(dp->bridge_dev))
+		return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_DEL, &info);
+
+	return 0;
 }
 
 int dsa_port_fixed_link_register_of(struct dsa_port *dp)
-- 
2.15.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] net: dsa: Don't add vlans when vlan filtering is disabled
  2017-11-06 23:04 [PATCH] net: dsa: Don't add vlans when vlan filtering is disabled Andrew Lunn
@ 2017-11-06 23:10 ` Andrew Lunn
  2017-11-06 23:13 ` Florian Fainelli
  2017-11-10  5:29 ` David Miller
  2 siblings, 0 replies; 5+ messages in thread
From: Andrew Lunn @ 2017-11-06 23:10 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Vivien Didelot, Florian Fainelli

Hi David

I forgot to put the tree in the subject. This is for net-next.

Sorry
	Andrew

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] net: dsa: Don't add vlans when vlan filtering is disabled
  2017-11-06 23:04 [PATCH] net: dsa: Don't add vlans when vlan filtering is disabled Andrew Lunn
  2017-11-06 23:10 ` Andrew Lunn
@ 2017-11-06 23:13 ` Florian Fainelli
  2017-11-06 23:26   ` Andrew Lunn
  2017-11-10  5:29 ` David Miller
  2 siblings, 1 reply; 5+ messages in thread
From: Florian Fainelli @ 2017-11-06 23:13 UTC (permalink / raw)
  To: Andrew Lunn, David Miller; +Cc: netdev, Vivien Didelot

Hi Andrew,

On 11/06/2017 03:04 PM, Andrew Lunn wrote:
> The software bridge can be build with vlan filtering support
> included. However, by default it is turned off. In its turned off
> state, it still passes VLANs via switchev, even though they are not to
> be used. Don't pass these VLANs to the hardware. Only do so when vlan
> filtering is enabled.

Do not we have a possible interface problem here in that we should know
whether VLAN filtering is enabled or not, and in such a case switch
drivers should do the following:

- if VLAN filtering is disabled, accept VLAN programming coming from
DSA, but do not enforce that frames ingress/egressing with an unknown
VID be rejected

- if VLAN filtering is enabled, accept VLAN programming coming from DSA,
enforce that frames ingressing/egressing with an unknown VID should be
rejected

> 
> This fixes at least one corner case. There are still issues in other
> corners, such as when vlan_filtering is later enabled.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  net/dsa/port.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/net/dsa/port.c b/net/dsa/port.c
> index bb30b1a7de3a..00d691765b9d 100644
> --- a/net/dsa/port.c
> +++ b/net/dsa/port.c
> @@ -252,7 +252,10 @@ int dsa_port_vlan_add(struct dsa_port *dp,
>  		.vlan = vlan,
>  	};
>  
> -	return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_ADD, &info);
> +	if (br_vlan_enabled(dp->bridge_dev))
> +		return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_ADD, &info);
> +
> +	return 0;
>  }
>  
>  int dsa_port_vlan_del(struct dsa_port *dp,
> @@ -264,7 +267,10 @@ int dsa_port_vlan_del(struct dsa_port *dp,
>  		.vlan = vlan,
>  	};
>  
> -	return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_DEL, &info);
> +	if (br_vlan_enabled(dp->bridge_dev))
> +		return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_DEL, &info);
> +
> +	return 0;
>  }
>  
>  int dsa_port_fixed_link_register_of(struct dsa_port *dp)
> 


-- 
Florian

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] net: dsa: Don't add vlans when vlan filtering is disabled
  2017-11-06 23:13 ` Florian Fainelli
@ 2017-11-06 23:26   ` Andrew Lunn
  0 siblings, 0 replies; 5+ messages in thread
From: Andrew Lunn @ 2017-11-06 23:26 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: David Miller, netdev, Vivien Didelot

On Mon, Nov 06, 2017 at 03:13:19PM -0800, Florian Fainelli wrote:
> Hi Andrew,
> 
> On 11/06/2017 03:04 PM, Andrew Lunn wrote:
> > The software bridge can be build with vlan filtering support
> > included. However, by default it is turned off. In its turned off
> > state, it still passes VLANs via switchev, even though they are not to
> > be used. Don't pass these VLANs to the hardware. Only do so when vlan
> > filtering is enabled.
> 
> Do not we have a possible interface problem here in that we should know
> whether VLAN filtering is enabled or not, and in such a case switch
> drivers should do the following:
> 
> - if VLAN filtering is disabled, accept VLAN programming coming from
> DSA, but do not enforce that frames ingress/egressing with an unknown
> VID be rejected

Hi Florian

We know if vlan filtering is enabled or not, via two different
mechanism.  br_vlan_enabled() tells us this, and we can call it
anytime. There is also a switchdev call when the state changes.

If vlan filtering is disabled, we allow all vlans through. Hence there
should not be any need to tell the hardware about the VLANs. It should
be passing them all anyway. If filtering is enabled, then clearly we
do have to program the hardware with the vlans.
 
> - if VLAN filtering is enabled, accept VLAN programming coming from DSA,
> enforce that frames ingressing/egressing with an unknown VID should be
> rejected

That is todays behaviour, and is unchanged.

     Andrew

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] net: dsa: Don't add vlans when vlan filtering is disabled
  2017-11-06 23:04 [PATCH] net: dsa: Don't add vlans when vlan filtering is disabled Andrew Lunn
  2017-11-06 23:10 ` Andrew Lunn
  2017-11-06 23:13 ` Florian Fainelli
@ 2017-11-10  5:29 ` David Miller
  2 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2017-11-10  5:29 UTC (permalink / raw)
  To: andrew; +Cc: netdev, vivien.didelot, f.fainelli

From: Andrew Lunn <andrew@lunn.ch>
Date: Tue,  7 Nov 2017 00:04:24 +0100

> The software bridge can be build with vlan filtering support
> included. However, by default it is turned off. In its turned off
> state, it still passes VLANs via switchev, even though they are not to
> be used. Don't pass these VLANs to the hardware. Only do so when vlan
> filtering is enabled.
> 
> This fixes at least one corner case. There are still issues in other
> corners, such as when vlan_filtering is later enabled.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Applied to net-next, thanks Andrew.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-11-10  5:29 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-06 23:04 [PATCH] net: dsa: Don't add vlans when vlan filtering is disabled Andrew Lunn
2017-11-06 23:10 ` Andrew Lunn
2017-11-06 23:13 ` Florian Fainelli
2017-11-06 23:26   ` Andrew Lunn
2017-11-10  5:29 ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).