Date: Fri, 5 Feb 2010 03:51:12 +0100 From: Max Laier <max@love2party.net> To: freebsd-stable@freebsd.org Cc: pyunyh@gmail.com, Nick Rogers <ncrogers@gmail.com>, jfv@freebsd.org, Jack Vogel <jfvogel@gmail.com> Subject: Re: em(4) + ALTQ broken Message-ID: <201002050351.12270.max@love2party.net> In-Reply-To: <2a41acea1002021447t1067ee42gc59b25216270459b@mail.gmail.com> References: <147432021001310037n1b67f01bx4b4e8781321cea8@mail.gmail.com> <2a41acea1002021443t1c298528i2df3cf40269c733@mail.gmail.com> <2a41acea1002021447t1067ee42gc59b25216270459b@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--Boundary-00=_ge4aL9vBl3gxbuc
Content-Type: Text/Plain;
charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
Okay ... attached is a patch to fix this for em(4) (and lay the groundwork to
fix it for other drbr_* consumer as well). I have tested it in VirtualBox,
but don't have real hardware to check for non-ALTQ performance or other
regressions.
Test, comments and review appreciated.
--
Max
--Boundary-00=_ge4aL9vBl3gxbuc
Content-Type: text/x-patch;
charset="ISO-8859-1";
name="drbr_altq.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename="drbr_altq.diff"
Index: sys/dev/e1000/if_em.c
===================================================================
--- sys/dev/e1000/if_em.c (revision 203362)
+++ sys/dev/e1000/if_em.c (working copy)
@@ -943,7 +943,7 @@
{
struct adapter *adapter = ifp->if_softc;
struct mbuf *next;
- int error = E1000_SUCCESS;
+ int ret, error = E1000_SUCCESS;
EM_TX_LOCK_ASSERT(adapter);
/* To allow being called from a tasklet */
@@ -955,28 +955,34 @@
|| (!adapter->link_active)) {
error = drbr_enqueue(ifp, adapter->br, m);
return (error);
- } else if (drbr_empty(ifp, adapter->br) &&
- (adapter->num_tx_desc_avail > EM_TX_OP_THRESHOLD)) {
- if ((error = em_xmit(adapter, &m)) != 0) {
- if (m)
- error = drbr_enqueue(ifp, adapter->br, m);
+ }
+ ret = drbr_maybe_enqueue(ifp, adapter->br, m);
+ if (ret == 0) {
+ if (adapter->num_tx_desc_avail > EM_TX_OP_THRESHOLD) {
+ if ((error = em_xmit(adapter, &m)) != 0) {
+ if (m)
+ error = drbr_enqueue(ifp, adapter->br,
+ m);
+ return (error);
+ } else {
+ /*
+ * We've bypassed the buf ring so we need to
+ * update ifp directly
+ */
+ drbr_stats_update(ifp, m->m_pkthdr.len,
+ m->m_flags);
+ /*
+ ** Send a copy of the frame to the BPF
+ ** listener and set the watchdog on.
+ */
+ ETHER_BPF_MTAP(ifp, m);
+ adapter->watchdog_check = TRUE;
+ }
+ } else if ((error = drbr_enqueue(ifp, adapter->br, m)) != 0)
return (error);
- } else {
- /*
- * We've bypassed the buf ring so we need to update
- * ifp directly
- */
- drbr_stats_update(ifp, m->m_pkthdr.len, m->m_flags);
- /*
- ** Send a copy of the frame to the BPF
- ** listener and set the watchdog on.
- */
- ETHER_BPF_MTAP(ifp, m);
- adapter->watchdog_check = TRUE;
- }
- } else if ((error = drbr_enqueue(ifp, adapter->br, m)) != 0)
- return (error);
-
+ } else if (ret < 0)
+ return (-ret);
+
process:
if (drbr_empty(ifp, adapter->br))
return(error);
@@ -989,7 +995,7 @@
break;
if ((error = em_xmit(adapter, &next)) != 0) {
if (next != NULL)
- error = drbr_enqueue(ifp, adapter->br, next);
+ error = drbr_requeue(ifp, adapter->br, next);
break;
}
drbr_stats_update(ifp, next->m_pkthdr.len, next->m_flags);
Index: sys/net/if_var.h
===================================================================
--- sys/net/if_var.h (revision 203362)
+++ sys/net/if_var.h (working copy)
@@ -597,18 +597,26 @@
return (error);
}
+static __inline int
+drbr_requeue(struct ifnet *ifp, struct buf_ring *br, struct mbuf *m)
+{
+#ifdef ALTQ
+ if (ALTQ_IS_ENABLED(&ifp->if_snd)) {
+ IFQ_DRV_PREPEND(&ifp->if_snd, m);
+ return (0);
+ }
+#endif
+ return drbr_enqueue(ifp, br, m);
+}
+
static __inline void
drbr_flush(struct ifnet *ifp, struct buf_ring *br)
{
struct mbuf *m;
#ifdef ALTQ
- if (ifp != NULL && ALTQ_IS_ENABLED(&ifp->if_snd)) {
- while (!IFQ_IS_EMPTY(&ifp->if_snd)) {
- IFQ_DRV_DEQUEUE(&ifp->if_snd, m);
- m_freem(m);
- }
- }
+ if (ifp != NULL && ALTQ_IS_ENABLED(&ifp->if_snd))
+ IFQ_DRV_PURGE(&ifp->if_snd);
#endif
while ((m = buf_ring_dequeue_sc(br)) != NULL)
m_freem(m);
@@ -642,11 +650,12 @@
{
struct mbuf *m;
#ifdef ALTQ
- /*
- * XXX need to evaluate / requeue
- */
if (ALTQ_IS_ENABLED(&ifp->if_snd)) {
IFQ_DRV_DEQUEUE(&ifp->if_snd, m);
+ if (m != NULL && func(m, arg) == 0) {
+ IFQ_DRV_PREPEND(&ifp->if_snd, m);
+ return (NULL);
+ }
return (m);
}
#endif
@@ -668,6 +677,24 @@
}
static __inline int
+drbr_maybe_enqueue(struct ifnet *ifp, struct buf_ring *br, struct mbuf *m)
+{
+ int error;
+
+#ifdef ALTQ
+ if (ALTQ_IS_ENABLED(&ifp->if_snd)) {
+ IFQ_ENQUEUE(&ifp->if_snd, m, error);
+ return error ? -error : 1;
+ }
+#endif
+ if (drbr_empty(ifp, br))
+ return 0;
+
+ error = drbr_enqueue(ifp, br, m);
+ return error ? -error : 1;
+}
+
+static __inline int
drbr_inuse(struct ifnet *ifp, struct buf_ring *br)
{
#ifdef ALTQ
--Boundary-00=_ge4aL9vBl3gxbuc
Content-Type: text/x-patch; charset="ISO-8859-1";
name="drbr_altq.stable_8.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename="drbr_altq.stable_8.diff"
diff --git a/sys/dev/e1000/if_em.c b/sys/dev/e1000/if_em.c
index 7a2dbad..94ad229 100644
--- a/sys/dev/e1000/if_em.c
+++ b/sys/dev/e1000/if_em.c
@@ -1020,7 +1020,7 @@ em_mq_start_locked(struct ifnet *ifp, struct mbuf *m)
{
struct adapter *adapter = ifp->if_softc;
struct mbuf *next;
- int error = E1000_SUCCESS;
+ int ret, error = E1000_SUCCESS;
EM_TX_LOCK_ASSERT(adapter);
/* To allow being called from a tasklet */
@@ -1032,28 +1032,34 @@ em_mq_start_locked(struct ifnet *ifp, struct mbuf *m)
|| (!adapter->link_active)) {
error = drbr_enqueue(ifp, adapter->br, m);
return (error);
- } else if (drbr_empty(ifp, adapter->br) &&
- (adapter->num_tx_desc_avail > EM_TX_OP_THRESHOLD)) {
- if ((error = em_xmit(adapter, &m)) != 0) {
- if (m != NULL)
- error = drbr_enqueue(ifp, adapter->br, m);
- return (error);
- } else {
- /*
- * We've bypassed the buf ring so we need to update
- * ifp directly
- */
- drbr_stats_update(ifp, m->m_pkthdr.len, m->m_flags);
- /*
- ** Send a copy of the frame to the BPF
- ** listener and set the watchdog on.
- */
- ETHER_BPF_MTAP(ifp, m);
- adapter->watchdog_timer = EM_TX_TIMEOUT;
- }
- } else if ((error = drbr_enqueue(ifp, adapter->br, m)) != 0)
- return (error);
-
+ }
+ ret = drbr_maybe_enqueue(ifp, adapter->br, m);
+ if (ret == 0) {
+ if (adapter->num_tx_desc_avail > EM_TX_OP_THRESHOLD) {
+ if ((error = em_xmit(adapter, &m)) != 0) {
+ if (m)
+ error = drbr_enqueue(ifp, adapter->br,
+ m);
+ return (error);
+ } else {
+ /*
+ * We've bypassed the buf ring so we need to
+ * update ifp directly
+ */
+ drbr_stats_update(ifp, m->m_pkthdr.len,
+ m->m_flags);
+ /*
+ ** Send a copy of the frame to the BPF
+ ** listener and set the watchdog on.
+ */
+ ETHER_BPF_MTAP(ifp, m);
+ adapter->watchdog_timer = EM_TX_TIMEOUT;
+ }
+ } else if ((error = drbr_enqueue(ifp, adapter->br, m)) != 0)
+ return (error);
+ } else if (ret < 0)
+ return (-ret);
+
process:
if (drbr_empty(ifp, adapter->br))
return(error);
@@ -1066,7 +1072,7 @@ process:
break;
if ((error = em_xmit(adapter, &next)) != 0) {
if (next != NULL)
- error = drbr_enqueue(ifp, adapter->br, next);
+ error = drbr_requeue(ifp, adapter->br, next);
break;
}
drbr_stats_update(ifp, next->m_pkthdr.len, next->m_flags);
diff --git a/sys/net/if_var.h b/sys/net/if_var.h
index eac7dbf..1d59402 100644
--- a/sys/net/if_var.h
+++ b/sys/net/if_var.h
@@ -595,18 +595,26 @@ drbr_enqueue(struct ifnet *ifp, struct buf_ring *br, struct mbuf *m)
return (error);
}
+static __inline int
+drbr_requeue(struct ifnet *ifp, struct buf_ring *br, struct mbuf *m)
+{
+#ifdef ALTQ
+ if (ALTQ_IS_ENABLED(&ifp->if_snd)) {
+ IFQ_DRV_PREPEND(&ifp->if_snd, m);
+ return (0);
+ }
+#endif
+ return drbr_enqueue(ifp, br, m);
+}
+
static __inline void
drbr_flush(struct ifnet *ifp, struct buf_ring *br)
{
struct mbuf *m;
#ifdef ALTQ
- if (ifp != NULL && ALTQ_IS_ENABLED(&ifp->if_snd)) {
- while (!IFQ_IS_EMPTY(&ifp->if_snd)) {
- IFQ_DRV_DEQUEUE(&ifp->if_snd, m);
- m_freem(m);
- }
- }
+ if (ifp != NULL && ALTQ_IS_ENABLED(&ifp->if_snd))
+ IFQ_DRV_PURGE(&ifp->if_snd);
#endif
while ((m = buf_ring_dequeue_sc(br)) != NULL)
m_freem(m);
@@ -640,11 +648,12 @@ drbr_dequeue_cond(struct ifnet *ifp, struct buf_ring *br,
{
struct mbuf *m;
#ifdef ALTQ
- /*
- * XXX need to evaluate / requeue
- */
if (ALTQ_IS_ENABLED(&ifp->if_snd)) {
IFQ_DRV_DEQUEUE(&ifp->if_snd, m);
+ if (m != NULL && func(m, arg) == 0) {
+ IFQ_DRV_PREPEND(&ifp->if_snd, m);
+ return (NULL);
+ }
return (m);
}
#endif
@@ -666,6 +675,24 @@ drbr_empty(struct ifnet *ifp, struct buf_ring *br)
}
static __inline int
+drbr_maybe_enqueue(struct ifnet *ifp, struct buf_ring *br, struct mbuf *m)
+{
+ int error;
+
+#ifdef ALTQ
+ if (ALTQ_IS_ENABLED(&ifp->if_snd)) {
+ IFQ_ENQUEUE(&ifp->if_snd, m, error);
+ return error ? -error : 1;
+ }
+#endif
+ if (drbr_empty(ifp, br))
+ return 0;
+
+ error = drbr_enqueue(ifp, br, m);
+ return error ? -error : 1;
+}
+
+static __inline int
drbr_inuse(struct ifnet *ifp, struct buf_ring *br)
{
#ifdef ALTQ
--Boundary-00=_ge4aL9vBl3gxbuc--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201002050351.12270.max>