From owner-freebsd-stable@FreeBSD.ORG Fri Feb 5 02:51:15 2010 Return-Path: Delivered-To: freebsd-stable@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 3A0C61065676 for ; Fri, 5 Feb 2010 02:51:15 +0000 (UTC) (envelope-from max@love2party.net) Received: from moutng.kundenserver.de (moutng.kundenserver.de [212.227.126.186]) by mx1.freebsd.org (Postfix) with ESMTP id D58EE8FC17 for ; Fri, 5 Feb 2010 02:51:14 +0000 (UTC) Received: from vampire.homelinux.org (dslb-088-064-184-208.pools.arcor-ip.net [88.64.184.208]) by mrelayeu.kundenserver.de (node=mreu0) with ESMTP (Nemesis) id 0M1kWE-1Ns13C3ckc-00t1iK; Fri, 05 Feb 2010 03:51:13 +0100 Received: (qmail 64812 invoked from network); 5 Feb 2010 02:51:12 -0000 Received: from f8x64.laiers.local (192.168.4.188) by mx.laiers.local with SMTP; 5 Feb 2010 02:51:12 -0000 From: Max Laier Organization: FreeBSD To: freebsd-stable@freebsd.org Date: Fri, 5 Feb 2010 03:51:12 +0100 User-Agent: KMail/1.12.4 (FreeBSD/8.0-RELEASE-p2; KDE/4.3.4; amd64; ; ) References: <147432021001310037n1b67f01bx4b4e8781321cea8@mail.gmail.com> <2a41acea1002021443t1c298528i2df3cf40269c733@mail.gmail.com> <2a41acea1002021447t1067ee42gc59b25216270459b@mail.gmail.com> In-Reply-To: <2a41acea1002021447t1067ee42gc59b25216270459b@mail.gmail.com> MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_ge4aL9vBl3gxbuc" Message-Id: <201002050351.12270.max@love2party.net> X-Provags-ID: V01U2FsdGVkX18Lr9EKfEyHrmgm+Xvkd+/3WI1cPcrAHDSsGiA w/XNL/djQ1x/aBmZfbPmx7JhcJEnkl9Q/LfqldKviX8CUQ7Q4X Wsb+wxFOTUKiiXP/9Hfpg== Cc: pyunyh@gmail.com, Nick Rogers , jfv@freebsd.org, Jack Vogel Subject: Re: em(4) + ALTQ broken X-BeenThere: freebsd-stable@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Production branch of FreeBSD source code List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 05 Feb 2010 02:51:15 -0000 --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--