* [PATCH 00/10] modify some logic of NFP PMD
@ 2024-10-10 9:17 Chaoyong He
2024-10-10 9:17 ` [PATCH 01/10] net/nfp: use strlcpy for copying string Chaoyong He
` (10 more replies)
0 siblings, 11 replies; 38+ messages in thread
From: Chaoyong He @ 2024-10-10 9:17 UTC (permalink / raw)
To: dev; +Cc: oss-drivers, Chaoyong He
This patch series refactor some logic of NFP PMD to make it more
clear, also fix some bugs.
Chaoyong He (10):
net/nfp: use strlcpy for copying string
net/nfp: fix malloc name problem in secondary process
net/nfp: simplify some function parameters
net/nfp: improve the logic readability
net/nfp: fix problem caused by configure function
net/nfp: add check logic for port up/down function
net/nfp: fix problem caused by commit end function
net/nfp: fix problem caused by FEC set
net/nfp: modify the comment of some control messages
net/nfp: fix memory leak in VF initialization logic
drivers/net/nfp/flower/nfp_flower_cmsg.h | 14 +-
.../net/nfp/flower/nfp_flower_representor.c | 22 +-
drivers/net/nfp/nfp_ethdev.c | 259 +++++++++---------
drivers/net/nfp/nfp_ethdev_vf.c | 3 +-
drivers/net/nfp/nfp_net_common.c | 49 +++-
drivers/net/nfp/nfp_net_common.h | 4 +-
6 files changed, 194 insertions(+), 157 deletions(-)
--
2.39.1
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 01/10] net/nfp: use strlcpy for copying string
2024-10-10 9:17 [PATCH 00/10] modify some logic of NFP PMD Chaoyong He
@ 2024-10-10 9:17 ` Chaoyong He
2024-10-10 15:08 ` Stephen Hemminger
2024-10-10 9:17 ` [PATCH 02/10] net/nfp: fix malloc name problem in secondary process Chaoyong He
` (9 subsequent siblings)
10 siblings, 1 reply; 38+ messages in thread
From: Chaoyong He @ 2024-10-10 9:17 UTC (permalink / raw)
To: dev; +Cc: oss-drivers, Chaoyong He, Long Wu, Peng Zhang
Replace 'snprintf()' with 'strlcpy()' where applicable.
Using 'strlcpy()' is safe practice when copying strings, as it
will include a null terminator.
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/flower/nfp_flower_representor.c | 4 ++--
drivers/net/nfp/nfp_net_common.c | 8 ++++----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/nfp/flower/nfp_flower_representor.c b/drivers/net/nfp/flower/nfp_flower_representor.c
index 5db7d50618..d1558b905c 100644
--- a/drivers/net/nfp/flower/nfp_flower_representor.c
+++ b/drivers/net/nfp/flower/nfp_flower_representor.c
@@ -576,7 +576,7 @@ nfp_flower_pf_repr_init(struct rte_eth_dev *eth_dev,
repr->repr_type = init_repr_data->repr_type;
repr->app_fw_flower = init_repr_data->app_fw_flower;
- snprintf(repr->name, sizeof(repr->name), "%s", init_repr_data->name);
+ strlcpy(repr->name, init_repr_data->name, sizeof(repr->name));
eth_dev->dev_ops = &nfp_flower_pf_repr_dev_ops;
eth_dev->rx_pkt_burst = nfp_net_recv_pkts;
@@ -652,7 +652,7 @@ nfp_flower_repr_init(struct rte_eth_dev *eth_dev,
repr->repr_type = init_repr_data->repr_type;
repr->app_fw_flower = init_repr_data->app_fw_flower;
- snprintf(repr->name, sizeof(repr->name), "%s", init_repr_data->name);
+ strlcpy(repr->name, init_repr_data->name, sizeof(repr->name));
eth_dev->dev_ops = &nfp_flower_repr_dev_ops;
eth_dev->rx_pkt_burst = nfp_flower_repr_rx_burst;
diff --git a/drivers/net/nfp/nfp_net_common.c b/drivers/net/nfp/nfp_net_common.c
index 4a2c536704..c9a95ed632 100644
--- a/drivers/net/nfp/nfp_net_common.c
+++ b/drivers/net/nfp/nfp_net_common.c
@@ -2306,7 +2306,7 @@ nfp_net_get_mip_name(struct nfp_net_hw_priv *hw_priv,
if (mip == NULL)
return;
- snprintf(mip_name, FW_VER_LEN, "%s", nfp_mip_name(mip));
+ strlcpy(mip_name, nfp_mip_name(mip), FW_VER_LEN);
nfp_mip_close(mip);
}
@@ -2317,13 +2317,13 @@ nfp_net_get_app_name(struct nfp_net_hw_priv *hw_priv,
{
switch (hw_priv->pf_dev->app_fw_id) {
case NFP_APP_FW_CORE_NIC:
- snprintf(app_name, FW_VER_LEN, "%s", "nic");
+ strlcpy(app_name, "nic", FW_VER_LEN);
break;
case NFP_APP_FW_FLOWER_NIC:
- snprintf(app_name, FW_VER_LEN, "%s", "flower");
+ strlcpy(app_name, "flower", FW_VER_LEN);
break;
default:
- snprintf(app_name, FW_VER_LEN, "%s", "unknown");
+ strlcpy(app_name, "unknown", FW_VER_LEN);
break;
}
}
--
2.39.1
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 02/10] net/nfp: fix malloc name problem in secondary process
2024-10-10 9:17 [PATCH 00/10] modify some logic of NFP PMD Chaoyong He
2024-10-10 9:17 ` [PATCH 01/10] net/nfp: use strlcpy for copying string Chaoyong He
@ 2024-10-10 9:17 ` Chaoyong He
2024-10-10 15:07 ` Stephen Hemminger
2024-10-10 9:17 ` [PATCH 03/10] net/nfp: simplify some function parameters Chaoyong He
` (8 subsequent siblings)
10 siblings, 1 reply; 38+ messages in thread
From: Chaoyong He @ 2024-10-10 9:17 UTC (permalink / raw)
To: dev; +Cc: oss-drivers, Chaoyong He, peng.zhang, stable, Long Wu
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 03/10] net/nfp: simplify some function parameters
2024-10-10 9:17 [PATCH 00/10] modify some logic of NFP PMD Chaoyong He
2024-10-10 9:17 ` [PATCH 01/10] net/nfp: use strlcpy for copying string Chaoyong He
2024-10-10 9:17 ` [PATCH 02/10] net/nfp: fix malloc name problem in secondary process Chaoyong He
@ 2024-10-10 9:17 ` Chaoyong He
2024-10-10 15:08 ` Stephen Hemminger
2024-10-10 9:17 ` [PATCH 04/10] net/nfp: improve the logic readability Chaoyong He
` (7 subsequent siblings)
10 siblings, 1 reply; 38+ messages in thread
From: Chaoyong He @ 2024-10-10 9:17 UTC (permalink / raw)
To: dev; +Cc: oss-drivers, Chaoyong He, Long Wu, Peng Zhang
Refactor to the logic of initialize process to simplify some function
parameters, both for primary and secodary process.
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 | 153 +++++++++++++++----------------
drivers/net/nfp/nfp_net_common.c | 5 +-
drivers/net/nfp/nfp_net_common.h | 3 +-
3 files changed, 76 insertions(+), 85 deletions(-)
diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index 3732abc9fe..121f82af6d 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -1160,14 +1160,15 @@ nfp_net_init(struct rte_eth_dev *eth_dev,
}
static int
-nfp_net_device_activate(struct nfp_cpp *cpp,
- struct nfp_multi_pf *multi_pf)
+nfp_net_device_activate(struct nfp_pf_dev *pf_dev)
{
int ret;
struct nfp_nsp *nsp;
+ struct nfp_multi_pf *multi_pf;
+ multi_pf = &pf_dev->multi_pf;
if (multi_pf->enabled && multi_pf->function_id != 0) {
- nsp = nfp_nsp_open(cpp);
+ nsp = nfp_nsp_open(pf_dev->cpp);
if (nsp == NULL) {
PMD_DRV_LOG(ERR, "NFP error when obtaining NSP handle");
return -EIO;
@@ -1185,10 +1186,7 @@ nfp_net_device_activate(struct nfp_cpp *cpp,
#define DEFAULT_FW_PATH "/lib/firmware/netronome"
static int
-nfp_fw_get_name(struct rte_pci_device *dev,
- struct nfp_cpp *cpp,
- struct nfp_eth_table *nfp_eth_table,
- struct nfp_hwinfo *hwinfo,
+nfp_fw_get_name(struct nfp_pf_dev *pf_dev,
char *fw_name,
size_t fw_size)
{
@@ -1199,11 +1197,11 @@ nfp_fw_get_name(struct rte_pci_device *dev,
const char *nfp_fw_model;
const uint8_t *cpp_serial;
- cpp_serial_len = nfp_cpp_serial(cpp, &cpp_serial);
+ cpp_serial_len = nfp_cpp_serial(pf_dev->cpp, &cpp_serial);
if (cpp_serial_len != NFP_SERIAL_LEN)
return -ERANGE;
- interface = nfp_cpp_interface(cpp);
+ interface = nfp_cpp_interface(pf_dev->cpp);
/* Looking for firmware file in order of priority */
@@ -1220,15 +1218,15 @@ nfp_fw_get_name(struct rte_pci_device *dev,
/* Then try the PCI name */
snprintf(fw_name, fw_size, "%s/pci-%s.nffw", DEFAULT_FW_PATH,
- dev->name);
+ pf_dev->pci_dev->name);
PMD_DRV_LOG(DEBUG, "Trying with fw file: %s", fw_name);
if (access(fw_name, F_OK) == 0)
return 0;
- nfp_fw_model = nfp_hwinfo_lookup(hwinfo, "nffw.partno");
+ nfp_fw_model = nfp_hwinfo_lookup(pf_dev->hwinfo, "nffw.partno");
if (nfp_fw_model == NULL) {
- nfp_fw_model = nfp_hwinfo_lookup(hwinfo, "assembly.partno");
+ nfp_fw_model = nfp_hwinfo_lookup(pf_dev->hwinfo, "assembly.partno");
if (nfp_fw_model == NULL) {
PMD_DRV_LOG(ERR, "firmware model NOT found");
return -EIO;
@@ -1244,8 +1242,8 @@ nfp_fw_get_name(struct rte_pci_device *dev,
/* Finally try the card type and media */
snprintf(card_desc, sizeof(card_desc), "nic_%s_%dx%d.nffw",
- nfp_fw_model, nfp_eth_table->count,
- nfp_eth_table->ports[0].speed / 1000);
+ nfp_fw_model, pf_dev->nfp_eth_table->count,
+ pf_dev->nfp_eth_table->ports[0].speed / 1000);
snprintf(fw_name, fw_size, "%s/%s", DEFAULT_FW_PATH, card_desc);
PMD_DRV_LOG(DEBUG, "Trying with fw file: %s", fw_name);
if (access(fw_name, F_OK) == 0)
@@ -1433,15 +1431,15 @@ nfp_fw_reload_from_flash(struct nfp_nsp *nsp)
static int
nfp_fw_reload_for_single_pf_from_disk(struct nfp_nsp *nsp,
char *fw_name,
- struct nfp_cpp *cpp,
- bool force_reload_fw,
+ struct nfp_pf_dev *pf_dev,
int reset)
{
int ret;
bool fw_changed = true;
- if (nfp_nsp_has_fw_loaded(nsp) && nfp_nsp_fw_loaded(nsp) && !force_reload_fw) {
- ret = nfp_fw_check_change(cpp, fw_name, &fw_changed);
+ if (nfp_nsp_has_fw_loaded(nsp) && nfp_nsp_fw_loaded(nsp) &&
+ !pf_dev->devargs.force_reload_fw) {
+ ret = nfp_fw_check_change(pf_dev->cpp, fw_name, &fw_changed);
if (ret != 0)
return ret;
}
@@ -1459,8 +1457,7 @@ nfp_fw_reload_for_single_pf_from_disk(struct nfp_nsp *nsp,
static int
nfp_fw_reload_for_single_pf(struct nfp_nsp *nsp,
char *fw_name,
- struct nfp_cpp *cpp,
- bool force_reload_fw,
+ struct nfp_pf_dev *pf_dev,
int reset,
int policy)
{
@@ -1473,8 +1470,7 @@ nfp_fw_reload_for_single_pf(struct nfp_nsp *nsp,
return ret;
}
} else if (fw_name[0] != 0) {
- ret = nfp_fw_reload_for_single_pf_from_disk(nsp, fw_name, cpp,
- force_reload_fw, reset);
+ ret = nfp_fw_reload_for_single_pf_from_disk(nsp, fw_name, pf_dev, reset);
if (ret != 0) {
PMD_DRV_LOG(ERR, "Load single PF firmware from disk failed.");
return ret;
@@ -1490,25 +1486,23 @@ nfp_fw_reload_for_single_pf(struct nfp_nsp *nsp,
static int
nfp_fw_reload_for_multi_pf_from_disk(struct nfp_nsp *nsp,
char *fw_name,
- struct nfp_cpp *cpp,
const struct nfp_dev_info *dev_info,
- struct nfp_multi_pf *multi_pf,
- bool force_reload_fw,
+ struct nfp_pf_dev *pf_dev,
int reset)
{
int err;
bool fw_changed = true;
bool skip_load_fw = false;
- bool reload_fw = force_reload_fw;
+ bool reload_fw = pf_dev->devargs.force_reload_fw;
if (nfp_nsp_has_fw_loaded(nsp) && nfp_nsp_fw_loaded(nsp) && !reload_fw) {
- err = nfp_fw_check_change(cpp, fw_name, &fw_changed);
+ err = nfp_fw_check_change(pf_dev->cpp, fw_name, &fw_changed);
if (err != 0)
return err;
}
if (!fw_changed || reload_fw)
- skip_load_fw = nfp_fw_skip_load(dev_info, multi_pf, &reload_fw);
+ skip_load_fw = nfp_fw_skip_load(dev_info, &pf_dev->multi_pf, &reload_fw);
if (skip_load_fw && !reload_fw)
return 0;
@@ -1523,16 +1517,17 @@ nfp_fw_reload_for_multi_pf_from_disk(struct nfp_nsp *nsp,
static int
nfp_fw_reload_for_multi_pf(struct nfp_nsp *nsp,
char *fw_name,
- struct nfp_cpp *cpp,
const struct nfp_dev_info *dev_info,
- struct nfp_multi_pf *multi_pf,
- bool force_reload_fw,
+ struct nfp_pf_dev *pf_dev,
int reset,
int policy)
{
int err;
+ struct nfp_multi_pf *multi_pf;
- err = nfp_net_keepalive_init(cpp, multi_pf);
+ multi_pf = &pf_dev->multi_pf;
+
+ err = nfp_net_keepalive_init(pf_dev->cpp, multi_pf);
if (err != 0) {
PMD_DRV_LOG(ERR, "NFP init beat failed");
return err;
@@ -1551,8 +1546,8 @@ nfp_fw_reload_for_multi_pf(struct nfp_nsp *nsp,
goto keepalive_stop;
}
} else if (fw_name[0] != 0) {
- err = nfp_fw_reload_for_multi_pf_from_disk(nsp, fw_name, cpp,
- dev_info, multi_pf, force_reload_fw, reset);
+ err = nfp_fw_reload_for_multi_pf_from_disk(nsp, fw_name, dev_info,
+ pf_dev, reset);
if (err != 0) {
PMD_DRV_LOG(ERR, "Load multi PF firmware from disk failed.");
goto keepalive_stop;
@@ -1627,13 +1622,8 @@ nfp_fw_policy_value_get(struct nfp_nsp *nsp,
}
static int
-nfp_fw_setup(struct rte_pci_device *dev,
- struct nfp_cpp *cpp,
- struct nfp_eth_table *nfp_eth_table,
- struct nfp_hwinfo *hwinfo,
- const struct nfp_dev_info *dev_info,
- struct nfp_multi_pf *multi_pf,
- bool force_reload_fw)
+nfp_fw_setup(struct nfp_pf_dev *pf_dev,
+ const struct nfp_dev_info *dev_info)
{
int err;
int reset;
@@ -1641,7 +1631,7 @@ nfp_fw_setup(struct rte_pci_device *dev,
char fw_name[125];
struct nfp_nsp *nsp;
- nsp = nfp_nsp_open(cpp);
+ nsp = nfp_nsp_open(pf_dev->cpp);
if (nsp == NULL) {
PMD_DRV_LOG(ERR, "NFP error when obtaining NSP handle");
return -EIO;
@@ -1665,20 +1655,19 @@ nfp_fw_setup(struct rte_pci_device *dev,
fw_name[0] = 0;
if (policy != NFP_NSP_APP_FW_LOAD_FLASH) {
- err = nfp_fw_get_name(dev, cpp, nfp_eth_table, hwinfo, fw_name,
- sizeof(fw_name));
+ err = nfp_fw_get_name(pf_dev, fw_name, sizeof(fw_name));
if (err != 0) {
PMD_DRV_LOG(ERR, "Can't find suitable firmware.");
goto close_nsp;
}
}
- if (multi_pf->enabled)
- err = nfp_fw_reload_for_multi_pf(nsp, fw_name, cpp, dev_info,
- multi_pf, force_reload_fw, reset, policy);
+ if (pf_dev->multi_pf.enabled)
+ err = nfp_fw_reload_for_multi_pf(nsp, fw_name, dev_info,
+ pf_dev, reset, policy);
else
- err = nfp_fw_reload_for_single_pf(nsp, fw_name, cpp,
- force_reload_fw, reset, policy);
+ err = nfp_fw_reload_for_single_pf(nsp, fw_name, pf_dev,
+ reset, policy);
close_nsp:
nfp_nsp_close(nsp);
@@ -2063,7 +2052,7 @@ nfp_net_speed_cap_get(struct nfp_pf_dev *pf_dev)
uint32_t id;
uint32_t count;
- count = nfp_net_get_port_num(pf_dev, pf_dev->nfp_eth_table);
+ count = pf_dev->total_phyports;
for (i = 0; i < count; i++) {
id = nfp_function_id_get(pf_dev, i);
ret = nfp_net_speed_cap_get_one(pf_dev, id);
@@ -2078,9 +2067,7 @@ nfp_net_speed_cap_get(struct nfp_pf_dev *pf_dev)
/* Force the physical port down to clear the possible DMA error */
static int
-nfp_net_force_port_down(struct nfp_pf_dev *pf_dev,
- struct nfp_eth_table *nfp_eth_table,
- struct nfp_cpp *cpp)
+nfp_net_force_port_down(struct nfp_pf_dev *pf_dev)
{
int ret;
uint32_t i;
@@ -2088,11 +2075,11 @@ nfp_net_force_port_down(struct nfp_pf_dev *pf_dev,
uint32_t index;
uint32_t count;
- count = nfp_net_get_port_num(pf_dev, nfp_eth_table);
+ count = pf_dev->total_phyports;
for (i = 0; i < count; i++) {
id = nfp_function_id_get(pf_dev, i);
- index = nfp_eth_table->ports[id].index;
- ret = nfp_eth_set_configured(cpp, index, 0);
+ index = pf_dev->nfp_eth_table->ports[id].index;
+ ret = nfp_eth_set_configured(pf_dev->cpp, index, 0);
if (ret < 0)
return ret;
}
@@ -2322,6 +2309,9 @@ nfp_pf_init(struct rte_pci_device *pci_dev)
goto hw_priv_free;
}
+ hw_priv->dev_info = dev_info;
+ hw_priv->pf_dev = pf_dev;
+
sync = nfp_sync_alloc();
if (sync == NULL) {
PMD_INIT_LOG(ERR, "Failed to alloc sync zone.");
@@ -2329,6 +2319,8 @@ nfp_pf_init(struct rte_pci_device *pci_dev)
goto pf_cleanup;
}
+ pf_dev->sync = sync;
+
/*
* When device bound to UIO, the device could be used, by mistake,
* by two DPDK apps, and the UIO driver does not avoid it. This
@@ -2347,6 +2339,9 @@ nfp_pf_init(struct rte_pci_device *pci_dev)
goto sync_free;
}
+ pf_dev->cpp = cpp;
+ pf_dev->pci_dev = pci_dev;
+
hwinfo = nfp_hwinfo_read(cpp);
if (hwinfo == NULL) {
PMD_INIT_LOG(ERR, "Error reading hwinfo table");
@@ -2354,6 +2349,8 @@ nfp_pf_init(struct rte_pci_device *pci_dev)
goto cpp_cleanup;
}
+ pf_dev->hwinfo = hwinfo;
+
/* Read the number of physical ports from hardware */
nfp_eth_table = nfp_eth_read_ports(cpp);
if (nfp_eth_table == NULL) {
@@ -2369,10 +2366,12 @@ nfp_pf_init(struct rte_pci_device *pci_dev)
goto eth_table_cleanup;
}
+ pf_dev->nfp_eth_table = nfp_eth_table;
pf_dev->multi_pf.enabled = nfp_check_multi_pf_from_nsp(pci_dev, cpp);
pf_dev->multi_pf.function_id = function_id;
+ pf_dev->total_phyports = nfp_net_get_port_num(pf_dev);
- ret = nfp_net_force_port_down(pf_dev, nfp_eth_table, cpp);
+ ret = nfp_net_force_port_down(pf_dev);
if (ret != 0) {
PMD_INIT_LOG(ERR, "Failed to force port down");
ret = -EIO;
@@ -2386,15 +2385,15 @@ nfp_pf_init(struct rte_pci_device *pci_dev)
goto eth_table_cleanup;
}
- ret = nfp_net_device_activate(cpp, &pf_dev->multi_pf);
+ ret = nfp_net_device_activate(pf_dev);
if (ret != 0) {
PMD_INIT_LOG(ERR, "Failed to activate the NFP device");
ret = -EIO;
goto eth_table_cleanup;
}
- if (nfp_fw_setup(pci_dev, cpp, nfp_eth_table, hwinfo,
- dev_info, &pf_dev->multi_pf, pf_dev->devargs.force_reload_fw) != 0) {
+ ret = nfp_fw_setup(pf_dev, dev_info);
+ if (ret != 0) {
PMD_INIT_LOG(ERR, "Error when uploading firmware");
ret = -EIO;
goto eth_table_cleanup;
@@ -2408,6 +2407,8 @@ nfp_pf_init(struct rte_pci_device *pci_dev)
goto fw_cleanup;
}
+ pf_dev->sym_tbl = sym_tbl;
+
/* Read the app ID of the firmware loaded */
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);
@@ -2417,6 +2418,8 @@ nfp_pf_init(struct rte_pci_device *pci_dev)
goto sym_tbl_cleanup;
}
+ pf_dev->app_fw_id = app_fw_id;
+
/* Write sp_indiff to hw_info */
ret = nfp_net_hwinfo_set(function_id, sym_tbl, cpp, app_fw_id);
if (ret != 0) {
@@ -2425,17 +2428,6 @@ nfp_pf_init(struct rte_pci_device *pci_dev)
goto sym_tbl_cleanup;
}
- /* Populate the newly created PF device */
- pf_dev->app_fw_id = app_fw_id;
- pf_dev->cpp = cpp;
- pf_dev->hwinfo = hwinfo;
- pf_dev->sym_tbl = sym_tbl;
- pf_dev->pci_dev = pci_dev;
- pf_dev->nfp_eth_table = nfp_eth_table;
- pf_dev->sync = sync;
- pf_dev->total_phyports = nfp_net_get_port_num(pf_dev, nfp_eth_table);
- pf_dev->speed_updated = false;
-
ret = nfp_net_speed_cap_get(pf_dev);
if (ret != 0) {
PMD_INIT_LOG(ERR, "Failed to get speed capability.");
@@ -2484,8 +2476,6 @@ nfp_pf_init(struct rte_pci_device *pci_dev)
}
hw_priv->is_pf = true;
- hw_priv->pf_dev = pf_dev;
- hw_priv->dev_info = dev_info;
/*
* PF initialization has been done at this point. Call app specific
@@ -2678,6 +2668,9 @@ nfp_pf_secondary_init(struct rte_pci_device *pci_dev)
goto hw_priv_free;
}
+ hw_priv->pf_dev = pf_dev;
+ hw_priv->dev_info = dev_info;
+
sync = nfp_sync_alloc();
if (sync == NULL) {
PMD_INIT_LOG(ERR, "Failed to alloc sync zone.");
@@ -2685,6 +2678,8 @@ nfp_pf_secondary_init(struct rte_pci_device *pci_dev)
goto pf_cleanup;
}
+ pf_dev->sync = sync;
+
/*
* When device bound to UIO, the device could be used, by mistake,
* by two DPDK apps, and the UIO driver does not avoid it. This
@@ -2703,6 +2698,9 @@ nfp_pf_secondary_init(struct rte_pci_device *pci_dev)
goto sync_free;
}
+ pf_dev->cpp = cpp;
+ pf_dev->pci_dev = pci_dev;
+
/*
* We don't have access to the PF created in the primary process
* here so we have to read the number of ports from firmware.
@@ -2714,6 +2712,8 @@ nfp_pf_secondary_init(struct rte_pci_device *pci_dev)
goto cpp_cleanup;
}
+ pf_dev->sym_tbl = sym_tbl;
+
/* Read the app ID of the firmware loaded */
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);
@@ -2723,16 +2723,9 @@ nfp_pf_secondary_init(struct rte_pci_device *pci_dev)
goto sym_tbl_cleanup;
}
- /* Populate the newly created PF device */
pf_dev->app_fw_id = app_fw_id;
- pf_dev->cpp = cpp;
- pf_dev->sym_tbl = sym_tbl;
- pf_dev->pci_dev = pci_dev;
- pf_dev->sync = sync;
hw_priv->is_pf = true;
- hw_priv->pf_dev = pf_dev;
- hw_priv->dev_info = dev_info;
/* Call app specific init code now */
ret = nfp_fw_app_secondary_init(hw_priv);
diff --git a/drivers/net/nfp/nfp_net_common.c b/drivers/net/nfp/nfp_net_common.c
index c9a95ed632..99f6b61947 100644
--- a/drivers/net/nfp/nfp_net_common.c
+++ b/drivers/net/nfp/nfp_net_common.c
@@ -2724,13 +2724,12 @@ nfp_net_fec_set(struct rte_eth_dev *dev,
}
uint32_t
-nfp_net_get_port_num(struct nfp_pf_dev *pf_dev,
- struct nfp_eth_table *nfp_eth_table)
+nfp_net_get_port_num(struct nfp_pf_dev *pf_dev)
{
if (pf_dev->multi_pf.enabled)
return 1;
else
- return nfp_eth_table->count;
+ return pf_dev->nfp_eth_table->count;
}
uint8_t
diff --git a/drivers/net/nfp/nfp_net_common.h b/drivers/net/nfp/nfp_net_common.h
index 0b5bba2a3e..fb244383b7 100644
--- a/drivers/net/nfp/nfp_net_common.h
+++ b/drivers/net/nfp/nfp_net_common.h
@@ -374,8 +374,7 @@ void nfp_net_get_fw_version(struct nfp_cpp *cpp,
uint32_t *fw_version);
int nfp_net_txrwb_alloc(struct rte_eth_dev *eth_dev);
void nfp_net_txrwb_free(struct rte_eth_dev *eth_dev);
-uint32_t nfp_net_get_port_num(struct nfp_pf_dev *pf_dev,
- struct nfp_eth_table *nfp_eth_table);
+uint32_t nfp_net_get_port_num(struct nfp_pf_dev *pf_dev);
uint8_t nfp_function_id_get(const struct nfp_pf_dev *pf_dev,
uint8_t port_id);
int nfp_net_vf_config_app_init(struct nfp_net_hw *net_hw,
--
2.39.1
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 04/10] net/nfp: improve the logic readability
2024-10-10 9:17 [PATCH 00/10] modify some logic of NFP PMD Chaoyong He
` (2 preceding siblings ...)
2024-10-10 9:17 ` [PATCH 03/10] net/nfp: simplify some function parameters Chaoyong He
@ 2024-10-10 9:17 ` Chaoyong He
2024-10-10 15:10 ` Stephen Hemminger
2024-10-10 9:17 ` [PATCH 05/10] net/nfp: fix problem caused by configure function Chaoyong He
` (6 subsequent siblings)
10 siblings, 1 reply; 38+ messages in thread
From: Chaoyong He @ 2024-10-10 9:17 UTC (permalink / raw)
To: dev; +Cc: oss-drivers, Chaoyong He, Long Wu, Peng Zhang
Try our best to make the logic in secondary proess the same with the
primary process and improve the readability, by add helper functions
and rename function.
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 | 86 +++++++++++++++-----------------
drivers/net/nfp/nfp_net_common.c | 22 +++++++-
drivers/net/nfp/nfp_net_common.h | 3 +-
3 files changed, 63 insertions(+), 48 deletions(-)
diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index 121f82af6d..405386e882 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -1764,19 +1764,9 @@ nfp_enable_multi_pf(struct nfp_pf_dev *pf_dev)
static bool
nfp_app_fw_nic_total_phyports_check(struct nfp_pf_dev *pf_dev)
{
- int ret;
- uint8_t id;
uint8_t total_phyports;
- char vnic_name[RTE_ETH_NAME_MAX_LEN];
- /* Read the number of vNIC's created for the PF */
- id = nfp_function_id_get(pf_dev, 0);
- snprintf(vnic_name, sizeof(vnic_name), "nfd_cfg_pf%u_num_ports", id);
- total_phyports = nfp_rtsym_read_le(pf_dev->sym_tbl, vnic_name, &ret);
- if (ret != 0 || total_phyports == 0 || total_phyports > 8) {
- PMD_INIT_LOG(ERR, "%s symbol with wrong value", vnic_name);
- return false;
- }
+ total_phyports = nfp_net_get_phyports_from_fw(pf_dev);
if (pf_dev->multi_pf.enabled) {
if (!nfp_check_multi_pf_from_fw(total_phyports)) {
@@ -1797,6 +1787,20 @@ nfp_app_fw_nic_total_phyports_check(struct nfp_pf_dev *pf_dev)
return true;
}
+static void
+nfp_port_name_generate(char *port_name,
+ size_t length,
+ int port_id,
+ struct nfp_pf_dev *pf_dev)
+{
+ const char *name = pf_dev->pci_dev->device.name;
+
+ if (pf_dev->multi_pf.enabled)
+ snprintf(port_name, length, "%s", name);
+ else
+ snprintf(port_name, length, "%s_port%u", name, port_id);
+}
+
static int
nfp_init_app_fw_nic(struct nfp_net_hw_priv *hw_priv)
{
@@ -1849,12 +1853,7 @@ nfp_init_app_fw_nic(struct nfp_net_hw_priv *hw_priv)
/* Loop through all physical ports on PF */
for (i = 0; i < pf_dev->total_phyports; i++) {
- if (pf_dev->multi_pf.enabled)
- snprintf(port_name, sizeof(port_name), "%s",
- pf_dev->pci_dev->device.name);
- else
- snprintf(port_name, sizeof(port_name), "%s_port%u",
- pf_dev->pci_dev->device.name, i);
+ nfp_port_name_generate(port_name, sizeof(port_name), i, pf_dev);
id = nfp_function_id_get(pf_dev, i);
hw_init.idx = id;
@@ -1870,15 +1869,10 @@ nfp_init_app_fw_nic(struct nfp_net_hw_priv *hw_priv)
return 0;
port_cleanup:
- for (i = 0; i < pf_dev->total_phyports; i++) {
+ for (uint32_t j = 0; j < i; j++) {
struct rte_eth_dev *eth_dev;
- if (pf_dev->multi_pf.enabled)
- snprintf(port_name, sizeof(port_name), "%s",
- pf_dev->pci_dev->device.name);
- else
- snprintf(port_name, sizeof(port_name), "%s_port%u",
- pf_dev->pci_dev->device.name, i);
+ nfp_port_name_generate(port_name, sizeof(port_name), j, pf_dev);
eth_dev = rte_eth_dev_get_by_name(port_name);
if (eth_dev != NULL)
rte_eth_dev_destroy(eth_dev, nfp_net_uninit);
@@ -2369,7 +2363,7 @@ nfp_pf_init(struct rte_pci_device *pci_dev)
pf_dev->nfp_eth_table = nfp_eth_table;
pf_dev->multi_pf.enabled = nfp_check_multi_pf_from_nsp(pci_dev, cpp);
pf_dev->multi_pf.function_id = function_id;
- pf_dev->total_phyports = nfp_net_get_port_num(pf_dev);
+ pf_dev->total_phyports = nfp_net_get_phyports_from_nsp(pf_dev);
ret = nfp_net_force_port_down(pf_dev);
if (ret != 0) {
@@ -2547,42 +2541,37 @@ static int
nfp_secondary_init_app_fw_nic(struct nfp_net_hw_priv *hw_priv)
{
uint32_t i;
- int err = 0;
int ret = 0;
- uint8_t function_id;
uint32_t total_vnics;
- char pf_name[RTE_ETH_NAME_MAX_LEN];
+ char port_name[RTE_ETH_NAME_MAX_LEN];
struct nfp_pf_dev *pf_dev = hw_priv->pf_dev;
- /* Read the number of vNIC's created for the PF */
- function_id = (pf_dev->pci_dev->addr.function) & 0x07;
- snprintf(pf_name, sizeof(pf_name), "nfd_cfg_pf%u_num_ports", function_id);
- total_vnics = nfp_rtsym_read_le(pf_dev->sym_tbl, pf_name, &err);
- if (err != 0 || total_vnics == 0 || total_vnics > 8) {
- PMD_INIT_LOG(ERR, "%s symbol with wrong value", pf_name);
- return -ENODEV;
- }
+ total_vnics = nfp_net_get_phyports_from_fw(pf_dev);
for (i = 0; i < total_vnics; i++) {
- char port_name[RTE_ETH_NAME_MAX_LEN];
-
- if (nfp_check_multi_pf_from_fw(total_vnics))
- snprintf(port_name, sizeof(port_name), "%s",
- pf_dev->pci_dev->device.name);
- else
- snprintf(port_name, sizeof(port_name), "%s_port%u",
- pf_dev->pci_dev->device.name, i);
+ nfp_port_name_generate(port_name, sizeof(port_name), i, pf_dev);
PMD_INIT_LOG(DEBUG, "Secondary attaching to port %s", port_name);
ret = rte_eth_dev_create(&pf_dev->pci_dev->device, port_name, 0,
NULL, NULL, nfp_secondary_net_init, hw_priv);
if (ret != 0) {
PMD_INIT_LOG(ERR, "Secondary process attach to port %s failed", port_name);
- ret = -ENODEV;
- break;
+ goto port_cleanup;
}
}
+ return 0;
+
+port_cleanup:
+ for (uint32_t j = 0; j < i; j++) {
+ struct rte_eth_dev *eth_dev;
+
+ nfp_port_name_generate(port_name, sizeof(port_name), j, pf_dev);
+ eth_dev = rte_eth_dev_get_by_name(port_name);
+ if (eth_dev != NULL)
+ rte_eth_dev_destroy(eth_dev, NULL);
+ }
+
return ret;
}
@@ -2714,6 +2703,11 @@ nfp_pf_secondary_init(struct rte_pci_device *pci_dev)
pf_dev->sym_tbl = sym_tbl;
+ /* Read the number of physical ports from firmware */
+ pf_dev->multi_pf.function_id = function_id;
+ pf_dev->total_phyports = nfp_net_get_phyports_from_fw(pf_dev);
+ pf_dev->multi_pf.enabled = nfp_check_multi_pf_from_fw(pf_dev->total_phyports);
+
/* Read the app ID of the firmware loaded */
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);
diff --git a/drivers/net/nfp/nfp_net_common.c b/drivers/net/nfp/nfp_net_common.c
index 99f6b61947..86a1fbfaf2 100644
--- a/drivers/net/nfp/nfp_net_common.c
+++ b/drivers/net/nfp/nfp_net_common.c
@@ -14,6 +14,7 @@
#include "nfdk/nfp_nfdk.h"
#include "nfpcore/nfp_mip.h"
#include "nfpcore/nfp_nsp.h"
+#include "nfpcore/nfp_rtsym.h"
#include "nfp_logs.h"
#include "nfp_net_meta.h"
@@ -2724,7 +2725,7 @@ nfp_net_fec_set(struct rte_eth_dev *dev,
}
uint32_t
-nfp_net_get_port_num(struct nfp_pf_dev *pf_dev)
+nfp_net_get_phyports_from_nsp(struct nfp_pf_dev *pf_dev)
{
if (pf_dev->multi_pf.enabled)
return 1;
@@ -2732,6 +2733,25 @@ nfp_net_get_port_num(struct nfp_pf_dev *pf_dev)
return pf_dev->nfp_eth_table->count;
}
+uint32_t
+nfp_net_get_phyports_from_fw(struct nfp_pf_dev *pf_dev)
+{
+ int ret = 0;
+ uint8_t total_phyports;
+ char pf_name[RTE_ETH_NAME_MAX_LEN];
+
+ /* Read the number of vNIC's created for the PF */
+ snprintf(pf_name, sizeof(pf_name), "nfd_cfg_pf%u_num_ports",
+ pf_dev->multi_pf.function_id);
+ total_phyports = nfp_rtsym_read_le(pf_dev->sym_tbl, pf_name, &ret);
+ if (ret != 0 || total_phyports == 0 || total_phyports > 8) {
+ PMD_INIT_LOG(ERR, "%s symbol with wrong value", pf_name);
+ return 0;
+ }
+
+ return total_phyports;
+}
+
uint8_t
nfp_function_id_get(const struct nfp_pf_dev *pf_dev,
uint8_t port_id)
diff --git a/drivers/net/nfp/nfp_net_common.h b/drivers/net/nfp/nfp_net_common.h
index fb244383b7..8429db68f0 100644
--- a/drivers/net/nfp/nfp_net_common.h
+++ b/drivers/net/nfp/nfp_net_common.h
@@ -374,7 +374,8 @@ void nfp_net_get_fw_version(struct nfp_cpp *cpp,
uint32_t *fw_version);
int nfp_net_txrwb_alloc(struct rte_eth_dev *eth_dev);
void nfp_net_txrwb_free(struct rte_eth_dev *eth_dev);
-uint32_t nfp_net_get_port_num(struct nfp_pf_dev *pf_dev);
+uint32_t nfp_net_get_phyports_from_nsp(struct nfp_pf_dev *pf_dev);
+uint32_t nfp_net_get_phyports_from_fw(struct nfp_pf_dev *pf_dev);
uint8_t nfp_function_id_get(const struct nfp_pf_dev *pf_dev,
uint8_t port_id);
int nfp_net_vf_config_app_init(struct nfp_net_hw *net_hw,
--
2.39.1
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 05/10] net/nfp: fix problem caused by configure function
2024-10-10 9:17 [PATCH 00/10] modify some logic of NFP PMD Chaoyong He
` (3 preceding siblings ...)
2024-10-10 9:17 ` [PATCH 04/10] net/nfp: improve the logic readability Chaoyong He
@ 2024-10-10 9:17 ` Chaoyong He
2024-10-10 15:13 ` Stephen Hemminger
2024-10-10 9:17 ` [PATCH 06/10] net/nfp: add check logic for port up/down function Chaoyong He
` (5 subsequent siblings)
10 siblings, 1 reply; 38+ messages in thread
From: Chaoyong He @ 2024-10-10 9:17 UTC (permalink / raw)
To: dev; +Cc: oss-drivers, Chaoyong He, stable, Long Wu, Peng Zhang
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 06/10] net/nfp: add check logic for port up/down function
2024-10-10 9:17 [PATCH 00/10] modify some logic of NFP PMD Chaoyong He
` (4 preceding siblings ...)
2024-10-10 9:17 ` [PATCH 05/10] net/nfp: fix problem caused by configure function Chaoyong He
@ 2024-10-10 9:17 ` Chaoyong He
2024-10-10 15:14 ` Stephen Hemminger
2024-10-10 9:17 ` [PATCH 07/10] net/nfp: fix problem caused by commit end function Chaoyong He
` (4 subsequent siblings)
10 siblings, 1 reply; 38+ messages in thread
From: Chaoyong He @ 2024-10-10 9:17 UTC (permalink / raw)
To: dev; +Cc: oss-drivers, Chaoyong He, Long Wu, Peng Zhang
The 'nfp_eth_set_configured()' function is not always success, so
need to check the return value of it.
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>
---
.../net/nfp/flower/nfp_flower_representor.c | 18 +++++++++++++-----
drivers/net/nfp/nfp_ethdev.c | 4 +++-
drivers/net/nfp/nfp_net_common.c | 5 ++++-
3 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/drivers/net/nfp/flower/nfp_flower_representor.c b/drivers/net/nfp/flower/nfp_flower_representor.c
index d1558b905c..eb0a02874b 100644
--- a/drivers/net/nfp/flower/nfp_flower_representor.c
+++ b/drivers/net/nfp/flower/nfp_flower_representor.c
@@ -83,6 +83,7 @@ nfp_flower_repr_dev_infos_get(__rte_unused struct rte_eth_dev *dev,
static int
nfp_flower_repr_dev_start(struct rte_eth_dev *dev)
{
+ int ret;
uint16_t i;
struct nfp_net_hw_priv *hw_priv;
struct nfp_flower_representor *repr;
@@ -92,8 +93,11 @@ nfp_flower_repr_dev_start(struct rte_eth_dev *dev)
hw_priv = dev->process_private;
app_fw_flower = repr->app_fw_flower;
- if (repr->repr_type == NFP_REPR_TYPE_PHYS_PORT)
- nfp_eth_set_configured(hw_priv->pf_dev->cpp, repr->nfp_idx, 1);
+ if (repr->repr_type == NFP_REPR_TYPE_PHYS_PORT) {
+ ret = nfp_eth_set_configured(hw_priv->pf_dev->cpp, repr->nfp_idx, 1);
+ if (ret < 0)
+ return ret;
+ }
nfp_flower_cmsg_port_mod(app_fw_flower, repr->port_id, true);
@@ -109,6 +113,7 @@ static int
nfp_flower_repr_dev_stop(struct rte_eth_dev *dev)
{
uint16_t i;
+ int ret = 0;
struct nfp_net_hw_priv *hw_priv;
struct nfp_flower_representor *repr;
struct nfp_app_fw_flower *app_fw_flower;
@@ -119,15 +124,18 @@ nfp_flower_repr_dev_stop(struct rte_eth_dev *dev)
nfp_flower_cmsg_port_mod(app_fw_flower, repr->port_id, false);
- if (repr->repr_type == NFP_REPR_TYPE_PHYS_PORT)
- nfp_eth_set_configured(hw_priv->pf_dev->cpp, repr->nfp_idx, 0);
+ if (repr->repr_type == NFP_REPR_TYPE_PHYS_PORT) {
+ ret = nfp_eth_set_configured(hw_priv->pf_dev->cpp, repr->nfp_idx, 0);
+ if (ret == 1)
+ ret = 0;
+ }
for (i = 0; i < dev->data->nb_rx_queues; i++)
dev->data->rx_queue_state[i] = RTE_ETH_QUEUE_STATE_STOPPED;
for (i = 0; i < dev->data->nb_tx_queues; i++)
dev->data->tx_queue_state[i] = RTE_ETH_QUEUE_STATE_STOPPED;
- return 0;
+ return ret;
}
static int
diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index 2fe6b1a292..302149e9dc 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -495,7 +495,9 @@ nfp_net_start(struct rte_eth_dev *dev)
}
/* Configure the physical port up */
- nfp_eth_set_configured(pf_dev->cpp, net_hw->nfp_idx, 1);
+ ret = nfp_eth_set_configured(pf_dev->cpp, net_hw->nfp_idx, 1);
+ if (ret < 0)
+ goto error;
for (i = 0; i < dev->data->nb_rx_queues; i++)
dev->data->rx_queue_state[i] = RTE_ETH_QUEUE_STATE_STARTED;
diff --git a/drivers/net/nfp/nfp_net_common.c b/drivers/net/nfp/nfp_net_common.c
index 86a1fbfaf2..80d60515d8 100644
--- a/drivers/net/nfp/nfp_net_common.c
+++ b/drivers/net/nfp/nfp_net_common.c
@@ -2427,6 +2427,7 @@ nfp_net_ctrl_bar_size_set(struct nfp_pf_dev *pf_dev)
int
nfp_net_stop(struct rte_eth_dev *dev)
{
+ int ret;
struct nfp_net_hw *hw;
struct nfp_net_hw_priv *hw_priv;
@@ -2439,7 +2440,9 @@ nfp_net_stop(struct rte_eth_dev *dev)
nfp_net_stop_tx_queue(dev);
nfp_net_stop_rx_queue(dev);
- 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;
}
--
2.39.1
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 07/10] net/nfp: fix problem caused by commit end function
2024-10-10 9:17 [PATCH 00/10] modify some logic of NFP PMD Chaoyong He
` (5 preceding siblings ...)
2024-10-10 9:17 ` [PATCH 06/10] net/nfp: add check logic for port up/down function Chaoyong He
@ 2024-10-10 9:17 ` Chaoyong He
2024-10-10 15:15 ` Stephen Hemminger
2024-10-10 9:17 ` [PATCH 08/10] net/nfp: fix problem caused by FEC set Chaoyong He
` (3 subsequent siblings)
10 siblings, 1 reply; 38+ messages in thread
From: Chaoyong He @ 2024-10-10 9:17 UTC (permalink / raw)
To: dev; +Cc: oss-drivers, Chaoyong He, zerun.fu, stable, Long Wu, Peng Zhang
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 08/10] net/nfp: fix problem caused by FEC set
2024-10-10 9:17 [PATCH 00/10] modify some logic of NFP PMD Chaoyong He
` (6 preceding siblings ...)
2024-10-10 9:17 ` [PATCH 07/10] net/nfp: fix problem caused by commit end function Chaoyong He
@ 2024-10-10 9:17 ` Chaoyong He
2024-10-10 15:15 ` Stephen Hemminger
2024-10-10 9:17 ` [PATCH 09/10] net/nfp: modify the comment of some control messages Chaoyong He
` (2 subsequent siblings)
10 siblings, 1 reply; 38+ messages in thread
From: Chaoyong He @ 2024-10-10 9:17 UTC (permalink / raw)
To: dev; +Cc: oss-drivers, Chaoyong He, zerun.fu, stable, Long Wu, Peng Zhang
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 09/10] net/nfp: modify the comment of some control messages
2024-10-10 9:17 [PATCH 00/10] modify some logic of NFP PMD Chaoyong He
` (7 preceding siblings ...)
2024-10-10 9:17 ` [PATCH 08/10] net/nfp: fix problem caused by FEC set Chaoyong He
@ 2024-10-10 9:17 ` Chaoyong He
2024-10-10 15:15 ` Stephen Hemminger
2024-10-10 9:17 ` [PATCH 10/10] net/nfp: fix memory leak in VF initialization logic Chaoyong He
2024-10-12 2:40 ` [PATCH v2 00/10] modify some logic of NFP PMD Chaoyong He
10 siblings, 1 reply; 38+ messages in thread
From: Chaoyong He @ 2024-10-10 9:17 UTC (permalink / raw)
To: dev; +Cc: oss-drivers, Chaoyong He, Long Wu
The comment of some control messages are not right, which conflict
with the data structure and may confuse the other developers.
Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
---
drivers/net/nfp/flower/nfp_flower_cmsg.h | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/net/nfp/flower/nfp_flower_cmsg.h b/drivers/net/nfp/flower/nfp_flower_cmsg.h
index 5fc4210d8b..eda047a404 100644
--- a/drivers/net/nfp/flower/nfp_flower_cmsg.h
+++ b/drivers/net/nfp/flower/nfp_flower_cmsg.h
@@ -738,7 +738,7 @@ struct nfp_fl_act_output {
* 3 2 1
* 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
- * | - |opcode | - |jump_id| - |
+ * | res | opcode | res | len_lw| - |
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
* | eth_dst_47_16_mask |
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
@@ -777,7 +777,7 @@ struct nfp_fl_act_push_vlan {
* 3 2 1
* 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
- * | - |opcode | |jump_id| - |
+ * | res | opcode | res | len_lw| - |
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
* | ipv4_saddr_mask |
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
@@ -802,7 +802,7 @@ struct nfp_fl_act_set_ip4_addrs {
* 3 2 1
* 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
- * | - |opcode | |jump_id| ttl_mask | tos_mask |
+ * | res | opcode | res | len_lw| ttl_mask | tos_mask |
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
* | ttl | tos | 0 |
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
@@ -821,7 +821,7 @@ struct nfp_fl_act_set_ip4_ttl_tos {
* 3 2 1
* 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
- * | - |opcode | |jump_id| - |
+ * | res | opcode | res | len_lw| - |
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
* | ipv6_addr_127_96_mask |
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
@@ -854,7 +854,7 @@ struct nfp_fl_act_set_ipv6_addr {
* 3 2 1
* 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
- * | - |opcode | |jump_id| tclass_mask | hlimit_mask |
+ * | res | opcode | res | len_lw| tclass_mask | hlimit_mask |
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
* | 0 | tclass | hlimit |
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
@@ -879,7 +879,7 @@ struct nfp_fl_act_set_ipv6_tc_hl_fl {
* 3 2 1
* 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
- * | - |opcode | |jump_id| - |
+ * | res | opcode | res | len_lw| - |
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
* | src_mask | dst_mask |
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
@@ -900,7 +900,7 @@ struct nfp_fl_act_set_tport {
* 3 2 1
* 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
- * | - | opcode | |jump_id| - |M| - |V|
+ * | res | opcode | res | len_lw| - |M| - |V|
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
* | ipv6_daddr_127_96 / ipv4_daddr |
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
--
2.39.1
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 10/10] net/nfp: fix memory leak in VF initialization logic
2024-10-10 9:17 [PATCH 00/10] modify some logic of NFP PMD Chaoyong He
` (8 preceding siblings ...)
2024-10-10 9:17 ` [PATCH 09/10] net/nfp: modify the comment of some control messages Chaoyong He
@ 2024-10-10 9:17 ` Chaoyong He
2024-10-10 15:19 ` Stephen Hemminger
2024-10-12 2:40 ` [PATCH v2 00/10] modify some logic of NFP PMD Chaoyong He
10 siblings, 1 reply; 38+ messages in thread
From: Chaoyong He @ 2024-10-10 9:17 UTC (permalink / raw)
To: dev; +Cc: oss-drivers, Chaoyong He, stable, Long Wu, Peng Zhang
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 02/10] net/nfp: fix malloc name problem in secondary process
2024-10-10 9:17 ` [PATCH 02/10] net/nfp: fix malloc name problem in secondary process Chaoyong He
@ 2024-10-10 15:07 ` Stephen Hemminger
2024-10-11 2:31 ` Chaoyong He
0 siblings, 1 reply; 38+ messages in thread
From: Stephen Hemminger @ 2024-10-10 15:07 UTC (permalink / raw)
To: Chaoyong He; +Cc: dev, oss-drivers, peng.zhang, stable, Long Wu
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.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 01/10] net/nfp: use strlcpy for copying string
2024-10-10 9:17 ` [PATCH 01/10] net/nfp: use strlcpy for copying string Chaoyong He
@ 2024-10-10 15:08 ` Stephen Hemminger
0 siblings, 0 replies; 38+ messages in thread
From: Stephen Hemminger @ 2024-10-10 15:08 UTC (permalink / raw)
To: Chaoyong He; +Cc: dev, oss-drivers, Long Wu, Peng Zhang
On Thu, 10 Oct 2024 17:17:07 +0800
Chaoyong He <chaoyong.he@corigine.com> wrote:
> Replace 'snprintf()' with 'strlcpy()' where applicable.
> Using 'strlcpy()' is safe practice when copying strings, as it
> will include a null terminator.
>
> 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>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 03/10] net/nfp: simplify some function parameters
2024-10-10 9:17 ` [PATCH 03/10] net/nfp: simplify some function parameters Chaoyong He
@ 2024-10-10 15:08 ` Stephen Hemminger
0 siblings, 0 replies; 38+ messages in thread
From: Stephen Hemminger @ 2024-10-10 15:08 UTC (permalink / raw)
To: Chaoyong He; +Cc: dev, oss-drivers, Long Wu, Peng Zhang
On Thu, 10 Oct 2024 17:17:09 +0800
Chaoyong He <chaoyong.he@corigine.com> wrote:
> Refactor to the logic of initialize process to simplify some function
> parameters, both for primary and secodary process.
>
> 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>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 04/10] net/nfp: improve the logic readability
2024-10-10 9:17 ` [PATCH 04/10] net/nfp: improve the logic readability Chaoyong He
@ 2024-10-10 15:10 ` Stephen Hemminger
2024-10-11 2:36 ` Chaoyong He
0 siblings, 1 reply; 38+ messages in thread
From: Stephen Hemminger @ 2024-10-10 15:10 UTC (permalink / raw)
To: Chaoyong He; +Cc: dev, oss-drivers, Long Wu, Peng Zhang
On Thu, 10 Oct 2024 17:17:10 +0800
Chaoyong He <chaoyong.he@corigine.com> wrote:
> +
> +port_cleanup:
> + for (uint32_t j = 0; j < i; j++) {
> + struct rte_eth_dev *eth_dev;
> +
> + nfp_port_name_generate(port_name, sizeof(port_name), j, pf_dev);
> + eth_dev = rte_eth_dev_get_by_name(port_name);
> + if (eth_dev != NULL)
> + rte_eth_dev_destroy(eth_dev, NULL);
> + }
> +
You could skip the lookup if you kept an array of eth_dev's that were created?
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 05/10] net/nfp: fix problem caused by configure function
2024-10-10 9:17 ` [PATCH 05/10] net/nfp: fix problem caused by configure function Chaoyong He
@ 2024-10-10 15:13 ` Stephen Hemminger
0 siblings, 0 replies; 38+ messages in thread
From: Stephen Hemminger @ 2024-10-10 15:13 UTC (permalink / raw)
To: Chaoyong He; +Cc: dev, oss-drivers, stable, Long Wu, Peng Zhang
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>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 06/10] net/nfp: add check logic for port up/down function
2024-10-10 9:17 ` [PATCH 06/10] net/nfp: add check logic for port up/down function Chaoyong He
@ 2024-10-10 15:14 ` Stephen Hemminger
0 siblings, 0 replies; 38+ messages in thread
From: Stephen Hemminger @ 2024-10-10 15:14 UTC (permalink / raw)
To: Chaoyong He; +Cc: dev, oss-drivers, Long Wu, Peng Zhang
On Thu, 10 Oct 2024 17:17:12 +0800
Chaoyong He <chaoyong.he@corigine.com> wrote:
> The 'nfp_eth_set_configured()' function is not always success, so
> need to check the return value of it.
>
> 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>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 07/10] net/nfp: fix problem caused by commit end function
2024-10-10 9:17 ` [PATCH 07/10] net/nfp: fix problem caused by commit end function Chaoyong He
@ 2024-10-10 15:15 ` Stephen Hemminger
0 siblings, 0 replies; 38+ messages in thread
From: Stephen Hemminger @ 2024-10-10 15:15 UTC (permalink / raw)
To: Chaoyong He; +Cc: dev, oss-drivers, zerun.fu, stable, Long Wu, Peng Zhang
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>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 08/10] net/nfp: fix problem caused by FEC set
2024-10-10 9:17 ` [PATCH 08/10] net/nfp: fix problem caused by FEC set Chaoyong He
@ 2024-10-10 15:15 ` Stephen Hemminger
0 siblings, 0 replies; 38+ messages in thread
From: Stephen Hemminger @ 2024-10-10 15:15 UTC (permalink / raw)
To: Chaoyong He; +Cc: dev, oss-drivers, zerun.fu, stable, Long Wu, Peng Zhang
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>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 09/10] net/nfp: modify the comment of some control messages
2024-10-10 9:17 ` [PATCH 09/10] net/nfp: modify the comment of some control messages Chaoyong He
@ 2024-10-10 15:15 ` Stephen Hemminger
0 siblings, 0 replies; 38+ messages in thread
From: Stephen Hemminger @ 2024-10-10 15:15 UTC (permalink / raw)
To: Chaoyong He; +Cc: dev, oss-drivers, Long Wu
On Thu, 10 Oct 2024 17:17:15 +0800
Chaoyong He <chaoyong.he@corigine.com> wrote:
> The comment of some control messages are not right, which conflict
> with the data structure and may confuse the other developers.
>
> Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
> Reviewed-by: Long Wu <long.wu@corigine.com>
> ---
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 10/10] net/nfp: fix memory leak in VF initialization logic
2024-10-10 9:17 ` [PATCH 10/10] net/nfp: fix memory leak in VF initialization logic Chaoyong He
@ 2024-10-10 15:19 ` Stephen Hemminger
2024-10-11 2:38 ` Chaoyong He
0 siblings, 1 reply; 38+ messages in thread
From: Stephen Hemminger @ 2024-10-10 15:19 UTC (permalink / raw)
To: Chaoyong He; +Cc: dev, oss-drivers, stable, Long Wu, Peng Zhang
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>
^ permalink raw reply [flat|nested] 38+ messages in thread
* RE: [PATCH 02/10] net/nfp: fix malloc name problem in secondary process
2024-10-10 15:07 ` Stephen Hemminger
@ 2024-10-11 2:31 ` Chaoyong He
0 siblings, 0 replies; 38+ messages in thread
From: Chaoyong He @ 2024-10-11 2:31 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev, oss-drivers, Nole Zhang, stable, Long Wu
> 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.
Yeah, I have learned this from some patch thread.
I will choose using 'NULL' in the newly added logic, treat this API as a 'white box'.
But for already exist logic like this one, I prefer to fix it, and treat it as a 'black box'.
^ permalink raw reply [flat|nested] 38+ messages in thread
* RE: [PATCH 04/10] net/nfp: improve the logic readability
2024-10-10 15:10 ` Stephen Hemminger
@ 2024-10-11 2:36 ` Chaoyong He
0 siblings, 0 replies; 38+ messages in thread
From: Chaoyong He @ 2024-10-11 2:36 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev, oss-drivers, Long Wu, Nole Zhang
> On Thu, 10 Oct 2024 17:17:10 +0800
> Chaoyong He <chaoyong.he@corigine.com> wrote:
>
> > +
> > +port_cleanup:
> > + for (uint32_t j = 0; j < i; j++) {
> > + struct rte_eth_dev *eth_dev;
> > +
> > + nfp_port_name_generate(port_name, sizeof(port_name), j,
> pf_dev);
> > + eth_dev = rte_eth_dev_get_by_name(port_name);
> > + if (eth_dev != NULL)
> > + rte_eth_dev_destroy(eth_dev, NULL);
> > + }
> > +
>
> You could skip the lookup if you kept an array of eth_dev's that were created?
Yes, it does make sense and sounds a good idea.
Maybe we can do it in a future patch rather than in this one?
Anyway, thanks for your review.
^ permalink raw reply [flat|nested] 38+ messages in thread
* RE: [PATCH 10/10] net/nfp: fix memory leak in VF initialization logic
2024-10-10 15:19 ` Stephen Hemminger
@ 2024-10-11 2:38 ` Chaoyong He
0 siblings, 0 replies; 38+ messages in thread
From: Chaoyong He @ 2024-10-11 2:38 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev, oss-drivers, stable, Long Wu, Nole Zhang
> 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.
Yes, I think what you said is better.
Will do it in next version.
Thanks for your review.
>
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v2 00/10] modify some logic of NFP PMD
2024-10-10 9:17 [PATCH 00/10] modify some logic of NFP PMD Chaoyong He
` (9 preceding siblings ...)
2024-10-10 9:17 ` [PATCH 10/10] net/nfp: fix memory leak in VF initialization logic Chaoyong He
@ 2024-10-12 2:40 ` Chaoyong He
2024-10-12 2:40 ` [PATCH v2 01/10] net/nfp: use strlcpy for copying string Chaoyong He
` (10 more replies)
10 siblings, 11 replies; 38+ messages in thread
From: Chaoyong He @ 2024-10-12 2:40 UTC (permalink / raw)
To: dev; +Cc: oss-drivers, Chaoyong He
This patch series refactor some logic of NFP PMD to make it more
clear, also fix some bugs.
---
v2:
* Following the reviewer's request, change a little logic of patch 10/10.
* Add 'Acked-by' tag from reviewer.
---
Chaoyong He (10):
net/nfp: use strlcpy for copying string
net/nfp: fix malloc name problem in secondary process
net/nfp: simplify some function parameters
net/nfp: improve the logic readability
net/nfp: fix problem caused by configure function
net/nfp: add check logic for port up/down function
net/nfp: fix problem caused by commit end function
net/nfp: fix problem caused by FEC set
net/nfp: modify the comment of some control messages
net/nfp: fix memory leak in VF initialization logic
drivers/net/nfp/flower/nfp_flower_cmsg.h | 14 +-
.../net/nfp/flower/nfp_flower_representor.c | 22 +-
drivers/net/nfp/nfp_ethdev.c | 259 +++++++++---------
drivers/net/nfp/nfp_ethdev_vf.c | 7 +-
drivers/net/nfp/nfp_net_common.c | 49 +++-
drivers/net/nfp/nfp_net_common.h | 4 +-
6 files changed, 196 insertions(+), 159 deletions(-)
--
2.39.1
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v2 01/10] net/nfp: use strlcpy for copying string
2024-10-12 2:40 ` [PATCH v2 00/10] modify some logic of NFP PMD Chaoyong He
@ 2024-10-12 2:40 ` Chaoyong He
2024-10-12 2:40 ` [PATCH v2 02/10] net/nfp: fix malloc name problem in secondary process Chaoyong He
` (9 subsequent siblings)
10 siblings, 0 replies; 38+ messages in thread
From: Chaoyong He @ 2024-10-12 2:40 UTC (permalink / raw)
To: dev; +Cc: oss-drivers, Chaoyong He, Long Wu, Peng Zhang, Stephen Hemminger
Replace 'snprintf()' with 'strlcpy()' where applicable.
Using 'strlcpy()' is safe practice when copying strings, as it
will include a null terminator.
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>
---
drivers/net/nfp/flower/nfp_flower_representor.c | 4 ++--
drivers/net/nfp/nfp_net_common.c | 8 ++++----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/nfp/flower/nfp_flower_representor.c b/drivers/net/nfp/flower/nfp_flower_representor.c
index 5db7d50618..d1558b905c 100644
--- a/drivers/net/nfp/flower/nfp_flower_representor.c
+++ b/drivers/net/nfp/flower/nfp_flower_representor.c
@@ -576,7 +576,7 @@ nfp_flower_pf_repr_init(struct rte_eth_dev *eth_dev,
repr->repr_type = init_repr_data->repr_type;
repr->app_fw_flower = init_repr_data->app_fw_flower;
- snprintf(repr->name, sizeof(repr->name), "%s", init_repr_data->name);
+ strlcpy(repr->name, init_repr_data->name, sizeof(repr->name));
eth_dev->dev_ops = &nfp_flower_pf_repr_dev_ops;
eth_dev->rx_pkt_burst = nfp_net_recv_pkts;
@@ -652,7 +652,7 @@ nfp_flower_repr_init(struct rte_eth_dev *eth_dev,
repr->repr_type = init_repr_data->repr_type;
repr->app_fw_flower = init_repr_data->app_fw_flower;
- snprintf(repr->name, sizeof(repr->name), "%s", init_repr_data->name);
+ strlcpy(repr->name, init_repr_data->name, sizeof(repr->name));
eth_dev->dev_ops = &nfp_flower_repr_dev_ops;
eth_dev->rx_pkt_burst = nfp_flower_repr_rx_burst;
diff --git a/drivers/net/nfp/nfp_net_common.c b/drivers/net/nfp/nfp_net_common.c
index 4a2c536704..c9a95ed632 100644
--- a/drivers/net/nfp/nfp_net_common.c
+++ b/drivers/net/nfp/nfp_net_common.c
@@ -2306,7 +2306,7 @@ nfp_net_get_mip_name(struct nfp_net_hw_priv *hw_priv,
if (mip == NULL)
return;
- snprintf(mip_name, FW_VER_LEN, "%s", nfp_mip_name(mip));
+ strlcpy(mip_name, nfp_mip_name(mip), FW_VER_LEN);
nfp_mip_close(mip);
}
@@ -2317,13 +2317,13 @@ nfp_net_get_app_name(struct nfp_net_hw_priv *hw_priv,
{
switch (hw_priv->pf_dev->app_fw_id) {
case NFP_APP_FW_CORE_NIC:
- snprintf(app_name, FW_VER_LEN, "%s", "nic");
+ strlcpy(app_name, "nic", FW_VER_LEN);
break;
case NFP_APP_FW_FLOWER_NIC:
- snprintf(app_name, FW_VER_LEN, "%s", "flower");
+ strlcpy(app_name, "flower", FW_VER_LEN);
break;
default:
- snprintf(app_name, FW_VER_LEN, "%s", "unknown");
+ strlcpy(app_name, "unknown", FW_VER_LEN);
break;
}
}
--
2.39.1
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v2 02/10] net/nfp: fix malloc name problem in secondary process
2024-10-12 2:40 ` [PATCH v2 00/10] modify some logic of NFP PMD Chaoyong He
2024-10-12 2:40 ` [PATCH v2 01/10] net/nfp: use strlcpy for copying string Chaoyong He
@ 2024-10-12 2:40 ` Chaoyong He
2024-10-12 2:45 ` Stephen Hemminger
2024-10-12 2:41 ` [PATCH v2 03/10] net/nfp: simplify some function parameters Chaoyong He
` (8 subsequent siblings)
10 siblings, 1 reply; 38+ messages in thread
From: Chaoyong He @ 2024-10-12 2:40 UTC (permalink / raw)
To: dev; +Cc: oss-drivers, Chaoyong He, peng.zhang, stable, Long Wu
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v2 03/10] net/nfp: simplify some function parameters
2024-10-12 2:40 ` [PATCH v2 00/10] modify some logic of NFP PMD Chaoyong He
2024-10-12 2:40 ` [PATCH v2 01/10] net/nfp: use strlcpy for copying string Chaoyong He
2024-10-12 2:40 ` [PATCH v2 02/10] net/nfp: fix malloc name problem in secondary process Chaoyong He
@ 2024-10-12 2:41 ` Chaoyong He
2024-10-12 2:41 ` [PATCH v2 04/10] net/nfp: improve the logic readability Chaoyong He
` (7 subsequent siblings)
10 siblings, 0 replies; 38+ messages in thread
From: Chaoyong He @ 2024-10-12 2:41 UTC (permalink / raw)
To: dev; +Cc: oss-drivers, Chaoyong He, Long Wu, Peng Zhang, Stephen Hemminger
Refactor to the logic of initialize process to simplify some function
parameters, both for primary and secodary process.
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>
---
drivers/net/nfp/nfp_ethdev.c | 153 +++++++++++++++----------------
drivers/net/nfp/nfp_net_common.c | 5 +-
drivers/net/nfp/nfp_net_common.h | 3 +-
3 files changed, 76 insertions(+), 85 deletions(-)
diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index 3732abc9fe..121f82af6d 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -1160,14 +1160,15 @@ nfp_net_init(struct rte_eth_dev *eth_dev,
}
static int
-nfp_net_device_activate(struct nfp_cpp *cpp,
- struct nfp_multi_pf *multi_pf)
+nfp_net_device_activate(struct nfp_pf_dev *pf_dev)
{
int ret;
struct nfp_nsp *nsp;
+ struct nfp_multi_pf *multi_pf;
+ multi_pf = &pf_dev->multi_pf;
if (multi_pf->enabled && multi_pf->function_id != 0) {
- nsp = nfp_nsp_open(cpp);
+ nsp = nfp_nsp_open(pf_dev->cpp);
if (nsp == NULL) {
PMD_DRV_LOG(ERR, "NFP error when obtaining NSP handle");
return -EIO;
@@ -1185,10 +1186,7 @@ nfp_net_device_activate(struct nfp_cpp *cpp,
#define DEFAULT_FW_PATH "/lib/firmware/netronome"
static int
-nfp_fw_get_name(struct rte_pci_device *dev,
- struct nfp_cpp *cpp,
- struct nfp_eth_table *nfp_eth_table,
- struct nfp_hwinfo *hwinfo,
+nfp_fw_get_name(struct nfp_pf_dev *pf_dev,
char *fw_name,
size_t fw_size)
{
@@ -1199,11 +1197,11 @@ nfp_fw_get_name(struct rte_pci_device *dev,
const char *nfp_fw_model;
const uint8_t *cpp_serial;
- cpp_serial_len = nfp_cpp_serial(cpp, &cpp_serial);
+ cpp_serial_len = nfp_cpp_serial(pf_dev->cpp, &cpp_serial);
if (cpp_serial_len != NFP_SERIAL_LEN)
return -ERANGE;
- interface = nfp_cpp_interface(cpp);
+ interface = nfp_cpp_interface(pf_dev->cpp);
/* Looking for firmware file in order of priority */
@@ -1220,15 +1218,15 @@ nfp_fw_get_name(struct rte_pci_device *dev,
/* Then try the PCI name */
snprintf(fw_name, fw_size, "%s/pci-%s.nffw", DEFAULT_FW_PATH,
- dev->name);
+ pf_dev->pci_dev->name);
PMD_DRV_LOG(DEBUG, "Trying with fw file: %s", fw_name);
if (access(fw_name, F_OK) == 0)
return 0;
- nfp_fw_model = nfp_hwinfo_lookup(hwinfo, "nffw.partno");
+ nfp_fw_model = nfp_hwinfo_lookup(pf_dev->hwinfo, "nffw.partno");
if (nfp_fw_model == NULL) {
- nfp_fw_model = nfp_hwinfo_lookup(hwinfo, "assembly.partno");
+ nfp_fw_model = nfp_hwinfo_lookup(pf_dev->hwinfo, "assembly.partno");
if (nfp_fw_model == NULL) {
PMD_DRV_LOG(ERR, "firmware model NOT found");
return -EIO;
@@ -1244,8 +1242,8 @@ nfp_fw_get_name(struct rte_pci_device *dev,
/* Finally try the card type and media */
snprintf(card_desc, sizeof(card_desc), "nic_%s_%dx%d.nffw",
- nfp_fw_model, nfp_eth_table->count,
- nfp_eth_table->ports[0].speed / 1000);
+ nfp_fw_model, pf_dev->nfp_eth_table->count,
+ pf_dev->nfp_eth_table->ports[0].speed / 1000);
snprintf(fw_name, fw_size, "%s/%s", DEFAULT_FW_PATH, card_desc);
PMD_DRV_LOG(DEBUG, "Trying with fw file: %s", fw_name);
if (access(fw_name, F_OK) == 0)
@@ -1433,15 +1431,15 @@ nfp_fw_reload_from_flash(struct nfp_nsp *nsp)
static int
nfp_fw_reload_for_single_pf_from_disk(struct nfp_nsp *nsp,
char *fw_name,
- struct nfp_cpp *cpp,
- bool force_reload_fw,
+ struct nfp_pf_dev *pf_dev,
int reset)
{
int ret;
bool fw_changed = true;
- if (nfp_nsp_has_fw_loaded(nsp) && nfp_nsp_fw_loaded(nsp) && !force_reload_fw) {
- ret = nfp_fw_check_change(cpp, fw_name, &fw_changed);
+ if (nfp_nsp_has_fw_loaded(nsp) && nfp_nsp_fw_loaded(nsp) &&
+ !pf_dev->devargs.force_reload_fw) {
+ ret = nfp_fw_check_change(pf_dev->cpp, fw_name, &fw_changed);
if (ret != 0)
return ret;
}
@@ -1459,8 +1457,7 @@ nfp_fw_reload_for_single_pf_from_disk(struct nfp_nsp *nsp,
static int
nfp_fw_reload_for_single_pf(struct nfp_nsp *nsp,
char *fw_name,
- struct nfp_cpp *cpp,
- bool force_reload_fw,
+ struct nfp_pf_dev *pf_dev,
int reset,
int policy)
{
@@ -1473,8 +1470,7 @@ nfp_fw_reload_for_single_pf(struct nfp_nsp *nsp,
return ret;
}
} else if (fw_name[0] != 0) {
- ret = nfp_fw_reload_for_single_pf_from_disk(nsp, fw_name, cpp,
- force_reload_fw, reset);
+ ret = nfp_fw_reload_for_single_pf_from_disk(nsp, fw_name, pf_dev, reset);
if (ret != 0) {
PMD_DRV_LOG(ERR, "Load single PF firmware from disk failed.");
return ret;
@@ -1490,25 +1486,23 @@ nfp_fw_reload_for_single_pf(struct nfp_nsp *nsp,
static int
nfp_fw_reload_for_multi_pf_from_disk(struct nfp_nsp *nsp,
char *fw_name,
- struct nfp_cpp *cpp,
const struct nfp_dev_info *dev_info,
- struct nfp_multi_pf *multi_pf,
- bool force_reload_fw,
+ struct nfp_pf_dev *pf_dev,
int reset)
{
int err;
bool fw_changed = true;
bool skip_load_fw = false;
- bool reload_fw = force_reload_fw;
+ bool reload_fw = pf_dev->devargs.force_reload_fw;
if (nfp_nsp_has_fw_loaded(nsp) && nfp_nsp_fw_loaded(nsp) && !reload_fw) {
- err = nfp_fw_check_change(cpp, fw_name, &fw_changed);
+ err = nfp_fw_check_change(pf_dev->cpp, fw_name, &fw_changed);
if (err != 0)
return err;
}
if (!fw_changed || reload_fw)
- skip_load_fw = nfp_fw_skip_load(dev_info, multi_pf, &reload_fw);
+ skip_load_fw = nfp_fw_skip_load(dev_info, &pf_dev->multi_pf, &reload_fw);
if (skip_load_fw && !reload_fw)
return 0;
@@ -1523,16 +1517,17 @@ nfp_fw_reload_for_multi_pf_from_disk(struct nfp_nsp *nsp,
static int
nfp_fw_reload_for_multi_pf(struct nfp_nsp *nsp,
char *fw_name,
- struct nfp_cpp *cpp,
const struct nfp_dev_info *dev_info,
- struct nfp_multi_pf *multi_pf,
- bool force_reload_fw,
+ struct nfp_pf_dev *pf_dev,
int reset,
int policy)
{
int err;
+ struct nfp_multi_pf *multi_pf;
- err = nfp_net_keepalive_init(cpp, multi_pf);
+ multi_pf = &pf_dev->multi_pf;
+
+ err = nfp_net_keepalive_init(pf_dev->cpp, multi_pf);
if (err != 0) {
PMD_DRV_LOG(ERR, "NFP init beat failed");
return err;
@@ -1551,8 +1546,8 @@ nfp_fw_reload_for_multi_pf(struct nfp_nsp *nsp,
goto keepalive_stop;
}
} else if (fw_name[0] != 0) {
- err = nfp_fw_reload_for_multi_pf_from_disk(nsp, fw_name, cpp,
- dev_info, multi_pf, force_reload_fw, reset);
+ err = nfp_fw_reload_for_multi_pf_from_disk(nsp, fw_name, dev_info,
+ pf_dev, reset);
if (err != 0) {
PMD_DRV_LOG(ERR, "Load multi PF firmware from disk failed.");
goto keepalive_stop;
@@ -1627,13 +1622,8 @@ nfp_fw_policy_value_get(struct nfp_nsp *nsp,
}
static int
-nfp_fw_setup(struct rte_pci_device *dev,
- struct nfp_cpp *cpp,
- struct nfp_eth_table *nfp_eth_table,
- struct nfp_hwinfo *hwinfo,
- const struct nfp_dev_info *dev_info,
- struct nfp_multi_pf *multi_pf,
- bool force_reload_fw)
+nfp_fw_setup(struct nfp_pf_dev *pf_dev,
+ const struct nfp_dev_info *dev_info)
{
int err;
int reset;
@@ -1641,7 +1631,7 @@ nfp_fw_setup(struct rte_pci_device *dev,
char fw_name[125];
struct nfp_nsp *nsp;
- nsp = nfp_nsp_open(cpp);
+ nsp = nfp_nsp_open(pf_dev->cpp);
if (nsp == NULL) {
PMD_DRV_LOG(ERR, "NFP error when obtaining NSP handle");
return -EIO;
@@ -1665,20 +1655,19 @@ nfp_fw_setup(struct rte_pci_device *dev,
fw_name[0] = 0;
if (policy != NFP_NSP_APP_FW_LOAD_FLASH) {
- err = nfp_fw_get_name(dev, cpp, nfp_eth_table, hwinfo, fw_name,
- sizeof(fw_name));
+ err = nfp_fw_get_name(pf_dev, fw_name, sizeof(fw_name));
if (err != 0) {
PMD_DRV_LOG(ERR, "Can't find suitable firmware.");
goto close_nsp;
}
}
- if (multi_pf->enabled)
- err = nfp_fw_reload_for_multi_pf(nsp, fw_name, cpp, dev_info,
- multi_pf, force_reload_fw, reset, policy);
+ if (pf_dev->multi_pf.enabled)
+ err = nfp_fw_reload_for_multi_pf(nsp, fw_name, dev_info,
+ pf_dev, reset, policy);
else
- err = nfp_fw_reload_for_single_pf(nsp, fw_name, cpp,
- force_reload_fw, reset, policy);
+ err = nfp_fw_reload_for_single_pf(nsp, fw_name, pf_dev,
+ reset, policy);
close_nsp:
nfp_nsp_close(nsp);
@@ -2063,7 +2052,7 @@ nfp_net_speed_cap_get(struct nfp_pf_dev *pf_dev)
uint32_t id;
uint32_t count;
- count = nfp_net_get_port_num(pf_dev, pf_dev->nfp_eth_table);
+ count = pf_dev->total_phyports;
for (i = 0; i < count; i++) {
id = nfp_function_id_get(pf_dev, i);
ret = nfp_net_speed_cap_get_one(pf_dev, id);
@@ -2078,9 +2067,7 @@ nfp_net_speed_cap_get(struct nfp_pf_dev *pf_dev)
/* Force the physical port down to clear the possible DMA error */
static int
-nfp_net_force_port_down(struct nfp_pf_dev *pf_dev,
- struct nfp_eth_table *nfp_eth_table,
- struct nfp_cpp *cpp)
+nfp_net_force_port_down(struct nfp_pf_dev *pf_dev)
{
int ret;
uint32_t i;
@@ -2088,11 +2075,11 @@ nfp_net_force_port_down(struct nfp_pf_dev *pf_dev,
uint32_t index;
uint32_t count;
- count = nfp_net_get_port_num(pf_dev, nfp_eth_table);
+ count = pf_dev->total_phyports;
for (i = 0; i < count; i++) {
id = nfp_function_id_get(pf_dev, i);
- index = nfp_eth_table->ports[id].index;
- ret = nfp_eth_set_configured(cpp, index, 0);
+ index = pf_dev->nfp_eth_table->ports[id].index;
+ ret = nfp_eth_set_configured(pf_dev->cpp, index, 0);
if (ret < 0)
return ret;
}
@@ -2322,6 +2309,9 @@ nfp_pf_init(struct rte_pci_device *pci_dev)
goto hw_priv_free;
}
+ hw_priv->dev_info = dev_info;
+ hw_priv->pf_dev = pf_dev;
+
sync = nfp_sync_alloc();
if (sync == NULL) {
PMD_INIT_LOG(ERR, "Failed to alloc sync zone.");
@@ -2329,6 +2319,8 @@ nfp_pf_init(struct rte_pci_device *pci_dev)
goto pf_cleanup;
}
+ pf_dev->sync = sync;
+
/*
* When device bound to UIO, the device could be used, by mistake,
* by two DPDK apps, and the UIO driver does not avoid it. This
@@ -2347,6 +2339,9 @@ nfp_pf_init(struct rte_pci_device *pci_dev)
goto sync_free;
}
+ pf_dev->cpp = cpp;
+ pf_dev->pci_dev = pci_dev;
+
hwinfo = nfp_hwinfo_read(cpp);
if (hwinfo == NULL) {
PMD_INIT_LOG(ERR, "Error reading hwinfo table");
@@ -2354,6 +2349,8 @@ nfp_pf_init(struct rte_pci_device *pci_dev)
goto cpp_cleanup;
}
+ pf_dev->hwinfo = hwinfo;
+
/* Read the number of physical ports from hardware */
nfp_eth_table = nfp_eth_read_ports(cpp);
if (nfp_eth_table == NULL) {
@@ -2369,10 +2366,12 @@ nfp_pf_init(struct rte_pci_device *pci_dev)
goto eth_table_cleanup;
}
+ pf_dev->nfp_eth_table = nfp_eth_table;
pf_dev->multi_pf.enabled = nfp_check_multi_pf_from_nsp(pci_dev, cpp);
pf_dev->multi_pf.function_id = function_id;
+ pf_dev->total_phyports = nfp_net_get_port_num(pf_dev);
- ret = nfp_net_force_port_down(pf_dev, nfp_eth_table, cpp);
+ ret = nfp_net_force_port_down(pf_dev);
if (ret != 0) {
PMD_INIT_LOG(ERR, "Failed to force port down");
ret = -EIO;
@@ -2386,15 +2385,15 @@ nfp_pf_init(struct rte_pci_device *pci_dev)
goto eth_table_cleanup;
}
- ret = nfp_net_device_activate(cpp, &pf_dev->multi_pf);
+ ret = nfp_net_device_activate(pf_dev);
if (ret != 0) {
PMD_INIT_LOG(ERR, "Failed to activate the NFP device");
ret = -EIO;
goto eth_table_cleanup;
}
- if (nfp_fw_setup(pci_dev, cpp, nfp_eth_table, hwinfo,
- dev_info, &pf_dev->multi_pf, pf_dev->devargs.force_reload_fw) != 0) {
+ ret = nfp_fw_setup(pf_dev, dev_info);
+ if (ret != 0) {
PMD_INIT_LOG(ERR, "Error when uploading firmware");
ret = -EIO;
goto eth_table_cleanup;
@@ -2408,6 +2407,8 @@ nfp_pf_init(struct rte_pci_device *pci_dev)
goto fw_cleanup;
}
+ pf_dev->sym_tbl = sym_tbl;
+
/* Read the app ID of the firmware loaded */
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);
@@ -2417,6 +2418,8 @@ nfp_pf_init(struct rte_pci_device *pci_dev)
goto sym_tbl_cleanup;
}
+ pf_dev->app_fw_id = app_fw_id;
+
/* Write sp_indiff to hw_info */
ret = nfp_net_hwinfo_set(function_id, sym_tbl, cpp, app_fw_id);
if (ret != 0) {
@@ -2425,17 +2428,6 @@ nfp_pf_init(struct rte_pci_device *pci_dev)
goto sym_tbl_cleanup;
}
- /* Populate the newly created PF device */
- pf_dev->app_fw_id = app_fw_id;
- pf_dev->cpp = cpp;
- pf_dev->hwinfo = hwinfo;
- pf_dev->sym_tbl = sym_tbl;
- pf_dev->pci_dev = pci_dev;
- pf_dev->nfp_eth_table = nfp_eth_table;
- pf_dev->sync = sync;
- pf_dev->total_phyports = nfp_net_get_port_num(pf_dev, nfp_eth_table);
- pf_dev->speed_updated = false;
-
ret = nfp_net_speed_cap_get(pf_dev);
if (ret != 0) {
PMD_INIT_LOG(ERR, "Failed to get speed capability.");
@@ -2484,8 +2476,6 @@ nfp_pf_init(struct rte_pci_device *pci_dev)
}
hw_priv->is_pf = true;
- hw_priv->pf_dev = pf_dev;
- hw_priv->dev_info = dev_info;
/*
* PF initialization has been done at this point. Call app specific
@@ -2678,6 +2668,9 @@ nfp_pf_secondary_init(struct rte_pci_device *pci_dev)
goto hw_priv_free;
}
+ hw_priv->pf_dev = pf_dev;
+ hw_priv->dev_info = dev_info;
+
sync = nfp_sync_alloc();
if (sync == NULL) {
PMD_INIT_LOG(ERR, "Failed to alloc sync zone.");
@@ -2685,6 +2678,8 @@ nfp_pf_secondary_init(struct rte_pci_device *pci_dev)
goto pf_cleanup;
}
+ pf_dev->sync = sync;
+
/*
* When device bound to UIO, the device could be used, by mistake,
* by two DPDK apps, and the UIO driver does not avoid it. This
@@ -2703,6 +2698,9 @@ nfp_pf_secondary_init(struct rte_pci_device *pci_dev)
goto sync_free;
}
+ pf_dev->cpp = cpp;
+ pf_dev->pci_dev = pci_dev;
+
/*
* We don't have access to the PF created in the primary process
* here so we have to read the number of ports from firmware.
@@ -2714,6 +2712,8 @@ nfp_pf_secondary_init(struct rte_pci_device *pci_dev)
goto cpp_cleanup;
}
+ pf_dev->sym_tbl = sym_tbl;
+
/* Read the app ID of the firmware loaded */
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);
@@ -2723,16 +2723,9 @@ nfp_pf_secondary_init(struct rte_pci_device *pci_dev)
goto sym_tbl_cleanup;
}
- /* Populate the newly created PF device */
pf_dev->app_fw_id = app_fw_id;
- pf_dev->cpp = cpp;
- pf_dev->sym_tbl = sym_tbl;
- pf_dev->pci_dev = pci_dev;
- pf_dev->sync = sync;
hw_priv->is_pf = true;
- hw_priv->pf_dev = pf_dev;
- hw_priv->dev_info = dev_info;
/* Call app specific init code now */
ret = nfp_fw_app_secondary_init(hw_priv);
diff --git a/drivers/net/nfp/nfp_net_common.c b/drivers/net/nfp/nfp_net_common.c
index c9a95ed632..99f6b61947 100644
--- a/drivers/net/nfp/nfp_net_common.c
+++ b/drivers/net/nfp/nfp_net_common.c
@@ -2724,13 +2724,12 @@ nfp_net_fec_set(struct rte_eth_dev *dev,
}
uint32_t
-nfp_net_get_port_num(struct nfp_pf_dev *pf_dev,
- struct nfp_eth_table *nfp_eth_table)
+nfp_net_get_port_num(struct nfp_pf_dev *pf_dev)
{
if (pf_dev->multi_pf.enabled)
return 1;
else
- return nfp_eth_table->count;
+ return pf_dev->nfp_eth_table->count;
}
uint8_t
diff --git a/drivers/net/nfp/nfp_net_common.h b/drivers/net/nfp/nfp_net_common.h
index 0b5bba2a3e..fb244383b7 100644
--- a/drivers/net/nfp/nfp_net_common.h
+++ b/drivers/net/nfp/nfp_net_common.h
@@ -374,8 +374,7 @@ void nfp_net_get_fw_version(struct nfp_cpp *cpp,
uint32_t *fw_version);
int nfp_net_txrwb_alloc(struct rte_eth_dev *eth_dev);
void nfp_net_txrwb_free(struct rte_eth_dev *eth_dev);
-uint32_t nfp_net_get_port_num(struct nfp_pf_dev *pf_dev,
- struct nfp_eth_table *nfp_eth_table);
+uint32_t nfp_net_get_port_num(struct nfp_pf_dev *pf_dev);
uint8_t nfp_function_id_get(const struct nfp_pf_dev *pf_dev,
uint8_t port_id);
int nfp_net_vf_config_app_init(struct nfp_net_hw *net_hw,
--
2.39.1
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v2 04/10] net/nfp: improve the logic readability
2024-10-12 2:40 ` [PATCH v2 00/10] modify some logic of NFP PMD Chaoyong He
` (2 preceding siblings ...)
2024-10-12 2:41 ` [PATCH v2 03/10] net/nfp: simplify some function parameters Chaoyong He
@ 2024-10-12 2:41 ` Chaoyong He
2024-10-12 2:41 ` [PATCH v2 05/10] net/nfp: fix problem caused by configure function Chaoyong He
` (6 subsequent siblings)
10 siblings, 0 replies; 38+ messages in thread
From: Chaoyong He @ 2024-10-12 2:41 UTC (permalink / raw)
To: dev; +Cc: oss-drivers, Chaoyong He, Long Wu, Peng Zhang
Try our best to make the logic in secondary proess the same with the
primary process and improve the readability, by add helper functions
and rename function.
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 | 86 +++++++++++++++-----------------
drivers/net/nfp/nfp_net_common.c | 22 +++++++-
drivers/net/nfp/nfp_net_common.h | 3 +-
3 files changed, 63 insertions(+), 48 deletions(-)
diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index 121f82af6d..405386e882 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -1764,19 +1764,9 @@ nfp_enable_multi_pf(struct nfp_pf_dev *pf_dev)
static bool
nfp_app_fw_nic_total_phyports_check(struct nfp_pf_dev *pf_dev)
{
- int ret;
- uint8_t id;
uint8_t total_phyports;
- char vnic_name[RTE_ETH_NAME_MAX_LEN];
- /* Read the number of vNIC's created for the PF */
- id = nfp_function_id_get(pf_dev, 0);
- snprintf(vnic_name, sizeof(vnic_name), "nfd_cfg_pf%u_num_ports", id);
- total_phyports = nfp_rtsym_read_le(pf_dev->sym_tbl, vnic_name, &ret);
- if (ret != 0 || total_phyports == 0 || total_phyports > 8) {
- PMD_INIT_LOG(ERR, "%s symbol with wrong value", vnic_name);
- return false;
- }
+ total_phyports = nfp_net_get_phyports_from_fw(pf_dev);
if (pf_dev->multi_pf.enabled) {
if (!nfp_check_multi_pf_from_fw(total_phyports)) {
@@ -1797,6 +1787,20 @@ nfp_app_fw_nic_total_phyports_check(struct nfp_pf_dev *pf_dev)
return true;
}
+static void
+nfp_port_name_generate(char *port_name,
+ size_t length,
+ int port_id,
+ struct nfp_pf_dev *pf_dev)
+{
+ const char *name = pf_dev->pci_dev->device.name;
+
+ if (pf_dev->multi_pf.enabled)
+ snprintf(port_name, length, "%s", name);
+ else
+ snprintf(port_name, length, "%s_port%u", name, port_id);
+}
+
static int
nfp_init_app_fw_nic(struct nfp_net_hw_priv *hw_priv)
{
@@ -1849,12 +1853,7 @@ nfp_init_app_fw_nic(struct nfp_net_hw_priv *hw_priv)
/* Loop through all physical ports on PF */
for (i = 0; i < pf_dev->total_phyports; i++) {
- if (pf_dev->multi_pf.enabled)
- snprintf(port_name, sizeof(port_name), "%s",
- pf_dev->pci_dev->device.name);
- else
- snprintf(port_name, sizeof(port_name), "%s_port%u",
- pf_dev->pci_dev->device.name, i);
+ nfp_port_name_generate(port_name, sizeof(port_name), i, pf_dev);
id = nfp_function_id_get(pf_dev, i);
hw_init.idx = id;
@@ -1870,15 +1869,10 @@ nfp_init_app_fw_nic(struct nfp_net_hw_priv *hw_priv)
return 0;
port_cleanup:
- for (i = 0; i < pf_dev->total_phyports; i++) {
+ for (uint32_t j = 0; j < i; j++) {
struct rte_eth_dev *eth_dev;
- if (pf_dev->multi_pf.enabled)
- snprintf(port_name, sizeof(port_name), "%s",
- pf_dev->pci_dev->device.name);
- else
- snprintf(port_name, sizeof(port_name), "%s_port%u",
- pf_dev->pci_dev->device.name, i);
+ nfp_port_name_generate(port_name, sizeof(port_name), j, pf_dev);
eth_dev = rte_eth_dev_get_by_name(port_name);
if (eth_dev != NULL)
rte_eth_dev_destroy(eth_dev, nfp_net_uninit);
@@ -2369,7 +2363,7 @@ nfp_pf_init(struct rte_pci_device *pci_dev)
pf_dev->nfp_eth_table = nfp_eth_table;
pf_dev->multi_pf.enabled = nfp_check_multi_pf_from_nsp(pci_dev, cpp);
pf_dev->multi_pf.function_id = function_id;
- pf_dev->total_phyports = nfp_net_get_port_num(pf_dev);
+ pf_dev->total_phyports = nfp_net_get_phyports_from_nsp(pf_dev);
ret = nfp_net_force_port_down(pf_dev);
if (ret != 0) {
@@ -2547,42 +2541,37 @@ static int
nfp_secondary_init_app_fw_nic(struct nfp_net_hw_priv *hw_priv)
{
uint32_t i;
- int err = 0;
int ret = 0;
- uint8_t function_id;
uint32_t total_vnics;
- char pf_name[RTE_ETH_NAME_MAX_LEN];
+ char port_name[RTE_ETH_NAME_MAX_LEN];
struct nfp_pf_dev *pf_dev = hw_priv->pf_dev;
- /* Read the number of vNIC's created for the PF */
- function_id = (pf_dev->pci_dev->addr.function) & 0x07;
- snprintf(pf_name, sizeof(pf_name), "nfd_cfg_pf%u_num_ports", function_id);
- total_vnics = nfp_rtsym_read_le(pf_dev->sym_tbl, pf_name, &err);
- if (err != 0 || total_vnics == 0 || total_vnics > 8) {
- PMD_INIT_LOG(ERR, "%s symbol with wrong value", pf_name);
- return -ENODEV;
- }
+ total_vnics = nfp_net_get_phyports_from_fw(pf_dev);
for (i = 0; i < total_vnics; i++) {
- char port_name[RTE_ETH_NAME_MAX_LEN];
-
- if (nfp_check_multi_pf_from_fw(total_vnics))
- snprintf(port_name, sizeof(port_name), "%s",
- pf_dev->pci_dev->device.name);
- else
- snprintf(port_name, sizeof(port_name), "%s_port%u",
- pf_dev->pci_dev->device.name, i);
+ nfp_port_name_generate(port_name, sizeof(port_name), i, pf_dev);
PMD_INIT_LOG(DEBUG, "Secondary attaching to port %s", port_name);
ret = rte_eth_dev_create(&pf_dev->pci_dev->device, port_name, 0,
NULL, NULL, nfp_secondary_net_init, hw_priv);
if (ret != 0) {
PMD_INIT_LOG(ERR, "Secondary process attach to port %s failed", port_name);
- ret = -ENODEV;
- break;
+ goto port_cleanup;
}
}
+ return 0;
+
+port_cleanup:
+ for (uint32_t j = 0; j < i; j++) {
+ struct rte_eth_dev *eth_dev;
+
+ nfp_port_name_generate(port_name, sizeof(port_name), j, pf_dev);
+ eth_dev = rte_eth_dev_get_by_name(port_name);
+ if (eth_dev != NULL)
+ rte_eth_dev_destroy(eth_dev, NULL);
+ }
+
return ret;
}
@@ -2714,6 +2703,11 @@ nfp_pf_secondary_init(struct rte_pci_device *pci_dev)
pf_dev->sym_tbl = sym_tbl;
+ /* Read the number of physical ports from firmware */
+ pf_dev->multi_pf.function_id = function_id;
+ pf_dev->total_phyports = nfp_net_get_phyports_from_fw(pf_dev);
+ pf_dev->multi_pf.enabled = nfp_check_multi_pf_from_fw(pf_dev->total_phyports);
+
/* Read the app ID of the firmware loaded */
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);
diff --git a/drivers/net/nfp/nfp_net_common.c b/drivers/net/nfp/nfp_net_common.c
index 99f6b61947..86a1fbfaf2 100644
--- a/drivers/net/nfp/nfp_net_common.c
+++ b/drivers/net/nfp/nfp_net_common.c
@@ -14,6 +14,7 @@
#include "nfdk/nfp_nfdk.h"
#include "nfpcore/nfp_mip.h"
#include "nfpcore/nfp_nsp.h"
+#include "nfpcore/nfp_rtsym.h"
#include "nfp_logs.h"
#include "nfp_net_meta.h"
@@ -2724,7 +2725,7 @@ nfp_net_fec_set(struct rte_eth_dev *dev,
}
uint32_t
-nfp_net_get_port_num(struct nfp_pf_dev *pf_dev)
+nfp_net_get_phyports_from_nsp(struct nfp_pf_dev *pf_dev)
{
if (pf_dev->multi_pf.enabled)
return 1;
@@ -2732,6 +2733,25 @@ nfp_net_get_port_num(struct nfp_pf_dev *pf_dev)
return pf_dev->nfp_eth_table->count;
}
+uint32_t
+nfp_net_get_phyports_from_fw(struct nfp_pf_dev *pf_dev)
+{
+ int ret = 0;
+ uint8_t total_phyports;
+ char pf_name[RTE_ETH_NAME_MAX_LEN];
+
+ /* Read the number of vNIC's created for the PF */
+ snprintf(pf_name, sizeof(pf_name), "nfd_cfg_pf%u_num_ports",
+ pf_dev->multi_pf.function_id);
+ total_phyports = nfp_rtsym_read_le(pf_dev->sym_tbl, pf_name, &ret);
+ if (ret != 0 || total_phyports == 0 || total_phyports > 8) {
+ PMD_INIT_LOG(ERR, "%s symbol with wrong value", pf_name);
+ return 0;
+ }
+
+ return total_phyports;
+}
+
uint8_t
nfp_function_id_get(const struct nfp_pf_dev *pf_dev,
uint8_t port_id)
diff --git a/drivers/net/nfp/nfp_net_common.h b/drivers/net/nfp/nfp_net_common.h
index fb244383b7..8429db68f0 100644
--- a/drivers/net/nfp/nfp_net_common.h
+++ b/drivers/net/nfp/nfp_net_common.h
@@ -374,7 +374,8 @@ void nfp_net_get_fw_version(struct nfp_cpp *cpp,
uint32_t *fw_version);
int nfp_net_txrwb_alloc(struct rte_eth_dev *eth_dev);
void nfp_net_txrwb_free(struct rte_eth_dev *eth_dev);
-uint32_t nfp_net_get_port_num(struct nfp_pf_dev *pf_dev);
+uint32_t nfp_net_get_phyports_from_nsp(struct nfp_pf_dev *pf_dev);
+uint32_t nfp_net_get_phyports_from_fw(struct nfp_pf_dev *pf_dev);
uint8_t nfp_function_id_get(const struct nfp_pf_dev *pf_dev,
uint8_t port_id);
int nfp_net_vf_config_app_init(struct nfp_net_hw *net_hw,
--
2.39.1
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v2 05/10] net/nfp: fix problem caused by configure function
2024-10-12 2:40 ` [PATCH v2 00/10] modify some logic of NFP PMD Chaoyong He
` (3 preceding siblings ...)
2024-10-12 2:41 ` [PATCH v2 04/10] net/nfp: improve the logic readability Chaoyong He
@ 2024-10-12 2:41 ` Chaoyong He
2024-10-12 2:41 ` [PATCH v2 06/10] net/nfp: add check logic for port up/down function Chaoyong He
` (5 subsequent siblings)
10 siblings, 0 replies; 38+ messages in thread
From: Chaoyong He @ 2024-10-12 2:41 UTC (permalink / raw)
To: dev
Cc: oss-drivers, Chaoyong He, stable, Long Wu, Peng Zhang, Stephen Hemminger
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>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v2 06/10] net/nfp: add check logic for port up/down function
2024-10-12 2:40 ` [PATCH v2 00/10] modify some logic of NFP PMD Chaoyong He
` (4 preceding siblings ...)
2024-10-12 2:41 ` [PATCH v2 05/10] net/nfp: fix problem caused by configure function Chaoyong He
@ 2024-10-12 2:41 ` Chaoyong He
2024-10-12 2:41 ` [PATCH v2 07/10] net/nfp: fix problem caused by commit end function Chaoyong He
` (4 subsequent siblings)
10 siblings, 0 replies; 38+ messages in thread
From: Chaoyong He @ 2024-10-12 2:41 UTC (permalink / raw)
To: dev; +Cc: oss-drivers, Chaoyong He, Long Wu, Peng Zhang, Stephen Hemminger
The 'nfp_eth_set_configured()' function is not always success, so
need to check the return value of it.
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>
---
.../net/nfp/flower/nfp_flower_representor.c | 18 +++++++++++++-----
drivers/net/nfp/nfp_ethdev.c | 4 +++-
drivers/net/nfp/nfp_net_common.c | 5 ++++-
3 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/drivers/net/nfp/flower/nfp_flower_representor.c b/drivers/net/nfp/flower/nfp_flower_representor.c
index d1558b905c..eb0a02874b 100644
--- a/drivers/net/nfp/flower/nfp_flower_representor.c
+++ b/drivers/net/nfp/flower/nfp_flower_representor.c
@@ -83,6 +83,7 @@ nfp_flower_repr_dev_infos_get(__rte_unused struct rte_eth_dev *dev,
static int
nfp_flower_repr_dev_start(struct rte_eth_dev *dev)
{
+ int ret;
uint16_t i;
struct nfp_net_hw_priv *hw_priv;
struct nfp_flower_representor *repr;
@@ -92,8 +93,11 @@ nfp_flower_repr_dev_start(struct rte_eth_dev *dev)
hw_priv = dev->process_private;
app_fw_flower = repr->app_fw_flower;
- if (repr->repr_type == NFP_REPR_TYPE_PHYS_PORT)
- nfp_eth_set_configured(hw_priv->pf_dev->cpp, repr->nfp_idx, 1);
+ if (repr->repr_type == NFP_REPR_TYPE_PHYS_PORT) {
+ ret = nfp_eth_set_configured(hw_priv->pf_dev->cpp, repr->nfp_idx, 1);
+ if (ret < 0)
+ return ret;
+ }
nfp_flower_cmsg_port_mod(app_fw_flower, repr->port_id, true);
@@ -109,6 +113,7 @@ static int
nfp_flower_repr_dev_stop(struct rte_eth_dev *dev)
{
uint16_t i;
+ int ret = 0;
struct nfp_net_hw_priv *hw_priv;
struct nfp_flower_representor *repr;
struct nfp_app_fw_flower *app_fw_flower;
@@ -119,15 +124,18 @@ nfp_flower_repr_dev_stop(struct rte_eth_dev *dev)
nfp_flower_cmsg_port_mod(app_fw_flower, repr->port_id, false);
- if (repr->repr_type == NFP_REPR_TYPE_PHYS_PORT)
- nfp_eth_set_configured(hw_priv->pf_dev->cpp, repr->nfp_idx, 0);
+ if (repr->repr_type == NFP_REPR_TYPE_PHYS_PORT) {
+ ret = nfp_eth_set_configured(hw_priv->pf_dev->cpp, repr->nfp_idx, 0);
+ if (ret == 1)
+ ret = 0;
+ }
for (i = 0; i < dev->data->nb_rx_queues; i++)
dev->data->rx_queue_state[i] = RTE_ETH_QUEUE_STATE_STOPPED;
for (i = 0; i < dev->data->nb_tx_queues; i++)
dev->data->tx_queue_state[i] = RTE_ETH_QUEUE_STATE_STOPPED;
- return 0;
+ return ret;
}
static int
diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index 2fe6b1a292..302149e9dc 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -495,7 +495,9 @@ nfp_net_start(struct rte_eth_dev *dev)
}
/* Configure the physical port up */
- nfp_eth_set_configured(pf_dev->cpp, net_hw->nfp_idx, 1);
+ ret = nfp_eth_set_configured(pf_dev->cpp, net_hw->nfp_idx, 1);
+ if (ret < 0)
+ goto error;
for (i = 0; i < dev->data->nb_rx_queues; i++)
dev->data->rx_queue_state[i] = RTE_ETH_QUEUE_STATE_STARTED;
diff --git a/drivers/net/nfp/nfp_net_common.c b/drivers/net/nfp/nfp_net_common.c
index 86a1fbfaf2..80d60515d8 100644
--- a/drivers/net/nfp/nfp_net_common.c
+++ b/drivers/net/nfp/nfp_net_common.c
@@ -2427,6 +2427,7 @@ nfp_net_ctrl_bar_size_set(struct nfp_pf_dev *pf_dev)
int
nfp_net_stop(struct rte_eth_dev *dev)
{
+ int ret;
struct nfp_net_hw *hw;
struct nfp_net_hw_priv *hw_priv;
@@ -2439,7 +2440,9 @@ nfp_net_stop(struct rte_eth_dev *dev)
nfp_net_stop_tx_queue(dev);
nfp_net_stop_rx_queue(dev);
- 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;
}
--
2.39.1
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v2 07/10] net/nfp: fix problem caused by commit end function
2024-10-12 2:40 ` [PATCH v2 00/10] modify some logic of NFP PMD Chaoyong He
` (5 preceding siblings ...)
2024-10-12 2:41 ` [PATCH v2 06/10] net/nfp: add check logic for port up/down function Chaoyong He
@ 2024-10-12 2:41 ` Chaoyong He
2024-10-12 2:41 ` [PATCH v2 08/10] net/nfp: fix problem caused by FEC set Chaoyong He
` (3 subsequent siblings)
10 siblings, 0 replies; 38+ messages in thread
From: Chaoyong He @ 2024-10-12 2:41 UTC (permalink / raw)
To: dev
Cc: oss-drivers, Chaoyong He, zerun.fu, stable, Long Wu, Peng Zhang,
Stephen Hemminger
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>
---
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v2 08/10] net/nfp: fix problem caused by FEC set
2024-10-12 2:40 ` [PATCH v2 00/10] modify some logic of NFP PMD Chaoyong He
` (6 preceding siblings ...)
2024-10-12 2:41 ` [PATCH v2 07/10] net/nfp: fix problem caused by commit end function Chaoyong He
@ 2024-10-12 2:41 ` Chaoyong He
2024-10-12 2:41 ` [PATCH v2 09/10] net/nfp: modify the comment of some control messages Chaoyong He
` (2 subsequent siblings)
10 siblings, 0 replies; 38+ messages in thread
From: Chaoyong He @ 2024-10-12 2:41 UTC (permalink / raw)
To: dev
Cc: oss-drivers, Chaoyong He, zerun.fu, stable, Long Wu, Peng Zhang,
Stephen Hemminger
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>
---
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v2 09/10] net/nfp: modify the comment of some control messages
2024-10-12 2:40 ` [PATCH v2 00/10] modify some logic of NFP PMD Chaoyong He
` (7 preceding siblings ...)
2024-10-12 2:41 ` [PATCH v2 08/10] net/nfp: fix problem caused by FEC set Chaoyong He
@ 2024-10-12 2:41 ` Chaoyong He
2024-10-12 2:41 ` [PATCH v2 10/10] net/nfp: fix memory leak in VF initialization logic Chaoyong He
2024-10-13 2:49 ` [PATCH v2 00/10] modify some logic of NFP PMD Ferruh Yigit
10 siblings, 0 replies; 38+ messages in thread
From: Chaoyong He @ 2024-10-12 2:41 UTC (permalink / raw)
To: dev; +Cc: oss-drivers, Chaoyong He, Long Wu, Stephen Hemminger
The comment of some control messages are not right, which conflict
with the data structure and may confuse the other developers.
Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
drivers/net/nfp/flower/nfp_flower_cmsg.h | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/net/nfp/flower/nfp_flower_cmsg.h b/drivers/net/nfp/flower/nfp_flower_cmsg.h
index 5fc4210d8b..eda047a404 100644
--- a/drivers/net/nfp/flower/nfp_flower_cmsg.h
+++ b/drivers/net/nfp/flower/nfp_flower_cmsg.h
@@ -738,7 +738,7 @@ struct nfp_fl_act_output {
* 3 2 1
* 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
- * | - |opcode | - |jump_id| - |
+ * | res | opcode | res | len_lw| - |
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
* | eth_dst_47_16_mask |
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
@@ -777,7 +777,7 @@ struct nfp_fl_act_push_vlan {
* 3 2 1
* 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
- * | - |opcode | |jump_id| - |
+ * | res | opcode | res | len_lw| - |
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
* | ipv4_saddr_mask |
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
@@ -802,7 +802,7 @@ struct nfp_fl_act_set_ip4_addrs {
* 3 2 1
* 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
- * | - |opcode | |jump_id| ttl_mask | tos_mask |
+ * | res | opcode | res | len_lw| ttl_mask | tos_mask |
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
* | ttl | tos | 0 |
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
@@ -821,7 +821,7 @@ struct nfp_fl_act_set_ip4_ttl_tos {
* 3 2 1
* 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
- * | - |opcode | |jump_id| - |
+ * | res | opcode | res | len_lw| - |
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
* | ipv6_addr_127_96_mask |
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
@@ -854,7 +854,7 @@ struct nfp_fl_act_set_ipv6_addr {
* 3 2 1
* 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
- * | - |opcode | |jump_id| tclass_mask | hlimit_mask |
+ * | res | opcode | res | len_lw| tclass_mask | hlimit_mask |
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
* | 0 | tclass | hlimit |
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
@@ -879,7 +879,7 @@ struct nfp_fl_act_set_ipv6_tc_hl_fl {
* 3 2 1
* 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
- * | - |opcode | |jump_id| - |
+ * | res | opcode | res | len_lw| - |
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
* | src_mask | dst_mask |
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
@@ -900,7 +900,7 @@ struct nfp_fl_act_set_tport {
* 3 2 1
* 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
- * | - | opcode | |jump_id| - |M| - |V|
+ * | res | opcode | res | len_lw| - |M| - |V|
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
* | ipv6_daddr_127_96 / ipv4_daddr |
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
--
2.39.1
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v2 10/10] net/nfp: fix memory leak in VF initialization logic
2024-10-12 2:40 ` [PATCH v2 00/10] modify some logic of NFP PMD Chaoyong He
` (8 preceding siblings ...)
2024-10-12 2:41 ` [PATCH v2 09/10] net/nfp: modify the comment of some control messages Chaoyong He
@ 2024-10-12 2:41 ` Chaoyong He
2024-10-13 2:49 ` [PATCH v2 00/10] modify some logic of NFP PMD Ferruh Yigit
10 siblings, 0 replies; 38+ messages in thread
From: Chaoyong He @ 2024-10-12 2:41 UTC (permalink / raw)
To: dev
Cc: oss-drivers, Chaoyong He, stable, Long Wu, Peng Zhang, Stephen Hemminger
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>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
drivers/net/nfp/nfp_ethdev_vf.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/nfp/nfp_ethdev_vf.c b/drivers/net/nfp/nfp_ethdev_vf.c
index ab413a2c5a..0aadca9010 100644
--- a/drivers/net/nfp/nfp_ethdev_vf.c
+++ b/drivers/net/nfp/nfp_ethdev_vf.c
@@ -322,12 +322,13 @@ nfp_netvf_init(struct rte_eth_dev *eth_dev)
if (rte_eal_process_type() != RTE_PROC_PRIMARY)
return 0;
- net_hw->eth_xstats_base = rte_malloc("rte_eth_xstat",
- sizeof(struct rte_eth_xstat) * nfp_net_xstats_size(eth_dev), 0);
+ net_hw->eth_xstats_base = rte_calloc("rte_eth_xstat",
+ nfp_net_xstats_size(eth_dev), sizeof(struct rte_eth_xstat), 0);
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 02/10] net/nfp: fix malloc name problem in secondary process
2024-10-12 2:40 ` [PATCH v2 02/10] net/nfp: fix malloc name problem in secondary process Chaoyong He
@ 2024-10-12 2:45 ` Stephen Hemminger
2024-10-12 2:47 ` Chaoyong He
0 siblings, 1 reply; 38+ messages in thread
From: Stephen Hemminger @ 2024-10-12 2:45 UTC (permalink / raw)
To: Chaoyong He; +Cc: dev, oss-drivers, peng.zhang, stable, Long Wu
On Sat, 12 Oct 2024 10:40:59 +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>
How, the name is ignore mostly by malloc. Only used for tracing.
^ permalink raw reply [flat|nested] 38+ messages in thread
* RE: [PATCH v2 02/10] net/nfp: fix malloc name problem in secondary process
2024-10-12 2:45 ` Stephen Hemminger
@ 2024-10-12 2:47 ` Chaoyong He
0 siblings, 0 replies; 38+ messages in thread
From: Chaoyong He @ 2024-10-12 2:47 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev, oss-drivers, Nole Zhang, stable, Long Wu
> On Sat, 12 Oct 2024 10:40:59 +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>
>
> How, the name is ignore mostly by malloc. Only used for tracing.
Okay, If you insist, we can abandon this one.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 00/10] modify some logic of NFP PMD
2024-10-12 2:40 ` [PATCH v2 00/10] modify some logic of NFP PMD Chaoyong He
` (9 preceding siblings ...)
2024-10-12 2:41 ` [PATCH v2 10/10] net/nfp: fix memory leak in VF initialization logic Chaoyong He
@ 2024-10-13 2:49 ` Ferruh Yigit
10 siblings, 0 replies; 38+ messages in thread
From: Ferruh Yigit @ 2024-10-13 2:49 UTC (permalink / raw)
To: Chaoyong He, dev; +Cc: oss-drivers
On 10/12/2024 3:40 AM, Chaoyong He wrote:
> This patch series refactor some logic of NFP PMD to make it more
> clear, also fix some bugs.
>
> ---
> v2:
> * Following the reviewer's request, change a little logic of patch 10/10.
> * Add 'Acked-by' tag from reviewer.
> ---
>
> Chaoyong He (10):
> net/nfp: use strlcpy for copying string
> net/nfp: fix malloc name problem in secondary process
> net/nfp: simplify some function parameters
> net/nfp: improve the logic readability
> net/nfp: fix problem caused by configure function
> net/nfp: add check logic for port up/down function
> net/nfp: fix problem caused by commit end function
> net/nfp: fix problem caused by FEC set
> net/nfp: modify the comment of some control messages
> net/nfp: fix memory leak in VF initialization logic
>
Except 2/10,
Series applied to dpdk-next-net/main, thanks.
The change in the 2/10 looks harmless, also I can see similar logic
already exist in the driver, but keeping it open to since discussion is
going on. Patch can be merged iteratively later if required.
^ permalink raw reply [flat|nested] 38+ messages in thread
end of thread, other threads:[~2024-10-13 2:49 UTC | newest]
Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-10-10 9:17 [PATCH 00/10] modify some logic of NFP PMD Chaoyong He
2024-10-10 9:17 ` [PATCH 01/10] net/nfp: use strlcpy for copying string Chaoyong He
2024-10-10 15:08 ` Stephen Hemminger
2024-10-10 9:17 ` [PATCH 02/10] net/nfp: fix malloc name problem in secondary process Chaoyong He
2024-10-10 15:07 ` Stephen Hemminger
2024-10-11 2:31 ` Chaoyong He
2024-10-10 9:17 ` [PATCH 03/10] net/nfp: simplify some function parameters Chaoyong He
2024-10-10 15:08 ` Stephen Hemminger
2024-10-10 9:17 ` [PATCH 04/10] net/nfp: improve the logic readability Chaoyong He
2024-10-10 15:10 ` Stephen Hemminger
2024-10-11 2:36 ` Chaoyong He
2024-10-10 9:17 ` [PATCH 05/10] net/nfp: fix problem caused by configure function Chaoyong He
2024-10-10 15:13 ` Stephen Hemminger
2024-10-10 9:17 ` [PATCH 06/10] net/nfp: add check logic for port up/down function Chaoyong He
2024-10-10 15:14 ` Stephen Hemminger
2024-10-10 9:17 ` [PATCH 07/10] net/nfp: fix problem caused by commit end function Chaoyong He
2024-10-10 15:15 ` Stephen Hemminger
2024-10-10 9:17 ` [PATCH 08/10] net/nfp: fix problem caused by FEC set Chaoyong He
2024-10-10 15:15 ` Stephen Hemminger
2024-10-10 9:17 ` [PATCH 09/10] net/nfp: modify the comment of some control messages Chaoyong He
2024-10-10 15:15 ` Stephen Hemminger
2024-10-10 9:17 ` [PATCH 10/10] net/nfp: fix memory leak in VF initialization logic Chaoyong He
2024-10-10 15:19 ` Stephen Hemminger
2024-10-11 2:38 ` Chaoyong He
2024-10-12 2:40 ` [PATCH v2 00/10] modify some logic of NFP PMD Chaoyong He
2024-10-12 2:40 ` [PATCH v2 01/10] net/nfp: use strlcpy for copying string Chaoyong He
2024-10-12 2:40 ` [PATCH v2 02/10] net/nfp: fix malloc name problem in secondary process Chaoyong He
2024-10-12 2:45 ` Stephen Hemminger
2024-10-12 2:47 ` Chaoyong He
2024-10-12 2:41 ` [PATCH v2 03/10] net/nfp: simplify some function parameters Chaoyong He
2024-10-12 2:41 ` [PATCH v2 04/10] net/nfp: improve the logic readability Chaoyong He
2024-10-12 2:41 ` [PATCH v2 05/10] net/nfp: fix problem caused by configure function Chaoyong He
2024-10-12 2:41 ` [PATCH v2 06/10] net/nfp: add check logic for port up/down function Chaoyong He
2024-10-12 2:41 ` [PATCH v2 07/10] net/nfp: fix problem caused by commit end function Chaoyong He
2024-10-12 2:41 ` [PATCH v2 08/10] net/nfp: fix problem caused by FEC set Chaoyong He
2024-10-12 2:41 ` [PATCH v2 09/10] net/nfp: modify the comment of some control messages Chaoyong He
2024-10-12 2:41 ` [PATCH v2 10/10] net/nfp: fix memory leak in VF initialization logic Chaoyong He
2024-10-13 2:49 ` [PATCH v2 00/10] modify some logic of NFP PMD Ferruh Yigit
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).