Re: [dpdk-dev] [PATCH v1] examples/ipsec-secgw: support flow director feature

[Anoob Joseph <anoobj@marvell.com>]

Hi Praveen,

I do have some review comments on the code. Before that, can you give a brief overview of what is being targeted? My understanding is that the primary objective is to use rte_flow (or flow director) to redirect a specific flow(/SA) to a specific queue. Can you confirm?

Couple of questions,
1. I would assume the new option of "flow-direction" is optional and is determined per SA. In that case, can I assume that RSS would be active for the other flows (or SAs). Let's say, I just want to add a SA for which I would like to enable "flow-direction" but leave the rest as is. How is that handled?
2. I see that the changes are only applicable for LOOKASIDE_PROTOCOL. The same feature would be useful for other modes as well, right?
3. I'm not sure "flow-direction" is the right wording for the option. This is just specifying the "rx-queue" per SA. @Akhil, Konstantin, comments?

Thanks,
Anoob

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Praveen Shetty
> Sent: Wednesday, March 11, 2020 8:25 PM
> To: dev@dpdk.org; declan.doherty@intel.com; bernard.iremonger@intel.com;
> konstantin.ananyev@intel.com
> Subject: [dpdk-dev] [PATCH v1] examples/ipsec-secgw: support flow director
> feature
> 
> Modified Secuirty gateway application to support configuration of flow director
> rule to direct inbound IPsec SA to a specified queue.
> 
> Signed-off-by: Praveen Shetty <praveen.shetty@intel.com>
> ---
>  examples/ipsec-secgw/ep0.cfg       | 11 +++++
>  examples/ipsec-secgw/ipsec-secgw.c | 56 ++++++++++++++++++++++++-
>  examples/ipsec-secgw/ipsec.c       | 67 ++++++++++++++++++++++++++++++
>  examples/ipsec-secgw/ipsec.h       | 11 +++++
>  examples/ipsec-secgw/sa.c          | 50 +++++++++++++++++++++-
>  5 files changed, 192 insertions(+), 3 deletions(-)
> 
> diff --git a/examples/ipsec-secgw/ep0.cfg b/examples/ipsec-secgw/ep0.cfg
> index dfd4aca7d..c9f80e81b 100644
> --- a/examples/ipsec-secgw/ep0.cfg
> +++ b/examples/ipsec-secgw/ep0.cfg
> @@ -29,6 +29,7 @@ sp ipv4 in esp protect 111 pri 1 dst 192.168.186.0/24 sport
> 0:65535 dport 0:6553  sp ipv4 in esp protect 115 pri 1 dst 192.168.210.0/24 sport
> 0:65535 dport 0:65535  sp ipv4 in esp protect 116 pri 1 dst 192.168.211.0/24
> sport 0:65535 dport 0:65535  sp ipv4 in esp protect 115 pri 1 dst
> 192.168.210.0/24 sport 0:65535 dport 0:65535
> +sp ipv4 in esp protect 117 pri 1 dst 192.168.212.0/24 sport 0:65535
> +dport 0:65535
>  sp ipv4 in esp protect 125 pri 1 dst 192.168.65.0/24 sport 0:65535 dport 0:65535
> sp ipv4 in esp protect 125 pri 1 dst 192.168.65.0/24 sport 0:65535 dport 0:65535
> sp ipv4 in esp protect 126 pri 1 dst 192.168.66.0/24 sport 0:65535 dport 0:65535
> @@ -61,6 +62,8 @@ sp ipv6 in esp protect 125 pri 1 dst
> ffff:0000:0000:0000:aaaa:aaaa:0000:0000/96
>  sport 0:65535 dport 0:65535
>  sp ipv6 in esp protect 126 pri 1 dst
> ffff:0000:0000:0000:bbbb:bbbb:0000:0000/96 \  sport 0:65535 dport 0:65535
> +sp ipv6 in esp protect 127 pri 1 dst
> +ffff:0000:0000:0000:cccc:dddd:0000:0000/96 \ sport 0:65535 dport
> +0:65535
> 
>  #SA rules
>  sa out 5 cipher_algo aes-128-cbc cipher_key 0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0 \
> @@ -118,6 +121,9 @@ dst 172.16.1.5
> 
>  sa in 116 cipher_algo null auth_algo null mode ipv4-tunnel src 172.16.2.6 dst
> 172.16.1.6
> 
> +sa in 117 cipher_algo null auth_algo null mode ipv4-tunnel src
> +172.16.2.7 \ dst 172.16.1.7 flow-direction 0 2 port_id 0 type
> +lookaside-protocol-offload
> +
>  sa in 125 cipher_algo aes-128-cbc cipher_key c3:c3:c3:c3:c3:c3:c3:c3:c3:c3:c3:\
>  c3:c3:c3:c3:c3 auth_algo sha1-hmac auth_key
> c3:c3:c3:c3:c3:c3:c3:c3:c3:c3:c3:\
>  c3:c3:c3:c3:c3:c3:c3:c3:c3 mode ipv6-tunnel \ @@ -130,6 +136,11 @@ sa in
> 126 cipher_algo aes-128-cbc cipher_key 4d:4d:4d:4d:4d:4d:4d:4d:4d:4d:4d:\
>  src 2222:2222:2222:2222:2222:2222:2222:6666 \  dst
> 1111:1111:1111:1111:1111:1111:1111:6666
> 
> +sa in 127 cipher_algo null auth_algo null mode ipv6-tunnel \ src
> +2222:2222:2222:2222:2222:2222:2222:7777 \ dst
> +1111:1111:1111:1111:1111:1111:1111:7777 \ flow-direction 0 3 port_id 0
> +type lookaside-protocol-offload
> +
>  #Routing rules
>  rt ipv4 dst 172.16.2.5/32 port 0
>  rt ipv4 dst 172.16.2.6/32 port 1
> diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-
> secgw/ipsec-secgw.c
> index 4799bc90c..132484422 100644
> --- a/examples/ipsec-secgw/ipsec-secgw.c
> +++ b/examples/ipsec-secgw/ipsec-secgw.c
> @@ -166,7 +166,6 @@ static const struct option lgopts[] = {
>  	{CMD_LINE_OPT_FRAG_TTL, 1, 0, CMD_LINE_OPT_FRAG_TTL_NUM},
>  	{NULL, 0, 0, 0}
>  };
> -
>  /* mask of enabled ports */
>  static uint32_t enabled_port_mask;
>  static uint64_t enabled_cryptodev_mask = UINT64_MAX; @@ -259,6 +258,30
> @@ static struct rte_eth_conf port_conf = {
>  	.txmode = {
>  		.mq_mode = ETH_MQ_TX_NONE,
>  	},
> +	.fdir_conf = {
> +	.mode = RTE_FDIR_MODE_NONE,
> +	.pballoc = RTE_FDIR_PBALLOC_64K,
> +	.status = RTE_FDIR_REPORT_STATUS,
> +	.mask = {
> +		.vlan_tci_mask = 0xFFEF,
> +		.ipv4_mask     = {
> +			.src_ip = 0xFFFFFFFF,
> +			.dst_ip = 0xFFFFFFFF,
> +		},
> +		.ipv6_mask     = {
> +			.src_ip = {0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF,
> +						0xFFFFFFFF},
> +			.dst_ip = {0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF,
> +						0xFFFFFFFF},
> +		},
> +		.src_port_mask = 0xFFFF,
> +		.dst_port_mask = 0xFFFF,
> +		.mac_addr_byte_mask = 0xFF,
> +		.tunnel_type_mask = 1,
> +		.tunnel_id_mask = 0xFFFFFFFF,
> +	},
> +	.drop_queue = 127,
> +	}
>  };
> 
>  static struct socket_ctx socket_ctx[NB_SOCKETS]; @@ -1184,7 +1207,6 @@
> main_loop(__attribute__((unused)) void *dummy)
> 
>  			if (nb_rx > 0)
>  				process_pkts(qconf, pkts, nb_rx, portid);
> -
>  			/* dequeue and process completed crypto-ops */
>  			if (UNPROTECTED_PORT(portid))
>  				drain_inbound_crypto_queues(qconf,
> @@ -1196,6 +1218,27 @@ main_loop(__attribute__((unused)) void *dummy)
>  	}
>  }
> 
> +int check_flow_params(uint16_t fdir_portid, uint8_t fdir_qid) {
> +	uint16_t i;
> +	uint16_t portid;
> +	uint8_t queueid;
> +
> +	for (i = 0; i < nb_lcore_params; ++i) {
> +		portid = lcore_params_array[i].port_id;
> +		if (portid == fdir_portid) {
> +			queueid = lcore_params_array[i].queue_id;
> +			if (queueid == fdir_qid)
> +				break;
> +		}
> +
> +		if (i == nb_lcore_params - 1)
> +			return -1;
> +	}
> +
> +	return 1;
> +}
> +
>  static int32_t
>  check_params(void)
>  {
> @@ -2503,6 +2546,15 @@ main(int32_t argc, char **argv)
>  			continue;
> 
>  		sa_check_offloads(portid, &req_rx_offloads,
> &req_tx_offloads);
> +		/* check if FDIR is configured on the port */
> +		if (check_fdir_configured(portid)) {
> +			/* Enable FDIR */
> +			port_conf.fdir_conf.mode =
> RTE_FDIR_MODE_PERFECT;
> +			/* Disable RSS */
> +			port_conf.rxmode.mq_mode = ETH_MQ_RX_NONE;
> +			port_conf.rx_adv_conf.rss_conf.rss_hf = 0;
> +			port_conf.rx_adv_conf.rss_conf.rss_key = NULL;
> +		}
>  		port_init(portid, req_rx_offloads, req_tx_offloads);
>  	}
> 
> diff --git a/examples/ipsec-secgw/ipsec.c b/examples/ipsec-secgw/ipsec.c index
> 6e8120702..363809cfd 100644
> --- a/examples/ipsec-secgw/ipsec.c
> +++ b/examples/ipsec-secgw/ipsec.c
> @@ -415,6 +415,73 @@ create_inline_session(struct socket_ctx *skt_ctx,
> struct ipsec_sa *sa,
>  	return 0;
>  }
> 
> +int
> +create_ipsec_esp_flow(struct ipsec_sa *sa) {
> +	int ret = 0;
> +	struct rte_flow_error err;
> +	if (sa->direction == RTE_SECURITY_IPSEC_SA_DIR_EGRESS)
> +		return 0; /* No Flow director rules for Egress traffic */
> +	if (sa->flags == TRANSPORT) {
> +		RTE_LOG(ERR, IPSEC,
> +			"No Flow director rule for transport mode:");
> +			return -1;
> +	}
> +	sa->action[0].type = RTE_FLOW_ACTION_TYPE_QUEUE;
> +	sa->pattern[0].type = RTE_FLOW_ITEM_TYPE_ETH;
> +	sa->action[0].conf =
> +			&(struct rte_flow_action_queue){
> +				.index = sa->fdir_qid,
> +	};
> +	sa->attr.egress = 0;
> +	sa->attr.ingress = 1;
> +	if (IS_IP6(sa->flags)) {
> +		sa->pattern[1].mask = &rte_flow_item_ipv6_mask;
> +		sa->pattern[1].type = RTE_FLOW_ITEM_TYPE_IPV6;
> +		sa->pattern[1].spec = &sa->ipv6_spec;
> +		memcpy(sa->ipv6_spec.hdr.dst_addr,
> +				sa->dst.ip.ip6.ip6_b, IPV6_ADDR_LEN);
> +		memcpy(sa->ipv6_spec.hdr.src_addr,
> +				sa->src.ip.ip6.ip6_b, IPV6_ADDR_LEN);
> +		sa->pattern[2].type = RTE_FLOW_ITEM_TYPE_ESP;
> +		sa->pattern[2].spec = &sa->esp_spec;
> +		sa->pattern[2].mask = &rte_flow_item_esp_mask;
> +		sa->esp_spec.hdr.spi = rte_cpu_to_be_32(sa->spi);
> +		sa->pattern[3].type = RTE_FLOW_ITEM_TYPE_END;
> +	} else if (IS_IP4(sa->flags)) {
> +		sa->pattern[1].mask = &rte_flow_item_ipv4_mask;
> +		sa->pattern[1].type = RTE_FLOW_ITEM_TYPE_IPV4;
> +		sa->pattern[1].spec = &sa->ipv4_spec;
> +		sa->ipv4_spec.hdr.dst_addr = sa->dst.ip.ip4;
> +		sa->ipv4_spec.hdr.src_addr = sa->src.ip.ip4;
> +		sa->pattern[2].type = RTE_FLOW_ITEM_TYPE_ESP;
> +		sa->pattern[2].spec = &sa->esp_spec;
> +		sa->pattern[2].mask = &rte_flow_item_esp_mask;
> +		sa->esp_spec.hdr.spi = rte_cpu_to_be_32(sa->spi);
> +		sa->pattern[3].type = RTE_FLOW_ITEM_TYPE_END;
> +	}
> +	sa->action[1].type = RTE_FLOW_ACTION_TYPE_END;
> +
> +	ret = rte_flow_validate(sa->fdir_portid, &sa->attr,
> +				sa->pattern, sa->action,
> +				&err);
> +	if (ret < 0) {
> +		RTE_LOG(ERR, IPSEC,
> +			"Flow Validation failed\n");
> +		return ret;
> +	}
> +	sa->flow = rte_flow_create(sa->fdir_portid,
> +				&sa->attr, sa->pattern, sa->action,
> +				&err);
> +	if (!sa->flow) {
> +		RTE_LOG(ERR, IPSEC,
> +			"Flow Creation failed\n");
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
>  /*
>   * queue crypto-ops into PMD queue.
>   */
> diff --git a/examples/ipsec-secgw/ipsec.h b/examples/ipsec-secgw/ipsec.h
> index 4f2fd6184..00147895a 100644
> --- a/examples/ipsec-secgw/ipsec.h
> +++ b/examples/ipsec-secgw/ipsec.h
> @@ -46,6 +46,8 @@
> 
>  #define IP6_VERSION (6)
> 
> +#define IPV6_ADDR_LEN   16
> +
>  struct rte_crypto_xform;
>  struct ipsec_xform;
>  struct rte_mbuf;
> @@ -138,6 +140,9 @@ struct ipsec_sa {
>  	};
>  	enum rte_security_ipsec_sa_direction direction;
>  	uint16_t portid;
> +	uint16_t fdir_portid;
> +	uint8_t fdir_qid;
> +	uint8_t fdir_flag;
> 
>  #define MAX_RTE_FLOW_PATTERN (4)
>  #define MAX_RTE_FLOW_ACTIONS (3)
> @@ -383,5 +388,11 @@ create_lookaside_session(struct ipsec_ctx *ipsec_ctx,
> struct ipsec_sa *sa,  int  create_inline_session(struct socket_ctx *skt_ctx, struct
> ipsec_sa *sa,
>  		struct rte_ipsec_session *ips);
> +int
> +check_flow_params(uint16_t fdir_portid, uint8_t fdir_qid);
> +
> +int
> +create_ipsec_esp_flow(struct ipsec_sa *sa);
> 
> +int check_fdir_configured(uint16_t portid);
>  #endif /* __IPSEC_H__ */
> diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c index
> 4822d6bda..9955dfcbe 100644
> --- a/examples/ipsec-secgw/sa.c
> +++ b/examples/ipsec-secgw/sa.c
> @@ -20,6 +20,9 @@
>  #include <rte_random.h>
>  #include <rte_ethdev.h>
>  #include <rte_malloc.h>
> +#include <rte_common.h>
> +#include <rte_string_fns.h>
> +#include <rte_ethdev_driver.h>
> 
>  #include "ipsec.h"
>  #include "esp.h"
> @@ -271,6 +274,7 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens,
>  	uint32_t type_p = 0;
>  	uint32_t portid_p = 0;
>  	uint32_t fallback_p = 0;
> +	int16_t status_p = 0;
> 
>  	if (strcmp(tokens[0], "in") == 0) {
>  		ri = &nb_sa_in;
> @@ -681,6 +685,25 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens,
>  			fallback_p = 1;
>  			continue;
>  		}
> +		if (strcmp(tokens[ti], "flow-direction") == 0) {
> +			rule->fdir_flag = 1;
> +			INCREMENT_TOKEN_INDEX(ti, n_tokens, status);
> +			if (status->status < 0)
> +				return;
> +			rule->fdir_portid = atoi(tokens[ti]);
> +			INCREMENT_TOKEN_INDEX(ti, n_tokens, status);
> +			if (status->status < 0)
> +				return;
> +			rule->fdir_qid = atoi(tokens[ti]);
> +			/* validating portid and queueid */
> +			status_p = check_flow_params(rule->fdir_portid,
> +					rule->fdir_qid);
> +			if (status_p < 0) {
> +				printf("port id %u / queue id %u is not valid\n",
> +					rule->fdir_portid, rule->fdir_qid);
> +			}
> +			continue;
> +		}
> 
>  		/* unrecognizeable input */
>  		APP_CHECK(0, status, "unrecognized input \"%s\"", @@ -823,6
> +846,9 @@ print_one_sa_rule(const struct ipsec_sa *sa, int inbound)
>  			break;
>  		}
>  	}
> +	if (sa->fdir_flag == 1)
> +		printf("flow-direction %d %d", sa->fdir_portid, sa->fdir_qid);
> +
>  	printf("\n");
>  }
> 
> @@ -1153,7 +1179,15 @@ sa_add_rules(struct sa_ctx *sa_ctx, const struct
> ipsec_sa entries[],
>  				return -EINVAL;
>  			}
>  		}
> -
> +		if (sa->fdir_flag &&
> +			ips->type ==
> +			RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL
> &&
> +			inbound) {
> +			rc = create_ipsec_esp_flow(sa);
> +			if (rc != 0)
> +				RTE_LOG(ERR, IPSEC_ESP,
> +					"create_ipsec_esp flow failed\n");
> +			}
>  		print_one_sa_rule(sa, inbound);
>  	}
> 
> @@ -1256,6 +1290,20 @@ fill_ipsec_session(struct rte_ipsec_session *ss, struct
> rte_ipsec_sa *sa)
>  	return rc;
>  }
> 
> +int
> +check_fdir_configured(uint16_t portid)
> +{
> +	struct ipsec_sa *sa = NULL;
> +	uint32_t idx_sa = 0;
> +
> +	for (idx_sa = 0; idx_sa < nb_sa_in; idx_sa++) {
> +		sa = &sa_in[idx_sa];
> +		if (sa->fdir_portid == portid)
> +			return sa->fdir_flag;
> +	}
> +	return 0;
> +}
> +
>  /*
>   * Initialise related rte_ipsec_sa object.
>   */
> --
> 2.17.1