From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 142BB2C54; Mon, 8 Apr 2019 12:41:32 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 08 Apr 2019 03:41:31 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,325,1549958400"; d="scan'208";a="159199334" Received: from irsmsx102.ger.corp.intel.com ([163.33.3.155]) by fmsmga002.fm.intel.com with ESMTP; 08 Apr 2019 03:41:30 -0700 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.31]) by IRSMSX102.ger.corp.intel.com ([169.254.2.21]) with mapi id 14.03.0415.000; Mon, 8 Apr 2019 11:41:29 +0100 From: "Ananyev, Konstantin" To: Yongseok Koh , "Nicolau, Radu" , "akhil.goyal@nxp.com" CC: "dev@dpdk.org" , "aviadye@mellanox.com" , "stable@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH] examples/ipsec-secgw: fix coherency of ESP sequence number Thread-Index: AQHU7BCp/4xiWHRoIUO24J2qa/Ab0aYyFuEA Date: Mon, 8 Apr 2019 10:41:29 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772580148A943CB@irsmsx105.ger.corp.intel.com> References: <20190406003512.1291-1-yskoh@mellanox.com> In-Reply-To: <20190406003512.1291-1-yskoh@mellanox.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNDcyY2ZlNmQtNjk1MS00ZWVhLTlkZTktNDU1Yjc2YjM0YmNmIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiUlRPc0QrTm5WN1A3eDJIZHZBRllBNGZUUFlvd1RTV1dGWGllbXA5NUUweXJmc29sXC9DU0phSVhITkpOVkFyQjEifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.600.7 dlp-reaction: no-action x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] examples/ipsec-secgw: fix coherency of ESP sequence number X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 08 Apr 2019 10:41:33 -0000 Hi Yongseok, > Outbound ESP sequence number should be incremented atomically and refeenc= ed > indirectly. Otherwise, concurrent access by multiple threads will break > coherency. I think MT mode per SA is not supported right now by ipsec-secgw. >>From https://doc.dpdk.org/guides/sample_app_ug/ipsec_secgw.html: 49.2. Constraints ... Each SA must be handle by a unique lcore (1 RX queue per port). Also the changes you proposed wouldn't handle inbound case. Anyway, if you still like to have atomic sqn updates for outbound, then probably better to make it configurable (otherwise it would introduce unnecessary slowdown). There is '-a' (atomic) command-line option. Right now it works for librte_ipsec mode only, but I suppose it wouldn't be big deal to make legacy mode to take it into account too. >=20 > Fixes: d299106e8e31 ("examples/ipsec-secgw: add IPsec sample application"= ) > Fixes: 906257e965b7 ("examples/ipsec-secgw: support IPv6") > Fixes: cef50fc6f1e2 ("examples/ipsec-secgw: change CBC IV generation") > Fixes: 2a41fb7c6525 ("examples/ipsec-secgw: convert IV to big endian") > Fixes: ec17993a145a ("examples/ipsec-secgw: support security offload") > Cc: sergio.gonzalez.monroy@intel.com > Cc: aviadye@mellanox.com > Cc: akhil.goyal@nxp.com > Cc: stable@dpdk.org >=20 > Signed-off-by: Yongseok Koh > --- > examples/ipsec-secgw/esp.c | 13 +++++++------ > examples/ipsec-secgw/ipsec.h | 2 +- > examples/ipsec-secgw/sa.c | 2 +- > 3 files changed, 9 insertions(+), 8 deletions(-) >=20 > diff --git a/examples/ipsec-secgw/esp.c b/examples/ipsec-secgw/esp.c > index faa84ddd13..6f616f3f69 100644 > --- a/examples/ipsec-secgw/esp.c > +++ b/examples/ipsec-secgw/esp.c > @@ -222,6 +222,7 @@ esp_outbound(struct rte_mbuf *m, struct ipsec_sa *sa, > struct rte_crypto_sym_op *sym_cop; > int32_t i; > uint16_t pad_payload_len, pad_len, ip_hdr_len; > + uint64_t sa_seq; >=20 > RTE_ASSERT(m !=3D NULL); > RTE_ASSERT(sa !=3D NULL); > @@ -316,14 +317,14 @@ esp_outbound(struct rte_mbuf *m, struct ipsec_sa *s= a, > } > } >=20 > - sa->seq++; > esp->spi =3D rte_cpu_to_be_32(sa->spi); > - esp->seq =3D rte_cpu_to_be_32((uint32_t)sa->seq); > + sa_seq =3D rte_atomic64_add_return(&sa->seq, 1); > + esp->seq =3D rte_cpu_to_be_32((uint32_t)sa_seq); >=20 > /* set iv */ > uint64_t *iv =3D (uint64_t *)(esp + 1); > if (sa->aead_algo =3D=3D RTE_CRYPTO_AEAD_AES_GCM) { > - *iv =3D rte_cpu_to_be_64(sa->seq); > + *iv =3D rte_cpu_to_be_64(sa_seq); > } else { > switch (sa->cipher_algo) { > case RTE_CRYPTO_CIPHER_NULL: > @@ -332,7 +333,7 @@ esp_outbound(struct rte_mbuf *m, struct ipsec_sa *sa, > memset(iv, 0, sa->iv_len); > break; > case RTE_CRYPTO_CIPHER_AES_CTR: > - *iv =3D rte_cpu_to_be_64(sa->seq); > + *iv =3D rte_cpu_to_be_64(sa_seq); > break; > default: > RTE_LOG(ERR, IPSEC_ESP, > @@ -373,7 +374,7 @@ esp_outbound(struct rte_mbuf *m, struct ipsec_sa *sa, >=20 > struct cnt_blk *icb =3D get_cnt_blk(m); > icb->salt =3D sa->salt; > - icb->iv =3D rte_cpu_to_be_64(sa->seq); > + icb->iv =3D rte_cpu_to_be_64(sa_seq); > icb->cnt =3D rte_cpu_to_be_32(1); >=20 > aad =3D get_aad(m); > @@ -414,7 +415,7 @@ esp_outbound(struct rte_mbuf *m, struct ipsec_sa *sa, >=20 > struct cnt_blk *icb =3D get_cnt_blk(m); > icb->salt =3D sa->salt; > - icb->iv =3D rte_cpu_to_be_64(sa->seq); > + icb->iv =3D rte_cpu_to_be_64(sa_seq); > icb->cnt =3D rte_cpu_to_be_32(1); >=20 > switch (sa->auth_algo) { > diff --git a/examples/ipsec-secgw/ipsec.h b/examples/ipsec-secgw/ipsec.h > index 99f49d65f8..742d09aa36 100644 > --- a/examples/ipsec-secgw/ipsec.h > +++ b/examples/ipsec-secgw/ipsec.h > @@ -87,7 +87,7 @@ struct ipsec_sa { > struct rte_ipsec_session ips; /* one session per sa for now */ > uint32_t spi; > uint32_t cdev_id_qp; > - uint64_t seq; > + rte_atomic64_t seq; > uint32_t salt; > union { > struct rte_cryptodev_sym_session *crypto_session; > diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c > index a7298a30c2..adf6ac3f9a 100644 > --- a/examples/ipsec-secgw/sa.c > +++ b/examples/ipsec-secgw/sa.c > @@ -795,7 +795,7 @@ sa_add_rules(struct sa_ctx *sa_ctx, const struct ipse= c_sa entries[], > return -EINVAL; > } > *sa =3D entries[i]; > - sa->seq =3D 0; > + rte_atomic64_set(&sa->seq, -1); I think initial value should remain zero. Konstantin From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 6EEF2A0096 for ; Mon, 8 Apr 2019 12:41:35 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 47F533572; Mon, 8 Apr 2019 12:41:34 +0200 (CEST) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 142BB2C54; Mon, 8 Apr 2019 12:41:32 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 08 Apr 2019 03:41:31 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,325,1549958400"; d="scan'208";a="159199334" Received: from irsmsx102.ger.corp.intel.com ([163.33.3.155]) by fmsmga002.fm.intel.com with ESMTP; 08 Apr 2019 03:41:30 -0700 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.31]) by IRSMSX102.ger.corp.intel.com ([169.254.2.21]) with mapi id 14.03.0415.000; Mon, 8 Apr 2019 11:41:29 +0100 From: "Ananyev, Konstantin" To: Yongseok Koh , "Nicolau, Radu" , "akhil.goyal@nxp.com" CC: "dev@dpdk.org" , "aviadye@mellanox.com" , "stable@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH] examples/ipsec-secgw: fix coherency of ESP sequence number Thread-Index: AQHU7BCp/4xiWHRoIUO24J2qa/Ab0aYyFuEA Date: Mon, 8 Apr 2019 10:41:29 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772580148A943CB@irsmsx105.ger.corp.intel.com> References: <20190406003512.1291-1-yskoh@mellanox.com> In-Reply-To: <20190406003512.1291-1-yskoh@mellanox.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNDcyY2ZlNmQtNjk1MS00ZWVhLTlkZTktNDU1Yjc2YjM0YmNmIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiUlRPc0QrTm5WN1A3eDJIZHZBRllBNGZUUFlvd1RTV1dGWGllbXA5NUUweXJmc29sXC9DU0phSVhITkpOVkFyQjEifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.600.7 dlp-reaction: no-action x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] examples/ipsec-secgw: fix coherency of ESP sequence number X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Message-ID: <20190408104129.GCSiU2JSmlRkFLHhsodnxZftMO6BVxIFiS8ejZUguMk@z> Hi Yongseok, > Outbound ESP sequence number should be incremented atomically and refeenc= ed > indirectly. Otherwise, concurrent access by multiple threads will break > coherency. I think MT mode per SA is not supported right now by ipsec-secgw. >From https://doc.dpdk.org/guides/sample_app_ug/ipsec_secgw.html: 49.2. Constraints ... Each SA must be handle by a unique lcore (1 RX queue per port). Also the changes you proposed wouldn't handle inbound case. Anyway, if you still like to have atomic sqn updates for outbound, then probably better to make it configurable (otherwise it would introduce unnecessary slowdown). There is '-a' (atomic) command-line option. Right now it works for librte_ipsec mode only, but I suppose it wouldn't be big deal to make legacy mode to take it into account too. >=20 > Fixes: d299106e8e31 ("examples/ipsec-secgw: add IPsec sample application"= ) > Fixes: 906257e965b7 ("examples/ipsec-secgw: support IPv6") > Fixes: cef50fc6f1e2 ("examples/ipsec-secgw: change CBC IV generation") > Fixes: 2a41fb7c6525 ("examples/ipsec-secgw: convert IV to big endian") > Fixes: ec17993a145a ("examples/ipsec-secgw: support security offload") > Cc: sergio.gonzalez.monroy@intel.com > Cc: aviadye@mellanox.com > Cc: akhil.goyal@nxp.com > Cc: stable@dpdk.org >=20 > Signed-off-by: Yongseok Koh > --- > examples/ipsec-secgw/esp.c | 13 +++++++------ > examples/ipsec-secgw/ipsec.h | 2 +- > examples/ipsec-secgw/sa.c | 2 +- > 3 files changed, 9 insertions(+), 8 deletions(-) >=20 > diff --git a/examples/ipsec-secgw/esp.c b/examples/ipsec-secgw/esp.c > index faa84ddd13..6f616f3f69 100644 > --- a/examples/ipsec-secgw/esp.c > +++ b/examples/ipsec-secgw/esp.c > @@ -222,6 +222,7 @@ esp_outbound(struct rte_mbuf *m, struct ipsec_sa *sa, > struct rte_crypto_sym_op *sym_cop; > int32_t i; > uint16_t pad_payload_len, pad_len, ip_hdr_len; > + uint64_t sa_seq; >=20 > RTE_ASSERT(m !=3D NULL); > RTE_ASSERT(sa !=3D NULL); > @@ -316,14 +317,14 @@ esp_outbound(struct rte_mbuf *m, struct ipsec_sa *s= a, > } > } >=20 > - sa->seq++; > esp->spi =3D rte_cpu_to_be_32(sa->spi); > - esp->seq =3D rte_cpu_to_be_32((uint32_t)sa->seq); > + sa_seq =3D rte_atomic64_add_return(&sa->seq, 1); > + esp->seq =3D rte_cpu_to_be_32((uint32_t)sa_seq); >=20 > /* set iv */ > uint64_t *iv =3D (uint64_t *)(esp + 1); > if (sa->aead_algo =3D=3D RTE_CRYPTO_AEAD_AES_GCM) { > - *iv =3D rte_cpu_to_be_64(sa->seq); > + *iv =3D rte_cpu_to_be_64(sa_seq); > } else { > switch (sa->cipher_algo) { > case RTE_CRYPTO_CIPHER_NULL: > @@ -332,7 +333,7 @@ esp_outbound(struct rte_mbuf *m, struct ipsec_sa *sa, > memset(iv, 0, sa->iv_len); > break; > case RTE_CRYPTO_CIPHER_AES_CTR: > - *iv =3D rte_cpu_to_be_64(sa->seq); > + *iv =3D rte_cpu_to_be_64(sa_seq); > break; > default: > RTE_LOG(ERR, IPSEC_ESP, > @@ -373,7 +374,7 @@ esp_outbound(struct rte_mbuf *m, struct ipsec_sa *sa, >=20 > struct cnt_blk *icb =3D get_cnt_blk(m); > icb->salt =3D sa->salt; > - icb->iv =3D rte_cpu_to_be_64(sa->seq); > + icb->iv =3D rte_cpu_to_be_64(sa_seq); > icb->cnt =3D rte_cpu_to_be_32(1); >=20 > aad =3D get_aad(m); > @@ -414,7 +415,7 @@ esp_outbound(struct rte_mbuf *m, struct ipsec_sa *sa, >=20 > struct cnt_blk *icb =3D get_cnt_blk(m); > icb->salt =3D sa->salt; > - icb->iv =3D rte_cpu_to_be_64(sa->seq); > + icb->iv =3D rte_cpu_to_be_64(sa_seq); > icb->cnt =3D rte_cpu_to_be_32(1); >=20 > switch (sa->auth_algo) { > diff --git a/examples/ipsec-secgw/ipsec.h b/examples/ipsec-secgw/ipsec.h > index 99f49d65f8..742d09aa36 100644 > --- a/examples/ipsec-secgw/ipsec.h > +++ b/examples/ipsec-secgw/ipsec.h > @@ -87,7 +87,7 @@ struct ipsec_sa { > struct rte_ipsec_session ips; /* one session per sa for now */ > uint32_t spi; > uint32_t cdev_id_qp; > - uint64_t seq; > + rte_atomic64_t seq; > uint32_t salt; > union { > struct rte_cryptodev_sym_session *crypto_session; > diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c > index a7298a30c2..adf6ac3f9a 100644 > --- a/examples/ipsec-secgw/sa.c > +++ b/examples/ipsec-secgw/sa.c > @@ -795,7 +795,7 @@ sa_add_rules(struct sa_ctx *sa_ctx, const struct ipse= c_sa entries[], > return -EINVAL; > } > *sa =3D entries[i]; > - sa->seq =3D 0; > + rte_atomic64_set(&sa->seq, -1); I think initial value should remain zero. Konstantin