Date: Sun, 18 Sep 2005 23:25:35 +0400 (MSD) From: Dmitrij Tejblum <tejblum@yandex-team.ru> To: FreeBSD-gnats-submit@FreeBSD.org Subject: kern/86306: [patch] if_em.c locks up while trying to send a highly fragmented packet Message-ID: <200509181925.j8IJPZPe046693@walrus-t.yandex.ru> Resent-Message-ID: <200509181930.j8IJU4bI098618@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>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:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200509181925.j8IJPZPe046693>