DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
To: Yongseok Koh <yskoh@mellanox.com>,
	"Nicolau, Radu" <radu.nicolau@intel.com>,
	"akhil.goyal@nxp.com" <akhil.goyal@nxp.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	"aviadye@mellanox.com" <aviadye@mellanox.com>,
	"stable@dpdk.org" <stable@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] examples/ipsec-secgw: fix coherency of ESP	sequence number
Date: Mon, 8 Apr 2019 10:41:29 +0000	[thread overview]
Message-ID: <2601191342CEEE43887BDE71AB9772580148A943CB@irsmsx105.ger.corp.intel.com> (raw)
Message-ID: <20190408104129.GCSiU2JSmlRkFLHhsodnxZftMO6BVxIFiS8ejZUguMk@z> (raw)
In-Reply-To: <20190406003512.1291-1-yskoh@mellanox.com>


Hi Yongseok,

> Outbound ESP sequence number should be incremented atomically and refeenced
> 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.

> 
> 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
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> ---
>  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(-)
> 
> 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;
> 
>  	RTE_ASSERT(m != NULL);
>  	RTE_ASSERT(sa != NULL);
> @@ -316,14 +317,14 @@ esp_outbound(struct rte_mbuf *m, struct ipsec_sa *sa,
>  		}
>  	}
> 
> -	sa->seq++;
>  	esp->spi = rte_cpu_to_be_32(sa->spi);
> -	esp->seq = rte_cpu_to_be_32((uint32_t)sa->seq);
> +	sa_seq = rte_atomic64_add_return(&sa->seq, 1);
> +	esp->seq = rte_cpu_to_be_32((uint32_t)sa_seq);
> 
>  	/* set iv */
>  	uint64_t *iv = (uint64_t *)(esp + 1);
>  	if (sa->aead_algo == RTE_CRYPTO_AEAD_AES_GCM) {
> -		*iv = rte_cpu_to_be_64(sa->seq);
> +		*iv = 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 = rte_cpu_to_be_64(sa->seq);
> +			*iv = 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,
> 
>  		struct cnt_blk *icb = get_cnt_blk(m);
>  		icb->salt = sa->salt;
> -		icb->iv = rte_cpu_to_be_64(sa->seq);
> +		icb->iv = rte_cpu_to_be_64(sa_seq);
>  		icb->cnt = rte_cpu_to_be_32(1);
> 
>  		aad = get_aad(m);
> @@ -414,7 +415,7 @@ esp_outbound(struct rte_mbuf *m, struct ipsec_sa *sa,
> 
>  		struct cnt_blk *icb = get_cnt_blk(m);
>  		icb->salt = sa->salt;
> -		icb->iv = rte_cpu_to_be_64(sa->seq);
> +		icb->iv = rte_cpu_to_be_64(sa_seq);
>  		icb->cnt = rte_cpu_to_be_32(1);
> 
>  		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 ipsec_sa entries[],
>  			return -EINVAL;
>  		}
>  		*sa = entries[i];
> -		sa->seq = 0;
> +		rte_atomic64_set(&sa->seq, -1);

I think initial value should remain zero.
Konstantin

  parent reply	other threads:[~2019-04-08 10:41 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-06  0:35 Yongseok Koh
2019-04-06  0:35 ` Yongseok Koh
2019-04-08 10:41 ` Ananyev, Konstantin [this message]
2019-04-08 10:41   ` Ananyev, Konstantin
2019-04-08 19:36   ` Yongseok Koh
2019-04-08 19:36     ` Yongseok Koh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2601191342CEEE43887BDE71AB9772580148A943CB@irsmsx105.ger.corp.intel.com \
    --to=konstantin.ananyev@intel.com \
    --cc=akhil.goyal@nxp.com \
    --cc=aviadye@mellanox.com \
    --cc=dev@dpdk.org \
    --cc=radu.nicolau@intel.com \
    --cc=stable@dpdk.org \
    --cc=yskoh@mellanox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).