[dpdk-dev,v6] net/i40e: determine number of queues per VF during run time

Message ID 1511770139-48286-1-git-send-email-wei.dai@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Helin Zhang
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Wei Dai Nov. 27, 2017, 8:08 a.m. UTC
  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,queue-num-per-vf=8,
the number of queues per VF is 8.
If there is no "queue-num-per-vf" setting in EAL parameters, it is 4
by default as before. And if the value after the "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.dai@intel.com>

---
v6:
    fix a small bug when detecting end character of strtoul
v5:
    fix git log message and WARNING of coding stype
v4:
    use rte_kvargs instead of pervious parsing function;
    use malloc/free instead of rte_zmalloc/rte_free.
v3:
    fix WARNING of coding style issues from checkpatch@dpdk.org
v2:
    fix WARNING of coding style issues from checkpatch@dpdk.org
---
 config/common_base             |  1 -
 drivers/net/i40e/i40e_ethdev.c | 67 ++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 65 insertions(+), 3 deletions(-)
  

Comments

Ananyev, Konstantin Dec. 3, 2017, 11:19 a.m. UTC | #1
Hi Wei,

> -----Original Message-----
> From: Dai, Wei
> Sent: Monday, November 27, 2017 8:09 AM
> To: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org; Dai, Wei <wei.dai@intel.com>
> Subject: [PATCH v6] 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,queue-num-per-vf=8,
> the number of queues per VF is 8.
> If there is no "queue-num-per-vf" setting in EAL parameters, it is 4
> by default as before. And if the value after the "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.dai@intel.com>
> 
> ---
> v6:
>     fix a small bug when detecting end character of strtoul
> v5:
>     fix git log message and WARNING of coding stype
> v4:
>     use rte_kvargs instead of pervious parsing function;
>     use malloc/free instead of rte_zmalloc/rte_free.
> v3:
>     fix WARNING of coding style issues from checkpatch@dpdk.org
> v2:
>     fix WARNING of coding style issues from checkpatch@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..6eceea1 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;
>  }
> 
> +#define RTE_LIBRTE_I40E_QUEUE_NUM_PER_VF	4
> +#define QUEUE_NUM_PER_VF_ARG			"queue-num-per-vf"
> +static int i40e_pf_parse_vf_queue_number_handler(const char *key,
> +		const char *value,
> +		void *opaque)
> +{
> +	struct i40e_pf *pf;
> +	unsigned long num;
> +	char *end;
> +
> +	pf = (struct i40e_pf *)opaque;
> +	RTE_SET_USED(key);
> +
> +	errno = 0;
> +	num = strtoul(value, &end, 0);
> +	if (errno != 0 || end == value || *end != 0) {
> +		PMD_DRV_LOG(WARNING, "Wrong VF queue number = %s, Now it is "
> +			    "kept the value = %hu", value, pf->vf_nb_qp_max);
> +		return -(EINVAL);
> +	}
> +
> +	if (num <= 16 && rte_is_power_of_2(num))

As a nit - better to use some macro instead of '16' here.
Apart from that - looks good to me.
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

> +		pf->vf_nb_qp_max = (uint16_t)num;
> +	else
> +		/* here return 0 to make next valid same argument work */
> +		PMD_DRV_LOG(WARNING, "Wrong VF queue number = %lu, it must be "
> +			    "power of 2 and equal or less than 16 !, Now it is "
> +			    "kept the value = %hu", num, pf->vf_nb_qp_max);
> +
> +	return 0;
> +}
> +
> +static int i40e_pf_config_vf_rxq_number(struct rte_eth_dev *dev)
> +{
> +	static const char * const valid_keys[] = {QUEUE_NUM_PER_VF_ARG, ""};
> +	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> +	struct rte_kvargs *kvlist;
> +
> +	/* 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;
> +
> +	kvlist = rte_kvargs_parse(dev->device->devargs->args, valid_keys);
> +	if (kvlist == NULL)
> +		return -(EINVAL);
> +
> +	if (rte_kvargs_count(kvlist, QUEUE_NUM_PER_VF_ARG) > 1)
> +		PMD_DRV_LOG(WARNING, "More than one argument \"%s\" and only "
> +			    "the first invalid or last valid one is used !",
> +			    QUEUE_NUM_PER_VF_ARG);
> +
> +	rte_kvargs_process(kvlist, QUEUE_NUM_PER_VF_ARG,
> +			   i40e_pf_parse_vf_queue_number_handler, pf);
> +
> +	rte_kvargs_free(kvlist);
> +
> +	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
  
Wei Dai Dec. 8, 2017, 2:07 a.m. UTC | #2
> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Sunday, December 3, 2017 7:20 PM
> To: Dai, Wei <wei.dai@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
> Xing, Beilei <beilei.xing@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH v6] net/i40e: determine number of queues per VF during
> run time
> 
> Hi Wei,
> 
> > -----Original Message-----
> > From: Dai, Wei
> > Sent: Monday, November 27, 2017 8:09 AM
> > To: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> > <beilei.xing@intel.com>; Ananyev, Konstantin
> > <konstantin.ananyev@intel.com>
> > Cc: dev@dpdk.org; Dai, Wei <wei.dai@intel.com>
> > Subject: [PATCH v6] 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
Here it should be 'if the PCI address of an i40e PF is'
I will correct it in my v7 patch.

> > aaaa:bb.cc, with the EAL parameter -w aaaa:bb.cc,queue-num-per-vf=8,
> > the number of queues per VF is 8.
> > If there is no "queue-num-per-vf" setting in EAL parameters, it is 4
> > by default as before. And if the value after the "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.dai@intel.com>
> >
> > ---
> > v6:
> >     fix a small bug when detecting end character of strtoul
> > v5:
> >     fix git log message and WARNING of coding stype
> > v4:
> >     use rte_kvargs instead of pervious parsing function;
> >     use malloc/free instead of rte_zmalloc/rte_free.
> > v3:
> >     fix WARNING of coding style issues from checkpatch@dpdk.org
> > v2:
> >     fix WARNING of coding style issues from checkpatch@dpdk.org
> > ---
> >  config/common_base             |  1 -
> >  drivers/net/i40e/i40e_ethdev.c | 67
> > ++++++++++++++++++++++++++++++++++++++++--
> >  2 files changed, 65 insertions(+), 3 deletions(-)
> >
> > +	if (errno != 0 || end == value || *end != 0) {
> > +		PMD_DRV_LOG(WARNING, "Wrong VF queue number = %s, Now it
> is "
> > +			    "kept the value = %hu", value, pf->vf_nb_qp_max);
> > +		return -(EINVAL);
> > +	}
> > +
> > +	if (num <= 16 && rte_is_power_of_2(num))
> 
> As a nit - better to use some macro instead of '16' here.
> Apart from that - looks good to me.
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

Thanks for your Ack, Konstantin.
I will use the macro I40E_MAX_QP_NUM_PER_VF instead of 16 in my v7 patch.
This macro is already defined as 16 in i40e_ethdev.h

> 
> > +		pf->vf_nb_qp_max = (uint16_t)num;
> > +	else
> > +		/* here return 0 to make next valid same argument work */
> > +		PMD_DRV_LOG(WARNING, "Wrong VF queue number = %lu, it
> must be "
> > +			    "power of 2 and equal or less than 16 !, Now it is "
> > +			    "kept the value = %hu", num, pf->vf_nb_qp_max);
> > +
> > +	return 0;
> > +}
  

Patch

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..6eceea1 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;
 }
 
+#define RTE_LIBRTE_I40E_QUEUE_NUM_PER_VF	4
+#define QUEUE_NUM_PER_VF_ARG			"queue-num-per-vf"
+static int i40e_pf_parse_vf_queue_number_handler(const char *key,
+		const char *value,
+		void *opaque)
+{
+	struct i40e_pf *pf;
+	unsigned long num;
+	char *end;
+
+	pf = (struct i40e_pf *)opaque;
+	RTE_SET_USED(key);
+
+	errno = 0;
+	num = strtoul(value, &end, 0);
+	if (errno != 0 || end == value || *end != 0) {
+		PMD_DRV_LOG(WARNING, "Wrong VF queue number = %s, Now it is "
+			    "kept the value = %hu", value, pf->vf_nb_qp_max);
+		return -(EINVAL);
+	}
+
+	if (num <= 16 && rte_is_power_of_2(num))
+		pf->vf_nb_qp_max = (uint16_t)num;
+	else
+		/* here return 0 to make next valid same argument work */
+		PMD_DRV_LOG(WARNING, "Wrong VF queue number = %lu, it must be "
+			    "power of 2 and equal or less than 16 !, Now it is "
+			    "kept the value = %hu", num, pf->vf_nb_qp_max);
+
+	return 0;
+}
+
+static int i40e_pf_config_vf_rxq_number(struct rte_eth_dev *dev)
+{
+	static const char * const valid_keys[] = {QUEUE_NUM_PER_VF_ARG, ""};
+	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
+	struct rte_kvargs *kvlist;
+
+	/* 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;
+
+	kvlist = rte_kvargs_parse(dev->device->devargs->args, valid_keys);
+	if (kvlist == NULL)
+		return -(EINVAL);
+
+	if (rte_kvargs_count(kvlist, QUEUE_NUM_PER_VF_ARG) > 1)
+		PMD_DRV_LOG(WARNING, "More than one argument \"%s\" and only "
+			    "the first invalid or last valid one is used !",
+			    QUEUE_NUM_PER_VF_ARG);
+
+	rte_kvargs_process(kvlist, QUEUE_NUM_PER_VF_ARG,
+			   i40e_pf_parse_vf_queue_number_handler, pf);
+
+	rte_kvargs_free(kvlist);
+
+	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",