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

Frank Wunderlich
2023-02-06T07:37:43Z

Am 6. Februar 2023 00:50:53 MEZ schrieb Vladimir Oltean <vladimir.oltean@nxp.com>:
>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);
> }
>
>
>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.

Arınç have you tested with or without this series?

https://patchwork.kernel.org/project/linux-mediatek/list/?series=707666

Maybe try the opposite.

Have not see it in net-next yet.
regards Frank