Hi Chenxu, <snip>
> [snip] > > > > --- a/drivers/net/i40e/i40e_flow.c > > > +++ b/drivers/net/i40e/i40e_flow.c > > > @@ -4424,10 +4424,10 @@ i40e_flow_parse_qinq_filter(struct > > > rte_eth_dev *dev, > > > * function for RSS, or flowtype for queue region configuration. > > > * For example: > > > * pattern: > > > - * Case 1: only ETH, indicate flowtype for queue region will be parsed. > > > - * Case 2: only VLAN, indicate user_priority for queue region will be > parsed. > > > - * Case 3: none, indicate RSS related will be parsed in action. > > > - * Any pattern other the ETH or VLAN will be treated as invalid except > END. > > > + * Case 1: try to transform patterns to pctype. valid pctype will be > > > + * used in parse action. > > > + * Case 2: only ETH, indicate flowtype for queue region will be parsed. > > > + * Case 3: only VLAN, indicate user_priority for queue region will be > parsed. > > > * So, pattern choice is depened on the purpose of configuration of > > > * that flow. > > > * action: > > > @@ -4438,15 +4438,66 @@ static int > > > i40e_flow_parse_rss_pattern(__rte_unused struct rte_eth_dev *dev, > > > const struct rte_flow_item *pattern, > > > struct rte_flow_error *error, > > > - uint8_t *action_flag, > > > + struct i40e_rss_pattern_info *p_info, > > > struct i40e_queue_regions *info) { const struct > > > rte_flow_item_vlan *vlan_spec, *vlan_mask; const struct > > > rte_flow_item *item = pattern; enum rte_flow_item_type item_type; > > > - > > > -if (item->type == RTE_FLOW_ITEM_TYPE_END) > > > +struct rte_flow_item *items; > > > +uint32_t item_num = 0; /* non-void item number of pattern*/ > > > +uint32_t i = 0; static const struct { enum rte_flow_item_type > > > +*item_array; uint64_t type; } i40e_rss_pctype_patterns[] = { { > > > +pattern_fdir_ipv4, > > > +ETH_RSS_FRAG_IPV4 | > > > ETH_RSS_NONFRAG_IPV4_OTHER }, > > > +{ pattern_fdir_ipv4_tcp, ETH_RSS_NONFRAG_IPV4_TCP }, { > > > +pattern_fdir_ipv4_udp, ETH_RSS_NONFRAG_IPV4_UDP }, { > > > +pattern_fdir_ipv4_sctp, ETH_RSS_NONFRAG_IPV4_SCTP }, { > > > +pattern_fdir_ipv6, > > > +ETH_RSS_FRAG_IPV6 | > > > ETH_RSS_NONFRAG_IPV6_OTHER }, > > > +{ pattern_fdir_ipv6_tcp, ETH_RSS_NONFRAG_IPV6_TCP }, { > > > +pattern_fdir_ipv6_udp, ETH_RSS_NONFRAG_IPV6_UDP }, { > > > +pattern_fdir_ipv6_sctp, ETH_RSS_NONFRAG_IPV6_SCTP }, }; > > > + > > > +p_info->types = I40E_RSS_TYPE_INVALID; > > > + > > > +if (item->type == RTE_FLOW_ITEM_TYPE_END) { p_info->types = > > > +I40E_RSS_TYPE_NONE; > > > return 0; > > > +} > > > + > > > +/* convert flow to pctype */ > > > +while ((pattern + i)->type != RTE_FLOW_ITEM_TYPE_END) { if > > > +((pattern > > > ++ i)->type != RTE_FLOW_ITEM_TYPE_VOID) item_num++; > > > +i++; > > > +} > > > +item_num++; > > > + > > > +items = rte_zmalloc("i40e_pattern", > > > + item_num * sizeof(struct rte_flow_item), 0); if (!items) { > > > +rte_flow_error_set(error, ENOMEM, > > > RTE_FLOW_ERROR_TYPE_ITEM_NUM, > > > + NULL, "No memory for PMD internal > > > items."); > > > +return -ENOMEM; > > > +} > > > + > > > +i40e_pattern_skip_void_item(items, pattern); > > > + > > > +for (i = 0; i < RTE_DIM(i40e_rss_pctype_patterns); i++) { if > > > (i40e_match_pattern(i40e_rss_pctype_patterns[i].item_array, > > > +items)) { > > > +p_info->types = i40e_rss_pctype_patterns[i].type; rte_free(items); > > > +return 0; } } > > > + > > > +rte_free(items); > > > > > > for (; item->type != RTE_FLOW_ITEM_TYPE_END; item++) { if > > > (item->last) { @@ -4459,7 +4510,7 @@ > > > i40e_flow_parse_rss_pattern(__rte_unused struct rte_eth_dev *dev, > > > item_type = item->type; switch (item_type) { case > > > RTE_FLOW_ITEM_TYPE_ETH: > > > -*action_flag = 1; > > > +p_info->action_flag = 1; > > > break; > > > case RTE_FLOW_ITEM_TYPE_VLAN: > > > vlan_spec = item->spec; > > > @@ -4472,7 +4523,7 @@ i40e_flow_parse_rss_pattern(__rte_unused > > > struct rte_eth_dev *dev, > > > vlan_spec->tci) >> 13) & 0x7; > > > info->region[0].user_priority_num = 1; info->queue_region_number = > > > 1; -*action_flag = 0; > > > +p_info->action_flag = 0; > > > } > > > } > > > break; > > > @@ -4500,12 +4551,14 @@ i40e_flow_parse_rss_pattern(__rte_unused > > > struct rte_eth_dev *dev, > > > * max index should be 7, and so on. And also, queue index should be > > > * continuous sequence and queue region index should be part of rss > > > * queue index for this port. > > > + * For hash params, the pctype in action and pattern must be same. > > > + * Set queue index or symmetric hash enable must be with non-types. > > > */ > > > static int > > > i40e_flow_parse_rss_action(struct rte_eth_dev *dev, > > > const struct rte_flow_action *actions, > > > struct rte_flow_error *error, > > > - uint8_t action_flag, > > > +struct i40e_rss_pattern_info p_info, > > > struct i40e_queue_regions *conf_info, > > > union i40e_filter_t *filter) > > > { > > > @@ -4516,7 +4569,7 @@ i40e_flow_parse_rss_action(struct rte_eth_dev > > > *dev, struct i40e_rte_flow_rss_conf *rss_config = > > > &filter->rss_conf; struct i40e_rte_flow_rss_conf *rss_info = > > > &pf->rss_info; -uint16_t i, j, n, tmp; > > > +uint16_t i, j, n, tmp, nb_types; > > > uint32_t index = 0; > > > uint64_t hf_bit = 1; > > > > > > @@ -4535,7 +4588,7 @@ i40e_flow_parse_rss_action(struct rte_eth_dev > > > *dev, return -rte_errno; } > > > > > > -if (action_flag) { > > > +if (p_info.action_flag) { > > > for (n = 0; n < 64; n++) { > > > if (rss->types & (hf_bit << n)) { > > > conf_info->region[0].hw_flowtype[0] = n; @@ -4674,11 +4727,11 @@ > > > i40e_flow_parse_rss_action(struct rte_eth_dev *dev, if > > > (rss_config->queue_region_conf) return 0; > > > > > > -if (!rss || !rss->queue_num) { > > > +if (!rss) { > > > rte_flow_error_set(error, EINVAL, > > > RTE_FLOW_ERROR_TYPE_ACTION, > > > act, > > > -"no valid queues"); > > > +"no valid rules"); > > > return -rte_errno; > > > } > > > > > > @@ -4692,19 +4745,40 @@ i40e_flow_parse_rss_action(struct > > > rte_eth_dev *dev, } } > > > > > > -if (rss_info->conf.queue_num) { > > > -rte_flow_error_set(error, EINVAL, > > > -RTE_FLOW_ERROR_TYPE_ACTION, > > > -act, > > > -"rss only allow one valid rule"); > > > -return -rte_errno; > > > +if (rss->queue_num && (p_info.types || rss->types)) > > > > Should the line above be > > > > if (conf_info->queue_region_number && (p_info.types || rss->types)) > > > > to allow RSS configuration of types and queues in a single rule, for > > example: > > > > flow create 0 ingress pattern eth / end actions rss types udp end > > queues 2 3 end / end > > > > For the conf_info->queue_region_number and rss->queue_num, In the old > codes, if there is eth or vlan, the conf_info->queue_region_number will be > set as 1 in the function parse pattern. > And in the parse action function it will check the conf_info- > >queue_region_number. > If conf_info->queue_region_number == 1, it will do some things and return. > It will not do the things but do others while conf_info- > >queue_region_number == 0. > And after parse it will call the function i40e_flush_queue_region_all_conf() > if > conf_info->queue_region_number == 1 While call the function > i40e_config_rss_filter() if conf_info->queue_region_number == 0. > > So what I changed is only when conf_info->queue_region_number == 0. > > Btw, in i40e, the queue configuration is for a port, it can't be for one rule > or > one type. > So I don't think it is a good idea for allowing RSS configuration of types and > queues in a single rule. Would you suggest two rules as follows? To configure the queues: flow create 0 ingress pattern end actions rss queues 2 3 end / end To configure the hash: flow create 0 ingress pattern eth / ipv4 / end actions rss types ipv4 end key_len 0 queues end / end (above rule is used on the ice pmd) Regards, Bernard