[dpdk-dev] [PATCH v6] net/i40e: implement hash function in rte flow API

[Iremonger, Bernard]

Hi Chenxu,

<snip>

> > > -----Original Message-----
> > > From: dev <dev-bounces at dpdk.org> On Behalf Of Chenxu Di
> > > Sent: Monday, March 30, 2020 8:40 AM
> > > To: dev at dpdk.org
> > > Cc: Yang, Qiming <qiming.yang at intel.com>; Xing, Beilei
> > > <beilei.xing at intel.com>; Zhao1, Wei <wei.zhao1 at intel.com>; Di,
> > > ChenxuX <chenxux.di at intel.com>
> > > Subject: [dpdk-dev] [PATCH v6] net/i40e: implement hash function in
> > > rte flow API
> > >
> > > implement set hash global configurations, set symmetric hash enable
> > > and set hash input set in rte flow API.
> > >
> > > Signed-off-by: Chenxu Di <chenxux.di at intel.com>
> 
> [snip]
> 
> > > -struct i40e_rte_flow_rss_conf *rss_info = &pf->rss_info;
> > >
> > > -if (!add) {
> > > -if (i40e_action_rss_same(&rss_info->conf, &conf->conf)) {
> >
> >
> > The function i40e_action_rss_same() is no longer used.
> > Should it be removed from i40e_ethdev.c and i40e_ethdev.h?
> >
> 
> It seems like no one use the function.
> However I check the commit of the function and find the
> commit(ac8d22de23) is so large.
> I don't know it is ok if remove it.

Best to check with Qi or Beilei if it is ok to remove this function.

> 
> >
> > > -i40e_pf_disable_rss(pf);
> > > -memset(rss_info, 0,
> > > -sizeof(struct i40e_rte_flow_rss_conf)); -return 0;
> > > +ret = 0;
> > > +
> > > +for (i = RTE_ETH_FLOW_IPV4; i <= RTE_ETH_FLOW_L2_PAYLOAD;
> > > i++) {
> > > +if (!(pf->adapter->flow_types_mask & (1ull << i)) ||
> > > +    !(types & (1ull << i)))
> > > +continue;
> > > +
> > > +conf.op = RTE_ETH_INPUT_SET_SELECT; conf.flow_type = i;
> > > +conf.inset_size = 0; for (j = 0; j < RTE_DIM(inset_type_table);
> > > +j++) { if ((types &
> > > +inset_type_table[j].type) ==
> > > +    inset_type_table[j].type) {
> > > +if (inset_type_table[j].field ==
> > > +    RTE_ETH_INPUT_SET_UNKNOWN) {
> > > +return -EINVAL;
> > > +}
> > > +conf.field[conf.inset_size] =
> > > +inset_type_table[j].field;
> > > +conf.inset_size++;
> > > +}
> > >  }
> > > +
> > > +if (conf.inset_size) {
> > > +ret = i40e_hash_filter_inset_select(hw, &conf); if (ret) return
> > > +ret; } }
> > > +
> > > +return ret;
> > > +}
> > > +
> > > +/* set existing rule invalid if it is covered */ static void
> > > +i40e_config_rss_invalidate_previous_rule(struct i40e_pf *pf, struct
> > > +i40e_rte_flow_rss_conf *conf) { struct i40e_flow_rss_filter
> > > +*rss_item; uint64_t rss_inset;
> > > +
> > > +/* to check pctype same need without input set bits */ rss_inset =
> > > +~(ETH_RSS_L3_SRC_ONLY | ETH_RSS_L3_DST_ONLY |
> > ETH_RSS_L4_SRC_ONLY |
> > > +ETH_RSS_L4_DST_ONLY);
> > > +
> > > +TAILQ_FOREACH(rss_item, &pf->rss_info_list, next) { if
> > > +(!rss_item->rss_filter_info.valid)
> > > +continue;
> > > +
> > > +/* config rss queue rule */
> > > +if (conf->conf.queue_num &&
> > > +    rss_item->rss_filter_info.conf.queue_num)
> > > +rss_item->rss_filter_info.valid = false;
> > > +
> > > +/* config rss input set rule */
> > > +if (conf->conf.types &&
> > > +    (rss_item->rss_filter_info.conf.types &
> > > +    rss_inset) ==
> > > +    (conf->conf.types & rss_inset)) rss_item->rss_filter_info.valid
> > > += false;
> > > +
> > > +/* config rss function symmetric rule */ if (conf->conf.func ==
> > > +    RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ &&
> > > +    rss_item->rss_filter_info.conf.func ==
> > > +    RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ)
> > > +rss_item->rss_filter_info.valid = false;
> > > +
> > > +/* config rss function xor or toeplitz rule */ if
> > > +(rss_item->rss_filter_info.conf.func !=
> > > +    RTE_ETH_HASH_FUNCTION_DEFAULT &&
> > > +    conf->conf.func != RTE_ETH_HASH_FUNCTION_DEFAULT
> > > &&
> > > +    (rss_item->rss_filter_info.conf.types & rss_inset) ==
> > > +    (conf->conf.types & rss_inset)) rss_item->rss_filter_info.valid
> > > += false; } }
> > > +
> > > +/* config  rss hash enable and set hash input set */ static int
> > > +i40e_config_hash_pctype_add(struct i40e_pf *pf, struct
> > > +i40e_rte_flow_rss_conf *conf, struct rte_eth_rss_conf *rss_conf) {
> > > +struct i40e_rte_flow_rss_conf *rss_info = &pf->rss_info;
> > > +
> > > +if (!(rss_conf->rss_hf & pf->adapter->flow_types_mask)) return
> > > +-ENOTSUP;
> > > +
> > > +/* Confirm hash input set */
> > > +if (i40e_config_rss_inputset(pf, rss_conf->rss_hf))
> > >  return -EINVAL;
> > > +
> > > +if (rss_conf->rss_key == NULL || rss_conf->rss_key_len <
> > > +    (I40E_PFQF_HKEY_MAX_INDEX + 1) * sizeof(uint32_t)) {
> > > +/* Random default keys */
> > > +static uint32_t rss_key_default[] = {0x6b793944, 0x23504cb5,
> > > +0x5bea75b6, 0x309f4f12, 0x3dc0a2b8, 0x024ddcdf, 0x339b8ca0,
> > > +0x4c4af64a, 0x34fac605, 0x55d85839, 0x3a58997d, 0x2ec938e1,
> > > +0x66031581};
> > > +
> > > +rss_conf->rss_key = (uint8_t *)rss_key_default;
> > > +rss_conf->rss_key_len = (I40E_PFQF_HKEY_MAX_INDEX + 1)
> > > *
> > > +sizeof(uint32_t);
> > > +PMD_DRV_LOG(INFO,
> > > +"No valid RSS key config for i40e, using default\n");
> > >  }
> > >
> > > +rss_conf->rss_hf |= rss_info->conf.types; i40e_hw_rss_hash_set(pf,
> > > +rss_conf);
> > > +
> > > +i40e_config_rss_invalidate_previous_rule(pf, conf);
> > > +
> > > +return 0;
> > > +}
> > > +
> > > +/* config rss queue region */
> > > +static int
> > > +i40e_config_hash_queue_add(struct i40e_pf *pf, struct
> > > +i40e_rte_flow_rss_conf *conf) { struct i40e_hw *hw =
> > > +I40E_PF_TO_HW(pf); uint32_t i, lut; uint16_t j, num;
> > > +
> > >  /* If both VMDQ and RSS enabled, not all of PF queues are configured.
> > >   * It's necessary to calculate the actual PF queues that are configured.
> > >   */
> > > @@ -13000,6 +13208,7 @@ i40e_config_rss_filter(struct i40e_pf *pf,
> > > return -ENOTSUP;  }
> > >
> > > +lut = 0;
> > >  /* Fill in redirection table */
> > >  for (i = 0, j = 0; i < hw->func_caps.rss_table_size; i++, j++) {
> > > if (j == num) @@ -13010,29 +13219,215 @@
> > > i40e_config_rss_filter(struct i40e_pf *pf,  I40E_WRITE_REG(hw,
> > > I40E_PFQF_HLUT(i >> 2), lut);  }
> > >
> > > -if ((rss_conf.rss_hf & pf->adapter->flow_types_mask) == 0) {
> > > -i40e_pf_disable_rss(pf);
> > > +i40e_config_rss_invalidate_previous_rule(pf, conf);
> > > +
> > > +return 0;
> > > +}
> > > +
> > > +/* config rss hash function */
> > > +static int
> > > +i40e_config_hash_function_add(struct i40e_pf *pf, struct
> > > +i40e_rte_flow_rss_conf *conf) { struct i40e_hw *hw =
> > > +I40E_PF_TO_HW(pf); struct rte_eth_hash_global_conf g_cfg; uint64_t
> > > +rss_inset;
> > > +
> > > +if (conf->conf.func ==
> > > +RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ){
> > > +i40e_set_symmetric_hash_enable_per_port(hw, 1); } else { rss_inset
> > > += ~(ETH_RSS_L3_SRC_ONLY |
> > > ETH_RSS_L3_DST_ONLY |
> > > +    ETH_RSS_L4_SRC_ONLY | ETH_RSS_L4_DST_ONLY); g_cfg.hash_func
> =
> > > +conf->conf.func; g_cfg.sym_hash_enable_mask[0] = conf->conf.types
> &
> > > rss_inset;
> > > +g_cfg.valid_bit_mask[0] = conf->conf.types & rss_inset;
> > > +i40e_set_hash_filter_global_config(hw, &g_cfg); }
> > > +
> > > +i40e_config_rss_invalidate_previous_rule(pf, conf);
> > > +
> > > +return 0;
> > > +}
> > > +
> > > +/* config rss hena disable and set hash input set to defalut */
> >
> > Typo: defalut should be default in above comment.
> >
> 
> Yeah ,I will fix it
> 
> > static
> > > +int i40e_config_hash_pctype_del(struct i40e_pf *pf, struct
> > > +i40e_rte_flow_rss_conf *conf) { struct i40e_hw *hw =
> > > +I40E_PF_TO_HW(pf); struct i40e_rte_flow_rss_conf *rss_info =
> > > +&pf->rss_info; struct rte_eth_rss_conf rss_conf = { .rss_key =
> > > +pf->rss_info.conf.key_len ?
> > > +(void *)(uintptr_t)conf->conf.key : NULL, .rss_key_len =
> > > +pf->rss_info.conf.key_len, }; uint32_t i;
> > > +
> 
> [snip]
> 
> > > --
> > > 2.17.1
> >
Regards,

Bernard