[PATCH 11/15] staging: unisys: Fix improper use of NETDEV_TX_BUSY

Benjamin Romer benjamin.romer at unisys.com
Tue Jul 21 13:55:45 UTC 2015


From: Neil Horman <nhorman at redhat.com>

Using NETDEV_TX_BUSY is tricky.  Its meant for situations where the error
in question is transient and quickly resolved.  But the driver rarely is
able to know that to a certainty.  And in the case of visornic, it just
uses it without any care for that, in the hopes that it won't loose frames,
even if the problem is that the skb is somehow malformed for the hardware.
If we get one of those kinds of skbs, NETDEV_TX_BUSY will just cause us to
spin, processing the same error over and over.

Fix it by dropping the frame, stopping the queue where appropriate, and
returning NETDEV_TX_OK

Signed-off-by: Neil Horman <nhorman at redhat.com>
Signed-off-by: Benjamin Romer <benjamin.romer at unisys.com>
---
 drivers/staging/unisys/visornic/visornic_main.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/unisys/visornic/visornic_main.c b/drivers/staging/unisys/visornic/visornic_main.c
index 338ca8c..8cc9017 100644
--- a/drivers/staging/unisys/visornic/visornic_main.c
+++ b/drivers/staging/unisys/visornic/visornic_main.c
@@ -787,7 +787,7 @@ visornic_close(struct net_device *netdev)
  *	function is protected from concurrent calls by a spinlock xmit_lock
  *	in the net_device struct, but as soon as the function returns it
  *	can be called again.
- *	Returns NETDEV_TX_OK for success, NETDEV_TX_BUSY for error.
+ *	Returns NETDEV_TX_OK.
  */
 static int
 visornic_xmit(struct sk_buff *skb, struct net_device *netdev)
@@ -806,7 +806,8 @@ visornic_xmit(struct sk_buff *skb, struct net_device *netdev)
 		devdata->busy_cnt++;
 		dev_dbg(&netdev->dev,
 			"%s busy - queue stopped\n", __func__);
-		return NETDEV_TX_BUSY;
+		kfree_skb(skb);
+		return NETDEV_TX_OK;
 	}
 
 	/* sk_buff struct is used to host network data throughout all the
@@ -827,7 +828,8 @@ visornic_xmit(struct sk_buff *skb, struct net_device *netdev)
 		dev_err(&netdev->dev,
 			"%s busy - first frag too small (%d)\n",
 			__func__, firstfraglen);
-		return NETDEV_TX_BUSY;
+		kfree_skb(skb);
+		return NETDEV_TX_OK;
 	}
 
 	if ((len < ETH_MIN_PACKET_SIZE) &&
@@ -869,7 +871,8 @@ visornic_xmit(struct sk_buff *skb, struct net_device *netdev)
 		dev_dbg(&netdev->dev,
 			"%s busy - waiting for iovm to catch up\n",
 			__func__);
-		return NETDEV_TX_BUSY;
+		kfree_skb(skb);
+		return NETDEV_TX_OK;
 	}
 	if (devdata->queuefullmsg_logged)
 		devdata->queuefullmsg_logged = 0;
@@ -911,7 +914,8 @@ visornic_xmit(struct sk_buff *skb, struct net_device *netdev)
 		devdata->busy_cnt++;
 		dev_err(&netdev->dev,
 			"%s busy - copy frags failed\n", __func__);
-		return NETDEV_TX_BUSY;
+		kfree_skb(skb);
+		return NETDEV_TX_OK;
 	}
 
 	if (!visorchannel_signalinsert(devdata->dev->visorchannel,
@@ -921,7 +925,8 @@ visornic_xmit(struct sk_buff *skb, struct net_device *netdev)
 		devdata->busy_cnt++;
 		dev_dbg(&netdev->dev,
 			"%s busy - signalinsert failed\n", __func__);
-		return NETDEV_TX_BUSY;
+		kfree_skb(skb);
+		return NETDEV_TX_OK;
 	}
 
 	/* Track the skbs that have been sent to the IOVM for XMIT */
-- 
2.1.4



More information about the devel mailing list