[dpdk-stable] patch 'examples/ipsec-secgw: fix SPD no-match case' has been queued to LTS release 18.11.2

Ananyev, Konstantin konstantin.ananyev at intel.com
Thu May 9 20:20:09 CEST 2019


Hi Kevin,

> Hi Konstantin,
> 
> I could rebase this patch but there is a lot of new code in master now,
> so can you check the diff and ack if it looks ok or if not, send a backport
> for 18.11 branch, or let me know it's not needed on stable.
> 
> This queued commit can be viewed at:
> https://github.com/kevintraynor/dpdk-stable-queue/commit/25df4326290ef72eb23d74c121a947c911376380

Looks good to me.
Thanks
Konstantin


> 
> Thanks.
> 
> Kevin Traynor
> 
> ---
> From 25df4326290ef72eb23d74c121a947c911376380 Mon Sep 17 00:00:00 2001
> From: Konstantin Ananyev <konstantin.ananyev at intel.com>
> Date: Thu, 4 Apr 2019 13:13:26 +0100
> Subject: [PATCH] examples/ipsec-secgw: fix SPD no-match case
> 
> [ upstream commit 49757b6845835394d1a9a6efad9356a6da58fd85 ]
> 
> acl_classify() returns zero value when no matching rule was found.
> Currently ipsec-secgw treats it as a valid SPI value, though it has
> to discard such packets.
> Error could be easily observed by sending outbound unmatched packets,
> user will see something like that in the log:
> IPSEC: No cryptodev: core 7, cipher_algo 0, auth_algo 0, aead_algo 0
> 
> To fix it we need to treat packets with zero result from acl_classify()
> as invalid ones. Also we can change DISCARD and BYPASS values to
> simplify checks and save some extra space for valid SPI values.
> To summarize the approach:
> 1. have special SPI values for DISCARD and BYPASS.
> 2. store in SPD full SPI value.
> 3. after acl_classify(), first check SPI value for DISCARD and BYPASS,
>    then convert it in SA index.
> 4. add check at initilisation time that for each SPD rule there is a
>    corresponding SA entry (with the same SPI).
> 
> Also marked few global variables as *static*.
> 
> Fixes: 906257e965b7 ("examples/ipsec-secgw: support IPv6")
> Fixes: 2a5106af132b ("examples/ipsec-secgw: fix corner case for SPI value")
> 
> Signed-off-by: Konstantin Ananyev <konstantin.ananyev at intel.com>
> Acked-by: Fan Zhang <roy.fan.zhang at intel.com>
> Acked-by: Akhil Goyal <akhil.goyal at nxp.com>
> ---
>  examples/ipsec-secgw/ipsec-secgw.c | 20 ++++++-------
>  examples/ipsec-secgw/ipsec.h       | 14 ++++++---
>  examples/ipsec-secgw/sa.c          | 35 ++++++++++++++++++----
>  examples/ipsec-secgw/sp4.c         | 47 ++++++++++++++++++++++++++++--
>  examples/ipsec-secgw/sp6.c         | 47 ++++++++++++++++++++++++++++--
>  5 files changed, 139 insertions(+), 24 deletions(-)
> 
> diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
> index 84fd4de12..dfb93375a 100644
> --- a/examples/ipsec-secgw/ipsec-secgw.c
> +++ b/examples/ipsec-secgw/ipsec-secgw.c
> @@ -427,9 +427,9 @@ inbound_sp_sa(struct sp_ctx *sp, struct sa_ctx *sa, struct traffic_type *ip,
>  		m = ip->pkts[i];
>  		res = ip->res[i];
> -		if (res & BYPASS) {
> +		if (res == BYPASS) {
>  			ip->pkts[j++] = m;
>  			continue;
>  		}
> -		if (res & DISCARD) {
> +		if (res == DISCARD) {
>  			rte_pktmbuf_free(m);
>  			continue;
> @@ -442,7 +442,6 @@ inbound_sp_sa(struct sp_ctx *sp, struct sa_ctx *sa, struct traffic_type *ip,
>  		}
> 
> -		sa_idx = ip->res[i] & PROTECT_MASK;
> -		if (sa_idx >= IPSEC_SA_MAX_ENTRIES ||
> -				!inbound_sa_check(sa, m, sa_idx)) {
> +		sa_idx = SPI2IDX(res);
> +		if (!inbound_sa_check(sa, m, sa_idx)) {
>  			rte_pktmbuf_free(m);
>  			continue;
> @@ -525,14 +524,13 @@ outbound_sp(struct sp_ctx *sp, struct traffic_type *ip,
>  	for (i = 0; i < ip->num; i++) {
>  		m = ip->pkts[i];
> -		sa_idx = ip->res[i] & PROTECT_MASK;
> -		if (ip->res[i] & DISCARD)
> +		sa_idx = SPI2IDX(ip->res[i]);
> +		if (ip->res[i] == DISCARD)
>  			rte_pktmbuf_free(m);
> -		else if (ip->res[i] & BYPASS)
> +		else if (ip->res[i] == BYPASS)
>  			ip->pkts[j++] = m;
> -		else if (sa_idx < IPSEC_SA_MAX_ENTRIES) {
> +		else {
>  			ipsec->res[ipsec->num] = sa_idx;
>  			ipsec->pkts[ipsec->num++] = m;
> -		} else /* invalid SA idx */
> -			rte_pktmbuf_free(m);
> +		}
>  	}
>  	ip->num = j;
> diff --git a/examples/ipsec-secgw/ipsec.h b/examples/ipsec-secgw/ipsec.h
> index 508d87af3..86d8f7df4 100644
> --- a/examples/ipsec-secgw/ipsec.h
> +++ b/examples/ipsec-secgw/ipsec.h
> @@ -41,8 +41,6 @@
>  #define INVALID_SPI (0)
> 
> -#define DISCARD (0x80000000)
> -#define BYPASS (0x40000000)
> -#define PROTECT_MASK (0x3fffffff)
> -#define PROTECT(sa_idx) (SPI2IDX(sa_idx) & PROTECT_MASK) /* SA idx 30 bits */
> +#define DISCARD	INVALID_SPI
> +#define BYPASS	UINT32_MAX
> 
>  #define IPSEC_XFORM_MAX 2
> @@ -242,4 +240,12 @@ void
>  sp6_init(struct socket_ctx *ctx, int32_t socket_id);
> 
> +/*
> + * Search through SA entries for given SPI.
> + * Returns first entry index if found(greater or equal then zero),
> + * or -ENOENT otherwise.
> + */
> +int
> +sa_spi_present(uint32_t spi, int inbound);
> +
>  void
>  sa_init(struct socket_ctx *ctx, int32_t socket_id);
> diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
> index cd4f369f3..f7b6eb0bb 100644
> --- a/examples/ipsec-secgw/sa.c
> +++ b/examples/ipsec-secgw/sa.c
> @@ -126,9 +126,9 @@ const struct supported_aead_algo aead_algos[] = {
>  };
> 
> -struct ipsec_sa sa_out[IPSEC_SA_MAX_ENTRIES];
> -uint32_t nb_sa_out;
> +static struct ipsec_sa sa_out[IPSEC_SA_MAX_ENTRIES];
> +static uint32_t nb_sa_out;
> 
> -struct ipsec_sa sa_in[IPSEC_SA_MAX_ENTRIES];
> -uint32_t nb_sa_in;
> +static struct ipsec_sa sa_in[IPSEC_SA_MAX_ENTRIES];
> +static uint32_t nb_sa_in;
> 
>  static const struct supported_cipher_algo *
> @@ -631,5 +631,5 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens,
>  }
> 
> -static inline void
> +static void
>  print_one_sa_rule(const struct ipsec_sa *sa, int inbound)
>  {
> @@ -917,4 +917,29 @@ sa_in_add_rules(struct sa_ctx *sa_ctx, const struct ipsec_sa entries[],
>  }
> 
> +/*
> + * Walk through all SA rules to find an SA with given SPI
> + */
> +int
> +sa_spi_present(uint32_t spi, int inbound)
> +{
> +	uint32_t i, num;
> +	const struct ipsec_sa *sar;
> +
> +	if (inbound != 0) {
> +		sar = sa_in;
> +		num = nb_sa_in;
> +	} else {
> +		sar = sa_out;
> +		num = nb_sa_out;
> +	}
> +
> +	for (i = 0; i != num; i++) {
> +		if (sar[i].spi == spi)
> +			return i;
> +	}
> +
> +	return -ENOENT;
> +}
> +
>  void
>  sa_init(struct socket_ctx *ctx, int32_t socket_id)
> diff --git a/examples/ipsec-secgw/sp4.c b/examples/ipsec-secgw/sp4.c
> index 6b05daaa9..99362a684 100644
> --- a/examples/ipsec-secgw/sp4.c
> +++ b/examples/ipsec-secgw/sp4.c
> @@ -100,4 +100,5 @@ parse_sp4_tokens(char **tokens, uint32_t n_tokens,
>  	uint32_t *ri = NULL; /* rule index */
>  	uint32_t ti = 0; /* token index */
> +	uint32_t tv;
> 
>  	uint32_t esp_p = 0;
> @@ -170,6 +171,10 @@ parse_sp4_tokens(char **tokens, uint32_t n_tokens,
>  				return;
> 
> -			rule_ipv4->data.userdata =
> -				PROTECT(atoi(tokens[ti]));
> +			tv = atoi(tokens[ti]);
> +			APP_CHECK(tv != DISCARD && tv != BYPASS, status,
> +				"invalid SPI: %s", tokens[ti]);
> +			if (status->status < 0)
> +				return;
> +			rule_ipv4->data.userdata = tv;
> 
>  			protect_p = 1;
> @@ -473,4 +478,34 @@ acl4_init(const char *name, int32_t socketid, const struct acl4_rules *rules,
>  }
> 
> +/*
> + * check that for each rule it's SPI has a correspondent entry in SAD
> + */
> +static int
> +check_spi_value(int inbound)
> +{
> +	uint32_t i, num, spi;
> +	const struct acl4_rules *acr;
> +
> +	if (inbound != 0) {
> +		acr = acl4_rules_in;
> +		num = nb_acl4_rules_in;
> +	} else {
> +		acr = acl4_rules_out;
> +		num = nb_acl4_rules_out;
> +	}
> +
> +	for (i = 0; i != num; i++) {
> +		spi = acr[i].data.userdata;
> +		if (spi != DISCARD && spi != BYPASS &&
> +				sa_spi_present(spi, inbound) < 0) {
> +			RTE_LOG(ERR, IPSEC, "SPI %u is not present in SAD\n",
> +				spi);
> +			return -ENOENT;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  void
>  sp4_init(struct socket_ctx *ctx, int32_t socket_id)
> @@ -489,4 +524,12 @@ sp4_init(struct socket_ctx *ctx, int32_t socket_id)
>  				"initialized\n", socket_id);
> 
> +	if (check_spi_value(1) < 0)
> +		rte_exit(EXIT_FAILURE,
> +			"Inbound IPv4 SP DB has unmatched in SAD SPIs\n");
> +
> +	if (check_spi_value(0) < 0)
> +		rte_exit(EXIT_FAILURE,
> +			"Outbound IPv4 SP DB has unmatched in SAD SPIs\n");
> +
>  	if (nb_acl4_rules_in > 0) {
>  		name = "sp_ip4_in";
> diff --git a/examples/ipsec-secgw/sp6.c b/examples/ipsec-secgw/sp6.c
> index dc5b94c6a..bfcabf39c 100644
> --- a/examples/ipsec-secgw/sp6.c
> +++ b/examples/ipsec-secgw/sp6.c
> @@ -131,4 +131,5 @@ parse_sp6_tokens(char **tokens, uint32_t n_tokens,
>  	uint32_t *ri = NULL; /* rule index */
>  	uint32_t ti = 0; /* token index */
> +	uint32_t tv;
> 
>  	uint32_t esp_p = 0;
> @@ -203,6 +204,10 @@ parse_sp6_tokens(char **tokens, uint32_t n_tokens,
>  				return;
> 
> -			rule_ipv6->data.userdata =
> -				PROTECT(atoi(tokens[ti]));
> +			tv = atoi(tokens[ti]);
> +			APP_CHECK(tv != DISCARD && tv != BYPASS, status,
> +				"invalid SPI: %s", tokens[ti]);
> +			if (status->status < 0)
> +				return;
> +			rule_ipv6->data.userdata = tv;
> 
>  			protect_p = 1;
> @@ -587,4 +592,34 @@ acl6_init(const char *name, int32_t socketid, const struct acl6_rules *rules,
>  }
> 
> +/*
> + * check that for each rule it's SPI has a correspondent entry in SAD
> + */
> +static int
> +check_spi_value(int inbound)
> +{
> +	uint32_t i, num, spi;
> +	const struct acl6_rules *acr;
> +
> +	if (inbound != 0) {
> +		acr = acl6_rules_in;
> +		num = nb_acl6_rules_in;
> +	} else {
> +		acr = acl6_rules_out;
> +		num = nb_acl6_rules_out;
> +	}
> +
> +	for (i = 0; i != num; i++) {
> +		spi = acr[i].data.userdata;
> +		if (spi != DISCARD && spi != BYPASS &&
> +				sa_spi_present(spi, inbound) < 0) {
> +			RTE_LOG(ERR, IPSEC, "SPI %u is not present in SAD\n",
> +				spi);
> +			return -ENOENT;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  void
>  sp6_init(struct socket_ctx *ctx, int32_t socket_id)
> @@ -603,4 +638,12 @@ sp6_init(struct socket_ctx *ctx, int32_t socket_id)
>  				"already initialized\n", socket_id);
> 
> +	if (check_spi_value(1) < 0)
> +		rte_exit(EXIT_FAILURE,
> +			"Inbound IPv6 SP DB has unmatched in SAD SPIs\n");
> +
> +	if (check_spi_value(0) < 0)
> +		rte_exit(EXIT_FAILURE,
> +			"Outbound IPv6 SP DB has unmatched in SAD SPIs\n");
> +
>  	if (nb_acl6_rules_in > 0) {
>  		name = "sp_ip6_in";
> --
> 2.20.1
> 
> ---
>   Diff of the applied patch vs upstream commit (please double-check if non-empty:
> ---
> --- -	2019-05-08 11:56:59.988734912 +0100
> +++ 0001-examples-ipsec-secgw-fix-SPD-no-match-case.patch	2019-05-08 11:56:59.945803452 +0100
> @@ -1 +1 @@
> -From 49757b6845835394d1a9a6efad9356a6da58fd85 Mon Sep 17 00:00:00 2001
> +From 25df4326290ef72eb23d74c121a947c911376380 Mon Sep 17 00:00:00 2001
> @@ -5,0 +6,2 @@
> +[ upstream commit 49757b6845835394d1a9a6efad9356a6da58fd85 ]
> +
> @@ -28 +29,0 @@
> -Cc: stable at dpdk.org
> @@ -34 +35 @@
> - examples/ipsec-secgw/ipsec-secgw.c | 20 ++++++------
> + examples/ipsec-secgw/ipsec-secgw.c | 20 ++++++-------
> @@ -36,4 +37,4 @@
> - examples/ipsec-secgw/sa.c          | 35 ++++++++++++++++++---
> - examples/ipsec-secgw/sp4.c         | 49 ++++++++++++++++++++++++++++--
> - examples/ipsec-secgw/sp6.c         | 49 ++++++++++++++++++++++++++++--
> - 5 files changed, 141 insertions(+), 26 deletions(-)
> + examples/ipsec-secgw/sa.c          | 35 ++++++++++++++++++----
> + examples/ipsec-secgw/sp4.c         | 47 ++++++++++++++++++++++++++++--
> + examples/ipsec-secgw/sp6.c         | 47 ++++++++++++++++++++++++++++--
> + 5 files changed, 139 insertions(+), 24 deletions(-)
> @@ -42 +43 @@
> -index ffbd00b08..2e203393d 100644
> +index 84fd4de12..dfb93375a 100644
> @@ -45 +46 @@
> -@@ -439,9 +439,9 @@ inbound_sp_sa(struct sp_ctx *sp, struct sa_ctx *sa, struct traffic_type *ip,
> +@@ -427,9 +427,9 @@ inbound_sp_sa(struct sp_ctx *sp, struct sa_ctx *sa, struct traffic_type *ip,
> @@ -57 +58 @@
> -@@ -454,7 +454,6 @@ inbound_sp_sa(struct sp_ctx *sp, struct sa_ctx *sa, struct traffic_type *ip,
> +@@ -442,7 +442,6 @@ inbound_sp_sa(struct sp_ctx *sp, struct sa_ctx *sa, struct traffic_type *ip,
> @@ -67 +68 @@
> -@@ -542,14 +541,13 @@ outbound_sp(struct sp_ctx *sp, struct traffic_type *ip,
> +@@ -525,14 +524,13 @@ outbound_sp(struct sp_ctx *sp, struct traffic_type *ip,
> @@ -88 +89 @@
> -index 99f49d65f..589398f6f 100644
> +index 508d87af3..86d8f7df4 100644
> @@ -91 +92 @@
> -@@ -42,8 +42,6 @@
> +@@ -41,8 +41,6 @@
> @@ -102,2 +103,2 @@
> -@@ -290,4 +288,12 @@ int
> - sp6_spi_present(uint32_t spi, int inbound);
> +@@ -242,4 +240,12 @@ void
> + sp6_init(struct socket_ctx *ctx, int32_t socket_id);
> @@ -116 +117 @@
> -index a7298a30c..b850e9839 100644
> +index cd4f369f3..f7b6eb0bb 100644
> @@ -119 +120 @@
> -@@ -127,9 +127,9 @@ const struct supported_aead_algo aead_algos[] = {
> +@@ -126,9 +126,9 @@ const struct supported_aead_algo aead_algos[] = {
> @@ -133 +134 @@
> -@@ -632,5 +632,5 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens,
> +@@ -631,5 +631,5 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens,
> @@ -140 +141 @@
> -@@ -1117,4 +1117,29 @@ ipsec_satbl_init(struct sa_ctx *ctx, const struct ipsec_sa *ent,
> +@@ -917,4 +917,29 @@ sa_in_add_rules(struct sa_ctx *sa_ctx, const struct ipsec_sa entries[],
> @@ -171 +172 @@
> -index d1dc64bad..ca9ee7f24 100644
> +index 6b05daaa9..99362a684 100644
> @@ -241,7 +241,0 @@
> -@@ -524,5 +567,5 @@ sp4_spi_present(uint32_t spi, int inbound)
> -
> - 	for (i = 0; i != num; i++) {
> --		if (acr[i].data.userdata == PROTECT(spi))
> -+		if (acr[i].data.userdata == spi)
> - 			return i;
> - 	}
> @@ -249 +243 @@
> -index e67d85aaf..76be3d3e9 100644
> +index dc5b94c6a..bfcabf39c 100644
> @@ -319,7 +312,0 @@
> -@@ -638,5 +681,5 @@ sp6_spi_present(uint32_t spi, int inbound)
> -
> - 	for (i = 0; i != num; i++) {
> --		if (acr[i].data.userdata == PROTECT(spi))
> -+		if (acr[i].data.userdata == spi)
> - 			return i;
> - 	}


More information about the stable mailing list