Hi Chenxu, > -----Original Message----- > From: Di, ChenxuX <chenxux...@intel.com> > Sent: Monday, March 23, 2020 9:19 AM > To: Iremonger, Bernard <bernard.iremon...@intel.com>; dev@dpdk.org; > Xing, Beilei <beilei.x...@intel.com>; Zhang, Qi Z <qi.z.zh...@intel.com>; > Doherty, Declan <declan.dohe...@intel.com> > Cc: Ananyev, Konstantin <konstantin.anan...@intel.com>; Iremonger, > Bernard <bernard.iremon...@intel.com> > Subject: RE: [dpdk-dev] [PATCH 3/3] net/i40e: configure RSS hash from RSS > action > > Hi, Bernard > This patch conflicts with the > patch(http://patchwork.dpdk.org/project/dpdk/list/?series=9005). In 20.05 > release, there will be some changes for creating RSS flows to support more > features and make it more flexible. > Changes for RSS flow: > - Configure the hash input set according to the packet type. > - Configure hash function. > - User can create more than one RSS flows and won't affect other rules when > destory. > For example, if you want to enable rss for ipv4-tcp, the command looks like > this: > flow create 0 ingress pattern eth / ipv4 / tcp / end actions rss types > ipv4-tcp > end queues end / end > > So, perhaps you need rework your feature depending on that patch. > > > -----Original Message----- > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Bernard > Iremonger > > Sent: Wednesday, March 18, 2020 8:38 PM > > To: dev@dpdk.org; Xing, Beilei <beilei.x...@intel.com>; Zhang, Qi Z > > <qi.z.zh...@intel.com>; Doherty, Declan <declan.dohe...@intel.com> > > Cc: Ananyev, Konstantin <konstantin.anan...@intel.com>; Iremonger, > > Bernard <bernard.iremon...@intel.com> > > Subject: [dpdk-dev] [PATCH 3/3] net/i40e: configure RSS hash from RSS > > action > > > > handle RSS offload types in RSS action handle ipv4, ipv6 and udp RSS > > patterns handle queue_num 0 in RSS action > > > > Signed-off-by: Bernard Iremonger <bernard.iremon...@intel.com> > > --- > > drivers/net/i40e/i40e_flow.c | 46 > > ++++++++++++++++++++++++++++++++------- > > ----- > > 1 file changed, 34 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/net/i40e/i40e_flow.c > > b/drivers/net/i40e/i40e_flow.c index d877ac2..b9a928c 100644 > > --- a/drivers/net/i40e/i40e_flow.c > > +++ b/drivers/net/i40e/i40e_flow.c > > @@ -4461,6 +4461,12 @@ i40e_flow_parse_rss_pattern(__rte_unused > struct > > rte_eth_dev *dev, case RTE_FLOW_ITEM_TYPE_ETH: > > *action_flag = 1; > > break; > > +case RTE_FLOW_ITEM_TYPE_IPV4: > > +break; > > +case RTE_FLOW_ITEM_TYPE_IPV6: > > +break; > > +case RTE_FLOW_ITEM_TYPE_UDP: > > +break; > > case RTE_FLOW_ITEM_TYPE_VLAN: > > vlan_spec = item->spec; > > vlan_mask = item->mask; > > @@ -4516,12 +4522,14 @@ 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; > > +struct rte_eth_rss_conf eth_rss_conf; int ret; > > uint16_t i, j, n, tmp; > > uint32_t index = 0; > > uint64_t hf_bit = 1; > > +uint64_t rss_hf; > > > > NEXT_ITEM_OF_ACTION(act, actions, index); -rss = act->conf; > > > > /** > > * rss only supports forwarding, > > @@ -4535,6 +4543,23 @@ i40e_flow_parse_rss_action(struct rte_eth_dev > > *dev, return -rte_errno; } > > > > +rss = act->conf; > > +rss_hf = rss->types; > > + > > +if (rss->queue_num == 0) { > > +memset(ð_rss_conf, 0, sizeof(eth_rss_conf)); ret = > > +rte_eth_dev_rss_hash_conf_get(dev->data->port_id, > > +ð_rss_conf); > > +if (ret != 0) > > +return ret; > > + > > +eth_rss_conf.rss_hf |= rss_hf; > > +ret = rte_eth_dev_rss_hash_update(dev->data->port_id, > > +ð_rss_conf); > > +if (ret != 0) > > +return ret; > > +} > > + > > I don't think it is a good idea that update rss hash enable status in the > parse > function. > it will update status while send command flow validate xxxxxx which can't be > destoried. > > > if (action_flag) { > > for (n = 0; n < 64; n++) { > > if (rss->types & (hf_bit << n)) { > > @@ -4552,7 +4577,7 @@ i40e_flow_parse_rss_action(struct rte_eth_dev > *dev, > > * continuous sequence and also to be part of RSS > > * queue index for this port. > > */ > > -if (conf_info->queue_region_number) { > > +if (conf_info->queue_region_number && rss->queue_num > 0) { > > for (i = 0; i < rss->queue_num; i++) { for (j = 0; j < > > rss_info->conf.queue_num; j++) { if (rss->queue[i] == > > rss_info->conf.queue[j]) @@ -4579,7 +4604,8 @@ > > i40e_flow_parse_rss_action(struct rte_eth_dev *dev, } > > > > /* Parse queue region related parameters from configuration */ -for > > (n = 0; n < conf_info->queue_region_number; n++) { > > +for (n = 0; n < conf_info->queue_region_number && > > +rss->queue_num > 0; n++) { > > if (conf_info->region[n].user_priority_num || > > conf_info->region[n].flowtype_num) { > > if (!((rte_is_power_of_2(rss->queue_num)) && @@ - > > 4674,14 +4700,6 @@ i40e_flow_parse_rss_action(struct rte_eth_dev *dev, > > if (rss_config->queue_region_conf) return 0; > > > > -if (!rss || !rss->queue_num) { > > -rte_flow_error_set(error, EINVAL, > > -RTE_FLOW_ERROR_TYPE_ACTION, > > -act, > > -"no valid queues"); > > -return -rte_errno; > > -} > > - > > for (n = 0; n < rss->queue_num; n++) { if (rss->queue[n] >= > > dev->data->nb_rx_queues) { rte_flow_error_set(error, EINVAL, @@ > > -4692,7 +4710,7 @@ i40e_flow_parse_rss_action(struct rte_eth_dev *dev, > > } } > > > > -if (rss_info->conf.queue_num) { > > +if (rss_info->conf.queue_num && rss->queue_num) { > > rte_flow_error_set(error, EINVAL, > > RTE_FLOW_ERROR_TYPE_ACTION, > > act, > > @@ -4709,6 +4727,10 @@ i40e_flow_parse_rss_action(struct rte_eth_dev > > *dev, return rte_flow_error_set (error, ENOTSUP, > > RTE_FLOW_ERROR_TYPE_ACTION, act, > > "a nonzero RSS encapsulation level is not supported"); > > +if (rss->types > ETH_RSS_ESP) > > +return rte_flow_error_set > > +(error, ENOTSUP, RTE_FLOW_ERROR_TYPE_ACTION, > > act, > > + "pctype greater than max allowed"); > > At the patch(http://patchwork.dpdk.org/project/dpdk/list/?series=9005), > the hash input set will be enable. > So rss->types may has the value ETH_RSS_L3_SRC_ONLY (1ULL << 63) and > ETH_RSS_L3_DST_ONLY (1ULL << 62) and ETH_RSS_L4_SRC_ONLY (1ULL << > 61) and ETH_RSS_L4_DST_ONLY (1ULL << 60). > > > if (rss->key_len && rss->key_len > RTE_DIM(rss_config->key)) return > > rte_flow_error_set (error, ENOTSUP, RTE_FLOW_ERROR_TYPE_ACTION, > act, > > -- > > 2.7.4 >
Yes, I will need to rework this patch based on top of your patch. Regards, Bernard.
