> > > > > Additionally the segmented packets UDP CB still refers to the original > > > > > GSO packet len. Overall that causes unexpected/wrong csum validation > > > > > errors later in the UDP receive path. > > > > > > > > > > We could possibly address the issue with some additional checks and > > > > > csum mangling in the UDP tunnel code. Since the issue affects only > > > > > this UDP receive slow path, let's set a suitable csum status there. > > > > > > > > > > v1 -> v2: > > > > > - restrict the csum update to the packets strictly needing them > > > > > - hopefully clarify the commit message and code comments > > > > > > > > > > Signed-off-by: Paolo Abeni <pab...@redhat.com> > > > > > + 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.. > > > > > > Packets with CHECKSUM_NONE are actually aggregated by the GRO engine. > > > > > > Their checksum is validated by: > > > > > > udp4_gro_receive -> skb_gro_checksum_validate_zero_check() > > > -> __skb_gro_checksum_validate -> > > > __skb_gro_checksum_validate_complete() > > > > > > and skb->ip_summed is changed to CHECKSUM_UNNECESSARY by: > > > > > > __skb_gro_checksum_validate -> skb_gro_incr_csum_unnecessary > > > -> __skb_incr_checksum_unnecessary() > > > > > > and finally to CHECKSUM_PARTIAL by: > > > > > > udp4_gro_complete() -> udp_gro_complete() -> udp_gro_complete_segment() > > > > > > Do you prefer I resubmit with some more comments, either in the commit > > > message or in the code? > > > > 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.
