Thread (13 messages) 13 messages, 4 authors, 2009-06-08

Re: [Security, resend] Instant crash with rtl8169 and large packets

From: Michael Tokarev <hidden>
Date: 2009-06-08 21:28:09
Also in: lkml

Eric Dumazet wrote:
Michael Tokarev a écrit :
quoted
Eric Dumazet wrote:
[]
quoted
OK I suspect driver is buggy since 2.6.10 days :)

Could you try this patch ?
Tried it, and it appears to work.  Tried various MTU combinations
and packet sizes.  Checked iperf too, with and without the patch
and with different MTU too, to be sure the patch does not introduce
any slowdowns - everything looks sane.  In case the incoming packet
is larger than the RX buffer size, `errors' and `frames' RX stats
gets incremented.

The only somewhat odd thing is that rx path accepts packets larger
than MTU by 3 bytes.  For example, if I set mtu to 2000, the
largest packet I can send is 2003 bytes; with mtu=2002, largest
actual packet size is 2005 bytes.  This is complete frame - in
terms of ping size (ping -s) it's 1975 and 1977 bytes.  That to
say, maybe we still have some corner case somewhere, for packets
larger than mtu by 1, 2 or 3 bytes.

Also I didn't try MTU < 1500.

Other than that,

Tested-By: Michael Tokarev <redacted>
Could you confirm this last patch was ok without former two patches ?
Yes, it's the last patch without former two which were for
debugging as I understand them.  Got fresh 2.6.29.4 source
and applied your last patch to it, recompiled.  All the testing
above were done this way.
quoted
And by the way, your email client uses quoted-printable encoding.
I had to use trivial perl one-liner to convert your patches to
plaintext.  JFYI.
Ah yes, this is when I reply to one of your mail, thank you for the hint.

When submitting a new mail, my thunderbird agent uses a regular "Content-Transfer-Encoding: 7bit"
Heh.  I know why it's this way.  Due to your
"Michael Tokarev a écrit" in first line.  Which
gets added by Thunderbird, and which causes it
to force quoted-printable instead of 7bit, because
of this "é".  Mine offers "пишет" instead of "écrit"
("wrote" in English) and the result is similar.
quoted hunk ↗ jump to hunk
BTW, this driver uses NAPI, but still calls dev_kfree_skb_irq() in rtl8169_tx_interrupt()

You probably can get better performance calling dev_kfree_skb(tx_skb->skb); instead
@@ -3372,7 +3372,7 @@ static void rtl8169_tx_interrupt(struct net_device *dev,
                rtl8169_unmap_tx_skb(tp->pci_dev, tx_skb, tp->TxDescArray + entry);

                if (status & LastFrag) {
-                       dev_kfree_skb_irq(tx_skb->skb);
+                       dev_kfree_skb(tx_skb->skb);
                        tx_skb->skb = NULL;
                }
                dirty_tx++;
Well, the performance is quite good -- 935Mb/sec according to iperf
for TCP.  With UDP I got 1.05Gb/sec, but CPU usage is 100% during
all test time (for TCP test the CPU is in use for less than 5%).
I'll try the change tomorrow (it's 01:27 here now already).

Thank you for the good work!

/mjt
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help