BUG: TCPDUMP invalid cksum persists after disabling TCP cksum offload

Hello,
I am seeing that tx checksum offload appears to be still running after disabling the feature with ethtool. I'm using kernel 3.6.0-rc6 and the latest ethtool from the git repo.

The default settings on my e1000e NIC:

# ethtool -k eth1 | grep ': on'
 rx-checksumming: on
 tx-checksumming: on
        tx-checksum-ip-generic: on
 scatter-gather: on
         tx-scatter-gather: on
 tcp-segmentation-offload: on
         tx-tcp-segmentation: on
         tx-tcp6-segmentation: on
 generic-segmentation-offload: on
 generic-receive-offload: on
 rx-vlan-offload: on
 tx-vlan-offload: on
 receive-hashing: on
 highdma: on [fixed]
 rx-vlan-filter: on [fixed]
 tx-nocache-copy: on

The results after disabling tcp cksum offload feature:

# ethtool -K eth1 tx off
Actual changes:
 tx-checksumming: off
         tx-checksum-ip-generic: off
 scatter-gather: off
         tx-scatter-gather: off [requested on]
 tcp-segmentation-offload: off
         tx-tcp-segmentation: off [requested on]
         tx-tcp6-segmentation: off [requested on]
 generic-segmentation-offload: off [requested on]

However, in tcpdump, I'm still observing incorrect tcp checksum:
14:44:38.838711 IP (tos 0x10, ttl 64, id 45798, offset 0, flags [DF], proto TCP (6), length 60)

     1.1.1.2.59748 > 1.1.1.1.23: Flags [S], cksum 0x0433 (incorrect -> 0x4137), seq 318222122, win 14600, options [mss 1460,sackOK,TS val 5447116 ecr 0,nop,wscale 7], length 0

Is this behaviour valid? I'm quite baffled.

Regards,
Jamie Gloudon

Instead of this, can you try

# ethtool -K eth1 tso off

Vijay

Doesn't TSO depend on TX CKO being enabled? That is, if TX CKO were disabled, shouldn't TSO have been implicitly disabled at the same time?

rick jones

Rick is correct. Disabling TX CKO also disable TSO. You can see that in my ethtool output.

Thats because dev_hard_start_xmit() calls dev_queue_xmit_nit() before doing the features tests :

tcpdump gets a copy of the packet before all mangling done (skb_checksum_help() in your case)

                if (!list_empty(&ptype_all))
                        dev_queue_xmit_nit(skb, dev);

                features = netif_skb_features(skb);

                if (vlan_tx_tag_present(skb) &&
                    !(features & NETIF_F_HW_VLAN_TX)) {
                        skb = __vlan_put_tag(skb, vlan_tx_tag_get(skb));
                        if (unlikely(!skb))
                                goto out;

                        skb->vlan_tci = 0;
                }

                if (netif_needs_gso(skb, features)) {
                        if (unlikely(dev_gso_segment(skb, features)))
                                goto out_kfree_skb;
                        if (skb->next)
                                goto gso;
                } else {
                        if (skb_needs_linearize(skb, features) &&
                            __skb_linearize(skb))
                                goto out_kfree_skb;

                        /* If packet is not checksummed and device does not
                         * support checksumming for this protocol, complete
                         * checksumming here.
                         */
                        if (skb->ip_summed == CHECKSUM_PARTIAL) {
                                skb_set_transport_header(skb,
                                        skb_checksum_start_offset(skb));
                                if (!(features & NETIF_F_ALL_CSUM) &&
                                     skb_checksum_help(skb))
                                        goto out_kfree_skb;
                        }
                }

                skb_len = skb->len;
                rc = ops->ndo_start_xmit(skb, dev);

I guess we could move dev_queue_xmit_nit(skb, dev) calls right before the ndo_start_xmit() calls...

From: Eric Dumazet <[email protected]>

dev_queue_xmit_nit() should be called right before ndo_start_xmit() calls or we might give wrong packet contents to taps users :

Packet checksum can be changed, or packet can be linearized or segmented, and segments partially sent for the later case.

Also a memory allocation can fail and packet never really hit the driver entry point.

---
 net/core/dev.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

index dcc673d..52cd1d7 100644
@@ -2213,9 +2213,6 @@ int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
 		if (dev->priv_flags & IFF_XMIT_DST_RELEASE)
 			skb_dst_drop(skb);
 
-		if (!list_empty(&ptype_all))
-			dev_queue_xmit_nit(skb, dev);
-
 		features = netif_skb_features(skb);
 
 		if (vlan_tx_tag_present(skb) &&

@@ -2250,6 +2247,9 @@ int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
 			}
 		}
 
+		if (!list_empty(&ptype_all))
+			dev_queue_xmit_nit(skb, dev);
+
 		skb_len = skb->len;
 		rc = ops->ndo_start_xmit(skb, dev);
 		trace_net_dev_xmit(skb, rc, dev, skb_len);

@@ -2272,6 +2272,9 @@ gso:
 		if (dev->priv_flags & IFF_XMIT_DST_RELEASE)
 			skb_dst_drop(nskb);
 
+		if (!list_empty(&ptype_all))
+			dev_queue_xmit_nit(nskb, dev);
+
 		skb_len = nskb->len;
 		rc = ops->ndo_start_xmit(nskb, dev);
 		trace_net_dev_xmit(nskb, rc, dev, skb_len);

Just to report. This patch fixed the invalid tcp tx checksum issue via tap for me. Thanks!

From: Eric Dumazet <[email protected]>
Date: Wed, 19 Sep 2012 08:44:49 +0200

Are you really sure that all the network tap implementations can handle software GSO segmented skbs using skb->next linkage?

Because that is what they can potentially see after this change.

I dont think so, because skb->next is NULL at the points I call the network tap.

Or did I miss something ?

From: Eric Dumazet <[email protected]>
Date: Wed, 19 Sep 2012 20:21:12 +0200

Indeed, you're right. I misread the control flow here.

Applied, thanks a lot Eric.