RE: [PATCH 7/7] dpaax: replace zero length array with flex array
Acked-by: Hemant Agrawal
Re: [PATCH] doc: add capability to access physical addresses
2023-01-14 18:27 (UTC-0800), Stephen Hemminger: > DAC_OVERRIDE is like having the master key. It opens all doors > and if so, running as non-root really doesn't matter that much. > > Ideally, a finer grain permission could be used. > Recommending this to users seems wrong. According to my tests, DAC_READ_SEARCH can be used instead of DAC_OVERRIDE. It seems slightly better, because it doesn't bypass write permission checks. Although I agree with Isaac that SYS_ADMIN is already very powerful, and remember that the final goal is to perform unrestricted DMA. Boris, Isaac, is DAC_READ_SEARCH sufficient on your systems?
[PATCH v2] Update mailmap file
Added myself as a contributor Signed-off-by: Yevgeny Kliteynik --- Changes from v1: Removed gmail address, added commit msg body .mailmap | 1 + 1 file changed, 1 insertion(+) diff --git a/.mailmap b/.mailmap index 75884b6fe2..c6f34d18a6 100644 --- a/.mailmap +++ b/.mailmap @@ -1495,6 +1495,7 @@ Yash Sharma Yasufumi Ogawa Yelena Krivosheev Yerden Zhumabekov +Yevgeny Kliteynik Yicai Lu Yiding Zhou Yi Li -- 2.27.0
Re: [PATCH] doc: add capability to access physical addresses
Hi, On Sun, Jan 15, 2023 at 2:46 PM Dmitry Kozlyuk wrote: > > 2023-01-14 18:27 (UTC-0800), Stephen Hemminger: > > DAC_OVERRIDE is like having the master key. It opens all doors > > and if so, running as non-root really doesn't matter that much. > > > > Ideally, a finer grain permission could be used. > > Recommending this to users seems wrong. > > According to my tests, DAC_READ_SEARCH can be used instead of DAC_OVERRIDE. > It seems slightly better, because it doesn't bypass write permission checks. > Although I agree with Isaac that SYS_ADMIN is already very powerful, > and remember that the final goal is to perform unrestricted DMA. > Boris, Isaac, is DAC_READ_SEARCH sufficient on your systems? Yes, I can confirm that DAC_READ_SEARCH works fine on my system as well. Thanks! [admin@localhost ~]$ getcap /usr/bin/dpdk-testpmd /usr/bin/dpdk-testpmd cap_dac_read_search,cap_ipc_lock,cap_sys_admin=ep [admin@localhost ~]$ dpdk-testpmd -l 2,3,4 -a 000:0b:00.0 --huge-dir /dev/hugepages/ --iova-mode pa EAL: Detected CPU lcores: 8 EAL: Detected NUMA nodes: 1 EAL: Detected shared linkage of DPDK EAL: Multi-process socket /tmp/dpdk/rte/mp_socket EAL: Selected IOVA mode 'PA' EAL: No available 1048576 kB hugepages reported TELEMETRY: No legacy callbacks, legacy socket not created testpmd: No probed ethernet devices testpmd: create a new mbuf pool : n=163456, size=2176, socket=0 testpmd: preferred mempool ops selected: ring_mp_mc Done
Re: [PATCH] Update mailmap file
On 12-Jan-23 18:26, Thomas Monjalon wrote: External email: Use caution opening links or attachments 12/01/2023 16:03, Yevgeny Kliteynik: +Yevgeny Kliteynik Your nvidia.com email should be enough. Thanks Thomas. I sent v2 of this stand-alone patch so that there won't be a need to send v2 of the whole other series just for this change. -- YK Please update this file in your first patch, it's enough.
Re: [PATCH] Update mailmap file
On 12-Jan-23 18:26, Thomas Monjalon wrote: External email: Use caution opening links or attachments 12/01/2023 16:03, Yevgeny Kliteynik: +Yevgeny Kliteynik Your nvidia.com email should be enough. Thanks Thomas. I sent v2 of this stand-alone patch so that there won't be a need to send v2 of the whole other series just for this change. -- YK Please update this file in your first patch, it's enough.
Re: [PATCH 5/7] nfp: replace zero length array with flex array
Hi Stephen,
Thanks for your patch.
On 2023-01-13 13:52:03 -0800, Stephen Hemminger wrote:
> Zero length arrays are GNU extension. Replace with
> standard flex array.
>
> Signed-off-by: Stephen Hemminger
I understand this works might depend on another work to raise the
minimum compiler versions to support this change. Provided this is
sorted out as part of the full series to move to flex arrays, for the
change itself.
Acked-by: Niklas Söderlund
> ---
> drivers/net/nfp/flower/nfp_flower_cmsg.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/nfp/flower/nfp_flower_cmsg.h
> b/drivers/net/nfp/flower/nfp_flower_cmsg.h
> index 04601cb0bd25..ee8b439d617f 100644
> --- a/drivers/net/nfp/flower/nfp_flower_cmsg.h
> +++ b/drivers/net/nfp/flower/nfp_flower_cmsg.h
> @@ -73,7 +73,7 @@ struct nfp_flower_cmsg_mac_repr {
> uint8_t info;
> uint8_t nbi_port;
> uint8_t phys_port;
> - } ports[0];
> + } ports[];
> };
>
> /*
> --
> 2.39.0
>
--
Kind Regards,
Niklas Söderlund
[PATCH] build: fix dependencies lookup
The first parameter of the Meson function "find_library()"
should be the library name without the "lib" prefix.
Otherwise Meson prints this warning:
WARNING: find_library('libexecinfo') starting in "lib"
only works by accident and is not portable
Fixes: 1cd512b2f532 ("build: detect execinfo library on Linux")
Fixes: e1defba4cf66 ("raw/ifpga/base: support device tree")
Fixes: a489f5dbf437 ("baseband/turbo_sw: support meson build")
Fixes: 72c00ae9dba7 ("regex/cn9k: use cnxk infrastructure")
Cc: sta...@dpdk.org
Signed-off-by: Thomas Monjalon
---
config/meson.build| 4 ++--
drivers/baseband/turbo_sw/meson.build | 10 +-
drivers/regex/cn9k/meson.build| 4 ++--
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/config/meson.build b/config/meson.build
index 0c4d1f06e5..bcc711479e 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -191,7 +191,7 @@ if find_libnuma
endif
has_libfdt = 0
-fdt_dep = cc.find_library('libfdt', required: false)
+fdt_dep = cc.find_library('fdt', required: false)
if fdt_dep.found() and cc.has_header('fdt.h')
dpdk_conf.set10('RTE_HAS_LIBFDT', true)
has_libfdt = 1
@@ -199,7 +199,7 @@ if fdt_dep.found() and cc.has_header('fdt.h')
dpdk_extra_ldflags += '-lfdt'
endif
-libexecinfo = cc.find_library('libexecinfo', required: false)
+libexecinfo = cc.find_library('execinfo', required: false)
if libexecinfo.found()
add_project_link_arguments('-lexecinfo', language: 'c')
dpdk_extra_ldflags += '-lexecinfo'
diff --git a/drivers/baseband/turbo_sw/meson.build
b/drivers/baseband/turbo_sw/meson.build
index 417ec63394..aeb9a76f9e 100644
--- a/drivers/baseband/turbo_sw/meson.build
+++ b/drivers/baseband/turbo_sw/meson.build
@@ -6,11 +6,11 @@ dep_turbo = dependency('flexran_sdk_turbo', required: false)
dep_dec5g = dependency('flexran_sdk_ldpc_decoder_5gnr', required: false)
if dep_turbo.found()
-ext_deps += cc.find_library('libstdc++', required: true)
-ext_deps += cc.find_library('libirc', required: true)
-ext_deps += cc.find_library('libimf', required: true)
-ext_deps += cc.find_library('libipps', required: true)
-ext_deps += cc.find_library('libsvml', required: true)
+ext_deps += cc.find_library('stdc++', required: true)
+ext_deps += cc.find_library('irc', required: true)
+ext_deps += cc.find_library('imf', required: true)
+ext_deps += cc.find_library('ipps', required: true)
+ext_deps += cc.find_library('svml', required: true)
ext_deps += dep_turbo
ext_deps += dependency('flexran_sdk_crc', required: true)
ext_deps += dependency('flexran_sdk_rate_matching', required: true)
diff --git a/drivers/regex/cn9k/meson.build b/drivers/regex/cn9k/meson.build
index 06c906710c..19b2e70111 100644
--- a/drivers/regex/cn9k/meson.build
+++ b/drivers/regex/cn9k/meson.build
@@ -8,10 +8,10 @@ if not is_linux or not dpdk_conf.get('RTE_ARCH_64')
subdir_done()
endif
-lib = cc.find_library('librxp_compiler', required: false)
+lib = cc.find_library('rxp_compiler', required: false)
if lib.found()
ext_deps += lib
-ext_deps += cc.find_library('libstdc++', required: true)
+ext_deps += cc.find_library('stdc++', required: true)
cflags += ['-DREE_COMPILER_SDK']
endif
--
2.39.0
[PATCH 0/2] *** Memory Allocation: Fixes ms_idx jump in fb_array library ***
***
The problem :
In the legacy mem mode, when the fb_array is being populated, if there are
holes in between, the ms_idx could go backward and there will be an overlap of
the region starting from the ms_idx returned later. i.e. it's being mapped to
two different physical regions in PA space to a contiguous region in VA space.
this would result in the allocator assuming that the memory is contiguous even
though there is a hole in between. In legacy mem, allocator assumes that PA
contiguous are VA contiguous as well.
Reason for the ms_idx going back :
In the lookahead logic of find_next_n(), if we fail to find contiguous "need"
bits, we ignore all masks till the lookahead when we run out of options i.e.
the LSB bit is not set or more accurately there are no "need" bits starting
from LSB, therefore breaking contiguity.
The optimisation is to avoid repeating this operation and therefore we start
from msk_idx = lookahead_idx in the next iter.. However, in the outer loop, we
increment msk_idx further, resulting in msk_idx = lookahead_idx + 1, thereby
inducing a jump. This problem becomes apparent when we have partially filled
bits in the lookahead_msk. Further iterations might find the necessary
contiguity from such a lookahead, potentially resulting in the returned ms_idx
value to be lower for subsequent iterations.
The assumption was that ms_idx from rte_fbarray_find_next_n_free() is supposed
to be incremental in nature, provided that the value of n is always constant.
For a sample input of {1, 1070, 1, 2, 1, 2, 4, 12, 2, 2, 1, 2, 1},
Without the patch :
6358 EAL: ms_idx:1097
6359 EAL: ms_idx:1098
6360 EAL: ms_idx:1099
6361 EAL: ms_idx:1155
6362 EAL: ms_idx:1156
6363 EAL: ms_idx:1158
6364 EAL: ms_idx:1159
6365 EAL: ms_idx:1154
6366 EAL: ms_idx:1161
6367 EAL: ms_idx:1162
6368 EAL: ms_idx:1164
With the patch :
12730 EAL: ms_idx:1096
12731 EAL: ms_idx:1097
12732 EAL: ms_idx:1098
12733 EAL: ms_idx:1099
12734 EAL: ms_idx:1101
12735 EAL: ms_idx:1102
12736 EAL: ms_idx:1104
12737 EAL: ms_idx:1105
12738 EAL: ms_idx:1107
12739 EAL: ms_idx:1109
12740 EAL: ms_idx:1110
12741 EAL: ms_idx:1112
***
Vipin P R (2):
Memory Allocation: Fixes ms_idx jump (lookahead) during find_next_n()
in fb_array library
Memory Allocation: Fixes ms_idx jump (lookbehind) during find_prev_n()
in fb_array library
.mailmap| 1 +
lib/eal/common/eal_common_fbarray.c | 4 ++--
2 files changed, 3 insertions(+), 2 deletions(-)
--
2.7.4
[PATCH 1/2] Memory Allocation: Fixes ms_idx jump (lookahead) during find_next_n() in fb_array library
In the legacy mem mode, when the fb_array is being populated, if there are holes in between, the ms_idx could go backward and there will be an overlap of the region starting from the ms_idx returned later. i.e. it's being mapped to two different physical regions in PA space to a contiguous region in VA space. this would result in the allocator assuming that the memory is contiguous even though there is a hole in between. In legacy mem, allocator assumes that PA contiguous are VA contiguous as well. Cc: sta...@dpdk.org Signed-off-by: Vipin P R Acked-by: Kumara Parameshwaran --- .mailmap| 1 + lib/eal/common/eal_common_fbarray.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.mailmap b/.mailmap index 75884b6..3707bf5 100644 --- a/.mailmap +++ b/.mailmap @@ -1391,6 +1391,7 @@ Vincent Guo Vincent Jardin Vincent Li Vincent S. Cojot +Vipin P R Vipin Varghese Vipul Ashri Vishal Kulkarni diff --git a/lib/eal/common/eal_common_fbarray.c b/lib/eal/common/eal_common_fbarray.c index f11f879..551bd87 100644 --- a/lib/eal/common/eal_common_fbarray.c +++ b/lib/eal/common/eal_common_fbarray.c @@ -236,7 +236,7 @@ find_next_n(const struct rte_fbarray *arr, unsigned int start, unsigned int n, * as well, so skip that on next iteration. */ ignore_msk = ~((1ULL << need) - 1); - msk_idx = lookahead_idx; + msk_idx = lookahead_idx - 1; break; } -- 2.7.4
[PATCH 2/2] Memory Allocation: Fixes ms_idx jump (lookbehind) during find_prev_n() in fb_array library
Cc: sta...@dpdk.org Signed-off-by: Vipin P R Acked-by: Kumara Parameshwaran --- lib/eal/common/eal_common_fbarray.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/eal/common/eal_common_fbarray.c b/lib/eal/common/eal_common_fbarray.c index 551bd87..90240e8 100644 --- a/lib/eal/common/eal_common_fbarray.c +++ b/lib/eal/common/eal_common_fbarray.c @@ -511,7 +511,7 @@ find_prev_n(const struct rte_fbarray *arr, unsigned int start, unsigned int n, * as well, so skip that on next iteration. */ ignore_msk = UINT64_MAX << need; - msk_idx = lookbehind_idx; + msk_idx = lookbehind_idx + 1; break; } -- 2.7.4
[PATCH] *** Memory Allocation: Adding a new UT for fb_array ***
*** Partial output of fbarray_autotest (app/test/dpdk-test --log-level="*:debug") [WITHOUT THE FIX] .. .. EAL: ms_idx:1096 EAL: ms_idx:1097 EAL: ms_idx:1098 EAL: ms_idx:1099 EAL: ms_idx:1155 EAL: ms_idx:1156 EAL: ms_idx:1158 EAL: ms_idx:1159 EAL: ms_idx:1154 EAL: ms_idx jumping behind. ms_idx: 1154 prev_ms_idx: 1159 EAL: Test assert test_jump line 445 failed: Incorrect ms_idx jump + TestCase [ 2] : test_jump failed + TestCase [ 3] : test_find succeeded + TestCase [ 4] : test_find succeeded + TestCase [ 5] : test_find succeeded + TestCase [ 6] : test_find succeeded + TestCase [ 7] : test_find succeeded + TestCase [ 8] : test_empty succeeded + --- + + Test Suite Summary + Tests Total :9 + Tests Skipped : 0 + Tests Executed : 9 + Tests Unsupported: 0 + Tests Passed : 8 + Tests Failed : 1 + --- + Test Failed RTE>> Partial output of fbarray_autotest (app/test/dpdk-test --log-level="*:debug") [WITH THE FIX] .. .. EAL: ms_idx:1109 EAL: ms_idx:1110 EAL: ms_idx:1112 + TestCase [ 2] : test_jump succeeded + TestCase [ 3] : test_find succeeded + TestCase [ 4] : test_find succeeded + TestCase [ 5] : test_find succeeded + TestCase [ 6] : test_find succeeded + TestCase [ 7] : test_find succeeded + TestCase [ 8] : test_empty succeeded + --- + + Test Suite Summary + Tests Total :9 + Tests Skipped : 0 + Tests Executed : 9 + Tests Unsupported: 0 + Tests Passed : 9 + Tests Failed : 0 + --- + Test OK RTE>> *** Vipin P R (1): Memory Allocation: Adding a new UT for fb_array app/test/test_fbarray.c | 49 + 1 file changed, 49 insertions(+) -- 2.7.4
[PATCH] Memory Allocation: Adding a new UT for fb_array
add test case coverage to cover the ms_idx jump
Cc: sta...@dpdk.org
Signed-off-by: Vipin P R
Acked-by: Kumara Parameshwaran
---
Depends-on: 0001-Memory-Allocation-Fixes-ms_idx-jump-lookahead-during.patch
Depends-on: 0002-Memory-Allocation-Fixes-ms_idx-jump-lookbehind-durin.patch
---
app/test/test_fbarray.c | 49 +
1 file changed, 49 insertions(+)
diff --git a/app/test/test_fbarray.c b/app/test/test_fbarray.c
index a691bf4..275449c 100644
--- a/app/test/test_fbarray.c
+++ b/app/test/test_fbarray.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include "test.h"
@@ -402,6 +403,53 @@ static int check_used_one(void)
return 0;
}
+/* the following test case verifies that the jump in ms_idx for an fb-array is
correct. */
+static int test_jump(void)
+{
+struct rte_fbarray test_array;
+int input[] = {1, 1070, 1, 2, 1, 2, 4, 12, 2, 2, 1, 2, 1};
+int ms_idx, prev_ms_idx, delta;
+int len;
+ms_idx = prev_ms_idx = 0;
+
+int ret = rte_fbarray_init(&test_array, "test", 32768, sizeof(struct
rte_memseg));
+if (ret == 0) {
+RTE_LOG(DEBUG, EAL, "FB array init success\n");
+int k = 0;
+for(int i=0; i < sizeof(input)/sizeof(int); i++) {
+if (i == 0) {
+len = input[i];
+} else {
+len = input[i] + 1;
+}
+prev_ms_idx = ms_idx;
+ms_idx = rte_fbarray_find_next_n_free(&test_array, k, len);
+
+if (i != 0) {
+ms_idx++;
+}
+
+for (int j=0; j < input[i]; j++) {
+RTE_LOG(DEBUG, EAL, "ms_idx:%d\n", ms_idx);
+rte_fbarray_set_used(&test_array, ms_idx);
+ms_idx++;
+}
+
+if (prev_ms_idx) {
+/* The value of ms_idx should be monotonically increasing
+ * given the above input sequence in test_array.
+ * */
+delta = ms_idx - prev_ms_idx;
+if (!(delta > 0)) {
+RTE_LOG(ERR, EAL, "ms_idx jumping behind. ms_idx: %d
prev_ms_idx: %d\n", ms_idx - 1, prev_ms_idx - 1);
+TEST_ASSERT(0, "Incorrect ms_idx jump");
+}
+}
+}
+}
+return 0;
+}
+
static int test_basic(void)
{
const int idx = 0;
@@ -717,6 +765,7 @@ static struct unit_test_suite fbarray_test_suite = {
.unit_test_cases = {
TEST_CASE(test_invalid),
TEST_CASE(test_basic),
+ TEST_CASE(test_jump),
TEST_CASE_ST(first_msk_test_setup, reset_array, test_find),
TEST_CASE_ST(cross_msk_test_setup, reset_array, test_find),
TEST_CASE_ST(multi_msk_test_setup, reset_array, test_find),
--
2.7.4
[PATCH 0/2] *** Memory Allocation: Fixes ignore_msk during find_next_n() in fb_array library***
***
In the lookahead logic, let's say after the Right-Shift-And operation to check
for contiguity, we hit case
http://code.dpdk.org/dpdk/latest/source/lib/eal/common/eal_common_fbarray.c#L235
/* if first bit is not set, we've lost the run */
if ((lookahead_msk & 1) == 0) {
/*
* we've scanned this far, so we know there are
* no runs in the space we've lookahead-scanned
* as well, so skip that on next iteration.
*/
ignore_msk = ~((1ULL << need) - 1);
msk_idx = lookahead_idx;
break;
}
lets say for mask size of 64 bits : in msk_idx 4 we need 4 consecutive
bits.
let need = 4.
lets say some of the bits starting from LSB are xx11011.
Operating on the inverted mask for better clarity. RSA - RightShiftAnd,
xx -- don't-care bits before
This condition could hit if there aren't "need" number of contiguous
bits starting from LSB.
But, that doesn't necessarily mean there aren't "need" number of such
bits elsewhere in the same lookahead_idx.
We seem to be ignoring "need" number of bits starting from the LSB for
the next iteration.
Due to ignore_mask we might end losing some bits.
/* if we have an ignore mask, ignore once */
if (ignore_msk) {
cur_msk &= ignore_msk;
ignore_msk = 0;
}
e.g.
lookahead_msk before RSA logic : xx11100 , need = 4, 2 bits lost
lookahead_msk before RSA logic : xx11011, need = 4, 1 bit lost
lookahead_msk before RSA logic : xx0, need = 4, 3 bits lost
NB : To understand the number of bits lost, look at need; that's the number of
bits (starting from LSB) that's cleared to zero before the next iteration.
***
Vipin P R (2):
Memory Allocation: Fixes ignore_msk during find_next_n() in fb_array
library
Memory Allocation: Alternative fix for ignore_msk during find_next_n()
in fb_array library
lib/eal/common/eal_common_fbarray.c | 26 +++---
1 file changed, 23 insertions(+), 3 deletions(-)
--
2.7.4
[PATCH 1/2] Memory Allocation: Fixes ignore_msk during find_next_n() in fb_array library
Ignore mask ignores essential bits WHICH could have been contiguous.
This commit aims to rectify that
Cc: sta...@dpdk.org
Signed-off-by: Vipin P R
Acked-by: Kumara Parameshwaran
---
Depends-on: 0001-Memory-Allocation-Fixes-ms_idx-jump-lookahead-during.patch
Depends-on: 0002-Memory-Allocation-Fixes-ms_idx-jump-lookbehind-durin.patch
---
lib/eal/common/eal_common_fbarray.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/lib/eal/common/eal_common_fbarray.c
b/lib/eal/common/eal_common_fbarray.c
index 90240e8..313681a 100644
--- a/lib/eal/common/eal_common_fbarray.c
+++ b/lib/eal/common/eal_common_fbarray.c
@@ -235,7 +235,12 @@ find_next_n(const struct rte_fbarray *arr, unsigned int
start, unsigned int n,
* no runs in the space we've lookahead-scanned
* as well, so skip that on next iteration.
*/
- ignore_msk = ~((1ULL << need) - 1);
+ if (!lookahead_msk) {
+ /* There aren't "need" number of
contiguous bits anywhere in the mask.
+* Ignore these many number of bits
from LSB for the next iteration.
+*/
+ ignore_msk = ~((1ULL << need) - 1);
+ }
msk_idx = lookahead_idx - 1;
break;
}
--
2.7.4
[PATCH 2/2] Memory Allocation: Alternative fix for ignore_msk during find_next_n() in fb_array library
Ignore mask ignores essential bits WHICH could have been contiguous.
This commit aims to rectify that
Cc: sta...@dpdk.org
Signed-off-by: Vipin P R
Acked-by: Kumara Parameshwaran
---
Depends-on: 0001-Memory-Allocation-Fixes-ms_idx-jump-lookahead-during.patch
Depends-on: 0002-Memory-Allocation-Fixes-ms_idx-jump-lookbehind-durin.patch
---
lib/eal/common/eal_common_fbarray.c | 21 ++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/lib/eal/common/eal_common_fbarray.c
b/lib/eal/common/eal_common_fbarray.c
index 313681a..29fffb6 100644
--- a/lib/eal/common/eal_common_fbarray.c
+++ b/lib/eal/common/eal_common_fbarray.c
@@ -138,7 +138,7 @@ find_next_n(const struct rte_fbarray *arr, unsigned int
start, unsigned int n,
last_msk = ~(UINT64_MAX << last_mod);
for (msk_idx = first; msk_idx < msk->n_masks; msk_idx++) {
- uint64_t cur_msk, lookahead_msk;
+ uint64_t cur_msk, lookahead_msk, lookahead_msk_;
unsigned int run_start, clz, left;
bool found = false;
/*
@@ -215,12 +215,14 @@ find_next_n(const struct rte_fbarray *arr, unsigned int
start, unsigned int n,
for (lookahead_idx = msk_idx + 1; lookahead_idx < msk->n_masks;
lookahead_idx++) {
- unsigned int s_idx, need;
+ unsigned int s_idx, need, fsb_idx, fcb_idx, ignore_bits;
lookahead_msk = msk->data[lookahead_idx];
/* if we're looking for free space, invert the mask */
if (!used)
lookahead_msk = ~lookahead_msk;
+
+ lookahead_msk_ = lookahead_msk;
/* figure out how many consecutive bits we need here */
need = RTE_MIN(left, MASK_ALIGN);
@@ -236,10 +238,23 @@ find_next_n(const struct rte_fbarray *arr, unsigned int
start, unsigned int n,
* as well, so skip that on next iteration.
*/
if (!lookahead_msk) {
- /* There aren't "need" number of
contiguous bits anywhere in the mask.
+ /* There aren't "need" number of
contiguous bits anywhere in the mask.
* Ignore these many number of bits
from LSB for the next iteration.
*/
ignore_msk = ~((1ULL << need) - 1);
+ } else {
+ /* Find the first clear bit */
+ fcb_idx =
__builtin_ffsll((~lookahead_msk_));
+ /* clear all bits upto the first clear
bit in lookahead_msk_. */
+ lookahead_msk_ = lookahead_msk_ &
((~0ULL) << fcb_idx);
+ /* find the first set bit in the
modified mask */
+ fsb_idx =
__builtin_ffsll(lookahead_msk_);
+ /* number of bits to ignore from the
next iteration */
+ ignore_bits = fsb_idx - 1;
+ /* ignore all bits preceding the first
set bit after the first clear bit
+* starting from LSB of lookahead_msk_.
+*/
+ ignore_msk = ~((1ULL << ignore_bits) -
1);
}
msk_idx = lookahead_idx - 1;
break;
--
2.7.4
[PATCH] *** Intel iavf: Return in the case of ADD/DEL ETH address ***
*** Intel iavf: Return in the case of ADD/DEL ETH address In case of i40vf, VIRTCHNL_OP_DEL_ETH_ADDR and VIRTCHNL_OP_ADD_ETH_ADDR are unsupported. i40evf_execute_vf_cmd is invoked with these operations as part of i40evf_set_mc_addr_list() The cases are not handled in i40evf_execute_vf_cmd() thus hitting the default case. There is a retry logic of upto 200 times (2000 in iavf) with a delay of 10ms (1ms in iavf). This results in a needless delay of 2s in the init phase for each VNIC. The patch aims to rectify that delay. In fe2a571c70cc397f2ad4e280f8d084148fea5d62, i40e_ethdev_vf.c was deleted. Hence adding this in iavf_vchnl.c. *** Vipin P R (1): Intel iavf: Return in the case of ADD/DEL ETH address drivers/net/iavf/iavf_vchnl.c | 8 1 file changed, 8 insertions(+) -- 2.7.4
[PATCH] Intel iavf: Return in the case of ADD/DEL ETH address
In case of i40vf, VIRTCHNL_OP_DEL_ETH_ADDR and VIRTCHNL_OP_ADD_ETH_ADDR are
unsupported.
i40evf_execute_vf_cmd is invoked with these operations as part of
i40evf_set_mc_addr_list()
The cases are not handled in i40evf_execute_vf_cmd() thus hitting the default
case.
There is a retry logic of upto 200 times (2000 in iavf) with a delay of 10ms
(1ms in iavf).
This results in a needless delay of 2s in the init phase for each VNIC.
The patch aims to rectify that delay.
In fe2a571c70cc397f2ad4e280f8d084148fea5d62, i40e_ethdev_vf.c was deleted.
Hence adding this in iavf_vchnl.c.
Cc: sta...@dpdk.org
Signed-off-by: Vipin P R
---
drivers/net/iavf/iavf_vchnl.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c
index f92daf9..e2f65f5 100644
--- a/drivers/net/iavf/iavf_vchnl.c
+++ b/drivers/net/iavf/iavf_vchnl.c
@@ -367,6 +367,14 @@ iavf_execute_vf_cmd(struct iavf_adapter *adapter, struct
iavf_cmd_info *args,
}
_clear_cmd(vf);
break;
+
+case VIRTCHNL_OP_ADD_ETH_ADDR:
+case VIRTCHNL_OP_DEL_ETH_ADDR:
+PMD_DRV_LOG(WARNING, "OP_{ADD/DEL}_ETH_ADDR unsupported");
+err = 0;
+_clear_cmd(vf);
+break;
+
default:
/* For other virtchnl ops in running time,
* wait for the cmd done flag.
--
2.7.4
Re: [RFC v3 2/2] ethdev: add API to set process to active or standby
26/12/2022 17:44, Ori Kam: > From: Rongwei Liu > > From: Jerin Jacob > > > On Wed, Dec 21, 2022 at 6:20 PM Rongwei Liu wrote: > > > > From: Jerin Jacob > > > > > On Wed, Dec 21, 2022 at 5:35 PM Rongwei Liu wrote: > > > > > > From: Jerin Jacob > > > > > > > On Wed, Dec 21, 2022 at 3:02 PM Rongwei Liu wrote: > > > > > > > > From: Jerin Jacob > > > > > > > > > On Wed, Dec 21, 2022 at 2:31 PM Rongwei Liu wrote: > > > > > > > > > > > > > > > > > > > > Users may want to change the DPDK process to different > > > > > > > > > > versions > > > > > > > > > > > > > > > > > > Different version of DPDK? If there is any ABI change how to > > > > > > > > > support > > > > > this? > > > > > > > > > > > > > > > > > There is a new member which was introduced into > > > > > > > > rte_eth_dev_info but it > > > > > > > shouldn’t be ABI breaking since using reserved fields. > > > > > > > > > > > > > > That is just for rte_eth_dev_info. What about the ABI change in > > > > > > > different ethdev structure and rte_flow structures across > > > > > > > different DPDK > > > > > ABI versions. > > > > > > > > > > > > > Besides this, there is no other ABI changes dependency. > > > > > > > > > > > > Assume there is a DPDK process A running with version v21.11 and > > > > > > plan to upgrade to version v22.11. Let' call v22.11 as process B. > > > > > > > > > > OK. That's a relief. I understand the use case now. > > > > > > > > > > Why not simply use standard DPDK multiprocess model then. > > > > > Primary process act as server for slow path API. Secondary process > > > > > can come and go(aka can be updated at runtime) and use as client to > > > > > update rules via primary-secondray communication mechanism. > > > > > > > > > Just image if process A and process B have ABI breakage like different > > > rte_flow_item_*** and rte_flow_action_*** size and members. > > > > How can we quickly accommodate primary/secondary to be ABI > > compatible > > > across different versions? > > > > It will be very huge effort and difficult to implement, at least in my > > opinion. > > > > What do you think? > > > > > > Yes. it difficult what ever approach we take, On other hand, ethdev > > subsystem > > > has other components like rte_tm and other offload etc, We can not simply > > > have rte_eth_process_set_active() and things magical works across > > different > > > DPDK versions. Example, if rte_flow rule has meter action will depend on > > > another HW piece in NIC card for doing the metering but set by flow API. > > > IMO, Customer can simply use standard multiprocess model if version are > > > compatible without any special intelligence in application. > > > Otherwise they can reload and start the application again or have special > > > intelligence in application to cater the specific area of API they need to > > > leverage based on server and client DPDK versions. > > > > Thanks for the message. > > IMO, we are trying to eliminate the version/ABI dependency with this new > > added API. > > For example, if meter action is in the flow rules: > > 1. Process A have rules like "eth / ipv4 src 1.1.1.1 / meter / queue / end" > > 2. Process B starts with "rte_eth_process_set_active(false, flags)" > > > > Just give Nvidia' hardware as example (other NIC vendors may not care if > > group 0 or not) > > If the process A' rules are in group 0, users should set them one by one to > > process B. > > Then either flush process A' rules or quit process A, then process B calls > > with > > "rte_eth_process_set_active(true, flags)" > > All is set. > > It will avoid complex operations with client/server model and avoid user > > mis- > > operation too. > > We should avoid reload as much as possible since reloading is very time > > consuming and may take up to few seconds. > > In this time slot, there is no application to handle the traffic, and > > everything is > > lost. > > For end user especially cloud service providers, they are sensitive to the > > traffic down time. > > From my viewpoint the upgrade has nothing to do with DPDK as a library, > the upgrade may be because of application upgrade. > Unless I'm missing something, this API is not meant for API/ABI it is created > to allow minimum downtime when doing upgrade of the application. > Unless I'm missing something critical, since the upgrade in any case is not > only for the DPDK but the entire app, there isn't any ABI/API issue. Yes we can consider the case of an application upgrade with the same DPDK. The patch needs to be reworded in this more realistic direction I think. We can also improve the usage explanations. That said, another high level question is about the scope of the feature. In this patch, only ethdev is targetted. Do you think we need the same migration mechanism in other classes like vDPA, crypto, etc?
RE: [PATCH 1/8] net/nfp: break out function to report device information
Hi all,
A gentle ping on this patch.
> -Original Message-
> From: Chaoyong He
> Sent: 2022年11月28日 14:54
> To: dev@dpdk.org
> Cc: oss-drivers ; Niklas Soderlund
> ; Nole Zhang ;
> Chaoyong He
> Subject: [PATCH 1/8] net/nfp: break out function to report device
> information
>
> From: Peng Zhang
>
> The method to report device information to the log is the same for both
> physical and virtual functions. The implementation is however open coded in
> each code path, break out the reporting logic to a helper function to reduce
> code duplication.
>
> Signed-off-by: Peng Zhang
> Reviewed-by: Niklas Söderlund
> Reviewed-by: Chaoyong He
> ---
> drivers/net/nfp/nfp_common.c| 27 +++
> drivers/net/nfp/nfp_common.h| 1 +
> drivers/net/nfp/nfp_ethdev.c| 23 +--
> drivers/net/nfp/nfp_ethdev_vf.c | 23 +--
> 4 files changed, 30 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/net/nfp/nfp_common.c b/drivers/net/nfp/nfp_common.c
> index 71711bfa22..f112a70980 100644
> --- a/drivers/net/nfp/nfp_common.c
> +++ b/drivers/net/nfp/nfp_common.c
> @@ -188,6 +188,33 @@ nfp_net_configure(struct rte_eth_dev *dev)
> return 0;
> }
>
> +void
> +nfp_net_log_device_information(const struct nfp_net_hw *hw) {
> + PMD_INIT_LOG(INFO, "VER: %u.%u, Maximum supported MTU: %d",
> + NFD_CFG_MAJOR_VERSION_of(hw->ver),
> + NFD_CFG_MINOR_VERSION_of(hw->ver), hw-
> >max_mtu);
> +
> + PMD_INIT_LOG(INFO,
> "CAP: %#x, %s%s%s%s%s%s%s%s%s%s%s%s%s%s", hw->cap,
> + hw->cap & NFP_NET_CFG_CTRL_PROMISC ?
> "PROMISC " : "",
> + hw->cap & NFP_NET_CFG_CTRL_L2BC ? "L2BCFILT
> " : "",
> + hw->cap & NFP_NET_CFG_CTRL_L2MC ?
> "L2MCFILT " : "",
> + hw->cap & NFP_NET_CFG_CTRL_RXCSUM?
> "RXCSUM ": "",
> + hw->cap & NFP_NET_CFG_CTRL_TXCSUM?
> "TXCSUM ": "",
> + hw->cap & NFP_NET_CFG_CTRL_RXVLAN? "RXVLAN
> ": "",
> + hw->cap & NFP_NET_CFG_CTRL_TXVLAN? "TXVLAN
> ": "",
> + hw->cap & NFP_NET_CFG_CTRL_SCATTER ?
> "SCATTER " : "",
> + hw->cap & NFP_NET_CFG_CTRL_GATHER?
> "GATHER ": "",
> + hw->cap & NFP_NET_CFG_CTRL_LIVE_ADDR ?
> "LIVE_ADDR " : "",
> + hw->cap & NFP_NET_CFG_CTRL_LSO ? "TSO " :
> "",
> + hw->cap & NFP_NET_CFG_CTRL_LSO2 ? "TSOv2
> " : "",
> + hw->cap & NFP_NET_CFG_CTRL_RSS ? "RSS " :
> "",
> + hw->cap & NFP_NET_CFG_CTRL_RSS2 ? "RSSv2 " :
> "");
> +
> + PMD_INIT_LOG(INFO, "max_rx_queues: %u, max_tx_queues: %u",
> + hw->max_rx_queues, hw->max_tx_queues); }
> +
> void
> nfp_net_enable_queues(struct rte_eth_dev *dev) { diff --git
> a/drivers/net/nfp/nfp_common.h b/drivers/net/nfp/nfp_common.h index
> 36c19b47e4..02612dbb58 100644
> --- a/drivers/net/nfp/nfp_common.h
> +++ b/drivers/net/nfp/nfp_common.h
> @@ -404,6 +404,7 @@ nfp_pci_queue(struct rte_pci_device *pdev, uint16_t
> queue)
> /* Prototypes for common NFP functions */ int nfp_net_reconfig(struct
> nfp_net_hw *hw, uint32_t ctrl, uint32_t update); int
> nfp_net_configure(struct rte_eth_dev *dev);
> +void nfp_net_log_device_information(const struct nfp_net_hw *hw);
> void nfp_net_enable_queues(struct rte_eth_dev *dev); void
> nfp_net_disable_queues(struct rte_eth_dev *dev); void
> nfp_net_params_setup(struct nfp_net_hw *hw); diff --git
> a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c index
> 0956ea81df..f661819fc0 100644
> --- a/drivers/net/nfp/nfp_ethdev.c
> +++ b/drivers/net/nfp/nfp_ethdev.c
> @@ -619,33 +619,12 @@ nfp_net_init(struct rte_eth_dev *eth_dev)
> else
> hw->rx_offset = nn_cfg_readl(hw,
> NFP_NET_CFG_RX_OFFSET_ADDR);
>
> - PMD_INIT_LOG(INFO, "VER: %u.%u, Maximum supported MTU: %d",
> -NFD_CFG_MAJOR_VERSION_of(hw->ver),
> -NFD_CFG_MINOR_VERSION_of(hw->ver), hw-
> >max_mtu);
> -
> - PMD_INIT_LOG(INFO,
> "CAP: %#x, %s%s%s%s%s%s%s%s%s%s%s%s%s%s", hw->cap,
> - hw->cap & NFP_NET_CFG_CTRL_PROMISC ? "PROMISC " :
> "",
> - hw->cap & NFP_NET_CFG_CTRL_L2BC? "L2BCFILT " : "",
> - hw->cap & NFP_NET_CFG_CTRL_L2MC? "L2MCFILT " : "",
> - hw->cap & NFP_NET_CFG_CTRL_RXCSUM ? "RXCSUM " :
> "",
> - hw->cap & NFP_NET_CFG_CTRL_TXCSUM ? "TXCSUM " :
> "",
> - hw->cap & NFP_NET_CFG_CTRL_RXVLAN ? "RXVLAN " : "",
> - hw->cap & NFP_NET_CFG_CTRL_TXVLAN ? "TXVLAN " : "",
> - hw->cap & NFP_NET_CFG_CTRL_SCATTER ? "SCATTER " : "",
> - hw->cap & NFP_NET_CFG_CTRL_GATHER ? "GATHER " : "",
> -
Re: [PATCH v4] eventdev/timer: add API to get remaining ticks
On Sat, Jan 14, 2023 at 1:20 AM Erik Gabriel Carrillo
wrote:
>
> Introduce an event timer adapter API which allows users to determine how
> many adapter ticks remain until an event timer expires.
>
> Signed-off-by: Erik Gabriel Carrillo
Acked-by: Jerin Jacob
Updated the git commit as follows and applied to
dpdk-next-net-eventdev/for-main. Thanks
eventdev/timer: support to get remaining ticks
Introduce an event timer adapter API which allows users to determine how
many adapter ticks remain until an event timer expires.
Signed-off-by: Erik Gabriel Carrillo
Acked-by: Jerin Jacob
> ---
> v4:
> * Rename API to rte_event_timer_remaining_ticks_get
> * Return error if API out param is NULL instead asserting it is non-NULL
> * Update documentation
>
> v3:
> * Handle ENOTSUP case in unit test
>
> v2:
> * Rename API to rte_event_timer_get_remaining_ticks
> * Assert that API out param is non-NULL instead of checking and returning
> error
>
> app/test/test_event_timer_adapter.c| 75 ++
> lib/eventdev/event_timer_adapter_pmd.h | 7 +++
> lib/eventdev/rte_event_timer_adapter.c | 53 ++
> lib/eventdev/rte_event_timer_adapter.h | 27 ++
> lib/eventdev/version.map | 3 ++
> 5 files changed, 165 insertions(+)
>
> diff --git a/app/test/test_event_timer_adapter.c
> b/app/test/test_event_timer_adapter.c
> index 1a440dfd10..5e7feec1c7 100644
> --- a/app/test/test_event_timer_adapter.c
> +++ b/app/test/test_event_timer_adapter.c
> @@ -1920,6 +1920,79 @@ adapter_create_max(void)
> return TEST_SUCCESS;
> }
>
> +static inline int
> +test_timer_ticks_remaining(void)
> +{
> + uint64_t ticks_remaining = UINT64_MAX;
> + struct rte_event_timer *ev_tim;
> + struct rte_event ev;
> + int ret, i;
> + const struct rte_event_timer tim = {
> + .ev.op = RTE_EVENT_OP_NEW,
> + .ev.queue_id = 0,
> + .ev.sched_type = RTE_SCHED_TYPE_ATOMIC,
> + .ev.priority = RTE_EVENT_DEV_PRIORITY_NORMAL,
> + .ev.event_type = RTE_EVENT_TYPE_TIMER,
> + .state = RTE_EVENT_TIMER_NOT_ARMED,
> + };
> +
> + rte_mempool_get(eventdev_test_mempool, (void **)&ev_tim);
> + *ev_tim = tim;
> + ev_tim->ev.event_ptr = ev_tim;
> +#define TEST_TICKS 5
> + ev_tim->timeout_ticks = CALC_TICKS(TEST_TICKS);
> +
> + ret = rte_event_timer_remaining_ticks_get(timdev, ev_tim,
> + &ticks_remaining);
> + if (ret == -ENOTSUP) {
> + rte_mempool_put(eventdev_test_mempool, (void *)ev_tim);
> + printf("API not supported, skipping test\n");
> + return TEST_SKIPPED;
> + }
> +
> + /* Test that unarmed timer returns error */
> + TEST_ASSERT_FAIL(ret,
> +"Didn't fail to get ticks for unarmed event timer");
> +
> + TEST_ASSERT_EQUAL(rte_event_timer_arm_burst(timdev, &ev_tim, 1), 1,
> + "Failed to arm timer with proper timeout.");
> + TEST_ASSERT_EQUAL(ev_tim->state, RTE_EVENT_TIMER_ARMED,
> + "Improper timer state set expected %d returned %d",
> + RTE_EVENT_TIMER_ARMED, ev_tim->state);
> +
> + for (i = 0; i < TEST_TICKS; i++) {
> + ret = rte_event_timer_remaining_ticks_get(timdev, ev_tim,
> + &ticks_remaining);
> + if (ret < 0)
> + return TEST_FAILED;
> +
> + TEST_ASSERT_EQUAL((int)ticks_remaining, TEST_TICKS - i,
> + "Expected %d ticks remaining, got
> %"PRIu64"",
> + TEST_TICKS - i, ticks_remaining);
> +
> + rte_delay_ms(100);
> + }
> +
> + rte_delay_ms(100);
> +
> + TEST_ASSERT_EQUAL(rte_event_dequeue_burst(evdev, 0, &ev, 1, 0), 1,
> + "Armed timer failed to trigger.");
> + TEST_ASSERT_EQUAL(ev_tim->state, RTE_EVENT_TIMER_NOT_ARMED,
> + "Improper timer state set expected %d returned %d",
> + RTE_EVENT_TIMER_NOT_ARMED, ev_tim->state);
> +
> + /* Test that timer that fired returns error */
> + TEST_ASSERT_FAIL(rte_event_timer_remaining_ticks_get(timdev, ev_tim,
> + &ticks_remaining),
> +"Didn't fail to get ticks for unarmed event timer");
> +
> + rte_mempool_put(eventdev_test_mempool, (void *)ev_tim);
> +
> +#undef TEST_TICKS
> + return TEST_SUCCESS;
> +}
> +
> +
> static struct unit_test_suite event_timer_adptr_functional_testsuite = {
> .suite_name = "event timer functional test suite",
> .setup = testsuite_setup,
> @@ -1982,6 +2055,8 @@ static struct unit_test_suite
> event_timer_adptr_functional_testsuite
RE: [PATCH v2 0/5] net/idpf: code refine
> -Original Message- > From: Xing, Beilei > Sent: Friday, January 6, 2023 5:05 PM > To: Zhang, Qi Z > Cc: dev@dpdk.org; Xing, Beilei > Subject: [PATCH v2 0/5] net/idpf: code refine > > From: Beilei Xing > > 1. Remove some unnecessary fields from idpf_adapter structure. > 2. Fix xmit free for split queue model. > 3. Fix driver init symbols. > 4. Refine MTU configuration. > > V2 changes: > - fix driver init symbols > - refine MTU setting > > Jingjing Wu (5): > net/idpf: remove vport req and recv info from adapter > net/idpf: remove req vports from adapter > net/idpf: fix splitq xmit free > net/idpf: fix driver init symbols > net/idpf: refine MTU setting > > drivers/net/idpf/idpf_ethdev.c | 301 ++--- > drivers/net/idpf/idpf_ethdev.h | 29 ++-- > drivers/net/idpf/idpf_rxtx.c | 29 ++-- > drivers/net/idpf/idpf_vchnl.c | 18 +- > 4 files changed, 166 insertions(+), 211 deletions(-) > > -- > 2.26.2 Acked-by: Qi Zhang Applied to dpdk-next-net-intel. Thanks Qi
