Daniel Borkmann <daniel@xxxxxxxxxxxxx> 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.