diff mbox series

[v1,1/4] net/iavf: stop the PCI probe in DCF mode

Message ID 20200309141437.11800-2-haiyue.wang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: xiaolong ye
Headers
Series add Intel DCF PMD support |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Performance-Testing fail build patch failure
ci/Intel-compilation fail apply issues

Commit Message

Wang, Haiyue March 9, 2020, 2:14 p.m. UTC 
  A new DCF PMD will be introduced, which runs on Intel VF hardware, and
it is a pure software design to control the advance functionality (such
as switch, ACL) for rest of the VFs.

So if the DCF (Device Config Function) mode is specified by the devarg
'cap=dcf', then it will stop the PCI probe in the iavf PMD.

Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
---
 drivers/net/iavf/iavf_ethdev.c | 41 ++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

Comments

Xiaolong Ye March 9, 2020, 3:38 p.m. UTC | #1 
On 03/09, Haiyue Wang wrote:
>A new DCF PMD will be introduced, which runs on Intel VF hardware, and
>it is a pure software design to control the advance functionality (such
>as switch, ACL) for rest of the VFs.
>
>So if the DCF (Device Config Function) mode is specified by the devarg
>'cap=dcf', then it will stop the PCI probe in the iavf PMD.
>
>Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
>---
> drivers/net/iavf/iavf_ethdev.c | 41 ++++++++++++++++++++++++++++++++++
> 1 file changed, 41 insertions(+)
>
>diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
>index 34913f9c4..8ff26c0e7 100644
>--- a/drivers/net/iavf/iavf_ethdev.c
>+++ b/drivers/net/iavf/iavf_ethdev.c
>@@ -1416,9 +1416,49 @@ iavf_dev_uninit(struct rte_eth_dev *dev)
> 	return 0;
> }
> 
>+static int
>+handle_dcf_arg(__rte_unused const char *key, const char *value,
>+	       __rte_unused void *arg)
>+{
>+	bool *dcf = arg;
>+
>+	if (arg == NULL || value == NULL)
>+		return -EINVAL;
>+
>+	if (strcmp(value, "dcf") == 0)
>+		*dcf = true;
>+	else
>+		*dcf = false;
>+
>+	return 0;
>+}
>+
>+static bool
>+check_cap_dcf_enable(struct rte_devargs *devargs)
>+{
>+	struct rte_kvargs *kvlist;
>+	bool enable = false;
>+
>+	if (devargs == NULL)
>+		return false;
>+
>+	kvlist = rte_kvargs_parse(devargs->args, NULL);
>+	if (kvlist == NULL)
>+		return false;
>+
>+	rte_kvargs_process(kvlist, "cap", handle_dcf_arg, &enable);

Need error handling for failure case.

>+
>+	rte_kvargs_free(kvlist);
>+
>+	return enable;
>+}
>+
> static int eth_iavf_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
> 			     struct rte_pci_device *pci_dev)
> {
>+	if (check_cap_dcf_enable(pci_dev->device.devargs))
>+		return 1; /* continue to probe */

This comment is confusing...

>+
> 	return rte_eth_dev_pci_generic_probe(pci_dev,
> 		sizeof(struct iavf_adapter), iavf_dev_init);
> }
>@@ -1439,6 +1479,7 @@ static struct rte_pci_driver rte_iavf_pmd = {
> RTE_PMD_REGISTER_PCI(net_iavf, rte_iavf_pmd);
> RTE_PMD_REGISTER_PCI_TABLE(net_iavf, pci_id_iavf_map);
> RTE_PMD_REGISTER_KMOD_DEP(net_iavf, "* igb_uio | vfio-pci");
>+RTE_PMD_REGISTER_PARAM_STRING(net_iavf, "cap=dcf");
> RTE_INIT(iavf_init_log)
> {
> 	iavf_logtype_init = rte_log_register("pmd.net.iavf.init");
>-- 
>2.25.1
>
Wang, Haiyue March 10, 2020, 2:19 a.m. UTC | #2 
> -----Original Message-----
> From: Ye, Xiaolong <xiaolong.ye@intel.com>
> Sent: Monday, March 9, 2020 23:38
> To: Wang, Haiyue <haiyue.wang@intel.com>
> Cc: dev@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Xing,
> Beilei <beilei.xing@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>
> Subject: Re: [PATCH v1 1/4] net/iavf: stop the PCI probe in DCF mode
> 
> On 03/09, Haiyue Wang wrote:
> >A new DCF PMD will be introduced, which runs on Intel VF hardware, and
> >it is a pure software design to control the advance functionality (such
> >as switch, ACL) for rest of the VFs.
> >
> >So if the DCF (Device Config Function) mode is specified by the devarg
> >'cap=dcf', then it will stop the PCI probe in the iavf PMD.
> >
> >Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> >---
> > drivers/net/iavf/iavf_ethdev.c | 41 ++++++++++++++++++++++++++++++++++
> > 1 file changed, 41 insertions(+)
> >
> >diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
> >index 34913f9c4..8ff26c0e7 100644
> >--- a/drivers/net/iavf/iavf_ethdev.c
> >+++ b/drivers/net/iavf/iavf_ethdev.c
> >@@ -1416,9 +1416,49 @@ iavf_dev_uninit(struct rte_eth_dev *dev)
> > 	return 0;
> > }
> >
> >+static int
> >+handle_dcf_arg(__rte_unused const char *key, const char *value,
> >+	       __rte_unused void *arg)
> >+{
> >+	bool *dcf = arg;
> >+
> >+	if (arg == NULL || value == NULL)
> >+		return -EINVAL;
> >+
> >+	if (strcmp(value, "dcf") == 0)
> >+		*dcf = true;
> >+	else
> >+		*dcf = false;
> >+
> >+	return 0;
> >+}
> >+
> >+static bool
> >+check_cap_dcf_enable(struct rte_devargs *devargs)
> >+{
> >+	struct rte_kvargs *kvlist;
> >+	bool enable = false;
> >+
> >+	if (devargs == NULL)
> >+		return false;
> >+
> >+	kvlist = rte_kvargs_parse(devargs->args, NULL);
> >+	if (kvlist == NULL)
> >+		return false;
> >+
> >+	rte_kvargs_process(kvlist, "cap", handle_dcf_arg, &enable);
> 
> Need error handling for failure case.
>   

We just need the 'cap=dcf' to check whether it is true, by default
'enable=false' can handle all the cases. ;-)

> >+
> >+	rte_kvargs_free(kvlist);
> >+
> >+	return enable;
> >+}
> >+
> > static int eth_iavf_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
> > 			     struct rte_pci_device *pci_dev)
> > {
> >+	if (check_cap_dcf_enable(pci_dev->device.devargs))
> >+		return 1; /* continue to probe */
> 
> This comment is confusing...

Yes, it should be like 'stop this driver probe to scan the next one'.

> 
> >+
> > 	return rte_eth_dev_pci_generic_probe(pci_dev,
> > 		sizeof(struct iavf_adapter), iavf_dev_init);
> > }
> >@@ -1439,6 +1479,7 @@ static struct rte_pci_driver rte_iavf_pmd = {
> > RTE_PMD_REGISTER_PCI(net_iavf, rte_iavf_pmd);
> > RTE_PMD_REGISTER_PCI_TABLE(net_iavf, pci_id_iavf_map);
> > RTE_PMD_REGISTER_KMOD_DEP(net_iavf, "* igb_uio | vfio-pci");
> >+RTE_PMD_REGISTER_PARAM_STRING(net_iavf, "cap=dcf");
> > RTE_INIT(iavf_init_log)
> > {
> > 	iavf_logtype_init = rte_log_register("pmd.net.iavf.init");
> >--
> >2.25.1
> >
Xiaolong Ye March 10, 2020, 3:37 a.m. UTC | #3 
On 03/10, Wang, Haiyue wrote:
>> -----Original Message-----
>> From: Ye, Xiaolong <xiaolong.ye@intel.com>
>> Sent: Monday, March 9, 2020 23:38
>> To: Wang, Haiyue <haiyue.wang@intel.com>
>> Cc: dev@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Xing,
>> Beilei <beilei.xing@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>
>> Subject: Re: [PATCH v1 1/4] net/iavf: stop the PCI probe in DCF mode
>> 
>> On 03/09, Haiyue Wang wrote:
>> >A new DCF PMD will be introduced, which runs on Intel VF hardware, and
>> >it is a pure software design to control the advance functionality (such
>> >as switch, ACL) for rest of the VFs.
>> >
>> >So if the DCF (Device Config Function) mode is specified by the devarg
>> >'cap=dcf', then it will stop the PCI probe in the iavf PMD.
>> >
>> >Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
>> >---
>> > drivers/net/iavf/iavf_ethdev.c | 41 ++++++++++++++++++++++++++++++++++
>> > 1 file changed, 41 insertions(+)
>> >
>> >diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
>> >index 34913f9c4..8ff26c0e7 100644
>> >--- a/drivers/net/iavf/iavf_ethdev.c
>> >+++ b/drivers/net/iavf/iavf_ethdev.c
>> >@@ -1416,9 +1416,49 @@ iavf_dev_uninit(struct rte_eth_dev *dev)
>> > 	return 0;
>> > }
>> >
>> >+static int
>> >+handle_dcf_arg(__rte_unused const char *key, const char *value,
>> >+	       __rte_unused void *arg)
>> >+{
>> >+	bool *dcf = arg;
>> >+
>> >+	if (arg == NULL || value == NULL)
>> >+		return -EINVAL;
>> >+
>> >+	if (strcmp(value, "dcf") == 0)
>> >+		*dcf = true;
>> >+	else
>> >+		*dcf = false;
>> >+
>> >+	return 0;
>> >+}
>> >+
>> >+static bool
>> >+check_cap_dcf_enable(struct rte_devargs *devargs)
>> >+{
>> >+	struct rte_kvargs *kvlist;
>> >+	bool enable = false;
>> >+
>> >+	if (devargs == NULL)
>> >+		return false;
>> >+
>> >+	kvlist = rte_kvargs_parse(devargs->args, NULL);
>> >+	if (kvlist == NULL)
>> >+		return false;
>> >+
>> >+	rte_kvargs_process(kvlist, "cap", handle_dcf_arg, &enable);
>> 
>> Need error handling for failure case.
>>   
>
>We just need the 'cap=dcf' to check whether it is true, by default
>'enable=false' can handle all the cases. ;-)
>

Yes, from function point of view, this could work. But it is still good practice
to check the return value of the function.

Thanks,
Xiaolong
Wang, Haiyue March 10, 2020, 4:26 a.m. UTC | #4 
> -----Original Message-----
> From: Ye, Xiaolong <xiaolong.ye@intel.com>
> Sent: Tuesday, March 10, 2020 11:38
> To: Wang, Haiyue <haiyue.wang@intel.com>
> Cc: dev@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Xing,
> Beilei <beilei.xing@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>
> Subject: Re: [PATCH v1 1/4] net/iavf: stop the PCI probe in DCF mode
> 
> On 03/10, Wang, Haiyue wrote:
> >> -----Original Message-----
> >> From: Ye, Xiaolong <xiaolong.ye@intel.com>
> >> Sent: Monday, March 9, 2020 23:38
> >> To: Wang, Haiyue <haiyue.wang@intel.com>
> >> Cc: dev@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Xing,
> >> Beilei <beilei.xing@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>
> >> Subject: Re: [PATCH v1 1/4] net/iavf: stop the PCI probe in DCF mode
> >>
> >> On 03/09, Haiyue Wang wrote:
> >> >A new DCF PMD will be introduced, which runs on Intel VF hardware, and
> >> >it is a pure software design to control the advance functionality (such
> >> >as switch, ACL) for rest of the VFs.
> >> >
> >> >So if the DCF (Device Config Function) mode is specified by the devarg
> >> >'cap=dcf', then it will stop the PCI probe in the iavf PMD.
> >> >
> >> >Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> >> >---
> >> > drivers/net/iavf/iavf_ethdev.c | 41 ++++++++++++++++++++++++++++++++++
> >> > 1 file changed, 41 insertions(+)
> >> >
> >> >diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
> >> >index 34913f9c4..8ff26c0e7 100644
> >> >--- a/drivers/net/iavf/iavf_ethdev.c
> >> >+++ b/drivers/net/iavf/iavf_ethdev.c
> >> >@@ -1416,9 +1416,49 @@ iavf_dev_uninit(struct rte_eth_dev *dev)
> >> > 	return 0;
> >> > }
> >> >
> >> >+static int
> >> >+handle_dcf_arg(__rte_unused const char *key, const char *value,
> >> >+	       __rte_unused void *arg)
> >> >+{
> >> >+	bool *dcf = arg;
> >> >+
> >> >+	if (arg == NULL || value == NULL)
> >> >+		return -EINVAL;
> >> >+
> >> >+	if (strcmp(value, "dcf") == 0)
> >> >+		*dcf = true;
> >> >+	else
> >> >+		*dcf = false;
> >> >+
> >> >+	return 0;
> >> >+}
> >> >+
> >> >+static bool
> >> >+check_cap_dcf_enable(struct rte_devargs *devargs)
> >> >+{
> >> >+	struct rte_kvargs *kvlist;
> >> >+	bool enable = false;
> >> >+
> >> >+	if (devargs == NULL)
> >> >+		return false;
> >> >+
> >> >+	kvlist = rte_kvargs_parse(devargs->args, NULL);
> >> >+	if (kvlist == NULL)
> >> >+		return false;
> >> >+
> >> >+	rte_kvargs_process(kvlist, "cap", handle_dcf_arg, &enable);
> >>
> >> Need error handling for failure case.
> >>
> >
> >We just need the 'cap=dcf' to check whether it is true, by default
> >'enable=false' can handle all the cases. ;-)
> >
> 
> Yes, from function point of view, this could work. But it is still good practice
> to check the return value of the function.
> 

Make sense, will handle it in v2.

> Thanks,
> Xiaolong
Jingjing Wu March 23, 2020, 1:50 a.m. UTC | #5 
+static int
+handle_dcf_arg(__rte_unused const char *key, const char *value,
+	       __rte_unused void *arg)
__rte_unused is not needed here.

+{
+	bool *dcf = arg;
+
+	if (arg == NULL || value == NULL)
+		return -EINVAL;
+
+	if (strcmp(value, "dcf") == 0)
+		*dcf = true;
+	else
+		*dcf = false;
+
+	return 0;
+}
+
Wang, Haiyue March 23, 2020, 1:55 a.m. UTC | #6 
> -----Original Message-----
> From: Wu, Jingjing <jingjing.wu@intel.com>
> Sent: Monday, March 23, 2020 09:51
> To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org; Ye, Xiaolong <xiaolong.ye@intel.com>; Zhang,
> Qi Z <qi.z.zhang@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> Cc: Zhao1, Wei <wei.zhao1@intel.com>; Wang, Haiyue <haiyue.wang@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v1 1/4] net/iavf: stop the PCI probe in DCF mode
> 
> +static int
> +handle_dcf_arg(__rte_unused const char *key, const char *value,
> +	       __rte_unused void *arg)
> __rte_unused is not needed here.

Yes, has been rewritten and fixed in v3. ;-)

> 
> +{
> +	bool *dcf = arg;
> +
> +	if (arg == NULL || value == NULL)
> +		return -EINVAL;
> +
> +	if (strcmp(value, "dcf") == 0)
> +		*dcf = true;
> +	else
> +		*dcf = false;
> +
> +	return 0;
> +}
> +
diff mbox series

Patch

diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index 34913f9c4..8ff26c0e7 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -1416,9 +1416,49 @@  iavf_dev_uninit(struct rte_eth_dev *dev)
 	return 0;
 }
 
+static int
+handle_dcf_arg(__rte_unused const char *key, const char *value,
+	       __rte_unused void *arg)
+{
+	bool *dcf = arg;
+
+	if (arg == NULL || value == NULL)
+		return -EINVAL;
+
+	if (strcmp(value, "dcf") == 0)
+		*dcf = true;
+	else
+		*dcf = false;
+
+	return 0;
+}
+
+static bool
+check_cap_dcf_enable(struct rte_devargs *devargs)
+{
+	struct rte_kvargs *kvlist;
+	bool enable = false;
+
+	if (devargs == NULL)
+		return false;
+
+	kvlist = rte_kvargs_parse(devargs->args, NULL);
+	if (kvlist == NULL)
+		return false;
+
+	rte_kvargs_process(kvlist, "cap", handle_dcf_arg, &enable);
+
+	rte_kvargs_free(kvlist);
+
+	return enable;
+}
+
 static int eth_iavf_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 			     struct rte_pci_device *pci_dev)
 {
+	if (check_cap_dcf_enable(pci_dev->device.devargs))
+		return 1; /* continue to probe */
+
 	return rte_eth_dev_pci_generic_probe(pci_dev,
 		sizeof(struct iavf_adapter), iavf_dev_init);
 }
@@ -1439,6 +1479,7 @@  static struct rte_pci_driver rte_iavf_pmd = {
 RTE_PMD_REGISTER_PCI(net_iavf, rte_iavf_pmd);
 RTE_PMD_REGISTER_PCI_TABLE(net_iavf, pci_id_iavf_map);
 RTE_PMD_REGISTER_KMOD_DEP(net_iavf, "* igb_uio | vfio-pci");
+RTE_PMD_REGISTER_PARAM_STRING(net_iavf, "cap=dcf");
 RTE_INIT(iavf_init_log)
 {
 	iavf_logtype_init = rte_log_register("pmd.net.iavf.init");