net/i40e: fix multiple device arguments parsing error

Message ID 1532500602-130919-1-git-send-email-beilei.xing@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Qi Zhang
Headers
Series net/i40e: fix multiple device arguments parsing error |

Checks

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

Commit Message

Xing, Beilei July 25, 2018, 6:36 a.m. UTC
  There will be parsing error when using multiple device
arguments. This patch fixes the issue

Fixes: cfdfca493cae ("net/i40e: fix multiple driver support")
Fixes: ee653bd80044 ("net/i40e: determine number of queues per VF at run time")
Cc: stable@dpdk.org

Signed-off-by: Beilei Xing <beilei.xing@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 45 ++++++++++++++++++++++++++++++------------
 1 file changed, 32 insertions(+), 13 deletions(-)
  

Comments

Qi Zhang July 25, 2018, 12:43 p.m. UTC | #1
Hi Beilei

> -----Original Message-----
> From: Xing, Beilei
> Sent: Wednesday, July 25, 2018 2:37 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>;
> stable@dpdk.org
> Subject: [PATCH] net/i40e: fix multiple device arguments parsing error
> 
> There will be parsing error when using multiple device arguments. This patch
> fixes the issue

Could you explain more about what is the issue this patch is going to fix.
Though I think the patch looks make things better, but I can't figure out what is the issue to fix.
Maybe you can add an example of the devargs with problem and the error message 

Regards
Qi

> 
> Fixes: cfdfca493cae ("net/i40e: fix multiple driver support")
> Fixes: ee653bd80044 ("net/i40e: determine number of queues per VF at run
> time")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Beilei Xing <beilei.xing@intel.com>
> ---
>  drivers/net/i40e/i40e_ethdev.c | 45
> ++++++++++++++++++++++++++++++------------
>  1 file changed, 32 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 66431e0..e3063cc 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -42,6 +42,8 @@
> 
>  #define ETH_I40E_FLOATING_VEB_ARG	"enable_floating_veb"
>  #define ETH_I40E_FLOATING_VEB_LIST_ARG	"floating_veb_list"
> +#define ETH_I40E_SUPPORT_MULTI_DRIVER	"support-multi-driver"
> +#define ETH_I40E_QUEUE_NUM_PER_VF_ARG	"queue-num-per-vf"
> 
>  #define I40E_CLEAR_PXE_WAIT_MS     200
> 
> @@ -401,6 +403,13 @@ static void i40e_notify_all_vfs_link_status(struct
> rte_eth_dev *dev);  int i40e_logtype_init;  int i40e_logtype_driver;
> 
> +static const char *const valid_keys[] = {
> +	ETH_I40E_FLOATING_VEB_ARG,
> +	ETH_I40E_FLOATING_VEB_LIST_ARG,
> +	ETH_I40E_SUPPORT_MULTI_DRIVER,
> +	ETH_I40E_QUEUE_NUM_PER_VF_ARG,
> +	NULL};
> +
>  static const struct rte_pci_id pci_id_i40e_map[] = {
>  	{ RTE_PCI_DEVICE(I40E_INTEL_VENDOR_ID, I40E_DEV_ID_SFP_XL710) },
>  	{ RTE_PCI_DEVICE(I40E_INTEL_VENDOR_ID, I40E_DEV_ID_QEMU) }, @@
> -852,7 +861,7 @@ config_vf_floating_veb(struct rte_devargs *devargs,
>  	if (devargs == NULL)
>  		return;
> 
> -	kvlist = rte_kvargs_parse(devargs->args, NULL);
> +	kvlist = rte_kvargs_parse(devargs->args, valid_keys);
>  	if (kvlist == NULL)
>  		return;
> 
> @@ -893,7 +902,7 @@ is_floating_veb_supported(struct rte_devargs
> *devargs)
>  	if (devargs == NULL)
>  		return 0;
> 
> -	kvlist = rte_kvargs_parse(devargs->args, NULL);
> +	kvlist = rte_kvargs_parse(devargs->args, valid_keys);
>  	if (kvlist == NULL)
>  		return 0;
> 
> @@ -1100,8 +1109,6 @@ i40e_init_queue_region_conf(struct rte_eth_dev
> *dev)
>  	memset(info, 0, sizeof(struct i40e_queue_regions));  }
> 
> -#define ETH_I40E_SUPPORT_MULTI_DRIVER	"support-multi-driver"
> -
>  static int
>  i40e_parse_multi_drv_handler(__rte_unused const char *key,
>  			       const char *value,
> @@ -1133,9 +1140,8 @@ static int
>  i40e_support_multi_driver(struct rte_eth_dev *dev)  {
>  	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> -	static const char *const valid_keys[] = {
> -		ETH_I40E_SUPPORT_MULTI_DRIVER, NULL};
>  	struct rte_kvargs *kvlist;
> +	int kvargs_count;
> 
>  	/* Enable global configuration by default */
>  	pf->support_multi_driver = false;
> @@ -1147,7 +1153,13 @@ i40e_support_multi_driver(struct rte_eth_dev
> *dev)
>  	if (!kvlist)
>  		return -EINVAL;
> 
> -	if (rte_kvargs_count(kvlist, ETH_I40E_SUPPORT_MULTI_DRIVER) > 1)
> +	kvargs_count = rte_kvargs_count(kvlist,
> ETH_I40E_SUPPORT_MULTI_DRIVER);
> +	if (!kvargs_count) {
> +		rte_kvargs_free(kvlist);
> +		return 0;
> +	}
> +
> +	if (kvargs_count > 1)
>  		PMD_DRV_LOG(WARNING, "More than one argument \"%s\" and
> only "
>  			    "the first invalid or last valid one is used !",
>  			    ETH_I40E_SUPPORT_MULTI_DRIVER);
> @@ -4357,7 +4369,6 @@ i40e_get_cap(struct i40e_hw *hw)  }
> 
>  #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,
> @@ -4391,9 +4402,9 @@ static int
> i40e_pf_parse_vf_queue_number_handler(const char *key,
> 
>  static int i40e_pf_config_vf_rxq_number(struct rte_eth_dev *dev)  {
> -	static const char * const valid_keys[] = {QUEUE_NUM_PER_VF_ARG,
> NULL};
>  	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
>  	struct rte_kvargs *kvlist;
> +	int kvargs_count;
> 
>  	/* set default queue number per VF as 4 */
>  	pf->vf_nb_qp_max = RTE_LIBRTE_I40E_QUEUE_NUM_PER_VF; @@
> -4405,12 +4416,18 @@ static int i40e_pf_config_vf_rxq_number(struct
> rte_eth_dev *dev)
>  	if (kvlist == NULL)
>  		return -(EINVAL);
> 
> -	if (rte_kvargs_count(kvlist, QUEUE_NUM_PER_VF_ARG) > 1)
> +	kvargs_count = rte_kvargs_count(kvlist,
> ETH_I40E_QUEUE_NUM_PER_VF_ARG);
> +	if (!kvargs_count) {
> +		rte_kvargs_free(kvlist);
> +		return 0;
> +	}
> +
> +	if (kvargs_count > 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);
> +			    ETH_I40E_QUEUE_NUM_PER_VF_ARG);
> 
> -	rte_kvargs_process(kvlist, QUEUE_NUM_PER_VF_ARG,
> +	rte_kvargs_process(kvlist, ETH_I40E_QUEUE_NUM_PER_VF_ARG,
>  			   i40e_pf_parse_vf_queue_number_handler, pf);
> 
>  	rte_kvargs_free(kvlist);
> @@ -12509,5 +12526,7 @@ i40e_init_log(void)  }
> 
>  RTE_PMD_REGISTER_PARAM_STRING(net_i40e,
> -			      QUEUE_NUM_PER_VF_ARG "=1|2|4|8|16"
> +			      ETH_I40E_FLOATING_VEB_ARG "=1"
> +			      ETH_I40E_FLOATING_VEB_LIST_ARG "=<string>"
> +			      ETH_I40E_QUEUE_NUM_PER_VF_ARG "=1|2|4|8|16"
>  			      ETH_I40E_SUPPORT_MULTI_DRIVER "=1");
> --
> 2.5.5
  

Patch

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 66431e0..e3063cc 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -42,6 +42,8 @@ 
 
 #define ETH_I40E_FLOATING_VEB_ARG	"enable_floating_veb"
 #define ETH_I40E_FLOATING_VEB_LIST_ARG	"floating_veb_list"
+#define ETH_I40E_SUPPORT_MULTI_DRIVER	"support-multi-driver"
+#define ETH_I40E_QUEUE_NUM_PER_VF_ARG	"queue-num-per-vf"
 
 #define I40E_CLEAR_PXE_WAIT_MS     200
 
@@ -401,6 +403,13 @@  static void i40e_notify_all_vfs_link_status(struct rte_eth_dev *dev);
 int i40e_logtype_init;
 int i40e_logtype_driver;
 
+static const char *const valid_keys[] = {
+	ETH_I40E_FLOATING_VEB_ARG,
+	ETH_I40E_FLOATING_VEB_LIST_ARG,
+	ETH_I40E_SUPPORT_MULTI_DRIVER,
+	ETH_I40E_QUEUE_NUM_PER_VF_ARG,
+	NULL};
+
 static const struct rte_pci_id pci_id_i40e_map[] = {
 	{ RTE_PCI_DEVICE(I40E_INTEL_VENDOR_ID, I40E_DEV_ID_SFP_XL710) },
 	{ RTE_PCI_DEVICE(I40E_INTEL_VENDOR_ID, I40E_DEV_ID_QEMU) },
@@ -852,7 +861,7 @@  config_vf_floating_veb(struct rte_devargs *devargs,
 	if (devargs == NULL)
 		return;
 
-	kvlist = rte_kvargs_parse(devargs->args, NULL);
+	kvlist = rte_kvargs_parse(devargs->args, valid_keys);
 	if (kvlist == NULL)
 		return;
 
@@ -893,7 +902,7 @@  is_floating_veb_supported(struct rte_devargs *devargs)
 	if (devargs == NULL)
 		return 0;
 
-	kvlist = rte_kvargs_parse(devargs->args, NULL);
+	kvlist = rte_kvargs_parse(devargs->args, valid_keys);
 	if (kvlist == NULL)
 		return 0;
 
@@ -1100,8 +1109,6 @@  i40e_init_queue_region_conf(struct rte_eth_dev *dev)
 	memset(info, 0, sizeof(struct i40e_queue_regions));
 }
 
-#define ETH_I40E_SUPPORT_MULTI_DRIVER	"support-multi-driver"
-
 static int
 i40e_parse_multi_drv_handler(__rte_unused const char *key,
 			       const char *value,
@@ -1133,9 +1140,8 @@  static int
 i40e_support_multi_driver(struct rte_eth_dev *dev)
 {
 	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
-	static const char *const valid_keys[] = {
-		ETH_I40E_SUPPORT_MULTI_DRIVER, NULL};
 	struct rte_kvargs *kvlist;
+	int kvargs_count;
 
 	/* Enable global configuration by default */
 	pf->support_multi_driver = false;
@@ -1147,7 +1153,13 @@  i40e_support_multi_driver(struct rte_eth_dev *dev)
 	if (!kvlist)
 		return -EINVAL;
 
-	if (rte_kvargs_count(kvlist, ETH_I40E_SUPPORT_MULTI_DRIVER) > 1)
+	kvargs_count = rte_kvargs_count(kvlist, ETH_I40E_SUPPORT_MULTI_DRIVER);
+	if (!kvargs_count) {
+		rte_kvargs_free(kvlist);
+		return 0;
+	}
+
+	if (kvargs_count > 1)
 		PMD_DRV_LOG(WARNING, "More than one argument \"%s\" and only "
 			    "the first invalid or last valid one is used !",
 			    ETH_I40E_SUPPORT_MULTI_DRIVER);
@@ -4357,7 +4369,6 @@  i40e_get_cap(struct i40e_hw *hw)
 }
 
 #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,
@@ -4391,9 +4402,9 @@  static int i40e_pf_parse_vf_queue_number_handler(const char *key,
 
 static int i40e_pf_config_vf_rxq_number(struct rte_eth_dev *dev)
 {
-	static const char * const valid_keys[] = {QUEUE_NUM_PER_VF_ARG, NULL};
 	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
 	struct rte_kvargs *kvlist;
+	int kvargs_count;
 
 	/* set default queue number per VF as 4 */
 	pf->vf_nb_qp_max = RTE_LIBRTE_I40E_QUEUE_NUM_PER_VF;
@@ -4405,12 +4416,18 @@  static int i40e_pf_config_vf_rxq_number(struct rte_eth_dev *dev)
 	if (kvlist == NULL)
 		return -(EINVAL);
 
-	if (rte_kvargs_count(kvlist, QUEUE_NUM_PER_VF_ARG) > 1)
+	kvargs_count = rte_kvargs_count(kvlist, ETH_I40E_QUEUE_NUM_PER_VF_ARG);
+	if (!kvargs_count) {
+		rte_kvargs_free(kvlist);
+		return 0;
+	}
+
+	if (kvargs_count > 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);
+			    ETH_I40E_QUEUE_NUM_PER_VF_ARG);
 
-	rte_kvargs_process(kvlist, QUEUE_NUM_PER_VF_ARG,
+	rte_kvargs_process(kvlist, ETH_I40E_QUEUE_NUM_PER_VF_ARG,
 			   i40e_pf_parse_vf_queue_number_handler, pf);
 
 	rte_kvargs_free(kvlist);
@@ -12509,5 +12526,7 @@  i40e_init_log(void)
 }
 
 RTE_PMD_REGISTER_PARAM_STRING(net_i40e,
-			      QUEUE_NUM_PER_VF_ARG "=1|2|4|8|16"
+			      ETH_I40E_FLOATING_VEB_ARG "=1"
+			      ETH_I40E_FLOATING_VEB_LIST_ARG "=<string>"
+			      ETH_I40E_QUEUE_NUM_PER_VF_ARG "=1|2|4|8|16"
 			      ETH_I40E_SUPPORT_MULTI_DRIVER "=1");