DTS WG Meeting Minutes - April 24, 2025

2025-04-25 Thread Patrick Robb
#
April 24, 2025
Attendees
* Patrick Robb
* Luca Vizzarro
* Paul Szczepanek

#
Minutes

=
General Discussion
* DPDK Prague will be held on May 8-9
   * Due to overlap, we will not hold a DTS meeting call on May 8, which
would have been the next regularly scheduled meeting
  * Patrick needs to cancel this on LF events

=
Patch discussions
* RTE_FLOW email thread has some updates, including Thomas’s suggestions
for test coverage and Dean’s new rough test plan:
   *
https://inbox.dpdk.org/dev/cabd7uxpfqcvlt4a5yrxsuah5nnrjzzhjhutxs9g-jbnnkij...@mail.gmail.com/
* Review discussion:
   * HIGH PRIORITY:
  * Shell refactoring patchseries:
https://patchwork.dpdk.org/project/dpdk/list/?series=34865
 * We really need this done - Patrick has blocked out his 3pm-5pm
today to get this reviewed and tested
   * Medium priority:
  * L4 port # packet matching:
https://patchwork.dpdk.org/project/dpdk/list/?series=34805
 * Patrick to review/merge to next-dts
  * RSS testsuites:
https://patchwork.dpdk.org/project/dpdk/list/?series=34713
 * Has a partial review - Thomas will submit a new version
 * Dean is also looking through this testsuite as it pertains to
flow rules
  * Tmp dir and dpdk tree dir:
https://patchwork.dpdk.org/project/dpdk/list/?series=35049
  ** Low Priority:
  * Nick submitted his RFC for adding performance TG support:
https://patchwork.dpdk.org/project/dpdk/list/?series=35097
 * TG abstraction structure is up for discussion and can be changed
 * We will start writing the single core forwarding perf test while
we wait for reviews
   * Merged patchseries:
  * Port_stats testsuite has been merged
* Old testsuites:
   * Patrick rebased/updated and resubmitted the port_control testsuite.
But I found one more change which is needed just now - can submit that
today.
   * The other old patches from Jeremy have been re-tagged to “changes
requested” on patchwork, and Patrick will be working through these.

=
Bugzilla discussions
* None

=
Any other business
* There will be a “Birds of a feather” session at DPDK Prague, which we
could suggest a topic for:
   * Idea #1: DTS Testsuite API/module. What functions are currently
exposed to testsuites, and what others can be added in the future?
* Next meeting is May 22, 2025


New AWS DPDK CI Testing "Lab" and Patchwork Test Label

2025-04-25 Thread Patrick Robb
Hello all,

As has been discussed at the DPDK CI Meetings, Shai Brandes of AWS has been
working to set up the infrastructure to do per-patch testing for DPDK on
some AWS x86 and ARM environments. Currently they are doing a DPDK build +
dpdk-test fast tests on those environments, and reporting those results to
Patchwork under the label aws-unit-testing.

You can see an example report here:
http://mails.dpdk.org/archives/test-report/2025-April/872683.html

And an example of it appearing in a Patchwork checks table:
https://patchwork.dpdk.org/project/dpdk/patch/20250424133128.133900-1-viachesl...@nvidia.com/

Shai will also be at DPDK Prague where we can chat with him about any next
steps for test coverage on these AWS environments (like running DTS or any
other testing options).

Thank you Shai for working to get this online!


Community CI Meeting Minutes - April 17, 2025

2025-04-25 Thread Patrick Robb
#
April 17, 2025
Attendees
1. Patrick Robb
2. Paul Szczepanek
3. Luca Vizzarro
4. Aaron Conole
5. Manit Mahajan

#
Minutes

=
General Announcements
* DPDK Summit Prague is May 8-9
   * Patrick will share out his presentation to the DTS group when its ready
   * Aaron has the metrics he needs for his presentation

=
CI Status

-
UNH-IOL Community Lab
* Intel Quickassist crypto tests: John McNamara gave us a contact for this
- Kai Ji (who is the quickassist maintainer for DPDK). UNH still needs to
pull together the performance metrics to share with Kai before we turn on
the reporting for this test.
* Environment info for reports:
   * There was some discussion on Slack about what environment information
is reported alongside tests, and what can be seen from the dashboard. Some
basic info (distro, kernel, compiler version) is included on the test
report emails, and can also be found on the dashboard like:
https://lab.dpdk.org/results/dashboard/test-matrix/105/
   * But, there are some improvements the lab can do, like extending the
amount of environment information included (so far Bruce suggested
including libc version) and also automatically updating the environment
info more frequently
   * Manit is working on running a python script on Jenkins every 48 hours
which will collect system info for a test environment and update the
respective environment’s fields in our database
   * We can also start inserting more environment info directly into the
top of the test log files which our dashboard hosts for download/view on
test results. This will be a follow up task.
*  Baremetal updates:
   * Cody is working on updates to our testbeds, either on a software level
or moving some NICs to new systems.
* UNH will perform a Jenkins migration in the next couple weeks which will
likely result in a few hours of lab downtime. This will be communicated
ahead of time.
* DTS:
   * Dean is adding some new network cards, and has made some small
improvements to our jenkins scripting which handles starting the DTS
execution and handling the output etc. We will be able to talk about this
some at the DTS presentation at DPDK Prague.
* Patch depends-on support:
   * The patchwork server series is approved and will be in the next release
   * The associated git-pw patch is also merged now:
https://github.com/getpatchwork/git-pw/pull/77
   * So we are on track to use this once the next patchwork patch is
released and we upgrade the dpdk.org patchwork instance
  * Patrick should email Ali now to start this conversation

-
Intel Lab
* Patrick will have to schedule a sync with the people that touch these
testbeds in the Summer to help them setup the DTS framework on their
systems.

-
Github Actions
* None

-
AWS
* They have been running their Build and Unit tests on x86 and ARM systems
for 2 weeks, and sending test reports to an external email (not the dpdk
test report mailing list). They are scheduled to begin sending the reports
to the dpdk test report mailing list this week.

-
Loongarch Lab
* None

=
DTS Improvements & Test Development
* Thomas M has provided some feedback on the rte_flow testsuite,
identifying what are the most common items and actions the rules tested
should be based on: https://inbox.dpdk.org/dev/5050697.TLnPLrj5Ze@thomas/
   *  Items:
  * - RTE_FLOW_ITEM_TYPE_ETH
  * - RTE_FLOW_ITEM_TYPE_IPV4 / RTE_FLOW_ITEM_TYPE_IPV6
  * - RTE_FLOW_ITEM_TYPE_UDP
  * - RTE_FLOW_ITEM_TYPE_TCP
  * - RTE_FLOW_ITEM_TYPE_VLAN
   * Actions:
  * - RTE_FLOW_ACTION_TYPE_QUEUE
  * - RTE_FLOW_ACTION_TYPE_REPRESENTED_PORT
  * - RTE_FLOW_ACTION_TYPE_RSS (Dean make sure you check which RSS
rules are validated from the RSS testsuites which are currently under
review)
  * - RTE_FLOW_ACTION_TYPE_DROP
* Reviews needed:
   * Shell improvements:
https://patchwork.dpdk.org/project/dpdk/list/?series=34865
  * Patrick and Nick please see this series
   * RSS functions:
  * Partial review from Patrick. Looks pretty good so far except for
some syntax errors and we need to agree where the RSS functions should be
kept (in a dedicated file to be imported to testsuites, or just in the
testsuite class)
   * Smoke tests:
  * Paul will do a review
   * Old patches from UNH: Various patches need to be rebased and
resubmitted. Patrick needs to

[PATCH] bus/pci: introduce get_iova_mode for pci dev

2025-04-25 Thread Kyo Liu
I propose this patch for DPDK to enable coexistence between
DPDK and kernel drivers for regular NICs.This solution requires
adding a new pci_ops in rte_pci_driver, through which DPDK will
retrieve the required IOVA mode from the vendor driver.
This mechanism is necessary to handle different IOMMU
configurations and operating modes. Below is a detailed
analysis of various scenarios:

1. When IOMMU is enabled:
1.1 With PT (Pass-Through) enabled:
In this case, the domain type is IOMMU_DOMAIN_IDENTITY,
which prevents vendor drivers from setting IOVA->PA mapping tables.
Therefore, DPDK must use PA mode. To achieve this:
The vendor kernel driver will register a character device (cdev) to
communicate with DPDK. This cdev handles device operations
(open, mmap, etc.) and ultimately
programs the hardware registers.

1.2 With PT disabled:
Here, the vendor driver doesn't enforce specific IOVA mode requirements.
Our implementation will:
Integrate a mediated device (mdev) in the vendor driver.
This mdev interacts with DPDK and manages IOVA->PA mapping configurations.

2. When IOMMU is disabled:
The vendor driver mandates PA mode (consistent with DPDK's PA mode
requirement in this scenario).
A character device (cdev) will similarly be registered for DPDK
communication.

Summary:
The solution leverages multiple technologies:
mdev for IOVA management when IOMMU is partially enabled.
VFIO for device passthrough operations.
cdev for register programming coordination.
A new pci_ops interface in DPDK to dynamically determine IOVA modes.
This architecture enables clean coexistence by establishing standardized
communication channels between DPDK and vendor drivers across different
IOMMU configurations.

Motivation for the Patch:
This patch is introduced to prepare for the upcoming open-source
contribution of our NebulaMatrix SNIC driver to DPDK. We aim to
ensure that our SNIC can seamlessly coexist with kernel drivers
using this mechanism. By adopting the proposed
architecture—leveraging dynamic IOVA mode negotiation via pci_ops,
mediated devices (mdev), and character device (cdev)
interactions—we enable our SNIC to operate in hybrid environments
here both DPDK and kernel drivers may manage the same hardware.
This design aligns with DPDK’s scalability goals and ensures
compatibility across diverse IOMMU configurations, which is critical
for real-world deployment scenarios.

Signed-off-by: Kyo Liu 
---
 .mailmap   |  2 ++
 doc/guides/rel_notes/release_25_07.rst |  4 
 drivers/bus/pci/bus_pci_driver.h   | 11 +++
 drivers/bus/pci/linux/pci.c|  2 ++
 4 files changed, 19 insertions(+)

diff --git a/.mailmap b/.mailmap
index d8439b79ce..509ff9a16f 100644
--- a/.mailmap
+++ b/.mailmap
@@ -78,6 +78,7 @@ Allen Hubbe 
 Alok Makhariya 
 Alok Prasad 
 Alvaro Karsz 
+Alvin Wang
 Alvin Zhang 
 Aman Singh 
 Amaranath Somalapuram 
@@ -829,6 +830,7 @@ Kumar Amber 
 Kumara Parameshwaran  
 Kumar Sanghvi 
 Kyle Larose 
+Kyo Liu 
 Lance Richardson 
 Laszlo Ersek 
 Laura Stroe 
diff --git a/doc/guides/rel_notes/release_25_07.rst 
b/doc/guides/rel_notes/release_25_07.rst
index 093b85d206..e220b3883f 100644
--- a/doc/guides/rel_notes/release_25_07.rst
+++ b/doc/guides/rel_notes/release_25_07.rst
@@ -54,6 +54,10 @@ New Features
  This section is a comment. Do not overwrite or remove it.
  Also, make sure to start the actual text at the margin.
  ===
+* **Added get_iova_mode for rte_pci_driver.**
+
+  Introduce `pci_get_iova_mode` rte_pci_ops for `pci_get_iova_mode`
+  to PCI bus so that PCI drivers could get their wanted iova_mode
 
 
 Removed Items
diff --git a/drivers/bus/pci/bus_pci_driver.h b/drivers/bus/pci/bus_pci_driver.h
index 2cc1119072..c57244d467 100644
--- a/drivers/bus/pci/bus_pci_driver.h
+++ b/drivers/bus/pci/bus_pci_driver.h
@@ -125,6 +125,16 @@ typedef int (pci_dma_map_t)(struct rte_pci_device *dev, 
void *addr,
 typedef int (pci_dma_unmap_t)(struct rte_pci_device *dev, void *addr,
  uint64_t iova, size_t len);
 
+/**
+ * retrieve the required IOVA mode from the vendor driver
+ *
+ * @param dev
+ *   Pointer to the PCI device.
+ * @return
+ *   - rte_iova_mode
+ */
+typedef enum rte_iova_mode (pci_get_iova_mode)(const struct rte_pci_device 
*pdev);
+
 /**
  * A structure describing a PCI driver.
  */
@@ -136,6 +146,7 @@ struct rte_pci_driver {
pci_dma_map_t *dma_map;/**< device dma map function. */
pci_dma_unmap_t *dma_unmap;/**< device dma unmap function. */
const struct rte_pci_id *id_table; /**< ID table, NULL terminated. */
+   pci_get_iova_mode *get_iova_mode;  /**< Device get iova_mode function */
uint32_t drv_flags;/**< Flags RTE_PCI_DRV_*. */
 };
 
diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index c20d159218..fd69a02989 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/lin

Re: [PATCH v2 2/7] dts: add blocking dpdk app class

2025-04-25 Thread Nicholas Pratte
This is perfect for my TREX implementation!

Reviewed-by: Nicholas Pratte 

On Fri, Mar 14, 2025 at 9:19 AM Luca Vizzarro  wrote:
>
> Add BlockingDPDKApp class. Some non-interactive applications are
> blocking and run until the user interrupts them. As their main intended
> usage is to be kept running in the background, this class exploits
> InteractiveShell to spawn a dedicated shell to keep the blocking
> application running, while detaching from it.
>
> This class works by providing the `wait_until_ready` and `close`
> methods. The former starts up the application and returns only when the
> application readiness output ends in the string provided as an argument
> to the same method. Whereas the latter works by simulating a Ctrl+C
> keystroke, therefore sending a SIGINT to the app.
>
> Signed-off-by: Luca Vizzarro 
> Reviewed-by: Paul Szczepanek 
> ---
>  dts/framework/remote_session/dpdk_app.py  | 73 +++
>  dts/framework/remote_session/dpdk_shell.py|  3 +-
>  .../single_active_interactive_shell.py| 12 ++-
>  dts/framework/remote_session/testpmd_shell.py |  2 +-
>  4 files changed, 85 insertions(+), 5 deletions(-)
>  create mode 100644 dts/framework/remote_session/dpdk_app.py
>
> diff --git a/dts/framework/remote_session/dpdk_app.py 
> b/dts/framework/remote_session/dpdk_app.py
> new file mode 100644
> index 00..c9945f302d
> --- /dev/null
> +++ b/dts/framework/remote_session/dpdk_app.py
> @@ -0,0 +1,73 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2025 Arm Limited
> +
> +"""Class to run blocking DPDK apps in the background.
> +
> +The class won't automatically start the app. The start-up is done as part of 
> the
> +:meth:`BlockingDPDKApp.wait_until_ready` method, which will return execution 
> to the caller only
> +when the desired stdout has been returned by the app. Usually this is used 
> to detect when the app
> +has been loaded and ready to be used.
> +
> +Example:
> +..code:: python
> +
> +pdump = BlockingDPDKApp(
> +PurePath("app/dpdk-pdump"),
> +app_params="--pdump 'port=0,queue=*,rx-dev=/tmp/rx-dev.pcap'"
> +)
> +pdump.wait_until_ready("65535") # start app
> +
> +# pdump is now ready to capture
> +
> +pdump.close() # stop/close app
> +"""
> +
> +from pathlib import PurePath
> +
> +from framework.params.eal import EalParams
> +from framework.remote_session.dpdk_shell import DPDKShell
> +
> +
> +class BlockingDPDKApp(DPDKShell):
> +"""Class to manage blocking DPDK apps."""
> +
> +def __init__(
> +self,
> +path: PurePath,
> +name: str | None = None,
> +privileged: bool = True,
> +app_params: EalParams | str = "",
> +) -> None:
> +"""Constructor.
> +
> +Overrides :meth:`~.dpdk_shell.DPDKShell.__init__`.
> +
> +Args:
> +path: Path relative to the DPDK build to the executable.
> +name: Name to identify this application.
> +privileged: Run as privileged user.
> +app_params: The application parameters. If a string or an 
> incomplete :class:`EalParams`
> +object are passed, the EAL params are computed based on the 
> current context.
> +"""
> +if isinstance(app_params, str):
> +eal_params = EalParams()
> +eal_params.append_str(app_params)
> +app_params = eal_params
> +
> +super().__init__(name, privileged, path, app_params)
> +
> +def wait_until_ready(self, end_token: str) -> None:
> +"""Start app and wait until ready.
> +
> +Args:
> +end_token: The string at the end of a line that indicates the 
> app is ready.
> +"""
> +self._start_application(end_token)
> +
> +def close(self) -> None:
> +"""Close the application.
> +
> +Sends a SIGINT to close the application.
> +"""
> +self.send_command("\x03")
> +self._close()
> diff --git a/dts/framework/remote_session/dpdk_shell.py 
> b/dts/framework/remote_session/dpdk_shell.py
> index 0962414876..f7ea2588ca 100644
> --- a/dts/framework/remote_session/dpdk_shell.py
> +++ b/dts/framework/remote_session/dpdk_shell.py
> @@ -65,13 +65,14 @@ def __init__(
>  self,
>  name: str | None = None,
>  privileged: bool = True,
> +path: PurePath | None = None,
>  app_params: EalParams = EalParams(),
>  ) -> None:
>  """Extends :meth:`~.interactive_shell.InteractiveShell.__init__`."""
>  app_params = compute_eal_params(app_params)
>  node = get_ctx().sut_node
>
> -super().__init__(node, name, privileged, app_params)
> +super().__init__(node, name, privileged, path, app_params)
>
>  def _update_real_path(self, path: PurePath) -> None:
>  """Extends 
> :meth:`~.interactive_shell.InteractiveShell._update_real_path`.
> diff --git a/dts/framework/remote_session/single_active_

[PATCH] net/mlx5: fix VLAN stripping on hairpin queues

2025-04-25 Thread Dariusz Sosnowski
Rx hairpin queues support VLAN stripping,
but if port was started and application attempted
to configure stripping on hairpin queue,
segfault was triggered because of NULL dereference.
Underlying function, which was updating the RQ was passing
wrong object handle for hairpin queues.
This patch fixes that.

Fixes: e79c9be91515 ("net/mlx5: support Rx hairpin queues")
Cc: sta...@dpdk.org

Signed-off-by: Dariusz Sosnowski 
---
 drivers/net/mlx5/mlx5_devx.c | 2 ++
 drivers/net/mlx5/mlx5_vlan.c | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_devx.c b/drivers/net/mlx5/mlx5_devx.c
index a12891a983..ed1826a612 100644
--- a/drivers/net/mlx5/mlx5_devx.c
+++ b/drivers/net/mlx5/mlx5_devx.c
@@ -88,6 +88,8 @@ mlx5_rxq_obj_modify_rq_vlan_strip(struct mlx5_rxq_priv *rxq, 
int on)
rq_attr.state = MLX5_RQC_STATE_RDY;
rq_attr.vsd = (on ? 0 : 1);
rq_attr.modify_bitmask = MLX5_MODIFY_RQ_IN_MODIFY_BITMASK_VSD;
+   if (rxq->ctrl->is_hairpin)
+   return mlx5_devx_cmd_modify_rq(rxq->ctrl->obj->rq, &rq_attr);
return mlx5_devx_cmd_modify_rq(rxq->devx_rq.rq, &rq_attr);
 }
 
diff --git a/drivers/net/mlx5/mlx5_vlan.c b/drivers/net/mlx5/mlx5_vlan.c
index 43a314a679..7c7ac78dfe 100644
--- a/drivers/net/mlx5/mlx5_vlan.c
+++ b/drivers/net/mlx5/mlx5_vlan.c
@@ -107,7 +107,7 @@ mlx5_vlan_strip_queue_set(struct rte_eth_dev *dev, uint16_t 
queue, int on)
dev->data->port_id, queue);
return;
}
-   DRV_LOG(DEBUG, "port %u set VLAN stripping offloads %d for port %uqueue 
%d",
+   DRV_LOG(DEBUG, "port %u set VLAN stripping offloads %d for port %u 
queue %d",
dev->data->port_id, on, rxq_data->port_id, queue);
if (rxq->ctrl->obj == NULL) {
/* Update related bits in RX queue. */
-- 
2.39.5



Re: [PATCH v2 1/7] dts: escape single quotes

2025-04-25 Thread Nicholas Pratte
Reviewed-by: Nicholas Pratte 

On Fri, Mar 14, 2025 at 9:19 AM Luca Vizzarro  wrote:
>
> When making any command a privileged one in a LinuxSession, there
> currently is no consideration whether this command already includes single
> quotes. Therefore escape the existing single quotes before making the
> command.
>
> Signed-off-by: Luca Vizzarro 
> Reviewed-by: Paul Szczepanek 
> ---
>  dts/framework/testbed_model/linux_session.py | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/dts/framework/testbed_model/linux_session.py 
> b/dts/framework/testbed_model/linux_session.py
> index 7c2b110c99..6c6a4b608d 100644
> --- a/dts/framework/testbed_model/linux_session.py
> +++ b/dts/framework/testbed_model/linux_session.py
> @@ -67,6 +67,7 @@ class LinuxSession(PosixSession):
>
>  @staticmethod
>  def _get_privileged_command(command: str) -> str:
> +command = command.replace(r"'", r"\'")
>  return f"sudo -- sh -c '{command}'"
>
>  def get_remote_cpus(self) -> list[LogicalCore]:
> --
> 2.43.0
>


[PATCH] net/mlx5: fix modify field action on group 0

2025-04-25 Thread Dariusz Sosnowski
HW modify header commands generated for multiple modify field
flow actions, which modify/access the same packet fields
do not have to be separated by NOPs when used on group 0.
This is because:

- On group > 0, HW uses Modify Header Pattern objects
  which require NOP explicitly.
- On group 0, modify field action is implemented using
  Modify Header Context object managed by FW.
  FW inserts requires NOPs internally.

mlx5 PMD inserted NOP always, which caused flow/table creation
failures on group 0 flow rules.
This patch addresses that.

Fixes: 0f4aa72b99da ("net/mlx5: support flow modify field with HWS")
Cc: suanmi...@nvidia.com
Cc: sta...@dpdk.org

Signed-off-by: Dariusz Sosnowski 
Acked-by: Bing Zhao 
---
 drivers/net/mlx5/mlx5_flow_hw.c | 21 -
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 20d38ce414..5ae6ac1cfa 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -1422,7 +1422,8 @@ flow_hw_action_modify_field_is_shared(const struct 
rte_flow_action *action,
 
 static __rte_always_inline bool
 flow_hw_should_insert_nop(const struct mlx5_hw_modify_header_action *mhdr,
- const struct mlx5_modification_cmd *cmd)
+ const struct mlx5_modification_cmd *cmd,
+ const struct rte_flow_attr *attr)
 {
struct mlx5_modification_cmd last_cmd = { { 0 } };
struct mlx5_modification_cmd new_cmd = { { 0 } };
@@ -1430,6 +1431,15 @@ flow_hw_should_insert_nop(const struct 
mlx5_hw_modify_header_action *mhdr,
unsigned int last_type;
bool should_insert = false;
 
+   /*
+* Modify header action list does not require NOPs in root table,
+* because different type of underlying object is used:
+* - in root table - MODIFY_HEADER_CONTEXT (does not require NOPs),
+* - in non-root - either inline modify action or based on Modify 
Header Pattern
+*   (which requires NOPs).
+*/
+   if (attr->group == 0)
+   return false;
if (cmds_num == 0)
return false;
last_cmd = *(&mhdr->mhdr_cmds[cmds_num - 1]);
@@ -1508,7 +1518,8 @@ flow_hw_mhdr_cmd_append(struct 
mlx5_hw_modify_header_action *mhdr,
 
 static __rte_always_inline int
 flow_hw_converted_mhdr_cmds_append(struct mlx5_hw_modify_header_action *mhdr,
-  struct mlx5_flow_dv_modify_hdr_resource 
*resource)
+  struct mlx5_flow_dv_modify_hdr_resource 
*resource,
+  const struct rte_flow_attr *attr)
 {
uint32_t idx;
int ret;
@@ -1516,7 +1527,7 @@ flow_hw_converted_mhdr_cmds_append(struct 
mlx5_hw_modify_header_action *mhdr,
for (idx = 0; idx < resource->actions_num; ++idx) {
struct mlx5_modification_cmd *src = &resource->actions[idx];
 
-   if (flow_hw_should_insert_nop(mhdr, src)) {
+   if (flow_hw_should_insert_nop(mhdr, src, attr)) {
ret = flow_hw_mhdr_cmd_nop_append(mhdr);
if (ret)
return ret;
@@ -1639,14 +1650,14 @@ flow_hw_modify_field_compile(struct rte_eth_dev *dev,
 * This NOP command will not be a part of action's command range used 
to update commands
 * on rule creation.
 */
-   if (flow_hw_should_insert_nop(mhdr, &resource->actions[0])) {
+   if (flow_hw_should_insert_nop(mhdr, &resource->actions[0], attr)) {
ret = flow_hw_mhdr_cmd_nop_append(mhdr);
if (ret)
return rte_flow_error_set(error, ret, 
RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
  NULL, "too many modify field 
operations specified");
}
cmds_start = mhdr->mhdr_cmds_num;
-   ret = flow_hw_converted_mhdr_cmds_append(mhdr, resource);
+   ret = flow_hw_converted_mhdr_cmds_append(mhdr, resource, attr);
if (ret)
return rte_flow_error_set(error, ret, 
RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
  NULL, "too many modify field 
operations specified");
-- 
2.39.5



[PATCH] net/mlx5: validate GTP PSC QFI width

2025-04-25 Thread Dariusz Sosnowski
Add missing validation of GTP PSC QFI flow field width for
modify field flow action.

Fixes: 0f4aa72b99da ("net/mlx5: support flow modify field with HWS")
Cc: suanmi...@nvidia.com
Cc: sta...@dpdk.org

Signed-off-by: Dariusz Sosnowski 
Acked-by: Bing Zhao 
---
 drivers/net/mlx5/mlx5_flow_dv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 608c42db1d..c217634d9b 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -1451,6 +1451,8 @@ mlx5_flow_item_field_width(struct rte_eth_dev *dev,
case RTE_FLOW_FIELD_META:
return (flow_dv_get_metadata_reg(dev, attr, error) == REG_C_0) ?
rte_popcount32(priv->sh->dv_meta_mask) : 32;
+   case RTE_FLOW_FIELD_GTP_PSC_QFI:
+   return 6;
case RTE_FLOW_FIELD_POINTER:
case RTE_FLOW_FIELD_VALUE:
return inherit < 0 ? 0 : inherit;
-- 
2.39.5



[PATCH 2/2] net/mlx5: fix counter service thread init

2025-04-25 Thread Dariusz Sosnowski
During counter pool initialization (mlx5_hws_cnt_pool_create()),
a background service thread is started by mlx5 PMD,
which refreshes counter values periodically.

During initialization it might happen that:

- background thread creation succeeds,
- pool creation fails (e.g. due to error in configuration
  provided by the user).

In this case, the procedure on error rollback will not clean up
counter service thread, because rollback assumes that
is there is no pool, then there is nothing to clean up.

This patch fixes that, by moving background thread creation
to the end of counter pool initialization.
This ensures proper order of resource release during rollback.
Also, this patch adds missing "goto error" in case of
failure to initialize the service thread.

Fixes: 4d368e1da3a4 ("net/mlx5: support flow counter action for HWS")
Cc: jack...@nvidia.com
Cc: sta...@dpdk.org

Signed-off-by: Dariusz Sosnowski 
Acked-by: Bing Zhao 
---
 drivers/net/mlx5/mlx5_hws_cnt.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_hws_cnt.c b/drivers/net/mlx5/mlx5_hws_cnt.c
index fd12bcd7ec..5c7c0041aa 100644
--- a/drivers/net/mlx5/mlx5_hws_cnt.c
+++ b/drivers/net/mlx5/mlx5_hws_cnt.c
@@ -729,12 +729,6 @@ mlx5_hws_cnt_pool_create(struct rte_eth_dev *dev,
}
goto success;
}
-   /* init cnt service if not. */
-   if (priv->sh->cnt_svc == NULL) {
-   ret = mlx5_hws_cnt_svc_init(priv->sh, error);
-   if (ret)
-   goto error;
-   }
cparam.fetch_sz = HWS_CNT_CACHE_FETCH_DEFAULT;
cparam.preload_sz = HWS_CNT_CACHE_PRELOAD_DEFAULT;
cparam.q_num = nb_queue;
@@ -769,6 +763,12 @@ mlx5_hws_cnt_pool_create(struct rte_eth_dev *dev,
   NULL, "failed to allocate counter actions");
goto error;
}
+   /* init cnt service if not. */
+   if (priv->sh->cnt_svc == NULL) {
+   ret = mlx5_hws_cnt_svc_init(priv->sh, error);
+   if (ret)
+   goto error;
+   }
priv->sh->cnt_svc->refcnt++;
cpool->priv = priv;
rte_spinlock_lock(&priv->sh->cpool_lock);
-- 
2.39.5



[PATCH 1/2] net/mlx5: fix counter pool init error propagation

2025-04-25 Thread Dariusz Sosnowski
In case of an error in mlx5_hws_cnt_pool_create(),
value stored in ret is used as "error code".
There are however a few cases inside this function,
when this variable is not set, leading to failed
assertions (e.g., when requested number of counters is bigger
than maximum supported).

This patch addresses these cases, by propagating the rte_errno
set by failing functions.

Fixes: e1c83d295dd9 ("net/mlx5: support ASO actions with non-template flow")
Cc: mkash...@nvidia.com
Cc: sta...@dpdk.org

Signed-off-by: Dariusz Sosnowski 
Acked-by: Bing Zhao 
---
 drivers/net/mlx5/mlx5_hws_cnt.c | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_hws_cnt.c b/drivers/net/mlx5/mlx5_hws_cnt.c
index 7b5e7310af..fd12bcd7ec 100644
--- a/drivers/net/mlx5/mlx5_hws_cnt.c
+++ b/drivers/net/mlx5/mlx5_hws_cnt.c
@@ -704,8 +704,11 @@ mlx5_hws_cnt_pool_create(struct rte_eth_dev *dev,
size_t sz;
 
mp_name = mlx5_malloc(MLX5_MEM_ZERO, RTE_MEMZONE_NAMESIZE, 0, 
SOCKET_ID_ANY);
-   if (mp_name == NULL)
+   if (mp_name == NULL) {
+   ret = rte_flow_error_set(error, ENOMEM, 
RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
+"failed to allocate counter pool name 
prefix");
goto error;
+   }
snprintf(mp_name, RTE_MEMZONE_NAMESIZE, "MLX5_HWS_CNT_P_%x", 
dev->data->port_id);
pcfg.name = mp_name;
pcfg.request_num = nb_counters;
@@ -713,8 +716,10 @@ mlx5_hws_cnt_pool_create(struct rte_eth_dev *dev,
if (chost) {
pcfg.host_cpool = chost;
cpool = mlx5_hws_cnt_pool_init(priv->sh, &pcfg, &cparam, error);
-   if (cpool == NULL)
+   if (cpool == NULL) {
+   ret = -rte_errno;
goto error;
+   }
ret = mlx5_hws_cnt_pool_action_create(priv, cpool);
if (ret != 0) {
rte_flow_error_set(error, -ret,
@@ -736,15 +741,19 @@ mlx5_hws_cnt_pool_create(struct rte_eth_dev *dev,
cparam.threshold = HWS_CNT_CACHE_THRESHOLD_DEFAULT;
cparam.size = HWS_CNT_CACHE_SZ_DEFAULT;
cpool = mlx5_hws_cnt_pool_init(priv->sh, &pcfg, &cparam, error);
-   if (cpool == NULL)
+   if (cpool == NULL) {
+   ret = -rte_errno;
goto error;
+   }
ret = mlx5_hws_cnt_pool_dcs_alloc(priv->sh, cpool, error);
if (ret != 0)
goto error;
sz = RTE_ALIGN_CEIL(mlx5_hws_cnt_pool_get_size(cpool), 4);
cpool->raw_mng = mlx5_hws_cnt_raw_data_alloc(priv->sh, sz, error);
-   if (cpool->raw_mng == NULL)
+   if (cpool->raw_mng == NULL) {
+   ret = -rte_errno;
goto error;
+   }
__hws_cnt_id_load(cpool);
/*
 * Bump query gen right after pool create so the
-- 
2.39.5



[PATCH 0/2] net/mlx5: flow counter pool fixes

2025-04-25 Thread Dariusz Sosnowski
This patch series includes several fixes for flow counter pool
used with HW Steering flow engine.

Dariusz Sosnowski (2):
  net/mlx5: fix counter pool init error propagation
  net/mlx5: fix counter service thread init

 drivers/net/mlx5/mlx5_hws_cnt.c | 29 +++--
 1 file changed, 19 insertions(+), 10 deletions(-)

--
2.39.5



rte_eth_stats_get seems slow

2025-04-25 Thread Morten Brørup
Bruce,

rte_eth_stats_get() on Intel NICs seems slow to me.

E.g. getting the stats on a single port takes ~132 us (~451,000 CPU cycles) 
using the igb driver, and ~50 us using the i40e driver.

Referring to the igb driver source code [1], it's 44 calls to E1000_READ_REG(), 
so the math says that each one takes 3 us (~10,000 CPU cycles).

Is this expected behavior?

It adds up, e.g. it takes a full millisecond to fetch the stats from eight 
ports using the igb driver.

[1]: 
https://elixir.bootlin.com/dpdk/v24.11.1/source/drivers/net/e1000/igb_ethdev.c#L1724


Med venlig hilsen / Kind regards,
-Morten Brørup



Re: [PATCH v2] net/mlx5: mitigate the Tx queue parameter adjustment

2025-04-25 Thread Lukáš Šišmiš

Hello all,

I tested v1 patch on CX-4 card and I can confirm my application boots now!
For traceability I am adding the original discussion thread:
https://mails.dpdk.org/archives/users/2025-April/008242.html

Probably a some other problem, but it still outputs these logs:

Config: dpdk: :b3:00.0: setting up TX queue 0: tx_desc: 32768 tx: 
offloads: 0x1 hthresh: 0 pthresh: 0 wthresh: 0 tx_free_thresh: 0 
tx_rs_thresh: 0 txq_deferred_start: 0 
[DeviceConfigureQueues:runmode-dpdk.c:1487]

mlx5_net: adjust txq_inline_max (290->18) due to large Tx queue on port 1
Config: dpdk: :b3:00.0: setting up TX queue 1: tx_desc: 32768 tx: 
offloads: 0x1 hthresh: 0 pthresh: 0 wthresh: 0 tx_free_thresh: 0 
tx_rs_thresh: 0 txq_deferred_start: 0 
[DeviceConfigureQueues:runmode-dpdk.c:1487]

mlx5_net: adjust txq_inline_max (290->18) due to large Tx queue on port 1
Config: dpdk: :b3:00.0: setting up TX queue 2: tx_desc: 32768 tx: 
offloads: 0x1 hthresh: 0 pthresh: 0 wthresh: 0 tx_free_thresh: 0 
tx_rs_thresh: 0 txq_deferred_start: 0 
[DeviceConfigureQueues:runmode-dpdk.c:1487]

mlx5_net: adjust txq_inline_max (290->18) due to large Tx queue on port 1

Is there any way how I can avoid these logs?

Thank you.

Lukas

On 4/24/25 15:31, Viacheslav Ovsiienko wrote:

he DPDK API rte_eth_tx_queue_setup() has a parameter nb_tx_desc
specifying the desired queue capacity, measured in packets.

The ConnectX NIC series has a hardware-imposed queue size
limit of 32K WQEs (packet hardware descriptors). Typically,
one packet requires one WQE to be sent.

There is a special offload option, data-inlining, to improve
performance for small packets. Also, NICs in some configurations
require a minimum amount of inline data for the steering engine
to operate correctly.

In the case of inline data, more than one WQEs might be required
to send a single packet. The mlx5 PMD takes this into account
and adjusts the number of queue WQEs accordingly.

If the requested queue capacity can't be satisfied due to
the hardware queue size limit, the mlx5 PMD rejected the queue
creation, causing unresolvable application failure.

The patch provides the following:

- fixes the calculation of the number of required WQEs
   to send a single packet with inline data, making it more precise
   and extending the painless operating range.

- If the requested queue capacity can't be satisfied due to WQE
   number adjustment for inline data, it no longer causes a severe
   error. Instead, a warning message is emitted, and the queue
   is created with the maximum available size, with a reported success.

   Please note that the inline data size depends on many options
   (NIC configuration, queue offload flags, packet offload flags,
packet size, etc.), so the actual queue capacity might not be
impacted at all.

Signed-off-by: Viacheslav Ovsiienko 
Acked-by: Dariusz Sosnowski 

---
v2: diagnostics messages made less wordy
---
  drivers/net/mlx5/mlx5_txq.c | 74 +++--
  1 file changed, 22 insertions(+), 52 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_txq.c b/drivers/net/mlx5/mlx5_txq.c
index 3e93517323..eebf3c2534 100644
--- a/drivers/net/mlx5/mlx5_txq.c
+++ b/drivers/net/mlx5/mlx5_txq.c
@@ -731,7 +731,7 @@ txq_calc_inline_max(struct mlx5_txq_ctrl *txq_ctrl)
if (!wqe_size)
return 0;
/*
-* This calculation is derived from tthe source of
+* This calculation is derived from the source of
 * mlx5_calc_send_wqe() in rdma_core library.
 */
wqe_size = wqe_size * MLX5_WQE_SIZE -
@@ -739,7 +739,7 @@ txq_calc_inline_max(struct mlx5_txq_ctrl *txq_ctrl)
   MLX5_WQE_ESEG_SIZE -
   MLX5_WSEG_SIZE -
   MLX5_WSEG_SIZE +
-  MLX5_DSEG_MIN_INLINE_SIZE;
+  MLX5_ESEG_MIN_INLINE_SIZE;
return wqe_size;
  }
  
@@ -964,11 +964,8 @@ txq_set_params(struct mlx5_txq_ctrl *txq_ctrl)

   *
   * @param txq_ctrl
   *   Pointer to Tx queue control structure.
- *
- * @return
- *   Zero on success, otherwise the parameters can not be adjusted.
   */
-static int
+static void
  txq_adjust_params(struct mlx5_txq_ctrl *txq_ctrl)
  {
struct mlx5_priv *priv = txq_ctrl->priv;
@@ -981,82 +978,56 @@ txq_adjust_params(struct mlx5_txq_ctrl *txq_ctrl)
 * Inline data feature is not engaged at all.
 * There is nothing to adjust.
 */
-   return 0;
+   return;
}
if (txq_ctrl->max_inline_data <= max_inline) {
/*
 * The requested inline data length does not
 * exceed queue capabilities.
 */
-   return 0;
+   return;
}
if (txq_ctrl->txq.inlen_mode > max_inline) {
-   DRV_LOG(ERR,
-   "minimal data inline requirements (%u) are not"
-   " satisfied (%u) on port

[PATCH] net/mlx5: optimize counters ID greneration logic

2025-04-25 Thread Alexander Kozyrev
Enqueue generated counter IDs on a ring in bulk.
Generate them and store in an array before putting them
on a ring all at once. That bring better cache access
and speeds up the mlx5_hws_cnt_pool_create() function.

Signed-off-by: Alexander Kozyrev 
---
 drivers/net/mlx5/mlx5_hws_cnt.c | 19 ++-
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_hws_cnt.c b/drivers/net/mlx5/mlx5_hws_cnt.c
index 7b5e7310af..2d2faa2c65 100644
--- a/drivers/net/mlx5/mlx5_hws_cnt.c
+++ b/drivers/net/mlx5/mlx5_hws_cnt.c
@@ -22,12 +22,17 @@
 #define HWS_CNT_CACHE_THRESHOLD_DEFAULT 254
 #define HWS_CNT_ALLOC_FACTOR_DEFAULT 20
 
-static void
+static int
 __hws_cnt_id_load(struct mlx5_hws_cnt_pool *cpool)
 {
uint32_t cnt_num = mlx5_hws_cnt_pool_get_size(cpool);
uint32_t iidx;
+   cnt_id_t *cnt_arr = NULL;
 
+   cnt_arr = mlx5_malloc(MLX5_MEM_ANY | MLX5_MEM_ZERO,
+ cnt_num * sizeof(cnt_id_t), 0, SOCKET_ID_ANY);
+   if (cnt_arr == NULL)
+   return -ENOMEM;
/*
 * Counter ID order is important for tracking the max number of in used
 * counter for querying, which means counter internal index order must
@@ -38,10 +43,12 @@ __hws_cnt_id_load(struct mlx5_hws_cnt_pool *cpool)
 */
for (iidx = 0; iidx < cnt_num; iidx++) {
cnt_id_t cnt_id  = mlx5_hws_cnt_id_gen(cpool, iidx);
-
-   rte_ring_enqueue_elem(cpool->free_list, &cnt_id,
-   sizeof(cnt_id));
+   cnt_arr[iidx] = cnt_id;
}
+   rte_ring_enqueue_bulk_elem(cpool->free_list, cnt_arr,
+  sizeof(cnt_id_t), cnt_num, NULL);
+   mlx5_free(cnt_arr);
+   return 0;
 }
 
 static void
@@ -745,7 +752,9 @@ mlx5_hws_cnt_pool_create(struct rte_eth_dev *dev,
cpool->raw_mng = mlx5_hws_cnt_raw_data_alloc(priv->sh, sz, error);
if (cpool->raw_mng == NULL)
goto error;
-   __hws_cnt_id_load(cpool);
+   ret = __hws_cnt_id_load(cpool);
+   if (ret != 0)
+   goto error;
/*
 * Bump query gen right after pool create so the
 * pre-loaded counters can be used directly
-- 
2.43.5



Re: [PATCH v2 5/7] dts: make shells path dynamic

2025-04-25 Thread Nicholas Pratte
Reviewed-by: Nicholas Pratte 

On Fri, Mar 14, 2025 at 9:19 AM Luca Vizzarro  wrote:
>
> Turn the `path` attribute of InteractiveShell into a method property.
> This allows path to both be defined statically by the class
> implementation and also to be defined dynamically as part of the class
> construction.
>
> Signed-off-by: Luca Vizzarro 
> Reviewed-by: Paul Szczepanek 
> ---
>  dts/framework/remote_session/dpdk_app.py  |  9 +++-
>  dts/framework/remote_session/dpdk_shell.py| 18 ---
>  .../remote_session/interactive_shell.py   | 22 ---
>  dts/framework/remote_session/python_shell.py  |  6 +++--
>  dts/framework/remote_session/testpmd_shell.py |  8 ---
>  5 files changed, 36 insertions(+), 27 deletions(-)
>
> diff --git a/dts/framework/remote_session/dpdk_app.py 
> b/dts/framework/remote_session/dpdk_app.py
> index c5aae05e02..dc4b817bdd 100644
> --- a/dts/framework/remote_session/dpdk_app.py
> +++ b/dts/framework/remote_session/dpdk_app.py
> @@ -54,7 +54,14 @@ def __init__(
>  eal_params.append_str(app_params)
>  app_params = eal_params
>
> -super().__init__(name, privileged, path, app_params)
> +self._path = path
> +
> +super().__init__(name, privileged, app_params)
> +
> +@property
> +def path(self) -> PurePath:
> +"""The path of the DPDK app relative to the DPDK build folder."""
> +return self._path
>
>  def wait_until_ready(self, end_token: str) -> None:
>  """Start app and wait until ready.
> diff --git a/dts/framework/remote_session/dpdk_shell.py 
> b/dts/framework/remote_session/dpdk_shell.py
> index 24a39482ce..2d4f91052d 100644
> --- a/dts/framework/remote_session/dpdk_shell.py
> +++ b/dts/framework/remote_session/dpdk_shell.py
> @@ -6,7 +6,7 @@
>  Provides a base class to create interactive shells based on DPDK.
>  """
>
> -from abc import ABC
> +from abc import ABC, abstractmethod
>  from pathlib import PurePath
>
>  from framework.context import get_ctx
> @@ -65,20 +65,22 @@ def __init__(
>  self,
>  name: str | None = None,
>  privileged: bool = True,
> -path: PurePath | None = None,
>  app_params: EalParams = EalParams(),
>  ) -> None:
>  """Extends :meth:`~.interactive_shell.InteractiveShell.__init__`."""
>  app_params = compute_eal_params(app_params)
>  node = get_ctx().sut_node
>
> -super().__init__(node, name, privileged, path, app_params)
> +super().__init__(node, name, privileged, app_params)
>
> -def _update_real_path(self, path: PurePath) -> None:
> -"""Extends 
> :meth:`~.interactive_shell.InteractiveShell._update_real_path`.
> +@property
> +@abstractmethod
> +def path(self) -> PurePath:
> +"""Relative path to the shell executable from the build folder."""
> +
> +def _make_real_path(self):
> +"""Overrides 
> :meth:`~.interactive_shell.InteractiveShell._make_real_path`.
>
>  Adds the remote DPDK build directory to the path.
>  """
> -super()._update_real_path(
> -
> PurePath(get_ctx().dpdk_build.remote_dpdk_build_dir).joinpath(path)
> -)
> +return get_ctx().dpdk_build.remote_dpdk_build_dir.joinpath(self.path)
> diff --git a/dts/framework/remote_session/interactive_shell.py 
> b/dts/framework/remote_session/interactive_shell.py
> index 62f9984d3b..6b7ca0b6af 100644
> --- a/dts/framework/remote_session/interactive_shell.py
> +++ b/dts/framework/remote_session/interactive_shell.py
> @@ -21,7 +21,7 @@
>  environment variable configure the timeout of getting the output from 
> command execution.
>  """
>
> -from abc import ABC
> +from abc import ABC, abstractmethod
>  from pathlib import PurePath
>  from typing import ClassVar
>
> @@ -66,7 +66,6 @@ class InteractiveShell(MultiInheritanceBaseClass, ABC):
>  _timeout: float
>  _app_params: Params
>  _privileged: bool
> -_real_path: PurePath
>
>  #: The number of times to try starting the application before 
> considering it a failure.
>  _init_attempts: ClassVar[int] = 5
> @@ -83,9 +82,6 @@ class InteractiveShell(MultiInheritanceBaseClass, ABC):
>  #: the expected prompt.
>  _command_extra_chars: ClassVar[str] = ""
>
> -#: Path to the executable to start the interactive application.
> -path: ClassVar[PurePath]
> -
>  is_alive: bool = False
>
>  def __init__(
> @@ -93,7 +89,6 @@ def __init__(
>  node: Node,
>  name: str | None = None,
>  privileged: bool = False,
> -path: PurePath | None = None,
>  app_params: Params = Params(),
>  **kwargs,
>  ) -> None:
> @@ -107,7 +102,6 @@ def __init__(
>  name: Name for the interactive shell to use for logging. This 
> name will be appended to
>  the name of the underlying node which it is running on.
>  privileged: Enables the shell to run as superuser.
> -

Regarding Mellanox bifurcated driver on Azure

2025-04-25 Thread Prashant Upadhyaya
Hi,

I am having a VM on Azure where I have got two 'accelerated networking'
interfaces of Mellanox
# lspci -nn|grep -i ether
6561:00:02.0 Ethernet controller [0200]: Mellanox Technologies MT27710
Family [ConnectX-4 Lx Virtual Function] [15b3:1016] (rev 80)
f08c:00:02.0 Ethernet controller [0200]: Mellanox Technologies MT27710
Family [ConnectX-4 Lx Virtual Function] [15b3:1016] (rev 80)

I have a DPDK application which needs to obtain 'all' packets from the NIC.
I installed the drivers, compiled DPDK24.11 (Ubuntu20.04), my app starts
and is able to detect the NIC's.
Everything looks good
myapp.out -c 0x07 -a f08c:00:02.0 -a 6561:00:02.0
EAL: Detected CPU lcores: 8
EAL: Detected NUMA nodes: 1
EAL: Detected shared linkage of DPDK
EAL: Multi-process socket /var/run/dpdk/rte/mp_socket
EAL: Selected IOVA mode 'PA'
EAL: VFIO support initialized
mlx5_net: Default miss action is not supported.
mlx5_net: Default miss action is not supported.
All Ports initialized
Port 0 is UP (5 Mbps)
Port 1 is UP (5 Mbps)

The trouble is that the ARP packets are not being picked up by my DPDK
application, I see them being delivered to the kernel via the eth interface
corresponding to the port (MLX is a bifurcated driver, you don't really
bind to the NIC, so you still see the eth interfaces at linux level and can
run tcpdump on those, I see ARP packets in the tcpdump there on the
interface)
I can receive UDP packets in my DPDK app though.

My application is not setting any rte_flow rules etc. so I was expecting
that by default my dpdk app would get all the packets as is normally the
case with other NIC's
Is there something I need to configure for Mellanox NIC somewhere such that
I get 'all' the packets including ARP packets in my DPDK app ?

Regards
-Prashant


Re: [PATCH v2 4/7] dts: revert back to a single InteractiveShell

2025-04-25 Thread Nicholas Pratte
Reviewed-by: Nicholas Pratte 

On Fri, Mar 14, 2025 at 9:19 AM Luca Vizzarro  wrote:
>
> Previously InteractiveShell was split into two classes to differentiate
> a shell which execution must be controlled in a tight scope through a
> context manager, from a more looser approach. With the addition of the
> shell pool this is no longer needed as the management of shells is now
> delegated to the test run instead of the test suites.
>
> Revert back to a single InteractiveShell to simplify the code. Keep the
> context manager implementation but also render start_application and
> close public methods again.
>
> Signed-off-by: Luca Vizzarro 
> Reviewed-by: Paul Szczepanek 
> ---
>  dts/framework/remote_session/dpdk_app.py  |   4 +-
>  dts/framework/remote_session/dpdk_shell.py|   6 +-
>  .../remote_session/interactive_shell.py   | 289 --
>  dts/framework/remote_session/python_shell.py  |   4 +
>  dts/framework/remote_session/shell_pool.py|   2 +-
>  .../single_active_interactive_shell.py| 275 -
>  dts/framework/remote_session/testpmd_shell.py |   4 +-
>  7 files changed, 275 insertions(+), 309 deletions(-)
>  delete mode 100644 
> dts/framework/remote_session/single_active_interactive_shell.py
>
> diff --git a/dts/framework/remote_session/dpdk_app.py 
> b/dts/framework/remote_session/dpdk_app.py
> index c9945f302d..c5aae05e02 100644
> --- a/dts/framework/remote_session/dpdk_app.py
> +++ b/dts/framework/remote_session/dpdk_app.py
> @@ -62,7 +62,7 @@ def wait_until_ready(self, end_token: str) -> None:
>  Args:
>  end_token: The string at the end of a line that indicates the 
> app is ready.
>  """
> -self._start_application(end_token)
> +self.start_application(end_token)
>
>  def close(self) -> None:
>  """Close the application.
> @@ -70,4 +70,4 @@ def close(self) -> None:
>  Sends a SIGINT to close the application.
>  """
>  self.send_command("\x03")
> -self._close()
> +super().close()
> diff --git a/dts/framework/remote_session/dpdk_shell.py 
> b/dts/framework/remote_session/dpdk_shell.py
> index f7ea2588ca..24a39482ce 100644
> --- a/dts/framework/remote_session/dpdk_shell.py
> +++ b/dts/framework/remote_session/dpdk_shell.py
> @@ -11,8 +11,8 @@
>
>  from framework.context import get_ctx
>  from framework.params.eal import EalParams
> -from framework.remote_session.single_active_interactive_shell import (
> -SingleActiveInteractiveShell,
> +from framework.remote_session.interactive_shell import (
> +InteractiveShell,
>  )
>  from framework.testbed_model.cpu import LogicalCoreList
>
> @@ -51,7 +51,7 @@ def compute_eal_params(
>  return params
>
>
> -class DPDKShell(SingleActiveInteractiveShell, ABC):
> +class DPDKShell(InteractiveShell, ABC):
>  """The base class for managing DPDK-based interactive shells.
>
>  This class shouldn't be instantiated directly, but instead be extended.
> diff --git a/dts/framework/remote_session/interactive_shell.py 
> b/dts/framework/remote_session/interactive_shell.py
> index 9ca285b604..62f9984d3b 100644
> --- a/dts/framework/remote_session/interactive_shell.py
> +++ b/dts/framework/remote_session/interactive_shell.py
> @@ -1,44 +1,281 @@
>  # SPDX-License-Identifier: BSD-3-Clause
> -# Copyright(c) 2023 University of New Hampshire
> +# Copyright(c) 2024 University of New Hampshire
>  # Copyright(c) 2024 Arm Limited
>
> -"""Interactive shell with manual stop/start functionality.
> +"""Common functionality for interactive shell handling.
>
> -Provides a class that doesn't require being started/stopped using a context 
> manager and can instead
> -be started and stopped manually, or have the stopping process be handled at 
> the time of garbage
> -collection.
> +The base class, :class:`InteractiveShell`, is meant to be extended by 
> subclasses that
> +contain functionality specific to that shell type. These subclasses will 
> often modify things like
> +the prompt to expect or the arguments to pass into the application, but 
> still utilize
> +the same method for sending a command and collecting output. How this output 
> is handled however
> +is often application specific. If an application needs elevated privileges 
> to start it is expected
> +that the method for gaining those privileges is provided when initializing 
> the class.
> +
> +This class is designed for applications like primary applications in DPDK 
> where only one instance
> +of the application can be running at a given time and, for this reason, is 
> managed using a context
> +manager. This context manager starts the application when you enter the 
> context and cleans up the
> +application when you exit. Using a context manager for this is useful since 
> it allows us to ensure
> +the application is cleaned up as soon as you leave the block regardless of 
> the reason.
> +
> +The :option:`--timeout` command line argument and the :envvar:`DTS_

Re: [PATCH v2 7/7] dts: enable shell pooling

2025-04-25 Thread Nicholas Pratte
Reviewed-by: Nicholas Pratte 

On Fri, Mar 14, 2025 at 9:19 AM Luca Vizzarro  wrote:
>
> Enable the ShellPool class to be used in the test run, and let
> InteractiveShells register themselves to the pool upon shell startup.
> Moreover, to avoid the ShellPool to call InteractiveShell.close more
> than once if the shell was not unregistered correctly, add a way to
> prevent the method to be called if the shell has already been closed.
>
> Signed-off-by: Luca Vizzarro 
> Reviewed-by: Paul Szczepanek 
> ---
>  .../remote_session/interactive_shell.py   | 24 ++-
>  dts/framework/remote_session/python_shell.py  |  3 ++-
>  dts/framework/remote_session/testpmd_shell.py |  2 ++
>  dts/framework/test_run.py |  5 
>  4 files changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/dts/framework/remote_session/interactive_shell.py 
> b/dts/framework/remote_session/interactive_shell.py
> index d7e566e5c4..ba8489eafa 100644
> --- a/dts/framework/remote_session/interactive_shell.py
> +++ b/dts/framework/remote_session/interactive_shell.py
> @@ -22,12 +22,14 @@
>  """
>
>  from abc import ABC, abstractmethod
> +from collections.abc import Callable
>  from pathlib import PurePath
> -from typing import ClassVar
> +from typing import ClassVar, Concatenate, ParamSpec, TypeAlias, TypeVar
>
>  from paramiko import Channel, channel
>  from typing_extensions import Self
>
> +from framework.context import get_ctx
>  from framework.exception import (
>  InteractiveCommandExecutionError,
>  InteractiveSSHSessionDeadError,
> @@ -38,6 +40,23 @@
>  from framework.settings import SETTINGS
>  from framework.testbed_model.node import Node
>
> +P = ParamSpec("P")
> +T = TypeVar("T", bound="InteractiveShell")
> +R = TypeVar("R")
> +InteractiveShellMethod = Callable[Concatenate[T, P], R]
> +InteractiveShellDecorator: TypeAlias = Callable[[InteractiveShellMethod], 
> InteractiveShellMethod]
> +
> +
> +def only_active(func: InteractiveShellMethod) -> InteractiveShellMethod:
> +"""This decorator will skip running the method if the SSH channel is not 
> active."""
> +
> +def _wrapper(self: "InteractiveShell", *args: P.args, **kwargs: 
> P.kwargs) -> R | None:
> +if self._ssh_channel.active:
> +return func(self, *args, **kwargs)
> +return None
> +
> +return _wrapper
> +
>
>  class InteractiveShell(ABC):
>  """The base class for managing interactive shells.
> @@ -155,6 +174,7 @@ def start_application(self, prompt: str | None = None) -> 
> None:
>  self.is_alive = False  # update state on failure to start
>  raise InteractiveCommandExecutionError("Failed to start 
> application.")
>  self._ssh_channel.settimeout(self._timeout)
> +get_ctx().shell_pool.register_shell(self)
>
>  def send_command(
>  self, command: str, prompt: str | None = None, skip_first_line: bool 
> = False
> @@ -219,6 +239,7 @@ def send_command(
>  self._logger.debug(f"Got output: {out}")
>  return out
>
> +@only_active
>  def close(self) -> None:
>  """Close the shell.
>
> @@ -234,6 +255,7 @@ def close(self) -> None:
>  self._logger.debug("Application failed to exit before set 
> timeout.")
>  raise InteractiveSSHTimeoutError("Application 'exit' command") 
> from e
>  self._ssh_channel.close()
> +get_ctx().shell_pool.unregister_shell(self)
>
>  @property
>  @abstractmethod
> diff --git a/dts/framework/remote_session/python_shell.py 
> b/dts/framework/remote_session/python_shell.py
> index 6331d047c4..5b380a5c7a 100644
> --- a/dts/framework/remote_session/python_shell.py
> +++ b/dts/framework/remote_session/python_shell.py
> @@ -15,7 +15,7 @@
>  from pathlib import PurePath
>  from typing import ClassVar
>
> -from .interactive_shell import InteractiveShell
> +from .interactive_shell import InteractiveShell, only_active
>
>
>  class PythonShell(InteractiveShell):
> @@ -32,6 +32,7 @@ def path(self) -> PurePath:
>  """Path to the Python3 executable."""
>  return PurePath("python3")
>
> +@only_active
>  def close(self):
>  """Close Python shell."""
>  return super().close()
> diff --git a/dts/framework/remote_session/testpmd_shell.py 
> b/dts/framework/remote_session/testpmd_shell.py
> index 19437b6233..b1939e4a51 100644
> --- a/dts/framework/remote_session/testpmd_shell.py
> +++ b/dts/framework/remote_session/testpmd_shell.py
> @@ -33,6 +33,7 @@
>  )
>
>  from framework.context import get_ctx
> +from framework.remote_session.interactive_shell import only_active
>  from framework.testbed_model.topology import TopologyType
>
>  if TYPE_CHECKING or environ.get("DTS_DOC_BUILD"):
> @@ -2314,6 +2315,7 @@ def rx_vxlan(self, vxlan_id: int, port_id: int, enable: 
> bool, verify: bool = Tru
>  self._logger.debug(f"Failed to set VXLAN:\n{vxlan_output}")
>  raise InteractiveCommandExecutionErro

[DPDK/core Bug 1700] BPF callback wait is not MP safe

2025-04-25 Thread bugzilla
https://bugs.dpdk.org/show_bug.cgi?id=1700

Bug ID: 1700
   Summary: BPF callback wait is not MP safe
   Product: DPDK
   Version: 25.03
  Hardware: All
OS: All
Status: UNCONFIRMED
  Severity: minor
  Priority: Normal
 Component: core
  Assignee: dev@dpdk.org
  Reporter: step...@networkplumber.org
  Target Milestone: ---

The mechanism implemented in bpf_pkt.c is like an open coded version of
seqlock.
There is an inherit race because:

If the CPU running the callback doesn't reach the before the count
is executed, it can rance with the CPU doing destroy.

CPU 1:CPU 2:
bpf_eth_unload()
bc = bpf_eth_cbh_find()
  bpf_rx_callback_vm (or
bpf_rx_callback_jit)
rte_eth_remove_rx_callback()
bpf_eth_cbi_unload(bc)
bpf_eth_cbi_wait(bc)

at this point bc->inuse == 0 because call back not started
but is going to be used by CPU 2. And calling rte_bpf_destroy
will lead to use after free.

There is no good way to fix this without using RCU.

Also, the code should be consistently using C11 atomic not barriers.
Not sure if anyone ever uses this code anyway!

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

Re: [PATCH v2 6/7] dts: remove multi-inheritance classes

2025-04-25 Thread Nicholas Pratte
Reviewed-by: Nicholas Pratte 

On Fri, Mar 14, 2025 at 9:19 AM Luca Vizzarro  wrote:
>
> Multi inheritance has proven to be flaky in Python, therefore revert
> back to a simpler approach where classes will only inherit one base
> class. As part of this change, instead of making the
> ScapyTrafficGenerator class inehrit a PythonShell, use simple class
> composition, by making the shell a class attribute.
>
> Signed-off-by: Luca Vizzarro 
> Reviewed-by: Paul Szczepanek 
> ---
>  .../remote_session/interactive_shell.py   |  9 +
>  .../testbed_model/traffic_generator/scapy.py  | 38 +--
>  .../traffic_generator/traffic_generator.py|  6 +--
>  dts/framework/utils.py| 14 ---
>  4 files changed, 22 insertions(+), 45 deletions(-)
>
> diff --git a/dts/framework/remote_session/interactive_shell.py 
> b/dts/framework/remote_session/interactive_shell.py
> index 6b7ca0b6af..d7e566e5c4 100644
> --- a/dts/framework/remote_session/interactive_shell.py
> +++ b/dts/framework/remote_session/interactive_shell.py
> @@ -37,10 +37,9 @@
>  from framework.params import Params
>  from framework.settings import SETTINGS
>  from framework.testbed_model.node import Node
> -from framework.utils import MultiInheritanceBaseClass
>
>
> -class InteractiveShell(MultiInheritanceBaseClass, ABC):
> +class InteractiveShell(ABC):
>  """The base class for managing interactive shells.
>
>  This class shouldn't be instantiated directly, but instead be extended. 
> It contains
> @@ -90,20 +89,15 @@ def __init__(
>  name: str | None = None,
>  privileged: bool = False,
>  app_params: Params = Params(),
> -**kwargs,
>  ) -> None:
>  """Create an SSH channel during initialization.
>
> -Additional keyword arguments can be passed through `kwargs` if 
> needed for fulfilling other
> -constructors in the case of multiple inheritance.
> -
>  Args:
>  node: The node on which to run start the interactive shell.
>  name: Name for the interactive shell to use for logging. This 
> name will be appended to
>  the name of the underlying node which it is running on.
>  privileged: Enables the shell to run as superuser.
>  app_params: The command line parameters to be passed to the 
> application on startup.
> -**kwargs: Any additional arguments if any.
>  """
>  self._node = node
>  if name is None:
> @@ -112,7 +106,6 @@ def __init__(
>  self._app_params = app_params
>  self._privileged = privileged
>  self._timeout = SETTINGS.timeout
> -super().__init__(**kwargs)
>
>  def _setup_ssh_channel(self):
>  self._ssh_channel = 
> self._node.main_session.interactive_session.session.invoke_shell()
> diff --git a/dts/framework/testbed_model/traffic_generator/scapy.py 
> b/dts/framework/testbed_model/traffic_generator/scapy.py
> index 520561b2eb..78a6ded74c 100644
> --- a/dts/framework/testbed_model/traffic_generator/scapy.py
> +++ b/dts/framework/testbed_model/traffic_generator/scapy.py
> @@ -34,7 +34,7 @@
>  from .capturing_traffic_generator import CapturingTrafficGenerator
>
>
> -class ScapyTrafficGenerator(PythonShell, CapturingTrafficGenerator):
> +class ScapyTrafficGenerator(CapturingTrafficGenerator):
>  """Provides access to scapy functions on a traffic generator node.
>
>  This class extends the base with remote execution of scapy functions. 
> All methods for
> @@ -56,6 +56,8 @@ class also extends 
> :class:`.capturing_traffic_generator.CapturingTrafficGenerato
>  first.
>  """
>
> +_shell: PythonShell
> +
>  _config: ScapyTrafficGeneratorConfig
>
>  #: Name of sniffer to ensure the same is used in all places
> @@ -82,25 +84,21 @@ def __init__(self, tg_node: Node, config: 
> ScapyTrafficGeneratorConfig, **kwargs)
>  tg_node.config.os == OS.linux
>  ), "Linux is the only supported OS for scapy traffic generation"
>
> -super().__init__(node=tg_node, config=config, tg_node=tg_node, 
> **kwargs)
> -self.start_application()
> +super().__init__(tg_node=tg_node, config=config, **kwargs)
> +self._shell = PythonShell(tg_node, "scapy", privileged=True)
>
>  def setup(self, ports: Iterable[Port]):
>  """Extends :meth:`.traffic_generator.TrafficGenerator.setup`.
>
> -Brings up the port links.
> +Starts up the traffic generator and brings up the port links.
>  """
> -super().setup(ports)
>  self._tg_node.main_session.bring_up_link(ports)
> +self._shell.start_application()
> +self._shell.send_command("from scapy.all import *")
>
> -def start_application(self) -> None:
> -"""Extends 
> :meth:`framework.remote_session.interactive_shell.start_application`.
> -
> -Adds a command that imports everything from the scapy library 
> immediately after st

Re: Regarding Mellanox bifurcated driver on Azure

2025-04-25 Thread Stephen Hemminger
Short answer Accelerated networking on Azure is not designed to support
bifurcated VF usage

On Fri, Apr 25, 2025, 10:47 Prashant Upadhyaya 
wrote:

> Hi,
>
> I am having a VM on Azure where I have got two 'accelerated networking'
> interfaces of Mellanox
> # lspci -nn|grep -i ether
> 6561:00:02.0 Ethernet controller [0200]: Mellanox Technologies MT27710
> Family [ConnectX-4 Lx Virtual Function] [15b3:1016] (rev 80)
> f08c:00:02.0 Ethernet controller [0200]: Mellanox Technologies MT27710
> Family [ConnectX-4 Lx Virtual Function] [15b3:1016] (rev 80)
>
> I have a DPDK application which needs to obtain 'all' packets from the NIC.
> I installed the drivers, compiled DPDK24.11 (Ubuntu20.04), my app starts
> and is able to detect the NIC's.
> Everything looks good
> myapp.out -c 0x07 -a f08c:00:02.0 -a 6561:00:02.0
> EAL: Detected CPU lcores: 8
> EAL: Detected NUMA nodes: 1
> EAL: Detected shared linkage of DPDK
> EAL: Multi-process socket /var/run/dpdk/rte/mp_socket
> EAL: Selected IOVA mode 'PA'
> EAL: VFIO support initialized
> mlx5_net: Default miss action is not supported.
> mlx5_net: Default miss action is not supported.
> All Ports initialized
> Port 0 is UP (5 Mbps)
> Port 1 is UP (5 Mbps)
>
> The trouble is that the ARP packets are not being picked up by my DPDK
> application, I see them being delivered to the kernel via the eth interface
> corresponding to the port (MLX is a bifurcated driver, you don't really
> bind to the NIC, so you still see the eth interfaces at linux level and can
> run tcpdump on those, I see ARP packets in the tcpdump there on the
> interface)
> I can receive UDP packets in my DPDK app though.
>
> My application is not setting any rte_flow rules etc. so I was expecting
> that by default my dpdk app would get all the packets as is normally the
> case with other NIC's
> Is there something I need to configure for Mellanox NIC somewhere such
> that I get 'all' the packets including ARP packets in my DPDK app ?
>
> Regards
> -Prashant
>
>