[dpdk-dev] [PATCH 1/3] net/ixgbe: revert default PF PMD device name

[dpdk-dev] [PATCH 1/3] net/ixgbe: revert default PF PMD device name

[Declan Doherty]

Changes introduced by cf80ba6e2038 modified the default name generated
for the IXGBE PF PMD, this patch reverts the default name to the
original PCI BDBF.

Fixes: cf80ba6e2038 ("net/ixgbe: add support for representor ports")
Signed-off-by: Declan Doherty <declan.doherty@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 6088c7e48..0ccf55dc8 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1736,10 +1736,7 @@ eth_ixgbe_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 			return retval;
 	}
 
-	/* physical port net_bdf_port */
-	snprintf(name, sizeof(name), "net_%s_%d", pci_dev->device.name, 0);
-
-	retval = rte_eth_dev_create(&pci_dev->device, name,
+	retval = rte_eth_dev_create(&pci_dev->device, pci_dev->device.name,
 		sizeof(struct ixgbe_adapter),
 		eth_dev_pci_specific_init, pci_dev,
 		eth_ixgbe_dev_init, NULL);
@@ -1748,7 +1745,8 @@ eth_ixgbe_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 		return retval;
 
 	/* probe VF representor ports */
-	struct rte_eth_dev *pf_ethdev = rte_eth_dev_allocated(name);
+	struct rte_eth_dev *pf_ethdev = rte_eth_dev_allocated(
+		pci_dev->device.name);
 
 	for (i = 0; i < eth_da.nb_representor_ports; i++) {
 		struct ixgbe_vf_info *vfinfo;
-- 
2.14.3

[dpdk-dev] [PATCH 2/3] net/ixgbe: initialise nb_representor_ports value

[Declan Doherty]

Initialise rte_ethdev_args nb_representor_ports to zero to handle
the case where no devargs are passed to the IXGBE PF on
device probe, so that there is no invalid attempts to create
representor ports.

Coverity Issue: 277231
Fixes: cf80ba6e2038 ("net/ixgbe: add support for representor ports")

Signed-off-by: Declan Doherty <declan.doherty@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 0ccf55dc8..283dd7e49 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1725,8 +1725,7 @@ eth_ixgbe_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 		struct rte_pci_device *pci_dev)
 {
 	char name[RTE_ETH_NAME_MAX_LEN];
-
-	struct rte_eth_devargs eth_da;
+	struct rte_eth_devargs eth_da = { .nb_representor_ports = 0 };
 	int i, retval;
 
 	if (pci_dev->device.devargs) {
-- 
2.14.3

[dpdk-dev] [PATCH 3/3] net/ixgbe: add null pointer check for pf_ethdev

[Declan Doherty]

Add NULL parameter check for rte_eth_dev_allocated() API call to
eth_ixgbe_pci_probe().

Coverity Issue: 277216
Fixes: cf80ba6e2038 ("net/ixgbe: add support for representor ports")

Signed-off-by: Declan Doherty <declan.doherty@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 283dd7e49..75f927c06 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1747,6 +1747,9 @@ eth_ixgbe_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 	struct rte_eth_dev *pf_ethdev = rte_eth_dev_allocated(
 		pci_dev->device.name);
 
+	if (pf_ethdev == NULL)
+		return -ENODEV;
+
 	for (i = 0; i < eth_da.nb_representor_ports; i++) {
 		struct ixgbe_vf_info *vfinfo;
 		struct ixgbe_vf_representor representor;
-- 
2.14.3

Re: [dpdk-dev] [PATCH 2/3] net/ixgbe: initialise nb_representor_ports value

[Ananyev, Konstantin]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Declan Doherty
> Sent: Monday, April 30, 2018 4:33 PM
> To: dev@dpdk.org
> Cc: Doherty, Declan <declan.doherty@intel.com>
> Subject: [dpdk-dev] [PATCH 2/3] net/ixgbe: initialise nb_representor_ports value
> 
> Initialise rte_ethdev_args nb_representor_ports to zero to handle
> the case where no devargs are passed to the IXGBE PF on
> device probe, so that there is no invalid attempts to create
> representor ports.
> 
> Coverity Issue: 277231
> Fixes: cf80ba6e2038 ("net/ixgbe: add support for representor ports")
> 
> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
> ---
>  drivers/net/ixgbe/ixgbe_ethdev.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 0ccf55dc8..283dd7e49 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -1725,8 +1725,7 @@ eth_ixgbe_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
>  		struct rte_pci_device *pci_dev)
>  {
>  	char name[RTE_ETH_NAME_MAX_LEN];
> -
> -	struct rte_eth_devargs eth_da;
> +	struct rte_eth_devargs eth_da = { .nb_representor_ports = 0 };
>  	int i, retval;
> 
>  	if (pci_dev->device.devargs) {

Might be a bit better:
If (pci_dev->device.devargs) { rte_eth_devargs_parse(...);...}
else memset(&eth_da, 0, sizeof(eth_da));
to be more consistent.
BTW, I think rte_eth_devargs_parse() need to add formal check for input parameters.
Konstantin 



> --
> 2.14.3

Re: [dpdk-dev] [PATCH 3/3] net/ixgbe: add null pointer check for pf_ethdev

[Ananyev, Konstantin]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Declan Doherty
> Sent: Monday, April 30, 2018 4:33 PM
> To: dev@dpdk.org
> Cc: Doherty, Declan <declan.doherty@intel.com>
> Subject: [dpdk-dev] [PATCH 3/3] net/ixgbe: add null pointer check for pf_ethdev
> 
> Add NULL parameter check for rte_eth_dev_allocated() API call to
> eth_ixgbe_pci_probe().
> 
> Coverity Issue: 277216
> Fixes: cf80ba6e2038 ("net/ixgbe: add support for representor ports")
> 
> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
> ---
>  drivers/net/ixgbe/ixgbe_ethdev.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 283dd7e49..75f927c06 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -1747,6 +1747,9 @@ eth_ixgbe_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
>  	struct rte_eth_dev *pf_ethdev = rte_eth_dev_allocated(
>  		pci_dev->device.name);
> 
> +	if (pf_ethdev == NULL)
> +		return -ENODEV;
> +
>  	for (i = 0; i < eth_da.nb_representor_ports; i++) {
>  		struct ixgbe_vf_info *vfinfo;
>  		struct ixgbe_vf_representor representor;
> --

Looks good - can I just ask to move pf_ethdev definition to the top of the function?
To comply with dpdk coding style.
Konstantin

> 2.14.3

Re: [dpdk-dev] [PATCH 2/3] net/ixgbe: initialise nb_representor_ports value

[Doherty, Declan]

On 01/05/2018 10:46 AM, Ananyev, Konstantin wrote:
> 
> 
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Declan Doherty
>> Sent: Monday, April 30, 2018 4:33 PM
>> To: dev@dpdk.org
>> Cc: Doherty, Declan <declan.doherty@intel.com>
>> Subject: [dpdk-dev] [PATCH 2/3] net/ixgbe: initialise nb_representor_ports value
>>
>> Initialise rte_ethdev_args nb_representor_ports to zero to handle
>> the case where no devargs are passed to the IXGBE PF on
>> device probe, so that there is no invalid attempts to create
>> representor ports.
>>
>> Coverity Issue: 277231
>> Fixes: cf80ba6e2038 ("net/ixgbe: add support for representor ports")
>>
>> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
>> ---
>>   drivers/net/ixgbe/ixgbe_ethdev.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
>> index 0ccf55dc8..283dd7e49 100644
>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>> @@ -1725,8 +1725,7 @@ eth_ixgbe_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
>>   		struct rte_pci_device *pci_dev)
>>   {
>>   	char name[RTE_ETH_NAME_MAX_LEN];
>> -
>> -	struct rte_eth_devargs eth_da;
>> +	struct rte_eth_devargs eth_da = { .nb_representor_ports = 0 };
>>   	int i, retval;
>>
>>   	if (pci_dev->device.devargs) {
> 
> Might be a bit better:
> If (pci_dev->device.devargs) { rte_eth_devargs_parse(...);...}
> else memset(&eth_da, 0, sizeof(eth_da));
> to be more consistent.
> BTW, I think rte_eth_devargs_parse() need to add formal check for input parameters.
> Konstantin
> 

Ok, thanks Konstantin, that makes sense, we're working on cleaning up 
rte_eth_devargs_parse() to align with kvargs so I'll make sure to add 
those checks.

> 
> 
>> --
>> 2.14.3
> 

Re: [dpdk-dev] [PATCH 3/3] net/ixgbe: add null pointer check for pf_ethdev

[Doherty, Declan]

On 01/05/2018 10:47 AM, Ananyev, Konstantin wrote:
> 
> 
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Declan Doherty
>> Sent: Monday, April 30, 2018 4:33 PM
>> To: dev@dpdk.org
>> Cc: Doherty, Declan <declan.doherty@intel.com>
>> Subject: [dpdk-dev] [PATCH 3/3] net/ixgbe: add null pointer check for pf_ethdev
>>
>> Add NULL parameter check for rte_eth_dev_allocated() API call to
>> eth_ixgbe_pci_probe().
>>
>> Coverity Issue: 277216
>> Fixes: cf80ba6e2038 ("net/ixgbe: add support for representor ports")
>>
>> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
>> ---
>>   drivers/net/ixgbe/ixgbe_ethdev.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
>> index 283dd7e49..75f927c06 100644
>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>> @@ -1747,6 +1747,9 @@ eth_ixgbe_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
>>   	struct rte_eth_dev *pf_ethdev = rte_eth_dev_allocated(
>>   		pci_dev->device.name);
>>
>> +	if (pf_ethdev == NULL)
>> +		return -ENODEV;
>> +
>>   	for (i = 0; i < eth_da.nb_representor_ports; i++) {
>>   		struct ixgbe_vf_info *vfinfo;
>>   		struct ixgbe_vf_representor representor;
>> --
> 
> Looks good - can I just ask to move pf_ethdev definition to the top of the function?
> To comply with dpdk coding style.
> Konstantin
> 
Will do.

>> 2.14.3
> 

[dpdk-dev] [dpdk-dev 1/3][PATCH v2] net/ixgbe: revert default PF PMD device name

[Declan Doherty]

Changes introduced by cf80ba6e2038 modified the default name generated
for the IXGBE PF PMD, this patch reverts the default name to the
original PCI BDBF.

Fixes: cf80ba6e2038 ("net/ixgbe: add support for representor ports")
Signed-off-by: Declan Doherty <declan.doherty@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 6088c7e48..e49319a14 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1725,7 +1725,7 @@ eth_ixgbe_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 		struct rte_pci_device *pci_dev)
 {
 	char name[RTE_ETH_NAME_MAX_LEN];
-
+	struct rte_eth_dev *pf_ethdev;
 	struct rte_eth_devargs eth_da;
 	int i, retval;
 
@@ -1736,10 +1736,7 @@ eth_ixgbe_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 			return retval;
 	}
 
-	/* physical port net_bdf_port */
-	snprintf(name, sizeof(name), "net_%s_%d", pci_dev->device.name, 0);
-
-	retval = rte_eth_dev_create(&pci_dev->device, name,
+	retval = rte_eth_dev_create(&pci_dev->device, pci_dev->device.name,
 		sizeof(struct ixgbe_adapter),
 		eth_dev_pci_specific_init, pci_dev,
 		eth_ixgbe_dev_init, NULL);
@@ -1748,7 +1745,7 @@ eth_ixgbe_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 		return retval;
 
 	/* probe VF representor ports */
-	struct rte_eth_dev *pf_ethdev = rte_eth_dev_allocated(name);
+	pf_ethdev = rte_eth_dev_allocated(pci_dev->device.name);
 
 	for (i = 0; i < eth_da.nb_representor_ports; i++) {
 		struct ixgbe_vf_info *vfinfo;
-- 
2.14.3

[dpdk-dev] [dpdk-dev 2/3][PATCH v2] net/ixgbe: default eth_da parameter

[Declan Doherty]

Initialise rte_ethdev_args parameters to zero to handle
the case where no devargs are passed to the IXGBE PF on
device probe, so that there is no invalid attempts to create
representor ports.

Coverity Issue: 277231
Fixes: cf80ba6e2038 ("net/ixgbe: add support for representor ports")

Signed-off-by: Declan Doherty <declan.doherty@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index e49319a14..a05527a91 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1734,7 +1734,8 @@ eth_ixgbe_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 				&eth_da);
 		if (retval)
 			return retval;
-	}
+	} else
+		memset(&eth_da, 0, sizeof(eth_da));
 
 	retval = rte_eth_dev_create(&pci_dev->device, pci_dev->device.name,
 		sizeof(struct ixgbe_adapter),
-- 
2.14.3

[dpdk-dev] [dpdk-dev 3/3][PATCH v2] net/ixgbe: add null pointer check for pf_ethdev

[Declan Doherty]

Add NULL parameter check for rte_eth_dev_allocated() API call to
eth_ixgbe_pci_probe().

Coverity Issue: 277216
Fixes: cf80ba6e2038 ("net/ixgbe: add support for representor ports")

Signed-off-by: Declan Doherty <declan.doherty@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index a05527a91..91179e9b2 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1745,9 +1745,11 @@ eth_ixgbe_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 	if (retval || eth_da.nb_representor_ports < 1)
 		return retval;
 
-	/* probe VF representor ports */
 	pf_ethdev = rte_eth_dev_allocated(pci_dev->device.name);
+	if (pf_ethdev == NULL)
+		return -ENODEV;
 
+	/* probe VF representor ports */
 	for (i = 0; i < eth_da.nb_representor_ports; i++) {
 		struct ixgbe_vf_info *vfinfo;
 		struct ixgbe_vf_representor representor;
-- 
2.14.3

Re: [dpdk-dev] [dpdk-dev 1/3][PATCH v2] net/ixgbe: revert default PF PMD device name

[Ferruh Yigit]

On 5/2/2018 4:59 PM, Declan Doherty wrote:
> Changes introduced by cf80ba6e2038 modified the default name generated
> for the IXGBE PF PMD, this patch reverts the default name to the
> original PCI BDBF.
> 
> Fixes: cf80ba6e2038 ("net/ixgbe: add support for representor ports")
> Signed-off-by: Declan Doherty <declan.doherty@intel.com>

For series,
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Re: [dpdk-dev] [dpdk-dev 1/3][PATCH v2] net/ixgbe: revert default PF PMD device name

[Ferruh Yigit]

On 5/2/2018 5:48 PM, Ferruh Yigit wrote:
> On 5/2/2018 4:59 PM, Declan Doherty wrote:
>> Changes introduced by cf80ba6e2038 modified the default name generated
>> for the IXGBE PF PMD, this patch reverts the default name to the
>> original PCI BDBF.
>>
>> Fixes: cf80ba6e2038 ("net/ixgbe: add support for representor ports")
>> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
> 
> For series,
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Series applied to dpdk-next-net/master, thanks.