Netdev Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH net] net: dsa: b53: Ensure the default VID is untagged
@ 2020-02-13 19:10 Florian Fainelli
  2020-02-14  8:00 ` Sergei Shtylyov
  2020-02-14 10:36 ` Vladimir Oltean
  0 siblings, 2 replies; 5+ messages in thread
From: Florian Fainelli @ 2020-02-13 19:10 UTC (permalink / raw)
  To: netdev
  Cc: olteanv, Florian Fainelli, Andrew Lunn, Vivien Didelot,
	David S. Miller, linux-kernel

We need to ensure that the default VID is untagged otherwise the switch
will be sending frames tagged frames and the results can be problematic.
This is especially true with b53 switches that use VID 0 as their
default VLAN since VID 0 has a special meaning.

Fixes: fea83353177a ("net: dsa: b53: Fix default VLAN ID")
Fixes: 061f6a505ac3 ("net: dsa: Add ndo_vlan_rx_{add, kill}_vid implementation")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 449a22172e07..f25c43b300d4 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1366,6 +1366,9 @@ void b53_vlan_add(struct dsa_switch *ds, int port,
 
 		b53_get_vlan_entry(dev, vid, vl);
 
+		if (vid == b53_default_pvid(dev))
+			untagged = true;
+
 		vl->members |= BIT(port);
 		if (untagged && !dsa_is_cpu_port(ds, port))
 			vl->untag |= BIT(port);
-- 
2.17.1


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

* Re: [PATCH net] net: dsa: b53: Ensure the default VID is untagged
  2020-02-13 19:10 [PATCH net] net: dsa: b53: Ensure the default VID is untagged Florian Fainelli
@ 2020-02-14  8:00 ` Sergei Shtylyov
  2020-02-14 10:36 ` Vladimir Oltean
  1 sibling, 0 replies; 5+ messages in thread
From: Sergei Shtylyov @ 2020-02-14  8:00 UTC (permalink / raw)
  To: Florian Fainelli, netdev
  Cc: olteanv, Andrew Lunn, Vivien Didelot, David S. Miller, open list

Hello!

On 13.02.2020 22:10, Florian Fainelli wrote:

> We need to ensure that the default VID is untagged otherwise the switch
> will be sending frames tagged frames and the results can be problematic.
                   ^^^^^^^^^^^^^^^^^^^^

   Perhaps just "tagged frames"?

> This is especially true with b53 switches that use VID 0 as their
> default VLAN since VID 0 has a special meaning.
> 
> Fixes: fea83353177a ("net: dsa: b53: Fix default VLAN ID")
> Fixes: 061f6a505ac3 ("net: dsa: Add ndo_vlan_rx_{add, kill}_vid implementation")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
[...]

MBR, Sergei


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

* Re: [PATCH net] net: dsa: b53: Ensure the default VID is untagged
  2020-02-13 19:10 [PATCH net] net: dsa: b53: Ensure the default VID is untagged Florian Fainelli
  2020-02-14  8:00 ` Sergei Shtylyov
@ 2020-02-14 10:36 ` Vladimir Oltean
  2020-02-14 17:08   ` Florian Fainelli
  1 sibling, 1 reply; 5+ messages in thread
From: Vladimir Oltean @ 2020-02-14 10:36 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: netdev, Andrew Lunn, Vivien Didelot, David S. Miller, open list

Hi Florian,

On Thu, 13 Feb 2020 at 21:10, Florian Fainelli <f.fainelli@gmail.com> wrote:
>
> We need to ensure that the default VID is untagged otherwise the switch
> will be sending frames tagged frames and the results can be problematic.
> This is especially true with b53 switches that use VID 0 as their
> default VLAN since VID 0 has a special meaning.
>
> Fixes: fea83353177a ("net: dsa: b53: Fix default VLAN ID")
> Fixes: 061f6a505ac3 ("net: dsa: Add ndo_vlan_rx_{add, kill}_vid implementation")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  drivers/net/dsa/b53/b53_common.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
> index 449a22172e07..f25c43b300d4 100644
> --- a/drivers/net/dsa/b53/b53_common.c
> +++ b/drivers/net/dsa/b53/b53_common.c
> @@ -1366,6 +1366,9 @@ void b53_vlan_add(struct dsa_switch *ds, int port,
>
>                 b53_get_vlan_entry(dev, vid, vl);
>
> +               if (vid == b53_default_pvid(dev))
> +                       untagged = true;
> +
>                 vl->members |= BIT(port);
>                 if (untagged && !dsa_is_cpu_port(ds, port))
>                         vl->untag |= BIT(port);
> --
> 2.17.1
>

Don't you mean to force untagged egress only for the pvid value of 0?

Thanks,
-Vladimir

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

* Re: [PATCH net] net: dsa: b53: Ensure the default VID is untagged
  2020-02-14 10:36 ` Vladimir Oltean
@ 2020-02-14 17:08   ` Florian Fainelli
  2020-02-14 17:15     ` Vladimir Oltean
  0 siblings, 1 reply; 5+ messages in thread
From: Florian Fainelli @ 2020-02-14 17:08 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: netdev, Andrew Lunn, Vivien Didelot, David S. Miller, open list



On 2/14/2020 2:36 AM, Vladimir Oltean wrote:
> Hi Florian,
> 
> On Thu, 13 Feb 2020 at 21:10, Florian Fainelli <f.fainelli@gmail.com> wrote:
>>
>> We need to ensure that the default VID is untagged otherwise the switch
>> will be sending frames tagged frames and the results can be problematic.
>> This is especially true with b53 switches that use VID 0 as their
>> default VLAN since VID 0 has a special meaning.
>>
>> Fixes: fea83353177a ("net: dsa: b53: Fix default VLAN ID")
>> Fixes: 061f6a505ac3 ("net: dsa: Add ndo_vlan_rx_{add, kill}_vid implementation")
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>>  drivers/net/dsa/b53/b53_common.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
>> index 449a22172e07..f25c43b300d4 100644
>> --- a/drivers/net/dsa/b53/b53_common.c
>> +++ b/drivers/net/dsa/b53/b53_common.c
>> @@ -1366,6 +1366,9 @@ void b53_vlan_add(struct dsa_switch *ds, int port,
>>
>>                 b53_get_vlan_entry(dev, vid, vl);
>>
>> +               if (vid == b53_default_pvid(dev))
>> +                       untagged = true;
>> +
>>                 vl->members |= BIT(port);
>>                 if (untagged && !dsa_is_cpu_port(ds, port))
>>                         vl->untag |= BIT(port);
>> --
>> 2.17.1
>>
> 
> Don't you mean to force untagged egress only for the pvid value of 0?

The default VID (0 for most switches, 1 for 5325/65) is configured as
pvid during b53_configure_vlan() so when we get a call to port_vlan_add
with VID == 0 this is coming exclusively from
dsa_slave_vlan_rx_add_vid() since the bridge will never program a VID <
1. When dsa_slave_vlan_rx_add_vid() calls us, we do not have any VLAN
flags set in the object.
-- 
Florian

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

* Re: [PATCH net] net: dsa: b53: Ensure the default VID is untagged
  2020-02-14 17:08   ` Florian Fainelli
@ 2020-02-14 17:15     ` Vladimir Oltean
  0 siblings, 0 replies; 5+ messages in thread
From: Vladimir Oltean @ 2020-02-14 17:15 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: netdev, Andrew Lunn, Vivien Didelot, David S. Miller, open list

On Fri, 14 Feb 2020 at 19:08, Florian Fainelli <f.fainelli@gmail.com> wrote:
>
>
>
> On 2/14/2020 2:36 AM, Vladimir Oltean wrote:
> > Hi Florian,
> >
> > On Thu, 13 Feb 2020 at 21:10, Florian Fainelli <f.fainelli@gmail.com> wrote:
> >>
> >> We need to ensure that the default VID is untagged otherwise the switch
> >> will be sending frames tagged frames and the results can be problematic.
> >> This is especially true with b53 switches that use VID 0 as their
> >> default VLAN since VID 0 has a special meaning.
> >>
> >> Fixes: fea83353177a ("net: dsa: b53: Fix default VLAN ID")
> >> Fixes: 061f6a505ac3 ("net: dsa: Add ndo_vlan_rx_{add, kill}_vid implementation")
> >> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> >> ---
> >>  drivers/net/dsa/b53/b53_common.c | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
> >> index 449a22172e07..f25c43b300d4 100644
> >> --- a/drivers/net/dsa/b53/b53_common.c
> >> +++ b/drivers/net/dsa/b53/b53_common.c
> >> @@ -1366,6 +1366,9 @@ void b53_vlan_add(struct dsa_switch *ds, int port,
> >>
> >>                 b53_get_vlan_entry(dev, vid, vl);
> >>
> >> +               if (vid == b53_default_pvid(dev))
> >> +                       untagged = true;
> >> +
> >>                 vl->members |= BIT(port);
> >>                 if (untagged && !dsa_is_cpu_port(ds, port))
> >>                         vl->untag |= BIT(port);
> >> --
> >> 2.17.1
> >>
> >
> > Don't you mean to force untagged egress only for the pvid value of 0?
>
> The default VID (0 for most switches, 1 for 5325/65) is configured as
> pvid during b53_configure_vlan() so when we get a call to port_vlan_add
> with VID == 0 this is coming exclusively from
> dsa_slave_vlan_rx_add_vid() since the bridge will never program a VID <
> 1. When dsa_slave_vlan_rx_add_vid() calls us, we do not have any VLAN
> flags set in the object.
> --
> Florian

Exactly. So the only case you need to guard against is when vid == 0
&& vid == b53_default_pvid(dev) - basically the 8021q module messing
with your untagged (pvid-tagged) traffic. So both have to be zero in
order to interfere. If vid is 0 but the b53_default_pvid is 1 - no
problem. If vid is 1 and the b53_default_pvid is 1, again no problem.
At least that's what you described in the previous patch. No?

-Vladimir

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

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-13 19:10 [PATCH net] net: dsa: b53: Ensure the default VID is untagged Florian Fainelli
2020-02-14  8:00 ` Sergei Shtylyov
2020-02-14 10:36 ` Vladimir Oltean
2020-02-14 17:08   ` Florian Fainelli
2020-02-14 17:15     ` Vladimir Oltean

Netdev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netdev/0 netdev/git/0.git
	git clone --mirror https://lore.kernel.org/netdev/1 netdev/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netdev netdev/ https://lore.kernel.org/netdev \
		netdev@vger.kernel.org
	public-inbox-index netdev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netdev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git