Re: [PATCH v12 1/1] app/testpmd: support multiple mbuf pools per Rx queue

[Singh, Aman Deep]

On 11/7/2022 11:01 AM, Hanumanth Pothula wrote:

Some of the HW has support for choosing memory pools based on
the packet's size. The pool sort capability allows PMD/NIC to
choose a memory pool based on the packet's length.

On multiple mempool support enabled, populate mempool array
accordingly. Also, print pool name on which packet is received.

Signed-off-by: Hanumanth Pothula 


Acked-by: Aman Singh 

[PATCH v5] ethdev: add special flags when creating async transfer table

[Rongwei Liu]

The transfer domain rule is able to match traffic wire/vport
origin which are corresponding to two kinds of underlayer resources.

Wire means traffic arrives from the uplink port while vport means
traffic initiated from VF/SF.

In customer deployments, they usually match only one kind of
traffic in single flow table: either from wire or from vport.
PMD can save significant resources if passing specical hint from rte
layer.

There are two possible approaches, using IPv4 as an example:
1. Use pattern item.
   pattern_template: pattern ANY_VPORT / eth / ipv4 is 1.1.1.1 / end
   async flow create: pattern ANY_VPORT / eth / ipv4 is 1.1.1.2 / end
   "ANY_VPORT" needs to be present in each async rule even if it's
   just a hint. No value to match.

2. Add special flags into table_attr. It will be:
   template_table 0 create table_id 0 group 1 transfer vf_orig

Approach 1 needs to specify the pattern in each flow rules which wastes
memory and not end user friendly.
This patch takes the 2nd approach and introduce one new member
specialize into rte_flow_table_attr to indicate async flow table matching
optimization: from wire, from vport.

It helps to save underlayer memory and also on insertion rate.

By default, there is no hint, so the behavior of the transfer domain
doesn't change.

1. Match wire origin only
   flow template_table 0 create group 0 priority 0 transfer wire_orig...
2. Match vf origin only
   flow template_table 0 create group 0 priority 0 transfer vport_orig...

Signed-off-by: Rongwei Liu 
Acked-by: Ori Kam 

v2: Move the new field to template table attribute.
v4: Mark it as optional and clear the concept.
v5: Change specialize type to uint32_t.
---
 app/test-pmd/cmdline_flow.c | 26 
 doc/guides/prog_guide/rte_flow.rst  | 15 +
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  3 +-
 lib/ethdev/rte_flow.h   | 34 +
 4 files changed, 77 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 88108498e0..15f2af9b40 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -184,6 +184,8 @@ enum index {
TABLE_INGRESS,
TABLE_EGRESS,
TABLE_TRANSFER,
+   TABLE_TRANSFER_WIRE_ORIG,
+   TABLE_TRANSFER_VPORT_ORIG,
TABLE_RULES_NUMBER,
TABLE_PATTERN_TEMPLATE,
TABLE_ACTIONS_TEMPLATE,
@@ -1158,6 +1160,8 @@ static const enum index next_table_attr[] = {
TABLE_INGRESS,
TABLE_EGRESS,
TABLE_TRANSFER,
+   TABLE_TRANSFER_WIRE_ORIG,
+   TABLE_TRANSFER_VPORT_ORIG,
TABLE_RULES_NUMBER,
TABLE_PATTERN_TEMPLATE,
TABLE_ACTIONS_TEMPLATE,
@@ -2933,6 +2937,18 @@ static const struct token token_list[] = {
.next = NEXT(next_table_attr),
.call = parse_table,
},
+   [TABLE_TRANSFER_WIRE_ORIG] = {
+   .name = "wire_orig",
+   .help = "affect rule direction to transfer",
+   .next = NEXT(next_table_attr),
+   .call = parse_table,
+   },
+   [TABLE_TRANSFER_VPORT_ORIG] = {
+   .name = "vport_orig",
+   .help = "affect rule direction to transfer",
+   .next = NEXT(next_table_attr),
+   .call = parse_table,
+   },
[TABLE_RULES_NUMBER] = {
.name = "rules_number",
.help = "number of rules in table",
@@ -8993,6 +9009,16 @@ parse_table(struct context *ctx, const struct token 
*token,
case TABLE_TRANSFER:
out->args.table.attr.flow_attr.transfer = 1;
return len;
+   case TABLE_TRANSFER_WIRE_ORIG:
+   if (!out->args.table.attr.flow_attr.transfer)
+   return -1;
+   out->args.table.attr.specialize = RTE_FLOW_TRANSFER_WIRE_ORIG;
+   return len;
+   case TABLE_TRANSFER_VPORT_ORIG:
+   if (!out->args.table.attr.flow_attr.transfer)
+   return -1;
+   out->args.table.attr.specialize = RTE_FLOW_TRANSFER_VPORT_ORIG;
+   return len;
default:
return -1;
}
diff --git a/doc/guides/prog_guide/rte_flow.rst 
b/doc/guides/prog_guide/rte_flow.rst
index 3e6242803d..d9ca041ae4 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -3605,6 +3605,21 @@ and pattern and actions templates are created.
&actions_templates, nb_actions_templ,
&error);
 
+Table Attribute: Specialize
+^^^
+
+Application can help optimizing underlayer resources and insertion rate
+by specializing template table.
+Specialization is done by providing hints
+in the template table attribute ``specialize``.
+
+This attribute is not mandatory for each PMD to implement.
+If a hint is not supported, it will be silently ignored,
+and no special optimization is done.

[PATCH v5] ethdev: add special flags when creating async transfer table

[Rongwei Liu]

The transfer domain rule is able to match traffic wire/vport
origin which are corresponding to two kinds of underlayer resources.

Wire means traffic arrives from the uplink port while vport means
traffic initiated from VF/SF.

In customer deployments, they usually match only one kind of
traffic in single flow table: either from wire or from vport.
PMD can save significant resources if passing special hint from rte
layer.

There are two possible approaches, using IPv4 as an example:
1. Use pattern item.
   pattern_template: pattern ANY_VPORT / eth / ipv4 is 1.1.1.1 / end
   async flow create: pattern ANY_VPORT / eth / ipv4 is 1.1.1.2 / end
   "ANY_VPORT" needs to be present in each async rule even if it's
   just a hint. No value to match.

2. Add special flags into table_attr. It will be:
   template_table 0 create table_id 0 group 1 transfer vf_orig

Approach 1 needs to specify the pattern in each flow rules which wastes
memory and not end user friendly.
This patch takes the 2nd approach and introduce one new member
specialize into rte_flow_table_attr to indicate async flow table matching
optimization: from wire, from vport.

It helps to save underlayer memory and also on insertion rate.

By default, there is no hint, so the behavior of the transfer domain
doesn't change.

1. Match wire origin only
   flow template_table 0 create group 0 priority 0 transfer wire_orig...
2. Match vf origin only
   flow template_table 0 create group 0 priority 0 transfer vport_orig...

Signed-off-by: Rongwei Liu 
Acked-by: Ori Kam 

v2: Move the new field to template table attribute.
v4: Mark it as optional and clear the concept.
v5: Change specialize type to uint32_t.
---
 app/test-pmd/cmdline_flow.c | 26 
 doc/guides/prog_guide/rte_flow.rst  | 15 +
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  3 +-
 lib/ethdev/rte_flow.h   | 34 +
 4 files changed, 77 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 88108498e0..15f2af9b40 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -184,6 +184,8 @@ enum index {
TABLE_INGRESS,
TABLE_EGRESS,
TABLE_TRANSFER,
+   TABLE_TRANSFER_WIRE_ORIG,
+   TABLE_TRANSFER_VPORT_ORIG,
TABLE_RULES_NUMBER,
TABLE_PATTERN_TEMPLATE,
TABLE_ACTIONS_TEMPLATE,
@@ -1158,6 +1160,8 @@ static const enum index next_table_attr[] = {
TABLE_INGRESS,
TABLE_EGRESS,
TABLE_TRANSFER,
+   TABLE_TRANSFER_WIRE_ORIG,
+   TABLE_TRANSFER_VPORT_ORIG,
TABLE_RULES_NUMBER,
TABLE_PATTERN_TEMPLATE,
TABLE_ACTIONS_TEMPLATE,
@@ -2933,6 +2937,18 @@ static const struct token token_list[] = {
.next = NEXT(next_table_attr),
.call = parse_table,
},
+   [TABLE_TRANSFER_WIRE_ORIG] = {
+   .name = "wire_orig",
+   .help = "affect rule direction to transfer",
+   .next = NEXT(next_table_attr),
+   .call = parse_table,
+   },
+   [TABLE_TRANSFER_VPORT_ORIG] = {
+   .name = "vport_orig",
+   .help = "affect rule direction to transfer",
+   .next = NEXT(next_table_attr),
+   .call = parse_table,
+   },
[TABLE_RULES_NUMBER] = {
.name = "rules_number",
.help = "number of rules in table",
@@ -8993,6 +9009,16 @@ parse_table(struct context *ctx, const struct token 
*token,
case TABLE_TRANSFER:
out->args.table.attr.flow_attr.transfer = 1;
return len;
+   case TABLE_TRANSFER_WIRE_ORIG:
+   if (!out->args.table.attr.flow_attr.transfer)
+   return -1;
+   out->args.table.attr.specialize = RTE_FLOW_TRANSFER_WIRE_ORIG;
+   return len;
+   case TABLE_TRANSFER_VPORT_ORIG:
+   if (!out->args.table.attr.flow_attr.transfer)
+   return -1;
+   out->args.table.attr.specialize = RTE_FLOW_TRANSFER_VPORT_ORIG;
+   return len;
default:
return -1;
}
diff --git a/doc/guides/prog_guide/rte_flow.rst 
b/doc/guides/prog_guide/rte_flow.rst
index 3e6242803d..d9ca041ae4 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -3605,6 +3605,21 @@ and pattern and actions templates are created.
&actions_templates, nb_actions_templ,
&error);
 
+Table Attribute: Specialize
+^^^
+
+Application can help optimizing underlayer resources and insertion rate
+by specializing template table.
+Specialization is done by providing hints
+in the template table attribute ``specialize``.
+
+This attribute is not mandatory for each PMD to implement.
+If a hint is not supported, it will be silently ignored,
+and no special optimization is done.

Re: FW: [PATCH v4 3/3] mempool: use cache for frequently updated stats

[Mattias Rönnblom]

On 2022-11-09 06:03, Morten Brørup wrote:
>> From: Konstantin Ananyev [mailto:konstantin.anan...@huawei.com]
>> Sent: Tuesday, 8 November 2022 18.38
>>>
>>> On Tue, Nov 08, 2022 at 04:51:11PM +0100, Thomas Monjalon wrote:
 08/11/2022 15:30, Morten Brørup:
>> From: Thomas Monjalon [mailto:tho...@monjalon.net]
>> 08/11/2022 12:25, Morten Brørup:
>>> From: Morten Brørup
 From: Konstantin Ananyev
>> [mailto:konstantin.anan...@huawei.com]
 Sent: Tuesday, 8 November 2022 10.20
> +#ifdef RTE_LIBRTE_MEMPOOL_STATS
> +#define RTE_MEMPOOL_CACHE_STAT_ADD(cache, name, n)
>> (cache)-
>>> https://protect2.fireeye.com/v1/url?k=31323334-501d5122-313273af-45444731-27a859e7ec13035a&q=1&e=a120e28e-caa7-4783-9686-5868c871553d&u=http%3A%2F%2Fstats.name%2F
>>>  += n

 As Andrew already pointed, it needs to be: ((cache)-
>>> https://protect2.fireeye.com/v1/url?k=31323334-501d5122-313273af-45444731-27a859e7ec13035a&q=1&e=a120e28e-caa7-4783-9686-5868c871553d&u=http%3A%2F%2Fstats.name%2F
>>>  +=
>> (n))
 Apart from that, LGTM.
 Series-Acked-by: Konstantin Ananyev
>> 
>>>
>>> @Thomas, this series should be ready to apply... it now has
>> been:
>>> Reviewed-by: (mempool maintainer) Andrew Rybchenko
>> 
>>> Reviewed-By: Mattias Rönnblom 
>>> Acked-by: Konstantin Ananyev 
>>
>> Being acked does not mean it is good to apply in -rc3.
>
> I understand that the RFC/v1 of this series was formally too late
>> to make it in 22.11, so I will not complain loudly if you choose to
>>> omit it for 22.11.
>
> With two independent reviews, including from a mempool
>> maintainer, I still have some hope. Also considering the risk
>> assessment
>>> below. ;-)
>
>> Please tell what is the benefit for 22.11 (before/after and
>> condition).
>
> Short version: With this series, mempool statistics can be used
>> in production. Without it, the performance cost
>>> (mempool_perf_autotest: -74 %) is prohibitive!
>
> Long version:
>
> The patch series provides significantly higher performance for
>> mempool statistics, which are readable through
>>> rte_mempool_dump(FILE *f, struct rte_mempool *mp).
>
> Without this series, you have to set RTE_LIBRTE_MEMPOOL_DEBUG at
>> build time to get mempool statistics.
>>> RTE_LIBRTE_MEMPOOL_DEBUG also enables protective cookies before and
>> after each mempool object, which are all verified on
>>> get/put from the mempool. According to mempool_perf_autotest, the
>> performance cost of mempool statistics (by setting
>>> RTE_LIBRTE_MEMPOOL_DEBUG) is a 74 % decrease in rate_persec for
>> mempools with cache (i.e. mbuf pools). Prohibitive for use in
>>> production!
>
> With this series, the performance cost of mempool statistics (by
>> setting RTE_LIBRTE_MEMPOOL_STATS) in
>>> mempool_perf_autotest is only 6.7 %, so mempool statistics can be
>> used in production.
>
>> Note there is a real risk doing such change that late.
>
> Risk assessment:
>
> The patch series has zero effect unless either
>> RTE_LIBRTE_MEMPOOL_DEBUG or RTE_LIBRTE_MEMPOOL_STATS are set when
>>> building. They are not set in the default build.

 If theses build flags are not set, there is no risk and no benefit.
 But if they are set, there is a risk of regression,
 for the benefit of an increased performance of a debug feature.
 I would say it is better to avoid any functional regression in a
>> debug feature
 at this stage.
 Any other opinion?

>>> While I agree that we should avoid any functional regression, I
>> wonder how
>>> widely used the debug feature is, and how big the risk of a
>> regression is?
>>> Even if there is one, having a regression in a debug feature is a lot
>> less
>>> serious than having one in something which goes into production.
>>>
>>
>> Unless it introduces an ABI breakage (as I understand it doesn't), I'll
>> wait till 23.03.
>> Just in case.
> 
> If built (both before and after this series) without RTE_LIBRTE_MEMPOOL_DEBUG 
> (and without RTE_LIBRTE_MEMPOOL_STATS, which is introduced by the series), 
> there is no ABI breakage.
> 
> If built (both before and after this series) with RTE_LIBRTE_MEMPOOL_DEBUG 
> (and without RTE_LIBRTE_MEMPOOL_STATS), the ABI differs between before and 
> after this series: The stats array disappears from struct rte_mempool, and 
> the output from rte_mempool_dump() does not include the statistics.
> 
> If built (both before and after this series) with RTE_LIBRTE_MEMPOOL_DEBUG 
> (and with RTE_LIBRTE_MEMPOOL_STATS), the ABI also differs between before and 
> after this series: The size of the stats array in struct rte_mempool grows by 
> one element.
> 
>> BTW, as a side thought - if the impact is really that small now, would
>> it make sense to make
>> it run-time option, instead of compile-time one?
> 
> The mempool get/put functions are very lean w

RE: [PATCH] net/mlx5: fix the first segment inline length

[Raslan Darawsheh]

Hi,

> -Original Message-
> From: Alexander Kozyrev 
> Sent: Tuesday, November 8, 2022 3:45 PM
> To: dev@dpdk.org
> Cc: sta...@dpdk.org; Raslan Darawsheh ; Slava
> Ovsiienko ; Matan Azrad ;
> Michael Baum 
> Subject: [PATCH] net/mlx5: fix the first segment inline length
> 
> Packets can be split into several mbufs with various data sizes.
> There is no limitation on how small these segments can be.
> But there is a limitation on Tx side for inline configuration:
> send WQEs with inline headers less than the required are dropped.
> The very first segment must be more than minimal inline eth segment.
> Enforce this requirement by merging a few segments in this case.
> 
> Fixes: ec837ad0fc7 ("net/mlx5: fix multi-segment inline for the first
> segments")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Alexander Kozyrev 

Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh

RE: [PATCH] net/mlx5: remove unneeded GENEVE option length assert

[Raslan Darawsheh]

Hi,

> -Original Message-
> From: Suanming Mou 
> Sent: Wednesday, November 9, 2022 3:40 AM
> To: Matan Azrad ; Slava Ovsiienko
> 
> Cc: dev@dpdk.org; Raslan Darawsheh 
> Subject: [PATCH] net/mlx5: remove unneeded GENEVE option length assert
> 
> In order to share flow items translate code, flow items translation of spec
> and mask was split individually.
> 
> In that case, the assert for GENEVE option length with mask becomes invalid,
> since the length in mask is bitmask. And as memcpy around the assert
> already checks the GENEVE option length, the assert looks redundant.
> 
> This commit removes the unneeded GENEVE option length assert.
> 
> Fixes: cd4ab742064a ("net/mlx5: split flow item matcher and value
> translation")
> 
> Signed-off-by: Suanming Mou 

Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh

Re: [PATCH v5] ethdev: add special flags when creating async transfer table

[Thomas Monjalon]

09/11/2022 09:13, Rongwei Liu:
> v2: Move the new field to template table attribute.
> v4: Mark it as optional and clear the concept.
> v5: Change specialize type to uint32_t.

There are more changes to do (replace enum with defines and update the commit 
message).

Re: [PATCH v4] ethdev: add special flags when creating async transfer table

[Andrew Rybchenko]

On 11/8/22 18:25, Thomas Monjalon wrote:

08/11/2022 15:38, Andrew Rybchenko:

On 11/8/22 16:29, Thomas Monjalon wrote:

08/11/2022 12:47, Andrew Rybchenko:

On 11/8/22 14:39, Andrew Rybchenko wrote:

On 11/4/22 13:44, Rongwei Liu wrote:

diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index 8858b56428..1eab12796f 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -5186,6 +5186,34 @@ rte_flow_actions_template_destroy(uint16_t
port_id,
 */
struct rte_flow_template_table;
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Special optional flags for template table attribute.
+ * Each bit stands for a table specialization
+ * offering a potential optimization at PMD layer.
+ * PMD can ignore the unsupported bits silently.
+ */
+enum rte_flow_template_table_specialize {
+/**
+ * Specialize table for transfer flows which come only from wire.
+ * It allows PMD not to allocate resources for non-wire
originated traffic.
+ * This bit is not a matching criteria, just an optimization hint.
+ * Flow rules which match non-wire originated traffic will be missed
+ * if the hint is supported.


Sorry, but if so, the hint changes behavior.


Yes the hint may change behaviour.


Let's consider a rule which matches both VF originating and
wire originating traffic. Will the rule be missed (ignored)
regardless if the hint is supported or not?


If the hint RTE_FLOW_TRANSFER_WIRE_ORIG is used,
the PMD may assume the table won't be used for traffic
which is not coming from wire ports.
As a consequence, the table may be implemented on the path
of wire traffic only.
In this case, the traffic coming from virtual ports
won't be affected by this table.
To answer the question, a rule matching both virtual and wire traffic
will be applied in a table affecting only wire traffic,
so it will still apply (not completely ignored).


If so, it is not a hint. It becomes matching criteria
which should be in pattern as we discussed.


It is not a strict matching because the PMD is free to support it or not.


It cannot be optional matching criteria. Matching criteria must
be always mandatory. Otherwise application does not know what
to expect and behaviour may legitimately vary on different
vendors.

RE: [PATCH 1/2] test/hash: fix coverity warning

[Ruifeng Wang]

> -Original Message-
> From: Vladimir Medvedkin 
> Sent: Friday, November 4, 2022 2:13 AM
> To: dev@dpdk.org
> Cc: Dharmik Thakkar ; sta...@dpdk.org; Yipeng Wang
> ; Sameh Gobriel ; Bruce 
> Richardson
> 
> Subject: [PATCH 1/2] test/hash: fix coverity warning
> 
> Remove unnecessary variable assignment
> 
> Coverity issue: 336800
> Fixes: 3f9aab961ed3 ("test/hash: check lock-free extendable bucket")
> Cc: dharmik.thak...@arm.com
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Vladimir Medvedkin 
> ---
>  app/test/test_hash_readwrite_lf_perf.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/app/test/test_hash_readwrite_lf_perf.c
> b/app/test/test_hash_readwrite_lf_perf.c
> index 32f9ec9250..cf86046a2f 100644
> --- a/app/test/test_hash_readwrite_lf_perf.c
> +++ b/app/test/test_hash_readwrite_lf_perf.c
> @@ -1102,7 +1102,6 @@ test_hash_multi_add_lookup(struct rwc_perf 
> *rwc_perf_results, int
> rwc_lf,
>   rte_eal_remote_launch(test_rwc_reader,
>   (void *)(uintptr_t)read_type,
>   enabled_core_ids[i]);
> - write_type = WRITE_KEY_SHIFT;
>   pos_core = 0;
> 
>   /* Launch writers */
> --
> 2.25.1
Reviewed-by: Ruifeng Wang 

Re: [PATCH v4] ethdev: add special flags when creating async transfer table

[Thomas Monjalon]

09/11/2022 09:53, Andrew Rybchenko:
> On 11/8/22 18:25, Thomas Monjalon wrote:
> > 08/11/2022 15:38, Andrew Rybchenko:
> >> On 11/8/22 16:29, Thomas Monjalon wrote:
> >>> 08/11/2022 12:47, Andrew Rybchenko:
>  On 11/8/22 14:39, Andrew Rybchenko wrote:
> > On 11/4/22 13:44, Rongwei Liu wrote:
> >> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> >> index 8858b56428..1eab12796f 100644
> >> --- a/lib/ethdev/rte_flow.h
> >> +++ b/lib/ethdev/rte_flow.h
> >> @@ -5186,6 +5186,34 @@ rte_flow_actions_template_destroy(uint16_t
> >> port_id,
> >>  */
> >> struct rte_flow_template_table;
> >> +/**
> >> + * @warning
> >> + * @b EXPERIMENTAL: this API may change without prior notice.
> >> + *
> >> + * Special optional flags for template table attribute.
> >> + * Each bit stands for a table specialization
> >> + * offering a potential optimization at PMD layer.
> >> + * PMD can ignore the unsupported bits silently.
> >> + */
> >> +enum rte_flow_template_table_specialize {
> >> +/**
> >> + * Specialize table for transfer flows which come only from wire.
> >> + * It allows PMD not to allocate resources for non-wire
> >> originated traffic.
> >> + * This bit is not a matching criteria, just an optimization hint.
> >> + * Flow rules which match non-wire originated traffic will be 
> >> missed
> >> + * if the hint is supported.
> 
>  Sorry, but if so, the hint changes behavior.
> >>>
> >>> Yes the hint may change behaviour.
> >>>
>  Let's consider a rule which matches both VF originating and
>  wire originating traffic. Will the rule be missed (ignored)
>  regardless if the hint is supported or not?
> >>>
> >>> If the hint RTE_FLOW_TRANSFER_WIRE_ORIG is used,
> >>> the PMD may assume the table won't be used for traffic
> >>> which is not coming from wire ports.
> >>> As a consequence, the table may be implemented on the path
> >>> of wire traffic only.
> >>> In this case, the traffic coming from virtual ports
> >>> won't be affected by this table.
> >>> To answer the question, a rule matching both virtual and wire traffic
> >>> will be applied in a table affecting only wire traffic,
> >>> so it will still apply (not completely ignored).
> >>
> >> If so, it is not a hint. It becomes matching criteria
> >> which should be in pattern as we discussed.
> > 
> > It is not a strict matching because the PMD is free to support it or not.
> 
> It cannot be optional matching criteria. Matching criteria must
> be always mandatory. Otherwise application does not know what
> to expect and behaviour may legitimately vary on different
> vendors.

I think you take it in the wrong direction.
The idea is not to have it as a criteria.
Let me explain again:

If an application is using a flow table to manage flows
which *always* come from the same type of port (wire or virtual),
then the application can give this information to the driver.
With this assumption coming from the application,
the driver may do some optimizations.

Now about what is explained above:
If the application gives such a hint
but does not respect its own assumption,
then confusion happens.

Re: [PATCH v4] ethdev: add special flags when creating async transfer table

[Andrew Rybchenko]

On 11/9/22 12:03, Thomas Monjalon wrote:

09/11/2022 09:53, Andrew Rybchenko:

On 11/8/22 18:25, Thomas Monjalon wrote:

08/11/2022 15:38, Andrew Rybchenko:

On 11/8/22 16:29, Thomas Monjalon wrote:

08/11/2022 12:47, Andrew Rybchenko:

On 11/8/22 14:39, Andrew Rybchenko wrote:

On 11/4/22 13:44, Rongwei Liu wrote:

diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index 8858b56428..1eab12796f 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -5186,6 +5186,34 @@ rte_flow_actions_template_destroy(uint16_t
port_id,
  */
 struct rte_flow_template_table;
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Special optional flags for template table attribute.
+ * Each bit stands for a table specialization
+ * offering a potential optimization at PMD layer.
+ * PMD can ignore the unsupported bits silently.
+ */
+enum rte_flow_template_table_specialize {
+/**
+ * Specialize table for transfer flows which come only from wire.
+ * It allows PMD not to allocate resources for non-wire
originated traffic.
+ * This bit is not a matching criteria, just an optimization hint.
+ * Flow rules which match non-wire originated traffic will be missed
+ * if the hint is supported.


Sorry, but if so, the hint changes behavior.


Yes the hint may change behaviour.


Let's consider a rule which matches both VF originating and
wire originating traffic. Will the rule be missed (ignored)
regardless if the hint is supported or not?


If the hint RTE_FLOW_TRANSFER_WIRE_ORIG is used,
the PMD may assume the table won't be used for traffic
which is not coming from wire ports.
As a consequence, the table may be implemented on the path
of wire traffic only.
In this case, the traffic coming from virtual ports
won't be affected by this table.
To answer the question, a rule matching both virtual and wire traffic
will be applied in a table affecting only wire traffic,
so it will still apply (not completely ignored).


If so, it is not a hint. It becomes matching criteria
which should be in pattern as we discussed.


It is not a strict matching because the PMD is free to support it or not.


It cannot be optional matching criteria. Matching criteria must
be always mandatory. Otherwise application does not know what
to expect and behaviour may legitimately vary on different
vendors.


I think you take it in the wrong direction.
The idea is not to have it as a criteria.
Let me explain again:

If an application is using a flow table to manage flows
which *always* come from the same type of port (wire or virtual),


What does guarantee it? Is it used a jump-table and jump rule
must guarantee it? Or has pattern corresponding unit?

It is very thin ice and I'm ready to bet money that finally
it will be used as a matching criteria intentionally or not
intentionally. Simply because it works as matching criteria
on, for example, Mellanox. I.e. if rules from table with
corresponding hint are programmed to HW which applies these
rules on traffic from wire only - effectively it is a matching
criteria. And it will be used this way. And it will be not
portable to other HW which does not support the hint.
So, we're making an API which is very easy to misuse if not
to say more.

You know better if it is OK or not to rely on liable users
in the case of DPDK.

It would be much safer if we do not rely on application in this
case, introduce a new pattern item to specify origin and
require PMD to check that pattern has either a new pattern item
or corresponding  REPRESENTED_PORT/PORT_REPRESENTOR pattern
item.

I realize that my concerns could be not valid and it is just
a paranoia. Just add your ack and let's move forward.


then the application can give this information to the driver.
With this assumption coming from the application,
the driver may do some optimizations.

Now about what is explained above:
If the application gives such a hint
but does not respect its own assumption,
then confusion happens.

[PATCH] examples/ipsec-secgw: fix uninitialized variable access

[Volodymyr Fialko]

Fix uninitialized variable access of outbound offloads flags.

Coverity issue: 381669
Fixes: 6938fc92c404 ("examples/ipsec-secgw: add lookaside event mode")

Signed-off-by: Volodymyr Fialko 
---
 examples/ipsec-secgw/ipsec-secgw.c  | 19 +++
 examples/ipsec-secgw/ipsec.h|  7 +++
 examples/ipsec-secgw/ipsec_worker.c |  2 ++
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/examples/ipsec-secgw/ipsec-secgw.c 
b/examples/ipsec-secgw/ipsec-secgw.c
index 24d895451a..a64a26c992 100644
--- a/examples/ipsec-secgw/ipsec-secgw.c
+++ b/examples/ipsec-secgw/ipsec-secgw.c
@@ -105,6 +105,8 @@ struct ethaddr_info ethaddr_tbl[RTE_MAX_ETHPORTS] = {
{ 0, ETHADDR(0x00, 0x16, 0x3e, 0x49, 0x9e, 0xdd) }
 };
 
+struct offloads tx_offloads;
+
 /*
  * To hold ethernet header per port, which will be applied
  * to outgoing packets.
@@ -3017,16 +3019,17 @@ main(int32_t argc, char **argv)
ipv4_cksum_port_mask |= 1U << portid;
}
 
-   for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
-   if (rte_lcore_is_enabled(lcore_id) == 0)
-   continue;
+   tx_offloads.ipv4_offloads = RTE_MBUF_F_TX_IPV4;
+   tx_offloads.ipv6_offloads = RTE_MBUF_F_TX_IPV6;
+   /* Update per lcore checksum offload support only if all ports support 
it */
+   if (ipv4_cksum_port_mask == enabled_port_mask)
+   tx_offloads.ipv4_offloads |= RTE_MBUF_F_TX_IP_CKSUM;
 
+   lcore_id = 0;
+   RTE_LCORE_FOREACH(lcore_id) {
/* Pre-populate pkt offloads based on capabilities */
-   lcore_conf[lcore_id].outbound.ipv4_offloads = 
RTE_MBUF_F_TX_IPV4;
-   lcore_conf[lcore_id].outbound.ipv6_offloads = 
RTE_MBUF_F_TX_IPV6;
-   /* Update per lcore checksum offload support only if all ports 
support it */
-   if (ipv4_cksum_port_mask == enabled_port_mask)
-   lcore_conf[lcore_id].outbound.ipv4_offloads |= 
RTE_MBUF_F_TX_IP_CKSUM;
+   lcore_conf[lcore_id].outbound.ipv4_offloads = 
tx_offloads.ipv4_offloads;
+   lcore_conf[lcore_id].outbound.ipv6_offloads = 
tx_offloads.ipv6_offloads;
}
 
/*
diff --git a/examples/ipsec-secgw/ipsec.h b/examples/ipsec-secgw/ipsec.h
index a21402ef5f..6bef2a7285 100644
--- a/examples/ipsec-secgw/ipsec.h
+++ b/examples/ipsec-secgw/ipsec.h
@@ -242,6 +242,13 @@ struct ipsec_ctx {
uint32_t lcore_id;
 };
 
+struct offloads {
+   uint64_t ipv4_offloads;
+   uint64_t ipv6_offloads;
+};
+
+extern struct offloads tx_offloads;
+
 struct cdev_key {
uint16_t lcore_id;
uint8_t cipher_algo;
diff --git a/examples/ipsec-secgw/ipsec_worker.c 
b/examples/ipsec-secgw/ipsec_worker.c
index cbb41bc192..2f02946f86 100644
--- a/examples/ipsec-secgw/ipsec_worker.c
+++ b/examples/ipsec-secgw/ipsec_worker.c
@@ -1342,6 +1342,8 @@ ipsec_wrkr_non_burst_int_port_app_mode(struct 
eh_event_link_info *links,
lconf.outbound.sp4_ctx = socket_ctx[socket_id].sp_ip4_out;
lconf.outbound.sp6_ctx = socket_ctx[socket_id].sp_ip6_out;
lconf.outbound.sa_ctx = socket_ctx[socket_id].sa_out;
+   lconf.outbound.ipv4_offloads = tx_offloads.ipv4_offloads;
+   lconf.outbound.ipv6_offloads = tx_offloads.ipv6_offloads;
lconf.outbound.lcore_id = lcore_id;
 
RTE_LOG(INFO, IPSEC,
-- 
2.25.1

RE: [PATCH] net/mlx5: fix assert when creating meter policy

[Raslan Darawsheh]

Hi,

> -Original Message-
> From: Shun Hao 
> Sent: Wednesday, November 9, 2022 9:53 AM
> To: Slava Ovsiienko ; Matan Azrad
> ; Ori Kam ; Jiawei(Jonny) Wang
> 
> Cc: dev@dpdk.org; Raslan Darawsheh ;
> sta...@dpdk.org
> Subject: [PATCH] net/mlx5: fix assert when creating meter policy
> 
> When creating meter policy rules, it's possible to use flow items
> translation to add src port match criteria. Currently the items
> translation process needs to get thread workspace to store vport
> metadata tag, but in policy creation, the thread workspace was not
> initialized so it will cause assert failure.
> 
> This patch adds initialization of thread-local workspace when creating
> meter policy rules to avoid that assert.
> 
> Fixes: 65c86202 ("net/mlx5: fix flow source port checking in sample flow
> rule")
> CC: sta...@dpdk.org
> 
> Signed-off-by: Shun Hao 
> Acked-by: Matan Azrad 

Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh

[PATCH 0/2] net/mlx5/hws: fix matcher clean up for FDB tables

[Dariusz Sosnowski]

Before these patches, if an application was configured to run with
HW Steering and E-Switch enabled, on EAL cleanup the assertion in
mlx5_dev_hw_global_release() was triggered - PD release was unsuccessful.

Root cause of this issue was linked to an inability to destroy RTC objects
used internally in mlx5, in HW Steering implementation.
PMD was unable to destroy RTC objects, because of dangling
references to those objects. More specifically, if all matchers
connected to a single flow table were created, this flow table
was still referencing RTC objects when theye were being destroyed.

This patch series fixes that behavior.
Matcher uninitilization is updated to remove the references to RTC objects
from flow table object if the last matcher related to the flow
table was destroyed.

Erez Shitrit (2):
  net/mlx5/hws: fix order of destroying default tables
  net/mlx5/hws: fix disconnecting matcher

 drivers/net/mlx5/hws/mlx5dr_matcher.c | 35 +++
 drivers/net/mlx5/hws/mlx5dr_table.c   |  2 +-
 2 files changed, 36 insertions(+), 1 deletion(-)

-- 
2.25.1

[PATCH 1/2] net/mlx5/hws: fix order of destroying default tables

[Dariusz Sosnowski]

From: Erez Shitrit 

This patch fixes the order dereferencing default FDB miss table and
destroying the flow table object. Flow table should be destroyed
before the dereference.

Fixes: 394cc7ba4033 ("net/mlx5/hws: add table object")
Cc: va...@nvidia.com

Signed-off-by: Erez Shitrit 
Signed-off-by: Dariusz Sosnowski 
Reviewed-by: Alex Vesker 
---
 drivers/net/mlx5/hws/mlx5dr_table.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/hws/mlx5dr_table.c 
b/drivers/net/mlx5/hws/mlx5dr_table.c
index d3f77e4780..33052abce8 100644
--- a/drivers/net/mlx5/hws/mlx5dr_table.c
+++ b/drivers/net/mlx5/hws/mlx5dr_table.c
@@ -131,8 +131,8 @@ mlx5dr_table_create_default_ft(struct mlx5dr_table *tbl)
 void mlx5dr_table_destroy_default_ft(struct mlx5dr_table *tbl,
 struct mlx5dr_devx_obj *ft_obj)
 {
-   mlx5dr_table_down_default_fdb_miss_tbl(tbl);
mlx5dr_cmd_destroy_obj(ft_obj);
+   mlx5dr_table_down_default_fdb_miss_tbl(tbl);
 }
 
 static int mlx5dr_table_init(struct mlx5dr_table *tbl)
-- 
2.25.1

[PATCH 2/2] net/mlx5/hws: fix disconnecting matcher

[Dariusz Sosnowski]

From: Erez Shitrit 

This patch fixes the matcher disconnection handling, by removing the RTC
references from flow table if the currently removed matcher was the last
one for the given table. As a result RTC in this matcher can be
correctly freed, since there are no dangling references to the RTC.

Fixes: c467608215b2 ("net/mlx5/hws: add matcher object")
Cc: va...@nvidia.com

Signed-off-by: Erez Shitrit 
Signed-off-by: Dariusz Sosnowski 
Reviewed-by: Alex Vesker 
---
 drivers/net/mlx5/hws/mlx5dr_matcher.c | 35 +++
 1 file changed, 35 insertions(+)

diff --git a/drivers/net/mlx5/hws/mlx5dr_matcher.c 
b/drivers/net/mlx5/hws/mlx5dr_matcher.c
index 67adfeec6c..2e444c1179 100644
--- a/drivers/net/mlx5/hws/mlx5dr_matcher.c
+++ b/drivers/net/mlx5/hws/mlx5dr_matcher.c
@@ -36,6 +36,30 @@ static void mlx5dr_matcher_destroy_end_ft(struct 
mlx5dr_matcher *matcher)
mlx5dr_table_destroy_default_ft(matcher->tbl, matcher->end_ft);
 }
 
+static int mlx5dr_matcher_free_rtc_pointing(uint32_t fw_ft_type,
+   enum mlx5dr_table_type type,
+   struct mlx5dr_devx_obj *devx_obj)
+{
+   struct mlx5dr_cmd_ft_modify_attr ft_attr = {0};
+   int ret;
+
+   if (type != MLX5DR_TABLE_TYPE_FDB)
+   return 0;
+
+   ft_attr.modify_fs = MLX5_IFC_MODIFY_FLOW_TABLE_RTC_ID;
+   ft_attr.type = fw_ft_type;
+   ft_attr.rtc_id_0 = 0;
+   ft_attr.rtc_id_1 = 0;
+
+   ret = mlx5dr_cmd_flow_table_modify(devx_obj, &ft_attr);
+   if (ret) {
+   DR_LOG(ERR, "Failed to disconnect previous RTC");
+   return ret;
+   }
+
+   return 0;
+}
+
 static int mlx5dr_matcher_connect(struct mlx5dr_matcher *matcher)
 {
struct mlx5dr_cmd_ft_modify_attr ft_attr = {0};
@@ -148,6 +172,17 @@ static int mlx5dr_matcher_disconnect(struct mlx5dr_matcher 
*matcher)
 
LIST_REMOVE(matcher, next);
 
+   if (!next) {
+   /* ft no longer points to any RTC, drop refcount */
+   ret = mlx5dr_matcher_free_rtc_pointing(tbl->fw_ft_type,
+  tbl->type,
+  prev_ft);
+   if (ret) {
+   DR_LOG(ERR, "Failed to reset last RTC refcount");
+   return ret;
+   }
+   }
+
return 0;
 }
 
-- 
2.25.1

RE: [PATCH v2] net/mlx5/hws: fix timestamp format on Tx queue creation

[Raslan Darawsheh]

Hi,

> -Original Message-
> From: Slava Ovsiienko 
> Sent: Wednesday, November 9, 2022 9:58 AM
> To: dev@dpdk.org
> Cc: Matan Azrad ; Raslan Darawsheh
> ; Alex Vesker 
> Subject: [PATCH v2] net/mlx5/hws: fix timestamp format on Tx queue
> creation
> 
> The NIC since 6DX supports multiple timestamp formats
> in CQEs configured via firmware. If real time timestamp
> format has been configured the correct attributes should
> be specified on queue creation via DevX. These attributes
> setting was missed on steering queue creation and hardware
> steering initialization failed.
> 
> Fixes: 3eb748869d2d ("net/mlx5/hws: add send layer")
> 
> Signed-off-by: Viacheslav Ovsiienko 
> ---
>  drivers/net/mlx5/hws/mlx5dr_cmd.c  | 4 
>  drivers/net/mlx5/hws/mlx5dr_cmd.h  | 2 ++
>  drivers/net/mlx5/hws/mlx5dr_send.c | 4 
>  3 files changed, 10 insertions(+)
> 
> v1:
>   - http://patches.dpdk.org/project/dpdk/patch/20221108185650.15489-1-
> viachesl...@nvidia.com/
> v2:
>   - minor style changes
>   - removed unnessesary variables
> 

Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh

RE: [PATCH v2] testpmd: make f_quit flag volatile

[Ruifeng Wang]

> -Original Message-
> From: Stephen Hemminger 
> Sent: Wednesday, November 9, 2022 2:08 AM
> To: dev@dpdk.org
> Cc: Phil Yang ; Stephen Hemminger 
> 
> Subject: [PATCH v2] testpmd: make f_quit flag volatile
> 
> Since f_quit is set in a signal handler it needs to be marked volatile.  
> Otherwise,
> compiler is allowed to optimize the loop because it can assume the value 
> never changes.
> The flag can also be made local to the file it is used in.
> 
> Fixes: d9a191a00e81 ("app/testpmd: fix quitting in container")
> Signed-off-by: Stephen Hemminger 
> ---
> v2 - not RFC and add fixes line
> 
>  app/test-pmd/testpmd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> aa7ea29f15ba..cf5942d0c422 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -231,7 +231,7 @@ unsigned int xstats_display_num; /**< Size of extended 
> statistics to
> show */
>   * In container, it cannot terminate the process which running with 
> 'stats-period'
>   * option. Set flag to exit stats period loop after received SIGINT/SIGTERM.
>   */
> -uint8_t f_quit;
> +static volatile uint8_t f_quit;
>  uint8_t cl_quit; /* Quit testpmd from cmdline. */
> 
>  /*
> --
> 2.35.1
Thanks for the change.
Reviewed-by: Ruifeng Wang 

RE: FW: [PATCH v4 3/3] mempool: use cache for frequently updated stats

[Konstantin Ananyev]

> On 2022-11-09 06:03, Morten Brørup wrote:
> >> From: Konstantin Ananyev [mailto:konstantin.anan...@huawei.com]
> >> Sent: Tuesday, 8 November 2022 18.38
> >>>
> >>> On Tue, Nov 08, 2022 at 04:51:11PM +0100, Thomas Monjalon wrote:
>  08/11/2022 15:30, Morten Brørup:
> >> From: Thomas Monjalon [mailto:tho...@monjalon.net]
> >> 08/11/2022 12:25, Morten Brørup:
> >>> From: Morten Brørup
>  From: Konstantin Ananyev
> >> [mailto:konstantin.anan...@huawei.com]
>  Sent: Tuesday, 8 November 2022 10.20
> > +#ifdef RTE_LIBRTE_MEMPOOL_STATS
> > +#define RTE_MEMPOOL_CACHE_STAT_ADD(cache, name, n)
> >> (cache)-
> >>> https://protect2.fireeye.com/v1/url?k=31323334-501d5122-313273af-45444731-27a859e7ec13035a&q=1&e=a120e28e-
> caa7-4783-9686-5868c871553d&u=http%3A%2F%2Fstats.name%2F += n
> 
>  As Andrew already pointed, it needs to be: ((cache)-
> >>> https://protect2.fireeye.com/v1/url?k=31323334-501d5122-313273af-45444731-27a859e7ec13035a&q=1&e=a120e28e-caa7-
> 4783-9686-5868c871553d&u=http%3A%2F%2Fstats.name%2F +=
> >> (n))
>  Apart from that, LGTM.
>  Series-Acked-by: Konstantin Ananyev
> >> 
> >>>
> >>> @Thomas, this series should be ready to apply... it now has
> >> been:
> >>> Reviewed-by: (mempool maintainer) Andrew Rybchenko
> >> 
> >>> Reviewed-By: Mattias Rönnblom 
> >>> Acked-by: Konstantin Ananyev 
> >>
> >> Being acked does not mean it is good to apply in -rc3.
> >
> > I understand that the RFC/v1 of this series was formally too late
> >> to make it in 22.11, so I will not complain loudly if you choose to
> >>> omit it for 22.11.
> >
> > With two independent reviews, including from a mempool
> >> maintainer, I still have some hope. Also considering the risk
> >> assessment
> >>> below. ;-)
> >
> >> Please tell what is the benefit for 22.11 (before/after and
> >> condition).
> >
> > Short version: With this series, mempool statistics can be used
> >> in production. Without it, the performance cost
> >>> (mempool_perf_autotest: -74 %) is prohibitive!
> >
> > Long version:
> >
> > The patch series provides significantly higher performance for
> >> mempool statistics, which are readable through
> >>> rte_mempool_dump(FILE *f, struct rte_mempool *mp).
> >
> > Without this series, you have to set RTE_LIBRTE_MEMPOOL_DEBUG at
> >> build time to get mempool statistics.
> >>> RTE_LIBRTE_MEMPOOL_DEBUG also enables protective cookies before and
> >> after each mempool object, which are all verified on
> >>> get/put from the mempool. According to mempool_perf_autotest, the
> >> performance cost of mempool statistics (by setting
> >>> RTE_LIBRTE_MEMPOOL_DEBUG) is a 74 % decrease in rate_persec for
> >> mempools with cache (i.e. mbuf pools). Prohibitive for use in
> >>> production!
> >
> > With this series, the performance cost of mempool statistics (by
> >> setting RTE_LIBRTE_MEMPOOL_STATS) in
> >>> mempool_perf_autotest is only 6.7 %, so mempool statistics can be
> >> used in production.
> >
> >> Note there is a real risk doing such change that late.
> >
> > Risk assessment:
> >
> > The patch series has zero effect unless either
> >> RTE_LIBRTE_MEMPOOL_DEBUG or RTE_LIBRTE_MEMPOOL_STATS are set when
> >>> building. They are not set in the default build.
> 
>  If theses build flags are not set, there is no risk and no benefit.
>  But if they are set, there is a risk of regression,
>  for the benefit of an increased performance of a debug feature.
>  I would say it is better to avoid any functional regression in a
> >> debug feature
>  at this stage.
>  Any other opinion?
> 
> >>> While I agree that we should avoid any functional regression, I
> >> wonder how
> >>> widely used the debug feature is, and how big the risk of a
> >> regression is?
> >>> Even if there is one, having a regression in a debug feature is a lot
> >> less
> >>> serious than having one in something which goes into production.
> >>>
> >>
> >> Unless it introduces an ABI breakage (as I understand it doesn't), I'll
> >> wait till 23.03.
> >> Just in case.
> >
> > If built (both before and after this series) without 
> > RTE_LIBRTE_MEMPOOL_DEBUG (and without RTE_LIBRTE_MEMPOOL_STATS,
> which is introduced by the series), there is no ABI breakage.
> >
> > If built (both before and after this series) with RTE_LIBRTE_MEMPOOL_DEBUG 
> > (and without RTE_LIBRTE_MEMPOOL_STATS), the
> ABI differs between before and after this series: The stats array disappears 
> from struct rte_mempool, and the output from
> rte_mempool_dump() does not include the statistics.
> >

Can we probably always enable RTE_LIBRTE_MEMPOOL_STATS when 
RTE_LIBRTE_MEMPOOL_DEBUG is on? 

> > If built (both before and after this series) with RTE_LIBRTE_MEMPOOL_DEBUG 
> > (and with RTE_LIBRTE_MEMPOOL_STATS), the ABI
> also differs between before and

RE: [PATCH 0/2] net/mlx5/hws: fix matcher clean up for FDB tables

[Matan Azrad]

From: Dariusz Sosnowski
 
> Before these patches, if an application was configured to run with HW
> Steering and E-Switch enabled, on EAL cleanup the assertion in
> mlx5_dev_hw_global_release() was triggered - PD release was unsuccessful.
> 
> Root cause of this issue was linked to an inability to destroy RTC objects 
> used
> internally in mlx5, in HW Steering implementation.
> PMD was unable to destroy RTC objects, because of dangling references to
> those objects. More specifically, if all matchers connected to a single flow
> table were created, this flow table was still referencing RTC objects when
> theye were being destroyed.
> 
> This patch series fixes that behavior.
> Matcher uninitilization is updated to remove the references to RTC objects
> from flow table object if the last matcher related to the flow table was
> destroyed.
> 
> Erez Shitrit (2):
>   net/mlx5/hws: fix order of destroying default tables
>   net/mlx5/hws: fix disconnecting matcher
> 
>  drivers/net/mlx5/hws/mlx5dr_matcher.c | 35
> +++
>  drivers/net/mlx5/hws/mlx5dr_table.c   |  2 +-
>  2 files changed, 36 insertions(+), 1 deletion(-)

Series-acked-by: Matan Azrad 

> --
> 2.25.1

Re: [PATCH v2] testpmd: make f_quit flag volatile

[Andrew Rybchenko]

On 11/9/22 13:11, Ruifeng Wang wrote:

-Original Message-
From: Stephen Hemminger 
Sent: Wednesday, November 9, 2022 2:08 AM
To: dev@dpdk.org
Cc: Phil Yang ; Stephen Hemminger 

Subject: [PATCH v2] testpmd: make f_quit flag volatile

Since f_quit is set in a signal handler it needs to be marked volatile.  
Otherwise,
compiler is allowed to optimize the loop because it can assume the value never 
changes.
The flag can also be made local to the file it is used in.

Fixes: d9a191a00e81 ("app/testpmd: fix quitting in container")
Signed-off-by: Stephen Hemminger 
---
v2 - not RFC and add fixes line

  app/test-pmd/testpmd.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
aa7ea29f15ba..cf5942d0c422 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -231,7 +231,7 @@ unsigned int xstats_display_num; /**< Size of extended 
statistics to
show */
   * In container, it cannot terminate the process which running with 
'stats-period'
   * option. Set flag to exit stats period loop after received SIGINT/SIGTERM.
   */
-uint8_t f_quit;
+static volatile uint8_t f_quit;
  uint8_t cl_quit; /* Quit testpmd from cmdline. */

  /*
--
2.35.1

Thanks for the change.
Reviewed-by: Ruifeng Wang 



Applied to dpdk-next-net/main, thanks.

Re: [PATCH V2] net/bonding: fix bond3 and bond4 process mbuf fast free

[Andrew Rybchenko]

On 11/9/22 06:24, Stephen Hemminger wrote:

On Wed, 9 Nov 2022 10:22:37 +0800
Huisong Li  wrote:


The RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE offload can't be used in bonding
mode Broadcast and mode 8023AD. Currently, bonding driver forcibly removes
from the dev->data->dev_conf.txmode.offloads and processes as success in
bond_ethdev_configure(). But this still cause that rte_eth_dev_configure()
fails to execute because of the failure of validating Tx offload in the
eth_dev_validate_offloads(). So this patch moves the modification of txmode
offlaods to the stage of adding slave device to report the correct txmode
offloads.

Fixes: 18c41457cbae ("net/bonding: fix mbuf fast free usage")
Cc: sta...@dpdk.org

Signed-off-by: Huisong Li 


Looks good thanks.
Acked-by: Stephen Hemminger 


Applied to dpdk-next-net/main, thanks.

Re: [PATCH v12 1/1] app/testpmd: support multiple mbuf pools per Rx queue

[Andrew Rybchenko]

On 11/9/22 11:04, Singh, Aman Deep wrote:

On 11/7/2022 11:01 AM, Hanumanth Pothula wrote:

Some of the HW has support for choosing memory pools based on
the packet's size. The pool sort capability allows PMD/NIC to
choose a memory pool based on the packet's length.

On multiple mempool support enabled, populate mempool array
accordingly. Also, print pool name on which packet is received.

Signed-off-by: Hanumanth Pothula 


Acked-by: Aman Singh 


Applied to dpdk-next-net/main, thanks.

[Bug 1123] [dpdk-22.11][ASan Test] the stack-buffer-overflow was found when quit testpmd in Redhat9

[bugzilla]

https://bugs.dpdk.org/show_bug.cgi?id=1123

Bug ID: 1123
   Summary: [dpdk-22.11][ASan Test] the stack-buffer-overflow was
found when quit testpmd in Redhat9
   Product: DPDK
   Version: 22.11
  Hardware: x86
OS: Linux
Status: UNCONFIRMED
  Severity: normal
  Priority: Normal
 Component: testpmd
  Assignee: dev@dpdk.org
  Reporter: zhiminx.hu...@intel.com
  Target Milestone: ---

Environment：
DPDK:DPDK22.11
HW:Intel(R) Xeon(R) Gold 6139 CPU @ 2.30GHz
OS:Red Hat Enterprise Linux release 9.0/5.14.0-70.13.1.el9_0.x86_64
gcc:gcc version 11.2.1 20220127 (Red Hat 11.2.1-9) (GCC)
NIC:Intel Corporation Ethernet Controller E810-C for QSFP [8086:1592]
driver: ice
version: 1.10.1
firmware-version: 4.10 0x80014596 1.3295.0


TestStep:
1.
rm x86_64-native-linuxapp-gcc/ -rf
CC=gcc meson -Denable_kmods=True -Dlibdir=lib  -Dbuildtype=debug
-Db_lundef=false -Db_sanitize=address --default-library=static
x86_64-native-linuxapp-gcc
ninja -C x86_64-native-linuxapp-gcc -j 70

2.
./usertools/dpdk-devbind.py -b vfio-pci :0b:00.0

3.
 ./x86_64-native-linuxapp-gcc/app/dpdk-testpmd -c 0xf -n 4 -- -i

4.
quit


Actual Result(Show the output from the previous commands)
=
==3933==ERROR: AddressSanitizer: stack-buffer-overflow on address
0x7f75435fb480 at pc 0x7f7547b88117 bp 0x7f75435fb450 sp 0x7f75435fabf8
WRITE of size 24 at 0x7f75435fb480 thread T16777215
    #0 0x7f7547b88116 in __interceptor_sigaltstack.part.0
(/lib64/libasan.so.6+0x54116)
    #1 0x7f7547c069e7 in __sanitizer::UnsetAlternateSignalStack()
(/lib64/libasan.so.6+0xd29e7)
    #2 0x7f7547bf678c in __asan::AsanThread::Destroy()
(/lib64/libasan.so.6+0xc278c)
    #3 0x7f754748f820 in __GI___nptl_deallocate_tsd (/lib64/libc.so.6+0xa1820)
    #4 0x7f7547492595 in start_thread (/lib64/libc.so.6+0xa4595)
    #5 0x7f75474323ef in clone3 (/lib64/libc.so.6+0x443ef)Address
0x7f75435fb480 is located in stack of thread T2 at offset 576 in frame
    #0 0x129e3ba in mp_handle ../lib/eal/common/eal_common_proc.c:390  This
frame has 2 object(s):
    [32, 142) 'sa' (line 392)
    [176, 540) 'msg' (line 391) <== Memory access at offset 576 overflows this
variable
HINT: this may be a false positive if your program uses some custom stack
unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
Thread T2 created by T0 here:
    #0 0x7f7547b8c7d5 in pthread_create (/lib64/libasan.so.6+0x587d5)
    #1 0x128126e in rte_ctrl_thread_create
../lib/eal/common/eal_common_thread.c:288
    #2 0x129f844 in rte_mp_channel_init ../lib/eal/common/eal_common_proc.c:638
    #3 0x12b99e6 in rte_eal_init ../lib/eal/linux/eal.c:1051
    #4 0x7abde1 in main ../app/test-pmd/testpmd.c:4284
    #5 0x7f7547432e4f in __libc_start_call_main
(/lib64/libc.so.6+0x44e4f)SUMMARY: AddressSanitizer: stack-buffer-overflow
(/lib64/libasan.so.6+0x54116) in __interceptor_sigaltstack.part.0
Shadow bytes around the buggy address:
  0x0fef286b7640: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00 00 00
  0x0fef286b7650: 00 00 00 00 00 00 00 00 00 06 f2 f2 f2 f2 00 00
  0x0fef286b7660: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0fef286b7670: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0fef286b7680: 00 00 00 00 00 00 00 00 00 00 00 04 f3 f3 f3 f3
=>0x0fef286b7690:[f3]f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
  0x0fef286b76a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0fef286b76b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0fef286b76c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0fef286b76d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0fef286b76e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==3933==ABORTING
Architecture:            x86_64
  CPU op-mode(s):        32-bit, 64-bit
  Address sizes:         45 bits physical, 48 bits virtual
  Byte Order:            Little Endian
CPU(s):                  16
  On-line CPU(s) list:   0-15
Vendor ID:               GenuineIntel
  BIOS Vendor ID:        GenuineIntel
  Model name:            Intel(R) Xeon(R) Gold 6140M CPU @ 2.30GHz
    BIOS Model name:     Intel(R) Xeon(R) Gold 6140M CPU @ 2.30GHz
    CPU family:          6
    Model:               85
    Thread(s) per c

Re: [PATCH] app/testpmd: fix flow list for async flows

[Andrew Rybchenko]

On 11/7/22 16:16, Alexander Kozyrev wrote:

Flows created with the new asynchronous Flow API lack attributes
(direction, priority, group number). These attributes are part of
a template table for flows created via rte_flow_async_create().

When testpmd tries to list all the flows it accesses flow
attributes via pointer and crashes. Save flow attributes during
the template table creation and use them in the "flow list" output.

Fixes: ecdc927b99 ("app/testpmd: add async flow create/destroy operations")

Signed-off-by: Alexander Kozyrev 


Reviewed-by: Andrew Rybchenko 

Applied to dpdk-next-net/main, thanks.

Re: [PATCH v4] vdpa/ifc: fix update_datapath error handling

[Taekyung Kim]

Hi Maxime,

Thanks for your review.

On Tue, Nov 08, 2022 at 02:49:39PM +0100, Maxime Coquelin wrote:
> Hi Taekyung,
> 
> On 11/8/22 09:56, Taekyung Kim wrote:
> > Stop and return the error code when update_datapath fails.
> > update_datapath prepares resources for the vdpa device.
> > The driver should not perform any further actions
> > if update_datapath returns an error.
> > 
> > Fixes: a3f8150eac6d ("net/ifcvf: add ifcvf vDPA driver")
> > Cc: sta...@dpdk.org
> > 
> > Signed-off-by: Taekyung Kim 
> > ---
> > v4:
> > * Add rte_vdpa_unregister_device in ifcvf_pci_probe
> > 
> > v3:
> > * Fix coding style
> > 
> > v2:
> > * Revert the prepared resources before returning an error
> > * Rebase to 22.11 rc2
> > * Add fixes and cc for backport
> > 
> > ---
> >   drivers/vdpa/ifc/ifcvf_vdpa.c | 27 +++
> >   1 file changed, 23 insertions(+), 4 deletions(-)
> > 
> 
> Reviewed-by: Maxime Coquelin 
> 
> Thanks,
> Maxime
> 

Re: [PATCH v4] vdpa/ifc: fix update_datapath error handling

[Taekyung Kim]

Hi Andy,

Thanks for your review.

On Wed, Nov 09, 2022 at 02:39:09AM +, Pei, Andy wrote:
> 
> 
> > -Original Message-
> > From: Taekyung Kim 
> > Sent: Tuesday, November 8, 2022 4:56 PM
> > To: dev@dpdk.org
> > Cc: Xia, Chenbo ; Pei, Andy ;
> > kim.tae.ky...@navercorp.com; maxime.coque...@redhat.com;
> > sta...@dpdk.org; Wang, Xiao W 
> > Subject: [PATCH v4] vdpa/ifc: fix update_datapath error handling
> > 
> > Stop and return the error code when update_datapath fails.
> > update_datapath prepares resources for the vdpa device.
> > The driver should not perform any further actions if update_datapath 
> > returns an
> > error.
> > 
> > Fixes: a3f8150eac6d ("net/ifcvf: add ifcvf vDPA driver")
> > Cc: sta...@dpdk.org
> > 
> > Signed-off-by: Taekyung Kim 
> > ---
> > v4:
> > * Add rte_vdpa_unregister_device in ifcvf_pci_probe
> > 
> > v3:
> > * Fix coding style
> > 
> > v2:
> > * Revert the prepared resources before returning an error
> > * Rebase to 22.11 rc2
> > * Add fixes and cc for backport
> > 
> > ---
> >  drivers/vdpa/ifc/ifcvf_vdpa.c | 27 +++
> >  1 file changed, 23 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/vdpa/ifc/ifcvf_vdpa.c b/drivers/vdpa/ifc/ifcvf_vdpa.c 
> > index
> > 8dfd49336e..49d68ad1b1 100644
> > --- a/drivers/vdpa/ifc/ifcvf_vdpa.c
> > +++ b/drivers/vdpa/ifc/ifcvf_vdpa.c
> > @@ -1098,7 +1098,12 @@ ifcvf_dev_config(int vid)
> > internal = list->internal;
> > internal->vid = vid;
> > rte_atomic32_set(&internal->dev_attached, 1);
> > -   update_datapath(internal);
> > +   if (update_datapath(internal) < 0) {
> > +   DRV_LOG(ERR, "failed to update datapath for vDPA device %s",
> > +   vdev->device->name);
> > +   rte_atomic32_set(&internal->dev_attached, 0);
> > +   return -1;
> > +   }
> > 
> > hw = &internal->hw;
> > for (i = 0; i < hw->nr_vring; i++) {
> > @@ -1146,7 +1151,12 @@ ifcvf_dev_close(int vid)
> > internal->sw_fallback_running = false;
> > } else {
> > rte_atomic32_set(&internal->dev_attached, 0);
> > -   update_datapath(internal);
> > +   if (update_datapath(internal) < 0) {
> > +   DRV_LOG(ERR, "failed to update datapath for vDPA
> > device %s",
> > +   vdev->device->name);
> > +   internal->configured = 0;
> > +   return -1;
> > +   }
> > }
> > 
> > internal->configured = 0;
> > @@ -1752,7 +1762,15 @@ ifcvf_pci_probe(struct rte_pci_driver *pci_drv
> > __rte_unused,
> > }
> > 
> > rte_atomic32_set(&internal->started, 1);
> > -   update_datapath(internal);
> > +   if (update_datapath(internal) < 0) {
> > +   DRV_LOG(ERR, "failed to update datapath %s", pci_dev->name);
> > +   rte_atomic32_set(&internal->started, 0);
> > +   rte_vdpa_unregister_device(internal->vdev);
> > +   pthread_mutex_lock(&internal_list_lock);
> > +   TAILQ_REMOVE(&internal_list, list, next);
> > +   pthread_mutex_unlock(&internal_list_lock);
> > +   goto error;
> > +   }
> > 
> > rte_kvargs_free(kvlist);
> > return 0;
> > @@ -1781,7 +1799,8 @@ ifcvf_pci_remove(struct rte_pci_device *pci_dev)
> > 
> > internal = list->internal;
> > rte_atomic32_set(&internal->started, 0);
> > -   update_datapath(internal);
> > +   if (update_datapath(internal) < 0)
> > +   DRV_LOG(ERR, "failed to update datapath %s", pci_dev->name);
> > 
> > rte_pci_unmap_device(internal->pdev);
> > rte_vfio_container_destroy(internal->vfio_container_fd);
> > --
> > 2.34.1
> 
> Acked-by: Andy Pei 

Re: [PATCH v4] ethdev: add special flags when creating async transfer table

[Thomas Monjalon]

09/11/2022 10:36, Andrew Rybchenko:
> On 11/9/22 12:03, Thomas Monjalon wrote:
> > 09/11/2022 09:53, Andrew Rybchenko:
> >> On 11/8/22 18:25, Thomas Monjalon wrote:
> >>> 08/11/2022 15:38, Andrew Rybchenko:
>  On 11/8/22 16:29, Thomas Monjalon wrote:
> > 08/11/2022 12:47, Andrew Rybchenko:
> >> On 11/8/22 14:39, Andrew Rybchenko wrote:
> >>> On 11/4/22 13:44, Rongwei Liu wrote:
>  diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
>  index 8858b56428..1eab12796f 100644
>  --- a/lib/ethdev/rte_flow.h
>  +++ b/lib/ethdev/rte_flow.h
>  @@ -5186,6 +5186,34 @@ rte_flow_actions_template_destroy(uint16_t
>  port_id,
>    */
>   struct rte_flow_template_table;
>  +/**
>  + * @warning
>  + * @b EXPERIMENTAL: this API may change without prior notice.
>  + *
>  + * Special optional flags for template table attribute.
>  + * Each bit stands for a table specialization
>  + * offering a potential optimization at PMD layer.
>  + * PMD can ignore the unsupported bits silently.
>  + */
>  +enum rte_flow_template_table_specialize {
>  +/**
>  + * Specialize table for transfer flows which come only from 
>  wire.
>  + * It allows PMD not to allocate resources for non-wire
>  originated traffic.
>  + * This bit is not a matching criteria, just an optimization 
>  hint.
>  + * Flow rules which match non-wire originated traffic will be 
>  missed
>  + * if the hint is supported.
> >>
> >> Sorry, but if so, the hint changes behavior.
> >
> > Yes the hint may change behaviour.
> >
> >> Let's consider a rule which matches both VF originating and
> >> wire originating traffic. Will the rule be missed (ignored)
> >> regardless if the hint is supported or not?
> >
> > If the hint RTE_FLOW_TRANSFER_WIRE_ORIG is used,
> > the PMD may assume the table won't be used for traffic
> > which is not coming from wire ports.
> > As a consequence, the table may be implemented on the path
> > of wire traffic only.
> > In this case, the traffic coming from virtual ports
> > won't be affected by this table.
> > To answer the question, a rule matching both virtual and wire traffic
> > will be applied in a table affecting only wire traffic,
> > so it will still apply (not completely ignored).
> 
>  If so, it is not a hint. It becomes matching criteria
>  which should be in pattern as we discussed.
> >>>
> >>> It is not a strict matching because the PMD is free to support it or not.
> >>
> >> It cannot be optional matching criteria. Matching criteria must
> >> be always mandatory. Otherwise application does not know what
> >> to expect and behaviour may legitimately vary on different
> >> vendors.
> > 
> > I think you take it in the wrong direction.
> > The idea is not to have it as a criteria.
> > Let me explain again:
> > 
> > If an application is using a flow table to manage flows
> > which *always* come from the same type of port (wire or virtual),
> 
> What does guarantee it? Is it used a jump-table and jump rule
> must guarantee it? Or has pattern corresponding unit?
> 
> It is very thin ice and I'm ready to bet money that finally
> it will be used as a matching criteria intentionally or not
> intentionally. Simply because it works as matching criteria
> on, for example, Mellanox. I.e. if rules from table with
> corresponding hint are programmed to HW which applies these
> rules on traffic from wire only - effectively it is a matching
> criteria. And it will be used this way. And it will be not
> portable to other HW which does not support the hint.
> So, we're making an API which is very easy to misuse if not
> to say more.

I completely understand your concern (I have same).
In other words, if the application misuse the hint,
it will become not portable.
That's why I made sure to highlight such misue consequence
in the API comments.

> You know better if it is OK or not to rely on liable users
> in the case of DPDK.

I do not rely on users, and I don't want to block innovation.
That's why I want to make sure all is explained and clear,
so freedom comes with responsibility.

> It would be much safer if we do not rely on application in this
> case, introduce a new pattern item to specify origin and
> require PMD to check that pattern has either a new pattern item
> or corresponding  REPRESENTED_PORT/PORT_REPRESENTOR pattern
> item.

Safer is not often compatible with fastest :)

> I realize that my concerns could be not valid and it is just
> a paranoia. Just add your ack and let's move forward.

Let's wait for other opinions.

> > then the application can give this information to the driver.
> > With this assumption coming from the application,
> > the driver may do some optimization

RE: FW: [PATCH v4 3/3] mempool: use cache for frequently updated stats

[Morten Brørup]

> From: Konstantin Ananyev [mailto:konstantin.anan...@huawei.com]
> Sent: Wednesday, 9 November 2022 11.20
> 
> > On 2022-11-09 06:03, Morten Brørup wrote:
> > >> From: Konstantin Ananyev [mailto:konstantin.anan...@huawei.com]
> > >> Sent: Tuesday, 8 November 2022 18.38
> > >>>
> > >>> On Tue, Nov 08, 2022 at 04:51:11PM +0100, Thomas Monjalon wrote:
> >  08/11/2022 15:30, Morten Brørup:
> > >> From: Thomas Monjalon [mailto:tho...@monjalon.net]
> > >> 08/11/2022 12:25, Morten Brørup:
> > >>> From: Morten Brørup
> >  From: Konstantin Ananyev
> > >> [mailto:konstantin.anan...@huawei.com]
> >  Sent: Tuesday, 8 November 2022 10.20
> > > +#ifdef RTE_LIBRTE_MEMPOOL_STATS
> > > +#define RTE_MEMPOOL_CACHE_STAT_ADD(cache, name, n)
> > >> (cache)-
> > >>> https://protect2.fireeye.com/v1/url?k=31323334-501d5122-
> 313273af-45444731-27a859e7ec13035a&q=1&e=a120e28e-
> > caa7-4783-9686-5868c871553d&u=http%3A%2F%2Fstats.name%2F += n
> > 
> >  As Andrew already pointed, it needs to be: ((cache)-
> > >>> https://protect2.fireeye.com/v1/url?k=31323334-501d5122-313273af-
> 45444731-27a859e7ec13035a&q=1&e=a120e28e-caa7-
> > 4783-9686-5868c871553d&u=http%3A%2F%2Fstats.name%2F +=
> > >> (n))
> >  Apart from that, LGTM.
> >  Series-Acked-by: Konstantin Ananyev
> > >> 
> > >>>
> > >>> @Thomas, this series should be ready to apply... it now has
> > >> been:
> > >>> Reviewed-by: (mempool maintainer) Andrew Rybchenko
> > >> 
> > >>> Reviewed-By: Mattias Rönnblom 
> > >>> Acked-by: Konstantin Ananyev 
> > >>
> > >> Being acked does not mean it is good to apply in -rc3.
> > >
> > > I understand that the RFC/v1 of this series was formally too
> late
> > >> to make it in 22.11, so I will not complain loudly if you choose
> to
> > >>> omit it for 22.11.
> > >
> > > With two independent reviews, including from a mempool
> > >> maintainer, I still have some hope. Also considering the risk
> > >> assessment
> > >>> below. ;-)
> > >
> > >> Please tell what is the benefit for 22.11 (before/after and
> > >> condition).
> > >
> > > Short version: With this series, mempool statistics can be used
> > >> in production. Without it, the performance cost
> > >>> (mempool_perf_autotest: -74 %) is prohibitive!
> > >
> > > Long version:
> > >
> > > The patch series provides significantly higher performance for
> > >> mempool statistics, which are readable through
> > >>> rte_mempool_dump(FILE *f, struct rte_mempool *mp).
> > >
> > > Without this series, you have to set RTE_LIBRTE_MEMPOOL_DEBUG
> at
> > >> build time to get mempool statistics.
> > >>> RTE_LIBRTE_MEMPOOL_DEBUG also enables protective cookies before
> and
> > >> after each mempool object, which are all verified on
> > >>> get/put from the mempool. According to mempool_perf_autotest, the
> > >> performance cost of mempool statistics (by setting
> > >>> RTE_LIBRTE_MEMPOOL_DEBUG) is a 74 % decrease in rate_persec for
> > >> mempools with cache (i.e. mbuf pools). Prohibitive for use in
> > >>> production!
> > >
> > > With this series, the performance cost of mempool statistics
> (by
> > >> setting RTE_LIBRTE_MEMPOOL_STATS) in
> > >>> mempool_perf_autotest is only 6.7 %, so mempool statistics can be
> > >> used in production.
> > >
> > >> Note there is a real risk doing such change that late.
> > >
> > > Risk assessment:
> > >
> > > The patch series has zero effect unless either
> > >> RTE_LIBRTE_MEMPOOL_DEBUG or RTE_LIBRTE_MEMPOOL_STATS are set when
> > >>> building. They are not set in the default build.
> > 
> >  If theses build flags are not set, there is no risk and no
> benefit.
> >  But if they are set, there is a risk of regression,
> >  for the benefit of an increased performance of a debug feature.
> >  I would say it is better to avoid any functional regression in a
> > >> debug feature
> >  at this stage.
> >  Any other opinion?
> > 
> > >>> While I agree that we should avoid any functional regression, I
> > >> wonder how
> > >>> widely used the debug feature is, and how big the risk of a
> > >> regression is?
> > >>> Even if there is one, having a regression in a debug feature is a
> lot
> > >> less
> > >>> serious than having one in something which goes into production.
> > >>>
> > >>
> > >> Unless it introduces an ABI breakage (as I understand it doesn't),
> I'll
> > >> wait till 23.03.
> > >> Just in case.
> > >
> > > If built (both before and after this series) without
> RTE_LIBRTE_MEMPOOL_DEBUG (and without RTE_LIBRTE_MEMPOOL_STATS,
> > which is introduced by the series), there is no ABI breakage.
> > >
> > > If built (both before and after this series) with
> RTE_LIBRTE_MEMPOOL_DEBUG (and without RTE_LIBRTE_MEMPOOL_STATS), the
> > ABI differs between before and after this series: The stats array
> disappears from struct rte_mempool, 

[PATCH] net/mlx5: fix indexed pool local cache crash

[Alexander Kozyrev]

Local cache for an indexed pool is not initialized in the situation when
all the indices are allocated on one CPU core and freed on another one.
That leads to a crash once we try to check its reference counter.
Check that the local cache is initialized before accessing this counter.

Fixes: d15c0946be ("net/mlx5: add indexed pool local cache")
Cc: sta...@dpdk.org

Signed-off-by: Alexander Kozyrev 
---
 drivers/net/mlx5/mlx5_utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_utils.c b/drivers/net/mlx5/mlx5_utils.c
index 4115a2ad77..b295702fd4 100644
--- a/drivers/net/mlx5/mlx5_utils.c
+++ b/drivers/net/mlx5/mlx5_utils.c
@@ -479,7 +479,7 @@ _mlx5_ipool_free_cache(struct mlx5_indexed_pool *pool, int 
cidx, uint32_t idx)
mlx5_ipool_lock(pool);
gc = pool->gc;
if (ilc->lc != gc) {
-   if (!(--ilc->lc->ref_cnt))
+   if (ilc->lc && !(--ilc->lc->ref_cnt))
olc = ilc->lc;
gc->ref_cnt++;
ilc->lc = gc;
-- 
2.18.2

RE: [PATCH] net/mlx5: fix indexed pool local cache crash

[Slava Ovsiienko]

> -Original Message-
> From: Alexander Kozyrev 
> Sent: Wednesday, November 9, 2022 14:58
> To: dev@dpdk.org
> Cc: sta...@dpdk.org; Raslan Darawsheh ; Slava Ovsiienko
> ; Matan Azrad ; Michael Baum
> 
> Subject: [PATCH] net/mlx5: fix indexed pool local cache crash
> 
> Local cache for an indexed pool is not initialized in the situation when all
> the indices are allocated on one CPU core and freed on another one.
> That leads to a crash once we try to check its reference counter.
> Check that the local cache is initialized before accessing this counter.
> 
> Fixes: d15c0946be ("net/mlx5: add indexed pool local cache")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Alexander Kozyrev 
Acked-by: Viacheslav Ovsiienko 

[PATCH] bus/auxiliary: prevent device from being probed again

[Bing Zhao]

The device on auxiliary bus doesn't support being probed again
without being removed firstly. The PMD will detect and return error
for this unsupported operation. Some of the resources would be
cleared wrongly. When quiting, there will be unexpected error like
crash.

To prevent this, the device driver will be checked before probing a
device.

Fixes: 1afce3086cf4 ("bus/auxiliary: introduce auxiliary bus")
Cc: xuemi...@nvidia.com
CC: sta...@dpdk.org

Signed-off-by: Bing Zhao 
Reviewed-by: Gregory Etelson 
Reviewed-by: Matan Azrad 
---
 drivers/bus/auxiliary/auxiliary_common.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/bus/auxiliary/auxiliary_common.c 
b/drivers/bus/auxiliary/auxiliary_common.c
index 6bb1fe7c96..8bc9f20f1b 100644
--- a/drivers/bus/auxiliary/auxiliary_common.c
+++ b/drivers/bus/auxiliary/auxiliary_common.c
@@ -89,6 +89,7 @@ rte_auxiliary_probe_one_driver(struct rte_auxiliary_driver 
*drv,
 {
enum rte_iova_mode iova_mode;
int ret;
+   bool already_probed;
 
if (drv == NULL || dev == NULL)
return -EINVAL;
@@ -116,6 +117,13 @@ rte_auxiliary_probe_one_driver(struct rte_auxiliary_driver 
*drv,
return -EINVAL;
}
 
+   already_probed = rte_dev_is_probed(&dev->device);
+   if (already_probed) {
+   RTE_LOG(DEBUG, EAL, "Device %s is already probed on auxiliary 
bus\n",
+   dev->device.name);
+   return -EEXIST;
+   }
+
/* Allocate interrupt instance */
dev->intr_handle =
rte_intr_instance_alloc(RTE_INTR_INSTANCE_F_PRIVATE);
-- 
2.21.0

[PATCH v2] bus/auxiliary: prevent device from being probed again

[Bing Zhao]

The device on auxiliary bus doesn't support being probed again
without being removed firstly. The PMD will detect and return error
for this unsupported operation. Some of the resources would be
cleared wrongly. When quitting, there will be unexpected error like
crash.

To prevent this, the device driver will be checked before probing a
device.

Fixes: 1afce3086cf4 ("bus/auxiliary: introduce auxiliary bus")
Cc: xuemi...@nvidia.com
CC: sta...@dpdk.org

Signed-off-by: Bing Zhao 
Reviewed-by: Gregory Etelson 
Reviewed-by: Matan Azrad 
---
v2: fix typo
---
 drivers/bus/auxiliary/auxiliary_common.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/bus/auxiliary/auxiliary_common.c 
b/drivers/bus/auxiliary/auxiliary_common.c
index 6bb1fe7c96..8bc9f20f1b 100644
--- a/drivers/bus/auxiliary/auxiliary_common.c
+++ b/drivers/bus/auxiliary/auxiliary_common.c
@@ -89,6 +89,7 @@ rte_auxiliary_probe_one_driver(struct rte_auxiliary_driver 
*drv,
 {
enum rte_iova_mode iova_mode;
int ret;
+   bool already_probed;
 
if (drv == NULL || dev == NULL)
return -EINVAL;
@@ -116,6 +117,13 @@ rte_auxiliary_probe_one_driver(struct rte_auxiliary_driver 
*drv,
return -EINVAL;
}
 
+   already_probed = rte_dev_is_probed(&dev->device);
+   if (already_probed) {
+   RTE_LOG(DEBUG, EAL, "Device %s is already probed on auxiliary 
bus\n",
+   dev->device.name);
+   return -EEXIST;
+   }
+
/* Allocate interrupt instance */
dev->intr_handle =
rte_intr_instance_alloc(RTE_INTR_INSTANCE_F_PRIVATE);
-- 
2.21.0

[dpdk-dev] [PATCH] lib: fix doxygen syntax issues

[jerinj]

From: Jerin Jacob 

Fix following syntax error reported by doxygen 1.9.5 version.

lib/eal/include/rte_uuid.h:89: error: RTE_UUID_STRLEN
has @param documentation sections but no arguments
(warning treated as error, aborting now)

lib/power/rte_power.h:169: error: rte_power_freq_up has
@param documentation sections but no arguments
(warning treated as error, aborting now)

Fixes: 6bc67c497a51 ("eal: add uuid API")
Fixes: d7937e2e3d12 ("power: initial import")
Cc: sta...@dpdk.org

Signed-off-by: Jerin Jacob 
---
 lib/eal/include/rte_uuid.h |  4 ++-
 lib/power/rte_power.h  | 55 --
 2 files changed, 3 insertions(+), 56 deletions(-)

diff --git a/lib/eal/include/rte_uuid.h b/lib/eal/include/rte_uuid.h
index 8b42e070af..cfefd4308a 100644
--- a/lib/eal/include/rte_uuid.h
+++ b/lib/eal/include/rte_uuid.h
@@ -37,6 +37,9 @@ typedef unsigned char rte_uuid_t[16];
((e) >> 8) & 0xff, (e) & 0xff   \
 }
 
+/** UUID string length */
+#define RTE_UUID_STRLEN(36 + 1)
+
 /**
  * Test if UUID is all zeros.
  *
@@ -95,7 +98,6 @@ int   rte_uuid_parse(const char *in, rte_uuid_t uu);
  * @param len
  *Sizeof the available string buffer
  */
-#define RTE_UUID_STRLEN(36 + 1)
 void   rte_uuid_unparse(const rte_uuid_t uu, char *out, size_t len);
 
 #ifdef __cplusplus
diff --git a/lib/power/rte_power.h b/lib/power/rte_power.h
index 47345e26df..7954299489 100644
--- a/lib/power/rte_power.h
+++ b/lib/power/rte_power.h
@@ -169,14 +169,6 @@ typedef int (*rte_power_freq_change_t)(unsigned int 
lcore_id);
  * Scale up the frequency of a specific lcore according to the available
  * frequencies.
  * Review each environments specific documentation for usage.
- *
- * @param lcore_id
- *  lcore id.
- *
- * @return
- *  - 1 on success with frequency changed.
- *  - 0 on success without frequency changed.
- *  - Negative on error.
  */
 extern rte_power_freq_change_t rte_power_freq_up;
 
@@ -184,30 +176,13 @@ extern rte_power_freq_change_t rte_power_freq_up;
  * Scale down the frequency of a specific lcore according to the available
  * frequencies.
  * Review each environments specific documentation for usage.
- *
- * @param lcore_id
- *  lcore id.
- *
- * @return
- *  - 1 on success with frequency changed.
- *  - 0 on success without frequency changed.
- *  - Negative on error.
  */
-
 extern rte_power_freq_change_t rte_power_freq_down;
 
 /**
  * Scale up the frequency of a specific lcore to the highest according to the
  * available frequencies.
  * Review each environments specific documentation for usage.
- *
- * @param lcore_id
- *  lcore id.
- *
- * @return
- *  - 1 on success with frequency changed.
- *  - 0 on success without frequency changed.
- *  - Negative on error.
  */
 extern rte_power_freq_change_t rte_power_freq_max;
 
@@ -215,54 +190,24 @@ extern rte_power_freq_change_t rte_power_freq_max;
  * Scale down the frequency of a specific lcore to the lowest according to the
  * available frequencies.
  * Review each environments specific documentation for usage..
- *
- * @param lcore_id
- *  lcore id.
- *
- * @return
- *  - 1 on success with frequency changed.
- *  - 0 on success without frequency changed.
- *  - Negative on error.
  */
 extern rte_power_freq_change_t rte_power_freq_min;
 
 /**
  * Query the Turbo Boost status of a specific lcore.
  * Review each environments specific documentation for usage..
- *
- * @param lcore_id
- *  lcore id.
- *
- * @return
- *  - 1 Turbo Boost is enabled for this lcore.
- *  - 0 Turbo Boost is disabled for this lcore.
- *  - Negative on error.
  */
 extern rte_power_freq_change_t rte_power_turbo_status;
 
 /**
  * Enable Turbo Boost for this lcore.
  * Review each environments specific documentation for usage..
- *
- * @param lcore_id
- *  lcore id.
- *
- * @return
- *  - 0 on success.
- *  - Negative on error.
  */
 extern rte_power_freq_change_t rte_power_freq_enable_turbo;
 
 /**
  * Disable Turbo Boost for this lcore.
  * Review each environments specific documentation for usage..
- *
- * @param lcore_id
- *  lcore id.
- *
- * @return
- *  - 0 on success.
- *  - Negative on error.
  */
 extern rte_power_freq_change_t rte_power_freq_disable_turbo;
 
-- 
2.38.1

Re: [dpdk-dev] [PATCH] lib: fix doxygen syntax issues

[Thomas Monjalon]

09/11/2022 15:54, jer...@marvell.com:
> From: Jerin Jacob 
> 
> Fix following syntax error reported by doxygen 1.9.5 version.
> 
> lib/eal/include/rte_uuid.h:89: error: RTE_UUID_STRLEN
> has @param documentation sections but no arguments
> (warning treated as error, aborting now)
> 
> lib/power/rte_power.h:169: error: rte_power_freq_up has
> @param documentation sections but no arguments
> (warning treated as error, aborting now)
> 
> Fixes: 6bc67c497a51 ("eal: add uuid API")
> Fixes: d7937e2e3d12 ("power: initial import")

I think it deserves to be split in 2 patches.
If you agree, I can split when merging.

Re: [dpdk-dev] [PATCH] lib: fix doxygen syntax issues

[Jerin Jacob]

On Wed, Nov 9, 2022 at 8:34 PM Thomas Monjalon  wrote:
>
> 09/11/2022 15:54, jer...@marvell.com:
> > From: Jerin Jacob 
> >
> > Fix following syntax error reported by doxygen 1.9.5 version.
> >
> > lib/eal/include/rte_uuid.h:89: error: RTE_UUID_STRLEN
> > has @param documentation sections but no arguments
> > (warning treated as error, aborting now)
> >
> > lib/power/rte_power.h:169: error: rte_power_freq_up has
> > @param documentation sections but no arguments
> > (warning treated as error, aborting now)
> >
> > Fixes: 6bc67c497a51 ("eal: add uuid API")
> > Fixes: d7937e2e3d12 ("power: initial import")
>
> I think it deserves to be split in 2 patches.
> If you agree, I can split when merging.

OK

>
>
>

[PATCH] app/test: add SHA3 test cases

[Volodymyr Fialko]

Add test cases for SHA3 hash family for Digest and Digest-Verify.

Signed-off-by: Volodymyr Fialko 

Expected vectors were generated with OpenSSL. Adding to CC original authors of 
PMDs that currently
have support for SHA3 to check if there's any issues.

---
 app/test/test_cryptodev_hash_test_vectors.h | 286 
 1 file changed, 286 insertions(+)

diff --git a/app/test/test_cryptodev_hash_test_vectors.h 
b/app/test/test_cryptodev_hash_test_vectors.h
index 62602310b2..4b57286fa5 100644
--- a/app/test/test_cryptodev_hash_test_vectors.h
+++ b/app/test/test_cryptodev_hash_test_vectors.h
@@ -332,6 +332,212 @@ hmac_sha512_test_vector = {
}
 };
 
+static const struct blockcipher_test_data
+sha3_224_test_vector = {
+   .auth_algo = RTE_CRYPTO_AUTH_SHA3_224,
+   .ciphertext = {
+   .data = plaintext_hash,
+   .len = 512
+   },
+   .digest = {
+   .data = {
+   0xFF, 0x7D, 0xAB, 0xC4, 0xB8, 0xF8, 0x0D, 0x5C,
+   0x3A, 0xD3, 0xCD, 0x71, 0x58, 0x62, 0x24, 0x0F,
+   0xCC, 0x58, 0xE4, 0x42, 0x1B, 0xA3, 0x6F, 0xE8,
+   0x9A, 0x44, 0xBF, 0x45
+   },
+   .len = 28,
+   .truncated_len = 28
+   }
+};
+
+static const struct blockcipher_test_data
+hmac_sha3_224_test_vector = {
+   .auth_algo = RTE_CRYPTO_AUTH_SHA3_224_HMAC,
+   .ciphertext = {
+   .data = plaintext_hash,
+   .len = 512
+   },
+   .auth_key = {
+   .data = {
+   0xF8, 0x2A, 0xC7, 0x54, 0xDB, 0x96, 0x18, 0xAA,
+   0xC3, 0xA1, 0x53, 0xF6, 0x1F, 0x17, 0x60, 0xBD,
+   0xDE, 0xF4, 0xDE, 0xAD, 0x26, 0xEB, 0xAB, 0x92,
+   0xFB, 0xBF, 0xB0, 0x8C
+   },
+   .len = 28
+   },
+   .digest = {
+   .data = {
+   0x86, 0xDB, 0x99, 0x80, 0xFC, 0x13, 0x75, 0x4E,
+   0xB5, 0x30, 0x7A, 0x58, 0xC1, 0x0D, 0xE4, 0x00,
+   0x7F, 0xE3, 0xD8, 0xC2, 0x0E, 0x0C, 0xC1, 0xFD,
+   0xF9, 0x33, 0x05, 0x40
+   },
+   .len = 28,
+   .truncated_len = 14
+   }
+};
+
+static const struct blockcipher_test_data
+sha3_256_test_vector = {
+   .auth_algo = RTE_CRYPTO_AUTH_SHA3_256,
+   .ciphertext = {
+   .data = plaintext_hash,
+   .len = 512
+   },
+   .digest = {
+   .data = {
+   0xC3, 0x6B, 0x73, 0xF4, 0x97, 0x7F, 0xFA, 0xD9,
+   0x35, 0xF1, 0x1F, 0x54, 0x35, 0xC2, 0x19, 0x6C,
+   0xA2, 0x24, 0xC7, 0x01, 0xAD, 0xCC, 0xD4, 0x35,
+   0x88, 0xB9, 0x0C, 0x15, 0xAE, 0x3F, 0x92, 0x47
+   },
+   .len = 32,
+   .truncated_len = 32
+   }
+};
+
+static const struct blockcipher_test_data
+hmac_sha3_256_test_vector = {
+   .auth_algo = RTE_CRYPTO_AUTH_SHA3_256_HMAC,
+   .ciphertext = {
+   .data = plaintext_hash,
+   .len = 512
+   },
+   .auth_key = {
+   .data = {
+   0xF8, 0x2A, 0xC7, 0x54, 0xDB, 0x96, 0x18, 0xAA,
+   0xC3, 0xA1, 0x53, 0xF6, 0x1F, 0x17, 0x60, 0xBD,
+   0xDE, 0xF4, 0xDE, 0xAD, 0x26, 0xEB, 0xAB, 0x92,
+   0xFB, 0xBF, 0xB0, 0x8C, 0x29, 0x87, 0x90, 0xAC
+   },
+   .len = 32
+   },
+   .digest = {
+   .data = {
+   0x8E, 0xB1, 0xBD, 0xEE, 0xEF, 0x26, 0xDD, 0xE7,
+   0x66, 0xBD, 0x9C, 0x2B, 0xBC, 0x5D, 0x6E, 0xC8,
+   0x8C, 0x4A, 0x9C, 0x79, 0xD7, 0x05, 0xDE, 0xFC,
+   0x21, 0x48, 0xD5, 0x95, 0x6D, 0x37, 0x0E, 0x00
+   },
+   .len = 32,
+   .truncated_len = 16
+   }
+};
+
+static const struct blockcipher_test_data
+sha3_384_test_vector = {
+   .auth_algo = RTE_CRYPTO_AUTH_SHA3_384,
+   .ciphertext = {
+   .data = plaintext_hash,
+   .len = 512
+   },
+   .digest = {
+   .data = {
+   0x56, 0x18, 0x64, 0x95, 0x70, 0x64, 0x32, 0x80,
+   0xB3, 0x67, 0x7B, 0xE3, 0x09, 0x75, 0x92, 0x2B,
+   0x56, 0x40, 0xA9, 0xC5, 0x19, 0x47, 0x50, 0x33,
+   0xA7, 0xA2, 0x2B, 0x45, 0x46, 0xFD, 0x69, 0xE5,
+   0xDE, 0x2B, 0x35, 0xE6, 0x06, 0xC7, 0x0D, 0x28,
+   0x5A, 0xFB, 0x37, 0x4A, 0xE5, 0x8F, 0x9F, 0x4A
+   },
+   .len = 48,
+   .truncated_len = 48
+   }
+};
+
+static const struct blockcipher_test_data
+hmac_sha3_384_test_vector = {
+   .auth_algo = RTE_CRYPTO_AUTH_SHA3_384_HMAC,
+   .ciphertext = {
+   .data = plaintext_hash,
+   

RE: [PATCH 0/2] net/mlx5/hws: fix matcher clean up for FDB tables

[Raslan Darawsheh]

Hi,

> -Original Message-
> From: Dariusz Sosnowski 
> Sent: Wednesday, November 9, 2022 11:54 AM
> To: Matan Azrad ; Slava Ovsiienko
> ; Alex Vesker ; Erez Shitrit
> 
> Cc: dev@dpdk.org; Raslan Darawsheh 
> Subject: [PATCH 0/2] net/mlx5/hws: fix matcher clean up for FDB tables
> 
> Before these patches, if an application was configured to run with HW
> Steering and E-Switch enabled, on EAL cleanup the assertion in
> mlx5_dev_hw_global_release() was triggered - PD release was unsuccessful.
> 
> Root cause of this issue was linked to an inability to destroy RTC objects 
> used
> internally in mlx5, in HW Steering implementation.
> PMD was unable to destroy RTC objects, because of dangling references to
> those objects. More specifically, if all matchers connected to a single flow
> table were created, this flow table was still referencing RTC objects when
> theye were being destroyed.
> 
> This patch series fixes that behavior.
> Matcher uninitilization is updated to remove the references to RTC objects
> from flow table object if the last matcher related to the flow table was
> destroyed.
> 
> Erez Shitrit (2):
>   net/mlx5/hws: fix order of destroying default tables
>   net/mlx5/hws: fix disconnecting matcher
> 
>  drivers/net/mlx5/hws/mlx5dr_matcher.c | 35
> +++
>  drivers/net/mlx5/hws/mlx5dr_table.c   |  2 +-
>  2 files changed, 36 insertions(+), 1 deletion(-)
> 
> --
> 2.25.1

series applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh

RE: [PATCH] net/mlx5: fix indexed pool local cache crash

[Raslan Darawsheh]

Hi,

> -Original Message-
> From: Alexander Kozyrev 
> Sent: Wednesday, November 9, 2022 2:58 PM
> To: dev@dpdk.org
> Cc: sta...@dpdk.org; Raslan Darawsheh ; Slava
> Ovsiienko ; Matan Azrad ;
> Michael Baum 
> Subject: [PATCH] net/mlx5: fix indexed pool local cache crash
> 
> Local cache for an indexed pool is not initialized in the situation when
> all the indices are allocated on one CPU core and freed on another one.
> That leads to a crash once we try to check its reference counter.
> Check that the local cache is initialized before accessing this counter.
> 
> Fixes: d15c0946be ("net/mlx5: add indexed pool local cache")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Alexander Kozyrev 

Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh

Re: [PATCH v8 0/9] dts: ssh connection to a node

[Thomas Monjalon]

04/11/2022 12:05, Juraj Linkeš:
> All the necessary code needed to connect to a node in a topology with
> a bit more, such as basic logging and some extra useful methods.

Applied, thanks.

That's the first step towards integration of DTS in DPDK repository.
Nice to see this becoming a reality.

[...]
> This is our current roadmap:
> 1. Review this patchset and do the rest of the items in parallel, if
> possible.
> 2. We have extracted the code needed to run the most basic testcase,
> HelloWorld, which runs the DPDK Hello World application. We'll split
> this along logical/functional boundaries and send after 1 is done.
> 3. Once we have 2 applied, we're planning on adding a basic functional
> testcase - pf_smoke. This send a bit of traffic, so the big addition is
> the software traffic generator, Scapy. There's some work already done on
> Traffic generators we'll be sending as a dependence on this patch
> series.
> 4. After 3, we'll add a basic performance testcase which doesn't use
> Scapy, but Trex or Ixia instead.
> 5. This is far in the future, but at this point we should have all of
> the core functionality in place. What then remains is adding the rest of
> the testcases.

Let's join the force and help making this project a pleasant tool.

RE: [PATCH v8 0/9] dts: ssh connection to a node

[Honnappa Nagarahalli]

> 
> 04/11/2022 12:05, Juraj Linkeš:
> > All the necessary code needed to connect to a node in a topology with
> > a bit more, such as basic logging and some extra useful methods.
> 
> Applied, thanks.
> 
> That's the first step towards integration of DTS in DPDK repository.
> Nice to see this becoming a reality.
Thanks Thomas and the community for the reviews and suggestions. This is the 
first important step, hopefully we can continue the collaboration in the future 
releases.

> 
> [...]
> > This is our current roadmap:
> > 1. Review this patchset and do the rest of the items in parallel, if
> > possible.
> > 2. We have extracted the code needed to run the most basic testcase,
> > HelloWorld, which runs the DPDK Hello World application. We'll split
> > this along logical/functional boundaries and send after 1 is done.
> > 3. Once we have 2 applied, we're planning on adding a basic functional
> > testcase - pf_smoke. This send a bit of traffic, so the big addition
> > is the software traffic generator, Scapy. There's some work already
> > done on Traffic generators we'll be sending as a dependence on this
> > patch series.
> > 4. After 3, we'll add a basic performance testcase which doesn't use
> > Scapy, but Trex or Ixia instead.
> > 5. This is far in the future, but at this point we should have all of
> > the core functionality in place. What then remains is adding the rest
> > of the testcases.
> 
> Let's join the force and help making this project a pleasant tool.
> 

[PATCH 1/3] net/mlx5: fix log level on failed transfer proxy stop

[Dariusz Sosnowski]

This patches increases log level for error reporting when stopping
the transfer proxy port failed. Stopping can fail with EBUSY when
related representor ports are still running.

Fixes: 483181f7b6dd ("net/mlx5: support device control of representor matching")

Signed-off-by: Dariusz Sosnowski 
---
 drivers/net/mlx5/mlx5_trigger.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_trigger.c b/drivers/net/mlx5/mlx5_trigger.c
index 4b821a1076..310a84729e 100644
--- a/drivers/net/mlx5/mlx5_trigger.c
+++ b/drivers/net/mlx5/mlx5_trigger.c
@@ -1361,9 +1361,9 @@ mlx5_hw_proxy_port_allowed_stop(struct rte_eth_dev *dev)
representor_started = true;
}
if (representor_started) {
-   DRV_LOG(INFO, "Failed to stop port %u: attached representor 
ports"
- " must be stopped before stopping transfer proxy 
port",
- dev->data->port_id);
+   DRV_LOG(ERR, "Failed to stop port %u: attached representor 
ports"
+" must be stopped before stopping transfer proxy 
port",
+dev->data->port_id);
rte_errno = EBUSY;
return -rte_errno;
}
-- 
2.25.1

[PATCH 0/3] ethdev: document special cases of port start and stop

[Dariusz Sosnowski]

This patch series attempts to address the special failure
cases of rte_eth_dev_stop() and rte_eth_dev_start().

In case of starting a port, If the port depends on another one
being started, PMDs might return (-EAGAIN) to notify
about such situation.

In case of stopping a port, If the port cannot be stopped,
because other port depends on its resources,
PMDs might return (-EBUSY) to notify about such situation.

These cases are addressed in ethdev API docs.
Testpmd is updated to allow users to manually retry start/stop operations.

Dariusz Sosnowski (3):
  net/mlx5: fix log level on failed transfer proxy stop
  doc: document E-Switch limitations with HWS in mlx5 PMD
  ethdev: document special cases of port start and stop

 app/test-pmd/testpmd.c  | 10 +-
 doc/guides/nics/mlx5.rst| 13 +
 drivers/net/mlx5/mlx5_trigger.c |  6 +++---
 lib/ethdev/rte_ethdev.h |  9 +
 4 files changed, 34 insertions(+), 4 deletions(-)

-- 
2.25.1

[PATCH 2/3] doc: document E-Switch limitations with HWS in mlx5 PMD

[Dariusz Sosnowski]

This patch adds the following limitations to the mlx5 PMD guide:

- With HW Steering and E-Switch enabled, transfer proxy port must
  be started before any port representor.
- With HW Steering and E-Switch enabled, all representors
  must be stopped before transfer proxy port is stopped.

Signed-off-by: Dariusz Sosnowski 
---
 doc/guides/nics/mlx5.rst | 13 +
 1 file changed, 13 insertions(+)

diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index 4f0db21dde..7e39bb2ea5 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -161,6 +161,19 @@ Limitations
   - NIC ConnectX-5 and before are not supported.
   - Partial match with item template is not supported.
   - IPv6 5-tuple matching is not supported.
+  - With E-Switch enabled, ports which share the E-Switch domain
+should be started and stopped in a specific order:
+
+- When starting ports, the transfer proxy port should be started first
+  and port representors should follow.
+- When stopping ports, all of the port representors
+  should be stopped before stopping the transfer proxy port.
+
+If ports are started/stopped in an incorrect order,
+``rte_eth_dev_start()``/``rte_eth_dev_stop()`` will return an appropriate 
error code:
+
+- ``-EAGAIN`` for ``rte_eth_dev_start()``.
+- ``-EBUSY`` for ``rte_eth_dev_stop().
 
 - When using Verbs flow engine (``dv_flow_en`` = 0), flow pattern without any
   specific VLAN will match for VLAN packets as well:
-- 
2.25.1

[PATCH 3/3] ethdev: document special cases of port start and stop

[Dariusz Sosnowski]

This patch clarifies the handling of following cases
in the ethdev API docs:

- If rte_eth_dev_start() returns (-EAGAIN) for some port,
  it cannot be started until other port is started.
- If rte_eth_dev_stop() returns (-EBUSY) for some port,
  it cannot be stopped until other port is stopped.

When stopping the port in testpmd fails due to (-EBUSY),
port's state is switched back to STARTED
to allow users to manually retry stopping the port.

No additional changes in testpmd are required to handle
failure to start port with (-EAGAIN).
If rte_eth_dev_start() fails, port's state is switched to STOPPED
and users are allowed to retry the operation.

Signed-off-by: Dariusz Sosnowski 
---
 app/test-pmd/testpmd.c  | 10 +-
 lib/ethdev/rte_ethdev.h |  9 +
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 7381dfd9e5..c9252031e8 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -3158,6 +3158,7 @@ stop_port(portid_t pid)
int need_check_link_status = 0;
portid_t peer_pl[RTE_MAX_ETHPORTS];
int peer_pi;
+   int ret;
 
if (port_id_is_invalid(pid, ENABLED_WARN))
return;
@@ -3207,9 +3208,16 @@ stop_port(portid_t pid)
if (port->flow_list)
port_flow_flush(pi);
 
-   if (eth_dev_stop_mp(pi) != 0)
+   ret = eth_dev_stop_mp(pi);
+   if (ret != 0) {
RTE_LOG(ERR, EAL, "rte_eth_dev_stop failed for port 
%u\n",
pi);
+   if (ret == -EBUSY) {
+   /* Allow to retry stopping the port. */
+   port->port_status = RTE_PORT_STARTED;
+   continue;
+   }
+   }
 
if (port->port_status == RTE_PORT_HANDLING)
port->port_status = RTE_PORT_STOPPED;
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 13fe73d5a3..abf5a24f92 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -2701,10 +2701,14 @@ int rte_eth_dev_tx_queue_stop(uint16_t port_id, 
uint16_t tx_queue_id);
  * On success, all basic functions exported by the Ethernet API (link status,
  * receive/transmit, and so on) can be invoked.
  *
+ * If the port depends on another one being started,
+ * PMDs might return (-EAGAIN) to notify about such requirement.
+ *
  * @param port_id
  *   The port identifier of the Ethernet device.
  * @return
  *   - 0: Success, Ethernet device started.
+ *   - -EAGAIN: If it depends on another port to be started first.
  *   - <0: Error code of the driver device start function.
  */
 int rte_eth_dev_start(uint16_t port_id);
@@ -2713,10 +2717,15 @@ int rte_eth_dev_start(uint16_t port_id);
  * Stop an Ethernet device. The device can be restarted with a call to
  * rte_eth_dev_start()
  *
+ * If the port provides some resources for other ports
+ * and it cannot be stopped before them,
+ * PMDs might return (-EBUSY) to notify about such requirement.
+ *
  * @param port_id
  *   The port identifier of the Ethernet device.
  * @return
  *   - 0: Success, Ethernet device stopped.
+ *   - -EBUSY: If it depends on another port to be stopped first.
  *   - <0: Error code of the driver device stop function.
  */
 int rte_eth_dev_stop(uint16_t port_id);
-- 
2.25.1

Re: [PATCH v2] kni: fix possible alloc_q starvation when mbufs are exhausted

[Stephen Hemminger]

On Wed,  9 Nov 2022 14:04:34 +0800
Yangchao Zhou  wrote:

> In some scenarios, mbufs returned by rte_kni_rx_burst are not freed
> immediately. So kni_allocate_mbufs may be failed, but we don't know.
> 
> Even worse, when alloc_q is completely exhausted, kni_net_tx in
> rte_kni.ko will drop all tx packets. kni_allocate_mbufs is never
> called again, even if the mbufs are eventually freed.
> 
> In this patch, we always try to allocate mbufs for alloc_q.
> 
> Don't worry about alloc_q being allocated too many mbufs, in fact,
> the old logic will gradually fill up alloc_q.
> Also, the cost of more calls to kni_allocate_mbufs should be acceptable.
> 
> Fixes: 3e12a98fe397 ("kni: optimize Rx burst")
> Cc: hem...@freescale.com
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Yangchao Zhou 

Since fifo_get returning 0 (no buffers) is very common would this
change impact performance.

If the problem is pool draining might be better to make the pool
bigger.

[PATCH] net/mlx5: fix port initialization with small LRO

[Gregory Etelson]

If application provided maximal LRO size was less than expected PMD
minimum, the PMD either crashed with assert, if asserts were enabled,
or proceeded with port initialization to set port private maximal
LRO size below supported minimum.

The patch terminates port start if LRO size
does not match PMD requirements and TCP LRO offload was requested
at least for one Rx queue.

Fixes: 50c00baff763 ("net/mlx5: limit LRO size to maximum Rx packet")

Cc: sta...@dpdk.org

Signed-off-by: Gregory Etelson 
Acked-by: Matan Azrad 
---
 drivers/net/mlx5/mlx5_rxq.c |  1 -
 drivers/net/mlx5/mlx5_trigger.c | 16 
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
index 0d9d11680b..724cd6c7e6 100644
--- a/drivers/net/mlx5/mlx5_rxq.c
+++ b/drivers/net/mlx5/mlx5_rxq.c
@@ -1533,7 +1533,6 @@ mlx5_max_lro_msg_size_adjust(struct rte_eth_dev *dev, 
uint16_t idx,
MLX5_MAX_TCP_HDR_OFFSET)
max_lro_size -= MLX5_MAX_TCP_HDR_OFFSET;
max_lro_size = RTE_MIN(max_lro_size, MLX5_MAX_LRO_SIZE);
-   MLX5_ASSERT(max_lro_size >= MLX5_LRO_SEG_CHUNK_SIZE);
max_lro_size /= MLX5_LRO_SEG_CHUNK_SIZE;
if (priv->max_lro_msg_size)
priv->max_lro_msg_size =
diff --git a/drivers/net/mlx5/mlx5_trigger.c b/drivers/net/mlx5/mlx5_trigger.c
index 4b821a1076..71089299b8 100644
--- a/drivers/net/mlx5/mlx5_trigger.c
+++ b/drivers/net/mlx5/mlx5_trigger.c
@@ -1167,6 +1167,22 @@ mlx5_dev_start(struct rte_eth_dev *dev)
else
rte_net_mlx5_dynf_inline_mask = 0;
if (dev->data->nb_rx_queues > 0) {
+   uint32_t max_lro_msg_size = priv->max_lro_msg_size;
+
+   if (max_lro_msg_size < MLX5_LRO_SEG_CHUNK_SIZE) {
+   uint32_t i;
+   struct mlx5_rxq_priv *rxq;
+
+   for (i = 0; i != priv->rxqs_n; ++i) {
+   rxq = mlx5_rxq_get(dev, i);
+   if (rxq && rxq->ctrl && rxq->ctrl->rxq.lro) {
+   DRV_LOG(ERR, "port %u invalid max LRO 
size",
+   dev->data->port_id);
+   rte_errno = EINVAL;
+   return -rte_errno;
+   }
+   }
+   }
ret = mlx5_dev_configure_rss_reta(dev);
if (ret) {
DRV_LOG(ERR, "port %u reta config failed: %s",
-- 
2.34.1

[PATCH] app/testpmd: fix interactive mode with no ports

[Gregory Etelson]

Testpmd terminated unconditionally if it failed to start all ports.

The patch allows testpmd to get into the command line,
if the interactive mode was requested.

Fixes: 6937d2103e22 ("app/testpmd: add option to not start device")
Signed-off-by: Gregory Etelson 
---
 app/test-pmd/testpmd.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 7381dfd9e5..8517f376db 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -4433,8 +4433,11 @@ main(int argc, char** argv)
}
}
 
-   if (!no_device_start && start_port(RTE_PORT_ALL) != 0)
-   rte_exit(EXIT_FAILURE, "Start ports failed\n");
+   if (!no_device_start && start_port(RTE_PORT_ALL) != 0) {
+   fprintf(stderr, "Start ports failed\n");
+   if (!interactive)
+   goto eal_out;
+   }
 
/* set all ports to promiscuous mode by default */
RTE_ETH_FOREACH_DEV(port_id) {
@@ -4526,6 +4529,7 @@ main(int argc, char** argv)
return 1;
}
 
+eal_out:
ret = rte_eal_cleanup();
if (ret != 0)
rte_exit(EXIT_FAILURE,
-- 
2.34.1

Re: [PATCH v8 0/9] dts: ssh connection to a node

[Owen Hilyard]

On Wed, Nov 9, 2022 at 11:23 AM Honnappa Nagarahalli <
honnappa.nagaraha...@arm.com> wrote:

> 
>
> >
> > 04/11/2022 12:05, Juraj Linkeš:
> > > All the necessary code needed to connect to a node in a topology with
> > > a bit more, such as basic logging and some extra useful methods.
> >
> > Applied, thanks.
> >
> > That's the first step towards integration of DTS in DPDK repository.
> > Nice to see this becoming a reality.
> Thanks Thomas and the community for the reviews and suggestions. This is
> the first important step, hopefully we can continue the collaboration in
> the future releases.
>
> >
> > [...]
> > > This is our current roadmap:
> > > 1. Review this patchset and do the rest of the items in parallel, if
> > > possible.
> > > 2. We have extracted the code needed to run the most basic testcase,
> > > HelloWorld, which runs the DPDK Hello World application. We'll split
> > > this along logical/functional boundaries and send after 1 is done.
> > > 3. Once we have 2 applied, we're planning on adding a basic functional
> > > testcase - pf_smoke. This send a bit of traffic, so the big addition
> > > is the software traffic generator, Scapy. There's some work already
> > > done on Traffic generators we'll be sending as a dependence on this
> > > patch series.
> > > 4. After 3, we'll add a basic performance testcase which doesn't use
> > > Scapy, but Trex or Ixia instead.
> > > 5. This is far in the future, but at this point we should have all of
> > > the core functionality in place. What then remains is adding the rest
> > > of the testcases.
> >
> > Let's join the force and help making this project a pleasant tool.
> >
>
>
I would also like to thank everyone for their reviews and suggestions
toward DTS. The primary goal of this rewrite is make it as easy as possible
to write robust tests so that DPDK can be heavily and reliably tested. This
rewrite allows us at the DTS Working Group to leverage more than 8 years of
experience with the prior DTS to help make it much easier for anyone to
write tests or set up their own testing, but we also want feedback from the
community, so please take a look and give us more feedback.

I would also like to remind everyone that if you have any features that you
want to see in DTS you should bring them to us as soon as possible. We are
starting with support for NICs, but want to be able to branch out to test
all DPDK-supported devices. If you have concerns or required features that
you don't want to discuss on the mailing list for some reason, you can send
your concerns directly to me and I will make sure your needs are
represented in discussions around DTS.

Planned but Unimplemented Features:
* telnet connections for the DUT
* scapy traffic generator (low performance but easy to use)
* trex traffic generator (DPDK-based software traffic generator)
* IXIA traffic generator (hardware traffic generator)
* Traffic generator abstraction layer
* Automatic skipping of tests based on the hardware under test, os and
other factors
* Abstractions to test development to allow simple tests (send this list of
packets then expect this list, etc) to be written as easily as possible.
* Automatic setup and teardown of virtual machines for virtio testing
* The ability to cross compile for a given target on the system running DTS
(assuming relevant libraries/compilers are installed), then install onto
the system under test (intended for embedded systems or SOCs).
* Structured logging for automated analysis
* and many more

[PATCH v2] app/testpmd: fix interactive mode with no ports

[Gregory Etelson]

Testpmd terminated unconditionally if it failed to start all ports.

The patch allows testpmd to get into the command line,
if the interactive mode was requested.

Fixes: 6937d2103e22 ("app/testpmd: add option to not start device")

Cc: step...@networkplumber.org
Cc: sta...@dpdk.org

Signed-off-by: Gregory Etelson 
---
v2: add Cc
---
 app/test-pmd/testpmd.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 7381dfd9e5..8517f376db 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -4433,8 +4433,11 @@ main(int argc, char** argv)
}
}
 
-   if (!no_device_start && start_port(RTE_PORT_ALL) != 0)
-   rte_exit(EXIT_FAILURE, "Start ports failed\n");
+   if (!no_device_start && start_port(RTE_PORT_ALL) != 0) {
+   fprintf(stderr, "Start ports failed\n");
+   if (!interactive)
+   goto eal_out;
+   }
 
/* set all ports to promiscuous mode by default */
RTE_ETH_FOREACH_DEV(port_id) {
@@ -4526,6 +4529,7 @@ main(int argc, char** argv)
return 1;
}
 
+eal_out:
ret = rte_eal_cleanup();
if (ret != 0)
rte_exit(EXIT_FAILURE,
-- 
2.34.1

[PATCH] devtools: set DTS directory to format check

[Thomas Monjalon]

The script was running on the current directory.
If not in the DTS directory, it would re-format every Python files.

A new positional argument is added to specify the directory to check.
In most cases, the (new) default value should be enough.

While updating argument handling,
the usage is printed in case of wrong argument.

Signed-off-by: Thomas Monjalon 
---
 devtools/dts-check-format.sh | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/devtools/dts-check-format.sh b/devtools/dts-check-format.sh
index dc07150775..eb1bdd2a01 100755
--- a/devtools/dts-check-format.sh
+++ b/devtools/dts-check-format.sh
@@ -3,11 +3,10 @@
 # Copyright(c) 2022 University of New Hampshire
 
 usage() {
-   echo "Run formatting and linting programs for DTS. Usage:"
-
+   echo 'Usage: $(basename $0) [options] [directory]'
+   echo 'Options:'
# Get source code comments after getopts arguments and print them both
grep -E '[a-zA-Z]+\) +#' "$0" | tr -d '#'
-   exit 0
 }
 
 format=true
@@ -17,7 +16,9 @@ lint=true
 while getopts "hfl" arg; do
case $arg in
h) # Display this message
+   echo 'Run formatting and linting programs for DTS.'
usage
+   exit 0
;;
f) # Don't run formatters
format=false
@@ -25,10 +26,15 @@ while getopts "hfl" arg; do
l) # Don't run linter
lint=false
;;
-   *)
+   ?)
+   usage
+   exit 1
esac
 done
+shift $(($OPTIND - 1))
 
+directory=${1:-$(dirname $0)/../dts}
+cd $directory || exit 1
 
 errors=0
 
-- 
2.36.1

[PATCH v5] testpmd: cleanup cleanly from signal

[Stephen Hemminger]

Do a clean shutdown of testpmd when a signal is received;
instead of having testpmd kill itself.
This fixes problem where a signal could be received
in the middle of a PMD and then the signal handler would call
PMD's close routine which could cause a deadlock.

Added benefit is it gets rid of Windows specific code.

Fixes: d9a191a00e81 ("app/testpmd: fix quitting in container")
Signed-off-by: Stephen Hemminger 
---
v5 - fix build on Windows.
 Windows has select() function but the prototype is in winsock.h
 not sys/select.h

 app/test-pmd/testpmd.c | 63 +++---
 1 file changed, 34 insertions(+), 29 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index cf5942d0c422..be8fa7f0a0b1 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -11,6 +11,7 @@
 #include 
 #ifndef RTE_EXEC_ENV_WINDOWS
 #include 
+#include 
 #endif
 #include 
 #include 
@@ -4251,26 +4252,11 @@ print_stats(void)
 static void
 signal_handler(int signum)
 {
-   if (signum == SIGINT || signum == SIGTERM) {
-   fprintf(stderr, "\nSignal %d received, preparing to exit...\n",
-   signum);
-#ifdef RTE_LIB_PDUMP
-   /* uninitialize packet capture framework */
-   rte_pdump_uninit();
-#endif
-#ifdef RTE_LIB_LATENCYSTATS
-   if (latencystats_enabled != 0)
-   rte_latencystats_uninit();
-#endif
-   force_quit();
-   /* Set flag to indicate the force termination. */
-   f_quit = 1;
-   /* exit with the expected status */
-#ifndef RTE_EXEC_ENV_WINDOWS
-   signal(signum, SIG_DFL);
-   kill(getpid(), signum);
-#endif
-   }
+   fprintf(stderr, "\nSignal %d received, preparing to exit...\n",
+   signum);
+
+   /* Set flag to indicate the force termination. */
+   f_quit = 1;
 }
 
 int
@@ -4449,9 +4435,6 @@ main(int argc, char** argv)
} else
 #endif
{
-   char c;
-   int rc;
-
f_quit = 0;
 
printf("No commandline core given, start packet forwarding\n");
@@ -4476,15 +4459,37 @@ main(int argc, char** argv)
prev_time = cur_time;
rte_delay_us_sleep(US_PER_S);
}
-   }
+   } else {
+   char c;
+   fd_set fds;
 
-   printf("Press enter to exit\n");
-   rc = read(0, &c, 1);
-   pmd_test_exit();
-   if (rc < 0)
-   return 1;
+   printf("Press enter to exit\n");
+
+   FD_ZERO(&fds);
+   FD_SET(0, &fds);
+
+   if (select(1, &fds, NULL, NULL, NULL) <= 0) {
+   fprintf(stderr, "Select failed: %s\n",
+   strerror(errno));
+   } else if (read(0, &c, 1) <= 0) {
+   fprintf(stderr,
+   "Read stdin failed: %s\n",
+   strerror(errno));
+   }
+   }
+   stop_packet_forwarding();
+   force_quit();
}
 
+#ifdef RTE_LIB_PDUMP
+   /* uninitialize packet capture framework */
+   rte_pdump_uninit();
+#endif
+#ifdef RTE_LIB_LATENCYSTATS
+   if (latencystats_enabled != 0)
+   rte_latencystats_uninit();
+#endif
+
ret = rte_eal_cleanup();
if (ret != 0)
rte_exit(EXIT_FAILURE,
-- 
2.35.1

Re: [PATCH] devtools: set DTS directory to format check

[Owen Hilyard]

On Wed, Nov 9, 2022 at 12:09 PM Thomas Monjalon  wrote:

> The script was running on the current directory.
> If not in the DTS directory, it would re-format every Python files.
>
> A new positional argument is added to specify the directory to check.
> In most cases, the (new) default value should be enough.
>
> While updating argument handling,
> the usage is printed in case of wrong argument.
>
> Signed-off-by: Thomas Monjalon 
> ---
>  devtools/dts-check-format.sh | 14 ++
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/devtools/dts-check-format.sh b/devtools/dts-check-format.sh
> index dc07150775..eb1bdd2a01 100755
> --- a/devtools/dts-check-format.sh
> +++ b/devtools/dts-check-format.sh
> @@ -3,11 +3,10 @@
>  # Copyright(c) 2022 University of New Hampshire
>
>  usage() {
> -   echo "Run formatting and linting programs for DTS. Usage:"
> -
> +   echo 'Usage: $(basename $0) [options] [directory]'
> +   echo 'Options:'
> # Get source code comments after getopts arguments and print them
> both
> grep -E '[a-zA-Z]+\) +#' "$0" | tr -d '#'
> -   exit 0
>  }
>
>  format=true
> @@ -17,7 +16,9 @@ lint=true
>  while getopts "hfl" arg; do
> case $arg in
> h) # Display this message
> +   echo 'Run formatting and linting programs for DTS.'
> usage
> +   exit 0
> ;;
> f) # Don't run formatters
> format=false
> @@ -25,10 +26,15 @@ while getopts "hfl" arg; do
> l) # Don't run linter
> lint=false
> ;;
> -   *)
> +   ?)
> +   usage
> +   exit 1
> esac
>  done
> +shift $(($OPTIND - 1))
>
> +directory=${1:-$(dirname $0)/../dts}
> +cd $directory || exit 1
>
>  errors=0
>
> --
> 2.36.1
>
>
Looks good to me.

Reviewed-by: Owen Hilyard 

RE: [RFC] mempool: zero-copy cache put bulk

[Honnappa Nagarahalli]

> 
> > From: Honnappa Nagarahalli [mailto:honnappa.nagaraha...@arm.com]
> > Sent: Sunday, 6 November 2022 00.11
> >
> > + Akshitha, she is working on similar patch
> >
> > Few comments inline
> >
> > > From: Morten Brørup 
> > > Sent: Saturday, November 5, 2022 8:40 AM
> > >
> > > Zero-copy access to the mempool cache is beneficial for PMD
> > performance,
> > > and must be provided by the mempool library to fix [Bug 1052]
> > > without
> > a
> > > performance regression.
> > >
> > > [Bug 1052]: https://bugs.dpdk.org/show_bug.cgi?id=1052
> > >
> > >
> > > This RFC offers a conceptual zero-copy put function, where the
> > application
> > > promises to store some objects, and in return gets an address where
> > to store
> > > them.
> > >
> > > I would like some early feedback.
> > >
> > > Notes:
> > > * Allowing the 'cache' parameter to be NULL, and getting it from the
> > > mempool instead, was inspired by rte_mempool_cache_flush().
> > I am not sure why the 'cache' parameter is required for this API. This
> > API should take the mem pool as the parameter.
> >
> > We have based our API on 'rte_mempool_do_generic_put' and removed
> the
> > 'cache' parameter.
> 
> I thoroughly considered omitting the 'cache' parameter, but included it for
> two reasons:
> 
> 1. The function is a "mempool cache" function (i.e. primarily working on the
> mempool cache), not a "mempool" function.
> 
> So it is appropriate to have a pointer directly to the structure it is 
> working on.
> Following this through, I also made 'cache' the first parameter and 'mp' the
> second, like in rte_mempool_cache_flush().
I am wondering if the PMD should be aware of the cache or not. For ex: in the 
case of pipeline mode, the RX and TX side of the PMD are running on different 
cores.
However, since the rte_mempool_cache_flush API is provided, may be that 
decision is already done? Interestingly, rte_mempool_cache_flush is called by 
just a single PMD.

So, the question is, should we allow zero-copy only for per-core cache or for 
other cases as well.

> 
> 2. In most cases, the function only accesses the mempool structure in order to
> get the cache pointer. Skipping this step improves performance.
> 
> And since the cache is created along with the mempool itself (and thus never
> changes for a mempool), it would be safe for the PMD to store the 'cache'
> pointer along with the 'mp' pointer in the PMD's queue structure.
Agreed

> 
> E.g. in the i40e PMD the i40e_rx_queue structure could include a "struct
> rte_mempool_cache *cache" field, which could be used i40e_rxq_rearm() [1]
> instead of "cache = rte_mempool_default_cache(rxq->mp, rte_lcore_id())".
> 
> [1] https://elixir.bootlin.com/dpdk/v22.11-
> rc2/source/drivers/net/i40e/i40e_rxtx_vec_avx512.c#L31
> 
> > This new API, on success, returns the pointer to memory where the
> > objects are copied. On failure it returns NULL and the caller has to
> > call 'rte_mempool_ops_enqueue_bulk'. Alternatively, the new API could
> > do this as well and PMD does not need to do anything if it gets a NULL
> > pointer.
> 
> Yes, we agree about these two details:
> 
> 1. The function should return a pointer, not an integer.
> It would be a waste to use a another CPU register to convey a success/error
> integer value, when the success/failure information is just as easily conveyed
> by the pointer return value (non-NULL/NULL), and rte_errno for various error
> values in the unlikely cases.
> 
> 2. The function should leave it up to the PMD what to do if direct access to
> the cache is unavailable.
Just wondering about the advantage of this. I do not think PMD's have much of a 
choice other than calling 'rte_mempool_ops_enqueue_bulk'

> 
> >
> > We should think about providing  similar API on the RX side to keep it
> > symmetric.
> 
> I sent an RFC for that too:
> http://inbox.dpdk.org/dev/98CBD80474FA8B44BF855DF32C47DC35D87488@
> smartserver.smartshare.dk/T/#u
> 
> 
> >
> > > * Asserting that the 'mp' parameter is not NULL is not done by other
> > > functions, so I omitted it here too.
> > >
> > > NB: Please ignore formatting. Also, this code has not even been
> > compile
> > > tested.
> > We are little bit ahead, tested the changes with i40e PF PMD, wrote
> > unit test cases, going through internal review, will send out RFC on
> > Monday
> 
> Sounds good. Looking forward to review.
> 
> >
> > >
> > > /**
> > >  * Promise to put objects in a mempool via zero-copy access to a
> > user-owned
> > > mempool cache.
> > >  *
> > >  * @param cache
> > >  *   A pointer to the mempool cache.
> > >  * @param mp
> > >  *   A pointer to the mempool.
> > >  * @param n
> > >  *   The number of objects to be put in the mempool cache.
> > >  * @return
> > >  *   The pointer to where to put the objects in the mempool cache.
> > >  *   NULL on error
> > >  *   with rte_errno set appropriately.
> > >  */
> > > static __rte_always_inline void *
> > > rte_mempool_cache_put_bulk_promise(struct rte_mempool_cache
> *cache,
> > 

[PATCH v5 1/3] mempool: split stats from debug

[Morten Brørup]

Split stats from debug, to make mempool statistics available without the
performance cost of continuously validating the debug cookies in the
mempool elements.

mempool_perf_autotest shows the following improvements in rate_persec.

The cost of enabling mempool debug without this patch:
-28.1 % and -74.0 %, respectively without and with cache.

The cost of enabling mempool stats (without debug) after this patch:
-5.8 % and -21.2 %, respectively without and with cache.

v5:
* No changes.
v4:
* No changes.
v3:
* Update the Programmer's Guide.
* Update the description of the RTE_MEMPOOL_STAT_ADD macro.
v2:
* Fix checkpatch warning:
  Use C style comments in rte_include.h, not C++ style.
* Do not rename the rte_mempool_debug_stats structure.

Signed-off-by: Morten Brørup 
Reviewed-by: Andrew Rybchenko 
Reviewed-by: Mattias Rönnblom 
Acked-by: Konstantin Ananyev 
---
 config/rte_config.h   |  2 ++
 doc/guides/prog_guide/mempool_lib.rst |  6 +-
 lib/mempool/rte_mempool.c |  6 +++---
 lib/mempool/rte_mempool.h | 11 ++-
 4 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/config/rte_config.h b/config/rte_config.h
index ae56a86394..3c4876d434 100644
--- a/config/rte_config.h
+++ b/config/rte_config.h
@@ -47,6 +47,8 @@
 
 /* mempool defines */
 #define RTE_MEMPOOL_CACHE_MAX_SIZE 512
+/* RTE_LIBRTE_MEMPOOL_STATS is not set */
+/* RTE_LIBRTE_MEMPOOL_DEBUG is not set */
 
 /* mbuf defines */
 #define RTE_MBUF_DEFAULT_MEMPOOL_OPS "ring_mp_mc"
diff --git a/doc/guides/prog_guide/mempool_lib.rst 
b/doc/guides/prog_guide/mempool_lib.rst
index 55838317b9..4f4ee33463 100644
--- a/doc/guides/prog_guide/mempool_lib.rst
+++ b/doc/guides/prog_guide/mempool_lib.rst
@@ -20,12 +20,16 @@ Cookies
 In debug mode, cookies are added at the beginning and end of allocated blocks.
 The allocated objects then contain overwrite protection fields to help 
debugging buffer overflows.
 
+Debug mode is disabled by default, but can be enabled by setting 
``RTE_LIBRTE_MEMPOOL_DEBUG`` in ``config/rte_config.h``.
+
 Stats
 -
 
-In debug mode, statistics about get from/put in the pool are stored in the 
mempool structure.
+In stats mode, statistics about get from/put in the pool are stored in the 
mempool structure.
 Statistics are per-lcore to avoid concurrent access to statistics counters.
 
+Stats mode is disabled by default, but can be enabled by setting 
``RTE_LIBRTE_MEMPOOL_STATS`` in ``config/rte_config.h``.
+
 Memory Alignment Constraints on x86 architecture
 
 
diff --git a/lib/mempool/rte_mempool.c b/lib/mempool/rte_mempool.c
index 21c94a2b9f..62d1ce764e 100644
--- a/lib/mempool/rte_mempool.c
+++ b/lib/mempool/rte_mempool.c
@@ -818,7 +818,7 @@ rte_mempool_create_empty(const char *name, unsigned n, 
unsigned elt_size,
  RTE_CACHE_LINE_MASK) != 0);
RTE_BUILD_BUG_ON((sizeof(struct rte_mempool_cache) &
  RTE_CACHE_LINE_MASK) != 0);
-#ifdef RTE_LIBRTE_MEMPOOL_DEBUG
+#ifdef RTE_LIBRTE_MEMPOOL_STATS
RTE_BUILD_BUG_ON((sizeof(struct rte_mempool_debug_stats) &
  RTE_CACHE_LINE_MASK) != 0);
RTE_BUILD_BUG_ON((offsetof(struct rte_mempool, stats) &
@@ -1221,7 +1221,7 @@ rte_mempool_audit(struct rte_mempool *mp)
 void
 rte_mempool_dump(FILE *f, struct rte_mempool *mp)
 {
-#ifdef RTE_LIBRTE_MEMPOOL_DEBUG
+#ifdef RTE_LIBRTE_MEMPOOL_STATS
struct rte_mempool_info info;
struct rte_mempool_debug_stats sum;
unsigned lcore_id;
@@ -1269,7 +1269,7 @@ rte_mempool_dump(FILE *f, struct rte_mempool *mp)
fprintf(f, "  common_pool_count=%u\n", common_count);
 
/* sum and dump statistics */
-#ifdef RTE_LIBRTE_MEMPOOL_DEBUG
+#ifdef RTE_LIBRTE_MEMPOOL_STATS
rte_mempool_ops_get_info(mp, &info);
memset(&sum, 0, sizeof(sum));
for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
diff --git a/lib/mempool/rte_mempool.h b/lib/mempool/rte_mempool.h
index 3725a72951..d5f7ea99fa 100644
--- a/lib/mempool/rte_mempool.h
+++ b/lib/mempool/rte_mempool.h
@@ -56,7 +56,7 @@ extern "C" {
 #define RTE_MEMPOOL_HEADER_COOKIE2  0xf2eef2eedadd2e55ULL /**< Header cookie. 
*/
 #define RTE_MEMPOOL_TRAILER_COOKIE  0xadd2e55badbadbadULL /**< Trailer 
cookie.*/
 
-#ifdef RTE_LIBRTE_MEMPOOL_DEBUG
+#ifdef RTE_LIBRTE_MEMPOOL_STATS
 /**
  * A structure that stores the mempool statistics (per-lcore).
  * Note: Cache stats (put_cache_bulk/objs, get_cache_bulk/objs) are not
@@ -237,7 +237,7 @@ struct rte_mempool {
uint32_t nb_mem_chunks;  /**< Number of memory chunks */
struct rte_mempool_memhdr_list mem_list; /**< List of memory chunks */
 
-#ifdef RTE_LIBRTE_MEMPOOL_DEBUG
+#ifdef RTE_LIBRTE_MEMPOOL_STATS
/** Per-lcore statistics. */
struct rte_mempool_debug_stats stats[RTE_MAX_LCORE];
 #endif
@@ -292,17 +292,18 @@ struct rte_mempool {
| RTE_MEMPOOL_F_SC_GET \
| RTE_MEMPOOL_F_NO

[PATCH v5 2/3] mempool: add stats for unregistered non-EAL threads

[Morten Brørup]

This patch adds statistics for unregistered non-EAL threads, which was
previously not included in the statistics.

Add one more entry to the stats array, and use the last index for
unregistered non-EAL threads.

The unregistered non-EAL thread statistics are incremented atomically.

In theory, the EAL thread counters should also be accessed atomically to
avoid tearing on 32 bit architectures. However, it was decided to avoid
the performance cost of using atomic operations, because:
1. these are debug counters, and
2. statistics counters in DPDK are usually incremented non-atomically.

v5:
* Add parentheses around n in the RTE_MEMPOOL_STAT_ADD macro.
v4:
* No changes.
v3 (feedback from Mattias Rönnblom):
* Use correct terminology: Unregistered non-EAL threads.
* Use atomic counting for the unregistered non-EAL threads.
* Reintroduce the conditional instead of offsetting the index by one.
v2:
* New. No v1 of this patch in the series.

Suggested-by: Stephen Hemminger 
Signed-off-by: Morten Brørup 
Reviewed-by: Andrew Rybchenko 
Reviewed-by: Mattias Rönnblom 
Acked-by: Konstantin Ananyev 
---
 lib/mempool/rte_mempool.c |  2 +-
 lib/mempool/rte_mempool.h | 19 ---
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/lib/mempool/rte_mempool.c b/lib/mempool/rte_mempool.c
index 62d1ce764e..e6208125e0 100644
--- a/lib/mempool/rte_mempool.c
+++ b/lib/mempool/rte_mempool.c
@@ -1272,7 +1272,7 @@ rte_mempool_dump(FILE *f, struct rte_mempool *mp)
 #ifdef RTE_LIBRTE_MEMPOOL_STATS
rte_mempool_ops_get_info(mp, &info);
memset(&sum, 0, sizeof(sum));
-   for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
+   for (lcore_id = 0; lcore_id < RTE_MAX_LCORE + 1; lcore_id++) {
sum.put_bulk += mp->stats[lcore_id].put_bulk;
sum.put_objs += mp->stats[lcore_id].put_objs;
sum.put_common_pool_bulk += 
mp->stats[lcore_id].put_common_pool_bulk;
diff --git a/lib/mempool/rte_mempool.h b/lib/mempool/rte_mempool.h
index d5f7ea99fa..d8ccb06e2e 100644
--- a/lib/mempool/rte_mempool.h
+++ b/lib/mempool/rte_mempool.h
@@ -238,8 +238,11 @@ struct rte_mempool {
struct rte_mempool_memhdr_list mem_list; /**< List of memory chunks */
 
 #ifdef RTE_LIBRTE_MEMPOOL_STATS
-   /** Per-lcore statistics. */
-   struct rte_mempool_debug_stats stats[RTE_MAX_LCORE];
+   /** Per-lcore statistics.
+*
+* Plus one, for unregistered non-EAL threads.
+*/
+   struct rte_mempool_debug_stats stats[RTE_MAX_LCORE + 1];
 #endif
 }  __rte_cache_aligned;
 
@@ -304,11 +307,13 @@ struct rte_mempool {
  *   Number to add to the statistics.
  */
 #ifdef RTE_LIBRTE_MEMPOOL_STATS
-#define RTE_MEMPOOL_STAT_ADD(mp, name, n) do {  \
-   unsigned __lcore_id = rte_lcore_id();   \
-   if (__lcore_id < RTE_MAX_LCORE) {   \
-   mp->stats[__lcore_id].name += n;\
-   }   \
+#define RTE_MEMPOOL_STAT_ADD(mp, name, n) do { 
 \
+   unsigned int __lcore_id = rte_lcore_id();   
\
+   if (likely(__lcore_id < RTE_MAX_LCORE)) 
\
+   (mp)->stats[__lcore_id].name += (n);
\
+   else
\
+   __atomic_fetch_add(&((mp)->stats[RTE_MAX_LCORE].name),  
\
+  (n), __ATOMIC_RELAXED);  
\
} while (0)
 #else
 #define RTE_MEMPOOL_STAT_ADD(mp, name, n) do {} while (0)
-- 
2.17.1

[PATCH v5 3/3] mempool: use cache for frequently updated stats

[Morten Brørup]

When built with stats enabled (RTE_LIBRTE_MEMPOOL_STATS defined), the
performance of mempools with caches is improved as follows.

When accessing objects in the mempool, either the put_bulk and put_objs or
the get_success_bulk and get_success_objs statistics counters are likely
to be incremented.

By adding an alternative set of these counters to the mempool cache
structure, accessing the dedicated statistics structure is avoided in the
likely cases where these counters are incremented.

The trick here is that the cache line holding the mempool cache structure
is accessed anyway, in order to access the 'len' or 'flushthresh' fields.
Updating some statistics counters in the same cache line has lower
performance cost than accessing the statistics counters in the dedicated
statistics structure, which resides in another cache line.

mempool_perf_autotest with this patch shows the following improvements in
rate_persec.

The cost of enabling mempool stats (without debug) after this patch:
-6.8 % and -6.7 %, respectively without and with cache.

v5:
* Add SmartShare Systems to the list of copyright holders.
* Add parentheses around n in the RTE_MEMPOOL_CACHE_STAT_ADD macro.
* Fix checkpatch warning:
  The macro to add to a mempool cache stat variable should be enclosed in
  parentheses.
v4:
* Fix checkpatch warnings:
  A couple of typos in the patch description.
  The macro to add to a mempool cache stat variable should not use
  do {} while (0). Personally, I would tend to disagree with this, but
  whatever keeps the CI happy.
v3:
* Don't update the description of the RTE_MEMPOOL_STAT_ADD macro.
  This change belongs in the first patch of the series.
v2:
* Move the statistics counters into a stats structure.

Signed-off-by: Morten Brørup 
Reviewed-by: Andrew Rybchenko 
Reviewed-by: Mattias Rönnblom 
Acked-by: Konstantin Ananyev 
---
 lib/mempool/rte_mempool.c | 10 ++
 lib/mempool/rte_mempool.h | 67 ---
 2 files changed, 66 insertions(+), 11 deletions(-)

diff --git a/lib/mempool/rte_mempool.c b/lib/mempool/rte_mempool.c
index e6208125e0..f33f455790 100644
--- a/lib/mempool/rte_mempool.c
+++ b/lib/mempool/rte_mempool.c
@@ -1,6 +1,7 @@
 /* SPDX-License-Identifier: BSD-3-Clause
  * Copyright(c) 2010-2014 Intel Corporation.
  * Copyright(c) 2016 6WIND S.A.
+ * Copyright(c) 2022 SmartShare Systems
  */
 
 #include 
@@ -1286,6 +1287,15 @@ rte_mempool_dump(FILE *f, struct rte_mempool *mp)
sum.get_success_blks += mp->stats[lcore_id].get_success_blks;
sum.get_fail_blks += mp->stats[lcore_id].get_fail_blks;
}
+   if (mp->cache_size != 0) {
+   /* Add the statistics stored in the mempool caches. */
+   for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
+   sum.put_bulk += 
mp->local_cache[lcore_id].stats.put_bulk;
+   sum.put_objs += 
mp->local_cache[lcore_id].stats.put_objs;
+   sum.get_success_bulk += 
mp->local_cache[lcore_id].stats.get_success_bulk;
+   sum.get_success_objs += 
mp->local_cache[lcore_id].stats.get_success_objs;
+   }
+   }
fprintf(f, "  stats:\n");
fprintf(f, "put_bulk=%"PRIu64"\n", sum.put_bulk);
fprintf(f, "put_objs=%"PRIu64"\n", sum.put_objs);
diff --git a/lib/mempool/rte_mempool.h b/lib/mempool/rte_mempool.h
index d8ccb06e2e..631f205d53 100644
--- a/lib/mempool/rte_mempool.h
+++ b/lib/mempool/rte_mempool.h
@@ -1,6 +1,7 @@
 /* SPDX-License-Identifier: BSD-3-Clause
  * Copyright(c) 2010-2014 Intel Corporation.
  * Copyright(c) 2016 6WIND S.A.
+ * Copyright(c) 2022 SmartShare Systems
  */
 
 #ifndef _RTE_MEMPOOL_H_
@@ -86,6 +87,19 @@ struct rte_mempool_cache {
uint32_t size;/**< Size of the cache */
uint32_t flushthresh; /**< Threshold before we flush excess elements */
uint32_t len; /**< Current cache count */
+#ifdef RTE_LIBRTE_MEMPOOL_STATS
+   uint32_t unused;
+   /*
+* Alternative location for the most frequently updated mempool 
statistics (per-lcore),
+* providing faster update access when using a mempool cache.
+*/
+   struct {
+   uint64_t put_bulk;  /**< Number of puts. */
+   uint64_t put_objs;  /**< Number of objects successfully 
put. */
+   uint64_t get_success_bulk;  /**< Successful allocation number. 
*/
+   uint64_t get_success_objs;  /**< Objects successfully 
allocated. */
+   } stats;/**< Statistics */
+#endif
/**
 * Cache objects
 *
@@ -319,6 +333,22 @@ struct rte_mempool {
 #define RTE_MEMPOOL_STAT_ADD(mp, name, n) do {} while (0)
 #endif
 
+/**
+ * @internal When stats is enabled, store some statistics.
+ *
+ * @param cache
+ *   Pointer to the memory pool cache.
+ * @param name
+ *   Name of the statistics field to increment in the memory pool cache.

[PATCH] doc: support IPsec Multi-buffer lib v1.3

[Pablo de Lara]

Updated AESNI MB and AESNI GCM, KASUMI, ZUC and SNOW3G PMD documentation
guides with information about the latest Intel IPSec Multi-buffer
library supported.

Signed-off-by: Pablo de Lara 
---
 doc/guides/cryptodevs/aesni_gcm.rst |  8 
 doc/guides/cryptodevs/aesni_mb.rst  | 18 --
 doc/guides/cryptodevs/kasumi.rst| 15 +++
 doc/guides/cryptodevs/snow3g.rst| 15 +++
 doc/guides/cryptodevs/zuc.rst   | 14 ++
 5 files changed, 48 insertions(+), 22 deletions(-)

diff --git a/doc/guides/cryptodevs/aesni_gcm.rst 
b/doc/guides/cryptodevs/aesni_gcm.rst
index 6229392f58..5192287ed8 100644
--- a/doc/guides/cryptodevs/aesni_gcm.rst
+++ b/doc/guides/cryptodevs/aesni_gcm.rst
@@ -40,8 +40,8 @@ Installation
 To build DPDK with the AESNI_GCM_PMD the user is required to download the 
multi-buffer
 library from `here `_
 and compile it on their user system before building DPDK.
-The latest version of the library supported by this PMD is v1.2, which
-can be downloaded in 
``_.
+The latest version of the library supported by this PMD is v1.3, which
+can be downloaded in 
``_.
 
 .. code-block:: console
 
@@ -84,8 +84,8 @@ and the external crypto libraries supported by them:
17.08 - 18.02  Multi-buffer library 0.46 - 0.48
18.05 - 19.02  Multi-buffer library 0.49 - 0.52
19.05 - 20.08  Multi-buffer library 0.52 - 0.55
-   20.11 - 21.08  Multi-buffer library 0.53 - 1.2*
-   21.11+ Multi-buffer library 1.0  - 1.2*
+   20.11 - 21.08  Multi-buffer library 0.53 - 1.3*
+   21.11+ Multi-buffer library 1.0  - 1.3*
=  
 
 \* Multi-buffer library 1.0 or newer only works for Meson but not Make build 
system.
diff --git a/doc/guides/cryptodevs/aesni_mb.rst 
b/doc/guides/cryptodevs/aesni_mb.rst
index 599ed5698f..3d0cd3de85 100644
--- a/doc/guides/cryptodevs/aesni_mb.rst
+++ b/doc/guides/cryptodevs/aesni_mb.rst
@@ -1,7 +1,7 @@
 ..  SPDX-License-Identifier: BSD-3-Clause
 Copyright(c) 2015-2018 Intel Corporation.
 
-AESN-NI Multi Buffer Crypto Poll Mode Driver
+AES-NI Multi Buffer Crypto Poll Mode Driver
 
 
 
@@ -10,8 +10,6 @@ support for utilizing Intel multi buffer library, see the 
white paper
 `Fast Multi-buffer IPsec Implementations on Intel® Architecture Processors
 
`_.
 
-The AES-NI MB PMD has current only been tested on Fedora 21 64-bit with gcc.
-
 The AES-NI MB PMD supports synchronous mode of operation with
 ``rte_cryptodev_sym_cpu_crypto_process`` function call.
 
@@ -77,6 +75,14 @@ Limitations
 * RTE_CRYPTO_CIPHER_DES_DOCSISBPI is not supported for combined Crypto-CRC
   DOCSIS security protocol.
 
+AESNI MB PMD selection over SNOW3G/ZUC/KASUMI PMDs
+--
+
+This PMD supports wireless cipher suite (SNOW3G, ZUC and KASUMI).
+On Intel processors, it is recommended to use this PMD instead of SNOW3G, ZUC 
and KASUMI PMDs,
+as it enables algorithm mixing (e.g. cipher algorithm SNOW3G-UEA2 with
+authentication algorithm AES-CMAC-128) and performance over IMIX (packet size 
mix) traffic
+is significantly higher.
 
 Installation
 
@@ -84,8 +90,8 @@ Installation
 To build DPDK with the AESNI_MB_PMD the user is required to download the 
multi-buffer
 library from `here `_
 and compile it on their user system before building DPDK.
-The latest version of the library supported by this PMD is v1.2, which
-can be downloaded from 
``_.
+The latest version of the library supported by this PMD is v1.3, which
+can be downloaded from 
``_.
 
 .. code-block:: console
 
@@ -131,7 +137,7 @@ and the Multi-Buffer library version supported by them:
19.05 - 19.08   0.52
19.11 - 20.08   0.52 - 0.55
20.11 - 21.08   0.53 - 1.2*
-   21.11+  1.0  - 1.2*
+   21.11+  1.0  - 1.3*
==  
 
 \* Multi-buffer library 1.0 or newer only works for Meson but not Make build 
system.
diff --git a/doc/guides/cryptodevs/kasumi.rst b/doc/guides/cryptodevs/kasumi.rst
index d8128928f8..dc3a300fb5 100644
--- a/doc/guides/cryptodevs/kasumi.rst
+++ b/doc/guides/cryptodevs/kasumi.rst
@@ -30,14 +30,21 @@ Limitations
   (if length and/or offset of data to be ciphered is not byte-aligned).
 
 
+KASUMI PMD vs AESNI MB PMD
+--
+
+AESNI MB PMD also supports KASUMI cipher and authentication algorithms,
+It is recommended to use the AESNI MB PMD, which offers better performance on 
Intel processors.
+Take a l

[PATCH v2] doc: support IPsec Multi-buffer lib v1.3

[Pablo de Lara]

Updated AESNI MB and AESNI GCM, KASUMI, ZUC and SNOW3G PMD documentation
guides with information about the latest Intel IPSec Multi-buffer
library supported.

Signed-off-by: Pablo de Lara 
---

- v2: Removed repeated word 'the'

---
 doc/guides/cryptodevs/aesni_gcm.rst |  8 
 doc/guides/cryptodevs/aesni_mb.rst  | 18 --
 doc/guides/cryptodevs/kasumi.rst| 15 +++
 doc/guides/cryptodevs/snow3g.rst| 15 +++
 doc/guides/cryptodevs/zuc.rst   | 14 ++
 5 files changed, 48 insertions(+), 22 deletions(-)

diff --git a/doc/guides/cryptodevs/aesni_gcm.rst 
b/doc/guides/cryptodevs/aesni_gcm.rst
index 6229392f58..5192287ed8 100644
--- a/doc/guides/cryptodevs/aesni_gcm.rst
+++ b/doc/guides/cryptodevs/aesni_gcm.rst
@@ -40,8 +40,8 @@ Installation
 To build DPDK with the AESNI_GCM_PMD the user is required to download the 
multi-buffer
 library from `here `_
 and compile it on their user system before building DPDK.
-The latest version of the library supported by this PMD is v1.2, which
-can be downloaded in 
``_.
+The latest version of the library supported by this PMD is v1.3, which
+can be downloaded in 
``_.
 
 .. code-block:: console
 
@@ -84,8 +84,8 @@ and the external crypto libraries supported by them:
17.08 - 18.02  Multi-buffer library 0.46 - 0.48
18.05 - 19.02  Multi-buffer library 0.49 - 0.52
19.05 - 20.08  Multi-buffer library 0.52 - 0.55
-   20.11 - 21.08  Multi-buffer library 0.53 - 1.2*
-   21.11+ Multi-buffer library 1.0  - 1.2*
+   20.11 - 21.08  Multi-buffer library 0.53 - 1.3*
+   21.11+ Multi-buffer library 1.0  - 1.3*
=  
 
 \* Multi-buffer library 1.0 or newer only works for Meson but not Make build 
system.
diff --git a/doc/guides/cryptodevs/aesni_mb.rst 
b/doc/guides/cryptodevs/aesni_mb.rst
index 599ed5698f..3d0cd3de85 100644
--- a/doc/guides/cryptodevs/aesni_mb.rst
+++ b/doc/guides/cryptodevs/aesni_mb.rst
@@ -1,7 +1,7 @@
 ..  SPDX-License-Identifier: BSD-3-Clause
 Copyright(c) 2015-2018 Intel Corporation.
 
-AESN-NI Multi Buffer Crypto Poll Mode Driver
+AES-NI Multi Buffer Crypto Poll Mode Driver
 
 
 
@@ -10,8 +10,6 @@ support for utilizing Intel multi buffer library, see the 
white paper
 `Fast Multi-buffer IPsec Implementations on Intel® Architecture Processors
 
`_.
 
-The AES-NI MB PMD has current only been tested on Fedora 21 64-bit with gcc.
-
 The AES-NI MB PMD supports synchronous mode of operation with
 ``rte_cryptodev_sym_cpu_crypto_process`` function call.
 
@@ -77,6 +75,14 @@ Limitations
 * RTE_CRYPTO_CIPHER_DES_DOCSISBPI is not supported for combined Crypto-CRC
   DOCSIS security protocol.
 
+AESNI MB PMD selection over SNOW3G/ZUC/KASUMI PMDs
+--
+
+This PMD supports wireless cipher suite (SNOW3G, ZUC and KASUMI).
+On Intel processors, it is recommended to use this PMD instead of SNOW3G, ZUC 
and KASUMI PMDs,
+as it enables algorithm mixing (e.g. cipher algorithm SNOW3G-UEA2 with
+authentication algorithm AES-CMAC-128) and performance over IMIX (packet size 
mix) traffic
+is significantly higher.
 
 Installation
 
@@ -84,8 +90,8 @@ Installation
 To build DPDK with the AESNI_MB_PMD the user is required to download the 
multi-buffer
 library from `here `_
 and compile it on their user system before building DPDK.
-The latest version of the library supported by this PMD is v1.2, which
-can be downloaded from 
``_.
+The latest version of the library supported by this PMD is v1.3, which
+can be downloaded from 
``_.
 
 .. code-block:: console
 
@@ -131,7 +137,7 @@ and the Multi-Buffer library version supported by them:
19.05 - 19.08   0.52
19.11 - 20.08   0.52 - 0.55
20.11 - 21.08   0.53 - 1.2*
-   21.11+  1.0  - 1.2*
+   21.11+  1.0  - 1.3*
==  
 
 \* Multi-buffer library 1.0 or newer only works for Meson but not Make build 
system.
diff --git a/doc/guides/cryptodevs/kasumi.rst b/doc/guides/cryptodevs/kasumi.rst
index d8128928f8..c8e8f1b847 100644
--- a/doc/guides/cryptodevs/kasumi.rst
+++ b/doc/guides/cryptodevs/kasumi.rst
@@ -30,14 +30,21 @@ Limitations
   (if length and/or offset of data to be ciphered is not byte-aligned).
 
 
+KASUMI PMD vs AESNI MB PMD
+--
+
+AESNI MB PMD also supports KASUMI cipher and authentication algorithms,
+It is recommended to use the AESNI MB PMD, which offers better per

[PATCH v2 0/3] ethdev: document special cases of port start and stop

[Dariusz Sosnowski]

This patch series attempts to address the special failure
cases of rte_eth_dev_stop() and rte_eth_dev_start().

In case of starting a port, If the port depends on another one
being started, PMDs might return (-EAGAIN) to notify
about such situation.

In case of stopping a port, If the port cannot be stopped,
because other port depends on its resources,
PMDs might return (-EBUSY) to notify about such situation.

These cases are addressed in ethdev API docs.
Testpmd is updated to allow users to manually retry start/stop operations.

v2:
- Fixed documentation build.

Dariusz Sosnowski (3):
  net/mlx5: fix log level on failed transfer proxy stop
  doc: document E-Switch limitations with HWS in mlx5 PMD
  ethdev: document special cases of port start and stop

 app/test-pmd/testpmd.c  | 10 +-
 doc/guides/nics/mlx5.rst| 13 +
 drivers/net/mlx5/mlx5_trigger.c |  6 +++---
 lib/ethdev/rte_ethdev.h |  9 +
 4 files changed, 34 insertions(+), 4 deletions(-)

-- 
2.25.1

[PATCH v2 1/3] net/mlx5: fix log level on failed transfer proxy stop

[Dariusz Sosnowski]

This patches increases log level for error reporting when stopping
the transfer proxy port failed. Stopping can fail with EBUSY when
related representor ports are still running.

Fixes: 483181f7b6dd ("net/mlx5: support device control of representor matching")

Signed-off-by: Dariusz Sosnowski 
---
 drivers/net/mlx5/mlx5_trigger.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_trigger.c b/drivers/net/mlx5/mlx5_trigger.c
index 4b821a1076..310a84729e 100644
--- a/drivers/net/mlx5/mlx5_trigger.c
+++ b/drivers/net/mlx5/mlx5_trigger.c
@@ -1361,9 +1361,9 @@ mlx5_hw_proxy_port_allowed_stop(struct rte_eth_dev *dev)
representor_started = true;
}
if (representor_started) {
-   DRV_LOG(INFO, "Failed to stop port %u: attached representor 
ports"
- " must be stopped before stopping transfer proxy 
port",
- dev->data->port_id);
+   DRV_LOG(ERR, "Failed to stop port %u: attached representor 
ports"
+" must be stopped before stopping transfer proxy 
port",
+dev->data->port_id);
rte_errno = EBUSY;
return -rte_errno;
}
-- 
2.25.1

[PATCH v2 3/3] ethdev: document special cases of port start and stop

[Dariusz Sosnowski]

This patch clarifies the handling of following cases
in the ethdev API docs:

- If rte_eth_dev_start() returns (-EAGAIN) for some port,
  it cannot be started until other port is started.
- If rte_eth_dev_stop() returns (-EBUSY) for some port,
  it cannot be stopped until other port is stopped.

When stopping the port in testpmd fails due to (-EBUSY),
port's state is switched back to STARTED
to allow users to manually retry stopping the port.

No additional changes in testpmd are required to handle
failure to start port with (-EAGAIN).
If rte_eth_dev_start() fails, port's state is switched to STOPPED
and users are allowed to retry the operation.

Signed-off-by: Dariusz Sosnowski 
---
 app/test-pmd/testpmd.c  | 10 +-
 lib/ethdev/rte_ethdev.h |  9 +
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index aa7ea29f15..5a69e3c77a 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -3118,6 +3118,7 @@ stop_port(portid_t pid)
int need_check_link_status = 0;
portid_t peer_pl[RTE_MAX_ETHPORTS];
int peer_pi;
+   int ret;
 
if (port_id_is_invalid(pid, ENABLED_WARN))
return;
@@ -3167,9 +3168,16 @@ stop_port(portid_t pid)
if (port->flow_list)
port_flow_flush(pi);
 
-   if (eth_dev_stop_mp(pi) != 0)
+   ret = eth_dev_stop_mp(pi);
+   if (ret != 0) {
RTE_LOG(ERR, EAL, "rte_eth_dev_stop failed for port 
%u\n",
pi);
+   if (ret == -EBUSY) {
+   /* Allow to retry stopping the port. */
+   port->port_status = RTE_PORT_STARTED;
+   continue;
+   }
+   }
 
if (port->port_status == RTE_PORT_HANDLING)
port->port_status = RTE_PORT_STOPPED;
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 13fe73d5a3..abf5a24f92 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -2701,10 +2701,14 @@ int rte_eth_dev_tx_queue_stop(uint16_t port_id, 
uint16_t tx_queue_id);
  * On success, all basic functions exported by the Ethernet API (link status,
  * receive/transmit, and so on) can be invoked.
  *
+ * If the port depends on another one being started,
+ * PMDs might return (-EAGAIN) to notify about such requirement.
+ *
  * @param port_id
  *   The port identifier of the Ethernet device.
  * @return
  *   - 0: Success, Ethernet device started.
+ *   - -EAGAIN: If it depends on another port to be started first.
  *   - <0: Error code of the driver device start function.
  */
 int rte_eth_dev_start(uint16_t port_id);
@@ -2713,10 +2717,15 @@ int rte_eth_dev_start(uint16_t port_id);
  * Stop an Ethernet device. The device can be restarted with a call to
  * rte_eth_dev_start()
  *
+ * If the port provides some resources for other ports
+ * and it cannot be stopped before them,
+ * PMDs might return (-EBUSY) to notify about such requirement.
+ *
  * @param port_id
  *   The port identifier of the Ethernet device.
  * @return
  *   - 0: Success, Ethernet device stopped.
+ *   - -EBUSY: If it depends on another port to be stopped first.
  *   - <0: Error code of the driver device stop function.
  */
 int rte_eth_dev_stop(uint16_t port_id);
-- 
2.25.1

[PATCH v2 2/3] doc: document E-Switch limitations with HWS in mlx5 PMD

[Dariusz Sosnowski]

This patch adds the following limitations to the mlx5 PMD guide:

- With HW Steering and E-Switch enabled, transfer proxy port must
  be started before any port representor.
- With HW Steering and E-Switch enabled, all representors
  must be stopped before transfer proxy port is stopped.

Signed-off-by: Dariusz Sosnowski 
---
 doc/guides/nics/mlx5.rst | 13 +
 1 file changed, 13 insertions(+)

diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index d5f9375a4e..ca555e7ca7 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -161,6 +161,19 @@ Limitations
   - NIC ConnectX-5 and before are not supported.
   - Partial match with item template is not supported.
   - IPv6 5-tuple matching is not supported.
+  - With E-Switch enabled, ports which share the E-Switch domain
+should be started and stopped in a specific order:
+
+- When starting ports, the transfer proxy port should be started first
+  and port representors should follow.
+- When stopping ports, all of the port representors
+  should be stopped before stopping the transfer proxy port.
+
+If ports are started/stopped in an incorrect order,
+``rte_eth_dev_start()``/``rte_eth_dev_stop()`` will return an appropriate 
error code:
+
+- ``-EAGAIN`` for ``rte_eth_dev_start()``.
+- ``-EBUSY`` for ``rte_eth_dev_stop()``.
 
 - When using Verbs flow engine (``dv_flow_en`` = 0), flow pattern without any
   specific VLAN will match for VLAN packets as well:
-- 
2.25.1

[PATCH 00/14] whitespace around keywords

[Stephen Hemminger]

The DPDK standard now enforced through checkpatch is to use
a space after keywords. Much of the older code that was grandfathered
in doesn't follow that, so fix it.

No urgency to this patchset, and it might just be viewed as churn.
But I prefer to read code that looks all the same.

“A foolish consistency is the hobgoblin of little minds, adored by
little statesmen and philosophers and divines. With consistency a
great soul has simply nothing to do. He may as well concern himself
with his shadow on the wall."  -- Ralph Waldo Emerson

Stephen Hemminger (14):
  eal: fix whitespace
  cmdline: fix whitespace
  timer: fix whitespace
  ip_frag: fix whitespace
  testpmd: fix whitspace
  test: fix whitespace
  examples/qos_sched: fix whitespace
  examples/vhost: fix whitespace
  l3fwd: fix whitespace
  examples: fix whitespace
  bus/pci: fix whitespace
  net/e1000: fix whitespace
  net/i40e: fix whitespace
  net/bnx2x: fix whitespace

 app/test-pmd/cmdline.c | 30 +++---
 app/test-pmd/parameters.c  |  8 +++---
 app/test-pmd/testpmd.c |  2 +-
 app/test/test_cmdline_cirbuf.c |  4 +--
 app/test/test_debug.c  |  4 +--
 app/test/test_hash.c   |  8 +++---
 app/test/test_lpm6_perf.c  |  2 +-
 app/test/test_malloc.c | 14 +-
 app/test/test_mbuf.c   | 12 -
 app/test/test_spinlock.c   |  2 +-
 drivers/bus/pci/bsd/pci.c  |  2 +-
 drivers/net/bnx2x/bnx2x.h  |  2 +-
 drivers/net/e1000/em_ethdev.c  |  4 +--
 drivers/net/e1000/igb_ethdev.c |  8 +++---
 drivers/net/i40e/i40e_pf.c |  8 +++---
 examples/cmdline/parse_obj_list.c  |  2 +-
 examples/ip_reassembly/main.c  |  2 +-
 examples/l3fwd-power/main.c|  6 ++---
 examples/l3fwd/main.c  |  6 ++---
 examples/multi_process/symmetric_mp/main.c |  6 ++---
 examples/qos_sched/app_thread.c|  6 ++---
 examples/qos_sched/args.c  |  2 +-
 examples/qos_sched/init.c  |  2 +-
 examples/qos_sched/main.c  |  4 +--
 examples/vhost/main.c  |  6 ++---
 lib/cmdline/cmdline_parse_string.c | 10 
 lib/cmdline/cmdline_rdline.c   |  6 ++---
 lib/eal/linux/eal_hugepage_info.c  |  6 ++---
 lib/eal/linux/eal_interrupts.c |  2 +-
 lib/ip_frag/rte_ipv4_reassembly.c  |  2 +-
 lib/timer/rte_timer.c  |  2 +-
 31 files changed, 90 insertions(+), 90 deletions(-)

-- 
2.35.1

[PATCH 01/14] eal: fix whitespace

[Stephen Hemminger]

Add space after keywords.

Signed-off-by: Stephen Hemminger 
---
 lib/eal/linux/eal_hugepage_info.c | 6 +++---
 lib/eal/linux/eal_interrupts.c| 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/eal/linux/eal_hugepage_info.c 
b/lib/eal/linux/eal_hugepage_info.c
index a1b6cb31ff1a..b6d751a2ee97 100644
--- a/lib/eal/linux/eal_hugepage_info.c
+++ b/lib/eal/linux/eal_hugepage_info.c
@@ -180,8 +180,8 @@ get_default_hp_size(void)
FILE *fd = fopen(proc_meminfo, "r");
if (fd == NULL)
rte_panic("Cannot open %s\n", proc_meminfo);
-   while(fgets(buffer, sizeof(buffer), fd)){
-   if (strncmp(buffer, str_hugepagesz, hugepagesz_len) == 0){
+   while (fgets(buffer, sizeof(buffer), fd)) {
+   if (strncmp(buffer, str_hugepagesz, hugepagesz_len) == 0) {
size = rte_str_to_size(&buffer[hugepagesz_len]);
break;
}
@@ -231,7 +231,7 @@ get_hugepage_dir(uint64_t hugepage_sz, char *hugedir, int 
len)
if (default_size == 0)
default_size = get_default_hp_size();
 
-   while (fgets(buf, sizeof(buf), fd)){
+   while (fgets(buf, sizeof(buf), fd)) {
const char *pagesz_str;
 
if (rte_strsplit(buf, sizeof(buf), splitstr, _FIELDNAME_MAX,
diff --git a/lib/eal/linux/eal_interrupts.c b/lib/eal/linux/eal_interrupts.c
index d52ec8eb4cd7..3be1b090be9e 100644
--- a/lib/eal/linux/eal_interrupts.c
+++ b/lib/eal/linux/eal_interrupts.c
@@ -1073,7 +1073,7 @@ eal_intr_handle_interrupts(int pfd, unsigned totalfds)
struct epoll_event events[totalfds];
int nfds = 0;
 
-   for(;;) {
+   for (;;) {
nfds = epoll_wait(pfd, events, totalfds,
EAL_INTR_EPOLL_WAIT_FOREVER);
/* epoll_wait fail */
-- 
2.35.1

[PATCH 02/14] cmdline: fix whitespace

[Stephen Hemminger]

Add space after keywords.
Remove unnecessary spaces in if expressions.

Signed-off-by: Stephen Hemminger 
---
 lib/cmdline/cmdline_parse_string.c | 10 +-
 lib/cmdline/cmdline_rdline.c   |  6 +++---
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/lib/cmdline/cmdline_parse_string.c 
b/lib/cmdline/cmdline_parse_string.c
index d756638905a2..0b53d0c800cc 100644
--- a/lib/cmdline/cmdline_parse_string.c
+++ b/lib/cmdline/cmdline_parse_string.c
@@ -77,16 +77,16 @@ cmdline_parse_string(cmdline_parse_token_hdr_t *tk, const 
char *buf, void *res,
continue;
}
 
-   if ( strncmp(buf, str, token_len) ) {
+   if (strncmp(buf, str, token_len)) {
continue;
}
 
-   if ( !cmdline_isendoftoken(*(buf+token_len)) ) {
+   if (!cmdline_isendoftoken(*(buf+token_len))) {
continue;
}
 
break;
-   } while ( (str = get_next_token(str)) != NULL );
+   } while ( (str = get_next_token(str)) != NULL);
 
if (!str)
return -1;
@@ -108,7 +108,7 @@ cmdline_parse_string(cmdline_parse_token_hdr_t *tk, const 
char *buf, void *res,
/* unspecified string (unknown single token) */
else {
token_len = 0;
-   while(!cmdline_isendoftoken(buf[token_len]) &&
+   while (!cmdline_isendoftoken(buf[token_len]) &&
  token_len < (STR_TOKEN_SIZE-1))
token_len++;
 
@@ -149,7 +149,7 @@ int 
cmdline_complete_get_nb_string(cmdline_parse_token_hdr_t *tk)
return 0;
 
str = sd->str;
-   while( (str = get_next_token(str)) != NULL ) {
+   while ((str = get_next_token(str)) != NULL) {
ret++;
}
return ret;
diff --git a/lib/cmdline/cmdline_rdline.c b/lib/cmdline/cmdline_rdline.c
index 5cf723a0126a..28fc54cdfebf 100644
--- a/lib/cmdline/cmdline_rdline.c
+++ b/lib/cmdline/cmdline_rdline.c
@@ -301,7 +301,7 @@ rdline_char_in(struct rdline *rdl, char c)
/* delete 1 char from the left */
case CMDLINE_KEY_BKSPACE:
case CMDLINE_KEY_BKSPACE2:
-   if(!cirbuf_del_tail_safe(&rdl->left)) {
+   if (!cirbuf_del_tail_safe(&rdl->left)) {
rdline_puts(rdl, vt100_bs);
display_right_buffer(rdl, 1);
}
@@ -354,7 +354,7 @@ rdline_char_in(struct rdline *rdl, char c)
/* paste contents of kill buffer to the left side of caret */
case CMDLINE_KEY_CTRL_Y:
i=0;
-   while(CIRBUF_GET_LEN(&rdl->right) + 
CIRBUF_GET_LEN(&rdl->left) <
+   while (CIRBUF_GET_LEN(&rdl->right) + 
CIRBUF_GET_LEN(&rdl->left) <
  RDLINE_BUF_SIZE &&
  i < rdl->kill_size) {
cirbuf_add_tail(&rdl->left, rdl->kill_buf[i]);
@@ -404,7 +404,7 @@ rdline_char_in(struct rdline *rdl, char c)
/* add chars */
if (ret == RDLINE_RES_COMPLETE) {
i=0;
-   while(CIRBUF_GET_LEN(&rdl->right) + 
CIRBUF_GET_LEN(&rdl->left) <
+   while (CIRBUF_GET_LEN(&rdl->right) + 
CIRBUF_GET_LEN(&rdl->left) <
  RDLINE_BUF_SIZE &&
  i < tmp_size) {
cirbuf_add_tail(&rdl->left, 
tmp_buf[i]);
-- 
2.35.1

[PATCH 04/14] ip_frag: fix whitespace

[Stephen Hemminger]

Add space after keywords.

Signed-off-by: Stephen Hemminger 
---
 lib/ip_frag/rte_ipv4_reassembly.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ip_frag/rte_ipv4_reassembly.c 
b/lib/ip_frag/rte_ipv4_reassembly.c
index 4a89a5f5365a..88ee0aa63f62 100644
--- a/lib/ip_frag/rte_ipv4_reassembly.c
+++ b/lib/ip_frag/rte_ipv4_reassembly.c
@@ -34,7 +34,7 @@ ipv4_frag_reassemble(struct ip_frag_pkt *fp)
for (i = n; i != IP_FIRST_FRAG_IDX && ofs != first_len; i--) {
 
/* previous fragment found. */
-   if(fp->frags[i].ofs + fp->frags[i].len == ofs) {
+   if (fp->frags[i].ofs + fp->frags[i].len == ofs) {
 
RTE_ASSERT(curr_idx != i);
 
-- 
2.35.1

[PATCH 03/14] timer: fix whitespace

[Stephen Hemminger]

Add space after keywords.

Signed-off-by: Stephen Hemminger 
---
 lib/timer/rte_timer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/timer/rte_timer.c b/lib/timer/rte_timer.c
index 85d67573eb9e..9eb73f1fed13 100644
--- a/lib/timer/rte_timer.c
+++ b/lib/timer/rte_timer.c
@@ -350,7 +350,7 @@ timer_get_prev_entries(uint64_t time_val, unsigned 
tim_lcore,
 {
unsigned lvl = priv_timer[tim_lcore].curr_skiplist_depth;
prev[lvl] = &priv_timer[tim_lcore].pending_head;
-   while(lvl != 0) {
+   while (lvl != 0) {
lvl--;
prev[lvl] = prev[lvl+1];
while (prev[lvl]->sl_next[lvl] &&
-- 
2.35.1

[PATCH 05/14] testpmd: fix whitspace

[Stephen Hemminger]

Add space after keywords.

Signed-off-by: Stephen Hemminger 
---
 app/test-pmd/cmdline.c| 30 +++---
 app/test-pmd/parameters.c |  8 
 app/test-pmd/testpmd.c|  2 +-
 3 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 8dc60e938830..ff1fd1b470af 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -2730,11 +2730,11 @@ parse_reta_config(const char *str,
 
while ((p = strchr(p0,'(')) != NULL) {
++p;
-   if((p0 = strchr(p,')')) == NULL)
+   if ((p0 = strchr(p,')')) == NULL)
return -1;
 
size = p0 - p;
-   if(size >= sizeof(s))
+   if (size >= sizeof(s))
return -1;
 
snprintf(s, sizeof(s), "%.*s", size, p);
@@ -3242,15 +3242,15 @@ cmd_config_thresh_parsed(void *parsed_result,
 
if (!strcmp(res->name, "txpt"))
tx_pthresh = res->value;
-   else if(!strcmp(res->name, "txht"))
+   else if (!strcmp(res->name, "txht"))
tx_hthresh = res->value;
-   else if(!strcmp(res->name, "txwt"))
+   else if (!strcmp(res->name, "txwt"))
tx_wthresh = res->value;
-   else if(!strcmp(res->name, "rxpt"))
+   else if (!strcmp(res->name, "rxpt"))
rx_pthresh = res->value;
-   else if(!strcmp(res->name, "rxht"))
+   else if (!strcmp(res->name, "rxht"))
rx_hthresh = res->value;
-   else if(!strcmp(res->name, "rxwt"))
+   else if (!strcmp(res->name, "rxwt"))
rx_wthresh = res->value;
else {
fprintf(stderr, "Unknown parameter\n");
@@ -4088,8 +4088,8 @@ cmd_vlan_offload_parsed(void *parsed_result,
len = strnlen(str, STR_TOKEN_SIZE);
i = 0;
/* Get port_id first */
-   while(i < len){
-   if(str[i] == ',')
+   while (i < len){
+   if (str[i] == ',')
break;
 
i++;
@@ -4097,7 +4097,7 @@ cmd_vlan_offload_parsed(void *parsed_result,
str[i]='\0';
tmp = strtoul(str, NULL, 0);
/* If port_id greater that what portid_t can represent, return */
-   if(tmp >= RTE_MAX_ETHPORTS)
+   if (tmp >= RTE_MAX_ETHPORTS)
return;
port_id = (portid_t)tmp;
 
@@ -4108,17 +4108,17 @@ cmd_vlan_offload_parsed(void *parsed_result,
 
if (!strcmp(res->what, "strip"))
rx_vlan_strip_set(port_id,  on);
-   else if(!strcmp(res->what, "stripq")){
+   else if (!strcmp(res->what, "stripq")){
uint16_t queue_id = 0;
 
/* No queue_id, return */
-   if(i + 1 >= len) {
+   if (i + 1 >= len) {
fprintf(stderr, "must specify (port,queue_id)\n");
return;
}
tmp = strtoul(str + i + 1, NULL, 0);
/* If queue_id greater that what 16-bits can represent, return 
*/
-   if(tmp > 0x)
+   if (tmp > 0x)
return;
 
queue_id = (uint16_t)tmp;
@@ -7207,7 +7207,7 @@ static void cmd_mac_addr_parsed(void *parsed_result,
ret = rte_eth_dev_mac_addr_remove(res->port_num, &res->address);
 
/* check the return value and print it if is < 0 */
-   if(ret < 0)
+   if (ret < 0)
fprintf(stderr, "mac_addr_cmd error: (%s)\n", strerror(-ret));
 
 }
@@ -7780,7 +7780,7 @@ static void cmd_vf_mac_addr_parsed(void *parsed_result,
res->vf_num);
 #endif
 
-   if(ret < 0)
+   if (ret < 0)
fprintf(stderr, "vf_mac_addr_cmd error: (%s)\n", 
strerror(-ret));
 
 }
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index aed4cdcb8485..b7372e025ffd 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -306,11 +306,11 @@ parse_portnuma_config(const char *q_arg)
/* reset from value set at definition */
while ((p = strchr(p0,'(')) != NULL) {
++p;
-   if((p0 = strchr(p,')')) == NULL)
+   if ((p0 = strchr(p,')')) == NULL)
return -1;
 
size = p0 - p;
-   if(size >= sizeof(s))
+   if (size >= sizeof(s))
return -1;
 
snprintf(s, sizeof(s), "%.*s", size, p);
@@ -366,11 +366,11 @@ parse_ringnuma_config(const char *q_arg)
/* reset from value set at definition */
while ((p = strchr(p0,'(')) != NULL) {
++p;
-   if((p0 = strchr(p,')')) == NULL)
+   if ((p0 = strchr(p,')')) == NULL)
return -1;
 
size = p0 - p;
-   if(size >= sizeof(s))
+   if (size >= sizeof(s))
return -

[PATCH 06/14] test: fix whitespace

[Stephen Hemminger]

Add space after keywords.

Signed-off-by: Stephen Hemminger 
---
 app/test/test_cmdline_cirbuf.c |  4 ++--
 app/test/test_debug.c  |  4 ++--
 app/test/test_hash.c   |  8 
 app/test/test_lpm6_perf.c  |  2 +-
 app/test/test_malloc.c | 14 +++---
 app/test/test_mbuf.c   | 12 ++--
 app/test/test_spinlock.c   |  2 +-
 7 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/app/test/test_cmdline_cirbuf.c b/app/test/test_cmdline_cirbuf.c
index 8ac326cb02e0..6f7aae6df59a 100644
--- a/app/test/test_cmdline_cirbuf.c
+++ b/app/test/test_cmdline_cirbuf.c
@@ -708,7 +708,7 @@ test_cirbuf_char_fill(void)
return -1;
}
/* delete buffer from tail */
-   for(i = 0; i < CMDLINE_TEST_BUFSIZE; i++)
+   for (i = 0; i < CMDLINE_TEST_BUFSIZE; i++)
cirbuf_del_tail_safe(&cb);
/* try to delete from an empty buffer */
if (cirbuf_del_tail_safe(&cb) >= 0) {
@@ -737,7 +737,7 @@ test_cirbuf_char_fill(void)
return -1;
}
/* delete buffer from head */
-   for(i = 0; i < CMDLINE_TEST_BUFSIZE; i++)
+   for (i = 0; i < CMDLINE_TEST_BUFSIZE; i++)
cirbuf_del_head_safe(&cb);
/* try to delete from an empty buffer */
if (cirbuf_del_head_safe(&cb) >= 0) {
diff --git a/app/test/test_debug.c b/app/test/test_debug.c
index 2704f5b92726..c66748019d9f 100644
--- a/app/test/test_debug.c
+++ b/app/test/test_debug.c
@@ -53,7 +53,7 @@ test_panic(void)
return -1;
}
wait(&status);
-   if(status == 0){
+   if (status == 0){
printf("Child process terminated normally!\n");
return -1;
} else
@@ -84,7 +84,7 @@ test_exit_val(int exit_val)
}
wait(&status);
printf("Child process status: %d\n", status);
-   if(!WIFEXITED(status) || WEXITSTATUS(status) != (uint8_t)exit_val){
+   if (!WIFEXITED(status) || WEXITSTATUS(status) != (uint8_t)exit_val){
printf("Child process terminated with incorrect status 
(expected = %d)!\n",
exit_val);
return -1;
diff --git a/app/test/test_hash.c b/app/test/test_hash.c
index 3e45afaa67fc..5e0618f0c340 100644
--- a/app/test/test_hash.c
+++ b/app/test/test_hash.c
@@ -72,7 +72,7 @@ static uint32_t hashtest_key_lens[] = {0, 2, 4, 5, 6, 7, 8, 
10, 11, 15, 16, 21,
rte_free(g_qsv);\
return -1;  \
}   \
-} while (0)
+} while(0)
 
 /*
  * 5-tuple key type.
@@ -724,12 +724,12 @@ static int test_five_keys(void)
}
 
/* Lookup */
-   for(i = 0; i < 5; i++)
+   for (i = 0; i < 5; i++)
key_array[i] = &keys[i];
 
ret = rte_hash_lookup_bulk(handle, &key_array[0], 5, (int32_t *)pos);
-   if(ret == 0)
-   for(i = 0; i < 5; i++) {
+   if (ret == 0)
+   for (i = 0; i < 5; i++) {
print_key_info("Lkp", key_array[i], pos[i]);
RETURN_IF_ERROR(pos[i] != expected_pos[i],
"failed to find key (pos[%u]=%d)", i, 
pos[i]);
diff --git a/app/test/test_lpm6_perf.c b/app/test/test_lpm6_perf.c
index aaf2773b6fac..2f9321345e4b 100644
--- a/app/test/test_lpm6_perf.c
+++ b/app/test/test_lpm6_perf.c
@@ -47,7 +47,7 @@ print_route_distribution(const struct rules_tbl_entry *table, 
uint32_t n)
printf("--- \n");
 
/* Count depths. */
-   for(i = 1; i <= 128; i++) {
+   for (i = 1; i <= 128; i++) {
unsigned depth_counter = 0;
double percent_hits;
 
diff --git a/app/test/test_malloc.c b/app/test/test_malloc.c
index de40e506113a..0bb18c8cf7aa 100644
--- a/app/test/test_malloc.c
+++ b/app/test/test_malloc.c
@@ -86,8 +86,8 @@ test_align_overlap_per_lcore(__rte_unused void *arg)
ret = -1;
break;
}
-   for(j = 0; j < 1000 ; j++) {
-   if( *(char *)p1 != 0) {
+   for (j = 0; j < 1000 ; j++) {
+   if ( *(char *)p1 != 0) {
printf("rte_zmalloc didn't zero the allocated 
memory\n");
ret = -1;
}
@@ -157,8 +157,8 @@ test_reordered_free_per_lcore(__rte_unused void *arg)
ret = -1;
break;
}
-   for(j = 0; j < 1000 ; j++) {
-   if( *(char *)p1 != 0) {
+   for (j = 0; j < 1000 ; j++) {
+   if ( *(char *)p1 != 0) {
printf("rte_zmalloc didn't zero the allocated 
memory\n");
ret = -1;
  

[PATCH 07/14] examples/qos_sched: fix whitespace

[Stephen Hemminger]

Add space after keywords.

Signed-off-by: Stephen Hemminger 
---
 examples/qos_sched/app_thread.c | 6 +++---
 examples/qos_sched/args.c   | 2 +-
 examples/qos_sched/init.c   | 2 +-
 examples/qos_sched/main.c   | 4 ++--
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/examples/qos_sched/app_thread.c b/examples/qos_sched/app_thread.c
index dbc878b55394..a49356fecf5f 100644
--- a/examples/qos_sched/app_thread.c
+++ b/examples/qos_sched/app_thread.c
@@ -79,7 +79,7 @@ app_rx_thread(struct thread_conf **confs)
if (likely(nb_rx != 0)) {
APP_STATS_ADD(conf->stat.nb_rx, nb_rx);
 
-   for(i = 0; i < nb_rx; i++) {
+   for (i = 0; i < nb_rx; i++) {
get_pkt_sched(rx_mbufs[i],
&subport, &pipe, 
&traffic_class, &queue, &color);
rte_sched_port_pkt_write(conf->sched_port,
@@ -91,7 +91,7 @@ app_rx_thread(struct thread_conf **confs)
 
if (unlikely(rte_ring_sp_enqueue_bulk(conf->rx_ring,
(void **)rx_mbufs, nb_rx, NULL) == 0)) {
-   for(i = 0; i < nb_rx; i++) {
+   for (i = 0; i < nb_rx; i++) {
rte_pktmbuf_free(rx_mbufs[i]);
 
APP_STATS_ADD(conf->stat.nb_drop, 1);
@@ -137,7 +137,7 @@ app_send_packets(struct thread_conf *qconf, struct rte_mbuf 
**mbufs, uint32_t nb
uint32_t i, len;
 
len = qconf->n_mbufs;
-   for(i = 0; i < nb_pkt; i++) {
+   for (i = 0; i < nb_pkt; i++) {
qconf->m_table[len] = mbufs[i];
len++;
/* enough pkts to be sent */
diff --git a/examples/qos_sched/args.c b/examples/qos_sched/args.c
index b2959499ae9a..b752eb221f7c 100644
--- a/examples/qos_sched/args.c
+++ b/examples/qos_sched/args.c
@@ -451,7 +451,7 @@ app_parse_args(int argc, char **argv)
/* sanity check for cores assignment */
nb_lcores = app_cpu_core_count();
 
-   for(i = 0; i < nb_pfc; i++) {
+   for (i = 0; i < nb_pfc; i++) {
if (qos_conf[i].rx_core >= nb_lcores) {
RTE_LOG(ERR, APP, "pfc %u: invalid RX lcore index 
%u\n", i + 1,
qos_conf[i].rx_core);
diff --git a/examples/qos_sched/init.c b/examples/qos_sched/init.c
index 0709aec10c31..41b46b296298 100644
--- a/examples/qos_sched/init.c
+++ b/examples/qos_sched/init.c
@@ -323,7 +323,7 @@ int app_init(void)
rte_exit(EXIT_FAILURE, "Invalid configuration profile\n");
 
/* Initialize each active flow */
-   for(i = 0; i < nb_pfc; i++) {
+   for (i = 0; i < nb_pfc; i++) {
uint32_t socket = rte_lcore_to_socket_id(qos_conf[i].rx_core);
struct rte_ring *ring;
 
diff --git a/examples/qos_sched/main.c b/examples/qos_sched/main.c
index dc6a17a6464a..7b757df0eb02 100644
--- a/examples/qos_sched/main.c
+++ b/examples/qos_sched/main.c
@@ -152,7 +152,7 @@ app_stat(void)
static struct rte_eth_stats tx_stats[MAX_DATA_STREAMS];
 
/* print statistics */
-   for(i = 0; i < nb_pfc; i++) {
+   for (i = 0; i < nb_pfc; i++) {
struct flow_conf *flow = &qos_conf[i];
 
rte_eth_stats_get(flow->rx_port, &stats);
@@ -212,7 +212,7 @@ main(int argc, char **argv)
}
else {
/* print statistics every second */
-   while(1) {
+   while (1) {
sleep(1);
app_stat();
}
-- 
2.35.1

[PATCH 08/14] examples/vhost: fix whitespace

[Stephen Hemminger]

Add space after keywords

Signed-off-by: Stephen Hemminger 
---
 examples/vhost/main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index 42e53a0f9a4f..cabc8682346c 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -1491,7 +1491,7 @@ switch_worker(void *arg __rte_unused)
}
}
 
-   while(1) {
+   while (1) {
drain_mbuf_table(tx_q);
drain_vhost_table();
/*
@@ -1583,7 +1583,7 @@ destroy_device(int vid)
return;
/*set the remove flag. */
vdev->remove = 1;
-   while(vdev->ready != DEVICE_SAFE_REMOVE) {
+   while (vdev->ready != DEVICE_SAFE_REMOVE) {
rte_pause();
}
 
@@ -1816,7 +1816,7 @@ print_stats(__rte_unused void *arg)
const char clr[] = { 27, '[', '2', 'J', '\0' };
const char top_left[] = { 27, '[', '1', ';', '1', 'H','\0' };
 
-   while(1) {
+   while (1) {
sleep(enable_stats);
 
/* Clear screen and move to top left */
-- 
2.35.1

[PATCH 09/14] l3fwd: fix whitespace

[Stephen Hemminger]

Add space after keywoords

Signed-off-by: Stephen Hemminger 
---
 examples/l3fwd-power/main.c | 6 +++---
 examples/l3fwd/main.c   | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index fd3ade330f82..b2270e1056c8 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -1787,11 +1787,11 @@ parse_config(const char *q_arg)
 
while ((p = strchr(p0,'(')) != NULL) {
++p;
-   if((p0 = strchr(p,')')) == NULL)
+   if ((p0 = strchr(p,')')) == NULL)
return -1;
 
size = p0 - p;
-   if(size >= sizeof(s))
+   if (size >= sizeof(s))
return -1;
 
snprintf(s, sizeof(s), "%.*s", size, p);
@@ -2946,7 +2946,7 @@ main(int argc, char **argv)
fflush(stdout);
 
/* init RX queues */
-   for(queue = 0; queue < qconf->n_rx_queue; ++queue) {
+   for (queue = 0; queue < qconf->n_rx_queue; ++queue) {
struct rte_eth_rxconf rxq_conf;
 
portid = qconf->rx_queue_list[queue].port_id;
diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
index 5198ff30dd00..2e1c4939f992 100644
--- a/examples/l3fwd/main.c
+++ b/examples/l3fwd/main.c
@@ -516,11 +516,11 @@ parse_config(const char *q_arg)
 
while ((p = strchr(p0,'(')) != NULL) {
++p;
-   if((p0 = strchr(p,')')) == NULL)
+   if ((p0 = strchr(p,')')) == NULL)
return -1;
 
size = p0 - p;
-   if(size >= sizeof(s))
+   if (size >= sizeof(s))
return -1;
 
snprintf(s, sizeof(s), "%.*s", size, p);
@@ -1366,7 +1366,7 @@ l3fwd_poll_resource_setup(void)
printf("\nInitializing rx queues on lcore %u ... ", lcore_id );
fflush(stdout);
/* init RX queues */
-   for(queue = 0; queue < qconf->n_rx_queue; ++queue) {
+   for (queue = 0; queue < qconf->n_rx_queue; ++queue) {
struct rte_eth_rxconf rxq_conf;
 
portid = qconf->rx_queue_list[queue].port_id;
-- 
2.35.1

[PATCH 10/14] examples: fix whitespace

[Stephen Hemminger]

Add space after keywords.

Signed-off-by: Stephen Hemminger 
---
 examples/cmdline/parse_obj_list.c  | 2 +-
 examples/ip_reassembly/main.c  | 2 +-
 examples/multi_process/symmetric_mp/main.c | 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/examples/cmdline/parse_obj_list.c 
b/examples/cmdline/parse_obj_list.c
index 959bcd14527e..759072d39620 100644
--- a/examples/cmdline/parse_obj_list.c
+++ b/examples/cmdline/parse_obj_list.c
@@ -42,7 +42,7 @@ parse_obj_list(cmdline_parse_token_hdr_t *tk, const char 
*buf, void *res,
if (res && ressize < sizeof(struct object *))
return -1;
 
-   while(!cmdline_isendoftoken(buf[token_len]))
+   while (!cmdline_isendoftoken(buf[token_len]))
token_len++;
 
SLIST_FOREACH(o, tkd->list, next) {
diff --git a/examples/ip_reassembly/main.c b/examples/ip_reassembly/main.c
index bd0b1d31decf..7e84b4944759 100644
--- a/examples/ip_reassembly/main.c
+++ b/examples/ip_reassembly/main.c
@@ -300,7 +300,7 @@ send_single_packet(struct rte_mbuf *m, uint16_t port)
 
TX_LCORE_STAT_UPDATE(&qconf->tx_stat, queue, 1);
txmb->m_table[txmb->head] = m;
-   if(++txmb->head == len)
+   if (++txmb->head == len)
txmb->head = 0;
 
return 0;
diff --git a/examples/multi_process/symmetric_mp/main.c 
b/examples/multi_process/symmetric_mp/main.c
index 1ff85875dfdf..9c5348bd36ed 100644
--- a/examples/multi_process/symmetric_mp/main.c
+++ b/examples/multi_process/symmetric_mp/main.c
@@ -156,7 +156,7 @@ smp_parse_args(int argc, char **argv)
 
/* get the port numbers from the port mask */
RTE_ETH_FOREACH_DEV(i)
-   if(port_mask & (1 << i))
+   if (port_mask & (1 << i))
ports[num_ports++] = (uint8_t)i;
 
ret = optind-1;
@@ -470,8 +470,8 @@ main(int argc, char **argv)
/* Primary instance initialized. 8< */
if (num_ports & 1)
rte_exit(EXIT_FAILURE, "Application must use an even number of 
ports\n");
-   for(i = 0; i < num_ports; i++){
-   if(proc_type == RTE_PROC_PRIMARY)
+   for (i = 0; i < num_ports; i++){
+   if (proc_type == RTE_PROC_PRIMARY)
if (smp_port_init(ports[i], mp, (uint16_t)num_procs) < 
0)
rte_exit(EXIT_FAILURE, "Error initialising 
ports\n");
}
-- 
2.35.1

[PATCH 11/14] bus/pci: fix whitespace

[Stephen Hemminger]

Add space after keywords.

Signed-off-by: Stephen Hemminger 
---
 drivers/bus/pci/bsd/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bus/pci/bsd/pci.c b/drivers/bus/pci/bsd/pci.c
index 7459d15c7e31..056e2eda2d5f 100644
--- a/drivers/bus/pci/bsd/pci.c
+++ b/drivers/bus/pci/bsd/pci.c
@@ -368,7 +368,7 @@ rte_pci_scan(void)
}
 
dev_count += conf_io.num_matches;
-   } while(conf_io.status == PCI_GETCONF_MORE_DEVS);
+   } while (conf_io.status == PCI_GETCONF_MORE_DEVS);
 
close(fd);
 
-- 
2.35.1

[PATCH 12/14] net/e1000: fix whitespace

[Stephen Hemminger]

Add space after keywords.

Signed-off-by: Stephen Hemminger 
---
 drivers/net/e1000/em_ethdev.c  | 4 ++--
 drivers/net/e1000/igb_ethdev.c | 8 
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index 8ee9be12ad19..146e74cfc76e 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -872,8 +872,8 @@ eth_em_stats_get(struct rte_eth_dev *dev, struct 
rte_eth_stats *rte_stats)
E1000_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
int pause_frames;
 
-   if(hw->phy.media_type == e1000_media_type_copper ||
-   (E1000_READ_REG(hw, E1000_STATUS) & E1000_STATUS_LU)) {
+   if (hw->phy.media_type == e1000_media_type_copper ||
+   (E1000_READ_REG(hw, E1000_STATUS) & E1000_STATUS_LU)) {
stats->symerrs += E1000_READ_REG(hw,E1000_SYMERRS);
stats->sec += E1000_READ_REG(hw, E1000_SEC);
}
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 8858f975f8cc..5c35b8349063 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -1683,7 +1683,7 @@ igb_read_stats_registers(struct e1000_hw *hw, struct 
e1000_hw_stats *stats)
uint64_t old_rpthc = stats->rpthc;
uint64_t old_hgptc = stats->hgptc;
 
-   if(hw->phy.media_type == e1000_media_type_copper ||
+   if (hw->phy.media_type == e1000_media_type_copper ||
(E1000_READ_REG(hw, E1000_STATUS) & E1000_STATUS_LU)) {
stats->symerrs +=
E1000_READ_REG(hw,E1000_SYMERRS);
@@ -3500,10 +3500,10 @@ static void igbvf_set_vfta_all(struct rte_eth_dev *dev, 
bool on)
 
for (i = 0; i < IGB_VFTA_SIZE; i++){
vfta = shadow_vfta->vfta[i];
-   if(vfta){
+   if (vfta){
mask = 1;
for (j = 0; j < 32; j++){
-   if(vfta & mask)
+   if (vfta & mask)
igbvf_set_vfta(hw,
(uint16_t)((i<<5)+j), on);
mask<<=1;
@@ -3528,7 +3528,7 @@ igbvf_vlan_filter_set(struct rte_eth_dev *dev, uint16_t 
vlan_id, int on)
 
/*vind is not used in VF driver, set to 0, check ixgbe_set_vfta_vf*/
ret = igbvf_set_vfta(hw, vlan_id, !!on);
-   if(ret){
+   if (ret){
PMD_INIT_LOG(ERR, "Unable to set VF vlan");
return ret;
}
-- 
2.35.1

[PATCH 13/14] net/i40e: fix whitespace

[Stephen Hemminger]

Add space after keywords.

Signed-off-by: Stephen Hemminger 
---
 drivers/net/i40e/i40e_pf.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/i40e/i40e_pf.c b/drivers/net/i40e/i40e_pf.c
index 15d9ff868f3a..7050e0057d8e 100644
--- a/drivers/net/i40e/i40e_pf.c
+++ b/drivers/net/i40e/i40e_pf.c
@@ -956,7 +956,7 @@ i40e_pf_host_process_cmd_add_vlan(struct i40e_pf_vf *vf,
 
for (i = 0; i < vlan_filter_list->num_elements; i++) {
ret = i40e_vsi_add_vlan(vf->vsi, vid[i]);
-   if(ret != I40E_SUCCESS)
+   if (ret != I40E_SUCCESS)
goto send_msg;
}
 
@@ -996,7 +996,7 @@ i40e_pf_host_process_cmd_del_vlan(struct i40e_pf_vf *vf,
vid = vlan_filter_list->vlan_id;
for (i = 0; i < vlan_filter_list->num_elements; i++) {
ret = i40e_vsi_delete_vlan(vf->vsi, vid[i]);
-   if(ret != I40E_SUCCESS)
+   if (ret != I40E_SUCCESS)
goto send_msg;
}
 
@@ -1577,12 +1577,12 @@ i40e_pf_host_init(struct rte_eth_dev *dev)
 * return if SRIOV not enabled, VF number not configured or
 * no queue assigned.
 */
-   if(!hw->func_caps.sr_iov_1_1 || pf->vf_num == 0 || pf->vf_nb_qps == 0)
+   if (!hw->func_caps.sr_iov_1_1 || pf->vf_num == 0 || pf->vf_nb_qps == 0)
return I40E_SUCCESS;
 
/* Allocate memory to store VF structure */
pf->vfs = rte_zmalloc("i40e_pf_vf",sizeof(*pf->vfs) * pf->vf_num, 0);
-   if(pf->vfs == NULL)
+   if (pf->vfs == NULL)
return -ENOMEM;
 
/* Disable irq0 for VFR event */
-- 
2.35.1

[PATCH 14/14] net/bnx2x: fix whitespace

[Stephen Hemminger]

Add space after keywords.

Signed-off-by: Stephen Hemminger 
---
 drivers/net/bnx2x/bnx2x.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/bnx2x/bnx2x.h b/drivers/net/bnx2x/bnx2x.h
index 89414ac88a93..9cb927e25449 100644
--- a/drivers/net/bnx2x/bnx2x.h
+++ b/drivers/net/bnx2x/bnx2x.h
@@ -96,7 +96,7 @@ int bnx2x_ilog2(int x)
int log = 0;
x >>= 1;
 
-   while(x) {
+   while (x) {
log++;
x >>= 1;
}
-- 
2.35.1

RE: [PATCH 07/14] examples/qos_sched: fix whitespace

[Dumitrescu, Cristian]

> -Original Message-
> From: Stephen Hemminger 
> Sent: Wednesday, November 9, 2022 7:15 PM
> To: dev@dpdk.org
> Cc: Stephen Hemminger ; Dumitrescu,
> Cristian 
> Subject: [PATCH 07/14] examples/qos_sched: fix whitespace
> 
> Add space after keywords.
> 
> Signed-off-by: Stephen Hemminger 
> ---
>  examples/qos_sched/app_thread.c | 6 +++---
>  examples/qos_sched/args.c   | 2 +-
>  examples/qos_sched/init.c   | 2 +-
>  examples/qos_sched/main.c   | 4 ++--
>  4 files changed, 7 insertions(+), 7 deletions(-)
> 

Acked-by: Cristian Dumitrescu 

[PATCH v2 01/14] eal: fix whitespace

[Stephen Hemminger]

Add space after keywords.

Signed-off-by: Stephen Hemminger 
---
 lib/eal/linux/eal_hugepage_info.c | 6 +++---
 lib/eal/linux/eal_interrupts.c| 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/eal/linux/eal_hugepage_info.c 
b/lib/eal/linux/eal_hugepage_info.c
index a1b6cb31ff1a..b6d751a2ee97 100644
--- a/lib/eal/linux/eal_hugepage_info.c
+++ b/lib/eal/linux/eal_hugepage_info.c
@@ -180,8 +180,8 @@ get_default_hp_size(void)
FILE *fd = fopen(proc_meminfo, "r");
if (fd == NULL)
rte_panic("Cannot open %s\n", proc_meminfo);
-   while(fgets(buffer, sizeof(buffer), fd)){
-   if (strncmp(buffer, str_hugepagesz, hugepagesz_len) == 0){
+   while (fgets(buffer, sizeof(buffer), fd)) {
+   if (strncmp(buffer, str_hugepagesz, hugepagesz_len) == 0) {
size = rte_str_to_size(&buffer[hugepagesz_len]);
break;
}
@@ -231,7 +231,7 @@ get_hugepage_dir(uint64_t hugepage_sz, char *hugedir, int 
len)
if (default_size == 0)
default_size = get_default_hp_size();
 
-   while (fgets(buf, sizeof(buf), fd)){
+   while (fgets(buf, sizeof(buf), fd)) {
const char *pagesz_str;
 
if (rte_strsplit(buf, sizeof(buf), splitstr, _FIELDNAME_MAX,
diff --git a/lib/eal/linux/eal_interrupts.c b/lib/eal/linux/eal_interrupts.c
index d52ec8eb4cd7..3be1b090be9e 100644
--- a/lib/eal/linux/eal_interrupts.c
+++ b/lib/eal/linux/eal_interrupts.c
@@ -1073,7 +1073,7 @@ eal_intr_handle_interrupts(int pfd, unsigned totalfds)
struct epoll_event events[totalfds];
int nfds = 0;
 
-   for(;;) {
+   for (;;) {
nfds = epoll_wait(pfd, events, totalfds,
EAL_INTR_EPOLL_WAIT_FOREVER);
/* epoll_wait fail */
-- 
2.35.1

[PATCH v2 00/14] whitespace after keywords

[Stephen Hemminger]

The DPDK standard now enforced through checkpatch is to use
a space after keywords. Much of the older code that was grandfathered
in doesn't follow that, so fix it.

No urgency to this patchset, and it might just be viewed as churn.
But I prefer to read code that looks all the same.

“A foolish consistency is the hobgoblin of little minds, adored by
little statesmen and philosophers and divines. With consistency a
great soul has simply nothing to do. He may as well concern himself
with his shadow on the wall."  -- Ralph Waldo Emerson

v2 - fix related complaints from checkpatch
 if you touch a line, then it wants to make all the rules apply.

Stephen Hemminger (14):
  eal: fix whitespace
  cmdline: fix whitespace
  timer: fix whitespace
  ip_frag: fix whitespace
  testpmd: fix whitespace
  test: fix whitespace
  examples/qos_sched: fix whitespace
  examples/vhost: fix whitespace
  l3fwd: fix whitespace
  examples: fix whitespace
  bus/pci: fix whitespace
  net/e1000: fix whitespace
  net/i40e: fix whitespace
  net/bnx2x: fix whitespace

 app/test-pmd/cmdline.c | 31 +++---
 app/test-pmd/parameters.c  | 10 ---
 app/test-pmd/testpmd.c |  2 +-
 app/test/test_cmdline_cirbuf.c |  4 +--
 app/test/test_debug.c  |  4 +--
 app/test/test_hash.c   |  6 ++---
 app/test/test_lpm6_perf.c  |  2 +-
 app/test/test_malloc.c | 14 +-
 app/test/test_mbuf.c   | 14 +-
 app/test/test_spinlock.c   |  2 +-
 drivers/bus/pci/bsd/pci.c  |  2 +-
 drivers/net/bnx2x/bnx2x.h  |  2 +-
 drivers/net/e1000/em_ethdev.c  |  4 +--
 drivers/net/e1000/igb_ethdev.c |  8 +++---
 drivers/net/i40e/i40e_pf.c |  8 +++---
 examples/cmdline/parse_obj_list.c  |  2 +-
 examples/ip_reassembly/main.c  |  2 +-
 examples/l3fwd-power/main.c|  7 ++---
 examples/l3fwd/main.c  |  7 ++---
 examples/multi_process/symmetric_mp/main.c |  6 ++---
 examples/qos_sched/app_thread.c|  6 ++---
 examples/qos_sched/args.c  |  2 +-
 examples/qos_sched/init.c  |  2 +-
 examples/qos_sched/main.c  |  4 +--
 examples/vhost/main.c  |  7 +++--
 lib/cmdline/cmdline_parse_string.c | 16 +--
 lib/cmdline/cmdline_rdline.c   | 12 -
 lib/eal/linux/eal_hugepage_info.c  |  6 ++---
 lib/eal/linux/eal_interrupts.c |  2 +-
 lib/ip_frag/rte_ipv4_reassembly.c  |  2 +-
 lib/timer/rte_timer.c  |  2 +-
 31 files changed, 99 insertions(+), 99 deletions(-)

-- 
2.35.1

[PATCH v2 02/14] cmdline: fix whitespace

[Stephen Hemminger]

Add space after keywords.
Remove unnecessary spaces in if expressions.

Signed-off-by: Stephen Hemminger 
---
 lib/cmdline/cmdline_parse_string.c | 16 +++-
 lib/cmdline/cmdline_rdline.c   | 12 ++--
 2 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/lib/cmdline/cmdline_parse_string.c 
b/lib/cmdline/cmdline_parse_string.c
index d756638905a2..ab1ef4b22e40 100644
--- a/lib/cmdline/cmdline_parse_string.c
+++ b/lib/cmdline/cmdline_parse_string.c
@@ -73,20 +73,18 @@ cmdline_parse_string(cmdline_parse_token_hdr_t *tk, const 
char *buf, void *res,
token_len = get_token_len(str);
 
/* if token is too big... */
-   if (token_len >= STR_TOKEN_SIZE - 1) {
+   if (token_len >= STR_TOKEN_SIZE - 1)
continue;
}
 
-   if ( strncmp(buf, str, token_len) ) {
+   if (strncmp(buf, str, token_len))
continue;
-   }
 
-   if ( !cmdline_isendoftoken(*(buf+token_len)) ) {
+   if (!cmdline_isendoftoken(*(buf+token_len)))
continue;
-   }
 
break;
-   } while ( (str = get_next_token(str)) != NULL );
+   } while ((str = get_next_token(str)) != NULL);
 
if (!str)
return -1;
@@ -108,7 +106,7 @@ cmdline_parse_string(cmdline_parse_token_hdr_t *tk, const 
char *buf, void *res,
/* unspecified string (unknown single token) */
else {
token_len = 0;
-   while(!cmdline_isendoftoken(buf[token_len]) &&
+   while (!cmdline_isendoftoken(buf[token_len]) &&
  token_len < (STR_TOKEN_SIZE-1))
token_len++;
 
@@ -149,9 +147,9 @@ int 
cmdline_complete_get_nb_string(cmdline_parse_token_hdr_t *tk)
return 0;
 
str = sd->str;
-   while( (str = get_next_token(str)) != NULL ) {
+   while ((str = get_next_token(str)) != NULL)
ret++;
-   }
+
return ret;
 }
 
diff --git a/lib/cmdline/cmdline_rdline.c b/lib/cmdline/cmdline_rdline.c
index 5cf723a0126a..b5927879d48f 100644
--- a/lib/cmdline/cmdline_rdline.c
+++ b/lib/cmdline/cmdline_rdline.c
@@ -301,7 +301,7 @@ rdline_char_in(struct rdline *rdl, char c)
/* delete 1 char from the left */
case CMDLINE_KEY_BKSPACE:
case CMDLINE_KEY_BKSPACE2:
-   if(!cirbuf_del_tail_safe(&rdl->left)) {
+   if (!cirbuf_del_tail_safe(&rdl->left)) {
rdline_puts(rdl, vt100_bs);
display_right_buffer(rdl, 1);
}
@@ -354,7 +354,7 @@ rdline_char_in(struct rdline *rdl, char c)
/* paste contents of kill buffer to the left side of caret */
case CMDLINE_KEY_CTRL_Y:
i=0;
-   while(CIRBUF_GET_LEN(&rdl->right) + 
CIRBUF_GET_LEN(&rdl->left) <
+   while (CIRBUF_GET_LEN(&rdl->right) + 
CIRBUF_GET_LEN(&rdl->left) <
  RDLINE_BUF_SIZE &&
  i < rdl->kill_size) {
cirbuf_add_tail(&rdl->left, rdl->kill_buf[i]);
@@ -403,10 +403,10 @@ rdline_char_in(struct rdline *rdl, char c)
tmp_size = strnlen(tmp_buf, sizeof(tmp_buf));
/* add chars */
if (ret == RDLINE_RES_COMPLETE) {
-   i=0;
-   while(CIRBUF_GET_LEN(&rdl->right) + 
CIRBUF_GET_LEN(&rdl->left) <
- RDLINE_BUF_SIZE &&
- i < tmp_size) {
+   i = 0;
+   while (CIRBUF_GET_LEN(&rdl->right) +
+  CIRBUF_GET_LEN(&rdl->left) <
+  RDLINE_BUF_SIZE && i < tmp_size) 
{
cirbuf_add_tail(&rdl->left, 
tmp_buf[i]);
rdl->write_char(rdl, 
tmp_buf[i]);
i++;
-- 
2.35.1

[PATCH v2 03/14] timer: fix whitespace

[Stephen Hemminger]

Add space after keywords.

Signed-off-by: Stephen Hemminger 
---
 lib/timer/rte_timer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/timer/rte_timer.c b/lib/timer/rte_timer.c
index 85d67573eb9e..9eb73f1fed13 100644
--- a/lib/timer/rte_timer.c
+++ b/lib/timer/rte_timer.c
@@ -350,7 +350,7 @@ timer_get_prev_entries(uint64_t time_val, unsigned 
tim_lcore,
 {
unsigned lvl = priv_timer[tim_lcore].curr_skiplist_depth;
prev[lvl] = &priv_timer[tim_lcore].pending_head;
-   while(lvl != 0) {
+   while (lvl != 0) {
lvl--;
prev[lvl] = prev[lvl+1];
while (prev[lvl]->sl_next[lvl] &&
-- 
2.35.1

[PATCH v2 04/14] ip_frag: fix whitespace

[Stephen Hemminger]

Add space after keywords.

Signed-off-by: Stephen Hemminger 
---
 lib/ip_frag/rte_ipv4_reassembly.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ip_frag/rte_ipv4_reassembly.c 
b/lib/ip_frag/rte_ipv4_reassembly.c
index 4a89a5f5365a..88ee0aa63f62 100644
--- a/lib/ip_frag/rte_ipv4_reassembly.c
+++ b/lib/ip_frag/rte_ipv4_reassembly.c
@@ -34,7 +34,7 @@ ipv4_frag_reassemble(struct ip_frag_pkt *fp)
for (i = n; i != IP_FIRST_FRAG_IDX && ofs != first_len; i--) {
 
/* previous fragment found. */
-   if(fp->frags[i].ofs + fp->frags[i].len == ofs) {
+   if (fp->frags[i].ofs + fp->frags[i].len == ofs) {
 
RTE_ASSERT(curr_idx != i);
 
-- 
2.35.1

[PATCH v2 05/14] testpmd: fix whitespace

[Stephen Hemminger]

Add space after keywords.

Signed-off-by: Stephen Hemminger 
---
 app/test-pmd/cmdline.c| 31 ---
 app/test-pmd/parameters.c | 10 ++
 app/test-pmd/testpmd.c|  2 +-
 3 files changed, 23 insertions(+), 20 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 8dc60e938830..7721006cc310 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -2730,11 +2730,12 @@ parse_reta_config(const char *str,
 
while ((p = strchr(p0,'(')) != NULL) {
++p;
-   if((p0 = strchr(p,')')) == NULL)
+   p0 = strchr(p, ')');
+   if (p0 == NULL)
return -1;
 
size = p0 - p;
-   if(size >= sizeof(s))
+   if (size >= sizeof(s))
return -1;
 
snprintf(s, sizeof(s), "%.*s", size, p);
@@ -3242,15 +3243,15 @@ cmd_config_thresh_parsed(void *parsed_result,
 
if (!strcmp(res->name, "txpt"))
tx_pthresh = res->value;
-   else if(!strcmp(res->name, "txht"))
+   else if (!strcmp(res->name, "txht"))
tx_hthresh = res->value;
-   else if(!strcmp(res->name, "txwt"))
+   else if (!strcmp(res->name, "txwt"))
tx_wthresh = res->value;
-   else if(!strcmp(res->name, "rxpt"))
+   else if (!strcmp(res->name, "rxpt"))
rx_pthresh = res->value;
-   else if(!strcmp(res->name, "rxht"))
+   else if (!strcmp(res->name, "rxht"))
rx_hthresh = res->value;
-   else if(!strcmp(res->name, "rxwt"))
+   else if (!strcmp(res->name, "rxwt"))
rx_wthresh = res->value;
else {
fprintf(stderr, "Unknown parameter\n");
@@ -4088,8 +4089,8 @@ cmd_vlan_offload_parsed(void *parsed_result,
len = strnlen(str, STR_TOKEN_SIZE);
i = 0;
/* Get port_id first */
-   while(i < len){
-   if(str[i] == ',')
+   while (i < len) {
+   if (str[i] == ',')
break;
 
i++;
@@ -4097,7 +4098,7 @@ cmd_vlan_offload_parsed(void *parsed_result,
str[i]='\0';
tmp = strtoul(str, NULL, 0);
/* If port_id greater that what portid_t can represent, return */
-   if(tmp >= RTE_MAX_ETHPORTS)
+   if (tmp >= RTE_MAX_ETHPORTS)
return;
port_id = (portid_t)tmp;
 
@@ -4108,17 +4109,17 @@ cmd_vlan_offload_parsed(void *parsed_result,
 
if (!strcmp(res->what, "strip"))
rx_vlan_strip_set(port_id,  on);
-   else if(!strcmp(res->what, "stripq")){
+   else if (!strcmp(res->what, "stripq")) {
uint16_t queue_id = 0;
 
/* No queue_id, return */
-   if(i + 1 >= len) {
+   if (i + 1 >= len) {
fprintf(stderr, "must specify (port,queue_id)\n");
return;
}
tmp = strtoul(str + i + 1, NULL, 0);
/* If queue_id greater that what 16-bits can represent, return 
*/
-   if(tmp > 0x)
+   if (tmp > 0x)
return;
 
queue_id = (uint16_t)tmp;
@@ -7207,7 +7208,7 @@ static void cmd_mac_addr_parsed(void *parsed_result,
ret = rte_eth_dev_mac_addr_remove(res->port_num, &res->address);
 
/* check the return value and print it if is < 0 */
-   if(ret < 0)
+   if (ret < 0)
fprintf(stderr, "mac_addr_cmd error: (%s)\n", strerror(-ret));
 
 }
@@ -7780,7 +7781,7 @@ static void cmd_vf_mac_addr_parsed(void *parsed_result,
res->vf_num);
 #endif
 
-   if(ret < 0)
+   if (ret < 0)
fprintf(stderr, "vf_mac_addr_cmd error: (%s)\n", 
strerror(-ret));
 
 }
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index aed4cdcb8485..7fc6d91f0210 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -306,11 +306,12 @@ parse_portnuma_config(const char *q_arg)
/* reset from value set at definition */
while ((p = strchr(p0,'(')) != NULL) {
++p;
-   if((p0 = strchr(p,')')) == NULL)
+   p0 = strchr(p, ')');
+   if (p0 == NULL)
return -1;
 
size = p0 - p;
-   if(size >= sizeof(s))
+   if (size >= sizeof(s))
return -1;
 
snprintf(s, sizeof(s), "%.*s", size, p);
@@ -366,11 +367,12 @@ parse_ringnuma_config(const char *q_arg)
/* reset from value set at definition */
while ((p = strchr(p0,'(')) != NULL) {
++p;
-   if((p0 = strchr(p,')')) == NULL)
+   p0 = strchr(p, ')');
+   if (p0 == NULL)
return -1;
 
size = p0 - p;
-   if(size >= sizeof(s))
+

[PATCH v2 06/14] test: fix whitespace

[Stephen Hemminger]

Add space after keywords.

Signed-off-by: Stephen Hemminger 
---
 app/test/test_cmdline_cirbuf.c |  4 ++--
 app/test/test_debug.c  |  4 ++--
 app/test/test_hash.c   |  6 +++---
 app/test/test_lpm6_perf.c  |  2 +-
 app/test/test_malloc.c | 14 +++---
 app/test/test_mbuf.c   | 14 ++
 app/test/test_spinlock.c   |  2 +-
 7 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/app/test/test_cmdline_cirbuf.c b/app/test/test_cmdline_cirbuf.c
index 8ac326cb02e0..6f7aae6df59a 100644
--- a/app/test/test_cmdline_cirbuf.c
+++ b/app/test/test_cmdline_cirbuf.c
@@ -708,7 +708,7 @@ test_cirbuf_char_fill(void)
return -1;
}
/* delete buffer from tail */
-   for(i = 0; i < CMDLINE_TEST_BUFSIZE; i++)
+   for (i = 0; i < CMDLINE_TEST_BUFSIZE; i++)
cirbuf_del_tail_safe(&cb);
/* try to delete from an empty buffer */
if (cirbuf_del_tail_safe(&cb) >= 0) {
@@ -737,7 +737,7 @@ test_cirbuf_char_fill(void)
return -1;
}
/* delete buffer from head */
-   for(i = 0; i < CMDLINE_TEST_BUFSIZE; i++)
+   for (i = 0; i < CMDLINE_TEST_BUFSIZE; i++)
cirbuf_del_head_safe(&cb);
/* try to delete from an empty buffer */
if (cirbuf_del_head_safe(&cb) >= 0) {
diff --git a/app/test/test_debug.c b/app/test/test_debug.c
index 2704f5b92726..28fc2180e2b0 100644
--- a/app/test/test_debug.c
+++ b/app/test/test_debug.c
@@ -53,7 +53,7 @@ test_panic(void)
return -1;
}
wait(&status);
-   if(status == 0){
+   if (status == 0) {
printf("Child process terminated normally!\n");
return -1;
} else
@@ -84,7 +84,7 @@ test_exit_val(int exit_val)
}
wait(&status);
printf("Child process status: %d\n", status);
-   if(!WIFEXITED(status) || WEXITSTATUS(status) != (uint8_t)exit_val){
+   if (!WIFEXITED(status) || WEXITSTATUS(status) != (uint8_t)exit_val) {
printf("Child process terminated with incorrect status 
(expected = %d)!\n",
exit_val);
return -1;
diff --git a/app/test/test_hash.c b/app/test/test_hash.c
index 3e45afaa67fc..35d9d8a62ef8 100644
--- a/app/test/test_hash.c
+++ b/app/test/test_hash.c
@@ -724,12 +724,12 @@ static int test_five_keys(void)
}
 
/* Lookup */
-   for(i = 0; i < 5; i++)
+   for (i = 0; i < 5; i++)
key_array[i] = &keys[i];
 
ret = rte_hash_lookup_bulk(handle, &key_array[0], 5, (int32_t *)pos);
-   if(ret == 0)
-   for(i = 0; i < 5; i++) {
+   if (ret == 0)
+   for (i = 0; i < 5; i++) {
print_key_info("Lkp", key_array[i], pos[i]);
RETURN_IF_ERROR(pos[i] != expected_pos[i],
"failed to find key (pos[%u]=%d)", i, 
pos[i]);
diff --git a/app/test/test_lpm6_perf.c b/app/test/test_lpm6_perf.c
index aaf2773b6fac..2f9321345e4b 100644
--- a/app/test/test_lpm6_perf.c
+++ b/app/test/test_lpm6_perf.c
@@ -47,7 +47,7 @@ print_route_distribution(const struct rules_tbl_entry *table, 
uint32_t n)
printf("--- \n");
 
/* Count depths. */
-   for(i = 1; i <= 128; i++) {
+   for (i = 1; i <= 128; i++) {
unsigned depth_counter = 0;
double percent_hits;
 
diff --git a/app/test/test_malloc.c b/app/test/test_malloc.c
index de40e506113a..8d3750a3f1f0 100644
--- a/app/test/test_malloc.c
+++ b/app/test/test_malloc.c
@@ -86,8 +86,8 @@ test_align_overlap_per_lcore(__rte_unused void *arg)
ret = -1;
break;
}
-   for(j = 0; j < 1000 ; j++) {
-   if( *(char *)p1 != 0) {
+   for (j = 0; j < 1000 ; j++) {
+   if (*(char *)p1 != 0) {
printf("rte_zmalloc didn't zero the allocated 
memory\n");
ret = -1;
}
@@ -157,8 +157,8 @@ test_reordered_free_per_lcore(__rte_unused void *arg)
ret = -1;
break;
}
-   for(j = 0; j < 1000 ; j++) {
-   if( *(char *)p1 != 0) {
+   for (j = 0; j < 1000 ; j++) {
+   if (*(char *)p1 != 0) {
printf("rte_zmalloc didn't zero the allocated 
memory\n");
ret = -1;
}
@@ -331,12 +331,12 @@ test_multi_alloc_statistics(void)
/* After freeing both allocations check stats return to original */
rte_malloc_get_socket_stats(socket, &post_stats);
 
-   if(second_stats.heap_totalsz_bytes != first_stats.heap_totalsz_bytes) {
+   if (second_stats.heap_totalsz_bytes != first_stats.heap_totalsz_bytes) {
 

[PATCH v2 07/14] examples/qos_sched: fix whitespace

[Stephen Hemminger]

Add space after keywords.

Signed-off-by: Stephen Hemminger 
---
 examples/qos_sched/app_thread.c | 6 +++---
 examples/qos_sched/args.c   | 2 +-
 examples/qos_sched/init.c   | 2 +-
 examples/qos_sched/main.c   | 4 ++--
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/examples/qos_sched/app_thread.c b/examples/qos_sched/app_thread.c
index dbc878b55394..a49356fecf5f 100644
--- a/examples/qos_sched/app_thread.c
+++ b/examples/qos_sched/app_thread.c
@@ -79,7 +79,7 @@ app_rx_thread(struct thread_conf **confs)
if (likely(nb_rx != 0)) {
APP_STATS_ADD(conf->stat.nb_rx, nb_rx);
 
-   for(i = 0; i < nb_rx; i++) {
+   for (i = 0; i < nb_rx; i++) {
get_pkt_sched(rx_mbufs[i],
&subport, &pipe, 
&traffic_class, &queue, &color);
rte_sched_port_pkt_write(conf->sched_port,
@@ -91,7 +91,7 @@ app_rx_thread(struct thread_conf **confs)
 
if (unlikely(rte_ring_sp_enqueue_bulk(conf->rx_ring,
(void **)rx_mbufs, nb_rx, NULL) == 0)) {
-   for(i = 0; i < nb_rx; i++) {
+   for (i = 0; i < nb_rx; i++) {
rte_pktmbuf_free(rx_mbufs[i]);
 
APP_STATS_ADD(conf->stat.nb_drop, 1);
@@ -137,7 +137,7 @@ app_send_packets(struct thread_conf *qconf, struct rte_mbuf 
**mbufs, uint32_t nb
uint32_t i, len;
 
len = qconf->n_mbufs;
-   for(i = 0; i < nb_pkt; i++) {
+   for (i = 0; i < nb_pkt; i++) {
qconf->m_table[len] = mbufs[i];
len++;
/* enough pkts to be sent */
diff --git a/examples/qos_sched/args.c b/examples/qos_sched/args.c
index b2959499ae9a..b752eb221f7c 100644
--- a/examples/qos_sched/args.c
+++ b/examples/qos_sched/args.c
@@ -451,7 +451,7 @@ app_parse_args(int argc, char **argv)
/* sanity check for cores assignment */
nb_lcores = app_cpu_core_count();
 
-   for(i = 0; i < nb_pfc; i++) {
+   for (i = 0; i < nb_pfc; i++) {
if (qos_conf[i].rx_core >= nb_lcores) {
RTE_LOG(ERR, APP, "pfc %u: invalid RX lcore index 
%u\n", i + 1,
qos_conf[i].rx_core);
diff --git a/examples/qos_sched/init.c b/examples/qos_sched/init.c
index 0709aec10c31..41b46b296298 100644
--- a/examples/qos_sched/init.c
+++ b/examples/qos_sched/init.c
@@ -323,7 +323,7 @@ int app_init(void)
rte_exit(EXIT_FAILURE, "Invalid configuration profile\n");
 
/* Initialize each active flow */
-   for(i = 0; i < nb_pfc; i++) {
+   for (i = 0; i < nb_pfc; i++) {
uint32_t socket = rte_lcore_to_socket_id(qos_conf[i].rx_core);
struct rte_ring *ring;
 
diff --git a/examples/qos_sched/main.c b/examples/qos_sched/main.c
index dc6a17a6464a..7b757df0eb02 100644
--- a/examples/qos_sched/main.c
+++ b/examples/qos_sched/main.c
@@ -152,7 +152,7 @@ app_stat(void)
static struct rte_eth_stats tx_stats[MAX_DATA_STREAMS];
 
/* print statistics */
-   for(i = 0; i < nb_pfc; i++) {
+   for (i = 0; i < nb_pfc; i++) {
struct flow_conf *flow = &qos_conf[i];
 
rte_eth_stats_get(flow->rx_port, &stats);
@@ -212,7 +212,7 @@ main(int argc, char **argv)
}
else {
/* print statistics every second */
-   while(1) {
+   while (1) {
sleep(1);
app_stat();
}
-- 
2.35.1

[PATCH v2 08/14] examples/vhost: fix whitespace

[Stephen Hemminger]

Add space after keywords.

Signed-off-by: Stephen Hemminger 
---
 examples/vhost/main.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index 42e53a0f9a4f..9fee966916e9 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -1491,7 +1491,7 @@ switch_worker(void *arg __rte_unused)
}
}
 
-   while(1) {
+   while (1) {
drain_mbuf_table(tx_q);
drain_vhost_table();
/*
@@ -1583,9 +1583,8 @@ destroy_device(int vid)
return;
/*set the remove flag. */
vdev->remove = 1;
-   while(vdev->ready != DEVICE_SAFE_REMOVE) {
+   while (vdev->ready != DEVICE_SAFE_REMOVE)
rte_pause();
-   }
 
for (i = 0; i < RTE_MAX_LCORE; i++)
rte_free(vhost_txbuff[i * RTE_MAX_VHOST_DEVICE + vid]);
@@ -1816,7 +1815,7 @@ print_stats(__rte_unused void *arg)
const char clr[] = { 27, '[', '2', 'J', '\0' };
const char top_left[] = { 27, '[', '1', ';', '1', 'H','\0' };
 
-   while(1) {
+   while (1) {
sleep(enable_stats);
 
/* Clear screen and move to top left */
-- 
2.35.1

[PATCH v2 10/14] examples: fix whitespace

[Stephen Hemminger]

Add space after keywords.

Signed-off-by: Stephen Hemminger 
---
 examples/cmdline/parse_obj_list.c  | 2 +-
 examples/ip_reassembly/main.c  | 2 +-
 examples/multi_process/symmetric_mp/main.c | 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/examples/cmdline/parse_obj_list.c 
b/examples/cmdline/parse_obj_list.c
index 959bcd14527e..759072d39620 100644
--- a/examples/cmdline/parse_obj_list.c
+++ b/examples/cmdline/parse_obj_list.c
@@ -42,7 +42,7 @@ parse_obj_list(cmdline_parse_token_hdr_t *tk, const char 
*buf, void *res,
if (res && ressize < sizeof(struct object *))
return -1;
 
-   while(!cmdline_isendoftoken(buf[token_len]))
+   while (!cmdline_isendoftoken(buf[token_len]))
token_len++;
 
SLIST_FOREACH(o, tkd->list, next) {
diff --git a/examples/ip_reassembly/main.c b/examples/ip_reassembly/main.c
index bd0b1d31decf..7e84b4944759 100644
--- a/examples/ip_reassembly/main.c
+++ b/examples/ip_reassembly/main.c
@@ -300,7 +300,7 @@ send_single_packet(struct rte_mbuf *m, uint16_t port)
 
TX_LCORE_STAT_UPDATE(&qconf->tx_stat, queue, 1);
txmb->m_table[txmb->head] = m;
-   if(++txmb->head == len)
+   if (++txmb->head == len)
txmb->head = 0;
 
return 0;
diff --git a/examples/multi_process/symmetric_mp/main.c 
b/examples/multi_process/symmetric_mp/main.c
index 1ff85875dfdf..2f4441049253 100644
--- a/examples/multi_process/symmetric_mp/main.c
+++ b/examples/multi_process/symmetric_mp/main.c
@@ -156,7 +156,7 @@ smp_parse_args(int argc, char **argv)
 
/* get the port numbers from the port mask */
RTE_ETH_FOREACH_DEV(i)
-   if(port_mask & (1 << i))
+   if (port_mask & (1 << i))
ports[num_ports++] = (uint8_t)i;
 
ret = optind-1;
@@ -470,8 +470,8 @@ main(int argc, char **argv)
/* Primary instance initialized. 8< */
if (num_ports & 1)
rte_exit(EXIT_FAILURE, "Application must use an even number of 
ports\n");
-   for(i = 0; i < num_ports; i++){
-   if(proc_type == RTE_PROC_PRIMARY)
+   for (i = 0; i < num_ports; i++) {
+   if (proc_type == RTE_PROC_PRIMARY)
if (smp_port_init(ports[i], mp, (uint16_t)num_procs) < 
0)
rte_exit(EXIT_FAILURE, "Error initialising 
ports\n");
}
-- 
2.35.1

[PATCH v2 09/14] l3fwd: fix whitespace

[Stephen Hemminger]

Add space after keywords.

Signed-off-by: Stephen Hemminger 
---
 examples/l3fwd-power/main.c | 7 ---
 examples/l3fwd/main.c   | 7 ---
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index fd3ade330f82..7bd5d8cb2281 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -1787,11 +1787,12 @@ parse_config(const char *q_arg)
 
while ((p = strchr(p0,'(')) != NULL) {
++p;
-   if((p0 = strchr(p,')')) == NULL)
+   p0 = strchr(p, ')');
+   if (p0 == NULL)
return -1;
 
size = p0 - p;
-   if(size >= sizeof(s))
+   if (size >= sizeof(s))
return -1;
 
snprintf(s, sizeof(s), "%.*s", size, p);
@@ -2946,7 +2947,7 @@ main(int argc, char **argv)
fflush(stdout);
 
/* init RX queues */
-   for(queue = 0; queue < qconf->n_rx_queue; ++queue) {
+   for (queue = 0; queue < qconf->n_rx_queue; ++queue) {
struct rte_eth_rxconf rxq_conf;
 
portid = qconf->rx_queue_list[queue].port_id;
diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
index 5198ff30dd00..2b8575137d50 100644
--- a/examples/l3fwd/main.c
+++ b/examples/l3fwd/main.c
@@ -516,11 +516,12 @@ parse_config(const char *q_arg)
 
while ((p = strchr(p0,'(')) != NULL) {
++p;
-   if((p0 = strchr(p,')')) == NULL)
+   p0 = strchr(p, ')');
+   if (p0 == NULL)
return -1;
 
size = p0 - p;
-   if(size >= sizeof(s))
+   if (size >= sizeof(s))
return -1;
 
snprintf(s, sizeof(s), "%.*s", size, p);
@@ -1366,7 +1367,7 @@ l3fwd_poll_resource_setup(void)
printf("\nInitializing rx queues on lcore %u ... ", lcore_id );
fflush(stdout);
/* init RX queues */
-   for(queue = 0; queue < qconf->n_rx_queue; ++queue) {
+   for (queue = 0; queue < qconf->n_rx_queue; ++queue) {
struct rte_eth_rxconf rxq_conf;
 
portid = qconf->rx_queue_list[queue].port_id;
-- 
2.35.1

[PATCH v2 11/14] bus/pci: fix whitespace

[Stephen Hemminger]

Add space after keywords.

Signed-off-by: Stephen Hemminger 
---
 drivers/bus/pci/bsd/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bus/pci/bsd/pci.c b/drivers/bus/pci/bsd/pci.c
index 7459d15c7e31..056e2eda2d5f 100644
--- a/drivers/bus/pci/bsd/pci.c
+++ b/drivers/bus/pci/bsd/pci.c
@@ -368,7 +368,7 @@ rte_pci_scan(void)
}
 
dev_count += conf_io.num_matches;
-   } while(conf_io.status == PCI_GETCONF_MORE_DEVS);
+   } while (conf_io.status == PCI_GETCONF_MORE_DEVS);
 
close(fd);
 
-- 
2.35.1

[PATCH v2 13/14] net/i40e: fix whitespace

[Stephen Hemminger]

Add space after keywords.

Signed-off-by: Stephen Hemminger 
---
 drivers/net/i40e/i40e_pf.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/i40e/i40e_pf.c b/drivers/net/i40e/i40e_pf.c
index 15d9ff868f3a..7050e0057d8e 100644
--- a/drivers/net/i40e/i40e_pf.c
+++ b/drivers/net/i40e/i40e_pf.c
@@ -956,7 +956,7 @@ i40e_pf_host_process_cmd_add_vlan(struct i40e_pf_vf *vf,
 
for (i = 0; i < vlan_filter_list->num_elements; i++) {
ret = i40e_vsi_add_vlan(vf->vsi, vid[i]);
-   if(ret != I40E_SUCCESS)
+   if (ret != I40E_SUCCESS)
goto send_msg;
}
 
@@ -996,7 +996,7 @@ i40e_pf_host_process_cmd_del_vlan(struct i40e_pf_vf *vf,
vid = vlan_filter_list->vlan_id;
for (i = 0; i < vlan_filter_list->num_elements; i++) {
ret = i40e_vsi_delete_vlan(vf->vsi, vid[i]);
-   if(ret != I40E_SUCCESS)
+   if (ret != I40E_SUCCESS)
goto send_msg;
}
 
@@ -1577,12 +1577,12 @@ i40e_pf_host_init(struct rte_eth_dev *dev)
 * return if SRIOV not enabled, VF number not configured or
 * no queue assigned.
 */
-   if(!hw->func_caps.sr_iov_1_1 || pf->vf_num == 0 || pf->vf_nb_qps == 0)
+   if (!hw->func_caps.sr_iov_1_1 || pf->vf_num == 0 || pf->vf_nb_qps == 0)
return I40E_SUCCESS;
 
/* Allocate memory to store VF structure */
pf->vfs = rte_zmalloc("i40e_pf_vf",sizeof(*pf->vfs) * pf->vf_num, 0);
-   if(pf->vfs == NULL)
+   if (pf->vfs == NULL)
return -ENOMEM;
 
/* Disable irq0 for VFR event */
-- 
2.35.1

[PATCH v2 12/14] net/e1000: fix whitespace

[Stephen Hemminger]

Add space after keywords.

Signed-off-by: Stephen Hemminger 
---
 drivers/net/e1000/em_ethdev.c  | 4 ++--
 drivers/net/e1000/igb_ethdev.c | 8 
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index 8ee9be12ad19..146e74cfc76e 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -872,8 +872,8 @@ eth_em_stats_get(struct rte_eth_dev *dev, struct 
rte_eth_stats *rte_stats)
E1000_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
int pause_frames;
 
-   if(hw->phy.media_type == e1000_media_type_copper ||
-   (E1000_READ_REG(hw, E1000_STATUS) & E1000_STATUS_LU)) {
+   if (hw->phy.media_type == e1000_media_type_copper ||
+   (E1000_READ_REG(hw, E1000_STATUS) & E1000_STATUS_LU)) {
stats->symerrs += E1000_READ_REG(hw,E1000_SYMERRS);
stats->sec += E1000_READ_REG(hw, E1000_SEC);
}
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 8858f975f8cc..67e82d708964 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -1683,7 +1683,7 @@ igb_read_stats_registers(struct e1000_hw *hw, struct 
e1000_hw_stats *stats)
uint64_t old_rpthc = stats->rpthc;
uint64_t old_hgptc = stats->hgptc;
 
-   if(hw->phy.media_type == e1000_media_type_copper ||
+   if (hw->phy.media_type == e1000_media_type_copper ||
(E1000_READ_REG(hw, E1000_STATUS) & E1000_STATUS_LU)) {
stats->symerrs +=
E1000_READ_REG(hw,E1000_SYMERRS);
@@ -3500,10 +3500,10 @@ static void igbvf_set_vfta_all(struct rte_eth_dev *dev, 
bool on)
 
for (i = 0; i < IGB_VFTA_SIZE; i++){
vfta = shadow_vfta->vfta[i];
-   if(vfta){
+   if (vfta) {
mask = 1;
for (j = 0; j < 32; j++){
-   if(vfta & mask)
+   if (vfta & mask)
igbvf_set_vfta(hw,
(uint16_t)((i<<5)+j), on);
mask<<=1;
@@ -3528,7 +3528,7 @@ igbvf_vlan_filter_set(struct rte_eth_dev *dev, uint16_t 
vlan_id, int on)
 
/*vind is not used in VF driver, set to 0, check ixgbe_set_vfta_vf*/
ret = igbvf_set_vfta(hw, vlan_id, !!on);
-   if(ret){
+   if (ret) {
PMD_INIT_LOG(ERR, "Unable to set VF vlan");
return ret;
}
-- 
2.35.1

[PATCH v2 14/14] net/bnx2x: fix whitespace

[Stephen Hemminger]

Add space after keywords.

Signed-off-by: Stephen Hemminger 
---
 drivers/net/bnx2x/bnx2x.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/bnx2x/bnx2x.h b/drivers/net/bnx2x/bnx2x.h
index 89414ac88a93..9cb927e25449 100644
--- a/drivers/net/bnx2x/bnx2x.h
+++ b/drivers/net/bnx2x/bnx2x.h
@@ -96,7 +96,7 @@ int bnx2x_ilog2(int x)
int log = 0;
x >>= 1;
 
-   while(x) {
+   while (x) {
log++;
x >>= 1;
}
-- 
2.35.1

RE: [RFC] mempool: zero-copy cache put bulk

[Morten Brørup]

+To: Bruce also showed interest in this topic, and might have more insights.

> From: Honnappa Nagarahalli [mailto:honnappa.nagaraha...@arm.com]
> Sent: Wednesday, 9 November 2022 18.58
> 
> 
> 
> >
> > > From: Honnappa Nagarahalli [mailto:honnappa.nagaraha...@arm.com]
> > > Sent: Sunday, 6 November 2022 00.11
> > >
> > > + Akshitha, she is working on similar patch
> > >
> > > Few comments inline
> > >
> > > > From: Morten Brørup 
> > > > Sent: Saturday, November 5, 2022 8:40 AM
> > > >
> > > > Zero-copy access to the mempool cache is beneficial for PMD
> > > performance,
> > > > and must be provided by the mempool library to fix [Bug 1052]
> > > > without
> > > a
> > > > performance regression.
> > > >
> > > > [Bug 1052]: https://bugs.dpdk.org/show_bug.cgi?id=1052
> > > >
> > > >
> > > > This RFC offers a conceptual zero-copy put function, where the
> > > application
> > > > promises to store some objects, and in return gets an address
> where
> > > to store
> > > > them.
> > > >
> > > > I would like some early feedback.
> > > >
> > > > Notes:
> > > > * Allowing the 'cache' parameter to be NULL, and getting it from
> the
> > > > mempool instead, was inspired by rte_mempool_cache_flush().
> > > I am not sure why the 'cache' parameter is required for this API.
> This
> > > API should take the mem pool as the parameter.
> > >
> > > We have based our API on 'rte_mempool_do_generic_put' and removed
> > the
> > > 'cache' parameter.
> >
> > I thoroughly considered omitting the 'cache' parameter, but included
> it for
> > two reasons:
> >
> > 1. The function is a "mempool cache" function (i.e. primarily working
> on the
> > mempool cache), not a "mempool" function.
> >
> > So it is appropriate to have a pointer directly to the structure it
> is working on.
> > Following this through, I also made 'cache' the first parameter and
> 'mp' the
> > second, like in rte_mempool_cache_flush().
> I am wondering if the PMD should be aware of the cache or not. For ex:
> in the case of pipeline mode, the RX and TX side of the PMD are running
> on different cores.

In that example, the PMD can store two cache pointers, one for each of the RX 
and TX side.

And if the PMD is unaware of the cache pointer, it can look it up at runtime 
using rte_lcore_id(), like it does in the current Intel PMDs.

> However, since the rte_mempool_cache_flush API is provided, may be that
> decision is already done? Interestingly, rte_mempool_cache_flush is
> called by just a single PMD.

I intentionally aligned this RFC with rte_mempool_cache_flush() to maintain 
consistency.

However, the API is not set in stone. It should always be acceptable to 
consider improved alternatives.

> 
> So, the question is, should we allow zero-copy only for per-core cache
> or for other cases as well.

I suppose that the mempool library was designed to have a mempool associated 
with exactly one mempool cache per core. (Alternatively, the mempool can be 
configured with no mempool caches at all.)

We should probably stay loyal to that design concept, and only allow zero-copy 
for per-core cache.

If you can come up with an example of the opposite, I would like to explore 
that option too... I can't think of a good example myself, and perhaps I'm 
overlooking a relevant use case.

> 
> >
> > 2. In most cases, the function only accesses the mempool structure in
> order to
> > get the cache pointer. Skipping this step improves performance.
> >
> > And since the cache is created along with the mempool itself (and
> thus never
> > changes for a mempool), it would be safe for the PMD to store the
> 'cache'
> > pointer along with the 'mp' pointer in the PMD's queue structure.
> Agreed
> 
> >
> > E.g. in the i40e PMD the i40e_rx_queue structure could include a
> "struct
> > rte_mempool_cache *cache" field, which could be used i40e_rxq_rearm()
> [1]
> > instead of "cache = rte_mempool_default_cache(rxq->mp,
> rte_lcore_id())".
> >
> > [1] https://elixir.bootlin.com/dpdk/v22.11-
> > rc2/source/drivers/net/i40e/i40e_rxtx_vec_avx512.c#L31
> >
> > > This new API, on success, returns the pointer to memory where the
> > > objects are copied. On failure it returns NULL and the caller has
> to
> > > call 'rte_mempool_ops_enqueue_bulk'. Alternatively, the new API
> could
> > > do this as well and PMD does not need to do anything if it gets a
> NULL
> > > pointer.
> >
> > Yes, we agree about these two details:
> >
> > 1. The function should return a pointer, not an integer.
> > It would be a waste to use a another CPU register to convey a
> success/error
> > integer value, when the success/failure information is just as easily
> conveyed
> > by the pointer return value (non-NULL/NULL), and rte_errno for
> various error
> > values in the unlikely cases.
> >
> > 2. The function should leave it up to the PMD what to do if direct
> access to
> > the cache is unavailable.
> Just wondering about the advantage of this. I do not think PMD's have
> much of a choice other than calling 'rte_mem

Re: [PATCH v4] testpmd: cleanup cleanly from signal

[Mattias Rönnblom]

On 2022-11-09 05:10, Stephen Hemminger wrote:

Do a clean shutdown of testpmd when a signal is received;
instead of having testpmd kill itself.
This fixes problem where a signal could be received
in the middle of a PMD and then the signal handler would call
PMD's close routine which could cause a deadlock.

Added benefit is it gets rid of Windows specific code.

Fixes: d9a191a00e81 ("app/testpmd: fix quitting in container")
Signed-off-by: Stephen Hemminger 
---
v4 - use select() because that is available on Windows; and other
  functions poll() and sigaction() are not.

  app/test-pmd/testpmd.c | 63 +++---
  1 file changed, 34 insertions(+), 29 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index cf5942d0c422..274e96cac2d4 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -12,6 +12,7 @@
  #ifndef RTE_EXEC_ENV_WINDOWS
  #include 
  #endif
+#include 
  #include 
  #include 
  #include 
@@ -4251,26 +4252,11 @@ print_stats(void)
  static void
  signal_handler(int signum)
  {
-   if (signum == SIGINT || signum == SIGTERM) {
-   fprintf(stderr, "\nSignal %d received, preparing to exit...\n",
-   signum);
-#ifdef RTE_LIB_PDUMP
-   /* uninitialize packet capture framework */
-   rte_pdump_uninit();
-#endif
-#ifdef RTE_LIB_LATENCYSTATS
-   if (latencystats_enabled != 0)
-   rte_latencystats_uninit();
-#endif
-   force_quit();
-   /* Set flag to indicate the force termination. */
-   f_quit = 1;
-   /* exit with the expected status */
-#ifndef RTE_EXEC_ENV_WINDOWS
-   signal(signum, SIG_DFL);
-   kill(getpid(), signum);
-#endif
-   }
+   fprintf(stderr, "\nSignal %d %s received, preparing to exit...\n",
+   signum, strsignal(signum));


fprintf() is not async signal safe, and neither is strsignal().

This is not a regression introduced by this patch, but I thought it 
might be worth fixing.



+
+   /* Set flag to indicate the force termination. */
+   f_quit = 1;
  }
  
  int

@@ -4449,9 +4435,6 @@ main(int argc, char** argv)
} else
  #endif
{
-   char c;
-   int rc;
-
f_quit = 0;
  
  		printf("No commandline core given, start packet forwarding\n");

@@ -4476,15 +4459,37 @@ main(int argc, char** argv)
prev_time = cur_time;
rte_delay_us_sleep(US_PER_S);
}
-   }
+   } else {
+   char c;
+   fd_set fds;
  
-		printf("Press enter to exit\n");

-   rc = read(0, &c, 1);
-   pmd_test_exit();
-   if (rc < 0)
-   return 1;
+   printf("Press enter to exit\n");
+
+   FD_ZERO(&fds);
+   FD_SET(0, &fds);
+
+   if (select(1, &fds, NULL, NULL, NULL) <= 0) {
+   fprintf(stderr, "Select failed: %s\n",
+   strerror(errno));


Why is select() needed? Wouldn't a blocking read suffice? Or getchar().


+   } else if (read(0, &c, 1) <= 0) {
+   fprintf(stderr,
+   "Read stdin failed: %s\n",
+   strerror(errno));
+   }
+   }
+   stop_packet_forwarding();
+   force_quit();
}
  
+#ifdef RTE_LIB_PDUMP

+   /* uninitialize packet capture framework */
+   rte_pdump_uninit();
+#endif
+#ifdef RTE_LIB_LATENCYSTATS
+   if (latencystats_enabled != 0)
+   rte_latencystats_uninit();
+#endif
+
ret = rte_eal_cleanup();
if (ret != 0)
rte_exit(EXIT_FAILURE,

Re: Question about naive XOR hash in DPDK

[Mattias Rönnblom]

On 2022-11-07 19:57, Bili Dong wrote:

Dear DPDK devs,

We are using DPDK as the backend target of a P4 pipeline 
(https://github.com/p4lang/p4-dpdk-target 
). A recent issue we are 
trying to solve is to support a naive XOR hash (something like this 
) in this pipeline. This requires an XOR hash implementation in DPDK.I have the following questions:



Why would it *require* an xor hash function in DPDK?



 1. Is there already an XOR hash implementation in DPDK? I haven't found
it myself, but I could have missed it.
 2. If it doesn't exist, I'm willing to contribute one, as the
implementation is quite straightforward. But I might need your help
on where to put the code, as I'm not that familiar with the code
organization.

Any help would be appreciated!

Thanks,
Bili

[PATCH] net/mlx5: fix port's event cleaning order

[Michael Baum]

The shared IB device (sh) has per port data with filed for interrupt
handler port_id. It used by shared interrupt handler to find the
corresponding rte_eth device by IB port index.
If value is equal or greater RTE_MAX_ETHPORTS it means there is no
subhandler installed for specified IB port index.

When a few ports are created under same sh, the sh is created with the
first port and the interrupt handler port_id is initialized to
RTE_MAX_ETHPORTS for each port.
In port creation, the interrupt handler port_id is updated with the
correct value. Since this updating, the mlx5_dev_interrupt_nl_cb
function uses this port and its priv structure.
However, when the ports are closed, this filed isn't updated and the
interrupt handler continue working untill it is uninstalled in SH
destruction.
If mlx5_dev_interrupt_nl_cb is called between port closing and SH
destruction, it uses invalid port causing a crash.

This patch adds interrupt handler port_id updating to the close function
and add memory barrier to make sure it is done before priv reset.

Fixes: 655c3c26c11e ("net/mlx5: fix initial link status detection")
Cc: dkozl...@nvidia.com
Cc: sta...@dpdk.org

Signed-off-by: Michael Baum 
Acked-by: Matan Azrad 
---
 drivers/net/mlx5/linux/mlx5_os.c | 3 +++
 drivers/net/mlx5/mlx5.c  | 6 ++
 2 files changed, 9 insertions(+)

diff --git a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c
index 2b6741396d..a71474c90a 100644
--- a/drivers/net/mlx5/linux/mlx5_os.c
+++ b/drivers/net/mlx5/linux/mlx5_os.c
@@ -1676,6 +1676,9 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
return eth_dev;
 error:
if (priv) {
+   priv->sh->port[priv->dev_port - 1].nl_ih_port_id =
+  RTE_MAX_ETHPORTS;
+   rte_io_wmb();
 #ifdef HAVE_MLX5_HWS_SUPPORT
if (eth_dev &&
priv->sh &&
diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 1cf6df6049..95b0151fbc 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -2137,6 +2137,12 @@ mlx5_dev_close(struct rte_eth_dev *dev)
if (!c)
claim_zero(rte_eth_switch_domain_free(priv->domain_id));
}
+   priv->sh->port[priv->dev_port - 1].nl_ih_port_id = RTE_MAX_ETHPORTS;
+   /*
+* The interrupt handler port id must be reset before priv is reset
+* since 'mlx5_dev_interrupt_nl_cb' uses priv.
+*/
+   rte_io_wmb();
memset(priv, 0, sizeof(*priv));
priv->domain_id = RTE_ETH_DEV_SWITCH_DOMAIN_ID_INVALID;
/*
-- 
2.25.1

[PATCH v2] net/mlx5: fix port's event cleaning order

[Michael Baum]

The shared IB device (sh) has per port data with filed for interrupt
handler port_id. It used by shared interrupt handler to find the
corresponding rte_eth device by IB port index.
If value is equal or greater RTE_MAX_ETHPORTS it means there is no
subhandler installed for specified IB port index.

When a few ports are created under same sh, the sh is created with the
first port and the interrupt handler port_id is initialized to
RTE_MAX_ETHPORTS for each port.
In port creation, the interrupt handler port_id is updated with the
correct value. Since this updating, the mlx5_dev_interrupt_nl_cb
function uses this port and its priv structure.
However, when the ports are closed, this filed isn't updated and the
interrupt handler continue working until it is uninstalled in SH
destruction.
If mlx5_dev_interrupt_nl_cb is called between port closing and SH
destruction, it uses invalid port causing a crash.

This patch adds interrupt handler port_id updating to the close function
and add memory barrier to make sure it is done before priv reset.

Fixes: 655c3c26c11e ("net/mlx5: fix initial link status detection")
Cc: dkozl...@nvidia.com
Cc: sta...@dpdk.org

Signed-off-by: Michael Baum 
Acked-by: Matan Azrad 
---

v2: fix typo in commit message.

 drivers/net/mlx5/linux/mlx5_os.c | 3 +++
 drivers/net/mlx5/mlx5.c  | 6 ++
 2 files changed, 9 insertions(+)

diff --git a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c
index 2b6741396d..a71474c90a 100644
--- a/drivers/net/mlx5/linux/mlx5_os.c
+++ b/drivers/net/mlx5/linux/mlx5_os.c
@@ -1676,6 +1676,9 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
return eth_dev;
 error:
if (priv) {
+   priv->sh->port[priv->dev_port - 1].nl_ih_port_id =
+  RTE_MAX_ETHPORTS;
+   rte_io_wmb();
 #ifdef HAVE_MLX5_HWS_SUPPORT
if (eth_dev &&
priv->sh &&
diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 1cf6df6049..95b0151fbc 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -2137,6 +2137,12 @@ mlx5_dev_close(struct rte_eth_dev *dev)
if (!c)
claim_zero(rte_eth_switch_domain_free(priv->domain_id));
}
+   priv->sh->port[priv->dev_port - 1].nl_ih_port_id = RTE_MAX_ETHPORTS;
+   /*
+* The interrupt handler port id must be reset before priv is reset
+* since 'mlx5_dev_interrupt_nl_cb' uses priv.
+*/
+   rte_io_wmb();
memset(priv, 0, sizeof(*priv));
priv->domain_id = RTE_ETH_DEV_SWITCH_DOMAIN_ID_INVALID;
/*
-- 
2.25.1

RE: [RFC] mempool: zero-copy cache put bulk

[Honnappa Nagarahalli]

> 
> +To: Bruce also showed interest in this topic, and might have more insights.
> 
> > From: Honnappa Nagarahalli [mailto:honnappa.nagaraha...@arm.com]
> > Sent: Wednesday, 9 November 2022 18.58
> >
> > 
> >
> > >
> > > > From: Honnappa Nagarahalli [mailto:honnappa.nagaraha...@arm.com]
> > > > Sent: Sunday, 6 November 2022 00.11
> > > >
> > > > + Akshitha, she is working on similar patch
> > > >
> > > > Few comments inline
> > > >
> > > > > From: Morten Brørup 
> > > > > Sent: Saturday, November 5, 2022 8:40 AM
> > > > >
> > > > > Zero-copy access to the mempool cache is beneficial for PMD
> > > > performance,
> > > > > and must be provided by the mempool library to fix [Bug 1052]
> > > > > without
> > > > a
> > > > > performance regression.
> > > > >
> > > > > [Bug 1052]: https://bugs.dpdk.org/show_bug.cgi?id=1052
> > > > >
> > > > >
> > > > > This RFC offers a conceptual zero-copy put function, where the
> > > > application
> > > > > promises to store some objects, and in return gets an address
> > where
> > > > to store
> > > > > them.
> > > > >
> > > > > I would like some early feedback.
> > > > >
> > > > > Notes:
> > > > > * Allowing the 'cache' parameter to be NULL, and getting it from
> > the
> > > > > mempool instead, was inspired by rte_mempool_cache_flush().
> > > > I am not sure why the 'cache' parameter is required for this API.
> > This
> > > > API should take the mem pool as the parameter.
> > > >
> > > > We have based our API on 'rte_mempool_do_generic_put' and removed
> > > the
> > > > 'cache' parameter.
> > >
> > > I thoroughly considered omitting the 'cache' parameter, but included
> > it for
> > > two reasons:
> > >
> > > 1. The function is a "mempool cache" function (i.e. primarily
> > > working
> > on the
> > > mempool cache), not a "mempool" function.
> > >
> > > So it is appropriate to have a pointer directly to the structure it
> > is working on.
> > > Following this through, I also made 'cache' the first parameter and
> > 'mp' the
> > > second, like in rte_mempool_cache_flush().
> > I am wondering if the PMD should be aware of the cache or not. For ex:
> > in the case of pipeline mode, the RX and TX side of the PMD are
> > running on different cores.
> 
> In that example, the PMD can store two cache pointers, one for each of the
> RX and TX side.
I did not understand this. If RX core and TX core have their own per-core 
caches the logic would not work. For ex: the RX core cache would not get filled.

In the case of pipeline mode, there will not be a per-core cache. The buffers 
would be allocated and freed from a global ring or a global lockless stack.

> 
> And if the PMD is unaware of the cache pointer, it can look it up at runtime
> using rte_lcore_id(), like it does in the current Intel PMDs.
> 
> > However, since the rte_mempool_cache_flush API is provided, may be
> > that decision is already done? Interestingly, rte_mempool_cache_flush
> > is called by just a single PMD.
> 
> I intentionally aligned this RFC with rte_mempool_cache_flush() to maintain
> consistency.
> 
> However, the API is not set in stone. It should always be acceptable to
> consider improved alternatives.
> 
> >
> > So, the question is, should we allow zero-copy only for per-core cache
> > or for other cases as well.
> 
> I suppose that the mempool library was designed to have a mempool
> associated with exactly one mempool cache per core. (Alternatively, the
> mempool can be configured with no mempool caches at all.)
> 
> We should probably stay loyal to that design concept, and only allow zero-
> copy for per-core cache.
> 
> If you can come up with an example of the opposite, I would like to explore
> that option too... I can't think of a good example myself, and perhaps I'm
> overlooking a relevant use case.
The use case I am talking about is the pipeline mode as I mentioned above. Let 
me know if you agree.

> 
> >
> > >
> > > 2. In most cases, the function only accesses the mempool structure
> > > in
> > order to
> > > get the cache pointer. Skipping this step improves performance.
> > >
> > > And since the cache is created along with the mempool itself (and
> > thus never
> > > changes for a mempool), it would be safe for the PMD to store the
> > 'cache'
> > > pointer along with the 'mp' pointer in the PMD's queue structure.
> > Agreed
> >
> > >
> > > E.g. in the i40e PMD the i40e_rx_queue structure could include a
> > "struct
> > > rte_mempool_cache *cache" field, which could be used
> > > i40e_rxq_rearm()
> > [1]
> > > instead of "cache = rte_mempool_default_cache(rxq->mp,
> > rte_lcore_id())".
> > >
> > > [1] https://elixir.bootlin.com/dpdk/v22.11-
> > > rc2/source/drivers/net/i40e/i40e_rxtx_vec_avx512.c#L31
> > >
> > > > This new API, on success, returns the pointer to memory where the
> > > > objects are copied. On failure it returns NULL and the caller has
> > to
> > > > call 'rte_mempool_ops_enqueue_bulk'. Alternatively, the new API
> > could
> > > > do this as well and PMD does not need t