RE: [EXTERNAL] Re: [v11 0/4] PCI Dev and SG copy support
Hi Thomas and Fengchengwen, > > 13/03/2024 09:22, Gowrishankar Muthukrishnan: > > Hi Fengchengwen > > > > > Hi Gowrishankar, > > > > > > On 2024/3/12 20:24, Gowrishankar Muthukrishnan wrote: > > > > Hi Fengchengwen > > > > > > > >> > > > >> Hi Thomas, > > > >> > > > >> On 2024/3/12 17:15, Thomas Monjalon wrote: > > > >>> 07/03/2024 14:55, Gowrishankar Muthukrishnan: > > > Hi Fengchengwen, > > > > > > >> Waiting for a confirmation that this series is good to go. > > > > > > > > In the discuss of thread [1], I hope this patchset continue > > > > take a step forward (means new version) to support > > > > bi-direction test just by > > > >> modify config.ini file. > > > > > > > > > > This patch set already exposes all configuration via > > > config.ini. I didn't follow > > > >> what is missing. For bi-direction, we can better continue > > > >> discussing on that patch. > > > >>> > > > >>> Chengwen, please can you confirm whether you require a new > version? > > > >>> Which change exactly is missing? > > > >> > > > >> This patchset is OK with one sub-test only tackle one DMA direction. > > > >> > > > > Thanks for the confirmation. > > > > > > > >> But there is a later patch [1] which will support multiple DMA > > > >> directions within one sub-test. > > > >> it will add a entry "xfer_mode", but I think it complicate the > > > >> test, I prefer we do more in this patchset to support some like > > > >> bi-direction just by modify config.ini, some like this: > > > >> > > > > I think we should discuss about that in bi-directional patch > > > > series. This series > > > is self-contained and there is no need to add bi-directional as part > > > of this series. As far as this patch set is concerned, all the > > > options are exposed via config.ini. Can you comment if there is > > > anything missing, assuming that we are taking bi-directional support as a > separate feature addition. > > > > > > I have identified some improvements to the dma-perf app, and I plan > > > to do it > > > > It is unclear at this point what is the issue that you have with the app or > > this > patch set. This series was first submitted on Aug 10 2023. You had acked v8 on > Jan 25 2024. After the patches were acked, there were still review comments > on variable renames etc, which were all addressed. The patches had been > under review for more than 8 months with very slow progress. > > > > > in 24.07, so if you don't mind, I will incorporate your commits > > > (keeping your > > > signed-off-by) and modify to the one that I described above, and > > > then send to community (also with my improvements commits). > > > > I would like to have this series merged first and not pulled into another > series. We do have few other features that we would like to add on top. I > would assume that you can also add your changes on top. To make > contribution easier, isn't it better to accept at least this patch set (as > you acked > earlier) and then you can continue working on the improvements? > > OK, one feature at a time. > Let's work on top of this patchset applied. > Thank you both for reviewing this series and accepting it in RC3. Regards, Gowrishankar
Re: [dpdk-dev] [PATCH v1 2/2] net/i40e: use movdiri to update queue tail registers
Re: Re: dumpcap coredump for 82599 NIC
The issue indeed lies with the ixgbe driver. After making the following
modifications, dpdk-dumpcap is now functioning properly.
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index
d6cf00317e77b64f9822c155115f388ae62241eb..99b26f3c758b3c7ced5d59c6b27f305efe6cc33c
100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -4301,48 +4301,50 @@ ixgbe_dev_link_update_share(struct rte_eth_dev *dev,
wait = 1;
#endif
- if (vf)
- diag = ixgbevf_check_link(hw, &link_speed, &link_up, wait);
- else
- diag = ixgbe_check_link(hw, &link_speed, &link_up, wait);
+ if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+ if (vf)
+ diag = ixgbevf_check_link(hw, &link_speed, &link_up,
wait);
+ else
+ diag = ixgbe_check_link(hw, &link_speed, &link_up,
wait);
- if (diag != 0) {
- link.link_speed = RTE_ETH_SPEED_NUM_100M;
- link.link_duplex = RTE_ETH_LINK_FULL_DUPLEX;
- return rte_eth_linkstatus_set(dev, &link);
- }
+ if (diag != 0) {
+ link.link_speed = RTE_ETH_SPEED_NUM_100M;
+ link.link_duplex = RTE_ETH_LINK_FULL_DUPLEX;
+ return rte_eth_linkstatus_set(dev, &link);
+ }
+
+ if (ixgbe_get_media_type(hw) == ixgbe_media_type_fiber &&
+ !ad->sdp3_no_tx_disable) {
+ esdp_reg = IXGBE_READ_REG(hw, IXGBE_ESDP);
+ if ((esdp_reg & IXGBE_ESDP_SDP3))
+ link_up = 0;
+ }
- if (ixgbe_get_media_type(hw) == ixgbe_media_type_fiber &&
- !ad->sdp3_no_tx_disable) {
- esdp_reg = IXGBE_READ_REG(hw, IXGBE_ESDP);
- if ((esdp_reg & IXGBE_ESDP_SDP3))
- link_up = 0;
- }
-
- if (link_up == 0) {
- if (ixgbe_get_media_type(hw) == ixgbe_media_type_fiber) {
- ixgbe_dev_wait_setup_link_complete(dev, 0);
- /* NOTE: review for potential ordering optimization */
- if (!__atomic_test_and_set(&ad->link_thread_running,
__ATOMIC_SEQ_CST)) {
- /* To avoid race condition between threads, set
-* the IXGBE_FLAG_NEED_LINK_CONFIG flag only
-* when there is no link thread running.
-*/
- intr->flags |= IXGBE_FLAG_NEED_LINK_CONFIG;
- if
(rte_thread_create_internal_control(&ad->link_thread_tid,
- "ixgbe-link",
-
ixgbe_dev_setup_link_thread_handler, dev) < 0) {
+ if (link_up == 0) {
+ if (ixgbe_get_media_type(hw) == ixgbe_media_type_fiber)
{
+ ixgbe_dev_wait_setup_link_complete(dev, 0);
+ /* NOTE: review for potential ordering
optimization */
+ if
(!__atomic_test_and_set(&ad->link_thread_running, __ATOMIC_SEQ_CST)) {
+ /* To avoid race condition between
threads, set
+* the IXGBE_FLAG_NEED_LINK_CONFIG flag
only
+* when there is no link thread running.
+*/
+ intr->flags |=
IXGBE_FLAG_NEED_LINK_CONFIG;
+ if
(rte_thread_create_internal_control(&ad->link_thread_tid,
+ "ixgbe-link",
+
ixgbe_dev_setup_link_thread_handler, dev) < 0) {
+ PMD_DRV_LOG(ERR,
+ "Create link thread
failed!");
+ /* NOTE: review for potential
ordering optimization */
+
__atomic_clear(&ad->link_thread_running, __ATOMIC_SEQ_CST);
+ }
+ } else {
PMD_DRV_LOG(ERR,
- "Create link thread failed!");
- /* NOTE: review for potential ordering
optimization */
-
__atomic_clear(&ad->link_thread_running, __ATOMIC_SEQ_CST);
+ "Other link thread is running
now!");
}
- } else {
- PMD_DRV_LOG(ERR,
-
[PATCH] app/testpmd: dump TCI when asking for VLAN insertion
I got some report for users that VLAN insertion was not working as the only thing they could see in verbose mode was a 0x0800 ethertype even though the RTE_MBUF_F_TX_VLAN flag was shown. Dump the VLAN TCI from mbuf metadata when VLAN insertion is requested. This should enhance the situation. Before: src=76:4E:EA:3F:78:1D - dst=50:7C:6F:3C:10:5B - pool=mb_pool_1 - type=0x0800 - length=64 - nb_segs=1 - sw ptype: L2_ETHER L3_IPV4 L4_UDP - l2_len=14 - l3_len=20 - l4_len=8 - Send queue=0x0 ol_flags: RTE_MBUF_F_TX_VLAN RTE_MBUF_F_TX_L4_NO_CKSUM After: src=76:4E:EA:3F:78:1D - dst=50:7C:6F:3C:10:5B - pool=mb_pool_1 - type=0x0800 - length=64 - nb_segs=1 - VLAN tci=0x2a - sw ptype: L2_ETHER L3_IPV4 L4_UDP - l2_len=14 - l3_len=20 - l4_len=8 - Send queue=0x0 ol_flags: RTE_MBUF_F_TX_VLAN RTE_MBUF_F_TX_L4_NO_CKSUM Signed-off-by: David Marchand --- app/test-pmd/util.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/test-pmd/util.c b/app/test-pmd/util.c index 5aa69ed545..bf9b639d95 100644 --- a/app/test-pmd/util.c +++ b/app/test-pmd/util.c @@ -180,11 +180,13 @@ dump_pkt_burst(uint16_t port_id, uint16_t queue, struct rte_mbuf *pkts[], if (is_timestamp_enabled(mb)) MKDUMPSTR(print_buf, buf_size, cur_len, " - timestamp %"PRIu64" ", get_timestamp(mb)); - if (ol_flags & RTE_MBUF_F_RX_QINQ) + if ((is_rx && (ol_flags & RTE_MBUF_F_RX_QINQ) != 0) || + (!is_rx && (ol_flags & RTE_MBUF_F_TX_QINQ) != 0)) MKDUMPSTR(print_buf, buf_size, cur_len, " - QinQ VLAN tci=0x%x, VLAN tci outer=0x%x", mb->vlan_tci, mb->vlan_tci_outer); - else if (ol_flags & RTE_MBUF_F_RX_VLAN) + else if ((is_rx && (ol_flags & RTE_MBUF_F_RX_VLAN) != 0) || + (!is_rx && (ol_flags & RTE_MBUF_F_TX_VLAN) != 0)) MKDUMPSTR(print_buf, buf_size, cur_len, " - VLAN tci=0x%x", mb->vlan_tci); if (!is_rx && (ol_flags & RTE_MBUF_DYNFLAG_TX_METADATA)) -- 2.44.0
[PATCH v3 2/6] argparse: remove dead code
The judgement "obj->callback == NULL" is dead code which can't be
reached, because verify_arg_saver() already make sure obj->callback
must not be NULL when arg->val_saver is NULL.
Signed-off-by: Chengwen Feng
---
app/test/test_argparse.c| 7 ---
lib/argparse/rte_argparse.c | 6 --
2 files changed, 13 deletions(-)
diff --git a/app/test/test_argparse.c b/app/test/test_argparse.c
index df11a129ba..c98bcee56d 100644
--- a/app/test/test_argparse.c
+++ b/app/test/test_argparse.c
@@ -288,13 +288,6 @@ test_argparse_invalid_arg_flags(void)
ret = rte_argparse_parse(obj, default_argc, default_argv);
TEST_ASSERT(ret == -EINVAL, "Argparse parse expect failed!");
- obj = test_argparse_init_obj();
- obj->args[0].val_saver = NULL;
- obj->args[0].flags = RTE_ARGPARSE_ARG_REQUIRED_VALUE |
RTE_ARGPARSE_ARG_SUPPORT_MULTI;
- obj->callback = NULL;
- ret = rte_argparse_parse(obj, default_argc, default_argv);
- TEST_ASSERT(ret == -EINVAL, "Argparse parse expect failed!");
-
return 0;
}
diff --git a/lib/argparse/rte_argparse.c b/lib/argparse/rte_argparse.c
index 48738cd07b..6e890cdc0d 100644
--- a/lib/argparse/rte_argparse.c
+++ b/lib/argparse/rte_argparse.c
@@ -195,12 +195,6 @@ verify_arg_flags(const struct rte_argparse *obj, uint32_t
index)
return -EINVAL;
}
- if (obj->callback == NULL) {
- ARGPARSE_LOG(ERR, "argument %s should use callback to parse,
but callback is NULL!",
-arg->name_long);
- return -EINVAL;
- }
-
return 0;
}
--
2.17.1
[PATCH v3 0/6] refine argparse library
I found a couple of issues when I revisited the argparse_autotest output, so got this patchset. Chengwen Feng (6): argparse: refine error message argparse: remove dead code argparse: replace flag enum with marco argparse: fix argument flags operate as uint32 type test/argparse: refine testcases argparse: fix doc don't display two hyphens --- v3: - address Thomas's comment on 4/6 comit. - add commit: fix doc don't display two hyphens. v2: address David Marchand's comment: - replace flag enum with marco. - replace flag's hardcode with macro in test_argparse.c. app/test/test_argparse.c | 68 - doc/guides/prog_guide/argparse_lib.rst | 47 +++--- lib/argparse/rte_argparse.c| 61 +- lib/argparse/rte_argparse.h| 85 -- 4 files changed, 130 insertions(+), 131 deletions(-) -- 2.17.1
[PATCH v3 4/6] argparse: fix argument flags operate as uint32 type
The struct rte_argparse_arg's flags was 64bit type, uint64_t should be
used instead of uint32_t where the operation happened.
Also, the flags' bit16 was also unused, so don't test bit16 in testcase
test_argparse_invalid_arg_flags.
In addition, this commit introduces two bitmask marcros and removes an
internal duplicate macro.
Fixes: 6c5c6571601c ("argparse: verify argument config")
Fixes: 31ed9f9f43bb ("argparse: parse parameters")
Signed-off-by: Chengwen Feng
---
app/test/test_argparse.c| 21 +
lib/argparse/rte_argparse.c | 24
lib/argparse/rte_argparse.h | 5 +
3 files changed, 30 insertions(+), 20 deletions(-)
diff --git a/app/test/test_argparse.c b/app/test/test_argparse.c
index c98bcee56d..d8dbbee499 100644
--- a/app/test/test_argparse.c
+++ b/app/test/test_argparse.c
@@ -16,6 +16,9 @@ static char *default_argv[1];
static char *strdup_store_array[MAX_STRDUP_STORE_NUM];
static uint32_t strdup_store_index;
+#define TEST_ARGPARSE_FLAG_HAS_ARG_BITMASK RTE_SHIFT_VAL64(3, 0)
+#define TEST_ARGPARSE_FLAG_VAL_TYPE_BITMASKRTE_SHIFT_VAL64(255, 2)
+
/*
* Define strdup wrapper.
* 1. Mainly to fix compile error "warning: assignment discards 'const'
@@ -188,7 +191,7 @@ test_argparse_invalid_arg_help(void)
static int
test_argparse_invalid_has_val(void)
{
- uint32_t set_mask[] = { 0,
+ uint64_t set_mask[] = { 0,
RTE_ARGPARSE_ARG_NO_VALUE,
RTE_ARGPARSE_ARG_OPTIONAL_VALUE
};
@@ -197,7 +200,7 @@ test_argparse_invalid_has_val(void)
int ret;
obj = test_argparse_init_obj();
- obj->args[0].flags &= ~0x3u;
+ obj->args[0].flags &= ~RTE_ARGPARSE_HAS_VAL_BITMASK;
ret = rte_argparse_parse(obj, default_argc, default_argv);
TEST_ASSERT(ret == -EINVAL, "Argparse parse expect failed!");
@@ -205,7 +208,7 @@ test_argparse_invalid_has_val(void)
obj = test_argparse_init_obj();
obj->args[0].name_long = "abc";
obj->args[0].name_short = NULL;
- obj->args[0].flags &= ~0x3u;
+ obj->args[0].flags &= ~RTE_ARGPARSE_HAS_VAL_BITMASK;
obj->args[0].flags |= set_mask[index];
ret = rte_argparse_parse(obj, default_argc, default_argv);
TEST_ASSERT(ret == -EINVAL, "Argparse parse expect failed!");
@@ -269,7 +272,9 @@ test_argparse_invalid_arg_flags(void)
int ret;
obj = test_argparse_init_obj();
- obj->args[0].flags |= ~0x107FFu;
+ obj->args[0].flags |= ~(RTE_ARGPARSE_HAS_VAL_BITMASK |
+ RTE_ARGPARSE_VAL_TYPE_BITMASK |
+ RTE_ARGPARSE_ARG_SUPPORT_MULTI);
ret = rte_argparse_parse(obj, default_argc, default_argv);
TEST_ASSERT(ret == -EINVAL, "Argparse parse expect failed!");
@@ -337,7 +342,7 @@ test_argparse_invalid_option(void)
static int
test_argparse_opt_autosave_parse_int_of_no_val(void)
{
- uint32_t flags = RTE_ARGPARSE_ARG_NO_VALUE | RTE_ARGPARSE_ARG_VALUE_INT;
+ uint64_t flags = RTE_ARGPARSE_ARG_NO_VALUE | RTE_ARGPARSE_ARG_VALUE_INT;
struct rte_argparse *obj;
int val_saver = 0;
char *argv[2];
@@ -369,7 +374,7 @@ test_argparse_opt_autosave_parse_int_of_no_val(void)
static int
test_argparse_opt_autosave_parse_int_of_required_val(void)
{
- uint32_t flags = RTE_ARGPARSE_ARG_REQUIRED_VALUE |
RTE_ARGPARSE_ARG_VALUE_INT;
+ uint64_t flags = RTE_ARGPARSE_ARG_REQUIRED_VALUE |
RTE_ARGPARSE_ARG_VALUE_INT;
struct rte_argparse *obj;
int val_saver = 0;
char *argv[3];
@@ -410,7 +415,7 @@ test_argparse_opt_autosave_parse_int_of_required_val(void)
static int
test_argparse_opt_autosave_parse_int_of_optional_val(void)
{
- uint32_t flags = RTE_ARGPARSE_ARG_OPTIONAL_VALUE |
RTE_ARGPARSE_ARG_VALUE_INT;
+ uint64_t flags = RTE_ARGPARSE_ARG_OPTIONAL_VALUE |
RTE_ARGPARSE_ARG_VALUE_INT;
struct rte_argparse *obj;
int val_saver = 0;
char *argv[2];
@@ -645,7 +650,7 @@ test_argparse_opt_callback_parse_int_of_optional_val(void)
static int
test_argparse_pos_autosave_parse_int(void)
{
- uint32_t flags = RTE_ARGPARSE_ARG_REQUIRED_VALUE |
RTE_ARGPARSE_ARG_VALUE_INT;
+ uint64_t flags = RTE_ARGPARSE_ARG_REQUIRED_VALUE |
RTE_ARGPARSE_ARG_VALUE_INT;
struct rte_argparse *obj;
int val_saver = 0;
char *argv[3];
diff --git a/lib/argparse/rte_argparse.c b/lib/argparse/rte_argparse.c
index 6e890cdc0d..e7007afc6a 100644
--- a/lib/argparse/rte_argparse.c
+++ b/lib/argparse/rte_argparse.c
@@ -15,9 +15,6 @@ RTE_LOG_REGISTER_DEFAULT(rte_argparse_logtype, INFO);
#define ARGPARSE_LOG(level, ...) \
RTE_LOG_LINE(level, ARGPARSE, "" __VA_ARGS__)
-#define ARG_ATTR_HAS_VAL_MASK RTE_GENMASK64(1, 0)
-#define ARG_ATTR_VAL_TYPE_MASK RTE_GENMASK64(9, 2)
-#define ARG_ATT
[PATCH v3 6/6] argparse: fix doc don't display two hyphens
With the line in rst file:
The single mode: "--aaa" or "-a".
corresponding line in html doc:
The single mode: -aaa or -a.
the two hyphens (--aaa) become one (-aaa).
According to [1], this commit uses the backquote (``xxx``) to fix it.
And for consistency, use this format for all arguments.
Fixes: e3e579f5bab5 ("argparse: introduce argparse library")
[1] https://stackoverflow.com/questions/51075907/display-two-dashes-in-rst-file
Signed-off-by: Chengwen Feng
---
doc/guides/prog_guide/argparse_lib.rst | 47 +-
lib/argparse/rte_argparse.h| 4 +--
2 files changed, 26 insertions(+), 25 deletions(-)
diff --git a/doc/guides/prog_guide/argparse_lib.rst
b/doc/guides/prog_guide/argparse_lib.rst
index a6ac11b1c0..f827312daa 100644
--- a/doc/guides/prog_guide/argparse_lib.rst
+++ b/doc/guides/prog_guide/argparse_lib.rst
@@ -90,9 +90,9 @@ The following code demonstrates how to use:
}
In this example, the arguments which start with a hyphen (-) are optional
-arguments (they're "--aaa"/"--bbb"/"--ccc"/"--ddd"/"--eee"/"--fff"); and the
-arguments which don't start with a hyphen (-) are positional arguments
-(they're "ooo"/"ppp").
+arguments (they're
``--aaa``/``--bbb``/``--ccc``/``--ddd``/``--eee``/``--fff``);
+and the arguments which don't start with a hyphen (-) are positional arguments
+(they're ``ooo``/``ppp``).
Every argument must be set whether to carry a value (one of
``RTE_ARGPARSE_ARG_NO_VALUE``, ``RTE_ARGPARSE_ARG_REQUIRED_VALUE`` and
@@ -106,23 +106,23 @@ User Input Requirements
~~~
For optional arguments which take no-value,
-the following mode is supported (take above "--aaa" as an example):
+the following mode is supported (take above ``--aaa`` as an example):
-- The single mode: "--aaa" or "-a".
+- The single mode: ``--aaa`` or ``-a``.
For optional arguments which take required-value,
-the following two modes are supported (take above "--bbb" as an example):
+the following two modes are supported (take above ``--bbb`` as an example):
-- The kv mode: "--bbb=1234" or "-b=1234".
+- The kv mode: ``--bbb=1234`` or ``-b=1234``.
-- The split mode: "--bbb 1234" or "-b 1234".
+- The split mode: ``--bbb 1234`` or ``-b 1234``.
For optional arguments which take optional-value,
-the following two modes are supported (take above "--ccc" as an example):
+the following two modes are supported (take above ``--ccc`` as an example):
-- The single mode: "--ccc" or "-c".
+- The single mode: ``--ccc`` or ``-c``.
-- The kv mode: "--ccc=123" or "-c=123".
+- The kv mode: ``--ccc=123`` or ``-c=123``.
For positional arguments which must take required-value,
their values are parsing in the order defined.
@@ -130,7 +130,7 @@ their values are parsing in the order defined.
.. note::
The compact mode is not supported.
- Take above "-a" and "-d" as an example, don't support "-ad" input.
+ Take above ``-a`` and ``-d`` as an example, don't support ``-ad`` input.
Parsing by autosave way
~~~
@@ -139,23 +139,23 @@ Argument of known value type (e.g.
``RTE_ARGPARSE_ARG_VALUE_INT``)
could be parsed using this autosave way,
and its result will save in the ``val_saver`` field.
-In the above example, the arguments "--aaa"/"--bbb"/"--ccc" and "ooo"
+In the above example, the arguments ``--aaa``/``--bbb``/``--ccc`` and ``ooo``
both use this way, the parsing is as follows:
-- For argument "--aaa", it is configured as no-value,
+- For argument ``--aaa``, it is configured as no-value,
so the ``aaa_val`` will be set to ``val_set`` field
which is 100 in the above example.
-- For argument "--bbb", it is configured as required-value,
+- For argument ``--bbb``, it is configured as required-value,
so the ``bbb_val`` will be set to user input's value
- (e.g. will be set to 1234 with input "--bbb 1234").
+ (e.g. will be set to 1234 with input ``--bbb 1234``).
-- For argument "--ccc", it is configured as optional-value,
- if user only input "--ccc" then the ``ccc_val`` will be set to ``val_set``
field
- which is 200 in the above example;
- if user input "--ccc=123", then the ``ccc_val`` will be set to 123.
+- For argument ``--ccc``, it is configured as optional-value,
+ if user only input ``--ccc`` then the ``ccc_val`` will be set to ``val_set``
+ field which is 200 in the above example;
+ if user input ``--ccc=123``, then the ``ccc_val`` will be set to 123.
-- For argument "ooo", it is positional argument,
+- For argument ``ooo``, it is positional argument,
the ``ooo_val`` will be set to user input's value.
Parsing by callback way
@@ -165,7 +165,8 @@ It could also choose to use callback to parse,
just define a unique index for the argument
and make the ``val_save`` field to be NULL also zero value-type.
-In the above example, the arguments "--ddd"/"--eee"/"--fff" and "ppp" both use
this way.
+In the above example, the arguments ``--ddd``/``--eee``/``--fff`` and ``ppp``
+bo
[PATCH v3 5/6] test/argparse: refine testcases
Refine testcases, including:
1. add testcase comment.
2. argv[0] should set obj->prog_name.
3. set val_set as NULL in test_argparse_invalid_arg_flags, let it
test to the specified code logic.
4. enable index verification in opt_callback_parse_int_of_no_val.
Fixes: 6c5c6571601c ("argparse: verify argument config")
Fixes: 31ed9f9f43bb ("argparse: parse parameters")
Signed-off-by: Chengwen Feng
---
app/test/test_argparse.c | 40 +---
1 file changed, 25 insertions(+), 15 deletions(-)
diff --git a/app/test/test_argparse.c b/app/test/test_argparse.c
index d8dbbee499..4034e7a343 100644
--- a/app/test/test_argparse.c
+++ b/app/test/test_argparse.c
@@ -199,11 +199,13 @@ test_argparse_invalid_has_val(void)
uint32_t index;
int ret;
+ /* test optional arg don't config has-value. */
obj = test_argparse_init_obj();
obj->args[0].flags &= ~RTE_ARGPARSE_HAS_VAL_BITMASK;
ret = rte_argparse_parse(obj, default_argc, default_argv);
TEST_ASSERT(ret == -EINVAL, "Argparse parse expect failed!");
+ /* test positional arg don't config required-value. */
for (index = 0; index < RTE_DIM(set_mask); index++) {
obj = test_argparse_init_obj();
obj->args[0].name_long = "abc";
@@ -271,6 +273,7 @@ test_argparse_invalid_arg_flags(void)
struct rte_argparse *obj;
int ret;
+ /* test set unused bits. */
obj = test_argparse_init_obj();
obj->args[0].flags |= ~(RTE_ARGPARSE_HAS_VAL_BITMASK |
RTE_ARGPARSE_VAL_TYPE_BITMASK |
@@ -278,16 +281,18 @@ test_argparse_invalid_arg_flags(void)
ret = rte_argparse_parse(obj, default_argc, default_argv);
TEST_ASSERT(ret == -EINVAL, "Argparse parse expect failed!");
+ /* test positional arg should not config multiple. */
obj = test_argparse_init_obj();
obj->args[0].name_long = "positional";
obj->args[0].name_short = NULL;
obj->args[0].val_saver = (void *)1;
- obj->args[0].val_set = (void *)1;
+ obj->args[0].val_set = NULL;
obj->args[0].flags = RTE_ARGPARSE_ARG_REQUIRED_VALUE |
RTE_ARGPARSE_ARG_VALUE_INT |
RTE_ARGPARSE_ARG_SUPPORT_MULTI;
ret = rte_argparse_parse(obj, default_argc, default_argv);
TEST_ASSERT(ret == -EINVAL, "Argparse parse expect failed!");
+ /* test optional arg enabled multiple but prased by autosave. */
obj = test_argparse_init_obj();
obj->args[0].flags |= RTE_ARGPARSE_ARG_SUPPORT_MULTI;
ret = rte_argparse_parse(obj, default_argc, default_argv);
@@ -325,13 +330,13 @@ test_argparse_invalid_option(void)
int ret;
obj = test_argparse_init_obj();
- argv[0] = test_strdup(obj->usage);
+ argv[0] = test_strdup(obj->prog_name);
argv[1] = test_strdup("--invalid");
ret = rte_argparse_parse(obj, 2, argv);
TEST_ASSERT(ret == -EINVAL, "Argparse parse expect failed!");
obj = test_argparse_init_obj();
- argv[0] = test_strdup(obj->usage);
+ argv[0] = test_strdup(obj->prog_name);
argv[1] = test_strdup("invalid");
ret = rte_argparse_parse(obj, 2, argv);
TEST_ASSERT(ret == -EINVAL, "Argparse parse expect failed!");
@@ -355,7 +360,7 @@ test_argparse_opt_autosave_parse_int_of_no_val(void)
obj->args[0].val_set = (void *)100;
obj->args[0].flags = flags;
obj->args[1].name_long = NULL;
- argv[0] = test_strdup(obj->usage);
+ argv[0] = test_strdup(obj->prog_name);
argv[1] = test_strdup("--test-long");
ret = rte_argparse_parse(obj, 2, argv);
TEST_ASSERT(ret == 0, "Argparse parse expect success!");
@@ -387,7 +392,7 @@ test_argparse_opt_autosave_parse_int_of_required_val(void)
obj->args[0].val_set = NULL;
obj->args[0].flags = flags;
obj->args[1].name_long = NULL;
- argv[0] = test_strdup(obj->usage);
+ argv[0] = test_strdup(obj->prog_name);
argv[1] = test_strdup("--test-long");
argv[2] = test_strdup("100");
ret = rte_argparse_parse(obj, 3, argv);
@@ -421,6 +426,7 @@ test_argparse_opt_autosave_parse_int_of_optional_val(void)
char *argv[2];
int ret;
+ /* test without value. */
obj = test_argparse_init_obj();
obj->args[0].name_long = "--test-long";
obj->args[0].name_short = "-t";
@@ -428,7 +434,7 @@ test_argparse_opt_autosave_parse_int_of_optional_val(void)
obj->args[0].val_set = (void *)100;
obj->args[0].flags = flags;
obj->args[1].name_long = NULL;
- argv[0] = test_strdup(obj->usage);
+ argv[0] = test_strdup(obj->prog_name);
argv[1] = test_strdup("--test-long");
ret = rte_argparse_parse(obj, 2, argv);
TEST_ASSERT(ret == 0, "Argparse parse expect success!");
@@ -472,7 +478,8 @@ test_argparse_opt_autosave_parse_int_of_optional_val(void)
static
[PATCH v3 1/6] argparse: refine error message
This patch refines the error message.
Signed-off-by: Chengwen Feng
---
lib/argparse/rte_argparse.c | 31 +--
1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/lib/argparse/rte_argparse.c b/lib/argparse/rte_argparse.c
index 2d953f1694..48738cd07b 100644
--- a/lib/argparse/rte_argparse.c
+++ b/lib/argparse/rte_argparse.c
@@ -67,7 +67,7 @@ verify_arg_name(const struct rte_argparse_arg *arg)
return -EINVAL;
}
if (arg->name_long[1] != '-') {
- ARGPARSE_LOG(ERR, "optional long name %s must only
start with '--'",
+ ARGPARSE_LOG(ERR, "optional long name %s doesn't start
with '--'",
arg->name_long);
return -EINVAL;
}
@@ -101,7 +101,7 @@ static int
verify_arg_help(const struct rte_argparse_arg *arg)
{
if (arg->help == NULL) {
- ARGPARSE_LOG(ERR, "argument %s must have help info!",
arg->name_long);
+ ARGPARSE_LOG(ERR, "argument %s doesn't have help info!",
arg->name_long);
return -EINVAL;
}
@@ -116,13 +116,13 @@ verify_arg_has_val(const struct rte_argparse_arg *arg)
if (is_arg_positional(arg)) {
if (has_val == RTE_ARGPARSE_ARG_REQUIRED_VALUE)
return 0;
- ARGPARSE_LOG(ERR, "argument %s is positional, should has zero
or required-val!",
+ ARGPARSE_LOG(ERR, "argument %s is positional, must config
required-val!",
arg->name_long);
return -EINVAL;
}
if (has_val == 0) {
- ARGPARSE_LOG(ERR, "argument %s is optional, has-val config
wrong!",
+ ARGPARSE_LOG(ERR, "argument %s is optional, has-value config
wrong!",
arg->name_long);
return -EINVAL;
}
@@ -140,13 +140,13 @@ verify_arg_saver(const struct rte_argparse *obj, uint32_t
index)
if (arg->val_saver == NULL) {
if (val_type != 0) {
- ARGPARSE_LOG(ERR, "argument %s parse by callback,
val-type must be zero!",
+ ARGPARSE_LOG(ERR, "argument %s parsed by callback,
value-type should not be set!",
arg->name_long);
return -EINVAL;
}
if (obj->callback == NULL) {
- ARGPARSE_LOG(ERR, "argument %s parse by callback, but
callback is NULL!",
+ ARGPARSE_LOG(ERR, "argument %s parsed by callback, but
callback is NULL!",
arg->name_long);
return -EINVAL;
}
@@ -155,12 +155,12 @@ verify_arg_saver(const struct rte_argparse *obj, uint32_t
index)
}
if (val_type == 0 || val_type >= cmp_max) {
- ARGPARSE_LOG(ERR, "argument %s val-type config wrong!",
arg->name_long);
+ ARGPARSE_LOG(ERR, "argument %s value-type config wrong!",
arg->name_long);
return -EINVAL;
}
if (has_val == RTE_ARGPARSE_ARG_REQUIRED_VALUE && arg->val_set != NULL)
{
- ARGPARSE_LOG(ERR, "argument %s has required value, val-set
should be NULL!",
+ ARGPARSE_LOG(ERR, "argument %s has required value, value-set
should be NULL!",
arg->name_long);
return -EINVAL;
}
@@ -175,7 +175,8 @@ verify_arg_flags(const struct rte_argparse *obj, uint32_t
index)
uint32_t unused_bits = arg_attr_unused_bits(arg);
if (unused_bits != 0) {
- ARGPARSE_LOG(ERR, "argument %s flags set wrong!",
arg->name_long);
+ ARGPARSE_LOG(ERR, "argument %s flags unused bits should not be
set!",
+arg->name_long);
return -EINVAL;
}
@@ -189,7 +190,7 @@ verify_arg_flags(const struct rte_argparse *obj, uint32_t
index)
}
if (arg->val_saver != NULL) {
- ARGPARSE_LOG(ERR, "argument %s could occur multiple times,
should use callback to parse!",
+ ARGPARSE_LOG(ERR, "argument %s supports multiple times, should
use callback to parse!",
arg->name_long);
return -EINVAL;
}
@@ -536,8 +537,10 @@ parse_arg_autosave(struct rte_argparse_arg *arg, const
char *value)
return ret;
}
+/* arg_parse indicates the name entered by the user, which can be long-name or
short-name. */
static int
-parse_arg_val(struct rte_argparse *obj, struct rte_argparse_arg *arg, char
*value)
+parse_arg_val(struct rte_argparse *obj, const char *arg_name,
+ struct rte_argparse_arg *arg, char *value)
{
int ret;
@@ -546,7 +549,7 @@ parse_arg_val(struct rte_argparse *obj, struct
rte_argparse_arg *arg, char *valu
else
[PATCH v3 3/6] argparse: replace flag enum with marco
The enum rte_argparse_flag's value is u64, but an enum in C is
represented as an int. This commit replace these enum values with
macro.
Fixes: e3e579f5bab5 ("argparse: introduce argparse library")
Fixes: 5357c248c960 ("argparse: parse unsigned integers")
Signed-off-by: Chengwen Feng
---
lib/argparse/rte_argparse.h | 78 -
1 file changed, 33 insertions(+), 45 deletions(-)
diff --git a/lib/argparse/rte_argparse.h b/lib/argparse/rte_argparse.h
index 47e231bef9..a6a7790cb4 100644
--- a/lib/argparse/rte_argparse.h
+++ b/lib/argparse/rte_argparse.h
@@ -37,52 +37,40 @@
extern "C" {
#endif
+/**@{@name Flag definition (in bitmask form) for an argument
+ *
+ * @note Bits[0~1] represent the argument whether has value,
+ * bits[2~9] represent the value type which used when autosave.
+ *
+ * @see struct rte_argparse_arg::flags
+ */
+/** The argument has no value. */
+#define RTE_ARGPARSE_ARG_NO_VALUE RTE_SHIFT_VAL64(1, 0)
+/** The argument must have a value. */
+#define RTE_ARGPARSE_ARG_REQUIRED_VALUE RTE_SHIFT_VAL64(2, 0)
+/** The argument has optional value. */
+#define RTE_ARGPARSE_ARG_OPTIONAL_VALUE RTE_SHIFT_VAL64(3, 0)
+/** The argument's value is int type. */
+#define RTE_ARGPARSE_ARG_VALUE_INT RTE_SHIFT_VAL64(1, 2)
+/** The argument's value is uint8 type. */
+#define RTE_ARGPARSE_ARG_VALUE_U8 RTE_SHIFT_VAL64(2, 2)
+/** The argument's value is uint16 type. */
+#define RTE_ARGPARSE_ARG_VALUE_U16 RTE_SHIFT_VAL64(3, 2)
+/** The argument's value is uint32 type. */
+#define RTE_ARGPARSE_ARG_VALUE_U32 RTE_SHIFT_VAL64(4, 2)
+/** The argument's value is uint64 type. */
+#define RTE_ARGPARSE_ARG_VALUE_U64 RTE_SHIFT_VAL64(5, 2)
+/** Max value type. */
+#define RTE_ARGPARSE_ARG_VALUE_MAX RTE_SHIFT_VAL64(6, 2)
/**
- * Flag definition (in bitmask form) for an argument.
+ * Flag for that argument support occur multiple times.
+ * This flag can be set only when the argument is optional.
+ * When this flag is set, the callback type must be used for parsing.
*/
-enum rte_argparse_flag {
- /*
-* Bits 0-1: represent the argument whether has value.
-*/
-
- /** The argument has no value. */
- RTE_ARGPARSE_ARG_NO_VALUE = RTE_SHIFT_VAL64(1, 0),
- /** The argument must have a value. */
- RTE_ARGPARSE_ARG_REQUIRED_VALUE = RTE_SHIFT_VAL64(2, 0),
- /** The argument has optional value. */
- RTE_ARGPARSE_ARG_OPTIONAL_VALUE = RTE_SHIFT_VAL64(3, 0),
-
-
- /*
-* Bits 2-9: represent the value type which used when autosave
-*/
-
- /** The argument's value is int type. */
- RTE_ARGPARSE_ARG_VALUE_INT = RTE_SHIFT_VAL64(1, 2),
- /** The argument's value is uint8 type. */
- RTE_ARGPARSE_ARG_VALUE_U8 = RTE_SHIFT_VAL64(2, 2),
- /** The argument's value is uint16 type. */
- RTE_ARGPARSE_ARG_VALUE_U16 = RTE_SHIFT_VAL64(3, 2),
- /** The argument's value is uint32 type. */
- RTE_ARGPARSE_ARG_VALUE_U32 = RTE_SHIFT_VAL64(4, 2),
- /** The argument's value is uint64 type. */
- RTE_ARGPARSE_ARG_VALUE_U64 = RTE_SHIFT_VAL64(5, 2),
- /** Max value type. */
- RTE_ARGPARSE_ARG_VALUE_MAX = RTE_SHIFT_VAL64(6, 2),
-
-
- /**
-* Bit 10: flag for that argument support occur multiple times.
-* This flag can be set only when the argument is optional.
-* When this flag is set, the callback type must be used for parsing.
-*/
- RTE_ARGPARSE_ARG_SUPPORT_MULTI = RTE_BIT64(10),
-
- /**
-* Bits 48-63: reserved for this library implementation usage.
-*/
- RTE_ARGPARSE_ARG_RESERVED_FIELD = RTE_GENMASK64(63, 48),
-};
+#define RTE_ARGPARSE_ARG_SUPPORT_MULTI RTE_BIT64(10)
+/** Reserved for this library implementation usage. */
+#define RTE_ARGPARSE_ARG_RESERVED_FIELD RTE_GENMASK64(63, 48)
+/**@}*/
/**
* A structure used to hold argument's configuration.
@@ -126,7 +114,7 @@ struct rte_argparse_arg {
*/
void *val_set;
- /** @see rte_argparse_flag */
+ /** Flag definition (RTE_ARGPARSE_ARG_*) for the argument. */
uint64_t flags;
};
--
2.17.1
Re: [PATCH v2 4/5] argparse: fix argument flags operate as uint32 type
Hi Thomas, On 2024/3/18 10:31, Thomas Monjalon wrote: > 07/03/2024 14:07, Chengwen Feng: >> --- a/app/test/test_argparse.c >> +++ b/app/test/test_argparse.c >> +#define TEST_ARGPARSE_FLAG_HAS_ARG_BITMASK RTE_SHIFT_VAL64(3, 0) >> +#define TEST_ARGPARSE_FLAG_VAL_TYPE_BITMASK RTE_SHIFT_VAL64(255, 2) > > These masks should be defined in the API probably. done in v3 Thanks > > > . >
Re: [PATCH v3 4/6] argparse: fix argument flags operate as uint32 type
18/03/2024 10:18, Chengwen Feng: > --- a/app/test/test_argparse.c > +++ b/app/test/test_argparse.c > +#define TEST_ARGPARSE_FLAG_HAS_ARG_BITMASK RTE_SHIFT_VAL64(3, 0) > +#define TEST_ARGPARSE_FLAG_VAL_TYPE_BITMASK RTE_SHIFT_VAL64(255, 2) Don't you want to make it part of the API? I thought it was what you did in this v3.
[PATCH] app/testpmd: fix auto completion for indirect list action
In the process of auto completion of a command in testpmd,
the parser splits the command into tokens, where each token
represents an argument and defines a parsing function.
The parsing function of the indirect_list action argument was returning
before having the opportunity to handle the argument.
The fix ensures that the function appropriately handles
the argument before finishing.
Fixes: 72a3dec7126f ("ethdev: add indirect flow list action")
Signed-off-by: Shani Peretz
---
app/test-pmd/cmdline_flow.c | 46 -
1 file changed, 25 insertions(+), 21 deletions(-)
diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index fd6c51f72d..60ee9337cf 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -7839,11 +7839,13 @@ static const struct token token_list[] = {
.type = "UNSIGNED",
.help = "unsigned integer value",
.call = parse_indlst_id2ptr,
+ .comp = comp_none,
},
[INDIRECT_LIST_ACTION_ID2PTR_CONF] = {
.type = "UNSIGNED",
.help = "unsigned integer value",
.call = parse_indlst_id2ptr,
+ .comp = comp_none,
},
[ACTION_SHARED_INDIRECT] = {
.name = "shared_indirect",
@@ -11912,34 +11914,36 @@ parse_indlst_id2ptr(struct context *ctx, const struct
token *token,
uint32_t id;
int ret;
- if (!action)
- return -1;
ctx->objdata = 0;
ctx->object = &id;
ctx->objmask = NULL;
ret = parse_int(ctx, token, str, len, ctx->object, sizeof(id));
+ ctx->object = action;
if (ret != (int)len)
return ret;
- ctx->object = action;
- action_conf = (void *)(uintptr_t)action->conf;
- action_conf->conf = NULL;
- switch (ctx->curr) {
- case INDIRECT_LIST_ACTION_ID2PTR_HANDLE:
- action_conf->handle = (typeof(action_conf->handle))
- port_action_handle_get_by_id(ctx->port, id);
- if (!action_conf->handle) {
- printf("no indirect list handle for id %u\n", id);
- return -1;
+
+ /* set handle and conf */
+ if (action) {
+ action_conf = (void *)(uintptr_t)action->conf;
+ action_conf->conf = NULL;
+ switch (ctx->curr) {
+ case INDIRECT_LIST_ACTION_ID2PTR_HANDLE:
+ action_conf->handle = (typeof(action_conf->handle))
+ port_action_handle_get_by_id(ctx->port,
id);
+ if (!action_conf->handle) {
+ printf("no indirect list handle for id %u\n",
id);
+ return -1;
+ }
+ break;
+ case INDIRECT_LIST_ACTION_ID2PTR_CONF:
+ indlst_conf = indirect_action_list_conf_get(id);
+ if (!indlst_conf)
+ return -1;
+ action_conf->conf = (const void **)indlst_conf->conf;
+ break;
+ default:
+ break;
}
- break;
- case INDIRECT_LIST_ACTION_ID2PTR_CONF:
- indlst_conf = indirect_action_list_conf_get(id);
- if (!indlst_conf)
- return -1;
- action_conf->conf = (const void **)indlst_conf->conf;
- break;
- default:
- break;
}
return ret;
}
--
2.34.1
[PATCH v4 0/6] refine argparse library
I found a couple of issues when I revisited the argparse_autotest output, so got this patchset. Chengwen Feng (6): argparse: refine error message argparse: remove dead code argparse: replace flag enum with marco argparse: fix argument flags operate as uint32 type test/argparse: refine testcases argparse: fix doc don't display two hyphens --- v4: address Thomas's comment on 4/6 commit: - remove unused macros TEST_ARGPARSE_FLAG_HAS_ARG_BITMASK and TEST_ARGPARSE_FLAG_VAL_TYPE_BITMASK. v3: - address Thomas's comment on 4/6 comit. - add commit: fix doc don't display two hyphens. v2: address David Marchand's comment: - replace flag enum with marco. - replace flag's hardcode with macro in test_argparse.c. app/test/test_argparse.c | 65 +++- doc/guides/prog_guide/argparse_lib.rst | 47 +++--- lib/argparse/rte_argparse.c| 61 +- lib/argparse/rte_argparse.h| 85 -- 4 files changed, 127 insertions(+), 131 deletions(-) -- 2.17.1
[PATCH v4 1/6] argparse: refine error message
This patch refines the error message.
Signed-off-by: Chengwen Feng
---
lib/argparse/rte_argparse.c | 31 +--
1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/lib/argparse/rte_argparse.c b/lib/argparse/rte_argparse.c
index 2d953f1694..48738cd07b 100644
--- a/lib/argparse/rte_argparse.c
+++ b/lib/argparse/rte_argparse.c
@@ -67,7 +67,7 @@ verify_arg_name(const struct rte_argparse_arg *arg)
return -EINVAL;
}
if (arg->name_long[1] != '-') {
- ARGPARSE_LOG(ERR, "optional long name %s must only
start with '--'",
+ ARGPARSE_LOG(ERR, "optional long name %s doesn't start
with '--'",
arg->name_long);
return -EINVAL;
}
@@ -101,7 +101,7 @@ static int
verify_arg_help(const struct rte_argparse_arg *arg)
{
if (arg->help == NULL) {
- ARGPARSE_LOG(ERR, "argument %s must have help info!",
arg->name_long);
+ ARGPARSE_LOG(ERR, "argument %s doesn't have help info!",
arg->name_long);
return -EINVAL;
}
@@ -116,13 +116,13 @@ verify_arg_has_val(const struct rte_argparse_arg *arg)
if (is_arg_positional(arg)) {
if (has_val == RTE_ARGPARSE_ARG_REQUIRED_VALUE)
return 0;
- ARGPARSE_LOG(ERR, "argument %s is positional, should has zero
or required-val!",
+ ARGPARSE_LOG(ERR, "argument %s is positional, must config
required-val!",
arg->name_long);
return -EINVAL;
}
if (has_val == 0) {
- ARGPARSE_LOG(ERR, "argument %s is optional, has-val config
wrong!",
+ ARGPARSE_LOG(ERR, "argument %s is optional, has-value config
wrong!",
arg->name_long);
return -EINVAL;
}
@@ -140,13 +140,13 @@ verify_arg_saver(const struct rte_argparse *obj, uint32_t
index)
if (arg->val_saver == NULL) {
if (val_type != 0) {
- ARGPARSE_LOG(ERR, "argument %s parse by callback,
val-type must be zero!",
+ ARGPARSE_LOG(ERR, "argument %s parsed by callback,
value-type should not be set!",
arg->name_long);
return -EINVAL;
}
if (obj->callback == NULL) {
- ARGPARSE_LOG(ERR, "argument %s parse by callback, but
callback is NULL!",
+ ARGPARSE_LOG(ERR, "argument %s parsed by callback, but
callback is NULL!",
arg->name_long);
return -EINVAL;
}
@@ -155,12 +155,12 @@ verify_arg_saver(const struct rte_argparse *obj, uint32_t
index)
}
if (val_type == 0 || val_type >= cmp_max) {
- ARGPARSE_LOG(ERR, "argument %s val-type config wrong!",
arg->name_long);
+ ARGPARSE_LOG(ERR, "argument %s value-type config wrong!",
arg->name_long);
return -EINVAL;
}
if (has_val == RTE_ARGPARSE_ARG_REQUIRED_VALUE && arg->val_set != NULL)
{
- ARGPARSE_LOG(ERR, "argument %s has required value, val-set
should be NULL!",
+ ARGPARSE_LOG(ERR, "argument %s has required value, value-set
should be NULL!",
arg->name_long);
return -EINVAL;
}
@@ -175,7 +175,8 @@ verify_arg_flags(const struct rte_argparse *obj, uint32_t
index)
uint32_t unused_bits = arg_attr_unused_bits(arg);
if (unused_bits != 0) {
- ARGPARSE_LOG(ERR, "argument %s flags set wrong!",
arg->name_long);
+ ARGPARSE_LOG(ERR, "argument %s flags unused bits should not be
set!",
+arg->name_long);
return -EINVAL;
}
@@ -189,7 +190,7 @@ verify_arg_flags(const struct rte_argparse *obj, uint32_t
index)
}
if (arg->val_saver != NULL) {
- ARGPARSE_LOG(ERR, "argument %s could occur multiple times,
should use callback to parse!",
+ ARGPARSE_LOG(ERR, "argument %s supports multiple times, should
use callback to parse!",
arg->name_long);
return -EINVAL;
}
@@ -536,8 +537,10 @@ parse_arg_autosave(struct rte_argparse_arg *arg, const
char *value)
return ret;
}
+/* arg_parse indicates the name entered by the user, which can be long-name or
short-name. */
static int
-parse_arg_val(struct rte_argparse *obj, struct rte_argparse_arg *arg, char
*value)
+parse_arg_val(struct rte_argparse *obj, const char *arg_name,
+ struct rte_argparse_arg *arg, char *value)
{
int ret;
@@ -546,7 +549,7 @@ parse_arg_val(struct rte_argparse *obj, struct
rte_argparse_arg *arg, char *valu
else
[PATCH v4 2/6] argparse: remove dead code
The judgement "obj->callback == NULL" is dead code which can't be
reached, because verify_arg_saver() already make sure obj->callback
must not be NULL when arg->val_saver is NULL.
Signed-off-by: Chengwen Feng
---
app/test/test_argparse.c| 7 ---
lib/argparse/rte_argparse.c | 6 --
2 files changed, 13 deletions(-)
diff --git a/app/test/test_argparse.c b/app/test/test_argparse.c
index df11a129ba..c98bcee56d 100644
--- a/app/test/test_argparse.c
+++ b/app/test/test_argparse.c
@@ -288,13 +288,6 @@ test_argparse_invalid_arg_flags(void)
ret = rte_argparse_parse(obj, default_argc, default_argv);
TEST_ASSERT(ret == -EINVAL, "Argparse parse expect failed!");
- obj = test_argparse_init_obj();
- obj->args[0].val_saver = NULL;
- obj->args[0].flags = RTE_ARGPARSE_ARG_REQUIRED_VALUE |
RTE_ARGPARSE_ARG_SUPPORT_MULTI;
- obj->callback = NULL;
- ret = rte_argparse_parse(obj, default_argc, default_argv);
- TEST_ASSERT(ret == -EINVAL, "Argparse parse expect failed!");
-
return 0;
}
diff --git a/lib/argparse/rte_argparse.c b/lib/argparse/rte_argparse.c
index 48738cd07b..6e890cdc0d 100644
--- a/lib/argparse/rte_argparse.c
+++ b/lib/argparse/rte_argparse.c
@@ -195,12 +195,6 @@ verify_arg_flags(const struct rte_argparse *obj, uint32_t
index)
return -EINVAL;
}
- if (obj->callback == NULL) {
- ARGPARSE_LOG(ERR, "argument %s should use callback to parse,
but callback is NULL!",
-arg->name_long);
- return -EINVAL;
- }
-
return 0;
}
--
2.17.1
[PATCH v4 5/6] test/argparse: refine testcases
Refine testcases, including:
1. add testcase comment.
2. argv[0] should set obj->prog_name.
3. set val_set as NULL in test_argparse_invalid_arg_flags, let it
test to the specified code logic.
4. enable index verification in opt_callback_parse_int_of_no_val.
Fixes: 6c5c6571601c ("argparse: verify argument config")
Fixes: 31ed9f9f43bb ("argparse: parse parameters")
Signed-off-by: Chengwen Feng
---
app/test/test_argparse.c | 40 +---
1 file changed, 25 insertions(+), 15 deletions(-)
diff --git a/app/test/test_argparse.c b/app/test/test_argparse.c
index 1a7211eb01..fcea620501 100644
--- a/app/test/test_argparse.c
+++ b/app/test/test_argparse.c
@@ -196,11 +196,13 @@ test_argparse_invalid_has_val(void)
uint32_t index;
int ret;
+ /* test optional arg don't config has-value. */
obj = test_argparse_init_obj();
obj->args[0].flags &= ~RTE_ARGPARSE_HAS_VAL_BITMASK;
ret = rte_argparse_parse(obj, default_argc, default_argv);
TEST_ASSERT(ret == -EINVAL, "Argparse parse expect failed!");
+ /* test positional arg don't config required-value. */
for (index = 0; index < RTE_DIM(set_mask); index++) {
obj = test_argparse_init_obj();
obj->args[0].name_long = "abc";
@@ -268,6 +270,7 @@ test_argparse_invalid_arg_flags(void)
struct rte_argparse *obj;
int ret;
+ /* test set unused bits. */
obj = test_argparse_init_obj();
obj->args[0].flags |= ~(RTE_ARGPARSE_HAS_VAL_BITMASK |
RTE_ARGPARSE_VAL_TYPE_BITMASK |
@@ -275,16 +278,18 @@ test_argparse_invalid_arg_flags(void)
ret = rte_argparse_parse(obj, default_argc, default_argv);
TEST_ASSERT(ret == -EINVAL, "Argparse parse expect failed!");
+ /* test positional arg should not config multiple. */
obj = test_argparse_init_obj();
obj->args[0].name_long = "positional";
obj->args[0].name_short = NULL;
obj->args[0].val_saver = (void *)1;
- obj->args[0].val_set = (void *)1;
+ obj->args[0].val_set = NULL;
obj->args[0].flags = RTE_ARGPARSE_ARG_REQUIRED_VALUE |
RTE_ARGPARSE_ARG_VALUE_INT |
RTE_ARGPARSE_ARG_SUPPORT_MULTI;
ret = rte_argparse_parse(obj, default_argc, default_argv);
TEST_ASSERT(ret == -EINVAL, "Argparse parse expect failed!");
+ /* test optional arg enabled multiple but prased by autosave. */
obj = test_argparse_init_obj();
obj->args[0].flags |= RTE_ARGPARSE_ARG_SUPPORT_MULTI;
ret = rte_argparse_parse(obj, default_argc, default_argv);
@@ -322,13 +327,13 @@ test_argparse_invalid_option(void)
int ret;
obj = test_argparse_init_obj();
- argv[0] = test_strdup(obj->usage);
+ argv[0] = test_strdup(obj->prog_name);
argv[1] = test_strdup("--invalid");
ret = rte_argparse_parse(obj, 2, argv);
TEST_ASSERT(ret == -EINVAL, "Argparse parse expect failed!");
obj = test_argparse_init_obj();
- argv[0] = test_strdup(obj->usage);
+ argv[0] = test_strdup(obj->prog_name);
argv[1] = test_strdup("invalid");
ret = rte_argparse_parse(obj, 2, argv);
TEST_ASSERT(ret == -EINVAL, "Argparse parse expect failed!");
@@ -352,7 +357,7 @@ test_argparse_opt_autosave_parse_int_of_no_val(void)
obj->args[0].val_set = (void *)100;
obj->args[0].flags = flags;
obj->args[1].name_long = NULL;
- argv[0] = test_strdup(obj->usage);
+ argv[0] = test_strdup(obj->prog_name);
argv[1] = test_strdup("--test-long");
ret = rte_argparse_parse(obj, 2, argv);
TEST_ASSERT(ret == 0, "Argparse parse expect success!");
@@ -384,7 +389,7 @@ test_argparse_opt_autosave_parse_int_of_required_val(void)
obj->args[0].val_set = NULL;
obj->args[0].flags = flags;
obj->args[1].name_long = NULL;
- argv[0] = test_strdup(obj->usage);
+ argv[0] = test_strdup(obj->prog_name);
argv[1] = test_strdup("--test-long");
argv[2] = test_strdup("100");
ret = rte_argparse_parse(obj, 3, argv);
@@ -418,6 +423,7 @@ test_argparse_opt_autosave_parse_int_of_optional_val(void)
char *argv[2];
int ret;
+ /* test without value. */
obj = test_argparse_init_obj();
obj->args[0].name_long = "--test-long";
obj->args[0].name_short = "-t";
@@ -425,7 +431,7 @@ test_argparse_opt_autosave_parse_int_of_optional_val(void)
obj->args[0].val_set = (void *)100;
obj->args[0].flags = flags;
obj->args[1].name_long = NULL;
- argv[0] = test_strdup(obj->usage);
+ argv[0] = test_strdup(obj->prog_name);
argv[1] = test_strdup("--test-long");
ret = rte_argparse_parse(obj, 2, argv);
TEST_ASSERT(ret == 0, "Argparse parse expect success!");
@@ -469,7 +475,8 @@ test_argparse_opt_autosave_parse_int_of_optional_val(void)
static
[PATCH v4 3/6] argparse: replace flag enum with marco
The enum rte_argparse_flag's value is u64, but an enum in C is
represented as an int. This commit replace these enum values with
macro.
Fixes: e3e579f5bab5 ("argparse: introduce argparse library")
Fixes: 5357c248c960 ("argparse: parse unsigned integers")
Signed-off-by: Chengwen Feng
---
lib/argparse/rte_argparse.h | 78 -
1 file changed, 33 insertions(+), 45 deletions(-)
diff --git a/lib/argparse/rte_argparse.h b/lib/argparse/rte_argparse.h
index 47e231bef9..a6a7790cb4 100644
--- a/lib/argparse/rte_argparse.h
+++ b/lib/argparse/rte_argparse.h
@@ -37,52 +37,40 @@
extern "C" {
#endif
+/**@{@name Flag definition (in bitmask form) for an argument
+ *
+ * @note Bits[0~1] represent the argument whether has value,
+ * bits[2~9] represent the value type which used when autosave.
+ *
+ * @see struct rte_argparse_arg::flags
+ */
+/** The argument has no value. */
+#define RTE_ARGPARSE_ARG_NO_VALUE RTE_SHIFT_VAL64(1, 0)
+/** The argument must have a value. */
+#define RTE_ARGPARSE_ARG_REQUIRED_VALUE RTE_SHIFT_VAL64(2, 0)
+/** The argument has optional value. */
+#define RTE_ARGPARSE_ARG_OPTIONAL_VALUE RTE_SHIFT_VAL64(3, 0)
+/** The argument's value is int type. */
+#define RTE_ARGPARSE_ARG_VALUE_INT RTE_SHIFT_VAL64(1, 2)
+/** The argument's value is uint8 type. */
+#define RTE_ARGPARSE_ARG_VALUE_U8 RTE_SHIFT_VAL64(2, 2)
+/** The argument's value is uint16 type. */
+#define RTE_ARGPARSE_ARG_VALUE_U16 RTE_SHIFT_VAL64(3, 2)
+/** The argument's value is uint32 type. */
+#define RTE_ARGPARSE_ARG_VALUE_U32 RTE_SHIFT_VAL64(4, 2)
+/** The argument's value is uint64 type. */
+#define RTE_ARGPARSE_ARG_VALUE_U64 RTE_SHIFT_VAL64(5, 2)
+/** Max value type. */
+#define RTE_ARGPARSE_ARG_VALUE_MAX RTE_SHIFT_VAL64(6, 2)
/**
- * Flag definition (in bitmask form) for an argument.
+ * Flag for that argument support occur multiple times.
+ * This flag can be set only when the argument is optional.
+ * When this flag is set, the callback type must be used for parsing.
*/
-enum rte_argparse_flag {
- /*
-* Bits 0-1: represent the argument whether has value.
-*/
-
- /** The argument has no value. */
- RTE_ARGPARSE_ARG_NO_VALUE = RTE_SHIFT_VAL64(1, 0),
- /** The argument must have a value. */
- RTE_ARGPARSE_ARG_REQUIRED_VALUE = RTE_SHIFT_VAL64(2, 0),
- /** The argument has optional value. */
- RTE_ARGPARSE_ARG_OPTIONAL_VALUE = RTE_SHIFT_VAL64(3, 0),
-
-
- /*
-* Bits 2-9: represent the value type which used when autosave
-*/
-
- /** The argument's value is int type. */
- RTE_ARGPARSE_ARG_VALUE_INT = RTE_SHIFT_VAL64(1, 2),
- /** The argument's value is uint8 type. */
- RTE_ARGPARSE_ARG_VALUE_U8 = RTE_SHIFT_VAL64(2, 2),
- /** The argument's value is uint16 type. */
- RTE_ARGPARSE_ARG_VALUE_U16 = RTE_SHIFT_VAL64(3, 2),
- /** The argument's value is uint32 type. */
- RTE_ARGPARSE_ARG_VALUE_U32 = RTE_SHIFT_VAL64(4, 2),
- /** The argument's value is uint64 type. */
- RTE_ARGPARSE_ARG_VALUE_U64 = RTE_SHIFT_VAL64(5, 2),
- /** Max value type. */
- RTE_ARGPARSE_ARG_VALUE_MAX = RTE_SHIFT_VAL64(6, 2),
-
-
- /**
-* Bit 10: flag for that argument support occur multiple times.
-* This flag can be set only when the argument is optional.
-* When this flag is set, the callback type must be used for parsing.
-*/
- RTE_ARGPARSE_ARG_SUPPORT_MULTI = RTE_BIT64(10),
-
- /**
-* Bits 48-63: reserved for this library implementation usage.
-*/
- RTE_ARGPARSE_ARG_RESERVED_FIELD = RTE_GENMASK64(63, 48),
-};
+#define RTE_ARGPARSE_ARG_SUPPORT_MULTI RTE_BIT64(10)
+/** Reserved for this library implementation usage. */
+#define RTE_ARGPARSE_ARG_RESERVED_FIELD RTE_GENMASK64(63, 48)
+/**@}*/
/**
* A structure used to hold argument's configuration.
@@ -126,7 +114,7 @@ struct rte_argparse_arg {
*/
void *val_set;
- /** @see rte_argparse_flag */
+ /** Flag definition (RTE_ARGPARSE_ARG_*) for the argument. */
uint64_t flags;
};
--
2.17.1
[PATCH v4 4/6] argparse: fix argument flags operate as uint32 type
The struct rte_argparse_arg's flags was 64bit type, uint64_t should be
used instead of uint32_t where the operation happened.
Also, the flags' bit16 was also unused, so don't test bit16 in testcase
test_argparse_invalid_arg_flags.
In addition, this commit introduces two bitmask marcros and removes an
internal duplicate macro.
Fixes: 6c5c6571601c ("argparse: verify argument config")
Fixes: 31ed9f9f43bb ("argparse: parse parameters")
Signed-off-by: Chengwen Feng
---
app/test/test_argparse.c| 18 ++
lib/argparse/rte_argparse.c | 24
lib/argparse/rte_argparse.h | 5 +
3 files changed, 27 insertions(+), 20 deletions(-)
diff --git a/app/test/test_argparse.c b/app/test/test_argparse.c
index c98bcee56d..1a7211eb01 100644
--- a/app/test/test_argparse.c
+++ b/app/test/test_argparse.c
@@ -188,7 +188,7 @@ test_argparse_invalid_arg_help(void)
static int
test_argparse_invalid_has_val(void)
{
- uint32_t set_mask[] = { 0,
+ uint64_t set_mask[] = { 0,
RTE_ARGPARSE_ARG_NO_VALUE,
RTE_ARGPARSE_ARG_OPTIONAL_VALUE
};
@@ -197,7 +197,7 @@ test_argparse_invalid_has_val(void)
int ret;
obj = test_argparse_init_obj();
- obj->args[0].flags &= ~0x3u;
+ obj->args[0].flags &= ~RTE_ARGPARSE_HAS_VAL_BITMASK;
ret = rte_argparse_parse(obj, default_argc, default_argv);
TEST_ASSERT(ret == -EINVAL, "Argparse parse expect failed!");
@@ -205,7 +205,7 @@ test_argparse_invalid_has_val(void)
obj = test_argparse_init_obj();
obj->args[0].name_long = "abc";
obj->args[0].name_short = NULL;
- obj->args[0].flags &= ~0x3u;
+ obj->args[0].flags &= ~RTE_ARGPARSE_HAS_VAL_BITMASK;
obj->args[0].flags |= set_mask[index];
ret = rte_argparse_parse(obj, default_argc, default_argv);
TEST_ASSERT(ret == -EINVAL, "Argparse parse expect failed!");
@@ -269,7 +269,9 @@ test_argparse_invalid_arg_flags(void)
int ret;
obj = test_argparse_init_obj();
- obj->args[0].flags |= ~0x107FFu;
+ obj->args[0].flags |= ~(RTE_ARGPARSE_HAS_VAL_BITMASK |
+ RTE_ARGPARSE_VAL_TYPE_BITMASK |
+ RTE_ARGPARSE_ARG_SUPPORT_MULTI);
ret = rte_argparse_parse(obj, default_argc, default_argv);
TEST_ASSERT(ret == -EINVAL, "Argparse parse expect failed!");
@@ -337,7 +339,7 @@ test_argparse_invalid_option(void)
static int
test_argparse_opt_autosave_parse_int_of_no_val(void)
{
- uint32_t flags = RTE_ARGPARSE_ARG_NO_VALUE | RTE_ARGPARSE_ARG_VALUE_INT;
+ uint64_t flags = RTE_ARGPARSE_ARG_NO_VALUE | RTE_ARGPARSE_ARG_VALUE_INT;
struct rte_argparse *obj;
int val_saver = 0;
char *argv[2];
@@ -369,7 +371,7 @@ test_argparse_opt_autosave_parse_int_of_no_val(void)
static int
test_argparse_opt_autosave_parse_int_of_required_val(void)
{
- uint32_t flags = RTE_ARGPARSE_ARG_REQUIRED_VALUE |
RTE_ARGPARSE_ARG_VALUE_INT;
+ uint64_t flags = RTE_ARGPARSE_ARG_REQUIRED_VALUE |
RTE_ARGPARSE_ARG_VALUE_INT;
struct rte_argparse *obj;
int val_saver = 0;
char *argv[3];
@@ -410,7 +412,7 @@ test_argparse_opt_autosave_parse_int_of_required_val(void)
static int
test_argparse_opt_autosave_parse_int_of_optional_val(void)
{
- uint32_t flags = RTE_ARGPARSE_ARG_OPTIONAL_VALUE |
RTE_ARGPARSE_ARG_VALUE_INT;
+ uint64_t flags = RTE_ARGPARSE_ARG_OPTIONAL_VALUE |
RTE_ARGPARSE_ARG_VALUE_INT;
struct rte_argparse *obj;
int val_saver = 0;
char *argv[2];
@@ -645,7 +647,7 @@ test_argparse_opt_callback_parse_int_of_optional_val(void)
static int
test_argparse_pos_autosave_parse_int(void)
{
- uint32_t flags = RTE_ARGPARSE_ARG_REQUIRED_VALUE |
RTE_ARGPARSE_ARG_VALUE_INT;
+ uint64_t flags = RTE_ARGPARSE_ARG_REQUIRED_VALUE |
RTE_ARGPARSE_ARG_VALUE_INT;
struct rte_argparse *obj;
int val_saver = 0;
char *argv[3];
diff --git a/lib/argparse/rte_argparse.c b/lib/argparse/rte_argparse.c
index 6e890cdc0d..e7007afc6a 100644
--- a/lib/argparse/rte_argparse.c
+++ b/lib/argparse/rte_argparse.c
@@ -15,9 +15,6 @@ RTE_LOG_REGISTER_DEFAULT(rte_argparse_logtype, INFO);
#define ARGPARSE_LOG(level, ...) \
RTE_LOG_LINE(level, ARGPARSE, "" __VA_ARGS__)
-#define ARG_ATTR_HAS_VAL_MASK RTE_GENMASK64(1, 0)
-#define ARG_ATTR_VAL_TYPE_MASK RTE_GENMASK64(9, 2)
-#define ARG_ATTR_SUPPORT_MULTI_MASKRTE_BIT64(10)
#define ARG_ATTR_FLAG_PARSED_MASK RTE_BIT64(63)
static inline bool
@@ -35,26 +32,27 @@ is_arg_positional(const struct rte_argparse_arg *arg)
static inline uint32_t
arg_attr_has_val(const struct rte_argparse_arg *arg)
{
- return RTE_FIELD_GET64(ARG_ATTR_HAS_VAL_MASK, arg->flags);
+ return RTE_FIELD_GET64(RTE_ARGPARSE_HAS_VAL_BITMAS
[PATCH v4 6/6] argparse: fix doc don't display two hyphens
With the line in rst file:
The single mode: "--aaa" or "-a".
corresponding line in html doc:
The single mode: -aaa or -a.
the two hyphens (--aaa) become one (-aaa).
According to [1], this commit uses the backquote (``xxx``) to fix it.
And for consistency, use this format for all arguments.
Fixes: e3e579f5bab5 ("argparse: introduce argparse library")
[1] https://stackoverflow.com/questions/51075907/display-two-dashes-in-rst-file
Signed-off-by: Chengwen Feng
---
doc/guides/prog_guide/argparse_lib.rst | 47 +-
lib/argparse/rte_argparse.h| 4 +--
2 files changed, 26 insertions(+), 25 deletions(-)
diff --git a/doc/guides/prog_guide/argparse_lib.rst
b/doc/guides/prog_guide/argparse_lib.rst
index a6ac11b1c0..f827312daa 100644
--- a/doc/guides/prog_guide/argparse_lib.rst
+++ b/doc/guides/prog_guide/argparse_lib.rst
@@ -90,9 +90,9 @@ The following code demonstrates how to use:
}
In this example, the arguments which start with a hyphen (-) are optional
-arguments (they're "--aaa"/"--bbb"/"--ccc"/"--ddd"/"--eee"/"--fff"); and the
-arguments which don't start with a hyphen (-) are positional arguments
-(they're "ooo"/"ppp").
+arguments (they're
``--aaa``/``--bbb``/``--ccc``/``--ddd``/``--eee``/``--fff``);
+and the arguments which don't start with a hyphen (-) are positional arguments
+(they're ``ooo``/``ppp``).
Every argument must be set whether to carry a value (one of
``RTE_ARGPARSE_ARG_NO_VALUE``, ``RTE_ARGPARSE_ARG_REQUIRED_VALUE`` and
@@ -106,23 +106,23 @@ User Input Requirements
~~~
For optional arguments which take no-value,
-the following mode is supported (take above "--aaa" as an example):
+the following mode is supported (take above ``--aaa`` as an example):
-- The single mode: "--aaa" or "-a".
+- The single mode: ``--aaa`` or ``-a``.
For optional arguments which take required-value,
-the following two modes are supported (take above "--bbb" as an example):
+the following two modes are supported (take above ``--bbb`` as an example):
-- The kv mode: "--bbb=1234" or "-b=1234".
+- The kv mode: ``--bbb=1234`` or ``-b=1234``.
-- The split mode: "--bbb 1234" or "-b 1234".
+- The split mode: ``--bbb 1234`` or ``-b 1234``.
For optional arguments which take optional-value,
-the following two modes are supported (take above "--ccc" as an example):
+the following two modes are supported (take above ``--ccc`` as an example):
-- The single mode: "--ccc" or "-c".
+- The single mode: ``--ccc`` or ``-c``.
-- The kv mode: "--ccc=123" or "-c=123".
+- The kv mode: ``--ccc=123`` or ``-c=123``.
For positional arguments which must take required-value,
their values are parsing in the order defined.
@@ -130,7 +130,7 @@ their values are parsing in the order defined.
.. note::
The compact mode is not supported.
- Take above "-a" and "-d" as an example, don't support "-ad" input.
+ Take above ``-a`` and ``-d`` as an example, don't support ``-ad`` input.
Parsing by autosave way
~~~
@@ -139,23 +139,23 @@ Argument of known value type (e.g.
``RTE_ARGPARSE_ARG_VALUE_INT``)
could be parsed using this autosave way,
and its result will save in the ``val_saver`` field.
-In the above example, the arguments "--aaa"/"--bbb"/"--ccc" and "ooo"
+In the above example, the arguments ``--aaa``/``--bbb``/``--ccc`` and ``ooo``
both use this way, the parsing is as follows:
-- For argument "--aaa", it is configured as no-value,
+- For argument ``--aaa``, it is configured as no-value,
so the ``aaa_val`` will be set to ``val_set`` field
which is 100 in the above example.
-- For argument "--bbb", it is configured as required-value,
+- For argument ``--bbb``, it is configured as required-value,
so the ``bbb_val`` will be set to user input's value
- (e.g. will be set to 1234 with input "--bbb 1234").
+ (e.g. will be set to 1234 with input ``--bbb 1234``).
-- For argument "--ccc", it is configured as optional-value,
- if user only input "--ccc" then the ``ccc_val`` will be set to ``val_set``
field
- which is 200 in the above example;
- if user input "--ccc=123", then the ``ccc_val`` will be set to 123.
+- For argument ``--ccc``, it is configured as optional-value,
+ if user only input ``--ccc`` then the ``ccc_val`` will be set to ``val_set``
+ field which is 200 in the above example;
+ if user input ``--ccc=123``, then the ``ccc_val`` will be set to 123.
-- For argument "ooo", it is positional argument,
+- For argument ``ooo``, it is positional argument,
the ``ooo_val`` will be set to user input's value.
Parsing by callback way
@@ -165,7 +165,8 @@ It could also choose to use callback to parse,
just define a unique index for the argument
and make the ``val_save`` field to be NULL also zero value-type.
-In the above example, the arguments "--ddd"/"--eee"/"--fff" and "ppp" both use
this way.
+In the above example, the arguments ``--ddd``/``--eee``/``--fff`` and ``ppp``
+bo
Re: [PATCH v3 4/6] argparse: fix argument flags operate as uint32 type
Hi Thomas, On 2024/3/18 17:38, Thomas Monjalon wrote: > 18/03/2024 10:18, Chengwen Feng: >> --- a/app/test/test_argparse.c >> +++ b/app/test/test_argparse.c >> +#define TEST_ARGPARSE_FLAG_HAS_ARG_BITMASK RTE_SHIFT_VAL64(3, 0) >> +#define TEST_ARGPARSE_FLAG_VAL_TYPE_BITMASK RTE_SHIFT_VAL64(255, 2) > > Don't you want to make it part of the API? > I thought it was what you did in this v3. In v3 I defined two macros: RTE_ARGPARSE_HAS_VAL_BITMASK and RTE_ARGPARSE_VAL_TYPE_BITMASK in API file. but forgot to remove the above two TEST_ARGPARSE_XXX_BITMASK macros in v3. I already send v4 to fix it, please review it. Thanks > > > > . >
RE: [PATCH 01/13] net/mlx5/hws: move warn into debug level when needed
Hi,
> -Original Message-
> From: Itamar Gozlan
> Sent: Thursday, March 14, 2024 1:42 PM
> To: Itamar Gozlan ; Erez Shitrit ;
> Hamdan Agbariya ; Yevgeny Kliteynik
> ; Alex Vesker ; Slava Ovsiienko
> ; NBU-Contact-Thomas Monjalon (EXTERNAL)
> ; Dariusz Sosnowski ; Ori
> Kam ; Suanming Mou ; Matan
> Azrad ; Mark Bloch
> Cc: dev@dpdk.org; Maayan Kashani
> Subject: [PATCH 01/13] net/mlx5/hws: move warn into debug level when
> needed
>
> From: Erez Shitrit
>
> When the user tries to create a matcher and if failed with specific errno
> (E2BIG) the message will be in debug level and not in warning.
> It is a part of a feature when the user re-try to insert a new matching
> depends
> on that errno, no need the annoying message.
>
> Fixes: c55c2bf3533 ("net/mlx5/hws: net/mlx5/hws: add definer layer")
>
> Signed-off-by: Erez Shitrit
> Acked-by: Matan Azrad
Fixed Cc stable on several patches on this series, and reworded the commits
Series applied to next-net-mlx,
Kindest regards
Raslan Darawsheh
Re: [PATCH 01/13] net/mlx5/hws: move warn into debug level when needed
18/03/2024 13:56, Raslan Darawsheh:
> From: Itamar Gozlan
> > From: Erez Shitrit
> >
> > When the user tries to create a matcher and if failed with specific errno
> > (E2BIG) the message will be in debug level and not in warning.
> > It is a part of a feature when the user re-try to insert a new matching
> > depends
> > on that errno, no need the annoying message.
> >
> > Fixes: c55c2bf3533 ("net/mlx5/hws: net/mlx5/hws: add definer layer")
> >
> > Signed-off-by: Erez Shitrit
> > Acked-by: Matan Azrad
> Fixed Cc stable on several patches on this series, and reworded the commits
> Series applied to next-net-mlx,
There is no cover letter for this series,
so we are not able to understand how critical it is,
and what is the general intent.
Is it supposed to be integrated in the last week of 24.03 release cycle?
Re: [PATCH 0/3] support setting lanes
12/03/2024 08:52, Dengdui Huang: > Some speeds can be achieved with different number of lanes. For example, > 100Gbps can be achieved using two lanes of 50Gbps or four lanes of 25Gbps. > When use different lanes, the port cannot be up. I'm not sure what you are referring to. I suppose it is not PCI lanes. Please could you link to an explanation of how a port is split in lanes? Which hardware does this?
Re: dumpcap coredump for 82599 NIC
On Mon, 18 Mar 2024 10:48:03 +0800
"junwan...@cestc.cn" wrote:
> The issue indeed lies with the ixgbe driver. After making the following
> modifications, dpdk-dumpcap is now functioning properly.
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index
> d6cf00317e77b64f9822c155115f388ae62241eb..99b26f3c758b3c7ced5d59c6b27f305efe6cc33c
> 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -4301,48 +4301,50 @@ ixgbe_dev_link_update_share(struct rte_eth_dev *dev,
> wait = 1;
> #endif
>
> - if (vf)
> - diag = ixgbevf_check_link(hw, &link_speed, &link_up, wait);
> - else
> - diag = ixgbe_check_link(hw, &link_speed, &link_up, wait);
> + if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> + if (vf)
> + diag = ixgbevf_check_link(hw, &link_speed, &link_up,
> wait);
> + else
> + diag = ixgbe_check_link(hw, &link_speed, &link_up,
> wait);
>
> - if (diag != 0) {
> - link.link_speed = RTE_ETH_SPEED_NUM_100M;
> - link.link_duplex = RTE_ETH_LINK_FULL_DUPLEX;
> - return rte_eth_linkstatus_set(dev, &link);
> - }
> + if (diag != 0) {
> + link.link_speed = RTE_ETH_SPEED_NUM_100M;
> + link.link_duplex = RTE_ETH_LINK_FULL_DUPLEX;
> + return rte_eth_linkstatus_set(dev, &link);
> + }
> +
> + if (ixgbe_get_media_type(hw) == ixgbe_media_type_fiber &&
> + !ad->sdp3_no_tx_disable) {
> + esdp_reg = IXGBE_READ_REG(hw, IXGBE_ESDP);
> + if ((esdp_reg & IXGBE_ESDP_SDP3))
> + link_up = 0;
> + }
>
> - if (ixgbe_get_media_type(hw) == ixgbe_media_type_fiber &&
> - !ad->sdp3_no_tx_disable) {
> - esdp_reg = IXGBE_READ_REG(hw, IXGBE_ESDP);
> - if ((esdp_reg & IXGBE_ESDP_SDP3))
> - link_up = 0;
> - }
> -
> - if (link_up == 0) {
> - if (ixgbe_get_media_type(hw) == ixgbe_media_type_fiber) {
> - ixgbe_dev_wait_setup_link_complete(dev, 0);
> - /* NOTE: review for potential ordering optimization */
> - if (!__atomic_test_and_set(&ad->link_thread_running,
> __ATOMIC_SEQ_CST)) {
> - /* To avoid race condition between threads, set
> - * the IXGBE_FLAG_NEED_LINK_CONFIG flag only
> - * when there is no link thread running.
> - */
> - intr->flags |= IXGBE_FLAG_NEED_LINK_CONFIG;
> - if
> (rte_thread_create_internal_control(&ad->link_thread_tid,
> - "ixgbe-link",
> -
> ixgbe_dev_setup_link_thread_handler, dev) < 0) {
> + if (link_up == 0) {
> + if (ixgbe_get_media_type(hw) == ixgbe_media_type_fiber)
> {
> + ixgbe_dev_wait_setup_link_complete(dev, 0);
> + /* NOTE: review for potential ordering
> optimization */
> + if
> (!__atomic_test_and_set(&ad->link_thread_running, __ATOMIC_SEQ_CST)) {
> + /* To avoid race condition between
> threads, set
> + * the IXGBE_FLAG_NEED_LINK_CONFIG flag
> only
> + * when there is no link thread running.
> + */
> + intr->flags |=
> IXGBE_FLAG_NEED_LINK_CONFIG;
> + if
> (rte_thread_create_internal_control(&ad->link_thread_tid,
> + "ixgbe-link",
> +
> ixgbe_dev_setup_link_thread_handler, dev) < 0) {
> + PMD_DRV_LOG(ERR,
> + "Create link thread
> failed!");
> + /* NOTE: review for potential
> ordering optimization */
> +
> __atomic_clear(&ad->link_thread_running, __ATOMIC_SEQ_CST);
> + }
> + } else {
> PMD_DRV_LOG(ERR,
> - "Create link thread failed!");
> - /* NOTE: review for potential ordering
> optimization */
> -
> __atomic_clear(&ad->link_thread_running, __ATOMIC_SEQ_CST);
> + "Other link thread is running
> now!");
>
Re: Email based retest request process: proposal for new pull/re-apply feature
On Thu, Mar 7, 2024 at 12:06 PM Adam Hassick wrote: > > > I'm not opposed to having the contexts be a key-value pair argument > like the others, however that does break backwards compatibility with > our existing syntax. If we don't care very much about backwards > compatibility, then we could make this change. > > Instead of having a boolean and a string parameter for whether to > rebase and the branch to rebase on, we could have a single argument > specifying a branch. Then, labs rebase on the given branch and then > rerun all tests if the "rebase=" argument is present. This > would look like: > > Recheck-request: rebase=main, iol-sample-apps-testing, > iol-unit-amd64-testing, iol-broadcom-Performance I agree with this approach because it preserves backward compatibility, while still providing us with all the functionality we need. We will also be able to accept key value arguments in the future if further feature requests come in which require it. > I don't think the context should be required if the request includes > the rebase argument, because we do not want to mix valid and invalid > test results as Aaron said. > This would be a valid format if contexts are optional: > > Recheck-request: rebase=main Okay, I agree that contexts should not be considered by labs when we use rebase - but of course we will still store the contexts (if they are submitted) alongside the key value args. In the future there may be an application for this. Zhoumin, does this sound acceptable, or do you think there are any flaws? If it works, we will implement the updates and try to upstream this week. Thanks!
[PATCH v2 0/3] dts: error and usage improvements
Hello! Sending in v2 for my patch series, which changes a lot compared to v1. The main and big change was the reworking of the arguments handling, this can potentially be seen as a controversial change but I tried to explain it as much as I could in the commit body message. v2: - complete rework of the arguments handling, to retain the environment variables and gain control over them - prefixing 'Stderr: ' to RemoteCommandExecutionError - rebased Luca Vizzarro (3): dts: rework arguments framework dts: constrain DPDK source argument dts: store stderr in RemoteCommandExecutionError doc/guides/tools/dts.rst | 55 ++- dts/framework/exception.py| 13 +- .../remote_session/remote_session.py | 3 +- dts/framework/settings.py | 459 +- dts/framework/utils.py| 44 +- 5 files changed, 417 insertions(+), 157 deletions(-) -- 2.34.1
[PATCH v2 1/3] dts: rework arguments framework
The existing argument handling in the code relies on basic argparse functionality and a custom argparse action to integrate environment variables. This commit improves the current handling by augmenting argparse through a custom implementation of the arguments. This rework implements the following improvements: - There are duplicate expressions scattered throughout the code. To improve readability and maintainability, these are refactored into list/dict comprehensions or factory functions. - Arguments are augmented with their own class allowing for more flexibility, enabling manipulation while reducing redundancy. - Arguments are grouped within a new class. This class would track the origin of each argument, ensuring consistent input handling and facilitating debugging. - Instead of relying solely on argument flags, error messages now accurately reference environment variables when applicable, enhancing user experience. For instance: error: environment variable DTS_DPDK_TARBALL: Invalid file - Knowing the number of environment variables and arguments set allow for a useful help page display when none are provided. - A display of which environment variables have been detected and their corresponding values in the help page, aiding user awareness. Signed-off-by: Luca Vizzarro Reviewed-by: Paul Szczepanek Reviewed-by: Jack Bond-Preston --- doc/guides/tools/dts.rst | 53 +++-- dts/framework/settings.py | 393 -- 2 files changed, 324 insertions(+), 122 deletions(-) diff --git a/doc/guides/tools/dts.rst b/doc/guides/tools/dts.rst index 47b218b2c6..6993443389 100644 --- a/doc/guides/tools/dts.rst +++ b/doc/guides/tools/dts.rst @@ -215,30 +215,41 @@ DTS is run with ``main.py`` located in the ``dts`` directory after entering Poet .. code-block:: console (dts-py3.10) $ ./main.py --help - usage: main.py [-h] [--config-file CONFIG_FILE] [--output-dir OUTPUT_DIR] [-t TIMEOUT] [-v] [-s] [--tarball TARBALL] [--compile-timeout COMPILE_TIMEOUT] [--test-suite TEST_SUITE [TEST_CASES ...]] [--re-run RE_RUN] + usage: main.py [-h] [--config-file FILE_PATH] [--output-dir DIR_PATH] [-t SECONDS] [-v] [-s] [--tarball FILE_PATH] + [--compile-timeout SECONDS] [--test-suite TEST_SUITE [TEST_CASES ...]] [--re-run N_TIMES] - Run DPDK test suites. All options may be specified with the environment variables provided in brackets. Command line arguments have higher priority. + Run DPDK test suites. All options may be specified with the environment variables provided in brackets. Command + line arguments have higher priority. options: - -h, --helpshow this help message and exit - --config-file CONFIG_FILE - [DTS_CFG_FILE] The configuration file that describes the test cases, SUTs and targets. (default: ./conf.yaml) - --output-dir OUTPUT_DIR, --output OUTPUT_DIR - [DTS_OUTPUT_DIR] Output directory where DTS logs and results are saved. (default: output) - -t TIMEOUT, --timeout TIMEOUT - [DTS_TIMEOUT] The default timeout for all DTS operations except for compiling DPDK. (default: 15) - -v, --verbose [DTS_VERBOSE] Specify to enable verbose output, logging all messages to the console. (default: False) - -s, --skip-setup [DTS_SKIP_SETUP] Specify to skip all setup steps on SUT and TG nodes. (default: False) - --tarball TARBALL, --snapshot TARBALL, --git-ref TARBALL - [DTS_DPDK_TARBALL] Path to DPDK source code tarball or a git commit ID, tag ID or tree ID to test. To test local changes, first commit them, then use the commit ID with this option. (default: dpdk.tar.xz) - --compile-timeout COMPILE_TIMEOUT - [DTS_COMPILE_TIMEOUT] The timeout for compiling DPDK. (default: 1200) - --test-suite TEST_SUITE [TEST_CASES ...] - [DTS_TEST_SUITES] A list containing a test suite with test cases. The first parameter is the test suite name, and the rest are test case names, which are optional. May be specified multiple times. To specify multiple test suites in the environment - variable, join the lists with a comma. Examples: --test-suite suite case case --test-suite suite case ... | DTS_TEST_SUITES='suite case case, suite case, ...' | --test-suite suite --test-suite suite case ... | DTS_TEST_SUITES='suite, suite case, ...' - (default: []) - --re-run RE_RUN, --re_run RE_RUN - [DTS_RERUN] Re-run each test case the specified number of times if a test failure occurs. (default: 0) + -h, --helpshow this help message and exit + --config-file FILE_PATH + [DTS_CFG_FILE] The configuration file that describes the test cases, SUTs and targets. + (default: conf.yaml) + --output-dir DIR_PATH, --output DIR_PATH +
[PATCH v2 2/3] dts: constrain DPDK source argument
DTS needs an input to gather the DPDK source code from. This is then built on the remote target. This commit makes sure that this input is more constrained, separating the Git revision ID – used to create a tarball using Git – and providing tarballed source code directly, while retaining mutual exclusion. This makes the code more readable and easier to handle for input validation, of which this commit introduces a basic one based on the pre-existing code. Moreover it ensures that these flags are explicitly required to be set by the user, dropping a default value. Signed-off-by: Luca Vizzarro Reviewed-by: Paul Szczepanek Reviewed-by: Jack Bond-Preston --- doc/guides/tools/dts.rst | 14 +++--- dts/framework/settings.py | 90 --- dts/framework/utils.py| 43 +++ 3 files changed, 98 insertions(+), 49 deletions(-) diff --git a/doc/guides/tools/dts.rst b/doc/guides/tools/dts.rst index 6993443389..ea2bb34ddc 100644 --- a/doc/guides/tools/dts.rst +++ b/doc/guides/tools/dts.rst @@ -215,14 +215,20 @@ DTS is run with ``main.py`` located in the ``dts`` directory after entering Poet .. code-block:: console (dts-py3.10) $ ./main.py --help - usage: main.py [-h] [--config-file FILE_PATH] [--output-dir DIR_PATH] [-t SECONDS] [-v] [-s] [--tarball FILE_PATH] - [--compile-timeout SECONDS] [--test-suite TEST_SUITE [TEST_CASES ...]] [--re-run N_TIMES] + usage: main.py [-h] (--tarball FILE_PATH | --revision ID) [--config-file FILE_PATH] [--output-dir DIR_PATH] + [-t SECONDS] [-v] [-s] [--compile-timeout SECONDS] [--test-suite TEST_SUITE [TEST_CASES ...]] + [--re-run N_TIMES] Run DPDK test suites. All options may be specified with the environment variables provided in brackets. Command line arguments have higher priority. options: -h, --helpshow this help message and exit + --tarball FILE_PATH, --snapshot FILE_PATH + [DTS_DPDK_TARBALL] Path to DPDK source code tarball to test. (default: None) + --revision ID, --rev ID, --git-ref ID + [DTS_DPDK_REVISION_ID] Git revision ID to test. Could be commit, tag, tree ID etc. To test + local changes, first commit them, then use their commit ID. (default: None) --config-file FILE_PATH [DTS_CFG_FILE] The configuration file that describes the test cases, SUTs and targets. (default: conf.yaml) @@ -234,10 +240,6 @@ DTS is run with ``main.py`` located in the ``dts`` directory after entering Poet -v, --verbose [DTS_VERBOSE] Specify to enable verbose output, logging all messages to the console. (default: False) -s, --skip-setup [DTS_SKIP_SETUP] Specify to skip all setup steps on SUT and TG nodes. (default: False) - --tarball FILE_PATH, --snapshot FILE_PATH, --git-ref FILE_PATH - [DTS_DPDK_TARBALL] Path to DPDK source code tarball or a git commit ID,tag ID or tree ID to - test. To test local changes, first commit them, then use the commit ID with this option. - (default: dpdk.tar.xz) --compile-timeout SECONDS [DTS_COMPILE_TIMEOUT] The timeout for compiling DPDK. (default: 1200) --test-suite TEST_SUITE [TEST_CASES ...] diff --git a/dts/framework/settings.py b/dts/framework/settings.py index 421a9cb15b..ec238cea33 100644 --- a/dts/framework/settings.py +++ b/dts/framework/settings.py @@ -14,6 +14,17 @@ The command line arguments along with the supported environment variables are: +.. option:: --tarball, --snapshot +.. envvar:: DTS_DPDK_TARBALL + +Path to DPDK source code tarball to test. + +.. option:: --revision, --rev, --git-ref +.. envvar:: DTS_DPDK_REVISION_ID + +Git revision ID to test. Could be commit, tag, tree ID etc. +To test local changes, first commit them, then use their commit ID. + .. option:: --config-file .. envvar:: DTS_CFG_FILE @@ -44,11 +55,6 @@ Set to any value to skip building DPDK. -.. option:: --tarball, --snapshot, --git-ref -.. envvar:: DTS_DPDK_TARBALL - -The path to a DPDK tarball, git commit ID, tag ID or tree ID to test. - .. option:: --test-suite .. envvar:: DTS_TEST_SUITES @@ -79,8 +85,9 @@ from pathlib import Path from typing import Any, Generator, NamedTuple +from .exception import ConfigurationError from .config import TestSuiteConfig -from .utils import DPDKGitTarball +from .utils import DPDKGitTarball, get_commit_id #: The prefix to be added to all of the environment variables. @@ -88,6 +95,7 @@ DPDK_TARBALL_PATH_ARGUMENT_NAME = "dpdk_tarball_path" +DPDK_REVISION_ID_ARGUMENT_NAME = "dpdk_revision_id" CONFIG_FILE_ARGUMENT_NAME = "config_file" OUTPUT_DIR_ARGUMENT_NAME = "output_dir" TIMEOUT_ARGUMENT_NAME = "timeout" @@ -98,
[PATCH v2 3/3] dts: store stderr in RemoteCommandExecutionError
Store the stderr of an executed command in RemoteCommandExecutionError.
Consequently, when the exception is logged the error message includes
the stderr.
Signed-off-by: Luca Vizzarro
Reviewed-by: Paul Szczepanek
Reviewed-by: Jack Bond-Preston
---
dts/framework/exception.py | 13 ++---
dts/framework/remote_session/remote_session.py | 3 ++-
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/dts/framework/exception.py b/dts/framework/exception.py
index cce1e0231a..50724acdf2 100644
--- a/dts/framework/exception.py
+++ b/dts/framework/exception.py
@@ -2,6 +2,7 @@
# Copyright(c) 2010-2014 Intel Corporation
# Copyright(c) 2022-2023 PANTHEON.tech s.r.o.
# Copyright(c) 2022-2023 University of New Hampshire
+# Copyright(c) 2024 Arm Limited
"""DTS exceptions.
@@ -129,21 +130,27 @@ class RemoteCommandExecutionError(DTSError):
severity: ClassVar[ErrorSeverity] = ErrorSeverity.REMOTE_CMD_EXEC_ERR
#: The executed command.
command: str
+_command_stderr: str
_command_return_code: int
-def __init__(self, command: str, command_return_code: int):
+def __init__(self, command: str, command_return_code: int, command_stderr:
str):
"""Define the meaning of the first two arguments.
Args:
command: The executed command.
command_return_code: The return code of the executed command.
+command_stderr: The stderr of the executed command.
"""
self.command = command
self._command_return_code = command_return_code
+self._command_stderr = command_stderr
def __str__(self) -> str:
-"""Include both the command and return code in the string
representation."""
-return f"Command {self.command} returned a non-zero exit code:
{self._command_return_code}"
+"""Include the command, its return code and stderr in the string
representation."""
+return (
+f"Command '{self.command}' returned a non-zero exit code: "
+f"{self._command_return_code}\nStderr: {self._command_stderr}"
+)
class InteractiveCommandExecutionError(DTSError):
diff --git a/dts/framework/remote_session/remote_session.py
b/dts/framework/remote_session/remote_session.py
index ad0f53720a..9aaa8c8a04 100644
--- a/dts/framework/remote_session/remote_session.py
+++ b/dts/framework/remote_session/remote_session.py
@@ -2,6 +2,7 @@
# Copyright(c) 2010-2014 Intel Corporation
# Copyright(c) 2022-2023 PANTHEON.tech s.r.o.
# Copyright(c) 2022-2023 University of New Hampshire
+# Copyright(c) 2024 Arm Limited
"""Base remote session.
@@ -172,7 +173,7 @@ def send_command(
)
self._logger.debug(f"stdout: '{result.stdout}'")
self._logger.debug(f"stderr: '{result.stderr}'")
-raise RemoteCommandExecutionError(command, result.return_code)
+raise RemoteCommandExecutionError(command, result.return_code,
result.stderr)
self._logger.debug(f"Received from '{command}':\n{result}")
self.history.append(result)
return result
--
2.34.1
[PATCH v5 1/6] examples/l3fwd: fix lcore ID restriction
Currently the config option allows lcore IDs up to 255,
irrespective of RTE_MAX_LCORES and needs to be fixed.
The patch allows config options based on DPDK config.
Fixes: af75078fece3 ("first public release")
Cc: sta...@dpdk.org
Signed-off-by: Sivaprasad Tummala
Acked-by: Konstantin Ananyev
---
examples/l3fwd/l3fwd.h | 2 +-
examples/l3fwd/l3fwd_acl.c | 4 ++--
examples/l3fwd/l3fwd_em.c| 4 ++--
examples/l3fwd/l3fwd_event.h | 2 +-
examples/l3fwd/l3fwd_fib.c | 4 ++--
examples/l3fwd/l3fwd_lpm.c | 5 ++---
examples/l3fwd/main.c| 40
7 files changed, 32 insertions(+), 29 deletions(-)
diff --git a/examples/l3fwd/l3fwd.h b/examples/l3fwd/l3fwd.h
index e7ae0e5834..12c264cb4c 100644
--- a/examples/l3fwd/l3fwd.h
+++ b/examples/l3fwd/l3fwd.h
@@ -74,7 +74,7 @@ struct mbuf_table {
struct lcore_rx_queue {
uint16_t port_id;
- uint8_t queue_id;
+ uint16_t queue_id;
} __rte_cache_aligned;
struct lcore_conf {
diff --git a/examples/l3fwd/l3fwd_acl.c b/examples/l3fwd/l3fwd_acl.c
index 401692bcec..2bd63181bc 100644
--- a/examples/l3fwd/l3fwd_acl.c
+++ b/examples/l3fwd/l3fwd_acl.c
@@ -997,7 +997,7 @@ acl_main_loop(__rte_unused void *dummy)
uint64_t prev_tsc, diff_tsc, cur_tsc;
int i, nb_rx;
uint16_t portid;
- uint8_t queueid;
+ uint16_t queueid;
struct lcore_conf *qconf;
int socketid;
const uint64_t drain_tsc = (rte_get_tsc_hz() + US_PER_S - 1)
@@ -1020,7 +1020,7 @@ acl_main_loop(__rte_unused void *dummy)
portid = qconf->rx_queue_list[i].port_id;
queueid = qconf->rx_queue_list[i].queue_id;
RTE_LOG(INFO, L3FWD,
- " -- lcoreid=%u portid=%u rxqueueid=%hhu\n",
+ " -- lcoreid=%u portid=%u rxqueueid=%hu\n",
lcore_id, portid, queueid);
}
diff --git a/examples/l3fwd/l3fwd_em.c b/examples/l3fwd/l3fwd_em.c
index 40e102b38a..cd2bb4a4bb 100644
--- a/examples/l3fwd/l3fwd_em.c
+++ b/examples/l3fwd/l3fwd_em.c
@@ -586,7 +586,7 @@ em_main_loop(__rte_unused void *dummy)
unsigned lcore_id;
uint64_t prev_tsc, diff_tsc, cur_tsc;
int i, nb_rx;
- uint8_t queueid;
+ uint16_t queueid;
uint16_t portid;
struct lcore_conf *qconf;
const uint64_t drain_tsc = (rte_get_tsc_hz() + US_PER_S - 1) /
@@ -609,7 +609,7 @@ em_main_loop(__rte_unused void *dummy)
portid = qconf->rx_queue_list[i].port_id;
queueid = qconf->rx_queue_list[i].queue_id;
RTE_LOG(INFO, L3FWD,
- " -- lcoreid=%u portid=%u rxqueueid=%hhu\n",
+ " -- lcoreid=%u portid=%u rxqueueid=%hu\n",
lcore_id, portid, queueid);
}
diff --git a/examples/l3fwd/l3fwd_event.h b/examples/l3fwd/l3fwd_event.h
index 9aad358003..c6a4a89127 100644
--- a/examples/l3fwd/l3fwd_event.h
+++ b/examples/l3fwd/l3fwd_event.h
@@ -78,8 +78,8 @@ struct l3fwd_event_resources {
uint8_t deq_depth;
uint8_t has_burst;
uint8_t enabled;
- uint8_t eth_rx_queues;
uint8_t vector_enabled;
+ uint16_t eth_rx_queues;
uint16_t vector_size;
uint64_t vector_tmo_ns;
};
diff --git a/examples/l3fwd/l3fwd_fib.c b/examples/l3fwd/l3fwd_fib.c
index 6a21984415..7da55f707a 100644
--- a/examples/l3fwd/l3fwd_fib.c
+++ b/examples/l3fwd/l3fwd_fib.c
@@ -186,7 +186,7 @@ fib_main_loop(__rte_unused void *dummy)
uint64_t prev_tsc, diff_tsc, cur_tsc;
int i, nb_rx;
uint16_t portid;
- uint8_t queueid;
+ uint16_t queueid;
struct lcore_conf *qconf;
const uint64_t drain_tsc = (rte_get_tsc_hz() + US_PER_S - 1) /
US_PER_S * BURST_TX_DRAIN_US;
@@ -208,7 +208,7 @@ fib_main_loop(__rte_unused void *dummy)
portid = qconf->rx_queue_list[i].port_id;
queueid = qconf->rx_queue_list[i].queue_id;
RTE_LOG(INFO, L3FWD,
- " -- lcoreid=%u portid=%u rxqueueid=%hhu\n",
+ " -- lcoreid=%u portid=%u rxqueueid=%hu\n",
lcore_id, portid, queueid);
}
diff --git a/examples/l3fwd/l3fwd_lpm.c b/examples/l3fwd/l3fwd_lpm.c
index a484a33089..01d38bc69c 100644
--- a/examples/l3fwd/l3fwd_lpm.c
+++ b/examples/l3fwd/l3fwd_lpm.c
@@ -148,8 +148,7 @@ lpm_main_loop(__rte_unused void *dummy)
unsigned lcore_id;
uint64_t prev_tsc, diff_tsc, cur_tsc;
int i, nb_rx;
- uint16_t portid;
- uint8_t queueid;
+ uint16_t portid, queueid;
struct lcore_conf *qconf;
const uint64_t drain_tsc = (rte_get_tsc_hz() + US_PER_S - 1) /
US_PER_S * BURST_TX_DRAIN_US;
@@ -171,7 +170,7 @@ lpm_main_loop(__rte_unused void *dummy)
portid = qconf->rx_queue_list[i].port_id;
queueid = qconf->r
[PATCH v5 0/6] fix lcore ID restriction
With modern CPUs, it is possible to have higher CPU count thus we can have higher RTE_MAX_LCORES. In DPDK sample applications, the current config lcore options are hard limited to 255. The patchset fixes these constraints by allowing all lcore IDs up to RTE_MAX_LCORES. Also the queue IDs are increased to support up to 65535. v5: - updated lcore_id type to uint32_t v4: - fixed build errors with queue_id type in ipsec-secgw v3: - updated queue_id type to uint16_t v2: - fixed typo with lcore_id type in l3fwd Sivaprasad Tummala (6): examples/l3fwd: fix lcore ID restriction examples/l3fwd-power: fix lcore ID restriction examples/l3fwd-graph: fix lcore ID restriction examples/ipsec-secgw: fix lcore ID restriction examples/qos_sched: fix lcore ID restriction examples/vm_power_manager: fix lcore ID restriction examples/ipsec-secgw/event_helper.h | 2 +- examples/ipsec-secgw/ipsec-secgw.c| 35 +-- examples/ipsec-secgw/ipsec.c | 2 +- examples/ipsec-secgw/ipsec.h | 6 +- examples/ipsec-secgw/ipsec_worker.c | 10 ++-- examples/l3fwd-graph/main.c | 33 ++- examples/l3fwd-power/main.c | 59 +-- examples/l3fwd-power/main.h | 4 +- examples/l3fwd-power/perf_core.c | 16 +++-- examples/l3fwd/l3fwd.h| 2 +- examples/l3fwd/l3fwd_acl.c| 4 +- examples/l3fwd/l3fwd_em.c | 4 +- examples/l3fwd/l3fwd_event.h | 2 +- examples/l3fwd/l3fwd_fib.c| 4 +- examples/l3fwd/l3fwd_lpm.c| 5 +- examples/l3fwd/main.c | 40 +++-- examples/qos_sched/args.c | 6 +- .../guest_cli/vm_power_cli_guest.c| 4 +- 18 files changed, 122 insertions(+), 116 deletions(-) -- 2.25.1
[PATCH v5 2/6] examples/l3fwd-power: fix lcore ID restriction
Currently the config option allows lcore IDs up to 255,
irrespective of RTE_MAX_LCORES and needs to be fixed.
The patch allows config options based on DPDK config.
Fixes: f88e7c175a68 ("examples/l3fwd-power: add high/regular perf cores
options")
Cc: radu.nico...@intel.com
Cc: sta...@dpdk.org
Signed-off-by: Sivaprasad Tummala
---
examples/l3fwd-power/main.c | 59
examples/l3fwd-power/main.h | 4 +--
examples/l3fwd-power/perf_core.c | 16 +
3 files changed, 41 insertions(+), 38 deletions(-)
diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index f4adcf41b5..4430605df0 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -214,7 +214,7 @@ enum freq_scale_hint_t
struct lcore_rx_queue {
uint16_t port_id;
- uint8_t queue_id;
+ uint16_t queue_id;
enum freq_scale_hint_t freq_up_hint;
uint32_t zero_rx_packet_count;
uint32_t idle_hint;
@@ -838,7 +838,7 @@ sleep_until_rx_interrupt(int num, int lcore)
struct rte_epoll_event event[num];
int n, i;
uint16_t port_id;
- uint8_t queue_id;
+ uint16_t queue_id;
void *data;
if (status[lcore].wakeup) {
@@ -850,9 +850,9 @@ sleep_until_rx_interrupt(int num, int lcore)
n = rte_epoll_wait(RTE_EPOLL_PER_THREAD, event, num, 10);
for (i = 0; i < n; i++) {
data = event[i].epdata.data;
- port_id = ((uintptr_t)data) >> CHAR_BIT;
+ port_id = ((uintptr_t)data) >> (sizeof(uint16_t) * CHAR_BIT);
queue_id = ((uintptr_t)data) &
- RTE_LEN2MASK(CHAR_BIT, uint8_t);
+ RTE_LEN2MASK((sizeof(uint16_t) * CHAR_BIT), uint16_t);
RTE_LOG(INFO, L3FWD_POWER,
"lcore %u is waked up from rx interrupt on"
" port %d queue %d\n",
@@ -867,7 +867,7 @@ static void turn_on_off_intr(struct lcore_conf *qconf, bool
on)
{
int i;
struct lcore_rx_queue *rx_queue;
- uint8_t queue_id;
+ uint16_t queue_id;
uint16_t port_id;
for (i = 0; i < qconf->n_rx_queue; ++i) {
@@ -887,7 +887,7 @@ static void turn_on_off_intr(struct lcore_conf *qconf, bool
on)
static int event_register(struct lcore_conf *qconf)
{
struct lcore_rx_queue *rx_queue;
- uint8_t queueid;
+ uint16_t queueid;
uint16_t portid;
uint32_t data;
int ret;
@@ -897,7 +897,7 @@ static int event_register(struct lcore_conf *qconf)
rx_queue = &(qconf->rx_queue_list[i]);
portid = rx_queue->port_id;
queueid = rx_queue->queue_id;
- data = portid << CHAR_BIT | queueid;
+ data = portid << (sizeof(uint16_t) * CHAR_BIT) | queueid;
ret = rte_eth_dev_rx_intr_ctl_q(portid, queueid,
RTE_EPOLL_PER_THREAD,
@@ -917,8 +917,7 @@ static int main_intr_loop(__rte_unused void *dummy)
unsigned int lcore_id;
uint64_t prev_tsc, diff_tsc, cur_tsc;
int i, j, nb_rx;
- uint8_t queueid;
- uint16_t portid;
+ uint16_t portid, queueid;
struct lcore_conf *qconf;
struct lcore_rx_queue *rx_queue;
uint32_t lcore_rx_idle_count = 0;
@@ -946,7 +945,7 @@ static int main_intr_loop(__rte_unused void *dummy)
portid = qconf->rx_queue_list[i].port_id;
queueid = qconf->rx_queue_list[i].queue_id;
RTE_LOG(INFO, L3FWD_POWER,
- " -- lcoreid=%u portid=%u rxqueueid=%hhu\n",
+ " -- lcoreid=%u portid=%u rxqueueid=%hu\n",
lcore_id, portid, queueid);
}
@@ -1083,8 +1082,7 @@ main_telemetry_loop(__rte_unused void *dummy)
unsigned int lcore_id;
uint64_t prev_tsc, diff_tsc, cur_tsc, prev_tel_tsc;
int i, j, nb_rx;
- uint8_t queueid;
- uint16_t portid;
+ uint16_t portid, queueid;
struct lcore_conf *qconf;
struct lcore_rx_queue *rx_queue;
uint64_t ep_nep[2] = {0}, fp_nfp[2] = {0};
@@ -1114,7 +1112,7 @@ main_telemetry_loop(__rte_unused void *dummy)
portid = qconf->rx_queue_list[i].port_id;
queueid = qconf->rx_queue_list[i].queue_id;
RTE_LOG(INFO, L3FWD_POWER, " -- lcoreid=%u portid=%u "
- "rxqueueid=%hhu\n", lcore_id, portid, queueid);
+ "rxqueueid=%hu\n", lcore_id, portid, queueid);
}
while (!is_done()) {
@@ -1205,8 +1203,7 @@ main_legacy_loop(__rte_unused void *dummy)
uint64_t prev_tsc, diff_tsc, cur_tsc, tim_res_tsc, hz;
uint64_t prev_tsc_power = 0, cur_tsc_power, diff_tsc_power;
int i, j, nb_rx;
- uint8_t queueid;
- uint16_t portid;
+ uint16_t portid, queueid;
struct lcore_conf *qconf;
[PATCH v5 3/6] examples/l3fwd-graph: fix lcore ID restriction
Currently the config option allows lcore IDs up to 255,
irrespective of RTE_MAX_LCORES and needs to be fixed.
The patch allows config options based on DPDK config.
Fixes: 08bd1a174461 ("examples/l3fwd-graph: add graph-based l3fwd skeleton")
Cc: ndabilpu...@marvell.com
Cc: sta...@dpdk.org
Signed-off-by: Sivaprasad Tummala
---
examples/l3fwd-graph/main.c | 33 +
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/examples/l3fwd-graph/main.c b/examples/l3fwd-graph/main.c
index 96cb1c81ff..557ac6d823 100644
--- a/examples/l3fwd-graph/main.c
+++ b/examples/l3fwd-graph/main.c
@@ -90,7 +90,7 @@ static int pcap_trace_enable;
struct lcore_rx_queue {
uint16_t port_id;
- uint8_t queue_id;
+ uint16_t queue_id;
char node_name[RTE_NODE_NAMESIZE];
};
@@ -110,8 +110,8 @@ static struct lcore_conf lcore_conf[RTE_MAX_LCORE];
struct lcore_params {
uint16_t port_id;
- uint8_t queue_id;
- uint8_t lcore_id;
+ uint16_t queue_id;
+ uint32_t lcore_id;
} __rte_cache_aligned;
static struct lcore_params lcore_params_array[MAX_LCORE_PARAMS];
@@ -205,19 +205,19 @@ check_worker_model_params(void)
static int
check_lcore_params(void)
{
- uint8_t queue, lcore;
+ uint16_t queue, i;
int socketid;
- uint16_t i;
+ uint32_t lcore;
for (i = 0; i < nb_lcore_params; ++i) {
queue = lcore_params[i].queue_id;
if (queue >= MAX_RX_QUEUE_PER_PORT) {
- printf("Invalid queue number: %hhu\n", queue);
+ printf("Invalid queue number: %hu\n", queue);
return -1;
}
lcore = lcore_params[i].lcore_id;
if (!rte_lcore_is_enabled(lcore)) {
- printf("Error: lcore %hhu is not enabled in lcore
mask\n",
+ printf("Error: lcore %u is not enabled in lcore mask\n",
lcore);
return -1;
}
@@ -228,7 +228,7 @@ check_lcore_params(void)
}
socketid = rte_lcore_to_socket_id(lcore);
if ((socketid != 0) && (numa_on == 0)) {
- printf("Warning: lcore %hhu is on socket %d with numa
off\n",
+ printf("Warning: lcore %u is on socket %d with numa
off\n",
lcore, socketid);
}
}
@@ -257,7 +257,7 @@ check_port_config(void)
return 0;
}
-static uint8_t
+static uint16_t
get_port_n_rx_queues(const uint16_t port)
{
int queue = -1;
@@ -275,14 +275,14 @@ get_port_n_rx_queues(const uint16_t port)
}
}
- return (uint8_t)(++queue);
+ return (uint16_t)(++queue);
}
static int
init_lcore_rx_queues(void)
{
uint16_t i, nb_rx_queue;
- uint8_t lcore;
+ uint32_t lcore;
for (i = 0; i < nb_lcore_params; ++i) {
lcore = lcore_params[i].lcore_id;
@@ -290,7 +290,7 @@ init_lcore_rx_queues(void)
if (nb_rx_queue >= MAX_RX_QUEUE_PER_LCORE) {
printf("Error: too many queues (%u) for lcore: %u\n",
(unsigned int)nb_rx_queue + 1,
- (unsigned int)lcore);
+ lcore);
return -1;
}
@@ -448,11 +448,11 @@ parse_config(const char *q_arg)
}
lcore_params_array[nb_lcore_params].port_id =
- (uint8_t)int_fld[FLD_PORT];
+ (uint16_t)int_fld[FLD_PORT];
lcore_params_array[nb_lcore_params].queue_id =
- (uint8_t)int_fld[FLD_QUEUE];
+ (uint16_t)int_fld[FLD_QUEUE];
lcore_params_array[nb_lcore_params].lcore_id =
- (uint8_t)int_fld[FLD_LCORE];
+ (uint32_t)int_fld[FLD_LCORE];
++nb_lcore_params;
}
lcore_params = lcore_params_array;
@@ -1011,7 +1011,8 @@ main(int argc, char **argv)
"ethdev_tx-*",
"pkt_drop",
};
- uint8_t nb_rx_queue, queue, socketid;
+ uint8_t socketid;
+ uint16_t nb_rx_queue, queue;
struct rte_graph_param graph_conf;
struct rte_eth_dev_info dev_info;
uint32_t nb_ports, nb_conf = 0;
--
2.25.1
[PATCH v5 4/6] examples/ipsec-secgw: fix lcore ID restriction
Currently the config option allows lcore IDs up to 255,
irrespective of RTE_MAX_LCORES and needs to be fixed.
The patch allows config options based on DPDK config.
Fixes: d299106e8e31 ("examples/ipsec-secgw: add IPsec sample application")
Cc: sergio.gonzalez.mon...@intel.com
Cc: sta...@dpdk.org
Signed-off-by: Sivaprasad Tummala
Acked-by: Konstantin Ananyev
---
examples/ipsec-secgw/event_helper.h | 2 +-
examples/ipsec-secgw/ipsec-secgw.c | 37 +++--
examples/ipsec-secgw/ipsec.c| 2 +-
examples/ipsec-secgw/ipsec.h| 6 ++---
examples/ipsec-secgw/ipsec_worker.c | 10
5 files changed, 28 insertions(+), 29 deletions(-)
diff --git a/examples/ipsec-secgw/event_helper.h
b/examples/ipsec-secgw/event_helper.h
index dfb81bfcf1..be635685b4 100644
--- a/examples/ipsec-secgw/event_helper.h
+++ b/examples/ipsec-secgw/event_helper.h
@@ -102,7 +102,7 @@ struct eh_event_link_info {
/**< Event port ID */
uint8_t eventq_id;
/**< Event queue to be linked to the port */
- uint8_t lcore_id;
+ uint32_t lcore_id;
/**< Lcore to be polling on this port */
};
diff --git a/examples/ipsec-secgw/ipsec-secgw.c
b/examples/ipsec-secgw/ipsec-secgw.c
index 45a303850d..dc7491a2b9 100644
--- a/examples/ipsec-secgw/ipsec-secgw.c
+++ b/examples/ipsec-secgw/ipsec-secgw.c
@@ -220,8 +220,8 @@ static const char *cfgfile;
struct lcore_params {
uint16_t port_id;
- uint8_t queue_id;
- uint8_t lcore_id;
+ uint16_t queue_id;
+ uint32_t lcore_id;
} __rte_cache_aligned;
static struct lcore_params lcore_params_array[MAX_LCORE_PARAMS];
@@ -695,8 +695,7 @@ ipsec_poll_mode_worker(void)
struct rte_mbuf *pkts[MAX_PKT_BURST];
uint32_t lcore_id;
uint64_t prev_tsc, diff_tsc, cur_tsc;
- uint16_t i, nb_rx, portid;
- uint8_t queueid;
+ uint16_t i, nb_rx, portid, queueid;
struct lcore_conf *qconf;
int32_t rc, socket_id;
const uint64_t drain_tsc = (rte_get_tsc_hz() + US_PER_S - 1)
@@ -743,7 +742,7 @@ ipsec_poll_mode_worker(void)
portid = rxql[i].port_id;
queueid = rxql[i].queue_id;
RTE_LOG(INFO, IPSEC,
- " -- lcoreid=%u portid=%u rxqueueid=%hhu\n",
+ " -- lcoreid=%u portid=%u rxqueueid=%hu\n",
lcore_id, portid, queueid);
}
@@ -788,8 +787,7 @@ int
check_flow_params(uint16_t fdir_portid, uint8_t fdir_qid)
{
uint16_t i;
- uint16_t portid;
- uint8_t queueid;
+ uint16_t portid, queueid;
for (i = 0; i < nb_lcore_params; ++i) {
portid = lcore_params_array[i].port_id;
@@ -809,7 +807,7 @@ check_flow_params(uint16_t fdir_portid, uint8_t fdir_qid)
static int32_t
check_poll_mode_params(struct eh_conf *eh_conf)
{
- uint8_t lcore;
+ uint32_t lcore;
uint16_t portid;
uint16_t i;
int32_t socket_id;
@@ -828,13 +826,13 @@ check_poll_mode_params(struct eh_conf *eh_conf)
for (i = 0; i < nb_lcore_params; ++i) {
lcore = lcore_params[i].lcore_id;
if (!rte_lcore_is_enabled(lcore)) {
- printf("error: lcore %hhu is not enabled in "
+ printf("error: lcore %u is not enabled in "
"lcore mask\n", lcore);
return -1;
}
socket_id = rte_lcore_to_socket_id(lcore);
if (socket_id != 0 && numa_on == 0) {
- printf("warning: lcore %hhu is on socket %d "
+ printf("warning: lcore %u is on socket %d "
"with numa off\n",
lcore, socket_id);
}
@@ -851,7 +849,7 @@ check_poll_mode_params(struct eh_conf *eh_conf)
return 0;
}
-static uint8_t
+static uint16_t
get_port_nb_rx_queues(const uint16_t port)
{
int32_t queue = -1;
@@ -862,14 +860,14 @@ get_port_nb_rx_queues(const uint16_t port)
lcore_params[i].queue_id > queue)
queue = lcore_params[i].queue_id;
}
- return (uint8_t)(++queue);
+ return (uint16_t)(++queue);
}
static int32_t
init_lcore_rx_queues(void)
{
uint16_t i, nb_rx_queue;
- uint8_t lcore;
+ uint32_t lcore;
for (i = 0; i < nb_lcore_params; ++i) {
lcore = lcore_params[i].lcore_id;
@@ -1050,6 +1048,8 @@ parse_config(const char *q_arg)
char *str_fld[_NUM_FLD];
int32_t i;
uint32_t size;
+ uint32_t max_fld[_NUM_FLD] = {RTE_MAX_ETHPORTS,
+ USHRT_MAX, RTE_MAX_LCORE};
nb_lcore_params = 0;
@@ -1070,7 +1070,7 @@ parse_config(const char *q_arg)
for (i = 0; i < _NUM_FLD; i++) {
errno = 0;
[PATCH v5 6/6] examples/vm_power_manager: fix lcore ID restriction
Currently the config option allows lcore IDs up to 255,
irrespective of RTE_MAX_LCORES and needs to be fixed.
The patch allows config options based on DPDK config.
Fixes: 0e8f47491f09 ("examples/vm_power: add command to query CPU frequency")
Cc: marcinx.hajkow...@intel.com
Cc: sta...@dpdk.org
Signed-off-by: Sivaprasad Tummala
---
examples/vm_power_manager/guest_cli/vm_power_cli_guest.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
index 94bfbbaf78..5eddb47847 100644
--- a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
+++ b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
@@ -401,7 +401,7 @@ check_response_cmd(unsigned int lcore_id, int *result)
struct cmd_set_cpu_freq_result {
cmdline_fixed_string_t set_cpu_freq;
- uint8_t lcore_id;
+ uint32_t lcore_id;
cmdline_fixed_string_t cmd;
};
@@ -444,7 +444,7 @@ cmdline_parse_token_string_t cmd_set_cpu_freq =
set_cpu_freq, "set_cpu_freq");
cmdline_parse_token_num_t cmd_set_cpu_freq_core_num =
TOKEN_NUM_INITIALIZER(struct cmd_set_cpu_freq_result,
- lcore_id, RTE_UINT8);
+ lcore_id, RTE_UINT32);
cmdline_parse_token_string_t cmd_set_cpu_freq_cmd_cmd =
TOKEN_STRING_INITIALIZER(struct cmd_set_cpu_freq_result,
cmd, "up#down#min#max#enable_turbo#disable_turbo");
--
2.25.1
[PATCH v5 5/6] examples/qos_sched: fix lcore ID restriction
Currently the config option allows lcore IDs up to 255,
irrespective of RTE_MAX_LCORES and needs to be fixed.
The patch allows config options based on DPDK config.
Fixes: de3cfa2c9823 ("sched: initial import")
Cc: sta...@dpdk.org
Signed-off-by: Sivaprasad Tummala
---
examples/qos_sched/args.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/examples/qos_sched/args.c b/examples/qos_sched/args.c
index 8d61d3e454..886542b3c1 100644
--- a/examples/qos_sched/args.c
+++ b/examples/qos_sched/args.c
@@ -184,10 +184,10 @@ app_parse_flow_conf(const char *conf_str)
pconf->rx_port = vals[0];
pconf->tx_port = vals[1];
- pconf->rx_core = (uint8_t)vals[2];
- pconf->wt_core = (uint8_t)vals[3];
+ pconf->rx_core = vals[2];
+ pconf->wt_core = vals[3];
if (ret == 5)
- pconf->tx_core = (uint8_t)vals[4];
+ pconf->tx_core = vals[4];
else
pconf->tx_core = pconf->wt_core;
--
2.25.1
[PATCH v5 0/6] fix lcore ID restriction
With modern CPUs, it is possible to have higher CPU count thus we can have higher RTE_MAX_LCORES. In DPDK sample applications, the current config lcore options are hard limited to 255. The patchset fixes these constraints by allowing all lcore IDs up to RTE_MAX_LCORES. Also the queue IDs are increased to support up to 65535. v5: - updated lcore_id type to uint32_t v4: - fixed build errors with queue_id type in ipsec-secgw v3: - updated queue_id type to uint16_t v2: - fixed typo with lcore_id type in l3fwd Sivaprasad Tummala (6): examples/l3fwd: fix lcore ID restriction examples/l3fwd-power: fix lcore ID restriction examples/l3fwd-graph: fix lcore ID restriction examples/ipsec-secgw: fix lcore ID restriction examples/qos_sched: fix lcore ID restriction examples/vm_power_manager: fix lcore ID restriction examples/ipsec-secgw/event_helper.h | 2 +- examples/ipsec-secgw/ipsec-secgw.c| 35 +-- examples/ipsec-secgw/ipsec.c | 2 +- examples/ipsec-secgw/ipsec.h | 6 +- examples/ipsec-secgw/ipsec_worker.c | 10 ++-- examples/l3fwd-graph/main.c | 33 ++- examples/l3fwd-power/main.c | 59 +-- examples/l3fwd-power/main.h | 4 +- examples/l3fwd-power/perf_core.c | 16 +++-- examples/l3fwd/l3fwd.h| 2 +- examples/l3fwd/l3fwd_acl.c| 4 +- examples/l3fwd/l3fwd_em.c | 4 +- examples/l3fwd/l3fwd_event.h | 2 +- examples/l3fwd/l3fwd_fib.c| 4 +- examples/l3fwd/l3fwd_lpm.c| 5 +- examples/l3fwd/main.c | 40 +++-- examples/qos_sched/args.c | 6 +- .../guest_cli/vm_power_cli_guest.c| 4 +- 18 files changed, 122 insertions(+), 116 deletions(-) -- 2.25.1
[PATCH] net/mlx5: fix sync meter action
PMD implements sync METER flow action as async.
Queue selected for sync operations is `MLX5_HW_INV_QUEUE`.
That dummy queue value is translated into `CTRL_QUEUE_ID(priv)`.
Async job allocation converts INV queue into the real value, but
job release does not.
This patch fixes the queue value provided to `flow_hw_job_put()`.
This patch also removes dead code found in METER_MARK
destroy handler.
Coverity issue: tag: 415806: Memory - corruptions (OVERRUN)
Coverity issue: tag: 415804: Control flow issues (DEADCODE)
Fixes: 4359d9d1f76b ("net/mlx5: fix sync meter processing in HWS")
Signed-off-by: Gregory Etelson
Acked-by: Dariusz Sosnowski
---
drivers/net/mlx5/mlx5_flow_hw.c| 5 +
drivers/net/mlx5/mlx5_flow_meter.c | 2 +-
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 35f1ed7a03..9ebbe664d1 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -11494,10 +11494,7 @@ flow_hw_action_handle_destroy(struct rte_eth_dev *dev,
uint32_t queue,
NULL, "Unable to wait for ASO meter CQE");
break;
}
- if (!job)
- mlx5_ipool_free(pool->idx_pool, idx);
- else
- aso = true;
+ aso = true;
break;
case MLX5_INDIRECT_ACTION_TYPE_RSS:
ret = flow_dv_action_destroy(dev, handle, error);
diff --git a/drivers/net/mlx5/mlx5_flow_meter.c
b/drivers/net/mlx5/mlx5_flow_meter.c
index 4045c4c249..ca361f7efa 100644
--- a/drivers/net/mlx5/mlx5_flow_meter.c
+++ b/drivers/net/mlx5/mlx5_flow_meter.c
@@ -2265,7 +2265,7 @@ mlx5_flow_meter_hws_create(struct rte_eth_dev *dev,
uint32_t meter_id,
ret = mlx5_aso_meter_update_by_wqe(priv, MLX5_HW_INV_QUEUE, aso_mtr,
&priv->mtr_bulk, job, true);
if (ret) {
- flow_hw_job_put(priv, job, MLX5_HW_INV_QUEUE);
+ flow_hw_job_put(priv, job, CTRL_QUEUE_ID(priv));
return -rte_mtr_error_set(error, ENOTSUP,
RTE_MTR_ERROR_TYPE_UNSPECIFIED,
NULL, "Failed to create devx meter.");
--
2.39.2
[PATCH] doc: add dma perf feature details
Update dma perf test document with below support features: 1. Memory-to-device and device-to-memory copy. 2. Skip support. 3. Scatter-gather support. Signed-off-by: Amit Prakash Shukla --- doc/guides/tools/dmaperf.rst | 89 ++-- 1 file changed, 64 insertions(+), 25 deletions(-) diff --git a/doc/guides/tools/dmaperf.rst b/doc/guides/tools/dmaperf.rst index 9e3e78a6b7..4a5702a628 100644 --- a/doc/guides/tools/dmaperf.rst +++ b/doc/guides/tools/dmaperf.rst @@ -5,27 +5,23 @@ dpdk-test-dma-perf Application == The ``dpdk-test-dma-perf`` tool is a Data Plane Development Kit (DPDK) application -that enables testing the performance of DMA (Direct Memory Access) devices available within DPDK. -It provides a test framework to assess the performance of CPU and DMA devices -under various scenarios, such as varying buffer lengths. -Doing so provides insight into the potential performance -when using these DMA devices for acceleration in DPDK applications. +that evaluates the performance of DMA (Direct Memory Access) devices accessible in DPDK environment. +It provides a benchmark framework to assess the performance of CPU and DMA devices +under various combinations, such as varying buffer lengths, scatter-gather copy, copying in remote +memory etc. It helps in evaluating performance of DMA device as hardware acceleration vehicle in +DPDK application. -It supports memory copy performance tests for now, -comparing the performance of CPU and DMA automatically in various conditions -with the help of a pre-set configuration file. +In addition, this tool supports memory-to-memory, memory-to-device and device-to-memory copy tests, +to compare the performance of CPU and DMA capabilities under various conditions with the help of a +pre-set configuration file. Configuration - -This application uses inherent DPDK EAL command-line options -as well as custom command-line options in the application. -An example configuration file for the application is provided -and gives the meanings for each parameter. - -Here is an extracted sample from the configuration file -(the complete sample can be found in the application source directory): +Along with EAL command-line arguments, this application supports various parameters for the +benchmarking through a configuration file. An example configuration file is provided below along +with the application to demonstrate all the parameters. .. code-block:: ini @@ -53,14 +49,35 @@ Here is an extracted sample from the configuration file lcore = 3, 4 eal_args=--in-memory --no-pci + [case3] + skip=1 + type=DMA_MEM_COPY + direction=mem2dev + vchan_dev=raddr=0x2,coreid=1,pfid=2,vfid=3 + dma_src_sge=4 + dma_dst_sge=1 + mem_size=10 + buf_size=64,8192,2,MUL + dma_ring_size=1024 + kick_batch=32 + src_numa_node=0 + dst_numa_node=0 + cache_flush=0 + test_seconds=2 + lcore_dma=lcore10@:00:04.2, lcore11@:00:04.3 + eal_args=--in-memory --file-prefix=test + The configuration file is divided into multiple sections, each section represents a test case. -The four variables ``mem_size``, ``buf_size``, ``dma_ring_size``, and ``kick_batch`` -can vary in each test case. -The format for this is ``variable=first,last,increment,ADD|MUL``. -This means that the first value of the variable is 'first', -the last value is 'last', -'increment' is the step size, -and 'ADD|MUL' indicates whether the change is by addition or multiplication. +The four mandatory variables ``mem_size``, ``buf_size``, ``dma_ring_size``, and ``kick_batch`` +can vary in each test case. The format for this is ``variable=first,last,increment,ADD|MUL``. +This means that the first value of the variable is 'first', the last value is 'last', +'increment' is the step size, and 'ADD|MUL' indicates whether the change is by addition or +multiplication. + +The variables for mem2dev and dev2mem copy are ``direction``, ``vchan_dev`` and can vary in each +test case. If the direction is not configured, the default is mem2mem copy. + +For scatter-gather copy test ``dma_src_sge``, ``dma_dst_sge`` must be configured. Each case can only have one variable change, and each change will generate a scenario, so each case can have multiple scenarios. @@ -69,10 +86,32 @@ and each change will generate a scenario, so each case can have multiple scenari Configuration Parameters +``skip`` + To skip a test-case, must be configured as ``1`` + ``type`` The type of the test. Currently supported types are ``DMA_MEM_COPY`` and ``CPU_MEM_COPY``. +``direction`` + The direction of data transfer. + Currently supported directions: + +* ``mem2mem`` - memory to memory copy + +* ``mem2dev`` - memory to device copy + +* ``dev2mem`` - device to memory copy + +``vchan_dev`` + Comma separated bus related parameters for ``mem2dev`` and ``dev2mem`` copy. + +``dma_src_sge``
[PATCH v8 0/5] Logging timestamp and related patches
Improvements and unification of logging library (for 24.07 release).
This is update to earlier patch set.
v8 - rebase to current code base where logging in in lib/log
use stdio for log timestamp
initialization changes (setup log earlier)
Stephen Hemminger (5):
log: unify logging code
eal: make eal_log_level_parse common
eal: allow user to set default log stream before init
eal: add option to put timestamp on console output
eal: initialize logging before plugins
app/dumpcap/main.c| 3 +
app/pdump/main.c | 3 +
app/proc-info/main.c | 3 +
app/test/test_eal_flags.c | 9 +++
doc/guides/linux_gsg/linux_eal_parameters.rst | 27 -
doc/guides/prog_guide/log_lib.rst | 28 +-
lib/eal/common/eal_common_options.c | 56 ++-
lib/eal/common/eal_options.h | 3 +
lib/eal/freebsd/eal.c | 50 +++--
lib/eal/linux/eal.c | 55 +++---
lib/eal/unix/eal_unix_log.c | 0
lib/eal/windows/eal.c | 35
lib/log/log.c | 6 ++
lib/log/log_freebsd.c | 12
lib/log/log_internal.h| 11
lib/log/{log_linux.c => log_unix.c} | 36 +++-
lib/log/log_windows.c | 6 ++
lib/log/meson.build | 12 ++--
lib/log/version.map | 2 +
19 files changed, 184 insertions(+), 173 deletions(-)
create mode 100644 lib/eal/unix/eal_unix_log.c
delete mode 100644 lib/log/log_freebsd.c
rename lib/log/{log_linux.c => log_unix.c} (58%)
--
2.43.0
[PATCH v8 1/5] log: unify logging code
FreeBSD and Linux logging code can use common code. This also
fixes FreeBSD not using syslog.
Signed-off-by: Stephen Hemminger
---
doc/guides/linux_gsg/linux_eal_parameters.rst | 27 ---
doc/guides/prog_guide/log_lib.rst | 18 +++--
lib/eal/freebsd/eal.c | 8 ++
lib/log/log_freebsd.c | 12 -
lib/log/{log_linux.c => log_unix.c} | 0
lib/log/meson.build | 12 ++---
6 files changed, 32 insertions(+), 45 deletions(-)
delete mode 100644 lib/log/log_freebsd.c
rename lib/log/{log_linux.c => log_unix.c} (100%)
diff --git a/doc/guides/linux_gsg/linux_eal_parameters.rst
b/doc/guides/linux_gsg/linux_eal_parameters.rst
index ea8f38139119..d86f94d8a85d 100644
--- a/doc/guides/linux_gsg/linux_eal_parameters.rst
+++ b/doc/guides/linux_gsg/linux_eal_parameters.rst
@@ -108,30 +108,3 @@ Memory-related options
* ``--match-allocations``
Free hugepages back to system exactly as they were originally allocated.
-
-Other options
-~
-
-* ``--syslog ``
-
-Set syslog facility. Valid syslog facilities are::
-
-auth
-cron
-daemon
-ftp
-kern
-lpr
-mail
-news
-syslog
-user
-uucp
-local0
-local1
-local2
-local3
-local4
-local5
-local6
-local7
diff --git a/doc/guides/prog_guide/log_lib.rst
b/doc/guides/prog_guide/log_lib.rst
index ff9d1b54a2c8..aacb36c36ce0 100644
--- a/doc/guides/prog_guide/log_lib.rst
+++ b/doc/guides/prog_guide/log_lib.rst
@@ -5,8 +5,8 @@ Log Library
===
The DPDK Log library provides the logging functionality for other DPDK
libraries and drivers.
-By default, in a Linux application, logs are sent to syslog and also to the
console.
-On FreeBSD and Windows applications, logs are sent only to the console.
+By default, in a Linux (or FreeBSD) application, logs are sent to syslog and
also to the console.
+In Windows applications, logs are sent only to the console.
However, the log function can be overridden by the user to use a different
logging mechanism.
Log Levels
@@ -29,6 +29,7 @@ will be emitted by the application to the log output.
That level can be configured either by the application calling the relevant
APIs from the logging library,
or by the user passing the ``--log-level`` parameter to the EAL via the
application.
+
Setting Global Log Level
@@ -59,6 +60,19 @@ For example::
Within an application, the same result can be got using the
``rte_log_set_level_pattern()`` or ``rte_log_set_level_regex()`` APIs.
+
+Setting syslog facility
+~~~
+
+On Linux and FreeBSD, where syslog is used a ``facility`` argument can be
+used to specify what type of program is logging.
+The default facility is ``daemon`` but it can be overridden
+by the ``--syslog`` EAL parameter. See ``syslog.3`` man page for full values.
+For example::
+
+ /path/to/app --syslog local0
+
+
Using Logging APIs to Generate Log Messages
---
diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
index bab77118e967..004b8fad2db3 100644
--- a/lib/eal/freebsd/eal.c
+++ b/lib/eal/freebsd/eal.c
@@ -760,6 +760,14 @@ rte_eal_init(int argc, char **argv)
#endif
}
+ if (eal_log_init(program_invocation_short_name,
+internal_conf->syslog_facility) < 0) {
+ rte_eal_init_alert("Cannot init logging.");
+ rte_errno = ENOMEM;
+ rte_atomic_store_explicit(&run_once, 0,
rte_memory_order_relaxed);
+ return -1;
+ }
+
/* in secondary processes, memory init may allocate additional fbarrays
* not present in primary processes, so to avoid any potential issues,
* initialize memzones first.
diff --git a/lib/log/log_freebsd.c b/lib/log/log_freebsd.c
deleted file mode 100644
index 698d3c542337..
--- a/lib/log/log_freebsd.c
+++ /dev/null
@@ -1,12 +0,0 @@
-/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2023 Intel Corporation
- */
-
-#include
-#include "log_internal.h"
-
-int
-eal_log_init(__rte_unused const char *id, __rte_unused int facility)
-{
- return 0;
-}
diff --git a/lib/log/log_linux.c b/lib/log/log_unix.c
similarity index 100%
rename from lib/log/log_linux.c
rename to lib/log/log_unix.c
diff --git a/lib/log/meson.build b/lib/log/meson.build
index 0d4319b36f77..60516a0b2a2d 100644
--- a/lib/log/meson.build
+++ b/lib/log/meson.build
@@ -2,8 +2,12 @@
# Copyright(c) 2023 Intel Corporation
includes += global_inc
-sources = files(
-'log.c',
-'log_' + exec_env + '.c',
-)
+sources = files('log.c')
+
+if is_windows
+sources += files('log_windows.c')
+else
+sources += files('log_unix.c')
+endif
+
headers = files('rte_log.h')
--
2.43.0
[PATCH v8 2/5] eal: make eal_log_level_parse common
The code to parse for log-level option should be same on
all OS variants.
Signed-off-by: Stephen Hemminger
---
lib/eal/common/eal_common_options.c | 46 +
lib/eal/common/eal_options.h| 1 +
lib/eal/freebsd/eal.c | 42 --
lib/eal/linux/eal.c | 39
lib/eal/windows/eal.c | 35 --
5 files changed, 47 insertions(+), 116 deletions(-)
diff --git a/lib/eal/common/eal_common_options.c
b/lib/eal/common/eal_common_options.c
index e541f0793964..7310d10dfd78 100644
--- a/lib/eal/common/eal_common_options.c
+++ b/lib/eal/common/eal_common_options.c
@@ -1640,6 +1640,51 @@ eal_parse_huge_unlink(const char *arg, struct
hugepage_file_discipline *out)
return -1;
}
+/* Parse the all arguments looking for --log-level */
+int
+eal_log_level_parse(int argc, char * const argv[])
+{
+ struct internal_config *internal_conf =
eal_get_internal_configuration();
+ int option_index, opt;
+ const int old_optind = optind;
+ const int old_optopt = optopt;
+ const int old_opterr = opterr;
+ char *old_optarg = optarg;
+#ifdef RTE_EXEC_ENV_FREEBSD
+ const int old_optreset = optreset;
+ optreset = 1;
+#endif
+
+ optind = 1;
+ opterr = 0;
+
+ while ((opt = getopt_long(argc, argv, eal_short_options,
+ eal_long_options, &option_index)) != EOF) {
+
+ switch (opt) {
+ case OPT_LOG_LEVEL_NUM:
+ if (eal_parse_common_option(opt, optarg, internal_conf)
< 0)
+ return -1;
+ break;
+ case '?':
+ /* getopt is not happy, stop right now */
+ goto out;
+ default:
+ continue;
+ }
+ }
+out:
+ /* restore getopt lib */
+ optind = old_optind;
+ optopt = old_optopt;
+ optarg = old_optarg;
+ opterr = old_opterr;
+#ifdef RTE_EXEC_ENV_FREEBSD
+ optreset = old_optreset;
+#endif
+ return 0;
+}
+
int
eal_parse_common_option(int opt, const char *optarg,
struct internal_config *conf)
@@ -2173,6 +2218,7 @@ rte_vect_set_max_simd_bitwidth(uint16_t bitwidth)
return 0;
}
+
void
eal_common_usage(void)
{
diff --git a/lib/eal/common/eal_options.h b/lib/eal/common/eal_options.h
index 3cc9cb641284..f3f2e104f6d7 100644
--- a/lib/eal/common/eal_options.h
+++ b/lib/eal/common/eal_options.h
@@ -96,6 +96,7 @@ enum {
extern const char eal_short_options[];
extern const struct option eal_long_options[];
+int eal_log_level_parse(int argc, char * const argv[]);
int eal_parse_common_option(int opt, const char *argv,
struct internal_config *conf);
int eal_option_device_parse(void);
diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
index 004b8fad2db3..b4f8d68b0a65 100644
--- a/lib/eal/freebsd/eal.c
+++ b/lib/eal/freebsd/eal.c
@@ -363,48 +363,6 @@ eal_get_hugepage_mem_size(void)
return (size < SIZE_MAX) ? (size_t)(size) : SIZE_MAX;
}
-/* Parse the arguments for --log-level only */
-static void
-eal_log_level_parse(int argc, char **argv)
-{
- int opt;
- char **argvopt;
- int option_index;
- const int old_optind = optind;
- const int old_optopt = optopt;
- const int old_optreset = optreset;
- char * const old_optarg = optarg;
- struct internal_config *internal_conf =
- eal_get_internal_configuration();
-
- argvopt = argv;
- optind = 1;
- optreset = 1;
-
- while ((opt = getopt_long(argc, argvopt, eal_short_options,
- eal_long_options, &option_index)) != EOF) {
-
- int ret;
-
- /* getopt is not happy, stop right now */
- if (opt == '?')
- break;
-
- ret = (opt == OPT_LOG_LEVEL_NUM) ?
- eal_parse_common_option(opt, optarg, internal_conf) : 0;
-
- /* common parser is not happy */
- if (ret < 0)
- break;
- }
-
- /* restore getopt lib */
- optind = old_optind;
- optopt = old_optopt;
- optreset = old_optreset;
- optarg = old_optarg;
-}
-
/* Parse the argument given in the command line of the application */
static int
eal_parse_args(int argc, char **argv)
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index fd422f1f6236..bffeb1f34eb9 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -546,45 +546,6 @@ eal_parse_vfio_vf_token(const char *vf_token)
return -1;
}
-/* Parse the arguments for --log-level only */
-static void
-eal_log_level_parse(int argc, char **argv)
-{
- int opt;
- char **argvopt;
- int option_index;
- const int old_optind = optind;
-
[PATCH v8 3/5] eal: allow user to set default log stream before init
It is useful for application to be able to set the default log
stream before call rte_eal_init(). This makes all messages go
to the new default.
For example, to skip using syslog; just doing
rte_openlog_stream(stderr);
There is no reason for helper command line applications to clutter
syslog with messages.
Signed-off-by: Stephen Hemminger
---
app/dumpcap/main.c | 3 +++
app/pdump/main.c | 3 +++
app/proc-info/main.c | 3 +++
lib/log/log.c | 6 ++
lib/log/log_internal.h | 2 ++
lib/log/log_unix.c | 4
lib/log/version.map| 1 +
7 files changed, 22 insertions(+)
diff --git a/app/dumpcap/main.c b/app/dumpcap/main.c
index cc0f66b2bc61..27934ca7e688 100644
--- a/app/dumpcap/main.c
+++ b/app/dumpcap/main.c
@@ -633,6 +633,9 @@ static void dpdk_init(void)
rte_panic("No memory\n");
}
+ /* keep any logging away from syslog. */
+ rte_openlog_stream(stderr);
+
if (rte_eal_init(eal_argc, eal_argv) < 0)
rte_exit(EXIT_FAILURE, "EAL init failed: is primary process
running?\n");
}
diff --git a/app/pdump/main.c b/app/pdump/main.c
index a9205e130bb1..7b9ba68b1a14 100644
--- a/app/pdump/main.c
+++ b/app/pdump/main.c
@@ -995,6 +995,9 @@ main(int argc, char **argv)
argc += 2;
+ /* keep any logging away from syslog. */
+ rte_openlog_stream(stderr);
+
diag = rte_eal_init(argc, argp);
if (diag < 0)
rte_panic("Cannot init EAL\n");
diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index b672aaefbe99..24ee52c4ac7a 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -2149,6 +2149,9 @@ main(int argc, char **argv)
argc += 4;
+ /* keep any logging away from syslog. */
+ rte_openlog_stream(stderr);
+
ret = rte_eal_init(argc, argp);
if (ret < 0)
rte_panic("Cannot init EAL\n");
diff --git a/lib/log/log.c b/lib/log/log.c
index 255f757d94cc..4cc944305057 100644
--- a/lib/log/log.c
+++ b/lib/log/log.c
@@ -519,6 +519,12 @@ eal_log_set_default(FILE *default_log)
#endif
}
+FILE *
+eal_log_get_default(void)
+{
+ return default_log_stream;
+}
+
/*
* Called by eal_cleanup
*/
diff --git a/lib/log/log_internal.h b/lib/log/log_internal.h
index 451629f1c1ba..c77e687e28bc 100644
--- a/lib/log/log_internal.h
+++ b/lib/log/log_internal.h
@@ -21,6 +21,8 @@ int eal_log_init(const char *id, int facility);
*/
__rte_internal
void eal_log_set_default(FILE *default_log);
+__rte_internal
+FILE *eal_log_get_default(void);
/*
* Save a log option for later.
diff --git a/lib/log/log_unix.c b/lib/log/log_unix.c
index 2dfb0c974b1d..a415bae5774d 100644
--- a/lib/log/log_unix.c
+++ b/lib/log/log_unix.c
@@ -49,6 +49,10 @@ eal_log_init(const char *id, int facility)
{
FILE *log_stream;
+ /* skip if user has already setup a log stream */
+ if (eal_log_get_default())
+ return 0;
+
log_stream = fopencookie(NULL, "w+", console_log_func);
if (log_stream == NULL)
return -1;
diff --git a/lib/log/version.map b/lib/log/version.map
index 0648f8831aff..6ecc656d1d65 100644
--- a/lib/log/version.map
+++ b/lib/log/version.map
@@ -25,6 +25,7 @@ DPDK_24 {
INTERNAL {
global:
+ eal_log_get_default;
eal_log_init;
eal_log_level2str;
eal_log_save_pattern;
--
2.43.0
[PATCH v8 4/5] eal: add option to put timestamp on console output
When debugging driver or startup issues, it is useful to have
a timestamp on each message printed. The messages in syslog
already have a timestamp, but often syslog is not available
during testing. The timestamp format is chosen to look
like the default Linux dmesg timestamp.
The first few lines are not timestamped because the flag is stored
in internal configuration which is stored in shared memory
which is not setup up until a little later in startup process.
This logging skips the unnecessary step of going through stdio,
which makes it more robust against being called in interrupt
handlers etc.
Example:
$ dpdk-testpmd --log-timestamp -- -i
EAL: Detected CPU lcores: 16
EAL: Detected NUMA nodes: 1
EAL: Detected static linkage of DPDK
EAL: Multi-process socket /var/run/dpdk/rte/mp_socket
EAL: Selected IOVA mode 'VA'
[ 0.112264] testpmd: No probed ethernet devices
Interactive-mode selected
[ 0.184573] testpmd: create a new mbuf pool : n=163456,
size=2176, socket=0
[ 0.184612] testpmd: preferred mempool ops selected: ring_mp_mc
Signed-off-by: Stephen Hemminger
---
app/test/test_eal_flags.c | 9
doc/guides/prog_guide/log_lib.rst | 10 +
lib/eal/common/eal_common_options.c | 10 +++--
lib/eal/common/eal_options.h| 2 ++
lib/log/log_internal.h | 9
lib/log/log_unix.c | 32 +++--
lib/log/log_windows.c | 6 ++
lib/log/version.map | 1 +
8 files changed, 75 insertions(+), 4 deletions(-)
diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c
index 6cb4b0675730..07a038fb6051 100644
--- a/app/test/test_eal_flags.c
+++ b/app/test/test_eal_flags.c
@@ -1055,6 +1055,10 @@ test_misc_flags(void)
const char * const argv22[] = {prgname, prefix, mp_flag,
"--huge-worker-stack=512"};
+ /* Try running with --log-timestamp */
+ const char * const argv23[] = {prgname, prefix, mp_flag,
+ "--log-timestamp" };
+
/* run all tests also applicable to FreeBSD first */
if (launch_proc(argv0) == 0) {
@@ -1162,6 +1166,11 @@ test_misc_flags(void)
printf("Error - process did not run ok with
--huge-worker-stack=size parameter\n");
goto fail;
}
+ if (launch_proc(argv23) != 0) {
+ printf("Error - process did not run ok with --log-timestamp
parameter\n");
+ goto fail;
+ }
+
rmdir(hugepath_dir3);
rmdir(hugepath_dir2);
diff --git a/doc/guides/prog_guide/log_lib.rst
b/doc/guides/prog_guide/log_lib.rst
index aacb36c36ce0..1d6b2e3cea5d 100644
--- a/doc/guides/prog_guide/log_lib.rst
+++ b/doc/guides/prog_guide/log_lib.rst
@@ -73,6 +73,16 @@ For example::
/path/to/app --syslog local0
+Console timestamp
+~
+
+On Linux and FreeBSD, an optional timestamp can be added before each
+message by adding the ``--log-timestamp`` option.
+For example::
+
+ /path/to/app --log-level=lib.*:debug --log-timestamp
+
+
Using Logging APIs to Generate Log Messages
---
diff --git a/lib/eal/common/eal_common_options.c
b/lib/eal/common/eal_common_options.c
index 7310d10dfd78..9bc95433d27c 100644
--- a/lib/eal/common/eal_common_options.c
+++ b/lib/eal/common/eal_common_options.c
@@ -77,6 +77,7 @@ eal_long_options[] = {
{OPT_IOVA_MODE, 1, NULL, OPT_IOVA_MODE_NUM},
{OPT_LCORES,1, NULL, OPT_LCORES_NUM },
{OPT_LOG_LEVEL, 1, NULL, OPT_LOG_LEVEL_NUM},
+ {OPT_LOG_TIMESTAMP, 0, NULL, OPT_LOG_TIMESTAMP_NUM},
{OPT_TRACE, 1, NULL, OPT_TRACE_NUM},
{OPT_TRACE_DIR, 1, NULL, OPT_TRACE_DIR_NUM},
{OPT_TRACE_BUF_SIZE,1, NULL, OPT_TRACE_BUF_SIZE_NUM },
@@ -1663,6 +1664,7 @@ eal_log_level_parse(int argc, char * const argv[])
switch (opt) {
case OPT_LOG_LEVEL_NUM:
+ case OPT_LOG_TIMESTAMP_NUM:
if (eal_parse_common_option(opt, optarg, internal_conf)
< 0)
return -1;
break;
@@ -1890,7 +1892,7 @@ eal_parse_common_option(int opt, const char *optarg,
break;
#endif
- case OPT_LOG_LEVEL_NUM: {
+ case OPT_LOG_LEVEL_NUM:
if (eal_parse_log_level(optarg) < 0) {
EAL_LOG(ERR,
"invalid parameters for --"
@@ -1898,7 +1900,10 @@ eal_parse_common_option(int opt, const char *optarg,
return -1;
}
break;
- }
+
+ case OPT_LOG_TIMESTAMP_NUM:
+ eal_log_enable_timestamp();
+ break;
#ifndef RTE_EXEC_ENV_WINDOWS
case OPT_TRACE_NUM: {
@@ -2261,6 +2266,7 @@ eal_common
[PATCH v8 5/5] eal: initialize logging before plugins
Want to make sure that as many log messages as possible
get added with the real log stream.
Signed-off-by: Stephen Hemminger
---
lib/eal/freebsd/eal.c | 16
lib/eal/linux/eal.c | 16
lib/eal/unix/eal_unix_log.c | 0
3 files changed, 16 insertions(+), 16 deletions(-)
create mode 100644 lib/eal/unix/eal_unix_log.c
diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
index b4f8d68b0a65..e4f00f31fbdd 100644
--- a/lib/eal/freebsd/eal.c
+++ b/lib/eal/freebsd/eal.c
@@ -592,6 +592,14 @@ rte_eal_init(int argc, char **argv)
internal_conf->in_memory = false;
}
+ if (eal_log_init(program_invocation_short_name,
+internal_conf->syslog_facility) < 0) {
+ rte_eal_init_alert("Cannot init logging.");
+ rte_errno = ENOMEM;
+ rte_atomic_store_explicit(&run_once, 0,
rte_memory_order_relaxed);
+ return -1;
+ }
+
if (eal_plugins_init() < 0) {
rte_eal_init_alert("Cannot init plugins");
rte_errno = EINVAL;
@@ -718,14 +726,6 @@ rte_eal_init(int argc, char **argv)
#endif
}
- if (eal_log_init(program_invocation_short_name,
-internal_conf->syslog_facility) < 0) {
- rte_eal_init_alert("Cannot init logging.");
- rte_errno = ENOMEM;
- rte_atomic_store_explicit(&run_once, 0,
rte_memory_order_relaxed);
- return -1;
- }
-
/* in secondary processes, memory init may allocate additional fbarrays
* not present in primary processes, so to avoid any potential issues,
* initialize memzones first.
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index bffeb1f34eb9..e24f24b1b0ce 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -973,6 +973,14 @@ rte_eal_init(int argc, char **argv)
return -1;
}
+ if (eal_log_init(program_invocation_short_name,
+internal_conf->syslog_facility) < 0) {
+ rte_eal_init_alert("Cannot init logging.");
+ rte_errno = ENOMEM;
+ rte_atomic_store_explicit(&run_once, 0,
rte_memory_order_relaxed);
+ return -1;
+ }
+
if (eal_plugins_init() < 0) {
rte_eal_init_alert("Cannot init plugins");
rte_errno = EINVAL;
@@ -1107,14 +1115,6 @@ rte_eal_init(int argc, char **argv)
#endif
}
- if (eal_log_init(program_invocation_short_name,
-internal_conf->syslog_facility) < 0) {
- rte_eal_init_alert("Cannot init logging.");
- rte_errno = ENOMEM;
- rte_atomic_store_explicit(&run_once, 0,
rte_memory_order_relaxed);
- return -1;
- }
-
#ifdef VFIO_PRESENT
if (rte_eal_vfio_setup() < 0) {
rte_eal_init_alert("Cannot init VFIO");
diff --git a/lib/eal/unix/eal_unix_log.c b/lib/eal/unix/eal_unix_log.c
new file mode 100644
index ..e69de29bb2d1
--
2.43.0
[PATCH v2] doc: add dma perf feature details
Update dma perf test document with below support features: 1. Memory-to-device and device-to-memory copy. 2. Skip support. 3. Scatter-gather support. Signed-off-by: Amit Prakash Shukla --- v2: - Rebased the patch. doc/guides/tools/dmaperf.rst | 89 ++-- 1 file changed, 64 insertions(+), 25 deletions(-) diff --git a/doc/guides/tools/dmaperf.rst b/doc/guides/tools/dmaperf.rst index 6f85fceb8a..dadcc97530 100644 --- a/doc/guides/tools/dmaperf.rst +++ b/doc/guides/tools/dmaperf.rst @@ -5,27 +5,23 @@ dpdk-test-dma-perf Application == The ``dpdk-test-dma-perf`` tool is a Data Plane Development Kit (DPDK) application -that enables testing the performance of DMA (Direct Memory Access) devices available within DPDK. -It provides a test framework to assess the performance of CPU and DMA devices -under various scenarios, such as varying buffer lengths. -Doing so provides insight into the potential performance -when using these DMA devices for acceleration in DPDK applications. +that evaluates the performance of DMA (Direct Memory Access) devices accessible in DPDK environment. +It provides a benchmark framework to assess the performance of CPU and DMA devices +under various combinations, such as varying buffer lengths, scatter-gather copy, copying in remote +memory etc. It helps in evaluating performance of DMA device as hardware acceleration vehicle in +DPDK application. -It supports memory copy performance tests for now, -comparing the performance of CPU and DMA automatically in various conditions -with the help of a pre-set configuration file. +In addition, this tool supports memory-to-memory, memory-to-device and device-to-memory copy tests, +to compare the performance of CPU and DMA capabilities under various conditions with the help of a +pre-set configuration file. Configuration - -This application uses inherent DPDK EAL command-line options -as well as custom command-line options in the application. -An example configuration file for the application is provided -and gives the meanings for each parameter. - -Here is an extracted sample from the configuration file -(the complete sample can be found in the application source directory): +Along with EAL command-line arguments, this application supports various parameters for the +benchmarking through a configuration file. An example configuration file is provided below along +with the application to demonstrate all the parameters. .. code-block:: ini @@ -53,14 +49,35 @@ Here is an extracted sample from the configuration file lcore = 3, 4 eal_args=--in-memory --no-pci + [case3] + skip=1 + type=DMA_MEM_COPY + direction=mem2dev + vchan_dev=raddr=0x2,coreid=1,pfid=2,vfid=3 + dma_src_sge=4 + dma_dst_sge=1 + mem_size=10 + buf_size=64,8192,2,MUL + dma_ring_size=1024 + kick_batch=32 + src_numa_node=0 + dst_numa_node=0 + cache_flush=0 + test_seconds=2 + lcore_dma=lcore10@:00:04.2, lcore11@:00:04.3 + eal_args=--in-memory --file-prefix=test + The configuration file is divided into multiple sections, each section represents a test case. -The four variables ``mem_size``, ``buf_size``, ``dma_ring_size``, and ``kick_batch`` -can vary in each test case. -The format for this is ``variable=first,last,increment,ADD|MUL``. -This means that the first value of the variable is 'first', -the last value is 'last', -'increment' is the step size, -and 'ADD|MUL' indicates whether the change is by addition or multiplication. +The four mandatory variables ``mem_size``, ``buf_size``, ``dma_ring_size``, and ``kick_batch`` +can vary in each test case. The format for this is ``variable=first,last,increment,ADD|MUL``. +This means that the first value of the variable is 'first', the last value is 'last', +'increment' is the step size, and 'ADD|MUL' indicates whether the change is by addition or +multiplication. + +The variables for mem2dev and dev2mem copy are ``direction``, ``vchan_dev`` and can vary in each +test case. If the direction is not configured, the default is mem2mem copy. + +For scatter-gather copy test ``dma_src_sge``, ``dma_dst_sge`` must be configured. Each case can only have one variable change, and each change will generate a scenario, so each case can have multiple scenarios. @@ -69,10 +86,32 @@ and each change will generate a scenario, so each case can have multiple scenari Configuration Parameters +``skip`` + To skip a test-case, must be configured as ``1`` + ``type`` The type of the test. Currently supported types are ``DMA_MEM_COPY`` and ``CPU_MEM_COPY``. +``direction`` + The direction of data transfer. + Currently supported directions: + +* ``mem2mem`` - memory to memory copy + +* ``mem2dev`` - memory to device copy + +* ``dev2mem`` - device to memory copy + +``vchan_dev`` + Comma separated bus related parameters for ``mem2dev`` and ``dev2mem``
Re: [PATCH 0/3] support setting lanes
On Mon, Mar 18, 2024 at 7:56 AM Thomas Monjalon wrote: > > 12/03/2024 08:52, Dengdui Huang: > > Some speeds can be achieved with different number of lanes. For example, > > 100Gbps can be achieved using two lanes of 50Gbps or four lanes of 25Gbps. > > When use different lanes, the port cannot be up. > > I'm not sure what you are referring to. > I suppose it is not PCI lanes. > Please could you link to an explanation of how a port is split in lanes? > Which hardware does this? > > > This is a snapshot of 100Gb that the latest BCM576xx supports. 100Gb (NRZ: 25G per lane, 4 lanes) link speed 100Gb (PAM4-56: 50G per lane, 2 lanes) link speed 100Gb (PAM4-112: 100G per lane, 1 lane) link speed Let the user feed in lanes=< integer value> and the NIC driver decides the matching combination speed x lanes that works. In future if a new speed is implemented with more than 8 lanes, there wouldn't be a need to touch this speed command. Using separate lane command would be a better alternative to support already shipped products and only new drivers would consider this lanes configuration, if applicable. -- This electronic communication and the information and any files transmitted with it, or attached to it, are confidential and are intended solely for the use of the individual or entity to whom it is addressed and may contain information that is confidential, legally privileged, protected by privacy laws, or otherwise restricted from disclosure to anyone else. If you are not the intended recipient or the person responsible for delivering the e-mail to the intended recipient, you are hereby notified that any use, copying, distributing, dissemination, forwarding, printing, or copying of this e-mail is strictly prohibited. If you received this e-mail in error, please return the e-mail to the sender, delete it from your computer, and destroy any printed copy of it. smime.p7s Description: S/MIME Cryptographic Signature
Re: [PATCH 0/3] support setting lanes
On Mon, 18 Mar 2024 14:26:33 -0700 Damodharam Ammepalli wrote: > On Mon, Mar 18, 2024 at 7:56 AM Thomas Monjalon wrote: > > > > 12/03/2024 08:52, Dengdui Huang: > > > Some speeds can be achieved with different number of lanes. For example, > > > 100Gbps can be achieved using two lanes of 50Gbps or four lanes of 25Gbps. > > > When use different lanes, the port cannot be up. > > > > I'm not sure what you are referring to. > > I suppose it is not PCI lanes. > > Please could you link to an explanation of how a port is split in lanes? > > Which hardware does this? > > > > > > > This is a snapshot of 100Gb that the latest BCM576xx supports. > 100Gb (NRZ: 25G per lane, 4 lanes) link speed > 100Gb (PAM4-56: 50G per lane, 2 lanes) link speed > 100Gb (PAM4-112: 100G per lane, 1 lane) link speed > > Let the user feed in lanes=< integer value> and the NIC driver decides > the matching combination speed x lanes that works. In future if a new speed > is implemented with more than 8 lanes, there wouldn't be a need > to touch this speed command. Using separate lane command would > be a better alternative to support already shipped products and only new > drivers would consider this lanes configuration, if applicable. > The DPDK does not need more driver specific knobs. Shouldn't the PMD be able to auto negotiate the speed? What does Linux do?
[PATCH v9 0/5] Logging unification and timestamp
Improvements and unification of logging library (for 24.07 release).
This is update to earlier patch set.
v9 - reorder patches and fix FreeBSD build
v8 - rebase to current code base where logging in in lib/log
use stdio for log timestamp
initialization changes (setup log earlier)
Stephen Hemminger (5):
log: unify logging code
eal: make eal_log_level_parse common
eal: initialize logging before plugins
eal: allow user to set default log stream before init
eal: add option to put timestamp on console output
app/dumpcap/main.c| 3 +
app/pdump/main.c | 3 +
app/proc-info/main.c | 3 +
app/test/test_eal_flags.c | 9 +++
doc/guides/linux_gsg/linux_eal_parameters.rst | 27 -
doc/guides/prog_guide/log_lib.rst | 28 +-
lib/eal/common/eal_common_options.c | 56 ++-
lib/eal/common/eal_options.h | 3 +
lib/eal/freebsd/eal.c | 50 +++--
lib/eal/linux/eal.c | 55 +++---
lib/eal/unix/eal_unix_log.c | 0
lib/eal/windows/eal.c | 35
lib/log/log.c | 6 ++
lib/log/log_freebsd.c | 12
lib/log/log_internal.h| 11
lib/log/{log_linux.c => log_unix.c} | 36 +++-
lib/log/log_windows.c | 6 ++
lib/log/meson.build | 12 ++--
lib/log/version.map | 2 +
19 files changed, 184 insertions(+), 173 deletions(-)
create mode 100644 lib/eal/unix/eal_unix_log.c
delete mode 100644 lib/log/log_freebsd.c
rename lib/log/{log_linux.c => log_unix.c} (58%)
--
2.43.0
[PATCH v9 1/5] log: unify logging code
FreeBSD and Linux logging code can use common code. This also
fixes FreeBSD not using syslog.
Signed-off-by: Stephen Hemminger
---
doc/guides/linux_gsg/linux_eal_parameters.rst | 27 ---
doc/guides/prog_guide/log_lib.rst | 18 +++--
lib/eal/freebsd/eal.c | 8 ++
lib/log/log_freebsd.c | 12 -
lib/log/{log_linux.c => log_unix.c} | 0
lib/log/meson.build | 12 ++---
6 files changed, 32 insertions(+), 45 deletions(-)
delete mode 100644 lib/log/log_freebsd.c
rename lib/log/{log_linux.c => log_unix.c} (100%)
diff --git a/doc/guides/linux_gsg/linux_eal_parameters.rst
b/doc/guides/linux_gsg/linux_eal_parameters.rst
index ea8f38139119..d86f94d8a85d 100644
--- a/doc/guides/linux_gsg/linux_eal_parameters.rst
+++ b/doc/guides/linux_gsg/linux_eal_parameters.rst
@@ -108,30 +108,3 @@ Memory-related options
* ``--match-allocations``
Free hugepages back to system exactly as they were originally allocated.
-
-Other options
-~
-
-* ``--syslog ``
-
-Set syslog facility. Valid syslog facilities are::
-
-auth
-cron
-daemon
-ftp
-kern
-lpr
-mail
-news
-syslog
-user
-uucp
-local0
-local1
-local2
-local3
-local4
-local5
-local6
-local7
diff --git a/doc/guides/prog_guide/log_lib.rst
b/doc/guides/prog_guide/log_lib.rst
index ff9d1b54a2c8..aacb36c36ce0 100644
--- a/doc/guides/prog_guide/log_lib.rst
+++ b/doc/guides/prog_guide/log_lib.rst
@@ -5,8 +5,8 @@ Log Library
===
The DPDK Log library provides the logging functionality for other DPDK
libraries and drivers.
-By default, in a Linux application, logs are sent to syslog and also to the
console.
-On FreeBSD and Windows applications, logs are sent only to the console.
+By default, in a Linux (or FreeBSD) application, logs are sent to syslog and
also to the console.
+In Windows applications, logs are sent only to the console.
However, the log function can be overridden by the user to use a different
logging mechanism.
Log Levels
@@ -29,6 +29,7 @@ will be emitted by the application to the log output.
That level can be configured either by the application calling the relevant
APIs from the logging library,
or by the user passing the ``--log-level`` parameter to the EAL via the
application.
+
Setting Global Log Level
@@ -59,6 +60,19 @@ For example::
Within an application, the same result can be got using the
``rte_log_set_level_pattern()`` or ``rte_log_set_level_regex()`` APIs.
+
+Setting syslog facility
+~~~
+
+On Linux and FreeBSD, where syslog is used a ``facility`` argument can be
+used to specify what type of program is logging.
+The default facility is ``daemon`` but it can be overridden
+by the ``--syslog`` EAL parameter. See ``syslog.3`` man page for full values.
+For example::
+
+ /path/to/app --syslog local0
+
+
Using Logging APIs to Generate Log Messages
---
diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
index bab77118e967..a57ee8406f0c 100644
--- a/lib/eal/freebsd/eal.c
+++ b/lib/eal/freebsd/eal.c
@@ -53,6 +53,7 @@
#include "eal_options.h"
#include "eal_memcfg.h"
#include "eal_trace.h"
+#include "log_internal.h"
#define MEMSIZE_IF_NO_HUGE_PAGE (64ULL * 1024ULL * 1024ULL)
@@ -760,6 +761,13 @@ rte_eal_init(int argc, char **argv)
#endif
}
+ if (eal_log_init(getprogname(), internal_conf->syslog_facility) < 0) {
+ rte_eal_init_alert("Cannot init logging.");
+ rte_errno = ENOMEM;
+ rte_atomic_store_explicit(&run_once, 0,
rte_memory_order_relaxed);
+ return -1;
+ }
+
/* in secondary processes, memory init may allocate additional fbarrays
* not present in primary processes, so to avoid any potential issues,
* initialize memzones first.
diff --git a/lib/log/log_freebsd.c b/lib/log/log_freebsd.c
deleted file mode 100644
index 698d3c542337..
--- a/lib/log/log_freebsd.c
+++ /dev/null
@@ -1,12 +0,0 @@
-/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2023 Intel Corporation
- */
-
-#include
-#include "log_internal.h"
-
-int
-eal_log_init(__rte_unused const char *id, __rte_unused int facility)
-{
- return 0;
-}
diff --git a/lib/log/log_linux.c b/lib/log/log_unix.c
similarity index 100%
rename from lib/log/log_linux.c
rename to lib/log/log_unix.c
diff --git a/lib/log/meson.build b/lib/log/meson.build
index 0d4319b36f77..60516a0b2a2d 100644
--- a/lib/log/meson.build
+++ b/lib/log/meson.build
@@ -2,8 +2,12 @@
# Copyright(c) 2023 Intel Corporation
includes += global_inc
-sources = files(
-'log.c',
-'log_' + exec_env + '.c',
-)
+sources = files('log.c')
+
+if is_windows
+so
[PATCH v9 2/5] eal: make eal_log_level_parse common
The code to parse for log-level option should be same on
all OS variants.
Signed-off-by: Stephen Hemminger
---
lib/eal/common/eal_common_options.c | 46 +
lib/eal/common/eal_options.h| 1 +
lib/eal/freebsd/eal.c | 42 --
lib/eal/linux/eal.c | 39
lib/eal/windows/eal.c | 35 --
5 files changed, 47 insertions(+), 116 deletions(-)
diff --git a/lib/eal/common/eal_common_options.c
b/lib/eal/common/eal_common_options.c
index e541f0793964..7310d10dfd78 100644
--- a/lib/eal/common/eal_common_options.c
+++ b/lib/eal/common/eal_common_options.c
@@ -1640,6 +1640,51 @@ eal_parse_huge_unlink(const char *arg, struct
hugepage_file_discipline *out)
return -1;
}
+/* Parse the all arguments looking for --log-level */
+int
+eal_log_level_parse(int argc, char * const argv[])
+{
+ struct internal_config *internal_conf =
eal_get_internal_configuration();
+ int option_index, opt;
+ const int old_optind = optind;
+ const int old_optopt = optopt;
+ const int old_opterr = opterr;
+ char *old_optarg = optarg;
+#ifdef RTE_EXEC_ENV_FREEBSD
+ const int old_optreset = optreset;
+ optreset = 1;
+#endif
+
+ optind = 1;
+ opterr = 0;
+
+ while ((opt = getopt_long(argc, argv, eal_short_options,
+ eal_long_options, &option_index)) != EOF) {
+
+ switch (opt) {
+ case OPT_LOG_LEVEL_NUM:
+ if (eal_parse_common_option(opt, optarg, internal_conf)
< 0)
+ return -1;
+ break;
+ case '?':
+ /* getopt is not happy, stop right now */
+ goto out;
+ default:
+ continue;
+ }
+ }
+out:
+ /* restore getopt lib */
+ optind = old_optind;
+ optopt = old_optopt;
+ optarg = old_optarg;
+ opterr = old_opterr;
+#ifdef RTE_EXEC_ENV_FREEBSD
+ optreset = old_optreset;
+#endif
+ return 0;
+}
+
int
eal_parse_common_option(int opt, const char *optarg,
struct internal_config *conf)
@@ -2173,6 +2218,7 @@ rte_vect_set_max_simd_bitwidth(uint16_t bitwidth)
return 0;
}
+
void
eal_common_usage(void)
{
diff --git a/lib/eal/common/eal_options.h b/lib/eal/common/eal_options.h
index 3cc9cb641284..f3f2e104f6d7 100644
--- a/lib/eal/common/eal_options.h
+++ b/lib/eal/common/eal_options.h
@@ -96,6 +96,7 @@ enum {
extern const char eal_short_options[];
extern const struct option eal_long_options[];
+int eal_log_level_parse(int argc, char * const argv[]);
int eal_parse_common_option(int opt, const char *argv,
struct internal_config *conf);
int eal_option_device_parse(void);
diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
index a57ee8406f0c..94927472edfe 100644
--- a/lib/eal/freebsd/eal.c
+++ b/lib/eal/freebsd/eal.c
@@ -364,48 +364,6 @@ eal_get_hugepage_mem_size(void)
return (size < SIZE_MAX) ? (size_t)(size) : SIZE_MAX;
}
-/* Parse the arguments for --log-level only */
-static void
-eal_log_level_parse(int argc, char **argv)
-{
- int opt;
- char **argvopt;
- int option_index;
- const int old_optind = optind;
- const int old_optopt = optopt;
- const int old_optreset = optreset;
- char * const old_optarg = optarg;
- struct internal_config *internal_conf =
- eal_get_internal_configuration();
-
- argvopt = argv;
- optind = 1;
- optreset = 1;
-
- while ((opt = getopt_long(argc, argvopt, eal_short_options,
- eal_long_options, &option_index)) != EOF) {
-
- int ret;
-
- /* getopt is not happy, stop right now */
- if (opt == '?')
- break;
-
- ret = (opt == OPT_LOG_LEVEL_NUM) ?
- eal_parse_common_option(opt, optarg, internal_conf) : 0;
-
- /* common parser is not happy */
- if (ret < 0)
- break;
- }
-
- /* restore getopt lib */
- optind = old_optind;
- optopt = old_optopt;
- optreset = old_optreset;
- optarg = old_optarg;
-}
-
/* Parse the argument given in the command line of the application */
static int
eal_parse_args(int argc, char **argv)
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index fd422f1f6236..bffeb1f34eb9 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -546,45 +546,6 @@ eal_parse_vfio_vf_token(const char *vf_token)
return -1;
}
-/* Parse the arguments for --log-level only */
-static void
-eal_log_level_parse(int argc, char **argv)
-{
- int opt;
- char **argvopt;
- int option_index;
- const int old_optind = optind;
-
[PATCH v9 3/5] eal: initialize logging before plugins
Want to make sure that as many log messages as possible
get added with the real log stream.
Signed-off-by: Stephen Hemminger
---
lib/eal/freebsd/eal.c | 14 +++---
lib/eal/linux/eal.c | 16
lib/eal/unix/eal_unix_log.c | 0
3 files changed, 15 insertions(+), 15 deletions(-)
create mode 100644 lib/eal/unix/eal_unix_log.c
diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
index 94927472edfe..6f0080c4d8c6 100644
--- a/lib/eal/freebsd/eal.c
+++ b/lib/eal/freebsd/eal.c
@@ -593,6 +593,13 @@ rte_eal_init(int argc, char **argv)
internal_conf->in_memory = false;
}
+ if (eal_log_init(getprogname(), internal_conf->syslog_facility) < 0) {
+ rte_eal_init_alert("Cannot init logging.");
+ rte_errno = ENOMEM;
+ rte_atomic_store_explicit(&run_once, 0,
rte_memory_order_relaxed);
+ return -1;
+ }
+
if (eal_plugins_init() < 0) {
rte_eal_init_alert("Cannot init plugins");
rte_errno = EINVAL;
@@ -719,13 +726,6 @@ rte_eal_init(int argc, char **argv)
#endif
}
- if (eal_log_init(getprogname(), internal_conf->syslog_facility) < 0) {
- rte_eal_init_alert("Cannot init logging.");
- rte_errno = ENOMEM;
- rte_atomic_store_explicit(&run_once, 0,
rte_memory_order_relaxed);
- return -1;
- }
-
/* in secondary processes, memory init may allocate additional fbarrays
* not present in primary processes, so to avoid any potential issues,
* initialize memzones first.
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index bffeb1f34eb9..e24f24b1b0ce 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -973,6 +973,14 @@ rte_eal_init(int argc, char **argv)
return -1;
}
+ if (eal_log_init(program_invocation_short_name,
+internal_conf->syslog_facility) < 0) {
+ rte_eal_init_alert("Cannot init logging.");
+ rte_errno = ENOMEM;
+ rte_atomic_store_explicit(&run_once, 0,
rte_memory_order_relaxed);
+ return -1;
+ }
+
if (eal_plugins_init() < 0) {
rte_eal_init_alert("Cannot init plugins");
rte_errno = EINVAL;
@@ -1107,14 +1115,6 @@ rte_eal_init(int argc, char **argv)
#endif
}
- if (eal_log_init(program_invocation_short_name,
-internal_conf->syslog_facility) < 0) {
- rte_eal_init_alert("Cannot init logging.");
- rte_errno = ENOMEM;
- rte_atomic_store_explicit(&run_once, 0,
rte_memory_order_relaxed);
- return -1;
- }
-
#ifdef VFIO_PRESENT
if (rte_eal_vfio_setup() < 0) {
rte_eal_init_alert("Cannot init VFIO");
diff --git a/lib/eal/unix/eal_unix_log.c b/lib/eal/unix/eal_unix_log.c
new file mode 100644
index ..e69de29bb2d1
--
2.43.0
[PATCH v9 4/5] eal: allow user to set default log stream before init
It is useful for application to be able to set the default log
stream before call rte_eal_init(). This makes all messages go
to the new default.
For example, to skip using syslog; just doing
rte_openlog_stream(stderr);
There is no reason for helper command line applications to clutter
syslog with messages.
Signed-off-by: Stephen Hemminger
---
app/dumpcap/main.c | 3 +++
app/pdump/main.c | 3 +++
app/proc-info/main.c | 3 +++
lib/log/log.c | 6 ++
lib/log/log_internal.h | 2 ++
lib/log/log_unix.c | 4
lib/log/version.map| 1 +
7 files changed, 22 insertions(+)
diff --git a/app/dumpcap/main.c b/app/dumpcap/main.c
index cc0f66b2bc61..27934ca7e688 100644
--- a/app/dumpcap/main.c
+++ b/app/dumpcap/main.c
@@ -633,6 +633,9 @@ static void dpdk_init(void)
rte_panic("No memory\n");
}
+ /* keep any logging away from syslog. */
+ rte_openlog_stream(stderr);
+
if (rte_eal_init(eal_argc, eal_argv) < 0)
rte_exit(EXIT_FAILURE, "EAL init failed: is primary process
running?\n");
}
diff --git a/app/pdump/main.c b/app/pdump/main.c
index a9205e130bb1..7b9ba68b1a14 100644
--- a/app/pdump/main.c
+++ b/app/pdump/main.c
@@ -995,6 +995,9 @@ main(int argc, char **argv)
argc += 2;
+ /* keep any logging away from syslog. */
+ rte_openlog_stream(stderr);
+
diag = rte_eal_init(argc, argp);
if (diag < 0)
rte_panic("Cannot init EAL\n");
diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index b672aaefbe99..24ee52c4ac7a 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -2149,6 +2149,9 @@ main(int argc, char **argv)
argc += 4;
+ /* keep any logging away from syslog. */
+ rte_openlog_stream(stderr);
+
ret = rte_eal_init(argc, argp);
if (ret < 0)
rte_panic("Cannot init EAL\n");
diff --git a/lib/log/log.c b/lib/log/log.c
index 255f757d94cc..4cc944305057 100644
--- a/lib/log/log.c
+++ b/lib/log/log.c
@@ -519,6 +519,12 @@ eal_log_set_default(FILE *default_log)
#endif
}
+FILE *
+eal_log_get_default(void)
+{
+ return default_log_stream;
+}
+
/*
* Called by eal_cleanup
*/
diff --git a/lib/log/log_internal.h b/lib/log/log_internal.h
index 451629f1c1ba..c77e687e28bc 100644
--- a/lib/log/log_internal.h
+++ b/lib/log/log_internal.h
@@ -21,6 +21,8 @@ int eal_log_init(const char *id, int facility);
*/
__rte_internal
void eal_log_set_default(FILE *default_log);
+__rte_internal
+FILE *eal_log_get_default(void);
/*
* Save a log option for later.
diff --git a/lib/log/log_unix.c b/lib/log/log_unix.c
index 2dfb0c974b1d..a415bae5774d 100644
--- a/lib/log/log_unix.c
+++ b/lib/log/log_unix.c
@@ -49,6 +49,10 @@ eal_log_init(const char *id, int facility)
{
FILE *log_stream;
+ /* skip if user has already setup a log stream */
+ if (eal_log_get_default())
+ return 0;
+
log_stream = fopencookie(NULL, "w+", console_log_func);
if (log_stream == NULL)
return -1;
diff --git a/lib/log/version.map b/lib/log/version.map
index 0648f8831aff..6ecc656d1d65 100644
--- a/lib/log/version.map
+++ b/lib/log/version.map
@@ -25,6 +25,7 @@ DPDK_24 {
INTERNAL {
global:
+ eal_log_get_default;
eal_log_init;
eal_log_level2str;
eal_log_save_pattern;
--
2.43.0
[PATCH v9 5/5] eal: add option to put timestamp on console output
When debugging driver or startup issues, it is useful to have
a timestamp on each message printed. The messages in syslog
already have a timestamp, but often syslog is not available
during testing. The timestamp format is chosen to look
like the default Linux dmesg timestamp.
The first few lines are not timestamped because the flag is stored
in internal configuration which is stored in shared memory
which is not setup up until a little later in startup process.
This logging skips the unnecessary step of going through stdio,
which makes it more robust against being called in interrupt
handlers etc.
Example:
$ dpdk-testpmd --log-timestamp -- -i
EAL: Detected CPU lcores: 16
EAL: Detected NUMA nodes: 1
EAL: Detected static linkage of DPDK
EAL: Multi-process socket /var/run/dpdk/rte/mp_socket
EAL: Selected IOVA mode 'VA'
[ 0.112264] testpmd: No probed ethernet devices
Interactive-mode selected
[ 0.184573] testpmd: create a new mbuf pool : n=163456,
size=2176, socket=0
[ 0.184612] testpmd: preferred mempool ops selected: ring_mp_mc
Signed-off-by: Stephen Hemminger
---
app/test/test_eal_flags.c | 9
doc/guides/prog_guide/log_lib.rst | 10 +
lib/eal/common/eal_common_options.c | 10 +++--
lib/eal/common/eal_options.h| 2 ++
lib/log/log_internal.h | 9
lib/log/log_unix.c | 32 +++--
lib/log/log_windows.c | 6 ++
lib/log/version.map | 1 +
8 files changed, 75 insertions(+), 4 deletions(-)
diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c
index 6cb4b0675730..07a038fb6051 100644
--- a/app/test/test_eal_flags.c
+++ b/app/test/test_eal_flags.c
@@ -1055,6 +1055,10 @@ test_misc_flags(void)
const char * const argv22[] = {prgname, prefix, mp_flag,
"--huge-worker-stack=512"};
+ /* Try running with --log-timestamp */
+ const char * const argv23[] = {prgname, prefix, mp_flag,
+ "--log-timestamp" };
+
/* run all tests also applicable to FreeBSD first */
if (launch_proc(argv0) == 0) {
@@ -1162,6 +1166,11 @@ test_misc_flags(void)
printf("Error - process did not run ok with
--huge-worker-stack=size parameter\n");
goto fail;
}
+ if (launch_proc(argv23) != 0) {
+ printf("Error - process did not run ok with --log-timestamp
parameter\n");
+ goto fail;
+ }
+
rmdir(hugepath_dir3);
rmdir(hugepath_dir2);
diff --git a/doc/guides/prog_guide/log_lib.rst
b/doc/guides/prog_guide/log_lib.rst
index aacb36c36ce0..1d6b2e3cea5d 100644
--- a/doc/guides/prog_guide/log_lib.rst
+++ b/doc/guides/prog_guide/log_lib.rst
@@ -73,6 +73,16 @@ For example::
/path/to/app --syslog local0
+Console timestamp
+~
+
+On Linux and FreeBSD, an optional timestamp can be added before each
+message by adding the ``--log-timestamp`` option.
+For example::
+
+ /path/to/app --log-level=lib.*:debug --log-timestamp
+
+
Using Logging APIs to Generate Log Messages
---
diff --git a/lib/eal/common/eal_common_options.c
b/lib/eal/common/eal_common_options.c
index 7310d10dfd78..9bc95433d27c 100644
--- a/lib/eal/common/eal_common_options.c
+++ b/lib/eal/common/eal_common_options.c
@@ -77,6 +77,7 @@ eal_long_options[] = {
{OPT_IOVA_MODE, 1, NULL, OPT_IOVA_MODE_NUM},
{OPT_LCORES,1, NULL, OPT_LCORES_NUM },
{OPT_LOG_LEVEL, 1, NULL, OPT_LOG_LEVEL_NUM},
+ {OPT_LOG_TIMESTAMP, 0, NULL, OPT_LOG_TIMESTAMP_NUM},
{OPT_TRACE, 1, NULL, OPT_TRACE_NUM},
{OPT_TRACE_DIR, 1, NULL, OPT_TRACE_DIR_NUM},
{OPT_TRACE_BUF_SIZE,1, NULL, OPT_TRACE_BUF_SIZE_NUM },
@@ -1663,6 +1664,7 @@ eal_log_level_parse(int argc, char * const argv[])
switch (opt) {
case OPT_LOG_LEVEL_NUM:
+ case OPT_LOG_TIMESTAMP_NUM:
if (eal_parse_common_option(opt, optarg, internal_conf)
< 0)
return -1;
break;
@@ -1890,7 +1892,7 @@ eal_parse_common_option(int opt, const char *optarg,
break;
#endif
- case OPT_LOG_LEVEL_NUM: {
+ case OPT_LOG_LEVEL_NUM:
if (eal_parse_log_level(optarg) < 0) {
EAL_LOG(ERR,
"invalid parameters for --"
@@ -1898,7 +1900,10 @@ eal_parse_common_option(int opt, const char *optarg,
return -1;
}
break;
- }
+
+ case OPT_LOG_TIMESTAMP_NUM:
+ eal_log_enable_timestamp();
+ break;
#ifndef RTE_EXEC_ENV_WINDOWS
case OPT_TRACE_NUM: {
@@ -2261,6 +2266,7 @@ eal_common
Re: [dpdk-dev] [PATCH v5 2/4] eal: improve options usage text
On Mon, 5 Apr 2021 21:39:52 +0200 Thomas Monjalon wrote: > The description of the EAL options was printed before the application > description provided via the hook. > It is better to let the application print the global syntax > and describes the detail of the EAL options below. > > Also, some useless lines are removed, > and the alignment of few options is fixed. > > Signed-off-by: Thomas Monjalon > Acked-by: Bruce Richardson > Acked-by: Andrew Rybchenko This patch no longer applies, please rebase and resubmit
Re: [dpdk-dev] [PATCH] interrupts: fix error log level
On Mon, 1 Nov 2021 23:14:47 +0530
Harman Kalra wrote:
> Fixing the error logs level, as currently default level is
> set to debug. Due to this failure is not getting captured.
>
> Fixes: b7c984291611 ("interrupts: add allocator and accessors")
>
> Signed-off-by: Harman Kalra
Patch concept is good, but no longer applies cleanly.
Please rebase and resubmit
Re: [PATCH 0/3] support setting lanes
On Mon, Mar 18, 2024 at 2:42 PM Stephen Hemminger wrote: > > On Mon, 18 Mar 2024 14:26:33 -0700 > Damodharam Ammepalli wrote: > > > On Mon, Mar 18, 2024 at 7:56 AM Thomas Monjalon wrote: > > > > > > 12/03/2024 08:52, Dengdui Huang: > > > > Some speeds can be achieved with different number of lanes. For example, > > > > 100Gbps can be achieved using two lanes of 50Gbps or four lanes of > > > > 25Gbps. > > > > When use different lanes, the port cannot be up. > > > > > > I'm not sure what you are referring to. > > > I suppose it is not PCI lanes. > > > Please could you link to an explanation of how a port is split in lanes? > > > Which hardware does this? > > > > > > > > > > > This is a snapshot of 100Gb that the latest BCM576xx supports. > > 100Gb (NRZ: 25G per lane, 4 lanes) link speed > > 100Gb (PAM4-56: 50G per lane, 2 lanes) link speed > > 100Gb (PAM4-112: 100G per lane, 1 lane) link speed > > > > Let the user feed in lanes=< integer value> and the NIC driver decides > > the matching combination speed x lanes that works. In future if a new speed > > is implemented with more than 8 lanes, there wouldn't be a need > > to touch this speed command. Using separate lane command would > > be a better alternative to support already shipped products and only new > > drivers would consider this lanes configuration, if applicable. > > > > The DPDK does not need more driver specific knobs. > Shouldn't the PMD be able to auto negotiate the speed? Yes. Its possible to auto negotiate. And that's the default. Even for the lane count, a default number can be arrived at. > What does Linux do? ethtool has been extended a while ago to allow configuring the number of lanes along with speed and other settings. But as usual, autoneg is possible. smime.p7s Description: S/MIME Cryptographic Signature
RE: [EXT] [PATCH] app/test-crypto-perf: add throughput OOP decryption
Hi Akhil
Sorry for the late reply.
> -Original Message-
> From: Akhil Goyal
> Sent: Friday, March 15, 2024 2:45 AM
> To: Suanming Mou ; Anoob Joseph
> ; ciara.po...@intel.com
> Cc: dev@dpdk.org
> Subject: RE: [EXT] [PATCH] app/test-crypto-perf: add throughput OOP decryption
>
> > During throughput running, re-filling the test data will impact the
> > performance test result. So for now, to run decrypt throughput testing
> > is not supported since the test data is not filled.
> >
> > But if user requires OOP(out-of-place) mode, the test data from source
> > mbuf will never be modified, and if the test data can be prepared out
> > of the running loop, the decryption test should be fine.
> >
> > This commit adds the support of out-of-place decryption testing for
> > throughput.
> >
>
>
> > Signed-off-by: Suanming Mou
> > ---
> > app/test-crypto-perf/cperf_ops.c | 5 ++-
> > app/test-crypto-perf/cperf_options_parsing.c | 8 +
> > app/test-crypto-perf/cperf_test_throughput.c | 37 +---
> > 3 files changed, 44 insertions(+), 6 deletions(-)
> >
> > diff --git a/app/test-crypto-perf/cperf_ops.c
> > b/app/test-crypto-perf/cperf_ops.c
> > index 84945d1313..1d57b78c2b 100644
> > --- a/app/test-crypto-perf/cperf_ops.c
> > +++ b/app/test-crypto-perf/cperf_ops.c
> > @@ -608,7 +608,10 @@ cperf_set_ops_aead(struct rte_crypto_op **ops,
> > }
> >
> > if ((options->test == CPERF_TEST_TYPE_VERIFY) ||
> > - (options->test == CPERF_TEST_TYPE_LATENCY)) {
> > + (options->test == CPERF_TEST_TYPE_LATENCY) ||
> > + (options->test == CPERF_TEST_TYPE_THROUGHPUT &&
> > +(options->aead_op == RTE_CRYPTO_AEAD_OP_DECRYPT ||
> > + options->cipher_op == RTE_CRYPTO_CIPHER_OP_DECRYPT))) {
> > for (i = 0; i < nb_ops; i++) {
> > uint8_t *iv_ptr = rte_crypto_op_ctod_offset(ops[i],
> > uint8_t *, iv_offset);
> > diff --git a/app/test-crypto-perf/cperf_options_parsing.c
> > b/app/test-crypto- perf/cperf_options_parsing.c index
> > 75afedc7fd..6caca44371 100644
> > --- a/app/test-crypto-perf/cperf_options_parsing.c
> > +++ b/app/test-crypto-perf/cperf_options_parsing.c
> > @@ -1291,6 +1291,14 @@ cperf_options_check(struct cperf_options *options)
> > }
> > }
> >
> > + if (options->test == CPERF_TEST_TYPE_THROUGHPUT &&
> > + (options->aead_op == RTE_CRYPTO_AEAD_OP_DECRYPT ||
> > +options->cipher_op == RTE_CRYPTO_CIPHER_OP_DECRYPT) &&
> > + !options->out_of_place) {
> > + RTE_LOG(ERR, USER1, "Only out-of-place is allowed in
> > throughput decryption.\n");
> > + return -EINVAL;
> > + }
>
> This check is blocking cipher_only decryption which should pass irrespective
> of
> inplace/oop and Data correct/incorrect.
Sorry, in that case I will remove "options->cipher_op ==
RTE_CRYPTO_CIPHER_OP_DECRYPT" and only kept " options->aead_op ==
RTE_CRYPTO_AEAD_OP_DECRYPT ", what do you think?
>
> > +
> > if (options->op_type == CPERF_CIPHER_ONLY ||
> > options->op_type == CPERF_CIPHER_THEN_AUTH ||
> > options->op_type == CPERF_AUTH_THEN_CIPHER) { diff
> --git
> > a/app/test-crypto-perf/cperf_test_throughput.c b/app/test-crypto-
> > perf/cperf_test_throughput.c index f8f8bd717f..eab25ec863 100644
> > --- a/app/test-crypto-perf/cperf_test_throughput.c
> > +++ b/app/test-crypto-perf/cperf_test_throughput.c
> > @@ -98,6 +98,29 @@ cperf_throughput_test_constructor(struct
> > rte_mempool *sess_mp,
> > return NULL;
> > }
> >
> > +static void
> > +cperf_verify_init_ops(struct rte_mempool *mp __rte_unused,
> > + void *opaque_arg,
> > + void *obj,
> > + __rte_unused unsigned int i)
> > +{
> > + uint16_t iv_offset = sizeof(struct rte_crypto_op) +
> > + sizeof(struct rte_crypto_sym_op);
> > + uint32_t imix_idx = 0;
> > + struct cperf_throughput_ctx *ctx = opaque_arg;
> > + struct rte_crypto_op *op = obj;
> > +
> > + (ctx->populate_ops)(&op, ctx->src_buf_offset,
> > + ctx->dst_buf_offset,
> > + 1, ctx->sess, ctx->options,
> > + ctx->test_vector, iv_offset, &imix_idx, NULL);
> > +
> > + cperf_mbuf_set(op->sym->m_src,
> > + ctx->options,
> > + ctx->test_vector);
> Unnecessary line break.
>
> > +
> Extra line
Will update.
>
> > +}
> > +
> > int
> > cperf_throughput_test_runner(void *test_ctx) { @@ -143,6 +166,9 @@
> > cperf_throughput_test_runner(void *test_ctx)
> > uint16_t iv_offset = sizeof(struct rte_crypto_op) +
> > sizeof(struct rte_crypto_sym_op);
> >
> > + if (ctx->options->out_of_place)
> > + rte_mempool_obj_iter(ctx->pool, cperf_verify_init_ops, (void
> > *)ctx);
> > +
> > while (test_burst_size <= ctx->options->max_burst_size) {
> > uint64_t ops_enqd = 0, ops_enqd_total
Re: [PATCH] app/testpmd: fix unpassed RSS algorithm
On 2024/3/15 19:17, Ferruh Yigit wrote:
On 3/15/2024 3:00 AM, Jie Hai wrote:
The RSS algorithm from user is parased but not passed to the
rte_eth_dev_rss_hash_update() API as we wanted, this patch
fixes it.
Fixes: 3da59f30a23f ("app/testpmd: set RSS hash algorithm")
Cc: sta...@dpdk.org
Signed-off-by: Jie Hai
Reviewed-by: Ferruh Yigit
Applied to dpdk-next-net/main, thanks.
Sorry to see we missed something this simple, wasn't this tested when
command is added?
.
I'm sorry for my mistake.
After the first round of functional tests passed,
I didn't double check it carefully after the second round of changes.
Re: [PATCH 3/3] app/testpmd: support setting lanes
On 2024/3/16 5:47, Damodharam Ammepalli wrote:
> On Tue, Mar 12, 2024 at 12:52 AM Dengdui Huang
> wrote:
>>
>> Extended speed command for setting lane number and
>> Show info print lane number.
>>
>> Signed-off-by: Dengdui Huang
>> ---
>> app/test-pmd/cmdline.c | 110 +++-
>> app/test-pmd/config.c | 60 +++
>> doc/guides/rel_notes/release_24_03.rst | 1 +
>> doc/guides/testpmd_app_ug/testpmd_funcs.rst | 3 +-
>> 4 files changed, 103 insertions(+), 71 deletions(-)
>>
>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
>> index f521a1fe9e..e66daf4bba 100644
>> --- a/app/test-pmd/cmdline.c
>> +++ b/app/test-pmd/cmdline.c
>> @@ -1356,15 +1356,20 @@ struct cmd_config_speed_all {
>> cmdline_fixed_string_t keyword;
>> cmdline_fixed_string_t all;
>> cmdline_fixed_string_t item1;
>> + cmdline_fixed_string_t lanes_item;
>> cmdline_fixed_string_t item2;
>> cmdline_fixed_string_t value1;
>> + uint8_t lanes_value;
>> cmdline_fixed_string_t value2;
>> };
>>
>> static int
>> -parse_and_check_speed_duplex(char *speedstr, char *duplexstr, uint32_t
>> *speed)
>> +parse_and_check_speed_duplex(char *speedstr, uint8_t lanes, char *duplexstr,
>> +uint32_t *speed)
>> {
>>
> We internally implemented a similar feature, without changing the
> existing testpmd speed cmd.
> Instead of modifying the existing command set we can have a separate
> cmd for the lanes
> configuration similar to FEC configuration. Our internal
> implementation looks something like this,
> without affecting existing implementations.
> testpmd> port stop 0
> testpmd> port config 0 speed_lanes 4
Hi, Damodharam,
Thanks for your review.
I think the lanes should be configured with speed and duplex,
they will eventually map to Link speed capabilities(RTE_ETH_LINK_SPEED_*).
Would it be better to add the following new command?
testpmd> port config 0 speed 20 lanes 4 duplex full
It can be used when the driver supports setting lanes;
It cannot be used when the driver does not support the setting lanes.
what do you think?
> testpmd> port config 0 speed 20 duplex full
> testpmd> port start 0
> testpmd> show port summary 0
> Number of available ports: 2
> Port MAC Address Name Driver Status Link Lanes
> 014:23:F2:C3:BA:D2 :b1:00.0 net_bnxt up 200 Gbps 4
The summary command adds print of the number of lanes.
I will implement this in the next version.
> testpmd>
> testpmd> show port info 0
>
> * Infos for port 0 *
> MAC address: 14:23:F2:C3:BA:D2
> Device name: :b1:00.0
> Driver name: net_bnxt
> Firmware-version: 228.9.115.0
> Connect to socket: 2
> memory allocation on the socket: 2
> Link status: up
> Link speed: 200 Gbps
> Lanes: 4
> Link duplex: full-duplex
> Autoneg status: Off
>
>> + uint32_t speed_num;
>> + char *endptr;
>> int duplex;
>>
>> if (!strcmp(duplexstr, "half")) {
>> @@ -1378,47 +1383,22 @@ parse_and_check_speed_duplex(char *speedstr, char
>> *duplexstr, uint32_t *speed)
>> return -1;
>> }
>>
>> - if (!strcmp(speedstr, "10")) {
>> - *speed = (duplex == RTE_ETH_LINK_HALF_DUPLEX) ?
>> - RTE_ETH_LINK_SPEED_10M_HD :
>> RTE_ETH_LINK_SPEED_10M;
>> - } else if (!strcmp(speedstr, "100")) {
>> - *speed = (duplex == RTE_ETH_LINK_HALF_DUPLEX) ?
>> - RTE_ETH_LINK_SPEED_100M_HD :
>> RTE_ETH_LINK_SPEED_100M;
>> - } else {
>> - if (duplex != RTE_ETH_LINK_FULL_DUPLEX) {
>> - fprintf(stderr, "Invalid speed/duplex parameters\n");
>> - return -1;
>> - }
>> - if (!strcmp(speedstr, "1000")) {
>> - *speed = RTE_ETH_LINK_SPEED_1G;
>> - } else if (!strcmp(speedstr, "2500")) {
>> - *speed = RTE_ETH_LINK_SPEED_2_5G;
>> - } else if (!strcmp(speedstr, "5000")) {
>> - *speed = RTE_ETH_LINK_SPEED_5G;
>> - } else if (!strcmp(speedstr, "1")) {
>> - *speed = RTE_ETH_LINK_SPEED_10G;
>> - } else if (!strcmp(speedstr, "25000")) {
>> - *speed = RTE_ETH_LINK_SPEED_25G;
>> - } else if (!strcmp(speedstr, "4")) {
>> - *speed = RTE_ETH_LINK_SPEED_40G;
>> - } else if (!strcmp(speedstr, "5")) {
>> - *speed = RTE_ETH_LINK_SPEED_50G;
>> - } else if (!strcmp(speedstr, "10")) {
>> - *speed = RTE_ETH_LINK_SPEED_100G;
>> - } else if (!strcmp(speedstr, "20")) {
>> - *speed = RTE_ETH_LINK_SPEED_200G;
>> - } else if (!strcmp(speedst
RedHat QE's test result against DPDK release candidate 24.03-rc3
I tested below 18 scenarios on RHEL 9.2 and didn't find any new dpdk issues. - VM with device assignment(PF) throughput testing(1G hugepage size): PASS - VM with device assignment(PF) throughput testing(2M hugepage size) : PASS - VM with device assignment(VF) throughput testing: PASS - PVP (host dpdk testpmd as vswitch) 1Q: throughput testing: PASS - PVP vhost-user 2Q throughput testing: PASS - PVP vhost-user 1Q - cross numa node throughput testing: PASS - VM with vhost-user 2 queues throughput testing: PASS - vhost-user reconnect with dpdk-client, qemu-server qemu reconnect: PASS - vhost-user reconnect with dpdk-client, qemu-server ovs reconnect: PASS - PVP reconnect with dpdk-client, qemu-server: PASS - PVP 1Q live migration testing: PASS - PVP 1Q cross numa node live migration testing: PASS - VM with ovs+dpdk+vhost-user 1Q live migration testing: PASS - VM with ovs+dpdk+vhost-user 1Q live migration testing (2M): PASS - VM with ovs+dpdk+vhost-user 2Q live migration testing: PASS - VM with ovs+dpdk+vhost-user 4Q live migration testing: PASS - Host PF + DPDK testing: PASS - Host VF + DPDK testing: PASS Test Versions: - qemu-kvm-7.2.0 - kernel 5.14 - libvirt 9.0 - git log commit 80ecef6d1f71fcebc0a51d7cabc51f73ee142ff2 Author: Thomas Monjalon Date: Mon Mar 18 04:34:16 2024 +0100 version: 24.03-rc3 Signed-off-by: Thomas Monjalon - Test device : X540-AT2 NIC(ixgbe, 10G) Tested-by: Yanghang Liu
Re: [PATCH 1/3] ethdev: support setting lanes
On Tue, 12 Mar 2024 15:52:36 +0800 Dengdui Huang wrote: > - ret = snprintf(str, len, "Link up at %s %s %s", > + ret = snprintf(str, len, "Link up at %s %ulanes %s %s", Don't you want a space after %u? Could you make it so that lanes is only part of the message if non-default value is used?
Re: [PATCH] doc: deprecate graph data structures
On Wed, Feb 21, 2024 at 9:43 PM wrote: > > From: Pavan Nikhilesh > > Deprecate rte_node, rte_node_register and rte_graph_cluster_node_stats > structures as will be extended to include node specific error counters > and error description. > > Signed-off-by: Pavan Nikhilesh Implementation patches for 24.11 at https://patches.dpdk.org/project/dpdk/list/?series=31181. The deprecation notice looks good to me. Acked-by: Jerin Jacob > --- > doc/guides/rel_notes/deprecation.rst | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/doc/guides/rel_notes/deprecation.rst > b/doc/guides/rel_notes/deprecation.rst > index 10630ba255..b3dfd06ed6 100644 > --- a/doc/guides/rel_notes/deprecation.rst > +++ b/doc/guides/rel_notes/deprecation.rst > @@ -142,3 +142,8 @@ Deprecation Notices >will be deprecated and subsequently removed in DPDK 24.11 release. >Before this, the new port library API (functions rte_swx_port_*) >will gradually transition from experimental to stable status. > + > +* graph: The graph library data structures will be modified to > + support node specific errors, the structures ``rte_node``, > + ``rte_node_register`` and ``rte_graph_cluster_node_stats`` will be > + extended to include node error counters and error description. > -- > 2.25.1 >
Re: [dpdk-dev] [v6] doc: define qualification criteria for external library
On Tue, Jan 9, 2024 at 7:40 PM wrote: > > From: Jerin Jacob > > Define qualification criteria for external library > based on a techboard meeting minutes [1] and past > learnings from mailing list discussion. > > [1] > http://mails.dpdk.org/archives/dev/2019-June/135847.html > https://mails.dpdk.org/archives/dev/2024-January/284849.html > > Signed-off-by: Jerin Jacob > Acked-by: Thomas Monjalon Ping for merge > --- > > v6: > - Address Morten's comments at > https://mails.dpdk.org/archives/dev/2024-January/285029.html > > v5: > - Added "Dependency nature" section based on Stephen's input > > v4: > - Address Thomas comments from > https://patches.dpdk.org/project/dpdk/patch/20240105121215.3950532-1-jer...@marvell.com/ > > v3: > - Updated the content based on TB discussion which is documented at > https://mails.dpdk.org/archives/dev/2024-January/284849.html > > v2: > - Added "Meson build integration" and "Code readability" sections. > > doc/guides/contributing/index.rst | 1 + > .../contributing/library_dependency.rst | 53 +++ > 2 files changed, 54 insertions(+) > create mode 100644 doc/guides/contributing/library_dependency.rst > > diff --git a/doc/guides/contributing/index.rst > b/doc/guides/contributing/index.rst > index dcb9b1fbf0..e5a8c2b0a3 100644 > --- a/doc/guides/contributing/index.rst > +++ b/doc/guides/contributing/index.rst > @@ -15,6 +15,7 @@ Contributor's Guidelines > documentation > unit_test > new_library > +library_dependency > patches > vulnerability > stable > diff --git a/doc/guides/contributing/library_dependency.rst > b/doc/guides/contributing/library_dependency.rst > new file mode 100644 > index 00..3b275f1c52 > --- /dev/null > +++ b/doc/guides/contributing/library_dependency.rst > @@ -0,0 +1,53 @@ > +.. SPDX-License-Identifier: BSD-3-Clause > + Copyright(c) 2024 Marvell. > + > +External Library dependency > +=== > + > +This document defines the qualification criteria for external libraries that > may be > +used as dependencies in DPDK drivers or libraries. > +The final decision to accept or reject is at the discretion of the DPDK > Project's Technical Board. > + > +#. **Documentation:** > + > + - Must have adequate documentation for the steps to build it. > + - Must have clear license documentation on distribution and usage aspects > of external library. > + > +#. **Free availability:** > + > + - The library must be freely available to build in either source or > binary form. > + - It shall be downloadable from a direct link. There shall not be any > requirement to explicitly > + login or sign a user agreement. > + > +#. **Usage License:** > + > + - Both permissive (e.g., BSD-3 or Apache) and non-permissive (e.g., > GPLv3) licenses are acceptable. > + - In the case of a permissive license, automatic inclusion in the build > process is assumed. > + For non-permissive licenses, an additional build configuration option > is required. > + > +#. **Distribution License:** > + > + - No specific constraints, but clear documentation on distribution usage > aspects is required. > + > +#. **Compiler compatibility:** > + > + - The library must be able to compile with a DPDK supported compiler for > the given target > + environment. > + For example, for Linux, the library must be able to compile with GCC > and/or clang. > + - Library may be limited to a specific OS and/or specific hardware. > + > +#. **Meson build integration:** > + > + - The library must have standard method like ``pkg-config`` for seamless > integration with > + DPDK's build environment. > + > +#. **Code readability:** > + > + - Optional dependencies should use stubs to minimize ``ifdef`` clutter, > promoting improved > + code readability. > + > +#. **Dependency nature:** > + > + - The external library dependency must be optional. > + i.e Missing external library must not impact the core functionality of > the DPDK, specific > + library and/or driver will not be built if dependencies are not met. > -- > 2.43.0 >
RE: [dpdk-dev] [v6] doc: define qualification criteria for external library
Acked-by: Hemant Agrawal smime.p7s Description: S/MIME cryptographic signature
Re: [PATCH] doc: deprecate graph data structures
Acked-by: Nithin Dabilpuram On Wed, Feb 21, 2024 at 9:50 PM wrote: > > From: Pavan Nikhilesh > > Deprecate rte_node, rte_node_register and rte_graph_cluster_node_stats > structures as will be extended to include node specific error counters > and error description. > > Signed-off-by: Pavan Nikhilesh > --- > doc/guides/rel_notes/deprecation.rst | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/doc/guides/rel_notes/deprecation.rst > b/doc/guides/rel_notes/deprecation.rst > index 10630ba255..b3dfd06ed6 100644 > --- a/doc/guides/rel_notes/deprecation.rst > +++ b/doc/guides/rel_notes/deprecation.rst > @@ -142,3 +142,8 @@ Deprecation Notices >will be deprecated and subsequently removed in DPDK 24.11 release. >Before this, the new port library API (functions rte_swx_port_*) >will gradually transition from experimental to stable status. > + > +* graph: The graph library data structures will be modified to > + support node specific errors, the structures ``rte_node``, > + ``rte_node_register`` and ``rte_graph_cluster_node_stats`` will be > + extended to include node error counters and error description. > -- > 2.25.1 >
[PATCH] graph: fix head move when graph walk in mcore dispatch
Head move should happen after the core id check, otherwise
source node will be missed.
Fixes: 35dfd9b9fd85 ("graph: introduce graph walk by cross-core dispatch")
Cc: sta...@dpdk.org
Signed-off-by: Jingjing Wu
---
lib/graph/rte_graph_model_mcore_dispatch.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/graph/rte_graph_model_mcore_dispatch.h
b/lib/graph/rte_graph_model_mcore_dispatch.h
index 75ec388cad..b96469296e 100644
--- a/lib/graph/rte_graph_model_mcore_dispatch.h
+++ b/lib/graph/rte_graph_model_mcore_dispatch.h
@@ -97,12 +97,12 @@ rte_graph_walk_mcore_dispatch(struct rte_graph *graph)
__rte_graph_mcore_dispatch_sched_wq_process(graph);
while (likely(head != graph->tail)) {
- node = (struct rte_node *)RTE_PTR_ADD(graph,
cir_start[(int32_t)head++]);
+ node = (struct rte_node *)RTE_PTR_ADD(graph,
cir_start[(int32_t)head]);
/* skip the src nodes which not bind with current worker */
if ((int32_t)head < 0 && node->dispatch.lcore_id !=
graph->dispatch.lcore_id)
continue;
-
+ head++;
/* Schedule the node until all task/objs are done */
if (node->dispatch.lcore_id != RTE_MAX_LCORE &&
graph->dispatch.lcore_id != node->dispatch.lcore_id &&
--
2.34.1
