RE: [net 2/2] net: fix a bug of dropping FCoE frames when disabling tx ip checksum

On Wed, 14 Mar 2012 18:54:32 +0000
"Zou, Yi" [off-list ref] wrote:

quoted

quoted

On Wed, 14 Mar 2012 00:01:58 -0700
Jeff Kirsher [off-list ref] wrote:

quoted

From: Yi Zou <redacted>

Fix a bug when using 'ethtool -K ethx tx off' to turn off tx ip

checksum,

quoted

FCoE CRC offload should not be impacte. The skb_checksum_help() is

needed

quoted

only if it's not FCoE traffic for ip checksum, regardless of

ethtool

quoted

quoted

toggling

quoted

the tx ip checksum on or off.

Signed-off-by: Yi Zou <redacted>
Tested-by: Ross Brattain <redacted>
Signed-off-by: Jeff Kirsher <redacted>
---
 net/core/dev.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 6ca32f6..9e378009 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c

@@ -1913,6 +1913,9 @@ int skb_checksum_help(struct sk_buff *skb)
 	__wsum csum;
 	int ret = 0, offset;

+	if (skb->protocol == htons(ETH_P_FCOE))
+		goto out;
+
 	if (skb->ip_summed == CHECKSUM_COMPLETE)
 		goto out_set_summed;

Shouldn't this be done in the offending driver rather than the core?

Well, yes and no, the driver is already using NETIF_F_FCOE_CRC to

indicate

quoted

the FCoE CRC offload capability, and can_checksum_protocol() indicates

that.

quoted

The fcoe protocol stack then uses CHECKSUM_PARTIAL and NETIF_F_FCOE_CRC

to

quoted

pass through netdev w/o doing skb_checksum(), which is not captured

here

quoted

in dev_hard_start_xmit() as it looks at NETIF_F_ALL_CSUM only before

calling

quoted

skb_checksum_help().

But your change would break any code (if there is any) generating this
kind of packet and going through firewall, etc. For example a bridge or
router for FCOE packets.

I don't know anything like that exists right now but in that case for fcoe,
you dan't want to use CHECKSUM_PARTIAL on output. Still, the point is the 
skb_checksum_help() only makes sense for ip checksum.

yi