Compiler could generate non-atomic stores for whole table entry
updating. This may cause incorrect nexthop to be returned, if
the byte with valid flag is updated prior to the byte with nexthop
is updated.
Besides, field by field updating of table entries follow
read-modify-write sequences. The operations are not atomic,
nor efficient. And could cause entries out of synchronization.
Changed to use atomic store to update whole table entry.
Suggested-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
Suggested-by: Gavin Hu <gavin.hu@arm.com>
Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Gavin Hu <gavin.hu@arm.com>
Acked-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
When a tbl8 group is getting attached to a tbl24 entry, lookup
might fail even though the entry is configured in the table.
For ex: consider a LPM table configured with 10.10.10.1/24.
When a new entry 10.10.10.32/28 is being added, a new tbl8
group is allocated and tbl24 entry is changed to point to
the tbl8 group. If the tbl24 entry is written without the tbl8
group entries updated, a lookup on 10.10.10.9 will return
failure.
Correct memory orderings are required to ensure that the
store to tbl24 does not happen before the stores to tbl8 group
entries complete.
Besides, explicit structure alignment is used to address atomic
operation building issue with older version clang.
Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Reviewed-by: Gavin Hu <gavin.hu@arm.com>
Acked-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
When a tbl8 group is getting attached to a tbl24 entry, lookup
might fail even though the entry is configured in the table.
For ex: consider a LPM table configured with 10.10.10.1/24.
When a new entry 10.10.10.32/28 is being added, a new tbl8
group is allocated and tbl24 entry is changed to point to
the tbl8 group. If the tbl24 entry is written without the tbl8
group entries updated, a lookup on 10.10.10.9 will return
failure.
Correct memory orderings are required to ensure that the
store to tbl24 does not happen before the stores to tbl8 group
entries complete.
The ordering patches in general have no notable impact on LPM
performance test on both Arm A72 platform and x86 E5 platform.
Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Gavin Hu <gavin.hu@arm.com>
Acked-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
Tests showed that the function inlining caused performance drop
on some x86 platforms with the memory ordering patches applied.
By force no-inline functions, the performance was better than
before on x86 and no impact to arm64 platforms.
Besides inlines of other functions are removed to let compiler
to decide whether to inline.
Suggested-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Gavin Hu <gavin.hu@arm.com>
Acked-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
Now that everything that has ever accessed the shared memory
config is doing so through the public API's, we can make it
internal. Since we're removing quite a few headers from
rte_eal_memconfig.h, we need to add them back in places
where this header is used.
This bumps the ABI, so also change all build files and make
update documentation.
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: David Marchand <david.marchand@redhat.com>
Currently, locking/unlocking the TAILQ list requires direct
access to the shared memory config. Add an API to do the same,
and search-and-replace all usages.
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: David Marchand <david.marchand@redhat.com>
For files that already have rte_string_fns.h included in them, we can
do a straight replacement of snprintf(..."%s",...) with strlcpy. The
changes in this patch were auto-generated via command:
spatch --sp-file devtools/cocci/strlcpy-with-header.cocci --dir . --in-place
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Rework the delete function and add additional
internal data structures to support incremental
LPM tree update rather than full tree rebuild.
Signed-off-by: Alex Kiselev <alex@therouter.net>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Rework the lpm6 rule subsystem and replace
current rules algorithm complexity O(n)
with hashtables which allow dealing with
large (50k) rule sets.
Signed-off-by: Alex Kiselev <alex@therouter.net>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Fix rte_lpm_create_*() functions to return NULL and set rte_errno to
EEXIST when lpm object name already exists.
This is the behavior described in the API documentation in the header
file.
Fixes: 134975073a ("lib: remove unnecessary pointer cast")
Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
Acked-by: Phil Yang <phil.yang@arm.com>
Reviewed-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
By making "compat" lib (which consists of a header only) a dependency of
the EAL, we make the header file available to all other libs, drivers and
apps, and thereby make it less work to do ABI versioning.
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Luca Boccassi <bluca@debian.org>
The main rte_lpm.h header file also includes architecture specific headers,
depending on the architecture on which it is used. These also need to be
installed into the include directory as part of the "ninja install"
process. Thankfully, since the vector headers all have different names we
can just install all 3 of them in all cases, which avoids conflicts or
issues with multi-architecture installs, or the need to use
architecture-specific subdirectories.
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Reviewed-by: Luca Boccassi <bluca@debian.org>
Add non-EAL libraries to DPDK build. The compat lib is a special case,
along with the previously-added EAL, but all other libs can be build using
the same set of commands, where the individual meson.build files only need
to specify their dependencies, source files, header files and ABI versions.
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Reviewed-by: Harry van Haaren <harry.van.haaren@intel.com>
Acked-by: Keith Wiles <keith.wiles@intel.com>
Acked-by: Luca Boccassi <luca.boccassi@gmail.com>
Many exported headers rely on definitions found in rte_config.h without
including it, as shown by the following command:
grep -L '^#include <rte_config.h>' -- \
$(grep -Rl \
$(sed -n '/^#define \([^ ]\+\).*$/{s//\1/;H;};${x;s/\n//;s/\n/\\|/g;p;}' \
build/include/rte_config.h) \
-- build/include/)
We cannot assume external applications will include rte_config.h on their
own, neither directly nor through a -include parameter like DPDK does
internally.
This not only causes obvious compilation failures that can be reproduced
with check-includes.sh such as:
[...]/rte_memory.h:88:43: error: ‘RTE_CACHE_LINE_SIZE’ was not declared in
this scope
#define __rte_cache_aligned __rte_aligned(RTE_CACHE_LINE_SIZE)
^
It also results in less visible issues, for instance rte_hash_crc.h relying
on RTE_ARCH_X86_64's presence to provide dedicated inline functions.
This patch partially reverts the commit below and adds missing include
lines to the remaining files.
Fixes: f1a7a5c5f4 ("remove include of generated config header")
Cc: stable@dpdk.org
Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Acked-by: Thomas Monjalon <thomas@monjalon.net>
void * pointer can be assigned to any data type pointer.
Unnecessary cast can be removed in order to keep code clearer.
Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
Compiling on ARM BE using Linaro toolchain caused following
error/warnings.
rte_lpm.c: In function ‘add_depth_big_v20’:
rte_lpm.c:911:4: error: braces around scalar initializer [-Werror]
{ .group_idx = (uint8_t)tbl8_group_index, },
^
rte_lpm.c:911:4: note: (near initialization for
‘new_tbl24_entry.depth’)
rte_lpm.c:911:6:error: field name not in record or union initializer
{ .group_idx = (uint8_t)tbl8_group_index, },
^
rte_lpm.c:911:6: note: (near initialization for
‘new_tbl24_entry.depth’)
rte_lpm.c:914:13: error: initialized field overwritten
[-Werror=override-init]
.depth = 0,
Fixes: dc81ebbaca ("lpm: extend IPv4 next hop field")
Cc: stable@dpdk.org
Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Replace the BSD license header with the SPDX tag for files
with only an Intel copyright on them.
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
The memzone header is often included without good reason.
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
The compilation with gcc-6.3.0 and EXTRA_CFLAGS=-Og gives the following
error:
CC rte_lpm6.o
rte_lpm6.c: In function ‘rte_lpm6_add_v1705’:
rte_lpm6.c:442:11: error: ‘tbl_next’ may be used uninitialized in
this function [-Werror=maybe-uninitialized]
if (!tbl[tbl_index].valid) {
^
rte_lpm6.c:521:29: note: ‘tbl_next’ was declared here
struct rte_lpm6_tbl_entry *tbl_next;
^~~~~~~~
This is a false positive from gcc. Fix it by initializing tbl_next
to NULL.
Fixes: 5c510e13a9 ("lpm: add IPv6 support")
Cc: stable@dpdk.org
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
The list of libraries in LDLIBS was generated from the DEPDIRS-xyz
variable. This is valid when the subdirectory name match the library
name, but it's not always the case, especially for PMDs.
The patches removes this feature and explicitly adds the proper
libraries in LDLIBS.
Some DEPDIRS-xyz variables become useless, remove them.
Reported-by: Gage Eads <gage.eads@intel.com>
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
Reviewed-by: Gage Eads <gage.eads@intel.com>
Replace the incorrect reference to "Cavium Networks", "Cavium Ltd"
company name with correct the "Cavium, Inc" company name in
copyright headers.
Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
From v20 to v1604, number of tbl8 can be up to 1<<24,
(uint8_t) or (uint16_t) may truncate the number of
index of tlb8 in v1604 and cause wrong number.
Fixes: dc81ebbaca ("lpm: extend IPv4 next hop field")
Cc: stable@dpdk.org
Signed-off-by: Wei Dai <wei.dai@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
When rte_lpm.h is used on x86, -O0 option (no optimization at all)
given to gcc causes a compile error like this:
error: the last argument must be an 8-bit immediate
i24 = _mm_srli_si128(i24, sizeof(uint64_t));
-O0 option is useful for debugging and code coverage measurement, but
this error prevents DPDK programs from building. This patch replaces
"sizeof(uint64_t)" with a constant literal "8" to work around the issue.
The issue occurs on gcc/g++ versions from 4.8 to 5.
Signed-off-by: Sangjin Han <sangjin@eecs.berkeley.edu>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Before this patch, the management of dependencies between directories
had several issues:
- the generation of .depdirs, done at configuration is slow: it can take
more than one minute on some slow targets (usually ~10s on a standard
PC without -j).
- for instance, it is possible to express a dependency like:
- app/foo depends on lib/librte_foo
- and lib/librte_foo depends on app/bar
But this won't work because the directories are traversed with a
depth-first algorithm, so we have to choose between doing 'app' before
or after 'lib'.
- the script depdirs-rule.sh is too complex.
- we cannot use "make -d" for debug, because the output of make is used for
the generation of .depdirs.
This patch moves the DEPDIRS-* variables in the upper Makefile, making
the dependencies much easier to calculate. A DEPDIRS variable is still
used to process library dependencies in LDLIBS.
After this commit, "make config" is almost immediate.
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
Tested-by: Robin Jarry <robin.jarry@6wind.com>
Tested-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
This patch extend next_hop field from 8-bits to 21-bits in LPM library
for IPv6.
Added versioning symbols to functions and updated
library and applications that have a dependency on LPM library.
Signed-off-by: Vladyslav Buslov <vladyslav.buslov@harmonicinc.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
The memory pointed by lpm->rules_tbl should also be freed
when memory malloc for tbl8 fails in rte_lpm_create_v1604( ).
And the memory pointed by lpm->tbl8 should also be freed
when the lpm object is freed in rte_lpm_free_v1604( ).
Fixes: f1f7261838 ("lpm: add a new config structure for IPv4")
Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
Signed-off-by: Wei Dai <wei.dai@intel.com>
When a rule with depth > 24 is added into an existing
rule with depth <=24, a new tbl8 is allocated, the existing
rule first fulfill whole new tbl8, so the filed valid of
each entry in this tbl8 is always true and depth of each
entry is always <= 24 before adding the new rule with depth > 24.
Signed-off-by: Wei Dai <wei.dai@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
When all rules with depth > 24 are deleted in a same sub-table
(tlb8 group) and only a rule with depth <=24 is left in it,
this sub-table (tlb8 group) should be recycled.
Fixes: dc81ebbaca ("lpm: extend IPv4 next hop field")
Fixes: af75078fec ("first public release")
Signed-off-by: Wei Dai <wei.dai@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Exported header files for use by applications should be self sufficient and
allow out of order inclusion. Moreover, they must include all the system
headers they need for types and macros.
This commit prevents the following errors:
error: `RTE_MAX_LCORE' undeclared here (not in a function)
error: `RTE_LPM_VALID_EXT_ENTRY_BITMASK' undeclared
(first use in this function)
error: #error "Unsupported cache line size"
error: `asm' undeclared (first use in this function)
error: implicit declaration of function `[...]'
error: unknown type name `[...]'
error: field `mac_addr' has incomplete type
error: `CHAR_BIT' undeclared here (not in a function)
error: `struct [...]' declared inside parameter list
error: unknown type name `uint8_t'
Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Exported header files used by applications should allow the strictest
compiler flags. Language extensions used in many places must be explicitly
marked to avoid warnings and compilation failures.
Unnamed structs/unions are allowed since C11, however many compiler
versions do not use this mode by default.
This commit prevents the following errors:
error: ISO C99 doesn't support unnamed structs/unions
error: struct has no named members
Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Exported header files used by applications should allow the strictest
compiler flags. Language extensions used in many places must be explicitly
marked or removed to avoid warnings and compilation failures.
This commit prevents the following errors:
error: type of bit-field `[...]' is a GCC extension
Note: the standard does not require implementations to issue a diagnostic
message with these, and such errors do not occur with recent GCC or clang
versions. However, GCC 4.7 is still common and using the extension keyword
is easier than checking compiler version.
Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Exported header files used by applications should allow the strictest
compiler flags. Language extensions used in many places must be explicitly
marked or removed to avoid warnings and compilation failures.
The extension keyword is used whenever the C99 syntax cannot do it.
This commit prevents the following errors:
error: ISO C forbids zero-size array `[...]'
Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
This patch adds ppc64le port for LPM library in DPDK.
Signed-off-by: Gowrishankar Muthukrishnan <gowrishankar.m@linux.vnet.ibm.com>
Acked-by: Chao Zhu <chaozhu@linux.vnet.ibm.com>
Fix issue reported by clang scan-build
Value of pointer tbl_next was uninitialized. When function lookup_step()
take else branch it may provide garbage into tbl = tbl_next;
Fixes: 5c510e13a9 ("lpm: add IPv6 support")
Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com>
Back then when we fixed the missing free lpm I was to quickly to say yes
if it applies not only to the lpm6 but also to all of the lpm code.
It turned out to not apply to all of them. In rte_lpm_create_v20 there
is an unexpected fused allocation:
mem_size = sizeof(*lpm) + (sizeof(lpm->rules_tbl[0]) * max_rules);
[...]
lpm = (struct rte_lpm_v20 *)rte_zmalloc_socket(mem_name,mem_size,
RTE_CACHE_LINE_SIZE, socket_id);
That causes lpm->rules_tbl not to have an own struct malloc_elem that
can be derived via RTE_PTR_SUB(data, MALLOC_ELEM_HEADER_LEN) in
malloc_elem_from_data.
Due to that the rte_lpm_free_v20 accidentially misderives the elem and
assumes it is ELEM_FREE triggering in malloc_elem_free
if (!malloc_elem_cookies_ok(elem) || elem->state !=
return -1;
While it seems counter-intuitive the way to properly remove rules_tbl in
the old fused allocation style of rte_lpm_free_v20 is to not remove it.
The newer rte_lpm_free_v1604 is safe because in rte_lpm_create_v1604
rules_tbl is a separate allocation.
Fixes: d4c18f0a1d ("lpm: fix missing free")
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Acked-by: Olivier Matz <olivier.matz@6wind.com>
Change rte_lpm*_create() functions to return NULL and set rte_errno to
EEXIST when the object name already exists. This is the behavior
described in the API documentation in the header file.
These functions were returning a pointer to the existing object in that
case, but it is a problem as the caller did not know if the object had
to be freed or not.
Doing this change also makes the lpm API more consistent with the other
APIs (mempool, rings, ...).
Fixes: 916e4f4f4e ("memory: fix for multi process support")
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
In SUSE11-SP3 i686 platform, with gcc 4.5.1, there is a
compile issue:
rte_lpm.c: In function ‘add_depth_small_v20’:
rte_lpm.c:778:7: error: unknown field ‘next_hop’
specified in initializer
The root cause is gcc only allow anonymous union initialized
according to the field it is defined. But next_hop is defined
in different field when in different platform(Endian).
One solution is add if define in the code to avoid this issue,
but there is a simple way, initialize it separately later.
Fixes: afc5c914a0 ("lpm: fix big endian support")
Signed-off-by: Michael Qiu <michael.qiu@intel.com>
In certain autotests lpm->max_rules turned out to be non initialized.
That was caused by a failing allocation for lpm->rules_tbl in rte_lpm6_create.
It then left the function via goto exit with lpm freed, but still a pointer
value being set.
In case of an allocation failure it resets lpm to NULL now, to avoid the
upper layers operate on that already freed memory.
Along that is also makes the RTE_LOG message of the failed allocation unique.
Fixes: 5c510e13a9 ("lpm: add IPv6 support")
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Olivier Matz <olivier.matz@6wind.com>
lpm6 autotests failed with the default alloc of 512M Memory.
While >=2500M was a workaround it became clear while debugging that it
had a leak.
One could see a lot of output like:
LPM Test tests6[i]: FAIL
LPM: LPM memory allocation failed
It turned out that in rte_lpm6_free
- lpm might not be freed if it didn't find a te (early return)
- lpm->rules_tbl was not freed ever
Fixes: 899d8bc9b3 ("lpm: make tailq fully local")
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Olivier Matz <olivier.matz@6wind.com>
There were further chances for a use after free by returning an already
freed pointer in rte_lpm_create for v20 and v1604.
Along that is also makes the RTE_LOG messages of the failed allocations
unique.
Fixes: f1f7261838 ("lpm: add a new config structure for IPv4")
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Acked-by: Olivier Matz <olivier.matz@6wind.com>
In rte_lpm_free lpm might not be freed if it didn't find a te (early return)
The two lpm interfaces rte_lpm_free_v20 and rte_lpm_free_v1604 had a leak.
rte_lpm_free_v20 might have missed to free rules_tbl
rte_lpm_free_v1604 due to an early exit might have missed to free
rules_tbl and lpm itself.
Fixes: 899d8bc9b3 ("lpm: make tailq fully local")
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Olivier Matz <olivier.matz@6wind.com>
