> -----Original Message----- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Wei Dai > Sent: Sunday, November 19, 2017 2:08 PM > To: Wu, Jingjing <jingjing...@intel.com>; Xing, Beilei <beilei.x...@intel.com> > Cc: dev@dpdk.org; Dai, Wei <wei....@intel.com> > Subject: [dpdk-dev] [PATCH v3] net/i40e: determine number of queues per VF > during run time > > Without this patch, the number of queues per i40e VF is defined as 4 > by CONFIG_RTE_LIBRTE_I40E_QUEUE_NUM_PER_VF=4 in config/common_base. > It is fixed value determined in building time and can't be changed > during run time. > With this patch, the number of queues per i40e VF can be determinated > during run time. For example, if the PCI address of an i40e VF is > aaaa:bb.cc, with the EAL parameter -w aaaa:bb.cc,i40e-queue-num-per-vf=8 > , the number of queues per VF is 8. > If there is no "i40e-queue-num-per-vf" setting in EAL parameters, > it is 4 by default as before. And if the value after the > "i40e-queue-num-per-vf" is invalid, it is set as 4 forcibly. > The valid values include 1, 2, 4, 8, 16 . > > Signed-off-by: Wei Dai <wei....@intel.com> > > --- > v3: fix WARNING of coding style issues from checkpa...@dpdk.org > v2: fix WARNING of coding style issues from checkpa...@dpdk.org > --- > config/common_base | 1 - > drivers/net/i40e/i40e_ethdev.c | 67 > ++++++++++++++++++++++++++++++++++++++++-- > 2 files changed, 65 insertions(+), 3 deletions(-) > > diff --git a/config/common_base b/config/common_base > index e74febe..4e20389 100644 > --- a/config/common_base > +++ b/config/common_base > @@ -208,7 +208,6 @@ CONFIG_RTE_LIBRTE_I40E_RX_ALLOW_BULK_ALLOC=y > CONFIG_RTE_LIBRTE_I40E_INC_VECTOR=y > CONFIG_RTE_LIBRTE_I40E_16BYTE_RX_DESC=n > CONFIG_RTE_LIBRTE_I40E_QUEUE_NUM_PER_PF=64 > -CONFIG_RTE_LIBRTE_I40E_QUEUE_NUM_PER_VF=4 > CONFIG_RTE_LIBRTE_I40E_QUEUE_NUM_PER_VM=4 > # interval up to 8160 us, aligned to 2 (or default value) > CONFIG_RTE_LIBRTE_I40E_ITR_INTERVAL=-1 > diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c > index 811cc9f..b23a39e 100644 > --- a/drivers/net/i40e/i40e_ethdev.c > +++ b/drivers/net/i40e/i40e_ethdev.c > @@ -3971,6 +3971,67 @@ i40e_get_cap(struct i40e_hw *hw) > return ret; > } > > +static int i40e_pf_config_vf_rxq_number(struct rte_eth_dev *dev) > +{ > +#define RTE_LIBRTE_I40E_QUEUE_NUM_PER_VF 4 > +#define I40E_QUEUE_NUM_PER_VF_ARG "i40e-queue-num-per-vf"
Does it need to have i40e prefix? Provably other HW type would support similar devarg? > + > + struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private); > + char *s; > + char *ps0, *ps1, *pv; > + char *end; > + unsigned long value; > + > + if (!is_i40e_supported(dev)) > + return -ENOTSUP; Not a big deal - but that function is static, so I suppose the check is redundant. > + > + /* set default queue number per VF as 4 */ > + pf->vf_nb_qp_max = RTE_LIBRTE_I40E_QUEUE_NUM_PER_VF; > + > + if (dev->device->devargs == NULL) > + return 0; > + > + s = rte_zmalloc(__func__, strlen(dev->device->devargs->args) + 1, 0); No need to allocate temp memory from hugepages here. Normal malloc, or even alloca() would do, I think. > + if (s == NULL) > + return -(ENOMEM); > + strcpy(s, dev->device->devargs->args); > + What is the point to write your own parsing function from scratch? Why just not use rte_kvargs API as most other PMDs do? BTW, i40e does the same in few other places. > + ps0 = s; > + do { > + while (isblank(*ps0)) > + ps0++; > + pv = strchr(ps0, '='); > + if (pv == NULL) > + break; > + ps1 = pv - 1; > + *pv++ = 0; > + > + while (isblank(*ps1)) > + *ps1-- = 0; > + > + if (!strcmp(ps0, I40E_QUEUE_NUM_PER_VF_ARG)) { > + errno = 0; > + value = strtoul(pv, &end, 0); > + if (errno != 0 || end == pv) That wouldn't catch something like ...=1blah, Should be *end != '0' (or whatever your stop-character is). Anyway would be much better to use kvargs. Even better to have a common function, something like: int devargs_get_uint(const struct rte_devargs *devargs, const char *name,uint64_t *value); and use it in all appropriate places. Konstantin > + break; > + if (value <= 16 && rte_is_power_of_2(value)) > + pf->vf_nb_qp_max = (uint16_t)value; > + else > + PMD_DRV_LOG(ERR, "Wrong VF queue number = %lu," > + " it must be power of 2 and equal or" > + " less than 16 !", value); > + break; > + } > + ps0 = strchr(pv, ','); > + if (ps0 == NULL) > + break; > + ps0++; > + } while (1); > + > + rte_free(s); > + return 0; > +} > + > static int > i40e_pf_parameter_init(struct rte_eth_dev *dev) > { > @@ -3983,6 +4044,9 @@ i40e_pf_parameter_init(struct rte_eth_dev *dev) > PMD_INIT_LOG(ERR, "HW configuration doesn't support SRIOV"); > return -EINVAL; > } > + > + i40e_pf_config_vf_rxq_number(dev); > + > /* Add the parameter init for LFC */ > pf->fc_conf.pause_time = I40E_DEFAULT_PAUSE_TIME; > pf->fc_conf.high_water[I40E_MAX_TRAFFIC_CLASS] = > I40E_DEFAULT_HIGH_WATER; > @@ -3992,7 +4056,6 @@ i40e_pf_parameter_init(struct rte_eth_dev *dev) > pf->max_num_vsi = hw->func_caps.num_vsis; > pf->lan_nb_qp_max = RTE_LIBRTE_I40E_QUEUE_NUM_PER_PF; > pf->vmdq_nb_qp_max = RTE_LIBRTE_I40E_QUEUE_NUM_PER_VM; > - pf->vf_nb_qp_max = RTE_LIBRTE_I40E_QUEUE_NUM_PER_VF; > > /* FDir queue/VSI allocation */ > pf->fdir_qp_offset = 0; > @@ -4022,7 +4085,7 @@ i40e_pf_parameter_init(struct rte_eth_dev *dev) > pf->vf_qp_offset = pf->lan_qp_offset + pf->lan_nb_qps; > if (hw->func_caps.sr_iov_1_1 && pci_dev->max_vfs) { > pf->flags |= I40E_FLAG_SRIOV; > - pf->vf_nb_qps = RTE_LIBRTE_I40E_QUEUE_NUM_PER_VF; > + pf->vf_nb_qps = pf->vf_nb_qp_max; > pf->vf_num = pci_dev->max_vfs; > PMD_DRV_LOG(DEBUG, > "%u VF VSIs, %u queues per VF VSI, in total %u queues", > -- > 2.7.5