[PATCH 1/2 net-next] ixgbe: use skb_padto

The code to pad packets here is the same effective code as
the existing inline function skb_padto(). There is a minor
performance gain since skb_padto() also uses unlikely().

Signed-off-by: Stephen Hemminger <redacted>

--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c	2012-06-18 10:53:09.130376800 -0700
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c	2012-06-18 10:55:13.104540844 -0700

@@ -6389,11 +6389,8 @@ static netdev_tx_t ixgbe_xmit_frame(stru
 	 * The minimum packet size for olinfo paylen is 17 so pad the skb
 	 * in order to meet this minimum size requirement.
 	 */
-	if (skb->len < 17) {
-		if (skb_padto(skb, 17))
-			return NETDEV_TX_OK;
-		skb->len = 17;
-	}
+	if (skb_padto(skb, 17))
+		return NETDEV_TX_OK;
 
 	tx_ring = adapter->tx_ring[skb->queue_mapping];
 	return ixgbe_xmit_frame_ring(skb, adapter, tx_ring);

------------------------------------------------------------------------------

Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired

The check here is bogus. Since len is unsigned, it can never
be negative. And it would be a bug in network stack to ever
send a zero length packet to device.

Signed-off-by: Stephen Hemminger <redacted>

--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c	2012-06-18 10:53:13.742310994 -0700
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c	2012-06-18 10:53:15.962279249 -0700

@@ -6380,11 +6380,6 @@ static netdev_tx_t ixgbe_xmit_frame(stru
 	struct ixgbe_adapter *adapter = netdev_priv(netdev);
 	struct ixgbe_ring *tx_ring;
 
-	if (skb->len <= 0) {
-		dev_kfree_skb_any(skb);
-		return NETDEV_TX_OK;
-	}
-
 	/*
 	 * The minimum packet size for olinfo paylen is 17 so pad the skb
 	 * in order to meet this minimum size requirement.

------------------------------------------------------------------------------

Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired

On Mon, 2012-06-18 at 10:58 -0700, Stephen Hemminger wrote:

The code to pad packets here is the same effective code as
the existing inline function skb_padto(). There is a minor
performance gain since skb_padto() also uses unlikely().

Signed-off-by: Stephen Hemminger <redacted> 

Thanks Stephen, I have added it to my queue.

On Mon, 2012-06-18 at 11:55 -0700, Stephen Hemminger wrote:

The check here is bogus. Since len is unsigned, it can never
be negative. And it would be a bug in network stack to ever
send a zero length packet to device.

Signed-off-by: Stephen Hemminger <redacted> 

Thanks, I have added this patch as well to my queue.

On 06/18/2012 10:58 AM, Stephen Hemminger wrote:

I don't think this will work.  We need to update the skb->len and last I
knew skb_padto doesn't do that.

Thanks,

Alex

The check for length <= 0 is bogus because length is unsigned, and network
stack never sends zero length packets (unless it is totally broken).

The check for really small packets can be optimized (using unlikely)
and calling skb_pad directly.

Signed-off-by: Stephen Hemminger <redacted>

--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c	2012-06-18 10:53:09.130376800 -0700
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c	2012-06-18 15:20:44.364951004 -0700

@@ -6380,17 +6380,12 @@ static netdev_tx_t ixgbe_xmit_frame(stru
 	struct ixgbe_adapter *adapter = netdev_priv(netdev);
 	struct ixgbe_ring *tx_ring;
 
-	if (skb->len <= 0) {
-		dev_kfree_skb_any(skb);
-		return NETDEV_TX_OK;
-	}
-
 	/*
 	 * The minimum packet size for olinfo paylen is 17 so pad the skb
 	 * in order to meet this minimum size requirement.
 	 */
-	if (skb->len < 17) {
-		if (skb_padto(skb, 17))
+	if (unlikely(skb->len < 17)) {
+		if (skb_pad(skb, 17 - skb->len))
 			return NETDEV_TX_OK;
 		skb->len = 17;
 	}

On Mon, 2012-06-18 at 16:31 -0700, Stephen Hemminger wrote:

The check for length <= 0 is bogus because length is unsigned, and
network
stack never sends zero length packets (unless it is totally broken).

The check for really small packets can be optimized (using unlikely)
and calling skb_pad directly.

Signed-off-by: Stephen Hemminger <redacted> 

I just realized I had not responded to this updated patch.

Thanks Stephen, I have dropped your previous 2 patch series and added
this patch to my queue.