Messages in this thread | | | Date | Wed, 22 Jan 2020 12:43:33 +0100 | From | Florian Westphal <> | Subject | Re: [PATCH v3] [net]: Fix skb->csum update in inet_proto_csum_replace16(). |
| |
Daniel Borkmann <daniel@iogearbox.net> wrote: > > @@ -449,9 +464,6 @@ void inet_proto_csum_replace16(__sum16 *sum, struct sk_buff *skb, > > if (skb->ip_summed != CHECKSUM_PARTIAL) { > > *sum = csum_fold(csum_partial(diff, sizeof(diff), > > ~csum_unfold(*sum))); > > - if (skb->ip_summed == CHECKSUM_COMPLETE && pseudohdr) > > - skb->csum = ~csum_partial(diff, sizeof(diff), > > - ~skb->csum); > > What is the technical rationale in removing this here but not in any of the > other inet_proto_csum_replace*() functions? You changelog has zero analysis > on why here but not elsewhere this change would be needed?
Right, I think it could be dropped everywhere BUT there is a major caveat:
At least for the nf_nat case ipv4 header manipulation (which uses the other helpers froum utils.c) will eventually also update iph->checksum field to account for the changed ip addresses.
And that update doesn't touch skb->csum.
So in a way the update of skb->csum in the other helpers indirectly account for later ip header checksum update.
At least that was my conclusion when reviewing the earlier incarnation of the patch.
| |