netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [net 2/3] net: do not do gso for CHECKSUM_UNNECESSARY in netif_needs_gso
@ 2012-03-17  9:08 Jeff Kirsher
  2012-03-17  9:08 ` [net 3/3] fcoe: use CHECKSUM_UNNECESSARY instead of CHECKSUM_PARTIAL on tx Jeff Kirsher
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Jeff Kirsher @ 2012-03-17  9:08 UTC (permalink / raw)
  To: davem; +Cc: Yi Zou, netdev, gospo, sassmann, 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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [net 3/3] fcoe: use CHECKSUM_UNNECESSARY instead of CHECKSUM_PARTIAL on tx
  2012-03-17  9:08 [net 2/3] net: do not do gso for CHECKSUM_UNNECESSARY in netif_needs_gso Jeff Kirsher
@ 2012-03-17  9:08 ` Jeff Kirsher
  2012-03-19 21:38   ` David Miller
  2012-03-17 18:41 ` [net 2/3] net: do not do gso for CHECKSUM_UNNECESSARY in netif_needs_gso Ben Hutchings
  2012-03-19 21:38 ` David Miller
  2 siblings, 1 reply; 6+ messages in thread
From: Jeff Kirsher @ 2012-03-17  9:08 UTC (permalink / raw)
  To: davem
  Cc: Yi Zou, netdev, gospo, sassmann, James E.J. Bottomley,
	Robert Love, 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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [net 2/3] net: do not do gso for CHECKSUM_UNNECESSARY in netif_needs_gso
  2012-03-17  9:08 [net 2/3] net: do not do gso for CHECKSUM_UNNECESSARY in netif_needs_gso Jeff Kirsher
  2012-03-17  9:08 ` [net 3/3] fcoe: use CHECKSUM_UNNECESSARY instead of CHECKSUM_PARTIAL on tx Jeff Kirsher
@ 2012-03-17 18:41 ` Ben Hutchings
  2012-03-19 15:11   ` Zou, Yi
  2012-03-19 21:38 ` David Miller
  2 siblings, 1 reply; 6+ messages in thread
From: Ben Hutchings @ 2012-03-17 18:41 UTC (permalink / raw)
  To: Jeff Kirsher; +Cc: davem, Yi Zou, netdev, gospo, sassmann

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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [net 2/3] net: do not do gso for CHECKSUM_UNNECESSARY in netif_needs_gso
  2012-03-17 18:41 ` [net 2/3] net: do not do gso for CHECKSUM_UNNECESSARY in netif_needs_gso Ben Hutchings
@ 2012-03-19 15:11   ` Zou, Yi
  0 siblings, 0 replies; 6+ messages in thread
From: Zou, Yi @ 2012-03-19 15:11 UTC (permalink / raw)
  To: Ben Hutchings, Kirsher, Jeffrey T
  Cc: davem@davemloft.net, netdev@vger.kernel.org, gospo@redhat.com,
	sassmann@redhat.com

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [net 2/3] net: do not do gso for CHECKSUM_UNNECESSARY in netif_needs_gso
  2012-03-17  9:08 [net 2/3] net: do not do gso for CHECKSUM_UNNECESSARY in netif_needs_gso Jeff Kirsher
  2012-03-17  9:08 ` [net 3/3] fcoe: use CHECKSUM_UNNECESSARY instead of CHECKSUM_PARTIAL on tx Jeff Kirsher
  2012-03-17 18:41 ` [net 2/3] net: do not do gso for CHECKSUM_UNNECESSARY in netif_needs_gso Ben Hutchings
@ 2012-03-19 21:38 ` David Miller
  2 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2012-03-19 21:38 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: yi.zou, netdev, gospo, sassmann

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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [net 3/3] fcoe: use CHECKSUM_UNNECESSARY instead of CHECKSUM_PARTIAL on tx
  2012-03-17  9:08 ` [net 3/3] fcoe: use CHECKSUM_UNNECESSARY instead of CHECKSUM_PARTIAL on tx Jeff Kirsher
@ 2012-03-19 21:38   ` David Miller
  0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2012-03-19 21:38 UTC (permalink / raw)
  To: jeffrey.t.kirsher
  Cc: yi.zou, netdev, gospo, sassmann, JBottomley, robert.w.love

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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-03-19 21:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-17  9:08 [net 2/3] net: do not do gso for CHECKSUM_UNNECESSARY in netif_needs_gso Jeff Kirsher
2012-03-17  9:08 ` [net 3/3] fcoe: use CHECKSUM_UNNECESSARY instead of CHECKSUM_PARTIAL on tx Jeff Kirsher
2012-03-19 21:38   ` David Miller
2012-03-17 18:41 ` [net 2/3] net: do not do gso for CHECKSUM_UNNECESSARY in netif_needs_gso Ben Hutchings
2012-03-19 15:11   ` Zou, Yi
2012-03-19 21:38 ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).