Hi Akhil, Praveen, Can't rte_flow and RSS co-exist? In rte_flow there is an ACTION type RSS in addition to QUEUE. With this patch, if rte_flow is enabled on any SA, then RSS would be disabled for the entire port. Is that the right behavior? And if we have to address this later, what would be the course of action?
Also, is flow director the right name we should use? Internally it is rte_flow, right? Thanks, Anoob > -----Original Message----- > From: Akhil Goyal <akhil.go...@nxp.com> > Sent: Wednesday, April 1, 2020 6:50 PM > To: Praveen Shetty <praveen.she...@intel.com>; dev@dpdk.org; > declan.dohe...@intel.com; Anoob Joseph <ano...@marvell.com> > Cc: bernard.iremon...@intel.com; konstantin.anan...@intel.com > Subject: [EXT] RE: [dpdk-dev] [PATCH v3] examples/ipsec-secgw: support flow > director feature > > External Email > > ---------------------------------------------------------------------- > Hi Praveen, > > Sorry for being late to reply on this, Please delegate the patches properly > from > next time in patchworks. > This patch was neither delegated to me, nor I was in to/cc. So it got missed. > > > > > Support load distribution in security gateway application using NIC > > load distribution feature(Flow Director). > > Flow Director is used to redirect the specified inbound ipsec flow to > > a specified queue.This is achieved by extending the SA rule syntax to > > support specification by adding new action_type of <flow-direction> to > > a specified <port_id> <queue_id>. > > > > Please add documentation (doc/guides/sample_app_ug/ipsec_secgw.rst) > changes to explain the new parameter. > > > Signed-off-by: Praveen Shetty <praveen.she...@intel.com> > > --- > > v3 changes: > > Incorporated Anoob review comments on v2. > > > > > > > diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec- > > secgw/ipsec-secgw.c index ce36e6d9c..4400b075c 100644 > > --- a/examples/ipsec-secgw/ipsec-secgw.c > > +++ b/examples/ipsec-secgw/ipsec-secgw.c > > @@ -246,6 +246,30 @@ static struct rte_eth_conf port_conf = { > > .txmode = { > > .mq_mode = ETH_MQ_TX_NONE, > > }, > > + .fdir_conf = { > > Fdir_conf is a deprecated parameter. It is not good to introduce Something new > in the application with a deprecated parameter. > Please use the recommended way to configure flows. > > > + .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, > > + } > > }; > > > > struct socket_ctx socket_ctx[NB_SOCKETS]; @@ -1183,6 +1207,28 @@ > > ipsec_poll_mode_worker(void) > > } > > } > > > > +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_poll_mode_params(struct eh_conf *eh_conf) { @@ -2813,6 > > +2859,15 @@ main(int32_t argc, char **argv) > > > > sa_check_offloads(portid, &req_rx_offloads[portid], > > &req_tx_offloads[portid]); > > + /* 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[portid], > > req_tx_offloads[portid]); > > } > > diff --git a/examples/ipsec-secgw/ipsec.c > > b/examples/ipsec-secgw/ipsec.c index d40657102..76ee9dbcf 100644 > > --- a/examples/ipsec-secgw/ipsec.c > > +++ b/examples/ipsec-secgw/ipsec.c > > @@ -418,6 +418,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, sizeof(sa->dst.ip.ip6.ip6_b)); > > + memcpy(sa->ipv6_spec.hdr.src_addr, > > + sa->src.ip.ip6.ip6_b, sizeof(sa->src.ip.ip6.ip6_b)); > > + 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->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->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 f8f29f9b1..b0e9f45cb 100644 > > --- a/examples/ipsec-secgw/ipsec.h > > +++ b/examples/ipsec-secgw/ipsec.h > > @@ -144,6 +144,8 @@ struct ipsec_sa { > > }; > > enum rte_security_ipsec_sa_direction direction; > > uint16_t portid; > > + uint8_t fdir_qid; > > + uint8_t fdir_flag; > > > > #define MAX_RTE_FLOW_PATTERN (4) > > #define MAX_RTE_FLOW_ACTIONS (3) > > @@ -408,5 +410,12 @@ 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 0eb52d141..ddd275142 100644 > > --- a/examples/ipsec-secgw/sa.c > > +++ b/examples/ipsec-secgw/sa.c > > @@ -271,6 +271,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; > > @@ -295,6 +296,7 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens, > > if (atoi(tokens[1]) == INVALID_SPI) > > return; > > rule->spi = atoi(tokens[1]); > > + rule->portid = UINT16_MAX; > > ips = ipsec_get_primary_session(rule); > > > > for (ti = 2; ti < n_tokens; ti++) { > > @@ -636,9 +638,14 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens, > > INCREMENT_TOKEN_INDEX(ti, n_tokens, status); > > if (status->status < 0) > > return; > > - rule->portid = atoi(tokens[ti]); > > - if (status->status < 0) > > + if (rule->portid == UINT16_MAX) > > + rule->portid = atoi(tokens[ti]); > > + else if (rule->portid != atoi(tokens[ti])) { > > + APP_CHECK(0, status, "portid %s " > > + "not matching with already assigned portid > %u", > > + tokens[ti], rule->portid); > > return; > > + } > > portid_p = 1; > > continue; > > } > > @@ -681,6 +688,43 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens, > > fallback_p = 1; > > continue; > > } > > + if (strcmp(tokens[ti], "flow-direction") == 0) { > > + if (ips->type == > > + > > RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL || > > + ips->type == > > + > > RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL || > > + ips->type == > > + > > RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO) { > > + APP_CHECK(0, status, "Flow Director not " > > + "supported for security session " > > + "type:%d", ips->type); > > + return; > > + } > It means it is supported in cpu crypto as well? Better to have a check for the > supported Action types, as in the future there may be some other action types. > > > + rule->fdir_flag = 1; > > + INCREMENT_TOKEN_INDEX(ti, n_tokens, status); > > + if (status->status < 0) > > + return; > > + if (rule->portid == UINT16_MAX) > > + rule->portid = atoi(tokens[ti]); > > + else if (rule->portid != atoi(tokens[ti])) { > > + APP_CHECK(0, status, "portid %s " > > + "not matching with already assigned portid > %u", > > + tokens[ti], rule->portid); > > + return; > > + } > > + 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->portid, > > + rule->fdir_qid); > > + if (status_p < 0) { > > + printf("port id %u / queue id %u is not > > valid\n", > > + rule->portid, rule->fdir_qid); > > + } > > + continue; > > + } > > > > /* unrecognizeable input */ > > APP_CHECK(0, status, "unrecognized input \"%s\"", @@ -719,7 > +763,6 > > @@ parse_sa_tokens(char **tokens, uint32_t n_tokens, > > if (!type_p || (!portid_p && ips->type != > > RTE_SECURITY_ACTION_TYPE_CPU_CRYPTO)) { > > ips->type = RTE_SECURITY_ACTION_TYPE_NONE; > > - rule->portid = -1; > > } > > > > *ri = *ri + 1; > > @@ -823,6 +866,9 @@ print_one_sa_rule(const struct ipsec_sa *sa, int > inbound) > > break; > > } > > } > > + if (sa->fdir_flag == 1) > > + printf("flow-direction %d %d", sa->portid, sa->fdir_qid); > > Better to print like below. > printf("flow-direction port %d queue %d ", sa->portid, sa->fdir_qid) > > > + > > printf("\n"); > > } > >