From owner-freebsd-bugs@FreeBSD.ORG Mon Sep 19 07:40:20 2005 Return-Path: X-Original-To: freebsd-bugs@hub.freebsd.org Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 7DEC816A41F for ; Mon, 19 Sep 2005 07:40:20 +0000 (GMT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id 3FA8D43D49 for ; Mon, 19 Sep 2005 07:40:20 +0000 (GMT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.13.3/8.13.3) with ESMTP id j8J7eKSh091464 for ; Mon, 19 Sep 2005 07:40:20 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.13.3/8.13.1/Submit) id j8J7eKbT091463; Mon, 19 Sep 2005 07:40:20 GMT (envelope-from gnats) Date: Mon, 19 Sep 2005 07:40:20 GMT Message-Id: <200509190740.j8J7eKbT091463@freefall.freebsd.org> To: freebsd-bugs@FreeBSD.org From: Dmitrij Tejblum Cc: Subject: Re: kern/86306: [patch] if_em.c locks up while trying to send a highly fragmented packet X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Dmitrij Tejblum List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 19 Sep 2005 07:40:20 -0000 The following reply was made to PR kern/86306; it has been noted by GNATS. From: Dmitrij Tejblum To: Ruslan Ermilov 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 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--