From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 24C9A42DF1; Fri, 7 Jul 2023 04:07:27 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id AAEB2406B5; Fri, 7 Jul 2023 04:07:26 +0200 (CEST) Received: from mail-vs1-f44.google.com (mail-vs1-f44.google.com [209.85.217.44]) by mails.dpdk.org (Postfix) with ESMTP id 7B68B40685 for ; Fri, 7 Jul 2023 04:07:25 +0200 (CEST) Received: by mail-vs1-f44.google.com with SMTP id ada2fe7eead31-44096f01658so571238137.0 for ; Thu, 06 Jul 2023 19:07:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1688695645; x=1691287645; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=Xr2Uj2enzUzd9cAarnyIRxojeXZ77kKPtYef2DLdwYw=; b=i4FBj65/RQ2Ho23Y1ujkihUhpc2ZoIHz5mUfWyr3iRGo01CrTRpGKCI1E0AdVkMeH2 +/4Un2x71VGSMuZWcuefzxNXIZJcOMV8d8UzAF/nmAf7SqdqX5z+QcGX2Zx/BGWFcm/N 0L+2zcsszoxaBRf935PGvtdVFy4HOM7E5OjYtRJQm2y66rcFq/nmaiqe82zoLxNMnN0h PZufpU6ECQqRBrN5lhFQENmIDGwM/nsWeIEDLOuoXzNisghJ7/7qoReDPo3ab48lgQDm BJEomROb8+fy6OgkFJ2MF0DyFO7fwPX4848tlmXqf2jf5fyy7C/9lKRNJJtXYqcm2POW VKAg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1688695645; x=1691287645; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=Xr2Uj2enzUzd9cAarnyIRxojeXZ77kKPtYef2DLdwYw=; b=FwpG5ljMgoeyuET8zz0nkSsqOEtGRTID5n8g7ZXYM4KHrCelYU/PfRlXhp+WOeJb2w jSYgU/nM+BDg4Vqwn5g0Klq3ZKa4q8Ntuvs9tzhlShkSD5wOO36hO778fIspcfCLysSv UvhRL7DShzt89WG4lGVlQLmXVU07vSSRhCNysG+vWg0b4Wr1pGGAqkGZDeOTQgdw09b0 EPSZ8yX/gpKR0zFDZC6wGv9V+57mQTAHMobbscL8Y840mP7DxS3XlVIxjdmh3P+bjH3m 1A/l4dJWMLbChqV6E7pqcTvSUMW0f7LGeSplueciOjSoQL8z3kt5XRNJyxlJbii+bxhu gEDA== X-Gm-Message-State: ABy/qLbIxU0B1LS+LFVxBHu+axU6iUoZmb6gEG0NSgqctuYktaaaugik tq6MoE10Oc6h8yeXIz9da4P2EO+yaXBYGu/JyAs= X-Google-Smtp-Source: APBJJlG35mGfH7tKN9U7ooCIymJBj3ltTMy7WnRxINs7Y/ivk5zFZkBEDXhjnFlWcWZjCnlxw+IqJ8cAiWrKQByIgxQ= X-Received: by 2002:a05:6102:44a:b0:445:1e76:c58b with SMTP id e10-20020a056102044a00b004451e76c58bmr2601752vsq.21.1688695644842; Thu, 06 Jul 2023 19:07:24 -0700 (PDT) MIME-Version: 1.0 References: <20230418084613.52740-1-shaw.leon@gmail.com> <05b87294-0b96-804c-f4e7-0fc3b3ccbbf4@intel.com> In-Reply-To: <05b87294-0b96-804c-f4e7-0fc3b3ccbbf4@intel.com> From: Xiao Liang Date: Fri, 7 Jul 2023 10:06:48 +0800 Message-ID: Subject: Re: [EXT] [PATCH] ipsec: fix NAT-T length calculation To: Radu Nicolau Cc: Konstantin Ananyev , Akhil Goyal , Konstantin Ananyev , Vladimir Medvedkin , "dev@dpdk.org" Content-Type: text/plain; charset="UTF-8" X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Well, let me explain. > >>> diff --git a/lib/ipsec/esp_outb.c b/lib/ipsec/esp_outb.c > >>> index 9cbd9202f6..ec87b1dce2 100644 > >>> --- a/lib/ipsec/esp_outb.c > >>> +++ b/lib/ipsec/esp_outb.c > >>> @@ -198,7 +198,7 @@ outb_tun_pkt_prepare(struct rte_ipsec_sa *sa, > >>> rte_be64_t sqc, > >>> struct rte_udp_hdr *udph = (struct rte_udp_hdr *) > >>> (ph + sa->hdr_len - sizeof(struct rte_udp_hdr)); > >>> udph->dgram_len = rte_cpu_to_be_16(mb->pkt_len - sqh_len - > >>> - sa->hdr_l3_off - sa->hdr_len); > >>> + sa->hdr_len + sizeof(struct rte_udp_hdr)); > > To be honest, it is not clear to me why we shouldn't take into account sa->hdr_l3_off > > any more. > > Probably the author can explain. > > Also would like author of NAT-T support to chime in. > > Radu, any comments on that patch? > I agree, hdr_l3_off should not be ignored. Also sa->hdr_len already > includes the size of UDP header, see line 366 in esp_outb_tun_init in > sa.c (or the line above this change, where the udph pointer is computed > assuming this) A few lines above, there is: ph = rte_pktmbuf_prepend(mb, hlen - l2len); The L2 header is already pulled from the packet, then the packet length has nothing to do with hdr_l3_off from this point, so use encap header length instead. You are right sa->hdr_len already includes the size of UDP header, and that's why it should be added back here. > >>> } > >>> > >>> /* update original and new ip header fields */ > >>> diff --git a/lib/ipsec/sa.c b/lib/ipsec/sa.c > >>> index 59a547637d..2297bd6d72 100644 > >>> --- a/lib/ipsec/sa.c > >>> +++ b/lib/ipsec/sa.c > >>> @@ -371,7 +371,7 @@ esp_outb_tun_init(struct rte_ipsec_sa *sa, const struct > >>> rte_ipsec_sa_prm *prm) > >>> > >>> /* update l2_len and l3_len fields for outbound mbuf */ > >>> sa->tx_offload.val = rte_mbuf_tx_offload(sa->hdr_l3_off, > >>> - sa->hdr_len - sa->hdr_l3_off, 0, 0, 0, 0, 0); > >>> + prm->tun.hdr_len - sa->hdr_l3_off, 0, 0, 0, 0, 0); > >>> > >>> esp_outb_init(sa, sa->hdr_len, prm->ipsec_xform.esn.value); > >>> } Again sa->hdr_len includes UDP header, which is not part of L3, so use the original prm->tun.hdr_len. Thanks, Xiao Liang