Re: [ovs-dev] [PATCH net v4 1/1] openvswitch: Fix setting ipv6 fields causing hw csum failure
You'll need to rebase, the patch which made everything force inlined
got merged.
On Sun, 20 Feb 2022 15:21:14 +0200 Paul Blakey wrote:
> +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);
Why still _block? Since the arguments are pre-shifted we should be able
to subtract and add the entire thing, and the math will work out.
Obviously you'd need to shift by the offset in the word, not in a byte
in the caller.
> +}
> +
> static inline __wsum csum_unfold(__sum16 n)
> {
> return (__force __wsum)n;
> @@ -184,4 +190,5 @@ static inline __wsum wsum_negate(__wsum val)
> {
> return (__force __wsum)-((__force u32)val);
> }
> +
Spurious?
> #endif
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net v4 1/1] openvswitch: Fix setting ipv6 fields causing hw csum failure
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, 0xffff + 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)
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net v4 1/1] openvswitch: Fix setting ipv6 fields causing hw csum failure
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, 0xffff + 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
