net: Optimize hard_start_xmit() return checking

Recent changes in the TX error propagation require additional checking and masking of values returned from hard_start_xmit(), mainly to separate cases where skb was consumed. This aim can be simplified by changing the order of NETDEV_TX and NET_XMIT codes, because the latter are treated similarly to negative (ERRNO) values. After this change much simpler dev_xmit_complete() is also used in sch_direct_xmit(), so it is moved to netdevice.h. Additionally NET_RX definitions in netdevice.h are moved up from between TX codes to avoid confusion while reading the TX comment. Signed-off-by: Jarek Poplawski <jarkao2@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net>

net: Optimize hard_start_xmit() return checking
Recent changes in the TX error propagation require additional checking and masking of values returned from hard_start_xmit(), mainly to separate cases where skb was consumed. This aim can be simplified by changing the order of NETDEV_TX and NET_XMIT codes, because the latter are treated similarly to negative (ERRNO) values. After this change much simpler dev_xmit_complete() is also used in sch_direct_xmit(), so it is moved to netdevice.h. Additionally NET_RX definitions in netdevice.h are moved up from between TX codes to avoid confusion while reading the TX comment. Signed-off-by: Jarek Poplawski <jarkao2@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net>
Jarek Poplawski · David S. Miller
1 parent cb43e23435
Showing 3 changed files with 35 additions and 47 deletions Side-by-side Diff
include/linux/netdevice.h
net/core/dev.c
net/sched/sch_generic.c
@@ -63,6 +63,10 @@
 #define HAVE_FREE_NETDEV		/* free_netdev() */
 #define HAVE_NETDEV_PRIV		/* netdev_priv() */
  
+/* Backlog congestion levels */
+#define NET_RX_SUCCESS		0	/* keep 'em coming, baby */
+#define NET_RX_DROP		1	/* packet dropped */
+
 /*
  * Transmit return codes: transmit return codes originate from three different
  * namespaces:
  
@@ -82,15 +86,11 @@
  
 /* qdisc ->enqueue() return codes. */
 #define NET_XMIT_SUCCESS	0x00
-#define NET_XMIT_DROP		0x10	/* skb dropped			*/
-#define NET_XMIT_CN		0x20	/* congestion notification	*/
-#define NET_XMIT_POLICED	0x30	/* skb is shot by police	*/
-#define NET_XMIT_MASK		0xf0	/* qdisc flags in net/sch_generic.h */
+#define NET_XMIT_DROP		0x01	/* skb dropped			*/
+#define NET_XMIT_CN		0x02	/* congestion notification	*/
+#define NET_XMIT_POLICED	0x03	/* skb is shot by police	*/
+#define NET_XMIT_MASK		0x0f	/* qdisc flags in net/sch_generic.h */
  
-/* Backlog congestion levels */
-#define NET_RX_SUCCESS		0	/* keep 'em coming, baby */
-#define NET_RX_DROP		1	/* packet dropped */
-
 /* NET_XMIT_CN is special. It does not guarantee that this packet is lost. It
  * indicates that the device will soon be dropping packets, or already drops
  * some packets of the same priority; prompting us to send less aggressively. */
  
  
@@ -98,15 +98,33 @@
 #define net_xmit_errno(e)	((e) != NET_XMIT_CN ? -ENOBUFS : 0)
  
 /* Driver transmit return codes */
-#define NETDEV_TX_MASK		0xf
+#define NETDEV_TX_MASK		0xf0
  
 enum netdev_tx {
 	__NETDEV_TX_MIN	 = INT_MIN,	/* make sure enum is signed */
-	NETDEV_TX_OK	 = 0,		/* driver took care of packet */
-	NETDEV_TX_BUSY	 = 1,		/* driver tx path was busy*/
-	NETDEV_TX_LOCKED = 2,		/* driver tx lock was already taken */
+	NETDEV_TX_OK	 = 0x00,	/* driver took care of packet */
+	NETDEV_TX_BUSY	 = 0x10,	/* driver tx path was busy*/
+	NETDEV_TX_LOCKED = 0x20,	/* driver tx lock was already taken */
 };
 typedef enum netdev_tx netdev_tx_t;
+
+/*
+ * Current order: NETDEV_TX_MASK > NET_XMIT_MASK >= 0 is significant;
+ * hard_start_xmit() return < NET_XMIT_MASK means skb was consumed.
+ */
+static inline bool dev_xmit_complete(int rc)
+{
+	/*
+	 * Positive cases with an skb consumed by a driver:
+	 * - successful transmission (rc == NETDEV_TX_OK)
+	 * - error while transmitting (rc < 0)
+	 * - error while queueing to a different device (rc & NET_XMIT_MASK)
+	 */
+	if (likely(rc < NET_XMIT_MASK))
+		return true;
+
+	return false;
+}
  
 #endif
  
@@ -1924,23 +1924,6 @@
 	return rc;
 }
  
-static inline bool dev_xmit_complete(int rc)
-{
-	/* successful transmission */
-	if (rc == NETDEV_TX_OK)
-		return true;
-
-	/* error while transmitting, driver consumed skb */
-	if (rc < 0)
-		return true;
-
-	/* error while queueing to a different device, driver consumed skb */
-	if (rc & NET_XMIT_MASK)
-		return true;
-
-	return false;
-}
-
 /**
  *	dev_queue_xmit - transmit a buffer
  *	@skb: buffer to transmit
@@ -119,39 +119,26 @@
 	spin_unlock(root_lock);
  
 	HARD_TX_LOCK(dev, txq, smp_processor_id());
-	if (!netif_tx_queue_stopped(txq) &&
-	    !netif_tx_queue_frozen(txq)) {
+	if (!netif_tx_queue_stopped(txq) && !netif_tx_queue_frozen(txq))
 		ret = dev_hard_start_xmit(skb, dev, txq);
  
-		/* an error implies that the skb was consumed */
-		if (ret < 0)
-			ret = NETDEV_TX_OK;
-		/* all NET_XMIT codes map to NETDEV_TX_OK */
-		ret &= ~NET_XMIT_MASK;
-	}
 	HARD_TX_UNLOCK(dev, txq);
  
 	spin_lock(root_lock);
  
-	switch (ret) {
-	case NETDEV_TX_OK:
-		/* Driver sent out skb successfully */
+	if (dev_xmit_complete(ret)) {
+		/* Driver sent out skb successfully or skb was consumed */
 		ret = qdisc_qlen(q);
-		break;
-
-	case NETDEV_TX_LOCKED:
+	} else if (ret == NETDEV_TX_LOCKED) {
 		/* Driver try lock failed */
 		ret = handle_dev_cpu_collision(skb, txq, q);
-		break;
-
-	default:
+	} else {
 		/* Driver returned NETDEV_TX_BUSY - requeue skb */
 		if (unlikely (ret != NETDEV_TX_BUSY && net_ratelimit()))
 			printk(KERN_WARNING "BUG %s code %d qlen %d\n",
 			       dev->name, ret, q->q.qlen);
  
 		ret = dev_requeue_skb(skb, q);
-		break;
 	}
  
 	if (ret && (netif_tx_queue_stopped(txq) ||
...	...	@@ -63,6 +63,10 @@
63	63	#define HAVE_FREE_NETDEV /* free_netdev() */
64	64	#define HAVE_NETDEV_PRIV /* netdev_priv() */
65	65
	66	+/* Backlog congestion levels */
	67	+#define NET_RX_SUCCESS 0 /* keep 'em coming, baby */
	68	+#define NET_RX_DROP 1 /* packet dropped */
	69	+
66	70	/*
67	71	* Transmit return codes: transmit return codes originate from three different
68	72	* namespaces:
69	73
...	...	@@ -82,15 +86,11 @@
82	86
83	87	/* qdisc ->enqueue() return codes. */
84	88	#define NET_XMIT_SUCCESS 0x00
85		-#define NET_XMIT_DROP 0x10 /* skb dropped */
86		-#define NET_XMIT_CN 0x20 /* congestion notification */
87		-#define NET_XMIT_POLICED 0x30 /* skb is shot by police */
88		-#define NET_XMIT_MASK 0xf0 /* qdisc flags in net/sch_generic.h */
	89	+#define NET_XMIT_DROP 0x01 /* skb dropped */
	90	+#define NET_XMIT_CN 0x02 /* congestion notification */
	91	+#define NET_XMIT_POLICED 0x03 /* skb is shot by police */
	92	+#define NET_XMIT_MASK 0x0f /* qdisc flags in net/sch_generic.h */
89	93
90		-/* Backlog congestion levels */
91		-#define NET_RX_SUCCESS 0 /* keep 'em coming, baby */
92		-#define NET_RX_DROP 1 /* packet dropped */
93		-
94	94	/* NET_XMIT_CN is special. It does not guarantee that this packet is lost. It
95	95	* indicates that the device will soon be dropping packets, or already drops
96	96	* some packets of the same priority; prompting us to send less aggressively. */
97	97
98	98
...	...	@@ -98,15 +98,33 @@
98	98	#define net_xmit_errno(e) ((e) != NET_XMIT_CN ? -ENOBUFS : 0)
99	99
100	100	/* Driver transmit return codes */
101		-#define NETDEV_TX_MASK 0xf
	101	+#define NETDEV_TX_MASK 0xf0
102	102
103	103	enum netdev_tx {
104	104	__NETDEV_TX_MIN = INT_MIN, /* make sure enum is signed */
105		- NETDEV_TX_OK = 0, /* driver took care of packet */
106		- NETDEV_TX_BUSY = 1, /* driver tx path was busy*/
107		- NETDEV_TX_LOCKED = 2, /* driver tx lock was already taken */
	105	+ NETDEV_TX_OK = 0x00, /* driver took care of packet */
	106	+ NETDEV_TX_BUSY = 0x10, /* driver tx path was busy*/
	107	+ NETDEV_TX_LOCKED = 0x20, /* driver tx lock was already taken */
108	108	};
109	109	typedef enum netdev_tx netdev_tx_t;
	110	+
	111	+/*
	112	+ * Current order: NETDEV_TX_MASK > NET_XMIT_MASK >= 0 is significant;
	113	+ * hard_start_xmit() return < NET_XMIT_MASK means skb was consumed.
	114	+ */
	115	+static inline bool dev_xmit_complete(int rc)
	116	+{
	117	+ /*
	118	+ * Positive cases with an skb consumed by a driver:
	119	+ * - successful transmission (rc == NETDEV_TX_OK)
	120	+ * - error while transmitting (rc < 0)
	121	+ * - error while queueing to a different device (rc & NET_XMIT_MASK)
	122	+ */
	123	+ if (likely(rc < NET_XMIT_MASK))
	124	+ return true;
	125	+
	126	+ return false;
	127	+}
110	128
111	129	#endif
112	130
...	...	@@ -1924,23 +1924,6 @@
1924	1924	return rc;
1925	1925	}
1926	1926
1927		-static inline bool dev_xmit_complete(int rc)
1928		-{
1929		- /* successful transmission */
1930		- if (rc == NETDEV_TX_OK)
1931		- return true;
1932		-
1933		- /* error while transmitting, driver consumed skb */
1934		- if (rc < 0)
1935		- return true;
1936		-
1937		- /* error while queueing to a different device, driver consumed skb */
1938		- if (rc & NET_XMIT_MASK)
1939		- return true;
1940		-
1941		- return false;
1942		-}
1943		-
1944	1927	/**
1945	1928	* dev_queue_xmit - transmit a buffer
1946	1929	* @skb: buffer to transmit
...	...	@@ -119,39 +119,26 @@
119	119	spin_unlock(root_lock);
120	120
121	121	HARD_TX_LOCK(dev, txq, smp_processor_id());
122		- if (!netif_tx_queue_stopped(txq) &&
123		- !netif_tx_queue_frozen(txq)) {
	122	+ if (!netif_tx_queue_stopped(txq) && !netif_tx_queue_frozen(txq))
124	123	ret = dev_hard_start_xmit(skb, dev, txq);
125	124
126		- /* an error implies that the skb was consumed */
127		- if (ret < 0)
128		- ret = NETDEV_TX_OK;
129		- /* all NET_XMIT codes map to NETDEV_TX_OK */
130		- ret &= ~NET_XMIT_MASK;
131		- }
132	125	HARD_TX_UNLOCK(dev, txq);
133	126
134	127	spin_lock(root_lock);
135	128
136		- switch (ret) {
137		- case NETDEV_TX_OK:
138		- /* Driver sent out skb successfully */
	129	+ if (dev_xmit_complete(ret)) {
	130	+ /* Driver sent out skb successfully or skb was consumed */
139	131	ret = qdisc_qlen(q);
140		- break;
141		-
142		- case NETDEV_TX_LOCKED:
	132	+ } else if (ret == NETDEV_TX_LOCKED) {
143	133	/* Driver try lock failed */
144	134	ret = handle_dev_cpu_collision(skb, txq, q);
145		- break;
146		-
147		- default:
	135	+ } else {
148	136	/* Driver returned NETDEV_TX_BUSY - requeue skb */
149	137	if (unlikely (ret != NETDEV_TX_BUSY && net_ratelimit()))
150	138	printk(KERN_WARNING "BUG %s code %d qlen %d\n",
151	139	dev->name, ret, q->q.qlen);
152	140
153	141	ret = dev_requeue_skb(skb, q);
154		- break;
155	142	}
156	143
157	144	if (ret && (netif_tx_queue_stopped(txq) \|\|