kern/86306: [patch] if_em.c locks up while trying to send a highly fragmented packet

[Dmitrij Tejblum]

>Number:         86306
>Category:       kern
>Synopsis:       [patch] if_em.c locks up while trying to send a highly fragmented packet
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Sun Sep 18 19:30:04 GMT 2005
>Closed-Date:
>Last-Modified:
>Originator:     Dmitrij Tejblum
>Release:        FreeBSD 5.4-STABLE i386
>Organization:
OOO Yandex
>Environment:

>Description:

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).

>How-To-Repeat:

This is more likely with jumbo frames enabled.

>Fix:


Index: if_em.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/em/if_em.c,v
retrieving revision 1.44.2.9
diff -u -r1.44.2.9 if_em.c
--- if_em.c	19 May 2005 08:23:06 -0000	1.44.2.9
+++ if_em.c	18 Sep 2005 18:43:15 -0000
@@ -631,13 +631,6 @@
 			break;
                 }
 
-		/* Send a copy of the frame to the BPF listener */
-#if __FreeBSD_version < 500000
-                if (ifp->if_bpf)
-                        bpf_mtap(ifp, m_head);
-#else
-		BPF_MTAP(ifp, m_head);
-#endif
         
                 /* Set timeout in case hardware has problems transmitting */
                 ifp->if_timer = EM_TX_TIMEOUT;
@@ -1221,10 +1214,37 @@
         }
         error = bus_dmamap_load_mbuf_sg(adapter->txtag, map, m_head, segs,
 					&nsegs, BUS_DMA_NOWAIT);
-        if (error != 0) {
+        if (error != 0 && error != EFBIG) {
+                printf("em%d: can't map mbuf (error %d)\n",
+                        adapter->unit, error);
                 adapter->no_tx_dma_setup++;
                 bus_dmamap_destroy(adapter->txtag, map);
-                return (error);
+                m_freem(m_head);
+                *m_headp = NULL;
+                return (0);
+        } else if (error == EFBIG) {
+                struct mbuf *mn;
+                mn = m_defrag(m_head, M_DONTWAIT);
+                if (mn == NULL) {
+                        printf("em%d: can't defrag mbuf\n", adapter->unit);
+                        bus_dmamap_destroy(adapter->txtag, map);
+                        m_freem(m_head);
+                        *m_headp = NULL;
+                        adapter->no_tx_dma_setup++;
+                        return (0);
+                }
+                m_head = mn;
+                error = bus_dmamap_load_mbuf_sg(adapter->txtag, map, m_head,
+                                                segs, &nsegs, BUS_DMA_NOWAIT);
+                if (error != 0) {
+                        printf("em%d: can't map mbuf2 (error %d)\n",
+                                adapter->unit, error);
+                        bus_dmamap_destroy(adapter->txtag, map);
+                        m_freem(m_head);
+                        *m_headp = NULL;
+                        adapter->no_tx_dma_setup++;
+                        return (0);
+                }
         }
         KASSERT(nsegs != 0, ("em_encap: empty packet"));
 
@@ -1390,6 +1410,13 @@
                 }
         }
 
+        /* Send a copy of the frame to the BPF listener */
+#if __FreeBSD_version < 500000
+        if (ifp->if_bpf)
+               bpf_mtap(ifp, m_head);
+#else
+        BPF_MTAP(ifp, m_head);
+#endif
         return(0);
 }
 
@@ -3258,7 +3285,8 @@
 	adapter->stats.mpc + adapter->stats.cexterr;
 
 	/* Tx Errors */
-	ifp->if_oerrors = adapter->stats.ecol + adapter->stats.latecol;
+	ifp->if_oerrors = adapter->stats.ecol + adapter->stats.latecol + 
+		adapter->no_tx_dma_setup + adapter->no_tx_map_avail;
 
 }
 


(When em_encap() return an error, futher transmission stops and the interface
is marked with OACTIVE. But OACTIVE mean that the *hardware* output queue is
full, and the code that clear OACTIVE assume so. Therefore I think that in 
case of mbuf or dmamap failure it is better to return 0 from em_encap()).

>Release-Note:
>Audit-Trail:
>Unformatted: