[net 2/3] net: do not do gso for CHECKSUM_UNNECESSARY in netif_needs_gso

[net 2/3] net: do not do gso for CHECKSUM_UNNECESSARY in netif_needs_gso

[Jeff Kirsher]

From: Yi Zou <yi.zou@intel.com>

This is related to fixing the bug of dropping FCoE frames when disabling tx ip
checksum by 'ethtool -K ethx tx off'. The FCoE protocol stack driver would
use CHECKSUM_UNNECESSARY on tx path instead of CHECKSUM_PARTIAL (as indicated in
the 2/2 of this series). To do so, netif_needs_gso() has to be changed here to
not do gso for both CHECKSUM_PARTIAL and CHECKSUM_UNNECESSARY.

Ref. to original discussion thread:
http://patchwork.ozlabs.org/patch/146567/

Signed-off-by: Yi Zou <yi.zou@intel.com>
Tested-by: Ross Brattain <ross.b.brattain@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 include/linux/netdevice.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 0eac07c..c1b2b5f 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2636,7 +2636,8 @@ static inline int netif_needs_gso(struct sk_buff *skb,
 	netdev_features_t features)
 {
 	return skb_is_gso(skb) && (!skb_gso_ok(skb, features) ||
-		unlikely(skb->ip_summed != CHECKSUM_PARTIAL));
+		unlikely((skb->ip_summed != CHECKSUM_PARTIAL) &&
+			 (skb->ip_summed != CHECKSUM_UNNECESSARY)));
 }
 
 static inline void netif_set_gso_max_size(struct net_device *dev,
-- 
1.7.7.6

[net 3/3] fcoe: use CHECKSUM_UNNECESSARY instead of CHECKSUM_PARTIAL on tx

[Jeff Kirsher]

From: Yi Zou <yi.zou@intel.com>

Fix a bug when using 'ethtool -K ethx tx off' to turn off tx ip checksum,
FCoE CRC offload should not be impacte. The skb_checksum_help() is needed
only if it's not FCoE traffic for ip checksum, regardless of ethtool toggling
the tx ip checksum on or off. Instead of using CHECKSUM_PARTIAL, we will
use CHECKSUM_UNNECESSARY as a proper indication to avoid sw ip checksum
on FCoE frames.

Ref. to original discussion thread:
http://patchwork.ozlabs.org/patch/146567/

CC: "James E.J. Bottomley" <JBottomley@parallels.com>
CC: Robert Love <robert.w.love@intel.com>
Signed-off-by: Yi Zou <yi.zou@intel.com>
Tested-by: Ross Brattain <ross.b.brattain@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/scsi/fcoe/fcoe.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index e959960..c164890 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -1498,7 +1498,7 @@ static int fcoe_xmit(struct fc_lport *lport, struct fc_frame *fp)
 
 	/* crc offload */
 	if (likely(lport->crc_offload)) {
-		skb->ip_summed = CHECKSUM_PARTIAL;
+		skb->ip_summed = CHECKSUM_UNNECESSARY;
 		skb->csum_start = skb_headroom(skb);
 		skb->csum_offset = skb->len;
 		crc = 0;
-- 
1.7.7.6

Re: [net 3/3] fcoe: use CHECKSUM_UNNECESSARY instead of CHECKSUM_PARTIAL on tx

[David Miller]

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Sat, 17 Mar 2012 02:08:12 -0700

> From: Yi Zou <yi.zou@intel.com>
> 
> Fix a bug when using 'ethtool -K ethx tx off' to turn off tx ip checksum,
> FCoE CRC offload should not be impacte. The skb_checksum_help() is needed
> only if it's not FCoE traffic for ip checksum, regardless of ethtool toggling
> the tx ip checksum on or off. Instead of using CHECKSUM_PARTIAL, we will
> use CHECKSUM_UNNECESSARY as a proper indication to avoid sw ip checksum
> on FCoE frames.
> 
> Ref. to original discussion thread:
> http://patchwork.ozlabs.org/patch/146567/
> 
> CC: "James E.J. Bottomley" <JBottomley@parallels.com>
> CC: Robert Love <robert.w.love@intel.com>
> Signed-off-by: Yi Zou <yi.zou@intel.com>
> Tested-by: Ross Brattain <ross.b.brattain@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

Applied.

Re: [net 2/3] net: do not do gso for CHECKSUM_UNNECESSARY in netif_needs_gso

[Ben Hutchings]

On Sat, 2012-03-17 at 02:08 -0700, Jeff Kirsher wrote:
> From: Yi Zou <yi.zou@intel.com>
> 
> This is related to fixing the bug of dropping FCoE frames when disabling tx ip
> checksum by 'ethtool -K ethx tx off'. The FCoE protocol stack driver would
> use CHECKSUM_UNNECESSARY on tx path instead of CHECKSUM_PARTIAL (as indicated in
> the 2/2 of this series). To do so, netif_needs_gso() has to be changed here to
> not do gso for both CHECKSUM_PARTIAL and CHECKSUM_UNNECESSARY.
[...]

This should also be documented as valid in include/linux/skbuff.h,
though I don't think the fix should be held up for that.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

RE: [net 2/3] net: do not do gso for CHECKSUM_UNNECESSARY in netif_needs_gso

[Zou, Yi]

> 
> On Sat, 2012-03-17 at 02:08 -0700, Jeff Kirsher wrote:
> > From: Yi Zou <yi.zou@intel.com>
> >
> > This is related to fixing the bug of dropping FCoE frames when
> disabling tx ip
> > checksum by 'ethtool -K ethx tx off'. The FCoE protocol stack driver
> would
> > use CHECKSUM_UNNECESSARY on tx path instead of CHECKSUM_PARTIAL (as
> indicated in
> > the 2/2 of this series). To do so, netif_needs_gso() has to be changed
> here to
> > not do gso for both CHECKSUM_PARTIAL and CHECKSUM_UNNECESSARY.
> [...]
> 
> This should also be documented as valid in include/linux/skbuff.h,
> though I don't think the fix should be held up for that.
> 
> Ben.
Sure, I will send a separate patch to update the skbuff.h shortly.

Thanks,
yi

> 
> --
> Ben Hutchings, Staff Engineer, Solarflare
> Not speaking for my employer; that's the marketing department's job.
> They asked us to note that Solarflare product names are trademarked.

Re: [net 2/3] net: do not do gso for CHECKSUM_UNNECESSARY in netif_needs_gso

[David Miller]

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Sat, 17 Mar 2012 02:08:11 -0700

> From: Yi Zou <yi.zou@intel.com>
> 
> This is related to fixing the bug of dropping FCoE frames when disabling tx ip
> checksum by 'ethtool -K ethx tx off'. The FCoE protocol stack driver would
> use CHECKSUM_UNNECESSARY on tx path instead of CHECKSUM_PARTIAL (as indicated in
> the 2/2 of this series). To do so, netif_needs_gso() has to be changed here to
> not do gso for both CHECKSUM_PARTIAL and CHECKSUM_UNNECESSARY.
> 
> Ref. to original discussion thread:
> http://patchwork.ozlabs.org/patch/146567/
> 
> Signed-off-by: Yi Zou <yi.zou@intel.com>
> Tested-by: Ross Brattain <ross.b.brattain@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

Applied.