Re: [PATCH net-next v2 1/8] udp: fixup csum for GSO receive slow path

On Mon, Mar 29, 2021 at 11:01 AM Paolo Abeni [off-list ref] wrote:

On Mon, 2021-03-29 at 09:52 -0400, Willem de Bruijn wrote:

quoted

quoted

quoted

That breaks the checksum-and-copy optimization when delivering to
local sockets. I wonder if that is a regression.

The conversion to CHECKSUM_UNNECESSARY happens since
commit 573e8fca255a27e3573b51f9b183d62641c47a3d.

Even the conversion to CHECKSUM_PARTIAL happens independently from this
series, since commit 6f1c0ea133a6e4a193a7b285efe209664caeea43.

I don't see a regression here ?!?

I mean that UDP packets with local destination socket and no tunnels
that arrive with CHECKSUM_NONE normally benefit from the
checksum-and-copy optimization in recvmsg() when copying to user.

If those packets are now checksummed during GRO, that voids that
optimization, and the packet payload is now touched twice.

The 'now' part confuses me. Nothing in this patch or this series
changes the processing of CHECKSUM_NONE UDP packets with no tunnel.

Agreed. I did not mean to imply that this patch changes that. I was
responding to

quoted

quoted

+       if (skb->ip_summed == CHECKSUM_NONE && !skb->csum_valid)
+               skb->csum_valid = 1;

Not entirely obvious is that UDP packets arriving on a device with rx
checksum offload off, i.e., with CHECKSUM_NONE, are not matched by
this test.

I assume that such packets are not coalesced by the GRO layer in the
first place. But I can't immediately spot the reason for it..

As you point out, such packets will already have had their checksum
verified at this point, so this branch only matches tunneled packets.
That point is just not immediately obvious from the code.

I do see checksum validation in the GRO engine for CHECKSUM_NONE UDP
packet prior to this series.

I *think* the checksum-and-copy optimization is lost
since 573e8fca255a27e3573b51f9b183d62641c47a3d.

Wouldn't this have been introduced with UDP_GRO?