Re: [EXT] Re: [PATCH v5 0/1] devtools: add tracepoint check in checkpatch
30/11/2023 06:56, Ankur Dwivedi: > From: Thomas Monjalon > > 28/11/2023 15:07, Ankur Dwivedi: > >> > 07/03/2023 13:05, Ankur Dwivedi: > >> >> This patch series adds a validation in checkpatch tool to check if > >> >> tracepoint is present in any new function added in ethdev, eventdev > >> >> cryptodev and mempool library. > >> >> > >> >> v5: > >> >> - Copied the build_map_changes function from check-symbol-change.sh > >to > >> >>check-tracepoint.sh. > >> >> - Added eventdev, cryptodev and mempool in libdir in check- > >tracepoint.sh. > >> > > >> >Why did you decide to copy the function in v5, instead of having a > >> >common file usable by different scripts? > >> > > >> There was comments in v2 of the patch that common scripts may not work > >well and to keep the scripts specialized. > > > >I meant you can have a common file specialized in symbols. > The build_map_changes() (in devtools/check-symbol-change.sh) which is a > common function can be moved to a new file named devtools/build-symbol-map.sh. > The build-symbol-map.sh can be included in check-symbol-change.sh and > check-tracepoint.sh. > Please let me know if this is fine. Yes We can imagine moving more symbol map related funtions in this new file. What about symbol-map-util.sh as filename?
[PATCH 0/7] fix resource leak problems
This patch series fix some resource leak problems in NFP PMD. Chaoyong He (7): net/nfp: fix resource leak for device initialization net/nfp: fix resource leak for CoreNIC firmware net/nfp: fix resource leak for PF initialization net/nfp: fix resource leak for flower firmware net/nfp: fix resource leak for exit of CoreNIC firmware net/nfp: fix resource leak for exit of flower firmware net/nfp: fix resource leak for VF drivers/net/nfp/flower/nfp_flower.c | 73 ++--- drivers/net/nfp/flower/nfp_flower.h | 1 + .../net/nfp/flower/nfp_flower_representor.c | 153 +- .../net/nfp/flower/nfp_flower_representor.h | 1 + drivers/net/nfp/nfp_ethdev.c | 138 drivers/net/nfp/nfp_ethdev_vf.c | 10 +- drivers/net/nfp/nfp_net_common.h | 1 + 7 files changed, 279 insertions(+), 98 deletions(-) -- 2.39.1
[PATCH 1/7] net/nfp: fix resource leak for device initialization
Fix the resource leak problem in the abnormal logic of device
initialize function.
Fixes: f26e82397f6d ("net/nfp: implement xstats")
Fixes: 547137405be7 ("net/nfp: initialize IPsec related content")
Cc: james.hers...@corigine.com
Cc: chang.m...@corigine.com
Cc: sta...@dpdk.org
Signed-off-by: Chaoyong He
Reviewed-by: Long Wu
Reviewed-by: Peng Zhang
---
drivers/net/nfp/nfp_ethdev.c | 24 +---
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index f02caf8056..25feb8e394 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -590,9 +590,6 @@ nfp_net_init(struct rte_eth_dev *eth_dev)
net_hw->mac_stats = net_hw->mac_stats_bar;
} else {
- if (pf_dev->ctrl_bar == NULL)
- return -ENODEV;
-
/* Use port offset in pf ctrl_bar for this ports control bar */
hw->ctrl_bar = pf_dev->ctrl_bar + (port * NFP_NET_CFG_BAR_SZ);
net_hw->mac_stats = app_fw_nic->ports[0]->mac_stats_bar +
@@ -604,18 +601,19 @@ nfp_net_init(struct rte_eth_dev *eth_dev)
err = nfp_net_common_init(pci_dev, net_hw);
if (err != 0)
- return err;
+ goto free_area;
err = nfp_net_tlv_caps_parse(eth_dev);
if (err != 0) {
PMD_INIT_LOG(ERR, "Failed to parser TLV caps");
return err;
+ goto free_area;
}
err = nfp_ipsec_init(eth_dev);
if (err != 0) {
PMD_INIT_LOG(ERR, "Failed to init IPsec module");
- return err;
+ goto free_area;
}
nfp_net_ethdev_ops_mount(net_hw, eth_dev);
@@ -625,7 +623,8 @@ nfp_net_init(struct rte_eth_dev *eth_dev)
if (net_hw->eth_xstats_base == NULL) {
PMD_INIT_LOG(ERR, "no memory for xstats base values on device
%s!",
pci_dev->device.name);
- return -ENOMEM;
+ err = -ENOMEM;
+ goto ipsec_exit;
}
/* Work out where in the BAR the queues start. */
@@ -655,7 +654,8 @@ nfp_net_init(struct rte_eth_dev *eth_dev)
eth_dev->data->mac_addrs = rte_zmalloc("mac_addr", RTE_ETHER_ADDR_LEN,
0);
if (eth_dev->data->mac_addrs == NULL) {
PMD_INIT_LOG(ERR, "Failed to space for MAC address");
- return -ENOMEM;
+ err = -ENOMEM;
+ goto xstats_free;
}
nfp_net_pf_read_mac(app_fw_nic, port);
@@ -693,6 +693,16 @@ nfp_net_init(struct rte_eth_dev *eth_dev)
nfp_net_stats_reset(eth_dev);
return 0;
+
+xstats_free:
+ rte_free(net_hw->eth_xstats_base);
+ipsec_exit:
+ nfp_ipsec_uninit(eth_dev);
+free_area:
+ if (net_hw->mac_stats_area != NULL)
+ nfp_cpp_area_release_free(net_hw->mac_stats_area);
+
+ return err;
}
#define DEFAULT_FW_PATH "/lib/firmware/netronome"
--
2.39.1
[PATCH 2/7] net/nfp: fix resource leak for CoreNIC firmware
Fix the resource leak problem in the logic of CoreNIC firmware
application.
Fixes: 646ea79ce481 ("net/nfp: move PF functions into its own file")
Cc: sta...@dpdk.org
Signed-off-by: Chaoyong He
Reviewed-by: Long Wu
Reviewed-by: Peng Zhang
---
drivers/net/nfp/nfp_ethdev.c | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index 25feb8e394..38ee1c399a 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -310,6 +310,18 @@ nfp_net_keepalive_stop(struct nfp_multi_pf *multi_pf)
rte_eal_alarm_cancel(nfp_net_beat_timer, (void *)multi_pf);
}
+static void
+nfp_net_uninit(struct rte_eth_dev *eth_dev)
+{
+ struct nfp_net_hw *net_hw;
+
+ net_hw = eth_dev->data->dev_private;
+ rte_free(net_hw->eth_xstats_base);
+ nfp_ipsec_uninit(eth_dev);
+ if (net_hw->mac_stats_area != NULL)
+ nfp_cpp_area_release_free(net_hw->mac_stats_area);
+}
+
/* Reset and stop device. The device can not be restarted. */
static int
nfp_net_close(struct rte_eth_dev *dev)
@@ -1130,12 +1142,11 @@ nfp_init_app_fw_nic(struct nfp_pf_dev *pf_dev,
app_fw_nic->ports[id]->eth_dev != NULL) {
struct rte_eth_dev *tmp_dev;
tmp_dev = app_fw_nic->ports[id]->eth_dev;
- nfp_ipsec_uninit(tmp_dev);
+ nfp_net_uninit(tmp_dev);
rte_eth_dev_release_port(tmp_dev);
- app_fw_nic->ports[id] = NULL;
}
}
- nfp_cpp_area_free(pf_dev->ctrl_area);
+ nfp_cpp_area_release_free(pf_dev->ctrl_area);
app_cleanup:
rte_free(app_fw_nic);
--
2.39.1
[PATCH 3/7] net/nfp: fix resource leak for PF initialization
Fix the resource leak problem in the abnormal logic of PF initialize
function.
Fixes: 646ea79ce481 ("net/nfp: move PF functions into its own file")
Fixes: 8ba461d1eecc ("net/nfp: introduce keepalive mechanism for multiple PF")
Cc: peng.zh...@corigine.com
Cc: sta...@dpdk.org
Signed-off-by: Chaoyong He
Reviewed-by: Long Wu
Reviewed-by: Peng Zhang
---
drivers/net/nfp/nfp_ethdev.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index 38ee1c399a..bb0ddf3d54 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -1326,12 +1326,13 @@ nfp_pf_init(struct rte_pci_device *pci_dev)
return 0;
hwqueues_cleanup:
- nfp_cpp_area_free(pf_dev->qc_area);
+ nfp_cpp_area_release_free(pf_dev->qc_area);
sym_tbl_cleanup:
free(sym_tbl);
fw_cleanup:
nfp_fw_unload(cpp);
nfp_net_keepalive_stop(&pf_dev->multi_pf);
+ nfp_net_keepalive_uninit(&pf_dev->multi_pf);
eth_table_cleanup:
free(nfp_eth_table);
hwinfo_cleanup:
--
2.39.1
[PATCH 4/7] net/nfp: fix resource leak for flower firmware
Fix the resource leak problem in the logic of flower firmware
application.
Fixes: e1124c4f8a45 ("net/nfp: add flower representor framework")
Cc: sta...@dpdk.org
Signed-off-by: Chaoyong He
Reviewed-by: Long Wu
Reviewed-by: Peng Zhang
---
.../net/nfp/flower/nfp_flower_representor.c | 89 ++-
.../net/nfp/flower/nfp_flower_representor.h | 1 +
2 files changed, 86 insertions(+), 4 deletions(-)
diff --git a/drivers/net/nfp/flower/nfp_flower_representor.c
b/drivers/net/nfp/flower/nfp_flower_representor.c
index 0f0e63aae0..7212d9e024 100644
--- a/drivers/net/nfp/flower/nfp_flower_representor.c
+++ b/drivers/net/nfp/flower/nfp_flower_representor.c
@@ -291,6 +291,43 @@ nfp_flower_repr_tx_burst(void *tx_queue,
return sent;
}
+static int
+nfp_flower_repr_uninit(struct rte_eth_dev *eth_dev)
+{
+ struct nfp_flower_representor *repr;
+
+ repr = eth_dev->data->dev_private;
+ rte_ring_free(repr->ring);
+
+ return 0;
+}
+
+static int
+nfp_flower_pf_repr_uninit(__rte_unused struct rte_eth_dev *eth_dev)
+{
+ return 0;
+}
+
+static void
+nfp_flower_repr_free(struct nfp_flower_representor *repr,
+ enum nfp_repr_type repr_type)
+{
+ switch (repr_type) {
+ case NFP_REPR_TYPE_PHYS_PORT:
+ rte_eth_dev_destroy(repr->eth_dev, nfp_flower_repr_uninit);
+ break;
+ case NFP_REPR_TYPE_PF:
+ rte_eth_dev_destroy(repr->eth_dev, nfp_flower_pf_repr_uninit);
+ break;
+ case NFP_REPR_TYPE_VF:
+ rte_eth_dev_destroy(repr->eth_dev, nfp_flower_repr_uninit);
+ break;
+ default:
+ PMD_DRV_LOG(ERR, "Unsupported repr port type.");
+ break;
+ }
+}
+
static const struct eth_dev_ops nfp_flower_pf_repr_dev_ops = {
.dev_infos_get= nfp_flower_repr_dev_infos_get,
@@ -410,6 +447,7 @@ nfp_flower_pf_repr_init(struct rte_eth_dev *eth_dev,
repr->app_fw_flower->pf_repr = repr;
repr->app_fw_flower->pf_hw->eth_dev = eth_dev;
+ repr->eth_dev = eth_dev;
return 0;
}
@@ -501,6 +539,8 @@ nfp_flower_repr_init(struct rte_eth_dev *eth_dev,
app_fw_flower->vf_reprs[index] = repr;
}
+ repr->eth_dev = eth_dev;
+
return 0;
mac_cleanup:
@@ -511,6 +551,35 @@ nfp_flower_repr_init(struct rte_eth_dev *eth_dev,
return ret;
}
+static void
+nfp_flower_repr_free_all(struct nfp_app_fw_flower *app_fw_flower)
+{
+ uint32_t i;
+ struct nfp_flower_representor *repr;
+
+ for (i = 0; i < MAX_FLOWER_VFS; i++) {
+ repr = app_fw_flower->vf_reprs[i];
+ if (repr != NULL) {
+ nfp_flower_repr_free(repr, NFP_REPR_TYPE_VF);
+ app_fw_flower->vf_reprs[i] = NULL;
+ }
+ }
+
+ for (i = 0; i < NFP_MAX_PHYPORTS; i++) {
+ repr = app_fw_flower->phy_reprs[i];
+ if (repr != NULL) {
+ nfp_flower_repr_free(repr, NFP_REPR_TYPE_PHYS_PORT);
+ app_fw_flower->phy_reprs[i] = NULL;
+ }
+ }
+
+ repr = app_fw_flower->pf_repr;
+ if (repr != NULL) {
+ nfp_flower_repr_free(repr, NFP_REPR_TYPE_PF);
+ app_fw_flower->pf_repr = NULL;
+ }
+}
+
static int
nfp_flower_repr_alloc(struct nfp_app_fw_flower *app_fw_flower)
{
@@ -585,7 +654,7 @@ nfp_flower_repr_alloc(struct nfp_app_fw_flower
*app_fw_flower)
}
if (i < app_fw_flower->num_phyport_reprs)
- return ret;
+ goto repr_free;
/*
* Now allocate eth_dev's for VF representors.
@@ -614,9 +683,14 @@ nfp_flower_repr_alloc(struct nfp_app_fw_flower
*app_fw_flower)
}
if (i < app_fw_flower->num_vf_reprs)
- return ret;
+ goto repr_free;
return 0;
+
+repr_free:
+ nfp_flower_repr_free_all(app_fw_flower);
+
+ return ret;
}
int
@@ -635,7 +709,7 @@ nfp_flower_repr_create(struct nfp_app_fw_flower
*app_fw_flower)
/* Allocate a switch domain for the flower app */
if (app_fw_flower->switch_domain_id ==
RTE_ETH_DEV_SWITCH_DOMAIN_ID_INVALID &&
-
rte_eth_switch_domain_alloc(&app_fw_flower->switch_domain_id)) {
+
rte_eth_switch_domain_alloc(&app_fw_flower->switch_domain_id) != 0) {
PMD_INIT_LOG(WARNING, "failed to allocate switch domain for
device");
}
@@ -677,8 +751,15 @@ nfp_flower_repr_create(struct nfp_app_fw_flower
*app_fw_flower)
ret = nfp_flower_repr_alloc(app_fw_flower);
if (ret != 0) {
PMD_INIT_LOG(ERR, "representors allocation failed");
- return -EINVAL;
+ ret = -EINVAL;
+ goto domain_free;
}
return 0;
+
+domain_free:
+ if (rte_eth_switch_domain_free(app_fw_flower->switch_domain_id) != 0)
+
[PATCH 5/7] net/nfp: fix resource leak for exit of CoreNIC firmware
Fix the resource leak problem in the exit logic of CoreNIC firmware.
Fixes: 646ea79ce481 ("net/nfp: move PF functions into its own file")
Cc: sta...@dpdk.org
Signed-off-by: Chaoyong He
Reviewed-by: Long Wu
Reviewed-by: Peng Zhang
---
drivers/net/nfp/nfp_ethdev.c | 94
drivers/net/nfp/nfp_net_common.h | 1 +
2 files changed, 73 insertions(+), 22 deletions(-)
diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index bb0ddf3d54..5c5e658671 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -322,6 +322,54 @@ nfp_net_uninit(struct rte_eth_dev *eth_dev)
nfp_cpp_area_release_free(net_hw->mac_stats_area);
}
+static void
+nfp_cleanup_port_app_fw_nic(struct nfp_pf_dev *pf_dev,
+ uint8_t id)
+{
+ struct rte_eth_dev *eth_dev;
+ struct nfp_app_fw_nic *app_fw_nic;
+
+ app_fw_nic = pf_dev->app_fw_priv;
+ if (app_fw_nic->ports[id] != NULL) {
+ eth_dev = app_fw_nic->ports[id]->eth_dev;
+ if (eth_dev != NULL)
+ nfp_net_uninit(eth_dev);
+
+ app_fw_nic->ports[id] = NULL;
+ }
+}
+
+static void
+nfp_uninit_app_fw_nic(struct nfp_pf_dev *pf_dev)
+{
+ nfp_cpp_area_release_free(pf_dev->ctrl_area);
+ rte_free(pf_dev->app_fw_priv);
+}
+
+void
+nfp_pf_uninit(struct nfp_pf_dev *pf_dev)
+{
+ nfp_cpp_area_release_free(pf_dev->qc_area);
+ free(pf_dev->sym_tbl);
+ if (pf_dev->multi_pf.enabled) {
+ nfp_net_keepalive_stop(&pf_dev->multi_pf);
+ nfp_net_keepalive_uninit(&pf_dev->multi_pf);
+ }
+ free(pf_dev->nfp_eth_table);
+ free(pf_dev->hwinfo);
+ nfp_cpp_free(pf_dev->cpp);
+ rte_free(pf_dev);
+}
+
+static int
+nfp_pf_secondary_uninit(struct nfp_pf_dev *pf_dev)
+{
+ free(pf_dev->sym_tbl);
+ rte_free(pf_dev);
+
+ return 0;
+}
+
/* Reset and stop device. The device can not be restarted. */
static int
nfp_net_close(struct rte_eth_dev *dev)
@@ -333,14 +381,25 @@ nfp_net_close(struct rte_eth_dev *dev)
struct rte_pci_device *pci_dev;
struct nfp_app_fw_nic *app_fw_nic;
- if (rte_eal_process_type() != RTE_PROC_PRIMARY)
- return 0;
-
hw = dev->data->dev_private;
pf_dev = hw->pf_dev;
pci_dev = RTE_ETH_DEV_TO_PCI(dev);
app_fw_nic = NFP_PRIV_TO_APP_FW_NIC(pf_dev->app_fw_priv);
+ /*
+* In secondary process, a released eth device can be found by its name
+* in shared memory.
+* If the state of the eth device is RTE_ETH_DEV_UNUSED, it means the
+* eth device has been released.
+*/
+ if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
+ if (dev->state == RTE_ETH_DEV_UNUSED)
+ return 0;
+
+ nfp_pf_secondary_uninit(pf_dev);
+ return 0;
+ }
+
/*
* We assume that the DPDK application is stopping all the
* threads/queues before calling the device close function.
@@ -351,16 +410,17 @@ nfp_net_close(struct rte_eth_dev *dev)
nfp_net_close_tx_queue(dev);
nfp_net_close_rx_queue(dev);
- /* Clear ipsec */
- nfp_ipsec_uninit(dev);
-
/* Cancel possible impending LSC work here before releasing the port */
rte_eal_alarm_cancel(nfp_net_dev_interrupt_delayed_handler, (void
*)dev);
/* Only free PF resources after all physical ports have been closed */
/* Mark this port as unused and free device priv resources */
nn_cfg_writeb(&hw->super, NFP_NET_CFG_LSC, 0xff);
- app_fw_nic->ports[hw->idx] = NULL;
+
+ if (pf_dev->app_fw_id != NFP_APP_FW_CORE_NIC)
+ return -EINVAL;
+
+ nfp_cleanup_port_app_fw_nic(pf_dev, hw->idx);
for (i = 0; i < app_fw_nic->total_phyports; i++) {
id = nfp_function_id_get(pf_dev, i);
@@ -370,26 +430,16 @@ nfp_net_close(struct rte_eth_dev *dev)
return 0;
}
- /* Now it is safe to free all PF resources */
- PMD_INIT_LOG(INFO, "Freeing PF resources");
- if (pf_dev->multi_pf.enabled) {
- nfp_net_keepalive_stop(&pf_dev->multi_pf);
- nfp_net_keepalive_uninit(&pf_dev->multi_pf);
- }
- nfp_cpp_area_free(pf_dev->ctrl_area);
- nfp_cpp_area_free(pf_dev->qc_area);
- free(pf_dev->hwinfo);
- free(pf_dev->sym_tbl);
- nfp_cpp_free(pf_dev->cpp);
- rte_free(app_fw_nic);
- rte_free(pf_dev);
-
+ /* Enable in nfp_net_start() */
rte_intr_disable(pci_dev->intr_handle);
- /* Unregister callback func from eal lib */
+ /* Register in nfp_net_init() */
rte_intr_callback_unregister(pci_dev->intr_handle,
nfp_net_dev_interrupt_handler, (void *)dev);
+ nfp_uninit_app_fw_nic(pf_dev);
+ nfp_pf_uninit(pf_dev);
+
return
[PATCH 6/7] net/nfp: fix resource leak for exit of flower firmware
Fix the resource leak problem in the exit logic of flower firmware.
Fixes: e1124c4f8a45 ("net/nfp: add flower representor framework")
Cc: sta...@dpdk.org
Signed-off-by: Chaoyong He
Reviewed-by: Long Wu
Reviewed-by: Peng Zhang
---
drivers/net/nfp/flower/nfp_flower.c | 73 ---
drivers/net/nfp/flower/nfp_flower.h | 1 +
.../net/nfp/flower/nfp_flower_representor.c | 64
3 files changed, 80 insertions(+), 58 deletions(-)
diff --git a/drivers/net/nfp/flower/nfp_flower.c
b/drivers/net/nfp/flower/nfp_flower.c
index 6b523d98b0..3698a3d4aa 100644
--- a/drivers/net/nfp/flower/nfp_flower.c
+++ b/drivers/net/nfp/flower/nfp_flower.c
@@ -82,63 +82,6 @@ nfp_flower_pf_start(struct rte_eth_dev *dev)
return 0;
}
-/* Reset and stop device. The device can not be restarted. */
-static int
-nfp_flower_pf_close(struct rte_eth_dev *dev)
-{
- uint16_t i;
- struct nfp_net_hw *hw;
- struct nfp_pf_dev *pf_dev;
- struct nfp_net_txq *this_tx_q;
- struct nfp_net_rxq *this_rx_q;
- struct nfp_flower_representor *repr;
- struct nfp_app_fw_flower *app_fw_flower;
-
- if (rte_eal_process_type() != RTE_PROC_PRIMARY)
- return 0;
-
- repr = dev->data->dev_private;
- hw = repr->app_fw_flower->pf_hw;
- pf_dev = hw->pf_dev;
- app_fw_flower = NFP_PRIV_TO_APP_FW_FLOWER(pf_dev->app_fw_priv);
-
- nfp_mtr_priv_uninit(pf_dev);
-
- /*
-* We assume that the DPDK application is stopping all the
-* threads/queues before calling the device close function.
-*/
- nfp_net_disable_queues(dev);
-
- /* Clear queues */
- for (i = 0; i < dev->data->nb_tx_queues; i++) {
- this_tx_q = dev->data->tx_queues[i];
- nfp_net_reset_tx_queue(this_tx_q);
- }
-
- for (i = 0; i < dev->data->nb_rx_queues; i++) {
- this_rx_q = dev->data->rx_queues[i];
- nfp_net_reset_rx_queue(this_rx_q);
- }
-
- /* Cancel possible impending LSC work here before releasing the port */
- rte_eal_alarm_cancel(nfp_net_dev_interrupt_delayed_handler, (void
*)dev);
-
- nn_cfg_writeb(&hw->super, NFP_NET_CFG_LSC, 0xff);
-
- /* Now it is safe to free all PF resources */
- PMD_DRV_LOG(INFO, "Freeing PF resources");
- nfp_cpp_area_free(pf_dev->ctrl_area);
- nfp_cpp_area_free(pf_dev->qc_area);
- free(pf_dev->hwinfo);
- free(pf_dev->sym_tbl);
- nfp_cpp_free(pf_dev->cpp);
- rte_free(app_fw_flower);
- rte_free(pf_dev);
-
- return 0;
-}
-
static const struct eth_dev_ops nfp_flower_pf_vnic_ops = {
.dev_infos_get = nfp_net_infos_get,
.link_update= nfp_net_link_update,
@@ -146,7 +89,6 @@ static const struct eth_dev_ops nfp_flower_pf_vnic_ops = {
.dev_start = nfp_flower_pf_start,
.dev_stop = nfp_net_stop,
- .dev_close = nfp_flower_pf_close,
};
static inline struct nfp_flower_representor *
@@ -858,6 +800,21 @@ nfp_init_app_fw_flower(struct nfp_pf_dev *pf_dev,
return ret;
}
+void
+nfp_uninit_app_fw_flower(struct nfp_pf_dev *pf_dev)
+{
+ struct nfp_app_fw_flower *app_fw_flower;
+
+ app_fw_flower = pf_dev->app_fw_priv;
+ nfp_flower_cleanup_ctrl_vnic(app_fw_flower->ctrl_hw);
+ nfp_cpp_area_free(app_fw_flower->ctrl_hw->ctrl_area);
+ nfp_cpp_area_free(pf_dev->ctrl_area);
+ rte_free(app_fw_flower->pf_hw);
+ nfp_mtr_priv_uninit(pf_dev);
+ nfp_flow_priv_uninit(pf_dev);
+ rte_free(app_fw_flower);
+}
+
int
nfp_secondary_init_app_fw_flower(struct nfp_pf_dev *pf_dev)
{
diff --git a/drivers/net/nfp/flower/nfp_flower.h
b/drivers/net/nfp/flower/nfp_flower.h
index 6f27c06acc..8393de66c5 100644
--- a/drivers/net/nfp/flower/nfp_flower.h
+++ b/drivers/net/nfp/flower/nfp_flower.h
@@ -106,6 +106,7 @@ nfp_flower_support_decap_v2(const struct nfp_app_fw_flower
*app_fw_flower)
int nfp_init_app_fw_flower(struct nfp_pf_dev *pf_dev,
const struct nfp_dev_info *dev_info);
+void nfp_uninit_app_fw_flower(struct nfp_pf_dev *pf_dev);
int nfp_secondary_init_app_fw_flower(struct nfp_pf_dev *pf_dev);
bool nfp_flower_pf_dispatch_pkts(struct nfp_net_hw *hw,
struct rte_mbuf *mbuf,
diff --git a/drivers/net/nfp/flower/nfp_flower_representor.c
b/drivers/net/nfp/flower/nfp_flower_representor.c
index 7212d9e024..02089d390e 100644
--- a/drivers/net/nfp/flower/nfp_flower_representor.c
+++ b/drivers/net/nfp/flower/nfp_flower_representor.c
@@ -328,12 +328,75 @@ nfp_flower_repr_free(struct nfp_flower_representor *repr,
}
}
+/* Reset and stop device. The device can not be restarted. */
+static int
+nfp_flower_repr_dev_close(struct rte_eth_dev *dev)
+{
+ uint16_t i;
+ struct nfp_net_hw *hw;
+ struct nfp_pf_dev *pf_dev;
+ struct nfp_net_txq *this_tx_q;
+ struct n
[PATCH 7/7] net/nfp: fix resource leak for VF
Fix the resource leak problem in the logic of VF.
Fixes: f26e82397f6d ("net/nfp: implement xstats")
Cc: james.hers...@corigine.com
Cc: sta...@dpdk.org
Signed-off-by: Chaoyong He
Reviewed-by: Long Wu
Reviewed-by: Peng Zhang
---
drivers/net/nfp/nfp_ethdev_vf.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/net/nfp/nfp_ethdev_vf.c b/drivers/net/nfp/nfp_ethdev_vf.c
index 7927f53403..88da593190 100644
--- a/drivers/net/nfp/nfp_ethdev_vf.c
+++ b/drivers/net/nfp/nfp_ethdev_vf.c
@@ -160,13 +160,17 @@ nfp_netvf_set_link_down(struct rte_eth_dev *dev
__rte_unused)
static int
nfp_netvf_close(struct rte_eth_dev *dev)
{
+ struct nfp_net_hw *net_hw;
struct rte_pci_device *pci_dev;
if (rte_eal_process_type() != RTE_PROC_PRIMARY)
return 0;
+ net_hw = dev->data->dev_private;
pci_dev = RTE_ETH_DEV_TO_PCI(dev);
+ rte_free(net_hw->eth_xstats_base);
+
/*
* We assume that the DPDK application is stopping all the
* threads/queues before calling the device close function.
@@ -323,7 +327,7 @@ nfp_netvf_init(struct rte_eth_dev *eth_dev)
if (eth_dev->data->mac_addrs == NULL) {
PMD_INIT_LOG(ERR, "Failed to space for MAC address");
err = -ENOMEM;
- goto dev_err_ctrl_map;
+ goto free_xstats;
}
nfp_read_mac(hw);
@@ -360,8 +364,8 @@ nfp_netvf_init(struct rte_eth_dev *eth_dev)
return 0;
-dev_err_ctrl_map:
- nfp_cpp_area_free(net_hw->ctrl_area);
+free_xstats:
+ rte_free(net_hw->eth_xstats_base);
return err;
}
--
2.39.1
Re: [PATCH 24.03 v2 1/5] arg_parser: new library for command line parsing
On Wed, Nov 29, 2023 at 02:14:13PM -0800, Stephen Hemminger wrote:
> On Tue, 28 Nov 2023 14:07:41 +
> Euan Bourke wrote:
>
> > +int
> > +rte_parse_corelist(const char *corelist, uint16_t *cores, uint32_t
> > cores_len)
> > +{
> > + int32_t min = -1;
> > + char *end = NULL;
> > +
> > + struct core_bits *mask = malloc(sizeof(struct core_bits));
> > + if (mask == NULL)
> > + return -1;
> > + memset(mask, 0, sizeof(struct core_bits));
> > +
>
> The structure is not variable size, and small, why bother using malloc().
> Just use on stack structure.
Well, it's just over 4k in size, and we hit problems in the past with
alpine linux where the stack size wasn't as big as expected, leading to
issues with telemetry string manipulation. In that case I think it was due
to nested calls as well as large stack structs, though. For this core
parsing, having this struct on stack is unlikely to cause issues, but for a
non-datapath API I suggests to Euan to use malloc as a lower-risk
alternative, since performance of an arg parsing API is unlikely to be a
major concern.
However, if you feel that it should be on the stack and won't cause any
issues, it does make the code shorter and easier to work with!
/Bruce
Re: [PATCH 0/2] app/testpmd: support set RSS hash algorithm
On 11/22/2023 9:48 AM, Jie Hai wrote: > This patch set supports setting RSS hash algorithm. > > Jie Hai (2): > ethdev: add new API to get RSS hash algorithm by name > app/testpmd: support set RSS hash algorithm > > app/test-pmd/cmdline.c | 79 + > doc/guides/rel_notes/release_23_11.rst | 3 + > Hi Jie, Can you rebase the patchset to new release please? > doc/guides/testpmd_app_ug/testpmd_funcs.rst | 11 +++ > lib/ethdev/rte_ethdev.c | 15 > lib/ethdev/rte_ethdev.h | 20 ++ > lib/ethdev/version.map | 1 + > 6 files changed, 129 insertions(+) >
[v1] net/af_xdp: enable a sock path alongside use_cni
With the original 'use_cni' implementation, (using a
hardcoded socket rather than a configurable one),
if a single pod is requesting multiple net devices
and these devices are from different pools, then
the container attempts to mount all the netdev UDSes
in the pod as /tmp/afxdp.sock. Which means that at best
only 1 netdev will handshake correctly with the AF_XDP
DP. This patch addresses this by making the socket
parameter configurable alongside the 'use_cni' param.
Tested with the AF_XDP DP CNI PR 81.
Signed-off-by: Maryam Tahhan
---
doc/guides/howto/af_xdp_cni.rst | 18 +++---
drivers/net/af_xdp/rte_eth_af_xdp.c | 56 +++--
2 files changed, 51 insertions(+), 23 deletions(-)
diff --git a/doc/guides/howto/af_xdp_cni.rst b/doc/guides/howto/af_xdp_cni.rst
index a1a6d5b99c..a2d90c665d 100644
--- a/doc/guides/howto/af_xdp_cni.rst
+++ b/doc/guides/howto/af_xdp_cni.rst
@@ -38,9 +38,10 @@ The XSKMAP is a BPF map of AF_XDP sockets (XSK).
The client can then proceed with creating an AF_XDP socket
and inserting that socket into the XSKMAP pointed to by the descriptor.
-The EAL vdev argument ``use_cni`` is used to indicate that the user wishes
+The EAL vdev arguments ``use_cni`` and ``sock`` are used to indicate that the
user wishes
to run the PMD in unprivileged mode and to receive the XSKMAP file descriptor
from the CNI.
+
When this flag is set,
the ``XSK_LIBBPF_FLAGS__INHIBIT_PROG_LOAD`` libbpf flag
should be used when creating the socket
@@ -49,7 +50,7 @@ Instead the loading is handled by the CNI.
.. note::
- The Unix Domain Socket file path appear in the end user is
"/tmp/afxdp.sock".
+ The Unix Domain Socket file path appears to the end user at
"/tmp/afxdp_dp//afxdp.sock".
Prerequisites
@@ -224,7 +225,6 @@ Howto run dpdk-testpmd with CNI plugin:
capabilities:
add:
- CAP_NET_RAW
- - CAP_BPF
resources:
requests:
hugepages-2Mi: 2Gi
@@ -245,7 +245,17 @@ Howto run dpdk-testpmd with CNI plugin:
kubectl exec -i --container -- \
//dpdk-testpmd -l 0,1 --no-pci \
- --vdev=net_af_xdp0,use_cni=1,iface= \
+ --vdev=net_af_xdp0,use_cni=1,iface=,sock=/tmp/afxdp_dp//afxdp.sock \
+ -- --no-mlockall --in-memory
+
+for multiple devices use:
+
+ .. code-block:: console
+
+ kubectl exec -i --container -- \
+ //dpdk-testpmd -l 0-2 --no-pci \
+ --vdev=net_af_xdp0,use_cni=1,iface=,sock=/tmp/afxdp_dp//afxdp.sock \
+ --vdev=net_af_xdp1,use_cni=1,iface=,sock=/tmp/afxdp_dp//afxdp.sock \
-- --no-mlockall --in-memory
For further reference please use the `e2e`_ test case in `AF_XDP Plugin for
Kubernetes`_
diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c
b/drivers/net/af_xdp/rte_eth_af_xdp.c
index 353c8688ec..f728dae2f9 100644
--- a/drivers/net/af_xdp/rte_eth_af_xdp.c
+++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
@@ -88,7 +88,6 @@ RTE_LOG_REGISTER_DEFAULT(af_xdp_logtype, NOTICE);
#define UDS_MAX_CMD_LEN64
#define UDS_MAX_CMD_RESP 128
#define UDS_XSK_MAP_FD_MSG "/xsk_map_fd"
-#define UDS_SOCK "/tmp/afxdp.sock"
#define UDS_CONNECT_MSG"/connect"
#define UDS_HOST_OK_MSG"/host_ok"
#define UDS_HOST_NAK_MSG "/host_nak"
@@ -171,6 +170,7 @@ struct pmd_internals {
bool custom_prog_configured;
bool force_copy;
bool use_cni;
+ char sock_path[PATH_MAX];
struct bpf_map *map;
struct rte_ether_addr eth_addr;
@@ -191,6 +191,7 @@ struct pmd_process_private {
#define ETH_AF_XDP_BUDGET_ARG "busy_budget"
#define ETH_AF_XDP_FORCE_COPY_ARG "force_copy"
#define ETH_AF_XDP_USE_CNI_ARG "use_cni"
+#define ETH_AF_XDP_SOCK_ARG"sock"
static const char * const valid_arguments[] = {
ETH_AF_XDP_IFACE_ARG,
@@ -201,6 +202,7 @@ static const char * const valid_arguments[] = {
ETH_AF_XDP_BUDGET_ARG,
ETH_AF_XDP_FORCE_COPY_ARG,
ETH_AF_XDP_USE_CNI_ARG,
+ ETH_AF_XDP_SOCK_ARG,
NULL
};
@@ -1351,7 +1353,7 @@ configure_preferred_busy_poll(struct pkt_rx_queue *rxq)
}
static int
-init_uds_sock(struct sockaddr_un *server)
+init_uds_sock(struct sockaddr_un *server, const char *sock_path)
{
int sock;
@@ -1362,7 +1364,7 @@ init_uds_sock(struct sockaddr_un *server)
}
server->sun_family = AF_UNIX;
- strlcpy(server->sun_path, UDS_SOCK, sizeof(server->sun_path));
+ strlcpy(server->sun_path, sock_path, sizeof(server->sun_path));
if (connect(sock, (struct sockaddr *)server, sizeof(struct
sockaddr_un)) < 0) {
close(sock);
@@ -1382,7 +1384,7 @@ struct msg_internal {
};
static int
-send_msg(int sock, char *request, int *fd)
+send_msg(int sock, char *request, int *fd, const c
Re: [PATCH] version: 24.03-rc0
On Wed, Nov 29, 2023 at 5:20 PM David Marchand wrote: > > Start a new release cycle with empty release notes. > Bump version and ABI minor. > Bump libabigail from 2.1 to 2.4 and enable ABI checks. > > Signed-off-by: David Marchand Applied, thanks. Copying subtree maintainers, and ci@ mailing list. As a result of the discussion on the ci ml (http://inbox.dpdk.org/ci/cajvnsub2zxmhcdzc4rlrurm75myohjuwfuttk8gnvredfp_...@mail.gmail.com/T/#t), we worked on setting up some automatic mirroring of dpdk.org repositories to the github DPDK repository. What it means: - for the https://dpdk.org/git/dpdk repository, all the branches and tags are mirrored to https://github.com/DPDK/dpdk as it was done so far, - for the https://git.dpdk.org/next/dpdk-next-* repositories, only branches named "main", "staging" or "for-*" are mirrored to https://github.com/DPDK/dpdk with a prefix. One example: changes to the for-main branch of the dpdk-next-crypto repo will be mirrored to a github branch next-crypto-for-main For subtree maintainers. It should change nothing to you guys. Please rebase your trees on v24.03-rc0 and push it to dpdk.org. And then double check that the mirror happened (looking at git push output, and checking https://github.com/DPDK/dpdk/branches). If you hit some issue, ping me. For CI. This is more a fyi, there will be some delay before all mirrors are up, so for now, don't swap the next-* repositories in your script yet. However, one action that can be taken as v24.03-rc0 is pushed is to re-enable ABI checks against the v23.11 official tag. Questions? -- David Marchand
Re: [PATCH] version: 24.03-rc0
On 11/30/2023 9:23 AM, David Marchand wrote: > On Wed, Nov 29, 2023 at 5:20 PM David Marchand > wrote: >> >> Start a new release cycle with empty release notes. >> Bump version and ABI minor. >> Bump libabigail from 2.1 to 2.4 and enable ABI checks. >> >> Signed-off-by: David Marchand > > Applied, thanks. > > Copying subtree maintainers, and ci@ mailing list. > As a result of the discussion on the ci ml > (http://inbox.dpdk.org/ci/cajvnsub2zxmhcdzc4rlrurm75myohjuwfuttk8gnvredfp_...@mail.gmail.com/T/#t), > we worked on setting up some automatic mirroring of dpdk.org > repositories to the github DPDK repository. > > > What it means: > - for the https://dpdk.org/git/dpdk repository, all the branches and > tags are mirrored to https://github.com/DPDK/dpdk as it was done so > far, > - for the https://git.dpdk.org/next/dpdk-next-* repositories, only > branches named "main", "staging" or "for-*" are mirrored to > https://github.com/DPDK/dpdk with a prefix. > One example: changes to the for-main branch of the dpdk-next-crypto > repo will be mirrored to a github branch next-crypto-for-main > > > For subtree maintainers. > > It should change nothing to you guys. > Please rebase your trees on v24.03-rc0 and push it to dpdk.org. > And then double check that the mirror happened (looking at git push > output, and checking https://github.com/DPDK/dpdk/branches). > If you hit some issue, ping me. > I force push a lot, is mirroring configured to cope with this? > > For CI. > > This is more a fyi, there will be some delay before all mirrors are > up, so for now, don't swap the next-* repositories in your script yet. > However, one action that can be taken as v24.03-rc0 is pushed is to > re-enable ABI checks against the v23.11 official tag. > > > Questions? > >
Re: [PATCH] version: 24.03-rc0
On Thu, Nov 30, 2023 at 10:30 AM Ferruh Yigit wrote: > > On 11/30/2023 9:23 AM, David Marchand wrote: > > On Wed, Nov 29, 2023 at 5:20 PM David Marchand > > wrote: > >> > >> Start a new release cycle with empty release notes. > >> Bump version and ABI minor. > >> Bump libabigail from 2.1 to 2.4 and enable ABI checks. > >> > >> Signed-off-by: David Marchand > > > > Applied, thanks. > > > > Copying subtree maintainers, and ci@ mailing list. > > As a result of the discussion on the ci ml > > (http://inbox.dpdk.org/ci/cajvnsub2zxmhcdzc4rlrurm75myohjuwfuttk8gnvredfp_...@mail.gmail.com/T/#t), > > we worked on setting up some automatic mirroring of dpdk.org > > repositories to the github DPDK repository. > > > > > > What it means: > > - for the https://dpdk.org/git/dpdk repository, all the branches and > > tags are mirrored to https://github.com/DPDK/dpdk as it was done so > > far, > > - for the https://git.dpdk.org/next/dpdk-next-* repositories, only > > branches named "main", "staging" or "for-*" are mirrored to > > https://github.com/DPDK/dpdk with a prefix. > > One example: changes to the for-main branch of the dpdk-next-crypto > > repo will be mirrored to a github branch next-crypto-for-main > > > > > > For subtree maintainers. > > > > It should change nothing to you guys. > > Please rebase your trees on v24.03-rc0 and push it to dpdk.org. > > And then double check that the mirror happened (looking at git push > > output, and checking https://github.com/DPDK/dpdk/branches). > > If you hit some issue, ping me. > > > > I force push a lot, is mirroring configured to cope with this? It is supposed to be handled, yes. -- David Marchand
Re: [PATCH] version: 24.03-rc0
On Thu, Nov 30, 2023 at 10:32 AM David Marchand wrote: > > > For subtree maintainers. > > > > > > It should change nothing to you guys. > > > Please rebase your trees on v24.03-rc0 and push it to dpdk.org. > > > And then double check that the mirror happened (looking at git push > > > output, and checking https://github.com/DPDK/dpdk/branches). > > > If you hit some issue, ping me. > > > > > > > I force push a lot, is mirroring configured to cope with this? > > It is supposed to be handled, yes. Thanks to Akhil and Jerin, I can confirm mirroring and force push work ;-). -- David Marchand
Re: [PATCH 0/2] app/testpmd: support set RSS hash algorithm
On 2023/11/30 17:02, Ferruh Yigit wrote: On 11/22/2023 9:48 AM, Jie Hai wrote: This patch set supports setting RSS hash algorithm. Jie Hai (2): ethdev: add new API to get RSS hash algorithm by name app/testpmd: support set RSS hash algorithm app/test-pmd/cmdline.c | 79 + doc/guides/rel_notes/release_23_11.rst | 3 + Hi Jie, Can you rebase the patchset to new release please? Hi, Ferruh, Thanks, I will do it as soon as possible. Thanks, Jie Hai doc/guides/testpmd_app_ug/testpmd_funcs.rst | 11 +++ lib/ethdev/rte_ethdev.c | 15 lib/ethdev/rte_ethdev.h | 20 ++ lib/ethdev/version.map | 1 + 6 files changed, 129 insertions(+) .
Re: [PATCH] version: 24.03-rc0
On 11/30/2023 9:53 AM, David Marchand wrote: > On Thu, Nov 30, 2023 at 10:32 AM David Marchand > wrote: For subtree maintainers. It should change nothing to you guys. Please rebase your trees on v24.03-rc0 and push it to dpdk.org. And then double check that the mirror happened (looking at git push output, and checking https://github.com/DPDK/dpdk/branches). If you hit some issue, ping me. >>> >>> I force push a lot, is mirroring configured to cope with this? >> >> It is supposed to be handled, yes. > > Thanks to Akhil and Jerin, I can confirm mirroring and force push work ;-). > Cool, thanks.
Re: [PATCH v2] lib/ethdev: modified the definition of 'NVGRE_ENCAP'
On 11/30/2023 6:22 AM, Sunyang Wu wrote:
> Fix the issue of incorrect definition of 'NVGRE_ENCAP', and
> modified the error comments of 'rte_flow_action_nvgre_encap'.
>
> Fixes: c2beb1d469d2 ("ethdev: add missing items/actions to flow object
> converter")
> Fixes: 3850cf0c8c37 ("ethdev: add tunnel encap/decap actions")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Joey Xing
> Signed-off-by: Sunyang Wu
>
Moving from previous version:
Acked-by: Ori Kam
Acked-by: Ferruh Yigit
Applied to dpdk-next-net/main, thanks.
[RESEND 0/2] app/testpmd: support set RSS hash algorithm
This patch set supports setting RSS hash algorithm. Jie Hai (2): ethdev: add new API to get RSS hash algorithm by name app/testpmd: support set RSS hash algorithm app/test-pmd/cmdline.c | 79 + doc/guides/rel_notes/release_24_03.rst | 5 ++ doc/guides/testpmd_app_ug/testpmd_funcs.rst | 11 +++ lib/ethdev/rte_ethdev.c | 15 lib/ethdev/rte_ethdev.h | 20 ++ lib/ethdev/version.map | 3 + 6 files changed, 133 insertions(+) -- 2.30.0
[RESEND 1/2] ethdev: add new API to get RSS hash algorithm by name
This patch supports conversion from names to hash algorithm
(see RTE_ETH_HASH_FUNCTION_XXX).
Signed-off-by: Jie Hai
Reviewed-by: Huisong Li
---
doc/guides/rel_notes/release_24_03.rst | 5 +
lib/ethdev/rte_ethdev.c| 15 +++
lib/ethdev/rte_ethdev.h| 20
lib/ethdev/version.map | 3 +++
4 files changed, 43 insertions(+)
diff --git a/doc/guides/rel_notes/release_24_03.rst
b/doc/guides/rel_notes/release_24_03.rst
index e9c9717706a0..bd84875d4f17 100644
--- a/doc/guides/rel_notes/release_24_03.rst
+++ b/doc/guides/rel_notes/release_24_03.rst
@@ -55,6 +55,11 @@ New Features
Also, make sure to start the actual text at the margin.
===
+* **Improved support of RSS hash algorithm.**
+
+ * Added new function ``rte_eth_find_rss_algo`` to get RSS hash
+algorithm by its name.
+
Removed Items
-
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 3858983fcc80..c0398d945502 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -4826,6 +4826,21 @@ rte_eth_dev_rss_algo_name(enum rte_eth_hash_function
rss_algo)
return name;
}
+int
+rte_eth_find_rss_algo(const char *name, uint32_t *algo)
+{
+ unsigned int i;
+
+ for (i = 0; i < RTE_DIM(rte_eth_dev_rss_algo_names); i++) {
+ if (strcmp(name, rte_eth_dev_rss_algo_names[i].name) == 0) {
+ *algo = rte_eth_dev_rss_algo_names[i].algo;
+ return 0;
+ }
+ }
+
+ return -EINVAL;
+}
+
int
rte_eth_dev_udp_tunnel_port_add(uint16_t port_id,
struct rte_eth_udp_tunnel *udp_tunnel)
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 77331ce6525e..3bde5c4c17a6 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -4667,6 +4667,26 @@ __rte_experimental
const char *
rte_eth_dev_rss_algo_name(enum rte_eth_hash_function rss_algo);
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice.
+ *
+ * Get RSS hash algorithm by its name.
+ *
+ * @param name
+ * RSS hash algorithm.
+ *
+ * @param algo
+ * return the RSS hash algorithm found, @see rte_eth_hash_function.
+ *
+ * @return
+ * - (0) if successful.
+ * - (-EINVAL) if not found.
+ */
+__rte_experimental
+int
+rte_eth_find_rss_algo(const char *name, uint32_t *algo);
+
/**
* Add UDP tunneling port for a type of tunnel.
*
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index 5c4917c020dc..a050baab0fe7 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -316,6 +316,9 @@ EXPERIMENTAL {
rte_eth_recycle_rx_queue_info_get;
rte_flow_group_set_miss_actions;
rte_flow_calc_table_hash;
+
+ # added in 24.03
+ rte_eth_find_rss_algo;
};
INTERNAL {
--
2.30.0
[RESEND 2/2] app/testpmd: support set RSS hash algorithm
Since API rte_eth_dev_rss_hash_update() supports setting RSS hash
algorithm, add new command to support it:
testpmd> port config 0 rss-hash-algo symmetric_toeplitz
Signed-off-by: Jie Hai
Reviewed-by: Huisong Li
---
app/test-pmd/cmdline.c | 79 +
doc/guides/testpmd_app_ug/testpmd_funcs.rst | 11 +++
2 files changed, 90 insertions(+)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 9369d3b4c526..2cd85c918a09 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -726,6 +726,10 @@ static void cmd_help_long_parsed(void *parsed_result,
"port config port-id rss reta
(hash,queue)[,(hash,queue)]\n"
"Set the RSS redirection table.\n\n"
+ "port config port-id rss-hash-algo
(default|simple_xor|toeplitz|"
+ "symmetric_toeplitz|symmetric_toeplitz_sort)\n"
+ "Set the RSS hash algorithm.\n\n"
+
"port config (port_id) dcb vt (on|off) (traffic_class)"
" pfc (on|off)\n"
"Set the DCB mode.\n\n"
@@ -2275,6 +2279,80 @@ static cmdline_parse_inst_t cmd_config_rss_hash_key = {
},
};
+/* *** configure rss hash algorithm *** */
+struct cmd_config_rss_hash_algo {
+ cmdline_fixed_string_t port;
+ cmdline_fixed_string_t config;
+ portid_t port_id;
+ cmdline_fixed_string_t rss_hash_algo;
+ cmdline_fixed_string_t algo;
+};
+
+static void
+cmd_config_rss_hash_algo_parsed(void *parsed_result,
+ __rte_unused struct cmdline *cl,
+ __rte_unused void *data)
+{
+ struct cmd_config_rss_hash_algo *res = parsed_result;
+ uint8_t rss_key[RSS_HASH_KEY_LENGTH];
+ struct rte_eth_rss_conf rss_conf;
+ int ret;
+
+ rss_conf.rss_key_len = RSS_HASH_KEY_LENGTH;
+ rss_conf.rss_key = rss_key;
+ ret = rte_eth_dev_rss_hash_conf_get(res->port_id, &rss_conf);
+ if (ret != 0) {
+ fprintf(stderr, "failed to get port %u RSS confinguration\n",
+ res->port_id);
+ return;
+ }
+
+ ret = rte_eth_find_rss_algo(res->algo, &rss_conf.algorithm);
+ if (ret != 0) {
+ fprintf(stderr, "port %u configured invalid RSS hash algorithm:
%s\n",
+ res->port_id, res->algo);
+ return;
+ }
+
+ ret = rte_eth_dev_rss_hash_update(res->port_id, &rss_conf);
+ if (ret != 0) {
+ fprintf(stderr, "failed to set port %u RSS hash algorithm\n",
+ res->port_id);
+ return;
+ }
+}
+
+static cmdline_parse_token_string_t cmd_config_rss_hash_algo_port =
+ TOKEN_STRING_INITIALIZER(struct cmd_config_rss_hash_algo, port, "port");
+static cmdline_parse_token_string_t cmd_config_rss_hash_algo_config =
+ TOKEN_STRING_INITIALIZER(struct cmd_config_rss_hash_algo, config,
+"config");
+static cmdline_parse_token_num_t cmd_config_rss_hash_algo_port_id =
+ TOKEN_NUM_INITIALIZER(struct cmd_config_rss_hash_algo, port_id,
+ RTE_UINT16);
+static cmdline_parse_token_string_t cmd_config_rss_hash_algo_rss_hash_algo =
+ TOKEN_STRING_INITIALIZER(struct cmd_config_rss_hash_algo,
+rss_hash_algo, "rss-hash-algo");
+static cmdline_parse_token_string_t cmd_config_rss_hash_algo_algo =
+ TOKEN_STRING_INITIALIZER(struct cmd_config_rss_hash_algo, algo,
+"default#simple_xor#toeplitz#"
+"symmetric_toeplitz#symmetric_toeplitz_sort");
+
+static cmdline_parse_inst_t cmd_config_rss_hash_algo = {
+ .f = cmd_config_rss_hash_algo_parsed,
+ .data = NULL,
+ .help_str = "port config rss-hash-algo "
+
"(default|simple_xor|toeplitz|symmetric_toeplitz|symmetric_toeplitz_sort)",
+ .tokens = {
+ (void *)&cmd_config_rss_hash_algo_port,
+ (void *)&cmd_config_rss_hash_algo_config,
+ (void *)&cmd_config_rss_hash_algo_port_id,
+ (void *)&cmd_config_rss_hash_algo_rss_hash_algo,
+ (void *)&cmd_config_rss_hash_algo_algo,
+ NULL,
+ },
+};
+
/* *** cleanup txq mbufs *** */
struct cmd_cleanup_txq_mbufs_result {
cmdline_fixed_string_t port;
@@ -13165,6 +13243,7 @@ static cmdline_parse_ctx_t builtin_ctx[] = {
(cmdline_parse_inst_t *)&cmd_showport_rss_hash_key,
(cmdline_parse_inst_t *)&cmd_showport_rss_hash_algo,
(cmdline_parse_inst_t *)&cmd_config_rss_hash_key,
+ (cmdline_parse_inst_t *)&cmd_config_rss_hash_algo,
(cmdline_parse_inst_t *)&cmd_cleanup_txq_mbufs,
(cmdline_parse_inst_t *)&cmd_dump,
(cmdline_parse_inst_t *)&cmd_dump_one,
diff --git a/doc/guides/testpmd_app_ug/testpmd_fu
Re: [PATCH 5/7] net/nfp: fix resource leak for exit of CoreNIC firmware
On 11/30/2023 8:52 AM, Chaoyong He wrote:
> Fix the resource leak problem in the exit logic of CoreNIC firmware.
>
> Fixes: 646ea79ce481 ("net/nfp: move PF functions into its own file")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Chaoyong He
> Reviewed-by: Long Wu
> Reviewed-by: Peng Zhang
<...>
> +static int
> +nfp_pf_secondary_uninit(struct nfp_pf_dev *pf_dev)
> +{
> + free(pf_dev->sym_tbl);
> + rte_free(pf_dev);
> +
> + return 0;
> +}
> +
> /* Reset and stop device. The device can not be restarted. */
> static int
> nfp_net_close(struct rte_eth_dev *dev)
> @@ -333,14 +381,25 @@ nfp_net_close(struct rte_eth_dev *dev)
> struct rte_pci_device *pci_dev;
> struct nfp_app_fw_nic *app_fw_nic;
>
> - if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> - return 0;
> -
> hw = dev->data->dev_private;
> pf_dev = hw->pf_dev;
> pci_dev = RTE_ETH_DEV_TO_PCI(dev);
> app_fw_nic = NFP_PRIV_TO_APP_FW_NIC(pf_dev->app_fw_priv);
>
> + /*
> + * In secondary process, a released eth device can be found by its name
> + * in shared memory.
> + * If the state of the eth device is RTE_ETH_DEV_UNUSED, it means the
> + * eth device has been released.
> + */
> + if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
> + if (dev->state == RTE_ETH_DEV_UNUSED)
> + return 0;
> +
> + nfp_pf_secondary_uninit(pf_dev);
> + return 0;
> + }
> +
>
Mostly expectation is secondary process doesn't free shared resources,
but init and free done by primary process.
When there are multiple secondaries active, and if one of them closes
the port, will system behave properly? Can you please double check above
logic?
Re: [RESEND 1/2] ethdev: add new API to get RSS hash algorithm by name
On 11/30/2023 10:44 AM, Jie Hai wrote: > This patch supports conversion from names to hash algorithm > (see RTE_ETH_HASH_FUNCTION_XXX). > > Signed-off-by: Jie Hai > Reviewed-by: Huisong Li > Reviewed-by: Ferruh Yigit
net/e1000: Test issues following change in max rx/tx queues
Hello, We recently started to observe concerning behaviour on our continuous integration platform following this commit, which set the maximum amount of tx/rx queues at 2 for the e1000 pmd: net/e1000: fix queue number initialization https://git.dpdk.org/dpdk/commit/?id=c1a42d646472fd3477429bf016f682e0865b77f0 Reverting this change locally on our side was enough to fix our test issues. There is a considerately long explanation in a comment just above the change explaining why the number of rx/tx queues is limited at 1, yet it was changed at 2 anyway. Since I couldn't see any mention in logs about theses restrictions being removed, I wished to inquire whether this change was truly intended and if there was any problem which motivated it. Maybe the max number of rx/tx queues should reverted back to 1? Or maybe the comment should be updated if limitations are no longer true? Regards, Edwin Brossette.
Re: [RESEND 2/2] app/testpmd: support set RSS hash algorithm
On 11/30/2023 10:44 AM, Jie Hai wrote:
> Since API rte_eth_dev_rss_hash_update() supports setting RSS hash
> algorithm, add new command to support it:
>
> testpmd> port config 0 rss-hash-algo symmetric_toeplitz
>
> Signed-off-by: Jie Hai
> Reviewed-by: Huisong Li
> ---
> app/test-pmd/cmdline.c | 79 +
> doc/guides/testpmd_app_ug/testpmd_funcs.rst | 11 +++
> 2 files changed, 90 insertions(+)
>
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 9369d3b4c526..2cd85c918a09 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -726,6 +726,10 @@ static void cmd_help_long_parsed(void *parsed_result,
> "port config port-id rss reta
> (hash,queue)[,(hash,queue)]\n"
> "Set the RSS redirection table.\n\n"
>
> + "port config port-id rss-hash-algo
> (default|simple_xor|toeplitz|"
>
Variables marked by putting them within (), so 'port-id' should be
(port_id).
<...>
> +static cmdline_parse_inst_t cmd_config_rss_hash_algo = {
> + .f = cmd_config_rss_hash_algo_parsed,
> + .data = NULL,
> + .help_str = "port config rss-hash-algo "
> +
> "(default|simple_xor|toeplitz|symmetric_toeplitz|symmetric_toeplitz_sort)",
>
'simple_xor', 'toeplitz', etc.. are keywords, not name of variable, so
should not use (). Instead like:
"port config rss-hash-algo default|simple_xor|..."
Re: [RESEND 2/2] app/testpmd: support set RSS hash algorithm
On 11/30/2023 10:44 AM, Jie Hai wrote:
> +static void
> +cmd_config_rss_hash_algo_parsed(void *parsed_result,
> + __rte_unused struct cmdline *cl,
> + __rte_unused void *data)
> +{
> + struct cmd_config_rss_hash_algo *res = parsed_result;
> + uint8_t rss_key[RSS_HASH_KEY_LENGTH];
> + struct rte_eth_rss_conf rss_conf;
> + int ret;
> +
> + rss_conf.rss_key_len = RSS_HASH_KEY_LENGTH;
> + rss_conf.rss_key = rss_key;
> + ret = rte_eth_dev_rss_hash_conf_get(res->port_id, &rss_conf);
> + if (ret != 0) {
> + fprintf(stderr, "failed to get port %u RSS confinguration\n",
>
s/confinguration/configuration/
Marvell DPDK 24.03 Roadmap
Marvell - DPDK v24.03 Roadmap for generic changes === cryptodev = 1) Support EDDSA asymmetric crypto algorithm ethdev == 1) API to get number of free descriptors available from a Tx queue eventdev 1) Support eventdev adapter for mldev (Backlog from previous release) mldev = 1) Support int64/uint64 datatypes virtio driver = 1) net/virtio-user: add VIRTIO_NET_F_RSS to supported features 2) net/virtio-user: improve kick performance with notification area mapping Test applications = 1) test/app: add TLS/DTLS test suite with vectors 2) app/graph: add L2FWD use case 3) app/test-mldev: Updates to output validation 4) app/testeventdev: support for dma adapter 5) app/dma-perf: support bi-directional transfer Marvell - DPDK v24.03 Roadmap for driver changes cnxk-ethdev === 1) Support port representor (Backlog from previous release) 2) Security Rx inject support cnxk-mldev == 1) ML event adapter support cnxk-dmadev: === 1) DMA adaptor support cnxk-cryptodev = 1) TLS 1.2, DTLS 1.2 & TLS 1.3 support 2) Security Rx inject support
RE: net/e1000: Test issues following change in max rx/tx queues
+TO: Qiming Yang, author of the patch. +CC: stable > From: Edwin Brossette [mailto:edwin.brosse...@6wind.com] > Sent: Thursday, 30 November 2023 12.25 > > Hello, > > We recently started to observe concerning behaviour on our continuous > integration platform following this commit, which set the maximum amount of > tx/rx queues at 2 for the e1000 pmd: > > net/e1000: fix queue number initialization > https://git.dpdk.org/dpdk/commit/?id=c1a42d646472fd3477429bf016f682e0865b77f0 > > Reverting this change locally on our side was enough to fix our test issues. > > There is a considerately long explanation in a comment just above the change > explaining why the number of rx/tx queues is limited at 1, yet it was changed > at 2 anyway. For reference, here is the explanation in the source code: Starting with 631xESB hw supports 2 TX/RX queues per port. Unfortunatelly, all these nics have just one TX context. So we have few choises for TX: - Use just one TX queue. - Allow cksum offload only for one TX queue. - Don't allow TX cksum offload at all. For now, option #1 was chosen. To use second RX queue we have to use extended RX descriptor (Multiple Receive Queues are mutually exclusive with UDP fragmentation and are not supported when a legacy receive descriptor format is used). Which means separate RX routinies - as legacy nics (82540, 82545) don't support extended RXD. To avoid it we support just one RX queue for now (no RSS). > > Since I couldn't see any mention in logs about theses restrictions being > removed, I wished to inquire whether this change was truly intended and if > there was any problem which motivated it. Qiming, what is the explanation, please? > Maybe the max number of rx/tx queues should reverted back to 1? Or maybe the > comment should be updated if limitations are no longer true? Your CI platform flagging problems is a strong indicator of something being wrong. So I would guess the limitations are still true, and the max number of rx/tx queues should be reverted back to 1. If someone really needs 2 queues, perhaps it could be a build time option to enable 2 queues without offloads. Qiming, what do you think? > > Regards, > Edwin Brossette.
RE: [v1] net/af_xdp: enable a sock path alongside use_cni
Hi Maryam,
I have added some suggestion below.
Regrads,
Shibin
> -Original Message-
> From: Maryam Tahhan
> Sent: Thursday, November 30, 2023 9:14 AM
> To: ferruh.yi...@amd.com; step...@networkplumber.org;
> lihuis...@huawei.com; fengcheng...@huawei.com;
> liuyongl...@huawei.com
> Cc: dev@dpdk.org; Tahhan, Maryam
> Subject: [v1] net/af_xdp: enable a sock path alongside use_cni
>
> With the original 'use_cni' implementation, (using a hardcoded socket rather
> than a configurable one), if a single pod is requesting multiple net devices
> and these devices are from different pools, then the container attempts to
> mount all the netdev UDSes in the pod as /tmp/afxdp.sock. Which means
> that at best only 1 netdev will handshake correctly with the AF_XDP DP. This
> patch addresses this by making the socket parameter configurable alongside
> the 'use_cni' param.
> Tested with the AF_XDP DP CNI PR 81.
>
> Signed-off-by: Maryam Tahhan
> ---
> doc/guides/howto/af_xdp_cni.rst | 18 +++---
> drivers/net/af_xdp/rte_eth_af_xdp.c | 56 +++--
> 2 files changed, 51 insertions(+), 23 deletions(-)
>
> diff --git a/doc/guides/howto/af_xdp_cni.rst
> b/doc/guides/howto/af_xdp_cni.rst index a1a6d5b99c..a2d90c665d 100644
> --- a/doc/guides/howto/af_xdp_cni.rst
> +++ b/doc/guides/howto/af_xdp_cni.rst
> @@ -38,9 +38,10 @@ The XSKMAP is a BPF map of AF_XDP sockets (XSK).
> The client can then proceed with creating an AF_XDP socket and inserting
> that socket into the XSKMAP pointed to by the descriptor.
>
> -The EAL vdev argument ``use_cni`` is used to indicate that the user wishes
> +The EAL vdev arguments ``use_cni`` and ``sock`` are used to indicate
> +that the user wishes
> to run the PMD in unprivileged mode and to receive the XSKMAP file
> descriptor from the CNI.
> +
> When this flag is set,
> the ``XSK_LIBBPF_FLAGS__INHIBIT_PROG_LOAD`` libbpf flag should be
> used when creating the socket @@ -49,7 +50,7 @@ Instead the loading is
> handled by the CNI.
>
> .. note::
>
> - The Unix Domain Socket file path appear in the end user is
> "/tmp/afxdp.sock".
> + The Unix Domain Socket file path appears to the end user at
> "/tmp/afxdp_dp//afxdp.sock".
>
>
> Prerequisites
> @@ -224,7 +225,6 @@ Howto run dpdk-testpmd with CNI plugin:
>capabilities:
> add:
> - CAP_NET_RAW
> - - CAP_BPF
> resources:
> requests:
> hugepages-2Mi: 2Gi
> @@ -245,7 +245,17 @@ Howto run dpdk-testpmd with CNI plugin:
>
> kubectl exec -i --container -- \
> //dpdk-testpmd -l 0,1 --no-pci \
> - --vdev=net_af_xdp0,use_cni=1,iface= \
> + --vdev=net_af_xdp0,use_cni=1,iface= name>,sock=/tmp/afxdp_dp//afxdp.sock \
> + -- --no-mlockall --in-memory
> +
> +for multiple devices use:
> +
> + .. code-block:: console
> +
> + kubectl exec -i --container -- \
> + //dpdk-testpmd -l 0-2 --no-pci \
> + --vdev=net_af_xdp0,use_cni=1,iface= name>,sock=/tmp/afxdp_dp//afxdp.sock \
> + --vdev=net_af_xdp1,use_cni=1,iface= + name>,sock=/tmp/afxdp_dp//afxdp.sock \
> -- --no-mlockall --in-memory
>
> For further reference please use the `e2e`_ test case in `AF_XDP Plugin for
> Kubernetes`_ diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c
> b/drivers/net/af_xdp/rte_eth_af_xdp.c
> index 353c8688ec..f728dae2f9 100644
> --- a/drivers/net/af_xdp/rte_eth_af_xdp.c
> +++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
> @@ -88,7 +88,6 @@ RTE_LOG_REGISTER_DEFAULT(af_xdp_logtype,
> NOTICE);
> #define UDS_MAX_CMD_LEN 64
> #define UDS_MAX_CMD_RESP 128
> #define UDS_XSK_MAP_FD_MSG "/xsk_map_fd"
> -#define UDS_SOCK "/tmp/afxdp.sock"
> #define UDS_CONNECT_MSG "/connect"
> #define UDS_HOST_OK_MSG "/host_ok"
> #define UDS_HOST_NAK_MSG "/host_nak"
> @@ -171,6 +170,7 @@ struct pmd_internals {
> bool custom_prog_configured;
> bool force_copy;
> bool use_cni;
> + char sock_path[PATH_MAX];
I would recommend using variable name as "uds_path".
> struct bpf_map *map;
>
> struct rte_ether_addr eth_addr;
> @@ -191,6 +191,7 @@ struct pmd_process_private {
> #define ETH_AF_XDP_BUDGET_ARG"busy_budget"
> #define ETH_AF_XDP_FORCE_COPY_ARG"force_copy"
> #define ETH_AF_XDP_USE_CNI_ARG "use_cni"
> +#define ETH_AF_XDP_SOCK_ARG "sock"
To make it clear would recommend using "sock_path" and also
ETH_AF_XDP_CNI_UDS_PATH_ARG or ETH_AF_XDP_SOCK_PATH_ARG.
>
> static const char * const valid_arguments[] = {
> ETH_AF_XDP_IFACE_ARG,
> @@ -201,6 +202,7 @@ static const char * const valid_arguments[] = {
> ETH_AF_XDP_BUDGET_ARG,
> ETH_AF_XDP_FORCE_COPY_ARG,
> ETH_AF_XDP_USE_CNI_ARG,
> + ETH_AF_XDP_SOCK_ARG,
> NULL
> };
>
Re: [v1] net/af_xdp: enable a sock path alongside use_cni
Hi Shibin
Thanks for the review. Comments inline.
BR
Maryam
On 30/11/2023 12:14, Koikkara Reeny, Shibin wrote:
Hi Maryam,
I have added some suggestion below.
Regrads,
Shibin
[snip]
NOTICE);
#define UDS_MAX_CMD_LEN 64
#define UDS_MAX_CMD_RESP 128
#define UDS_XSK_MAP_FD_MSG"/xsk_map_fd"
-#define UDS_SOCK "/tmp/afxdp.sock"
#define UDS_CONNECT_MSG "/connect"
#define UDS_HOST_OK_MSG "/host_ok"
#define UDS_HOST_NAK_MSG "/host_nak"
@@ -171,6 +170,7 @@ struct pmd_internals {
bool custom_prog_configured;
bool force_copy;
bool use_cni;
+ char sock_path[PATH_MAX];
I would recommend using variable name as "uds_path".
No issues in renaming
struct bpf_map *map;
struct rte_ether_addr eth_addr;
@@ -191,6 +191,7 @@ struct pmd_process_private {
#define ETH_AF_XDP_BUDGET_ARG "busy_budget"
#define ETH_AF_XDP_FORCE_COPY_ARG "force_copy"
#define ETH_AF_XDP_USE_CNI_ARG"use_cni"
+#define ETH_AF_XDP_SOCK_ARG"sock"
To make it clear would recommend using "sock_path" and also
ETH_AF_XDP_CNI_UDS_PATH_ARG or ETH_AF_XDP_SOCK_PATH_ARG
No issues in renaming
.
static const char * const valid_arguments[] = {
ETH_AF_XDP_IFACE_ARG,
@@ -201,6 +202,7 @@ static const char * const valid_arguments[] = {
ETH_AF_XDP_BUDGET_ARG,
ETH_AF_XDP_FORCE_COPY_ARG,
ETH_AF_XDP_USE_CNI_ARG,
+ ETH_AF_XDP_SOCK_ARG,
NULL
};
@@ -1351,7 +1353,7 @@ configure_preferred_busy_poll(struct
pkt_rx_queue *rxq) }
static int
-init_uds_sock(struct sockaddr_un *server)
+init_uds_sock(struct sockaddr_un *server, const char *sock_path)
{
int sock;
@@ -1362,7 +1364,7 @@ init_uds_sock(struct sockaddr_un *server)
}
server->sun_family = AF_UNIX;
- strlcpy(server->sun_path, UDS_SOCK, sizeof(server->sun_path));
+ strlcpy(server->sun_path, sock_path, sizeof(server->sun_path));
if (connect(sock, (struct sockaddr *)server, sizeof(struct
sockaddr_un)) < 0) {
close(sock);
@@ -1382,7 +1384,7 @@ struct msg_internal { };
static int
-send_msg(int sock, char *request, int *fd)
+send_msg(int sock, char *request, int *fd, const char *sock_path)
{
int snd;
struct iovec iov;
@@ -1393,7 +1395,7 @@ send_msg(int sock, char *request, int *fd)
memset(&dst, 0, sizeof(dst));
dst.sun_family = AF_UNIX;
- strlcpy(dst.sun_path, UDS_SOCK, sizeof(dst.sun_path));
+ strlcpy(dst.sun_path, sock_path, sizeof(dst.sun_path));
/* Initialize message header structure */
memset(&msgh, 0, sizeof(msgh));
@@ -1471,7 +1473,7 @@ read_msg(int sock, char *response, struct
sockaddr_un *s, int *fd)
static int
make_request_cni(int sock, struct sockaddr_un *server, char *request,
-int *req_fd, char *response, int *out_fd)
+int *req_fd, char *response, int *out_fd, const char
*sock_path)
{
int rval;
@@ -1483,7 +1485,7 @@ make_request_cni(int sock, struct sockaddr_un
*server, char *request,
if (req_fd == NULL)
rval = write(sock, request, strlen(request));
else
- rval = send_msg(sock, request, req_fd);
+ rval = send_msg(sock, request, req_fd, sock_path);
if (rval < 0) {
AF_XDP_LOG(ERR, "Write error %s\n", strerror(errno)); @@
-1507,7 +1509,7 @@ check_response(char *response, char *exp_resp, long
size) }
static int
-get_cni_fd(char *if_name)
+get_cni_fd(char *if_name, const char *sock_path)
{
char request[UDS_MAX_CMD_LEN],
response[UDS_MAX_CMD_RESP];
char hostname[MAX_LONG_OPT_SZ],
exp_resp[UDS_MAX_CMD_RESP]; @@ -1520,14 +1522,14 @@
get_cni_fd(char *if_name)
return -1;
memset(&server, 0, sizeof(server));
- sock = init_uds_sock(&server);
+ sock = init_uds_sock(&server, sock_path);
if (sock < 0)
return -1;
/* Initiates handshake to CNI send: /connect,hostname */
snprintf(request, sizeof(request), "%s,%s", UDS_CONNECT_MSG,
hostname);
memset(response, 0, sizeof(response));
- if (make_request_cni(sock, &server, request, NULL, response,
&out_fd) < 0) {
+ if (make_request_cni(sock, &server, request, NULL, response,
&out_fd,
+sock_path) < 0) {
Why do we need to pass "sock_path" here as we have already connected the sock
with sock_path in init_uds_sock()?
it's used in the send_msg() function insidemake_request_cni(). In the
original implementation of send_msg used a #defined value which is why
it wasn't needed before. so as far as I can tell send_msg still needs
that path. but I'm open to suggestions.
AF_XDP_LOG(ERR, "Error in processing cmd [%s]\n",
request);
goto err_close;
RE: [EXT] Re: [PATCH v5 0/1] devtools: add tracepoint check in checkpatch
>-Original Message- >From: Thomas Monjalon >Sent: Thursday, November 30, 2023 2:11 PM >To: Ankur Dwivedi >Cc: dev@dpdk.org; Jerin Jacob Kollanukkaran >Subject: Re: [EXT] Re: [PATCH v5 0/1] devtools: add tracepoint check in >checkpatch > >30/11/2023 06:56, Ankur Dwivedi: >> From: Thomas Monjalon >> > 28/11/2023 15:07, Ankur Dwivedi: >> >> > 07/03/2023 13:05, Ankur Dwivedi: >> >> >> This patch series adds a validation in checkpatch tool to check >> >> >> if tracepoint is present in any new function added in ethdev, >> >> >> eventdev cryptodev and mempool library. >> >> >> >> >> >> v5: >> >> >> - Copied the build_map_changes function from >> >> >> check-symbol-change.sh >> >to >> >> >>check-tracepoint.sh. >> >> >> - Added eventdev, cryptodev and mempool in libdir in check- >> >tracepoint.sh. >> >> > >> >> >Why did you decide to copy the function in v5, instead of having a >> >> >common file usable by different scripts? >> >> > >> >> There was comments in v2 of the patch that common scripts may not >> >> work >> >well and to keep the scripts specialized. >> > >> >I meant you can have a common file specialized in symbols. >> The build_map_changes() (in devtools/check-symbol-change.sh) which is a >common function can be moved to a new file named devtools/build-symbol- >map.sh. >> The build-symbol-map.sh can be included in check-symbol-change.sh and >check-tracepoint.sh. >> Please let me know if this is fine. > >Yes >We can imagine moving more symbol map related funtions in this new file. >What about symbol-map-util.sh as filename? I am ok with this filename. >
Re: [PATCH v2] eal/linux: force iova-mode va without pa available
On Wed, Nov 29, 2023 at 10:51 PM David Christensen
wrote:
>
>
> On 11/24/23 2:09 AM, christian.ehrha...@canonical.com wrote:
> > From: David Wilder
> >
> > When using --no-huge option physical address are not guaranteed
> > to be persistent.
> >
> > This change effectively makes "--no-huge" the same as
> > "--no-huge --iova-mode=va".
> >
> > When --no-huge is used (or any other condition making physical
> > addresses unavailable) setting --iova-mode=pa will have no effect.
> >
> > Signed-off-by: Christian Ehrhardt
> > ---
> > doc/guides/prog_guide/env_abstraction_layer.rst | 9 ++---
> > lib/eal/linux/eal.c | 16 ++--
> > 2 files changed, 16 insertions(+), 9 deletions(-)
> >
> > diff --git a/doc/guides/prog_guide/env_abstraction_layer.rst
> b/doc/guides/prog_guide/env_abstraction_layer.rst
> > index 6debf54efb..20c7355e0f 100644
> > --- a/doc/guides/prog_guide/env_abstraction_layer.rst
> > +++ b/doc/guides/prog_guide/env_abstraction_layer.rst
> > @@ -559,9 +559,12 @@ IOVA Mode is selected by considering what the
> current usable Devices on the
> > system require and/or support.
> >
> > On FreeBSD, RTE_IOVA_PA is always the default. On Linux, the IOVA mode
> is
> > -detected based on a 2-step heuristic detailed below.
> > +detected based on a heuristic detailed below.
> >
> > -For the first step, EAL asks each bus its requirement in terms of IOVA
> mode
> > +For the first step, if no Physical Addresses are available RTE_IOVA_VA
> is
> > +selected.
> > +
> > +Then EAL asks each bus its requirement in terms of IOVA mode
> > and decides on a preferred IOVA mode.
> >
> > - if all buses report RTE_IOVA_PA, then the preferred IOVA mode is
> RTE_IOVA_PA,
> > @@ -575,7 +578,7 @@ and decides on a preferred IOVA mode.
> > If the buses have expressed no preference on which IOVA mode to pick,
> then a
> > default is selected using the following logic:
> >
> > -- if physical addresses are not available, RTE_IOVA_VA mode is used
> > +- if enable_iova_as_pa was not set at build RTE_IOVA_VA mode is used
> > - if /sys/kernel/iommu_groups is not empty, RTE_IOVA_VA mode is used
> > - otherwise, RTE_IOVA_PA mode is used
> >
> > diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
> > index 57da058cec..2f1fce3c54 100644
> > --- a/lib/eal/linux/eal.c
> > +++ b/lib/eal/linux/eal.c
> > @@ -1067,6 +1067,16 @@ rte_eal_init(int argc, char **argv)
> >
> > phys_addrs = rte_eal_using_phys_addrs() != 0;
> >
> > + if (!phys_addrs) {
> > + /* if we have no access to physical addresses, pick IOVA
> as VA mode. */
> > + if (internal_conf->iova_mode == RTE_IOVA_PA)
> > + RTE_LOG(WARNING, EAL, "WARNING: --iova-mode=pa,
> but Physical addresses are unavailable, selecting IOVA as VA mode.\n");
> > + else
> > + RTE_LOG(DEBUG, EAL, "Physical addresses are
> unavailable, selecting IOVA as VA mode.\n");
> > + internal_conf->iova_mode = RTE_IOVA_VA;
> > + rte_eal_get_configuration()->iova_mode =
> internal_conf->iova_mode;
> > + }
> > +
> > /* if no EAL option "--iova-mode=", use bus IOVA scheme */
> > if (internal_conf->iova_mode == RTE_IOVA_DC) {
> > /* autodetect the IOVA mapping mode */
> > @@ -1078,12 +1088,6 @@ rte_eal_init(int argc, char **argv)
> > if (!RTE_IOVA_IN_MBUF) {
> > iova_mode = RTE_IOVA_VA;
> > RTE_LOG(DEBUG, EAL, "IOVA as VA mode is
> forced by build option.\n");
> > - } else if (!phys_addrs) {
> > - /* if we have no access to physical
> addresses,
> > - * pick IOVA as VA mode.
> > - */
> > - iova_mode = RTE_IOVA_VA;
> > - RTE_LOG(DEBUG, EAL, "Physical addresses
> are unavailable, selecting IOVA as VA mode.\n");
> > } else if (is_iommu_enabled()) {
> > /* we have an IOMMU, pick IOVA as VA mode
> */
> > iova_mode = RTE_IOVA_VA;
>
> What tests are you running that generate an error without explicitly
> selecting the iova-mode? When I run the fast-tests I see the system
> correctly selecting VA without any additional command-line parameters
> when running 23.11:
>
>
Hi, we run the tests as they are defined in debian/ubuntu autopkgtest, here
a full log:
https://autopkgtest.ubuntu.com/results/autopkgtest-noble-paelzer-dpdk-23.11-test-builds/noble/ppc64el/d/dpdk/20231123_134814_ed029@/log.gz
Fasttests, just like yours, fail for us.
But this is a virtual test env that might not be as powerful as yours.
But as I've said, consider this one withdrawn and the one just fixing the
test config preferred.
> 21:47:05 DPDK_TEST=acl_autotest MALLOC_PERTURB_=201
> /home/drc/src/dpdk/build/app/dpdk-test --no-huge -m
RE: [v1] net/af_xdp: enable a sock path alongside use_cni
Hi Maryam,
I have replied inline.
Regards,
Shibin
From: Maryam Tahhan
Sent: Thursday, November 30, 2023 12:33 PM
To: Koikkara Reeny, Shibin ;
ferruh.yi...@amd.com; step...@networkplumber.org; lihuis...@huawei.com;
fengcheng...@huawei.com; liuyongl...@huawei.com
Cc: dev@dpdk.org
Subject: Re: [v1] net/af_xdp: enable a sock path alongside use_cni
Hi Shibin
Thanks for the review. Comments inline.
BR
Maryam
On 30/11/2023 12:14, Koikkara Reeny, Shibin wrote:
Hi Maryam,
I have added some suggestion below.
Regrads,
Shibin
[snip]
NOTICE);
#define UDS_MAX_CMD_LEN 64
#define UDS_MAX_CMD_RESP 128
#define UDS_XSK_MAP_FD_MSG"/xsk_map_fd"
-#define UDS_SOCK "/tmp/afxdp.sock"
#define UDS_CONNECT_MSG "/connect"
#define UDS_HOST_OK_MSG "/host_ok"
#define UDS_HOST_NAK_MSG "/host_nak"
@@ -171,6 +170,7 @@ struct pmd_internals {
bool custom_prog_configured;
bool force_copy;
bool use_cni;
+ char sock_path[PATH_MAX];
I would recommend using variable name as "uds_path".
No issues in renaming
struct bpf_map *map;
struct rte_ether_addr eth_addr;
@@ -191,6 +191,7 @@ struct pmd_process_private {
#define ETH_AF_XDP_BUDGET_ARG "busy_budget"
#define ETH_AF_XDP_FORCE_COPY_ARG "force_copy"
#define ETH_AF_XDP_USE_CNI_ARG "use_cni"
+#define ETH_AF_XDP_SOCK_ARG"sock"
To make it clear would recommend using "sock_path" and also
ETH_AF_XDP_CNI_UDS_PATH_ARG or ETH_AF_XDP_SOCK_PATH_ARG
No issues in renaming
.
static const char * const valid_arguments[] = {
ETH_AF_XDP_IFACE_ARG,
@@ -201,6 +202,7 @@ static const char * const valid_arguments[] = {
ETH_AF_XDP_BUDGET_ARG,
ETH_AF_XDP_FORCE_COPY_ARG,
ETH_AF_XDP_USE_CNI_ARG,
+ ETH_AF_XDP_SOCK_ARG,
NULL
};
@@ -1351,7 +1353,7 @@ configure_preferred_busy_poll(struct
pkt_rx_queue *rxq) }
static int
-init_uds_sock(struct sockaddr_un *server)
+init_uds_sock(struct sockaddr_un *server, const char *sock_path)
{
int sock;
@@ -1362,7 +1364,7 @@ init_uds_sock(struct sockaddr_un *server)
}
server->sun_family = AF_UNIX;
- strlcpy(server->sun_path, UDS_SOCK, sizeof(server->sun_path));
+ strlcpy(server->sun_path, sock_path, sizeof(server->sun_path));
if (connect(sock, (struct sockaddr *)server, sizeof(struct
sockaddr_un)) < 0) {
close(sock);
@@ -1382,7 +1384,7 @@ struct msg_internal { };
static int
-send_msg(int sock, char *request, int *fd)
+send_msg(int sock, char *request, int *fd, const char *sock_path)
{
int snd;
struct iovec iov;
@@ -1393,7 +1395,7 @@ send_msg(int sock, char *request, int *fd)
memset(&dst, 0, sizeof(dst));
dst.sun_family = AF_UNIX;
- strlcpy(dst.sun_path, UDS_SOCK, sizeof(dst.sun_path));
+ strlcpy(dst.sun_path, sock_path, sizeof(dst.sun_path));
/* Initialize message header structure */
memset(&msgh, 0, sizeof(msgh));
@@ -1471,7 +1473,7 @@ read_msg(int sock, char *response, struct
sockaddr_un *s, int *fd)
static int
make_request_cni(int sock, struct sockaddr_un *server, char *request,
- int *req_fd, char *response, int *out_fd)
+ int *req_fd, char *response, int *out_fd, const char
*sock_path)
{
int rval;
@@ -1483,7 +1485,7 @@ make_request_cni(int sock, struct sockaddr_un
*server, char *request,
if (req_fd == NULL)
rval = write(sock, request, strlen(request));
else
- rval = send_msg(sock, request, req_fd);
+ rval = send_msg(sock, request, req_fd, sock_path);
if (rval < 0) {
AF_XDP_LOG(ERR, "Write error %s\n", strerror(errno)); @@
-1507,7 +1509,7 @@ check_response(char *response, char *exp_resp, long
size) }
static int
-get_cni_fd(char *if_name)
+get_cni_fd(char *if_name, const char *sock_path)
{
char request[UDS_MAX_CMD_LEN],
response[UDS_MAX_CMD_RESP];
char hostname[MAX_LONG_OPT_SZ],
exp_resp[UDS_MAX_CMD_RESP]; @@ -1520,14 +1522,14 @@
get_cni_fd(char *if_name)
return -1;
memset(&server, 0, sizeof(server));
- sock = init_uds_sock(&server);
+ sock = init_uds_sock(&server, sock_path);
if (sock < 0)
return -1;
/* Initiates handshake to CNI send: /connect,hostname */
snprintf(request, sizeof(request), "%s,%s", UDS_CONNECT_MSG,
hostname);
memset(response, 0, sizeof(response));
- if (make_request_cni(sock, &server, request, NULL, response,
&out_fd) < 0) {
+ if (make_request_cni(sock, &server, request, NULL, response,
&out_fd,
+sock_path) < 0) {
Why do we need to pass "sock_path" here as we have already connected the sock
with sock_path in init_uds_sock()?
it's used in the send_msg() function inside make_request_cni(). In the original
implementation of send_msg used a #defined value which is why it wasn't needed
before. so as far as I can tell send_
RE: [v1] net/af_xdp: enable a sock path alongside use_cni
Hi Maryam,
I have one more question.
Regards,
Shibin
> -Original Message-
> From: Koikkara Reeny, Shibin
> Sent: Thursday, November 30, 2023 12:14 PM
> To: Tahhan, Maryam ; ferruh.yi...@amd.com;
> step...@networkplumber.org; lihuis...@huawei.com;
> fengcheng...@huawei.com; liuyongl...@huawei.com
> Cc: dev@dpdk.org; Tahhan, Maryam
> Subject: RE: [v1] net/af_xdp: enable a sock path alongside use_cni
>
> Hi Maryam,
>
> I have added some suggestion below.
>
> Regrads,
> Shibin
>
> > -Original Message-
> > From: Maryam Tahhan
> > Sent: Thursday, November 30, 2023 9:14 AM
> > To: ferruh.yi...@amd.com; step...@networkplumber.org;
> > lihuis...@huawei.com; fengcheng...@huawei.com;
> liuyongl...@huawei.com
> > Cc: dev@dpdk.org; Tahhan, Maryam
> > Subject: [v1] net/af_xdp: enable a sock path alongside use_cni
> >
> > With the original 'use_cni' implementation, (using a hardcoded socket
> > rather than a configurable one), if a single pod is requesting
> > multiple net devices and these devices are from different pools, then
> > the container attempts to mount all the netdev UDSes in the pod as
> > /tmp/afxdp.sock. Which means that at best only 1 netdev will handshake
> > correctly with the AF_XDP DP. This patch addresses this by making the
> > socket parameter configurable alongside the 'use_cni' param.
> > Tested with the AF_XDP DP CNI PR 81.
> >
> > Signed-off-by: Maryam Tahhan
> > ---
> > doc/guides/howto/af_xdp_cni.rst | 18 +++---
> > drivers/net/af_xdp/rte_eth_af_xdp.c | 56
> > +++--
> > 2 files changed, 51 insertions(+), 23 deletions(-)
> >
> > diff --git a/doc/guides/howto/af_xdp_cni.rst
> > b/doc/guides/howto/af_xdp_cni.rst index a1a6d5b99c..a2d90c665d 100644
> > --- a/doc/guides/howto/af_xdp_cni.rst
> > +++ b/doc/guides/howto/af_xdp_cni.rst
> > @@ -38,9 +38,10 @@ The XSKMAP is a BPF map of AF_XDP sockets (XSK).
> > The client can then proceed with creating an AF_XDP socket and
> > inserting that socket into the XSKMAP pointed to by the descriptor.
> >
> > -The EAL vdev argument ``use_cni`` is used to indicate that the user
> > wishes
> > +The EAL vdev arguments ``use_cni`` and ``sock`` are used to indicate
> > +that the user wishes
> > to run the PMD in unprivileged mode and to receive the XSKMAP file
> > descriptor from the CNI.
> > +
> > When this flag is set,
> > the ``XSK_LIBBPF_FLAGS__INHIBIT_PROG_LOAD`` libbpf flag should be
> > used when creating the socket @@ -49,7 +50,7 @@ Instead the loading is
> > handled by the CNI.
> >
> > .. note::
> >
> > - The Unix Domain Socket file path appear in the end user is
> > "/tmp/afxdp.sock".
> > + The Unix Domain Socket file path appears to the end user at
> > "/tmp/afxdp_dp//afxdp.sock".
> >
> >
> > Prerequisites
> > @@ -224,7 +225,6 @@ Howto run dpdk-testpmd with CNI plugin:
> >capabilities:
> > add:
> > - CAP_NET_RAW
> > - - CAP_BPF
Why the CAP_BPF is removed?
> > resources:
> > requests:
> > hugepages-2Mi: 2Gi
> > @@ -245,7 +245,17 @@ Howto run dpdk-testpmd with CNI plugin:
> >
> > kubectl exec -i --container -- \
> > //dpdk-testpmd -l 0,1 --no-pci \
> > - --vdev=net_af_xdp0,use_cni=1,iface= \
> > + --vdev=net_af_xdp0,use_cni=1,iface= > name>,sock=/tmp/afxdp_dp//afxdp.sock \
> > + -- --no-mlockall --in-memory
> > +
> > +for multiple devices use:
> > +
> > + .. code-block:: console
> > +
> > + kubectl exec -i --container -- \
> > + //dpdk-testpmd -l 0-2 --no-pci \
> > + --vdev=net_af_xdp0,use_cni=1,iface= > name>,sock=/tmp/afxdp_dp//afxdp.sock \
> > + --vdev=net_af_xdp1,use_cni=1,iface= > + name>,sock=/tmp/afxdp_dp//afxdp.sock \
> > -- --no-mlockall --in-memory
> >
> > For further reference please use the `e2e`_ test case in `AF_XDP
> > Plugin for Kubernetes`_ diff --git
> > a/drivers/net/af_xdp/rte_eth_af_xdp.c
> > b/drivers/net/af_xdp/rte_eth_af_xdp.c
> > index 353c8688ec..f728dae2f9 100644
> > --- a/drivers/net/af_xdp/rte_eth_af_xdp.c
> > +++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
> > @@ -88,7 +88,6 @@ RTE_LOG_REGISTER_DEFAULT(af_xdp_logtype,
> > NOTICE);
> > #define UDS_MAX_CMD_LEN64
> > #define UDS_MAX_CMD_RESP 128
> > #define UDS_XSK_MAP_FD_MSG "/xsk_map_fd"
> > -#define UDS_SOCK "/tmp/afxdp.sock"
> > #define UDS_CONNECT_MSG"/connect"
> > #define UDS_HOST_OK_MSG"/host_ok"
> > #define UDS_HOST_NAK_MSG "/host_nak"
> > @@ -171,6 +170,7 @@ struct pmd_internals {
> > bool custom_prog_configured;
> > bool force_copy;
> > bool use_cni;
> > + char sock_path[PATH_MAX];
> I would recommend using variable name as "uds_path".
>
> > struct bpf_map *map;
> >
> > struct rte_ether_addr eth_addr;
> > @@ -191,6 +191,7 @@ struct pmd_process_private {
> > #define ETH_AF_XDP_BU
Re: [v1] net/af_xdp: enable a sock path alongside use_cni
Hi Shibin No problem. Answer below. BR Maryam On 30/11/2023 13:56, Koikkara Reeny, Shibin wrote: Hi Maryam, I have one more question. Regards, Shibin -Original Message- From: Koikkara Reeny, Shibin Sent: Thursday, November 30, 2023 12:14 PM To: Tahhan, Maryam;ferruh.yi...@amd.com; step...@networkplumber.org;lihuis...@huawei.com; fengcheng...@huawei.com;liuyongl...@huawei.com Cc:dev@dpdk.org; Tahhan, Maryam Subject: RE: [v1] net/af_xdp: enable a sock path alongside use_cni Hi Maryam, I have added some suggestion below. Regrads, Shibin [snip] Prerequisites @@ -224,7 +225,6 @@ Howto run dpdk-testpmd with CNI plugin: capabilities: add: - CAP_NET_RAW - - CAP_BPF Why the CAP_BPF is removed? Good question. It's removed because in our case CAP_BPF is only needed when we want to load or unload the XDP program on the interface inside the Pod. In our case the CNI is loading the xdp program on the interface and then we are doing a handshake to get the xskmap file descriptor and so we don't need the CAP_BPF. You will find a detailed listing of the permissions used at different stages when utilizing an XDP prog in this article https://next.redhat.com/2023/07/18/using-ebpf-in-unprivileged-pods/ I'm currently also working on enabling pinned map sharing with the af_xdp vdev eal arguments so we can integrate with bpfman for centralized BPF program lifecycle management [currently under test]. [snip]
Re: [v1] net/af_xdp: enable a sock path alongside use_cni
Hi Shibin
I will add a more detailed note re CAP_BPF in the documentation on the V2.
BR
Maryam
On 30/11/2023 13:56, Koikkara Reeny, Shibin wrote:
Hi Maryam,
I have one more question.
Regards,
Shibin
-Original Message-
From: Koikkara Reeny, Shibin
Sent: Thursday, November 30, 2023 12:14 PM
To: Tahhan, Maryam ; ferruh.yi...@amd.com;
step...@networkplumber.org; lihuis...@huawei.com;
fengcheng...@huawei.com; liuyongl...@huawei.com
Cc: dev@dpdk.org; Tahhan, Maryam
Subject: RE: [v1] net/af_xdp: enable a sock path alongside use_cni
Hi Maryam,
I have added some suggestion below.
Regrads,
Shibin
-Original Message-
From: Maryam Tahhan
Sent: Thursday, November 30, 2023 9:14 AM
To: ferruh.yi...@amd.com; step...@networkplumber.org;
lihuis...@huawei.com; fengcheng...@huawei.com;
liuyongl...@huawei.com
Cc: dev@dpdk.org; Tahhan, Maryam
Subject: [v1] net/af_xdp: enable a sock path alongside use_cni
With the original 'use_cni' implementation, (using a hardcoded socket
rather than a configurable one), if a single pod is requesting
multiple net devices and these devices are from different pools, then
the container attempts to mount all the netdev UDSes in the pod as
/tmp/afxdp.sock. Which means that at best only 1 netdev will handshake
correctly with the AF_XDP DP. This patch addresses this by making the
socket parameter configurable alongside the 'use_cni' param.
Tested with the AF_XDP DP CNI PR 81.
Signed-off-by: Maryam Tahhan
---
doc/guides/howto/af_xdp_cni.rst | 18 +++---
drivers/net/af_xdp/rte_eth_af_xdp.c | 56
+++--
2 files changed, 51 insertions(+), 23 deletions(-)
diff --git a/doc/guides/howto/af_xdp_cni.rst
b/doc/guides/howto/af_xdp_cni.rst index a1a6d5b99c..a2d90c665d 100644
--- a/doc/guides/howto/af_xdp_cni.rst
+++ b/doc/guides/howto/af_xdp_cni.rst
@@ -38,9 +38,10 @@ The XSKMAP is a BPF map of AF_XDP sockets (XSK).
The client can then proceed with creating an AF_XDP socket and
inserting that socket into the XSKMAP pointed to by the descriptor.
-The EAL vdev argument ``use_cni`` is used to indicate that the user
wishes
+The EAL vdev arguments ``use_cni`` and ``sock`` are used to indicate
+that the user wishes
to run the PMD in unprivileged mode and to receive the XSKMAP file
descriptor from the CNI.
+
When this flag is set,
the ``XSK_LIBBPF_FLAGS__INHIBIT_PROG_LOAD`` libbpf flag should be
used when creating the socket @@ -49,7 +50,7 @@ Instead the loading is
handled by the CNI.
.. note::
- The Unix Domain Socket file path appear in the end user is
"/tmp/afxdp.sock".
+ The Unix Domain Socket file path appears to the end user at
"/tmp/afxdp_dp//afxdp.sock".
Prerequisites
@@ -224,7 +225,6 @@ Howto run dpdk-testpmd with CNI plugin:
capabilities:
add:
- CAP_NET_RAW
- - CAP_BPF
Why the CAP_BPF is removed?
resources:
requests:
hugepages-2Mi: 2Gi
@@ -245,7 +245,17 @@ Howto run dpdk-testpmd with CNI plugin:
kubectl exec -i --container -- \
//dpdk-testpmd -l 0,1 --no-pci \
- --vdev=net_af_xdp0,use_cni=1,iface= \
+ --vdev=net_af_xdp0,use_cni=1,iface=,sock=/tmp/afxdp_dp//afxdp.sock \
+ -- --no-mlockall --in-memory
+
+for multiple devices use:
+
+ .. code-block:: console
+
+ kubectl exec -i --container -- \
+ //dpdk-testpmd -l 0-2 --no-pci \
+ --vdev=net_af_xdp0,use_cni=1,iface=,sock=/tmp/afxdp_dp//afxdp.sock \
+ --vdev=net_af_xdp1,use_cni=1,iface=,sock=/tmp/afxdp_dp//afxdp.sock \
-- --no-mlockall --in-memory
For further reference please use the `e2e`_ test case in `AF_XDP
Plugin for Kubernetes`_ diff --git
a/drivers/net/af_xdp/rte_eth_af_xdp.c
b/drivers/net/af_xdp/rte_eth_af_xdp.c
index 353c8688ec..f728dae2f9 100644
--- a/drivers/net/af_xdp/rte_eth_af_xdp.c
+++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
@@ -88,7 +88,6 @@ RTE_LOG_REGISTER_DEFAULT(af_xdp_logtype,
NOTICE);
#define UDS_MAX_CMD_LEN 64
#define UDS_MAX_CMD_RESP 128
#define UDS_XSK_MAP_FD_MSG"/xsk_map_fd"
-#define UDS_SOCK "/tmp/afxdp.sock"
#define UDS_CONNECT_MSG "/connect"
#define UDS_HOST_OK_MSG "/host_ok"
#define UDS_HOST_NAK_MSG "/host_nak"
@@ -171,6 +170,7 @@ struct pmd_internals {
bool custom_prog_configured;
bool force_copy;
bool use_cni;
+ char sock_path[PATH_MAX];
I would recommend using variable name as "uds_path".
struct bpf_map *map;
struct rte_ether_addr eth_addr;
@@ -191,6 +191,7 @@ struct pmd_process_private {
#define ETH_AF_XDP_BUDGET_ARG
"busy_budget"
#define ETH_AF_XDP_FORCE_COPY_ARG "force_copy"
#define ETH_AF_XDP_USE_CNI_ARG"use_cni"
+#define ETH_AF_XDP_SOCK_ARG"sock"
To make it clear would re
[PATCH v3 0/2] ethdev: add random item support
Add support for matching random value using new "rte_flow_item_random" structure. This random value is not based on the packet data/headers. Application shouldn't assume that this value is kept during the life time of the packet. v2: - Rabase. - Fix copy-paste mistake in release notes. v3: - Rabase. - Move release notes to the new release file. - Improve documentation. Michael Baum (2): ethdev: add random item support app/testpmd: add random item support app/test-pmd/cmdline_flow.c | 28 + doc/guides/nics/features/default.ini| 1 + doc/guides/prog_guide/rte_flow.rst | 13 doc/guides/rel_notes/release_24_03.rst | 4 +++ doc/guides/testpmd_app_ug/testpmd_funcs.rst | 4 +++ lib/ethdev/rte_flow.c | 1 + lib/ethdev/rte_flow.h | 35 - 7 files changed, 85 insertions(+), 1 deletion(-) -- 2.25.1
[PATCH v3 1/2] ethdev: add random item support
Add support for a new item type "RTE_FLOW_ITEM_TYPE_RANDOM".
This item enables to match on some random value as a part of flow rule.
Signed-off-by: Michael Baum
---
doc/guides/nics/features/default.ini | 1 +
doc/guides/prog_guide/rte_flow.rst | 13 ++
doc/guides/rel_notes/release_24_03.rst | 4 +++
lib/ethdev/rte_flow.c | 1 +
lib/ethdev/rte_flow.h | 35 +-
5 files changed, 53 insertions(+), 1 deletion(-)
diff --git a/doc/guides/nics/features/default.ini
b/doc/guides/nics/features/default.ini
index 806cb033ff..6d50236292 100644
--- a/doc/guides/nics/features/default.ini
+++ b/doc/guides/nics/features/default.ini
@@ -140,6 +140,7 @@ pppoe_proto_id =
ptype=
quota=
raw =
+random =
represented_port =
sctp =
tag =
diff --git a/doc/guides/prog_guide/rte_flow.rst
b/doc/guides/prog_guide/rte_flow.rst
index 627b845bfb..fd7fddb6cd 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -1573,6 +1573,19 @@ Matches the packet type as defined in rte_mbuf_ptype.
- ``packet_type``: L2/L3/L4 and tunnel information.
+Item: ``RANDOM``
+
+
+Matches a random value.
+
+The rundom number is generated by PMD,
+application can match on either exact value or range of values.
+This value is not based on the packet data/headers.
+Application shouldn't assume that this value is kept during the life time of
+the packet.
+
+- ``value``: Specific value to match.
+
Actions
~~~
diff --git a/doc/guides/rel_notes/release_24_03.rst
b/doc/guides/rel_notes/release_24_03.rst
index e9c9717706..ab91ce2b21 100644
--- a/doc/guides/rel_notes/release_24_03.rst
+++ b/doc/guides/rel_notes/release_24_03.rst
@@ -55,6 +55,10 @@ New Features
Also, make sure to start the actual text at the margin.
===
+* **Added flow matching of random value.**
+
+ Added ``RTE_FLOW_ITEM_RANDOM`` to match random value.
+
Removed Items
-
diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
index 549e329558..090b936ca9 100644
--- a/lib/ethdev/rte_flow.c
+++ b/lib/ethdev/rte_flow.c
@@ -136,6 +136,7 @@ static const struct rte_flow_desc_data rte_flow_desc_item[]
= {
sizeof(struct rte_flow_item_icmp6_nd_opt_tla_eth)),
MK_FLOW_ITEM(MARK, sizeof(struct rte_flow_item_mark)),
MK_FLOW_ITEM(META, sizeof(struct rte_flow_item_meta)),
+ MK_FLOW_ITEM(RANDOM, sizeof(struct rte_flow_item_random)),
MK_FLOW_ITEM(TAG, sizeof(struct rte_flow_item_tag)),
MK_FLOW_ITEM(GRE_KEY, sizeof(rte_be32_t)),
MK_FLOW_ITEM(GRE_OPTION, sizeof(struct rte_flow_item_gre_opt)),
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index affdc8121b..887401bb86 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -704,6 +704,19 @@ enum rte_flow_item_type {
*
*/
RTE_FLOW_ITEM_TYPE_PTYPE,
+
+ /**
+* [META]
+*
+* Matches a random value.
+*
+* This value is not based on the packet data/headers.
+* Application shouldn't assume that this value is kept during the life
+* time of the packet.
+*
+* @see struct rte_flow_item_random.
+*/
+ RTE_FLOW_ITEM_TYPE_RANDOM,
};
/**
@@ -2047,6 +2060,25 @@ static const struct rte_flow_item_ib_bth
rte_flow_item_ib_bth_mask = {
};
#endif
+/**
+ * @warning
+ * @b EXPERIMENTAL: this structure may change without prior notice
+ *
+ * RTE_FLOW_ITEM_TYPE_RANDOM
+ *
+ * Matches a random value.
+ */
+struct rte_flow_item_random {
+ uint32_t value;
+};
+
+/** Default mask for RTE_FLOW_ITEM_TYPE_RANDOM. */
+#ifndef __cplusplus
+static const struct rte_flow_item_random rte_flow_item_random_mask = {
+ .value = UINT32_MAX,
+};
+#endif
+
/**
* Matching pattern item definition.
*
@@ -3903,7 +3935,8 @@ enum rte_flow_field_id {
RTE_FLOW_FIELD_TCP_DATA_OFFSET, /**< TCP data offset. */
RTE_FLOW_FIELD_IPV4_IHL,/**< IPv4 IHL. */
RTE_FLOW_FIELD_IPV4_TOTAL_LEN, /**< IPv4 total length. */
- RTE_FLOW_FIELD_IPV6_PAYLOAD_LEN /**< IPv6 payload length. */
+ RTE_FLOW_FIELD_IPV6_PAYLOAD_LEN,/**< IPv6 payload length. */
+ RTE_FLOW_FIELD_RANDOM /**< Random value. */
};
/**
--
2.25.1
[PATCH v3 2/2] app/testpmd: add random item support
Add support for random item, usage example:
pattern random spec value 0x1 mask value 0x3 / eth / end
Flow rule with above pattern matching 25% of the traffic, it hits only
when random value suffix is "01" and miss the others ("00", "10", "11").
Signed-off-by: Michael Baum
---
app/test-pmd/cmdline_flow.c | 28 +
doc/guides/testpmd_app_ug/testpmd_funcs.rst | 4 +++
2 files changed, 32 insertions(+)
diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index ce71818705..359c187b3c 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -433,6 +433,8 @@ enum index {
ITEM_ICMP6_ND_OPT_TLA_ETH_TLA,
ITEM_META,
ITEM_META_DATA,
+ ITEM_RANDOM,
+ ITEM_RANDOM_VALUE,
ITEM_GRE_KEY,
ITEM_GRE_KEY_VALUE,
ITEM_GRE_OPTION,
@@ -956,6 +958,7 @@ static const char *const modify_field_ids[] = {
"hash_result",
"geneve_opt_type", "geneve_opt_class", "geneve_opt_data", "mpls",
"tcp_data_off", "ipv4_ihl", "ipv4_total_len", "ipv6_payload_len",
+ "random",
NULL
};
@@ -1562,6 +1565,7 @@ static const enum index next_item[] = {
ITEM_ICMP6_ND_OPT_SLA_ETH,
ITEM_ICMP6_ND_OPT_TLA_ETH,
ITEM_META,
+ ITEM_RANDOM,
ITEM_GRE_KEY,
ITEM_GRE_OPTION,
ITEM_GTP_PSC,
@@ -1873,6 +1877,12 @@ static const enum index item_meta[] = {
ZERO,
};
+static const enum index item_random[] = {
+ ITEM_RANDOM_VALUE,
+ ITEM_NEXT,
+ ZERO,
+};
+
static const enum index item_gtp_psc[] = {
ITEM_GTP_PSC_QFI,
ITEM_GTP_PSC_PDU_T,
@@ -5195,6 +5205,21 @@ static const struct token token_list[] = {
.args = ARGS(ARGS_ENTRY_MASK(struct rte_flow_item_meta,
data, "\xff\xff\xff\xff")),
},
+ [ITEM_RANDOM] = {
+ .name = "random",
+ .help = "match random value",
+ .priv = PRIV_ITEM(RANDOM, sizeof(struct rte_flow_item_random)),
+ .next = NEXT(item_random),
+ .call = parse_vc,
+ },
+ [ITEM_RANDOM_VALUE] = {
+ .name = "value",
+ .help = "random value",
+ .next = NEXT(item_random, NEXT_ENTRY(COMMON_UNSIGNED),
+item_param),
+ .args = ARGS(ARGS_ENTRY_MASK(struct rte_flow_item_random,
+value, "\xff\xff")),
+ },
[ITEM_GRE_KEY] = {
.name = "gre_key",
.help = "match GRE key",
@@ -12883,6 +12908,9 @@ flow_item_default_mask(const struct rte_flow_item *item)
case RTE_FLOW_ITEM_TYPE_META:
mask = &rte_flow_item_meta_mask;
break;
+ case RTE_FLOW_ITEM_TYPE_RANDOM:
+ mask = &rte_flow_item_random_mask;
+ break;
case RTE_FLOW_ITEM_TYPE_FUZZY:
mask = &rte_flow_item_fuzzy_mask;
break;
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 447e28e694..0cd1615d9e 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -3756,6 +3756,10 @@ This section lists supported pattern items and their
attributes, if any.
- ``data {unsigned}``: metadata value.
+- ``random``: match application specific random value.
+
+ - ``value {unsigned}``: random value.
+
- ``gtp_psc``: match GTP PDU extension header with type 0x85.
- ``pdu_type {unsigned}``: PDU type.
--
2.25.1
[PATCH v2 0/2] net/mlx5: add random item support
Add support for matching random value using the "rte_flow_item_random"
structure.
Depends-on: series-29472 ("ethdev: add random item support")
v2:
- Rebase.
- Move release notes to the new release file.
Erez Shitrit (1):
net/mlx5/hws: add support for random number match
Michael Baum (1):
net/mlx5: add random item support
doc/guides/nics/features/mlx5.ini | 1 +
doc/guides/nics/mlx5.rst | 10 +++-
doc/guides/rel_notes/release_24_03.rst | 3 +++
drivers/net/mlx5/hws/mlx5dr_definer.c | 33 ++
drivers/net/mlx5/hws/mlx5dr_definer.h | 8 ++-
drivers/net/mlx5/mlx5_flow.h | 3 +++
drivers/net/mlx5/mlx5_flow_dv.c| 5
drivers/net/mlx5/mlx5_flow_hw.c| 5
8 files changed, 66 insertions(+), 2 deletions(-)
--
2.25.1
[PATCH v2 1/2] net/mlx5/hws: add support for random number match
From: Erez Shitrit
The HW adds a random number per each hash, this value can be used for
statistic calculation over the packets, for example by setting one bit in
the mask of that filed we will get half of the traffic in the flow, and
so on with the rest of the mask.
Signed-off-by: Erez Shitrit
---
drivers/net/mlx5/hws/mlx5dr_definer.c | 33 +++
drivers/net/mlx5/hws/mlx5dr_definer.h | 8 ++-
drivers/net/mlx5/mlx5_flow.h | 3 +++
3 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/drivers/net/mlx5/hws/mlx5dr_definer.c
b/drivers/net/mlx5/hws/mlx5dr_definer.c
index 0b60479406..005733372a 100644
--- a/drivers/net/mlx5/hws/mlx5dr_definer.c
+++ b/drivers/net/mlx5/hws/mlx5dr_definer.c
@@ -182,6 +182,7 @@ struct mlx5dr_definer_conv_data {
X(SET_BE32, ipsec_sequence_number, v->hdr.seq,
rte_flow_item_esp) \
X(SET, ib_l4_udp_port, UDP_ROCEV2_PORT,
rte_flow_item_ib_bth) \
X(SET, ib_l4_opcode, v->hdr.opcode,
rte_flow_item_ib_bth) \
+ X(SET, random_number, v->value,
rte_flow_item_random) \
X(SET, ib_l4_bth_a,v->hdr.a,
rte_flow_item_ib_bth) \
/* Item set function format */
@@ -2173,6 +2174,33 @@ mlx5dr_definer_conv_item_ipv6_routing_ext(struct
mlx5dr_definer_conv_data *cd,
return 0;
}
+static int
+mlx5dr_definer_conv_item_random(struct mlx5dr_definer_conv_data *cd,
+ struct rte_flow_item *item,
+ int item_idx)
+{
+ const struct rte_flow_item_random *m = item->mask;
+ const struct rte_flow_item_random *l = item->last;
+ struct mlx5dr_definer_fc *fc;
+
+ if (!m)
+ return 0;
+
+ if (m->value != (m->value & UINT16_MAX)) {
+ DR_LOG(ERR, "Random value is 16 bits only");
+ rte_errno = EINVAL;
+ return rte_errno;
+ }
+
+ fc = &cd->fc[MLX5DR_DEFINER_FNAME_RANDOM_NUM];
+ fc->item_idx = item_idx;
+ fc->tag_set = &mlx5dr_definer_random_number_set;
+ fc->is_range = l && l->value;
+ DR_CALC_SET_HDR(fc, random_number, random_number);
+
+ return 0;
+}
+
static int
mlx5dr_definer_mt_set_fc(struct mlx5dr_match_template *mt,
struct mlx5dr_definer_fc *fc,
@@ -2224,6 +2252,7 @@ mlx5dr_definer_check_item_range_supp(struct rte_flow_item
*item)
case RTE_FLOW_ITEM_TYPE_TAG:
case RTE_FLOW_ITEM_TYPE_META:
case MLX5_RTE_FLOW_ITEM_TYPE_TAG:
+ case RTE_FLOW_ITEM_TYPE_RANDOM:
return 0;
default:
DR_LOG(ERR, "Range not supported over item type %d",
item->type);
@@ -2537,6 +2566,10 @@ mlx5dr_definer_conv_items_to_hl(struct mlx5dr_context
*ctx,
ret = mlx5dr_definer_conv_item_ptype(&cd, items, i);
item_flags |= MLX5_FLOW_ITEM_PTYPE;
break;
+ case RTE_FLOW_ITEM_TYPE_RANDOM:
+ ret = mlx5dr_definer_conv_item_random(&cd, items, i);
+ item_flags |= MLX5_FLOW_ITEM_RANDOM;
+ break;
default:
DR_LOG(ERR, "Unsupported item type %d", items->type);
rte_errno = ENOTSUP;
diff --git a/drivers/net/mlx5/hws/mlx5dr_definer.h
b/drivers/net/mlx5/hws/mlx5dr_definer.h
index 6f1c99e37a..18591ef853 100644
--- a/drivers/net/mlx5/hws/mlx5dr_definer.h
+++ b/drivers/net/mlx5/hws/mlx5dr_definer.h
@@ -150,6 +150,7 @@ enum mlx5dr_definer_fname {
MLX5DR_DEFINER_FNAME_PTYPE_TUNNEL,
MLX5DR_DEFINER_FNAME_PTYPE_FRAG_O,
MLX5DR_DEFINER_FNAME_PTYPE_FRAG_I,
+ MLX5DR_DEFINER_FNAME_RANDOM_NUM,
MLX5DR_DEFINER_FNAME_MAX,
};
@@ -407,6 +408,11 @@ struct mlx5_ifc_definer_hl_ipv4_src_dst_bits {
u8 destination_address[0x20];
};
+struct mlx5_ifc_definer_hl_random_number_bits {
+ u8 random_number[0x10];
+ u8 reserved[0x10];
+};
+
struct mlx5_ifc_definer_hl_ipv6_addr_bits {
u8 ipv6_address_127_96[0x20];
u8 ipv6_address_95_64[0x20];
@@ -516,7 +522,7 @@ struct mlx5_ifc_definer_hl_bits {
struct mlx5_ifc_definer_hl_mpls_bits mpls_inner;
u8 unsupported_config_headers_outer[0x80];
u8 unsupported_config_headers_inner[0x80];
- u8 unsupported_random_number[0x20];
+ struct mlx5_ifc_definer_hl_random_number_bits random_number;
struct mlx5_ifc_definer_hl_ipsec_bits ipsec;
struct mlx5_ifc_definer_hl_metadata_bits metadata;
u8 unsupported_utc_timestamp[0x40];
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 6dde9de688..14311eff10 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -277,6 +277,9 @@ enum mlx5_feature_name {
/* NSH ITEM */
#define MLX5_FLOW_ITEM_NSH (1ull << 53)
+/* R
[PATCH v2 2/2] net/mlx5: add random item support
Add support for random item in HWS mode. Signed-off-by: Michael Baum --- doc/guides/nics/features/mlx5.ini | 1 + doc/guides/nics/mlx5.rst | 10 +- doc/guides/rel_notes/release_24_03.rst | 3 +++ drivers/net/mlx5/mlx5_flow_dv.c| 5 + drivers/net/mlx5/mlx5_flow_hw.c| 5 + 5 files changed, 23 insertions(+), 1 deletion(-) diff --git a/doc/guides/nics/features/mlx5.ini b/doc/guides/nics/features/mlx5.ini index 0739fe9d63..6261b7d657 100644 --- a/doc/guides/nics/features/mlx5.ini +++ b/doc/guides/nics/features/mlx5.ini @@ -88,6 +88,7 @@ port_id = Y port_representor = Y ptype= Y quota= Y +random = Y tag = Y tcp = Y udp = Y diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst index 6b52fb93c5..129f7d9d24 100644 --- a/doc/guides/nics/mlx5.rst +++ b/doc/guides/nics/mlx5.rst @@ -167,7 +167,7 @@ Features - Sub-Function. - Matching on represented port. - Matching on aggregated affinity. - +- Matching on random value. Limitations --- @@ -564,6 +564,7 @@ Limitations - Modification of the MPLS header is supported only in HWS and only to copy from, the encapsulation level is always 0. - Modification of the 802.1Q Tag, VXLAN Network or GENEVE Network ID's is not supported. + - Modify field action using ``RTE_FLOW_FIELD_RANDOM`` is not supported. - Encapsulation levels are not supported, can modify outermost header fields only. - Offsets cannot skip past the boundary of a field. - If the field type is ``RTE_FLOW_FIELD_MAC_TYPE`` @@ -770,6 +771,13 @@ Limitations - In HW steering (``dv_flow_en`` = 2): - not supported on guest port. +- Match on random value: + + - Supported only with HW Steering enabled (``dv_flow_en`` = 2). + - Supported only in table with ``nb_flows=1``. + - NIC ingress flow in group 0 is not supported. + - Supports matching only 16 bits (LSB). + - During live migration to a new process set its flow engine as standby mode, the user should only program flow rules in group 0 (``fdb_def_rule_en=0``). Live migration is only supported under SWS (``dv_flow_en=1``). diff --git a/doc/guides/rel_notes/release_24_03.rst b/doc/guides/rel_notes/release_24_03.rst index ab91ce2b21..164cf74f5b 100644 --- a/doc/guides/rel_notes/release_24_03.rst +++ b/doc/guides/rel_notes/release_24_03.rst @@ -59,6 +59,9 @@ New Features Added ``RTE_FLOW_ITEM_RANDOM`` to match random value. +* **Updated NVIDIA mlx5 net driver.** + + * Added support for ``RTE_FLOW_ITEM_TYPE_RUNDOM`` flow item. Removed Items - diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c index 115d730317..67a44095d7 100644 --- a/drivers/net/mlx5/mlx5_flow_dv.c +++ b/drivers/net/mlx5/mlx5_flow_dv.c @@ -5396,6 +5396,11 @@ flow_dv_validate_action_modify_field(struct rte_eth_dev *dev, RTE_FLOW_ERROR_TYPE_ACTION, action, "modifications of the MPLS header " "is not supported"); + if (dst_data->field == RTE_FLOW_FIELD_RANDOM || + src_data->field == RTE_FLOW_FIELD_RANDOM) + return rte_flow_error_set(error, ENOTSUP, + RTE_FLOW_ERROR_TYPE_ACTION, action, + "modifications of random value is not supported"); if (dst_data->field == RTE_FLOW_FIELD_MARK || src_data->field == RTE_FLOW_FIELD_MARK) if (config->dv_xmeta_en == MLX5_XMETA_MODE_LEGACY || diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c index da873ae2e2..af4e9abd89 100644 --- a/drivers/net/mlx5/mlx5_flow_hw.c +++ b/drivers/net/mlx5/mlx5_flow_hw.c @@ -5047,6 +5047,10 @@ flow_hw_validate_action_modify_field(struct rte_eth_dev *dev, return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION, action, "modifying vlan_type is not supported"); + if (flow_hw_modify_field_is_used(action_conf, RTE_FLOW_FIELD_RANDOM)) + return rte_flow_error_set(error, EINVAL, + RTE_FLOW_ERROR_TYPE_ACTION, action, + "modifying random value is not supported"); if (flow_hw_modify_field_is_used(action_conf, RTE_FLOW_FIELD_GENEVE_VNI)) return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION, action, @@ -6807,6 +6811,7 @@ flow_hw_pattern_validate(struct rte_eth_dev *dev, case RTE_FLOW_ITEM_TYPE_FLEX: case RTE_FLOW_ITEM_TYPE_IB_BTH: case RTE_FLOW_ITEM_TYPE_PTYPE: + case RTE_FLOW_ITEM_TYPE_RANDOM: break; case RTE_FLOW_ITEM_TYPE_INTEGRITY: /* -- 2.25.1
Re: [PATCH] version: 24.03-rc0
On Thu, Nov 30, 2023 at 4:24 AM David Marchand wrote: > > What it means: > - for the https://dpdk.org/git/dpdk repository, all the branches and > tags are mirrored to https://github.com/DPDK/dpdk as it was done so > far, > - for the https://git.dpdk.org/next/dpdk-next-* repositories, only > branches named "main", "staging" or "for-*" are mirrored to > https://github.com/DPDK/dpdk with a prefix. > Thank you David for clearing some of this up on the CI testing meeting. I think the final loose end was you were wondering which branches within the next-* repos we were running from. I'll paste that below: dpdk-next-crypto: for-main dpdk-next-eventdev: for-main dpdk-next-net: main dpdk-next-net-brcm: main dpdk-next-net-intel: main dpdk-next-net-mlx: main dpdk-next-net-mrvl: for-next-net dpdk-next-virtio: main dpdk-next-baseband: for-main > One example: changes to the for-main branch of the dpdk-next-crypto > repo will be mirrored to a github branch next-crypto-for-main > > > For subtree maintainers. > > It should change nothing to you guys. > Please rebase your trees on v24.03-rc0 and push it to dpdk.org. > And then double check that the mirror happened (looking at git push > output, and checking https://github.com/DPDK/dpdk/branches). > If you hit some issue, ping me. > > > For CI. > > This is more a fyi, there will be some delay before all mirrors are > up, so for now, don't swap the next-* repositories in your script yet. > However, one action that can be taken as v24.03-rc0 is pushed is to > re-enable ABI checks against the v23.11 official tag. > > Okay, we will rebuild our container images to bake in new ABI references this week. Thanks! > > Questions? > > > -- > David Marchand > >
[PATCH v4] eal: verify mmu type for DPDK support (ppc64le)
IBM POWER systems support more than one type of memory management unit
(MMU). The Power ISA 3.0 specification, which applies to P9 and later
CPUs, defined a new Radix MMU which, among other things, allows an
anonymous memory page mapping to be converted into a hugepage mapping
at a specific address. This is a required feature in DPDK so we need
to test the MMU type when POWER systems are used and provide a more
useful error message for the user when running on an unsupported
system.
All architectures other than ppc64le unconditionally report that the
MMU is supported. When running with ppc64le on Linux, the MMU is
tested and the actual result is returned, while running with ppc64le
on FreeBSD unconditionally reports that the MMU is supported to avoid
unnecessary breakage until an actual test can be implemented for that
environment.
Bugzilla ID: 1221
Suggested-by: Thomas Monjalon
Signed-off-by: David Christensen
---
lib/eal/arm/meson.build | 1 +
lib/eal/arm/rte_mmu.c | 11 ++
lib/eal/common/eal_private.h | 7
lib/eal/freebsd/eal.c | 7
lib/eal/linux/eal.c | 7
lib/eal/loongarch/meson.build | 1 +
lib/eal/loongarch/rte_mmu.c | 11 ++
lib/eal/ppc/meson.build | 1 +
lib/eal/ppc/rte_mmu.c | 68 +++
lib/eal/riscv/meson.build | 1 +
lib/eal/riscv/rte_mmu.c | 11 ++
lib/eal/x86/meson.build | 1 +
lib/eal/x86/rte_mmu.c | 11 ++
13 files changed, 138 insertions(+)
create mode 100644 lib/eal/arm/rte_mmu.c
create mode 100644 lib/eal/loongarch/rte_mmu.c
create mode 100644 lib/eal/ppc/rte_mmu.c
create mode 100644 lib/eal/riscv/rte_mmu.c
create mode 100644 lib/eal/x86/rte_mmu.c
diff --git a/lib/eal/arm/meson.build b/lib/eal/arm/meson.build
index dca1106aaeec..6fba3d6ba7b8 100644
--- a/lib/eal/arm/meson.build
+++ b/lib/eal/arm/meson.build
@@ -7,5 +7,6 @@ sources += files(
'rte_cpuflags.c',
'rte_cycles.c',
'rte_hypervisor.c',
+'rte_mmu.c',
'rte_power_intrinsics.c',
)
diff --git a/lib/eal/arm/rte_mmu.c b/lib/eal/arm/rte_mmu.c
new file mode 100644
index ..3d40bfde386a
--- /dev/null
+++ b/lib/eal/arm/rte_mmu.c
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (C) IBM Corporation 2023
+ */
+
+#include "eal_private.h"
+
+bool
+eal_mmu_supported(void)
+{
+ return true;
+}
diff --git a/lib/eal/common/eal_private.h b/lib/eal/common/eal_private.h
index 4d2e80661023..c001c917de1d 100644
--- a/lib/eal/common/eal_private.h
+++ b/lib/eal/common/eal_private.h
@@ -93,6 +93,13 @@ int rte_eal_memzone_init(void);
*/
int rte_eal_cpu_init(void);
+/**
+ * Check for architecture supported MMU.
+ *
+ * This function is private to EAL.
+ */
+bool eal_mmu_supported(void);
+
/**
* Create memseg lists
*
diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
index 568e06e9ed91..0a6a9edc7213 100644
--- a/lib/eal/freebsd/eal.c
+++ b/lib/eal/freebsd/eal.c
@@ -597,6 +597,13 @@ rte_eal_init(int argc, char **argv)
return -1;
}
+ /* verify if DPDK supported on architecture MMU */
+ if (!eal_mmu_supported()) {
+ rte_eal_init_alert("unsupported MMU type.");
+ rte_errno = ENOTSUP;
+ return -1;
+ }
+
if (!rte_atomic_compare_exchange_strong_explicit(&run_once, &has_run, 1,
rte_memory_order_relaxed,
rte_memory_order_relaxed)) {
rte_eal_init_alert("already called initialization.");
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index 57da058cec60..b22c19ff0492 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -983,6 +983,13 @@ rte_eal_init(int argc, char **argv)
return -1;
}
+ /* verify if DPDK supported on architecture MMU */
+ if (!eal_mmu_supported()) {
+ rte_eal_init_alert("unsupported MMU type.");
+ rte_errno = ENOTSUP;
+ return -1;
+ }
+
if (!rte_atomic_compare_exchange_strong_explicit(&run_once, &has_run, 1,
rte_memory_order_relaxed,
rte_memory_order_relaxed)) {
rte_eal_init_alert("already called initialization.");
diff --git a/lib/eal/loongarch/meson.build b/lib/eal/loongarch/meson.build
index 4dcc27babb9b..3acfe6c3bd77 100644
--- a/lib/eal/loongarch/meson.build
+++ b/lib/eal/loongarch/meson.build
@@ -7,5 +7,6 @@ sources += files(
'rte_cpuflags.c',
'rte_cycles.c',
'rte_hypervisor.c',
+'rte_mmu.c',
'rte_power_intrinsics.c',
)
diff --git a/lib/eal/loongarch/rte_mmu.c b/lib/eal/loongarch/rte_mmu.c
new file mode 100644
index ..3d40bfde386a
--- /dev/null
+++ b/lib/eal/loongarch/rte_mmu.c
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (C) IBM Corporation 2023
+ */
+
+#include "eal_private.h"
+
+bool
+eal_mmu_su
Re: [PATCH v7 09/21] dts: test result docstring update
On Mon, Nov 20, 2023 at 11:33 AM Juraj Linkeš wrote: > On Thu, Nov 16, 2023 at 11:47 PM Jeremy Spewock > wrote: > > > > The only comments I had on this were a few places where I think > attribute sections should be class variables instead. I tried to mark all > of the places I saw it and it could be a difference where because of the > way they are subclassed they might do it differently but I'm unsure. > > > > On Wed, Nov 15, 2023 at 8:12 AM Juraj Linkeš > wrote: > >> > >> Format according to the Google format and PEP257, with slight > >> deviations. > >> > >> Signed-off-by: Juraj Linkeš > >> --- > >> dts/framework/test_result.py | 292 --- > >> 1 file changed, 234 insertions(+), 58 deletions(-) > >> > >> diff --git a/dts/framework/test_result.py b/dts/framework/test_result.py > >> index 603e18872c..05e210f6e7 100644 > >> --- a/dts/framework/test_result.py > >> +++ b/dts/framework/test_result.py > >> @@ -2,8 +2,25 @@ > >> # Copyright(c) 2023 PANTHEON.tech s.r.o. > >> # Copyright(c) 2023 University of New Hampshire > >> > >> -""" > >> -Generic result container and reporters > >> +r"""Record and process DTS results. > >> + > >> +The results are recorded in a hierarchical manner: > >> + > >> +* :class:`DTSResult` contains > >> +* :class:`ExecutionResult` contains > >> +* :class:`BuildTargetResult` contains > >> +* :class:`TestSuiteResult` contains > >> +* :class:`TestCaseResult` > >> + > >> +Each result may contain multiple lower level results, e.g. there are > multiple > >> +:class:`TestSuiteResult`\s in a :class:`BuildTargetResult`. > >> +The results have common parts, such as setup and teardown results, > captured in :class:`BaseResult`, > >> +which also defines some common behaviors in its methods. > >> + > >> +Each result class has its own idiosyncrasies which they implement in > overridden methods. > >> + > >> +The :option:`--output` command line argument and the > :envvar:`DTS_OUTPUT_DIR` environment > >> +variable modify the directory where the files with results will be > stored. > >> """ > >> > >> import os.path > >> @@ -26,26 +43,34 @@ > >> > >> > >> class Result(Enum): > >> -""" > >> -An Enum defining the possible states that > >> -a setup, a teardown or a test case may end up in. > >> -""" > >> +"""The possible states that a setup, a teardown or a test case may > end up in.""" > >> > >> +#: > >> PASS = auto() > >> +#: > >> FAIL = auto() > >> +#: > >> ERROR = auto() > >> +#: > >> SKIP = auto() > >> > >> def __bool__(self) -> bool: > >> +"""Only PASS is True.""" > >> return self is self.PASS > >> > >> > >> class FixtureResult(object): > >> -""" > >> -A record that stored the result of a setup or a teardown. > >> -The default is FAIL because immediately after creating the object > >> -the setup of the corresponding stage will be executed, which also > guarantees > >> -the execution of teardown. > >> +"""A record that stores the result of a setup or a teardown. > >> + > >> +FAIL is a sensible default since it prevents false positives > >> +(which could happen if the default was PASS). > >> + > >> +Preventing false positives or other false results is preferable > since a failure > >> +is mostly likely to be investigated (the other false results may > not be investigated at all). > >> + > >> +Attributes: > >> +result: The associated result. > >> +error: The error in case of a failure. > >> """ > > > > > > I think the items in the attributes section should instead be "#:" > because they are class variables. > > > > Making these class variables would make the value the same for all > instances, of which there are plenty. Why do you think these should be > class variables? > That explanation makes more sense. I guess I was thinking of class variables as anything we statically define as part of the class (i.e., like we say the class will always have a `result` and an `error` attribute), but I could have just been mistaken. Using the definition of instance variables as they can differ between instances I agree makes this comment and the other ones you touched on obsolete. > > >> > >> > >> result: Result > >> @@ -56,21 +81,32 @@ def __init__( > >> result: Result = Result.FAIL, > >> error: Exception | None = None, > >> ): > >> +"""Initialize the constructor with the fixture result and > store a possible error. > >> + > >> +Args: > >> +result: The result to store. > >> +error: The error which happened when a failure occurred. > >> +""" > >> self.result = result > >> self.error = error > >> > >> def __bool__(self) -> bool: > >> +"""A wrapper around the stored :class:`Result`.""" > >> return bool(self.result) > >> > >> > >> class Statistics(dict): > >> -""" > >> -A helper class u
Re: [PATCH v8 12/21] dts: interactive remote session docstring update
On Thu, Nov 23, 2023 at 10:14 AM Juraj Linkeš wrote: > Format according to the Google format and PEP257, with slight > deviations. > > Signed-off-by: Juraj Linkeš > --- > .../interactive_remote_session.py | 36 +++ > .../remote_session/interactive_shell.py | 99 +++ > dts/framework/remote_session/python_shell.py | 26 - > dts/framework/remote_session/testpmd_shell.py | 58 +-- > 4 files changed, 149 insertions(+), 70 deletions(-) > > diff --git a/dts/framework/remote_session/interactive_remote_session.py > b/dts/framework/remote_session/interactive_remote_session.py > index 098ded1bb0..1cc82e3377 100644 > --- a/dts/framework/remote_session/interactive_remote_session.py > +++ b/dts/framework/remote_session/interactive_remote_session.py > @@ -22,27 +22,23 @@ > class InteractiveRemoteSession: > """SSH connection dedicated to interactive applications. > > -This connection is created using paramiko and is a persistent > connection to the > -host. This class defines methods for connecting to the node and > configures this > -connection to send "keep alive" packets every 30 seconds. Because > paramiko attempts > -to use SSH keys to establish a connection first, providing a password > is optional. > -This session is utilized by InteractiveShells and cannot be > interacted with > -directly. > - > -Arguments: > -node_config: Configuration class for the node you are connecting > to. > -_logger: Desired logger for this session to use. > +The connection is created using `paramiko < > https://docs.paramiko.org/en/latest/>`_ > +and is a persistent connection to the host. This class defines the > methods for connecting > +to the node and configures the connection to send "keep alive" > packets every 30 seconds. > +Because paramiko attempts to use SSH keys to establish a connection > first, providing > +a password is optional. This session is utilized by InteractiveShells > +and cannot be interacted with directly. > > Attributes: > -hostname: Hostname that will be used to initialize a connection > to the node. > -ip: A subsection of hostname that removes the port for the > connection if there > +hostname: The hostname that will be used to initialize a > connection to the node. > +ip: A subsection of `hostname` that removes the port for the > connection if there > is one. If there is no port, this will be the same as > hostname. > -port: Port to use for the ssh connection. This will be extracted > from the > -hostname if there is a port included, otherwise it will > default to 22. > +port: Port to use for the ssh connection. This will be extracted > from `hostname` > +if there is a port included, otherwise it will default to > ``22``. > username: User to connect to the node with. > password: Password of the user connecting to the host. This will > default to an > empty string if a password is not provided. > -session: Underlying paramiko connection. > +session: The underlying paramiko connection. > > Raises: > SSHConnectionError: There is an error creating the SSH connection. > @@ -58,9 +54,15 @@ class InteractiveRemoteSession: > _node_config: NodeConfiguration > _transport: Transport | None > > -def __init__(self, node_config: NodeConfiguration, _logger: DTSLOG) > -> None: > +def __init__(self, node_config: NodeConfiguration, logger: DTSLOG) -> > None: > +"""Connect to the node during initialization. > + > +Args: > +node_config: The test run configuration of the node to > connect to. > +logger: The logger instance this session will use. > +""" > self._node_config = node_config > -self._logger = _logger > +self._logger = logger > self.hostname = node_config.hostname > self.username = node_config.user > self.password = node_config.password if node_config.password else > "" > diff --git a/dts/framework/remote_session/interactive_shell.py > b/dts/framework/remote_session/interactive_shell.py > index 4db19fb9b3..b158f963b6 100644 > --- a/dts/framework/remote_session/interactive_shell.py > +++ b/dts/framework/remote_session/interactive_shell.py > @@ -3,18 +3,20 @@ > > """Common functionality for interactive shell handling. > > -This base class, InteractiveShell, is meant to be extended by other > classes that > -contain functionality specific to that shell type. These derived classes > will often > -modify things like the prompt to expect or the arguments to pass into the > application, > -but still utilize the same method for sending a command and collecting > output. How > -this output is handled however is often application specific. If an > application needs > -elevated privileges to start it is expected that the method for gaining >
RE: [PATCH 5/7] net/nfp: fix resource leak for exit of CoreNIC firmware
> On 11/30/2023 8:52 AM, Chaoyong He wrote:
> > Fix the resource leak problem in the exit logic of CoreNIC firmware.
> >
> > Fixes: 646ea79ce481 ("net/nfp: move PF functions into its own file")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Chaoyong He
> > Reviewed-by: Long Wu
> > Reviewed-by: Peng Zhang
>
> <...>
>
> > +static int
> > +nfp_pf_secondary_uninit(struct nfp_pf_dev *pf_dev) {
> > + free(pf_dev->sym_tbl);
> > + rte_free(pf_dev);
> > +
> > + return 0;
> > +}
> > +
> > /* Reset and stop device. The device can not be restarted. */ static
> > int nfp_net_close(struct rte_eth_dev *dev) @@ -333,14 +381,25 @@
> > nfp_net_close(struct rte_eth_dev *dev)
> > struct rte_pci_device *pci_dev;
> > struct nfp_app_fw_nic *app_fw_nic;
> >
> > - if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> > - return 0;
> > -
> > hw = dev->data->dev_private;
> > pf_dev = hw->pf_dev;
> > pci_dev = RTE_ETH_DEV_TO_PCI(dev);
> > app_fw_nic = NFP_PRIV_TO_APP_FW_NIC(pf_dev->app_fw_priv);
> >
> > + /*
> > +* In secondary process, a released eth device can be found by its
> name
> > +* in shared memory.
> > +* If the state of the eth device is RTE_ETH_DEV_UNUSED, it means the
> > +* eth device has been released.
> > +*/
> > + if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
> > + if (dev->state == RTE_ETH_DEV_UNUSED)
> > + return 0;
> > +
> > + nfp_pf_secondary_uninit(pf_dev);
> > + return 0;
> > + }
> > +
> >
>
> Mostly expectation is secondary process doesn't free shared resources, but
> init and free done by primary process.
I agree.
Maybe the comment here make reader a little confused.
But the `nfp_pf_secondary_uninit()` does not free any shared resources, it only
free two memory which private to each secondary process.
> When there are multiple secondaries active, and if one of them closes the
> port,
> will system behave properly? Can you please double check above logic?
Yes, the system behave properly.
