All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next] net: dsa: mt7530: set CPU port to fallback mode
@ 2020-05-13 15:37 ` DENG Qingfang
  0 siblings, 0 replies; 14+ messages in thread
From: DENG Qingfang @ 2020-05-13 15:37 UTC (permalink / raw)
  To: netdev
  Cc: Sean Wang, Andrew Lunn, Vivien Didelot, Florian Fainelli,
	David S . Miller, linux-mediatek, Russell King, Matthias Brugger,
	René van Dorst, Tom James, Stijn Segers, riddlariddla,
	Szabolcs Hubai, Paul Fertser

Currently, setting a bridge's self PVID to other value and deleting
the default VID 1 renders untagged ports of that VLAN unable to talk to
the CPU port:

	bridge vlan add dev br0 vid 2 pvid untagged self
	bridge vlan del dev br0 vid 1 self
	bridge vlan add dev sw0p0 vid 2 pvid untagged
	bridge vlan del dev sw0p0 vid 1
	# br0 cannot send untagged frames out of sw0p0 anymore

That is because the CPU port is set to security mode and its PVID is
still 1, and untagged frames are dropped due to VLAN member violation.

Set the CPU port to fallback mode so untagged frames can pass through.

Fixes: 83163f7dca56 ("net: dsa: mediatek: add VLAN support for MT7530")
Signed-off-by: DENG Qingfang <dqfext@gmail.com>
---
 drivers/net/dsa/mt7530.c | 11 ++++++++---
 drivers/net/dsa/mt7530.h |  6 ++++++
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 5c444cd722bd..a063d914c23f 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -810,10 +810,15 @@ mt7530_port_set_vlan_aware(struct dsa_switch *ds, int port)
 		   PCR_MATRIX_MASK, PCR_MATRIX(MT7530_ALL_MEMBERS));
 
 	/* Trapped into security mode allows packet forwarding through VLAN
-	 * table lookup.
+	 * table lookup. CPU port is set to fallback mode to let untagged
+	 * frames pass through.
 	 */
-	mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK,
-		   MT7530_PORT_SECURITY_MODE);
+	if (dsa_is_cpu_port(ds, port))
+		mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK,
+			   MT7530_PORT_FALLBACK_MODE);
+	else
+		mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK,
+			   MT7530_PORT_SECURITY_MODE);
 
 	/* Set the port as a user port which is to be able to recognize VID
 	 * from incoming packets before fetching entry within the VLAN table.
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index 979bb6374678..d45eb7540703 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -152,6 +152,12 @@ enum mt7530_port_mode {
 	/* Port Matrix Mode: Frames are forwarded by the PCR_MATRIX members. */
 	MT7530_PORT_MATRIX_MODE = PORT_VLAN(0),
 
+	/* Fallback Mode: Forward received frames with ingress ports that do
+	 * not belong to the VLAN member. Frames whose VID is not listed on
+	 * the VLAN table are forwarded by the PCR_MATRIX members.
+	 */
+	MT7530_PORT_FALLBACK_MODE = PORT_VLAN(1),
+
 	/* Security Mode: Discard any frame due to ingress membership
 	 * violation or VID missed on the VLAN table.
 	 */
-- 
2.26.2


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

* [PATCH net-next] net: dsa: mt7530: set CPU port to fallback mode
@ 2020-05-13 15:37 ` DENG Qingfang
  0 siblings, 0 replies; 14+ messages in thread
From: DENG Qingfang @ 2020-05-13 15:37 UTC (permalink / raw)
  To: netdev
  Cc: Andrew Lunn, Florian Fainelli, riddlariddla, Paul Fertser,
	Sean Wang, Russell King, David S . Miller, René van Dorst,
	linux-mediatek, Stijn Segers, Szabolcs Hubai, Matthias Brugger,
	Vivien Didelot, Tom James

Currently, setting a bridge's self PVID to other value and deleting
the default VID 1 renders untagged ports of that VLAN unable to talk to
the CPU port:

	bridge vlan add dev br0 vid 2 pvid untagged self
	bridge vlan del dev br0 vid 1 self
	bridge vlan add dev sw0p0 vid 2 pvid untagged
	bridge vlan del dev sw0p0 vid 1
	# br0 cannot send untagged frames out of sw0p0 anymore

That is because the CPU port is set to security mode and its PVID is
still 1, and untagged frames are dropped due to VLAN member violation.

Set the CPU port to fallback mode so untagged frames can pass through.

Fixes: 83163f7dca56 ("net: dsa: mediatek: add VLAN support for MT7530")
Signed-off-by: DENG Qingfang <dqfext@gmail.com>
---
 drivers/net/dsa/mt7530.c | 11 ++++++++---
 drivers/net/dsa/mt7530.h |  6 ++++++
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 5c444cd722bd..a063d914c23f 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -810,10 +810,15 @@ mt7530_port_set_vlan_aware(struct dsa_switch *ds, int port)
 		   PCR_MATRIX_MASK, PCR_MATRIX(MT7530_ALL_MEMBERS));
 
 	/* Trapped into security mode allows packet forwarding through VLAN
-	 * table lookup.
+	 * table lookup. CPU port is set to fallback mode to let untagged
+	 * frames pass through.
 	 */
-	mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK,
-		   MT7530_PORT_SECURITY_MODE);
+	if (dsa_is_cpu_port(ds, port))
+		mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK,
+			   MT7530_PORT_FALLBACK_MODE);
+	else
+		mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK,
+			   MT7530_PORT_SECURITY_MODE);
 
 	/* Set the port as a user port which is to be able to recognize VID
 	 * from incoming packets before fetching entry within the VLAN table.
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index 979bb6374678..d45eb7540703 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -152,6 +152,12 @@ enum mt7530_port_mode {
 	/* Port Matrix Mode: Frames are forwarded by the PCR_MATRIX members. */
 	MT7530_PORT_MATRIX_MODE = PORT_VLAN(0),
 
+	/* Fallback Mode: Forward received frames with ingress ports that do
+	 * not belong to the VLAN member. Frames whose VID is not listed on
+	 * the VLAN table are forwarded by the PCR_MATRIX members.
+	 */
+	MT7530_PORT_FALLBACK_MODE = PORT_VLAN(1),
+
 	/* Security Mode: Discard any frame due to ingress membership
 	 * violation or VID missed on the VLAN table.
 	 */
-- 
2.26.2


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

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

* Re: [PATCH net-next] net: dsa: mt7530: set CPU port to fallback mode
  2020-05-13 15:37 ` DENG Qingfang
@ 2020-05-13 15:46   ` Florian Fainelli
  -1 siblings, 0 replies; 14+ messages in thread
From: Florian Fainelli @ 2020-05-13 15:46 UTC (permalink / raw)
  To: DENG Qingfang, netdev
  Cc: Sean Wang, Andrew Lunn, Vivien Didelot, David S . Miller,
	linux-mediatek, Russell King, Matthias Brugger,
	René van Dorst, Tom James, Stijn Segers, riddlariddla,
	Szabolcs Hubai, Paul Fertser



On 5/13/2020 8:37 AM, DENG Qingfang wrote:
> Currently, setting a bridge's self PVID to other value and deleting
> the default VID 1 renders untagged ports of that VLAN unable to talk to
> the CPU port:
> 
> 	bridge vlan add dev br0 vid 2 pvid untagged self
> 	bridge vlan del dev br0 vid 1 self
> 	bridge vlan add dev sw0p0 vid 2 pvid untagged
> 	bridge vlan del dev sw0p0 vid 1
> 	# br0 cannot send untagged frames out of sw0p0 anymore
> 
> That is because the CPU port is set to security mode and its PVID is
> still 1, and untagged frames are dropped due to VLAN member violation.
> 
> Set the CPU port to fallback mode so untagged frames can pass through.

How about if the bridge has vlan_filtering=1? The use case you present
seems to be valid to me, that is, you may create a VLAN just for the
user ports and not have the CPU port be part of it at all.

> 
> Fixes: 83163f7dca56 ("net: dsa: mediatek: add VLAN support for MT7530")
> Signed-off-by: DENG Qingfang <dqfext@gmail.com>
> ---
>  drivers/net/dsa/mt7530.c | 11 ++++++++---
>  drivers/net/dsa/mt7530.h |  6 ++++++
>  2 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index 5c444cd722bd..a063d914c23f 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -810,10 +810,15 @@ mt7530_port_set_vlan_aware(struct dsa_switch *ds, int port)
>  		   PCR_MATRIX_MASK, PCR_MATRIX(MT7530_ALL_MEMBERS));
>  
>  	/* Trapped into security mode allows packet forwarding through VLAN
> -	 * table lookup.
> +	 * table lookup. CPU port is set to fallback mode to let untagged
> +	 * frames pass through.
>  	 */
> -	mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK,
> -		   MT7530_PORT_SECURITY_MODE);
> +	if (dsa_is_cpu_port(ds, port))
> +		mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK,
> +			   MT7530_PORT_FALLBACK_MODE);
> +	else
> +		mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK,
> +			   MT7530_PORT_SECURITY_MODE);
>  
>  	/* Set the port as a user port which is to be able to recognize VID
>  	 * from incoming packets before fetching entry within the VLAN table.
> diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
> index 979bb6374678..d45eb7540703 100644
> --- a/drivers/net/dsa/mt7530.h
> +++ b/drivers/net/dsa/mt7530.h
> @@ -152,6 +152,12 @@ enum mt7530_port_mode {
>  	/* Port Matrix Mode: Frames are forwarded by the PCR_MATRIX members. */
>  	MT7530_PORT_MATRIX_MODE = PORT_VLAN(0),
>  
> +	/* Fallback Mode: Forward received frames with ingress ports that do
> +	 * not belong to the VLAN member. Frames whose VID is not listed on
> +	 * the VLAN table are forwarded by the PCR_MATRIX members.
> +	 */
> +	MT7530_PORT_FALLBACK_MODE = PORT_VLAN(1),
> +
>  	/* Security Mode: Discard any frame due to ingress membership
>  	 * violation or VID missed on the VLAN table.
>  	 */
> 

-- 
Florian

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

* Re: [PATCH net-next] net: dsa: mt7530: set CPU port to fallback mode
@ 2020-05-13 15:46   ` Florian Fainelli
  0 siblings, 0 replies; 14+ messages in thread
From: Florian Fainelli @ 2020-05-13 15:46 UTC (permalink / raw)
  To: DENG Qingfang, netdev
  Cc: Andrew Lunn, riddlariddla, Paul Fertser, Sean Wang, Russell King,
	David S . Miller, René van Dorst, linux-mediatek,
	Stijn Segers, Szabolcs Hubai, Matthias Brugger, Vivien Didelot,
	Tom James



On 5/13/2020 8:37 AM, DENG Qingfang wrote:
> Currently, setting a bridge's self PVID to other value and deleting
> the default VID 1 renders untagged ports of that VLAN unable to talk to
> the CPU port:
> 
> 	bridge vlan add dev br0 vid 2 pvid untagged self
> 	bridge vlan del dev br0 vid 1 self
> 	bridge vlan add dev sw0p0 vid 2 pvid untagged
> 	bridge vlan del dev sw0p0 vid 1
> 	# br0 cannot send untagged frames out of sw0p0 anymore
> 
> That is because the CPU port is set to security mode and its PVID is
> still 1, and untagged frames are dropped due to VLAN member violation.
> 
> Set the CPU port to fallback mode so untagged frames can pass through.

How about if the bridge has vlan_filtering=1? The use case you present
seems to be valid to me, that is, you may create a VLAN just for the
user ports and not have the CPU port be part of it at all.

> 
> Fixes: 83163f7dca56 ("net: dsa: mediatek: add VLAN support for MT7530")
> Signed-off-by: DENG Qingfang <dqfext@gmail.com>
> ---
>  drivers/net/dsa/mt7530.c | 11 ++++++++---
>  drivers/net/dsa/mt7530.h |  6 ++++++
>  2 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index 5c444cd722bd..a063d914c23f 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -810,10 +810,15 @@ mt7530_port_set_vlan_aware(struct dsa_switch *ds, int port)
>  		   PCR_MATRIX_MASK, PCR_MATRIX(MT7530_ALL_MEMBERS));
>  
>  	/* Trapped into security mode allows packet forwarding through VLAN
> -	 * table lookup.
> +	 * table lookup. CPU port is set to fallback mode to let untagged
> +	 * frames pass through.
>  	 */
> -	mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK,
> -		   MT7530_PORT_SECURITY_MODE);
> +	if (dsa_is_cpu_port(ds, port))
> +		mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK,
> +			   MT7530_PORT_FALLBACK_MODE);
> +	else
> +		mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK,
> +			   MT7530_PORT_SECURITY_MODE);
>  
>  	/* Set the port as a user port which is to be able to recognize VID
>  	 * from incoming packets before fetching entry within the VLAN table.
> diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
> index 979bb6374678..d45eb7540703 100644
> --- a/drivers/net/dsa/mt7530.h
> +++ b/drivers/net/dsa/mt7530.h
> @@ -152,6 +152,12 @@ enum mt7530_port_mode {
>  	/* Port Matrix Mode: Frames are forwarded by the PCR_MATRIX members. */
>  	MT7530_PORT_MATRIX_MODE = PORT_VLAN(0),
>  
> +	/* Fallback Mode: Forward received frames with ingress ports that do
> +	 * not belong to the VLAN member. Frames whose VID is not listed on
> +	 * the VLAN table are forwarded by the PCR_MATRIX members.
> +	 */
> +	MT7530_PORT_FALLBACK_MODE = PORT_VLAN(1),
> +
>  	/* Security Mode: Discard any frame due to ingress membership
>  	 * violation or VID missed on the VLAN table.
>  	 */
> 

-- 
Florian

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

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

* Re: [PATCH net-next] net: dsa: mt7530: set CPU port to fallback mode
  2020-05-13 15:46   ` Florian Fainelli
@ 2020-05-13 15:56     ` DENG Qingfang
  -1 siblings, 0 replies; 14+ messages in thread
From: DENG Qingfang @ 2020-05-13 15:56 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: netdev, Sean Wang, Andrew Lunn, Vivien Didelot, David S . Miller,
	moderated list:ARM/Mediatek SoC support, Russell King,
	Matthias Brugger, René van Dorst, Tom James, Stijn Segers,
	riddlariddla, Szabolcs Hubai, Paul Fertser

Hi Florian

On Wed, May 13, 2020 at 11:46 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>
>
>
> On 5/13/2020 8:37 AM, DENG Qingfang wrote:
> > Currently, setting a bridge's self PVID to other value and deleting
> > the default VID 1 renders untagged ports of that VLAN unable to talk to
> > the CPU port:
> >
> >       bridge vlan add dev br0 vid 2 pvid untagged self
> >       bridge vlan del dev br0 vid 1 self
> >       bridge vlan add dev sw0p0 vid 2 pvid untagged
> >       bridge vlan del dev sw0p0 vid 1
> >       # br0 cannot send untagged frames out of sw0p0 anymore
> >
> > That is because the CPU port is set to security mode and its PVID is
> > still 1, and untagged frames are dropped due to VLAN member violation.
> >
> > Set the CPU port to fallback mode so untagged frames can pass through.
>
> How about if the bridge has vlan_filtering=1? The use case you present
> seems to be valid to me, that is, you may create a VLAN just for the
> user ports and not have the CPU port be part of it at all.

I forgot to mention that this is ONLY for vlan_filtering=1
`bridge vlan` simply won't do anything if VLAN filtering is disabled.

>
> >
> > Fixes: 83163f7dca56 ("net: dsa: mediatek: add VLAN support for MT7530")
> > Signed-off-by: DENG Qingfang <dqfext@gmail.com>
> > ---
> >  drivers/net/dsa/mt7530.c | 11 ++++++++---
> >  drivers/net/dsa/mt7530.h |  6 ++++++
> >  2 files changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> > index 5c444cd722bd..a063d914c23f 100644
> > --- a/drivers/net/dsa/mt7530.c
> > +++ b/drivers/net/dsa/mt7530.c
> > @@ -810,10 +810,15 @@ mt7530_port_set_vlan_aware(struct dsa_switch *ds, int port)
> >                  PCR_MATRIX_MASK, PCR_MATRIX(MT7530_ALL_MEMBERS));
> >
> >       /* Trapped into security mode allows packet forwarding through VLAN
> > -      * table lookup.
> > +      * table lookup. CPU port is set to fallback mode to let untagged
> > +      * frames pass through.
> >        */
> > -     mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK,
> > -                MT7530_PORT_SECURITY_MODE);
> > +     if (dsa_is_cpu_port(ds, port))
> > +             mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK,
> > +                        MT7530_PORT_FALLBACK_MODE);
> > +     else
> > +             mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK,
> > +                        MT7530_PORT_SECURITY_MODE);
> >
> >       /* Set the port as a user port which is to be able to recognize VID
> >        * from incoming packets before fetching entry within the VLAN table.
> > diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
> > index 979bb6374678..d45eb7540703 100644
> > --- a/drivers/net/dsa/mt7530.h
> > +++ b/drivers/net/dsa/mt7530.h
> > @@ -152,6 +152,12 @@ enum mt7530_port_mode {
> >       /* Port Matrix Mode: Frames are forwarded by the PCR_MATRIX members. */
> >       MT7530_PORT_MATRIX_MODE = PORT_VLAN(0),
> >
> > +     /* Fallback Mode: Forward received frames with ingress ports that do
> > +      * not belong to the VLAN member. Frames whose VID is not listed on
> > +      * the VLAN table are forwarded by the PCR_MATRIX members.
> > +      */
> > +     MT7530_PORT_FALLBACK_MODE = PORT_VLAN(1),
> > +
> >       /* Security Mode: Discard any frame due to ingress membership
> >        * violation or VID missed on the VLAN table.
> >        */
> >
>
> --
> Florian

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

* Re: [PATCH net-next] net: dsa: mt7530: set CPU port to fallback mode
@ 2020-05-13 15:56     ` DENG Qingfang
  0 siblings, 0 replies; 14+ messages in thread
From: DENG Qingfang @ 2020-05-13 15:56 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Andrew Lunn, riddlariddla, Paul Fertser, netdev, Sean Wang,
	Russell King, Vivien Didelot, René van Dorst,
	moderated list:ARM/Mediatek SoC support, Stijn Segers,
	Szabolcs Hubai, Matthias Brugger, David S . Miller, Tom James

Hi Florian

On Wed, May 13, 2020 at 11:46 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>
>
>
> On 5/13/2020 8:37 AM, DENG Qingfang wrote:
> > Currently, setting a bridge's self PVID to other value and deleting
> > the default VID 1 renders untagged ports of that VLAN unable to talk to
> > the CPU port:
> >
> >       bridge vlan add dev br0 vid 2 pvid untagged self
> >       bridge vlan del dev br0 vid 1 self
> >       bridge vlan add dev sw0p0 vid 2 pvid untagged
> >       bridge vlan del dev sw0p0 vid 1
> >       # br0 cannot send untagged frames out of sw0p0 anymore
> >
> > That is because the CPU port is set to security mode and its PVID is
> > still 1, and untagged frames are dropped due to VLAN member violation.
> >
> > Set the CPU port to fallback mode so untagged frames can pass through.
>
> How about if the bridge has vlan_filtering=1? The use case you present
> seems to be valid to me, that is, you may create a VLAN just for the
> user ports and not have the CPU port be part of it at all.

I forgot to mention that this is ONLY for vlan_filtering=1
`bridge vlan` simply won't do anything if VLAN filtering is disabled.

>
> >
> > Fixes: 83163f7dca56 ("net: dsa: mediatek: add VLAN support for MT7530")
> > Signed-off-by: DENG Qingfang <dqfext@gmail.com>
> > ---
> >  drivers/net/dsa/mt7530.c | 11 ++++++++---
> >  drivers/net/dsa/mt7530.h |  6 ++++++
> >  2 files changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> > index 5c444cd722bd..a063d914c23f 100644
> > --- a/drivers/net/dsa/mt7530.c
> > +++ b/drivers/net/dsa/mt7530.c
> > @@ -810,10 +810,15 @@ mt7530_port_set_vlan_aware(struct dsa_switch *ds, int port)
> >                  PCR_MATRIX_MASK, PCR_MATRIX(MT7530_ALL_MEMBERS));
> >
> >       /* Trapped into security mode allows packet forwarding through VLAN
> > -      * table lookup.
> > +      * table lookup. CPU port is set to fallback mode to let untagged
> > +      * frames pass through.
> >        */
> > -     mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK,
> > -                MT7530_PORT_SECURITY_MODE);
> > +     if (dsa_is_cpu_port(ds, port))
> > +             mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK,
> > +                        MT7530_PORT_FALLBACK_MODE);
> > +     else
> > +             mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK,
> > +                        MT7530_PORT_SECURITY_MODE);
> >
> >       /* Set the port as a user port which is to be able to recognize VID
> >        * from incoming packets before fetching entry within the VLAN table.
> > diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
> > index 979bb6374678..d45eb7540703 100644
> > --- a/drivers/net/dsa/mt7530.h
> > +++ b/drivers/net/dsa/mt7530.h
> > @@ -152,6 +152,12 @@ enum mt7530_port_mode {
> >       /* Port Matrix Mode: Frames are forwarded by the PCR_MATRIX members. */
> >       MT7530_PORT_MATRIX_MODE = PORT_VLAN(0),
> >
> > +     /* Fallback Mode: Forward received frames with ingress ports that do
> > +      * not belong to the VLAN member. Frames whose VID is not listed on
> > +      * the VLAN table are forwarded by the PCR_MATRIX members.
> > +      */
> > +     MT7530_PORT_FALLBACK_MODE = PORT_VLAN(1),
> > +
> >       /* Security Mode: Discard any frame due to ingress membership
> >        * violation or VID missed on the VLAN table.
> >        */
> >
>
> --
> Florian

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

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

* Re: [PATCH net-next] net: dsa: mt7530: set CPU port to fallback mode
  2020-05-13 15:56     ` DENG Qingfang
@ 2020-05-13 16:16       ` Florian Fainelli
  -1 siblings, 0 replies; 14+ messages in thread
From: Florian Fainelli @ 2020-05-13 16:16 UTC (permalink / raw)
  To: DENG Qingfang
  Cc: netdev, Sean Wang, Andrew Lunn, Vivien Didelot, David S . Miller,
	moderated list:ARM/Mediatek SoC support, Russell King,
	Matthias Brugger, René van Dorst, Tom James, Stijn Segers,
	riddlariddla, Szabolcs Hubai, Paul Fertser



On 5/13/2020 8:56 AM, DENG Qingfang wrote:
> Hi Florian
> 
> On Wed, May 13, 2020 at 11:46 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>>
>>
>>
>> On 5/13/2020 8:37 AM, DENG Qingfang wrote:
>>> Currently, setting a bridge's self PVID to other value and deleting
>>> the default VID 1 renders untagged ports of that VLAN unable to talk to
>>> the CPU port:
>>>
>>>       bridge vlan add dev br0 vid 2 pvid untagged self
>>>       bridge vlan del dev br0 vid 1 self
>>>       bridge vlan add dev sw0p0 vid 2 pvid untagged
>>>       bridge vlan del dev sw0p0 vid 1
>>>       # br0 cannot send untagged frames out of sw0p0 anymore
>>>
>>> That is because the CPU port is set to security mode and its PVID is
>>> still 1, and untagged frames are dropped due to VLAN member violation.
>>>
>>> Set the CPU port to fallback mode so untagged frames can pass through.
>>
>> How about if the bridge has vlan_filtering=1? The use case you present
>> seems to be valid to me, that is, you may create a VLAN just for the
>> user ports and not have the CPU port be part of it at all.
> 
> I forgot to mention that this is ONLY for vlan_filtering=1
> `bridge vlan` simply won't do anything if VLAN filtering is disabled.

It depends now as of this patch:

https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=54a0ed0df49609f4e3f098f8943e38e389dc2e15

and sorry I misunderstood your use case, you are changing the default
VLAN for the CPU port through the bridge's master device and you still
want it to be in the same VLAN membership as sw0p0, so yes that looks
correct.
-- 
Florian

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

* Re: [PATCH net-next] net: dsa: mt7530: set CPU port to fallback mode
@ 2020-05-13 16:16       ` Florian Fainelli
  0 siblings, 0 replies; 14+ messages in thread
From: Florian Fainelli @ 2020-05-13 16:16 UTC (permalink / raw)
  To: DENG Qingfang
  Cc: Andrew Lunn, riddlariddla, Paul Fertser, netdev, Sean Wang,
	Russell King, Vivien Didelot, René van Dorst,
	moderated list:ARM/Mediatek SoC support, Stijn Segers,
	Szabolcs Hubai, Matthias Brugger, David S . Miller, Tom James



On 5/13/2020 8:56 AM, DENG Qingfang wrote:
> Hi Florian
> 
> On Wed, May 13, 2020 at 11:46 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>>
>>
>>
>> On 5/13/2020 8:37 AM, DENG Qingfang wrote:
>>> Currently, setting a bridge's self PVID to other value and deleting
>>> the default VID 1 renders untagged ports of that VLAN unable to talk to
>>> the CPU port:
>>>
>>>       bridge vlan add dev br0 vid 2 pvid untagged self
>>>       bridge vlan del dev br0 vid 1 self
>>>       bridge vlan add dev sw0p0 vid 2 pvid untagged
>>>       bridge vlan del dev sw0p0 vid 1
>>>       # br0 cannot send untagged frames out of sw0p0 anymore
>>>
>>> That is because the CPU port is set to security mode and its PVID is
>>> still 1, and untagged frames are dropped due to VLAN member violation.
>>>
>>> Set the CPU port to fallback mode so untagged frames can pass through.
>>
>> How about if the bridge has vlan_filtering=1? The use case you present
>> seems to be valid to me, that is, you may create a VLAN just for the
>> user ports and not have the CPU port be part of it at all.
> 
> I forgot to mention that this is ONLY for vlan_filtering=1
> `bridge vlan` simply won't do anything if VLAN filtering is disabled.

It depends now as of this patch:

https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=54a0ed0df49609f4e3f098f8943e38e389dc2e15

and sorry I misunderstood your use case, you are changing the default
VLAN for the CPU port through the bridge's master device and you still
want it to be in the same VLAN membership as sw0p0, so yes that looks
correct.
-- 
Florian

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

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

* Re: [PATCH net-next] net: dsa: mt7530: set CPU port to fallback mode
  2020-05-13 15:46   ` Florian Fainelli
@ 2020-05-13 16:17     ` Vladimir Oltean
  -1 siblings, 0 replies; 14+ messages in thread
From: Vladimir Oltean @ 2020-05-13 16:17 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: DENG Qingfang, netdev, Sean Wang, Andrew Lunn, Vivien Didelot,
	David S . Miller, moderated list:ARM/Mediatek SoC support,
	Russell King, Matthias Brugger, René van Dorst, Tom James,
	Stijn Segers, riddlariddla, Szabolcs Hubai, Paul Fertser

On Wed, 13 May 2020 at 18:49, Florian Fainelli <f.fainelli@gmail.com> wrote:
>
>
>
> On 5/13/2020 8:37 AM, DENG Qingfang wrote:
> > Currently, setting a bridge's self PVID to other value and deleting
> > the default VID 1 renders untagged ports of that VLAN unable to talk to
> > the CPU port:
> >
> >       bridge vlan add dev br0 vid 2 pvid untagged self
> >       bridge vlan del dev br0 vid 1 self
> >       bridge vlan add dev sw0p0 vid 2 pvid untagged
> >       bridge vlan del dev sw0p0 vid 1
> >       # br0 cannot send untagged frames out of sw0p0 anymore
> >
> > That is because the CPU port is set to security mode and its PVID is
> > still 1, and untagged frames are dropped due to VLAN member violation.
> >
> > Set the CPU port to fallback mode so untagged frames can pass through.
>
> How about if the bridge has vlan_filtering=1? The use case you present
> seems to be valid to me, that is, you may create a VLAN just for the
> user ports and not have the CPU port be part of it at all.
>

What Qingfang is doing is in effect (but not by intention) removing
the front panel port sw0p0 from the membership list of the CPU port's
pvid. What you seem to be thinking of (VLAN of which the CPU is not a
member of) does not seem to be supported in DSA at the moment.

As a fix, there's nothing wrong with the patch actually, I don't even
know how it would work otherwise. DSA doesn't change the pvid of the
CPU port when the pvid of a slave changes, because 4 slave ports could
have 4 different pvids and the CPU port pvid would keep changing.
Fallback mode should only apply on ingress from CPU, so there's no
danger really.

Thanks,
-Vladimir

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

* Re: [PATCH net-next] net: dsa: mt7530: set CPU port to fallback mode
@ 2020-05-13 16:17     ` Vladimir Oltean
  0 siblings, 0 replies; 14+ messages in thread
From: Vladimir Oltean @ 2020-05-13 16:17 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Andrew Lunn, René van Dorst, Paul Fertser, netdev,
	Sean Wang, Russell King, Vivien Didelot, DENG Qingfang,
	moderated list:ARM/Mediatek SoC support, Stijn Segers,
	Szabolcs Hubai, Matthias Brugger, riddlariddla, David S . Miller,
	Tom James

On Wed, 13 May 2020 at 18:49, Florian Fainelli <f.fainelli@gmail.com> wrote:
>
>
>
> On 5/13/2020 8:37 AM, DENG Qingfang wrote:
> > Currently, setting a bridge's self PVID to other value and deleting
> > the default VID 1 renders untagged ports of that VLAN unable to talk to
> > the CPU port:
> >
> >       bridge vlan add dev br0 vid 2 pvid untagged self
> >       bridge vlan del dev br0 vid 1 self
> >       bridge vlan add dev sw0p0 vid 2 pvid untagged
> >       bridge vlan del dev sw0p0 vid 1
> >       # br0 cannot send untagged frames out of sw0p0 anymore
> >
> > That is because the CPU port is set to security mode and its PVID is
> > still 1, and untagged frames are dropped due to VLAN member violation.
> >
> > Set the CPU port to fallback mode so untagged frames can pass through.
>
> How about if the bridge has vlan_filtering=1? The use case you present
> seems to be valid to me, that is, you may create a VLAN just for the
> user ports and not have the CPU port be part of it at all.
>

What Qingfang is doing is in effect (but not by intention) removing
the front panel port sw0p0 from the membership list of the CPU port's
pvid. What you seem to be thinking of (VLAN of which the CPU is not a
member of) does not seem to be supported in DSA at the moment.

As a fix, there's nothing wrong with the patch actually, I don't even
know how it would work otherwise. DSA doesn't change the pvid of the
CPU port when the pvid of a slave changes, because 4 slave ports could
have 4 different pvids and the CPU port pvid would keep changing.
Fallback mode should only apply on ingress from CPU, so there's no
danger really.

Thanks,
-Vladimir

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

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

* Re: [PATCH net-next] net: dsa: mt7530: set CPU port to fallback mode
  2020-05-13 16:17     ` Vladimir Oltean
@ 2020-05-13 16:22       ` Florian Fainelli
  -1 siblings, 0 replies; 14+ messages in thread
From: Florian Fainelli @ 2020-05-13 16:22 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: DENG Qingfang, netdev, Sean Wang, Andrew Lunn, Vivien Didelot,
	David S . Miller, moderated list:ARM/Mediatek SoC support,
	Russell King, Matthias Brugger, René van Dorst, Tom James,
	Stijn Segers, riddlariddla, Szabolcs Hubai, Paul Fertser



On 5/13/2020 9:17 AM, Vladimir Oltean wrote:
> On Wed, 13 May 2020 at 18:49, Florian Fainelli <f.fainelli@gmail.com> wrote:
>>
>>
>>
>> On 5/13/2020 8:37 AM, DENG Qingfang wrote:
>>> Currently, setting a bridge's self PVID to other value and deleting
>>> the default VID 1 renders untagged ports of that VLAN unable to talk to
>>> the CPU port:
>>>
>>>       bridge vlan add dev br0 vid 2 pvid untagged self
>>>       bridge vlan del dev br0 vid 1 self
>>>       bridge vlan add dev sw0p0 vid 2 pvid untagged
>>>       bridge vlan del dev sw0p0 vid 1
>>>       # br0 cannot send untagged frames out of sw0p0 anymore
>>>
>>> That is because the CPU port is set to security mode and its PVID is
>>> still 1, and untagged frames are dropped due to VLAN member violation.
>>>
>>> Set the CPU port to fallback mode so untagged frames can pass through.
>>
>> How about if the bridge has vlan_filtering=1? The use case you present
>> seems to be valid to me, that is, you may create a VLAN just for the
>> user ports and not have the CPU port be part of it at all.
>>
> 
> What Qingfang is doing is in effect (but not by intention) removing
> the front panel port sw0p0 from the membership list of the CPU port's
> pvid. What you seem to be thinking of (VLAN of which the CPU is not a
> member of) does not seem to be supported in DSA at the moment.

Indeed and I replied to Qingfang already that I had misunderstood his
patch originally.

> 
> As a fix, there's nothing wrong with the patch actually, I don't even
> know how it would work otherwise. DSA doesn't change the pvid of the
> CPU port when the pvid of a slave changes, because 4 slave ports could
> have 4 different pvids and the CPU port pvid would keep changing.
> Fallback mode should only apply on ingress from CPU, so there's no
> danger really.

Agreed.

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

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

* Re: [PATCH net-next] net: dsa: mt7530: set CPU port to fallback mode
@ 2020-05-13 16:22       ` Florian Fainelli
  0 siblings, 0 replies; 14+ messages in thread
From: Florian Fainelli @ 2020-05-13 16:22 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: Andrew Lunn, René van Dorst, Paul Fertser, netdev,
	Sean Wang, Russell King, Vivien Didelot, DENG Qingfang,
	moderated list:ARM/Mediatek SoC support, Stijn Segers,
	Szabolcs Hubai, Matthias Brugger, riddlariddla, David S . Miller,
	Tom James



On 5/13/2020 9:17 AM, Vladimir Oltean wrote:
> On Wed, 13 May 2020 at 18:49, Florian Fainelli <f.fainelli@gmail.com> wrote:
>>
>>
>>
>> On 5/13/2020 8:37 AM, DENG Qingfang wrote:
>>> Currently, setting a bridge's self PVID to other value and deleting
>>> the default VID 1 renders untagged ports of that VLAN unable to talk to
>>> the CPU port:
>>>
>>>       bridge vlan add dev br0 vid 2 pvid untagged self
>>>       bridge vlan del dev br0 vid 1 self
>>>       bridge vlan add dev sw0p0 vid 2 pvid untagged
>>>       bridge vlan del dev sw0p0 vid 1
>>>       # br0 cannot send untagged frames out of sw0p0 anymore
>>>
>>> That is because the CPU port is set to security mode and its PVID is
>>> still 1, and untagged frames are dropped due to VLAN member violation.
>>>
>>> Set the CPU port to fallback mode so untagged frames can pass through.
>>
>> How about if the bridge has vlan_filtering=1? The use case you present
>> seems to be valid to me, that is, you may create a VLAN just for the
>> user ports and not have the CPU port be part of it at all.
>>
> 
> What Qingfang is doing is in effect (but not by intention) removing
> the front panel port sw0p0 from the membership list of the CPU port's
> pvid. What you seem to be thinking of (VLAN of which the CPU is not a
> member of) does not seem to be supported in DSA at the moment.

Indeed and I replied to Qingfang already that I had misunderstood his
patch originally.

> 
> As a fix, there's nothing wrong with the patch actually, I don't even
> know how it would work otherwise. DSA doesn't change the pvid of the
> CPU port when the pvid of a slave changes, because 4 slave ports could
> have 4 different pvids and the CPU port pvid would keep changing.
> Fallback mode should only apply on ingress from CPU, so there's no
> danger really.

Agreed.

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

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

* Re: [PATCH net-next] net: dsa: mt7530: set CPU port to fallback mode
  2020-05-13 15:37 ` DENG Qingfang
@ 2020-05-13 22:25   ` David Miller
  -1 siblings, 0 replies; 14+ messages in thread
From: David Miller @ 2020-05-13 22:25 UTC (permalink / raw)
  To: dqfext
  Cc: netdev, sean.wang, andrew, vivien.didelot, f.fainelli,
	linux-mediatek, linux, matthias.bgg, opensource, tj17, foss,
	riddlariddla, szab.hu, fercerpav

From: DENG Qingfang <dqfext@gmail.com>
Date: Wed, 13 May 2020 23:37:17 +0800

> Currently, setting a bridge's self PVID to other value and deleting
> the default VID 1 renders untagged ports of that VLAN unable to talk to
> the CPU port:
> 
> 	bridge vlan add dev br0 vid 2 pvid untagged self
> 	bridge vlan del dev br0 vid 1 self
> 	bridge vlan add dev sw0p0 vid 2 pvid untagged
> 	bridge vlan del dev sw0p0 vid 1
> 	# br0 cannot send untagged frames out of sw0p0 anymore
> 
> That is because the CPU port is set to security mode and its PVID is
> still 1, and untagged frames are dropped due to VLAN member violation.
> 
> Set the CPU port to fallback mode so untagged frames can pass through.
> 
> Fixes: 83163f7dca56 ("net: dsa: mediatek: add VLAN support for MT7530")
> Signed-off-by: DENG Qingfang <dqfext@gmail.com>

Applied to net-next, thanks.

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

* Re: [PATCH net-next] net: dsa: mt7530: set CPU port to fallback mode
@ 2020-05-13 22:25   ` David Miller
  0 siblings, 0 replies; 14+ messages in thread
From: David Miller @ 2020-05-13 22:25 UTC (permalink / raw)
  To: dqfext
  Cc: andrew, f.fainelli, fercerpav, netdev, sean.wang, linux,
	riddlariddla, opensource, linux-mediatek, foss, szab.hu,
	matthias.bgg, vivien.didelot, tj17

From: DENG Qingfang <dqfext@gmail.com>
Date: Wed, 13 May 2020 23:37:17 +0800

> Currently, setting a bridge's self PVID to other value and deleting
> the default VID 1 renders untagged ports of that VLAN unable to talk to
> the CPU port:
> 
> 	bridge vlan add dev br0 vid 2 pvid untagged self
> 	bridge vlan del dev br0 vid 1 self
> 	bridge vlan add dev sw0p0 vid 2 pvid untagged
> 	bridge vlan del dev sw0p0 vid 1
> 	# br0 cannot send untagged frames out of sw0p0 anymore
> 
> That is because the CPU port is set to security mode and its PVID is
> still 1, and untagged frames are dropped due to VLAN member violation.
> 
> Set the CPU port to fallback mode so untagged frames can pass through.
> 
> Fixes: 83163f7dca56 ("net: dsa: mediatek: add VLAN support for MT7530")
> Signed-off-by: DENG Qingfang <dqfext@gmail.com>

Applied to net-next, thanks.

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

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

end of thread, other threads:[~2020-05-13 22:25 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-13 15:37 [PATCH net-next] net: dsa: mt7530: set CPU port to fallback mode DENG Qingfang
2020-05-13 15:37 ` DENG Qingfang
2020-05-13 15:46 ` Florian Fainelli
2020-05-13 15:46   ` Florian Fainelli
2020-05-13 15:56   ` DENG Qingfang
2020-05-13 15:56     ` DENG Qingfang
2020-05-13 16:16     ` Florian Fainelli
2020-05-13 16:16       ` Florian Fainelli
2020-05-13 16:17   ` Vladimir Oltean
2020-05-13 16:17     ` Vladimir Oltean
2020-05-13 16:22     ` Florian Fainelli
2020-05-13 16:22       ` Florian Fainelli
2020-05-13 22:25 ` David Miller
2020-05-13 22:25   ` David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.