RE: [PATCH 0/3] flexible IPv4 fragment action
> -Original Message- > From: Richardson, Bruce > Sent: Wednesday, January 22, 2025 7:24 PM > To: Ye, MingjinX > Cc: dev@dpdk.org > Subject: Re: [PATCH 0/3] flexible IPv4 fragment action > > On Wed, Jan 22, 2025 at 08:23:07AM +, Mingjin Ye wrote: > > Support for distributing the first and other segments of an IPv4 > > segmented packet to different RX queues. > > > > Mingjin Ye (3): > > net/ice/base: add ipv4 fragment related field > > net/ice: FDIR support IPv4 fragment masks > > net/ice: ACL filter support for IPv4 fragment > > > > drivers/net/ice/base/ice_fdir.h| 2 + > > drivers/net/ice/base/ice_flow.c| 5 +++ > > drivers/net/ice/base/ice_flow.h| 1 + > > drivers/net/ice/ice_acl_filter.c | 61 +++--- > > drivers/net/ice/ice_ethdev.c | 1 - > > drivers/net/ice/ice_fdir_filter.c | 15 ++-- > > drivers/net/ice/ice_generic_flow.h | 2 + > > 7 files changed, 77 insertions(+), 10 deletions(-) > > > > This patchset has no documentation updates included in it. Does there not > need to be some documentation for this new feature, or is the ability to > filter > segmented packets already covered as a standard flow feature elsewhere? Yes. IPv4 segmentation is already supported in FDIR filters. Therefore, no additional documentation has been added.
RE: [PATCH 1/1] test/crypto: additional RSA tests for CNXK PMD
> Subject: [PATCH 1/1] test/crypto: additional RSA tests for CNXK PMD > > Include additional RSA tests for CNXK PMD. These tests validates RSA > operations using private key in exponent form. > > Signed-off-by: Gowrishankar Muthukrishnan Acked-by: Anoob Joseph
Re: [PATCH v6 01/15] net/xsc: add xsc PMD framework
On 2025/1/23 15:59, Thomas Monjalon wrote: > 23/01/2025 06:48, WanRenyong: >> On 2025/1/22 21:39, Thomas Monjalon wrote: +Yunsilicon xsc +M: WanRenyong +M: Na Na +M: Rong Qian +M: Xiaoxiong Zhang +M: Dongwei Xu >>> Looking at how the names are codified in email addresses, >>> I feel "Renyong Wan" is the right form for your name in English format. >>> >>> >> Hello Tomas Monjalon, >> >> Yes, you are right, but if I use "Renyong Wan" as my English name, every >> patch alway gets a misspelling warning from checkpatch. :( >> It's really annoying. If it isn't unacceptable to DPDK for using >> "WanRenyong" as my name, I don't mind of it too. > Can't you set "Renyong Wan" in your .gitconfig? > I think it would solve your issue. > > Hello Tomas Monjalon, I tried it , there are still a few warnings from checkpatch like below: WARNING:TYPO_SPELLING: 'Wan' may be misspelled - perhaps 'Want'? #8: Renyong Wan (15): ^^^ If DPDK accept it , I'll change my name to "Renyong Wan" in the next version. Thank you. -- Best regards, WanRenyong
RE: [PATCH] examples/ipsec-secgw: fix cryptodev and eventdev ID
> Subject: [PATCH] examples/ipsec-secgw: fix cryptodev and eventdev ID
>
> Fixing cryptodev and eventdev ID numbers.
>
> Fixes: 0dbe550a4af5 ("examples/ipsec-secgw: initialize event crypto adapter")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Tejasree Kondoj
Acked-by: Anoob Joseph
Re: [PATCH v1] common/idpf: fix heap use after free error
On Mon, Jan 20, 2025 at 02:32:49PM +, Bruce Richardson wrote:
> On Mon, Jan 13, 2025 at 08:30:01AM -0800, Stephen Hemminger wrote:
> > On Mon, 13 Jan 2025 08:54:04 +
> > Praveen Shetty wrote:
> >
> > > Heap use after free error is detected in AddressSanitizer while quitting
> > > the testpmd application.Issue is due to accessing the empty control
> > > queue in the idpf_ctlq_deinit function.idpf_ctlq_deinit function is called
> > > during the rte_eal_cleanup routine.
> > > This patch will fix this issue.
> > >
> > > Fixes: fb4ac04e9bfa ("common/idpf: introduce common library")
> > > Cc: sta...@dpdk.org
> > >
> > > Signed-off-by: Praveen Shetty
> >
> > This should not be needed. LIST_FOR_EACH_ENTRY_SAFE part, don't understand.
>
> I would tend to agree. Is there an actual confirmed bug here? If so, then
> either our standard list macros are broken, or the code using them is doing
> something rather strange.
>
I followed up on with with Praveen, and he went through the code and
possible solutions with me. The issue flagged by ASAN is correct, because
it turns out that the version of the _SAFE macro provided in this
particular driver is not actually safe! :-(
There are therefore two options to fixing this: 1) fix the macro/use a
different copy of the macro, or 2) rework the code as in this patch and drop
the macro. Copies of the driver in other OS use the style given in this patch,
so we will go with the second option. However, we will do a v2 to include
the removal of the bad macro, alongside fixing this. That should hopefully
prevent this issue from reoccurring.
Praveen, will review v2 when you send it.
/Bruce
Re: [PATCH v16 00/60] remove use of VLAs for Windows
On Tue, Jan 14, 2025 at 3:32 AM Andre Muezerie
wrote:
>
> As per guidance technical board meeting 2024/04/17. This series
> removes the use of VLAs from code built for Windows for all 3
> toolchains. If there are additional opportunities to convert VLAs
> to regular C arrays please provide the details for incorporation
> into the series.
>
> MSVC does not support VLAs, replace VLAs with standard C arrays
> or alloca(). alloca() is available for all toolchain/platform
> combinations officially supported by DPDK.
>
> v16:
> * remove -Wvla from drivers/common/mlx5/meson.build and
> drivers/common/qat/meson.build
>
> v15:
> * inverted some of the logic added during v14:
> add -Wvla to meson build files in app and lib directories, adding
> -Wno-vla to the few subdirectories which are not yet VLA free
>
> v14:
> * add -Wvla to meson build for directories that are VLA free
> under app, lib, drivers. This is to ensure that new VLAs are
> not added to these directories in the future.
Thanks for working on this topic.
I see there is some back and forth on the topic of passing -Wvla.
It would be less fragile to put a -Wla in a upper level meson.build
(like config/meson.build for example), then disable explicitly in the
parts that are not ready.
Something like:
diff --git a/config/meson.build b/config/meson.build
index 6aaad6d8a4..be603bd45b 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -348,6 +348,17 @@ foreach arg: warning_flags
endif
endforeach
+if cc.has_argument('-Wvla')
+add_project_arguments('-Wvla', language: 'c')
+if not is_windows
+no_vla_cflag = '-Wno-vla'
+else
+no_vla_cflag = []
+endif
+else
+no_vla_cflag = []
+endif
+
# set other values pulled from the build options
dpdk_conf.set('RTE_MAX_ETHPORTS', get_option('max_ethports'))
dpdk_conf.set('RTE_LIBEAL_USE_HPET', get_option('use_hpet'))
This has the benefit of avoiding repeating those if cc.has_argument()
loops in all meson.build.
Disabling becomes simply a matter of adding cflags += no_vla_cflag.
This also enforces -Wvla for code that is built on windows (with mingw build).
I had a try, and flagged all remaining components that have VLA in them.
You can have a look at:
https://github.com/david-marchand/dpdk/commit/vla_v16_dma
This helped me catch a new VLA in the recently merged soring test code:
https://github.com/david-marchand/dpdk/commit/vla_v16_dma~1
WDYT?
--
David Marchand
RE: [PATCH 3/3] net/ice: ACL filter support for IPv4 fragment
> -Original Message- > From: Mingjin Ye > Sent: Wednesday, January 22, 2025 4:23 PM > To: dev@dpdk.org > Cc: Ye, MingjinX ; Richardson, Bruce > ; Burakov, Anatoly > > Subject: [PATCH 3/3] net/ice: ACL filter support for IPv4 fragment > > Enable ACL filter on PF. Add support for FRAG_IPV4 pattern and queue action. > > Flow rule can be created by the following command: >flow create 0 ingress group 1 pattern eth / >ipv4 fragment_offset spec 0x2000 fragment_offset mask 0x3FFF / >end actions queue index / end > > Signed-off-by: Mingjin Ye > --- Tested-by: Jiale Song
Re: [PATCH v1] common/idpf: fix heap use after free error
On Thu, Jan 23, 2025 at 12:18 PM Bruce Richardson
wrote:
>
> On Mon, Jan 20, 2025 at 02:32:49PM +, Bruce Richardson wrote:
> > On Mon, Jan 13, 2025 at 08:30:01AM -0800, Stephen Hemminger wrote:
> > > On Mon, 13 Jan 2025 08:54:04 +
> > > Praveen Shetty wrote:
> > >
> > > > Heap use after free error is detected in AddressSanitizer while quitting
> > > > the testpmd application.Issue is due to accessing the empty control
> > > > queue in the idpf_ctlq_deinit function.idpf_ctlq_deinit function is
> > > > called
> > > > during the rte_eal_cleanup routine.
> > > > This patch will fix this issue.
> > > >
> > > > Fixes: fb4ac04e9bfa ("common/idpf: introduce common library")
> > > > Cc: sta...@dpdk.org
> > > >
> > > > Signed-off-by: Praveen Shetty
> > >
> > > This should not be needed. LIST_FOR_EACH_ENTRY_SAFE part, don't
> > > understand.
> >
> > I would tend to agree. Is there an actual confirmed bug here? If so, then
> > either our standard list macros are broken, or the code using them is doing
> > something rather strange.
> >
>
> I followed up on with with Praveen, and he went through the code and
> possible solutions with me. The issue flagged by ASAN is correct, because
> it turns out that the version of the _SAFE macro provided in this
> particular driver is not actually safe! :-(
>
> There are therefore two options to fixing this: 1) fix the macro/use a
> different copy of the macro, or 2) rework the code as in this patch and drop
> the macro. Copies of the driver in other OS use the style given in this patch,
> so we will go with the second option. However, we will do a v2 to include
> the removal of the bad macro, alongside fixing this. That should hopefully
> prevent this issue from reoccurring.
>
> Praveen, will review v2 when you send it.
Sorry, I am not following.
1) seems the best way as it does not require touching base driver code.
Afaiu, the LIST_FOR_EACH_ENTRY_SAFE macro is defined in the
"abstraction" header that is DPDK specific
(drivers/common/idpf/base/idpf_osdep.h).
There is already an implementation of LIST_FOR_EACH_ENTRY_SAFE in
driver/net/ice/base/ice_osdep.h.
(note that it may be worth providing such a macro in a common place in
DPDK and remove copies of it in various drivers).
--
David Marchand
Re: [PATCH v2 0/2] fix void function returning a value
Hello Bruce, On Wed, Jan 22, 2025 at 4:21 PM Andre Muezerie wrote: > > v2: > * Updated commit messages to follow standard format. > > This patch avoids warnings like the one below emitted by MSVC, and is > needed to get the code to compile cleanly with MSVC. > > ../drivers/common/idpf/idpf_common_rxtx_avx512.c(139): > warning C4098: 'idpf_singleq_rearm': > 'void' function returning a value > > Andre Muezerie (2): > drivers/common: fix void function returning a value > drivers/net: fix void function returning a value > > drivers/common/idpf/idpf_common_rxtx_avx512.c | 12 > drivers/net/i40e/i40e_rxtx_vec_avx2.c | 2 +- > drivers/net/i40e/i40e_rxtx_vec_avx512.c | 2 +- > drivers/net/iavf/iavf_rxtx_vec_avx2.c | 2 +- > drivers/net/ice/ice_rxtx_vec_avx2.c | 2 +- > 5 files changed, 12 insertions(+), 8 deletions(-) I see the series is delegated to the main repo (Thomas). This touches only Intel drivers and the code deduplication effort you started may conflict (though trivially) with such changes depending on when it lands. Would you mind merging this fixes from Andre through the next-net-intel tree, right now? If so, please mark it as delegated to you in patchwork. Thanks! -- David Marchand
Re: [PATCH v2 0/2] fix void function returning a value
On Thu, Jan 23, 2025 at 01:23:15PM +0100, David Marchand wrote: > Hello Bruce, > > On Wed, Jan 22, 2025 at 4:21 PM Andre Muezerie > wrote: > > > > v2: > > * Updated commit messages to follow standard format. > > > > This patch avoids warnings like the one below emitted by MSVC, and is > > needed to get the code to compile cleanly with MSVC. > > > > ../drivers/common/idpf/idpf_common_rxtx_avx512.c(139): > > warning C4098: 'idpf_singleq_rearm': > > 'void' function returning a value > > > > Andre Muezerie (2): > > drivers/common: fix void function returning a value > > drivers/net: fix void function returning a value > > > > drivers/common/idpf/idpf_common_rxtx_avx512.c | 12 > > drivers/net/i40e/i40e_rxtx_vec_avx2.c | 2 +- > > drivers/net/i40e/i40e_rxtx_vec_avx512.c | 2 +- > > drivers/net/iavf/iavf_rxtx_vec_avx2.c | 2 +- > > drivers/net/ice/ice_rxtx_vec_avx2.c | 2 +- > > 5 files changed, 12 insertions(+), 8 deletions(-) > > I see the series is delegated to the main repo (Thomas). > > This touches only Intel drivers and the code deduplication effort you > started may conflict (though trivially) with such changes depending on > when it lands. > > Would you mind merging this fixes from Andre through the > next-net-intel tree, right now? > If so, please mark it as delegated to you in patchwork. > > > Thanks! > Hi David, the code deduplication effort patchsets are similarly delegated to the main repo. I was assuming this was deliberate, but perhaps it isn't? I'm ok to take these patches in next-net-intel, but just would like to confirm that neither you, Thomas or Stephen (as net maintainer) want to review the dedup work ahead of that initial merge? /Bruce
Re: [PATCH v16 00/60] remove use of VLAs for Windows
On Thu, Jan 23, 2025 at 12:58:49PM +0100, David Marchand wrote:
> On Tue, Jan 14, 2025 at 3:32 AM Andre Muezerie
> wrote:
> >
> > As per guidance technical board meeting 2024/04/17. This series
> > removes the use of VLAs from code built for Windows for all 3
> > toolchains. If there are additional opportunities to convert VLAs
> > to regular C arrays please provide the details for incorporation
> > into the series.
> >
> > MSVC does not support VLAs, replace VLAs with standard C arrays
> > or alloca(). alloca() is available for all toolchain/platform
> > combinations officially supported by DPDK.
> >
> > v16:
> > * remove -Wvla from drivers/common/mlx5/meson.build and
> > drivers/common/qat/meson.build
> >
> > v15:
> > * inverted some of the logic added during v14:
> > add -Wvla to meson build files in app and lib directories, adding
> > -Wno-vla to the few subdirectories which are not yet VLA free
> >
> > v14:
> > * add -Wvla to meson build for directories that are VLA free
> > under app, lib, drivers. This is to ensure that new VLAs are
> > not added to these directories in the future.
>
> Thanks for working on this topic.
>
> I see there is some back and forth on the topic of passing -Wvla.
> It would be less fragile to put a -Wla in a upper level meson.build
> (like config/meson.build for example), then disable explicitly in the
> parts that are not ready.
>
> Something like:
> diff --git a/config/meson.build b/config/meson.build
> index 6aaad6d8a4..be603bd45b 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -348,6 +348,17 @@ foreach arg: warning_flags
> endif
> endforeach
>
> +if cc.has_argument('-Wvla')
> +add_project_arguments('-Wvla', language: 'c')
> +if not is_windows
> +no_vla_cflag = '-Wno-vla'
> +else
> +no_vla_cflag = []
> +endif
> +else
> +no_vla_cflag = []
> +endif
> +
Minor simplification suggestion, put "no_vla_cflag = []" outside the
conditionals at the start, as the default value. Save having multiple
copies of that assignment, and having to do "else" legs.
/Bruce
Re: [PATCH v1] common/idpf: fix heap use after free error
On Thu, Jan 23, 2025 at 12:43:50PM +0100, David Marchand wrote:
> On Thu, Jan 23, 2025 at 12:18 PM Bruce Richardson
> wrote:
> >
> > On Mon, Jan 20, 2025 at 02:32:49PM +, Bruce Richardson wrote:
> > > On Mon, Jan 13, 2025 at 08:30:01AM -0800, Stephen Hemminger wrote:
> > > > On Mon, 13 Jan 2025 08:54:04 + Praveen Shetty
> > > > wrote:
> > > >
> > > > > Heap use after free error is detected in AddressSanitizer while
> > > > > quitting the testpmd application.Issue is due to accessing the
> > > > > empty control queue in the idpf_ctlq_deinit
> > > > > function.idpf_ctlq_deinit function is called during the
> > > > > rte_eal_cleanup routine. This patch will fix this issue.
> > > > >
> > > > > Fixes: fb4ac04e9bfa ("common/idpf: introduce common library") Cc:
> > > > > sta...@dpdk.org
> > > > >
> > > > > Signed-off-by: Praveen Shetty
> > > >
> > > > This should not be needed. LIST_FOR_EACH_ENTRY_SAFE part, don't
> > > > understand.
> > >
> > > I would tend to agree. Is there an actual confirmed bug here? If so,
> > > then either our standard list macros are broken, or the code using
> > > them is doing something rather strange.
> > >
> >
> > I followed up on with with Praveen, and he went through the code and
> > possible solutions with me. The issue flagged by ASAN is correct,
> > because it turns out that the version of the _SAFE macro provided in
> > this particular driver is not actually safe! :-(
> >
> > There are therefore two options to fixing this: 1) fix the macro/use a
> > different copy of the macro, or 2) rework the code as in this patch and
> > drop the macro. Copies of the driver in other OS use the style given in
> > this patch, so we will go with the second option. However, we will do a
> > v2 to include the removal of the bad macro, alongside fixing this. That
> > should hopefully prevent this issue from reoccurring.
> >
> > Praveen, will review v2 when you send it.
>
> Sorry, I am not following.
>
> 1) seems the best way as it does not require touching base driver code.
> Afaiu, the LIST_FOR_EACH_ENTRY_SAFE macro is defined in the "abstraction"
> header that is DPDK specific (drivers/common/idpf/base/idpf_osdep.h).
>
> There is already an implementation of LIST_FOR_EACH_ENTRY_SAFE in
> driver/net/ice/base/ice_osdep.h.
>
> (note that it may be worth providing such a macro in a common place in
> DPDK and remove copies of it in various drivers).
>
Yes, that is correct, and double checking the IDPF shared code confirms
that is the best solution.
/Bruce
Re: [PATCH v2 0/2] fix void function returning a value
On Thu, Jan 23, 2025 at 1:46 PM Bruce Richardson wrote: > > On Thu, Jan 23, 2025 at 01:23:15PM +0100, David Marchand wrote: > > Hello Bruce, > > > > On Wed, Jan 22, 2025 at 4:21 PM Andre Muezerie > > wrote: > > > > > > v2: > > > * Updated commit messages to follow standard format. > > > > > > This patch avoids warnings like the one below emitted by MSVC, and is > > > needed to get the code to compile cleanly with MSVC. > > > > > > ../drivers/common/idpf/idpf_common_rxtx_avx512.c(139): > > > warning C4098: 'idpf_singleq_rearm': > > > 'void' function returning a value > > > > > > Andre Muezerie (2): > > > drivers/common: fix void function returning a value > > > drivers/net: fix void function returning a value > > > > > > drivers/common/idpf/idpf_common_rxtx_avx512.c | 12 > > > drivers/net/i40e/i40e_rxtx_vec_avx2.c | 2 +- > > > drivers/net/i40e/i40e_rxtx_vec_avx512.c | 2 +- > > > drivers/net/iavf/iavf_rxtx_vec_avx2.c | 2 +- > > > drivers/net/ice/ice_rxtx_vec_avx2.c | 2 +- > > > 5 files changed, 12 insertions(+), 8 deletions(-) > > > > I see the series is delegated to the main repo (Thomas). > > > > This touches only Intel drivers and the code deduplication effort you > > started may conflict (though trivially) with such changes depending on > > when it lands. > > > > Would you mind merging this fixes from Andre through the > > next-net-intel tree, right now? > > If so, please mark it as delegated to you in patchwork. > > > > > > Thanks! > > > Hi David, > > the code deduplication effort patchsets are similarly delegated to the main > repo. I was assuming this was deliberate, but perhaps it isn't? I'm ok to I suspect it was delegated to main because of the change on devtools/check-git-log.sh. But it is really only about net driver changes, so it should go either through your or Stephen tree. (we have enough patches waiting in main ;-)) > take these patches in next-net-intel, but just would like to confirm that > neither you, Thomas or Stephen (as net maintainer) want to review the dedup > work ahead of that initial merge? I don't have an objection on this series (on the contrary, I am quite happy to see such effort). I'll have a deeper look at it, this afternoon. -- David Marchand
Re: [PATCH v3] eal: fix macros for MSVC: noinline, alwaysinline, hot
On Wed, Jan 22, 2025 at 6:10 PM Morten Brørup wrote: > > > From: Andre Muezerie [mailto:andre...@linux.microsoft.com] > > Sent: Wednesday, 22 January 2025 17.24 > > > > MSVC supports forcing code to be inlined or forcing code to not be > > inlined, like other compilers. It does not support the "hot" hint > > though. > > > > This patch fixes existing macros __rte_noinline and > > __rte_always_inline so that they also do what is expected from them > > when used with MSVC. __rte_hot is updated to become a noop when > > MSCS is used. > > > > Signed-off-by: Andre Muezerie > Acked-by: Morten Brørup Applied, thanks Andre. -- David Marchand
Re: [PATCH] eal: fix undeclared function error on old CPUs
On Tue, Jan 14, 2025 at 5:33 PM Bruce Richardson
wrote:
>
> On Tue, Jan 14, 2025 at 08:21:13AM -0800, Andre Muezerie wrote:
> > Error reported:
> > ../lib/net/net_crc_sse.c:49:17: error: call to undeclared function
> > '_mm_clmulepi64_si128'; ISO C99 and later do not support implicit
> > function declarations [-Wimplicit-function-declaration]
> >
> > The fix is to remove the unnecessary ifdef around the inclusion of
> > header file immintrin.h. This header also contains functions that do
> > not require AVX instructions, so should not be included only when AVX
> > is available.
> >
> > Bugzilla ID: 1595
> > Fixes: da826b7135a4 ("eal: introduce ymm type for AVX 256-bit")
> > Cc: sta...@dpdk.org
> >
Reported-by: Pier Damouny
> > Signed-off-by: Andre Muezerie
> Acked-by: Bruce Richardson
Applied, thanks Andre.
--
David Marchand
Re: [PATCH v6 01/15] net/xsc: add xsc PMD framework
23/01/2025 06:48, WanRenyong: > On 2025/1/22 21:39, Thomas Monjalon wrote: > >> +Yunsilicon xsc > >> +M: WanRenyong > >> +M: Na Na > >> +M: Rong Qian > >> +M: Xiaoxiong Zhang > >> +M: Dongwei Xu > > Looking at how the names are codified in email addresses, > > I feel "Renyong Wan" is the right form for your name in English format. > > > > > Hello Tomas Monjalon, > > Yes, you are right, but if I use "Renyong Wan" as my English name, every > patch alway gets a misspelling warning from checkpatch. :( > It's really annoying. If it isn't unacceptable to DPDK for using > "WanRenyong" as my name, I don't mind of it too. Can't you set "Renyong Wan" in your .gitconfig? I think it would solve your issue.
RE: [PATCH v16 2/3] drivers/common: add diagnostics macros to make code portable
> From: Andre Muezerie [mailto:andre...@linux.microsoft.com] > Sent: Tuesday, 21 January 2025 23.36 > > It was a common pattern to have "GCC diagnostic ignored" pragmas > sprinkled over the code and only activate these pragmas for certain > compilers (gcc and clang). Clang supports GCC's pragma for > compatibility with existing source code, so #pragma GCC diagnostic > and #pragma clang diagnostic are synonyms for Clang > (https://clang.llvm.org/docs/UsersManual.html). > > Now that effort is being made to make the code compatible with MSVC > these expressions would become more complex. It makes sense to hide > this complexity behind macros. This makes maintenance easier as these > macros are defined in a single place. As a plus the code becomes > more readable as well. > > Signed-off-by: Andre Muezerie > --- Acked-by: Morten Brørup
RE: [PATCH v16 1/3] eal: add diagnostics macros to make code portable
> From: Andre Muezerie [mailto:andre...@linux.microsoft.com] > Sent: Tuesday, 21 January 2025 23.36 > > It was a common pattern to have "GCC diagnostic ignored" pragmas > sprinkled over the code and only activate these pragmas for certain > compilers (gcc and clang). Clang supports GCC's pragma for > compatibility with existing source code, so #pragma GCC diagnostic > and #pragma clang diagnostic are synonyms for Clang > (https://clang.llvm.org/docs/UsersManual.html). > > Now that effort is being made to make the code compatible with MSVC > these expressions would become more complex. It makes sense to hide > this complexity behind macros. This makes maintenance easier as these > macros are defined in a single place. As a plus the code becomes > more readable as well. > > Signed-off-by: Andre Muezerie > --- Thank you for your extra effort on making this as clean as practically possible, Andre. Reviewed-by: Morten Brørup
RE: [PATCH v16 3/3] drivers/net: add diagnostics macros to make code portable
> From: Andre Muezerie [mailto:andre...@linux.microsoft.com] > Sent: Tuesday, 21 January 2025 23.36 > > It was a common pattern to have "GCC diagnostic ignored" pragmas > sprinkled over the code and only activate these pragmas for certain > compilers (gcc and clang). Clang supports GCC's pragma for > compatibility with existing source code, so #pragma GCC diagnostic > and #pragma clang diagnostic are synonyms for Clang > (https://clang.llvm.org/docs/UsersManual.html). > > Now that effort is being made to make the code compatible with MSVC > these expressions would become more complex. It makes sense to hide > this complexity behind macros. This makes maintenance easier as these > macros are defined in a single place. As a plus the code becomes > more readable as well. > > Signed-off-by: Andre Muezerie > --- Acked-by: Morten Brørup
Re: [PATCH v5 01/25] net: move intel drivers to intel subdirectory
On Thu, Jan 23, 2025 at 03:16:40PM +0100, David Marchand wrote: > Hello Bruce, Thomas, > > On Mon, Jan 20, 2025 at 1:00 PM Bruce Richardson > wrote: > > > > Consolidate all Intel HW NIC drivers into a driver/net/intel This > > matches the layout used for drivers in the kernel, and potentially > > enabling easier sharing among drivers. > > > > Signed-off-by: Bruce Richardson > > - This deserves a RN entry has it impacts how users select compiled drivers. > > > - Trying to select net/intel/* triggers a meson error: > > $ meson configure build -Denable_drivers=net/intel/* > $ ninja -C build > ... > Message: drivers/net/intel/i40e: Defining dependency "net_i40e" > > ../drivers/net/intel/iavf/meson.build:33:46: ERROR: Unknown variable > "static_rte_common_iavf". > Thanks for catching that. I suspect in my testing with intel/* I also added common/iavf out of habit! Will fix in v6. > A full log can be found at > /home/dmarchan/git/pub/dpdk.org/dedup/build/meson-logs/meson-log.txt > FAILED: build.ninja > /usr/bin/meson --internal regenerate /home/dmarchan/git/pub/dpdk.org/dedup . > > > - I see some remaining references to the old path. One is to be fixed: > doc/guides/nics/ice.rst: These ICE_DBG_XXX are defined in > ``drivers/net/ice/base/ice_type.h``. > Ack, will fix. > > - Thomas, please have a look at this part. > > On the check-git-log.sh update, we will have many warnings with current > update. > > Wrong headline prefix: > net/intel/common: add pkt reassembly fn for intel drivers > net/intel/common: provide common Tx entry structures > net/intel/common: add Tx mbuf ring replenish fn > net/intel: align Tx queue struct field names > net/intel: add prefix for driver-specific structs > net/intel/common: merge ice and i40e Tx queue struct > net/iavf: use common Tx queue structure > net/ixgbe: use common Tx queue structure > net/intel/common: pack Tx queue structure > net/intel/common: add post-Tx buffer free function > net/intel/common: add Tx buffer free fn for AVX-512 > net/iavf: use common Tx free fn for AVX-512 > net/ice: move Tx queue mbuf cleanup fn to common > net/iavf: use common Tx queue mbuf cleanup fn > net/ice: use vector SW ring for all vector paths > net/intel/common: remove unneeded code > net/intel/common: create common mbuf initializer fn > net/intel/common: extract common Rx vector criteria > > Invalid patch(es) found - checked 25 patches > > I tried to tweak this a bit, with the following heuristic: > * if touching only net/intel/common, accept net/intel/common: > * if touching multiple drivers under net/intel, then accept net/intel: > as prefix, > * if touching some net/intel/$drv (and optionnally net/intel/common), > accept net/$drv, > > diff --git a/devtools/check-git-log.sh b/devtools/check-git-log.sh > index b2da013f6c..41c290f0ca 100755 > --- a/devtools/check-git-log.sh > +++ b/devtools/check-git-log.sh > @@ -79,11 +79,18 @@ bad=$(for commit in $commits ; do > [ -z "$(echo "$files" | grep -v '^\(drivers\|doc\|config\)/')" ] || > continue > drv=$(echo "$files" | grep '^drivers/' | cut -d "/" -f 2,3 | sort -u) > - # for drivers/net/intel/* use 2nd and 4th fields not 2nd and 3rd > if [ "$drv" = "net/intel" ] ; then > - drv=$(echo "$files" | grep '^drivers/' | cut -d "/" -f > 2,4 | sort -u) > + drvgrp="net/intel" > + drv=$(echo "$files" | grep '^drivers/' | grep -v > '^drivers/net/intel/common' | > + cut -d "/" -f 2,4 | sort -u) > + if [ $(echo "$drv" | wc -l) -eq 0 ] ; then > + drv='net/intel/common:' > + elif [ $(echo "$drv" | wc -l) -gt 1 ] ; then > + drv='net/intel:' > + fi > + else > + drvgrp=$(echo "$drv" | cut -d "/" -f 1 | uniq) > fi > - drvgrp=$(echo "$drv" | cut -d "/" -f 1 | uniq) > if [ $(echo "$drvgrp" | wc -l) -gt 1 ] ; then > echo "$headline" | grep -v '^drivers:' > elif [ $(echo "$drv" | wc -l) -gt 1 ] ; then > > Which then complains on patches in this series that touch many drivers > (but have net/intel/common: as prefix where I would suggest net/intel: > instead). > I tend to disagree with this suggestion. I think that a prefix can be valid so long as the prefix matches at least one component in the patch. For example, for the first patch in the set, I think net/intel/common is a better prefix than just "net/intel". I don't massively object to your suggestion, I just prefer patches identify the most relevant component, if possible, rather than generalities. > Wrong headline prefix: > net/intel/common: add pkt reassembly fn for intel drivers > net/intel/common: provide common Tx entry structures > net/intel/common: add Tx mbuf ring replenish fn > net/intel/common: merge ice and i40e Tx queue struct >
Re: [PATCH v5 01/25] net: move intel drivers to intel subdirectory
Hello Bruce, Thomas, On Mon, Jan 20, 2025 at 1:00 PM Bruce Richardson wrote: > > Consolidate all Intel HW NIC drivers into a driver/net/intel This > matches the layout used for drivers in the kernel, and potentially > enabling easier sharing among drivers. > > Signed-off-by: Bruce Richardson - This deserves a RN entry has it impacts how users select compiled drivers. - Trying to select net/intel/* triggers a meson error: $ meson configure build -Denable_drivers=net/intel/* $ ninja -C build ... Message: drivers/net/intel/i40e: Defining dependency "net_i40e" ../drivers/net/intel/iavf/meson.build:33:46: ERROR: Unknown variable "static_rte_common_iavf". A full log can be found at /home/dmarchan/git/pub/dpdk.org/dedup/build/meson-logs/meson-log.txt FAILED: build.ninja /usr/bin/meson --internal regenerate /home/dmarchan/git/pub/dpdk.org/dedup . - I see some remaining references to the old path. One is to be fixed: doc/guides/nics/ice.rst: These ICE_DBG_XXX are defined in ``drivers/net/ice/base/ice_type.h``. - Thomas, please have a look at this part. On the check-git-log.sh update, we will have many warnings with current update. Wrong headline prefix: net/intel/common: add pkt reassembly fn for intel drivers net/intel/common: provide common Tx entry structures net/intel/common: add Tx mbuf ring replenish fn net/intel: align Tx queue struct field names net/intel: add prefix for driver-specific structs net/intel/common: merge ice and i40e Tx queue struct net/iavf: use common Tx queue structure net/ixgbe: use common Tx queue structure net/intel/common: pack Tx queue structure net/intel/common: add post-Tx buffer free function net/intel/common: add Tx buffer free fn for AVX-512 net/iavf: use common Tx free fn for AVX-512 net/ice: move Tx queue mbuf cleanup fn to common net/iavf: use common Tx queue mbuf cleanup fn net/ice: use vector SW ring for all vector paths net/intel/common: remove unneeded code net/intel/common: create common mbuf initializer fn net/intel/common: extract common Rx vector criteria Invalid patch(es) found - checked 25 patches I tried to tweak this a bit, with the following heuristic: * if touching only net/intel/common, accept net/intel/common: * if touching multiple drivers under net/intel, then accept net/intel: as prefix, * if touching some net/intel/$drv (and optionnally net/intel/common), accept net/$drv, diff --git a/devtools/check-git-log.sh b/devtools/check-git-log.sh index b2da013f6c..41c290f0ca 100755 --- a/devtools/check-git-log.sh +++ b/devtools/check-git-log.sh @@ -79,11 +79,18 @@ bad=$(for commit in $commits ; do [ -z "$(echo "$files" | grep -v '^\(drivers\|doc\|config\)/')" ] || continue drv=$(echo "$files" | grep '^drivers/' | cut -d "/" -f 2,3 | sort -u) - # for drivers/net/intel/* use 2nd and 4th fields not 2nd and 3rd if [ "$drv" = "net/intel" ] ; then - drv=$(echo "$files" | grep '^drivers/' | cut -d "/" -f 2,4 | sort -u) + drvgrp="net/intel" + drv=$(echo "$files" | grep '^drivers/' | grep -v '^drivers/net/intel/common' | + cut -d "/" -f 2,4 | sort -u) + if [ $(echo "$drv" | wc -l) -eq 0 ] ; then + drv='net/intel/common:' + elif [ $(echo "$drv" | wc -l) -gt 1 ] ; then + drv='net/intel:' + fi + else + drvgrp=$(echo "$drv" | cut -d "/" -f 1 | uniq) fi - drvgrp=$(echo "$drv" | cut -d "/" -f 1 | uniq) if [ $(echo "$drvgrp" | wc -l) -gt 1 ] ; then echo "$headline" | grep -v '^drivers:' elif [ $(echo "$drv" | wc -l) -gt 1 ] ; then Which then complains on patches in this series that touch many drivers (but have net/intel/common: as prefix where I would suggest net/intel: instead). Wrong headline prefix: net/intel/common: add pkt reassembly fn for intel drivers net/intel/common: provide common Tx entry structures net/intel/common: add Tx mbuf ring replenish fn net/intel/common: merge ice and i40e Tx queue struct net/intel/common: pack Tx queue structure net/intel/common: add post-Tx buffer free function net/intel/common: add Tx buffer free fn for AVX-512 net/iavf: use common Tx free fn for AVX-512 net/iavf: use common Tx queue mbuf cleanup fn net/intel/common: remove unneeded code net/intel/common: create common mbuf initializer fn net/intel/common: extract common Rx vector criteria -- David Marchand
Re: [PATCH v2 0/2] fix void function returning a value
On Thu, Jan 23, 2025 at 02:03:29PM +0100, David Marchand wrote: > On Thu, Jan 23, 2025 at 1:46 PM Bruce Richardson > wrote: > > > > On Thu, Jan 23, 2025 at 01:23:15PM +0100, David Marchand wrote: > > > Hello Bruce, > > > > > > On Wed, Jan 22, 2025 at 4:21 PM Andre Muezerie > > > wrote: > > > > > > > > v2: > > > > * Updated commit messages to follow standard format. > > > > > > > > This patch avoids warnings like the one below emitted by MSVC, and is > > > > needed to get the code to compile cleanly with MSVC. > > > > > > > > ../drivers/common/idpf/idpf_common_rxtx_avx512.c(139): > > > > warning C4098: 'idpf_singleq_rearm': > > > > 'void' function returning a value > > > > > > > > Andre Muezerie (2): > > > > drivers/common: fix void function returning a value > > > > drivers/net: fix void function returning a value > > > > > > > > drivers/common/idpf/idpf_common_rxtx_avx512.c | 12 > > > > drivers/net/i40e/i40e_rxtx_vec_avx2.c | 2 +- > > > > drivers/net/i40e/i40e_rxtx_vec_avx512.c | 2 +- > > > > drivers/net/iavf/iavf_rxtx_vec_avx2.c | 2 +- > > > > drivers/net/ice/ice_rxtx_vec_avx2.c | 2 +- > > > > 5 files changed, 12 insertions(+), 8 deletions(-) > > > > > > I see the series is delegated to the main repo (Thomas). > > > > > > This touches only Intel drivers and the code deduplication effort you > > > started may conflict (though trivially) with such changes depending on > > > when it lands. > > > > > > Would you mind merging this fixes from Andre through the > > > next-net-intel tree, right now? > > > If so, please mark it as delegated to you in patchwork. > > > > > > > > > Thanks! > > > > > Hi David, > > > > the code deduplication effort patchsets are similarly delegated to the main > > repo. I was assuming this was deliberate, but perhaps it isn't? I'm ok to > > I suspect it was delegated to main because of the change on > devtools/check-git-log.sh. > But it is really only about net driver changes, so it should go either > through your or Stephen tree. > (we have enough patches waiting in main ;-)) > I'm ok to take it in my tree, unless Stephen would rather in his tree. > > > take these patches in next-net-intel, but just would like to confirm that > > neither you, Thomas or Stephen (as net maintainer) want to review the dedup > > work ahead of that initial merge? > > I don't have an objection on this series (on the contrary, I am quite > happy to see such effort). > I'll have a deeper look at it, this afternoon. > Thanks. I'm having it[1] reviewed internally, but I'd appreciate even a cursory review, especially at the new directory layout and build implications, from the higher-level directory maintainers. /Bruce [1] https://patches.dpdk.org/project/dpdk/list/?series=34398
RE: [EXTERNAL] [PATCH] net/cnxk: toggle link status for representors
> -Original Message- > From: Harman Kalra > Sent: Thursday, November 14, 2024 3:09 PM > To: Nithin Kumar Dabilpuram ; Kiran Kumar > Kokkilagadda ; Sunil Kumar Kori > ; Satha Koteswara Rao Kottidi > ; Harman Kalra > Cc: dev@dpdk.org > Subject: [EXTERNAL] [PATCH] net/cnxk: toggle link status for representors > > Representor port link status should be toggled based on representee state i. > e. if > representee comes up representor link status should be up while when > representee goes down representor port should indicate the same. Signed-off- > by: Harman Kalra > Representor port link status should be toggled based on representee state > i.e. if > representee comes up representor link status should be up while when > representee goes down representor port should indicate the same. > > Signed-off-by: Harman Kalra Applied to dpdk-next-net-mrvl/for-main. Thanks <>
[PATCH v3 01/15] fib: add allocation function attributes
Use function attributes to catch cases where fib table is allocated
but not freed correctly.
Signed-off-by: Stephen Hemminger
---
lib/fib/rte_fib.h | 26 +++---
lib/fib/rte_fib6.h | 24 +---
lib/fib/trie.h | 7 ---
3 files changed, 32 insertions(+), 25 deletions(-)
diff --git a/lib/fib/rte_fib.h b/lib/fib/rte_fib.h
index 496d137d48..65c24d5459 100644
--- a/lib/fib/rte_fib.h
+++ b/lib/fib/rte_fib.h
@@ -17,8 +17,10 @@
#include
+#include
#include
+
#ifdef __cplusplus
extern "C" {
#endif
@@ -128,6 +130,17 @@ struct rte_fib_rcu_config {
uint32_t reclaim_max;
};
+
+/**
+ * Free an FIB object.
+ *
+ * @param fib
+ * FIB object handle created by rte_fib_create().
+ * If fib is NULL, no operation is performed.
+ */
+void
+rte_fib_free(struct rte_fib *fib);
+
/**
* Create FIB
*
@@ -142,7 +155,8 @@ struct rte_fib_rcu_config {
* NULL otherwise with rte_errno set to an appropriate values.
*/
struct rte_fib *
-rte_fib_create(const char *name, int socket_id, struct rte_fib_conf *conf);
+rte_fib_create(const char *name, int socket_id, struct rte_fib_conf *conf)
+ __rte_malloc __rte_dealloc(rte_fib_free, 1);
/**
* Find an existing FIB object and return a pointer to it.
@@ -157,16 +171,6 @@ rte_fib_create(const char *name, int socket_id, struct
rte_fib_conf *conf);
struct rte_fib *
rte_fib_find_existing(const char *name);
-/**
- * Free an FIB object.
- *
- * @param fib
- * FIB object handle created by rte_fib_create().
- * If fib is NULL, no operation is performed.
- */
-void
-rte_fib_free(struct rte_fib *fib);
-
/**
* Add a route to the FIB.
*
diff --git a/lib/fib/rte_fib6.h b/lib/fib/rte_fib6.h
index 21f0492374..b03b24421c 100644
--- a/lib/fib/rte_fib6.h
+++ b/lib/fib/rte_fib6.h
@@ -82,6 +82,17 @@ struct rte_fib6_conf {
};
};
+
+/**
+ * Free an FIB object.
+ *
+ * @param fib
+ * FIB object handle created by rte_fib6_create().
+ * If fib is NULL, no operation is performed.
+ */
+void
+rte_fib6_free(struct rte_fib6 *fib);
+
/**
* Create FIB
*
@@ -96,7 +107,8 @@ struct rte_fib6_conf {
* NULL otherwise with rte_errno set to an appropriate values.
*/
struct rte_fib6 *
-rte_fib6_create(const char *name, int socket_id, struct rte_fib6_conf *conf);
+rte_fib6_create(const char *name, int socket_id, struct rte_fib6_conf *conf)
+ __rte_malloc __rte_dealloc(rte_fib6_free, 1);
/**
* Find an existing FIB object and return a pointer to it.
@@ -111,16 +123,6 @@ rte_fib6_create(const char *name, int socket_id, struct
rte_fib6_conf *conf);
struct rte_fib6 *
rte_fib6_find_existing(const char *name);
-/**
- * Free an FIB object.
- *
- * @param fib
- * FIB object handle created by rte_fib6_create().
- * If fib is NULL, no operation is performed.
- */
-void
-rte_fib6_free(struct rte_fib6 *fib);
-
/**
* Add a route to the FIB.
*
diff --git a/lib/fib/trie.h b/lib/fib/trie.h
index f87fc0f6d2..bcb161702b 100644
--- a/lib/fib/trie.h
+++ b/lib/fib/trie.h
@@ -129,12 +129,13 @@ LOOKUP_FUNC(2b, uint16_t, 1)
LOOKUP_FUNC(4b, uint32_t, 2)
LOOKUP_FUNC(8b, uint64_t, 3)
-void *
-trie_create(const char *name, int socket_id, struct rte_fib6_conf *conf);
-
void
trie_free(void *p);
+void *
+trie_create(const char *name, int socket_id, struct rte_fib6_conf *conf)
+ __rte_malloc __rte_dealloc(trie_free, 1);
+
rte_fib6_lookup_fn_t
trie_get_lookup_fn(void *p, enum rte_fib6_lookup_type type);
--
2.45.2
[PATCH v3 02/15] rib: annotate rib allocation functions
Add function attributes to catch cases where rib is allocated and not freed correctly. Signed-off-by: Stephen Hemminger --- lib/rib/rte_rib.h | 24 +--- lib/rib/rte_rib6.h | 24 +--- 2 files changed, 26 insertions(+), 22 deletions(-) diff --git a/lib/rib/rte_rib.h b/lib/rib/rte_rib.h index 2054d3cebd..f30b85d79a 100644 --- a/lib/rib/rte_rib.h +++ b/lib/rib/rte_rib.h @@ -230,6 +230,17 @@ rte_rib_get_nh(const struct rte_rib_node *node, uint64_t *nh); int rte_rib_set_nh(struct rte_rib_node *node, uint64_t nh); + +/** + * Free an RIB object. + * + * @param rib + * RIB object handle created with rte_rib_create(). + * If rib is NULL, no operation is performed. + */ +void +rte_rib_free(struct rte_rib *rib); + /** * Create RIB * @@ -245,7 +256,8 @@ rte_rib_set_nh(struct rte_rib_node *node, uint64_t nh); */ struct rte_rib * rte_rib_create(const char *name, int socket_id, - const struct rte_rib_conf *conf); + const struct rte_rib_conf *conf) + __rte_malloc __rte_dealloc(rte_rib_free, 1); /** * Find an existing RIB object and return a pointer to it. @@ -259,16 +271,6 @@ rte_rib_create(const char *name, int socket_id, struct rte_rib * rte_rib_find_existing(const char *name); -/** - * Free an RIB object. - * - * @param rib - * RIB object handle created with rte_rib_create(). - * If rib is NULL, no operation is performed. - */ -void -rte_rib_free(struct rte_rib *rib); - #ifdef __cplusplus } #endif diff --git a/lib/rib/rte_rib6.h b/lib/rib/rte_rib6.h index a60756f798..d9514acf82 100644 --- a/lib/rib/rte_rib6.h +++ b/lib/rib/rte_rib6.h @@ -294,6 +294,17 @@ rte_rib6_get_nh(const struct rte_rib6_node *node, uint64_t *nh); int rte_rib6_set_nh(struct rte_rib6_node *node, uint64_t nh); + +/** + * Free an RIB object. + * + * @param rib + * RIB object handle created with rte_rib6_create(). + * If rib is NULL, no operation is performed. + */ +void +rte_rib6_free(struct rte_rib6 *rib); + /** * Create RIB * @@ -309,7 +320,8 @@ rte_rib6_set_nh(struct rte_rib6_node *node, uint64_t nh); */ struct rte_rib6 * rte_rib6_create(const char *name, int socket_id, - const struct rte_rib6_conf *conf); + const struct rte_rib6_conf *conf) + __rte_malloc __rte_dealloc(rte_rib6_free, 1); /** * Find an existing RIB object and return a pointer to it. @@ -323,16 +335,6 @@ rte_rib6_create(const char *name, int socket_id, struct rte_rib6 * rte_rib6_find_existing(const char *name); -/** - * Free an RIB object. - * - * @param rib - * RIB object handle created with rte_rib6_create(). - * If rib is NULL, no operation is performed. - */ -void -rte_rib6_free(struct rte_rib6 *rib); - #ifdef __cplusplus } #endif -- 2.45.2
[PATCH v3 03/15] hash: add allocation function attributes
Use function attributes to catch cases where hash table is allocated
but not freed correctly.
Signed-off-by: Stephen Hemminger
---
lib/hash/rte_fbk_hash.h | 24 +---
lib/hash/rte_hash.h | 21 +++--
2 files changed, 24 insertions(+), 21 deletions(-)
diff --git a/lib/hash/rte_fbk_hash.h b/lib/hash/rte_fbk_hash.h
index 1f0c1d1b6c..b1a43f37b4 100644
--- a/lib/hash/rte_fbk_hash.h
+++ b/lib/hash/rte_fbk_hash.h
@@ -322,6 +322,16 @@ rte_fbk_hash_get_load_factor(struct rte_fbk_hash_table *ht)
*/
struct rte_fbk_hash_table *rte_fbk_hash_find_existing(const char *name);
+
+/**
+ * Free all memory used by a hash table.
+ * Has no effect on hash tables allocated in memory zones
+ *
+ * @param ht
+ * Hash table to deallocate.
+ */
+void rte_fbk_hash_free(struct rte_fbk_hash_table *ht);
+
/**
* Create a new hash table for use with four byte keys.
*
@@ -339,17 +349,9 @@ struct rte_fbk_hash_table
*rte_fbk_hash_find_existing(const char *name);
*- EEXIST - a memzone with the same name already exists
*- ENOMEM - no appropriate memory area found in which to create memzone
*/
-struct rte_fbk_hash_table * \
-rte_fbk_hash_create(const struct rte_fbk_hash_params *params);
-
-/**
- * Free all memory used by a hash table.
- * Has no effect on hash tables allocated in memory zones
- *
- * @param ht
- * Hash table to deallocate.
- */
-void rte_fbk_hash_free(struct rte_fbk_hash_table *ht);
+struct rte_fbk_hash_table *
+rte_fbk_hash_create(const struct rte_fbk_hash_params *params)
+ __rte_malloc __rte_dealloc(rte_fbk_hash_free, 1);
#ifdef __cplusplus
}
diff --git a/lib/hash/rte_hash.h b/lib/hash/rte_hash.h
index 05ab447e4a..736fb15885 100644
--- a/lib/hash/rte_hash.h
+++ b/lib/hash/rte_hash.h
@@ -125,6 +125,15 @@ struct rte_hash_rcu_config {
/** @internal A hash table structure. */
struct rte_hash;
+/**
+ * De-allocate all memory used by hash table.
+ *
+ * @param h
+ * Hash table to free, if NULL, the function does nothing.
+ */
+void
+rte_hash_free(struct rte_hash *h);
+
/**
* Create a new hash table.
*
@@ -143,7 +152,8 @@ struct rte_hash;
*- ENOMEM - no appropriate memory area found in which to create memzone
*/
struct rte_hash *
-rte_hash_create(const struct rte_hash_parameters *params);
+rte_hash_create(const struct rte_hash_parameters *params)
+ __rte_malloc __rte_dealloc(rte_hash_free, 1);
/**
* Set a new hash compare function other than the default one.
@@ -171,15 +181,6 @@ void rte_hash_set_cmp_func(struct rte_hash *h,
rte_hash_cmp_eq_t func);
struct rte_hash *
rte_hash_find_existing(const char *name);
-/**
- * De-allocate all memory used by hash table.
- *
- * @param h
- * Hash table to free, if NULL, the function does nothing.
- */
-void
-rte_hash_free(struct rte_hash *h);
-
/**
* Reset all hash structure, by zeroing all entries.
* When RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF is enabled,
--
2.45.2
[PATCH v3 00/15] Add attributes to allocation functions
This patch series builds on the allocation function attributes added in 24.11 release. These annotations will allow for compiler to flag cases where a pointer is allocated with one function but incorrectly passed to a different free function. The current code base does this correctly now, but adding attributes will catch future bugs, or errors in user programs. For each of these patches, the free function prototype needs to be reordered to be before the function attribute of the allocator. Checkpatch perl script falsely complains in a couple patches because it doesn't really understand C syntax for attributes. v3 - fix another spot where free function prototype needs to be moved. Stephen Hemminger (15): fib: add allocation function attributes rib: annotate rib allocation functions hash: add allocation function attributes lpm: add allocation function attributes pipeline: add allocation function attributes acl: add allocation function attributes bitratestats: add allocation function attributes member: add allocation function attributes mempool: add allocation function attributes eventdev: add allocation function attributes ring: add allocation function attributes reorder: add allocation function attributes compressdev: add allocation function attributes telemetry: add allocation function attributes sched: add allocation function attributes lib/acl/rte_acl.h | 26 --- lib/bitratestats/rte_bitrate.h| 20 +-- lib/compressdev/rte_comp.h| 28 lib/eventdev/rte_event_ring.h | 27 +++ lib/fib/rte_fib.h | 26 --- lib/fib/rte_fib6.h| 24 +++--- lib/fib/trie.h| 7 ++-- lib/hash/rte_fbk_hash.h | 24 +++--- lib/hash/rte_hash.h | 21 ++-- lib/lpm/rte_lpm.h | 23 ++--- lib/lpm/rte_lpm6.h| 23 ++--- lib/member/rte_member.h | 24 +++--- lib/mempool/rte_mempool.h | 37 +++-- lib/pipeline/rte_port_in_action.h | 55 --- lib/pipeline/rte_table_action.h | 53 +++-- lib/reorder/rte_reorder.h | 23 ++--- lib/rib/rte_rib.h | 24 +++--- lib/rib/rte_rib6.h| 24 +++--- lib/ring/rte_ring.h | 22 +++-- lib/sched/rte_sched.h | 23 +++-- lib/telemetry/rte_telemetry.h | 21 ++-- 21 files changed, 296 insertions(+), 259 deletions(-) -- 2.45.2
[PATCH v3 04/15] lpm: add allocation function attributes
Use function attributes to catch cases where lpm table is allocated
but not freed correctly.
Signed-off-by: Stephen Hemminger
---
lib/lpm/rte_lpm.h | 23 ---
lib/lpm/rte_lpm6.h | 23 ---
2 files changed, 24 insertions(+), 22 deletions(-)
diff --git a/lib/lpm/rte_lpm.h b/lib/lpm/rte_lpm.h
index 329dc1aad4..7df64f06b1 100644
--- a/lib/lpm/rte_lpm.h
+++ b/lib/lpm/rte_lpm.h
@@ -140,6 +140,16 @@ struct rte_lpm_rcu_config {
*/
};
+/**
+ * Free an LPM object.
+ *
+ * @param lpm
+ * LPM object handle
+ * If lpm is NULL, no operation is performed.
+ */
+void
+rte_lpm_free(struct rte_lpm *lpm);
+
/**
* Create an LPM object.
*
@@ -161,7 +171,8 @@ struct rte_lpm_rcu_config {
*/
struct rte_lpm *
rte_lpm_create(const char *name, int socket_id,
- const struct rte_lpm_config *config);
+ const struct rte_lpm_config *config)
+ __rte_malloc __rte_dealloc(rte_lpm_free, 1);
/**
* Find an existing LPM object and return a pointer to it.
@@ -176,16 +187,6 @@ rte_lpm_create(const char *name, int socket_id,
struct rte_lpm *
rte_lpm_find_existing(const char *name);
-/**
- * Free an LPM object.
- *
- * @param lpm
- * LPM object handle
- * If lpm is NULL, no operation is performed.
- */
-void
-rte_lpm_free(struct rte_lpm *lpm);
-
/**
* Associate RCU QSBR variable with an LPM object.
*
diff --git a/lib/lpm/rte_lpm6.h b/lib/lpm/rte_lpm6.h
index 079187ca56..08b5618613 100644
--- a/lib/lpm/rte_lpm6.h
+++ b/lib/lpm/rte_lpm6.h
@@ -34,6 +34,16 @@ struct rte_lpm6_config {
int flags; /**< This field is currently unused. */
};
+/**
+ * Free an LPM object.
+ *
+ * @param lpm
+ * LPM object handle
+ * If lpm is NULL, no operation is performed.
+ */
+void
+rte_lpm6_free(struct rte_lpm6 *lpm);
+
/**
* Create an LPM object.
*
@@ -55,7 +65,8 @@ struct rte_lpm6_config {
*/
struct rte_lpm6 *
rte_lpm6_create(const char *name, int socket_id,
- const struct rte_lpm6_config *config);
+ const struct rte_lpm6_config *config)
+ __rte_malloc __rte_dealloc(rte_lpm6_free, 1);
/**
* Find an existing LPM object and return a pointer to it.
@@ -70,16 +81,6 @@ rte_lpm6_create(const char *name, int socket_id,
struct rte_lpm6 *
rte_lpm6_find_existing(const char *name);
-/**
- * Free an LPM object.
- *
- * @param lpm
- * LPM object handle
- * If lpm is NULL, no operation is performed.
- */
-void
-rte_lpm6_free(struct rte_lpm6 *lpm);
-
/**
* Add a rule to the LPM table.
*
--
2.45.2
[PATCH v3 11/15] ring: add allocation function attributes
Use function attributes to catch cases where ring is allocated but not freed correctly. Signed-off-by: Stephen Hemminger --- lib/ring/rte_ring.h | 22 -- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/lib/ring/rte_ring.h b/lib/ring/rte_ring.h index 63a71d5871..15340a1981 100644 --- a/lib/ring/rte_ring.h +++ b/lib/ring/rte_ring.h @@ -119,6 +119,16 @@ ssize_t rte_ring_get_memsize(unsigned int count); int rte_ring_init(struct rte_ring *r, const char *name, unsigned int count, unsigned int flags); + +/** + * De-allocate all memory used by the ring. + * + * @param r + * Ring to free. + * If NULL then, the function does nothing. + */ +void rte_ring_free(struct rte_ring *r); + /** * Create a new ring named *name* in memory. * @@ -183,16 +193,8 @@ int rte_ring_init(struct rte_ring *r, const char *name, unsigned int count, *- ENOMEM - no appropriate memory area found in which to create memzone */ struct rte_ring *rte_ring_create(const char *name, unsigned int count, -int socket_id, unsigned int flags); - -/** - * De-allocate all memory used by the ring. - * - * @param r - * Ring to free. - * If NULL then, the function does nothing. - */ -void rte_ring_free(struct rte_ring *r); +int socket_id, unsigned int flags) + __rte_malloc __rte_dealloc(rte_ring_free, 1); /** * Dump the status of the ring to a file. -- 2.45.2
[PATCH v3 06/15] acl: add allocation function attributes
Use function attributes to catch cases where acl table is allocated
but not freed correctly.
Signed-off-by: Stephen Hemminger
---
lib/acl/rte_acl.h | 26 +++---
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/lib/acl/rte_acl.h b/lib/acl/rte_acl.h
index ca75a6f220..b95f8778c3 100644
--- a/lib/acl/rte_acl.h
+++ b/lib/acl/rte_acl.h
@@ -133,6 +133,19 @@ struct rte_acl_param {
};
+/** @internal opaque ACL handle */
+struct rte_acl_ctx;
+
+/**
+ * De-allocate all memory used by ACL context.
+ *
+ * @param ctx
+ * ACL context to free
+ * If ctx is NULL, no operation is performed.
+ */
+void
+rte_acl_free(struct rte_acl_ctx *ctx);
+
/**
* Create a new ACL context.
*
@@ -145,7 +158,8 @@ struct rte_acl_param {
* - EINVAL - invalid parameter passed to function
*/
struct rte_acl_ctx *
-rte_acl_create(const struct rte_acl_param *param);
+rte_acl_create(const struct rte_acl_param *param)
+ __rte_malloc __rte_dealloc(rte_acl_free, 1);
/**
* Find an existing ACL context object and return a pointer to it.
@@ -160,16 +174,6 @@ rte_acl_create(const struct rte_acl_param *param);
struct rte_acl_ctx *
rte_acl_find_existing(const char *name);
-/**
- * De-allocate all memory used by ACL context.
- *
- * @param ctx
- * ACL context to free
- * If ctx is NULL, no operation is performed.
- */
-void
-rte_acl_free(struct rte_acl_ctx *ctx);
-
/**
* Add rules to an existing ACL context.
* This function is not multi-thread safe.
--
2.45.2
[PATCH v3 05/15] pipeline: add allocation function attributes
Use function attributes to catch cases where pipeline is allocated
but not freed correctly.
Signed-off-by: Stephen Hemminger
---
lib/pipeline/rte_port_in_action.h | 55 ---
lib/pipeline/rte_table_action.h | 53 +++--
2 files changed, 56 insertions(+), 52 deletions(-)
diff --git a/lib/pipeline/rte_port_in_action.h
b/lib/pipeline/rte_port_in_action.h
index 9d17bae988..ee6cc59fae 100644
--- a/lib/pipeline/rte_port_in_action.h
+++ b/lib/pipeline/rte_port_in_action.h
@@ -164,18 +164,6 @@ struct rte_port_in_action_lb_params {
*/
struct rte_port_in_action_profile;
-/**
- * Input port action profile create.
- *
- * @param[in] socket_id
- * CPU socket ID for the internal data structures memory allocation.
- * @return
- * Input port action profile handle on success, NULL otherwise.
- */
-__rte_experimental
-struct rte_port_in_action_profile *
-rte_port_in_action_profile_create(uint32_t socket_id);
-
/**
* Input port action profile free.
*
@@ -189,6 +177,19 @@ __rte_experimental
int
rte_port_in_action_profile_free(struct rte_port_in_action_profile *profile);
+/**
+ * Input port action profile create.
+ *
+ * @param[in] socket_id
+ * CPU socket ID for the internal data structures memory allocation.
+ * @return
+ * Input port action profile handle on success, NULL otherwise.
+ */
+__rte_experimental
+struct rte_port_in_action_profile *
+rte_port_in_action_profile_create(uint32_t socket_id)
+ __rte_malloc __rte_dealloc(rte_port_in_action_profile_free, 1);
+
/**
* Input port action profile action register.
*
@@ -236,6 +237,19 @@ rte_port_in_action_profile_freeze(struct
rte_port_in_action_profile *profile);
*/
struct rte_port_in_action;
+/**
+ * Input port action free.
+ *
+ * @param[in] action
+ * Handle to input port action object (needs to be valid).
+ * If action is NULL, no operation is performed.
+ * @return
+ * Always zero.
+ */
+__rte_experimental
+int
+rte_port_in_action_free(struct rte_port_in_action *action);
+
/**
* Input port action create.
*
@@ -252,21 +266,8 @@ struct rte_port_in_action;
*/
__rte_experimental
struct rte_port_in_action *
-rte_port_in_action_create(struct rte_port_in_action_profile *profile,
- uint32_t socket_id);
-
-/**
- * Input port action free.
- *
- * @param[in] action
- * Handle to input port action object (needs to be valid).
- * If action is NULL, no operation is performed.
- * @return
- * Always zero.
- */
-__rte_experimental
-int
-rte_port_in_action_free(struct rte_port_in_action *action);
+rte_port_in_action_create(struct rte_port_in_action_profile *profile, uint32_t
socket_id)
+ __rte_malloc __rte_dealloc(rte_port_in_action_free, 1);
/**
* Input port params get.
diff --git a/lib/pipeline/rte_table_action.h b/lib/pipeline/rte_table_action.h
index 47a7bdfc01..e8b4d8b33d 100644
--- a/lib/pipeline/rte_table_action.h
+++ b/lib/pipeline/rte_table_action.h
@@ -54,6 +54,7 @@
#include
+#include
#include
#include
#include
@@ -812,17 +813,6 @@ struct rte_table_action_decap_params {
*/
struct rte_table_action_profile;
-/**
- * Table action profile create.
- *
- * @param[in] common
- * Common action configuration.
- * @return
- * Table action profile handle on success, NULL otherwise.
- */
-__rte_experimental
-struct rte_table_action_profile *
-rte_table_action_profile_create(struct rte_table_action_common_config *common);
/**
* Table action profile free.
@@ -836,6 +826,19 @@ __rte_experimental
int
rte_table_action_profile_free(struct rte_table_action_profile *profile);
+/**
+ * Table action profile create.
+ *
+ * @param[in] common
+ * Common action configuration.
+ * @return
+ * Table action profile handle on success, NULL otherwise.
+ */
+__rte_experimental
+struct rte_table_action_profile *
+rte_table_action_profile_create(struct rte_table_action_common_config *common)
+ __rte_malloc __rte_dealloc(rte_table_action_profile_free, 1);
+
/**
* Table action profile action register.
*
@@ -881,6 +884,18 @@ rte_table_action_profile_freeze(struct
rte_table_action_profile *profile);
*/
struct rte_table_action;
+/**
+ * Table action free.
+ *
+ * @param[in] action
+ * Handle to table action object (needs to be valid).
+ * @return
+ * Zero on success, non-zero error code otherwise.
+ */
+__rte_experimental
+int
+rte_table_action_free(struct rte_table_action *action);
+
/**
* Table action create.
*
@@ -898,20 +913,8 @@ struct rte_table_action;
*/
__rte_experimental
struct rte_table_action *
-rte_table_action_create(struct rte_table_action_profile *profile,
- uint32_t socket_id);
-
-/**
- * Table action free.
- *
- * @param[in] action
- * Handle to table action object (needs to be valid).
- * @return
- * Zero on success, non-zero error code otherwise.
- */
-__rte_experimental
-int
-rte_table_action_free(struct rte_table_action *action);
+rte_table_action_create(struct rte_table_action_profile *p
[PATCH v3 07/15] bitratestats: add allocation function attributes
Use function attributes to catch cases where bitratestats is allocated
but not freed correctly.
Signed-off-by: Stephen Hemminger
---
lib/bitratestats/rte_bitrate.h | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/lib/bitratestats/rte_bitrate.h b/lib/bitratestats/rte_bitrate.h
index 979a712837..27951f3e28 100644
--- a/lib/bitratestats/rte_bitrate.h
+++ b/lib/bitratestats/rte_bitrate.h
@@ -17,16 +17,6 @@ extern "C" {
*/
struct rte_stats_bitrates;
-
-/**
- * Allocate a bitrate statistics structure
- *
- * @return
- * - Pointer to structure on success
- * - NULL on error (zmalloc failure)
- */
-struct rte_stats_bitrates *rte_stats_bitrate_create(void);
-
/**
* Free bitrate statistics structure
*
@@ -36,6 +26,16 @@ struct rte_stats_bitrates *rte_stats_bitrate_create(void);
*/
void rte_stats_bitrate_free(struct rte_stats_bitrates *bitrate_data);
+/**
+ * Allocate a bitrate statistics structure
+ *
+ * @return
+ * - Pointer to structure on success
+ * - NULL on error (zmalloc failure)
+ */
+struct rte_stats_bitrates *rte_stats_bitrate_create(void)
+ __rte_malloc __rte_dealloc(rte_stats_bitrate_free, 1);
+
/**
* Register bitrate statistics with the metric library.
*
--
2.45.2
[PATCH v3 08/15] member: add allocation function attributes
Use function attributes to catch cases where member table is allocated
but not freed correctly.
Signed-off-by: Stephen Hemminger
---
lib/member/rte_member.h | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/lib/member/rte_member.h b/lib/member/rte_member.h
index 109bdd000b..0235bb0a81 100644
--- a/lib/member/rte_member.h
+++ b/lib/member/rte_member.h
@@ -341,6 +341,16 @@ struct __rte_cache_aligned rte_member_parameters {
struct rte_member_setsum *
rte_member_find_existing(const char *name);
+/**
+ * De-allocate memory used by set-summary.
+ *
+ * @param setsum
+ * Pointer to the set summary.
+ * If setsum is NULL, no operation is performed.
+ */
+void
+rte_member_free(struct rte_member_setsum *setsum);
+
/**
* Create set-summary (SS).
*
@@ -351,7 +361,8 @@ rte_member_find_existing(const char *name);
* Return value is NULL if the creation failed.
*/
struct rte_member_setsum *
-rte_member_create(const struct rte_member_parameters *params);
+rte_member_create(const struct rte_member_parameters *params)
+ __rte_malloc __rte_dealloc(rte_member_free, 1);
/**
* Lookup key in set-summary (SS).
@@ -528,17 +539,6 @@ int
rte_member_report_heavyhitter(const struct rte_member_setsum *setsum,
void **keys, uint64_t *counts);
-
-/**
- * De-allocate memory used by set-summary.
- *
- * @param setsum
- * Pointer to the set summary.
- * If setsum is NULL, no operation is performed.
- */
-void
-rte_member_free(struct rte_member_setsum *setsum);
-
/**
* Reset the set-summary tables. E.g. reset bits to be 0 in BF,
* reset set_id in each entry to be RTE_MEMBER_NO_MATCH in HT based SS.
--
2.45.2
[PATCH v3 10/15] eventdev: add allocation function attributes
Use function attributes to catch cases where eventdev is allocated but not freed correctly. Signed-off-by: Stephen Hemminger --- lib/eventdev/rte_event_ring.h | 27 ++- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/lib/eventdev/rte_event_ring.h b/lib/eventdev/rte_event_ring.h index 5769da269e..a8f308e4d6 100644 --- a/lib/eventdev/rte_event_ring.h +++ b/lib/eventdev/rte_event_ring.h @@ -247,7 +247,18 @@ int rte_event_ring_init(struct rte_event_ring *r, const char *name, unsigned int count, unsigned int flags); -/* + +/** + * De-allocate all memory used by the ring. + * + * @param r + * Pointer to ring to created with rte_event_ring_create(). + * If r is NULL, no operation is performed. + */ +void +rte_event_ring_free(struct rte_event_ring *r); + +/** * Create an event ring structure * * This function allocates memory and initializes an event ring inside that @@ -288,8 +299,8 @@ rte_event_ring_init(struct rte_event_ring *r, const char *name, *- ENOMEM - no appropriate memory area found in which to create memzone */ struct rte_event_ring * -rte_event_ring_create(const char *name, unsigned int count, int socket_id, - unsigned int flags); +rte_event_ring_create(const char *name, unsigned int count, int socket_id, unsigned int flags) + __rte_malloc __rte_dealloc(rte_event_ring_free, 1); /** * Search for an event ring based on its name @@ -304,16 +315,6 @@ rte_event_ring_create(const char *name, unsigned int count, int socket_id, struct rte_event_ring * rte_event_ring_lookup(const char *name); -/** - * De-allocate all memory used by the ring. - * - * @param r - * Pointer to ring to created with rte_event_ring_create(). - * If r is NULL, no operation is performed. - */ -void -rte_event_ring_free(struct rte_event_ring *r); - /** * Return the size of the event ring. * -- 2.45.2
[PATCH v3 09/15] mempool: add allocation function attributes
Use function attributes to catch cases where mempool is allocated but not freed correctly. Signed-off-by: Stephen Hemminger Reviewed-by: Morten Brørup --- lib/mempool/rte_mempool.h | 37 - 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/lib/mempool/rte_mempool.h b/lib/mempool/rte_mempool.h index 7bdc92b812..c495cc012f 100644 --- a/lib/mempool/rte_mempool.h +++ b/lib/mempool/rte_mempool.h @@ -1012,6 +1012,20 @@ typedef void (rte_mempool_mem_cb_t)(struct rte_mempool *mp, */ typedef void (rte_mempool_ctor_t)(struct rte_mempool *, void *); +/** + * Free a mempool + * + * Unlink the mempool from global list, free the memory chunks, and all + * memory referenced by the mempool. The objects must not be used by + * other cores as they will be freed. + * + * @param mp + * A pointer to the mempool structure. + * If NULL then, the function does nothing. + */ +void +rte_mempool_free(struct rte_mempool *mp); + /** * Create a new mempool named *name* in memory. * @@ -1095,7 +1109,8 @@ rte_mempool_create(const char *name, unsigned n, unsigned elt_size, unsigned cache_size, unsigned private_data_size, rte_mempool_ctor_t *mp_init, void *mp_init_arg, rte_mempool_obj_cb_t *obj_init, void *obj_init_arg, - int socket_id, unsigned flags); + int socket_id, unsigned int flags) + __rte_malloc __rte_dealloc(rte_mempool_free, 1); /** * Create an empty mempool @@ -1132,22 +1147,10 @@ rte_mempool_create(const char *name, unsigned n, unsigned elt_size, * with rte_errno set appropriately. See rte_mempool_create() for details. */ struct rte_mempool * -rte_mempool_create_empty(const char *name, unsigned n, unsigned elt_size, - unsigned cache_size, unsigned private_data_size, - int socket_id, unsigned flags); -/** - * Free a mempool - * - * Unlink the mempool from global list, free the memory chunks, and all - * memory referenced by the mempool. The objects must not be used by - * other cores as they will be freed. - * - * @param mp - * A pointer to the mempool structure. - * If NULL then, the function does nothing. - */ -void -rte_mempool_free(struct rte_mempool *mp); +rte_mempool_create_empty(const char *name, unsigned int n, unsigned int elt_size, +unsigned int cache_size, unsigned int private_data_size, +int socket_id, unsigned int flags) + __rte_malloc __rte_dealloc(rte_mempool_free, 1); /** * Add physically contiguous memory for objects in the pool at init -- 2.45.2
[PATCH v3 12/15] reorder: add allocation function attributes
Use function attributes to catch cases where reorder table is allocated but not freed correctly. Signed-off-by: Stephen Hemminger Acked-by: Volodymyr Fialko --- lib/reorder/rte_reorder.h | 23 --- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/lib/reorder/rte_reorder.h b/lib/reorder/rte_reorder.h index 56a6507f9f..2f26ed7df3 100644 --- a/lib/reorder/rte_reorder.h +++ b/lib/reorder/rte_reorder.h @@ -44,6 +44,16 @@ rte_reorder_seqn(struct rte_mbuf *mbuf) rte_reorder_seqn_t *); } +/** + * Free reorder buffer instance. + * + * @param b + * Pointer to reorder buffer instance. + * If b is NULL, no operation is performed. + */ +void +rte_reorder_free(struct rte_reorder_buffer *b); + /** * Create a new reorder buffer instance * @@ -64,7 +74,8 @@ rte_reorder_seqn(struct rte_mbuf *mbuf) *- EINVAL - invalid parameters */ struct rte_reorder_buffer * -rte_reorder_create(const char *name, unsigned socket_id, unsigned int size); +rte_reorder_create(const char *name, unsigned int socket_id, unsigned int size) + __rte_malloc __rte_dealloc(rte_reorder_free, 1); /** * Initializes given reorder buffer instance @@ -111,16 +122,6 @@ rte_reorder_find_existing(const char *name); void rte_reorder_reset(struct rte_reorder_buffer *b); -/** - * Free reorder buffer instance. - * - * @param b - * Pointer to reorder buffer instance. - * If b is NULL, no operation is performed. - */ -void -rte_reorder_free(struct rte_reorder_buffer *b); - /** * Insert given mbuf in reorder buffer in its correct position * -- 2.45.2
[PATCH v3 15/15] sched: add allocation function attributes
Use function attributes to catch cases where sched port config
is allocated but not freed correctly.
Signed-off-by: Stephen Hemminger
---
lib/sched/rte_sched.h | 23 +--
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/lib/sched/rte_sched.h b/lib/sched/rte_sched.h
index 222e6b3583..7ae570aa1b 100644
--- a/lib/sched/rte_sched.h
+++ b/lib/sched/rte_sched.h
@@ -310,16 +310,7 @@ struct rte_sched_port_params {
* Configuration
*/
-/**
- * Hierarchical scheduler port configuration
- *
- * @param params
- * Port scheduler configuration parameter structure
- * @return
- * Handle to port scheduler instance upon success or NULL otherwise.
- */
-struct rte_sched_port *
-rte_sched_port_config(struct rte_sched_port_params *params);
+struct rte_sched_port;
/**
* Hierarchical scheduler port free
@@ -331,6 +322,18 @@ rte_sched_port_config(struct rte_sched_port_params
*params);
void
rte_sched_port_free(struct rte_sched_port *port);
+/**
+ * Hierarchical scheduler port configuration
+ *
+ * @param params
+ * Port scheduler configuration parameter structure
+ * @return
+ * Handle to port scheduler instance upon success or NULL otherwise.
+ */
+struct rte_sched_port *
+rte_sched_port_config(struct rte_sched_port_params *params)
+ __rte_malloc __rte_dealloc(rte_sched_port_free, 1);
+
/**
* Hierarchical scheduler pipe profile add
*
--
2.45.2
[PATCH v3 13/15] compressdev: add allocation function attributes
Use function attributes to catch cases where compressdev is allocated
but not freed correctly.
Signed-off-by: Stephen Hemminger
---
lib/compressdev/rte_comp.h | 28 +++-
1 file changed, 15 insertions(+), 13 deletions(-)
diff --git a/lib/compressdev/rte_comp.h b/lib/compressdev/rte_comp.h
index d66a4b1cb9..f86e773b28 100644
--- a/lib/compressdev/rte_comp.h
+++ b/lib/compressdev/rte_comp.h
@@ -480,6 +480,19 @@ struct __rte_cache_aligned rte_comp_op {
*/
};
+
+/**
+ * Free operation structure
+ * If operation has been allocate from a rte_mempool, then the operation will
+ * be returned to the mempool.
+ *
+ * @param op
+ * Compress operation pointer allocated from rte_comp_op_alloc()
+ * If op is NULL, no operation is performed.
+ */
+void
+rte_comp_op_free(struct rte_comp_op *op);
+
/**
* Creates an operation pool
*
@@ -501,7 +514,8 @@ struct __rte_cache_aligned rte_comp_op {
struct rte_mempool *
rte_comp_op_pool_create(const char *name,
unsigned int nb_elts, unsigned int cache_size,
- uint16_t user_size, int socket_id);
+ uint16_t user_size, int socket_id)
+ __rte_malloc __rte_dealloc(rte_comp_op_free, 1);
/**
* Allocate an operation from a mempool with default parameters set
@@ -533,18 +547,6 @@ int
rte_comp_op_bulk_alloc(struct rte_mempool *mempool,
struct rte_comp_op **ops, uint16_t nb_ops);
-/**
- * Free operation structure
- * If operation has been allocate from a rte_mempool, then the operation will
- * be returned to the mempool.
- *
- * @param op
- * Compress operation pointer allocated from rte_comp_op_alloc()
- * If op is NULL, no operation is performed.
- */
-void
-rte_comp_op_free(struct rte_comp_op *op);
-
/**
* Bulk free operation structures
* If operations have been allocated from an rte_mempool, then the operations
--
2.45.2
Re: [PATCH v6 01/15] net/xsc: add xsc PMD framework
23/01/2025 10:19, WanRenyong: > On 2025/1/23 15:59, Thomas Monjalon wrote: > > 23/01/2025 06:48, WanRenyong: > >> On 2025/1/22 21:39, Thomas Monjalon wrote: > +Yunsilicon xsc > +M: WanRenyong > +M: Na Na > +M: Rong Qian > +M: Xiaoxiong Zhang > +M: Dongwei Xu > >>> Looking at how the names are codified in email addresses, > >>> I feel "Renyong Wan" is the right form for your name in English format. > >>> > >>> > >> Hello Tomas Monjalon, > >> > >> Yes, you are right, but if I use "Renyong Wan" as my English name, every > >> patch alway gets a misspelling warning from checkpatch. :( > >> It's really annoying. If it isn't unacceptable to DPDK for using > >> "WanRenyong" as my name, I don't mind of it too. > > Can't you set "Renyong Wan" in your .gitconfig? > > I think it would solve your issue. > > > > > Hello Tomas Monjalon, > > I tried it , there are still a few warnings from checkpatch like below: > > WARNING:TYPO_SPELLING: 'Wan' may be misspelled - perhaps 'Want'? > #8: > Renyong Wan (15): > ^^^ > > If DPDK accept it , I'll change my name to "Renyong Wan" in the next > version. Yes it is just a false warning, no problem. Thank you
Re: [PATCH v5 02/25] net/intel/common: add pkt reassembly fn for intel drivers
On Mon, Jan 20, 2025 at 1:00 PM Bruce Richardson wrote: > > The code for reassembling a single, multi-mbuf packet from multiple > buffers received from the NIC is duplicated across many drivers. Rather > than having multiple copies of this function, we can create an > "intel/common" directory to hold such functions and consolidate > multiple functions down to a single one for easier maintenance. > > Signed-off-by: Bruce Richardson The new drivers/net/intel/common directory is an orphan. Please add it to MAINTAINERS. An alternative: to avoid such miss in the future, it could be worth having a dedicated block in MAINTAINERS for drivers/net/intel/ with subtree set to next-net-intel and an associated subtree maintainer name, then move all Intel drivers in this block. Thomas, opinion? -- David Marchand
[PATCH] build: force gcc to initialize padding bits
With GCC 15, the compiler has changed the default behavior when
initialization is used for aggregate variables. The new default
is to follow the standard (C23) and not initialize everything by
default. This breaks assumptions in some drivers and can be
lead to other bugs.
Use the new zero initialization flag to force the old behavior
of initializing everything to zero.
Signed-off-by: Stephen Hemminger
---
config/meson.build | 3 +++
1 file changed, 3 insertions(+)
diff --git a/config/meson.build b/config/meson.build
index 6aaad6d8a4..5c8b5a15f5 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -330,6 +330,9 @@ warning_flags = [
# globally disabled warnings
'-Wno-packed-not-aligned',
'-Wno-missing-field-initializers',
+
+# guarantee that everything is zero when using initialization
+'-fzero-init-padding-bits=all',
]
if not dpdk_conf.get('RTE_ARCH_64')
--
2.45.2
Re: [PATCH 2/2] lib/hash: avoid implicit conversion to 64 bit number
On Thu, Jan 23, 2025 at 08:55:29AM +0100, Morten Brørup wrote: > > From: Andre Muezerie [mailto:andre...@linux.microsoft.com] > > Sent: Wednesday, 22 January 2025 22.37 > > > > On Wed, Jan 22, 2025 at 04:12:49PM +, Bruce Richardson wrote: > > > On Wed, Nov 27, 2024 at 05:53:57PM -0800, Andre Muezerie wrote: > > > > MSVC issues the warnings below: > > > > > > > > 1) ../lib/hash/rte_thash_gf2_poly_math.c(128): warning C4334: '<<': > > > > result of 32-bit shift implicitly converted to 64 bits > > > > (was 64-bit shift intended?) > > > > > > > > The code would be better off by using 64 bit numbers to begin with. > > > > That eliminates the need for a conversion to 64 bits later. > > > > > > > > 2) ../lib/hash/rte_thash.c(568): warning C4334: '<<': > > > > result of 32-bit shift implicitly converted to 64 bits > > > > (was 64-bit shift intended?) > > > > > > > > 1ULL should be used as the result of the bit shift gets multiplied > > > > by sizeof(uint32_t). > > > > > > > > Signed-off-by: Andre Muezerie > > > > --- > > > > > > Acked-by: Bruce Richardson > > > > > > > lib/hash/rte_thash.c | 2 +- > > > > lib/hash/rte_thash_gf2_poly_math.c | 6 +++--- > > > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/lib/hash/rte_thash.c b/lib/hash/rte_thash.c > > > > index fa78787143..f076311b57 100644 > > > > --- a/lib/hash/rte_thash.c > > > > +++ b/lib/hash/rte_thash.c > > > > @@ -565,7 +565,7 @@ rte_thash_add_helper(struct rte_thash_ctx *ctx, > > const char *name, uint32_t len, > > > > offset; > > > > > > > > ent = rte_zmalloc(NULL, sizeof(struct rte_thash_subtuple_helper) > > + > > > > - sizeof(uint32_t) * (1 << ctx->reta_sz_log), > > > > + sizeof(uint32_t) * (1ULL << ctx->reta_sz_log), > > > > RTE_CACHE_LINE_SIZE); > > > > > > Is there a reason not to use RTE_BIT64 here too? > > > > Here we are calculating the size to be passed to the second argument of > > rte_zmalloc, which is of type size_t. size_t is implementation > > dependent, typically 4 bytes on 32-bit systems and 8 bytes on 64-bit > > systems, so using 1ULL seems more appropriate. > > 1ULL makes it 8 byte on 32-bit systems too. Did you mean 1UL? > > How about reducing the formula to directly shift the sizeof() instead, i.e.: > sizeof(uint32_t) << ctx->reta_sz_log, Shifting the sizeof() directly is better indeed. Let me know how we should proceed. Do you want me to send out a new series incorporating this suggestion?
Re: [PATCH v16 00/60] remove use of VLAs for Windows
On Thu, Jan 23, 2025 at 12:43:04PM +, Bruce Richardson wrote:
> On Thu, Jan 23, 2025 at 12:58:49PM +0100, David Marchand wrote:
> > On Tue, Jan 14, 2025 at 3:32 AM Andre Muezerie
> > wrote:
> > >
> > > As per guidance technical board meeting 2024/04/17. This series
> > > removes the use of VLAs from code built for Windows for all 3
> > > toolchains. If there are additional opportunities to convert VLAs
> > > to regular C arrays please provide the details for incorporation
> > > into the series.
> > >
> > > MSVC does not support VLAs, replace VLAs with standard C arrays
> > > or alloca(). alloca() is available for all toolchain/platform
> > > combinations officially supported by DPDK.
> > >
> > > v16:
> > > * remove -Wvla from drivers/common/mlx5/meson.build and
> > > drivers/common/qat/meson.build
> > >
> > > v15:
> > > * inverted some of the logic added during v14:
> > > add -Wvla to meson build files in app and lib directories, adding
> > > -Wno-vla to the few subdirectories which are not yet VLA free
> > >
> > > v14:
> > > * add -Wvla to meson build for directories that are VLA free
> > > under app, lib, drivers. This is to ensure that new VLAs are
> > > not added to these directories in the future.
> >
> > Thanks for working on this topic.
> >
> > I see there is some back and forth on the topic of passing -Wvla.
> > It would be less fragile to put a -Wla in a upper level meson.build
> > (like config/meson.build for example), then disable explicitly in the
> > parts that are not ready.
> >
> > Something like:
> > diff --git a/config/meson.build b/config/meson.build
> > index 6aaad6d8a4..be603bd45b 100644
> > --- a/config/meson.build
> > +++ b/config/meson.build
> > @@ -348,6 +348,17 @@ foreach arg: warning_flags
> > endif
> > endforeach
> >
> > +if cc.has_argument('-Wvla')
> > +add_project_arguments('-Wvla', language: 'c')
> > +if not is_windows
> > +no_vla_cflag = '-Wno-vla'
> > +else
> > +no_vla_cflag = []
> > +endif
> > +else
> > +no_vla_cflag = []
> > +endif
> > +
>
> Minor simplification suggestion, put "no_vla_cflag = []" outside the
> conditionals at the start, as the default value. Save having multiple
> copies of that assignment, and having to do "else" legs.
>
> /Bruce
These look like great improvements. I especially like the idea of using -Wvla
from the very top.
[PATCH v3 14/15] telemetry: add allocation function attributes
Use function attributes to catch cases where telemetry data is allocated but not freed correctly. Signed-off-by: Stephen Hemminger Acked-by: Bruce Richardson --- lib/telemetry/rte_telemetry.h | 21 +++-- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/lib/telemetry/rte_telemetry.h b/lib/telemetry/rte_telemetry.h index 2ccfc73a5f..c4554e4028 100644 --- a/lib/telemetry/rte_telemetry.h +++ b/lib/telemetry/rte_telemetry.h @@ -414,16 +414,6 @@ __rte_experimental int rte_telemetry_register_cmd_arg(const char *cmd, telemetry_arg_cb fn, void *arg, const char *help); -/** - * Get a pointer to a container with memory allocated. The container is to be - * used embedded within an existing telemetry dict/array. - * - * @return - * Pointer to a container. - */ -struct rte_tel_data * -rte_tel_data_alloc(void); - /** * @internal * Free a container that has memory allocated. @@ -435,6 +425,17 @@ rte_tel_data_alloc(void); void rte_tel_data_free(struct rte_tel_data *data); +/** + * Get a pointer to a container with memory allocated. The container is to be + * used embedded within an existing telemetry dict/array. + * + * @return + * Pointer to a container. + */ +struct rte_tel_data * +rte_tel_data_alloc(void) + __rte_malloc __rte_dealloc(rte_tel_data_free, 1); + #ifdef __cplusplus } #endif -- 2.45.2
Re: [PATCH v5 01/25] net: move intel drivers to intel subdirectory
23/01/2025 15:35, Bruce Richardson: > On Thu, Jan 23, 2025 at 03:16:40PM +0100, David Marchand wrote: > > On the check-git-log.sh update, we will have many warnings with current > > update. > > > > Wrong headline prefix: > > net/intel/common: add pkt reassembly fn for intel drivers > > net/intel/common: provide common Tx entry structures > > net/intel/common: add Tx mbuf ring replenish fn > > net/intel: align Tx queue struct field names > > net/intel: add prefix for driver-specific structs > > net/intel/common: merge ice and i40e Tx queue struct > > net/iavf: use common Tx queue structure > > net/ixgbe: use common Tx queue structure > > net/intel/common: pack Tx queue structure > > net/intel/common: add post-Tx buffer free function > > net/intel/common: add Tx buffer free fn for AVX-512 > > net/iavf: use common Tx free fn for AVX-512 > > net/ice: move Tx queue mbuf cleanup fn to common > > net/iavf: use common Tx queue mbuf cleanup fn > > net/ice: use vector SW ring for all vector paths > > net/intel/common: remove unneeded code > > net/intel/common: create common mbuf initializer fn > > net/intel/common: extract common Rx vector criteria > > > > Invalid patch(es) found - checked 25 patches > > > > I tried to tweak this a bit, with the following heuristic: > > * if touching only net/intel/common, accept net/intel/common: > > * if touching multiple drivers under net/intel, then accept net/intel: > > as prefix, > > * if touching some net/intel/$drv (and optionnally net/intel/common), > > accept net/$drv, > > > > diff --git a/devtools/check-git-log.sh b/devtools/check-git-log.sh > > index b2da013f6c..41c290f0ca 100755 > > --- a/devtools/check-git-log.sh > > +++ b/devtools/check-git-log.sh > > @@ -79,11 +79,18 @@ bad=$(for commit in $commits ; do > > [ -z "$(echo "$files" | grep -v '^\(drivers\|doc\|config\)/')" ] || > > continue > > drv=$(echo "$files" | grep '^drivers/' | cut -d "/" -f 2,3 | sort > > -u) > > - # for drivers/net/intel/* use 2nd and 4th fields not 2nd and 3rd > > if [ "$drv" = "net/intel" ] ; then > > - drv=$(echo "$files" | grep '^drivers/' | cut -d "/" -f > > 2,4 | sort -u) > > + drvgrp="net/intel" > > + drv=$(echo "$files" | grep '^drivers/' | grep -v > > '^drivers/net/intel/common' | > > + cut -d "/" -f 2,4 | sort -u) > > + if [ $(echo "$drv" | wc -l) -eq 0 ] ; then > > + drv='net/intel/common:' > > + elif [ $(echo "$drv" | wc -l) -gt 1 ] ; then > > + drv='net/intel:' > > + fi > > + else > > + drvgrp=$(echo "$drv" | cut -d "/" -f 1 | uniq) > > fi > > - drvgrp=$(echo "$drv" | cut -d "/" -f 1 | uniq) > > if [ $(echo "$drvgrp" | wc -l) -gt 1 ] ; then > > echo "$headline" | grep -v '^drivers:' > > elif [ $(echo "$drv" | wc -l) -gt 1 ] ; then > > > > Which then complains on patches in this series that touch many drivers > > (but have net/intel/common: as prefix where I would suggest net/intel: > > instead). > > > > I tend to disagree with this suggestion. I think that a prefix can be valid > so long as the prefix matches at least one component in the patch. For > example, > for the first patch in the set, I think net/intel/common is a better prefix > than just "net/intel". I don't massively object to your suggestion, I just > prefer patches identify the most relevant component, if possible, rather > than generalities. Saying net/intel means the same thing as net/intel/common to me. Why do we care whether the change is in common or multiple drivers? At the end it impacts multiple Intel drivers. The goal of the prefix is to quickly catch the scope of the change impact. One more argument: net/intel is shorter :)
Re: [PATCH v5 02/25] net/intel/common: add pkt reassembly fn for intel drivers
23/01/2025 15:17, David Marchand: > On Mon, Jan 20, 2025 at 1:00 PM Bruce Richardson > wrote: > > > > The code for reassembling a single, multi-mbuf packet from multiple > > buffers received from the NIC is duplicated across many drivers. Rather > > than having multiple copies of this function, we can create an > > "intel/common" directory to hold such functions and consolidate > > multiple functions down to a single one for easier maintenance. > > > > Signed-off-by: Bruce Richardson > > The new drivers/net/intel/common directory is an orphan. > Please add it to MAINTAINERS. > > An alternative: to avoid such miss in the future, it could be worth > having a dedicated block in MAINTAINERS for drivers/net/intel/ with > subtree set to next-net-intel and an associated subtree maintainer > name, then move all Intel drivers in this block. > > Thomas, opinion? I'm not sure about creating a new level. It needs to be the last one of the parent block to make our script working. Can we start with just a new entry for the common directory?
Re: [PATCH v6 01/15] net/xsc: add xsc PMD framework
On Thu, 23 Jan 2025 13:48:25 +0800 "WanRenyong" wrote: > On 2025/1/22 21:39, Thomas Monjalon wrote: > >> +Yunsilicon xsc > >> +M: WanRenyong > >> +M: Na Na > >> +M: Rong Qian > >> +M: Xiaoxiong Zhang > >> +M: Dongwei Xu > > Looking at how the names are codified in email addresses, > > I feel "Renyong Wan" is the right form for your name in English format. > > > > > Hello Tomas Monjalon, > > Yes, you are right, but if I use "Renyong Wan" as my English name, every > patch alway gets a misspelling warning from checkpatch. :( > It's really annoying. If it isn't unacceptable to DPDK for using > "WanRenyong" as my name, I don't mind of it too. > > Thank your for review. > > -- > Best regards, > WanRenyong No worries, we don't blindly trust checkpatch. What matters is that Signed-off-by must match your legal name because it has Signed-off-by has some legal significance to lawyers.
RE: [PATCH] build: force gcc to initialize padding bits
> From: Stephen Hemminger [mailto:step...@networkplumber.org]
> Sent: Thursday, 23 January 2025 18.21
>
> With GCC 15, the compiler has changed the default behavior when
> initialization is used for aggregate variables. The new default
> is to follow the standard (C23) and not initialize everything by
> default. This breaks assumptions in some drivers and can be
> lead to other bugs.
>
> Use the new zero initialization flag to force the old behavior
> of initializing everything to zero.
>
> Signed-off-by: Stephen Hemminger
> ---
> config/meson.build | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/config/meson.build b/config/meson.build
> index 6aaad6d8a4..5c8b5a15f5 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -330,6 +330,9 @@ warning_flags = [
Is warning_flags the right location for this?
Alternatively, should warning_flags be renamed?
> # globally disabled warnings
> '-Wno-packed-not-aligned',
> '-Wno-missing-field-initializers',
> +
> +# guarantee that everything is zero when using initialization
Maybe add ", like in the C23 standard" to the comment.
> +'-fzero-init-padding-bits=all',
> ]
>
> if not dpdk_conf.get('RTE_ARCH_64')
> --
> 2.45.2
I have read up on -fzero-init-padding-bits, and this is the correct solution.
With or without suggested changes:
Acked-by: Morten Brørup
Re: [PATCH v3] test: improve resiliency of malloc autotest
The new impl don't support re-test, how about add a wrap:
1. rename test_multi_alloc_statistics with do_test_multi_alloc_statistics, and
make it take socket as parameter
2. create a new function test_multi_alloc_statistics {
// prepare a new malloc heap
ret = do_test_multi_alloc_statistics(socket);
// free the heap
return ret;
}
On 2025/1/17 22:40, Bruce Richardson wrote:
> The test case "test_multi_alloc_statistics" was brittle in that it did
> some allocations and frees and then checked statistics without
> considering the initial state of the malloc heaps. This meant that,
> depending on what allocations/frees were done beforehand, the test can
> sometimes fail.
>
> We can improve resiliency by running the test using a new malloc heap,
> which means it is unaffected by any previous allocations.
>
> Bugzilla ID: 1579
> Fixes: a40a1f8231b4 ("app: various tests update")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Bruce Richardson
> ---
> v3:
> * switched allocation from mmap to malloc allowing it work on windows
> * use explicit alignment of the malloc return value to ensure memory
> added to heap is page-aligned.
>
> v2:
> * removed unnecessary extra include
> * only added new code for non-windows, since using mmap for allocation.
> ---
> app/test/test_malloc.c | 35 +++
> 1 file changed, 35 insertions(+)
>
> diff --git a/app/test/test_malloc.c b/app/test/test_malloc.c
> index 02a7d8ef20..9e73c0da09 100644
> --- a/app/test/test_malloc.c
> +++ b/app/test/test_malloc.c
> @@ -25,6 +25,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> #define N 1
> @@ -272,6 +273,34 @@ test_multi_alloc_statistics(void)
> size_t size = 2048;
> int align = 1024;
> int overhead = 0;
> + const size_t pgsz = rte_mem_page_size();
> + const size_t heap_size = (1 << 21);
> +
> + if (pgsz < heap_size) {
> + printf("Page size is smaller than heap size\n");
> + return TEST_SKIPPED;
> + }
> +
> + if (rte_malloc_heap_create(__func__) != 0) {
> + printf("Failed to create test malloc heap\n");
> + return -1;
> + }
> + /* Allocate some memory using malloc and add it to our test heap. */
> + void *unaligned_memory = malloc(heap_size + pgsz);
> + if (unaligned_memory == NULL) {
> + printf("Failed to allocate memory\n");
> + return -1;
> + }
> + void *memory = RTE_PTR_ALIGN(unaligned_memory, pgsz);
> + if (rte_malloc_heap_memory_add(__func__, memory, heap_size, NULL, 1,
> heap_size) != 0) {
> + printf("Failed to add memory to heap\n");
> + return -1;
> + }
> + socket = rte_malloc_heap_get_socket(__func__);
> + if (socket < 0) {
> + printf("Failed to get socket for test malloc heap.\n");
> + return -1;
> + }
>
> /* Dynamically calculate the overhead by allocating one cacheline and
>* then comparing what was allocated from the heap.
> @@ -371,6 +400,12 @@ test_multi_alloc_statistics(void)
> printf("Malloc statistics are incorrect - freed alloc\n");
> return -1;
> }
> +
> + /* cleanup */
> + rte_malloc_heap_memory_remove(__func__, memory, heap_size);
> + rte_malloc_heap_destroy(__func__);
> + free(unaligned_memory);
> +
> return 0;
> }
>
RE: [PATCH v0 1/1] common/cnxk: fix DPI mailbox structure
> -Original Message-
> From: Vamsi Krishna
> Sent: Wednesday, January 15, 2025 6:07 PM
> To: dev@dpdk.org
> Cc: Jerin Jacob ; Vamsi Krishna Attunuru
>
> Subject: [PATCH v0 1/1] common/cnxk: fix DPI mailbox structure
>
> From: Vamsi Attunuru
>
> In the existing DPI mailbox structure, one of the fields spans a 64-bit
> boundary,
> making it appear unusual and complicatng extraction using bit operations.
>
> Patch enlarges csize fields to ensure that mailbox fields are correctly
> positioned.
>
> Fixes: b6e395692b6d ("common/cnxk: add DPI DMA support")
>
> Signed-off-by: Vamsi Attunuru
Applied to dpdk-next-net-mrvl/for-main. Thanks
> ---
> drivers/common/cnxk/roc_dpi_priv.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/common/cnxk/roc_dpi_priv.h
> b/drivers/common/cnxk/roc_dpi_priv.h
> index 1f975915f7..05b6751ca6 100644
> --- a/drivers/common/cnxk/roc_dpi_priv.h
> +++ b/drivers/common/cnxk/roc_dpi_priv.h
> @@ -27,7 +27,7 @@ typedef union dpi_mbox_msg_t {
> /* Command code */
> uint64_t cmd : 4;
> /* Command buffer size in 8-byte words */
> - uint64_t csize : 14;
> + uint64_t csize : 16;
> /* aura of the command buffer */
> uint64_t aura : 20;
> /* SSO PF function */
> --
> 2.34.1
Re: [PATCH v3 0/2] ethdev: clarify something about new event
Series-acked-by: Chengwen Feng On 2025/1/17 17:12, Huisong Li wrote: > I've had some issues when I add the verification of the port id in the > event callback, which are discussed in another patch series[1]. So this > series clarify something about RTE_ETH_EVENT_NEW based on the previous > discussion. > > [1] > https://patches.dpdk.org/project/dpdk/cover/20250113025521.32703-1-lihuis...@huawei.com/ > > --- > -v3: >- add "the" before application as Thomas suggested. >- add Acked-by Thomas Monjalon > -v2: fix some descriptions as Thomas suggested. > > Huisong Li (2): > ethdev: clarify something about the new event > ethdev: fix some functions are available in the new event > > lib/ethdev/rte_ethdev.c | 14 +++--- > lib/ethdev/rte_ethdev.h | 7 ++- > 2 files changed, 17 insertions(+), 4 deletions(-) >
[PATCH v2 2/3] baseband/acc: add trace point
Improvement of logging to notably use trace point
for driver specific error logging and tracepoint.
Signed-off-by: Nicolas Chautru
---
drivers/baseband/acc/acc_common.c | 8
drivers/baseband/acc/acc_common.h | 55 ++
drivers/baseband/acc/rte_vrb_pmd.c | 63 +-
drivers/baseband/acc/vrb_trace.h | 35 +
4 files changed, 133 insertions(+), 28 deletions(-)
create mode 100644 drivers/baseband/acc/vrb_trace.h
diff --git a/drivers/baseband/acc/acc_common.c
b/drivers/baseband/acc/acc_common.c
index f8d2b19570..25ddef8a6c 100644
--- a/drivers/baseband/acc/acc_common.c
+++ b/drivers/baseband/acc/acc_common.c
@@ -3,5 +3,13 @@
*/
#include
+#include
+#include "vrb_trace.h"
RTE_LOG_REGISTER_SUFFIX(acc_common_logtype, common, INFO);
+
+RTE_TRACE_POINT_REGISTER(rte_bbdev_vrb_trace_error,
+ bbdev.vrb.device.error);
+
+RTE_TRACE_POINT_REGISTER(rte_bbdev_vrb_trace_queue_error,
+ bbdev.vrb.queue.error);
diff --git a/drivers/baseband/acc/acc_common.h
b/drivers/baseband/acc/acc_common.h
index a49b154a0c..488050 100644
--- a/drivers/baseband/acc/acc_common.h
+++ b/drivers/baseband/acc/acc_common.h
@@ -7,6 +7,7 @@
#include
#include "rte_acc_common_cfg.h"
+#include "vrb_trace.h"
/* Values used in filling in descriptors */
#define ACC_DMA_DESC_TYPE 2
@@ -653,6 +654,56 @@ struct __rte_cache_aligned acc_queue {
struct acc_device *d;
};
+/* These strings for rte_trace must be limited to
RTE_TRACE_EMIT_STRING_LEN_MAX. */
+static const char * const acc_error_string[] = {
+ "Warn: HARQ offset unexpected.",
+ "HARQ in/output is not defined.",
+ "Mismatch related to Mbuf data.",
+ "Soft output is not defined.",
+ "Device incompatible cap.",
+ "HARQ cannot be appended.",
+ "Undefined error message.",
+};
+
+/* Matching indexes for acc_error_string. */
+enum acc_error_enum {
+ ACC_ERR_HARQ_UNEXPECTED,
+ ACC_ERR_REJ_HARQ,
+ ACC_ERR_REJ_MBUF,
+ ACC_ERR_REJ_SOFT,
+ ACC_ERR_REJ_CAP,
+ ACC_ERR_REJ_HARQ_OUT,
+ ACC_ERR_MAX
+};
+
+/**
+ * @brief Report error both through RTE logging and into trace point.
+ *
+ * This function is used to log an error for a specific ACC queue and
operation.
+ *
+ * @param q Pointer to the ACC queue.
+ * @param op Pointer to the operation.
+ * @param fmt Format string for the error message.
+ * @param ... Additional arguments for the format string.
+ */
+__rte_format_printf(4, 5)
+static inline void
+acc_error_log(struct acc_queue *q, void *op, uint8_t acc_error_idx, const char
*fmt, ...)
+{
+ va_list args;
+ RTE_SET_USED(op);
+ va_start(args, fmt);
+ rte_vlog(RTE_LOG_ERR, acc_common_logtype, fmt, args);
+
+ if (acc_error_idx > ACC_ERR_MAX)
+ acc_error_idx = ACC_ERR_MAX;
+
+ rte_bbdev_vrb_trace_error(0, rte_bbdev_op_type_str(q->op_type),
+ acc_error_string[acc_error_idx]);
+
+ va_end(args);
+}
+
/* Write to MMIO register address */
static inline void
mmio_write(void *addr, uint32_t value)
@@ -1511,6 +1562,10 @@ acc_enqueue_status(struct rte_bbdev_queue_data *q_data,
{
q_data->enqueue_status = status;
q_data->queue_stats.enqueue_status_count[status]++;
+ struct acc_queue *q = q_data->queue_private;
+
+ rte_bbdev_vrb_trace_queue_error(q->qgrp_id, q->aq_id,
+ rte_bbdev_enqueue_status_str(status));
rte_acc_log(WARNING, "Enqueue Status: %s %#"PRIx64"",
rte_bbdev_enqueue_status_str(status),
diff --git a/drivers/baseband/acc/rte_vrb_pmd.c
b/drivers/baseband/acc/rte_vrb_pmd.c
index eb9892ff31..27620ccc10 100644
--- a/drivers/baseband/acc/rte_vrb_pmd.c
+++ b/drivers/baseband/acc/rte_vrb_pmd.c
@@ -1816,7 +1816,7 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op *op,
uint32_t *in_offset, uint32_t *h_out_offset,
uint32_t *s_out_offset, uint32_t *h_out_length,
uint32_t *s_out_length, uint32_t *mbuf_total_left,
- uint32_t *seg_total_left, uint8_t r)
+ uint32_t *seg_total_left, uint8_t r, struct acc_queue *q)
{
int next_triplet = 1; /* FCW already done. */
uint16_t k;
@@ -1860,8 +1860,8 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op *op,
kw = RTE_ALIGN_CEIL(k + 4, 32) * 3;
if (unlikely((*mbuf_total_left == 0) || (*mbuf_total_left < kw))) {
- rte_bbdev_log(ERR,
- "Mismatch between mbuf length and included CB
sizes: mbuf len %u, cb len %u",
+ acc_error_log(q, (void *)op, ACC_ERR_REJ_MBUF,
+ "Mismatch between mbuf length and included CB
sizes: mbuf len %u, cb len %u\n",
*mbuf_total_left, kw);
return -1;
}
@@ -1871,8 +1871,8 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op *op,
[PATCH v2 1/3] bbdev: add trace point
Adds trace points for rte_bbdev.
Signed-off-by: Nicolas Chautru
---
lib/bbdev/bbdev_trace.h| 69 ++
lib/bbdev/bbdev_trace_points.c | 27 +
lib/bbdev/meson.build | 6 ++-
lib/bbdev/rte_bbdev.c | 17 +
lib/bbdev/rte_bbdev.h | 50 +---
lib/bbdev/rte_bbdev_trace_fp.h | 41
lib/bbdev/version.map | 4 ++
7 files changed, 206 insertions(+), 8 deletions(-)
create mode 100644 lib/bbdev/bbdev_trace.h
create mode 100644 lib/bbdev/bbdev_trace_points.c
create mode 100644 lib/bbdev/rte_bbdev_trace_fp.h
diff --git a/lib/bbdev/bbdev_trace.h b/lib/bbdev/bbdev_trace.h
new file mode 100644
index 00..7256d6b703
--- /dev/null
+++ b/lib/bbdev/bbdev_trace.h
@@ -0,0 +1,69 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(C) 2025 Intel Corporation
+ */
+
+#ifndef BBDEV_TRACE_H
+#define BBDEV_TRACE_H
+
+/**
+ * @file
+ *
+ * API for bbdev trace support
+ */
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include
+
+#include "rte_bbdev.h"
+
+RTE_TRACE_POINT(
+ rte_bbdev_trace_setup_queues,
+ RTE_TRACE_POINT_ARGS(uint8_t dev_id, uint16_t num_queues, int
socket_id),
+ rte_trace_point_emit_u8(dev_id);
+ rte_trace_point_emit_u16(num_queues);
+ rte_trace_point_emit_int(socket_id);
+)
+RTE_TRACE_POINT(
+ rte_bbdev_trace_queue_configure,
+ RTE_TRACE_POINT_ARGS(uint8_t dev_id, uint16_t queue_id, const char
*op_str, uint8_t pri),
+ rte_trace_point_emit_u8(dev_id);
+ rte_trace_point_emit_u16(queue_id);
+ rte_trace_point_emit_string(op_str);
+ rte_trace_point_emit_u8(pri);
+)
+RTE_TRACE_POINT(
+ rte_bbdev_trace_start,
+ RTE_TRACE_POINT_ARGS(uint8_t dev_id),
+ rte_trace_point_emit_u8(dev_id);
+)
+RTE_TRACE_POINT(
+ rte_bbdev_trace_stop,
+ RTE_TRACE_POINT_ARGS(uint8_t dev_id),
+ rte_trace_point_emit_u8(dev_id);
+)
+RTE_TRACE_POINT(
+ rte_bbdev_trace_close,
+ RTE_TRACE_POINT_ARGS(uint8_t dev_id),
+ rte_trace_point_emit_u8(dev_id);
+)
+RTE_TRACE_POINT(
+ rte_bbdev_trace_queue_start,
+ RTE_TRACE_POINT_ARGS(uint8_t dev_id, uint16_t queue_id),
+ rte_trace_point_emit_u8(dev_id);
+ rte_trace_point_emit_u16(queue_id);
+)
+RTE_TRACE_POINT(
+ rte_bbdev_trace_queue_stop,
+ RTE_TRACE_POINT_ARGS(uint8_t dev_id, uint16_t queue_id),
+ rte_trace_point_emit_u8(dev_id);
+ rte_trace_point_emit_u16(queue_id);
+)
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* BBDEV_TRACE_H */
diff --git a/lib/bbdev/bbdev_trace_points.c b/lib/bbdev/bbdev_trace_points.c
new file mode 100644
index 00..6f90e2aa65
--- /dev/null
+++ b/lib/bbdev/bbdev_trace_points.c
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(C) 2025 Intel Corporation
+ */
+
+#include
+
+#include "bbdev_trace.h"
+
+RTE_TRACE_POINT_REGISTER(rte_bbdev_trace_setup_queues,
+ lib.bbdev.queue.setup)
+RTE_TRACE_POINT_REGISTER(rte_bbdev_trace_queue_configure,
+ lib.bbdev.queue.configure)
+RTE_TRACE_POINT_REGISTER(rte_bbdev_trace_start,
+ lib.bbdev.start)
+RTE_TRACE_POINT_REGISTER(rte_bbdev_trace_stop,
+ lib.bbdev.stop)
+RTE_TRACE_POINT_REGISTER(rte_bbdev_trace_close,
+ lib.bbdev.close)
+RTE_TRACE_POINT_REGISTER(rte_bbdev_trace_queue_start,
+ lib.bbdev.queue.start)
+RTE_TRACE_POINT_REGISTER(rte_bbdev_trace_queue_stop,
+ lib.bbdev.queue.stop)
+
+RTE_TRACE_POINT_REGISTER(rte_bbdev_trace_enqueue,
+ lib.bbdev.enq)
+RTE_TRACE_POINT_REGISTER(rte_bbdev_trace_dequeue,
+ lib.bbdev.deq)
diff --git a/lib/bbdev/meson.build b/lib/bbdev/meson.build
index 07685e7578..d8b95a400e 100644
--- a/lib/bbdev/meson.build
+++ b/lib/bbdev/meson.build
@@ -7,8 +7,10 @@ if is_windows
subdir_done()
endif
-sources = files('rte_bbdev.c')
+sources = files('rte_bbdev.c',
+'bbdev_trace_points.c')
headers = files('rte_bbdev.h',
'rte_bbdev_pmd.h',
-'rte_bbdev_op.h')
+'rte_bbdev_op.h',
+'rte_bbdev_trace_fp.h')
deps += ['mbuf']
diff --git a/lib/bbdev/rte_bbdev.c b/lib/bbdev/rte_bbdev.c
index bd32da79b0..d7901cd29d 100644
--- a/lib/bbdev/rte_bbdev.c
+++ b/lib/bbdev/rte_bbdev.c
@@ -20,6 +20,7 @@
#include "rte_bbdev_op.h"
#include "rte_bbdev.h"
#include "rte_bbdev_pmd.h"
+#include "bbdev_trace.h"
#define DEV_NAME "BBDEV"
@@ -321,6 +322,8 @@ rte_bbdev_setup_queues(uint16_t dev_id, uint16_t
num_queues, int socket_id)
VALID_DEV_OPS_OR_RET_ERR(dev, dev_id);
+ rte_bbdev_trace_setup_queues(dev_id, num_queues, socket_id);
+
if (dev->data->started) {
rte_bbdev_log(ERR,
"Device %u cannot be configured when started",
@@ -436,6 +439,10 @@ int
rte_bbdev_queue_configure(uint16_t dev_id, uint16_t queue_id,
const struct rte_bbdev_queue_conf *conf)
{
+
+ rte_bbdev_trace_queue_configure(dev_id, queue_id,
rte_bbdev_op
[PATCH v2 0/3] bbdev: trace point and logging
v2: fix build error. Hi, Based on previous discussion improving logging for bbdev and PMD using notably trace points and internal logging extension. The trace point impacting real time are not built by default. This is added at bbdev level and also in the PMD specific implementation. Thanks Nic Nicolas Chautru (3): bbdev: add trace point baseband/acc: add trace point baseband/acc: add internal logging drivers/baseband/acc/acc_common.c | 8 +++ drivers/baseband/acc/acc_common.h | 71 ++ drivers/baseband/acc/rte_vrb_pmd.c | 81 +++--- drivers/baseband/acc/vrb_trace.h | 35 + lib/bbdev/bbdev_trace.h| 69 + lib/bbdev/bbdev_trace_points.c | 27 ++ lib/bbdev/meson.build | 6 ++- lib/bbdev/rte_bbdev.c | 17 +++ lib/bbdev/rte_bbdev.h | 50 +++--- lib/bbdev/rte_bbdev_trace_fp.h | 41 +++ lib/bbdev/version.map | 4 ++ 11 files changed, 372 insertions(+), 37 deletions(-) create mode 100644 drivers/baseband/acc/vrb_trace.h create mode 100644 lib/bbdev/bbdev_trace.h create mode 100644 lib/bbdev/bbdev_trace_points.c create mode 100644 lib/bbdev/rte_bbdev_trace_fp.h -- 2.34.1
[PATCH v2 3/3] baseband/acc: add internal logging
Adds internal buffer for more flexible logging.
Signed-off-by: Nicolas Chautru
---
drivers/baseband/acc/acc_common.h | 22 +++---
drivers/baseband/acc/rte_vrb_pmd.c | 18 +-
2 files changed, 36 insertions(+), 4 deletions(-)
diff --git a/drivers/baseband/acc/acc_common.h
b/drivers/baseband/acc/acc_common.h
index 488050..06255ff5f1 100644
--- a/drivers/baseband/acc/acc_common.h
+++ b/drivers/baseband/acc/acc_common.h
@@ -152,6 +152,8 @@
#define ACC_MAX_FFT_WIN 16
#define ACC_MAX_RING_BUFFER 64
#define VRB2_MAX_Q_PER_OP 256
+#define ACC_MAX_LOGLEN256
+#define ACC_MAX_BUFFERLEN 256
extern int acc_common_logtype;
#define RTE_LOGTYPE_ACC_COMMON acc_common_logtype
@@ -652,6 +654,9 @@ struct __rte_cache_aligned acc_queue {
rte_iova_t fcw_ring_addr_iova;
int8_t *derm_buffer; /* interim buffer for de-rm in SDK */
struct acc_device *d;
+ char error_bufs[ACC_MAX_BUFFERLEN][ACC_MAX_LOGLEN]; /**< Buffer for
error log. */
+ uint16_t error_head; /**< Head - Buffer for error log. */
+ uint16_t error_wrap; /**< Wrap Counter - Buffer for error log. */
};
/* These strings for rte_trace must be limited to
RTE_TRACE_EMIT_STRING_LEN_MAX. */
@@ -690,11 +695,21 @@ __rte_format_printf(4, 5)
static inline void
acc_error_log(struct acc_queue *q, void *op, uint8_t acc_error_idx, const char
*fmt, ...)
{
- va_list args;
- RTE_SET_USED(op);
+ va_list args, args2;
+ static char str[1024];
+
va_start(args, fmt);
+ va_copy(args2, args);
rte_vlog(RTE_LOG_ERR, acc_common_logtype, fmt, args);
-
+ vsnprintf(q->error_bufs[q->error_head], ACC_MAX_LOGLEN, fmt, args2);
+ q->error_head++;
+ snprintf(q->error_bufs[q->error_head], ACC_MAX_LOGLEN,
+ "%s", rte_bbdev_ops_param_string(op, q->op_type, str,
sizeof(str)));
+ q->error_head++;
+ if (q->error_head == ACC_MAX_LOGLEN) {
+ q->error_head = 0;
+ q->error_wrap++;
+ }
if (acc_error_idx > ACC_ERR_MAX)
acc_error_idx = ACC_ERR_MAX;
@@ -702,6 +717,7 @@ acc_error_log(struct acc_queue *q, void *op, uint8_t
acc_error_idx, const char *
acc_error_string[acc_error_idx]);
va_end(args);
+ va_end(args2);
}
/* Write to MMIO register address */
diff --git a/drivers/baseband/acc/rte_vrb_pmd.c
b/drivers/baseband/acc/rte_vrb_pmd.c
index 27620ccc10..d81c5d460c 100644
--- a/drivers/baseband/acc/rte_vrb_pmd.c
+++ b/drivers/baseband/acc/rte_vrb_pmd.c
@@ -1135,6 +1135,10 @@ vrb_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
q->mmio_reg_enqueue = RTE_PTR_ADD(d->mmio_base,
d->queue_offset(d->pf_device, q->vf_id, q->qgrp_id,
q->aq_id));
+ /** initialize the error buffer. */
+ q->error_head = 0;
+ q->error_wrap = 0;
+
rte_bbdev_log_debug(
"Setup dev%u q%u: qgrp_id=%u, vf_id=%u, aq_id=%u,
aq_depth=%u, mmio_reg_enqueue=%p base %p",
dev->data->dev_id, queue_id, q->qgrp_id, q->vf_id,
@@ -1516,7 +1520,7 @@ vrb_queue_ops_dump(struct rte_bbdev *dev, uint16_t
queue_id, FILE *f)
{
struct acc_queue *q = dev->data->queues[queue_id].queue_private;
struct rte_bbdev_dec_op *op;
- uint16_t i, int_nb;
+ uint16_t start_err, end_err, i, int_nb;
volatile union acc_info_ring_data *ring_data;
uint16_t info_ring_head = q->d->info_ring_head;
static char str[1024];
@@ -1533,6 +1537,18 @@ vrb_queue_ops_dump(struct rte_bbdev *dev, uint16_t
queue_id, FILE *f)
q->aq_enqueued, q->aq_dequeued, q->aq_depth,
acc_ring_avail_enq(q), acc_ring_avail_deq(q));
+ /** Print information captured in the error buffer. */
+ if (q->error_wrap == 0) {
+ start_err = 0;
+ end_err = q->error_head;
+ } else {
+ start_err = q->error_head;
+ end_err = q->error_head + ACC_MAX_BUFFERLEN;
+ }
+ fprintf(f, "Error Buffer - Head %d Wrap %d\n", q->error_head,
q->error_wrap);
+ for (i = start_err; i < end_err; ++i)
+ fprintf(f, " %d\t%s", i, q->error_bufs[i % ACC_MAX_BUFFERLEN]);
+
/** Print information captured in the info ring. */
if (q->d->info_ring != NULL) {
fprintf(f, "Info Ring Buffer - Head %d\n",
q->d->info_ring_head);
--
2.34.1
RE: [RFC PATCH] eventdev: adapter API to configure multiple Rx queues
> -Original Message- > From: Shijith Thotton > Sent: Wednesday, January 22, 2025 7:13 PM > To: Naga Harish K, S V ; dev@dpdk.org > Cc: Pavan Nikhilesh Bhagavatula ; Pathak, > Pravin ; Hemant Agrawal > ; Sachin Saxena ; > Mattias R_nnblom ; Jerin Jacob > ; Liang Ma ; Mccarthy, Peter > ; Van Haaren, Harry > ; Carrillo, Erik G ; > Gujjar, Abhinandan S ; Amit Prakash Shukla > ; Burakov, Anatoly > > Subject: RE: [RFC PATCH] eventdev: adapter API to configure multiple Rx > queues > > >> >> >> >>> This RFC introduces a new API, > >> >> >> >>> rte_event_eth_rx_adapter_queues_add(), > >> >> >> >>> designed to enhance the flexibility of configuring multiple > >> >> >> >>> Rx queues in eventdev Rx adapter. > >> >> >> >>> > >> >> >> >>> The existing rte_event_eth_rx_adapter_queue_add() API > >> >> >> >>> supports adding multiple queues by specifying rx_queue_id = > >> >> >> >>> -1, but it lacks the ability to > >> >> >> >apply > >> >> >> >>> specific configurations to each of the added queues. > >> >> >> >>> > >> >> >> >> > >> >> >> >>The application can still use the existing > >> >> >> >>rte_event_eth_rx_adapter_queue_add() API in a loop with > >> >> >> >>different configurations for different queues. > >> >> >> >> > >> >> >> >>The proposed API is not enabling new features that cannot be > >> >> >> >>achieved with the existing API. > >> >> >> >>Adding new APIs without much usefulness causes unnecessary > >> >> >> >>complexity/confusion for users. > >> >> >> >> > >> > >> The eth_rx_adapter_queue_add eventdev PMD operation can be updated > to > >> support burst mode. Internally, both the new and existing APIs can > >> utilize this updated operation. This enables applications to use > >> either API and achieve > >the > >> same results while adding a single queue. For adding multiple RX > >> queues to > >the > >> adapter, the new API can be used as it is not supported by the old API. > >> > > > >Not all platforms implement the eventdev PMD operation for > >eth_rx_adapter_queue_add, so this does not apply to all platforms. > > > > Yes, but there are hardware PMDs that implement eth_rx_adapter_queue_add > op, and I am looking for a solution that works for both cases. > > The idea is to use the new eventdev PMD operation > (eth_rx_adapter_queues_add) within the > rte_event_eth_rx_adapter_queue_add() API. The parameters of this API can > be easily mapped to and supported by the new PMD operation. > This requires a change to the rte_event_eth_rx_adapter_queue_add() stable API parameters. This is an ABI breakage and may not be possible now. It requires changes to many current applications that are using the rte_event_eth_rx_adapter_queue_add() stable API. > typedef int (*eventdev_eth_rx_adapter_queues_add_t)( > const struct rte_eventdev *dev, > const struct rte_eth_dev *eth_dev, > int32_t rx_queue_id[], > const struct rte_event_eth_rx_adapter_queue_conf queue_conf[], > uint16_t nb_rx_queues); > > With this, the old PMD op (eth_rx_adapter_queue_add) can be removed. > > >> >> >> > > >> >> >> >The new API was introduced because the existing API does not > >> >> >> >support adding multiple queues with specific configurations. > >> >> >> >It serves as a burst variant of the existing API, like many > >> >> >> >other APIs in > >> DPDK. > >> >> >> > > >> >> > > >> >> >The other burst APIs may be there for dataplane functionalities, > >> >> >but may not be for the control plane functionalities. > >> >> > > >> >> > >> >> rte_acl_add_rules() is an example of burst API in control path. > >> >> > >> > > >> >I mean, In general, burst APIs are for data-plane functions. > >> >This may be one of the rare cases where a burst API is in the control > >> >path. > >> > > >> >> >> >For better clarity, the API can be renamed to > >> >> >> >rte_event_eth_rx_adapter_queue_add_burst() if needed. > >> >> >> > > >> >> >> >In hardware, adding each queue individually incurs significant > >> >> >> >overheads, such as mailbox operations. A burst API helps to > >> >> >> >amortize this overhead. Since real- world applications often > >> >> >> >call the API with specific queue_ids, the burst API can > >> >> >> >provide considerable > >> benefits. > >> >> >> >Testing shows a 75% reduction in time when adding multiple > >> >> >> >queues to the RX adapter using the burst API on our platform. > >> >> >> > > >> >> > > >> >> > As batching helps for a particular hardware device, this may not > >> >> >be applicable for all platforms/cases. > >> >> > Since queue_add is a control plane operation, latency may not be > >> >> >a concern. > >> >> > >> >> In certain use cases, these APIs can be considered semi-fast path. > >> >> For > >> >instance, > >> >> in an application that hotplugs a port on demand, configuring all > >> >> available queues simultaneously can significantly reduce latency. > >> >> > >> > > >> >As said earlier, this latency reduction (when trying to add multiple > >> >RX queues to the Event Ethernet Rx adapt
Re: [PATCH v3] mbuf: add fast free bulk and raw alloc bulk functions
LGTM Acked-by: Chengwen Feng On 2025/1/21 21:40, Morten Brørup wrote: > When putting an mbuf back into its mempool, there are certain requirements > to the mbuf. Specifically, some of its fields must be initialized. > > These requirements are in fact invariants about free mbufs, held in > mempools, and thus also apply when allocating an mbuf from a mempool. > With this in mind, the additional assertions in rte_mbuf_raw_free() were > moved to __rte_mbuf_raw_sanity_check(). > Furthermore, the assertion regarding pinned external buffer was enhanced; > it now also asserts that the referenced pinned external buffer has > refcnt == 1. > > The description of RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE was updated to > include the remaining requirements, which were missing here. > > And finally: > A new rte_mbuf_fast_free_bulk() inline function was added for the benefit > of ethdev drivers supporting fast release of mbufs. > It asserts these requirements and that the mbufs belong to the specified > mempool, and then calls rte_mempool_put_bulk(). > > For symmetry, a new rte_mbuf_raw_alloc_bulk() inline function was also > added. > > Signed-off-by: Morten Brørup > Acked-by: Dengdui Huang
[PATCH v1] net/axgbe: support TSO
Added TSO(Transmit Segmentation offload) support for axgbe PMD.
Signed-off-by: Jesna K E
---
doc/guides/nics/features/axgbe.ini | 1 +
drivers/net/axgbe/axgbe_common.h | 13 ++
drivers/net/axgbe/axgbe_dev.c | 12 ++
drivers/net/axgbe/axgbe_ethdev.c | 2 +
drivers/net/axgbe/axgbe_ethdev.h | 1 +
drivers/net/axgbe/axgbe_rxtx.c | 247 ++---
6 files changed, 179 insertions(+), 97 deletions(-)
diff --git a/doc/guides/nics/features/axgbe.ini
b/doc/guides/nics/features/axgbe.ini
index 5e2d6498e5..ce4a5075f4 100644
--- a/doc/guides/nics/features/axgbe.ini
+++ b/doc/guides/nics/features/axgbe.ini
@@ -7,6 +7,7 @@
Speed capabilities = Y
Link status = Y
Scattered Rx = Y
+TSO = Y
Promiscuous mode = Y
Allmulticast mode= Y
RSS hash = Y
diff --git a/drivers/net/axgbe/axgbe_common.h b/drivers/net/axgbe/axgbe_common.h
index 0e1b2c1500..93e6c177b6 100644
--- a/drivers/net/axgbe/axgbe_common.h
+++ b/drivers/net/axgbe/axgbe_common.h
@@ -161,6 +161,10 @@
#define DMA_CH_CARBR_LO0x5c
#define DMA_CH_SR 0x60
+/* Setting MSS register entry bit positions and sizes for TSO */
+#define DMA_CH_CR_MSS_INDEX 0
+#define DMA_CH_CR_MSS_WIDTH 14
+
/* DMA channel register entry bit positions and sizes */
#define DMA_CH_CR_PBLX8_INDEX 16
#define DMA_CH_CR_PBLX8_WIDTH 1
@@ -1230,6 +1234,15 @@
#define TX_CONTEXT_DESC3_VT_INDEX 0
#define TX_CONTEXT_DESC3_VT_WIDTH 16
+/* TSO related register entry bit positions and sizes*/
+#define TX_NORMAL_DESC3_TPL_INDEX 0
+#define TX_NORMAL_DESC3_TPL_WIDTH 18
+#define TX_NORMAL_DESC3_THL_INDEX 19
+#define TX_NORMAL_DESC3_THL_WIDTH 4
+#define TX_CONTEXT_DESC3_OSTC_INDEX 27
+#define TX_CONTEXT_DESC3_OSTC_WIDTH 1
+
+
#define TX_NORMAL_DESC2_HL_B1L_INDEX 0
#define TX_NORMAL_DESC2_HL_B1L_WIDTH 14
#define TX_NORMAL_DESC2_IC_INDEX 31
diff --git a/drivers/net/axgbe/axgbe_dev.c b/drivers/net/axgbe/axgbe_dev.c
index 9173a6fea6..634d4ee4a5 100644
--- a/drivers/net/axgbe/axgbe_dev.c
+++ b/drivers/net/axgbe/axgbe_dev.c
@@ -872,6 +872,17 @@ int axgbe_write_rss_lookup_table(struct axgbe_port *pdata)
return 0;
}
+static void axgbe_config_tso_mode(struct axgbe_port *pdata)
+{
+ unsigned int i;
+ struct axgbe_tx_queue *txq;
+
+ for (i = 0; i < pdata->eth_dev->data->nb_tx_queues; i++) {
+ txq = pdata->eth_dev->data->tx_queues[i];
+ AXGMAC_DMA_IOWRITE_BITS(txq, DMA_CH_TCR, TSE, 1);
+ }
+}
+
static int axgbe_enable_rss(struct axgbe_port *pdata)
{
int ret;
@@ -1378,6 +1389,7 @@ static int axgbe_init(struct axgbe_port *pdata)
axgbe_config_rx_pbl_val(pdata);
axgbe_config_rx_buffer_size(pdata);
axgbe_config_rss(pdata);
+ axgbe_config_tso_mode(pdata);
wrapper_tx_desc_init(pdata);
ret = wrapper_rx_desc_init(pdata);
if (ret)
diff --git a/drivers/net/axgbe/axgbe_ethdev.c b/drivers/net/axgbe/axgbe_ethdev.c
index 5448a5f3d7..c42cac5b8b 100644
--- a/drivers/net/axgbe/axgbe_ethdev.c
+++ b/drivers/net/axgbe/axgbe_ethdev.c
@@ -11,6 +11,7 @@
#include "rte_time.h"
#include "eal_filesystem.h"
+#include
#include
@@ -1241,6 +1242,7 @@ axgbe_dev_info_get(struct rte_eth_dev *dev, struct
rte_eth_dev_info *dev_info)
RTE_ETH_TX_OFFLOAD_QINQ_INSERT |
RTE_ETH_TX_OFFLOAD_IPV4_CKSUM |
RTE_ETH_TX_OFFLOAD_MULTI_SEGS |
+ RTE_ETH_TX_OFFLOAD_TCP_TSO |
RTE_ETH_TX_OFFLOAD_UDP_CKSUM |
RTE_ETH_TX_OFFLOAD_TCP_CKSUM;
diff --git a/drivers/net/axgbe/axgbe_ethdev.h b/drivers/net/axgbe/axgbe_ethdev.h
index dd00ae8af5..5cd4317d7a 100644
--- a/drivers/net/axgbe/axgbe_ethdev.h
+++ b/drivers/net/axgbe/axgbe_ethdev.h
@@ -623,6 +623,7 @@ struct axgbe_port {
unsigned int tx_osp_mode;
unsigned int tx_max_fifo_size;
unsigned int multi_segs_tx;
+ unsigned int tso_tx;
/* Rx settings */
unsigned int rx_sf_mode;
diff --git a/drivers/net/axgbe/axgbe_rxtx.c b/drivers/net/axgbe/axgbe_rxtx.c
index 974ade9ab7..51a1aeb0b9 100644
--- a/drivers/net/axgbe/axgbe_rxtx.c
+++ b/drivers/net/axgbe/axgbe_rxtx.c
@@ -627,6 +627,9 @@ int axgbe_dev_tx_queue_setup(struct rte_eth_dev *dev,
uint16_t queue_idx,
RTE_ETH_TX_OFFLOAD_MULTI_SEGS))
pdata->multi_segs_tx = true;
+ if ((dev_data->dev_conf.txmode.offloads &
+ RTE_ETH_TX_OFFLOAD_TCP_TSO))
+ pdata->tso_tx = true;
return 0;
}
@@ -824,26 +827,77 @@ static int axgbe_xmit_hw(struct axgbe_tx_queue *txq,
volatile struct axgbe_tx_desc *desc;
uint16_t idx;
uint64_t mask;
+ int start_index;
Re: [PATCH v2 1/2] mempool: fix rte_errno in rte_mempool_create_empty
Acked-by: Chengwen Feng
On 2025/1/20 20:21, Ariel Otilibili wrote:
> When returning from rte_mempool_set_ops_byname(), rte_errno is not set
> for error exits.
>
> The API requires rte_errno to be set in that case.
>
> Bugzilla ID: 1559
> Fixes: c2c6b2f41305 ("mempool: fix default ops for an empty mempool")
> Link:
> https://doc.dpdk.org/api/rte__mempool_8h.html#a82e301ee33ed7a263ceb4582655dc3ea
> Signed-off-by: Ariel Otilibili
Re: [PATCH v2 3/3] baseband/acc: add internal logging
On Thu, 23 Jan 2025 14:55:19 -0800 Nicolas Chautru wrote: > Adds internal buffer for more flexible logging. > > Signed-off-by: Nicolas Chautru Inventing another device specific error log seems like a short sighted concept. Why doesn't existing DPDK logging work well enough?
[PATCH v1 0/3] bbdev: trace point and logging
Hi, Based on previous discussion improving logging for bbdev and PMD using notably trace points and internal logging extension. The trace point impacting real time are not built by default. This is added at bbdev level and also in the PMD specific implementation. Thanks Nic Nicolas Chautru (3): bbdev: add trace point baseband/acc: add trace point baseband/acc: add internal logging drivers/baseband/acc/acc_common.c | 8 +++ drivers/baseband/acc/acc_common.h | 71 ++ drivers/baseband/acc/rte_vrb_pmd.c | 81 +++--- drivers/baseband/acc/vrb_trace.h | 35 + lib/bbdev/bbdev_trace.h| 69 + lib/bbdev/bbdev_trace_points.c | 27 ++ lib/bbdev/meson.build | 6 ++- lib/bbdev/rte_bbdev.c | 16 ++ lib/bbdev/rte_bbdev.h | 51 --- lib/bbdev/rte_bbdev_trace_fp.h | 41 +++ lib/bbdev/version.map | 4 ++ 11 files changed, 372 insertions(+), 37 deletions(-) create mode 100644 drivers/baseband/acc/vrb_trace.h create mode 100644 lib/bbdev/bbdev_trace.h create mode 100644 lib/bbdev/bbdev_trace_points.c create mode 100644 lib/bbdev/rte_bbdev_trace_fp.h -- 2.34.1
[PATCH v1 3/3] baseband/acc: add internal logging
Adds internal buffer for more flexible logging.
Signed-off-by: Nicolas Chautru
---
drivers/baseband/acc/acc_common.h | 22 +++---
drivers/baseband/acc/rte_vrb_pmd.c | 18 +-
2 files changed, 36 insertions(+), 4 deletions(-)
diff --git a/drivers/baseband/acc/acc_common.h
b/drivers/baseband/acc/acc_common.h
index 488050..06255ff5f1 100644
--- a/drivers/baseband/acc/acc_common.h
+++ b/drivers/baseband/acc/acc_common.h
@@ -152,6 +152,8 @@
#define ACC_MAX_FFT_WIN 16
#define ACC_MAX_RING_BUFFER 64
#define VRB2_MAX_Q_PER_OP 256
+#define ACC_MAX_LOGLEN256
+#define ACC_MAX_BUFFERLEN 256
extern int acc_common_logtype;
#define RTE_LOGTYPE_ACC_COMMON acc_common_logtype
@@ -652,6 +654,9 @@ struct __rte_cache_aligned acc_queue {
rte_iova_t fcw_ring_addr_iova;
int8_t *derm_buffer; /* interim buffer for de-rm in SDK */
struct acc_device *d;
+ char error_bufs[ACC_MAX_BUFFERLEN][ACC_MAX_LOGLEN]; /**< Buffer for
error log. */
+ uint16_t error_head; /**< Head - Buffer for error log. */
+ uint16_t error_wrap; /**< Wrap Counter - Buffer for error log. */
};
/* These strings for rte_trace must be limited to
RTE_TRACE_EMIT_STRING_LEN_MAX. */
@@ -690,11 +695,21 @@ __rte_format_printf(4, 5)
static inline void
acc_error_log(struct acc_queue *q, void *op, uint8_t acc_error_idx, const char
*fmt, ...)
{
- va_list args;
- RTE_SET_USED(op);
+ va_list args, args2;
+ static char str[1024];
+
va_start(args, fmt);
+ va_copy(args2, args);
rte_vlog(RTE_LOG_ERR, acc_common_logtype, fmt, args);
-
+ vsnprintf(q->error_bufs[q->error_head], ACC_MAX_LOGLEN, fmt, args2);
+ q->error_head++;
+ snprintf(q->error_bufs[q->error_head], ACC_MAX_LOGLEN,
+ "%s", rte_bbdev_ops_param_string(op, q->op_type, str,
sizeof(str)));
+ q->error_head++;
+ if (q->error_head == ACC_MAX_LOGLEN) {
+ q->error_head = 0;
+ q->error_wrap++;
+ }
if (acc_error_idx > ACC_ERR_MAX)
acc_error_idx = ACC_ERR_MAX;
@@ -702,6 +717,7 @@ acc_error_log(struct acc_queue *q, void *op, uint8_t
acc_error_idx, const char *
acc_error_string[acc_error_idx]);
va_end(args);
+ va_end(args2);
}
/* Write to MMIO register address */
diff --git a/drivers/baseband/acc/rte_vrb_pmd.c
b/drivers/baseband/acc/rte_vrb_pmd.c
index 27620ccc10..d81c5d460c 100644
--- a/drivers/baseband/acc/rte_vrb_pmd.c
+++ b/drivers/baseband/acc/rte_vrb_pmd.c
@@ -1135,6 +1135,10 @@ vrb_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
q->mmio_reg_enqueue = RTE_PTR_ADD(d->mmio_base,
d->queue_offset(d->pf_device, q->vf_id, q->qgrp_id,
q->aq_id));
+ /** initialize the error buffer. */
+ q->error_head = 0;
+ q->error_wrap = 0;
+
rte_bbdev_log_debug(
"Setup dev%u q%u: qgrp_id=%u, vf_id=%u, aq_id=%u,
aq_depth=%u, mmio_reg_enqueue=%p base %p",
dev->data->dev_id, queue_id, q->qgrp_id, q->vf_id,
@@ -1516,7 +1520,7 @@ vrb_queue_ops_dump(struct rte_bbdev *dev, uint16_t
queue_id, FILE *f)
{
struct acc_queue *q = dev->data->queues[queue_id].queue_private;
struct rte_bbdev_dec_op *op;
- uint16_t i, int_nb;
+ uint16_t start_err, end_err, i, int_nb;
volatile union acc_info_ring_data *ring_data;
uint16_t info_ring_head = q->d->info_ring_head;
static char str[1024];
@@ -1533,6 +1537,18 @@ vrb_queue_ops_dump(struct rte_bbdev *dev, uint16_t
queue_id, FILE *f)
q->aq_enqueued, q->aq_dequeued, q->aq_depth,
acc_ring_avail_enq(q), acc_ring_avail_deq(q));
+ /** Print information captured in the error buffer. */
+ if (q->error_wrap == 0) {
+ start_err = 0;
+ end_err = q->error_head;
+ } else {
+ start_err = q->error_head;
+ end_err = q->error_head + ACC_MAX_BUFFERLEN;
+ }
+ fprintf(f, "Error Buffer - Head %d Wrap %d\n", q->error_head,
q->error_wrap);
+ for (i = start_err; i < end_err; ++i)
+ fprintf(f, " %d\t%s", i, q->error_bufs[i % ACC_MAX_BUFFERLEN]);
+
/** Print information captured in the info ring. */
if (q->d->info_ring != NULL) {
fprintf(f, "Info Ring Buffer - Head %d\n",
q->d->info_ring_head);
--
2.34.1
[PATCH v1 2/3] baseband/acc: add trace point
Improvement of logging to notably use trace point
for driver specific error logging and tracepoint.
Signed-off-by: Nicolas Chautru
---
drivers/baseband/acc/acc_common.c | 8
drivers/baseband/acc/acc_common.h | 55 ++
drivers/baseband/acc/rte_vrb_pmd.c | 63 +-
drivers/baseband/acc/vrb_trace.h | 35 +
4 files changed, 133 insertions(+), 28 deletions(-)
create mode 100644 drivers/baseband/acc/vrb_trace.h
diff --git a/drivers/baseband/acc/acc_common.c
b/drivers/baseband/acc/acc_common.c
index f8d2b19570..25ddef8a6c 100644
--- a/drivers/baseband/acc/acc_common.c
+++ b/drivers/baseband/acc/acc_common.c
@@ -3,5 +3,13 @@
*/
#include
+#include
+#include "vrb_trace.h"
RTE_LOG_REGISTER_SUFFIX(acc_common_logtype, common, INFO);
+
+RTE_TRACE_POINT_REGISTER(rte_bbdev_vrb_trace_error,
+ bbdev.vrb.device.error);
+
+RTE_TRACE_POINT_REGISTER(rte_bbdev_vrb_trace_queue_error,
+ bbdev.vrb.queue.error);
diff --git a/drivers/baseband/acc/acc_common.h
b/drivers/baseband/acc/acc_common.h
index a49b154a0c..488050 100644
--- a/drivers/baseband/acc/acc_common.h
+++ b/drivers/baseband/acc/acc_common.h
@@ -7,6 +7,7 @@
#include
#include "rte_acc_common_cfg.h"
+#include "vrb_trace.h"
/* Values used in filling in descriptors */
#define ACC_DMA_DESC_TYPE 2
@@ -653,6 +654,56 @@ struct __rte_cache_aligned acc_queue {
struct acc_device *d;
};
+/* These strings for rte_trace must be limited to
RTE_TRACE_EMIT_STRING_LEN_MAX. */
+static const char * const acc_error_string[] = {
+ "Warn: HARQ offset unexpected.",
+ "HARQ in/output is not defined.",
+ "Mismatch related to Mbuf data.",
+ "Soft output is not defined.",
+ "Device incompatible cap.",
+ "HARQ cannot be appended.",
+ "Undefined error message.",
+};
+
+/* Matching indexes for acc_error_string. */
+enum acc_error_enum {
+ ACC_ERR_HARQ_UNEXPECTED,
+ ACC_ERR_REJ_HARQ,
+ ACC_ERR_REJ_MBUF,
+ ACC_ERR_REJ_SOFT,
+ ACC_ERR_REJ_CAP,
+ ACC_ERR_REJ_HARQ_OUT,
+ ACC_ERR_MAX
+};
+
+/**
+ * @brief Report error both through RTE logging and into trace point.
+ *
+ * This function is used to log an error for a specific ACC queue and
operation.
+ *
+ * @param q Pointer to the ACC queue.
+ * @param op Pointer to the operation.
+ * @param fmt Format string for the error message.
+ * @param ... Additional arguments for the format string.
+ */
+__rte_format_printf(4, 5)
+static inline void
+acc_error_log(struct acc_queue *q, void *op, uint8_t acc_error_idx, const char
*fmt, ...)
+{
+ va_list args;
+ RTE_SET_USED(op);
+ va_start(args, fmt);
+ rte_vlog(RTE_LOG_ERR, acc_common_logtype, fmt, args);
+
+ if (acc_error_idx > ACC_ERR_MAX)
+ acc_error_idx = ACC_ERR_MAX;
+
+ rte_bbdev_vrb_trace_error(0, rte_bbdev_op_type_str(q->op_type),
+ acc_error_string[acc_error_idx]);
+
+ va_end(args);
+}
+
/* Write to MMIO register address */
static inline void
mmio_write(void *addr, uint32_t value)
@@ -1511,6 +1562,10 @@ acc_enqueue_status(struct rte_bbdev_queue_data *q_data,
{
q_data->enqueue_status = status;
q_data->queue_stats.enqueue_status_count[status]++;
+ struct acc_queue *q = q_data->queue_private;
+
+ rte_bbdev_vrb_trace_queue_error(q->qgrp_id, q->aq_id,
+ rte_bbdev_enqueue_status_str(status));
rte_acc_log(WARNING, "Enqueue Status: %s %#"PRIx64"",
rte_bbdev_enqueue_status_str(status),
diff --git a/drivers/baseband/acc/rte_vrb_pmd.c
b/drivers/baseband/acc/rte_vrb_pmd.c
index eb9892ff31..27620ccc10 100644
--- a/drivers/baseband/acc/rte_vrb_pmd.c
+++ b/drivers/baseband/acc/rte_vrb_pmd.c
@@ -1816,7 +1816,7 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op *op,
uint32_t *in_offset, uint32_t *h_out_offset,
uint32_t *s_out_offset, uint32_t *h_out_length,
uint32_t *s_out_length, uint32_t *mbuf_total_left,
- uint32_t *seg_total_left, uint8_t r)
+ uint32_t *seg_total_left, uint8_t r, struct acc_queue *q)
{
int next_triplet = 1; /* FCW already done. */
uint16_t k;
@@ -1860,8 +1860,8 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op *op,
kw = RTE_ALIGN_CEIL(k + 4, 32) * 3;
if (unlikely((*mbuf_total_left == 0) || (*mbuf_total_left < kw))) {
- rte_bbdev_log(ERR,
- "Mismatch between mbuf length and included CB
sizes: mbuf len %u, cb len %u",
+ acc_error_log(q, (void *)op, ACC_ERR_REJ_MBUF,
+ "Mismatch between mbuf length and included CB
sizes: mbuf len %u, cb len %u\n",
*mbuf_total_left, kw);
return -1;
}
@@ -1871,8 +1871,8 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op *op,
[PATCH v1 1/3] bbdev: add trace point
Adds trace points for rte_bbdev.
Signed-off-by: Nicolas Chautru
---
lib/bbdev/bbdev_trace.h| 69 ++
lib/bbdev/bbdev_trace_points.c | 27 +
lib/bbdev/meson.build | 6 ++-
lib/bbdev/rte_bbdev.c | 16
lib/bbdev/rte_bbdev.h | 51 ++---
lib/bbdev/rte_bbdev_trace_fp.h | 41
lib/bbdev/version.map | 4 ++
7 files changed, 206 insertions(+), 8 deletions(-)
create mode 100644 lib/bbdev/bbdev_trace.h
create mode 100644 lib/bbdev/bbdev_trace_points.c
create mode 100644 lib/bbdev/rte_bbdev_trace_fp.h
diff --git a/lib/bbdev/bbdev_trace.h b/lib/bbdev/bbdev_trace.h
new file mode 100644
index 00..7256d6b703
--- /dev/null
+++ b/lib/bbdev/bbdev_trace.h
@@ -0,0 +1,69 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(C) 2025 Intel Corporation
+ */
+
+#ifndef BBDEV_TRACE_H
+#define BBDEV_TRACE_H
+
+/**
+ * @file
+ *
+ * API for bbdev trace support
+ */
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include
+
+#include "rte_bbdev.h"
+
+RTE_TRACE_POINT(
+ rte_bbdev_trace_setup_queues,
+ RTE_TRACE_POINT_ARGS(uint8_t dev_id, uint16_t num_queues, int
socket_id),
+ rte_trace_point_emit_u8(dev_id);
+ rte_trace_point_emit_u16(num_queues);
+ rte_trace_point_emit_int(socket_id);
+)
+RTE_TRACE_POINT(
+ rte_bbdev_trace_queue_configure,
+ RTE_TRACE_POINT_ARGS(uint8_t dev_id, uint16_t queue_id, const char
*op_str, uint8_t pri),
+ rte_trace_point_emit_u8(dev_id);
+ rte_trace_point_emit_u16(queue_id);
+ rte_trace_point_emit_string(op_str);
+ rte_trace_point_emit_u8(pri);
+)
+RTE_TRACE_POINT(
+ rte_bbdev_trace_start,
+ RTE_TRACE_POINT_ARGS(uint8_t dev_id),
+ rte_trace_point_emit_u8(dev_id);
+)
+RTE_TRACE_POINT(
+ rte_bbdev_trace_stop,
+ RTE_TRACE_POINT_ARGS(uint8_t dev_id),
+ rte_trace_point_emit_u8(dev_id);
+)
+RTE_TRACE_POINT(
+ rte_bbdev_trace_close,
+ RTE_TRACE_POINT_ARGS(uint8_t dev_id),
+ rte_trace_point_emit_u8(dev_id);
+)
+RTE_TRACE_POINT(
+ rte_bbdev_trace_queue_start,
+ RTE_TRACE_POINT_ARGS(uint8_t dev_id, uint16_t queue_id),
+ rte_trace_point_emit_u8(dev_id);
+ rte_trace_point_emit_u16(queue_id);
+)
+RTE_TRACE_POINT(
+ rte_bbdev_trace_queue_stop,
+ RTE_TRACE_POINT_ARGS(uint8_t dev_id, uint16_t queue_id),
+ rte_trace_point_emit_u8(dev_id);
+ rte_trace_point_emit_u16(queue_id);
+)
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* BBDEV_TRACE_H */
diff --git a/lib/bbdev/bbdev_trace_points.c b/lib/bbdev/bbdev_trace_points.c
new file mode 100644
index 00..6f90e2aa65
--- /dev/null
+++ b/lib/bbdev/bbdev_trace_points.c
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(C) 2025 Intel Corporation
+ */
+
+#include
+
+#include "bbdev_trace.h"
+
+RTE_TRACE_POINT_REGISTER(rte_bbdev_trace_setup_queues,
+ lib.bbdev.queue.setup)
+RTE_TRACE_POINT_REGISTER(rte_bbdev_trace_queue_configure,
+ lib.bbdev.queue.configure)
+RTE_TRACE_POINT_REGISTER(rte_bbdev_trace_start,
+ lib.bbdev.start)
+RTE_TRACE_POINT_REGISTER(rte_bbdev_trace_stop,
+ lib.bbdev.stop)
+RTE_TRACE_POINT_REGISTER(rte_bbdev_trace_close,
+ lib.bbdev.close)
+RTE_TRACE_POINT_REGISTER(rte_bbdev_trace_queue_start,
+ lib.bbdev.queue.start)
+RTE_TRACE_POINT_REGISTER(rte_bbdev_trace_queue_stop,
+ lib.bbdev.queue.stop)
+
+RTE_TRACE_POINT_REGISTER(rte_bbdev_trace_enqueue,
+ lib.bbdev.enq)
+RTE_TRACE_POINT_REGISTER(rte_bbdev_trace_dequeue,
+ lib.bbdev.deq)
diff --git a/lib/bbdev/meson.build b/lib/bbdev/meson.build
index 07685e7578..d8b95a400e 100644
--- a/lib/bbdev/meson.build
+++ b/lib/bbdev/meson.build
@@ -7,8 +7,10 @@ if is_windows
subdir_done()
endif
-sources = files('rte_bbdev.c')
+sources = files('rte_bbdev.c',
+'bbdev_trace_points.c')
headers = files('rte_bbdev.h',
'rte_bbdev_pmd.h',
-'rte_bbdev_op.h')
+'rte_bbdev_op.h',
+'rte_bbdev_trace_fp.h')
deps += ['mbuf']
diff --git a/lib/bbdev/rte_bbdev.c b/lib/bbdev/rte_bbdev.c
index bd32da79b0..eda74591bf 100644
--- a/lib/bbdev/rte_bbdev.c
+++ b/lib/bbdev/rte_bbdev.c
@@ -321,6 +321,8 @@ rte_bbdev_setup_queues(uint16_t dev_id, uint16_t
num_queues, int socket_id)
VALID_DEV_OPS_OR_RET_ERR(dev, dev_id);
+ rte_bbdev_trace_setup_queues(dev_id, num_queues, socket_id);
+
if (dev->data->started) {
rte_bbdev_log(ERR,
"Device %u cannot be configured when started",
@@ -436,6 +438,10 @@ int
rte_bbdev_queue_configure(uint16_t dev_id, uint16_t queue_id,
const struct rte_bbdev_queue_conf *conf)
{
+
+ rte_bbdev_trace_queue_configure(dev_id, queue_id,
rte_bbdev_op_type_str(conf->op_type),
+ conf->priority);
+
int ret = 0;
struct rte_bbdev_driver_info dev_info;
struct r
