On 08/05/2021 17:39, Bjørn Mork wrote:

But I'm not convinced it works so well for the default either...  I see
this received on the native VLAN 1:


canardo:/tmp# tshark -nVi xeth0 -f 'udp port 67'
Running as user "root" and group "root". This could be dangerous.
Capturing on 'xeth0'
Frame 1: 346 bytes on wire (2768 bits), 346 bytes captured (2768 bits) on 
interface 0
     Interface id: 0 (xeth0)
         Interface name: xeth0
     Encapsulation type: Ethernet (1)
     Arrival Time: May  8, 2021 16:56:20.581072322 CEST
     [Time shift for this packet: 0.000000000 seconds]
     Epoch Time: 1620485780.581072322 seconds
     [Time delta from previous captured frame: 0.000000000 seconds]
     [Time delta from previous displayed frame: 0.000000000 seconds]
     [Time since reference or first frame: 0.000000000 seconds]
     Frame Number: 1
     Frame Length: 346 bytes (2768 bits)
     Capture Length: 346 bytes (2768 bits)
     [Frame is marked: False]
     [Frame is ignored: False]
     [Protocols in frame: eth:ethertype:ip:udp:bootp]
Ethernet II, Src: bc:a5:11:9f:e1:23, Dst: ff:ff:ff:ff:ff:ff
     Destination: ff:ff:ff:ff:ff:ff
         Address: ff:ff:ff:ff:ff:ff
         .... ..1. .... .... .... .... = LG bit: Locally administered address 
(this is NOT the factory default)
         .... ...1 .... .... .... .... = IG bit: Group address 
(multicast/broadcast)
     Source: bc:a5:11:9f:e1:23
         Address: bc:a5:11:9f:e1:23
         .... ..0. .... .... .... .... = LG bit: Globally unique address 
(factory default)
         .... ...0 .... .... .... .... = IG bit: Individual address (unicast)
     Type: IPv4 (0x0800)
     Frame check sequence: 0x80091000 incorrect, should be 0x63ff5c43
         [Expert Info (Error/Checksum): Bad checksum [should be 0x63ff5c43]]
             [Bad checksum [should be 0x63ff5c43]]
             [Severity level: Error]
             [Group: Checksum]
     [FCS Status: Bad]
Internet Protocol Version 4, Src: 0.0.0.0, Dst: 255.255.255.255
     0100 .... = Version: 4
     .... 0101 = Header Length: 20 bytes (5)
     Differentiated Services Field: 0x00 (DSCP: CS0, ECN: Not-ECT)
         0000 00.. = Differentiated Services Codepoint: Default (0)
         .... ..00 = Explicit Congestion Notification: Not ECN-Capable 
Transport (0)
     Total Length: 328
     Identification: 0x0000 (0)
     Flags: 0x0000
         0... .... .... .... = Reserved bit: Not set
         .0.. .... .... .... = Don't fragment: Not set
         ..0. .... .... .... = More fragments: Not set
     Fragment offset: 0
     Time to live: 64
     Protocol: UDP (17)
     Header checksum: 0x79a6 [validation disabled]
     [Header checksum status: Unverified]
     Source: 0.0.0.0
     Destination: 255.255.255.255
User Datagram Protocol, Src Port: 68, Dst Port: 67
     Source Port: 68
     Destination Port: 67
     Length: 308
     Checksum: 0xd82b [unverified]
     [Checksum Status: Unverified]
[etc]



That seemingly magic number 0x80091000 instead of the expected FCS is
pretty suspicious. Leaking tag?  Or what?

Yes, this looks like the DSA-tag in the packet buffer is not being overwritten 
by the CRC.
Could you try out the following patch, which tries to follow the convolutions 
of the SDK for the 8380,
with respect of buffer length and packet length when the CRC is being 
calculated by offloading.
On my DLink 1210-10hp I see properly formed packets with small (<60 octets) and 
larger size.
It fixes also the detection of DSA tags with a high port number (>=28) on the 
8390.

diff --git a/target/linux/realtek/files-5.4/drivers/net/ethernet/rtl838x_eth.c 
b/target/linux/realtek/files-5.4/drivers/net/ethernet/rtl838x_eth.c
index c5c6e3b6b7..9fe4fcd647 100644
--- a/target/linux/realtek/files-5.4/drivers/net/ethernet/rtl838x_eth.c
+++ b/target/linux/realtek/files-5.4/drivers/net/ethernet/rtl838x_eth.c
@@ -1129,23 +1129,16 @@ static int rtl838x_eth_tx(struct sk_buff *skb, struct 
net_device *dev)

        /* Check for DSA tagging at the end of the buffer */
        if (netdev_uses_dsa(dev) && skb->data[len-4] == 0x80 && skb->data[len-3] 
> 0
-                       && skb->data[len-3] < 28 &&  skb->data[len-2] == 0x10
+                       && skb->data[len-3] < priv->cpu_port &&  
skb->data[len-2] == 0x10
                        &&  skb->data[len-1] == 0x00) {
                /* Reuse tag space for CRC if possible */
                dest_port = skb->data[len-3];
+               skb->data[len-4] = skb->data[len-3] = skb->data[len-2] = 
skb->data[len-1] = 0x00;
                len -= 4;
        }

        len += 4; // Add space for CRC

-       // On RTL8380 SoCs, the packet needs extra padding
-       if (priv->family_id == RTL8380_FAMILY_ID) {
-               if (len < ETH_ZLEN)
-                       len = ETH_ZLEN; // SoC not automatically padding to 
ETH_ZLEN
-               else
-                       len += 4;
-       }
-
        if (skb_padto(skb, len)) {
                ret = NETDEV_TX_OK;
                goto txdone;
@@ -1158,6 +1151,11 @@ static int rtl838x_eth_tx(struct sk_buff *skb, struct 
net_device *dev)
                h = &ring->tx_header[q][ring->c_tx[q]];
                h->size = len;
                h->len = len;
+               // On RTL8380 SoCs, small packet lengths being sent need 
adjustments
+               if (priv->family_id == RTL8380_FAMILY_ID) {
+                       if (len < ETH_ZLEN - 4)
+                               h->len -= 4;
+               }

                priv->r->create_tx_header(h, dest_port, skb->priority >> 1);



_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Reply via email to