[DPDK/vhost/virtio Bug 1603] net/vhost: some devargs are not documented

[bugzilla]

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

Bug ID: 1603
   Summary: net/vhost: some devargs are not documented
   Product: DPDK
   Version: unspecified
  Hardware: All
OS: All
Status: UNCONFIRMED
  Severity: normal
  Priority: Normal
 Component: vhost/virtio
  Assignee: dev@dpdk.org
  Reporter: david.march...@redhat.com
  Target Milestone: ---

This was reported internally at RH.

https://doc.dpdk.org/guides/nics/vhost.html does not provide a description of
all available devargs for the net/vhost pmd.
Especially, the client= option is something that is needed by users regularly.

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

Re: 22.11.7 patches review and test

[David Marchand]

Hello LTS maintainers, Stephen,

On Fri, Dec 13, 2024 at 2:01 AM Xu, HailinX  wrote:
> > Hi all,
> >
> > Here is a list of patches targeted for stable release 22.11.7.
> >
> > The planned date for the final release is December 17th.
> >
> > Please help with testing and validation of your use cases and report any
> > issues/results with reply-all to this mail. For the final release the fixes 
> > and
> > reported validations will be added to the release notes.
> >
> > A release candidate tarball can be found at:
> >
> > https://dpdk.org/browse/dpdk-stable/tag/?id=v22.11.7-rc1
> >
> > These patches are located at branch 22.11 of dpdk-stable repo:
> > https://dpdk.org/browse/dpdk-stable/
> >
> > Thanks.
> >
> > Luca Boccassi
>
> Update the test status for Intel part. dpdk22.11.7-rc1 all validation test 
> done. Found 1 meson test bug.
>
> new issue:
> Bug 1589 - meson_tests/driver link_bonding_autotest test failed  -> This 
> issue also exists on 24.11

This bug affects the main branch and all LTS branches currently being validated.
The commit 112ce3917674 ("test/bonding: fix loop on members") revealed
a pre existing bug, so my advice is to revert this backport in all LTS
until we have a proper fix in main.

Thanks.

-- 
David Marchand

[v2 1/1] app/test: remove useless calls to rte_bitmap_free

[Ariel Otilibili]

* rte_bitmap_free is only useful for its return value
* and its return value is not used.

```
$ < lib/eal/include/rte_bitmap.h sed -ne '/bitmap_free/,/^}/p'

rte_bitmap_free(struct rte_bitmap *bmp)
{
/* Check input arguments */
if (bmp == NULL) {
return -1;
}

return 0;
}
```

Coverity issue: 357712
Coverity issue: 357737
Signed-off-by: Ariel Otilibili 
---
 app/test/test_bitmap.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/app/test/test_bitmap.c b/app/test/test_bitmap.c
index bab11812c7..a21210a215 100644
--- a/app/test/test_bitmap.c
+++ b/app/test/test_bitmap.c
@@ -208,7 +208,6 @@ test_bitmap_all_clear(void)
if (test_bitmap_scan_operations(bmp) < 0)
return TEST_FAILED;
 
-   rte_bitmap_free(bmp);
rte_free(mem);
 
return TEST_SUCCESS;
@@ -254,7 +253,6 @@ test_bitmap_all_set(void)
return TEST_FAILED;
}
 
-   rte_bitmap_free(bmp);
rte_free(mem);
 
return TEST_SUCCESS;
-- 
2.47.1

[v2 0/1] app/test: remove useless calls to rte_bitmap_free

[Ariel Otilibili]

*** BLURB HERE ***

Ariel Otilibili (1):
  app/test: remove useless calls to rte_bitmap_free

 app/test/test_bitmap.c | 2 --
 1 file changed, 2 deletions(-)

--
v2:
* fix style issues.

2.47.1

Re: [PATCH v1 1/4] net/e1000: prevent crashes in secondary processes

[Burakov, Anatoly]

On 12/12/2024 7:02 PM, Stephen Hemminger wrote:

On Thu, 12 Dec 2024 16:19:03 +
Anatoly Burakov  wrote:


Currently, the architecture of the base driver is such that it uses
function pointers internally. These are not guaranteed to be valid in
secondary processes, which can lead to crashes. This patch prevents these
functions from being executed in e1000 driver.

Fixes: 805803445a02 ("e1000: support EM devices (also known as e1000/e1000e)")
Cc: sta...@dpdk.org

Signed-off-by: Anatoly Burakov 
---


Not a fan of this. It creates so many special cases like: "This is ixgbe, and
it can do X but not Y in secondary process".

Either the driver should get fixed correctly so that all operations work
in secondary process, yes you would have to fix the base code.

Or the driver should be not support secondary process model at all.

If you have to write lots of documentation about limitations, it is not helping
the user.


That is the intention. Fixing these issues will take some effort as 
there's a lot of code to fix due to how endemic function pointers' usage 
are to these drivers, but in the meantime, things "arbitrarily not 
working" is better than things crashing outright.


--
Thanks,
Anatoly

[PATCH 1/2] net/mlx5: improve socket file path

[Yang Ming]

1. /var/tmp is hard code which is not a good style
2. /var/tmp may be not allowed to be written via container's
read only mode.

Signed-off-by: Yang Ming 
---
 drivers/net/mlx5/linux/mlx5_socket.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/mlx5/linux/mlx5_socket.c 
b/drivers/net/mlx5/linux/mlx5_socket.c
index 6ce0e59643..7fa6e5345c 100644
--- a/drivers/net/mlx5/linux/mlx5_socket.c
+++ b/drivers/net/mlx5/linux/mlx5_socket.c
@@ -20,7 +20,7 @@
 
 /* PMD socket service for tools. */
 
-#define MLX5_SOCKET_PATH "/var/tmp/dpdk_net_mlx5_%d"
+#define MLX5_SOCKET_FNAME "dpdk_net_mlx5"
 #define MLX5_ALL_PORT_IDS 0x
 
 int server_socket = -1; /* Unix socket for primary process. */
@@ -177,8 +177,8 @@ mlx5_pmd_socket_init(void)
ret = fcntl(server_socket, F_SETFL, flags | O_NONBLOCK);
if (ret < 0)
goto close;
-   snprintf(sun.sun_path, sizeof(sun.sun_path), MLX5_SOCKET_PATH,
-getpid());
+   snprintf(sun.sun_path, sizeof(sun.sun_path), "%s/%s_%d",
+rte_eal_get_runtime_dir(), MLX5_SOCKET_FNAME, getpid());
remove(sun.sun_path);
ret = bind(server_socket, (const struct sockaddr *)&sun, sizeof(sun));
if (ret < 0) {
@@ -223,6 +223,7 @@ mlx5_pmd_socket_uninit(void)
  mlx5_pmd_socket_handle, NULL);
claim_zero(close(server_socket));
server_socket = -1;
-   MKSTR(path, MLX5_SOCKET_PATH, getpid());
+   MKSTR(path, "%s/%s_%d", rte_eal_get_runtime_dir(), MLX5_SOCKET_FNAME,
+ getpid());
claim_zero(remove(path));
 }
-- 
2.34.1

[PATCH 2/2] net/mlx5: improve log file path

[Yang Ming]

1. /var/log is hard code which is not a good coding style.
2. /var/log may be not allowed to be written via container's
read-only mode.

Signed-off-by: Yang Ming 
---
 drivers/net/mlx5/mlx5_rxtx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
index eadadcdffb..a0da73c9c3 100644
--- a/drivers/net/mlx5/mlx5_rxtx.c
+++ b/drivers/net/mlx5/mlx5_rxtx.c
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -311,7 +312,7 @@ mlx5_set_swp_types_table(void)
}
 }
 
-#define MLX5_SYSTEM_LOG_DIR "/var/log"
+#define MLX5_SYSTEM_LOG_DIR rte_eal_get_runtime_dir()
 /**
  * Dump debug information to log file.
  *
-- 
2.34.1

Re: 22.11.7 patches review and test

[Kevin Traynor]

On 13/12/2024 08:33, David Marchand wrote:
> Hello LTS maintainers, Stephen,
> 
> On Fri, Dec 13, 2024 at 2:01 AM Xu, HailinX  wrote:
>>> Hi all,
>>>
>>> Here is a list of patches targeted for stable release 22.11.7.
>>>
>>> The planned date for the final release is December 17th.
>>>
>>> Please help with testing and validation of your use cases and report any
>>> issues/results with reply-all to this mail. For the final release the fixes 
>>> and
>>> reported validations will be added to the release notes.
>>>
>>> A release candidate tarball can be found at:
>>>
>>> https://dpdk.org/browse/dpdk-stable/tag/?id=v22.11.7-rc1
>>>
>>> These patches are located at branch 22.11 of dpdk-stable repo:
>>> https://dpdk.org/browse/dpdk-stable/
>>>
>>> Thanks.
>>>
>>> Luca Boccassi
>>
>> Update the test status for Intel part. dpdk22.11.7-rc1 all validation test 
>> done. Found 1 meson test bug.
>>
>> new issue:
>> Bug 1589 - meson_tests/driver link_bonding_autotest test failed  -> This 
>> issue also exists on 24.11
> 
> This bug affects the main branch and all LTS branches currently being 
> validated.
> The commit 112ce3917674 ("test/bonding: fix loop on members") revealed
> a pre existing bug, so my advice is to revert this backport in all LTS
> until we have a proper fix in main.
> 
> Thanks.
> 

Thanks David. Queued patch to revert on 21.11
https://github.com/kevintraynor/dpdk-stable/commit/4e9bfc8965d9fe6ce9ca988dbd84a4ec8647abb5

Re: [PATCH] app/test: fix stack overflow in lpm6_perf_autotest

[Medvedkin, Vladimir]

Hi Andre,

On 13/12/2024 02:39, Andre Muezerie wrote:

Test lpm6_perf_autotest was hitting a stack overflow on Windows
with both MSVC and Clang.

The fix is to move some of the data from the stack to the heap.

Signed-off-by: Andre Muezerie 
---
  app/test/test_lpm6_perf.c | 11 +--
  1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/app/test/test_lpm6_perf.c b/app/test/test_lpm6_perf.c
index 1860a99ed6..8231ad825d 100644
--- a/app/test/test_lpm6_perf.c
+++ b/app/test/test_lpm6_perf.c
@@ -117,8 +117,12 @@ test_lpm6_perf(void)
total_time = 0;
count = 0;
  
-	struct rte_ipv6_addr ip_batch[NUM_IPS_ENTRIES];

-   int32_t next_hops[NUM_IPS_ENTRIES];
+   struct rte_ipv6_addr *ip_batch = (struct rte_ipv6_addr *)malloc(

why not rte_malloc?

+   sizeof(struct rte_ipv6_addr) * NUM_IPS_ENTRIES);
+   TEST_LPM_ASSERT(ip_batch != NULL);
+
+   int32_t *next_hops = (int32_t *)malloc(sizeof(int32_t) * 
NUM_IPS_ENTRIES);
+   TEST_LPM_ASSERT(next_hops != NULL);
  
  	for (i = 0; i < NUM_IPS_ENTRIES; i++)

ip_batch[i] = large_ips_table[i].ip;
@@ -153,6 +157,9 @@ test_lpm6_perf(void)
printf("Average LPM Delete: %g cycles\n",
(double)total_time / NUM_ROUTE_ENTRIES);
  
+	free(next_hops);

+   free(ip_batch);
+
rte_lpm6_delete_all(lpm);
rte_lpm6_free(lpm);
  


--
Regards,
Vladimir

Re: GCP cloud : Virtio-PMD performance Issue

[Maxime Coquelin]

(with DPDK ML that got removed)

On 12/13/24 11:46, Maxime Coquelin wrote:



On 12/13/24 11:21, Mukul Sinha wrote:
Thanks @joshw...@google.com  @Maxime 
Coquelin  for the inputs.


@Maxime Coquelin 
I did code bisecting and was able to pin-point through test-pmd run 
that *this issue we are starting to see since DPDK-21.11 version 
onwards. Till DPDK-21.08 this issue is not seen.*
To remind the issue what we see is that while actual amount of 
serviced traffic by the hypervisor remains almost same between the two 
versions (implying the underlying GCP hypervisor is only capable of 
handling that much) but in >=dpdk-21.11 versions the virtio-PMD is 
pushing almost 20x traffic compared to dpdk-21.08 (This humongous 
traffic rate in  >=dpdk-21.11 versions leads to high packet drop rates 
since the underlying hypervisor is only capable of max handling the 
same load it was servicing in <=dpdk-21.08)
The same pattern can be seen even if we run traffic for a longer 
duration.


*_Eg:_*
Testpmd traffic run (for packet-size=1518) for exact same 
time-interval of 15 seconds:


_*>=21.11 DPDK version*_
   -- Forward statistics for port 0 
  --

   RX-packets: 2              RX-dropped: 0             RX-total: 2
   TX-packets: 19497570 *TX-dropped: 364674686 *    TX-total: 384172256

_*Upto 21.08 DPDK version *_
   -- Forward statistics for port 0 
  --

   RX-packets: 3              RX-dropped: 0             RX-total: 3
   TX-packets: 19480319       TX-dropped: 0             TX-total: 
19480319



As you can see
 >=dpdk-21.11
Packets generated : 384 million Packets serviced : ~19.5 million : 
Tx-dropped : 364 million

<=dpdk-21.08
Packets generated : ~19.5 million Packets serviced : ~19.5 million : 
Tx-dropped : 0


==
@Maxime Coquelin 
I have run through all the commits made by virtio-team between 
DPDK-21.11 and DPDK-21.08 as per the commit-logs available at 
https://git.dpdk.org/dpdk/log/drivers/net/virtio 

I even tried undoing all the possible relevant commits (I could think 
of) on a dpdk-21.11 workspace & then re-running testpmd in order to 
track down which commit has introduced this regression but no luck.
Need your inputs further if you could glance through the commits made 
in between these releases and let us know if there's any particular 
commit of interest which you think can cause the behavior as seen 
above (or if there's any commit not captured in the above git link; 
maybe a commit checkin outside the virtio PMD code perhaps?).


As your issue seems 100% reproducible, using git bisect you should be 
able to point to the commit introducing the regression.


This is what I need to be able to help you.

Regards,
Maxime



Thanks,
Mukul


On Mon, Dec 9, 2024 at 9:54 PM Joshua Washington > wrote:


    Hello,

    Based on your VM shape (8 vcpu VM) and packet size (1518B packets),
    what you are seeing is exactly expected. 8 vCPU Gen 2 VMs has a
    default egress cap of 16 Gbps. This equates to roughly 1.3Mpps when
    using 1518B packets, including IFG. Over the course of 15 seconds,
    19.5 million packets should be sent, which matches both cases. The
    difference here seems to be what happens on DPDK, not GCP. I don't
    believe that packet drops on the host NIC are captured in DPDK
    stats; likely the descriptor ring just filled up because the egress
    bandwidth cap was hit and queue servicing was throttled. This would
    cause a TX burst to return less packets than the burst size. The
    difference between 20.05 and 22.11 might have to do with this
    reporting, or a change in testpmd logic for when to send new bursts
    of traffic.

    Best,
    Josh


    On Mon, Dec 9, 2024, 07:39 Mukul Sinha mailto:mukul.si...@broadcom.com>> wrote:

    GCP-dev team
    @jeroe...@google.com
    @rush...@google.com
     @joshw...@google.com
    
    Can you please check the following email & get back ?


    On Fri, Dec 6, 2024 at 2:04 AM Mukul Sinha
    mailto:mukul.si...@broadcom.com>> 
wrote:


    Hi GCP & Virtio-PMD dev teams,
    We are from VMware NSX Advanced Load Balancer Team whereby
    in GCP-cloud (*custom-8-8192 VM instance type 8core8G*) we
    are triaging an issue of TCP profile application throughput
    performance with single dispatcher core single Rx/Tx queue
    (queue depth: 2048) the throughput performanc

Re: [PATCH v2 0/3] Improve lock annotations

[David Marchand]

On Thu, Dec 12, 2024 at 5:02 PM David Marchand
 wrote:
>
> A recent bug (see 22aa9a9c7099 ("vhost: fix deadlock in Rx async path"))
> made more visible a gap in the clang thread safety annotations that
> DPDK uses: no distinction is made between releasing a read lock and
> releasing a write lock.
>
> Clang 3.6 and later offers improved thread safety checks.
>
> Marking objects as "lockable" has evolved into flagging some named
> "capability". clang reports the capability name when an error is
> reported (making this report a bit easier to understand).
>
> For example, a spinlock is now flagged as:
> typedef struct __rte_capability("spinlock") {
>   volatile RTE_ATOMIC(int) locked;
> } rte_spinlock_t;
>
>
> For "exclusive" locking (spinlocks / write locks), the conversion is:
> - exclusive_lock_function -> acquire_capability
> - exclusive_trylock_function -> try_acquire_capability
> - unlock_function -> release_capability
> ...
>
> For "shared" locking (read locks):
> - shared_lock_function -> acquire_shared_capability
> - shared_trylock_function -> try_acquire_shared_capability
> - unlock_function -> release_shared_capability
> ...
>
>
> This series proposes to use those annotations (sticking to the
> convention of simply prefixing the compiler attributes with __rte_).
> The existing "old" annotations macros are left in place in case users
> started to rely on them.
>
> Note: DPDK requirements state that clang version must be >= 3.6
> (following use of C11 standard).
>
> Comments welcome.

Just a note on Intel CI report.
http://mails.dpdk.org/archives/test-report/2024-December/834120.html

(As reported a few times), this CI reports an error on documentation generation.
I can't be sure but this failure here is most likely due to this CI
filtering out of the patches any update on doc/.


-- 
David Marchand

[PATCH v2 6/6] buildtools: externally check exported headers

[David Marchand]

At the moment, the headers check (triggered via the check_includes meson
option) is run "internally", iow with compilation flags and include path
coming from the meson components.

One issue is that both internal and public headers are usually stored
in a single directory in the DPDK components.
If a lib/foo library exports a header rte_foo.h (iow rte_foo.h is part
of the headers list in lib/foo/meson.build) and this rte_foo.h includes
an internal header foo_internal.h, then the headers check won't detect
such an issue as rte_foo.h is compiled with -Ilib/foo.

Reimplement the headers check by inspecting (compiling) all DPDK headers
installed in a staging directory.
To identify the directory where DPDK headers are, this check relies on
pkg-config and skips EAL generic/ and internal/ headers.

The existing headers check (though less accurate) is kept as is,
as it provides a first level of check and may have existing users.

Signed-off-by: David Marchand 
---
 .ci/linux-build.sh|  7 ++--
 buildtools/chkincs/Makefile   | 77 +++
 devtools/test-meson-builds.sh |  6 ++-
 3 files changed, 85 insertions(+), 5 deletions(-)
 create mode 100644 buildtools/chkincs/Makefile

diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index fdb5787621..3146d9ccd8 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -95,8 +95,6 @@ OPTS="$OPTS -Ddefault_library=$DEF_LIB"
 OPTS="$OPTS -Dbuildtype=$buildtype"
 if [ "$STDATOMIC" = "true" ]; then
OPTS="$OPTS -Denable_stdatomic=true"
-else
-   OPTS="$OPTS -Dcheck_includes=true"
 fi
 if [ "$MINI" = "true" ]; then
 OPTS="$OPTS -Denable_drivers=net/null"
@@ -176,13 +174,16 @@ if [ "$RUN_TESTS" = "true" ]; then
 [ "$failed" != "true" ]
 fi
 
-# Test examples compilation with an installed dpdk
+# Test headers and examples compilation with an installed dpdk
 if [ "$BUILD_EXAMPLES" = "true" ]; then
 [ -d install ] || DESTDIR=$(pwd)/install meson install -C build
 export LD_LIBRARY_PATH=$(dirname $(find $(pwd)/install -name 
librte_eal.so)):$LD_LIBRARY_PATH
 export PATH=$(dirname $(find $(pwd)/install -name dpdk-devbind.py)):$PATH
 export PKG_CONFIG_PATH=$(dirname $(find $(pwd)/install -name 
libdpdk.pc)):$PKG_CONFIG_PATH
 export PKGCONF="pkg-config --define-prefix"
+
+make -C buildtools/chkincs O=build/buildtools/chkincs
+
 find build/examples -maxdepth 1 -type f -name "dpdk-*" |
 while read target; do
 target=${target%%:*}
diff --git a/buildtools/chkincs/Makefile b/buildtools/chkincs/Makefile
new file mode 100644
index 00..838819c617
--- /dev/null
+++ b/buildtools/chkincs/Makefile
@@ -0,0 +1,77 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright (c) 2024 Red Hat, Inc.
+
+ifeq ($(V),)
+Q ?= @
+else
+Q =
+endif
+
+O ?= build
+
+PKGCONF ?= pkg-config
+
+ifneq ($(shell $(PKGCONF) --exists libdpdk && echo 0),0)
+$(error "no installation of DPDK found")
+endif
+
+ifeq ($(I),)
+
+.PHONY: headers_list
+.ONESHELL:
+headers_list:
+   $(Q)for dir in $$($(PKGCONF) --cflags-only-I libdpdk); do
+   dir=$${dir##-I}
+   [ -e $$dir/rte_build_config.h ] || continue
+   $(MAKE) I="$$dir" O="$(O)"
+   break
+   done
+else
+
+HEADERS := $(shell find $(I) -name "*.h" | grep -vE $(I)'/(generic|internal)/')
+SRCS := $(patsubst $(I)/%.h, $(O)/%.c, $(HEADERS))
+.PRECIOUS: $(SRCS)
+
+PC_FILE := $(shell $(PKGCONF) --path libdpdk 2>/dev/null)
+CFLAGS = $(shell $(PKGCONF) --cflags libdpdk) -Wall -Wextra -Werror
+LDFLAGS = $(shell $(PKGCONF) --libs libdpdk)
+
+OBJS := $(patsubst $(I)/%.h, $(O)/%.o, $(HEADERS))
+OBJS_EXP := $(patsubst $(I)/%.h, $(O)/%+exp.o, $(HEADERS))
+OBJS_ALL := $(patsubst $(I)/%.h, $(O)/%+all.o, $(HEADERS))
+
+all: $(OBJS) $(OBJS_EXP) $(OBJS_ALL)
+
+$(O):
+   $(Q)mkdir -p $@
+
+$(O)/%.c: $(I)/%.h $(O) gen_c_file_for_header.py Makefile
+   $(Q)python3 gen_c_file_for_header.py $< $@
+
+$(O)/%.o: $(O)/%.c Makefile $(PC_FILE)
+   $(Q)$(CC) $(CFLAGS) -o $@ -c $<
+
+$(O)/%+exp.o: $(O)/%.c Makefile $(PC_FILE)
+   $(Q)$(CC) $(CFLAGS) -DALLOW_EXPERIMENTAL_API -o $@ -c $<
+
+$(O)/%+all.o: $(O)/%.c Makefile $(PC_FILE)
+   $(Q)$(CC) $(CFLAGS) -DALLOW_EXPERIMENTAL_API -DALLOW_INTERNAL_API -o $@ 
-c $<
+
+CXXFLAGS = $(shell $(PKGCONF) --cflags libdpdk) -Wall -Wextra -Werror
+
+OBJS_CPP := $(patsubst $(I)/%.h, $(O)/%.cpp.o, $(HEADERS))
+OBJS_CPP_EXP := $(patsubst $(I)/%.h, $(O)/%.cpp+exp.o, $(HEADERS))
+OBJS_CPP_ALL := $(patsubst $(I)/%.h, $(O)/%.cpp+all.o, $(HEADERS))
+
+all: $(OBJS_CPP) $(OBJS_CPP_EXP) $(OBJS_CPP_ALL)
+
+$(O)/%.cpp.o: $(O)/%.c Makefile $(PC_FILE)
+   $(Q)$(CXX) $(CXXFLAGS) -o $@ -c $<
+
+$(O)/%.cpp+exp.o: $(O)/%.c Makefile $(PC_FILE)
+   $(Q)$(CXX) $(CXXFLAGS) -DALLOW_EXPERIMENTAL_API -o $@ -c $<
+
+$(O)/%.cpp+all.o: $(O)/%.c Makefile $(PC_FILE)
+   $(Q)$(CXX) $(CXXFLAGS) -DALLOW_EXPERIMENTAL_API -DALLOW_INTERNAL_API -o 
$@ -c $<
+
+endif
diff --git a/devtools/test-meson-builds.sh b/devto

[PATCH v2 4/6] drivers: fix exported headers

[David Marchand]

Those headers could not be included individually as they were not
including their dependencies, or were subject to some build warnings.

Fixes: 831dba47bd36 ("bus/vmbus: add Hyper-V virtual bus support")
Fixes: 5b2a1a02dcaf ("crypto/cnxk: fix experimental version for PMD API")
Fixes: e5abbfa5 ("crypto/cnxk: add PMD API for getting CPTR")
Fixes: 3ca607402c4d ("crypto/cnxk: add PMD API to flush CTX")
Fixes: 8c3495f5d2dd ("net/dpaa: support loopback API")
Fixes: 12b435bf8f2f ("net/iavf: support flex desc metadata extraction")
Fixes: 23f627e0ed28 ("net/mlx5: add flow sync API")
Fixes: f5177bdc8b76 ("net/mlx5: add GENEVE TLV options parser API")
Fixes: 7cf197684589 ("raw/cnxk_bphy: support interrupt init and cleanup")
Fixes: 633dae698070 ("raw/cnxk_gpio: add standard GPIO operations")
Fixes: 53c71586c789 ("raw/dpaa2_cmdif: support enqueue/dequeue operations")
Fixes: c39d1e082a4b ("raw/ntb: setup queues")

Signed-off-by: David Marchand 
---
 drivers/bus/vmbus/rte_vmbus_reg.h |  6 ++
 drivers/crypto/cnxk/rte_pmd_cnxk_crypto.h |  4 
 drivers/net/dpaa/rte_pmd_dpaa.h   |  2 ++
 drivers/net/iavf/rte_pmd_iavf.h   |  6 ++
 drivers/net/mlx5/rte_pmd_mlx5.h   |  3 +++
 drivers/raw/cnxk_bphy/rte_pmd_bphy.h  | 16 
 drivers/raw/cnxk_gpio/rte_pmd_cnxk_gpio.h |  3 +++
 drivers/raw/dpaa2_cmdif/rte_pmd_dpaa2_cmdif.h |  2 ++
 drivers/raw/ntb/rte_pmd_ntb.h |  2 ++
 9 files changed, 44 insertions(+)

diff --git a/drivers/bus/vmbus/rte_vmbus_reg.h 
b/drivers/bus/vmbus/rte_vmbus_reg.h
index e3299aa871..95c8eb29b4 100644
--- a/drivers/bus/vmbus/rte_vmbus_reg.h
+++ b/drivers/bus/vmbus/rte_vmbus_reg.h
@@ -6,6 +6,12 @@
 #ifndef _VMBUS_REG_H_
 #define _VMBUS_REG_H_
 
+#include 
+
+#include 
+#include 
+#include 
+
 /*
  * Hyper-V SynIC message format.
  */
diff --git a/drivers/crypto/cnxk/rte_pmd_cnxk_crypto.h 
b/drivers/crypto/cnxk/rte_pmd_cnxk_crypto.h
index 02278605a2..2bb0ff9e95 100644
--- a/drivers/crypto/cnxk/rte_pmd_cnxk_crypto.h
+++ b/drivers/crypto/cnxk/rte_pmd_cnxk_crypto.h
@@ -11,8 +11,12 @@
 #ifndef _PMD_CNXK_CRYPTO_H_
 #define _PMD_CNXK_CRYPTO_H_
 
+#include 
 #include 
 
+#include 
+#include 
+
 /* Forward declarations */
 
 /**
diff --git a/drivers/net/dpaa/rte_pmd_dpaa.h b/drivers/net/dpaa/rte_pmd_dpaa.h
index ec45633ba2..0a57e2097a 100644
--- a/drivers/net/dpaa/rte_pmd_dpaa.h
+++ b/drivers/net/dpaa/rte_pmd_dpaa.h
@@ -5,6 +5,8 @@
 #ifndef _PMD_DPAA_H_
 #define _PMD_DPAA_H_
 
+#include 
+
 /**
  * @file rte_pmd_dpaa.h
  *
diff --git a/drivers/net/iavf/rte_pmd_iavf.h b/drivers/net/iavf/rte_pmd_iavf.h
index 56d453fc4c..04b86a5dd7 100644
--- a/drivers/net/iavf/rte_pmd_iavf.h
+++ b/drivers/net/iavf/rte_pmd_iavf.h
@@ -15,6 +15,7 @@
  */
 
 #include 
+
 #include 
 #include 
 #include 
@@ -184,6 +185,7 @@ __rte_experimental
 static inline void
 rte_pmd_ifd_dump_proto_xtr_metadata(struct rte_mbuf *m)
 {
+#ifdef ALLOW_EXPERIMENTAL_API
union rte_pmd_ifd_proto_xtr_metadata data;
 
if (!rte_pmd_ifd_dynf_proto_xtr_metadata_avail())
@@ -243,6 +245,10 @@ rte_pmd_ifd_dump_proto_xtr_metadata(struct rte_mbuf *m)
else if (m->ol_flags & RTE_IAVF_PKT_RX_DYNF_PROTO_XTR_IP_OFFSET)
printf(" - Flexible descriptor's Extraction: ip_offset=%u",
   data.ip_ofs);
+#else
+   RTE_SET_USED(m);
+   RTE_VERIFY(false);
+#endif
 }
 
 #ifdef __cplusplus
diff --git a/drivers/net/mlx5/rte_pmd_mlx5.h b/drivers/net/mlx5/rte_pmd_mlx5.h
index fdd2f65888..f2c6aebe0b 100644
--- a/drivers/net/mlx5/rte_pmd_mlx5.h
+++ b/drivers/net/mlx5/rte_pmd_mlx5.h
@@ -5,6 +5,9 @@
 #ifndef RTE_PMD_PRIVATE_MLX5_H_
 #define RTE_PMD_PRIVATE_MLX5_H_
 
+#include 
+
+#include 
 #include 
 
 /**
diff --git a/drivers/raw/cnxk_bphy/rte_pmd_bphy.h 
b/drivers/raw/cnxk_bphy/rte_pmd_bphy.h
index f668e6ea82..c200c935ff 100644
--- a/drivers/raw/cnxk_bphy/rte_pmd_bphy.h
+++ b/drivers/raw/cnxk_bphy/rte_pmd_bphy.h
@@ -391,6 +391,7 @@ rte_pmd_bphy_intr_init(uint16_t dev_id)
 {
struct cnxk_bphy_irq_msg msg = {
.type = CNXK_BPHY_IRQ_MSG_TYPE_INIT,
+   .data = NULL,
};
 
return __rte_pmd_bphy_enq_deq(dev_id, CNXK_BPHY_DEF_QUEUE, &msg,
@@ -411,6 +412,7 @@ rte_pmd_bphy_intr_fini(uint16_t dev_id)
 {
struct cnxk_bphy_irq_msg msg = {
.type = CNXK_BPHY_IRQ_MSG_TYPE_FINI,
+   .data = NULL,
};
 
return __rte_pmd_bphy_enq_deq(dev_id, CNXK_BPHY_DEF_QUEUE, &msg,
@@ -470,6 +472,9 @@ rte_pmd_bphy_intr_unregister(uint16_t dev_id, int irq_num)
 {
struct cnxk_bphy_irq_info info = {
.irq_num = irq_num,
+   .handler = NULL,
+   .data = NULL,
+   .cpu = -1,
};
struct cnxk_bphy_irq_msg msg = {
.type = CNXK_BPHY_IRQ_MSG_TYPE_UNREGISTER,
@@ -496,6 +501,7 @@ rte_pmd_bphy_intr_mem_get(uint16_t dev_id, struct 
cnxk_bphy_mem *mem)
 {
struct cnxk_bphy_irq_m

[PATCH v2 3/6] eventdev: do not include driver header in DMA adapter

[David Marchand]

The dma adapter header does not require including rte_dmadev_pmd.h which
is a driver header.

Fixes: 66a30a29387a ("eventdev/dma: introduce DMA adapter")

Signed-off-by: David Marchand 
Acked-by: Amit Prakash Shukla 
---
 lib/eventdev/rte_event_dma_adapter.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/eventdev/rte_event_dma_adapter.h 
b/lib/eventdev/rte_event_dma_adapter.h
index 5c480b82ff..34142f26db 100644
--- a/lib/eventdev/rte_event_dma_adapter.h
+++ b/lib/eventdev/rte_event_dma_adapter.h
@@ -144,7 +144,7 @@
 #include 
 
 #include 
-#include 
+#include 
 #include 
 
 #ifdef __cplusplus
-- 
2.47.0

[PATCH v2 1/6] baseband/acc: fix exported header

[David Marchand]

rte_acc_cfg.h relies on rte_acc_common_cfg.h.

Fixes: 32e8b7ea35dd ("baseband/acc100: refactor to segregate common code")

Signed-off-by: David Marchand 
---
 drivers/baseband/acc/meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/baseband/acc/meson.build b/drivers/baseband/acc/meson.build
index 64fcf1537a..d9fb947eaa 100644
--- a/drivers/baseband/acc/meson.build
+++ b/drivers/baseband/acc/meson.build
@@ -26,4 +26,4 @@ deps += ['bus_pci']
 
 sources = files('acc_common.c', 'rte_acc100_pmd.c', 'rte_vrb_pmd.c')
 
-headers = files('rte_acc_cfg.h')
+headers = files('rte_acc_cfg.h', 'rte_acc_common_cfg.h')
-- 
2.47.0

[PATCH v2 2/6] drivers: drop export of driver headers

[David Marchand]

Many classes are exposing driver only headers as public headers.
Move them to the driver_sdk_headers list.

Signed-off-by: David Marchand 
---
 lib/bbdev/meson.build| 5 ++---
 lib/ethdev/meson.build   | 6 +++---
 lib/mldev/meson.build| 5 +
 lib/rawdev/meson.build   | 3 ++-
 lib/regexdev/meson.build | 3 ++-
 lib/security/meson.build | 3 ++-
 6 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/lib/bbdev/meson.build b/lib/bbdev/meson.build
index 07685e7578..7d035065f1 100644
--- a/lib/bbdev/meson.build
+++ b/lib/bbdev/meson.build
@@ -8,7 +8,6 @@ if is_windows
 endif
 
 sources = files('rte_bbdev.c')
-headers = files('rte_bbdev.h',
-'rte_bbdev_pmd.h',
-'rte_bbdev_op.h')
+headers = files('rte_bbdev.h', 'rte_bbdev_op.h')
+driver_sdk_headers = files('rte_bbdev_pmd.h')
 deps += ['mbuf']
diff --git a/lib/ethdev/meson.build b/lib/ethdev/meson.build
index f1d2586591..8ba6c708a2 100644
--- a/lib/ethdev/meson.build
+++ b/lib/ethdev/meson.build
@@ -26,11 +26,8 @@ headers = files(
 'rte_ethdev_trace_fp.h',
 'rte_dev_info.h',
 'rte_flow.h',
-'rte_flow_driver.h',
 'rte_mtr.h',
-'rte_mtr_driver.h',
 'rte_tm.h',
-'rte_tm_driver.h',
 )
 
 indirect_headers += files(
@@ -42,6 +39,9 @@ driver_sdk_headers += files(
 'ethdev_driver.h',
 'ethdev_pci.h',
 'ethdev_vdev.h',
+'rte_flow_driver.h',
+'rte_mtr_driver.h',
+'rte_tm_driver.h',
 )
 
 if is_linux
diff --git a/lib/mldev/meson.build b/lib/mldev/meson.build
index 2c933baad6..efc3edd288 100644
--- a/lib/mldev/meson.build
+++ b/lib/mldev/meson.build
@@ -32,11 +32,8 @@ headers = files(
 'rte_mldev.h',
 )
 
-indirect_headers += files(
-'rte_mldev_core.h',
-)
-
 driver_sdk_headers += files(
+'rte_mldev_core.h',
 'rte_mldev_pmd.h',
 'mldev_utils.h',
 )
diff --git a/lib/rawdev/meson.build b/lib/rawdev/meson.build
index 7dfc3d5cf9..ccfd922fda 100644
--- a/lib/rawdev/meson.build
+++ b/lib/rawdev/meson.build
@@ -8,6 +8,7 @@ if is_windows
 endif
 
 sources = files('rte_rawdev.c')
-headers = files('rte_rawdev.h', 'rte_rawdev_pmd.h')
+headers = files('rte_rawdev.h')
+driver_sdk_headers = files('rte_rawdev_pmd.h')
 
 deps += ['telemetry']
diff --git a/lib/regexdev/meson.build b/lib/regexdev/meson.build
index 426e764ece..05040051c5 100644
--- a/lib/regexdev/meson.build
+++ b/lib/regexdev/meson.build
@@ -8,6 +8,7 @@ if is_windows
 endif
 
 sources = files('rte_regexdev.c')
-headers = files('rte_regexdev.h', 'rte_regexdev_driver.h')
+headers = files('rte_regexdev.h')
 indirect_headers += files('rte_regexdev_core.h')
+driver_sdk_headers = files('rte_regexdev_driver.h')
 deps += ['mbuf']
diff --git a/lib/security/meson.build b/lib/security/meson.build
index 1034a7a299..d5431d472c 100644
--- a/lib/security/meson.build
+++ b/lib/security/meson.build
@@ -2,5 +2,6 @@
 # Copyright(c) 2017-2019 Intel Corporation
 
 sources = files('rte_security.c')
-headers = files('rte_security.h', 'rte_security_driver.h')
+headers = files('rte_security.h')
+driver_sdk_headers = files('rte_security_driver.h')
 deps += ['mempool', 'cryptodev', 'net']
-- 
2.47.0

[PATCH v2 0/6] Add a stricter headers check

[David Marchand]

As explained in patch 6, the current headers check can not catch
issues when a public header includes an internal header.
Fixing this from meson does not seem an easy task.

This series approach is to reimplement the check in a Makefile invoked
out of DPDK (like what is done for external compilation of examples).
This has the advantage of being simple, and avoiding any (non intentional)
implicit include path coming from the meson framework.

As there was no easy way to distinguish "indirect" headers in an
installed DPDK, I chose to move those headers in a new sub directory
(patch 5).

Patch 1-4 fixes have not been marked as backport material as those bugs
seems minor/easy to fix externally (by either including missing headers,
or enabling enable_driver_sdk option).

For now, I left the check_includes= option untouched, as there may be
users of this check and this check still catches issues without
requiring to install DPDK.


-- 
David Marchand

David Marchand (6):
  baseband/acc: fix exported header
  drivers: drop export of driver headers
  eventdev: do not include driver header in DMA adapter
  drivers: fix exported headers
  build: install indirect headers to a dedicated directory
  buildtools: externally check exported headers

 .ci/linux-build.sh|  7 +-
 buildtools/chkincs/Makefile   | 77 +++
 buildtools/pkg-config/meson.build |  8 +-
 devtools/test-meson-builds.sh |  6 +-
 drivers/baseband/acc/meson.build  |  2 +-
 drivers/bus/vmbus/rte_vmbus_reg.h |  6 ++
 drivers/crypto/cnxk/rte_pmd_cnxk_crypto.h |  4 +
 drivers/net/dpaa/rte_pmd_dpaa.h   |  2 +
 drivers/net/iavf/rte_pmd_iavf.h   |  6 ++
 drivers/net/mlx5/rte_pmd_mlx5.h   |  3 +
 drivers/raw/cnxk_bphy/rte_pmd_bphy.h  | 16 
 drivers/raw/cnxk_gpio/rte_pmd_cnxk_gpio.h |  3 +
 drivers/raw/dpaa2_cmdif/rte_pmd_dpaa2_cmdif.h |  2 +
 drivers/raw/ntb/rte_pmd_ntb.h |  2 +
 lib/bbdev/meson.build |  5 +-
 lib/eal/x86/include/meson.build   |  3 +-
 lib/ethdev/meson.build|  6 +-
 lib/eventdev/rte_event_dma_adapter.h  |  2 +-
 lib/meson.build   |  2 +-
 lib/mldev/meson.build |  5 +-
 lib/rawdev/meson.build|  3 +-
 lib/regexdev/meson.build  |  3 +-
 lib/security/meson.build  |  3 +-
 23 files changed, 153 insertions(+), 23 deletions(-)
 create mode 100644 buildtools/chkincs/Makefile

-- 
2.47.0

Re: [PATCH v2 1/6] baseband/acc: fix exported header

[Bruce Richardson]

On Fri, Dec 13, 2024 at 11:50:05AM +0100, David Marchand wrote:
> rte_acc_cfg.h relies on rte_acc_common_cfg.h.
> 
> Fixes: 32e8b7ea35dd ("baseband/acc100: refactor to segregate common code")
> 
> Signed-off-by: David Marchand 
> ---
Indeed it does depend on it!

Acked-by: Bruce Richardson 

Re: [PATCH v2 3/6] eventdev: do not include driver header in DMA adapter

[Bruce Richardson]

On Fri, Dec 13, 2024 at 11:50:07AM +0100, David Marchand wrote:
> The dma adapter header does not require including rte_dmadev_pmd.h which
> is a driver header.
> 
> Fixes: 66a30a29387a ("eventdev/dma: introduce DMA adapter")
> 
> Signed-off-by: David Marchand 
> Acked-by: Amit Prakash Shukla 

Acked-by: Bruce Richardson 
> ---
>  lib/eventdev/rte_event_dma_adapter.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/eventdev/rte_event_dma_adapter.h 
> b/lib/eventdev/rte_event_dma_adapter.h
> index 5c480b82ff..34142f26db 100644
> --- a/lib/eventdev/rte_event_dma_adapter.h
> +++ b/lib/eventdev/rte_event_dma_adapter.h
> @@ -144,7 +144,7 @@
>  #include 
>  
>  #include 
> -#include 
> +#include 
>  #include 
>  
>  #ifdef __cplusplus
> -- 
> 2.47.0
> 

[PATCH v2 5/6] build: install indirect headers to a dedicated directory

[David Marchand]

The headers check currently skips "indirect" headers as instructed via
the indirect_headers meson variable.

This headers check has some limitation that will be addressed in a next
change by inspected all exported headers.
However, exported headers lack the information about "indirect" quality.

Separate "indirect" headers by exporting them in a internal/ sub directory.
This also makes it more obvious which headers are not to be directly used
by an application.

Signed-off-by: David Marchand 
---
 buildtools/pkg-config/meson.build | 8 +++-
 lib/eal/x86/include/meson.build   | 3 ++-
 lib/meson.build   | 2 +-
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/buildtools/pkg-config/meson.build 
b/buildtools/pkg-config/meson.build
index b36add17e3..b2afca9eec 100644
--- a/buildtools/pkg-config/meson.build
+++ b/buildtools/pkg-config/meson.build
@@ -27,12 +27,18 @@ endif
 # are skipped in the case of static linkage thanks to the flag --as-needed.
 
 
+subdirs = ['.', 'internal']
+if get_option('include_subdir_arch') != ''
+subdirs += [get_option('include_subdir_arch')]
+subdirs += [join_paths(get_option('include_subdir_arch'), 'internal')]
+endif
+
 pkg.generate(name: 'dpdk-libs',
 filebase: 'libdpdk-libs',
 description: '''Internal-only DPDK pkgconfig file. Not for direct use.
 Use libdpdk.pc instead of this file to query DPDK compile/link arguments''',
 version: meson.project_version(),
-subdirs: [get_option('include_subdir_arch'), '.'],
+subdirs: subdirs,
 extra_cflags: pkg_extra_cflags,
 libraries: ['-Wl,--as-needed'] + dpdk_libraries,
 libraries_private: dpdk_extra_ldflags)
diff --git a/lib/eal/x86/include/meson.build b/lib/eal/x86/include/meson.build
index 52d2f8e969..a100330208 100644
--- a/lib/eal/x86/include/meson.build
+++ b/lib/eal/x86/include/meson.build
@@ -22,5 +22,6 @@ arch_indirect_headers = files(
 'rte_byteorder_32.h',
 'rte_byteorder_64.h',
 )
-install_headers(arch_headers + arch_indirect_headers, subdir: 
get_option('include_subdir_arch'))
+install_headers(arch_headers, subdir: get_option('include_subdir_arch'))
+install_headers(arch_indirect_headers, subdir: 
join_paths(get_option('include_subdir_arch'), 'internal'))
 dpdk_chkinc_headers += arch_headers
diff --git a/lib/meson.build b/lib/meson.build
index ce92cb5537..78ada7782e 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -202,7 +202,7 @@ foreach l:libraries
 dpdk_libs_enabled += name
 dpdk_conf.set('RTE_LIB_' + name.to_upper(), 1)
 install_headers(headers)
-install_headers(indirect_headers)
+install_headers(indirect_headers, subdir: 'internal')
 if get_option('enable_driver_sdk')
 install_headers(driver_sdk_headers)
 endif
-- 
2.47.0

Re: [PATCH v2 2/6] drivers: drop export of driver headers

[Bruce Richardson]

On Fri, Dec 13, 2024 at 11:50:06AM +0100, David Marchand wrote:
> Many classes are exposing driver only headers as public headers.
> Move them to the driver_sdk_headers list.
> 
> Signed-off-by: David Marchand 
> ---

LGTM. The names of most of the headers are pretty clear that they should be
sdk-only headers.

Acked-by: Bruce Richardson 

Re: [PATCH v2 4/6] drivers: fix exported headers

[Bruce Richardson]

On Fri, Dec 13, 2024 at 11:50:08AM +0100, David Marchand wrote:
> Those headers could not be included individually as they were not
> including their dependencies, or were subject to some build warnings.
> 
> Fixes: 831dba47bd36 ("bus/vmbus: add Hyper-V virtual bus support")
> Fixes: 5b2a1a02dcaf ("crypto/cnxk: fix experimental version for PMD API")
> Fixes: e5abbfa5 ("crypto/cnxk: add PMD API for getting CPTR")
> Fixes: 3ca607402c4d ("crypto/cnxk: add PMD API to flush CTX")
> Fixes: 8c3495f5d2dd ("net/dpaa: support loopback API")
> Fixes: 12b435bf8f2f ("net/iavf: support flex desc metadata extraction")
> Fixes: 23f627e0ed28 ("net/mlx5: add flow sync API")
> Fixes: f5177bdc8b76 ("net/mlx5: add GENEVE TLV options parser API")
> Fixes: 7cf197684589 ("raw/cnxk_bphy: support interrupt init and cleanup")
> Fixes: 633dae698070 ("raw/cnxk_gpio: add standard GPIO operations")
> Fixes: 53c71586c789 ("raw/dpaa2_cmdif: support enqueue/dequeue operations")
> Fixes: c39d1e082a4b ("raw/ntb: setup queues")
> 
> Signed-off-by: David Marchand 

Generally LGTM, some queries inline below.

/Bruce

> ---
>  drivers/bus/vmbus/rte_vmbus_reg.h |  6 ++
>  drivers/crypto/cnxk/rte_pmd_cnxk_crypto.h |  4 
>  drivers/net/dpaa/rte_pmd_dpaa.h   |  2 ++
>  drivers/net/iavf/rte_pmd_iavf.h   |  6 ++
>  drivers/net/mlx5/rte_pmd_mlx5.h   |  3 +++
>  drivers/raw/cnxk_bphy/rte_pmd_bphy.h  | 16 
>  drivers/raw/cnxk_gpio/rte_pmd_cnxk_gpio.h |  3 +++
>  drivers/raw/dpaa2_cmdif/rte_pmd_dpaa2_cmdif.h |  2 ++
>  drivers/raw/ntb/rte_pmd_ntb.h |  2 ++
>  9 files changed, 44 insertions(+)
> 
> diff --git a/drivers/bus/vmbus/rte_vmbus_reg.h 
> b/drivers/bus/vmbus/rte_vmbus_reg.h
> index e3299aa871..95c8eb29b4 100644
> --- a/drivers/bus/vmbus/rte_vmbus_reg.h
> +++ b/drivers/bus/vmbus/rte_vmbus_reg.h
> @@ -6,6 +6,12 @@
>  #ifndef _VMBUS_REG_H_
>  #define _VMBUS_REG_H_
>  
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
>  /*
>   * Hyper-V SynIC message format.
>   */
> diff --git a/drivers/crypto/cnxk/rte_pmd_cnxk_crypto.h 
> b/drivers/crypto/cnxk/rte_pmd_cnxk_crypto.h
> index 02278605a2..2bb0ff9e95 100644
> --- a/drivers/crypto/cnxk/rte_pmd_cnxk_crypto.h
> +++ b/drivers/crypto/cnxk/rte_pmd_cnxk_crypto.h
> @@ -11,8 +11,12 @@
>  #ifndef _PMD_CNXK_CRYPTO_H_
>  #define _PMD_CNXK_CRYPTO_H_
>  
> +#include 
>  #include 
>  
> +#include 
> +#include 
> +
>  /* Forward declarations */
>  
>  /**
> diff --git a/drivers/net/dpaa/rte_pmd_dpaa.h b/drivers/net/dpaa/rte_pmd_dpaa.h
> index ec45633ba2..0a57e2097a 100644
> --- a/drivers/net/dpaa/rte_pmd_dpaa.h
> +++ b/drivers/net/dpaa/rte_pmd_dpaa.h
> @@ -5,6 +5,8 @@
>  #ifndef _PMD_DPAA_H_
>  #define _PMD_DPAA_H_
>  
> +#include 
> +
>  /**
>   * @file rte_pmd_dpaa.h
>   *
> diff --git a/drivers/net/iavf/rte_pmd_iavf.h b/drivers/net/iavf/rte_pmd_iavf.h
> index 56d453fc4c..04b86a5dd7 100644
> --- a/drivers/net/iavf/rte_pmd_iavf.h
> +++ b/drivers/net/iavf/rte_pmd_iavf.h
> @@ -15,6 +15,7 @@
>   */
>  
>  #include 
> +
>  #include 
>  #include 
>  #include 
> @@ -184,6 +185,7 @@ __rte_experimental
>  static inline void
>  rte_pmd_ifd_dump_proto_xtr_metadata(struct rte_mbuf *m)
>  {
> +#ifdef ALLOW_EXPERIMENTAL_API
>   union rte_pmd_ifd_proto_xtr_metadata data;
>  
>   if (!rte_pmd_ifd_dynf_proto_xtr_metadata_avail())
> @@ -243,6 +245,10 @@ rte_pmd_ifd_dump_proto_xtr_metadata(struct rte_mbuf *m)
>   else if (m->ol_flags & RTE_IAVF_PKT_RX_DYNF_PROTO_XTR_IP_OFFSET)
>   printf(" - Flexible descriptor's Extraction: ip_offset=%u",
>  data.ip_ofs);
> +#else
> + RTE_SET_USED(m);
> + RTE_VERIFY(false);

Is panicking the behaviour we want here? Seems rather severe, no?

> +#endif
>  }
>  
>  #ifdef __cplusplus
> diff --git a/drivers/net/mlx5/rte_pmd_mlx5.h b/drivers/net/mlx5/rte_pmd_mlx5.h
> index fdd2f65888..f2c6aebe0b 100644
> --- a/drivers/net/mlx5/rte_pmd_mlx5.h
> +++ b/drivers/net/mlx5/rte_pmd_mlx5.h
> @@ -5,6 +5,9 @@
>  #ifndef RTE_PMD_PRIVATE_MLX5_H_
>  #define RTE_PMD_PRIVATE_MLX5_H_
>  
> +#include 
> +
> +#include 
>  #include 
>  
>  /**
> diff --git a/drivers/raw/cnxk_bphy/rte_pmd_bphy.h 
> b/drivers/raw/cnxk_bphy/rte_pmd_bphy.h
> index f668e6ea82..c200c935ff 100644
> --- a/drivers/raw/cnxk_bphy/rte_pmd_bphy.h
> +++ b/drivers/raw/cnxk_bphy/rte_pmd_bphy.h
> @@ -391,6 +391,7 @@ rte_pmd_bphy_intr_init(uint16_t dev_id)
>  {
>   struct cnxk_bphy_irq_msg msg = {
>   .type = CNXK_BPHY_IRQ_MSG_TYPE_INIT,
> + .data = NULL,
>   };
> 

Why is this addition necessary? Is it for C++ compile?
 
>   return __rte_pmd_bphy_enq_deq(dev_id, CNXK_BPHY_DEF_QUEUE, &msg,
> @@ -411,6 +412,7 @@ rte_pmd_bphy_intr_fini(uint16_t dev_id)
>  {
>   struct cnxk_bphy_irq_msg msg = {
>   .type = CNXK_BPHY_IRQ_MSG_TYPE_FINI,
> + .data = NULL,
>   };
>  
>   return __rte_pmd_bphy_enq_deq(dev_id, CNXK_BPHY_DEF_QUE

Re: [PATCH v2 0/6] Add a stricter headers check

[Bruce Richardson]

On Fri, Dec 13, 2024 at 11:50:04AM +0100, David Marchand wrote:
> As explained in patch 6, the current headers check can not catch
> issues when a public header includes an internal header.
> Fixing this from meson does not seem an easy task.
> 
> This series approach is to reimplement the check in a Makefile invoked
> out of DPDK (like what is done for external compilation of examples).
> This has the advantage of being simple, and avoiding any (non intentional)
> implicit include path coming from the meson framework.
> 
> As there was no easy way to distinguish "indirect" headers in an
> installed DPDK, I chose to move those headers in a new sub directory
> (patch 5).
> 
> Patch 1-4 fixes have not been marked as backport material as those bugs
> seems minor/easy to fix externally (by either including missing headers,
> or enabling enable_driver_sdk option).
> 
> For now, I left the check_includes= option untouched, as there may be
> users of this check and this check still catches issues without
> requiring to install DPDK.
> 
For patches 5 & 6, I wonder if we can find a slightly different way to do
this. I like the idea of using make to build chkincs free from possible
environmental contamination from meson, but I really don't like the
complexity of the resulting makefile!

Rather than having to move the indirect headers to a subdirectory, and then
have a makefile run a scan from the headers directory, how about instead
generating the makefile (or possibly a build.ninja file) directly from
meson itself? This means the makefile can already have the list of headers
and C files necessary - no need for an extra subdirectory - and no need for
large amounts of wildcard matching and replacements.

The downside is that the makefile is no longer with the source, but in the
build directory. However, since the most likely use for this makefile is
from the test-meson-builds script and from automated CIs, I don't see this
being a major issue.

WDYT?

/Bruce

[PATCH 0/1] app/test: remove useless calls to rte_bitmap_free

[Ariel Otilibili]

Hello,

This patch fixes the Coverity IDs 357712 & 357737.

Afterwards the application compiles without warnings.

Thank you,

Ariel Otilibili (1):
  app/test: remove useless calls to rte_bitmap_free

 app/test/test_bitmap.c | 2 --
 1 file changed, 2 deletions(-)

-- 
2.47.1

[PATCH 1/1] app/test: remove useless calls to rte_bitmap_free

[Ariel Otilibili]

* rte_bitmap_free is only useful for its return value
* and its return value is not used.

```
$ < lib/eal/include/rte_bitmap.h sed -ne '/bitmap_free/,/^}/p'

rte_bitmap_free(struct rte_bitmap *bmp)
{
/* Check input arguments */
if (bmp == NULL) {
return -1;
}

return 0;
}
```

Reported-by: Coverity, IDs 357712 & 357737
Signed-off-by: Ariel Otilibili 
---
 app/test/test_bitmap.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/app/test/test_bitmap.c b/app/test/test_bitmap.c
index bab11812c7..a21210a215 100644
--- a/app/test/test_bitmap.c
+++ b/app/test/test_bitmap.c
@@ -208,7 +208,6 @@ test_bitmap_all_clear(void)
if (test_bitmap_scan_operations(bmp) < 0)
return TEST_FAILED;
 
-   rte_bitmap_free(bmp);
rte_free(mem);
 
return TEST_SUCCESS;
@@ -254,7 +253,6 @@ test_bitmap_all_set(void)
return TEST_FAILED;
}
 
-   rte_bitmap_free(bmp);
rte_free(mem);
 
return TEST_SUCCESS;
-- 
2.47.1

[PATCH v1 1/2] crypto/ipsec_mb: add SM4 GCM support

[Brian Dooley]

This patch introduces SM4 GCM algorithm support to the AESNI_MB PMD.
SM4 GCM is available in the v2.0 release of Intel IPsec MB.

Signed-off-by: Brian Dooley 
---
 doc/guides/cryptodevs/aesni_mb.rst  |  1 +
 doc/guides/cryptodevs/features/aesni_mb.ini |  1 +
 doc/guides/cryptodevs/features/default.ini  |  2 ++
 doc/guides/rel_notes/release_25_03.rst  |  4 +++
 drivers/crypto/ipsec_mb/pmd_aesni_mb.c  | 37 +++--
 drivers/crypto/ipsec_mb/pmd_aesni_mb_priv.h | 30 +
 lib/cryptodev/rte_crypto_sym.h  |  4 ++-
 lib/cryptodev/rte_cryptodev.c   |  3 +-
 8 files changed, 78 insertions(+), 4 deletions(-)

diff --git a/doc/guides/cryptodevs/aesni_mb.rst 
b/doc/guides/cryptodevs/aesni_mb.rst
index 16d82147b2..8d7e221e79 100644
--- a/doc/guides/cryptodevs/aesni_mb.rst
+++ b/doc/guides/cryptodevs/aesni_mb.rst
@@ -67,6 +67,7 @@ AEAD algorithms:
 * RTE_CRYPTO_AEAD_AES_CCM
 * RTE_CRYPTO_AEAD_AES_GCM
 * RTE_CRYPTO_AEAD_CHACHA20_POLY1305
+* RTE_CRYPTO_AEAD_SM4_GCM
 
 Protocol offloads:
 
diff --git a/doc/guides/cryptodevs/features/aesni_mb.ini 
b/doc/guides/cryptodevs/features/aesni_mb.ini
index ebe00d075d..c648be62fb 100644
--- a/doc/guides/cryptodevs/features/aesni_mb.ini
+++ b/doc/guides/cryptodevs/features/aesni_mb.ini
@@ -80,6 +80,7 @@ AES GCM (128) = Y
 AES GCM (192) = Y
 AES GCM (256) = Y
 CHACHA20-POLY1305 = Y
+SM4 GCM   = Y
 ;
 ; Supported Asymmetric algorithms of the 'aesni_mb' crypto driver.
 ;
diff --git a/doc/guides/cryptodevs/features/default.ini 
b/doc/guides/cryptodevs/features/default.ini
index 592af48026..116ffce249 100644
--- a/doc/guides/cryptodevs/features/default.ini
+++ b/doc/guides/cryptodevs/features/default.ini
@@ -118,6 +118,8 @@ AES CCM (128) =
 AES CCM (192) =
 AES CCM (256) =
 CHACHA20-POLY1305 =
+SM4 GCM   =
+
 ;
 ; Supported Asymmetric algorithms of a default crypto driver.
 ;
diff --git a/doc/guides/rel_notes/release_25_03.rst 
b/doc/guides/rel_notes/release_25_03.rst
index 426dfcd982..6f2b0bb5cb 100644
--- a/doc/guides/rel_notes/release_25_03.rst
+++ b/doc/guides/rel_notes/release_25_03.rst
@@ -55,6 +55,10 @@ New Features
  Also, make sure to start the actual text at the margin.
  ===
 
+**Updated IPsec_MB crypto driver.**
+
+   * Added support for the SM4 GCM algorithm.
+
 
 Removed Items
 -
diff --git a/drivers/crypto/ipsec_mb/pmd_aesni_mb.c 
b/drivers/crypto/ipsec_mb/pmd_aesni_mb.c
index 05dc1a039f..1bb47fb5ad 100644
--- a/drivers/crypto/ipsec_mb/pmd_aesni_mb.c
+++ b/drivers/crypto/ipsec_mb/pmd_aesni_mb.c
@@ -20,7 +20,8 @@ is_aead_algo(IMB_HASH_ALG hash_alg, IMB_CIPHER_MODE 
cipher_mode)
 {
return (hash_alg == IMB_AUTH_CHACHA20_POLY1305 ||
hash_alg == IMB_AUTH_AES_CCM ||
-   cipher_mode == IMB_CIPHER_GCM);
+   cipher_mode == IMB_CIPHER_GCM ||
+   cipher_mode == IMB_CIPHER_SM4_GCM);
 }
 
 /** Set session authentication parameters */
@@ -602,7 +603,7 @@ aesni_mb_set_session_cipher_parameters(const IMB_MGR 
*mb_mgr,
 }
 
 static int
-aesni_mb_set_session_aead_parameters(const IMB_MGR *mb_mgr,
+aesni_mb_set_session_aead_parameters(IMB_MGR *mb_mgr,
struct aesni_mb_session *sess,
const struct rte_crypto_sym_xform *xform)
 {
@@ -720,6 +721,21 @@ aesni_mb_set_session_aead_parameters(const IMB_MGR *mb_mgr,
return -EINVAL;
}
break;
+
+   case RTE_CRYPTO_AEAD_SM4_GCM:
+   sess->template_job.cipher_mode = IMB_CIPHER_SM4_GCM;
+   sess->template_job.hash_alg = IMB_AUTH_SM4_GCM;
+   sess->template_job.u.GCM.aad_len_in_bytes = 
xform->aead.aad_length;
+
+   if (xform->aead.key.length != 16) {
+   IPSEC_MB_LOG(ERR, "Invalid key length");
+   return -EINVAL;
+   }
+   sess->template_job.key_len_in_bytes = 16;
+   imb_sm4_gcm_pre(mb_mgr, xform->aead.key.data, 
&sess->cipher.gcm_key);
+   sess->template_job.enc_keys = &sess->cipher.gcm_key;
+   sess->template_job.dec_keys = &sess->cipher.gcm_key;
+   break;
default:
IPSEC_MB_LOG(ERR, "Unsupported aead mode parameter");
return -ENOTSUP;
@@ -1559,6 +1575,9 @@ set_mb_job_params(IMB_JOB *job, struct ipsec_mb_qp *qp,
imb_set_session(mb_mgr, job);
}
break;
+   case IMB_AUTH_SM4_GCM:
+   job->u.GCM.aad = op->sym->aead.aad.data;
+   break;
default:
break;
}
@@ -1687,6 +1706,17 @@ set_mb_job_params(IMB_JOB *job, struct ipsec_mb_qp *qp,
job->iv = rte_crypto_op_ctod_offset(op, uint8_t *,
session->iv.offset);
break;
+   case IMB_AUTH_SM4_GCM:
+   job->hash_start

[PATCH v1 2/2] app/test: add SM4 GCM tests

[Brian Dooley]

Added SM4-GCM tests for the AESNI MB PMD.

Signed-off-by: Brian Dooley 
---
 app/test/test_cryptodev.c   | 158 +
 app/test/test_cryptodev_aead_test_vectors.h | 708 
 2 files changed, 866 insertions(+)

diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index a33ef574cc..5e23f30286 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -1143,6 +1143,35 @@ chacha20_poly1305_testsuite_setup(void)
return 0;
 }
 
+static int
+sm4_gcm_testsuite_setup(void)
+{
+   struct crypto_testsuite_params *ts_params = &testsuite_params;
+   uint8_t dev_id = ts_params->valid_devs[0];
+   struct rte_cryptodev_info dev_info;
+   const enum rte_crypto_aead_algorithm aeads[] = {
+   RTE_CRYPTO_AEAD_SM4_GCM
+   };
+
+   rte_cryptodev_info_get(dev_id, &dev_info);
+
+   if (!(dev_info.feature_flags & RTE_CRYPTODEV_FF_SYMMETRIC_CRYPTO) ||
+   ((global_api_test_type == CRYPTODEV_RAW_API_TEST) &&
+   !(dev_info.feature_flags & 
RTE_CRYPTODEV_FF_SYM_RAW_DP))) {
+   RTE_LOG(INFO, USER1, "Feature flag requirements for "
+   "SM4 GCM testsuite not met\n");
+   return TEST_SKIPPED;
+   }
+
+   if (check_aead_capabilities_supported(aeads, RTE_DIM(aeads)) != 0) {
+   RTE_LOG(INFO, USER1, "Capability requirements for "
+   "SM4 GCM testsuite not met\n");
+   return TEST_SKIPPED;
+   }
+
+   return 0;
+}
+
 static int
 snow3g_testsuite_setup(void)
 {
@@ -17490,6 +17519,96 @@ test_chacha20_poly1305_encrypt_SGL_out_of_place(void)
chacha20_poly1305_case_2.plaintext.len);
 }
 
+static int
+test_SM4_GCM_case_1(void)
+{
+   return test_authenticated_encryption(&sm4_gcm_case_1);
+}
+
+static int
+test_SM4_GCM_case_2(void)
+{
+   return test_authenticated_encryption(&sm4_gcm_case_2);
+}
+
+static int
+test_SM4_GCM_case_3(void)
+{
+   return test_authenticated_encryption(&sm4_gcm_case_3);
+}
+
+static int
+test_SM4_GCM_case_4(void)
+{
+   return test_authenticated_encryption(&sm4_gcm_case_4);
+}
+
+static int
+test_SM4_GCM_case_5(void)
+{
+   return test_authenticated_encryption(&sm4_gcm_case_5);
+}
+
+static int
+test_SM4_GCM_case_6(void)
+{
+   return test_authenticated_encryption(&sm4_gcm_case_6);
+}
+
+static int
+test_SM4_GCM_case_7(void)
+{
+   return test_authenticated_encryption(&sm4_gcm_case_7);
+}
+
+static int
+test_SM4_GCM_case_8(void)
+{
+   return test_authenticated_encryption(&sm4_gcm_case_8);
+}
+
+static int
+test_SM4_GCM_case_9(void)
+{
+   return test_authenticated_encryption(&sm4_gcm_case_9);
+}
+
+static int
+test_SM4_GCM_case_10(void)
+{
+   return test_authenticated_encryption(&sm4_gcm_case_10);
+}
+
+static int
+test_SM4_GCM_case_11(void)
+{
+   return test_authenticated_encryption(&sm4_gcm_case_11);
+}
+
+static int
+test_SM4_GCM_case_12(void)
+{
+   return test_authenticated_encryption(&sm4_gcm_case_12);
+}
+
+static int
+test_SM4_GCM_case_13(void)
+{
+   return test_authenticated_encryption(&sm4_gcm_case_13);
+}
+
+static int
+test_SM4_GCM_case_14(void)
+{
+   return test_authenticated_encryption(&sm4_gcm_case_14);
+}
+
+static int
+test_SM4_GCM_case_15(void)
+{
+   return test_authenticated_encryption(&sm4_gcm_case_15);
+}
+
 #ifdef RTE_CRYPTO_SCHEDULER
 
 /* global AESNI worker IDs for the scheduler test */
@@ -19598,6 +19717,44 @@ static struct unit_test_suite 
cryptodev_mixed_cipher_hash_testsuite  = {
}
 };
 
+static struct unit_test_suite cryptodev_sm4_gcm_testsuite  = {
+   .suite_name = "SM4 GCM Test Suite",
+   .setup = sm4_gcm_testsuite_setup,
+   .unit_test_cases = {
+   TEST_CASE_ST(ut_setup, ut_teardown,
+   test_SM4_GCM_case_1),
+   TEST_CASE_ST(ut_setup, ut_teardown,
+   test_SM4_GCM_case_2),
+   TEST_CASE_ST(ut_setup, ut_teardown,
+   test_SM4_GCM_case_3),
+   TEST_CASE_ST(ut_setup, ut_teardown,
+   test_SM4_GCM_case_4),
+   TEST_CASE_ST(ut_setup, ut_teardown,
+   test_SM4_GCM_case_5),
+   TEST_CASE_ST(ut_setup, ut_teardown,
+   test_SM4_GCM_case_6),
+   TEST_CASE_ST(ut_setup, ut_teardown,
+   test_SM4_GCM_case_7),
+   TEST_CASE_ST(ut_setup, ut_teardown,
+   test_SM4_GCM_case_8),
+   TEST_CASE_ST(ut_setup, ut_teardown,
+   test_SM4_GCM_case_9),
+   TEST_CASE_ST(ut_setup, ut_teardown,
+   test_SM4_GCM_case_10),
+   TEST_CASE_ST(ut_setup, ut_teardown,
+   test_SM4_GCM_case_11),
+   TEST_CASE_ST(ut_setup, ut_teardown,
+   test_SM4_GCM_case_12),
+   TEST

Re: [PATCH 1/1] app/test: remove useless calls to rte_bitmap_free

[Ariel Otilibili-Anieli]

On Friday, December 13, 2024 17:59 CET, Stephen Hemminger 
 wrote:

> On Fri, 13 Dec 2024 12:30:00 +0100
> Ariel Otilibili  wrote:
> 
> > * rte_bitmap_free is only useful for its return value
> > * and its return value is not used.
> > 
> > ```
> > $ < lib/eal/include/rte_bitmap.h sed -ne '/bitmap_free/,/^}/p'
> > 
> > rte_bitmap_free(struct rte_bitmap *bmp)
> > {
> > /* Check input arguments */
> > if (bmp == NULL) {
> > return -1;
> > }
> > 
> > return 0;
> > }
> > ```
> > 
> > Reported-by: Coverity, IDs 357712 & 357737
> > Signed-off-by: Ariel Otilibili 
> > ---
> >  app/test/test_bitmap.c | 2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/app/test/test_bitmap.c b/app/test/test_bitmap.c
> > index bab11812c7..a21210a215 100644
> > --- a/app/test/test_bitmap.c
> > +++ b/app/test/test_bitmap.c
> > @@ -208,7 +208,6 @@ test_bitmap_all_clear(void)
> > if (test_bitmap_scan_operations(bmp) < 0)
> > return TEST_FAILED;
> >  
> > -   rte_bitmap_free(bmp);
> > rte_free(mem);
> >  
> > return TEST_SUCCESS;
> > @@ -254,7 +253,6 @@ test_bitmap_all_set(void)
> > return TEST_FAILED;
> > }
> >  
> > -   rte_bitmap_free(bmp);
> > rte_free(mem);
> >  
> > return TEST_SUCCESS;
> 
> I would rather that, rte_bitmap_free() be made a real function
> which would shutup Coverity.
> 
> And rte_bitmap_free() should just be a void function, like all the
> other XXX_free() variants in DPDK.

I will work on a Version 3 of this series, Stephen; your feedback landed into 
Version 1.

For my understanding, on which of the XXX_free should I model a rte_bitmap_free 
returning void?

Here are the occurrences of rte_bitmap_free()

```
$ git grep -Pn 'rte_bitmap_free\('

app/test/test_bitmap.c:211: rte_bitmap_free(bmp);
app/test/test_bitmap.c:257: rte_bitmap_free(bmp);
devtools/cocci/nullfree.cocci:19:- if (E != NULL) rte_bitmap_free(E);
devtools/cocci/nullfree.cocci:20:+ rte_bitmap_free(E);
drivers/common/mlx5/mlx5_common_mr.c:497:   rte_bitmap_free(mr->ms_bmp);
drivers/crypto/ionic/ionic_crypto_main.c:817:   
rte_bitmap_free(dev->sess_bm);
drivers/net/bonding/rte_eth_bond_pmd.c:2246:
rte_bitmap_free(internals->vlan_filter_bmp);
drivers/net/cxgbe/cxgbe_main.c:468: rte_bitmap_free(t->ftid_bmap);
drivers/net/mlx4/mlx4_mr.c:474: rte_bitmap_free(mr->ms_bmp);
drivers/net/netvsc/hn_rxtx.c:186:   rte_bitmap_free(hv->chim_bmap);
drivers/net/sfc/sfc_sw_stats.c:818: 
rte_bitmap_free(sa->sw_stats.queues_bitmap);
lib/eal/include/rte_bitmap.h:291:rte_bitmap_free(struct rte_bitmap *bmp)
```

[PATCH 3/5] net/dpaa2: remove unneeded deferred start check

[Stephen Hemminger]

Already checked in ethdev now.

Signed-off-by: Stephen Hemminger 
---
 drivers/net/dpaa2/dpaa2_ethdev.c | 14 --
 1 file changed, 14 deletions(-)

diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c
index a9bce854c3..8624bba0ce 100644
--- a/drivers/net/dpaa2/dpaa2_ethdev.c
+++ b/drivers/net/dpaa2/dpaa2_ethdev.c
@@ -726,13 +726,6 @@ dpaa2_dev_rx_queue_setup(struct rte_eth_dev *dev,
DPAA2_PMD_WARN("To use Normal buffers, run 'export 
DPNI_NORMAL_BUF=1' before running dynamic_dpl.sh script");
}
 
-   /* Rx deferred start is not supported */
-   if (rx_conf->rx_deferred_start) {
-   DPAA2_PMD_ERR("%s:Rx deferred start not supported",
-   dev->data->name);
-   return -EINVAL;
-   }
-
if (!priv->bp_list || priv->bp_list->mp != mb_pool) {
if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
ret = rte_dpaa2_bpid_info_init(mb_pool);
@@ -889,13 +882,6 @@ dpaa2_dev_tx_queue_setup(struct rte_eth_dev *dev,
 
PMD_INIT_FUNC_TRACE();
 
-   /* Tx deferred start is not supported */
-   if (tx_conf->tx_deferred_start) {
-   DPAA2_PMD_ERR("%s:Tx deferred start not supported",
-   dev->data->name);
-   return -EINVAL;
-   }
-
dpaa2_q->nb_desc = UINT16_MAX;
dpaa2_q->offloads = tx_conf->offloads;
 
-- 
2.45.2

[PATCH 4/5] net/enetfec: remove unneeded deferred start check

[Stephen Hemminger]

Already checked in ethdev now.

Signed-off-by: Stephen Hemminger 
---
 drivers/net/enetfec/enet_ethdev.c | 12 
 1 file changed, 12 deletions(-)

diff --git a/drivers/net/enetfec/enet_ethdev.c 
b/drivers/net/enetfec/enet_ethdev.c
index 91c0f60490..d8c81f4a70 100644
--- a/drivers/net/enetfec/enet_ethdev.c
+++ b/drivers/net/enetfec/enet_ethdev.c
@@ -377,12 +377,6 @@ enetfec_tx_queue_setup(struct rte_eth_dev *dev,
sizeof(struct bufdesc);
unsigned int dsize_log2 = rte_fls_u64(dsize) - 1;
 
-   /* Tx deferred start is not supported */
-   if (tx_conf->tx_deferred_start) {
-   ENETFEC_PMD_ERR("Tx deferred start not supported");
-   return -EINVAL;
-   }
-
/* allocate transmit queue */
txq = rte_zmalloc(NULL, sizeof(*txq), RTE_CACHE_LINE_SIZE);
if (txq == NULL) {
@@ -456,12 +450,6 @@ enetfec_rx_queue_setup(struct rte_eth_dev *dev,
sizeof(struct bufdesc);
unsigned int dsize_log2 = rte_fls_u64(dsize) - 1;
 
-   /* Rx deferred start is not supported */
-   if (rx_conf->rx_deferred_start) {
-   ENETFEC_PMD_ERR("Rx deferred start not supported");
-   return -EINVAL;
-   }
-
if (queue_idx >= ENETFEC_MAX_Q) {
ENETFEC_PMD_ERR("Invalid queue id %" PRIu16 ", max %d",
queue_idx, ENETFEC_MAX_Q);
-- 
2.45.2

[PATCH 0/5] centralize deferred start checks

[Stephen Hemminger]

Recent zxdh driver review raised the question of why should
drivers have to check rx_conf for deferred start support.
This can be better handled across all drivers at ethdev level.

Stephen Hemminger (5):
  ethdev: check that device supports deferred start
  net/dpaa: remove unnecessary deferred start check
  net/dpaa2: remove unneeded deferred start check
  net/enetfec: remove unneeded deferred start check
  net/virtio: remove unneeded deferred start check

 drivers/net/dpaa/dpaa_ethdev.c| 10 --
 drivers/net/dpaa2/dpaa2_ethdev.c  | 14 --
 drivers/net/enetfec/enet_ethdev.c | 12 
 drivers/net/virtio/virtio_rxtx.c  | 10 --
 lib/ethdev/rte_ethdev.c   | 10 ++
 5 files changed, 10 insertions(+), 46 deletions(-)

-- 
2.45.2

[PATCH 0/2] clear out Coverity issues 384444 & 451221

[Ariel Otilibili]

Hello,

This series clears out the Coverity issues 38 & 451221.

Being one-liners, as advised by David Marchand, it is backported onto stable.

Thank you,

Link: 
https://inbox.dpdk.org/dev/cajfav8yeawsx2kdianwukx7zsvtenrvovjnzaoq_ocdzm8z...@mail.gmail.com/

Ariel Otilibili (2):
  examples/flow_filtering: remove unused variable
  drivers/net/sfc: remove unused value

 drivers/net/sfc/sfc_repr.c| 2 --
 examples/flow_filtering/snippets/snippet_match_ipv4.c | 1 -
 2 files changed, 3 deletions(-)

-- 
2.47.1

[PATCH 2/5] net/dpaa: remove unnecessary deferred start check

[Stephen Hemminger]

This check is now done in ethdev layer.

Signed-off-by: Stephen Hemminger 
---
 drivers/net/dpaa/dpaa_ethdev.c | 10 --
 1 file changed, 10 deletions(-)

diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c
index e8d34e5898..ce48f0 100644
--- a/drivers/net/dpaa/dpaa_ethdev.c
+++ b/drivers/net/dpaa/dpaa_ethdev.c
@@ -1089,11 +1089,6 @@ int dpaa_eth_rx_queue_setup(struct rte_eth_dev *dev, 
uint16_t queue_idx,
return -rte_errno;
}
 
-   /* Rx deferred start is not supported */
-   if (rx_conf->rx_deferred_start) {
-   DPAA_PMD_ERR("%p:Rx deferred start not supported", (void *)dev);
-   return -EINVAL;
-   }
rxq->nb_desc = UINT16_MAX;
rxq->offloads = rx_conf->offloads;
 
@@ -1399,11 +1394,6 @@ int dpaa_eth_tx_queue_setup(struct rte_eth_dev *dev, 
uint16_t queue_idx,
 
PMD_INIT_FUNC_TRACE();
 
-   /* Tx deferred start is not supported */
-   if (tx_conf->tx_deferred_start) {
-   DPAA_PMD_ERR("%p:Tx deferred start not supported", (void *)dev);
-   return -EINVAL;
-   }
txq->nb_desc = UINT16_MAX;
txq->offloads = tx_conf->offloads;
 
-- 
2.45.2

[PATCH 5/5] net/virtio: remove unneeded deferred start check

[Stephen Hemminger]

Already checked in ethdev now.

Signed-off-by: Stephen Hemminger 
---
 drivers/net/virtio/virtio_rxtx.c | 10 --
 1 file changed, 10 deletions(-)

diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index b67f063b31..6ae3a6eb12 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -654,11 +654,6 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
 
PMD_INIT_FUNC_TRACE();
 
-   if (rx_conf->rx_deferred_start) {
-   PMD_INIT_LOG(ERR, "Rx deferred start is not supported");
-   return -EINVAL;
-   }
-
buf_size = virtio_rx_mem_pool_buf_size(mp);
if (!virtio_rx_check_scatter(hw->max_rx_pkt_len, buf_size,
 hw->rx_ol_scatter, &error)) {
@@ -819,11 +814,6 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
 
PMD_INIT_FUNC_TRACE();
 
-   if (tx_conf->tx_deferred_start) {
-   PMD_INIT_LOG(ERR, "Tx deferred start is not supported");
-   return -EINVAL;
-   }
-
if (nb_desc == 0 || nb_desc > vq->vq_nentries)
nb_desc = vq->vq_nentries;
vq->vq_free_cnt = RTE_MIN(vq->vq_free_cnt, nb_desc);
-- 
2.45.2

[PATCH 2/2] drivers/net/sfc: remove unused value

[Ariel Otilibili]

Coverity issue: 38
Fixes: a62ec90522a ("net/sfc: add port representors infrastructure")
Cc: Andrew Rybchenko 
Cc: sta...@dpdk.org
Signed-off-by: Ariel Otilibili 
---
 drivers/net/sfc/sfc_repr.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/sfc/sfc_repr.c b/drivers/net/sfc/sfc_repr.c
index c2e5d4eb9e..a0712bf9fb 100644
--- a/drivers/net/sfc/sfc_repr.c
+++ b/drivers/net/sfc/sfc_repr.c
@@ -769,9 +769,7 @@ static void
 sfc_repr_close(struct sfc_repr *sr)
 {
SFC_ASSERT(sfc_repr_lock_is_locked(sr));
-
SFC_ASSERT(sr->state == SFC_ETHDEV_CONFIGURED);
-   sr->state = SFC_ETHDEV_CLOSING;
 
/* Put representor close actions here */
 
-- 
2.47.1

Re: [PATCH v2 06/15] net/zxdh: dev start/stop ops implementations

[Stephen Hemminger]

On Tue, 10 Dec 2024 13:53:24 +0800
Junlong Wang  wrote:

> v start/stop implementations, start/stop the rx/tx queues.
> 
> Signed-off-by: Junlong Wang 
> ---

If I re-enable the warnings for format and unaligned packed then this shows up:

../drivers/net/zxdh/zxdh_queue.c: In function ‘zxdh_dev_rx_queue_setup_finish’:
../drivers/net/zxdh/zxdh_queue.c:321:59: warning: taking address of packed 
member of ‘struct zxdh_virtnet_rx’ may result in an unaligned pointer value 
[-Waddress-of-packed-member]
  321 | vq->sw_ring[vq->vq_nentries + desc_idx] = 
&rxvq->fake_mbuf;
  |   
^~~~

The problem is that the driver is using __rte_packed on structures like 
zxdh_virtnet_rx.

Unlike some other OS's. DPDK best practice is to only use packed where required
by the hardware. Please don't use __rte_packed unless needed.

To save space, it makes sense to reorder structure members to fill holes
and put hot members together for caching.

You should put fake_mbuf at the end and mark it with __rte_cache_aligned.

Re: [PATCH v2 03/15] net/zxdh: port tables init implementations

[Stephen Hemminger]

On Tue, 10 Dec 2024 13:53:21 +0800
Junlong Wang  wrote:

> insert port tables in host.
> 
> Signed-off-by: Junlong Wang 
> ---
>  drivers/net/zxdh/meson.build   |   1 +
>  drivers/net/zxdh/zxdh_ethdev.c |  23 ++
>  drivers/net/zxdh/zxdh_msg.c|  63 
>  drivers/net/zxdh/zxdh_msg.h|  72 
>  drivers/net/zxdh/zxdh_np.c | 662 +
>  drivers/net/zxdh/zxdh_np.h | 210 +++
>  drivers/net/zxdh/zxdh_pci.h|   2 +
>  drivers/net/zxdh/zxdh_tables.c | 104 ++
>  drivers/net/zxdh/zxdh_tables.h | 148 
>  9 files changed, 1285 insertions(+)
>  create mode 100644 drivers/net/zxdh/zxdh_tables.c
>  create mode 100644 drivers/net/zxdh/zxdh_tables.h

Is this a problem? 

### [PATCH] net/zxdh: port tables init implementations

WARNING:MACRO_ARG_UNUSED: Argument 'INDEX' is not used in function-like macro
#329: FILE: drivers/net/zxdh/zxdh_np.c:124:
+#define ZXDH_DTB_TAB_DOWN_PHY_ADDR_GET(DEV_ID, QUEUE_ID, INDEX)   \
+   
(p_dpp_dtb_mgr[(DEV_ID)]->queue_info[(QUEUE_ID)].tab_down.start_phy_addr)

total: 0 errors, 1 warnings, 0 checks, 1396 lines checked

Re: [PATCH v2 02/15] net/zxdh: zxdh np uninit implementation

[Stephen Hemminger]

On Tue, 10 Dec 2024 13:53:20 +0800
Junlong Wang  wrote:

> +static uint32_t
> +zxdh_np_comm_read_bits(uint8_t *p_base, uint32_t base_size_bit,
> + uint32_t *p_data, uint32_t start_bit, uint32_t end_bit)
> +{
> + uint32_t   len   =  0;
> + uint32_t   start_byte_index  =  0;
> + uint32_t   end_byte_index=  0;
> + uint32_t   byte_num  =  0;
> + uint32_t   buffer_size   =  0;
> +
> + if (0 != (base_size_bit % 8))
> + return 1;
> +
> + if (start_bit > end_bit)
> + return 1;
> +
> + if (base_size_bit < end_bit)
> + return 1;
> +
> + len = end_bit - start_bit + 1;

Another case of the "initialize everything" style.
len is set to zero when declared then first use assigns it.

Re: [PATCH v2 09/15] net/zxdh: link info update, set link up/down

[Stephen Hemminger]

On Tue, 10 Dec 2024 13:53:27 +0800
Junlong Wang  wrote:

> provided link info update, set link up /down,
> and link intr.
> 
> Signed-off-by: Junlong Wang 
> ---
>  doc/guides/nics/features/zxdh.ini  |   2 +
>  doc/guides/nics/zxdh.rst   |   3 +
>  drivers/net/zxdh/meson.build   |   1 +
>  drivers/net/zxdh/zxdh_ethdev.c |  13 ++
>  drivers/net/zxdh/zxdh_ethdev.h |   2 +
>  drivers/net/zxdh/zxdh_ethdev_ops.c | 166 ++
>  drivers/net/zxdh/zxdh_ethdev_ops.h |  14 +++
>  drivers/net/zxdh/zxdh_msg.c|  56 +
>  drivers/net/zxdh/zxdh_msg.h|  40 +++
>  drivers/net/zxdh/zxdh_np.c | 183 +
>  drivers/net/zxdh/zxdh_np.h |  20 
>  drivers/net/zxdh/zxdh_tables.c |  15 +++
>  drivers/net/zxdh/zxdh_tables.h |   3 +
>  13 files changed, 518 insertions(+)
>  create mode 100644 drivers/net/zxdh/zxdh_ethdev_ops.c
>  create mode 100644 drivers/net/zxdh/zxdh_ethdev_ops.h
> 
> diff --git a/doc/guides/nics/features/zxdh.ini 
> b/doc/guides/nics/features/zxdh.ini
> index bb44e93fad..7da3aaced1 100644
> --- a/doc/guides/nics/features/zxdh.ini
> +++ b/doc/guides/nics/features/zxdh.ini
> @@ -10,3 +10,5 @@ ARMv8= Y
>  SR-IOV   = Y
>  Multiprocess aware   = Y
>  Scattered Rx = Y
> +Link status  = Y
> +Link status event= Y
> diff --git a/doc/guides/nics/zxdh.rst b/doc/guides/nics/zxdh.rst
> index f42db9c1f1..fdbc3b3923 100644
> --- a/doc/guides/nics/zxdh.rst
> +++ b/doc/guides/nics/zxdh.rst
> @@ -21,6 +21,9 @@ Features of the ZXDH PMD are:
>  - Multiple queues for TX and RX
>  - SR-IOV VF
>  - Scattered and gather for TX and RX
> +- Link Auto-negotiation
> +- Link state information
> +- Set Link down or up
>  
>  
>  Driver compilation and testing
> diff --git a/drivers/net/zxdh/meson.build b/drivers/net/zxdh/meson.build
> index 20b2cf484a..48f8f5e1ee 100644
> --- a/drivers/net/zxdh/meson.build
> +++ b/drivers/net/zxdh/meson.build
> @@ -22,4 +22,5 @@ sources = files(
>  'zxdh_np.c',
>  'zxdh_tables.c',
>  'zxdh_rxtx.c',
> +'zxdh_ethdev_ops.c',
>  )
> diff --git a/drivers/net/zxdh/zxdh_ethdev.c b/drivers/net/zxdh/zxdh_ethdev.c
> index acf11adb9e..3636da2184 100644
> --- a/drivers/net/zxdh/zxdh_ethdev.c
> +++ b/drivers/net/zxdh/zxdh_ethdev.c
> @@ -16,6 +16,7 @@
>  #include "zxdh_np.h"
>  #include "zxdh_tables.h"
>  #include "zxdh_rxtx.h"
> +#include "zxdh_ethdev_ops.h"
>  
>  struct zxdh_hw_internal zxdh_hw_internal[RTE_MAX_ETHPORTS];
>  struct zxdh_shared_data *zxdh_shared_data;
> @@ -105,9 +106,16 @@ static void
>  zxdh_devconf_intr_handler(void *param)
>  {
>   struct rte_eth_dev *dev = param;
> + struct zxdh_hw *hw = dev->data->dev_private;
> +
> + uint8_t isr = zxdh_pci_isr(hw);
>  
>   if (zxdh_intr_unmask(dev) < 0)
>   PMD_DRV_LOG(ERR, "interrupt enable failed");
> + if (isr & ZXDH_PCI_ISR_CONFIG) {
> + if (zxdh_dev_link_update(dev, 0) == 0)
> + rte_eth_dev_callback_process(dev, 
> RTE_ETH_EVENT_INTR_LSC, NULL);
> + }
>  }
>  
>  
> @@ -1007,6 +1015,8 @@ zxdh_dev_start(struct rte_eth_dev *dev)
>   vq = hw->vqs[logic_qidx];
>   zxdh_queue_notify(vq);
>   }
> + zxdh_dev_set_link_up(dev);
> +
>   return 0;
>  }
>  
> @@ -1021,6 +1031,9 @@ static const struct eth_dev_ops zxdh_eth_dev_ops = {
>   .tx_queue_setup  = zxdh_dev_tx_queue_setup,
>   .rx_queue_intr_enable= zxdh_dev_rx_queue_intr_enable,
>   .rx_queue_intr_disable   = zxdh_dev_rx_queue_intr_disable,
> + .link_update = zxdh_dev_link_update,
> + .dev_set_link_up = zxdh_dev_set_link_up,
> + .dev_set_link_down   = zxdh_dev_set_link_down,
>  };
>  
>  static int32_t
> diff --git a/drivers/net/zxdh/zxdh_ethdev.h b/drivers/net/zxdh/zxdh_ethdev.h
> index 78b1edd5a4..ac8fd2c294 100644
> --- a/drivers/net/zxdh/zxdh_ethdev.h
> +++ b/drivers/net/zxdh/zxdh_ethdev.h
> @@ -69,6 +69,7 @@ struct zxdh_hw {
>   uint64_t guest_features;
>   uint32_t max_queue_pairs;
>   uint32_t speed;
> + uint32_t speed_mode;
>   uint32_t notify_off_multiplier;
>   uint16_t *notify_base;
>   uint16_t pcie_id;
> @@ -90,6 +91,7 @@ struct zxdh_hw {
>   uint8_t panel_id;
>   uint8_t has_tx_offload;
>   uint8_t has_rx_offload;
> + uint8_t admin_status;
>  };
>  
>  struct zxdh_dtb_shared_data {
> diff --git a/drivers/net/zxdh/zxdh_ethdev_ops.c 
> b/drivers/net/zxdh/zxdh_ethdev_ops.c
> new file mode 100644
> index 00..635868c4c0
> --- /dev/null
> +++ b/drivers/net/zxdh/zxdh_ethdev_ops.c
> @@ -0,0 +1,166 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2024 ZTE Corporation
> + */
> +
> +#include "zxdh_ethdev.h"
> +#include "zxdh_pci.h"
> +#include "zxdh_msg.h"
> +#include "zxdh_ethdev_ops.h"
> +#include "zxdh_tables.h"
> +#include "zxdh_logs.h"
> +
> +static

Re: [PATCH v2 02/15] net/zxdh: zxdh np uninit implementation

[Stephen Hemminger]

On Tue, 10 Dec 2024 13:53:20 +0800
Junlong Wang  wrote:

> (np)network processor release resources in host.
> 
> Signed-off-by: Junlong Wang 
> ---
>  drivers/net/zxdh/zxdh_ethdev.c |  48 
>  drivers/net/zxdh/zxdh_np.c | 494 -
>  drivers/net/zxdh/zxdh_np.h | 107 +++
>  3 files changed, 647 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/zxdh/zxdh_ethdev.c b/drivers/net/zxdh/zxdh_ethdev.c
> index c54d1f6669..b28ea4ae6f 100644
> --- a/drivers/net/zxdh/zxdh_ethdev.c
> +++ b/drivers/net/zxdh/zxdh_ethdev.c
> @@ -841,6 +841,51 @@ zxdh_dev_configure(struct rte_eth_dev *dev)
>   return ret;
>  }
>  
> +static void
> +zxdh_np_dtb_data_res_free(struct zxdh_hw *hw)
> +{
> + struct rte_eth_dev *dev = hw->eth_dev;
> + int ret = 0;
> + int i = 0;
> +

Why initialize these variables (ret and i)?
They are set immediately later in the loop.

Programmers are often taught to always initialize all variables,
but doing so defeats the checking in modern compilers that are able
to detect variables that are used uninitialized.

Re: [PATCH v2 09/15] net/zxdh: link info update, set link up/down

[Stephen Hemminger]

On Tue, 10 Dec 2024 13:53:27 +0800
Junlong Wang  wrote:

> provided link info update, set link up /down,
> and link intr.
> 
> Signed-off-by: Junlong Wang 
> ---
>  doc/guides/nics/features/zxdh.ini  |   2 +
>  doc/guides/nics/zxdh.rst   |   3 +
>  drivers/net/zxdh/meson.build   |   1 +
>  drivers/net/zxdh/zxdh_ethdev.c |  13 ++
>  drivers/net/zxdh/zxdh_ethdev.h |   2 +
>  drivers/net/zxdh/zxdh_ethdev_ops.c | 166 ++
>  drivers/net/zxdh/zxdh_ethdev_ops.h |  14 +++
>  drivers/net/zxdh/zxdh_msg.c|  56 +
>  drivers/net/zxdh/zxdh_msg.h|  40 +++
>  drivers/net/zxdh/zxdh_np.c | 183 +
>  drivers/net/zxdh/zxdh_np.h |  20 
>  drivers/net/zxdh/zxdh_tables.c |  15 +++
>  drivers/net/zxdh/zxdh_tables.h |   3 +
>  13 files changed, 518 insertions(+)
>  create mode 100644 drivers/net/zxdh/zxdh_ethdev_ops.c
>  create mode 100644 drivers/net/zxdh/zxdh_ethdev_ops.h

WARNING:MACRO_ARG_UNUSED: Argument '_uisrc_' is not used in function-like macro
#486: FILE: drivers/net/zxdh/zxdh_np.c:43:
+#define ZXDH_COMM_UINT32_WRITE_BITS(_uidst_, _uisrc_, _uistartpos_, _uilen_)\
+   (((_uidst_) & ~(ZXDH_COMM_GET_BIT_MASK(uint32_t, (_uilen_)) << 
(_uistartpos_

total: 0 errors, 1 warnings, 0 checks, 664 lines checked

Re: [PATCH v2 04/15] net/zxdh: port tables unint implementations

[Stephen Hemminger]

On Tue, 10 Dec 2024 13:53:22 +0800
Junlong Wang  wrote:

> elete port tables in host.
> 
> Signed-off-by: Junlong Wang 
> ---
>  drivers/net/zxdh/zxdh_ethdev.c |  19 ++
>  drivers/net/zxdh/zxdh_msg.h|   1 +
>  drivers/net/zxdh/zxdh_np.c | 113 +
>  drivers/net/zxdh/zxdh_np.h |   9 +++
>  drivers/net/zxdh/zxdh_tables.c |  36 ++-
>  drivers/net/zxdh/zxdh_tables.h |   1 +
>  6 files changed, 177 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/zxdh/zxdh_ethdev.c b/drivers/net/zxdh/zxdh_ethdev.c
> index 8a9ca87183..a72319758a 100644
> --- a/drivers/net/zxdh/zxdh_ethdev.c
> +++ b/drivers/net/zxdh/zxdh_ethdev.c
> @@ -887,12 +887,31 @@ zxdh_np_uninit(struct rte_eth_dev *dev)
>   zxdh_np_dtb_data_res_free(hw);
>  }
>  
> +static int
> +zxdh_tables_uninit(struct rte_eth_dev *dev)
> +{
> + int ret = 0;
> +
> + ret = zxdh_port_attr_uninit(dev);
> + if (ret) {
> + PMD_DRV_LOG(ERR, "zxdh_port_attr_uninit failed");
> + return ret;
> + }
> + return ret;
> +}
> +

Could be simplified to:

static int
zxdh_tables_uninit(struct rte_eth_dev *dev)
{
int ret;

ret = zxdh_port_attr_uninit(dev);
if (ret)
PMD_DRV_LOG(ERR, "zxdh_port_attr_uninit failed");

return ret;
}

[PATCH 1/5] ethdev: check that device supports deferred start

[Stephen Hemminger]

The check for supporting deferred start should be handled at
the ethdev level for all devices.

Signed-off-by: Stephen Hemminger 
---
 lib/ethdev/rte_ethdev.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 6413c54e3b..7768058f6d 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -2264,6 +2264,11 @@ rte_eth_rx_queue_setup(uint16_t port_id, uint16_t 
rx_queue_id,
if (rx_conf != NULL)
rx_offloads |= rx_conf->offloads;
 
+   /* Deferred start requires that device supports queue start */
+   if (rx_conf != NULL && rx_conf->rx_deferred_start &&
+   *dev->dev_ops->rx_queue_start == NULL)
+   return -ENOTSUP;
+
/* Ensure that we have one and only one source of Rx buffers */
if ((mp != NULL) +
(rx_conf != NULL && rx_conf->rx_nseg > 0) +
@@ -2575,6 +2580,11 @@ rte_eth_tx_queue_setup(uint16_t port_id, uint16_t 
tx_queue_id,
return -EINVAL;
}
 
+   /* Deferred start requires that device supports queue start */
+   if (tx_conf != NULL && tx_conf->tx_deferred_start &&
+   *dev->dev_ops->tx_queue_start == NULL)
+   return -ENOTSUP;
+
ret = rte_eth_dev_info_get(port_id, &dev_info);
if (ret != 0)
return ret;
-- 
2.45.2

[PATCH 1/2] examples/flow_filtering: remove unused variable

[Ariel Otilibili]

Coverity issue: 451221
Fixes: 16158f3490 ("examples/flow_filtering: introduce use cases snippets")
Cc: Ori Kam 
Cc: sta...@dpdk.org
Signed-off-by: Ariel Otilibili 
---
 examples/flow_filtering/snippets/snippet_match_ipv4.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/examples/flow_filtering/snippets/snippet_match_ipv4.c 
b/examples/flow_filtering/snippets/snippet_match_ipv4.c
index 808208e7b0..65fb045e8b 100644
--- a/examples/flow_filtering/snippets/snippet_match_ipv4.c
+++ b/examples/flow_filtering/snippets/snippet_match_ipv4.c
@@ -76,7 +76,6 @@ snippet_ipv4_flow_create_actions_template(uint16_t port_id, 
struct rte_flow_erro
.ingress = 1,
};
 
-   tactions[0].type = RTE_FLOW_ACTION_TYPE_QUEUE;
tactions[0].type = RTE_FLOW_ACTION_TYPE_END;
 
/* This sets the masks to match the actions, indicating that all fields 
of the actions
-- 
2.47.1

Re: 23.11.3 patches review and test

[Xueming Li]

Hi Ali,

Thanks very much for your help and confirmation!

Regards,
Xueming


From: Ali Alnubani 
Sent: Friday, December 13, 2024 12:25 AM
To: Xueming Li ; sta...@dpdk.org 
Cc: dev@dpdk.org ; Abhishek Marathe 
; David Christensen ; 
Hemant Agrawal ; Ian Stokes ; 
Jerin Jacob ; John McNamara ; 
Ju-Hyoung Lee ; Kevin Traynor ; Luca 
Boccassi ; Pei Zhang ; Raslan Darawsheh 
; NBU-Contact-Thomas Monjalon (EXTERNAL) 
; Yanghang Liu ; 
benjamin.wal...@intel.com ; qian.q...@intel.com 
; yuan.p...@intel.com ; 
zhaoyan.c...@intel.com 
Subject: RE: 23.11.3 patches review and test

> -Original Message-
> From: Xueming Li 
> Sent: Tuesday, December 10, 2024 4:49 PM
> To: sta...@dpdk.org
> Cc: dev@dpdk.org; Abhishek Marathe ; Ali
> Alnubani ; David Christensen ;
> Hemant Agrawal ; Ian Stokes
> ; Jerin Jacob ; John McNamara
> ; Ju-Hyoung Lee ; Kevin
> Traynor ; Luca Boccassi ; Pei Zhang
> ; Raslan Darawsheh ; NBU-
> Contact-Thomas Monjalon (EXTERNAL) ; Yanghang Liu
> ; benjamin.wal...@intel.com; qian.q...@intel.com;
> yuan.p...@intel.com; zhaoyan.c...@intel.com
> Subject: 23.11.3 patches review and test
>
> Hi all,
>
> Here is a list of patches targeted for stable release 23.11.3.
>
> The planned date for the final release is 17th December.
>
> Please help with testing and validation of your use cases and report
> any issues/results with reply-all to this mail. For the final release
> the fixes and reported validations will be added to the release notes.
>
> A release candidate tarball can be found at:
>
> https://dpdk.org/browse/dpdk-stable/tag/?id=v23.11.3-rc1
>
> These patches are located at branch 23.11 of dpdk-stable repo:
> https://dpdk.org/browse/dpdk-stable/
>
> Thanks.
>
> Xueming Li 
>
> ---

Hello,

We ran the following functional tests with Nvidia hardware on v23.11.3-rc1:
- Basic functionality:
  Send and receive multiple types of traffic.
- testpmd xstats counter test.
- testpmd timestamp test.
- Changing/checking link status through testpmd.
- rte_flow tests 
(https://doc.dpdk.org/guides/nics/mlx5.html#supported-hardware-offloads)
- RSS tests.
- VLAN filtering, stripping, and insertion tests.
- Checksum and TSO tests.
- ptype tests.
- link_status_interrupt example application tests.
- l3fwd-power example application tests.
- Multi-process example applications tests.
- Hardware LRO tests.
- Buffer Split tests.
- Tx scheduling tests.

Functional tests ran on:
- NIC: ConnectX-6 Dx / OS: Ubuntu 22.04 / Driver: MLNX_OFED_LINUX-24.10-1.1.4.0 
/ Firmware: 22.43.2026
- NIC: ConnectX-7 / OS: Ubuntu 22.04 / Driver: MLNX_OFED_LINUX-24.10-1.1.4.0 / 
Firmware: 28.43.2026
- DPU: BlueField-2 / DOCA SW version: 2.9.1 / Firmware: 24.43.2026


Additionally, we ran build tests with multiple configurations on the following 
OS/driver combinations (all passed):
- Debian 12 with MLNX_OFED_LINUX-24.10-1.1.4.0.
- Ubuntu 22.04 with MLNX_OFED_LINUX-24.10-1.1.4.0.
- Ubuntu 24.04 with MLNX_OFED_LINUX-24.10-1.1.4.0.
- Ubuntu 24.04 with rdma-core v50.0.
- Fedora 40 with rdma-core v48.0.
- Fedora 42 (Rawhide) with rdma-core v51.0.
- OpenSUSE Leap 15.6 with rdma-core v49.1.

I did not see new issues caused by the changes in this release.

Thanks,
Ali

Re: [PATCH 1/1] buildtools: clear out regex syntax warnings

[Ariel Otilibili-Anieli]

On Friday, December 13, 2024 15:07 CET, "Robin Jarry"  wrote:

> Ariel Otilibili, Dec 08, 2024 at 03:25:
> > * invalid escape sequences now generate SyntaxWarning
> > * therefore changed syntax to raw string noration.
> >
> > Link: https://docs.python.org/3/library/re.html#module-re
> > Signed-off-by: Ariel Otilibili 
> 
> Hi Ariel, thanks for the fix!
> 
> Acked-by: Robin Jarry 

Awesome, Robin!
> 
> If you have time, I there are other bits that would need the same fix:
> 
> buildtools/get-numa-count.py:14:numa_nodes.sort(key=lambda l: 
> int(re.findall('\d+', l)[0]))
> devtools/check-meson.py:54:if re.match('^ *\t', code):
> doc/api/generate_doxygen.py:8:pattern = re.compile('^Preprocessing (.*)...$')
> 

Good, then; I'll push them in a subsequent series.
> Cheers.
>

Re: [PATCH 1/1] app/test: remove useless calls to rte_bitmap_free

[Stephen Hemminger]

On Fri, 13 Dec 2024 12:30:00 +0100
Ariel Otilibili  wrote:

> * rte_bitmap_free is only useful for its return value
> * and its return value is not used.
> 
> ```
> $ < lib/eal/include/rte_bitmap.h sed -ne '/bitmap_free/,/^}/p'
> 
> rte_bitmap_free(struct rte_bitmap *bmp)
> {
> /* Check input arguments */
> if (bmp == NULL) {
> return -1;
> }
> 
> return 0;
> }
> ```
> 
> Reported-by: Coverity, IDs 357712 & 357737
> Signed-off-by: Ariel Otilibili 
> ---
>  app/test/test_bitmap.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/app/test/test_bitmap.c b/app/test/test_bitmap.c
> index bab11812c7..a21210a215 100644
> --- a/app/test/test_bitmap.c
> +++ b/app/test/test_bitmap.c
> @@ -208,7 +208,6 @@ test_bitmap_all_clear(void)
>   if (test_bitmap_scan_operations(bmp) < 0)
>   return TEST_FAILED;
>  
> - rte_bitmap_free(bmp);
>   rte_free(mem);
>  
>   return TEST_SUCCESS;
> @@ -254,7 +253,6 @@ test_bitmap_all_set(void)
>   return TEST_FAILED;
>   }
>  
> - rte_bitmap_free(bmp);
>   rte_free(mem);
>  
>   return TEST_SUCCESS;

I would rather that, rte_bitmap_free() be made a real function
which would shutup Coverity.

And rte_bitmap_free() should just be a void function, like all the
other XXX_free() variants in DPDK.

[PATCH v2 1/1] buildtools: clear out regex syntax warnings

[Ariel Otilibili]

* invalid escape sequences now generate SyntaxWarning
* therefore changed syntax to raw string noration.

Link: https://docs.python.org/3/library/re.html#module-re
Fixes: 0aeaf75df87 ("test: define unit tests suites based on test types")
Fixes: 25065ef1f6c ("test: emit warning for orphaned tests")
Cc: sta...@dpdk.org
Signed-off-by: Ariel Otilibili 
---
 buildtools/get-test-suites.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/buildtools/get-test-suites.py b/buildtools/get-test-suites.py
index c61f6a273f..fd22d25f36 100644
--- a/buildtools/get-test-suites.py
+++ b/buildtools/get-test-suites.py
@@ -6,10 +6,10 @@
 import re
 
 input_list = sys.argv[1:]
-test_def_regex = re.compile("REGISTER_([A-Z]+)_TEST\s*\(\s*([a-z0-9_]+)")
+test_def_regex = re.compile(r"REGISTER_([A-Z]+)_TEST\s*\(\s*([a-z0-9_]+)")
 test_suites = {}
 # track tests not in any test suite.
-non_suite_regex = re.compile("REGISTER_TEST_COMMAND\s*\(\s*([a-z0-9_]+)")
+non_suite_regex = re.compile(r"REGISTER_TEST_COMMAND\s*\(\s*([a-z0-9_]+)")
 non_suite_tests = []
 
 def get_fast_test_params(test_name, ln):
-- 
2.47.1

[PATCH v2 0/1] buildtools: clear out regex syntax warnings

[Ariel Otilibili]

Hello,

This is my first patch to the list; your feedback is much appreciated.

My aim is to understand the internals of DPDK.

While compiling from scratch, I came across these Python warnings.

Thank you,


Ariel Otilibili (1):
  buildtools: clear out regex syntax warnings

 buildtools/get-test-suites.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--
v2:
* redone commit message
* backport to stable

2.47.1

Re: [PATCH] devtools: enhance the license check

[David Marchand]

On Wed, Dec 11, 2024 at 4:01 PM Thomas Monjalon  wrote:
>
> 11/12/2024 15:55, Stephen Hemminger:
> > On Wed, 11 Dec 2024 10:00:38 +0100
> > David Marchand  wrote:
> >
> > > On Tue, Dec 10, 2024 at 6:00 PM Stephen Hemminger
> > >  wrote:
> > > >
> > > > On Tue, 10 Dec 2024 10:10:39 +0100
> > > > David Marchand  wrote:
> > > >
> > > > > +no_license_list=\
> > > > > +':^.git* :^.mailmap :^.ci/* :^README :^MAINTAINERS :^VERSION 
> > > > > :^ABI_VERSION :^*/Kbuild '\
> > > > > +':^*/README* :^license/ :^config/ :^buildtools/ :^*/poetry.lock '\
> > > > > +':^kernel/linux/uapi/.gitignore :^kernel/linux/uapi/version 
> > > > > :^*.cocci :^*.abignore '\
> > > > > +':^*.map :^*.ini :^*.data :^*.json :^*.cfg :^*.txt :^*.svg :^*.png'
> > > > > +
> > > >
> > > > What is poetry.lock?
> > >
> > > I don't know.
> > > It looks like some python packaging config for dts, and it is a generated 
> > > file.
> > > # This file is automatically @generated by Poetry 1.8.3 and should not
> > > be changed by hand.
> > >
> > > Cc: dts maintainers.
> > >
> > >
> >
> > Put it in .gitignore then please, and the script will ignore that.
>
> How is it related?
>
> I don't like adding versioned files to .gitignore.
> (this poetry file is in the repository)

Apart from this file, any other comment?


-- 
David Marchand

Re: 22.11.7 patches review and test

[Luca Boccassi]

On Fri, 13 Dec 2024 at 01:01, Xu, HailinX  wrote:
>
> > -Original Message-
> > From: luca.bocca...@gmail.com 
> > Sent: Sunday, December 1, 2024 8:24 AM
> > To: sta...@dpdk.org
> > Cc: dev@dpdk.org; Abhishek Marathe ;
> > Ali Alnubani ; David Christensen
> > ; Hemant Agrawal ;
> > Stokes, Ian ; Jerin Jacob ;
> > Mcnamara, John ; Ju-Hyoung Lee
> > ; Kevin Traynor ; Luca
> > Boccassi ; Pei Zhang ; Raslan
> > Darawsheh ; Thomas Monjalon
> > ; Yanghang Liu 
> > Subject: 22.11.7 patches review and test
> >
> > Hi all,
> >
> > Here is a list of patches targeted for stable release 22.11.7.
> >
> > The planned date for the final release is December 17th.
> >
> > Please help with testing and validation of your use cases and report any
> > issues/results with reply-all to this mail. For the final release the fixes 
> > and
> > reported validations will be added to the release notes.
> >
> > A release candidate tarball can be found at:
> >
> > https://dpdk.org/browse/dpdk-stable/tag/?id=v22.11.7-rc1
> >
> > These patches are located at branch 22.11 of dpdk-stable repo:
> > https://dpdk.org/browse/dpdk-stable/
> >
> > Thanks.
> >
> > Luca Boccassi
>
> Update the test status for Intel part. dpdk22.11.7-rc1 all validation test 
> done. Found 1 meson test bug.
>
> new issue:
> Bug 1589 - meson_tests/driver link_bonding_autotest test failed  -> This 
> issue also exists on 24.11
>
> # Basic Intel(R) NIC testing
> * Build & CFLAG compile: cover the build test combination with latest 
> GCC/Clang version and the popular OS revision such as
>   Ubuntu24.04, Ubuntu24.10, Fedora40, RHEL8.10, RHEL9.4, FreeBSD14.1, SUSE15, 
> AzureLinux3.0, OpenAnolis8.9 etc.
> - All test done. No new dpdk issue is found.
> * PF(i40e, ixgbe): test scenarios including RTE_FLOW/TSO/Jumboframe/checksum 
> offload/VLAN/VXLAN, etc.
> - All test done. Found 1 meson test bug.
> * VF(i40e, ixgbe): test scenarios including 
> VF-RTE_FLOW/TSO/Jumboframe/checksum offload/VLAN/VXLAN, etc.
> - All test done. Found 1 meson test bug.
> * PF/VF(ice): test scenarios including Switch features/Package 
> Management/Flow Director/Advanced Tx/Advanced RSS/ACL/DCF/Flexible 
> Descriptor, etc.
> - All test done. Found 1 meson test bug.
> * Intel NIC single core/NIC performance: test scenarios including PF/VF 
> single core performance test, etc.
> - All test done. No new dpdk issue is found.
> * IPsec: test scenarios including ipsec/ipsec-gw/ipsec library basic test - 
> QAT&SW/FIB library, etc.
> - All test done. No new dpdk issue is found.
>
> # Basic cryptodev and virtio testing
> * Virtio: both function and performance test are covered. Such as 
> PVP/Virtio_loopback/virtio-user loopback/virtio-net VM2VM perf 
> testing/VMAWARE ESXI 8.0, etc.
> - All test done. No new dpdk issue is found.
> * Cryptodev:
>   *Function test: test scenarios including Cryptodev API testing/CompressDev 
> ISA-L/QAT/ZLIB PMD Testing/FIPS, etc.
> - All test done. No new dpdk issue is found.
>   *Performance test: test scenarios including Thoughput Performance/Cryptodev 
> Latency, etc.
> - All test done. No new dpdk issue is found.
>
>
> Regards,
> Xu, Hailin

Perfect, thanks

Re: [PATCH 1/1] buildtools: clear out regex syntax warnings

[David Marchand]

On Fri, Dec 13, 2024 at 3:11 PM Ariel Otilibili-Anieli
 wrote:
>
> On Friday, December 13, 2024 15:07 CET, "Robin Jarry"  
> wrote:
>
> > Ariel Otilibili, Dec 08, 2024 at 03:25:
> > > * invalid escape sequences now generate SyntaxWarning
> > > * therefore changed syntax to raw string noration.
> > >
> > > Link: https://docs.python.org/3/library/re.html#module-re
> > > Signed-off-by: Ariel Otilibili 
> >
> > Hi Ariel, thanks for the fix!
> >
> > Acked-by: Robin Jarry 
>
> Awesome, Robin!
> >
> > If you have time, I there are other bits that would need the same fix:
> >
> > buildtools/get-numa-count.py:14:numa_nodes.sort(key=lambda l: 
> > int(re.findall('\d+', l)[0]))
> > devtools/check-meson.py:54:if re.match('^ *\t', code):
> > doc/api/generate_doxygen.py:8:pattern = re.compile('^Preprocessing 
> > (.*)...$')
> >
>
> Good, then; I'll push them in a subsequent series.

Those fixes are little one-liners worth backporting.

Please add a Fixes: tag identifying the commit introducing the issue,
then Cc: sta...@dpdk.org.
If you need more details on what is expected, see
https://doc.dpdk.org/guides/contributing/patches.html#commit-messages-body
and 
https://doc.dpdk.org/guides/contributing/patches.html#patch-for-stable-releases


-- 
David Marchand

Re: 22.11.7 patches review and test

[Xueming Li]

Hi All,

Thanks for the veri6ficiation and suggestion, queued patch to revert on 23.11:
Revert "test/bonding: fix loop on members" - dpdk-stable - Data Plane 
Development Kit - stable 
branches


From: Kevin Traynor 
Sent: Friday, December 13, 2024 5:54 PM
To: David Marchand ; luca.bocca...@gmail.com 
; Xueming Li ; Stephen Hemminger 

Cc: sta...@dpdk.org ; dev@dpdk.org ; Abhishek 
Marathe ; Ali Alnubani ; 
David Christensen ; Hemant Agrawal 
; Stokes, Ian ; Jerin Jacob 
; Mcnamara, John ; Ju-Hyoung Lee 
; Luca Boccassi ; Pei Zhang 
; Raslan Darawsheh ; NBU-Contact-Thomas 
Monjalon (EXTERNAL) ; Yanghang Liu ; 
Xu, HailinX 
Subject: Re: 22.11.7 patches review and test

On 13/12/2024 08:33, David Marchand wrote:
> Hello LTS maintainers, Stephen,
>
> On Fri, Dec 13, 2024 at 2:01 AM Xu, HailinX  wrote:
>>> Hi all,
>>>
>>> Here is a list of patches targeted for stable release 22.11.7.
>>>
>>> The planned date for the final release is December 17th.
>>>
>>> Please help with testing and validation of your use cases and report any
>>> issues/results with reply-all to this mail. For the final release the fixes 
>>> and
>>> reported validations will be added to the release notes.
>>>
>>> A release candidate tarball can be found at:
>>>
>>> https://dpdk.org/browse/dpdk-stable/tag/?id=v22.11.7-rc1
>>>
>>> These patches are located at branch 22.11 of dpdk-stable repo:
>>> https://dpdk.org/browse/dpdk-stable/
>>>
>>> Thanks.
>>>
>>> Luca Boccassi
>>
>> Update the test status for Intel part. dpdk22.11.7-rc1 all validation test 
>> done. Found 1 meson test bug.
>>
>> new issue:
>> Bug 1589 - meson_tests/driver link_bonding_autotest test failed  -> This 
>> issue also exists on 24.11
>
> This bug affects the main branch and all LTS branches currently being 
> validated.
> The commit 112ce3917674 ("test/bonding: fix loop on members") revealed
> a pre existing bug, so my advice is to revert this backport in all LTS
> until we have a proper fix in main.
>
> Thanks.
>

Thanks David. Queued patch to revert on 21.11
https://github.com/kevintraynor/dpdk-stable/commit/4e9bfc8965d9fe6ce9ca988dbd84a4ec8647abb5

Re: [PATCH v2 0/6] Add a stricter headers check

[David Marchand]

On Fri, Dec 13, 2024 at 12:27 PM Bruce Richardson
 wrote:
>
> On Fri, Dec 13, 2024 at 11:50:04AM +0100, David Marchand wrote:
> > As explained in patch 6, the current headers check can not catch
> > issues when a public header includes an internal header.
> > Fixing this from meson does not seem an easy task.
> >
> > This series approach is to reimplement the check in a Makefile invoked
> > out of DPDK (like what is done for external compilation of examples).
> > This has the advantage of being simple, and avoiding any (non intentional)
> > implicit include path coming from the meson framework.
> >
> > As there was no easy way to distinguish "indirect" headers in an
> > installed DPDK, I chose to move those headers in a new sub directory
> > (patch 5).
> >
> > Patch 1-4 fixes have not been marked as backport material as those bugs
> > seems minor/easy to fix externally (by either including missing headers,
> > or enabling enable_driver_sdk option).
> >
> > For now, I left the check_includes= option untouched, as there may be
> > users of this check and this check still catches issues without
> > requiring to install DPDK.
> >
> For patches 5 & 6, I wonder if we can find a slightly different way to do
> this. I like the idea of using make to build chkincs free from possible
> environmental contamination from meson, but I really don't like the
> complexity of the resulting makefile!

Well, I am no makefile expert, though I find this one straightforward.
Thomas could probably enhance it :-).


> Rather than having to move the indirect headers to a subdirectory, and then
> have a makefile run a scan from the headers directory, how about instead
> generating the makefile (or possibly a build.ninja file) directly from
> meson itself? This means the makefile can already have the list of headers
> and C files necessary - no need for an extra subdirectory - and no need for
> large amounts of wildcard matching and replacements.

Scanning a staging directory insulates from bugs in meson and this is
the main point of this series.
If we add a new framework in meson (a list of headers or whatever),
any driver can still add some install_headers() somewhere and we are
back with a new hole.

What will ensure that nobody introduce a bug back with some include
path to the sources, being passed into the generated Makefile?

Headers should be checked from an installed directory (like a final
user) with minimal (ideally no) knowledge of what header is safe to
include.


> The downside is that the makefile is no longer with the source, but in the
> build directory. However, since the most likely use for this makefile is
> from the test-meson-builds script and from automated CIs, I don't see this
> being a major issue.

This part is not an issue.


-- 
David Marchand

Re: [PATCH 1/1] buildtools: clear out regex syntax warnings

[Robin Jarry]

Ariel Otilibili, Dec 08, 2024 at 03:25:

* invalid escape sequences now generate SyntaxWarning
* therefore changed syntax to raw string noration.

Link: https://docs.python.org/3/library/re.html#module-re
Signed-off-by: Ariel Otilibili 


Hi Ariel, thanks for the fix!

Acked-by: Robin Jarry 

If you have time, I there are other bits that would need the same fix:

buildtools/get-numa-count.py:14:numa_nodes.sort(key=lambda l: 
int(re.findall('\d+', l)[0]))
devtools/check-meson.py:54:if re.match('^ *\t', code):
doc/api/generate_doxygen.py:8:pattern = re.compile('^Preprocessing (.*)...$')

Cheers.

Re: [PATCH v5] graph: mcore: optimize graph search

[David Marchand]

On Fri, Dec 13, 2024 at 3:22 AM Huichao Cai  wrote:
> diff --git a/lib/graph/rte_graph_worker_common.h 
> b/lib/graph/rte_graph_worker_common.h
> index d3ec88519d..aef0f65673 100644
> --- a/lib/graph/rte_graph_worker_common.h
> +++ b/lib/graph/rte_graph_worker_common.h
> @@ -110,6 +110,7 @@ struct __rte_cache_aligned rte_node {
> unsigned int lcore_id;  /**< Node running lcore. */
> uint64_t total_sched_objs; /**< Number of objects 
> scheduled. */
> uint64_t total_sched_fail; /**< Number of scheduled 
> failure. */
> +   struct rte_graph *graph;  /**< Graph corresponding to 
> lcore_id. */
> } dispatch;
> };

The rte_node struct size is not changed with this patch.
In v24.11, rte_node objects are populated/allocated in
graph_nodes_populate which zero's the whole rte_node.
So this change looks safe from an ABI compat with v24.11 pov.

However, we need to waive the warning from libabigail:
http://mails.dpdk.org/archives/test-report/2024-December/834167.html

Please add a temporary exception in devtools/libabigail.abignore.

It should be something like:
[suppress_type]
   name = rte_node
   has_size_change = no
   has_data_member_inserted_between =
{offset_of(total_sched_fail), offset_of(xstat_off)}


-- 
David Marchand

Re: [PATCH v2 1/1] buildtools: clear out regex syntax warnings

[Robin Jarry]

Ariel Otilibili, Dec 13, 2024 at 15:40:

* invalid escape sequences now generate SyntaxWarning
* therefore changed syntax to raw string noration.

Link: https://docs.python.org/3/library/re.html#module-re
Fixes: 0aeaf75df87 ("test: define unit tests suites based on test types")
Fixes: 25065ef1f6c ("test: emit warning for orphaned tests")
Cc: sta...@dpdk.org
Signed-off-by: Ariel Otilibili 
---


Acked-by: Robin Jarry 

[PATCH] net/iavf: remove reset of Tx prepare function ptr

[Bruce Richardson]

The iavf driver only contains a single Tx prepare function, so when
selecting the Tx path, there is no need to reset and reassign the
function pointer in the ethdev structure. This fixes an issue where the
pointer was reset to NULL, but never assigned back later on function
selection.

Fixes: 5712bf9d6e14 ("net/iavf: add Tx AVX2 offload path")
Fixes: 08eb6a9cc2e1 ("net/iavf: fix Tx L4 checksum")
Fixes: 4f8259df563a ("net/iavf: enable Tx outer checksum offload on AVX512")
Cc: sta...@dpdk.org

Signed-off-by: Bruce Richardson 
---
 drivers/net/iavf/iavf_rxtx.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/net/iavf/iavf_rxtx.c b/drivers/net/iavf/iavf_rxtx.c
index 6a093c6746..98a1d3f69d 100644
--- a/drivers/net/iavf/iavf_rxtx.c
+++ b/drivers/net/iavf/iavf_rxtx.c
@@ -4173,7 +4173,6 @@ iavf_set_tx_function(struct rte_eth_dev *dev)
if (!use_sse && !use_avx2 && !use_avx512)
goto normal;
 
-   dev->tx_pkt_prepare = NULL;
if (use_sse) {
PMD_DRV_LOG(DEBUG, "Using Vector Tx (port %d).",
dev->data->port_id);
@@ -4190,7 +4189,6 @@ iavf_set_tx_function(struct rte_eth_dev *dev)
goto normal;
} else {
tx_burst_type = IAVF_TX_AVX2_OFFLOAD;
-   dev->tx_pkt_prepare = iavf_prep_pkts;
PMD_DRV_LOG(DEBUG, "Using AVX2 OFFLOAD Vector 
Tx (port %d).",
dev->data->port_id);
}
@@ -4203,17 +4201,14 @@ iavf_set_tx_function(struct rte_eth_dev *dev)
dev->data->port_id);
} else if (check_ret == IAVF_VECTOR_OFFLOAD_PATH) {
tx_burst_type = IAVF_TX_AVX512_OFFLOAD;
-   dev->tx_pkt_prepare = iavf_prep_pkts;
PMD_DRV_LOG(DEBUG, "Using AVX512 OFFLOAD Vector 
Tx (port %d).",
dev->data->port_id);
} else if (check_ret == IAVF_VECTOR_CTX_PATH) {
tx_burst_type = IAVF_TX_AVX512_CTX;
-   dev->tx_pkt_prepare = iavf_prep_pkts;
PMD_DRV_LOG(DEBUG, "Using AVX512 CONTEXT Vector 
Tx (port %d).",
dev->data->port_id);
} else {
tx_burst_type = IAVF_TX_AVX512_CTX_OFFLOAD;
-   dev->tx_pkt_prepare = iavf_prep_pkts;
PMD_DRV_LOG(DEBUG, "Using AVX512 CONTEXT 
OFFLOAD Vector Tx (port %d).",
dev->data->port_id);
}
@@ -4251,7 +4246,6 @@ iavf_set_tx_function(struct rte_eth_dev *dev)
PMD_DRV_LOG(DEBUG, "Using Basic Tx callback (port=%d).",
dev->data->port_id);
tx_burst_type = IAVF_TX_DEFAULT;
-   dev->tx_pkt_prepare = iavf_prep_pkts;
 
if (no_poll_on_link_down) {
adapter->tx_burst_type = tx_burst_type;
-- 
2.43.0

Re: [PATCH v2 4/6] drivers: fix exported headers

[David Marchand]

On Fri, Dec 13, 2024 at 12:15 PM Bruce Richardson
 wrote:
>
> On Fri, Dec 13, 2024 at 11:50:08AM +0100, David Marchand wrote:
> > Those headers could not be included individually as they were not
> > including their dependencies, or were subject to some build warnings.
> >
> > Fixes: 831dba47bd36 ("bus/vmbus: add Hyper-V virtual bus support")
> > Fixes: 5b2a1a02dcaf ("crypto/cnxk: fix experimental version for PMD API")
> > Fixes: e5abbfa5 ("crypto/cnxk: add PMD API for getting CPTR")
> > Fixes: 3ca607402c4d ("crypto/cnxk: add PMD API to flush CTX")
> > Fixes: 8c3495f5d2dd ("net/dpaa: support loopback API")
> > Fixes: 12b435bf8f2f ("net/iavf: support flex desc metadata extraction")
> > Fixes: 23f627e0ed28 ("net/mlx5: add flow sync API")
> > Fixes: f5177bdc8b76 ("net/mlx5: add GENEVE TLV options parser API")
> > Fixes: 7cf197684589 ("raw/cnxk_bphy: support interrupt init and cleanup")
> > Fixes: 633dae698070 ("raw/cnxk_gpio: add standard GPIO operations")
> > Fixes: 53c71586c789 ("raw/dpaa2_cmdif: support enqueue/dequeue operations")
> > Fixes: c39d1e082a4b ("raw/ntb: setup queues")
> >
> > Signed-off-by: David Marchand 
>
> Generally LGTM, some queries inline below.
>
> /Bruce
>
> > ---
> >  drivers/bus/vmbus/rte_vmbus_reg.h |  6 ++
> >  drivers/crypto/cnxk/rte_pmd_cnxk_crypto.h |  4 
> >  drivers/net/dpaa/rte_pmd_dpaa.h   |  2 ++
> >  drivers/net/iavf/rte_pmd_iavf.h   |  6 ++
> >  drivers/net/mlx5/rte_pmd_mlx5.h   |  3 +++
> >  drivers/raw/cnxk_bphy/rte_pmd_bphy.h  | 16 
> >  drivers/raw/cnxk_gpio/rte_pmd_cnxk_gpio.h |  3 +++
> >  drivers/raw/dpaa2_cmdif/rte_pmd_dpaa2_cmdif.h |  2 ++
> >  drivers/raw/ntb/rte_pmd_ntb.h |  2 ++
> >  9 files changed, 44 insertions(+)
> >
> > diff --git a/drivers/bus/vmbus/rte_vmbus_reg.h 
> > b/drivers/bus/vmbus/rte_vmbus_reg.h
> > index e3299aa871..95c8eb29b4 100644
> > --- a/drivers/bus/vmbus/rte_vmbus_reg.h
> > +++ b/drivers/bus/vmbus/rte_vmbus_reg.h
> > @@ -6,6 +6,12 @@
> >  #ifndef _VMBUS_REG_H_
> >  #define _VMBUS_REG_H_
> >
> > +#include 
> > +
> > +#include 
> > +#include 
> > +#include 
> > +
> >  /*
> >   * Hyper-V SynIC message format.
> >   */
> > diff --git a/drivers/crypto/cnxk/rte_pmd_cnxk_crypto.h 
> > b/drivers/crypto/cnxk/rte_pmd_cnxk_crypto.h
> > index 02278605a2..2bb0ff9e95 100644
> > --- a/drivers/crypto/cnxk/rte_pmd_cnxk_crypto.h
> > +++ b/drivers/crypto/cnxk/rte_pmd_cnxk_crypto.h
> > @@ -11,8 +11,12 @@
> >  #ifndef _PMD_CNXK_CRYPTO_H_
> >  #define _PMD_CNXK_CRYPTO_H_
> >
> > +#include 
> >  #include 
> >
> > +#include 
> > +#include 
> > +
> >  /* Forward declarations */
> >
> >  /**
> > diff --git a/drivers/net/dpaa/rte_pmd_dpaa.h 
> > b/drivers/net/dpaa/rte_pmd_dpaa.h
> > index ec45633ba2..0a57e2097a 100644
> > --- a/drivers/net/dpaa/rte_pmd_dpaa.h
> > +++ b/drivers/net/dpaa/rte_pmd_dpaa.h
> > @@ -5,6 +5,8 @@
> >  #ifndef _PMD_DPAA_H_
> >  #define _PMD_DPAA_H_
> >
> > +#include 
> > +
> >  /**
> >   * @file rte_pmd_dpaa.h
> >   *
> > diff --git a/drivers/net/iavf/rte_pmd_iavf.h 
> > b/drivers/net/iavf/rte_pmd_iavf.h
> > index 56d453fc4c..04b86a5dd7 100644
> > --- a/drivers/net/iavf/rte_pmd_iavf.h
> > +++ b/drivers/net/iavf/rte_pmd_iavf.h
> > @@ -15,6 +15,7 @@
> >   */
> >
> >  #include 
> > +
> >  #include 
> >  #include 
> >  #include 
> > @@ -184,6 +185,7 @@ __rte_experimental
> >  static inline void
> >  rte_pmd_ifd_dump_proto_xtr_metadata(struct rte_mbuf *m)
> >  {
> > +#ifdef ALLOW_EXPERIMENTAL_API
> >   union rte_pmd_ifd_proto_xtr_metadata data;
> >
> >   if (!rte_pmd_ifd_dynf_proto_xtr_metadata_avail())
> > @@ -243,6 +245,10 @@ rte_pmd_ifd_dump_proto_xtr_metadata(struct rte_mbuf *m)
> >   else if (m->ol_flags & RTE_IAVF_PKT_RX_DYNF_PROTO_XTR_IP_OFFSET)
> >   printf(" - Flexible descriptor's Extraction: ip_offset=%u",
> >  data.ip_ofs);
> > +#else
> > + RTE_SET_USED(m);
> > + RTE_VERIFY(false);
>
> Is panicking the behaviour we want here? Seems rather severe, no?

You are not supposed to call this symbol without building with
ALLOW_EXPERIMENTAL_API.

Another option would be to mark those symbols as stable (they have
been untouched for years).
12b435bf8f2 (Jeff Guo  2020-10-30 16:40:30 +0800 186)
rte_pmd_ifd_dump_proto_xtr_metadata(struct rte_mbuf *m)


>
> > +#endif
> >  }
> >
> >  #ifdef __cplusplus
> > diff --git a/drivers/net/mlx5/rte_pmd_mlx5.h 
> > b/drivers/net/mlx5/rte_pmd_mlx5.h
> > index fdd2f65888..f2c6aebe0b 100644
> > --- a/drivers/net/mlx5/rte_pmd_mlx5.h
> > +++ b/drivers/net/mlx5/rte_pmd_mlx5.h
> > @@ -5,6 +5,9 @@
> >  #ifndef RTE_PMD_PRIVATE_MLX5_H_
> >  #define RTE_PMD_PRIVATE_MLX5_H_
> >
> > +#include 
> > +
> > +#include 
> >  #include 
> >
> >  /**
> > diff --git a/drivers/raw/cnxk_bphy/rte_pmd_bphy.h 
> > b/drivers/raw/cnxk_bphy/rte_pmd_bphy.h
> > index f668e6ea82..c200c935ff 100644
> > --- a/drivers/raw/cnxk_bphy/rte_pmd_bphy.h
> > +++ b/drivers/raw/cnxk_b

Re: 22.11.7 patches review and test

[Luca Boccassi]

Thanks, reverted

On Fri, 13 Dec 2024 at 13:36, Xueming Li  wrote:
>
> Hi All,
>
> Thanks for the veri6ficiation and suggestion, queued patch to revert on 23.11:
> Revert "test/bonding: fix loop on members" - dpdk-stable - Data Plane 
> Development Kit - stable branches
>
> 
> From: Kevin Traynor 
> Sent: Friday, December 13, 2024 5:54 PM
> To: David Marchand ; luca.bocca...@gmail.com 
> ; Xueming Li ; Stephen 
> Hemminger 
> Cc: sta...@dpdk.org ; dev@dpdk.org ; Abhishek 
> Marathe ; Ali Alnubani ; 
> David Christensen ; Hemant Agrawal 
> ; Stokes, Ian ; Jerin Jacob 
> ; Mcnamara, John ; Ju-Hyoung Lee 
> ; Luca Boccassi ; Pei Zhang 
> ; Raslan Darawsheh ; 
> NBU-Contact-Thomas Monjalon (EXTERNAL) ; Yanghang Liu 
> ; Xu, HailinX 
> Subject: Re: 22.11.7 patches review and test
>
> On 13/12/2024 08:33, David Marchand wrote:
> > Hello LTS maintainers, Stephen,
> >
> > On Fri, Dec 13, 2024 at 2:01 AM Xu, HailinX  wrote:
> >>> Hi all,
> >>>
> >>> Here is a list of patches targeted for stable release 22.11.7.
> >>>
> >>> The planned date for the final release is December 17th.
> >>>
> >>> Please help with testing and validation of your use cases and report any
> >>> issues/results with reply-all to this mail. For the final release the 
> >>> fixes and
> >>> reported validations will be added to the release notes.
> >>>
> >>> A release candidate tarball can be found at:
> >>>
> >>> https://dpdk.org/browse/dpdk-stable/tag/?id=v22.11.7-rc1
> >>>
> >>> These patches are located at branch 22.11 of dpdk-stable repo:
> >>> https://dpdk.org/browse/dpdk-stable/
> >>>
> >>> Thanks.
> >>>
> >>> Luca Boccassi
> >>
> >> Update the test status for Intel part. dpdk22.11.7-rc1 all validation test 
> >> done. Found 1 meson test bug.
> >>
> >> new issue:
> >> Bug 1589 - meson_tests/driver link_bonding_autotest test failed  -> 
> >> This issue also exists on 24.11
> >
> > This bug affects the main branch and all LTS branches currently being 
> > validated.
> > The commit 112ce3917674 ("test/bonding: fix loop on members") revealed
> > a pre existing bug, so my advice is to revert this backport in all LTS
> > until we have a proper fix in main.
> >
> > Thanks.
> >
>
> Thanks David. Queued patch to revert on 21.11
> https://github.com/kevintraynor/dpdk-stable/commit/4e9bfc8965d9fe6ce9ca988dbd84a4ec8647abb5
>

Re: [PATCH v2 4/6] drivers: fix exported headers

[Stephen Hemminger]

On Fri, 13 Dec 2024 11:50:08 +0100
David Marchand  wrote:

> @@ -496,6 +501,7 @@ rte_pmd_bphy_intr_mem_get(uint16_t dev_id, struct 
> cnxk_bphy_mem *mem)
>  {
>   struct cnxk_bphy_irq_msg msg = {
>   .type = CNXK_BPHY_IRQ_MSG_TYPE_MEM_GET,
> + .data = NULL,
>   };
>  

Why add these, compiler already initializes these to NULL.
Are you trying to enable -Wmissing-field-initializers?

Re: [PATCH] app/test: fix stack overflow in lpm6_perf_autotest

[Andre Muezerie]

On Fri, Dec 13, 2024 at 10:22:20AM +, Medvedkin, Vladimir wrote:
> Hi Andre,
> 
> On 13/12/2024 02:39, Andre Muezerie wrote:
> >Test lpm6_perf_autotest was hitting a stack overflow on Windows
> >with both MSVC and Clang.
> >
> >The fix is to move some of the data from the stack to the heap.
> >
> >Signed-off-by: Andre Muezerie 
> >---
> >  app/test/test_lpm6_perf.c | 11 +--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> >
> >diff --git a/app/test/test_lpm6_perf.c b/app/test/test_lpm6_perf.c
> >index 1860a99ed6..8231ad825d 100644
> >--- a/app/test/test_lpm6_perf.c
> >+++ b/app/test/test_lpm6_perf.c
> >@@ -117,8 +117,12 @@ test_lpm6_perf(void)
> > total_time = 0;
> > count = 0;
> >-struct rte_ipv6_addr ip_batch[NUM_IPS_ENTRIES];
> >-int32_t next_hops[NUM_IPS_ENTRIES];
> >+struct rte_ipv6_addr *ip_batch = (struct rte_ipv6_addr *)malloc(
> why not rte_malloc?

For no good reason :-)
I'll update the patch.
Thanks for the suggestion.

Andre Muezerie

> >+sizeof(struct rte_ipv6_addr) * NUM_IPS_ENTRIES);
> >+TEST_LPM_ASSERT(ip_batch != NULL);
> >+
> >+int32_t *next_hops = (int32_t *)malloc(sizeof(int32_t) * 
> >NUM_IPS_ENTRIES);
> >+TEST_LPM_ASSERT(next_hops != NULL);
> > for (i = 0; i < NUM_IPS_ENTRIES; i++)
> > ip_batch[i] = large_ips_table[i].ip;
> >@@ -153,6 +157,9 @@ test_lpm6_perf(void)
> > printf("Average LPM Delete: %g cycles\n",
> > (double)total_time / NUM_ROUTE_ENTRIES);
> >+free(next_hops);
> >+free(ip_batch);
> >+
> > rte_lpm6_delete_all(lpm);
> > rte_lpm6_free(lpm);
> 
> -- 
> Regards,
> Vladimir

Re: [PATCH] app/test: fix stack overflow in lpm6_perf_autotest

[Stephen Hemminger]

On Fri, 13 Dec 2024 10:22:20 +
"Medvedkin, Vladimir"  wrote:

> Hi Andre,
> 
> On 13/12/2024 02:39, Andre Muezerie wrote:
> > Test lpm6_perf_autotest was hitting a stack overflow on Windows
> > with both MSVC and Clang.
> >
> > The fix is to move some of the data from the stack to the heap.
> >
> > Signed-off-by: Andre Muezerie 
> > ---
> >   app/test/test_lpm6_perf.c | 11 +--
> >   1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/app/test/test_lpm6_perf.c b/app/test/test_lpm6_perf.c
> > index 1860a99ed6..8231ad825d 100644
> > --- a/app/test/test_lpm6_perf.c
> > +++ b/app/test/test_lpm6_perf.c
> > @@ -117,8 +117,12 @@ test_lpm6_perf(void)
> > total_time = 0;
> > count = 0;
> >   
> > -   struct rte_ipv6_addr ip_batch[NUM_IPS_ENTRIES];
> > -   int32_t next_hops[NUM_IPS_ENTRIES];
> > +   struct rte_ipv6_addr *ip_batch = (struct rte_ipv6_addr *)malloc(  
> why not rte_malloc?
> > +   sizeof(struct rte_ipv6_addr) * NUM_IPS_ENTRIES);
> > +   TEST_LPM_ASSERT(ip_batch != NULL);

There is no need for rte_malloc() here. The data doesn't need to
come from hugepages and regular malloc() has more checking.

But the cast is unnecessary in C.

[PATCH v2] app/test: fix stack overflow in lpm6_perf_autotest

[Andre Muezerie]

Test lpm6_perf_autotest was hitting a stack overflow on Windows
with both MSVC and Clang.

The fix is to move some of the data from the stack to the heap.

Signed-off-by: Andre Muezerie 
---
 app/test/test_lpm6_perf.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/app/test/test_lpm6_perf.c b/app/test/test_lpm6_perf.c
index 1860a99ed6..a09e8611ce 100644
--- a/app/test/test_lpm6_perf.c
+++ b/app/test/test_lpm6_perf.c
@@ -10,6 +10,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -117,8 +118,14 @@ test_lpm6_perf(void)
total_time = 0;
count = 0;
 
-   struct rte_ipv6_addr ip_batch[NUM_IPS_ENTRIES];
-   int32_t next_hops[NUM_IPS_ENTRIES];
+   struct rte_ipv6_addr *ip_batch =
+   (struct rte_ipv6_addr *)rte_malloc("ip_batch",
+   sizeof(struct rte_ipv6_addr) * NUM_IPS_ENTRIES, 0);
+   TEST_LPM_ASSERT(ip_batch != NULL);
+
+   int32_t *next_hops = (int32_t *)rte_malloc("next_hops",
+   sizeof(int32_t) * NUM_IPS_ENTRIES, 0);
+   TEST_LPM_ASSERT(next_hops != NULL);
 
for (i = 0; i < NUM_IPS_ENTRIES; i++)
ip_batch[i] = large_ips_table[i].ip;
@@ -153,6 +160,9 @@ test_lpm6_perf(void)
printf("Average LPM Delete: %g cycles\n",
(double)total_time / NUM_ROUTE_ENTRIES);
 
+   rte_free(next_hops);
+   rte_free(ip_batch);
+
rte_lpm6_delete_all(lpm);
rte_lpm6_free(lpm);
 
-- 
2.47.0.vfs.0.3

Re: [PATCH v2] app/test: fix stack overflow in lpm6_perf_autotest

[Stephen Hemminger]

On Fri, 13 Dec 2024 09:08:22 -0800
Andre Muezerie  wrote:

> + struct rte_ipv6_addr *ip_batch =
> + (struct rte_ipv6_addr *)rte_malloc("ip_batch",
> + sizeof(struct rte_ipv6_addr) * NUM_IPS_ENTRIES, 0

Cast is not needed here.
If you are going to allocate an array, use calloc() or rte_calloc()

Re: [PATCH 1/2] net/mlx5: improve socket file path

[Bruce Richardson]

On Fri, Dec 13, 2024 at 09:12:39AM -0800, Stephen Hemminger wrote:
> On Fri, 13 Dec 2024 17:24:42 +0800
> Yang Ming  wrote:
> 
> > 1. /var/tmp is hard code which is not a good style
> > 2. /var/tmp may be not allowed to be written via container's
> > read only mode.
> > 
> > Signed-off-by: Yang Ming 
> 
> Since this is a unix domain socket, why not use abstract socket
> that doesn't have to be associated with filesystem?

In general, I think we should avoid abstract sockets in DPDK. Primary
reason is that they are linux-specific. Last time I checked other unixes,
like BSD, don't support them. A secondary concern is that having a
filesystem path allows permission checks, so for e.g. telemetry sockets,
only users with appropriate permissions can connect. With an abstract socket
we'd have to open up the area of user authentication.

/Bruce

[PATCH v2] test/bonding: fix active backup rx test

[Stephen Hemminger]

The test had incorrect assumptions about how active backup
should work. When in active backup mode, the secondary (not primary)
ports should be ignored. The test was always broken since initially
written but earlier bug was masking the part of the test which
tested non-primary ports.

Bugzilla ID: 1589
Fixes: 112ce3917674 ("test/bonding: fix loop on members")
Cc: sta...@dpdk.org

Signed-off-by: Stephen Hemminger 
---
v2 - fix checkpatch warning from long line

 app/test/test_link_bonding.c | 69 ++--
 1 file changed, 34 insertions(+), 35 deletions(-)

diff --git a/app/test/test_link_bonding.c b/app/test/test_link_bonding.c
index b752a5ecbf..19b064771a 100644
--- a/app/test/test_link_bonding.c
+++ b/app/test/test_link_bonding.c
@@ -2246,49 +2246,48 @@ test_activebackup_rx_burst(void)

virtual_ethdev_add_mbufs_to_rx_queue(test_params->member_port_ids[i],
&gen_pkt_burst[0], burst_size);
 
+   /* Expect burst if this was the active port, zero otherwise */
+   unsigned int rx_expect
+   = (test_params->member_port_ids[i] == primary_port) ? 
burst_size : 0;
+
/* Call rx burst on bonding device */
-   
TEST_ASSERT_EQUAL(rte_eth_rx_burst(test_params->bonding_port_id, 0,
-   &rx_pkt_burst[0], MAX_PKT_BURST), burst_size,
-   "rte_eth_rx_burst failed");
+   unsigned int rx_count = 
rte_eth_rx_burst(test_params->bonding_port_id, 0,
+&rx_pkt_burst[0], 
MAX_PKT_BURST);
+   TEST_ASSERT_EQUAL(rx_count, rx_expect,
+ "rte_eth_rx_burst (%u) not as expected (%u)",
+ rx_count, rx_expect);
 
-   if (test_params->member_port_ids[i] == primary_port) {
-   /* Verify bonding device rx count */
-   rte_eth_stats_get(test_params->bonding_port_id, 
&port_stats);
-   TEST_ASSERT_EQUAL(port_stats.ipackets, 
(uint64_t)burst_size,
-   "Bonding Port (%d) ipackets value (%u) 
not as expected (%d)",
+   /* Verify bonding device rx count */
+   rte_eth_stats_get(test_params->bonding_port_id, &port_stats);
+   TEST_ASSERT_EQUAL(port_stats.ipackets, rx_expect,
+ "Bonding Port (%d) ipackets value (%u) not as 
expected (%u)",
test_params->bonding_port_id,
-   (unsigned int)port_stats.ipackets, 
burst_size);
+ (unsigned int)port_stats.ipackets, rx_expect);
 
-   /* Verify bonding member devices rx count */
-   for (j = 0; j < test_params->bonding_member_count; j++) 
{
-   
rte_eth_stats_get(test_params->member_port_ids[j], &port_stats);
-   if (i == j) {
-   TEST_ASSERT_EQUAL(port_stats.ipackets, 
(uint64_t)burst_size,
-   "Member Port (%d) 
ipackets value (%u) not as "
-   "expected (%d)",
-   
test_params->member_port_ids[i],
-   (unsigned 
int)port_stats.ipackets,
-   burst_size);
-   } else {
-   TEST_ASSERT_EQUAL(port_stats.ipackets, 
0,
-   "Member Port (%d) 
ipackets value (%u) not as "
-   "expected (%d)\n",
-   
test_params->member_port_ids[i],
-   (unsigned 
int)port_stats.ipackets, 0);
-   }
-   }
-   } else {
-   for (j = 0; j < test_params->bonding_member_count; j++) 
{
-   
rte_eth_stats_get(test_params->member_port_ids[j], &port_stats);
+   for (j = 0; j < test_params->bonding_member_count; j++) {
+   rte_eth_stats_get(test_params->member_port_ids[j], 
&port_stats);
+   if (i == j) {
+   TEST_ASSERT_EQUAL(port_stats.ipackets, 
rx_expect,
+ "Member Port (%d) ipackets (%u) not 
as expected (%d)",
+ test_params->member_port_ids[i],
+ (unsigned int)port_stats.ipackets, 
rx_expect);
+
+   /* reset member device stats */
+ 

Re: [PATCH] test/bonding: fix active backup rx test

[Stephen Hemminger]

On Fri, 13 Dec 2024 14:57:23 +0800
"lihuisong (C)"  wrote:

> Hi Stephen,
> 
> This patch looks good to me.
> 
> But this test case still runs failure when I run "link_bonding_autotest" 
> twice. like:
> -->  
> ...
> EAL: Test assert configure_ethdev line 181 failed: rte_eth_dev_configure 
> for port 7 failed
> EAL: Test assert initialize_bonding_device_with_members line 1138 
> failed: Failed to configure bonding port (7) in mode 3 with (4) members.
> EAL: Test assert 
> test_broadcast_verify_member_link_status_change_behaviour line 4060 
> failed: Failed to initialise bonding device
>   + TestCase [62] : 
> test_broadcast_verify_member_link_status_change_behaviour failed
> ETHDEV: Invalid port_id=7
> EAL: Test assert configure_ethdev line 181 failed: rte_eth_dev_configure 
> for port 7 failed
> EAL: Test assert test_reconfigure_bonding_device line 4162 failed: 
> failed to reconfigure bonding device
>   + TestCase [63] : test_reconfigure_bonding_device failed
> ETHDEV: Invalid port_id=7
>   + TestCase [64] : test_close_bonding_device succeeded
> ETHDEV: Invalid port_id=7
> EAL: Test assert remove_members_and_stop_bonding_device line 659 failed: 
> Failed to stop bonding port 7
>   + --- +
>   + Test Suite Summary : Link Bonding Unit Test Suite
>   + --- +
>   + Tests Total :   65
>   + Tests Skipped :  0
>   + Tests Executed :    65
>   + Tests Unsupported:   0
>   + Tests Passed :   5
>   + Tests Failed :  60
>   + --- +
> Test Failed
> 
> 
> I guess the bonding use case needs to close all bonding devices.


The test has lots of issues that would make it bad to run twice.
Like leaking mbufs, etc.
But these always existed and will leave that to bonding maintainers to fix.