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 ABFA442DF1; Fri, 7 Jul 2023 05:13:01 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 36BD9406B5; Fri, 7 Jul 2023 05:13:01 +0200 (CEST) Received: from mail-vs1-f47.google.com (mail-vs1-f47.google.com [209.85.217.47]) by mails.dpdk.org (Postfix) with ESMTP id A3A1540685 for ; Fri, 7 Jul 2023 05:12:59 +0200 (CEST) Received: by mail-vs1-f47.google.com with SMTP id ada2fe7eead31-440b54708f2so587402137.0 for ; Thu, 06 Jul 2023 20:12:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1688699579; x=1691291579; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=dQBNzinvazjLpyjcW0dOoxmASrre8aCjl6CqJp1taeE=; b=hQHS2OqrC/Nu9VMsHaamNx4gjao3ljqfi0JicbusMBX57zNeZDNeoahSnjgttr6puU SroD7FU4S1jLLs6z+SIb0nc84ktvsrLkL9N1CWbvdeG6JjIIYcmm5PGYHSUZPOnBSEQ4 TeqMrN1RnqlrZAqKl5sMajMX/lFXAOK9yOBXOzwHAX83kzX98MmJ8aewW16q9AvW5Uqe VrlLBkGXVwfi3A9o53V7Bs3rHURXQoLC+z3pr9czNYSixMRrl2d2ZGTNynY0DsOmMG1f VuqbWjZT9T7SKew3BaTX4/qaf0qKRB966GddIIKbf137cikv7Yyi8EdtRiB6+bVElr2C hFVA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1688699579; x=1691291579; h=content-transfer-encoding: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=dQBNzinvazjLpyjcW0dOoxmASrre8aCjl6CqJp1taeE=; b=XwRIhkV/JMw/wl+j/SrHGHLmaQruFIGskGuLrV5Wz15SgsjQytkkmGYjIOArj9Leri t5w/+8hBiwDtn0Tom+NrDQ7U1I23G+ExBXbAvQRWr3Pi2jB2aV3CnCkwFwOe5q8Ky30t lgalHDwtlM+fN6G0zM+ldOl7BRCfX1hPHzQgXBMveteHcSxPPW6vlzKtJC4IRMU9stcX 7wWhE0I/9rwD+NOWA35C4gywtOk0+yymrXe+m3xilIYBtHL05oGyV77HbIqrnBatdW7M B8MyqJQivddQ5gCv2abq0baU68L580LFDf3Iqjt7yRv8FPBQXa8EAL6LcxeHyT93o5lI dgOw== X-Gm-Message-State: ABy/qLbaa3RMY5Siev0OXwYbel5/857BBE4pvAvm1iN8Uqcd/CffKENm HMCDt1hDXlS/Wzg+mCARGNzDR3ZwdxIkbEOs1oE= X-Google-Smtp-Source: APBJJlFCmDuNpvr7WhTtvuo0W9ynsPOgEbBa0bz4WTnneKb49EpdYrsMwFlqvVDvu/jMvUdtLesYgSOqc9BCy7F/bk8= X-Received: by 2002:a05:6102:302e:b0:443:5d85:99f3 with SMTP id v14-20020a056102302e00b004435d8599f3mr695228vsa.7.1688699578847; Thu, 06 Jul 2023 20:12:58 -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 11:12:22 +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" Content-Transfer-Encoding: quoted-printable 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 |<- mb->pkt_len - sqh_len ->| |<- sa->hdr_len ->| |<- sa->hdr_l3_off ->| |<- udph->dgram_len ->| +--------------------+------------+-----+-----+---------+-----+ | ETH | IP | UDP | ESP | payload | sqh | +--------------------+------------+-----+-----+---------+-----+ |<- sa->hdr_l3_off ->|<- l3_len ->| |<- prm->tun.hdr_len ->| |<- sa->hdr_len ->| The figure above shows how udph->dgram_len =3D mb->pkt_len - sqh_len - sa->hdr_len + sizeof(struct rte_udp_hdr); and l3_len =3D prm->tun.hdr_len - sa->hdr_l3_off; Correct me if anything wrong. Thanks, Xiao Liang On Thu, Jul 6, 2023 at 6:20=E2=80=AFPM Radu Nicolau wrote: > > > On 06-Jul-23 10:08 AM, Konstantin Ananyev wrote: > > Hi Akhil, > > > >> Hi Konstantin, > >> Can you review this patch? > >> > >>> UDP header length is included in sa->hdr_len. Take care of that in > >>> L3 header and pakcet length calculation. > >>> > >>> Fixes: 01eef5907fc3 ("ipsec: support NAT-T") > >>> > >>> Signed-off-by: Xiao Liang > >>> --- > >>> lib/ipsec/esp_outb.c | 2 +- > >>> lib/ipsec/sa.c | 2 +- > >>> 2 files changed, 2 insertions(+), 2 deletions(-) > >>> > >>> 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 =3D (struct rte_udp_hdr *) > >>> (ph + sa->hdr_len - sizeof(struct rte_udp_hdr)); > >>> udph->dgram_len =3D rte_cpu_to_be_16(mb->pkt_len - sqh_le= n - > >>> - 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) > > Thanks > > Konstantin > > > >>> } > >>> > >>> /* 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 =3D 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); > >>> } > >>> -- > >>> 2.40.0