Skip to contentSkip to conversation

net/ixgbe: revert default PF PMD device name

PATCH Superseded Declan Doherty 3 participants 12 messages Apr 30, 2018 – May 2, 2018 Network drivers

lore
Share & tools
lore
#0 lore raw

PATCH 1/3 net/ixgbe: revert default PF PMD device name

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")
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)
drivers/net/ixgbe/ixgbe_ethdev.c +3 -5
index 6088c7e48..0ccf55dc8 100644
@@ -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;
Signature · 1 line
2.14.3
#1 lore raw

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")
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/net/ixgbe/ixgbe_ethdev.c +1 -2
index 0ccf55dc8..283dd7e49 100644
@@ -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) {
Signature · 1 line
2.14.3
#2 lore raw

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

Quoted 27 lines

-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Declan Doherty
Sent: Monday, April 30, 2018 4:33 PM
To: [email protected]
Cc: Doherty, Declan <[email protected]>
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")
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/net/ixgbe/ixgbe_ethdev.c +0 -0
index 0ccf55dc8..283dd7e49 100644

drivers/net/ixgbe/ixgbe_ethdev.c eth_ixgbe_pci_probe(struct rte_pci_driver *pci_drv __rte_unused, · line 1731

Quoted 9 lines
drivers/net/ixgbe/ixgbe_ethdev.c +1 -2
@@ -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

Quoted 2 lines

--
2.14.3

#3 lore raw

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

Quoted 35 lines · Konstantin
Quoted 35 lines

-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Declan Doherty
Sent: Monday, April 30, 2018 4:33 PM
To: [email protected]
Cc: Doherty, Declan <[email protected]>
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")
---
  drivers/net/ixgbe/ixgbe_ethdev.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)
drivers/net/ixgbe/ixgbe_ethdev.c +1 -2
index 0ccf55dc8..283dd7e49 100644
@@ -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;
 
Quoted 9 lines

drivers/net/ixgbe/ixgbe_ethdev.c eth_ixgbe_pci_probe(struct rte_pci_driver *pci_drv __rte_unused, · line 1731

Quoted 1 line
drivers/net/ixgbe/ixgbe_ethdev.c +0 -0
  	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.

Quoted 5 lines
Quoted 4 lines

--
2.14.3

#4 lore raw

PATCH 3/3 net/ixgbe: add null pointer check for pf_ethdev

In reply to 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")
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/net/ixgbe/ixgbe_ethdev.c +3 -0
index 283dd7e49..75f927c06 100644
@@ -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;
Signature · 1 line
2.14.3
#5 lore raw

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

Quoted 25 lines

-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Declan Doherty
Sent: Monday, April 30, 2018 4:33 PM
To: [email protected]
Cc: Doherty, Declan <[email protected]>
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")
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/net/ixgbe/ixgbe_ethdev.c +0 -0
index 283dd7e49..75f927c06 100644

drivers/net/ixgbe/ixgbe_ethdev.c eth_ixgbe_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,

Quoted 10 lines
drivers/net/ixgbe/ixgbe_ethdev.c +3 -1
@@ -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

Quoted 1 line

2.14.3

#6 lore raw

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

Quoted 28 lines · Konstantin
Quoted 28 lines

-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Declan Doherty
Sent: Monday, April 30, 2018 4:33 PM
To: [email protected]
Cc: Doherty, Declan <[email protected]>
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")
---
  drivers/net/ixgbe/ixgbe_ethdev.c | 3 +++
  1 file changed, 3 insertions(+)
drivers/net/ixgbe/ixgbe_ethdev.c +0 -0
index 283dd7e49..75f927c06 100644
@@ -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);
 
Quoted 12 lines

drivers/net/ixgbe/ixgbe_ethdev.c eth_ixgbe_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,

Quoted 7 lines
drivers/net/ixgbe/ixgbe_ethdev.c +3 -1
+	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.

Quoted 2 lines
Quoted 1 line

2.14.3

#7 lore raw

PATCH v2 1/3 net/ixgbe: revert default PF PMD device name

In reply to 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")
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)
drivers/net/ixgbe/ixgbe_ethdev.c +3 -6
index 6088c7e48..e49319a14 100644
@@ -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;
Signature · 1 line
2.14.3
#8 lore raw

PATCH v2 2/3 net/ixgbe: default eth_da parameter

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")
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/net/ixgbe/ixgbe_ethdev.c +2 -1
index e49319a14..a05527a91 100644
@@ -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),
Signature · 1 line
2.14.3
#9 lore raw

PATCH v2 3/3 net/ixgbe: add null pointer check for pf_ethdev

In reply to 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")
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/net/ixgbe/ixgbe_ethdev.c +3 -1
index a05527a91..91179e9b2 100644
@@ -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;
Signature · 1 line
2.14.3
#11 lore raw
Quoted 10 lines · Ferruh Yigit
Quoted 6 lines · 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")

For series,

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