From b056acb8640e378c0d878874ce7f56a67b32ac83 Mon Sep 17 00:00:00 2001 From: mlaier Date: Sat, 13 Feb 2010 16:04:58 +0000 Subject: [PATCH] Fix drbr and altq interaction: - introduce drbr_needs_enqueue that returns whether the interface/br needs an enqueue operation: returns true if altq is enabled or there are already packets in the ring (as we need to maintain packet order) - update all drbr consumers - fix drbr_flush - avoid using the driver queue (IFQ_DRV_*) in the altq case as the multiqueue consumer does not provide enough protection, serialize altq interaction with the main queue lock - make drbr_dequeue_cond work with altq Discussed with: kmacy, yongari, jfv MFC after: 4 weeks --- sys/dev/cxgb/cxgb_sge.c | 4 +++- sys/dev/e1000/if_em.c | 2 +- sys/dev/e1000/if_igb.c | 8 ++++++-- sys/dev/ixgbe/ixgbe.c | 2 +- sys/dev/mxge/if_mxge.c | 2 +- sys/net/if_var.h | 36 +++++++++++++++++++++++------------- 6 files changed, 35 insertions(+), 19 deletions(-) diff --git a/sys/dev/cxgb/cxgb_sge.c b/sys/dev/cxgb/cxgb_sge.c index 88478b261a37..27a7c899209a 100644 --- a/sys/dev/cxgb/cxgb_sge.c +++ b/sys/dev/cxgb/cxgb_sge.c @@ -228,6 +228,8 @@ static uint8_t flit_desc_map[] = { #define TXQ_LOCK(qs) mtx_lock(&(qs)->lock) #define TXQ_UNLOCK(qs) mtx_unlock(&(qs)->lock) #define TXQ_RING_EMPTY(qs) drbr_empty((qs)->port->ifp, (qs)->txq[TXQ_ETH].txq_mr) +#define TXQ_RING_NEEDS_ENQUEUE(qs) \ + drbr_needs_enqueue((qs)->port->ifp, (qs)->txq[TXQ_ETH].txq_mr) #define TXQ_RING_FLUSH(qs) drbr_flush((qs)->port->ifp, (qs)->txq[TXQ_ETH].txq_mr) #define TXQ_RING_DEQUEUE_COND(qs, func, arg) \ drbr_dequeue_cond((qs)->port->ifp, (qs)->txq[TXQ_ETH].txq_mr, func, arg) @@ -1712,7 +1714,7 @@ cxgb_transmit_locked(struct ifnet *ifp, struct sge_qset *qs, struct mbuf *m) * - there is space in hardware transmit queue */ if (check_pkt_coalesce(qs) == 0 && - TXQ_RING_EMPTY(qs) && avail > 4) { + !TXQ_RING_NEEDS_ENQUEUE(qs) && avail > 4) { if (t3_encap(qs, &m)) { if (m != NULL && (error = drbr_enqueue(ifp, br, m)) != 0) diff --git a/sys/dev/e1000/if_em.c b/sys/dev/e1000/if_em.c index 546b453c687c..b45e4784014d 100644 --- a/sys/dev/e1000/if_em.c +++ b/sys/dev/e1000/if_em.c @@ -955,7 +955,7 @@ 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) && + } else if (!drbr_needs_enqueue(ifp, adapter->br) && (adapter->num_tx_desc_avail > EM_TX_OP_THRESHOLD)) { if ((error = em_xmit(adapter, &m)) != 0) { if (m) diff --git a/sys/dev/e1000/if_igb.c b/sys/dev/e1000/if_igb.c index 9fe113c76c47..2eb113aa4d7c 100644 --- a/sys/dev/e1000/if_igb.c +++ b/sys/dev/e1000/if_igb.c @@ -842,9 +842,13 @@ igb_mq_start_locked(struct ifnet *ifp, struct tx_ring *txr, struct mbuf *m) } enq = 0; - if (m == NULL) + if (m == NULL) { next = drbr_dequeue(ifp, txr->br); - else + } else if (drbr_needs_enqueue(ifp, txr->br)) { + if ((err = drbr_enqueue(ifp, txr->br, m)) != 0) + return (err); + next = drbr_dequeue(ifp, txr->br); + } else next = m; /* Process the queue */ while (next != NULL) { diff --git a/sys/dev/ixgbe/ixgbe.c b/sys/dev/ixgbe/ixgbe.c index bde8b94084a4..f66443ae77f8 100644 --- a/sys/dev/ixgbe/ixgbe.c +++ b/sys/dev/ixgbe/ixgbe.c @@ -861,7 +861,7 @@ ixgbe_mq_start_locked(struct ifnet *ifp, struct tx_ring *txr, struct mbuf *m) goto process; /* If nothing queued go right to xmit */ - if (drbr_empty(ifp, txr->br)) { + if (!drbr_needs_enqueue(ifp, txr->br)) { if ((err = ixgbe_xmit(txr, &m)) != 0) { if (m != NULL) err = drbr_enqueue(ifp, txr->br, m); diff --git a/sys/dev/mxge/if_mxge.c b/sys/dev/mxge/if_mxge.c index 219c506844e1..6dcba391e9bc 100644 --- a/sys/dev/mxge/if_mxge.c +++ b/sys/dev/mxge/if_mxge.c @@ -2249,7 +2249,7 @@ mxge_transmit_locked(struct mxge_slice_state *ss, struct mbuf *m) return (err); } - if (drbr_empty(ifp, tx->br) && + if (!drbr_needs_enqueue(ifp, tx->br) && ((tx->mask - (tx->req - tx->done)) > tx->max_desc)) { /* let BPF see it */ BPF_MTAP(ifp, m); diff --git a/sys/net/if_var.h b/sys/net/if_var.h index 37ebea5b484f..4127069641a4 100644 --- a/sys/net/if_var.h +++ b/sys/net/if_var.h @@ -603,12 +603,8 @@ 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_PURGE(&ifp->if_snd); #endif while ((m = buf_ring_dequeue_sc(br)) != NULL) m_freem(m); @@ -629,7 +625,7 @@ drbr_dequeue(struct ifnet *ifp, struct buf_ring *br) struct mbuf *m; if (ALTQ_IS_ENABLED(&ifp->if_snd)) { - IFQ_DRV_DEQUEUE(&ifp->if_snd, m); + IFQ_DEQUEUE(&ifp->if_snd, m); return (m); } #endif @@ -642,11 +638,15 @@ 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 (ALTQ_IS_ENABLED(&ifp->if_snd)) { + IFQ_LOCK(&ifp->if_snd); + IFQ_POLL_NOLOCK(&ifp->if_snd, m); + if (m != NULL && func(m, arg) == 0) { + IFQ_UNLOCK(&ifp->if_snd); + return (NULL); + } + IFQ_DEQUEUE(&ifp->if_snd, m); + IFQ_UNLOCK(&ifp->if_snd); return (m); } #endif @@ -662,11 +662,21 @@ drbr_empty(struct ifnet *ifp, struct buf_ring *br) { #ifdef ALTQ if (ALTQ_IS_ENABLED(&ifp->if_snd)) - return (IFQ_DRV_IS_EMPTY(&ifp->if_snd)); + return (IFQ_IS_EMPTY(&ifp->if_snd)); #endif return (buf_ring_empty(br)); } +static __inline int +drbr_needs_enqueue(struct ifnet *ifp, struct buf_ring *br) +{ +#ifdef ALTQ + if (ALTQ_IS_ENABLED(&ifp->if_snd)) + return (1); +#endif + return (!buf_ring_empty(br)); +} + static __inline int drbr_inuse(struct ifnet *ifp, struct buf_ring *br) {