Re: [PATCH v9 02/23] test: replace use of term segregate
05/02/2024 18:43, Stephen Hemminger: > Change comment based on inclusive naming recommendations. > > https://inclusivenaming.org/word-lists/tier-3/ > > Signed-off-by: Stephen Hemminger > Acked-by: Tyler Retzlaff Applied individually, there is another (deferred) series for "sanity" word.
Re: [PATCH v2] replace use of word segregate in comments
09/10/2024 02:36, fengchengwen: > Acked-by: Chengwen Feng > > On 2024/10/8 23:49, Stephen Hemminger wrote: > > The use of the word segregate should be avoided. > > Rationale from https://inclusivenaming.org/word-lists/tier-3/segregate/ > > > > The word segregation carries strong context in regard to civil rights > > movements in the US and South Africa, segregation in the US South, and > > racist history. > > > > Signed-off-by: Stephen Hemminger > > Acked-by: Morten Brørup Applied, thanks.
Re: [PATCH v9 04/23] test: remove use of word master in test_red
05/02/2024 18:43, Stephen Hemminger: > No need to use term master here. > > Signed-off-by: Stephen Hemminger > Acked-by: Tyler Retzlaff Applied individually, there is another (deferred) series for "sanity" word.
Re: [PATCH v2 0/3] bugfix about KEEP CRC offload
On Thu, 18 Jul 2024 19:48:02 +0800 Jie Hai wrote: > From: Dengdui Huang > > For hns3 NIC, when KEEP_CRC offload is enabled, the CRC data is still be > stripped in rare scenarios. > Some users of hns3 are already using this feature. So driver has to > recaculate packet CRC. > > In addition, in the mbuf, the data that exceeds the length specified by > pkt_len is invalid. > Therefore, if the packet contains CRC data, pkt_len should contain the CRC > data length. > However, almost of drivers supported KEEP CRC feature didn't add the CRC data > length to pkt_len. > This patch adds description for KEEP CRC offload. > > Dengdui Huang (3): > ethdev: add description for KEEP CRC offload > net/hns3: fix packet length do not contain CRC data length > net/hns3: fix Rx packet without CRC data > > drivers/net/hns3/hns3_ethdev.c| 5 + > drivers/net/hns3/hns3_ethdev.h| 23 + > drivers/net/hns3/hns3_rxtx.c | 134 -- > drivers/net/hns3/hns3_rxtx.h | 3 + > drivers/net/hns3/hns3_rxtx_vec.c | 3 +- > drivers/net/hns3/hns3_rxtx_vec_neon.h | 19 > drivers/net/hns3/hns3_rxtx_vec_sve.c | 3 +- > lib/ethdev/rte_ethdev.h | 6 ++ > 8 files changed, 124 insertions(+), 72 deletions(-) > > -- Reworded some of the commit message to improve readability. Applied to next-net.
[RFC] net/hns3: clarify handling of crc reinsert
Use a static assert rather than a comment to express why there will be space in the buffer. Use memcpy() rather than rte_memcpy(). Signed-off-by: Stephen Hemminger --- drivers/net/hns3/hns3_rxtx.c | 24 ++-- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/drivers/net/hns3/hns3_rxtx.c b/drivers/net/hns3/hns3_rxtx.c index 861511547d..9272584d24 100644 --- a/drivers/net/hns3/hns3_rxtx.c +++ b/drivers/net/hns3/hns3_rxtx.c @@ -2449,6 +2449,15 @@ hns3_need_recalculate_crc(struct hns3_rx_queue *rxq, struct rte_mbuf *m) return true; } +/* + * The hns3 driver requires that mbuf size must be at least 512B. + * When CRC is stripped by hardware, the pkt_len must be less than + * or equal to 60B. Therefore, the space of the mbuf is enough + * to insert the CRC. + */ +static_assert(HNS3_KEEP_CRC_OK_MIN_PKT_LEN < HNS3_MIN_BD_BUF_SIZE, + "buffer size too small to insert CRC"); + static inline void hns3_recalculate_crc(struct rte_mbuf *m) { @@ -2459,18 +2468,13 @@ hns3_recalculate_crc(struct rte_mbuf *m) m->data_len, RTE_NET_CRC32_ETH); /* -* The hns3 driver requires that mbuf size must be at least 512B. -* When CRC is stripped by hardware, the pkt_len must be less than -* or equal to 60B. Therefore, the space of the mbuf is enough -* to insert the CRC. -* -* In addition, after CRC is stripped by hardware, pkt_len and data_len -* do not contain the CRC length. Therefore, after CRC data is appended -* by PMD again, both pkt_len and data_len add the CRC length. +* After CRC is stripped by hardware, pkt_len and data_len do not contain the CRC length. +* Therefore, after CRC data is appended by PMD again. */ append_data = rte_pktmbuf_append(m, RTE_NET_CRC32_ETH); - /* The CRC data is binary data and does not care about the byte order. */ - rte_memcpy(append_data, (void *)&crc, RTE_NET_CRC32_ETH); + + /* CRC data is binary data and does not care about the byte order. */ + memcpy(append_data, &crc, RTE_NET_CRC32_ETH); } uint16_t -- 2.45.2
Re: [RFC PATCH 00/21] Reduce code duplication across Intel NIC drivers
25/11/2024 17:31, Bruce Richardson: > On Mon, Nov 25, 2024 at 05:25:47PM +0100, David Marchand wrote: > > Hello Bruce, > > > > On Fri, Nov 22, 2024 at 1:54 PM Bruce Richardson > > wrote: > > > > > > This RFC attempts to reduce the amount of code duplication across a > > > number of Intel NIC drivers, specifically: ixgbe, i40e, iavf, and ice. > > > > Thanks for starting this effort! > > > > > > > > The first patch extract a function from the Rx side, otherwise the > > > majority of the changes are on the Tx side, leading to a converged Tx > > > queue structure across the 4 drivers, and a large number of common > > > functions. > > > > > > Open question: > > > * How should common code across drivers within a single device class be > > > managed? > > > - For now, I've created an "intel_eth" folder within the "common" > > > driver directory, thinking about it after, it implies to me that > > > it is common across driver classes. > > > - Would it be better to create an "intel_common" directory within the > > > "net" folder? > > > > common/ drivers currently host code that is device class agnostic, > > like providing helpers to talk with hw. > > No common/ driver has a dependency on some device class library. > > > > This series adds code that is not built into a library so there is no > > need to express dependencies in meson. > > But if the need arises, could it become a problem? (adding a > > dependency to lib/ethdev to some drivers/common/xx/). > > > > > > For now, I prefer the second proposition and have this code hosted in > > drivers/net/. > > > Thanks for the feedback. While when I started this prototyping I felt that > common was the right place for it, at this point I'm now tending towards > this second location - keeping it in net. > Any other thoughts on the relative merits of the various locations? We just need to know in which order building the common directory. It can be before bus drivers or later, but you cannot have bus common code, and some ethdev code at the same time. If you just want to share code inside drivers/net/, I suppose it is OK to keep it there. In any choice you do, you will have to maintain some restrictions on the content due to the location.
Re: [PATCH] devtools: download scripts from linux if not found
Thomas Monjalon, Nov 26, 2024 at 15:03: I would prefer we host our own fork of checkpatch.pl. There are too many irrelevant checks that we should drop. How about get_maintainer.pl? Should we host our copy as well?
Re: [PATCH v4 7/9] test/eal: fix core check in c flag test
Stephen Hemminger writes: > The expression for checking which lcore is enabled for 0-7 > was wrong (missing case for 6). > > Link: https://pvs-studio.com/en/blog/posts/cpp/1179/ > > Fixes: b0209034f2bb ("test/eal: check number of cores before running > subtests") > Cc: msant...@redhat.com > Cc: sta...@dpdk.org > > Signed-off-by: Stephen Hemminger > Acked-by: Bruce Richardson > --- Acked-by: Aaron Conole > app/test/test_eal_flags.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c > index d37d6b8627..e32f83d3c8 100644 > --- a/app/test/test_eal_flags.c > +++ b/app/test/test_eal_flags.c > @@ -677,8 +677,8 @@ test_missing_c_flag(void) > > if (rte_lcore_is_enabled(0) && rte_lcore_is_enabled(1) && > rte_lcore_is_enabled(2) && rte_lcore_is_enabled(3) && > - rte_lcore_is_enabled(3) && rte_lcore_is_enabled(5) && > - rte_lcore_is_enabled(4) && rte_lcore_is_enabled(7) && > + rte_lcore_is_enabled(4) && rte_lcore_is_enabled(5) && > + rte_lcore_is_enabled(6) && rte_lcore_is_enabled(7) && > launch_proc(argv29) != 0) { > printf("Error - " > "process did not run ok with valid corelist value\n");
Re: release candidate 24.11-rc3
IBM - Power Systems DPDK v24.11-rc3-1-gf4ccce58c1 * Build CI on Fedora 38,39,40 and 41 container images for ppc64le * Basic PF on Mellanox: No issue found * Performance: not tested. * OS: RHEL 9.4 kernel: 5.14.0-427.40.1.el9_4.ppc64le with gcc version 11.4.1 20231218 (Red Hat 11.4.1-3) (GCC) SLES15 SP5 kernel: 5.14.21-150500.55.49-default with gcc version 13.2.1 20230912 (SUSE Linux) Systems tested: - LPARs on IBM Power10 CHRP IBM,9105-22A NICs: - Mellanox Mellanox Technologies MT2894 Family [ConnectX-6 Lx] - firmware version: 26.42.1000 - MLNX_OFED_LINUX-24.07-0.6.1.5 Regards, Thinh Tran On 11/19/2024 8:12 PM, Thomas Monjalon wrote: A new DPDK release candidate is ready for testing: https://git.dpdk.org/dpdk/tag/?id=v24.11-rc3 There are 192 new patches in this snapshot. Hopefully almost everything expected should be in. Release notes: https://doc.dpdk.org/guides/rel_notes/release_24_11.html Highlights of 24.11-rc3: - Realtek r8169 net driver - ZTE gdtc raw driver As usual, you can report any issue on https://bugs.dpdk.org Only documentation and critical bug fixes should be accepted at this stage. There are only 10 days remaining before the end of the month. DPDK 24.11-rc4 should be the last release candidate (in one week). Thank you everyone
[PATCH] fib: rename configuration flag
Rename RTE_FIB_F_NETWORK_ORDER with RTE_FIB_F_LOOKUP_NETWORK_ORDER to explicitly indicate that it is only used in lookup. Fixes: e194f3cd5685 ("fib: lookup IPv4 address in network order") Signed-off-by: Vladimir Medvedkin --- lib/fib/rte_fib.c | 4 ++-- lib/fib/rte_fib.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/fib/rte_fib.c b/lib/fib/rte_fib.c index db79fc428e..49211fe3fa 100644 --- a/lib/fib/rte_fib.c +++ b/lib/fib/rte_fib.c @@ -111,7 +111,7 @@ init_dataplane(struct rte_fib *fib, __rte_unused int socket_id, if (fib->dp == NULL) return -rte_errno; fib->lookup = dir24_8_get_lookup_fn(fib->dp, - RTE_FIB_LOOKUP_DEFAULT, !!(fib->flags & RTE_FIB_F_NETWORK_ORDER)); + RTE_FIB_LOOKUP_DEFAULT, !!(fib->flags & RTE_FIB_F_LOOKUP_NETWORK_ORDER)); fib->modify = dir24_8_modify; return 0; default: @@ -333,7 +333,7 @@ rte_fib_select_lookup(struct rte_fib *fib, switch (fib->type) { case RTE_FIB_DIR24_8: fn = dir24_8_get_lookup_fn(fib->dp, type, - !!(fib->flags & RTE_FIB_F_NETWORK_ORDER)); + !!(fib->flags & RTE_FIB_F_LOOKUP_NETWORK_ORDER)); if (fn == NULL) return -EINVAL; fib->lookup = fn; diff --git a/lib/fib/rte_fib.h b/lib/fib/rte_fib.h index 362bb8f08c..1eee79c955 100644 --- a/lib/fib/rte_fib.h +++ b/lib/fib/rte_fib.h @@ -88,8 +88,8 @@ enum rte_fib_lookup_type { }; /** If set, fib lookup is expecting IPv4 address in network byte order */ -#define RTE_FIB_F_NETWORK_ORDER1 -#define RTE_FIB_ALLOWED_FLAGS (RTE_FIB_F_NETWORK_ORDER) +#define RTE_FIB_F_LOOKUP_NETWORK_ORDER 1 +#define RTE_FIB_ALLOWED_FLAGS (RTE_FIB_F_LOOKUP_NETWORK_ORDER) /** FIB configuration structure */ struct rte_fib_conf { -- 2.43.0
[PATCH] dts: remove leftover Node methods
The "remove redundant test suite" removed an unused test suite and some dead code with it. Some dead code which references now-removed symbols, remained though. This removes this code, therefore fixing the related mypy errors. Fixes: e3ab9dd5cd5d ("dts: remove redundant test suite") Signed-off-by: Luca Vizzarro Reviewed-by: Paul Szczepanek --- dts/framework/testbed_model/node.py | 26 -- 1 file changed, 26 deletions(-) diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py index 85144f6f4e..c1844ecd5d 100644 --- a/dts/framework/testbed_model/node.py +++ b/dts/framework/testbed_model/node.py @@ -14,8 +14,6 @@ """ from abc import ABC -from ipaddress import IPv4Interface, IPv6Interface -from typing import Union from framework.config import ( OS, @@ -192,30 +190,6 @@ def _setup_hugepages(self) -> None: self.config.hugepages.force_first_numa, ) -def configure_port_state(self, port: Port, enable: bool = True) -> None: -"""Enable/disable `port`. - -Args: -port: The port to enable/disable. -enable: :data:`True` to enable, :data:`False` to disable. -""" -self.main_session.configure_port_state(port, enable) - -def configure_port_ip_address( -self, -address: Union[IPv4Interface, IPv6Interface], -port: Port, -delete: bool = False, -) -> None: -"""Add an IP address to `port` on this node. - -Args: -address: The IP address with mask in CIDR format. Can be either IPv4 or IPv6. -port: The port to which to add the address. -delete: If :data:`True`, will delete the address from the port instead of adding it. -""" -self.main_session.configure_port_ip_address(address, port, delete) - def close(self) -> None: """Close all connections and free other resources.""" if self.main_session: -- 2.43.0
Re: [PATCH v4 0/8] Record and rework component dependencies
On 11/26/2024 3:39 PM, Anatoly Burakov wrote: As part of the meson build, we can record the dependencies for each component as we process it, logging them to a file. This file can be used as input to a number of other scripts and tools, for example, to graph the dependencies, or to allow higher-level build-config tools to automatically enable component requirements, etc. The first patch of this set separates dependencies inside meson into optional or mandatory. The second patch of this set generates the basic dependency tree. The third patch does some processing of that dependency tree to identify cases where dependencies are being unnecessarily specified. Reducing these makes it easier to have readable dependency graphs in future, without affecting the build. The following 4 patches are based on the output of the second patch, and greatly cut down the number of direct dependency links between components. Even with the cut-down dependencies, the full dependency graph is nigh-unreadable, so the final patch adds a new script to generate dependency tree subgraphs, creating dot files for e.g. the dependencies of a particular component, or a component class such as mempool drivers. v3 -> v4: - Update to latest main v2 -> v3: - Split dependencies into optional and mandatory - Fixup graph scripts to read and generate graphs that encode optional dependencies into the graph - Python version fixes to avoid using features not available in minimum supported Python version - Formatting with Ruff, and PEP-484 compliance Forgot the series ack: Acked-by: Morten Brørup -- Thanks, Anatoly
[PATCH] dts: remove html dir from dts doc path
To facilitate deploying docs to the website and make paths more consistent remove the html directory from the dts doc path. Signed-off-by: Paul Szczepanek Reviewed-by: Luca Vizzarro --- buildtools/call-sphinx-build.py | 5 +++-- doc/api/meson.build | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/buildtools/call-sphinx-build.py b/buildtools/call-sphinx-build.py index 2c7de54285..93ed81ac88 100755 --- a/buildtools/call-sphinx-build.py +++ b/buildtools/call-sphinx-build.py @@ -30,8 +30,9 @@ # run sphinx, putting the html output in a "html" directory with open(join(dst, 'sphinx_html.out'), 'w') as out: -process = run(sphinx_cmd + ['-b', 'html', src, join(dst, 'html')], - stdout=out) +# dts is a special case which doesn't require an html dir in destination path +html_dst = dst if 'dts' in src else join(dst, 'html') +process = run(sphinx_cmd + ['-b', 'html', src, html_dst], stdout=out) # create a gcc format .d file giving all the dependencies of this doc build with open(join(dst, '.html.d'), 'w') as d: diff --git a/doc/api/meson.build b/doc/api/meson.build index f9f1326bbd..ac6eb8236d 100644 --- a/doc/api/meson.build +++ b/doc/api/meson.build @@ -37,7 +37,7 @@ cdata.set('OUTPUT', join_paths(dpdk_build_root, 'doc', 'api')) cdata.set('TOPDIR', dpdk_source_root) cdata.set('STRIP_FROM_PATH', ' '.join([dpdk_source_root, join_paths(dpdk_build_root, 'doc', 'api')])) cdata.set('WARN_AS_ERROR', 'NO') -cdata.set('DTS_API_MAIN_PAGE', join_paths('.', 'dts', 'html', 'index.html')) +cdata.set('DTS_API_MAIN_PAGE', join_paths('.', 'dts', 'index.html')) if get_option('werror') cdata.set('WARN_AS_ERROR', 'YES') endif -- 2.39.2
Re: [PATCH] fib: rename configuration flag
26/11/2024 18:18, Vladimir Medvedkin: > Rename RTE_FIB_F_NETWORK_ORDER with RTE_FIB_F_LOOKUP_NETWORK_ORDER to > explicitly indicate that it is only used in lookup. > > Fixes: e194f3cd5685 ("fib: lookup IPv4 address in network order") > > Signed-off-by: Vladimir Medvedkin Applied, thanks.
Re: [PATCH] test: raise fast test timeout to 60s on RISC-V
On Sat, Nov 23, 2024 at 4:00 PM Eric Long wrote: > > Current RISC-V hardware (e.g. HiFive Unmatched) is still way too slow > compared to other architectures' counterparts. On the most powerful > RISC-V CPU available (SG2042), DPDK still fails with 4 timeouts: > > ``` > Summary of Failures: > > 23/77 DPDK:fast-tests / eventdev_selftest_swTIMEOUT > 10.06s killed by signal 15 SIGTERM > 56/77 DPDK:fast-tests / rib6_autotest TIMEOUT > 10.04s > 64/77 DPDK:fast-tests / spinlock_autotest TIMEOUT > 10.06s killed by signal 15 SIGTERM > 66/77 DPDK:fast-tests / table_autotest TIMEOUT > 11.28s killed by signal 15 SIGTERM > ``` > > On HiFive Unmatched, the longest test takes 53 seconds: > > ``` > 37/77 DPDK:fast-tests / lpm6_autotest OK > 53.29s > ``` > > Raising timeout to 60s on RISC-V target will help test effectiveness > on it. You can extend the timeout via the multiplier option (default timeout of 10s * multiplier). So in your case: $ meson test -C --suite fast-tests -t 6 -- David Marchand
Re: [PATCH v2] ci: run more checks in private repositories
On Tue, Jul 9, 2024 at 3:17 PM Robin Jarry wrote: > > From: David Marchand > > Though devtools/checkpatches.sh is run as part of our CI, some other > (not well known) checks could help when run in private repositories > before submitting to the mailing list and even when run from the > ovsrobot. > > Most of them require a git history or checked sources to run. > And I can't guarantee there won't be false positives. > > Add a new job just for those checks so that it won't block compilation > tests in other jobs. > > Signed-off-by: David Marchand > Signed-off-by: Robin Jarry > Acked-by: Aaron Conole Thanks for respinning, applied. -- David Marchand
Re: [PATCH v6 4/4] usertools/dpdk-devbind: print NUMA node
On Sat, Oct 12, 2024 at 7:58 PM Stephen Hemminger wrote: > On Thu, 22 Aug 2024 11:38:26 +0100 > Anatoly Burakov wrote: > > > Currently, devbind does not print out any NUMA information, which makes > > figuring out which NUMA node device belongs to not trivial. Add printouts > > for NUMA information if NUMA support is enabled on the system. > > > > Signed-off-by: Anatoly Burakov > > Acked-by: Robin Jarry > Acked-by: Stephen Hemminger Series applied, thanks for the cleanups and enhancements. -- David Marchand
Re: [PATCH] dts: remove leftover Node methods
This looks good, and I can confirm dts-check-format.sh is passing with this patch. Reviewed-by: Patrick Robb Tested-by: Patrick Robb On Tue, Nov 26, 2024 at 10:09 AM Luca Vizzarro wrote: > The "remove redundant test suite" removed an unused test suite and some > dead code with it. Some dead code which references now-removed symbols, > remained though. This removes this code, therefore fixing the related > mypy errors. > > Fixes: e3ab9dd5cd5d ("dts: remove redundant test suite") > > Signed-off-by: Luca Vizzarro > Reviewed-by: Paul Szczepanek > --- > dts/framework/testbed_model/node.py | 26 -- > 1 file changed, 26 deletions(-) > > diff --git a/dts/framework/testbed_model/node.py > b/dts/framework/testbed_model/node.py > index 85144f6f4e..c1844ecd5d 100644 > --- a/dts/framework/testbed_model/node.py > +++ b/dts/framework/testbed_model/node.py > @@ -14,8 +14,6 @@ > """ > > from abc import ABC > -from ipaddress import IPv4Interface, IPv6Interface > -from typing import Union > > from framework.config import ( > OS, > @@ -192,30 +190,6 @@ def _setup_hugepages(self) -> None: > self.config.hugepages.force_first_numa, > ) > > -def configure_port_state(self, port: Port, enable: bool = True) -> > None: > -"""Enable/disable `port`. > - > -Args: > -port: The port to enable/disable. > -enable: :data:`True` to enable, :data:`False` to disable. > -""" > -self.main_session.configure_port_state(port, enable) > - > -def configure_port_ip_address( > -self, > -address: Union[IPv4Interface, IPv6Interface], > -port: Port, > -delete: bool = False, > -) -> None: > -"""Add an IP address to `port` on this node. > - > -Args: > -address: The IP address with mask in CIDR format. Can be > either IPv4 or IPv6. > -port: The port to which to add the address. > -delete: If :data:`True`, will delete the address from the > port instead of adding it. > -""" > -self.main_session.configure_port_ip_address(address, port, delete) > - > def close(self) -> None: > """Close all connections and free other resources.""" > if self.main_session: > -- > 2.43.0 > >
Re: [PATCH] dts: remove leftover Node methods
Applied to next-dts, thanks. On Tue, Nov 26, 2024 at 10:09 AM Luca Vizzarro wrote: > The "remove redundant test suite" removed an unused test suite and some > dead code with it. Some dead code which references now-removed symbols, > remained though. This removes this code, therefore fixing the related > mypy errors. > > Fixes: e3ab9dd5cd5d ("dts: remove redundant test suite") > > Signed-off-by: Luca Vizzarro > Reviewed-by: Paul Szczepanek > --- > dts/framework/testbed_model/node.py | 26 -- > 1 file changed, 26 deletions(-) > > diff --git a/dts/framework/testbed_model/node.py > b/dts/framework/testbed_model/node.py > index 85144f6f4e..c1844ecd5d 100644 > --- a/dts/framework/testbed_model/node.py > +++ b/dts/framework/testbed_model/node.py > @@ -14,8 +14,6 @@ > """ > > from abc import ABC > -from ipaddress import IPv4Interface, IPv6Interface > -from typing import Union > > from framework.config import ( > OS, > @@ -192,30 +190,6 @@ def _setup_hugepages(self) -> None: > self.config.hugepages.force_first_numa, > ) > > -def configure_port_state(self, port: Port, enable: bool = True) -> > None: > -"""Enable/disable `port`. > - > -Args: > -port: The port to enable/disable. > -enable: :data:`True` to enable, :data:`False` to disable. > -""" > -self.main_session.configure_port_state(port, enable) > - > -def configure_port_ip_address( > -self, > -address: Union[IPv4Interface, IPv6Interface], > -port: Port, > -delete: bool = False, > -) -> None: > -"""Add an IP address to `port` on this node. > - > -Args: > -address: The IP address with mask in CIDR format. Can be > either IPv4 or IPv6. > -port: The port to which to add the address. > -delete: If :data:`True`, will delete the address from the > port instead of adding it. > -""" > -self.main_session.configure_port_ip_address(address, port, delete) > - > def close(self) -> None: > """Close all connections and free other resources.""" > if self.main_session: > -- > 2.43.0 > >
Re: [PATCH v4 0/9] Bug fixes for standalone tests
21/11/2024 19:23, Stephen Hemminger: > Recent blog post from PVS studio referenced lots bugs found by > their analyzer against DPDK. This set addresses the ones in > the test suite. > > v4 - rebase and drop code that was already fixed (removed) > > Stephen Hemminger (9): > app/test: do not duplicate loop variable > app/test: fix typo in address compare > app/test: fix paren typo > app/test: avoid duplicate initialization > app/test: fix TLS zero length record > app/test: fix operator precedence bug > test/eal: fix core check in c flag test > app/test-pmd: remove redundant condition > app/test-pmd: avoid potential outside of array reference Applied, thanks.
Re: [PATCH] test/cfgfile: add check for file removal
20/11/2024 17:50, Stephen Hemminger: > The test makes temporary files for parsing and then cleans up. > It was not checking the return value from the remove step > which makes Coverity unhappy. > > Coverity ID: 451207 > Coverity ID: 451209 > Coverity ID: 451212 > Coverity ID: 451213 > Coverity ID: 451215 > Coverity ID: 451217 > Coverity ID: 451222 > Coverity ID: 451225 We use the tag "Coverity issue" and allow for a list with commas. > > Fixes: be22019a58c4 ("test: restore cfgfile tests") > > Signed-off-by: Stephen Hemminger Applied, thanks
Re: [PATCH v2 1/1] usertools/devbind: allow changing UID/GID for VFIO
On Tue, Nov 26, 2024 at 03:02:38PM +, Anatoly Burakov wrote: > Currently, when binding a device to VFIO, the UID/GID for the device will > always stay as system default (`root`). Yet, when running DPDK as non-root > user, one has to change the UID/GID of the device to match the user's > UID/GID to use the device. > > This patch adds an option to `dpdk-devbind.py` to change the UID/GID of > the device when binding it to VFIO. > > Signed-off-by: Anatoly Burakov > --- Acked-by: Bruce Richardson
Re: [PATCH v1] net/i40e: updated latest recommended matching list
On Mon, Nov 25, 2024 at 10:45:53AM +0800, hailinx wrote: > Signed-off-by: hailinx > --- > doc/guides/nics/i40e.rst | 8 > 1 file changed, 8 insertions(+) > > diff --git a/doc/guides/nics/i40e.rst b/doc/guides/nics/i40e.rst > index ca6caa0cff..35585efc44 100644 > --- a/doc/guides/nics/i40e.rst > +++ b/doc/guides/nics/i40e.rst Applied to dpdk-next-net-intel, thanks, /Bruce
Re: [RFC PATCH 00/21] Reduce code duplication across Intel NIC drivers
On Tue, Nov 26, 2024 at 03:57:42PM +0100, Thomas Monjalon wrote: > 25/11/2024 17:31, Bruce Richardson: > > On Mon, Nov 25, 2024 at 05:25:47PM +0100, David Marchand wrote: > > > Hello Bruce, > > > > > > On Fri, Nov 22, 2024 at 1:54 PM Bruce Richardson > > > wrote: > > > > > > > > This RFC attempts to reduce the amount of code duplication across a > > > > number of Intel NIC drivers, specifically: ixgbe, i40e, iavf, and ice. > > > > > > Thanks for starting this effort! > > > > > > > > > > > The first patch extract a function from the Rx side, otherwise the > > > > majority of the changes are on the Tx side, leading to a converged Tx > > > > queue structure across the 4 drivers, and a large number of common > > > > functions. > > > > > > > > Open question: > > > > * How should common code across drivers within a single device class be > > > > managed? > > > > - For now, I've created an "intel_eth" folder within the "common" > > > > driver directory, thinking about it after, it implies to me that > > > > it is common across driver classes. > > > > - Would it be better to create an "intel_common" directory within the > > > > "net" folder? > > > > > > common/ drivers currently host code that is device class agnostic, > > > like providing helpers to talk with hw. > > > No common/ driver has a dependency on some device class library. > > > > > > This series adds code that is not built into a library so there is no > > > need to express dependencies in meson. > > > But if the need arises, could it become a problem? (adding a > > > dependency to lib/ethdev to some drivers/common/xx/). > > > > > > > > > For now, I prefer the second proposition and have this code hosted in > > > drivers/net/. > > > > > Thanks for the feedback. While when I started this prototyping I felt that > > common was the right place for it, at this point I'm now tending towards > > this second location - keeping it in net. > > Any other thoughts on the relative merits of the various locations? > > We just need to know in which order building the common directory. > It can be before bus drivers or later, but you cannot have bus common code, > and some ethdev code at the same time. > If you just want to share code inside drivers/net/, I suppose it is OK to > keep it there. > In any choice you do, you will have to maintain some restrictions on the > content due to the location. > Yes, good point. However, that would only apply if the common code was being built into a separate component to be linked into the other components using it. The initial prototype work has resulted in header files only, and my thinking was that even if .c files are added, they would not be compiled into a .a file, but rather compiled as source into each individual driver using them. This would allow for a certain amount of ifdef usage, for example, where things may have slight differences. However, I think we cross that bridge when we come to it. /Bruce
Re: [PATCH v1] net/ice: updated latest recommended matching list
On Mon, Nov 25, 2024 at 10:52:20AM +0800, hailinx wrote: > Signed-off-by: hailinx > --- > doc/guides/nics/ice.rst | 4 > 1 file changed, 4 insertions(+) > Applied to dpdk-next-net-intel. Thanks, /Bruce
Re: [PATCH v2 1/3] net/ixgbe: initialize PTP to system time
On Mon, Nov 25, 2024 at 11:33:24AM +, Anatoly Burakov wrote: > Currently, ixgbe driver initializes PTP timestamp to 0. This is different > from what kernel driver does (which initializes it to system time). > > Align the DPDK driver to kernel driver by setting PTP timestamp to system > time when enabling PTP. > > Note that ixgbe driver always uses zero-based timestamps for PTP, so we > would only ever update the internal timecounter and not the actual NIC > registers. > > Implementation note: in order to get access to clock_gettime on MinGW, we > have to use rte_os_shim.h header, which provides a wrapper around that > function. However, what it *also* provides is wrapper macros around various > other OS-related functions such as read(). Due to one of the mailbox ops > in base code being called "read", MinGW will misinterpret a call into > that op as an attempt to call read() the OS function, and produce a > compile error. This is being worked around by using parentheses around > access to the read op. > > Signed-off-by: Anatoly Burakov > --- Series applied to dpdk-next-net-intel. Thanks, /Bruce
Re: [PATCH v2 1/1] usertools/devbind: allow changing UID/GID for VFIO
Hi Anatoly, Anatoly Burakov, Nov 26, 2024 at 16:02: Currently, when binding a device to VFIO, the UID/GID for the device will always stay as system default (`root`). Yet, when running DPDK as non-root user, one has to change the UID/GID of the device to match the user's UID/GID to use the device. This patch adds an option to `dpdk-devbind.py` to change the UID/GID of the device when binding it to VFIO. Signed-off-by: Anatoly Burakov --- Notes: v1 -> v2: - Replaced hard exit with an error printout Sorry I had missed that particular detail. I don't think this should only print a warning. Otherwise, the user has no way to detect if the operation failed. usertools/dpdk-devbind.py | 41 +-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py index f2a2a9a12f..496d0e90e8 100755 --- a/usertools/dpdk-devbind.py +++ b/usertools/dpdk-devbind.py @@ -8,6 +8,8 @@ import subprocess import argparse import platform +import grp +import pwd We may already be past this but could you try to sort these imports alphabetically? from glob import glob from os.path import exists, basename @@ -108,6 +110,8 @@ status_flag = False force_flag = False noiommu_flag = False +vfio_uid = "" +vfio_gid = "" These are supposed to be integers. Initialize them to -1. args = [] @@ -463,6 +467,22 @@ def bind_one(dev_id, driver, force): % (dev_id, filename, err)) +def own_one(dev_id, uid, gid): +"""Set the IOMMU group ownership for a device""" +# find IOMMU group for a particular device +iommu_grp_base_path = os.path.join("/sys/bus/pci/devices", dev_id, "iommu_group") +try: +iommu_grp = os.path.basename(os.readlink(iommu_grp_base_path)) +# we found IOMMU group, now find the device +dev_path = os.path.join("/dev/vfio", iommu_grp) +# set the ownership +_uid = pwd.getpwnam(uid).pw_uid if uid else -1 +_gid = grp.getgrnam(gid).gr_gid if gid else -1 The validity of these values should be checked when parsing command line arguments. +os.chown(dev_path, _uid, _gid) +except OSError as err: +print(f"Error: failed to read IOMMU group for {dev_id}: {err}") Remove the try/except block and let the error bubble up the stack. This probably does not require a dedicated function. Moreover, the name own_one() is ambiguous. + + def unbind_all(dev_list, force=False): """Unbind method, takes a list of device locations""" @@ -512,7 +532,7 @@ def check_noiommu_mode(): print("Warning: enabling unsafe no IOMMU mode for VFIO drivers") -def bind_all(dev_list, driver, force=False): +def bind_all(dev_list, driver, uid, gid, force=False): Not required. These are global variables. """Bind method, takes a list of device locations""" global devices @@ -544,6 +564,9 @@ def bind_all(dev_list, driver, force=False): for d in dev_list: bind_one(d, driver, force) +# if we're binding to vfio-pci, set the IOMMU user/group ownership if one was specified +if driver == "vfio-pci" and (uid or gid): if driver == "vfio-pci" and (vfio_uid != -1 or vfio_gid != -1): +own_one(d, uid, gid) It will be better to store the chmod code path here: iommu_grp = os.path.join("/sys/bus/pci/devices", d, "iommu_group") iommu_grp = os.path.basename(os.readlink(iommu_grp)) os.chown(os.path.join("/dev/vfio", iommu_grp), vfio_uid, vfio_gid) # For kernels < 3.15 when binding devices to a generic driver # (i.e. one that doesn't have a PCI ID table) using new_id, some devices @@ -697,6 +720,8 @@ def parse_args(): global force_flag global noiommu_flag global args +global vfio_uid +global vfio_gid parser = argparse.ArgumentParser( description='Utility to bind and unbind devices from Linux kernel', @@ -746,6 +771,12 @@ def parse_args(): '--noiommu-mode', action='store_true', help="If IOMMU is not available, enable no IOMMU mode for VFIO drivers") +parser.add_argument( +"-U", "--uid", help="For VFIO, specify the UID to set IOMMU group ownership" In order to fail early if an invalid user name is passed, add these two lines: type=lambda u: pwd.getpwnam(u).pw_uid, default=-1, +) +parser.add_argument( +"-G", "--gid", help="For VFIO, specify the GID to set IOMMU group ownership" In order to fail early if an invalid group name is passed, add these two lines: type=lambda g: grp.getgrnam(g).gr_gid, default=-1, +) parser.add_argument( '--force', action='store_true', @@ -778,6 +809,10 @@ def parse_args(): b_flag = opt.bind elif opt.unbind: b_flag = "none" +if opt.uid: +vfio_uid = opt.uid +if opt.gi
Re: [PATCH] devtools: download scripts from linux if not found
04/10/2024 18:31, Stephen Hemminger: > On Fri, 15 Mar 2024 15:14:42 +0100 > Robin Jarry wrote: > > > Both checkpatches.sh and get-maintainer.sh require scripts that come > > from the linux sources. And they require DPDK_*_PATH variables to be set > > to point at these scripts locally. For new contributors this can be > > tedious since they will have to clone the whole linux sources just to > > have simple perl scripts. > > > > Update checkpatches.sh and get-maintainer.sh to have them download the > > upstream scripts if they are not found locally. Store the downloaded > > scripts in the .git/ folder so that they are found for future runs. > > > > If either DPDK_CHECKPATCH_PATH or DPDK_GETMAINTAINER_PATH are set, they > > will be used in priority. > > > > Signed-off-by: Robin Jarry > > This makes live easier for new contributors. > Not sure if is the best way to autodownload, or where to put the scripts > but something is better than the current way. > > Acked-by: Stephen Hemminger I would prefer we host our own fork of checkpatch.pl. There are too many irrelevant checks that we should drop.
[PATCH v2 0/1] Add Visual Studio Code configuration script
Lots of developers (myself included) uses Visual Studio Code as their primary IDE for DPDK development. I have been successfully using various incarnations of this script internally to quickly set up my development trees whenever I need a new configuration, so this script is being shared in hopes that it will be useful both to new developers starting with DPDK, and to seasoned DPDK developers who are already or may want to start using Visual Studio Code. ** NOTE: While code analysis configuration is now populated from Meson and should pick up platform- or OS-specifc configuration, I have no way to test the code analysis configuration on anything but Linux/x86. ** NOTE 2: this is not for *Visual Studio* the Windows IDE, this is for *Visual Studio Code* the cross-platform code editor. Specifically, main target audience for this script is people who either run DPDK directly on their Linux machine, or who use Remote SSH functionality to connect to a remote Linux machine and set up VSCode build there. While the script should in theory work with any OS/platform supported by DPDK, it was not tested under anything but Linux/x86. (if you're unaware of what is Remote SSH, I highly suggest checking it out [1]) At a glance, this script will do the following: - At meson setup time, configuration entries will be created under /.vscode - Any new compiled executable will be added to configuration - Nothing will ever be deleted or modified, only added if it doesn't exist - Launch configuration is created using `which gdb`, so by default non-root users will have to do additional system configuration for things to work. This is now documented in a new section called "Integration with IDE's". - For those concerned about "bloat", the configuration is just a few text files - .vscode directory starts with a dot, so it'll be excluded from any Git updates [1] https://code.visualstudio.com/docs/remote/ssh Anatoly Burakov (1): buildtools: add VSCode configuration generator app/meson.build | 14 +- buildtools/gen-vscode-conf.py | 570 buildtools/meson.build | 5 + devtools/test-meson-builds.sh | 3 + doc/guides/contributing/ide_integration.rst | 89 +++ doc/guides/contributing/index.rst | 1 + doc/guides/rel_notes/release_24_11.rst | 5 + examples/meson.build| 15 +- meson.build | 14 + 9 files changed, 714 insertions(+), 2 deletions(-) create mode 100755 buildtools/gen-vscode-conf.py create mode 100644 doc/guides/contributing/ide_integration.rst -- 2.43.5
[PATCH v2 1/1] devtools: add DPDK build directory setup script
Currently, the only way to set up a build directory for DPDK development is through running Meson directly. This has a number of drawbacks. For one, the default configuration is very "fat", meaning everything gets enabled and built (aside from examples, which have to be enabled manually), so while Meson is very good at minimizing work needed to rebuild DPDK, for any change that affects a lot of components (such as editing an EAL header), there's a lot of rebuilding to do, which may not be needed. It is of course possible to reduce the number of components built through meson options, but this mechanism isn't perfect, as the user needs to remember exact spelling of all the options and components, and currently it doesn't handle inter-component dependencies very well (e.g. if net/ice is enabled, common/iavf is not automatically enabled, so net/ice can't be built unless user also doesn't forget to specify common/iavf). Error messages are displayed, but to an untrained eye it is not always clear what the user has to do for them to go away. Enter this script. It relies on Meson's introspection capabilities as well as the dependency graphs generated by our build system to display all available components, and handle any dependencies for them automatically, while also not forcing user to remember any command-line options and lists of drivers, and instead relying on interactive TUI to display list of available options. It can also produce builds that are as minimal as possible (including cutting down libraries being built) by utilizing the fact that our dependency graphs report which dependency is mandatory and which one is optional. Because it is not meant to replace native Meson build configuration but is rather targeted at developers who are not intimately familiar with DPDK's build system or want to quickly enable this or that without thinking about dependencies, it is run in interactive mode by default. However, it is also possible to run it without interaction, in which case it will pass all its parameters to Meson directly, with added benefit of dependency tracking and producing minimal builds if desired. Reconfiguring existing build directory is also supported, in which case existing configuration for the flags managed by the script will be kept updated (for other options we can rely on the fact that Meson keeps track of all of the specified options and thus we don't have to re-specify them when we reconfigure). Signed-off-by: Anatoly Burakov --- Notes: v2 -> v1: - Moved to devtools - Added a --dry-run mode - Improved support for reconfiguring existing directory - Some refactoring and code improvements - Different menus now display different prompts - Added documentation devtools/dpdk-setup.py | 784 doc/guides/linux_gsg/build_dpdk.rst | 26 + 2 files changed, 810 insertions(+) create mode 100755 devtools/dpdk-setup.py diff --git a/devtools/dpdk-setup.py b/devtools/dpdk-setup.py new file mode 100755 index 00..b0e28f3d36 --- /dev/null +++ b/devtools/dpdk-setup.py @@ -0,0 +1,784 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: BSD-3-Clause +# Copyright(c) 2024 Intel Corporation + +""" +Displays an interactive TUI-based menu for configuring a DPDK build directory. +""" + +# This is an interactive script that allows the user to configure a DPDK build directory using a +# text-based user interface (TUI). The script will prompt the user to select various configuration +# options, and will then call `meson setup|configure` to configure the build directory with the +# selected options. +# +# To be more user-friendly, the script will also run `meson setup` into a temporary directory in +# the background, which will generate both the list of available options, and any dependencies +# between them, so whenever the user selects an option, we automatically enable its dependencies. +# This will also allow us to use meson introspection to get list of things we are capable of +# building, and warn the user if they selected something that can't be built. + +import argparse +import collections +import fnmatch +import json +import os +import subprocess +import sys +import textwrap +import typing as T +from tempfile import TemporaryDirectory + + +# some apps have different names in the Meson build system +APP_RENAME_MAP = { +"testpmd": "test-pmd", +} + + +# cut off dpdk- prefix +def _unprefix_app(app: str) -> str: +return app[5:] + + +def _prefix_app(app: str) -> str: +return f"dpdk-{app}" + + +def _slash_driver(driver: str) -> str: +return driver.replace("/", "_", 1) + + +def _unslash_driver(driver: str) -> str: +return driver.replace("_", "/", 1) + + +def create_meson_build(src_dir: str, build_dir: str) -> subprocess.Popen[bytes]: +"""Create a Meson build directory in the background.""" +# we want all examples +args = ["meson", "setup", build_dir, "-Dexamples=all"] +return subprocess.Po
[PATCH v2 0/1] Add DPDK build directory configuration script
Note: this patch depends upon Bruce's v4 patchset: https://patches.dpdk.org/project/dpdk/list/?series=34036 This patch is based on initial script for VSCode configuration: https://patches.dpdk.org/project/dpdk/patch/6a6b20c037cffcc5f68a341c4b4e4f21990ae991.1721997016.git.anatoly.bura...@intel.com/ This is a TUI frontend for Meson. It is by no means meant to be used as a replacement for using Meson proper, it is merely a shortcut for those who constantly deal with creating new Meson build directories but don't want to type out all components each time. It relies on dependency graphs from the above Bruce's patchset (v3 introduced support for optional dependencies, which this script requires) to work. It'll create a Meson build directory in the background, enabling all options, and then using both dependency graph and meson introspection to figure out what can be built, and what dependencies it has. With this script it is possible to produce very minimal builds - the script is not only able to track dependencies between components to enable them, but it can also (with a command line switch) specify which libraries we want to enable (omitting those not required by currently selected components). This can be useful for users who frequently reconfigure their tree with e.g. debug/release, shared/static etc. builds while keeping the reconfiguration time fairly small. We used to have a "setup.sh" script to "set up" DPDK. This is not that, but it's a good start. Anatoly Burakov (1): devtools: add DPDK build directory setup script devtools/dpdk-setup.py | 784 doc/guides/linux_gsg/build_dpdk.rst | 26 + 2 files changed, 810 insertions(+) create mode 100755 devtools/dpdk-setup.py -- 2.43.5
Re: [PATCH] ci: remove workaround for ASan in Ubuntu GHA images
On Thu, Nov 7, 2024 at 2:15 PM David Marchand wrote: > > This workaround is directly applied inside Ubuntu GHA images themselves. > > Link: > https://github.com/actions/runner-images/commit/9485052d98ba055be3355565e23630de8f8c4ef8 > Signed-off-by: David Marchand Applied, thanks. -- David Marchand
[PATCH v4 1/8] build: split dependencies into mandatory and optional
Allow specifying dependencies as either mandatory or optional. This does not change anything about the build, but it is useful for tooling to know if a dependency is required or not. Signed-off-by: Anatoly Burakov --- app/meson.build | 3 ++- app/proc-info/meson.build | 2 +- app/test-bbdev/meson.build| 8 app/test-crypto-perf/meson.build | 2 +- app/test-pmd/meson.build | 26 +- app/test/meson.build | 12 ++-- drivers/meson.build | 3 ++- examples/ethtool/meson.build | 2 +- examples/l2fwd-crypto/meson.build | 2 +- examples/l3fwd/meson.build| 2 +- examples/meson.build | 3 ++- examples/vm_power_manager/meson.build | 6 +++--- lib/meson.build | 3 ++- 13 files changed, 39 insertions(+), 35 deletions(-) diff --git a/app/meson.build b/app/meson.build index e2db888ae1..4e88e652ca 100644 --- a/app/meson.build +++ b/app/meson.build @@ -66,6 +66,7 @@ foreach app:apps # external package/library requirements ext_deps = [] deps = [] +optional_deps = [] if not enable_apps.contains(app) build = false @@ -85,7 +86,7 @@ foreach app:apps if build dep_objs = [] -foreach d:deps +foreach d:deps + optional_deps var_name = get_option('default_library') + '_rte_' + d if not is_variable(var_name) build = false diff --git a/app/proc-info/meson.build b/app/proc-info/meson.build index 4f83f29a64..156592119b 100644 --- a/app/proc-info/meson.build +++ b/app/proc-info/meson.build @@ -10,5 +10,5 @@ endif sources = files('main.c') deps += ['ethdev', 'security', 'eventdev'] if dpdk_conf.has('RTE_LIB_METRICS') -deps += 'metrics' +optional_deps += 'metrics' endif diff --git a/app/test-bbdev/meson.build b/app/test-bbdev/meson.build index 926e0a5271..c26e46a987 100644 --- a/app/test-bbdev/meson.build +++ b/app/test-bbdev/meson.build @@ -15,14 +15,14 @@ sources = files( ) deps += ['bbdev', 'bus_vdev'] if dpdk_conf.has('RTE_BASEBAND_FPGA_LTE_FEC') -deps += ['baseband_fpga_lte_fec'] +optional_deps += ['baseband_fpga_lte_fec'] endif if dpdk_conf.has('RTE_BASEBAND_FPGA_5GNR_FEC') -deps += ['baseband_fpga_5gnr_fec'] +optional_deps += ['baseband_fpga_5gnr_fec'] endif if dpdk_conf.has('RTE_BASEBAND_ACC') -deps += ['baseband_acc'] +optional_deps += ['baseband_acc'] endif if dpdk_conf.has('RTE_BASEBAND_LA12XX') -deps += ['baseband_la12xx'] +optional_deps += ['baseband_la12xx'] endif diff --git a/app/test-crypto-perf/meson.build b/app/test-crypto-perf/meson.build index 7b02b518f0..05c71e0a0c 100644 --- a/app/test-crypto-perf/meson.build +++ b/app/test-crypto-perf/meson.build @@ -21,5 +21,5 @@ sources = files( ) deps += ['cryptodev', 'net', 'security'] if dpdk_conf.has('RTE_CRYPTO_SCHEDULER') -deps += 'crypto_scheduler' +optional_deps += 'crypto_scheduler' endif diff --git a/app/test-pmd/meson.build b/app/test-pmd/meson.build index f1c36529b4..1b54e479fe 100644 --- a/app/test-pmd/meson.build +++ b/app/test-pmd/meson.build @@ -37,44 +37,44 @@ endif deps += ['ethdev', 'cmdline'] if dpdk_conf.has('RTE_CRYPTO_SCHEDULER') -deps += 'crypto_scheduler' +optional_deps += 'crypto_scheduler' endif if dpdk_conf.has('RTE_LIB_BITRATESTATS') -deps += 'bitratestats' +optional_deps += 'bitratestats' endif if dpdk_conf.has('RTE_LIB_BPF') sources += files('bpf_cmd.c') -deps += 'bpf' +optional_deps += 'bpf' endif if dpdk_conf.has('RTE_LIB_GRO') -deps += 'gro' +optional_deps += 'gro' endif if dpdk_conf.has('RTE_LIB_GSO') -deps += 'gso' +optional_deps += 'gso' endif if dpdk_conf.has('RTE_LIB_LATENCYSTATS') -deps += 'latencystats' +optional_deps += 'latencystats' endif if dpdk_conf.has('RTE_LIB_METRICS') -deps += 'metrics' +optional_deps += 'metrics' endif if dpdk_conf.has('RTE_LIB_PDUMP') -deps += 'pdump' +optional_deps += 'pdump' endif if dpdk_conf.has('RTE_NET_BNXT') -deps += 'net_bnxt' +optional_deps += 'net_bnxt' endif if dpdk_conf.has('RTE_NET_I40E') -deps += 'net_i40e' +optional_deps += 'net_i40e' endif if dpdk_conf.has('RTE_NET_IXGBE') -deps += 'net_ixgbe' +optional_deps += 'net_ixgbe' endif if dpdk_conf.has('RTE_NET_DPAA') -deps += ['bus_dpaa', 'mempool_dpaa', 'net_dpaa'] +optional_deps += ['bus_dpaa', 'mempool_dpaa', 'net_dpaa'] endif # Driver-specific commands are located in driver directories. includes = include_directories('.') sources += testpmd_drivers_sources -deps += testpmd_drivers_deps +optional_deps += testpmd_drivers_deps diff --git a/app/test/meson.build b/app/test/meson.build index d5cb6a7f7a..9383b241a5 100644 --- a/app/test/meson.build +++ b/app/test/meson.build @@ -7,7 +7,7 @@ sources += files('commands.c', 'test.c') # option
[PATCH v4 2/8] build: output a dependency log in build directory
From: Bruce Richardson As meson processes our DPDK source tree it manages dependencies specified by each individual driver. To enable future analysis of the dependency links between components, log the dependencies of each DPDK component as it gets processed. This could potentially allow other tools to automatically enable or disable components based on the desired end components to be built, e.g. if the user requests net/ice, ensure that common/iavf is also enabled in the drivers. The output file produced is in "dot" or "graphviz" format, which allows producing a graphical representation of the DPDK dependency tree if so desired. For example: "dot -Tpng -O build/deps.dot" to produce the image file "build/deps.dot.png". Signed-off-by: Bruce Richardson Acked-by: Konstantin Ananyev Signed-off-by: Anatoly Burakov --- app/meson.build| 8 - buildtools/log-deps.py | 81 ++ buildtools/meson.build | 2 ++ drivers/meson.build| 6 examples/meson.build | 8 - lib/meson.build| 5 +++ 6 files changed, 108 insertions(+), 2 deletions(-) create mode 100644 buildtools/log-deps.py diff --git a/app/meson.build b/app/meson.build index 4e88e652ca..a770fed802 100644 --- a/app/meson.build +++ b/app/meson.build @@ -76,8 +76,14 @@ foreach app:apps reason = 'explicitly disabled via build config' endif +app_name = 'dpdk-' + name if build subdir(name) +type_cmd = '--type=app' +run_command([log_deps_cmd, type_cmd, app_name, deps], check: false) +if optional_deps.length() > 0 +run_command([log_deps_cmd, '--optional', type_cmd, app_name, optional_deps], check: false) +endif if not build and require_apps error('Cannot build explicitly requested app "@0@".\n'.format(name) + '\tReason: ' + reason) @@ -116,7 +122,7 @@ foreach app:apps link_libs = dpdk_static_libraries + dpdk_drivers endif -exec = executable('dpdk-' + name, +exec = executable(app_name, [ sources, resources ], c_args: cflags, link_args: ldflags, diff --git a/buildtools/log-deps.py b/buildtools/log-deps.py new file mode 100644 index 00..e11a102242 --- /dev/null +++ b/buildtools/log-deps.py @@ -0,0 +1,81 @@ +#! /usr/bin/env python3 +# SPDX-License-Identifier: BSD-3-Clause +# Copyright(c) 2024 Intel Corporation + +"""Utility script to build up a list of dependencies from meson.""" + +import os +import sys +import argparse +import typing as T + + +def file_to_list(filename: str) -> T.List[str]: +"""Read file into a list of strings.""" +with open(filename, encoding="utf-8") as f: +return f.readlines() + + +def list_to_file(filename: str, lines: T.List[str]): +"""Write a list of strings out to a file.""" +with open(filename, "w", encoding="utf-8") as f: +f.writelines(lines) + + +def gen_deps( +component_type: str, optional: bool, component: str, deps: T.List[str] +) -> str: +"""Generate a dependency graph for meson.""" +dep_list_str = '", "'.join(deps) +deps_str = "" if not deps else f' -> {{ "{dep_list_str}" }}' +# we define custom attributes for the nodes +attr_str = f'dpdk_componentType="{component_type}"' +if optional: +# we use a dotted line to represent optional dependencies +attr_str += ',style="dotted"' +return f'"{component}"{deps_str} [{attr_str}]\n' + + +def _main(): +depsfile = f'{os.environ["MESON_BUILD_ROOT"]}/deps.dot' + +# to reset the deps file on each build, the script is called without any params +if len(sys.argv) == 1: +os.remove(depsfile) +sys.exit(0) + +# we got arguments, parse them +parser = argparse.ArgumentParser( +description="Generate a dependency graph for meson." +) +# type is required +parser.add_argument( +"--type", required=True, help="Type of dependency (lib, examples, etc.)" +) +parser.add_argument( +"--optional", action="store_true", help="Whether the dependency is optional" +) +# component is required +parser.add_argument("component", help="The component to add to the graph") +parser.add_argument("deps", nargs="*", help="The dependencies of the component") + +parsed = parser.parse_args() + +try: +contents = file_to_list(depsfile) +except FileNotFoundError: +contents = ["digraph {\n", "}\n"] + +c_type = parsed.type +optional = parsed.optional +component = parsed.component +deps = parsed.deps +contents[-1] = gen_deps(c_type, optional, component, deps) + +contents.append("}\n") + +list_to_file(depsfile, contents) + + +if __name__ == "__main__": +_main() diff --git a/buildtools/meson.build b/buildtools/meson.build index 4e2c1217a2..13a0477245 100644 --- a/buildtools/meson.build +++ b/buildtools/meson.build @@ -26,6 +26,8 @@ header_gen_cmd
[PATCH v4 0/8] Record and rework component dependencies
As part of the meson build, we can record the dependencies for each component as we process it, logging them to a file. This file can be used as input to a number of other scripts and tools, for example, to graph the dependencies, or to allow higher-level build-config tools to automatically enable component requirements, etc. The first patch of this set separates dependencies inside meson into optional or mandatory. The second patch of this set generates the basic dependency tree. The third patch does some processing of that dependency tree to identify cases where dependencies are being unnecessarily specified. Reducing these makes it easier to have readable dependency graphs in future, without affecting the build. The following 4 patches are based on the output of the second patch, and greatly cut down the number of direct dependency links between components. Even with the cut-down dependencies, the full dependency graph is nigh-unreadable, so the final patch adds a new script to generate dependency tree subgraphs, creating dot files for e.g. the dependencies of a particular component, or a component class such as mempool drivers. v3 -> v4: - Update to latest main v2 -> v3: - Split dependencies into optional and mandatory - Fixup graph scripts to read and generate graphs that encode optional dependencies into the graph - Python version fixes to avoid using features not available in minimum supported Python version - Formatting with Ruff, and PEP-484 compliance Anatoly Burakov (1): build: split dependencies into mandatory and optional Bruce Richardson (7): build: output a dependency log in build directory devtools: add script to flag unneeded dependencies build: remove kvargs from driver class dependencies build: reduce library dependencies build: reduce driver dependencies build: reduce app dependencies devtools: add script to generate DPDK dependency graphs app/dumpcap/meson.build| 2 +- app/graph/meson.build | 2 +- app/meson.build| 11 +- app/pdump/meson.build | 2 +- app/proc-info/meson.build | 4 +- app/test-bbdev/meson.build | 8 +- app/test-crypto-perf/meson.build | 4 +- app/test-fib/meson.build | 2 +- app/test-pmd/meson.build | 26 +-- app/test-sad/meson.build | 2 +- app/test/meson.build | 14 +- buildtools/log-deps.py | 81 buildtools/meson.build | 2 + devtools/draw-dependency-graphs.py | 223 + devtools/find-duplicate-deps.py| 53 + drivers/baseband/fpga_5gnr_fec/meson.build | 2 +- drivers/baseband/fpga_lte_fec/meson.build | 2 +- drivers/baseband/la12xx/meson.build| 2 +- drivers/baseband/null/meson.build | 2 +- drivers/baseband/turbo_sw/meson.build | 2 +- drivers/bus/auxiliary/meson.build | 2 - drivers/bus/dpaa/meson.build | 2 +- drivers/bus/fslmc/meson.build | 2 +- drivers/bus/ifpga/meson.build | 2 +- drivers/bus/pci/meson.build| 4 +- drivers/bus/platform/meson.build | 1 - drivers/bus/uacce/meson.build | 2 - drivers/bus/vdev/meson.build | 2 - drivers/common/cnxk/meson.build| 4 +- drivers/common/cpt/meson.build | 2 +- drivers/common/idpf/meson.build| 2 +- drivers/common/mlx5/meson.build| 2 +- drivers/compress/mlx5/meson.build | 2 +- drivers/compress/nitrox/meson.build| 2 +- drivers/compress/octeontx/meson.build | 2 +- drivers/crypto/bcmfs/meson.build | 2 +- drivers/crypto/cnxk/meson.build| 2 +- drivers/crypto/dpaa_sec/meson.build| 2 +- drivers/crypto/ipsec_mb/meson.build| 2 +- drivers/crypto/mlx5/meson.build| 2 +- drivers/crypto/nitrox/meson.build | 2 +- drivers/dma/cnxk/meson.build | 2 +- drivers/dma/dpaa/meson.build | 2 +- drivers/dma/dpaa2/meson.build | 2 +- drivers/dma/odm/meson.build| 2 +- drivers/dma/skeleton/meson.build | 2 +- drivers/event/cnxk/meson.build | 2 +- drivers/event/dlb2/meson.build | 2 +- drivers/event/dpaa2/meson.build| 2 +- drivers/event/meson.build | 2 +- drivers/event/octeontx/meson.build | 3 +- drivers/event/sw/meson.build | 2 +- drivers/mempool/cnxk/meson.build | 2 +- drivers/mempool/dpaa/meson.build | 2 +- drivers/mempool/dpaa2/meson.build | 2 +- drivers/mempool/octeontx/meson.build | 2 +- drivers/meson.build| 9 +- drivers/net/cnxk/meson.build
[PATCH v4 5/8] build: reduce library dependencies
From: Bruce Richardson Rather than having each library depend up on EAL + any extra libs, we can take advantage of recursive dependency support in meson and just assign the dependencies of each directory directly, rather than appending to the array. For libraries which only depend upon EAL, keep that as a default, but for libraries which depend upon even a single extra lib, that EAL dependency is unnecessary. Going further, we can identify using the find_duplicate_deps.py script any unnecessary deps in each library's list, and remove them to slim the dependency tree down. Reducing number of dependencies means that meson takes less time processing and deduplicating the dependency tree for each component, and also shrinks the dependency graph for DPDK itself. Signed-off-by: Bruce Richardson --- lib/argparse/meson.build | 2 +- lib/bbdev/meson.build| 2 +- lib/bitratestats/meson.build | 2 +- lib/bpf/meson.build | 2 +- lib/cmdline/meson.build | 2 +- lib/compressdev/meson.build | 2 +- lib/cryptodev/meson.build| 2 +- lib/dispatcher/meson.build | 2 +- lib/distributor/meson.build | 2 +- lib/dmadev/meson.build | 2 -- lib/eal/meson.build | 5 + lib/efd/meson.build | 2 +- lib/ethdev/meson.build | 2 +- lib/eventdev/meson.build | 3 +-- lib/fib/meson.build | 4 +--- lib/gpudev/meson.build | 2 +- lib/graph/meson.build| 2 +- lib/gro/meson.build | 2 +- lib/gso/meson.build | 2 +- lib/hash/meson.build | 4 +--- lib/ip_frag/meson.build | 2 +- lib/ipsec/meson.build| 2 +- lib/kvargs/meson.build | 2 +- lib/latencystats/meson.build | 2 +- lib/lpm/meson.build | 3 +-- lib/mbuf/meson.build | 2 +- lib/member/meson.build | 2 +- lib/mempool/meson.build | 2 +- lib/metrics/meson.build | 2 +- lib/mldev/meson.build| 2 +- lib/net/meson.build | 2 +- lib/node/meson.build | 2 +- lib/pcapng/meson.build | 2 +- lib/pdcp/meson.build | 2 +- lib/pdump/meson.build| 2 +- lib/pipeline/meson.build | 2 +- lib/port/meson.build | 2 +- lib/power/meson.build| 3 +-- lib/rawdev/meson.build | 2 -- lib/rcu/meson.build | 2 +- lib/regexdev/meson.build | 2 +- lib/reorder/meson.build | 2 +- lib/rib/meson.build | 2 +- lib/ring/meson.build | 1 - lib/sched/meson.build| 2 +- lib/security/meson.build | 2 +- lib/table/meson.build| 2 +- lib/telemetry/meson.build| 2 +- lib/vhost/meson.build| 2 +- 49 files changed, 46 insertions(+), 61 deletions(-) diff --git a/lib/argparse/meson.build b/lib/argparse/meson.build index b6a08ca049..96abc8766f 100644 --- a/lib/argparse/meson.build +++ b/lib/argparse/meson.build @@ -4,4 +4,4 @@ sources = files('rte_argparse.c') headers = files('rte_argparse.h') -deps += ['log'] +deps = ['log'] diff --git a/lib/bbdev/meson.build b/lib/bbdev/meson.build index 07685e7578..2e68aa7873 100644 --- a/lib/bbdev/meson.build +++ b/lib/bbdev/meson.build @@ -11,4 +11,4 @@ sources = files('rte_bbdev.c') headers = files('rte_bbdev.h', 'rte_bbdev_pmd.h', 'rte_bbdev_op.h') -deps += ['mbuf'] +deps = ['mbuf'] diff --git a/lib/bitratestats/meson.build b/lib/bitratestats/meson.build index ede7e0a579..8defcd53bf 100644 --- a/lib/bitratestats/meson.build +++ b/lib/bitratestats/meson.build @@ -3,4 +3,4 @@ sources = files('rte_bitrate.c') headers = files('rte_bitrate.h') -deps += ['ethdev', 'metrics'] +deps = ['metrics'] diff --git a/lib/bpf/meson.build b/lib/bpf/meson.build index aa258a9061..82127bc657 100644 --- a/lib/bpf/meson.build +++ b/lib/bpf/meson.build @@ -31,7 +31,7 @@ headers = files('bpf_def.h', 'rte_bpf.h', 'rte_bpf_ethdev.h') -deps += ['mbuf', 'net', 'ethdev'] +deps = ['ethdev'] dep = dependency('libelf', required: false, method: 'pkg-config') if dep.found() diff --git a/lib/cmdline/meson.build b/lib/cmdline/meson.build index 63fb69100d..4451f3da29 100644 --- a/lib/cmdline/meson.build +++ b/lib/cmdline/meson.build @@ -31,4 +31,4 @@ else sources += files('cmdline_os_unix.c') endif -deps += ['net'] +deps = ['net'] diff --git a/lib/compressdev/meson.build b/lib/compressdev/meson.build index c80295dc0d..4b86955baf 100644 --- a/lib/compressdev/meson.build +++ b/lib/compressdev/meson.build @@ -16,4 +16,4 @@ driver_sdk_headers = files( 'rte_compressdev_pmd.h', 'rte_compressdev_internal.h', ) -deps += ['kvargs', 'mbuf'] +deps = ['mbuf'] diff --git a/lib/cryptodev/meson.build b/lib/cryptodev/meson.build index 4734acf321..74e42ac700 100644 --- a/lib/cryptodev/meson.build +++ b/lib/cryptodev/meson.build @@ -20,4 +20,4 @@ driver_sdk_headers += files( 'cryptodev_pmd.h', ) -deps += ['kvargs', 'mbuf', 'rcu', 'telemetry'] +deps = ['mbuf', 'rcu'] diff --git a/lib/dispatcher/meson.build b/lib/dispatcher/meson.build index ff
[PATCH v4 3/8] devtools: add script to flag unneeded dependencies
From: Bruce Richardson While not a serious problem, DPDK components often list more dependencies than are actually necessary to build, due to the use of recursive dependencies. In extreme cases, such as with core libraries, this can lead to longer configuration times due to meson having to deduplicate long lists of dependencies. Therefore we can add a script to identify when a component has got unnecessary dependencies listed. Signed-off-by: Bruce Richardson --- devtools/find-duplicate-deps.py | 53 + 1 file changed, 53 insertions(+) create mode 100755 devtools/find-duplicate-deps.py diff --git a/devtools/find-duplicate-deps.py b/devtools/find-duplicate-deps.py new file mode 100755 index 00..b1eacf21ce --- /dev/null +++ b/devtools/find-duplicate-deps.py @@ -0,0 +1,53 @@ +#! /usr/bin/env python3 +# SPDX-License-Identifier: BSD-3-Clause +# Copyright(c) 2024 Intel Corporation + +"""Identify any superfluous dependencies listed in DPDK deps graph.""" + +import sys + +all_deps = {} + + +class dep: +"""Holds a component and its dependencies.""" + +def __init__(self, name, dep_names): +"""Create and process a component and its deps.""" +self.name = name.strip('" ') +self.base_deps = [all_deps[dn.strip('" ')] for dn in dep_names] +self.recursive_deps = [] +for d in self.base_deps: +self.recursive_deps.extend(d.base_deps) +self.recursive_deps.extend(d.recursive_deps) +self.extra_deps = [] +for d in self.base_deps: +if d in self.recursive_deps: +self.extra_deps.append(d.name) +if self.extra_deps: +print(f'{self.name}: extra deps {self.extra_deps}') + +def dict_add(self, d): +"""Add this object to a dictionary by name.""" +d[self.name] = self + + +def main(argv): +"""Read the dependency tree from a dot file and process it.""" +if len(argv) != 2: +print(f'Usage: {argv[0]} /deps.dot', file=sys.stderr) +sys.exit(1) + +with open(argv[1]) as f: +for ln in f.readlines(): +ln = ln.strip() +if '->' in ln: +name, deps = ln.split('->') +deps = deps.strip(' {}') +dep(name, deps.split(',')).dict_add(all_deps) +elif ln.startswith('"') and ln.endswith('"'): +dep(ln.strip('"'), []).dict_add(all_deps) + + +if __name__ == '__main__': +main(sys.argv) -- 2.43.5
[PATCH v4 6/8] build: reduce driver dependencies
From: Bruce Richardson Remove any unnecessary dependencies from the driver dependency lists. This will give each driver a near-minimum set of required dependencies. Signed-off-by: Bruce Richardson --- drivers/baseband/fpga_5gnr_fec/meson.build | 2 +- drivers/baseband/fpga_lte_fec/meson.build | 2 +- drivers/baseband/la12xx/meson.build| 2 +- drivers/baseband/null/meson.build | 2 +- drivers/baseband/turbo_sw/meson.build | 2 +- drivers/bus/auxiliary/meson.build | 2 -- drivers/bus/dpaa/meson.build | 2 +- drivers/bus/fslmc/meson.build | 2 +- drivers/bus/ifpga/meson.build | 2 +- drivers/bus/pci/meson.build| 4 +--- drivers/bus/platform/meson.build | 1 - drivers/bus/uacce/meson.build | 2 -- drivers/bus/vdev/meson.build | 2 -- drivers/common/cnxk/meson.build| 4 ++-- drivers/common/cpt/meson.build | 2 +- drivers/common/idpf/meson.build| 2 +- drivers/common/mlx5/meson.build| 2 +- drivers/compress/mlx5/meson.build | 2 +- drivers/compress/nitrox/meson.build| 2 +- drivers/compress/octeontx/meson.build | 2 +- drivers/crypto/bcmfs/meson.build | 2 +- drivers/crypto/cnxk/meson.build| 2 +- drivers/crypto/dpaa_sec/meson.build| 2 +- drivers/crypto/ipsec_mb/meson.build| 2 +- drivers/crypto/mlx5/meson.build| 2 +- drivers/crypto/nitrox/meson.build | 2 +- drivers/dma/cnxk/meson.build | 2 +- drivers/dma/dpaa/meson.build | 2 +- drivers/dma/dpaa2/meson.build | 2 +- drivers/dma/odm/meson.build| 2 +- drivers/dma/skeleton/meson.build | 2 +- drivers/event/cnxk/meson.build | 2 +- drivers/event/dlb2/meson.build | 2 +- drivers/event/dpaa2/meson.build| 2 +- drivers/event/octeontx/meson.build | 3 +-- drivers/event/sw/meson.build | 2 +- drivers/mempool/cnxk/meson.build | 2 +- drivers/mempool/dpaa/meson.build | 2 +- drivers/mempool/dpaa2/meson.build | 2 +- drivers/mempool/octeontx/meson.build | 2 +- drivers/net/cnxk/meson.build | 4 +--- drivers/net/iavf/meson.build | 2 +- drivers/net/ice/meson.build| 2 +- drivers/net/mana/meson.build | 2 +- drivers/net/mlx5/meson.build | 2 +- drivers/net/sfc/meson.build| 2 +- drivers/net/softnic/meson.build| 2 +- drivers/raw/cnxk_bphy/meson.build | 2 +- drivers/raw/cnxk_gpio/meson.build | 2 +- drivers/raw/ntb/meson.build| 2 +- drivers/raw/skeleton/meson.build | 2 +- drivers/regex/mlx5/meson.build | 2 +- drivers/vdpa/ifc/meson.build | 2 +- drivers/vdpa/meson.build | 3 +-- drivers/vdpa/mlx5/meson.build | 2 +- drivers/vdpa/sfc/meson.build | 2 +- 56 files changed, 53 insertions(+), 66 deletions(-) diff --git a/drivers/baseband/fpga_5gnr_fec/meson.build b/drivers/baseband/fpga_5gnr_fec/meson.build index c3678d23eb..31b9e92fbb 100644 --- a/drivers/baseband/fpga_5gnr_fec/meson.build +++ b/drivers/baseband/fpga_5gnr_fec/meson.build @@ -1,7 +1,7 @@ # SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2020 Intel Corporation -deps += ['bus_vdev', 'ring', 'pci', 'bus_pci'] +deps += ['bus_vdev', 'bus_pci'] sources = files('rte_fpga_5gnr_fec.c') diff --git a/drivers/baseband/fpga_lte_fec/meson.build b/drivers/baseband/fpga_lte_fec/meson.build index 14e07826ef..fbf24755db 100644 --- a/drivers/baseband/fpga_lte_fec/meson.build +++ b/drivers/baseband/fpga_lte_fec/meson.build @@ -1,5 +1,5 @@ # SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2019 Intel Corporation -deps += ['bus_vdev', 'ring', 'pci', 'bus_pci'] +deps += ['bus_vdev', 'bus_pci'] sources = files('fpga_lte_fec.c') diff --git a/drivers/baseband/la12xx/meson.build b/drivers/baseband/la12xx/meson.build index 7b7e41c961..e1dbdd0fa7 100644 --- a/drivers/baseband/la12xx/meson.build +++ b/drivers/baseband/la12xx/meson.build @@ -1,6 +1,6 @@ # SPDX-License-Identifier: BSD-3-Clause # Copyright 2020-2021 NXP -deps += ['bus_vdev', 'ring'] +deps += ['bus_vdev'] sources = files('bbdev_la12xx.c') diff --git a/drivers/baseband/null/meson.build b/drivers/baseband/null/meson.build index 22863f0bd8..716d6c6fdb 100644 --- a/drivers/baseband/null/meson.build +++ b/drivers/baseband/null/meson.build @@ -1,5 +1,5 @@ # SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2018 Luca Boccassi -deps += ['bus_vdev', 'ring'] +deps += ['bus_vdev'] sources = files('bbdev_null.c') diff --git a/drivers/baseband/turbo_sw/meson.build b/drivers/baseband/turbo_sw/meson.build index a9035a753e..ac18e8a9b6 100644 --- a/drivers/baseband/turbo_sw/meson.build +++ b/drivers/baseband/turbo_sw/meson
[PATCH v4 7/8] build: reduce app dependencies
From: Bruce Richardson Remove any unnecessary dependencies from the app dependency lists. This will give each app a near-minimum set of required dependencies. Signed-off-by: Bruce Richardson --- app/dumpcap/meson.build | 2 +- app/graph/meson.build| 2 +- app/pdump/meson.build| 2 +- app/proc-info/meson.build| 2 +- app/test-crypto-perf/meson.build | 2 +- app/test-fib/meson.build | 2 +- app/test-sad/meson.build | 2 +- app/test/meson.build | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/dumpcap/meson.build b/app/dumpcap/meson.build index 69c016c780..6204cf051a 100644 --- a/app/dumpcap/meson.build +++ b/app/dumpcap/meson.build @@ -14,4 +14,4 @@ endif ext_deps += pcap_dep sources = files('main.c') -deps += ['ethdev', 'pdump', 'pcapng', 'bpf'] +deps += ['pdump'] diff --git a/app/graph/meson.build b/app/graph/meson.build index 344e4a418f..9c4cd080d9 100644 --- a/app/graph/meson.build +++ b/app/graph/meson.build @@ -9,7 +9,7 @@ if not build subdir_done() endif -deps += ['graph', 'eal', 'lpm', 'ethdev', 'node', 'cmdline', 'net'] +deps += ['node', 'cmdline'] sources = files( 'cli.c', 'conn.c', diff --git a/app/pdump/meson.build b/app/pdump/meson.build index fb282bba1f..a10f9d6124 100644 --- a/app/pdump/meson.build +++ b/app/pdump/meson.build @@ -8,4 +8,4 @@ if is_windows endif sources = files('main.c') -deps += ['ethdev', 'kvargs', 'pdump'] +deps += ['pdump'] diff --git a/app/proc-info/meson.build b/app/proc-info/meson.build index 156592119b..24b75b9ace 100644 --- a/app/proc-info/meson.build +++ b/app/proc-info/meson.build @@ -8,7 +8,7 @@ if is_windows endif sources = files('main.c') -deps += ['ethdev', 'security', 'eventdev'] +deps += ['security', 'eventdev'] if dpdk_conf.has('RTE_LIB_METRICS') optional_deps += 'metrics' endif diff --git a/app/test-crypto-perf/meson.build b/app/test-crypto-perf/meson.build index 05c71e0a0c..8904f8607d 100644 --- a/app/test-crypto-perf/meson.build +++ b/app/test-crypto-perf/meson.build @@ -19,7 +19,7 @@ sources = files( 'cperf_test_verify.c', 'main.c', ) -deps += ['cryptodev', 'net', 'security'] +deps += ['cryptodev'] if dpdk_conf.has('RTE_CRYPTO_SCHEDULER') optional_deps += 'crypto_scheduler' endif diff --git a/app/test-fib/meson.build b/app/test-fib/meson.build index eb36772cf3..25e2ea1a1d 100644 --- a/app/test-fib/meson.build +++ b/app/test-fib/meson.build @@ -8,4 +8,4 @@ if is_windows endif sources = files('main.c') -deps += ['fib', 'lpm', 'net'] +deps += ['fib', 'lpm'] diff --git a/app/test-sad/meson.build b/app/test-sad/meson.build index a50616a9c7..414e2a05cb 100644 --- a/app/test-sad/meson.build +++ b/app/test-sad/meson.build @@ -8,4 +8,4 @@ if is_windows endif sources = files('main.c') -deps += ['ipsec', 'net'] +deps += ['ipsec'] diff --git a/app/test/meson.build b/app/test/meson.build index 9383b241a5..c7c90f1267 100644 --- a/app/test/meson.build +++ b/app/test/meson.build @@ -2,7 +2,7 @@ # Copyright(c) 2017-2023 Intel Corporation # the main test files [test.c and commands.c] relies on these libraries -deps += ['cmdline', 'ring', 'mempool', 'mbuf'] +deps += ['cmdline'] sources += files('commands.c', 'test.c') # optional dependencies: some files may use these - and so we should link them in - -- 2.43.5
[PATCH v4 8/8] devtools: add script to generate DPDK dependency graphs
From: Bruce Richardson Rather than the single monolithic graph that would be output from the deps.dot file in a build directory, we can post-process that to generate simpler graphs for different tasks. This new "draw_dependency_graphs" script takes the "deps.dot" as input and generates an output file that has the nodes categorized, filtering them based off the requested node or category. For example, use "--match net/ice" to show the dependency tree from that driver, or "--match lib" to show just the library dependency tree. Signed-off-by: Bruce Richardson Signed-off-by: Anatoly Burakov --- devtools/draw-dependency-graphs.py | 223 + 1 file changed, 223 insertions(+) create mode 100755 devtools/draw-dependency-graphs.py diff --git a/devtools/draw-dependency-graphs.py b/devtools/draw-dependency-graphs.py new file mode 100755 index 00..4fb765498d --- /dev/null +++ b/devtools/draw-dependency-graphs.py @@ -0,0 +1,223 @@ +#! /usr/bin/env python3 +# SPDX-License-Identifier: BSD-3-Clause +# Copyright(c) 2024 Intel Corporation + +import argparse +import collections +import sys +import typing as T + +# typedef for dependency data types +Deps = T.Set[str] +DepData = T.Dict[str, T.Dict[str, T.Dict[bool, Deps]]] + + +def parse_dep_line(line: str) -> T.Tuple[str, Deps, str, bool]: +"""Parse digraph line into (component, {dependencies}, type, optional).""" +# extract attributes first +first, last = line.index("["), line.rindex("]") +edge_str, attr_str = line[:first], line[first + 1 : last] +# key=value, key=value, ... +attrs = { +key.strip('" '): value.strip('" ') +for attr_kv in attr_str.split(",") +for key, value in [attr_kv.strip().split("=", 1)] +} +# check if edge is defined as dotted line, meaning it's optional +optional = "dotted" in attrs.get("style", "") +try: +component_type = attrs["dpdk_componentType"] +except KeyError as _e: +raise ValueError(f"Error: missing component type: {line}") from _e + +# now, extract component name and any of its dependencies +deps: T.Set[str] = set() +try: +component, deps_str = edge_str.strip('" ').split("->", 1) +component = component.strip().strip('" ') +deps_str = deps_str.strip().strip("{}") +deps = {d.strip('" ') for d in deps_str.split(",")} +except ValueError as _e: +component = edge_str.strip('" ') + +return component, deps, component_type, optional + + +def gen_dep_line(component: str, deps: T.Set[str], optional: bool) -> str: +"""Generate a dependency line for a component.""" +# we use dotted line to represent optional components +attr_str = ' [style="dotted"]' if optional else "" +dep_list_str = '", "'.join(deps) +deps_str = "" if not deps else f' -> {{ "{dep_list_str}" }}' +return f'"{component}"{deps_str}{attr_str}\n' + + +def read_deps_list(lines: T.List[str]) -> DepData: +"""Read a list of dependency lines into a dictionary.""" +deps_data: T.Dict[str, T.Any] = {} +for ln in lines: +if ln.startswith("digraph") or ln == "}": +continue + +component, deps, component_type, optional = parse_dep_line(ln) + +# each component will have two sets of dependencies - required and optional +c_dict = deps_data.setdefault(component_type, {}).setdefault(component, {}) +c_dict[optional] = deps +return deps_data + + +def create_classified_graph(deps_data: DepData) -> T.Iterator[str]: +"""Create a graph of dependencies with components classified by type.""" +yield "digraph dpdk_dependencies {\n overlap=false\n model=subset\n" +for n, deps_t in enumerate(deps_data.items()): +component_type, component_dict = deps_t +yield f' subgraph cluster_{n} {{\nlabel = "{component_type}"\n' +for component, optional_d in component_dict.items(): +for optional, deps in optional_d.items(): +yield gen_dep_line(component, deps, optional) +yield " }\n" +yield "}\n" + + +def parse_match(line: str, dep_data: DepData) -> T.List[str]: +"""Extract list of components from a category string.""" +# if this is not a compound string, we have very few valid choices +if "/" not in line: +# is this a category? +if line in dep_data: +return list(dep_data[line].keys()) +# this isn't a category. maybe an app name? +maybe_app_name = f"dpdk-{line}" +if maybe_app_name in dep_data["app"]: +return [maybe_app_name] +if maybe_app_name in dep_data["examples"]: +return [maybe_app_name] +# this isn't an app name either, so just look for component with that name +for _, component_dict in dep_data.items(): +if line in component_dict: +return [line] +# nothing found still. one last try: maybe it's a driver? we have to be ca
[PATCH v4 4/8] build: remove kvargs from driver class dependencies
From: Bruce Richardson The kvargs library is used by EAL, and therefore is implicitly a dependency of every DPDK driver. Remove it from the minimum set of dependencies for each driver class as it's unnecessary to call it out there. Signed-off-by: Bruce Richardson --- drivers/event/meson.build | 2 +- drivers/net/meson.build | 2 +- drivers/regex/meson.build | 2 +- drivers/vdpa/meson.build | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/event/meson.build b/drivers/event/meson.build index d6706b57f7..2708833adf 100644 --- a/drivers/event/meson.build +++ b/drivers/event/meson.build @@ -19,4 +19,4 @@ if not (toolchain == 'gcc' and cc.version().version_compare('<4.8.6') and dpdk_conf.has('RTE_ARCH_ARM64')) drivers += 'octeontx' endif -std_deps = ['eventdev', 'kvargs'] +std_deps = ['eventdev'] diff --git a/drivers/net/meson.build b/drivers/net/meson.build index dafd637ba4..3b388ead24 100644 --- a/drivers/net/meson.build +++ b/drivers/net/meson.build @@ -65,6 +65,6 @@ drivers = [ 'vmxnet3', 'zxdh', ] -std_deps = ['ethdev', 'kvargs'] # 'ethdev' also pulls in mbuf, net, eal etc +std_deps = ['ethdev'] # 'ethdev' also pulls in mbuf, net, eal etc std_deps += ['bus_pci'] # very many PMDs depend on PCI, so make std std_deps += ['bus_vdev']# same with vdev bus diff --git a/drivers/regex/meson.build b/drivers/regex/meson.build index ff2a8fea89..10192e7c77 100644 --- a/drivers/regex/meson.build +++ b/drivers/regex/meson.build @@ -5,4 +5,4 @@ drivers = [ 'mlx5', 'cn9k', ] -std_deps = ['ethdev', 'kvargs', 'regexdev'] # 'ethdev' also pulls in mbuf, net, eal etc +std_deps = ['ethdev', 'regexdev'] # 'ethdev' also pulls in mbuf, net, eal etc diff --git a/drivers/vdpa/meson.build b/drivers/vdpa/meson.build index 896e8e0304..e01c277b9e 100644 --- a/drivers/vdpa/meson.build +++ b/drivers/vdpa/meson.build @@ -11,5 +11,5 @@ drivers = [ 'nfp', 'sfc', ] -std_deps = ['bus_pci', 'kvargs'] +std_deps = ['bus_pci'] std_deps += ['vhost'] -- 2.43.5
Re: [PATCH v1] net/i40e: updated latest recommended matching list
On Mon, Nov 25, 2024 at 10:45:53AM +0800, hailinx wrote: > Signed-off-by: hailinx > --- > doc/guides/nics/i40e.rst | 8 > 1 file changed, 8 insertions(+) > > diff --git a/doc/guides/nics/i40e.rst b/doc/guides/nics/i40e.rst > index ca6caa0cff..35585efc44 100644 > --- a/doc/guides/nics/i40e.rst > +++ b/doc/guides/nics/i40e.rst > @@ -104,6 +104,10 @@ For X710/XL710/XXV710, > +--+---+--+ > | DPDK version | Kernel driver version | Firmware version | > +==+===+==+ > + |24.11 | 2.26.8| 9.52 | > + +--+---+--+ > + |24.07 | 2.25.9| 9.50 | > + +--+---+--+ > |24.03 | 2.24.6| 9.40 | > +--+---+--+ > |23.11 | 2.23.17 | 9.30 | > @@ -171,6 +175,10 @@ For X722, > +--+---+--+ > | DPDK version | Kernel driver version | Firmware version | > +==+===+==+ > + |24.11 | 2.26.8| 6.50 | > + +--+---+--+ > + |24.07 | 2.25.9| 6.50 | > + +--+---+--+ > |24.03 | 2.24.6| 6.20 | > +--+---+--+ > |23.11 | 2.23.17 | 6.20 | By the way, these lists are getting rather long. For next release, when doing an update maybe trim the really old releases e.g. pre-21.11, which would be the oldest supported LTS, from the tables. /Bruce
Re: [PATCH] devtools: download scripts from linux if not found
Thomas Monjalon, Nov 26, 2024 at 16:36: The central question is to know how to get those GPL2 scripts accepted in DPDK. These are scripts, not linked to any DPDK related binary. So I think it is OK in terms of license contamination. We MUST preserve the original license headers in these imported scripts though. I could find this link that discusses the matter: https://wiki.tcl-lang.org/page/GPL+Scripts I am not a legal expert though :)
Re: [PATCH] net/ixgbe: add support for new device
On Tue, Nov 26, 2024 at 09:04:52AM +, Xu, HailinX wrote: > > -Original Message- > > From: Zhichao Zeng > > Sent: Tuesday, November 26, 2024 11:16 AM > > To: dev@dpdk.org > > Cc: Zeng, ZhichaoX ; Burakov, Anatoly > > ; Medvedkin, Vladimir > > > > Subject: [PATCH] net/ixgbe: add support for new device > > > > Add support for loopback_mode for new device. > > > > Signed-off-by: Zhichao Zeng > > --- > Tested-by: Hailin Xu Fixes: 316637762a5f ("net/ixgbe/base: enable E610 device") Applied to dpdk-next-net-intel Thanks, /Bruce
[PATCH v2 1/1] usertools/devbind: allow changing UID/GID for VFIO
Currently, when binding a device to VFIO, the UID/GID for the device will always stay as system default (`root`). Yet, when running DPDK as non-root user, one has to change the UID/GID of the device to match the user's UID/GID to use the device. This patch adds an option to `dpdk-devbind.py` to change the UID/GID of the device when binding it to VFIO. Signed-off-by: Anatoly Burakov --- Notes: v1 -> v2: - Replaced hard exit with an error printout usertools/dpdk-devbind.py | 41 +-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py index f2a2a9a12f..496d0e90e8 100755 --- a/usertools/dpdk-devbind.py +++ b/usertools/dpdk-devbind.py @@ -8,6 +8,8 @@ import subprocess import argparse import platform +import grp +import pwd from glob import glob from os.path import exists, basename @@ -108,6 +110,8 @@ status_flag = False force_flag = False noiommu_flag = False +vfio_uid = "" +vfio_gid = "" args = [] @@ -463,6 +467,22 @@ def bind_one(dev_id, driver, force): % (dev_id, filename, err)) +def own_one(dev_id, uid, gid): +"""Set the IOMMU group ownership for a device""" +# find IOMMU group for a particular device +iommu_grp_base_path = os.path.join("/sys/bus/pci/devices", dev_id, "iommu_group") +try: +iommu_grp = os.path.basename(os.readlink(iommu_grp_base_path)) +# we found IOMMU group, now find the device +dev_path = os.path.join("/dev/vfio", iommu_grp) +# set the ownership +_uid = pwd.getpwnam(uid).pw_uid if uid else -1 +_gid = grp.getgrnam(gid).gr_gid if gid else -1 +os.chown(dev_path, _uid, _gid) +except OSError as err: +print(f"Error: failed to read IOMMU group for {dev_id}: {err}") + + def unbind_all(dev_list, force=False): """Unbind method, takes a list of device locations""" @@ -512,7 +532,7 @@ def check_noiommu_mode(): print("Warning: enabling unsafe no IOMMU mode for VFIO drivers") -def bind_all(dev_list, driver, force=False): +def bind_all(dev_list, driver, uid, gid, force=False): """Bind method, takes a list of device locations""" global devices @@ -544,6 +564,9 @@ def bind_all(dev_list, driver, force=False): for d in dev_list: bind_one(d, driver, force) +# if we're binding to vfio-pci, set the IOMMU user/group ownership if one was specified +if driver == "vfio-pci" and (uid or gid): +own_one(d, uid, gid) # For kernels < 3.15 when binding devices to a generic driver # (i.e. one that doesn't have a PCI ID table) using new_id, some devices @@ -697,6 +720,8 @@ def parse_args(): global force_flag global noiommu_flag global args +global vfio_uid +global vfio_gid parser = argparse.ArgumentParser( description='Utility to bind and unbind devices from Linux kernel', @@ -746,6 +771,12 @@ def parse_args(): '--noiommu-mode', action='store_true', help="If IOMMU is not available, enable no IOMMU mode for VFIO drivers") +parser.add_argument( +"-U", "--uid", help="For VFIO, specify the UID to set IOMMU group ownership" +) +parser.add_argument( +"-G", "--gid", help="For VFIO, specify the GID to set IOMMU group ownership" +) parser.add_argument( '--force', action='store_true', @@ -778,6 +809,10 @@ def parse_args(): b_flag = opt.bind elif opt.unbind: b_flag = "none" +if opt.uid: +vfio_uid = opt.uid +if opt.gid: +vfio_gid = opt.gid args = opt.devices if not b_flag and not status_flag: @@ -805,11 +840,13 @@ def do_arg_actions(): global status_flag global force_flag global args +global vfio_uid +global vfio_gid if b_flag in ["none", "None"]: unbind_all(args, force_flag) elif b_flag is not None: -bind_all(args, b_flag, force_flag) +bind_all(args, b_flag, vfio_uid, vfio_gid, force_flag) if status_flag: if b_flag is not None: clear_data() -- 2.43.5
Re: [PATCH v1 1/1] net/ixgbe: fix PTP initialization for E610
On Tue, Nov 26, 2024 at 08:32:09AM +, Xu, HailinX wrote: > > -Original Message- > > From: Anatoly Burakov > > Sent: Friday, November 22, 2024 11:17 PM > > To: dev@dpdk.org; Medvedkin, Vladimir ; > > Kwapulinski, Piotr ; Carolyn Wyborny > > ; Jagielski, Jedrzej > > > > Subject: [PATCH v1 1/1] net/ixgbe: fix PTP initialization for E610 > > > > Current codepath does not have case labels for E610 when initializing PTP. > > Add them in relevant places. > > > > Fixes: 316637762a5f ("net/ixgbe/base: enable E610 device") > > > > Signed-off-by: Anatoly Burakov > > --- > Tested-by: Hailin Xu Applied to dpdk-next-net-intel Thanks, /Bruce
Re: [PATCH] devtools: download scripts from linux if not found
26/11/2024 16:16, Robin Jarry: > Thomas Monjalon, Nov 26, 2024 at 15:03: > > I would prefer we host our own fork of checkpatch.pl. > > There are too many irrelevant checks that we should drop. > > How about get_maintainer.pl? Should we host our copy as well? It may makes sense. The central question is to know how to get those GPL2 scripts accepted in DPDK.
Re: [PATCH] net/ixgbe: fix RSS redirection table configuration for E610
On Tue, Nov 26, 2024 at 08:50:47AM +, Xu, HailinX wrote: > > -Original Message- > > From: Yuan Wang > > Sent: Monday, November 25, 2024 5:36 PM > > To: Burakov, Anatoly ; Medvedkin, Vladimir > > > > Cc: dev@dpdk.org; Wang, YuanX > > Subject: [PATCH] net/ixgbe: fix RSS redirection table configuration for E610 > > > > Add labels to get the correct table size and register address for RETA. > > > > Fixes: 316637762a5f ("net/ixgbe/base: enable E610 device") > > > > Signed-off-by: Yuan Wang > > --- > Tested-by: Hailin Xu Applied to dpdk-next-net-intel. Thanks, /Bruce
Re: [PATCH v2 1/3] net/ixgbe: initialize PTP to system time
On Mon, Nov 25, 2024 at 11:33:24AM +, Anatoly Burakov wrote: > Currently, ixgbe driver initializes PTP timestamp to 0. This is different > from what kernel driver does (which initializes it to system time). > > Align the DPDK driver to kernel driver by setting PTP timestamp to system > time when enabling PTP. > > Note that ixgbe driver always uses zero-based timestamps for PTP, so we > would only ever update the internal timecounter and not the actual NIC > registers. > > Implementation note: in order to get access to clock_gettime on MinGW, we > have to use rte_os_shim.h header, which provides a wrapper around that > function. However, what it *also* provides is wrapper macros around various > other OS-related functions such as read(). Due to one of the mailbox ops > in base code being called "read", MinGW will misinterpret a call into > that op as an attempt to call read() the OS function, and produce a > compile error. This is being worked around by using parentheses around > access to the read op. > > Signed-off-by: Anatoly Burakov > --- Acked-by: Bruce Richardson
Re: [PATCH v2 2/3] net/i40e: initialize PTP to system time
On Mon, Nov 25, 2024 at 11:33:25AM +, Anatoly Burakov wrote: > Currently, i40e driver initializes PTP timestamp to 0. This is different > from what kernel driver does (which initializes it to system time). > > Align the DPDK driver to kernel driver by setting PTP timestamp to system > time when enabling PTP. > > Note that i40e driver always uses zero-based timestamps for PTP, so we > would only ever update the internal timecounter and not the actual NIC > registers. > > Signed-off-by: Anatoly Burakov > --- Acked-by: Bruce Richardson
Re: [PATCH v2 3/3] net/e1000: initialize PTP to system time
On Mon, Nov 25, 2024 at 11:33:26AM +, Anatoly Burakov wrote: > Currently, e1000 driver initializes PTP timestamp to 0. This is different > from what kernel driver does (which initializes it to system time). > > Align the DPDK driver to kernel driver by setting PTP timestamp to system > time when enabling PTP. > > Note that e1000 driver always uses zero-based timestamps for PTP, so we > would only ever update the internal timecounter and not the actual NIC > registers. > > Signed-off-by: Anatoly Burakov > --- Acked-by: Bruce Richardson
Re: [PATCH] pcapng: avoid potential unaligned data
21/11/2024 00:06, Stephen Hemminger: > The buffer used to construct headers (which contain 32 bit values) > was declared as uint8_t which can lead to unaligned access. > Change to declare buffer as uint32_t. > > Signed-off-by: Stephen Hemminger Fixes: dc2d6d20047e ("pcapng: avoid using alloca") Cc: sta...@dpdk.org Applied, thanks.
RE: [RFC] crypto/virtio: add vhost-vdpa backend
Hi, I wanted to follow up on my previous message regarding the development of a vhost vDPA host driver for crypto/virtio. We have proposed creating a driver/common/virtio/ directory to hold common implementations for both net and crypto functionalities. This approach aims to help in fixing common issues and extending virtio functionalities efficiently. As we plan to include this feature in DPDK 25.03, we would like to conclude on the implementation direction soon. We would greatly appreciate your valuable feedback or any suggestions on this proposal. Thank you! Thanks, Gowrishankar > Hi, > We are adding support for vDPA user backend for virtio-crypto PMD in DPDK. > We have come up with functional changes which is similar to the support > available in net: > > commit 6b901437056eed3ed7c9932c333ba24ac5be116f > > net/virtio: introduce vhost-vDPA backend > vhost-vDPA is a new virtio backend type introduced by vDPA kernel > framework, which provides abstraction to the vDPA devices and > exposes an unified control interface through a char dev. > > Our current development reuses some code from net/virtio/virtio_user/, and > we realize that we could keep a few things in common between net and > crypto, such as: > > -> vhost_vdpa.c (and its header file) from net/virtio/virtio_user/: >Except for VHOST_VDPA_GET_DEVICE_ID and enabling queue pairs, > virtio_user_backend_ops can be reused. > -> virtio_user_dev.c (and its header file) from net/virtio/virtio_user/: >virtio_user_dev_init and its capabilities differ. > -> virtio_cvq.c (and its header file) from net/virtio/: >There is a difference in the usage of the first and last descriptors for > the virtio > header and status (net vs. crypto). > > We need to standardize these codes to ensure they work universally. > Therefore, we propose creating a driver/common/virtio/ directory to house > them. This approach will help address common issues and extend Virtio > functionalities shared between crypto and net. For example, the crypto PMD > can benefit from packed ring support. We welcome your valuable feedback > and any suggestions. > > Thanks, > Gowrishankar > -- > 2.37.1
Re: [PATCH v3 1/3] ethdev: add description for KEEP CRC offload
On Tue, 26 Nov 2024 15:47:32 +0800 Jie Hai wrote: > Hi, Stephen Hemminger > > Thanks for your review. > I will add doc and fix on drivers in the next version. > The test will be done later. > > On 2024/11/23 1:10, Stephen Hemminger wrote: > > On Fri, 19 Jul 2024 17:04:13 +0800 > > Jie Hai wrote: > > > >> From: Dengdui Huang > >> > >> The data exceeds the pkt_len in mbuf is inavailable for user. > >unavailable > > > >> When KEEP CRC offload is enabled, CRC field length should be > >> included in the pkt_len in mbuf. However, almost of drivers > >> supported KEEP CRC feature didn't add the CRC data length to > >> pkt_len. So it is very necessary to add comments for this. > > > > All drivers must do the same thing, or this is a serious bug > > in the drivers. Just changing a comment is not going to be helpful. > > > > To fix this right: > > 1. Do a test with one of the original drivers in DPDK that has this > > feature. I would suggest ixgbe, mlx5 or bnxt. > > I can test it on ixgbe and mlx5. > > 2. Add a test to the PMD tests that validates this (if there is not > > one already). > > > Maybe later and not come with this patchset. > > 3. Put the documentation in a place where it shows up in user > > documentation. > > Either in doxygen comment or in doc/guides/nics > > > Will add in the next version. > > 4. Verify that all devices conform to the desired behavior > > > > I can help, but only have some old mlx5 cards to test here. > Thanks. > > Just putting comment in ethdev.h is not enough. > > > > Thanks, > Jie Hai > > > After more discussion, decided to not include this patch (only the two bugfixes). The proper place to put it is in the programmer's guide, and also you have discovered a messy place in the driver API. In 10 minutes, already found several issues in other drivers around this. Like checking wrong bits, dead code, etc.
[PATCH v6 25/30] lib/eal: replace packed attributes
MSVC struct packing is not compatible with GCC. Replace macro __rte_packed with __rte_packed_begin to push existing pack value and set packing to 1-byte and macro __rte_packed_end to restore the pack value prior to the push. Macro __rte_packed_end is deliberately utilized to trigger a MSVC compiler warning if no existing packing has been pushed allowing easy identification of locations where the __rte_packed_begin is missing. Removed the packed attributes from some structures that don't need them. Signed-off-by: Andre Muezerie --- lib/eal/common/eal_private.h | 2 +- lib/eal/include/rte_memory.h | 3 ++- lib/eal/include/rte_memzone.h | 3 ++- lib/eal/include/rte_trace_point.h | 2 +- lib/eal/x86/include/rte_memcpy.h | 9 ++--- 5 files changed, 12 insertions(+), 7 deletions(-) diff --git a/lib/eal/common/eal_private.h b/lib/eal/common/eal_private.h index bb315dab04..3b94e01b5b 100644 --- a/lib/eal/common/eal_private.h +++ b/lib/eal/common/eal_private.h @@ -62,7 +62,7 @@ struct rte_config { * DPDK instances */ struct rte_mem_config *mem_config; -} __rte_packed; +}; /** * Get the global configuration structure. diff --git a/lib/eal/include/rte_memory.h b/lib/eal/include/rte_memory.h index dbd0a6bedc..25fd7e0a6b 100644 --- a/lib/eal/include/rte_memory.h +++ b/lib/eal/include/rte_memory.h @@ -46,6 +46,7 @@ extern "C" { /** * Physical memory segment descriptor. */ +__rte_packed_begin struct rte_memseg { rte_iova_t iova;/**< Start IO address. */ union { @@ -58,7 +59,7 @@ struct rte_memseg { uint32_t nchannel; /**< Number of channels. */ uint32_t nrank; /**< Number of ranks. */ uint32_t flags; /**< Memseg-specific flags */ -} __rte_packed; +} __rte_packed_end; /** * memseg list is a special case as we need to store a bunch of other data diff --git a/lib/eal/include/rte_memzone.h b/lib/eal/include/rte_memzone.h index e1563994d5..28c2262e12 100644 --- a/lib/eal/include/rte_memzone.h +++ b/lib/eal/include/rte_memzone.h @@ -45,6 +45,7 @@ extern "C" { * A structure describing a memzone, which is a contiguous portion of * physical memory identified by a name. */ +__rte_packed_begin struct rte_memzone { #define RTE_MEMZONE_NAMESIZE 32 /**< Maximum length of memory zone name.*/ @@ -62,7 +63,7 @@ struct rte_memzone { int32_t socket_id;/**< NUMA socket ID. */ uint32_t flags; /**< Characteristics of this memzone. */ -} __rte_packed; +} __rte_packed_end; /** * Set the maximum number of memzones. diff --git a/lib/eal/include/rte_trace_point.h b/lib/eal/include/rte_trace_point.h index 87b5f43c3c..b24db3b6da 100644 --- a/lib/eal/include/rte_trace_point.h +++ b/lib/eal/include/rte_trace_point.h @@ -298,7 +298,7 @@ struct __rte_trace_stream_header { rte_uuid_t uuid; uint32_t lcore_id; char thread_name[__RTE_TRACE_EMIT_STRING_LEN_MAX]; -} __rte_packed; +}; struct __rte_trace_header { uint32_t offset; diff --git a/lib/eal/x86/include/rte_memcpy.h b/lib/eal/x86/include/rte_memcpy.h index 42058e4a3f..58368145c4 100644 --- a/lib/eal/x86/include/rte_memcpy.h +++ b/lib/eal/x86/include/rte_memcpy.h @@ -67,15 +67,18 @@ rte_mov15_or_less(void *dst, const void *src, size_t n) * Use the following structs to avoid violating C standard * alignment requirements and to avoid strict aliasing bugs */ + __rte_packed_begin struct rte_uint64_alias { uint64_t val; - } __rte_packed __rte_may_alias; + } __rte_packed_end __rte_may_alias; + __rte_packed_begin struct rte_uint32_alias { uint32_t val; - } __rte_packed __rte_may_alias; + } __rte_packed_end __rte_may_alias; + __rte_packed_begin struct rte_uint16_alias { uint16_t val; - } __rte_packed __rte_may_alias; + } __rte_packed_end __rte_may_alias; void *ret = dst; if (n & 8) { -- 2.34.1
[PATCH v6 10/30] drivers/crypto: replace packed attributes
MSVC struct packing is not compatible with GCC. Replace macro __rte_packed with __rte_packed_begin to push existing pack value and set packing to 1-byte and macro __rte_packed_end to restore the pack value prior to the push. Macro __rte_packed_end is deliberately utilized to trigger a MSVC compiler warning if no existing packing has been pushed allowing easy identification of locations where the __rte_packed_begin is missing. Signed-off-by: Andre Muezerie --- drivers/crypto/caam_jr/caam_jr.c | 4 +- drivers/crypto/caam_jr/caam_jr_desc.h| 64 ++-- drivers/crypto/caam_jr/caam_jr_hw_specific.h | 48 +++ drivers/crypto/dpaa_sec/dpaa_sec.h | 12 ++-- drivers/crypto/ionic/ionic_crypto_if.h | 36 +-- drivers/crypto/mlx5/mlx5_crypto.h| 6 +- drivers/crypto/mlx5/mlx5_crypto_gcm.c| 3 +- drivers/crypto/qat/qat_sym.h | 7 ++- drivers/crypto/qat/qat_sym_session.h | 4 +- 9 files changed, 94 insertions(+), 90 deletions(-) diff --git a/drivers/crypto/caam_jr/caam_jr.c b/drivers/crypto/caam_jr/caam_jr.c index 4082b3f422..fedeefdf80 100644 --- a/drivers/crypto/caam_jr/caam_jr.c +++ b/drivers/crypto/caam_jr/caam_jr.c @@ -53,10 +53,10 @@ static enum sec_driver_state_e g_driver_state = SEC_DRIVER_STATE_IDLE; static int g_job_rings_no; static int g_job_rings_max; -struct sec_outring_entry { +__rte_packed_begin struct sec_outring_entry { phys_addr_t desc; /* Pointer to completed descriptor */ uint32_t status;/* Status for completed descriptor */ -} __rte_packed; +} __rte_packed_end; /* virtual address conversin when mempool support is available for ctx */ static inline phys_addr_t diff --git a/drivers/crypto/caam_jr/caam_jr_desc.h b/drivers/crypto/caam_jr/caam_jr_desc.h index a4507613be..76cc07a94b 100644 --- a/drivers/crypto/caam_jr/caam_jr_desc.h +++ b/drivers/crypto/caam_jr/caam_jr_desc.h @@ -123,8 +123,8 @@ } /* Union describing a descriptor header. */ -struct descriptor_header_s { - union { +__rte_packed_begin struct descriptor_header_s { + __rte_packed_begin union { uint32_t word; struct { /* 4 */ unsigned int ctype:5; @@ -162,15 +162,15 @@ struct descriptor_header_s { /* 26 */ unsigned int res1:1; /* 27 */ unsigned int ctype:5; } jd; - } __rte_packed command; -} __rte_packed; + } __rte_packed_end command; +} __rte_packed_end; /* Union describing a KEY command in a descriptor. */ -struct key_command_s { - union { +__rte_packed_begin struct key_command_s { + __rte_packed_begin union { uint32_t word; - struct { + __rte_packed_begin struct { unsigned int ctype:5; unsigned int cls:2; unsigned int sgf:1; @@ -182,30 +182,30 @@ struct key_command_s { unsigned int tk:1; unsigned int rsvd1:5; unsigned int length:10; - } __rte_packed field; - } __rte_packed command; -} __rte_packed; + } __rte_packed_end field; + } __rte_packed_end command; +} __rte_packed_end; /* Union describing a PROTOCOL command * in a descriptor. */ -struct protocol_operation_command_s { - union { +__rte_packed_begin struct protocol_operation_command_s { + __rte_packed_begin union { uint32_t word; - struct { + __rte_packed_begin struct { unsigned int ctype:5; unsigned int optype:3; unsigned char protid; unsigned short protinfo; - } __rte_packed field; - } __rte_packed command; -} __rte_packed; + } __rte_packed_end field; + } __rte_packed_end command; +} __rte_packed_end; /* Union describing a SEQIN command in a * descriptor. */ -struct seq_in_command_s { - union { +__rte_packed_begin struct seq_in_command_s { + __rte_packed_begin union { uint32_t word; struct { unsigned int ctype:5; @@ -219,14 +219,14 @@ struct seq_in_command_s { unsigned int res2:4; unsigned int length:16; } field; - } __rte_packed command; -} __rte_packed; + } __rte_packed_end command; +} __rte_packed_end; /* Union describing a SEQOUT command in a * descriptor. */ -struct seq_out_command_s { - union { +__rte_packed_begin struct seq_out_command_s { + __rte_packed_begin union { uint32_t word; struct { unsigned int ctype:5; @@ -238,11 +238,11 @@ struct seq_out_command_s { unsigned int res2:5;
[PATCH v6 13/30] drivers/mempool: replace packed attributes
MSVC struct packing is not compatible with GCC. Replace macro __rte_packed with __rte_packed_begin to push existing pack value and set packing to 1-byte and macro __rte_packed_end to restore the pack value prior to the push. Macro __rte_packed_end is deliberately utilized to trigger a MSVC compiler warning if no existing packing has been pushed allowing easy identification of locations where the __rte_packed_begin is missing. Signed-off-by: Andre Muezerie --- drivers/mempool/octeontx/octeontx_fpavf.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/mempool/octeontx/octeontx_fpavf.c b/drivers/mempool/octeontx/octeontx_fpavf.c index 966fee8bfe..d2eb837c17 100644 --- a/drivers/mempool/octeontx/octeontx_fpavf.c +++ b/drivers/mempool/octeontx/octeontx_fpavf.c @@ -46,20 +46,20 @@ struct octeontx_mbox_fpa_cfg { uint64_taura_cfg; }; -struct __rte_packed gen_req { +__rte_packed_begin struct gen_req { uint32_tvalue; -}; +} __rte_packed_end; -struct __rte_packed idn_req { +__rte_packed_begin struct idn_req { uint8_t domain_id; -}; +} __rte_packed_end; -struct __rte_packed gen_resp { +__rte_packed_begin struct gen_resp { uint16_tdomain_id; uint16_tvfid; -}; +} __rte_packed_end; -struct __rte_packed dcfg_resp { +__rte_packed_begin struct dcfg_resp { uint8_t sso_count; uint8_t ssow_count; uint8_t fpa_count; @@ -67,7 +67,7 @@ struct __rte_packed dcfg_resp { uint8_t tim_count; uint8_t net_port_count; uint8_t virt_port_count; -}; +} __rte_packed_end; #define FPA_MAX_POOL 32 #define FPA_PF_PAGE_SZ 4096 -- 2.34.1
[PATCH v6 19/30] examples/ip-pipeline: remove packed attributes
Removed packed attributes from structs that are naturally packed already, or don't require packing. Signed-off-by: Andre Muezerie --- examples/ip_pipeline/cli.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/examples/ip_pipeline/cli.c b/examples/ip_pipeline/cli.c index 92dfacdeb0..766fc8e46e 100644 --- a/examples/ip_pipeline/cli.c +++ b/examples/ip_pipeline/cli.c @@ -2571,7 +2571,7 @@ struct pkt_key_qinq { uint16_t svlan; uint16_t ethertype_cvlan; uint16_t cvlan; -} __rte_packed; +}; struct pkt_key_ipv4_5tuple { uint8_t time_to_live; @@ -2581,7 +2581,7 @@ struct pkt_key_ipv4_5tuple { uint32_t da; uint16_t sp; uint16_t dp; -} __rte_packed; +}; struct pkt_key_ipv6_5tuple { uint16_t payload_length; @@ -2591,15 +2591,15 @@ struct pkt_key_ipv6_5tuple { struct rte_ipv6_addr da; uint16_t sp; uint16_t dp; -} __rte_packed; +}; struct pkt_key_ipv4_addr { uint32_t addr; -} __rte_packed; +}; struct pkt_key_ipv6_addr { struct rte_ipv6_addr addr; -} __rte_packed; +}; static uint32_t parse_match(char **tokens, -- 2.34.1
[PATCH v6 15/30] drivers/raw: replace packed attributes
MSVC struct packing is not compatible with GCC. Replace macro __rte_packed with __rte_packed_begin to push existing pack value and set packing to 1-byte and macro __rte_packed_end to restore the pack value prior to the push. Macro __rte_packed_end is deliberately utilized to trigger a MSVC compiler warning if no existing packing has been pushed allowing easy identification of locations where the __rte_packed_begin is missing. Signed-off-by: Andre Muezerie --- drivers/raw/ifpga/afu_pmd_n3000.h| 8 drivers/raw/ifpga/base/opae_hw_api.h | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/raw/ifpga/afu_pmd_n3000.h b/drivers/raw/ifpga/afu_pmd_n3000.h index f6b6e07c6b..a74025cb3c 100644 --- a/drivers/raw/ifpga/afu_pmd_n3000.h +++ b/drivers/raw/ifpga/afu_pmd_n3000.h @@ -211,7 +211,7 @@ typedef union { }; } msgdma_desc_ctrl; -typedef struct __rte_packed { +typedef __rte_packed_begin struct { uint32_t rd_address; uint32_t wr_address; uint32_t len; @@ -223,7 +223,7 @@ typedef struct __rte_packed { uint32_t rd_address_ext; uint32_t wr_address_ext; msgdma_desc_ctrl control; -} msgdma_ext_desc; +} __rte_packed_end msgdma_ext_desc; typedef union { uint32_t csr; @@ -279,13 +279,13 @@ typedef union { }; } msgdma_seq_num; -typedef struct __rte_packed { +typedef __rte_packed_begin struct { msgdma_status status; msgdma_ctrl ctrl; msgdma_fill_level fill_level; msgdma_rsp_level rsp; msgdma_seq_num seq_num; -} msgdma_csr; +} __rte_packed_end msgdma_csr; #define CSR_STATUS(csr) (&(((msgdma_csr *)(csr))->status)) #define CSR_CONTROL(csr) (&(((msgdma_csr *)(csr))->ctrl)) diff --git a/drivers/raw/ifpga/base/opae_hw_api.h b/drivers/raw/ifpga/base/opae_hw_api.h index 613563a0b4..5957f67417 100644 --- a/drivers/raw/ifpga/base/opae_hw_api.h +++ b/drivers/raw/ifpga/base/opae_hw_api.h @@ -343,9 +343,9 @@ static inline void opae_adapter_remove_acc(struct opae_adapter *adapter, /* OPAE vBNG network datastruct */ #define OPAE_ETHER_ADDR_LEN 6 -struct opae_ether_addr { +__rte_packed_begin struct opae_ether_addr { unsigned char addr_bytes[OPAE_ETHER_ADDR_LEN]; -} __rte_packed; +} __rte_packed_end; /* OPAE vBNG network API*/ int opae_manager_read_mac_rom(struct opae_manager *mgr, int port, -- 2.34.1
[PATCH v6 12/30] drivers/event: replace packed attributes
MSVC struct packing is not compatible with GCC. Replace macro __rte_packed with __rte_packed_begin to push existing pack value and set packing to 1-byte and macro __rte_packed_end to restore the pack value prior to the push. Macro __rte_packed_end is deliberately utilized to trigger a MSVC compiler warning if no existing packing has been pushed allowing easy identification of locations where the __rte_packed_begin is missing. Signed-off-by: Andre Muezerie --- drivers/event/octeontx/timvf_evdev.c | 4 ++-- drivers/event/octeontx/timvf_evdev.h | 12 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/event/octeontx/timvf_evdev.c b/drivers/event/octeontx/timvf_evdev.c index 82f17144a6..9339b13941 100644 --- a/drivers/event/octeontx/timvf_evdev.c +++ b/drivers/event/octeontx/timvf_evdev.c @@ -9,10 +9,10 @@ RTE_LOG_REGISTER_SUFFIX(otx_logtype_timvf, timer, NOTICE); static struct rte_eventdev *event_dev; -struct __rte_packed timvf_mbox_dev_info { +__rte_packed_begin struct timvf_mbox_dev_info { uint64_t ring_active[4]; uint64_t clk_freq; -}; +} __rte_packed_end; /* Response messages */ enum { diff --git a/drivers/event/octeontx/timvf_evdev.h b/drivers/event/octeontx/timvf_evdev.h index 44a4ee41c4..0dd3734773 100644 --- a/drivers/event/octeontx/timvf_evdev.h +++ b/drivers/event/octeontx/timvf_evdev.h @@ -123,7 +123,7 @@ enum timvf_clk_src { }; /* TIM_MEM_BUCKET */ -struct __rte_aligned(8) tim_mem_bucket { +__rte_packed_begin struct __rte_aligned(8) tim_mem_bucket { uint64_t first_chunk; union { RTE_ATOMIC(uint64_t) w1; @@ -139,19 +139,19 @@ struct __rte_aligned(8) tim_mem_bucket { }; uint64_t current_chunk; uint64_t pad; -} __rte_packed; +} __rte_packed_end; -struct tim_mem_entry { +__rte_packed_begin struct tim_mem_entry { uint64_t w0; uint64_t wqe; -} __rte_packed; +} __rte_packed_end; -struct timvf_ctrl_reg { +__rte_packed_begin struct timvf_ctrl_reg { uint64_t rctrl0; uint64_t rctrl1; uint64_t rctrl2; uint8_t use_pmu; -} __rte_packed; +} __rte_packed_end; struct timvf_ring; -- 2.34.1
[PATCH v6 05/30] doc/guides: replace packed attributes
MSVC struct packing is not compatible with GCC. Replace macro __rte_packed with __rte_packed_begin to push existing pack value and set packing to 1-byte and macro __rte_packed_end to restore the pack value prior to the push. Macro __rte_packed_end is deliberately utilized to trigger a MSVC compiler warning if no existing packing has been pushed allowing easy identification of locations where the __rte_packed_begin is missing. Signed-off-by: Andre Muezerie --- doc/guides/nics/ark.rst | 3 ++- doc/guides/prog_guide/packet_classif_access_ctrl.rst | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/doc/guides/nics/ark.rst b/doc/guides/nics/ark.rst index a9f6d4cdb8..0cb5460199 100644 --- a/doc/guides/nics/ark.rst +++ b/doc/guides/nics/ark.rst @@ -172,10 +172,11 @@ during RX from user meta data coming from FPGA hardware. }; /* RX tuser field based on user's hardware */ + __rte_packed_begin struct user_rx_meta { uint64_t timestamp; uint32_t rss; - } __rte_packed; + } __rte_packed_end; /* Create ark_user_extension object for use in other hook functions */ void *rte_pmd_ark_dev_init(struct rte_eth_dev * dev, diff --git a/doc/guides/prog_guide/packet_classif_access_ctrl.rst b/doc/guides/prog_guide/packet_classif_access_ctrl.rst index c8844d0616..55db983a05 100644 --- a/doc/guides/prog_guide/packet_classif_access_ctrl.rst +++ b/doc/guides/prog_guide/packet_classif_access_ctrl.rst @@ -154,6 +154,7 @@ To define classification for the IPv6 2-tuple: o .. code-block:: c +__rte_packed_begin struct rte_ipv6_hdr { uint32_t vtc_flow; /* IP version, traffic class & flow label. */ uint16_t payload_len; /* IP packet length - includes sizeof(ip_header). */ @@ -161,7 +162,7 @@ To define classification for the IPv6 2-tuple: o uint8_t hop_limits;/* Hop limits. */ uint8_t src_addr[16]; /* IP address of source host. */ uint8_t dst_addr[16]; /* IP address of destination host(s). */ -} __rte_packed; +} __rte_packed_end; The following array of field definitions can be used: -- 2.34.1
[PATCH v6 18/30] examples/common: replace packed attributes
MSVC struct packing is not compatible with GCC. Replace macro __rte_packed with __rte_packed_begin to push existing pack value and set packing to 1-byte and macro __rte_packed_end to restore the pack value prior to the push. Macro __rte_packed_end is deliberately utilized to trigger a MSVC compiler warning if no existing packing has been pushed allowing easy identification of locations where the __rte_packed_begin is missing. Added parenthesis around *pnum in line __ rte_packed_end (*pnum) = (void *)pn; to avoid patch check error below: ERROR:SPACING: need consistent spacing around '*' (ctx:WxV) 34: FILE: examples/common/neon/port_group.h:27: + } __ rte_packed_end *pnum = (void *)pn; With the parenthesis it becomes a warning, still not ideal, but better Running checkpatch.pl: WARNING:SPACING: space prohibited between function name and open parenthesis '(' 34: FILE: examples/common/neon/port_group.h:27: + } __ rte_packed_end (*pnum) = (void *)pn; Signed-off-by: Andre Muezerie --- examples/common/neon/port_group.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/common/neon/port_group.h b/examples/common/neon/port_group.h index 421e2e8613..0c8ea4c085 100644 --- a/examples/common/neon/port_group.h +++ b/examples/common/neon/port_group.h @@ -21,10 +21,10 @@ static inline uint16_t * port_groupx4(uint16_t pn[FWDSTEP + 1], uint16_t *lp, uint16x8_t dp1, uint16x8_t dp2) { - union { + __rte_packed_begin union { uint16_t u16[FWDSTEP + 1]; uint64_t u64; - } __rte_packed *pnum = (void *)pn; + } __rte_packed_end (*pnum) = (void *)pn; uint16x8_t mask = {1, 2, 4, 8, 0, 0, 0, 0}; int32_t v; -- 2.34.1
[PATCH v6 22/30] examples/l3fwd: replace packed attributes
MSVC struct packing is not compatible with GCC. Replace macro __rte_packed with __rte_packed_begin to push existing pack value and set packing to 1-byte and macro __rte_packed_end to restore the pack value prior to the push. Macro __rte_packed_end is deliberately utilized to trigger a MSVC compiler warning if no existing packing has been pushed allowing easy identification of locations where the __rte_packed_begin is missing. Signed-off-by: Andre Muezerie --- examples/l3fwd/l3fwd_route.h | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/examples/l3fwd/l3fwd_route.h b/examples/l3fwd/l3fwd_route.h index 62263c3540..5303e7ce6f 100644 --- a/examples/l3fwd/l3fwd_route.h +++ b/examples/l3fwd/l3fwd_route.h @@ -36,21 +36,23 @@ struct ipv6_l3fwd_route { uint8_t if_out; }; +__rte_packed_begin struct ipv4_5tuple { uint32_t ip_dst; uint32_t ip_src; uint16_t port_dst; uint16_t port_src; uint8_t proto; -} __rte_packed; +} __rte_packed_end; +__rte_packed_begin struct ipv6_5tuple { uint8_t ip_dst[IPV6_ADDR_LEN]; uint8_t ip_src[IPV6_ADDR_LEN]; uint16_t port_dst; uint16_t port_src; uint8_t proto; -} __rte_packed; +} __rte_packed_end; struct lpm_route_rule { union { -- 2.34.1
[PATCH v6 24/30] examples/vhost_blk: replace packed attributes
MSVC struct packing is not compatible with GCC. Replace macro __rte_packed with __rte_packed_begin to push existing pack value and set packing to 1-byte and macro __rte_packed_end to restore the pack value prior to the push. Macro __rte_packed_end is deliberately utilized to trigger a MSVC compiler warning if no existing packing has been pushed allowing easy identification of locations where the __rte_packed_begin is missing. Signed-off-by: Andre Muezerie --- examples/vhost_blk/blk_spec.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/examples/vhost_blk/blk_spec.h b/examples/vhost_blk/blk_spec.h index 3c54f70eaf..6c6a36d471 100644 --- a/examples/vhost_blk/blk_spec.h +++ b/examples/vhost_blk/blk_spec.h @@ -73,6 +73,7 @@ struct vhost_memory_padded { struct vhost_memory_region regions[VHOST_USER_MEMORY_MAX_NREGIONS]; }; +__rte_packed_begin struct vhost_user_msg { enum vhost_user_request request; @@ -89,6 +90,6 @@ struct vhost_user_msg { struct vhost_memory_padded memory; struct vhost_user_config cfg; } payload; -} __rte_packed; +} __rte_packed_end; #endif -- 2.34.1
[PATCH v6 17/30] drivers/vdpa: replace packed attributes
MSVC struct packing is not compatible with GCC. Replace macro __rte_packed with __rte_packed_begin to push existing pack value and set packing to 1-byte and macro __rte_packed_end to restore the pack value prior to the push. Macro __rte_packed_end is deliberately utilized to trigger a MSVC compiler warning if no existing packing has been pushed allowing easy identification of locations where the __rte_packed_begin is missing. Signed-off-by: Andre Muezerie --- drivers/vdpa/ifc/base/ifcvf.h | 4 ++-- drivers/vdpa/mlx5/mlx5_vdpa.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/vdpa/ifc/base/ifcvf.h b/drivers/vdpa/ifc/base/ifcvf.h index 04c81c8196..7be1596556 100644 --- a/drivers/vdpa/ifc/base/ifcvf.h +++ b/drivers/vdpa/ifc/base/ifcvf.h @@ -115,11 +115,11 @@ struct ifcvf_pci_common_cfg { u32 queue_used_hi; }; -struct ifcvf_net_config { +__rte_packed_begin struct ifcvf_net_config { u8mac[6]; u16 status; u16 max_virtqueue_pairs; -} __rte_packed; +} __rte_packed_end; struct ifcvf_pci_mem_resource { u64 phys_addr; /**< Physical address, 0 if not resource. */ diff --git a/drivers/vdpa/mlx5/mlx5_vdpa.h b/drivers/vdpa/mlx5/mlx5_vdpa.h index e156520172..6f25eea641 100644 --- a/drivers/vdpa/mlx5/mlx5_vdpa.h +++ b/drivers/vdpa/mlx5/mlx5_vdpa.h @@ -90,13 +90,13 @@ enum mlx5_vdpa_task_type { }; /* Generic task information and size must be multiple of 4B. */ -struct __rte_aligned(4) mlx5_vdpa_task { +__rte_packed_begin struct __rte_aligned(4) mlx5_vdpa_task { struct mlx5_vdpa_priv *priv; enum mlx5_vdpa_task_type type; RTE_ATOMIC(uint32_t) *remaining_cnt; RTE_ATOMIC(uint32_t) *err_cnt; uint32_t idx; -} __rte_packed; +} __rte_packed_end; /* Generic mlx5_vdpa_c_thread information. */ struct mlx5_vdpa_c_thread { -- 2.34.1
release candidate 24.11-rc4
A new DPDK release candidate is ready for testing: https://git.dpdk.org/dpdk/tag/?id=v24.11-rc4 There are 47 new patches in this snapshot, mostly fixes and doc. Release notes: https://doc.dpdk.org/guides/rel_notes/release_24_11.html As usual, you can report any issue on https://bugs.dpdk.org You may share some release validation results by replying to this message at dev@dpdk.org and by adding tested hardware in the release notes. The final release will happen in few days, before the end of the month. Please think about sharing your roadmap now for DPDK 25.03. Thank you everyone
[PATCH v6 00/30] fix packing of structs when building with MSVC
MSVC struct packing is not compatible with GCC. Provide a macro (__rte_packed_begin) that can be used to push existing pack value and sets packing to 1-byte. The existing __rte_packed macro is replaced with __rte_packed_end and restores the pack value prior to the push. Instead of providing macros exclusively for MSVC and for GCC, macro __rte_packed_end is deliberately utilized to trigger a MSVC compiler warning if no existing packing has been pushed allowing easy identification of locations where the __rte_packed_begin is missing. Macro __rte_packed is removed and the two new macros represent the new way to enable packing in the DPDK code. Script checkpatches.sh was enhanced to ensure __rte_packed_begin and __rte_packed_end show up in pairs when checking patches. If as a part of review maintainers identify structs they believe don't require packing so long as they are explicitly identified I'll remove the __rte_packed as a part of this series. v6: * replace __rte_msvc_pack with __rte_packed_begin * replace __rte_packed with __rte_packed_end * update checkpatches.sh to ensure __rte_packed_begin and __rte_packed_end are used in pairs * remove __rte_packed v5: * rebase on top of latest main v4: * add another missing __rte_msvc_pack to crypto/mlx5 patch * correct commit message for duplicated packing packing in crypto/mlx5 patch v3: * add missing __rte_msvc_pack to crypto/mlx5 * fix commit messages to reference __rte_msvc_pack macro instead of __rte_msvc_pushpack(1) v2: * app/testpmd, remove packing from simple_gre_hdr * net/iavf, remove packing from iavf_ipsec_crypto_pkt_metadata, simple_gre_hdr * examples, remove packing from pkt_key_qinq, pkt_key_ipv4_5tuple, pkt_key_ipv6_5tuple, pkt_key_ipv4_addr, pkt_key_ipv6_addr * eal, remove packing from rte_config, __rte_trace_stream_header Andre Muezerie (30): devtools: check packed attributes eal/include: add new packing macros app/test-pmd: remove unnecessary packed attributes app/test: replace packed attributes doc/guides: replace packed attributes drivers/baseband: replace packed attributes drivers/bus: replace packed attributes drivers/common: replace packed attributes drivers/compress: replace packed attributes drivers/crypto: replace packed attributes drivers/dma: replace packed attributes drivers/event: replace packed attributes drivers/mempool: replace packed attributes drivers/net: replace packed attributes drivers/raw: replace packed attributes drivers/regex: replace packed attributes drivers/vdpa: replace packed attributes examples/common: replace packed attributes examples/ip-pipeline: remove packed attributes examples/ipsec_secgw: replace packed attributes examples/l3fwd-power: replace packed attributes examples/l3fwd: replace packed attributes examples/ptpclient: replace packed attributes examples/vhost_blk: replace packed attributes lib/eal: replace packed attributes lib/ipsec: replace packed attributes lib/net: replace packed attributes lib/pipeline: replace packed attributes lib/vhost: replace packed attributes lib/eal: remove __rte_packed app/test-pmd/csumonly.c |2 +- app/test/test_efd.c |3 +- app/test/test_hash.c |3 +- app/test/test_member.c|3 +- devtools/checkpatches.sh | 23 + doc/guides/nics/ark.rst |3 +- .../prog_guide/packet_classif_access_ctrl.rst |3 +- drivers/baseband/acc/acc_common.h | 59 +- drivers/baseband/fpga_5gnr_fec/agx100_pmd.h | 16 +- .../baseband/fpga_5gnr_fec/fpga_5gnr_fec.h|4 +- drivers/baseband/fpga_5gnr_fec/vc_5gnr_pmd.h |8 +- drivers/baseband/fpga_lte_fec/fpga_lte_fec.c | 12 +- drivers/baseband/la12xx/bbdev_la12xx_ipc.h| 32 +- drivers/bus/dpaa/include/fsl_bman.h | 15 +- drivers/bus/dpaa/include/fsl_fman.h |4 +- drivers/bus/dpaa/include/fsl_qman.h | 158 +- drivers/bus/ifpga/bus_ifpga_driver.h |8 +- drivers/bus/vmbus/rte_vmbus_reg.h | 108 +- drivers/common/cnxk/hw/sdp.h |4 +- drivers/common/cnxk/roc_npc.h | 16 +- drivers/common/cnxk/roc_npc_mcam_dump.c |4 +- drivers/common/cnxk/roc_platform.h|3 +- drivers/common/dpaax/compat.h |3 - drivers/common/iavf/iavf_osdep.h |8 +- drivers/common/iavf/virtchnl_inline_ipsec.h | 44 +- drivers/common/idpf/base/idpf_osdep.h |8 +- drivers/common/mlx5/mlx5_common_mr.h | 12 +- drivers/common/mlx5/mlx5_common_utils.h |3 +- drivers/common/mlx5/mlx5_prm.h| 90 +- drivers/common/qat/qat_adf/icp_qat_fw_la.h|8 +- drivers/common/qat/qat_common.h |8 +- drivers/compress/qat/qat_comp.h |4 +- driver
[PATCH v6 03/30] app/test-pmd: remove unnecessary packed attributes
Removed __rte_packed attribute from structure that is naturally packed already. Signed-off-by: Andre Muezerie --- app/test-pmd/csumonly.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c index 2246c22e8e..d77a140641 100644 --- a/app/test-pmd/csumonly.c +++ b/app/test-pmd/csumonly.c @@ -92,7 +92,7 @@ struct testpmd_offload_info { struct simple_gre_hdr { uint16_t flags; uint16_t proto; -} __rte_packed; +}; static uint16_t get_udptcp_checksum(struct rte_mbuf *m, void *l3_hdr, uint16_t l4_off, -- 2.34.1
[PATCH v6 02/30] eal/include: add new packing macros
MSVC struct packing is not compatible with GCC. Add macro __rte_packed_begin which can be used to push existing pack value and set packing to 1-byte. Add macro __rte_packed_end to restore the pack value prior to the push. Macro __rte_packed_end is deliberately utilized to trigger a MSVC compiler warning if no existing packing has been pushed allowing easy identification of locations where the __rte_packed_begin is missing. Macro __rte_packed will be removed in a subsequent patch. Signed-off-by: Andre Muezerie --- lib/eal/include/rte_common.h | 11 +++ 1 file changed, 11 insertions(+) diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h index 4d299f2b36..affdcaf3c1 100644 --- a/lib/eal/include/rte_common.h +++ b/lib/eal/include/rte_common.h @@ -108,6 +108,17 @@ typedef uint16_t unaligned_uint16_t; #define __rte_packed __attribute__((__packed__)) #endif +/** + * Force a structure to be packed + */ +#ifdef RTE_TOOLCHAIN_MSVC +#define __rte_packed_begin __pragma(pack(push, 1)) +#define __rte_packed_end __pragma(pack(pop)) +#else +#define __rte_packed_begin +#define __rte_packed_end __attribute__((__packed__)) +#endif + /** * Macro to mark a type that is not subject to type-based aliasing rules */ -- 2.34.1
[PATCH v6 07/30] drivers/bus: replace packed attributes
MSVC struct packing is not compatible with GCC. Replace macro __rte_packed with __rte_packed_begin to push existing pack value and set packing to 1-byte and macro __rte_packed_end to restore the pack value prior to the push. Macro __rte_packed_end is deliberately utilized to trigger a MSVC compiler warning if no existing packing has been pushed allowing easy identification of locations where the __rte_packed_begin is missing. Signed-off-by: Andre Muezerie --- drivers/bus/dpaa/include/fsl_bman.h | 15 ++- drivers/bus/dpaa/include/fsl_fman.h | 4 +- drivers/bus/dpaa/include/fsl_qman.h | 155 ++- drivers/bus/ifpga/bus_ifpga_driver.h | 8 +- drivers/bus/vmbus/rte_vmbus_reg.h| 108 +-- 5 files changed, 150 insertions(+), 140 deletions(-) diff --git a/drivers/bus/dpaa/include/fsl_bman.h b/drivers/bus/dpaa/include/fsl_bman.h index 34d7eb32ce..9d3fc9f395 100644 --- a/drivers/bus/dpaa/include/fsl_bman.h +++ b/drivers/bus/dpaa/include/fsl_bman.h @@ -86,6 +86,7 @@ static inline dma_addr_t bm_buf_addr(const struct bm_buffer *buf) } while (0) /* See 1.5.3.5.4: "Release Command" */ +__rte_packed_begin struct bm_rcr_entry { union { struct { @@ -95,7 +96,7 @@ struct bm_rcr_entry { }; struct bm_buffer bufs[8]; }; -} __packed; +} __rte_packed_end; #define BM_RCR_VERB_VBIT 0x80 #define BM_RCR_VERB_CMD_MASK 0x70/* one of two values; */ #define BM_RCR_VERB_CMD_BPID_SINGLE0x20 @@ -104,20 +105,23 @@ struct bm_rcr_entry { /* See 1.5.3.1: "Acquire Command" */ /* See 1.5.3.2: "Query Command" */ +__rte_packed_begin struct bm_mcc_acquire { u8 bpid; u8 __reserved1[62]; -} __packed; +} __rte_packed_end; +__rte_packed_begin struct bm_mcc_query { u8 __reserved2[63]; -} __packed; +} __rte_packed_end; +__rte_packed_begin struct bm_mc_command { u8 __dont_write_directly__verb; union { struct bm_mcc_acquire acquire; struct bm_mcc_query query; }; -} __packed; +} __rte_packed_end; #define BM_MCC_VERB_VBIT 0x80 #define BM_MCC_VERB_CMD_MASK 0x70/* where the verb contains; */ #define BM_MCC_VERB_CMD_ACQUIRE0x10 @@ -136,6 +140,7 @@ struct bm_pool_state { } as, ds; }; +__rte_packed_begin struct bm_mc_result { union { struct { @@ -152,7 +157,7 @@ struct bm_mc_result { } acquire; struct bm_pool_state query; }; -} __packed; +} __rte_packed_end; #define BM_MCR_VERB_VBIT 0x80 #define BM_MCR_VERB_CMD_MASK BM_MCC_VERB_CMD_MASK #define BM_MCR_VERB_CMD_ACQUIREBM_MCC_VERB_CMD_ACQUIRE diff --git a/drivers/bus/dpaa/include/fsl_fman.h b/drivers/bus/dpaa/include/fsl_fman.h index 5a9750ad0c..513d14cced 100644 --- a/drivers/bus/dpaa/include/fsl_fman.h +++ b/drivers/bus/dpaa/include/fsl_fman.h @@ -12,7 +12,7 @@ /* Status field in FD is updated on Rx side by FMAN with following information. * Refer to field description in FM BG. */ -struct fm_status_t { +__rte_packed_begin struct fm_status_t { unsigned int reserved0:3; unsigned int dcl4c:1; /* Don't Check L4 Checksum */ unsigned int reserved1:1; @@ -38,7 +38,7 @@ struct fm_status_t { unsigned int phe:1; /* Header Error during parsing */ unsigned int frdr:1; /* Frame Dropped by disabled port */ unsigned int reserved5:4; -} __rte_packed; +} __rte_packed_end; /* Set MAC address for a particular interface */ __rte_internal diff --git a/drivers/bus/dpaa/include/fsl_qman.h b/drivers/bus/dpaa/include/fsl_qman.h index 25dbf72fd4..7886d466d1 100644 --- a/drivers/bus/dpaa/include/fsl_qman.h +++ b/drivers/bus/dpaa/include/fsl_qman.h @@ -221,6 +221,7 @@ static inline dma_addr_t qm_fd_addr(const struct qm_fd *fd) } while (0) /* Scatter/Gather table entry */ +__rte_packed_begin struct qm_sg_entry { union { struct { @@ -273,7 +274,7 @@ struct qm_sg_entry { }; u16 val_off; }; -} __packed; +} __rte_packed_end; static inline u64 qm_sg_entry_get64(const struct qm_sg_entry *sg) { return sg->addr; @@ -292,6 +293,7 @@ static inline dma_addr_t qm_sg_addr(const struct qm_sg_entry *sg) } while (0) /* See 1.5.8.1: "Enqueue Command" */ +__rte_packed_begin struct __rte_aligned(8) qm_eqcr_entry { u8 __dont_write_directly__verb; u8 dca; @@ -301,7 +303,7 @@ struct __rte_aligned(8) qm_eqcr_entry { u32 tag; struct qm_fd fd; /* this has alignment 8 */ u8 __reserved3[32]; -} __packed; +} __rte_packed_end; /* "Frame Dequeue Response" */ @@ -330,8 +332,9 @@ struct __rte_aligned(8) qm_dqrr_entry { /* "ERN Message Response" */ /* "FQ State Change Notification" */ -struct __rte_aligned(8) qm_mr_entry { +__rte_packed_begin struct __rte_ali
[PATCH v6 08/30] drivers/common: replace packed attributes
MSVC struct packing is not compatible with GCC. Replace macro __rte_packed with __rte_packed_begin to push existing pack value and set packing to 1-byte and macro __rte_packed_end to restore the pack value prior to the push. Macro __rte_packed_end is deliberately utilized to trigger a MSVC compiler warning if no existing packing has been pushed allowing easy identification of locations where the __rte_packed_begin is missing. Signed-off-by: Andre Muezerie --- drivers/common/cnxk/hw/sdp.h| 4 +- drivers/common/cnxk/roc_npc.h | 16 ++-- drivers/common/cnxk/roc_npc_mcam_dump.c | 4 +- drivers/common/cnxk/roc_platform.h | 3 +- drivers/common/dpaax/compat.h | 3 - drivers/common/iavf/iavf_osdep.h| 8 +- drivers/common/iavf/virtchnl_inline_ipsec.h | 44 +- drivers/common/idpf/base/idpf_osdep.h | 8 +- drivers/common/mlx5/mlx5_common_mr.h| 12 ++- drivers/common/mlx5/mlx5_common_utils.h | 3 +- drivers/common/mlx5/mlx5_prm.h | 90 ++--- drivers/common/qat/qat_adf/icp_qat_fw_la.h | 8 +- drivers/common/qat/qat_common.h | 8 +- 13 files changed, 122 insertions(+), 89 deletions(-) diff --git a/drivers/common/cnxk/hw/sdp.h b/drivers/common/cnxk/hw/sdp.h index 686f516097..c5b42c08f6 100644 --- a/drivers/common/cnxk/hw/sdp.h +++ b/drivers/common/cnxk/hw/sdp.h @@ -156,7 +156,7 @@ #define SDP_VF_R_OUT_INT_LEVELS_TIMET (32) /* SDP Instruction Header */ -struct sdp_instr_ih { +__plt_packed_begin struct sdp_instr_ih { /* Data Len */ uint64_t tlen : 16; @@ -177,6 +177,6 @@ struct sdp_instr_ih { /* Reserved2 */ uint64_t rsvd2 : 1; -} __plt_packed; +} __plt_packed_end; #endif /* __SDP_HW_H_ */ diff --git a/drivers/common/cnxk/roc_npc.h b/drivers/common/cnxk/roc_npc.h index bf8c65aa9c..4f7247f867 100644 --- a/drivers/common/cnxk/roc_npc.h +++ b/drivers/common/cnxk/roc_npc.h @@ -97,10 +97,10 @@ struct roc_npc_flow_item_eth { uint32_t reserved : 31; /**< Reserved, must be zero. */ }; -struct roc_vlan_hdr { +__plt_packed_begin struct roc_vlan_hdr { uint16_t vlan_tci; /**< Priority (3) + CFI (1) + Identifier Code (12) */ uint16_t eth_proto; /**< Ethernet type of encapsulated frame. */ -} __plt_packed; +} __plt_packed_end; struct roc_npc_flow_item_vlan { union { @@ -115,23 +115,23 @@ struct roc_npc_flow_item_vlan { uint32_t reserved : 31; /**< Reserved, must be zero. */ }; -struct roc_ipv6_hdr { +__plt_packed_begin struct roc_ipv6_hdr { uint32_t vtc_flow;/**< IP version, traffic class & flow label. */ uint16_t payload_len; /**< IP payload size, including ext. headers */ uint8_t proto;/**< Protocol, next header. */ uint8_t hop_limits; /**< Hop limits. */ uint8_t src_addr[16]; /**< IP address of source host. */ uint8_t dst_addr[16]; /**< IP address of destination host(s). */ -} __plt_packed; +} __plt_packed_end; -struct roc_ipv6_fragment_ext { +__plt_packed_begin struct roc_ipv6_fragment_ext { uint8_t next_header; /**< Next header type */ uint8_t reserved;/**< Reserved */ uint16_t frag_data; /**< All fragmentation data */ uint32_t id; /**< Packet ID */ -} __plt_packed; +} __plt_packed_end; -struct roc_ipv6_routing_ext { +__plt_packed_begin struct roc_ipv6_routing_ext { uint8_t next_hdr; /**< Protocol, next header. */ uint8_t hdr_len;/**< Header length. */ uint8_t type; /**< Extension header type. */ @@ -145,7 +145,7 @@ struct roc_ipv6_routing_ext { }; }; /* Next are 128-bit IPv6 address fields to describe segments. */ -} __plt_packed; +} __plt_packed_end; struct roc_flow_item_ipv6_ext { uint8_t next_hdr; /**< Next header. */ diff --git a/drivers/common/cnxk/roc_npc_mcam_dump.c b/drivers/common/cnxk/roc_npc_mcam_dump.c index ebd2dd69c2..ca566d2b44 100644 --- a/drivers/common/cnxk/roc_npc_mcam_dump.c +++ b/drivers/common/cnxk/roc_npc_mcam_dump.c @@ -35,7 +35,7 @@ #define NIX_TX_VTAGACT_VTAG1_OP_MASKGENMASK(45, 44) #define NIX_TX_VTAGACT_VTAG1_DEF_MASK GENMASK(57, 48) -struct npc_rx_parse_nibble_s { +__plt_packed_begin struct npc_rx_parse_nibble_s { uint16_t chan : 3; uint16_t errlev : 1; uint16_t errcode : 2; @@ -56,7 +56,7 @@ struct npc_rx_parse_nibble_s { uint16_t lgtype : 1; uint16_t lhflags : 2; uint16_t lhtype : 1; -} __plt_packed; +} __plt_packed_end; static const char *const intf_str[] = { "NIX-RX", diff --git a/drivers/common/cnxk/roc_platform.h b/drivers/common/cnxk/roc_platform.h index df4f88f288..d6f3ea9acc 100644 --- a/drivers/common/cnxk/roc_platform.h +++ b/drivers/common/cnxk/roc_platform.h @@ -97,7 +97,8 @@ #define __plt_cache_aligned __rte_cache_aligned #define __plt_always_inline __rte_always_inline -#defin
[PATCH v6 04/30] app/test: replace packed attributes
MSVC struct packing is not compatible with GCC. Replace macro __rte_packed with __rte_packed_begin to push existing pack value and set packing to 1-byte and macro __rte_packed_end to restore the pack value prior to the push. Macro __rte_packed_end is deliberately utilized to trigger a MSVC compiler warning if no existing packing has been pushed allowing easy identification of locations where the __rte_packed_begin is missing. Signed-off-by: Andre Muezerie --- app/test/test_efd.c| 3 ++- app/test/test_hash.c | 3 ++- app/test/test_member.c | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/app/test/test_efd.c b/app/test/test_efd.c index 1c0986b9bc..0cc764 100644 --- a/app/test/test_efd.c +++ b/app/test/test_efd.c @@ -33,13 +33,14 @@ test_efd(void) static unsigned int test_socket_id; /* 5-tuple key type */ +__rte_packed_begin struct flow_key { uint32_t ip_src; uint32_t ip_dst; uint16_t port_src; uint16_t port_dst; uint8_t proto; -} __rte_packed; +} __rte_packed_end; RTE_LOG_REGISTER(efd_logtype_test, test.efd, INFO); diff --git a/app/test/test_hash.c b/app/test/test_hash.c index 65b9cad93c..6dbd038efd 100644 --- a/app/test/test_hash.c +++ b/app/test/test_hash.c @@ -79,13 +79,14 @@ static uint32_t hashtest_key_lens[] = {0, 2, 4, 5, 6, 7, 8, 10, 11, 15, 16, 21, * Should be packed to avoid holes with potentially * undefined content in the middle. */ +__rte_packed_begin struct flow_key { uint32_t ip_src; uint32_t ip_dst; uint16_t port_src; uint16_t port_dst; uint32_t proto; -} __rte_packed; +} __rte_packed_end; /* * Hash function that always returns the same value, to easily test what diff --git a/app/test/test_member.c b/app/test/test_member.c index 5a4d2750db..72ea7c239a 100644 --- a/app/test/test_member.c +++ b/app/test/test_member.c @@ -32,13 +32,14 @@ struct rte_member_setsum *setsum_vbf; struct rte_member_setsum *setsum_sketch; /* 5-tuple key type */ +__rte_packed_begin struct flow_key { uint32_t ip_src; uint32_t ip_dst; uint16_t port_src; uint16_t port_dst; uint8_t proto; -} __rte_packed; +} __rte_packed_end; /* Set ID Macros for multimatch test usage */ #define M_MATCH_S 1/* Not start with 0 since by default 0 means no match */ -- 2.34.1
[PATCH v6 06/30] drivers/baseband: replace packed attributes
MSVC struct packing is not compatible with GCC. Replace macro __rte_packed with __rte_packed_begin to push existing pack value and set packing to 1-byte and macro __rte_packed_end to restore the pack value prior to the push. Macro __rte_packed_end is deliberately utilized to trigger a MSVC compiler warning if no existing packing has been pushed allowing easy identification of locations where the __rte_packed_begin is missing. Signed-off-by: Andre Muezerie --- drivers/baseband/acc/acc_common.h | 59 +++ drivers/baseband/fpga_5gnr_fec/agx100_pmd.h | 16 ++--- .../baseband/fpga_5gnr_fec/fpga_5gnr_fec.h| 4 +- drivers/baseband/fpga_5gnr_fec/vc_5gnr_pmd.h | 8 +-- drivers/baseband/fpga_lte_fec/fpga_lte_fec.c | 12 ++-- drivers/baseband/la12xx/bbdev_la12xx_ipc.h| 32 +- 6 files changed, 72 insertions(+), 59 deletions(-) diff --git a/drivers/baseband/acc/acc_common.h b/drivers/baseband/acc/acc_common.h index bf218332be..74c6787e4b 100644 --- a/drivers/baseband/acc/acc_common.h +++ b/drivers/baseband/acc/acc_common.h @@ -160,6 +160,7 @@ extern int acc_common_logtype; RTE_LOG_LINE(level, ACC_COMMON, __VA_ARGS__) /* ACC100 DMA Descriptor triplet */ +__rte_packed_begin struct acc_dma_triplet { uint64_t address; uint32_t blen:20, @@ -168,7 +169,7 @@ struct acc_dma_triplet { dma_ext:1, res1:2, blkid:4; -} __rte_packed; +} __rte_packed_end; /* ACC100 Queue Manager Enqueue PCI Register */ @@ -183,7 +184,8 @@ union acc_enqueue_reg_fmt { }; /* FEC 4G Uplink Frame Control Word */ -struct __rte_packed acc_fcw_td { +__rte_packed_begin +struct acc_fcw_td { uint8_t fcw_ver:4, num_maps:4; /* Unused in ACC100 */ uint8_t filler:6, /* Unused in ACC100 */ @@ -220,10 +222,11 @@ struct __rte_packed acc_fcw_td { rsrvd4:10; }; }; -}; +} __rte_packed_end; /* FEC 4G Downlink Frame Control Word */ -struct __rte_packed acc_fcw_te { +__rte_packed_begin +struct acc_fcw_te { uint16_t k_neg; uint16_t k_pos; uint8_t c_neg; @@ -251,10 +254,11 @@ struct __rte_packed acc_fcw_te { uint8_t code_block_mode:1, rsrvd8:7; uint64_t rsrvd9; -}; +} __rte_packed_end; /* FEC 5GNR Downlink Frame Control Word */ -struct __rte_packed acc_fcw_le { +__rte_packed_begin +struct acc_fcw_le { uint32_t FCWversion:4, qm:4, nfiller:11, @@ -279,10 +283,11 @@ struct __rte_packed acc_fcw_le { uint32_t res6; uint32_t res7; uint32_t res8; -}; +} __rte_packed_end; /* FEC 5GNR Uplink Frame Control Word */ -struct __rte_packed acc_fcw_ld { +__rte_packed_begin +struct acc_fcw_ld { uint32_t FCWversion:4, qm:4, nfiller:11, @@ -326,10 +331,11 @@ struct __rte_packed acc_fcw_ld { tb_crc_select:2, /* Not supported in ACC100 */ dec_llrclip:2, /* Not supported in VRB1 */ tb_trailer_size:20; /* Not supported in ACC100 */ -}; +} __rte_packed_end; /* FFT Frame Control Word */ -struct __rte_packed acc_fcw_fft { +__rte_packed_begin +struct acc_fcw_fft { uint32_t in_frame_size:16, leading_pad_size:16; uint32_t out_frame_size:16, @@ -351,10 +357,11 @@ struct __rte_packed acc_fcw_fft { power_shift:4, power_en:1, res:19; -}; +} __rte_packed_end; /* FFT Frame Control Word. */ -struct __rte_packed acc_fcw_fft_3 { +__rte_packed_begin +struct acc_fcw_fft_3 { uint32_t in_frame_size:16, leading_pad_size:16; uint32_t out_frame_size:16, @@ -381,11 +388,12 @@ struct __rte_packed acc_fcw_fft_3 { uint16_t cs_theta_0[ACC_MAX_CS]; uint32_t cs_theta_d[ACC_MAX_CS]; int8_t cs_time_offset[ACC_MAX_CS]; -}; +} __rte_packed_end; /* MLD-TS Frame Control Word */ -struct __rte_packed acc_fcw_mldts { +__rte_packed_begin +struct acc_fcw_mldts { uint32_t fcw_version:4, res0:12, nrb:13, /* 1 to 1925 */ @@ -409,7 +417,7 @@ struct __rte_packed acc_fcw_mldts { uint32_t pad2; uint32_t pad3; uint32_t pad4; -}; +} __rte_packed_end; /* DMA Response Descriptor */ union acc_dma_rsp_desc { @@ -435,7 +443,8 @@ union acc_dma_rsp_desc { }; /* DMA Request Descriptor */ -struct __rte_packed acc_dma_req_desc { +__rte_packed_begin +struct acc_dma_req_desc { union { struct{ uint32_t type:4, @@ -496,7 +505,7 @@ struct __rte_packed acc_dma_req_desc { }; uint64_t pad3[ACC_DMA_DESC_PADDINGS]; /* pad to 64 bits */ }; -}; +} __rte_packed_end; /* ACC100 DMA Descriptor */ union acc_dma_desc { @@ -506,6 +515,7 @@ union acc_dma_desc { }; /* Union describing Info Ring entry */ +__rte_packed_begin union
[PATCH v6 09/30] drivers/compress: replace packed attributes
MSVC struct packing is not compatible with GCC. Replace macro __rte_packed with __rte_packed_begin to push existing pack value and set packing to 1-byte and macro __rte_packed_end to restore the pack value prior to the push. Macro __rte_packed_end is deliberately utilized to trigger a MSVC compiler warning if no existing packing has been pushed allowing easy identification of locations where the __rte_packed_begin is missing. Signed-off-by: Andre Muezerie --- drivers/compress/qat/qat_comp.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/compress/qat/qat_comp.h b/drivers/compress/qat/qat_comp.h index 1da4770b1c..90231ad42a 100644 --- a/drivers/compress/qat/qat_comp.h +++ b/drivers/compress/qat/qat_comp.h @@ -50,10 +50,10 @@ struct array_of_ptrs { phys_addr_t pointer[0]; }; -struct __rte_cache_aligned qat_inter_sgl { +__rte_packed_begin struct __rte_cache_aligned qat_inter_sgl { qat_sgl_hdr; struct qat_flat_buf buffers[QAT_NUM_BUFS_IN_IM_SGL]; -} __rte_packed; +} __rte_packed_end; struct qat_comp_op_cookie { -- 2.34.1
[PATCH v6 01/30] devtools: check packed attributes
Ensure __rte_packed_begin and __rte_packed_end show up in pairs when checking patches. Signed-off-by: Andre Muezerie --- devtools/checkpatches.sh | 23 +++ 1 file changed, 23 insertions(+) diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh index 4a8591be22..d304a84df3 100755 --- a/devtools/checkpatches.sh +++ b/devtools/checkpatches.sh @@ -362,6 +362,21 @@ check_aligned_attributes() { # return $res } +check_packed_attributes() { # + res=0 + + begin_count=$(grep '__rte_packed_begin' "$1" | \ + wc -l) + end_count=$(grep '__rte_packed_end' "$1" | \ + wc -l) + if [ $begin_count != $end_count ]; then + echo "__rte_packed_begin and __rte_packed_end mismatch. They should always be used in pairs." + res=1 + fi + + return $res +} + check_release_notes() { # rel_notes_prefix=doc/guides/rel_notes/release_ IFS=. read year month release < VERSION @@ -479,6 +494,14 @@ check () { # ret=1 fi + ! $verbose || printf '\nChecking packed attributes:\n' + report=$(check_packed_attributes "$tmpinput") + if [ $? -ne 0 ] ; then + $headline_printed || print_headline "$subject" + printf '%s\n' "$report" + ret=1 + fi + ! $verbose || printf '\nChecking release notes updates:\n' report=$(check_release_notes "$tmpinput") if [ $? -ne 0 ] ; then -- 2.34.1
[PATCH v6 11/30] drivers/dma: replace packed attributes
MSVC struct packing is not compatible with GCC. Replace macro __rte_packed with __rte_packed_begin to push existing pack value and set packing to 1-byte and macro __rte_packed_end to restore the pack value prior to the push. Macro __rte_packed_end is deliberately utilized to trigger a MSVC compiler warning if no existing packing has been pushed allowing easy identification of locations where the __rte_packed_begin is missing. Signed-off-by: Andre Muezerie --- drivers/dma/dpaa/dpaa_qdma.h| 20 ++-- drivers/dma/dpaa2/dpaa2_qdma.h | 16 drivers/dma/ioat/ioat_hw_defs.h | 3 ++- 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/drivers/dma/dpaa/dpaa_qdma.h b/drivers/dma/dpaa/dpaa_qdma.h index 91eaf1455a..579483ac34 100644 --- a/drivers/dma/dpaa/dpaa_qdma.h +++ b/drivers/dma/dpaa/dpaa_qdma.h @@ -119,7 +119,7 @@ (((fsl_qdma_engine)->block_offset) * (x)) /* qDMA Command Descriptor Formats */ -struct fsl_qdma_comp_cmd_desc { +__rte_packed_begin struct fsl_qdma_comp_cmd_desc { uint8_t status; uint32_t rsv0:22; uint32_t ser:1; @@ -132,9 +132,9 @@ struct fsl_qdma_comp_cmd_desc { uint8_t queue:3; uint8_t rsv4:3; uint8_t dd:2; -} __rte_packed; +} __rte_packed_end; -struct fsl_qdma_comp_sg_desc { +__rte_packed_begin struct fsl_qdma_comp_sg_desc { uint32_t offset:13; uint32_t rsv0:19; uint32_t length:30; @@ -143,9 +143,9 @@ struct fsl_qdma_comp_sg_desc { uint32_t addr_lo; uint8_t addr_hi; uint32_t rsv1:24; -} __rte_packed; +} __rte_packed_end; -struct fsl_qdma_sdf { +__rte_packed_begin struct fsl_qdma_sdf { uint32_t rsv0; uint32_t ssd:12; uint32_t sss:12; @@ -160,9 +160,9 @@ struct fsl_qdma_sdf { uint32_t sqos:3; uint32_t ns:1; uint32_t srttype:4; -} __rte_packed; +} __rte_packed_end; -struct fsl_qdma_ddf { +__rte_packed_begin struct fsl_qdma_ddf { uint32_t rsv0; uint32_t dsd:12; uint32_t dss:12; @@ -177,7 +177,7 @@ struct fsl_qdma_ddf { uint32_t dqos:3; uint32_t ns:1; uint32_t dwttype:4; -} __rte_packed; +} __rte_packed_end; struct fsl_qdma_df { struct fsl_qdma_sdf sdf; @@ -186,7 +186,7 @@ struct fsl_qdma_df { #define FSL_QDMA_SG_MAX_ENTRY 64 #define FSL_QDMA_MAX_DESC_NUM (FSL_QDMA_SG_MAX_ENTRY * QDMA_QUEUE_SIZE) -struct fsl_qdma_cmpd_ft { +__rte_packed_begin struct fsl_qdma_cmpd_ft { struct fsl_qdma_comp_sg_desc desc_buf; struct fsl_qdma_comp_sg_desc desc_sbuf; struct fsl_qdma_comp_sg_desc desc_dbuf; @@ -197,7 +197,7 @@ struct fsl_qdma_cmpd_ft { uint64_t phy_ssge; uint64_t phy_dsge; uint64_t phy_df; -} __rte_packed; +} __rte_packed_end; #define FSL_QDMA_ERR_REG_STATUS_OFFSET 0xe00 diff --git a/drivers/dma/dpaa2/dpaa2_qdma.h b/drivers/dma/dpaa2/dpaa2_qdma.h index 0fd1debaf8..664304e1ff 100644 --- a/drivers/dma/dpaa2/dpaa2_qdma.h +++ b/drivers/dma/dpaa2/dpaa2_qdma.h @@ -39,7 +39,7 @@ #define DPAA2_QDMA_BMT_DISABLE 0x0 /** Source/Destination Descriptor */ -struct qdma_sdd { +__rte_packed_begin struct qdma_sdd { uint32_t rsv; /** Stride configuration */ uint32_t stride; @@ -85,7 +85,7 @@ struct qdma_sdd { uint32_t wrttype:4; } write_cmd; }; -} __rte_packed; +} __rte_packed_end; #define QDMA_SG_FMT_SDB0x0 /* single data buffer */ #define QDMA_SG_FMT_FDS0x1 /* frame data section */ @@ -96,7 +96,7 @@ struct qdma_sdd { #define QDMA_SG_BMT_ENABLE DPAA2_QDMA_BMT_ENABLE #define QDMA_SG_BMT_DISABLE DPAA2_QDMA_BMT_DISABLE -struct qdma_sg_entry { +__rte_packed_begin struct qdma_sg_entry { uint32_t addr_lo; /* address 0:31 */ uint32_t addr_hi:17;/* address 32:48 */ uint32_t rsv:15; @@ -122,7 +122,7 @@ struct qdma_sg_entry { uint32_t f:1; } ctrl; }; -} __rte_packed; +} __rte_packed_end; struct dpaa2_qdma_rbp { uint32_t use_ultrashort:1; @@ -213,19 +213,19 @@ enum { DPAA2_QDMA_MAX_SDD }; -struct qdma_cntx_fle_sdd { +__rte_packed_begin struct qdma_cntx_fle_sdd { struct qbman_fle fle[DPAA2_QDMA_MAX_FLE]; struct qdma_sdd sdd[DPAA2_QDMA_MAX_SDD]; -} __rte_packed; +} __rte_packed_end; -struct qdma_cntx_sg { +__rte_packed_begin struct qdma_cntx_sg { struct qdma_cntx_fle_sdd fle_sdd; struct qdma_sg_entry sg_src_entry[RTE_DPAAX_QDMA_JOB_SUBMIT_MAX]; struct qdma_sg_entry sg_dst_entry[RTE_DPAAX_QDMA_JOB_SUBMIT_MAX]; uint16_t cntx_idx[RTE_DPAAX_QDMA_JOB_SUBMIT_MAX]; uint16_t job_nb; uint16_t rsv[3]; -} __rte_packed; +} __rte_packed_end; #define DPAA2_QDMA_IDXADDR_FROM_SG_FLAG(flag) \ ((void *)(uintptr_t)((flag) - ((flag) & RTE_DPAAX_QDMA_SG_IDX_ADDR_MASK))) diff --git a/drivers/dma/ioat/ioat_hw_defs.h b/drivers/dma/ioat/ioat_hw_defs.h
[PATCH v6 30/30] lib/eal: remove __rte_packed
Remove macro __rte_packed now that the code was made portable using __rte_packed_begin to push existing pack value and set packing to 1-byte and macro __rte_packed_end to restore the pack value prior to the push when MSVC is used. Signed-off-by: Andre Muezerie --- lib/eal/include/rte_common.h | 9 - 1 file changed, 9 deletions(-) diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h index affdcaf3c1..21bfd26b2b 100644 --- a/lib/eal/include/rte_common.h +++ b/lib/eal/include/rte_common.h @@ -99,15 +99,6 @@ typedef uint32_t unaligned_uint32_t; typedef uint16_t unaligned_uint16_t; #endif -/** - * Force a structure to be packed - */ -#ifdef RTE_TOOLCHAIN_MSVC -#define __rte_packed -#else -#define __rte_packed __attribute__((__packed__)) -#endif - /** * Force a structure to be packed */ -- 2.34.1
[PATCH v6 28/30] lib/pipeline: replace packed attributes
MSVC struct packing is not compatible with GCC. Replace macro __rte_packed with __rte_packed_begin to push existing pack value and set packing to 1-byte and macro __rte_packed_end to restore the pack value prior to the push. Macro __rte_packed_end is deliberately utilized to trigger a MSVC compiler warning if no existing packing has been pushed allowing easy identification of locations where the __rte_packed_begin is missing. Signed-off-by: Andre Muezerie --- lib/pipeline/rte_table_action.c | 64 - 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/lib/pipeline/rte_table_action.c b/lib/pipeline/rte_table_action.c index a431f8f128..60cc9ae659 100644 --- a/lib/pipeline/rte_table_action.c +++ b/lib/pipeline/rte_table_action.c @@ -60,9 +60,9 @@ lb_cfg_check(struct rte_table_action_lb_config *cfg) return 0; } -struct lb_data { +__rte_packed_begin struct lb_data { uint32_t out[RTE_TABLE_ACTION_LB_TABLE_SIZE]; -} __rte_packed; +} __rte_packed_end; static int lb_apply(struct lb_data *data, @@ -356,10 +356,10 @@ tm_cfg_check(struct rte_table_action_tm_config *tm) return 0; } -struct tm_data { +__rte_packed_begin struct tm_data { uint32_t queue_id; uint32_t reserved; -} __rte_packed; +} __rte_packed_end; static int tm_apply_check(struct rte_table_action_tm_params *p, @@ -465,11 +465,11 @@ struct encap_qinq_data { uint64_t)(s)) & 0x1LLU) << 8) |\ (((uint64_t)(ttl)) & 0xFFLLU))) -struct __rte_aligned(2) encap_mpls_data { +__rte_packed_begin struct __rte_aligned(2) encap_mpls_data { struct rte_ether_hdr ether; uint32_t mpls[RTE_TABLE_ACTION_MPLS_LABELS_MAX]; uint32_t mpls_count; -} __rte_packed; +} __rte_packed_end; #define PPP_PROTOCOL_IP0x0021 @@ -487,42 +487,42 @@ struct encap_pppoe_data { #define IP_PROTO_UDP 17 -struct __rte_aligned(2) encap_vxlan_ipv4_data { +__rte_packed_begin struct __rte_aligned(2) encap_vxlan_ipv4_data { struct rte_ether_hdr ether; struct rte_ipv4_hdr ipv4; struct rte_udp_hdr udp; struct rte_vxlan_hdr vxlan; -} __rte_packed; +} __rte_packed_end; -struct __rte_aligned(2) encap_vxlan_ipv4_vlan_data { +__rte_packed_begin struct __rte_aligned(2) encap_vxlan_ipv4_vlan_data { struct rte_ether_hdr ether; struct rte_vlan_hdr vlan; struct rte_ipv4_hdr ipv4; struct rte_udp_hdr udp; struct rte_vxlan_hdr vxlan; -} __rte_packed; +} __rte_packed_end; -struct __rte_aligned(2) encap_vxlan_ipv6_data { +__rte_packed_begin struct __rte_aligned(2) encap_vxlan_ipv6_data { struct rte_ether_hdr ether; struct rte_ipv6_hdr ipv6; struct rte_udp_hdr udp; struct rte_vxlan_hdr vxlan; -} __rte_packed; +} __rte_packed_end; -struct __rte_aligned(2) encap_vxlan_ipv6_vlan_data { +__rte_packed_begin struct __rte_aligned(2) encap_vxlan_ipv6_vlan_data { struct rte_ether_hdr ether; struct rte_vlan_hdr vlan; struct rte_ipv6_hdr ipv6; struct rte_udp_hdr udp; struct rte_vxlan_hdr vxlan; -} __rte_packed; +} __rte_packed_end; -struct __rte_aligned(2) encap_qinq_pppoe_data { +__rte_packed_begin struct __rte_aligned(2) encap_qinq_pppoe_data { struct rte_ether_hdr ether; struct rte_vlan_hdr svlan; struct rte_vlan_hdr cvlan; struct pppoe_ppp_hdr pppoe_ppp; -} __rte_packed; +} __rte_packed_end; static size_t encap_data_size(struct rte_table_action_encap_config *encap) @@ -1196,15 +1196,15 @@ nat_cfg_check(struct rte_table_action_nat_config *nat) return 0; } -struct nat_ipv4_data { +__rte_packed_begin struct nat_ipv4_data { uint32_t addr; uint16_t port; -} __rte_packed; +} __rte_packed_end; -struct nat_ipv6_data { +__rte_packed_begin struct nat_ipv6_data { struct rte_ipv6_addr addr; uint16_t port; -} __rte_packed; +} __rte_packed_end; static size_t nat_data_size(struct rte_table_action_nat_config *nat __rte_unused, @@ -1493,9 +1493,9 @@ ttl_cfg_check(struct rte_table_action_ttl_config *ttl) return 0; } -struct ttl_data { +__rte_packed_begin struct ttl_data { uint32_t n_packets; -} __rte_packed; +} __rte_packed_end; #define TTL_INIT(data, decrement) \ ((data)->n_packets = (decrement) ? 1 : 0) @@ -1576,10 +1576,10 @@ stats_cfg_check(struct rte_table_action_stats_config *stats) return 0; } -struct stats_data { +__rte_packed_begin struct stats_data { uint64_t n_packets; uint64_t n_bytes; -} __rte_packed; +} __rte_packed_end; static int stats_apply(struct stats_data *data, @@ -1602,9 +1602,9 @@ pkt_work_stats(struct stats_data *data, /** * RTE_TABLE_ACTION_TIME */ -struct time_data { +__rte_packed_begin struct time_data { uint64_t time; -} __rte_packed; +} __rte_packed_en
[PATCH v6 21/30] examples/l3fwd-power: replace packed attributes
MSVC struct packing is not compatible with GCC. Replace macro __rte_packed with __rte_packed_begin to push existing pack value and set packing to 1-byte and macro __rte_packed_end to restore the pack value prior to the push. Macro __rte_packed_end is deliberately utilized to trigger a MSVC compiler warning if no existing packing has been pushed allowing easy identification of locations where the __rte_packed_begin is missing. Signed-off-by: Andre Muezerie --- examples/l3fwd-power/main.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c index ae8b55924e..65b6bd3756 100644 --- a/examples/l3fwd-power/main.c +++ b/examples/l3fwd-power/main.c @@ -282,21 +282,23 @@ static struct rte_mempool * pktmbuf_pool[NB_SOCKETS]; #define DEFAULT_HASH_FUNC rte_jhash #endif +__rte_packed_begin struct ipv4_5tuple { uint32_t ip_dst; uint32_t ip_src; uint16_t port_dst; uint16_t port_src; uint8_t proto; -} __rte_packed; +} __rte_packed_end; +__rte_packed_begin struct ipv6_5tuple { uint8_t ip_dst[IPV6_ADDR_LEN]; uint8_t ip_src[IPV6_ADDR_LEN]; uint16_t port_dst; uint16_t port_src; uint8_t proto; -} __rte_packed; +} __rte_packed_end; struct ipv4_l3fwd_route { struct ipv4_5tuple key; -- 2.34.1
[PATCH v6 29/30] lib/vhost: replace packed attributes
MSVC struct packing is not compatible with GCC. Replace macro __rte_packed with __rte_packed_begin to push existing pack value and set packing to 1-byte and macro __rte_packed_end to restore the pack value prior to the push. Macro __rte_packed_end is deliberately utilized to trigger a MSVC compiler warning if no existing packing has been pushed allowing easy identification of locations where the __rte_packed_begin is missing. Signed-off-by: Andre Muezerie --- lib/vhost/vhost_user.h | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/vhost/vhost_user.h b/lib/vhost/vhost_user.h index edf7adb3c0..d48542e72f 100644 --- a/lib/vhost/vhost_user.h +++ b/lib/vhost/vhost_user.h @@ -143,7 +143,7 @@ struct vhost_user_config { uint8_t region[VHOST_USER_MAX_CONFIG_SIZE]; }; -typedef struct VhostUserMsg { +typedef __rte_packed_begin struct VhostUserMsg { union { uint32_t frontend; /* a VhostUserRequest value */ uint32_t backend; /* a VhostUserBackendRequest value*/ @@ -169,16 +169,16 @@ typedef struct VhostUserMsg { struct vhost_user_config cfg; } payload; /* Nothing should be added after the payload */ -} __rte_packed VhostUserMsg; +} __rte_packed_end VhostUserMsg; /* Note: this structure and VhostUserMsg can't be changed carelessly as * external message handlers rely on them. */ -struct __rte_packed vhu_msg_context { +__rte_packed_begin struct vhu_msg_context { VhostUserMsg msg; int fds[VHOST_MEMORY_MAX_NREGIONS]; int fd_num; -}; +} __rte_packed_end; #define VHOST_USER_HDR_SIZE offsetof(VhostUserMsg, payload.u64) -- 2.34.1
[PATCH v6 26/30] lib/ipsec: replace packed attributes
MSVC struct packing is not compatible with GCC. Replace macro __rte_packed with __rte_packed_begin to push existing pack value and set packing to 1-byte and macro __rte_packed_end to restore the pack value prior to the push. Macro __rte_packed_end is deliberately utilized to trigger a MSVC compiler warning if no existing packing has been pushed allowing easy identification of locations where the __rte_packed_begin is missing. Signed-off-by: Andre Muezerie --- lib/ipsec/crypto.h | 44 ++-- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/lib/ipsec/crypto.h b/lib/ipsec/crypto.h index 93d200..bbadd38257 100644 --- a/lib/ipsec/crypto.h +++ b/lib/ipsec/crypto.h @@ -15,11 +15,11 @@ * AES-CTR counter block format. */ -struct aesctr_cnt_blk { +__rte_packed_begin struct aesctr_cnt_blk { uint32_t nonce; uint64_t iv; uint32_t cnt; -} __rte_packed; +} __rte_packed_end; /* * CHACHA20-POLY1305 devices have some specific requirements @@ -27,13 +27,13 @@ struct aesctr_cnt_blk { * Ideally that to be done by the driver itself. */ -struct aead_chacha20_poly1305_iv { +__rte_packed_begin struct aead_chacha20_poly1305_iv { uint32_t salt; uint64_t iv; uint32_t cnt; -} __rte_packed; +} __rte_packed_end; -struct aead_chacha20_poly1305_aad { +__rte_packed_begin struct aead_chacha20_poly1305_aad { uint32_t spi; /* * RFC 4106, section 5: @@ -45,25 +45,25 @@ struct aead_chacha20_poly1305_aad { uint64_t u64; } sqn; uint32_t align0; /* align to 16B boundary */ -} __rte_packed; +} __rte_packed_end; -struct chacha20_poly1305_esph_iv { +__rte_packed_begin struct chacha20_poly1305_esph_iv { struct rte_esp_hdr esph; uint64_t iv; -} __rte_packed; +} __rte_packed_end; /* * AES-GCM devices have some specific requirements for IV and AAD formats. * Ideally that to be done by the driver itself. */ -struct aead_gcm_iv { +__rte_packed_begin struct aead_gcm_iv { uint32_t salt; uint64_t iv; uint32_t cnt; -} __rte_packed; +} __rte_packed_end; -struct aead_gcm_aad { +__rte_packed_begin struct aead_gcm_aad { uint32_t spi; /* * RFC 4106, section 5: @@ -75,34 +75,34 @@ struct aead_gcm_aad { uint64_t u64; } sqn; uint32_t align0; /* align to 16B boundary */ -} __rte_packed; +} __rte_packed_end; -struct gcm_esph_iv { +__rte_packed_begin struct gcm_esph_iv { struct rte_esp_hdr esph; uint64_t iv; -} __rte_packed; +} __rte_packed_end; /* * AES-CCM devices have some specific requirements for IV and AAD formats. * Ideally that to be done by the driver itself. */ -union aead_ccm_salt { +__rte_packed_begin union aead_ccm_salt { uint32_t salt; struct inner { uint8_t salt8[3]; uint8_t ccm_flags; } inner; -} __rte_packed; +} __rte_packed_end; -struct aead_ccm_iv { +__rte_packed_begin struct aead_ccm_iv { uint8_t ccm_flags; uint8_t salt[3]; uint64_t iv; uint32_t cnt; -} __rte_packed; +} __rte_packed_end; -struct aead_ccm_aad { +__rte_packed_begin struct aead_ccm_aad { uint8_t padding[18]; uint32_t spi; /* @@ -115,12 +115,12 @@ struct aead_ccm_aad { uint64_t u64; } sqn; uint32_t align0; /* align to 16B boundary */ -} __rte_packed; +} __rte_packed_end; -struct ccm_esph_iv { +__rte_packed_begin struct ccm_esph_iv { struct rte_esp_hdr esph; uint64_t iv; -} __rte_packed; +} __rte_packed_end; static inline void -- 2.34.1
[PATCH v6 20/30] examples/ipsec_secgw: replace packed attributes
MSVC struct packing is not compatible with GCC. Replace macro __rte_packed with __rte_packed_begin to push existing pack value and set packing to 1-byte and macro __rte_packed_end to restore the pack value prior to the push. Macro __rte_packed_end is deliberately utilized to trigger a MSVC compiler warning if no existing packing has been pushed allowing easy identification of locations where the __rte_packed_begin is missing. Signed-off-by: Andre Muezerie --- examples/ipsec-secgw/ipsec.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/examples/ipsec-secgw/ipsec.h b/examples/ipsec-secgw/ipsec.h index f12f57e2d5..afa0f28cc5 100644 --- a/examples/ipsec-secgw/ipsec.h +++ b/examples/ipsec-secgw/ipsec.h @@ -274,11 +274,12 @@ struct socket_ctx { struct rte_mempool *session_pool; }; +__rte_packed_begin struct cnt_blk { uint32_t salt; uint64_t iv; uint32_t cnt; -} __rte_packed; +} __rte_packed_end; struct __rte_cache_aligned lcore_rx_queue { uint16_t port_id; -- 2.34.1
[PATCH v6 23/30] examples/ptpclient: replace packed attributes
MSVC struct packing is not compatible with GCC. Replace macro __rte_packed with __rte_packed_begin to push existing pack value and set packing to 1-byte and macro __rte_packed_end to restore the pack value prior to the push. Macro __rte_packed_end is deliberately utilized to trigger a MSVC compiler warning if no existing packing has been pushed allowing easy identification of locations where the __rte_packed_begin is missing. Signed-off-by: Andre Muezerie --- examples/ptpclient/ptpclient.c | 24 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/examples/ptpclient/ptpclient.c b/examples/ptpclient/ptpclient.c index 23fa487081..5486a87119 100644 --- a/examples/ptpclient/ptpclient.c +++ b/examples/ptpclient/ptpclient.c @@ -85,21 +85,24 @@ static const struct rte_ether_addr ether_multicast = { }; /* Structs used for PTP handling. */ +__rte_packed_begin struct tstamp { uint16_t sec_msb; uint32_t sec_lsb; uint32_t ns; -} __rte_packed; +} __rte_packed_end; struct clock_id { uint8_t id[8]; }; +__rte_packed_begin struct port_id { struct clock_idclock_id; uint16_t port_number; -} __rte_packed; +} __rte_packed_end; +__rte_packed_begin struct ptp_header { uint8_t msg_type; uint8_t ver; @@ -113,39 +116,44 @@ struct ptp_header { uint16_t seq_id; uint8_t control; int8_t log_message_interval; -} __rte_packed; +} __rte_packed_end; +__rte_packed_begin struct sync_msg { struct ptp_header hdr; struct tstamp origin_tstamp; -} __rte_packed; +} __rte_packed_end; +__rte_packed_begin struct follow_up_msg { struct ptp_header hdr; struct tstamp precise_origin_tstamp; uint8_t suffix[]; -} __rte_packed; +} __rte_packed_end; +__rte_packed_begin struct delay_req_msg { struct ptp_header hdr; struct tstamp origin_tstamp; -} __rte_packed; +} __rte_packed_end; +__rte_packed_begin struct delay_resp_msg { struct ptp_headerhdr; struct tstamprx_tstamp; struct port_id req_port_id; uint8_t suffix[]; -} __rte_packed; +} __rte_packed_end; struct ptp_message { + __rte_packed_begin union { struct ptp_header header; struct sync_msgsync; struct delay_req_msg delay_req; struct follow_up_msg follow_up; struct delay_resp_msg delay_resp; - } __rte_packed; + } __rte_packed_end; }; struct ptpv2_time_receiver_ordinary { -- 2.34.1
[PATCH v6 16/30] drivers/regex: replace packed attributes
MSVC struct packing is not compatible with GCC. Replace macro __rte_packed with __rte_packed_begin to push existing pack value and set packing to 1-byte and macro __rte_packed_end to restore the pack value prior to the push. Macro __rte_packed_end is deliberately utilized to trigger a MSVC compiler warning if no existing packing has been pushed allowing easy identification of locations where the __rte_packed_begin is missing. Signed-off-by: Andre Muezerie --- drivers/regex/cn9k/cn9k_regexdev.c | 4 ++-- drivers/regex/mlx5/mlx5_rxp.h | 16 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/regex/cn9k/cn9k_regexdev.c b/drivers/regex/cn9k/cn9k_regexdev.c index aa809ab5bf..33de38e7f9 100644 --- a/drivers/regex/cn9k/cn9k_regexdev.c +++ b/drivers/regex/cn9k/cn9k_regexdev.c @@ -40,12 +40,12 @@ struct ree_rule_db_entry { uint64_tvalue; }; -struct ree_rule_db { +__rte_packed_begin struct ree_rule_db { uint32_t version; uint32_t revision; uint32_t number_of_entries; struct ree_rule_db_entry entries[]; -} __rte_packed; +} __rte_packed_end; static void qp_memzone_name_get(char *name, int size, int dev_id, int qp_id) diff --git a/drivers/regex/mlx5/mlx5_rxp.h b/drivers/regex/mlx5/mlx5_rxp.h index b38b53cc14..6deb25e13d 100644 --- a/drivers/regex/mlx5/mlx5_rxp.h +++ b/drivers/regex/mlx5/mlx5_rxp.h @@ -42,14 +42,14 @@ #define MLX5_RXP_RESP_STATUS_PMI_EOJ (1 << 14) /* This describes the header the RXP expects for any search data. */ -struct mlx5_rxp_job_desc { +__rte_packed_begin struct mlx5_rxp_job_desc { uint32_t job_id; uint16_t ctrl; uint16_t len; uint16_t subset[4]; -} __rte_packed; +} __rte_packed_end; -struct mlx5_rxp_response_desc { +__rte_packed_begin struct mlx5_rxp_response_desc { uint32_t job_id; uint16_t status; uint8_t detected_match_count; @@ -58,13 +58,13 @@ struct mlx5_rxp_response_desc { uint16_t instruction_count; uint16_t latency_count; uint16_t pmi_min_byte_ptr; -} __rte_packed; +} __rte_packed_end; -struct mlx5_rxp_match_tuple { +__rte_packed_begin struct mlx5_rxp_match_tuple { uint32_t rule_id; uint16_t start_ptr; uint16_t length; -} __rte_packed; +} __rte_packed_end; struct mlx5_rxp_response { struct mlx5_rxp_response_desc header; @@ -115,11 +115,11 @@ struct mlx5_rxp_rof { struct mlx5_rxp_rof_entry *rof_entries; }; -struct mlx5_rxp_ctl_rules_pgm { +__rte_packed_begin struct mlx5_rxp_ctl_rules_pgm { struct mlx5_rxp_ctl_hdr hdr; uint32_t count; struct mlx5_rxp_rof_entry rules[]; -} __rte_packed; +} __rte_packed_end; /* RXP programming mode setting. */ enum mlx5_rxp_program_mode { -- 2.34.1
Re: [PATCH v3 3/3] net/hns3: fix Rx packet without CRC data
On 2024/11/27 8:16, Stephen Hemminger wrote: On Fri, 19 Jul 2024 17:04:15 +0800 Jie Hai wrote: From: Dengdui Huang When KEEP_CRC offload is enabled, the CRC data is still stripped in following cases: 1. For HIP08 network engine, the packet type is TCP and the length is less than or equal to 60B. 2. For HIP09 network engine, the packet type is IP and the length is less than or equal to 60B. So driver has to recaculate packet CRC for this rare scenarios. In addition, to avoid impacting performance, KEEP_CRC is not supported when NEON or SVE algorithm is used. Fixes: 8973d7c4ca12 ("net/hns3: support keeping CRC") Cc: sta...@dpdk.org Signed-off-by: Dengdui Huang Acked-by: Huisong Li Acked-by: Jie Hai Changed my mind on these patches after digging deeper into what other drivers are doing. The proposed patches for hns3 do the opposite of what the consensus of drivers is. When looking at internals, all other drivers do not include the CRC in the packet length calculation. It is hard to go back and determine the rational for this, but my assumption is that if a packet is received (with KEEP_CRC enabled), the application will likely want to send that packet to another location, and the transmit side doesn't want the CRC. There are a couple of related driver bugs in some drivers in handling of the flag as well. One driver (idpf) thinks the CRC should count for the byte statistics. This should be clarified and fixed. One driver (atlantic) adds a check but doesn't implement the flag; the check for valid offload flags is already handled by ethdev API. Please resubmit for a later release, and can be picked up then by 24.11 stable. There is indeed much work to be done to clarify the relationship between keep crc and fields in mbuf. In the current patchset, patch 1 and patch 2 are used for this purpose, but a bug occurs. If CRC is not processed in the TX direction, CRC is forwarded as packet data. As a result, the packet length is increased by 4. I agree that this part should be carefully investigated before a decision is made. But for patch 3, this is a serious bug of the hns3 driver and and is irrelevant to the preceding questions. Could you please apply the patch 3 first to fix the bug of hns3 ? I can send a single patch of hns3 driver for the next version. You have found an area of DPDK which is poorly documented. Will raise an agenda at next techboard to get a final agreement, then put that into the programmer's guide. .
RE: [PATCH v4 6/8] build: reduce driver dependencies
Hi, > -Original Message- > From: Burakov, Anatoly > Sent: Tuesday, November 26, 2024 10:40 PM > To: dev@dpdk.org; Chautru, Nicolas ; > Gagandeep Singh ; Hemant Agrawal > ; Parav Pandit ; Xueming Li > ; Sachin Saxena ; Xu, > Rosen ; Chenbo Xia ; Nipun > Gupta ; Tomasz Duszynski > ; Chengwen Feng > ; Nithin Dabilpuram > ; Kiran Kumar K ; > Sunil Kumar Kori ; Satha Rao > ; Harman Kalra ; Anoob > Joseph ; Wu, Jingjing ; > Shetty, Praveen ; Dariusz Sosnowski > ; Viacheslav Ovsiienko ; > Bing Zhao ; Ori Kam ; Suanming > Mou ; Matan Azrad ; Fan > Zhang ; Ashish Gupta > ; Nagadheeraj Rottela > ; Ajit Khaparde > ; Vikas Gupta > ; Ankur Dwivedi ; > Tejasree Kondoj ; Ji, Kai ; De > Lara Guarch, Pablo ; Srikanth Jampala > ; Vamsi Attunuru ; > Gowrishankar Muthukrishnan ; Vidya Sagar > Velumuri ; Laatz, Kevin ; > Richardson, Bruce ; Pavan Nikhilesh > ; Shijith Thotton ; > Pathak, Pravin ; Jerin Jacob > ; Van Haaren, Harry ; > Ashwin Sekhar T K ; Medvedkin, Vladimir > ; Stokes, Ian ; Long > Li ; Wei Hu ; Andrew > Rybchenko ; Dumitrescu, Cristian > ; Jakub Palider ; > Vijay Kumar Srivastava > Subject: [PATCH v4 6/8] build: reduce driver dependencies > > From: Bruce Richardson > > Remove any unnecessary dependencies from the driver dependency lists. > This will give each driver a near-minimum set of required dependencies. > > Signed-off-by: Bruce Richardson > --- > drivers/bus/ifpga/meson.build | 2 +- > diff --git a/drivers/bus/ifpga/meson.build b/drivers/bus/ifpga/meson.build > index dedc94db2d..e87b08eaf4 100644 > --- a/drivers/bus/ifpga/meson.build > +++ b/drivers/bus/ifpga/meson.build > @@ -7,6 +7,6 @@ if is_windows > subdir_done() > endif > > -deps += ['pci', 'kvargs', 'rawdev'] > +deps = ['pci', 'rawdev'] > driver_sdk_headers += files('bus_ifpga_driver.h') sources = > files('ifpga_bus.c') diff --git a/drivers/bus/pci/meson.build > b/drivers/bus/pci/meson.build index fede114dc7..776c3d1550 100644 Reviewed-by: Rosen Xu
[PATCH V1] doc: add tested Intel platforms with Intel NICs
Add tested Intel platforms with Intel NICs to v24.11 release note. Signed-off-by: Lingli Chen --- doc/guides/rel_notes/release_24_11.rst | 165 + 1 file changed, 165 insertions(+) diff --git a/doc/guides/rel_notes/release_24_11.rst b/doc/guides/rel_notes/release_24_11.rst index 48b399cda7..26ee250264 100644 --- a/doc/guides/rel_notes/release_24_11.rst +++ b/doc/guides/rel_notes/release_24_11.rst @@ -533,3 +533,168 @@ Tested Platforms This section is a comment. Do not overwrite or remove it. Also, make sure to start the actual text at the margin. === + +* Intel\ |reg| platforms with Intel\ |reg| NICs combinations + + * CPU + +* Intel Atom\ |reg| P5342 processor +* Intel\ |reg| Atom\ |trade| CPU C3758 @ 2.20GHz +* Intel\ |reg| Xeon\ |reg| CPU D-1553N @ 2.30GHz +* Intel\ |reg| Xeon\ |reg| CPU E5-2699 v4 @ 2.20GHz +* Intel\ |reg| Xeon\ |reg| D-1747NTE CPU @ 2.50GHz +* Intel\ |reg| Xeon\ |reg| D-2796NT CPU @ 2.00GHz +* Intel\ |reg| Xeon\ |reg| Gold 6139 CPU @ 2.30GHz +* Intel\ |reg| Xeon\ |reg| Gold 6140M CPU @ 2.30GHz +* Intel\ |reg| Xeon\ |reg| Gold 6252N CPU @ 2.30GHz +* Intel\ |reg| Xeon\ |reg| Gold 6348 CPU @ 2.60GHz +* Intel\ |reg| Xeon\ |reg| Platinum 8280M CPU @ 2.70GHz +* Intel\ |reg| Xeon\ |reg| Platinum 8358 CPU @ 2.60GHz +* Intel\ |reg| Xeon\ |reg| Platinum 8380 CPU @ 2.30GHz +* Intel\ |reg| Xeon\ |reg| Platinum 8468H +* Intel\ |reg| Xeon\ |reg| Platinum 8490H + + * OS: + +* Microsoft Azure Linux 3.0 +* Fedora 40 +* FreeBSD 14.1 +* OpenAnolis OS 8.9 +* openEuler 24.03 (LTS) +* Red Hat Enterprise Linux Server release 9.4 +* Ubuntu 22.04.3 +* Ubuntu 22.04.4 +* Ubuntu 24.04 +* Ubuntu 24.04.1 + + * NICs: + +* Intel\ |reg| Ethernet Controller E810-C for SFP (4x25G) + + * Firmware version: 4.60 0x8001e8b2 1.3682.0 + * Device id (pf/vf): 8086:1593 / 8086:1889 + * Driver version(out-tree): 1.15.4 (ice) + * Driver version(in-tree): 6.8.0-48-generic (Ubuntu24.04.1) / +5.14.0-427.13.1.el9_4.x86_64+rt (RHEL9.4) (ice) + * OS Default DDP: 1.3.36.0 + * COMMS DDP: 1.3.46.0 + * Wireless Edge DDP: 1.3.14.0 + +* Intel\ |reg| Ethernet Controller E810-C for QSFP (2x100G) + + * Firmware version: 4.60 0x8001e8b1 1.3682.0 + * Device id (pf/vf): 8086:1592 / 8086:1889 + * Driver version(out-tree): 1.15.4 (ice) + * Driver version(in-tree): 6.6.12.1-1.azl3+ice+ (Microsoft Azure Linux 3.0) (ice) + * OS Default DDP: 1.3.36.0 + * COMMS DDP: 1.3.46.0 + * Wireless Edge DDP: 1.3.14.0 + +* Intel\ |reg| Ethernet Controller E810-XXV for SFP (2x25G) + + * Firmware version: 4.60 0x8001e8b0 1.3682.0 + * Device id (pf/vf): 8086:159b / 8086:1889 + * Driver version: 1.15.4 (ice) + * OS Default DDP: 1.3.36.0 + * COMMS DDP: 1.3.46.0 + +* Intel\ |reg| Ethernet Connection E823-C for QSFP + + * Firmware version: 3.42 0x8001f66b 1.3682.0 + * Device id (pf/vf): 8086:188b / 8086:1889 + * Driver version: 1.15.4 (ice) + * OS Default DDP: 1.3.36.0 + * COMMS DDP: 1.3.46.0 + * Wireless Edge DDP: 1.3.14.0 + +* Intel\ |reg| Ethernet Connection E823-L for QSFP + + * Firmware version: 3.42 0x8001ea89 1.3636.0 + * Device id (pf/vf): 8086:124c / 8086:1889 + * Driver version: 1.15.4 (ice) + * OS Default DDP: 1.3.36.0 + * COMMS DDP: 1.3.46.0 + * Wireless Edge DDP: 1.3.14.0 + +* Intel\ |reg| Ethernet Connection E822-L for backplane + + * Firmware version: 3.42 0x8001eaad 1.3636.0 + * Device id (pf/vf): 8086:1897 / 8086:1889 + * Driver version: 1.15.4 (ice) + * OS Default DDP: 1.3.36.0 + * COMMS DDP: 1.3.46.0 + * Wireless Edge DDP: 1.3.14.0 + +* Intel\ |reg| Ethernet Network Adapter E830-XXVDA2 for OCP + + * Firmware version: 1.00 0x8000942a 1.3672.0 + * Device id (pf/vf): 8086:12d3 / 8086:1889 + * Driver version: 1.15.4 (ice) + * OS Default DDP: 1.3.38.0 + +* Intel\ |reg| Ethernet Network Adapter E830-CQDA2 + + * Firmware version: 1.00 0x8000d294 1.3722.0 + * Device id (pf/vf): 8086:12d2 / 8086:1889 + * Driver version: 1.15.4 (ice) + * OS Default DDP: 1.3.39.0 + * COMMS DDP: 1.3.51.0 + * Wireless Edge DDP: 1.3.19.0 + +* Intel\ |reg| 82599ES 10 Gigabit Ethernet Controller + + * Firmware version: 0x000161bf + * Device id (pf/vf): 8086:10fb / 8086:10ed + * Driver version(out-tree): 5.21.5 (ixgbe) + * Driver version(in-tree): 6.8.0-48-generic (Ubuntu24.04.1) + +* Intel\ |reg| Ethernet Network Adapter E610-XT2 + + * Firmware version: 1.00 0x800066ae 0.0.0 + * Device id (pf/vf): 8086:57b0 / 8086:57ad + * Driver version(out-tree): 5.21.5 (ixgbe) + +* Intel\ |reg| Ethernet Network Adapter E610-XT4 + + * Firmware version: 1.00 0x80004ef2 0.0.0 + * Devic
Re: [PATCH] doc: correct definition of Stats per queue feature
11/10/2024 21:25, Ferruh Yigit: > On 10/11/2024 2:38 AM, Stephen Hemminger wrote: > > Change the documentation to match current usage of this feature > > in the NIC table. Moved this sub heading to be after basic > > stats because the queue stats reported now are in the same structure. > > > > Although the "Stats per Queue" feature was originally intended > > to be related to stats mapping, the overwhelming majority of drivers > > report this feature with a different meaning. > > > > Hopefully in later release the per-queue stats limitations > > can be fixed, but this requires and API, ABI, and lots of driver > > changes. > > > > Fixes: dad1ec72a377 ("doc: document NIC features") > > Cc: ferruh.yi...@intel.com > > Signed-off-by: Stephen Hemminger > > Acked-by: Ferruh Yigit Applied with spacing fixed, thanks.
Re: [PATCH v2] doc: add mlx5 xstats send scheduling counters description
On Thu, 31 Oct 2024 10:04:38 +0200 Viacheslav Ovsiienko wrote: > The mlx5 provides the scheduling send on time capability. > To check the operating status of this feature the extended statistics > counters are provided. This patch adds the counter descriptions > and provides some meaningful information how to interpret > the counter values in runtime. > > Signed-off-by: Viacheslav Ovsiienko > --- > doc/guides/nics/mlx5.rst | 59 > 1 file changed, 59 insertions(+) > > diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst > index b5522d50c5..5db4aeda1b 100644 > --- a/doc/guides/nics/mlx5.rst > +++ b/doc/guides/nics/mlx5.rst > @@ -2662,3 +2662,62 @@ Destroy GENEVE TLV parser for specific port:: > > This command doesn't destroy the global list, > For releasing options, ``flush`` command should be used. > + > + > +Extended Statistics Counters > + > + > +Send Scheduling Extended Statistics Counters > + It is great to see description of counters, it would be good if all drivers did this for xstats. There are some issues with the documentation that should be addressed in later update. The section number is wrong. It ends up as a subset of the "Testpmd driver specific commands". It should be in the Statistics section. The section headings overall for this NIC guide need to be reorganized/reworded. Kind of mess now.
Re: [PATCH] test: raise fast test timeout to 60s on RISC-V
On 27/11/2024 04:29, David Marchand wrote: You can extend the timeout via the multiplier option (default timeout of 10s * multiplier). So in your case: $ meson test -C --suite fast-tests -t 6 I hope the RISC-V specific extended timeout could be upstreamed though, in this way we won't need to bump timeout in every distro supporting the architecture (like Debian [1]) or even not running the tests altogether (like OpenSUSE [2] and Fedora [3]), just because tests are failing due to timeout. Cheers, Eric [1]: https://salsa.debian.org/debian/dpdk/-/blob/unstable/debian/tests/test-fastsuite?ref_type=heads#L31 [2]: https://build.opensuse.org/projects/openSUSE:Factory/packages/dpdk/files/dpdk.spec?expand=1 [3]: https://src.fedoraproject.org/rpms/dpdk/blob/rawhide/f/dpdk.spec
[PATCH] examples/ptpclient: revert add frequency adjustment
This commit references GPL-licensed code and therefore cannot be applied to the DPDK. Therefore the following commit was reverted accordingly. Fixes: 6d55af611fd5 ("examples/ptpclient: add frequency adjustment") By resuming this commit, the basic functionality (PMD support for high-precision clocks) will not be affected, but its accuracy will be reduced to the microsecond level. Fixes: 6d55af611fd5 ("examples/ptpclient: add frequency adjustment") Signed-off-by: Mingjin Ye --- doc/guides/sample_app_ug/ptpclient.rst | 17 +- examples/ptpclient/ptpclient.c | 302 ++--- 2 files changed, 25 insertions(+), 294 deletions(-) diff --git a/doc/guides/sample_app_ug/ptpclient.rst b/doc/guides/sample_app_ug/ptpclient.rst index e03a8452d8..38f0479a0f 100644 --- a/doc/guides/sample_app_ug/ptpclient.rst +++ b/doc/guides/sample_app_ug/ptpclient.rst @@ -50,12 +50,6 @@ The adjustment for time receiver can be represented as: If the command line parameter ``-T 1`` is used the application also synchronizes the PTP PHC clock with the Linux kernel clock. -If the command line parameter ``-c 1`` is used, -the application will also use the servo of the local clock. -Only one type of servo is currently implemented, the PI controller. -Default 0 (not used). - - Compiling the Application - @@ -71,7 +65,7 @@ To run the example in a ``linux`` environment: .. code-block:: console -.//examples/dpdk-ptpclient -l 1 -n 4 -- -p 0x1 -T 0 -c 1 +.//examples/dpdk-ptpclient -l 1 -n 4 -- -p 0x1 -T 0 Refer to *DPDK Getting Started Guide* for general information on running applications and the Environment Abstraction Layer (EAL) options. @@ -79,15 +73,6 @@ applications and the Environment Abstraction Layer (EAL) options. * ``-p portmask``: Hexadecimal portmask. * ``-T 0``: Update only the PTP time receiver clock. * ``-T 1``: Update the PTP time receiver clock and synchronize the Linux Kernel to the PTP clock. -* ``-c 0``: Not used clock servo controller. -* ``-c 1``: The clock servo PI controller is used and the log will print information -about time transmitter offset. -Note that the PMD needs to support the ``rte_eth_timesync_adjust_freq()`` API -to enable the servo controller. - -Also, by adding ``-T 1`` and ``-c 1``, the time transmitter offset value printed in the log -will slowly converge and eventually stabilise at the nanosecond level. -The synchronisation accuracy is much higher compared to not using a servo controller. Code Explanation diff --git a/examples/ptpclient/ptpclient.c b/examples/ptpclient/ptpclient.c index 23fa487081..7b6862d951 100644 --- a/examples/ptpclient/ptpclient.c +++ b/examples/ptpclient/ptpclient.c @@ -46,35 +46,6 @@ static volatile bool force_quit; #define KERNEL_TIME_ADJUST_LIMIT 2 #define PTP_PROTOCOL 0x88F7 -#define KP 0.7 -#define KI 0.3 -#define FREQ_EST_MARGIN 0.001 - -enum servo_state { - SERVO_UNLOCKED, - SERVO_JUMP, - SERVO_LOCKED, -}; - -struct pi_servo { - double offset[2]; - double local[2]; - double drift; - double last_freq; - int count; - - double max_frequency; - double step_threshold; - double first_step_threshold; - int first_update; -}; - -enum controller_mode { - MODE_NONE, - MODE_PI, - MAX_ALL -} mode = MODE_NONE; - struct rte_mempool *mbuf_pool; uint32_t ptp_enabled_port_mask; uint8_t ptp_enabled_port_nb; @@ -164,9 +135,6 @@ struct ptpv2_time_receiver_ordinary { uint8_t ptpset; uint8_t kernel_time_set; uint16_t current_ptp_port; - int64_t master_offset; - int64_t path_delay; - struct pi_servo *servo; }; static struct ptpv2_time_receiver_ordinary ptp_data; @@ -294,19 +262,6 @@ port_init(uint16_t port, struct rte_mempool *mbuf_pool) return retval; } - /* -* If the clock servo controller is enabled, the PMD must support -* adjustment of the clock frequency. -*/ - if (mode != MODE_NONE) { - retval = rte_eth_timesync_adjust_freq(port, 0); - if (retval == -ENOTSUP) { - printf("The servo controller cannot work on devices that" - " do not support frequency adjustment.\n"); - return retval; - } - } - return 0; } @@ -342,40 +297,33 @@ print_clock_info(struct ptpv2_time_receiver_ordinary *ptp_data) ptp_data->tstamp4.tv_sec, (ptp_data->tstamp4.tv_nsec)); - if (mode == MODE_NONE) { - printf("\nDelta between transmitter and receiver clocks:%"PRId64"ns\n", - ptp_data->delta); + printf("\nDelta between transmitter and receiver clocks:%"PRId64"ns\n", + ptp_data->delta); - clock_gettime(CLOCK_REA
Re: [RFC] crypto/virtio: add vhost-vdpa backend
Hi Gowrishankar, > On Nov 27, 2024, at 14:50, Gowrishankar Muthukrishnan > wrote: > > External email: Use caution opening links or attachments > > > Hi, > I wanted to follow up on my previous message regarding the development of a > vhost vDPA host driver for crypto/virtio. We have proposed creating a > driver/common/virtio/ directory to hold common implementations for both net > and crypto functionalities. This approach aims to help in fixing common > issues and extending virtio functionalities efficiently. > > As we plan to include this feature in DPDK 25.03, we would like to conclude > on the implementation direction soon. We would greatly appreciate your > valuable feedback or any suggestions on this proposal. Thank you! Thanks for the efforts! IIUC, you want to make sure it’s good direction to have a common folder for both net and crypto. Of course overall this is a good idea as it’s more friendly for maintaining the code. I checked your list of code to be shared, seems it will be many code in the common layer. So please go on with this :) Cheers, Chenbo > > Thanks, > Gowrishankar > >> Hi, >> We are adding support for vDPA user backend for virtio-crypto PMD in DPDK. >> We have come up with functional changes which is similar to the support >> available in net: >> >>commit 6b901437056eed3ed7c9932c333ba24ac5be116f >> >>net/virtio: introduce vhost-vDPA backend >>vhost-vDPA is a new virtio backend type introduced by vDPA kernel >>framework, which provides abstraction to the vDPA devices and >>exposes an unified control interface through a char dev. >> >> Our current development reuses some code from net/virtio/virtio_user/, and >> we realize that we could keep a few things in common between net and >> crypto, such as: >> >> -> vhost_vdpa.c (and its header file) from net/virtio/virtio_user/: >> Except for VHOST_VDPA_GET_DEVICE_ID and enabling queue pairs, >> virtio_user_backend_ops can be reused. >> -> virtio_user_dev.c (and its header file) from net/virtio/virtio_user/: >> virtio_user_dev_init and its capabilities differ. >> -> virtio_cvq.c (and its header file) from net/virtio/: >> There is a difference in the usage of the first and last descriptors for >> the virtio >> header and status (net vs. crypto). >> >> We need to standardize these codes to ensure they work universally. >> Therefore, we propose creating a driver/common/virtio/ directory to house >> them. This approach will help address common issues and extend Virtio >> functionalities shared between crypto and net. For example, the crypto PMD >> can benefit from packed ring support. We welcome your valuable feedback >> and any suggestions. >> >> Thanks, >> Gowrishankar >> -- >> 2.37.1 >
[PATCH v4] vhost/user: clear ring addresses when getting vring base
Clear ring addresses during vring base retrieval to handle guest reboot scenarios correctly. This is particularly important for vdpa-blk devices where the following issue occurs: When a guest OS with vdpa-blk device reboots, during UEFI stage, only one vring is actually used and configured. However, QEMU still sends enable messages for all configured queues. The remaining queues retain their addresses from before reboot, which reference invalid memory mappings in the rebooted guest. The issue manifests in vq_is_ready(): static bool vq_is_ready(struct virtio_net *dev, struct vhost_virtqueue *vq) { /* Only checks pointer validity, not address freshness */ rings_ok = vq->desc && vq->avail && vq->used; ... } vq_is_ready() incorrectly considers these queues as ready because it only checks if desc/avail/used pointers are non-NULL, but cannot detect that these addresses are stale from the previous boot. Clear the ring addresses in vhost_user_get_vring_base() to force the guest driver to reconfigure them before use. This ensures that vq_is_ready() will return false for queues with stale addresses until they are properly reconfigured by the guest driver. Fixes: 3ea7052f4b1b ("vhost: postpone rings addresses translation") Signed-off-by: Jianping Zhao --- lib/vhost/vhost_user.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c index 6d92ad904e..52d8078d7c 100644 --- a/lib/vhost/vhost_user.c +++ b/lib/vhost/vhost_user.c @@ -2277,6 +2277,7 @@ vhost_user_get_vring_base(struct virtio_net **pdev, rte_rwlock_write_lock(&vq->access_lock); vring_invalidate(dev, vq); + memset(&vq->ring_addrs, 0, sizeof(struct vhost_vring_addr)); rte_rwlock_write_unlock(&vq->access_lock); return RTE_VHOST_MSG_RESULT_REPLY; -- 2.34.1
[PATCH v6 27/30] lib/net: replace packed attributes
MSVC struct packing is not compatible with GCC. Replace macro __rte_packed with __rte_packed_begin to push existing pack value and set packing to 1-byte and macro __rte_packed_end to restore the pack value prior to the push. Macro __rte_packed_end is deliberately utilized to trigger a MSVC compiler warning if no existing packing has been pushed allowing easy identification of locations where the __rte_packed_begin is missing. Signed-off-by: Andre Muezerie --- lib/net/rte_arp.h | 6 -- lib/net/rte_dtls.h | 3 ++- lib/net/rte_esp.h | 6 -- lib/net/rte_geneve.h | 3 ++- lib/net/rte_gre.h | 12 lib/net/rte_gtp.h | 15 ++- lib/net/rte_ib.h | 3 ++- lib/net/rte_icmp.h | 9 ++--- lib/net/rte_ip4.h | 3 ++- lib/net/rte_ip6.h | 12 lib/net/rte_l2tpv2.h | 12 lib/net/rte_macsec.h | 6 -- lib/net/rte_mpls.h | 3 ++- lib/net/rte_pdcp_hdr.h | 12 lib/net/rte_ppp.h | 3 ++- lib/net/rte_sctp.h | 3 ++- lib/net/rte_tcp.h | 3 ++- lib/net/rte_tls.h | 3 ++- lib/net/rte_udp.h | 3 ++- lib/net/rte_vxlan.h| 21 ++--- 20 files changed, 94 insertions(+), 47 deletions(-) diff --git a/lib/net/rte_arp.h b/lib/net/rte_arp.h index 668cea1704..4032dd72a5 100644 --- a/lib/net/rte_arp.h +++ b/lib/net/rte_arp.h @@ -21,16 +21,18 @@ extern "C" { /** * ARP header IPv4 payload. */ +__rte_packed_begin struct __rte_aligned(2) rte_arp_ipv4 { struct rte_ether_addr arp_sha; /**< sender hardware address */ rte_be32_tarp_sip; /**< sender IP address */ struct rte_ether_addr arp_tha; /**< target hardware address */ rte_be32_tarp_tip; /**< target IP address */ -} __rte_packed; +} __rte_packed_end; /** * ARP header. */ +__rte_packed_begin struct __rte_aligned(2) rte_arp_hdr { rte_be16_t arp_hardware; /**< format of hardware address */ #define RTE_ARP_HRD_ETHER 1 /**< ARP Ethernet address format */ @@ -47,7 +49,7 @@ struct __rte_aligned(2) rte_arp_hdr { #defineRTE_ARP_OP_INVREPLY 9 /**< response identifying peer */ struct rte_arp_ipv4 arp_data; -} __rte_packed; +} __rte_packed_end; /** * Make a RARP packet based on MAC addr. diff --git a/lib/net/rte_dtls.h b/lib/net/rte_dtls.h index 246cd8a72d..a341b2155e 100644 --- a/lib/net/rte_dtls.h +++ b/lib/net/rte_dtls.h @@ -30,6 +30,7 @@ * DTLS Header */ __extension__ +__rte_packed_begin struct rte_dtls_hdr { /** Content type of DTLS packet. Defined as RTE_DTLS_TYPE_*. */ uint8_t type; @@ -48,6 +49,6 @@ struct rte_dtls_hdr { #endif /** The length (in bytes) of the following DTLS packet. */ rte_be16_t length; -} __rte_packed; +} __rte_packed_end; #endif /* RTE_DTLS_H */ diff --git a/lib/net/rte_esp.h b/lib/net/rte_esp.h index 745a9847fe..156e26ec0a 100644 --- a/lib/net/rte_esp.h +++ b/lib/net/rte_esp.h @@ -16,17 +16,19 @@ /** * ESP Header */ +__rte_packed_begin struct rte_esp_hdr { rte_be32_t spi; /**< Security Parameters Index */ rte_be32_t seq; /**< packet sequence number */ -} __rte_packed; +} __rte_packed_end; /** * ESP Trailer */ +__rte_packed_begin struct rte_esp_tail { uint8_t pad_len; /**< number of pad bytes (0-255) */ uint8_t next_proto; /**< IPv4 or IPv6 or next layer header */ -} __rte_packed; +} __rte_packed_end; #endif /* RTE_ESP_H_ */ diff --git a/lib/net/rte_geneve.h b/lib/net/rte_geneve.h index eb2c85f1e9..66489c524e 100644 --- a/lib/net/rte_geneve.h +++ b/lib/net/rte_geneve.h @@ -34,6 +34,7 @@ * More-bits (optional) variable length options. */ __extension__ +__rte_packed_begin struct rte_geneve_hdr { #if RTE_BYTE_ORDER == RTE_BIG_ENDIAN uint8_t ver:2; /**< Version. */ @@ -52,7 +53,7 @@ struct rte_geneve_hdr { uint8_t vni[3]; /**< Virtual network identifier. */ uint8_t reserved2; /**< Reserved. */ uint32_t opts[];/**< Variable length options. */ -} __rte_packed; +} __rte_packed_end; /* GENEVE ETH next protocol types */ #define RTE_GENEVE_TYPE_ETH0x6558 /**< Ethernet Protocol. */ diff --git a/lib/net/rte_gre.h b/lib/net/rte_gre.h index 1483e1b42d..d9e70c43c6 100644 --- a/lib/net/rte_gre.h +++ b/lib/net/rte_gre.h @@ -23,6 +23,7 @@ * GRE Header */ __extension__ +__rte_packed_begin struct rte_gre_hdr { #if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN uint16_t res2:4; /**< Reserved */ @@ -42,28 +43,31 @@ struct rte_gre_hdr { uint16_t ver:3; /**< Version Number */ #endif rte_be16_t proto; /**< Protocol Type */ -} __rte_packed; +} __rte_packed_end; /** * Optional field checksum in GRE header */ +__rte_packed_begin struct rte_gre_hdr_opt_checksum_rsvd { rte_be16_t checksum; rte_be16_t reserved1; -} __rte_packed; +} __rte_packed_end; /** * Optional field key in GRE header
Re: [PATCH v3 3/3] net/hns3: fix Rx packet without CRC data
On Fri, 19 Jul 2024 17:04:15 +0800 Jie Hai wrote: > From: Dengdui Huang > > When KEEP_CRC offload is enabled, the CRC data is still stripped > in following cases: > 1. For HIP08 network engine, the packet type is TCP and the length >is less than or equal to 60B. > 2. For HIP09 network engine, the packet type is IP and the length >is less than or equal to 60B. > > So driver has to recaculate packet CRC for this rare scenarios. > > In addition, to avoid impacting performance, KEEP_CRC is not > supported when NEON or SVE algorithm is used. > > Fixes: 8973d7c4ca12 ("net/hns3: support keeping CRC") > Cc: sta...@dpdk.org > > Signed-off-by: Dengdui Huang > Acked-by: Huisong Li > Acked-by: Jie Hai Changed my mind on these patches after digging deeper into what other drivers are doing. The proposed patches for hns3 do the opposite of what the consensus of drivers is. When looking at internals, all other drivers do not include the CRC in the packet length calculation. It is hard to go back and determine the rational for this, but my assumption is that if a packet is received (with KEEP_CRC enabled), the application will likely want to send that packet to another location, and the transmit side doesn't want the CRC. There are a couple of related driver bugs in some drivers in handling of the flag as well. One driver (idpf) thinks the CRC should count for the byte statistics. This should be clarified and fixed. One driver (atlantic) adds a check but doesn't implement the flag; the check for valid offload flags is already handled by ethdev API. Please resubmit for a later release, and can be picked up then by 24.11 stable. You have found an area of DPDK which is poorly documented. Will raise an agenda at next techboard to get a final agreement, then put that into the programmer's guide.
Re: [PATCH] dts: remove html dir from dts doc path
26/11/2024 17:25, Paul Szczepanek: > To facilitate deploying docs to the website > and make paths more consistent remove the html > directory from the dts doc path. > > Signed-off-by: Paul Szczepanek > Reviewed-by: Luca Vizzarro > --- > # run sphinx, putting the html output in a "html" directory > with open(join(dst, 'sphinx_html.out'), 'w') as out: > -process = run(sphinx_cmd + ['-b', 'html', src, join(dst, 'html')], > - stdout=out) > +# dts is a special case which doesn't require an html dir in destination > path The comment could be simpler like: # DTS API doc is already located in the API html dir > +html_dst = dst if 'dts' in src else join(dst, 'html') This check is too weak. It would be better to check for /html/dts in dst or for /html/ in 1 of the last 2 path children. > +process = run(sphinx_cmd + ['-b', 'html', src, html_dst], stdout=out)
Re: [PATCH 3/3] eventdev: fix uninitialized variable
> -Original Message- > From: Ma, WenwuX > Sent: 25 November 2024 14:47 > To: ajit.khapa...@broadcom.com; somnath.ko...@broadcom.com; > amitpraka...@marvell.com; Gujjar, Abhinandan S; dev@dpdk.org;sta...@dpdk.org > Cc: Liao, TingtingX; Ma, WenwuX > Subject: [PATCH 3/3] eventdev: fix uninitialized variable > > This patch fixes the variable 'events' may be used uninitialized. > > Fixes: 3c89e8c42022 ("eventdev/dma: support adapter service function") > Cc: sta...@dpdk.org > > Signed-off-by: Wenwu Ma > --- Tested-by: Liao Tingting
Re: [PATCH] net/mlx5: fix Rx queue control deref
Hi, From: Bing Zhao Sent: Monday, November 25, 2024 7:23 PM To: Dariusz Sosnowski; Slava Ovsiienko; dev@dpdk.org; Raslan Darawsheh Cc: Ori Kam; Suanming Mou; Matan Azrad; sta...@dpdk.org Subject: [PATCH] net/mlx5: fix Rx queue control deref When the Rx queue is shared, only the control structure is shared and the private structure of each Rx queue is still independent. During the port stop stage, the hardware resource will be released, and the memory will be freed in the device close stage. Then the control structure reference count should be decreased when freeing a private structure. In the previous implementation, the decreasing action was wrongly put inside the owners list empty condition. Indeed, they should be in the same level. And since the reference count was set to 1 after the 1st queue is created, when checking the value, it should be subtracted firstly and then check the value. With this commit, the reference calculation and condition checking will be corrected. The shared Rx queues' control structures will be freed successlly to avoid the crash in the port restarting. Fixes: 3c9a82fa6edc ("net/mlx5: fix Rx queue control management") Cc: sta...@dpdk.org Signed-off-by: Bing Zhao Acked-by: Dariusz Sosnowski Patch applied to next-net-mlx, Kindest regards, Raslan Darawsheh
Re: [PATCH v2] doc: update match with compare result item limitation
Hi, From: Suanming Mou Sent: Wednesday, November 6, 2024 11:34 AM To: Dariusz Sosnowski; Slava Ovsiienko; Bing Zhao; Ori Kam; Matan Azrad Cc: dev@dpdk.org; sta...@dpdk.org Subject: [PATCH v2] doc: update match with compare result item limitation In switch mode, when ``repr_matching_en`` flag is enabled in the devarg, the match with compare result item is not supported to the ``ingress`` rule as an implicit REPRESENTED_PORT need to be added to the matcher. That REPRESENTED_PORT item conflicts with the single item limitation for match with compare result item. Fixes: cb25df7ce9d6 ("net/mlx5: support comparison matching") Cc: sta...@dpdk.org Signed-off-by: Suanming Mou --- v2: Add `repr_matching_en`` enabled by default explanation. Patch applied to next-net-mlx, Kindest regards, Raslan Darawsheh
Re: [PATCH] vhost: fix read vs write lock mismatch
On 11/18/24 17:24, Stephen Hemminger wrote: If lock is acquired for write, it must be released for write or a deadlock is likely. Bugzilla ID: 1582 Fixes: 9fc93a1e2320 ("vhost: fix virtqueue access check in datapath") Cc: david.march...@redhat.com Cc: sta...@dpdk.org Signed-off-by: Stephen Hemminger --- lib/vhost/virtio_net.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c index 298a5dae74..d764d4bc6a 100644 --- a/lib/vhost/virtio_net.c +++ b/lib/vhost/virtio_net.c @@ -2538,7 +2538,7 @@ virtio_dev_rx_async_submit(struct virtio_net *dev, struct vhost_virtqueue *vq, if (unlikely(!vq->access_ok)) { vhost_user_iotlb_rd_unlock(vq); - rte_rwlock_read_unlock(&vq->access_lock); + rte_rwlock_write_unlock(&vq->access_lock); virtio_dev_vring_translate(dev, vq); goto out_no_unlock; Applied to dpdk-next-virtio/for-main with suggested title change. Thanks, Maxime