Re: [dpdk-dev] [PATCH v2 1/4] ring: future proof flag settings
> -Original Message-
> From: dev On Behalf Of Stephen Hemminger
> Sent: Friday, April 24, 2020 8:02 PM
> To: Honnappa Nagarahalli
> Cc: dev@dpdk.org; nd
> Subject: Re: [dpdk-dev] [PATCH v2 1/4] ring: future proof flag settings
>
> On Fri, 24 Apr 2020 18:07:15 +
> Honnappa Nagarahalli wrote:
>
> >
> >
> > >
> > > All API's should check that they support the flag values passed.
> > > These checks ensure that the extra bits can safely be used without risk
> > > of ABI
> > > breakage.
> > >
> > > Signed-off-by: Stephen Hemminger
> > > ---
> > > lib/librte_ring/rte_ring.c | 10 ++
> > > 1 file changed, 10 insertions(+)
> > >
> > > diff --git a/lib/librte_ring/rte_ring.c b/lib/librte_ring/rte_ring.c index
> > > ebe5ccf0de68..70685121581f 100644
> > > --- a/lib/librte_ring/rte_ring.c
> > > +++ b/lib/librte_ring/rte_ring.c
> > > @@ -42,6 +42,9 @@ static struct rte_tailq_elem rte_ring_tailq = { };
> > > EAL_REGISTER_TAILQ(rte_ring_tailq)
> > >
> > > +/* mask of all valid flag values to ring_create() */
> > > +#define RING_F_MASK 0x007F
> > Is it better to construct this using the actual flag #defines?
>
> sure, but it gets long
+1 to use public defines here.
[dpdk-dev] [PATCH v7 0/6] dpdk: introduce __rte_internal tag
Move the internal function into INTERNAL session to avoid the ABI
checking, and it is only used for DPDK drivers or related library.
__rte_internal funA
INTERNAL {
global:
funA
};
v7: Fix the meson build error
v6: split into small patches, and add the missed handling.
v5: add the checkpatch for __rte_internal style
v4: add the ABI check suppression rules
v3: based on Neil's v2 patch https://patchwork.dpdk.org/cover/54771/
Use the ALLOW_INTERNAL_API to mark this new feature.
Haiyue Wang (6):
eal: add internal ABI tag definition
build: enable internal API tag
mk: add internal tag check
devtools: ignore internal ABI check
devtools: exempt internal ABI checking
devtools: enforce internal tag at the beginning
MAINTAINERS | 2 +-
...-experimental-syms.sh => check-symbols.sh} | 31 +++
buildtools/meson.build| 2 +-
devtools/check-symbol-change.sh | 8
devtools/checkpatches.sh | 39 +++
devtools/libabigail.abignore | 5 +++
drivers/meson.build | 5 ++-
lib/librte_eal/include/rte_compat.h | 13 +++
lib/meson.build | 5 ++-
mk/internal/rte.compile-pre.mk| 6 +--
mk/target/generic/rte.vars.mk | 1 +
11 files changed, 110 insertions(+), 7 deletions(-)
rename buildtools/{check-experimental-syms.sh => check-symbols.sh} (61%)
--
2.26.2
[dpdk-dev] [PATCH v7 1/6] eal: add internal ABI tag definition
Introduce the __rte_internal tag to mark internal ABI function which is
used only by the drivers or other libraries.
Signed-off-by: Haiyue Wang
---
lib/librte_eal/include/rte_compat.h | 13 +
1 file changed, 13 insertions(+)
diff --git a/lib/librte_eal/include/rte_compat.h
b/lib/librte_eal/include/rte_compat.h
index 3eb33784b..4cd8f68d6 100644
--- a/lib/librte_eal/include/rte_compat.h
+++ b/lib/librte_eal/include/rte_compat.h
@@ -19,4 +19,17 @@ __attribute__((section(".text.experimental")))
#endif
+#ifndef ALLOW_INTERNAL_API
+
+#define __rte_internal \
+__attribute__((error("Symbol is not public ABI"), \
+section(".text.internal")))
+
+#else
+
+#define __rte_internal \
+__attribute__((section(".text.internal")))
+
+#endif
+
#endif /* _RTE_COMPAT_H_ */
--
2.26.2
[dpdk-dev] [PATCH v7 2/6] build: enable internal API tag
Allow the drivers and libraries to use the internal tag for marking
internal ABI symbols.
Signed-off-by: Haiyue Wang
---
drivers/meson.build | 5 -
lib/meson.build | 5 -
mk/target/generic/rte.vars.mk | 1 +
3 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/meson.build b/drivers/meson.build
index 4d8f842ab..f3dd23dd4 100644
--- a/drivers/meson.build
+++ b/drivers/meson.build
@@ -20,7 +20,10 @@ dpdk_driver_classes = ['common',
disabled_drivers = run_command(list_dir_globs, get_option('disable_drivers'),
).stdout().split()
-default_cflags = machine_args + ['-DALLOW_EXPERIMENTAL_API']
+default_cflags = machine_args
+default_cflags += ['-DALLOW_EXPERIMENTAL_API']
+default_cflags += ['-DALLOW_INTERNAL_API']
+
if cc.has_argument('-Wno-format-truncation')
default_cflags += '-Wno-format-truncation'
endif
diff --git a/lib/meson.build b/lib/meson.build
index c28b8df83..8697941ae 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -38,7 +38,10 @@ if is_windows
libraries = ['kvargs','eal'] # only supported libraries for windows
endif
-default_cflags = machine_args + ['-DALLOW_EXPERIMENTAL_API']
+default_cflags = machine_args
+default_cflags += ['-DALLOW_EXPERIMENTAL_API']
+default_cflags += ['-DALLOW_INTERNAL_API']
+
if cc.has_argument('-Wno-format-truncation')
default_cflags += '-Wno-format-truncation'
endif
diff --git a/mk/target/generic/rte.vars.mk b/mk/target/generic/rte.vars.mk
index ec2672897..11b0418e5 100644
--- a/mk/target/generic/rte.vars.mk
+++ b/mk/target/generic/rte.vars.mk
@@ -106,6 +106,7 @@ ifeq ($(BUILDING_RTE_SDK),1)
# building sdk
CFLAGS += -include $(RTE_OUTPUT)/include/rte_config.h
CFLAGS += -DALLOW_EXPERIMENTAL_API
+CFLAGS += -DALLOW_INTERNAL_API
else
# if we are building an external application, include SDK's lib and
# includes too
--
2.26.2
[dpdk-dev] [PATCH v7 6/6] devtools: enforce internal tag at the beginning
Move the internal tag on a separate line and make it the first thing of
function prototypes.
Signed-off-by: Haiyue Wang
---
devtools/checkpatches.sh | 39 +++
1 file changed, 39 insertions(+)
diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index c30ce64cc..42b833e0d 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -111,6 +111,37 @@ check_experimental_tags() { #
return $res
}
+check_internal_tags() { #
+ res=0
+
+ cat "$1" |awk '
+ BEGIN {
+ current_file = "";
+ ret = 0;
+ }
+ /^+++ b\// {
+ current_file = $2;
+ }
+ /^+.*__rte_internal/ {
+ if (current_file ~ ".c$" ) {
+ print "Please only put __rte_internal tags in " \
+ "headers ("current_file")";
+ ret = 1;
+ }
+ if ($1 != "+__rte_internal" || $2 != "") {
+ print "__rte_internal must appear alone on the line" \
+ " immediately preceding the return type of" \
+ " a function."
+ ret = 1;
+ }
+ }
+ END {
+ exit ret;
+ }' || res=1
+
+ return $res
+}
+
number=0
range='origin/master..'
quiet=false
@@ -194,6 +225,14 @@ check () { #
ret=1
fi
+ ! $verbose || printf '\nChecking __rte_internal tags:\n'
+ report=$(check_internal_tags "$tmpinput")
+ if [ $? -ne 0 ] ; then
+ $headline_printed || print_headline "$3"
+ printf '%s\n' "$report"
+ ret=1
+ fi
+
if [ "$tmpinput" != "$1" ]; then
rm -f "$tmpinput"
trap - INT
--
2.26.2
[dpdk-dev] [PATCH v7 4/6] devtools: ignore internal ABI check
Ignore the internal version ABI check, this kind of ABI is used only by drivers and libraries. Signed-off-by: Haiyue Wang --- devtools/libabigail.abignore | 5 + 1 file changed, 5 insertions(+) diff --git a/devtools/libabigail.abignore b/devtools/libabigail.abignore index 1911890a7..986a52771 100644 --- a/devtools/libabigail.abignore +++ b/devtools/libabigail.abignore @@ -3,6 +3,11 @@ [suppress_variable] symbol_version = EXPERIMENTAL +[suppress_function] +symbol_version = INTERNAL +[suppress_variable] +symbol_version = INTERNAL + ; Explicit ignore for driver-only ABI [suppress_type] name = rte_cryptodev_ops -- 2.26.2
[dpdk-dev] [PATCH v7 3/6] mk: add internal tag check
Add checks during build to ensure that all symbols in the INTERNAL
version map section have __internal tags on their definitions, and
enable the warnings needed to announce their use.
Signed-off-by: Haiyue Wang
---
MAINTAINERS | 2 +-
...-experimental-syms.sh => check-symbols.sh} | 31 +++
buildtools/meson.build| 2 +-
mk/internal/rte.compile-pre.mk| 6 ++--
4 files changed, 36 insertions(+), 5 deletions(-)
rename buildtools/{check-experimental-syms.sh => check-symbols.sh} (61%)
diff --git a/MAINTAINERS b/MAINTAINERS
index a8d24e332..85298d426 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -153,7 +153,7 @@ F: devtools/libabigail.abignore
F: devtools/update-abi.sh
F: devtools/update_version_map_abi.py
F: devtools/validate-abi.sh
-F: buildtools/check-experimental-syms.sh
+F: buildtools/check-symbols.sh
F: buildtools/map-list-symbol.sh
Driver information
diff --git a/buildtools/check-experimental-syms.sh b/buildtools/check-symbols.sh
similarity index 61%
rename from buildtools/check-experimental-syms.sh
rename to buildtools/check-symbols.sh
index f3603e5ba..3df57c322 100755
--- a/buildtools/check-experimental-syms.sh
+++ b/buildtools/check-symbols.sh
@@ -54,4 +54,35 @@ do
}
done
+for SYM in `$LIST_SYMBOL -S INTERNAL $MAPFILE |cut -d ' ' -f 3`
+do
+ if grep -q "\.text.*[[:space:]]$SYM$" $DUMPFILE &&
+ ! grep -q "\.text\.internal.*[[:space:]]$SYM$" $DUMPFILE
+ then
+ cat >&2 <<- END_OF_MESSAGE
+ $SYM is not flagged as internal
+ but is listed in version map
+ Please add __rte_internal to the definition of $SYM
+ END_OF_MESSAGE
+ ret=1
+ fi
+done
+
+# Filter out symbols suffixed with a . for icc
+for SYM in `awk '{
+ if ($2 != "l" && $4 == ".text.internal" && !($NF ~ /\.$/)) {
+ print $NF
+ }
+}' $DUMPFILE`
+do
+ $LIST_SYMBOL -S INTERNAL -s $SYM -q $MAPFILE || {
+ cat >&2 <<- END_OF_MESSAGE
+ $SYM is flagged as internal
+ but is not listed in version map
+ Please add $SYM to the version map
+ END_OF_MESSAGE
+ ret=1
+ }
+done
+
exit $ret
diff --git a/buildtools/meson.build b/buildtools/meson.build
index 9812917e5..3e8d31b0c 100644
--- a/buildtools/meson.build
+++ b/buildtools/meson.build
@@ -6,7 +6,7 @@ subdir('pmdinfogen')
pkgconf = find_program('pkg-config', 'pkgconf', required: false)
pmdinfo = find_program('gen-pmdinfo-cfile.sh')
list_dir_globs = find_program('list-dir-globs.py')
-check_experimental_syms = find_program('check-experimental-syms.sh')
+check_experimental_syms = find_program('check-symbols.sh')
ldflags_ibverbs_static = find_program('options-ibverbs-static.sh')
# set up map-to-def script using python, either built-in or external
diff --git a/mk/internal/rte.compile-pre.mk b/mk/internal/rte.compile-pre.mk
index 82fe098f7..df05b5576 100644
--- a/mk/internal/rte.compile-pre.mk
+++ b/mk/internal/rte.compile-pre.mk
@@ -56,8 +56,8 @@ C_TO_O = $(CC) -Wp,-MD,$(call obj2dep,$(@)).tmp $(CPPFLAGS)
$(CFLAGS) \
C_TO_O_STR = $(subst ','\'',$(C_TO_O)) #'# fix syntax highlight
C_TO_O_DISP = $(if $(V),"$(C_TO_O_STR)"," CC $(@)")
endif
-EXPERIMENTAL_CHECK = $(RTE_SDK)/buildtools/check-experimental-syms.sh
-CHECK_EXPERIMENTAL = $(EXPERIMENTAL_CHECK) $(SRCDIR)/$(EXPORT_MAP) $@
+CHECK_SYMBOLS_SCRIPT = $(RTE_SDK)/buildtools/check-symbols.sh
+CHECK_SYMBOLS = $(CHECK_SYMBOLS_SCRIPT) $(SRCDIR)/$(EXPORT_MAP) $@
PMDINFO_GEN = $(RTE_SDK_BIN)/app/dpdk-pmdinfogen $@ $@.pmd.c
PMDINFO_CC = $(CC) $(CPPFLAGS) $(CFLAGS) $(EXTRA_CFLAGS) -c -o $@.pmd.o
$@.pmd.c
@@ -75,7 +75,7 @@ C_TO_O_DO = @set -e; \
echo $(C_TO_O_DISP); \
$(C_TO_O) && \
$(PMDINFO_TO_O) && \
- $(CHECK_EXPERIMENTAL) && \
+ $(CHECK_SYMBOLS) && \
echo $(C_TO_O_CMD) > $(call obj2cmd,$(@)) && \
sed 's,'$@':,dep_'$@' =,' $(call obj2dep,$(@)).tmp > $(call
obj2dep,$(@)) && \
rm -f $(call obj2dep,$(@)).tmp
--
2.26.2
[dpdk-dev] [PATCH v7 5/6] devtools: exempt internal ABI checking
No need to restrict the ABI on symbols that are only used by core libraries. Signed-off-by: Haiyue Wang --- devtools/check-symbol-change.sh | 8 1 file changed, 8 insertions(+) diff --git a/devtools/check-symbol-change.sh b/devtools/check-symbol-change.sh index ed2178e36..7b6d5f40f 100755 --- a/devtools/check-symbol-change.sh +++ b/devtools/check-symbol-change.sh @@ -91,6 +91,13 @@ check_for_rule_violations() if [ "$ar" = "add" ] then + if [ "$secname" = "INTERNAL" ] + then + # these are absolved from any further checking + echo "Skipping symbol $symname in INTERNAL" + continue + fi + if [ "$secname" = "unknown" ] then # Just inform the user of this occurrence, but @@ -148,6 +155,7 @@ check_for_rule_violations() else if ! grep -q "$mname $symname .* add" "$mapdb" && \ + [ "$secname" != "INTERNAL" ] && \ [ "$secname" != "EXPERIMENTAL" ] then # Just inform users that non-experimenal -- 2.26.2
Re: [dpdk-dev] [PATCH v5 3/3] ipfrag: add unit test case
20/04/2020 19:34, Aaron Conole: > "Burakov, Anatoly" writes: > > Nitpicking, but i believe the coding style guide discourages using > > boolean syntax for anything other than boolean checks, and it is > > better to use a more explicit `if (x == NULL)`. > > I see, it does. Looking at the code-base, I see it mixed all over, some > places using 'if (!ptr)' and others 'if (ptr == NULL)'. Actually, even > in the flow_filtering.rst doc, it implies that if (!ptr) is acceptable. > > Since I'm spinning a v6 with the constants, I'll fold this change in - > maybe it makes sense to clean it up everywhere to help mitigate the > confusion (for example, I most recently did work in the eal and the !ptr > is all over there). WDYT? In general I agree cleanups are good, avoiding confusion. About changing the whole codebase, just for styling I am not sure. Please let's start with documentation fixes and discuss whether to move forward.
Re: [dpdk-dev] [PATCH] mbuf: fix to update documentation of QinQ stripped bit interpretation
I'll review v2 promptly, some minor comments from me below (taking into account that Olivier's review notes are applied). On 1/6/20 11:34 AM, Somnath Kotur wrote: > Certain hardware may be able to strip and/or save only the outermost > VLAN instead of both the VLANs in the mbuf in a QinQ scenario. > To handle such cases, we could re-interpret setting of just > PKT_RX_QINQ_STRIPPED to indicate that only the outermost VLAN has > been stripped and saved in mbuf->vlan_tci_outer. > Only When both PKT_RX_QINQ_STRIPPED and PKT_RX_VLAN_STRIPPED are set, the 2 > VLANs have been stripped by the hardware and their TCI are saved in > mbuf->vlan_tci (inner) and mbuf->vlan_tci_outer (outer). > > Signed-off-by: Somnath Kotur > --- > lib/librte_mbuf/rte_mbuf_core.h | 15 +++ > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/lib/librte_mbuf/rte_mbuf_core.h b/lib/librte_mbuf/rte_mbuf_core.h > index 9a8557d..db1070b 100644 > --- a/lib/librte_mbuf/rte_mbuf_core.h > +++ b/lib/librte_mbuf/rte_mbuf_core.h > @@ -124,12 +124,19 @@ > #define PKT_RX_FDIR_FLX (1ULL << 14) > > /** > - * The 2 vlans have been stripped by the hardware and their tci are > - * saved in mbuf->vlan_tci (inner) and mbuf->vlan_tci_outer (outer). > + * The outer vlan has been stripped by the hardware and their tci are vlan -> VLAN, tci -> TCI > + * saved in mbuf->vlan_tci_outer (outer). > * This can only happen if vlan stripping is enabled in the RX vlan -> VLAN, RX -> Rx > * configuration of the PMD. > - * When PKT_RX_QINQ_STRIPPED is set, the flags (PKT_RX_VLAN | > - * PKT_RX_VLAN_STRIPPED | PKT_RX_QINQ) must also be set. > + * When PKT_RX_QINQ_STRIPPED is set, the flags (PKT_RX_VLAN | PKT_RX_QINQ) > + * must also be set. > + * When both PKT_RX_QINQ_STRIPPED and PKT_RX_VLAN_STRIPPED are set, the 2 > vlans vlans -> VLANs > + * have been stripped by the hardware and their tci are saved in tci -> TCI > + * mbuf->vlan_tci (inner) and mbuf->vlan_tci_outer (outer). > + * This can only happen if vlan stripping is enabled in the RX configuration vlan -> VLAN, RX -> Rx > + * of the PMD. > + * When PKT_RX_QINQ_STRIPPED and PKT_RX_VLAN_STRIPPED are set, > + * (PKT_RX_VLAN | PKT_RX_QINQ) must also be set. > */ > #define PKT_RX_QINQ_STRIPPED (1ULL << 15) > > I realize that some of my comment above touch not modified lines, but ~90% of the description is updated and I see no point to keep remaining 10% untouched.
Re: [dpdk-dev] [PATCH 0/2] net/tap: simplfication and servicabilty improvements
> On Apr 24, 2020, at 6:36 PM, Stephen Hemminger > wrote: > > These are a couple of small fixes to the TAP driver. The first makes it > more robust to random signals, and the second one adds better error > reporting. > > Stephen Hemminger (2): > net/tap: simplify netlink send/receive functions > net/tap: use netlink extended ack support > > drivers/net/tap/tap_netlink.c | 123 +- > 1 file changed, 92 insertions(+), 31 deletions(-) > > 2.20.1 > Acked-by: Keith Wiles
Re: [dpdk-dev] [PATCH v6 0/3] ip_frag: add a unit test for fragmentation
> Aaron Conole (3): > ip_frag: ensure minimum v4 fragmentation length > ip_frag: ensure minimum v6 fragmentation length > ipfrag: add unit test case Applied, thanks
Re: [dpdk-dev] [PATCH v5] hash: add hash bulk lookup with hash signatures array
17/04/2020 01:46, Wang, Yipeng1: > From: Medvedkin, Vladimir > > > > Implement rte_hash_lookup_with_hash_bulk_data() and > > rte_hash_lookup_with_hash_bulk() - bulk lookup functions with precomputed > > hash signatures. > > Add these two functions into performance tests. > > > > Signed-off-by: Vladimir Medvedkin > > --- > [Wang, Yipeng] Hi, Vladimir, thanks for the changes per my comment. > It looks good now. > > Acked-by: Yipeng Wang Applied, thanks
Re: [dpdk-dev] [PATCH v5] hash: add hash bulk lookup with hash signatures array
25/04/2020 15:30, Thomas Monjalon: > 17/04/2020 01:46, Wang, Yipeng1: > > From: Medvedkin, Vladimir > > > > > > Implement rte_hash_lookup_with_hash_bulk_data() and > > > rte_hash_lookup_with_hash_bulk() - bulk lookup functions with precomputed > > > hash signatures. > > > Add these two functions into performance tests. > > > > > > Signed-off-by: Vladimir Medvedkin > > > --- > > [Wang, Yipeng] Hi, Vladimir, thanks for the changes per my comment. > > It looks good now. > > > > Acked-by: Yipeng Wang > > Applied, thanks Note: I've added this doxygen comment in the new functions: * @warning * @b EXPERIMENTAL: this API may change without prior notice
Re: [dpdk-dev] [PATCH v3] ipsec: use hash lookup with hash sigs in sad lookup
20/04/2020 20:27, Vladimir Medvedkin: > Change hash function from jhash to crc. > Precalculate hash signatures for a bulk of keys and then > use rte_hash_lookup_with_hash_bulk_data() to speed up sad lookup > Also use rte_hash_add_key_with_hash and _del_key_with_hash with > precalculated hash signature for a key in rte_ipsec_sad_add and > rte_ipsec_sad_del > > Signed-off-by: Vladimir Medvedkin > Acked-by: Konstantin Ananyev > --- > This patch depends on https://patches.dpdk.org/patch/68700/ The dependency was merged today, so this patch on IPsec library is applied as well for -rc1, thanks.
Re: [dpdk-dev] [PATCH] doc: refine ethernet and VLAN flow rule items
On 4/23/20 9:30 PM, Dekel Peled wrote:
> Specified pattern may be translated in different manner.
> For example the pattern "eth / ipv4" can be translated to match
> untagged packets only, since the pattern doesn't specify a vlan item.
vlan -> VLAN
> It can also be translated to match both tagged and untagged packets,
> for the same reason.
> This patch updates the rte_flow documentation to clearly specify the
> required pattern to use.
> For example:
> To match tagged ipv4 packets, the pattern "eth type is 0x8100 /
> vlan / ipv4 / end" should be used.
Isn't eth / vlan / ipv4 /end sufficient? What's the difference?
I guess later should allow any VLAN TPID, but it is greyish
since it is HW dependent.
> To match untagged ipv4 packets, the pattern "eth type is 0x0800 /
> ipv4 / end" should be used.
What about eth / ipv4 / end?
Does usage of ipv4 assume that EtherType is 0x0800?
> To match both tagged and untagged packets, the pattern "eth / end"
> should be used.
The interesting question is what should be used if I want
either tagged or untagged IPv4 packets. I think it worse
to mention to make the picture complete.
> Signed-off-by: Dekel Peled
> ---
> doc/guides/prog_guide/rte_flow.rst | 8
> lib/librte_ethdev/rte_flow.h | 9 +
> 2 files changed, 17 insertions(+)
>
> diff --git a/doc/guides/prog_guide/rte_flow.rst
> b/doc/guides/prog_guide/rte_flow.rst
> index cf4368e..0d1c305 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -905,6 +905,12 @@ so-called layer 2.5 pattern items such as
> ``RTE_FLOW_ITEM_TYPE_VLAN``. In
> the latter case, ``type`` refers to that of the outer header, with the inner
> EtherType/TPID provided by the subsequent pattern item. This is the same
> order as on the wire.
> +If the ``type`` field contains a TPID value, then only tagged packets will
> match
> +the pattern.
Shouldn't we emphasis that "tagged packets with specified TPID
will match the pattern." since tagged packets could have
various TPIDs.
> +If the ``type`` field contains another EtherType value, then only untagged
> +packets will match the pattern.
I'm afraid "another EtherType" is too ambiguous.
"non-TPID EtherType" is ambiguous as well and HW
dependent. May be it is better to remove the sentence
completely.
> +If the ``ETH`` item is the only item in the pattern, and the ``type`` field
> is
> +not specified, then both tagged and untagged packets will match the pattern.
>
> - ``dst``: destination MAC.
> - ``src``: source MAC.
> @@ -919,6 +925,8 @@ Matches an 802.1Q/ad VLAN tag.
> The corresponding standard outer EtherType (TPID) values are
> ``RTE_ETHER_TYPE_VLAN`` or ``RTE_ETHER_TYPE_QINQ``. It can be overridden by
> the
> preceding pattern item.
> +If a ``VLAN`` item is present in the pattern, then only tagged packets will
> +match the pattern.
>
> - ``tci``: tag control information.
> - ``inner_type``: inner EtherType or TPID.
> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> index 132b44e..178e87e 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -710,6 +710,13 @@ struct rte_flow_item_raw {
> * the latter case, @p type refers to that of the outer header, with the
> * inner EtherType/TPID provided by the subsequent pattern item. This is the
> * same order as on the wire.
> + * If the @p type field contains a TPID value, then only tagged packets will
> + * match the pattern.
> + * If the @p type field contains another EtherType value, then only untagged
> + * packets will match the pattern.
> + * If the @p ETH item is the only item in the pattern, and the @p type field
> + * is not specified, then both tagged and untagged packets will match the
> + * pattern.
> */
> struct rte_flow_item_eth {
> struct rte_ether_addr dst; /**< Destination MAC. */
> @@ -734,6 +741,8 @@ struct rte_flow_item_eth {
> * The corresponding standard outer EtherType (TPID) values are
> * RTE_ETHER_TYPE_VLAN or RTE_ETHER_TYPE_QINQ. It can be overridden by
> * the preceding pattern item.
> + * If a @p VLAN item is present in the pattern, then only tagged packets will
> + * match the pattern.
> */
> struct rte_flow_item_vlan {
> rte_be16_t tci; /**< Tag control information. */
>
[dpdk-dev] [PATCH v2 0/4] introduce changes to support flow scaling
This patchset introduces changes to the action record allocation, flow database entry deletion, and hw flow cache updates. Action record allocation now allows the actions to scale with the flows. Additionally, resources attached to a flow database entry are now correctly released when the critical resource has not been added to the flow. Finally, the hw flow cache has a timer to periodically invalidate flow entries. v1->v2: Squashed patches 4 & 5 into single patch. Farah Smith (1): net/bnxt: update action record external pool Mike Baucom (2): net/bnxt: reserve a flowdb resource function as invalid net/bnxt: ulp changes to handle action/index tables Shahaji Bhosle (1): net/bnxt: add truflow flush-timer to alloc table scope API drivers/net/bnxt/tf_core/tf_core.c| 3 - drivers/net/bnxt/tf_core/tf_core.h| 21 +++- drivers/net/bnxt/tf_core/tf_msg.c | 3 + drivers/net/bnxt/tf_core/tf_msg.h | 1 + drivers/net/bnxt/tf_core/tf_rm.c | 3 - drivers/net/bnxt/tf_core/tf_session.h | 6 - drivers/net/bnxt/tf_core/tf_tbl.c | 137 -- drivers/net/bnxt/tf_core/tf_tbl.h | 4 +- drivers/net/bnxt/tf_ulp/bnxt_ulp.c| 6 + drivers/net/bnxt/tf_ulp/ulp_mapper.c | 30 +++-- drivers/net/bnxt/tf_ulp/ulp_template_db.h | 15 +-- 11 files changed, 106 insertions(+), 123 deletions(-) -- 2.21.1 (Apple Git-122.3)
[dpdk-dev] [PATCH v2 1/4] net/bnxt: reserve a flowdb resource function as invalid
From: Mike Baucom
The resource function did not have a method of invalidating or
indicating that a resource is uninitialized. Added an invalid enum so
that processing works correctly for partially added flows.
Signed-off-by: Mike Baucom
Reviewed-by: Kishore Padmanabha
Reviewed-by: Ajit Khaparde
---
drivers/net/bnxt/tf_ulp/ulp_template_db.h | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/net/bnxt/tf_ulp/ulp_template_db.h
b/drivers/net/bnxt/tf_ulp/ulp_template_db.h
index a5606bdc4..e6065d2fb 100644
--- a/drivers/net/bnxt/tf_ulp/ulp_template_db.h
+++ b/drivers/net/bnxt/tf_ulp/ulp_template_db.h
@@ -204,13 +204,14 @@ enum bnxt_ulp_regfile_index {
};
enum bnxt_ulp_resource_func {
- BNXT_ULP_RESOURCE_FUNC_TCAM_TABLE = 0,
- BNXT_ULP_RESOURCE_FUNC_EM_TABLE = 1,
- BNXT_ULP_RESOURCE_FUNC_INDEX_TABLE = 2,
- BNXT_ULP_RESOURCE_FUNC_CACHE_TABLE = 3,
- BNXT_ULP_RESOURCE_FUNC_IDENTIFIER = 4,
- BNXT_ULP_RESOURCE_FUNC_HW_FID = 5,
- BNXT_ULP_RESOURCE_FUNC_LAST = 6
+ BNXT_ULP_RESOURCE_FUNC_INVALID = 0,
+ BNXT_ULP_RESOURCE_FUNC_TCAM_TABLE = 1,
+ BNXT_ULP_RESOURCE_FUNC_EM_TABLE = 2,
+ BNXT_ULP_RESOURCE_FUNC_INDEX_TABLE = 3,
+ BNXT_ULP_RESOURCE_FUNC_CACHE_TABLE = 4,
+ BNXT_ULP_RESOURCE_FUNC_IDENTIFIER = 5,
+ BNXT_ULP_RESOURCE_FUNC_HW_FID = 6,
+ BNXT_ULP_RESOURCE_FUNC_LAST = 7
};
enum bnxt_ulp_result_opc {
--
2.21.1 (Apple Git-122.3)
[dpdk-dev] [PATCH v2 3/4] net/bnxt: ulp changes to handle action/index tables
From: Mike Baucom
The ulp required changes to properly call the index table management
routines and use the index for external memory indices. The ulp no
longer has to account for stride as the tf_core returns the actual
offset, not a 0 based index.
Signed-off-by: Mike Baucom
Reviewed-by: Kishore Padmanabha
Reviewed-by: Venkat Duvvuru
Reviewed-by: Ajit Khaparde
---
drivers/net/bnxt/tf_ulp/ulp_mapper.c | 30 ++--
1 file changed, 19 insertions(+), 11 deletions(-)
diff --git a/drivers/net/bnxt/tf_ulp/ulp_mapper.c
b/drivers/net/bnxt/tf_ulp/ulp_mapper.c
index dc7b7ca5e..9ea6fdba0 100644
--- a/drivers/net/bnxt/tf_ulp/ulp_mapper.c
+++ b/drivers/net/bnxt/tf_ulp/ulp_mapper.c
@@ -401,7 +401,7 @@ ulp_mapper_tcam_entry_free(struct bnxt_ulp_context *ulp
__rte_unused,
}
static inline int32_t
-ulp_mapper_index_entry_free(struct bnxt_ulp_context *ulp __rte_unused,
+ulp_mapper_index_entry_free(struct bnxt_ulp_context *ulp,
struct tf *tfp,
struct ulp_flow_db_res_params *res)
{
@@ -411,6 +411,12 @@ ulp_mapper_index_entry_free(struct bnxt_ulp_context *ulp
__rte_unused,
.idx= (uint32_t)res->resource_hndl
};
+ /*
+* Just set the table scope, it will be ignored if not necessary
+* by the tf_free_tbl_entry
+*/
+ bnxt_ulp_cntxt_tbl_scope_id_get(ulp, &fparms.tbl_scope_id);
+
return tf_free_tbl_entry(tfp, &fparms);
}
@@ -805,6 +811,9 @@ ulp_mapper_action_alloc_and_set(struct
bnxt_ulp_mapper_parms *parms,
int32_t rc = 0;
int32_t trc;
uint64_tidx;
+ uint32_t tbl_scope_id;
+
+ bnxt_ulp_cntxt_tbl_scope_id_get(parms->ulp_ctx, &tbl_scope_id);
/* Set the allocation parameters for the table*/
alloc_parms.dir = atbls->direction;
@@ -812,6 +821,7 @@ ulp_mapper_action_alloc_and_set(struct
bnxt_ulp_mapper_parms *parms,
alloc_parms.search_enable = atbls->srch_b4_alloc;
alloc_parms.result = ulp_blob_data_get(blob,
&alloc_parms.result_sz_in_bytes);
+ alloc_parms.tbl_scope_id = tbl_scope_id;
if (!alloc_parms.result) {
BNXT_TF_DBG(ERR, "blob is not populated\n");
return -EINVAL;
@@ -826,14 +836,10 @@ ulp_mapper_action_alloc_and_set(struct
bnxt_ulp_mapper_parms *parms,
}
/* Need to calculate the idx for the result record */
- /*
-* TBD: Need to get the stride from tflib instead of having to
-* understand the construction of the pointer
-*/
uint64_t tmpidx = alloc_parms.idx;
if (atbls->table_type == TF_TBL_TYPE_EXT)
- tmpidx = (alloc_parms.idx * TF_ACTION_RECORD_SZ) >> 4;
+ tmpidx = TF_ACT_REC_OFFSET_2_PTR(alloc_parms.idx);
else
tmpidx = alloc_parms.idx;
@@ -863,10 +869,7 @@ ulp_mapper_action_alloc_and_set(struct
bnxt_ulp_mapper_parms *parms,
set_parm.data_sz_in_bytes = length / 8;
if (set_parm.type == TF_TBL_TYPE_EXT)
- bnxt_ulp_cntxt_tbl_scope_id_get(parms->ulp_ctx,
- &set_parm.tbl_scope_id);
- else
- set_parm.tbl_scope_id = 0;
+ set_parm.tbl_scope_id = tbl_scope_id;
/* set the table entry */
rc = tf_set_tbl_entry(parms->tfp, &set_parm);
@@ -1396,9 +1399,11 @@ ulp_mapper_index_tbl_process(struct
bnxt_ulp_mapper_parms *parms,
struct tf_alloc_tbl_entry_parms aparms = { 0 };
struct tf_set_tbl_entry_parms sparms = { 0 };
struct tf_free_tbl_entry_parms free_parms = { 0 };
-
+ uint32_t tbl_scope_id;
struct tf *tfp = bnxt_ulp_cntxt_tfp_get(parms->ulp_ctx);
+ bnxt_ulp_cntxt_tbl_scope_id_get(parms->ulp_ctx, &tbl_scope_id);
+
if (!ulp_blob_init(&data, tbl->result_bit_size, parms->order)) {
BNXT_TF_DBG(ERR, "Failed initial index table blob\n");
return -EINVAL;
@@ -1427,6 +1432,7 @@ ulp_mapper_index_tbl_process(struct bnxt_ulp_mapper_parms
*parms,
aparms.search_enable= tbl->srch_b4_alloc;
aparms.result = ulp_blob_data_get(&data, &tmplen);
aparms.result_sz_in_bytes = ULP_SZ_BITS2BYTES(tbl->result_bit_size);
+ aparms.tbl_scope_id = tbl_scope_id;
/* All failures after the alloc succeeds require a free */
rc = tf_alloc_tbl_entry(tfp, &aparms);
@@ -1454,6 +1460,7 @@ ulp_mapper_index_tbl_process(struct bnxt_ulp_mapper_parms
*parms,
sparms.data_sz_in_bytes =
ULP_SZ_BITS2BYTES(tbl->result_bit_size);
sparms.idx = aparms.idx;
+ sparms.tbl_scope_id = tbl_scope_id;
rc = tf_set_tbl_entry(tfp, &sparm
[dpdk-dev] [PATCH v2 4/4] net/bnxt: add truflow flush-timer to alloc table scope API
From: Shahaji Bhosle
Updated the params list to include flush timer, this will
allow users to set the HW flush timer value in 10th of second.
Setting 0 will disable the pending cache flush feature.
Signed-off-by: Shahaji Bhosle
Signed-off-by: Randy Schacher
Signed-off-by: Venkat Duvvuru
Reviewed-by: Mike Baucom
Reviewed-by: Ajit Khaparde
---
drivers/net/bnxt/tf_core/tf_core.h | 6 ++
drivers/net/bnxt/tf_core/tf_msg.c | 3 +++
drivers/net/bnxt/tf_core/tf_msg.h | 1 +
drivers/net/bnxt/tf_core/tf_tbl.c | 1 +
drivers/net/bnxt/tf_ulp/bnxt_ulp.c | 6 ++
5 files changed, 17 insertions(+)
diff --git a/drivers/net/bnxt/tf_core/tf_core.h
b/drivers/net/bnxt/tf_core/tf_core.h
index 4b60973ee..1eedd80e7 100644
--- a/drivers/net/bnxt/tf_core/tf_core.h
+++ b/drivers/net/bnxt/tf_core/tf_core.h
@@ -560,6 +560,12 @@ struct tf_alloc_tbl_scope_parms {
* [in] Brd4 only receive table access interface id
*/
uint32_t tx_tbl_if_id;
+ /**
+* [in] Flush pending HW cached flows every 1/10th of value
+* set in seconds, both idle and active flows are flushed
+* from the HW cache. If set to 0, this feature will be disabled.
+*/
+ uint8_t hw_flow_cache_flush_timer;
/**
* [out] table scope identifier
*/
diff --git a/drivers/net/bnxt/tf_core/tf_msg.c
b/drivers/net/bnxt/tf_core/tf_msg.c
index bdf8f155f..beecafdeb 100644
--- a/drivers/net/bnxt/tf_core/tf_msg.c
+++ b/drivers/net/bnxt/tf_core/tf_msg.c
@@ -978,6 +978,7 @@ int tf_msg_em_cfg(struct tf *tfp,
uint16_t key1_ctx_id,
uint16_t record_ctx_id,
uint16_t efc_ctx_id,
+ uint8_tflush_interval,
intdir)
{
int rc;
@@ -993,6 +994,8 @@ int tf_msg_em_cfg(struct tf *tfp,
req.flags = tfp_cpu_to_le_32(flags);
req.num_entries = tfp_cpu_to_le_32(num_entries);
+ req.flush_interval = flush_interval;
+
req.key0_ctx_id = tfp_cpu_to_le_16(key0_ctx_id);
req.key1_ctx_id = tfp_cpu_to_le_16(key1_ctx_id);
req.record_ctx_id = tfp_cpu_to_le_16(record_ctx_id);
diff --git a/drivers/net/bnxt/tf_core/tf_msg.h
b/drivers/net/bnxt/tf_core/tf_msg.h
index b8d8c1ede..030d1881e 100644
--- a/drivers/net/bnxt/tf_core/tf_msg.h
+++ b/drivers/net/bnxt/tf_core/tf_msg.h
@@ -152,6 +152,7 @@ int tf_msg_em_cfg(struct tf *tfp,
uint16_t key1_ctx_id,
uint16_t record_ctx_id,
uint16_t efc_ctx_id,
+ uint8_t flush_interval,
int dir);
/**
diff --git a/drivers/net/bnxt/tf_core/tf_tbl.c
b/drivers/net/bnxt/tf_core/tf_tbl.c
index 236affe25..93f387e86 100644
--- a/drivers/net/bnxt/tf_core/tf_tbl.c
+++ b/drivers/net/bnxt/tf_core/tf_tbl.c
@@ -1500,6 +1500,7 @@ tf_alloc_eem_tbl_scope(struct tf *tfp,
em_tables[KEY1_TABLE].ctx_id,
em_tables[RECORD_TABLE].ctx_id,
em_tables[EFC_TABLE].ctx_id,
+ parms->hw_flow_cache_flush_timer,
dir);
if (rc) {
PMD_DRV_LOG(ERR,
diff --git a/drivers/net/bnxt/tf_ulp/bnxt_ulp.c
b/drivers/net/bnxt/tf_ulp/bnxt_ulp.c
index f8047f0d6..a9cc92d34 100644
--- a/drivers/net/bnxt/tf_ulp/bnxt_ulp.c
+++ b/drivers/net/bnxt/tf_ulp/bnxt_ulp.c
@@ -121,6 +121,12 @@ bnxt_init_tbl_scope_parms(struct bnxt *bp,
else
dparms = bnxt_ulp_device_params_get(dev_id);
+ /*
+* Set the flush timer for EEM entries. The value is in 100ms intervals,
+* so 100 is 10s.
+*/
+ params->hw_flow_cache_flush_timer = 100;
+
if (!dparms) {
params->rx_max_key_sz_in_bits = BNXT_ULP_DFLT_RX_MAX_KEY;
params->rx_max_action_entry_sz_in_bits =
--
2.21.1 (Apple Git-122.3)
[dpdk-dev] [PATCH v2 2/4] net/bnxt: update action record external pool
From: Farah Smith
- Added support variable sized action records
- Additional error checking on table scope params
- Single external pool supported per direction
- Changed to return action record pointer
- Allows action pool to fully utilize the number of flows
Signed-off-by: Farah Smith
Signed-off-by: Mike Baucom
Reviewed-by: Peter Spreadborough
Reviewed-by: Kishore Padmanabha
---
drivers/net/bnxt/tf_core/tf_core.c| 3 -
drivers/net/bnxt/tf_core/tf_core.h| 15 +--
drivers/net/bnxt/tf_core/tf_rm.c | 3 -
drivers/net/bnxt/tf_core/tf_session.h | 6 --
drivers/net/bnxt/tf_core/tf_tbl.c | 136 ++
drivers/net/bnxt/tf_core/tf_tbl.h | 4 +-
6 files changed, 62 insertions(+), 105 deletions(-)
diff --git a/drivers/net/bnxt/tf_core/tf_core.c
b/drivers/net/bnxt/tf_core/tf_core.c
index fc7d6381f..cf9f36adb 100644
--- a/drivers/net/bnxt/tf_core/tf_core.c
+++ b/drivers/net/bnxt/tf_core/tf_core.c
@@ -175,9 +175,6 @@ tf_open_session(struct tf*tfp,
/* Setup hash seeds */
tf_seeds_init(session);
- /* Initialize external pool data structures */
- tf_init_tbl_pool(session);
-
session->ref_count++;
/* Return session ID */
diff --git a/drivers/net/bnxt/tf_core/tf_core.h
b/drivers/net/bnxt/tf_core/tf_core.h
index 6a1f3a106..4b60973ee 100644
--- a/drivers/net/bnxt/tf_core/tf_core.h
+++ b/drivers/net/bnxt/tf_core/tf_core.h
@@ -83,7 +83,7 @@ enum tf_mem {
/** EEM record AR helper
*
- * Helpers to handle the Action Record Pointer in the EEM Record Entry.
+ * Helper to handle the Action Record Pointer in the EEM Record Entry.
*
* Convert absolute offset to action record pointer in EEM record entry
* Convert action record pointer in EEM record entry to absolute offset
@@ -91,8 +91,6 @@ enum tf_mem {
#define TF_ACT_REC_OFFSET_2_PTR(offset) ((offset) >> 4)
#define TF_ACT_REC_PTR_2_OFFSET(offset) ((offset) << 4)
-#define TF_ACT_REC_INDEX_2_OFFSET(idx) ((idx) << 9)
-
/*
* Helper Macros
*/
@@ -943,8 +941,6 @@ enum tf_tbl_type {
* scope. Internal types are not.
*/
TF_TBL_TYPE_EXT,
- /** Future - external pool of size0 entries */
- TF_TBL_TYPE_EXT_0,
TF_TBL_TYPE_MAX
};
@@ -959,6 +955,10 @@ struct tf_alloc_tbl_entry_parms {
* [in] Type of the allocation
*/
enum tf_tbl_type type;
+ /**
+* [in] Table scope identifier (ignored unless TF_TBL_TYPE_EXT)
+*/
+ uint32_t tbl_scope_id;
/**
* [in] Enable search for matching entry. If the table type is
* internal the shadow copy will be searched before
@@ -1028,6 +1028,10 @@ struct tf_free_tbl_entry_parms {
* [in] Type of the allocation type
*/
enum tf_tbl_type type;
+ /**
+* [in] Table scope identifier (ignored unless TF_TBL_TYPE_EXT)
+*/
+ uint32_t tbl_scope_id;
/**
* [in] Index to free
*/
@@ -1070,7 +1074,6 @@ int tf_free_tbl_entry(struct tf *tfp,
struct tf_set_tbl_entry_parms {
/**
* [in] Table scope identifier
-*
*/
uint32_t tbl_scope_id;
/**
diff --git a/drivers/net/bnxt/tf_core/tf_rm.c b/drivers/net/bnxt/tf_core/tf_rm.c
index a5e96f29b..38b1e71cd 100644
--- a/drivers/net/bnxt/tf_core/tf_rm.c
+++ b/drivers/net/bnxt/tf_core/tf_rm.c
@@ -3104,7 +3104,6 @@ tf_rm_lookup_tbl_type_pool(struct tf_session *tfs,
break;
/* No bitalloc pools for these types */
case TF_TBL_TYPE_EXT:
- case TF_TBL_TYPE_EXT_0:
default:
break;
}
@@ -3211,7 +3210,6 @@ tf_rm_convert_tbl_type(enum tf_tbl_type type,
case TF_TBL_TYPE_ACT_MODIFY_IPV6_SRC:
case TF_TBL_TYPE_VNIC_SVIF:
case TF_TBL_TYPE_EXT: /* No pools for this type */
- case TF_TBL_TYPE_EXT_0: /* No pools for this type */
default:
*hcapi_type = -1;
rc = -EOPNOTSUPP;
@@ -3277,7 +3275,6 @@ tf_rm_convert_index(struct tf_session *tfs,
/* Not yet supported */
case TF_TBL_TYPE_VNIC_SVIF:
case TF_TBL_TYPE_EXT: /* No pools for this type */
- case TF_TBL_TYPE_EXT_0: /* No pools for this type */
default:
return -EOPNOTSUPP;
}
diff --git a/drivers/net/bnxt/tf_core/tf_session.h
b/drivers/net/bnxt/tf_core/tf_session.h
index fed34f146..50ef2d530 100644
--- a/drivers/net/bnxt/tf_core/tf_session.h
+++ b/drivers/net/bnxt/tf_core/tf_session.h
@@ -289,12 +289,6 @@ struct tf_session {
/** Table scope array */
struct tf_tbl_scope_cb tbl_scopes[TF_NUM_TBL_SCOPE];
-
- /** Each external pool is associated with a single table scope
-* For each external pool store the associated table scope in
-* this data structure
-*/
- uint32_t ext_pool_2_scope[TF_DIR_MAX][TF_EXT_POOL_CNT_MAX];
};
#endif /* _TF_SESSION_H_ */
diff --git a/drivers
Re: [dpdk-dev] [PATCH v5 1/1] eal: add internal ABI marking support
On Sat, Apr 25, 2020 at 8:10 AM Wang, Haiyue wrote: > > Hi David, > > Try to fix the issues you mentioned, except below, plan to > another patch set, I need more time to test these adding. Thanks for working on this topic. > > > > We are missing updates on devtools/check-abi-version.sh and > devtools/update_version_map_abi.py. Those two scripts can be updated later: - I suspect the first one to be broken already, - the 2nd one is for 20.11 when we will update all map files. > > More importantly on this file: > > - drivers/meson.build is not updated to check for internal symbols, see: > https://git.dpdk.org/dpdk/tree/drivers/meson.build#n166 On this point, your series is almost good to go, I would just get rid of the "experimental" mentions. I will reply on the patch. > - For fully experimental libraries, we have a special so version: > https://git.dpdk.org/dpdk/tree/drivers/meson.build#n131 > > This will apply to common drivers that will be 100% internal. > Not sure if this is an issue. This part should be fine, I want others to be aware of this. -- David Marchand
Re: [dpdk-dev] [PATCH v5 1/1] eal: add internal ABI marking support
25/04/2020 16:21, David Marchand: > On Sat, Apr 25, 2020 at 8:10 AM Wang, Haiyue wrote: > > - For fully experimental libraries, we have a special so version: > > https://git.dpdk.org/dpdk/tree/drivers/meson.build#n131 > > > > This will apply to common drivers that will be 100% internal. > > Not sure if this is an issue. > > This part should be fine, I want others to be aware of this. I am not one of the ABI maintainers, but in my opinion it is OK to have "pure internal" libs with version 0.x.
Re: [dpdk-dev] [PATCH v7 3/6] mk: add internal tag check
On Sat, Apr 25, 2020 at 1:02 PM Haiyue Wang wrote:
>
> Add checks during build to ensure that all symbols in the INTERNAL
> version map section have __internal tags on their definitions, and
> enable the warnings needed to announce their use.
>
> Signed-off-by: Haiyue Wang
> ---
> MAINTAINERS | 2 +-
> ...-experimental-syms.sh => check-symbols.sh} | 31 +++
> buildtools/meson.build| 2 +-
> mk/internal/rte.compile-pre.mk| 6 ++--
> 4 files changed, 36 insertions(+), 5 deletions(-)
> rename buildtools/{check-experimental-syms.sh => check-symbols.sh} (61%)
Just missing a little update on drivers/meson.build and lib/meson.build.
Squashed with:
diff --git a/buildtools/meson.build b/buildtools/meson.build
index 3e8d31b0c5..d5f8291beb 100644
--- a/buildtools/meson.build
+++ b/buildtools/meson.build
@@ -6,7 +6,7 @@ subdir('pmdinfogen')
pkgconf = find_program('pkg-config', 'pkgconf', required: false)
pmdinfo = find_program('gen-pmdinfo-cfile.sh')
list_dir_globs = find_program('list-dir-globs.py')
-check_experimental_syms = find_program('check-symbols.sh')
+check_symbols = find_program('check-symbols.sh')
ldflags_ibverbs_static = find_program('options-ibverbs-static.sh')
# set up map-to-def script using python, either built-in or external
@@ -20,4 +20,4 @@ map_to_def_cmd = py3 + files('map_to_def.py')
sphinx_wrapper = py3 + files('call-sphinx-build.py')
# stable ABI always starts with "DPDK_"
-is_experimental_cmd = [find_program('grep', 'findstr'), '^DPDK_']
+is_stable_cmd = [find_program('grep', 'findstr'), '^DPDK_']
diff --git a/drivers/meson.build b/drivers/meson.build
index f3dd23dd43..dc293b270b 100644
--- a/drivers/meson.build
+++ b/drivers/meson.build
@@ -131,15 +131,15 @@ foreach class:dpdk_driver_classes
meson.current_source_dir(),
drv_path, lib_name)
- is_experimental = run_command(is_experimental_cmd,
- files(version_map)).returncode()
+ is_stable = run_command(is_stable_cmd,
+ files(version_map)).returncode() == 0
- if is_experimental != 0
- lib_version = experimental_abi_version
- so_version = experimental_so_version
- else
+ if is_stable
lib_version = abi_version
so_version = stable_so_version
+ else
+ lib_version = experimental_abi_version
+ so_version = experimental_so_version
endif
# now build the static driver
@@ -168,14 +168,14 @@ foreach class:dpdk_driver_classes
else
lk_args = ['-Wl,--version-script=' +
version_map]
# on unix systems check the output of the
- # experimental syms script, using it as a
+ # check-symbols.sh script, using it as a
# dependency of the .so build
- lk_deps += custom_target(lib_name + '.exp_chk',
- command: [check_experimental_syms,
+ lk_deps += custom_target(lib_name + '.sym_chk',
+ command: [check_symbols,
version_map, '@INPUT@'],
capture: true,
input: static_lib,
- output: lib_name + '.exp_chk')
+ output: lib_name + '.sym_chk')
endif
shared_lib = shared_library(lib_name,
diff --git a/lib/meson.build b/lib/meson.build
index 8697941ae0..07a65a6256 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -109,15 +109,15 @@ foreach l:libraries
version_map = '@0@/@1@/rte_@2@_version.map'.format(
meson.current_source_dir(),
dir_name, name)
- is_experimental = run_command(is_experimental_cmd,
- files(version_map)).returncode()
+ is_stable = run_command(is_stable_cmd,
+ files(version_map)).returncode() == 0
- if is_experimental != 0
- lib_version = experimental_abi_version
- so_version = experimental_so_version
- else
+ if is_stable
lib_version = abi_version
Re: [dpdk-dev] [PATCH v7 5/6] devtools: exempt internal ABI checking
On Sat, Apr 25, 2020 at 1:02 PM Haiyue Wang wrote:
>
> No need to restrict the ABI on symbols that are only used by core
> libraries.
>
> Signed-off-by: Haiyue Wang
Rather than add a special case for INTERNAL, we can invert the logic
in this script: identify "stable" sections symbol.
I went with the following patch:
diff --git a/devtools/check-symbol-change.sh b/devtools/check-symbol-change.sh
index ed2178e36e..f329d5fa62 100755
--- a/devtools/check-symbol-change.sh
+++ b/devtools/check-symbol-change.sh
@@ -77,6 +77,10 @@ build_map_changes()
}
+is_stable_section() {
+ [ "$1" != 'EXPERIMENTAL' ] && [ "$1" != 'INTERNAL' ]
+}
+
check_for_rule_violations()
{
local mapdb="$1"
@@ -110,11 +114,11 @@ check_for_rule_violations()
# section directly
if [ -z "$oldsecname" ]
then
- if [ "$secname" = 'EXPERIMENTAL' ]
+ if ! is_stable_section $secname
then
echo -n "INFO: symbol $symname has "
echo -n "been added to the "
- echo -n "EXPERIMENTAL section of the "
+ echo -n "$secname section of the "
echo "version map"
continue
else
@@ -137,7 +141,7 @@ check_for_rule_violations()
# This symbol is moving between two sections (the
# original section is not experimental).
# This can be legit, just warn.
- if [ "$oldsecname" != 'EXPERIMENTAL' ]
+ if is_stable_section $oldsecname
then
echo -n "INFO: symbol $symname is being "
echo -n "moved from $oldsecname to $secname. "
@@ -148,9 +152,9 @@ check_for_rule_violations()
else
if ! grep -q "$mname $symname .* add" "$mapdb" && \
- [ "$secname" != "EXPERIMENTAL" ]
+ is_stable_section $secname
then
- # Just inform users that non-experimenal
+ # Just inform users that stable
# symbols need to go through a deprecation
# process
echo -n "INFO: symbol $symname is being "
--
David Marchand
Re: [dpdk-dev] [PATCH v2] lib/timer: relax barrier for status update
20/04/2020 18:05, Phil Yang: > This patch depends on patch: > http://patchwork.dpdk.org/patch/65997/ In order to ease patch tracking, you should have kept the first patch in the next version of your series. We don't split series in general.
Re: [dpdk-dev] [PATCH v7 0/6] dpdk: introduce __rte_internal tag
On Sat, Apr 25, 2020 at 1:02 PM Haiyue Wang wrote:
>
> Move the internal function into INTERNAL session to avoid the ABI
> checking, and it is only used for DPDK drivers or related library.
>
> __rte_internal funA
>
> INTERNAL {
> global:
>
> funA
> };
Thanks a lot for working on this.
I did some modifications (see my replies on patch 3 and 5) and applied
this series.
We are just missing the update on the scripts mentioned in a previous mail.
Can you work on this for rc2?
Thanks again!
--
David Marchand
Re: [dpdk-dev] [PATCH v2] bus/pci: support iova=va on PowerNV systems
On Mon, Mar 16, 2020 at 9:38 PM David Christensen wrote: > > All recent POWER systems, Power 8 and 9 specifically, support an IOMMU > (it can't be disabled). The functionality of the IOMMU is different > depending on whether it's running on a bare metal PowerNV system or in > a virtual environment (PowerVM LPAR or KVM/QEMU). DPDK currently > supports the IOMMU found on PowerNV platforms, sPAPRv2, so IOVA=VA > mode can be enabled when the correct platform is detected. > > The POWER IOMMU type can't be detected through mechansims such as mechanisms > parsing files in the /sys heirarchy like x86_64 systems so the hierarchy > /proc/cpuinfo file is parsed to determine whether Linux is running > on bare metal (i.e. PowerNV) or in a virtual environment (KVM/QEMU). > > Signed-off-by: David Christensen Applied, thanks. -- David Marchand
Re: [dpdk-dev] [PATCH v2] lib/timer: relax barrier for status update
> -Original Message- > From: Thomas Monjalon > Sent: Saturday, April 25, 2020 10:36 PM > To: Phil Yang > Cc: erik.g.carri...@intel.com; rsanf...@akamai.com; > david.march...@redhat.com; konstantin.anan...@intel.com; > jer...@marvell.com; hemant.agra...@nxp.com; Honnappa Nagarahalli > ; Gavin Hu ; nd > ; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2] lib/timer: relax barrier for status update > > 20/04/2020 18:05, Phil Yang: > > This patch depends on patch: > > http://patchwork.dpdk.org/patch/65997/ > > In order to ease patch tracking, you should have kept the first patch > in the next version of your series. We don't split series in general. Thanks for reminding me. I will update the patch series in the new version. Thanks, Phil > >
Re: [dpdk-dev] [PATCH 10/14] tap: close netlink socket on device close
On Sat, Jan 4, 2020 at 2:35 AM Stephen Hemminger
wrote:
>
> The netlink socket for flow creation was left open and never
> closed.
>
> Fixes: bf7b7f437b49 ("net/tap: create netdevice during probing")
> Cc: pascal.ma...@6wind.com
> Cc: sta...@dpdk.org
> Signed-off-by: Stephen Hemminger
Afaics, superseded by the "fixes for tap" series recently merged.
http://patchwork.dpdk.org/cover/68602/
--
David Marchand
Re: [dpdk-dev] [PATCH 12/14] ethdev: raise priority of old driver warning
04/01/2020 02:33, Stephen Hemminger: > The priority of the message about drivers not using new (correct) > behaviour on close was debug. And debug messages are typically surpressed > and never seen. Raise the priority so that broken drivers are visible > and hopefully get developers to fix. > > Signed-off-by: Stephen Hemminger > --- > --- a/lib/librte_ethdev/rte_ethdev.c > +++ b/lib/librte_ethdev/rte_ethdev.c > @@ -1717,7 +1717,7 @@ rte_eth_dev_close(uint16_t port_id) > - RTE_ETHDEV_LOG(DEBUG, "Port closing is using an old behaviour.\n" > + RTE_ETHDEV_LOG(NOTICE, "Port closing is using an old behaviour.\n" Acked-by: Thomas Monjalon PS: I did not notice this patch earlier. Please, next time, Cc maintainers with this git-send-email option: --cc-cmd devtools/get-maintainer.sh
Re: [dpdk-dev] [PATCH v4] eal/cpuflags: add x86 based cpu flags
16/04/2020 13:00, Kevin Laatz: > This patch adds CPU flags which will enable the detection of ISA > features available on more recent x86 based CPUs. [...] > --- a/devtools/libabigail.abignore > +++ b/devtools/libabigail.abignore > +; Ignore this enum update as it should not be allocated by the application > +[suppress_type] > + type_kind = enum > + name = rte_cpu_flag_t > + changed_enumerators = RTE_CPUFLAG_NUMFLAGS The justification is not correct. The application is allowed to use RTE_CPUFLAG_NUMFLAGS in array allocation. But no API is returning a CPU flag, so the new flags will remain unknown to the application. However, there is a behaviour change: The functions rte_cpu_get_flag_name() and rte_cpu_get_flag_enabled() will now accept new values, which were previously considered as an error. Is it an ABI breakage? I would say no. PS: Who is REALLY maintaining the ABI? We really miss someone who carefully check all these things, and take care of the doc and tooling.
Re: [dpdk-dev] [PATCH v2] lib/timer: relax barrier for status update
25/04/2020 17:51, Phil Yang: > From: Thomas Monjalon > > 20/04/2020 18:05, Phil Yang: > > > This patch depends on patch: > > > http://patchwork.dpdk.org/patch/65997/ > > > > In order to ease patch tracking, you should have kept the first patch > > in the next version of your series. We don't split series in general. > > Thanks for reminding me. > I will update the patch series in the new version. No that's fine, I'm merging it already. Next time :-)
Re: [dpdk-dev] [PATCH 02/14] eal: log: free dynamic state on cleanup
On Sat, Jan 4, 2020 at 2:34 AM Stephen Hemminger
wrote:
>
> When rte_eal_cleanup is called, free all the memory
> associated with dynamic log levels and types.
>
> Fixes: c1b5fa94a46f ("eal: support dynamic log types")
> Cc: olivier.m...@6wind.com
> Signed-off-by: Stephen Hemminger
> ---
> lib/librte_eal/common/eal_common_log.c | 18 +-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_eal/common/eal_common_log.c
> b/lib/librte_eal/common/eal_common_log.c
> index 64d6e20947ed..7583bdc57619 100644
> --- a/lib/librte_eal/common/eal_common_log.c
> +++ b/lib/librte_eal/common/eal_common_log.c
> @@ -470,8 +471,23 @@ eal_log_set_default(FILE *default_log)
> void
> eal_log_cleanup(void)
> {
> + struct rte_eal_opt_loglevel *opt_ll, *tmp;
> + size_t i;
> +
> if (default_log_stream) {
> fclose(default_log_stream);
> default_log_stream = NULL;
> }
> +
> + TAILQ_FOREACH_SAFE(opt_ll, &opt_loglevel_list, next, tmp) {
> + free(opt_ll->pattern);
In regexp case, we have a leak on the regexp buffer.
Fixed with:
+if (opt_ll->pattern != NULL)
+free(opt_ll->pattern);
+else
+regfree(&opt_ll->re_match);
> + free(opt_ll);
> + }
> +
> + for (i = 0; i < rte_logs.dynamic_types_len; i++)
> + free(rte_logs.dynamic_types[i].name);
> +
> + rte_logs.dynamic_types_len = 0;
> + free(rte_logs.dynamic_types);
> + rte_logs.dynamic_types = NULL;
> }
> --
> 2.20.1
>
--
David Marchand
Re: [dpdk-dev] [PATCH 03/14] eal: alarm: close timerfd on eal cleanup
On Sat, Jan 4, 2020 at 2:34 AM Stephen Hemminger
wrote:
>
> Calling rte_eal_cleanup() should cause DPDK to cleanup all
> outstanding resources including file descriptors.
>
> Signed-off-by: Stephen Hemminger
> ---
> lib/librte_eal/common/eal_private.h | 7 +++
> lib/librte_eal/linux/eal/eal.c | 1 +
> lib/librte_eal/linux/eal/eal_alarm.c | 11 +++
> 3 files changed, 19 insertions(+)
I won't merge this as my FreeBSD vm is broken but I suppose the bits
for FreeBSD would be:
diff --git a/lib/librte_eal/freebsd/eal.c b/lib/librte_eal/freebsd/eal.c
index 540b7d38c5..582ff0920a 100644
--- a/lib/librte_eal/freebsd/eal.c
+++ b/lib/librte_eal/freebsd/eal.c
@@ -973,6 +973,7 @@ int
rte_eal_cleanup(void)
{
rte_service_finalize();
+ rte_eal_alarm_cleanup();
rte_mp_channel_cleanup();
rte_trace_save();
eal_trace_fini();
diff --git a/lib/librte_eal/freebsd/eal_alarm.c
b/lib/librte_eal/freebsd/eal_alarm.c
index c38b2e04f8..b2089d0b53 100644
--- a/lib/librte_eal/freebsd/eal_alarm.c
+++ b/lib/librte_eal/freebsd/eal_alarm.c
@@ -61,6 +61,16 @@ rte_eal_alarm_init(void)
return 0;
}
+void
+rte_eal_alarm_cleanup(void)
+{
+ if (intr_handle.fd == -1)
+ return;
+
+ close(intr_handle.fd);
+ intr_handle.fd = -1;
+}
+
static inline int
timespec_cmp(const struct timespec *now, const struct timespec *at)
{
--
David Marchand
Re: [dpdk-dev] [PATCH v7 0/6] dpdk: introduce __rte_internal tag
> -Original Message-
> From: David Marchand
> Sent: Saturday, April 25, 2020 22:39
> To: Wang, Haiyue
> Cc: dev ; Thomas Monjalon ; Richardson,
> Bruce
> ; Yigit, Ferruh ; Neil
> Horman
> ; Ray Kinsella
> Subject: Re: [PATCH v7 0/6] dpdk: introduce __rte_internal tag
>
> On Sat, Apr 25, 2020 at 1:02 PM Haiyue Wang wrote:
> >
> > Move the internal function into INTERNAL session to avoid the ABI
> > checking, and it is only used for DPDK drivers or related library.
> >
> > __rte_internal funA
> >
> > INTERNAL {
> > global:
> >
> > funA
> > };
>
> Thanks a lot for working on this.
> I did some modifications (see my replies on patch 3 and 5) and applied
> this series.
>
> We are just missing the update on the scripts mentioned in a previous mail.
> Can you work on this for rc2?
>
Sure, it's my pleasure. ;-)
> Thanks again!
>
>
> --
> David Marchand
Re: [dpdk-dev] [PATCH 05/14] eal: intr: cleanup resources
On Sat, Jan 4, 2020 at 2:34 AM Stephen Hemminger
wrote:
>
> When rte_eal_cleanup is called the interrupt thread and
> associated resources should be cleaned up.
>
> Signed-off-by: Stephen Hemminger
> ---
> lib/librte_eal/common/eal_private.h | 10 ++
> lib/librte_eal/linux/eal/eal.c| 1 +
> lib/librte_eal/linux/eal/eal_interrupts.c | 9 +
> 3 files changed, 20 insertions(+)
>
> diff --git a/lib/librte_eal/common/eal_private.h
> b/lib/librte_eal/common/eal_private.h
> index 38682e79827c..c62f35d3ac0f 100644
> --- a/lib/librte_eal/common/eal_private.h
> +++ b/lib/librte_eal/common/eal_private.h
> @@ -191,6 +191,16 @@ int rte_eal_tailqs_init(void);
> */
> int rte_eal_intr_init(void);
>
> +/**
> + * Cleanup interrupt handling.
> + *
> + * This function is private to EAL.
> + *
> + * @return
> + * 0 on success, negative on error
> + */
> +void rte_eal_intr_cleanup(void);
> +
> /**
> * Init alarm mechanism. This is to allow a callback be called after
> * specific time.
> diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
> index d98a2afe85da..eb95f4f0c317 100644
> --- a/lib/librte_eal/linux/eal/eal.c
> +++ b/lib/librte_eal/linux/eal/eal.c
> @@ -1338,6 +1338,7 @@ rte_eal_cleanup(void)
> }
>
> rte_service_finalize();
> + rte_eal_intr_cleanup();
> rte_eal_alarm_cleanup();
> rte_mp_channel_cleanup();
> eal_cleanup_config(&internal_config);
> diff --git a/lib/librte_eal/linux/eal/eal_interrupts.c
> b/lib/librte_eal/linux/eal/eal_interrupts.c
> index 14ebb108cee9..fa08ac4171bd 100644
> --- a/lib/librte_eal/linux/eal/eal_interrupts.c
> +++ b/lib/librte_eal/linux/eal/eal_interrupts.c
> @@ -1137,6 +1137,15 @@ rte_eal_intr_init(void)
> return ret;
> }
>
> +void
> +rte_eal_intr_cleanup(void)
> +{
> + pthread_cancel(intr_thread);
> + pthread_join(intr_thread, NULL);
> + close(intr_pipe.readfd);
> + close(intr_pipe.writefd);
What happens to the intr_sources callbacks?
I am unsure we can expect the application to clean this before the eal cleanup.
It would be worth a followup patch.
> +}
> +
> static void
> eal_intr_proc_rxtx_intr(int fd, const struct rte_intr_handle *intr_handle)
> {
> --
> 2.20.1
>
--
David Marchand
Re: [dpdk-dev] [PATCH v3] lib/timer: relax barrier for status update
24/04/2020 09:24, Phil Yang: > Volatile has no ordering semantics. The rte_timer structure defines > timer status as a volatile variable and uses the rte_r/wmb barrier > to guarantee inter-thread visibility. > > This patch optimized the volatile operation with c11 atomic operations > and one-way barrier to save the performance penalty. According to the > timer_perf_autotest benchmarking results, this patch can uplift 10%~16% > timer appending performance, 3%~20% timer resetting performance and 45% > timer callbacks scheduling performance on aarch64 and no loss in > performance for x86. > > Suggested-by: Honnappa Nagarahalli > Signed-off-by: Phil Yang > Reviewed-by: Gavin Hu > Acked-by: Erik Gabriel Carrillo [...] > --- a/lib/librte_timer/rte_timer.h > +++ b/lib/librte_timer/rte_timer.h > @@ -101,7 +101,7 @@ struct rte_timer > - volatile union rte_timer_status status; /**< Status of timer. */ > + union rte_timer_status status; /**< Status of timer. */ Unfortunately, I cannot merge this patch because it breaks the ABI: [C]'function void rte_timer_init(rte_timer*)' at rte_timer.c:214:1 has some indirect sub-type changes: parameter 1 of type 'rte_timer*' has sub-type changes: in pointed to type 'struct rte_timer' at rte_timer.h:100:1: type size hasn't changed 1 data member changes (2 filtered): type of 'volatile rte_timer_status rte_timer::status' changed: entity changed from 'volatile rte_timer_status' to 'union rte_timer_status' at rte_timer.h:67:1 type size hasn't changed
Re: [dpdk-dev] [dpdk-stable] [PATCH 1/2] lib/timer: protect timer subsystem initialized with lock
> > rte_timer_subsystem_initialized is a global variable that can be accessed by
> > multiple processes simultaneously. Hence, any access to
> > rte_timer_subsystem_initialized should be protected by
> > rte_mcfg_timer_lock.
> >
> > Fixes: f9d6cd8bfe9e ("timer: fix resource leak in finalize")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Honnappa Nagarahalli
> > Reviewed-by: Gavin Hu
> > Reviewed-by: Phil Yang
> Acked-by: Erik Gabriel Carrillo
Applied (without patch 2), thanks.
Re: [dpdk-dev] [PATCH v2] doc: use glob terminology
12/04/2020 17:04, jer...@marvell.com: > --- a/devtools/check-includes.sh > +++ b/devtools/check-includes.sh > @@ -23,7 +23,7 @@ > # PEDANTIC_CFLAGS, PEDANTIC_CXXFLAGS and PEDANTIC_CPPFLAGS provide strict > # C/C++ compilation flags. > # > -# IGNORE contains a list of shell patterns matching files (relative to the > +# IGNORE contains a list of glob matching files (relative to the > # include directory) to avoid. It is set by default to known DPDK headers > # which must not be included on their own. > # > diff --git a/lib/librte_eal/include/rte_log.h > b/lib/librte_eal/include/rte_log.h > index a497e195d..fa60177c0 100644 > --- a/lib/librte_eal/include/rte_log.h > +++ b/lib/librte_eal/include/rte_log.h > @@ -158,7 +158,7 @@ __rte_experimental > bool rte_log_can_log(uint32_t logtype, uint32_t loglevel); > > /** > - * Set the log level for a given type based on shell pattern. > + * Set the log level for a given type based on glob. > * > * @param pattern > * The match pattern identifying the log type. My comment on v1 was sent at the same time as this v2. Pasting it here: "match pattern" can be replaced with "globbing pattern". I think there are few other places which can be improved. I see this one in lib/librte_eal/common/eal_common_log.c: glob (file match) pattern I suggest "globbing pattern" And "Glob match string option" -> "Globbing pattern option"
Re: [dpdk-dev] [PATCH] Fix various typos found by Lintian
04/03/2020 16:28, Luca Boccassi: > On Wed, 2020-03-04 at 14:34 +, Kevin Traynor wrote: > > On 29/02/2020 16:37, luca.bocca...@gmail.com wrote: > > > Debian's linter is getting more and more annoy^^smart and now > > > parses binaries > > > for typos too - CC stable to get it off my back in the next release > > > > Minor: Probably linter is better trained in the Queen's English than > > me > > or it could be personal preference, but 'one' seems to be referring > > to > > the user and it reads a bit strange for me. e.g. > > > > "Slave %d capabilities doesn't allow one to allocate additional > > queues" > > "hardware specifications that allow one to handle virtual memory" > > "Do not allow one to send packet if the maximum DMA.." > > > > as opposed to > > > > "Slave %d capabilities don't allow allocation of additional queues" > > "hardware specifications that allow handling of virtual memory" > > "Do not allow sending of a packet if the maximum DMA.." > > You might be right - but the intent here is not to be correct, it's to > get the linter to leave me alone :-) I agree with Kevin that the wording "allow one to make" is strange. Would lintian leave you alone with "allow making"? Anyway the "allow to" sentences are not typos. They could be reworded in a separate patch. Patch partly applied, except the "allow one to" changes, thanks.
Re: [dpdk-dev] [PATCH v2 0/4] introduce changes to support flow scaling
On Sat, Apr 25, 2020 at 7:02 AM Ajit Khaparde wrote: > This patchset introduces changes to the action record allocation, flow > database entry deletion, and hw flow cache updates. Action record > allocation now allows the actions to scale with the flows. > Additionally, resources attached to a flow database entry are now > correctly released when the critical resource has not been added to > the flow. Finally, the hw flow cache has a timer to periodically > invalidate flow entries. > > v1->v2: > Squashed patches 4 & 5 into single patch. > Applied to dpdk-next-net-brcm. > > Farah Smith (1): > net/bnxt: update action record external pool > > Mike Baucom (2): > net/bnxt: reserve a flowdb resource function as invalid > net/bnxt: ulp changes to handle action/index tables > > Shahaji Bhosle (1): > net/bnxt: add truflow flush-timer to alloc table scope API > > drivers/net/bnxt/tf_core/tf_core.c| 3 - > drivers/net/bnxt/tf_core/tf_core.h| 21 +++- > drivers/net/bnxt/tf_core/tf_msg.c | 3 + > drivers/net/bnxt/tf_core/tf_msg.h | 1 + > drivers/net/bnxt/tf_core/tf_rm.c | 3 - > drivers/net/bnxt/tf_core/tf_session.h | 6 - > drivers/net/bnxt/tf_core/tf_tbl.c | 137 -- > drivers/net/bnxt/tf_core/tf_tbl.h | 4 +- > drivers/net/bnxt/tf_ulp/bnxt_ulp.c| 6 + > drivers/net/bnxt/tf_ulp/ulp_mapper.c | 30 +++-- > drivers/net/bnxt/tf_ulp/ulp_template_db.h | 15 +-- > 11 files changed, 106 insertions(+), 123 deletions(-) > > -- > 2.21.1 (Apple Git-122.3) > >
Re: [dpdk-dev] [PATCH v7 0/6] dpdk: introduce __rte_internal tag
Hi David,
> -Original Message-
> From: David Marchand
> Sent: Saturday, April 25, 2020 22:39
> To: Wang, Haiyue
> Cc: dev ; Thomas Monjalon ; Richardson,
> Bruce
> ; Yigit, Ferruh ; Neil
> Horman
> ; Ray Kinsella
> Subject: Re: [PATCH v7 0/6] dpdk: introduce __rte_internal tag
>
> On Sat, Apr 25, 2020 at 1:02 PM Haiyue Wang wrote:
> >
> > Move the internal function into INTERNAL session to avoid the ABI
> > checking, and it is only used for DPDK drivers or related library.
> >
> > __rte_internal funA
> >
> > INTERNAL {
> > global:
> >
> > funA
> > };
>
> Thanks a lot for working on this.
> I did some modifications (see my replies on patch 3 and 5) and applied
> this series.
>
> We are just missing the update on the scripts mentioned in a previous mail.
> Can you work on this for rc2?
>
Do you mean ?
> > > This will apply to common drivers that will be 100% internal.
> > > Not sure if this is an issue.
> >
> > This part should be fine, I want others to be aware of this.
> I am not one of the ABI maintainers, but in my opinion it is OK
> to have "pure internal" libs with version 0.x.
I've tested it with Intel's drivers/common/iavf, it works as expected.
a). librte_common_iavf.so.0.200.2
b). Skipped experimental library librte_common_iavf.dump.
This has been updated by your modification.
+ if is_stable
lib_version = abi_version
so_version = stable_so_version
+ else
+ lib_version = experimental_abi_version
+ so_version = experimental_so_version
endif
> Thanks again!
>
>
> --
> David Marchand
[dpdk-dev] Common netlink parsing?
While adding error handling to tap device, I noticed we already have two places rolling their own netlink message handling (tap and mlx5) and now with proposed IF proxy there is a third. Netlink is non-trivial and easy to get wrong and doing error handling also requires work. There should be a common library for this. My preference would be to use pre-existing code (libmnl) but the DPDK maintainers seem to have an aversion to taking a dependency on any external code and reinvent everything (see RCU etc).
Re: [dpdk-dev] [PATCH 00/14] cleanup resources on shutdown
On Sat, Jan 4, 2020 at 2:34 AM Stephen Hemminger
wrote:
>
> Recently started using valgrind with DPDK, and the results
> are not clean.
>
> The DPDK has a function that applications can use to tell it
> to cleanup resources on shutdown (rte_eal_cleanup). But the
> current coverage of that API is spotty. Many internal parts of
> DPDK leave files and allocated memory behind.
>
> This patch set is a start at getting the sub-parts of
> DPDK to cleanup after themselves. These are the easier ones,
> the harder and more critical ones are in the drivers
> and the memory subsystem.
>
> There are no visible API or ABI changes here.
I was about to push the series (except patch 10), but I hit a crash
when passing an invalid option to test-null.sh.
Reproduced with:
Core was generated by
`/home/dmarchan/builds/x86_64-native-linux-gcc+shared+kmods/app/testpmd
-c 0x3 --log-level='.
Program terminated with signal SIGSEGV, Segmentation fault.
#0 0x7fd5231dba64 in pthread_cancel () from /usr/lib64/libpthread.so.0
Missing separate debuginfos, use: dnf debuginfo-install
elfutils-libelf-0.178-7.fc30.x86_64 glibc-2.29-28.fc30.x86_64
jansson-2.12-2.fc30.x86_64 libgcc-9.2.1-1.fc30.x86_64
libpcap-1.9.1-1.fc30.x86_64 numactl-libs-2.0.12-2.fc30.x86_64
zlib-1.2.11-19.fc30.x86_64
(gdb) bt full
#0 0x7fd5231dba64 in pthread_cancel () from /usr/lib64/libpthread.so.0
No symbol table info available.
#1 0x7fd52320c586 in rte_eal_cleanup () at
/home/dmarchan/dpdk/lib/librte_eal/linux/eal.c:1339
i = 1
#2 0x7fd523215f5e in rte_exit (exit_code=exit_code@entry=1,
format=format@entry=0x47ada4 "Cannot init EAL: %s\n") at
/home/dmarchan/dpdk/lib/librte_eal/linux/eal_debug.c:83
ap = {{gp_offset = 24, fp_offset = 48, overflow_arg_area =
0x7ffecdf7aa70, reg_save_area = 0x7ffecdf7a9a0}}
#3 0x0043535b in main (argc=21, argv=0x7ffecdf7abc8) at
/home/dmarchan/dpdk/app/test-pmd/testpmd.c:3647
diag = -1
port_id =
count =
ret =
(gdb) f 1
#1 0x7fd52320c586 in rte_eal_cleanup () at
/home/dmarchan/dpdk/lib/librte_eal/linux/eal.c:1339
1339pthread_cancel(lcore_config[i].thread_id);
(gdb) p lcore_config[1].thread_id
$1 = 0
rte_eal_cleanup() is called from rte_exit() by testpmd.
But since rte_eal_init() failed at parsing, lcore_config[*].thread_id
are invalid, and we crash on pthread_cancel.
I have no quick idea to fix this, series postponed to rc2.
--
David Marchand
Re: [dpdk-dev] Common netlink parsing?
25/04/2020 21:24, Stephen Hemminger: > While adding error handling to tap device, I noticed we already have two > places rolling their own netlink message handling (tap and mlx5) and now > with proposed IF proxy there is a third. > > Netlink is non-trivial and easy to get wrong and doing error handling > also requires work. There should be a common library for this. > > My preference would be to use pre-existing code (libmnl) but the > DPDK maintainers seem to have an aversion to taking a dependency on > any external code and reinvent everything (see RCU etc). We should avoid reinventing wheels. About RCU, I think it was said no library meets the same requirements. Honnappa, any comment? About libmnl, it was used in mlx5 some time ago. It has been removed when it was thought it is not required anymore. Maybe you could demonstrate libmnl benefit by integrating it with the tap PMD first.
Re: [dpdk-dev] [PATCH v9 1/6] lib/eal: implement the family of common bit operation APIs
24/04/2020 05:21, Joyce Kong: > Bitwise operation APIs are defined and used in a lot of PMDs, > which caused a huge code duplication. To reduce duplication, > this patch consolidates them into a common API family. [...] > +rte_get_bit32_relaxed(unsigned int nr, volatile uint32_t *addr) > +rte_set_bit32_relaxed(unsigned int nr, volatile uint32_t *addr) > +rte_clear_bit32_relaxed(unsigned int nr, volatile uint32_t *addr) > +rte_test_and_set_bit32_relaxed(unsigned int nr, volatile uint32_t *addr) > +rte_test_and_clear_bit32_relaxed(unsigned int nr, volatile uint32_t *addr) > +rte_get_bit64_relaxed(unsigned int nr, volatile uint64_t *addr) > +rte_set_bit64_relaxed(unsigned int nr, volatile uint64_t *addr) > +rte_clear_bit64_relaxed(unsigned int nr, volatile uint64_t *addr) > +rte_test_and_set_bit64_relaxed(unsigned int nr, volatile uint64_t *addr) > +rte_test_and_clear_bit64_relaxed(unsigned int nr, volatile uint64_t *addr) Sorry, I have one more naming concern with this series. I prefer a common namespace for bit operations. Would you be OK to prefix all function names with rte_bit_relaxed_?
Re: [dpdk-dev] [PATCH v4 1/4] ethdev: add tm support for shaper config in pkt mode
On 4/24/2020 11:28 AM, Dumitrescu, Cristian wrote: > > >> -Original Message- >> From: Nithin Dabilpuram >> Sent: Wednesday, April 22, 2020 6:21 PM >> To: Singh, Jasvinder ; Dumitrescu, Cristian >> ; Thomas Monjalon >> ; Yigit, Ferruh ; Andrew >> Rybchenko >> Cc: dev@dpdk.org; jer...@marvell.com; kka...@marvell.com; Nithin >> Dabilpuram >> Subject: [PATCH v4 1/4] ethdev: add tm support for shaper config in pkt >> mode >> >> From: Nithin Dabilpuram >> >> Some NIC hardware support shaper to work in packet mode i.e >> shaping or ratelimiting traffic is in packets per second (PPS) as >> opposed to default bytes per second (BPS). Hence this patch >> adds support to configure shared or private shaper in packet mode, >> provide rate in PPS and add related tm capabilities in port/level/node >> capability structures. >> >> This patch also updates tm port/level/node capability structures with >> exiting features of scheduler wfq packet mode, scheduler wfq byte mode >> and private/shared shaper byte mode. >> >> SoftNIC PMD is also updated with new capabilities. >> >> Signed-off-by: Nithin Dabilpuram >> --- >> v3..v4: >> - Update text under packet_mode as per Cristian. >> - Update rte_eth_softnic_tm.c based on Jasvinder's comments. >> - Add error enum RTE_TM_ERROR_TYPE_SHAPER_PROFILE_PACKET_MODE >> - Fix shaper_profile_check() with packet mode check >> - Fix typo's >> > > Acked-by: Cristian Dumitrescu > Hi Nithin, It looks like patch is causing ABI break, I am getting following warning [1], can you please check? [1] https://pastebin.com/XYNFg14u
Re: [dpdk-dev] [dpdk-stable] [PATCH] app: fix usage help of options separated by dashes
On Tue, Apr 21, 2020 at 1:26 AM Thomas Monjalon wrote:
>
> The EAL options and app-specific options are separated
> with double dashes.
>
> The help of testpmd, test-acl and pdump were missing
> the dashes after EAL options.
> Note: testpmd was completely missing the EAL options.
>
> Fixes: af75078fece3 ("first public release")
> Fixes: 26c057ab6c45 ("acl: new test-acl application")
> Fixes: b2854d5317e8 ("app/pdump: support multi-core capture")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Thomas Monjalon
Acked-by: Bruce Richardson
Applied, thanks.
--
David Marchand
Re: [dpdk-dev] [PATCH] pmdinfo: check for pci.ids in /usr/share/misc
On Thu, Mar 12, 2020 at 5:30 PM wrote: > > From: Luca Boccassi > > Debian and Ubuntu switched years ago from /usr/share/hwdata to > /usr/share/misc, > and the former is just a compat symlink now. > We are starting to get bug reports to nudge us into changing. > So check the new path first, and the old one as a fallback. > > Cc: sta...@dpdk.org > > Signed-off-by: Luca Boccassi Acked-by: David Marchand Applied, thanks. -- David Marchand
Re: [dpdk-dev] [PATCH] bus/pci: set boot-up log prints to absolute minimum
06/02/2020 15:36, Jerin Jacob: > On Thu, Feb 6, 2020 at 7:44 PM Thomas Monjalon wrote: > > 21/01/2020 09:00, jer...@marvell.com: > > > From: Jerin Jacob > > > > > > Some machines may have a lot of PCI devices, logs from PCI probe > > > creates a lot of clutter on boot-up, typically one needs > > > to scroll the screen to find other issues in boot-up. > > > > > > This patch changes the loglevel of PCI probes to `debug` > > > to reduce the clutter on default boot-up logs > > > > I think the PCI probe informations are... informational. > > Maybe you are just not interested in info logs. > > If this is the case, I suggest to change the log level at runtime. > > I am wondering, what would be the right balance, Following is DPDK > startup output from octeontx2[1] > It creates a lot of clutter in the "default" boot up. Why not enable > below prints using log level at runtime? > I believe it comes as a debug category, i.e information required to > debug if something is not working, > dpdk bind script already lists what is bound to DPDK. > > Suggestion to remove clutter? I suggest using dynamic log level in the PCI driver. Unfortunately a lot of old DPDK code is still using the old log macros. Some cleanup work is needed here.
Re: [dpdk-dev] [PATCH] bus/pci: pcidev access from secondary process
24/04/2020 19:08, Vijaya Mohan Guvva: > For pci devices presented through igb_uio, pcidev->mem_resource[] is > not populated when the device is initialized for secondary process. > > Initialize pcidev->mem_resource[] with pci-bar mapped addresses. > > Fixes: c752998b (pci: introduce library and driver) > Cc: sta...@dpdk.org > > Signed-off-by: Vijaya Mohan Guvva Reviewed-by: Ferruh Yigit Applied, thanks
Re: [dpdk-dev] [PATCH v2] putting null checks on ops_name
07/04/2020 09:56, Muhammad Bilal: > Bugzilla ID: 353 > Cc: dev@dpdk.org > Cc: sta...@dpdk.org > Cc: hemant.agra...@nxp.com > Signed-off-by: Muhammad Bilal Acked-by: Hemant Agrawal No need to Cc stable in my opinion, as there is no bug fixed. Changing the name to: mbuf: prevent setting mempool ops name empty Applied with below minor change, thanks The blank line below should remain: > const struct rte_memzone *mz; > - > - if (strlen(ops_name) >= RTE_MEMPOOL_OPS_NAMESIZE) > + size_t len = strnlen(ops_name, RTE_MEMPOOL_OPS_NAMESIZE); > + if (len == 0) > + return -EINVAL;
[dpdk-dev] [PATCH v3] mempool: return 0 if area is too small on populate
From: Olivier Matz
Change rte_mempool_populate_iova() and rte_mempool_populate_iova() to
return 0 instead of -EINVAL when there is not enough room to store one
object, as it can be helpful for applications to distinguish this
specific case.
As this is an ABI change, use symbol versioning to preserve old
behavior for binary applications.
Signed-off-by: Olivier Matz
---
changes in v3:
- rebase
- remove deprecation notice
- notify API change in release notes
- fix ABI version from 20.0.1 to 20.0.2 (should be 21 maybe)
This v3 cannot be merged because of a false positive ABI check:
2 Removed functions:
'function int rte_mempool_populate_iova(rte_mempool*, char*, rte_iova_t,
size_t, rte_mempool_memchunk_free_cb_t*, void*)'
{rte_mempool_populate_iova@@DPDK_20.0}
'function int rte_mempool_populate_virt(rte_mempool*, char*, size_t, size_t,
rte_mempool_memchunk_free_cb_t*, void*)'
{rte_mempool_populate_virt@@DPDK_20.0}
---
doc/guides/rel_notes/deprecation.rst | 5 --
doc/guides/rel_notes/release_20_05.rst | 4 ++
examples/ntb/ntb_fwd.c | 2 +-
lib/librte_mempool/meson.build | 2 +
lib/librte_mempool/rte_mempool.c | 77 ++
lib/librte_mempool/rte_mempool.h | 14 ++--
lib/librte_mempool/rte_mempool_version.map | 7 ++
7 files changed, 90 insertions(+), 21 deletions(-)
diff --git a/doc/guides/rel_notes/deprecation.rst
b/doc/guides/rel_notes/deprecation.rst
index 1339f54f5f..20aa745b77 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -65,11 +65,6 @@ Deprecation Notices
structure would be made internal (or removed if all dependencies are cleared)
in future releases.
-* mempool: starting from v20.05, the API of rte_mempool_populate_iova()
- and rte_mempool_populate_virt() will change to return 0 instead
- of -EINVAL when there is not enough room to store one object. The ABI
- will be preserved until 20.11.
-
* ethdev: the legacy filter API, including
``rte_eth_dev_filter_supported()``, ``rte_eth_dev_filter_ctrl()`` as well
as filter types MACVLAN, ETHERTYPE, FLEXIBLE, SYN, NTUPLE, TUNNEL, FDIR,
diff --git a/doc/guides/rel_notes/release_20_05.rst
b/doc/guides/rel_notes/release_20_05.rst
index b124c3f287..ab20a7d021 100644
--- a/doc/guides/rel_notes/release_20_05.rst
+++ b/doc/guides/rel_notes/release_20_05.rst
@@ -241,6 +241,10 @@ API Changes
Also, make sure to start the actual text at the margin.
=
+* mempool: The API of ``rte_mempool_populate_iova()`` and
+ ``rte_mempool_populate_virt()`` changed to return 0 instead of -EINVAL
+ when there is not enough room to store one object.
+
ABI Changes
---
diff --git a/examples/ntb/ntb_fwd.c b/examples/ntb/ntb_fwd.c
index d49189e175..eba8ebf9fa 100644
--- a/examples/ntb/ntb_fwd.c
+++ b/examples/ntb/ntb_fwd.c
@@ -1319,7 +1319,7 @@ ntb_mbuf_pool_create(uint16_t mbuf_seg_size, uint32_t
nb_mbuf,
mz->len - ntb_info.ntb_hdr_size,
ntb_mempool_mz_free,
(void *)(uintptr_t)mz);
- if (ret < 0) {
+ if (ret <= 0) {
rte_memzone_free(mz);
rte_mempool_free(mp);
return NULL;
diff --git a/lib/librte_mempool/meson.build b/lib/librte_mempool/meson.build
index a6e861cbfc..7dbe6b9bea 100644
--- a/lib/librte_mempool/meson.build
+++ b/lib/librte_mempool/meson.build
@@ -9,6 +9,8 @@ foreach flag: extra_flags
endif
endforeach
+use_function_versioning = true
+
sources = files('rte_mempool.c', 'rte_mempool_ops.c',
'rte_mempool_ops_default.c', 'mempool_trace_points.c')
headers = files('rte_mempool.h', 'rte_mempool_trace.h',
diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
index 0be8f9f59d..edbdafaafb 100644
--- a/lib/librte_mempool/rte_mempool.c
+++ b/lib/librte_mempool/rte_mempool.c
@@ -31,6 +31,7 @@
#include
#include
#include
+#include
#include "rte_mempool.h"
#include "rte_mempool_trace.h"
@@ -303,12 +304,17 @@ mempool_ops_alloc_once(struct rte_mempool *mp)
return 0;
}
+__vsym int
+rte_mempool_populate_iova_v20_0_2(struct rte_mempool *mp, char *vaddr,
+ rte_iova_t iova, size_t len, rte_mempool_memchunk_free_cb_t *free_cb,
+ void *opaque);
+
/* Add objects in the pool, using a physically contiguous memory
* zone. Return the number of objects added, or a negative value
* on error.
*/
-static int
-__rte_mempool_populate_iova(struct rte_mempool *mp, char *vaddr,
+__vsym int
+rte_mempool_populate_iova_v20_0_2(struct rte_mempool *mp, char *vaddr,
rte_iova_t iova, size_t len, rte_mempool_memchunk_free_cb_t *free_cb,
void *opaque)
{
@@ -366,14 +372,27 @@ __rte_mempool_populate_iova(struct rte_mempool *mp, char
[dpdk-dev] [Bug 225] ethdev API for firmware version request is not tested
https://bugs.dpdk.org/show_bug.cgi?id=225 Thomas Monjalon (tho...@monjalon.net) changed: What|Removed |Added Resolution|--- |FIXED Status|CONFIRMED |RESOLVED --- Comment #3 from Thomas Monjalon (tho...@monjalon.net) --- Thank you Muhammad Ahmad for working on this. http://git.dpdk.org/dpdk/commit/?id=476ec8e278 -- You are receiving this mail because: You are the assignee for the bug.
[dpdk-dev] [Bug 377] CRYPTODEV: set_sym_session_private_data() line 489: Set private data for driver 0 not allowed
https://bugs.dpdk.org/show_bug.cgi?id=377 Thomas Monjalon (tho...@monjalon.net) changed: What|Removed |Added Status|IN_PROGRESS |RESOLVED Resolution|--- |FIXED --- Comment #4 from Thomas Monjalon (tho...@monjalon.net) --- Resolved in http://git.dpdk.org/dpdk/commit/?id=a0c2b3d8ee -- You are receiving this mail because: You are the assignee for the bug.
[dpdk-dev] [Bug 400] start testpmd with vmxnet3 can't receive and forward packets
https://bugs.dpdk.org/show_bug.cgi?id=400 Thomas Monjalon (tho...@monjalon.net) changed: What|Removed |Added Resolution|--- |FIXED Status|UNCONFIRMED |RESOLVED --- Comment #1 from Thomas Monjalon (tho...@monjalon.net) --- Resolved in http://git.dpdk.org/dpdk/commit/?id=52ec00fd14 -- You are receiving this mail because: You are the assignee for the bug.
[dpdk-dev] [Bug 237] Running test-build.sh Fails on ppc_64 fails due to hard-coded requirement for IXGBE_PMD in examples/vm_power_manager
https://bugs.dpdk.org/show_bug.cgi?id=237 Thomas Monjalon (tho...@monjalon.net) changed: What|Removed |Added Resolution|--- |FIXED Status|CONFIRMED |RESOLVED --- Comment #2 from Thomas Monjalon (tho...@monjalon.net) --- Resolved in http://git.dpdk.org/dpdk/commit/?id=70b2c7f12c -- You are receiving this mail because: You are the assignee for the bug.
[dpdk-dev] [Bug 253] Unable to run DPDK test with "make test" command
https://bugs.dpdk.org/show_bug.cgi?id=253 Thomas Monjalon (tho...@monjalon.net) changed: What|Removed |Added Resolution|--- |FIXED Status|CONFIRMED |RESOLVED --- Comment #8 from Thomas Monjalon (tho...@monjalon.net) --- Resolved in http://git.dpdk.org/dpdk/commit/?id=c6ad35c468 -- You are receiving this mail because: You are the assignee for the bug.
[dpdk-dev] [PATCH V2] app/testpmd: fix forward stats after clear stats command
From: "Wei Hu (Xavier)"
Currently, when running start/clear stats&xstats/stop command many times
based on testpmd application, there are incorrect forward Rx/Tx-packets
stats as below:
-- Forward statistics for port 0 --
RX-packets: 18446744073709544808 RX-dropped: 0
TX-packets: 18446744073709536616 TX-dropped: 0
The root cause as below:
1. The struct rte_port of testpmd.h has a member variable
"struct rte_eth_stats stats" to store the last port statistics.
2. When runnig start command, it execute cmd_start_parsed ->
start_packet_forwarding -> fwd_stats_reset, which call rte_eth_stats_get
API function to save current port statistics.
3. When running stop command, it execute fwd_stats_display, which call
rte_eth_stats_get to get current port statistics, and then minus last
port statistics.
4. If we run clear stats or xstats after start command, then run stop,
it may display above incorrect stats because the current Rx/Tx-packets
is lower than the last saved RX/TX-packets(uint64_t overflow).
This patch fixes it by clearing last port statistics when executing
"clear stats/xstats" command.
Fixes: af75078fece3 ("first public release")
Cc: sta...@dpdk.org
Signed-off-by: Chengwen Feng
Signed-off-by: Wei Hu (Xavier)
---
v1 -> v2:
Update the title and the documentation
(doc/guides/testpmd_app_ug/testpmd_funcs.rst) based on
Ferruh Yigit's comment as below:
http://patches.dpdk.org/patch/69252/
---
app/test-pmd/config.c | 11 +++
doc/guides/testpmd_app_ug/testpmd_funcs.rst | 2 +-
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 72f25d152..0d2375607 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -234,10 +234,16 @@ nic_stats_display(portid_t port_id)
void
nic_stats_clear(portid_t port_id)
{
+ struct rte_port *port;
+
if (port_id_is_invalid(port_id, ENABLED_WARN)) {
print_valid_ports();
return;
}
+
+ port = &ports[port_id];
+ /* clear last port statistics because eth stats reset */
+ memset(&port->stats, 0, sizeof(port->stats));
rte_eth_stats_reset(port_id);
printf("\n NIC statistics for port %d cleared\n", port_id);
}
@@ -308,12 +314,17 @@ nic_xstats_display(portid_t port_id)
void
nic_xstats_clear(portid_t port_id)
{
+ struct rte_port *port;
int ret;
if (port_id_is_invalid(port_id, ENABLED_WARN)) {
print_valid_ports();
return;
}
+
+ port = &ports[port_id];
+ /* clear last port statistics because eth xstats(include stats) reset */
+ memset(&port->stats, 0, sizeof(port->stats));
ret = rte_eth_xstats_reset(port_id);
if (ret != 0) {
printf("%s: Error: failed to reset xstats (port %u): %s",
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index a360ecccf..ca83a2ab5 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -237,7 +237,7 @@ Display the RSS hash functions and RSS hash key of a port::
clear port
~~
-Clear the port statistics for a given port or for all ports::
+Clear the port statistics and forward engine statistics for a given port or
for all ports::
testpmd> clear port (info|stats|xstats|fdir|stat_qmap) (port_id|all)
--
2.23.0
Re: [dpdk-dev] [PATCH] app/testpmd: fix Rx/Tx stats after clear stats command
Hi, Ferruh Yigit
On 2020/4/25 0:12, Ferruh Yigit wrote:
On 4/24/2020 12:07 PM, Wei Hu (Xavier) wrote:
From: Chengwen Feng
Currently, when running start/clear stats&xstats/stop command many times
based on testpmd application, there are incorrect RX/TX-packets stats as
below:
-- Forward statistics for port 0 --
RX-packets: 18446744073709544808 RX-dropped: 0 ...ignore
TX-packets: 18446744073709536616 TX-dropped: 0 ...ignore
The root cause as below:
1. The struct rte_port of testpmd.h has a member variable
"struct rte_eth_stats stats" to store the last port statistics.
2. When runnig start command, it execute cmd_start_parsed ->
start_packet_forwarding -> fwd_stats_reset, which call rte_eth_stats_get
API function to save current port statistics.
3. When running stop command, it execute fwd_stats_display, which call
rte_eth_stats_get to get current port statistics, and then minus last
port statistics.
4. If we run clear stats or xstats after start command, then run stop,
it may display above incorrect stats because the current Rx/Tx-packets
is lower than the last saved RX/TX-packets(uint64_t overflow).
Looks like valid issue.
Can you please update the title to mention this fixes the forward stats (to
prevent the misunderstanding that issue is in the port stats).
Also can you please update the documentation
(doc/guides/testpmd_app_ug/testpmd_funcs.rst), "clear port" command to say this
will also affect the forward stats output (show fwd)?
OK, Thanks for your comments.
I will send V2.
Regards
Xavier
This patch fixes it by clearing last port statistics when executing
"clear stats/xstats" command.
Fixes: af75078fece3 ("first public release")
Cc: sta...@dpdk.org
Signed-off-by: Chengwen Feng
Signed-off-by: Wei Hu (Xavier)
---
app/test-pmd/config.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 72f25d152..0d2375607 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -234,10 +234,16 @@ nic_stats_display(portid_t port_id)
void
nic_stats_clear(portid_t port_id)
{
+ struct rte_port *port;
+
if (port_id_is_invalid(port_id, ENABLED_WARN)) {
print_valid_ports();
return;
}
+
+ port = &ports[port_id];
+ /* clear last port statistics because eth stats reset */
+ memset(&port->stats, 0, sizeof(port->stats));
"clear fwd stats" command does same thing in "fwd_stats_reset()" as:
rte_eth_stats_get(pt_id, &ports[pt_id].stats);
I suggest doing same here for consistency, but it should be after
'rte_eth_stats_reset()' in that case.
rte_eth_stats_reset(port_id);
printf("\n NIC statistics for port %d cleared\n", port_id);
}
@@ -308,12 +314,17 @@ nic_xstats_display(portid_t port_id)
void
nic_xstats_clear(portid_t port_id)
{
+ struct rte_port *port;
int ret;
if (port_id_is_invalid(port_id, ENABLED_WARN)) {
print_valid_ports();
return;
}
+
+ port = &ports[port_id];
+ /* clear last port statistics because eth xstats(include stats) reset */
+ memset(&port->stats, 0, sizeof(port->stats));
ret = rte_eth_xstats_reset(port_id);
if (ret != 0) {
printf("%s: Error: failed to reset xstats (port %u): %s",
Re: [dpdk-dev] [PATCH] app/testpmd: fix Rx/Tx stats after clear stats command
On 4/26/2020 2:36 AM, Wei Hu (Xavier) wrote:
> Hi, Ferruh Yigit
>
> On 2020/4/25 0:12, Ferruh Yigit wrote:
>> On 4/24/2020 12:07 PM, Wei Hu (Xavier) wrote:
>>> From: Chengwen Feng
>>>
>>> Currently, when running start/clear stats&xstats/stop command many times
>>> based on testpmd application, there are incorrect RX/TX-packets stats as
>>> below:
>>> -- Forward statistics for port 0 --
>>> RX-packets: 18446744073709544808 RX-dropped: 0 ...ignore
>>> TX-packets: 18446744073709536616 TX-dropped: 0 ...ignore
>>>
>>>
>>> The root cause as below:
>>> 1. The struct rte_port of testpmd.h has a member variable
>>> "struct rte_eth_stats stats" to store the last port statistics.
>>> 2. When runnig start command, it execute cmd_start_parsed ->
>>> start_packet_forwarding -> fwd_stats_reset, which call rte_eth_stats_get
>>> API function to save current port statistics.
>>> 3. When running stop command, it execute fwd_stats_display, which call
>>> rte_eth_stats_get to get current port statistics, and then minus last
>>> port statistics.
>>> 4. If we run clear stats or xstats after start command, then run stop,
>>> it may display above incorrect stats because the current Rx/Tx-packets
>>> is lower than the last saved RX/TX-packets(uint64_t overflow).
>>
>> Looks like valid issue.
>>
>> Can you please update the title to mention this fixes the forward stats (to
>> prevent the misunderstanding that issue is in the port stats).
>>
>> Also can you please update the documentation
>> (doc/guides/testpmd_app_ug/testpmd_funcs.rst), "clear port" command to say
>> this
>> will also affect the forward stats output (show fwd)?
>>
>OK, Thanks for your comments.
>I will send V2.
>
>Regards
> Xavier
>>>
>>> This patch fixes it by clearing last port statistics when executing
>>> "clear stats/xstats" command.
>>>
>>> Fixes: af75078fece3 ("first public release")
>>> Cc: sta...@dpdk.org
>>>
>>> Signed-off-by: Chengwen Feng
>>> Signed-off-by: Wei Hu (Xavier)
>>> ---
>>> app/test-pmd/config.c | 11 +++
>>> 1 file changed, 11 insertions(+)
>>>
>>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>>> index 72f25d152..0d2375607 100644
>>> --- a/app/test-pmd/config.c
>>> +++ b/app/test-pmd/config.c
>>> @@ -234,10 +234,16 @@ nic_stats_display(portid_t port_id)
>>> void
>>> nic_stats_clear(portid_t port_id)
>>> {
>>> + struct rte_port *port;
>>> +
>>> if (port_id_is_invalid(port_id, ENABLED_WARN)) {
>>> print_valid_ports();
>>> return;
>>> }
>>> +
>>> + port = &ports[port_id];
>>> + /* clear last port statistics because eth stats reset */
>>> + memset(&port->stats, 0, sizeof(port->stats));
>>
>> "clear fwd stats" command does same thing in "fwd_stats_reset()" as:
>> rte_eth_stats_get(pt_id, &ports[pt_id].stats);
>>
>> I suggest doing same here for consistency, but it should be after
>> 'rte_eth_stats_reset()' in that case.
What do you think about above comment?
>>
>>> rte_eth_stats_reset(port_id);
>>> printf("\n NIC statistics for port %d cleared\n", port_id);
>>> }
>>> @@ -308,12 +314,17 @@ nic_xstats_display(portid_t port_id)
>>> void
>>> nic_xstats_clear(portid_t port_id)
>>> {
>>> + struct rte_port *port;
>>> int ret;
>>>
>>> if (port_id_is_invalid(port_id, ENABLED_WARN)) {
>>> print_valid_ports();
>>> return;
>>> }
>>> +
>>> + port = &ports[port_id];
>>> + /* clear last port statistics because eth xstats(include stats) reset */
>>> + memset(&port->stats, 0, sizeof(port->stats));
>>> ret = rte_eth_xstats_reset(port_id);
>>> if (ret != 0) {
>>> printf("%s: Error: failed to reset xstats (port %u): %s",
>>>
[dpdk-dev] [PATCH v10 2/2] eal: support for VFIO-PCI VF token
The kernel module vfio-pci introduces the VF token to enable SR-IOV
support since 5.7.
The VF token can be set by a vfio-pci based PF driver and must be known
by the vfio-pci based VF driver in order to gain access to the device.
Signed-off-by: Haiyue Wang
Acked-by: Vamsi Attunuru
Tested-by: Vamsi Attunuru
---
doc/guides/linux_gsg/linux_drivers.rst | 41 +-
doc/guides/rel_notes/release_20_05.rst | 6 +++
drivers/bus/pci/linux/pci_vfio.c | 74 +-
lib/librte_eal/freebsd/eal.c | 3 +-
lib/librte_eal/include/rte_vfio.h | 26 -
lib/librte_eal/linux/eal_vfio.c| 20 +--
lib/librte_eal/rte_eal_version.map | 8 ++-
7 files changed, 169 insertions(+), 9 deletions(-)
diff --git a/doc/guides/linux_gsg/linux_drivers.rst
b/doc/guides/linux_gsg/linux_drivers.rst
index 238f3e900..b42fd708b 100644
--- a/doc/guides/linux_gsg/linux_drivers.rst
+++ b/doc/guides/linux_gsg/linux_drivers.rst
@@ -72,11 +72,50 @@ Note that in order to use VFIO, your kernel must support it.
VFIO kernel modules have been included in the Linux kernel since version 3.6.0
and are usually present by default,
however please consult your distributions documentation to make sure that is
the case.
+The ``vfio-pci`` module since Linux version 5.7 supports the creation of
virtual
+functions. After the PF is bound to vfio-pci module, the user can create the
VFs
+by sysfs interface, and these VFs are bound to vfio-pci module automatically.
+
+When the PF is bound to vfio-pci, it has initial VF token generated by random.
For
+security reason, this token is write only, the user can't read it from the
kernel
+directly. For accessing the VF, the user needs to start the PF with token
parameter
+to setup a VF token (uuid format), then the VF can be accessed with this new
known
+VF token.
+
+Also if the DPDK application running on the PF device exits, the user wants to
start
+the PF with another different VF token value, it has no issue if no
application like
+DPDK or KVM runs on VFs. Otherwise, the PF will fail to start until all VFs
are free
+to use, after that, the user can select a new VF token to start the PF device.
+
+DPDK will use the keyword ``vf_token`` as the device argument to pass the VF
token
+value to PF and its related VFs, the PMD should not use it, and this argument
will
+be pruned from the device argument list, so the PMD can parse its own valid
device
+arguments successfully.
+
+.. code-block:: console
+
+1. Generate the VF token by uuid command
+14d63f20-8445-11ea-8900-1f9ce7d5650d
+
+2. sudo modprobe vfio-pci enable_sriov=1
+
+2. ./usertools/dpdk-devbind.py -b vfio-pci :86:00.0
+
+3. echo 2 > /sys/bus/pci/devices/:86:00.0/sriov_numvfs
+
+4. Start the PF:
+./x86_64-native-linux-gcc/app/testpmd -l 22-25 -n 4 \
+ -w 86:00.0,vf_token=14d63f20-8445-11ea-8900-1f9ce7d5650d
--file-prefix=pf -- -i
+
+5. Start the VF:
+./x86_64-native-linux-gcc/app/testpmd -l 26-29 -n 4 \
+ -w 86:02.0,vf_token=14d63f20-8445-11ea-8900-1f9ce7d5650d
--file-prefix=vf0 -- -i
+
Also, to use VFIO, both kernel and BIOS must support and be configured to use
IO virtualization (such as Intel® VT-d).
.. note::
-``vfio-pci`` module doesn't support the creation of virtual functions.
+``vfio-pci`` module doesn't support the creation of virtual functions
before Linux version 5.7.
For proper operation of VFIO when running DPDK applications as a
non-privileged user, correct permissions should also be set up.
This can be done by using the DPDK setup script (called dpdk-setup.sh and
located in the usertools directory).
diff --git a/doc/guides/rel_notes/release_20_05.rst
b/doc/guides/rel_notes/release_20_05.rst
index b124c3f28..722c61e67 100644
--- a/doc/guides/rel_notes/release_20_05.rst
+++ b/doc/guides/rel_notes/release_20_05.rst
@@ -93,6 +93,12 @@ New Features
* Added new query: ``rte_flow_get_aged_flows`` to get the aged-out flows
contexts from the port.
+* **Added the support for vfio-pci new VF token interface.**
+
+ Since Linux version 5.7, vfio-pci supports a shared VF token (UUID) to
represent
+ the trust between SR-IOV PF and the created VFs. Update the method to gain
access
+ to the PF and VFs devices by appending the VF token parameter.
+
* **Updated Amazon ena driver.**
Updated ena PMD with new features and improvements, including:
diff --git a/drivers/bus/pci/linux/pci_vfio.c b/drivers/bus/pci/linux/pci_vfio.c
index 64cd84a68..efb64e2ba 100644
--- a/drivers/bus/pci/linux/pci_vfio.c
+++ b/drivers/bus/pci/linux/pci_vfio.c
@@ -11,6 +11,7 @@
#include
#include
+#include
#include
#include
#include
@@ -644,12 +645,72 @@ pci_vfio_msix_is_mappable(int vfio_dev_fd, int
msix_region)
return ret;
}
+static int
+vfio_pci_vf_token_arg(struct rte_devargs *devargs, rte_uuid_t uuid)
+{
+#define VF_TOKEN_ARG "vf_token="
+ char c, *p, *vf_token
[dpdk-dev] [PATCH v10 1/2] eal: add uuid dependent header files explicitly
Add the dependent header files explicitly, so that the user just needs
to include the 'rte_uuid.h' header file directly to avoid compile error:
(1). rte_uuid.h:97:55: error: unknown type name ‘size_t’
(2). rte_uuid.h:58:2: error: implicit declaration of function ‘memcpy’
Fixes: 6bc67c497a51 ("eal: add uuid API")
Cc: sta...@dpdk.org
Signed-off-by: Haiyue Wang
---
lib/librte_eal/include/rte_uuid.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/librte_eal/include/rte_uuid.h
b/lib/librte_eal/include/rte_uuid.h
index 044afbdfa..8b42e070a 100644
--- a/lib/librte_eal/include/rte_uuid.h
+++ b/lib/librte_eal/include/rte_uuid.h
@@ -15,6 +15,8 @@ extern "C" {
#endif
#include
+#include
+#include
/**
* Struct describing a Universal Unique Identifier
--
2.26.2
[dpdk-dev] [PATCH v10 0/2] support for VFIO-PCI VF token interface
v10: Use the __rte_internal to mark the internal API changing. v9: Rewrite the document. v8: Update the document. v7: Add the Fixes tag in uuid, the release note and help document. v6: Drop the Fixes tag in uuid, since the file has been moved to another place, not suitable to apply on stable. And this is not a bug, just some kind of enhancement. v5: 1. Add the VF token parse error handling. 2. Split into two patches for different logic module. 3. Add more comments into the code for explaining the design. 4. Drop the ABI change workaround, this patch set focuses on code review. v4: 1. Ignore rte_vfio_setup_device ABI check since it is for Linux driver use. v3: Fix the Travis build failed: (1). rte_uuid.h:97:55: error: unknown type name ‘size_t’ (2). rte_uuid.h:58:2: error: implicit declaration of function ‘memcpy’ v2: Fix the FreeBSD build error. v1: Update the commit message. RFC v2: Based on Vamsi's RFC v1, and Alex's patch for Qemu [https://lore.kernel.org/lkml/20200204161737.34696...@w520.home/]: Use the devarg to pass-down the VF token. RFC v1: https://patchwork.dpdk.org/patch/66281/ by Vamsi. Haiyue Wang (2): eal: add uuid dependent header files explicitly eal: support for VFIO-PCI VF token doc/guides/linux_gsg/linux_drivers.rst | 41 +- doc/guides/rel_notes/release_20_05.rst | 6 +++ drivers/bus/pci/linux/pci_vfio.c | 74 +- lib/librte_eal/freebsd/eal.c | 3 +- lib/librte_eal/include/rte_uuid.h | 2 + lib/librte_eal/include/rte_vfio.h | 26 - lib/librte_eal/linux/eal_vfio.c| 20 +-- lib/librte_eal/rte_eal_version.map | 8 ++- 8 files changed, 171 insertions(+), 9 deletions(-) -- 2.26.2
Re: [dpdk-dev] [PATCH v9 1/6] lib/eal: implement the family of common bit operation APIs
> -Original Message- > From: Thomas Monjalon > Sent: Friday, April 24, 2020 4:09 PM > To: Joyce Kong > Cc: step...@networkplumber.org; david.march...@redhat.com; > m...@smartsharesystems.com; jer...@marvell.com; > bruce.richard...@intel.com; ravi1.ku...@amd.com; rm...@marvell.com; > shsha...@marvell.com; xuanziya...@huawei.com; > cloud.wangxiao...@huawei.com; zhouguoy...@huawei.com; Honnappa > Nagarahalli ; Gavin Hu > ; Phil Yang ; nd ; > dev@dpdk.org > Subject: Re: [PATCH v9 1/6] lib/eal: implement the family of common bit > operation APIs > > 24/04/2020 05:21, Joyce Kong: > > --- a/doc/api/doxy-api-index.md > > +++ b/doc/api/doxy-api-index.md > > - **containers**: > > + [bitmap] (@ref rte_bitmap.h), > >[mbuf] (@ref rte_mbuf.h), > >[mbuf pool ops] (@ref rte_mbuf_pool_ops.h), > >[ring] (@ref rte_ring.h), > >[stack] (@ref rte_stack.h), > > - [tailq] (@ref rte_tailq.h), > > - [bitmap] (@ref rte_bitmap.h) > > + [tailq] (@ref rte_tailq.h) > > Why do you move bitmap? > I like having mbuf as the one. > Yeah, will move bitmap back in v10.
[dpdk-dev] [PATCH v10 2/9] net/virtio: inorder should depend on feature bit
Ring initialization is different when inorder feature negotiated. This
action should dependent on negotiated feature bits.
Signed-off-by: Marvin Liu
Reviewed-by: Maxime Coquelin
diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index 94ba7a3ec..e450477e8 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -989,6 +989,7 @@ virtio_dev_rx_queue_setup_finish(struct rte_eth_dev *dev,
uint16_t queue_idx)
struct rte_mbuf *m;
uint16_t desc_idx;
int error, nbufs, i;
+ bool in_order = vtpci_with_feature(hw, VIRTIO_F_IN_ORDER);
PMD_INIT_FUNC_TRACE();
@@ -1018,7 +1019,7 @@ virtio_dev_rx_queue_setup_finish(struct rte_eth_dev *dev,
uint16_t queue_idx)
virtio_rxq_rearm_vec(rxvq);
nbufs += RTE_VIRTIO_VPMD_RX_REARM_THRESH;
}
- } else if (hw->use_inorder_rx) {
+ } else if (!vtpci_packed_queue(vq->hw) && in_order) {
if ((!virtqueue_full(vq))) {
uint16_t free_cnt = vq->vq_free_cnt;
struct rte_mbuf *pkts[free_cnt];
@@ -1133,7 +1134,7 @@ virtio_dev_tx_queue_setup_finish(struct rte_eth_dev *dev,
PMD_INIT_FUNC_TRACE();
if (!vtpci_packed_queue(hw)) {
- if (hw->use_inorder_tx)
+ if (vtpci_with_feature(hw, VIRTIO_F_IN_ORDER))
vq->vq_split.ring.desc[vq->vq_nentries - 1].next = 0;
}
@@ -2046,7 +2047,7 @@ virtio_xmit_pkts_packed(void *tx_queue, struct rte_mbuf
**tx_pkts,
struct virtio_hw *hw = vq->hw;
uint16_t hdr_size = hw->vtnet_hdr_size;
uint16_t nb_tx = 0;
- bool in_order = hw->use_inorder_tx;
+ bool in_order = vtpci_with_feature(hw, VIRTIO_F_IN_ORDER);
if (unlikely(hw->started == 0 && tx_pkts != hw->inject_pkts))
return nb_tx;
--
2.17.1
[dpdk-dev] [PATCH v10 3/9] net/virtio: add vectorized devarg
Previously, virtio split ring vectorized path was enabled by default.
This is not suitable for everyone because that path dose not follow
virtio spec. Add new devarg for virtio vectorized path selection. By
default vectorized path is disabled.
Signed-off-by: Marvin Liu
Reviewed-by: Maxime Coquelin
diff --git a/doc/guides/nics/virtio.rst b/doc/guides/nics/virtio.rst
index 6286286db..902a1f0cf 100644
--- a/doc/guides/nics/virtio.rst
+++ b/doc/guides/nics/virtio.rst
@@ -363,6 +363,13 @@ Below devargs are supported by the PCI virtio driver:
rte_eth_link_get_nowait function.
(Default: 1 (10G))
+#. ``vectorized``:
+
+It is used to specify whether virtio device perfer to use vectorized path.
+Afterwards, dependencies of vectorized path will be checked in path
+election.
+(Default: 0 (disabled))
+
Below devargs are supported by the virtio-user vdev:
#. ``path``:
diff --git a/drivers/net/virtio/virtio_ethdev.c
b/drivers/net/virtio/virtio_ethdev.c
index 37766cbb6..0a69a4db1 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -48,7 +48,8 @@ static int virtio_dev_allmulticast_disable(struct rte_eth_dev
*dev);
static uint32_t virtio_dev_speed_capa_get(uint32_t speed);
static int virtio_dev_devargs_parse(struct rte_devargs *devargs,
int *vdpa,
- uint32_t *speed);
+ uint32_t *speed,
+ int *vectorized);
static int virtio_dev_info_get(struct rte_eth_dev *dev,
struct rte_eth_dev_info *dev_info);
static int virtio_dev_link_update(struct rte_eth_dev *dev,
@@ -1551,8 +1552,8 @@ set_rxtx_funcs(struct rte_eth_dev *eth_dev)
eth_dev->rx_pkt_burst = &virtio_recv_pkts_packed;
}
} else {
- if (hw->use_simple_rx) {
- PMD_INIT_LOG(INFO, "virtio: using simple Rx path on
port %u",
+ if (hw->use_vec_rx) {
+ PMD_INIT_LOG(INFO, "virtio: using vectorized Rx path on
port %u",
eth_dev->data->port_id);
eth_dev->rx_pkt_burst = virtio_recv_pkts_vec;
} else if (hw->use_inorder_rx) {
@@ -1886,6 +1887,7 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
{
struct virtio_hw *hw = eth_dev->data->dev_private;
uint32_t speed = SPEED_UNKNOWN;
+ int vectorized = 0;
int ret;
if (sizeof(struct virtio_net_hdr_mrg_rxbuf) > RTE_PKTMBUF_HEADROOM) {
@@ -1912,7 +1914,7 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
return 0;
}
ret = virtio_dev_devargs_parse(eth_dev->device->devargs,
-NULL, &speed);
+NULL, &speed, &vectorized);
if (ret < 0)
return ret;
hw->speed = speed;
@@ -1949,6 +1951,11 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
if (ret < 0)
goto err_virtio_init;
+ if (vectorized) {
+ if (!vtpci_packed_queue(hw))
+ hw->use_vec_rx = 1;
+ }
+
hw->opened = true;
return 0;
@@ -2021,9 +2028,20 @@ virtio_dev_speed_capa_get(uint32_t speed)
}
}
+static int vectorized_check_handler(__rte_unused const char *key,
+ const char *value, void *ret_val)
+{
+ if (strcmp(value, "1") == 0)
+ *(int *)ret_val = 1;
+ else
+ *(int *)ret_val = 0;
+
+ return 0;
+}
#define VIRTIO_ARG_SPEED "speed"
#define VIRTIO_ARG_VDPA "vdpa"
+#define VIRTIO_ARG_VECTORIZED "vectorized"
static int
@@ -2045,7 +2063,7 @@ link_speed_handler(const char *key __rte_unused,
static int
virtio_dev_devargs_parse(struct rte_devargs *devargs, int *vdpa,
- uint32_t *speed)
+ uint32_t *speed, int *vectorized)
{
struct rte_kvargs *kvlist;
int ret = 0;
@@ -2081,6 +2099,18 @@ virtio_dev_devargs_parse(struct rte_devargs *devargs,
int *vdpa,
}
}
+ if (vectorized &&
+ rte_kvargs_count(kvlist, VIRTIO_ARG_VECTORIZED) == 1) {
+ ret = rte_kvargs_process(kvlist,
+ VIRTIO_ARG_VECTORIZED,
+ vectorized_check_handler, vectorized);
+ if (ret < 0) {
+ PMD_INIT_LOG(ERR, "Failed to parse %s",
+ VIRTIO_ARG_VECTORIZED);
+ goto exit;
+ }
+ }
+
exit:
rte_kvargs_free(kvlist);
return ret;
@@ -2092,7 +2122,8 @@ static int eth_virtio_pci_probe(struct rte_pci_driver
*pci_drv __rte_unused,
int vdpa = 0;
int ret = 0;
- ret = virtio_dev_devargs_parse(pci_dev->device.devargs, &vdpa, NULL);
+ ret = virtio_dev_devargs_parse(pci_dev->device.devargs, &vdpa, NULL,
+ NULL);
if (ret < 0) {
PMD_INIT_LOG(ERR, "devargs parsing is failed");
[dpdk-dev] [PATCH v9 0/9] add packed ring vectorized path
This patch set introduced vectorized path for packed ring. The size of packed ring descriptor is 16Bytes. Four batched descriptors are just placed into one cacheline. AVX512 instructions can well handle this kind of data. Packed ring TX path can fully transformed into vectorized path. Packed ring Rx path can be vectorized when requirements met(LRO and mergeable disabled). New option RTE_LIBRTE_VIRTIO_INC_VECTOR will be introduced in this patch set. This option will unify split and packed ring vectorized path default setting. Meanwhile user can specify whether enable vectorized path at runtime by 'vectorized' parameter of virtio user vdev. v10: * reuse packed ring xmit cleanup v9: * replace RTE_LIBRTE_VIRTIO_INC_VECTOR with vectorized devarg * reorder patch sequence v8: * fix meson build error on ubuntu16.04 and suse15 v7: * default vectorization is disabled * compilation time check dependency on rte_mbuf structure * offsets are calcuated when compiling * remove useless barrier as descs are batched store&load * vindex of scatter is directly set * some comments updates * enable vectorized path in meson build v6: * fix issue when size not power of 2 v5: * remove cpuflags definition as required extensions always come with AVX512F on x86_64 * inorder actions should depend on feature bit * check ring type in rx queue setup * rewrite some commit logs * fix some checkpatch warnings v4: * rename 'packed_vec' to 'vectorized', also used in split ring * add RTE_LIBRTE_VIRTIO_INC_VECTOR config for virtio ethdev * check required AVX512 extensions cpuflags * combine split and packed ring datapath selection logic * remove limitation that size must power of two * clear 12Bytes virtio_net_hdr v3: * remove virtio_net_hdr array for better performance * disable 'packed_vec' by default v2: * more function blocks replaced by vector instructions * clean virtio_net_hdr by vector instruction * allow header room size change * add 'packed_vec' option in virtio_user vdev * fix build not check whether AVX512 enabled * doc update Tested-by: Wang, Yinan Marvin Liu (9): net/virtio: add Rx free threshold setting net/virtio: inorder should depend on feature bit net/virtio: add vectorized devarg net/virtio-user: add vectorized devarg net/virtio: reuse packed ring functions net/virtio: add vectorized packed ring Rx path net/virtio: add vectorized packed ring Tx path net/virtio: add election for vectorized path doc: add packed vectorized path doc/guides/nics/virtio.rst | 52 +- drivers/net/virtio/Makefile | 35 ++ drivers/net/virtio/meson.build | 14 + drivers/net/virtio/virtio_ethdev.c | 137 - drivers/net/virtio/virtio_ethdev.h | 6 + drivers/net/virtio/virtio_pci.h | 3 +- drivers/net/virtio/virtio_rxtx.c| 349 ++- drivers/net/virtio/virtio_rxtx_packed_avx.c | 623 drivers/net/virtio/virtio_user_ethdev.c | 32 +- drivers/net/virtio/virtqueue.c | 7 +- drivers/net/virtio/virtqueue.h | 307 +- 11 files changed, 1210 insertions(+), 355 deletions(-) create mode 100644 drivers/net/virtio/virtio_rxtx_packed_avx.c -- 2.17.1
[dpdk-dev] [PATCH v10 4/9] net/virtio-user: add vectorized devarg
Add new devarg for virtio user device vectorized path selection. By
default vectorized path is disabled.
Signed-off-by: Marvin Liu
diff --git a/doc/guides/nics/virtio.rst b/doc/guides/nics/virtio.rst
index 902a1f0cf..d59add23e 100644
--- a/doc/guides/nics/virtio.rst
+++ b/doc/guides/nics/virtio.rst
@@ -424,6 +424,12 @@ Below devargs are supported by the virtio-user vdev:
rte_eth_link_get_nowait function.
(Default: 1 (10G))
+#. ``vectorized``:
+
+It is used to specify whether virtio device perfer to use vectorized path.
+Afterwards, dependencies of vectorized path will be checked in path
+election.
+(Default: 0 (disabled))
Virtio paths Selection and Usage
diff --git a/drivers/net/virtio/virtio_user_ethdev.c
b/drivers/net/virtio/virtio_user_ethdev.c
index 150a8d987..40ad786cc 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -452,6 +452,8 @@ static const char *valid_args[] = {
VIRTIO_USER_ARG_PACKED_VQ,
#define VIRTIO_USER_ARG_SPEED "speed"
VIRTIO_USER_ARG_SPEED,
+#define VIRTIO_USER_ARG_VECTORIZED "vectorized"
+ VIRTIO_USER_ARG_VECTORIZED,
NULL
};
@@ -559,6 +561,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
uint64_t mrg_rxbuf = 1;
uint64_t in_order = 1;
uint64_t packed_vq = 0;
+ uint64_t vectorized = 0;
char *path = NULL;
char *ifname = NULL;
char *mac_addr = NULL;
@@ -675,6 +678,15 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
}
}
+ if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_VECTORIZED) == 1) {
+ if (rte_kvargs_process(kvlist, VIRTIO_USER_ARG_VECTORIZED,
+ &get_integer_arg, &vectorized) < 0) {
+ PMD_INIT_LOG(ERR, "error to parse %s",
+VIRTIO_USER_ARG_VECTORIZED);
+ goto end;
+ }
+ }
+
if (queues > 1 && cq == 0) {
PMD_INIT_LOG(ERR, "multi-q requires ctrl-q");
goto end;
@@ -727,6 +739,9 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
goto end;
}
+ if (vectorized)
+ hw->use_vec_rx = 1;
+
rte_eth_dev_probing_finish(eth_dev);
ret = 0;
@@ -785,4 +800,5 @@ RTE_PMD_REGISTER_PARAM_STRING(net_virtio_user,
"mrg_rxbuf=<0|1> "
"in_order=<0|1> "
"packed_vq=<0|1> "
- "speed=");
+ "speed= "
+ "vectorized=<0|1>");
--
2.17.1
[dpdk-dev] [PATCH v10 1/9] net/virtio: add Rx free threshold setting
Introduce free threshold setting in Rx queue, its default value is 32.
Limit the threshold size to multiple of four as only vectorized packed
Rx function will utilize it. Virtio driver will rearm Rx queue when
more than rx_free_thresh descs were dequeued.
Signed-off-by: Marvin Liu
Reviewed-by: Maxime Coquelin
diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index 060410577..94ba7a3ec 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -936,6 +936,7 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
struct virtio_hw *hw = dev->data->dev_private;
struct virtqueue *vq = hw->vqs[vtpci_queue_idx];
struct virtnet_rx *rxvq;
+ uint16_t rx_free_thresh;
PMD_INIT_FUNC_TRACE();
@@ -944,6 +945,28 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
return -EINVAL;
}
+ rx_free_thresh = rx_conf->rx_free_thresh;
+ if (rx_free_thresh == 0)
+ rx_free_thresh =
+ RTE_MIN(vq->vq_nentries / 4, DEFAULT_RX_FREE_THRESH);
+
+ if (rx_free_thresh & 0x3) {
+ RTE_LOG(ERR, PMD, "rx_free_thresh must be multiples of four."
+ " (rx_free_thresh=%u port=%u queue=%u)\n",
+ rx_free_thresh, dev->data->port_id, queue_idx);
+ return -EINVAL;
+ }
+
+ if (rx_free_thresh >= vq->vq_nentries) {
+ RTE_LOG(ERR, PMD, "rx_free_thresh must be less than the "
+ "number of RX entries (%u)."
+ " (rx_free_thresh=%u port=%u queue=%u)\n",
+ vq->vq_nentries,
+ rx_free_thresh, dev->data->port_id, queue_idx);
+ return -EINVAL;
+ }
+ vq->vq_free_thresh = rx_free_thresh;
+
if (nb_desc == 0 || nb_desc > vq->vq_nentries)
nb_desc = vq->vq_nentries;
vq->vq_free_cnt = RTE_MIN(vq->vq_free_cnt, nb_desc);
diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h
index 58ad7309a..6301c56b2 100644
--- a/drivers/net/virtio/virtqueue.h
+++ b/drivers/net/virtio/virtqueue.h
@@ -18,6 +18,8 @@
struct rte_mbuf;
+#define DEFAULT_RX_FREE_THRESH 32
+
/*
* Per virtio_ring.h in Linux.
* For virtio_pci on SMP, we don't need to order with respect to MMIO
--
2.17.1
[dpdk-dev] [PATCH v10 5/9] net/virtio: reuse packed ring functions
Move offload, xmit cleanup and packed xmit enqueue function to header
file. These functions will be reused by packed ring vectorized path.
Signed-off-by: Marvin Liu
diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index 84f4cf946..a549991aa 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -89,23 +89,6 @@ vq_ring_free_chain(struct virtqueue *vq, uint16_t desc_idx)
dp->next = VQ_RING_DESC_CHAIN_END;
}
-static void
-vq_ring_free_id_packed(struct virtqueue *vq, uint16_t id)
-{
- struct vq_desc_extra *dxp;
-
- dxp = &vq->vq_descx[id];
- vq->vq_free_cnt += dxp->ndescs;
-
- if (vq->vq_desc_tail_idx == VQ_RING_DESC_CHAIN_END)
- vq->vq_desc_head_idx = id;
- else
- vq->vq_descx[vq->vq_desc_tail_idx].next = id;
-
- vq->vq_desc_tail_idx = id;
- dxp->next = VQ_RING_DESC_CHAIN_END;
-}
-
void
virtio_update_packet_stats(struct virtnet_stats *stats, struct rte_mbuf *mbuf)
{
@@ -264,130 +247,6 @@ virtqueue_dequeue_rx_inorder(struct virtqueue *vq,
return i;
}
-#ifndef DEFAULT_TX_FREE_THRESH
-#define DEFAULT_TX_FREE_THRESH 32
-#endif
-
-static void
-virtio_xmit_cleanup_inorder_packed(struct virtqueue *vq, int num)
-{
- uint16_t used_idx, id, curr_id, free_cnt = 0;
- uint16_t size = vq->vq_nentries;
- struct vring_packed_desc *desc = vq->vq_packed.ring.desc;
- struct vq_desc_extra *dxp;
-
- used_idx = vq->vq_used_cons_idx;
- /* desc_is_used has a load-acquire or rte_cio_rmb inside
-* and wait for used desc in virtqueue.
-*/
- while (num > 0 && desc_is_used(&desc[used_idx], vq)) {
- id = desc[used_idx].id;
- do {
- curr_id = used_idx;
- dxp = &vq->vq_descx[used_idx];
- used_idx += dxp->ndescs;
- free_cnt += dxp->ndescs;
- num -= dxp->ndescs;
- if (used_idx >= size) {
- used_idx -= size;
- vq->vq_packed.used_wrap_counter ^= 1;
- }
- if (dxp->cookie != NULL) {
- rte_pktmbuf_free(dxp->cookie);
- dxp->cookie = NULL;
- }
- } while (curr_id != id);
- }
- vq->vq_used_cons_idx = used_idx;
- vq->vq_free_cnt += free_cnt;
-}
-
-static void
-virtio_xmit_cleanup_normal_packed(struct virtqueue *vq, int num)
-{
- uint16_t used_idx, id;
- uint16_t size = vq->vq_nentries;
- struct vring_packed_desc *desc = vq->vq_packed.ring.desc;
- struct vq_desc_extra *dxp;
-
- used_idx = vq->vq_used_cons_idx;
- /* desc_is_used has a load-acquire or rte_cio_rmb inside
-* and wait for used desc in virtqueue.
-*/
- while (num-- && desc_is_used(&desc[used_idx], vq)) {
- id = desc[used_idx].id;
- dxp = &vq->vq_descx[id];
- vq->vq_used_cons_idx += dxp->ndescs;
- if (vq->vq_used_cons_idx >= size) {
- vq->vq_used_cons_idx -= size;
- vq->vq_packed.used_wrap_counter ^= 1;
- }
- vq_ring_free_id_packed(vq, id);
- if (dxp->cookie != NULL) {
- rte_pktmbuf_free(dxp->cookie);
- dxp->cookie = NULL;
- }
- used_idx = vq->vq_used_cons_idx;
- }
-}
-
-/* Cleanup from completed transmits. */
-static inline void
-virtio_xmit_cleanup_packed(struct virtqueue *vq, int num, int in_order)
-{
- if (in_order)
- virtio_xmit_cleanup_inorder_packed(vq, num);
- else
- virtio_xmit_cleanup_normal_packed(vq, num);
-}
-
-static void
-virtio_xmit_cleanup(struct virtqueue *vq, uint16_t num)
-{
- uint16_t i, used_idx, desc_idx;
- for (i = 0; i < num; i++) {
- struct vring_used_elem *uep;
- struct vq_desc_extra *dxp;
-
- used_idx = (uint16_t)(vq->vq_used_cons_idx & (vq->vq_nentries -
1));
- uep = &vq->vq_split.ring.used->ring[used_idx];
-
- desc_idx = (uint16_t) uep->id;
- dxp = &vq->vq_descx[desc_idx];
- vq->vq_used_cons_idx++;
- vq_ring_free_chain(vq, desc_idx);
-
- if (dxp->cookie != NULL) {
- rte_pktmbuf_free(dxp->cookie);
- dxp->cookie = NULL;
- }
- }
-}
-
-/* Cleanup from completed inorder transmits. */
-static __rte_always_inline void
-virtio_xmit_cleanup_inorder(struct virtqueue *vq, uint16_t num)
-{
- uint16_t i, idx = vq->vq_used_cons_idx;
- int16_t free_cnt = 0;
- struct vq_desc_extra *dxp = NULL;
-
- if (unlikely(num == 0))
- return;
-
- for
[dpdk-dev] [PATCH v10 8/9] net/virtio: add election for vectorized path
Rewrite vectorized path selection logic. Default setting comes from
vectorized devarg, then checks each criteria.
Packed ring vectorized path need:
AVX512F and required extensions are supported by compiler and host
VERSION_1 and IN_ORDER features are negotiated
mergeable feature is not negotiated
LRO offloading is disabled
Split ring vectorized rx path need:
mergeable and IN_ORDER features are not negotiated
LRO, chksum and vlan strip offloadings are disabled
Signed-off-by: Marvin Liu
Reviewed-by: Maxime Coquelin
diff --git a/drivers/net/virtio/virtio_ethdev.c
b/drivers/net/virtio/virtio_ethdev.c
index 0a69a4db1..f8ff41d99 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1523,9 +1523,12 @@ set_rxtx_funcs(struct rte_eth_dev *eth_dev)
if (vtpci_packed_queue(hw)) {
PMD_INIT_LOG(INFO,
"virtio: using packed ring %s Tx path on port %u",
- hw->use_inorder_tx ? "inorder" : "standard",
+ hw->use_vec_tx ? "vectorized" : "standard",
eth_dev->data->port_id);
- eth_dev->tx_pkt_burst = virtio_xmit_pkts_packed;
+ if (hw->use_vec_tx)
+ eth_dev->tx_pkt_burst = virtio_xmit_pkts_packed_vec;
+ else
+ eth_dev->tx_pkt_burst = virtio_xmit_pkts_packed;
} else {
if (hw->use_inorder_tx) {
PMD_INIT_LOG(INFO, "virtio: using inorder Tx path on
port %u",
@@ -1539,7 +1542,13 @@ set_rxtx_funcs(struct rte_eth_dev *eth_dev)
}
if (vtpci_packed_queue(hw)) {
- if (vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) {
+ if (hw->use_vec_rx) {
+ PMD_INIT_LOG(INFO,
+ "virtio: using packed ring vectorized Rx path
on port %u",
+ eth_dev->data->port_id);
+ eth_dev->rx_pkt_burst =
+ &virtio_recv_pkts_packed_vec;
+ } else if (vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) {
PMD_INIT_LOG(INFO,
"virtio: using packed ring mergeable buffer Rx
path on port %u",
eth_dev->data->port_id);
@@ -1952,8 +1961,17 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
goto err_virtio_init;
if (vectorized) {
- if (!vtpci_packed_queue(hw))
+ if (!vtpci_packed_queue(hw)) {
+ hw->use_vec_rx = 1;
+ } else {
+#if !defined(CC_AVX512_SUPPORT)
+ PMD_DRV_LOG(INFO,
+ "building environment do not support packed
ring vectorized");
+#else
hw->use_vec_rx = 1;
+ hw->use_vec_tx = 1;
+#endif
+ }
}
hw->opened = true;
@@ -2102,8 +2120,8 @@ virtio_dev_devargs_parse(struct rte_devargs *devargs, int
*vdpa,
if (vectorized &&
rte_kvargs_count(kvlist, VIRTIO_ARG_VECTORIZED) == 1) {
ret = rte_kvargs_process(kvlist,
- VIRTIO_ARG_VECTORIZED,
- vectorized_check_handler, vectorized);
+ VIRTIO_ARG_VECTORIZED,
+ vectorized_check_handler, vectorized);
if (ret < 0) {
PMD_INIT_LOG(ERR, "Failed to parse %s",
VIRTIO_ARG_VECTORIZED);
@@ -2288,31 +2306,61 @@ virtio_dev_configure(struct rte_eth_dev *dev)
return -EBUSY;
}
- if (vtpci_with_feature(hw, VIRTIO_F_IN_ORDER)) {
- hw->use_inorder_tx = 1;
- hw->use_inorder_rx = 1;
- hw->use_vec_rx = 0;
- }
-
if (vtpci_packed_queue(hw)) {
- hw->use_vec_rx = 0;
- hw->use_inorder_rx = 0;
- }
+ if ((hw->use_vec_rx || hw->use_vec_tx) &&
+ (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX512F) ||
+!vtpci_with_feature(hw, VIRTIO_F_IN_ORDER) ||
+!vtpci_with_feature(hw, VIRTIO_F_VERSION_1))) {
+ PMD_DRV_LOG(INFO,
+ "disabled packed ring vectorized path for
requirements not met");
+ hw->use_vec_rx = 0;
+ hw->use_vec_tx = 0;
+ }
+ if (hw->use_vec_rx) {
+ if (vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) {
+ PMD_DRV_LOG(INFO,
+ "disabled packed ring vectorized rx for
mrg_rxbuf enabled");
+ hw->use_vec_rx = 0;
+ }
+
+ if (rx_off
[dpdk-dev] [PATCH v10 9/9] doc: add packed vectorized path
Document packed virtqueue vectorized path selection logic in virtio net PMD. Signed-off-by: Marvin Liu Reviewed-by: Maxime Coquelin diff --git a/doc/guides/nics/virtio.rst b/doc/guides/nics/virtio.rst index d59add23e..dbcf49ae1 100644 --- a/doc/guides/nics/virtio.rst +++ b/doc/guides/nics/virtio.rst @@ -482,6 +482,13 @@ according to below configuration: both negotiated, this path will be selected. #. Packed virtqueue in-order non-mergeable path: If in-order feature is negotiated and Rx mergeable is not negotiated, this path will be selected. +#. Packed virtqueue vectorized Rx path: If building and running environment support + AVX512 && in-order feature is negotiated && Rx mergeable is not negotiated && + TCP_LRO Rx offloading is disabled && vectorized option enabled, + this path will be selected. +#. Packed virtqueue vectorized Tx path: If building and running environment support + AVX512 && in-order feature is negotiated && vectorized option enabled, + this path will be selected. Rx/Tx callbacks of each Virtio path ~~~ @@ -504,6 +511,8 @@ are shown in below table: Packed virtqueue non-meregable path virtio_recv_pkts_packed virtio_xmit_pkts_packed Packed virtqueue in-order mergeable path virtio_recv_mergeable_pkts_packed virtio_xmit_pkts_packed Packed virtqueue in-order non-mergeable path virtio_recv_pkts_packed virtio_xmit_pkts_packed + Packed virtqueue vectorized Rx path virtio_recv_pkts_packed_vec virtio_xmit_pkts_packed + Packed virtqueue vectorized Tx path virtio_recv_pkts_packed virtio_xmit_pkts_packed_vec = Virtio paths Support Status from Release to Release @@ -521,20 +530,22 @@ All virtio paths support status are shown in below table: .. table:: Virtio Paths and Releases - = = = - Virtio paths 16.11 ~ 18.05 18.08 ~ 18.11 19.02 ~ 19.11 - = = = - Split virtqueue mergeable path Y Y Y - Split virtqueue non-mergeable path Y Y Y - Split virtqueue vectorized Rx path Y Y Y - Split virtqueue simple Tx path Y N N - Split virtqueue in-order mergeable path Y Y - Split virtqueue in-order non-mergeable path Y Y - Packed virtqueue mergeable path Y - Packed virtqueue non-mergeable path Y - Packed virtqueue in-order mergeable path Y - Packed virtqueue in-order non-mergeable path Y - = = = + = = = === + Virtio paths 16.11 ~ 18.05 18.08 ~ 18.11 19.02 ~ 19.11 20.05 ~ + = = = === + Split virtqueue mergeable path Y Y Y Y + Split virtqueue non-mergeable path Y Y Y Y + Split virtqueue vectorized Rx path Y Y Y Y + Split virtqueue simple Tx path Y N N N + Split virtqueue in-order mergeable path Y Y Y + Split virtqueue in-order non-mergeable path Y Y Y + Packed virtqueue mergeable path Y Y + Packed virtqueue non-mergeable path Y Y + Packed virtqueue in-order mergeable path Y Y + Packed virtqueue in-order non-mergeable path Y Y + Packed virtqueue vectorized Rx path Y + Packed virtqueue vectorized Tx path Y + = = = === QEMU Support Status ~~~ -- 2.17.1
[dpdk-dev] [PATCH v10 6/9] net/virtio: add vectorized packed ring Rx path
Optimize packed ring Rx path with SIMD instructions. Solution of
optimization is pretty like vhost, is that split path into batch and
single functions. Batch function is further optimized by AVX512
instructions. Also pad desc extra structure to 16 bytes aligned, thus
four elements will be saved in one batch.
Signed-off-by: Marvin Liu
diff --git a/drivers/net/virtio/Makefile b/drivers/net/virtio/Makefile
index c9edb84ee..102b1deab 100644
--- a/drivers/net/virtio/Makefile
+++ b/drivers/net/virtio/Makefile
@@ -36,6 +36,41 @@ else ifneq ($(filter y,$(CONFIG_RTE_ARCH_ARM)
$(CONFIG_RTE_ARCH_ARM64)),)
SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx_simple_neon.c
endif
+ifneq ($(FORCE_DISABLE_AVX512), y)
+ CC_AVX512_SUPPORT=\
+ $(shell $(CC) -march=native -dM -E - &1 | \
+ sed '/./{H;$$!d} ; x ; /AVX512F/!d; /AVX512BW/!d; /AVX512VL/!d' | \
+ grep -q AVX512 && echo 1)
+endif
+
+ifeq ($(CC_AVX512_SUPPORT), 1)
+CFLAGS += -DCC_AVX512_SUPPORT
+SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx_packed_avx.c
+
+ifeq ($(RTE_TOOLCHAIN), gcc)
+ifeq ($(shell test $(GCC_VERSION) -ge 83 && echo 1), 1)
+CFLAGS += -DVIRTIO_GCC_UNROLL_PRAGMA
+endif
+endif
+
+ifeq ($(RTE_TOOLCHAIN), clang)
+ifeq ($(shell test $(CLANG_MAJOR_VERSION)$(CLANG_MINOR_VERSION) -ge 37 && echo
1), 1)
+CFLAGS += -DVIRTIO_CLANG_UNROLL_PRAGMA
+endif
+endif
+
+ifeq ($(RTE_TOOLCHAIN), icc)
+ifeq ($(shell test $(ICC_MAJOR_VERSION) -ge 16 && echo 1), 1)
+CFLAGS += -DVIRTIO_ICC_UNROLL_PRAGMA
+endif
+endif
+
+CFLAGS_virtio_rxtx_packed_avx.o += -mavx512f -mavx512bw -mavx512vl
+ifeq ($(shell test $(GCC_VERSION) -ge 100 && echo 1), 1)
+CFLAGS_virtio_rxtx_packed_avx.o += -Wno-zero-length-bounds
+endif
+endif
+
ifeq ($(CONFIG_RTE_VIRTIO_USER),y)
SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_user/vhost_user.c
SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_user/vhost_kernel.c
diff --git a/drivers/net/virtio/meson.build b/drivers/net/virtio/meson.build
index 15150eea1..8e68c3039 100644
--- a/drivers/net/virtio/meson.build
+++ b/drivers/net/virtio/meson.build
@@ -9,6 +9,20 @@ sources += files('virtio_ethdev.c',
deps += ['kvargs', 'bus_pci']
if arch_subdir == 'x86'
+ if '-mno-avx512f' not in machine_args
+ if cc.has_argument('-mavx512f') and
cc.has_argument('-mavx512vl') and cc.has_argument('-mavx512bw')
+ cflags += ['-mavx512f', '-mavx512bw', '-mavx512vl']
+ cflags += ['-DCC_AVX512_SUPPORT']
+ if (toolchain == 'gcc' and
cc.version().version_compare('>=8.3.0'))
+ cflags += '-DVHOST_GCC_UNROLL_PRAGMA'
+ elif (toolchain == 'clang' and
cc.version().version_compare('>=3.7.0'))
+ cflags += '-DVHOST_CLANG_UNROLL_PRAGMA'
+ elif (toolchain == 'icc' and
cc.version().version_compare('>=16.0.0'))
+ cflags += '-DVHOST_ICC_UNROLL_PRAGMA'
+ endif
+ sources += files('virtio_rxtx_packed_avx.c')
+ endif
+ endif
sources += files('virtio_rxtx_simple_sse.c')
elif arch_subdir == 'ppc'
sources += files('virtio_rxtx_simple_altivec.c')
diff --git a/drivers/net/virtio/virtio_ethdev.h
b/drivers/net/virtio/virtio_ethdev.h
index febaf17a8..5c112cac7 100644
--- a/drivers/net/virtio/virtio_ethdev.h
+++ b/drivers/net/virtio/virtio_ethdev.h
@@ -105,6 +105,9 @@ uint16_t virtio_xmit_pkts_inorder(void *tx_queue, struct
rte_mbuf **tx_pkts,
uint16_t virtio_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
uint16_t nb_pkts);
+uint16_t virtio_recv_pkts_packed_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
+ uint16_t nb_pkts);
+
int eth_virtio_dev_init(struct rte_eth_dev *eth_dev);
void virtio_interrupt_handler(void *param);
diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index a549991aa..534562cca 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -2030,3 +2030,11 @@ virtio_xmit_pkts_inorder(void *tx_queue,
return nb_tx;
}
+
+__rte_weak uint16_t
+virtio_recv_pkts_packed_vec(void *rx_queue __rte_unused,
+ struct rte_mbuf **rx_pkts __rte_unused,
+ uint16_t nb_pkts __rte_unused)
+{
+ return 0;
+}
diff --git a/drivers/net/virtio/virtio_rxtx_packed_avx.c
b/drivers/net/virtio/virtio_rxtx_packed_avx.c
new file mode 100644
index 0..8a7b459eb
--- /dev/null
+++ b/drivers/net/virtio/virtio_rxtx_packed_avx.c
@@ -0,0 +1,374 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2010-2020 Intel Corporation
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#include "virtio_logs.h"
+#include "virtio_ethdev.h"
+#include "virtio_pci.h"
+#include "virtqueue.h"
+
+#define BYTE_SIZE 8
+/* flag bits offset in packed ring desc higher 64bits */
+#define FLAGS_BITS_OFF
[dpdk-dev] [PATCH v10 7/9] net/virtio: add vectorized packed ring Tx path
Optimize packed ring Tx path like Rx path. Split Tx path into batch and
single Tx functions. Batch function is further optimized by AVX512
instructions.
Signed-off-by: Marvin Liu
diff --git a/drivers/net/virtio/virtio_ethdev.h
b/drivers/net/virtio/virtio_ethdev.h
index 5c112cac7..b7d52d497 100644
--- a/drivers/net/virtio/virtio_ethdev.h
+++ b/drivers/net/virtio/virtio_ethdev.h
@@ -108,6 +108,9 @@ uint16_t virtio_recv_pkts_vec(void *rx_queue, struct
rte_mbuf **rx_pkts,
uint16_t virtio_recv_pkts_packed_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
uint16_t nb_pkts);
+uint16_t virtio_xmit_pkts_packed_vec(void *tx_queue, struct rte_mbuf **tx_pkts,
+ uint16_t nb_pkts);
+
int eth_virtio_dev_init(struct rte_eth_dev *eth_dev);
void virtio_interrupt_handler(void *param);
diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index 534562cca..460e9d4a2 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -2038,3 +2038,11 @@ virtio_recv_pkts_packed_vec(void *rx_queue __rte_unused,
{
return 0;
}
+
+__rte_weak uint16_t
+virtio_xmit_pkts_packed_vec(void *tx_queue __rte_unused,
+ struct rte_mbuf **tx_pkts __rte_unused,
+ uint16_t nb_pkts __rte_unused)
+{
+ return 0;
+}
diff --git a/drivers/net/virtio/virtio_rxtx_packed_avx.c
b/drivers/net/virtio/virtio_rxtx_packed_avx.c
index 8a7b459eb..43cee4244 100644
--- a/drivers/net/virtio/virtio_rxtx_packed_avx.c
+++ b/drivers/net/virtio/virtio_rxtx_packed_avx.c
@@ -23,6 +23,24 @@
#define PACKED_FLAGS_MASK ((0ULL | VRING_PACKED_DESC_F_AVAIL_USED) << \
FLAGS_BITS_OFFSET)
+/* reference count offset in mbuf rearm data */
+#define REFCNT_BITS_OFFSET ((offsetof(struct rte_mbuf, refcnt) - \
+ offsetof(struct rte_mbuf, rearm_data)) * BYTE_SIZE)
+/* segment number offset in mbuf rearm data */
+#define SEG_NUM_BITS_OFFSET ((offsetof(struct rte_mbuf, nb_segs) - \
+ offsetof(struct rte_mbuf, rearm_data)) * BYTE_SIZE)
+
+/* default rearm data */
+#define DEFAULT_REARM_DATA (1ULL << SEG_NUM_BITS_OFFSET | \
+ 1ULL << REFCNT_BITS_OFFSET)
+
+/* id bits offset in packed ring desc higher 64bits */
+#define ID_BITS_OFFSET ((offsetof(struct vring_packed_desc, id) - \
+ offsetof(struct vring_packed_desc, len)) * BYTE_SIZE)
+
+/* net hdr short size mask */
+#define NET_HDR_MASK 0x3F
+
#define PACKED_BATCH_SIZE (RTE_CACHE_LINE_SIZE / \
sizeof(struct vring_packed_desc))
#define PACKED_BATCH_MASK (PACKED_BATCH_SIZE - 1)
@@ -60,6 +78,237 @@ virtio_update_batch_stats(struct virtnet_stats *stats,
stats->bytes += pkt_len4;
}
+static inline int
+virtqueue_enqueue_batch_packed_vec(struct virtnet_tx *txvq,
+ struct rte_mbuf **tx_pkts)
+{
+ struct virtqueue *vq = txvq->vq;
+ uint16_t head_size = vq->hw->vtnet_hdr_size;
+ uint16_t idx = vq->vq_avail_idx;
+ struct virtio_net_hdr *hdr;
+ uint16_t i, cmp;
+
+ if (vq->vq_avail_idx & PACKED_BATCH_MASK)
+ return -1;
+
+ if (unlikely((idx + PACKED_BATCH_SIZE) > vq->vq_nentries))
+ return -1;
+
+ /* Load four mbufs rearm data */
+ RTE_BUILD_BUG_ON(REFCNT_BITS_OFFSET >= 64);
+ RTE_BUILD_BUG_ON(SEG_NUM_BITS_OFFSET >= 64);
+ __m256i mbufs = _mm256_set_epi64x(*tx_pkts[3]->rearm_data,
+ *tx_pkts[2]->rearm_data,
+ *tx_pkts[1]->rearm_data,
+ *tx_pkts[0]->rearm_data);
+
+ /* refcnt=1 and nb_segs=1 */
+ __m256i mbuf_ref = _mm256_set1_epi64x(DEFAULT_REARM_DATA);
+ __m256i head_rooms = _mm256_set1_epi16(head_size);
+
+ /* Check refcnt and nb_segs */
+ const __mmask16 mask = 0x6 | 0x6 << 4 | 0x6 << 8 | 0x6 << 12;
+ cmp = _mm256_mask_cmpneq_epu16_mask(mask, mbufs, mbuf_ref);
+ if (unlikely(cmp))
+ return -1;
+
+ /* Check headroom is enough */
+ const __mmask16 data_mask = 0x1 | 0x1 << 4 | 0x1 << 8 | 0x1 << 12;
+ RTE_BUILD_BUG_ON(offsetof(struct rte_mbuf, data_off) !=
+ offsetof(struct rte_mbuf, rearm_data));
+ cmp = _mm256_mask_cmplt_epu16_mask(data_mask, mbufs, head_rooms);
+ if (unlikely(cmp))
+ return -1;
+
+ __m512i v_descx = _mm512_set_epi64(0x1, (uint64_t)tx_pkts[3],
+ 0x1, (uint64_t)tx_pkts[2],
+ 0x1, (uint64_t)tx_pkts[1],
+ 0x1, (uint64_t)tx_pkts[0]);
+
+ _mm512_storeu_si512((void *)&vq->vq_descx[idx], v_descx);
+
+ virtio_for_each_try_unroll(i, 0, PACKED_BATCH_SIZE) {
+ tx_pkts[i]->data_off -= head_size;
+ tx_pkts[i]->data_len += head_size;
+ }
+
+#ifdef RTE_VIRTIO_USER
+ __m512i descs_base = _mm512_set_epi64(tx_pkts[3]-
[dpdk-dev] [PATCH] net/mlx5: save meter index instead of meter id
Currently, while creating the flow with meter, meter id is saved to the
rte flow. While destroying the flow, the meter object will be found by
the meter id, so the meter object will be released accordingly. But as
the meter id is configured by user, while the meter id is set to 0, it
doesn't make any sense to flow destroy since 0 means flow doesn't have
meter. The meter object with id 0 will be leaked.
As meter object is allocated from indexed memory, and the index starts
from 1, save the internal generated index instead of user defined meter
id will never meet the issue as above.
This patch saves meter index instead of meter id in rte flow.
Signed-off-by: Suanming Mou
---
drivers/net/mlx5/mlx5_flow_dv.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 6263ecc..2fdd403 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -7867,11 +7867,12 @@ struct field_modify_info modify_tcp[] = {
NULL,
"meter not found "
"or invalid parameters");
- flow->meter = fm->meter_id;
+ flow->meter = fm->idx;
}
/* Set the meter action. */
if (!fm) {
- fm = mlx5_flow_meter_find(priv, flow->meter);
+ fm = mlx5_ipool_get(priv->sh->ipool
+ [MLX5_IPOOL_MTR], flow->meter);
if (!fm)
return rte_flow_error_set(error,
rte_errno,
@@ -8591,7 +8592,8 @@ struct field_modify_info modify_tcp[] = {
if (flow->meter) {
struct mlx5_flow_meter *fm;
- fm = mlx5_flow_meter_find(priv, flow->meter);
+ fm = mlx5_ipool_get(priv->sh->ipool[MLX5_IPOOL_MTR],
+ flow->meter);
if (fm)
mlx5_flow_meter_detach(fm);
flow->meter = 0;
--
1.8.3.1
Re: [dpdk-dev] [PATCH] mem: mark pages as not accessed when returning back to memory pool
Thanks,
Feng Li
David Marchand 于2020年4月25日周六 上午1:10写道：
>
> On Fri, Apr 24, 2020 at 12:43 PM Li Feng wrote:
> >
> > Commit 8a4baf06c17a ("mem: mark pages as not accessed when reserving VA")
> > has mapped the initialized memory with PROT_NONE, and when it's unmapped,
> > eal_memalloc.c should remmap the anonymous memory with PROT_NONE too.
> >
> > Signed-off-by: Li Feng
>
> Fixes: 8a4baf06c17a ("mem: mark pages as not accessed when reserving VA")
> Cc: sta...@dpdk.org
>
> Acked-by: Anatoly Burakov
>
> Applied, thanks.
>
>
>
> Please for future contributions, could you get this footer removed?
I think It's attached by the corporation mail server.
Maybe I need ask IT department for help.
>
> > The SmartX email address is only for business purpose. Any sent message
> > that is not related to the business is not authorized or permitted by
> > SmartX.
> > 本邮箱为北京志凌海纳科技有限公司（SmartX）工作邮箱. 如本邮箱发出的邮件与工作无关,该邮件未得到本公司任何的明示或默示的授权.
>
>
> --
> David Marchand
>
--
The SmartX email address is only for business purpose. Any sent message
that is not related to the business is not authorized or permitted by
SmartX.
本邮箱为北京志凌海纳科技有限公司（SmartX）工作邮箱. 如本邮箱发出的邮件与工作无关,该邮件未得到本公司任何的明示或默示的授权.
[dpdk-dev] [PATCH 0/2] eal/windows: fix build by supporing trace
This patch fixes errors caused by using Unix-only functions in tracing EAL. It introduces new internal EAL wrappers for directory management and provides simple, but correct implementation for some EAL functions required for tracing. This patch implements rte_get_tsc_hz() instead of basing upon a pending patchset, because fixing the build allows testing said patchset in the first place, and also re-implemented code is only a few lines. Dmitry Kozlyuk (2): eal/windows: replace sys/queue.h with a complete one from FreeBSD eal/windows: fix build by supporting trace config/meson.build| 2 + .../common/eal_common_trace_utils.c | 29 +- lib/librte_eal/common/eal_private.h | 26 + lib/librte_eal/common/meson.build | 5 + lib/librte_eal/freebsd/Makefile | 4 + .../include/generic/rte_byteorder.h | 4 +- lib/librte_eal/linux/Makefile | 4 + lib/librte_eal/meson.build| 4 + lib/librte_eal/unix/eal_unix.c| 45 ++ lib/librte_eal/unix/meson.build | 6 + lib/librte_eal/windows/eal.c | 91 +++ lib/librte_eal/windows/eal_thread.c | 9 + lib/librte_eal/windows/eal_windows.h | 3 + lib/librte_eal/windows/include/rte_os.h | 33 +- lib/librte_eal/windows/include/sys/queue.h| 663 -- 15 files changed, 838 insertions(+), 90 deletions(-) create mode 100644 lib/librte_eal/unix/eal_unix.c create mode 100644 lib/librte_eal/unix/meson.build -- 2.25.1
[dpdk-dev] [PATCH 2/2] eal/windows: fix build by supporting trace
Add EAL private functions to support trace storage:
* eal_persistent_data_path()
* eal_dir_create()
Replace clock_gettime(CLOCK_REALTIME) with C11 timespec_get().
Implementation is provided for MinGW-w64 that misses this function.
Provide minimum viable implementations of malloc and timer functions
used by tracing.
Fixes: 185b7dc1d467 ("trace: save bootup timestamp")
Fixes: 321dd5f8fa62 ("trace: add internal init and fini interface")
Reported-by: Pallavi Kadam
Signed-off-by: Dmitry Kozlyuk
---
config/meson.build| 2 +
.../common/eal_common_trace_utils.c | 29 ++
lib/librte_eal/common/eal_private.h | 26 ++
lib/librte_eal/common/meson.build | 5 +
lib/librte_eal/freebsd/Makefile | 4 +
.../include/generic/rte_byteorder.h | 4 +-
lib/librte_eal/linux/Makefile | 4 +
lib/librte_eal/meson.build| 4 +
lib/librte_eal/unix/eal_unix.c| 45 +
lib/librte_eal/unix/meson.build | 6 ++
lib/librte_eal/windows/eal.c | 91 +++
lib/librte_eal/windows/eal_thread.c | 9 ++
lib/librte_eal/windows/eal_windows.h | 3 +
lib/librte_eal/windows/include/rte_os.h | 33 +--
14 files changed, 237 insertions(+), 28 deletions(-)
create mode 100644 lib/librte_eal/unix/eal_unix.c
create mode 100644 lib/librte_eal/unix/meson.build
diff --git a/config/meson.build b/config/meson.build
index e851b407b..91cba9313 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -267,6 +267,8 @@ if is_windows
# Minimum supported API is Windows 7.
add_project_arguments('-D_WIN32_WINNT=0x0601', language: 'c')
+ add_project_link_arguments(['-lshell32', '-lshlwapi'], language: 'c')
+
# Use MinGW-w64 stdio, because DPDK assumes ANSI-compliant formatting.
if cc.get_id() == 'gcc'
add_project_arguments('-D__USE_MINGW_ANSI_STDIO', language: 'c')
diff --git a/lib/librte_eal/common/eal_common_trace_utils.c
b/lib/librte_eal/common/eal_common_trace_utils.c
index fce8892c3..1fb5bc772 100644
--- a/lib/librte_eal/common/eal_common_trace_utils.c
+++ b/lib/librte_eal/common/eal_common_trace_utils.c
@@ -3,12 +3,11 @@
*/
#include
-#include
-#include
#include
#include
#include
+#include
#include
#include "eal_filesystem.h"
@@ -302,7 +301,7 @@ trace_epoch_time_save(void)
uint64_t avg, start, end;
start = rte_get_tsc_cycles();
- if (clock_gettime(CLOCK_REALTIME, &epoch) < 0) {
+ if (timespec_get(&epoch, TIME_UTC) < 0) {
trace_err("failed to get the epoch time");
return -1;
}
@@ -321,22 +320,14 @@ trace_dir_default_path_get(char *dir_path)
{
struct trace *trace = trace_obj_get();
uint32_t size = sizeof(trace->dir);
- struct passwd *pwd;
- char *home_dir;
-
- /* First check for shell environment variable */
- home_dir = getenv("HOME");
- if (home_dir == NULL) {
- /* Fallback to password file entry */
- pwd = getpwuid(getuid());
- if (pwd == NULL)
- return -EINVAL;
-
- home_dir = pwd->pw_dir;
- }
+ const char *perm_dir;
+
+ perm_dir = eal_permanent_data_path();
+ if (perm_dir == NULL)
+ return -EINVAL;
/* Append dpdk-traces to directory */
- if (snprintf(dir_path, size, "%s/dpdk-traces/", home_dir) < 0)
+ if (snprintf(dir_path, size, "%s/dpdk-traces/", perm_dir) < 0)
return -ENAMETOOLONG;
return 0;
@@ -371,7 +362,7 @@ trace_mkdir(void)
}
/* Create the path if it t exist, no "mkdir -p" available here */
- rc = mkdir(trace->dir, 0700);
+ rc = eal_dir_create(trace->dir);
if (rc < 0 && errno != EEXIST) {
trace_err("mkdir %s failed [%s]", trace->dir, strerror(errno));
rte_errno = errno;
@@ -385,7 +376,7 @@ trace_mkdir(void)
if (rc < 0)
return rc;
- rc = mkdir(trace->dir, 0700);
+ rc = eal_dir_create(trace->dir);
if (rc < 0) {
trace_err("mkdir %s failed [%s]", trace->dir, strerror(errno));
rte_errno = errno;
diff --git a/lib/librte_eal/common/eal_private.h
b/lib/librte_eal/common/eal_private.h
index ecf827914..b3504e484 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -448,4 +448,30 @@ eal_malloc_no_trace(const char *type, size_t size,
unsigned int align);
void eal_free_no_trace(void *addr);
+/**
+ * Get absolute path to the directory where permanent data can be stored.
+ *
+ * @return
+ * Statically allocated string on success, NULL on failure.
+ */
+const char *
+eal_permanent_data_path(void);
+
+/**
+ * Create a directory accessible to the current user only.
+ *
+ * This function does n
[dpdk-dev] [PATCH 1/2] eal/windows: replace sys/queue.h with a complete one from FreeBSD
Limited version imported previously lacks STAILQ macros used by tracing
and SLIST macros used by memory management. Import a complete file from
FreeBSD, since its license exception is already approved by Technical
Board.
Fixes: 321dd5f8fa62 ("trace: add internal init and fini interface")
Signed-off-by: Dmitry Kozlyuk
---
lib/librte_eal/windows/include/sys/queue.h | 663 +++--
1 file changed, 601 insertions(+), 62 deletions(-)
diff --git a/lib/librte_eal/windows/include/sys/queue.h
b/lib/librte_eal/windows/include/sys/queue.h
index a65949a78..9756bee6f 100644
--- a/lib/librte_eal/windows/include/sys/queue.h
+++ b/lib/librte_eal/windows/include/sys/queue.h
@@ -8,7 +8,36 @@
#define_SYS_QUEUE_H_
/*
- * This file defines tail queues.
+ * This file defines four types of data structures: singly-linked lists,
+ * singly-linked tail queues, lists and tail queues.
+ *
+ * A singly-linked list is headed by a single forward pointer. The elements
+ * are singly linked for minimum space and pointer manipulation overhead at
+ * the expense of O(n) removal for arbitrary elements. New elements can be
+ * added to the list after an existing element or at the head of the list.
+ * Elements being removed from the head of the list should use the explicit
+ * macro for this purpose for optimum efficiency. A singly-linked list may
+ * only be traversed in the forward direction. Singly-linked lists are ideal
+ * for applications with large datasets and few or no removals or for
+ * implementing a LIFO queue.
+ *
+ * A singly-linked tail queue is headed by a pair of pointers, one to the
+ * head of the list and the other to the tail of the list. The elements are
+ * singly linked for minimum space and pointer manipulation overhead at the
+ * expense of O(n) removal for arbitrary elements. New elements can be added
+ * to the list after an existing element, at the head of the list, or at the
+ * end of the list. Elements being removed from the head of the tail queue
+ * should use the explicit macro for this purpose for optimum efficiency.
+ * A singly-linked tail queue may only be traversed in the forward direction.
+ * Singly-linked tail queues are ideal for applications with large datasets
+ * and few or no removals or for implementing a FIFO queue.
+ *
+ * A list is headed by a single forward pointer (or an array of forward
+ * pointers for a hash table header). The elements are doubly linked
+ * so that an arbitrary element can be removed without a need to
+ * traverse the list. New elements can be added to the list before
+ * or after an existing element or at the head of the list. A list
+ * may be traversed in either direction.
*
* A tail queue is headed by a pair of pointers, one to the head of the
* list and the other to the tail of the list. The elements are doubly
@@ -17,65 +46,93 @@
* after an existing element, at the head of the list, or at the end of
* the list. A tail queue may be traversed in either direction.
*
+ * For details on the use of these macros, see the queue(3) manual page.
+ *
* Below is a summary of implemented functions where:
* + means the macro is available
* - means the macro is not available
* s means the macro is available but is slow (runs in O(n) time)
*
- * TAILQ
- * _HEAD +
- * _CLASS_HEAD +
- * _HEAD_INITIALIZER +
- * _ENTRY +
- * _CLASS_ENTRY+
- * _INIT +
- * _EMPTY +
- * _FIRST +
- * _NEXT +
- * _PREV +
- * _LAST +
- * _LAST_FAST +
- * _FOREACH+
- * _FOREACH_FROM +
- * _FOREACH_SAFE +
- * _FOREACH_FROM_SAFE +
- * _FOREACH_REVERSE+
- * _FOREACH_REVERSE_FROM +
- * _FOREACH_REVERSE_SAFE +
- * _FOREACH_REVERSE_FROM_SAFE +
- * _INSERT_HEAD+
- * _INSERT_BEFORE +
- * _INSERT_AFTER +
- * _INSERT_TAIL+
- * _CONCAT +
- * _REMOVE_AFTER -
- * _REMOVE_HEAD-
- * _REMOVE +
- * _SWAP +
+ * SLIST LISTSTAILQ TAILQ
+ * _HEAD + + + +
+ * _CLASS_HEAD + + + +
+ * _HEAD_INITIALIZER + + + +
+ * _ENTRY + + + +
+ * _CLASS_ENTRY+ + + +
+ * _INIT + + + +
+ * _EMPTY + + + +
+ * _FIRST + + + +
+ * _NEXT + + + +
+ * _PREV - + - +
+ * _
Re: [dpdk-dev] [DPDK] net/ixgbe: fix status synchronization on BSD
Sorry, because the mail filtering is wrong, I have not seen this mail, and the community canceled the patch. I resubmitted a pacth, only modified the format. Judging this flag, it feels a bit more complicated. New patch: http://patches.dpdk.org/patch/68713/ -Original Message- From: Stephen Hemminger [mailto:step...@networkplumber.org] Sent: Friday, March 27, 2020 6:02 AM To: Peng, ZhihongX Cc: Ye, Xiaolong ; Lu, Wenzhuo ; Ananyev, Konstantin ; dev@dpdk.org; Wang, Liang-min Subject: Re: [dpdk-dev] [DPDK] net/ixgbe: fix status synchronization on BSD On Tue, 24 Mar 2020 23:31:11 -0400 zhihongx.p...@intel.com wrote: > +/*BSD has no interrupt mechanism, so force NIC status > +synchronization.*/ #ifdef RTE_EXEC_ENV_FREEBSD > + wait = 1; > +#endif > + Please format comments correctly. Is there a better way to detect interrupt mechanism with a function? #ifdef's make for hard to maintain code.
[dpdk-dev] [PATCH] crypto/ccp: fix fd leak on probe failure
From: Yunjian Wang
Zero is a valid fd. When ccp_probe_device() is failed, the uio_fd won't be
closed thus leading fd leak.
Fixes: ef4b04f87fa6 ("crypto/ccp: support device init")
Cc: sta...@dpdk.org
Signed-off-by: Yunjian Wang
---
drivers/crypto/ccp/ccp_dev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/crypto/ccp/ccp_dev.c b/drivers/crypto/ccp/ccp_dev.c
index 80fe6a453..7d98b2eb2 100644
--- a/drivers/crypto/ccp/ccp_dev.c
+++ b/drivers/crypto/ccp/ccp_dev.c
@@ -760,7 +760,7 @@ ccp_probe_device(const char *dirname, uint16_t domain,
return 0;
fail:
CCP_LOG_ERR("CCP Device probe failed");
- if (uio_fd > 0)
+ if (uio_fd >= 0)
close(uio_fd);
if (ccp_dev)
rte_free(ccp_dev);
--
2.19.1
[dpdk-dev] [Bug 462] command kvargs_autotest excute failed on freebsd with gcc and clang
https://bugs.dpdk.org/show_bug.cgi?id=462 Bug ID: 462 Summary: command kvargs_autotest excute failed on freebsd with gcc and clang Product: DPDK Version: 20.05 Hardware: x86 OS: FreeBSD Status: UNCONFIRMED Severity: normal Priority: Normal Component: testpmd Assignee: dev@dpdk.org Reporter: zhiminx.hu...@intel.com Target Milestone: --- -Description--- when i start up testpmd and excuted 'kvargs_autotest' with gcc and clang on freebsd,the dpdk output 'Segmentation fault' and testpmd drop out,the expected output is 'test ok'. -reproduce steps--- 1.compile dpdk by clang export RTE_SDK=`pwd` export RTE_TARGET=x86_64-native-bsdapp-clang gmake -j install T=x86_64-native-bsdapp-clang 2.load driver and bind nic driver in freebsd kldload ./x86_64-native-bsdapp-clang/kmod/contigmem.ko kldload ./x86_64-native-bsdapp-clang/kmod/nic_uio.ko Pciconf -l kenv hw.nic_uio.bdfs="134:0:0,134:0:1" 3.start testpmd ./x86_64-native-bsdapp-clang/app/test -l 1,2,4,6 -n 4 4.execute command kvargs_autotest 5.output RTE>>kvargs_autotest == test valid case == == test invalid case == Segmentation fault 6.Expected Result RTE>>kvargs_autotest == test valid case == == test invalid case == Test OK comments：gcc have the same issue as clang. -- You are receiving this mail because: You are the assignee for the bug.
