Re: [PATCH v5 1/2] ethdev: Add link_speed lanes support
On Sun, Sep 22, 2024 at 10:02 AM Ferruh Yigit wrote:
>
> On 9/4/2024 6:50 PM, Damodharam Ammepalli wrote:
> > Update the eth_dev_ops structure with new function vectors
> > to get, get capabilities and set ethernet link speed lanes.
> > Update the testpmd to provide required config and information
> > display infrastructure.
> >
> > The supporting ethernet controller driver will register callbacks
> > to avail link speed lanes config and get services. This lanes
> > configuration is applicable only when the nic is forced to fixed
> > speeds. In Autonegiation mode, the hardware automatically
> > negotiates the number of lanes.
> >
> > These are the new commands.
> >
> > testpmd> show port 0 speed_lanes capabilities
> >
> > Supported speeds Valid lanes
> > ---
> > 10 Gbps 1
> > 25 Gbps 1
> > 40 Gbps 4
> > 50 Gbps 1 2
> > 100 Gbps 1 2 4
> > 200 Gbps 2 4
> > 400 Gbps 4 8
> > testpmd>
> >
> > testpmd>
> > testpmd> port stop 0
> > testpmd> port config 0 speed_lanes 4
> > testpmd> port config 0 speed 20 duplex full
> > testpmd> port start 0
> > testpmd>
> > testpmd> show port info 0
> >
> > * Infos for port 0 *
> > MAC address: 14:23:F2:C3:BA:D2
> > Device name: :b1:00.0
> > Driver name: net_bnxt
> > Firmware-version: 228.9.115.0
> > Connect to socket: 2
> > memory allocation on the socket: 2
> > Link status: up
> > Link speed: 200 Gbps
> > Active Lanes: 4
> > Link duplex: full-duplex
> > Autoneg status: Off
> >
> > Signed-off-by: Damodharam Ammepalli
> > ---
> > app/test-pmd/cmdline.c | 248 -
> > app/test-pmd/config.c | 4 +
> > lib/ethdev/ethdev_driver.h | 91 ++
> > lib/ethdev/rte_ethdev.c| 52
> > lib/ethdev/rte_ethdev.h| 95 ++
> > lib/ethdev/version.map | 5 +
> > 6 files changed, 494 insertions(+), 1 deletion(-)
> >
> > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> > index b7759e38a8..643102032e 100644
> > --- a/app/test-pmd/cmdline.c
> > +++ b/app/test-pmd/cmdline.c
> > @@ -284,6 +284,9 @@ static void cmd_help_long_parsed(void *parsed_result,
> >
> > "dump_log_types\n"
> > "Dumps the log level for all the dpdk modules\n\n"
> > +
> > + "show port (port_id) speed_lanes capabilities"
> > + " Show speed lanes capabilities of a port.\n\n"
> > );
> > }
> >
> > @@ -823,6 +826,9 @@ static void cmd_help_long_parsed(void *parsed_result,
> > "port config (port_id) txq (queue_id) affinity
> > (value)\n"
> > "Map a Tx queue with an aggregated port "
> > "of the DPDK port\n\n"
> > +
> > + "port config (port_id|all) speed_lanes (0|1|4|8)\n"
> >
>
> This help string, and the implementation, implies there has to be fixed
> lane values, like 1, 4, 8. Why not leave this part to the capability
> reporting, and not limit (and worry) those values in the command, so
> from command's perspective it can be only .
>
ok, will update the help string to
> > + "Set number of lanes for all ports or port_id for
> > a forced speed\n\n"
> > );
> > }
> >
> > @@ -1560,6 +1566,244 @@ static cmdline_parse_inst_t
> > cmd_config_speed_specific = {
> > },
> > };
> >
> > +static int
> > +parse_speed_lanes_cfg(portid_t pid, uint32_t lanes)
> > +{
> > + int ret;
> > +
> > + ret = rte_eth_speed_lanes_set(pid, lanes);
> >
>
> As a sample application usage, I think it is better if it gets the
> capability and verify that 'lanes' is withing the capability and later
> set it, what do you think?
>
> <...>
Makes sense, will try out and get back to you soon.
> > diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h> index
> 548fada1c7..9444e0a836 100644
> > --- a/lib/ethdev/rte_ethdev.h
> > +++ b/lib/ethdev/rte_ethdev.h
> > @@ -357,6 +357,27 @@ struct rte_eth_link {
> > #define RTE_ETH_LINK_MAX_STR_LEN 40 /**< Max length of default link
> > string. */
> > /**@}*/
> >
> > +/**
> > + * Constants used to indicate the possible link speed lanes of an ethdev
> > port.
> > + */
> > +#define RTE_ETH_SPEED_LANE_UNKNOWN 0 /**< speed lanes unsupported mode
> > or default */
> > +#define RTE_ETH_SPEED_LANE_1 1/**< Link speed lane 1 */
> > +#define RTE_ETH_SPEED_LANE_2 2/**< Link speed lanes 2 */
> > +#define RTE_ETH_SPEED_LANE_4 4/**< Link speed lanes 4 */
> > +#define RTE_ETH_SPEED_LANE_8 8/**< Link speed lanes 8 */
> > +
> > +/* Translate from link speed lanes to speed lanes capa */
> > +#define RTE_ETH_SPEED_LANES_TO_CAPA(x) RTE_BIT32(x)
> > +
> > +/* This macro indicates link speed lanes capa mask */
> > +#define RTE_ETH
Re: [PATCH] eal: fix device unregister for event registered with device_name NULL
On Thu, 18 Jul 2024 12:37:28 -0700
lon...@linuxonhyperv.com wrote:
> From: Malcolm Bumgardner
>
> In the device event unregister code, it unconditionally remove all
> callbacks which are registered with device_name set to NULL. This results
> in many callbacks uncorrectly removed.
>
> Fix this by only removing callbacks with matching cb_fn and cb_arg.
>
> Signed-off-by: Malcolm Bumgardner
>
> Fixes: a753e53d517b ("eal: add device event monitor framework")
> Cc: sta...@dpdk.org
> Signed-off-by: Long Li
> ---
> lib/eal/common/eal_common_dev.c | 13 +++--
> 1 file changed, 7 insertions(+), 6 deletions(-)
This patch solves the problem in the netvsc driver.
The netvsc driver registers a device event callback to discover when the VF
is added/removed. When the netvsc driver is stoppped, it unregisters this
callback.
The bug is that the rte_dev code would end up removing all callbacks, including
the callback for other netvsc devices.
Perhaps a bugzilla entry or more direct example would have gotten more attention
on this.
Reviewed-by: Stephen Hemminger
RE: [PATCH v4] eal: add build-time option to omit trace
> From: Jerin Jacob [mailto:jerinjac...@gmail.com] > Sent: Tuesday, 24 September 2024 11.42 > > On Tue, Sep 24, 2024 at 7:10 PM Morten Brørup > wrote: > > > > Some applications want to omit the trace feature. > > Either to reduce the memory footprint, to reduce the exposed attack > > surface, or for other reasons. > > > > This patch adds an option in rte_config.h to include or omit trace in > the > > build. Trace is included by default. > > > > Omitting trace works by omitting all trace points. > > For API and ABI compatibility, the trace feature itself remains. > > > > Signed-off-by: Morten Brørup > > --- > > v4: > > * Added check for generic trace enabled when registering trace points, > in > > RTE_INIT. (Jerin Jacob) > > * Test application uses function instead of macro to check if generic > > trace is enabled. (Jerin Jacob) > > * Performance test application uses function to check if generic trace > is > > enabled. > > v3: > > * Simpler version with much fewer ifdefs. (Jerin Jacob) > > v2: > > * Added/modified macros required for building applications with trace > > omitted. > > > > > +/** > > + * @internal > > Since it is used in app/test. Can we remove it as internal and make > symbol as rte_trace_point_is_enabled I don't think we should make functions public if only used for test purposes. Do you think it is useful for normal usage too? Does rte_trace_is_enabled() not suffice? > > > + * > > + * Test if the tracepoint compile-time option is enabled. > > + * > > + * @return > > + * true if tracepoint enabled, false otherwise. > > + */ > > +__rte_experimental > > +static __rte_always_inline bool > > +__rte_trace_point_generic_is_enabled(void) > > Do we need to keep _generic_ Other internal functions have _generic_ too, so I added it. If we decide to make it public, I agree _generic_ should be removed. > > Rest looks good to me.
Re: [PATCH v2] dts: add VLAN methods to testpmd shell
We should bring up the exact names of method used in this patch and talk
through them in the call.
On 18. 9. 2024 21:41, Dean Marx wrote:
added the following methods to testpmd shell class:
Capitalize please.
vlan set filter on/off, rx vlan add/rm,
vlan set strip on/off, tx vlan set/reset,
set promisc/verbose
Fixes: 61d5bc9bf974 ("dts: add port info command to testpmd shell")
Is the fix related to the rest of the changes? It looks like it is. The
relation to the other changes should be explained in the commit message
as well as what the fix fixes (or in which cases the original
implementation didn't work properly).
Signed-off-by: Dean Marx
---
dts/framework/remote_session/testpmd_shell.py | 175 +-
1 file changed, 174 insertions(+), 1 deletion(-)
diff --git a/dts/framework/remote_session/testpmd_shell.py
b/dts/framework/remote_session/testpmd_shell.py
index 8c228af39f..5c5e681841 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -102,7 +102,7 @@ def make_parser(cls) -> ParserFn:
r"strip (?Pon|off), "
r"filter (?Pon|off), "
r"extend (?Pon|off), "
-r"qinq strip (?Pon|off)$",
+r"qinq strip (?Pon|off)",
re.MULTILINE,
named=True,
),
@@ -982,6 +982,179 @@ def set_port_mtu_all(self, mtu: int, verify: bool = True)
-> None:
for port in self.ports:
self.set_port_mtu(port.id, mtu, verify)
+def vlan_filter_set(self, port: int, on: bool, verify: bool = True) -> None:
The names should follow the same naming scheme. I think the scheme says
it doesn't necessarily have to follow testpmd commands, so I'd say let's
name it the same way as the mtu methods - set_vlan_filter.
+"""Set vlan filter on.
Out of curiosity, what is vlan filter?
+
+Args:
+port: The port number to enable VLAN filter on, should be within
0-32.
Where is this 0-32 coming from? And why 32 and not 31?
+on: Sets filter on if :data:`True`, otherwise turns off.
I'd use third person and add the port:
Enable the filter on `port` if :data:`True`, otherwise disable it.
+verify: If :data:`True`, the output of the command and show port
info
+is scanned to verify that vlan filtering was enabled
successfully.
+If not, it is considered an error.
This is not 100% clear whether this means if False or if not enabled
successfully. Looks like this should be updated everywhere.
+
+Raises:
+InteractiveCommandExecutionError: If `verify` is :data:`True` and
the filter
+fails to update.
+"""
+filter_cmd_output = self.send_command(f"vlan set filter {'on' if on else
'off'} {port}")
+if verify:
+vlan_settings = self.show_port_info(port_id=port).vlan_offload
+if on ^ (vlan_settings is not None and VLANOffloadFlag.FILTER in
vlan_settings):
This is an awesome condition.
+self._logger.debug(f"Failed to set filter on port {port}:
\n{filter_cmd_output}")
+raise InteractiveCommandExecutionError(
+f"Testpmd failed to set VLAN filter on port {port}."
+)
We should say whether we're enabling or disabling the filter in both the
log and error. Does filter_cmd_output contain this? What about the other
commands (every one of them except verbose_output)?
+
+def rx_vlan(self, vlan: int, port: int, add: bool, verify: bool = True) ->
None:
We have set as the prefix for boolean configs and configs that only set
the value, here we probabaly want add_del_rx_vlan. We really should talk
about these names in the call.
+"""Add specified vlan tag to the filter list on a port.
The command doesn't mention the filter. Does this mean the filter needs
to be enabled before we can config vlans on the port?
+
+Args:
+vlan: The vlan tag to add, should be within 1-1005, 1-4094
extended.
The method mentions extended vlans only in this place. It's not clear
when can we use extended vlans (where is it configured or enforced).
+port: The port number to add the tag on, should be within 0-32.
+add: Adds the tag if :data:`True`, otherwise removes tag.
removes the tag
+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 `verify` is :data:`True` and
the tag
+is not added.
I guess this would also be raised if removal is unsuccessful.
+"""
+rx_output = self.send_command(f"rx_vlan {'add' if add else 'rm'} {vlan}
{port}")
As Patrick
[Azure] [ConnectX-3] net_mlx4: cannot access device, is mlx4_ib loaded?
Hi Team, Unable to attach ConnectX-3 mlx4 interfaces(uio_hv_generic) in DPDK. testpmd application reports below errors. mlx4_ib is loaded. Not sure if I am missing anything here. Appreciate any help to resolve this. *EAL: Detected CPU lcores: 8EAL: Detected NUMA nodes: 1EAL: Detected static linkage of DPDKEAL: Multi-process socket /var/run/dpdk/rte/mp_socketEAL: Selected IOVA mode 'PA'EAL: Probe PCI driver: net_mlx4 (15b3:1004) device: 44be:00:02.0 (socket 0)net_mlx4: cannot access device, is mlx4_ib loaded?EAL: Requested device 44be:00:02.0 cannot be usedEAL: Probe PCI driver: net_mlx4 (15b3:1004) device: 9d69:00:02.0 (socket 0)net_mlx4: cannot access device, is mlx4_ib loaded?EAL: Requested device 9d69:00:02.0 cannot be usedEAL: Probe PCI driver: net_mlx4 (15b3:1004) device: a5e7:00:02.0 (socket 0)net_mlx4: cannot access device, is mlx4_ib loaded?EAL: Requested device a5e7:00:02.0 cannot be usedEAL: Probe PCI driver: net_mlx4 (15b3:1004) device: f25c:00:02.0 (socket 0)net_mlx4: cannot access device, is mlx4_ib loaded?EAL: Requested device f25c:00:02.0 cannot be usedhn_vf_attach(): Couldn't find port for VFhn_vf_add(): RNDIS reports VF but device not found, retryinghn_vf_attach(): Couldn't find port for VFhn_vf_add(): RNDIS reports VF but device not found, retryinghn_vf_attach(): Couldn't find port for VFhn_vf_add(): RNDIS reports VF but device not found, retryingTELEMETRY: No legacy callbacks, legacy socket not createdtestpmd: create a new mbuf pool : n=171456, size=2176, socket=0testpmd: preferred mempool ops selected: ring_mp_mchn_vf_attach(): Couldn't find port for VFhn_vf_add(): RNDIS reports VF but device not found, retryinghn_vf_attach(): Couldn't find port for VFhn_vf_add(): RNDIS reports VF but device not found, retryingEAL: Error - exiting with code: 1 Cause: Creation of mbuf pool for socket 0 failed: Cannot allocate memory* Thanks & Regards, Rajasekhar
[PATCH v3 2/2] dts: add binding to different drivers to TG node
From: Jeremy Spewock
The DTS framework in its current state supports binding ports to
different drivers on the SUT node but not the TG node. The TG node
already has the information that it needs about the different drivers
that it has available in the configuration file, but it did not
previously have access to the devbind script, so it did not use that
information for anything.
This patch moves the location of the tmp directory as well as the method
for binding ports into the node class rather than the SUT node class and
adds an abstract method for getting the path to the devbind script into
the node class. Then, binding ports to the correct drivers is moved into
the build target setup and run on both nodes.
Bugzilla ID: 1420
Signed-off-by: Jeremy Spewock
---
dts/framework/runner.py | 2 +
dts/framework/testbed_model/node.py | 49 -
dts/framework/testbed_model/sut_node.py | 49 -
dts/framework/testbed_model/tg_node.py | 21 +++
dts/framework/utils.py | 2 +
5 files changed, 88 insertions(+), 35 deletions(-)
diff --git a/dts/framework/runner.py b/dts/framework/runner.py
index ab98de8353..4c884fbcd4 100644
--- a/dts/framework/runner.py
+++ b/dts/framework/runner.py
@@ -486,6 +486,7 @@ def _run_build_target(
try:
sut_node.set_up_build_target(build_target_config)
+tg_node.set_up_build_target(build_target_config)
self._result.dpdk_version = sut_node.dpdk_version
build_target_result.add_build_target_info(sut_node.get_build_target_info())
build_target_result.update_setup(Result.PASS)
@@ -500,6 +501,7 @@ def _run_build_target(
try:
self._logger.set_stage(DtsStage.build_target_teardown)
sut_node.tear_down_build_target()
+tg_node.tear_down_build_target()
build_target_result.update_teardown(Result.PASS)
except Exception as e:
self._logger.exception("Build target teardown failed.")
diff --git a/dts/framework/testbed_model/node.py
b/dts/framework/testbed_model/node.py
index 12a40170ac..aa26f2a2a7 100644
--- a/dts/framework/testbed_model/node.py
+++ b/dts/framework/testbed_model/node.py
@@ -13,11 +13,18 @@
The :func:`~Node.skip_setup` decorator can be used without subclassing.
"""
-from abc import ABC
+
+from abc import ABC, abstractmethod
from ipaddress import IPv4Interface, IPv6Interface
+from pathlib import PurePath
from typing import Any, Callable, Union
-from framework.config import OS, NodeConfiguration, TestRunConfiguration
+from framework.config import (
+OS,
+BuildTargetConfiguration,
+NodeConfiguration,
+TestRunConfiguration,
+)
from framework.exception import ConfigurationError
from framework.logger import DTSLogger, get_dts_logger
from framework.settings import SETTINGS
@@ -58,8 +65,10 @@ class Node(ABC):
lcores: list[LogicalCore]
ports: list[Port]
_logger: DTSLogger
+_tmp_dir: PurePath
_other_sessions: list[OSSession]
_test_run_config: TestRunConfiguration
+_path_to_devbind_script: PurePath | None
def __init__(self, node_config: NodeConfiguration):
"""Connect to the node and gather info during initialization.
@@ -88,6 +97,8 @@ def __init__(self, node_config: NodeConfiguration):
self._other_sessions = []
self._init_ports()
+self._tmp_dir = self.main_session.get_remote_tmp_dir()
+self._path_to_devbind_script = None
def _init_ports(self) -> None:
self.ports = [Port(self.name, port_config) for port_config in
self.config.ports]
@@ -95,6 +106,11 @@ def _init_ports(self) -> None:
for port in self.ports:
self.configure_port_state(port)
+@property
+@abstractmethod
+def path_to_devbind_script(self) -> PurePath:
+"""The path to the dpdk-devbind.py script on the node."""
+
def set_up_test_run(self, test_run_config: TestRunConfiguration) -> None:
"""Test run setup steps.
@@ -114,6 +130,20 @@ def tear_down_test_run(self) -> None:
Additional steps can be added by extending the method in subclasses
with the use of super().
"""
+def set_up_build_target(self, build_target_config:
BuildTargetConfiguration) -> None:
+"""Bind ports to their DPDK drivers.
+
+Args:
+build_target_config: The build target test run configuration
according to which
+the setup steps will be taken. This is unused in this method,
but subclasses that
+extend this method may need it.
+"""
+self.bind_ports_to_driver()
+
+def tear_down_build_target(self) -> None:
+"""Bind ports to their OS drivers."""
+self.bind_ports_to_driver(for_dpdk=False)
+
def create_session(self, name: str) -> OSSession:
"""Create and return a new OS-aware remote session.
@@
[PATCH v3 0/2] dts: add driver binding on TG
From: Jeremy Spewock v3: * removed _remote from _remote_tmp_dir in node * switched to using Path where appropriate * added symlink to dpdk-devbind to dts/ and excluded from the the formatting script Jeremy Spewock (2): dts: add synbolic link to dpdk-devbind script dts: add binding to different drivers to TG node devtools/dts-check-format.sh| 9 +++-- dts/dpdk-devbind.py | 1 + dts/framework/runner.py | 2 + dts/framework/testbed_model/node.py | 49 - dts/framework/testbed_model/sut_node.py | 49 - dts/framework/testbed_model/tg_node.py | 21 +++ dts/framework/utils.py | 2 + 7 files changed, 94 insertions(+), 39 deletions(-) create mode 12 dts/dpdk-devbind.py -- 2.46.0
[PATCH v3 1/2] dts: add symbolic link to dpdk-devbind script
From: Jeremy Spewock
The devbind script is used throughout DTS to manage drivers on the
remote hosts. Currently, the only way to copy this script onto a host
is to either copy the entire DPDK directory onto a host, or reach out
of the dts directory into its parent DPDK directory to access the
script. The first is undesirable if the host doesn't require any other
DPDK tools since you would be copying extra unneeded information and
the second is undesirable since it enforces the assumption that DTS is
being run from within the DPDK directory. To solve this issue a symbolic
link is added which links the devbind script from the parent into the
DTS directory.
Since this file is not part of DTS and therefore is not expected to
follow DTS formatting rules, it is excluded from the DTS formatting
script.
Signed-off-by: Jeremy Spewock
---
devtools/dts-check-format.sh | 9 +
dts/dpdk-devbind.py | 1 +
2 files changed, 6 insertions(+), 4 deletions(-)
create mode 12 dts/dpdk-devbind.py
diff --git a/devtools/dts-check-format.sh b/devtools/dts-check-format.sh
index 3f43e17e88..adc199d34e 100755
--- a/devtools/dts-check-format.sh
+++ b/devtools/dts-check-format.sh
@@ -13,6 +13,7 @@ usage() {
format=true
lint=true
typecheck=true
+ignore="dpdk-devbind.py"
# Comments after args serve as documentation; must be present
while getopts "hflt" arg; do
@@ -54,14 +55,14 @@ if $format; then
heading "Formatting in $directory/"
if command -v black > /dev/null; then
echo "Formatting code with black:"
- black .
+ black --exclude "$ignore" .
else
echo "black is not installed, not formatting"
errors=$((errors + 1))
fi
if command -v isort > /dev/null; then
echo "Sorting imports with isort:"
- isort .
+ isort --skip "$ignore" .
else
echo "isort is not installed, not sorting
imports"
errors=$((errors + 1))
@@ -90,7 +91,7 @@ if $lint; then
fi
heading "Linting in $directory/"
if command -v pylama > /dev/null; then
- pylama .
+ pylama --skip "$ignore" .
errors=$((errors + $?))
else
echo "pylama not found, unable to run linter"
@@ -104,7 +105,7 @@ if $typecheck; then
fi
heading "Checking types in $directory/"
if command -v mypy > /dev/null; then
- mypy .
+ mypy --exclude "$ignore" .
errors=$((errors + $?))
else
echo "mypy not found, unable to check types"
diff --git a/dts/dpdk-devbind.py b/dts/dpdk-devbind.py
new file mode 12
index 00..9d042fad14
--- /dev/null
+++ b/dts/dpdk-devbind.py
@@ -0,0 +1 @@
+../usertools/dpdk-devbind.py
\ No newline at end of file
--
2.46.0
Re: [PATCH v4] eal: add build-time option to omit trace
On Tue, 24 Sep 2024 13:39:57 + Morten Brørup wrote: > Some applications want to omit the trace feature. > Either to reduce the memory footprint, to reduce the exposed attack > surface, or for other reasons. > > This patch adds an option in rte_config.h to include or omit trace in the > build. Trace is included by default. > > Omitting trace works by omitting all trace points. > For API and ABI compatibility, the trace feature itself remains. > > Signed-off-by: Morten Brørup This is good compact solution. In future, it would be nice if DPDK could use static keys to isolate rarely used features. But static keys require runtime modification of instructions. https://docs.kernel.org/6.7/staging/static-keys.html Acked-by: Stephen Hemminger
Re: [PATCH v2 1/1] dts: add binding to different drivers to TG node
On 24. 9. 2024 15:57, Jeremy Spewock wrote: On Tue, Sep 24, 2024 at 5:12 AM Juraj Linkeš wrote: I have some thoughts for the future: 1a. The traffic generator is specified per-node, so maybe we could also change the binding to be for the whole lifetime of the TG node, 1b. But the same is true for the SUT node as well, right? After we do the port update (with kernel driver), we can just bind to DPDK driver. With SUT in the mix, this looks like a change for a different patch, Right, these are good points. A good observation too that we only really need the kernel driver at the start in both cases. You had mentioned in your previous comments as well that we should only be binding on the TG once per lifetime, but I ended up not adding it for that very reason of I still wanted the binding to be in Node, but I didn't want to change the process for the SUT. 2. We could add a symlink to the devbind script with the target being in the dts directory. This way we don't have to go outside the dts directory and if DTS ever become a python package, we could just copy the script to the appropriate place. This is also something we don't really need to do. I like this idea a lot actually. It feels very weird to me having to step out of the DTS directory and I like the idea of keeping it together like it were a package (even if it isn't yet). Ok, you can add that to the next version. diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py @@ -58,8 +65,10 @@ class Node(ABC): lcores: list[LogicalCore] ports: list[Port] _logger: DTSLogger +_remote_tmp_dir: PurePath _other_sessions: list[OSSession] _test_run_config: TestRunConfiguration +_path_to_devbind_script: PurePath | None A note on the naming. We have _remote_tmp_dir and _path_to_devbind_script. Both are pointing to a remote file/dir, but only one has the _remote prefix. They should probably be unified. I didn't think of this but you're right, the two are very similar but named differently. I've thought a bit about what the right name is. Dropping the prefix makes sense; sut_node.tmp_dir should mean the tmp dir on the SUT node (which would make it remote from the execution host's point of view, but not the node's view; the file is local to SUT node). This could be a good separate patch (improving the remote/local naming scheme to make it consistent and as sensible as possible). I also like the sound of it without the prefix and how it actually has a more fitting meaning from the two perspectives. I agree that there is probably some other work to be done on this in another patch, but since I am moving the _remote_tmp_dir variable around anyway I think it wouldn't hurt for me to rename it. Yes, we should pick one naming convention to be consistent in this patch and we can have a broader (framework-wide) look at this in a separate patch.
Re: [PATCH v3 0/2] vhost: add VDUSE reconnection support
On 9/23/24 21:51, Maxime Coquelin wrote: This series adds support for VDUSE reconnection. First patch introduces the reconnection file layout and track the virtqueues available index updates in the datapath and control queue. Second patch adds VDUSE reconnect intialization and some sanity checks to prevent incompatible reconnections. Changes in v3: == - Fixed missing avail index updates (David) - Fixed typos in commit message (David) - Applied R-by's Changes in v2: == - Added more sanity checks at reconnection - Improve versionning - Fix error loggin (Chenbo) - Clarify why offloading device start is required (Chenbo) - Change runtime path to /vduse instead of /dpdk/vduse Maxime Coquelin (2): vhost: add logging mechanism for reconnection vhost: add reconnection support to VDUSE lib/vhost/vduse.c | 308 +++- lib/vhost/vhost.c | 2 + lib/vhost/vhost.h | 41 - lib/vhost/vhost_user.c | 4 + lib/vhost/virtio_net.c | 8 + lib/vhost/virtio_net_ctrl.c | 2 + 6 files changed, 322 insertions(+), 43 deletions(-) Applied to next-virtio/for-next-net
[PATCH] net/r8169: add phy registers access routines
Signed-off-by: Howard Wang
---
drivers/net/r8169/r8169_ethdev.h | 1 +
drivers/net/r8169/r8169_phy.c| 219 +++
drivers/net/r8169/r8169_phy.h| 18 +++
3 files changed, 238 insertions(+)
diff --git a/drivers/net/r8169/r8169_ethdev.h b/drivers/net/r8169/r8169_ethdev.h
index 20dbf06c9b..9656a26eb0 100644
--- a/drivers/net/r8169/r8169_ethdev.h
+++ b/drivers/net/r8169/r8169_ethdev.h
@@ -18,6 +18,7 @@ struct rtl_hw {
u8 *mmio_addr;
u32 mcfg;
u8 HwSuppIntMitiVer;
+ u16 cur_page;
/* Enable Tx No Close */
u8 EnableTxNoClose;
diff --git a/drivers/net/r8169/r8169_phy.c b/drivers/net/r8169/r8169_phy.c
index f0a880eeca..cfec426ee1 100644
--- a/drivers/net/r8169/r8169_phy.c
+++ b/drivers/net/r8169/r8169_phy.c
@@ -39,3 +39,222 @@ rtl_set_mac_ocp_bit(struct rtl_hw *hw, u16 addr, u16 mask)
rtl_clear_set_mac_ocp_bit(hw, addr, 0, mask);
}
+static u16
+rtl_map_phy_ocp_addr(u16 PageNum, u8 RegNum)
+{
+ u8 ocp_reg_num = 0;
+ u16 ocp_page_num = 0;
+ u16 ocp_phy_address = 0;
+
+ if (PageNum == 0) {
+ ocp_page_num = OCP_STD_PHY_BASE_PAGE + (RegNum / 8);
+ ocp_reg_num = 0x10 + (RegNum % 8);
+ } else {
+ ocp_page_num = PageNum;
+ ocp_reg_num = RegNum;
+ }
+
+ ocp_page_num <<= 4;
+
+ if (ocp_reg_num < 16)
+ ocp_phy_address = 0;
+ else {
+ ocp_reg_num -= 16;
+ ocp_reg_num <<= 1;
+
+ ocp_phy_address = ocp_page_num + ocp_reg_num;
+ }
+
+ return ocp_phy_address;
+}
+
+static u32
+rtl_mdio_real_read_phy_ocp(struct rtl_hw *hw, u32 RegAddr)
+{
+ u32 data32;
+ int i, value = 0;
+
+ data32 = RegAddr / 2;
+ data32 <<= OCPR_Addr_Reg_shift;
+
+ RTL_W32(hw, PHYOCP, data32);
+ for (i = 0; i < 100; i++) {
+ udelay(1);
+
+ if (RTL_R32(hw, PHYOCP) & OCPR_Flag)
+ break;
+ }
+ value = RTL_R32(hw, PHYOCP) & OCPDR_Data_Mask;
+
+ return value;
+}
+
+u32
+rtl_mdio_direct_read_phy_ocp(struct rtl_hw *hw, u32 RegAddr)
+{
+ return rtl_mdio_real_read_phy_ocp(hw, RegAddr);
+}
+
+static u32
+rtl_mdio_read_phy_ocp(struct rtl_hw *hw, u16 PageNum, u32 RegAddr)
+{
+ u16 ocp_addr;
+
+ ocp_addr = rtl_map_phy_ocp_addr(PageNum, RegAddr);
+
+ return rtl_mdio_direct_read_phy_ocp(hw, ocp_addr);
+}
+
+static u32
+rtl_mdio_real_read(struct rtl_hw *hw, u32 RegAddr)
+{
+ return rtl_mdio_read_phy_ocp(hw, hw->cur_page, RegAddr);
+}
+
+static void
+rtl_mdio_real_write_phy_ocp(struct rtl_hw *hw, u32 RegAddr, u32 value)
+{
+ u32 data32;
+ int i;
+
+ data32 = RegAddr / 2;
+ data32 <<= OCPR_Addr_Reg_shift;
+ data32 |= OCPR_Write | value;
+
+ RTL_W32(hw, PHYOCP, data32);
+ for (i = 0; i < 100; i++) {
+ udelay(1);
+
+ if (!(RTL_R32(hw, PHYOCP) & OCPR_Flag))
+ break;
+ }
+}
+
+void
+rtl_mdio_direct_write_phy_ocp(struct rtl_hw *hw, u32 RegAddr, u32 value)
+{
+ rtl_mdio_real_write_phy_ocp(hw, RegAddr, value);
+}
+
+static void
+rtl_mdio_write_phy_ocp(struct rtl_hw *hw, u16 PageNum, u32 RegAddr, u32 value)
+{
+ u16 ocp_addr;
+
+ ocp_addr = rtl_map_phy_ocp_addr(PageNum, RegAddr);
+
+ rtl_mdio_direct_write_phy_ocp(hw, ocp_addr, value);
+}
+
+static void
+rtl_mdio_real_write(struct rtl_hw *hw, u32 RegAddr, u32 value)
+{
+ if (RegAddr == 0x1F)
+ hw->cur_page = value;
+ rtl_mdio_write_phy_ocp(hw, hw->cur_page, RegAddr, value);
+}
+
+u32
+rtl_mdio_read(struct rtl_hw *hw, u32 RegAddr)
+{
+ return rtl_mdio_real_read(hw, RegAddr);
+}
+
+void
+rtl_mdio_write(struct rtl_hw *hw, u32 RegAddr, u32 value)
+{
+ rtl_mdio_real_write(hw, RegAddr, value);
+}
+
+void
+rtl_clear_and_set_eth_phy_ocp_bit(struct rtl_hw *hw, u16 addr, u16 clearmask,
+ u16 setmask)
+{
+ u16 phy_reg_value;
+
+ phy_reg_value = rtl_mdio_direct_read_phy_ocp(hw, addr);
+ phy_reg_value &= ~clearmask;
+ phy_reg_value |= setmask;
+ rtl_mdio_direct_write_phy_ocp(hw, addr, phy_reg_value);
+}
+
+void
+rtl_clear_eth_phy_ocp_bit(struct rtl_hw *hw, u16 addr, u16 mask)
+{
+ rtl_clear_and_set_eth_phy_ocp_bit(hw, addr, mask, 0);
+}
+
+void
+rtl_set_eth_phy_ocp_bit(struct rtl_hw *hw, u16 addr, u16 mask)
+{
+ rtl_clear_and_set_eth_phy_ocp_bit(hw, addr, 0, mask);
+}
+
+void
+rtl_ephy_write(struct rtl_hw *hw, int addr, int value)
+{
+ int i;
+
+ RTL_W32(hw, EPHYAR, EPHYAR_Write |
+ (addr & EPHYAR_Reg_Mask_v2) << EPHYAR_Reg_shift |
+ (value & EPHYAR_Data_Mask));
+
+ for (i = 0; i < 10; i++) {
+ udelay(100);
+
+ /* Check if the NIC has completed EPHY write */
+ if (!(RTL_R32(hw, EPHYAR) & EPHYAR_Flag))
+ break;
Re: [PATCH 2/2] timer/linux: override TSC freq if no tsc_known_freq
Perhaps this was a bit hastily, I got an automated failure report on "arm64-native-linuxapp-gcc": https://lab.dpdk.org/results/dashboard/patchsets/31106/ Not sure which patch caused the problem, looking at the kernel code it looks like 'tsc_known_freq' is only set for x86 arch, but it could be the other patch and maybe lowering the rounding to 10KHz is too aggressive, maybe better 100KHz or 1MHz. As an alternative to 'tsc_known_freq' detection, maybe just provide an init parameter to set the frequency manually, along with some known-issue documentation. Maybe we should just allow to specify the frequency as a parameter at init
Re: [RFC PATCH v7] mempool: fix mempool cache size
Recheck-request: iol-intel-Performance
On Tue, Sep 24, 2024 at 2:12 PM Morten Brørup
wrote:
> This patch refactors the mempool cache to fix two bugs:
> 1. When a mempool is created with a cache size of N objects, the cache was
> actually created with a size of 1.5 * N objects.
> 2. The mempool cache field names did not reflect their purpose;
> the "flushthresh" field held the size, and the "size" field held the
> number of objects remaining in the cache when returning from a get
> operation refilling it from the backend.
>
> Especially the first item could be fatal:
> When more objects than a mempool's configured cache size is held in the
> mempool's caches associated with other lcores, a rightsized mempool may
> unexpectedly run out of objects, causing the application to fail.
>
> Furthermore, this patch introduces two optimizations:
> 1. The mempool caches are flushed to/filled from the backend in their
> entirety, so backend accesses are CPU cache line aligned. (Assuming the
> mempool cache size is a multiplum of a CPU cache line size divided by the
> size of a pointer.)
> 2. The unlikely paths in the get and put functions, where the cache is
> flushed to/filled from the backend, are moved from the inline functions to
> separate helper functions, thereby reducing the code size of the inline
> functions.
> Note: Accessing the backend for cacheless mempools remains inline.
>
> Various drivers accessing the mempool directly have been updated
> accordingly.
> These drivers did not update mempool statistics when accessing the mempool
> directly, so that is fixed too.
>
> Note: Performance not yet benchmarked.
>
> Signed-off-by: Morten Brørup
> ---
> v7:
> * Increased max mempool cache size from 512 to 1024 objects.
> Mainly for CI performance test purposes.
> Originally, the max mempool cache size was 768 objects, and used a fixed
> size array of 1024 objects in the mempool cache structure.
> v6:
> * Fix v5 incomplete implementation of passing large requests directly to
> the backend.
> * Use memcpy instead of rte_memcpy where compiler complains about it.
> * Added const to some function parameters.
> v5:
> * Moved helper functions back into the header file, for improved
> performance.
> * Pass large requests directly to the backend. This also simplifies the
> code.
> v4:
> * Updated subject to reflect that misleading names are considered bugs.
> * Rewrote patch description to provide more details about the bugs fixed.
> (Mattias Rönnblom)
> * Moved helper functions, not to be inlined, to mempool C file.
> (Mattias Rönnblom)
> * Pass requests for n >= RTE_MEMPOOL_CACHE_MAX_SIZE objects known at build
> time directly to backend driver, to avoid calling the helper functions.
> This also fixes the compiler warnings about out of bounds array access.
> v3:
> * Removed __attribute__(assume).
> v2:
> * Removed mempool perf test; not part of patch set.
> ---
> config/rte_config.h | 2 +-
> drivers/common/idpf/idpf_common_rxtx_avx512.c | 54 +---
> drivers/mempool/dpaa/dpaa_mempool.c | 16 +-
> drivers/mempool/dpaa2/dpaa2_hw_mempool.c | 14 -
> drivers/net/i40e/i40e_rxtx_vec_avx512.c | 17 +-
> drivers/net/iavf/iavf_rxtx_vec_avx512.c | 27 +-
> drivers/net/ice/ice_rxtx_vec_avx512.c | 27 +-
> lib/mempool/mempool_trace.h | 1 -
> lib/mempool/rte_mempool.c | 12 +-
> lib/mempool/rte_mempool.h | 287 --
> 10 files changed, 232 insertions(+), 225 deletions(-)
>
> diff --git a/config/rte_config.h b/config/rte_config.h
> index dd7bb0d35b..2488ff167d 100644
> --- a/config/rte_config.h
> +++ b/config/rte_config.h
> @@ -56,7 +56,7 @@
> #define RTE_CONTIGMEM_DEFAULT_BUF_SIZE (512*1024*1024)
>
> /* mempool defines */
> -#define RTE_MEMPOOL_CACHE_MAX_SIZE 512
> +#define RTE_MEMPOOL_CACHE_MAX_SIZE 1024
> /* RTE_LIBRTE_MEMPOOL_STATS is not set */
> /* RTE_LIBRTE_MEMPOOL_DEBUG is not set */
>
> diff --git a/drivers/common/idpf/idpf_common_rxtx_avx512.c
> b/drivers/common/idpf/idpf_common_rxtx_avx512.c
> index 3b5e124ec8..98535a48f3 100644
> --- a/drivers/common/idpf/idpf_common_rxtx_avx512.c
> +++ b/drivers/common/idpf/idpf_common_rxtx_avx512.c
> @@ -1024,21 +1024,13 @@ idpf_tx_singleq_free_bufs_avx512(struct
> idpf_tx_queue *txq)
>
> rte_lcore_id());
> void **cache_objs;
>
> - if (cache == NULL || cache->len == 0)
> - goto normal;
> -
> - cache_objs = &cache->objs[cache->len];
> -
> - if (n > RTE_MEMPOOL_CACHE_MAX_SIZE) {
> - rte_mempool_ops_enqueue_bulk(mp, (void *)txep, n);
> + if (!cache || unlikely(n + cache->len > cache->size)) {
> + rte_mempool_generic_put(mp, (void *)txep, n,
> cache);
> goto done;
> }
>
> - /* The cache follows the following algorithm
> -
Re: [RFC] ethdev: introduce PTP device capability
Hi Ferruh,
在 2024/9/23 6:23, Ferruh Yigit 写道:
On 1/30/2024 3:58 AM, Huisong Li wrote:
Currently, the PTP feature is a little messy and has some issue.
1> There is different implementation for PTP. This feature of some
hardware depends on the Rx HW timestamp offload, and use this
offload to detect if enable PTP feature. Others can enable PTP
feature with only ethdev ops.
2> For some drivers, enabling PTP feature also depends on the macro
RTE_LIBRTE_IEEE1588. Actually, whether device support, enable
or disable this feature should not be determined at compilation
time. This has been discussed in thread [1].
3> The user haven't a good way to distinguish which port supports
the PTP feature in the case that a couple of device belong to the
same driver. And PTP related API in ethdev layer doesn't do check
for PTP capability. This has been mentioned and discussed in
thread [2].
In the thread [1], there is a conclusion that remove the compiling
macro RTE_LIBRTE_IEEE1588. And in the thread [2], there is a common
opinion that the RTE_ETH_RX_OFFLOAD_TIMESTAMP cannot be used as the
PTP capability.
For the above issuse, this patch introduces a PTP capability in
rte_eth_dev_info.dev_capa to report PTP capability.
Welcome to jumping into discussion.
[1]
https://patchwork.dpdk.org/project/dpdk/patch/20230203132810.14187-1-tho...@monjalon.net/
[2] https://inbox.dpdk.org/dev/20230817084226.55327-1-lihuis...@huawei.com/
Signed-off-by: Huisong Li
---
lib/ethdev/rte_ethdev.h | 6 ++
1 file changed, 6 insertions(+)
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index efa4f12b2a..4c8bd691bd 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -1613,6 +1613,12 @@ struct rte_eth_conf {
#define RTE_ETH_DEV_CAPA_FLOW_RULE_KEEP RTE_BIT64(3)
/** Device supports keeping shared flow objects across restart. */
#define RTE_ETH_DEV_CAPA_FLOW_SHARED_OBJECT_KEEP RTE_BIT64(4)
+/**
+ * Device supports PTP feature.
+ * For some hardware, this feature also need to set the offload
+ * RTE_ETH_RX_OFFLOAD_TIMESTAMP, please check rte_eth_dev_info.rx_offload_capa.
+ */
+#define RTE_ETH_DEV_CAPA_PTP RTE_BIT64(5)
/**@}*/
/*
Hi Huisong,
Thanks for the effort, as you mentioned PTP feature can be improved and
there is a target to remove RTE_LIBRTE_IEEE1588 build time macro.
As far as I remember, one of the main reasons of the RTE_LIBRTE_IEEE1588
macro is some HW has resource restrictions, when RTE_LIBRTE_IEEE1588 is
enabled some other features, like vector datapath, are not usable, that
is why this is a build time selection.
I think the main reason that driver don't support PTP feature in vector
datapath is for performance.
This can be controled by releated dev_ops API or TIMESTAMP offload and
no need to use RTE_LIBRTE_IEEE1588 macro, like hns3.
And related to the PTP capability, can you please give some more
information, what does device having PTP capability exactly means.
Just the device having PTP capability can be used to enable PTP feature.
PTP is protocol and it is implemented in userspace daemon. I guess even
NIC does not support timestamp offloading, by using time information
from SW it can still implement PTP, right?
AFAIS, PTP feature implement requires the collaboration of HW and SW, as
describted by the releated dev_ops API.
Is PTP offload means, offloading some part of the protocol communication
withing the device?
Normally, a feature offload means this feature is done in hardware and
the software doesn't need to something for this.
I reviewed our discussion in [1], we all think it's unreasonable to name
it "PTP offload".
Btw, the relevant RTE_ETH_RX_OFFLOAD_TIMESTAMP offload is, a little more
generic HW capability that HW can add timestamp to Rx/Tx packets, which
can be used for custom diagnostics. HW supporting this offload means
that HW has some specific clock HW in it.
Yes.
Both having RTE_ETH_RX_OFFLOAD_TIMESTAMP and RTE_ETH_DEV_CAPA_PTP
capability can be confusing, lets clarify it more.
Let me try to clearify them:
1) RTE_ETH_DEV_CAPA_PTP just means the ethdev support PTP capability because of
application no way to know if the device support PTP feature.
2) As you said above, setting RTE_ETH_RX_OFFLOAD_TIMESTAMP offload is not
necessary for PTP feature, but also may be for custom diagnostics.
Some NICs enable PTP feature using only the rte_eth_timesync_xxx API,
and many NICs also need this offload to fill timestamp in mbuf to cooperate
with the implementation of PTP feature.
[1] https://inbox.dpdk.org/dev/20230817084226.55327-1-lihuis...@huawei.com/
.
[PATCH] net/ice/base: update README
Update the README in base folder of ice driver to reflect base code update release date and supported devices. Signed-off-by: Ian Stokes --- drivers/net/ice/base/README | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/net/ice/base/README b/drivers/net/ice/base/README index da065f9be2..7d5a01d223 100644 --- a/drivers/net/ice/base/README +++ b/drivers/net/ice/base/README @@ -1,17 +1,18 @@ /* SPDX-License-Identifier: BSD-3-Clause - * Copyright(c) 2020-2023 Intel Corporation + * Copyright(c) 2020-2024 Intel Corporation */ Intel® ICE driver == This directory contains source code of FreeBSD ice driver of version -2023.03.30 released by the team which develops +2024.08.19 released by the team which develops basic drivers for any ice NIC. The directory of base/ contains the original source package. This driver is valid for the product(s) listed below * Intel® Ethernet Network Adapters E810 +* Intel® Ethernet Network Adapters E830 Updating the driver === -- 2.34.1
Re: [PATCH v3 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell
I like how this looks. I have a number of minor comments (mainly wording
and naming), but overall it looks very good.
On 19. 9. 2024 21:02, jspew...@iol.unh.edu wrote:
From: Jeremy Spewock
Previously all scapy commands were handled using an XML-RPC server that
ran on the TGNode. This unnecessarily enforces a minimum Python version
of 3.10 on the server that is being used as a traffic generator and
complicates the implementation of scapy methods.
What is the TG's minimum Python version now?
https://bugs.dpdk.org/show_bug.cgi?id=1388 says this will become a moot
point, but we're still using Python on the remote node.
diff --git a/dts/framework/remote_session/single_active_interactive_shell.py
b/dts/framework/remote_session/single_active_interactive_shell.py
@@ -93,9 +94,13 @@ def __init__(
timeout: float = SETTINGS.timeout,
app_params: Params = Params(),
name: str | None = None,
+**kwargs,
) -> None:
"""Create an SSH channel during initialization.
+Additional key-word arguments can be passed through `kwargs` is needed to fulfill other
Typo: is -> if
And key-word should be just keyword.
Also, should we add super().__init__(), seeing as we also added it to
TrafficGenerator?
diff --git a/dts/framework/testbed_model/traffic_generator/scapy.py
b/dts/framework/testbed_model/traffic_generator/scapy.py
+class ScapyTrafficGenerator(PythonShell, CapturingTrafficGenerator):
+def __init__(self, tg_node: Node, config: ScapyTrafficGeneratorConfig,
**kwargs):
The usage of privileged=True when creating the instance confused me for
a bit, because it's not this class's argument, but rather
SingleActiveInteractiveShell's. I thing we should document the arguments
of SingleActiveInteractiveShell in the Keyword Args section. We probably
need just a link to SingleActiveInteractiveShell.
+def _create_packet_list(self, packets: list[Packet]) -> None:
Maybe we could apply some of the ideas from the local/remote naming
scheme I talked about in the tg devbind script patch here. Whatever
happens on the TG could be prefixed with remote and whatever is
happening locally would be without the prefix (or maybe whatever is
happnening on the TG shouldn't be prefixed (or a different prefix -
shell)? Makes sense, but then we'd need a good prefix for what's
happening on the execution environment. Maybe this also needs to be in a
different patch, after it's been though through with everything else in
mind.). That would make this _create_remote_packet_list, but we're just
setting a variable (the passed packets have already been built), so
maybe _set_remote_packet_list?
Or maybe all of the remote methods could start with remote, making it
_remote_set_packet_list (or _shell_set_packet_list? Doesn't sound bad.).
Not sure which is better, maybe after renaming more of these it's going
to be clearer.
Whatever we go with, the naming scheme should be explained in the
class's dosctring.
+def _create_sniffer(
+self, packets_to_send: list[Packet], send_port: Port, recv_port: Port,
filter_config: str
+) -> None:
+"""Create an asynchronous sniffer in the shell.
+
+A list of packets to send is added to the sniffer inside of a callback
function so that
+they are immediately sent at the time sniffing is started.
This is still a bit confusing (where are the packets added and what is
inside the function?); We may not need the Scapy implementation details.
We could just say the packets are sent immediately at the time sniffing
is started. Or maybe:
A list of packets is passed to the sniffer's callback function so that
they are immediately sent at the time sniffing is started.
It's a private method, so maybe the implementation detail could be
valuable, even though it's not fully clear why the implication is true -
you still need some knowledge of how the sniffer works to put everything
together.
+sniffer_commands = [
+f"{self._sniffer_name} = AsyncSniffer(",
+f"iface='{recv_port.logical_name}',",
+"store=True,",
+"started_callback=lambda *args: sendp(",
As far as I can tell, we're not using the args, so we can just use
"lambda: sendp()"
+(
+f"{self._python_indentation}{self._send_packet_list_name},"
Is the indentation needed here?
@@ -335,32 +204,19 @@ def _send_packets_and_capture(
I think we can improve the order of methods in the class. I'd put
_send_packets() and _send_packets_and_capture() after the public
methods. These two methods are the most important ones (implementing the
abstract methods). The other methods should come after that in the order
they're used in _send_packets() and _send_packets_and_capture().
self,
packets: list[Packet],
send_port: Port,
-receive_port: Port,
+recv_port: Port,
filter_config:
[PATCH V3 2/3] net/mlx5/hws: add log for failing to create rule in HWS
Add log messages about the reason why the flow was failed to be created.
Signed-off-by: Minggang Li(Gavin)
Acked-by: Alex Vesker
---
drivers/net/mlx5/hws/mlx5dr_rule.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/mlx5/hws/mlx5dr_rule.c
b/drivers/net/mlx5/hws/mlx5dr_rule.c
index 1edb7eac74..5d66d81ea5 100644
--- a/drivers/net/mlx5/hws/mlx5dr_rule.c
+++ b/drivers/net/mlx5/hws/mlx5dr_rule.c
@@ -638,6 +638,7 @@ static int mlx5dr_rule_destroy_hws(struct mlx5dr_rule *rule,
/* Rule is not completed yet */
if (rule->status == MLX5DR_RULE_STATUS_CREATING) {
+ DR_LOG(NOTICE, "Cannot destroy, rule creation still in
progress");
rte_errno = EBUSY;
return rte_errno;
}
@@ -806,12 +807,14 @@ static int mlx5dr_rule_enqueue_precheck(struct
mlx5dr_rule *rule,
struct mlx5dr_context *ctx = rule->matcher->tbl->ctx;
if (unlikely(!attr->user_data)) {
+ DR_LOG(DEBUG, "User data must be provided for rule operations");
rte_errno = EINVAL;
return rte_errno;
}
/* Check if there is room in queue */
if
(unlikely(mlx5dr_send_engine_full(&ctx->send_queue[attr->queue_id]))) {
+ DR_LOG(NOTICE, "No room in queue[%d]", attr->queue_id);
rte_errno = EBUSY;
return rte_errno;
}
@@ -823,6 +826,7 @@ static int mlx5dr_rule_enqueue_precheck_move(struct
mlx5dr_rule *rule,
struct mlx5dr_rule_attr *attr)
{
if (unlikely(rule->status != MLX5DR_RULE_STATUS_CREATED)) {
+ DR_LOG(DEBUG, "Cannot move, rule status is invalid");
rte_errno = EINVAL;
return rte_errno;
}
@@ -835,6 +839,7 @@ static int mlx5dr_rule_enqueue_precheck_create(struct
mlx5dr_rule *rule,
{
if (unlikely(mlx5dr_matcher_is_in_resize(rule->matcher))) {
/* Matcher in resize - new rules are not allowed */
+ DR_LOG(NOTICE, "Resizing in progress, cannot create rule");
rte_errno = EAGAIN;
return rte_errno;
}
@@ -1068,6 +1073,7 @@ int mlx5dr_rule_hash_calculate(struct mlx5dr_matcher
*matcher,
mlx5dr_table_is_root(matcher->tbl) ||
matcher->tbl->ctx->caps->access_index_mode ==
MLX5DR_MATCHER_INSERT_BY_HASH ||
matcher->tbl->ctx->caps->flow_table_hash_type !=
MLX5_FLOW_TABLE_HASH_TYPE_CRC32) {
+ DR_LOG(DEBUG, "Matcher is not supported");
rte_errno = ENOTSUP;
return -rte_errno;
}
--
2.34.1
[PATCH V3 0/3] Error report improvement and fix
This patch set is to improve error handling in pmd and under layer. Gavin Li (3): net/mlx5: set rte errno if malloc failed --- changelog: v0->v1 - Fix typo in commit message v1->v2 - Fix checkpatch warning --- net/mlx5/hws: add log for failing to create rule in HWS --- changelog: v1->v2 - Fix checkpatch warning v2->v3 - Fix checkpatch warning --- net/mlx5/hws: print CQE error syndrome and more information --- changelog: v1->v2 - Fix checkpatch warning v2->v3 - Fix checkpatch warning --- drivers/net/mlx5/hws/mlx5dr_rule.c | 6 ++ drivers/net/mlx5/hws/mlx5dr_send.c | 9 - drivers/net/mlx5/mlx5_flow_hw.c| 31 +++--- 3 files changed, 38 insertions(+), 8 deletions(-) -- 2.34.1
[PATCH V3 1/3] net/mlx5: set rte errno if malloc failed
rte_errno should be set if anything wrong happened in under layer so that
user can figure out what's going on.
There were some cases that did not set it when ipool allocation failed. To
fix the issue, set rte_errno to ENOMEM if mlx5_ipool_malloc failed to
allocate ID.
Fixes: c40c061a022e ("net/mlx5: add basic flow queue operation")
Fixes: 48fbb0e93d06 ("net/mlx5: support flow meter mark indirect action with
HWS")
cc: sta...@dpdk.org
Signed-off-by: Minggang Li(Gavin)
Acked-by: Bing Zhao
---
drivers/net/mlx5/mlx5_flow_hw.c | 31 ---
1 file changed, 24 insertions(+), 7 deletions(-)
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index a275154d4b..f34670b3ec 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -1905,7 +1905,7 @@ flow_hw_meter_mark_alloc(struct rte_eth_dev *dev,
uint32_t queue,
const struct rte_flow_action_meter_mark *meter_mark = action->conf;
struct mlx5_aso_mtr *aso_mtr;
struct mlx5_flow_meter_info *fm;
- uint32_t mtr_id;
+ uint32_t mtr_id = 0;
uintptr_t handle = (uintptr_t)MLX5_INDIRECT_ACTION_TYPE_METER_MARK <<
MLX5_INDIRECT_ACTION_TYPE_OFFSET;
@@ -1917,8 +1917,15 @@ flow_hw_meter_mark_alloc(struct rte_eth_dev *dev,
uint32_t queue,
if (meter_mark->profile == NULL)
return NULL;
aso_mtr = mlx5_ipool_malloc(pool->idx_pool, &mtr_id);
- if (!aso_mtr)
+ if (!aso_mtr) {
+ rte_flow_error_set(error, ENOMEM,
+ RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+ NULL,
+ "failed to allocate aso meter entry");
+ if (mtr_id)
+ mlx5_ipool_free(pool->idx_pool, mtr_id);
return NULL;
+ }
/* Fill the flow meter parameters. */
aso_mtr->type = ASO_METER_INDIRECT;
fm = &aso_mtr->fm;
@@ -3926,8 +3933,10 @@ flow_hw_async_flow_create(struct rte_eth_dev *dev,
return NULL;
}
flow = mlx5_ipool_malloc(table->flow, &flow_idx);
- if (!flow)
+ if (!flow) {
+ rte_errno = ENOMEM;
goto error;
+ }
rule_acts = flow_hw_get_dr_action_buffer(priv, table,
action_template_index, queue);
/*
* Set the table here in order to know the destination table
@@ -3938,8 +3947,10 @@ flow_hw_async_flow_create(struct rte_eth_dev *dev,
flow->idx = flow_idx;
if (table->resource) {
mlx5_ipool_malloc(table->resource, &res_idx);
- if (!res_idx)
+ if (!res_idx) {
+ rte_errno = ENOMEM;
goto error;
+ }
flow->res_idx = res_idx;
} else {
flow->res_idx = flow_idx;
@@ -4070,8 +4081,10 @@ flow_hw_async_flow_create_by_index(struct rte_eth_dev
*dev,
return NULL;
}
flow = mlx5_ipool_malloc(table->flow, &flow_idx);
- if (!flow)
+ if (!flow) {
+ rte_errno = ENOMEM;
goto error;
+ }
rule_acts = flow_hw_get_dr_action_buffer(priv, table,
action_template_index, queue);
/*
* Set the table here in order to know the destination table
@@ -4082,8 +4095,10 @@ flow_hw_async_flow_create_by_index(struct rte_eth_dev
*dev,
flow->idx = flow_idx;
if (table->resource) {
mlx5_ipool_malloc(table->resource, &res_idx);
- if (!res_idx)
+ if (!res_idx) {
+ rte_errno = ENOMEM;
goto error;
+ }
flow->res_idx = res_idx;
} else {
flow->res_idx = flow_idx;
@@ -4218,8 +4233,10 @@ flow_hw_async_flow_update(struct rte_eth_dev *dev,
nf->idx = of->idx;
if (table->resource) {
mlx5_ipool_malloc(table->resource, &res_idx);
- if (!res_idx)
+ if (!res_idx) {
+ rte_errno = ENOMEM;
goto error;
+ }
nf->res_idx = res_idx;
} else {
nf->res_idx = of->res_idx;
--
2.34.1
[PATCH V3 3/3] net/mlx5/hws: print CQE error syndrome and more information
Print CQE error syndrome and more information in case of queue error.
Signed-off-by: Minggang Li(Gavin)
Acked-by: Alex Vesker
---
drivers/net/mlx5/hws/mlx5dr_send.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/net/mlx5/hws/mlx5dr_send.c
b/drivers/net/mlx5/hws/mlx5dr_send.c
index 3022c50260..e9abf3dddb 100644
--- a/drivers/net/mlx5/hws/mlx5dr_send.c
+++ b/drivers/net/mlx5/hws/mlx5dr_send.c
@@ -598,8 +598,15 @@ static void mlx5dr_send_engine_poll_cq(struct
mlx5dr_send_engine *queue,
cqe_owner != sw_own)
return;
- if (unlikely(cqe_opcode != MLX5_CQE_REQ))
+ if (unlikely(cqe_opcode != MLX5_CQE_REQ)) {
+ struct mlx5_err_cqe *err_cqe = (struct mlx5_err_cqe *)cqe;
+
+ DR_LOG(ERR, "CQE ERR:0x%x, Vendor_ERR:0x%x, OP:0x%x, QPN:0x%x,
WQE_CNT:0x%x",
+ err_cqe->syndrome, err_cqe->vendor_err_synd, cqe_opcode,
+ (rte_be_to_cpu_32(err_cqe->s_wqe_opcode_qpn) &
0xff),
+ rte_be_to_cpu_16(err_cqe->wqe_counter));
queue->err = true;
+ }
rte_io_rmb();
--
2.34.1
[RFC PATCH v6] mempool: fix mempool cache size
This patch refactors the mempool cache to fix two bugs:
1. When a mempool is created with a cache size of N objects, the cache was
actually created with a size of 1.5 * N objects.
2. The mempool cache field names did not reflect their purpose;
the "flushthresh" field held the size, and the "size" field held the
number of objects remaining in the cache when returning from a get
operation refilling it from the backend.
Especially the first item could be fatal:
When more objects than a mempool's configured cache size is held in the
mempool's caches associated with other lcores, a rightsized mempool may
unexpectedly run out of objects, causing the application to fail.
Furthermore, this patch introduces two optimizations:
1. The mempool caches are flushed to/filled from the backend in their
entirety, so backend accesses are CPU cache line aligned. (Assuming the
mempool cache size is a multiplum of a CPU cache line size divided by the
size of a pointer.)
2. The unlikely paths in the get and put functions, where the cache is
flushed to/filled from the backend, are moved from the inline functions to
separate helper functions, thereby reducing the code size of the inline
functions.
Note: Accessing the backend for cacheless mempools remains inline.
Various drivers accessing the mempool directly have been updated
accordingly.
These drivers did not update mempool statistics when accessing the mempool
directly, so that is fixed too.
Note: Performance not yet benchmarked.
Signed-off-by: Morten Brørup
---
v6:
* Fix v5 incomplete implementation of passing large requests directly to
the backend.
* Use memcpy instead of rte_memcpy where compiler complains about it.
* Added const to some function parameters.
v5:
* Moved helper functions back into the header file, for improved
performance.
* Pass large requests directly to the backend. This also simplifies the
code.
v4:
* Updated subject to reflect that misleading names are considered bugs.
* Rewrote patch description to provide more details about the bugs fixed.
(Mattias Rönnblom)
* Moved helper functions, not to be inlined, to mempool C file.
(Mattias Rönnblom)
* Pass requests for n >= RTE_MEMPOOL_CACHE_MAX_SIZE objects known at build
time directly to backend driver, to avoid calling the helper functions.
This also fixes the compiler warnings about out of bounds array access.
v3:
* Removed __attribute__(assume).
v2:
* Removed mempool perf test; not part of patch set.
---
drivers/common/idpf/idpf_common_rxtx_avx512.c | 54 +---
drivers/mempool/dpaa/dpaa_mempool.c | 16 +-
drivers/mempool/dpaa2/dpaa2_hw_mempool.c | 14 -
drivers/net/i40e/i40e_rxtx_vec_avx512.c | 17 +-
drivers/net/iavf/iavf_rxtx_vec_avx512.c | 27 +-
drivers/net/ice/ice_rxtx_vec_avx512.c | 27 +-
lib/mempool/mempool_trace.h | 1 -
lib/mempool/rte_mempool.c | 12 +-
lib/mempool/rte_mempool.h | 287 --
9 files changed, 231 insertions(+), 224 deletions(-)
diff --git a/drivers/common/idpf/idpf_common_rxtx_avx512.c
b/drivers/common/idpf/idpf_common_rxtx_avx512.c
index 3b5e124ec8..98535a48f3 100644
--- a/drivers/common/idpf/idpf_common_rxtx_avx512.c
+++ b/drivers/common/idpf/idpf_common_rxtx_avx512.c
@@ -1024,21 +1024,13 @@ idpf_tx_singleq_free_bufs_avx512(struct idpf_tx_queue
*txq)
rte_lcore_id());
void **cache_objs;
- if (cache == NULL || cache->len == 0)
- goto normal;
-
- cache_objs = &cache->objs[cache->len];
-
- if (n > RTE_MEMPOOL_CACHE_MAX_SIZE) {
- rte_mempool_ops_enqueue_bulk(mp, (void *)txep, n);
+ if (!cache || unlikely(n + cache->len > cache->size)) {
+ rte_mempool_generic_put(mp, (void *)txep, n, cache);
goto done;
}
- /* The cache follows the following algorithm
-* 1. Add the objects to the cache
-* 2. Anything greater than the cache min value (if it
crosses the
-* cache flush threshold) is flushed to the ring.
-*/
+ cache_objs = &cache->objs[cache->len];
+
/* Add elements back into the cache */
uint32_t copied = 0;
/* n is multiple of 32 */
@@ -1056,16 +1048,13 @@ idpf_tx_singleq_free_bufs_avx512(struct idpf_tx_queue
*txq)
}
cache->len += n;
- if (cache->len >= cache->flushthresh) {
- rte_mempool_ops_enqueue_bulk(mp,
-&cache->objs[cache->size],
-cache->len - cache->size);
- cache->len = cache->size;
- }
+ /* Increment stat. */
+ RTE_MEMPOOL
Re: [PATCH v4 02/11] dts: add test case decorators
On 23. 9. 2024 21:26, Jeremy Spewock wrote: One super nit-pick comment below, even without that though I still think this looks good. Reviewed-by: Jeremy Spewock On Mon, Sep 23, 2024 at 11:02 AM Juraj Linkeš wrote: +def is_test_case(function: Callable) -> bool: +if inspect.isfunction(function): +# TestCase is not used at runtime, so we can't use isinstance() with `function`. +# But function.test_type exists. +if hasattr(function, "test_type"): +return isinstance(function.test_type, TestCaseType) +return False + +if test_case_sublist is None: +test_case_sublist = [] + +# the copy is needed so that the condition "elif test_case_sublist" doesn't +# change mid-cycle +test_case_sublist_copy = list(test_case_sublist) +func_test_cases = set() +perf_test_cases = set() + +for test_case_name, test_case_function in inspect.getmembers(cls, is_test_case): +if test_case_name in test_case_sublist_copy: +# if test_case_sublist_copy is non-empty, remove the found test case +# so that we can look at the remainder at the end +test_case_sublist_copy.remove(test_case_name) +elif test_case_sublist: +# the original list not being empty means we're filtering test cases This might read a little better if there was a period at the end, but I still think this gets the point across as is. Ack, I'll capitalize and add periods to all comments in this method. +# since we didn't remove test_case_name in the previous branch, +# it doesn't match the filter and we don't want to remove it +continue + +match test_case_function.test_type: +case TestCaseType.PERFORMANCE: +perf_test_cases.add(test_case_function) +case TestCaseType.FUNCTIONAL: +func_test_cases.add(test_case_function) + 2.43.0
Re: [PATCH v4 06/11] dts: add NIC capability support
On 23. 9. 2024 21:26, Jeremy Spewock wrote: On Mon, Sep 23, 2024 at 11:02 AM Juraj Linkeš wrote: Some test cases or suites may be testing a NIC feature that is not supported on all NICs, so add support for marking test cases or suites as requiring NIC capabilities. The marking is done with a decorator, which populates the internal required_capabilities attribute of TestProtocol. The NIC capability itself is a wrapper around the NicCapability defined in testpmd_shell. The reason is Enums cannot be extended and the class implements the methods of its abstract base superclass. The decorator API is designed to be simple to use. The arguments passed to it are all from the testpmd shell. Everything else (even the actual capability object creation) is done internally. Signed-off-by: Juraj Linkeš Reviewed-by: Dean Marx Thank you for addressing all my comments! Thanks you for giving them. Very helpful. Reviewed-by: Jeremy Spewock
[PATCH] net/r8169: add support for hw config
Implement the rtl_hw_config function to configure the hardware.
Signed-off-by: Howard Wang
---
drivers/net/r8169/meson.build| 1 +
drivers/net/r8169/r8169_base.h | 125 ++
drivers/net/r8169/r8169_ethdev.c | 2 +
drivers/net/r8169/r8169_ethdev.h | 15 +-
drivers/net/r8169/r8169_hw.c | 701 +++
drivers/net/r8169/r8169_hw.h | 17 +
drivers/net/r8169/r8169_phy.c| 41 ++
drivers/net/r8169/r8169_phy.h| 21 +
8 files changed, 921 insertions(+), 2 deletions(-)
create mode 100644 drivers/net/r8169/r8169_phy.c
create mode 100644 drivers/net/r8169/r8169_phy.h
diff --git a/drivers/net/r8169/meson.build b/drivers/net/r8169/meson.build
index ff7d6ca4b8..56f857ac8c 100644
--- a/drivers/net/r8169/meson.build
+++ b/drivers/net/r8169/meson.build
@@ -5,5 +5,6 @@ sources = files(
'r8169_ethdev.c',
'r8169_hw.c',
'r8169_rxtx.c',
+ 'r8169_phy.c',
)
diff --git a/drivers/net/r8169/r8169_base.h b/drivers/net/r8169/r8169_base.h
index 0e79d8d22a..2e72faeb2c 100644
--- a/drivers/net/r8169/r8169_base.h
+++ b/drivers/net/r8169/r8169_base.h
@@ -23,6 +23,117 @@ typedef uint16_t u16;
typedef uint32_t u32;
typedef uint64_t u64;
+enum mcfg {
+ CFG_METHOD_1 = 1,
+ CFG_METHOD_2,
+ CFG_METHOD_3,
+ CFG_METHOD_4,
+ CFG_METHOD_5,
+ CFG_METHOD_6,
+ CFG_METHOD_7,
+ CFG_METHOD_8,
+ CFG_METHOD_9,
+ CFG_METHOD_10,
+ CFG_METHOD_11,
+ CFG_METHOD_12,
+ CFG_METHOD_13,
+ CFG_METHOD_14,
+ CFG_METHOD_15,
+ CFG_METHOD_16,
+ CFG_METHOD_17,
+ CFG_METHOD_18,
+ CFG_METHOD_19,
+ CFG_METHOD_20,
+ CFG_METHOD_21,
+ CFG_METHOD_22,
+ CFG_METHOD_23,
+ CFG_METHOD_24,
+ CFG_METHOD_25,
+ CFG_METHOD_26,
+ CFG_METHOD_27,
+ CFG_METHOD_28,
+ CFG_METHOD_29,
+ CFG_METHOD_30,
+ CFG_METHOD_31,
+ CFG_METHOD_32,
+ CFG_METHOD_33,
+ CFG_METHOD_34,
+ CFG_METHOD_35,
+ CFG_METHOD_36,
+ CFG_METHOD_37,
+ CFG_METHOD_38,
+ CFG_METHOD_39,
+ CFG_METHOD_40,
+ CFG_METHOD_41,
+ CFG_METHOD_42,
+ CFG_METHOD_43,
+ CFG_METHOD_44,
+ CFG_METHOD_45,
+ CFG_METHOD_46,
+ CFG_METHOD_47,
+ CFG_METHOD_48,
+ CFG_METHOD_49,
+ CFG_METHOD_50,
+ CFG_METHOD_51,
+ CFG_METHOD_52,
+ CFG_METHOD_53,
+ CFG_METHOD_54,
+ CFG_METHOD_55,
+ CFG_METHOD_56,
+ CFG_METHOD_57,
+ CFG_METHOD_58,
+ CFG_METHOD_59,
+ CFG_METHOD_60,
+ CFG_METHOD_61,
+ CFG_METHOD_62,
+ CFG_METHOD_63,
+ CFG_METHOD_64,
+ CFG_METHOD_65,
+ CFG_METHOD_66,
+ CFG_METHOD_67,
+ CFG_METHOD_68,
+ CFG_METHOD_69,
+ CFG_METHOD_70,
+ CFG_METHOD_71,
+ CFG_METHOD_MAX,
+ CFG_METHOD_DEFAULT = 0xFF
+};
+
+enum bits {
+ BIT_0 = (1UL << 0),
+ BIT_1 = (1UL << 1),
+ BIT_2 = (1UL << 2),
+ BIT_3 = (1UL << 3),
+ BIT_4 = (1UL << 4),
+ BIT_5 = (1UL << 5),
+ BIT_6 = (1UL << 6),
+ BIT_7 = (1UL << 7),
+ BIT_8 = (1UL << 8),
+ BIT_9 = (1UL << 9),
+ BIT_10 = (1UL << 10),
+ BIT_11 = (1UL << 11),
+ BIT_12 = (1UL << 12),
+ BIT_13 = (1UL << 13),
+ BIT_14 = (1UL << 14),
+ BIT_15 = (1UL << 15),
+ BIT_16 = (1UL << 16),
+ BIT_17 = (1UL << 17),
+ BIT_18 = (1UL << 18),
+ BIT_19 = (1UL << 19),
+ BIT_20 = (1UL << 20),
+ BIT_21 = (1UL << 21),
+ BIT_22 = (1UL << 22),
+ BIT_23 = (1UL << 23),
+ BIT_24 = (1UL << 24),
+ BIT_25 = (1UL << 25),
+ BIT_26 = (1UL << 26),
+ BIT_27 = (1UL << 27),
+ BIT_28 = (1UL << 28),
+ BIT_29 = (1UL << 29),
+ BIT_30 = (1UL << 30),
+ BIT_31 = (1UL << 31)
+};
+
enum RTL_registers {
MAC0= 0x00, /* Ethernet hardware address */
MAC4= 0x04,
@@ -363,6 +474,8 @@ enum RTL_register_content {
INT_CFG0_ENABLE_8125= (1 << 0),
INT_CFG0_TIMEOUT0_BYPASS_8125 = (1 << 1),
INT_CFG0_MITIGATION_BYPASS_8125 = (1 << 2),
+ INT_CFG0_RDU_BYPASS_8126= (1 << 4),
+ INT_CFG0_MSIX_ENTRY_NUM_MODE= (1 << 5),
ISRIMR_V2_ROK_Q0 = (1 << 0),
ISRIMR_TOK_Q0= (1 << 16),
ISRIMR_TOK_Q1= (1 << 18),
@@ -389,6 +502,18 @@ enum RTL_register_content {
#define msleep rte_delay_ms
#define usleep rte_delay_us
+#define RX_DMA_BURST_unlimited 7 /* Maximum PCI burst, '7' is unlimited */
+#define RX_DMA_BURST_5125
+#define TX_DMA_BURST_unlimited 7
+#define TX_DMA_BURST_1024 6
+#define TX_DMA_BURST_5125
+#define TX_DMA_BURST_2564
+#define TX_DMA_BURST_1283
+#define TX_DMA_BURST_64 2
+#define TX_DMA_BURST_32 1
+#define TX_DMA_BURST_16 0
+#define InterFrameGap 0x03/* 3 means InterFrameG
grout: DPDK router project
Hello, dpdk.org is welcoming a new project: grout grout is a DPDK-based network processing application. It uses the graph library for data path processing. Its main purpose is to simulate a network function or a physical router for testing/replicating real (usually closed source) VNF/CNF behavior with an Open Source tool. The project is still in its early stages and it is very promising. Please check it by yourself and don't hesitate to participate in its growing. It could groute a lot more new protocol layers! The project has been presented at the DPDK summit 2024 in Montréal this Tuesday, September 24th: https://sched.co/1iAtk The source code is available here: https://git.dpdk.org/apps/grout/about/ https://github.com/DPDK/grout/ For any questions, you can send a message to gr...@dpdk.org or DPDK Slack #grout Happy grouting
Re: [PATCH v2 1/1] dts: add send_packets to test suites and rework packet addressing
On 20. 9. 2024 20:08, jspew...@iol.unh.edu wrote: From: Jeremy Spewock Currently the only method provided in the test suite class for sending packets sends a single packet and then captures the results. There is, in some cases, a need to send multiple packets at once while not really needing to capture any traffic received back. The method to do this exists in the traffic generator already, but this patch exposes the method to test suites. Patrick mentioned there is now a method that does it (send_packets_and_capture()), but that one captures the packets. I think we should add this method though, as it does something different, but maybe the most important point is that non-capturing TGs are going to support send_packets_and_capture(), so we need it either way. This patch also updates the _adjust_addresses method of test suites so that addresses of packets are only modified if the developer did not configure them beforehand. This allows for developers to have more control over the content of their packets when sending them through the framework. I think this could and should be split into two patches. They aren't really connected. diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py @@ -243,41 +255,74 @@ def get_expected_packet(self, packet: Packet) -> Packet: Returns: `packet` with injected L2/L3 addresses. """ -return self._adjust_addresses(packet, expected=True) +return self._adjust_addresses([packet], expected=True)[0] -def _adjust_addresses(self, packet: Packet, expected: bool = False) -> Packet: +def _adjust_addresses(self, packets: list[Packet], expected: bool = False) -> list[Packet]: """L2 and L3 address additions in both directions. +Packets in `packets` will be directly modified in this method. The returned list of packets +however will be copies of the modified packets in order to keep the two lists distinct. + Do we need to do this? I guess you needed this in a test suite - what is the reason? +Only missing addresses are added to packets, existing addresses will not be overridden. If +any packet in `packets` has multiple IP layers (using GRE, for example) only the inner-most +IP layer will have its addresses adjusted. + Assumptions: Two links between SUT and TG, one link is TG -> SUT, the other SUT -> TG. Args: -packet: The packet to modify. +packets: The packets to modify. expected: If :data:`True`, the direction is SUT -> TG, otherwise the direction is TG -> SUT. + +Returns: +A list containing copies of all packets in `packets` after modification. """ -if expected: -# The packet enters the TG from SUT -# update l2 addresses -packet.src = self._sut_port_egress.mac_address -packet.dst = self._tg_port_ingress.mac_address +ret_packets = [] +for packet in packets: +# The fields parameter of a packet does not include fields of the payload, so this can +# only be the Ether src/dst. +pkt_src_is_unset = "src" not in packet.fields +pkt_dst_is_unset = "dst" not in packet.fields +num_ip_layers = packet.layers().count(IP) + +# Update the last IP layer if there are multiple to account for GRE addressing (the This comment should be more generic as it's going to apply to other things than just GRE. +# framework should be modifying the packet address instead of the tunnel). +l3_to_use = packet.getlayer(IP, num_ip_layers) Would this make more sense inside the positive if branch right below? +if num_ip_layers > 0: +ip_src_is_unset = "src" not in l3_to_use.fields +ip_dst_is_unset = "dst" not in l3_to_use.fields +else: +ip_src_is_unset = None +ip_dst_is_unset = None -# The packet is routed from TG egress to TG ingress -# update l3 addresses -packet.payload.src = self._tg_ip_address_egress.ip.exploded -packet.payload.dst = self._tg_ip_address_ingress.ip.exploded -else: -# The packet leaves TG towards SUT # update l2 addresses -packet.src = self._tg_port_egress.mac_address -packet.dst = self._sut_port_ingress.mac_address +# If `expected` is :data:`True`, the packet enters the TG from SUT, otherwise the +# packet leaves the TG towards the SUT +if pkt_src_is_unset: +packet.src = ( +self._sut_port_egress.mac_address +if expected +else self._tg_port_egress.mac_address +) +if pkt_dst_is_unset: +packet
Re: [PATCH v2 1/1] dts: add binding to different drivers to TG node
On Tue, Sep 24, 2024 at 5:12 AM Juraj Linkeš wrote:
>
> I have some thoughts for the future:
> 1a. The traffic generator is specified per-node, so maybe we could also
> change the binding to be for the whole lifetime of the TG node,
> 1b. But the same is true for the SUT node as well, right? After we do
> the port update (with kernel driver), we can just bind to DPDK driver.
> With SUT in the mix, this looks like a change for a different patch,
Right, these are good points. A good observation too that we only
really need the kernel driver at the start in both cases. You had
mentioned in your previous comments as well that we should only be
binding on the TG once per lifetime, but I ended up not adding it for
that very reason of I still wanted the binding to be in Node, but I
didn't want to change the process for the SUT.
> 2. We could add a symlink to the devbind script with the target being in
> the dts directory. This way we don't have to go outside the dts
> directory and if DTS ever become a python package, we could just copy
> the script to the appropriate place. This is also something we don't
> really need to do.
I like this idea a lot actually. It feels very weird to me having to
step out of the DTS directory and I like the idea of keeping it
together like it were a package (even if it isn't yet).
>
> And also two minor comments. A lot of suggestions for separate patches
> overall. :-)
>
> On 19. 9. 2024 20:16, jspew...@iol.unh.edu wrote:
> > From: Jeremy Spewock
> >
> > The DTS framework in its current state supports binding ports to
> > different drivers on the SUT node but not the TG node. The TG node
> > already has the information that it needs about the different drivers
> > that it has available in the configuration file, but it did not
> > previously have access to the devbind script, so it did not use that
> > information for anything.
> >
> > This patch moves the location of the tmp directory as well as the method
> > for binding ports into the node class rather than the SUT node class and
> > adds an abstract method for getting the path to the devbind script into
> > the node class. Then, binding ports to the correct drivers is moved into
> > the build target setup and run on both nodes.
> >
> > Bugzilla ID: 1420
> >
> > Signed-off-by: Jeremy Spewock
> > ---
>
> With the two minor comments,
> Reviewed-by: Juraj Linkeš
>
> > diff --git a/dts/framework/testbed_model/node.py
> > b/dts/framework/testbed_model/node.py
>
> > @@ -58,8 +65,10 @@ class Node(ABC):
> > lcores: list[LogicalCore]
> > ports: list[Port]
> > _logger: DTSLogger
> > +_remote_tmp_dir: PurePath
> > _other_sessions: list[OSSession]
> > _test_run_config: TestRunConfiguration
> > +_path_to_devbind_script: PurePath | None
>
> A note on the naming. We have _remote_tmp_dir and
> _path_to_devbind_script. Both are pointing to a remote file/dir, but
> only one has the _remote prefix. They should probably be unified.
I didn't think of this but you're right, the two are very similar but
named differently.
>
> I've thought a bit about what the right name is. Dropping the prefix
> makes sense; sut_node.tmp_dir should mean the tmp dir on the SUT node
> (which would make it remote from the execution host's point of view, but
> not the node's view; the file is local to SUT node). This could be a
> good separate patch (improving the remote/local naming scheme to make it
> consistent and as sensible as possible).
I also like the sound of it without the prefix and how it actually has
a more fitting meaning from the two perspectives. I agree that there
is probably some other work to be done on this in another patch, but
since I am moving the _remote_tmp_dir variable around anyway I think
it wouldn't hurt for me to rename it.
>
>
> > diff --git a/dts/framework/utils.py b/dts/framework/utils.py
>
> > @@ -29,6 +29,8 @@
> > from .exception import ConfigurationError, InternalError
> >
> > REGEX_FOR_PCI_ADDRESS: str =
> > "/[0-9a-fA-F]{4}:[0-9a-fA-F]{2}:[0-9a-fA-F]{2}.[0-9]{1}/"
> > +#: Path to DPDK directory on the host where DTS is being run.
> > +LOCAL_DPDK_DIR: PurePath = PurePath(__file__).parents[2]
>
> Local paths can be just pathlib.Path. PurePaths are for path
> manipulations only (useful for remote paths in RemoteSessions,
> OSSessions and Nodes), but for local existing paths we should use Path.
>
> The OSSession and subclasses need a bit of an update in this regard -
> use Path for local paths and PurePaths for remote ones. We added this
> into our pre-built DPDK patch.
Ack. I tried to search for the differences between the two actually
and saw a similar answer that PurePaths are used for manipulation, but
I didn't fully understand as I am still manipulated this path and it
isn't referencing a real file on host where I am defining the variable
(what ever host is running DTS that is). This is a good distinction
though to use Paths when it is used as a local path somewhere.
>
> >
> >
RE: [PATCH v3] eal: add build-time option to omit trace
> > +++ b/app/test/test_trace.c
> > @@ -12,7 +12,16 @@
> >
> > int app_dpdk_test_tp_count;
> >
> > -#ifdef RTE_EXEC_ENV_WINDOWS
> > +#if !defined(RTE_TRACE)
>
> Now that, we are is not disabling any symbols,
> This conditional compilation can be removed. Use
> __rte_trace_point_generic_is_enabled() in another leg.
Done in v4.
Also added to perf test.
> > +/**
> > + * @internal
> > + *
> > + * Test if the tracepoint compile-time option is enabled.
> > + *
> > + * @return
> > + * true if tracepoint enabled, false otherwise.
> > + */
> > +__rte_experimental
> > +static __rte_always_inline bool
> > +__rte_trace_point_generic_is_enabled(void)
>
> Add to version.map file
It's static inline, so not an exposed symbol.
__rte_trace_point_fp_is_enabled is also not in the version.map file.
> > #define __rte_trace_point_emit_header_generic(t) \
> > void *mem; \
> > do { \
> > + if (!__rte_trace_point_generic_is_enabled()) \
> > + return; \
>
>
> To be super safe, Add similar check in
> RTE_INIT(trace##_init)
Done in v4.
Re: [PATCH v4] eal: add build-time option to omit trace
On Tue, Sep 24, 2024 at 7:10 PM Morten Brørup wrote: > > Some applications want to omit the trace feature. > Either to reduce the memory footprint, to reduce the exposed attack > surface, or for other reasons. > > This patch adds an option in rte_config.h to include or omit trace in the > build. Trace is included by default. > > Omitting trace works by omitting all trace points. > For API and ABI compatibility, the trace feature itself remains. > > Signed-off-by: Morten Brørup > --- > v4: > * Added check for generic trace enabled when registering trace points, in > RTE_INIT. (Jerin Jacob) > * Test application uses function instead of macro to check if generic > trace is enabled. (Jerin Jacob) > * Performance test application uses function to check if generic trace is > enabled. > v3: > * Simpler version with much fewer ifdefs. (Jerin Jacob) > v2: > * Added/modified macros required for building applications with trace > omitted. > > +/** > + * @internal Since it is used in app/test. Can we remove it as internal and make symbol as rte_trace_point_is_enabled > + * > + * Test if the tracepoint compile-time option is enabled. > + * > + * @return > + * true if tracepoint enabled, false otherwise. > + */ > +__rte_experimental > +static __rte_always_inline bool > +__rte_trace_point_generic_is_enabled(void) Do we need to keep _generic_ Rest looks good to me.
[RFC PATCH v7] mempool: fix mempool cache size
This patch refactors the mempool cache to fix two bugs:
1. When a mempool is created with a cache size of N objects, the cache was
actually created with a size of 1.5 * N objects.
2. The mempool cache field names did not reflect their purpose;
the "flushthresh" field held the size, and the "size" field held the
number of objects remaining in the cache when returning from a get
operation refilling it from the backend.
Especially the first item could be fatal:
When more objects than a mempool's configured cache size is held in the
mempool's caches associated with other lcores, a rightsized mempool may
unexpectedly run out of objects, causing the application to fail.
Furthermore, this patch introduces two optimizations:
1. The mempool caches are flushed to/filled from the backend in their
entirety, so backend accesses are CPU cache line aligned. (Assuming the
mempool cache size is a multiplum of a CPU cache line size divided by the
size of a pointer.)
2. The unlikely paths in the get and put functions, where the cache is
flushed to/filled from the backend, are moved from the inline functions to
separate helper functions, thereby reducing the code size of the inline
functions.
Note: Accessing the backend for cacheless mempools remains inline.
Various drivers accessing the mempool directly have been updated
accordingly.
These drivers did not update mempool statistics when accessing the mempool
directly, so that is fixed too.
Note: Performance not yet benchmarked.
Signed-off-by: Morten Brørup
---
v7:
* Increased max mempool cache size from 512 to 1024 objects.
Mainly for CI performance test purposes.
Originally, the max mempool cache size was 768 objects, and used a fixed
size array of 1024 objects in the mempool cache structure.
v6:
* Fix v5 incomplete implementation of passing large requests directly to
the backend.
* Use memcpy instead of rte_memcpy where compiler complains about it.
* Added const to some function parameters.
v5:
* Moved helper functions back into the header file, for improved
performance.
* Pass large requests directly to the backend. This also simplifies the
code.
v4:
* Updated subject to reflect that misleading names are considered bugs.
* Rewrote patch description to provide more details about the bugs fixed.
(Mattias Rönnblom)
* Moved helper functions, not to be inlined, to mempool C file.
(Mattias Rönnblom)
* Pass requests for n >= RTE_MEMPOOL_CACHE_MAX_SIZE objects known at build
time directly to backend driver, to avoid calling the helper functions.
This also fixes the compiler warnings about out of bounds array access.
v3:
* Removed __attribute__(assume).
v2:
* Removed mempool perf test; not part of patch set.
---
config/rte_config.h | 2 +-
drivers/common/idpf/idpf_common_rxtx_avx512.c | 54 +---
drivers/mempool/dpaa/dpaa_mempool.c | 16 +-
drivers/mempool/dpaa2/dpaa2_hw_mempool.c | 14 -
drivers/net/i40e/i40e_rxtx_vec_avx512.c | 17 +-
drivers/net/iavf/iavf_rxtx_vec_avx512.c | 27 +-
drivers/net/ice/ice_rxtx_vec_avx512.c | 27 +-
lib/mempool/mempool_trace.h | 1 -
lib/mempool/rte_mempool.c | 12 +-
lib/mempool/rte_mempool.h | 287 --
10 files changed, 232 insertions(+), 225 deletions(-)
diff --git a/config/rte_config.h b/config/rte_config.h
index dd7bb0d35b..2488ff167d 100644
--- a/config/rte_config.h
+++ b/config/rte_config.h
@@ -56,7 +56,7 @@
#define RTE_CONTIGMEM_DEFAULT_BUF_SIZE (512*1024*1024)
/* mempool defines */
-#define RTE_MEMPOOL_CACHE_MAX_SIZE 512
+#define RTE_MEMPOOL_CACHE_MAX_SIZE 1024
/* RTE_LIBRTE_MEMPOOL_STATS is not set */
/* RTE_LIBRTE_MEMPOOL_DEBUG is not set */
diff --git a/drivers/common/idpf/idpf_common_rxtx_avx512.c
b/drivers/common/idpf/idpf_common_rxtx_avx512.c
index 3b5e124ec8..98535a48f3 100644
--- a/drivers/common/idpf/idpf_common_rxtx_avx512.c
+++ b/drivers/common/idpf/idpf_common_rxtx_avx512.c
@@ -1024,21 +1024,13 @@ idpf_tx_singleq_free_bufs_avx512(struct idpf_tx_queue
*txq)
rte_lcore_id());
void **cache_objs;
- if (cache == NULL || cache->len == 0)
- goto normal;
-
- cache_objs = &cache->objs[cache->len];
-
- if (n > RTE_MEMPOOL_CACHE_MAX_SIZE) {
- rte_mempool_ops_enqueue_bulk(mp, (void *)txep, n);
+ if (!cache || unlikely(n + cache->len > cache->size)) {
+ rte_mempool_generic_put(mp, (void *)txep, n, cache);
goto done;
}
- /* The cache follows the following algorithm
-* 1. Add the objects to the cache
-* 2. Anything greater than the cache min value (if it
crosses the
-* cache flush threshold) is flushed to the ring.
-*/
+ cache_o
Re: [PATCH v4] eal: add build-time option to omit trace
On Tue, Sep 24, 2024 at 9:23 PM Morten Brørup wrote: > > > From: Jerin Jacob [mailto:jerinjac...@gmail.com] > > Sent: Tuesday, 24 September 2024 11.42 > > > > On Tue, Sep 24, 2024 at 7:10 PM Morten Brørup > > wrote: > > > > > > Some applications want to omit the trace feature. > > > Either to reduce the memory footprint, to reduce the exposed attack > > > surface, or for other reasons. > > > > > > This patch adds an option in rte_config.h to include or omit trace in > > the > > > build. Trace is included by default. > > > > > > Omitting trace works by omitting all trace points. > > > For API and ABI compatibility, the trace feature itself remains. > > > > > > Signed-off-by: Morten Brørup > > > --- > > > v4: > > > * Added check for generic trace enabled when registering trace points, > > in > > > RTE_INIT. (Jerin Jacob) > > > * Test application uses function instead of macro to check if generic > > > trace is enabled. (Jerin Jacob) > > > * Performance test application uses function to check if generic trace > > is > > > enabled. > > > v3: > > > * Simpler version with much fewer ifdefs. (Jerin Jacob) > > > v2: > > > * Added/modified macros required for building applications with trace > > > omitted. > > > > > > > > +/** > > > + * @internal > > > > Since it is used in app/test. Can we remove it as internal and make > > symbol as rte_trace_point_is_enabled > > I don't think we should make functions public if only used for test purposes. > > Do you think it is useful for normal usage too? Does rte_trace_is_enabled() > not suffice? It will be good for an app to know, Is trace feature disabled if the application cares about. Yes. rte_trace_is_enabled() suffice. > > > > > > + * > > > + * Test if the tracepoint compile-time option is enabled. > > > + * > > > + * @return > > > + * true if tracepoint enabled, false otherwise. > > > + */ > > > +__rte_experimental > > > +static __rte_always_inline bool > > > +__rte_trace_point_generic_is_enabled(void) > > > > Do we need to keep _generic_ > > Other internal functions have _generic_ too, so I added it. > If we decide to make it public, I agree _generic_ should be removed. > > > > > Rest looks good to me.
RE: [EXT] [PATCH 1/3] cryptodev: add ec points to sm2 op
Hi Arek, Can you resend this series if you still want to pursue so that CI can run? > > > In the case when PMD cannot support full process of the SM2, but > > > elliptic curve computation only, additional fields are needed to > > > handle such a case. > > > > > > > Asym crypto APIs are no longer experimental. > > Hence adding new fields would lead to ABI break. > > It seems that > `__rte_crypto_op_reset` > `rte_crypto_op_pool_create` > functions do not need versioning, and we could easily do it if needed. > But the field `flags` changes an offset, and this is actually problematic. > Which means that we cannot do this change before 24.11. > > > > > > Points C1, kP therefore were added to the SM2 crypto operation struct. > > > > > > Signed-off-by: Arkadiusz Kusztal
Re: [PATCH v3 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell
On Tue, Sep 24, 2024 at 6:55 AM Juraj Linkeš wrote: > > I like how this looks. I have a number of minor comments (mainly wording > and naming), but overall it looks very good. > > On 19. 9. 2024 21:02, jspew...@iol.unh.edu wrote: > > From: Jeremy Spewock > > > > Previously all scapy commands were handled using an XML-RPC server that > > ran on the TGNode. This unnecessarily enforces a minimum Python version > > of 3.10 on the server that is being used as a traffic generator and > > complicates the implementation of scapy methods. > > What is the TG's minimum Python version now? > https://bugs.dpdk.org/show_bug.cgi?id=1388 says this will become a moot > point, but we're still using Python on the remote node. Right, there is still some dependency there. I'm not sure the exact versions, but doing some looking around I believe one of the newest things scapy tools we use in the framework is the AsyncSniffer and the scapy documentation [1] says that it has been available since version 2.4.3, and then when I looked at that version of the scapy package [2] it looks like it claims to support python 2.7 and python 3.4-7. Poking around in the documentation/code from scapy version 2.4.3 it also looks like the syntax is very similar, so I believe it would work, but I'm not sure I have any hosts that I could run on which have python 3.4. Maybe that does make the dependency essentially a moot point considering these are fairly old python versions. [1] https://github.com/secdev/scapy/blob/master/doc/scapy/usage.rst [2] https://pypi.org/project/scapy/2.4.3/ > > > > diff --git > > a/dts/framework/remote_session/single_active_interactive_shell.py > > b/dts/framework/remote_session/single_active_interactive_shell.py > > > @@ -93,9 +94,13 @@ def __init__( > > timeout: float = SETTINGS.timeout, > > app_params: Params = Params(), > > name: str | None = None, > > +**kwargs, > > ) -> None: > > """Create an SSH channel during initialization. > > > > +Additional key-word arguments can be passed through `kwargs` is > > needed to fulfill other > > Typo: is -> if > > And key-word should be just keyword. > Ack. > Also, should we add super().__init__(), seeing as we also added it to > TrafficGenerator? > We probably should so that it doesn't matter which order you specify the two in and they still work, this is probably something that I missed in the rebase. > > > diff --git a/dts/framework/testbed_model/traffic_generator/scapy.py > > b/dts/framework/testbed_model/traffic_generator/scapy.py > > > +class ScapyTrafficGenerator(PythonShell, CapturingTrafficGenerator): > > > +def __init__(self, tg_node: Node, config: ScapyTrafficGeneratorConfig, > > **kwargs): > > The usage of privileged=True when creating the instance confused me for > a bit, because it's not this class's argument, but rather > SingleActiveInteractiveShell's. I thing we should document the arguments > of SingleActiveInteractiveShell in the Keyword Args section. We probably > need just a link to SingleActiveInteractiveShell. Sure, I can add some kind of reference. I agree that it's confusing, I wish there was a simple way we could unpack the parameters of the interactive shell into kwargs, but when I was searching I couldn't really find anything great. Other than doing something like making TypedDict classes for interactive shell parameters, but I'm not sure that's super sleek either. > > > > +def _create_packet_list(self, packets: list[Packet]) -> None: > > Maybe we could apply some of the ideas from the local/remote naming > scheme I talked about in the tg devbind script patch here. Whatever > happens on the TG could be prefixed with remote and whatever is > happening locally would be without the prefix (or maybe whatever is > happnening on the TG shouldn't be prefixed (or a different prefix - > shell)? Makes sense, but then we'd need a good prefix for what's > happening on the execution environment. Maybe this also needs to be in a > different patch, after it's been though through with everything else in I'll still write something here that makes the distinction and that other patch could reformat if the author thought something else was better. > mind.). That would make this _create_remote_packet_list, but we're just > setting a variable (the passed packets have already been built), so > maybe _set_remote_packet_list? > > Or maybe all of the remote methods could start with remote, making it > _remote_set_packet_list (or _shell_set_packet_list? Doesn't sound bad.). > Not sure which is better, maybe after renaming more of these it's going > to be clearer. This is a good idea. I like remote more initially, but I'll try it out on some of the methods and see if shell fits better. Regardless, I think one of those two would be a good option as well. > > Whatever we go with, the naming scheme should be explained in the > class's dosctring. Ack. > > > > +def _create_sniffer( > > +
[PATCH v2 0/2] cryptodev: add queue pair priority
Changes in v2: - added release notes and removed deprecation notice. - removed gerrit-id Akhil Goyal (2): cryptodev: add queue pair priority app/crypto-perf: test queue pair priority app/test-crypto-perf/cperf_options.h | 3 +++ app/test-crypto-perf/cperf_options_parsing.c | 20 ++ app/test-crypto-perf/main.c | 6 +- doc/guides/rel_notes/deprecation.rst | 3 --- doc/guides/rel_notes/release_24_11.rst | 3 +++ doc/guides/tools/cryptoperf.rst | 5 + lib/cryptodev/rte_cryptodev.h| 22 7 files changed, 58 insertions(+), 4 deletions(-) -- 2.25.1
[PATCH v2 2/2] app/crypto-perf: test queue pair priority
Updated the application to test variable queue pair priority.
A mask using `--low-prio-qp-mask` can be set to lower the priority
of queues which are set in the mask.
This would result in lower performance for those queues.
By default the priority is set as highest.
This option is added just to verify the performance drop
of queues which have lower priority set.
Signed-off-by: Akhil Goyal
---
app/test-crypto-perf/cperf_options.h | 3 +++
app/test-crypto-perf/cperf_options_parsing.c | 20
app/test-crypto-perf/main.c | 6 +-
doc/guides/tools/cryptoperf.rst | 5 +
4 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/app/test-crypto-perf/cperf_options.h
b/app/test-crypto-perf/cperf_options.h
index 131ecfdffb..48f409c3ba 100644
--- a/app/test-crypto-perf/cperf_options.h
+++ b/app/test-crypto-perf/cperf_options.h
@@ -32,6 +32,8 @@
#define CPERF_TEST_FILE("test-file")
#define CPERF_TEST_NAME("test-name")
+#define CPERF_LOW_PRIO_QP_MASK ("low-prio-qp-mask")
+
#define CPERF_CIPHER_ALGO ("cipher-algo")
#define CPERF_CIPHER_OP("cipher-op")
#define CPERF_CIPHER_KEY_SZ("cipher-key-sz")
@@ -107,6 +109,7 @@ struct cperf_options {
uint32_t *imix_buffer_sizes;
uint32_t nb_descriptors;
uint16_t nb_qps;
+ uint64_t low_prio_qp_mask;
uint32_t sessionless:1;
uint32_t shared_session:1;
diff --git a/app/test-crypto-perf/cperf_options_parsing.c
b/app/test-crypto-perf/cperf_options_parsing.c
index c91fcf0479..8c15cd813f 100644
--- a/app/test-crypto-perf/cperf_options_parsing.c
+++ b/app/test-crypto-perf/cperf_options_parsing.c
@@ -37,6 +37,7 @@ usage(char *progname)
" --segment-sz N: set the size of the segment to use\n"
" --desc-nb N: set number of descriptors for each crypto
device\n"
" --devtype TYPE: set crypto device type to use\n"
+ " --low-prio-qp-mask mask: set low priority for queues set in
mask(hex)\n"
" --optype cipher-only / auth-only / cipher-then-auth /
auth-then-cipher /\n"
"aead / pdcp / docsis / ipsec / modex / secp256r1 / sm2
/ tls-record : set operation type\n"
" --sessionless: enable session-less crypto operations\n"
@@ -941,6 +942,22 @@ parse_pmd_cyclecount_delay_ms(struct cperf_options *opts,
return 0;
}
+static int
+parse_low_prio_qp_mask(struct cperf_options *opts, const char *arg)
+{
+ char *end = NULL;
+ unsigned long n;
+
+ /* parse hexadecimal string */
+ n = strtoul(arg, &end, 16);
+ if ((optarg[0] == '\0') || (end == NULL) || (*end != '\0'))
+ return -1;
+
+ opts->low_prio_qp_mask = n;
+
+ return 0;
+}
+
typedef int (*option_parser_t)(struct cperf_options *opts,
const char *arg);
@@ -962,6 +979,8 @@ static struct option lgopts[] = {
{ CPERF_SEGMENT_SIZE, required_argument, 0, 0 },
{ CPERF_DESC_NB, required_argument, 0, 0 },
+ { CPERF_LOW_PRIO_QP_MASK, required_argument, 0, 0 },
+
{ CPERF_IMIX, required_argument, 0, 0 },
{ CPERF_DEVTYPE, required_argument, 0, 0 },
{ CPERF_OPTYPE, required_argument, 0, 0 },
@@ -1097,6 +1116,7 @@ cperf_opts_parse_long(int opt_idx, struct cperf_options
*opts)
{ CPERF_BUFFER_SIZE,parse_buffer_sz },
{ CPERF_SEGMENT_SIZE, parse_segment_sz },
{ CPERF_DESC_NB,parse_desc_nb },
+ { CPERF_LOW_PRIO_QP_MASK, parse_low_prio_qp_mask },
{ CPERF_DEVTYPE,parse_device_type },
{ CPERF_OPTYPE, parse_op_type },
{ CPERF_SESSIONLESS,parse_sessionless },
diff --git a/app/test-crypto-perf/main.c b/app/test-crypto-perf/main.c
index 75810dbf0b..eac87091a1 100644
--- a/app/test-crypto-perf/main.c
+++ b/app/test-crypto-perf/main.c
@@ -249,7 +249,8 @@ cperf_initialize_cryptodev(struct cperf_options *opts,
uint8_t *enabled_cdevs)
}
struct rte_cryptodev_qp_conf qp_conf = {
- .nb_descriptors = opts->nb_descriptors
+ .nb_descriptors = opts->nb_descriptors,
+ .priority = RTE_CRYPTODEV_QP_PRIORITY_HIGHEST
};
/**
@@ -315,6 +316,9 @@ cperf_initialize_cryptodev(struct cperf_options *opts,
uint8_t *enabled_cdevs)
}
for (j = 0; j < opts->nb_qps; j++) {
+ if ((1 << j) & opts->low_prio_qp_mask)
+ qp_conf.priority =
RTE_CRYPTODEV_QP_PRIORITY_LOWEST;
+
ret = rte_cryptodev_queue_pair_setup(cdev_id, j,
&qp_conf, socket_id);
if (ret < 0) {
diff --git a/doc/guides/tools/cryptoperf.rst b/doc/guides/tools/cryptoperf.rst
index 0510a3
[PATCH v2 1/2] cryptodev: add queue pair priority
Added a new field priority in `rte_cryptodev_qp_conf`,
to set the queue pair priority while setting up.
The priorities can be set between
`RTE_CRYPTODEV_QP_PRIORITY_HIGHEST` and
`RTE_CRYPTODEV_QP_PRIORITY_LOWEST`.
The underlying implementation may normalize the value
as per the supported priority levels.
If the implementation does not support setting up
priority, all queue pairs will be on same priority level
and this field will be ignored.
Signed-off-by: Akhil Goyal
---
doc/guides/rel_notes/deprecation.rst | 3 ---
doc/guides/rel_notes/release_24_11.rst | 3 +++
lib/cryptodev/rte_cryptodev.h | 22 ++
3 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/doc/guides/rel_notes/deprecation.rst
b/doc/guides/rel_notes/deprecation.rst
index 211f59fdc9..3605e9cddc 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -174,9 +174,6 @@ Deprecation Notices
which got error interrupt to the application,
so that application can reset that particular queue pair.
-* cryptodev: The structure ``rte_cryptodev_qp_conf`` will be updated
- to have a new parameter to set priority of that particular queue pair.
-
* cryptodev: The enum ``rte_crypto_asym_xform_type`` and struct
``rte_crypto_asym_op``
will be extended to include new values to support EDDSA.
This will break ABI compatibility with existing applications that use these
data types.
diff --git a/doc/guides/rel_notes/release_24_11.rst
b/doc/guides/rel_notes/release_24_11.rst
index 0ff70d9057..fcdc738cf6 100644
--- a/doc/guides/rel_notes/release_24_11.rst
+++ b/doc/guides/rel_notes/release_24_11.rst
@@ -100,6 +100,9 @@ ABI Changes
Also, make sure to start the actual text at the margin.
===
+* cryptodev: The queue pair configuration structure ``rte_cryptodev_qp_conf``
+ is updated to have a new parameter to set priority of that particular queue
pair.
+
Known Issues
diff --git a/lib/cryptodev/rte_cryptodev.h b/lib/cryptodev/rte_cryptodev.h
index bec947f6d5..0a9cd718ea 100644
--- a/lib/cryptodev/rte_cryptodev.h
+++ b/lib/cryptodev/rte_cryptodev.h
@@ -608,12 +608,34 @@ enum rte_cryptodev_event_type {
RTE_CRYPTODEV_EVENT_MAX /**< max value of this enum */
};
+/* Crypto queue pair priority levels */
+#define RTE_CRYPTODEV_QP_PRIORITY_HIGHEST 0
+/**< Highest priority of a cryptodev queue pair
+ * @see rte_cryptodev_queue_pair_setup(), rte_cryptodev_enqueue_burst()
+ */
+#define RTE_CRYPTODEV_QP_PRIORITY_NORMAL128
+/**< Normal priority of a cryptodev queue pair
+ * @see rte_cryptodev_queue_pair_setup(), rte_cryptodev_enqueue_burst()
+ */
+#define RTE_CRYPTODEV_QP_PRIORITY_LOWEST255
+/**< Lowest priority of a cryptodev queue pair
+ * @see rte_cryptodev_queue_pair_setup(), rte_cryptodev_enqueue_burst()
+ */
+
/** Crypto device queue pair configuration structure. */
/* Structure rte_cryptodev_qp_conf 8<*/
struct rte_cryptodev_qp_conf {
uint32_t nb_descriptors; /**< Number of descriptors per queue pair */
struct rte_mempool *mp_session;
/**< The mempool for creating session in sessionless mode */
+ uint8_t priority;
+ /**< Priority for this queue pair relative to other queue pairs.
+*
+* The requested priority should in the range of
+* [@ref RTE_CRYPTODEV_QP_PRIORITY_HIGHEST, @ref
RTE_CRYPTODEV_QP_PRIORITY_LOWEST].
+* The implementation may normalize the requested priority to
+* device supported priority value.
+*/
};
/* >8 End of structure rte_cryptodev_qp_conf. */
--
2.25.1
[PATCH v4] eal: add build-time option to omit trace
Some applications want to omit the trace feature.
Either to reduce the memory footprint, to reduce the exposed attack
surface, or for other reasons.
This patch adds an option in rte_config.h to include or omit trace in the
build. Trace is included by default.
Omitting trace works by omitting all trace points.
For API and ABI compatibility, the trace feature itself remains.
Signed-off-by: Morten Brørup
---
v4:
* Added check for generic trace enabled when registering trace points, in
RTE_INIT. (Jerin Jacob)
* Test application uses function instead of macro to check if generic
trace is enabled. (Jerin Jacob)
* Performance test application uses function to check if generic trace is
enabled.
v3:
* Simpler version with much fewer ifdefs. (Jerin Jacob)
v2:
* Added/modified macros required for building applications with trace
omitted.
---
app/test/test_trace.c | 4
app/test/test_trace_perf.c | 5 +
config/rte_config.h| 1 +
lib/eal/include/rte_trace_point.h | 21 +
lib/eal/include/rte_trace_point_register.h | 2 ++
5 files changed, 33 insertions(+)
diff --git a/app/test/test_trace.c b/app/test/test_trace.c
index 00809f433b..26656fe024 100644
--- a/app/test/test_trace.c
+++ b/app/test/test_trace.c
@@ -245,6 +245,10 @@ static struct unit_test_suite trace_tests = {
static int
test_trace(void)
{
+ if (!__rte_trace_point_generic_is_enabled()) {
+ printf("Trace omitted at build-time, skipping test\n");
+ return TEST_SKIPPED;
+ }
return unit_test_suite_runner(&trace_tests);
}
diff --git a/app/test/test_trace_perf.c b/app/test/test_trace_perf.c
index 8257cc02be..504574ef8a 100644
--- a/app/test/test_trace_perf.c
+++ b/app/test/test_trace_perf.c
@@ -150,6 +150,11 @@ test_trace_perf(void)
struct test_data *data;
size_t sz;
+ if (!__rte_trace_point_generic_is_enabled()) {
+ printf("Trace omitted at build-time, skipping test\n");
+ return TEST_SKIPPED;
+ }
+
nb_cores = rte_lcore_count();
nb_workers = nb_cores - 1;
if (nb_cores < 2) {
diff --git a/config/rte_config.h b/config/rte_config.h
index dd7bb0d35b..fd6f8a2f1a 100644
--- a/config/rte_config.h
+++ b/config/rte_config.h
@@ -49,6 +49,7 @@
#define RTE_MAX_TAILQ 32
#define RTE_LOG_DP_LEVEL RTE_LOG_INFO
#define RTE_MAX_VFIO_CONTAINERS 64
+#define RTE_TRACE 1
/* bsd module defines */
#define RTE_CONTIGMEM_MAX_NUM_BUFS 64
diff --git a/lib/eal/include/rte_trace_point.h
b/lib/eal/include/rte_trace_point.h
index 41e2a7f99e..b80688ce89 100644
--- a/lib/eal/include/rte_trace_point.h
+++ b/lib/eal/include/rte_trace_point.h
@@ -212,6 +212,25 @@ bool rte_trace_point_is_enabled(rte_trace_point_t *tp);
__rte_experimental
rte_trace_point_t *rte_trace_point_lookup(const char *name);
+/**
+ * @internal
+ *
+ * Test if the tracepoint compile-time option is enabled.
+ *
+ * @return
+ * true if tracepoint enabled, false otherwise.
+ */
+__rte_experimental
+static __rte_always_inline bool
+__rte_trace_point_generic_is_enabled(void)
+{
+#ifdef RTE_TRACE
+ return true;
+#else
+ return false;
+#endif
+}
+
/**
* @internal
*
@@ -359,6 +378,8 @@ __rte_trace_point_emit_ev_header(void *mem, uint64_t in)
#define __rte_trace_point_emit_header_generic(t) \
void *mem; \
do { \
+ if (!__rte_trace_point_generic_is_enabled()) \
+ return; \
const uint64_t val = rte_atomic_load_explicit(t,
rte_memory_order_acquire); \
if (likely(!(val & __RTE_TRACE_FIELD_ENABLE_MASK))) \
return; \
diff --git a/lib/eal/include/rte_trace_point_register.h
b/lib/eal/include/rte_trace_point_register.h
index 41260e5964..429b993fc2 100644
--- a/lib/eal/include/rte_trace_point_register.h
+++ b/lib/eal/include/rte_trace_point_register.h
@@ -23,6 +23,8 @@ rte_trace_point_t __rte_section("__rte_trace_point")
__##trace; \
static const char __##trace##_name[] = RTE_STR(name); \
RTE_INIT(trace##_init) \
{ \
+ if (!__rte_trace_point_generic_is_enabled()) \
+ return; \
__rte_trace_point_register(&__##trace, __##trace##_name, \
(void (*)(void)) trace); \
}
--
2.43.0
Re: [PATCH v3 2/2] dts: add binding to different drivers to TG node
On 24. 9. 2024 18:28, jspew...@iol.unh.edu wrote: From: Jeremy Spewock The DTS framework in its current state supports binding ports to different drivers on the SUT node but not the TG node. The TG node already has the information that it needs about the different drivers that it has available in the configuration file, but it did not previously have access to the devbind script, so it did not use that information for anything. This patch moves the location of the tmp directory as well as the method for binding ports into the node class rather than the SUT node class and adds an abstract method for getting the path to the devbind script into the node class. Then, binding ports to the correct drivers is moved into the build target setup and run on both nodes. Bugzilla ID: 1420 Signed-off-by: Jeremy Spewock --- Reviewed-by: Juraj Linkeš
[PATCH v8 0/8] support dump register names and filter
The registers can be dumped through the API rte_eth_dev_get_reg_info. However, only register values are exported, which is inconvenient for users to interpret. Therefore, an extension of the structure "rte_dev_reg_info" and a new API rte_eth_dev_get_reg_info_ext is added to support the capability of exporting the name of the corresponding register and filtering by module names. -- v8: 1. fix typo on patch[0/8]. 2. add Reviewed-bys and Acked-bys. 3. fix potentital memory leak and error information on telemetry. 4. fix on reviews about hns3 driver. v7: fix doc indention. -- Jie Hai (8): ethdev: support report register names and filter ethdev: add telemetry cmd for registers net/hns3: remove some basic address dump net/hns3: fix dump counter of registers net/hns3: remove separators between register module net/hns3: refactor register dump net/hns3: support report names of registers net/hns3: support filter registers by module names doc/guides/nics/hns3.rst |7 + doc/guides/rel_notes/release_24_11.rst |8 + drivers/net/hns3/hns3_regs.c | 1384 +++- lib/ethdev/ethdev_trace.h |2 + lib/ethdev/rte_dev_info.h | 11 + lib/ethdev/rte_ethdev.c| 38 + lib/ethdev/rte_ethdev.h| 29 + lib/ethdev/rte_ethdev_telemetry.c | 130 +++ lib/ethdev/version.map |1 + 9 files changed, 1344 insertions(+), 266 deletions(-) -- 2.33.0
[PATCH v8 3/8] net/hns3: remove some basic address dump
For security reasons, some address registers are not suitable
to be exposed, remove them.
Cc: sta...@dpdk.org
Signed-off-by: Jie Hai
Acked-by: Huisong Li
Acked-by: Chengwen Feng
---
drivers/net/hns3/hns3_regs.c | 12 ++--
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/net/hns3/hns3_regs.c b/drivers/net/hns3/hns3_regs.c
index be1be6a89c94..53d829a4fc68 100644
--- a/drivers/net/hns3/hns3_regs.c
+++ b/drivers/net/hns3/hns3_regs.c
@@ -17,13 +17,9 @@
static int hns3_get_dfx_reg_line(struct hns3_hw *hw, uint32_t *lines);
-static const uint32_t cmdq_reg_addrs[] = {HNS3_CMDQ_TX_ADDR_L_REG,
- HNS3_CMDQ_TX_ADDR_H_REG,
- HNS3_CMDQ_TX_DEPTH_REG,
+static const uint32_t cmdq_reg_addrs[] = {HNS3_CMDQ_TX_DEPTH_REG,
HNS3_CMDQ_TX_TAIL_REG,
HNS3_CMDQ_TX_HEAD_REG,
- HNS3_CMDQ_RX_ADDR_L_REG,
- HNS3_CMDQ_RX_ADDR_H_REG,
HNS3_CMDQ_RX_DEPTH_REG,
HNS3_CMDQ_RX_TAIL_REG,
HNS3_CMDQ_RX_HEAD_REG,
@@ -44,9 +40,7 @@ static const uint32_t common_vf_reg_addrs[] =
{HNS3_MISC_VECTOR_REG_BASE,
HNS3_FUN_RST_ING,
HNS3_GRO_EN_REG};
-static const uint32_t ring_reg_addrs[] = {HNS3_RING_RX_BASEADDR_L_REG,
- HNS3_RING_RX_BASEADDR_H_REG,
- HNS3_RING_RX_BD_NUM_REG,
+static const uint32_t ring_reg_addrs[] = {HNS3_RING_RX_BD_NUM_REG,
HNS3_RING_RX_BD_LEN_REG,
HNS3_RING_RX_EN_REG,
HNS3_RING_RX_MERGE_EN_REG,
@@ -57,8 +51,6 @@ static const uint32_t ring_reg_addrs[] =
{HNS3_RING_RX_BASEADDR_L_REG,
HNS3_RING_RX_FBD_OFFSET_REG,
HNS3_RING_RX_STASH_REG,
HNS3_RING_RX_BD_ERR_REG,
- HNS3_RING_TX_BASEADDR_L_REG,
- HNS3_RING_TX_BASEADDR_H_REG,
HNS3_RING_TX_BD_NUM_REG,
HNS3_RING_TX_EN_REG,
HNS3_RING_TX_PRIORITY_REG,
--
2.33.0
[PATCH v8 7/8] net/hns3: support report names of registers
This patch adds names for register lists, and support report
names of registers. Some registers has different names on
different platform, use names of HIP08 as default names.
Signed-off-by: Jie Hai
Acked-by: Chengwen Feng
---
drivers/net/hns3/hns3_regs.c | 1088 +-
1 file changed, 955 insertions(+), 133 deletions(-)
diff --git a/drivers/net/hns3/hns3_regs.c b/drivers/net/hns3/hns3_regs.c
index 89858c2b1c09..a0d130302839 100644
--- a/drivers/net/hns3/hns3_regs.c
+++ b/drivers/net/hns3/hns3_regs.c
@@ -14,73 +14,827 @@
static int hns3_get_dfx_reg_cnt(struct hns3_hw *hw, uint32_t *count);
-static const uint32_t cmdq_reg_addrs[] = {HNS3_CMDQ_TX_DEPTH_REG,
- HNS3_CMDQ_TX_TAIL_REG,
- HNS3_CMDQ_TX_HEAD_REG,
- HNS3_CMDQ_RX_DEPTH_REG,
- HNS3_CMDQ_RX_TAIL_REG,
- HNS3_CMDQ_RX_HEAD_REG,
- HNS3_VECTOR0_CMDQ_SRC_REG,
- HNS3_CMDQ_INTR_STS_REG,
- HNS3_CMDQ_INTR_EN_REG,
- HNS3_CMDQ_INTR_GEN_REG};
-
-static const uint32_t common_reg_addrs[] = {HNS3_MISC_VECTOR_REG_BASE,
- HNS3_VECTOR0_OTER_EN_REG,
- HNS3_MISC_RESET_STS_REG,
- HNS3_VECTOR0_OTHER_INT_STS_REG,
- HNS3_GLOBAL_RESET_REG,
- HNS3_FUN_RST_ING,
- HNS3_GRO_EN_REG};
-
-static const uint32_t common_vf_reg_addrs[] = {HNS3_MISC_VECTOR_REG_BASE,
- HNS3_FUN_RST_ING,
- HNS3_GRO_EN_REG};
-
-static const uint32_t ring_reg_addrs[] = {HNS3_RING_RX_BD_NUM_REG,
- HNS3_RING_RX_BD_LEN_REG,
- HNS3_RING_RX_EN_REG,
- HNS3_RING_RX_MERGE_EN_REG,
- HNS3_RING_RX_TAIL_REG,
- HNS3_RING_RX_HEAD_REG,
- HNS3_RING_RX_FBDNUM_REG,
- HNS3_RING_RX_OFFSET_REG,
- HNS3_RING_RX_FBD_OFFSET_REG,
- HNS3_RING_RX_STASH_REG,
- HNS3_RING_RX_BD_ERR_REG,
- HNS3_RING_TX_BD_NUM_REG,
- HNS3_RING_TX_EN_REG,
- HNS3_RING_TX_PRIORITY_REG,
- HNS3_RING_TX_TC_REG,
- HNS3_RING_TX_MERGE_EN_REG,
- HNS3_RING_TX_TAIL_REG,
- HNS3_RING_TX_HEAD_REG,
- HNS3_RING_TX_FBDNUM_REG,
- HNS3_RING_TX_OFFSET_REG,
- HNS3_RING_TX_EBD_NUM_REG,
- HNS3_RING_TX_EBD_OFFSET_REG,
- HNS3_RING_TX_BD_ERR_REG,
- HNS3_RING_EN_REG};
-
-static const uint32_t tqp_intr_reg_addrs[] = {HNS3_TQP_INTR_CTRL_REG,
- HNS3_TQP_INTR_GL0_REG,
- HNS3_TQP_INTR_GL1_REG,
- HNS3_TQP_INTR_GL2_REG,
- HNS3_TQP_INTR_RL_REG};
+struct hns3_dirt_reg_entry {
+ const char *name;
+ uint32_t addr;
+};
+
+static const struct hns3_dirt_reg_entry cmdq_reg_list[] = {
+ {"cmdq_tx_depth", HNS3_CMDQ_TX_DEPTH_REG},
+ {"cmdq_tx_tail",HNS3_CMDQ_TX_TAIL_REG},
+ {"cmdq_tx_head",HNS3_CMDQ_TX_HEAD_REG},
+ {"cmdq_rx_depth", HNS3_CMDQ_RX_DEPTH_REG},
+ {"cmdq_rx_tail",HNS3_CMDQ_RX_TAIL_REG},
+ {"cmdq_rx_head",HNS3_CMDQ_RX_HEAD_REG},
+ {"vector0_cmdq_src",HNS3_VECTOR0_CMDQ_SRC_REG},
+ {"cmdq_intr_sts", HNS3_CMDQ_INTR_STS_REG},
+ {"cmdq_intr_en",HNS3_CMDQ_INTR_EN_REG},
+ {"cmdq_intr_gen", HNS3_CMDQ_INTR_GEN_REG},
+};
+
+static const struct hns3_dirt_reg_entry common_reg_list[] = {
+ {"misc_vector_reg_base",HNS3_MISC_VECTOR_REG_BASE},
+ {"vector0_oter_en", HNS3_VECTOR0_OTER_EN_REG},
+ {"misc_reset_sts", HNS3_MISC_RESET_STS_REG},
+
[PATCH v8 6/8] net/hns3: refactor register dump
This patch refactors codes dumping registers from firmware.
Signed-off-by: Jie Hai
Acked-by: Chengwen Feng
---
drivers/net/hns3/hns3_regs.c | 203 ---
1 file changed, 115 insertions(+), 88 deletions(-)
diff --git a/drivers/net/hns3/hns3_regs.c b/drivers/net/hns3/hns3_regs.c
index c8e3fb118e4b..89858c2b1c09 100644
--- a/drivers/net/hns3/hns3_regs.c
+++ b/drivers/net/hns3/hns3_regs.c
@@ -104,12 +104,93 @@ hns3_get_regs_num(struct hns3_hw *hw, uint32_t
*regs_num_32_bit,
return 0;
}
+static int
+hns3_get_32_64_regs_cnt(struct hns3_hw *hw, uint32_t *count)
+{
+ uint32_t regs_num_32_bit, regs_num_64_bit;
+ int ret;
+
+ ret = hns3_get_regs_num(hw, ®s_num_32_bit, ®s_num_64_bit);
+ if (ret) {
+ hns3_err(hw, "fail to get the number of registers, "
+"ret = %d.", ret);
+ return ret;
+ }
+
+ *count += regs_num_32_bit + regs_num_64_bit *
HNS3_64_BIT_REG_OUTPUT_SIZE;
+ return 0;
+}
+
+static int
+hns3_get_dfx_reg_bd_num(struct hns3_hw *hw, uint32_t *bd_num_list,
+ uint32_t list_size)
+{
+#define HNS3_GET_DFX_REG_BD_NUM_SIZE 4
+ struct hns3_cmd_desc desc[HNS3_GET_DFX_REG_BD_NUM_SIZE];
+ uint32_t index, desc_index;
+ uint32_t bd_num;
+ uint32_t i;
+ int ret;
+
+ for (i = 0; i < HNS3_GET_DFX_REG_BD_NUM_SIZE - 1; i++) {
+ hns3_cmd_setup_basic_desc(&desc[i], HNS3_OPC_DFX_BD_NUM, true);
+ desc[i].flag |= rte_cpu_to_le_16(HNS3_CMD_FLAG_NEXT);
+ }
+ /* The last BD does not need a next flag */
+ hns3_cmd_setup_basic_desc(&desc[i], HNS3_OPC_DFX_BD_NUM, true);
+
+ ret = hns3_cmd_send(hw, desc, HNS3_GET_DFX_REG_BD_NUM_SIZE);
+ if (ret) {
+ hns3_err(hw, "fail to get dfx bd num, ret = %d.\n", ret);
+ return ret;
+ }
+
+ /* The first data in the first BD is a reserved field */
+ for (i = 1; i <= list_size; i++) {
+ desc_index = i / HNS3_CMD_DESC_DATA_NUM;
+ index = i % HNS3_CMD_DESC_DATA_NUM;
+ bd_num = rte_le_to_cpu_32(desc[desc_index].data[index]);
+ bd_num_list[i - 1] = bd_num;
+ }
+
+ return 0;
+}
+
+static int
+hns3_get_dfx_reg_cnt(struct hns3_hw *hw, uint32_t *count)
+{
+ int opcode_num = RTE_DIM(hns3_dfx_reg_opcode_list);
+ uint32_t bd_num_list[opcode_num];
+ int ret;
+ int i;
+
+ ret = hns3_get_dfx_reg_bd_num(hw, bd_num_list, opcode_num);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < opcode_num; i++)
+ *count += bd_num_list[i] * HNS3_CMD_DESC_DATA_NUM;
+
+ return 0;
+}
+
+static int
+hns3_get_firmware_reg_cnt(struct hns3_hw *hw, uint32_t *count)
+{
+ int ret;
+
+ ret = hns3_get_32_64_regs_cnt(hw, count);
+ if (ret < 0)
+ return ret;
+
+ return hns3_get_dfx_reg_cnt(hw, count);
+}
+
static int
hns3_get_regs_length(struct hns3_hw *hw, uint32_t *length)
{
struct hns3_adapter *hns = HNS3_DEV_HW_TO_ADAPTER(hw);
- uint32_t regs_num_32_bit, regs_num_64_bit;
- uint32_t dfx_reg_cnt;
+ uint32_t dfx_reg_cnt = 0;
uint32_t common_cnt;
uint32_t len;
int ret;
@@ -125,16 +206,7 @@ hns3_get_regs_length(struct hns3_hw *hw, uint32_t *length)
len /= sizeof(uint32_t);
if (!hns->is_vf) {
- ret = hns3_get_regs_num(hw, ®s_num_32_bit, ®s_num_64_bit);
- if (ret) {
- hns3_err(hw, "fail to get the number of registers, "
-"ret = %d.", ret);
- return ret;
- }
- dfx_reg_cnt = regs_num_32_bit +
- regs_num_64_bit * HNS3_64_BIT_REG_OUTPUT_SIZE;
-
- ret = hns3_get_dfx_reg_cnt(hw, &dfx_reg_cnt);
+ ret = hns3_get_firmware_reg_cnt(hw, &dfx_reg_cnt);
if (ret) {
hns3_err(hw, "fail to get the number of dfx registers, "
"ret = %d.", ret);
@@ -304,41 +376,6 @@ hns3_direct_access_regs(struct hns3_hw *hw, uint32_t *data)
return data - origin_data_ptr;
}
-static int
-hns3_get_dfx_reg_bd_num(struct hns3_hw *hw, uint32_t *bd_num_list,
- uint32_t list_size)
-{
-#define HNS3_GET_DFX_REG_BD_NUM_SIZE 4
- struct hns3_cmd_desc desc[HNS3_GET_DFX_REG_BD_NUM_SIZE];
- uint32_t index, desc_index;
- uint32_t bd_num;
- uint32_t i;
- int ret;
-
- for (i = 0; i < HNS3_GET_DFX_REG_BD_NUM_SIZE - 1; i++) {
- hns3_cmd_setup_basic_desc(&desc[i], HNS3_OPC_DFX_BD_NUM, true);
- desc[i].flag |= rte_cpu_to_le_16(HNS3_CMD_FLAG_NEXT);
- }
- /* The last BD does not need a next flag */
- hns3_cmd_setup_basic_desc(&desc[i], HNS3_OPC_DFX_BD_NUM, true);
-
- ret = hns3_cmd_send(hw,
[PATCH v8 8/8] net/hns3: support filter registers by module names
This patch support dumping registers which module name is the
`filter` string. The module names are in lower case and so is
the `filter`. Available module names are cmdq, common_pf,
common_vf, ring, tqp_intr, 32_bit_dfx, 64_bit_dfx, bios, igu_egu,
ssu, ppp, rpu, ncsi, rtc, rcb, etc.
Signed-off-by: Jie Hai
---
doc/guides/nics/hns3.rst | 7 +
drivers/net/hns3/hns3_regs.c | 257 ---
2 files changed, 157 insertions(+), 107 deletions(-)
diff --git a/doc/guides/nics/hns3.rst b/doc/guides/nics/hns3.rst
index 97b4686bfabe..20765bd7d145 100644
--- a/doc/guides/nics/hns3.rst
+++ b/doc/guides/nics/hns3.rst
@@ -407,6 +407,13 @@ be provided to avoid scheduling the CPU core used by DPDK
application threads fo
other tasks. Before starting the Linux OS, add the kernel isolation boot
parameter.
For example, "isolcpus=1-18 nohz_full=1-18 rcu_nocbs=1-18".
+Dump registers
+--
+HNS3 supports dumping registers values with their names, and suppotrt filtering
+by module names. The available modules are ``bios``, ``ssu``, ``igu_egu``,
+``rpu``, ``ncsi``, ``rtc``, ``ppp``, ``rcb``, ``tqp``, ``rtc``, ``cmdq``,
+``common_pf``, ``common_vf``, ``ring``, ``tqp_intr``, ``32_bit_dfx``,
+``64_bit_dfx``, etc.
Limitations or Known issues
---
diff --git a/drivers/net/hns3/hns3_regs.c b/drivers/net/hns3/hns3_regs.c
index a0d130302839..2e19add21265 100644
--- a/drivers/net/hns3/hns3_regs.c
+++ b/drivers/net/hns3/hns3_regs.c
@@ -12,7 +12,7 @@
#define HNS3_64_BIT_REG_OUTPUT_SIZE (sizeof(uint64_t) / sizeof(uint32_t))
-static int hns3_get_dfx_reg_cnt(struct hns3_hw *hw, uint32_t *count);
+#define HNS3_MAX_MODULES_LEN 512
struct hns3_dirt_reg_entry {
const char *name;
@@ -795,11 +795,39 @@ enum hns3_reg_modules {
HNS3_64_BIT_DFX,
};
+#define HNS3_MODULE_MASK(x) RTE_BIT32(x)
+#define HNS3_VF_MODULES (HNS3_MODULE_MASK(HNS3_CMDQ) |
HNS3_MODULE_MASK(HNS3_COMMON_VF) | \
+HNS3_MODULE_MASK(HNS3_RING) |
HNS3_MODULE_MASK(HNS3_TQP_INTR))
+#define HNS3_VF_ONLY_MODULES HNS3_MODULE_MASK(HNS3_COMMON_VF)
+
struct hns3_reg_list {
const void *reg_list;
uint32_t entry_num;
};
+struct {
+ const char *name;
+ uint32_t module;
+} hns3_module_name_map[] = {
+ { "bios", HNS3_MODULE_MASK(HNS3_BIOS_COMMON) },
+ { "ssu",HNS3_MODULE_MASK(HNS3_SSU_0) |
HNS3_MODULE_MASK(HNS3_SSU_1) |
+ HNS3_MODULE_MASK(HNS3_SSU_2) },
+ { "igu_egu",HNS3_MODULE_MASK(HNS3_IGU_EGU) },
+ { "rpu",HNS3_MODULE_MASK(HNS3_RPU_0) |
HNS3_MODULE_MASK(HNS3_RPU_1) },
+ { "ncsi", HNS3_MODULE_MASK(HNS3_NCSI) },
+ { "rtc",HNS3_MODULE_MASK(HNS3_RTC) },
+ { "ppp",HNS3_MODULE_MASK(HNS3_PPP) },
+ { "rcb",HNS3_MODULE_MASK(HNS3_RCB) },
+ { "tqp",HNS3_MODULE_MASK(HNS3_TQP) },
+ { "cmdq", HNS3_MODULE_MASK(HNS3_CMDQ) },
+ { "common_pf", HNS3_MODULE_MASK(HNS3_COMMON_PF) },
+ { "common_vf", HNS3_MODULE_MASK(HNS3_COMMON_VF) },
+ { "ring", HNS3_MODULE_MASK(HNS3_RING) },
+ { "tqp_intr", HNS3_MODULE_MASK(HNS3_TQP_INTR) },
+ { "32_bit_dfx", HNS3_MODULE_MASK(HNS3_32_BIT_DFX) },
+ { "64_bit_dfx", HNS3_MODULE_MASK(HNS3_64_BIT_DFX) },
+};
+
static struct hns3_reg_list hns3_reg_lists[] = {
[HNS3_BIOS_COMMON] = { dfx_bios_common_reg_list,
RTE_DIM(dfx_bios_common_reg_list) },
[HNS3_SSU_0]= { dfx_ssu_reg_0_list,
RTE_DIM(dfx_ssu_reg_0_list) },
@@ -863,21 +891,58 @@ hns3_get_regs_num(struct hns3_hw *hw, uint32_t
*regs_num_32_bit,
return 0;
}
-static int
-hns3_get_32_64_regs_cnt(struct hns3_hw *hw, uint32_t *count)
+static const char *
+hns3_get_name_by_module(enum hns3_reg_modules module)
{
- uint32_t regs_num_32_bit, regs_num_64_bit;
- int ret;
+ size_t i;
- ret = hns3_get_regs_num(hw, ®s_num_32_bit, ®s_num_64_bit);
- if (ret) {
- hns3_err(hw, "fail to get the number of registers, "
-"ret = %d.", ret);
- return ret;
+ for (i = 0; i < RTE_DIM(hns3_module_name_map); i++) {
+ if (hns3_module_name_map[i].module && HNS3_MODULE_MASK(module)
!= 0)
+ return hns3_module_name_map[i].name;
}
+ return "unknown";
+}
- *count += regs_num_32_bit + regs_num_64_bit *
HNS3_64_BIT_REG_OUTPUT_SIZE;
- return 0;
+static void
+hns3_get_module_names(char *names, uint32_t len)
+{
+ size_t i;
+
+ for (i = 0; i < RTE_DIM(hns3_module_name_map); i++) {
+ strlcat(names, " ", len);
+ strlcat(names, hns3_module_name_map[i].name, len);
+ }
+}
+
+static uint32_t
+hns3_parse_modules_by_filter(struct hns3_hw *hw, const char *filter)
+{
+ struct hns3_adapter *hns = HNS3_DEV_HW_TO_ADAPTER(hw);
+ char names[HNS3_MAX_MODULES_LEN] = {0};
+
[PATCH v8 5/8] net/hns3: remove separators between register module
Since the driver is going to support reporting names of
all registers, remove the counter and insert of separators
between different register modules.
Signed-off-by: Jie Hai
Reviewed-by: Huisong Li
Acked-by: Chengwen Feng
---
drivers/net/hns3/hns3_regs.c | 68 ++--
1 file changed, 18 insertions(+), 50 deletions(-)
diff --git a/drivers/net/hns3/hns3_regs.c b/drivers/net/hns3/hns3_regs.c
index d9c546470dbe..c8e3fb118e4b 100644
--- a/drivers/net/hns3/hns3_regs.c
+++ b/drivers/net/hns3/hns3_regs.c
@@ -10,12 +10,9 @@
#include "hns3_rxtx.h"
#include "hns3_regs.h"
-#define MAX_SEPARATE_NUM 4
-#define SEPARATOR_VALUE0x
-#define REG_NUM_PER_LINE 4
-#define REG_LEN_PER_LINE (REG_NUM_PER_LINE * sizeof(uint32_t))
+#define HNS3_64_BIT_REG_OUTPUT_SIZE (sizeof(uint64_t) / sizeof(uint32_t))
-static int hns3_get_dfx_reg_line(struct hns3_hw *hw, uint32_t *lines);
+static int hns3_get_dfx_reg_cnt(struct hns3_hw *hw, uint32_t *count);
static const uint32_t cmdq_reg_addrs[] = {HNS3_CMDQ_TX_DEPTH_REG,
HNS3_CMDQ_TX_TAIL_REG,
@@ -111,23 +108,21 @@ static int
hns3_get_regs_length(struct hns3_hw *hw, uint32_t *length)
{
struct hns3_adapter *hns = HNS3_DEV_HW_TO_ADAPTER(hw);
- uint32_t cmdq_lines, common_lines, ring_lines, tqp_intr_lines;
uint32_t regs_num_32_bit, regs_num_64_bit;
- uint32_t dfx_reg_lines;
+ uint32_t dfx_reg_cnt;
+ uint32_t common_cnt;
uint32_t len;
int ret;
- cmdq_lines = sizeof(cmdq_reg_addrs) / REG_LEN_PER_LINE + 1;
if (hns->is_vf)
- common_lines =
- sizeof(common_vf_reg_addrs) / REG_LEN_PER_LINE + 1;
+ common_cnt = sizeof(common_vf_reg_addrs);
else
- common_lines = sizeof(common_reg_addrs) / REG_LEN_PER_LINE + 1;
- ring_lines = sizeof(ring_reg_addrs) / REG_LEN_PER_LINE + 1;
- tqp_intr_lines = sizeof(tqp_intr_reg_addrs) / REG_LEN_PER_LINE + 1;
+ common_cnt = sizeof(common_reg_addrs);
- len = (cmdq_lines + common_lines + ring_lines * hw->tqps_num +
- tqp_intr_lines * hw->intr_tqps_num) * REG_NUM_PER_LINE;
+ len = sizeof(cmdq_reg_addrs) + common_cnt +
+ sizeof(ring_reg_addrs) * hw->tqps_num +
+ sizeof(tqp_intr_reg_addrs) * hw->intr_tqps_num;
+ len /= sizeof(uint32_t);
if (!hns->is_vf) {
ret = hns3_get_regs_num(hw, ®s_num_32_bit, ®s_num_64_bit);
@@ -136,18 +131,16 @@ hns3_get_regs_length(struct hns3_hw *hw, uint32_t *length)
"ret = %d.", ret);
return ret;
}
- dfx_reg_lines = regs_num_32_bit * sizeof(uint32_t) /
- REG_LEN_PER_LINE + 1;
- dfx_reg_lines += regs_num_64_bit * sizeof(uint64_t) /
- REG_LEN_PER_LINE + 1;
+ dfx_reg_cnt = regs_num_32_bit +
+ regs_num_64_bit * HNS3_64_BIT_REG_OUTPUT_SIZE;
- ret = hns3_get_dfx_reg_line(hw, &dfx_reg_lines);
+ ret = hns3_get_dfx_reg_cnt(hw, &dfx_reg_cnt);
if (ret) {
hns3_err(hw, "fail to get the number of dfx registers, "
"ret = %d.", ret);
return ret;
}
- len += dfx_reg_lines * REG_NUM_PER_LINE;
+ len += dfx_reg_cnt;
}
*length = len;
@@ -268,18 +261,6 @@ hns3_get_64_bit_regs(struct hns3_hw *hw, uint32_t
regs_num, void *data)
return 0;
}
-static int
-hns3_insert_reg_separator(int reg_num, uint32_t *data)
-{
- int separator_num;
- int i;
-
- separator_num = MAX_SEPARATE_NUM - reg_num % REG_NUM_PER_LINE;
- for (i = 0; i < separator_num; i++)
- *data++ = SEPARATOR_VALUE;
- return separator_num;
-}
-
static int
hns3_direct_access_regs(struct hns3_hw *hw, uint32_t *data)
{
@@ -294,7 +275,6 @@ hns3_direct_access_regs(struct hns3_hw *hw, uint32_t *data)
reg_num = sizeof(cmdq_reg_addrs) / sizeof(uint32_t);
for (i = 0; i < reg_num; i++)
*data++ = hns3_read_dev(hw, cmdq_reg_addrs[i]);
- data += hns3_insert_reg_separator(reg_num, data);
if (hns->is_vf)
reg_num = sizeof(common_vf_reg_addrs) / sizeof(uint32_t);
@@ -305,7 +285,6 @@ hns3_direct_access_regs(struct hns3_hw *hw, uint32_t *data)
*data++ = hns3_read_dev(hw, common_vf_reg_addrs[i]);
else
*data++ = hns3_read_dev(hw, common_reg_addrs[i]);
- data += hns3_insert_reg_separator(reg_num, data);
reg_num = sizeof(ring_reg_addrs) / sizeof(uint32_t);
for (j = 0; j < hw->tqps_num; j++) {
@@ -313,7 +292,6 @@ hns3_direct_access_regs(struct hns3_hw *hw, uint32
[PATCH v8 2/8] ethdev: add telemetry cmd for registers
This patch adds a telemetry command for registers dump,
and supports obtaining the registers of a specified module.
In one way, the number of registers that can be exported
is limited by the number of elements carried by dict and
container. In another way, the length of the string
exported by telemetry is limited by MAX_OUTPUT_LEN.
Therefore, when the number of registers to be exported
exceeds, some information will be lost. Warn on the former
case.
An example usage is shown below:
--> /ethdev/regs,0,ring
{
"/ethdev/regs": {
"registers_length": 318,
"registers_width": 4,
"register_offset": "0x0",
"version": "0x1140011",
"group_0": {
"Q0_ring_rx_bd_num": "0x0",
"Q0_ring_rx_bd_len": "0x0",
...
},
"group_1": {
...
},
...
}
Signed-off-by: Jie Hai
Reviewed-by: Ferruh Yigit
---
lib/ethdev/rte_ethdev_telemetry.c | 130 ++
1 file changed, 130 insertions(+)
diff --git a/lib/ethdev/rte_ethdev_telemetry.c
b/lib/ethdev/rte_ethdev_telemetry.c
index 6b873e7abe68..181390691099 100644
--- a/lib/ethdev/rte_ethdev_telemetry.c
+++ b/lib/ethdev/rte_ethdev_telemetry.c
@@ -1395,6 +1395,134 @@ eth_dev_handle_port_tm_node_caps(const char *cmd
__rte_unused,
return ret;
}
+static void
+eth_dev_add_reg_data(struct rte_tel_data *d, struct rte_dev_reg_info *reg_info,
+uint32_t idx)
+{
+ if (reg_info->width == sizeof(uint32_t))
+ rte_tel_data_add_dict_uint_hex(d, reg_info->names[idx].name,
+ *((uint32_t *)reg_info->data + idx), 0);
+ else
+ rte_tel_data_add_dict_uint_hex(d, reg_info->names[idx].name,
+ *((uint64_t *)reg_info->data + idx), 0);
+}
+
+static int
+eth_dev_store_regs(struct rte_tel_data *d, struct rte_dev_reg_info *reg_info)
+{
+ struct rte_tel_data *groups[RTE_TEL_MAX_DICT_ENTRIES];
+ char group_name[RTE_TEL_MAX_STRING_LEN] = {0};
+ struct rte_tel_data *group = NULL;
+ uint32_t grp_num = 0;
+ uint32_t i, max_cap;
+ int ret;
+
+ rte_tel_data_start_dict(d);
+ rte_tel_data_add_dict_uint(d, "register_length", reg_info->length);
+ rte_tel_data_add_dict_uint(d, "register_width", reg_info->width);
+ rte_tel_data_add_dict_uint_hex(d, "register_offset", reg_info->offset,
0);
+ rte_tel_data_add_dict_uint_hex(d, "version", reg_info->version, 0);
+
+ max_cap = (RTE_TEL_MAX_DICT_ENTRIES - 4) * RTE_TEL_MAX_DICT_ENTRIES;
+ if (reg_info->length > max_cap) {
+ RTE_ETHDEV_LOG_LINE(WARNING,
+ "Registers to be displayed are reduced from %u to %u
due to limited capacity",
+ reg_info->length, max_cap);
+ reg_info->length = max_cap;
+ }
+
+ for (i = 0; i < reg_info->length; i++) {
+ if (i % RTE_TEL_MAX_DICT_ENTRIES != 0) {
+ eth_dev_add_reg_data(group, reg_info, i);
+ continue;
+ }
+
+ group = rte_tel_data_alloc();
+ if (group == NULL) {
+ ret = -ENOMEM;
+ RTE_ETHDEV_LOG_LINE(WARNING, "No enough memory for
group data");
+ goto out;
+ }
+ groups[grp_num++] = group;
+ rte_tel_data_start_dict(group);
+ eth_dev_add_reg_data(group, reg_info, i);
+ }
+
+ for (i = 0; i < grp_num; i++) {
+ snprintf(group_name, RTE_TEL_MAX_STRING_LEN, "group_%u", i);
+ rte_tel_data_add_dict_container(d, group_name, groups[i], 0);
+ }
+ return 0;
+out:
+ for (i = 0; i < grp_num; i++)
+ rte_tel_data_free(groups[i]);
+
+ return ret;
+}
+
+static int
+eth_dev_get_port_regs(int port_id, struct rte_tel_data *d, char *filter)
+{
+ struct rte_dev_reg_info reg_info;
+ int ret;
+
+ memset(®_info, 0, sizeof(reg_info));
+ reg_info.filter = filter;
+
+ ret = rte_eth_dev_get_reg_info_ext(port_id, ®_info);
+ if (ret != 0) {
+ RTE_ETHDEV_LOG_LINE(ERR, "Failed to get device reg info: %d",
ret);
+ return ret;
+ }
+
+ reg_info.data = calloc(reg_info.length, reg_info.width);
+ if (reg_info.data == NULL) {
+ RTE_ETHDEV_LOG_LINE(ERR, "Failed to allocate memory for
reg_info.data");
+ return -ENOMEM;
+ }
+
+ reg_info.names = calloc(reg_info.length, sizeof(struct
rte_eth_reg_name));
+ if (reg_info.names == NULL) {
+ RTE_ETHDEV_LOG_LINE(ERR, "Failed to allocate memory for
reg_info.names");
+ free(reg_info.data);
+ return -ENOMEM;
+ }
+
+ ret = rte_eth_dev_get_reg_info_ext(port_id, ®_info);
+ if (ret != 0) {
+ RTE_ETHDEV_LOG_LINE(ERR, "Failed to get device reg info: %d",
ret);
+ ret = -EINVAL;
+ g
[PATCH v8 4/8] net/hns3: fix dump counter of registers
Since the driver dumps the queue interrupt registers according
to the intr_tqps_num, the counter should be the same.
Fixes: acb3260fac5c ("net/hns3: fix dump register out of range")
Fixes: 936eda25e8da ("net/hns3: support dump register")
Cc: sta...@dpdk.org
Signed-off-by: Jie Hai
Acked-by: Huisong Li
Acked-by: Chengwen Feng
---
drivers/net/hns3/hns3_regs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/hns3/hns3_regs.c b/drivers/net/hns3/hns3_regs.c
index 53d829a4fc68..d9c546470dbe 100644
--- a/drivers/net/hns3/hns3_regs.c
+++ b/drivers/net/hns3/hns3_regs.c
@@ -127,7 +127,7 @@ hns3_get_regs_length(struct hns3_hw *hw, uint32_t *length)
tqp_intr_lines = sizeof(tqp_intr_reg_addrs) / REG_LEN_PER_LINE + 1;
len = (cmdq_lines + common_lines + ring_lines * hw->tqps_num +
- tqp_intr_lines * hw->num_msi) * REG_NUM_PER_LINE;
+ tqp_intr_lines * hw->intr_tqps_num) * REG_NUM_PER_LINE;
if (!hns->is_vf) {
ret = hns3_get_regs_num(hw, ®s_num_32_bit, ®s_num_64_bit);
--
2.33.0
[PATCH v8 1/8] ethdev: support report register names and filter
This patch adds "filter" and "names" fields to "rte_dev_reg_info"
structure. Names of registers in data fields can be reported and
the registers can be filtered by their module names.
The new API rte_eth_dev_get_reg_info_ext() is added to support
reporting names and filtering by modules. And the original API
rte_eth_dev_get_reg_info() does not use the names and filter fields.
A local variable is used in rte_eth_dev_get_reg_info for
compatibility. If the drivers does not report the names, set them
to "index_XXX", which means the location in the register table.
Signed-off-by: Jie Hai
Acked-by: Huisong Li
Acked-by: Chengwen Feng
Reviewed-by: Ferruh Yigit
---
doc/guides/rel_notes/release_24_11.rst | 8 ++
lib/ethdev/ethdev_trace.h | 2 ++
lib/ethdev/rte_dev_info.h | 11
lib/ethdev/rte_ethdev.c| 38 ++
lib/ethdev/rte_ethdev.h| 29
lib/ethdev/version.map | 1 +
6 files changed, 89 insertions(+)
diff --git a/doc/guides/rel_notes/release_24_11.rst
b/doc/guides/rel_notes/release_24_11.rst
index 5dd575fb39df..85960e71d0ca 100644
--- a/doc/guides/rel_notes/release_24_11.rst
+++ b/doc/guides/rel_notes/release_24_11.rst
@@ -60,6 +60,11 @@ New Features
* Added SR-IOV VF support.
* Added recent 1400/14000 and 15000 models to the supported list.
+* **Added support for dumping registers with names and filtering by modules.**
+
+ Added new API functions ``rte_eth_dev_get_reg_info_ext()`` to filter the
+ registers by module names and get the information (names, values and other
+ attributes) of the filtered registers.
Removed Items
-
@@ -108,6 +113,9 @@ ABI Changes
Also, make sure to start the actual text at the margin.
===
+* ethdev: Added ``filter`` and ``names`` fields to ``rte_dev_reg_info``
+ structure for filtering by modules and reporting names of registers.
+
Known Issues
diff --git a/lib/ethdev/ethdev_trace.h b/lib/ethdev/ethdev_trace.h
index 3bec87bfdb70..0c4780a09ef5 100644
--- a/lib/ethdev/ethdev_trace.h
+++ b/lib/ethdev/ethdev_trace.h
@@ -1152,6 +1152,8 @@ RTE_TRACE_POINT(
rte_trace_point_emit_u32(info->length);
rte_trace_point_emit_u32(info->width);
rte_trace_point_emit_u32(info->version);
+ rte_trace_point_emit_ptr(info->names);
+ rte_trace_point_emit_ptr(info->filter);
rte_trace_point_emit_int(ret);
)
diff --git a/lib/ethdev/rte_dev_info.h b/lib/ethdev/rte_dev_info.h
index 67cf0ae52668..26b777f9836e 100644
--- a/lib/ethdev/rte_dev_info.h
+++ b/lib/ethdev/rte_dev_info.h
@@ -11,6 +11,11 @@ extern "C" {
#include
+#define RTE_ETH_REG_NAME_SIZE 64
+struct rte_eth_reg_name {
+ char name[RTE_ETH_REG_NAME_SIZE];
+};
+
/*
* Placeholder for accessing device registers
*/
@@ -20,6 +25,12 @@ struct rte_dev_reg_info {
uint32_t length; /**< Number of registers to fetch */
uint32_t width; /**< Size of device register */
uint32_t version; /**< Device version */
+ /**
+* Name of target module, filter for target subset of registers.
+* This field could affects register selection for data/length/names.
+*/
+ const char *filter;
+ struct rte_eth_reg_name *names; /**< Registers name saver */
};
/*
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index f1c658f49e80..30ca4a0043c5 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -6388,8 +6388,37 @@ rte_eth_read_clock(uint16_t port_id, uint64_t *clock)
int
rte_eth_dev_get_reg_info(uint16_t port_id, struct rte_dev_reg_info *info)
+{
+ struct rte_dev_reg_info reg_info = { 0 };
+ int ret;
+
+ if (info == NULL) {
+ RTE_ETHDEV_LOG_LINE(ERR,
+ "Cannot get ethdev port %u register info to NULL",
+ port_id);
+ return -EINVAL;
+ }
+
+ reg_info.length = info->length;
+ reg_info.data = info->data;
+
+ ret = rte_eth_dev_get_reg_info_ext(port_id, ®_info);
+ if (ret != 0)
+ return ret;
+
+ info->length = reg_info.length;
+ info->width = reg_info.width;
+ info->version = reg_info.version;
+ info->offset = reg_info.offset;
+
+ return 0;
+}
+
+int
+rte_eth_dev_get_reg_info_ext(uint16_t port_id, struct rte_dev_reg_info *info)
{
struct rte_eth_dev *dev;
+ uint32_t i;
int ret;
RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
@@ -6402,12 +6431,21 @@ rte_eth_dev_get_reg_info(uint16_t port_id, struct
rte_dev_reg_info *info)
return -EINVAL;
}
+ if (info->names != NULL && info->length != 0)
+ memset(info->names, 0, sizeof(struct rte_eth_reg_name) *
info->length);
+
if (*dev->dev_ops->get_reg == NULL)
return -ENOTSUP;
ret
Re: [PATCH v3 1/2] dts: add symbolic link to dpdk-devbind script
On 24. 9. 2024 18:28, jspew...@iol.unh.edu wrote: From: Jeremy Spewock The devbind script is used throughout DTS to manage drivers on the remote hosts. Currently, the only way to copy this script onto a host is to either copy the entire DPDK directory onto a host, or reach out of the dts directory into its parent DPDK directory to access the script. The first is undesirable if the host doesn't require any other DPDK tools since you would be copying extra unneeded information and the second is undesirable since it enforces the assumption that DTS is being run from within the DPDK directory. To solve this issue a symbolic link is added which links the devbind script from the parent into the DTS directory. Since this file is not part of DTS and therefore is not expected to follow DTS formatting rules, it is excluded from the DTS formatting script. Signed-off-by: Jeremy Spewock --- Reviewed-by: Juraj Linkeš
Re: [PATCH v2 1/1] dts: add binding to different drivers to TG node
I have some thoughts for the future:
1a. The traffic generator is specified per-node, so maybe we could also
change the binding to be for the whole lifetime of the TG node,
1b. But the same is true for the SUT node as well, right? After we do
the port update (with kernel driver), we can just bind to DPDK driver.
With SUT in the mix, this looks like a change for a different patch,
2. We could add a symlink to the devbind script with the target being in
the dts directory. This way we don't have to go outside the dts
directory and if DTS ever become a python package, we could just copy
the script to the appropriate place. This is also something we don't
really need to do.
And also two minor comments. A lot of suggestions for separate patches
overall. :-)
On 19. 9. 2024 20:16, jspew...@iol.unh.edu wrote:
From: Jeremy Spewock
The DTS framework in its current state supports binding ports to
different drivers on the SUT node but not the TG node. The TG node
already has the information that it needs about the different drivers
that it has available in the configuration file, but it did not
previously have access to the devbind script, so it did not use that
information for anything.
This patch moves the location of the tmp directory as well as the method
for binding ports into the node class rather than the SUT node class and
adds an abstract method for getting the path to the devbind script into
the node class. Then, binding ports to the correct drivers is moved into
the build target setup and run on both nodes.
Bugzilla ID: 1420
Signed-off-by: Jeremy Spewock
---
With the two minor comments,
Reviewed-by: Juraj Linkeš
diff --git a/dts/framework/testbed_model/node.py
b/dts/framework/testbed_model/node.py
@@ -58,8 +65,10 @@ class Node(ABC):
lcores: list[LogicalCore]
ports: list[Port]
_logger: DTSLogger
+_remote_tmp_dir: PurePath
_other_sessions: list[OSSession]
_test_run_config: TestRunConfiguration
+_path_to_devbind_script: PurePath | None
A note on the naming. We have _remote_tmp_dir and
_path_to_devbind_script. Both are pointing to a remote file/dir, but
only one has the _remote prefix. They should probably be unified.
I've thought a bit about what the right name is. Dropping the prefix
makes sense; sut_node.tmp_dir should mean the tmp dir on the SUT node
(which would make it remote from the execution host's point of view, but
not the node's view; the file is local to SUT node). This could be a
good separate patch (improving the remote/local naming scheme to make it
consistent and as sensible as possible).
diff --git a/dts/framework/utils.py b/dts/framework/utils.py
@@ -29,6 +29,8 @@
from .exception import ConfigurationError, InternalError
REGEX_FOR_PCI_ADDRESS: str = "/[0-9a-fA-F]{4}:[0-9a-fA-F]{2}:[0-9a-fA-F]{2}.[0-9]{1}/"
+#: Path to DPDK directory on the host where DTS is being run.
+LOCAL_DPDK_DIR: PurePath = PurePath(__file__).parents[2]
Local paths can be just pathlib.Path. PurePaths are for path
manipulations only (useful for remote paths in RemoteSessions,
OSSessions and Nodes), but for local existing paths we should use Path.
The OSSession and subclasses need a bit of an update in this regard -
use Path for local paths and PurePaths for remote ones. We added this
into our pre-built DPDK patch.
def expand_range(range_str: str) -> list[int]:
[PATCH v4 00/11] dts: add test skipping based on capabilities
Add an automated way to gather available capabilities of the tested hardware and skip test suites or cases which require capabilities that are not available. This is done through two decorators: 1. The first marks a test suite method as test case. This populates the default attributes of each test case. 2. The seconds adds the required capabilities to a test suite or case, using the attributes from 1). Two types of capabilities are added: 1. NIC capabilities. These are gathered once DPDK has been built because we use testpmd for this. It's possible to add a function that will add configuration before assessing capabilities associated with the function. This is because some capabilities return different status with different configuration present. 2. The topology capability. Each test case is marked as requiring a default topology. The required topology of a test case (or a whole test suite) may be change with the second decorator. This is how it all works: 1. The required capabilities are first all gathered from all test suites and test cases. 2. The list of required capabilities is divided into supported and unsupported capabilities. In this step, the probing of hardware and/or anything else that needs to happen to gauge whether a capability is supported is done. 3. Each test suite and test case is then marked to be skipped if any of their required capabilities are not supported. v4: Rebased on next-dts. Adjusted the decorator for setting and reverting 9000 MTU to be parametrized. Added references to DPDK libraries and testpmd code to testpmd command parsing methods. Updated capability docstrings. Moved the capability decorator to NicCapability. Juraj Linkeš (11): dts: add the aenum dependency dts: add test case decorators dts: add mechanism to skip test cases or suites dts: add support for simpler topologies dts: add basic capability support dts: add NIC capability support dts: add NIC capabilities from show rxq info dts: add topology capability doc: add DTS capability doc sources dts: add Rx offload capabilities dts: add NIC capabilities from show port info .../framework.testbed_model.capability.rst| 6 + doc/api/dts/framework.testbed_model.rst | 2 + .../dts/framework.testbed_model.topology.rst | 6 + dts/framework/remote_session/testpmd_shell.py | 461 +++- dts/framework/runner.py | 155 +++--- dts/framework/test_result.py | 119 ++-- dts/framework/test_suite.py | 161 +- dts/framework/testbed_model/capability.py | 507 ++ dts/framework/testbed_model/node.py | 2 +- dts/framework/testbed_model/port.py | 4 +- dts/framework/testbed_model/topology.py | 127 + dts/poetry.lock | 14 +- dts/pyproject.toml| 1 + dts/tests/TestSuite_hello_world.py| 10 +- dts/tests/TestSuite_os_udp.py | 3 +- dts/tests/TestSuite_pmd_buffer_scatter.py | 14 +- dts/tests/TestSuite_smoke_tests.py| 8 +- 17 files changed, 1447 insertions(+), 153 deletions(-) create mode 100644 doc/api/dts/framework.testbed_model.capability.rst create mode 100644 doc/api/dts/framework.testbed_model.topology.rst create mode 100644 dts/framework/testbed_model/capability.py create mode 100644 dts/framework/testbed_model/topology.py -- 2.43.0
