Bug in pskb_trim_rcsum()

Since network device can auto calculate and verify the checksum of a
packet, for example: some e1000 interface. Different device will set different value of skb->ip_summed.
a) If device do nothing to checksum, skb->ip_summed would be set to
CHECKSUM_NONE.
b) If device can only calculate a checksum, and the checksum is
correct, skb->ip_summed would be set to CHECKSUM_HW.
c) If device can verify the checksum, and the checksum is correct,
skb->ip_summed would be set to CHECKSUM_UNNECESSARY.
So if I want to trim a skb, I think I must do a checksum even if the
skb->ip_summed is CHECKSUM_UNNECESSARY.

Following is the comment about CHECKSUM_UNNECESSARY in include/linux/skbuff.h:
* UNNECESSARY: device parsed packet and wouldbe verified checksum.
* skb->csum is undefined.

 *	      It is bad option, but, unfortunately, many of vendors do this.
 *	      Apparently with secret goal to sell you new device, when you
 *	      will add new protocol to your host. F.e. IPv6. 8)

@@ -1208,8 +1208,8 @@ static inline int pskb_trim_rcsum(struct
 {
 	if (likely(len >= skb->len))
 		return 0;
-	if (skb->ip_summed == CHECKSUM_HW)
-		skb->ip_summed = CHECKSUM_NONE;
+
+	skb->ip_summed = CHECKSUM_NONE;
 	return __pskb_trim(skb, len);
 }
 

Nope. CHECKSUM_UNNECESSARY means that the hardware has already verified the checksum to be correct for the given protocol. Trimming a packet does not affect it.

Cheers,

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

The whole point of CHECKSUM_UNNECESSARY is that the hardware parses the protocol header for us. So in this case it must calculate the checksum for only the first 8 bytes of the payload.

If it does this incorrectly, then it doesn't support RX checksums at all.

Which NIC is doing this BTW?

Cheers,

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Hello!

I remember this place, I stalled there for a minute ages ago.

He is right in his doubts, the place is confusing, device do it wrong (but they are innocent, it is just a flaw of UDP, people did not know what to do with redundant two octets in header and placed some useless thing there :-)) and if we want to be paranoid about checksum, we should really clear CHECKSUM_UNNECESSARY tag and to recheck.

I preferred optimistic approach: if the checksum comes out correct, we do not really care, how device calculated it. Probably, it calculated checksum over wrong data, but got a good checksum. So what? It is not a crypto digest yet. And if device found wrong checksum, we will recalculate it anyway.

I would like to add that CHECKSUM_UNNECESSARY can be used, when checksum is really wrong (on loopback), that's why it is not cleared, when trimming. CHECKSUM_HW can always fall back to CHECKSUM_NONE, but CHECKSUM_UNNECESSARY cannot. Probably, this was bad idea, but it still means that if some generic function starts to clear it, all the code using it should be reverified.

Alexey

Agreed.

Actually, I plan to differentiate between RX CHECKSUM_HW and TX CHECKSUM_HW. Now that we have things like Xen it is possible for RX packets to have patial checksums too.

When this is done loopback can send TX CHECKSUM_HW packets instead of CHECKSUM_UNNECESSARY (I'm currently calling this CHECKSUM_PARTIAL).

Cheers,

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

From: Herbert Xu <[email protected]>
Date: Tue, 18 Jul 2006 23:04:04 +1000

This sounds like a good idea.

Undoing CHECKSUM_UNNECESSARY when trimming is definitely going to break loopback the way things are done currently.