From e136c2407c03ca4a4436c62e5ef915bec7c0c0b8 Mon Sep 17 00:00:00 2001
From: Sepherosa Ziehau <sephe@dragonflybsd.org>
Date: Mon, 17 Dec 2012 12:18:08 +0800
Subject: [PATCH] if_start: Fix a race that could delay the packets
 transmission

Since if_start_need_schedule is called w/ the cached IFF_OACTIVE out
side of ifnet's TX serializer, there could be a race that IFF_OACTIVE
could be cleared before if_start_need_schedule but after releasing
ifnet's TX serializer.  This could delay already queued packets
transmission until the new packet is coming.  Fix this race by calling
if_start_need_schedule inside ifnet's TX serializer.
---
 sys/net/if.c | 51 ++++++++++++++++++++++-----------------------------
 1 file changed, 22 insertions(+), 29 deletions(-)

diff --git a/sys/net/if.c b/sys/net/if.c
index c1ee3847e3..65fcc28d7e 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -311,7 +311,7 @@ if_start_dispatch(netmsg_t msg)
 	struct lwkt_msg *lmsg = &msg->base.lmsg;
 	struct ifnet *ifp = lmsg->u.ms_resultp;
 	struct ifaltq *ifq = &ifp->if_snd;
-	int running = 0;
+	int running = 0, need_sched;
 
 	crit_enter();
 	lwkt_replymsg(lmsg, 0);	/* reply ASAP */
@@ -320,34 +320,26 @@ if_start_dispatch(netmsg_t msg)
 #ifdef SMP
 	if (mycpuid != ifp->if_start_cpuid(ifp)) {
 		/*
-		 * If the ifnet is still up, we need to
-		 * chase its CPU change.
+		 * We need to chase the ifnet CPU change.
 		 */
-		if (ifp->if_flags & IFF_UP) {
-			logifstart(chase_sched, ifp);
-			if_start_schedule(ifp);
-			return;
-		} else {
-			goto check;
-		}
+		logifstart(chase_sched, ifp);
+		if_start_schedule(ifp);
+		return;
 	}
 #endif
 
-	if (ifp->if_flags & IFF_UP) {
-		ifnet_serialize_tx(ifp); /* XXX try? */
-		if ((ifp->if_flags & IFF_OACTIVE) == 0) {
-			logifstart(run, ifp);
-			ifp->if_start(ifp);
-			if ((ifp->if_flags &
-			(IFF_OACTIVE | IFF_RUNNING)) == IFF_RUNNING)
-				running = 1;
-		}
-		ifnet_deserialize_tx(ifp);
+	ifnet_serialize_tx(ifp);
+	if ((ifp->if_flags & (IFF_OACTIVE | IFF_RUNNING)) == IFF_RUNNING) {
+		logifstart(run, ifp);
+		ifp->if_start(ifp);
+		if ((ifp->if_flags & (IFF_OACTIVE | IFF_RUNNING)) ==
+		    IFF_RUNNING)
+			running = 1;
 	}
-#ifdef SMP
-check:
-#endif
-	if (if_start_need_schedule(ifq, running)) {
+	need_sched = if_start_need_schedule(ifq, running);
+	ifnet_deserialize_tx(ifp);
+
+	if (need_sched) {
 		crit_enter();
 		if (lmsg->ms_flags & MSGF_DONE)	{ /* XXX necessary? */
 			logifstart(sched, ifp);
@@ -2444,7 +2436,7 @@ int
 ifq_dispatch(struct ifnet *ifp, struct mbuf *m, struct altq_pktattr *pa)
 {
 	struct ifaltq *ifq = &ifp->if_snd;
-	int running = 0, error, start = 0;
+	int running = 0, error, start = 0, need_sched;
 
 	ASSERT_IFNET_NOT_SERIALIZED_TX(ifp);
 
@@ -2488,17 +2480,18 @@ ifq_dispatch(struct ifnet *ifp, struct mbuf *m, struct altq_pktattr *pa)
 		return 0;
 	}
 
-	if ((ifp->if_flags & IFF_OACTIVE) == 0) {
+	if ((ifp->if_flags & (IFF_OACTIVE | IFF_RUNNING)) == IFF_RUNNING) {
 		logifstart(run, ifp);
 		ifp->if_start(ifp);
-		if ((ifp->if_flags &
-		     (IFF_OACTIVE | IFF_RUNNING)) == IFF_RUNNING)
+		if ((ifp->if_flags & (IFF_OACTIVE | IFF_RUNNING)) ==
+		    IFF_RUNNING)
 			running = 1;
 	}
+	need_sched = if_start_need_schedule(ifq, running);
 
 	ifnet_deserialize_tx(ifp);
 
-	if (if_start_need_schedule(ifq, running)) {
+	if (need_sched) {
 		/*
 		 * More data need to be transmitted, ifnet.if_start is
 		 * scheduled on ifnet's CPU, and we keep going.
-- 
2.41.0