[net,v3,4/5] net: ethernet: mtk_eth_soc: drop generic vlan rx offload, only use DSA untagging

Message ID 20221230073145.53386-4-nbd@nbd.name
State New
Headers
Series [net,v3,1/5] net: ethernet: mtk_eth_soc: account for vlan in rx header length |

Commit Message

Felix Fietkau Dec. 30, 2022, 7:31 a.m. UTC
  Through testing I found out that hardware vlan rx offload support seems to
have some hardware issues. At least when using multiple MACs and when receiving
tagged packets on the secondary MAC, the hardware can sometimes start to emit
wrong tags on the first MAC as well.

In order to avoid such issues, drop the feature configuration and use the
offload feature only for DSA hardware untagging on MT7621/MT7622 devices which
only use one MAC.

Tested-By: Frank Wunderlich <frank-w@public-files.de>
Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
v3: rebase

 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 101 ++++++++------------
 drivers/net/ethernet/mediatek/mtk_eth_soc.h |   1 -
 2 files changed, 39 insertions(+), 63 deletions(-)
  

Comments

Frank Wunderlich Dec. 30, 2022, 11:46 a.m. UTC | #1
Hi,

v2 or v3 seems to break vlan on mt7986 over eth0 (mt7531 switch). v1 was working on next from end of November. But my rebased tree with v1 on 6.2-rc1 has same issue, so something after next 2711 was added which break vlan over mt7531.

Directly over eth1 it works (was not working before).

if i made no mistake there is still something wrong.

btw. mt7622/r64 can also use second gmac (over vlan aware bridge with aux-port of switch to wan-port) it is only not default in mainline. But maybe this should not be used as decision for dropping "dsa-tag" (wrongly vlan-tag).

regards Frank
  
Felix Fietkau Dec. 30, 2022, 12:56 p.m. UTC | #2
On 30.12.22 12:46, Frank Wunderlich wrote:
> Hi,
> 
> v2 or v3 seems to break vlan on mt7986 over eth0 (mt7531 switch). v1 was working on next from end of November. But my rebased tree with v1 on 6.2-rc1 has same issue, so something after next 2711 was added which break vlan over mt7531.
> 
> Directly over eth1 it works (was not working before).
> 
> if i made no mistake there is still something wrong.
> 
> btw. mt7622/r64 can also use second gmac (over vlan aware bridge with aux-port of switch to wan-port) it is only not default in mainline. But maybe this should not be used as decision for dropping "dsa-tag" (wrongly vlan-tag).
> 
> regards Frank
Thanks for reporting.
Please try this patch on top of the series:
---
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -3218,10 +3218,8 @@ static int mtk_open(struct net_device *dev)
  	phylink_start(mac->phylink);
  	netif_tx_start_all_queues(dev);
  
-	if (MTK_HAS_CAPS(eth->soc->caps, MTK_NETSYS_V2))
-		return 0;
-
-	if (mtk_uses_dsa(dev) && !eth->prog) {
+	if (!MTK_HAS_CAPS(eth->soc->caps, MTK_NETSYS_V2) &&
+	    mtk_uses_dsa(dev) && !eth->prog) {
  		for (i = 0; i < ARRAY_SIZE(eth->dsa_meta); i++) {
  			struct metadata_dst *md_dst = eth->dsa_meta[i];
  
@@ -3244,10 +3242,6 @@ static int mtk_open(struct net_device *dev)
  		val &= ~MTK_CDMP_STAG_EN;
  		mtk_w32(eth, val, MTK_CDMP_IG_CTRL);
  
-		val = mtk_r32(eth, MTK_CDMQ_IG_CTRL);
-		val &= ~MTK_CDMQ_STAG_EN;
-		mtk_w32(eth, val, MTK_CDMQ_IG_CTRL);
-
  		mtk_w32(eth, 0, MTK_CDMP_EG_CTRL);
  	}
  
Frank Wunderlich Dec. 30, 2022, 1:56 p.m. UTC | #3
Hi

thanks for fast response

> Gesendet: Freitag, 30. Dezember 2022 um 13:56 Uhr
> Von: "Felix Fietkau" <nbd@nbd.name>
> An: "Frank Wunderlich" <frank-w@public-files.de>
> Cc: netdev@vger.kernel.org, "John Crispin" <john@phrozen.org>, "Sean Wang" <sean.wang@mediatek.com>, "Mark Lee" <Mark-MC.Lee@mediatek.com>, "Lorenzo Bianconi" <lorenzo@kernel.org>, "David S. Miller" <davem@davemloft.net>, "Eric Dumazet" <edumazet@google.com>, "Jakub Kicinski" <kuba@kernel.org>, "Paolo Abeni" <pabeni@redhat.com>, "Matthias Brugger" <matthias.bgg@gmail.com>, "Russell King" <linux@armlinux.org.uk>, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org
> Betreff: Re: Aw: [PATCH net v3 4/5] net: ethernet: mtk_eth_soc: drop generic vlan rx offload, only use DSA untagging
>
> On 30.12.22 12:46, Frank Wunderlich wrote:
> > Hi,
> >
> > v2 or v3 seems to break vlan on mt7986 over eth0 (mt7531 switch). v1 was working on next from end of November. But my rebased tree with v1 on 6.2-rc1 has same issue, so something after next 2711 was added which break vlan over mt7531.
> >
> > Directly over eth1 it works (was not working before).
> >
> > if i made no mistake there is still something wrong.
> >
> > btw. mt7622/r64 can also use second gmac (over vlan aware bridge with aux-port of switch to wan-port) it is only not default in mainline. But maybe this should not be used as decision for dropping "dsa-tag" (wrongly vlan-tag).
> >
> > regards Frank
> Thanks for reporting.
> Please try this patch on top of the series:
> ---
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -3218,10 +3218,8 @@ static int mtk_open(struct net_device *dev)
>   	phylink_start(mac->phylink);
>   	netif_tx_start_all_queues(dev);
>
> -	if (MTK_HAS_CAPS(eth->soc->caps, MTK_NETSYS_V2))
> -		return 0;
> -
> -	if (mtk_uses_dsa(dev) && !eth->prog) {
> +	if (!MTK_HAS_CAPS(eth->soc->caps, MTK_NETSYS_V2) &&
> +	    mtk_uses_dsa(dev) && !eth->prog) {
>   		for (i = 0; i < ARRAY_SIZE(eth->dsa_meta); i++) {
>   			struct metadata_dst *md_dst = eth->dsa_meta[i];
>
> @@ -3244,10 +3242,6 @@ static int mtk_open(struct net_device *dev)
>   		val &= ~MTK_CDMP_STAG_EN;
>   		mtk_w32(eth, val, MTK_CDMP_IG_CTRL);
>
> -		val = mtk_r32(eth, MTK_CDMQ_IG_CTRL);
> -		val &= ~MTK_CDMQ_STAG_EN;
> -		mtk_w32(eth, val, MTK_CDMQ_IG_CTRL);
> -
>   		mtk_w32(eth, 0, MTK_CDMP_EG_CTRL);
>   	}
>
>
>

seems not helping...this is how i test it:

ip link set eth1 up
ip link add link eth1 name vlan500 type vlan id 500
ip link add link wan name vlan600 type vlan id 600
ip addr add 192.168.50.1/24 dev vlan500
ip addr add 192.168.60.1/24 dev vlan600
ip link set vlan500 up
ip link set wan up
ip link set vlan600 up

#do this on the other side:
#netif=enp3s0
#sudo ip link add link $netif name vlan500 type vlan id 500
#sudo ip link add link $netif name vlan600 type vlan id 600
#sudo ip link set vlan500 up
#sudo ip link set vlan600 up
#sudo ip addr add 192.168.50.2/24 dev vlan500
#sudo ip addr add 192.168.60.2/24 dev vlan600

verified all used ports on my switch are in trunk-mode with vlan-membership of these 2 vlan.



booted 6.1 and there is vlan on dsa-port broken too, so either my test-setup is broken or code...but wonder why 6.1 is broken too...

with tcp-dump on my laptop i see that some packets came in for both vlan, but they seem not valid, arp only for vlan 500 (eth1 on r3).

$ sudo tcpdump -i enp3s0 -nn -e  vlan
tcpdump: verbose output suppressed, use -v[v]... for full protocol decode
listening on enp3s0, link-type EN10MB (Ethernet), snapshot length 262144 bytes
14:49:09.548909 e4:b9:7a:f7:c4:8b > 33:33:00:00:83:84, ethertype 802.1Q (0x8100), length 524: vlan 500, p 0, ethertype IPv6 (0x86dd), fe80::e6b9:7aff:fef7:c48b.34177 > ff12::8384.21027: UDP, length 458
14:49:09.548929 e4:b9:7a:f7:c4:8b > 33:33:00:00:83:84, ethertype 802.1Q (0x8100), length 524: vlan 600, p 0, ethertype IPv6 (0x86dd), fe80::e6b9:7aff:fef7:c48b.34177 > ff12::8384.21027: UDP, length 458
14:49:09.549470 e4:b9:7a:f7:c4:8b > ff:ff:ff:ff:ff:ff, ethertype 802.1Q (0x8100), length 504: vlan 500, p 0, ethertype IPv4 (0x0800), 192.168.50.2.33050 > 192.168.50.255.21027: UDP, length 458
14:49:09.549522 e4:b9:7a:f7:c4:8b > ff:ff:ff:ff:ff:ff, ethertype 802.1Q (0x8100), length 504: vlan 600, p 0, ethertype IPv4 (0x0800), 192.168.60.2.33050 > 192.168.60.255.21027: UDP, length 458
14:49:26.324503 92:65:f3:ec:b0:19 > ff:ff:ff:ff:ff:ff, ethertype 802.1Q (0x8100), length 60: vlan 500, p 0, ethertype ARP (0x0806), Request who-has 192.168.50.2 tell 192.168.50.1, length 42
14:49:26.324525 e4:b9:7a:f7:c4:8b > 92:65:f3:ec:b0:19, ethertype 802.1Q (0x8100), length 46: vlan 500, p 0, ethertype ARP (0x0806), Reply 192.168.50.2 is-at e4:b9:7a:f7:c4:8b, length 28
14:49:26.325091 92:65:f3:ec:b0:19 > e4:b9:7a:f7:c4:8b, ethertype 802.1Q (0x8100), length 102: vlan 500, p 0, ethertype IPv4 (0x0800), 192.168.50.1 > 192.168.50.2: ICMP echo request, id 44246, seq 1, length 64
14:49:26.325158 e4:b9:7a:f7:c4:8b > 92:65:f3:ec:b0:19, ethertype 802.1Q (0x8100), length 102: vlan 500, p 0, ethertype IPv4 (0x0800), 192.168.50.2 > 192.168.50.1: ICMP echo reply, id 44246, seq 1, length 64

on r3 i see these packets going out (so far it looks good):

root@bpi-r3:~# ping 192.168.60.2
PING 192.168.60.2 (192.168.60.2) 56(84) bytes of data.
13:30:29.782003 08:22:33:44:55:77 > ff:ff:ff:ff:ff:ff, ethertype 802.1Q (0x8100), length 46: vlan 600, p 0, ethertype ARP (0x0806),
 Request who-has 192.168.60.2 tell 192.168.60.1, length 28
13:30:30.788175 08:22:33:44:55:77 > ff:ff:ff:ff:ff:ff, ethertype 802.1Q (0x8100), length 46: vlan 600, p 0, ethertype ARP (0x0806),
 Request who-has 192.168.60.2 tell 192.168.60.1, length 28
13:30:31.828181 08:22:33:44:55:77 > ff:ff:ff:ff:ff:ff, ethertype 802.1Q (0x8100), length 46: vlan 600, p 0, ethertype ARP (0x0806),
 Request who-has 192.168.60.2 tell 192.168.60.1, length 28
From 192.168.60.1 icmp_seq=1 Destination Host Unreachable
13:30:32.868205 08:22:33:44:55:77 > ff:ff:ff:ff:ff:ff, ethertype 802.1Q (0x8100), length 46: vlan 600, p 0, ethertype ARP (0x0806),
 Request who-has 192.168.60.2 tell 192.168.60.1, length 28
13:30:33.908171 08:22:33:44:55:77 > ff:ff:ff:ff:ff:ff, ethertype 802.1Q (0x8100), length 46: vlan 600, p 0, ethertype ARP (0x0806),
 Request who-has 192.168.60.2 tell 192.168.60.1, length 28

HTH, maybe daniel or anyone other can confirm this

regards Frank
  
Felix Fietkau Dec. 30, 2022, 2:58 p.m. UTC | #4
On 30.12.22 14:56, Frank Wunderlich wrote:
> Hi
> 
> thanks for fast response
> 
>> Gesendet: Freitag, 30. Dezember 2022 um 13:56 Uhr
>> Von: "Felix Fietkau" <nbd@nbd.name>
>> An: "Frank Wunderlich" <frank-w@public-files.de>
>> Cc: netdev@vger.kernel.org, "John Crispin" <john@phrozen.org>, "Sean Wang" <sean.wang@mediatek.com>, "Mark Lee" <Mark-MC.Lee@mediatek.com>, "Lorenzo Bianconi" <lorenzo@kernel.org>, "David S. Miller" <davem@davemloft.net>, "Eric Dumazet" <edumazet@google.com>, "Jakub Kicinski" <kuba@kernel.org>, "Paolo Abeni" <pabeni@redhat.com>, "Matthias Brugger" <matthias.bgg@gmail.com>, "Russell King" <linux@armlinux.org.uk>, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org
>> Betreff: Re: Aw: [PATCH net v3 4/5] net: ethernet: mtk_eth_soc: drop generic vlan rx offload, only use DSA untagging
>>
>> On 30.12.22 12:46, Frank Wunderlich wrote:
>> > Hi,
>> >
>> > v2 or v3 seems to break vlan on mt7986 over eth0 (mt7531 switch). v1 was working on next from end of November. But my rebased tree with v1 on 6.2-rc1 has same issue, so something after next 2711 was added which break vlan over mt7531.
>> >
>> > Directly over eth1 it works (was not working before).
>> >
>> > if i made no mistake there is still something wrong.
>> >
>> > btw. mt7622/r64 can also use second gmac (over vlan aware bridge with aux-port of switch to wan-port) it is only not default in mainline. But maybe this should not be used as decision for dropping "dsa-tag" (wrongly vlan-tag).
>> >
>> > regards Frank
>> Thanks for reporting.
>> Please try this patch on top of the series:
>> ---
>> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>> @@ -3218,10 +3218,8 @@ static int mtk_open(struct net_device *dev)
>>   	phylink_start(mac->phylink);
>>   	netif_tx_start_all_queues(dev);
>>
>> -	if (MTK_HAS_CAPS(eth->soc->caps, MTK_NETSYS_V2))
>> -		return 0;
>> -
>> -	if (mtk_uses_dsa(dev) && !eth->prog) {
>> +	if (!MTK_HAS_CAPS(eth->soc->caps, MTK_NETSYS_V2) &&
>> +	    mtk_uses_dsa(dev) && !eth->prog) {
>>   		for (i = 0; i < ARRAY_SIZE(eth->dsa_meta); i++) {
>>   			struct metadata_dst *md_dst = eth->dsa_meta[i];
>>
>> @@ -3244,10 +3242,6 @@ static int mtk_open(struct net_device *dev)
>>   		val &= ~MTK_CDMP_STAG_EN;
>>   		mtk_w32(eth, val, MTK_CDMP_IG_CTRL);
>>
>> -		val = mtk_r32(eth, MTK_CDMQ_IG_CTRL);
>> -		val &= ~MTK_CDMQ_STAG_EN;
>> -		mtk_w32(eth, val, MTK_CDMQ_IG_CTRL);
>> -
>>   		mtk_w32(eth, 0, MTK_CDMP_EG_CTRL);
>>   	}
>>
>>
>>
> 
> seems not helping...this is how i test it:
> 
> ip link set eth1 up
> ip link add link eth1 name vlan500 type vlan id 500
> ip link add link wan name vlan600 type vlan id 600
> ip addr add 192.168.50.1/24 dev vlan500
> ip addr add 192.168.60.1/24 dev vlan600
> ip link set vlan500 up
> ip link set wan up
> ip link set vlan600 up
> 
> #do this on the other side:
> #netif=enp3s0
> #sudo ip link add link $netif name vlan500 type vlan id 500
> #sudo ip link add link $netif name vlan600 type vlan id 600
> #sudo ip link set vlan500 up
> #sudo ip link set vlan600 up
> #sudo ip addr add 192.168.50.2/24 dev vlan500
> #sudo ip addr add 192.168.60.2/24 dev vlan600
> 
> verified all used ports on my switch are in trunk-mode with vlan-membership of these 2 vlan.
> 
> 
> 
> booted 6.1 and there is vlan on dsa-port broken too, so either my test-setup is broken or code...but wonder why 6.1 is broken too...
> 
> with tcp-dump on my laptop i see that some packets came in for both vlan, but they seem not valid, arp only for vlan 500 (eth1 on r3).
> 
> $ sudo tcpdump -i enp3s0 -nn -e  vlan
> tcpdump: verbose output suppressed, use -v[v]... for full protocol decode
> listening on enp3s0, link-type EN10MB (Ethernet), snapshot length 262144 bytes
> 14:49:09.548909 e4:b9:7a:f7:c4:8b > 33:33:00:00:83:84, ethertype 802.1Q (0x8100), length 524: vlan 500, p 0, ethertype IPv6 (0x86dd), fe80::e6b9:7aff:fef7:c48b.34177 > ff12::8384.21027: UDP, length 458
> 14:49:09.548929 e4:b9:7a:f7:c4:8b > 33:33:00:00:83:84, ethertype 802.1Q (0x8100), length 524: vlan 600, p 0, ethertype IPv6 (0x86dd), fe80::e6b9:7aff:fef7:c48b.34177 > ff12::8384.21027: UDP, length 458
> 14:49:09.549470 e4:b9:7a:f7:c4:8b > ff:ff:ff:ff:ff:ff, ethertype 802.1Q (0x8100), length 504: vlan 500, p 0, ethertype IPv4 (0x0800), 192.168.50.2.33050 > 192.168.50.255.21027: UDP, length 458
> 14:49:09.549522 e4:b9:7a:f7:c4:8b > ff:ff:ff:ff:ff:ff, ethertype 802.1Q (0x8100), length 504: vlan 600, p 0, ethertype IPv4 (0x0800), 192.168.60.2.33050 > 192.168.60.255.21027: UDP, length 458
> 14:49:26.324503 92:65:f3:ec:b0:19 > ff:ff:ff:ff:ff:ff, ethertype 802.1Q (0x8100), length 60: vlan 500, p 0, ethertype ARP (0x0806), Request who-has 192.168.50.2 tell 192.168.50.1, length 42
> 14:49:26.324525 e4:b9:7a:f7:c4:8b > 92:65:f3:ec:b0:19, ethertype 802.1Q (0x8100), length 46: vlan 500, p 0, ethertype ARP (0x0806), Reply 192.168.50.2 is-at e4:b9:7a:f7:c4:8b, length 28
> 14:49:26.325091 92:65:f3:ec:b0:19 > e4:b9:7a:f7:c4:8b, ethertype 802.1Q (0x8100), length 102: vlan 500, p 0, ethertype IPv4 (0x0800), 192.168.50.1 > 192.168.50.2: ICMP echo request, id 44246, seq 1, length 64
> 14:49:26.325158 e4:b9:7a:f7:c4:8b > 92:65:f3:ec:b0:19, ethertype 802.1Q (0x8100), length 102: vlan 500, p 0, ethertype IPv4 (0x0800), 192.168.50.2 > 192.168.50.1: ICMP echo reply, id 44246, seq 1, length 64
> 
> on r3 i see these packets going out (so far it looks good):
> 
> root@bpi-r3:~# ping 192.168.60.2
> PING 192.168.60.2 (192.168.60.2) 56(84) bytes of data.
> 13:30:29.782003 08:22:33:44:55:77 > ff:ff:ff:ff:ff:ff, ethertype 802.1Q (0x8100), length 46: vlan 600, p 0, ethertype ARP (0x0806),
>   Request who-has 192.168.60.2 tell 192.168.60.1, length 28
> 13:30:30.788175 08:22:33:44:55:77 > ff:ff:ff:ff:ff:ff, ethertype 802.1Q (0x8100), length 46: vlan 600, p 0, ethertype ARP (0x0806),
>   Request who-has 192.168.60.2 tell 192.168.60.1, length 28
> 13:30:31.828181 08:22:33:44:55:77 > ff:ff:ff:ff:ff:ff, ethertype 802.1Q (0x8100), length 46: vlan 600, p 0, ethertype ARP (0x0806),
>   Request who-has 192.168.60.2 tell 192.168.60.1, length 28
>  From 192.168.60.1 icmp_seq=1 Destination Host Unreachable
> 13:30:32.868205 08:22:33:44:55:77 > ff:ff:ff:ff:ff:ff, ethertype 802.1Q (0x8100), length 46: vlan 600, p 0, ethertype ARP (0x0806),
>   Request who-has 192.168.60.2 tell 192.168.60.1, length 28
> 13:30:33.908171 08:22:33:44:55:77 > ff:ff:ff:ff:ff:ff, ethertype 802.1Q (0x8100), length 46: vlan 600, p 0, ethertype ARP (0x0806),
>   Request who-has 192.168.60.2 tell 192.168.60.1, length 28
> 
> HTH, maybe daniel or anyone other can confirm this
Does this help?
---
--- a/net/dsa/tag_mtk.c
+++ b/net/dsa/tag_mtk.c
@@ -25,6 +25,14 @@ static struct sk_buff *mtk_tag_xmit(stru
  	u8 xmit_tpid;
  	u8 *mtk_tag;
  
+	/* The Ethernet switch we are interfaced with needs packets to be at
+	 * least 64 bytes (including FCS) otherwise their padding might be
+	 * corrupted. With tags enabled, we need to make sure that packets are
+	 * at least 68 bytes (including FCS and tag).
+	 */
+	if (__skb_put_padto(skb, ETH_ZLEN + MTK_HDR_LEN, false))
+		return NULL;
+
  	/* Build the special tag after the MAC Source Address. If VLAN header
  	 * is present, it's required that VLAN header and special tag is
  	 * being combined. Only in this way we can allow the switch can parse
  
Frank Wunderlich Dec. 30, 2022, 3:33 p.m. UTC | #5
Hi

> Gesendet: Freitag, 30. Dezember 2022 um 15:58 Uhr
> Von: "Felix Fietkau" <nbd@nbd.name>

> Does this help?
> ---
> --- a/net/dsa/tag_mtk.c
> +++ b/net/dsa/tag_mtk.c
> @@ -25,6 +25,14 @@ static struct sk_buff *mtk_tag_xmit(stru
>   	u8 xmit_tpid;
>   	u8 *mtk_tag;
>
> +	/* The Ethernet switch we are interfaced with needs packets to be at
> +	 * least 64 bytes (including FCS) otherwise their padding might be
> +	 * corrupted. With tags enabled, we need to make sure that packets are
> +	 * at least 68 bytes (including FCS and tag).
> +	 */
> +	if (__skb_put_padto(skb, ETH_ZLEN + MTK_HDR_LEN, false))
> +		return NULL;
> +
>   	/* Build the special tag after the MAC Source Address. If VLAN header
>   	 * is present, it's required that VLAN header and special tag is
>   	 * being combined. Only in this way we can allow the switch can parse

no, i verified my vlan-setup ist right by adding additional device (my r2) into the same 2 vlans. My Laptop can ping both vlans of R2, but on r3/mt7986 only vlan500 (on eth1) works, not 600 on wan-port of mt7986.

see arp going out on r3, but not received in the other side (laptop/r2)....

any debug i can add for this? ethtool does not show stats for vlans, for wan i see no CRC/drops

regards Frank
  
Frank Wunderlich Dec. 30, 2022, 3:38 p.m. UTC | #6
seems only tx is affected on r3, as i see packets on the vlan from my laptop

tcpdump on R3 (e4:b9:7a:f7:c4:8b is mac from laptop):

13:47:05.265508 e4:b9:7a:f7:c4:8b > ff:ff:ff:ff:ff:ff, ethertype 802.1Q (0x8100), length 577: vlan 500, p 0, ethertype IPv4 (0x0800), 192.168.50.2.59389 > 192.168.50.255.21027: UDP, length 531
13:47:05.265548 e4:b9:7a:f7:c4:8b > ff:ff:ff:ff:ff:ff, ethertype 802.1Q (0x8100), length 577: vlan 600, p 0, ethertype IPv4 (0x0800), 192.168.60.2.59389 > 192.168.60.255.21027: UDP, length 531

regards Frank
  
Felix Fietkau Dec. 30, 2022, 4:13 p.m. UTC | #7
On 30.12.22 16:38, Frank Wunderlich wrote:
> seems only tx is affected on r3, as i see packets on the vlan from my laptop
> 
> tcpdump on R3 (e4:b9:7a:f7:c4:8b is mac from laptop):
> 
> 13:47:05.265508 e4:b9:7a:f7:c4:8b > ff:ff:ff:ff:ff:ff, ethertype 802.1Q (0x8100), length 577: vlan 500, p 0, ethertype IPv4 (0x0800), 192.168.50.2.59389 > 192.168.50.255.21027: UDP, length 531
> 13:47:05.265548 e4:b9:7a:f7:c4:8b > ff:ff:ff:ff:ff:ff, ethertype 802.1Q (0x8100), length 577: vlan 600, p 0, ethertype IPv4 (0x0800), 192.168.60.2.59389 > 192.168.60.255.21027: UDP, length 531
> 
> regards Frank
I don't have a setup to test 6.2 on my MT7986 board right now, but I did 
test latest OpenWrt with my changes and couldn't reproduce the issue there.
I checked the diff between my tree and upstream and didn't find any 
relevant differences in mtk_eth_soc.c
Not sure what's going on or how to narrow it down further.

- Felix
  
Frank Wunderlich Jan. 6, 2023, 12:18 p.m. UTC | #8
Hi,

update on my tests...something in my systemd-networkd breaks vlan on dsa-port.

if i boot with this disabled vlan added manually works on dsa-port and gmac too. so felix you can keep my tested-by.

if vlan is working and i activate networkd afterwards vlans are still working...i guess systemd puts the wan-interface into some kind of non-vlan-mode (vlan-filtering?).

this is my basic systemd-config:

/etc/systemd/network/15-wan.network
[Match]
Name=wan

[Network]
BindCarrier=eth0

#static setup
Address=192.168.0.19/24
Gateway=192.168.0.10
DNS=192.168.0.10

IPForward=yes

/etc/systemd/network/20-lanbr.netdev
[NetDev]
Name=lanbr0
Kind=bridge

[Bridge]
DefaultPVID=1
VLANFiltering=1
/etc/systemd/network/22-lanbr.network

[Match]
Name=lanbr0

[Network]
BindCarrier=eth0
ConfigureWithoutCarrier=true

Address=192.168.1.1/24
Address=fd00:A::10/64

[DHCPServer]
PoolOffset=100
PoolSize=150

/etc/systemd/network/05-eth0.network
[Match]
Name=eth0

[Network]
DHCP=no
LinkLocalAddressing=no
ConfigureWithoutCarrier=true

/etc/systemd/network/21-lanbr-bind.network
[Match]
Name=lan0 lan1 lan2 lan3

[Network]
Bridge=lanbr0

/etc/systemd/network/10-wan.link
[Match]
OriginalName=wan

[Link]
Name=wan
#MACAddressPolicy=none
#MACAddress=08:22:33:44:55:77


regards Frank


> Gesendet: Freitag, 30. Dezember 2022 um 17:13 Uhr
> Von: "Felix Fietkau" <nbd@nbd.name>
> An: "Frank Wunderlich" <frank-w@public-files.de>
> Cc: netdev@vger.kernel.org, "John Crispin" <john@phrozen.org>, "Sean Wang" <sean.wang@mediatek.com>, "Mark Lee" <Mark-MC.Lee@mediatek.com>, "Lorenzo Bianconi" <lorenzo@kernel.org>, "David S. Miller" <davem@davemloft.net>, "Eric Dumazet" <edumazet@google.com>, "Jakub Kicinski" <kuba@kernel.org>, "Paolo Abeni" <pabeni@redhat.com>, "Matthias Brugger" <matthias.bgg@gmail.com>, "Russell King" <linux@armlinux.org.uk>, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org
> Betreff: Re: Aw: Re: Re: [PATCH net v3 4/5] net: ethernet: mtk_eth_soc: drop generic vlan rx offload, only use DSA untagging
>
> On 30.12.22 16:38, Frank Wunderlich wrote:
> > seems only tx is affected on r3, as i see packets on the vlan from my laptop
> >
> > tcpdump on R3 (e4:b9:7a:f7:c4:8b is mac from laptop):
> >
> > 13:47:05.265508 e4:b9:7a:f7:c4:8b > ff:ff:ff:ff:ff:ff, ethertype 802.1Q (0x8100), length 577: vlan 500, p 0, ethertype IPv4 (0x0800), 192.168.50.2.59389 > 192.168.50.255.21027: UDP, length 531
> > 13:47:05.265548 e4:b9:7a:f7:c4:8b > ff:ff:ff:ff:ff:ff, ethertype 802.1Q (0x8100), length 577: vlan 600, p 0, ethertype IPv4 (0x0800), 192.168.60.2.59389 > 192.168.60.255.21027: UDP, length 531
> >
> > regards Frank
> I don't have a setup to test 6.2 on my MT7986 board right now, but I did
> test latest OpenWrt with my changes and couldn't reproduce the issue there.
> I checked the diff between my tree and upstream and didn't find any
> relevant differences in mtk_eth_soc.c
> Not sure what's going on or how to narrow it down further.
>
> - Felix
>
  
Arınç ÜNAL Jan. 28, 2023, 5:38 p.m. UTC | #9
On 30.12.2022 10:31, Felix Fietkau wrote:
> Through testing I found out that hardware vlan rx offload support seems to
> have some hardware issues. At least when using multiple MACs and when receiving
> tagged packets on the secondary MAC, the hardware can sometimes start to emit
> wrong tags on the first MAC as well.
> 
> In order to avoid such issues, drop the feature configuration and use the
> offload feature only for DSA hardware untagging on MT7621/MT7622 devices which
> only use one MAC.
> 
> Tested-By: Frank Wunderlich <frank-w@public-files.de>
> Signed-off-by: Felix Fietkau <nbd@nbd.name>

You can add this to all patches on the series.

Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>

Tested on Unielec U7621-06 (MT7621AT) and Bananapi BPI-R2 (MT7623NI) on 
latest netnext with buildroot as initramfs. These are tested with my fix 
[0] applied on top.

VLAN on DSA master gmac0.
   Works on MT7621 SoC with multi-chip module MT7530 switch and MT7623
   SoC with standalone MT7530 switch.

VLAN on DSA master gmac0 and non-DSA gmac1.
   Works on MT7621 SoC with multi-chip module MT7530 switch and MT7623
   SoC with standalone MT7530 switch.

VLAN on DSA master gmac1.
   Can’t test on MT7621 as an unrelated issue prevents from testing.
     Define port@6 and gmac0, otherwise gmac1 DSA master receives
     malformed frames from port@5. This issue appears only on MT7621 SoC
     with multi-chip module MT7530 switch.
   Works on MT7623 SoC with standalone MT7530 switch.

VLAN on DSA master gmac0 and DSA master gmac1.
   Works on MT7621 SoC with multi-chip module MT7530 and MT7623 SoC with
   standalone MT7530 switch switch after compensating an unrelated issue.
     When both MACs are DSA masters, ping from gmac1 DSA master first,
     otherwise frames received from user ports won’t reach to gmac1 DSA
     master. This issue appears on MT7621 SoC with multi-chip module
     MT7530 switch and MT7623 SoC with standalone MT7530 switch.

It'd be great if you could take a look at these issues.

Network configuration:

For DSA master gmac0/gmac1
ip l add link lan3 name lan3.50 type vlan id 50
ip a add 192.168.3.1/24 dev lan3.50
ip l set up lan3 && ip l set up lan3.50

For non-DSA gmac1
ip l del lan3.50
ip l add link eth1 name eth1.50 type vlan id 50
ip a add 192.168.3.1/24 dev eth1.50
ip l set up eth1 && ip l set up eth1.50

Other side
ip l add link enp9s0 name enp9s0.50 type vlan id 50
ip a add 192.168.3.2/24 dev enp9s0.50
ip l set up enp9s0 && ip l set up enp9s0.50

[0] 
https://lore.kernel.org/netdev/20230128094232.2451947-1-arinc.unal@arinc9.com/

Arınç
  
Frank Wunderlich Feb. 8, 2023, 7:46 a.m. UTC | #10
Hi,

just to confirm, my issues were not caused by this series, it was there before and now fixed with [1]

at least one issue arinc is mention is fixed with [2]

so please apply

regards Frank

[1] https://patchwork.kernel.org/project/linux-mediatek/patch/20230205140713.1609281-1-vladimir.oltean@nxp.com/
[2] https://patchwork.kernel.org/project/linux-mediatek/patch/20230207103027.1203344-1-vladimir.oltean@nxp.com/
  
Arınç ÜNAL Feb. 8, 2023, 8:16 a.m. UTC | #11
On 8.02.2023 10:46, Frank Wunderlich wrote:
> Hi,
> 
> just to confirm, my issues were not caused by this series, it was there before and now fixed with [1]
> 
> at least one issue arinc is mention is fixed with [2]

Actually, [2] fixes another issue that I didn't mention here. But all 
the issues I mentioned here is unrelated to this patch series, and I 
already have some patches to submit that will fix them, so this series 
is in the clear.

> 
> so please apply

Agreed.

Arınç
  

Patch

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 04b75c7ad5b0..ab6c571a39ae 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -2042,29 +2042,16 @@  static int mtk_poll_rx(struct napi_struct *napi, int budget,
 		if (reason == MTK_PPE_CPU_REASON_HIT_UNBIND_RATE_REACHED)
 			mtk_ppe_check_skb(eth->ppe[0], skb, hash);
 
-		if (netdev->features & NETIF_F_HW_VLAN_CTAG_RX) {
-			if (MTK_HAS_CAPS(eth->soc->caps, MTK_NETSYS_V2)) {
-				if (trxd.rxd3 & RX_DMA_VTAG_V2)
-					__vlan_hwaccel_put_tag(skb,
-						htons(RX_DMA_VPID(trxd.rxd4)),
-						RX_DMA_VID(trxd.rxd4));
-			} else if (trxd.rxd2 & RX_DMA_VTAG) {
-				__vlan_hwaccel_put_tag(skb, htons(RX_DMA_VPID(trxd.rxd3)),
-						       RX_DMA_VID(trxd.rxd3));
-			}
-		}
-
 		/* When using VLAN untagging in combination with DSA, the
 		 * hardware treats the MTK special tag as a VLAN and untags it.
 		 */
-		if (skb_vlan_tag_present(skb) && netdev_uses_dsa(netdev)) {
-			unsigned int port = ntohs(skb->vlan_proto) & GENMASK(2, 0);
+		if (!MTK_HAS_CAPS(eth->soc->caps, MTK_NETSYS_V2) &&
+		    (trxd.rxd2 & RX_DMA_VTAG) && netdev_uses_dsa(netdev)) {
+			unsigned int port = RX_DMA_VPID(trxd.rxd3) & GENMASK(2, 0);
 
 			if (port < ARRAY_SIZE(eth->dsa_meta) &&
 			    eth->dsa_meta[port])
 				skb_dst_set_noref(skb, &eth->dsa_meta[port]->dst);
-
-			__vlan_hwaccel_clear_tag(skb);
 		}
 
 		skb_record_rx_queue(skb, 0);
@@ -2889,29 +2876,11 @@  static netdev_features_t mtk_fix_features(struct net_device *dev,
 
 static int mtk_set_features(struct net_device *dev, netdev_features_t features)
 {
-	struct mtk_mac *mac = netdev_priv(dev);
-	struct mtk_eth *eth = mac->hw;
 	netdev_features_t diff = dev->features ^ features;
-	int i;
 
 	if ((diff & NETIF_F_LRO) && !(features & NETIF_F_LRO))
 		mtk_hwlro_netdev_disable(dev);
 
-	/* Set RX VLAN offloading */
-	if (!(diff & NETIF_F_HW_VLAN_CTAG_RX))
-		return 0;
-
-	mtk_w32(eth, !!(features & NETIF_F_HW_VLAN_CTAG_RX),
-		MTK_CDMP_EG_CTRL);
-
-	/* sync features with other MAC */
-	for (i = 0; i < MTK_MAC_COUNT; i++) {
-		if (!eth->netdev[i] || eth->netdev[i] == dev)
-			continue;
-		eth->netdev[i]->features &= ~NETIF_F_HW_VLAN_CTAG_RX;
-		eth->netdev[i]->features |= features & NETIF_F_HW_VLAN_CTAG_RX;
-	}
-
 	return 0;
 }
 
@@ -3211,30 +3180,6 @@  static int mtk_open(struct net_device *dev)
 	struct mtk_eth *eth = mac->hw;
 	int i, err;
 
-	if (mtk_uses_dsa(dev) && !eth->prog) {
-		for (i = 0; i < ARRAY_SIZE(eth->dsa_meta); i++) {
-			struct metadata_dst *md_dst = eth->dsa_meta[i];
-
-			if (md_dst)
-				continue;
-
-			md_dst = metadata_dst_alloc(0, METADATA_HW_PORT_MUX,
-						    GFP_KERNEL);
-			if (!md_dst)
-				return -ENOMEM;
-
-			md_dst->u.port_info.port_id = i;
-			eth->dsa_meta[i] = md_dst;
-		}
-	} else {
-		/* Hardware special tag parsing needs to be disabled if at least
-		 * one MAC does not use DSA.
-		 */
-		u32 val = mtk_r32(eth, MTK_CDMP_IG_CTRL);
-		val &= ~MTK_CDMP_STAG_EN;
-		mtk_w32(eth, val, MTK_CDMP_IG_CTRL);
-	}
-
 	err = phylink_of_phy_connect(mac->phylink, mac->of_node, 0);
 	if (err) {
 		netdev_err(dev, "%s: could not attach PHY: %d\n", __func__,
@@ -3273,6 +3218,39 @@  static int mtk_open(struct net_device *dev)
 	phylink_start(mac->phylink);
 	netif_tx_start_all_queues(dev);
 
+	if (MTK_HAS_CAPS(eth->soc->caps, MTK_NETSYS_V2))
+		return 0;
+
+	if (mtk_uses_dsa(dev) && !eth->prog) {
+		for (i = 0; i < ARRAY_SIZE(eth->dsa_meta); i++) {
+			struct metadata_dst *md_dst = eth->dsa_meta[i];
+
+			if (md_dst)
+				continue;
+
+			md_dst = metadata_dst_alloc(0, METADATA_HW_PORT_MUX,
+						    GFP_KERNEL);
+			if (!md_dst)
+				return -ENOMEM;
+
+			md_dst->u.port_info.port_id = i;
+			eth->dsa_meta[i] = md_dst;
+		}
+	} else {
+		/* Hardware special tag parsing needs to be disabled if at least
+		 * one MAC does not use DSA.
+		 */
+		u32 val = mtk_r32(eth, MTK_CDMP_IG_CTRL);
+		val &= ~MTK_CDMP_STAG_EN;
+		mtk_w32(eth, val, MTK_CDMP_IG_CTRL);
+
+		val = mtk_r32(eth, MTK_CDMQ_IG_CTRL);
+		val &= ~MTK_CDMQ_STAG_EN;
+		mtk_w32(eth, val, MTK_CDMQ_IG_CTRL);
+
+		mtk_w32(eth, 0, MTK_CDMP_EG_CTRL);
+	}
+
 	return 0;
 }
 
@@ -3599,10 +3577,9 @@  static int mtk_hw_init(struct mtk_eth *eth)
 	if (!MTK_HAS_CAPS(eth->soc->caps, MTK_NETSYS_V2)) {
 		val = mtk_r32(eth, MTK_CDMP_IG_CTRL);
 		mtk_w32(eth, val | MTK_CDMP_STAG_EN, MTK_CDMP_IG_CTRL);
-	}
 
-	/* Enable RX VLan Offloading */
-	mtk_w32(eth, 1, MTK_CDMP_EG_CTRL);
+		mtk_w32(eth, 1, MTK_CDMP_EG_CTRL);
+	}
 
 	/* set interrupt delays based on current Net DIM sample */
 	mtk_dim_rx(&eth->rx_dim.work);
@@ -4203,7 +4180,7 @@  static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np)
 		eth->netdev[id]->hw_features |= NETIF_F_LRO;
 
 	eth->netdev[id]->vlan_features = eth->soc->hw_features &
-		~(NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_CTAG_RX);
+		~NETIF_F_HW_VLAN_CTAG_TX;
 	eth->netdev[id]->features |= eth->soc->hw_features;
 	eth->netdev[id]->ethtool_ops = &mtk_ethtool_ops;
 
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index 420a7d412f24..e8bb8b511f06 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -48,7 +48,6 @@ 
 #define MTK_HW_FEATURES		(NETIF_F_IP_CSUM | \
 				 NETIF_F_RXCSUM | \
 				 NETIF_F_HW_VLAN_CTAG_TX | \
-				 NETIF_F_HW_VLAN_CTAG_RX | \
 				 NETIF_F_SG | NETIF_F_TSO | \
 				 NETIF_F_TSO6 | \
 				 NETIF_F_IPV6_CSUM |\