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

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

[Stephen Hemminger]

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 <shemminger@vyatta.com>


--- 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&#174; Ethernet, visit http://communities.intel.com/community/wired

[PATCH 2/2 net-next] ixgbe: remove xmit length check

[Stephen Hemminger]

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 <shemminger@vyatta.com>


--- 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&#174; Ethernet, visit http://communities.intel.com/community/wired

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

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 358 bytes --]

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 <shemminger@vyatta.com> 

Thanks Stephen, I have added it to my queue.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2 net-next] ixgbe: remove xmit length check

[Jeff Kirsher]

[-- Attachment #1.1: Type: text/plain, Size: 349 bytes --]

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 <shemminger@vyatta.com> 

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

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 395 bytes --]

------------------------------------------------------------------------------
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/

[-- Attachment #3: Type: text/plain, Size: 257 bytes --]

_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

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

[Alexander Duyck]

On 06/18/2012 10:58 AM, 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 <shemminger@vyatta.com>
>
>
> --- 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);
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

[PATCH net-next] ixgbe: simplify padding and length checks (v2)

[Stephen Hemminger]

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 <shemminger@vyatta.com>


--- 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;
 	}

Re: [PATCH net-next] ixgbe: simplify padding and length checks (v2)

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 548 bytes --]

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 <shemminger@vyatta.com> 

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.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]