Re: [PATCH] baseband/la12xx: fix issue with secondary process

[Maxime Coquelin]

On 7/2/24 08:09, Hemant Agrawal wrote:

The la12xx driver do not have any checks for secondary process
and it causes the system to try to initialize the driver, causing
segmentation faults.
LA12xx driver do not support multi-processing.
Return when not called from Primary process.

Fixes: f218a1f92017 ("baseband/la12xx: introduce NXP LA12xx driver")
Cc: sta...@dpdk.org

Signed-off-by: Hemant Agrawal 
---
  drivers/baseband/la12xx/bbdev_la12xx.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/drivers/baseband/la12xx/bbdev_la12xx.c 
b/drivers/baseband/la12xx/bbdev_la12xx.c
index bb754a5395..1a56e73abd 100644
--- a/drivers/baseband/la12xx/bbdev_la12xx.c
+++ b/drivers/baseband/la12xx/bbdev_la12xx.c
@@ -1084,6 +1084,9 @@ la12xx_bbdev_remove(struct rte_vdev_device *vdev)
  
  	PMD_INIT_FUNC_TRACE();
  
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)

+   return 0;
+
if (vdev == NULL)
return -EINVAL;
  


Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

[PATCH v6 0/2] fix secondary process PCI UIO resource problem

[Chaoyong He]

This patch series aims to fix some problems in secondary process PCI UIO
resource map logic.

---
v2:
* Modify as the advice from reviewer.
v3:
* Modify logic as the comments of reviewers.
v4:
* Split the commits to make it easier to understand.
v5:
* Fix the unmapping UIO resources probelm found by the reviewer.
v6:
* Drop the variable rename logic.
* Add the 'Ack-by' label.
---

Zerun Fu (2):
  bus/pci: fix secondary process PCI uio resource map problem
  bus/pci: fix secondary process save 'FD' problem

 drivers/bus/pci/linux/pci_uio.c  |  5 +++-
 drivers/bus/pci/pci_common_uio.c | 48 +++-
 2 files changed, 33 insertions(+), 20 deletions(-)

-- 
2.39.1

[PATCH v6 1/2] bus/pci: fix secondary process PCI uio resource map problem

[Chaoyong He]

From: Zerun Fu 

For the primary process, the logic loops all BARs and will skip
the map of BAR with an invalid physical address (0), also will
assign 'uio_res->nb_maps' with the real mapped BARs number. But
for the secondary process, instead of loops all BARs, the logic
using the 'uio_res->nb_map' as index. If the device uses continuous
BARs there will be no problem, whereas if it uses discrete BARs,
it will lead to mapping errors.

Fix this problem by also loops all BARs and skip the map of BAR
with an invalid physical address in secondary process.

Fixes: 9b957f378abf ("pci: merge uio functions for linux and bsd")
Cc: muk...@igel.co.jp
Cc: sta...@dpdk.org

Signed-off-by: Zerun Fu 
Reviewed-by: Chaoyong He 
Reviewed-by: Long Wu 
Reviewed-by: Peng Zhang 
Acked-by: Anatoly Burakov 
---
 drivers/bus/pci/pci_common_uio.c | 16 +---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/bus/pci/pci_common_uio.c b/drivers/bus/pci/pci_common_uio.c
index 76c661f054..f44ccdf27c 100644
--- a/drivers/bus/pci/pci_common_uio.c
+++ b/drivers/bus/pci/pci_common_uio.c
@@ -26,7 +26,7 @@ EAL_REGISTER_TAILQ(rte_uio_tailq)
 static int
 pci_uio_map_secondary(struct rte_pci_device *dev)
 {
-   int fd, i, j;
+   int fd, i = 0, j, res_idx;
struct mapped_pci_resource *uio_res;
struct mapped_pci_res_list *uio_res_list =
RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
@@ -37,7 +37,15 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
if (rte_pci_addr_cmp(&uio_res->pci_addr, &dev->addr))
continue;
 
-   for (i = 0; i != uio_res->nb_maps; i++) {
+   /* Map all BARs */
+   for (res_idx = 0; res_idx != PCI_MAX_RESOURCE; res_idx++) {
+   /* skip empty BAR */
+   if (dev->mem_resource[res_idx].phys_addr == 0)
+   continue;
+
+   if (i >= uio_res->nb_maps)
+   return -1;
+
/*
 * open devname, to mmap it
 */
@@ -71,7 +79,9 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
}
return -1;
}
-   dev->mem_resource[i].addr = mapaddr;
+   dev->mem_resource[res_idx].addr = mapaddr;
+
+   i++;
}
return 0;
}
-- 
2.39.1

[PATCH v6 2/2] bus/pci: fix secondary process save 'FD' problem

[Chaoyong He]

From: Zerun Fu 

In the previous logic the 'fd' was only saved in the primary process,
but for some devices this value is also used in the secondary logic.

For example, the call of 'rte_pci_find_ext_capability()' will fail in
the secondary process.

Fix this problem by getting and saving the value of 'fd' also in the
secondary process logic.

Fixes: 9b957f378abf ("pci: merge uio functions for linux and bsd")
Cc: muk...@igel.co.jp
Cc: sta...@dpdk.org

Signed-off-by: Zerun Fu 
Reviewed-by: Chaoyong He 
Reviewed-by: Long Wu 
Reviewed-by: Peng Zhang 
Acked-by: Anatoly Burakov 
---
 drivers/bus/pci/linux/pci_uio.c  |  5 -
 drivers/bus/pci/pci_common_uio.c | 32 
 2 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/drivers/bus/pci/linux/pci_uio.c b/drivers/bus/pci/linux/pci_uio.c
index 97d740dfe5..4afda97858 100644
--- a/drivers/bus/pci/linux/pci_uio.c
+++ b/drivers/bus/pci/linux/pci_uio.c
@@ -237,7 +237,7 @@ pci_uio_alloc_resource(struct rte_pci_device *dev,
}
snprintf(devname, sizeof(devname), "/dev/uio%u", uio_num);
 
-   /* save fd if in primary process */
+   /* save fd */
fd = open(devname, O_RDWR);
if (fd < 0) {
RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
@@ -275,6 +275,9 @@ pci_uio_alloc_resource(struct rte_pci_device *dev,
}
}
 
+   if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+   return 0;
+
/* allocate the mapping details for secondary processes*/
*uio_res = rte_zmalloc("UIO_RES", sizeof(**uio_res), 0);
if (*uio_res == NULL) {
diff --git a/drivers/bus/pci/pci_common_uio.c b/drivers/bus/pci/pci_common_uio.c
index f44ccdf27c..a06378b239 100644
--- a/drivers/bus/pci/pci_common_uio.c
+++ b/drivers/bus/pci/pci_common_uio.c
@@ -106,15 +106,15 @@ pci_uio_map_resource(struct rte_pci_device *dev)
if (rte_intr_dev_fd_set(dev->intr_handle, -1))
return -1;
 
-   /* secondary processes - use already recorded details */
-   if (rte_eal_process_type() != RTE_PROC_PRIMARY)
-   return pci_uio_map_secondary(dev);
-
/* allocate uio resource */
ret = pci_uio_alloc_resource(dev, &uio_res);
if (ret)
return ret;
 
+   /* secondary processes - use already recorded details */
+   if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+   return pci_uio_map_secondary(dev);
+
/* Map all BARs */
for (i = 0; i != PCI_MAX_RESOURCE; i++) {
/* skip empty BAR */
@@ -230,6 +230,18 @@ pci_uio_unmap_resource(struct rte_pci_device *dev)
if (uio_res == NULL)
return;
 
+   /* close fd */
+   if (rte_intr_fd_get(dev->intr_handle) >= 0)
+   close(rte_intr_fd_get(dev->intr_handle));
+   uio_cfg_fd = rte_intr_dev_fd_get(dev->intr_handle);
+   if (uio_cfg_fd >= 0) {
+   close(uio_cfg_fd);
+   rte_intr_dev_fd_set(dev->intr_handle, -1);
+   }
+
+   rte_intr_fd_set(dev->intr_handle, -1);
+   rte_intr_type_set(dev->intr_handle, RTE_INTR_HANDLE_UNKNOWN);
+
/* secondary processes - just free maps */
if (rte_eal_process_type() != RTE_PROC_PRIMARY)
return pci_uio_unmap(uio_res);
@@ -241,16 +253,4 @@ pci_uio_unmap_resource(struct rte_pci_device *dev)
 
/* free uio resource */
rte_free(uio_res);
-
-   /* close fd if in primary process */
-   if (rte_intr_fd_get(dev->intr_handle) >= 0)
-   close(rte_intr_fd_get(dev->intr_handle));
-   uio_cfg_fd = rte_intr_dev_fd_get(dev->intr_handle);
-   if (uio_cfg_fd >= 0) {
-   close(uio_cfg_fd);
-   rte_intr_dev_fd_set(dev->intr_handle, -1);
-   }
-
-   rte_intr_fd_set(dev->intr_handle, -1);
-   rte_intr_type_set(dev->intr_handle, RTE_INTR_HANDLE_UNKNOWN);
 }
-- 
2.39.1

Re: [PATCH v1] vhost: fix crash caused by accessing a freed vsocket

[Maxime Coquelin]

Hi Gongming,

On 5/10/24 09:28, Gongming Chen wrote:

Hi Maxime and Chenbo,

Do you have any suggestions for how to address this?

Looking forward to hearing from you!


Could you please have a try with latest DPDK main branch,
and if it reproduces, rebase your series on top of it.

I don't think it has been fixed, but we've done significant changes in
fdman in this release so we need a rebase anyways.

Thanks in advance,
Maxime



Thanks,
Gongming


On Apr 3, 2024, at 11:52 PM, Gongming Chen  wrote:

Hi Maxime,
Thanks for review.


On Apr 3, 2024, at 5:39 PM, Maxime Coquelin  wrote:

Hi Gongming,

It's the 9th time the patch has been sent.
I'm not sure whether there are changes between them or these are just
re-sends, but that's something to avoid.



Sorry, there's something wrong with my mailbox.
I will send a v1 version as the latest patch, but they are actually the same.


If there are differences, you should use versionning to highlight it.
If unsure, please check the contributions guidelines first.

Regarding the patch itself, I don't know if this is avoidable, but I
would prefer we do not introduce yet another lock in there.

Thanks,
Maxime



I totally agree with your.
Therefore, initially I hoped to solve this problem without introducing
new lock. However, the result was not expected.

1. The vsocket is shared between the event and reconnect threads by
transmitting the vsocket pointer. Therefore, there is no way to protect
vsocket through a simple vsocket lock.

2. The event and reconnect threads can transmit vsocket pointers to
each other, so there is no way to ensure that vsocket will not be
accessed by locking the two threads separately.

3. Therefore, on the vsocket resource, event and reconnect are in the
same critical section. Only by locking two threads at the same time
can the vsocket be ensured that it will not be accessed and can be
freed safely.

Currently, app config, event, and reconnect threads respectively have
locks corresponding to their own maintenance resources,
vhost_user.mutex, pfdset->fd_mutex, and reconn_list.mutex.

I think there is a thread-level lock missing here to protect the
critical section between threads, just like the rcu scene protection.

After app config acquires the write lock, it ensures that the event and
reconnect threads are outside the critical section.
This is to completely clean up the resources associated with vsocket
and safely free vsocket.

Therefore, considering future expansion, if there may be more
resources like vsocket, this thread lock can also be used to ensure
that resources are safely released after complete cleanup.

In this way, the threads will be clearer, and the complicated try lock
method is no longer needed.

Thanks,
Gongming

[PATCH v6] net/i40e: support FEC feature

[Zhichao Zeng]

This patch enabled querying Forward Error Correction(FEC) capabilities,
set FEC mode and get current FEC mode functions.

Signed-off-by: Zhichao Zeng 

---
v6: fix some judgments
v5: fix some judgments
v4: fix some logic
v3: optimize code details
v2: update NIC feature document
---
 doc/guides/nics/features/i40e.ini  |   1 +
 doc/guides/rel_notes/release_24_07.rst |   5 +
 drivers/net/i40e/i40e_ethdev.c | 239 +
 3 files changed, 245 insertions(+)

diff --git a/doc/guides/nics/features/i40e.ini 
b/doc/guides/nics/features/i40e.ini
index ef7514c44b..4610444ace 100644
--- a/doc/guides/nics/features/i40e.ini
+++ b/doc/guides/nics/features/i40e.ini
@@ -32,6 +32,7 @@ Traffic manager  = Y
 CRC offload  = Y
 VLAN offload = Y
 QinQ offload = P
+FEC  = Y
 L3 checksum offload  = P
 L4 checksum offload  = P
 Inner L3 checksum= P
diff --git a/doc/guides/rel_notes/release_24_07.rst 
b/doc/guides/rel_notes/release_24_07.rst
index 56c03ed6c9..9f63d77673 100644
--- a/doc/guides/rel_notes/release_24_07.rst
+++ b/doc/guides/rel_notes/release_24_07.rst
@@ -83,6 +83,11 @@ New Features
 without having to do a full handshake over a Unix Domain Socket
 with the Device Plugin.
 
+* **Updated Intel i40e driver.**
+
+  * Added support for configuring the Forward Error Correction(FEC) mode, 
querying
+  * FEC capabilities and current FEC mode from a device.
+
 * **Updated Intel ixgbe driver.**
 
   * Updated base code with E610 device family support.
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index c38515a758..f513ab1220 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -406,6 +406,10 @@ static void i40e_ethertype_filter_restore(struct i40e_pf 
*pf);
 static void i40e_tunnel_filter_restore(struct i40e_pf *pf);
 static void i40e_filter_restore(struct i40e_pf *pf);
 static void i40e_notify_all_vfs_link_status(struct rte_eth_dev *dev);
+static int i40e_fec_get_capability(struct rte_eth_dev *dev,
+   struct rte_eth_fec_capa *speed_fec_capa, unsigned int num);
+static int i40e_fec_get(struct rte_eth_dev *dev, uint32_t *fec_capa);
+static int i40e_fec_set(struct rte_eth_dev *dev, uint32_t fec_capa);
 
 static const char *const valid_keys[] = {
ETH_I40E_FLOATING_VEB_ARG,
@@ -521,6 +525,9 @@ static const struct eth_dev_ops i40e_eth_dev_ops = {
.tm_ops_get   = i40e_tm_ops_get,
.tx_done_cleanup  = i40e_tx_done_cleanup,
.get_monitor_addr = i40e_get_monitor_addr,
+   .fec_get_capability   = i40e_fec_get_capability,
+   .fec_get  = i40e_fec_get,
+   .fec_set  = i40e_fec_set,
 };
 
 /* store statistics names and its offset in stats structure */
@@ -12301,6 +12308,238 @@ i40e_cloud_filter_qinq_create(struct i40e_pf *pf)
return ret;
 }
 
+static int
+i40e_fec_get_capability(struct rte_eth_dev *dev,
+   struct rte_eth_fec_capa *speed_fec_capa, __rte_unused unsigned int num)
+{
+   struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+   if (hw->mac.type == I40E_MAC_X722 &&
+   !(hw->flags & I40E_HW_FLAG_X722_FEC_REQUEST_CAPABLE)) {
+   PMD_DRV_LOG(ERR, "Setting FEC encoding not supported by"
+" firmware. Please update the NVM image.\n");
+   return -ENOTSUP;
+   }
+
+   if (hw->device_id == I40E_DEV_ID_25G_SFP28 ||
+   hw->device_id == I40E_DEV_ID_25G_B) {
+   if (speed_fec_capa) {
+   speed_fec_capa->speed = RTE_ETH_SPEED_NUM_25G;
+   speed_fec_capa->capa = 
RTE_ETH_FEC_MODE_CAPA_MASK(NOFEC) |
+RTE_ETH_FEC_MODE_CAPA_MASK(BASER) |
+RTE_ETH_FEC_MODE_CAPA_MASK(AUTO) |
+RTE_ETH_FEC_MODE_CAPA_MASK(RS);
+   }
+
+   /* since HW only supports 25G */
+   return 1;
+   } else if (hw->device_id == I40E_DEV_ID_KX_X722) {
+   if (speed_fec_capa) {
+   speed_fec_capa->speed = RTE_ETH_SPEED_NUM_25G;
+   speed_fec_capa->capa = RTE_ETH_FEC_MODE_CAPA_MASK(AUTO) 
|
+RTE_ETH_FEC_MODE_CAPA_MASK(RS);
+   }
+   return 1;
+   }
+
+   return -ENOTSUP;
+}
+
+static int
+i40e_fec_get(struct rte_eth_dev *dev, uint32_t *fec_capa)
+{
+   struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+   struct i40e_aq_get_phy_abilities_resp abilities = {0};
+   struct i40e_link_status link_status = {0};
+   uint8_t current_fec_mode = 0, fec_config = 0;
+   bool link_up, enable_lse;
+   int ret = 0;
+
+   enable_lse = dev->data->dev_conf.intr_conf.lsc ? true : false;
+   /* Get link info */
+   

[PATCH v2] net/ice: support FEC feature

[Mingjin Ye]

This patch enable three Forward Error Correction(FEC) related ops
in ice driver. As no speed information can get from HW, this patch
only show FEC capability.

Signed-off-by: Qiming Yang 
Signed-off-by: Mingjin Ye 
---
v2: fix some logic
---
 doc/guides/nics/features/ice.ini |   1 +
 doc/guides/nics/ice.rst  |   5 +
 drivers/net/ice/ice_ethdev.c | 292 +++
 3 files changed, 298 insertions(+)

diff --git a/doc/guides/nics/features/ice.ini b/doc/guides/nics/features/ice.ini
index 62869ef0a0..9c8569740a 100644
--- a/doc/guides/nics/features/ice.ini
+++ b/doc/guides/nics/features/ice.ini
@@ -11,6 +11,7 @@ Speed capabilities   = Y
 Link speed configuration = Y
 Link status  = Y
 Link status event= Y
+FEC  = Y
 Rx interrupt = Y
 Fast mbuf free   = P
 Queue start/stop = Y
diff --git a/doc/guides/nics/ice.rst b/doc/guides/nics/ice.rst
index 3deeea9e6c..3d7e4ed7f1 100644
--- a/doc/guides/nics/ice.rst
+++ b/doc/guides/nics/ice.rst
@@ -323,6 +323,11 @@ The DCF PMD needs to advertise and acquire DCF capability 
which allows DCF to
 send AdminQ commands that it would like to execute over to the PF and receive
 responses for the same from PF.
 
+Forward Error Correction (FEC)
+
+
+Supports get/set FEC mode and get FEC capability.
+
 Generic Flow Support
 
 
diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index 194109b0f6..3caacfa48a 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -181,6 +181,10 @@ static int ice_timesync_read_time(struct rte_eth_dev *dev,
 static int ice_timesync_write_time(struct rte_eth_dev *dev,
   const struct timespec *timestamp);
 static int ice_timesync_disable(struct rte_eth_dev *dev);
+static int ice_fec_get_capability(struct rte_eth_dev *dev, struct 
rte_eth_fec_capa *speed_fec_capa,
+  unsigned int num);
+static int ice_fec_get(struct rte_eth_dev *dev, uint32_t *fec_capa);
+static int ice_fec_set(struct rte_eth_dev *dev, uint32_t fec_capa);
 static const uint32_t *ice_buffer_split_supported_hdr_ptypes_get(struct 
rte_eth_dev *dev,
size_t *no_of_elements);
 
@@ -298,6 +302,9 @@ static const struct eth_dev_ops ice_eth_dev_ops = {
.timesync_write_time  = ice_timesync_write_time,
.timesync_disable = ice_timesync_disable,
.tm_ops_get   = ice_tm_ops_get,
+   .fec_get_capability   = ice_fec_get_capability,
+   .fec_get  = ice_fec_get,
+   .fec_set  = ice_fec_set,
.buffer_split_supported_hdr_ptypes_get = 
ice_buffer_split_supported_hdr_ptypes_get,
 };
 
@@ -6677,6 +6684,291 @@ ice_buffer_split_supported_hdr_ptypes_get(struct 
rte_eth_dev *dev __rte_unused,
return ptypes;
 }
 
+static unsigned int
+ice_fec_get_capa_num(struct ice_aqc_get_phy_caps_data *pcaps,
+  struct rte_eth_fec_capa *speed_fec_capa)
+{
+   unsigned int num = 0;
+   int auto_fec = (pcaps->caps & ICE_AQC_PHY_EN_AUTO_FEC) ?
+   RTE_ETH_FEC_MODE_CAPA_MASK(AUTO) : 0;
+   int link_nofec = (pcaps->link_fec_options & ICE_AQC_PHY_FEC_DIS) ?
+   RTE_ETH_FEC_MODE_CAPA_MASK(NOFEC) : 0;
+
+   if (pcaps->eee_cap & ICE_AQC_PHY_EEE_EN_100BASE_TX) {
+   if (speed_fec_capa) {
+   speed_fec_capa[num].speed = RTE_ETH_SPEED_NUM_100M;
+   speed_fec_capa[num].capa = 
RTE_ETH_FEC_MODE_CAPA_MASK(NOFEC);
+   }
+   num++;
+   }
+
+   if (pcaps->eee_cap & (ICE_AQC_PHY_EEE_EN_1000BASE_T |
+   ICE_AQC_PHY_EEE_EN_1000BASE_KX)) {
+   if (speed_fec_capa) {
+   speed_fec_capa[num].speed = RTE_ETH_SPEED_NUM_1G;
+   speed_fec_capa[num].capa = 
RTE_ETH_FEC_MODE_CAPA_MASK(NOFEC);
+   }
+   num++;
+   }
+
+   if (pcaps->eee_cap & (ICE_AQC_PHY_EEE_EN_10GBASE_T |
+   ICE_AQC_PHY_EEE_EN_10GBASE_KR)) {
+   if (speed_fec_capa) {
+   speed_fec_capa[num].speed = RTE_ETH_SPEED_NUM_10G;
+   speed_fec_capa[num].capa = auto_fec | link_nofec;
+
+   if (pcaps->link_fec_options & 
ICE_AQC_PHY_FEC_10G_KR_40G_KR4_EN)
+   speed_fec_capa[num].capa |= 
RTE_ETH_FEC_MODE_CAPA_MASK(BASER);
+   }
+   num++;
+   }
+
+   if (pcaps->eee_cap & ICE_AQC_PHY_EEE_EN_25GBASE_KR) {
+   if (speed_fec_capa) {
+   speed_fec_capa[num].speed = RTE_ETH_SPEED_NUM_25G;
+   speed_fec_capa[num].capa = auto_fec | link_nofec;
+
+   if (pcaps->link_fec_options & 
ICE_AQC_PHY_FEC_25G_KR_CLAUSE74_EN)
+   speed_fec_

Re: [PATCH v2 2/2] eal: add Arm WFET in power management intrinsics

[Thomas Monjalon]

01/07/2024 23:34, Wathsala Wathawana Vithanage:
> > 19/06/2024 08:45, Wathsala Vithanage:
> > >  rte_cpu_get_intrinsics_support(struct rte_cpu_intrinsics *intrinsics)
> > > {
> > >   memset(intrinsics, 0, sizeof(*intrinsics)); -#ifdef RTE_ARM_USE_WFE
> > >   intrinsics->power_monitor = 1;
> > > -#endif
> > 
> > Why removing this #ifdef?
> 
> WFE is available in all the Arm platforms DPDK currently supports. Therefore, 
> an explicit flag is not
> required at build time. 
> 
> The purpose of RTE_ARM_USE_WFE seems to be controlling the use of WFE in 
> rte_wait_until_equal
> functions by defining RTE_WAIT_UNTIL_EQUAL_ARCH_DEFINED macro  only when 
> RTE_ARM_USE_WFE
> is defined. (eal/arm/include/rte_pause_64.h:15)
> From what I'm hearing this was done due to a performance issue 
> rte_wait_until_equal_X functions had
> when using WFE.
> However, that design is flawed because it made other users of WFE dependent 
> on the choice of
> the use of WFE in rte_wait_until_equal functions as __RTE_ARM_WFE was wrapped 
> in an
> RTE_WAIT_UNTIL_EQUAL_ARCH_DEFINED #ifdef block.
> This patch fixes this issue by moving __RTE_ARM_WFE out of 
> RTE_WAIT_UNTIL_EQUAL_ARCH_DEFINED
> block.
> 
> Perhaps we should change RTE_ARM_USE_WFE to something like 
> RTE_ARM_USE_WFE_IN_WAIT_UNTIL_EQUAL ?

Yes perhaps.
And more importantly, you should do such change in a separate patch.
Don't hide it in WFET patch.

> > > +uint8_t wfet_en;
> > 
> > It should be made static probably.
> > This variable will be unused in some cases, needs #ifdef.
> > 
> 
> This variable is used in all cases. It's 1 when WFET is available, 0 when 
> it's not.

It would be 0 if you make it static yes.

> > > +
> > > +RTE_INIT(rte_power_intrinsics_init)
> > > +{
> > > +#ifdef RTE_ARCH_64
> > > + if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_WFXT))
> > > + wfet_en = 1;
> > > +#endif
> > > +}
> > > +
> > > +/**
> > > + * This function uses WFE/WFET instruction to make lcore suspend
> > >   * execution on ARM.
> > > - * Note that timestamp based timeout is not supported yet.
> > >   */
> > >  int
> > >  rte_power_monitor(const struct rte_power_monitor_cond *pmc,
> > >   const uint64_t tsc_timestamp)
> > >  {
> > > - RTE_SET_USED(tsc_timestamp);
> > > -
> > > -#ifdef RTE_ARM_USE_WFE
> > > +#ifdef RTE_ARCH_64
> > 
> > It looks wrong.
> > If RTE_ARM_USE_WFE is disabled, you should not call __RTE_ARM_WFE().
> > 
> 
> RTE_ARM_USE_WFE should be renamed to reflect its actual use. It's safe to 
> assume that
> WFE is available universally in Arm DPDK context.

[PATCH v6] net/i40e: support FEC feature

[Zhichao Zeng]

This patch enabled querying Forward Error Correction(FEC) capabilities,
set FEC mode and get current FEC mode functions.

Signed-off-by: Zhichao Zeng 

---
v6: fix some judgments
v5: fix some judgments
v4: fix some logic
v3: optimize code details
v2: update NIC feature document
---
 doc/guides/nics/features/i40e.ini  |   1 +
 doc/guides/rel_notes/release_24_07.rst |   5 +
 drivers/net/i40e/i40e_ethdev.c | 239 +
 3 files changed, 245 insertions(+)

diff --git a/doc/guides/nics/features/i40e.ini 
b/doc/guides/nics/features/i40e.ini
index ef7514c44b..4610444ace 100644
--- a/doc/guides/nics/features/i40e.ini
+++ b/doc/guides/nics/features/i40e.ini
@@ -32,6 +32,7 @@ Traffic manager  = Y
 CRC offload  = Y
 VLAN offload = Y
 QinQ offload = P
+FEC  = Y
 L3 checksum offload  = P
 L4 checksum offload  = P
 Inner L3 checksum= P
diff --git a/doc/guides/rel_notes/release_24_07.rst 
b/doc/guides/rel_notes/release_24_07.rst
index 56c03ed6c9..9f63d77673 100644
--- a/doc/guides/rel_notes/release_24_07.rst
+++ b/doc/guides/rel_notes/release_24_07.rst
@@ -83,6 +83,11 @@ New Features
 without having to do a full handshake over a Unix Domain Socket
 with the Device Plugin.
 
+* **Updated Intel i40e driver.**
+
+  * Added support for configuring the Forward Error Correction(FEC) mode, 
querying
+  * FEC capabilities and current FEC mode from a device.
+
 * **Updated Intel ixgbe driver.**
 
   * Updated base code with E610 device family support.
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index c38515a758..f847bf82bc 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -406,6 +406,10 @@ static void i40e_ethertype_filter_restore(struct i40e_pf 
*pf);
 static void i40e_tunnel_filter_restore(struct i40e_pf *pf);
 static void i40e_filter_restore(struct i40e_pf *pf);
 static void i40e_notify_all_vfs_link_status(struct rte_eth_dev *dev);
+static int i40e_fec_get_capability(struct rte_eth_dev *dev,
+   struct rte_eth_fec_capa *speed_fec_capa, unsigned int num);
+static int i40e_fec_get(struct rte_eth_dev *dev, uint32_t *fec_capa);
+static int i40e_fec_set(struct rte_eth_dev *dev, uint32_t fec_capa);
 
 static const char *const valid_keys[] = {
ETH_I40E_FLOATING_VEB_ARG,
@@ -521,6 +525,9 @@ static const struct eth_dev_ops i40e_eth_dev_ops = {
.tm_ops_get   = i40e_tm_ops_get,
.tx_done_cleanup  = i40e_tx_done_cleanup,
.get_monitor_addr = i40e_get_monitor_addr,
+   .fec_get_capability   = i40e_fec_get_capability,
+   .fec_get  = i40e_fec_get,
+   .fec_set  = i40e_fec_set,
 };
 
 /* store statistics names and its offset in stats structure */
@@ -12301,6 +12308,238 @@ i40e_cloud_filter_qinq_create(struct i40e_pf *pf)
return ret;
 }
 
+static int
+i40e_fec_get_capability(struct rte_eth_dev *dev,
+   struct rte_eth_fec_capa *speed_fec_capa, __rte_unused unsigned int num)
+{
+   struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+   if (hw->mac.type == I40E_MAC_X722 &&
+   !(hw->flags & I40E_HW_FLAG_X722_FEC_REQUEST_CAPABLE)) {
+   PMD_DRV_LOG(ERR, "Setting FEC encoding not supported by"
+" firmware. Please update the NVM image.\n");
+   return -ENOTSUP;
+   }
+
+   if (hw->device_id == I40E_DEV_ID_25G_SFP28 ||
+   hw->device_id == I40E_DEV_ID_25G_B) {
+   if (speed_fec_capa) {
+   speed_fec_capa->speed = RTE_ETH_SPEED_NUM_25G;
+   speed_fec_capa->capa = 
RTE_ETH_FEC_MODE_CAPA_MASK(NOFEC) |
+RTE_ETH_FEC_MODE_CAPA_MASK(BASER) |
+RTE_ETH_FEC_MODE_CAPA_MASK(AUTO) |
+RTE_ETH_FEC_MODE_CAPA_MASK(RS);
+   }
+
+   /* since HW only supports 25G */
+   return 1;
+   } else if (hw->device_id == I40E_DEV_ID_KX_X722) {
+   if (speed_fec_capa) {
+   speed_fec_capa->speed = RTE_ETH_SPEED_NUM_25G;
+   speed_fec_capa->capa = RTE_ETH_FEC_MODE_CAPA_MASK(AUTO) 
|
+RTE_ETH_FEC_MODE_CAPA_MASK(RS);
+   }
+   return 1;
+   }
+
+   return -ENOTSUP;
+}
+
+static int
+i40e_fec_get(struct rte_eth_dev *dev, uint32_t *fec_capa)
+{
+   struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+   struct i40e_aq_get_phy_abilities_resp abilities = {0};
+   struct i40e_link_status link_status = {0};
+   uint8_t current_fec_mode = 0, fec_config = 0;
+   bool link_up, enable_lse;
+   int ret = 0;
+
+   enable_lse = dev->data->dev_conf.intr_conf.lsc ? true : false;
+   /* Get link info */
+   

[PATCH] driver: crypto: scheduler: fix session size computation

[jhascoet]

The crypto scheduler session size computation was taking
into account only the worker session sizes and not its own.

Fixes: e2af4e403c1 ("crypto/scheduler: support DOCSIS security protocol")
Cc: sta...@dpdk.org

Signed-off-by: Julien Hascoet 
---
 drivers/crypto/scheduler/scheduler_pmd_ops.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/scheduler/scheduler_pmd_ops.c 
b/drivers/crypto/scheduler/scheduler_pmd_ops.c
index a18f7a08b0..6e43438469 100644
--- a/drivers/crypto/scheduler/scheduler_pmd_ops.c
+++ b/drivers/crypto/scheduler/scheduler_pmd_ops.c
@@ -185,7 +185,7 @@ scheduler_session_size_get(struct scheduler_ctx *sched_ctx,
uint8_t session_type)
 {
uint8_t i = 0;
-   uint32_t max_priv_sess_size = 0;
+   uint32_t max_priv_sess_size = sizeof(struct scheduler_session_ctx);
 
/* Check what is the maximum private session size for all workers */
for (i = 0; i < sched_ctx->nb_workers; i++) {
-- 
2.34.1

Re: [PATCH] zsda:introduce zsda drivers and examples

[David Marchand]

Hello,

Thanks for the submission.

On Tue, Jul 2, 2024 at 10:42 AM lhx  wrote:
>
> From: = Hanxiao Li 

There is an extra = sign here, please remove.

>
> Introduce zsda (ZTE Storage Data Accelerator) drivers and examples,
> which can help to accelerate some calculation process.

It looks like this is an upstreaming effort for code written against
DPDK v21.11.
There are unrelated / broken changes (probably due to some rebase issue).

Please fix at least what I quickly noticed:
- no version.map if unneeded,
- no touching of rte_config.h like disabling RTE_BACKTRACE,
- don't reintroduce renamed headers, like for example rte_cryptodev_pmd.h,
- don't remove "dma" device support in usertools/dpdk-devbind.py,
- don't introduce a hardware specific example,

Important: overall, this patch should be split in logical smaller
patches introducing feature one at a time.
And for next revisions, please version your patches, and register to
the dev@ mailing list and patchwork.


-- 
David Marchand

Re: [PATCH] net/netvsc: fix mtu_set in netvsc devices

[Alexander Skorichenko]

Hello,
The failure happens when running under VPP. In terms of dpdk calls the
following sequence of events occurs for a NetVSC device:

Durung device probing stage of VPP
   1. eth_hn_dev_init() memory for a single primary rx_queue is allocated
dev->data->rx_queues[0] = 0, allocated, but not set yet
no allocation happened for tx_queues[i] and non-primary
rx_queues[i]

During device setup stage of VPP from VPP's own generic dpdk_device_setup():
   2.  rte_eth_dev_set_mtu()
currently it segfaults in hn_reinit() when trying to reach into
dev->data->rx_queues[i], dev->data->tx_queues[i]
   3.  rte_eth_tx_queue_setup()
dev->data->tx_queues[i] are being allocated and set
   4.  rte_eth_rx_queue_setup()
dev->data->rx_queues[i] get allocated (i > 0) and set

So rx_queues[0] could be set in step 1, but rx_queues[i] and tx_queues[i]
are still NULL.
Allocating all the remaining rx/tx queues in step 1 would prevent the
crash, but then in steps 3-4 would go through releasing and allocating all
of the queues again.

Another comment regarding the uniform condition introduced in hn_reinit()
in the patch:
if (dev->data->rx_queues[0] != NULL) ...
Because of the difference between rx/tx queues described above, it's
probably safer to extend the condition to check both rx and tx separately
if (dev->data->rx_queues[0] != NULL && dev->data->tx_queues[0] !=
NULL)

- Alexander Skorichenko


On Sun, Jun 30, 2024 at 5:40 PM Stephen Hemminger <
step...@networkplumber.org> wrote:

> On Fri, 28 Jun 2024 18:35:03 +0200
> Alexander Skorichenko  wrote:
>
> > Prevent segfault in hn_reinit() caused by changing the MTU for
> > an incompletely initialized device.
> >
> > Signed-off-by: Alexander Skorichenko 
>
> How do you get in that state?
> Maybe the init code should set up these pointers and avoid the problem.
>

Re: [PATCH] net/mlx5: increase max pattern templates

[Raslan Darawsheh]

Hi,

From: Gregory Etelson 
Sent: Monday, July 1, 2024 1:26 PM
To: dev@dpdk.org
Cc: Gregory Etelson; Maayan Kashani; Raslan Darawsheh; Ori Kam; Dariusz 
Sosnowski; Slava Ovsiienko; Suanming Mou; Matan Azrad
Subject: [PATCH] net/mlx5: increase max pattern templates

From: Ori Kam 

Until now the number of pattern templates that was
supported per table was limited to 2.
This was the result of the limitation that the table
could only support 1 matcher.
which meant that we could only support merge of
Ipv4 + TCP and IPv4 + UDP.

With the added ability to use extended hash it is now
possible to use more than 2 pattern templates in a
single table.

Extended match works by creating the hash of the rule
based on the intersection of all pattern templates.
As a result this is good for tables with small number
of rules or that the intersection is very large. for
example ACL table.

Using this feature is not recommended to for use
in tables with large number of rules or with small
intersection. Using this feature on such cases may
result in PPS degradation and rule insertion failures.

This patch increase the max number to 32.

Signed-off-by: Ori Kam 
Acked-by: Dariusz Sosnowski 


Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh

Re: [PATCH 0/2] Added hairpin out of buffer counters

[Raslan Darawsheh]

Hi,

From: Shani Peretz 
Sent: Monday, July 1, 2024 9:12 PM
To: dev@dpdk.org
Cc: Shani Peretz; Maayan Kashani; Raslan Darawsheh; Dariusz Sosnowski
Subject: [PATCH 0/2] Added hairpin out of buffer counters

Added global hairpin out of buffer counter (from ethtool)
Also added port-level hairpin out of buffer counter.

Shani Peretz (2):
  net/mlx5: add global hairpin out of buffer counter
  net/mlx5: add hairpin out of buffer counter

 doc/guides/nics/mlx5.rst|  3 ++
 doc/guides/rel_notes/release_24_07.rst  |  2 +
 drivers/net/mlx5/linux/mlx5_ethdev_os.c |  9 +
 drivers/net/mlx5/linux/mlx5_os.c| 14 ++-
 drivers/net/mlx5/mlx5.c |  4 ++
 drivers/net/mlx5/mlx5.h |  4 ++
 drivers/net/mlx5/mlx5_devx.c| 54 -
 drivers/net/mlx5/windows/mlx5_os.c  |  1 +
 8 files changed, 89 insertions(+), 2 deletions(-)


--
2.34.1


series applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh

Re: [PATCH v6 0/2] fix secondary process PCI UIO resource problem

[David Marchand]

On Tue, Jul 2, 2024 at 9:40 AM Chaoyong He  wrote:
>
> This patch series aims to fix some problems in secondary process PCI UIO
> resource map logic.

Chaoyong, this series touches the bus/pci driver.
Please Cc: its maintainers.


Chenbo, Nipun,
Please review.


-- 
David Marchand

Re: [PATCH 2/2] drivers: replace printf with fprintf for debug functions

[David Marchand]

Hello Hemant,

On Thu, Feb 16, 2023 at 10:28 AM Hemant Agrawal
 wrote:
> On 15-Feb-23 8:46 PM, Thomas Monjalon wrote:
> > 15/02/2023 11:29, Hemant Agrawal:
> >> This patch replaces simple printf with fprintf for debug dump
> >> related functions for various NXP dpaaX related drivers.
> > Why replacing with fprintf(stdout)?
> >
> > Would it be better to provide a FILE* parameter to the functions?
> yes, I will update.

Any news for this series?
At least a rebase is needed, I marked it as changes requested.


-- 
David Marchand

Re: [PATCH 2/2] drivers: replace printf with fprintf for debug functions

[Hemant Agrawal]

On 02-07-2024 15:01, David Marchand wrote:

Hello Hemant,

On Thu, Feb 16, 2023 at 10:28 AM Hemant Agrawal
 wrote:

On 15-Feb-23 8:46 PM, Thomas Monjalon wrote:

15/02/2023 11:29, Hemant Agrawal:

This patch replaces simple printf with fprintf for debug dump
related functions for various NXP dpaaX related drivers.

Why replacing with fprintf(stdout)?

Would it be better to provide a FILE* parameter to the functions?

yes, I will update.

Any news for this series?
At least a rebase is needed, I marked it as changes requested.


I missed it. i will send it.

[PATCH v2 1/2] drivers: replace printf with log macros

[Hemant Agrawal]

This patch replaces the printf with related log macros and functions at
various places in NXP dpaaX drivers.

Signed-off-by: Hemant Agrawal 
---
 drivers/bus/dpaa/base/fman/fman.c  |  8 +++
 drivers/bus/dpaa/base/qbman/process.c  | 29 +-
 drivers/bus/dpaa/base/qbman/qman.c |  2 +-
 drivers/bus/fslmc/fslmc_vfio.c |  2 +-
 drivers/bus/fslmc/qbman/qbman_portal.c |  2 +-
 drivers/crypto/caam_jr/caam_jr.c   |  2 +-
 drivers/crypto/caam_jr/caam_jr_uio.c   |  2 +-
 drivers/net/dpaa/dpaa_ethdev.c |  6 +++---
 drivers/net/dpaa/dpaa_flow.c   |  8 +++
 drivers/net/dpaa/dpaa_rxtx.c   |  2 +-
 drivers/net/dpaa2/dpaa2_ethdev.c   |  2 +-
 drivers/net/dpaa2/dpaa2_tm.c   | 10 -
 12 files changed, 37 insertions(+), 38 deletions(-)

diff --git a/drivers/bus/dpaa/base/fman/fman.c 
b/drivers/bus/dpaa/base/fman/fman.c
index 1814372a40..2fc1e64f36 100644
--- a/drivers/bus/dpaa/base/fman/fman.c
+++ b/drivers/bus/dpaa/base/fman/fman.c
@@ -425,7 +425,7 @@ fman_if_init(const struct device_node *dpa_node)
char_prop = of_get_property(mac_node, "phy-connection-type",
NULL);
if (!char_prop) {
-   printf("memac: unknown MII type assuming 1G\n");
+   FMAN_ERR(-EINVAL, "memac: unknown MII type assuming 
1G\n");
/* Right now forcing memac to 1g in case of error*/
__if->__if.mac_type = fman_mac_1g;
} else {
@@ -723,10 +723,8 @@ fman_finish(void)
/* release the mapping */
_errno = munmap(__if->ccsr_map, __if->regs_size);
if (unlikely(_errno < 0))
-   fprintf(stderr, "%s:%d:%s(): munmap() = %d (%s)\n",
-   __FILE__, __LINE__, __func__,
-   -errno, strerror(errno));
-   printf("Tearing down %s\n", __if->node_path);
+   FMAN_ERR(_errno, "munmap() = (%s)\n", strerror(errno));
+   DPAA_BUS_INFO("Tearing down %s\n", __if->node_path);
list_del(&__if->__if.node);
rte_free(__if);
}
diff --git a/drivers/bus/dpaa/base/qbman/process.c 
b/drivers/bus/dpaa/base/qbman/process.c
index 3504ec97db..af1e459641 100644
--- a/drivers/bus/dpaa/base/qbman/process.c
+++ b/drivers/bus/dpaa/base/qbman/process.c
@@ -13,6 +13,7 @@
 #include "process.h"
 
 #include 
+#include "rte_dpaa_logs.h"
 
 /* As higher-level drivers will be built on top of this (dma_mem, qbman, ...),
  * it's preferable that the process driver itself not provide any exported API.
@@ -99,12 +100,12 @@ void process_release(enum dpaa_id_type id_type, uint32_t 
base, uint32_t num)
int ret = check_fd();
 
if (ret) {
-   fprintf(stderr, "Process FD failure\n");
+   DPAA_BUS_ERR("Process FD failure\n");
return;
}
ret = ioctl(fd, DPAA_IOCTL_ID_RELEASE, &id);
if (ret)
-   fprintf(stderr, "Process FD ioctl failure type %d base 0x%x num 
%d\n",
+   DPAA_BUS_ERR("Process FD ioctl failure type %d base 0x%x num 
%d\n",
id_type, base, num);
 }
 
@@ -333,9 +334,9 @@ int dpaa_intr_disable(char *if_name)
ret = ioctl(fd, DPAA_IOCTL_DISABLE_LINK_STATUS_INTERRUPT, if_name);
if (ret) {
if (errno == EINVAL)
-   printf("Failed to disable interrupt: Not Supported\n");
+   DPAA_BUS_ERR("Failed to disable interrupt: Not 
Supported\n");
else
-   printf("Failed to disable interrupt\n");
+   DPAA_BUS_ERR("Failed to disable interrupt\n");
return ret;
}
 
@@ -357,7 +358,7 @@ int dpaa_get_ioctl_version_number(void)
if (errno == EINVAL) {
version_num = 1;
} else {
-   printf("Failed to get ioctl version number\n");
+   DPAA_BUS_ERR("Failed to get ioctl version number\n");
version_num = -1;
}
}
@@ -388,7 +389,7 @@ int dpaa_get_link_status(char *if_name, struct rte_eth_link 
*link)
 
ret = ioctl(fd, DPAA_IOCTL_GET_LINK_STATUS, &args);
if (ret) {
-   printf("Failed to get link status\n");
+   DPAA_BUS_ERR("Failed to get link status\n");
return ret;
}
 
@@ -404,9 +405,9 @@ int dpaa_get_link_status(char *if_name, struct rte_eth_link 
*link)
ret = ioctl(fd, DPAA_IOCTL_GET_LINK_STATUS_OLD, &args);
if (ret) {
if (errno == EINVAL)
-   printf("Get link status: Not Supported\n");
+   DPAA_BUS_ERR("Get link status: Not 
Supported\n")

[PATCH v2 2/2] drivers: replace printf with fprintf for debug functions

[Hemant Agrawal]

This patch replaces simple printf with fprintf for debug dump
related functions for various NXP dpaaX related drivers.

Signed-off-by: Hemant Agrawal 
---
 drivers/bus/dpaa/base/fman/netcfg_layer.c   | 22 +++
 drivers/bus/dpaa/dpaa_bus.c |  2 +-
 drivers/bus/dpaa/include/netcfg.h   |  2 +-
 drivers/crypto/caam_jr/caam_jr.c|  6 +-
 drivers/crypto/caam_jr/caam_jr_desc.h   |  4 +-
 drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 32 +-
 drivers/crypto/dpaa_sec/dpaa_sec.c  | 62 +--
 drivers/net/dpaa2/dpaa2_flow.c  | 66 +++--
 8 files changed, 99 insertions(+), 97 deletions(-)

diff --git a/drivers/bus/dpaa/base/fman/netcfg_layer.c 
b/drivers/bus/dpaa/base/fman/netcfg_layer.c
index 6a405c984d..57d87afcb0 100644
--- a/drivers/bus/dpaa/base/fman/netcfg_layer.c
+++ b/drivers/bus/dpaa/base/fman/netcfg_layer.c
@@ -29,37 +29,37 @@ static int skfd = -1;
 
 #ifdef RTE_LIBRTE_DPAA_DEBUG_DRIVER
 void
-dump_netcfg(struct netcfg_info *cfg_ptr)
+dump_netcfg(struct netcfg_info *cfg_ptr, FILE *f)
 {
int i;
 
-   printf("..  DPAA Configuration  ..\n\n");
+   fprintf(f, "..  DPAA Configuration  ..\n\n");
 
/* Network interfaces */
-   printf("Network interfaces: %d\n", cfg_ptr->num_ethports);
+   fprintf(f, "Network interfaces: %d\n", cfg_ptr->num_ethports);
for (i = 0; i < cfg_ptr->num_ethports; i++) {
struct fman_if_bpool *bpool;
struct fm_eth_port_cfg *p_cfg = &cfg_ptr->port_cfg[i];
struct fman_if *__if = p_cfg->fman_if;
 
-   printf("\n+ Fman %d, MAC %d (%s);\n",
+   fprintf(f, "\n+ Fman %d, MAC %d (%s);\n",
   __if->fman_idx, __if->mac_idx,
   (__if->mac_type == fman_mac_1g) ? "1G" :
   (__if->mac_type == fman_mac_2_5g) ? "2.5G" : "10G");
 
-   printf("\tmac_addr: " RTE_ETHER_ADDR_PRT_FMT "\n",
+   fprintf(f, "\tmac_addr: " RTE_ETHER_ADDR_PRT_FMT "\n",
   RTE_ETHER_ADDR_BYTES(&__if->mac_addr));
 
-   printf("\ttx_channel_id: 0x%02x\n",
+   fprintf(f, "\ttx_channel_id: 0x%02x\n",
   __if->tx_channel_id);
 
-   printf("\tfqid_rx_def: 0x%x\n", p_cfg->rx_def);
-   printf("\tfqid_rx_err: 0x%x\n", __if->fqid_rx_err);
+   fprintf(f, "\tfqid_rx_def: 0x%x\n", p_cfg->rx_def);
+   fprintf(f, "\tfqid_rx_err: 0x%x\n", __if->fqid_rx_err);
 
-   printf("\tfqid_tx_err: 0x%x\n", __if->fqid_tx_err);
-   printf("\tfqid_tx_confirm: 0x%x\n", __if->fqid_tx_confirm);
+   fprintf(f, "\tfqid_tx_err: 0x%x\n", __if->fqid_tx_err);
+   fprintf(f, "\tfqid_tx_confirm: 0x%x\n", __if->fqid_tx_confirm);
fman_if_for_each_bpool(bpool, __if)
-   printf("\tbuffer pool: (bpid=%d, count=%"PRId64
+   fprintf(f, "\tbuffer pool: (bpid=%d, count=%"PRId64
   " size=%"PRId64", addr=0x%"PRIx64")\n",
   bpool->bpid, bpool->count, bpool->size,
   bpool->addr);
diff --git a/drivers/bus/dpaa/dpaa_bus.c b/drivers/bus/dpaa/dpaa_bus.c
index 5d4352bb3c..7e5d713bb9 100644
--- a/drivers/bus/dpaa/dpaa_bus.c
+++ b/drivers/bus/dpaa/dpaa_bus.c
@@ -586,7 +586,7 @@ rte_dpaa_bus_dev_build(void)
}
 
 #ifdef RTE_LIBRTE_DPAA_DEBUG_DRIVER
-   dump_netcfg(dpaa_netcfg);
+   dump_netcfg(dpaa_netcfg, stdout);
 #endif
 
DPAA_BUS_LOG(DEBUG, "Number of ethernet devices = %d",
diff --git a/drivers/bus/dpaa/include/netcfg.h 
b/drivers/bus/dpaa/include/netcfg.h
index 5bdcc9186a..ebbbaf6d10 100644
--- a/drivers/bus/dpaa/include/netcfg.h
+++ b/drivers/bus/dpaa/include/netcfg.h
@@ -60,7 +60,7 @@ void netcfg_release(struct netcfg_info *cfg_ptr);
 /* cfg_ptr: configuration information pointer.
  * This function dumps configuration data to stdout.
  */
-void dump_netcfg(struct netcfg_info *cfg_ptr);
+void dump_netcfg(struct netcfg_info *cfg_ptr, FILE *f);
 #endif
 
 #endif /* __NETCFG_H */
diff --git a/drivers/crypto/caam_jr/caam_jr.c b/drivers/crypto/caam_jr/caam_jr.c
index ab06fab387..ea474a079f 100644
--- a/drivers/crypto/caam_jr/caam_jr.c
+++ b/drivers/crypto/caam_jr/caam_jr.c
@@ -461,7 +461,7 @@ caam_jr_prep_cdb(struct caam_jr_session *ses)
}
 
 #if CAAM_JR_DBG
-   SEC_DUMP_DESC(cdb->sh_desc);
+   SEC_DUMP_DESC(cdb->sh_desc, stdout);
 #endif
 
cdb->sh_hdr.hi.field.idlen = shared_desc_len;
@@ -1410,9 +1410,9 @@ caam_jr_enqueue_op(struct rte_crypto_op *op, struct 
caam_jr_qp *qp)
rte_pktmbuf_mtod(op->sym->m_src, void *),
rte_pktmbuf_data_len(op->sym->m_src));
 
-   printf("\n JD before conversion\n");
+   fprintf(stdout, "\n JD before conversion\n");
for (i = 0; i < 12; i++)

Re: [PATCH v1 1/2] bbdev: add new function to dump debug information

[Hemant Agrawal]

Hi Nicolas,

    Few comments inline.

On 02-07-2024 04:04, Nicolas Chautru wrote:

This provides a new API to dump more debug information
related to the status on a given bbdev queue.
Some of this information is visible at bbdev level.
This also provides a new option dev op, to print more
information at the lower PMD level.
This helps user to troubleshoot issues related to
previous operations provided into a queue causing
possible hard-to-debug negative scenarios.

Signed-off-by: Nicolas Chautru 
---
  lib/bbdev/rte_bbdev.c | 214 ++
  lib/bbdev/rte_bbdev.h |  41 
  lib/bbdev/rte_bbdev_pmd.h |   9 ++
  lib/bbdev/version.map |   4 +
  4 files changed, 268 insertions(+)

diff --git a/lib/bbdev/rte_bbdev.c b/lib/bbdev/rte_bbdev.c
index 13bde3c25b..81c031fc09 100644
--- a/lib/bbdev/rte_bbdev.c
+++ b/lib/bbdev/rte_bbdev.c
@@ -1190,3 +1190,217 @@ rte_bbdev_enqueue_status_str(enum 
rte_bbdev_enqueue_status status)
rte_bbdev_log(ERR, "Invalid enqueue status");
return NULL;
  }
+
+
+int
+rte_bbdev_queue_ops_dump(uint16_t dev_id, uint16_t queue_id, FILE *f)
+{
+   struct rte_bbdev_queue_data *q_data;
+   struct rte_bbdev_stats *stats;
+   uint16_t i;
+   struct rte_bbdev *dev = get_dev(dev_id);
+
+   VALID_DEV_OR_RET_ERR(dev, dev_id);
+   VALID_QUEUE_OR_RET_ERR(queue_id, dev);
+   VALID_DEV_OPS_OR_RET_ERR(dev, dev_id);
+   VALID_FUNC_OR_RET_ERR(dev->dev_ops->queue_ops_dump, dev_id);
+
+   q_data = &dev->data->queues[queue_id];
+
+   if (f == NULL)
+   return -EINVAL;
+
+   fprintf(f, "Dump of operations on %s queue %d\n",
+   dev->data->name, queue_id);
+   fprintf(f, "  Last Enqueue Status %s\n",
+   rte_bbdev_enqueue_status_str(q_data->enqueue_status));
+   for (i = 0; i < 4; i++)


It shall be RTE_BBDEV_ENQ_STATUS_SIZE_MAX instead of 4 ?



+   if (q_data->queue_stats.enqueue_status_count[i] > 0)
+   fprintf(f, "  Enqueue Status Counters %s %" PRIu64 "\n",
+   rte_bbdev_enqueue_status_str(i),
+   
q_data->queue_stats.enqueue_status_count[i]);
+   stats = &dev->data->queues[queue_id].queue_stats;
+
+   fprintf(f, "  Enqueue Count %" PRIu64 " Warning %" PRIu64 " Error %" PRIu64 
"\n",
+   stats->enqueued_count, stats->enqueue_warn_count,
+   stats->enqueue_err_count);
+   fprintf(f, "  Dequeue Count %" PRIu64 " Warning %" PRIu64 " Error %" PRIu64 
"\n",
+   stats->dequeued_count, stats->dequeue_warn_count,
+   stats->dequeue_err_count);
+

why not print acc_offload_cycles aas well?

+   return dev->dev_ops->queue_ops_dump(dev, queue_id, f);
+}
+
+char *
+rte_bbdev_ops_param_string(void *op, enum rte_bbdev_op_type op_type)
+{
+   static char str[1024];
+   static char partial[1024];
+   struct rte_bbdev_dec_op *op_dec;
+   struct rte_bbdev_enc_op *op_enc;
+   struct rte_bbdev_fft_op *op_fft;
+   struct rte_bbdev_mldts_op *op_mldts;
+
+   rte_iova_t add0 = 0, add1 = 0, add2 = 0, add3 = 0, add4 = 0;
+
+   if (op == NULL) {
+   snprintf(str, sizeof(str), "Invalid Operation pointer\n");
+   return str;
+   }
+
how about also covering mempool and opaque_data pointer - they may also 
be helpful in debugging?

RE: [EXTERNAL] Re: [PATCH v2 1/3] net/virtio_user: avoid cq descriptor buffer address accessing

[Srujana Challa]

> On 2/29/24 14:29, Srujana Challa wrote:
> > This patch makes changes to avoid descriptor buffer address accessing
> > while processing shadow control queue.
> > So that Virtio-user can work with having IOVA as descriptor buffer
> > address.
> >
> > Signed-off-by: Srujana Challa 
> > ---
> >   .../net/virtio/virtio_user/virtio_user_dev.c  | 68 +--
> >   1 file changed, 33 insertions(+), 35 deletions(-)
> >
> > diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > index d395fc1676..bf3da4340f 100644
> > --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > @@ -885,11 +885,11 @@ static uint32_t
> >   virtio_user_handle_ctrl_msg_split(struct virtio_user_dev *dev, struct 
> > vring
> *vring,
> > uint16_t idx_hdr)
> >   {
> > -   struct virtio_net_ctrl_hdr *hdr;
> > virtio_net_ctrl_ack status = ~0;
> > -   uint16_t i, idx_data, idx_status;
> > +   uint16_t i, idx_data;
> > uint32_t n_descs = 0;
> > int dlen[CVQ_MAX_DATA_DESCS], nb_dlen = 0;
> > +   struct virtio_pmd_ctrl *ctrl;
> >
> > /* locate desc for header, data, and status */
> > idx_data = vring->desc[idx_hdr].next; @@ -902,34 +902,33 @@
> > virtio_user_handle_ctrl_msg_split(struct virtio_user_dev *dev, struct vring
> *vri
> > n_descs++;
> > }
> >
> > -   /* locate desc for status */
> > -   idx_status = i;
> > n_descs++;
> >
> > -   hdr = (void *)(uintptr_t)vring->desc[idx_hdr].addr;
> > -   if (hdr->class == VIRTIO_NET_CTRL_MQ &&
> > -   hdr->cmd == VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET) {
> > -   uint16_t queues;
> > +   /* Access control command via VA from CVQ */
> > +   ctrl = (struct virtio_pmd_ctrl *)dev->hw.cvq->hdr_mz->addr;
> > +   if (ctrl->hdr.class == VIRTIO_NET_CTRL_MQ &&
> > +   ctrl->hdr.cmd == VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET) {
> > +   uint16_t *queues;
> 
> This is not future proof, as it just discards the index and assume the buffer 
> will
> always be at the same place.
> We should find a way to perform the desc address translation.
Can we use rte_mem_iova2virt() here?

> 
> >
> > -   queues = *(uint16_t *)(uintptr_t)vring->desc[idx_data].addr;
> > -   status = virtio_user_handle_mq(dev, queues);
> > -   } else if (hdr->class == VIRTIO_NET_CTRL_MQ && hdr->cmd ==
> VIRTIO_NET_CTRL_MQ_RSS_CONFIG) {
> > +   queues = (uint16_t *)ctrl->data;
> > +   status = virtio_user_handle_mq(dev, *queues);
> > +   } else if (ctrl->hdr.class == VIRTIO_NET_CTRL_MQ &&
> > +  ctrl->hdr.cmd == VIRTIO_NET_CTRL_MQ_RSS_CONFIG) {
> > struct virtio_net_ctrl_rss *rss;
> >
> > -   rss = (struct virtio_net_ctrl_rss *)(uintptr_t)vring-
> >desc[idx_data].addr;
> > +   rss = (struct virtio_net_ctrl_rss *)ctrl->data;
> > status = virtio_user_handle_mq(dev, rss->max_tx_vq);
> > -   } else if (hdr->class == VIRTIO_NET_CTRL_RX  ||
> > -  hdr->class == VIRTIO_NET_CTRL_MAC ||
> > -  hdr->class == VIRTIO_NET_CTRL_VLAN) {
> > +   } else if (ctrl->hdr.class == VIRTIO_NET_CTRL_RX  ||
> > +  ctrl->hdr.class == VIRTIO_NET_CTRL_MAC ||
> > +  ctrl->hdr.class == VIRTIO_NET_CTRL_VLAN) {
> > status = 0;
> > }
> >
> > if (!status && dev->scvq)
> > -   status = virtio_send_command(&dev->scvq->cq,
> > -   (struct virtio_pmd_ctrl *)hdr, dlen, nb_dlen);
> > +   status = virtio_send_command(&dev->scvq->cq, ctrl, dlen,
> nb_dlen);
> >
> > /* Update status */
> > -   *(virtio_net_ctrl_ack *)(uintptr_t)vring->desc[idx_status].addr =
> status;
> > +   ctrl->status = status;
> >
> > return n_descs;
> >   }
> > @@ -948,7 +947,7 @@ virtio_user_handle_ctrl_msg_packed(struct
> virtio_user_dev *dev,
> >struct vring_packed *vring,
> >uint16_t idx_hdr)
> >   {
> > -   struct virtio_net_ctrl_hdr *hdr;
> > +   struct virtio_pmd_ctrl *ctrl;
> > virtio_net_ctrl_ack status = ~0;
> > uint16_t idx_data, idx_status;
> > /* initialize to one, header is first */ @@ -971,32 +970,31 @@
> > virtio_user_handle_ctrl_msg_packed(struct virtio_user_dev *dev,
> > n_descs++;
> > }
> >
> > -   hdr = (void *)(uintptr_t)vring->desc[idx_hdr].addr;
> > -   if (hdr->class == VIRTIO_NET_CTRL_MQ &&
> > -   hdr->cmd == VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET) {
> > -   uint16_t queues;
> > +   /* Access control command via VA from CVQ */
> > +   ctrl = (struct virtio_pmd_ctrl *)dev->hw.cvq->hdr_mz->addr;
> > +   if (ctrl->hdr.class == VIRTIO_NET_CTRL_MQ &&
> > +   ctrl->hdr.cmd == VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET) {
> > +   uint16_t *queues;
> >
> > -   queues = *(uint16_t *)(uintptr_t)
> > -   vring->desc[idx_data].addr;
> > -   status = virtio_user_handle_mq(de

Re: [PATCH] dpdk-devbind: fix indentation

[Robin Jarry]

Stephen Hemminger, Jul 01, 2024 at 19:45:

The python check tool (flake8) is picky about the indentation
of continuation lines, and dpdk-devbind was not following standard.

Error is:
 E127 continuation line over-indented for visual indent

Fixes: 2ff801515e49 ("usertools/devbind: update octeontx2 DMA device")
Signed-off-by: Stephen Hemminger 
---


Acked-by: Robin Jarry 

Re: [PATCH] dpdk-pmdinfo: remove unneeded whitespace

[Robin Jarry]

Stephen Hemminger, Jul 01, 2024 at 19:58:

Fix the warning
$ flake8 --max-line-length=100 dpdk-pmdinfo.py
dpdk-pmdinfo.py:217:40: E203 whitespace before ':'

Signed-off-by: Stephen Hemminger 


Acked-by: Robin Jarry 

Re: [PATCH v2 1/2] drivers: replace printf with log macros

[David Marchand]

On Tue, Jul 2, 2024 at 12:40 PM Hemant Agrawal  wrote:
>
> This patch replaces the printf with related log macros and functions at
> various places in NXP dpaaX drivers.
>
> Signed-off-by: Hemant Agrawal 

Please double check callers of DPAA_BUS_LOG to avoid log messages
ending with \n\n.
I see that DPAA_BUS_LOG is called from DPAA_BUS_(ERR|INFO|WARN) and FMAN_ERR.


-- 
David Marchand

[PATCH] net/ice: fix use of ice_bitmap_t in promisc functions

[Ian Stokes]

Promisc functions were modified to use ice_bitmap_t.
However use of ice_bitmap_t requires specific helper
functions to ensure correctness.

Fix this by adding correct calls to declare, zero and set
ice_bitmap_t within the promisc functions.

Signed-off-by: Ian Stokes 
---
 drivers/net/ice/ice_ethdev.c | 59 +++-
 1 file changed, 38 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index f7189b2dfe..23f14d7c42 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -3807,7 +3807,8 @@ ice_dev_start(struct rte_eth_dev *dev)
uint8_t timer = hw->func_caps.ts_func_info.tmr_index_owned;
uint32_t pin_idx = ad->devargs.pin_idx;
struct rte_tm_error tm_err;
-   ice_bitmap_t pmask;
+   ice_declare_bitmap(pmask, ICE_PROMISC_MAX);
+   ice_zero_bitmap(pmask, ICE_PROMISC_MAX);
 
/* program Tx queues' context in hardware */
for (nb_txq = 0; nb_txq < data->nb_tx_queues; nb_txq++) {
@@ -3864,9 +3865,12 @@ ice_dev_start(struct rte_eth_dev *dev)
return -EIO;
 
/* Enable receiving broadcast packets and transmitting packets */
-   pmask = ICE_PROMISC_BCAST_RX | ICE_PROMISC_BCAST_TX |
-   ICE_PROMISC_UCAST_TX | ICE_PROMISC_MCAST_TX;
-   ret = ice_set_vsi_promisc(hw, vsi->idx, &pmask, 0);
+   ice_set_bit(ICE_PROMISC_BCAST_RX, pmask);
+   ice_set_bit(ICE_PROMISC_BCAST_TX, pmask);
+   ice_set_bit(ICE_PROMISC_UCAST_TX, pmask);
+   ice_set_bit(ICE_PROMISC_MCAST_TX, pmask);
+
+   ret = ice_set_vsi_promisc(hw, vsi->idx, pmask, 0);
if (ret != ICE_SUCCESS)
PMD_DRV_LOG(INFO, "fail to set vsi broadcast");
 
@@ -5211,12 +5215,15 @@ ice_promisc_enable(struct rte_eth_dev *dev)
struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
struct ice_vsi *vsi = pf->main_vsi;
int status, ret = 0;
-   ice_bitmap_t pmask;
+   ice_declare_bitmap(pmask, ICE_PROMISC_MAX);
+   ice_zero_bitmap(pmask, ICE_PROMISC_MAX);
 
-   pmask = ICE_PROMISC_UCAST_RX | ICE_PROMISC_UCAST_TX |
-   ICE_PROMISC_MCAST_RX | ICE_PROMISC_MCAST_TX;
+   ice_set_bit(ICE_PROMISC_UCAST_RX, pmask);
+   ice_set_bit(ICE_PROMISC_UCAST_TX, pmask);
+   ice_set_bit(ICE_PROMISC_MCAST_RX, pmask);
+   ice_set_bit(ICE_PROMISC_MCAST_TX, pmask);
 
-   status = ice_set_vsi_promisc(hw, vsi->idx, &pmask, 0);
+   status = ice_set_vsi_promisc(hw, vsi->idx, pmask, 0);
switch (status) {
case ICE_ERR_ALREADY_EXISTS:
PMD_DRV_LOG(DEBUG, "Promisc mode has already been enabled");
@@ -5237,15 +5244,21 @@ ice_promisc_disable(struct rte_eth_dev *dev)
struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
struct ice_vsi *vsi = pf->main_vsi;
int status, ret = 0;
-   ice_bitmap_t pmask;
+   ice_declare_bitmap(pmask, ICE_PROMISC_MAX);
+   ice_zero_bitmap(pmask, ICE_PROMISC_MAX);
 
-   if (dev->data->all_multicast == 1)
-   pmask = ICE_PROMISC_UCAST_RX | ICE_PROMISC_UCAST_TX;
-   else
-   pmask = ICE_PROMISC_UCAST_RX | ICE_PROMISC_UCAST_TX |
-   ICE_PROMISC_MCAST_RX | ICE_PROMISC_MCAST_TX;
+   if (dev->data->all_multicast == 1) {
+   ice_set_bit(ICE_PROMISC_UCAST_RX, pmask);
+   ice_set_bit(ICE_PROMISC_UCAST_TX, pmask);
+   }
+   else {
+   ice_set_bit(ICE_PROMISC_UCAST_RX, pmask);
+   ice_set_bit(ICE_PROMISC_UCAST_TX, pmask);
+   ice_set_bit(ICE_PROMISC_MCAST_RX, pmask);
+   ice_set_bit(ICE_PROMISC_MCAST_TX, pmask);
+   }
 
-   status = ice_clear_vsi_promisc(hw, vsi->idx, &pmask, 0);
+   status = ice_clear_vsi_promisc(hw, vsi->idx, pmask, 0);
if (status != ICE_SUCCESS) {
PMD_DRV_LOG(ERR, "Failed to clear promisc, err=%d", status);
ret = -EAGAIN;
@@ -5261,11 +5274,13 @@ ice_allmulti_enable(struct rte_eth_dev *dev)
struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
struct ice_vsi *vsi = pf->main_vsi;
int status, ret = 0;
-   ice_bitmap_t pmask;
+   ice_declare_bitmap(pmask, ICE_PROMISC_MAX);
+   ice_zero_bitmap(pmask, ICE_PROMISC_MAX);
 
-   pmask = ICE_PROMISC_MCAST_RX | ICE_PROMISC_MCAST_TX;
+   ice_set_bit(ICE_PROMISC_MCAST_RX, pmask);
+   ice_set_bit(ICE_PROMISC_MCAST_TX, pmask);
 
-   status = ice_set_vsi_promisc(hw, vsi->idx, &pmask, 0);
+   status = ice_set_vsi_promisc(hw, vsi->idx, pmask, 0);
 
switch (status) {
case ICE_ERR_ALREADY_EXISTS:
@@ -5287,14 +5302,16 @@ ice_allmulti_disable(struct rte_eth_dev *dev)
struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
struct ice_vsi *vsi = pf->main_vsi;
int status, ret = 0;
-   ice_bitmap_t pmask;
+   ice_declare_bitmap(pmask, ICE_PROMI

RE: [PATCH] net/ice: fix use of ice_bitmap_t in promisc functions

[Stokes, Ian]

> Promisc functions were modified to use ice_bitmap_t.
> However use of ice_bitmap_t requires specific helper
> functions to ensure correctness.
> 
> Fix this by adding correct calls to declare, zero and set
> ice_bitmap_t within the promisc functions.
> 
> Signed-off-by: Ian Stokes 

Just to clarify, this patch is intended to be applied with the ice shared code 
update
which I believe is in net-next. It can either be added as an extra patch to 
that series
or merged with the original patch, whatever is preferred.

Thanks
Ian

> ---
>  drivers/net/ice/ice_ethdev.c | 59 +++-
>  1 file changed, 38 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
> index f7189b2dfe..23f14d7c42 100644
> --- a/drivers/net/ice/ice_ethdev.c
> +++ b/drivers/net/ice/ice_ethdev.c
> @@ -3807,7 +3807,8 @@ ice_dev_start(struct rte_eth_dev *dev)
>   uint8_t timer = hw->func_caps.ts_func_info.tmr_index_owned;
>   uint32_t pin_idx = ad->devargs.pin_idx;
>   struct rte_tm_error tm_err;
> - ice_bitmap_t pmask;
> + ice_declare_bitmap(pmask, ICE_PROMISC_MAX);
> + ice_zero_bitmap(pmask, ICE_PROMISC_MAX);
> 
>   /* program Tx queues' context in hardware */
>   for (nb_txq = 0; nb_txq < data->nb_tx_queues; nb_txq++) {
> @@ -3864,9 +3865,12 @@ ice_dev_start(struct rte_eth_dev *dev)
>   return -EIO;
> 
>   /* Enable receiving broadcast packets and transmitting packets */
> - pmask = ICE_PROMISC_BCAST_RX | ICE_PROMISC_BCAST_TX |
> - ICE_PROMISC_UCAST_TX |
> ICE_PROMISC_MCAST_TX;
> - ret = ice_set_vsi_promisc(hw, vsi->idx, &pmask, 0);
> + ice_set_bit(ICE_PROMISC_BCAST_RX, pmask);
> + ice_set_bit(ICE_PROMISC_BCAST_TX, pmask);
> + ice_set_bit(ICE_PROMISC_UCAST_TX, pmask);
> + ice_set_bit(ICE_PROMISC_MCAST_TX, pmask);
> +
> + ret = ice_set_vsi_promisc(hw, vsi->idx, pmask, 0);
>   if (ret != ICE_SUCCESS)
>   PMD_DRV_LOG(INFO, "fail to set vsi broadcast");
> 
> @@ -5211,12 +5215,15 @@ ice_promisc_enable(struct rte_eth_dev *dev)
>   struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
>   struct ice_vsi *vsi = pf->main_vsi;
>   int status, ret = 0;
> - ice_bitmap_t pmask;
> + ice_declare_bitmap(pmask, ICE_PROMISC_MAX);
> + ice_zero_bitmap(pmask, ICE_PROMISC_MAX);
> 
> - pmask = ICE_PROMISC_UCAST_RX | ICE_PROMISC_UCAST_TX |
> - ICE_PROMISC_MCAST_RX | ICE_PROMISC_MCAST_TX;
> + ice_set_bit(ICE_PROMISC_UCAST_RX, pmask);
> + ice_set_bit(ICE_PROMISC_UCAST_TX, pmask);
> + ice_set_bit(ICE_PROMISC_MCAST_RX, pmask);
> + ice_set_bit(ICE_PROMISC_MCAST_TX, pmask);
> 
> - status = ice_set_vsi_promisc(hw, vsi->idx, &pmask, 0);
> + status = ice_set_vsi_promisc(hw, vsi->idx, pmask, 0);
>   switch (status) {
>   case ICE_ERR_ALREADY_EXISTS:
>   PMD_DRV_LOG(DEBUG, "Promisc mode has already been
> enabled");
> @@ -5237,15 +5244,21 @@ ice_promisc_disable(struct rte_eth_dev *dev)
>   struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
>   struct ice_vsi *vsi = pf->main_vsi;
>   int status, ret = 0;
> - ice_bitmap_t pmask;
> + ice_declare_bitmap(pmask, ICE_PROMISC_MAX);
> + ice_zero_bitmap(pmask, ICE_PROMISC_MAX);
> 
> - if (dev->data->all_multicast == 1)
> - pmask = ICE_PROMISC_UCAST_RX |
> ICE_PROMISC_UCAST_TX;
> - else
> - pmask = ICE_PROMISC_UCAST_RX |
> ICE_PROMISC_UCAST_TX |
> - ICE_PROMISC_MCAST_RX |
> ICE_PROMISC_MCAST_TX;
> + if (dev->data->all_multicast == 1) {
> + ice_set_bit(ICE_PROMISC_UCAST_RX, pmask);
> + ice_set_bit(ICE_PROMISC_UCAST_TX, pmask);
> + }
> + else {
> + ice_set_bit(ICE_PROMISC_UCAST_RX, pmask);
> + ice_set_bit(ICE_PROMISC_UCAST_TX, pmask);
> + ice_set_bit(ICE_PROMISC_MCAST_RX, pmask);
> + ice_set_bit(ICE_PROMISC_MCAST_TX, pmask);
> + }
> 
> - status = ice_clear_vsi_promisc(hw, vsi->idx, &pmask, 0);
> + status = ice_clear_vsi_promisc(hw, vsi->idx, pmask, 0);
>   if (status != ICE_SUCCESS) {
>   PMD_DRV_LOG(ERR, "Failed to clear promisc, err=%d",
> status);
>   ret = -EAGAIN;
> @@ -5261,11 +5274,13 @@ ice_allmulti_enable(struct rte_eth_dev *dev)
>   struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
>   struct ice_vsi *vsi = pf->main_vsi;
>   int status, ret = 0;
> - ice_bitmap_t pmask;
> + ice_declare_bitmap(pmask, ICE_PROMISC_MAX);
> + ice_zero_bitmap(pmask, ICE_PROMISC_MAX);
> 
> - pmask = ICE_PROMISC_MCAST_RX | ICE_PROMISC_MCAST_TX;
> + ice_set_bit(ICE_PROMISC_MCAST_RX, pmask);
> + ice_set_bit(ICE_PROMISC_MCAST_TX, pmask);
> 
> - status = ice_set_vsi_promisc(hw, vsi->idx, &pmask, 0);
> + status = ice_set_vsi_promisc(hw, vsi->idx, pmask, 0);
> 
>   switch (status) {
>  

Re: [PATCH] net/ice: fix use of ice_bitmap_t in promisc functions

[Bruce Richardson]

On Tue, Jul 02, 2024 at 01:24:03PM +0100, Stokes, Ian wrote:
> > Promisc functions were modified to use ice_bitmap_t.
> > However use of ice_bitmap_t requires specific helper
> > functions to ensure correctness.
> > 
> > Fix this by adding correct calls to declare, zero and set
> > ice_bitmap_t within the promisc functions.
> > 
> > Signed-off-by: Ian Stokes 
> 
> Just to clarify, this patch is intended to be applied with the ice shared 
> code update
> which I believe is in net-next. It can either be added as an extra patch to 
> that series
> or merged with the original patch, whatever is preferred.
> 
> Thanks
> Ian
> 

I think this change should be squashed to commit [1], since it is part of
the overall change from using bitmasks to actual bitmap type.

/Bruce

[1] 
http://git.dpdk.org/next/dpdk-next-net-intel/commit/?id=126916f331dfa9fd6e997c0d00b1e29de9cc

Re: [EXTERNAL] Re: [PATCH v2 1/3] net/virtio_user: avoid cq descriptor buffer address accessing

[Maxime Coquelin]

On 7/2/24 13:09, Srujana Challa wrote:

On 2/29/24 14:29, Srujana Challa wrote:

This patch makes changes to avoid descriptor buffer address accessing
while processing shadow control queue.
So that Virtio-user can work with having IOVA as descriptor buffer
address.

Signed-off-by: Srujana Challa 
---
   .../net/virtio/virtio_user/virtio_user_dev.c  | 68 +--
   1 file changed, 33 insertions(+), 35 deletions(-)

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index d395fc1676..bf3da4340f 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -885,11 +885,11 @@ static uint32_t
   virtio_user_handle_ctrl_msg_split(struct virtio_user_dev *dev, struct vring

*vring,

uint16_t idx_hdr)
   {
-   struct virtio_net_ctrl_hdr *hdr;
virtio_net_ctrl_ack status = ~0;
-   uint16_t i, idx_data, idx_status;
+   uint16_t i, idx_data;
uint32_t n_descs = 0;
int dlen[CVQ_MAX_DATA_DESCS], nb_dlen = 0;
+   struct virtio_pmd_ctrl *ctrl;

/* locate desc for header, data, and status */
idx_data = vring->desc[idx_hdr].next; @@ -902,34 +902,33 @@
virtio_user_handle_ctrl_msg_split(struct virtio_user_dev *dev, struct vring

*vri

n_descs++;
}

-   /* locate desc for status */
-   idx_status = i;
n_descs++;

-   hdr = (void *)(uintptr_t)vring->desc[idx_hdr].addr;
-   if (hdr->class == VIRTIO_NET_CTRL_MQ &&
-   hdr->cmd == VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET) {
-   uint16_t queues;
+   /* Access control command via VA from CVQ */
+   ctrl = (struct virtio_pmd_ctrl *)dev->hw.cvq->hdr_mz->addr;
+   if (ctrl->hdr.class == VIRTIO_NET_CTRL_MQ &&
+   ctrl->hdr.cmd == VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET) {
+   uint16_t *queues;


This is not future proof, as it just discards the index and assume the buffer 
will
always be at the same place.
We should find a way to perform the desc address translation.

Can we use rte_mem_iova2virt() here?


It should be safe to use it here.
Can you send a new revision ASAP, which would use this API and not take
the shortcurt, i.e. keep fetching buffer addres from descriptors?

Thanks,
Maxime







-   queues = *(uint16_t *)(uintptr_t)vring->desc[idx_data].addr;
-   status = virtio_user_handle_mq(dev, queues);
-   } else if (hdr->class == VIRTIO_NET_CTRL_MQ && hdr->cmd ==

VIRTIO_NET_CTRL_MQ_RSS_CONFIG) {

+   queues = (uint16_t *)ctrl->data;
+   status = virtio_user_handle_mq(dev, *queues);
+   } else if (ctrl->hdr.class == VIRTIO_NET_CTRL_MQ &&
+  ctrl->hdr.cmd == VIRTIO_NET_CTRL_MQ_RSS_CONFIG) {
struct virtio_net_ctrl_rss *rss;

-   rss = (struct virtio_net_ctrl_rss *)(uintptr_t)vring-
desc[idx_data].addr;
+   rss = (struct virtio_net_ctrl_rss *)ctrl->data;
status = virtio_user_handle_mq(dev, rss->max_tx_vq);
-   } else if (hdr->class == VIRTIO_NET_CTRL_RX  ||
-  hdr->class == VIRTIO_NET_CTRL_MAC ||
-  hdr->class == VIRTIO_NET_CTRL_VLAN) {
+   } else if (ctrl->hdr.class == VIRTIO_NET_CTRL_RX  ||
+  ctrl->hdr.class == VIRTIO_NET_CTRL_MAC ||
+  ctrl->hdr.class == VIRTIO_NET_CTRL_VLAN) {
status = 0;
}

if (!status && dev->scvq)
-   status = virtio_send_command(&dev->scvq->cq,
-   (struct virtio_pmd_ctrl *)hdr, dlen, nb_dlen);
+   status = virtio_send_command(&dev->scvq->cq, ctrl, dlen,

nb_dlen);


/* Update status */
-   *(virtio_net_ctrl_ack *)(uintptr_t)vring->desc[idx_status].addr =

status;

+   ctrl->status = status;

return n_descs;
   }
@@ -948,7 +947,7 @@ virtio_user_handle_ctrl_msg_packed(struct

virtio_user_dev *dev,

   struct vring_packed *vring,
   uint16_t idx_hdr)
   {
-   struct virtio_net_ctrl_hdr *hdr;
+   struct virtio_pmd_ctrl *ctrl;
virtio_net_ctrl_ack status = ~0;
uint16_t idx_data, idx_status;
/* initialize to one, header is first */ @@ -971,32 +970,31 @@
virtio_user_handle_ctrl_msg_packed(struct virtio_user_dev *dev,
n_descs++;
}

-   hdr = (void *)(uintptr_t)vring->desc[idx_hdr].addr;
-   if (hdr->class == VIRTIO_NET_CTRL_MQ &&
-   hdr->cmd == VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET) {
-   uint16_t queues;
+   /* Access control command via VA from CVQ */
+   ctrl = (struct virtio_pmd_ctrl *)dev->hw.cvq->hdr_mz->addr;
+   if (ctrl->hdr.class == VIRTIO_NET_CTRL_MQ &&
+   ctrl->hdr.cmd == VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET) {
+   uint16_t *queues;

-   queues = *(uint16_t *)(uintptr_t)
-   

[PATCH v3 1/3] drivers: replace printf with log macros

[Hemant Agrawal]

This patch replaces the printf with related log macros and functions at
various places in NXP dpaaX drivers.

Signed-off-by: Hemant Agrawal 
---
 drivers/bus/dpaa/base/fman/fman.c  |  6 ++
 drivers/bus/dpaa/base/qbman/process.c  | 29 +-
 drivers/bus/dpaa/base/qbman/qman.c |  2 +-
 drivers/bus/fslmc/fslmc_vfio.c |  2 +-
 drivers/bus/fslmc/qbman/qbman_portal.c |  2 +-
 drivers/crypto/caam_jr/caam_jr.c   |  6 +++---
 drivers/crypto/caam_jr/caam_jr_uio.c   |  2 +-
 drivers/net/dpaa/dpaa_ethdev.c |  6 +++---
 drivers/net/dpaa/dpaa_flow.c   |  8 +++
 drivers/net/dpaa/dpaa_rxtx.c   |  2 +-
 drivers/net/dpaa2/dpaa2_ethdev.c   |  2 +-
 drivers/net/dpaa2/dpaa2_tm.c   | 12 +--
 12 files changed, 39 insertions(+), 40 deletions(-)

diff --git a/drivers/bus/dpaa/base/fman/fman.c 
b/drivers/bus/dpaa/base/fman/fman.c
index 1814372a40..953f7d7061 100644
--- a/drivers/bus/dpaa/base/fman/fman.c
+++ b/drivers/bus/dpaa/base/fman/fman.c
@@ -723,10 +723,8 @@ fman_finish(void)
/* release the mapping */
_errno = munmap(__if->ccsr_map, __if->regs_size);
if (unlikely(_errno < 0))
-   fprintf(stderr, "%s:%d:%s(): munmap() = %d (%s)\n",
-   __FILE__, __LINE__, __func__,
-   -errno, strerror(errno));
-   printf("Tearing down %s\n", __if->node_path);
+   FMAN_ERR(_errno, "munmap() = (%s)", strerror(errno));
+   DPAA_BUS_INFO("Tearing down %s", __if->node_path);
list_del(&__if->__if.node);
rte_free(__if);
}
diff --git a/drivers/bus/dpaa/base/qbman/process.c 
b/drivers/bus/dpaa/base/qbman/process.c
index 3504ec97db..59e0d641ce 100644
--- a/drivers/bus/dpaa/base/qbman/process.c
+++ b/drivers/bus/dpaa/base/qbman/process.c
@@ -13,6 +13,7 @@
 #include "process.h"
 
 #include 
+#include "rte_dpaa_logs.h"
 
 /* As higher-level drivers will be built on top of this (dma_mem, qbman, ...),
  * it's preferable that the process driver itself not provide any exported API.
@@ -99,12 +100,12 @@ void process_release(enum dpaa_id_type id_type, uint32_t 
base, uint32_t num)
int ret = check_fd();
 
if (ret) {
-   fprintf(stderr, "Process FD failure\n");
+   DPAA_BUS_ERR("Process FD failure");
return;
}
ret = ioctl(fd, DPAA_IOCTL_ID_RELEASE, &id);
if (ret)
-   fprintf(stderr, "Process FD ioctl failure type %d base 0x%x num 
%d\n",
+   DPAA_BUS_ERR("Process FD ioctl failure type %d base 0x%x num 
%d",
id_type, base, num);
 }
 
@@ -333,9 +334,9 @@ int dpaa_intr_disable(char *if_name)
ret = ioctl(fd, DPAA_IOCTL_DISABLE_LINK_STATUS_INTERRUPT, if_name);
if (ret) {
if (errno == EINVAL)
-   printf("Failed to disable interrupt: Not Supported\n");
+   DPAA_BUS_ERR("Failed to disable interrupt: Not 
Supported");
else
-   printf("Failed to disable interrupt\n");
+   DPAA_BUS_ERR("Failed to disable interrupt");
return ret;
}
 
@@ -357,7 +358,7 @@ int dpaa_get_ioctl_version_number(void)
if (errno == EINVAL) {
version_num = 1;
} else {
-   printf("Failed to get ioctl version number\n");
+   DPAA_BUS_ERR("Failed to get ioctl version number");
version_num = -1;
}
}
@@ -388,7 +389,7 @@ int dpaa_get_link_status(char *if_name, struct rte_eth_link 
*link)
 
ret = ioctl(fd, DPAA_IOCTL_GET_LINK_STATUS, &args);
if (ret) {
-   printf("Failed to get link status\n");
+   DPAA_BUS_ERR("Failed to get link status");
return ret;
}
 
@@ -404,9 +405,9 @@ int dpaa_get_link_status(char *if_name, struct rte_eth_link 
*link)
ret = ioctl(fd, DPAA_IOCTL_GET_LINK_STATUS_OLD, &args);
if (ret) {
if (errno == EINVAL)
-   printf("Get link status: Not Supported\n");
+   DPAA_BUS_ERR("Get link status: Not Supported");
else
-   printf("Failed to get link status\n");
+   DPAA_BUS_ERR("Failed to get link status");
return ret;
}
 
@@ -434,9 +435,9 @@ int dpaa_update_link_status(char *if_name, int link_status)
ret = ioctl(fd, DPAA_IOCTL_UPDATE_LINK_STATUS, &args);
if (ret) {
if (errno == EINVAL)
-   printf("Failed to set link status: Not Supported\n");
+   DPAA_BUS_ERR("Failed to set l

[PATCH v3 2/3] bus/dpaa: remove double newline

[Hemant Agrawal]

Remove duplicate newline `\n` from the debugging macros
to avoid double `\n\n`.

Signed-off-by: Hemant Agrawal 
---
 drivers/bus/dpaa/base/fman/fman.c | 58 +++
 drivers/bus/dpaa/dpaa_bus.c   | 12 +++
 2 files changed, 34 insertions(+), 36 deletions(-)

diff --git a/drivers/bus/dpaa/base/fman/fman.c 
b/drivers/bus/dpaa/base/fman/fman.c
index 953f7d7061..41195eb0a7 100644
--- a/drivers/bus/dpaa/base/fman/fman.c
+++ b/drivers/bus/dpaa/base/fman/fman.c
@@ -244,13 +244,13 @@ fman_if_init(const struct device_node *dpa_node)
/* Obtain the MAC node used by this interface except macless */
mac_phandle = of_get_property(dpa_node, mprop, &lenp);
if (!mac_phandle) {
-   FMAN_ERR(-EINVAL, "%s: no %s\n", dname, mprop);
+   FMAN_ERR(-EINVAL, "%s: no %s", dname, mprop);
return -EINVAL;
}
assert(lenp == sizeof(phandle));
mac_node = of_find_node_by_phandle(*mac_phandle);
if (!mac_node) {
-   FMAN_ERR(-ENXIO, "%s: bad 'fsl,fman-mac\n", dname);
+   FMAN_ERR(-ENXIO, "%s: bad 'fsl,fman-mac", dname);
return -ENXIO;
}
mname = mac_node->full_name;
@@ -262,19 +262,19 @@ fman_if_init(const struct device_node *dpa_node)
ports_phandle = of_get_property(mac_node, "fsl,fman-ports",
&lenp);
if (!ports_phandle) {
-   FMAN_ERR(-EINVAL, "%s: no fsl,port-handles\n",
+   FMAN_ERR(-EINVAL, "%s: no fsl,port-handles",
 mname);
return -EINVAL;
}
assert(lenp == (2 * sizeof(phandle)));
rx_node = of_find_node_by_phandle(ports_phandle[0]);
if (!rx_node) {
-   FMAN_ERR(-ENXIO, "%s: bad fsl,port-handle[0]\n", mname);
+   FMAN_ERR(-ENXIO, "%s: bad fsl,port-handle[0]", mname);
return -ENXIO;
}
tx_node = of_find_node_by_phandle(ports_phandle[1]);
if (!tx_node) {
-   FMAN_ERR(-ENXIO, "%s: bad fsl,port-handle[1]\n", mname);
+   FMAN_ERR(-ENXIO, "%s: bad fsl,port-handle[1]", mname);
return -ENXIO;
}
 
@@ -282,8 +282,7 @@ fman_if_init(const struct device_node *dpa_node)
if (of_device_is_compatible(dpa_node, "fsl,dpa-ethernet")) {
port_cell_idx = of_get_property(rx_node, "cell-index", &lenp);
if (!port_cell_idx) {
-   FMAN_ERR(-ENXIO,
-"%s: no cell-index for port\n", mname);
+   FMAN_ERR(-ENXIO, "%s: no cell-index for port", mname);
return -ENXIO;
}
assert(lenp == sizeof(*port_cell_idx));
@@ -305,8 +304,7 @@ fman_if_init(const struct device_node *dpa_node)
ext_args_cell_idx = of_get_property(ext_args_node,
"cell-index", &lenp);
if (!ext_args_cell_idx) {
-   FMAN_ERR(-ENXIO,
-"%s: no cell-index for ext args\n",
+   FMAN_ERR(-ENXIO, "%s: no cell-index for ext 
args",
 mname);
return -ENXIO;
}
@@ -343,7 +341,7 @@ fman_if_init(const struct device_node *dpa_node)
/* Allocate an object for this network interface */
__if = rte_malloc(NULL, sizeof(*__if), RTE_CACHE_LINE_SIZE);
if (!__if) {
-   FMAN_ERR(-ENOMEM, "malloc(%zu)\n", sizeof(*__if));
+   FMAN_ERR(-ENOMEM, "malloc(%zu)", sizeof(*__if));
goto err;
}
memset(__if, 0, sizeof(*__if));
@@ -356,12 +354,12 @@ fman_if_init(const struct device_node *dpa_node)
/* Map the CCSR regs for the MAC node */
regs_addr = of_get_address(mac_node, 0, &__if->regs_size, NULL);
if (!regs_addr) {
-   FMAN_ERR(-EINVAL, "of_get_address(%s)\n", mname);
+   FMAN_ERR(-EINVAL, "of_get_address(%s)", mname);
goto err;
}
phys_addr = of_translate_address(mac_node, regs_addr);
if (!phys_addr) {
-   FMAN_ERR(-EINVAL, "of_translate_address(%s, %p)\n",
+   FMAN_ERR(-EINVAL, "of_translate_address(%s, %p)",
 mname, regs_addr);
goto err;
}
@@ -369,7 +367,7 @@ fman_if_init(const struct device_node *dpa_node)
  PROT_READ | PROT_WRITE, MAP_SHARED,
  fman_ccsr_map_fd, phys_addr);
if (__if->ccsr_map == MAP_FAILED) {
-   FMAN_ERR(-errno, "mmap(0x%"PRIx64")\n", phys_addr);
+   FMAN_ERR(-errno, "mmap(0x%"PRIx64")", phys_addr);
goto err;
}
na = of_n_addr_cells(mac_node);
@@ -380,13 +378,13 @@ fman_if_init(const struc

[PATCH v3 3/3] drivers: replace printf with fprintf for debug functions

[Hemant Agrawal]

This patch replaces simple printf with fprintf for debug dump
related functions for various NXP dpaaX related drivers.

Signed-off-by: Hemant Agrawal 
---
 drivers/bus/dpaa/base/fman/netcfg_layer.c   | 22 +++
 drivers/bus/dpaa/dpaa_bus.c |  2 +-
 drivers/bus/dpaa/include/netcfg.h   |  2 +-
 drivers/crypto/caam_jr/caam_jr.c|  4 +-
 drivers/crypto/caam_jr/caam_jr_desc.h   |  4 +-
 drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 32 +-
 drivers/crypto/dpaa_sec/dpaa_sec.c  | 62 +--
 drivers/net/dpaa2/dpaa2_flow.c  | 66 +++--
 8 files changed, 98 insertions(+), 96 deletions(-)

diff --git a/drivers/bus/dpaa/base/fman/netcfg_layer.c 
b/drivers/bus/dpaa/base/fman/netcfg_layer.c
index 6a405c984d..57d87afcb0 100644
--- a/drivers/bus/dpaa/base/fman/netcfg_layer.c
+++ b/drivers/bus/dpaa/base/fman/netcfg_layer.c
@@ -29,37 +29,37 @@ static int skfd = -1;
 
 #ifdef RTE_LIBRTE_DPAA_DEBUG_DRIVER
 void
-dump_netcfg(struct netcfg_info *cfg_ptr)
+dump_netcfg(struct netcfg_info *cfg_ptr, FILE *f)
 {
int i;
 
-   printf("..  DPAA Configuration  ..\n\n");
+   fprintf(f, "..  DPAA Configuration  ..\n\n");
 
/* Network interfaces */
-   printf("Network interfaces: %d\n", cfg_ptr->num_ethports);
+   fprintf(f, "Network interfaces: %d\n", cfg_ptr->num_ethports);
for (i = 0; i < cfg_ptr->num_ethports; i++) {
struct fman_if_bpool *bpool;
struct fm_eth_port_cfg *p_cfg = &cfg_ptr->port_cfg[i];
struct fman_if *__if = p_cfg->fman_if;
 
-   printf("\n+ Fman %d, MAC %d (%s);\n",
+   fprintf(f, "\n+ Fman %d, MAC %d (%s);\n",
   __if->fman_idx, __if->mac_idx,
   (__if->mac_type == fman_mac_1g) ? "1G" :
   (__if->mac_type == fman_mac_2_5g) ? "2.5G" : "10G");
 
-   printf("\tmac_addr: " RTE_ETHER_ADDR_PRT_FMT "\n",
+   fprintf(f, "\tmac_addr: " RTE_ETHER_ADDR_PRT_FMT "\n",
   RTE_ETHER_ADDR_BYTES(&__if->mac_addr));
 
-   printf("\ttx_channel_id: 0x%02x\n",
+   fprintf(f, "\ttx_channel_id: 0x%02x\n",
   __if->tx_channel_id);
 
-   printf("\tfqid_rx_def: 0x%x\n", p_cfg->rx_def);
-   printf("\tfqid_rx_err: 0x%x\n", __if->fqid_rx_err);
+   fprintf(f, "\tfqid_rx_def: 0x%x\n", p_cfg->rx_def);
+   fprintf(f, "\tfqid_rx_err: 0x%x\n", __if->fqid_rx_err);
 
-   printf("\tfqid_tx_err: 0x%x\n", __if->fqid_tx_err);
-   printf("\tfqid_tx_confirm: 0x%x\n", __if->fqid_tx_confirm);
+   fprintf(f, "\tfqid_tx_err: 0x%x\n", __if->fqid_tx_err);
+   fprintf(f, "\tfqid_tx_confirm: 0x%x\n", __if->fqid_tx_confirm);
fman_if_for_each_bpool(bpool, __if)
-   printf("\tbuffer pool: (bpid=%d, count=%"PRId64
+   fprintf(f, "\tbuffer pool: (bpid=%d, count=%"PRId64
   " size=%"PRId64", addr=0x%"PRIx64")\n",
   bpool->bpid, bpool->count, bpool->size,
   bpool->addr);
diff --git a/drivers/bus/dpaa/dpaa_bus.c b/drivers/bus/dpaa/dpaa_bus.c
index e483428ddc..8ce93abd84 100644
--- a/drivers/bus/dpaa/dpaa_bus.c
+++ b/drivers/bus/dpaa/dpaa_bus.c
@@ -586,7 +586,7 @@ rte_dpaa_bus_dev_build(void)
}
 
 #ifdef RTE_LIBRTE_DPAA_DEBUG_DRIVER
-   dump_netcfg(dpaa_netcfg);
+   dump_netcfg(dpaa_netcfg, stdout);
 #endif
 
DPAA_BUS_LOG(DEBUG, "Number of ethernet devices = %d",
diff --git a/drivers/bus/dpaa/include/netcfg.h 
b/drivers/bus/dpaa/include/netcfg.h
index 5bdcc9186a..ebbbaf6d10 100644
--- a/drivers/bus/dpaa/include/netcfg.h
+++ b/drivers/bus/dpaa/include/netcfg.h
@@ -60,7 +60,7 @@ void netcfg_release(struct netcfg_info *cfg_ptr);
 /* cfg_ptr: configuration information pointer.
  * This function dumps configuration data to stdout.
  */
-void dump_netcfg(struct netcfg_info *cfg_ptr);
+void dump_netcfg(struct netcfg_info *cfg_ptr, FILE *f);
 #endif
 
 #endif /* __NETCFG_H */
diff --git a/drivers/crypto/caam_jr/caam_jr.c b/drivers/crypto/caam_jr/caam_jr.c
index fb9ac9cb30..c836cc128a 100644
--- a/drivers/crypto/caam_jr/caam_jr.c
+++ b/drivers/crypto/caam_jr/caam_jr.c
@@ -1410,9 +1410,9 @@ caam_jr_enqueue_op(struct rte_crypto_op *op, struct 
caam_jr_qp *qp)
rte_pktmbuf_mtod(op->sym->m_src, void *),
rte_pktmbuf_data_len(op->sym->m_src));
 
-   printf("\n JD before conversion\n");
+   fprintf(stdout, "\n JD before conversion\n");
for (i = 0; i < 12; i++)
-   printf("\n 0x%08x", ctx->jobdes.desc[i]);
+   fprintf(stdout, "\n 0x%08x", ctx->jobdes.desc[i]);
 #endif
 
CAAM_JR_DP_DEBUG("Jr[%p] pi[%d] ci[%d].Before sending desc",
diff --git a/drivers/crypto/caam_jr/c

RE: [EXTERNAL] [PATCH] zsda:introduce zsda drivers and examples

[Akhil Goyal]

>  create mode 100644 examples/zsda/Makefile
>  create mode 100644 examples/zsda/commands.c
>  create mode 100644 examples/zsda/meson.build
>  create mode 100644 examples/zsda/test.c
>  create mode 100644 examples/zsda/test.h
>  create mode 100644 examples/zsda/test_zsda.c
>  create mode 100644 examples/zsda/test_zsda.h
>  create mode 100644 examples/zsda/test_zsda_compressdev.c
>  create mode 100644 examples/zsda/test_zsda_compressdev.h
>  create mode 100644 examples/zsda/test_zsda_cryptodev.c
>  create mode 100644 examples/zsda/test_zsda_cryptodev.h
>  create mode 100644 examples/zsda/test_zsda_cryptodev_aes_test_vectors.h
>  create mode 100644 examples/zsda/test_zsda_cryptodev_data.h
>  create mode 100644 examples/zsda/test_zsda_cryptodev_hash_test_vectors.h

Along with the comments from David, one more high level comment.

Why do you need to have a separate example application?
Can you align with app/test/test_cryptodev.c and app/test/test_compressdev.c?

[PATCH 1/2] app/proc-info: add memory heap dump

[Gagandeep Singh]

This patch add the heap dump support in proc-info
memory dump option.

Signed-off-by: Gagandeep Singh 
---
 app/proc-info/main.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index b672aaefbe..7137891c14 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -637,6 +638,10 @@ meminfo_display(void)
rte_memzone_dump(stdout);
printf("-- END_MEMORY_ZONES ---\n");
 
+   printf(" HEAP DUMP -\n");
+   rte_malloc_dump_heaps(stdout);
+   printf("-- END_HEAP_DUMP ---\n");
+
printf("- TAIL_QUEUES -\n");
rte_dump_tailq(stdout);
printf("-- END_TAIL_QUEUES \n");
-- 
2.25.1

[PATCH 2/2] eal: add total memory size in memory dump APIs

[Gagandeep Singh]

This patch add total memory size dump in memzone and
memsegments dump APIs.

Signed-off-by: Gagandeep Singh 
---
 lib/eal/common/eal_common_memory.c  |  2 ++
 lib/eal/common/eal_common_memzone.c | 18 --
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/lib/eal/common/eal_common_memory.c 
b/lib/eal/common/eal_common_memory.c
index 60ddc30580..c6b9c16617 100644
--- a/lib/eal/common/eal_common_memory.c
+++ b/lib/eal/common/eal_common_memory.c
@@ -531,6 +531,8 @@ void
 rte_dump_physmem_layout(FILE *f)
 {
rte_memseg_walk(dump_memseg, f);
+   fprintf(f, "Total Memory Segments size = %uM\n",
+   (unsigned int) rte_eal_get_physmem_size() / (1024 * 
1024));
 }
 
 static int
diff --git a/lib/eal/common/eal_common_memzone.c 
b/lib/eal/common/eal_common_memzone.c
index 32e6b78f87..1d12ccc443 100644
--- a/lib/eal/common/eal_common_memzone.c
+++ b/lib/eal/common/eal_common_memzone.c
@@ -58,6 +58,11 @@ rte_memzone_max_get(void)
return mcfg->max_memzone;
 }
 
+struct memzone_info {
+   FILE *f;
+   uint64_t t_size;
+};
+
 static inline const struct rte_memzone *
 memzone_lookup_thread_unsafe(const char *name)
 {
@@ -369,7 +374,8 @@ dump_memzone(const struct rte_memzone *mz, void *arg)
struct rte_memseg *ms;
int mz_idx, ms_idx;
size_t page_sz;
-   FILE *f = arg;
+   struct memzone_info *info = arg;
+   FILE *f = info->f;
 
mz_idx = rte_fbarray_find_idx(&mcfg->memzones, mz);
 
@@ -382,6 +388,7 @@ dump_memzone(const struct rte_memzone *mz, void *arg)
mz->socket_id,
mz->flags);
 
+   info->t_size += mz->len;
/* go through each page occupied by this memzone */
msl = rte_mem_virt2memseg_list(mz->addr);
if (!msl) {
@@ -414,7 +421,14 @@ dump_memzone(const struct rte_memzone *mz, void *arg)
 void
 rte_memzone_dump(FILE *f)
 {
-   rte_memzone_walk(dump_memzone, f);
+   struct memzone_info info;
+
+   memset(&info, 0, sizeof(info));
+   info.f = f;
+
+   rte_memzone_walk(dump_memzone, &info);
+   fprintf(f, "Total Memory Zones size = %uM\n", (unsigned int)info.t_size
+   / (1024 * 1024));
 }
 
 /*
-- 
2.25.1

Re: [PATCH v1] crypto/ipsec_mb: use new ipad/opad calculation API

[Patrick Robb]

Recheck-request: iol-unit-arm64-testing

Re: [PATCH 2/3] dts: add testpmd methods for test suite

[Nicholas Pratte]

On Wed, Jun 26, 2024 at 11:50 AM Jeremy Spewock  wrote:
>
> I think the subject line of this commit makes sense in the context of
> this series, but might be less helpful when the commit is applied to
> the git tree. For this reason I might favor changing it to something
> that briefly says what the added testpmd commands actually do.
>
> On Fri, Jun 21, 2024 at 1:23 PM Nicholas Pratte  wrote:
> >
> > Several methods are either imported from currently in-develoment test
> > suites, or they have been created specifically for this test suite.
> >
> > Methods here that can be found in other test suites include vlan
> > filtering, adding and removing vlan tags, and setting promiscuous mode.
> >
> > Methods that are specific to this test suite include adding or removing
> > mac addresses, and adding or removing multicast mac addresses.
>
> You probably could make the subject about the adding/modification of
> mac addresses judging by the body.
>

Good point. I'll make adjustments to the commit subject as well as the
commit messages.

> >
> > Bugzilla ID: 1454
> > Signed-off-by: Nicholas Pratte 
> > ---
> >  dts/framework/remote_session/testpmd_shell.py | 209 ++
> >  1 file changed, 209 insertions(+)
> >
> > diff --git a/dts/framework/remote_session/testpmd_shell.py 
> > b/dts/framework/remote_session/testpmd_shell.py
> > index ec22f72221..8a7d5905b3 100644
> > --- a/dts/framework/remote_session/testpmd_shell.py
> > +++ b/dts/framework/remote_session/testpmd_shell.py
> > @@ -767,6 +767,215 @@ def show_port_info(self, port_id: int) -> TestPmdPort:
> >
> >  return TestPmdPort.parse(output)
> >
> > +def set_mac_addr(self, port_id: int, mac_address: str, add: bool = 
> > True, verify: bool = True):
>
> I'm not sure it makes sense to default the `add` parameter in this
> method or the following. It seems to me like this method would likely
> be split pretty evenly between adding and removing and I can't think
> of a reason why one would be done in the general case over the other,
> so I think it would be more clear if we didn't default it.
>

Good point. It made sense for me to do it this way in a vacuum (since
I'm mostly adding addresses in each test case), but there's no good
reason it should be this way.

> > +"""Add or remove a mac address on a given port.
> > +
>
> Is it worth mentioning that we are adding the MAC address to the
> allowlist of the port?

Yes, I made a subtle change to the docstring.

>
> > +Args:
> > +port_id: The port ID the mac address is set on.
> > +mac_address: The mac address to be added or removed to the 
> > specified port.
> > +add: If :data:`True`, add the specified mac address. If 
> > :data:`False`, remove specified
> > +mac address.
> > +verify: If :data:'True', assert that the 'mac_addr' operation 
> > was successful. If
> > +:data:'False', run the command and skip this assertion.
> > +
> > +Raises:
> > +InteractiveCommandExecutionError: If either the 'add' or 
> > 'remove' operations fail.
>
> This error documentation confuses me a little because it sounds like
> both `add` and `remove` operations are happening in this method when
> really either one happens or the other happens. Maybe changing this to
> just saying the "mac address operation" failed would fix this.
>

Agreed. I made the change.

> > +"""
>
> I think something that could save you a lot of space in this method
> (and the next one) is creating a variable for what command to use if
> `add` is true or not.
> You could do something like this:
>
> mac_cmd = "add" if add else "remove"
>
> > +if add:
> > +output = self.send_command(f"mac_addr add {port_id} 
> > {mac_address}")
> > +else:
> > +output = self.send_command(f"mac_addr remove {port_id} 
> > {mac_address}")
>
> Then you don't need this if-statement because this just becomes:
> output = self.send_command(f"mac_addr {mac_cmd} {port_id} {mac_address}")
>

I love this! Good thinking. I'll make the changes.


> > +):
> > +self._logger.debug(f"Failed to add {multi_addr} on port 
> > {port_id}")
> > +raise InteractiveCommandExecutionError(
> > +f"Failed to add {multi_addr} on port {port_id} 
> > \n{output}"
> > +)
> > +if (
> > +not add
> > +and "Invalid multicast addr"
>
> Is this a typo or does testpmd remove the underscore when you're removing?

Not a typo. Good attention to detail though.


> > +
> > +def rx_vlan_rm(self, vlan: int, port: int, verify: bool = True):
> > +"""Remove specified vlan tag from filter list on a port.
> > +
> > +Args:
> > +vlan: The vlan tag to remove, should be within 1-4094.
> > +port: The port number to remove the tag from, should be within 
> > 0-32.
> > +verify: If :data

Re: [PATCH] net/ice: fix use of ice_bitmap_t in promisc functions

[Bruce Richardson]

On Tue, Jul 02, 2024 at 01:27:35PM +0100, Bruce Richardson wrote:
> On Tue, Jul 02, 2024 at 01:24:03PM +0100, Stokes, Ian wrote:
> > > Promisc functions were modified to use ice_bitmap_t.
> > > However use of ice_bitmap_t requires specific helper
> > > functions to ensure correctness.
> > > 
> > > Fix this by adding correct calls to declare, zero and set
> > > ice_bitmap_t within the promisc functions.
> > > 
> > > Signed-off-by: Ian Stokes 
> > 
> > Just to clarify, this patch is intended to be applied with the ice shared 
> > code update
> > which I believe is in net-next. It can either be added as an extra patch to 
> > that series
> > or merged with the original patch, whatever is preferred.
> > 
> > Thanks
> > Ian
> > 
> 
> I think this change should be squashed to commit [1], since it is part of
> the overall change from using bitmasks to actual bitmap type.
> 
> /Bruce
> 
> [1] 
> http://git.dpdk.org/next/dpdk-next-net-intel/commit/?id=126916f331dfa9fd6e997c0d00b1e29de9cc

Fix merged to appropriate commit on next-net-intel tree.

/Bruce

Re: [PATCH 1/3] dts: add boolean to adjust addresses

[Nicholas Pratte]

Yeah. I vaguely remember a conversation regarding the need and
validity of the 'adjust_addresses' functionality within DTS, going as
far as discussing whether it is needed or not, but maybe I'm wrong?
I'm honestly not sure.

I'll add the argument to the doc-string.

On Wed, Jun 26, 2024 at 11:49 AM Jeremy Spewock  wrote:
>
> This is funny because I actually ended up trying to solve the same
> problem when writing the dynamic queue test suite. We ended up taking
> different approaches, so we should probably have a discussion about
> the best way to handle this. Now that we have a few use cases for why
> this fix is needed, it will probably make the discussion easier since
> there is less speculation.
>
> On Fri, Jun 21, 2024 at 1:22 PM Nicholas Pratte  wrote:
> >
> > Various test cases in the mac filter test suite called for granular
> > manipulation of destination mac addresses to properly test mac address
> > filtering functionality. To compensate, there is now an
> > adjust_addresses boolean which the user can toggle if they wish to send
> > their own addressing; the boolean is true by default.
> >
> > Bugzilla ID: 1454
> > Signed-off-by: Nicholas Pratte 
> > ---
> >  dts/framework/test_suite.py | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
> > index 694b2eba65..5044d5f9bb 100644
> > --- a/dts/framework/test_suite.py
> > +++ b/dts/framework/test_suite.py
> > @@ -185,6 +185,7 @@ def send_packet_and_capture(
> >  packet: Packet,
> >  filter_config: PacketFilteringConfig = PacketFilteringConfig(),
> >  duration: float = 1,
> > +adjust_addresses: bool = True,
>
> This should probably get added to the Args section of this doc-string
> since it's a public method.
>
>
>
> >  ) -> list[Packet]:
> >  """Send and receive `packet` using the associated TG.
> >
> > @@ -199,7 +200,8 @@ def send_packet_and_capture(
> >  Returns:
> >  A list of received packets.
> >  """
> > -packet = self._adjust_addresses(packet)
> > +if adjust_addresses:
> > +packet = self._adjust_addresses(packet)
> >  return self.tg_node.send_packet_and_capture(
> >  packet,
> >  self._tg_port_egress,
> > --
> > 2.44.0
> >

Re: [PATCH] app/eventdev: increase default queue depth for cryptodev

[Jerin Jacob]

On Thu, Jun 27, 2024 at 5:39 PM Gujjar, Abhinandan S
 wrote:
>
> Acked-by: Abhinandan Gujjar 

Applied to dpdk-next-eventdev/for-main. Thanks



>
> > -Original Message-
> > From: Aakash Sasidharan 
> > Sent: Thursday, June 27, 2024 10:45 AM
> > To: Jerin Jacob 
> > Cc: gak...@marvell.com; ano...@marvell.com; vvelum...@marvell.com;
> > asasidha...@marvell.com; Gujjar, Abhinandan S
> > ; dev@dpdk.org
> > Subject: [PATCH] app/eventdev: increase default queue depth for cryptodev
> >
> > With crypto adapter, larger queue depths are desirable since same queue 
> > could
> > be used from multiple cores at the same time. With devices that are capable 
> > of
> > doing large bursts, larger queues would help in multi core management of 
> > same
> > queue.
> >
> > Increase default queue depth in cryptodev to cater to such use cases.
> >
> > Signed-off-by: Aakash Sasidharan 
> > ---
> >  app/test-eventdev/test_perf_common.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/app/test-eventdev/test_perf_common.c b/app/test-
> > eventdev/test_perf_common.c
> > index db0f9c1f3b..66d22cd559 100644
> > --- a/app/test-eventdev/test_perf_common.c
> > +++ b/app/test-eventdev/test_perf_common.c
> > @@ -6,7 +6,7 @@
> >
> >  #include "test_perf_common.h"
> >
> > -#define NB_CRYPTODEV_DESCRIPTORS 1024
> > +#define NB_CRYPTODEV_DESCRIPTORS 4096
> >  #define DATA_SIZE512
> >  #define IV_OFFSET (sizeof(struct rte_crypto_op) + \
> >  sizeof(struct rte_crypto_sym_op) + \
> > --
> > 2.25.1
>

Re: [PATCH 2/2] net/cnxk: avoid NPC Rx and MCAM entries disable

[Jerin Jacob]

On Thu, Jun 27, 2024 at 2:47 PM Rahul Bhansali  wrote:
>
> For inline IPsec, Rx misses are observed during dev stop process.
> There can be a situation of 2nd pass packets are being dropped and
> can cause a buffer leak.
> To handle such case, will avoid NPC Rx and MCAM entries disable in
> dev stop. These will be handled in dev close routine.
>
> Signed-off-by: Rahul Bhansali 

Since it is a fix, Please send the next version with Fixes: tag and
update git commit to make as fix.


> ---
>  drivers/net/cnxk/cnxk_ethdev.c | 27 ---
>  1 file changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/cnxk/cnxk_ethdev.c b/drivers/net/cnxk/cnxk_ethdev.c
> index db8feca620..38746c81c5 100644
> --- a/drivers/net/cnxk/cnxk_ethdev.c
> +++ b/drivers/net/cnxk/cnxk_ethdev.c
> @@ -1625,18 +1625,26 @@ cnxk_nix_dev_stop(struct rte_eth_dev *eth_dev)
> int count, i, j, rc;
> void *rxq;
>
> -   /* Disable all the NPC entries */
> -   rc = roc_npc_mcam_enable_all_entries(&dev->npc, 0);
> -   if (rc)
> -   return rc;
> +   /* In case of Inline IPSec, will need to avoid disabling the MCAM 
> rules and NPC Rx
> +* in this routine to continue processing of second pass inflight 
> packets if any.
> +* Drop of second pass packets will leak first pass buffers on some 
> platforms
> +* due to hardware limitations.
> +*/
> +   if (roc_feature_nix_has_second_pass_drop() ||
> +   !(dev->rx_offloads & RTE_ETH_RX_OFFLOAD_SECURITY)) {
> +   /* Disable all the NPC entries */
> +   rc = roc_npc_mcam_enable_all_entries(&dev->npc, 0);
> +   if (rc)
> +   return rc;
> +
> +   /* Disable Rx via NPC */
> +   roc_nix_npc_rx_ena_dis(&dev->nix, false);
> +   }
>
> /* Stop link change events */
> if (!roc_nix_is_vf_or_sdp(&dev->nix))
> roc_nix_mac_link_event_start_stop(&dev->nix, false);
>
> -   /* Disable Rx via NPC */
> -   roc_nix_npc_rx_ena_dis(&dev->nix, false);
> -
> roc_nix_inl_outb_soft_exp_poll_switch(&dev->nix, false);
>
> /* Stop inline device RQ first */
> @@ -2047,6 +2055,11 @@ cnxk_eth_dev_uninit(struct rte_eth_dev *eth_dev, bool 
> reset)
> /* Clear the flag since we are closing down */
> dev->configured = 0;
>
> +   /* Disable all the NPC entries */
> +   rc = roc_npc_mcam_enable_all_entries(&dev->npc, 0);
> +   if (rc)
> +   return rc;
> +
> roc_nix_npc_rx_ena_dis(nix, false);
>
> /* Restore 802.3 Flow control configuration */
> --
> 2.25.1
>

[DPDK/testpmd Bug 1479] mlx5: Not able to create rte_flows to match head fragments and sub fragments

[bugzilla]

https://bugs.dpdk.org/show_bug.cgi?id=1479

Bug ID: 1479
   Summary: mlx5: Not able to create rte_flows to match head
fragments and sub fragments
   Product: DPDK
   Version: 21.11
  Hardware: All
OS: All
Status: UNCONFIRMED
  Severity: major
  Priority: Normal
 Component: testpmd
  Assignee: dev@dpdk.org
  Reporter: pingtos...@gmail.com
  Target Milestone: ---

I am trying to create an RTE flow rule to match head and non-head fragments to
compute NIC RSS based on 5tuple/3tuple respectively on connectX-6 DX NIC and
mlx5 driver. 

As part of it when trying to install RTE flow rule using testpmd on dpdk
21.11/23.07 version, the following errors are thrown.

Flow rule to match head fragment
=
testpmd>  flow create 0 ingress pattern eth / ipv4 fragment_offset spec 0x2000
fragment_offset mask 0x3fff / end actions drop / count / end
port_flow_complain(): Caught PMD error type 13 (specific pattern item): cause:
0x7ffd1954c548, match on first fragment not supported: Operation not supported

Flow rule to match non-head fragments
==
testpmd> flow validate 0 ingress pattern eth / ipv4 fragment_offset is 0x2001
fragment_offset last 0x1fff / end actions drop / end
port_flow_complain(): Caught PMD error type 11 (item specification range):
cause: 0x7ffc6f629534, specified range not supported: Operation not supported

When I browsed mlx5_flow_dv.c driver file, there are set of conditions
implemented to block this configurations.

Could you kindly help is there a way to compute different RSS hash for
fragments and non-fragments using RTE flow rules on mellanox?

Thanks! in advance.

   /*
 * Match on fragment_offset 0x2000 means MF is 1 and frag-offset is 0,
 * indicating this is 1st fragment of fragmented packet.
 * This is not yet supported in MLX5, return appropriate error message.
 */
if (fragment_offset_spec == RTE_BE16(RTE_IPV4_HDR_MF_FLAG))
return rte_flow_error_set(error, ENOTSUP,
  RTE_FLOW_ERROR_TYPE_ITEM, item,
  "match on first fragment not "
  "supported");
if (fragment_offset_spec && !last)
return rte_flow_error_set(error, ENOTSUP,
  RTE_FLOW_ERROR_TYPE_ITEM, item,
  "specified value not supported");
/*
 * Match on fragment_offset spec 0x2001 and last 0x3fff
 * means MF is 1 and frag-offset is > 0.
 * This packet is fragment 2nd and onward, excluding last.
 * This is not yet supported in MLX5, return appropriate
 * error message.
 */
if (fragment_offset_spec == RTE_BE16(RTE_IPV4_HDR_MF_FLAG + 1) &&
fragment_offset_last == RTE_BE16(MLX5_IPV4_FRAG_OFFSET_MASK))
return rte_flow_error_set(error, ENOTSUP,
  RTE_FLOW_ERROR_TYPE_ITEM_LAST,
  last, "match on following "
  "fragments not supported");
/*
 * Match on fragment_offset spec 0x0001 and last 0x1fff
 * means MF is 0 and frag-offset is > 0.
 * This packet is last fragment of fragmented packet.
 * This is not yet supported in MLX5, return appropriate
 * error message.
 */
if (fragment_offset_spec == RTE_BE16(1) &&
fragment_offset_last == RTE_BE16(RTE_IPV4_HDR_OFFSET_MASK))
return rte_flow_error_set(error, ENOTSUP,
  RTE_FLOW_ERROR_TYPE_ITEM_LAST,
  last, "match on last "
  "fragment not supported");

-- 
You are receiving this mail because:
You are the assignee for the bug.

[PATCH v2 1/2] common/cnxk: enable second pass RQ in mask config

[Rahul Bhansali]

This will enable second pass RQ and drop interrupt by default in
mask configuration to avoid buffer leak possibilities during dev
stop and interrupts to indicate drops if any.

Signed-off-by: Rahul Bhansali 
---
Changes in v2: No change.

 drivers/common/cnxk/roc_features.h | 6 ++
 drivers/common/cnxk/roc_nix_inl.c  | 9 -
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/common/cnxk/roc_features.h 
b/drivers/common/cnxk/roc_features.h
index 3b512be132..6abb35c296 100644
--- a/drivers/common/cnxk/roc_features.h
+++ b/drivers/common/cnxk/roc_features.h
@@ -90,4 +90,10 @@ roc_feature_nix_has_rx_inject(void)
return (roc_model_is_cn10ka_b0() || roc_model_is_cn10kb());
 }

+static inline bool
+roc_feature_nix_has_second_pass_drop(void)
+{
+   return 0;
+}
+
 #endif
diff --git a/drivers/common/cnxk/roc_nix_inl.c 
b/drivers/common/cnxk/roc_nix_inl.c
index 74a688abbd..a984ac56d9 100644
--- a/drivers/common/cnxk/roc_nix_inl.c
+++ b/drivers/common/cnxk/roc_nix_inl.c
@@ -734,6 +734,13 @@ nix_inl_rq_mask_cfg(struct roc_nix *roc_nix, bool enable)
msk_req->rq_set.xqe_drop_ena = 0;
msk_req->rq_set.spb_ena = 1;

+   if (!roc_feature_nix_has_second_pass_drop()) {
+   msk_req->rq_set.ena = 1;
+   msk_req->rq_set.rq_int_ena = 1;
+   msk_req->rq_mask.ena = 0;
+   msk_req->rq_mask.rq_int_ena = 0;
+   }
+
msk_req->rq_mask.len_ol3_dis = 0;
msk_req->rq_mask.len_ol4_dis = 0;
msk_req->rq_mask.len_il3_dis = 0;
@@ -1467,7 +1474,7 @@ roc_nix_inl_rq_ena_dis(struct roc_nix *roc_nix, bool 
enable)
if (!idev)
return -EFAULT;

-   if (roc_feature_nix_has_inl_rq_mask()) {
+   if (roc_feature_nix_has_inl_rq_mask() && enable) {
rc = nix_inl_rq_mask_cfg(roc_nix, enable);
if (rc) {
plt_err("Failed to get rq mask rc=%d", rc);
--
2.25.1

[PATCH v2 2/2] net/cnxk: fix to avoid NPC Rx and MCAM disable

[Rahul Bhansali]

For inline IPsec, Rx misses are observed during dev stop process.
There can be a situation of 2nd pass packets are being dropped and
can cause a buffer leak.
To handle such case, will avoid NPC Rx and MCAM entries disable in
dev stop. These will be handled in dev close routine.

Fixes: fbc0fa749919 ("net/cnxk: keep flow rules across restart")

Signed-off-by: Rahul Bhansali 
---
Changes in v2: added fixes tag

 drivers/net/cnxk/cnxk_ethdev.c | 27 ---
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/net/cnxk/cnxk_ethdev.c b/drivers/net/cnxk/cnxk_ethdev.c
index db8feca620..38746c81c5 100644
--- a/drivers/net/cnxk/cnxk_ethdev.c
+++ b/drivers/net/cnxk/cnxk_ethdev.c
@@ -1625,18 +1625,26 @@ cnxk_nix_dev_stop(struct rte_eth_dev *eth_dev)
int count, i, j, rc;
void *rxq;

-   /* Disable all the NPC entries */
-   rc = roc_npc_mcam_enable_all_entries(&dev->npc, 0);
-   if (rc)
-   return rc;
+   /* In case of Inline IPSec, will need to avoid disabling the MCAM rules 
and NPC Rx
+* in this routine to continue processing of second pass inflight 
packets if any.
+* Drop of second pass packets will leak first pass buffers on some 
platforms
+* due to hardware limitations.
+*/
+   if (roc_feature_nix_has_second_pass_drop() ||
+   !(dev->rx_offloads & RTE_ETH_RX_OFFLOAD_SECURITY)) {
+   /* Disable all the NPC entries */
+   rc = roc_npc_mcam_enable_all_entries(&dev->npc, 0);
+   if (rc)
+   return rc;
+
+   /* Disable Rx via NPC */
+   roc_nix_npc_rx_ena_dis(&dev->nix, false);
+   }

/* Stop link change events */
if (!roc_nix_is_vf_or_sdp(&dev->nix))
roc_nix_mac_link_event_start_stop(&dev->nix, false);

-   /* Disable Rx via NPC */
-   roc_nix_npc_rx_ena_dis(&dev->nix, false);
-
roc_nix_inl_outb_soft_exp_poll_switch(&dev->nix, false);

/* Stop inline device RQ first */
@@ -2047,6 +2055,11 @@ cnxk_eth_dev_uninit(struct rte_eth_dev *eth_dev, bool 
reset)
/* Clear the flag since we are closing down */
dev->configured = 0;

+   /* Disable all the NPC entries */
+   rc = roc_npc_mcam_enable_all_entries(&dev->npc, 0);
+   if (rc)
+   return rc;
+
roc_nix_npc_rx_ena_dis(nix, false);

/* Restore 802.3 Flow control configuration */
--
2.25.1

[PATCH 01/15] net/ena/base: add descriptor dump capability

[shaibran]

From: Shai Brandes 

This patch adds the capability to print rx/tx descriptors.
This patch introduces a new function ena_com_tx_cdesc_idx_to_ptr
which is the equivalent of ena_com_rx_cdesc_idx_to_ptr but for tx cdesc.

Finally, this patch moves the io_cq header incrementation in
ena_com_cdesc_rx_pkt_get() function to be after the possible if fail
cases since it makes more sense to point into the next descriptor only
if the last one is valid.

Signed-off-by: Shai Brandes 
---
 drivers/net/ena/base/ena_eth_com.c | 56 +++---
 drivers/net/ena/base/ena_eth_com.h |  8 +
 2 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ena/base/ena_eth_com.c 
b/drivers/net/ena/base/ena_eth_com.c
index 0de736fbe0..9ba0beb868 100644
--- a/drivers/net/ena/base/ena_eth_com.c
+++ b/drivers/net/ena/base/ena_eth_com.c
@@ -5,8 +5,7 @@
 
 #include "ena_eth_com.h"
 
-static struct ena_eth_io_rx_cdesc_base *ena_com_get_next_rx_cdesc(
-   struct ena_com_io_cq *io_cq)
+struct ena_eth_io_rx_cdesc_base *ena_com_get_next_rx_cdesc(struct 
ena_com_io_cq *io_cq)
 {
struct ena_eth_io_rx_cdesc_base *cdesc;
u16 expected_phase, head_masked;
@@ -32,6 +31,55 @@ static struct ena_eth_io_rx_cdesc_base 
*ena_com_get_next_rx_cdesc(
return cdesc;
 }
 
+void ena_com_dump_single_rx_cdesc(struct ena_com_io_cq *io_cq,
+ struct ena_eth_io_rx_cdesc_base *desc)
+{
+   if (desc) {
+   uint32_t *desc_arr = (uint32_t *)desc;
+
+   ena_trc_err(ena_com_io_cq_to_ena_dev(io_cq),
+   "RX descriptor value[0x%08x 0x%08x 0x%08x 0x%08x] 
phase[%u] first[%u] last[%u] MBZ7[%u] MZB17[%u]\n",
+   desc_arr[0], desc_arr[1], desc_arr[2], desc_arr[3],
+   ENA_FIELD_GET(desc->status, 
(uint32_t)ENA_ETH_IO_RX_DESC_PHASE_MASK,
+ 0),
+   ENA_FIELD_GET(desc->status, 
(uint32_t)ENA_ETH_IO_RX_DESC_FIRST_MASK,
+ ENA_ETH_IO_RX_DESC_FIRST_SHIFT),
+   ENA_FIELD_GET(desc->status, 
(uint32_t)ENA_ETH_IO_RX_DESC_LAST_MASK,
+ ENA_ETH_IO_RX_DESC_LAST_SHIFT),
+   ENA_FIELD_GET(desc->status,
+ 
(uint32_t)ENA_ETH_IO_RX_CDESC_BASE_MBZ7_MASK,
+ ENA_ETH_IO_RX_CDESC_BASE_MBZ7_SHIFT),
+   ENA_FIELD_GET(desc->status,
+ 
(uint32_t)ENA_ETH_IO_RX_CDESC_BASE_MBZ17_MASK,
+ 
ENA_ETH_IO_RX_CDESC_BASE_MBZ17_SHIFT));
+   }
+}
+
+void ena_com_dump_single_tx_cdesc(struct ena_com_io_cq *io_cq,
+ struct ena_eth_io_tx_cdesc *desc)
+{
+   if (desc) {
+   uint32_t *desc_arr = (uint32_t *)desc;
+
+   ena_trc_err(ena_com_io_cq_to_ena_dev(io_cq),
+   "TX descriptor value[0x%08x 0x%08x] phase[%u] 
MBZ6[%u]\n",
+   desc_arr[0], desc_arr[1],
+   ENA_FIELD_GET(desc->flags, 
(uint32_t)ENA_ETH_IO_TX_CDESC_PHASE_MASK,
+ 0),
+   ENA_FIELD_GET(desc->flags, 
(uint32_t)ENA_ETH_IO_TX_CDESC_MBZ6_MASK,
+ ENA_ETH_IO_TX_CDESC_MBZ6_SHIFT));
+   }
+}
+
+struct ena_eth_io_tx_cdesc *ena_com_tx_cdesc_idx_to_ptr(struct ena_com_io_cq 
*io_cq, u16 idx)
+{
+   idx &= (io_cq->q_depth - 1);
+
+   return (struct ena_eth_io_tx_cdesc *)
+   ((uintptr_t)io_cq->cdesc_addr.virt_addr +
+   idx * io_cq->cdesc_entry_size_in_bytes);
+}
+
 static void *get_sq_desc_regular_queue(struct ena_com_io_sq *io_sq)
 {
u16 tail_masked;
@@ -228,7 +276,7 @@ static int ena_com_sq_update_tail(struct ena_com_io_sq 
*io_sq)
return ena_com_sq_update_reqular_queue_tail(io_sq);
 }
 
-static struct ena_eth_io_rx_cdesc_base *
+struct ena_eth_io_rx_cdesc_base *
ena_com_rx_cdesc_idx_to_ptr(struct ena_com_io_cq *io_cq, u16 idx)
 {
idx &= (io_cq->q_depth - 1);
@@ -254,7 +302,6 @@ static int ena_com_cdesc_rx_pkt_get(struct ena_com_io_cq 
*io_cq,
break;
status = READ_ONCE32(cdesc->status);
 
-   ena_com_cq_inc_head(io_cq);
if (unlikely((status & ENA_ETH_IO_RX_CDESC_BASE_FIRST_MASK) >>
ENA_ETH_IO_RX_CDESC_BASE_FIRST_SHIFT && count != 0)) {
ena_trc_err(dev,
@@ -272,6 +319,7 @@ static int ena_com_cdesc_rx_pkt_get(struct ena_com_io_cq 
*io_cq,
return ENA_COM_FAULT;
}
 
+   ena_com_cq_inc_head(io_cq);
count++;
last = (status & ENA_ETH_IO_RX_CDESC_BASE_LAST_MASK) >>
ENA_ETH_IO_RX_CDESC_BASE_LAST_SHIFT

[PATCH 02/15] net/ena/base: remove unused param

[shaibran]

From: Shai Brandes 

Remove an unused dev_node parameter when allocating DMA memory.

Signed-off-by: Shai Brandes 
---
 drivers/net/ena/base/ena_com.c   |  9 ++---
 drivers/net/ena/base/ena_plat_dpdk.h | 11 +--
 2 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ena/base/ena_com.c b/drivers/net/ena/base/ena_com.c
index 24756e5e76..9e1fa40c0c 100644
--- a/drivers/net/ena/base/ena_com.c
+++ b/drivers/net/ena/base/ena_com.c
@@ -328,7 +328,6 @@ static int ena_com_init_io_sq(struct ena_com_dev *ena_dev,
  struct ena_com_io_sq *io_sq)
 {
size_t size;
-   int dev_node = 0;
 
memset(&io_sq->desc_addr, 0x0, sizeof(io_sq->desc_addr));
 
@@ -347,8 +346,7 @@ static int ena_com_init_io_sq(struct ena_com_dev *ena_dev,
io_sq->desc_addr.virt_addr,
io_sq->desc_addr.phys_addr,
io_sq->desc_addr.mem_handle,
-   ctx->numa_node,
-   dev_node);
+   ctx->numa_node);
if (!io_sq->desc_addr.virt_addr) {
ENA_MEM_ALLOC_COHERENT(ena_dev->dmadev,
   size,
@@ -377,8 +375,7 @@ static int ena_com_init_io_sq(struct ena_com_dev *ena_dev,
ENA_MEM_ALLOC_NODE(ena_dev->dmadev,
   size,
   io_sq->bounce_buf_ctrl.base_buffer,
-  ctx->numa_node,
-  dev_node);
+  ctx->numa_node);
if (!io_sq->bounce_buf_ctrl.base_buffer)
io_sq->bounce_buf_ctrl.base_buffer = 
ENA_MEM_ALLOC(ena_dev->dmadev, size);
 
@@ -417,7 +414,6 @@ static int ena_com_init_io_cq(struct ena_com_dev *ena_dev,
  struct ena_com_io_cq *io_cq)
 {
size_t size;
-   int prev_node = 0;
 
memset(&io_cq->cdesc_addr, 0x0, sizeof(io_cq->cdesc_addr));
 
@@ -436,7 +432,6 @@ static int ena_com_init_io_cq(struct ena_com_dev *ena_dev,
io_cq->cdesc_addr.phys_addr,
io_cq->cdesc_addr.mem_handle,
ctx->numa_node,
-   prev_node,
ENA_CDESC_RING_SIZE_ALIGNMENT);
if (!io_cq->cdesc_addr.virt_addr) {
ENA_MEM_ALLOC_COHERENT_ALIGNED(ena_dev->dmadev,
diff --git a/drivers/net/ena/base/ena_plat_dpdk.h 
b/drivers/net/ena/base/ena_plat_dpdk.h
index dffe60705d..eaa509bc2a 100644
--- a/drivers/net/ena/base/ena_plat_dpdk.h
+++ b/drivers/net/ena/base/ena_plat_dpdk.h
@@ -240,23 +240,22 @@ ena_mem_alloc_coherent(struct rte_eth_dev_data *data, 
size_t size,
   rte_memzone_free(mem_handle); })
 
 #define ENA_MEM_ALLOC_COHERENT_NODE_ALIGNED(  \
-   dmadev, size, virt, phys, mem_handle, node, dev_node, alignment)   \
+   dmadev, size, virt, phys, mem_handle, node, alignment)  
\
do {   \
void *virt_addr;   \
dma_addr_t phys_addr;  \
-   ENA_TOUCH(dev_node);   \
(mem_handle) = ena_mem_alloc_coherent((dmadev), (size),\
(node), (alignment), &virt_addr, &phys_addr);  \
(virt) = virt_addr;\
(phys) = phys_addr;\
} while (0)
 #define ENA_MEM_ALLOC_COHERENT_NODE(  \
-   dmadev, size, virt, phys, mem_handle, node, dev_node)  \
+   dmadev, size, virt, phys, mem_handle, node) 
\
ENA_MEM_ALLOC_COHERENT_NODE_ALIGNED(dmadev, size, virt, phys,  \
-   mem_handle, node, dev_node, RTE_CACHE_LINE_SIZE)
-#define ENA_MEM_ALLOC_NODE(dmadev, size, virt, node, dev_node)\
+   mem_handle, node, RTE_CACHE_LINE_SIZE)
+#define ENA_MEM_ALLOC_NODE(dmadev, size, virt, node)   
\
do {   \
-   ENA_TOUCH(dmadev); ENA_TOUCH(dev_node);\
+   ENA_TOUCH(dmadev);  
\
virt = rte_zmalloc_socket(NULL, size, 0, node);\
} while (0)
 
-- 
2.17.1

[PATCH 00/15] net/ena: driver release 2.10.0

[shaibran]

From: Shai Brandes 

Hi all,
This release contains:
1. HAL upgrade to latest divided into separate patches.
2. three bug fixes.
3. restructuring of the Rx checksum code for readability
4. restructuring of the device uninit flow which is also needed
   for adding hot un/plug support later.
5. modification of logs on the datapath that leads to improved performance.

Will appreciate your feedbacks!
All the best and thanks in advance,
Shai

Shai Brandes (15):
  net/ena/base: add descriptor dump capability
  net/ena/base: remove unused param
  net/ena/base: remove redundant assert checks
  net/ena/base: update memory barrier comment
  net/ena/base: add method to check used entries
  net/ena/base: add an additional reset reason
  net/ena/base: update copyrights comments
  net/ena/base: add macro for bitfield access
  net/ena: logger change to improve performance
  net/ena: rework device uninit
  net/ena: fix bad checksum handling
  net/ena: fix invalid return value check
  net/ena: fix wrong handling of checksum
  net/ena: rework Rx checksum inspection
  net/ena: upgrade driver version to 2.10.0

 doc/guides/rel_notes/release_24_07.rst|  11 +
 drivers/net/ena/base/ena_com.c| 167 +++--
 drivers/net/ena/base/ena_com.h|  14 +-
 .../net/ena/base/ena_defs/ena_admin_defs.h|   5 +-
 .../net/ena/base/ena_defs/ena_common_defs.h   |   4 +-
 .../net/ena/base/ena_defs/ena_eth_io_defs.h   |   5 +-
 drivers/net/ena/base/ena_defs/ena_includes.h  |   4 +-
 drivers/net/ena/base/ena_defs/ena_regs_defs.h |   5 +-
 drivers/net/ena/base/ena_eth_com.c| 231 +++---
 drivers/net/ena/base/ena_eth_com.h|  33 ++-
 drivers/net/ena/base/ena_plat.h   |   4 +-
 drivers/net/ena/base/ena_plat_dpdk.h  |  24 +-
 drivers/net/ena/ena_ethdev.c  | 134 +-
 13 files changed, 374 insertions(+), 267 deletions(-)

-- 
2.17.1

[PATCH 04/15] net/ena/base: update memory barrier comment

[shaibran]

From: Shai Brandes 

Update the comment above the phase bit descriptor
read in AENQ processing.

Signed-off-by: Shai Brandes 
---
 drivers/net/ena/base/ena_com.c | 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ena/base/ena_com.c b/drivers/net/ena/base/ena_com.c
index f9dd086484..ad4f3f9431 100644
--- a/drivers/net/ena/base/ena_com.c
+++ b/drivers/net/ena/base/ena_com.c
@@ -2409,8 +2409,12 @@ void ena_com_aenq_intr_handler(struct ena_com_dev 
*ena_dev, void *data)
/* Go over all the events */
while ((READ_ONCE8(aenq_common->flags) &
ENA_ADMIN_AENQ_COMMON_DESC_PHASE_MASK) == phase) {
-   /* Make sure the device finished writing the rest of the 
descriptor
-* before reading it.
+   /* When the phase bit of the AENQ descriptor aligns with the 
driver's phase bit,
+* it signifies the readiness of the entire AENQ descriptor.
+* The driver should proceed to read the descriptor's data only 
after confirming
+* and synchronizing the phase bit.
+* This memory fence guarantees the correct sequence of 
accesses to the
+* descriptor's memory.
 */
dma_rmb();
 
@@ -2468,8 +2472,12 @@ bool ena_com_aenq_has_keep_alive(struct ena_com_dev 
*ena_dev)
/* Go over all the events */
while ((READ_ONCE8(aenq_common->flags) &
ENA_ADMIN_AENQ_COMMON_DESC_PHASE_MASK) == phase) {
-   /* Make sure the device finished writing the rest of the 
descriptor
-* before reading it.
+   /* When the phase bit of the AENQ descriptor aligns with the 
driver's phase bit,
+* it signifies the readiness of the entire AENQ descriptor.
+* The driver should proceed to read the descriptor's data only 
after confirming
+* and synchronizing the phase bit.
+* This memory fence guarantees the correct sequence of 
accesses to the
+* descriptor's memory.
 */
dma_rmb();
 
-- 
2.17.1

[PATCH 03/15] net/ena/base: remove redundant assert checks

[shaibran]

From: Shai Brandes 

Remove ENA_WARN checks from ena_com_wait_and_process_admin_cq_polling
since once the execution flow reaches the check, it must be
ENA_CMD_COMPLETED because it can't be either of the other options:
1. ENA_CMD_ABORTED - in such case it will perform "goto err" in the "if"
   block above, thus skipping the ENA_WARN check.
2. ENA_CMD_SUBMITTED - in such case it will timeout inside the while(1)
   loop above and perform "goto err", thus skipping the ENA_WARN check.

Remove ENA_WARN check from ena_com_wait_and_process_admin_cq_interrupts
since once the execution flow reaches the check, it must be
ENA_CMD_COMPLETED because it can't be either of the other options:
1. ENA_CMD_ABORTED - same as above, i will perform "goto err" in the
   "if" block above, thus skipping the ENA_WARN check.
2. ENA_CMD_SUBMITTED - in such case it will perform "goto err" in the
   nested if block above, since "admin_queue->polling" is false (because
   of the interrupt mode execution of admin commands)

Signed-off-by: Shai Brandes 
---
 drivers/net/ena/base/ena_com.c | 8 
 1 file changed, 8 deletions(-)

diff --git a/drivers/net/ena/base/ena_com.c b/drivers/net/ena/base/ena_com.c
index 9e1fa40c0c..f9dd086484 100644
--- a/drivers/net/ena/base/ena_com.c
+++ b/drivers/net/ena/base/ena_com.c
@@ -598,10 +598,6 @@ static int 
ena_com_wait_and_process_admin_cq_polling(struct ena_comp_ctx *comp_c
goto err;
}
 
-   ENA_WARN(comp_ctx->status != ENA_CMD_COMPLETED,
-admin_queue->ena_dev, "Invalid comp status %d\n",
-comp_ctx->status);
-
ret = ena_com_comp_status_to_errno(admin_queue, comp_ctx->comp_status);
 err:
comp_ctxt_release(admin_queue, comp_ctx);
@@ -828,10 +824,6 @@ static int 
ena_com_wait_and_process_admin_cq_interrupts(struct ena_comp_ctx *com
goto err;
}
 
-   ENA_WARN(comp_ctx->status != ENA_CMD_COMPLETED,
-admin_queue->ena_dev, "Invalid comp status %d\n",
-comp_ctx->status);
-
ret = ena_com_comp_status_to_errno(admin_queue, comp_ctx->comp_status);
 err:
comp_ctxt_release(admin_queue, comp_ctx);
-- 
2.17.1

[PATCH 05/15] net/ena/base: add method to check used entries

[shaibran]

From: Shai Brandes 

Provide a method to check the number of used entries in the send queue

Signed-off-by: Shai Brandes 
---
 drivers/net/ena/base/ena_eth_com.h | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ena/base/ena_eth_com.h 
b/drivers/net/ena/base/ena_eth_com.h
index 4e3d0fb6fd..877668612c 100644
--- a/drivers/net/ena/base/ena_eth_com.h
+++ b/drivers/net/ena/base/ena_eth_com.h
@@ -82,15 +82,14 @@ static inline void ena_com_unmask_intr(struct ena_com_io_cq 
*io_cq,
ENA_REG_WRITE32(io_cq->bus, intr_reg->intr_control, io_cq->unmask_reg);
 }
 
-static inline int ena_com_free_q_entries(struct ena_com_io_sq *io_sq)
+static inline u16 ena_com_used_q_entries(struct ena_com_io_sq *io_sq)
 {
-   u16 tail, next_to_comp, cnt;
-
-   next_to_comp = io_sq->next_to_comp;
-   tail = io_sq->tail;
-   cnt = tail - next_to_comp;
+   return io_sq->tail - io_sq->next_to_comp;
+}
 
-   return io_sq->q_depth - 1 - cnt;
+static inline int ena_com_free_q_entries(struct ena_com_io_sq *io_sq)
+{
+   return io_sq->q_depth - 1 - ena_com_used_q_entries(io_sq);
 }
 
 /* Check if the submission queue has enough space to hold required_buffers */
-- 
2.17.1

[PATCH 07/15] net/ena/base: update copyrights comments

[shaibran]

From: Shai Brandes 

copyright dates are not mandatory to be maintained,
therefore the range of years was removed.
In addition, the copyrights lines were separated
into two comments.

Signed-off-by: Shai Brandes 
---
 drivers/net/ena/base/ena_com.c  | 4 ++--
 drivers/net/ena/base/ena_com.h  | 4 ++--
 drivers/net/ena/base/ena_defs/ena_admin_defs.h  | 5 +++--
 drivers/net/ena/base/ena_defs/ena_common_defs.h | 4 ++--
 drivers/net/ena/base/ena_defs/ena_eth_io_defs.h | 5 +++--
 drivers/net/ena/base/ena_defs/ena_includes.h| 4 ++--
 drivers/net/ena/base/ena_defs/ena_regs_defs.h   | 4 ++--
 drivers/net/ena/base/ena_eth_com.c  | 4 ++--
 drivers/net/ena/base/ena_eth_com.h  | 4 ++--
 drivers/net/ena/base/ena_plat.h | 4 ++--
 drivers/net/ena/base/ena_plat_dpdk.h| 4 ++--
 11 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ena/base/ena_com.c b/drivers/net/ena/base/ena_com.c
index ad4f3f9431..5f46e692b3 100644
--- a/drivers/net/ena/base/ena_com.c
+++ b/drivers/net/ena/base/ena_com.c
@@ -1,5 +1,5 @@
-/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright (c) 2015-2020 Amazon.com, Inc. or its affiliates.
+/* SPDX-License-Identifier: BSD-3-Clause */
+/* Copyright (c) Amazon.com, Inc. or its affiliates.
  * All rights reserved.
  */
 
diff --git a/drivers/net/ena/base/ena_com.h b/drivers/net/ena/base/ena_com.h
index 737747f64b..5d38b69c0f 100644
--- a/drivers/net/ena/base/ena_com.h
+++ b/drivers/net/ena/base/ena_com.h
@@ -1,5 +1,5 @@
-/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright (c) 2015-2020 Amazon.com, Inc. or its affiliates.
+/* SPDX-License-Identifier: BSD-3-Clause */
+/* Copyright (c) Amazon.com, Inc. or its affiliates.
  * All rights reserved.
  */
 
diff --git a/drivers/net/ena/base/ena_defs/ena_admin_defs.h 
b/drivers/net/ena/base/ena_defs/ena_admin_defs.h
index cff6451c96..8a1bb0bb76 100644
--- a/drivers/net/ena/base/ena_defs/ena_admin_defs.h
+++ b/drivers/net/ena/base/ena_defs/ena_admin_defs.h
@@ -1,7 +1,8 @@
-/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright (c) 2015-2020 Amazon.com, Inc. or its affiliates.
+/* SPDX-License-Identifier: BSD-3-Clause */
+/* Copyright (c) Amazon.com, Inc. or its affiliates.
  * All rights reserved.
  */
+
 #ifndef _ENA_ADMIN_H_
 #define _ENA_ADMIN_H_
 
diff --git a/drivers/net/ena/base/ena_defs/ena_common_defs.h 
b/drivers/net/ena/base/ena_defs/ena_common_defs.h
index d1ee40de32..ed5359cb99 100644
--- a/drivers/net/ena/base/ena_defs/ena_common_defs.h
+++ b/drivers/net/ena/base/ena_defs/ena_common_defs.h
@@ -1,5 +1,5 @@
-/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright (c) 2015-2020 Amazon.com, Inc. or its affiliates.
+/* SPDX-License-Identifier: BSD-3-Clause */
+/* Copyright (c) Amazon.com, Inc. or its affiliates.
  * All rights reserved.
  */
 
diff --git a/drivers/net/ena/base/ena_defs/ena_eth_io_defs.h 
b/drivers/net/ena/base/ena_defs/ena_eth_io_defs.h
index f811dd261e..c93cd85632 100644
--- a/drivers/net/ena/base/ena_defs/ena_eth_io_defs.h
+++ b/drivers/net/ena/base/ena_defs/ena_eth_io_defs.h
@@ -1,7 +1,8 @@
-/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright (c) 2015-2020 Amazon.com, Inc. or its affiliates.
+/* SPDX-License-Identifier: BSD-3-Clause */
+/* Copyright (c) Amazon.com, Inc. or its affiliates.
  * All rights reserved.
  */
+
 #ifndef _ENA_ETH_IO_H_
 #define _ENA_ETH_IO_H_
 
diff --git a/drivers/net/ena/base/ena_defs/ena_includes.h 
b/drivers/net/ena/base/ena_defs/ena_includes.h
index 20dba04d52..4bcb1e3a4a 100644
--- a/drivers/net/ena/base/ena_defs/ena_includes.h
+++ b/drivers/net/ena/base/ena_defs/ena_includes.h
@@ -1,5 +1,5 @@
-/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright (c) 2015-2019 Amazon.com, Inc. or its affiliates.
+/* SPDX-License-Identifier: BSD-3-Clause */
+/* Copyright (c) Amazon.com, Inc. or its affiliates.
  * All rights reserved.
  */
 
diff --git a/drivers/net/ena/base/ena_defs/ena_regs_defs.h 
b/drivers/net/ena/base/ena_defs/ena_regs_defs.h
index e12a578fac..823dccd841 100644
--- a/drivers/net/ena/base/ena_defs/ena_regs_defs.h
+++ b/drivers/net/ena/base/ena_defs/ena_regs_defs.h
@@ -1,5 +1,5 @@
-/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright (c) 2015-2020 Amazon.com, Inc. or its affiliates.
+/* SPDX-License-Identifier: BSD-3-Clause */
+/* Copyright (c) Amazon.com, Inc. or its affiliates.
  * All rights reserved.
  */
 #ifndef _ENA_REGS_H_
diff --git a/drivers/net/ena/base/ena_eth_com.c 
b/drivers/net/ena/base/ena_eth_com.c
index 9ba0beb868..e26678827c 100644
--- a/drivers/net/ena/base/ena_eth_com.c
+++ b/drivers/net/ena/base/ena_eth_com.c
@@ -1,5 +1,5 @@
-/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright (c) 2015-2020 Amazon.com, Inc. or its affiliates.
+/* SPDX-License-Identifier: BSD-3-Clause */
+/* Copyright (c) Amazon.com, Inc. or its affiliates.
  * All rights reserved.
  */
 
diff --git a/drivers/net/ena/base/ena_eth_com.h 
b/drivers/net/ena/base/ena_eth_com.h
index 877668612c..f91cf67c

[PATCH 08/15] net/ena/base: add macro for bitfield access

[shaibran]

From: Shai Brandes 

Add ENA_FIELD_GET and ENA_FIELD_PREP macro to make access to
fields more readable.

Signed-off-by: Shai Brandes 
---
 drivers/net/ena/base/ena_com.c   | 111 
 drivers/net/ena/base/ena_com.h   |  10 +-
 drivers/net/ena/base/ena_eth_com.c   | 146 +++
 drivers/net/ena/base/ena_eth_com.h   |   8 +-
 drivers/net/ena/base/ena_plat_dpdk.h |   3 +
 5 files changed, 165 insertions(+), 113 deletions(-)

diff --git a/drivers/net/ena/base/ena_com.c b/drivers/net/ena/base/ena_com.c
index 5f46e692b3..e5f1a31c9e 100644
--- a/drivers/net/ena/base/ena_com.c
+++ b/drivers/net/ena/base/ena_com.c
@@ -162,10 +162,13 @@ static int ena_com_admin_init_aenq(struct ena_com_dev 
*ena_dev,
ENA_REG_WRITE32(ena_dev->bus, addr_high, ena_dev->reg_bar + 
ENA_REGS_AENQ_BASE_HI_OFF);
 
aenq_caps = 0;
-   aenq_caps |= ena_dev->aenq.q_depth & ENA_REGS_AENQ_CAPS_AENQ_DEPTH_MASK;
-   aenq_caps |= (sizeof(struct ena_admin_aenq_entry) <<
-   ENA_REGS_AENQ_CAPS_AENQ_ENTRY_SIZE_SHIFT) &
-   ENA_REGS_AENQ_CAPS_AENQ_ENTRY_SIZE_MASK;
+   aenq_caps |= ENA_FIELD_PREP(ena_dev->aenq.q_depth,
+   ENA_REGS_AENQ_CAPS_AENQ_DEPTH_MASK,
+   ENA_ZERO_SHIFT);
+
+   aenq_caps |= ENA_FIELD_PREP(sizeof(struct ena_admin_aenq_entry),
+   ENA_REGS_AENQ_CAPS_AENQ_ENTRY_SIZE_MASK,
+   ENA_REGS_AENQ_CAPS_AENQ_ENTRY_SIZE_SHIFT);
ENA_REG_WRITE32(ena_dev->bus, aenq_caps, ena_dev->reg_bar + 
ENA_REGS_AENQ_CAPS_OFF);
 
if (unlikely(!aenq_handlers)) {
@@ -856,8 +859,9 @@ static u32 ena_com_reg_bar_read32(struct ena_com_dev 
*ena_dev, u16 offset)
mmio_read->seq_num++;
 
read_resp->req_id = mmio_read->seq_num + 0xDEAD;
-   mmio_read_reg = (offset << ENA_REGS_MMIO_REG_READ_REG_OFF_SHIFT) &
-   ENA_REGS_MMIO_REG_READ_REG_OFF_MASK;
+   mmio_read_reg = ENA_FIELD_PREP(offset,
+  ENA_REGS_MMIO_REG_READ_REG_OFF_MASK,
+  ENA_REGS_MMIO_REG_READ_REG_OFF_SHIFT);
mmio_read_reg |= mmio_read->seq_num &
ENA_REGS_MMIO_REG_READ_REQ_ID_MASK;
 
@@ -927,9 +931,10 @@ static int ena_com_destroy_io_sq(struct ena_com_dev 
*ena_dev,
else
direction = ENA_ADMIN_SQ_DIRECTION_RX;
 
-   destroy_cmd.sq.sq_identity |= (direction <<
-   ENA_ADMIN_SQ_SQ_DIRECTION_SHIFT) &
-   ENA_ADMIN_SQ_SQ_DIRECTION_MASK;
+   destroy_cmd.sq.sq_identity |=
+   ENA_FIELD_PREP(direction,
+  ENA_ADMIN_SQ_SQ_DIRECTION_MASK,
+  ENA_ADMIN_SQ_SQ_DIRECTION_SHIFT);
 
destroy_cmd.sq.sq_idx = io_sq->idx;
destroy_cmd.aq_common_descriptor.opcode = ENA_ADMIN_DESTROY_SQ;
@@ -1267,16 +1272,18 @@ static int ena_com_create_io_sq(struct ena_com_dev 
*ena_dev,
else
direction = ENA_ADMIN_SQ_DIRECTION_RX;
 
-   create_cmd.sq_identity |= (direction <<
-   ENA_ADMIN_AQ_CREATE_SQ_CMD_SQ_DIRECTION_SHIFT) &
-   ENA_ADMIN_AQ_CREATE_SQ_CMD_SQ_DIRECTION_MASK;
+   create_cmd.sq_identity |=
+   ENA_FIELD_PREP(direction,
+  ENA_ADMIN_AQ_CREATE_SQ_CMD_SQ_DIRECTION_MASK,
+  ENA_ADMIN_AQ_CREATE_SQ_CMD_SQ_DIRECTION_SHIFT);
 
create_cmd.sq_caps_2 |= io_sq->mem_queue_type &
ENA_ADMIN_AQ_CREATE_SQ_CMD_PLACEMENT_POLICY_MASK;
 
-   create_cmd.sq_caps_2 |= (ENA_ADMIN_COMPLETION_POLICY_DESC <<
-   ENA_ADMIN_AQ_CREATE_SQ_CMD_COMPLETION_POLICY_SHIFT) &
-   ENA_ADMIN_AQ_CREATE_SQ_CMD_COMPLETION_POLICY_MASK;
+   create_cmd.sq_caps_2 |=
+   ENA_FIELD_PREP(ENA_ADMIN_COMPLETION_POLICY_DESC,
+  
ENA_ADMIN_AQ_CREATE_SQ_CMD_COMPLETION_POLICY_MASK,
+  
ENA_ADMIN_AQ_CREATE_SQ_CMD_COMPLETION_POLICY_SHIFT);
 
create_cmd.sq_caps_3 |=
ENA_ADMIN_AQ_CREATE_SQ_CMD_IS_PHYSICALLY_CONTIGUOUS_MASK;
@@ -1616,8 +1623,9 @@ int ena_com_get_dma_width(struct ena_com_dev *ena_dev)
return ENA_COM_TIMER_EXPIRED;
}
 
-   width = (caps & ENA_REGS_CAPS_DMA_ADDR_WIDTH_MASK) >>
-   ENA_REGS_CAPS_DMA_ADDR_WIDTH_SHIFT;
+   width = ENA_FIELD_GET(caps,
+ ENA_REGS_CAPS_DMA_ADDR_WIDTH_MASK,
+ ENA_REGS_CAPS_DMA_ADDR_WIDTH_SHIFT);
 
ena_trc_dbg(ena_dev, "ENA dma width: %d\n", width);
 
@@ -1651,18 +1659,26 @@ int ena_com_validate_version(struct ena_com_dev 
*ena_dev)
}
 
ena_trc_info(ena_dev, "ENA device version: %d.%d\n",
-(ver & ENA_REGS_VERSION_MAJOR_VERSION_MASK) >>
-ENA_REGS_VERSION_MAJOR_VERSION_SHIFT,
-ver & ENA

[PATCH 09/15] net/ena: logger change to improve performance

[shaibran]

From: Shai Brandes 

Current implementation of ena_trc_dbg on every TX packet has a major
performance impact on DPDK TX flow.
Profiling revealed that these calls, which trigger rte_log usage,
consume a significant amount of CPU resources.

Change details:
1. Several warning prints that incorrectly used ena_trc_dbg will now be
   compiled out. They have been changed to ena_trc_warn to avoid
   compiler warnings, such as empty if/else body or unused parameter.
2. Removed variables which is used only inside prints and thus
   may be unreferenced.
3. calls for ena_trc_dbg will be enabled only if
   RTE_ETHDEV_DEBUG_TX or RTE_ETHDEV_DEBUG_RX defined

Signed-off-by: Shai Brandes 
---
 doc/guides/rel_notes/release_24_07.rst |  4 
 drivers/net/ena/base/ena_com.c | 19 +--
 drivers/net/ena/base/ena_eth_com.c | 25 -
 drivers/net/ena/base/ena_plat_dpdk.h   |  6 +-
 drivers/net/ena/ena_ethdev.c   |  2 +-
 5 files changed, 27 insertions(+), 29 deletions(-)

diff --git a/doc/guides/rel_notes/release_24_07.rst 
b/doc/guides/rel_notes/release_24_07.rst
index e68a53d757..1e46a4b7c7 100644
--- a/doc/guides/rel_notes/release_24_07.rst
+++ b/doc/guides/rel_notes/release_24_07.rst
@@ -73,6 +73,10 @@ New Features
   ``bpf_obj_get()`` for an xskmap pinned (by the AF_XDP DP) inside the
   container.
 
+* **Updated Amazon ena (Elastic Network Adapter) net driver.**
+
+  * Reworked the driver logger usage in order to improve Tx performance.
+
 * **Update Tap PMD driver.**
 
   * Updated to support up to 8 queues when used by secondary process.
diff --git a/drivers/net/ena/base/ena_com.c b/drivers/net/ena/base/ena_com.c
index e5f1a31c9e..24bad19848 100644
--- a/drivers/net/ena/base/ena_com.c
+++ b/drivers/net/ena/base/ena_com.c
@@ -1392,11 +1392,7 @@ int ena_com_execute_admin_command(struct 
ena_com_admin_queue *admin_queue,
comp, comp_size);
if (IS_ERR(comp_ctx)) {
ret = PTR_ERR(comp_ctx);
-   if (ret == ENA_COM_NO_DEVICE)
-   ena_trc_dbg(admin_queue->ena_dev,
-   "Failed to submit command [%d]\n",
-   ret);
-   else
+   if (ret != ENA_COM_NO_DEVICE)
ena_trc_err(admin_queue->ena_dev,
"Failed to submit command [%d]\n",
ret);
@@ -1408,10 +1404,8 @@ int ena_com_execute_admin_command(struct 
ena_com_admin_queue *admin_queue,
if (unlikely(ret)) {
if (admin_queue->running_state)
ena_trc_err(admin_queue->ena_dev,
-   "Failed to process command. ret = %d\n", 
ret);
-   else
-   ena_trc_dbg(admin_queue->ena_dev,
-   "Failed to process command. ret = %d\n", 
ret);
+   "Failed to process command [%d]\n",
+   ret);
}
return ret;
 }
@@ -2416,7 +2410,6 @@ void ena_com_aenq_intr_handler(struct ena_com_dev 
*ena_dev, void *data)
struct ena_admin_aenq_entry *aenq_e;
struct ena_admin_aenq_common_desc *aenq_common;
struct ena_com_aenq *aenq  = &ena_dev->aenq;
-   u64 timestamp;
ena_aenq_handler handler_cb;
u16 masked_head, processed = 0;
u8 phase;
@@ -2438,13 +2431,11 @@ void ena_com_aenq_intr_handler(struct ena_com_dev 
*ena_dev, void *data)
 */
dma_rmb();
 
-   timestamp = (u64)aenq_common->timestamp_low |
-   ((u64)aenq_common->timestamp_high << 32);
-
ena_trc_dbg(ena_dev, "AENQ! Group[%x] Syndrome[%x] timestamp: 
[%" ENA_PRIu64 "s]\n",
aenq_common->group,
aenq_common->syndrome,
-   timestamp);
+   ((u64)aenq_common->timestamp_low |
+   ((u64)aenq_common->timestamp_high << 32)));
 
/* Handle specific event*/
handler_cb = ena_com_get_specific_aenq_cb(ena_dev,
diff --git a/drivers/net/ena/base/ena_eth_com.c 
b/drivers/net/ena/base/ena_eth_com.c
index 29cc331b1b..90dd85c7ff 100644
--- a/drivers/net/ena/base/ena_eth_com.c
+++ b/drivers/net/ena/base/ena_eth_com.c
@@ -426,8 +426,7 @@ static int ena_com_create_and_store_tx_meta_desc(struct 
ena_com_io_sq *io_sq,
return ENA_COM_OK;
 }
 
-static void ena_com_rx_set_flags(struct ena_com_io_cq *io_cq,
-struct ena_com_rx_ctx *ena_rx_ctx,
+static void ena_com_rx_set_flags(struct ena_com_rx_ctx *ena_rx_ctx,
 struct ena_eth_io_rx_cdesc_base *cdesc)
 {
ena_rx_ctx->l3_proto = cdesc->status &
@@ -453,16 +452,6 @@ static void ena_com_rx_set_flags(struct ena_com_io_cq 
*io_cq,
   

[PATCH 11/15] net/ena: fix bad checksum handling

[shaibran]

From: Shai Brandes 

Removed a workaround for a false L4 bad Rx csum
indication from the device. The workaround was to set it
as unknown so the application would check it instead.
The issue was fixed in the device, thus the driver bad csum
handling should be fixed in the PMD.

Fixes: b2d2f1cf89a6 ("net/ena: fix checksum flag for L4")
Cc: sta...@dpdk.org
Signed-off-by: Shai Brandes 
---
 doc/guides/rel_notes/release_24_07.rst | 1 +
 drivers/net/ena/ena_ethdev.c   | 8 +---
 2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/doc/guides/rel_notes/release_24_07.rst 
b/doc/guides/rel_notes/release_24_07.rst
index a59fb2a21f..f000dec54b 100644
--- a/doc/guides/rel_notes/release_24_07.rst
+++ b/doc/guides/rel_notes/release_24_07.rst
@@ -78,6 +78,7 @@ New Features
   * Reworked the driver logger usage in order to improve Tx performance.
   * Reworked the device uninitialization flow to ensure complete resource
 cleanup and lay the groundwork for hot-unplug support.
+  * Removed an obsolete workaround for a false L4 bad Rx checksum indication.
 
 * **Update Tap PMD driver.**
 
diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 4e7171e629..b43b913903 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -674,13 +674,7 @@ static inline void ena_rx_mbuf_prepare(struct ena_ring 
*rx_ring,
} else {
if (unlikely(ena_rx_ctx->l4_csum_err)) {
++rx_stats->l4_csum_bad;
-   /*
-* For the L4 Rx checksum offload the HW may indicate
-* bad checksum although it's valid. Because of that,
-* we're setting the UNKNOWN flag to let the app
-* re-verify the checksum.
-*/
-   ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_UNKNOWN;
+   ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_BAD;
} else {
++rx_stats->l4_csum_good;
ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_GOOD;
-- 
2.17.1

[PATCH 10/15] net/ena: rework device uninit

[shaibran]

From: Shai Brandes 

Rework device uninitialization flow to ensure complete resource
cleanup, and lay the groundwork for hot-unplug support. With this
change, `ena_destroy_device()` is removed, its functionality now
incorporated into `ena_close()`.

Signed-off-by: Shai Brandes 
---
 doc/guides/rel_notes/release_24_07.rst |  2 +
 drivers/net/ena/ena_ethdev.c   | 55 --
 2 files changed, 27 insertions(+), 30 deletions(-)

diff --git a/doc/guides/rel_notes/release_24_07.rst 
b/doc/guides/rel_notes/release_24_07.rst
index 1e46a4b7c7..a59fb2a21f 100644
--- a/doc/guides/rel_notes/release_24_07.rst
+++ b/doc/guides/rel_notes/release_24_07.rst
@@ -76,6 +76,8 @@ New Features
 * **Updated Amazon ena (Elastic Network Adapter) net driver.**
 
   * Reworked the driver logger usage in order to improve Tx performance.
+  * Reworked the device uninitialization flow to ensure complete resource
+cleanup and lay the groundwork for hot-unplug support.
 
 * **Update Tap PMD driver.**
 
diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 56dbe3b9cd..4e7171e629 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -280,8 +280,8 @@ static int ena_infos_get(struct rte_eth_dev *dev,
 static void ena_control_path_handler(void *cb_arg);
 static void ena_control_path_poll_handler(void *cb_arg);
 static void ena_timer_wd_callback(struct rte_timer *timer, void *arg);
-static void ena_destroy_device(struct rte_eth_dev *eth_dev);
 static int eth_ena_dev_init(struct rte_eth_dev *eth_dev);
+static int eth_ena_dev_uninit(struct rte_eth_dev *eth_dev);
 static int ena_xstats_get_names(struct rte_eth_dev *dev,
struct rte_eth_xstat_name *xstats_names,
unsigned int n);
@@ -880,12 +880,16 @@ static int ena_close(struct rte_eth_dev *dev)
struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
struct ena_adapter *adapter = dev->data->dev_private;
+   struct ena_com_dev *ena_dev = &adapter->ena_dev;
int ret = 0;
int rc;
 
if (rte_eal_process_type() != RTE_PROC_PRIMARY)
return 0;
 
+   if (adapter->state == ENA_ADAPTER_STATE_CLOSED)
+   return 0;
+
if (adapter->state == ENA_ADAPTER_STATE_RUNNING)
ret = ena_stop(dev);
adapter->state = ENA_ADAPTER_STATE_CLOSED;
@@ -905,6 +909,19 @@ static int ena_close(struct rte_eth_dev *dev)
rte_free(adapter->drv_stats);
adapter->drv_stats = NULL;
 
+   ena_com_set_admin_running_state(ena_dev, false);
+
+   ena_com_rss_destroy(ena_dev);
+
+   ena_com_delete_debug_area(ena_dev);
+   ena_com_delete_host_info(ena_dev);
+
+   ena_com_abort_admin_commands(ena_dev);
+   ena_com_wait_for_abort_completion(ena_dev);
+   ena_com_admin_destroy(ena_dev);
+   ena_com_mmio_reg_read_request_destroy(ena_dev);
+   ena_com_delete_customer_metrics_buffer(ena_dev);
+
/*
 * MAC is not allocated dynamically. Setting NULL should prevent from
 * release of the resource in the rte_eth_dev_release_port().
@@ -925,7 +942,12 @@ ena_dev_reset(struct rte_eth_dev *dev)
return -EPERM;
}
 
-   ena_destroy_device(dev);
+   rc = eth_ena_dev_uninit(dev);
+   if (rc) {
+   PMD_INIT_LOG(CRIT, "Failed to un-initialize device\n");
+   return rc;
+   }
+
rc = eth_ena_dev_init(dev);
if (rc)
PMD_INIT_LOG(CRIT, "Cannot initialize device\n");
@@ -2434,39 +2456,12 @@ static int eth_ena_dev_init(struct rte_eth_dev *eth_dev)
return rc;
 }
 
-static void ena_destroy_device(struct rte_eth_dev *eth_dev)
-{
-   struct ena_adapter *adapter = eth_dev->data->dev_private;
-   struct ena_com_dev *ena_dev = &adapter->ena_dev;
-
-   if (adapter->state == ENA_ADAPTER_STATE_FREE)
-   return;
-
-   ena_com_set_admin_running_state(ena_dev, false);
-
-   if (adapter->state != ENA_ADAPTER_STATE_CLOSED)
-   ena_close(eth_dev);
-
-   ena_com_rss_destroy(ena_dev);
-
-   ena_com_delete_debug_area(ena_dev);
-   ena_com_delete_host_info(ena_dev);
-
-   ena_com_abort_admin_commands(ena_dev);
-   ena_com_wait_for_abort_completion(ena_dev);
-   ena_com_admin_destroy(ena_dev);
-   ena_com_mmio_reg_read_request_destroy(ena_dev);
-   ena_com_delete_customer_metrics_buffer(ena_dev);
-
-   adapter->state = ENA_ADAPTER_STATE_FREE;
-}
-
 static int eth_ena_dev_uninit(struct rte_eth_dev *eth_dev)
 {
if (rte_eal_process_type() != RTE_PROC_PRIMARY)
return 0;
 
-   ena_destroy_device(eth_dev);
+   ena_close(eth_dev);
 
return 0;
 }
-- 
2.17.1

[PATCH 12/15] net/ena: fix invalid return value check

[shaibran]

From: Shai Brandes 

Removed the sign inversion for when checking if
ena_com_set_host_attributes returns ENA_COM_UNSUPPORTED.
ENA_COM_UNSUPPORTED is defined as -EOPNOTSUPP, so the extra sign
inversion is wrong.

Fixes: 3adcba9a8987 ("net/ena: update HAL to the newer version")
Cc: sta...@dpdk.org

Signed-off-by: Shai Brandes 
---
 doc/guides/rel_notes/release_24_07.rst | 1 +
 drivers/net/ena/ena_ethdev.c   | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/doc/guides/rel_notes/release_24_07.rst 
b/doc/guides/rel_notes/release_24_07.rst
index f000dec54b..24bb91ad46 100644
--- a/doc/guides/rel_notes/release_24_07.rst
+++ b/doc/guides/rel_notes/release_24_07.rst
@@ -79,6 +79,7 @@ New Features
   * Reworked the device uninitialization flow to ensure complete resource
 cleanup and lay the groundwork for hot-unplug support.
   * Removed an obsolete workaround for a false L4 bad Rx checksum indication.
+  * Fixed an invalid return value check.
 
 * **Update Tap PMD driver.**
 
diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index b43b913903..67a1d86f9a 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -812,7 +812,7 @@ static void ena_config_host_info(struct ena_com_dev 
*ena_dev)
 
rc = ena_com_set_host_attributes(ena_dev);
if (rc) {
-   if (rc == -ENA_COM_UNSUPPORTED)
+   if (rc == ENA_COM_UNSUPPORTED)
PMD_DRV_LOG(WARNING, "Cannot set host attributes\n");
else
PMD_DRV_LOG(ERR, "Cannot set host attributes\n");
@@ -856,7 +856,7 @@ static void ena_config_debug_area(struct ena_adapter 
*adapter)
 
rc = ena_com_set_host_attributes(&adapter->ena_dev);
if (rc) {
-   if (rc == -ENA_COM_UNSUPPORTED)
+   if (rc == ENA_COM_UNSUPPORTED)
PMD_DRV_LOG(WARNING, "Cannot set host attributes\n");
else
PMD_DRV_LOG(ERR, "Cannot set host attributes\n");
-- 
2.17.1

[PATCH 06/15] net/ena/base: add an additional reset reason

[shaibran]

From: Shai Brandes 

This commit adds the support for a new reset reason
for `MISS_FIRST_INTERRUPT` in order to distinguish
between resets where no interrupts have been received
and sporadic missed interrupts.

Signed-off-by: Shai Brandes 
---
 drivers/net/ena/base/ena_defs/ena_regs_defs.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ena/base/ena_defs/ena_regs_defs.h 
b/drivers/net/ena/base/ena_defs/ena_regs_defs.h
index dd9b629f10..e12a578fac 100644
--- a/drivers/net/ena/base/ena_defs/ena_regs_defs.h
+++ b/drivers/net/ena/base/ena_defs/ena_regs_defs.h
@@ -26,6 +26,7 @@ enum ena_regs_reset_reason_types {
ENA_REGS_RESET_TX_DESCRIPTOR_MALFORMED  = 17,
ENA_REGS_RESET_MISSING_ADMIN_INTERRUPT  = 18,
ENA_REGS_RESET_DEVICE_REQUEST   = 19,
+   ENA_REGS_RESET_MISS_FIRST_INTERRUPT = 20,
ENA_REGS_RESET_LAST,
 };
 
-- 
2.17.1

[PATCH 13/15] net/ena: fix wrong handling of checksum

[shaibran]

From: Shai Brandes 

This change fixes an issue where a non tcp/udp packet can be indicated
to have an invalid csum. If the device erroneously tries to verify the
csum on a non tcp/udp packet it will result in false indication that
there is a csum error. This change make the driver ignore the
indication for csum error on such packets.

Fixes: 84daba9962b5 ("net/ena: add extra Rx checksum related xstats")
Cc: sta...@dpdk.org
Signed-off-by: Shai Brandes 
---
 doc/guides/rel_notes/release_24_07.rst | 1 +
 drivers/net/ena/ena_ethdev.c   | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/doc/guides/rel_notes/release_24_07.rst 
b/doc/guides/rel_notes/release_24_07.rst
index 24bb91ad46..ec960d93cc 100644
--- a/doc/guides/rel_notes/release_24_07.rst
+++ b/doc/guides/rel_notes/release_24_07.rst
@@ -80,6 +80,7 @@ New Features
 cleanup and lay the groundwork for hot-unplug support.
   * Removed an obsolete workaround for a false L4 bad Rx checksum indication.
   * Fixed an invalid return value check.
+  * Fixed Rx chcecksum inspection to check only TCP/UDP packets.
 
 * **Update Tap PMD driver.**
 
diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 67a1d86f9a..a18c94df28 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -669,7 +669,8 @@ static inline void ena_rx_mbuf_prepare(struct ena_ring 
*rx_ring,
packet_type |= RTE_PTYPE_L3_IPV6;
}
 
-   if (!ena_rx_ctx->l4_csum_checked || ena_rx_ctx->frag) {
+   if (!ena_rx_ctx->l4_csum_checked || ena_rx_ctx->frag ||
+   !(packet_type & (RTE_PTYPE_L4_TCP | RTE_PTYPE_L4_UDP))) {
ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_UNKNOWN;
} else {
if (unlikely(ena_rx_ctx->l4_csum_err)) {
-- 
2.17.1

[PATCH 14/15] net/ena: rework Rx checksum inspection

[shaibran]

From: Shai Brandes 

This restructure is a simplification of the
Rx checksum inspection logic in ena_rx_mbuf_prepare.
Its purpose is to improve readability and maintainability
by consolidating conditions.

Signed-off-by: Shai Brandes 
---
 doc/guides/rel_notes/release_24_07.rst |  2 +
 drivers/net/ena/ena_ethdev.c   | 66 +++---
 2 files changed, 39 insertions(+), 29 deletions(-)

diff --git a/doc/guides/rel_notes/release_24_07.rst 
b/doc/guides/rel_notes/release_24_07.rst
index ec960d93cc..d2253999fa 100644
--- a/doc/guides/rel_notes/release_24_07.rst
+++ b/doc/guides/rel_notes/release_24_07.rst
@@ -81,6 +81,8 @@ New Features
   * Removed an obsolete workaround for a false L4 bad Rx checksum indication.
   * Fixed an invalid return value check.
   * Fixed Rx chcecksum inspection to check only TCP/UDP packets.
+  * Reworked the Rx checksum inspection routine to improve
+readability and maintainability.
 
 * **Update Tap PMD driver.**
 
diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index a18c94df28..feb229c5ec 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -53,8 +53,6 @@
  */
 #define ENA_CLEANUP_BUF_THRESH 256
 
-#define ENA_PTYPE_HAS_HASH (RTE_PTYPE_L4_TCP | RTE_PTYPE_L4_UDP)
-
 struct ena_stats {
char name[ETH_GSTRING_LEN];
int stat_offset;
@@ -645,19 +643,14 @@ static inline void ena_trigger_reset(struct ena_adapter 
*adapter,
 
 static inline void ena_rx_mbuf_prepare(struct ena_ring *rx_ring,
   struct rte_mbuf *mbuf,
-  struct ena_com_rx_ctx *ena_rx_ctx,
-  bool fill_hash)
+  struct ena_com_rx_ctx *ena_rx_ctx)
 {
struct ena_stats_rx *rx_stats = &rx_ring->rx_stats;
uint64_t ol_flags = 0;
uint32_t packet_type = 0;
 
-   if (ena_rx_ctx->l4_proto == ENA_ETH_IO_L4_PROTO_TCP)
-   packet_type |= RTE_PTYPE_L4_TCP;
-   else if (ena_rx_ctx->l4_proto == ENA_ETH_IO_L4_PROTO_UDP)
-   packet_type |= RTE_PTYPE_L4_UDP;
-
-   if (ena_rx_ctx->l3_proto == ENA_ETH_IO_L3_PROTO_IPV4) {
+   switch (ena_rx_ctx->l3_proto) {
+   case ENA_ETH_IO_L3_PROTO_IPV4:
packet_type |= RTE_PTYPE_L3_IPV4;
if (unlikely(ena_rx_ctx->l3_csum_err)) {
++rx_stats->l3_csum_bad;
@@ -665,27 +658,45 @@ static inline void ena_rx_mbuf_prepare(struct ena_ring 
*rx_ring,
} else {
ol_flags |= RTE_MBUF_F_RX_IP_CKSUM_GOOD;
}
-   } else if (ena_rx_ctx->l3_proto == ENA_ETH_IO_L3_PROTO_IPV6) {
+   break;
+   case ENA_ETH_IO_L3_PROTO_IPV6:
packet_type |= RTE_PTYPE_L3_IPV6;
+   break;
+   default:
+   break;
}
 
-   if (!ena_rx_ctx->l4_csum_checked || ena_rx_ctx->frag ||
-   !(packet_type & (RTE_PTYPE_L4_TCP | RTE_PTYPE_L4_UDP))) {
-   ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_UNKNOWN;
-   } else {
-   if (unlikely(ena_rx_ctx->l4_csum_err)) {
-   ++rx_stats->l4_csum_bad;
-   ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_BAD;
+   switch (ena_rx_ctx->l4_proto) {
+   case ENA_ETH_IO_L4_PROTO_TCP:
+   packet_type |= RTE_PTYPE_L4_TCP;
+   break;
+   case ENA_ETH_IO_L4_PROTO_UDP:
+   packet_type |= RTE_PTYPE_L4_UDP;
+   break;
+   default:
+   break;
+   }
+
+   /* L4 csum is relevant only for TCP/UDP packets */
+   if ((packet_type & (RTE_PTYPE_L4_TCP | RTE_PTYPE_L4_UDP)) && 
!ena_rx_ctx->frag) {
+   if (ena_rx_ctx->l4_csum_checked) {
+   if (likely(!ena_rx_ctx->l4_csum_err)) {
+   ++rx_stats->l4_csum_good;
+   ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_GOOD;
+   } else {
+   ++rx_stats->l4_csum_bad;
+   ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_BAD;
+   }
} else {
-   ++rx_stats->l4_csum_good;
-   ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_GOOD;
+   ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_UNKNOWN;
}
-   }
 
-   if (fill_hash &&
-   likely((packet_type & ENA_PTYPE_HAS_HASH) && !ena_rx_ctx->frag)) {
-   ol_flags |= RTE_MBUF_F_RX_RSS_HASH;
-   mbuf->hash.rss = ena_rx_ctx->hash;
+   if (rx_ring->offloads & RTE_ETH_RX_OFFLOAD_RSS_HASH) {
+   ol_flags |= RTE_MBUF_F_RX_RSS_HASH;
+   mbuf->hash.rss = ena_rx_ctx->hash;
+   }
+   } else {
+   ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_UNKNOWN;
}
 
mbuf->ol_flags = ol_flags;
@@ -2765,7 +2776,6 @@ static uint16

[PATCH 15/15] net/ena: upgrade driver version to 2.10.0

[shaibran]

From: Shai Brandes 

upgrade driver version to 2.10.0.

Signed-off-by: Shai Brandes 
---
 drivers/net/ena/ena_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index feb229c5ec..e0c239e88f 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -22,7 +22,7 @@
 #include 
 
 #define DRV_MODULE_VER_MAJOR   2
-#define DRV_MODULE_VER_MINOR   9
+#define DRV_MODULE_VER_MINOR   10
 #define DRV_MODULE_VER_SUBMINOR0
 
 #define __MERGE_64B_H_L(h, l) (((uint64_t)h << 32) | l)
-- 
2.17.1

Re: i40e add UDP GENEVE tunnel

[Bruce Richardson]

On Thu, May 30, 2024 at 05:04:39PM +0300, Taras Bilous wrote:
>Hi All,
>I see that GENEVE tunnel type is currently not supported in
>i40e_dev_udp_tunnel_port_add(). Is there any reason for this or a
>roadmap for when this is planned to be done? I looked into the latest
>DPDK (24.03) and it is still not available there.
>I'm using XXV710 with an i40e driver and want to associate UDP port
>6081 as a GENEVE tunnel on all ports so that RSS works correctly and
>distributes traffic based on inner packets. I checked the XXV710
>datasheet and found that the following patch works for me, but I would
>like someone else familiar with this to take a look at it.
>--- a/drivers/net/i40e/base/i40e_adminq_cmd.h
>+++ b/drivers/net/i40e/base/i40e_adminq_cmd.h
>@@ -2800,7 +2800,7 @@ struct i40e_aqc_add_udp_tunnel {
> u8  reserved0[3];
> u8  protocol_type;
> #define I40E_AQC_TUNNEL_TYPE_VXLAN  0x00
>-#define I40E_AQC_TUNNEL_TYPE_NGE0x01
>+#define I40E_AQC_TUNNEL_TYPE_GENEVE 0x01
> #define I40E_AQC_TUNNEL_TYPE_TEREDO 0x10
> #define I40E_AQC_TUNNEL_TYPE_VXLAN_GPE  0x11
> u8  reserved1[10];
>diff --git a/drivers/net/i40e/i40e_ethdev.c
>b/drivers/net/i40e/i40e_ethdev.c
>index cb0070f..464e0be 100644
>--- a/drivers/net/i40e/i40e_ethdev.c
>+++ b/drivers/net/i40e/i40e_ethdev.c
>@@ -8753,6 +8753,9 @@ i40e_dev_udp_tunnel_port_add(struct rte_eth_dev
>*dev,
>   I40E_AQC_TUNNEL_TYPE_VXLAN_GPE);
> break;
> case RTE_ETH_TUNNEL_TYPE_GENEVE:
>+ret = i40e_add_vxlan_port(pf, udp_tunnel->udp_port,
>+  I40E_AQC_TUNNEL_TYPE_GENEVE);
>+break;
> case RTE_ETH_TUNNEL_TYPE_TEREDO:
>Best regards,
>Taras

Hi Taras,

Follwing a quick consultation of the datasheet, your proposed patch looks
fine to me - especially if it works for you in reality too! :-)

Can you please submit your change above as a proper patch with appropriate
commit message and sign-off, and I'll try and merge it into the
dpdk-next-net-intel tree for inclusion in the next DPDK release. Some
guidelines for doing a proper DPDK patch can be found at [1], with guide on
doing a commit title and body at [2] and [3] on that page.

Thanks,
/Bruce

[1] https://doc.dpdk.org/guides-24.03/contributing/patches.html
[2] 
https://doc.dpdk.org/guides-24.03/contributing/patches.html#commit-messages-subject-line
[3] 
https://doc.dpdk.org/guides-24.03/contributing/patches.html#commit-messages-body

RE: [EXTERNAL] [PATCH v4 1/2] cryptodev: fix crypto callbacks on unsetting callbacks macro

[Akhil Goyal]

> Crypto callbacks APIs are available in header files but when
> the macro RTE_CRYPTO_CALLBACKS unset, test application need to
> put #ifdef in its code.
> 
> The test application should be able to build and run, regardless
> DPDK library is built with RTE_CRYPTO_CALLBACKS defined or not.
> 
> Added ENOTSUP from the beginning of the APIs implementation
> if RTE_CRYPTO_CALLBACKS macro is unset/undefined.
> 
> Fixes: 1c3ffb95595e ("cryptodev: add enqueue and dequeue callbacks")
> Fixes: 5523a75af539 ("test/crypto: add case for enqueue/dequeue callbacks")

Cc: sta...@dpdk.org
> 
> Signed-off-by: Ganapati Kundapura 
Acked-by: Akhil Goyal 

Applied to dpdk-next-crypto

RE: [EXTERNAL] [PATCH v4 1/5] crypto/openssl: fix GCM and CCM thread unsafe ctxs

[Akhil Goyal]

> Commit 67ab783b5d70 ("crypto/openssl: use local copy for session
> contexts") introduced a fix for concurrency bugs which could occur when
> using one OpenSSL PMD session across multiple cores simultaneously. The
> solution was to clone the EVP contexts per-buffer to avoid them being
> used concurrently.
> 
> However, part of commit 75adf1eae44f ("crypto/openssl: update HMAC
> routine with 3.0 EVP API") reverted this fix, only for combined ops
> (AES-GCM and AES-CCM).
> 
> Fix the concurrency issue by cloning EVP contexts per-buffer. An extra
> workaround is required for OpenSSL versions which are >= 3.0.0, and
> <= 3.2.0. This is because, prior to OpenSSL 3.2.0, EVP_CIPHER_CTX_copy()
> is not implemented for AES-GCM or AES-CCM. When using these OpenSSL
> versions, create and initialise the context from scratch, per-buffer.
> 
> Throughput performance uplift measurements for AES-GCM-128 encrypt on
> Ampere Altra Max platform:
> 1 worker lcore
> |   buffer sz (B) |   prev (Gbps) |   optimised (Gbps) |   uplift |
> |-+---++--|
> |  64 |  2.60 |   1.31 |   -49.5% |
> | 256 |  7.69 |   4.45 |   -42.1% |
> |1024 | 15.33 |  11.30 |   -26.3% |
> |2048 | 18.74 |  15.37 |   -18.0% |
> |4096 | 21.11 |  18.80 |   -10.9% |
> 
> 8 worker lcores
> |   buffer sz (B) |   prev (Gbps) |   optimised (Gbps) |   uplift |
> |-+---++--|
> |  64 | 19.94 |   2.83 |   -85.8% |
> | 256 | 58.84 |  11.00 |   -81.3% |
> |1024 |119.71 |  42.46 |   -64.5% |
> |2048 |147.69 |  80.91 |   -45.2% |
> |4096 |167.39 | 121.25 |   -27.6% |
> 
> Fixes: 75adf1eae44f ("crypto/openssl: update HMAC routine with 3.0 EVP API")
> Cc: sta...@dpdk.org
> Signed-off-by: Jack Bond-Preston 
> Reviewed-by: Wathsala Vithanage 


I am seeing below errors when it is compiled with openssl 3.0

[1/30] Compiling C object 
'drivers/a715181@@tmp_rte_crypto_openssl@sta/crypto_openssl_rte_openssl_pmd_ops.c.o'.
FAILED: 
drivers/a715181@@tmp_rte_crypto_openssl@sta/crypto_openssl_rte_openssl_pmd_ops.c.o
ccache gcc -Idrivers/a715181@@tmp_rte_crypto_openssl@sta -Idrivers -I../drivers 
-Idrivers/crypto/openssl -I../drivers/crypto/openssl -Ilib/cryptodev 
-I../lib/cryptodev -I. -I../ -Iconfig -I../config -Ilib/eal/include 
-I../lib/eal/include -Ilib/eal/linux/include -I../lib/eal/linux/include 
-Ilib/eal/x86/include -I../lib/eal/x86/include -Ilib/eal/common 
-I../lib/eal/common -Ilib/eal -I../lib/eal -Ilib/kvargs -I../lib/kvargs 
-Ilib/log -I../lib/log -Ilib/telemetry/../metrics -I../lib/telemetry/../metrics 
-Ilib/telemetry -I../lib/telemetry -Ilib/mbuf -I../lib/mbuf -Ilib/mempool 
-I../lib/mempool -Ilib/ring -I../lib/ring -Ilib/rcu -I../lib/rcu 
-Idrivers/bus/vdev -I../drivers/bus/vdev -I/usr/local/include 
-fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch 
-Wextra -Werror -std=c11 -O2 -g -include rte_config.h -Wcast-qual -Wdeprecated 
-Wformat -Wformat-nonliteral -Wformat-security -Wmissing-declarations 
-Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wpointer-arith 
-Wsign-compare -Wstrict-prototypes -Wundef -Wwrite-strings 
-Wno-address-of-packed-member -Wno-packed-not-aligned 
-Wno-missing-field-initializers -D_GNU_SOURCE -fPIC -march=native -mrtm 
-DALLOW_EXPERIMENTAL_API -DALLOW_INTERNAL_API -Wno-format-truncation 
-DRTE_LOG_DEFAULT_LOGTYPE=pmd.crypto.openssl -MD -MQ 
'drivers/a715181@@tmp_rte_crypto_openssl@sta/crypto_openssl_rte_openssl_pmd_ops.c.o'
 -MF 
'drivers/a715181@@tmp_rte_crypto_openssl@sta/crypto_openssl_rte_openssl_pmd_ops.c.o.d'
 -o 
'drivers/a715181@@tmp_rte_crypto_openssl@sta/crypto_openssl_rte_openssl_pmd_ops.c.o'
 -c ../drivers/crypto/openssl/rte_openssl_pmd_ops.c
In file included from ../drivers/crypto/openssl/rte_openssl_pmd_ops.c:12:
../drivers/crypto/openssl/compat.h: In function 'free_hmac_ctx':
../drivers/crypto/openssl/compat.h:24:2: error: 'HMAC_CTX_free' is deprecated: 
Since OpenSSL 3.0 [-Werror=deprecated-declarations]
   24 |  HMAC_CTX_free(ctx);
  |  ^
In file included from ../drivers/crypto/openssl/openssl_pmd_private.h:10,
 from ../drivers/crypto/openssl/rte_openssl_pmd_ops.c:11:
/usr/local/include/openssl/hmac.h:35:28: note: declared here
   35 | OSSL_DEPRECATEDIN_3_0 void HMAC_CTX_free(HMAC_CTX *ctx);
  |^
In file included from ../drivers/crypto/openssl/rte_openssl_pmd_ops.c:12:
../drivers/crypto/openssl/compat.h: In function 'free_cmac_ctx':
../drivers/crypto/openssl/compat.h:30:2: error: 'CMAC_CTX_free' is deprecated: 
Since OpenSSL 3.0 [-Werror=deprecated-declarations]
   30 |  CMAC_CTX_free(

RE: [PATCH] examples/fips_validation: fix coverity issues

[Akhil Goyal]

> Hey Gowrishankar,
> 
> > -Original Message-
> > From: Gowrishankar Muthukrishnan 
> > Sent: Saturday, June 15, 2024 12:31 PM
> > To: dev@dpdk.org; Dooley, Brian ; Gowrishankar
> > Muthukrishnan 
> > Cc: Anoob Joseph ; sta...@dpdk.org
> > Subject: [PATCH] examples/fips_validation: fix coverity issues
> >
> > Fix NULL dereference, out-of-bound, bad bit shift issues reported by 
> > coverity
> > scan.
> >
> > Coverity issue: 384440, 384435, 384433, 384429
> > Fixes: 36128a67c27 ("examples/fips_validation: add asymmetric validation")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Gowrishankar Muthukrishnan 
> 
> Acked-by: Brian Dooley 
Applied to dpdk-next-crypto
Title updated as "examples/fips_validation: fix null dereference and 
out-of-bound"

Thanks.

RE: [PATCH v3 2/2] eventdev: add support for enqueue reorder

[Pathak, Pravin]

>+ Add a device-level
>+ RTE_EVENT_DEV_CAP_INDEPENDENT_ENQ
>+ as well.
>+ The documentation of that flag should probably house the detailed 
>description of this feature.

So, this capability will be advertised by DSW and DLB devices with detailed 
documentation. 

>+ Here's how I would describe this feature:

>+ #define RTE_EVENT_PORT_CFG_INDEPENDENT_ENQ   (1ULL << 5)

>+/**< Flag to enable independent enqueue. Must be unset if the device
>+  * is not RTE_EVENT_DEV_CAP_INDEPENDENT_ENQ capable. This feature
>+  * allows an application to enqueue RTE_EVENT_OP_FORWARD or
>+  * RTE_EVENT_OP_RELEASE in an order different than the order the
>+  * events were dequeued from the event device, while maintaining
>+  * RTE_SCHED_TYPE_ATOMIC or RTE_SCHED_TYPE_ORDERED semantics.
>+  *
>+  * If the application wish to change the order of two events *within
>+  * a flow*, it must both change the enqueue order and exchange the
>+  * impl_opaque field, to be portable across all event devices.
>+  */

>+That second paragraph allows DSW to support this feature without modification 
>since this is the only difference between DSW-style independent enqueue and 
>DLB enqueue reordering. DLB will restore a total order, while DSW doesn't 
>(since it would be both pointless and costly, given its design).

Can we skip mentioning this mechanism to change the order of any two events 
intentionally? For DLB, those two events should have been received from the 
same queue and, if atomic, with the same atomic flow ID. If there is no use 
case, we can skip it to avoid confusion. 

>+The upside with DSW-style implementation is that it's very simple and 
>efficient, and does not impose any head-of-line blocking (which follows from 
>restoring a total order between dequeue and enqueue). The downside is it does 
>not allow for a scenario where a particular flow is split across different 
>modules, the application performs reordering >+(e.g., with the dispatcher 
>library) *and* wants to maintain ordering between events pertaining to those 
>"sub flows". I've never come across such a scenario, but it may well exist.

>+If we fail to make DLB2 and DSW compatible, we'll probably need another flag 
>for DSW, because needlessly imposing a total order DSW does not make a lot of 
>sense.

If the device has the capability to support independent enqueue, it should 
enable it on applicable ports using the RTE_EVENT_PORT_CFG_INDEPENDENT_ENQ  
flag. Some devices like DSW will not do any reordering inside as they can 
support it without changing the order whereas devices like DLB which depend on 
order will reorder events inside their PMD.


>+You may want to add an example as well. And a note on the importance of 
>maintaining impl_opaque between dequeue and enqueue.

We will consider this a separate patch later with an example based on the 
dispatcher library, which can work with DSW and DLB.  Is the port provided to 
rte_dispatcher_bind_port_to_lcore() already set up by the application? In that 
case configuring this feature on the port becomes part of the application. 

> + *  @see rte_event_port_setup()
> + */
> +
>   /** Event port configuration structure */
>   struct rte_event_port_conf {
>   int32_t new_event_threshold;

Re: [PATCH v2 2/2] drivers: replace printf with fprintf for debug functions

[Hemant Agrawal]

On 02-07-2024 20:41, Stephen Hemminger wrote:

On Tue,  2 Jul 2024 16:10:13 +0530
Hemant Agrawal  wrote:

  
-	printf("\n JD before conversion\n");

+   fprintf(stdout, "\n JD before conversion\n");
for (i = 0; i < 12; i++)
-   printf("\n 0x%08x", ctx->jobdes.desc[i]);
+   fprintf(stdout, "\n 0x%08x", ctx->jobdes.desc[i]);

Don't see the point of this change, printf() and fprintf(stdout) are same thing.


Yes, but no harm in this change. It was originally a script based change 
in the files. Now the code is consistent - only using fprintf. Either 
with f or with stdout.

RE: [PATCH v1 1/2] bbdev: add new function to dump debug information

[Chautru, Nicolas]

Hi Hemant, 

> -Original Message-
> From: Hemant Agrawal 
> Sent: Tuesday, July 2, 2024 3:54 AM
> To: Chautru, Nicolas ; dev@dpdk.org;
> maxime.coque...@redhat.com
> Cc: hemant.agra...@nxp.com; Marchand, David
> ; Vargas, Hernan
> 
> Subject: Re: [PATCH v1 1/2] bbdev: add new function to dump debug
> information
> 
> Hi Nicolas,
> 
>      Few comments inline.
> 
> On 02-07-2024 04:04, Nicolas Chautru wrote:
> > This provides a new API to dump more debug information related to the
> > status on a given bbdev queue.
> > Some of this information is visible at bbdev level.
> > This also provides a new option dev op, to print more information at
> > the lower PMD level.
> > This helps user to troubleshoot issues related to previous operations
> > provided into a queue causing possible hard-to-debug negative
> > scenarios.
> >
> > Signed-off-by: Nicolas Chautru 
> > ---
> >   lib/bbdev/rte_bbdev.c | 214
> ++
> >   lib/bbdev/rte_bbdev.h |  41 
> >   lib/bbdev/rte_bbdev_pmd.h |   9 ++
> >   lib/bbdev/version.map |   4 +
> >   4 files changed, 268 insertions(+)
> >
> > diff --git a/lib/bbdev/rte_bbdev.c b/lib/bbdev/rte_bbdev.c index
> > 13bde3c25b..81c031fc09 100644
> > --- a/lib/bbdev/rte_bbdev.c
> > +++ b/lib/bbdev/rte_bbdev.c
> > @@ -1190,3 +1190,217 @@ rte_bbdev_enqueue_status_str(enum
> rte_bbdev_enqueue_status status)
> > rte_bbdev_log(ERR, "Invalid enqueue status");
> > return NULL;
> >   }
> > +
> > +
> > +int
> > +rte_bbdev_queue_ops_dump(uint16_t dev_id, uint16_t queue_id, FILE
> *f)
> > +{
> > +   struct rte_bbdev_queue_data *q_data;
> > +   struct rte_bbdev_stats *stats;
> > +   uint16_t i;
> > +   struct rte_bbdev *dev = get_dev(dev_id);
> > +
> > +   VALID_DEV_OR_RET_ERR(dev, dev_id);
> > +   VALID_QUEUE_OR_RET_ERR(queue_id, dev);
> > +   VALID_DEV_OPS_OR_RET_ERR(dev, dev_id);
> > +   VALID_FUNC_OR_RET_ERR(dev->dev_ops->queue_ops_dump,
> dev_id);
> > +
> > +   q_data = &dev->data->queues[queue_id];
> > +
> > +   if (f == NULL)
> > +   return -EINVAL;
> > +
> > +   fprintf(f, "Dump of operations on %s queue %d\n",
> > +   dev->data->name, queue_id);
> > +   fprintf(f, "  Last Enqueue Status %s\n",
> > +   rte_bbdev_enqueue_status_str(q_data-
> >enqueue_status));
> > +   for (i = 0; i < 4; i++)
> 
> It shall be RTE_BBDEV_ENQ_STATUS_SIZE_MAX instead of 4 ?

Thanks, I can update this in the v2.

> 
> 
> > +   if (q_data->queue_stats.enqueue_status_count[i] > 0)
> > +   fprintf(f, "  Enqueue Status Counters %s %" PRIu64
> "\n",
> > +   rte_bbdev_enqueue_status_str(i),
> > +   q_data-
> >queue_stats.enqueue_status_count[i]);
> > +   stats = &dev->data->queues[queue_id].queue_stats;
> > +
> > +   fprintf(f, "  Enqueue Count %" PRIu64 " Warning %" PRIu64 " Error %"
> PRIu64 "\n",
> > +   stats->enqueued_count, stats-
> >enqueue_warn_count,
> > +   stats->enqueue_err_count);
> > +   fprintf(f, "  Dequeue Count %" PRIu64 " Warning %" PRIu64 " Error %"
> PRIu64 "\n",
> > +   stats->dequeued_count, stats-
> >dequeue_warn_count,
> > +   stats->dequeue_err_count);
> > +
> why not print acc_offload_cycles aas well?

I don’t personally find them necessarily useful for this kind. 
But still if you believe these might help sometimes, no problem I can add them 
in the v2. Kindly confirm your preference. 

> > +   return dev->dev_ops->queue_ops_dump(dev, queue_id, f); }
> > +
> > +char *
> > +rte_bbdev_ops_param_string(void *op, enum rte_bbdev_op_type
> op_type)
> > +{
> > +   static char str[1024];
> > +   static char partial[1024];
> > +   struct rte_bbdev_dec_op *op_dec;
> > +   struct rte_bbdev_enc_op *op_enc;
> > +   struct rte_bbdev_fft_op *op_fft;
> > +   struct rte_bbdev_mldts_op *op_mldts;
> > +
> > +   rte_iova_t add0 = 0, add1 = 0, add2 = 0, add3 = 0, add4 = 0;
> > +
> > +   if (op == NULL) {
> > +   snprintf(str, sizeof(str), "Invalid Operation pointer\n");
> > +   return str;
> > +   }
> > +
> how about also covering mempool and opaque_data pointer - they may also
> be helpful in debugging?

What have got in mind specifically?
Note that underlying memory may not always trace to actual mempool (due to 
signal processing sizes not always fitting with mbuf size requirements, some 
mbuf are sometimes indirectly constructed).
Any specific suggestion welcome though.


Thanks, 
Nic

[PATCH v2 0/3] dts: mac filter port to new dts

[Nicholas Pratte]

v2:
  - refactored mac filter suite and cleared out hidden bugs.
  - add/remove mac and multicast addresses code semantics
  improvements.
  - minor documentation changes based on suggestions.

Nicholas Pratte (3):
  dts: add boolean to adjust addresses
  dts: add methods for setting mac and multicast addresses
  dts: mac filter test suite refactored for new dts

 dts/framework/config/conf_yaml_schema.json|   3 +-
 dts/framework/remote_session/testpmd_shell.py | 177 ++
 dts/framework/test_suite.py   |   7 +-
 dts/tests/TestSuite_mac_filter.py | 220 ++
 4 files changed, 405 insertions(+), 2 deletions(-)
 create mode 100644 dts/tests/TestSuite_mac_filter.py

-- 
2.44.0

[PATCH v2 1/3] dts: add boolean to adjust addresses

[Nicholas Pratte]

Various test cases in the mac filter test suite called for granular
manipulation of destination mac addresses to properly test mac address
filtering functionality. To compensate, there is now an
adjust_addresses boolean which the user can toggle if they wish to send
their own addressing; the boolean is true by default.

Bugzilla ID: 1454
Signed-off-by: Nicholas Pratte 
---
 dts/framework/test_suite.py | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
index 694b2eba65..551a587525 100644
--- a/dts/framework/test_suite.py
+++ b/dts/framework/test_suite.py
@@ -185,6 +185,7 @@ def send_packet_and_capture(
 packet: Packet,
 filter_config: PacketFilteringConfig = PacketFilteringConfig(),
 duration: float = 1,
+adjust_addresses: bool = True,
 ) -> list[Packet]:
 """Send and receive `packet` using the associated TG.
 
@@ -195,11 +196,15 @@ def send_packet_and_capture(
 packet: The packet to send.
 filter_config: The filter to use when capturing packets.
 duration: Capture traffic for this amount of time after sending 
`packet`.
+adjust_addresses: If :data:'True', adjust addresses of the 
egressing packet with
+a default addressing scheme. If :data:'False', do not adjust 
the addresses of
+egressing packet.
 
 Returns:
 A list of received packets.
 """
-packet = self._adjust_addresses(packet)
+if adjust_addresses:
+packet = self._adjust_addresses(packet)
 return self.tg_node.send_packet_and_capture(
 packet,
 self._tg_port_egress,
-- 
2.44.0

[PATCH v2 2/3] dts: add methods for setting mac and multicast addresses

[Nicholas Pratte]

Several new methods have been added to TestPMDShell in order to produce
the mac filter's individual test cases:
 - set_mac_addr
 - set_multicast_mac_addr
 - rx_vlan_add
 - rx_vlan_rm
 - vlan_filter_set_on
 - vlan_filter_set_off
 - set_promisc

set_mac_addr and set_multicast_addr were created for the mac filter test
suite, enabling users to both add or remove mac and multicast
addresses based on a booling 'add or remove' parameter. The success or
failure of each call can be verified if a user deems it necessary.

The other methods listed are implemented in other respective test
suites, and their implementations have been copied, but are subject to
change; they are not the focus of this patch.

Bugzilla ID: 1454
Signed-off-by: Nicholas Pratte 
---
 dts/framework/remote_session/testpmd_shell.py | 177 ++
 dts/tests/TestSuite_mac_filter.py |   0
 2 files changed, 177 insertions(+)
 create mode 100644 dts/tests/TestSuite_mac_filter.py

diff --git a/dts/framework/remote_session/testpmd_shell.py 
b/dts/framework/remote_session/testpmd_shell.py
index ec22f72221..0be1fb8754 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -767,6 +767,183 @@ def show_port_info(self, port_id: int) -> TestPmdPort:
 
 return TestPmdPort.parse(output)
 
+def set_mac_addr(self, port_id: int, mac_address: str, add: bool, verify: 
bool = True):
+"""Add or remove a mac address on a given port's Allowlist.
+
+Args:
+port_id: The port ID the mac address is set on.
+mac_address: The mac address to be added or removed to the 
specified port.
+add: If :data:`True`, add the specified mac address. If 
:data:`False`, remove specified
+mac address.
+verify: If :data:'True', assert that the 'mac_addr' operation was 
successful. If
+:data:'False', run the command and skip this assertion.
+
+Raises:
+InteractiveCommandExecutionError: If the set mac address operation 
fails.
+"""
+mac_cmd = "add" if add else "remove"
+output = self.send_command(f"mac_addr {mac_cmd} {port_id} 
{mac_address}")
+if "Bad arguments" in output:
+self._logger.debug("Invalid argument provided to mac_addr")
+raise InteractiveCommandExecutionError("Invalid argument provided")
+
+if verify:
+if "mac_addr_cmd error:" in output:
+self._logger.debug(f"Failed to {mac_cmd} {mac_address} on port 
{port_id}")
+raise InteractiveCommandExecutionError(
+f"Failed to {mac_cmd} {mac_address} on port {port_id} 
\n{output}"
+)
+
+def set_multicast_mac_addr(self, port_id: int, multi_addr: str, add: bool, 
verify: bool = True):
+"""Add or remove multicast mac address to a specified port filter.
+
+Args:
+port_id: The port ID the multicast address is set on.
+multi_addr: The multicast address to be added to the filter.
+add: If :data:'True', add the specified multicast address to the 
port filter.
+If :data:'False', remove the specified multicast address from 
the port filter.
+verify: If :data:'True', assert that the 'mcast_addr' operations 
was successful.
+If :data:'False', execute the 'mcast_addr' operation and skip 
the assertion.
+
+Raises:
+InteractiveCommandExecutionError: If either the 'add' or 'remove' 
operations fails.
+"""
+mcast_cmd = "add" if add else "remove"
+output = self.send_command(f"mcast_addr {mcast_cmd} {port_id} 
{multi_addr}")
+if "Bad arguments" in output:
+self._logger.debug("Invalid arguments provided to mcast_addr")
+raise InteractiveCommandExecutionError("Invalid argument provided")
+
+if verify:
+if (
+"Invalid multicast_addr" in output
+or f'multicast address {"already" if add else "not"} filtered 
by port' in output
+):
+self._logger.debug(f"Failed to {mcast_cmd} {multi_addr} on 
port {port_id}")
+raise InteractiveCommandExecutionError(
+f"Failed to {mcast_cmd} {multi_addr} on port {port_id} 
\n{output}"
+)
+
+def rx_vlan_add(self, vlan: int, port: int, verify: bool = True):
+"""Add specified vlan tag to the filter list on a port.
+
+Args:
+vlan: The vlan tag to add, should be within 1-1005, 1-4094 
extended.
+port: The port number to add the tag on, should be within 0-32.
+verify: If :data:`True`, the output of the command is scanned to 
verify that
+the vlan tag was added to the filter list on the specified 
port. If not, it is
+considered an error.
+
+Raises:
+InteractiveCommandExecutionError: If 

[PATCH v2 3/3] dts: mac filter test suite refactored for new dts

[Nicholas Pratte]

The mac address filter test suite, whose test cases are based on old
DTS's test cases, has been refactored to interface with the new DTS
framework.

In porting over this test suite into the new framework, some
adjustments were made, namely in the EAL and TestPMD parameter provided
before executing the application. While the original test plan was
referenced, by and large, only for the individual test cases, I'll leave
the parameters the original test plan was asking for below for the sake
of discussion:

--burst=1 --rxpt=0 --rxht=0 --rxwt=0 --txpt=36 --txht=0 --txwt=0
--txfreet=32 --rxfreet=64 --mbcache=250 --portmask=0x3

Bugzilla ID: 1454
Signed-off-by: Nicholas Pratte 

---
v2:
 * Refactored the address pool capacity tests to use all available
   octets in the mac address.
 * Change the payload to 'X' characters instead of 'P' characters.
---
 dts/framework/config/conf_yaml_schema.json |   3 +-
 dts/tests/TestSuite_mac_filter.py  | 220 +
 2 files changed, 222 insertions(+), 1 deletion(-)

diff --git a/dts/framework/config/conf_yaml_schema.json 
b/dts/framework/config/conf_yaml_schema.json
index f02a310bb5..ad1f3757f7 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -187,7 +187,8 @@
   "enum": [
 "hello_world",
 "os_udp",
-"pmd_buffer_scatter"
+"pmd_buffer_scatter",
+"mac_filter"
   ]
 },
 "test_target": {
diff --git a/dts/tests/TestSuite_mac_filter.py 
b/dts/tests/TestSuite_mac_filter.py
index e69de29bb2..c960d6b36c 100644
--- a/dts/tests/TestSuite_mac_filter.py
+++ b/dts/tests/TestSuite_mac_filter.py
@@ -0,0 +1,220 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2023-2024 University of New Hampshire
+"""Mac address filtering test suite.
+
+This test suite ensures proper and expected behavior of Allowlist filtering 
via mac
+addresses on devices bound to the Poll Mode Driver. If a packet received on a 
device
+contains a mac address not contained with its mac address pool, the packet 
should
+be dropped. Alternatively, if a packet is received that contains a destination 
mac
+within the devices address pool, the packet should be accepted and forwarded. 
This
+behavior should remain consistent across all packets, namely those containing 
dot1q
+tags or otherwise.
+
+The following test suite assesses behaviors based on the aforementioned logic.
+Additionally, testing is done within the PMD itself to ensure that the mac 
address
+allow list is behaving as expected.
+"""
+
+from time import sleep
+
+from scapy.layers.inet import IP  # type: ignore[import-untyped]
+from scapy.layers.l2 import Dot1Q, Ether  # type: ignore[import-untyped]
+from scapy.packet import Raw  # type: ignore[import-untyped]
+
+from framework.exception import InteractiveCommandExecutionError
+from framework.remote_session.testpmd_shell import TestPmdShell
+from framework.test_suite import TestSuite
+
+
+class TestMacFilter(TestSuite):
+"""Mac address allowlist filtering test suite.
+
+Configure mac address filtering on a given port, and test the port's 
filtering behavior
+using both a given port's hardware address as well as dummy addresses. If 
a port accepts
+a packet that is not contained within its mac address allowlist, then a 
given test case
+fails. Alternatively, if a port drops a packet that is designated within 
its mac address
+allowlist, a given test case will fail.
+
+Moreover, a given port should demonstrate proper behavior when bound to 
the Poll Mode
+Driver. A port should not have an mac address allowlist that exceeds its 
designated size.
+A port's default hardware address should not be removed from its address 
pool, and invalid
+addresses should not be included in the allowlist. If a port abides by the 
above rules, the
+test case passes.
+"""
+
+def send_packet_and_verify(
+self,
+mac_address: str,
+add_vlan: bool = False,
+should_receive: bool = True,
+) -> None:
+"""Generate, send, and verify a packet based on specified parameters.
+
+Test cases within this suite utilize this method to create, send, and 
verify
+packets based on criteria relating to the packet's destination mac 
address,
+vlan tag, and whether or not the packet should be received or not. 
Packets
+are verified using an inserted payload. If the list of received packets
+contains this payload within any of its packets, the test case passes. 
Each
+call with this method sends exactly one packet.
+
+Args:
+mac_address: The destination mac address of the packet being sent.
+add_vlan: Add a vlan tag to the packet being sent. :data:'2' if 
the packet
+should be received, :data:'1' if the packet should not be 
received but
+requires a vlan tag, and None for any other condition.
+should

[PATCH v2 0/3] dts: mac filter port to new dts

[Nicholas Pratte]

v2:
  - refactored mac filter suite and cleared out hidden bugs.
  - add/remove mac and multicast addresses code semantics
  improvements.
  - minor documentation changes based on suggestions.

Nicholas Pratte (3):
  dts: add boolean to adjust addresses
  dts: add methods for setting mac and multicast addresses
  dts: mac filter test suite refactored for new dts

 dts/framework/config/conf_yaml_schema.json|   3 +-
 dts/framework/remote_session/testpmd_shell.py | 177 ++
 dts/framework/test_suite.py   |   7 +-
 dts/tests/TestSuite_mac_filter.py | 220 ++
 4 files changed, 405 insertions(+), 2 deletions(-)
 create mode 100644 dts/tests/TestSuite_mac_filter.py

-- 
2.44.0

[PATCH v2 1/3] dts: add boolean to adjust addresses

[Nicholas Pratte]

Various test cases in the mac filter test suite called for granular
manipulation of destination mac addresses to properly test mac address
filtering functionality. To compensate, there is now an
adjust_addresses boolean which the user can toggle if they wish to send
their own addressing; the boolean is true by default.

Bugzilla ID: 1454
Signed-off-by: Nicholas Pratte 
---
 dts/framework/test_suite.py | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
index 694b2eba65..551a587525 100644
--- a/dts/framework/test_suite.py
+++ b/dts/framework/test_suite.py
@@ -185,6 +185,7 @@ def send_packet_and_capture(
 packet: Packet,
 filter_config: PacketFilteringConfig = PacketFilteringConfig(),
 duration: float = 1,
+adjust_addresses: bool = True,
 ) -> list[Packet]:
 """Send and receive `packet` using the associated TG.
 
@@ -195,11 +196,15 @@ def send_packet_and_capture(
 packet: The packet to send.
 filter_config: The filter to use when capturing packets.
 duration: Capture traffic for this amount of time after sending 
`packet`.
+adjust_addresses: If :data:'True', adjust addresses of the 
egressing packet with
+a default addressing scheme. If :data:'False', do not adjust 
the addresses of
+egressing packet.
 
 Returns:
 A list of received packets.
 """
-packet = self._adjust_addresses(packet)
+if adjust_addresses:
+packet = self._adjust_addresses(packet)
 return self.tg_node.send_packet_and_capture(
 packet,
 self._tg_port_egress,
-- 
2.44.0

[PATCH v2 2/3] dts: add methods for setting mac and multicast addresses

[Nicholas Pratte]

Several new methods have been added to TestPMDShell in order to produce
the mac filter's individual test cases:
 - set_mac_addr
 - set_multicast_mac_addr
 - rx_vlan_add
 - rx_vlan_rm
 - vlan_filter_set_on
 - vlan_filter_set_off
 - set_promisc

set_mac_addr and set_multicast_addr were created for the mac filter test
suite, enabling users to both add or remove mac and multicast
addresses based on a booling 'add or remove' parameter. The success or
failure of each call can be verified if a user deems it necessary.

The other methods listed are implemented in other respective test
suites, and their implementations have been copied, but are subject to
change; they are not the focus of this patch.

Bugzilla ID: 1454
Signed-off-by: Nicholas Pratte 
---
 dts/framework/remote_session/testpmd_shell.py | 177 ++
 dts/tests/TestSuite_mac_filter.py |   0
 2 files changed, 177 insertions(+)
 create mode 100644 dts/tests/TestSuite_mac_filter.py

diff --git a/dts/framework/remote_session/testpmd_shell.py 
b/dts/framework/remote_session/testpmd_shell.py
index ec22f72221..0be1fb8754 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -767,6 +767,183 @@ def show_port_info(self, port_id: int) -> TestPmdPort:
 
 return TestPmdPort.parse(output)
 
+def set_mac_addr(self, port_id: int, mac_address: str, add: bool, verify: 
bool = True):
+"""Add or remove a mac address on a given port's Allowlist.
+
+Args:
+port_id: The port ID the mac address is set on.
+mac_address: The mac address to be added or removed to the 
specified port.
+add: If :data:`True`, add the specified mac address. If 
:data:`False`, remove specified
+mac address.
+verify: If :data:'True', assert that the 'mac_addr' operation was 
successful. If
+:data:'False', run the command and skip this assertion.
+
+Raises:
+InteractiveCommandExecutionError: If the set mac address operation 
fails.
+"""
+mac_cmd = "add" if add else "remove"
+output = self.send_command(f"mac_addr {mac_cmd} {port_id} 
{mac_address}")
+if "Bad arguments" in output:
+self._logger.debug("Invalid argument provided to mac_addr")
+raise InteractiveCommandExecutionError("Invalid argument provided")
+
+if verify:
+if "mac_addr_cmd error:" in output:
+self._logger.debug(f"Failed to {mac_cmd} {mac_address} on port 
{port_id}")
+raise InteractiveCommandExecutionError(
+f"Failed to {mac_cmd} {mac_address} on port {port_id} 
\n{output}"
+)
+
+def set_multicast_mac_addr(self, port_id: int, multi_addr: str, add: bool, 
verify: bool = True):
+"""Add or remove multicast mac address to a specified port filter.
+
+Args:
+port_id: The port ID the multicast address is set on.
+multi_addr: The multicast address to be added to the filter.
+add: If :data:'True', add the specified multicast address to the 
port filter.
+If :data:'False', remove the specified multicast address from 
the port filter.
+verify: If :data:'True', assert that the 'mcast_addr' operations 
was successful.
+If :data:'False', execute the 'mcast_addr' operation and skip 
the assertion.
+
+Raises:
+InteractiveCommandExecutionError: If either the 'add' or 'remove' 
operations fails.
+"""
+mcast_cmd = "add" if add else "remove"
+output = self.send_command(f"mcast_addr {mcast_cmd} {port_id} 
{multi_addr}")
+if "Bad arguments" in output:
+self._logger.debug("Invalid arguments provided to mcast_addr")
+raise InteractiveCommandExecutionError("Invalid argument provided")
+
+if verify:
+if (
+"Invalid multicast_addr" in output
+or f'multicast address {"already" if add else "not"} filtered 
by port' in output
+):
+self._logger.debug(f"Failed to {mcast_cmd} {multi_addr} on 
port {port_id}")
+raise InteractiveCommandExecutionError(
+f"Failed to {mcast_cmd} {multi_addr} on port {port_id} 
\n{output}"
+)
+
+def rx_vlan_add(self, vlan: int, port: int, verify: bool = True):
+"""Add specified vlan tag to the filter list on a port.
+
+Args:
+vlan: The vlan tag to add, should be within 1-1005, 1-4094 
extended.
+port: The port number to add the tag on, should be within 0-32.
+verify: If :data:`True`, the output of the command is scanned to 
verify that
+the vlan tag was added to the filter list on the specified 
port. If not, it is
+considered an error.
+
+Raises:
+InteractiveCommandExecutionError: If 

[PATCH v2 0/3] Mac Filter Port to New DTS

[Nicholas Pratte]

v2:
  - refactored mac filter suite and cleared out hidden bugs.
  - add/remove mac and multicast addresses code semantics
  improvements.
  - minor documentation changes based on suggestions.

Nicholas Pratte (3):
  dts: add boolean to adjust addresses
  dts: add methods for setting mac and multicast addresses
  dts: mac filter test suite refactored for new dts

 dts/framework/config/conf_yaml_schema.json|   3 +-
 dts/framework/remote_session/testpmd_shell.py | 177 ++
 dts/framework/test_suite.py   |   7 +-
 dts/tests/TestSuite_mac_filter.py | 220 ++
 4 files changed, 405 insertions(+), 2 deletions(-)
 create mode 100644 dts/tests/TestSuite_mac_filter.py

-- 
2.44.0

[PATCH v2 1/3] dts: add boolean to adjust addresses

[Nicholas Pratte]

Various test cases in the mac filter test suite called for granular
manipulation of destination mac addresses to properly test mac address
filtering functionality. To compensate, there is now an
adjust_addresses boolean which the user can toggle if they wish to send
their own addressing; the boolean is true by default.

Bugzilla ID: 1454
Signed-off-by: Nicholas Pratte 
---
 dts/framework/test_suite.py | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
index 694b2eba65..551a587525 100644
--- a/dts/framework/test_suite.py
+++ b/dts/framework/test_suite.py
@@ -185,6 +185,7 @@ def send_packet_and_capture(
 packet: Packet,
 filter_config: PacketFilteringConfig = PacketFilteringConfig(),
 duration: float = 1,
+adjust_addresses: bool = True,
 ) -> list[Packet]:
 """Send and receive `packet` using the associated TG.
 
@@ -195,11 +196,15 @@ def send_packet_and_capture(
 packet: The packet to send.
 filter_config: The filter to use when capturing packets.
 duration: Capture traffic for this amount of time after sending 
`packet`.
+adjust_addresses: If :data:'True', adjust addresses of the 
egressing packet with
+a default addressing scheme. If :data:'False', do not adjust 
the addresses of
+egressing packet.
 
 Returns:
 A list of received packets.
 """
-packet = self._adjust_addresses(packet)
+if adjust_addresses:
+packet = self._adjust_addresses(packet)
 return self.tg_node.send_packet_and_capture(
 packet,
 self._tg_port_egress,
-- 
2.44.0

[PATCH v2 2/3] dts: add methods for setting mac and multicast addresses

[Nicholas Pratte]

Several new methods have been added to TestPMDShell in order to produce
the mac filter's individual test cases:
 - set_mac_addr
 - set_multicast_mac_addr
 - rx_vlan_add
 - rx_vlan_rm
 - vlan_filter_set_on
 - vlan_filter_set_off
 - set_promisc

set_mac_addr and set_multicast_addr were created for the mac filter test
suite, enabling users to both add or remove mac and multicast
addresses based on a booling 'add or remove' parameter. The success or
failure of each call can be verified if a user deems it necessary.

The other methods listed are implemented in other respective test
suites, and their implementations have been copied, but are subject to
change; they are not the focus of this patch.

Bugzilla ID: 1454
Signed-off-by: Nicholas Pratte 
---
 dts/framework/remote_session/testpmd_shell.py | 177 ++
 dts/tests/TestSuite_mac_filter.py |   0
 2 files changed, 177 insertions(+)
 create mode 100644 dts/tests/TestSuite_mac_filter.py

diff --git a/dts/framework/remote_session/testpmd_shell.py 
b/dts/framework/remote_session/testpmd_shell.py
index ec22f72221..0be1fb8754 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -767,6 +767,183 @@ def show_port_info(self, port_id: int) -> TestPmdPort:
 
 return TestPmdPort.parse(output)
 
+def set_mac_addr(self, port_id: int, mac_address: str, add: bool, verify: 
bool = True):
+"""Add or remove a mac address on a given port's Allowlist.
+
+Args:
+port_id: The port ID the mac address is set on.
+mac_address: The mac address to be added or removed to the 
specified port.
+add: If :data:`True`, add the specified mac address. If 
:data:`False`, remove specified
+mac address.
+verify: If :data:'True', assert that the 'mac_addr' operation was 
successful. If
+:data:'False', run the command and skip this assertion.
+
+Raises:
+InteractiveCommandExecutionError: If the set mac address operation 
fails.
+"""
+mac_cmd = "add" if add else "remove"
+output = self.send_command(f"mac_addr {mac_cmd} {port_id} 
{mac_address}")
+if "Bad arguments" in output:
+self._logger.debug("Invalid argument provided to mac_addr")
+raise InteractiveCommandExecutionError("Invalid argument provided")
+
+if verify:
+if "mac_addr_cmd error:" in output:
+self._logger.debug(f"Failed to {mac_cmd} {mac_address} on port 
{port_id}")
+raise InteractiveCommandExecutionError(
+f"Failed to {mac_cmd} {mac_address} on port {port_id} 
\n{output}"
+)
+
+def set_multicast_mac_addr(self, port_id: int, multi_addr: str, add: bool, 
verify: bool = True):
+"""Add or remove multicast mac address to a specified port filter.
+
+Args:
+port_id: The port ID the multicast address is set on.
+multi_addr: The multicast address to be added to the filter.
+add: If :data:'True', add the specified multicast address to the 
port filter.
+If :data:'False', remove the specified multicast address from 
the port filter.
+verify: If :data:'True', assert that the 'mcast_addr' operations 
was successful.
+If :data:'False', execute the 'mcast_addr' operation and skip 
the assertion.
+
+Raises:
+InteractiveCommandExecutionError: If either the 'add' or 'remove' 
operations fails.
+"""
+mcast_cmd = "add" if add else "remove"
+output = self.send_command(f"mcast_addr {mcast_cmd} {port_id} 
{multi_addr}")
+if "Bad arguments" in output:
+self._logger.debug("Invalid arguments provided to mcast_addr")
+raise InteractiveCommandExecutionError("Invalid argument provided")
+
+if verify:
+if (
+"Invalid multicast_addr" in output
+or f'multicast address {"already" if add else "not"} filtered 
by port' in output
+):
+self._logger.debug(f"Failed to {mcast_cmd} {multi_addr} on 
port {port_id}")
+raise InteractiveCommandExecutionError(
+f"Failed to {mcast_cmd} {multi_addr} on port {port_id} 
\n{output}"
+)
+
+def rx_vlan_add(self, vlan: int, port: int, verify: bool = True):
+"""Add specified vlan tag to the filter list on a port.
+
+Args:
+vlan: The vlan tag to add, should be within 1-1005, 1-4094 
extended.
+port: The port number to add the tag on, should be within 0-32.
+verify: If :data:`True`, the output of the command is scanned to 
verify that
+the vlan tag was added to the filter list on the specified 
port. If not, it is
+considered an error.
+
+Raises:
+InteractiveCommandExecutionError: If 

[PATCH v2 3/3] dts: mac filter test suite refactored for new dts

[Nicholas Pratte]

The mac address filter test suite, whose test cases are based on old
DTS's test cases, has been refactored to interface with the new DTS
framework.

In porting over this test suite into the new framework, some
adjustments were made, namely in the EAL and TestPMD parameter provided
before executing the application. While the original test plan was
referenced, by and large, only for the individual test cases, I'll leave
the parameters the original test plan was asking for below for the sake
of discussion:

--burst=1 --rxpt=0 --rxht=0 --rxwt=0 --txpt=36 --txht=0 --txwt=0
--txfreet=32 --rxfreet=64 --mbcache=250 --portmask=0x3

Bugzilla ID: 1454
Signed-off-by: Nicholas Pratte 

---
v2:
 * Refactored the address pool capacity tests to use all available
   octets in the mac address.
 * Change the payload to 'X' characters instead of 'P' characters.
---
 dts/framework/config/conf_yaml_schema.json |   3 +-
 dts/tests/TestSuite_mac_filter.py  | 220 +
 2 files changed, 222 insertions(+), 1 deletion(-)

diff --git a/dts/framework/config/conf_yaml_schema.json 
b/dts/framework/config/conf_yaml_schema.json
index f02a310bb5..ad1f3757f7 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -187,7 +187,8 @@
   "enum": [
 "hello_world",
 "os_udp",
-"pmd_buffer_scatter"
+"pmd_buffer_scatter",
+"mac_filter"
   ]
 },
 "test_target": {
diff --git a/dts/tests/TestSuite_mac_filter.py 
b/dts/tests/TestSuite_mac_filter.py
index e69de29bb2..c960d6b36c 100644
--- a/dts/tests/TestSuite_mac_filter.py
+++ b/dts/tests/TestSuite_mac_filter.py
@@ -0,0 +1,220 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2023-2024 University of New Hampshire
+"""Mac address filtering test suite.
+
+This test suite ensures proper and expected behavior of Allowlist filtering 
via mac
+addresses on devices bound to the Poll Mode Driver. If a packet received on a 
device
+contains a mac address not contained with its mac address pool, the packet 
should
+be dropped. Alternatively, if a packet is received that contains a destination 
mac
+within the devices address pool, the packet should be accepted and forwarded. 
This
+behavior should remain consistent across all packets, namely those containing 
dot1q
+tags or otherwise.
+
+The following test suite assesses behaviors based on the aforementioned logic.
+Additionally, testing is done within the PMD itself to ensure that the mac 
address
+allow list is behaving as expected.
+"""
+
+from time import sleep
+
+from scapy.layers.inet import IP  # type: ignore[import-untyped]
+from scapy.layers.l2 import Dot1Q, Ether  # type: ignore[import-untyped]
+from scapy.packet import Raw  # type: ignore[import-untyped]
+
+from framework.exception import InteractiveCommandExecutionError
+from framework.remote_session.testpmd_shell import TestPmdShell
+from framework.test_suite import TestSuite
+
+
+class TestMacFilter(TestSuite):
+"""Mac address allowlist filtering test suite.
+
+Configure mac address filtering on a given port, and test the port's 
filtering behavior
+using both a given port's hardware address as well as dummy addresses. If 
a port accepts
+a packet that is not contained within its mac address allowlist, then a 
given test case
+fails. Alternatively, if a port drops a packet that is designated within 
its mac address
+allowlist, a given test case will fail.
+
+Moreover, a given port should demonstrate proper behavior when bound to 
the Poll Mode
+Driver. A port should not have an mac address allowlist that exceeds its 
designated size.
+A port's default hardware address should not be removed from its address 
pool, and invalid
+addresses should not be included in the allowlist. If a port abides by the 
above rules, the
+test case passes.
+"""
+
+def send_packet_and_verify(
+self,
+mac_address: str,
+add_vlan: bool = False,
+should_receive: bool = True,
+) -> None:
+"""Generate, send, and verify a packet based on specified parameters.
+
+Test cases within this suite utilize this method to create, send, and 
verify
+packets based on criteria relating to the packet's destination mac 
address,
+vlan tag, and whether or not the packet should be received or not. 
Packets
+are verified using an inserted payload. If the list of received packets
+contains this payload within any of its packets, the test case passes. 
Each
+call with this method sends exactly one packet.
+
+Args:
+mac_address: The destination mac address of the packet being sent.
+add_vlan: Add a vlan tag to the packet being sent. :data:'2' if 
the packet
+should be received, :data:'1' if the packet should not be 
received but
+requires a vlan tag, and None for any other condition.
+should

[PATCH v2 0/3] Mac Filter Port to New DTS

[Nicholas Pratte]

v2:
  - refactored mac filter suite and cleared out hidden bugs.
  - add/remove mac and multicast addresses code semantics
  improvements.
  - minor documentation changes based on suggestions.

Nicholas Pratte (3):
  dts: add boolean to adjust addresses
  dts: add methods for setting mac and multicast addresses
  dts: mac filter test suite refactored for new dts

 dts/framework/config/conf_yaml_schema.json|   3 +-
 dts/framework/remote_session/testpmd_shell.py | 177 ++
 dts/framework/test_suite.py   |   7 +-
 dts/tests/TestSuite_mac_filter.py | 220 ++
 4 files changed, 405 insertions(+), 2 deletions(-)
 create mode 100644 dts/tests/TestSuite_mac_filter.py

-- 
2.44.0

[PATCH v2 1/3] dts: add boolean to adjust addresses

[Nicholas Pratte]

Various test cases in the mac filter test suite called for granular
manipulation of destination mac addresses to properly test mac address
filtering functionality. To compensate, there is now an
adjust_addresses boolean which the user can toggle if they wish to send
their own addressing; the boolean is true by default.

Bugzilla ID: 1454
Signed-off-by: Nicholas Pratte 
---
 dts/framework/test_suite.py | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
index 694b2eba65..551a587525 100644
--- a/dts/framework/test_suite.py
+++ b/dts/framework/test_suite.py
@@ -185,6 +185,7 @@ def send_packet_and_capture(
 packet: Packet,
 filter_config: PacketFilteringConfig = PacketFilteringConfig(),
 duration: float = 1,
+adjust_addresses: bool = True,
 ) -> list[Packet]:
 """Send and receive `packet` using the associated TG.
 
@@ -195,11 +196,15 @@ def send_packet_and_capture(
 packet: The packet to send.
 filter_config: The filter to use when capturing packets.
 duration: Capture traffic for this amount of time after sending 
`packet`.
+adjust_addresses: If :data:'True', adjust addresses of the 
egressing packet with
+a default addressing scheme. If :data:'False', do not adjust 
the addresses of
+egressing packet.
 
 Returns:
 A list of received packets.
 """
-packet = self._adjust_addresses(packet)
+if adjust_addresses:
+packet = self._adjust_addresses(packet)
 return self.tg_node.send_packet_and_capture(
 packet,
 self._tg_port_egress,
-- 
2.44.0

[PATCH v2 2/3] dts: add testpmd methods for test suite

[Nicholas Pratte]

Several new methods have been added to TestPMDShell in order to produce
the mac filter's individual test cases:
 - set_mac_addr
 - set_multicast_mac_addr
 - rx_vlan_add
 - rx_vlan_rm
 - vlan_filter_set_on
 - vlan_filter_set_off
 - set_promisc

set_mac_addr and set_multicast_addr were created for the mac filter test
suite, enabling users to both add or remove mac and multicast
addresses based on a booling 'add or remove' parameter. The success or
failure of each call can be verified if a user deems it necessary.

The other methods listed are implemented in other respective test
suites, and their implementations have been copied, but are subject to
change; they are not the focus of this patch.

Bugzilla ID: 1454
Signed-off-by: Nicholas Pratte 
---
 dts/framework/remote_session/testpmd_shell.py | 177 ++
 dts/tests/TestSuite_mac_filter.py |   0
 2 files changed, 177 insertions(+)
 create mode 100644 dts/tests/TestSuite_mac_filter.py

diff --git a/dts/framework/remote_session/testpmd_shell.py 
b/dts/framework/remote_session/testpmd_shell.py
index ec22f72221..0be1fb8754 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -767,6 +767,183 @@ def show_port_info(self, port_id: int) -> TestPmdPort:
 
 return TestPmdPort.parse(output)
 
+def set_mac_addr(self, port_id: int, mac_address: str, add: bool, verify: 
bool = True):
+"""Add or remove a mac address on a given port's Allowlist.
+
+Args:
+port_id: The port ID the mac address is set on.
+mac_address: The mac address to be added or removed to the 
specified port.
+add: If :data:`True`, add the specified mac address. If 
:data:`False`, remove specified
+mac address.
+verify: If :data:'True', assert that the 'mac_addr' operation was 
successful. If
+:data:'False', run the command and skip this assertion.
+
+Raises:
+InteractiveCommandExecutionError: If the set mac address operation 
fails.
+"""
+mac_cmd = "add" if add else "remove"
+output = self.send_command(f"mac_addr {mac_cmd} {port_id} 
{mac_address}")
+if "Bad arguments" in output:
+self._logger.debug("Invalid argument provided to mac_addr")
+raise InteractiveCommandExecutionError("Invalid argument provided")
+
+if verify:
+if "mac_addr_cmd error:" in output:
+self._logger.debug(f"Failed to {mac_cmd} {mac_address} on port 
{port_id}")
+raise InteractiveCommandExecutionError(
+f"Failed to {mac_cmd} {mac_address} on port {port_id} 
\n{output}"
+)
+
+def set_multicast_mac_addr(self, port_id: int, multi_addr: str, add: bool, 
verify: bool = True):
+"""Add or remove multicast mac address to a specified port filter.
+
+Args:
+port_id: The port ID the multicast address is set on.
+multi_addr: The multicast address to be added to the filter.
+add: If :data:'True', add the specified multicast address to the 
port filter.
+If :data:'False', remove the specified multicast address from 
the port filter.
+verify: If :data:'True', assert that the 'mcast_addr' operations 
was successful.
+If :data:'False', execute the 'mcast_addr' operation and skip 
the assertion.
+
+Raises:
+InteractiveCommandExecutionError: If either the 'add' or 'remove' 
operations fails.
+"""
+mcast_cmd = "add" if add else "remove"
+output = self.send_command(f"mcast_addr {mcast_cmd} {port_id} 
{multi_addr}")
+if "Bad arguments" in output:
+self._logger.debug("Invalid arguments provided to mcast_addr")
+raise InteractiveCommandExecutionError("Invalid argument provided")
+
+if verify:
+if (
+"Invalid multicast_addr" in output
+or f'multicast address {"already" if add else "not"} filtered 
by port' in output
+):
+self._logger.debug(f"Failed to {mcast_cmd} {multi_addr} on 
port {port_id}")
+raise InteractiveCommandExecutionError(
+f"Failed to {mcast_cmd} {multi_addr} on port {port_id} 
\n{output}"
+)
+
+def rx_vlan_add(self, vlan: int, port: int, verify: bool = True):
+"""Add specified vlan tag to the filter list on a port.
+
+Args:
+vlan: The vlan tag to add, should be within 1-1005, 1-4094 
extended.
+port: The port number to add the tag on, should be within 0-32.
+verify: If :data:`True`, the output of the command is scanned to 
verify that
+the vlan tag was added to the filter list on the specified 
port. If not, it is
+considered an error.
+
+Raises:
+InteractiveCommandExecutionError: If 

[PATCH v2 3/3] dts: mac filter test suite refactored for new dts

[Nicholas Pratte]

The mac address filter test suite, whose test cases are based on old
DTS's test cases, has been refactored to interface with the new DTS
framework.

In porting over this test suite into the new framework, some
adjustments were made, namely in the EAL and TestPMD parameter provided
before executing the application. While the original test plan was
referenced, by and large, only for the individual test cases, I'll leave
the parameters the original test plan was asking for below for the sake
of discussion:

--burst=1 --rxpt=0 --rxht=0 --rxwt=0 --txpt=36 --txht=0 --txwt=0
--txfreet=32 --rxfreet=64 --mbcache=250 --portmask=0x3

Bugzilla ID: 1454
Signed-off-by: Nicholas Pratte 

---
v2:
 * Refactored the address pool capacity tests to use all available
   octets in the mac address.
 * Change the payload to 'X' characters instead of 'P' characters.
---
 dts/framework/config/conf_yaml_schema.json |   3 +-
 dts/tests/TestSuite_mac_filter.py  | 220 +
 2 files changed, 222 insertions(+), 1 deletion(-)

diff --git a/dts/framework/config/conf_yaml_schema.json 
b/dts/framework/config/conf_yaml_schema.json
index f02a310bb5..ad1f3757f7 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -187,7 +187,8 @@
   "enum": [
 "hello_world",
 "os_udp",
-"pmd_buffer_scatter"
+"pmd_buffer_scatter",
+"mac_filter"
   ]
 },
 "test_target": {
diff --git a/dts/tests/TestSuite_mac_filter.py 
b/dts/tests/TestSuite_mac_filter.py
index e69de29bb2..c960d6b36c 100644
--- a/dts/tests/TestSuite_mac_filter.py
+++ b/dts/tests/TestSuite_mac_filter.py
@@ -0,0 +1,220 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2023-2024 University of New Hampshire
+"""Mac address filtering test suite.
+
+This test suite ensures proper and expected behavior of Allowlist filtering 
via mac
+addresses on devices bound to the Poll Mode Driver. If a packet received on a 
device
+contains a mac address not contained with its mac address pool, the packet 
should
+be dropped. Alternatively, if a packet is received that contains a destination 
mac
+within the devices address pool, the packet should be accepted and forwarded. 
This
+behavior should remain consistent across all packets, namely those containing 
dot1q
+tags or otherwise.
+
+The following test suite assesses behaviors based on the aforementioned logic.
+Additionally, testing is done within the PMD itself to ensure that the mac 
address
+allow list is behaving as expected.
+"""
+
+from time import sleep
+
+from scapy.layers.inet import IP  # type: ignore[import-untyped]
+from scapy.layers.l2 import Dot1Q, Ether  # type: ignore[import-untyped]
+from scapy.packet import Raw  # type: ignore[import-untyped]
+
+from framework.exception import InteractiveCommandExecutionError
+from framework.remote_session.testpmd_shell import TestPmdShell
+from framework.test_suite import TestSuite
+
+
+class TestMacFilter(TestSuite):
+"""Mac address allowlist filtering test suite.
+
+Configure mac address filtering on a given port, and test the port's 
filtering behavior
+using both a given port's hardware address as well as dummy addresses. If 
a port accepts
+a packet that is not contained within its mac address allowlist, then a 
given test case
+fails. Alternatively, if a port drops a packet that is designated within 
its mac address
+allowlist, a given test case will fail.
+
+Moreover, a given port should demonstrate proper behavior when bound to 
the Poll Mode
+Driver. A port should not have an mac address allowlist that exceeds its 
designated size.
+A port's default hardware address should not be removed from its address 
pool, and invalid
+addresses should not be included in the allowlist. If a port abides by the 
above rules, the
+test case passes.
+"""
+
+def send_packet_and_verify(
+self,
+mac_address: str,
+add_vlan: bool = False,
+should_receive: bool = True,
+) -> None:
+"""Generate, send, and verify a packet based on specified parameters.
+
+Test cases within this suite utilize this method to create, send, and 
verify
+packets based on criteria relating to the packet's destination mac 
address,
+vlan tag, and whether or not the packet should be received or not. 
Packets
+are verified using an inserted payload. If the list of received packets
+contains this payload within any of its packets, the test case passes. 
Each
+call with this method sends exactly one packet.
+
+Args:
+mac_address: The destination mac address of the packet being sent.
+add_vlan: Add a vlan tag to the packet being sent. :data:'2' if 
the packet
+should be received, :data:'1' if the packet should not be 
received but
+requires a vlan tag, and None for any other condition.
+should

[PATCH v2 1/3] dts: add boolean to adjust addresses

[Nicholas Pratte]

Various test cases in the mac filter test suite called for granular
manipulation of destination mac addresses to properly test mac address
filtering functionality. To compensate, there is now an
adjust_addresses boolean which the user can toggle if they wish to send
their own addressing; the boolean is true by default.

Bugzilla ID: 1454
Signed-off-by: Nicholas Pratte 
---
 dts/framework/test_suite.py | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
index 694b2eba65..551a587525 100644
--- a/dts/framework/test_suite.py
+++ b/dts/framework/test_suite.py
@@ -185,6 +185,7 @@ def send_packet_and_capture(
 packet: Packet,
 filter_config: PacketFilteringConfig = PacketFilteringConfig(),
 duration: float = 1,
+adjust_addresses: bool = True,
 ) -> list[Packet]:
 """Send and receive `packet` using the associated TG.
 
@@ -195,11 +196,15 @@ def send_packet_and_capture(
 packet: The packet to send.
 filter_config: The filter to use when capturing packets.
 duration: Capture traffic for this amount of time after sending 
`packet`.
+adjust_addresses: If :data:'True', adjust addresses of the 
egressing packet with
+a default addressing scheme. If :data:'False', do not adjust 
the addresses of
+egressing packet.
 
 Returns:
 A list of received packets.
 """
-packet = self._adjust_addresses(packet)
+if adjust_addresses:
+packet = self._adjust_addresses(packet)
 return self.tg_node.send_packet_and_capture(
 packet,
 self._tg_port_egress,
-- 
2.44.0

[PATCH v2 0/3] Mac Filter Port to New DTS

[Nicholas Pratte]

This series incorporates the test suite itself, in addition to framwork
changes needed to complete the suite's stipulated test cases. The test
suite consists of three test cases: one testing basically mac filtering
functionality, the other testing the PMD's mac address pool behavior
within a TestPMD shell, and the third testing filtering functionality
of multicast mac addresses.

The port from old DTS has removed some potentially important details
regarding EAL and TestPMD parameters, and these can be found within the
test suites corresponding commit messages. Moreover, unlike old DTS,
instead of validating packet behavior using TestPMD, this test suite
validates behavior by sending, and analysing,a list of received packets.

Nicholas Pratte (3):
  dts: add boolean to adjust addresses
  dts: add methods for setting mac and multicast addresses
  dts: mac filter test suite refactored for new dts

 dts/framework/config/conf_yaml_schema.json|   3 +-
 dts/framework/remote_session/testpmd_shell.py | 177 ++
 dts/framework/test_suite.py   |   7 +-
 dts/tests/TestSuite_mac_filter.py | 220 ++
 4 files changed, 405 insertions(+), 2 deletions(-)
 create mode 100644 dts/tests/TestSuite_mac_filter.py

-- 
2.44.0

[PATCH v2 2/3] dts: add testpmd methods for test suite

[Nicholas Pratte]

Several new methods have been added to TestPMDShell in order to produce
the mac filter's individual test cases:
 - set_mac_addr
 - set_multicast_mac_addr
 - rx_vlan_add
 - rx_vlan_rm
 - vlan_filter_set_on
 - vlan_filter_set_off
 - set_promisc

set_mac_addr and set_multicast_addr were created for the mac filter test
suite, enabling users to both add or remove mac and multicast
addresses based on a booling 'add or remove' parameter. The success or
failure of each call can be verified if a user deems it necessary.

The other methods listed are implemented in other respective test
suites, and their implementations have been copied, but are subject to
change; they are not the focus of this patch.

Bugzilla ID: 1454
Signed-off-by: Nicholas Pratte 
---
 dts/framework/remote_session/testpmd_shell.py | 177 ++
 dts/tests/TestSuite_mac_filter.py |   0
 2 files changed, 177 insertions(+)
 create mode 100644 dts/tests/TestSuite_mac_filter.py

diff --git a/dts/framework/remote_session/testpmd_shell.py 
b/dts/framework/remote_session/testpmd_shell.py
index ec22f72221..0be1fb8754 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -767,6 +767,183 @@ def show_port_info(self, port_id: int) -> TestPmdPort:
 
 return TestPmdPort.parse(output)
 
+def set_mac_addr(self, port_id: int, mac_address: str, add: bool, verify: 
bool = True):
+"""Add or remove a mac address on a given port's Allowlist.
+
+Args:
+port_id: The port ID the mac address is set on.
+mac_address: The mac address to be added or removed to the 
specified port.
+add: If :data:`True`, add the specified mac address. If 
:data:`False`, remove specified
+mac address.
+verify: If :data:'True', assert that the 'mac_addr' operation was 
successful. If
+:data:'False', run the command and skip this assertion.
+
+Raises:
+InteractiveCommandExecutionError: If the set mac address operation 
fails.
+"""
+mac_cmd = "add" if add else "remove"
+output = self.send_command(f"mac_addr {mac_cmd} {port_id} 
{mac_address}")
+if "Bad arguments" in output:
+self._logger.debug("Invalid argument provided to mac_addr")
+raise InteractiveCommandExecutionError("Invalid argument provided")
+
+if verify:
+if "mac_addr_cmd error:" in output:
+self._logger.debug(f"Failed to {mac_cmd} {mac_address} on port 
{port_id}")
+raise InteractiveCommandExecutionError(
+f"Failed to {mac_cmd} {mac_address} on port {port_id} 
\n{output}"
+)
+
+def set_multicast_mac_addr(self, port_id: int, multi_addr: str, add: bool, 
verify: bool = True):
+"""Add or remove multicast mac address to a specified port filter.
+
+Args:
+port_id: The port ID the multicast address is set on.
+multi_addr: The multicast address to be added to the filter.
+add: If :data:'True', add the specified multicast address to the 
port filter.
+If :data:'False', remove the specified multicast address from 
the port filter.
+verify: If :data:'True', assert that the 'mcast_addr' operations 
was successful.
+If :data:'False', execute the 'mcast_addr' operation and skip 
the assertion.
+
+Raises:
+InteractiveCommandExecutionError: If either the 'add' or 'remove' 
operations fails.
+"""
+mcast_cmd = "add" if add else "remove"
+output = self.send_command(f"mcast_addr {mcast_cmd} {port_id} 
{multi_addr}")
+if "Bad arguments" in output:
+self._logger.debug("Invalid arguments provided to mcast_addr")
+raise InteractiveCommandExecutionError("Invalid argument provided")
+
+if verify:
+if (
+"Invalid multicast_addr" in output
+or f'multicast address {"already" if add else "not"} filtered 
by port' in output
+):
+self._logger.debug(f"Failed to {mcast_cmd} {multi_addr} on 
port {port_id}")
+raise InteractiveCommandExecutionError(
+f"Failed to {mcast_cmd} {multi_addr} on port {port_id} 
\n{output}"
+)
+
+def rx_vlan_add(self, vlan: int, port: int, verify: bool = True):
+"""Add specified vlan tag to the filter list on a port.
+
+Args:
+vlan: The vlan tag to add, should be within 1-1005, 1-4094 
extended.
+port: The port number to add the tag on, should be within 0-32.
+verify: If :data:`True`, the output of the command is scanned to 
verify that
+the vlan tag was added to the filter list on the specified 
port. If not, it is
+considered an error.
+
+Raises:
+InteractiveCommandExecutionError: If 

[PATCH v2 3/3] dts: mac filter test suite refactored for new dts

[Nicholas Pratte]

The mac address filter test suite, whose test cases are based on old
DTS's test cases, has been refactored to interface with the new DTS
framework.

In porting over this test suite into the new framework, some
adjustments were made, namely in the EAL and TestPMD parameter provided
before executing the application. While the original test plan was
referenced, by and large, only for the individual test cases, I'll leave
the parameters the original test plan was asking for below for the sake
of discussion:

--burst=1 --rxpt=0 --rxht=0 --rxwt=0 --txpt=36 --txht=0 --txwt=0
--txfreet=32 --rxfreet=64 --mbcache=250 --portmask=0x3

Bugzilla ID: 1454
Signed-off-by: Nicholas Pratte 

---
v2:
 * Refactored the address pool capacity tests to use all available
   octets in the mac address.
 * Change the payload to 'X' characters instead of 'P' characters.
---
 dts/framework/config/conf_yaml_schema.json |   3 +-
 dts/tests/TestSuite_mac_filter.py  | 220 +
 2 files changed, 222 insertions(+), 1 deletion(-)

diff --git a/dts/framework/config/conf_yaml_schema.json 
b/dts/framework/config/conf_yaml_schema.json
index f02a310bb5..ad1f3757f7 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -187,7 +187,8 @@
   "enum": [
 "hello_world",
 "os_udp",
-"pmd_buffer_scatter"
+"pmd_buffer_scatter",
+"mac_filter"
   ]
 },
 "test_target": {
diff --git a/dts/tests/TestSuite_mac_filter.py 
b/dts/tests/TestSuite_mac_filter.py
index e69de29bb2..c960d6b36c 100644
--- a/dts/tests/TestSuite_mac_filter.py
+++ b/dts/tests/TestSuite_mac_filter.py
@@ -0,0 +1,220 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2023-2024 University of New Hampshire
+"""Mac address filtering test suite.
+
+This test suite ensures proper and expected behavior of Allowlist filtering 
via mac
+addresses on devices bound to the Poll Mode Driver. If a packet received on a 
device
+contains a mac address not contained with its mac address pool, the packet 
should
+be dropped. Alternatively, if a packet is received that contains a destination 
mac
+within the devices address pool, the packet should be accepted and forwarded. 
This
+behavior should remain consistent across all packets, namely those containing 
dot1q
+tags or otherwise.
+
+The following test suite assesses behaviors based on the aforementioned logic.
+Additionally, testing is done within the PMD itself to ensure that the mac 
address
+allow list is behaving as expected.
+"""
+
+from time import sleep
+
+from scapy.layers.inet import IP  # type: ignore[import-untyped]
+from scapy.layers.l2 import Dot1Q, Ether  # type: ignore[import-untyped]
+from scapy.packet import Raw  # type: ignore[import-untyped]
+
+from framework.exception import InteractiveCommandExecutionError
+from framework.remote_session.testpmd_shell import TestPmdShell
+from framework.test_suite import TestSuite
+
+
+class TestMacFilter(TestSuite):
+"""Mac address allowlist filtering test suite.
+
+Configure mac address filtering on a given port, and test the port's 
filtering behavior
+using both a given port's hardware address as well as dummy addresses. If 
a port accepts
+a packet that is not contained within its mac address allowlist, then a 
given test case
+fails. Alternatively, if a port drops a packet that is designated within 
its mac address
+allowlist, a given test case will fail.
+
+Moreover, a given port should demonstrate proper behavior when bound to 
the Poll Mode
+Driver. A port should not have an mac address allowlist that exceeds its 
designated size.
+A port's default hardware address should not be removed from its address 
pool, and invalid
+addresses should not be included in the allowlist. If a port abides by the 
above rules, the
+test case passes.
+"""
+
+def send_packet_and_verify(
+self,
+mac_address: str,
+add_vlan: bool = False,
+should_receive: bool = True,
+) -> None:
+"""Generate, send, and verify a packet based on specified parameters.
+
+Test cases within this suite utilize this method to create, send, and 
verify
+packets based on criteria relating to the packet's destination mac 
address,
+vlan tag, and whether or not the packet should be received or not. 
Packets
+are verified using an inserted payload. If the list of received packets
+contains this payload within any of its packets, the test case passes. 
Each
+call with this method sends exactly one packet.
+
+Args:
+mac_address: The destination mac address of the packet being sent.
+add_vlan: Add a vlan tag to the packet being sent. :data:'2' if 
the packet
+should be received, :data:'1' if the packet should not be 
received but
+requires a vlan tag, and None for any other condition.
+should

[PATCH v3] net/mlx5: fix matcher object memory leak

[Mahmoud Maatuq]

This makes sure that the allocated matcher object is freed
for all branches that return NULL.

Coverity issue: 426424
Fixes: 27d171b88031 ("net/mlx5: abstract flow action and enable reconfigure")
Cc: mkash...@nvidia.com

Signed-off-by: Mahmoud Maatuq 
---
v3:
* changed commit message.
* fixed typos
v2:
* fixed Fixes and Cc tags.
---
 drivers/net/mlx5/mlx5_flow_dv.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index d46beffd4c..8a0d58cb05 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -12010,9 +12010,12 @@ flow_matcher_create_cb(void *tool_ctx, void *cb_ctx)
items = *((const struct rte_flow_item **)(ctx->data2));
resource->matcher_object = mlx5dr_bwc_matcher_create
(resource->group->tbl, resource->priority, 
items);
-   if (!(resource->matcher_object))
+   if (!resource->matcher_object) {
+   mlx5_free(resource);
return NULL;
+   }
 #else
+   mlx5_free(resource);
return NULL;
 #endif
}
--
2.43.0

RE: [PATCH] net/netvsc: fix mtu_set in netvsc devices

[Long Li]

Thank you, Alexander.

The patch looks good to me. With this patch, do you see other problems with VPP?

I'd like to get a review from @Sam Andrew.

Acked-by: Long Li 


From: Alexander Skorichenko 
Sent: Tuesday, July 2, 2024 1:49 AM
To: step...@networkplumber.org
Cc: Sam Andrew ; ferruh.yi...@amd.com; 
andrew.rybche...@oktetlabs.ru; Long Li ; Wei Hu 
; dev@dpdk.org; Matthew Smith 
Subject: Re: [PATCH] net/netvsc: fix mtu_set in netvsc devices

You don't often get email from 
askoriche...@netgate.com. Learn why this is 
important
Hello,
The failure happens when running under VPP. In terms of dpdk calls the 
following sequence of events occurs for a NetVSC device:

Durung device probing stage of VPP
   1. eth_hn_dev_init() memory for a single primary rx_queue is allocated
dev->data->rx_queues[0] = 0, allocated, but not set yet
no allocation happened for tx_queues[i] and non-primary rx_queues[i]

During device setup stage of VPP from VPP's own generic dpdk_device_setup():
   2.  rte_eth_dev_set_mtu()
currently it segfaults in hn_reinit() when trying to reach into
dev->data->rx_queues[i], dev->data->tx_queues[i]
   3.  rte_eth_tx_queue_setup()
dev->data->tx_queues[i] are being allocated and set
   4.  rte_eth_rx_queue_setup()
dev->data->rx_queues[i] get allocated (i > 0) and set

So rx_queues[0] could be set in step 1, but rx_queues[i] and tx_queues[i] are 
still NULL.
Allocating all the remaining rx/tx queues in step 1 would prevent the crash, 
but then in steps 3-4 would go through releasing and allocating all of the 
queues again.

Another comment regarding the uniform condition introduced in hn_reinit() in 
the patch:
if (dev->data->rx_queues[0] != NULL) ...
Because of the difference between rx/tx queues described above, it's probably 
safer to extend the condition to check both rx and tx separately
if (dev->data->rx_queues[0] != NULL && dev->data->tx_queues[0] != NULL)

- Alexander Skorichenko

RE: [PATCH v2] buildtools: fix invalid symbols

[Jiale, SongX]

> -Original Message-
> From: Mingjin Ye 
> Sent: Monday, July 1, 2024 6:33 PM
> To: dev@dpdk.org
> Cc: Ye, MingjinX ; sta...@dpdk.org; Dmitry Kozlyuk
> 
> Subject: [PATCH v2] buildtools: fix invalid symbols
> 
> Elf files generated by higher version compilers wrap multiple symbols prefixed
> with "this_pmd_name".
> 
> The patch uses the regex "^this_pmd_name[0-9]+$" to match the symbol
> name.
> 
> Bugzilla ID: 1466
> Fixes: 6c4bf8f42432 ("buildtools: add Python pmdinfogen")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Mingjin Ye 
> ---
> v2: Use regex ^this_pmd_name[0-9]+$ to filter symbols *names*
> ---
Tested-by: Song Jiale 

Re: [PATCH v5 1/2] power: introduce PM QoS API on CPU wide

[zhoumin]

Hi Huisong,

I knew that this patchset was based on the *dpdk-next-net* repository in 
our previous communication. However, maybe it's better to rebase this 
pactchset on the *dpdk* repository. Because the CI system is not smart 
enough to recognize the patchset as a change for the *dpdk-next-net* 
repository. I personally feel this patchset is more likely a change for 
the *dpdk* repository because it modified the `lib` directory which is 
the infrastructure of DPDK instead of a feature for *dpdk-next-net*.



Best regard,

Min Zhou

On Tue, July 2, 2024 at 11:50AM, Huisong Li wrote:

The deeper the idle state, the lower the power consumption, but the longer
the resume time. Some service are delay sensitive and very except the low
resume time, like interrupt packet receiving mode.

And the "/sys/devices/system/cpu/cpuX/power/pm_qos_resume_latency_us" sysfs
interface is used to set and get the resume latency limit on the cpuX for
userspace. Each cpuidle governor in Linux select which idle state to enter
based on this CPU resume latency in their idle task.

The per-CPU PM QoS API can be used to control this CPU's idle state
selection and limit just enter the shallowest idle state to low the delay
after sleep by setting strict resume latency (zero value).

Signed-off-by: Huisong Li 
Acked-by: Morten Brørup 
---
  doc/guides/prog_guide/power_man.rst|  24 ++
  doc/guides/rel_notes/release_24_07.rst |   4 +
  lib/power/meson.build  |   2 +
  lib/power/rte_power_qos.c  | 114 +
  lib/power/rte_power_qos.h  |  73 
  lib/power/version.map  |   2 +
  6 files changed, 219 insertions(+)
  create mode 100644 lib/power/rte_power_qos.c
  create mode 100644 lib/power/rte_power_qos.h

Re: [PATCH v5 1/2] power: introduce PM QoS API on CPU wide

[lihuisong (C)]

Hi

在 2024/7/3 9:32, zhoumin 写道:

Hi Huisong,

I knew that this patchset was based on the *dpdk-next-net* repository 
in our previous communication. However, maybe it's better to rebase 
this pactchset on the *dpdk* repository. Because the CI system is not 
smart enough to recognize the patchset as a change for the 
*dpdk-next-net* repository. I personally feel this patchset is more 
likely a change for the *dpdk* repository because it modified the 
`lib` directory which is the infrastructure of DPDK instead of a 
feature for *dpdk-next-net*.


Generally, the 'dpdk-next-net' repo is ahead of 'dpdk' repo. And we make 
some development based on 'dpdk-next-net' repo.


I found that there are some patches also happen to the same issue that 
CI applt patch failed.
So I think the reason why this series trigger warning should be analyzed 
and resolved.


On Tue, July 2, 2024 at 11:50AM, Huisong Li wrote:
The deeper the idle state, the lower the power consumption, but the 
longer
the resume time. Some service are delay sensitive and very except the 
low

resume time, like interrupt packet receiving mode.

And the "/sys/devices/system/cpu/cpuX/power/pm_qos_resume_latency_us" 
sysfs
interface is used to set and get the resume latency limit on the cpuX 
for
userspace. Each cpuidle governor in Linux select which idle state to 
enter

based on this CPU resume latency in their idle task.

The per-CPU PM QoS API can be used to control this CPU's idle state
selection and limit just enter the shallowest idle state to low the 
delay

after sleep by setting strict resume latency (zero value).

Signed-off-by: Huisong Li 
Acked-by: Morten Brørup 
---
  doc/guides/prog_guide/power_man.rst    |  24 ++
  doc/guides/rel_notes/release_24_07.rst |   4 +
  lib/power/meson.build  |   2 +
  lib/power/rte_power_qos.c  | 114 +
  lib/power/rte_power_qos.h  |  73 
  lib/power/version.map  |   2 +
  6 files changed, 219 insertions(+)
  create mode 100644 lib/power/rte_power_qos.c
  create mode 100644 lib/power/rte_power_qos.h



.

Re: [PATCH v1 1/2] bbdev: add new function to dump debug information

[Hemant Agrawal]

On 03-07-2024 00:25, Chautru, Nicolas wrote:

Hi Hemant,


-Original Message-
From: Hemant Agrawal 
Sent: Tuesday, July 2, 2024 3:54 AM
To: Chautru, Nicolas ; dev@dpdk.org;
maxime.coque...@redhat.com
Cc: hemant.agra...@nxp.com; Marchand, David
; Vargas, Hernan

Subject: Re: [PATCH v1 1/2] bbdev: add new function to dump debug
information

Hi Nicolas,

      Few comments inline.

On 02-07-2024 04:04, Nicolas Chautru wrote:

This provides a new API to dump more debug information related to the
status on a given bbdev queue.
Some of this information is visible at bbdev level.
This also provides a new option dev op, to print more information at
the lower PMD level.
This helps user to troubleshoot issues related to previous operations
provided into a queue causing possible hard-to-debug negative
scenarios.

Signed-off-by: Nicolas Chautru 
---
   lib/bbdev/rte_bbdev.c | 214

++

   lib/bbdev/rte_bbdev.h |  41 
   lib/bbdev/rte_bbdev_pmd.h |   9 ++
   lib/bbdev/version.map |   4 +
   4 files changed, 268 insertions(+)

diff --git a/lib/bbdev/rte_bbdev.c b/lib/bbdev/rte_bbdev.c index
13bde3c25b..81c031fc09 100644
--- a/lib/bbdev/rte_bbdev.c
+++ b/lib/bbdev/rte_bbdev.c
@@ -1190,3 +1190,217 @@ rte_bbdev_enqueue_status_str(enum

rte_bbdev_enqueue_status status)

rte_bbdev_log(ERR, "Invalid enqueue status");
return NULL;
   }
+
+
+int
+rte_bbdev_queue_ops_dump(uint16_t dev_id, uint16_t queue_id, FILE

*f)

+{
+   struct rte_bbdev_queue_data *q_data;
+   struct rte_bbdev_stats *stats;
+   uint16_t i;
+   struct rte_bbdev *dev = get_dev(dev_id);
+
+   VALID_DEV_OR_RET_ERR(dev, dev_id);
+   VALID_QUEUE_OR_RET_ERR(queue_id, dev);
+   VALID_DEV_OPS_OR_RET_ERR(dev, dev_id);
+   VALID_FUNC_OR_RET_ERR(dev->dev_ops->queue_ops_dump,

dev_id);

+
+   q_data = &dev->data->queues[queue_id];
+
+   if (f == NULL)
+   return -EINVAL;
+
+   fprintf(f, "Dump of operations on %s queue %d\n",
+   dev->data->name, queue_id);
+   fprintf(f, "  Last Enqueue Status %s\n",
+   rte_bbdev_enqueue_status_str(q_data-
enqueue_status));
+   for (i = 0; i < 4; i++)

It shall be RTE_BBDEV_ENQ_STATUS_SIZE_MAX instead of 4 ?

Thanks, I can update this in the v2.




+   if (q_data->queue_stats.enqueue_status_count[i] > 0)
+   fprintf(f, "  Enqueue Status Counters %s %" PRIu64

"\n",

+   rte_bbdev_enqueue_status_str(i),
+   q_data-
queue_stats.enqueue_status_count[i]);
+   stats = &dev->data->queues[queue_id].queue_stats;
+
+   fprintf(f, "  Enqueue Count %" PRIu64 " Warning %" PRIu64 " Error %"

PRIu64 "\n",

+   stats->enqueued_count, stats-
enqueue_warn_count,
+   stats->enqueue_err_count);
+   fprintf(f, "  Dequeue Count %" PRIu64 " Warning %" PRIu64 " Error %"

PRIu64 "\n",

+   stats->dequeued_count, stats-
dequeue_warn_count,
+   stats->dequeue_err_count);
+

why not print acc_offload_cycles aas well?

I don’t personally find them necessarily useful for this kind.
But still if you believe these might help sometimes, no problem I can add them 
in the v2. Kindly confirm your preference.
This is the only remaining element in the structure not getting dumped. 
If you think, it is not useful at all - you may leave it.



+   return dev->dev_ops->queue_ops_dump(dev, queue_id, f); }
+
+char *
+rte_bbdev_ops_param_string(void *op, enum rte_bbdev_op_type

op_type)

+{
+   static char str[1024];
+   static char partial[1024];
+   struct rte_bbdev_dec_op *op_dec;
+   struct rte_bbdev_enc_op *op_enc;
+   struct rte_bbdev_fft_op *op_fft;
+   struct rte_bbdev_mldts_op *op_mldts;
+
+   rte_iova_t add0 = 0, add1 = 0, add2 = 0, add3 = 0, add4 = 0;
+
+   if (op == NULL) {
+   snprintf(str, sizeof(str), "Invalid Operation pointer\n");
+   return str;
+   }
+

how about also covering mempool and opaque_data pointer - they may also
be helpful in debugging?

What have got in mind specifically?
Note that underlying memory may not always trace to actual mempool (due to 
signal processing sizes not always fitting with mbuf size requirements, some 
mbuf are sometimes indirectly constructed).
Any specific suggestion welcome though.


Yes, It is understood that underlying mbufs may not belong to this 
mempool .  However, being a library function the underlying PMD 
implementations may be different.  I see no harm in to dump most/all of 
the elements, they may be helpful in debugging.







Thanks,
Nic

RE: [DPDK/testpmd Bug 1479] mlx5: Not able to create rte_flows to match head fragments and sub fragments

[Asaf Penso]

Hello

Mlx5 pmd supports only these two modes:

  1.  fragment_offset=0x with mask 0x means MF is 0 and frag-offset is 
0.​
This is an unfragmented packet.​
  2.  fragment_offset spec 0x0001 and last 0x3fff with mask 0x, means MF 
and/or frag-offset is not 0.​
This is a fragmented packet.
To answer your questions, mlx5 doesn’t support match on head fragment and to 
match on non-head fragment, try mode 2 above.

Regards,
Asaf Penso

From: bugzi...@dpdk.org 
Sent: Tuesday, 2 July 2024 17:21
To: dev@dpdk.org
Subject: [DPDK/testpmd Bug 1479] mlx5: Not able to create rte_flows to match 
head fragments and sub fragments

Bug ID
1479
Summary
mlx5: Not able to create rte_flows to match head fragments and sub fragments
Product
DPDK
Version
21.11
Hardware
All
OS
All
Status
UNCONFIRMED
Severity
major
Priority
Normal
Component
testpmd
Assignee
dev@dpdk.org
Reporter
pingtos...@gmail.com
Target Milestone
---

I am trying to create an RTE flow rule to match head and non-head fragments to

compute NIC RSS based on 5tuple/3tuple respectively on connectX-6 DX NIC and

mlx5 driver.



As part of it when trying to install RTE flow rule using testpmd on dpdk

21.11/23.07 version, the following errors are thrown.



Flow rule to match head fragment

=

testpmd>  flow create 0 ingress pattern eth / ipv4 fragment_offset spec 0x2000

fragment_offset mask 0x3fff / end actions drop / count / end

port_flow_complain(): Caught PMD error type 13 (specific pattern item): cause:

0x7ffd1954c548, match on first fragment not supported: Operation not supported



Flow rule to match non-head fragments

==

testpmd> flow validate 0 ingress pattern eth / ipv4 fragment_offset is 0x2001

fragment_offset last 0x1fff / end actions drop / end

port_flow_complain(): Caught PMD error type 11 (item specification range):

cause: 0x7ffc6f629534, specified range not supported: Operation not supported



When I browsed mlx5_flow_dv.c driver file, there are set of conditions

implemented to block this configurations.



Could you kindly help is there a way to compute different RSS hash for

fragments and non-fragments using RTE flow rules on mellanox?



Thanks! in advance.



   /*

 * Match on fragment_offset 0x2000 means MF is 1 and frag-offset is 0,

 * indicating this is 1st fragment of fragmented packet.

 * This is not yet supported in MLX5, return appropriate error message.

 */

if (fragment_offset_spec == RTE_BE16(RTE_IPV4_HDR_MF_FLAG))

return rte_flow_error_set(error, ENOTSUP,

  RTE_FLOW_ERROR_TYPE_ITEM, item,

  "match on first fragment not "

  "supported");

if (fragment_offset_spec && !last)

return rte_flow_error_set(error, ENOTSUP,

  RTE_FLOW_ERROR_TYPE_ITEM, item,

  "specified value not supported");

/*

 * Match on fragment_offset spec 0x2001 and last 0x3fff

 * means MF is 1 and frag-offset is > 0.

 * This packet is fragment 2nd and onward, excluding last.

 * This is not yet supported in MLX5, return appropriate

 * error message.

 */

if (fragment_offset_spec == RTE_BE16(RTE_IPV4_HDR_MF_FLAG + 1) &&

fragment_offset_last == RTE_BE16(MLX5_IPV4_FRAG_OFFSET_MASK))

return rte_flow_error_set(error, ENOTSUP,

  RTE_FLOW_ERROR_TYPE_ITEM_LAST,

  last, "match on following "

  "fragments not supported");

/*

 * Match on fragment_offset spec 0x0001 and last 0x1fff

 * means MF is 0 and frag-offset is > 0.

 * This packet is last fragment of fragmented packet.

 * This is not yet supported in MLX5, return appropriate

 * error message.

 */

if (fragment_offset_spec == RTE_BE16(1) &&

fragment_offset_last == RTE_BE16(RTE_IPV4_HDR_OFFSET_MASK))

return rte_flow_error_set(error, ENOTSUP,

  RTE_FLOW_ERROR_TYPE_ITEM_LAST,

  last, "match on last "

  "fragment not supported");




You are receiving this mail because:

  *   You are the assignee for the bug.