[PATCH net] ixgbe: Only set gso_type to SKB_GSO_TCPV4 as RSC does not support IPv6

[PATCH net] ixgbe: Only set gso_type to SKB_GSO_TCPV4 as RSC does not support IPv6

[Alexander Duyck]

The original fix that was applied for setting gso_type required more change
than necessary because it was assumed ixgbe does RSC on IPv6 frames and this
is not correct.  RSC is only supported with IPv4/TCP frames only.  As such we
can simplify the fix and avoid the unnecessary move of eth_type_trans.

The previous patch "ixgbe: fix gso type" and this patch reduce the entire fix
to one line that sets gso_type to TCPV4 if the frame is RSC.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---

Sorry about not speaking up last week which would have allowed us to avoid
this patch but I was out on a business trip and had limited access to email
between flights.

 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index e1b2d22..b3e3294 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -1401,10 +1401,7 @@ static void ixgbe_set_rsc_gso_size(struct ixgbe_ring *ring,
 	/* set gso_size to avoid messing up TCP MSS */
 	skb_shinfo(skb)->gso_size = DIV_ROUND_UP((skb->len - hdr_len),
 						 IXGBE_CB(skb)->append_cnt);
-	if (skb->protocol == __constant_htons(ETH_P_IPV6))
-		skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
-	else
-		skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
+	skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
 }
 
 static void ixgbe_update_rsc_stats(struct ixgbe_ring *rx_ring,
@@ -1439,8 +1436,6 @@ static void ixgbe_process_skb_fields(struct ixgbe_ring *rx_ring,
 {
 	struct net_device *dev = rx_ring->netdev;
 
-	skb->protocol = eth_type_trans(skb, dev);
-
 	ixgbe_update_rsc_stats(rx_ring, skb);
 
 	ixgbe_rx_hash(rx_ring, rx_desc, skb);
@@ -1456,6 +1451,8 @@ static void ixgbe_process_skb_fields(struct ixgbe_ring *rx_ring,
 	}
 
 	skb_record_rx_queue(skb, rx_ring->queue_index);
+
+	skb->protocol = eth_type_trans(skb, dev);
 }
 
 static void ixgbe_rx_skb(struct ixgbe_q_vector *q_vector,

Re: [PATCH net] ixgbe: Only set gso_type to SKB_GSO_TCPV4 as RSC does not support IPv6

[Michael S. Tsirkin]

On Tue, Feb 12, 2013 at 11:45:44AM -0800, Alexander Duyck wrote:
> The original fix that was applied for setting gso_type required more change
> than necessary because it was assumed ixgbe does RSC on IPv6 frames and this
> is not correct.  RSC is only supported with IPv4/TCP frames only.  As such we
> can simplify the fix and avoid the unnecessary move of eth_type_trans.
> 
> The previous patch "ixgbe: fix gso type" and this patch reduce the entire fix
> to one line that sets gso_type to TCPV4 if the frame is RSC.
> 
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>

Is this architectural? Is it safe to assume there won't be an update
that does RSC on IPv6?

> ---
> 
> Sorry about not speaking up last week which would have allowed us to avoid
> this patch but I was out on a business trip and had limited access to email
> between flights.
> 
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    9 +++------
>  1 files changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index e1b2d22..b3e3294 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -1401,10 +1401,7 @@ static void ixgbe_set_rsc_gso_size(struct ixgbe_ring *ring,
>  	/* set gso_size to avoid messing up TCP MSS */
>  	skb_shinfo(skb)->gso_size = DIV_ROUND_UP((skb->len - hdr_len),
>  						 IXGBE_CB(skb)->append_cnt);
> -	if (skb->protocol == __constant_htons(ETH_P_IPV6))
> -		skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
> -	else
> -		skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
> +	skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
>  }
>  
>  static void ixgbe_update_rsc_stats(struct ixgbe_ring *rx_ring,
> @@ -1439,8 +1436,6 @@ static void ixgbe_process_skb_fields(struct ixgbe_ring *rx_ring,
>  {
>  	struct net_device *dev = rx_ring->netdev;
>  
> -	skb->protocol = eth_type_trans(skb, dev);
> -
>  	ixgbe_update_rsc_stats(rx_ring, skb);
>  
>  	ixgbe_rx_hash(rx_ring, rx_desc, skb);
> @@ -1456,6 +1451,8 @@ static void ixgbe_process_skb_fields(struct ixgbe_ring *rx_ring,
>  	}
>  
>  	skb_record_rx_queue(skb, rx_ring->queue_index);
> +
> +	skb->protocol = eth_type_trans(skb, dev);
>  }
>  
>  static void ixgbe_rx_skb(struct ixgbe_q_vector *q_vector,

Re: [PATCH net] ixgbe: Only set gso_type to SKB_GSO_TCPV4 as RSC does not support IPv6

[David Miller]

From: Alexander Duyck <alexander.h.duyck@intel.com>
Date: Tue, 12 Feb 2013 11:45:44 -0800

> The original fix that was applied for setting gso_type required more change
> than necessary because it was assumed ixgbe does RSC on IPv6 frames and this
> is not correct.  RSC is only supported with IPv4/TCP frames only.  As such we
> can simplify the fix and avoid the unnecessary move of eth_type_trans.
> 
> The previous patch "ixgbe: fix gso type" and this patch reduce the entire fix
> to one line that sets gso_type to TCPV4 if the frame is RSC.
> 
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>

Applied.

Re: [PATCH net] ixgbe: Only set gso_type to SKB_GSO_TCPV4 as RSC does not support IPv6

[Alexander Duyck]

On 02/12/2013 12:51 PM, Michael S. Tsirkin wrote:
> On Tue, Feb 12, 2013 at 11:45:44AM -0800, Alexander Duyck wrote:
>> The original fix that was applied for setting gso_type required more change
>> than necessary because it was assumed ixgbe does RSC on IPv6 frames and this
>> is not correct.  RSC is only supported with IPv4/TCP frames only.  As such we
>> can simplify the fix and avoid the unnecessary move of eth_type_trans.
>>
>> The previous patch "ixgbe: fix gso type" and this patch reduce the entire fix
>> to one line that sets gso_type to TCPV4 if the frame is RSC.
>>
>> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> Is this architectural? Is it safe to assume there won't be an update
> that does RSC on IPv6?

If there is an update at some point in the future we could address it
then.  I just wanted to keep changes to net minimal to avoid a possible
regression, and this patch does that.  No point in future proofing it
for features we may or may not implement at some point in the future.

If we were to add IPv6 support to RSC it would be a serious rewrite of
the feature anyway.  This code would hopefully be refactored as a part
of that rewrite since we would want the hardware to give us the MSS
instead of making us have to approximate it.

Thanks,

Alex