[PATCH v2 00/34] net/ntnic: bugfixes and refactoring
This patch set addresses fixing issues in the ntnic PMD driver. Changes in this patch: The issues detected by the Coverity Scan tool. The problems that were detected by the internal tests. Fix for Bug 1622: ntnic: using signals and threads: https://bugs.dpdk.org/show_bug.cgi?id=1622. The handling of signals within the PMD driver was removed. For manipulation with all threads dedicated EAL API (rte_thread_create_internal_control) is used. Product by design requires usage of threads inside PMD driver. Danylo Vodopianov (26): net/ntnic: fix index verification net/ntnic: add thread check return code net/ntnic: add return code handling net/ntnic: add array index verification net/ntnic: fix realloc memory leak net/ntnic: fix array index verification net/ntnic: add var definition transparently net/ntnic: add proper var freed net/ntnic: remove unused code net/ntnic: fix potentially overflow net/ntnic: add null checking net/ntnic: fix overflow issue net/ntnic: fix untrusted loop bound net/ntnic: add null checking net/ntnic: move null checking net/ntnic: fix var size net/ntnic: fix var overflow net/ntnic: remove unused code net/ntnic: remove convert error func net/ntnic: fix array verification net/ntnic: fix memory leak net/ntnic: remove unused code net/ntnic: refactor RSS implementation net/ntnic: fix age timeout recalculation into fpga unit net/ntnic: rework age event generation net/ntnic: fix group print Oleksandr Kolomeiets (2): net/ntnic: remove extra address-of operator net/ntnic: remove extra check for null Serhii Iliushyk (6): net/ntnic: extend module mapping net/ntnic: refactoring of the FPGA initialization net/ntnic: remove shutdown thread net/ntnic: add checks for action modify net/ntnic: add IFR DROP counter net/ntnic: remove tag EXPERIMENTAL MAINTAINERS | 2 +- .../net/ntnic/adapter/nt4ga_stat/nt4ga_stat.c | 18 +- drivers/net/ntnic/dbsconfig/ntnic_dbsconfig.c | 10 +- drivers/net/ntnic/include/create_elements.h | 1 - drivers/net/ntnic/include/flow_api.h | 10 +- drivers/net/ntnic/include/flow_api_engine.h | 2 + drivers/net/ntnic/include/hw_mod_backend.h| 24 +- drivers/net/ntnic/include/hw_mod_tpe_v3.h | 5 + drivers/net/ntnic/include/ntnic_stat.h| 9 + .../link_mgmt/link_100g/nt4ga_link_100g.c | 2 +- drivers/net/ntnic/meson.build | 1 + drivers/net/ntnic/nthw/core/nthw_fpga.c | 14 +- drivers/net/ntnic/nthw/flow_api/flow_api.c| 83 +- .../nthw/flow_api/flow_backend/flow_backend.c | 28 +- drivers/net/ntnic/nthw/flow_api/flow_group.c | 26 + .../net/ntnic/nthw/flow_api/flow_hsh_cfg.c| 661 + .../net/ntnic/nthw/flow_api/flow_hsh_cfg.h| 17 + .../ntnic/nthw/flow_api/hw_mod/hw_mod_flm.c | 14 +- .../ntnic/nthw/flow_api/hw_mod/hw_mod_hsh.c | 19 +- .../ntnic/nthw/flow_api/hw_mod/hw_mod_pdb.c | 18 +- .../ntnic/nthw/flow_api/hw_mod/hw_mod_tpe.c | 58 +- .../profile_inline/flow_api_hw_db_inline.c| 29 +- .../profile_inline/flow_api_profile_inline.c | 925 +++--- .../profile_inline/flow_api_profile_inline.h | 8 +- .../ntnic/nthw/flow_filter/flow_nthw_ifr.c| 32 + .../ntnic/nthw/flow_filter/flow_nthw_ifr.h| 15 +- .../ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c | 10 +- drivers/net/ntnic/nthw/stat/nthw_stat.c | 2 + .../nthw/supported/nthw_fpga_mod_str_map.c| 24 + drivers/net/ntnic/ntnic_ethdev.c | 69 +- drivers/net/ntnic/ntnic_filter/ntnic_filter.c | 155 ++- drivers/net/ntnic/ntnic_mod_reg.h | 11 +- drivers/net/ntnic/ntnic_xstats/ntnic_xstats.c | 10 +- 33 files changed, 1204 insertions(+), 1108 deletions(-) create mode 100644 drivers/net/ntnic/nthw/flow_api/flow_hsh_cfg.c create mode 100644 drivers/net/ntnic/nthw/flow_api/flow_hsh_cfg.h -- 2.45.0
[PATCH v2 03/34] net/ntnic: add return code handling
From: Danylo Vodopianov
CI found couple coverity problems which were fixed in this commit.
CID: 448984, 448982, 448975, 448969, 448968, 448967
API usage errors (BAD_COMPARE).
Add memcmp return value checking.
Coverity issue: 448984
Fixes: 6e8b7f11205f ("net/ntnic: add categorizer (CAT) FPGA module")
Signed-off-by: Danylo Vodopianov
---
v2
* replace return with break for macro DO_COMPARE_INDEXS
---
drivers/net/ntnic/include/hw_mod_backend.h | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ntnic/include/hw_mod_backend.h
b/drivers/net/ntnic/include/hw_mod_backend.h
index f91a3ed058..544962751a 100644
--- a/drivers/net/ntnic/include/hw_mod_backend.h
+++ b/drivers/net/ntnic/include/hw_mod_backend.h
@@ -114,10 +114,10 @@ enum {
typeof(be_module_reg) *temp_be_module = &(be_module_reg);
\
typeof(idx) tmp_idx = (idx);
\
typeof(cmp_idx) tmp_cmp_idx = (cmp_idx);
\
- if ((unsigned int)(tmp_idx) != (unsigned int)(tmp_cmp_idx)) {
\
- (void)memcmp(temp_be_module + tmp_idx,
&temp_be_module[tmp_cmp_idx], \
-sizeof(type));
\
- }
\
+ if ((unsigned int)(tmp_idx) != (unsigned int)(tmp_cmp_idx))
\
+ if (memcmp(temp_be_module + tmp_idx,
&temp_be_module[tmp_cmp_idx], \
+ sizeof(type)) == 0) \
+ break;
\
} while (0)
static inline int is_non_zero(const void *addr, size_t n)
--
2.45.0
[PATCH v2 01/34] net/ntnic: fix index verification
From: Danylo Vodopianov
CI found couple coverity problems which were fixed in this commit.
CID: 448974, 448977, 448978 (OVERRUN).
These issues were fixed with updating index verification statement.
Coverity issue: 448974
Fixes: effa04693274 ("net/ntnic: add statistics")
Signed-off-by: Danylo Vodopianov
---
drivers/net/ntnic/ntnic_filter/ntnic_filter.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ntnic/ntnic_filter/ntnic_filter.c
b/drivers/net/ntnic/ntnic_filter/ntnic_filter.c
index 4c8503f689..e00b10ff82 100644
--- a/drivers/net/ntnic/ntnic_filter/ntnic_filter.c
+++ b/drivers/net/ntnic/ntnic_filter/ntnic_filter.c
@@ -1201,7 +1201,7 @@ static int poll_statistics(struct pmd_internals
*internals)
const int if_index = internals->n_intf_no;
uint64_t last_stat_rtc = 0;
- if (!p_nt4ga_stat || if_index < 0 || if_index > NUM_ADAPTER_PORTS_MAX)
+ if (!p_nt4ga_stat || if_index < 0 || if_index >= NUM_ADAPTER_PORTS_MAX)
return -1;
assert(rte_tsc_freq > 0);
--
2.45.0
[PATCH v2 04/34] net/ntnic: add array index verification
From: Danylo Vodopianov
CI found couple coverity problems which were fixed in this commit.
CID: 448983, 448980
Memory - corruptions (OVERRUN)
Add check both indices within bounds before calling the macro
Coverity issue: 448983
Fixes: 6e8b7f11205f ("net/ntnic: add categorizer (CAT) FPGA module")
Signed-off-by: Danylo Vodopianov
---
.../ntnic/nthw/flow_api/hw_mod/hw_mod_hsh.c| 17 -
.../ntnic/nthw/flow_api/hw_mod/hw_mod_pdb.c| 18 --
2 files changed, 32 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ntnic/nthw/flow_api/hw_mod/hw_mod_hsh.c
b/drivers/net/ntnic/nthw/flow_api/hw_mod/hw_mod_hsh.c
index 1750d09afb..cc8db2fae5 100644
--- a/drivers/net/ntnic/nthw/flow_api/hw_mod/hw_mod_hsh.c
+++ b/drivers/net/ntnic/nthw/flow_api/hw_mod/hw_mod_hsh.c
@@ -121,8 +121,23 @@ static int hw_mod_hsh_rcp_mod(struct flow_api_backend_s
*be, enum hw_hsh_e field
INDEX_TOO_LARGE_LOG;
return INDEX_TOO_LARGE;
}
+ /* Size of the structure */
+ size_t element_size = sizeof(struct hsh_v5_rcp_s);
+ /* Size of the buffer */
+ size_t buffer_size = sizeof(be->hsh.v5.rcp);
- DO_COMPARE_INDEXS(be->hsh.v5.rcp, struct hsh_v5_rcp_s,
index, word_off);
+ /* Calculate the maximum valid index (number of
elements in the buffer) */
+ size_t max_idx = buffer_size / element_size;
+
+ /* Check that both indices are within bounds before
calling the macro */
+ if (index < max_idx && word_off < max_idx) {
+ DO_COMPARE_INDEXS(be->hsh.v5.rcp, struct
hsh_v5_rcp_s, index,
+ word_off);
+
+ } else {
+ INDEX_TOO_LARGE_LOG;
+ return INDEX_TOO_LARGE;
+ }
break;
case HW_HSH_RCP_FIND:
diff --git a/drivers/net/ntnic/nthw/flow_api/hw_mod/hw_mod_pdb.c
b/drivers/net/ntnic/nthw/flow_api/hw_mod/hw_mod_pdb.c
index 59285405ba..147a06ac2b 100644
--- a/drivers/net/ntnic/nthw/flow_api/hw_mod/hw_mod_pdb.c
+++ b/drivers/net/ntnic/nthw/flow_api/hw_mod/hw_mod_pdb.c
@@ -131,8 +131,22 @@ static int hw_mod_pdb_rcp_mod(struct flow_api_backend_s
*be, enum hw_pdb_e field
INDEX_TOO_LARGE_LOG;
return INDEX_TOO_LARGE;
}
-
- DO_COMPARE_INDEXS(be->pdb.v9.rcp, struct pdb_v9_rcp_s,
index, *value);
+ /* Size of the structure */
+ size_t element_size = sizeof(struct pdb_v9_rcp_s);
+ /* Size of the buffer */
+ size_t buffer_size = sizeof(be->pdb.v9.rcp);
+
+ /* Calculate the maximum valid index (number of
elements in the buffer) */
+ size_t max_idx = buffer_size / element_size;
+
+ /* Check that both indices are within bounds before
calling the macro */
+ if (index < max_idx && *value < max_idx) {
+ DO_COMPARE_INDEXS(be->pdb.v9.rcp, struct
pdb_v9_rcp_s, index,
+ *value);
+ } else {
+ INDEX_TOO_LARGE_LOG;
+ return INDEX_TOO_LARGE;
+ }
break;
case HW_PDB_RCP_DESCRIPTOR:
--
2.45.0
[PATCH v2 07/34] net/ntnic: add var definition transparently
From: Danylo Vodopianov
set eth_base to NULL after freeing to prevent
use-after-free
CID: 446746 Use after free (USE_AFTER_FREE)
Coverity issue: 446746
Fixes: 1d3f62a0c4f1 ("net/ntnic: add base init and deinit of flow API")
Signed-off-by: Danylo Vodopianov
---
drivers/net/ntnic/nthw/flow_api/flow_api.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ntnic/nthw/flow_api/flow_api.c
b/drivers/net/ntnic/nthw/flow_api/flow_api.c
index 82d4e34ae9..d25d1a3dd1 100644
--- a/drivers/net/ntnic/nthw/flow_api/flow_api.c
+++ b/drivers/net/ntnic/nthw/flow_api/flow_api.c
@@ -385,8 +385,10 @@ static void flow_ndev_reset(struct flow_nic_dev *ndev)
}
/* Delete all eth-port devices created on this NIC device */
- while (ndev->eth_base)
+ while (ndev->eth_base) {
flow_delete_eth_dev(ndev->eth_base);
+ ndev->eth_base = NULL;
+ }
/* Error check */
while (ndev->flow_base) {
--
2.45.0
[PATCH v2 09/34] net/ntnic: remove unused code
From: Danylo Vodopianov
Unused code was removed.
CID: 448981 Logically dead code (DEADCODE)
Coverity issue: 448981
Fixes: e02fdb65c2a8 ("net/ntnic: add flow create/destroy")
Signed-off-by: Danylo Vodopianov
---
.../flow_api/profile_inline/flow_api_profile_inline.c| 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git
a/drivers/net/ntnic/nthw/flow_api/profile_inline/flow_api_profile_inline.c
b/drivers/net/ntnic/nthw/flow_api/profile_inline/flow_api_profile_inline.c
index 1c7d5cac3e..535047d246 100644
--- a/drivers/net/ntnic/nthw/flow_api/profile_inline/flow_api_profile_inline.c
+++ b/drivers/net/ntnic/nthw/flow_api/profile_inline/flow_api_profile_inline.c
@@ -4268,13 +4268,8 @@ struct flow_handle *flow_create_profile_inline(struct
flow_eth_dev *dev __rte_un
err_exit:
- if (fh) {
- flow_destroy_locked_profile_inline(dev, fh, NULL);
- fh = NULL;
- } else {
- free(fd);
- fd = NULL;
- }
+ free(fd);
+ fd = NULL;
rte_spinlock_unlock(&dev->ndev->mtx);
--
2.45.0
[PATCH v2 05/34] net/ntnic: fix realloc memory leak
From: Danylo Vodopianov
Issue was fixed with verification in case of the
successful memory re-allocation.
Coverity issue: 448959
Fixes: 4033e0539435 ("net/ntnic: add flow meter")
Signed-off-by: Danylo Vodopianov
---
.../profile_inline/flow_api_profile_inline.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git
a/drivers/net/ntnic/nthw/flow_api/profile_inline/flow_api_profile_inline.c
b/drivers/net/ntnic/nthw/flow_api/profile_inline/flow_api_profile_inline.c
index ff8eb502f4..1c7d5cac3e 100644
--- a/drivers/net/ntnic/nthw/flow_api/profile_inline/flow_api_profile_inline.c
+++ b/drivers/net/ntnic/nthw/flow_api/profile_inline/flow_api_profile_inline.c
@@ -5523,11 +5523,16 @@ int flow_configure_profile_inline(struct flow_eth_dev
*dev, uint8_t caller_id,
struct flm_flow_mtr_handle_s *mtr_handle =
dev->ndev->flm_mtr_handle;
if (mtr_handle->port_stats[caller_id]->shared == 1) {
- res = realloc(mtr_handle->port_stats[caller_id]->stats,
- port_attr->nb_meters) == NULL
- ? -1
- : 0;
- mtr_handle->port_stats[caller_id]->size =
port_attr->nb_meters;
+ struct flm_mtr_stat_s *temp_stats =
+
realloc(mtr_handle->port_stats[caller_id]->stats,
+ port_attr->nb_meters);
+ if (temp_stats == NULL) {
+ res = -1;
+ } else {
+ res = 0;
+ mtr_handle->port_stats[caller_id]->stats =
temp_stats;
+ mtr_handle->port_stats[caller_id]->size =
port_attr->nb_meters;
+ }
} else {
mtr_handle->port_stats[caller_id] =
--
2.45.0
[PATCH v2 13/34] net/ntnic: fix untrusted loop bound
From: Danylo Vodopianov
Replace while with if statement to avoid infinite loop
in case ther_type will be modified with extenral source.
Coverity issue: 448917
Fixes: c6821abf58e8 ("net/ntnic: add flow items GTP and actions raw
encap/decap")
Signed-off-by: Danylo Vodopianov
---
drivers/net/ntnic/ntnic_filter/ntnic_filter.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ntnic/ntnic_filter/ntnic_filter.c
b/drivers/net/ntnic/ntnic_filter/ntnic_filter.c
index e00b10ff82..b07e16c1d3 100644
--- a/drivers/net/ntnic/ntnic_filter/ntnic_filter.c
+++ b/drivers/net/ntnic/ntnic_filter/ntnic_filter.c
@@ -53,7 +53,7 @@ int interpret_raw_data(uint8_t *data, uint8_t *preserve, int
size, struct rte_fl
goto interpret_end;
/* VLAN */
- while (ether_type == rte_cpu_to_be_16(RTE_ETHER_TYPE_VLAN) ||
+ if (ether_type == rte_cpu_to_be_16(RTE_ETHER_TYPE_VLAN) ||
ether_type == rte_cpu_to_be_16(RTE_ETHER_TYPE_QINQ) ||
ether_type == rte_cpu_to_be_16(RTE_ETHER_TYPE_QINQ1)) {
if (size - pkti == 0)
--
2.45.0
[PATCH v2 19/34] net/ntnic: remove convert error func
From: Danylo Vodopianov
convert_error func was removed as far as this approach was deprecated.
Coverity issue: 448973
Fixes: e526adf1fdef ("net/ntnic: add minimal create/destroy flow operations")
Signed-off-by: Danylo Vodopianov
---
drivers/net/ntnic/include/create_elements.h | 1 -
drivers/net/ntnic/ntnic_filter/ntnic_filter.c | 151 ++
2 files changed, 53 insertions(+), 99 deletions(-)
diff --git a/drivers/net/ntnic/include/create_elements.h
b/drivers/net/ntnic/include/create_elements.h
index 1456977837..f0b9410cb9 100644
--- a/drivers/net/ntnic/include/create_elements.h
+++ b/drivers/net/ntnic/include/create_elements.h
@@ -61,7 +61,6 @@ enum nt_rte_flow_item_type {
extern rte_spinlock_t flow_lock;
int interpret_raw_data(uint8_t *data, uint8_t *preserve, int size, struct
rte_flow_item *out);
-int convert_error(struct rte_flow_error *error, struct rte_flow_error
*rte_flow_error);
int create_attr(struct cnv_attr_s *attribute, const struct rte_flow_attr
*attr);
int create_match_elements(struct cnv_match_s *match, const struct
rte_flow_item items[],
int max_elem);
diff --git a/drivers/net/ntnic/ntnic_filter/ntnic_filter.c
b/drivers/net/ntnic/ntnic_filter/ntnic_filter.c
index b07e16c1d3..70bff776be 100644
--- a/drivers/net/ntnic/ntnic_filter/ntnic_filter.c
+++ b/drivers/net/ntnic/ntnic_filter/ntnic_filter.c
@@ -246,23 +246,6 @@ int interpret_raw_data(uint8_t *data, uint8_t *preserve,
int size, struct rte_fl
return hdri + 1;
}
-int convert_error(struct rte_flow_error *error, struct rte_flow_error
*rte_flow_error)
-{
- if (error) {
- error->cause = NULL;
- error->message = rte_flow_error->message;
-
- if (rte_flow_error->type == RTE_FLOW_ERROR_TYPE_NONE ||
- rte_flow_error->type == RTE_FLOW_ERROR_TYPE_NONE)
- error->type = RTE_FLOW_ERROR_TYPE_NONE;
-
- else
- error->type = RTE_FLOW_ERROR_TYPE_UNSPECIFIED;
- }
-
- return 0;
-}
-
int create_attr(struct cnv_attr_s *attribute, const struct rte_flow_attr *attr)
{
memset(&attribute->attr, 0x0, sizeof(struct rte_flow_attr));
@@ -497,13 +480,10 @@ static int convert_flow(struct rte_eth_dev *eth_dev,
struct pmd_internals *internals = eth_dev->data->dev_private;
struct fpga_info_s *fpga_info =
&internals->p_drv->ntdrv.adapter_info.fpga_info;
- static struct rte_flow_error flow_error = {
- .type = RTE_FLOW_ERROR_TYPE_NONE, .message = "none" };
+ error->type = RTE_FLOW_ERROR_TYPE_NONE;
+ error->message = "none";
uint32_t queue_offset = 0;
- /* Set initial error */
- convert_error(error, &flow_error);
-
if (!internals) {
rte_flow_error_set(error, EINVAL,
RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
"Missing eth_dev");
@@ -559,23 +539,19 @@ eth_flow_destroy(struct rte_eth_dev *eth_dev, struct
rte_flow *flow, struct rte_
struct pmd_internals *internals = eth_dev->data->dev_private;
- static struct rte_flow_error flow_error = {
- .type = RTE_FLOW_ERROR_TYPE_NONE, .message = "none" };
+ error->type = RTE_FLOW_ERROR_TYPE_NONE;
+ error->message = "none";
int res = 0;
- /* Set initial error */
- convert_error(error, &flow_error);
if (!flow)
return 0;
if (is_flow_handle_typecast(flow)) {
- res = flow_filter_ops->flow_destroy(internals->flw_dev, (void
*)flow, &flow_error);
- convert_error(error, &flow_error);
+ res = flow_filter_ops->flow_destroy(internals->flw_dev, (void
*)flow, error);
} else {
res = flow_filter_ops->flow_destroy(internals->flw_dev,
flow->flw_hdl,
- &flow_error);
- convert_error(error, &flow_error);
+ error);
rte_spinlock_lock(&flow_lock);
flow->used = 0;
@@ -606,8 +582,8 @@ static struct rte_flow *eth_flow_create(struct rte_eth_dev
*eth_dev,
struct cnv_match_s match = { 0 };
struct cnv_action_s action = { 0 };
- static struct rte_flow_error flow_error = {
- .type = RTE_FLOW_ERROR_TYPE_NONE, .message = "none" };
+ error->type = RTE_FLOW_ERROR_TYPE_NONE;
+ error->message = "none";
uint32_t flow_stat_id = 0;
if (convert_flow(eth_dev, attr, items, actions, &attribute, &match,
&action, error) < 0)
@@ -620,8 +596,7 @@ static struct rte_flow *eth_flow_create(struct rte_eth_dev
*eth_dev,
void *flw_hdl =
flow_filter_ops->flow_create(internals->flw_dev, &attribute.attr,
attribute.forced_vlan_vid, attribute.caller_id,
match.rte_flow_item, action.flow_actions,
- &flow_error);
- convert_error(error, &flow_error);
+
[PATCH v2 18/34] net/ntnic: remove unused code
From: Danylo Vodopianov
Unused code path in the condition irq_vector >= 0
because the condition irq_vector < 0 is already
established
Coverity issue: 446745
Fixes: 01e34ed9c756 ("net/ntnic: add availability monitor management")
Signed-off-by: Danylo Vodopianov
---
drivers/net/ntnic/dbsconfig/ntnic_dbsconfig.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ntnic/dbsconfig/ntnic_dbsconfig.c
b/drivers/net/ntnic/dbsconfig/ntnic_dbsconfig.c
index a2c9ef27f0..e1eccb647c 100644
--- a/drivers/net/ntnic/dbsconfig/ntnic_dbsconfig.c
+++ b/drivers/net/ntnic/dbsconfig/ntnic_dbsconfig.c
@@ -415,7 +415,7 @@ static struct nthw_virt_queue
*nthw_setup_rx_virt_queue(nthw_dbs_t *p_nthw_dbs,
if (irq_vector < 0) {
if (set_rx_am_data(p_nthw_dbs, index,
(uint64_t)avail_struct_phys_addr,
RX_AM_DISABLE, host_id, 0,
- irq_vector >= 0 ? 1 : 0) != 0) {
+ 0) != 0) {
return NULL;
}
}
--
2.45.0
[PATCH v2 11/34] net/ntnic: add null checking
From: Danylo Vodopianov
Fix issue with potential rereferencing a pointer
that might be NULL p->m_rpp_lr when calling
nthw_module_query_register
CID 448923: Dereference null return value (NULL_RETURNS)
Coverity issue: 448923
Fixes: f543ca6b9ab2 ("net/ntnic: add RPP local retransmit (RPP LR) flow module")
Signed-off-by: Danylo Vodopianov
---
drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c
b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c
index 28c7a05fe2..e69a1ca823 100644
--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c
+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c
@@ -54,18 +54,20 @@ int rpp_lr_nthw_init(struct rpp_lr_nthw *p, nthw_fpga_t
*p_fpga, int n_instance)
p->mp_fpga = p_fpga;
p->m_physical_adapter_no = (uint8_t)n_instance;
p->m_rpp_lr = nthw_fpga_query_module(p_fpga, MOD_RPP_LR, n_instance);
-
- p->mp_rcp_ctrl = nthw_module_get_register(p->m_rpp_lr, RPP_LR_RCP_CTRL);
+ if (p->m_rpp_lr)
+ p->mp_rcp_ctrl = nthw_module_get_register(p->m_rpp_lr,
RPP_LR_RCP_CTRL);
p->mp_rcp_addr = nthw_register_get_field(p->mp_rcp_ctrl,
RPP_LR_RCP_CTRL_ADR);
p->mp_rcp_cnt = nthw_register_get_field(p->mp_rcp_ctrl,
RPP_LR_RCP_CTRL_CNT);
- p->mp_rcp_data = nthw_module_get_register(p->m_rpp_lr, RPP_LR_RCP_DATA);
+ if (p->m_rpp_lr)
+ p->mp_rcp_data = nthw_module_get_register(p->m_rpp_lr,
RPP_LR_RCP_DATA);
p->mp_rcp_data_exp = nthw_register_get_field(p->mp_rcp_data,
RPP_LR_RCP_DATA_EXP);
p->mp_ifr_rcp_ctrl = nthw_module_query_register(p->m_rpp_lr,
RPP_LR_IFR_RCP_CTRL);
p->mp_ifr_rcp_addr =
nthw_register_query_field(p->mp_ifr_rcp_ctrl,
RPP_LR_IFR_RCP_CTRL_ADR);
p->mp_ifr_rcp_cnt = nthw_register_query_field(p->mp_ifr_rcp_ctrl,
RPP_LR_IFR_RCP_CTRL_CNT);
- p->mp_ifr_rcp_data = nthw_module_query_register(p->m_rpp_lr,
RPP_LR_IFR_RCP_DATA);
+ if (p->m_rpp_lr)
+ p->mp_ifr_rcp_data = nthw_module_query_register(p->m_rpp_lr,
RPP_LR_IFR_RCP_DATA);
p->mp_ifr_rcp_data_ipv4_en =
nthw_register_query_field(p->mp_ifr_rcp_data,
RPP_LR_IFR_RCP_DATA_IPV4_EN);
p->mp_ifr_rcp_data_ipv6_en =
--
2.45.0
[PATCH v2 15/34] net/ntnic: move null checking
From: Danylo Vodopianov
Move null checking before using
Coverity issue: 446759
Fixes: f0fe222ea9cf ("net/ntnic: add releasing virtqueues")
Signed-off-by: Danylo Vodopianov
---
drivers/net/ntnic/dbsconfig/ntnic_dbsconfig.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ntnic/dbsconfig/ntnic_dbsconfig.c
b/drivers/net/ntnic/dbsconfig/ntnic_dbsconfig.c
index c178144d42..279a852a1c 100644
--- a/drivers/net/ntnic/dbsconfig/ntnic_dbsconfig.c
+++ b/drivers/net/ntnic/dbsconfig/ntnic_dbsconfig.c
@@ -510,11 +510,11 @@ static int dbs_wait_hw_queue_shutdown(struct
nthw_virt_queue *vq, int rx)
static int dbs_internal_release_rx_virt_queue(struct nthw_virt_queue *rxvq)
{
- nthw_dbs_t *p_nthw_dbs = rxvq->mp_nthw_dbs;
-
if (rxvq == NULL)
return -1;
+ nthw_dbs_t *p_nthw_dbs = rxvq->mp_nthw_dbs;
+
/* Clear UW */
rxvq->used_struct_phys_addr = NULL;
--
2.45.0
[PATCH v2 08/34] net/ntnic: add proper var freed
From: Danylo Vodopianov
p_fpga_mgr is properly freed when it's no longer needed
CID 440546: Resource leak (RESOURCE_LEAK)
Fixes: ddf184d0b6c2 ("net/ntnic: add FPGA initialization")
Signed-off-by: Danylo Vodopianov
---
drivers/net/ntnic/nthw/core/nthw_fpga.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ntnic/nthw/core/nthw_fpga.c
b/drivers/net/ntnic/nthw/core/nthw_fpga.c
index ca69a9d5b1..88641145ec 100644
--- a/drivers/net/ntnic/nthw/core/nthw_fpga.c
+++ b/drivers/net/ntnic/nthw/core/nthw_fpga.c
@@ -230,6 +230,8 @@ int nthw_fpga_init(struct fpga_info_s *p_fpga_info)
if (p_fpga == NULL) {
NT_LOG(ERR, NTHW, "%s: Unsupported FPGA: %s (%08X)",
p_adapter_id_str,
s_fpga_prod_ver_rev_str,
p_fpga_info->n_fpga_build_time);
+ nthw_fpga_mgr_delete(p_fpga_mgr);
+ p_fpga_mgr = NULL;
return -1;
}
--
2.45.0
[PATCH v2 22/34] net/ntnic: remove extra address-of operator
From: Oleksandr Kolomeiets
Macro DO_COMPARE_INDEXS expects pointer to array.
If condition is true, one of array's elements get overwritten.
Misinterpreting pointer to pointer as pointer to array
may result in out-of-bounds access.
Coverity issue: 448966, 448970, 448971, 448972
Fixes: 6e8b7f11205f ("net/ntnic: add categorizer (CAT) FPGA module")
Signed-off-by: Oleksandr Kolomeiets
---
drivers/net/ntnic/include/hw_mod_backend.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ntnic/include/hw_mod_backend.h
b/drivers/net/ntnic/include/hw_mod_backend.h
index 544962751a..1941692ddf 100644
--- a/drivers/net/ntnic/include/hw_mod_backend.h
+++ b/drivers/net/ntnic/include/hw_mod_backend.h
@@ -111,7 +111,7 @@ enum {
#define DO_COMPARE_INDEXS(be_module_reg, type, idx, cmp_idx)
\
do {
\
- typeof(be_module_reg) *temp_be_module = &(be_module_reg);
\
+ typeof(be_module_reg) temp_be_module = (be_module_reg);
\
typeof(idx) tmp_idx = (idx);
\
typeof(cmp_idx) tmp_cmp_idx = (cmp_idx);
\
if ((unsigned int)(tmp_idx) != (unsigned int)(tmp_cmp_idx))
\
--
2.45.0
[PATCH v2 23/34] net/ntnic: remove extra check for null
From: Oleksandr Kolomeiets
pld_ptr points to mp_port_load's element,
which is initialized during driver's probe,
otherwise probe fails and xstats_get_by_id is not called.
Coverity issue: 448945
Fixes: cf6007eac498 ("net/ntnic: add xstats")
Signed-off-by: Oleksandr Kolomeiets
---
drivers/net/ntnic/ntnic_xstats/ntnic_xstats.c | 10 +-
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/net/ntnic/ntnic_xstats/ntnic_xstats.c
b/drivers/net/ntnic/ntnic_xstats/ntnic_xstats.c
index 7604afe6a0..cf3271d5de 100644
--- a/drivers/net/ntnic/ntnic_xstats/ntnic_xstats.c
+++ b/drivers/net/ntnic/ntnic_xstats/ntnic_xstats.c
@@ -645,16 +645,8 @@ static int nthw_xstats_get_by_id(nt4ga_stat_t
*p_nt4ga_stat,
break;
case 4:
-
/* Port Load stat */
- if (pld_ptr) {
- /* No reset */
- values[i] = *((uint64_t
*)&pld_ptr[names[i].offset]);
-
- } else {
- values[i] = 0;
- }
-
+ values[i] = *((uint64_t
*)&pld_ptr[names[i].offset]);
break;
default:
--
2.45.0
[PATCH v2 14/34] net/ntnic: add null checking
From: Danylo Vodopianov
Add null checking for fpga var.
Coverity issue: 448916
Fixes: 30b2f87ac650 ("net/ntnic: add GMF module")
Signed-off-by: Danylo Vodopianov
---
drivers/net/ntnic/link_mgmt/link_100g/nt4ga_link_100g.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ntnic/link_mgmt/link_100g/nt4ga_link_100g.c
b/drivers/net/ntnic/link_mgmt/link_100g/nt4ga_link_100g.c
index d8e0cad7cd..67dc0d01f6 100644
--- a/drivers/net/ntnic/link_mgmt/link_100g/nt4ga_link_100g.c
+++ b/drivers/net/ntnic/link_mgmt/link_100g/nt4ga_link_100g.c
@@ -405,7 +405,7 @@ static int _port_init(adapter_info_t *drv, nthw_fpga_t
*fpga, int port)
_reset_rx(drv, mac_pcs);
/* 2.2) Nt4gaPort::setup() */
- if (nthw_gmf_init(NULL, fpga, port) == 0) {
+ if (fpga && nthw_gmf_init(NULL, fpga, port) == 0) {
nthw_gmf_t gmf;
if (nthw_gmf_init(&gmf, fpga, port) == 0)
--
2.45.0
[PATCH v2 16/34] net/ntnic: fix var size
From: Danylo Vodopianov
Function operate with uint16_r value, meanwhile return value with
uint32_t size.
This conversion was aligned to the uint8_t as far as max value of the
return value could be equal 16.
Coverity issue: 446747
Fixes: 67aee0a69665 ("net/ntnic: add used writer data handling")
Signed-off-by: Danylo Vodopianov
---
drivers/net/ntnic/dbsconfig/ntnic_dbsconfig.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ntnic/dbsconfig/ntnic_dbsconfig.c
b/drivers/net/ntnic/dbsconfig/ntnic_dbsconfig.c
index 279a852a1c..a2c9ef27f0 100644
--- a/drivers/net/ntnic/dbsconfig/ntnic_dbsconfig.c
+++ b/drivers/net/ntnic/dbsconfig/ntnic_dbsconfig.c
@@ -345,9 +345,9 @@ dbs_initialize_virt_queue_structs(void *avail_struct_addr,
void *used_struct_add
flgs);
}
-static uint16_t dbs_qsize_log2(uint16_t qsize)
+static uint8_t dbs_qsize_log2(uint16_t qsize)
{
- uint32_t qs = 0;
+ uint8_t qs = 0;
while (qsize) {
qsize = qsize >> 1;
--
2.45.0
[PATCH v2 20/34] net/ntnic: fix array verification
From: Danylo Vodopianov
if statement was modify to ensure that word_off
doesn't exceed the size of the array
Coverity issue: 448958
Fixes: 7fa0bf29e667 ("net/ntnic: add hash module")
Signed-off-by: Danylo Vodopianov
---
drivers/net/ntnic/nthw/flow_api/hw_mod/hw_mod_hsh.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ntnic/nthw/flow_api/hw_mod/hw_mod_hsh.c
b/drivers/net/ntnic/nthw/flow_api/hw_mod/hw_mod_hsh.c
index cc8db2fae5..d19e72e323 100644
--- a/drivers/net/ntnic/nthw/flow_api/hw_mod/hw_mod_hsh.c
+++ b/drivers/net/ntnic/nthw/flow_api/hw_mod/hw_mod_hsh.c
@@ -221,7 +221,7 @@ static int hw_mod_hsh_rcp_mod(struct flow_api_backend_s
*be, enum hw_hsh_e field
break;
case HW_HSH_RCP_WORD_MASK:
- if (word_off > HSH_RCP_WORD_MASK_SIZE) {
+ if (word_off >= HSH_RCP_WORD_MASK_SIZE) {
WORD_OFF_TOO_LARGE_LOG;
return WORD_OFF_TOO_LARGE;
}
--
2.45.0
[PATCH v2 21/34] net/ntnic: fix memory leak
From: Danylo Vodopianov
free for kvlist was added before return to avoid memory leak.
Coveriry issue: 446751
Fixes: fe91ade9f5db ("net/ntnic: add basic queue operations")
Signed-off-by: Danylo Vodopianov
---
drivers/net/ntnic/ntnic_ethdev.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ntnic/ntnic_ethdev.c b/drivers/net/ntnic/ntnic_ethdev.c
index 6fcbb8fa9b..d1360cc925 100644
--- a/drivers/net/ntnic/ntnic_ethdev.c
+++ b/drivers/net/ntnic/ntnic_ethdev.c
@@ -2089,6 +2089,7 @@ nthw_pci_dev_init(struct rte_pci_device *pci_dev)
NT_LOG_DBGX(ERR, NTNIC,
"problem with command line arguments:
res=%d",
res);
+ free(kvlist);
return -1;
}
@@ -2112,6 +2113,7 @@ nthw_pci_dev_init(struct rte_pci_device *pci_dev)
NT_LOG_DBGX(ERR, NTNIC,
"problem with command line arguments:
res=%d",
res);
+ free(kvlist);
return -1;
}
--
2.45.0
[PATCH v2 34/34] net/ntnic: remove tag EXPERIMENTAL
Since nitnic PMD driver for is fully added and verified, the tag EXPERIMENTAL may be removed Signed-off-by: Serhii Iliushyk --- MAINTAINERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index b86cdd266b..b04951de85 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -890,7 +890,7 @@ F: drivers/net/octeon_ep/ F: doc/guides/nics/features/octeon_ep.ini F: doc/guides/nics/octeon_ep.rst -Napatech ntnic - EXPERIMENTAL +Napatech ntnic M: Christian Koue Muf M: Serhii Iliushyk F: drivers/net/ntnic/ -- 2.45.0
[PATCH v2 25/34] net/ntnic: refactor RSS implementation
From: Danylo Vodopianov
Virtualization backward compatible RSS implementation is no longer
needed, thus RSS was refactored as follows:
* conversion of RTE_ETH_RSS fields into HSH registers was moved to
separate files
* profile wrapper for RSS configuration was removed
* flow_nic_set_hasher(), to configure default 5-tuple hash, was
replaced by call of hsh_set() with proper RTE_ETH_RSS*
fields
Signed-off-by: Danylo Vodopianov
---
drivers/net/ntnic/include/flow_api.h | 9 -
drivers/net/ntnic/include/hw_mod_backend.h| 6 -
drivers/net/ntnic/meson.build | 1 +
drivers/net/ntnic/nthw/flow_api/flow_api.c| 63 --
.../net/ntnic/nthw/flow_api/flow_hsh_cfg.c| 661 +++
.../net/ntnic/nthw/flow_api/flow_hsh_cfg.h| 17 +
.../profile_inline/flow_api_hw_db_inline.c| 5 +-
.../profile_inline/flow_api_profile_inline.c | 782 +-
.../profile_inline/flow_api_profile_inline.h | 4 -
drivers/net/ntnic/ntnic_ethdev.c | 3 +-
drivers/net/ntnic/ntnic_mod_reg.h | 6 -
11 files changed, 695 insertions(+), 862 deletions(-)
create mode 100644 drivers/net/ntnic/nthw/flow_api/flow_hsh_cfg.c
create mode 100644 drivers/net/ntnic/nthw/flow_api/flow_hsh_cfg.h
diff --git a/drivers/net/ntnic/include/flow_api.h
b/drivers/net/ntnic/include/flow_api.h
index 0af766fe5b..9201b8a3ae 100644
--- a/drivers/net/ntnic/include/flow_api.h
+++ b/drivers/net/ntnic/include/flow_api.h
@@ -83,11 +83,6 @@ struct flow_eth_dev {
struct flow_eth_dev *next;
};
-enum flow_nic_hash_e {
- HASH_ALGO_ROUND_ROBIN = 0,
- HASH_ALGO_5TUPLE,
-};
-
/* registered NIC backends */
struct flow_nic_dev {
uint8_t adapter_no; /* physical adapter no in the host system */
@@ -234,10 +229,6 @@ void flow_nic_free_resource(struct flow_nic_dev *ndev,
enum res_type_e res_type,
int flow_nic_ref_resource(struct flow_nic_dev *ndev, enum res_type_e res_type,
int index);
int flow_nic_deref_resource(struct flow_nic_dev *ndev, enum res_type_e
res_type, int index);
-int flow_nic_set_hasher(struct flow_nic_dev *ndev, int hsh_idx, enum
flow_nic_hash_e algorithm);
-int flow_nic_set_hasher_fields(struct flow_nic_dev *ndev, int hsh_idx,
- struct nt_eth_rss_conf rss_conf);
-
int flow_get_flm_stats(struct flow_nic_dev *ndev, uint64_t *data, uint64_t
size);
#endif
diff --git a/drivers/net/ntnic/include/hw_mod_backend.h
b/drivers/net/ntnic/include/hw_mod_backend.h
index 1941692ddf..4061d3f9e5 100644
--- a/drivers/net/ntnic/include/hw_mod_backend.h
+++ b/drivers/net/ntnic/include/hw_mod_backend.h
@@ -239,12 +239,6 @@ enum {
PROT_TUN_L4_ICMP = 4
};
-
-enum {
- HASH_HASH_NONE = 0,
- HASH_5TUPLE = 8,
-};
-
enum {
CPY_SELECT_DSCP_IPV4 = 0,
CPY_SELECT_DSCP_IPV6 = 1,
diff --git a/drivers/net/ntnic/meson.build b/drivers/net/ntnic/meson.build
index 3c05ad1d87..bfc5ae5aa8 100644
--- a/drivers/net/ntnic/meson.build
+++ b/drivers/net/ntnic/meson.build
@@ -70,6 +70,7 @@ sources = files(
'nthw/flow_api/flow_backend/flow_backend.c',
'nthw/flow_api/flow_filter.c',
'nthw/flow_api/flow_hasher.c',
+'nthw/flow_api/flow_hsh_cfg.c',
'nthw/flow_api/flow_kcc.c',
'nthw/flow_api/flow_km.c',
'nthw/flow_api/hw_mod/hw_mod_backend.c',
diff --git a/drivers/net/ntnic/nthw/flow_api/flow_api.c
b/drivers/net/ntnic/nthw/flow_api/flow_api.c
index d25d1a3dd1..857051fe14 100644
--- a/drivers/net/ntnic/nthw/flow_api/flow_api.c
+++ b/drivers/net/ntnic/nthw/flow_api/flow_api.c
@@ -1009,55 +1009,6 @@ int sprint_nt_rss_mask(char *str, uint16_t str_len,
const char *prefix, uint64_t
return 0;
}
-/*
- * Hash
- */
-
-int flow_nic_set_hasher(struct flow_nic_dev *ndev, int hsh_idx, enum
flow_nic_hash_e algorithm)
-{
- hw_mod_hsh_rcp_set(&ndev->be, HW_HSH_RCP_PRESET_ALL, hsh_idx, 0, 0);
-
- switch (algorithm) {
- case HASH_ALGO_5TUPLE:
- /* need to create an IPv6 hashing and enable the adaptive ip
mask bit */
- hw_mod_hsh_rcp_set(&ndev->be, HW_HSH_RCP_LOAD_DIST_TYPE,
hsh_idx, 0, 2);
- hw_mod_hsh_rcp_set(&ndev->be, HW_HSH_RCP_QW0_PE, hsh_idx, 0,
DYN_FINAL_IP_DST);
- hw_mod_hsh_rcp_set(&ndev->be, HW_HSH_RCP_QW0_OFS, hsh_idx, 0,
-16);
- hw_mod_hsh_rcp_set(&ndev->be, HW_HSH_RCP_QW4_PE, hsh_idx, 0,
DYN_FINAL_IP_DST);
- hw_mod_hsh_rcp_set(&ndev->be, HW_HSH_RCP_QW4_OFS, hsh_idx, 0,
0);
- hw_mod_hsh_rcp_set(&ndev->be, HW_HSH_RCP_W8_PE, hsh_idx, 0,
DYN_L4);
- hw_mod_hsh_rcp_set(&ndev->be, HW_HSH_RCP_W8_OFS, hsh_idx, 0, 0);
- hw_mod_hsh_rcp_set(&ndev->be, HW_HSH_RCP_W9_PE, hsh_idx, 0, 0);
- hw_mod_hsh_rcp_set(&ndev->be, HW_HSH_RCP_W9_OFS, hsh_idx, 0, 0);
- hw_mod_hsh_rcp_set(&ndev->be, HW_HSH_RCP_W9_P, hsh_idx, 0, 0);
- hw_mod_hsh_rcp_set(&ndev->be, HW_HSH_RCP_P_MASK, hsh_idx, 0,
[PATCH v2 32/34] net/ntnic: add checks for action modify
Following checks were added for `action modify`:
* range check to trigger an error in case that value is too large
* check for unsupported types of action modify for group 0
Signed-off-by: Serhii Iliushyk
---
.../profile_inline/flow_api_profile_inline.c | 89 ---
1 file changed, 79 insertions(+), 10 deletions(-)
diff --git
a/drivers/net/ntnic/nthw/flow_api/profile_inline/flow_api_profile_inline.c
b/drivers/net/ntnic/nthw/flow_api/profile_inline/flow_api_profile_inline.c
index e911860c38..fe72865140 100644
--- a/drivers/net/ntnic/nthw/flow_api/profile_inline/flow_api_profile_inline.c
+++ b/drivers/net/ntnic/nthw/flow_api/profile_inline/flow_api_profile_inline.c
@@ -2990,7 +2990,36 @@ static int interpret_flow_elements(const struct
flow_eth_dev *dev,
return 0;
}
-static void copy_fd_to_fh_flm(struct flow_handle *fh, const struct
nic_flow_def *fd,
+static bool has_only_valid_bits_set(const uint8_t *byte_array, const uint16_t
byte_array_len,
+ uint16_t bit_len)
+{
+ if (byte_array_len * 8 < bit_len)
+ bit_len = byte_array_len * 8;
+
+ uint8_t mask;
+ uint16_t byte;
+
+ for (byte = 0; byte < byte_array_len; byte++) {
+ if (bit_len >= 8) {
+ bit_len -= 8;
+ mask = 0x00;
+
+ } else if (bit_len > 0) {
+ mask = 0xff >> bit_len << bit_len;
+ bit_len = 0;
+
+ } else {
+ mask = 0xFF;
+ }
+
+ if (byte_array[byte] & mask)
+ return false;
+ }
+
+ return true;
+}
+
+static int copy_fd_to_fh_flm(struct flow_handle *fh, const struct nic_flow_def
*fd,
const uint32_t *packet_data, uint32_t flm_key_id, uint32_t flm_ft,
uint16_t rpl_ext_ptr, uint32_t flm_scrub __rte_unused, uint32_t
priority)
{
@@ -3056,23 +3085,47 @@ static void copy_fd_to_fh_flm(struct flow_handle *fh,
const struct nic_flow_def
switch (fd->modify_field[i].select) {
case CPY_SELECT_DSCP_IPV4:
case CPY_SELECT_DSCP_IPV6:
+ if
(!has_only_valid_bits_set(fd->modify_field[i].value8, 16, 8)) {
+ NT_LOG(ERR, FILTER, "IP DSCP value is out of
the range");
+ return -1;
+ }
+
fh->flm_dscp = fd->modify_field[i].value8[0];
break;
case CPY_SELECT_RQI_QFI:
+ if
(!has_only_valid_bits_set(fd->modify_field[i].value8, 16, 6)) {
+ NT_LOG(ERR, FILTER, "GTPU QFI value is out of
the range");
+ return -1;
+ }
+
fh->flm_rqi = (fd->modify_field[i].value8[0] >> 6) &
0x1;
fh->flm_qfi = fd->modify_field[i].value8[0] & 0x3f;
break;
case CPY_SELECT_IPV4:
+ if
(!has_only_valid_bits_set(fd->modify_field[i].value8, 16, 32)) {
+ NT_LOG(ERR, FILTER, "IPv4 address value is out
of the range");
+ return -1;
+ }
+
fh->flm_nat_ipv4 =
ntohl(fd->modify_field[i].value32[0]);
break;
case CPY_SELECT_PORT:
+ if
(!has_only_valid_bits_set(fd->modify_field[i].value8, 16, 16)) {
+ NT_LOG(ERR, FILTER, "NAT port value is out of
the range");
+ return -1;
+ }
+
fh->flm_nat_port =
ntohs(fd->modify_field[i].value16[0]);
break;
case CPY_SELECT_TEID:
+ if
(!has_only_valid_bits_set(fd->modify_field[i].value8, 16, 32)) {
+ NT_LOG(ERR, FILTER, "GTPU TEID value is out of
the range");
+ return -1;
+ }
fh->flm_teid = ntohl(fd->modify_field[i].value32[0]);
break;
@@ -3085,6 +3138,8 @@ static void copy_fd_to_fh_flm(struct flow_handle *fh,
const struct nic_flow_def
fh->flm_mtu_fragmentation_recipe = fd->flm_mtu_fragmentation_recipe;
fh->context = fd->age.context;
+
+ return 0;
}
static int convert_fh_to_fh_flm(struct flow_handle *fh, const uint32_t
*packet_data,
@@ -3113,8 +3168,10 @@ static int convert_fh_to_fh_flm(struct flow_handle *fh,
const uint32_t *packet_d
for (int i = 0; i < RES_COUNT; ++i)
fh->flm_db_idxs[i] = fh_copy.db_idxs[i];
- copy_fd_to_fh_flm(fh, fd, packet_data, flm_key_id, flm_ft, rpl_ext_ptr,
flm_scrub,
- priority);
+ if (copy_fd_to_fh_flm(fh, fd, packet_data, flm_key_id, flm_ft,
rpl_ext_ptr,
+ flm_sc
[PATCH v2 33/34] net/ntnic: add IFR DROP counter
Support for IP Fragmenter DROP counter to display number of packets
dropped due to size larger than MTU.
NOTE: Frames are dropped only in case that both IPV4_EN and IPV4_DF_DROP
are set to one (resp. their IPV6 counterparts).
Signed-off-by: Serhii Iliushyk
---
.../net/ntnic/adapter/nt4ga_stat/nt4ga_stat.c | 9 +++
drivers/net/ntnic/include/flow_api.h | 1 +
drivers/net/ntnic/include/hw_mod_backend.h| 8 +++
drivers/net/ntnic/include/hw_mod_tpe_v3.h | 5 ++
drivers/net/ntnic/include/ntnic_stat.h| 9 +++
drivers/net/ntnic/nthw/flow_api/flow_api.c| 16 +
.../nthw/flow_api/flow_backend/flow_backend.c | 28 -
.../ntnic/nthw/flow_api/hw_mod/hw_mod_tpe.c | 58 ++-
.../profile_inline/flow_api_profile_inline.c | 19 ++
.../profile_inline/flow_api_profile_inline.h | 4 ++
.../ntnic/nthw/flow_filter/flow_nthw_ifr.c| 32 ++
.../ntnic/nthw/flow_filter/flow_nthw_ifr.h| 15 -
drivers/net/ntnic/nthw/stat/nthw_stat.c | 2 +
drivers/net/ntnic/ntnic_mod_reg.h | 5 ++
14 files changed, 208 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ntnic/adapter/nt4ga_stat/nt4ga_stat.c
b/drivers/net/ntnic/adapter/nt4ga_stat/nt4ga_stat.c
index 2add43639a..2f1e12f891 100644
--- a/drivers/net/ntnic/adapter/nt4ga_stat/nt4ga_stat.c
+++ b/drivers/net/ntnic/adapter/nt4ga_stat/nt4ga_stat.c
@@ -100,6 +100,8 @@ static int nt4ga_stat_init(struct adapter_info_s
*p_adapter_info)
p_nt4ga_stat->mn_rx_ports = p_nthw_stat->m_nb_rx_ports;
p_nt4ga_stat->mn_tx_ports = p_nthw_stat->m_nb_tx_ports;
+
+ p_nt4ga_stat->mn_ifr_counters = p_nthw_stat->m_nb_ifr_counters;
}
return 0;
@@ -205,6 +207,9 @@ static int nt4ga_stat_setup(struct adapter_info_s
*p_adapter_info)
p_nt4ga_stat->mp_stat_structs_flm->max_lps =
nthw_fpga_get_product_param(p_adapter_info->fpga_info.mp_fpga,
NT_FLM_LOAD_LPS_MAX, 0);
+
+ p_nt4ga_stat->mp_stat_structs_ifr =
+ calloc(1, sizeof(struct ifr_counters) *
p_nt4ga_stat->mn_ifr_counters);
}
p_nt4ga_stat->mp_port_load =
@@ -556,6 +561,10 @@ static int nt4ga_stat_collect_cap_v1_stats(struct
adapter_info_s *p_adapter_info
flow_filter_ops->flow_get_flm_stats(ndev, (uint64_t
*)p_nt4ga_stat->mp_stat_structs_flm,
sizeof(struct flm_counters_v1) / sizeof(uint64_t));
+ /* Update and get IFR stats */
+ flow_filter_ops->flow_get_ifr_stats(ndev, (uint64_t
*)p_nt4ga_stat->mp_stat_structs_ifr,
+ p_nt4ga_stat->mn_ifr_counters - 1);
+
/*
* Calculate correct load values:
* rpp = nthw_fpga_get_product_param(p_fpga, NT_RPP_PER_PS, 0);
diff --git a/drivers/net/ntnic/include/flow_api.h
b/drivers/net/ntnic/include/flow_api.h
index 9201b8a3ae..7f6aa82ee0 100644
--- a/drivers/net/ntnic/include/flow_api.h
+++ b/drivers/net/ntnic/include/flow_api.h
@@ -230,5 +230,6 @@ int flow_nic_ref_resource(struct flow_nic_dev *ndev, enum
res_type_e res_type, i
int flow_nic_deref_resource(struct flow_nic_dev *ndev, enum res_type_e
res_type, int index);
int flow_get_flm_stats(struct flow_nic_dev *ndev, uint64_t *data, uint64_t
size);
+int flow_get_ifr_stats(struct flow_nic_dev *ndev, uint64_t *data, uint8_t
port_count);
#endif
diff --git a/drivers/net/ntnic/include/hw_mod_backend.h
b/drivers/net/ntnic/include/hw_mod_backend.h
index 4061d3f9e5..594bdab2a6 100644
--- a/drivers/net/ntnic/include/hw_mod_backend.h
+++ b/drivers/net/ntnic/include/hw_mod_backend.h
@@ -899,6 +899,7 @@ enum hw_tpe_e {
HW_TPE_IFR_RCP_IPV6_EN,
HW_TPE_IFR_RCP_IPV6_DROP,
HW_TPE_IFR_RCP_MTU,
+ HW_TPE_IFR_COUNTERS_DROP,
HW_TPE_INS_RCP_DYN,
HW_TPE_INS_RCP_OFS,
HW_TPE_INS_RCP_LEN,
@@ -959,6 +960,12 @@ int hw_mod_tpe_ifr_rcp_flush(struct flow_api_backend_s
*be, int start_idx, int c
int hw_mod_tpe_ifr_rcp_set(struct flow_api_backend_s *be, enum hw_tpe_e field,
int index,
uint32_t value);
+int hw_mod_tpe_ifr_counters_update(struct flow_api_backend_s *be, int
start_idx, int count);
+int hw_mod_tpe_ifr_counters_set(struct flow_api_backend_s *be, enum hw_tpe_e
field, int index,
+ uint32_t value);
+int hw_mod_tpe_ifr_counters_get(struct flow_api_backend_s *be, enum hw_tpe_e
field, int index,
+ uint32_t *value);
+
int hw_mod_tpe_ins_rcp_flush(struct flow_api_backend_s *be, int start_idx, int
count);
int hw_mod_tpe_ins_rcp_set(struct flow_api_backend_s *be, enum hw_tpe_e field,
int index,
uint32_t value);
@@ -1124,6 +1131,7 @@ struct flow_api_backend_ops {
int (*tpe_rpp_rcp_flush)(void *dev, const struct tpe_func_s *tpe, int
index, int cnt);
int (*tpe_rpp_ifr_rcp_flush)(void *dev, const struct tpe_func_s *tpe,
int index, int cnt);
int (*tpe_ifr_rcp_flush)(void *dev, const struct tpe_func_s *tpe, int
inde
[PATCH v2 30/34] net/ntnic: refactoring of the FPGA initialization
Remove unnecessary checks and logs.
Signed-off-by: Serhii Iliushyk
---
drivers/net/ntnic/nthw/core/nthw_fpga.c | 12 +---
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/net/ntnic/nthw/core/nthw_fpga.c
b/drivers/net/ntnic/nthw/core/nthw_fpga.c
index 88641145ec..5ca186209a 100644
--- a/drivers/net/ntnic/nthw/core/nthw_fpga.c
+++ b/drivers/net/ntnic/nthw/core/nthw_fpga.c
@@ -265,18 +265,14 @@ int nthw_fpga_init(struct fpga_info_s *p_fpga_info)
nthw_rac_rab_flush(p_nthw_rac);
p_fpga_info->mp_nthw_rac = p_nthw_rac;
- bool included = true;
struct nt200a0x_ops *nt200a0x_ops = get_nt200a0x_ops();
switch (p_fpga_info->n_nthw_adapter_id) {
case NT_HW_ADAPTER_ID_NT200A02:
if (nt200a0x_ops != NULL)
res =
nt200a0x_ops->nthw_fpga_nt200a0x_init(p_fpga_info);
-
- else
- included = false;
-
break;
+
default:
NT_LOG(ERR, NTHW, "%s: Unsupported HW product id: %d",
p_adapter_id_str,
p_fpga_info->n_nthw_adapter_id);
@@ -284,12 +280,6 @@ int nthw_fpga_init(struct fpga_info_s *p_fpga_info)
break;
}
- if (!included) {
- NT_LOG(ERR, NTHW, "%s: NOT INCLUDED HW product: %d",
p_adapter_id_str,
- p_fpga_info->n_nthw_adapter_id);
- res = -1;
- }
-
if (res) {
NT_LOG(ERR, NTHW, "%s: status: 0x%08X", p_adapter_id_str, res);
return res;
--
2.45.0
[PATCH v2 28/34] net/ntnic: fix group print
From: Danylo Vodopianov
Flow dump output was fixed to show original group
number for `jumps` stored in action and match sets.
Fixes: 6f0fe142caed ("net/ntnic: add flow dump")
Signed-off-by: Danylo Vodopianov
---
drivers/net/ntnic/include/flow_api_engine.h | 2 ++
drivers/net/ntnic/nthw/flow_api/flow_group.c | 26 +++
.../profile_inline/flow_api_hw_db_inline.c| 24 -
3 files changed, 45 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ntnic/include/flow_api_engine.h
b/drivers/net/ntnic/include/flow_api_engine.h
index 636c53b260..262317002f 100644
--- a/drivers/net/ntnic/include/flow_api_engine.h
+++ b/drivers/net/ntnic/include/flow_api_engine.h
@@ -425,5 +425,7 @@ int flow_group_handle_destroy(void **handle);
int flow_group_translate_get(void *handle, uint8_t owner_id, uint8_t port_id,
uint32_t group_in,
uint32_t *group_out);
+int flow_group_translate_get_orig_group(void *handle, uint32_t
translated_group,
+ uint32_t *group_orig);
#endif /* _FLOW_API_ENGINE_H_ */
diff --git a/drivers/net/ntnic/nthw/flow_api/flow_group.c
b/drivers/net/ntnic/nthw/flow_api/flow_group.c
index f76986b178..d1fa0193e1 100644
--- a/drivers/net/ntnic/nthw/flow_api/flow_group.c
+++ b/drivers/net/ntnic/nthw/flow_api/flow_group.c
@@ -14,6 +14,7 @@
struct group_lookup_entry_s {
uint64_t ref_counter;
uint32_t *reverse_lookup;
+ uint32_t group_orig;
};
struct group_handle_s {
@@ -83,6 +84,7 @@ int flow_group_translate_get(void *handle, uint8_t owner_id,
uint8_t port_id, ui
if (lookup < group_handle->group_count) {
group_handle->lookup_entries[lookup].reverse_lookup =
table_ptr;
group_handle->lookup_entries[lookup].ref_counter += 1;
+ group_handle->lookup_entries[lookup].group_orig =
group_in;
*table_ptr = lookup;
@@ -97,3 +99,27 @@ int flow_group_translate_get(void *handle, uint8_t owner_id,
uint8_t port_id, ui
*group_out = lookup;
return 0;
}
+
+int flow_group_translate_get_orig_group(void *handle, uint32_t
translated_group,
+ uint32_t *group_orig)
+{
+ struct group_handle_s *group_handle = (struct group_handle_s *)handle;
+ struct group_lookup_entry_s *lookup;
+
+ if (group_handle == NULL || translated_group >=
group_handle->group_count)
+ return -1;
+
+ /* Don't translate group 0 */
+ if (translated_group == 0) {
+ *group_orig = 0;
+ return 0;
+ }
+
+ lookup = &group_handle->lookup_entries[translated_group];
+
+ if (lookup->reverse_lookup && lookup->ref_counter > 0) {
+ *group_orig = lookup->group_orig;
+ return 0;
+ }
+ return -1;
+}
diff --git
a/drivers/net/ntnic/nthw/flow_api/profile_inline/flow_api_hw_db_inline.c
b/drivers/net/ntnic/nthw/flow_api/profile_inline/flow_api_hw_db_inline.c
index 22cbf61b60..278be8b180 100644
--- a/drivers/net/ntnic/nthw/flow_api/profile_inline/flow_api_hw_db_inline.c
+++ b/drivers/net/ntnic/nthw/flow_api/profile_inline/flow_api_hw_db_inline.c
@@ -429,9 +429,14 @@ void hw_db_inline_dump(struct flow_nic_dev *ndev, void
*db_handle, const struct
data->cat.ids, data->km.id1, data->km_ft.id1,
data->action_set.ids);
- if (data->jump)
- fprintf(file, "Jumps to %d\n", data->jump);
-
+ if (data->jump) {
+ uint32_t group_orig = 0;
+ if
(flow_group_translate_get_orig_group(ndev->group_handle,
+ data->jump, &group_orig) < 0)
+ fprintf(file, "Jumps to %d
(encoded)\n", data->jump);
+ else
+ fprintf(file, "Jumps to %d\n",
group_orig);
+ }
break;
}
@@ -440,15 +445,20 @@ void hw_db_inline_dump(struct flow_nic_dev *ndev, void
*db_handle, const struct
&db->action_set[idxs[i].ids].data;
fprintf(file, " ACTION_SET %d\n", idxs[i].ids);
- if (data->contains_jump)
- fprintf(file, "Jumps to %d\n", data->jump);
+ if (data->contains_jump) {
+ uint32_t group_orig = 0;
+ if
(flow_group_translate_get_orig_group(ndev->group_handle,
+ data->jump, &group_orig) < 0)
+ fprintf(file, "Jumps to %d
(encoded)\n", data->jump);
- else
+ else
+ fprintf(file, "Jumps to %d\n",
grou
[PATCH v2 24/34] net/ntnic: remove unused code
From: Danylo Vodopianov
NTNIC currently cupport only 100G link.
Signed-off-by: Danylo Vodopianov
---
drivers/net/ntnic/adapter/nt4ga_stat/nt4ga_stat.c | 9 -
1 file changed, 9 deletions(-)
diff --git a/drivers/net/ntnic/adapter/nt4ga_stat/nt4ga_stat.c
b/drivers/net/ntnic/adapter/nt4ga_stat/nt4ga_stat.c
index 8fedfdcd04..2add43639a 100644
--- a/drivers/net/ntnic/adapter/nt4ga_stat/nt4ga_stat.c
+++ b/drivers/net/ntnic/adapter/nt4ga_stat/nt4ga_stat.c
@@ -215,16 +215,7 @@ static int nt4ga_stat_setup(struct adapter_info_s
*p_adapter_info)
return -1;
}
-#ifdef NIM_TRIGGER
- uint64_t max_bps_speed =
nt_get_max_link_speed(p_adapter_info->nt4ga_link.speed_capa);
-
- if (max_bps_speed == 0)
- max_bps_speed = DEFAULT_MAX_BPS_SPEED;
-
-#else
uint64_t max_bps_speed = DEFAULT_MAX_BPS_SPEED;
- NT_LOG(ERR, NTNIC, "NIM module not included");
-#endif
for (int p = 0; p < NUM_ADAPTER_PORTS_MAX; p++) {
p_nt4ga_stat->mp_port_load[p].rx_bps_max = max_bps_speed;
--
2.45.0
[PATCH v2 26/34] net/ntnic: fix age timeout recalculation into fpga unit
From: Danylo Vodopianov
The FPGA scrub T parameter shall be encoded timeout using internal unit,
which is measured in 2^30 nanoseconds. It is approx 1.074 times longer
than age timeout specified in seconds. Internal method
hw_mod_flm_inf_sta_data_update_get() was updated to perform conversion
between age time and internal FPGA scrub T time unit.
Fixes: c0d2b831 ("net/ntnic: add flow aging event")
Signed-off-by: Danylo Vodopianov
---
.../net/ntnic/nthw/flow_api/hw_mod/hw_mod_flm.c| 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ntnic/nthw/flow_api/hw_mod/hw_mod_flm.c
b/drivers/net/ntnic/nthw/flow_api/hw_mod/hw_mod_flm.c
index 14dd95a150..5cf8264909 100644
--- a/drivers/net/ntnic/nthw/flow_api/hw_mod/hw_mod_flm.c
+++ b/drivers/net/ntnic/nthw/flow_api/hw_mod/hw_mod_flm.c
@@ -969,16 +969,22 @@ int hw_mod_flm_inf_sta_data_update_get(struct
flow_api_backend_s *be, enum hw_fl
*
* (T[7:3] != 0) ? ((8 + T[2:0]) shift-left (T[7:3] - 1)) : T[2:0]
*
- * The maximum allowed value is 0xEF (127 years).
+ * The maximum allowed value is 0xEF (137 years).
*
* Note that this represents a lower bound on the timeout, depending on the
flow
* scanner interval and overall load, the timeout can be substantially longer.
*/
uint32_t hw_mod_flm_scrub_timeout_decode(uint32_t t_enc)
{
- uint8_t t_bits_2_0 = t_enc & 0x07;
- uint8_t t_bits_7_3 = (t_enc >> 3) & 0x1F;
- return t_bits_7_3 != 0 ? ((8 + t_bits_2_0) << (t_bits_7_3 - 1)) :
t_bits_2_0;
+ uint32_t t_bits_2_0 = t_enc & 0x07;
+ uint32_t t_bits_7_3 = (t_enc >> 3) & 0x1F;
+ uint32_t t_dec = t_bits_7_3 != 0 ? ((8U + t_bits_2_0) << (t_bits_7_3 -
1U))
+ : t_bits_2_0;
+ /* convert internal FPGA scrub time unit into seconds, i.e. 2^30/1e9 is
+* approx 1.074 sec per internal unit
+*/
+ uint64_t t_sec = (uint64_t)t_dec * 1074UL / 1000UL;
+ return t_sec > UINT32_MAX ? UINT32_MAX : (uint32_t)t_sec;
}
uint32_t hw_mod_flm_scrub_timeout_encode(uint32_t t)
--
2.45.0
[PATCH v2 29/34] net/ntnic: extend module mapping
FPGA module mapping(to string) was extended with new fpga's modules.
Signed-off-by: Serhii Iliushyk
---
.../nthw/supported/nthw_fpga_mod_str_map.c| 24 +++
1 file changed, 24 insertions(+)
diff --git a/drivers/net/ntnic/nthw/supported/nthw_fpga_mod_str_map.c
b/drivers/net/ntnic/nthw/supported/nthw_fpga_mod_str_map.c
index e8ed7faf0d..459cc724c2 100644
--- a/drivers/net/ntnic/nthw/supported/nthw_fpga_mod_str_map.c
+++ b/drivers/net/ntnic/nthw/supported/nthw_fpga_mod_str_map.c
@@ -6,20 +6,44 @@
#include "nthw_fpga_mod_str_map.h"
const struct nthw_fpga_mod_str_s sa_nthw_fpga_mod_str_map[] = {
+ { MOD_CAT, "CAT" },
+ { MOD_CPY, "CPY" },
+ { MOD_CSU, "CSU" },
+ { MOD_DBS, "DBS" },
+ { MOD_FLM, "FLM" },
{ MOD_GFG, "GFG" },
{ MOD_GMF, "GMF" },
{ MOD_GPIO_PHY, "GPIO_PHY" },
+ { MOD_HFU, "HFU" },
{ MOD_HIF, "HIF" },
+ { MOD_HSH, "HSH" },
{ MOD_I2CM, "I2CM" },
+ { MOD_IFR, "IFR" },
{ MOD_IIC, "IIC" },
+ { MOD_INS, "INS" },
+ { MOD_KM, "KM" },
{ MOD_MAC_PCS, "MAC_PCS" },
{ MOD_PCIE3, "PCIE3" },
+ { MOD_MAC_RX, "MAC_RX" },
+ { MOD_MAC_TX, "MAC_TX" },
{ MOD_PCI_RD_TG, "PCI_RD_TG" },
{ MOD_PCI_WR_TG, "PCI_WR_TG" },
+ { MOD_PCIE3, "PCIE3" },
+ { MOD_PDB, "PDB" },
+ { MOD_QSL, "QSL" },
{ MOD_RAC, "RAC" },
+ { MOD_RMC, "RMC" },
+ { MOD_RPF, "RPF" },
+ { MOD_RPL, "RPL" },
+ { MOD_RPP_LR, "RPP_LR" },
{ MOD_RST9563, "RST9563" },
{ MOD_SDC, "SDC" },
+ { MOD_SLC_LR, "SLC_LR" },
+ { MOD_SLC, "SLC" },
{ MOD_STA, "STA" },
{ MOD_TSM, "TSM" },
+ { MOD_TX_CPY, "TX_CPY" },
+ { MOD_TX_INS, "TX_INS" },
+ { MOD_TX_RPL, "TX_RPL" },
{ 0UL, NULL }
};
--
2.45.0
[PATCH v2 10/34] net/ntnic: fix potentially overflow
From: Danylo Vodopianov
Issue with potentially overflow was fixed.
CID 448944: Unintentional integer overflow (OVERFLOW_BEFORE_WIDEN)
Coverity issue: 448944
Fixes: 339ca124e659 ("net/ntnic: add flow action modify field")
Signed-off-by: Danylo Vodopianov
---
.../nthw/flow_api/profile_inline/flow_api_profile_inline.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git
a/drivers/net/ntnic/nthw/flow_api/profile_inline/flow_api_profile_inline.c
b/drivers/net/ntnic/nthw/flow_api/profile_inline/flow_api_profile_inline.c
index 535047d246..a68c3ea702 100644
--- a/drivers/net/ntnic/nthw/flow_api/profile_inline/flow_api_profile_inline.c
+++ b/drivers/net/ntnic/nthw/flow_api/profile_inline/flow_api_profile_inline.c
@@ -1632,7 +1632,7 @@ static int interpret_flow_actions(const struct
flow_eth_dev *dev,
return -1;
}
- modify_field_use_flag = 1
+ modify_field_use_flag = (uint64_t)1
<<
fd->modify_field[fd->modify_field_count].select;
if (modify_field_use_flag &
modify_field_use_flags) {
--
2.45.0
[PATCH v2 02/34] net/ntnic: add thread check return code
From: Danylo Vodopianov
CI found couple coverity problems which were fixed in this commit.
CID: 448965 Error handling issues (CHECKED_RETURN).
Thread return code check was added.
Coverity issue: 448965
Fixes: a1ba8c473f5c ("net/ntnic: add statistics poll")
Signed-off-by: Danylo Vodopianov
---
drivers/net/ntnic/ntnic_ethdev.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ntnic/ntnic_ethdev.c b/drivers/net/ntnic/ntnic_ethdev.c
index 2a2643a106..620d023a71 100644
--- a/drivers/net/ntnic/ntnic_ethdev.c
+++ b/drivers/net/ntnic/ntnic_ethdev.c
@@ -2516,8 +2516,11 @@ static int init_shutdown(void)
NT_LOG(DBG, NTNIC, "Starting shutdown handler");
kill_pmd = 0;
previous_handler = signal(SIGINT, signal_handler_func_int);
- THREAD_CREATE(&shutdown_tid, shutdown_thread, NULL);
-
+ int ret = THREAD_CREATE(&shutdown_tid, shutdown_thread, NULL);
+ if (ret != 0) {
+ NT_LOG(ERR, NTNIC, "Failed to create shutdown thread, error
code: %d", ret);
+ return -1;
+ }
/*
* 1 time calculation of 1 sec stat update rtc cycles to prevent stat
poll
* flooding by OVS from multiple virtual port threads - no need to be
precise
--
2.45.0
Re: [PATCH v2 03/54] net/igc: remove the driver
On Tue, Feb 04, 2025 at 03:10:09PM +, Anatoly Burakov wrote: > Now that e1000 and igc drivers have been merged, remove the igc driver > directory. This removes: > > - igc base driver directory together with all files in it > - igc logs header file > - igc and igc base meson build files > - igc README file > > Signed-off-by: Anatoly Burakov > --- At this point, you should also remove MAINTAINERS file entry and any other such general references to igc (docs perhaps)?
[PATCH v2 17/34] net/ntnic: fix var overflow
From: Danylo Vodopianov
Casting either buf_size and num_descr to uint64_t before performing the
multiplication was done.
Coverity issue: 446740
Fixes: 6b0047fadf41 ("net/ntnic: add queue setup operations")
Signed-off-by: Danylo Vodopianov
---
drivers/net/ntnic/ntnic_ethdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ntnic/ntnic_ethdev.c b/drivers/net/ntnic/ntnic_ethdev.c
index 28b086c009..6fcbb8fa9b 100644
--- a/drivers/net/ntnic/ntnic_ethdev.c
+++ b/drivers/net/ntnic/ntnic_ethdev.c
@@ -790,7 +790,7 @@ static int allocate_hw_virtio_queues(struct rte_eth_dev
*eth_dev, int vf_num, st
NT_LOG(DBG, NTNIC, "* Configure IOMMU for HW queues on VF %i
*", vf_num);
/* Just allocate 1MB to hold all combined descr rings */
- uint64_t tot_alloc_size = 0x10 + buf_size * num_descr;
+ uint64_t tot_alloc_size = 0x10 + (uint64_t)buf_size *
(uint64_t)num_descr;
void *virt =
rte_malloc_socket("VirtQDescr", tot_alloc_size,
nt_util_align_size(tot_alloc_size),
--
2.45.0
[PATCH v2 12/34] net/ntnic: fix overflow issue
From: Danylo Vodopianov
Fix overflow issue with checking max bit shifting value.
Coverity issue: 448921
Fixes: 833962ebb893 ("net/ntnic: add CAT module")
Signed-off-by: Danylo Vodopianov
---
.../nthw/flow_api/profile_inline/flow_api_profile_inline.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git
a/drivers/net/ntnic/nthw/flow_api/profile_inline/flow_api_profile_inline.c
b/drivers/net/ntnic/nthw/flow_api/profile_inline/flow_api_profile_inline.c
index a68c3ea702..574e51c2fa 100644
--- a/drivers/net/ntnic/nthw/flow_api/profile_inline/flow_api_profile_inline.c
+++ b/drivers/net/ntnic/nthw/flow_api/profile_inline/flow_api_profile_inline.c
@@ -3605,7 +3605,7 @@ static struct flow_handle *create_flow_filter(struct
flow_eth_dev *dev, struct n
.err_mask_ttl = (fd->ttl_sub_enable &&
fd->ttl_sub_outer) ? -1 : 0x1,
.ptc_mask_tunnel = fd->tunnel_prot !=
- -1 ? (1 << fd->tunnel_prot) : -1,
+ -1 ? (1 << (fd->tunnel_prot > 10 ? 10 :
fd->tunnel_prot)) : -1,
.ptc_mask_l3_tunnel =
fd->tunnel_l3_prot != -1 ? (1 <<
fd->tunnel_l3_prot) : -1,
.ptc_mask_l4_tunnel =
--
2.45.0
[PATCH v2 27/34] net/ntnic: rework age event generation
From: Danylo Vodopianov
Add age event generating only for physical ports.
Fixes: 4033e0539435 ("net/ntnic: add flow meter")
Signed-off-by: Danylo Vodopianov
---
.../nthw/flow_api/profile_inline/flow_api_profile_inline.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git
a/drivers/net/ntnic/nthw/flow_api/profile_inline/flow_api_profile_inline.c
b/drivers/net/ntnic/nthw/flow_api/profile_inline/flow_api_profile_inline.c
index e5abd372bc..e911860c38 100644
--- a/drivers/net/ntnic/nthw/flow_api/profile_inline/flow_api_profile_inline.c
+++ b/drivers/net/ntnic/nthw/flow_api/profile_inline/flow_api_profile_inline.c
@@ -479,13 +479,16 @@ static void flm_mtr_read_inf_records(struct flow_eth_dev
*dev, uint32_t *data, u
struct flow_handle *fh = (struct flow_handle
*)flm_h.p;
struct flm_age_event_s age_event;
uint8_t port;
+ bool is_remote;
age_event.context = fh->context;
- is_remote_caller(caller_id, &port);
+ is_remote = is_remote_caller(caller_id, &port);
flm_age_queue_put(caller_id, &age_event);
- flm_age_event_set(port);
+ /* age events are supported only for physical
ports */
+ if (!is_remote)
+ flm_age_event_set(port);
}
break;
--
2.45.0
[PATCH v2 06/34] net/ntnic: fix array index verification
From: Danylo Vodopianov
CI found couple coverity problems which were fixed in this commit.
CID: 448983 Out-of-bounds write (OVERRUN).
These issues were fixed with updating index verification statement.
Coverity issue: 448983
Fixes: 96c8249be53e ("net/ntnic: learn flow queue handling")
Signed-off-by: Danylo Vodopianov
---
drivers/net/ntnic/ntnic_ethdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ntnic/ntnic_ethdev.c b/drivers/net/ntnic/ntnic_ethdev.c
index 620d023a71..28b086c009 100644
--- a/drivers/net/ntnic/ntnic_ethdev.c
+++ b/drivers/net/ntnic/ntnic_ethdev.c
@@ -140,7 +140,7 @@ store_pdrv(struct drv_s *p_drv)
static void clear_pdrv(struct drv_s *p_drv)
{
- if (p_drv->adapter_no > NUM_ADAPTER_MAX)
+ if (p_drv->adapter_no >= NUM_ADAPTER_MAX)
return;
rte_spinlock_lock(&hwlock);
--
2.45.0
Re: [PATCH v3 0/4] remove common iavf and idpf drivers
On Thu, Jan 30, 2025 at 4:12 PM Bruce Richardson wrote: > > The iavf and idpf common directories were used only to share code > between multiple net drivers and did not need to be drivers in their own > right, since it is just as easy to have a dependency from one net driver > on another as a net driver on a common one. > > This patchset therefore aims to eliminate the two unnecessary common > drivers. It does so as follows: > > * merging common/idpf into net/idpf and updating the cpfl dependency to > point to the net driver. > * merging common/iavf into net/iavf and similarly updating the > dependencies, including the paths from idpf (which does not directly > depend on iavf, but does make use of the definitions in the iavf > header files). > > Separately, two other cleanups are done - one to remove an unnecessary > warning disable flag. The second is a little more complex - it makes the > dependency between ice and iavf an optional one, by having ice compile > in the necessary iavf shared code files in case iavf is disabled in the > build. > > v3: add libabigail exclusions for removed libs > v2: include Release note updates v3 has some meson style issues. UNH is picky about this as it (recently started to?) runs check-meson.py. $ ./devtools/check-meson.py Error: Incorrect indent at drivers/net/intel/ice/meson.build:29 Error: Incorrect indent at drivers/net/intel/ice/meson.build:30 Error: Incorrect indent at drivers/net/intel/ice/meson.build:31 -- David Marchand
[PATCH v2 31/34] net/ntnic: remove shutdown thread
Remove the shutdown thread and SIGINT handling on the level of PMD
Signed-off-by: Serhii Iliushyk
---
drivers/net/ntnic/ntnic_ethdev.c | 63
1 file changed, 7 insertions(+), 56 deletions(-)
diff --git a/drivers/net/ntnic/ntnic_ethdev.c b/drivers/net/ntnic/ntnic_ethdev.c
index 9000264804..e65be67c44 100644
--- a/drivers/net/ntnic/ntnic_ethdev.c
+++ b/drivers/net/ntnic/ntnic_ethdev.c
@@ -76,10 +76,6 @@ const rte_thread_attr_t thread_attr = { .priority =
RTE_THREAD_PRIORITY_NORMAL }
uint64_t rte_tsc_freq;
-static void (*previous_handler)(int sig);
-static rte_thread_t shutdown_tid;
-
-int kill_pmd;
#define ETH_DEV_NTNIC_HELP_ARG "help"
#define ETH_DEV_NTHW_RXQUEUES_ARG "rxqs"
@@ -480,9 +476,6 @@ static uint16_t eth_dev_rx_scg(void *queue, struct rte_mbuf
**bufs, uint16_t nb_
struct nthw_received_packets hw_recv[MAX_RX_PACKETS];
- if (kill_pmd)
- return 0;
-
if (unlikely(nb_pkts == 0))
return 0;
@@ -693,9 +686,6 @@ static uint16_t eth_dev_tx_scg(void *queue, struct rte_mbuf
**bufs, uint16_t nb_
int pkts_sent = 0;
uint16_t nb_segs_arr[MAX_TX_PACKETS];
- if (kill_pmd)
- return 0;
-
if (nb_pkts > MAX_TX_PACKETS)
nb_pkts = MAX_TX_PACKETS;
@@ -2490,51 +2480,6 @@ nthw_pci_dev_deinit(struct rte_eth_dev *eth_dev
__rte_unused)
return 0;
}
-static void signal_handler_func_int(int sig)
-{
- if (sig != SIGINT) {
- signal(sig, previous_handler);
- raise(sig);
- return;
- }
-
- kill_pmd = 1;
-}
-
-THREAD_FUNC shutdown_thread(void *arg __rte_unused)
-{
- while (!kill_pmd)
- nt_os_wait_usec(100 * 1000);
-
- NT_LOG_DBGX(DBG, NTNIC, "Shutting down because of ctrl+C");
-
- signal(SIGINT, previous_handler);
- raise(SIGINT);
-
- return THREAD_RETURN;
-}
-
-static int init_shutdown(void)
-{
- NT_LOG(DBG, NTNIC, "Starting shutdown handler");
- kill_pmd = 0;
- previous_handler = signal(SIGINT, signal_handler_func_int);
- int ret = THREAD_CREATE(&shutdown_tid, shutdown_thread, NULL);
- if (ret != 0) {
- NT_LOG(ERR, NTNIC, "Failed to create shutdown thread, error
code: %d", ret);
- return -1;
- }
- /*
-* 1 time calculation of 1 sec stat update rtc cycles to prevent stat
poll
-* flooding by OVS from multiple virtual port threads - no need to be
precise
-*/
- uint64_t now_rtc = rte_get_tsc_cycles();
- nt_os_wait_usec(10 * 1000);
- rte_tsc_freq = 100 * (rte_get_tsc_cycles() - now_rtc);
-
- return 0;
-}
-
static int
nthw_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
struct rte_pci_device *pci_dev)
@@ -2577,7 +2522,13 @@ nthw_pci_probe(struct rte_pci_driver *pci_drv
__rte_unused,
ret = nthw_pci_dev_init(pci_dev);
- init_shutdown();
+ /*
+* 1 time calculation of 1 sec stat update rtc cycles to prevent stat
poll
+* flooding by OVS from multiple virtual port threads - no need to be
precise
+*/
+ uint64_t now_rtc = rte_get_tsc_cycles();
+ nt_os_wait_usec(10 * 1000);
+ rte_tsc_freq = 100 * (rte_get_tsc_cycles() - now_rtc);
NT_LOG_DBGX(DBG, NTNIC, "leave: ret=%d", ret);
return ret;
--
2.45.0
Re: [PATCH v2 02/54] net/e1000: merge igc with e1000
On Tue, Feb 04, 2025 at 03:10:08PM +, Anatoly Burakov wrote:
> IGC and E1000 drivers are derived from the same base code. Now that e1000
> code has enabled support for i225 devices, move IGC ethdev code to e1000
> directory (renaming references to base code from igc_* to e1000_*).
>
> Signed-off-by: Anatoly Burakov
> ---
> drivers/net/intel/{igc => e1000}/igc_ethdev.c | 910 +-
> drivers/net/intel/{igc => e1000}/igc_ethdev.h | 32 +-
> drivers/net/intel/{igc => e1000}/igc_filter.c | 84 +-
> drivers/net/intel/{igc => e1000}/igc_filter.h | 0
> drivers/net/intel/{igc => e1000}/igc_flow.c | 2 +-
> drivers/net/intel/{igc => e1000}/igc_flow.h | 0
> drivers/net/intel/{igc => e1000}/igc_logs.c | 2 +-
> drivers/net/intel/{igc => e1000}/igc_txrx.c | 376
> drivers/net/intel/{igc => e1000}/igc_txrx.h | 6 +-
> drivers/net/intel/e1000/meson.build | 11 +
> drivers/net/meson.build | 1 -
> 11 files changed, 717 insertions(+), 707 deletions(-)
> rename drivers/net/intel/{igc => e1000}/igc_ethdev.c (73%)
> rename drivers/net/intel/{igc => e1000}/igc_ethdev.h (91%)
> rename drivers/net/intel/{igc => e1000}/igc_filter.c (81%)
> rename drivers/net/intel/{igc => e1000}/igc_filter.h (100%)
> rename drivers/net/intel/{igc => e1000}/igc_flow.c (99%)
> rename drivers/net/intel/{igc => e1000}/igc_flow.h (100%)
> rename drivers/net/intel/{igc => e1000}/igc_logs.c (90%)
> rename drivers/net/intel/{igc => e1000}/igc_txrx.c (87%)
> rename drivers/net/intel/{igc => e1000}/igc_txrx.h (97%)
>
Either in this patch or in the next one, the igc documentation needs an
update to inform users it is merged into e1000. Interestingly, looking at
[1] there is no chapter for e1000 - just for igb, which is strange. Maybe
as part of this merge, you could rename that chapter to Intel 1G/2.5G driver?
/Bruce
[1] https://doc.dpdk.org/guides/nics/index.html
[PATCH v4 3/4] drivers: move iavf common folder to iavf net
The common/iavf driver folder contains the base code for the iavf
driver, which is also linked against by the ice driver and others.
However, there is no need for this to be in common, and we can
move it to the net/intel/iavf as a base code driver. This involves
updating dependencies that were on common/iavf to net/iavf
Signed-off-by: Bruce Richardson
---
devtools/libabigail.abignore | 1 +
doc/guides/rel_notes/release_25_03.rst | 5 -
drivers/common/iavf/version.map| 13 -
drivers/common/meson.build | 1 -
.../{common/iavf => net/intel/iavf/base}/README| 0
.../iavf => net/intel/iavf/base}/iavf_adminq.c | 0
.../iavf => net/intel/iavf/base}/iavf_adminq.h | 0
.../iavf => net/intel/iavf/base}/iavf_adminq_cmd.h | 0
.../iavf => net/intel/iavf/base}/iavf_alloc.h | 0
.../iavf => net/intel/iavf/base}/iavf_common.c | 0
.../iavf => net/intel/iavf/base}/iavf_devids.h | 0
.../iavf => net/intel/iavf/base}/iavf_impl.c | 0
.../iavf => net/intel/iavf/base}/iavf_osdep.h | 0
.../iavf => net/intel/iavf/base}/iavf_prototype.h | 0
.../iavf => net/intel/iavf/base}/iavf_register.h | 0
.../iavf => net/intel/iavf/base}/iavf_status.h | 0
.../iavf => net/intel/iavf/base}/iavf_type.h | 0
.../iavf => net/intel/iavf/base}/meson.build | 0
.../iavf => net/intel/iavf/base}/virtchnl.h| 0
.../intel/iavf/base}/virtchnl_inline_ipsec.h | 0
drivers/net/intel/iavf/meson.build | 13 +
drivers/net/intel/iavf/version.map | 14 ++
drivers/net/intel/ice/meson.build | 7 +++
drivers/net/intel/idpf/meson.build | 2 +-
24 files changed, 32 insertions(+), 24 deletions(-)
delete mode 100644 drivers/common/iavf/version.map
rename drivers/{common/iavf => net/intel/iavf/base}/README (100%)
rename drivers/{common/iavf => net/intel/iavf/base}/iavf_adminq.c (100%)
rename drivers/{common/iavf => net/intel/iavf/base}/iavf_adminq.h (100%)
rename drivers/{common/iavf => net/intel/iavf/base}/iavf_adminq_cmd.h (100%)
rename drivers/{common/iavf => net/intel/iavf/base}/iavf_alloc.h (100%)
rename drivers/{common/iavf => net/intel/iavf/base}/iavf_common.c (100%)
rename drivers/{common/iavf => net/intel/iavf/base}/iavf_devids.h (100%)
rename drivers/{common/iavf => net/intel/iavf/base}/iavf_impl.c (100%)
rename drivers/{common/iavf => net/intel/iavf/base}/iavf_osdep.h (100%)
rename drivers/{common/iavf => net/intel/iavf/base}/iavf_prototype.h (100%)
rename drivers/{common/iavf => net/intel/iavf/base}/iavf_register.h (100%)
rename drivers/{common/iavf => net/intel/iavf/base}/iavf_status.h (100%)
rename drivers/{common/iavf => net/intel/iavf/base}/iavf_type.h (100%)
rename drivers/{common/iavf => net/intel/iavf/base}/meson.build (100%)
rename drivers/{common/iavf => net/intel/iavf/base}/virtchnl.h (100%)
rename drivers/{common/iavf => net/intel/iavf/base}/virtchnl_inline_ipsec.h
(100%)
diff --git a/devtools/libabigail.abignore b/devtools/libabigail.abignore
index 1dee6a954f..4d7079abbc 100644
--- a/devtools/libabigail.abignore
+++ b/devtools/libabigail.abignore
@@ -26,6 +26,7 @@
; SKIP_LIBRARY=librte_common_mlx5_glue
; SKIP_LIBRARY=librte_net_mlx4_glue
; SKIP_LIBRARY=librte_common_idpf
+; SKIP_LIBRARY=librte_common_iavf
; Experimental APIs exceptions ;
diff --git a/doc/guides/rel_notes/release_25_03.rst
b/doc/guides/rel_notes/release_25_03.rst
index 79b1116f6e..1f22efa79f 100644
--- a/doc/guides/rel_notes/release_25_03.rst
+++ b/doc/guides/rel_notes/release_25_03.rst
@@ -116,9 +116,12 @@ API Changes
For example, ``-Denable_drivers=/net/i40e`` becomes
``-Denable_drivers=/net/intel/i40e``.
* The driver ``common/idpf`` has been merged into the ``net/intel/idpf``
driver.
- This change should have no impact to end applications, but,
+ Similarly, the ``common/iavf`` driver has been merged into the
``net/intel/iavf`` driver.
+ These changes should have no impact to end applications, but,
when specifying the ``idpf`` or ``cpfl`` net drivers to meson via
``-Denable_drivers`` option,
there is no longer any need to also specify the ``common/idpf`` driver.
+ In the same way, when specifying the ``iavf`` or ``ice`` net drivers,
+ there is no need to also specify the ``common/iavf`` driver.
Note, however, ``net/intel/cpfl`` driver now depends upon the
``net/intel/idpf`` driver.
diff --git a/drivers/common/iavf/version.map b/drivers/common/iavf/version.map
deleted file mode 100644
index 6c1427cca4..00
--- a/drivers/common/iavf/version.map
+++ /dev/null
@@ -1,13 +0,0 @@
-INTERNAL {
- global:
-
- iavf_aq_send_msg_to_pf;
- iavf_clean_arq_element;
- iavf_init_adminq;
- iavf_set_mac_type;
- iavf_shutdown_adminq;
- iavf_vf_parse_hw_config;
- iavf_vf_reset;
-
-
[PATCH v4 1/4] drivers: merge common and net idpf drivers
Rather than having some of the idpf code split out into the "common"
directory, used by both a net/idpf and a net/cpfl driver, we can
merge all idpf code together under net/idpf and have the cpfl driver
depend on "net/idpf" rather than "common/idpf".
Signed-off-by: Bruce Richardson
Acked-by: Praveen Shetty
---
devtools/libabigail.abignore | 1 +
doc/guides/rel_notes/release_25_03.rst| 6
drivers/common/idpf/meson.build | 34 ---
drivers/common/meson.build| 1 -
drivers/net/intel/cpfl/meson.build| 2 +-
.../{common => net/intel}/idpf/base/README| 0
.../intel}/idpf/base/idpf_alloc.h | 0
.../intel}/idpf/base/idpf_controlq.c | 0
.../intel}/idpf/base/idpf_controlq.h | 0
.../intel}/idpf/base/idpf_controlq_api.h | 0
.../intel}/idpf/base/idpf_controlq_setup.c| 0
.../intel}/idpf/base/idpf_devids.h| 0
.../intel}/idpf/base/idpf_lan_pf_regs.h | 0
.../intel}/idpf/base/idpf_lan_txrx.h | 0
.../intel}/idpf/base/idpf_lan_vf_regs.h | 0
.../intel}/idpf/base/idpf_osdep.h | 0
.../intel}/idpf/base/idpf_prototype.h | 0
.../intel}/idpf/base/idpf_type.h | 0
.../intel}/idpf/base/meson.build | 0
.../intel}/idpf/base/siov_regs.h | 0
.../intel}/idpf/base/virtchnl2.h | 0
.../intel}/idpf/base/virtchnl2_lan_desc.h | 0
.../intel}/idpf/idpf_common_device.c | 0
.../intel}/idpf/idpf_common_device.h | 0
.../intel}/idpf/idpf_common_logs.h| 0
.../intel}/idpf/idpf_common_rxtx.c| 0
.../intel}/idpf/idpf_common_rxtx.h| 0
.../intel}/idpf/idpf_common_rxtx_avx512.c | 0
.../intel}/idpf/idpf_common_virtchnl.c| 0
.../intel}/idpf/idpf_common_virtchnl.h| 0
drivers/net/intel/idpf/meson.build| 20 +--
.../{common => net/intel}/idpf/version.map| 0
drivers/net/meson.build | 2 +-
33 files changed, 27 insertions(+), 39 deletions(-)
delete mode 100644 drivers/common/idpf/meson.build
rename drivers/{common => net/intel}/idpf/base/README (100%)
rename drivers/{common => net/intel}/idpf/base/idpf_alloc.h (100%)
rename drivers/{common => net/intel}/idpf/base/idpf_controlq.c (100%)
rename drivers/{common => net/intel}/idpf/base/idpf_controlq.h (100%)
rename drivers/{common => net/intel}/idpf/base/idpf_controlq_api.h (100%)
rename drivers/{common => net/intel}/idpf/base/idpf_controlq_setup.c (100%)
rename drivers/{common => net/intel}/idpf/base/idpf_devids.h (100%)
rename drivers/{common => net/intel}/idpf/base/idpf_lan_pf_regs.h (100%)
rename drivers/{common => net/intel}/idpf/base/idpf_lan_txrx.h (100%)
rename drivers/{common => net/intel}/idpf/base/idpf_lan_vf_regs.h (100%)
rename drivers/{common => net/intel}/idpf/base/idpf_osdep.h (100%)
rename drivers/{common => net/intel}/idpf/base/idpf_prototype.h (100%)
rename drivers/{common => net/intel}/idpf/base/idpf_type.h (100%)
rename drivers/{common => net/intel}/idpf/base/meson.build (100%)
rename drivers/{common => net/intel}/idpf/base/siov_regs.h (100%)
rename drivers/{common => net/intel}/idpf/base/virtchnl2.h (100%)
rename drivers/{common => net/intel}/idpf/base/virtchnl2_lan_desc.h (100%)
rename drivers/{common => net/intel}/idpf/idpf_common_device.c (100%)
rename drivers/{common => net/intel}/idpf/idpf_common_device.h (100%)
rename drivers/{common => net/intel}/idpf/idpf_common_logs.h (100%)
rename drivers/{common => net/intel}/idpf/idpf_common_rxtx.c (100%)
rename drivers/{common => net/intel}/idpf/idpf_common_rxtx.h (100%)
rename drivers/{common => net/intel}/idpf/idpf_common_rxtx_avx512.c (100%)
rename drivers/{common => net/intel}/idpf/idpf_common_virtchnl.c (100%)
rename drivers/{common => net/intel}/idpf/idpf_common_virtchnl.h (100%)
rename drivers/{common => net/intel}/idpf/version.map (100%)
diff --git a/devtools/libabigail.abignore b/devtools/libabigail.abignore
index 21b8cd6113..1dee6a954f 100644
--- a/devtools/libabigail.abignore
+++ b/devtools/libabigail.abignore
@@ -25,6 +25,7 @@
;
; SKIP_LIBRARY=librte_common_mlx5_glue
; SKIP_LIBRARY=librte_net_mlx4_glue
+; SKIP_LIBRARY=librte_common_idpf
; Experimental APIs exceptions ;
diff --git a/doc/guides/rel_notes/release_25_03.rst
b/doc/guides/rel_notes/release_25_03.rst
index a88b04d958..79b1116f6e 100644
--- a/doc/guides/rel_notes/release_25_03.rst
+++ b/doc/guides/rel_notes/release_25_03.rst
@@ -115,6 +115,12 @@ API Changes
but to enable/disable these drivers via Meson option requires use of the new
paths.
For example, ``-Denable_drivers=/net/i40e`` becomes
``-Denable_drivers=/net/intel/i40e``.
+* The driver ``common/idpf`` has been merged into the ``net/intel/idpf``
driver.
+ This change should have no impact to end applications, but,
+ when specifying th
[PATCH v4 0/4] remove common iavf and idpf drivers
The iavf and idpf common directories were used only to share code
between multiple net drivers and did not need to be drivers in their own
right, since it is just as easy to have a dependency from one net driver
on another as a net driver on a common one.
This patchset therefore aims to eliminate the two unnecessary common
drivers. It does so as follows:
* merging common/idpf into net/idpf and updating the cpfl dependency to
point to the net driver.
* merging common/iavf into net/iavf and similarly updating the
dependencies, including the paths from idpf (which does not directly
depend on iavf, but does make use of the definitions in the iavf
header files).
Separately, two other cleanups are done - one to remove an unnecessary
warning disable flag. The second is a little more complex - it makes the
dependency between ice and iavf an optional one, by having ice compile
in the necessary iavf shared code files in case iavf is disabled in the
build.
v4: fix meson indentation issue flagged by check-meson.py
v3: add libabigail exclusions for removed libs
v2: include Release note updates
Bruce Richardson (4):
drivers: merge common and net idpf drivers
net/idpf: re-enable unused variable warnings
drivers: move iavf common folder to iavf net
net/intel: allow building ice driver without iavf
devtools/libabigail.abignore | 2 ++
doc/guides/rel_notes/release_25_03.rst| 9 +
drivers/common/iavf/version.map | 13 ---
drivers/common/idpf/meson.build | 34 ---
drivers/common/meson.build| 2 --
drivers/net/intel/cpfl/meson.build| 2 +-
.../iavf => net/intel/iavf/base}/README | 0
.../intel/iavf/base}/iavf_adminq.c| 0
.../intel/iavf/base}/iavf_adminq.h| 0
.../intel/iavf/base}/iavf_adminq_cmd.h| 0
.../iavf => net/intel/iavf/base}/iavf_alloc.h | 0
.../intel/iavf/base}/iavf_common.c| 0
.../intel/iavf/base}/iavf_devids.h| 0
.../iavf => net/intel/iavf/base}/iavf_impl.c | 0
.../iavf => net/intel/iavf/base}/iavf_osdep.h | 0
.../intel/iavf/base}/iavf_prototype.h | 8 +
.../intel/iavf/base}/iavf_register.h | 0
.../intel/iavf/base}/iavf_status.h| 0
.../iavf => net/intel/iavf/base}/iavf_type.h | 0
.../iavf => net/intel/iavf/base}/meson.build | 0
.../iavf => net/intel/iavf/base}/virtchnl.h | 0
.../intel/iavf/base}/virtchnl_inline_ipsec.h | 0
drivers/net/intel/iavf/meson.build| 13 ---
drivers/net/intel/iavf/version.map| 14
drivers/net/intel/ice/meson.build | 18 +++---
.../{common => net/intel}/idpf/base/README| 0
.../intel}/idpf/base/idpf_alloc.h | 0
.../intel}/idpf/base/idpf_controlq.c | 0
.../intel}/idpf/base/idpf_controlq.h | 0
.../intel}/idpf/base/idpf_controlq_api.h | 0
.../intel}/idpf/base/idpf_controlq_setup.c| 0
.../intel}/idpf/base/idpf_devids.h| 0
.../intel}/idpf/base/idpf_lan_pf_regs.h | 0
.../intel}/idpf/base/idpf_lan_txrx.h | 0
.../intel}/idpf/base/idpf_lan_vf_regs.h | 0
.../intel}/idpf/base/idpf_osdep.h | 0
.../intel}/idpf/base/idpf_prototype.h | 0
.../intel}/idpf/base/idpf_type.h | 0
.../intel}/idpf/base/meson.build | 9 -
.../intel}/idpf/base/siov_regs.h | 0
.../intel}/idpf/base/virtchnl2.h | 0
.../intel}/idpf/base/virtchnl2_lan_desc.h | 0
.../intel}/idpf/idpf_common_device.c | 0
.../intel}/idpf/idpf_common_device.h | 0
.../intel}/idpf/idpf_common_logs.h| 0
.../intel}/idpf/idpf_common_rxtx.c| 2 --
.../intel}/idpf/idpf_common_rxtx.h| 0
.../intel}/idpf/idpf_common_rxtx_avx512.c | 0
.../intel}/idpf/idpf_common_virtchnl.c| 4 +--
.../intel}/idpf/idpf_common_virtchnl.h| 0
drivers/net/intel/idpf/meson.build| 20 +--
.../{common => net/intel}/idpf/version.map| 0
drivers/net/meson.build | 2 +-
53 files changed, 78 insertions(+), 74 deletions(-)
delete mode 100644 drivers/common/iavf/version.map
delete mode 100644 drivers/common/idpf/meson.build
rename drivers/{common/iavf => net/intel/iavf/base}/README (100%)
rename drivers/{common/iavf => net/intel/iavf/base}/iavf_adminq.c (100%)
rename drivers/{common/iavf => net/intel/iavf/base}/iavf_adminq.h (100%)
rename drivers/{common/iavf => net/intel/iavf/base}/iavf_adminq_cmd.h (100%)
rename drivers/{common/iavf => net/intel/iavf/base}/iavf_alloc.h (100%)
rename drivers/{common/iavf => net/intel/iavf/base}/iavf_common.c (100%)
rename drivers/{common/iavf => net/intel/iavf/base}/iavf_devids.h (100%)
rename drivers/{common/iavf => net/intel/iavf/base}/iavf_impl.c (100%)
rename drivers/{common/iavf => net/intel/iavf/base}/iavf_osdep.h (100
[PATCH v4 2/4] net/idpf: re-enable unused variable warnings
The idpf driver was being built with warnings disabled for unused
variables. However, while the warning was being disabled in the base
code directory, all suppressed warnings were in the main directory.
Therefore, just remove the unused variables and re-enable the
warnings.
Signed-off-by: Bruce Richardson
Acked-by: Praveen Shetty
---
drivers/net/intel/idpf/base/meson.build | 9 -
drivers/net/intel/idpf/idpf_common_rxtx.c | 2 --
drivers/net/intel/idpf/idpf_common_virtchnl.c | 4 ++--
3 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/drivers/net/intel/idpf/base/meson.build
b/drivers/net/intel/idpf/base/meson.build
index f30ec7dfc2..7316e0a805 100644
--- a/drivers/net/intel/idpf/base/meson.build
+++ b/drivers/net/intel/idpf/base/meson.build
@@ -5,12 +5,3 @@ sources += files(
'idpf_controlq.c',
'idpf_controlq_setup.c',
)
-
-error_cflags = [
-'-Wno-unused-variable',
-]
-foreach flag: error_cflags
-if cc.has_argument(flag)
-cflags += flag
-endif
-endforeach
diff --git a/drivers/net/intel/idpf/idpf_common_rxtx.c
b/drivers/net/intel/idpf/idpf_common_rxtx.c
index a04e54ce26..9b17279181 100644
--- a/drivers/net/intel/idpf/idpf_common_rxtx.c
+++ b/drivers/net/intel/idpf/idpf_common_rxtx.c
@@ -1178,7 +1178,6 @@ idpf_dp_singleq_recv_scatter_pkts(void *rx_queue, struct
rte_mbuf **rx_pkts,
struct rte_mbuf *last_seg = rxq->pkt_last_seg;
struct rte_mbuf *rxm;
struct rte_mbuf *nmb;
- struct rte_eth_dev *dev;
const uint32_t *ptype_tbl = rxq->adapter->ptype_tbl;
uint16_t rx_id = rxq->rx_tail;
uint16_t rx_packet_len;
@@ -1310,7 +1309,6 @@ idpf_xmit_cleanup(struct idpf_tx_queue *txq)
uint16_t nb_tx_desc = txq->nb_tx_desc;
uint16_t desc_to_clean_to;
uint16_t nb_tx_to_clean;
- uint16_t i;
volatile struct idpf_base_tx_desc *txd = txq->tx_ring;
diff --git a/drivers/net/intel/idpf/idpf_common_virtchnl.c
b/drivers/net/intel/idpf/idpf_common_virtchnl.c
index de511da788..0ae1d55d79 100644
--- a/drivers/net/intel/idpf/idpf_common_virtchnl.c
+++ b/drivers/net/intel/idpf/idpf_common_virtchnl.c
@@ -362,7 +362,7 @@ idpf_vc_queue_grps_add(struct idpf_vport *vport,
{
struct idpf_adapter *adapter = vport->adapter;
struct idpf_cmd_info args;
- int size, qg_info_size;
+ int size;
int err = -1;
size = sizeof(*p2p_queue_grps_info) +
@@ -1044,7 +1044,7 @@ int idpf_vc_rxq_config_by_info(struct idpf_vport *vport,
struct virtchnl2_rxq_in
struct idpf_adapter *adapter = vport->adapter;
struct virtchnl2_config_rx_queues *vc_rxqs = NULL;
struct idpf_cmd_info args;
- int size, err, i;
+ int size, err;
size = sizeof(*vc_rxqs) + (num_qs - 1) *
sizeof(struct virtchnl2_rxq_info);
--
2.43.0
[PATCH v4 4/4] net/intel: allow building ice driver without iavf
The ice PMD relies on a number of functions from the iavf base code,
which can be got by linking against that iavf driver. However, since
only three C files are necessary here, we can allow ice to be built
independently of iavf by including the base files directly in cases
where iavf is not part of the build. If it is part of the build, the
dependency remains as now.
Signed-off-by: Bruce Richardson
---
drivers/net/intel/iavf/base/iavf_prototype.h | 8
drivers/net/intel/ice/meson.build| 13 -
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/net/intel/iavf/base/iavf_prototype.h
b/drivers/net/intel/iavf/base/iavf_prototype.h
index 7c43a817bb..5d2ee0a785 100644
--- a/drivers/net/intel/iavf/base/iavf_prototype.h
+++ b/drivers/net/intel/iavf/base/iavf_prototype.h
@@ -11,6 +11,14 @@
#include
+/* functions only need exporting if this is being built into
+ * iavf driver itself. If included in ice driver, then no export
+ */
+#ifndef RTE_NET_IAVF
+#undef __rte_internal
+#define __rte_internal
+#endif
+
/* Prototypes for shared code functions that are not in
* the standard function pointer structures. These are
* mostly because they are needed even before the init
diff --git a/drivers/net/intel/ice/meson.build
b/drivers/net/intel/ice/meson.build
index 5faf887386..ff7f84597a 100644
--- a/drivers/net/intel/ice/meson.build
+++ b/drivers/net/intel/ice/meson.build
@@ -18,9 +18,20 @@ sources = files(
testpmd_sources = files('ice_testpmd.c')
-deps += ['hash', 'net', 'net_iavf']
+deps += ['hash', 'net']
includes += include_directories('base')
+if dpdk_conf.has('RTE_NET_IAVF')
+deps += 'net_iavf'
+else
+includes += include_directories('../iavf/base')
+sources += files(
+'../iavf/base/iavf_adminq.c',
+'../iavf/base/iavf_common.c',
+'../iavf/base/iavf_impl.c',
+)
+endif
+
if arch_subdir == 'x86'
sources += files('ice_rxtx_vec_sse.c')
--
2.43.0
[PATCH v3 06/19] eal: fix out of bounds access in devargs
The code for parsing layers in devargs could reference past
the end of layers[] array on stack.
Link: https://pvs-studio.com/en/blog/posts/cpp/1183/
Fixes: 9a1a9e4a2ddd ("devargs: support path value with global device syntax")
Cc: xuemi...@nvidia.com
Cc: sta...@dpdk.org
Signed-off-by: Stephen Hemminger
---
lib/eal/common/eal_common_devargs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/eal/common/eal_common_devargs.c
b/lib/eal/common/eal_common_devargs.c
index a64805b268..dd857fc839 100644
--- a/lib/eal/common/eal_common_devargs.c
+++ b/lib/eal/common/eal_common_devargs.c
@@ -88,7 +88,7 @@ rte_devargs_layers_parse(struct rte_devargs *devargs,
s = devargs->data;
while (s != NULL) {
- if (nblayer > RTE_DIM(layers)) {
+ if (nblayer >= RTE_DIM(layers)) {
ret = -E2BIG;
goto get_out;
}
--
2.47.2
[PATCH v3 01/19] common/cnxk: remove duplicate condition
The same condition is checked twice in an if statement.
Harmless, but redundant.
Link: https://pvs-studio.com/en/blog/posts/cpp/1183/
Signed-off-by: Stephen Hemminger
Acked-by: Anoob Joseph
---
drivers/common/cnxk/cnxk_security.c | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/common/cnxk/cnxk_security.c
b/drivers/common/cnxk/cnxk_security.c
index c2871ad2bd..9446c14ac8 100644
--- a/drivers/common/cnxk/cnxk_security.c
+++ b/drivers/common/cnxk/cnxk_security.c
@@ -174,9 +174,11 @@ ot_ipsec_sa_common_param_fill(union roc_ot_ipsec_sa_word2
*w2, uint8_t *cipher_k
}
/* Set AES key length */
- if (w2->s.enc_type == ROC_IE_SA_ENC_AES_CBC || w2->s.enc_type ==
ROC_IE_SA_ENC_AES_CCM ||
- w2->s.enc_type == ROC_IE_SA_ENC_AES_CTR || w2->s.enc_type ==
ROC_IE_SA_ENC_AES_GCM ||
- w2->s.enc_type == ROC_IE_SA_ENC_AES_CCM || w2->s.auth_type ==
ROC_IE_SA_AUTH_AES_GMAC) {
+ if (w2->s.enc_type == ROC_IE_SA_ENC_AES_CBC ||
+ w2->s.enc_type == ROC_IE_SA_ENC_AES_CTR ||
+ w2->s.enc_type == ROC_IE_SA_ENC_AES_GCM ||
+ w2->s.enc_type == ROC_IE_SA_ENC_AES_CCM ||
+ w2->s.auth_type == ROC_IE_SA_AUTH_AES_GMAC) {
switch (length) {
case ROC_CPT_AES128_KEY_LEN:
w2->s.aes_key_len = ROC_IE_SA_AES_KEY_LEN_128;
@@ -879,9 +881,11 @@ on_ipsec_sa_ctl_set(struct rte_security_ipsec_xform *ipsec,
}
/* Set AES key length */
- if (ctl->enc_type == ROC_IE_SA_ENC_AES_CBC || ctl->enc_type ==
ROC_IE_SA_ENC_AES_CCM ||
- ctl->enc_type == ROC_IE_SA_ENC_AES_CTR || ctl->enc_type ==
ROC_IE_SA_ENC_AES_GCM ||
- ctl->enc_type == ROC_IE_SA_ENC_AES_CCM || ctl->auth_type ==
ROC_IE_SA_AUTH_AES_GMAC) {
+ if (ctl->enc_type == ROC_IE_SA_ENC_AES_CBC ||
+ ctl->enc_type == ROC_IE_SA_ENC_AES_CTR ||
+ ctl->enc_type == ROC_IE_SA_ENC_AES_GCM ||
+ ctl->enc_type == ROC_IE_SA_ENC_AES_CCM ||
+ ctl->auth_type == ROC_IE_SA_AUTH_AES_GMAC) {
switch (aes_key_len) {
case 16:
ctl->aes_key_len = ROC_IE_SA_AES_KEY_LEN_128;
--
2.47.2
[PATCH v3 12/19] crypto/dpaa2_sec: fix bitmask truncation
The dqrr_held mask is 64 bit but updates were getting truncated
because 1 is of type int (32 bit) and the result shift of int is of
type int (32 bit); therefore any value >= 32 would get truncated.
Link: https://pvs-studio.com/en/blog/posts/cpp/1183/
Fixes: a77db24643b7 ("crypto/dpaa2_sec: support atomic queues")
Cc: ashish.j...@nxp.com
Cc: sta...@dpdk.org
Signed-off-by: Stephen Hemminger
Acked-by: Hemant Agrawal
---
drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
index ec6577f64c..7ad8fd47dd 100644
--- a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
+++ b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
@@ -1491,8 +1491,8 @@ dpaa2_sec_enqueue_burst(void *qp, struct rte_crypto_op
**ops,
if (*dpaa2_seqn((*ops)->sym->m_src)) {
if (*dpaa2_seqn((*ops)->sym->m_src) &
QBMAN_ENQUEUE_FLAG_DCA) {
DPAA2_PER_LCORE_DQRR_SIZE--;
- DPAA2_PER_LCORE_DQRR_HELD &= ~(1 <<
- *dpaa2_seqn((*ops)->sym->m_src) &
+ DPAA2_PER_LCORE_DQRR_HELD &=
~(UINT64_C(1) <<
+ *dpaa2_seqn((*ops)->sym->m_src)
&
QBMAN_EQCR_DCA_IDXMASK);
}
flags[loop] = *dpaa2_seqn((*ops)->sym->m_src);
@@ -1772,7 +1772,7 @@ dpaa2_sec_set_enqueue_descriptor(struct dpaa2_queue
*dpaa2_q,
dq_idx = *dpaa2_seqn(m) - 1;
qbman_eq_desc_set_dca(eqdesc, 1, dq_idx, 0);
DPAA2_PER_LCORE_DQRR_SIZE--;
- DPAA2_PER_LCORE_DQRR_HELD &= ~(1 << dq_idx);
+ DPAA2_PER_LCORE_DQRR_HELD &= ~(UINT64_C(1) << dq_idx);
}
*dpaa2_seqn(m) = DPAA2_INVALID_MBUF_SEQN;
}
@@ -4055,7 +4055,7 @@ dpaa2_sec_process_atomic_event(struct qbman_swp *swp
__rte_unused,
dqrr_index = qbman_get_dqrr_idx(dq);
*dpaa2_seqn(crypto_op->sym->m_src) = QBMAN_ENQUEUE_FLAG_DCA |
dqrr_index;
DPAA2_PER_LCORE_DQRR_SIZE++;
- DPAA2_PER_LCORE_DQRR_HELD |= 1 << dqrr_index;
+ DPAA2_PER_LCORE_DQRR_HELD |= UINT64_C(1) << dqrr_index;
DPAA2_PER_LCORE_DQRR_MBUF(dqrr_index) = crypto_op->sym->m_src;
ev->event_ptr = crypto_op;
}
--
2.47.2
Re: [PATCH v21 18/27] test: remove use of VLAs for Windows built code in bitset tests
Hello André,
On Tue, Feb 4, 2025 at 9:57 PM Andre Muezerie
wrote:
> @@ -168,10 +169,20 @@ test_flip_size(test_fun test_fun, assign_fun
> assign_fun, flip_fun flip_fun, size
> rand_bitset(bitset, size);
>
> for (i = 0; i < size; i++) {
> - RTE_BITSET_DECLARE(reference, size);
> + RTE_BITSET_DECLARE(reference, RAND_SET_MAX_SIZE);
> +
> +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 11)
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Warray-bounds"
> +#endif
>
> + /* gcc is giving false positives here when code is optimized
> */
Why not simply alloca(te the right size)?
I tested with my gcc 14 (for which I could reproduce the array bound warning).
By replacing with uint64_t *reference = alloca(RTE_BITSET_SIZE(size)),
gcc seems to be less smart and won't inspect 'reference' and 'bitset'
arrays boundaries.
> rte_bitset_copy(reference, bitset, size);
>
> +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 11)
> +#pragma GCC diagnostic pop
> +#endif
> +
> bool value = test_fun(bitset, i);
>
> flip_fun(bitset, i);
The rest of the series lgtm and the plan is to merge it for rc1.
Just beware that, if you send a new revision, new drivers (net/xsc and
net/zxdh) landed in main.
Both use VLA, so both require cflags += no_wvla_cflag in their meson.build.
--
David Marchand
RE: [PATCH v4] bus: fix inconsistent representation of PCI numbers
> -Original Message-
> From: Stephen Hemminger
> Sent: Wednesday, 29 January 2025 18:25
> To: Shani Peretz
> Cc: dev@dpdk.org; NBU-Contact-Thomas Monjalon (EXTERNAL)
> ; Tyler Retzlaff ; Parav
> Pandit ; Xueming Li ; Nipun Gupta
> ; Nikhil Agarwal ; Hemant
> Agrawal ; Sachin Saxena
> ; Rosen Xu ; Chenbo Xia
> ; Tomasz Duszynski ;
> Chengwen Feng ; NBU-Contact-longli
> (EXTERNAL) ; Wei Hu ; Bruce
> Richardson ; Kevin Laatz
> ; Jan Blunck
> Subject: Re: [PATCH v4] bus: fix inconsistent representation of PCI numbers
>
> External email: Use caution opening links or attachments
>
>
> On Wed, 29 Jan 2025 10:54:16 +0200
> Shani Peretz wrote:
>
> > +create_pci_dev(const char *name)
> > +{
> > + int port_id;
> > + uint8_t slave_mac1[] = {0x00, 0xFF, 0x00, 0xFF, 0x00, 0x00 };
> > + struct rte_ether_addr *mac_addr = (struct rte_ether_addr
> > +*)slave_mac1;
>
> Use different initializer and you can avoid the need for cast here.
>
>
> >
> > +/**
> > + * General device name comparison. Will compare by using the specific
> > +bus
> > + * compare function or by comparing the names directly.
> > + *
> > + * @param dev
> > + * Device handle.
> > + * @param name
> > + * Name to compare against.
> > + * @return
> > + * 0 if the device matches the name. Nonzero otherwise.
> > + */
> > +__rte_internal
> > +int rte_cmp_dev_name(const struct rte_device *dev, const void *name);
>
> It would make more sense to me if name was a character not void pointer.
>
> The design might be clearer if bus address was more of an typedef with a
> pointer and size together. Treat it more like an object.
Okay so just to understand,
this is the struct I am adding:
struct rte_bus_address {
const void *addr;
size_t size;
};
This is how I pass it to find_device:
struct rte_bus_address dev_addr = {
.addr = da->name,
.size = RTE_DEV_NAME_MAX_LEN
};
dev = da->bus->find_device(NULL, rte_cmp_dev_name, &dev_addr);
And this is how I use it in rte_cmp_dev_name:
rte_cmp_dev_name(const struct rte_device *dev1, const void *addr)
{
const struct rte_bus_address *dev2_addr = addr;
…
}
Is that what you meant?
Also, I'm not sure if the size is really needed, because we check the size of
the parsed name, which may be different than the size of the original name
Re: [PATCH] build: remove support for icc compiler
On Wed, Feb 05, 2025 at 06:03:16PM +0100, David Marchand wrote: > On Wed, Feb 5, 2025 at 5:19 PM Bruce Richardson > wrote: > > > > The Intel-produced compiler "icc" has been replaced by the newer > > clang-based "icx" compiler. DPDK compilation has also not been tested > > recently with the icc compiler, so let's remove doc and code references > > to icc, and any special macros or build support that was added for it. > > > > Signed-off-by: Bruce Richardson > > I noticed remaining references, from which some can be cleaned up too: > > app/test-pmd/testpmd.h: * Work-around of a compilation error with ICC > on invocations of the > Yes, I spotted this and a few of the others too - but forgot to reference them in the commit log ntoes. The trouble is that for a number of these cases I've no idea what specific part of the code the workaround is, or what it should look like without the workaround. For this specific instance, the ifdefs in the testpmd.h file just refer to GCC and non-GCC (presumably including clang), so it doesn't seem that there is code that is ICC specific. Shall I just remove the comment? > lib/eal/common/eal_common_dynmem.c: /* to prevent > icc errors */ Same here. It's not clear to me what way the code should be reworked if not supporting ICC. Again, I can just remove the comment and be done with it. > lib/eal/x86/include/rte_vect.h:#if defined(__ICC) || defined(_WIN64) > This seems as miss on my part. Will fix in a v2. > buildtools/check-symbols.sh:# Filter out symbols suffixed with a . for icc > buildtools/check-symbols.sh:# Filter out symbols suffixed with a . for icc > This you may be able to advise me on. I assume that the icc-specific bit is just he "&& !($NF ~ /\.$/)" bit, and not the whole block the comment is put on? > And please add a release note update. > Yes. I've also been testing with the newer "icx" and will probably add something in the docs about it working ok as a replacement. /Bruce
RE: [PATCH v4] bus: fix inconsistent representation of PCI numbers
> -Original Message-
> From: Stephen Hemminger
> Sent: Wednesday, 5 February 2025 18:42
> To: Shani Peretz
> Cc: dev@dpdk.org; NBU-Contact-Thomas Monjalon (EXTERNAL)
> ; Tyler Retzlaff ; Parav
> Pandit ; Xueming Li ; Nipun Gupta
> ; Nikhil Agarwal ; Hemant
> Agrawal ; Sachin Saxena
> ; Rosen Xu ; Chenbo Xia
> ; Tomasz Duszynski ;
> Chengwen Feng ; NBU-Contact-longli
> (EXTERNAL) ; Wei Hu ; Bruce
> Richardson ; Kevin Laatz
> ; Jan Blunck
> Subject: Re: [PATCH v4] bus: fix inconsistent representation of PCI numbers
>
> External email: Use caution opening links or attachments
>
>
> On Wed, 5 Feb 2025 16:36:11 +
> Shani Peretz wrote:
>
> > > -Original Message-
> > > From: Stephen Hemminger
> > > Sent: Wednesday, 29 January 2025 18:25
> > > To: Shani Peretz
> > > Cc: dev@dpdk.org; NBU-Contact-Thomas Monjalon (EXTERNAL)
> > > ; Tyler Retzlaff
> > > ; Parav Pandit ;
> > > Xueming Li ; Nipun Gupta
> ;
> > > Nikhil Agarwal ; Hemant Agrawal
> > > ; Sachin Saxena ;
> > > Rosen Xu ; Chenbo Xia ;
> > > Tomasz Duszynski ; Chengwen Feng
> > > ; NBU-Contact-longli
> > > (EXTERNAL) ; Wei Hu ; Bruce
> > > Richardson ; Kevin Laatz
> > > ; Jan Blunck
> > > Subject: Re: [PATCH v4] bus: fix inconsistent representation of PCI
> > > numbers
> > >
> > > External email: Use caution opening links or attachments
> > >
> > >
> > > On Wed, 29 Jan 2025 10:54:16 +0200
> > > Shani Peretz wrote:
> > >
> > > > +create_pci_dev(const char *name)
> > > > +{
> > > > + int port_id;
> > > > + uint8_t slave_mac1[] = {0x00, 0xFF, 0x00, 0xFF, 0x00, 0x00 };
> > > > + struct rte_ether_addr *mac_addr = (struct rte_ether_addr
> > > > +*)slave_mac1;
> > >
> > > Use different initializer and you can avoid the need for cast here.
> > >
> > >
> > > >
> > > > +/**
> > > > + * General device name comparison. Will compare by using the
> > > > +specific bus
> > > > + * compare function or by comparing the names directly.
> > > > + *
> > > > + * @param dev
> > > > + * Device handle.
> > > > + * @param name
> > > > + * Name to compare against.
> > > > + * @return
> > > > + * 0 if the device matches the name. Nonzero otherwise.
> > > > + */
> > > > +__rte_internal
> > > > +int rte_cmp_dev_name(const struct rte_device *dev, const void
> > > > +*name);
> > >
> > > It would make more sense to me if name was a character not void pointer.
> > >
> > > The design might be clearer if bus address was more of an typedef
> > > with a pointer and size together. Treat it more like an object.
> >
> >
> > Okay so just to understand,
> > this is the struct I am adding:
> >
> > struct rte_bus_address {
> > const void *addr;
> > size_t size;
> > };
> >
> > This is how I pass it to find_device:
> >
> > struct rte_bus_address dev_addr = {
> > .addr = da->name,
> > .size = RTE_DEV_NAME_MAX_LEN
> > };
> > dev = da->bus->find_device(NULL, rte_cmp_dev_name, &dev_addr);
> >
> > And this is how I use it in rte_cmp_dev_name:
> >
> > rte_cmp_dev_name(const struct rte_device *dev1, const void *addr)
> >{
> > const struct rte_bus_address *dev2_addr = addr;
> > …
> > }
> >
> > Is that what you meant?
> > Also, I'm not sure if the size is really needed, because we check the
> > size of the parsed name, which may be different than the size of the
> > original name
> >
>
> It would be best to pass rte_bus_address to rte_cmp_dev_name rather than
> having implied cast.
> Not sure if that is possible without breaking API though.
I think you are right, also it will require to change the signature of
find_device which will make it even bigger change
Re: [PATCH] build: remove support for icc compiler
On Wed, Feb 05, 2025 at 05:32:11PM +, Bruce Richardson wrote:
> On Wed, Feb 05, 2025 at 06:03:16PM +0100, David Marchand wrote:
> > On Wed, Feb 5, 2025 at 5:19 PM Bruce Richardson
> > wrote:
> > >
> > > The Intel-produced compiler "icc" has been replaced by the newer
> > > clang-based "icx" compiler. DPDK compilation has also not been tested
> > > recently with the icc compiler, so let's remove doc and code references
> > > to icc, and any special macros or build support that was added for it.
> > >
> > > Signed-off-by: Bruce Richardson
> >
> > I noticed remaining references, from which some can be cleaned up too:
> >
> > app/test-pmd/testpmd.h: * Work-around of a compilation error with ICC
> > on invocations of the
> >
>
> Yes, I spotted this and a few of the others too - but forgot to reference
> them in the commit log ntoes.
>
> The trouble is that for a number of these cases I've no idea what specific
> part of the code the workaround is, or what it should look like without the
> workaround.
>
> For this specific instance, the ifdefs in the testpmd.h file just refer to
> GCC and non-GCC (presumably including clang), so it doesn't seem that there
> is code that is ICC specific. Shall I just remove the comment?
>
> > lib/eal/common/eal_common_dynmem.c: /* to prevent
> > icc errors */
>
> Same here. It's not clear to me what way the code should be reworked if not
> supporting ICC. Again, I can just remove the comment and be done with it.
>
> > lib/eal/x86/include/rte_vect.h:#if defined(__ICC) || defined(_WIN64)
> >
>
> This seems as miss on my part. Will fix in a v2.
>
> > buildtools/check-symbols.sh:# Filter out symbols suffixed with a . for icc
> > buildtools/check-symbols.sh:# Filter out symbols suffixed with a . for icc
> >
>
> This you may be able to advise me on. I assume that the icc-specific bit is
> just he "&& !($NF ~ /\.$/)" bit, and not the whole block the comment is put
> on?
>
There is also a reference in ena driver, again, not sure what the correct
code replacement is - setting the struct initializer to {0} perhaps? Adding
some ena maintainers on CC so they can comment on this.
struct ena_com_create_io_ctx ctx =
/* policy set to _HOST just to satisfy icc compiler */
{ ENA_ADMIN_PLACEMENT_POLICY_HOST,
0, 0, 0, 0, 0 };
Ena maintainers - any suggestions on what the code should look like if we
no longer have to support the icc compiler?
/Bruce
Re: [PATCH v4 0/7] eliminate dependency on non-portable __SIZEOF_LONG__
On Wed, Feb 05, 2025 at 03:50:03PM +, Konstantin Ananyev wrote: > > > On Wed, Feb 05, 2025 at 07:37:21AM -0800, Andre Muezerie wrote: > > > On Wed, Feb 05, 2025 at 09:15:43AM +, Bruce Richardson wrote: > > > > On Tue, Feb 04, 2025 at 10:54:24AM -0800, Andre Muezerie wrote: > > > > > Macro __SIZEOF_LONG__ is not standardized and MSVC does not define it. > > > > > Therefore the errors below are seen with MSVC: > > > > > > > > > > ../lib/mldev/mldev_utils_scalar.c(465): error C2065: > > > > > '__SIZEOF_LONG__': undeclared identifier > > > > > ../lib/mldev/mldev_utils_scalar.c(478): error C2051: > > > > > case expression not constant > > > > > > > > > > ../lib/mldev/mldev_utils_scalar_bfloat16.c(33): error C2065: > > > > > '__SIZEOF_LONG__': undeclared identifier > > > > > ../lib/mldev/mldev_utils_scalar_bfloat16.c(49): error C2051: > > > > > case expression not constant > > > > > > > > > > Turns out that the places where __SIZEOF_LONG__ is currently > > > > > being used can equally well use sizeof(long) instead. > > > > > > > > > > v4: > > > > > * rebased on latest main as previous patch was not applying cleanly > > > > >anymore. > > > > > > > > > > v3: > > > > > * added prefix RTE_ to BITS_PER_LONG* and moved them to rte_common.h > > > > > * defined PLT_BITS_PER_LONG* in drivers/common/cnxk/roc_platform.h to > > > > >avoid warnings from checkpatches.sh like: > > > > > > > > > >Warning in drivers/common/cnxk/roc_bits.h: > > > > >Warning in drivers/common/cnxk/roc_ie_ot.h: > > > > >Warning in drivers/common/cnxk/roc_ie_ot_tls.h: > > > > >Use plt_ symbols instead of rte_ API in cnxk base driver > > > > > > > > > >It can be seen that the same was done in the past for similar > > > > >macros like PLT_CACHE_LINE_SIZE > > > > > > > > > > v2: > > > > > * fixed typo in commit message > > > > > > > > > > Andre Muezerie (7): > > > > > eal: eliminate dependency on non-portable __SIZEOF_LONG__ > > > > > drivers/bus: eliminate dependency on non-portable __SIZEOF_LONG__ > > > > > drivers/common: eliminate dependency on non-portable __SIZEOF_LONG__ > > > > > drivers/dma: eliminate dependency on non-portable __SIZEOF_LONG__ > > > > > drivers/net: eliminate dependency on non-portable __SIZEOF_LONG__ > > > > > drivers/raw: eliminate dependency on non-portable __SIZEOF_LONG__ > > > > > mldev: eliminate dependency on non-portable __SIZEOF_LONG__ > > > > > > > > > Just out of interest, is there are reason why the simple solution of > > > > just > > > > putting "#define __SIZEOF_LONG__ (sizeof(long))" in a header file for > > > > MSVC > > > > is not done? Should be a couple of lines in a single patch, rather than > > > > a > > > > 7-patch series, no? > > > > > > > > After all, just because something is non-standard, doesn't mean that we > > > > can't use it if its widely available. > > > > > > > > /Bruce > > > > > > Yes, that can be done instead. I'll send out a new series with that > > > approach. > > > > > Maybe wait in case there is input from others before spending time on a > > patch? I think the simpler solution is better, but others may feel that > > removing the macro completely is the better approach. > > +1 to what Bruce suggested. > Thomas also mentioned he would prefer to minimize changes. I'm fine with that too, as that is good enough to get the code to compile with MSVC.
[PATCH v5] eal: define __SIZEOF_LONG__ when using MSVC
Macro __SIZEOF_LONG__ is not standardized and MSVC does not define it.
Therefore the errors below are seen with MSVC:
../lib/mldev/mldev_utils_scalar.c(465): error C2065:
'__SIZEOF_LONG__': undeclared identifier
../lib/mldev/mldev_utils_scalar.c(478): error C2051:
case expression not constant
../lib/mldev/mldev_utils_scalar_bfloat16.c(33): error C2065:
'__SIZEOF_LONG__': undeclared identifier
../lib/mldev/mldev_utils_scalar_bfloat16.c(49): error C2051:
case expression not constant
The fix is to define __SIZEOF_LONG__ in a common header when
MSVC is used.
Signed-off-by: Andre Muezerie
---
lib/eal/include/rte_compat.h | 5 +
1 file changed, 5 insertions(+)
diff --git a/lib/eal/include/rte_compat.h b/lib/eal/include/rte_compat.h
index 97c1540bd0..6676a1bb13 100644
--- a/lib/eal/include/rte_compat.h
+++ b/lib/eal/include/rte_compat.h
@@ -66,4 +66,9 @@ __attribute__((section(".text.internal")))
#endif
+#ifdef RTE_TOOLCHAIN_MSVC
+#define __SIZEOF_LONG__(sizeof(long))
+#define __SIZEOF_LONG_LONG__ (sizeof(long long))
+#endif
+
#endif /* _RTE_COMPAT_H_ */
--
2.47.2.vfs.0.1
Re: [PATCH v7 04/15] net/xsc: add xsc dev ops to support VFIO driver
On Wed, Feb 5, 2025 at 5:44 PM Renyong Wan wrote: > >> That's how we designed it. > >> We designed a low-level device operations framework named xsc_dev_ops to > >> support both VFIO drivers and kernel drivers. Each xsc_dev_ops is > >> registered before the main function runs. During the PCI device probe > >> phase, the XSC PMD selects the corresponding xsc_dev_ops based on > >> rte_pci_device->driver, therefore, there is no need for devargs. > > I don't understand. > > If both VFIO and the kernel driver are loaded, > > we'll scan the device twice? > > Will it be probed 2 times? > > > > > No, it won't probe twice. > I suppose that each PCI device will only be bound to either the VFIO > driver or a kernel driver. The drv_flags of the xsc_pmd PCI driver will > not preset with RTE_PCI_DRV_NEED_MAPPING. Therefore, in the > rte_pci_probe_one_driver function, rte_pci_map_device() will not be > called. After entering the probe phase of the xsc PMD PCI driver, > rte_pci_map_device() will be called in xsc_dev_ops->init() based on > whether it is a VFIO driver. (side note, this means that this driver should probably call rte_pci_unmap_device() in its release path, though I see none) -- David Marchand
Re: [PATCH v4 0/7] eliminate dependency on non-portable __SIZEOF_LONG__
On Wed, Feb 05, 2025 at 09:15:43AM +, Bruce Richardson wrote: > On Tue, Feb 04, 2025 at 10:54:24AM -0800, Andre Muezerie wrote: > > Macro __SIZEOF_LONG__ is not standardized and MSVC does not define it. > > Therefore the errors below are seen with MSVC: > > > > ../lib/mldev/mldev_utils_scalar.c(465): error C2065: > > '__SIZEOF_LONG__': undeclared identifier > > ../lib/mldev/mldev_utils_scalar.c(478): error C2051: > > case expression not constant > > > > ../lib/mldev/mldev_utils_scalar_bfloat16.c(33): error C2065: > > '__SIZEOF_LONG__': undeclared identifier > > ../lib/mldev/mldev_utils_scalar_bfloat16.c(49): error C2051: > > case expression not constant > > > > Turns out that the places where __SIZEOF_LONG__ is currently > > being used can equally well use sizeof(long) instead. > > > > v4: > > * rebased on latest main as previous patch was not applying cleanly > >anymore. > > > > v3: > > * added prefix RTE_ to BITS_PER_LONG* and moved them to rte_common.h > > * defined PLT_BITS_PER_LONG* in drivers/common/cnxk/roc_platform.h to > >avoid warnings from checkpatches.sh like: > > > >Warning in drivers/common/cnxk/roc_bits.h: > >Warning in drivers/common/cnxk/roc_ie_ot.h: > >Warning in drivers/common/cnxk/roc_ie_ot_tls.h: > >Use plt_ symbols instead of rte_ API in cnxk base driver > > > >It can be seen that the same was done in the past for similar > >macros like PLT_CACHE_LINE_SIZE > > > > v2: > > * fixed typo in commit message > > > > Andre Muezerie (7): > > eal: eliminate dependency on non-portable __SIZEOF_LONG__ > > drivers/bus: eliminate dependency on non-portable __SIZEOF_LONG__ > > drivers/common: eliminate dependency on non-portable __SIZEOF_LONG__ > > drivers/dma: eliminate dependency on non-portable __SIZEOF_LONG__ > > drivers/net: eliminate dependency on non-portable __SIZEOF_LONG__ > > drivers/raw: eliminate dependency on non-portable __SIZEOF_LONG__ > > mldev: eliminate dependency on non-portable __SIZEOF_LONG__ > > > Just out of interest, is there are reason why the simple solution of just > putting "#define __SIZEOF_LONG__ (sizeof(long))" in a header file for MSVC > is not done? Should be a couple of lines in a single patch, rather than a > 7-patch series, no? > > After all, just because something is non-standard, doesn't mean that we > can't use it if its widely available. > > /Bruce Yes, that can be done instead. I'll send out a new series with that approach.
Re: [PATCH v7 04/15] net/xsc: add xsc dev ops to support VFIO driver
On 2025/2/5 22:43, Thomas Monjalon wrote: > 05/02/2025 15:37, Renyong Wan: >> On 2025/2/5 19:44, Thomas Monjalon wrote: >>> 28/01/2025 15:46, Renyong Wan: XSC PMD is designed to support both VFIO and private kernel drivers. >>> What's the benefit of private kernel drivers? >>> Why are they private? >> Hello Thomas, >> >> Thanks for your review. >> >> It can support the bifurcation model without unbinding the kernel >> driver, by utilizing our private kernel driver in conjunction with >> rdma-core. Currently, our kernel driver is not open-source, so it is >> considered a private kernel driver. This patch series only supports the >> VFIO driver. Our kernel driver is currently in the process of being >> open-sourced on kernel.org, and once it is available there, we also plan >> to submit the code that supports our kernel driver to DPDK. > OK that's interesting, thank you. > > I think it would be the first DPDK driver to support both VFIO or bifurcated > model. > How will we choose which one to use? With devargs? > > That's how we designed it. We designed a low-level device operations framework named xsc_dev_ops to support both VFIO drivers and kernel drivers. Each xsc_dev_ops is registered before the main function runs. During the PCI device probe phase, the XSC PMD selects the corresponding xsc_dev_ops based on rte_pci_device->driver, therefore, there is no need for devargs. Thank you. -- Best regards, Renyong Wan
Re: [PATCH v7 04/15] net/xsc: add xsc dev ops to support VFIO driver
On Wed, Feb 05, 2025 at 04:47:20PM +0100, Thomas Monjalon wrote: > 05/02/2025 15:59, Bruce Richardson: > > On Wed, Feb 05, 2025 at 03:43:30PM +0100, Thomas Monjalon wrote: > > > 05/02/2025 15:37, Renyong Wan: > > > > On 2025/2/5 19:44, Thomas Monjalon wrote: > > > > > 28/01/2025 15:46, Renyong Wan: > > > > >> XSC PMD is designed to support both VFIO and private kernel drivers. > > > > > What's the benefit of private kernel drivers? Why are they private? > > > > > > > > Hello Thomas, > > > > > > > > Thanks for your review. > > > > > > > > It can support the bifurcation model without unbinding the kernel > > > > driver, by utilizing our private kernel driver in conjunction with > > > > rdma-core. Currently, our kernel driver is not open-source, so it is > > > > considered a private kernel driver. This patch series only supports the > > > > VFIO driver. Our kernel driver is currently in the process of being > > > > open-sourced on kernel.org, and once it is available there, we also > > > > plan to submit the code that supports our kernel driver to DPDK. > > > > > > OK that's interesting, thank you. > > > > > > I think it would be the first DPDK driver to support both VFIO or > > > bifurcated model. > > > > > > > Not quite the first, but possibly the first net driver? :-). The idxd > > dmadev driver supports both. It can be used either with VFIO or the kernel > > idxd driver. > > It announces only VFIO: > RTE_PMD_REGISTER_KMOD_DEP(IDXD_PMD_DMADEV_NAME_PCI, "vfio-pci"); > > How does it work? > It also has its own bus driver that scans for dev nodes (/dev/dsa/wq*) on probe, and uses those configured for DPDK use. On a system with multiple device instances you can have one device used by DPDK bound to vfio, and use a couple of work queues from another device bound to the idxd kernel driver. More info on the setup is in the docs [1] /Bruce [1] https://doc.dpdk.org/guides-24.11/dmadevs/idxd.html#device-setup
Re: [PATCH v4] bus: fix inconsistent representation of PCI numbers
On Wed, 5 Feb 2025 16:36:11 +
Shani Peretz wrote:
> > -Original Message-
> > From: Stephen Hemminger
> > Sent: Wednesday, 29 January 2025 18:25
> > To: Shani Peretz
> > Cc: dev@dpdk.org; NBU-Contact-Thomas Monjalon (EXTERNAL)
> > ; Tyler Retzlaff ; Parav
> > Pandit ; Xueming Li ; Nipun Gupta
> > ; Nikhil Agarwal ; Hemant
> > Agrawal ; Sachin Saxena
> > ; Rosen Xu ; Chenbo Xia
> > ; Tomasz Duszynski ;
> > Chengwen Feng ; NBU-Contact-longli
> > (EXTERNAL) ; Wei Hu ; Bruce
> > Richardson ; Kevin Laatz
> > ; Jan Blunck
> > Subject: Re: [PATCH v4] bus: fix inconsistent representation of PCI numbers
> >
> > External email: Use caution opening links or attachments
> >
> >
> > On Wed, 29 Jan 2025 10:54:16 +0200
> > Shani Peretz wrote:
> >
> > > +create_pci_dev(const char *name)
> > > +{
> > > + int port_id;
> > > + uint8_t slave_mac1[] = {0x00, 0xFF, 0x00, 0xFF, 0x00, 0x00 };
> > > + struct rte_ether_addr *mac_addr = (struct rte_ether_addr
> > > +*)slave_mac1;
> >
> > Use different initializer and you can avoid the need for cast here.
> >
> >
> > >
> > > +/**
> > > + * General device name comparison. Will compare by using the specific
> > > +bus
> > > + * compare function or by comparing the names directly.
> > > + *
> > > + * @param dev
> > > + * Device handle.
> > > + * @param name
> > > + * Name to compare against.
> > > + * @return
> > > + * 0 if the device matches the name. Nonzero otherwise.
> > > + */
> > > +__rte_internal
> > > +int rte_cmp_dev_name(const struct rte_device *dev, const void *name);
> >
> > It would make more sense to me if name was a character not void pointer.
> >
> > The design might be clearer if bus address was more of an typedef with a
> > pointer and size together. Treat it more like an object.
>
>
> Okay so just to understand,
> this is the struct I am adding:
>
> struct rte_bus_address {
> const void *addr;
> size_t size;
> };
>
> This is how I pass it to find_device:
>
> struct rte_bus_address dev_addr = {
> .addr = da->name,
> .size = RTE_DEV_NAME_MAX_LEN
> };
> dev = da->bus->find_device(NULL, rte_cmp_dev_name, &dev_addr);
>
> And this is how I use it in rte_cmp_dev_name:
>
> rte_cmp_dev_name(const struct rte_device *dev1, const void *addr)
>{
> const struct rte_bus_address *dev2_addr = addr;
> …
> }
>
> Is that what you meant?
> Also, I'm not sure if the size is really needed, because we check the size of
> the parsed name, which may be different than the size of the original name
>
It would be best to pass rte_bus_address to rte_cmp_dev_name rather than having
implied cast.
Not sure if that is possible without breaking API though.
Re: [PATCH v21 18/27] test: remove use of VLAs for Windows built code in bitset tests
On Tue, 4 Feb 2025 12:57:09 -0800 Andre Muezerie wrote: > +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 11) > +#pragma GCC diagnostic push > +#pragma GCC diagnostic ignored "-Warray-bounds" > +#endif > > + /* gcc is giving false positives here when code is optimized */ > rte_bitset_copy(reference, bitset, size); > > +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 11) > +#pragma GCC diagnostic pop > +#endif > + > Any change requiring pragma workaround is suspect. What happens with Gcc 15? and ASAN
[PATCH v3 15/19] net/dpaa: fix bitmask truncation
The dqrr_held mask is 64 bit but updates were getting truncated
because 1 is of type int (32 bit) and the result shift of int is of
type int (32 bit); therefore any value >= 32 would get truncated.
Link: https://pvs-studio.com/en/blog/posts/cpp/1183/
Fixes: 5e7455931442 ("net/dpaa: support Rx queue configurations with eventdev")
Cc: sunil.k...@nxp.com
Cc: sta...@dpdk.org
Signed-off-by: Stephen Hemminger
Acked-by: Hemant Agrawal
---
drivers/net/dpaa/dpaa_rxtx.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/net/dpaa/dpaa_rxtx.c b/drivers/net/dpaa/dpaa_rxtx.c
index 247e7b92ba..05bd73becf 100644
--- a/drivers/net/dpaa/dpaa_rxtx.c
+++ b/drivers/net/dpaa/dpaa_rxtx.c
@@ -842,7 +842,7 @@ dpaa_rx_cb_atomic(void *event,
/* Save active dqrr entries */
index = DQRR_PTR2IDX(dqrr);
DPAA_PER_LCORE_DQRR_SIZE++;
- DPAA_PER_LCORE_DQRR_HELD |= 1 << index;
+ DPAA_PER_LCORE_DQRR_HELD |= UINT64_C(1) << index;
DPAA_PER_LCORE_DQRR_MBUF(index) = mbuf;
ev->impl_opaque = index + 1;
*dpaa_seqn(mbuf) = (uint32_t)index + 1;
@@ -1338,13 +1338,12 @@ dpaa_eth_queue_tx(void *q, struct rte_mbuf **bufs,
uint16_t nb_bufs)
seqn = *dpaa_seqn(mbuf);
if (seqn != DPAA_INVALID_MBUF_SEQN) {
index = seqn - 1;
- if (DPAA_PER_LCORE_DQRR_HELD & (1 << index)) {
+ if (DPAA_PER_LCORE_DQRR_HELD & (UINT64_C(1) <<
index)) {
flags[loop] =
((index & QM_EQCR_DCA_IDXMASK) << 8);
flags[loop] |= QMAN_ENQUEUE_FLAG_DCA;
DPAA_PER_LCORE_DQRR_SIZE--;
- DPAA_PER_LCORE_DQRR_HELD &=
- ~(1 << index);
+ DPAA_PER_LCORE_DQRR_HELD &=
~(UINT64_C(1) << index);
}
}
--
2.47.2
Re: [PATCH v7 04/15] net/xsc: add xsc dev ops to support VFIO driver
On 2025/2/5 23:45, Thomas Monjalon wrote: > 05/02/2025 16:37, Renyong Wan: >> On 2025/2/5 22:43, Thomas Monjalon wrote: >>> 05/02/2025 15:37, Renyong Wan: On 2025/2/5 19:44, Thomas Monjalon wrote: > 28/01/2025 15:46, Renyong Wan: >> XSC PMD is designed to support both VFIO and private kernel drivers. > What's the benefit of private kernel drivers? > Why are they private? Hello Thomas, Thanks for your review. It can support the bifurcation model without unbinding the kernel driver, by utilizing our private kernel driver in conjunction with rdma-core. Currently, our kernel driver is not open-source, so it is considered a private kernel driver. This patch series only supports the VFIO driver. Our kernel driver is currently in the process of being open-sourced on kernel.org, and once it is available there, we also plan to submit the code that supports our kernel driver to DPDK. >>> OK that's interesting, thank you. >>> >>> I think it would be the first DPDK driver to support both VFIO or >>> bifurcated model. >>> How will we choose which one to use? With devargs? >>> >>> >> That's how we designed it. >> We designed a low-level device operations framework named xsc_dev_ops to >> support both VFIO drivers and kernel drivers. Each xsc_dev_ops is >> registered before the main function runs. During the PCI device probe >> phase, the XSC PMD selects the corresponding xsc_dev_ops based on >> rte_pci_device->driver, therefore, there is no need for devargs. > I don't understand. > If both VFIO and the kernel driver are loaded, > we'll scan the device twice? > Will it be probed 2 times? > > No, it won't probe twice. I suppose that each PCI device will only be bound to either the VFIO driver or a kernel driver. The drv_flags of the xsc_pmd PCI driver will not preset with RTE_PCI_DRV_NEED_MAPPING. Therefore, in the rte_pci_probe_one_driver function, rte_pci_map_device() will not be called. After entering the probe phase of the xsc PMD PCI driver, rte_pci_map_device() will be called in xsc_dev_ops->init() based on whether it is a VFIO driver. Thank you. -- Best regards, Renyong Wan
[PATCH v3 13/19] crypto/dpaa_sec: fix bitmask truncation
The dqrr_held mask is 64 bit but updates were getting truncated
because 1 is of type int (32 bit) and the result shift of int is of
type int (32 bit); therefore any value >= 32 would get truncated.
Link: https://pvs-studio.com/en/blog/posts/cpp/1183/
Fixes: fe3688ba7950 ("crypto/dpaa_sec: support event crypto adapter")
Cc: akhil.go...@nxp.com
Cc: sta...@dpdk.org
Signed-off-by: Stephen Hemminger
Acked-by: Hemant Agrawal
---
drivers/crypto/dpaa_sec/dpaa_sec.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/crypto/dpaa_sec/dpaa_sec.c
b/drivers/crypto/dpaa_sec/dpaa_sec.c
index 3fa88ca968..e117cd77a6 100644
--- a/drivers/crypto/dpaa_sec/dpaa_sec.c
+++ b/drivers/crypto/dpaa_sec/dpaa_sec.c
@@ -1907,13 +1907,12 @@ dpaa_sec_enqueue_burst(void *qp, struct rte_crypto_op
**ops,
op = *(ops++);
if (*dpaa_seqn(op->sym->m_src) != 0) {
index = *dpaa_seqn(op->sym->m_src) - 1;
- if (DPAA_PER_LCORE_DQRR_HELD & (1 << index)) {
+ if (DPAA_PER_LCORE_DQRR_HELD & (UINT64_C(1) <<
index)) {
/* QM_EQCR_DCA_IDXMASK = 0x0f */
flags[loop] = ((index & 0x0f) << 8);
flags[loop] |= QMAN_ENQUEUE_FLAG_DCA;
DPAA_PER_LCORE_DQRR_SIZE--;
- DPAA_PER_LCORE_DQRR_HELD &=
- ~(1 << index);
+ DPAA_PER_LCORE_DQRR_HELD &=
~(UINT64_C(1) << index);
}
}
@@ -3500,7 +3499,7 @@ dpaa_sec_process_atomic_event(void *event,
/* Save active dqrr entries */
index = ((uintptr_t)dqrr >> 6) & (16/*QM_DQRR_SIZE*/ - 1);
DPAA_PER_LCORE_DQRR_SIZE++;
- DPAA_PER_LCORE_DQRR_HELD |= 1 << index;
+ DPAA_PER_LCORE_DQRR_HELD |= UINT64_C(1) << index;
DPAA_PER_LCORE_DQRR_MBUF(index) = ctx->op->sym->m_src;
ev->impl_opaque = index + 1;
*dpaa_seqn(ctx->op->sym->m_src) = (uint32_t)index + 1;
--
2.47.2
Re: [PATCH v1 00/42] Merge Intel IGC and E1000 drivers, and update E1000 base code
On Tue, Feb 4, 2025 at 4:36 PM Burakov, Anatoly
wrote:
>
> On 03/02/2025 9:18, David Marchand wrote:
> > Hello Anatoly,
> >
> > On Fri, Jan 31, 2025 at 1:59 PM Anatoly Burakov
> > wrote:
> >>
> >> Intel IGC and E1000 drivers are distinct, but they are actually generated
> >> from the same base code. This patchset will merge together all
> >> e1000-derived
> >> drivers into one common base, with three different ethdev driver
> >> frontends (EM, IGB, and IGC).
> >>
> >> After the merge is done, base code is also updated to latest snapshot.
> >>
> >> Adam Ludkiewicz (1):
> >>net/e1000/base: add WoL definitions
> >>
> >> Aleksandr Loktionov (1):
> >>net/e1000/base: fix mac addr hash bit_shift
> >>
> >> Amir Avivi (1):
> >>net/e1000/base: fix iterator type
> >>
> >> Anatoly Burakov (13):
> >>net/e1000/base: add initial support for i225
> >>net/e1000/base: add link bringup support for i225
> >>net/e1000/base: add LED blink support for i225
> >>net/e1000/base: add NVM/EEPROM support for i225
> >>net/e1000/base: add LTR support in i225
> >>net/e1000/base: add eee support for i225
> >>net/e1000/base: add misc definitions for i225
> >>net/e1000: merge igc with e1000
> >>net/e1000: add missing i225 devices
> >>net/e1000: add missing hardware support
> >>net/e1000/base: correct minor formatting issues
> >>net/e1000/base: correct mPHY access logic
> >>net/e1000/base: update readme
> >>
> >> Barbara Skobiej (2):
> >>net/e1000/base: fix reset for 82580
> >>net/e1000/base: fix data type in MAC hash
> >>
> >> Carolyn Wyborny (1):
> >>net/e1000/base: skip MANC check for 82575
> >>
> >> Dima Ruinskiy (4):
> >>net/e1000/base: make e1000_access_phy_wakeup_reg_bm non-static
> >>net/e1000/base: make debug prints more informative
> >>net/e1000/base: hardcode bus parameters for ICH8
> >>net/e1000/base: fix unchecked return
> >>
> >> Evgeny Efimov (1):
> >>net/e1000/base: add EEE common API function
> >>
> >> Jakub Buchocki (1):
> >>net/e1000/base: fix uninitialized variable usage
> >>
> >> Marcin Jurczak (1):
> >>net/e1000/base: remove non-inclusive language
> >>
> >> Nir Efrati (6):
> >>net/e1000/base: workaround for packet loss
> >>net/e1000/base: add definition for EXFWSM register
> >>net/e1000/base: use longer ULP exit timeout on more HW
> >>net/e1000/base: remove redundant access to RO register
> >>net/e1000/base: introduce PHY ID retry mechanism
> >>net/e1000/base: add PHY read/write retry mechanism
> >>
> >> Pawel Malinowski (1):
> >>net/e1000/base: fix semaphore timeout value
> >>
> >> Piotr Kubaj (1):
> >>net/e1000/base: rename NVM version variable
> >>
> >> Piotr Pietruszewski (1):
> >>net/e1000/base: improve code flow in ICH8LAN
> >>
> >> Przemyslaw Ciesielski (1):
> >>net/e1000/base: fix static analysis warnings
> >>
> >> Sasha Neftin (4):
> >>net/e1000/base: add queue select definitions
> >>net/e1000/base: add profile information field
> >>net/e1000/base: add LPI counters
> >>net/e1000/base: improve NVM checksum handling
> >>
> >> Vitaly Lifshits (2):
> >>net/e1000: add support for more I219 devices
> >>net/e1000/base: correct disable k1 logic
> >>
> >> drivers/net/intel/e1000/base/README |8 +-
> >> .../net/intel/e1000/base/e1000_80003es2lan.c | 10 +-
> >> drivers/net/intel/e1000/base/e1000_82571.c|4 +-
> >> drivers/net/intel/e1000/base/e1000_82575.c| 21 +-
> >> drivers/net/intel/e1000/base/e1000_82575.h| 29 -
> >> drivers/net/intel/e1000/base/e1000_api.c | 76 +-
> >> drivers/net/intel/e1000/base/e1000_api.h |4 +-
> >> drivers/net/intel/e1000/base/e1000_base.c |3 +-
> >> drivers/net/intel/e1000/base/e1000_defines.h | 259 +-
> >> drivers/net/intel/e1000/base/e1000_hw.h | 86 +-
> >> drivers/net/intel/e1000/base/e1000_i210.c | 14 +-
> >> drivers/net/intel/e1000/base/e1000_i210.h |4 +
> >> drivers/net/intel/e1000/base/e1000_i225.c | 1384 ++
> >> drivers/net/intel/e1000/base/e1000_i225.h | 117 +
> >> drivers/net/intel/e1000/base/e1000_ich8lan.c | 224 +-
> >> drivers/net/intel/e1000/base/e1000_ich8lan.h |3 +-
> >> drivers/net/intel/e1000/base/e1000_mac.c | 62 +-
> >> drivers/net/intel/e1000/base/e1000_mac.h |2 +-
> >> drivers/net/intel/e1000/base/e1000_nvm.c |7 +-
> >> drivers/net/intel/e1000/base/e1000_osdep.h| 33 +-
> >> drivers/net/intel/e1000/base/e1000_phy.c | 447 +-
> >> drivers/net/intel/e1000/base/e1000_phy.h | 21 +
> >> drivers/net/intel/e1000/base/e1000_regs.h | 48 +-
> >> drivers/net/intel/e1000/base/e1000_vf.c | 14 +-
> >> drivers/net/intel/e1000/base/meson.build |1 +
> >> drivers/net/intel/e1000/em_ethdev.c | 36 +-
> >> drivers/net/intel/e1000/igb_ethdev.c |1 +
> >> drivers/net/intel/{igc => e10
Re: [PATCH v4 0/7] eliminate dependency on non-portable __SIZEOF_LONG__
On Tue, Feb 04, 2025 at 10:54:24AM -0800, Andre Muezerie wrote: > Macro __SIZEOF_LONG__ is not standardized and MSVC does not define it. > Therefore the errors below are seen with MSVC: > > ../lib/mldev/mldev_utils_scalar.c(465): error C2065: > '__SIZEOF_LONG__': undeclared identifier > ../lib/mldev/mldev_utils_scalar.c(478): error C2051: > case expression not constant > > ../lib/mldev/mldev_utils_scalar_bfloat16.c(33): error C2065: > '__SIZEOF_LONG__': undeclared identifier > ../lib/mldev/mldev_utils_scalar_bfloat16.c(49): error C2051: > case expression not constant > > Turns out that the places where __SIZEOF_LONG__ is currently > being used can equally well use sizeof(long) instead. > > v4: > * rebased on latest main as previous patch was not applying cleanly >anymore. > > v3: > * added prefix RTE_ to BITS_PER_LONG* and moved them to rte_common.h > * defined PLT_BITS_PER_LONG* in drivers/common/cnxk/roc_platform.h to >avoid warnings from checkpatches.sh like: > >Warning in drivers/common/cnxk/roc_bits.h: >Warning in drivers/common/cnxk/roc_ie_ot.h: >Warning in drivers/common/cnxk/roc_ie_ot_tls.h: >Use plt_ symbols instead of rte_ API in cnxk base driver > >It can be seen that the same was done in the past for similar >macros like PLT_CACHE_LINE_SIZE > > v2: > * fixed typo in commit message > > Andre Muezerie (7): > eal: eliminate dependency on non-portable __SIZEOF_LONG__ > drivers/bus: eliminate dependency on non-portable __SIZEOF_LONG__ > drivers/common: eliminate dependency on non-portable __SIZEOF_LONG__ > drivers/dma: eliminate dependency on non-portable __SIZEOF_LONG__ > drivers/net: eliminate dependency on non-portable __SIZEOF_LONG__ > drivers/raw: eliminate dependency on non-portable __SIZEOF_LONG__ > mldev: eliminate dependency on non-portable __SIZEOF_LONG__ > Just out of interest, is there are reason why the simple solution of just putting "#define __SIZEOF_LONG__ (sizeof(long))" in a header file for MSVC is not done? Should be a couple of lines in a single patch, rather than a 7-patch series, no? After all, just because something is non-standard, doesn't mean that we can't use it if its widely available. /Bruce
Re: [PATCH v7 04/15] net/xsc: add xsc dev ops to support VFIO driver
28/01/2025 15:46, Renyong Wan: > XSC PMD is designed to support both VFIO and private kernel drivers. What's the benefit of private kernel drivers? Why are they private?
RE: [PATCH v4 0/7] eliminate dependency on non-portable __SIZEOF_LONG__
> On Wed, Feb 05, 2025 at 07:37:21AM -0800, Andre Muezerie wrote: > > On Wed, Feb 05, 2025 at 09:15:43AM +, Bruce Richardson wrote: > > > On Tue, Feb 04, 2025 at 10:54:24AM -0800, Andre Muezerie wrote: > > > > Macro __SIZEOF_LONG__ is not standardized and MSVC does not define it. > > > > Therefore the errors below are seen with MSVC: > > > > > > > > ../lib/mldev/mldev_utils_scalar.c(465): error C2065: > > > > '__SIZEOF_LONG__': undeclared identifier > > > > ../lib/mldev/mldev_utils_scalar.c(478): error C2051: > > > > case expression not constant > > > > > > > > ../lib/mldev/mldev_utils_scalar_bfloat16.c(33): error C2065: > > > > '__SIZEOF_LONG__': undeclared identifier > > > > ../lib/mldev/mldev_utils_scalar_bfloat16.c(49): error C2051: > > > > case expression not constant > > > > > > > > Turns out that the places where __SIZEOF_LONG__ is currently > > > > being used can equally well use sizeof(long) instead. > > > > > > > > v4: > > > > * rebased on latest main as previous patch was not applying cleanly > > > >anymore. > > > > > > > > v3: > > > > * added prefix RTE_ to BITS_PER_LONG* and moved them to rte_common.h > > > > * defined PLT_BITS_PER_LONG* in drivers/common/cnxk/roc_platform.h to > > > >avoid warnings from checkpatches.sh like: > > > > > > > >Warning in drivers/common/cnxk/roc_bits.h: > > > >Warning in drivers/common/cnxk/roc_ie_ot.h: > > > >Warning in drivers/common/cnxk/roc_ie_ot_tls.h: > > > >Use plt_ symbols instead of rte_ API in cnxk base driver > > > > > > > >It can be seen that the same was done in the past for similar > > > >macros like PLT_CACHE_LINE_SIZE > > > > > > > > v2: > > > > * fixed typo in commit message > > > > > > > > Andre Muezerie (7): > > > > eal: eliminate dependency on non-portable __SIZEOF_LONG__ > > > > drivers/bus: eliminate dependency on non-portable __SIZEOF_LONG__ > > > > drivers/common: eliminate dependency on non-portable __SIZEOF_LONG__ > > > > drivers/dma: eliminate dependency on non-portable __SIZEOF_LONG__ > > > > drivers/net: eliminate dependency on non-portable __SIZEOF_LONG__ > > > > drivers/raw: eliminate dependency on non-portable __SIZEOF_LONG__ > > > > mldev: eliminate dependency on non-portable __SIZEOF_LONG__ > > > > > > > Just out of interest, is there are reason why the simple solution of just > > > putting "#define __SIZEOF_LONG__ (sizeof(long))" in a header file for MSVC > > > is not done? Should be a couple of lines in a single patch, rather than a > > > 7-patch series, no? > > > > > > After all, just because something is non-standard, doesn't mean that we > > > can't use it if its widely available. > > > > > > /Bruce > > > > Yes, that can be done instead. I'll send out a new series with that > > approach. > > > Maybe wait in case there is input from others before spending time on a > patch? I think the simpler solution is better, but others may feel that > removing the macro completely is the better approach. +1 to what Bruce suggested.
[PATCH 03/11] net/bnxt: fix Rx handler
Fix the code accounting the status of compressed CQE handler.
The code was inside the block handling the normal CQE mode.
Moved the code out. Without this the Rx datapath was broken
for compressed CQEs in scalar mode.
Fixes: 5c980062d895 ("net/bnxt: support compressed Rx CQE")
Cc: sta...@dpdk.org
Signed-off-by: Ajit Khaparde
---
drivers/net/bnxt/bnxt_rxr.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
index 5b43bcbea6..b53d9a917a 100644
--- a/drivers/net/bnxt/bnxt_rxr.c
+++ b/drivers/net/bnxt/bnxt_rxr.c
@@ -1390,14 +1390,6 @@ uint16_t bnxt_recv_pkts(void *rx_queue, struct rte_mbuf
**rx_pkts,
} else if ((CMP_TYPE(rxcmp) >= CMPL_BASE_TYPE_RX_TPA_START_V2)
&&
(CMP_TYPE(rxcmp) <= CMPL_BASE_TYPE_RX_TPA_START_V3))
{
rc = bnxt_rx_pkt(&rx_pkts[nb_rx_pkts], rxq, &raw_cons);
- if (!rc)
- nb_rx_pkts++;
- else if (rc == -EBUSY) /* partial completion */
- break;
- else if (rc == -ENODEV) /* completion for representor */
- nb_rep_rx_pkts++;
- else if (rc == -ENOMEM)
- nb_rx_pkts++;
} else if (!BNXT_NUM_ASYNC_CPR(rxq->bp)) {
evt =
bnxt_event_hwrm_resp_handler(rxq->bp,
@@ -1407,6 +1399,14 @@ uint16_t bnxt_recv_pkts(void *rx_queue, struct rte_mbuf
**rx_pkts,
goto done;
}
+ if (!rc)
+ nb_rx_pkts++;
+ else if (rc == -EBUSY) /* partial completion */
+ break;
+ else if (rc == -ENODEV) /* completion for representor */
+ nb_rep_rx_pkts++;
+ else if (rc == -ENOMEM)
+ nb_rx_pkts++;
raw_cons = NEXT_RAW_CMP(raw_cons);
/*
* The HW reposting may fall behind if mbuf allocation has
--
2.39.5 (Apple Git-154)
[PATCH 00/11] bnxt patch set
Patchset with bug fixes for bnxt PMD. Also addresses various Coverity issues reported in the recent Coverity scan. Please apply. Ajit Khaparde (4): net/bnxt: disable TruFlow if compressed CQE is set net/bnxt: simplify check for CQE mode net/bnxt: fix Rx handler net/bnxt: fix burst handler initialization Peter Spreadborough (6): net/bnxt: address coverity deadcode issue net/bnxt: address coverity checked return issues net/bnxt: address coverity overflow issues net/bnxt: address coverity integer overflow issues net/bnxt: address coverity uninitialized variables issues net/bnxt: address coverity control flow issues Sangtani Parag Satishbhai (1): net/bnxt/truFlow: Fix seg fault when rep are re-attached drivers/net/bnxt/bnxt.h | 4 - drivers/net/bnxt/bnxt_ethdev.c| 31 ++-- drivers/net/bnxt/bnxt_hwrm.c | 2 +- drivers/net/bnxt/bnxt_rxr.c | 16 +- .../hcapi/cfa_v3/bld/p70/cfa_bld_p70_mpc.c| 90 +-- .../p70/host/cfa_bld_p70_host_mpc_wrapper.c | 150 +- drivers/net/bnxt/hcapi/cfa_v3/mm/cfa_mm.c | 6 +- drivers/net/bnxt/tf_core/v3/tfc_em.c | 1 + drivers/net/bnxt/tf_core/v3/tfc_tbl_scope.c | 2 +- drivers/net/bnxt/tf_ulp/bnxt_ulp_utils.h | 2 +- drivers/net/bnxt/tf_ulp/ulp_mapper.c | 10 +- drivers/net/bnxt/tf_ulp/ulp_rte_parser.c | 8 +- drivers/net/bnxt/tf_ulp/ulp_sc_mgr.c | 1 + 13 files changed, 174 insertions(+), 149 deletions(-) -- 2.39.5 (Apple Git-154)
[PATCH 01/11] net/bnxt: disable TruFlow if compressed CQE is set
If a user selects compressed CQE mode using the cqe-mode devarg,
it is clear that the user does not intend to use TruFlow mode.
Since host backed TruFlow setting is enable by default now, disable
TruFlow during initialization to prevent unexpected behavior
when the two get enabled.
Signed-off-by: Ajit Khaparde
Signed-off-by: Kalesh AP
---
drivers/net/bnxt/bnxt_ethdev.c | 5 +
drivers/net/bnxt/bnxt_hwrm.c | 2 +-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 21e9aa902c..81a7723c7e 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -5785,6 +5785,11 @@ static int bnxt_get_config(struct bnxt *bp)
if (rc)
return rc;
+ if (bnxt_compressed_rx_cqe_mode_enabled(bp)) {
+ PMD_DRV_LOG_LINE(INFO, "Compressed CQE is set. Truflow is
disabled.");
+ bp->fw_cap &= ~BNXT_FW_CAP_TRUFLOW_EN;
+ }
+
rc = bnxt_hwrm_queue_qportcfg(bp);
if (rc)
return rc;
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index d015ba2b9c..305c419051 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -1731,7 +1731,7 @@ int bnxt_hwrm_ver_get(struct bnxt *bp, uint32_t timeout)
if (dev_caps_cfg &
HWRM_VER_GET_OUTPUT_DEV_CAPS_CFG_CFA_TRUFLOW_SUPPORTED) {
- PMD_DRV_LOG_LINE(DEBUG, "Host-based truflow feature enabled.");
+ PMD_DRV_LOG_LINE(DEBUG, "Host-based truflow feature
supported.");
bp->fw_cap |= BNXT_FW_CAP_TRUFLOW_EN;
}
--
2.39.5 (Apple Git-154)
[PATCH 05/11] net/bnxt/truFlow: Fix seg fault when rep are re-attached
From: Sangtani Parag Satishbhai
When the PCI port is detached using the testpmd command,
as part of cleanup testpmd removes resources of parent
port and all the children's ports and calls the driver specific
pci_remove API with the parent rte ethdev to clean-up ethdevs.
For the bnxt driver, a condition to check type of ethdev is added
in bnxt_pci_remove and based on the condition relevant
ethdev is removed (VF/PF or VFR). As the RTE layer always
calls PCI remove with the parent ethdev, the bnxt_pci_remove
never frees children (VFRs) ethdev. As, these ethdevs were not
freed it gives spurious status in re-allocation check(when pci
port attach command is executed) and when RTE layers tries to
access interrupt specific info from the ethdev due to uninitialized
members it access NULL pointer which results in seg fault. The fix
is made in bnxt_pci_remove to clean ethdev for parent (PF/VF) along
with children (VFRs).
Fixes: 322bd6e70272 ("net/bnxt: add port representor infrastructure")
Cc: sta...@dpdk.org
Signed-off-by: Sangtani Parag Satishbhai
Reviewed-by: Somnath Kotur
Reviewed-by: Kalesh AP
Reviewed-by: Ajit Khaparde
---
drivers/net/bnxt/bnxt_ethdev.c | 22 +++---
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index b18247feb2..144d4377bd 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -6993,6 +6993,8 @@ static int bnxt_pci_probe(struct rte_pci_driver *pci_drv
__rte_unused,
static int bnxt_pci_remove(struct rte_pci_device *pci_dev)
{
struct rte_eth_dev *eth_dev;
+ uint16_t port_id;
+ int rc = 0;
eth_dev = rte_eth_dev_allocated(pci_dev->device.name);
if (!eth_dev)
@@ -7002,14 +7004,20 @@ static int bnxt_pci_remove(struct rte_pci_device
*pci_dev)
* +ve value will at least help in proper cleanup
*/
- PMD_DRV_LOG_LINE(DEBUG, "BNXT Port:%d pci remove",
eth_dev->data->port_id);
if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
- if (rte_eth_dev_is_repr(eth_dev))
- return rte_eth_dev_destroy(eth_dev,
- bnxt_representor_uninit);
- else
- return rte_eth_dev_destroy(eth_dev,
- bnxt_dev_uninit);
+ RTE_ETH_FOREACH_DEV_OF(port_id, &pci_dev->device) {
+ PMD_DRV_LOG_LINE(DEBUG, "BNXT Port:%d pci remove",
port_id);
+ eth_dev = &rte_eth_devices[port_id];
+ if (eth_dev->data->dev_flags & RTE_ETH_DEV_REPRESENTOR)
+ rc = rte_eth_dev_destroy(eth_dev,
+
bnxt_representor_uninit);
+ else
+ rc = rte_eth_dev_destroy(eth_dev,
+bnxt_dev_uninit);
+ if (rc != 0)
+ return rc;
+ }
+ return rc;
} else {
return rte_eth_dev_pci_generic_remove(pci_dev, NULL);
}
--
2.39.5 (Apple Git-154)
[PATCH 04/11] net/bnxt: set burst handler correctly
We are incorrectly setting the Rx and Tx burst handlers to static mode by default. Fix that by using the bnxt_receive_function and bnxt_transmit_function calls to determine if the vector mode is enabled and identify the appropriate burst handler during the initialization. Signed-off-by: Ajit Khaparde Reviewed-by: Somnath Kotur Reviewed-by: Damodharam Ammepalli Reviewed-by: Kalesh AP --- drivers/net/bnxt/bnxt_ethdev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c index 81a7723c7e..b18247feb2 100644 --- a/drivers/net/bnxt/bnxt_ethdev.c +++ b/drivers/net/bnxt/bnxt_ethdev.c @@ -6532,8 +6532,8 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev, void *params __rte_unused) eth_dev->rx_queue_count = bnxt_rx_queue_count_op; eth_dev->rx_descriptor_status = bnxt_rx_descriptor_status_op; eth_dev->tx_descriptor_status = bnxt_tx_descriptor_status_op; - eth_dev->rx_pkt_burst = &bnxt_recv_pkts; - eth_dev->tx_pkt_burst = &bnxt_xmit_pkts; + eth_dev->rx_pkt_burst = bnxt_receive_function(eth_dev); + eth_dev->tx_pkt_burst = bnxt_transmit_function(eth_dev); /* * For secondary processes, we don't initialise any further -- 2.39.5 (Apple Git-154)
[PATCH 02/11] net/bnxt: simplify check for CQE mode
Simplify the check for the CQE mode.
We don't have to check the Rx offload mode to determine
which CQE mode is supported.
CQE mode is configured at load time and once set will decide
if TCP LRO or buffer split can be supported or not.
Signed-off-by: Ajit Khaparde
Reviewed-by: Somnath Kotur
Reviewed-by: Damodharam Ammepalli
Reviewed-by: Kalesh AP
---
drivers/net/bnxt/bnxt.h | 4
1 file changed, 4 deletions(-)
diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index 4a5c224c09..c9fdd36d3e 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -1112,12 +1112,8 @@ inline uint16_t bnxt_max_rings(struct bnxt *bp)
static inline bool
bnxt_compressed_rx_cqe_mode_enabled(struct bnxt *bp)
{
- uint64_t rx_offloads = bp->eth_dev->data->dev_conf.rxmode.offloads;
-
if (bp->vnic_cap_flags & BNXT_VNIC_CAP_L2_CQE_MODE &&
bp->flags2 & BNXT_FLAGS2_COMPRESSED_RX_CQE &&
- !(rx_offloads & RTE_ETH_RX_OFFLOAD_TCP_LRO) &&
- !(rx_offloads & RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT) &&
!bp->num_reps && !bp->ieee_1588)
return true;
--
2.39.5 (Apple Git-154)
[PATCH 06/11] net/bnxt: address coverity deadcode issue
From: Peter Spreadborough
This change addresses the CID 449075: Control flow issues (DEADCODE)
reported by coverity associated with the ASSERT_RETURN macro. The
change renames the macro and replaces the log macro to use
PMD_DRV_LOG_LINE.
Coverity issue: 449075
Signed-off-by: Peter Spreadborough
Reviewed-by: Ajit Khaparde
---
.../hcapi/cfa_v3/bld/p70/cfa_bld_p70_mpc.c| 90 +--
.../p70/host/cfa_bld_p70_host_mpc_wrapper.c | 150 +-
2 files changed, 120 insertions(+), 120 deletions(-)
diff --git a/drivers/net/bnxt/hcapi/cfa_v3/bld/p70/cfa_bld_p70_mpc.c
b/drivers/net/bnxt/hcapi/cfa_v3/bld/p70/cfa_bld_p70_mpc.c
index 445d9aad87..3f8ef3a5f1 100644
--- a/drivers/net/bnxt/hcapi/cfa_v3/bld/p70/cfa_bld_p70_mpc.c
+++ b/drivers/net/bnxt/hcapi/cfa_v3/bld/p70/cfa_bld_p70_mpc.c
@@ -31,9 +31,9 @@
} while (0)
#ifdef NXT_ENV_DEBUG
-#define ASSERT_RETURN(ERRNO) CFA_LOG_ERR("Returning error: %d\n", (ERRNO))
+#define LOG_RC(ERRNO) PMD_DRV_LOG_LINE(ERR, "Returning error: %d", (ERRNO))
#else
-#define ASSERT_RETURN(ERRNO)
+#define LOG_RC(ERRNO)
#endif
/**
@@ -68,7 +68,7 @@ static int fill_mpc_header(uint8_t *cmd, uint32_t size,
uint32_t opaque_val)
};
if (size < sizeof(struct mpc_header)) {
- ASSERT_RETURN(-EINVAL);
+ LOG_RC(-EINVAL);
return -EINVAL;
}
@@ -87,17 +87,17 @@ static int compose_mpc_read_clr_msg(uint8_t *cmd_buff,
uint32_t *cmd_buff_len,
sizeof(struct mpc_header) + sizeof(struct cfa_mpc_read_clr_cmd);
if (parms->data_size != 1) {
- ASSERT_RETURN(-EINVAL);
+ LOG_RC(-EINVAL);
return -EINVAL;
}
if (parms->tbl_type >= CFA_HW_TABLE_MAX) {
- ASSERT_RETURN(-EINVAL);
+ LOG_RC(-EINVAL);
return -EINVAL;
}
if (*cmd_buff_len < cmd_size) {
- ASSERT_RETURN(-EINVAL);
+ LOG_RC(-EINVAL);
return -EINVAL;
}
@@ -138,17 +138,17 @@ static int compose_mpc_read_msg(uint8_t *cmd_buff,
uint32_t *cmd_buff_len,
sizeof(struct mpc_header) + sizeof(struct cfa_mpc_read_cmd);
if (parms->data_size < 1 || parms->data_size > 4) {
- ASSERT_RETURN(-EINVAL);
+ LOG_RC(-EINVAL);
return -EINVAL;
}
if (parms->tbl_type >= CFA_HW_TABLE_MAX) {
- ASSERT_RETURN(-EINVAL);
+ LOG_RC(-EINVAL);
return -EINVAL;
}
if (*cmd_buff_len < cmd_size) {
- ASSERT_RETURN(-EINVAL);
+ LOG_RC(-EINVAL);
return -EINVAL;
}
@@ -194,22 +194,22 @@ static int compose_mpc_write_msg(uint8_t *cmd_buff,
uint32_t *cmd_buff_len,
parms->data_size * MPC_CFA_CACHE_ACCESS_UNIT_SIZE;
if (parms->data_size < 1 || parms->data_size > 4) {
- ASSERT_RETURN(-EINVAL);
+ LOG_RC(-EINVAL);
return -EINVAL;
}
if (parms->tbl_type >= CFA_HW_TABLE_MAX) {
- ASSERT_RETURN(-EINVAL);
+ LOG_RC(-EINVAL);
return -EINVAL;
}
if (!parms->write.data_ptr) {
- ASSERT_RETURN(-EINVAL);
+ LOG_RC(-EINVAL);
return -EINVAL;
}
if (*cmd_buff_len < cmd_size) {
- ASSERT_RETURN(-EINVAL);
+ LOG_RC(-EINVAL);
return -EINVAL;
}
@@ -252,17 +252,17 @@ static int compose_mpc_evict_msg(uint8_t *cmd_buff,
uint32_t *cmd_buff_len,
sizeof(struct cfa_mpc_invalidate_cmd);
if (parms->data_size < 1 || parms->data_size > 4) {
- ASSERT_RETURN(-EINVAL);
+ LOG_RC(-EINVAL);
return -EINVAL;
}
if (parms->tbl_type >= CFA_HW_TABLE_MAX) {
- ASSERT_RETURN(-EINVAL);
+ LOG_RC(-EINVAL);
return -EINVAL;
}
if (*cmd_buff_len < cmd_size) {
- ASSERT_RETURN(-EINVAL);
+ LOG_RC(-EINVAL);
return -EINVAL;
}
@@ -293,7 +293,7 @@ static int compose_mpc_evict_msg(uint8_t *cmd_buff,
uint32_t *cmd_buff_len,
break;
case CFA_MPC_EV_EVICT_TABLE_SCOPE:
/* Not supported */
- ASSERT_RETURN(-ENOTSUP);
+ LOG_RC(-ENOTSUP);
return -ENOTSUP;
default:
case CFA_MPC_EV_EVICT_SCOPE_ADDRESS:
@@ -326,7 +326,7 @@ int cfa_mpc_build_cache_axs_cmd(enum cfa_mpc_opcode opc,
uint8_t *cmd_buff,
{
int rc;
if (!cmd_buff || !cmd_buff_len || *cmd_buff_len == 0 || !parms) {
- ASSERT_RETURN(-EINVAL);
+ LOG_RC(-EINVAL);
return -EINVAL;
}
@@ -344,7 +344,7 @@ int cfa_mpc_build_cache_axs_cmd(enum cfa_mpc_opcode opc,
uint8_t *cmd_buff,
[PATCH 07/11] net/bnxt: address coverity checked return issues
From: Peter Spreadborough
This change addresses the CID 449056: Error handling issues (CHECKED_RETURN)
reported by coverity. The changes add return code handling to
address the issue.
Coverity issue: 449056
Signed-off-by: Peter Spreadborough
Reviewed-by: Ajit Khaparde
---
drivers/net/bnxt/tf_ulp/ulp_mapper.c | 10 +++---
drivers/net/bnxt/tf_ulp/ulp_rte_parser.c | 8 +++-
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/net/bnxt/tf_ulp/ulp_mapper.c
b/drivers/net/bnxt/tf_ulp/ulp_mapper.c
index 2429ac2f1a..1a68cf5dfd 100644
--- a/drivers/net/bnxt/tf_ulp/ulp_mapper.c
+++ b/drivers/net/bnxt/tf_ulp/ulp_mapper.c
@@ -3612,9 +3612,13 @@ ulp_mapper_func_cond_list_process(struct
bnxt_ulp_mapper_parms *parms,
}
}
/* write the value into result */
- ulp_operand_read(val, res_local + res_size -
-ULP_BITS_2_BYTE_NR(oper_size),
-ULP_BITS_2_BYTE_NR(val_len));
+ if (unlikely(ulp_operand_read(val, res_local + res_size -
+ ULP_BITS_2_BYTE_NR(oper_size),
+ ULP_BITS_2_BYTE_NR(val_len {
+ BNXT_DRV_DBG(ERR,
+"field idx operand read failed\n");
+ return -EINVAL;
+ }
/* convert the data to cpu format */
*res = tfp_be_to_cpu_64(*res);
diff --git a/drivers/net/bnxt/tf_ulp/ulp_rte_parser.c
b/drivers/net/bnxt/tf_ulp/ulp_rte_parser.c
index dd5985cd7b..bf3a3deb18 100644
--- a/drivers/net/bnxt/tf_ulp/ulp_rte_parser.c
+++ b/drivers/net/bnxt/tf_ulp/ulp_rte_parser.c
@@ -517,7 +517,13 @@ ulp_rte_parser_svif_set(struct ulp_rte_parser_params
*params,
else
svif_type = BNXT_ULP_DRV_FUNC_SVIF;
}
- ulp_port_db_svif_get(params->ulp_ctx, ifindex, svif_type, &svif);
+
+ if (ulp_port_db_svif_get(params->ulp_ctx, ifindex,
+svif_type, &svif)) {
+ BNXT_DRV_DBG(ERR, "ParseErr:ifindex is not valid\n");
+ return BNXT_TF_RC_ERROR;
+ }
+
svif = rte_cpu_to_be_16(svif);
mask = rte_cpu_to_be_16(mask);
hdr_field = ¶ms->hdr_field[BNXT_ULP_PROTO_HDR_FIELD_SVIF_IDX];
--
2.39.5 (Apple Git-154)
[PATCH 09/11] net/bnxt: address coverity integer overflow issues
From: Peter Spreadborough
This change addresses the CID 449059: Integer handling issues
(INTEGER_OVERFLOW) reported by coverity.
Coverity issue: 449059
Signed-off-by: Peter Spreadborough
Reviewed-by: Ajit Khaparde
---
drivers/net/bnxt/hcapi/cfa_v3/mm/cfa_mm.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/bnxt/hcapi/cfa_v3/mm/cfa_mm.c
b/drivers/net/bnxt/hcapi/cfa_v3/mm/cfa_mm.c
index 61fafadb20..05528dd3e4 100644
--- a/drivers/net/bnxt/hcapi/cfa_v3/mm/cfa_mm.c
+++ b/drivers/net/bnxt/hcapi/cfa_v3/mm/cfa_mm.c
@@ -123,7 +123,11 @@ int cfa_mm_open(void *cmm, struct cfa_mm_open_parms *parms)
}
for (i = 0; i < num_blocks; i++) {
- context->blk_tbl[i].prev_blk_idx = i - 1;
+ if (i == 0)
+ context->blk_tbl[i].prev_blk_idx = CFA_MM_INVALID32;
+ else
+ context->blk_tbl[i].prev_blk_idx = i - 1;
+
context->blk_tbl[i].next_blk_idx = i + 1;
context->blk_tbl[i].num_free_records = records_per_block;
context->blk_tbl[i].first_free_record = 0;
--
2.39.5 (Apple Git-154)
[PATCH 11/11] net/bnxt: address coverity control flow issues
From: Peter Spreadborough This change addresses the CID 449072: Control flow issues (DEADCODE) reported by coverity. Coverity issue: 449072 Signed-off-by: Peter Spreadborough Reviewed-by: Ajit Khaparde --- drivers/net/bnxt/tf_ulp/bnxt_ulp_utils.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/bnxt/tf_ulp/bnxt_ulp_utils.h b/drivers/net/bnxt/tf_ulp/bnxt_ulp_utils.h index 5e0d906fbd..e6f316539c 100644 --- a/drivers/net/bnxt/tf_ulp/bnxt_ulp_utils.h +++ b/drivers/net/bnxt/tf_ulp/bnxt_ulp_utils.h @@ -1084,7 +1084,7 @@ bnxt_ulp_cap_feat_process(uint64_t feat_bits, uint64_t *out_bits) if (bit & BNXT_ULP_FEATURE_BIT_PARENT_DMAC) BNXT_DRV_DBG(ERR, "Parent Mac Address Feature is enabled\n"); - if (bit & BNXT_ULP_FEATURE_BIT_PORT_DMAC) + else if (bit & BNXT_ULP_FEATURE_BIT_PORT_DMAC) BNXT_DRV_DBG(ERR, "Port Mac Address Feature is enabled\n"); if (bit & BNXT_ULP_FEATURE_BIT_MULTI_TUNNEL_FLOW) BNXT_DRV_DBG(ERR, "Multi Tunnel Flow Feature is enabled\n"); -- 2.39.5 (Apple Git-154)
[PATCH 08/11] net/bnxt: address coverity overflow issues
From: Peter Spreadborough This change addresses the CID 449058: Integer handling issues (OVERFLOW_BEFORE_WIDEN) reported by coverity. Coverity issue: 449058 Signed-off-by: Peter Spreadborough Reviewed-by: Ajit Khaparde --- drivers/net/bnxt/tf_core/v3/tfc_tbl_scope.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/bnxt/tf_core/v3/tfc_tbl_scope.c b/drivers/net/bnxt/tf_core/v3/tfc_tbl_scope.c index 1770069295..c29933b803 100644 --- a/drivers/net/bnxt/tf_core/v3/tfc_tbl_scope.c +++ b/drivers/net/bnxt/tf_core/v3/tfc_tbl_scope.c @@ -468,7 +468,7 @@ static int alloc_link_pbl(struct tfc_ts_mem_cfg *mem_cfg, uint32_t page_size, * and page tables. The allocation will occur once only per backing * store and will located by name and reused on subsequent runs. */ - total_size = page_size * total_pages; + total_size = (uint64_t)page_size * (uint64_t)total_pages; if (total_size <= (1024 * 256)) mz_size = RTE_MEMZONE_256KB; -- 2.39.5 (Apple Git-154)
[PATCH 10/11] net/bnxt: address coverity uninitialized variables issues
From: Peter Spreadborough
This change addresses the CID 449065: Uninitialized variables (UNINIT)
issues reported by coverity.
Coverity issue: 449065
Signed-off-by: Peter Spreadborough
Reviewed-by: Ajit Khaparde
---
drivers/net/bnxt/tf_core/v3/tfc_em.c | 1 +
drivers/net/bnxt/tf_ulp/ulp_sc_mgr.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/net/bnxt/tf_core/v3/tfc_em.c
b/drivers/net/bnxt/tf_core/v3/tfc_em.c
index a70e35b6b1..d460ff2ee0 100644
--- a/drivers/net/bnxt/tf_core/v3/tfc_em.c
+++ b/drivers/net/bnxt/tf_core/v3/tfc_em.c
@@ -560,6 +560,7 @@ int tfc_em_delete_raw(struct tfc *tfcp,
mpc_msg_out.cmp_type = CMPL_BASE_TYPE_MID_PATH_LONG;
mpc_msg_out.msg_data = &rx_msg[TFC_MPC_HEADER_SIZE_BYTES];
mpc_msg_out.msg_size = TFC_MPC_MAX_RX_BYTES;
+ mpc_msg_out.chnl_id = 0;
rc = tfc_mpc_send(tfcp->bp,
&mpc_msg_in,
diff --git a/drivers/net/bnxt/tf_ulp/ulp_sc_mgr.c
b/drivers/net/bnxt/tf_ulp/ulp_sc_mgr.c
index 5fa8e240db..57cbaaf09c 100644
--- a/drivers/net/bnxt/tf_ulp/ulp_sc_mgr.c
+++ b/drivers/net/bnxt/tf_ulp/ulp_sc_mgr.c
@@ -224,6 +224,7 @@ static uint32_t ulp_stats_cache_main_loop(void *arg)
if (bnxt_ulp_cntxt_acquire_fdb_lock(ctxt))
break;
+ batch_info.enabled = false;
rc = tfc_mpc_batch_start(&batch_info);
if (unlikely(rc)) {
PMD_DRV_LOG_LINE(ERR,
--
2.39.5 (Apple Git-154)
Re: [PATCH] build: remove support for icc compiler
On Wed, Feb 5, 2025 at 5:19 PM Bruce Richardson wrote: > > The Intel-produced compiler "icc" has been replaced by the newer > clang-based "icx" compiler. DPDK compilation has also not been tested > recently with the icc compiler, so let's remove doc and code references > to icc, and any special macros or build support that was added for it. > > Signed-off-by: Bruce Richardson I noticed remaining references, from which some can be cleaned up too: app/test-pmd/testpmd.h: * Work-around of a compilation error with ICC on invocations of the lib/eal/common/eal_common_dynmem.c: /* to prevent icc errors */ lib/eal/x86/include/rte_vect.h:#if defined(__ICC) || defined(_WIN64) buildtools/check-symbols.sh:# Filter out symbols suffixed with a . for icc buildtools/check-symbols.sh:# Filter out symbols suffixed with a . for icc And please add a release note update. Thanks. -- David Marchand
[PATCH v3 0/2] remove unused variables and add MSVC compiler flag
v3: * rebased. Removed unused variables and added MSVC specific compiler flag to ignore warnings about unused variables, like is being done for other compilers. Andre Muezerie (2): drivers/common: remove unused variables and add MSVC compiler flag drivers/net: add MSVC compiler flag for unused variables drivers/common/idpf/base/meson.build | 13 +++-- drivers/common/idpf/idpf_common_rxtx.c | 2 - drivers/common/idpf/idpf_common_virtchnl.c | 4 +- drivers/common/sfc_efx/base/efx_mae.c | 2 +- drivers/common/sfc_efx/base/efx_table.c| 1 - drivers/common/sfc_efx/base/meson.build| 20 +--- drivers/net/qede/base/meson.build | 57 -- 7 files changed, 58 insertions(+), 41 deletions(-) -- 2.47.2.vfs.0.1
[PATCH v3 1/2] drivers/common: remove unused variables and add MSVC compiler flag
Removed unused variables and added MSVC specific compiler flag to
ignore warnings about unused variables, like is being done for
other compilers.
Signed-off-by: Andre Muezerie
---
drivers/common/idpf/base/meson.build | 13 ++---
drivers/common/idpf/idpf_common_rxtx.c | 2 --
drivers/common/idpf/idpf_common_virtchnl.c | 4 ++--
drivers/common/sfc_efx/base/efx_mae.c | 2 +-
drivers/common/sfc_efx/base/efx_table.c| 1 -
drivers/common/sfc_efx/base/meson.build| 20 +---
6 files changed, 26 insertions(+), 16 deletions(-)
diff --git a/drivers/common/idpf/base/meson.build
b/drivers/common/idpf/base/meson.build
index f30ec7dfc2..c069507f12 100644
--- a/drivers/common/idpf/base/meson.build
+++ b/drivers/common/idpf/base/meson.build
@@ -6,9 +6,16 @@ sources += files(
'idpf_controlq_setup.c',
)
-error_cflags = [
-'-Wno-unused-variable',
-]
+if is_ms_compiler
+error_cflags = [
+'/wd4101', # unreferenced local variable
+]
+else
+error_cflags = [
+'-Wno-unused-variable',
+]
+endif
+
foreach flag: error_cflags
if cc.has_argument(flag)
cflags += flag
diff --git a/drivers/common/idpf/idpf_common_rxtx.c
b/drivers/common/idpf/idpf_common_rxtx.c
index a04e54ce26..9b17279181 100644
--- a/drivers/common/idpf/idpf_common_rxtx.c
+++ b/drivers/common/idpf/idpf_common_rxtx.c
@@ -1178,7 +1178,6 @@ idpf_dp_singleq_recv_scatter_pkts(void *rx_queue, struct
rte_mbuf **rx_pkts,
struct rte_mbuf *last_seg = rxq->pkt_last_seg;
struct rte_mbuf *rxm;
struct rte_mbuf *nmb;
- struct rte_eth_dev *dev;
const uint32_t *ptype_tbl = rxq->adapter->ptype_tbl;
uint16_t rx_id = rxq->rx_tail;
uint16_t rx_packet_len;
@@ -1310,7 +1309,6 @@ idpf_xmit_cleanup(struct idpf_tx_queue *txq)
uint16_t nb_tx_desc = txq->nb_tx_desc;
uint16_t desc_to_clean_to;
uint16_t nb_tx_to_clean;
- uint16_t i;
volatile struct idpf_base_tx_desc *txd = txq->tx_ring;
diff --git a/drivers/common/idpf/idpf_common_virtchnl.c
b/drivers/common/idpf/idpf_common_virtchnl.c
index de511da788..0ae1d55d79 100644
--- a/drivers/common/idpf/idpf_common_virtchnl.c
+++ b/drivers/common/idpf/idpf_common_virtchnl.c
@@ -362,7 +362,7 @@ idpf_vc_queue_grps_add(struct idpf_vport *vport,
{
struct idpf_adapter *adapter = vport->adapter;
struct idpf_cmd_info args;
- int size, qg_info_size;
+ int size;
int err = -1;
size = sizeof(*p2p_queue_grps_info) +
@@ -1044,7 +1044,7 @@ int idpf_vc_rxq_config_by_info(struct idpf_vport *vport,
struct virtchnl2_rxq_in
struct idpf_adapter *adapter = vport->adapter;
struct virtchnl2_config_rx_queues *vc_rxqs = NULL;
struct idpf_cmd_info args;
- int size, err, i;
+ int size, err;
size = sizeof(*vc_rxqs) + (num_qs - 1) *
sizeof(struct virtchnl2_rxq_info);
diff --git a/drivers/common/sfc_efx/base/efx_mae.c
b/drivers/common/sfc_efx/base/efx_mae.c
index 9ae136dcce..1429e7dd0a 100644
--- a/drivers/common/sfc_efx/base/efx_mae.c
+++ b/drivers/common/sfc_efx/base/efx_mae.c
@@ -740,7 +740,6 @@ efx_mae_mport_by_pcie_function(
__inuint32_t vf,
__out efx_mport_sel_t *mportp)
{
- efx_dword_t dword;
efx_rc_t rc;
rc = efx_mae_mport_by_pcie_mh_function(EFX_PCIE_INTERFACE_CALLER,
@@ -4280,6 +4279,7 @@ efx_mae_action_set_replay(
__out efx_mae_actions_t **spec_clonep)
{
const efx_nic_cfg_t *encp = efx_nic_cfg_get(enp);
+ (void)encp;
efx_mae_actions_t *spec_clone;
efx_rc_t rc;
diff --git a/drivers/common/sfc_efx/base/efx_table.c
b/drivers/common/sfc_efx/base/efx_table.c
index 13a12a116e..50e9a37d1c 100644
--- a/drivers/common/sfc_efx/base/efx_table.c
+++ b/drivers/common/sfc_efx/base/efx_table.c
@@ -240,7 +240,6 @@ efx_table_describe(
const efx_nic_cfg_t *encp = efx_nic_cfg_get(enp);
unsigned int n_entries;
efx_mcdi_req_t req;
- unsigned int i;
efx_rc_t rc;
EFX_MCDI_DECLARE_BUF(payload,
MC_CMD_TABLE_DESCRIPTOR_IN_LEN,
diff --git a/drivers/common/sfc_efx/base/meson.build
b/drivers/common/sfc_efx/base/meson.build
index 7fc04aa57b..c8deb4555e 100644
--- a/drivers/common/sfc_efx/base/meson.build
+++ b/drivers/common/sfc_efx/base/meson.build
@@ -66,13 +66,19 @@ sources = [
'rhead_virtio.c',
]
-extra_flags = [
-'-Wno-sign-compare',
-'-Wno-unused-parameter',
-'-Wno-unused-variable',
-'-Wno-empty-body',
-'-Wno-unused-but-set-variable',
-]
+if is_ms_compiler
+extra_flags = [
+'/wd4101', # unreferenced local variable
+]
+else
+extra_flags = [
+'-Wno-sign-compare',
+'-Wno-unused-parameter',
+'-Wno-unused-variable',
+'-Wno-empty-body',
+
[PATCH v3 2/2] drivers/net: add MSVC compiler flag for unused variables
Added MSVC specific compiler flag to ignore warnings about unused variables, like is being done for other compilers. Signed-off-by: Andre Muezerie --- drivers/net/qede/base/meson.build | 57 +-- 1 file changed, 32 insertions(+), 25 deletions(-) diff --git a/drivers/net/qede/base/meson.build b/drivers/net/qede/base/meson.build index 4ad177b478..66251360bf 100644 --- a/drivers/net/qede/base/meson.build +++ b/drivers/net/qede/base/meson.build @@ -19,31 +19,38 @@ sources = [ ] -error_cflags = [ -'-Wno-unused-parameter', -'-Wno-sign-compare', -'-Wno-missing-prototypes', -'-Wno-cast-qual', -'-Wno-unused-function', -'-Wno-unused-variable', -'-Wno-strict-aliasing', -'-Wno-missing-prototypes', -'-Wno-unused-value', -'-Wno-format-nonliteral', -'-Wno-shift-negative-value', -'-Wno-unused-but-set-variable', -'-Wno-missing-declarations', -'-Wno-maybe-uninitialized', -'-Wno-strict-prototypes', -'-Wno-shift-negative-value', -'-Wno-implicit-fallthrough', -'-Wno-format-extra-args', -'-Wno-visibility', -'-Wno-empty-body', -'-Wno-invalid-source-encoding', -'-Wno-sometimes-uninitialized', -'-Wno-pointer-bool-conversion', -] +if is_ms_compiler +error_cflags = [ +'/wd4101', # unreferenced local variable +] +else +error_cflags = [ +'-Wno-unused-parameter', +'-Wno-sign-compare', +'-Wno-missing-prototypes', +'-Wno-cast-qual', +'-Wno-unused-function', +'-Wno-unused-variable', +'-Wno-strict-aliasing', +'-Wno-missing-prototypes', +'-Wno-unused-value', +'-Wno-format-nonliteral', +'-Wno-shift-negative-value', +'-Wno-unused-but-set-variable', +'-Wno-missing-declarations', +'-Wno-maybe-uninitialized', +'-Wno-strict-prototypes', +'-Wno-shift-negative-value', +'-Wno-implicit-fallthrough', +'-Wno-format-extra-args', +'-Wno-visibility', +'-Wno-empty-body', +'-Wno-invalid-source-encoding', +'-Wno-sometimes-uninitialized', +'-Wno-pointer-bool-conversion', +] +endif + c_args = cflags foreach flag: error_cflags if cc.has_argument(flag) -- 2.47.2.vfs.0.1
Re: [PATCH v2 2/2] drivers/net: add MSVC compiler flag for unused variables
On Wed, Feb 05, 2025 at 10:33:57AM -0800, Stephen Hemminger wrote: > On Tue, 14 Jan 2025 12:46:51 -0800 > Andre Muezerie wrote: > > > Added MSVC specific compiler flag to ignore warnings about unused > > variables, like is being done for other compilers. > > > > Signed-off-by: Andre Muezerie > > This patch series needs rebase now that Intel drivers got reorganized. Thanks for letting me know Stephen. I sent out a rebased v3 of the series.
RE: [EXTERNAL] [dpdk-dev] [PATCH v5 1/2] common/cnxk: support NPC flow on cn20k
> -Original Message- > From: psathe...@marvell.com > Sent: Wednesday, February 5, 2025 11:13 AM > To: Nithin Kumar Dabilpuram ; Kiran Kumar > Kokkilagadda ; Sunil Kumar Kori > ; Satha Koteswara Rao Kottidi > ; Harman Kalra > Cc: dev@dpdk.org; Satheesh Paul Antonysamy > Subject: [EXTERNAL] [dpdk-dev] [PATCH v5 1/2] common/cnxk: support NPC > flow on cn20k > > From: Satheesh Paul ROC changes to support NPC > flow on cn20k. Signed-off-by: Satheesh Paul > Reviewed-by: Kiran Kumar K --- v2: * Fixed > generic platform > > From: Satheesh Paul > > ROC changes to support NPC flow on cn20k. > > Signed-off-by: Satheesh Paul > Reviewed-by: Kiran Kumar K Series applied to dpdk-next-net-mrvl/for-main. Thanks
Re: [PATCH v5 2/4] lib: fix comparison between devices
On 06-02-2025 05:38, Shani Peretz wrote:
DPDK supports multiple formats for specifying buses,
(such as ":08:00.0" and "08:00.0" for PCI).
This flexibility can lead to inconsistencies when using one
format while running testpmd, then attempts to use the other
format in a later command, resulting in a failure.
The issue arises from the find_device function, which compares
the user-provided string directly with the device->name in
the rte_device structure.
If we want to accurately compare these names, we'll need to bring both
sides to the same representation by invoking the parse function
on the user input.
The proposed solution is to utilize the parse function implemented
by each bus. When comparing names, we will call parse on the supplied
string as well as on the device name itself and compare the results.
As part of the change the parse function will now return the size of the
parsed address.
This will allow consistent comparisons between different representations
of same devices.
In addition, fixed vdev test to use the rte_cmp_dev_name function
instead of the custom one.
Fixes: a3ee360f4440 ("eal: add hotplug add/remove device")
Signed-off-by: Shani Peretz
---
app/test/test_vdev.c | 10 ++
drivers/bus/auxiliary/auxiliary_common.c | 17 +++---
drivers/bus/cdx/cdx.c| 13 +---
drivers/bus/dpaa/dpaa_bus.c | 7 ++--
drivers/bus/fslmc/fslmc_bus.c| 9 --
drivers/bus/ifpga/ifpga_bus.c| 14 +---
drivers/bus/pci/pci_common.c | 7 ++--
drivers/bus/platform/platform.c | 15 ++---
drivers/bus/uacce/uacce.c| 14 +---
drivers/bus/vdev/vdev.c | 23 +++--
drivers/bus/vmbus/vmbus_common.c | 9 --
drivers/dma/idxd/idxd_bus.c | 9 --
drivers/raw/ifpga/ifpga_rawdev.c | 8 +
lib/eal/common/eal_common_bus.c | 2 +-
lib/eal/common/eal_common_dev.c | 41 +---
lib/eal/common/hotplug_mp.c | 11 ++-
lib/eal/include/bus_driver.h | 24 +-
lib/eal/include/rte_dev.h| 15 +
lib/eal/linux/eal_dev.c | 10 +-
lib/eal/version.map | 1 +
20 files changed, 180 insertions(+), 79 deletions(-)
diff --git a/app/test/test_vdev.c b/app/test/test_vdev.c
index 3e262f30bc..860fa260af 100644
--- a/app/test/test_vdev.c
+++ b/app/test/test_vdev.c
@@ -20,12 +20,6 @@ static const char * const valid_keys[] = {
NULL,
};
-static int
-cmp_dev_name(const struct rte_device *dev, const void *name)
-{
- return strcmp(rte_dev_name(dev), name);
-}
-
static int
cmp_dev_match(const struct rte_device *dev, const void *_kvlist)
{
@@ -82,7 +76,7 @@ test_vdev_bus(void)
printf("Failed to create vdev net_null_test0\n");
goto fail;
}
- dev0 = vdev_bus->find_device(NULL, cmp_dev_name, "net_null_test0");
+ dev0 = vdev_bus->find_device(NULL, rte_cmp_dev_name, "net_null_test0");
if (dev0 == NULL) {
printf("Cannot find net_null_test0 vdev\n");
goto fail;
@@ -93,7 +87,7 @@ test_vdev_bus(void)
printf("Failed to create vdev net_null_test1\n");
goto fail;
}
- dev1 = vdev_bus->find_device(NULL, cmp_dev_name, "net_null_test1");
+ dev1 = vdev_bus->find_device(NULL, rte_cmp_dev_name, "net_null_test1");
if (dev1 == NULL) {
printf("Cannot find net_null_test1 vdev\n");
goto fail;
diff --git a/drivers/bus/auxiliary/auxiliary_common.c
b/drivers/bus/auxiliary/auxiliary_common.c
index e6cbc4d356..ba2f69e851 100644
--- a/drivers/bus/auxiliary/auxiliary_common.c
+++ b/drivers/bus/auxiliary/auxiliary_common.c
@@ -237,10 +237,9 @@ auxiliary_probe(void)
}
static int
-auxiliary_parse(const char *name, void *addr)
+auxiliary_parse(const char *name, void *addr, int *size)
{
struct rte_auxiliary_driver *drv = NULL;
- const char **out = addr;
/* Allow empty device name "auxiliary:" to bypass entire bus scan. */
if (strlen(name) == 0)
@@ -250,9 +249,17 @@ auxiliary_parse(const char *name, void *addr)
if (drv->match(name))
break;
}
- if (drv != NULL && addr != NULL)
- *out = name;
- return drv != NULL ? 0 : -1;
+
+ if (drv == NULL)
+ return -1;
+
+ if (size != NULL)
+ *size = strlen(name) + 1;
+
+ if (addr != NULL)
+ rte_strscpy(addr, name, strlen(name) + 1);
+
+ return 0;
}
/* Register a driver */
diff --git a/drivers/bus/cdx/cdx.c b/drivers/bus/cdx/cdx.c
index 62b108e082..b97f1ce1af 100644
--- a/drivers/bus/cdx/cdx.c
+++ b/drivers/bus/cdx/cdx.c
@@ -464,15 +464,20 @@ cdx_probe(void)
}
static int
-cdx_parse(const cha
Re: [RFC] ci: Add support for Cirrus-CI service to test FreeBSD.
Patrick Robb writes:
> On Tue, Feb 4, 2025 at 11:07 AM Aaron Conole wrote:
>
> This commit adds preliminary support for developer driven FreeBSD testing
> via the Cirrus-CI cloud continuous integration system.
>
> NOTE: Currently, this does not successfully execute. See the following
> build result:
>https://cirrus-ci.com/task/5626189961756672
>
> Full Logs:
>https://api.cirrus-ci.com/v1/task/5626189961756672/logs/check.log
>
> The tests themselves may need to be run as root.
>
> Signed-off-by: Aaron Conole
> ---
> .ci/freebsd-build.sh | 5 +
> .ci/freebsd-setup.sh | 3 +++
> .cirrus.yml | 33 +
> MAINTAINERS | 1 +
> 4 files changed, 42 insertions(+)
> create mode 100755 .ci/freebsd-build.sh
> create mode 100755 .ci/freebsd-setup.sh
> create mode 100644 .cirrus.yml
>
> diff --git a/.ci/freebsd-build.sh b/.ci/freebsd-build.sh
> new file mode 100755
> index 00..099f9fd448
> --- /dev/null
> +++ b/.ci/freebsd-build.sh
> @@ -0,0 +1,5 @@
> +#!/bin/sh
> +
> +cd build
> +ninja
> +meson install
> diff --git a/.ci/freebsd-setup.sh b/.ci/freebsd-setup.sh
> new file mode 100755
> index 00..762a8383c3
> --- /dev/null
> +++ b/.ci/freebsd-setup.sh
> @@ -0,0 +1,3 @@
> +#!/bin/sh
> +
> +meson setup build
> diff --git a/.cirrus.yml b/.cirrus.yml
> new file mode 100644
> index 00..727dcb14f4
> --- /dev/null
> +++ b/.cirrus.yml
> @@ -0,0 +1,33 @@
> +freebsd_build_task:
> +
> + freebsd_instance:
> +matrix:
> + image_family: freebsd-15-0-snap
> + image_family: freebsd-14-2-snap
> +cpu: 4
> +memory: 4G
> +
> + env:
> +DEPENDENCIES: git gcc wget openssl python3 meson pkgconf
> +PY_DEPS: pyelftools
> +matrix:
> + COMPILER: gcc
> + COMPILER: clang
> +
> + prepare_script:
> +- sysctl -w kern.coredump=0
> +- pkg update -f
> +- pkg install -y ${DEPENDENCIES}
> +$(pkg search -xq "^py3[0-9]+-(${PY_DEPS})-[0-9]+" | xargs)
> +- mkdir -p /usr/src
> +- git clone --depth 1 https://git.freebsd.org/src.git /usr/src
>
> Sorry, do you mind explaining why this is needed? And should it be cloning to
> main or to the latest LTS tag? I think I am ignorant of what is
> required for your build process.
[1933/1938] Generating kernel/freebsd/contigmem with a custom command
FAILED: kernel/freebsd/contigmem.ko
/usr/bin/make -f ../kernel/freebsd/BSDmakefile.meson KMOD_OBJDIR=kernel/freebsd
KMOD_SRC=../kernel/freebsd/contigmem/contigmem.c KMOD=contigmem
'KMOD_CFLAGS=-I/tmp/cirrus-ci-build/build -I/tmp/cirrus-ci-build/config
-include rte_config.h' CC=clang
make: "/usr/share/mk/bsd.sysdir.mk" line 16: Unable to locate the kernel source
tree. Set SYSDIR to override.
in /usr/share/mk/bsd.kmod.mk:4
in ../kernel/freebsd/BSDmakefile.meson:18
without this, no contigmem support.
> +
> + configure_script:
> +- ./.ci/freebsd-setup.sh
> +
> + build_script:
> +- ./.ci/freebsd-build.sh
> +
> + check_script:
> +- meson test -C build --suite fast-tests -t 3
>
> You might be interested in this thread:
> https://bugs.dpdk.org/show_bug.cgi?id=761
>
> See Cody's comment from last April about unit tests being broken on FreeBSD
> 13.0 and 14.0. My recollection is that there is no volunteer to
> resolve these issues currently, which is why we are not targeting unit tests
> for FreeBSD at the Community Lab - only build tests.
Hrrm... that's an interesting issue. We probably do need some eyeballs
on the FreeBSD support side.
Thanks for taking a look.
> +|| { cat ./build/meson-logs/testlog.txt; exit 1; }
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b86cdd266b..ed1df17f3c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -142,6 +142,7 @@ M: Aaron Conole
> M: Michael Santana
> F: .github/workflows/build.yml
> F: .ci/
> +F: .cirrus.yml
>
> Driver information
> M: Dmitry Kozlyuk
> --
> 2.47.1
>
> Looks like it makes sense overall, thanks.
>
> Reviewed-by: Patrick Robb
Re: [PATCH] vhost: fix VDUSE devices registration
Hi Ariel,
On 1/31/25 6:34 PM, Ariel Otilibili-Anieli wrote:
Hello Maxime,
On Friday, January 31, 2025 14:09 CET, Maxime Coquelin
wrote:
This patch fixes a regression in vhost_driver_register()
causing VDUSE devices registration to fail systematically
because the return value was initialized to -1 and not
changed later on for this type of devices.
Fixes: 4d2aa150769b ("vhost: remove check around mutex init")
Thanks for the heads up. I indeed committed 4d2aa150769b ("vhost: remove check
around mutex init"); and it contained a hunk for vhost_driver_register().
I applied this patch against the tip of the main; from what I saw, there is no overlap
with 4d2aa150769b ("vhost: remove check around mutex init").
It looks 4d2aa150769b ("vhost: remove check around mutex init") came up first,
because I was the last person who edited the file.
The tags should be rather these ones:
Fixes: 0adb8eccc6a6 ("vhost: add VDUSE device creation and destruction")
Fixes: 78b2e3bae1af ("vhost: fix initialization")
Fixes: 64ab701c3d1e ("vhost: add vhost-user client mode")
Fixes: 8f972312b8f4 ("vhost: support vhost-user")
Does my reasoning make sense? Let me know.
Not really :)
Without your patch, ret was assigned by the pthread_mutex_init() return,
so always 0. With your patch, this assignment is removed so ret will
always be -1 for VDUSE devices.
So before your patch, VDUSE devices registration was functional, with
your patch it breaks systematically.
We don't want to backport my patch to LTS that aren't imapcted, so
tagging your patch as the one introducing the regression is the right
thing to do.
:) For my understanding; now that 4d2aa150769b ("vhost: remove check around mutex
init") needs a fix, is there a way by which I could have detect the regression?
It could have been detected by testing VDUSE, that's how I noticed it.
But VDUSE is still fairly recent, and it is not yet tested by the CI.
Now that it is supported in at least Fedora without any kernel change,
we should work on adding CI testing for it.
Your help will be much appreciated,
Ariel
Thanks for your contribution,
Maxime
Re: [RFC 0/3] Vhost: fix FD entries cleanup
Hi Chenbo & David,
On 2/5/25 8:27 AM, Chenbo Xia wrote:
Hi David,
On Feb 4, 2025, at 21:18, David Marchand wrote:
External email: Use caution opening links or attachments
Hello vhost maintainers,
On Tue, Dec 24, 2024 at 4:50 PM Maxime Coquelin
wrote:
The vhost FD manager provides a way for the read/write
callbacks to request removal of their associated FD from
the epoll FD set. Problem is that it is missing a cleanup
callback, so the read/write callback requesting the removal
have to perform cleanups before the FD is removed from the
FD set. It includes closing the FD before it is removed
from the epoll FD set.
This series introduces a new cleanup callback which, if
implemented, is closed right after the FD is removed from
FD set.
Maxime Coquelin (3):
vhost: add cleanup callback to FD entries
vhost: fix vhost-user socket cleanup order
vhost: improve VDUSE reconnect handler cleanup
lib/vhost/fd_man.c | 16
lib/vhost/fd_man.h | 3 ++-
lib/vhost/socket.c | 46 ++
lib/vhost/vduse.c | 16 +++-
4 files changed, 51 insertions(+), 30 deletions(-)
I tried this series, and it fixes the error log I reported.
On the other hand, I wonder if we could do something simpler.
The fd is only used by the registered handlers.
If a handler reports that it does not want to watch this fd anymore,
then there is no remaining user in the vhost library for this fd.
So my proposal would be to rename the "remove" flag as a "close" flag:
@@ -12,7 +12,7 @@ struct fdset;
#define MAX_FDS 1024
-typedef void (*fd_cb)(int fd, void *dat, int *remove);
+typedef void (*fd_cb)(int fd, void *dat, int *close);
struct fdset *fdset_init(const char *name);
And defer closing to fd_man.
Something like:
@@ -367,9 +367,9 @@ fdset_event_dispatch(void *arg)
pthread_mutex_unlock(&pfdset->fd_mutex);
if (rcb && events[i].events & (EPOLLIN |
EPOLLERR | EPOLLHUP))
- rcb(fd, dat, &remove1);
+ rcb(fd, dat, &close1);
if (wcb && events[i].events & (EPOLLOUT |
EPOLLERR | EPOLLHUP))
- wcb(fd, dat, &remove2);
+ wcb(fd, dat, &close2);
pfdentry->busy = 0;
/*
* fdset_del needs to check busy flag.
@@ -381,8 +381,10 @@ fdset_event_dispatch(void *arg)
* fdentry not to be busy, so we can't call
* fdset_del_locked().
*/
- if (remove1 || remove2)
+ if (close1 || close2) {
fdset_del(pfdset, fd);
+ close(fd);
+ }
}
if (pfdset->destroy)
And the only thing to move out of the socket and vduse handlers is the
close(fd) call.
Like:
@@ -303,7 +303,7 @@ vhost_user_server_new_connection(int fd, void
*dat, int *remove __rte_unused)
}
static void
-vhost_user_read_cb(int connfd, void *dat, int *remove)
+vhost_user_read_cb(int connfd, void *dat, int *close)
{
struct vhost_user_connection *conn = dat;
struct vhost_user_socket *vsocket = conn->vsocket;
@@ -313,8 +313,7 @@ vhost_user_read_cb(int connfd, void *dat, int *remove)
if (ret < 0) {
struct virtio_net *dev = get_device(conn->vid);
- close(connfd);
- *remove = 1;
+ *close = 1;
I have one concern here is compared with this RFC, the proposal changed the
timing
of close connfd，which means on QEMU side, cleaning up resources will happen
later.
Currently I can’t think of issues could be introduced by this change (maybe you
and
Maxime could remind me of something :)
That's a good point.
I just tested David's suggestion with Vhost-user with OVS and QEMU:
- guest shutdown + reconnect
- live-migration
- OVS restart
It seems to behave very well.
Besides this, definitely this proposal is cleaner.
I agree, I will send a new revision re-using David's proposal.
Thanks,
Maxime
Thanks,
Chenbo
if (dev)
vhost_destroy_device_notify(dev);
Maxime, Chenbo, opinions?
--
David Marchand
Re: [PATCH v7 04/15] net/xsc: add xsc dev ops to support VFIO driver
05/02/2025 15:59, Bruce Richardson: > On Wed, Feb 05, 2025 at 03:43:30PM +0100, Thomas Monjalon wrote: > > 05/02/2025 15:37, Renyong Wan: > > > On 2025/2/5 19:44, Thomas Monjalon wrote: > > > > 28/01/2025 15:46, Renyong Wan: > > > >> XSC PMD is designed to support both VFIO and private kernel drivers. > > > > What's the benefit of private kernel drivers? Why are they private? > > > > > > Hello Thomas, > > > > > > Thanks for your review. > > > > > > It can support the bifurcation model without unbinding the kernel > > > driver, by utilizing our private kernel driver in conjunction with > > > rdma-core. Currently, our kernel driver is not open-source, so it is > > > considered a private kernel driver. This patch series only supports the > > > VFIO driver. Our kernel driver is currently in the process of being > > > open-sourced on kernel.org, and once it is available there, we also > > > plan to submit the code that supports our kernel driver to DPDK. > > > > OK that's interesting, thank you. > > > > I think it would be the first DPDK driver to support both VFIO or > > bifurcated model. > > > > Not quite the first, but possibly the first net driver? :-). The idxd > dmadev driver supports both. It can be used either with VFIO or the kernel > idxd driver. It announces only VFIO: RTE_PMD_REGISTER_KMOD_DEP(IDXD_PMD_DMADEV_NAME_PCI, "vfio-pci"); How does it work?
[PATCH v3 09/19] examples/ptpclient: fix self memcmp
Calling memcmp on same structure will always be true.
Replace with same conditional used elsewhere.
Link: https://pvs-studio.com/en/blog/posts/cpp/1183/
Fixes: ab129e9065a5 ("examples/ptpclient: add minimal PTP client")
Cc: danielx.t.mrzyg...@intel.com
Cc: sta...@dpdk.org
Signed-off-by: Stephen Hemminger
---
examples/ptpclient/ptpclient.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/examples/ptpclient/ptpclient.c b/examples/ptpclient/ptpclient.c
index 27d06dd91d..c344e7db1e 100644
--- a/examples/ptpclient/ptpclient.c
+++ b/examples/ptpclient/ptpclient.c
@@ -367,7 +367,7 @@ parse_sync(struct ptpv2_time_receiver_ordinary *ptp_data,
uint16_t rx_tstamp_idx
ptp_data->ptpset = 1;
}
- if (memcmp(&ptp_hdr->source_port_id.clock_id,
+ if (memcmp(&ptp_data->transmitter_clock_id,
&ptp_hdr->source_port_id.clock_id,
sizeof(struct clock_id)) == 0) {
--
2.47.2
[PATCH v3 19/19] common/cnxk: fix null ptr check
The pointer mode is used then checked which is a bug reported
by PVS studio.
Fixes: bd2fd34ab86f ("common/cnxk: sync eth mode change command with firmware")
Cc: tduszyn...@marvell.com
Cc: sta...@dpdk.org
Signed-off-by: Stephen Hemminger
---
drivers/common/cnxk/roc_bphy_cgx.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/common/cnxk/roc_bphy_cgx.c
b/drivers/common/cnxk/roc_bphy_cgx.c
index 882cf65474..db70bafd9b 100644
--- a/drivers/common/cnxk/roc_bphy_cgx.c
+++ b/drivers/common/cnxk/roc_bphy_cgx.c
@@ -366,20 +366,20 @@ roc_bphy_cgx_set_link_mode(struct roc_bphy_cgx *roc_cgx,
unsigned int lmac,
{
uint64_t scr1, scr0;
+ if (!mode)
+ return -EINVAL;
+
+ if (!roc_cgx)
+ return -EINVAL;
+
if (roc_model_is_cn9k() &&
(mode->use_portm_idx || mode->portm_idx || mode->mode_group_idx)) {
return -ENOTSUP;
}
- if (!roc_cgx)
- return -EINVAL;
-
if (!roc_bphy_cgx_lmac_exists(roc_cgx, lmac))
return -ENODEV;
- if (!mode)
- return -EINVAL;
-
scr1 = FIELD_PREP(SCR1_ETH_CMD_ID, ETH_CMD_MODE_CHANGE) |
FIELD_PREP(SCR1_ETH_MODE_CHANGE_ARGS_SPEED, mode->speed) |
FIELD_PREP(SCR1_ETH_MODE_CHANGE_ARGS_DUPLEX, mode->full_duplex) |
--
2.47.2
[PATCH v3 14/19] event/dpaa: fix bitmask truncation
More bitmask truncation from mask computation.
Fixes: 0ee17f79ebd0 ("event/dpaa: add enqueue/dequeue")
Cc: sunil.k...@nxp.com
Cc: sta...@dpdk.org
Signed-off-by: Stephen Hemminger
Acked-by: Hemant Agrawal
---
drivers/event/dpaa/dpaa_eventdev.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/event/dpaa/dpaa_eventdev.c
b/drivers/event/dpaa/dpaa_eventdev.c
index 853cc1ecf9..400e0ecd1c 100644
--- a/drivers/event/dpaa/dpaa_eventdev.c
+++ b/drivers/event/dpaa/dpaa_eventdev.c
@@ -102,7 +102,7 @@ dpaa_event_enqueue_burst(void *port, const struct rte_event
ev[],
qman_dca_index(ev[i].impl_opaque, 0);
mbuf = DPAA_PER_LCORE_DQRR_MBUF(i);
*dpaa_seqn(mbuf) = DPAA_INVALID_MBUF_SEQN;
- DPAA_PER_LCORE_DQRR_HELD &= ~(1 << i);
+ DPAA_PER_LCORE_DQRR_HELD &= ~(UINT64_C(1) << i);
DPAA_PER_LCORE_DQRR_SIZE--;
break;
default:
@@ -199,11 +199,11 @@ dpaa_event_dequeue_burst(void *port, struct rte_event
ev[],
/* Check if there are atomic contexts to be released */
i = 0;
while (DPAA_PER_LCORE_DQRR_SIZE) {
- if (DPAA_PER_LCORE_DQRR_HELD & (1 << i)) {
+ if (DPAA_PER_LCORE_DQRR_HELD & (UINT64_C(1) << i)) {
qman_dca_index(i, 0);
mbuf = DPAA_PER_LCORE_DQRR_MBUF(i);
*dpaa_seqn(mbuf) = DPAA_INVALID_MBUF_SEQN;
- DPAA_PER_LCORE_DQRR_HELD &= ~(1 << i);
+ DPAA_PER_LCORE_DQRR_HELD &= ~(UINT64_C(1) << i);
DPAA_PER_LCORE_DQRR_SIZE--;
}
i++;
@@ -263,11 +263,11 @@ dpaa_event_dequeue_burst_intr(void *port, struct
rte_event ev[],
/* Check if there are atomic contexts to be released */
i = 0;
while (DPAA_PER_LCORE_DQRR_SIZE) {
- if (DPAA_PER_LCORE_DQRR_HELD & (1 << i)) {
+ if (DPAA_PER_LCORE_DQRR_HELD & (UINT64_C(1) << i)) {
qman_dca_index(i, 0);
mbuf = DPAA_PER_LCORE_DQRR_MBUF(i);
*dpaa_seqn(mbuf) = DPAA_INVALID_MBUF_SEQN;
- DPAA_PER_LCORE_DQRR_HELD &= ~(1 << i);
+ DPAA_PER_LCORE_DQRR_HELD &= ~(UINT64_C(1) << i);
DPAA_PER_LCORE_DQRR_SIZE--;
}
i++;
--
2.47.2
[PATCH v3 18/19] examples/l3fwd: fix operator precedence bugs
The expression:
if ((socketid = rte_lcore_to_socket_id(lcore) != 0) &&
gets evaluated as sockeid = (rte_lcore_to_socket_id(lcore) != 0)
which is not what was intended. This is goes all the way back
to first release.
Link: https://pvs-studio.com/en/blog/posts/cpp/1183/
Fixes: af75078fece3 ("first public release")
Cc: sta...@dpdk.org
Signed-off-by: Stephen Hemminger
---
examples/l3fwd-power/main.c | 4 ++--
examples/l3fwd/main.c | 5 +++--
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index d279e664b3..e27b8531b5 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -1412,8 +1412,8 @@ check_lcore_params(void)
"mask\n", lcore);
return -1;
}
- if ((socketid = rte_lcore_to_socket_id(lcore) != 0) &&
- (numa_on == 0)) {
+ socketid = rte_lcore_to_socket_id(lcore);
+ if (socketid != 0 && numa_on == 0) {
printf("warning: lcore %u is on socket %d with numa "
"off\n", lcore, socketid);
}
diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
index 994b7dd8e5..ae3b4f6439 100644
--- a/examples/l3fwd/main.c
+++ b/examples/l3fwd/main.c
@@ -311,8 +311,9 @@ check_lcore_params(void)
printf("error: lcore %u is not enabled in lcore
mask\n", lcore);
return -1;
}
- if ((socketid = rte_lcore_to_socket_id(lcore) != 0) &&
- (numa_on == 0)) {
+
+ socketid = rte_lcore_to_socket_id(lcore);
+ if (socketid != 0 && numa_on == 0) {
printf("warning: lcore %u is on socket %d with numa
off\n",
lcore, socketid);
}
--
2.47.2
[PATCH v3 00/19] minor fixes from PVS studio bug list
More bug fixes from PVS studio bug reports. And one other fix to ptpclient. Stephen Hemminger (19): common/cnxk: remove duplicate condition net/cpfl: avoid calling log (printf) with null raw/cnxk_gpio: fix file descriptor leak net/ntnic: remove dead code net/i40e: remove duplicate code eal: fix out of bounds access in devargs net/qede: fix missing debug string examples/ptpclient: replace rte_memcpy with assignment examples/ptpclient: fix self memcmp net/octeon_ep: remove duplicate code net/hinic: fix flow type bitmask overflow crypto/dpaa2_sec: fix bitmask truncation crypto/dpaa_sec: fix bitmask truncation event/dpaa: fix bitmask truncation net/dpaa: fix bitmask truncation net/dpaa2: fix bitmask truncation net/qede: don't use same loop variable twice examples/l3fwd: fix operator precedence bugs common/cnxk: fix null ptr check v3 - rebase; Intel drivers moved Stephen Hemminger (19): common/cnxk: remove duplicate condition net/cpfl: avoid calling log (printf) with null raw/cnxk_gpio: fix file descriptor leak net/ntnic: remove dead code net/i40e: remove duplicate code eal: fix out of bounds access in devargs net/qede: fix missing debug string examples/ptpclient: replace rte_memcpy with assignment examples/ptpclient: fix self memcmp net/octeon_ep: remove duplicate code net/hinic: fix flow type bitmask overflow crypto/dpaa2_sec: fix bitmask truncation crypto/dpaa_sec: fix bitmask truncation event/dpaa: fix bitmask truncation net/dpaa: fix bitmask truncation net/dpaa2: fix bitmask truncation net/qede: don't use same loop variable twice examples/l3fwd: fix operator precedence bugs common/cnxk: fix null ptr check drivers/common/cnxk/cnxk_security.c | 16 -- drivers/common/cnxk/roc_bphy_cgx.c | 12 +-- drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 8 +++ drivers/crypto/dpaa_sec/dpaa_sec.c | 7 +++--- drivers/event/dpaa/dpaa_eventdev.c | 10 - drivers/net/dpaa/dpaa_rxtx.c| 7 +++--- drivers/net/dpaa2/dpaa2_rxtx.c | 6 +++--- drivers/net/hinic/hinic_pmd_flow.c | 14 ++-- drivers/net/intel/cpfl/cpfl_ethdev.c| 2 +- drivers/net/intel/i40e/i40e_fdir.c | 10 - drivers/net/ntnic/ntnic_ethdev.c| 8 --- drivers/net/octeon_ep/otx_ep_ethdev.c | 9 ++-- drivers/net/qede/base/ecore_dcbx.c | 8 +++ drivers/net/qede/qede_debug.c | 5 + drivers/raw/cnxk_gpio/cnxk_gpio_selftest.c | 24 + examples/l3fwd-power/main.c | 4 ++-- examples/l3fwd/main.c | 5 +++-- examples/ptpclient/ptpclient.c | 10 +++-- lib/eal/common/eal_common_devargs.c | 2 +- 19 files changed, 81 insertions(+), 86 deletions(-) -- 2.47.2
[PATCH v3 17/19] net/qede: don't use same loop variable twice
Using variable in outer loop, and inner loop is obvious bug.
This bug is in base code, so likely on other platforms as well.
Link: https://pvs-studio.com/en/blog/posts/cpp/1183/
Fixes: 81dba2b2ff61 ("net/qede/base: add LLDP support")
Cc: rasesh.m...@cavium.com
Cc: sta...@dpdk.org
Signed-off-by: Stephen Hemminger
---
drivers/net/qede/base/ecore_dcbx.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/qede/base/ecore_dcbx.c
b/drivers/net/qede/base/ecore_dcbx.c
index 31234f18cf..72bbedd65a 100644
--- a/drivers/net/qede/base/ecore_dcbx.c
+++ b/drivers/net/qede/base/ecore_dcbx.c
@@ -1363,7 +1363,7 @@ ecore_lldp_mib_update_event(struct ecore_hwfn *p_hwfn,
struct ecore_ptt *p_ptt)
struct ecore_dcbx_mib_meta_data data;
enum _ecore_status_t rc = ECORE_SUCCESS;
struct lldp_received_tlvs_s tlvs;
- int i;
+ int i, j;
for (i = 0; i < LLDP_MAX_LLDP_AGENTS; i++) {
OSAL_MEM_ZERO(&data, sizeof(data));
@@ -1381,9 +1381,9 @@ ecore_lldp_mib_update_event(struct ecore_hwfn *p_hwfn,
struct ecore_ptt *p_ptt)
if (!tlvs.length)
continue;
- for (i = 0; i < MAX_TLV_BUFFER; i++)
- tlvs.tlvs_buffer[i] =
- OSAL_CPU_TO_BE32(tlvs.tlvs_buffer[i]);
+ for (j = 0; j < MAX_TLV_BUFFER; j++)
+ tlvs.tlvs_buffer[j] =
+ OSAL_CPU_TO_BE32(tlvs.tlvs_buffer[j]);
OSAL_LLDP_RX_TLVS(p_hwfn, tlvs.tlvs_buffer, tlvs.length);
}
--
2.47.2
[PATCH v3 16/19] net/dpaa2: fix bitmask truncation
The dqrr_held mask is 64 bit but updates were getting truncated
because 1 is of type int (32 bit) and the result shift of int is of
type int (32 bit); therefore any value >= 32 would get truncated.
Link: https://pvs-studio.com/en/blog/posts/cpp/1183/
Fixes: 2d3788631862 ("net/dpaa2: support atomic queues")
Cc: nipun.gu...@nxp.com
Cc: sta...@dpdk.org
Signed-off-by: Stephen Hemminger
Acked-by: Hemant Agrawal
---
drivers/net/dpaa2/dpaa2_rxtx.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/dpaa2/dpaa2_rxtx.c b/drivers/net/dpaa2/dpaa2_rxtx.c
index bfb5542bbc..cad15d8f75 100644
--- a/drivers/net/dpaa2/dpaa2_rxtx.c
+++ b/drivers/net/dpaa2/dpaa2_rxtx.c
@@ -933,7 +933,7 @@ dpaa2_dev_process_atomic_event(struct qbman_swp *swp
__rte_unused,
dqrr_index = qbman_get_dqrr_idx(dq);
*dpaa2_seqn(ev->mbuf) = dqrr_index + 1;
DPAA2_PER_LCORE_DQRR_SIZE++;
- DPAA2_PER_LCORE_DQRR_HELD |= 1 << dqrr_index;
+ DPAA2_PER_LCORE_DQRR_HELD |= UINT64_C(1) << dqrr_index;
DPAA2_PER_LCORE_DQRR_MBUF(dqrr_index) = ev->mbuf;
}
@@ -1317,7 +1317,7 @@ dpaa2_dev_tx(void *queue, struct rte_mbuf **bufs,
uint16_t nb_pkts)
flags[loop] = QBMAN_ENQUEUE_FLAG_DCA |
dqrr_index;
DPAA2_PER_LCORE_DQRR_SIZE--;
- DPAA2_PER_LCORE_DQRR_HELD &= ~(1 << dqrr_index);
+ DPAA2_PER_LCORE_DQRR_HELD &= ~(UINT64_C(1) <<
dqrr_index);
*dpaa2_seqn(*bufs) = DPAA2_INVALID_MBUF_SEQN;
}
@@ -1575,7 +1575,7 @@ dpaa2_set_enqueue_descriptor(struct dpaa2_queue *dpaa2_q,
dq_idx = *dpaa2_seqn(m) - 1;
qbman_eq_desc_set_dca(eqdesc, 1, dq_idx, 0);
DPAA2_PER_LCORE_DQRR_SIZE--;
- DPAA2_PER_LCORE_DQRR_HELD &= ~(1 << dq_idx);
+ DPAA2_PER_LCORE_DQRR_HELD &= ~(UINT64_C(1) << dq_idx);
}
*dpaa2_seqn(m) = DPAA2_INVALID_MBUF_SEQN;
}
--
2.47.2
Re: [PATCH v4] bus: fix inconsistent representation of PCI numbers
On Wed, 5 Feb 2025 17:37:54 +
Shani Peretz wrote:
> > -Original Message-
> > From: Stephen Hemminger
> > Sent: Wednesday, 5 February 2025 18:42
> > To: Shani Peretz
> > Cc: dev@dpdk.org; NBU-Contact-Thomas Monjalon (EXTERNAL)
> > ; Tyler Retzlaff ; Parav
> > Pandit ; Xueming Li ; Nipun Gupta
> > ; Nikhil Agarwal ; Hemant
> > Agrawal ; Sachin Saxena
> > ; Rosen Xu ; Chenbo Xia
> > ; Tomasz Duszynski ;
> > Chengwen Feng ; NBU-Contact-longli
> > (EXTERNAL) ; Wei Hu ; Bruce
> > Richardson ; Kevin Laatz
> > ; Jan Blunck
> > Subject: Re: [PATCH v4] bus: fix inconsistent representation of PCI numbers
> >
> > External email: Use caution opening links or attachments
> >
> >
> > On Wed, 5 Feb 2025 16:36:11 +
> > Shani Peretz wrote:
> >
> > > > -Original Message-
> > > > From: Stephen Hemminger
> > > > Sent: Wednesday, 29 January 2025 18:25
> > > > To: Shani Peretz
> > > > Cc: dev@dpdk.org; NBU-Contact-Thomas Monjalon (EXTERNAL)
> > > > ; Tyler Retzlaff
> > > > ; Parav Pandit ;
> > > > Xueming Li ; Nipun Gupta
> > ;
> > > > Nikhil Agarwal ; Hemant Agrawal
> > > > ; Sachin Saxena ;
> > > > Rosen Xu ; Chenbo Xia ;
> > > > Tomasz Duszynski ; Chengwen Feng
> > > > ; NBU-Contact-longli
> > > > (EXTERNAL) ; Wei Hu ; Bruce
> > > > Richardson ; Kevin Laatz
> > > > ; Jan Blunck
> > > > Subject: Re: [PATCH v4] bus: fix inconsistent representation of PCI
> > > > numbers
> > > >
> > > > External email: Use caution opening links or attachments
> > > >
> > > >
> > > > On Wed, 29 Jan 2025 10:54:16 +0200
> > > > Shani Peretz wrote:
> > > >
> > > > > +create_pci_dev(const char *name)
> > > > > +{
> > > > > + int port_id;
> > > > > + uint8_t slave_mac1[] = {0x00, 0xFF, 0x00, 0xFF, 0x00, 0x00 };
> > > > > + struct rte_ether_addr *mac_addr = (struct rte_ether_addr
> > > > > +*)slave_mac1;
> > > >
> > > > Use different initializer and you can avoid the need for cast here.
> > > >
> > > >
> > > > >
> > > > > +/**
> > > > > + * General device name comparison. Will compare by using the
> > > > > +specific bus
> > > > > + * compare function or by comparing the names directly.
> > > > > + *
> > > > > + * @param dev
> > > > > + * Device handle.
> > > > > + * @param name
> > > > > + * Name to compare against.
> > > > > + * @return
> > > > > + * 0 if the device matches the name. Nonzero otherwise.
> > > > > + */
> > > > > +__rte_internal
> > > > > +int rte_cmp_dev_name(const struct rte_device *dev, const void
> > > > > +*name);
> > > >
> > > > It would make more sense to me if name was a character not void pointer.
> > > >
> > > > The design might be clearer if bus address was more of an typedef
> > > > with a pointer and size together. Treat it more like an object.
> > >
> > >
> > > Okay so just to understand,
> > > this is the struct I am adding:
> > >
> > > struct rte_bus_address {
> > > const void *addr;
> > > size_t size;
> > > };
> > >
> > > This is how I pass it to find_device:
> > >
> > > struct rte_bus_address dev_addr = {
> > > .addr = da->name,
> > > .size = RTE_DEV_NAME_MAX_LEN
> > > };
> > > dev = da->bus->find_device(NULL, rte_cmp_dev_name, &dev_addr);
> > >
> > > And this is how I use it in rte_cmp_dev_name:
> > >
> > > rte_cmp_dev_name(const struct rte_device *dev1, const void *addr)
> > >{
> > > const struct rte_bus_address *dev2_addr = addr;
> > > …
> > > }
> > >
> > > Is that what you meant?
> > > Also, I'm not sure if the size is really needed, because we check the
> > > size of the parsed name, which may be different than the size of the
> > > original name
> > >
> >
> > It would be best to pass rte_bus_address to rte_cmp_dev_name rather than
> > having implied cast.
> > Not sure if that is possible without breaking API though.
>
> I think you are right, also it will require to change the signature of
> find_device which will make it even bigger change
Could you post patch with something "good enough" that builds and passes tests?
Re: [PATCH v7 04/15] net/xsc: add xsc dev ops to support VFIO driver
On Wed, Feb 05, 2025 at 03:43:30PM +0100, Thomas Monjalon wrote: > 05/02/2025 15:37, Renyong Wan: > > On 2025/2/5 19:44, Thomas Monjalon wrote: > > > 28/01/2025 15:46, Renyong Wan: > > >> XSC PMD is designed to support both VFIO and private kernel drivers. > > > What's the benefit of private kernel drivers? Why are they private? > > > > Hello Thomas, > > > > Thanks for your review. > > > > It can support the bifurcation model without unbinding the kernel > > driver, by utilizing our private kernel driver in conjunction with > > rdma-core. Currently, our kernel driver is not open-source, so it is > > considered a private kernel driver. This patch series only supports the > > VFIO driver. Our kernel driver is currently in the process of being > > open-sourced on kernel.org, and once it is available there, we also > > plan to submit the code that supports our kernel driver to DPDK. > > OK that's interesting, thank you. > > I think it would be the first DPDK driver to support both VFIO or > bifurcated model. > Not quite the first, but possibly the first net driver? :-). The idxd dmadev driver supports both. It can be used either with VFIO or the kernel idxd driver. /Bruce
Re: [PATCH v7 04/15] net/xsc: add xsc dev ops to support VFIO driver
On 2025/2/5 19:44, Thomas Monjalon wrote: > 28/01/2025 15:46, Renyong Wan: >> XSC PMD is designed to support both VFIO and private kernel drivers. > What's the benefit of private kernel drivers? > Why are they private? > > > Hello Thomas, Thanks for your review. It can support the bifurcation model without unbinding the kernel driver, by utilizing our private kernel driver in conjunction with rdma-core. Currently, our kernel driver is not open-source, so it is considered a private kernel driver. This patch series only supports the VFIO driver. Our kernel driver is currently in the process of being open-sourced on kernel.org, and once it is available there, we also plan to submit the code that supports our kernel driver to DPDK. -- Best regards, Renyong Wan
Re: [PATCH v7 04/15] net/xsc: add xsc dev ops to support VFIO driver
05/02/2025 15:37, Renyong Wan: > On 2025/2/5 19:44, Thomas Monjalon wrote: > > 28/01/2025 15:46, Renyong Wan: > >> XSC PMD is designed to support both VFIO and private kernel drivers. > > What's the benefit of private kernel drivers? > > Why are they private? > > Hello Thomas, > > Thanks for your review. > > It can support the bifurcation model without unbinding the kernel > driver, by utilizing our private kernel driver in conjunction with > rdma-core. Currently, our kernel driver is not open-source, so it is > considered a private kernel driver. This patch series only supports the > VFIO driver. Our kernel driver is currently in the process of being > open-sourced on kernel.org, and once it is available there, we also plan > to submit the code that supports our kernel driver to DPDK. OK that's interesting, thank you. I think it would be the first DPDK driver to support both VFIO or bifurcated model. How will we choose which one to use? With devargs?
