date:20240923

Re: [PATCH v3] eal: add build-time option to omit trace

2024-09-23 Thread Jerin Jacob

On Fri, Sep 20, 2024 at 2:38 PM Morten Brørup  
wrote:
>
> Some applications want to omit the trace feature.
> Either to reduce the memory footprint, to reduce the exposed attack
> surface, or for other reasons.
>
> This patch adds an option in rte_config.h to include or omit trace in the
> build. Trace is included by default.
>
> Omitting trace works by omitting all trace points.
> For API and ABI compatibility, the trace feature itself remains.
>
> Signed-off-by: Morten Brørup 
> ---
> v3:
> * Simpler version with much fewer ifdefs. (Jerin Jacob)
> v2:
> * Added/modified macros required for building applications with trace
>   omitted.
> ---
>  app/test/test_trace.c | 11 ++-
>  config/rte_config.h   |  1 +
>  lib/eal/include/rte_trace_point.h | 21 +
>  3 files changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/app/test/test_trace.c b/app/test/test_trace.c
> index 00809f433b..7918cc865d 100644
> --- a/app/test/test_trace.c
> +++ b/app/test/test_trace.c
> @@ -12,7 +12,16 @@
>
>  int app_dpdk_test_tp_count;
>
> -#ifdef RTE_EXEC_ENV_WINDOWS
> +#if !defined(RTE_TRACE)

Now that, we are is not disabling any symbols,
This conditional compilation can be removed. Use
__rte_trace_point_generic_is_enabled() in another leg.


> +
> +static int
> +test_trace(void)
> +{
> +   printf("trace omitted at build-time, skipping test\n");
> +   return TEST_SKIPPED;
> +}
> +
> +#elif defined(RTE_EXEC_ENV_WINDOWS)
>
>  static int
>  test_trace(void)
> diff --git a/config/rte_config.h b/config/rte_config.h
> index dd7bb0d35b..fd6f8a2f1a 100644
> --- a/config/rte_config.h
> +++ b/config/rte_config.h
> @@ -49,6 +49,7 @@
>  #define RTE_MAX_TAILQ 32
>  #define RTE_LOG_DP_LEVEL RTE_LOG_INFO
>  #define RTE_MAX_VFIO_CONTAINERS 64
> +#define RTE_TRACE 1
>
>  /* bsd module defines */
>  #define RTE_CONTIGMEM_MAX_NUM_BUFS 64
> diff --git a/lib/eal/include/rte_trace_point.h 
> b/lib/eal/include/rte_trace_point.h
> index 41e2a7f99e..b80688ce89 100644
> --- a/lib/eal/include/rte_trace_point.h
> +++ b/lib/eal/include/rte_trace_point.h
> @@ -212,6 +212,25 @@ bool rte_trace_point_is_enabled(rte_trace_point_t *tp);
>  __rte_experimental
>  rte_trace_point_t *rte_trace_point_lookup(const char *name);
>
> +/**
> + * @internal
> + *
> + * Test if the tracepoint compile-time option is enabled.
> + *
> + * @return
> + *   true if tracepoint enabled, false otherwise.
> + */
> +__rte_experimental
> +static __rte_always_inline bool
> +__rte_trace_point_generic_is_enabled(void)

Add to version.map file

> +{
> +#ifdef RTE_TRACE
> +   return true;
> +#else
> +   return false;
> +#endif
> +}
> +
>  /**
>   * @internal
>   *
> @@ -359,6 +378,8 @@ __rte_trace_point_emit_ev_header(void *mem, uint64_t in)
>  #define __rte_trace_point_emit_header_generic(t) \
>  void *mem; \
>  do { \
> +   if (!__rte_trace_point_generic_is_enabled()) \
> +   return; \


To be super safe, Add similar check in
RTE_INIT(trace##_init)

> const uint64_t val = rte_atomic_load_explicit(t, 
> rte_memory_order_acquire); \
> if (likely(!(val & __RTE_TRACE_FIELD_ENABLE_MASK))) \
> return; \
> --
> 2.43.0
>

Re: [PATCH v1] ethdev: fix int overflow in descriptor count logic

2024-09-23 Thread Andrew Rybchenko


On 9/23/24 12:26, Niall Meade wrote:

Addressed a specific overflow issue in the eth_dev_adjust_nb_desc()
function where the uint16_t variable nb_desc would overflow when its
value was greater than (2^16 - nb_align). This overflow caused nb_desc
to incorrectly wrap around between 0 and nb_align-1, leading to the
function setting nb_desc to nb_min instead of the expected nb_max.

The resolution involves upcasting nb_desc to a uint32_t before the
RTE_ALIGN_CEIL macro is applied. This change ensures that the subsequent
call to RTE_ALIGN_FLOOR(nb_desc + (nb_align - 1), nb_align) does not
result in an overflow, as it would when nb_desc is a uint16_t. By using
a uint32_t for these operations, the correct behavior is maintained
without the risk of overflow.

Fixes: 0f67fc3baeb9 ("ethdev: add function to adjust number of descriptors")

Signed-off-by: Niall Meade 


Thanks a lot for the fix.

Reviewed-by: Andrew Rybchenko

Re: [PATCH v2 1/3] eventdev: introduce event pre-scheduling

2024-09-23 Thread Mattias Rönnblom

On 2024-09-19 15:13, Pavan Nikhilesh Bhagavatula wrote:

From: pbhagavat...@marvell.com 
Sent: Tuesday, September 17, 2024 3:11 AM
To: jer...@marvell.com; sthot...@marvell.com; Sevincer, Abdullah
; hemant.agra...@nxp.com;
sachin.sax...@oss.nxp.com; Van Haaren, Harry

;

mattias.ronnb...@ericsson.com; lian...@liangbit.com; Mccarthy, Peter

Cc: dev@dpdk.org; Pavan Nikhilesh 
Subject: [PATCH v2 1/3] eventdev: introduce event pre-scheduling

From: Pavan Nikhilesh 

Event pre-scheduling improves scheduling performance by assigning events

to

event ports in advance when dequeues are issued.
The dequeue operation initiates the pre-schedule operation, which

completes in

parallel without affecting the dequeued event flow contexts and dequeue
latency.

Is the prescheduling done to get the event more quickly in the next dequeue?
The first dequeue executes pre-schedule to make events available for the next
dequeue.
Is this how it is supposed to work?

Yes, that is correct.

"improves scheduling performance" may be a bit misleading, in that case. 
I suggest "reduces scheduling overhead" instead. You can argue it likely 
reduces scheduling performance, in certain scenarios. "reduces 
scheduling overhead, at the cost of load balancing performance."

It seems to me that this should be a simple hint-type API, where the 
hint is used by the event device to decide if pre-scheduling should be 
used or not (assuming pre-scheduling on/off is even an option). The hint 
would just be a way for the application to express whether or not it 
want the scheduler to prioritize load balancing agility and port-to-port 
wall-time latency, or scheduling overhead, which in turn could 
potentially be rephrased as the app being throughput or latency/RT-oriented.

It could also be useful for the event device to know which priority 
levels are to be considered latency-sensitive, and which are 
throughput-oriented - maybe in the form of a threshold.

Event devices can indicate pre-scheduling capabilities using
`RTE_EVENT_DEV_CAP_EVENT_PRESCHEDULE` and
`RTE_EVENT_DEV_CAP_EVENT_PRESCHEDULE_ADAPTIVE` via the event

device

info function `info.event_dev_cap`.

Applications can select the pre-schedule type and configure it through
`rte_event_dev_config.preschedule_type` during `rte_event_dev_configure`.

The supported pre-schedule types are:
  * `RTE_EVENT_DEV_PRESCHEDULE_NONE` - No pre-scheduling.
  * `RTE_EVENT_DEV_PRESCHEDULE` - Always issue a pre-schedule on

dequeue.

  * `RTE_EVENT_DEV_PRESCHEDULE_ADAPTIVE` - Delay issuing pre-schedule
until
there are no forward progress constraints with the held flow contexts.

Signed-off-by: Pavan Nikhilesh 
---
  app/test/test_eventdev.c| 63 +
  doc/guides/prog_guide/eventdev/eventdev.rst | 22 +++
  lib/eventdev/rte_eventdev.h | 48 
  3 files changed, 133 insertions(+)

diff --git a/app/test/test_eventdev.c b/app/test/test_eventdev.c index
e4e234dc98..cf496ee88d 100644
--- a/app/test/test_eventdev.c
+++ b/app/test/test_eventdev.c
@@ -1250,6 +1250,67 @@ test_eventdev_profile_switch(void)
return TEST_SUCCESS;
  }

+static int
+preschedule_test(rte_event_dev_preschedule_type_t preschedule_type,
+const char *preschedule_name) {
+#define NB_EVENTS 1024
+   uint64_t start, total;
+   struct rte_event ev;
+   int rc, cnt;
+
+   ev.event_type = RTE_EVENT_TYPE_CPU;
+   ev.queue_id = 0;
+   ev.op = RTE_EVENT_OP_NEW;
+   ev.u64 = 0xBADF00D0;
+
+   for (cnt = 0; cnt < NB_EVENTS; cnt++) {
+   ev.flow_id = cnt;
+   rc = rte_event_enqueue_burst(TEST_DEV_ID, 0, &ev, 1);
+   TEST_ASSERT(rc == 1, "Failed to enqueue event");
+   }
+
+   RTE_SET_USED(preschedule_type);
+   total = 0;
+   while (cnt) {
+   start = rte_rdtsc_precise();
+   rc = rte_event_dequeue_burst(TEST_DEV_ID, 0, &ev, 1, 0);
+   if (rc) {
+   total += rte_rdtsc_precise() - start;
+   cnt--;
+   }
+   }
+   printf("Preschedule type : %s, avg cycles %" PRIu64 "\n",
preschedule_name,
+  total / NB_EVENTS);
+
+   return TEST_SUCCESS;
+}
+
+static int
+test_eventdev_preschedule_configure(void)
+{
+   struct rte_event_dev_config dev_conf;
+   struct rte_event_dev_info info;
+   int rc;
+
+   rte_event_dev_info_get(TEST_DEV_ID, &info);
+
+   if ((info.event_dev_cap &

RTE_EVENT_DEV_CAP_EVENT_PRESCHEDULE)

== 0)
+   return TEST_SKIPPED;
+
+   devconf_set_default_sane_values(&dev_conf, &info);
+   dev_conf.preschedule_type = RTE_EVENT_DEV_PRESCHEDULE;
+   rc = rte_event_dev_configure(TEST_DEV_ID, &dev_conf);
+   TEST_ASSERT_SUCCESS(rc, "Failed to configure eventdev");
+
+   rc = preschedule_test(RTE_EVENT_DEV_PRESCHEDULE_NONE,
"RTE_EVENT_DEV_PRESCHEDULE_NONE");
+   rc |= preschedule_test(RTE_EVENT_DEV_PRESCHEDULE,
"RT

Minutes of DPDK Technical Board Meeting, 2024-09-18

2024-09-23 Thread Thomas Monjalon

Members Attending: 11/11
- Aaron Conole
- Bruce Richardson
- Hemant Agrawal
- Honnappa Nagarahalli
- Jerin Jacob
- Kevin Traynor
- Konstantin Ananyev
- Maxime Coquelin
- Morten Brørup
- Stephen Hemminger
- Thomas Monjalon (Chair)

NOTE: The Technical Board meetings take place every second Wednesday at 3 pm UTC
on
https://zoom-lfx.platform.linuxfoundation.org/meeting/96459488340?password=d808f1f6-0a28-4165-929e-5a5bcae7efeb
Meetings are public, and DPDK community members are welcome to attend.
Agenda and minutes can be found at http://core.dpdk.org/techboard/minutes

1/ DPDK summit remote speakers

The techboard prefers talks happening on stage in general.
One week before the summit it appears that many speakers
won't be able to travel because of VISA issues.
It has been decided to allow these speakers to present remotely,
preferably with the help of someone being physically on stage.
The same decision applies in case a speaker has another issue
preventing a travel to Canada.
Remote speakers are asked to provide a link to a pre-recorded video backup.

2/ dpdk-next-dts tree maintainer

Juraj Linkeš won't be able to continue his involvement in DTS.
He will be replaced in the role of git tree maintainer for DTS
by Paul Szczepanek (Arm) and Patrick Robb (UNH).
Paul agreed to be the primary maintainer.

3/ Linux uAPI headers import

Maxime asked whether his proposal to import some Linux headers
inside the DPDK repository was accepted.
The goal is to be able to use a recent Linux API without having
to consider whether the build machine knows it or not.
As always, if a call is not supported in the running kernel,
an error will be returned by the kernel.
The board confirmed it is the right direction.

4/ DPDK + AI

Nathan raised a recurring question about the position of DPDK
regarding AI techniques. He asked how to integrate it.
Some usages of AI for networking were discussed briefly.
There will be more information and discussions during the summit.

Then another discussion started about helping the AI training with DPDK.
There is a desire to have such protocol layers implemented with DPDK,
and we are looking for volunteers.

Re: [PATCH v5 1/1] dts: add text parser for testpmd verbose output

2024-09-23 Thread Juraj Linkeš






The question is when the exception would be raised, or, in other words,
what should we do when hasattr(cls, name) is False. If I understand this
correctly, is it's False, then name is not among the flags and that
means testpmd returned an unsupported flag, which shouldn't happen, but
if it does in the future, we would be better off throwing an exception,
or at very least, log a warning, so that we have an indication that we
need to add support for a new flag.


This is a good point. Realistically if it is ever false that would
mean we have a gap in implementation. I like the idea of flagging a
loud warning over throwing an exception in this case though since if
we threw an exception that would stop all test cases that use OL flags
to stop working even if they don't require the new flag. That would
definitely get the problem fixed sooner, but would also shutdown
automated testing until it then.



It's a tradeoff between risking CI being affected (if a new flag is 
added without also adding it to DTS) and how noticable the warning is. I 
guess we can implement something in CI that will look for warnings like 
these?





+flag |= cls[name]
+return flag
+
+@classmethod
+def make_parser(cls) -> ParserFn:
+"""Makes a parser function.
+
+Returns:
+ParserFn: A dictionary for the `dataclasses.field` metadata 
argument containing a
+parser function that makes an instance of this flag from text.
+"""
+return TextParser.wrap(
+TextParser.wrap(TextParser.find(r"ol_flags: ([^\n]+)"), str.split),
+cls.from_str_list,
+)


The RSSOffloadTypesFlag does the split in its from_list_string method.
Do we want to do the same here?

Maybe could create a ParsableFlag (or Creatable? Or something else)
superclass that would implement these from_* methods (from_list_string,
from_str) and subclass it. Flags should be subclassable if they don't
contain members.

The superclass would be useful so that we don't redefine the same method
over and over and so that it's clear what's already available.


I like this idea a lot. Basically all of these flags that are used in
parsers are going to need something like that which is going to be
basically the same so just implementing it one time would be great.
I'm not sure if it fits the scope of this series though, do you think
I should write it and add it here or in a separate patch?



A separate patch seems better, as it touches different parts of the 
code. We should probably implement the same logic in this patch (without 
the exception or warning and with the same if condition) and then make 
changes in the other patch.






@@ -656,6 +1147,9 @@ def stop(self, verify: bool = True) -> None:
   Raises:
   InteractiveCommandExecutionError: If `verify` is :data:`True` 
and the command to stop
   forwarding results in an error.
+
+Returns:
+Output gathered from sending the stop command.


This not just from sending the stop command, but everything else that
preceded (when collecting the verbose output), right?


Technically yes, but that's just due to the nature of how interactive
shells aren't perfect when it comes to asynchronous output. That's why
I tried to be sneaky and say that it is the "output gathered from
sending the stop command" trying to imply that it is not just the
output of the `stop` command, but all the output that is gathered from
sending it. I can update this though.





diff --git a/dts/framework/utils.py b/dts/framework/utils.py



@@ -27,6 +27,12 @@
   from .exception import ConfigurationError

   REGEX_FOR_PCI_ADDRESS: str = 
"/[0-9a-fA-F]{4}:[0-9a-fA-F]{2}:[0-9a-fA-F]{2}.[0-9]{1}/"
+_REGEX_FOR_COLON_SEP_MAC: str = r"(?:[\da-fA-F]{2}:){5}[\da-fA-F]{2}"
+_REGEX_FOR_HYPHEN_SEP_MAC: str = r"(?:[\da-fA-F]{2}-){5,7}[\da-fA-F]{2}"


{5,7} should be just 5 repetitions. When could it be more?


I added it for EUI-64 addresses, but maybe this isn't very relevant
here since I just read that they are encouraged on non-ethernet
devices. I can remove it if it doesn't seem worth it to capture.



From what I gather the EUI-64 address is composed from MAC addresses, 
but it's a different identifier. I'd say if we ever need it we can add 
it as a separate regex (and look for both if we need to).

Re: [PATCH 2/9] drivers/raw: introduce cnxk rvu lf device driver

2024-09-23 Thread Jerin Jacob

On Sun, Sep 8, 2024 at 1:28 AM Akhil Goyal  wrote:
>
> CNXK product families can have a use case to allow PF and VF
> applications to communicate using mailboxes and also get notified
> of any interrupt that may occur on the device.
> Hence, a new raw device driver is added for such RVU LF devices.
> These devices can map to a PF or a VF which can send mailboxes to
> each other.
>
> Signed-off-by: Akhil Goyal 

> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(C) 2024 Marvell.
> +#
> +
> +deps += ['bus_pci', 'common_cnxk', 'rawdev']
> +sources = files(
> +'cnxk_rvu_lf.c',
> +)
> +require_iova_in_mbuf = false
> diff --git a/drivers/raw/cnxk_rvu_lf/rte_pmd_rvu_lf.h 
> b/drivers/raw/cnxk_rvu_lf/rte_pmd_rvu_lf.h
> new file mode 100644
> index 00..2d3cd032b7
> --- /dev/null
> +++ b/drivers/raw/cnxk_rvu_lf/rte_pmd_rvu_lf.h
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(C) 2024 Marvell.
> + */
> +
> +#ifndef _CNXK_RVU_LF_H_
> +#define _CNXK_RVU_LF_H_
> +
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 


Missing update to doc/api/doxy-api-index.md

> +
> +/**
> + * @file rte_pmd_rvu_lf.h
> + *
> + * Marvell RVU LF raw PMD specific structures and interface
> + *
> + * This API allows applications to manage RVU LF device in user space along 
> with
> + * installing interrupt handlers for low latency signal processing.
> + */
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif


> +extern int cnxk_logtype_rvu_lf;

Public symbol. Please add Doxygen symbol

> +
> +#define CNXK_RVU_LF_LOG(level, fmt, args...)   \

Public symbol. Please add Doxygen symbol

> +   rte_log(RTE_LOG_ ## level, cnxk_logtype_rvu_lf, \
> +   "%s(): " fmt "\n", __func__, ## args)
> +

Do we need to make this public symbols?

Re: [PATCH v2 1/2] vhost: add logging mechanism for reconnection

2024-09-23 Thread David Marchand

On Fri, Sep 20, 2024 at 11:09 AM Maxime Coquelin
 wrote:
>
> This patch introduces a way for backend to keep track
> of the needed information to be able to reconnect without
> frontend cooperation.
>
> It will be used for VDUSE, which does not provide interface
> for the backend to save and later recover local virtqueues
> metadata needed to reconnect.
>
> Vhost-user support could also be added for improved packed
> ring reconnection support.
>
> Signed-off-by: Maxime Coquelin 

vq->last_avail_idx gets updated in other places and I suspect we are
missing some calls to vhost_virtqueue_reconnect_log_split/packed.
I spotted:
- lib/vhost/vhost.c: rte_vhost_set_vring_base()
- lib/vhost/vhost_user.c:  translate_ring_addresses(),
vhost_user_set_vring_base().

The rest lgtm.


-- 
David Marchand

Re: [PATCH 00/33] add Marvell cn20k SOC support for mempool and net

2024-09-23 Thread Jerin Jacob

On Tue, Sep 10, 2024 at 2:54 PM Nithin Dabilpuram
 wrote:
>
> This series adds support for Marvell cn20k SOC for mempool and
> net PMD's.
>
> This series also adds few net/cnxk PMD updates to expose IPsec
> features supported by HW that are very custom in nature and
> some enhancements for cn10k.


# Please update release note for mempool cn20k driver support
# Please update release note for ethdev cn20k driver support
# Please split non cn20k driver patches to separate series.
# Please fix the following build issue


total: 0 errors, 0 warnings
Applying: net/cnxk: add PMD APIs to submit CPT instruction


ccache clang -Idrivers/libtmp_rte_net_cnxk.a.p -Idrivers -I../drivers
-Idrivers/net/cnxk -I../drivers/net/cnxk -Ilib/ethdev -I../lib/ethdev
-I. -I.. -Iconfig -I../config -Ilib/eal/include -I../lib/eal/include
-Ilib/eal/linux/include -I..
/lib/eal/linux/include -Ilib/eal/x86/include -I../lib/eal/x86/include
-Ilib/eal/common -I../lib/eal/common -Ilib/eal -I../lib/eal
-Ilib/kvargs -I../lib/kvargs -Ilib/log -I../lib/log -Ilib/metrics
-I../lib/metrics -Ilib/telemetry -I../lib
/telemetry -Ilib/net -I../lib/net -Ilib/mbuf -I../lib/mbuf
-Ilib/mempool -I../lib/mempool -Ilib/ring -I../lib/ring -Ilib/meter
-I../lib/meter -Idrivers/bus/pci -I../drivers/bus/pci
-I../drivers/bus/pci/linux -Ilib/pci -I../lib/pci -Idriv
ers/bus/vdev -I../drivers/bus/vdev -Ilib/cryptodev -I../lib/cryptodev
-Ilib/rcu -I../lib/rcu -Ilib/eventdev -I../lib/eventdev -Ilib/hash
-I../lib/hash -Ilib/timer -I../lib/timer -Ilib/dmadev -I../lib/dmadev
-Ilib/security -I../lib/securi
ty -Idrivers/common/cnxk -I../drivers/common/cnxk
-Idrivers/mempool/cnxk -I../drivers/mempool/cnxk
-fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch
-Wextra -Werror -std=c11 -O2 -g -include rte_config.h -Wcast-qual -W
deprecated -Wformat -Wformat-nonliteral -Wformat-security
-Wmissing-declarations -Wmissing-prototypes -Wnested-externs
-Wold-style-definition -Wpointer-arith -Wsign-compare
-Wstrict-prototypes -Wundef -Wwrite-strings -Wno-address-of-pack
ed-member -Wno-missing-field-initializers -D_GNU_SOURCE -fPIC
-march=native -mrtm -DALLOW_EXPERIMENTAL_API -DALLOW_INTERNAL_API
-Wno-format-truncation -flax-vector-conversions -Wno-strict-aliasing
-Wno-asm-operand-widths -DRTE_LOG_DEFAUL
T_LOGTYPE=pmd.net.cnxk -MD -MQ
drivers/libtmp_rte_net_cnxk.a.p/net_cnxk_cn20k_ethdev.c.o -MF
drivers/libtmp_rte_net_cnxk.a.p/net_cnxk_cn20k_ethdev.c.o.d -o
drivers/libtmp_rte_net_cnxk.a.p/net_cnxk_cn20k_ethdev.c.o -c
../drivers/net/cnxk/
cn20k_ethdev.c
In file included from ../drivers/net/cnxk/cn20k_ethdev.c:4:
In file included from ../drivers/net/cnxk/cn20k_ethdev.h:8:
../drivers/net/cnxk/cnxk_ethdev.h:516:8: error: address argument to
atomic operation must be a pointer to _Atomic type ('int32_t *' (aka
'int *') invalid)
  516 | val = rte_atomic_fetch_sub_explicit(fc_sw, nb_inst,
__ATOMIC_RELAXED) - nb_inst;
  |   ^ ~
../lib/eal/include/rte_stdatomic.h:95:2: note: expanded from macro
'rte_atomic_fetch_sub_explicit'
   95 | atomic_fetch_sub_explicit(ptr, val, memorder)
  | ^ ~~~
/usr/lib/clang/18/include/stdatomic.h:154:35: note: expanded from
macro 'atomic_fetch_sub_explicit'
  154 | #define atomic_fetch_sub_explicit __c11_atomic_fetch_sub
  |   ^
In file included from ../drivers/net/cnxk/cn20k_ethdev.c:4:
In file included from ../drivers/net/cnxk/cn20k_ethdev.h:8:
../drivers/net/cnxk/cnxk_ethdev.h:520:30: error: address argument to
atomic operation must be a pointer to _Atomic type ('uint64_t *' (aka
'unsigned long *') invalid)
  520 | newval = (int64_t)nb_desc -
rte_atomic_load_explicit(fc, __ATOMIC_RELAXED);
  | ^~~
../lib/eal/include/rte_stdatomic.h:73:2: note: expanded from macro
'rte_atomic_load_explicit'
   73 | atomic_load_explicit(ptr, memorder)
  | ^~~~
/usr/lib/clang/18/include/stdatomic.h:139:30: note: expanded from
macro 'atomic_load_explicit'
  139 | #define atomic_load_explicit __c11_atomic_load
  |  ^
In file included from ../drivers/net/cnxk/cn20k_ethdev.c:4:
In file included from ../drivers/net/cnxk/cn20k_ethdev.h:8:
../drivers/net/cnxk/cnxk_ethdev.h:523:7: error: address argument to
atomic operation must be a pointer to _Atomic type ('int32_t *' (aka
'int *') invalid)
  523 | if
(!rte_atomic_compare_exchange_strong_explicit(fc_sw, &val, newval,
__ATOMIC_RELEASE,
  |  ^   ~
../lib/eal/include/rte_stdatomic.h:83:2: note: expanded from macro
'rte_atomic_compare_exchange_strong_explicit'
   83 | atomic_compare_exchange_strong_explicit(ptr, expected,
desired, \
  | ^   ~~~
/usr/lib/clang/18/include/stdatomic.h:145:49: note: expanded from
macro 'atomic_compare_e

[PATCH v1] ethdev: fix int overflow in descriptor count logic

2024-09-23 Thread Niall Meade

Addressed a specific overflow issue in the eth_dev_adjust_nb_desc()
function where the uint16_t variable nb_desc would overflow when its
value was greater than (2^16 - nb_align). This overflow caused nb_desc
to incorrectly wrap around between 0 and nb_align-1, leading to the
function setting nb_desc to nb_min instead of the expected nb_max.

The resolution involves upcasting nb_desc to a uint32_t before the
RTE_ALIGN_CEIL macro is applied. This change ensures that the subsequent
call to RTE_ALIGN_FLOOR(nb_desc + (nb_align - 1), nb_align) does not
result in an overflow, as it would when nb_desc is a uint16_t. By using
a uint32_t for these operations, the correct behavior is maintained
without the risk of overflow.

Fixes: 0f67fc3baeb9 ("ethdev: add function to adjust number of descriptors")

Signed-off-by: Niall Meade 
---
 .mailmap|  1 +
 lib/ethdev/rte_ethdev.c | 12 +---
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/.mailmap b/.mailmap
index 4a508bafad..c1941e78bb 100644
--- a/.mailmap
+++ b/.mailmap
@@ -1053,6 +1053,7 @@ Nelson Escobar 
 Nemanja Marjanovic 
 Netanel Belgazal 
 Netanel Gonen 
+Niall Meade 
 Niall Power 
 Nicholas Pratte 
 Nick Connolly  
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index f1c658f49e..f978283edf 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -6577,13 +6577,19 @@ static void
 eth_dev_adjust_nb_desc(uint16_t *nb_desc,
const struct rte_eth_desc_lim *desc_lim)
 {
+   /* Upcast to uint32 to avoid potential overflow with RTE_ALIGN_CEIL(). 
*/
+   uint32_t nb_desc_32 = *nb_desc;
+
if (desc_lim->nb_align != 0)
-   *nb_desc = RTE_ALIGN_CEIL(*nb_desc, desc_lim->nb_align);
+   nb_desc_32 = RTE_ALIGN_CEIL(nb_desc_32, desc_lim->nb_align);
 
if (desc_lim->nb_max != 0)
-   *nb_desc = RTE_MIN(*nb_desc, desc_lim->nb_max);
+   nb_desc_32 = RTE_MIN(nb_desc_32, desc_lim->nb_max);
+
+   nb_desc_32 = RTE_MAX(nb_desc_32, desc_lim->nb_min);
 
-   *nb_desc = RTE_MAX(*nb_desc, desc_lim->nb_min);
+   /* Assign clipped u32 back to u16. */
+   *nb_desc = nb_desc_32;
 }
 
 int
-- 
2.34.1

--
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.

Re: [PATCH v3] ethdev: optimize the activation of fast-path tracepoints

2024-09-23 Thread Adel Belkhiri

Thanks to all of you. Much appreciated!

On Sat, Sep 21, 2024 at 9:07 PM Ferruh Yigit  wrote:

> On 9/19/2024 5:37 PM, Jerin Jacob wrote:
> > On Thu, Sep 19, 2024 at 12:16 AM Adel Belkhiri 
> wrote:
> >>
> >> From: Adel Belkhiri 
> >>
> >> Split the tracepoints rte_ethdev_trace_rx_burst and
> >> rte_eth_trace_call_rx_callbacks into two separate ones
> >> for empty and non-empty calls to avoid saturating
> >> quickly the trace buffer.
> >>
> >> Signed-off-by: Adel Belkhiri 
> >
> > Acked-by: Jerin Jacob 
> >
>
> Applied to dpdk-next-net/main, thanks.
>

Re: [PATCH v2] common/cnxk: unregister IRQ before reconfigure

2024-09-23 Thread Jerin Jacob

On Thu, Sep 19, 2024 at 11:33 PM  wrote:
>
> From: Pavan Nikhilesh 
>
> Unregister SSO device and NPA IRQs before resizing
> them.
>
> Signed-off-by: Pavan Nikhilesh 


# Add fixes: tag
# Change commit message to common/cnxk: fix...


> ---
> v2 Changes:
> - Reorder npa interrupt un-registration.
>
>  drivers/common/cnxk/roc_dev.c  | 16 +++-
>  drivers/common/cnxk/roc_dev_priv.h |  2 ++
>  drivers/common/cnxk/roc_sso.c  |  7 +++
>  3 files changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/common/cnxk/roc_dev.c b/drivers/common/cnxk/roc_dev.c
> index daf7684d8e..4a5787e7f9 100644
> --- a/drivers/common/cnxk/roc_dev.c
> +++ b/drivers/common/cnxk/roc_dev.c
> @@ -1017,8 +1017,8 @@ mbox_unregister_vf_irq(struct plt_pci_device *pci_dev, 
> struct dev *dev)
>RVU_VF_INT_VEC_MBOX);
>  }
>
> -static void
> -mbox_unregister_irq(struct plt_pci_device *pci_dev, struct dev *dev)
> +void
> +dev_mbox_unregister_irq(struct plt_pci_device *pci_dev, struct dev *dev)
>  {
> if (dev_is_vf(dev))
> mbox_unregister_vf_irq(pci_dev, dev);
> @@ -1096,8 +1096,8 @@ roc_pf_vf_flr_irq(void *param)
> }
>  }
>
> -static int
> -vf_flr_unregister_irqs(struct plt_pci_device *pci_dev, struct dev *dev)
> +void
> +dev_vf_flr_unregister_irqs(struct plt_pci_device *pci_dev, struct dev *dev)
>  {
> struct plt_intr_handle *intr_handle = pci_dev->intr_handle;
> int i;
> @@ -1113,8 +1113,6 @@ vf_flr_unregister_irqs(struct plt_pci_device *pci_dev, 
> struct dev *dev)
>
> dev_irq_unregister(intr_handle, roc_pf_vf_flr_irq, dev,
>RVU_PF_INT_VEC_VFFLR1);
> -
> -   return 0;
>  }
>
>  int
> @@ -1600,7 +1598,7 @@ dev_init(struct dev *dev, struct plt_pci_device 
> *pci_dev)
>  iounmap:
> dev_vf_mbase_put(pci_dev, vf_mbase);
>  mbox_unregister:
> -   mbox_unregister_irq(pci_dev, dev);
> +   dev_mbox_unregister_irq(pci_dev, dev);
> if (dev->ops)
> plt_free(dev->ops);
>  mbox_fini:
> @@ -1636,10 +1634,10 @@ dev_fini(struct dev *dev, struct plt_pci_device 
> *pci_dev)
> if (dev->lmt_mz)
> plt_memzone_free(dev->lmt_mz);
>
> -   mbox_unregister_irq(pci_dev, dev);
> +   dev_mbox_unregister_irq(pci_dev, dev);
>
> if (!dev_is_vf(dev))
> -   vf_flr_unregister_irqs(pci_dev, dev);
> +   dev_vf_flr_unregister_irqs(pci_dev, dev);
> /* Release PF - VF */
> mbox = &dev->mbox_vfpf;
> if (mbox->hwbase && mbox->dev)
> diff --git a/drivers/common/cnxk/roc_dev_priv.h 
> b/drivers/common/cnxk/roc_dev_priv.h
> index 50e12cbf17..df7dc222ed 100644
> --- a/drivers/common/cnxk/roc_dev_priv.h
> +++ b/drivers/common/cnxk/roc_dev_priv.h
> @@ -131,6 +131,8 @@ int dev_irqs_disable(struct plt_intr_handle *intr_handle);
>  int dev_irq_reconfigure(struct plt_intr_handle *intr_handle, uint16_t 
> max_intr);
>
>  int dev_mbox_register_irq(struct plt_pci_device *pci_dev, struct dev *dev);
> +void dev_mbox_unregister_irq(struct plt_pci_device *pci_dev, struct dev 
> *dev);
>  int dev_vf_flr_register_irqs(struct plt_pci_device *pci_dev, struct dev 
> *dev);
> +void dev_vf_flr_unregister_irqs(struct plt_pci_device *pci_dev, struct dev 
> *dev);
>
>  #endif /* _ROC_DEV_PRIV_H */
> diff --git a/drivers/common/cnxk/roc_sso.c b/drivers/common/cnxk/roc_sso.c
> index 293b0c81a1..7d45b06dda 100644
> --- a/drivers/common/cnxk/roc_sso.c
> +++ b/drivers/common/cnxk/roc_sso.c
> @@ -842,7 +842,14 @@ sso_update_msix_vec_count(struct roc_sso *roc_sso, 
> uint16_t sso_vec_cnt)
> return dev_irq_reconfigure(pci_dev->intr_handle, mbox_vec_cnt 
> + npa_vec_cnt);
> }
>
> +   /* Before re-configuring unregister irqs */
> npa_vec_cnt = (dev->npa.pci_dev == pci_dev) ? NPA_LF_INT_VEC_POISON + 
> 1 : 0;
> +   if (npa_vec_cnt)
> +   npa_unregister_irqs(&dev->npa);
> +
> +   dev_mbox_unregister_irq(pci_dev, dev);
> +   if (!dev_is_vf(dev))
> +   dev_vf_flr_unregister_irqs(pci_dev, dev);
>
> /* Re-configure to include SSO vectors */
> rc = dev_irq_reconfigure(pci_dev->intr_handle, mbox_vec_cnt + 
> npa_vec_cnt + sso_vec_cnt);
> --
> 2.25.1
>

62 matches

Mail list logo