On Sun, 20 Feb 2022, David Laight wrote:

> From: Paul Blakey
> > Sent: 20 February 2022 13:21
> > 
> > Ipv6 ttl, label and tos fields are modified without first
> > pulling/pushing the ipv6 header, which would have updated
> > the hw csum (if available). This might cause csum validation
> > when sending the packet to the stack, as can be seen in
> > the trace below.
> > 
> > Fix this by updating skb->csum if available.
> ...
> > +static inline __wsum
> > +csum_block_replace(__wsum csum, __wsum old, __wsum new, int offset)
> > +{
> > +   return csum_block_add(csum_block_sub(csum, old, offset), new, offset);
> > +}
> 
> That look computationally OTT for sub 32bit adjustments.
> 
> It ought to be enough to do:
>       return csum_add(old_csum, 0xf0000fff + new - old);
> 
> Although it will need 'tweaking' for odd aligned 24bit values.
> 
>       David
> 
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 
> 1PT, UK
> Registration No: 1397386 (Wales)
> 
> 

I'm not comfortable with manually implementing some low-level
bit fideling checksum calculation, while covering all cases.
And as you said it also doesnt have the offset param (odd aligned).
The same could be said about csum_block_add()/sub(), replace()
just uses them. I think the csum operations are more readable and
should  be inlined/optimize, so can we do it the readable way,
and then,  if needed, optimize it?
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to