[PATCH 02/10] net/nfp: fix malloc name problem in secondary process

[PATCH 02/10] net/nfp: fix malloc name problem in secondary process

[Chaoyong He]

The original logic keeps using the same name parameter when malloc
memory in secondary process, which may cause error when using
multiple PF cards.

Fixes: 3b00109d2b65 ("net/nfp: add PF ID used to format symbols")
Cc: peng.zhang@corigine.com
Cc: stable@dpdk.org

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
---
 drivers/net/nfp/nfp_ethdev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index 64a06440f5..3732abc9fe 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -2669,7 +2669,8 @@ nfp_pf_secondary_init(struct rte_pci_device *pci_dev)
 	}
 
 	/* Allocate memory for the PF "device" */
-	snprintf(name, sizeof(name), "nfp_pf%d", 0);
+	function_id = pci_dev->addr.function & 0x7;
+	snprintf(name, sizeof(name), "nfp_pf%d", function_id);
 	pf_dev = rte_zmalloc(name, sizeof(*pf_dev), 0);
 	if (pf_dev == NULL) {
 		PMD_INIT_LOG(ERR, "Can't allocate memory for the PF device");
@@ -2714,7 +2715,6 @@ nfp_pf_secondary_init(struct rte_pci_device *pci_dev)
 	}
 
 	/* Read the app ID of the firmware loaded */
-	function_id = pci_dev->addr.function & 0x7;
 	snprintf(app_name, sizeof(app_name), "_pf%u_net_app_id", function_id);
 	app_fw_id = nfp_rtsym_read_le(sym_tbl, app_name, &ret);
 	if (ret != 0) {
-- 
2.39.1

[PATCH 05/10] net/nfp: fix problem caused by configure function

[Chaoyong He]

The return value of 'nfp_eth_set_configured()' is three ways, the
original logic considered it as two ways wrongly.

Fixes: 61d4008fe6bb ("net/nfp: support setting link up/down")
Cc: stable@dpdk.org

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
---
 drivers/net/nfp/nfp_ethdev.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index 405386e882..2fe6b1a292 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -527,26 +527,36 @@ nfp_net_start(struct rte_eth_dev *dev)
 static int
 nfp_net_set_link_up(struct rte_eth_dev *dev)
 {
+	int ret;
 	struct nfp_net_hw *hw;
 	struct nfp_net_hw_priv *hw_priv;
 
 	hw = dev->data->dev_private;
 	hw_priv = dev->process_private;
 
-	return nfp_eth_set_configured(hw_priv->pf_dev->cpp, hw->nfp_idx, 1);
+	ret = nfp_eth_set_configured(hw_priv->pf_dev->cpp, hw->nfp_idx, 1);
+	if (ret < 0)
+		return ret;
+
+	return 0;
 }
 
 /* Set the link down. */
 static int
 nfp_net_set_link_down(struct rte_eth_dev *dev)
 {
+	int ret;
 	struct nfp_net_hw *hw;
 	struct nfp_net_hw_priv *hw_priv;
 
 	hw = dev->data->dev_private;
 	hw_priv = dev->process_private;
 
-	return nfp_eth_set_configured(hw_priv->pf_dev->cpp, hw->nfp_idx, 0);
+	ret = nfp_eth_set_configured(hw_priv->pf_dev->cpp, hw->nfp_idx, 0);
+	if (ret < 0)
+		return ret;
+
+	return 0;
 }
 
 static void
-- 
2.39.1

[PATCH 07/10] net/nfp: fix problem caused by commit end function

[Chaoyong He]

The return value of 'nfp_eth_config_commit_end()' is three ways, the
original logic considered it as two ways wrongly.

Fixes: 68aa35373a94 ("net/nfp: support setting pause frame switch mode")
Cc: zerun.fu@corigine.com
Cc: stable@dpdk.org

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
---
 drivers/net/nfp/nfp_net_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/nfp/nfp_net_common.c b/drivers/net/nfp/nfp_net_common.c
index 80d60515d8..5c3a9a7ae7 100644
--- a/drivers/net/nfp/nfp_net_common.c
+++ b/drivers/net/nfp/nfp_net_common.c
@@ -2520,7 +2520,7 @@ nfp_net_pause_frame_set(struct nfp_net_hw_priv *hw_priv,
 	}
 
 	err = nfp_eth_config_commit_end(nsp);
-	if (err != 0) {
+	if (err < 0) {
 		PMD_DRV_LOG(ERR, "Failed to configure pause frame.");
 		return err;
 	}
-- 
2.39.1

[PATCH 08/10] net/nfp: fix problem caused by FEC set

[Chaoyong He]

The return value of 'nfp_eth_set_fec()' is three ways, the original
logic considered it as two ways wrongly.

Fixes: 37bd1b843a20 ("net/nfp: support setting FEC mode")
Cc: zerun.fu@corigine.com
Cc: stable@dpdk.org

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
---
 drivers/net/nfp/nfp_net_common.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/nfp/nfp_net_common.c b/drivers/net/nfp/nfp_net_common.c
index 5c3a9a7ae7..b986ed4622 100644
--- a/drivers/net/nfp/nfp_net_common.c
+++ b/drivers/net/nfp/nfp_net_common.c
@@ -2692,6 +2692,7 @@ int
 nfp_net_fec_set(struct rte_eth_dev *dev,
 		uint32_t fec_capa)
 {
+	int ret;
 	uint8_t idx;
 	enum nfp_eth_fec fec;
 	uint32_t supported_fec;
@@ -2724,7 +2725,13 @@ nfp_net_fec_set(struct rte_eth_dev *dev,
 		return -EIO;
 	}
 
-	return nfp_eth_set_fec(hw_priv->pf_dev->cpp, eth_port->index, fec);
+	ret = nfp_eth_set_fec(hw_priv->pf_dev->cpp, eth_port->index, fec);
+	if (ret < 0) {
+		PMD_DRV_LOG(ERR, "NFP set FEC mode failed.");
+		return ret;
+	}
+
+	return 0;
 }
 
 uint32_t
-- 
2.39.1

[PATCH 10/10] net/nfp: fix memory leak in VF initialization logic

[Chaoyong He]

Fix one memory leak problem in the logic of VF initialization.

Fixes: d81e2b514dc9 ("net/nfp: move device info into process private data")
Cc: stable@dpdk.org

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
---
 drivers/net/nfp/nfp_ethdev_vf.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/nfp/nfp_ethdev_vf.c b/drivers/net/nfp/nfp_ethdev_vf.c
index ab413a2c5a..0a25730bf1 100644
--- a/drivers/net/nfp/nfp_ethdev_vf.c
+++ b/drivers/net/nfp/nfp_ethdev_vf.c
@@ -327,7 +327,8 @@ nfp_netvf_init(struct rte_eth_dev *eth_dev)
 	if (net_hw->eth_xstats_base == NULL) {
 		PMD_INIT_LOG(ERR, "No memory for xstats base values on device %s!",
 				pci_dev->device.name);
-		return -ENOMEM;
+		err = -ENOMEM;
+		goto hw_priv_free;
 	}
 
 	/* Work out where in the BAR the queues start. */
-- 
2.39.1

Re: [PATCH 02/10] net/nfp: fix malloc name problem in secondary process

[Stephen Hemminger]

On Thu, 10 Oct 2024 17:17:08 +0800
Chaoyong He <chaoyong.he@corigine.com> wrote:

> The original logic keeps using the same name parameter when malloc
> memory in secondary process, which may cause error when using
> multiple PF cards.
> 
> Fixes: 3b00109d2b65 ("net/nfp: add PF ID used to format symbols")
> Cc: peng.zhang@corigine.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
> Reviewed-by: Long Wu <long.wu@corigine.com>
> Reviewed-by: Peng Zhang <peng.zhang@corigine.com>

Huh? the name is ignored by rte_malloc(), it only shows up in tracing.
 in fact you could just always pass NULL.

Re: [PATCH 05/10] net/nfp: fix problem caused by configure function

[Stephen Hemminger]

On Thu, 10 Oct 2024 17:17:11 +0800
Chaoyong He <chaoyong.he@corigine.com> wrote:

> The return value of 'nfp_eth_set_configured()' is three ways, the
> original logic considered it as two ways wrongly.
> 
> Fixes: 61d4008fe6bb ("net/nfp: support setting link up/down")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
> Reviewed-by: Long Wu <long.wu@corigine.com>
> Reviewed-by: Peng Zhang <peng.zhang@corigine.com>

Maybe refering to the return values in nfp_nsp_eth.c would be clearer
but makes sense.

Acked-by: Stephen Hemminger <stephen@networkplumber.org>

Re: [PATCH 07/10] net/nfp: fix problem caused by commit end function

[Stephen Hemminger]

On Thu, 10 Oct 2024 17:17:13 +0800
Chaoyong He <chaoyong.he@corigine.com> wrote:

> The return value of 'nfp_eth_config_commit_end()' is three ways, the
> original logic considered it as two ways wrongly.
> 
> Fixes: 68aa35373a94 ("net/nfp: support setting pause frame switch mode")
> Cc: zerun.fu@corigine.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
> Reviewed-by: Long Wu <long.wu@corigine.com>
> Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
> ---

Acked-by: Stephen Hemminger <stephen@networkplumber.org>

Re: [PATCH 08/10] net/nfp: fix problem caused by FEC set

[Stephen Hemminger]

On Thu, 10 Oct 2024 17:17:14 +0800
Chaoyong He <chaoyong.he@corigine.com> wrote:

> The return value of 'nfp_eth_set_fec()' is three ways, the original
> logic considered it as two ways wrongly.
> 
> Fixes: 37bd1b843a20 ("net/nfp: support setting FEC mode")
> Cc: zerun.fu@corigine.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
> Reviewed-by: Long Wu <long.wu@corigine.com>
> Reviewed-by: Peng Zhang <peng.zhang@corigine.com>

Acked-by: Stephen Hemminger <stephen@networkplumber.org>

Re: [PATCH 10/10] net/nfp: fix memory leak in VF initialization logic

[Stephen Hemminger]

On Thu, 10 Oct 2024 17:17:16 +0800
Chaoyong He <chaoyong.he@corigine.com> wrote:

> Fix one memory leak problem in the logic of VF initialization.
> 
> Fixes: d81e2b514dc9 ("net/nfp: move device info into process private data")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
> Reviewed-by: Long Wu <long.wu@corigine.com>
> Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
> ---

The code as is looks fine, but would have been better to use rte_calloc()
when allocating eth_xstats_base.

Acked-by: Stephen Hemminger <stephen@networkplumber.org>