Subject: Re: [PATCH] net/sched: add ACT_CSUM action to update packets checksums

From: jamal <[email-masked]>
Date: Mon, 16 Aug 2010 18:00:19 -0400

On Mon, 2010-08-16 at 23:15 +0200, Grégoire Baron wrote:
> net/sched: add ACT_CSUM action to update packets checksums
> 
> ACT_CSUM can be called just after ACT_PEDIT in order to re-compute some
> altered checksums in IPv4 and IPv6 packets. The following checksums are
> supported by this patch:
>  - IPv4: IPv4 header, ICMP, IGMP, TCP, UDP & UDPLite
>  - IPv6: ICMPv6, TCP, UDP & UDPLite
> It's possible to request in the same action to update different kind of
> checksums, if the packets flow mix TCP, UDP and UDPLite, ...
> 
> An example of usage is done in the associated iproute2 patch.
> 
> Signed-off-by: Gregoire Baron <[email-masked]>

Excellent work! Ive always wanted to do this albeit slightly
differently.

I think it would be nice to factor a lot of the code repeated
everywhere into some boilerplate function that gets invoked by all;
users. Example, code such as:

+       struct icmphdr *icmph;
+       int hl;
+       int ntkoff;
+
+       ntkoff = skb_network_offset(skb);
+
+       hl = ihl + sizeof(*icmph);
+
+       if (!pskb_may_pull(skb, ipl + ntkoff) || (ipl < hl) ||
+           (skb_cloned(skb) &&
+            !skb_clone_writable(skb, hl + ntkoff) &&
+            pskb_expand_head(skb, 0, 0, GFP_ATOMIC)))
+               goto fail;
+
+       icmph = (void *)(skb_network_header(skb) + ihl);
+
+       icmph->checksum = 0;

cheers,
jamal