netdev - [RFC] net: stmmac: RX Jumbo packet size

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 25 Oct 2018 15:41:39 -0500
From:   Thor Thayer <thor.thayer@...ux.intel.com>
To:     Giuseppe CAVALLARO <peppe.cavallaro@...com>,
        alexandre.torgue@...com, joabreu@...opsys.com,
        netdev@...r.kernel.org
Subject: [RFC] net: stmmac: RX Jumbo packet size > 8191 problem

Hi,

I'm running into a weird issue at the DMA boundary for large packets 
(>8192) that I can't explain.  I'm hoping someone here has an idea on 
why I'm seeing this issue.

This is the Synopsys DesignWare Ethernet GMAC core (3.74) using the 
stmmac driver found at drivers/net/ethernet/stmicro/stmmac.

If I ping with data sizes that exceed the first DMA buffer size (size 
set to 8191), ping reports a data mismatch as follows at byte #8144:

$ ping -c 1 -M do -s 8150 192.168.1.99
PING 192.168.1.99 (192.168.1.99) 8150(8178) bytes of data.
8158 bytes from 192.168.1.99: icmp_seq=1 ttl=64 time=0.669 ms
wrong data byte #8144 should be 0xd0 but was 0x0
#16	10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f 20 21 22 23 24 25 26 
27 28 29 2a 2b 2c 2d 2e 2f
%< ---------------snip--------------------------------------
#8112	b0 b1 b2 b3 b4 b5 b6 b7 b8 b9 ba bb bc bd be bf c0 c1 c2 c3 c4 c5 
c6 c7 c8 c9 ca cb cc cd ce cf
#8144	0 0 0 0 d0 d1
         ^^^^^^^
Notice the 4 bytes of 0 there before the expected byte of d0. I 
confirmed the on-wire result with wireshark - same data packet as shown 
above.

Looking at the queue, I'm seeing these values in the RX descriptors (I'm 
using ring mode, enhanced descriptors).
0xa0040320 0x9fff1fff 0x7a358042 0x7a35a042
  ^des0      ^des1      ^des2      ^desc3

desc0 => 8196 bytes, OWN, First & Last Descriptor, Frame type = Eth
desc1 => Disable IRQ on done, Rx Buffer2 sz = 8191, Rx Buffer1 sz = 8191
desc2 => Buffer 1 Addr Pointer
desc3 => Buffer 2 Addr Pointer

If I adjust init_desc3() and refill_desc3() to initialize desc3 to 
desc2+BUF_SIZE_8KiB-4, I get a descriptor as show below and ping 
completes successfully.
0xa0040320 0x9fff1fff 0x77df8042 0x77dfa03e
                                   ^ this is now different

But I'm not sure why the -4 works because desc3 overlaps into the end of 
the first DMA buffer area (des2) which is counterintuitive.

At first I thought the 4 extra bytes were the FCS but that should occur 
at the end of the complete transfer, so I'd expect it to be at the end 
of all the data (in buffer2)

Here is the change that works. I ran a ping sweep with packet sizes from 
8100 to 8300 successfully with this change.
-------------------------------------------------------
$ git diff
diff --git a/drivers/net/ethernet/stmicro/stmmac/ring_mode.c 
b/drivers/net/ethernet/stmicro/stmmac/ring_mode.c
index abc3f85270cd..b52be0235d8f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/ring_mode.c
+++ b/drivers/net/ethernet/stmicro/stmmac/ring_mode.c
@@ -115,13 +115,13 @@ static void refill_desc3(void *priv_ptr, struct 
dma_desc *p)

         /* Fill DES3 in case of RING mode */
         if (priv->dma_buf_sz >= BUF_SIZE_8KiB)
-               p->des3 = cpu_to_le32(le32_to_cpu(p->des2) + BUF_SIZE_8KiB);
+               p->des3 = cpu_to_le32(le32_to_cpu(p->des2) +
+                                     BUF_SIZE_8KiB - 4);

  }

  /* In ring mode we need to fill the desc3 because it is used as buffer */
  static void init_desc3(struct dma_desc *p)
  {
-       p->des3 = cpu_to_le32(le32_to_cpu(p->des2) + BUF_SIZE_8KiB);
+       p->des3 = cpu_to_le32(le32_to_cpu(p->des2) + BUF_SIZE_8KiB - 4);
  }

  static void clean_desc3(void *priv_ptr, struct dma_desc *p)
-----------------------------------------------------------

Any thoughts on why I need to change the indexing?

Thanks,

Thor