Date: Mon, 19 Sep 2005 07:40:20 GMT From: Dmitrij Tejblum <tejblum@yandex-team.ru> To: freebsd-bugs@FreeBSD.org Subject: Re: kern/86306: [patch] if_em.c locks up while trying to send a highly fragmented packet Message-ID: <200509190740.j8J7eKbT091463@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/86306; it has been noted by GNATS.
From: Dmitrij Tejblum <tejblum@yandex-team.ru>
To: Ruslan Ermilov <ru@freebsd.org>
Cc: FreeBSD-gnats-submit@freebsd.org
Subject: Re: kern/86306: [patch] if_em.c locks up while trying to send a highly
fragmented packet
Date: Mon, 19 Sep 2005 11:38:47 +0400
This is a multi-part message in MIME format.
--------------080903090009080801040205
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Ruslan Ermilov wrote:
>Hi Dmitrij,
>
>On Sun, Sep 18, 2005 at 11:25:35PM +0400, Dmitrij Tejblum wrote:
>
>
>>When em_encap() tries to send a very long mbuf chain (i.e. more than
>>EM_MAX_SCATTER == 64 mbufs), bus_dmamap_load_mbuf_sg() may fail with EFBIG.
>>Then em_encap() fail, the packet is not sent and left in the output queue,
>>and thus no futher transmission is possible.
>>
>>Some other driver handle similar condition with m_defrag(9) function
>>(which is intended for this purpose).
>>
>>
>>
>Can you please modify your patch as follows:
>
>1) Count how much fragments are in the packet in em_encap() first, and
> do m_defrag() if it exceeeds EM_MAX_SCATTER, like in if_dc.c. If it
> is still EFBIG after that and bus_dmamap_load_mbuf_sg(), then free it
> as you do to prevent re-enqueue.
>
>
So you think that if_dc.c is a better example than e.g. if_fxp.c and
if_xl.c? Why? I also suspect that your suggestion would be a pessimisation.
>2) Put BPF processing back to em_start_locked().
>
>
As I wrote, I think that if e.g. we were unable to defragment a packet
it is better to drop it and try to send the next, rather than stop and
set OACTIVE (since the code that clear OACTIVE assume that it is about
TX descriptors). (You didn't suggest to change that). I moved BPF
processing since it would not be a good idea to pass NULL to BPF.
>3) Pull up to the HEAD version of the driver.
>
>
Well, please answer 1 and 2 first :-).
>
>Cheers,
>
>
--------------080903090009080801040205
Content-Type: text/html; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<meta content="text/html;charset=ISO-8859-1" http-equiv="Content-Type">
</head>
<body bgcolor="#ffffff" text="#000000">
Ruslan Ermilov wrote:
<blockquote cite="mid20050919062926.GF65954@ip.net.ua" type="cite">
<pre wrap="">Hi Dmitrij,
On Sun, Sep 18, 2005 at 11:25:35PM +0400, Dmitrij Tejblum wrote:
</pre>
<blockquote type="cite">
<pre wrap="">When em_encap() tries to send a very long mbuf chain (i.e. more than
EM_MAX_SCATTER == 64 mbufs), bus_dmamap_load_mbuf_sg() may fail with EFBIG.
Then em_encap() fail, the packet is not sent and left in the output queue,
and thus no futher transmission is possible.
Some other driver handle similar condition with m_defrag(9) function
(which is intended for this purpose).
</pre>
</blockquote>
<pre wrap=""><!---->Can you please modify your patch as follows:
1) Count how much fragments are in the packet in em_encap() first, and
do m_defrag() if it exceeeds EM_MAX_SCATTER, like in if_dc.c. If it
is still EFBIG after that and bus_dmamap_load_mbuf_sg(), then free it
as you do to prevent re-enqueue.
</pre>
</blockquote>
So you think that if_dc.c is a better example than e.g. if_fxp.c and
if_xl.c? Why? I also suspect that your suggestion would be a
pessimisation.<br>
<blockquote cite="mid20050919062926.GF65954@ip.net.ua" type="cite">
<pre wrap="">
2) Put BPF processing back to em_start_locked().
</pre>
</blockquote>
As I wrote, I think that if e.g. we were unable to defragment a packet
it is better to drop it and try to send the next, rather than stop and
set OACTIVE (since the code that clear OACTIVE assume that it is about
TX descriptors). (You didn't suggest to change that). I moved BPF
processing since it would not be a good idea to pass NULL to BPF.<br>
<blockquote cite="mid20050919062926.GF65954@ip.net.ua" type="cite">
<pre wrap="">
3) Pull up to the HEAD version of the driver.
</pre>
</blockquote>
Well, please answer 1 and 2 first :-).<br>
<blockquote cite="mid20050919062926.GF65954@ip.net.ua" type="cite">
<pre wrap="">
Cheers,
</pre>
</blockquote>
<br>
</body>
</html>
--------------080903090009080801040205--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200509190740.j8J7eKbT091463>