svn commit: r259642 - head/sys/netinet

[Adrian Chadd]

Author: adrian
Date: Fri Dec 20 07:41:03 2013
New Revision: 259642
URL: http://svnweb.freebsd.org/changeset/base/259642

Log:
  Disable the now unpredicably bogus check for whether we have
  eneough queue space before queuing a bunch of IP fragments.
  
  As the comment in the committed change says, in the post-if_transmit(),
  post-SMP, post-preemption world, there's just too much overlapping
  concurrent code paths and different approaches to driver transmit
  queue management to have this code even remotely be effective.
  
  The only specific place it could be useful is if ALTQ is enabled
  but again it doesn't at all promise that all the fragments will be
  transmitted anyway.
  
  The main reason for committing this change is to disable a parallel
  place where the drops counter is incremented.  This is a side effect
  of an upcoming change to ixgbe/cxgbe to handle the queue drops
  counter slightly better.
  
  Sponsored by:	Netflix, Inc.

Modified:
  head/sys/netinet/ip_output.c

Modified: head/sys/netinet/ip_output.c
==============================================================================
--- head/sys/netinet/ip_output.c	Fri Dec 20 05:50:22 2013	(r259641)
+++ head/sys/netinet/ip_output.c	Fri Dec 20 07:41:03 2013	(r259642)
@@ -123,7 +123,9 @@ ip_output(struct mbuf *m, struct mbuf *o
 	struct mbuf *m0;
 	int hlen = sizeof (struct ip);
 	int mtu;
+#if 0
 	int n;	/* scratchpad */
+#endif
 	int error = 0;
 	struct sockaddr_in *dst;
 	const struct sockaddr_in *gw;
@@ -431,6 +433,25 @@ again:
 	}
 
 	/*
+	 * Both in the SMP world, pre-emption world if_transmit() world,
+	 * the following code doesn't really function as intended any further.
+	 *
+	 * + There can and will be multiple CPUs running this code path
+	 *   in parallel, and we do no lock holding when checking the
+	 *   queue depth;
+	 * + And since other threads can be running concurrently, even if
+	 *   we do pass this check, another thread may queue some frames
+	 *   before this thread does and it will end up partially or fully
+	 *   failing to send anyway;
+	 * + if_transmit() based drivers don't necessarily set ifq_len
+	 *   at all.
+	 *
+	 * This should be replaced with a method of pushing an entire list
+	 * of fragment frames to the driver and have the driver decide
+	 * whether it can queue or not queue the entire set.
+	 */
+#if 0
+	/*
 	 * Verify that we have any chance at all of being able to queue the
 	 * packet or packet fragments, unless ALTQ is enabled on the given
 	 * interface in which case packetdrop should be done by queueing.
@@ -446,6 +467,7 @@ again:
 		ifp->if_snd.ifq_drops += n;
 		goto bad;
 	}
+#endif
 
 	/*
 	 * Look for broadcast address and