From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C57CCC433E0 for ; Mon, 18 Jan 2021 12:22:01 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 80CFC222B3 for ; Mon, 18 Jan 2021 12:22:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2391425AbhARMVr (ORCPT ); Mon, 18 Jan 2021 07:21:47 -0500 Received: from mail1.protonmail.ch ([185.70.40.18]:44291 "EHLO mail1.protonmail.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2391247AbhARMSf (ORCPT ); Mon, 18 Jan 2021 07:18:35 -0500 Date: Mon, 18 Jan 2021 12:17:34 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pm.me; s=protonmail; t=1610972263; bh=7UZIzTfB4fZUh0LfKdB0wEPFEMIHF3bWhX9FdPzdAaE=; h=Date:To:From:Cc:Reply-To:Subject:In-Reply-To:References:From; b=pcbve/zhJHHnPC6G87DsIHZRVvmDTB8sQVPS5IYdVMFRiFrdHcucxG5d1vC33OiFj A1ROQje7/utT1Wfvbtyo8Gz0h8L7MZRkaKL5rrXrC4eqhEB/AXYeUjaHRgHyOIU1Dm kmh/Gcg791KRxhy024EoqB1ObMAJEzWnhMczxBgYldR7r8XjCJqkz5LHok9EjL++FL KtinPlnkc4UpXSvRfCChIQku105/c5BuaBre29OXI65rQsSLfpgvawoeC0TBeIdNvc EQoCAj/EyrvocL6Gz6uMR5k9FNAywEOMVKN5cqYiKfcvW5CL4Lg1IM5lcu7BTQ6n7H KJ9D9mI3reEWw== To: Steffen Klassert From: Alexander Lobakin Cc: Alexander Lobakin , Dongseok Yi , "David S. Miller" , namkyu78.kim@samsung.com, Jakub Kicinski , Hideaki YOSHIFUJI , Willem de Bruijn , netdev@vger.kernel.org, linux-kernel@vger.kernel.org Reply-To: Alexander Lobakin Subject: Re: [PATCH net v2] udp: ipv4: manipulate network header of NATed UDP GRO fraglist Message-ID: <20210118121707.2130-1-alobakin@pm.me> In-Reply-To: <20210118063759.GK3576117@gauss3.secunet.de> References: <1610716836-140533-1-git-send-email-dseok.yi@samsung.com> <20210115171203.175115-1-alobakin@pm.me> <20210118063759.GK3576117@gauss3.secunet.de> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > From: Steffen Klassert > 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 >> 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 >>> --- >>> 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