Re: [PATCH net] net: dsa: mt7530: don't change PVC_EG_TAG when CPU port becomes VLAN-aware

From: Arınç ÜNAL
Date: Mon Feb 06 2023 - 11:42:05 EST

Next message: kernel test robot: "Re: [PATCH v2 2/2] nvmem: brcm_nvram: use bcm47xx buffered data"
Previous message: Steven Rostedt: "Re: [PATCH 5/5] platform/x86/intel/ifs: Trace support for array test"
In reply to: Arınç ÜNAL: "Re: [PATCH net] net: dsa: mt7530: don't change PVC_EG_TAG when CPU port becomes VLAN-aware"
Next in thread: Vladimir Oltean: "Re: [PATCH net] net: dsa: mt7530: don't change PVC_EG_TAG when CPU port becomes VLAN-aware"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Finally I got time. It's been a seismically active day where I'm from.

On 6.02.2023 02:50, Vladimir Oltean wrote:

On Mon, Feb 06, 2023 at 02:02:48AM +0300, Arınç ÜNAL wrote:

# ethtool -S eth1 | grep -v ': 0'
NIC statistics:
tx_bytes: 6272
tx_packets: 81
rx_bytes: 9089
rx_packets: 136
p05_TxUnicast: 52
p05_TxMulticast: 3
p05_TxBroadcast: 81
p05_TxPktSz65To127: 136
p05_TxBytes: 9633
p05_RxFiltering: 11
p05_RxUnicast: 11
p05_RxMulticast: 26
p05_RxBroadcast: 44
p05_RxPktSz64: 47
p05_RxPktSz65To127: 34
p05_RxBytes: 6272
# ethtool -S eth1 | grep -v ': 0'
NIC statistics:
tx_bytes: 6784
tx_packets: 89
rx_bytes: 9601
rx_packets: 144
p05_TxUnicast: 60
p05_TxMulticast: 3
p05_TxBroadcast: 81
p05_TxPktSz65To127: 144
p05_TxBytes: 10177
p05_RxFiltering: 11
p05_RxUnicast: 11
p05_RxMulticast: 26
p05_RxBroadcast: 52
p05_RxPktSz64: 55
p05_RxPktSz65To127: 34
p05_RxBytes: 6784
# ethtool -S eth1 | grep -v ': 0'
NIC statistics:
tx_bytes: 7424
tx_packets: 99
rx_bytes: 10241
rx_packets: 154
p05_TxUnicast: 70
p05_TxMulticast: 3
p05_TxBroadcast: 81
p05_TxPktSz65To127: 154
p05_TxBytes: 10857
p05_RxFiltering: 11
p05_RxUnicast: 11
p05_RxMulticast: 26
p05_RxBroadcast: 62
p05_RxPktSz64: 65
p05_RxPktSz65To127: 34
p05_RxBytes: 7424

I see no signs of packet loss on the DSA master or the CPU port.
However my analysis of the packets shows:

# tcpdump -i eth1 -e -n -Q in -XX
tcpdump: verbose output suppressed, use -v[v]... for full protocol decode
listening on eth1, link-type NULL (BSD loopback), snapshot length 262144 bytes
03:50:38.645568 AF Unknown (2459068999), length 60:
0x0000: 9292 6a47 1ac0 e0d5 5ea4 edcc 0806 0001 ..jG....^.......

^ ^ ^
| | |
| | ETH_P_ARP
| MAC SA:
| e0:d5:5e:a4:ed:cc
MAC DA:
92:92:6a:47:1a:c0

0x0010: 0800 0604 0002 e0d5 5ea4 edcc c0a8 0202 ........^.......
0x0020: 9292 6a47 1ac0 c0a8 0201 0000 0000 0000 ..jG............
0x0030: 0000 0000 0000 0000 0000 0000 ............

So you have no tag_mtk header in the EtherType position where it's
supposed to be. This means you must be making use of the hardware DSA
untagging feature that Felix Fietkau added.

Let's do some debugging. I'd like to know 2 things, in this order.
First, whether DSA sees the accelerated header (stripped by hardware, as
opposed to being present in the packet):

diff --git a/net/dsa/tag.c b/net/dsa/tag.c
index b2fba1a003ce..e64628cf7fc1 100644
--- a/net/dsa/tag.c
+++ b/net/dsa/tag.c
@@ -75,12 +75,17 @@ static int dsa_switch_rcv(struct sk_buff *skb, struct net_device *dev,
if (!skb_has_extensions(skb))
skb->slow_gro = 0;
+ netdev_err(dev, "%s: skb %px metadata dst contains port id %d attached\n",
+ __func__, skb, port);
+
skb->dev = dsa_master_find_slave(dev, 0, port);
if (likely(skb->dev)) {
dsa_default_offload_fwd_mark(skb);
nskb = skb;
}
} else {
+ netdev_err(dev, "%s: there is no metadata dst attached to skb 0x%px\n",
+ __func__, skb);
nskb = cpu_dp->rcv(skb, dev);
}

# ping 192.168.2.2
PING 192.168.2.2[ 39.508013] mtk_soc_eth 1b100000.ethernet eth1: dsa_switch_rcv: there is no metadata dst attached to skb 0xc2dfecc0
(192.168.2.2): 56 data bytes
[ 40.558253] mtk_soc_eth 1b100000.ethernet eth1: dsa_switch_rcv: there is no metadata dst attached to skb 0xc2dfed80
^C
--- 192.168.2.2 ping statistics ---
2 packets transmitted, 0 packets received, 100% packet loss
# [ 41.598312] mtk_soc_eth 1b100000.ethernet eth1: dsa_switch_rcv: there is no metadata dst attached to skb 0xc2dfee40
[ 55.432363] mtk_soc_eth 1b100000.ethernet eth1: dsa_switch_rcv: there is no metadata dst attached to skb 0xc2dfef00
[ 56.442233] mtk_soc_eth 1b100000.ethernet eth1: dsa_switch_rcv: there is no metadata dst attached to skb 0xc2dfef00
[ 57.466253] mtk_soc_eth 1b100000.ethernet eth1: dsa_switch_rcv: there is no metadata dst attached to skb 0xc2dfef00
[ 60.538211] mtk_soc_eth 1b100000.ethernet eth1: dsa_switch_rcv: there is no metadata dst attached to skb 0xc2dfef00
[ 61.562191] mtk_soc_eth 1b100000.ethernet eth1: dsa_switch_rcv: there is no metadata dst attached to skb 0xc2dfec00
[ 62.586190] mtk_soc_eth 1b100000.ethernet eth1: dsa_switch_rcv: there is no metadata dst attached to skb 0xc2dfeb40

On a working port:

[ 113.278462] mt7530 mdio-bus:1f wan: Link is Down
[ 113.283214] br0: port 1(wan) entered disabled state
[ 115.438955] mt7530 mdio-bus:1f lan0: Link is Up - 1Gbps/Full - flow control off
[ 115.446332] br0: port 2(lan0) entered blocking state
[ 115.451346] br0: port 2(lan0) entered forwarding state
[ 118.007199] mtk_soc_eth 1b100000.ethernet eth1: dsa_switch_rcv: skb c2dfeb40 metadata dst contains port id 1 attached
[ 118.018209] mtk_soc_eth 1b100000.ethernet eth1: dsa_switch_rcv: skb c2dfeb40 metadata dst contains port id 1 attached
[ 119.009252] mtk_soc_eth 1b100000.ethernet eth1: dsa_switch_rcv: skb c2dfed80 metadata dst contains port id 1 attached
[ 120.010470] mtk_soc_eth 1b100000.ethernet eth1: dsa_switch_rcv: skb c2dfed80 metadata dst contains port id 1 attached
[ 123.038246] mtk_soc_eth 1b100000.ethernet eth1: dsa_switch_rcv: skb c2dfe900 metadata dst contains port id 1 attached

And second, which is what does the DSA master actually see, and put in
the skb metadata dst field:

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index f1cb1efc94cf..e7ff569959b4 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -2077,11 +2077,23 @@ static int mtk_poll_rx(struct napi_struct *napi, int budget,
if (skb_vlan_tag_present(skb) && netdev_uses_dsa(netdev)) {
unsigned int port = ntohs(skb->vlan_proto) & GENMASK(2, 0);
+ netdev_err(netdev, "%s: skb->vlan_proto 0x%x port %d\n", __func__,
+ ntohs(skb->vlan_proto), port);
+
if (port < ARRAY_SIZE(eth->dsa_meta) &&
- eth->dsa_meta[port])
+ eth->dsa_meta[port]) {
+ netdev_err(netdev, "%s: attaching metadata dst with port %d to skb 0x%px\n",
+ __func__, port, skb);
skb_dst_set_noref(skb, &eth->dsa_meta[port]->dst);
+ } else {
+ netdev_err(netdev, "%s: not attaching any metadata dst to skb 0x%px\n",
+ __func__, skb);
+ }
__vlan_hwaccel_clear_tag(skb);
+ } else if (netdev_uses_dsa(netdev)) {
+ netdev_err(netdev, "%s: received skb 0x%px without VLAN/DSA tag present\n",
+ __func__, skb);
}
skb_record_rx_queue(skb, 0);

Be warned that there may be a considerable amount of output to the console,
so it would be best if you used a single switch port with small amounts
of traffic.

# ping 192.168.2.2
PING 192.168.2.2[ 22.674182] mtk_soc_eth 1b100000.ethernet eth1: mtk_poll_rx: received skb 0xc2d67840 without VLAN/DSA tag present
(192.168.2.2): 56 data bytes
[ 23.678336] mtk_soc_eth 1b100000.ethernet eth1: mtk_poll_rx: received skb 0xc2d67840 without VLAN/DSA tag present
[ 24.718355] mtk_soc_eth 1b100000.ethernet eth1: mtk_poll_rx: received skb 0xc2d67840 without VLAN/DSA tag present
^C
--- 192.168.2.2 ping statistics ---
4 packets transmitted, 0 packets received, 100% packet loss
# [ 28.757693] mtk_soc_eth 1b100000.ethernet eth1: mtk_poll_rx: received skb 0xc2d67840 without VLAN/DSA tag present
[ 29.758347] mtk_soc_eth 1b100000.ethernet eth1: mtk_poll_rx: received skb 0xc2d67840 without VLAN/DSA tag present
[ 30.782404] mtk_soc_eth 1b100000.ethernet eth1: mtk_poll_rx: received skb 0xc2d67840 without VLAN/DSA tag present
[ 33.854281] mtk_soc_eth 1b100000.ethernet eth1: mtk_poll_rx: received skb 0xc2d67840 without VLAN/DSA tag present

On a working port:

[ 48.798419] mt7530 mdio-bus:1f wan: Link is Down
[ 48.803166] br0: port 1(wan) entered disabled state
[ 50.958903] mt7530 mdio-bus:1f lan0: Link is Up - 1Gbps/Full - flow control off
[ 50.966282] br0: port 2(lan0) entered blocking state
[ 50.971300] br0: port 2(lan0) entered forwarding state
[ 54.261846] mtk_soc_eth 1b100000.ethernet eth1: mtk_poll_rx: skb->vlan_proto 0x1 port 1
[ 54.269905] mtk_soc_eth 1b100000.ethernet eth1: mtk_poll_rx: attaching metadata dst with port 1 to skb 0xc2d67840
[ 54.280412] mtk_soc_eth 1b100000.ethernet eth1: mtk_poll_rx: skb->vlan_proto 0x1 port 1
[ 54.288460] mtk_soc_eth 1b100000.ethernet eth1: mtk_poll_rx: attaching metadata dst with port 1 to skb 0xc2d67840
[ 55.263241] mtk_soc_eth 1b100000.ethernet eth1: mtk_poll_rx: skb->vlan_proto 0x1 port 1
[ 55.271292] mtk_soc_eth 1b100000.ethernet eth1: mtk_poll_rx: attaching metadata dst with port 1 to skb 0xc2d67840
[ 59.358317] mtk_soc_eth 1b100000.ethernet eth1: mtk_poll_rx: skb->vlan_proto 0x1 port 1
[ 59.366361] mtk_soc_eth 1b100000.ethernet eth1: mtk_poll_rx: attaching metadata dst with port 1 to skb 0xc2d67a80

Arınç

Next message: kernel test robot: "Re: [PATCH v2 2/2] nvmem: brcm_nvram: use bcm47xx buffered data"
Previous message: Steven Rostedt: "Re: [PATCH 5/5] platform/x86/intel/ifs: Trace support for array test"
In reply to: Arınç ÜNAL: "Re: [PATCH net] net: dsa: mt7530: don't change PVC_EG_TAG when CPU port becomes VLAN-aware"
Next in thread: Vladimir Oltean: "Re: [PATCH net] net: dsa: mt7530: don't change PVC_EG_TAG when CPU port becomes VLAN-aware"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]