On Mon, Mar 29, 2021 at 11:01 AM Paolo Abeni <pab...@redhat.com> wrote: > > On Mon, 2021-03-29 at 09:52 -0400, Willem de Bruijn wrote: > > > > 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 > > > + 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?
