Re: [PATCH net v2] udp: ipv4: manipulate network header of NATed UDP GRO fraglist

On Mon, Jan 18, 2021 at 12:17:34PM +0000, Alexander Lobakin wrote:

quoted

From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Mon, 18 Jan 2021 07:37:59 +0100
On Fri, Jan 15, 2021 at 05:12:33PM +0000, Alexander Lobakin wrote:

quoted

I used another approach, tried to make fraglist GRO closer to plain
in terms of checksummming, as it is confusing to me why GSO packet
should have CHECKSUM_UNNECESSARY.

This is intentional. With fraglist GRO, we don't mangle packets
in the standard (non NAT) case. So the checksum is still correct
after segmentation. That is one reason why it has good forwarding
performance when software segmentation is needed. Checksuming
touches the whole packet and has a lot of overhead, so it is
heplfull to avoid it whenever possible.

We should find a way to do the checksum only when we really
need it. I.e. only if the headers of the head skb changed.

I suggest to do memcmp() between skb_network_header(skb) and
skb_network_header(skb->frag_list) with the len of
skb->data - skb_network_header(skb). This way we will detect changes
in IPv4/IPv6 and UDP headers.

I thought about that too. Bbut with fraglist GRO, the length of
the packets can vary. Unlike standard GRO, there is no requirement
that the packets in the fraglist must be equal in length here. So
we can't compare the full headers. I think we need to test for
addresses and ports.

If so, copy the full headers and fall back to the standard checksum,
recalculation, else use the current path.

I agree that we should fallback to standard checksum recalculation
if the addresses or ports changed.