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 814CE459AF; Tue, 17 Sep 2024 19:13:07 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 547AF40280; Tue, 17 Sep 2024 19:13:07 +0200 (CEST) Received: from frasgout.his.huawei.com (frasgout.his.huawei.com [185.176.79.56]) by mails.dpdk.org (Postfix) with ESMTP id 9BF1240261 for ; Tue, 17 Sep 2024 19:13:06 +0200 (CEST) Received: from mail.maildlp.com (unknown [172.18.186.31]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4X7Syy0DWSz6K5df; Wed, 18 Sep 2024 01:12:58 +0800 (CST) Received: from frapeml100007.china.huawei.com (unknown [7.182.85.133]) by mail.maildlp.com (Postfix) with ESMTPS id DBB0E140558; Wed, 18 Sep 2024 01:13:05 +0800 (CST) Received: from frapeml500007.china.huawei.com (7.182.85.172) by frapeml100007.china.huawei.com (7.182.85.133) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.39; Tue, 17 Sep 2024 19:13:02 +0200 Received: from frapeml500007.china.huawei.com ([7.182.85.172]) by frapeml500007.china.huawei.com ([7.182.85.172]) with mapi id 15.01.2507.039; Tue, 17 Sep 2024 19:13:02 +0200 From: Konstantin Ananyev To: Aakash Sasidharan , Konstantin Ananyev , Vladimir Medvedkin CC: "gakhil@marvell.com" , "jerinj@marvell.com" , "anoobj@marvell.com" , "vvelumuri@marvell.com" , "dev@dpdk.org" Subject: RE: [PATCH v2] ipsec: allow stateless IPsec processing Thread-Topic: [PATCH v2] ipsec: allow stateless IPsec processing Thread-Index: AQHbAeZp/OeYu0yqDk2AJ/iP9MBeaLJcQPVA Date: Tue, 17 Sep 2024 17:13:01 +0000 Message-ID: <692b6f6259a0429bae57826d3ec08dc8@huawei.com> References: <20240907102524.2686767-1-asasidharan@marvell.com> <20240908115733.2708942-1-asasidharan@marvell.com> In-Reply-To: <20240908115733.2708942-1-asasidharan@marvell.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.206.138.42] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 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 > Introduce stateless packet preparation API for IPsec > processing. The new API would allow preparation of IPsec > packets without altering the internal state of an IPsec > session. >=20 > For outbound IPsec processing, the change enables user to > provide sequence number to be used for the IPsec operation. Few questions/nits below. As a generic one - we need to add a use-case/test-case for it. Without it I think the patch is incomplete. >=20 > Signed-off-by: Aakash Sasidharan > --- > lib/ipsec/esp_outb.c | 85 +++++++++++++++++++++++++++++++------------ > lib/ipsec/rte_ipsec.h | 68 ++++++++++++++++++++++++++++++++++ > lib/ipsec/sa.c | 4 +- > lib/ipsec/sa.h | 8 ++++ > 4 files changed, 140 insertions(+), 25 deletions(-) >=20 > diff --git a/lib/ipsec/esp_outb.c b/lib/ipsec/esp_outb.c > index ec87b1dce2..de28cb166d 100644 > --- a/lib/ipsec/esp_outb.c > +++ b/lib/ipsec/esp_outb.c > @@ -288,13 +288,12 @@ outb_pkt_xprepare(const struct rte_ipsec_sa *sa, rt= e_be64_t sqc, > /* > * setup/update packets and crypto ops for ESP outbound tunnel case. > */ > -uint16_t > -esp_outb_tun_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf= *mb[], > - struct rte_crypto_op *cop[], uint16_t num) > +static inline uint16_t > +esp_outb_tun_prepare_helper(const struct rte_ipsec_session *ss, struct r= te_mbuf *mb[], > + struct rte_crypto_op *cop[], uint16_t num, uint64_t sqn) > { > int32_t rc; > - uint32_t i, k, n; > - uint64_t sqn; > + uint32_t i, k, n =3D num; You can just do s/num/n/ in function parameter list, then you don't need to= keep 'num' at all. > rte_be64_t sqc; > struct rte_ipsec_sa *sa; > struct rte_cryptodev_sym_session *cs; > @@ -305,11 +304,6 @@ esp_outb_tun_prepare(const struct rte_ipsec_session = *ss, struct rte_mbuf *mb[], > sa =3D ss->sa; > cs =3D ss->crypto.ses; >=20 > - n =3D num; > - sqn =3D esn_outb_update_sqn(sa, &n); > - if (n !=3D num) > - rte_errno =3D EOVERFLOW; > - > k =3D 0; > for (i =3D 0; i !=3D n; i++) { >=20 > @@ -339,6 +333,30 @@ esp_outb_tun_prepare(const struct rte_ipsec_session = *ss, struct rte_mbuf *mb[], > return k; > } >=20 > +uint16_t > +esp_outb_tun_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf= *mb[], > + struct rte_crypto_op *cop[], uint16_t num) > +{ > + uint64_t sqn; > + uint32_t n; > + > + n =3D num; > + sqn =3D esn_outb_update_sqn(ss->sa, &n); > + if (n !=3D num) > + rte_errno =3D EOVERFLOW; > + > + return esp_outb_tun_prepare_helper(ss, mb, cop, n, sqn); > +} > + > +uint16_t > +esp_outb_tun_prepare_stateless(const struct rte_ipsec_session *ss, struc= t rte_mbuf *mb[], > + struct rte_crypto_op *cop[], uint16_t num, struct rte_ipsec_state *stat= e) > +{ > + uint64_t sqn =3D state->sqn; > + > + return esp_outb_tun_prepare_helper(ss, mb, cop, num, sqn); > +} > + > /* > * setup/update packet data and metadata for ESP outbound transport case= . > */ > @@ -529,16 +547,15 @@ outb_cpu_crypto_prepare(const struct rte_ipsec_sa *= sa, uint32_t *pofs, > return clen; > } >=20 > -static uint16_t > -cpu_outb_pkt_prepare(const struct rte_ipsec_session *ss, > - struct rte_mbuf *mb[], uint16_t num, > - esp_outb_prepare_t prepare, uint32_t cofs_mask) > +static inline uint16_t > +cpu_outb_pkt_prepare_helper(const struct rte_ipsec_session *ss, > + struct rte_mbuf *mb[], uint16_t num, esp_outb_prepare_t prepare, > + uint32_t cofs_mask, uint64_t sqn) > { > int32_t rc; > - uint64_t sqn; > rte_be64_t sqc; > struct rte_ipsec_sa *sa; > - uint32_t i, k, n; > + uint32_t i, k, n =3D num; Same here, you can just use 'n' instead of 'num'. > uint32_t l2, l3; > union sym_op_data icv; > struct rte_crypto_va_iova_ptr iv[num]; > @@ -551,11 +568,6 @@ cpu_outb_pkt_prepare(const struct rte_ipsec_session = *ss, >=20 > sa =3D ss->sa; >=20 > - n =3D num; > - sqn =3D esn_outb_update_sqn(sa, &n); > - if (n !=3D num) > - rte_errno =3D EOVERFLOW; > - > for (i =3D 0, k =3D 0; i !=3D n; i++) { >=20 > l2 =3D mb[i]->l2_len; > @@ -604,15 +616,40 @@ uint16_t > cpu_outb_tun_pkt_prepare(const struct rte_ipsec_session *ss, > struct rte_mbuf *mb[], uint16_t num) > { > - return cpu_outb_pkt_prepare(ss, mb, num, outb_tun_pkt_prepare, 0); > + uint64_t sqn; > + uint32_t n; > + > + n =3D num; > + sqn =3D esn_outb_update_sqn(ss->sa, &n); > + if (n !=3D num) > + rte_errno =3D EOVERFLOW; > + > + return cpu_outb_pkt_prepare_helper(ss, mb, n, outb_tun_pkt_prepare, 0, = sqn); > +} > + > +uint16_t > +cpu_outb_tun_pkt_prepare_stateless(const struct rte_ipsec_session *ss, > + struct rte_mbuf *mb[], uint16_t num, struct rte_ipsec_state *state) > +{ > + uint64_t sqn =3D state->sqn; > + > + return cpu_outb_pkt_prepare_helper(ss, mb, num, outb_tun_pkt_prepare, 0= , sqn); > } >=20 > uint16_t > cpu_outb_trs_pkt_prepare(const struct rte_ipsec_session *ss, > struct rte_mbuf *mb[], uint16_t num) > { > - return cpu_outb_pkt_prepare(ss, mb, num, outb_trs_pkt_prepare, > - UINT32_MAX); > + uint64_t sqn; > + uint32_t n; > + > + n =3D num; > + sqn =3D esn_outb_update_sqn(ss->sa, &n); > + if (n !=3D num) > + rte_errno =3D EOVERFLOW; > + > + return cpu_outb_pkt_prepare_helper(ss, mb, n, outb_trs_pkt_prepare, > + UINT32_MAX, sqn); > } >=20 > /* > diff --git a/lib/ipsec/rte_ipsec.h b/lib/ipsec/rte_ipsec.h > index f15f6f2966..b462068203 100644 > --- a/lib/ipsec/rte_ipsec.h > +++ b/lib/ipsec/rte_ipsec.h > @@ -23,11 +23,26 @@ extern "C" { >=20 > struct rte_ipsec_session; >=20 > +/** > + * IPsec state for stateless processing of a batch of IPsec packets. > + */ > +struct rte_ipsec_state { > + union { Curious what is the purpose of 'union' here? What other mutually exclusive fields you plan to add here? > + /** > + * 64 bit sequence number to be used for the first packet of the > + * batch of packets. > + */ > + uint64_t sqn; > + }; > +}; > + > /** > * IPsec session specific functions that will be used to: > * - prepare - for input mbufs and given IPsec session prepare crypto op= s > * that can be enqueued into the cryptodev associated with given sessi= on > * (see *rte_ipsec_pkt_crypto_prepare* below for more details). > + * - prepare_stateless - similar to prepare, but further processing is d= one > + * based on IPsec state provided by the 'state' parameter. > * - process - finalize processing of packets after crypto-dev finished > * with them or process packets that are subjects to inline IPsec offl= oad > * (see rte_ipsec_pkt_process for more details). > @@ -42,6 +57,17 @@ struct rte_ipsec_sa_pkt_func { > struct rte_mbuf *mb[], > uint16_t num); > } prepare; > + union { > + uint16_t (*async)(const struct rte_ipsec_session *ss, > + struct rte_mbuf *mb[], > + struct rte_crypto_op *cop[], > + uint16_t num, > + struct rte_ipsec_state *state); > + uint16_t (*sync)(const struct rte_ipsec_session *ss, > + struct rte_mbuf *mb[], > + uint16_t num, > + struct rte_ipsec_state *state); > + } prepare_stateless; > uint16_t (*process)(const struct rte_ipsec_session *ss, > struct rte_mbuf *mb[], > uint16_t num); > @@ -128,6 +154,48 @@ rte_ipsec_pkt_cpu_prepare(const struct rte_ipsec_ses= sion *ss, > return ss->pkt_func.prepare.sync(ss, mb, num); > } >=20 > +/** > + * Same as rte_ipsec_pkt_crypto_prepare, but processing is done based on > + * IPsec state provided by the 'state' parameter. Internal IPsec state w= on't > + * be updated when this API is called. > + * > + * For input mbufs and given IPsec session prepare crypto ops that can b= e > + * enqueued into the cryptodev associated with given session. > + * expects that for each input packet: > + * - l2_len, l3_len are setup correctly > + * Note that erroneous mbufs are not freed by the function, > + * but are placed beyond last valid mbuf in the *mb* array. > + * It is a user responsibility to handle them further. > + * @param ss > + * Pointer to the *rte_ipsec_session* object the packets belong to. > + * @param mb > + * The address of an array of *num* pointers to *rte_mbuf* structures > + * which contain the input packets. > + * @param cop > + * The address of an array of *num* pointers to the output *rte_crypto= _op* > + * structures. > + * @param num > + * The maximum number of packets to process. > + * @param state > + * The IPsec state to be used for processing current batch of packets. > + * @return > + * Number of successfully processed packets, with error code set in rt= e_errno. > + */ We probably need to mark new API as 'rte_experimental'. > +static inline uint16_t > +rte_ipsec_pkt_crypto_prepare_stateless(const struct rte_ipsec_session *s= s, > + struct rte_mbuf *mb[], struct rte_crypto_op *cop[], uint16_t num, > + struct rte_ipsec_state *state) > +{ > + return ss->pkt_func.prepare_stateless.async(ss, mb, cop, num, state); > +} > + > +static inline uint16_t > +rte_ipsec_pkt_cpu_prepare_stateless(const struct rte_ipsec_session *ss, > + struct rte_mbuf *mb[], uint16_t num, struct rte_ipsec_state *state) > +{ > + return ss->pkt_func.prepare_stateless.sync(ss, mb, num, state); > +} > + > /** > * Finalise processing of packets after crypto-dev finished with them or > * process packets that are subjects to inline IPsec offload. > diff --git a/lib/ipsec/sa.c b/lib/ipsec/sa.c > index 2297bd6d72..741e079831 100644 > --- a/lib/ipsec/sa.c > +++ b/lib/ipsec/sa.c > @@ -710,6 +710,7 @@ lksd_none_pkt_func_select(const struct rte_ipsec_sa *= sa, > case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV4): > case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV6): > pf->prepare.async =3D esp_outb_tun_prepare; > + pf->prepare_stateless.async =3D esp_outb_tun_prepare_stateless; > pf->process =3D (sa->sqh_len !=3D 0) ? > esp_outb_sqh_process : pkt_flag_process; > break; > @@ -748,6 +749,7 @@ cpu_crypto_pkt_func_select(const struct rte_ipsec_sa = *sa, > case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV4): > case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV6): > pf->prepare.sync =3D cpu_outb_tun_pkt_prepare; > + pf->prepare_stateless.sync =3D cpu_outb_tun_pkt_prepare_stateless; > pf->process =3D (sa->sqh_len !=3D 0) ? > esp_outb_sqh_process : pkt_flag_process; > break; > @@ -810,7 +812,7 @@ ipsec_sa_pkt_func_select(const struct rte_ipsec_sessi= on *ss, > int32_t rc; >=20 > rc =3D 0; > - pf[0] =3D (struct rte_ipsec_sa_pkt_func) { {NULL}, NULL }; > + pf[0] =3D (struct rte_ipsec_sa_pkt_func) { {NULL}, {NULL}, NULL }; >=20 > switch (ss->type) { > case RTE_SECURITY_ACTION_TYPE_NONE: > diff --git a/lib/ipsec/sa.h b/lib/ipsec/sa.h > index 719b5c735c..9b53586b2d 100644 > --- a/lib/ipsec/sa.h > +++ b/lib/ipsec/sa.h > @@ -179,6 +179,10 @@ uint16_t > esp_outb_tun_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf= *mb[], > struct rte_crypto_op *cop[], uint16_t num); >=20 > +uint16_t > +esp_outb_tun_prepare_stateless(const struct rte_ipsec_session *ss, struc= t rte_mbuf *mb[], > + struct rte_crypto_op *cop[], uint16_t num, struct rte_ipsec_state *stat= e); > + > uint16_t > esp_outb_trs_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf= *mb[], > struct rte_crypto_op *cop[], uint16_t num); > @@ -207,6 +211,10 @@ uint16_t > cpu_outb_tun_pkt_prepare(const struct rte_ipsec_session *ss, > struct rte_mbuf *mb[], uint16_t num); > uint16_t > +cpu_outb_tun_pkt_prepare_stateless(const struct rte_ipsec_session *ss, > + struct rte_mbuf *mb[], uint16_t num, struct rte_ipsec_state *state); > + > +uint16_t > cpu_outb_trs_pkt_prepare(const struct rte_ipsec_session *ss, > struct rte_mbuf *mb[], uint16_t num); >=20 > -- > 2.25.1