[RFC net 00/03]: dev_queue_xmit error propagation

[RFC net 00/03]: dev_queue_xmit error propagation

[Patrick McHardy]

Currently the ->ndo_hard_start_xmit() callbacks are only permitted to return
one of the NETDEV_TX codes. This prevents any kind of error propagation for
virtual devices, like queue congestion of the underlying device in case of
layered devices, or unreachability in case of tunnels.

These patches change the return conventions so hard_start_xmit() can return
a NETDEV_TX code, an errno code or a NET_XMIT code. This means it can
additionally to the NETDEV_TX codes simply return the value of dev_queue_xmit().

When queueing is used, errors can't be propagated back since transmission is
asynchronously and all non-NETDEV codes are mapped to NETDEV_TX_OK, reflecting
the fact that the skb is consumed unconditionally.

In case of virtual devices not using queueing, the NETDEV_TX codes are consumed
by dev_queue_xmit(), everything else is returned to the higher layers.

The end result is that virtual network devices can propagate queue congestion
state of the underlying device, or other errors, back to the socket layer.
One immediately useful effect is that TCP can make use of this information
as is currently done with non-virtual devices. But it also seems useful in case
of tunnels, which can return more meaningful errors, or for statistics.

Comments welcome. A review of my changes in dev_hard_start_xmit() would be
especially appreciated :)


 include/linux/netdevice.h |   39 +++++++++++++++++++++++++++++----------
 net/8021q/vlan_dev.c      |   29 ++++++++++++++++++++++-------
 net/core/dev.c            |    7 +++++--
 net/sched/sch_atm.c       |    4 ++--
 net/sched/sch_generic.c   |   10 +++++++++-
 net/sched/sch_sfq.c       |    2 +-
 net/sched/sch_tbf.c       |    4 ++--
 net/sched/sch_teql.c      |    2 +-
 8 files changed, 71 insertions(+), 26 deletions(-)

Patrick McHardy (3):
      net-sched: use symbolic NET_XMIT constants in qdiscs
      net: allow to propagate errors through ->ndo_hard_start_xmit()
      vlan: propagate transmission state

[RFC net-sched 01/03]: use symbolic NET_XMIT constants in qdiscs

[Patrick McHardy]

commit 49ffbac7a80175238e550479c44e1e897bc56911
Author: Patrick McHardy <kaber@trash.net>
Date:   Tue Jun 9 17:55:06 2009 +0200

    net-sched: use symbolic NET_XMIT constants in qdiscs
    
    Convert the remaining spots using raw values to use the symbolic NET_XMIT constants.
    
    Signed-off-by: Patrick McHardy <kaber@trash.net>

diff --git a/net/sched/sch_atm.c b/net/sched/sch_atm.c
index 2a8b83a..15f06ce 100644
--- a/net/sched/sch_atm.c
+++ b/net/sched/sch_atm.c
@@ -431,7 +431,7 @@ static int atm_tc_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 	}
 
 	ret = qdisc_enqueue(skb, flow->q);
-	if (ret != 0) {
+	if (ret != NET_XMIT_SUCCESS) {
 drop: __maybe_unused
 		if (net_xmit_drop_count(ret)) {
 			sch->qstats.drops++;
@@ -455,7 +455,7 @@ drop: __maybe_unused
 	 */
 	if (flow == &p->link) {
 		sch->q.qlen++;
-		return 0;
+		return NET_XMIT_SUCCESS;
 	}
 	tasklet_schedule(&p->task);
 	return NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index 8706920..b994569 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -322,7 +322,7 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 	if (++sch->q.qlen <= q->limit) {
 		sch->bstats.bytes += qdisc_pkt_len(skb);
 		sch->bstats.packets++;
-		return 0;
+		return NET_XMIT_SUCCESS;
 	}
 
 	sfq_drop(sch);
diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
index e22dfe8..407fa57 100644
--- a/net/sched/sch_tbf.c
+++ b/net/sched/sch_tbf.c
@@ -127,7 +127,7 @@ static int tbf_enqueue(struct sk_buff *skb, struct Qdisc* sch)
 		return qdisc_reshape_fail(skb, sch);
 
 	ret = qdisc_enqueue(skb, q->qdisc);
-	if (ret != 0) {
+	if (ret != NET_XMIT_SUCCESS) {
 		if (net_xmit_drop_count(ret))
 			sch->qstats.drops++;
 		return ret;
@@ -136,7 +136,7 @@ static int tbf_enqueue(struct sk_buff *skb, struct Qdisc* sch)
 	sch->q.qlen++;
 	sch->bstats.bytes += qdisc_pkt_len(skb);
 	sch->bstats.packets++;
-	return 0;
+	return NET_XMIT_SUCCESS;
 }
 
 static unsigned int tbf_drop(struct Qdisc* sch)
diff --git a/net/sched/sch_teql.c b/net/sched/sch_teql.c
index cb1cb1e..2b4f5ef 100644
--- a/net/sched/sch_teql.c
+++ b/net/sched/sch_teql.c
@@ -84,7 +84,7 @@ teql_enqueue(struct sk_buff *skb, struct Qdisc* sch)
 		__skb_queue_tail(&q->q, skb);
 		sch->bstats.bytes += qdisc_pkt_len(skb);
 		sch->bstats.packets++;
-		return 0;
+		return NET_XMIT_SUCCESS;
 	}
 
 	kfree_skb(skb);

[RFC net 02/03]: allow to propagate errors through ->ndo_hard_start_xmit()

[Patrick McHardy]

commit c2104095c3a1023b4f4809c90092145dff093c88
Author: Patrick McHardy <kaber@trash.net>
Date:   Tue Jun 9 17:55:34 2009 +0200

    net: allow to propagate errors through ->ndo_hard_start_xmit()
    
    Currently the ->ndo_hard_start_xmit() callbacks are only permitted to return
    one of the NETDEV_TX codes. This prevents any kind of error propagation for
    virtual devices, like queue congestion of the underlying device in case of
    layered devices, or unreachability in case of tunnels.
    
    This patches changes the NET_XMIT codes to avoid clashes with the NETDEV_TX
    codes and changes the two callers of dev_hard_start_xmit() to expect either
    errno codes, NET_XMIT codes or NETDEV_TX codes as return value.
    
    In case of qdisc_restart(), all non NETDEV_TX codes are mapped to NETDEV_TX_OK
    since no error propagation is possible when using qdiscs. In case of
    dev_queue_xmit(), the error is propagated upwards.
    
    Signed-off-by: Patrick McHardy <kaber@trash.net>

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 9ea8d6d..ff76c3e 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -63,11 +63,35 @@ struct wireless_dev;
 #define HAVE_FREE_NETDEV		/* free_netdev() */
 #define HAVE_NETDEV_PRIV		/* netdev_priv() */
 
-#define NET_XMIT_SUCCESS	0
-#define NET_XMIT_DROP		1	/* skb dropped			*/
-#define NET_XMIT_CN		2	/* congestion notification	*/
-#define NET_XMIT_POLICED	3	/* skb is shot by police	*/
-#define NET_XMIT_MASK		0xFFFF	/* qdisc flags in net/sch_generic.h */
+/*
+ * Transmit return codes: transmit return codes originate from three different
+ * namespaces:
+ *
+ * - qdisc return codes
+ * - driver transmit return codes
+ * - errno values
+ *
+ * Drivers are allowed to return any one of those in their hard_start_xmit()
+ * function. Real network devices commonly used with qdiscs should only return
+ * the driver transmit return codes though - when qdiscs are used, the actual
+ * transmission happens asynchronously, so the value is not propagated to
+ * higher layers. Virtual network devices transmit synchronously, in this case
+ * the driver transmit return codes are consumed by dev_queue_xmit(), all
+ * others are propagated to higher layers.
+ */
+
+/* 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 */
+
+/* Driver transmit return codes */
+#define NETDEV_TX_OK		0x0	/* driver took care of packet */
+#define NETDEV_TX_BUSY		0x1	/* driver tx path was busy*/
+#define NETDEV_TX_LOCKED	0x2	/* driver tx lock was already taken */
+#define NETDEV_TX_MASK		0xf
 
 /* Backlog congestion levels */
 #define NET_RX_SUCCESS		0   /* keep 'em coming, baby */
@@ -87,11 +111,6 @@ struct wireless_dev;
 
 #define MAX_ADDR_LEN	32		/* Largest hardware address length */
 
-/* Driver transmit return codes */
-#define NETDEV_TX_OK 0		/* driver took care of packet */
-#define NETDEV_TX_BUSY 1	/* driver tx path was busy*/
-#define NETDEV_TX_LOCKED -1	/* driver tx lock was already taken */
-
 #ifdef  __KERNEL__
 
 /*
diff --git a/net/core/dev.c b/net/core/dev.c
index 11560e3..b2e9953 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1731,6 +1731,8 @@ gso:
 		nskb->next = NULL;
 		rc = ops->ndo_start_xmit(nskb, dev);
 		if (unlikely(rc)) {
+			if (rc & ~NETDEV_TX_MASK)
+				return rc;
 			nskb->next = skb->next;
 			skb->next = nskb;
 			return rc;
@@ -1895,8 +1897,9 @@ gso:
 			HARD_TX_LOCK(dev, txq, cpu);
 
 			if (!netif_tx_queue_stopped(txq)) {
-				rc = 0;
-				if (!dev_hard_start_xmit(skb, dev, txq)) {
+				rc = dev_hard_start_xmit(skb, dev, txq);
+				if (rc == NETDEV_TX_OK || rc < 0 ||
+				    rc & NET_XMIT_MASK) {
 					HARD_TX_UNLOCK(dev, txq);
 					goto out;
 				}
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 27d0381..7e3e514 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -143,8 +143,16 @@ static inline int qdisc_restart(struct Qdisc *q)
 
 	HARD_TX_LOCK(dev, txq, smp_processor_id());
 	if (!netif_tx_queue_stopped(txq) &&
-	    !netif_tx_queue_frozen(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);

[RFC vlan 03/03]: propagate transmission state

[Patrick McHardy]

commit b124eaaf852e982c7af9b130ee81cfc293ae30ff
Author: Patrick McHardy <kaber@trash.net>
Date:   Tue Jun 9 18:01:11 2009 +0200

    vlan: propagate transmission state
    
    Propagate transmission state to upper layers and maintain error statistics.
    
    Signed-off-by: Patrick McHardy <kaber@trash.net>

diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 96bad8f..39a8e03 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -292,6 +292,8 @@ static int vlan_dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct netdev_queue *txq = netdev_get_tx_queue(dev, 0);
 	struct vlan_ethhdr *veth = (struct vlan_ethhdr *)(skb->data);
+	unsigned int len;
+	int err;
 
 	/* Handle non-VLAN frames if they are sent to us, for example by DHCP.
 	 *
@@ -317,19 +319,25 @@ static int vlan_dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev)
 			vlan_dev_info(dev)->cnt_inc_headroom_on_tx++;
 	}
 
-	txq->tx_packets++;
-	txq->tx_bytes += skb->len;
-
 	skb->dev = vlan_dev_info(dev)->real_dev;
-	dev_queue_xmit(skb);
-	return NETDEV_TX_OK;
+	len = skb->len;
+	err = dev_queue_xmit(skb);
+	if (err == NET_XMIT_SUCCESS) {
+		txq->tx_packets++;
+		txq->tx_bytes += len;
+	} else
+		txq->tx_dropped++;
+
+	return err;
 }
 
 static int vlan_dev_hwaccel_hard_start_xmit(struct sk_buff *skb,
 					    struct net_device *dev)
 {
 	struct netdev_queue *txq = netdev_get_tx_queue(dev, 0);
+	unsigned int len;
 	u16 vlan_tci;
+	int err;
 
 	vlan_tci = vlan_dev_info(dev)->vlan_id;
 	vlan_tci |= vlan_dev_get_egress_qos_mask(dev, skb);
@@ -339,8 +347,15 @@ static int vlan_dev_hwaccel_hard_start_xmit(struct sk_buff *skb,
 	txq->tx_bytes += skb->len;
 
 	skb->dev = vlan_dev_info(dev)->real_dev;
-	dev_queue_xmit(skb);
-	return NETDEV_TX_OK;
+	len = skb->len;
+	err = dev_queue_xmit(skb);
+	if (err == NET_XMIT_SUCCESS) {
+		txq->tx_packets++;
+		txq->tx_bytes += len;
+	} else
+		txq->tx_dropped++;
+
+	return err;
 }
 
 static int vlan_dev_change_mtu(struct net_device *dev, int new_mtu)

Re: [RFC vlan 03/03]: propagate transmission state

[Eric Dumazet]

Patrick McHardy a écrit :
> commit b124eaaf852e982c7af9b130ee81cfc293ae30ff
> Author: Patrick McHardy <kaber@trash.net>
> Date:   Tue Jun 9 18:01:11 2009 +0200
> 
>     vlan: propagate transmission state
>     
>     Propagate transmission state to upper layers and maintain error statistics.
>     
>     Signed-off-by: Patrick McHardy <kaber@trash.net>
> 
> diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
> index 96bad8f..39a8e03 100644
> --- a/net/8021q/vlan_dev.c
> +++ b/net/8021q/vlan_dev.c
> @@ -292,6 +292,8 @@ static int vlan_dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev)
>  {
>  	struct netdev_queue *txq = netdev_get_tx_queue(dev, 0);
>  	struct vlan_ethhdr *veth = (struct vlan_ethhdr *)(skb->data);
> +	unsigned int len;
> +	int err;
>  
>  	/* Handle non-VLAN frames if they are sent to us, for example by DHCP.
>  	 *
> @@ -317,19 +319,25 @@ static int vlan_dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev)
>  			vlan_dev_info(dev)->cnt_inc_headroom_on_tx++;
>  	}
>  
> -	txq->tx_packets++;
> -	txq->tx_bytes += skb->len;
> -
>  	skb->dev = vlan_dev_info(dev)->real_dev;
> -	dev_queue_xmit(skb);
> -	return NETDEV_TX_OK;
> +	len = skb->len;
> +	err = dev_queue_xmit(skb);
> +	if (err == NET_XMIT_SUCCESS) {
> +		txq->tx_packets++;
> +		txq->tx_bytes += len;
> +	} else
> +		txq->tx_dropped++;
> +
> +	return err;
>  }
>  
>  static int vlan_dev_hwaccel_hard_start_xmit(struct sk_buff *skb,
>  					    struct net_device *dev)
>  {
>  	struct netdev_queue *txq = netdev_get_tx_queue(dev, 0);
> +	unsigned int len;
>  	u16 vlan_tci;
> +	int err;
>  
>  	vlan_tci = vlan_dev_info(dev)->vlan_id;
>  	vlan_tci |= vlan_dev_get_egress_qos_mask(dev, skb);
> @@ -339,8 +347,15 @@ static int vlan_dev_hwaccel_hard_start_xmit(struct sk_buff *skb,
>  	txq->tx_bytes += skb->len;

It seems you forgot to delete previous txq->tx_bytes += skb->len; && txq->tx_packets++; ?

>  
>  	skb->dev = vlan_dev_info(dev)->real_dev;
> -	dev_queue_xmit(skb);
> -	return NETDEV_TX_OK;
> +	len = skb->len;
> +	err = dev_queue_xmit(skb);
> +	if (err == NET_XMIT_SUCCESS) {
> +		txq->tx_packets++;
> +		txq->tx_bytes += len;
> +	} else
> +		txq->tx_dropped++;
> +
> +	return err;
>  }
>  
>  static int vlan_dev_change_mtu(struct net_device *dev, int new_mtu)

Re: [RFC vlan 03/03]: propagate transmission state

[Patrick McHardy]

Eric Dumazet wrote:
> Patrick McHardy a écrit :
>>  static int vlan_dev_hwaccel_hard_start_xmit(struct sk_buff *skb,
>>  					    struct net_device *dev)
>>  {
>>  	struct netdev_queue *txq = netdev_get_tx_queue(dev, 0);
>> +	unsigned int len;
>>  	u16 vlan_tci;
>> +	int err;
>>  
>>  	vlan_tci = vlan_dev_info(dev)->vlan_id;
>>  	vlan_tci |= vlan_dev_get_egress_qos_mask(dev, skb);
>> @@ -339,8 +347,15 @@ static int vlan_dev_hwaccel_hard_start_xmit(struct sk_buff *skb,
>>  	txq->tx_bytes += skb->len;
> 
> It seems you forgot to delete previous txq->tx_bytes += skb->len; && txq->tx_packets++; ?

Indeed, thanks, I've fixed it in my tree.

Re: [RFC net 00/03]: dev_queue_xmit error propagation

[David Miller]

From: Patrick McHardy <kaber@trash.net>
Date: Tue,  9 Jun 2009 18:16:59 +0200 (MEST)

> Currently the ->ndo_hard_start_xmit() callbacks are only permitted to return
> one of the NETDEV_TX codes. This prevents any kind of error propagation for
> virtual devices, like queue congestion of the underlying device in case of
> layered devices, or unreachability in case of tunnels.
> 
> These patches change the return conventions so hard_start_xmit() can return
> a NETDEV_TX code, an errno code or a NET_XMIT code. This means it can
> additionally to the NETDEV_TX codes simply return the value of dev_queue_xmit().
> 
> When queueing is used, errors can't be propagated back since transmission is
> asynchronously and all non-NETDEV codes are mapped to NETDEV_TX_OK, reflecting
> the fact that the skb is consumed unconditionally.
> 
> In case of virtual devices not using queueing, the NETDEV_TX codes are consumed
> by dev_queue_xmit(), everything else is returned to the higher layers.
> 
> The end result is that virtual network devices can propagate queue congestion
> state of the underlying device, or other errors, back to the socket layer.
> One immediately useful effect is that TCP can make use of this information
> as is currently done with non-virtual devices. But it also seems useful in case
> of tunnels, which can return more meaningful errors, or for statistics.
> 
> Comments welcome. A review of my changes in dev_hard_start_xmit() would be
> especially appreciated :)

These changes look fine to me.

That one if() conditional checking 'ret' for three different
properties is stretching the brain a bit.  Maybe you can
encapsulate that sucker into an inline function with suitable
comments.  That way when someone reads the test, it just says
what it is looking for, rather than being some complicated
set of tests.

Once you fix that up and the problem Eric spotted in patch #3
feel free to formally submit this.

Thanks!

Re: [RFC net 00/03]: dev_queue_xmit error propagation

[Patrick McHardy]

David Miller wrote:
> From: Patrick McHardy <kaber@trash.net>
> Date: Tue,  9 Jun 2009 18:16:59 +0200 (MEST)
> 
>> Currently the ->ndo_hard_start_xmit() callbacks are only permitted to return
>> one of the NETDEV_TX codes. This prevents any kind of error propagation for
>> virtual devices, like queue congestion of the underlying device in case of
>> layered devices, or unreachability in case of tunnels.
>>
>> These patches change the return conventions so hard_start_xmit() can return
>> a NETDEV_TX code, an errno code or a NET_XMIT code. This means it can
>> additionally to the NETDEV_TX codes simply return the value of dev_queue_xmit().
>>

> These changes look fine to me.
> 
> That one if() conditional checking 'ret' for three different
> properties is stretching the brain a bit.  Maybe you can
> encapsulate that sucker into an inline function with suitable
> comments.  That way when someone reads the test, it just says
> what it is looking for, rather than being some complicated
> set of tests.

Yeah, I mainly failed to come up with a suitable name, but I've now
moved it to an inline function called "dev_xmit_skb_consumed".
Better suggestions are still welcome though :)

> Once you fix that up and the problem Eric spotted in patch #3
> feel free to formally submit this.

I still need to verify that no drivers are using the raw values for
the NETDEV_TX codes since NETDEV_TX_LOCKED is using a different
value with my patch. I'll probably submit them later today.

Re: [RFC net 00/03]: dev_queue_xmit error propagation

[David Miller]

From: Patrick McHardy <kaber@trash.net>
Date: Thu, 11 Jun 2009 18:33:26 +0200

> I still need to verify that no drivers are using the raw values for
> the NETDEV_TX codes since NETDEV_TX_LOCKED is using a different
> value with my patch.

Yeah, auditing 500 ->hard_start_xmit implementations is fun.

Re: [RFC net 00/03]: dev_queue_xmit error propagation

[Patrick McHardy]

David Miller wrote:
> From: Patrick McHardy <kaber@trash.net>
> Date: Thu, 11 Jun 2009 18:33:26 +0200
>
>   
>> I still need to verify that no drivers are using the raw values for
>> the NETDEV_TX codes since NETDEV_TX_LOCKED is using a different
>> value with my patch.
>>     
>
> Yeah, auditing 500 ->hard_start_xmit implementations is fun.
>   

Indeed. Found about 30 which are already returning errno codes :)

Re: [RFC net 02/03]: allow to propagate errors through ->ndo_hard_start_xmit()

[Jarek Poplawski]

On 09-06-2009 18:17, Patrick McHardy wrote:
> commit c2104095c3a1023b4f4809c90092145dff093c88
> Author: Patrick McHardy <kaber@trash.net>
> Date:   Tue Jun 9 17:55:34 2009 +0200
> 
>     net: allow to propagate errors through ->ndo_hard_start_xmit()
>     
...
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 11560e3..b2e9953 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1731,6 +1731,8 @@ gso:
>  		nskb->next = NULL;
>  		rc = ops->ndo_start_xmit(nskb, dev);
>  		if (unlikely(rc)) {
> +			if (rc & ~NETDEV_TX_MASK)
> +				return rc;

This is probably not enough: what about the rest of the skb
(qdisc_restart() would skip requeuing)?

Jarek P.

>  			nskb->next = skb->next;
>  			skb->next = nskb;
>  			return rc;
> @@ -1895,8 +1897,9 @@ gso:
>  			HARD_TX_LOCK(dev, txq, cpu);
>  
>  			if (!netif_tx_queue_stopped(txq)) {
> -				rc = 0;
> -				if (!dev_hard_start_xmit(skb, dev, txq)) {
> +				rc = dev_hard_start_xmit(skb, dev, txq);
> +				if (rc == NETDEV_TX_OK || rc < 0 ||
> +				    rc & NET_XMIT_MASK) {
>  					HARD_TX_UNLOCK(dev, txq);
>  					goto out;
>  				}
> diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
> index 27d0381..7e3e514 100644
> --- a/net/sched/sch_generic.c
> +++ b/net/sched/sch_generic.c
> @@ -143,8 +143,16 @@ static inline int qdisc_restart(struct Qdisc *q)
>  
>  	HARD_TX_LOCK(dev, txq, smp_processor_id());
>  	if (!netif_tx_queue_stopped(txq) &&
> -	    !netif_tx_queue_frozen(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);
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [RFC net 02/03]: allow to propagate errors through ->ndo_hard_start_xmit()

[Patrick McHardy]

Jarek Poplawski wrote:
> On 09-06-2009 18:17, Patrick McHardy wrote:
>> commit c2104095c3a1023b4f4809c90092145dff093c88
>> Author: Patrick McHardy <kaber@trash.net>
>> Date:   Tue Jun 9 17:55:34 2009 +0200
>>
>>     net: allow to propagate errors through ->ndo_hard_start_xmit()
>>     
> ...
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 11560e3..b2e9953 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -1731,6 +1731,8 @@ gso:
>>  		nskb->next = NULL;
>>  		rc = ops->ndo_start_xmit(nskb, dev);
>>  		if (unlikely(rc)) {
>> +			if (rc & ~NETDEV_TX_MASK)
>> +				return rc;
> 
> This is probably not enough: what about the rest of the skb
> (qdisc_restart() would skip requeuing)?

Good point, I guess we should drop the remaining parts.
I'll fix that up, thanks.