Re: [PATCH net v3 3/4] udp: do not transition UDP GRO fraglist partial checksums to unnecessary

[Antoine Tenart <atenart@kernel.org>]

Quoting Willem de Bruijn (2024-03-25 19:39:46)
> Antoine Tenart wrote:
> > Quoting Willem de Bruijn (2024-03-22 18:29:40)
> > > 
> > > Should fraglist UDP GRO and non-fraglist (udp_gro_complete_segment)
> > > have the same checksumming behavior?
> > 
> > They can't as non-fraglist GRO packets can be aggregated, csum can't
> > just be converted there.
> 
> I suppose this could be done. But it is just simpler to convert to
> CHECKSUM_UNNECESSARY.

Oh, do you mean using the non-fraglist behavior in fraglist?
udp_gro_complete_segment converts all packets to CHECKSUM_PARTIAL (as
packets could have been aggregated) but that's not required in fraglist.

To say it another way: my understanding is packets in the non-fraglist
case have to be converted to CHECKSUM_PARTIAL, while the fraglist case
can keep the checksum info as-is (and have the conversion to unnecessary
as an optimization when applicable).

> You mean that on segmentation, the segments are restored and thus
> skb->csum of each segment is again correct, right?

In the fraglist case, yes.

> I suppose this could be converted to CHECKSUM_UNNECESSARY if just
> for equivalence between the two UDP_GRO methods and simplicity.
> 
> But also fine to leave as is.

I'm not sure I got your suggestion as I don't see how non-fraglist
packets could be converted to CHECKSUM_UNNECESSARY when being aggregated
(uh->len can change there).

This series is aiming at fixing known issues and unifying the behavior
would be net-next material IMO. I'll send a v4 for those fixes, but then
I'm happy to discuss the above suggestion and investigate; so let's
continue the discussion here in parallel.

> Can you at least summarize this in the commit message? Currently
> CHECKSUM_COMPLETE is not mentioned, but the behavior is not trivial.
> It may be helpful next time we again stumble on this code and do a
> git blame.

Sure, I'll try to improve the commit log.

Thanks!
Antoine