Re: [PATCH v5 1/2] ethdev: Add link_speed lanes support

[Ferruh Yigit]

On 9/24/2024 11:59 PM, Damodharam Ammepalli wrote:
> On Sun, Sep 22, 2024 at 10:02 AM Ferruh Yigit  wrote:
>>
>> On 9/4/2024 6:50 PM, Damodharam Ammepalli wrote:
>>> Update the eth_dev_ops structure with new function vectors
>>> to get, get capabilities and set ethernet link speed lanes.
>>> Update the testpmd to provide required config and information
>>> display infrastructure.
>>>
>>> The supporting ethernet controller driver will register callbacks
>>> to avail link speed lanes config and get services. This lanes
>>> configuration is applicable only when the nic is forced to fixed
>>> speeds. In Autonegiation mode, the hardware automatically
>>> negotiates the number of lanes.
>>>
>>> These are the new commands.
>>>
>>> testpmd> show port 0 speed_lanes capabilities
>>>
>>>  Supported speeds Valid lanes
>>> ---
>>>  10 Gbps  1
>>>  25 Gbps  1
>>>  40 Gbps  4
>>>  50 Gbps  1 2
>>>  100 Gbps 1 2 4
>>>  200 Gbps 2 4
>>>  400 Gbps 4 8
>>> testpmd>
>>>
>>> testpmd>
>>> testpmd> port stop 0
>>> testpmd> port config 0 speed_lanes 4
>>> testpmd> port config 0 speed 20 duplex full
>>> testpmd> port start 0
>>> testpmd>
>>> testpmd> show port info 0
>>>
>>> * Infos for port 0  *
>>> MAC address: 14:23:F2:C3:BA:D2
>>> Device name: :b1:00.0
>>> Driver name: net_bnxt
>>> Firmware-version: 228.9.115.0
>>> Connect to socket: 2
>>> memory allocation on the socket: 2
>>> Link status: up
>>> Link speed: 200 Gbps
>>> Active Lanes: 4
>>> Link duplex: full-duplex
>>> Autoneg status: Off
>>>
>>> Signed-off-by: Damodharam Ammepalli 
>>> ---
>>>  app/test-pmd/cmdline.c | 248 -
>>>  app/test-pmd/config.c  |   4 +
>>>  lib/ethdev/ethdev_driver.h |  91 ++
>>>  lib/ethdev/rte_ethdev.c|  52 
>>>  lib/ethdev/rte_ethdev.h|  95 ++
>>>  lib/ethdev/version.map |   5 +
>>>  6 files changed, 494 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
>>> index b7759e38a8..643102032e 100644
>>> --- a/app/test-pmd/cmdline.c
>>> +++ b/app/test-pmd/cmdline.c
>>> @@ -284,6 +284,9 @@ static void cmd_help_long_parsed(void *parsed_result,
>>>
>>>   "dump_log_types\n"
>>>   "Dumps the log level for all the dpdk modules\n\n"
>>> +
>>> + "show port (port_id) speed_lanes capabilities"
>>> + "   Show speed lanes capabilities of a port.\n\n"
>>>   );
>>>   }
>>>
>>> @@ -823,6 +826,9 @@ static void cmd_help_long_parsed(void *parsed_result,
>>>   "port config (port_id) txq (queue_id) affinity 
>>> (value)\n"
>>>   "Map a Tx queue with an aggregated port "
>>>   "of the DPDK port\n\n"
>>> +
>>> + "port config (port_id|all) speed_lanes (0|1|4|8)\n"
>>>
>>
>> This help string, and the implementation, implies there has to be fixed
>> lane values, like 1, 4, 8. Why not leave this part to the capability
>> reporting, and not limit (and worry) those values in the command, so
>> from command's perspective it can be only .
>>
> 
> ok, will update the help string to 
> 
>>> + "Set number of lanes for all ports or port_id for 
>>> a forced speed\n\n"
>>>   );
>>>   }
>>>
>>> @@ -1560,6 +1566,244 @@ static cmdline_parse_inst_t 
>>> cmd_config_speed_specific = {
>>>   },
>>>  };
>>>
>>> +static int
>>> +parse_speed_lanes_cfg(portid_t pid, uint32_t lanes)
>>> +{
>>> + int ret;
>>> +
>>> + ret = rte_eth_speed_lanes_set(pid, lanes);
>>>
>>
>> As a sample application usage, I think it is better if it gets the
>> capability and verify that 'lanes' is withing the capability and later
>> set it, what do you think?
>>
>> <...>
> 
> Makes sense, will try out and get back to you soon.
> 
> 
>>> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h> index
>> 548fada1c7..9444e0a836 100644
>>> --- a/lib/ethdev/rte_ethdev.h
>>> +++ b/lib/ethdev/rte_ethdev.h
>>> @@ -357,6 +357,27 @@ struct rte_eth_link {
>>>  #define RTE_ETH_LINK_MAX_STR_LEN 40 /**< Max length of default link 
>>> string. */
>>>  /**@}*/
>>>
>>> +/**
>>> + * Constants used to indicate the possible link speed lanes of an ethdev 
>>> port.
>>> + */
>>> +#define RTE_ETH_SPEED_LANE_UNKNOWN  0  /**< speed lanes unsupported mode 
>>> or default */
>>> +#define RTE_ETH_SPEED_LANE_1  1/**< Link speed lane  1 */
>>> +#define RTE_ETH_SPEED_LANE_2  2/**< Link speed lanes 2 */
>>> +#define RTE_ETH_SPEED_LANE_4  4/**< Link speed lanes 4 */
>>> +#define RTE_ETH_SPEED_LANE_8  8/**< Link speed lanes 8 */
>>> +
>>> +/* Translate from link speed lanes to speed lanes capa */
>>> +#define RTE_ETH_SPEED_LANES_TO_CAPA(x) RTE_BIT32(x)
>>>

[RFC PATCH v8] mempool: fix mempool cache size

[Morten Brørup]

This patch refactors the mempool cache to fix two bugs:
1. When a mempool is created with a cache size of N objects, the cache was
actually created with a size of 1.5 * N objects.
2. The mempool cache field names did not reflect their purpose;
the "flushthresh" field held the size, and the "size" field held the
number of objects remaining in the cache when returning from a get
operation refilling it from the backend.

Especially the first item could be fatal:
When more objects than a mempool's configured cache size is held in the
mempool's caches associated with other lcores, a rightsized mempool may
unexpectedly run out of objects, causing the application to fail.

Furthermore, this patch introduces two optimizations:
1. The mempool caches are flushed to/filled from the backend in their
entirety, so backend accesses are CPU cache line aligned. (Assuming the
mempool cache size is a multiplum of a CPU cache line size divided by the
size of a pointer.)
2. The unlikely paths in the get and put functions, where the cache is
flushed to/filled from the backend, are moved from the inline functions to
separate helper functions, thereby reducing the code size of the inline
functions.
Note: Accessing the backend for cacheless mempools remains inline.

Various drivers accessing the mempool directly have been updated
accordingly.
These drivers did not update mempool statistics when accessing the mempool
directly, so that is fixed too.

Note: Performance not yet benchmarked.

Signed-off-by: Morten Brørup 
---
v8:
* Rewrote rte_mempool_do_generic_put() to get rid of transaction
  splitting. Use a method similar to the existing put method with fill
  followed by flush if overfilled.
  This alo made rte_mempool_do_generic_put_split() obsolete.
v7:
* Increased max mempool cache size from 512 to 1024 objects.
  Mainly for CI performance test purposes.
  Originally, the max mempool cache size was 768 objects, and used a fixed
  size array of 1024 objects in the mempool cache structure.
v6:
* Fix v5 incomplete implementation of passing large requests directly to
  the backend.
* Use memcpy instead of rte_memcpy where compiler complains about it.
* Added const to some function parameters.
v5:
* Moved helper functions back into the header file, for improved
  performance.
* Pass large requests directly to the backend. This also simplifies the
  code.
v4:
* Updated subject to reflect that misleading names are considered bugs.
* Rewrote patch description to provide more details about the bugs fixed.
  (Mattias Rönnblom)
* Moved helper functions, not to be inlined, to mempool C file.
  (Mattias Rönnblom)
* Pass requests for n >= RTE_MEMPOOL_CACHE_MAX_SIZE objects known at build
  time directly to backend driver, to avoid calling the helper functions.
  This also fixes the compiler warnings about out of bounds array access.
v3:
* Removed __attribute__(assume).
v2:
* Removed mempool perf test; not part of patch set.
---
 drivers/common/idpf/idpf_common_rxtx_avx512.c |  54 ++--
 drivers/mempool/dpaa/dpaa_mempool.c   |  16 +-
 drivers/mempool/dpaa2/dpaa2_hw_mempool.c  |  14 -
 drivers/net/i40e/i40e_rxtx_vec_avx512.c   |  17 +-
 drivers/net/iavf/iavf_rxtx_vec_avx512.c   |  27 +-
 drivers/net/ice/ice_rxtx_vec_avx512.c |  27 +-
 lib/mempool/mempool_trace.h   |   1 -
 lib/mempool/rte_mempool.c |  12 +-
 lib/mempool/rte_mempool.h | 250 +++---
 9 files changed, 196 insertions(+), 222 deletions(-)

diff --git a/drivers/common/idpf/idpf_common_rxtx_avx512.c 
b/drivers/common/idpf/idpf_common_rxtx_avx512.c
index 3b5e124ec8..98535a48f3 100644
--- a/drivers/common/idpf/idpf_common_rxtx_avx512.c
+++ b/drivers/common/idpf/idpf_common_rxtx_avx512.c
@@ -1024,21 +1024,13 @@ idpf_tx_singleq_free_bufs_avx512(struct idpf_tx_queue 
*txq)
rte_lcore_id());
void **cache_objs;
 
-   if (cache == NULL || cache->len == 0)
-   goto normal;
-
-   cache_objs = &cache->objs[cache->len];
-
-   if (n > RTE_MEMPOOL_CACHE_MAX_SIZE) {
-   rte_mempool_ops_enqueue_bulk(mp, (void *)txep, n);
+   if (!cache || unlikely(n + cache->len > cache->size)) {
+   rte_mempool_generic_put(mp, (void *)txep, n, cache);
goto done;
}
 
-   /* The cache follows the following algorithm
-*   1. Add the objects to the cache
-*   2. Anything greater than the cache min value (if it 
crosses the
-*   cache flush threshold) is flushed to the ring.
-*/
+   cache_objs = &cache->objs[cache->len];
+
/* Add elements back into the cache */
uint32_t copied = 0;
/* n is multiple of 32 */
@@ -1056,16 +1048,13 @@ idpf_tx_singleq_free_bufs_avx512(struct 

Re: [PATCH v5 1/2] ethdev: Add link_speed lanes support

[Ferruh Yigit]

On 9/25/2024 4:00 PM, Damodharam Ammepalli wrote:
> On Tue, Sep 24, 2024 at 3:59 PM Damodharam Ammepalli
>  wrote:
>>
>> On Sun, Sep 22, 2024 at 10:02 AM Ferruh Yigit  wrote:
>>>
>>> On 9/4/2024 6:50 PM, Damodharam Ammepalli wrote:
 Update the eth_dev_ops structure with new function vectors
 to get, get capabilities and set ethernet link speed lanes.
 Update the testpmd to provide required config and information
 display infrastructure.

 The supporting ethernet controller driver will register callbacks
 to avail link speed lanes config and get services. This lanes
 configuration is applicable only when the nic is forced to fixed
 speeds. In Autonegiation mode, the hardware automatically
 negotiates the number of lanes.

 These are the new commands.

 testpmd> show port 0 speed_lanes capabilities

  Supported speeds Valid lanes
 ---
  10 Gbps  1
  25 Gbps  1
  40 Gbps  4
  50 Gbps  1 2
  100 Gbps 1 2 4
  200 Gbps 2 4
  400 Gbps 4 8
 testpmd>

 testpmd>
 testpmd> port stop 0
 testpmd> port config 0 speed_lanes 4
 testpmd> port config 0 speed 20 duplex full
 testpmd> port start 0
 testpmd>
 testpmd> show port info 0

 * Infos for port 0  *
 MAC address: 14:23:F2:C3:BA:D2
 Device name: :b1:00.0
 Driver name: net_bnxt
 Firmware-version: 228.9.115.0
 Connect to socket: 2
 memory allocation on the socket: 2
 Link status: up
 Link speed: 200 Gbps
 Active Lanes: 4
 Link duplex: full-duplex
 Autoneg status: Off

 Signed-off-by: Damodharam Ammepalli 
 ---
  app/test-pmd/cmdline.c | 248 -
  app/test-pmd/config.c  |   4 +
  lib/ethdev/ethdev_driver.h |  91 ++
  lib/ethdev/rte_ethdev.c|  52 
  lib/ethdev/rte_ethdev.h|  95 ++
  lib/ethdev/version.map |   5 +
  6 files changed, 494 insertions(+), 1 deletion(-)

 diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
 index b7759e38a8..643102032e 100644
 --- a/app/test-pmd/cmdline.c
 +++ b/app/test-pmd/cmdline.c
 @@ -284,6 +284,9 @@ static void cmd_help_long_parsed(void *parsed_result,

   "dump_log_types\n"
   "Dumps the log level for all the dpdk 
 modules\n\n"
 +
 + "show port (port_id) speed_lanes capabilities"
 + "   Show speed lanes capabilities of a port.\n\n"
   );
   }

 @@ -823,6 +826,9 @@ static void cmd_help_long_parsed(void *parsed_result,
   "port config (port_id) txq (queue_id) affinity 
 (value)\n"
   "Map a Tx queue with an aggregated port "
   "of the DPDK port\n\n"
 +
 + "port config (port_id|all) speed_lanes (0|1|4|8)\n"

>>>
>>> This help string, and the implementation, implies there has to be fixed
>>> lane values, like 1, 4, 8. Why not leave this part to the capability
>>> reporting, and not limit (and worry) those values in the command, so
>>> from command's perspective it can be only .
>>>
>>
>> ok, will update the help string to 
>>
 + "Set number of lanes for all ports or port_id 
 for a forced speed\n\n"
   );
   }

 @@ -1560,6 +1566,244 @@ static cmdline_parse_inst_t 
 cmd_config_speed_specific = {
   },
  };

 +static int
 +parse_speed_lanes_cfg(portid_t pid, uint32_t lanes)
 +{
 + int ret;
 +
 + ret = rte_eth_speed_lanes_set(pid, lanes);

>>>
>>> As a sample application usage, I think it is better if it gets the
>>> capability and verify that 'lanes' is withing the capability and later
>>> set it, what do you think?
>>>
>>> <...>
>>
>> Makes sense, will try out and get back to you soon.
>>
> I guess a similar comment was discussed in the past, slipped my mind
> to mention in my last response.
> These are the reasons for this logic.
> -  same lane number can be supported by any speed (eg: 4 Lanes
> supported in 100,200,400 etc)
> so, which speed row should app check against, since the user has not
> configured fixed speed yet.
> If the link is already up in AN mode, this lane config doesn't have
> any significance.
> - one approach would be to validate lanes in
> parse_and_check_speed_duplex(), but in this case, the app should
> already have non-default lanes value. We also need to consider the
> case, if the user wants default lanes.
> - In the ethtool, the driver does the validation of the speed x lanes
> combo. Tried to match similar b

Re: [PATCH v4] net/zxdh: Provided zxdh basic init

[Ferruh Yigit]

On 9/10/2024 1:00 PM, Junlong Wang wrote:
> provided zxdh initialization of zxdh PMD driver.
> include msg channel, np init and etc.
> 

Hi Junlong,

It is very hard to review the driver as a single patch, it helps to
split driver into multiple patches, please check the suggestion on it:
https://patches.dpdk.org/project/dpdk/patch/20240916162856.11566-1-step...@networkplumber.org/

Also there are errors reported by scripts, please fix them:
./devtools/checkpatches.sh -n1
./devtools/check-meson.py


> Signed-off-by: Junlong Wang 
> ---
> V4: Resolve compilation issues
> V3: Resolve compilation issues
> V2: Resolve compilation issues and modify doc(zxdh.ini zdh.rst)
> V1: Provide zxdh basic init and open source NPSDK lib
> ---
>  doc/guides/nics/features/zxdh.ini |   10 +
>  doc/guides/nics/index.rst |1 +
>  doc/guides/nics/zxdh.rst  |   34 +
>  drivers/net/meson.build   |1 +
>  drivers/net/zxdh/meson.build  |   23 +
>  drivers/net/zxdh/zxdh_common.c|   59 ++
>  drivers/net/zxdh/zxdh_common.h|   32 +
>  drivers/net/zxdh/zxdh_ethdev.c| 1328 +
>  drivers/net/zxdh/zxdh_ethdev.h|  202 +
>  drivers/net/zxdh/zxdh_logs.h  |   38 +
>  drivers/net/zxdh/zxdh_msg.c   | 1177 +
>  drivers/net/zxdh/zxdh_msg.h   |  408 +
>  drivers/net/zxdh/zxdh_npsdk.c |  158 
>  drivers/net/zxdh/zxdh_npsdk.h |  216 +
>  drivers/net/zxdh/zxdh_pci.c   |  462 ++
>  drivers/net/zxdh/zxdh_pci.h   |  259 ++
>  drivers/net/zxdh/zxdh_queue.c |  138 +++
>  drivers/net/zxdh/zxdh_queue.h |   85 ++
>  drivers/net/zxdh/zxdh_ring.h  |   87 ++
>  drivers/net/zxdh/zxdh_rxtx.h  |   48 ++
>  20 files changed, 4766 insertions(+)
>  create mode 100644 doc/guides/nics/features/zxdh.ini
>  create mode 100644 doc/guides/nics/zxdh.rst
>  create mode 100644 drivers/net/zxdh/meson.build
>  create mode 100644 drivers/net/zxdh/zxdh_common.c
>  create mode 100644 drivers/net/zxdh/zxdh_common.h
>  create mode 100644 drivers/net/zxdh/zxdh_ethdev.c
>  create mode 100644 drivers/net/zxdh/zxdh_ethdev.h
>  create mode 100644 drivers/net/zxdh/zxdh_logs.h
>  create mode 100644 drivers/net/zxdh/zxdh_msg.c
>  create mode 100644 drivers/net/zxdh/zxdh_msg.h
>  create mode 100644 drivers/net/zxdh/zxdh_npsdk.c
>  create mode 100644 drivers/net/zxdh/zxdh_npsdk.h
>  create mode 100644 drivers/net/zxdh/zxdh_pci.c
>  create mode 100644 drivers/net/zxdh/zxdh_pci.h
>  create mode 100644 drivers/net/zxdh/zxdh_queue.c
>  create mode 100644 drivers/net/zxdh/zxdh_queue.h
>  create mode 100644 drivers/net/zxdh/zxdh_ring.h
>  create mode 100644 drivers/net/zxdh/zxdh_rxtx.h
> 
> diff --git a/doc/guides/nics/features/zxdh.ini b/doc/guides/nics/
> features/zxdh.ini
> new file mode 100644
> index 00..083c75511b
> --- /dev/null
> +++ b/doc/guides/nics/features/zxdh.ini
> @@ -0,0 +1,10 @@
> +;
> +; Supported features of the 'zxdh' network poll mode driver.
> +;
> +; Refer to default.ini for the full list of available PMD features.
> +;
> +[Features]
> +Linux= Y
> +x86-64   = Y
> +ARMv8= Y
> +
> diff --git a/doc/guides/nics/index.rst b/doc/guides/nics/index.rst
> index c14bc7988a..8e371ac4a5 100644
> --- a/doc/guides/nics/index.rst
> +++ b/doc/guides/nics/index.rst
> @@ -69,3 +69,4 @@ Network Interface Controller Drivers
>  vhost
>  virtio
>  vmxnet3
> +zxdh
> diff --git a/doc/guides/nics/zxdh.rst b/doc/guides/nics/zxdh.rst
> new file mode 100644
> index 00..e878058b7b
> --- /dev/null
> +++ b/doc/guides/nics/zxdh.rst
> @@ -0,0 +1,34 @@
> +..  SPDX-License-Identifier: BSD-3-Clause
> +Copyright(c) 2023 ZTE Corporation.
> +
> +ZXDH Poll Mode Driver
> +==
> +
> +The ZXDH PMD (**librte_net_zxdh**) provides poll mode driver support
> +for 25/100 Gbps ZXDH NX Series Ethernet Controller based on
> +the ZTE Ethernet Controller E310/E312.
> +
>

Can you please provide a link for the product?
There is a link in the prerequisetes section below, if that link is for
product please move it here.

> +
> +Features
> +
> +
> +Features of the zxdh PMD are:
> +
> +- Multi arch support: x86_64, ARMv8.
> +
> +Prerequisites
> +-
> +
> +- Learning about ZXDH NX Series Ethernet Controller NICs using
> +  ``_.
> +
> +Driver compilation and testing
> +--
> +
> +Refer to the document :ref:`compiling and testing a PMD for a NIC 
> `
> +for details.
> +
> +Limitations or Known issues
> +---
> +X86-32, Power8, ARMv7 and BSD are not supported yet.
> +
> diff --git a/drivers/net/meson.build b/drivers/net/meson.build
> index fb6d34b782..1a3db8a04d 100644
> --- a/drivers/net/meson.build
> +++ b/drivers/net/meson.build
> @@ -62,6 +62,7 @@ drivers = [
>  'vhost',
>  'virtio',
>  'vmxnet3',
> +'zxdh'

Re: [PATCH] maintainers: fix prog guide paths

[fengchengwen]

Acked-by: Chengwen Feng 

On 2024/9/26 5:22, Thomas Monjalon wrote:
> When moving some files in doc subdirectories for ethdev and eventdev,
> the references in the file MAINTAINERS were forgotten.
> The new paths are used in this fix.
> 
> Fixes: 41dd9a6bc2d9 ("doc: reorganize prog guide")
> 
> Signed-off-by: Thomas Monjalon 

RE: [PATCH 1/2] ethdev: add Rx packet type offload control flag

[Chaoyong He]

> On 9/24/2024 3:03 AM, Chaoyong He wrote:
> >> On 6/19/2024 11:11 AM, Chaoyong He wrote:
> >>> From: Long Wu 
> >>>
> >>> The Rx packet type offload feature may affect the performance, so
> >>> add a control flag for applications to turn it on or off.
> >>>
> >>> Signed-off-by: Long Wu 
> >>> ---
> >>>  lib/ethdev/rte_ethdev.h | 1 +
> >>>  1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h index
> >>> 548fada1c7..be86983e24 100644
> >>> --- a/lib/ethdev/rte_ethdev.h
> >>> +++ b/lib/ethdev/rte_ethdev.h
> >>> @@ -1555,6 +1555,7 @@ struct rte_eth_conf {  #define
> >>> RTE_ETH_RX_OFFLOAD_OUTER_UDP_CKSUM  RTE_BIT64(18)
> >>>  #define RTE_ETH_RX_OFFLOAD_RSS_HASH RTE_BIT64(19)
> >>>  #define RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT RTE_BIT64(20)
> >>> +#define RTE_ETH_RX_OFFLOAD_PTYPES   RTE_BIT64(21)
> >>>
> >>>  #define RTE_ETH_RX_OFFLOAD_CHECKSUM
> >> (RTE_ETH_RX_OFFLOAD_IPV4_CKSUM | \
> >>>RTE_ETH_RX_OFFLOAD_UDP_CKSUM | \
> >>
> >> Hi Chaoyong,
> >>
> >> Instead of having an offload for ptypes, we have APIs for this,
> >>
> >> First one is 'rte_eth_dev_get_supported_ptypes()' that application
> >> can learn the supported packet types.
> >>
> >> Second one is more related to above flag, it is 'rte_eth_dev_set_ptypes()'
> >> which application can set which pytpes is required, it can be set to
> >> disable all packet type parsing, can be similar to not requesting
> >> 'RTE_ETH_RX_OFFLOAD_PTYPES'.
> >>
> >> With above two APIs, do we still need the offload flag?
> >>
> >
> > At present, the purpose of the ops 'rte_eth_dev_set_ptypes()' is to set the
> range of packet types to handle.
> >
> 
> Yes, and setting 'ptype_mask' to zero should disable packet type parsing.
> 
> Packet type parsing is an offload, but when we have an API that has finer
> grained control to what packet type to parse, why not use it instead of having
> offload flag, which is all on or off configuration.
> 
> > Of course, we can maintain a flag for each application and driver
> > based on the return value of this ops; but this is a bit troublesome.
> >
> 
> I didn't get your point, why maintain a flag?
> 
> > So, we hope to follow the example of RSS, in addition to
> > 'rte_eth_dev_rss_hash_update()' and 'rte_eth_dev_rss_hash_conf_get()',
> > we also want to set a flag for the ptype function similar to
> RTE_ETH_RX_OFFLOAD_RSS_HASH.
> >
> >>
> >> Another concern with adding new offload flag is backward
> >> compatibility, all existing drivers that support packet type parsing
> >> should be updated to list this offload flag as capability. Also they
> >> need to be updated to configure packet parsing based on user requested
> offload configuration.
> >>
> >
> > If you agree with this design suggestion, we will adapt all the related 
> > code to
> ptypes for each PMDs and 'test-pmd' applications in the next patch.
> > Do you think this okay?
> >
> >> Briefly, we can't just introduce a new offload flag for an existing
> >> capability and update only one driver, all drivers needs to be updated.

Hi Ferruh,
Thanks for your explanation, we understand what you mean now.

We'll send a new version patch to drop the 'RTE_ETH_RX_OFFLOAD_PTYPES'.

[PATCH] eal/alarm_cancel: Fix thread starvation

[Wojciech Panfil]

Issue:
Two threads:

- A, executing rte_eal_alarm_cancel,
- B, executing eal_alarm_callback.

Such case can cause starvation of thread B. Please see that there is a
small time window between lock and unlock in thread A, so thread B must
be switched to within a very small time window, so that it can obtain
the lock.

Solution to this problem is use sched_yield(), which puts current thread
(A) at the end of thread execution priority queue and allows thread B to
execute.

The issue can be observed e.g. on hot-pluggable device detach path.
On such path, rte_alarm can used to check if DPDK has completed
the detachment. Waiting for completion, rte_eal_alarm_cancel
is called, while another thread periodically calls eal_alarm_callback
causing the issue to occur.

Signed-off-by: Wojciech Panfil 
---
 lib/eal/freebsd/eal_alarm.c | 6 ++
 lib/eal/linux/eal_alarm.c   | 6 ++
 lib/eal/windows/eal_alarm.c | 5 +
 3 files changed, 17 insertions(+)

diff --git a/lib/eal/freebsd/eal_alarm.c b/lib/eal/freebsd/eal_alarm.c
index 94cae5f4b6..3680f5caba 100644
--- a/lib/eal/freebsd/eal_alarm.c
+++ b/lib/eal/freebsd/eal_alarm.c
@@ -318,7 +318,13 @@ rte_eal_alarm_cancel(rte_eal_alarm_callback cb_fn, void 
*cb_arg)
}
ap_prev = ap;
}
+
rte_spinlock_unlock(&alarm_list_lk);
+
+   /* Yield control to a second thread executing 
eal_alarm_callback to avoid
+* its starvation, as it is waiting for the lock we have just 
released.
+*/
+   sched_yield();
} while (executing != 0);
 
if (count == 0 && err == 0)
diff --git a/lib/eal/linux/eal_alarm.c b/lib/eal/linux/eal_alarm.c
index eeb096213b..9fe14ade63 100644
--- a/lib/eal/linux/eal_alarm.c
+++ b/lib/eal/linux/eal_alarm.c
@@ -248,7 +248,13 @@ rte_eal_alarm_cancel(rte_eal_alarm_callback cb_fn, void 
*cb_arg)
}
ap_prev = ap;
}
+
rte_spinlock_unlock(&alarm_list_lk);
+
+   /* Yield control to a second thread executing 
eal_alarm_callback to avoid
+* its starvation, as it is waiting for the lock we have just 
released.
+*/
+   sched_yield();
} while (executing != 0);
 
if (count == 0 && err == 0)
diff --git a/lib/eal/windows/eal_alarm.c b/lib/eal/windows/eal_alarm.c
index 052af4b21b..9ad530dd31 100644
--- a/lib/eal/windows/eal_alarm.c
+++ b/lib/eal/windows/eal_alarm.c
@@ -211,6 +211,11 @@ rte_eal_alarm_cancel(rte_eal_alarm_callback cb_fn, void 
*cb_arg)
}
 
rte_spinlock_unlock(&alarm_lock);
+
+   /* Yield control to a second thread executing 
eal_alarm_callback to avoid
+* its starvation, as it is waiting for the lock we have just 
released.
+*/
+   SwitchToThread();
} while (executing);
 
rte_eal_trace_alarm_cancel(cb_fn, cb_arg, removed);
-- 
2.46.0

Re: [PATCH 1/2] ethdev: add Rx packet type offload control flag

[Ferruh Yigit]

On 9/24/2024 3:03 AM, Chaoyong He wrote:
>> On 6/19/2024 11:11 AM, Chaoyong He wrote:
>>> From: Long Wu 
>>>
>>> The Rx packet type offload feature may affect the performance, so add
>>> a control flag for applications to turn it on or off.
>>>
>>> Signed-off-by: Long Wu 
>>> ---
>>>  lib/ethdev/rte_ethdev.h | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h index
>>> 548fada1c7..be86983e24 100644
>>> --- a/lib/ethdev/rte_ethdev.h
>>> +++ b/lib/ethdev/rte_ethdev.h
>>> @@ -1555,6 +1555,7 @@ struct rte_eth_conf {  #define
>>> RTE_ETH_RX_OFFLOAD_OUTER_UDP_CKSUM  RTE_BIT64(18)
>>>  #define RTE_ETH_RX_OFFLOAD_RSS_HASH RTE_BIT64(19)
>>>  #define RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT RTE_BIT64(20)
>>> +#define RTE_ETH_RX_OFFLOAD_PTYPES   RTE_BIT64(21)
>>>
>>>  #define RTE_ETH_RX_OFFLOAD_CHECKSUM
>> (RTE_ETH_RX_OFFLOAD_IPV4_CKSUM | \
>>>  RTE_ETH_RX_OFFLOAD_UDP_CKSUM | \
>>
>> Hi Chaoyong,
>>
>> Instead of having an offload for ptypes, we have APIs for this,
>>
>> First one is 'rte_eth_dev_get_supported_ptypes()' that application can learn
>> the supported packet types.
>>
>> Second one is more related to above flag, it is 'rte_eth_dev_set_ptypes()'
>> which application can set which pytpes is required, it can be set to disable 
>> all
>> packet type parsing, can be similar to not requesting
>> 'RTE_ETH_RX_OFFLOAD_PTYPES'.
>>
>> With above two APIs, do we still need the offload flag?
>>
> 
> At present, the purpose of the ops 'rte_eth_dev_set_ptypes()' is to set the 
> range of packet types to handle.
>

Yes, and setting 'ptype_mask' to zero should disable packet type parsing.

Packet type parsing is an offload, but when we have an API that has
finer grained control to what packet type to parse, why not use it
instead of having offload flag, which is all on or off configuration.

> Of course, we can maintain a flag for each application and driver based on 
> the return value of this ops;
> but this is a bit troublesome.
>

I didn't get your point, why maintain a flag?

> So, we hope to follow the example of RSS, in addition to
> 'rte_eth_dev_rss_hash_update()' and 'rte_eth_dev_rss_hash_conf_get()', we 
> also want to set a flag for
> the ptype function similar to RTE_ETH_RX_OFFLOAD_RSS_HASH.
> 
>>
>> Another concern with adding new offload flag is backward compatibility, all
>> existing drivers that support packet type parsing should be updated to list 
>> this
>> offload flag as capability. Also they need to be updated to configure packet
>> parsing based on user requested offload configuration.
>>
> 
> If you agree with this design suggestion, we will adapt all the related code 
> to ptypes for each PMDs and 'test-pmd' applications in the next patch.
> Do you think this okay?
> 
>> Briefly, we can't just introduce a new offload flag for an existing 
>> capability and
>> update only one driver, all drivers needs to be updated.

Re: [PATCH v5 00/10] support reinit flow

[Stephen Hemminger]

On Thu, 17 Aug 2023 00:28:11 -0400
ok...@kernel.org wrote:

> From: Sinan Kaya 
> 
> We want to be able to call rte_eal_init() and rte_eal_cleanup()
> APIs back to back for maintanance reasons.
> 
> Here is a summary of the code we have seen so far:
> 
> 1. some code support getting called multiple times by keeping
> a static variable.
> 2. some code initializes once but never clean up after them and
> don't have a cleanup API.
> 3. some code assumes that they only get called once during the
> lifecycle of the process.
> 
> Most changes in this patch center around following the #1 design
> principle.
> 
> Why?
> 
> It is not always ideal to reinitialize a DPDK process. Memory needs
> to be reinitialized, hugetables need to warm up etc.
> 
> Limitations:
> 
> This sequence could only be done by main lcore, and never ever in a signal 
> handler.
> Do not try and trap signals like abort, bus error, illegal instruction and 
> try to
> use this for recovery. It is a recipe for failure.
> 

This patch series suffers bit rot and does not apply anymore. Needs to be rebase
and resubmit.

There probably needs to be more unit-tests for restart.
Also some documentation for example, for which NIC's does this work?
Probably not all.

Re: [PATCH v8 8/8] net/hns3: support filter registers by module names

[fengchengwen]

On 2024/9/25 14:40, Jie Hai wrote:
> This patch support dumping registers which module name is the
> `filter` string. The module names are in lower case and so is
> the `filter`. Available module names are cmdq, common_pf,
> common_vf, ring, tqp_intr, 32_bit_dfx, 64_bit_dfx, bios, igu_egu,
> ssu, ppp, rpu, ncsi, rtc, rcb, etc.
> 
> Signed-off-by: Jie Hai 
> ---
>  doc/guides/nics/hns3.rst |   7 +
>  drivers/net/hns3/hns3_regs.c | 257 ---
>  2 files changed, 157 insertions(+), 107 deletions(-)
> 
> diff --git a/doc/guides/nics/hns3.rst b/doc/guides/nics/hns3.rst
> index 97b4686bfabe..20765bd7d145 100644
> --- a/doc/guides/nics/hns3.rst
> +++ b/doc/guides/nics/hns3.rst
> @@ -407,6 +407,13 @@ be provided to avoid scheduling the CPU core used by 
> DPDK application threads fo
>  other tasks. Before starting the Linux OS, add the kernel isolation boot 
> parameter.
>  For example, "isolcpus=1-18 nohz_full=1-18 rcu_nocbs=1-18".
>  
> +Dump registers
> +--
> +HNS3 supports dumping registers values with their names, and suppotrt 
> filtering

typo of suppotrt

> +by module names. The available modules are ``bios``, ``ssu``, ``igu_egu``,

The available modules names?

> +``rpu``, ``ncsi``, ``rtc``, ``ppp``, ``rcb``, ``tqp``, ``rtc``, ``cmdq``,
> +``common_pf``, ``common_vf``, ``ring``, ``tqp_intr``, ``32_bit_dfx``,
> +``64_bit_dfx``, etc.

etc means there are more which don't list above, please don't add it because 
the doc should contain all modules names.

>  
>  Limitations or Known issues
>  ---
> diff --git a/drivers/net/hns3/hns3_regs.c b/drivers/net/hns3/hns3_regs.c
> index a0d130302839..2e19add21265 100644
> --- a/drivers/net/hns3/hns3_regs.c
> +++ b/drivers/net/hns3/hns3_regs.c
> @@ -12,7 +12,7 @@
>  
>  #define HNS3_64_BIT_REG_OUTPUT_SIZE (sizeof(uint64_t) / sizeof(uint32_t))
>  
> -static int hns3_get_dfx_reg_cnt(struct hns3_hw *hw, uint32_t *count);
> +#define HNS3_MAX_MODULES_LEN 512
>  
>  struct hns3_dirt_reg_entry {
>   const char *name;
> @@ -795,11 +795,39 @@ enum hns3_reg_modules {
>   HNS3_64_BIT_DFX,
>  };
>  
> +#define HNS3_MODULE_MASK(x) RTE_BIT32(x)
> +#define HNS3_VF_MODULES (HNS3_MODULE_MASK(HNS3_CMDQ) | 
> HNS3_MODULE_MASK(HNS3_COMMON_VF) | \
> +  HNS3_MODULE_MASK(HNS3_RING) | 
> HNS3_MODULE_MASK(HNS3_TQP_INTR))
> +#define HNS3_VF_ONLY_MODULES HNS3_MODULE_MASK(HNS3_COMMON_VF)
> +
>  struct hns3_reg_list {
>   const void *reg_list;
>   uint32_t entry_num;
>  };
>  
> +struct {
> + const char *name;
> + uint32_t module;
> +} hns3_module_name_map[] = {
> + { "bios",   HNS3_MODULE_MASK(HNS3_BIOS_COMMON) },
> + { "ssu",HNS3_MODULE_MASK(HNS3_SSU_0) | 
> HNS3_MODULE_MASK(HNS3_SSU_1) |
> + HNS3_MODULE_MASK(HNS3_SSU_2) },
> + { "igu_egu",HNS3_MODULE_MASK(HNS3_IGU_EGU) },
> + { "rpu",HNS3_MODULE_MASK(HNS3_RPU_0) | 
> HNS3_MODULE_MASK(HNS3_RPU_1) },
> + { "ncsi",   HNS3_MODULE_MASK(HNS3_NCSI) },
> + { "rtc",HNS3_MODULE_MASK(HNS3_RTC) },
> + { "ppp",HNS3_MODULE_MASK(HNS3_PPP) },
> + { "rcb",HNS3_MODULE_MASK(HNS3_RCB) },
> + { "tqp",HNS3_MODULE_MASK(HNS3_TQP) },
> + { "cmdq",   HNS3_MODULE_MASK(HNS3_CMDQ) },
> + { "common_pf",  HNS3_MODULE_MASK(HNS3_COMMON_PF) },
> + { "common_vf",  HNS3_MODULE_MASK(HNS3_COMMON_VF) },
> + { "ring",   HNS3_MODULE_MASK(HNS3_RING) },
> + { "tqp_intr",   HNS3_MODULE_MASK(HNS3_TQP_INTR) },
> + { "32_bit_dfx", HNS3_MODULE_MASK(HNS3_32_BIT_DFX) },
> + { "64_bit_dfx", HNS3_MODULE_MASK(HNS3_64_BIT_DFX) },
> +};
> +
>  static struct hns3_reg_list hns3_reg_lists[] = {
>   [HNS3_BIOS_COMMON]  = { dfx_bios_common_reg_list,   
> RTE_DIM(dfx_bios_common_reg_list) },
>   [HNS3_SSU_0]= { dfx_ssu_reg_0_list, 
> RTE_DIM(dfx_ssu_reg_0_list) },
> @@ -863,21 +891,58 @@ hns3_get_regs_num(struct hns3_hw *hw, uint32_t 
> *regs_num_32_bit,
>   return 0;
>  }
>  
> -static int
> -hns3_get_32_64_regs_cnt(struct hns3_hw *hw, uint32_t *count)
> +static const char *
> +hns3_get_name_by_module(enum hns3_reg_modules module)
>  {
> - uint32_t regs_num_32_bit, regs_num_64_bit;
> - int ret;
> + size_t i;
>  
> - ret = hns3_get_regs_num(hw, ®s_num_32_bit, ®s_num_64_bit);
> - if (ret) {
> - hns3_err(hw, "fail to get the number of registers, "
> -  "ret = %d.", ret);
> - return ret;
> + for (i = 0; i < RTE_DIM(hns3_module_name_map); i++) {
> + if (hns3_module_name_map[i].module && HNS3_MODULE_MASK(module) 
> != 0)
> + return hns3_module_name_map[i].name;
>   }
> + return "unknown";
> +}
>  
> - *count += regs_num_32_bit + regs_num_64_bit * 
> HNS3_64_BIT_REG_OUTPUT_SIZE;
> - return 0;
> +static void
> +hns3_get_module_names(char *names, uint32_t len)
> +{
> + size_t i;
> +
> + for (i =

[PATCH] app/test: refactor cryptodev test cases

[Arkadiusz Kusztal]

This commit introduces several changes to the cryptodev
test cases that should make it easier to maintain.

Changes included in this patch:
- If not needed by the specific test case, the device should be
started/stopped in the particular testsuite setup/teardown
function.
- Most of the remaining test vectors were moved from test.c file
to the specific header vector files.
- Part of GCM redundant functions were replaced by named test cases.
- Unit tests do not need to check for the symmetric cryptography feature,
if this feature were not present, the test should not even reach this stage.

Signed-off-by: Arkadiusz Kusztal 
---
Please note that this is work in progress, what is left to be done:
- Rework security test cases, these will fail with current setup.
- Fix OOP issue. OOP tests do not check for the prepended data in the OOP 
buffer.
- Remove remaining test vectors from the .c file.
- Remove redundant test functions that call common function, replace with named 
test cases.
- Refactor block cipher functions, there are only three block cipher algorithms 
in the cryptodev.

 app/test/test_cryptodev.c   | 1913 +++
 app/test/test_cryptodev.h   |6 +-
 app/test/test_cryptodev_aead_test_vectors.h |1 +
 app/test/test_cryptodev_aes_test_vectors.h  |  106 ++
 app/test/test_cryptodev_blockcipher.c   |   28 +-
 app/test/test_cryptodev_hmac_test_vectors.h |   75 ++
 6 files changed, 1001 insertions(+), 1128 deletions(-)

diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index c846b26ed1..e9adf95c98 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -85,7 +85,6 @@ struct crypto_unittest_params {
 #ifdef RTE_LIB_SECURITY
struct rte_security_docsis_xform docsis_xform;
 #endif
-
union {
void *sess;
 #ifdef RTE_LIB_SECURITY
@@ -96,11 +95,9 @@ struct crypto_unittest_params {
enum rte_security_session_action_type type;
 #endif
struct rte_crypto_op *op;
-
struct rte_mbuf *obuf, *ibuf;
-
uint8_t *digest;
-};
+} unittest_params, *self = &unittest_params;
 
 #define ALIGN_POW2_ROUNDUP(num, align) \
(((num) + (align) - 1) & ~((align) - 1))
@@ -121,6 +118,9 @@ struct crypto_unittest_params {
 /*
  * Forward declarations.
  */
+static inline void
+ext_mbuf_memzone_free(int nb_segs);
+
 static int
 test_AES_CBC_HMAC_SHA512_decrypt_create_session_params(
struct crypto_unittest_params *ut_params, uint8_t *cipher_key,
@@ -196,9 +196,8 @@ post_process_raw_dp_op(void *user_data, uint32_t index 
__rte_unused,
RTE_CRYPTO_OP_STATUS_ERROR;
 }
 
-static struct crypto_testsuite_params testsuite_params = { NULL };
-struct crypto_testsuite_params *p_testsuite_params = &testsuite_params;
-static struct crypto_unittest_params unittest_params;
+static struct crypto_testsuite_params testsuite_params = { .dev_id = -1 };
+struct crypto_testsuite_params *p_ts_params = &testsuite_params;
 static bool enq_cb_called;
 static bool deq_cb_called;
 
@@ -558,12 +557,16 @@ process_crypto_request(uint8_t dev_id, struct 
rte_crypto_op *op)
return op;
 }
 
+/*
+ * Setup/teardown functions of the entire testsuite file
+ */
+
 static int
 testsuite_setup(void)
 {
struct crypto_testsuite_params *ts_params = &testsuite_params;
struct rte_cryptodev_info info;
-   uint32_t i = 0, nb_devs, dev_id;
+   uint32_t i = 0, nb_devs;
uint16_t qp_id;
 
memset(ts_params, 0, sizeof(*ts_params));
@@ -631,22 +634,26 @@ testsuite_setup(void)
if (ts_params->valid_dev_count < 1)
return TEST_FAILED;
 
-   /* Set up all the qps on the first of the valid devices found */
-
-   dev_id = ts_params->valid_devs[0];
-
-   rte_cryptodev_info_get(dev_id, &info);
+   ts_params->dev_id = ts_params->valid_devs[0];
+   rte_cryptodev_info_get(ts_params->dev_id, &ts_params->dev_info);
+   /* Check if this device does not support symmetric crypto */
+   if (!(ts_params->dev_info.feature_flags & 
RTE_CRYPTODEV_FF_SYMMETRIC_CRYPTO)) {
+   RTE_LOG(INFO, USER1, "Feature flag requirements for AES GCM "
+   "testsuite not met\n");
+   return TEST_SKIPPED;
+   }
 
-   ts_params->conf.nb_queue_pairs = info.max_nb_queue_pairs;
+   /* Set up all the qps on the first of the valid devices found */
+   ts_params->conf.nb_queue_pairs = ts_params->dev_info.max_nb_queue_pairs;
ts_params->conf.socket_id = SOCKET_ID_ANY;
ts_params->conf.ff_disable = RTE_CRYPTODEV_FF_SECURITY;
 
unsigned int session_size =
-   rte_cryptodev_sym_get_private_session_size(dev_id);
+   rte_cryptodev_sym_get_private_session_size(ts_params->dev_id);
 
 #ifdef RTE_LIB_SECURITY
unsigned int security_session_size = rte_security_session_get_size(
-   rte_cryptodev_get_sec_ctx(dev_id))

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

[Pathak, Pravin]

> -Original Message-
> From: Pavan Nikhilesh Bhagavatula 
> Sent: Wednesday, September 25, 2024 6:30 AM
> To: Mattias Rönnblom ; Pathak, Pravin
> ; Jerin Jacob ; Shijith Thotton
> ; 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
> Subject: RE: [EXTERNAL] Re: [PATCH v2 1/3] eventdev: introduce event pre-
> scheduling
> 
> > 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."
> >
> 
> In case of OCTEON, we see double the scheduling performance with
> prescheduling without effecting any priority/weight aspects.
> 
> > 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.
> >
> 
> The three prescheduling types are designed based on real world use-cases that
> some of our customers require in their applications.
> Relying on application to provide hits might not be possible in all the cases 
> as it
> is very timing sensitive.
> 
> 
> > 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`.

What is PRESCHEDULE_ADAPTIVE? Can you please add more description?
This will be more useful as per port configuration instead of device-level 
configuration.
The application can choose a type based on its requirement on the port it is 
serving.
As Mattias suggested, if this is made HINT flag for port configuration, other 
PMDs can
Ignore it based on either they may not need it depending on their architecture 
or not support it.

> > >>>
> > >>> 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;
> > >>> 

Re: [v4] net/zxdh: Provided zxdh basic init

[Junlong Wang]

>> +if not is_linux
>> +build = false
>> +reason = 'only supported on Linux'
>> +subdir_done()
>> +endif
>> +
>> +if arch_subdir != 'x86' and arch_subdir !
>> = 'arm' or not dpdk_conf.get('RTE_ARCH_64')
>>

>Why not check 'RTE_ARCH_X86_64' and 'RTE_ARCH_ARM64'?

we will fix it and use 'RTE_ARCH_X86_64' and 'RTE_ARCH_ARM64' to check,
 
other comments will be modified, and split the driver into multiple patches.

Thanks!

[PATCH] maintainers: fix prog guide paths

[Thomas Monjalon]

When moving some files in doc subdirectories for ethdev and eventdev,
the references in the file MAINTAINERS were forgotten.
The new paths are used in this fix.

Fixes: 41dd9a6bc2d9 ("doc: reorganize prog guide")

Signed-off-by: Thomas Monjalon 
---
 MAINTAINERS | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index c5a703b5c0..af2620bf0a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -423,27 +423,27 @@ T: git://dpdk.org/next/dpdk-next-net
 F: lib/ethdev/
 F: app/test/test_ethdev*
 F: devtools/test-null.sh
-F: doc/guides/prog_guide/switch_representation.rst
+F: doc/guides/prog_guide/ethdev/switch_representation.rst
 
 Flow API
 M: Ori Kam 
 T: git://dpdk.org/next/dpdk-next-net
 F: app/test-pmd/cmdline_flow.c
-F: doc/guides/prog_guide/rte_flow.rst
+F: doc/guides/prog_guide/ethdev/flow_offload.rst
 F: lib/ethdev/rte_flow*
 
 Traffic Management API
 M: Cristian Dumitrescu 
 T: git://dpdk.org/next/dpdk-next-net
 F: lib/ethdev/rte_tm*
-F: doc/guides/prog_guide/traffic_management.rst
+F: doc/guides/prog_guide/ethdev/traffic_management.rst
 F: app/test-pmd/cmdline_tm.*
 
 Traffic Metering and Policing API - EXPERIMENTAL
 M: Cristian Dumitrescu 
 T: git://dpdk.org/next/dpdk-next-net
 F: lib/ethdev/rte_mtr*
-F: doc/guides/prog_guide/traffic_metering_and_policing.rst
+F: doc/guides/prog_guide/ethdev/traffic_metering_and_policing.rst
 F: app/test-pmd/cmdline_mtr.*
 
 Baseband API
@@ -520,7 +520,7 @@ Eventdev API
 M: Jerin Jacob 
 T: git://dpdk.org/next/dpdk-next-eventdev
 F: lib/eventdev/
-F: doc/guides/prog_guide/eventdev.rst
+F: doc/guides/prog_guide/eventdev/eventdev.rst
 F: drivers/event/skeleton/
 F: app/test/test_eventdev.c
 F: examples/l3fwd/l3fwd_event*
@@ -530,35 +530,35 @@ M: Naga Harish K S V 
 T: git://dpdk.org/next/dpdk-next-eventdev
 F: lib/eventdev/*eth_rx_adapter*
 F: app/test/test_event_eth_rx_adapter.c
-F: doc/guides/prog_guide/event_ethernet_rx_adapter.rst
+F: doc/guides/prog_guide/eventdev/event_ethernet_rx_adapter.rst
 
 Eventdev Ethdev Tx Adapter API
 M: Naga Harish K S V 
 T: git://dpdk.org/next/dpdk-next-eventdev
 F: lib/eventdev/*eth_tx_adapter*
 F: app/test/test_event_eth_tx_adapter.c
-F: doc/guides/prog_guide/event_ethernet_tx_adapter.rst
+F: doc/guides/prog_guide/eventdev/event_ethernet_tx_adapter.rst
 
 Eventdev Timer Adapter API
 M: Erik Gabriel Carrillo 
 T: git://dpdk.org/next/dpdk-next-eventdev
 F: lib/eventdev/*timer_adapter*
 F: app/test/test_event_timer_adapter.c
-F: doc/guides/prog_guide/event_timer_adapter.rst
+F: doc/guides/prog_guide/eventdev/event_timer_adapter.rst
 
 Eventdev Crypto Adapter API
 M: Abhinandan Gujjar 
 T: git://dpdk.org/next/dpdk-next-eventdev
 F: lib/eventdev/*crypto_adapter*
 F: app/test/test_event_crypto_adapter.c
-F: doc/guides/prog_guide/event_crypto_adapter.rst
+F: doc/guides/prog_guide/eventdev/event_crypto_adapter.rst
 
 Eventdev DMA Adapter API
 M: Amit Prakash Shukla 
 T: git://dpdk.org/next/dpdk-next-eventdev
 F: lib/eventdev/*dma_adapter*
 F: app/test/test_event_dma_adapter.c
-F: doc/guides/prog_guide/event_dma_adapter.rst
+F: doc/guides/prog_guide/eventdev/event_dma_adapter.rst
 
 Raw device API
 M: Sachin Saxena 
@@ -1586,7 +1586,7 @@ F: doc/guides/sample_app_ug/packet_ordering.rst
 Hierarchical scheduler
 M: Cristian Dumitrescu 
 F: lib/sched/
-F: doc/guides/prog_guide/qos_framework.rst
+F: doc/guides/prog_guide/ethdev/qos_framework.rst
 F: app/test/test_pie.c
 F: app/test/test_red.c
 F: app/test/test_sched.c
@@ -1757,7 +1757,7 @@ Dispatcher - EXPERIMENTAL
 M: Mattias Rönnblom 
 F: lib/dispatcher/
 F: app/test/test_dispatcher.c
-F: doc/guides/prog_guide/dispatcher_lib.rst
+F: doc/guides/prog_guide/eventdev/dispatcher_lib.rst
 
 Job statistics
 F: lib/jobstats/
-- 
2.46.0

Re: [PATCH v8 2/8] ethdev: add telemetry cmd for registers

[fengchengwen]

Acked-by: Chengwen Feng 

On 2024/9/25 14:40, Jie Hai wrote:
> This patch adds a telemetry command for registers dump,
> and supports obtaining the registers of a specified module.
> 
> In one way, the number of registers that can be exported
> is limited by the number of elements carried by dict and
> container. In another way, the length of the string
> exported by telemetry is limited by MAX_OUTPUT_LEN.
> Therefore, when the number of registers to be exported
> exceeds, some information will be lost. Warn on the former
> case.
> 
> An example usage is shown below:
> --> /ethdev/regs,0,ring
> {
>   "/ethdev/regs": {
> "registers_length": 318,
> "registers_width": 4,
> "register_offset": "0x0",
> "version": "0x1140011",
> "group_0": {
>   "Q0_ring_rx_bd_num": "0x0",
>   "Q0_ring_rx_bd_len": "0x0",
>   ...
>   },
> "group_1": {
> ...
> },
> ...
> }
> 
> Signed-off-by: Jie Hai 
> Reviewed-by: Ferruh Yigit 

Re: [RFC] ethdev: introduce PTP device capability

[Ferruh Yigit]

On 9/24/2024 8:24 AM, lihuisong (C) wrote:
> Hi Ferruh,
> 
> 
> 在 2024/9/23 6:23, Ferruh Yigit 写道:
>> On 1/30/2024 3:58 AM, Huisong Li wrote:
>>> Currently, the PTP feature is a little messy and has some issue.
>>> 1> There is different implementation for PTP. This feature of some
>>>     hardware depends on the Rx HW timestamp offload, and use this
>>>     offload to detect if enable PTP feature. Others can enable PTP
>>>     feature with only ethdev ops.
>>> 2> For some drivers, enabling PTP feature also depends on the macro
>>>     RTE_LIBRTE_IEEE1588. Actually, whether device support, enable
>>>     or disable this feature should not be determined at compilation
>>>     time. This has been discussed in thread [1].
>>> 3> The user haven't a good way to distinguish which port supports
>>>     the PTP feature in the case that a couple of device belong to the
>>>     same driver. And PTP related API in ethdev layer doesn't do check
>>>     for PTP capability. This has been mentioned and discussed in
>>>     thread [2].
>>>
>>> In the thread [1], there is a conclusion that remove the compiling
>>> macro RTE_LIBRTE_IEEE1588. And in the thread [2], there is a common
>>> opinion that the RTE_ETH_RX_OFFLOAD_TIMESTAMP cannot be used as the
>>> PTP capability.
>>>
>>> For the above issuse, this patch introduces a PTP capability in
>>> rte_eth_dev_info.dev_capa to report PTP capability.
>>>
>>> Welcome to jumping into discussion.
>>>
>>> [1] https://patchwork.dpdk.org/project/dpdk/
>>> patch/20230203132810.14187-1-tho...@monjalon.net/
>>> [2] https://inbox.dpdk.org/dev/20230817084226.55327-1-
>>> lihuis...@huawei.com/
>>>
>>> Signed-off-by: Huisong Li 
>>> ---
>>>   lib/ethdev/rte_ethdev.h | 6 ++
>>>   1 file changed, 6 insertions(+)
>>>
>>> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
>>> index efa4f12b2a..4c8bd691bd 100644
>>> --- a/lib/ethdev/rte_ethdev.h
>>> +++ b/lib/ethdev/rte_ethdev.h
>>> @@ -1613,6 +1613,12 @@ struct rte_eth_conf {
>>>   #define RTE_ETH_DEV_CAPA_FLOW_RULE_KEEP RTE_BIT64(3)
>>>   /** Device supports keeping shared flow objects across restart. */
>>>   #define RTE_ETH_DEV_CAPA_FLOW_SHARED_OBJECT_KEEP RTE_BIT64(4)
>>> +/**
>>> + * Device supports PTP feature.
>>> + * For some hardware, this feature also need to set the offload
>>> + * RTE_ETH_RX_OFFLOAD_TIMESTAMP, please check
>>> rte_eth_dev_info.rx_offload_capa.
>>> + */
>>> +#define RTE_ETH_DEV_CAPA_PTP RTE_BIT64(5)
>>>   /**@}*/
>>>     /*
>> Hi Huisong,
>>
>> Thanks for the effort, as you mentioned PTP feature can be improved and
>> there is a target to remove RTE_LIBRTE_IEEE1588 build time macro.
>>
>> As far as I remember, one of the main reasons of the RTE_LIBRTE_IEEE1588
>> macro is some HW has resource restrictions, when RTE_LIBRTE_IEEE1588 is
>> enabled some other features, like vector datapath, are not usable, that
>> is why this is a build time selection.
> I think the main reason that driver don't support PTP feature in vector
> datapath is for performance.
> This can be controled by releated dev_ops API or TIMESTAMP offload and
> no need to use RTE_LIBRTE_IEEE1588 macro, like hns3.
>>
>> And related to the PTP capability, can you please give some more
>> information, what does device having PTP capability exactly means.
> Just the device having PTP capability can be used to enable PTP feature.
>

Hi Huisong,

I am asking for your support but not able to get detailed information is
not helping to progress the patch.

If application implements PTP protocol, we already have APIs for
application to read time from NIC, or to adjust NIC clock, etc..:
'rte_eth_timesync_read_time()'
'rte_eth_timesync_adjust_time()'
...

Application can check if these APIs are supported by the device to
deduce if PTP can be supported by device or not, why this is not sufficient?

If application PTP implementation requires HW timestamp offload,
availability of this offload also can be checked.

I think for most of the cases, with combination of above two,
application can decide if it can support PTP with given device or not.

What else is missing so that application needs this additional
capability flag from NIC?


>>
>> PTP is protocol and it is implemented in userspace daemon. I guess even
>> NIC does not support timestamp offloading, by using time information
>> from SW it can still implement PTP, right?
> 
> AFAIS, PTP feature implement requires the collaboration of HW and SW, as
> describted by the releated dev_ops API.
> 
>> Is PTP offload means, offloading some part of the protocol communication
>> withing the device?
> Normally, a feature offload means this feature is done in hardware and
> the software doesn't need to something for this.
> I reviewed our discussion in [1], we all think it's unreasonable to name
> it "PTP offload".
> 
>>
>> Btw, the relevant RTE_ETH_RX_OFFLOAD_TIMESTAMP offload is, a little more
>> generic HW capability that HW can add timestamp to Rx/Tx packets, which
>> can be u

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

[Ferruh Yigit]

On 9/23/2024 10:26 AM, 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.
> 

Hi Niall,


Thanks for the patch.

For the 'RTE_ALIGN_CEIL(val, align)' macro, 'align' should be power of
two, as 'desc_lim->nb_align' is uint16_t, max value it can get is 2^15.
'val' should be smaller than or equal to 'align', so '*nb_desc' can be
maximum 2^15.

So RTE_ALIGN_CEIL(2^15-1, 2^15) = 2^15, I think this should work fine
(although I didn't test).

And even with your uint32_t cast, I think following will fail:
RTE_ALIGN_CEIL(2^16-1, 2^15)
(again, not tested).

Or maybe I am missing a case, can you please give some actual numbers to
show the problem and the fix?


Perhaps what we need is to verify mentioned requirements of the macro in
the function:
- 'align' should be power of two
- val <= align
But as this is a static function, these checks can be done in caller
function and preconditions can be enforced.


> 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

[PATCH] net/r8169: add support for phy configuration

[Howard Wang]

This patch contains phy config, ephy config and so on.

Signed-off-by: Howard Wang 
---
 drivers/net/r8169/r8169_ethdev.c |  10 +
 drivers/net/r8169/r8169_ethdev.h |   6 +
 drivers/net/r8169/r8169_phy.c| 446 +++
 drivers/net/r8169/r8169_phy.h| 100 +++
 4 files changed, 562 insertions(+)

diff --git a/drivers/net/r8169/r8169_ethdev.c b/drivers/net/r8169/r8169_ethdev.c
index 2bd21d4877..b24d39450e 100644
--- a/drivers/net/r8169/r8169_ethdev.c
+++ b/drivers/net/r8169/r8169_ethdev.c
@@ -72,6 +72,12 @@ rtl_dev_start(struct rte_eth_dev *dev)
struct rtl_hw *hw = &adapter->hw;
int err;
 
+   rtl_powerup_pll(hw);
+
+   rtl_hw_ephy_config(hw);
+
+   rtl_hw_phy_config(hw);
+
rtl_hw_config(hw);
 
/* Initialize transmission unit */
@@ -84,6 +90,8 @@ rtl_dev_start(struct rte_eth_dev *dev)
goto error;
}
 
+   rtl_mdio_write(hw, 0x1F, 0x);
+
hw->adapter_stopped = 0;
 
return 0;
@@ -104,6 +112,8 @@ rtl_dev_stop(struct rte_eth_dev *dev)
if (hw->adapter_stopped)
return 0;
 
+   rtl_powerdown_pll(hw);
+
hw->adapter_stopped = 1;
dev->data->dev_started = 0;
 
diff --git a/drivers/net/r8169/r8169_ethdev.h b/drivers/net/r8169/r8169_ethdev.h
index c9acaabf8e..a0da173685 100644
--- a/drivers/net/r8169/r8169_ethdev.h
+++ b/drivers/net/r8169/r8169_ethdev.h
@@ -39,6 +39,12 @@ struct rtl_hw {
 
u8  NotWrRamCodeToMicroP;
u8  HwHasWrRamCodeToMicroP;
+   u8  HwSuppCheckPhyDisableModeVer;
+
+   u16 sw_ram_code_ver;
+   u16 hw_ram_code_ver;
+
+   u32 HwSuppMaxPhyLinkSpeed;
 
/* Enable Tx No Close */
u8 EnableTxNoClose;
diff --git a/drivers/net/r8169/r8169_phy.c b/drivers/net/r8169/r8169_phy.c
index 5a9bd9a504..9a0bfa2d92 100644
--- a/drivers/net/r8169/r8169_phy.c
+++ b/drivers/net/r8169/r8169_phy.c
@@ -329,3 +329,449 @@ rtl_set_phy_mcu_ram_code(struct rtl_hw *hw, const u16 
*ramcode, u16 codesize)
return;
 }
 
+static u8
+rtl_is_phy_disable_mode_enabled(struct rtl_hw *hw)
+{
+   u8 phy_disable_mode_enabled = FALSE;
+
+   switch (hw->HwSuppCheckPhyDisableModeVer) {
+   case 3:
+   if (RTL_R8(hw, 0xF2) & BIT_5)
+   phy_disable_mode_enabled = TRUE;
+   break;
+   }
+
+   return phy_disable_mode_enabled;
+}
+
+static u8
+rtl_is_gpio_low(struct rtl_hw *hw)
+{
+   u8 gpio_low = FALSE;
+
+   switch (hw->HwSuppCheckPhyDisableModeVer) {
+   case 3:
+   if (!(rtl_mac_ocp_read(hw, 0xDC04) & BIT_13))
+   gpio_low = TRUE;
+   break;
+   }
+
+   return gpio_low;
+}
+
+static u8
+rtl_is_in_phy_disable_mode(struct rtl_hw *hw)
+{
+   u8 in_phy_disable_mode = FALSE;
+
+   if (rtl_is_phy_disable_mode_enabled(hw) && rtl_is_gpio_low(hw))
+   in_phy_disable_mode = TRUE;
+
+   return in_phy_disable_mode;
+}
+
+static void
+rtl_wait_phy_ups_resume(struct rtl_hw *hw, u16 PhyState)
+{
+   u16 tmp_phy_state;
+   int i = 0;
+
+   switch (hw->mcfg) {
+   case CFG_METHOD_48 ... CFG_METHOD_57:
+   case CFG_METHOD_69 ... CFG_METHOD_71:
+   do {
+   tmp_phy_state = rtl_mdio_direct_read_phy_ocp(hw, 
0xA420);
+   tmp_phy_state &= 0x7;
+   mdelay(1);
+   i++;
+   } while ((i < 100) && (tmp_phy_state != PhyState));
+   }
+}
+
+static void
+rtl_phy_power_up(struct rtl_hw *hw)
+{
+   if (rtl_is_in_phy_disable_mode(hw))
+   return;
+
+   rtl_mdio_write(hw, 0x1F, 0x);
+   rtl_mdio_write(hw, MII_BMCR, BMCR_ANENABLE);
+
+   /* Wait ups resume (phy state 3) */
+   switch (hw->mcfg) {
+   case CFG_METHOD_48 ... CFG_METHOD_57:
+   case CFG_METHOD_69 ... CFG_METHOD_71:
+   rtl_wait_phy_ups_resume(hw, 3);
+   }
+}
+
+void
+rtl_powerup_pll(struct rtl_hw *hw)
+{
+   switch (hw->mcfg) {
+   case CFG_METHOD_48 ... CFG_METHOD_57:
+   case CFG_METHOD_69 ... CFG_METHOD_71:
+   RTL_W8(hw, PMCH, RTL_R8(hw, PMCH) | BIT_7 | BIT_6);
+   }
+
+   rtl_phy_power_up(hw);
+}
+
+static void
+rtl_phy_power_down(struct rtl_hw *hw)
+{
+   rtl_mdio_write(hw, 0x1F, 0x);
+   rtl_mdio_write(hw, MII_BMCR, BMCR_ANENABLE | BMCR_PDOWN);
+}
+
+void
+rtl_powerdown_pll(struct rtl_hw *hw)
+{
+   if (hw->DASH)
+   return;
+
+   rtl_phy_power_down(hw);
+
+   switch (hw->mcfg) {
+   case CFG_METHOD_48 ... CFG_METHOD_57:
+   case CFG_METHOD_69 ... CFG_METHOD_71:
+   RTL_W8(hw, PMCH, RTL_R8(hw, PMCH) & ~BIT_7);
+   break;
+   }
+}
+
+void
+rtl_hw_ephy_config(struct rtl_hw *hw)
+{
+   hw->hw_ops.hw_ephy_config(hw);
+}
+
+static int
+rtl_wait_phy_reset_complete(struct rtl_hw *hw)
+{
+   int i, val;
+
+   for (i = 0; i < 2500; i++) {
+   val 

Re: [PATCH v4 1/5] dts: allow binding only a single port to a different driver

[Juraj Linkeš]

On 23. 9. 2024 20:42, jspew...@iol.unh.edu wrote:

From: Jeremy Spewock 

Previously the DTS framework only included methods that bind all ports
that the test run was aware of to either the DPDK driver or the OS
driver. There are however some cases, like creating virtual functions,
where you would want some ports bound to the OS driver and others bound
to their DPDK driver. This patch adds the ability to bind individual
ports to their respective drviers to solve this problem.


Typo: drviers



Depends-on: patch-144318 ("dts: add binding to different drivers to TG
node")

Signed-off-by: Jeremy Spewock 
---
  dts/framework/testbed_model/node.py | 21 -
  dts/tests/TestSuite_os_udp.py   |  4 ++--
  2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/dts/framework/testbed_model/node.py 
b/dts/framework/testbed_model/node.py



@@ -266,12 +266,15 @@ def bind_ports_to_driver(self, for_dpdk: bool = True) -> 
None:
  If :data:`False`, binds to os_driver.
  """
  for port in self.ports:
-driver = port.os_driver_for_dpdk if for_dpdk else port.os_driver
-self.main_session.send_command(
-f"{self.path_to_devbind_script} -b {driver} --force 
{port.pci}",
-privileged=True,
-verify=True,
-)
+self._bind_port_to_driver(port, for_dpdk)
+
+def _bind_port_to_driver(self, port: Port, for_dpdk: bool = True) -> None:
+driver = port.os_driver_for_dpdk if for_dpdk else port.os_driver
+self.main_session.send_command(
+f"{self.path_to_devbind_script} -b {driver} --force {port.pci}",
+privileged=True,
+verify=True,
+)


I noticed we're doing this port by port, but we can bind multiple ports 
in one devbind run - just pass all ports to it. I think this improvement 
is well worth it.

Re: [PATCH V3 0/3] Error report improvement and fix

[Raslan Darawsheh]

Hi,


From: Minggang(Gavin) Li 
Sent: Tuesday, September 24, 2024 1:52 PM
To: Matan Azrad; Slava Ovsiienko; Ori Kam; NBU-Contact-Thomas Monjalon 
(EXTERNAL)
Cc: dev@dpdk.org; Raslan Darawsheh
Subject: [PATCH V3 0/3] Error report improvement and fix

This patch set is to improve error handling in pmd and under layer.

Gavin Li (3):
  net/mlx5: set rte errno if malloc failed
---
changelog:
v0->v1
- Fix typo in commit message
v1->v2
- Fix checkpatch warning
---
  net/mlx5/hws: add log for failing to create rule in HWS
---
changelog:
v1->v2
- Fix checkpatch warning
v2->v3
- Fix checkpatch warning
---
  net/mlx5/hws: print CQE error syndrome and more information
---
changelog:
v1->v2
- Fix checkpatch warning
v2->v3
- Fix checkpatch warning
---

 drivers/net/mlx5/hws/mlx5dr_rule.c |  6 ++
 drivers/net/mlx5/hws/mlx5dr_send.c |  9 -
 drivers/net/mlx5/mlx5_flow_hw.c| 31 +++---
 3 files changed, 38 insertions(+), 8 deletions(-)

--
2.34.1


Series applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh

Re: [PATCH v4 0/5] dts: add VFs to the framework

[Juraj Linkeš]

On 23. 9. 2024 20:42, jspew...@iol.unh.edu wrote:

From: Jeremy Spewock 



This is no longer an RFC, the body should be there so that we know what 
the state after it stopped being an RFC is.



v4:
  * apply to next-dts

Jeremy Spewock (5):
   dts: allow binding only a single port to a different driver
   dts: parameterize what ports the TG sends packets to
   dts: add class for virtual functions
   dts: add OS abstractions for creating virtual functions
   dts: add functions for managing VFs to Node

  dts/framework/test_suite.py  |  48 ++--
  dts/framework/testbed_model/linux_session.py |  40 ++-
  dts/framework/testbed_model/node.py  | 116 +--
  dts/framework/testbed_model/os_session.py|  40 +++
  dts/framework/testbed_model/port.py  |  37 +-
  dts/tests/TestSuite_os_udp.py|   4 +-
  6 files changed, 260 insertions(+), 25 deletions(-)

Re: [PATCH v3 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell

[Juraj Linkeš]

On 24. 9. 2024 18:34, Jeremy Spewock wrote:

On Tue, Sep 24, 2024 at 6:55 AM Juraj Linkeš  wrote:


I like how this looks. I have a number of minor comments (mainly wording
and naming), but overall it looks very good.

On 19. 9. 2024 21:02, jspew...@iol.unh.edu wrote:

From: Jeremy Spewock 

Previously all scapy commands were handled using an XML-RPC server that
ran on the TGNode. This unnecessarily enforces a minimum Python version
of 3.10 on the server that is being used as a traffic generator and
complicates the implementation of scapy methods.


What is the TG's minimum Python version now?
https://bugs.dpdk.org/show_bug.cgi?id=1388 says this will become a moot
point, but we're still using Python on the remote node.


Right, there is still some dependency there. I'm not sure the exact
versions, but doing some looking around I believe one of the newest
things scapy tools we use in the framework is the AsyncSniffer and the
scapy documentation [1] says that it has been available since version
2.4.3, and then when I looked at that version of the scapy package [2]
it looks like it claims to support python 2.7 and python 3.4-7. Poking
around in the documentation/code from scapy version 2.4.3 it also
looks like the syntax is very similar, so I believe it would work, but
I'm not sure I have any hosts that I could run on which have python
3.4. Maybe that does make the dependency essentially a moot point
considering these are fairly old python versions.

[1] https://github.com/secdev/scapy/blob/master/doc/scapy/usage.rst
[2] https://pypi.org/project/scapy/2.4.3/



Great. We should still document that the TG needs Python. I've updated 
the ticket. [0]


[0] https://bugs.dpdk.org/show_bug.cgi?id=1388



+def _create_packet_list(self, packets: list[Packet]) -> None:


Maybe we could apply some of the ideas from the local/remote naming
scheme I talked about in the tg devbind script patch here. Whatever
happens on the TG could be prefixed with remote and whatever is
happening locally would be without the prefix (or maybe whatever is
happnening on the TG shouldn't be prefixed (or a different prefix -
shell)? Makes sense, but then we'd need a good prefix for what's
happening on the execution environment. Maybe this also needs to be in a
different patch, after it's been though through with everything else in


I'll still write something here that makes the distinction and that
other patch could reformat if the author thought something else was
better.



Great.



+sniffer_commands = [
+f"{self._sniffer_name} = AsyncSniffer(",
+f"iface='{recv_port.logical_name}',",
+"store=True,",
+"started_callback=lambda *args: sendp(",


As far as I can tell, we're not using the args, so we can just use
"lambda: sendp()"


We don't use the argument, but there are positional arguments passed
into this function by scapy which is why we have to catch and ignore
them.



Makes sense now, thanks for the explanation. Maybe we could put 
somewhere in here a little comment pointing this out?





+(
+f"{self._python_indentation}{self._send_packet_list_name},"


Is the indentation needed here?


It's not required, but I think it makes the logs more readable.



That's a worthy use, let's keep it. Maybe also add an explanatory 
comment here?

[PATCH] net/r8169: add support for hw initialization

[Howard Wang]

Signed-off-by: Howard Wang 
---
 drivers/net/r8169/meson.build|   1 +
 drivers/net/r8169/r8169_base.h   |  43 +++
 drivers/net/r8169/r8169_dash.c   |  87 +
 drivers/net/r8169/r8169_dash.h   |  35 ++
 drivers/net/r8169/r8169_ethdev.c |  57 ++-
 drivers/net/r8169/r8169_ethdev.h |  30 +-
 drivers/net/r8169/r8169_hw.c | 588 +++
 drivers/net/r8169/r8169_hw.h |  42 +++
 drivers/net/r8169/r8169_phy.h|  16 +-
 9 files changed, 889 insertions(+), 10 deletions(-)
 create mode 100644 drivers/net/r8169/r8169_dash.c
 create mode 100644 drivers/net/r8169/r8169_dash.h

diff --git a/drivers/net/r8169/meson.build b/drivers/net/r8169/meson.build
index 08995453c7..8235e8ca43 100644
--- a/drivers/net/r8169/meson.build
+++ b/drivers/net/r8169/meson.build
@@ -6,6 +6,7 @@ sources = files(
'r8169_hw.c',
'r8169_rxtx.c',
'r8169_phy.c',
+   'r8169_dash.c',
'base/rtl8125a.c',
'base/rtl8125a_mcu.c',
'base/rtl8125b.c',
diff --git a/drivers/net/r8169/r8169_base.h b/drivers/net/r8169/r8169_base.h
index e01b1e3470..2ee6fc6782 100644
--- a/drivers/net/r8169/r8169_base.h
+++ b/drivers/net/r8169/r8169_base.h
@@ -237,6 +237,10 @@ enum RTL_registers {
IMR_V4_L2_CLEAR_REG_8125 = 0x0D10,
IMR_V4_L2_SET_REG_8125   = 0x0D18,
ISR_V4_L2_8125  = 0x0D14,
+   SW_TAIL_PTR0_8125BP = 0x0D30,
+   SW_TAIL_PTR1_8125BP = 0x0D38,
+   HW_CLO_PTR0_8125BP = 0x0D34,
+   HW_CLO_PTR1_8125BP = 0x0D3C,
DOUBLE_VLAN_CONFIG = 0x1000,
TX_NEW_CTRL= 0x203E,
TNPDS_Q1_LOW_8125  = 0x2100,
@@ -482,6 +486,16 @@ enum RTL_register_content {
ISRIMR_V2_LINKCHG= (1 << 21),
 };
 
+enum RTL_chipset_name {
+   RTL8125A = 0,
+   RTL8125B,
+   RTL8168KB,
+   RTL8125BP,
+   RTL8125D,
+   RTL8126A,
+   UNKNOWN
+};
+
 #define PCI_VENDOR_ID_REALTEK 0x10EC
 
 #define RTL_PCI_REG_ADDR(hw, reg) ((u8 *)(hw)->mmio_addr + (reg))
@@ -522,6 +536,35 @@ enum RTL_register_content {
 
 #define ETH_HLEN14
 
+#define SPEED_10   10
+#define SPEED_100  100
+#define SPEED_1000 1000
+#define SPEED_2500 2500
+#define SPEED_5000 5000
+
+#define DUPLEX_HALF1
+#define DUPLEX_FULL2
+
+#define AUTONEG_ENABLE 1
+#define AUTONEG_DISABLE0
+
+#define ADVERTISE_10_HALF 0x0001
+#define ADVERTISE_10_FULL 0x0002
+#define ADVERTISE_100_HALF0x0004
+#define ADVERTISE_100_FULL0x0008
+#define ADVERTISE_1000_HALF   0x0010 /* Not used, just FYI */
+#define ADVERTISE_1000_FULL   0x0020
+#define ADVERTISE_2500_HALF   0x0040 /* NOT used, just FYI */
+#define ADVERTISE_2500_FULL   0x0080
+#define ADVERTISE_5000_HALF   0x0100 /* NOT used, just FYI */
+#define ADVERTISE_5000_FULL   0x0200
+
+#define RTL8126_ALL_SPEED_DUPLEX (ADVERTISE_10_HALF | ADVERTISE_10_FULL | \
+ADVERTISE_100_HALF | ADVERTISE_100_FULL | ADVERTISE_1000_FULL | \
+ADVERTISE_2500_FULL | ADVERTISE_5000_FULL)
+
+#define MAC_ADDR_LENRTE_ETHER_ADDR_LEN
+
 static inline u32
 rtl_read32(volatile void *addr)
 {
diff --git a/drivers/net/r8169/r8169_dash.c b/drivers/net/r8169/r8169_dash.c
new file mode 100644
index 00..9422f10402
--- /dev/null
+++ b/drivers/net/r8169/r8169_dash.c
@@ -0,0 +1,87 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2024 Realtek Corporation. All rights reserved
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+#include "r8169_base.h"
+#include "r8169_dash.h"
+#include "r8169_hw.h"
+
+bool
+rtl_is_allow_access_dash_ocp(struct rtl_hw *hw)
+{
+   bool allow_access = false;
+   u16 mac_ocp_data;
+
+   if (!HW_DASH_SUPPORT_DASH(hw))
+   goto exit;
+
+   allow_access = true;
+   switch (hw->mcfg) {
+   case CFG_METHOD_2:
+   case CFG_METHOD_3:
+   mac_ocp_data = rtl_mac_ocp_read(hw, 0xd460);
+   if (mac_ocp_data == 0x || !(mac_ocp_data & BIT_0))
+   allow_access = false;
+   break;
+   case CFG_METHOD_8:
+   case CFG_METHOD_9:
+   mac_ocp_data = rtl_mac_ocp_read(hw, 0xd4c0);
+   if (mac_ocp_data == 0x || (mac_ocp_data & BIT_3))
+   allow_access = false;
+   break;
+   default:
+   goto exit;
+   }
+exit:
+   return allow_access;
+}
+
+static u32
+rtl_get_dash_fw_ver(struct rtl_hw *hw)
+{
+   u32 ver = 0x;
+
+   if (FALSE == HW_DASH_SUPPORT_GET_FIRMWARE_VERSION(hw))
+   goto exit;
+
+   ver = rtl_ocp_read(hw, OCP_REG_FIRMWARE_MAJOR_VERSION, 4);
+
+exit:
+   return ver;
+}
+
+static int
+_rtl_check_dash(struct rtl_hw *hw)
+{
+   if (!hw->AllowAccessDashOcp)
+   return 0;
+
+   if (HW_DASH_SUPPORT_TYPE_2(hw) || HW_DASH_SUPPORT_TYPE_4(hw)) {
+   if (rtl_ocp_read(hw, 0x128, 1) & BIT_0)
+  

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

[Pavan Nikhilesh Bhagavatula]

> 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."
> 

In case of OCTEON, we see double the scheduling performance with
prescheduling without effecting any priority/weight aspects.

> 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.
> 

The three prescheduling types are designed based on real world use-cases
that some of our customers require in their applications.
Relying on application to provide hits might not be possible in all the cases
as it is very timing sensitive.


> 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);

[DPDK/vhost/virtio Bug 1553] free called on rte_malloc block in vhost

[bugzilla]

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

Bug ID: 1553
   Summary: free called on rte_malloc block in vhost
   Product: DPDK
   Version: unspecified
  Hardware: All
OS: All
Status: UNCONFIRMED
  Severity: major
  Priority: Normal
 Component: vhost/virtio
  Assignee: dev@dpdk.org
  Reporter: step...@networkplumber.org
  Target Milestone: ---

If Gcc function attributes are added to rte_malloc, the compiler is able to
detect bugs where free is called on rte_malloc memory such as:

[2737/2957] Compiling C object
examples/dpdk-vhost_blk.p/vhost_blk_vhost_blk.c.o
In function ‘vhost_blk_bdev_construct’,
inlined from ‘vhost_blk_ctrlr_construct.constprop’ at
../examples/vhost_blk/vhost_blk.c:826:16:
../examples/vhost_blk/vhost_blk.c:779:17: warning: ‘free’ called on pointer
returned from a mismatched allocation function [-Wmismatched-dealloc]
  779 | free(bdev);
  | ^~
../examples/vhost_blk/vhost_blk.c:761:16: note: returned from ‘rte_zmalloc’
  761 | bdev = rte_zmalloc(NULL, sizeof(*bdev), RTE_CACHE_LINE_SIZE);
  |^

-- 
You are receiving this mail because:
You are the assignee for the bug.

[DPDK/ethdev Bug 1549] free() of non rte_malloc() memory in DMA dev

[bugzilla]

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

Bug ID: 1549
   Summary: free() of non rte_malloc() memory in DMA dev
   Product: DPDK
   Version: unspecified
  Hardware: All
OS: All
Status: UNCONFIRMED
  Severity: major
  Priority: Normal
 Component: ethdev
  Assignee: dev@dpdk.org
  Reporter: step...@networkplumber.org
  Target Milestone: ---

If GCC checking of rte_malloc functions is added, then the following bug in the
DMA device driver is shown:

[982/2957] Compiling C object
drivers/libtmp_rte_dma_idxd.a.p/dma_idxd_idxd_pci.c.o
In function ‘init_pci_device’,
inlined from ‘idxd_dmadev_probe_pci’ at
../drivers/dma/idxd/idxd_pci.c:362:8:
../drivers/dma/idxd/idxd_pci.c:304:9: warning: ‘free’ called on pointer
returned from a mismatched allocation function [-Wmismatched-dealloc]
  304 | free(pci);
  | ^
../drivers/dma/idxd/idxd_pci.c:182:15: note: returned from ‘rte_malloc’
  182 | pci = rte_malloc(NULL, sizeof(*pci), 0);
  |   ^
../drivers/dma/idxd/idxd_pci.c:304:9: warning: ‘free’ called on pointer
returned from a mismatched allocation function [-Wmismatched-dealloc]
  304 | free(pci);
  | ^
../drivers/dma/idxd/idxd_pci.c:182:15: note: returned from ‘rte_malloc’
  182 | pci = rte_malloc(NULL, sizeof(*pci), 0);
  |   ^

-- 
You are receiving this mail because:
You are the assignee for the bug.

[DPDK/ethdev Bug 1550] Use after free in E1000 driver

[bugzilla]

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

Bug ID: 1550
   Summary: Use after free in E1000 driver
   Product: DPDK
   Version: unspecified
  Hardware: All
OS: All
Status: UNCONFIRMED
  Severity: major
  Priority: Normal
 Component: ethdev
  Assignee: dev@dpdk.org
  Reporter: step...@networkplumber.org
  Target Milestone: ---

If function attributes are added to rte_malloc() Gcc will detect use after free
in e1000.

[1048/2957] Compiling C object
drivers/libtmp_rte_net_e1000.a.p/net_e1000_igb_ethdev.c.o
In file included from ../drivers/net/e1000/base/e1000_hw.h:8,
 from ../drivers/net/e1000/base/e1000_api.h:8,
 from ../drivers/net/e1000/igb_ethdev.c:28:
../drivers/net/e1000/igb_ethdev.c: In function ‘igb_delete_2tuple_filter’:
../drivers/net/e1000/igb_ethdev.c:3914:49: warning: pointer ‘filter’ used after
‘rte_free’ [-Wuse-after-free]
 3914 | E1000_WRITE_REG(hw, E1000_IMIREXT(filter->index), 0);
../drivers/net/e1000/base/e1000_osdep.h:76:48: note: in definition of macro
‘E1000_PCI_REG_WRITE’
   76 | rte_write32((rte_cpu_to_le_32(value)), reg)
  |^~~
../drivers/net/e1000/base/e1000_osdep.h:121:29: note: in expansion of macro
‘E1000_PCI_REG_ADDR’
  121 | E1000_PCI_REG_WRITE(E1000_PCI_REG_ADDR((hw), (reg)), (value))
  | ^~
../drivers/net/e1000/igb_ethdev.c:3914:9: note: in expansion of macro
‘E1000_WRITE_REG’
 3914 | E1000_WRITE_REG(hw, E1000_IMIREXT(filter->index), 0);
  | ^~~
../drivers/net/e1000/igb_ethdev.c:3914:29: note: in expansion of macro
‘E1000_IMIREXT’
 3914 | E1000_WRITE_REG(hw, E1000_IMIREXT(filter->index), 0);
  | ^
../drivers/net/e1000/igb_ethdev.c:3910:9: note: call to ‘rte_free’ here
 3910 | rte_free(filter);
  | ^~~~
../drivers/net/e1000/igb_ethdev.c:3913:46: warning: pointer ‘filter’ used after
‘rte_free’ [-Wuse-after-free]
 3913 | E1000_WRITE_REG(hw, E1000_IMIR(filter->index), 0);
../drivers/net/e1000/base/e1000_osdep.h:76:48: note: in definition of macro
‘E1000_PCI_REG_WRITE’
   76 | rte_write32((rte_cpu_to_le_32(value)), reg)
  |^~~
../drivers/net/e1000/base/e1000_osdep.h:121:29: note: in expansion of macro
‘E1000_PCI_REG_ADDR’
  121 | E1000_PCI_REG_WRITE(E1000_PCI_REG_ADDR((hw), (reg)), (value))
  | ^~
../drivers/net/e1000/igb_ethdev.c:3913:9: note: in expansion of macro
‘E1000_WRITE_REG’
 3913 | E1000_WRITE_REG(hw, E1000_IMIR(filter->index), 0);
  | ^~~
../drivers/net/e1000/igb_ethdev.c:3913:29: note: in expansion of macro
‘E1000_IMIR’
 3913 | E1000_WRITE_REG(hw, E1000_IMIR(filter->index), 0);
  | ^~
../drivers/net/e1000/igb_ethdev.c:3910:9: note: call to ‘rte_free’ here
 3910 | rte_free(filter);
  | ^~~~
../drivers/net/e1000/igb_ethdev.c:3912:46: warning: pointer ‘filter’ used after
‘rte_free’ [-Wuse-after-free]
 3912 | E1000_WRITE_REG(hw, E1000_TTQF(filter->index),
E1000_TTQF_DISABLE_MASK);
../drivers/net/e1000/base/e1000_osdep.h:76:48: note: in definition of macro
‘E1000_PCI_REG_WRITE’
   76 | rte_write32((rte_cpu_to_le_32(value)), reg)
  |^~~
../drivers/net/e1000/base/e1000_osdep.h:121:29: note: in expansion of macro
‘E1000_PCI_REG_ADDR’
  121 | E1000_PCI_REG_WRITE(E1000_PCI_REG_ADDR((hw), (reg)), (value))
  | ^~
../drivers/net/e1000/igb_ethdev.c:3912:9: note: in expansion of macro
‘E1000_WRITE_REG’
 3912 | E1000_WRITE_REG(hw, E1000_TTQF(filter->index),
E1000_TTQF_DISABLE_MASK);
  | ^~~
../drivers/net/e1000/igb_ethdev.c:3912:29: note: in expansion of macro
‘E1000_TTQF’
 3912 | E1000_WRITE_REG(hw, E1000_TTQF(filter->index),
E1000_TTQF_DISABLE_MASK);
  | ^~
../drivers/net/e1000/igb_ethdev.c:3910:9: note: call to ‘rte_free’ here
 3910 | rte_free(filter);
  | ^~~~
../drivers/net/e1000/igb_ethdev.c: In function
‘igb_delete_5tuple_filter_82576’:
../drivers/net/e1000/igb_ethdev.c:4359:49: warning: pointer ‘filter’ used after
‘rte_free’ [-Wuse-after-free]
 4359 | E1000_WRITE_REG(hw, E1000_IMIREXT(filter->index), 0);
../drivers/net/e1000/base/e1000_osdep.h:76:48: note: in definition of macro
‘E1000_PCI_REG_WRITE’
   76 | rte_write32((rte_cpu_to_le_32(value)), reg)
  |^~~
../drivers/net/e1000/base/e1000_osdep.h:121:29: note: in expansion of macro
‘E1000_PCI_REG_ADDR’
  121 | E

[DPDK/cryptodev Bug 1552] free miss match in cryptodev

[bugzilla]

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

Bug ID: 1552
   Summary: free miss match in cryptodev
   Product: DPDK
   Version: unspecified
  Hardware: All
OS: All
Status: UNCONFIRMED
  Severity: major
  Priority: Normal
 Component: cryptodev
  Assignee: dev@dpdk.org
  Reporter: step...@networkplumber.org
  Target Milestone: ---

If GCC function attributes are added to rte_malloc, then Gcc is able to
identify rte_malloc to free mismatches like:

[1319/2957] Compiling C object
drivers/libtmp_rte_crypto_bcmfs.a.p/crypto_bcmfs_bcmfs_device.c.o
In function ‘fsdev_allocate_one_dev’,
inlined from ‘bcmfs_vdev_probe’ at
../drivers/crypto/bcmfs/bcmfs_device.c:283:11:
../drivers/crypto/bcmfs/bcmfs_device.c:142:9: warning: ‘free’ called on pointer
returned from a mismatched allocation function [-Wmismatched-dealloc]
  142 | free(fsdev);
  | ^~~
../drivers/crypto/bcmfs/bcmfs_device.c:102:17: note: returned from ‘rte_calloc’
  102 | fsdev = rte_calloc(__func__, 1, sizeof(*fsdev), 0);
  | ^~
In function ‘fsdev_release’,
inlined from ‘bcmfs_vdev_probe’ at
../drivers/crypto/bcmfs/bcmfs_device.c:308:2:
../drivers/crypto/bcmfs/bcmfs_device.c:166:9: warning: ‘free’ called on pointer
returned from a mismatched allocation function [-Wmismatched-dealloc]
  166 | free(fsdev);
  | ^~~
In function ‘fsdev_allocate_one_dev’,
inlined from ‘bcmfs_vdev_probe’ at
../drivers/crypto/bcmfs/bcmfs_device.c:283:11:
../drivers/crypto/bcmfs/bcmfs_device.c:102:17: note: returned from ‘rte_calloc’
  102 | fsdev = rte_calloc(__func__, 1, sizeof(*fsdev), 0);
  |  
   ^~

-- 
You are receiving this mail because:
You are the assignee for the bug.

[DPDK/ethdev Bug 1551] use after free in Sfc driver

[bugzilla]

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

Bug ID: 1551
   Summary: use after free in Sfc driver
   Product: DPDK
   Version: unspecified
  Hardware: All
OS: All
Status: UNCONFIRMED
  Severity: normal
  Priority: Normal
 Component: ethdev
  Assignee: dev@dpdk.org
  Reporter: step...@networkplumber.org
  Target Milestone: ---

If GCC function attributes are added to rte_malloc, then it is able to spot use
after free in several places.

1255/2957] Compiling C object
drivers/libtmp_rte_net_sfc.a.p/net_sfc_sfc_flow_rss.c.o
In file included from ../drivers/net/sfc/sfc.h:28,
 from ../drivers/net/sfc/sfc_flow_rss.c:15:
../drivers/net/sfc/sfc_flow_rss.c: In function ‘sfc_flow_rss_ctx_del’:
../drivers/net/sfc/sfc_log.h:38:17: warning: pointer ‘ctx’ used after
‘rte_free’ [-Wuse-after-free]
   38 | rte_log(level, type,   
\
  |
^
   39 | RTE_FMT("%s" RTE_FMT_HEAD(__VA_ARGS__ ,) "\n", 
\
  |
~
   40 | _sas->log_prefix,  
\
  |
~
   41 | RTE_FMT_TAIL(__VA_ARGS__,)));  
\
  | 
../drivers/net/sfc/sfc_log.h:80:17: note: in expansion of macro ‘SFC_LOG’
   80 | SFC_LOG(_sa->priv.shared, RTE_LOG_DEBUG,   
\
  | ^~~
../drivers/net/sfc/sfc_flow_rss.c:308:9: note: in expansion of macro ‘sfc_dbg’
  308 | sfc_dbg(sa, "flow-rss: deleted ctx=%p", ctx);
  | ^~~
../drivers/net/sfc/sfc_flow_rss.c:306:9: note: call to ‘rte_free’ here
  306 | rte_free(ctx);
  | ^
[1262/2957] Compiling C object
drivers/libtmp_rte_net_sfc.a.p/net_sfc_sfc_mae.c.o
In file included from ../drivers/net/sfc/sfc.h:28,
 from ../drivers/net/sfc/sfc_mae.c:19:
../drivers/net/sfc/sfc_mae.c: In function ‘sfc_mae_encap_header_del’:
../drivers/net/sfc/sfc_log.h:38:17: warning: pointer ‘encap_header’ used after
‘rte_free’ [-Wuse-after-free]
   38 | rte_log(level, type,   
\
  |
^
   39 | RTE_FMT("%s" RTE_FMT_HEAD(__VA_ARGS__ ,) "\n", 
\
  |
~
   40 | _sas->log_prefix,  
\
  |
~
   41 | RTE_FMT_TAIL(__VA_ARGS__,)));  
\
  | 
../drivers/net/sfc/sfc_log.h:80:17: note: in expansion of macro ‘SFC_LOG’
   80 | SFC_LOG(_sa->priv.shared, RTE_LOG_DEBUG,   
\
  | ^~~
../drivers/net/sfc/sfc_mae.c:791:9: note: in expansion of macro ‘sfc_dbg’
  791 | sfc_dbg(sa, "deleted encap_header=%p", encap_header);
  | ^~~
../drivers/net/sfc/sfc_mae.c:789:9: note: call to ‘rte_free’ here
  789 | rte_free(encap_header);
  | ^~
../drivers/net/sfc/sfc_mae.c: In function ‘sfc_mae_mac_addr_del’:
../drivers/net/sfc/sfc_log.h:38:17: warning: pointer ‘mac_addr’ used after
‘rte_free’ [-Wuse-after-free]
   38 | rte_log(level, type,   
\
  |
^
   39 | RTE_FMT("%s" RTE_FMT_HEAD(__VA_ARGS__ ,) "\n", 
\
  |
~
   40 | _sas->log_prefix,  
\
  |
~
   41 | RTE_FMT_TAIL(__VA_ARGS__,)));  
\
  | 
../drivers/net/sfc/sfc_log.h:80:17: note: in expansion of macro ‘SFC_LOG’
   80 | SFC_LOG(_sa->priv.shared, RTE_LOG_DEBUG,   
\
  | ^~~
../drivers/net/sfc/sfc_mae.c:590:9: note: in expansion of macro ‘sfc_dbg’
  590 | sfc_dbg(sa, "deleted mac_addr=%p", mac_addr);
  | ^~~
../drivers/net/sfc/sfc_mae.c:588:9: note: call to ‘rte_free’ here
  588 | rte_free(mac_addr);
  | ^~
../drivers/net/sfc/sfc_mae.c: In function ‘sfc_mae_outer_rule_del’:
../drivers/net/sfc/sfc_log.h:38:17: warning: pointer ‘rule’ used after
‘r

[PATCH v4 0/1] dts: replace XML-RPC server

[jspewock]

From: Jeremy Spewock 

v4:
 * update naming scheme of methods in the scapy traffic generator and
   reorganized them
 * update doc-strings to add consistency and fix typos
 * add supoer().__init__() call to single_active_interactive_shell so
   that it and the traffic generator can be given in any order
 * added comments for additional clarity where needed

Jeremy Spewock (1):
  dts: Remove XML-RPC server for Scapy TG and instead use PythonShell

 .../single_active_interactive_shell.py|   8 +-
 .../traffic_generator/__init__.py |   2 +-
 .../testbed_model/traffic_generator/scapy.py  | 454 +++---
 .../traffic_generator/traffic_generator.py|  15 +-
 dts/framework/utils.py|  15 +
 5 files changed, 195 insertions(+), 299 deletions(-)

-- 
2.46.0

[PATCH v4 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell

[jspewock]

From: Jeremy Spewock 

Previously all scapy commands were handled using an XML-RPC server that
ran on the TGNode. This unnecessarily enforces a minimum Python version
of 3.10 on the server that is being used as a traffic generator and
complicates the implementation of scapy methods. This patch removes the
XML-RPC server completely and instead allows the Scapy TG to extend from
the PythonShell to implement the functionality of a traffic generator.
This is done by importing the Scapy library in the PythonShell and
sending commands directly to the interactive session on the TG Node.

Bugzilla ID: 1374

Signed-off-by: Jeremy Spewock 
---
 .../single_active_interactive_shell.py|   8 +-
 .../traffic_generator/__init__.py |   2 +-
 .../testbed_model/traffic_generator/scapy.py  | 454 +++---
 .../traffic_generator/traffic_generator.py|  15 +-
 dts/framework/utils.py|  15 +
 5 files changed, 195 insertions(+), 299 deletions(-)

diff --git a/dts/framework/remote_session/single_active_interactive_shell.py 
b/dts/framework/remote_session/single_active_interactive_shell.py
index 77a4dcefdf..f41d729655 100644
--- a/dts/framework/remote_session/single_active_interactive_shell.py
+++ b/dts/framework/remote_session/single_active_interactive_shell.py
@@ -36,9 +36,10 @@
 from framework.params import Params
 from framework.settings import SETTINGS
 from framework.testbed_model.node import Node
+from framework.utils import MultiInheritanceBaseClass
 
 
-class SingleActiveInteractiveShell(ABC):
+class SingleActiveInteractiveShell(MultiInheritanceBaseClass, ABC):
 """The base class for managing interactive shells.
 
 This class shouldn't be instantiated directly, but instead be extended. It 
contains
@@ -93,9 +94,13 @@ def __init__(
 timeout: float = SETTINGS.timeout,
 app_params: Params = Params(),
 name: str | None = None,
+**kwargs,
 ) -> None:
 """Create an SSH channel during initialization.
 
+Additional key-word arguments can be passed through `kwargs` if needed 
for fulfilling other
+constructors in the case of multiple-inheritance.
+
 Args:
 node: The node on which to run start the interactive shell.
 privileged: Enables the shell to run as superuser.
@@ -115,6 +120,7 @@ def __init__(
 self._timeout = timeout
 # Ensure path is properly formatted for the host
 self._update_real_path(self.path)
+super().__init__(node, **kwargs)
 
 def _setup_ssh_channel(self):
 self._ssh_channel = 
self._node.main_session.interactive_session.session.invoke_shell()
diff --git a/dts/framework/testbed_model/traffic_generator/__init__.py 
b/dts/framework/testbed_model/traffic_generator/__init__.py
index 6dac86a224..a319fa5320 100644
--- a/dts/framework/testbed_model/traffic_generator/__init__.py
+++ b/dts/framework/testbed_model/traffic_generator/__init__.py
@@ -36,7 +36,7 @@ def create_traffic_generator(
 """
 match traffic_generator_config:
 case ScapyTrafficGeneratorConfig():
-return ScapyTrafficGenerator(tg_node, traffic_generator_config)
+return ScapyTrafficGenerator(tg_node, traffic_generator_config, 
privileged=True)
 case _:
 raise ConfigurationError(
 f"Unknown traffic generator: 
{traffic_generator_config.traffic_generator_type}"
diff --git a/dts/framework/testbed_model/traffic_generator/scapy.py 
b/dts/framework/testbed_model/traffic_generator/scapy.py
index 13fc1107aa..07f11b7f78 100644
--- a/dts/framework/testbed_model/traffic_generator/scapy.py
+++ b/dts/framework/testbed_model/traffic_generator/scapy.py
@@ -6,311 +6,150 @@
 
 A traffic generator used for functional testing, implemented with
 `the Scapy library `_.
-The traffic generator uses an XML-RPC server to run Scapy on the remote TG 
node.
+The traffic generator uses an interactive shell to run Scapy on the remote TG 
node.
 
-The traffic generator uses the :mod:`xmlrpc.server` module to run an XML-RPC 
server
-in an interactive remote Python SSH session. The communication with the server 
is facilitated
-with a local server proxy from the :mod:`xmlrpc.client` module.
+The traffic generator extends 
:class:`framework.remote_session.python_shell.PythonShell` to
+implement the methods for handling packets by sending commands into the 
interactive shell.
 """
 
-import inspect
-import marshal
+
+import re
 import time
-import types
-import xmlrpc.client
-from xmlrpc.server import SimpleXMLRPCServer
+from typing import ClassVar
 
-import scapy.all  # type: ignore[import-untyped]
+from scapy.compat import base64_bytes  # type: ignore[import-untyped]
 from scapy.layers.l2 import Ether  # type: ignore[import-untyped]
 from scapy.packet import Packet  # type: ignore[import-untyped]
 
 from framework.config import OS, ScapyTrafficGeneratorConfig
 from framework.remote

Re: [PATCH v2 1/1] dts: add send_packets to test suites and rework packet addressing

[Jeremy Spewock]

On Tue, Sep 24, 2024 at 10:30 AM Juraj Linkeš
 wrote:
>
>
>
> On 20. 9. 2024 20:08, jspew...@iol.unh.edu wrote:
> > From: Jeremy Spewock 
> >
> > Currently the only method provided in the test suite class for sending
> > packets sends a single packet and then captures the results. There is,
> > in some cases, a need to send multiple packets at once while not really
> > needing to capture any traffic received back. The method to do this
> > exists in the traffic generator already, but this patch exposes the
> > method to test suites.
> >
>
> Patrick mentioned there is now a method that does it
> (send_packets_and_capture()), but that one captures the packets. I think
> we should add this method though, as it does something different, but
> maybe the most important point is that non-capturing TGs are going to
> support send_packets_and_capture(), so we need it either way.
>
> > This patch also updates the _adjust_addresses method of test suites so
> > that addresses of packets are only modified if the developer did not
> > configure them beforehand. This allows for developers to have more
> > control over the content of their packets when sending them through the
> > framework.
> >
>
> I think this could and should be split into two patches. They aren't
> really connected.

Sure, this makes sense. They were in the same one originally when this
was tied to the test suite I was writing it for because they were just
smaller framework updates that allowed the suite to function. Probably
didn't make much sense to keep them together then, but it makes even
less sense when the patch is standalone like this.

>
>
> > diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
>
> > @@ -243,41 +255,74 @@ def get_expected_packet(self, packet: Packet) -> 
> > Packet:
> >   Returns:
> >   `packet` with injected L2/L3 addresses.
> >   """
> > -return self._adjust_addresses(packet, expected=True)
> > +return self._adjust_addresses([packet], expected=True)[0]
> >
> > -def _adjust_addresses(self, packet: Packet, expected: bool = False) -> 
> > Packet:
> > +def _adjust_addresses(self, packets: list[Packet], expected: bool = 
> > False) -> list[Packet]:
> >   """L2 and L3 address additions in both directions.
> >
> > +Packets in `packets` will be directly modified in this method. The 
> > returned list of packets
> > +however will be copies of the modified packets in order to keep 
> > the two lists distinct.
> > +
>
> Do we need to do this? I guess you needed this in a test suite - what is
> the reason?

This is actually just me documenting something that was already
happening behind the scenes in this method that I thought would be
helpful for people to know. Right now this method ends in
`Ether(packet.build())` which essentially just copies the packet, but
it does so after the modifications were made. I think there is some
logical reason to do this which is this method is called internally in
the framework, so it allows the send methods where these packets are
used to essentially modify the list and do whatever they want to/need
to to get the packets sent and the only change that is reflected on
the user is the addresses are added.

>
> > +Only missing addresses are added to packets, existing addresses 
> > will not be overridden. If
> > +any packet in `packets` has multiple IP layers (using GRE, for 
> > example) only the inner-most
> > +IP layer will have its addresses adjusted.
> > +
> >   Assumptions:
> >   Two links between SUT and TG, one link is TG -> SUT, the 
> > other SUT -> TG.
> >
> >   Args:
> > -packet: The packet to modify.
> > +packets: The packets to modify.
> >   expected: If :data:`True`, the direction is SUT -> TG,
> >   otherwise the direction is TG -> SUT.
> > +
> > +Returns:
> > +A list containing copies of all packets in `packets` after 
> > modification.
> >   """
> > -if expected:
> > -# The packet enters the TG from SUT
> > -# update l2 addresses
> > -packet.src = self._sut_port_egress.mac_address
> > -packet.dst = self._tg_port_ingress.mac_address
> > +ret_packets = []
> > +for packet in packets:
> > +# The fields parameter of a packet does not include fields of 
> > the payload, so this can
> > +# only be the Ether src/dst.
> > +pkt_src_is_unset = "src" not in packet.fields
> > +pkt_dst_is_unset = "dst" not in packet.fields
> > +num_ip_layers = packet.layers().count(IP)
> > +
> > +# Update the last IP layer if there are multiple to account 
> > for GRE addressing (the
>
> This comment should be more generic as it's going to apply to other
> things than just GRE.

Ack.

>
> > +# framework should be modifying the packet address 

Re: [dpdk-dev] [PATCH v5 4/4] app: hook in EAL usage help

[Stephen Hemminger]

On Mon,  5 Apr 2021 21:39:54 +0200
Thomas Monjalon  wrote:

> From: Thomas Monjalon 
> To: dev@dpdk.org
> Cc: david.march...@redhat.com, Wisam Jaddo ,  Bruce 
> Richardson ,  Andrew Rybchenko 
> ,  Reshma Pattan ,  
> Maryam Tahhan ,  Konstantin Ananyev 
> ,  Nicolas Chautru , 
>  Declan Doherty ,  Ciara Power 
> ,  Vladimir Medvedkin ,  
> Xiaoyun Li , Ori Kam ,  Bernard 
> Iremonger 
> Subject: [dpdk-dev] [PATCH v5 4/4] app: hook in EAL usage help
> Date: Mon,  5 Apr 2021 21:39:54 +0200
> Sender: "dev" 
> X-Mailer: git-send-email 2.31.1
> 
> Use rte_set_application_usage_hook() in the test applications,
> so the full help including EAL options can be printed in one go
> with the EAL option -h or --help.
> 
> Signed-off-by: Thomas Monjalon 
> Acked-by: Wisam Jaddo 
> Acked-by: Bruce Richardson 
> Acked-by: Andrew Rybchenko 

The patch makes sense, but no longer applies.
It needs to be rebased.

[PATCH v2 0/7] ethdev: jump to table support

[Alexander Kozyrev]

Introduce new Flow API JUMP_TO_TABLE_INDEX action.
It allows bypassing a hierarchy of groups and going directly
to a specified flow table. That gives a user the flexibility
to jump between different priorities in a group and eliminates
the need to do a table lookup in the group hierarchy.
The JUMP_TO_TABLE_INDEX action forwards a packet to the
specified rule index inside the index-based flow table.

The current index-based flow table doesn't do any matching
on the packet and executes the actions immediately.
Add a new index-based flow table with pattern matching.
The JUMP_TO_TABLE_INDEX can redirect a packet to another
matching criteria at the specified index in this case.

RFC: 
https://patchwork.dpdk.org/project/dpdk/patch/20240822202753.3856703-1-akozy...@nvidia.com/
v2: added trace point to flow insertion by index functions.

Alexander Kozyrev (7):
  ethdev: add insertion by index with pattern
  app/testpmd: add index with pattern insertion type
  ethdev: add flow rule insertion by index with pattern
  app/testpmd: add insertion by index with pattern option
  ethdev: add jump to table index action
  app/testpmd: add jump to table index action
  ethdev: add trace points to flow insertion by index

 app/test-pmd/cmdline_flow.c| 44 +-
 app/test-pmd/config.c  | 22 +--
 app/test-pmd/testpmd.h |  2 +-
 doc/guides/prog_guide/rte_flow.rst | 20 +++
 doc/guides/rel_notes/release_24_11.rst | 13 +
 lib/ethdev/ethdev_trace.h  | 44 ++
 lib/ethdev/ethdev_trace_points.c   |  6 ++
 lib/ethdev/rte_flow.c  | 72 ++-
 lib/ethdev/rte_flow.h  | 81 ++
 lib/ethdev/rte_flow_driver.h   | 14 +
 lib/ethdev/version.map |  1 +
 11 files changed, 309 insertions(+), 10 deletions(-)

-- 
2.18.2

[PATCH v2 1/7] ethdev: add insertion by index with pattern

[Alexander Kozyrev]

There are two flow table rules insertion type today:
pattern-based insertion when packets match on the pattern and
index-based insertion when packets always hit at the index.
We need another mode that allows to match on the pattern at
the index: insertion by index with pattern.

Signed-off-by: Alexander Kozyrev 
---
 doc/guides/rel_notes/release_24_11.rst | 4 
 lib/ethdev/rte_flow.h  | 4 
 2 files changed, 8 insertions(+)

diff --git a/doc/guides/rel_notes/release_24_11.rst 
b/doc/guides/rel_notes/release_24_11.rst
index ef0124a9e6..8d311aead2 100644
--- a/doc/guides/rel_notes/release_24_11.rst
+++ b/doc/guides/rel_notes/release_24_11.rst
@@ -55,6 +55,10 @@ New Features
  Also, make sure to start the actual text at the margin.
  ===
 
+* **Added a new insertion by index with pattern table insertion type.**
+
+  Extended rte_flow_table_insertion_type enum with new
+  RTE_FLOW_TABLE_INSERTION_TYPE_INDEX_WITH_PATTERN type.
 
 Removed Items
 -
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index f864578f80..6f30dd7ae9 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -5898,6 +5898,10 @@ enum rte_flow_table_insertion_type {
 * Index-based insertion.
 */
RTE_FLOW_TABLE_INSERTION_TYPE_INDEX,
+   /**
+* Index-based insertion with pattern.
+*/
+   RTE_FLOW_TABLE_INSERTION_TYPE_INDEX_WITH_PATTERN,
 };
 
 /**
-- 
2.18.2

[PATCH v2 2/7] app/testpmd: add index with pattern insertion type

[Alexander Kozyrev]

Provide index_with_pattern command line option
for the template table insertion type.

flow template_table 0 create table_id 2 group 13 priority 0
  insertion_type index_with_pattern ingress rules_number 64
  pattern_template 2 actions_template 2

Signed-off-by: Alexander Kozyrev 
---
 app/test-pmd/cmdline_flow.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 858f4077bd..b048821e85 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -1029,7 +1029,7 @@ static const char *const meter_colors[] = {
 };
 
 static const char *const table_insertion_types[] = {
-   "pattern", "index", NULL
+   "pattern", "index", "index_with_pattern", NULL
 };
 
 static const char *const table_hash_funcs[] = {
-- 
2.18.2

[PATCH v2 4/7] app/testpmd: add insertion by index with pattern option

[Alexander Kozyrev]

Allow to specify both the rule index and the pattern
in the flow rule creation command line parameters.
Both are needed for rte_flow_async_create_by_index_with_pattern().

flow queue 0 create 0 template_table 2 rule_index 5
  pattern_template 0 actions_template 0 postpone no pattern eth / end
  actions count / queue index 1 / end

Signed-off-by: Alexander Kozyrev 
---
 app/test-pmd/cmdline_flow.c |  8 +++-
 app/test-pmd/config.c   | 22 --
 app/test-pmd/testpmd.h  |  2 +-
 3 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index b048821e85..65030936d2 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -1583,6 +1583,12 @@ static const enum index next_async_insert_subcmd[] = {
ZERO,
 };
 
+static const enum index next_async_pattern_subcmd[] = {
+   QUEUE_PATTERN_TEMPLATE,
+   QUEUE_ACTIONS_TEMPLATE,
+   ZERO,
+};
+
 static const enum index item_param[] = {
ITEM_PARAM_IS,
ITEM_PARAM_SPEC,
@@ -3786,7 +3792,7 @@ static const struct token token_list[] = {
[QUEUE_RULE_ID] = {
.name = "rule_index",
.help = "specify flow rule index",
-   .next = NEXT(NEXT_ENTRY(QUEUE_ACTIONS_TEMPLATE),
+   .next = NEXT(next_async_pattern_subcmd,
 NEXT_ENTRY(COMMON_UNSIGNED)),
.args = ARGS(ARGS_ENTRY(struct buffer,
args.vc.rule_id)),
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 6f0beafa27..39924d8da9 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -2636,8 +2636,8 @@ port_flow_template_table_create(portid_t port_id, 
uint32_t id,
}
pt->nb_pattern_templates = nb_pattern_templates;
pt->nb_actions_templates = nb_actions_templates;
-   rte_memcpy(&pt->flow_attr, &table_attr->flow_attr,
-  sizeof(struct rte_flow_attr));
+   rte_memcpy(&pt->attr, table_attr,
+  sizeof(struct rte_flow_template_table_attr));
printf("Template table #%u created\n", pt->id);
return 0;
 }
@@ -2835,7 +2835,7 @@ port_queue_flow_create(portid_t port_id, queueid_t 
queue_id,
}
job->type = QUEUE_JOB_TYPE_FLOW_CREATE;
 
-   pf = port_flow_new(&pt->flow_attr, pattern, actions, &error);
+   pf = port_flow_new(&pt->attr.flow_attr, pattern, actions, &error);
if (!pf) {
free(job);
return port_flow_complain(&error);
@@ -2846,12 +2846,22 @@ port_queue_flow_create(portid_t port_id, queueid_t 
queue_id,
}
/* Poisoning to make sure PMDs update it in case of error. */
memset(&error, 0x11, sizeof(error));
-   if (rule_idx == UINT32_MAX)
+   if (pt->attr.insertion_type == RTE_FLOW_TABLE_INSERTION_TYPE_PATTERN)
flow = rte_flow_async_create(port_id, queue_id, &op_attr, 
pt->table,
pattern, pattern_idx, actions, actions_idx, job, 
&error);
-   else
+   else if (pt->attr.insertion_type == RTE_FLOW_TABLE_INSERTION_TYPE_INDEX)
flow = rte_flow_async_create_by_index(port_id, queue_id, 
&op_attr, pt->table,
rule_idx, actions, actions_idx, job, &error);
+   else if (pt->attr.insertion_type == 
RTE_FLOW_TABLE_INSERTION_TYPE_INDEX_WITH_PATTERN)
+   flow = rte_flow_async_create_by_index_with_pattern(port_id, 
queue_id, &op_attr,
+   pt->table, rule_idx, pattern, pattern_idx, actions, 
actions_idx, job,
+   &error);
+   else {
+   free(pf);
+   free(job);
+   printf("Insertion type %d is invalid\n", 
pt->attr.insertion_type);
+   return -EINVAL;
+   }
if (!flow) {
free(pf);
free(job);
@@ -3060,7 +3070,7 @@ port_queue_flow_update(portid_t port_id, queueid_t 
queue_id,
}
job->type = QUEUE_JOB_TYPE_FLOW_UPDATE;
 
-   uf = port_flow_new(&pt->flow_attr, pf->rule.pattern_ro, actions, 
&error);
+   uf = port_flow_new(&pt->attr.flow_attr, pf->rule.pattern_ro, actions, 
&error);
if (!uf) {
free(job);
return port_flow_complain(&error);
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 9facd7f281..f9ab88d667 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -220,7 +220,7 @@ struct port_table {
uint32_t id; /**< Table ID. */
uint32_t nb_pattern_templates; /**< Number of pattern templates. */
uint32_t nb_actions_templates; /**< Number of actions templates. */
-   struct rte_flow_attr flow_attr; /**< Flow attributes. */
+   struct rte_flow_template_table_attr attr; /**< Table attributes. */
struct rte_flow_template_table *table; /**< PMD opaque template object 
*/
 };
 
-- 
2.18.2

[PATCH v2 5/7] ethdev: add jump to table index action

[Alexander Kozyrev]

Introduce the RTE_FLOW_ACTION_TYPE_JUMP_TO_TABLE_INDEX action.
It redirects packets to a particular index in a flow table.

Signed-off-by: Alexander Kozyrev 
---
 doc/guides/rel_notes/release_24_11.rst |  4 
 lib/ethdev/rte_flow.c  |  1 +
 lib/ethdev/rte_flow.h  | 23 +++
 3 files changed, 28 insertions(+)

diff --git a/doc/guides/rel_notes/release_24_11.rst 
b/doc/guides/rel_notes/release_24_11.rst
index fd461128ee..67196c099a 100644
--- a/doc/guides/rel_notes/release_24_11.rst
+++ b/doc/guides/rel_notes/release_24_11.rst
@@ -65,6 +65,10 @@ New Features
   Added API for inserting the rule by index with pattern.
   Introduced rte_flow_async_create_by_index_with_pattern() function.
 
+* **Added the action to redirect packets to a particular index in a flow 
table.**
+
+  Introduced RTE_FLOW_ACTION_TYPE_JUMP_TO_TABLE_INDEX action type.
+
 Removed Items
 -
 
diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
index f6259343e8..a56391b156 100644
--- a/lib/ethdev/rte_flow.c
+++ b/lib/ethdev/rte_flow.c
@@ -275,6 +275,7 @@ static const struct rte_flow_desc_data 
rte_flow_desc_action[] = {
MK_FLOW_ACTION(PROG,
   sizeof(struct rte_flow_action_prog)),
MK_FLOW_ACTION(NAT64, sizeof(struct rte_flow_action_nat64)),
+   MK_FLOW_ACTION(JUMP_TO_TABLE_INDEX, sizeof(struct 
rte_flow_action_jump_to_table_index)),
 };
 
 int
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index 84473241fb..a2929438bf 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -3262,6 +3262,15 @@ enum rte_flow_action_type {
 * @see struct rte_flow_action_nat64
 */
RTE_FLOW_ACTION_TYPE_NAT64,
+
+   /**
+* RTE_FLOW_ACTION_TYPE_JUMP_TO_TABLE_INDEX,
+*
+* Redirects packets to a particular index in a flow table.
+*
+* @see struct rte_flow_action_jump_to_table_index.
+*/
+   RTE_FLOW_ACTION_TYPE_JUMP_TO_TABLE_INDEX,
 };
 
 /**
@@ -4266,6 +4275,20 @@ rte_flow_dynf_metadata_set(struct rte_mbuf *m, uint32_t 
v)
*RTE_FLOW_DYNF_METADATA(m) = v;
 }
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this structure may change without prior notice
+ *
+ * RTE_FLOW_ACTION_TYPE_JUMP_TO_TABLE_INDEX
+ *
+ * Redirects packets to a particular index in a flow table.
+ *
+ */
+struct rte_flow_action_jump_to_table_index {
+   struct rte_flow_template_table *table;
+   uint32_t index;
+};
+
 /**
  * Definition of a single action.
  *
-- 
2.18.2

[PATCH v2 3/7] ethdev: add flow rule insertion by index with pattern

[Alexander Kozyrev]

Add a new API to enqueue flow rule creation by index with pattern.
The new template table rules insertion type,
index-based insertion with pattern, requires a new flow rule creation
function with both rule index and pattern provided.
Packets will match on the provided pattern at the provided index.

Signed-off-by: Alexander Kozyrev 
---
 doc/guides/prog_guide/rte_flow.rst | 20 ++
 doc/guides/rel_notes/release_24_11.rst |  5 +++
 lib/ethdev/rte_flow.c  | 55 ++
 lib/ethdev/rte_flow.h  | 54 +
 lib/ethdev/rte_flow_driver.h   | 14 +++
 lib/ethdev/version.map |  1 +
 6 files changed, 149 insertions(+)

diff --git a/doc/guides/prog_guide/rte_flow.rst 
b/doc/guides/prog_guide/rte_flow.rst
index dad588763f..adbd9b1c20 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -4156,6 +4156,26 @@ Enqueueing a flow rule creation operation to insert a 
rule at a table index.
 A valid handle in case of success is returned. It must be destroyed later
 by calling ``rte_flow_async_destroy()`` even if the rule is rejected by HW.
 
+Enqueue creation by index with pattern
+~~
+
+Enqueueing a flow rule creation operation to insert a rule at a table index 
with pattern.
+
+.. code-block:: c
+
+   struct rte_flow *
+   rte_flow_async_create_by_index_with_pattern(uint16_t port_id,
+   uint32_t queue_id,
+   const struct rte_flow_op_attr 
*op_attr,
+   struct rte_flow_template_table 
*template_table,
+   uint32_t rule_index,
+   const struct rte_flow_item 
pattern[],
+   uint8_t pattern_template_index,
+   const struct rte_flow_action 
actions[],
+   uint8_t actions_template_index,
+   void *user_data,
+   struct rte_flow_error *error);
+
 Enqueue destruction operation
 ~
 
diff --git a/doc/guides/rel_notes/release_24_11.rst 
b/doc/guides/rel_notes/release_24_11.rst
index 8d311aead2..fd461128ee 100644
--- a/doc/guides/rel_notes/release_24_11.rst
+++ b/doc/guides/rel_notes/release_24_11.rst
@@ -60,6 +60,11 @@ New Features
   Extended rte_flow_table_insertion_type enum with new
   RTE_FLOW_TABLE_INSERTION_TYPE_INDEX_WITH_PATTERN type.
 
+* **Added flow rule insertion by index with pattern to the Flow API.**
+
+  Added API for inserting the rule by index with pattern.
+  Introduced rte_flow_async_create_by_index_with_pattern() function.
+
 Removed Items
 -
 
diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
index 4076ae4ee1..f6259343e8 100644
--- a/lib/ethdev/rte_flow.c
+++ b/lib/ethdev/rte_flow.c
@@ -2109,6 +2109,42 @@ rte_flow_async_create_by_index(uint16_t port_id,
   user_data, error);
 }
 
+struct rte_flow *
+rte_flow_async_create_by_index_with_pattern(uint16_t port_id,
+   uint32_t queue_id,
+   const struct rte_flow_op_attr 
*op_attr,
+   struct rte_flow_template_table 
*template_table,
+   uint32_t rule_index,
+   const struct rte_flow_item 
pattern[],
+   uint8_t pattern_template_index,
+   const struct rte_flow_action 
actions[],
+   uint8_t actions_template_index,
+   void *user_data,
+   struct rte_flow_error *error)
+{
+   struct rte_eth_dev *dev = &rte_eth_devices[port_id];
+
+#ifdef RTE_FLOW_DEBUG
+   if (!rte_eth_dev_is_valid_port(port_id)) {
+   rte_flow_error_set(error, ENODEV, 
RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
+  rte_strerror(ENODEV));
+   return NULL;
+   }
+   if (dev->flow_fp_ops == NULL ||
+   dev->flow_fp_ops->async_create_by_index_with_pattern == NULL) {
+   rte_flow_error_set(error, ENOSYS, 
RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
+  rte_strerror(ENOSYS));
+   return NULL;
+   }
+#endif
+
+   return dev->flow_fp_ops->async_create_by_index_with_pattern(dev, 
queue_id, op_attr,
+   
template_table, rule_index,
+   pattern, 
patte

[PATCH v2 7/7] ethdev: add trace points to flow insertion by index

[Alexander Kozyrev]

Adds trace points for rte_flow rule insertion by index functions:
rte_flow_async_create_by_index and
rte_flow_async_create_by_index_with_pattern.

Signed-off-by: Alexander Kozyrev 
---
 lib/ethdev/ethdev_trace.h| 44 
 lib/ethdev/ethdev_trace_points.c |  6 +
 lib/ethdev/rte_flow.c| 18 +++--
 3 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/lib/ethdev/ethdev_trace.h b/lib/ethdev/ethdev_trace.h
index 3bec87bfdb..910bedbebd 100644
--- a/lib/ethdev/ethdev_trace.h
+++ b/lib/ethdev/ethdev_trace.h
@@ -2343,6 +2343,50 @@ RTE_TRACE_POINT_FP(
rte_trace_point_emit_ptr(flow);
 )
 
+RTE_TRACE_POINT_FP(
+   rte_flow_trace_async_create_by_index,
+   RTE_TRACE_POINT_ARGS(uint16_t port_id, uint32_t queue_id,
+   const struct rte_flow_op_attr *op_attr,
+   const struct rte_flow_template_table *template_table,
+   uint32_t rule_index,
+   const struct rte_flow_action *actions,
+   uint8_t actions_template_index,
+   const void *user_data, const struct rte_flow *flow),
+   rte_trace_point_emit_u16(port_id);
+   rte_trace_point_emit_u32(queue_id);
+   rte_trace_point_emit_ptr(op_attr);
+   rte_trace_point_emit_ptr(template_table);
+   rte_trace_point_emit_u32(rule_index);
+   rte_trace_point_emit_ptr(actions);
+   rte_trace_point_emit_u8(actions_template_index);
+   rte_trace_point_emit_ptr(user_data);
+   rte_trace_point_emit_ptr(flow);
+)
+
+RTE_TRACE_POINT_FP(
+   rte_flow_trace_async_create_by_index_with_pattern,
+   RTE_TRACE_POINT_ARGS(uint16_t port_id, uint32_t queue_id,
+   const struct rte_flow_op_attr *op_attr,
+   const struct rte_flow_template_table *template_table,
+   uint32_t rule_index,
+   const struct rte_flow_item *pattern,
+   uint8_t pattern_template_index,
+   const struct rte_flow_action *actions,
+   uint8_t actions_template_index,
+   const void *user_data, const struct rte_flow *flow),
+   rte_trace_point_emit_u16(port_id);
+   rte_trace_point_emit_u32(queue_id);
+   rte_trace_point_emit_ptr(op_attr);
+   rte_trace_point_emit_ptr(template_table);
+   rte_trace_point_emit_u32(rule_index);
+   rte_trace_point_emit_ptr(pattern);
+   rte_trace_point_emit_u8(pattern_template_index);
+   rte_trace_point_emit_ptr(actions);
+   rte_trace_point_emit_u8(actions_template_index);
+   rte_trace_point_emit_ptr(user_data);
+   rte_trace_point_emit_ptr(flow);
+)
+
 RTE_TRACE_POINT_FP(
rte_flow_trace_async_destroy,
RTE_TRACE_POINT_ARGS(uint16_t port_id, uint32_t queue_id,
diff --git a/lib/ethdev/ethdev_trace_points.c b/lib/ethdev/ethdev_trace_points.c
index 6ecbee289b..902e4f7533 100644
--- a/lib/ethdev/ethdev_trace_points.c
+++ b/lib/ethdev/ethdev_trace_points.c
@@ -589,6 +589,12 @@ 
RTE_TRACE_POINT_REGISTER(rte_flow_trace_template_table_destroy,
 RTE_TRACE_POINT_REGISTER(rte_flow_trace_async_create,
lib.ethdev.flow.async_create)
 
+RTE_TRACE_POINT_REGISTER(rte_flow_trace_async_create_by_index,
+   lib.ethdev.flow.async_create_by_index)
+
+RTE_TRACE_POINT_REGISTER(rte_flow_trace_async_create_by_index_with_pattern,
+   lib.ethdev.flow.async_create_by_index_with_pattern)
+
 RTE_TRACE_POINT_REGISTER(rte_flow_trace_async_destroy,
lib.ethdev.flow.async_destroy)
 
diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
index a56391b156..4a7735b5ab 100644
--- a/lib/ethdev/rte_flow.c
+++ b/lib/ethdev/rte_flow.c
@@ -2090,6 +2090,7 @@ rte_flow_async_create_by_index(uint16_t port_id,
   struct rte_flow_error *error)
 {
struct rte_eth_dev *dev = &rte_eth_devices[port_id];
+   struct rte_flow *flow;
 
 #ifdef RTE_FLOW_DEBUG
if (!rte_eth_dev_is_valid_port(port_id)) {
@@ -2104,10 +2105,15 @@ rte_flow_async_create_by_index(uint16_t port_id,
}
 #endif
 
-   return dev->flow_fp_ops->async_create_by_index(dev, queue_id,
+   flow = dev->flow_fp_ops->async_create_by_index(dev, queue_id,
   op_attr, template_table, 
rule_index,
   actions, 
actions_template_index,
   user_data, error);
+
+   rte_flow_trace_async_create_by_index(port_id, queue_id, op_attr, 
template_table, rule_index,
+actions, actions_template_index, 
user_data, flow);
+
+   return flow;
 }
 
 struct rte_flow *
@@ -2124,6 +2130,7 @@ rte_flow_async_create_by_index_with_pattern(uint16_t 
port_id,
struct rte_flow_error *error)
 {
struct rte_eth_dev *dev = &rte_eth_devices[port_id];
+   struct rte_flow *flow;
 
 #ifdef RTE_FLOW_DEBUG
if (!rte_eth_dev_is_valid_p

[PATCH v2 6/7] app/testpmd: add jump to table index action

[Alexander Kozyrev]

Add a new command line options to create the
RTE_FLOW_ACTION_TYPE_JUMP_TO_TABLE_INDEX action
from the testpmd command line.

flow queue 0 create 0 template_table 0 pattern_template 0
  actions_template 0 postpone no pattern eth / end
  actions jump_to_table_index table 0x166f9ce00 index 5 / end

Signed-off-by: Alexander Kozyrev 
---
 app/test-pmd/cmdline_flow.c | 34 ++
 1 file changed, 34 insertions(+)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 65030936d2..d150f6cca8 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -784,6 +784,9 @@ enum index {
ACTION_IPV6_EXT_PUSH_INDEX_VALUE,
ACTION_NAT64,
ACTION_NAT64_MODE,
+   ACTION_JUMP_TO_TABLE_INDEX,
+   ACTION_JUMP_TO_TABLE_INDEX_TABLE,
+   ACTION_JUMP_TO_TABLE_INDEX_INDEX,
 };
 
 /** Maximum size for pattern in struct rte_flow_item_raw. */
@@ -2326,6 +2329,7 @@ static const enum index next_action[] = {
ACTION_IPV6_EXT_REMOVE,
ACTION_IPV6_EXT_PUSH,
ACTION_NAT64,
+   ACTION_JUMP_TO_TABLE_INDEX,
ZERO,
 };
 
@@ -2686,6 +2690,13 @@ static const enum index next_hash_encap_dest_subcmd[] = {
ZERO,
 };
 
+static const enum index action_jump_to_table_index[] = {
+   ACTION_JUMP_TO_TABLE_INDEX_TABLE,
+   ACTION_JUMP_TO_TABLE_INDEX_INDEX,
+   ACTION_NEXT,
+   ZERO,
+};
+
 static int parse_set_raw_encap_decap(struct context *, const struct token *,
 const char *, unsigned int,
 void *, unsigned int);
@@ -7597,6 +7608,29 @@ static const struct token token_list[] = {
.args = ARGS(ARGS_ENTRY(struct rte_flow_action_nat64, type)),
.call = parse_vc_conf,
},
+   [ACTION_JUMP_TO_TABLE_INDEX] = {
+   .name = "jump_to_table_index",
+   .help = "Jump to table index",
+   .priv = PRIV_ACTION(JUMP_TO_TABLE_INDEX,
+   sizeof(struct 
rte_flow_action_jump_to_table_index)),
+   .next = NEXT(action_jump_to_table_index),
+   .call = parse_vc,
+   },
+   [ACTION_JUMP_TO_TABLE_INDEX_TABLE] = {
+   .name = "table",
+   .help = "table to redirect traffic to",
+   .next = NEXT(action_jump_to_table_index, 
NEXT_ENTRY(COMMON_UNSIGNED)),
+   .args = ARGS(ARGS_ENTRY(struct 
rte_flow_action_jump_to_table_index, table)),
+   .call = parse_vc_conf,
+   },
+   [ACTION_JUMP_TO_TABLE_INDEX_INDEX] = {
+   .name = "index",
+   .help = "rule index to redirect traffic to",
+   .next = NEXT(action_jump_to_table_index, 
NEXT_ENTRY(COMMON_UNSIGNED)),
+   .args = ARGS(ARGS_ENTRY(struct 
rte_flow_action_jump_to_table_index, index)),
+   .call = parse_vc_conf,
+   },
+
/* Top level command. */
[SET] = {
.name = "set",
-- 
2.18.2

Re: [PATCH v3] dts: add flow rule dataclass to testpmd shell

[Juraj Linkeš]

On 13. 8. 2024 16:41, Dean Marx wrote:

add dataclass for passing in flow rule creation arguments, as well as a


Capitalize please.


__str__ method for converting to a sendable testpmd command. Add
flow_create method to TestPmdShell class for initializing flow rules.

Signed-off-by: Dean Marx 
---
  dts/framework/remote_session/testpmd_shell.py | 57 +++
  1 file changed, 57 insertions(+)

diff --git a/dts/framework/remote_session/testpmd_shell.py 
b/dts/framework/remote_session/testpmd_shell.py
index 43e9f56517..17b9c7811d 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -577,6 +577,44 @@ class TestPmdPortStats(TextParser):
  tx_bps: int = field(metadata=TextParser.find_int(r"Tx-bps:\s+(\d+)"))
  
  
+@dataclass

+class FlowRule:
+"""Dataclass for setting flow rule parameters."""
+
+#:
+port_id: int
+#:
+ingress: bool
+#:
+pattern: str
+#:
+actions: str
+
+#:
+group_id: int | None = None
+#:
+priority_level: int | None = None
+#:
+user_id: int | None = None
+
+def __str__(self) -> str:
+"""Returns the string representation of a flow_func instance.
+
+In this case, a properly formatted flow create command that can be 
sent to testpmd.


I think it would be beneficial for a complicated command like this to 
add the actual format of the command as returned by testpmd, which 
should be (according to app/test-pmd/cmdline.c):


flow create {port_id}
 [group {group_id}] [priority {level}]
 [ingress] [egress]
 pattern {item} [/ {item} [...]] / end
 actions {action} [/ {action} [...]] / end

Looking at this, there could be multiple patterns and actions. Maybe we 
should add classes for these two as well - what do the patterns and 
actions look like?



+"""
+ret = f"flow create {self.port_id} "
+if self.group_id is not None:
+ret += f"group {self.group_id} "
+if self.priority_level is not None:
+ret += f"priority {self.priority_level} "
+ret += "ingress " if self.ingress else "egress "
+if self.user_id is not None:
+ret += f"user_id {self.user_id} "
+ret += f"pattern {self.pattern} / end "
+ret += f"actions {self.actions} / end"
+return ret
+
+
  class TestPmdShell(DPDKShell):
  """Testpmd interactive shell.
  
@@ -806,6 +844,25 @@ def show_port_stats(self, port_id: int) -> TestPmdPortStats:
  
  return TestPmdPortStats.parse(output)
  
+def flow_create(self, cmd: FlowRule, verify: bool = True) -> None:


Do we also need to add a method for deleting (I guess it would be called 
destroying per the command) the rule? Or any other flow rule commands? 
We could expand the test suite (I assume we're adding this because of a 
test suite) if such tests make sense.



+"""Creates a flow rule in the testpmd session.
+
+Args:
+cmd: String from flow_func instance to send as a flow rule.


The cmd docstring has not been updated. I'd also rename cmd to flow_rule.


+verify: If :data:`True`, the output of the command is scanned
+to ensure the flow rule was created successfully.
+
+Raises:
+InteractiveCommandExecutionError: If flow rule is invalid.
+"""
+flow_output = self.send_command(str(cmd))
+if verify:
+if "created" not in flow_output:
+self._logger.debug(f"Failed to create flow 
rule:\n{flow_output}")
+raise InteractiveCommandExecutionError(
+f"Failed to create flow rule:\n{flow_output}"
+)
+
  def _close(self) -> None:
  """Overrides :meth:`~.interactive_shell.close`."""
  self.stop()

RE: [PATCH 3/6] ethdev: add flow rule insertion by index with pattern

[Dariusz Sosnowski]

> -Original Message-
> From: Alexander Kozyrev 
> Sent: Thursday, September 19, 2024 02:48
> To: dev@dpdk.org
> Cc: Dariusz Sosnowski ; Ori Kam
> ; NBU-Contact-Thomas Monjalon (EXTERNAL)
> ; Matan Azrad ;
> ferruh.yi...@amd.com; step...@networkplumber.org
> Subject: [PATCH 3/6] ethdev: add flow rule insertion by index with pattern
> 
> Add a new API to enqueue flow rule creation by index with pattern.
> The new template table rules insertion type, index-based insertion with
> pattern, requires a new flow rule creation function with both rule index and
> pattern provided.
> Packets will match on the provided pattern at the provided index.
> 
> Signed-off-by: Alexander Kozyrev 

[snip]

> +RTE_TRACE_POINT_FP(
> + rte_flow_trace_async_create_by_index,
> + RTE_TRACE_POINT_ARGS(uint16_t port_id, uint32_t queue_id,
> + const struct rte_flow_op_attr *op_attr,
> + const struct rte_flow_template_table *template_table,
> + uint32_t rule_index,
> + const struct rte_flow_action *actions,
> + uint8_t actions_template_index,
> + const void *user_data, const struct rte_flow *flow),
> + rte_trace_point_emit_u16(port_id);
> + rte_trace_point_emit_u32(queue_id);
> + rte_trace_point_emit_ptr(op_attr);
> + rte_trace_point_emit_ptr(template_table);
> + rte_trace_point_emit_u32(rule_index);
> + rte_trace_point_emit_ptr(actions);
> + rte_trace_point_emit_u8(actions_template_index);
> + rte_trace_point_emit_ptr(user_data);
> + rte_trace_point_emit_ptr(flow);
> +)

This tracepoint is not used anywhere and is not related to the addition of 
rte_flow_trace_async_create_by_index_with_pattern.
Maybe this tracepoint should be added in a separate commit?

> +RTE_TRACE_POINT_FP(
> + rte_flow_trace_async_create_by_index_with_pattern,
> + RTE_TRACE_POINT_ARGS(uint16_t port_id, uint32_t queue_id,
> + const struct rte_flow_op_attr *op_attr,
> + const struct rte_flow_template_table *template_table,
> + uint32_t rule_index,
> + const struct rte_flow_item *pattern,
> + uint8_t pattern_template_index,
> + const struct rte_flow_action *actions,
> + uint8_t actions_template_index,
> + const void *user_data, const struct rte_flow *flow),
> + rte_trace_point_emit_u16(port_id);
> + rte_trace_point_emit_u32(queue_id);
> + rte_trace_point_emit_ptr(op_attr);
> + rte_trace_point_emit_ptr(template_table);
> + rte_trace_point_emit_u32(rule_index);
> + rte_trace_point_emit_ptr(pattern);
> + rte_trace_point_emit_u8(pattern_template_index);
> + rte_trace_point_emit_ptr(actions);
> + rte_trace_point_emit_u8(actions_template_index);
> + rte_trace_point_emit_ptr(user_data);
> + rte_trace_point_emit_ptr(flow);
> +)

This tracepoint is not used in this commit.
Could you please add the usage?

Best regards,
Dariusz Sosnowski

RE: [PATCH 2/6] app/testpmd: add index with pattern insertion type

[Dariusz Sosnowski]

> -Original Message-
> From: Alexander Kozyrev 
> Sent: Thursday, September 19, 2024 02:48
> To: dev@dpdk.org
> Cc: Dariusz Sosnowski ; Ori Kam
> ; NBU-Contact-Thomas Monjalon (EXTERNAL)
> ; Matan Azrad ;
> ferruh.yi...@amd.com; step...@networkplumber.org
> Subject: [PATCH 2/6] app/testpmd: add index with pattern insertion type
> 
> Provide index_with_pattern command line option for the template table
> insertion type.
> 
> flow template_table 0 create table_id 2 group 13 priority 0
>   insertion_type index_with_pattern ingress rules_number 64
>   pattern_template 2 actions_template 2
> 
> Signed-off-by: Alexander Kozyrev 

Acked-by: Dariusz Sosnowski 

Best regards,
Dariusz Sosnowski

RE: [PATCH 4/6] app/testpmd: add insertion by index with pattern option

[Dariusz Sosnowski]

> -Original Message-
> From: Alexander Kozyrev 
> Sent: Thursday, September 19, 2024 02:48
> To: dev@dpdk.org
> Cc: Dariusz Sosnowski ; Ori Kam
> ; NBU-Contact-Thomas Monjalon (EXTERNAL)
> ; Matan Azrad ;
> ferruh.yi...@amd.com; step...@networkplumber.org
> Subject: [PATCH 4/6] app/testpmd: add insertion by index with pattern option
> 
> Allow to specify both the rule index and the pattern in the flow rule creation
> command line parameters.
> Both are needed for rte_flow_async_create_by_index_with_pattern().
> 
> flow queue 0 create 0 template_table 2 rule_index 5
>   pattern_template 0 actions_template 0 postpone no pattern eth / end
>   actions count / queue index 1 / end
> 
> Signed-off-by: Alexander Kozyrev 

Acked-by: Dariusz Sosnowski 

Best regards,
Dariusz Sosnowski

RE: [PATCH 5/6] ethdev: add jump to table index action

[Dariusz Sosnowski]

> -Original Message-
> From: Alexander Kozyrev 
> Sent: Thursday, September 19, 2024 02:48
> To: dev@dpdk.org
> Cc: Dariusz Sosnowski ; Ori Kam
> ; NBU-Contact-Thomas Monjalon (EXTERNAL)
> ; Matan Azrad ;
> ferruh.yi...@amd.com; step...@networkplumber.org
> Subject: [PATCH 5/6] ethdev: add jump to table index action
> 
> Introduce the RTE_FLOW_ACTION_TYPE_JUMP_TO_TABLE_INDEX action.
> It redirects packets to a particular index in a flow table.
> 
> Signed-off-by: Alexander Kozyrev 

Acked-by: Dariusz Sosnowski 

Best regards,
Dariusz Sosnowski

RE: [PATCH 1/6] ethdev: add insertion by index with pattern

[Dariusz Sosnowski]

> -Original Message-
> From: Alexander Kozyrev 
> Sent: Thursday, September 19, 2024 02:48
> To: dev@dpdk.org
> Cc: Dariusz Sosnowski ; Ori Kam
> ; NBU-Contact-Thomas Monjalon (EXTERNAL)
> ; Matan Azrad ;
> ferruh.yi...@amd.com; step...@networkplumber.org
> Subject: [PATCH 1/6] ethdev: add insertion by index with pattern
> 
> There are two flow table rules insertion type today:
> pattern-based insertion when packets match on the pattern and index-based
> insertion when packets always hit at the index.
> We need another mode that allows to match on the pattern at the index:
> insertion by index with pattern.
> 
> Signed-off-by: Alexander Kozyrev 

Acked-by: Dariusz Sosnowski 

Best regards,
Dariusz Sosnowski

Re: [PATCH] net/r8169: add support for hw initialization

[Stephen Hemminger]

On Wed, 25 Sep 2024 15:54:21 +0800
Howard Wang  wrote:

> From: Howard Wang 
> To: 
> CC: , Howard Wang 
> Subject: [PATCH] net/r8169: add support for hw initialization
> Date: Wed, 25 Sep 2024 15:54:21 +0800
> X-Mailer: git-send-email 2.34.1
> 
> Signed-off-by: Howard Wang 

Please use git-send-email in batch mode so that each patch in set has a number
and then gets organized in patchwork. Follow ups should use in-reply-to option.
Use cover-letter to describe the overall patch series

See:
   https://core.dpdk.org/contribute/

Would expect something like:
   $ git send-email -5 --to dev@dpdk.org --cover-letter --annotate \
   --in-reply-to  \
   --subject-prefix 'PATCH v4' 

RE: [PATCH dpdk] mbuf: fix strict aliasing error in allocator

[Morten Brørup]

> From: Robin Jarry [mailto:rja...@redhat.com]
> Sent: Wednesday, 25 September 2024 10.00
> 
> When building an application with -fstrict-aliasing -Wstrict-
> aliasing=2,
> we get errors triggered by rte_mbuf_raw_alloc() which is called inline
> from rte_pktmbuf_alloc().
> 
>  ../dpdk/lib/mbuf/rte_mbuf.h: In function ‘rte_mbuf_raw_alloc’:
>  ../dpdk/lib/mbuf/rte_mbuf.h:600:42: error: dereferencing type-punned
>  pointer might break strict-aliasing rules [-Werror=strict-aliasing]
>600 | if (rte_mempool_get(mp, (void **)&m) < 0)
>|  ^~
> 
> Avoid incorrect casting by changing the type of the returned variable.
> 
> Signed-off-by: Robin Jarry 
> ---
>  lib/mbuf/rte_mbuf.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/mbuf/rte_mbuf.h b/lib/mbuf/rte_mbuf.h
> index babe16c72ccb..bab1fb94d41d 100644
> --- a/lib/mbuf/rte_mbuf.h
> +++ b/lib/mbuf/rte_mbuf.h
> @@ -595,9 +595,9 @@ __rte_mbuf_raw_sanity_check(__rte_unused const
> struct rte_mbuf *m)
>   */
>  static inline struct rte_mbuf *rte_mbuf_raw_alloc(struct rte_mempool
> *mp)
>  {
> - struct rte_mbuf *m;
> + void *m;
> 
> - if (rte_mempool_get(mp, (void **)&m) < 0)
> + if (rte_mempool_get(mp, &m) < 0)
>   return NULL;
>   __rte_mbuf_raw_sanity_check(m);
>   return m;

Suggest:
__rte_mbuf_raw_sanity_check((struct rte_mbuf *)m);
return (struct rte_mbuf *)m;

> --
> 2.46.1

Please confirm that this builds with RTE_LIBRTE_MBUF_DEBUG and 
RTE_ENABLE_ASSERT set in config/rte_config.h. When confirmed, you may add:
Acked-by: Morten Brørup 

Re: [PATCH dpdk] mbuf: fix strict aliasing error in allocator

[Stephen Hemminger]

On Wed, 25 Sep 2024 17:21:12 +0200
Morten Brørup  wrote:

> From: Morten Brørup 
> To: "Robin Jarry" ,  
> Subject: RE: [PATCH dpdk] mbuf: fix strict aliasing error in allocator
> Date: Wed, 25 Sep 2024 17:21:12 +0200
> 
> > From: Robin Jarry [mailto:rja...@redhat.com]
> > Sent: Wednesday, 25 September 2024 10.00
> > 
> > When building an application with -fstrict-aliasing -Wstrict-
> > aliasing=2,
> > we get errors triggered by rte_mbuf_raw_alloc() which is called inline
> > from rte_pktmbuf_alloc().
> > 
> >  ../dpdk/lib/mbuf/rte_mbuf.h: In function ‘rte_mbuf_raw_alloc’:
> >  ../dpdk/lib/mbuf/rte_mbuf.h:600:42: error: dereferencing type-punned
> >  pointer might break strict-aliasing rules [-Werror=strict-aliasing]
> >600 | if (rte_mempool_get(mp, (void **)&m) < 0)
> >|  ^~
> > 
> > Avoid incorrect casting by changing the type of the returned variable.
> > 
> > Signed-off-by: Robin Jarry 
> > ---
> >  lib/mbuf/rte_mbuf.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/mbuf/rte_mbuf.h b/lib/mbuf/rte_mbuf.h
> > index babe16c72ccb..bab1fb94d41d 100644
> > --- a/lib/mbuf/rte_mbuf.h
> > +++ b/lib/mbuf/rte_mbuf.h
> > @@ -595,9 +595,9 @@ __rte_mbuf_raw_sanity_check(__rte_unused const
> > struct rte_mbuf *m)
> >   */
> >  static inline struct rte_mbuf *rte_mbuf_raw_alloc(struct rte_mempool
> > *mp)
> >  {
> > -   struct rte_mbuf *m;
> > +   void *m;
> > 
> > -   if (rte_mempool_get(mp, (void **)&m) < 0)
> > +   if (rte_mempool_get(mp, &m) < 0)
> > return NULL;
> > __rte_mbuf_raw_sanity_check(m);
> > return m;  
> 
> Suggest:
>   __rte_mbuf_raw_sanity_check((struct rte_mbuf *)m);
>   return (struct rte_mbuf *)m;

Another way to avoid the warning would be to use a union?

Re: [PATCH v5 1/2] ethdev: Add link_speed lanes support

[Damodharam Ammepalli]

On Tue, Sep 24, 2024 at 3:59 PM Damodharam Ammepalli
 wrote:
>
> On Sun, Sep 22, 2024 at 10:02 AM Ferruh Yigit  wrote:
> >
> > On 9/4/2024 6:50 PM, Damodharam Ammepalli wrote:
> > > Update the eth_dev_ops structure with new function vectors
> > > to get, get capabilities and set ethernet link speed lanes.
> > > Update the testpmd to provide required config and information
> > > display infrastructure.
> > >
> > > The supporting ethernet controller driver will register callbacks
> > > to avail link speed lanes config and get services. This lanes
> > > configuration is applicable only when the nic is forced to fixed
> > > speeds. In Autonegiation mode, the hardware automatically
> > > negotiates the number of lanes.
> > >
> > > These are the new commands.
> > >
> > > testpmd> show port 0 speed_lanes capabilities
> > >
> > >  Supported speeds Valid lanes
> > > ---
> > >  10 Gbps  1
> > >  25 Gbps  1
> > >  40 Gbps  4
> > >  50 Gbps  1 2
> > >  100 Gbps 1 2 4
> > >  200 Gbps 2 4
> > >  400 Gbps 4 8
> > > testpmd>
> > >
> > > testpmd>
> > > testpmd> port stop 0
> > > testpmd> port config 0 speed_lanes 4
> > > testpmd> port config 0 speed 20 duplex full
> > > testpmd> port start 0
> > > testpmd>
> > > testpmd> show port info 0
> > >
> > > * Infos for port 0  *
> > > MAC address: 14:23:F2:C3:BA:D2
> > > Device name: :b1:00.0
> > > Driver name: net_bnxt
> > > Firmware-version: 228.9.115.0
> > > Connect to socket: 2
> > > memory allocation on the socket: 2
> > > Link status: up
> > > Link speed: 200 Gbps
> > > Active Lanes: 4
> > > Link duplex: full-duplex
> > > Autoneg status: Off
> > >
> > > Signed-off-by: Damodharam Ammepalli 
> > > ---
> > >  app/test-pmd/cmdline.c | 248 -
> > >  app/test-pmd/config.c  |   4 +
> > >  lib/ethdev/ethdev_driver.h |  91 ++
> > >  lib/ethdev/rte_ethdev.c|  52 
> > >  lib/ethdev/rte_ethdev.h|  95 ++
> > >  lib/ethdev/version.map |   5 +
> > >  6 files changed, 494 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> > > index b7759e38a8..643102032e 100644
> > > --- a/app/test-pmd/cmdline.c
> > > +++ b/app/test-pmd/cmdline.c
> > > @@ -284,6 +284,9 @@ static void cmd_help_long_parsed(void *parsed_result,
> > >
> > >   "dump_log_types\n"
> > >   "Dumps the log level for all the dpdk 
> > > modules\n\n"
> > > +
> > > + "show port (port_id) speed_lanes capabilities"
> > > + "   Show speed lanes capabilities of a 
> > > port.\n\n"
> > >   );
> > >   }
> > >
> > > @@ -823,6 +826,9 @@ static void cmd_help_long_parsed(void *parsed_result,
> > >   "port config (port_id) txq (queue_id) affinity 
> > > (value)\n"
> > >   "Map a Tx queue with an aggregated port "
> > >   "of the DPDK port\n\n"
> > > +
> > > + "port config (port_id|all) speed_lanes (0|1|4|8)\n"
> > >
> >
> > This help string, and the implementation, implies there has to be fixed
> > lane values, like 1, 4, 8. Why not leave this part to the capability
> > reporting, and not limit (and worry) those values in the command, so
> > from command's perspective it can be only .
> >
>
> ok, will update the help string to 
>
> > > + "Set number of lanes for all ports or port_id 
> > > for a forced speed\n\n"
> > >   );
> > >   }
> > >
> > > @@ -1560,6 +1566,244 @@ static cmdline_parse_inst_t 
> > > cmd_config_speed_specific = {
> > >   },
> > >  };
> > >
> > > +static int
> > > +parse_speed_lanes_cfg(portid_t pid, uint32_t lanes)
> > > +{
> > > + int ret;
> > > +
> > > + ret = rte_eth_speed_lanes_set(pid, lanes);
> > >
> >
> > As a sample application usage, I think it is better if it gets the
> > capability and verify that 'lanes' is withing the capability and later
> > set it, what do you think?
> >
> > <...>
>
> Makes sense, will try out and get back to you soon.
>
I guess a similar comment was discussed in the past, slipped my mind
to mention in my last response.
These are the reasons for this logic.
-  same lane number can be supported by any speed (eg: 4 Lanes
supported in 100,200,400 etc)
so, which speed row should app check against, since the user has not
configured fixed speed yet.
If the link is already up in AN mode, this lane config doesn't have
any significance.
- one approach would be to validate lanes in
parse_and_check_speed_duplex(), but in this case, the app should
already have non-default lanes value. We also need to consider the
case, if the user wants default lanes.
- In the ethtool, the driver does the validation of the speed x lanes
combo. Tried to mat

RE: [PATCH 6/6] app/testpmd: add jump to table index action

[Dariusz Sosnowski]

> -Original Message-
> From: Alexander Kozyrev 
> Sent: Thursday, September 19, 2024 02:48
> To: dev@dpdk.org
> Cc: Dariusz Sosnowski ; Ori Kam
> ; NBU-Contact-Thomas Monjalon (EXTERNAL)
> ; Matan Azrad ;
> ferruh.yi...@amd.com; step...@networkplumber.org
> Subject: [PATCH 6/6] app/testpmd: add jump to table index action
> 
> Add a new command line options to create the
> RTE_FLOW_ACTION_TYPE_JUMP_TO_TABLE_INDEX action from the testpmd
> command line.
> 
> flow queue 0 create 0 template_table 0 pattern_template 0
>   actions_template 0 postpone no pattern eth / end
>   actions jump_to_table_index table 0x166f9ce00 index 5 / end
> 
> Signed-off-by: Alexander Kozyrev 

Acked-by: Dariusz Sosnowski 

Best regards,
Dariusz Sosnowski

Re: [PATCH] net/r8169: add support for hw initialization

[Stephen Hemminger]

On Wed, 25 Sep 2024 15:54:21 +0800
Howard Wang  wrote:

> + PMD_INIT_LOG(NOTICE, "r8169: Assign randomly generated MAC 
> address "
> +  "%02x:%02x:%02x:%02x:%02x:%02x",
> +  perm_addr->addr_bytes[0],
> +  perm_addr->addr_bytes[1],
> +  perm_addr->addr_bytes[2],
> +  perm_addr->addr_bytes[3],
> +  perm_addr->addr_bytes[4],
> +  perm_addr->addr_bytes[5]);

DPDK rte_ether.h has a function to format ethernet addresses for printing, 
please
use that.

Re: [PATCH dpdk] mbuf: fix strict aliasing error in allocator

[Robin Jarry]

Stephen Hemminger, Sep 25, 2024 at 11:23:

On Wed, 25 Sep 2024 17:21:12 +0200
Morten Brørup  wrote:

> From: Morten Brørup 
> To: "Robin Jarry" ,  
> Subject: RE: [PATCH dpdk] mbuf: fix strict aliasing error in allocator
> Date: Wed, 25 Sep 2024 17:21:12 +0200
> 
> > From: Robin Jarry [mailto:rja...@redhat.com]

> > Sent: Wednesday, 25 September 2024 10.00
> > 
> > When building an application with -fstrict-aliasing -Wstrict-

> > aliasing=2,
> > we get errors triggered by rte_mbuf_raw_alloc() which is called inline
> > from rte_pktmbuf_alloc().
> > 
> >  ../dpdk/lib/mbuf/rte_mbuf.h: In function ‘rte_mbuf_raw_alloc’:

> >  ../dpdk/lib/mbuf/rte_mbuf.h:600:42: error: dereferencing type-punned
> >  pointer might break strict-aliasing rules [-Werror=strict-aliasing]
> >600 | if (rte_mempool_get(mp, (void **)&m) < 0)
> >|  ^~
> > 
> > Avoid incorrect casting by changing the type of the returned variable.
> > 
> > Signed-off-by: Robin Jarry 

> > ---
> >  lib/mbuf/rte_mbuf.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/mbuf/rte_mbuf.h b/lib/mbuf/rte_mbuf.h

> > index babe16c72ccb..bab1fb94d41d 100644
> > --- a/lib/mbuf/rte_mbuf.h
> > +++ b/lib/mbuf/rte_mbuf.h
> > @@ -595,9 +595,9 @@ __rte_mbuf_raw_sanity_check(__rte_unused const
> > struct rte_mbuf *m)
> >   */
> >  static inline struct rte_mbuf *rte_mbuf_raw_alloc(struct rte_mempool
> > *mp)
> >  {
> > - struct rte_mbuf *m;
> > + void *m;
> > 
> > -	if (rte_mempool_get(mp, (void **)&m) < 0)

> > + if (rte_mempool_get(mp, &m) < 0)
> >   return NULL;
> >   __rte_mbuf_raw_sanity_check(m);
> >  	return m;  
> 
> Suggest:

>__rte_mbuf_raw_sanity_check((struct rte_mbuf *)m);
>return (struct rte_mbuf *)m;

Another way to avoid the warning would be to use a union?


I can use an inline union, it will be less code.

Re: [PATCH] net/r8169: add support for hw initialization

[Stephen Hemminger]

On Wed, 25 Sep 2024 15:54:21 +0800
Howard Wang  wrote:

>  static int
>  rtl_dev_close(struct rte_eth_dev *dev)
>  {
> + //struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
> + //struct rte_intr_handle *intr_handle = pci_dev->intr_handle;

No commented out code please.
As checkpatch warns, DPDK does not use C99 style comments.

[PATCH dpdk v2] mbuf: fix strict aliasing error in allocator

[Robin Jarry]

When building an application with -fstrict-aliasing -Wstrict-aliasing=2,
we get errors triggered by rte_mbuf_raw_alloc() which is called inline
from rte_pktmbuf_alloc().

 ../dpdk/lib/mbuf/rte_mbuf.h: In function ‘rte_mbuf_raw_alloc’:
 ../dpdk/lib/mbuf/rte_mbuf.h:600:42: error: dereferencing type-punned
 pointer might break strict-aliasing rules [-Werror=strict-aliasing]
   600 | if (rte_mempool_get(mp, (void **)&m) < 0)
   |  ^~

Avoid incorrect casting by using an inline union variable.

Signed-off-by: Robin Jarry 
---

Notes:
v2: use inline union to fix -Wincompatible-pointer-types

 lib/mbuf/rte_mbuf.h | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/lib/mbuf/rte_mbuf.h b/lib/mbuf/rte_mbuf.h
index babe16c72ccb..0d2e0e64b3ce 100644
--- a/lib/mbuf/rte_mbuf.h
+++ b/lib/mbuf/rte_mbuf.h
@@ -595,12 +595,15 @@ __rte_mbuf_raw_sanity_check(__rte_unused const struct 
rte_mbuf *m)
  */
 static inline struct rte_mbuf *rte_mbuf_raw_alloc(struct rte_mempool *mp)
 {
-   struct rte_mbuf *m;
+   union {
+   void *ptr;
+   struct rte_mbuf *m;
+   } ret;
 
-   if (rte_mempool_get(mp, (void **)&m) < 0)
+   if (rte_mempool_get(mp, &ret.ptr) < 0)
return NULL;
-   __rte_mbuf_raw_sanity_check(m);
-   return m;
+   __rte_mbuf_raw_sanity_check(ret.m);
+   return ret.m;
 }
 
 /**
-- 
2.46.1

[PATCH v6 0/1] dts: testpmd verbose parser

[jspewock]

From: Jeremy Spewock 

v6:
 * apply on next-dts
 * update PacketOffloadFlag values which were equal to combinations of
   other values to make them unique
 * remove support of EUI-64 MAC addresses for now
 * update RtePtype and PacketOffloadFlag so that their parser methods
   don't bother themselves with splitting the string into a list
 * reformatted and updated doc-strings

Jeremy Spewock (1):
  dts: add text parser for testpmd verbose output

 dts/framework/remote_session/testpmd_shell.py | 537 +-
 dts/framework/utils.py|   3 +
 2 files changed, 538 insertions(+), 2 deletions(-)

-- 
2.46.0

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

[jspewock]

From: Jeremy Spewock 

Multiple test suites from the old DTS framework rely on being able to
consume and interpret the verbose output of testpmd. The new framework
doesn't have an elegant way for handling the verbose output, but test
suites are starting to be written that rely on it. This patch creates a
TextParser class that can be used to extract the verbose information
from any testpmd output and also adjusts the `stop` method of the shell
to return all output that it collected.

Signed-off-by: Jeremy Spewock 
---
 dts/framework/remote_session/testpmd_shell.py | 537 +-
 dts/framework/utils.py|   3 +
 2 files changed, 538 insertions(+), 2 deletions(-)

diff --git a/dts/framework/remote_session/testpmd_shell.py 
b/dts/framework/remote_session/testpmd_shell.py
index 77902a468d..4ca34505b7 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -32,7 +32,7 @@
 from framework.settings import SETTINGS
 from framework.testbed_model.cpu import LogicalCoreCount, LogicalCoreList
 from framework.testbed_model.sut_node import SutNode
-from framework.utils import StrEnum
+from framework.utils import REGEX_FOR_MAC_ADDRESS, StrEnum
 
 P = ParamSpec("P")
 TestPmdShellMethod = Callable[Concatenate["TestPmdShell", P], Any]
@@ -581,6 +581,506 @@ class TestPmdPortStats(TextParser):
 tx_bps: int = field(metadata=TextParser.find_int(r"Tx-bps:\s+(\d+)"))
 
 
+class PacketOffloadFlag(Flag):
+"""Flag representing the Packet Offload Features Flags in DPDK.
+
+Values in this class are taken from the definitions in the RTE MBUF core 
library in DPDK
+located in ``lib/mbuf/rte_mbuf_core.h``. It is expected that flag values 
in this class will
+match the values they are set to in said DPDK library with one exception; 
all values must be
+unique. For example, the definitions for unknown checksum flags in 
``rte_mbuf_core.h`` are all
+set to :data:`0`, but it is valuable to distinguish between them in this 
framework. For this
+reason flags that are not unique in the DPDK library are set either to 
values within the
+RTE_MBUF_F_FIRST_FREE-RTE_MBUF_F_LAST_FREE range for Rx or shifted 61+ 
bits for Tx.
+
+References:
+DPDK lib: ``lib/mbuf/rte_mbuf_core.h``
+"""
+
+# RX flags
+
+#: The RX packet is a 802.1q VLAN packet, and the tci has been saved in 
mbuf->vlan_tci. If the
+#: flag RTE_MBUF_F_RX_VLAN_STRIPPED is also present, the VLAN header has 
been stripped from
+#: mbuf data, else it is still present.
+RTE_MBUF_F_RX_VLAN = auto()
+
+#: RX packet with RSS hash result.
+RTE_MBUF_F_RX_RSS_HASH = auto()
+
+#: RX packet with FDIR match indicate.
+RTE_MBUF_F_RX_FDIR = auto()
+
+#: This flag is set when the outermost IP header checksum is detected as 
wrong by the hardware.
+RTE_MBUF_F_RX_OUTER_IP_CKSUM_BAD = 1 << 5
+
+#: A vlan has been stripped by the hardware and its tci is saved in 
mbuf->vlan_tci. This can
+#: only happen if vlan stripping is enabled in the RX configuration of the 
PMD. When
+#: RTE_MBUF_F_RX_VLAN_STRIPPED is set, RTE_MBUF_F_RX_VLAN must also be set.
+RTE_MBUF_F_RX_VLAN_STRIPPED = auto()
+
+#: No information about the RX IP checksum. Value is 0 in the DPDK library.
+RTE_MBUF_F_RX_IP_CKSUM_UNKNOWN = 1 << 23
+#: The IP checksum in the packet is wrong.
+RTE_MBUF_F_RX_IP_CKSUM_BAD = 1 << 4
+#: The IP checksum in the packet is valid.
+RTE_MBUF_F_RX_IP_CKSUM_GOOD = 1 << 7
+#: The IP checksum is not correct in the packet data, but the integrity of 
the IP header is
+#: verified. Value is RTE_MBUF_F_RX_IP_CKSUM_BAD | 
RTE_MBUF_F_RX_IP_CKSUM_GOOD in the DPDK
+#: library.
+RTE_MBUF_F_RX_IP_CKSUM_NONE = 1 << 24
+
+#: No information about the RX L4 checksum. Value is 0 in the DPDK library.
+RTE_MBUF_F_RX_L4_CKSUM_UNKNOWN = 1 << 25
+#: The L4 checksum in the packet is wrong.
+RTE_MBUF_F_RX_L4_CKSUM_BAD = 1 << 3
+#: The L4 checksum in the packet is valid.
+RTE_MBUF_F_RX_L4_CKSUM_GOOD = 1 << 8
+#: The L4 checksum is not correct in the packet data, but the integrity of 
the L4 data is
+#: verified. Value is RTE_MBUF_F_RX_L4_CKSUM_BAD | 
RTE_MBUF_F_RX_L4_CKSUM_GOOD in the DPDK
+#: library.
+RTE_MBUF_F_RX_L4_CKSUM_NONE = 1 << 26
+
+#: RX IEEE1588 L2 Ethernet PT Packet.
+RTE_MBUF_F_RX_IEEE1588_PTP = 1 << 9
+#: RX IEEE1588 L2/L4 timestamped packet.
+RTE_MBUF_F_RX_IEEE1588_TMST = 1 << 10
+
+#: FD id reported if FDIR match.
+RTE_MBUF_F_RX_FDIR_ID = 1 << 13
+#: Flexible bytes reported if FDIR match.
+RTE_MBUF_F_RX_FDIR_FLX = 1 << 14
+
+#: If both RTE_MBUF_F_RX_QINQ_STRIPPED and RTE_MBUF_F_RX_VLAN_STRIPPED are 
set, the 2 VLANs
+#: have been stripped by the hardware. If RTE_MBUF_F_RX_QINQ_STRIPPED is 
set and
+#: RTE_MBUF_F_RX_VLAN_STRIPPED is unset, only the outer VLAN is removed 
from packet data.
+RTE_MBUF_F

Re: [PATCH dpdk v2] mbuf: fix strict aliasing error in allocator

[Stephen Hemminger]

On Wed, 25 Sep 2024 11:40:54 -0400
Robin Jarry  wrote:

> From: Robin Jarry 
> To: dev@dpdk.org
> Subject: [PATCH dpdk v2] mbuf: fix strict aliasing error in allocator
> Date: Wed, 25 Sep 2024 11:40:54 -0400
> 
> When building an application with -fstrict-aliasing -Wstrict-aliasing=2,
> we get errors triggered by rte_mbuf_raw_alloc() which is called inline
> from rte_pktmbuf_alloc().
> 
>  ../dpdk/lib/mbuf/rte_mbuf.h: In function ‘rte_mbuf_raw_alloc’:
>  ../dpdk/lib/mbuf/rte_mbuf.h:600:42: error: dereferencing type-punned
>  pointer might break strict-aliasing rules [-Werror=strict-aliasing]
>600 | if (rte_mempool_get(mp, (void **)&m) < 0)
>|  ^~
> 
> Avoid incorrect casting by using an inline union variable.
> 
> Signed-off-by: Robin Jarry 

Thanks, union is safer than cast.

Reviewed-by: Stephen Hemminger 

[PATCH dpdk] mbuf: fix strict aliasing error in allocator

[Robin Jarry]

When building an application with -fstrict-aliasing -Wstrict-aliasing=2,
we get errors triggered by rte_mbuf_raw_alloc() which is called inline
from rte_pktmbuf_alloc().

 ../dpdk/lib/mbuf/rte_mbuf.h: In function ‘rte_mbuf_raw_alloc’:
 ../dpdk/lib/mbuf/rte_mbuf.h:600:42: error: dereferencing type-punned
 pointer might break strict-aliasing rules [-Werror=strict-aliasing]
   600 | if (rte_mempool_get(mp, (void **)&m) < 0)
   |  ^~

Avoid incorrect casting by changing the type of the returned variable.

Signed-off-by: Robin Jarry 
---
 lib/mbuf/rte_mbuf.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/mbuf/rte_mbuf.h b/lib/mbuf/rte_mbuf.h
index babe16c72ccb..bab1fb94d41d 100644
--- a/lib/mbuf/rte_mbuf.h
+++ b/lib/mbuf/rte_mbuf.h
@@ -595,9 +595,9 @@ __rte_mbuf_raw_sanity_check(__rte_unused const struct 
rte_mbuf *m)
  */
 static inline struct rte_mbuf *rte_mbuf_raw_alloc(struct rte_mempool *mp)
 {
-   struct rte_mbuf *m;
+   void *m;
 
-   if (rte_mempool_get(mp, (void **)&m) < 0)
+   if (rte_mempool_get(mp, &m) < 0)
return NULL;
__rte_mbuf_raw_sanity_check(m);
return m;
-- 
2.46.1

Re: [PATCH 00/47] TruFlow update for Thor2

[Ajit Khaparde]

On Fri, Aug 30, 2024 at 9:51 AM Sriharsha Basavapatna
 wrote:
>
> Update TruFlow to support the Thor2 device.

Patchset applied to the dpdk-next-net-brcm branch.
Thanks

>
> Farah Smith (2):
>   net/bnxt: tf_core: Thor TF EM key size check
>   net/bnxt: tfc: support tf-core for Thor2
>
> Jay Ding (2):
>   net/bnxt: tf_ulp: add action read and clear support
>   net/bnxt: tf_ulp: add rte_mtr support for Thor2
>
> Kishore Padmanabha (21):
>   net/bnxt: tf_core: flow scale improvement
>   net/bnxt: tf_ulp: add support for vf to vf flow offload
>   net/bnxt: tf_ulp: add support for overlapping flows
>   net/bnxt: tf_ulp: convert recipe table to dynamic memory
>   net/bnxt: tf_ulp: add feature bit support
>   net/bnxt: tf_ulp: update template files
>   net/bnxt: tf_ulp: add support for tunnel flow stats
>   net/bnxt: tf_ulp: update template files
>   net/bnxt: tf_ulp: enable recipe id generation
>   net/bnxt: tf_ulp: fixed parent child db counters
>   net/bnxt: tf_ulp: update template files
>   net/bnxt: tf_ulp: add mask defaults when mask is not specified
>   net/bnxt: tf_ulp: add jump action support
>   net/bnxt: tf_ulp: add support for flow priority
>   net/bnxt: tf_ulp: support for dynamic tunnel ports
>   net/bnxt: tf_ulp: add track type feature to tables
>   net/bnxt: tf_ulp: update template files
>   net/bnxt: tf_ulp: support a few generic template items
>   net/bnxt: tf_ulp: update template files
>   net/bnxt: tf_ulp: enable support for truflow feature configuration
>   net/bnxt: tf_ulp: support a few feature extensions
>
> Manish Kurup (1):
>   net/bnxt: tf_ulp: Wh+ mirroring support
>
> Mike Baucom (2):
>   net/bnxt: tf_ulp: miscellaneous fixes
>   net/bnxt: tf_ulp: VFR updates for Thor 2
>
> Peter Spreadborough (4):
>   net/bnxt: tf_ulp: inline utility functions and use likely/unlikely
>   net/bnxt: tf_ulp: switch ulp to use rte crc32 hash
>   net/bnxt: update template files
>   net/bnxt: tf_ulp: add stats cache for thor2
>
> Randy Schacher (5):
>   net/bnxt: tf_core: convert priority based TCAM manager to dynamic
> allocation
>   net/bnxt: tf_core: remove dead AFM code from session-based priority
> TCAM mgr
>   net/bnxt: tf_core: remove dead code from session-based priority TCAM
> mgr
>   net/bnxt: tf_ulp: add vxlan-gpe base support
>   net/bnxt: tf_ulp: add support for rss flow query to ULP
>
> Sangtani Parag Satishbhai (1):
>   net/bnxt: tf_core: fix slice count in case of HA entry move
>
> Shahaji Bhosle (3):
>   net/bnxt: tf_core: fix wc tcam multi slice delete issue
>   net/bnxt: tf_core: tcam manager data corruption
>   net/bnxt: tf_ulp: add custom l2 etype tunnel support
>
> Shuanglin Wang (6):
>   net/bnxt: tf_core: External EM support cleanup
>   net/bnxt: tf_core: TF support flow scale query
>   net/bnxt: tf_ulp: support for Thor2 ulp layer
>   net/bnxt: tf_ulp: modify return values to adhere to C coding standard
>   net/bnxt: tf_ulp: TF support flow scale query
>   net/bnxt: tf_ulp: TFC support flow scale query for Thor2
>
>  drivers/net/bnxt/bnxt.h   |41 +-
>  drivers/net/bnxt/bnxt_cpr.c   |63 +-
>  drivers/net/bnxt/bnxt_cpr.h   |24 +-
>  drivers/net/bnxt/bnxt_ethdev.c|   111 +-
>  drivers/net/bnxt/bnxt_flow.c  | 5 +-
>  drivers/net/bnxt/bnxt_hwrm.c  |   326 +-
>  drivers/net/bnxt/bnxt_hwrm.h  |20 +
>  drivers/net/bnxt/bnxt_mpc.c   |   853 +
>  drivers/net/bnxt/bnxt_mpc.h   |   117 +
>  drivers/net/bnxt/bnxt_reps.c  |   108 +-
>  drivers/net/bnxt/bnxt_ring.c  |19 +-
>  drivers/net/bnxt/bnxt_ring.h  |54 +-
>  drivers/net/bnxt/bnxt_rxr.c   | 5 +-
>  drivers/net/bnxt/bnxt_txr.c   |30 +-
>  drivers/net/bnxt/bnxt_vnic.c  |39 +-
>  drivers/net/bnxt/bnxt_vnic.h  | 8 +
>  drivers/net/bnxt/hcapi/cfa/hcapi_cfa.h|15 +-
>  drivers/net/bnxt/hcapi/cfa/hcapi_cfa_defs.h   |   576 +-
>  drivers/net/bnxt/hcapi/cfa_v3/CMakeLists.txt  |92 +
>  .../bnxt/hcapi/cfa_v3/bld/host/cfa_bld_mpc.c  |42 +
>  .../hcapi/cfa_v3/bld/include/cfa_bld_defs.h   |   578 +
>  .../hcapi/cfa_v3/bld/include/host/cfa_bld.h   |   524 +
>  .../cfa_v3/bld/include/host/cfa_bld_devops.h  |   297 +
>  .../bld/include/host/cfa_bld_field_ids.h  |  1542 +
>  .../bld/include/host/cfa_bld_mpc_field_ids.h  |  1286 +
>  .../cfa_v3/bld/include/host/cfa_bld_mpcops.h  |   598 +
>  .../cfa_v3/bld/include/p70/cfa_bld_p70_defs.h |   543 +
>  .../bld/include/p70/cfa_bld_p70_field_ids.h   |  1542 +
>  .../cfa_v3/bld/include/p70/cfa_bld_p70_mpc.h  |   548 +
>  .../hcapi/cfa_v3/bld/include/p70/cfa_p70.h|   164 +
>  .../hcapi/cfa_v3/bld/include/p70/cfa_p70_hw.h |  4286 +
>  .../bld/include/p70/cfa_p70_mpc_structs.h |  1496 +
>  .../hcapi/cfa_v3/bld/p70/cfa_bld_p70_mpc.c|   927 +
>  .../cfa_v3/bld/p70/cfa_bld_p7

Re: [PATCH v4 4/5] dts: add OS abstractions for creating virtual functions

[Juraj Linkeš]

diff --git a/dts/framework/testbed_model/linux_session.py 
b/dts/framework/testbed_model/linux_session.py



@@ -210,3 +214,37 @@ def configure_ipv4_forwarding(self, enable: bool) -> None:
  """Overrides 
:meth:`~.os_session.OSSession.configure_ipv4_forwarding`."""
  state = 1 if enable else 0
  self.send_command(f"sysctl -w net.ipv4.ip_forward={state}", 
privileged=True)
+
+def set_num_virtual_functions(self, num: int, pf_port: Port) -> bool:
+"""Overrides 
:meth:`~.os_session.OSSession.set_num_virtual_functions`."""
+sys_bus_path = f"/sys/bus/pci/devices/{pf_port.pci}/sriov_numvfs".replace(":", 
"\\:")
+curr_num_vfs = int(self.send_command(f"cat {sys_bus_path}").stdout)
+if num > 0 and curr_num_vfs >= num:
+self._logger.info(
+f"{curr_num_vfs} VFs already configured on port 
{pf_port.identifier.pci} on node "
+f"{pf_port.identifier.node}."
+)
+return False
+elif num > 0 and curr_num_vfs > 0:


These two conditions could be written as:
if num > 0:
if curr_num_vfs >= num:
return False
elif curr_num_vfs > 0:
raise InternalError()

Maybe it's not worth the extra indent, but it's easier to understand, so 
I lean towards doing it this way.




+def get_pci_addr_of_vfs(self, pf_port: Port) -> list[str]:
+"""Overrides :meth:`~.os_session.OSSession.get_pci_addr_of_vfs`."""
+sys_bus_path = f"/sys/bus/pci/devices/{pf_port.pci}".replace(":", 
"\\:")
+curr_num_vfs = int(self.send_command(f"cat 
{sys_bus_path}/sriov_numvfs").stdout)
+if curr_num_vfs > 0:
+pci_addrs = self.send_command(
+'awk -F "PCI_SLOT_NAME=" "/PCI_SLOT_NAME=/ {print \\$2}" '
++ f"{sys_bus_path}/virtfn*/uevent",


We could use a TextParser here. Not sure if it's a good fit though.



diff --git a/dts/framework/testbed_model/os_session.py 
b/dts/framework/testbed_model/os_session.py



@@ -395,3 +395,43 @@ def configure_ipv4_forwarding(self, enable: bool) -> None:



+@abstractmethod
+def set_num_virtual_functions(self, num: int, pf_port: Port) -> bool:
+"""Update the number of virtual functions (VFs) on a port.
+
+It should be noted that, due to the nature of VFs, if there are 
already VFs that exist on
+the physical function (PF) prior to calling this function, additional 
ones cannot be added.
+The only way to add more VFs is to remove the existing and then set 
the desired amount. For
+this reason, this method will handle creation in the following order:
+
+1. Use existing VFs on the PF if the number of existing VFs is greater 
than or equal to
+`num`
+2. Throw an exception noting that VFs cannot be created if the PF has 
some VFs already set
+on it, but the total VFs that it has are less then `num`.
+3. Create `num` VFs on the PF if there are none on it already
+
+Args:
+num: The number of VFs to set on the port.
+pf_port: The port to add the VFs to.
+
+Raises:
+InternalError: If `pf_port` has less than `num` VFs configured on 
it
+already.
+
+Returns:
+:data:`True` if this method successfully created VFs, 
:data:`False` if existing VFs
+were used instead.
+"""


The whole docstring talks only about the creation of VFs, but we can 
also remove VFs with this.

Re: [PATCH v4 2/5] dts: parameterize what ports the TG sends packets to

[Juraj Linkeš]

On 23. 9. 2024 20:42, jspew...@iol.unh.edu wrote:

From: Jeremy Spewock 

Previously in the DTS framework the helper methods in the TestSuite
class designated ports as either ingress or egress ports and would wrap
the methods of the traffic generator to allow packets to only flow to
those designated ingress or egress ports. This is undesirable in some
cases, such as when you have virtual functions on top of your port,
where the TG ports can send to more than one SUT port. This patch
solves this problem by creating optional parameters that allow the user
to specify which port to gather the MAC addresses from when sending and
receiving packets.

Signed-off-by: Jeremy Spewock 
---


I'm not a fan of exposing the functionality in this way. The developers 
needs to fiddle with ports and there are likely better ways to 
accomplish this.


Ideally, the only information the dev would provide that a test case is 
a VF test case and everything else would happen under the hood in the 
TestCase class.


Barring that, we could decorate the whole TestSuite as requiring VFs, 
which would result in automatically creating and removing the VFs in 
setup/teardown (test case marking would be similar, but possibly more 
complicated, especially if we wanted to abide only by test cases 
selected in a given test run). Then the test cases could pass a simple 
vf=True parameter to the send/receive methods.

Re: [PATCH v4 3/5] dts: add class for virtual functions

[Juraj Linkeš]

On 23. 9. 2024 20:42, jspew...@iol.unh.edu wrote:

From: Jeremy Spewock 

In DPDK applications virtual functions are treated the same as ports,
but within the framework there are benefits to differentiating the two
in order to add more metadata to VFs about where they originate from.
For this reason this patch adds a new class for handling virtual
functions that extends the Port class with some additional information
about the VF.

Bugzilla ID: 1500

Signed-off-by: Jeremy Spewock 
---
  dts/framework/testbed_model/port.py | 37 -
  1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/dts/framework/testbed_model/port.py 
b/dts/framework/testbed_model/port.py
index 817405bea4..c1d85fec2b 100644
--- a/dts/framework/testbed_model/port.py
+++ b/dts/framework/testbed_model/port.py
@@ -27,7 +27,7 @@ class PortIdentifier:
  pci: str
  
  
-@dataclass(slots=True)

+@dataclass


This should be explained in the commit message.


  class Port:
  """Physical port on a node.
  
@@ -80,6 +80,41 @@ def pci(self) -> str:

  return self.identifier.pci
  
  
+@dataclass

+class VirtualFunction(Port):
+"""Virtual Function (VF) on a port.
+
+DPDK applications often treat VFs the same as they do the physical ports 
(PFs) on the host.
+For this reason VFs are represented in the framework as a type of port 
with some additional
+metadata that allows the framework to more easily identify which device 
the VF belongs to as
+well as where the VF originated from.
+
+Attributes:
+created_by_framework: :data:`True` if this VF represents one that the 
DTS framework created
+on the node, :data:`False` otherwise.


I had to go look in the other patches to understand why this is here. 
The patch split should be redone along logical lines (one patch should 
contain related logic, now the related logic is in basically all 
patches), not files (that doesn't help with review and it's also not 
going to result in better git history).


But I figured out this is here because of cleanup. It makes more sense 
to me for framework to remember whether it created the port or not as 
opposed to port remembering it, especially when the framework is doing 
the cleanup and not the ports.



+pf_port: The PF that this VF was created on/gathered from.


Maybe it would make more sense to store the VF ports in the PF port. If 
we need to use VF ports, we can just refer to the PF port which has the 
benefit making it easier to use the proper link between ports.


And the framework can store which PF ports needs VF cleanup instead of 
storing which VFs needs cleaning.



+"""
+
+created_by_framework: bool = False
+pf_port: Port | None = None
+
+def __init__(
+self, node_name: str, config: PortConfig, created_by_framework: bool, 
pf_port: Port
+) -> None:
+"""Extends :meth:`Port.__init__` with VF specific metadata.
+
+Args:
+node_name: The name of the node the VF resides on.
+config: Configuration information about the VF.
+created_by_framework: :data:`True` if DTS created this VF, 
otherwise :data:`False` if
+this class represents a VF that was preexisting on the node.
+pf_port: The PF that this VF was created on/gathered from.
+"""
+super().__init__(node_name, config)
+self.created_by_framework = created_by_framework
+self.pf_port = pf_port
+
+
  @dataclass(slots=True, frozen=True)
  class PortLink:
  """The physical, cabled connection between the ports.

RE: [PATCH v2] ipsec: allow stateless IPsec processing

[Aakash Sasidharan]

Realized that I missed to respond to one of your comments regarding the use of 
union.
Please find the comment below.

> > > > Introduce stateless packet preparation API for IPsec processing.
> > > > The new API would allow preparation of IPsec packets without
> > > > altering the internal state of an IPsec session.
> > > >
> > > > For outbound IPsec processing, the change enables user to provide
> > > > sequence number to be used for the IPsec operation.
> > >
> > > Few questions/nits below.
> > > As a generic one - we need to add a use-case/test-case for it.
> > > Without it I think the patch is incomplete.
> >
> > Ack. Will add test-case with v3.
> >
> > >
> > > >
> > > > Signed-off-by: Aakash Sasidharan 
> > > > ---
> > > >  lib/ipsec/esp_outb.c  | 85
> > > > +++
> > > >  lib/ipsec/rte_ipsec.h | 68 ++
> > > >  lib/ipsec/sa.c|  4 +-
> > > >  lib/ipsec/sa.h|  8 
> > > >  4 files changed, 140 insertions(+), 25 deletions(-)
> > > >
> > > > diff --git a/lib/ipsec/esp_outb.c b/lib/ipsec/esp_outb.c index
> > > > ec87b1dce2..de28cb166d 100644
> > > > --- a/lib/ipsec/esp_outb.c
> > > > +++ b/lib/ipsec/esp_outb.c
> > > > @@ -288,13 +288,12 @@ outb_pkt_xprepare(const struct rte_ipsec_sa
> > > > *sa, rte_be64_t sqc,
> > > >  /*
> > > >   * setup/update packets and crypto ops for ESP outbound tunnel case.
> > > >   */
> > > > -uint16_t
> > > > -esp_outb_tun_prepare(const struct rte_ipsec_session *ss, struct
> > > > rte_mbuf
> > > *mb[],
> > > > -   struct rte_crypto_op *cop[], uint16_t num)
> > > > +static inline uint16_t
> > > > +esp_outb_tun_prepare_helper(const struct rte_ipsec_session *ss,
> > > > +struct
> > > rte_mbuf *mb[],
> > > > +   struct rte_crypto_op *cop[], uint16_t num, uint64_t sqn)
> > > >  {
> > > > int32_t rc;
> > > > -   uint32_t i, k, n;
> > > > -   uint64_t sqn;
> > > > +   uint32_t i, k, n = num;
> > >
> > > You can just do s/num/n/ in function parameter list, then you don't
> > > need to keep 'num' at all.
> >
> > This function will be called for normal IPsec processing. The function
> > esn_outb_update_sqn() updates the local variable n passed as parameter
> > in OVERFLOW case. If we remove the local variable n, this error path
> > would be lost and it is not our intention I believe.
> 
> Apologies. Replied too soon. I understand your suggestion and will update
> that in v3.
> 
> >
> > >
> > > > rte_be64_t sqc;
> > > > struct rte_ipsec_sa *sa;
> > > > struct rte_cryptodev_sym_session *cs; @@ -305,11 +304,6 @@
> > > > esp_outb_tun_prepare(const struct rte_ipsec_session *ss, struct
> > > > rte_mbuf
> > > *mb[],
> > > > sa = ss->sa;
> > > > cs = ss->crypto.ses;
> > > >
> > > > -   n = num;
> > > > -   sqn = esn_outb_update_sqn(sa, &n);
> > > > -   if (n != num)
> > > > -   rte_errno = EOVERFLOW;
> > > > -
> > > > k = 0;
> > > > for (i = 0; i != n; i++) {
> > > >
> > > > @@ -339,6 +333,30 @@ esp_outb_tun_prepare(const struct
> > > rte_ipsec_session *ss, struct rte_mbuf *mb[],
> > > > return k;
> > > >  }
> > > >
> > > > +uint16_t
> > > > +esp_outb_tun_prepare(const struct rte_ipsec_session *ss, struct
> > > > +rte_mbuf
> > > *mb[],
> > > > +   struct rte_crypto_op *cop[], uint16_t num) {
> > > > +   uint64_t sqn;
> > > > +   uint32_t n;
> > > > +
> > > > +   n = num;
> > > > +   sqn = esn_outb_update_sqn(ss->sa, &n);
> > > > +   if (n != num)
> > > > +   rte_errno = EOVERFLOW;
> > > > +
> > > > +   return esp_outb_tun_prepare_helper(ss, mb, cop, n, sqn); }
> > > > +
> > > > +uint16_t
> > > > +esp_outb_tun_prepare_stateless(const struct rte_ipsec_session
> > > > +*ss, struct
> > > rte_mbuf *mb[],
> > > > +   struct rte_crypto_op *cop[], uint16_t num, struct
> > > > +rte_ipsec_state
> > > > +*state) {
> > > > +   uint64_t sqn = state->sqn;
> > > > +
> > > > +   return esp_outb_tun_prepare_helper(ss, mb, cop, num, sqn); }
> > > > +
> > > >  /*
> > > >   * setup/update packet data and metadata for ESP outbound
> > > > transport
> > case.
> > > >   */
> > > > @@ -529,16 +547,15 @@ outb_cpu_crypto_prepare(const struct
> > > rte_ipsec_sa *sa, uint32_t *pofs,
> > > > return clen;
> > > >  }
> > > >
> > > > -static uint16_t
> > > > -cpu_outb_pkt_prepare(const struct rte_ipsec_session *ss,
> > > > -   struct rte_mbuf *mb[], uint16_t num,
> > > > -   esp_outb_prepare_t prepare, uint32_t cofs_mask)
> > > > +static inline uint16_t
> > > > +cpu_outb_pkt_prepare_helper(const struct rte_ipsec_session *ss,
> > > > +   struct rte_mbuf *mb[], uint16_t num, esp_outb_prepare_t
> > > prepare,
> > > > +   uint32_t cofs_mask, uint64_t sqn)
> > > >  {
> > > > int32_t rc;
> > > > -   uint64_t sqn;
> > > > rte_be64_t sqc;
> > > > struct rte_ipsec_sa *sa;
> > > > -   uint32_t 

Re: [PATCH v4 5/5] dts: add functions for managing VFs to Node

[Juraj Linkeš]

I'm wondering whether we should move some of the functionality to the 
Port class, such as creating VFs and related logic. I wanted to move 
update_port and such there, but I ran into problems with imports. Maybe 
if we utilize the if TYPE_CHECKING: guard the imports would work.


Seems like a lot would be simplified if we moved the VFs ports inside 
the Port class.



diff --git a/dts/framework/testbed_model/node.py 
b/dts/framework/testbed_model/node.py



  class Node(ABC):
@@ -276,6 +277,96 @@ def _bind_port_to_driver(self, port: Port, for_dpdk: bool = 
True) -> None:
  verify=True,
  )
  
+def create_virtual_functions(

+self, num: int, pf_port: Port, dpdk_driver: str | None = None
+) -> list[VirtualFunction]:
+"""Create virtual functions (VFs) from a given physical function (PF) 
on the node.
+
+Virtual functions will be created if there are not any currently 
configured on `pf_port`.
+If there are greater than or equal to `num` VFs already configured on 
`pf_port`, those will
+be used instead of creating more. In order to create VFs, the PF must 
be bound to its
+kernel driver. This method will handle binding `pf_port` and any other 
ports in the test
+run that reside on the same device back to their OS drivers if this 
was not done already.
+VFs gathered in this method will be bound to `driver` if one is 
provided, or the DPDK
+driver for `pf_port` and then added to `self.ports`.
+
+Args:
+num: The number of VFs to create. Must be greater than 0.
+pf_port: The PF to create the VFs on.


We should check that the passed port actually resides on this node.


+dpdk_driver: Optional driver to bind the VFs to after they are 
created. Defaults to the
+DPDK driver of `pf_port`.
+
+Raises:
+InternalError: If `num` is less than or equal to 0.
+"""
+if num <= 0:
+raise InternalError(
+"Method for creating virtual functions received a non-positive 
value."
+)
+if not dpdk_driver:
+dpdk_driver = pf_port.os_driver_for_dpdk
+# Get any other port that is on the same device which DTS is aware of
+all_device_ports = [
+p for p in self.ports if p.pci.split(".")[0] == 
pf_port.pci.split(".")[0]
+]


Maybe we should create a PciAddress class that would process the address 
and provide useful methods, one of which we'd use here.



+# Ports must be bound to the kernel driver in order to create VFs from 
them
+for port in all_device_ports:
+self._bind_port_to_driver(port, False)
+# Some PMDs require the interface being up in order to make VFs


These are OS drivers, not PMDs.


+self.configure_port_state(port)
+created_vfs = self.main_session.set_num_virtual_functions(num, pf_port)
+# We don't need more then `num` VFs from the list
+vf_pcis = self.main_session.get_pci_addr_of_vfs(pf_port)[:num]
+devbind_info = self.main_session.send_command(
+f"{self.path_to_devbind_script} -s", privileged=True
+).stdout


This looks like a good candidate for TextParser.


+
+ret = []
+
+for pci in vf_pcis:
+original_driver = re.search(f"{pci}.*drv=([\\d\\w-]*)", 
devbind_info)
+os_driver = original_driver[1] if original_driver else 
pf_port.os_driver
+vf_config = PortConfig(
+self.name, pci, dpdk_driver, os_driver, pf_port.peer.node, 
pf_port.peer.pci
+)
+vf_port = VirtualFunction(self.name, vf_config, created_vfs, 
pf_port)
+self.main_session.update_ports([vf_port])


This should be called after the for cycle so we only call it once. We 
can bind all VF ports after (again, preferably with just one call).

[PATCH v3 0/1] dts: adjust packet addressing

[jspewock]

From: Jeremy Spewock 

v3:
 * split send_packets into a different patch
 * updated code comments
 * moved variable delcaration

Jeremy Spewock (1):
  dts: rework packet addressing

 dts/framework/test_suite.py | 75 ++---
 1 file changed, 54 insertions(+), 21 deletions(-)

-- 
2.46.0

[PATCH v3 1/1] dts: rework packet addressing

[jspewock]

From: Jeremy Spewock 

This patch updates the _adjust_addresses method of test suites so
that addresses of packets are only modified if the developer did not
configure them beforehand. This allows for developers to have more
control over the content of their packets when sending them through the
framework.

Signed-off-by: Jeremy Spewock 
---
 dts/framework/test_suite.py | 75 ++---
 1 file changed, 54 insertions(+), 21 deletions(-)

diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
index 051509fb86..69388ff5ab 100644
--- a/dts/framework/test_suite.py
+++ b/dts/framework/test_suite.py
@@ -225,7 +225,7 @@ def send_packets_and_capture(
 Returns:
 A list of received packets.
 """
-packets = [self._adjust_addresses(packet) for packet in packets]
+packets = self._adjust_addresses(packets)
 return self.tg_node.send_packets_and_capture(
 packets,
 self._tg_port_egress,
@@ -243,41 +243,74 @@ def get_expected_packet(self, packet: Packet) -> Packet:
 Returns:
 `packet` with injected L2/L3 addresses.
 """
-return self._adjust_addresses(packet, expected=True)
+return self._adjust_addresses([packet], expected=True)[0]
 
-def _adjust_addresses(self, packet: Packet, expected: bool = False) -> 
Packet:
+def _adjust_addresses(self, packets: list[Packet], expected: bool = False) 
-> list[Packet]:
 """L2 and L3 address additions in both directions.
 
+Packets in `packets` will be directly modified in this method. The 
returned list of packets
+however will be copies of the modified packets in order to keep the 
two lists distinct.
+
+Only missing addresses are added to packets, existing addresses will 
not be overridden. If
+any packet in `packets` has multiple IP layers (using GRE, for 
example) only the inner-most
+IP layer will have its addresses adjusted.
+
 Assumptions:
 Two links between SUT and TG, one link is TG -> SUT, the other SUT 
-> TG.
 
 Args:
-packet: The packet to modify.
+packets: The packets to modify.
 expected: If :data:`True`, the direction is SUT -> TG,
 otherwise the direction is TG -> SUT.
+
+Returns:
+A list containing copies of all packets in `packets` after 
modification.
 """
-if expected:
-# The packet enters the TG from SUT
-# update l2 addresses
-packet.src = self._sut_port_egress.mac_address
-packet.dst = self._tg_port_ingress.mac_address
+ret_packets = []
+for packet in packets:
+# The fields parameter of a packet does not include fields of the 
payload, so this can
+# only be the Ether src/dst.
+pkt_src_is_unset = "src" not in packet.fields
+pkt_dst_is_unset = "dst" not in packet.fields
+num_ip_layers = packet.layers().count(IP)
+
+if num_ip_layers > 0:
+# Update the last IP layer if there are multiple (the 
framework should be modifying
+# the packet address instead of the tunnel address if there is 
one).
+l3_to_use = packet.getlayer(IP, num_ip_layers)
+ip_src_is_unset = "src" not in l3_to_use.fields
+ip_dst_is_unset = "dst" not in l3_to_use.fields
+else:
+ip_src_is_unset = None
+ip_dst_is_unset = None
 
-# The packet is routed from TG egress to TG ingress
-# update l3 addresses
-packet.payload.src = self._tg_ip_address_egress.ip.exploded
-packet.payload.dst = self._tg_ip_address_ingress.ip.exploded
-else:
-# The packet leaves TG towards SUT
 # update l2 addresses
-packet.src = self._tg_port_egress.mac_address
-packet.dst = self._sut_port_ingress.mac_address
+# If `expected` is :data:`True`, the packet enters the TG from 
SUT, otherwise the
+# packet leaves the TG towards the SUT
+if pkt_src_is_unset:
+packet.src = (
+self._sut_port_egress.mac_address
+if expected
+else self._tg_port_egress.mac_address
+)
+if pkt_dst_is_unset:
+packet.dst = (
+self._tg_port_ingress.mac_address
+if expected
+else self._sut_port_ingress.mac_address
+)
 
-# The packet is routed from TG egress to TG ingress
 # update l3 addresses
-packet.payload.src = self._tg_ip_address_egress.ip.exploded
-packet.payload.dst = self._tg_ip_address_ingress.ip.exploded
+# The packet is routed from TG egress to TG ingress regardless of 
whether it i

[PATCH v1] dts: add send_packets to test_suite

[jspewock]

From: Jeremy Spewock 

Currently the only methods provided in the test suite class for sending
packets capture the resulting received traffic after sending. There is,
in some cases, a need to send multiple packets at once while not really
needing to capture any of said received traffic. It is favorable to
avoid capturing received traffic when you don't need it since not all
traffic generators will necessarily be capturing traffic generators.
The method to fulfill this need exists in the traffic generator
already, but this patch exposes the method to test suites.

Depends-on: patch-10 ("dts: rework packet addressing")

Signed-off-by: Jeremy Spewock 
---
 dts/framework/test_suite.py| 12 
 dts/framework/testbed_model/tg_node.py |  9 +
 2 files changed, 21 insertions(+)

diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
index 69388ff5ab..8945663bae 100644
--- a/dts/framework/test_suite.py
+++ b/dts/framework/test_suite.py
@@ -234,6 +234,18 @@ def send_packets_and_capture(
 duration,
 )
 
+def send_packets(
+self,
+packets: list[Packet],
+) -> None:
+"""Send packets using the traffic generator and do not capture 
received traffic.
+
+Args:
+packets: Packets to send.
+"""
+packets = self._adjust_addresses(packets)
+self.tg_node.send_packets(packets, self._tg_port_egress)
+
 def get_expected_packet(self, packet: Packet) -> Packet:
 """Inject the proper L2/L3 addresses into `packet`.
 
diff --git a/dts/framework/testbed_model/tg_node.py 
b/dts/framework/testbed_model/tg_node.py
index 19b5b6e74c..4179365abb 100644
--- a/dts/framework/testbed_model/tg_node.py
+++ b/dts/framework/testbed_model/tg_node.py
@@ -83,6 +83,15 @@ def send_packets_and_capture(
 duration,
 )
 
+def send_packets(self, packets: list[Packet], port: Port):
+"""Send packets without capturing resulting received packets.
+
+Args:
+packets: Packets to send.
+port: Port to send the packets on.
+"""
+self.traffic_generator.send_packets(packets, port)
+
 def close(self) -> None:
 """Free all resources used by the node.
 
-- 
2.46.0

[PATCH v5 2/2] dts: add dynamic queue test suite

[jspewock]

From: Jeremy Spewock 

This patch adds a new test suite that is designed to test the stopping
and modification of port queues at runtime. Specifically, there are
test cases that display the ports ability to stop some queues but still
send and receive traffic on others, as well as the ability to configure
the ring size of the queue without blocking the traffic on other queues.

Depends-on: patch-11 ("dts: add send_packets to test_suite")

Signed-off-by: Jeremy Spewock 
---
 dts/framework/config/conf_yaml_schema.json |   3 +-
 dts/tests/TestSuite_dynamic_queue_conf.py  | 286 +
 2 files changed, 288 insertions(+), 1 deletion(-)
 create mode 100644 dts/tests/TestSuite_dynamic_queue_conf.py

diff --git a/dts/framework/config/conf_yaml_schema.json 
b/dts/framework/config/conf_yaml_schema.json
index df390e8ae2..11c6f25aa1 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -187,7 +187,8 @@
   "enum": [
 "hello_world",
 "os_udp",
-"pmd_buffer_scatter"
+"pmd_buffer_scatter",
+"dynamic_queue_conf"
   ]
 },
 "test_target": {
diff --git a/dts/tests/TestSuite_dynamic_queue_conf.py 
b/dts/tests/TestSuite_dynamic_queue_conf.py
new file mode 100644
index 00..f5c667cdeb
--- /dev/null
+++ b/dts/tests/TestSuite_dynamic_queue_conf.py
@@ -0,0 +1,286 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2024 University of New Hampshire
+
+"""Dynamic configuration of port queues test suite.
+
+This test suite tests the support of being able to either stop or reconfigure 
port queues at
+runtime without stopping the entire device. Previously, to configure a DPDK 
ethdev, the application
+first specifies how many Tx and Rx queues to include in the ethdev and then 
application sets up
+each queue individually. Only once all the queues have been set up can the 
application then start
+the device, and at this point traffic can flow. If device stops, this halts 
the flow of traffic on
+all queues in the ethdev completely. Dynamic queue is a capability present on 
some NICs that
+specifies whether the NIC is able to delay the configuration of queues on its 
port. This capability
+allows for the support of stopping and reconfiguring queues on a port at 
runtime without stopping
+the entire device.
+
+Support of this capability is shown by starting the Poll Mode Driver with 
multiple Rx and Tx queues
+configured and stopping some prior to forwarding packets, then examining 
whether or not the stopped
+ports and the unmodified ports were able to handle traffic. In addition to 
just stopping the ports,
+the ports must also show that they support configuration changes on their 
queues at runtime without
+stopping the entire device. This is shown by changing the ring size of the 
queues.
+
+If the Poll Mode Driver is able to stop some queues on a port and modify them 
then handle traffic
+on the unmodified queues while the others are stopped, then it is the case 
that the device properly
+supports dynamic configuration of its queues.
+"""
+
+import random
+from typing import Callable, ClassVar, MutableSet
+
+from scapy.layers.inet import IP  # type: ignore[import-untyped]
+from scapy.layers.l2 import Ether  # type: ignore[import-untyped]
+from scapy.packet import Raw  # type: ignore[import-untyped]
+
+from framework.exception import InteractiveCommandExecutionError
+from framework.params.testpmd import PortTopology, SimpleForwardingModes
+from framework.remote_session.testpmd_shell import TestPmdShell
+from framework.test_suite import TestSuite
+
+
+def setup_and_teardown_test(
+test_meth: Callable[
+["TestDynamicQueueConf", int, MutableSet, MutableSet, TestPmdShell, 
bool], None
+],
+) -> Callable[["TestDynamicQueueConf", bool], None]:
+"""Decorator that provides a setup and teardown for testing methods.
+
+This decorator provides a method that sets up the environment for testing, 
runs the test
+method, and then does a clean-up verification step after the queues are 
started again. The
+decorated method will be provided with all the variables it should need to 
run testing
+including: The ID of the port where the queues for testing reside, 
disjoint sets of IDs for
+queues that are/aren't modified, a testpmd session to run testing with, 
and a flag that
+indicates whether or not testing should be done on Rx or Tx queues.
+
+Args:
+test_meth: The decorated method that tests configuration of port 
queues at runtime.
+This method must have the following parameters in order: An int 
that represents a
+port ID, a set of queues for testing, a set of unmodified queues, 
a testpmd
+interactive shell, and a boolean that, when :data:`True`, does Rx 
testing,
+otherwise does Tx testing. This method must also be a member of the
+:class:`TestDynamicQueueConf` class.
+
+Returns:
+

[PATCH v5 0/2] dts: add dynamic queue configuration test suite

[jspewock]

From: Jeremy Spewock 

v5:
 * applied on next-dts

Jeremy Spewock (2):
  dts: add port queue modification and forwarding stats to testpmd
  dts: add dynamic queue test suite

 dts/framework/config/conf_yaml_schema.json|   3 +-
 dts/framework/remote_session/testpmd_shell.py | 233 +-
 dts/tests/TestSuite_dynamic_queue_conf.py | 286 ++
 3 files changed, 519 insertions(+), 3 deletions(-)
 create mode 100644 dts/tests/TestSuite_dynamic_queue_conf.py

-- 
2.46.0

[PATCH v5 1/2] dts: add port queue modification and forwarding stats to testpmd

[jspewock]

From: Jeremy Spewock 

This patch adds methods for querying and modifying port queue state and
configuration. In addition to this, it also adds the ability to capture
the forwarding statistics that get outputted when you send the "stop"
command in testpmd. Querying of port queue information is handled
through a TextParser dataclass in case there is future need for using
more of the output from the command used to query the information.

Signed-off-by: Jeremy Spewock 
---

The rxq info command has been added to the capabilities patch series
so this version should likely extend from that to utilize that version
of the TextParser if it is preferred. Additionally, the test suite
itself would benefit from a capability check to make sure individual
queues can be stopped/started/modified. I will look into adding this in
a future version.

 dts/framework/remote_session/testpmd_shell.py | 233 +-
 1 file changed, 231 insertions(+), 2 deletions(-)

diff --git a/dts/framework/remote_session/testpmd_shell.py 
b/dts/framework/remote_session/testpmd_shell.py
index 77902a468d..119c5e0cba 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -20,7 +20,7 @@
 from dataclasses import dataclass, field
 from enum import Flag, auto
 from pathlib import PurePath
-from typing import Any, Callable, ClassVar, Concatenate, ParamSpec
+from typing import Any, Callable, ClassVar, Concatenate, ParamSpec, cast
 
 from typing_extensions import Self, Unpack
 
@@ -545,6 +545,56 @@ class TestPmdPort(TextParser):
 )
 
 
+@dataclass
+class TestPmdPortQueue(TextParser):
+"""Dataclass representation of the common parts of the testpmd `show 
rxq/txq info` commands."""
+
+#:
+prefetch_threshold: int = field(metadata=TextParser.find_int(r"prefetch 
threshold: (\d+)"))
+#:
+host_threshold: int = field(metadata=TextParser.find_int(r"host threshold: 
(\d+)"))
+#:
+writeback_threshold: int = field(metadata=TextParser.find_int(r"writeback 
threshold: (\d+)"))
+#:
+free_threshold: int = field(metadata=TextParser.find_int(r"free threshold: 
(\d+)"))
+#:
+deferred_start: bool = field(metadata=TextParser.find("deferred start: 
on"))
+#: The number of RXD/TXDs is just the ring size of the queue.
+ring_size: int = field(metadata=TextParser.find_int(r"Number of 
(?:RXDs|TXDs): (\d+)"))
+#:
+is_queue_started: bool = field(metadata=TextParser.find("queue state: 
started"))
+#:
+burst_mode: str | None = field(
+default=None, metadata=TextParser.find(r"Burst mode: ([^\r\n]+)")
+)
+
+
+@dataclass
+class TestPmdTxPortQueue(TestPmdPortQueue):
+"""Dataclass representation for testpmd `show txq info` command."""
+
+#:
+rs_threshold: int | None = field(
+default=None, metadata=TextParser.find_int(r"RS threshold: (\d+)")
+)
+
+
+@dataclass
+class TestPmdRxPortQueue(TestPmdPortQueue):
+"""Dataclass representation for testpmd `show rxq info` command."""
+
+#:
+mempool: str | None = field(default=None, 
metadata=TextParser.find(r"Mempool: ([^\r\n]+)"))
+#:
+can_drop_packets: bool | None = field(
+default=None, metadata=TextParser.find(r"drop packets: on")
+)
+#:
+is_scattering_packets: bool | None = field(
+default=None, metadata=TextParser.find(r"scattered packets: on")
+)
+
+
 @dataclass
 class TestPmdPortStats(TextParser):
 """Port statistics."""
@@ -714,7 +764,7 @@ def start(self, verify: bool = True) -> None:
 "Not all ports came up after starting packet 
forwarding in testpmd."
 )
 
-def stop(self, verify: bool = True) -> None:
+def stop(self, verify: bool = True) -> str:
 """Stop packet forwarding.
 
 Args:
@@ -722,6 +772,9 @@ def stop(self, verify: bool = True) -> None:
 forwarding was stopped successfully or not started. If neither 
is found, it is
 considered an error.
 
+Returns:
+Output gathered from sending the stop command.
+
 Raises:
 InteractiveCommandExecutionError: If `verify` is :data:`True` and 
the command to stop
 forwarding results in an error.
@@ -734,6 +787,7 @@ def stop(self, verify: bool = True) -> None:
 ):
 self._logger.debug(f"Failed to stop packet forwarding: 
\n{stop_cmd_output}")
 raise InteractiveCommandExecutionError("Testpmd failed to stop 
packet forwarding.")
+return stop_cmd_output
 
 def get_devices(self) -> list[TestPmdDevice]:
 """Get a list of device names that are known to testpmd.
@@ -981,6 +1035,181 @@ def set_port_mtu_all(self, mtu: int, verify: bool = 
True) -> None:
 for port in self.ports:
 self.set_port_mtu(port.id, mtu, verify)
 
+def show_port_queue_info(
+self, port_id: int, queue_id: int, is_rx_queue: bool
+) -> TestPmdPortQueue:
+   

RE: [PATCH v2 0/7] ethdev: jump to table support

[Dariusz Sosnowski]

> -Original Message-
> From: Alexander Kozyrev 
> Sent: Wednesday, September 25, 2024 20:05
> To: dev@dpdk.org
> Cc: Dariusz Sosnowski ; Ori Kam
> ; NBU-Contact-Thomas Monjalon (EXTERNAL)
> ; Matan Azrad ;
> ferruh.yi...@amd.com; step...@networkplumber.org
> Subject: [PATCH v2 0/7] ethdev: jump to table support
> 
> Introduce new Flow API JUMP_TO_TABLE_INDEX action.
> It allows bypassing a hierarchy of groups and going directly to a specified 
> flow
> table. That gives a user the flexibility to jump between different priorities 
> in a
> group and eliminates the need to do a table lookup in the group hierarchy.
> The JUMP_TO_TABLE_INDEX action forwards a packet to the specified rule
> index inside the index-based flow table.
> 
> The current index-based flow table doesn't do any matching on the packet and
> executes the actions immediately.
> Add a new index-based flow table with pattern matching.
> The JUMP_TO_TABLE_INDEX can redirect a packet to another matching criteria
> at the specified index in this case.
> 
> RFC:
> https://patchwork.dpdk.org/project/dpdk/patch/20240822202753.3856703-
> 1-akozy...@nvidia.com/
> v2: added trace point to flow insertion by index functions.
> 
> Alexander Kozyrev (7):
>   ethdev: add insertion by index with pattern
>   app/testpmd: add index with pattern insertion type
>   ethdev: add flow rule insertion by index with pattern
>   app/testpmd: add insertion by index with pattern option
>   ethdev: add jump to table index action
>   app/testpmd: add jump to table index action
>   ethdev: add trace points to flow insertion by index
> 
>  app/test-pmd/cmdline_flow.c| 44 +-
>  app/test-pmd/config.c  | 22 +--
>  app/test-pmd/testpmd.h |  2 +-
>  doc/guides/prog_guide/rte_flow.rst | 20 +++
>  doc/guides/rel_notes/release_24_11.rst | 13 +
>  lib/ethdev/ethdev_trace.h  | 44 ++
>  lib/ethdev/ethdev_trace_points.c   |  6 ++
>  lib/ethdev/rte_flow.c  | 72 ++-
>  lib/ethdev/rte_flow.h  | 81 ++
>  lib/ethdev/rte_flow_driver.h   | 14 +
>  lib/ethdev/version.map |  1 +
>  11 files changed, 309 insertions(+), 10 deletions(-)
> 
> --
> 2.18.2

Series-acked-by: Dariusz Sosnowski 

Best regards,
Dariusz Sosnowski