> From: Steffen Klassert <steffen.klass...@secunet.com> > Date: Mon, 18 Jan 2021 07:37:59 +0100 > > On Fri, Jan 15, 2021 at 05:12:33PM +0000, Alexander Lobakin wrote: >> From: Dongseok Yi <dseok...@samsung.com> >> Date: Fri, 15 Jan 2021 22:20:35 +0900 >> >>> UDP/IP header of UDP GROed frag_skbs are not updated even after NAT >>> forwarding. Only the header of head_skb from ip_finish_output_gso -> >>> skb_gso_segment is updated but following frag_skbs are not updated. >>> >>> A call path skb_mac_gso_segment -> inet_gso_segment -> >>> udp4_ufo_fragment -> __udp_gso_segment -> __udp_gso_segment_list >>> does not try to update UDP/IP header of the segment list but copy >>> only the MAC header. >>> >>> Update dport, daddr and checksums of each skb of the segment list >>> in __udp_gso_segment_list. It covers both SNAT and DNAT. >>> >>> Fixes: 9fd1ff5d2ac7 (udp: Support UDP fraglist GRO/GSO.) >>> Signed-off-by: Dongseok Yi <dseok...@samsung.com> >>> --- >>> v1: >>> Steffen Klassert said, there could be 2 options. >>> https://lore.kernel.org/patchwork/patch/1362257/ >>> I was trying to write a quick fix, but it was not easy to forward >>> segmented list. Currently, assuming DNAT only. >>> >>> v2: >>> Per Steffen Klassert request, move the procedure from >>> udp4_ufo_fragment to __udp_gso_segment_list and support SNAT. >>> >>> To Alexander Lobakin, I've checked your email late. Just use this >>> patch as a reference. It support SNAT too, but does not support IPv6 >>> yet. I cannot make IPv6 header changes in __udp_gso_segment_list due >>> to the file is in IPv4 directory. >> >> 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. If so, copy the full headers and fall back to the standard checksum, recalculation, else use the current path. Al
