PROBLEM: (DSA/Microchip): 802.1Q-Header lost on KSZ9477-DSA ingress without bridge

PROBLEM: (DSA/Microchip): 802.1Q-Header lost on KSZ9477-DSA ingress without bridge

[Gaube, Marvin (THSE-TL1)]

Summary: 802.1Q-Header lost on KSZ9477-DSA ingress without bridge
Keywords: networking, dsa, microchip, 802.1q, vlan
Full description:

Hello,
we're trying to get 802.1Q-Tagged Ethernet Frames through an KSZ9477 DSA-enabled switch without creating a bridge on the kernel side.
Following setup:
Switchport 1 <-- KSZ9477 --> eth1 (CPU-Port) <---> lan1

No bridge is configured, only the interface directly. Untagged packets are working without problems. The Switch uses the ksz9477-DSA-Driver with Tail-Tagging ("DSA_TAG_PROTO_KSZ9477").
When sending packets with 802.1Q-Header (tagged VLAN) into the Switchport, I see them including the 802.1Q-Header on eth1.
They also appear on lan1, but with the 802.1Q-Header missing.
When I create an VLAN-Interface over lan1 (e.g. lan1.21), nothing arrives there.
The other way around, everything works fine: Packets transmitted into lan1.21 are appearing in 802.1Q-VLAN 21 on the Switchport 1.

I assume that is not the intended behavior.
I haven't found an obvious reason for this behavior yet, but I suspect the VLAN-Header gets stripped of anywhere around "dsa_switch_rcv" in net/dsa/dsa.c or "ksz9477_rcv" in net/dsa/tag_ksz.c.
Hints where the problem could be in detail are welcomed, I will try patches and looking into details.

Kernel Version: v5.4.51
Device Tree from example (https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/net/dsa/ksz.txt?h=v5.4 )

Thanks
Marvin Gaube

________________________________

Tesat-Spacecom GmbH & Co. KG
Sitz: Backnang; Registergericht: Amtsgericht Stuttgart HRA 270977
Persoenlich haftender Gesellschafter: Tesat-Spacecom Geschaeftsfuehrungs GmbH;
Sitz: Backnang; Registergericht: Amtsgericht Stuttgart HRB 271658;
Geschaeftsfuehrung: Dr. Marc Steckling, Kerstin Basche, Ralf Zimmermann

[banner]

Re: PROBLEM: (DSA/Microchip): 802.1Q-Header lost on KSZ9477-DSA ingress without bridge

[Florian Fainelli]

On 7/28/2020 11:05 PM, Gaube, Marvin (THSE-TL1) wrote:
> Summary: 802.1Q-Header lost on KSZ9477-DSA ingress without bridge
> Keywords: networking, dsa, microchip, 802.1q, vlan
> Full description:
> 
> Hello,
> we're trying to get 802.1Q-Tagged Ethernet Frames through an KSZ9477 DSA-enabled switch without creating a bridge on the kernel side.

Does it work if you have a bridge that is VLAN aware though? If it does,
this would suggest that the default VLAN behavior without a bridge is
too restrictive and needs changing.

> Following setup:
> Switchport 1 <-- KSZ9477 --> eth1 (CPU-Port) <---> lan1

This representation is confusing, is switchport 1 a network device or is
this meant to be physical switch port number of 1 of the KSZ9477?

> 
> No bridge is configured, only the interface directly. Untagged packets are working without problems. The Switch uses the ksz9477-DSA-Driver with Tail-Tagging ("DSA_TAG_PROTO_KSZ9477").
> When sending packets with 802.1Q-Header (tagged VLAN) into the Switchport, I see them including the 802.1Q-Header on eth1.
> They also appear on lan1, but with the 802.1Q-Header missing.
> When I create an VLAN-Interface over lan1 (e.g. lan1.21), nothing arrives there.
> The other way around, everything works fine: Packets transmitted into lan1.21 are appearing in 802.1Q-VLAN 21 on the Switchport 1.
> 
> I assume that is not the intended behavior.
> I haven't found an obvious reason for this behavior yet, but I suspect the VLAN-Header gets stripped of anywhere around "dsa_switch_rcv" in net/dsa/dsa.c or "ksz9477_rcv" in net/dsa/tag_ksz.c.

Not sure how though, ksz9477_rcv() only removes the trail tag, this
should leave any header intact. It seems to me that the switch is
incorrectly configured and is not VLAN aware at all, nor passing VLAN
tagged frames through on ingress to CPU when it should.
-- 
Florian

AW: PROBLEM: (DSA/Microchip): 802.1Q-Header lost on KSZ9477-DSA ingress without bridge

[Gaube, Marvin (THSE-TL1)]

Hello,
I just tried a VLAN-enabled bridge.
All ingress packets definitely have the 802.1q-Tag on CPU ingress, double-checked that. Tried again with VLAN21-Tagged frames coming in the physical port.
It seems that the bridge also handles all packets from lan1 as untagged. When I add lan1 to the bridge, the following happens:

If lan1 has (only) VLAN 21 tagged on the bridge, no packet appears.
As soon as I add an untagged/pvid VLAN to lan1 on the bridge, all packets appear on the bridge with whichever VLAN I added.
I checked simultaneously with the CPU Ingress-Port (eth1), the same packets had Ethertype 8100 with VLAN 21 when they entered CPU.

With Switchport 1, the physical switch port of the KSZ is meant.

About the last thing: VLAN tagged frames are definitively passed to the CPU.
If I "tcpdump -xx" onto eth1, I see for example "(12 byte MAC) 8100 0015 86dd (IPv6-Payload)". The tail tag is also visible.
Exactly the same frame appears on lan1 as "(12 byte MAC) 86dd (IPv6-Payload)", so the 802.1q-Header is present on CPU ingress.
Therefore the VLAN tag probably is lost between eth1 (Ingress) and the respective DSA-Interface, and is not filtered on the KSZ9477.

Best Regards
Marvin Gaube

-----Ursprüngliche Nachricht-----
Von: Florian Fainelli <f.fainelli@gmail.com>
Gesendet: Mittwoch, 29. Juli 2020 15:48
An: Gaube, Marvin (THSE-TL1) <Marvin.Gaube@tesat.de>; Woojung Huh <woojung.huh@microchip.com>; Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
Cc: netdev@vger.kernel.org
Betreff: Re: PROBLEM: (DSA/Microchip): 802.1Q-Header lost on KSZ9477-DSA ingress without bridge



On 7/28/2020 11:05 PM, Gaube, Marvin (THSE-TL1) wrote:
> Summary: 802.1Q-Header lost on KSZ9477-DSA ingress without bridge
> Keywords: networking, dsa, microchip, 802.1q, vlan Full description:
>
> Hello,
> we're trying to get 802.1Q-Tagged Ethernet Frames through an KSZ9477 DSA-enabled switch without creating a bridge on the kernel side.

Does it work if you have a bridge that is VLAN aware though? If it does, this would suggest that the default VLAN behavior without a bridge is too restrictive and needs changing.

> Following setup:
> Switchport 1 <-- KSZ9477 --> eth1 (CPU-Port) <---> lan1

This representation is confusing, is switchport 1 a network device or is this meant to be physical switch port number of 1 of the KSZ9477?

>
> No bridge is configured, only the interface directly. Untagged packets are working without problems. The Switch uses the ksz9477-DSA-Driver with Tail-Tagging ("DSA_TAG_PROTO_KSZ9477").
> When sending packets with 802.1Q-Header (tagged VLAN) into the Switchport, I see them including the 802.1Q-Header on eth1.
> They also appear on lan1, but with the 802.1Q-Header missing.
> When I create an VLAN-Interface over lan1 (e.g. lan1.21), nothing arrives there.
> The other way around, everything works fine: Packets transmitted into lan1.21 are appearing in 802.1Q-VLAN 21 on the Switchport 1.
>
> I assume that is not the intended behavior.
> I haven't found an obvious reason for this behavior yet, but I suspect the VLAN-Header gets stripped of anywhere around "dsa_switch_rcv" in net/dsa/dsa.c or "ksz9477_rcv" in net/dsa/tag_ksz.c.

Not sure how though, ksz9477_rcv() only removes the trail tag, this should leave any header intact. It seems to me that the switch is incorrectly configured and is not VLAN aware at all, nor passing VLAN tagged frames through on ingress to CPU when it should.
--
Florian

________________________________

Tesat-Spacecom GmbH & Co. KG
Sitz: Backnang; Registergericht: Amtsgericht Stuttgart HRA 270977
Persoenlich haftender Gesellschafter: Tesat-Spacecom Geschaeftsfuehrungs GmbH;
Sitz: Backnang; Registergericht: Amtsgericht Stuttgart HRB 271658;
Geschaeftsfuehrung: Dr. Marc Steckling, Kerstin Basche, Ralf Zimmermann

[banner]

Re: AW: PROBLEM: (DSA/Microchip): 802.1Q-Header lost on KSZ9477-DSA ingress without bridge

[Florian Fainelli]

On 7/29/2020 7:49 AM, Gaube, Marvin (THSE-TL1) wrote:
> Hello,
> I just tried a VLAN-enabled bridge.
> All ingress packets definitely have the 802.1q-Tag on CPU ingress, double-checked that. Tried again with VLAN21-Tagged frames coming in the physical port.
> It seems that the bridge also handles all packets from lan1 as untagged. When I add lan1 to the bridge, the following happens:
> 
> If lan1 has (only) VLAN 21 tagged on the bridge, no packet appears.
> As soon as I add an untagged/pvid VLAN to lan1 on the bridge, all packets appear on the bridge with whichever VLAN I added.
> I checked simultaneously with the CPU Ingress-Port (eth1), the same packets had Ethertype 8100 with VLAN 21 when they entered CPU.

Can you share the commands you use to set-up your bridge with VLAN
filtering and VLAN21 added to the VLAN database of the bridge for lan1?

> 
> With Switchport 1, the physical switch port of the KSZ is meant.

OK.

> 
> About the last thing: VLAN tagged frames are definitively passed to the CPU.
> If I "tcpdump -xx" onto eth1, I see for example "(12 byte MAC) 8100 0015 86dd (IPv6-Payload)". The tail tag is also visible.
> Exactly the same frame appears on lan1 as "(12 byte MAC) 86dd (IPv6-Payload)", so the 802.1q-Header is present on CPU ingress.
> Therefore the VLAN tag probably is lost between eth1 (Ingress) and the respective DSA-Interface, and is not filtered on the KSZ9477.

What Ethernet controller driver is eth1, does it support VLAN receive
filter (NETIF_F_HW_VLAN_CTAG_FILTER)?

> 
> Best Regards
> Marvin Gaube
> 
> -----Ursprüngliche Nachricht-----
> Von: Florian Fainelli <f.fainelli@gmail.com>
> Gesendet: Mittwoch, 29. Juli 2020 15:48
> An: Gaube, Marvin (THSE-TL1) <Marvin.Gaube@tesat.de>; Woojung Huh <woojung.huh@microchip.com>; Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
> Cc: netdev@vger.kernel.org
> Betreff: Re: PROBLEM: (DSA/Microchip): 802.1Q-Header lost on KSZ9477-DSA ingress without bridge
> 
> 
> 
> On 7/28/2020 11:05 PM, Gaube, Marvin (THSE-TL1) wrote:
>> Summary: 802.1Q-Header lost on KSZ9477-DSA ingress without bridge
>> Keywords: networking, dsa, microchip, 802.1q, vlan Full description:
>>
>> Hello,
>> we're trying to get 802.1Q-Tagged Ethernet Frames through an KSZ9477 DSA-enabled switch without creating a bridge on the kernel side.
> 
> Does it work if you have a bridge that is VLAN aware though? If it does, this would suggest that the default VLAN behavior without a bridge is too restrictive and needs changing.
> 
>> Following setup:
>> Switchport 1 <-- KSZ9477 --> eth1 (CPU-Port) <---> lan1
> 
> This representation is confusing, is switchport 1 a network device or is this meant to be physical switch port number of 1 of the KSZ9477?
> 
>>
>> No bridge is configured, only the interface directly. Untagged packets are working without problems. The Switch uses the ksz9477-DSA-Driver with Tail-Tagging ("DSA_TAG_PROTO_KSZ9477").
>> When sending packets with 802.1Q-Header (tagged VLAN) into the Switchport, I see them including the 802.1Q-Header on eth1.
>> They also appear on lan1, but with the 802.1Q-Header missing.
>> When I create an VLAN-Interface over lan1 (e.g. lan1.21), nothing arrives there.
>> The other way around, everything works fine: Packets transmitted into lan1.21 are appearing in 802.1Q-VLAN 21 on the Switchport 1.
>>
>> I assume that is not the intended behavior.
>> I haven't found an obvious reason for this behavior yet, but I suspect the VLAN-Header gets stripped of anywhere around "dsa_switch_rcv" in net/dsa/dsa.c or "ksz9477_rcv" in net/dsa/tag_ksz.c.
> 
> Not sure how though, ksz9477_rcv() only removes the trail tag, this should leave any header intact. It seems to me that the switch is incorrectly configured and is not VLAN aware at all, nor passing VLAN tagged frames through on ingress to CPU when it should.
> --
> Florian
> 
> ________________________________
> 
> Tesat-Spacecom GmbH & Co. KG
> Sitz: Backnang; Registergericht: Amtsgericht Stuttgart HRA 270977
> Persoenlich haftender Gesellschafter: Tesat-Spacecom Geschaeftsfuehrungs GmbH;
> Sitz: Backnang; Registergericht: Amtsgericht Stuttgart HRB 271658;
> Geschaeftsfuehrung: Dr. Marc Steckling, Kerstin Basche, Ralf Zimmermann
> 
> [banner]
> 

-- 
Florian

AW: AW: PROBLEM: (DSA/Microchip): 802.1Q-Header lost on KSZ9477-DSA ingress without bridge

[Gaube, Marvin (THSE-TL1)]

Hello,
the following was tested:

ip link add name br0 type bridge
echo 1 >/sys/class/net/br0/bridge/vlan_filtering
ip link set dev lan1 master br0
ip link set dev lan1 up
ip link set dev br0 up
bridge vlan show
> port              vlan-id  
> lan1              1 PVID Egress Untagged
> br0               1 PVID Egress Untagged
tcpdump -i br0 -e
> de:1c:87:(..) (oui Unknown) > 33:33:00:01:00:06 (oui Unknown), ethertype IPv6 (0x86dd), length 308 ...
bridge vlan del dev lan1 vid 1
bridge vlan add dev lan1 vid 21 tagged
bridge vlan show
> port              vlan-id  
> lan1              21
> br0               1 PVID Egress Untagged
tcpdump -i br0 -e
> Nothing. The frames with VLAN 21 ingress should appear here
bridge vlan del dev lan1 vid 21
bridge vlan add dev lan1 vid 25 pvid
bridge vlan show
> port              vlan-id  
> lan1              25 PVID
> br0               1 PVID Egress Untagged
tcpdump -i br0 -e
> de:1c:87:(..) (oui Unknown) > 33:33:00:01:00:06 (oui Unknown), ethertype 802.1Q (0x8100), length 312: vlan 25, p 0, ethertype IPv6 ...

When I tcpdump onto eth1, I see the packets with 0x8100 vid 21 all the time.

The MAC driver is freescale/fec on imx7d (compatible string in device tree: "fsl,imx7d-fecfsl,imx6sx-fec"). 
It seems, that it not sets NETIF_F_HW_VLAN_CTAG_FILTER.

Best Regards
Marvin Gaube

-----Ursprüngliche Nachricht-----
Von: Florian Fainelli <f.fainelli@gmail.com> 
Gesendet: Mittwoch, 29. Juli 2020 17:03
An: Gaube, Marvin (THSE-TL1) <Marvin.Gaube@tesat.de>; Woojung Huh <woojung.huh@microchip.com>; Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
Cc: netdev@vger.kernel.org
Betreff: Re: AW: PROBLEM: (DSA/Microchip): 802.1Q-Header lost on KSZ9477-DSA ingress without bridge



On 7/29/2020 7:49 AM, Gaube, Marvin (THSE-TL1) wrote:
> Hello,
> I just tried a VLAN-enabled bridge.
> All ingress packets definitely have the 802.1q-Tag on CPU ingress, double-checked that. Tried again with VLAN21-Tagged frames coming in the physical port.
> It seems that the bridge also handles all packets from lan1 as untagged. When I add lan1 to the bridge, the following happens:
> 
> If lan1 has (only) VLAN 21 tagged on the bridge, no packet appears.
> As soon as I add an untagged/pvid VLAN to lan1 on the bridge, all packets appear on the bridge with whichever VLAN I added.
> I checked simultaneously with the CPU Ingress-Port (eth1), the same packets had Ethertype 8100 with VLAN 21 when they entered CPU.

Can you share the commands you use to set-up your bridge with VLAN filtering and VLAN21 added to the VLAN database of the bridge for lan1?

> 
> With Switchport 1, the physical switch port of the KSZ is meant.

OK.

> 
> About the last thing: VLAN tagged frames are definitively passed to the CPU.
> If I "tcpdump -xx" onto eth1, I see for example "(12 byte MAC) 8100 0015 86dd (IPv6-Payload)". The tail tag is also visible.
> Exactly the same frame appears on lan1 as "(12 byte MAC) 86dd (IPv6-Payload)", so the 802.1q-Header is present on CPU ingress.
> Therefore the VLAN tag probably is lost between eth1 (Ingress) and the respective DSA-Interface, and is not filtered on the KSZ9477.

What Ethernet controller driver is eth1, does it support VLAN receive filter (NETIF_F_HW_VLAN_CTAG_FILTER)?

> 
> Best Regards
> Marvin Gaube
> 
> -----Ursprüngliche Nachricht-----
> Von: Florian Fainelli <f.fainelli@gmail.com>
> Gesendet: Mittwoch, 29. Juli 2020 15:48
> An: Gaube, Marvin (THSE-TL1) <Marvin.Gaube@tesat.de>; Woojung Huh 
> <woojung.huh@microchip.com>; Microchip Linux Driver Support 
> <UNGLinuxDriver@microchip.com>
> Cc: netdev@vger.kernel.org
> Betreff: Re: PROBLEM: (DSA/Microchip): 802.1Q-Header lost on 
> KSZ9477-DSA ingress without bridge
> 
> 
> 
> On 7/28/2020 11:05 PM, Gaube, Marvin (THSE-TL1) wrote:
>> Summary: 802.1Q-Header lost on KSZ9477-DSA ingress without bridge
>> Keywords: networking, dsa, microchip, 802.1q, vlan Full description:
>>
>> Hello,
>> we're trying to get 802.1Q-Tagged Ethernet Frames through an KSZ9477 DSA-enabled switch without creating a bridge on the kernel side.
> 
> Does it work if you have a bridge that is VLAN aware though? If it does, this would suggest that the default VLAN behavior without a bridge is too restrictive and needs changing.
> 
>> Following setup:
>> Switchport 1 <-- KSZ9477 --> eth1 (CPU-Port) <---> lan1
> 
> This representation is confusing, is switchport 1 a network device or is this meant to be physical switch port number of 1 of the KSZ9477?
> 
>>
>> No bridge is configured, only the interface directly. Untagged packets are working without problems. The Switch uses the ksz9477-DSA-Driver with Tail-Tagging ("DSA_TAG_PROTO_KSZ9477").
>> When sending packets with 802.1Q-Header (tagged VLAN) into the Switchport, I see them including the 802.1Q-Header on eth1.
>> They also appear on lan1, but with the 802.1Q-Header missing.
>> When I create an VLAN-Interface over lan1 (e.g. lan1.21), nothing arrives there.
>> The other way around, everything works fine: Packets transmitted into lan1.21 are appearing in 802.1Q-VLAN 21 on the Switchport 1.
>>
>> I assume that is not the intended behavior.
>> I haven't found an obvious reason for this behavior yet, but I suspect the VLAN-Header gets stripped of anywhere around "dsa_switch_rcv" in net/dsa/dsa.c or "ksz9477_rcv" in net/dsa/tag_ksz.c.
> 
> Not sure how though, ksz9477_rcv() only removes the trail tag, this should leave any header intact. It seems to me that the switch is incorrectly configured and is not VLAN aware at all, nor passing VLAN tagged frames through on ingress to CPU when it should.
> --
> Florian
> 
> ________________________________
> 
> Tesat-Spacecom GmbH & Co. KG
> Sitz: Backnang; Registergericht: Amtsgericht Stuttgart HRA 270977 
> Persoenlich haftender Gesellschafter: Tesat-Spacecom 
> Geschaeftsfuehrungs GmbH;
> Sitz: Backnang; Registergericht: Amtsgericht Stuttgart HRB 271658;
> Geschaeftsfuehrung: Dr. Marc Steckling, Kerstin Basche, Ralf 
> Zimmermann
> 
> [banner]
> 

--
Florian

AW: AW: PROBLEM: (DSA/Microchip): 802.1Q-Header lost on KSZ9477-DSA ingress without bridge

[Gaube, Marvin (THSE-TL1)]

Hello,
I looked into it deeper, the driver does rxvlan offloading. 
By disabling it manually trough ethtool, the behavior becomes as expected.

I've taken "net: dsa: sja1105: disable rxvlan offload for the DSA master" from (https://lore.kernel.org/netdev/20200512234921.25460-1-olteanv@gmail.com/) and also applied it to the KSZ9477-Driver, which fixes the problem.
It's probably a workaround, but fixes the VLAN behavior for now. I would suggest also applying "ds->disable_master_rxvlan = true;" to KSZ9477 after the mentioned patch is merged.

Best Regards
Marvin Gaube

-----Ursprüngliche Nachricht-----
Von: Gaube, Marvin (THSE-TL1) 
Gesendet: Donnerstag, 30. Juli 2020 11:35
An: 'Florian Fainelli' <f.fainelli@gmail.com>; Woojung Huh <woojung.huh@microchip.com>; Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
Cc: netdev@vger.kernel.org
Betreff: AW: AW: PROBLEM: (DSA/Microchip): 802.1Q-Header lost on KSZ9477-DSA ingress without bridge

Hello,
the following was tested:

ip link add name br0 type bridge
echo 1 >/sys/class/net/br0/bridge/vlan_filtering
ip link set dev lan1 master br0
ip link set dev lan1 up
ip link set dev br0 up
bridge vlan show
> port              vlan-id  
> lan1              1 PVID Egress Untagged
> br0               1 PVID Egress Untagged
tcpdump -i br0 -e
> de:1c:87:(..) (oui Unknown) > 33:33:00:01:00:06 (oui Unknown), ethertype IPv6 (0x86dd), length 308 ...
bridge vlan del dev lan1 vid 1
bridge vlan add dev lan1 vid 21 tagged
bridge vlan show
> port              vlan-id  
> lan1              21
> br0               1 PVID Egress Untagged
tcpdump -i br0 -e
> Nothing. The frames with VLAN 21 ingress should appear here
bridge vlan del dev lan1 vid 21
bridge vlan add dev lan1 vid 25 pvid
bridge vlan show
> port              vlan-id  
> lan1              25 PVID
> br0               1 PVID Egress Untagged
tcpdump -i br0 -e
> de:1c:87:(..) (oui Unknown) > 33:33:00:01:00:06 (oui Unknown), ethertype 802.1Q (0x8100), length 312: vlan 25, p 0, ethertype IPv6 ...

When I tcpdump onto eth1, I see the packets with 0x8100 vid 21 all the time.

The MAC driver is freescale/fec on imx7d (compatible string in device tree: "fsl,imx7d-fecfsl,imx6sx-fec"). 
It seems, that it not sets NETIF_F_HW_VLAN_CTAG_FILTER.

Best Regards
Marvin Gaube

-----Ursprüngliche Nachricht-----
Von: Florian Fainelli <f.fainelli@gmail.com> 
Gesendet: Mittwoch, 29. Juli 2020 17:03
An: Gaube, Marvin (THSE-TL1) <Marvin.Gaube@tesat.de>; Woojung Huh <woojung.huh@microchip.com>; Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
Cc: netdev@vger.kernel.org
Betreff: Re: AW: PROBLEM: (DSA/Microchip): 802.1Q-Header lost on KSZ9477-DSA ingress without bridge



On 7/29/2020 7:49 AM, Gaube, Marvin (THSE-TL1) wrote:
> Hello,
> I just tried a VLAN-enabled bridge.
> All ingress packets definitely have the 802.1q-Tag on CPU ingress, double-checked that. Tried again with VLAN21-Tagged frames coming in the physical port.
> It seems that the bridge also handles all packets from lan1 as untagged. When I add lan1 to the bridge, the following happens:
> 
> If lan1 has (only) VLAN 21 tagged on the bridge, no packet appears.
> As soon as I add an untagged/pvid VLAN to lan1 on the bridge, all packets appear on the bridge with whichever VLAN I added.
> I checked simultaneously with the CPU Ingress-Port (eth1), the same packets had Ethertype 8100 with VLAN 21 when they entered CPU.

Can you share the commands you use to set-up your bridge with VLAN filtering and VLAN21 added to the VLAN database of the bridge for lan1?

> 
> With Switchport 1, the physical switch port of the KSZ is meant.

OK.

> 
> About the last thing: VLAN tagged frames are definitively passed to the CPU.
> If I "tcpdump -xx" onto eth1, I see for example "(12 byte MAC) 8100 0015 86dd (IPv6-Payload)". The tail tag is also visible.
> Exactly the same frame appears on lan1 as "(12 byte MAC) 86dd (IPv6-Payload)", so the 802.1q-Header is present on CPU ingress.
> Therefore the VLAN tag probably is lost between eth1 (Ingress) and the respective DSA-Interface, and is not filtered on the KSZ9477.

What Ethernet controller driver is eth1, does it support VLAN receive filter (NETIF_F_HW_VLAN_CTAG_FILTER)?

> 
> Best Regards
> Marvin Gaube
> 
> -----Ursprüngliche Nachricht-----
> Von: Florian Fainelli <f.fainelli@gmail.com>
> Gesendet: Mittwoch, 29. Juli 2020 15:48
> An: Gaube, Marvin (THSE-TL1) <Marvin.Gaube@tesat.de>; Woojung Huh 
> <woojung.huh@microchip.com>; Microchip Linux Driver Support 
> <UNGLinuxDriver@microchip.com>
> Cc: netdev@vger.kernel.org
> Betreff: Re: PROBLEM: (DSA/Microchip): 802.1Q-Header lost on 
> KSZ9477-DSA ingress without bridge
> 
> 
> 
> On 7/28/2020 11:05 PM, Gaube, Marvin (THSE-TL1) wrote:
>> Summary: 802.1Q-Header lost on KSZ9477-DSA ingress without bridge
>> Keywords: networking, dsa, microchip, 802.1q, vlan Full description:
>>
>> Hello,
>> we're trying to get 802.1Q-Tagged Ethernet Frames through an KSZ9477 DSA-enabled switch without creating a bridge on the kernel side.
> 
> Does it work if you have a bridge that is VLAN aware though? If it does, this would suggest that the default VLAN behavior without a bridge is too restrictive and needs changing.
> 
>> Following setup:
>> Switchport 1 <-- KSZ9477 --> eth1 (CPU-Port) <---> lan1
> 
> This representation is confusing, is switchport 1 a network device or is this meant to be physical switch port number of 1 of the KSZ9477?
> 
>>
>> No bridge is configured, only the interface directly. Untagged packets are working without problems. The Switch uses the ksz9477-DSA-Driver with Tail-Tagging ("DSA_TAG_PROTO_KSZ9477").
>> When sending packets with 802.1Q-Header (tagged VLAN) into the Switchport, I see them including the 802.1Q-Header on eth1.
>> They also appear on lan1, but with the 802.1Q-Header missing.
>> When I create an VLAN-Interface over lan1 (e.g. lan1.21), nothing arrives there.
>> The other way around, everything works fine: Packets transmitted into lan1.21 are appearing in 802.1Q-VLAN 21 on the Switchport 1.
>>
>> I assume that is not the intended behavior.
>> I haven't found an obvious reason for this behavior yet, but I suspect the VLAN-Header gets stripped of anywhere around "dsa_switch_rcv" in net/dsa/dsa.c or "ksz9477_rcv" in net/dsa/tag_ksz.c.
> 
> Not sure how though, ksz9477_rcv() only removes the trail tag, this should leave any header intact. It seems to me that the switch is incorrectly configured and is not VLAN aware at all, nor passing VLAN tagged frames through on ingress to CPU when it should.
> --
> Florian
> 
> ________________________________
> 
> Tesat-Spacecom GmbH & Co. KG
> Sitz: Backnang; Registergericht: Amtsgericht Stuttgart HRA 270977 
> Persoenlich haftender Gesellschafter: Tesat-Spacecom 
> Geschaeftsfuehrungs GmbH;
> Sitz: Backnang; Registergericht: Amtsgericht Stuttgart HRB 271658;
> Geschaeftsfuehrung: Dr. Marc Steckling, Kerstin Basche, Ralf 
> Zimmermann
> 
> [banner]
> 

--
Florian

Re: PROBLEM: (DSA/Microchip): 802.1Q-Header lost on KSZ9477-DSA ingress without bridge

[Vladimir Oltean]

On Tue, Aug 04, 2020 at 02:14:33PM +0000, Gaube, Marvin (THSE-TL1) wrote:
> Hello,
> I looked into it deeper, the driver does rxvlan offloading. 
> By disabling it manually trough ethtool, the behavior becomes as
> expected.
> 
> I've taken "net: dsa: sja1105: disable rxvlan offload for the DSA
> master" from
> (https://lore.kernel.org/netdev/20200512234921.25460-1-olteanv@gmail.com/)
> and also applied it to the KSZ9477-Driver, which fixes the problem.
> It's probably a workaround, but fixes the VLAN behavior for now. I
> would suggest also applying "ds->disable_master_rxvlan = true;" to
> KSZ9477 after the mentioned patch is merged.
> 
> Best Regards
> Marvin Gaube
> 

And I wanted to suggest that, but it seemed too freaky to be what's
going on.... But since ksz9477 uses a tail tag, it makes perfect sense.

My patch is in limbo because Eric, who started zeroing the skb offloaded
VLAN data in the first place, hasn't said anything.

Thanks,
-Vladimir

Re: PROBLEM: (DSA/Microchip): 802.1Q-Header lost on KSZ9477-DSA ingress without bridge

[Eric Dumazet]

On Tue, Aug 4, 2020 at 7:27 AM Vladimir Oltean <olteanv@gmail.com> wrote:
>
> On Tue, Aug 04, 2020 at 02:14:33PM +0000, Gaube, Marvin (THSE-TL1) wrote:
> > Hello,
> > I looked into it deeper, the driver does rxvlan offloading.
> > By disabling it manually trough ethtool, the behavior becomes as
> > expected.
> >
> > I've taken "net: dsa: sja1105: disable rxvlan offload for the DSA
> > master" from
> > (https://lore.kernel.org/netdev/20200512234921.25460-1-olteanv@gmail.com/)
> > and also applied it to the KSZ9477-Driver, which fixes the problem.
> > It's probably a workaround, but fixes the VLAN behavior for now. I
> > would suggest also applying "ds->disable_master_rxvlan = true;" to
> > KSZ9477 after the mentioned patch is merged.
> >
> > Best Regards
> > Marvin Gaube
> >
>
> And I wanted to suggest that, but it seemed too freaky to be what's
> going on.... But since ksz9477 uses a tail tag, it makes perfect sense.
>
> My patch is in limbo because Eric, who started zeroing the skb offloaded
> VLAN data in the first place, hasn't said anything.

I said nothing because I was not aware you were expecting something
special from me ;)

I receive hundreds of emails per day.

My 2013 commit was a bug fix, and hinted that in the future (eg in
net-next tree) the stop-the-bleed could be refined.

+               /* Note: we might in the future use prio bits
+                * and set skb->priority like in vlan_do_receive()
+                * For the time being, just ignore Priority Code Point
+                */
+               skb->vlan_tci = 0;

If you believe this can be done, this is great.

Re: AW: AW: PROBLEM: (DSA/Microchip): 802.1Q-Header lost on KSZ9477-DSA ingress without bridge

[Florian Fainelli]

"I looked into it deeper, the driver does rxvlan offloading."

Is this part of the driver upstream or are you using a vendor tree from
Freescale which has that change included?

On 8/4/2020 7:14 AM, Gaube, Marvin (THSE-TL1) wrote:
> Hello,
> I looked into it deeper, the driver does rxvlan offloading. 
> By disabling it manually trough ethtool, the behavior becomes as expected.
> 
> I've taken "net: dsa: sja1105: disable rxvlan offload for the DSA master" from (https://lore.kernel.org/netdev/20200512234921.25460-1-olteanv@gmail.com/) and also applied it to the KSZ9477-Driver, which fixes the problem.
> It's probably a workaround, but fixes the VLAN behavior for now. I would suggest also applying "ds->disable_master_rxvlan = true;" to KSZ9477 after the mentioned patch is merged.
> 
> Best Regards
> Marvin Gaube
> 
> -----Ursprüngliche Nachricht-----
> Von: Gaube, Marvin (THSE-TL1) 
> Gesendet: Donnerstag, 30. Juli 2020 11:35
> An: 'Florian Fainelli' <f.fainelli@gmail.com>; Woojung Huh <woojung.huh@microchip.com>; Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
> Cc: netdev@vger.kernel.org
> Betreff: AW: AW: PROBLEM: (DSA/Microchip): 802.1Q-Header lost on KSZ9477-DSA ingress without bridge
> 
> Hello,
> the following was tested:
> 
> ip link add name br0 type bridge
> echo 1 >/sys/class/net/br0/bridge/vlan_filtering
> ip link set dev lan1 master br0
> ip link set dev lan1 up
> ip link set dev br0 up
> bridge vlan show
>> port              vlan-id  
>> lan1              1 PVID Egress Untagged
>> br0               1 PVID Egress Untagged
> tcpdump -i br0 -e
>> de:1c:87:(..) (oui Unknown) > 33:33:00:01:00:06 (oui Unknown), ethertype IPv6 (0x86dd), length 308 ...
> bridge vlan del dev lan1 vid 1
> bridge vlan add dev lan1 vid 21 tagged
> bridge vlan show
>> port              vlan-id  
>> lan1              21
>> br0               1 PVID Egress Untagged
> tcpdump -i br0 -e
>> Nothing. The frames with VLAN 21 ingress should appear here
> bridge vlan del dev lan1 vid 21
> bridge vlan add dev lan1 vid 25 pvid
> bridge vlan show
>> port              vlan-id  
>> lan1              25 PVID
>> br0               1 PVID Egress Untagged
> tcpdump -i br0 -e
>> de:1c:87:(..) (oui Unknown) > 33:33:00:01:00:06 (oui Unknown), ethertype 802.1Q (0x8100), length 312: vlan 25, p 0, ethertype IPv6 ...
> 
> When I tcpdump onto eth1, I see the packets with 0x8100 vid 21 all the time.
> 
> The MAC driver is freescale/fec on imx7d (compatible string in device tree: "fsl,imx7d-fecfsl,imx6sx-fec"). 
> It seems, that it not sets NETIF_F_HW_VLAN_CTAG_FILTER.
> 
> Best Regards
> Marvin Gaube
> 
> -----Ursprüngliche Nachricht-----
> Von: Florian Fainelli <f.fainelli@gmail.com> 
> Gesendet: Mittwoch, 29. Juli 2020 17:03
> An: Gaube, Marvin (THSE-TL1) <Marvin.Gaube@tesat.de>; Woojung Huh <woojung.huh@microchip.com>; Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
> Cc: netdev@vger.kernel.org
> Betreff: Re: AW: PROBLEM: (DSA/Microchip): 802.1Q-Header lost on KSZ9477-DSA ingress without bridge
> 
> 
> 
> On 7/29/2020 7:49 AM, Gaube, Marvin (THSE-TL1) wrote:
>> Hello,
>> I just tried a VLAN-enabled bridge.
>> All ingress packets definitely have the 802.1q-Tag on CPU ingress, double-checked that. Tried again with VLAN21-Tagged frames coming in the physical port.
>> It seems that the bridge also handles all packets from lan1 as untagged. When I add lan1 to the bridge, the following happens:
>>
>> If lan1 has (only) VLAN 21 tagged on the bridge, no packet appears.
>> As soon as I add an untagged/pvid VLAN to lan1 on the bridge, all packets appear on the bridge with whichever VLAN I added.
>> I checked simultaneously with the CPU Ingress-Port (eth1), the same packets had Ethertype 8100 with VLAN 21 when they entered CPU.
> 
> Can you share the commands you use to set-up your bridge with VLAN filtering and VLAN21 added to the VLAN database of the bridge for lan1?
> 
>>
>> With Switchport 1, the physical switch port of the KSZ is meant.
> 
> OK.
> 
>>
>> About the last thing: VLAN tagged frames are definitively passed to the CPU.
>> If I "tcpdump -xx" onto eth1, I see for example "(12 byte MAC) 8100 0015 86dd (IPv6-Payload)". The tail tag is also visible.
>> Exactly the same frame appears on lan1 as "(12 byte MAC) 86dd (IPv6-Payload)", so the 802.1q-Header is present on CPU ingress.
>> Therefore the VLAN tag probably is lost between eth1 (Ingress) and the respective DSA-Interface, and is not filtered on the KSZ9477.
> 
> What Ethernet controller driver is eth1, does it support VLAN receive filter (NETIF_F_HW_VLAN_CTAG_FILTER)?
> 
>>
>> Best Regards
>> Marvin Gaube
>>
>> -----Ursprüngliche Nachricht-----
>> Von: Florian Fainelli <f.fainelli@gmail.com>
>> Gesendet: Mittwoch, 29. Juli 2020 15:48
>> An: Gaube, Marvin (THSE-TL1) <Marvin.Gaube@tesat.de>; Woojung Huh 
>> <woojung.huh@microchip.com>; Microchip Linux Driver Support 
>> <UNGLinuxDriver@microchip.com>
>> Cc: netdev@vger.kernel.org
>> Betreff: Re: PROBLEM: (DSA/Microchip): 802.1Q-Header lost on 
>> KSZ9477-DSA ingress without bridge
>>
>>
>>
>> On 7/28/2020 11:05 PM, Gaube, Marvin (THSE-TL1) wrote:
>>> Summary: 802.1Q-Header lost on KSZ9477-DSA ingress without bridge
>>> Keywords: networking, dsa, microchip, 802.1q, vlan Full description:
>>>
>>> Hello,
>>> we're trying to get 802.1Q-Tagged Ethernet Frames through an KSZ9477 DSA-enabled switch without creating a bridge on the kernel side.
>>
>> Does it work if you have a bridge that is VLAN aware though? If it does, this would suggest that the default VLAN behavior without a bridge is too restrictive and needs changing.
>>
>>> Following setup:
>>> Switchport 1 <-- KSZ9477 --> eth1 (CPU-Port) <---> lan1
>>
>> This representation is confusing, is switchport 1 a network device or is this meant to be physical switch port number of 1 of the KSZ9477?
>>
>>>
>>> No bridge is configured, only the interface directly. Untagged packets are working without problems. The Switch uses the ksz9477-DSA-Driver with Tail-Tagging ("DSA_TAG_PROTO_KSZ9477").
>>> When sending packets with 802.1Q-Header (tagged VLAN) into the Switchport, I see them including the 802.1Q-Header on eth1.
>>> They also appear on lan1, but with the 802.1Q-Header missing.
>>> When I create an VLAN-Interface over lan1 (e.g. lan1.21), nothing arrives there.
>>> The other way around, everything works fine: Packets transmitted into lan1.21 are appearing in 802.1Q-VLAN 21 on the Switchport 1.
>>>
>>> I assume that is not the intended behavior.
>>> I haven't found an obvious reason for this behavior yet, but I suspect the VLAN-Header gets stripped of anywhere around "dsa_switch_rcv" in net/dsa/dsa.c or "ksz9477_rcv" in net/dsa/tag_ksz.c.
>>
>> Not sure how though, ksz9477_rcv() only removes the trail tag, this should leave any header intact. It seems to me that the switch is incorrectly configured and is not VLAN aware at all, nor passing VLAN tagged frames through on ingress to CPU when it should.
>> --
>> Florian
>>
>> ________________________________
>>
>> Tesat-Spacecom GmbH & Co. KG
>> Sitz: Backnang; Registergericht: Amtsgericht Stuttgart HRA 270977 
>> Persoenlich haftender Gesellschafter: Tesat-Spacecom 
>> Geschaeftsfuehrungs GmbH;
>> Sitz: Backnang; Registergericht: Amtsgericht Stuttgart HRB 271658;
>> Geschaeftsfuehrung: Dr. Marc Steckling, Kerstin Basche, Ralf 
>> Zimmermann
>>
>> [banner]
>>
> 
> --
> Florian
> 

-- 
Florian

Re: PROBLEM: (DSA/Microchip): 802.1Q-Header lost on KSZ9477-DSA ingress without bridge

[Vladimir Oltean]

On Tue, Aug 04, 2020 at 07:54:18AM -0700, Eric Dumazet wrote:
> 
> My 2013 commit was a bug fix, and hinted that in the future (eg in
> net-next tree) the stop-the-bleed could be refined.
> 
> +               /* Note: we might in the future use prio bits
> +                * and set skb->priority like in vlan_do_receive()
> +                * For the time being, just ignore Priority Code Point
> +                */
> +               skb->vlan_tci = 0;
> 
> If you believe this can be done, this is great.

Do you have a reproducer for that bug? I am willing to spend some time
understand what is going on. This has nothing to do with priority. You
vaguely described a problem with 802.1p (VLAN 0) and used that as an
excuse to clear the entire vlan hwaccel tag regardless of VLAN ID. I'm
curious because we also now have commit 36b2f61a42c2 ("net: handle
802.1P vlan 0 packets properly") in that general area, and I simply want
to know if your patch still serves a valid purpose or not.

Thanks,
-Vladimir

Re: PROBLEM: (DSA/Microchip): 802.1Q-Header lost on KSZ9477-DSA ingress without bridge

[Eric Dumazet]

On Tue, Aug 4, 2020 at 12:29 PM Vladimir Oltean <olteanv@gmail.com> wrote:
>
> On Tue, Aug 04, 2020 at 07:54:18AM -0700, Eric Dumazet wrote:
> >
> > My 2013 commit was a bug fix, and hinted that in the future (eg in
> > net-next tree) the stop-the-bleed could be refined.
> >
> > +               /* Note: we might in the future use prio bits
> > +                * and set skb->priority like in vlan_do_receive()
> > +                * For the time being, just ignore Priority Code Point
> > +                */
> > +               skb->vlan_tci = 0;
> >
> > If you believe this can be done, this is great.
>
> Do you have a reproducer for that bug? I am willing to spend some time
> understand what is going on. This has nothing to do with priority. You
> vaguely described a problem with 802.1p (VLAN 0) and used that as an
> excuse to clear the entire vlan hwaccel tag regardless of VLAN ID. I'm
> curious because we also now have commit 36b2f61a42c2 ("net: handle
> 802.1P vlan 0 packets properly") in that general area, and I simply want
> to know if your patch still serves a valid purpose or not.
>

I do not have a repro, the patch seemed to help at that time,
according to the reporter.

Re: PROBLEM: (DSA/Microchip): 802.1Q-Header lost on KSZ9477-DSA ingress without bridge

[Vladimir Oltean]

On Tue, Aug 04, 2020 at 12:40:24PM -0700, Eric Dumazet wrote:
> On Tue, Aug 4, 2020 at 12:29 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> >
> > On Tue, Aug 04, 2020 at 07:54:18AM -0700, Eric Dumazet wrote:
> > >
> > > My 2013 commit was a bug fix, and hinted that in the future (eg in
> > > net-next tree) the stop-the-bleed could be refined.
> > >
> > > +               /* Note: we might in the future use prio bits
> > > +                * and set skb->priority like in vlan_do_receive()
> > > +                * For the time being, just ignore Priority Code Point
> > > +                */
> > > +               skb->vlan_tci = 0;
> > >
> > > If you believe this can be done, this is great.
> >
> > Do you have a reproducer for that bug? I am willing to spend some time
> > understand what is going on. This has nothing to do with priority. You
> > vaguely described a problem with 802.1p (VLAN 0) and used that as an
> > excuse to clear the entire vlan hwaccel tag regardless of VLAN ID. I'm
> > curious because we also now have commit 36b2f61a42c2 ("net: handle
> > 802.1P vlan 0 packets properly") in that general area, and I simply want
> > to know if your patch still serves a valid purpose or not.
> >
> 
> I do not have a repro, the patch seemed to help at that time,
> according to the reporter.

Do you mind if I respectfully revert then? It's clear that the patch has
loopholes already (it clears the vlan if it's hwaccel, but leaves it
alone if it isn't) and that the proper solution should be different
anyway.

Re: PROBLEM: (DSA/Microchip): 802.1Q-Header lost on KSZ9477-DSA ingress without bridge

[Vladimir Oltean]

On Tue, Aug 04, 2020 at 08:51:00AM -0700, Florian Fainelli wrote:
> "I looked into it deeper, the driver does rxvlan offloading."
> 
> Is this part of the driver upstream or are you using a vendor tree from
> Freescale which has that change included?
> 

Does it matter?
FWIW, mainline fec does declare NETIF_F_HW_VLAN_CTAG_RX:
https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/freescale/fec_main.c#L3317
and move it to the hwaccel area on RX:
https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/freescale/fec_main.c#L1513

Re: PROBLEM: (DSA/Microchip): 802.1Q-Header lost on KSZ9477-DSA ingress without bridge

[Florian Fainelli]

On 8/4/2020 12:54 PM, Vladimir Oltean wrote:
> On Tue, Aug 04, 2020 at 08:51:00AM -0700, Florian Fainelli wrote:
>> "I looked into it deeper, the driver does rxvlan offloading."
>>
>> Is this part of the driver upstream or are you using a vendor tree from
>> Freescale which has that change included?
>>
> 
> Does it matter?

If this was a downstream problem yes, there would not necessarily be a
pressing need to address it with a patch targeting 'net' for instance.
This is not the case, therefore it should be addressed.

> FWIW, mainline fec does declare NETIF_F_HW_VLAN_CTAG_RX:
> https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/freescale/fec_main.c#L3317
> and move it to the hwaccel area on RX:
> https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/freescale/fec_main.c#L1513
> 

-- 
Florian

Re: PROBLEM: (DSA/Microchip): 802.1Q-Header lost on KSZ9477-DSA ingress without bridge

[Eric Dumazet]

On Tue, Aug 4, 2020 at 12:43 PM Vladimir Oltean <olteanv@gmail.com> wrote:
>
> On Tue, Aug 04, 2020 at 12:40:24PM -0700, Eric Dumazet wrote:
> > On Tue, Aug 4, 2020 at 12:29 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> > >
> > > On Tue, Aug 04, 2020 at 07:54:18AM -0700, Eric Dumazet wrote:
> > > >
> > > > My 2013 commit was a bug fix, and hinted that in the future (eg in
> > > > net-next tree) the stop-the-bleed could be refined.
> > > >
> > > > +               /* Note: we might in the future use prio bits
> > > > +                * and set skb->priority like in vlan_do_receive()
> > > > +                * For the time being, just ignore Priority Code Point
> > > > +                */
> > > > +               skb->vlan_tci = 0;
> > > >
> > > > If you believe this can be done, this is great.
> > >
> > > Do you have a reproducer for that bug? I am willing to spend some time
> > > understand what is going on. This has nothing to do with priority. You
> > > vaguely described a problem with 802.1p (VLAN 0) and used that as an
> > > excuse to clear the entire vlan hwaccel tag regardless of VLAN ID. I'm
> > > curious because we also now have commit 36b2f61a42c2 ("net: handle
> > > 802.1P vlan 0 packets properly") in that general area, and I simply want
> > > to know if your patch still serves a valid purpose or not.
> > >
> >
> > I do not have a repro, the patch seemed to help at that time,
> > according to the reporter.
>
> Do you mind if I respectfully revert then? It's clear that the patch has
> loopholes already (it clears the vlan if it's hwaccel, but leaves it
> alone if it isn't) and that the proper solution should be different
> anyway.

Clearly the situation before the patch was not good, it seems well
explained in the changelog.

If you want to revert, you will need to convince the bug has been
solved in another way.

So it seems you might have to repro the initial problem.

Re: PROBLEM: (DSA/Microchip): 802.1Q-Header lost on KSZ9477-DSA ingress without bridge

[Vladimir Oltean]

On Tue, Aug 04, 2020 at 01:36:56PM -0700, Eric Dumazet wrote:
> On Tue, Aug 4, 2020 at 12:43 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> >
> > On Tue, Aug 04, 2020 at 12:40:24PM -0700, Eric Dumazet wrote:
> > > On Tue, Aug 4, 2020 at 12:29 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> > > >
> > > > On Tue, Aug 04, 2020 at 07:54:18AM -0700, Eric Dumazet wrote:
> > > > >
> > > > > My 2013 commit was a bug fix, and hinted that in the future (eg in
> > > > > net-next tree) the stop-the-bleed could be refined.
> > > > >
> > > > > +               /* Note: we might in the future use prio bits
> > > > > +                * and set skb->priority like in vlan_do_receive()
> > > > > +                * For the time being, just ignore Priority Code Point
> > > > > +                */
> > > > > +               skb->vlan_tci = 0;
> > > > >
> > > > > If you believe this can be done, this is great.
> > > >
> > > > Do you have a reproducer for that bug? I am willing to spend some time
> > > > understand what is going on. This has nothing to do with priority. You
> > > > vaguely described a problem with 802.1p (VLAN 0) and used that as an
> > > > excuse to clear the entire vlan hwaccel tag regardless of VLAN ID. I'm
> > > > curious because we also now have commit 36b2f61a42c2 ("net: handle
> > > > 802.1P vlan 0 packets properly") in that general area, and I simply want
> > > > to know if your patch still serves a valid purpose or not.
> > > >
> > >
> > > I do not have a repro, the patch seemed to help at that time,
> > > according to the reporter.
> >
> > Do you mind if I respectfully revert then? It's clear that the patch has
> > loopholes already (it clears the vlan if it's hwaccel, but leaves it
> > alone if it isn't) and that the proper solution should be different
> > anyway.
> 
> Clearly the situation before the patch was not good, it seems well
> explained in the changelog.
> 
> If you want to revert, you will need to convince the bug has been
> solved in another way.
> 
> So it seems you might have to repro the initial problem.

What bug? What repro? You just said you don't have any.

Maybe I'm dumb, but the changelog is vague to me. It isn't clear what
kind of routing it is, what type of traffic was the router being
subjected to, from what direction was the VLAN traffic coming, was it
just VLAN 0 that was problematic, what drivers those were, what kernel
was used, what has any of that have to do with the referenced commit
48cc32d38a52 ("vlan: don't deliver frames for unknown vlans to
protocols") which is about macvlan returning RX_HANDLER_PASS instead of
RX_HANDLER_ANOTHER, were there other sub-interfaces as well?
And there are also obvious mistakes in the commit description: "if the
vlan id is set and we could find a vlan device for this particular id."
-> "couldn't" should be instead of "could".

This is ridiculous. Not only will I not waste my time as long as there
is nothing actionable (I could test stuff until the cows come home and I
would never know if I'm under your scenario or not), but I do feel that
it's fundamentally your job to prove that there's a bug for which this
is the right fix, rather than me to disprove it.

I'm sorry, I want to give a helping hand, but it doesn't look like I
can.

Re: PROBLEM: (DSA/Microchip): 802.1Q-Header lost on KSZ9477-DSA ingress without bridge

[Eric Dumazet]

On Tue, Aug 4, 2020 at 2:24 PM Vladimir Oltean <olteanv@gmail.com> wrote:
>
> On Tue, Aug 04, 2020 at 01:36:56PM -0700, Eric Dumazet wrote:
> > On Tue, Aug 4, 2020 at 12:43 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> > >
> > > On Tue, Aug 04, 2020 at 12:40:24PM -0700, Eric Dumazet wrote:
> > > > On Tue, Aug 4, 2020 at 12:29 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> > > > >
> > > > > On Tue, Aug 04, 2020 at 07:54:18AM -0700, Eric Dumazet wrote:
> > > > > >
> > > > > > My 2013 commit was a bug fix, and hinted that in the future (eg in
> > > > > > net-next tree) the stop-the-bleed could be refined.
> > > > > >
> > > > > > +               /* Note: we might in the future use prio bits
> > > > > > +                * and set skb->priority like in vlan_do_receive()
> > > > > > +                * For the time being, just ignore Priority Code Point
> > > > > > +                */
> > > > > > +               skb->vlan_tci = 0;
> > > > > >
> > > > > > If you believe this can be done, this is great.
> > > > >
> > > > > Do you have a reproducer for that bug? I am willing to spend some time
> > > > > understand what is going on. This has nothing to do with priority. You
> > > > > vaguely described a problem with 802.1p (VLAN 0) and used that as an
> > > > > excuse to clear the entire vlan hwaccel tag regardless of VLAN ID. I'm
> > > > > curious because we also now have commit 36b2f61a42c2 ("net: handle
> > > > > 802.1P vlan 0 packets properly") in that general area, and I simply want
> > > > > to know if your patch still serves a valid purpose or not.
> > > > >
> > > >
> > > > I do not have a repro, the patch seemed to help at that time,
> > > > according to the reporter.
> > >
> > > Do you mind if I respectfully revert then? It's clear that the patch has
> > > loopholes already (it clears the vlan if it's hwaccel, but leaves it
> > > alone if it isn't) and that the proper solution should be different
> > > anyway.
> >
> > Clearly the situation before the patch was not good, it seems well
> > explained in the changelog.
> >
> > If you want to revert, you will need to convince the bug has been
> > solved in another way.
> >
> > So it seems you might have to repro the initial problem.
>
> What bug? What repro? You just said you don't have any.

Ask Steinar ?


>
> Maybe I'm dumb, but the changelog is vague to me. It isn't clear what
> kind of routing it is, what type of traffic was the router being
> subjected to, from what direction was the VLAN traffic coming, was it
> just VLAN 0 that was problematic, what drivers those were, what kernel
> was used, what has any of that have to do with the referenced commit
> 48cc32d38a52 ("vlan: don't deliver frames for unknown vlans to
> protocols") which is about macvlan returning RX_HANDLER_PASS instead of
> RX_HANDLER_ANOTHER, were there other sub-interfaces as well?

The kernel was the very kernel right before the change. git will tell
you that easily.

> And there are also obvious mistakes in the commit description: "if the
> vlan id is set and we could find a vlan device for this particular id."
> -> "couldn't" should be instead of "could".
>

Complaining about a change log seven years after the commit is rather useless.

> This is ridiculous.

Ok

Re: PROBLEM: (DSA/Microchip): 802.1Q-Header lost on KSZ9477-DSA ingress without bridge

[Vladimir Oltean]

On Tue, Aug 04, 2020 at 03:29:05PM -0700, Eric Dumazet wrote:
> On Tue, Aug 4, 2020 at 2:24 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> >
> > On Tue, Aug 04, 2020 at 01:36:56PM -0700, Eric Dumazet wrote:
> > > On Tue, Aug 4, 2020 at 12:43 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> > > >
> > > > On Tue, Aug 04, 2020 at 12:40:24PM -0700, Eric Dumazet wrote:
> > > > > On Tue, Aug 4, 2020 at 12:29 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> > > > > >
> > > > > > On Tue, Aug 04, 2020 at 07:54:18AM -0700, Eric Dumazet wrote:
> > > > > > >
> > > > > > > My 2013 commit was a bug fix, and hinted that in the future (eg in
> > > > > > > net-next tree) the stop-the-bleed could be refined.
> > > > > > >
> > > > > > > +               /* Note: we might in the future use prio bits
> > > > > > > +                * and set skb->priority like in vlan_do_receive()
> > > > > > > +                * For the time being, just ignore Priority Code Point
> > > > > > > +                */
> > > > > > > +               skb->vlan_tci = 0;
> > > > > > >
> > > > > > > If you believe this can be done, this is great.
> > > > > >
> > > > > > Do you have a reproducer for that bug? I am willing to spend some time
> > > > > > understand what is going on. This has nothing to do with priority. You
> > > > > > vaguely described a problem with 802.1p (VLAN 0) and used that as an
> > > > > > excuse to clear the entire vlan hwaccel tag regardless of VLAN ID. I'm
> > > > > > curious because we also now have commit 36b2f61a42c2 ("net: handle
> > > > > > 802.1P vlan 0 packets properly") in that general area, and I simply want
> > > > > > to know if your patch still serves a valid purpose or not.
> > > > > >
> > > > >
> > > > > I do not have a repro, the patch seemed to help at that time,
> > > > > according to the reporter.
> > > >
> > > > Do you mind if I respectfully revert then? It's clear that the patch has
> > > > loopholes already (it clears the vlan if it's hwaccel, but leaves it
> > > > alone if it isn't) and that the proper solution should be different
> > > > anyway.
> > >
> > > Clearly the situation before the patch was not good, it seems well
> > > explained in the changelog.
> > >
> > > If you want to revert, you will need to convince the bug has been
> > > solved in another way.
> > >
> > > So it seems you might have to repro the initial problem.
> >
> > What bug? What repro? You just said you don't have any.
> 
> Ask Steinar ?
> 

Hi Steinar, do you have a reproducer for the bug that Eric fixed in
commit d4b812dea4a2 ("vlan: mask vlan prio bits")?

Thanks,
-Vladimir

Re: PROBLEM: (DSA/Microchip): 802.1Q-Header lost on KSZ9477-DSA ingress without bridge

[Vladimir Oltean]

On Wed, Aug 05, 2020 at 01:39:52AM +0300, Vladimir Oltean wrote:
> On Tue, Aug 04, 2020 at 03:29:05PM -0700, Eric Dumazet wrote:
> > On Tue, Aug 4, 2020 at 2:24 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> > >
> > > On Tue, Aug 04, 2020 at 01:36:56PM -0700, Eric Dumazet wrote:
> > > > On Tue, Aug 4, 2020 at 12:43 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> > > > >
> > > > > On Tue, Aug 04, 2020 at 12:40:24PM -0700, Eric Dumazet wrote:
> > > > > > On Tue, Aug 4, 2020 at 12:29 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> > > > > > >
> > > > > > > On Tue, Aug 04, 2020 at 07:54:18AM -0700, Eric Dumazet wrote:
> > > > > > > >
> > > > > > > > My 2013 commit was a bug fix, and hinted that in the future (eg in
> > > > > > > > net-next tree) the stop-the-bleed could be refined.
> > > > > > > >
> > > > > > > > +               /* Note: we might in the future use prio bits
> > > > > > > > +                * and set skb->priority like in vlan_do_receive()
> > > > > > > > +                * For the time being, just ignore Priority Code Point
> > > > > > > > +                */
> > > > > > > > +               skb->vlan_tci = 0;
> > > > > > > >
> > > > > > > > If you believe this can be done, this is great.
> > > > > > >
> > > > > > > Do you have a reproducer for that bug? I am willing to spend some time
> > > > > > > understand what is going on. This has nothing to do with priority. You
> > > > > > > vaguely described a problem with 802.1p (VLAN 0) and used that as an
> > > > > > > excuse to clear the entire vlan hwaccel tag regardless of VLAN ID. I'm
> > > > > > > curious because we also now have commit 36b2f61a42c2 ("net: handle
> > > > > > > 802.1P vlan 0 packets properly") in that general area, and I simply want
> > > > > > > to know if your patch still serves a valid purpose or not.
> > > > > > >
> > > > > >
> > > > > > I do not have a repro, the patch seemed to help at that time,
> > > > > > according to the reporter.
> > > > >
> > > > > Do you mind if I respectfully revert then? It's clear that the patch has
> > > > > loopholes already (it clears the vlan if it's hwaccel, but leaves it
> > > > > alone if it isn't) and that the proper solution should be different
> > > > > anyway.
> > > >
> > > > Clearly the situation before the patch was not good, it seems well
> > > > explained in the changelog.
> > > >
> > > > If you want to revert, you will need to convince the bug has been
> > > > solved in another way.
> > > >
> > > > So it seems you might have to repro the initial problem.
> > >
> > > What bug? What repro? You just said you don't have any.
> > 
> > Ask Steinar ?
> > 
> 
> Hi Steinar, do you have a reproducer for the bug that Eric fixed in
> commit d4b812dea4a2 ("vlan: mask vlan prio bits")?
> 
> Thanks,
> -Vladimir

The Google email address from the original report bounces back. Adding
another address found by searching for your name on netdev.

Re: PROBLEM: (DSA/Microchip): 802.1Q-Header lost on KSZ9477-DSA ingress without bridge

[Steinar H. Gunderson]

On Wed, Aug 05, 2020 at 01:44:30AM +0300, Vladimir Oltean wrote:
>>>> What bug? What repro? You just said you don't have any.
>>> Ask Steinar ?
>>> 
>> Hi Steinar, do you have a reproducer for the bug that Eric fixed in
>> commit d4b812dea4a2 ("vlan: mask vlan prio bits")?
> The Google email address from the original report bounces back. Adding
> another address found by searching for your name on netdev.

Yeah, I don't work at Google anymore, so sesse@google.com does not exist.
(Hi, Eric! Hoping you're fine despite the pandemic.)

By accident, I'm actually sitting right next to the router in question
right now. But the setup has changed at least twice since 2013, and it
doesn't use sit anymore since native IPv6 is where it's at. So no, I don't
have a reproducer anymore. I also really cannot remember the details;
I think maybe the outgoing sit device was for 6rd? And the priority tag was
added by a fairly cheap Zyxel switch that might still be in the loop, but now
there's tagged VLANs anyway...

If you want to spend time to try to reproduce this with the old kernel
(to verify you have a reproducer that you can use to test the bug with),
this is probably what I'd test: Send untagged packets with 802.1p priority
set (most cheap managed switches allow you to force that somehow, I believe;
tcpdump -e will show an 802.1q tag with VLAN 0), try to route them into a sit
tunnel, and see if they become corrupted or not. That's the only thing I can
recommend, sorry. I hoard a lot of things, but reproducers for fixed bugs
from 2013 at my parents' house isn't among them :-)

/* Steinar */
-- 
Homepage: https://www.sesse.net/

AW: PROBLEM: (DSA/Microchip): 802.1Q-Header lost on KSZ9477-DSA ingress without bridge

[Gaube, Marvin (THSE-TL1)]

Hello,
I'm using the upstream driver Vladimir mentioned, as of 5.4.51.

Best regards
Marvin Gaube

-----Ursprüngliche Nachricht-----
Von: Florian Fainelli <f.fainelli@gmail.com>
Gesendet: Dienstag, 4. August 2020 22:20
An: Vladimir Oltean <olteanv@gmail.com>
Cc: Gaube, Marvin (THSE-TL1) <Marvin.Gaube@tesat.de>; Woojung Huh <woojung.huh@microchip.com>; Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>; netdev@vger.kernel.org
Betreff: Re: PROBLEM: (DSA/Microchip): 802.1Q-Header lost on KSZ9477-DSA ingress without bridge



On 8/4/2020 12:54 PM, Vladimir Oltean wrote:
> On Tue, Aug 04, 2020 at 08:51:00AM -0700, Florian Fainelli wrote:
>> "I looked into it deeper, the driver does rxvlan offloading."
>>
>> Is this part of the driver upstream or are you using a vendor tree
>> from Freescale which has that change included?
>>
>
> Does it matter?

If this was a downstream problem yes, there would not necessarily be a pressing need to address it with a patch targeting 'net' for instance.
This is not the case, therefore it should be addressed.

> FWIW, mainline fec does declare NETIF_F_HW_VLAN_CTAG_RX:
> https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/fr
> eescale/fec_main.c#L3317 and move it to the hwaccel area on RX:
> https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/fr
> eescale/fec_main.c#L1513
>

--
Florian

________________________________

Tesat-Spacecom GmbH & Co. KG
Sitz: Backnang; Registergericht: Amtsgericht Stuttgart HRA 270977
Persoenlich haftender Gesellschafter: Tesat-Spacecom Geschaeftsfuehrungs GmbH;
Sitz: Backnang; Registergericht: Amtsgericht Stuttgart HRB 271658;
Geschaeftsfuehrung: Dr. Marc Steckling, Kerstin Basche, Ralf Zimmermann

[banner]

Re: PROBLEM: (DSA/Microchip): 802.1Q-Header lost on KSZ9477-DSA ingress without bridge

[Vladimir Oltean]

Hi Marvin,

On Wed, Aug 05, 2020 at 05:45:51AM +0000, Gaube, Marvin (THSE-TL1) wrote:
> Hello,
> I'm using the upstream driver Vladimir mentioned, as of 5.4.51.
>
> Best regards
> Marvin Gaube

Could you try net-next again, now that David has merged this patch?

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

Thanks,
-Vladimir