Skip site navigation (1)Skip section navigation (2) 
Date:      Mon, 26 Nov 2012 20:03:57 +0000 (UTC)
From:      Gleb Smirnoff <glebius@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r243570 - head/sys/dev/e1000
Message-ID:  <201211262003.qAQK3vsU063354@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: glebius
Date: Mon Nov 26 20:03:57 2012
New Revision: 243570
URL: http://svnweb.freebsd.org/changeset/base/243570

Log:
    drbr_enqueue() awlays consumes mbuf, no matter did it
  fail or not. The mbuf pointer is no longer valid, so
  can't be reused after.
  
    Fix igb_mq_start() where mbuf pointer was used after
  drbr_enqueue().
  
    This eventually leads us to all invocations of
  igb_mq_start_locked() called with third argument as NULL.
  This allows us to simplify this function.
  
  Submitted by:	Karim Fodil-Lemelin <fodillemlinkarim gmail.com>
  Reviewed by:	jfv

Modified:
  head/sys/dev/e1000/if_igb.c

Modified: head/sys/dev/e1000/if_igb.c
==============================================================================
--- head/sys/dev/e1000/if_igb.c	Mon Nov 26 19:45:01 2012	(r243569)
+++ head/sys/dev/e1000/if_igb.c	Mon Nov 26 20:03:57 2012	(r243570)
@@ -181,8 +181,7 @@ static int	igb_suspend(device_t);
 static int	igb_resume(device_t);
 #if __FreeBSD_version >= 800000
 static int	igb_mq_start(struct ifnet *, struct mbuf *);
-static int	igb_mq_start_locked(struct ifnet *,
-		    struct tx_ring *, struct mbuf *);
+static int	igb_mq_start_locked(struct ifnet *, struct tx_ring *);
 static void	igb_qflush(struct ifnet *);
 static void	igb_deferred_mq_start(void *, int);
 #else
@@ -845,7 +844,7 @@ igb_resume(device_t dev)
 			/* Process the stack queue only if not depleted */
 			if (((txr->queue_status & IGB_QUEUE_DEPLETED) == 0) &&
 			    !drbr_empty(ifp, txr->br))
-				igb_mq_start_locked(ifp, txr, NULL);
+				igb_mq_start_locked(ifp, txr);
 #else
 			if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd))
 				igb_start_locked(txr, ifp);
@@ -961,15 +960,14 @@ igb_mq_start(struct ifnet *ifp, struct m
 	que = &adapter->queues[i];
 	if (((txr->queue_status & IGB_QUEUE_DEPLETED) == 0) &&
 	    IGB_TX_TRYLOCK(txr)) {
-		struct mbuf *pm = NULL;
 		/*
 		** Try to queue first to avoid
 		** out-of-order delivery, but 
 		** settle for it if that fails
 		*/
-		if (m && drbr_enqueue(ifp, txr->br, m))
-			pm = m;
-		err = igb_mq_start_locked(ifp, txr, pm);
+		if (m)
+			drbr_enqueue(ifp, txr->br, m);
+		err = igb_mq_start_locked(ifp, txr);
 		IGB_TX_UNLOCK(txr);
 	} else {
 		err = drbr_enqueue(ifp, txr->br, m);
@@ -980,7 +978,7 @@ igb_mq_start(struct ifnet *ifp, struct m
 }
 
 static int
-igb_mq_start_locked(struct ifnet *ifp, struct tx_ring *txr, struct mbuf *m)
+igb_mq_start_locked(struct ifnet *ifp, struct tx_ring *txr)
 {
 	struct adapter  *adapter = txr->adapter;
         struct mbuf     *next;
@@ -990,24 +988,13 @@ igb_mq_start_locked(struct ifnet *ifp, s
 
 	if (((ifp->if_drv_flags & IFF_DRV_RUNNING) == 0) ||
 	    (txr->queue_status & IGB_QUEUE_DEPLETED) ||
-	    adapter->link_active == 0) {
-		if (m != NULL)
-			err = drbr_enqueue(ifp, txr->br, m);
+	    adapter->link_active == 0)
 		return (err);
-	}
 
 	enq = 0;
-	if (m == NULL) {
-		next = drbr_dequeue(ifp, txr->br);
-	} 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) {
+	while ((next = drbr_dequeue(ifp, txr->br)) != NULL) {
 		if ((err = igb_xmit(txr, &next)) != 0) {
 			if (next != NULL)
 				err = drbr_enqueue(ifp, txr->br, next);
@@ -1020,7 +1007,6 @@ igb_mq_start_locked(struct ifnet *ifp, s
 		ETHER_BPF_MTAP(ifp, next);
 		if ((ifp->if_drv_flags & IFF_DRV_RUNNING) == 0)
 			break;
-		next = drbr_dequeue(ifp, txr->br);
 	}
 	if (enq > 0) {
 		/* Set the watchdog */
@@ -1046,7 +1032,7 @@ igb_deferred_mq_start(void *arg, int pen
 
 	IGB_TX_LOCK(txr);
 	if (!drbr_empty(ifp, txr->br))
-		igb_mq_start_locked(ifp, txr, NULL);
+		igb_mq_start_locked(ifp, txr);
 	IGB_TX_UNLOCK(txr);
 }
 
@@ -1398,7 +1384,7 @@ igb_handle_que(void *context, int pendin
 		/* Process the stack queue only if not depleted */
 		if (((txr->queue_status & IGB_QUEUE_DEPLETED) == 0) &&
 		    !drbr_empty(ifp, txr->br))
-			igb_mq_start_locked(ifp, txr, NULL);
+			igb_mq_start_locked(ifp, txr);
 #else
 		if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd))
 			igb_start_locked(txr, ifp);
@@ -1449,7 +1435,7 @@ igb_handle_link_locked(struct adapter *a
 			/* Process the stack queue only if not depleted */
 			if (((txr->queue_status & IGB_QUEUE_DEPLETED) == 0) &&
 			    !drbr_empty(ifp, txr->br))
-				igb_mq_start_locked(ifp, txr, NULL);
+				igb_mq_start_locked(ifp, txr);
 #else
 			if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd))
 				igb_start_locked(txr, ifp);
@@ -1549,7 +1535,7 @@ igb_poll(struct ifnet *ifp, enum poll_cm
 		} while (loop-- && more);
 #if __FreeBSD_version >= 800000
 		if (!drbr_empty(ifp, txr->br))
-			igb_mq_start_locked(ifp, txr, NULL);
+			igb_mq_start_locked(ifp, txr);
 #else
 		if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd))
 			igb_start_locked(txr, ifp);
@@ -1586,7 +1572,7 @@ igb_msix_que(void *arg)
 	/* Process the stack queue only if not depleted */
 	if (((txr->queue_status & IGB_QUEUE_DEPLETED) == 0) &&
 	    !drbr_empty(ifp, txr->br))
-		igb_mq_start_locked(ifp, txr, NULL);
+		igb_mq_start_locked(ifp, txr);
 #else
 	if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd))
 		igb_start_locked(txr, ifp);

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201211262003.qAQK3vsU063354>