Re: [PATCH] doc: fix test eventdev example commands

[Jerin Jacob]

On Fri, Jan 5, 2024 at 9:24 PM  wrote:
>
> From: Pavan Nikhilesh 
>
> Fix incorrect core masks in testeventdev example
> commands.
>
> Fixes: f6dda59153f1 ("doc: add order queue test in eventdev test guide")
> Fixes: dd37027f2ba6 ("doc: add order all types queue test in eventdev test 
> guide")
> Fixes: 43bc2fef79cd ("doc: add perf queue test in eventdev test guide")
> Fixes: b3d4e665ed3d ("doc: add perf all types queue test in eventdev test 
> guide")
> Fixes: b01974da9f25 ("app/eventdev: add ethernet device producer option")
> Fixes: ba9de463abeb ("doc: add pipeline queue test in testeventdev guide")
> Fixes: d1b46daf7484 ("doc: add pipeline atq test in testeventdev guide")
> Fixes: d008f20bce23 ("app/eventdev: add event timer adapter as a producer")
> Fixes: 2eaa37b86635 ("app/eventdev: add vector mode in pipeline test")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Pavan Nikhilesh 

>
> -   sudo /app/dpdk-test-eventdev --vdev=event_octeontx -- \
> +   sudo /app/dpdk-test-eventdev -c 0x1f --vdev=event_octeontx -- \
>  --test=order_atq --plcores 1 --wlcores 2,3

Since you are here, Could you remove "--vdev=event_octeontx" from the
documentation, as there are
HW PMDs without vdev now. This doc was created there was only
event_octeontx PMD.

Re: [dpdk-dev] [RFC] ethdev: support Tx queue free descriptor query

[Bruce Richardson]

On Mon, Jan 08, 2024 at 10:15:40PM +0100, Morten Brørup wrote:
> > From: Bruce Richardson [mailto:bruce.richard...@intel.com]
> > Sent: Monday, 8 January 2024 11.54
> > 
> > On Tue, Dec 19, 2023 at 10:59:48PM +0530, jer...@marvell.com wrote:
> > > From: Jerin Jacob 
> > >
> > > Introduce a new API to retrieve the number of available free
> > descriptors
> > > in a Tx queue. Applications can leverage this API in the fast path to
> > > inspect the Tx queue occupancy and take appropriate actions based on
> > the
> > > available free descriptors.
> > >
> > > A notable use case could be implementing Random Early Discard (RED)
> > > in software based on Tx queue occupancy.
> > >
> > > Signed-off-by: Jerin Jacob 
> > > ---
> > 
> > Hi Jerin,
> > 
> > while I don't strongly object to this patch, I wonder if it encourages
> > sub-optimal code implementations. To determine the number of free
> > descriptors in a ring, the driver in many cases will need to do a scan
> > of
> > the descriptor ring to see how many are free/used. However, I suspect
> > that
> > in most cases we will see something like the following being done:
> > 
> > count = rte_eth_rx_free_count();
> 
> Typo: rte_eth_rx_free_count() -> rte_eth_tx_free_count()
> 
> > if (count > X) {
> > /* Do something */
> > }
> > 
> > For instances like this, scanning the entire ring is wasteful of
> > resources.
> > Instead it would be better to just check the descriptor at position X
> > explicitly. Going to the trouble of checking the exact descriptor count
> > is
> > unnecessary in this case.
> 
> Yes, it introduces such a risk.
> All DPDK examples simply call tx_burst() without checking free space first, 
> so I think the probability (of the simple case) is low.
> And the probability for the case comparing to X could be mitigated by 
> referring to rte_eth_tx_descriptor_status() in the function description.
> 
> > 
> > Out of interest, are you aware of a case where an app would need to
> > know
> > exactly the number of free descriptors, and where the result would not
> > be
> > just compared to one or more threshold values? Do we see cases where it
> > would be used in a computation, perhaps?
> 
> Yes: RED. When exceeding the minimum threshold, the probability of 
> marking/dropping a packet increases linearly with the queue's fill level.
> E.g.:
> 0-900 packets in queue: don't drop,
> 901-1000 packets in queue: probability of dropping the packet is 1-100 % 
> (e.g. 980 packets in queue = 80 % drop probability).
> 
Ok, thanks for the info. All good with me so.

/Bruce

Re: 21.11.6 patches review and test

[Kevin Traynor]

On 03/01/2024 14:43, Ali Alnubani wrote:
>> -Original Message-
>> From: Kevin Traynor 
>> Sent: Wednesday, December 20, 2023 3:23 PM
>> To: sta...@dpdk.org
>> Cc: dev@dpdk.org; Abhishek Marathe ;
>> Ali Alnubani ; benjamin.wal...@intel.com; David
>> Christensen ; Hemant Agrawal
>> ; Ian Stokes ; Jerin Jacob
>> ; John McNamara ; Ju-
>> Hyoung Lee ; Kevin Traynor ;
>> Luca Boccassi ; Pei Zhang ;
>> qian.q...@intel.com; Raslan Darawsheh ; NBU-
>> Contact-Thomas Monjalon (EXTERNAL) ;
>> yangh...@redhat.com; yuan.p...@intel.com; zhaoyan.c...@intel.com
>> Subject: 21.11.6 patches review and test
>>
>> Hi all,
>>
>> Here is a list of patches targeted for stable release 21.11.6.
>>
>> The planned date for the final release is 12 January.
>>
>> Please help with testing and validation of your use cases and report
>> any issues/results with reply-all to this mail. For the final release
>> the fixes and reported validations will be added to the release notes.
>>
>> A release candidate tarball can be found at:
>>
>> https://dpdk.org/browse/dpdk-stable/tag/?id=v21.11.6-rc1
>>
>> These patches are located at branch 21.11 of dpdk-stable repo:
>> https://dpdk.org/browse/dpdk-stable/
>>
>> Thanks.
>>
> 
> Hello,
> 
> We ran the following functional tests with Nvidia hardware on 21.11.6-rc1:
> - Basic functionality:
>   Send and receive multiple types of traffic.
> - testpmd xstats counter test.
> - testpmd timestamp test.
> - Changing/checking link status through testpmd.
> - rte_flow tests 
> (https://doc.dpdk.org/guides/nics/mlx5.html#supported-hardware-offloads)
> - RSS tests.
> - VLAN filtering, stripping, and insertion tests.
> - Checksum and TSO tests.
> - ptype tests.
> - link_status_interrupt example application tests.
> - l3fwd-power example application tests.
> - Multi-process example applications tests.
> - Hardware LRO tests.
> - Regex application tests.
> - Buffer Split tests.
> - Tx scheduling tests.
> 
> Functional tests ran on:
> - NIC: ConnectX-6 Dx / OS: Ubuntu 20.04 / Driver: 
> MLNX_OFED_LINUX-23.10-1.1.9.0 / Firmware: 22.39.2048
> - NIC: ConnectX-7 / OS: Ubuntu 20.04 / Driver: MLNX_OFED_LINUX-23.10-1.1.9.0 
> / Firmware: 28.39.2048
> - DPU: BlueField-2 / DOCA SW version: 2.5.0 / Firmware: 24.39.2048
> 
> Additionally, we ran build tests with multiple configurations on the 
> following OS/driver combinations (all passed):
> - Ubuntu 20.04.6 with MLNX_OFED_LINUX-23.10-1.1.9.0.
> - Ubuntu 20.04.6 with rdma-core master (9016f34).
> - Ubuntu 20.04.6 with rdma-core v28.0.
> - Fedora 38 with rdma-core v44.0.
> - Fedora 40 (Rawhide) with rdma-core v48.0.
> - OpenSUSE Leap 15.5 with rdma-core v42.0.
> - Windows Server 2019 with Clang 16.0.6.
> 
> We don't see new issues caused by the changes in this release.
> 
> Thanks,
> Ali

Thanks Ali. Will add to the validation notes,
Kevin.

Re: 21.11.6 patches review and test

[Kevin Traynor]

On 20/12/2023 13:22, Kevin Traynor wrote:
> Hi all,
> 
> Here is a list of patches targeted for stable release 21.11.6.
> 
> The planned date for the final release is 12 January.
> 

I have validation reports for Red Hat and Nvidia. Thank you.

Is there any validation results from Intel, or an ETA ?

> Please help with testing and validation of your use cases and report
> any issues/results with reply-all to this mail. For the final release
> the fixes and reported validations will be added to the release notes.
> 
> A release candidate tarball can be found at:
> 
> https://dpdk.org/browse/dpdk-stable/tag/?id=v21.11.6-rc1
> 
> These patches are located at branch 21.11 of dpdk-stable repo:
> https://dpdk.org/browse/dpdk-stable/
> 
> Thanks.
> 
> Kevin
> 

RE: [PATCH 2/2] common/idpf: enable AVX2 for single queue Tx

[Huang, ZhiminX]

> -Original Message-
> From: Wenzhuo Lu 
> Sent: Thursday, December 7, 2023 2:35 PM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo 
> Subject: [PATCH 2/2] common/idpf: enable AVX2 for single queue Tx
> 
> In case some CPUs don't support AVX512. Enable AVX2 for them to get better
> per-core performance.
> 
> Signed-off-by: Wenzhuo Lu 
> ---

Verified idpf single queue + AVX2 in functional regression test, tested pass.
Tested-by: Zhimin Huang 

Re: vmxnet3 no longer functional on DPDK 21.11

[Bruce Richardson]

On Sat, Jan 06, 2024 at 08:50:03AM -0600, Lewis Donzis wrote:
> Good morning.
> 
> I just wanted to mention that this problem still persists in 22.11.3, and we 
> still have to patch the vmxnet3 driver every time we upgrade.
> 
> As mentioned before, ixgbe_ethdev.c is an example of a driver that ifdef's 
> out the attempt to use interrupts on FreeBSD.
> 
> Thanks,
> lew
> 
> 
> - On Jun 3, 2022, at 8:19 AM, Lewis Donzis l...@perftech.com wrote:
> 
> > Hi, all.
> > 
> > Resurrecting this thread from six months ago, I apologize for not having 
> > more
> > time to dig into it, but in light of recent findings, I see numerous other
> > drivers and other parts of the code that have comments to the effect that
> > "FreeBSD doesn't support interrupts" and they effectively #ifdef out the
> > attempt.
> > 
> > Could this be as simple as needing to do the same in vmxnet3?  Empirically,
> > ignoring the error from rte_intr_enable() allows the driver to work 
> > normally,
> > for what that's worth.
> > 

I'm not at all familiar with the vmxnet3 driver, so apologies for the lack
of response up till now. Does something like the below simple fix work for
you? If so, I'm happy enough to submit as a patch for upstream merge and
then backport.

/Bruce

diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c 
b/drivers/net/vmxnet3/vmxnet3_ethdev.c
index e49191718a..d088b42d35 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
@@ -1129,6 +1129,7 @@ vmxnet3_dev_start(struct rte_eth_dev *dev)
/* Setting proper Rx Mode and issue Rx Mode Update command */
vmxnet3_dev_set_rxmode(hw, VMXNET3_RXM_UCAST | VMXNET3_RXM_BCAST, 1);
 
+#ifndef RTE_EXEC_ENV_FREEBSD
/* Setup interrupt callback  */
rte_intr_callback_register(dev->intr_handle,
   vmxnet3_interrupt_handler, dev);
@@ -1140,6 +1141,7 @@ vmxnet3_dev_start(struct rte_eth_dev *dev)
 
/* enable all intrs */
vmxnet3_enable_all_intrs(hw);
+#endif
 
vmxnet3_process_events(dev);
 

[PATCH v11] net/iavf: add diagnostic support in TX path

[Mingjin Ye]

Implemented a Tx wrapper to perform a thorough check on mbufs,
categorizing and counting invalid cases by types for diagnostic
purposes. The count of invalid cases is accessible through xstats_get.

Also, the devarg option "mbuf_check" was introduced to configure the
diagnostic parameters to enable the appropriate diagnostic features.

supported cases: mbuf, size, segment, offload.
 1. mbuf: check for corrupted mbuf.
 2. size: check min/max packet length according to hw spec.
 3. segment: check number of mbuf segments not exceed hw limitation.
 4. offload: check any unsupported offload flag.

parameter format: "mbuf_check=" or "mbuf_check=[,]"
eg: dpdk-testpmd -a :81:01.0,mbuf_check=[mbuf,size] -- -i

Signed-off-by: Mingjin Ye 
---
v2: Remove call chain.
---
v3: Optimisation implementation.
---
v4: Fix Windows os compilation error.
---
v5: Split Patch.
---
v6: remove strict.
---
v9: Modify the description document.
---
v10: Modify vf rst document.
---
v11: modify comment log.
---
 doc/guides/nics/intel_vf.rst   | 11 
 drivers/net/iavf/iavf.h| 11 
 drivers/net/iavf/iavf_ethdev.c | 72 +
 drivers/net/iavf/iavf_rxtx.c   | 98 ++
 drivers/net/iavf/iavf_rxtx.h   |  2 +
 5 files changed, 194 insertions(+)

diff --git a/doc/guides/nics/intel_vf.rst b/doc/guides/nics/intel_vf.rst
index ce96c2e1f8..f62bb4233c 100644
--- a/doc/guides/nics/intel_vf.rst
+++ b/doc/guides/nics/intel_vf.rst
@@ -111,6 +111,17 @@ For more detail on SR-IOV, please refer to the following 
documents:
 by setting the ``devargs`` parameter like ``-a 
18:01.0,no-poll-on-link-down=1``
 when IAVF is backed by an Intel\ |reg| E810 device or an Intel\ |reg| 700 
Series Ethernet device.
 
+When IAVF is backed by an Intel\ |reg| E810 device or an Intel\ |reg| 700 
series Ethernet devices.
+Set the ``devargs`` parameter ``mbuf_check`` to enable TX diagnostics. For 
example,
+``-a 18:01.0,mbuf_check=`` or ``-a 
18:01.0,mbuf_check=[,...]``. Also,
+``xstats_get`` can be used to get the error counts, which are collected in 
``tx_mbuf_error_packets``
+xstats. For example, ``testpmd> show port xstats all``. Supported cases:
+
+*   mbuf: Check for corrupted mbuf.
+*   size: Check min/max packet length according to hw spec.
+*   segment: Check number of mbuf segments not exceed hw limitation.
+*   offload: Check any unsupported offload flag.
+
 The PCIE host-interface of Intel Ethernet Switch FM1 Series VF 
infrastructure
 
^
 
diff --git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h
index ab24cb02c3..824ae4aa02 100644
--- a/drivers/net/iavf/iavf.h
+++ b/drivers/net/iavf/iavf.h
@@ -114,9 +114,14 @@ struct iavf_ipsec_crypto_stats {
} ierrors;
 };
 
+struct iavf_mbuf_stats {
+   uint64_t tx_pkt_errors;
+};
+
 struct iavf_eth_xstats {
struct virtchnl_eth_stats eth_stats;
struct iavf_ipsec_crypto_stats ips_stats;
+   struct iavf_mbuf_stats mbuf_stats;
 };
 
 /* Structure that defines a VSI, associated with a adapter. */
@@ -310,6 +315,7 @@ struct iavf_devargs {
uint32_t watchdog_period;
int auto_reset;
int no_poll_on_link_down;
+   uint64_t mbuf_check;
 };
 
 struct iavf_security_ctx;
@@ -353,6 +359,11 @@ enum iavf_tx_burst_type {
IAVF_TX_AVX512_CTX_OFFLOAD,
 };
 
+#define IAVF_MBUF_CHECK_F_TX_MBUF(1ULL << 0)
+#define IAVF_MBUF_CHECK_F_TX_SIZE(1ULL << 1)
+#define IAVF_MBUF_CHECK_F_TX_SEGMENT (1ULL << 2)
+#define IAVF_MBUF_CHECK_F_TX_OFFLOAD (1ULL << 3)
+
 /* Structure to store private data for each VF instance. */
 struct iavf_adapter {
struct iavf_hw hw;
diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index 1fb876e827..fca57b50b3 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -39,6 +40,7 @@
 #define IAVF_RESET_WATCHDOG_ARG"watchdog_period"
 #define IAVF_ENABLE_AUTO_RESET_ARG "auto_reset"
 #define IAVF_NO_POLL_ON_LINK_DOWN_ARG "no-poll-on-link-down"
+#define IAVF_MBUF_CHECK_ARG   "mbuf_check"
 uint64_t iavf_timestamp_dynflag;
 int iavf_timestamp_dynfield_offset = -1;
 int rte_pmd_iavf_tx_lldp_dynfield_offset = -1;
@@ -49,6 +51,7 @@ static const char * const iavf_valid_args[] = {
IAVF_RESET_WATCHDOG_ARG,
IAVF_ENABLE_AUTO_RESET_ARG,
IAVF_NO_POLL_ON_LINK_DOWN_ARG,
+   IAVF_MBUF_CHECK_ARG,
NULL
 };
 
@@ -175,6 +178,7 @@ static const struct rte_iavf_xstats_name_off 
rte_iavf_stats_strings[] = {
{"tx_broadcast_packets", _OFF_OF(eth_stats.tx_broadcast)},
{"tx_dropped_packets", _OFF_OF(eth_stats.tx_discards)},
{"tx_error_packets", _OFF_OF(eth_stats.tx_errors)},
+   {"tx_mbuf_error_packets", _OFF_OF(mbuf_stats.tx_pkt_errors)},
 
{"inline_ipsec

Re: [PATCH v1] net/axgbe: read and save the port property register

[Ferruh Yigit]

On 1/5/2024 11:32 AM, Venkat Kumar Ande wrote:
> From: Venkat Kumar Ande 
> 
> Read and save the port property registers once during the device probe
> and then use the saved values as they are needed.
> 

Hi Venkat,

Can you please describe what is the motivation/reason for the change?

Is it addressing a functional problem or refactoring for coming feature
etc...?

> Signed-off-by: Venkat Kumar Ande 
> ---
>  drivers/net/axgbe/axgbe_ethdev.c   | 21 +
>  drivers/net/axgbe/axgbe_ethdev.h   |  7 +++
>  drivers/net/axgbe/axgbe_phy_impl.c | 68 --
>  3 files changed, 48 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/net/axgbe/axgbe_ethdev.c 
> b/drivers/net/axgbe/axgbe_ethdev.c
> index f174d46143..3450374535 100644
> --- a/drivers/net/axgbe/axgbe_ethdev.c
> +++ b/drivers/net/axgbe/axgbe_ethdev.c
> @@ -2342,23 +2342,28 @@ eth_axgbe_dev_init(struct rte_eth_dev *eth_dev)
>   pdata->arcache = AXGBE_DMA_OS_ARCACHE;
>   pdata->awcache = AXGBE_DMA_OS_AWCACHE;
>  
> + /* Read the port property registers */
> + pdata->pp0 = XP_IOREAD(pdata, XP_PROP_0);
> + pdata->pp1 = XP_IOREAD(pdata, XP_PROP_1);
> + pdata->pp2 = XP_IOREAD(pdata, XP_PROP_2);
> + pdata->pp3 = XP_IOREAD(pdata, XP_PROP_3);
> + pdata->pp4 = XP_IOREAD(pdata, XP_PROP_4);
> +

<...>

RE: [PATCH v1] net/axgbe: read and save the port property register

[Ande, Venkat Kumar]

[AMD Official Use Only - General]

Hi Ferruh,

This change will be refactoring for future code readability of AMD AXGBE driver.

Regards,
Venkat

-Original Message-
From: Yigit, Ferruh 
Sent: Tuesday, January 9, 2024 4:05 PM
To: Ande, Venkat Kumar ; dev@dpdk.org
Cc: Sebastian, Selwin 
Subject: Re: [PATCH v1] net/axgbe: read and save the port property register

On 1/5/2024 11:32 AM, Venkat Kumar Ande wrote:
> From: Venkat Kumar Ande 
>
> Read and save the port property registers once during the device probe
> and then use the saved values as they are needed.
>

Hi Venkat,

Can you please describe what is the motivation/reason for the change?

Is it addressing a functional problem or refactoring for coming feature etc...?

> Signed-off-by: Venkat Kumar Ande 
> ---
>  drivers/net/axgbe/axgbe_ethdev.c   | 21 +
>  drivers/net/axgbe/axgbe_ethdev.h   |  7 +++
>  drivers/net/axgbe/axgbe_phy_impl.c | 68
> --
>  3 files changed, 48 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/net/axgbe/axgbe_ethdev.c
> b/drivers/net/axgbe/axgbe_ethdev.c
> index f174d46143..3450374535 100644
> --- a/drivers/net/axgbe/axgbe_ethdev.c
> +++ b/drivers/net/axgbe/axgbe_ethdev.c
> @@ -2342,23 +2342,28 @@ eth_axgbe_dev_init(struct rte_eth_dev *eth_dev)
>   pdata->arcache = AXGBE_DMA_OS_ARCACHE;
>   pdata->awcache = AXGBE_DMA_OS_AWCACHE;
>
> + /* Read the port property registers */
> + pdata->pp0 = XP_IOREAD(pdata, XP_PROP_0);
> + pdata->pp1 = XP_IOREAD(pdata, XP_PROP_1);
> + pdata->pp2 = XP_IOREAD(pdata, XP_PROP_2);
> + pdata->pp3 = XP_IOREAD(pdata, XP_PROP_3);
> + pdata->pp4 = XP_IOREAD(pdata, XP_PROP_4);
> +

<...>

Re: unnecessary rx callbacks when zero packets

[Ferruh Yigit]

On 1/8/2024 3:20 PM, Konstantin Ananyev wrote:
> 
> rx callbacks when zero packets
>>
>> I noticed while looking at packet capture that currently the receive 
>> callbacks
>> get called even if there are no packets. This seems unnecessary since if
>> nb_rx is zero, then there are no packets to look at.  My one concern is that
>> an application could be using callbacks as some form of scheduling mechanism
>> which would be broken.
> 
> As I remember, original idea was to allow callbacks to inject new packets if 
> needed.
> 

Right, callback function updating 'nb_rx' enables this.

Also Honnappa has a good point that callback can be used to calculate
zero packet polls.

These points are not documented and not obvious from the code,
@Stephen does it make sense to document that callback function called
with zero packet intentionally, in the 'rte_eth_rx_burst()' function
comment?

>>
>> The change would be:
>>
>>
>> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
>> index 21e3a21903ec..f64bf977c46e 100644
>> --- a/lib/ethdev/rte_ethdev.h
>> +++ b/lib/ethdev/rte_ethdev.h
>> @@ -6077,7 +6077,7 @@ rte_eth_rx_burst(uint16_t port_id, uint16_t queue_id,
>> nb_rx = p->rx_pkt_burst(qd, rx_pkts, nb_pkts);
>>
>>  #ifdef RTE_ETHDEV_RXTX_CALLBACKS
>> -   {
>> +   if (nb_rx > 0) {
>> void *cb;
>>
>> /* rte_memory_order_release memory order was used when the

Re: [BUG] [bonding] bonding member delete bug

[Ferruh Yigit]

On 1/8/2024 4:07 PM, Kevin Traynor wrote:
> On 08/01/2024 15:55, Ferruh Yigit wrote:
>> On 12/18/2023 6:37 AM, Simon Jones wrote:
>>> Oh, it's fixed by 0911d4ec and f5e72e8e
>>>
>>
>> Thanks Simon for reporting.
>>
>> Do you know if the above fixes backported to the 21.11.x LTS release?
>>
> 
> Yes, 0911d4ec as part of 18.11 [0] and f5e72e8e backported to 21.11
> branch since v21.11.2 [1]
> 

Thanks Kevin for confirming.

> [0]
> https://git.dpdk.org/dpdk-stable/commit/?h=21.11&id=0911d4ec01839c9149a0df5758d00d9d57a47cea
> 
> [1]
> https://git.dpdk.org/dpdk-stable/commit/?h=21.11&id=5a8afc69afabd3c69efbc1b0c048f31d06f7d875
> 

<...>

Re: [PATCH v6 1/7] dts: add startup verification and forwarding modes to testpmd shell

[Juraj Linkeš]

On Mon, Jan 8, 2024 at 5:36 PM Jeremy Spewock  wrote:
>
>
>
> On Mon, Jan 8, 2024 at 6:35 AM Juraj Linkeš  
> wrote:
>>
>> On Wed, Jan 3, 2024 at 11:33 PM  wrote:
>> >
>> > From: Jeremy Spewock 
>> >
>> > Added commonly used methods in testpmd such as starting and stopping
>> > packet forwarding, changing forward modes, and verifying link status of
>> > ports so that developers can configure testpmd and start forwarding
>> > through the provided class rather than sending commands to the testpmd
>> > session directly.
>> >
>> > Signed-off-by: Jeremy Spewock 
>> > ---
>> >  dts/framework/exception.py|   7 +
>> >  dts/framework/remote_session/testpmd_shell.py | 149 +-
>> >  2 files changed, 155 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/dts/framework/exception.py b/dts/framework/exception.py
>> > index 658eee2c38..cce1e0231a 100644
>> > --- a/dts/framework/exception.py
>> > +++ b/dts/framework/exception.py
>> 
>> > @@ -65,9 +108,66 @@ class TestPmdShell(InteractiveShell):
>> >  _command_extra_chars: ClassVar[str] = "\n"
>> >
>> >  def _start_application(self, get_privileged_command: Callable[[str], 
>> > str] | None) -> None:
>> > -self._app_args += " -- -i"
>> > +"""Overrides :meth:`~.interactive_shell._start_application`.
>> > +
>> > +Add flags for starting testpmd in interactive mode and disabling 
>> > messages for link state
>> > +change events before starting the application. Link state is 
>> > verified before starting
>> > +packet forwarding and the messages create unexpected newlines in 
>> > the terminal which
>> > +complicates output collection.
>> > +
>>
>> We should adjust the collection so that it can handle the newlines.
>> Also, can you explain exactly why we are disabling the initial link
>> state messages?
>
>
> The problem really comes from the newlines causing the prompt to exist in the 
> buffer before any command is sent. So, what ends up happening is after 
> starting the application these link state change events happen at some point, 
> and they cause an empty "testpmd>" line to exist in the buffer and the next 
> time you send a command it will stop as soon as it encounters that line.

These buffer issues keep cropping up. We should think about making
this more robust. Can we flush the buffer before sending a new command
(because any previous output is irrelevant)? This probably won't fix
all the problems, but it sounds like it could help. Maybe it could
help with the scapy docstring issue we're seen in the past as well.

In this patch though, we should just make this functional (I
understand disabling the messages achieves that) and address the
buffer issues in a separate patch.

> An additional issue with this prompt is it is put in the buffer before the 
> link state change event occurs, and there is no prompt that appears after the 
> event messages, just an empty line. This makes it much more difficult to 
> detect when the link state change event occurs and consume it because the 
> event isn't captured the next time you collect output, all that is consumed 
> is a line containing the prompt.. So, this makes you essentially one 
> command's worth of output behind because the next time you send a command you 
> will consume what you were supposed to get from the last command where you 
> stopped early, and this causes false positives for things like the link state 
> detection method and failures in output verification.
> This puts you in a position where the only way you can really detect that one 
> of these events happened is either assuming that only getting an empty prompt 
> means one of these events happened, or trying to consume output twice and 
> looking ahead to see if one of these events happened. However, because we 
> wouldn't be doing anything with these events and we verify link status before 
> starting anyway, it seemed like the less complex but still functional 
> solution would just be to mask these events.

Right, these are basically random events, which makes it hard to
collect (but not impossible). Checking the status explicitly is way
better. Thanks for the explanation.

>
>>
>>
>> > +Also find the number of pci addresses which were allowed on the 
>> > command line when the app
>> > +was started.
>> > +"""
>> > +self._app_args += " -- -i --mask-event intr_lsc"
>> > +self.number_of_ports = self._app_args.count("-a ")
>> >  super()._start_application(get_privileged_command)
>> >
>> > +def start(self, verify: bool = True) -> None:
>> > +"""Start packet forwarding with the current configuration.
>> > +
>> > +Args:
>> > +verify: If :data:`True` , a second start command will be sent 
>> > in an attempt to verify
>> > +packet forwarding started as expected.
>> > +
>> > +Raises:
>> > +InteractiveCommandExecutionError: If `verify` is :data:`True` 
>>

[Bug 1344] Split testbed and test configuration

[bugzilla]

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

Bug ID: 1344
   Summary: Split testbed and test configuration
   Product: DPDK
   Version: unspecified
  Hardware: All
OS: All
Status: UNCONFIRMED
  Severity: normal
  Priority: Normal
 Component: dts
  Assignee: dev@dpdk.org
  Reporter: juraj.lin...@pantheon.tech
CC: juraj.lin...@pantheon.tech, pr...@iol.unh.edu
  Target Milestone: ---

The current conf.yaml combines both the testbed (machines) configuration and
the test configuration. Splitting the two into different files could improve
user experience.

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

Re: [dpdk-dev] [v5] doc: define qualification criteria for external library

[Jerin Jacob]

On Mon, Jan 8, 2024 at 6:57 PM Hemant Agrawal  wrote:
>
>
> On 08-Jan-24 2:01 PM, Jerin Jacob wrote:
> > On Mon, Jan 8, 2024 at 1:47 PM Hemant Agrawal  
> > wrote:
> >>
> >>
> >> On 08-Jan-24 1:28 PM, jer...@marvell.com wrote:
> >>> From: Jerin Jacob 
> >>>
> >>> Define qualification criteria for external library
> >>> based on a techboard meeting minutes [1] and past
> >>> learnings from mailing list discussion.
> >>>
> >>> [1]
> >>> http://mails.dpdk.org/archives/dev/2019-June/135847.html
> >>> https://mails.dpdk.org/archives/dev/2024-January/284849.html
> >>>
> >>> Signed-off-by: Jerin Jacob 
> >>> Acked-by: Thomas Monjalon 
> >
> >>> +#. **Distributions License:**
> >>> +
> >>> +   - No specific constraints beyond documentation.
> >>
> >> Though we are not mandatory open distribution. However we should ask for 
> >> the
> >> defining the distribution aspect clearly in the library.
> >
> > How about following then,
> >
> > No specific constraints, but clear documentation on distribution usage
> > aspects is required.
> >
> > If not, please suggest the exact wording.
>
> I think above is ok.

I will add it next version.

>
>
> >

Re: [dpdk-dev] [v5] doc: define qualification criteria for external library

[Jerin Jacob]

On Tue, Jan 9, 2024 at 1:25 AM Morten Brørup  wrote:
>
> > From: Stephen Hemminger [mailto:step...@networkplumber.org]
> > Sent: Monday, 8 January 2024 18.19
> >
> > The wording specifies the intent here, and that is not what matters.
> > This is not a legal document where someone will take us to court
> > if the board doesn't accept a library.
>
> The introduction/preamble of the page should mention this, or we risk 
> upsetting people very much if we don't accept a compliant library. E.g.:
>
> This document serves as an overall description of what is expected of 
> dependent libraries.

The similar text already present.

> The final decision to accept or reject is at the discretion of the DPDK 
> Project's Technical Board.

I will add the above in next version.


[main]dell[dpdk.org] $ git diff
diff --git a/doc/guides/contributing/library_dependency.rst
b/doc/guides/contributing/library_dependency.rst
index 94025fdf60..f60ed92a64 100644
--- a/doc/guides/contributing/library_dependency.rst
+++ b/doc/guides/contributing/library_dependency.rst
@@ -6,6 +6,7 @@ External Library dependency

 This document defines the qualification criteria for external
libraries that may be
 used as dependencies in DPDK drivers or libraries.
+The final decision to accept or reject is at the discretion of the
DPDK Project's Technical Board.


>

Re: [dpdk-dev] [v5] doc: define qualification criteria for external library

[Jerin Jacob]

On Mon, Jan 8, 2024 at 2:55 PM Morten Brørup  wrote:
>
> > From: jer...@marvell.com [mailto:jer...@marvell.com]
> > Sent: Monday, 8 January 2024 08.59
> >
> > Define qualification criteria for external library
> > based on a techboard meeting minutes [1] and past
> > learnings from mailing list discussion.
>
> According to the DPDK project charter, the Governing Board deals with legal 
> and licensing issues, so we need their approval before publishing this.
> Perhaps the Governing Board should be invited to join the discussion?

+ govbo...@dpdk.org

Not sure. I will Cc the govbo...@dpdk.org in next version of patch anyway.

>
> >
> > [1]
> > http://mails.dpdk.org/archives/dev/2019-June/135847.html
> > https://mails.dpdk.org/archives/dev/2024-January/284849.html
> >
> > Signed-off-by: Jerin Jacob 
> > Acked-by: Thomas Monjalon 
> > ---
> >  doc/guides/contributing/index.rst |  1 +
> >  .../contributing/library_dependency.rst   | 52 +++
> >  2 files changed, 53 insertions(+)
> >  create mode 100644 doc/guides/contributing/library_dependency.rst
> >
> > v5:
> > - Added "Dependency nature" section based on Stephen's input
> >
> > v4:
> > - Address Thomas comments from
> > https://patches.dpdk.org/project/dpdk/patch/20240105121215.3950532-1-
> > jer...@marvell.com/
> >
> > v3:
> > - Updated the content based on TB discussion which is documented at
> > https://mails.dpdk.org/archives/dev/2024-January/284849.html
> >
> > v2:
> > - Added "Meson build integration" and "Code readability" sections.
> >
> >
> > diff --git a/doc/guides/contributing/index.rst
> > b/doc/guides/contributing/index.rst
> > index dcb9b1fbf0..e5a8c2b0a3 100644
> > --- a/doc/guides/contributing/index.rst
> > +++ b/doc/guides/contributing/index.rst
> > @@ -15,6 +15,7 @@ Contributor's Guidelines
> >  documentation
> >  unit_test
> >  new_library
> > +library_dependency
> >  patches
> >  vulnerability
> >  stable
> > diff --git a/doc/guides/contributing/library_dependency.rst
> > b/doc/guides/contributing/library_dependency.rst
> > new file mode 100644
> > index 00..94025fdf60
> > --- /dev/null
> > +++ b/doc/guides/contributing/library_dependency.rst
> > @@ -0,0 +1,52 @@
> > +.. SPDX-License-Identifier: BSD-3-Clause
> > +   Copyright(c) 2024 Marvell.
> > +
> > +External Library dependency
> > +===
> > +
> > +This document defines the qualification criteria for external
> > libraries that may be
> > +used as dependencies in DPDK drivers or libraries.
>
> More background information could be added here, for context.
>
> Although DPDK is a BSD licensed project, we want to open the door for non-BSD 
> licensed external libraries in those drivers and libraries, where the 
> developer has the choice to omit them at build time. But not in the core 
> parts of DPDK, which must remain fully BSD licensed.
>
> Stephen shared some concerns about source code availability, so DPDK doesn't 
> become a shim for a bunch of binary blobs, like some other "open" project (I 
> cannot remember the name of the project he mentioned). We are allowing binary 
> blobs, but it would be nice if we could somehow state our intentions in this 
> regard.
>
> > +
> > +#. **Documentation:**
> > +
> > +   - Must have adequate documentation for the steps to build it.
> > +   - Must have clear license documentation on distribution and usage
> > aspects of external library.
> > +
> > +#. **Free availability:**
> > +
> > +   - The library must be freely available to build in either source or
> > binary form.
> > +   - It shall be downloadable from a direct link. There shall not be
> > any requirement to explicitly
> > + login or sign a user agreement.
>
> Remove "explicitly".

Adding "explicitly" was decided in the meeting. There may “implicit"
signing of user agreement by the distribution installation procedure.
Like clicking allowing to install non permissive license libraries in
the context of general OS installation. Not at the time of
installing a specific package.


>
> > +
> > +#. **Usage License:**
> > +
> > +   - Both permissive (e.g., BSD-3 or Apache) and non-permissive (e.g.,
> > GPLv3) licenses are acceptable.
> > +   - In the case of a permissive license, automatic inclusion in the
> > build process is assumed.
> > + For non-permissive licenses, an additional build configuration
> > option is required.
>
> We must ensure that automatic inclusion only applies to libraries with a 
> license that allows both free usage and free distribution. IANAL, so please 
> confirm that this bullet covers both?
>
> The default DPDK build should not include anything that is not freely 
> distributable, comes with an EULA or any other limitations or restrictions. 
> Optimally, the default DPDK build should remain 100 % compatible with the BSD 
> license. DPDK is a BSD licensed project, so any deviations from the BSD 
> license must be explicitly selected (opt-in) at build time.
>
> > +
> > +

RE: [PATCH v2 1/3] ethdev: rename action modify field data structure

[Ori Kam]

> -Original Message-
> From: Suanming Mou 
> Sent: Tuesday, December 19, 2023 3:34 AM
> 
> Current rte_flow_action_modify_data struct describes the pkt
> field perfectly and is used only in action.
> 
> It is planned to be used for item as well. This commit renames
> it to "rte_flow_field_data" making it compatible to be used by item.
> 
> Signed-off-by: Suanming Mou 
> ---

Acked-by: Ori Kam 
Best,
Ori

[dpdk-dev] [v6] doc: define qualification criteria for external library

[jerinj]

From: Jerin Jacob 

Define qualification criteria for external library
based on a techboard meeting minutes [1] and past
learnings from mailing list discussion.

[1]
http://mails.dpdk.org/archives/dev/2019-June/135847.html
https://mails.dpdk.org/archives/dev/2024-January/284849.html

Signed-off-by: Jerin Jacob 
Acked-by: Thomas Monjalon 
---

v6:
- Address Morten's comments at 
https://mails.dpdk.org/archives/dev/2024-January/285029.html

v5:
- Added "Dependency nature" section based on Stephen's input

v4:
- Address Thomas comments from 
https://patches.dpdk.org/project/dpdk/patch/20240105121215.3950532-1-jer...@marvell.com/

v3:
- Updated the content based on TB discussion which is documented at
https://mails.dpdk.org/archives/dev/2024-January/284849.html

v2:
- Added "Meson build integration" and "Code readability" sections.

 doc/guides/contributing/index.rst |  1 +
 .../contributing/library_dependency.rst   | 53 +++
 2 files changed, 54 insertions(+)
 create mode 100644 doc/guides/contributing/library_dependency.rst

diff --git a/doc/guides/contributing/index.rst 
b/doc/guides/contributing/index.rst
index dcb9b1fbf0..e5a8c2b0a3 100644
--- a/doc/guides/contributing/index.rst
+++ b/doc/guides/contributing/index.rst
@@ -15,6 +15,7 @@ Contributor's Guidelines
 documentation
 unit_test
 new_library
+library_dependency
 patches
 vulnerability
 stable
diff --git a/doc/guides/contributing/library_dependency.rst 
b/doc/guides/contributing/library_dependency.rst
new file mode 100644
index 00..3b275f1c52
--- /dev/null
+++ b/doc/guides/contributing/library_dependency.rst
@@ -0,0 +1,53 @@
+.. SPDX-License-Identifier: BSD-3-Clause
+   Copyright(c) 2024 Marvell.
+
+External Library dependency
+===
+
+This document defines the qualification criteria for external libraries that 
may be
+used as dependencies in DPDK drivers or libraries.
+The final decision to accept or reject is at the discretion of the DPDK 
Project's Technical Board.
+
+#. **Documentation:**
+
+   - Must have adequate documentation for the steps to build it.
+   - Must have clear license documentation on distribution and usage aspects 
of external library.
+
+#. **Free availability:**
+
+   - The library must be freely available to build in either source or binary 
form.
+   - It shall be downloadable from a direct link. There shall not be any 
requirement to explicitly
+ login or sign a user agreement.
+
+#. **Usage License:**
+
+   - Both permissive (e.g., BSD-3 or Apache) and non-permissive (e.g., GPLv3) 
licenses are acceptable.
+   - In the case of a permissive license, automatic inclusion in the build 
process is assumed.
+ For non-permissive licenses, an additional build configuration option is 
required.
+
+#. **Distribution License:**
+
+   - No specific constraints, but clear documentation on distribution usage 
aspects is required.
+
+#. **Compiler compatibility:**
+
+   - The library must be able to compile with a DPDK supported compiler for 
the given target
+ environment.
+ For example, for Linux, the library must be able to compile with GCC 
and/or clang.
+   - Library may be limited to a specific OS and/or specific hardware.
+
+#. **Meson build integration:**
+
+   - The library must have standard method like ``pkg-config`` for seamless 
integration with
+ DPDK's build environment.
+
+#. **Code readability:**
+
+   - Optional dependencies should use stubs to minimize ``ifdef`` clutter, 
promoting improved
+ code readability.
+
+#. **Dependency nature:**
+
+   - The external library dependency must be optional.
+ i.e Missing external library must not impact the core functionality of 
the DPDK, specific
+ library and/or driver will not be built if dependencies are not met.
-- 
2.43.0

[PATCH] net/vmxnet3: fix use of interrupts on FreeBSD

[Bruce Richardson]

DPDK does not support interrupts on FreeBSD, so the vmxnet3 driver
returns error when enabling interrupts as it initializes. We can fix
this by #ifdef'ing out the interrupt calls when building for FreeBSD,
allowing the driver to initialize correctly.

Fixes: 046f11619567 ("net/vmxnet3: support MSI-X interrupt")

Reported-by: Lewis Donzis 
Signed-off-by: Bruce Richardson 
---
 drivers/net/vmxnet3/vmxnet3_ethdev.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c 
b/drivers/net/vmxnet3/vmxnet3_ethdev.c
index e49191718a..7032f0e324 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
@@ -257,6 +257,7 @@ vmxnet3_disable_all_intrs(struct vmxnet3_hw *hw)
vmxnet3_disable_intr(hw, i);
 }
 
+#ifndef RTE_EXEC_ENV_FREEBSD
 /*
  * Enable all intrs used by the device
  */
@@ -280,6 +281,7 @@ vmxnet3_enable_all_intrs(struct vmxnet3_hw *hw)
vmxnet3_enable_intr(hw, i);
}
 }
+#endif
 
 /*
  * Gets tx data ring descriptor size.
@@ -1129,6 +1131,7 @@ vmxnet3_dev_start(struct rte_eth_dev *dev)
/* Setting proper Rx Mode and issue Rx Mode Update command */
vmxnet3_dev_set_rxmode(hw, VMXNET3_RXM_UCAST | VMXNET3_RXM_BCAST, 1);
 
+#ifndef RTE_EXEC_ENV_FREEBSD
/* Setup interrupt callback  */
rte_intr_callback_register(dev->intr_handle,
   vmxnet3_interrupt_handler, dev);
@@ -1140,6 +1143,7 @@ vmxnet3_dev_start(struct rte_eth_dev *dev)
 
/* enable all intrs */
vmxnet3_enable_all_intrs(hw);
+#endif
 
vmxnet3_process_events(dev);
 
-- 
2.42.0

RE: [PATCH v2 2/3] ethdev: add compare item

[Ori Kam]

> -Original Message-
> From: Suanming Mou 
> Sent: Tuesday, December 19, 2023 3:34 AM
> 
> The new item type is added for the case user wants to match traffic
> based on packet field compare result with other fields or immediate
> value.
> 
> e.g. take advantage the compare item user will be able to accumulate
> a IPv4/TCP packet's TCP data_offset and IPv4 IHL field to a tag
> register, then compare the tag register with IPv4 header total length
> to understand the packet has payload or not.
> 
> The supported operations can be as below:
>  - RTE_FLOW_ITEM_COMPARE_EQ (equal)
>  - RTE_FLOW_ITEM_COMPARE_NE (not equal)
>  - RTE_FLOW_ITEM_COMPARE_LT (less than)
>  - RTE_FLOW_ITEM_COMPARE_LE (less than or equal)
>  - RTE_FLOW_ITEM_COMPARE_GT (great than)
>  - RTE_FLOW_ITEM_COMPARE_GE (great than or equal)
> 
> Signed-off-by: Suanming Mou 
> ---

Acked-by: Ori Kam 
Best,
Ori

RE: [PATCH v2 3/3] net/mlx5: add compare item support

[Ori Kam]

> -Original Message-
> From: Suanming Mou 
> Sent: Tuesday, December 19, 2023 3:34 AM
> The compare item allows adding flow match with comparison
> result. This commit adds compare item support to the PMD
> code.
> 
> Due to HW limitation:
>  - Only HWS supported.
>  - Only 32-bit comparison is supported.
>  - Only single compare flow is supported in the flow table.
>  - Only match with compare result between packet fields is
> supported.
> 
> Signed-off-by: Suanming Mou 
> ---

Acked-by: Ori Kam 
Best
Ori

Re: vmxnet3 no longer functional on DPDK 21.11

[Bruce Richardson]

On Tue, Jan 09, 2024 at 07:46:47AM -0600, Lewis Donzis wrote:
> Hi, Bruce.
> 
> I'm even less familiar with it, but we do quite a lot of testing using VMs, 
> so it's been quite handy.
> 
> Your patch seems very reasonable, however it also produces a warning:
> 
> ../drivers/net/vmxnet3/vmxnet3_ethdev.c:264:1: warning: unused function 
> 'vmxnet3_enable_all_intrs' [-Wunused-function]
> 
> Adding an #ifndef around vmxnet3_enable_all_intrs() eliminates that warning.

Right, I should have compile-tested on FreeBSD myself, before sending the
suggestion. Patch has now been submitted. Please test and ack if the fix
works for your use-cases, thanks.

> 
> Please pardon the uninformed view, but we've been using FreeBSD + DPDK for 
> nearly a decade, and I thought the whole point was to avoid using interrupts. 
>  We have no need or desire for them in our applications, so we just hope the 
> sprinkling of interrupt support code throughout the drivers doesn't cause any 
> harm.  But I also realize we're probably in the minority on this.
> 
In general, yes we try and avoid interrupts on the data-path or fast-path
and use polling. However, for some use-cases where traffic levels are low,
interrupts may make sense to save power for fast-path. Even if not,
interrupts are useful for things like error conditions or for monitoring
link-status changes (LSC). Unfortunately, we don't have any interrupt
support on BSD, so fixes like this are necessary.

/Bruce

Re: [PATCH v6 1/7] dts: add startup verification and forwarding modes to testpmd shell

[Jeremy Spewock]

On Tue, Jan 9, 2024 at 6:55 AM Juraj Linkeš 
wrote:

> On Mon, Jan 8, 2024 at 5:36 PM Jeremy Spewock 
> wrote:
> >
> >
> >
> > On Mon, Jan 8, 2024 at 6:35 AM Juraj Linkeš 
> wrote:
> >>
> >> On Wed, Jan 3, 2024 at 11:33 PM  wrote:
> >> >
> >> > From: Jeremy Spewock 
> >> >
> >> > Added commonly used methods in testpmd such as starting and stopping
> >> > packet forwarding, changing forward modes, and verifying link status
> of
> >> > ports so that developers can configure testpmd and start forwarding
> >> > through the provided class rather than sending commands to the testpmd
> >> > session directly.
> >> >
> >> > Signed-off-by: Jeremy Spewock 
> >> > ---
> >> >  dts/framework/exception.py|   7 +
> >> >  dts/framework/remote_session/testpmd_shell.py | 149
> +-
> >> >  2 files changed, 155 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/dts/framework/exception.py b/dts/framework/exception.py
> >> > index 658eee2c38..cce1e0231a 100644
> >> > --- a/dts/framework/exception.py
> >> > +++ b/dts/framework/exception.py
> >> 
> >> > @@ -65,9 +108,66 @@ class TestPmdShell(InteractiveShell):
> >> >  _command_extra_chars: ClassVar[str] = "\n"
> >> >
> >> >  def _start_application(self, get_privileged_command:
> Callable[[str], str] | None) -> None:
> >> > -self._app_args += " -- -i"
> >> > +"""Overrides :meth:`~.interactive_shell._start_application`.
> >> > +
> >> > +Add flags for starting testpmd in interactive mode and
> disabling messages for link state
> >> > +change events before starting the application. Link state is
> verified before starting
> >> > +packet forwarding and the messages create unexpected
> newlines in the terminal which
> >> > +complicates output collection.
> >> > +
> >>
> >> We should adjust the collection so that it can handle the newlines.
> >> Also, can you explain exactly why we are disabling the initial link
> >> state messages?
> >
> >
> > The problem really comes from the newlines causing the prompt to exist
> in the buffer before any command is sent. So, what ends up happening is
> after starting the application these link state change events happen at
> some point, and they cause an empty "testpmd>" line to exist in the buffer
> and the next time you send a command it will stop as soon as it encounters
> that line.
>
> These buffer issues keep cropping up. We should think about making
> this more robust. Can we flush the buffer before sending a new command
> (because any previous output is irrelevant)? This probably won't fix
> all the problems, but it sounds like it could help. Maybe it could
>

I definitely think this would be something that is useful because it allows
us to at least make some more helpful assumptions like the buffer only
contains the output after you sent your command. When I was first looking
into making the interactive shells I was looking for ways to do this and
the only way I could really find for flushing the buffer is just consuming
everything that's in it. One way to do this is just to keep collecting
output until you reach the timeout which occurs when there is nothing left.
Waiting for a timeout has its flaws as well, but I believe it was the
cleanest way to make sure the buffer was empty. Overall though, I agree
this would be something good to look into and I'd happily explore expanding
this and also looking into things we discussed earlier like changing the
delimiter on the buffer.


> help with the scapy docstring issue we're seen in the past as well.
>

I'm not sure it would do much for this issue because this stemmed more from
the multiline command I believe. However, making output collection more
robust does also make the opportunity to fix these issues by doing things
like removing empty lines before sending as a sanitizing step.


> In this patch though, we should just make this functional (I
> understand disabling the messages achieves that) and address the
> buffer issues in a separate patch.
>
> > An additional issue with this prompt is it is put in the buffer before
> the link state change event occurs, and there is no prompt that appears
> after the event messages, just an empty line. This makes it much more
> difficult to detect when the link state change event occurs and consume it
> because the event isn't captured the next time you collect output, all that
> is consumed is a line containing the prompt.. So, this makes you
> essentially one command's worth of output behind because the next time you
> send a command you will consume what you were supposed to get from the last
> command where you stopped early, and this causes false positives for things
> like the link state detection method and failures in output verification.
> > This puts you in a position where the only way you can really detect
> that one of these events happened is either assuming that only getting an
> empty prompt means one of these events happened, or trying to consum

Re: [dpdk-dev] [RFC] ethdev: support Tx queue free descriptor query

[Jerin Jacob]

On Tue, Jan 9, 2024 at 2:24 AM Morten Brørup  wrote:
>
> > From: Konstantin Ananyev [mailto:konstantin.anan...@huawei.com]
> > Sent: Friday, 5 January 2024 12.13
> >
> > > From: Thomas Monjalon 
> > > Sent: Friday, January 5, 2024 10:04 AM
> > >
> > > 05/01/2024 10:57, Jerin Jacob:
> > > > On Thu, Jan 4, 2024 at 11:59 PM Thomas Monjalon
> >  wrote:
> > > > >
> > > > > 04/01/2024 15:21, Konstantin Ananyev:
> > > > > >
> > > > > > > > > Introduce a new API to retrieve the number of available
> > free descriptors
> > > > > > > > > in a Tx queue. Applications can leverage this API in the
> > fast path to
> > > > > > > > > inspect the Tx queue occupancy and take appropriate
> > actions based on the
> > > > > > > > > available free descriptors.
> > > > > > > > >
> > > > > > > > > A notable use case could be implementing Random Early
> > Discard (RED)
> > > > > > > > > in software based on Tx queue occupancy.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Jerin Jacob 
> > > > > > > >
> > > > > > > > I think having an API to get the number of free descriptors
> > per queue is a good idea. Why have it only for TX queues and not
> > > for RX
> > > > > > > queues as well?
> > > > > > >
> > > > > > > I see no harm in adding for Rx as well. I think, it is better
> > to have
> > > > > > > separate API for each instead of adding argument as it is
> > fast path
> > > > > > > API.
> > > > > > > If so, we could add a new API when there is any PMD
> > implementation or
> > > > > > > need for this.
> > > > > >
> > > > > > I think for RX we already have similar one:
> > > > > > /** @internal Get number of used descriptors on a receive
> > queue. */
> > > > > > typedef uint32_t (*eth_rx_queue_count_t)(void *rxq);
> > > > >
> > > > > rte_eth_rx_queue_count() gives the number of Rx used descriptors
> > > > > rte_eth_rx_descriptor_status() gives the status of one Rx
> > descriptor
> > > > > rte_eth_tx_descriptor_status() gives the status of one Tx
> > descriptor
> > > > >
> > > > > This patch is adding a function to get Tx available descriptors,
> > > > > rte_eth_tx_queue_free_desc_get().
> > > > > I can see a symmetry with rte_eth_rx_queue_count().
> > > > > For consistency I would rename it to
> > rte_eth_tx_queue_free_count().
>
> For consistency with rte_eth_rx_queue_count() regarding both naming and 
> behavior of the API, I would prefer:
>
> rte_eth_tx_queue_count(), returning the number of used descriptors.

Ack. I will change to "used" version.

>
> > > > >
> > > > > Should we add rte_eth_tx_queue_count() and
> > rte_eth_rx_queue_free_count()?
> > > >
> > > > IMO, rte_eth_rx_queue_free_count() is enough as
> > > > used count =  total desc number(configured via nb_tx_desc with
> > > > rte_eth_tx_queue_setup())  - free count
> > >
> > > I'm fine with that.
> > >
> >
> > Yep, agree.
> > If we ever need  rte_eth_rx_queue_free_count() and
> > rte_eth_tx_queue_used_count(),
> > it could be done via slow-path as Jerin outlined above, no need to
> > waste entries in fp_ops
> > for that.
>
> Yes, rte_eth_rx/tx_queue_avail_count() could be added in the ethdev library, 
> without driver callbacks, but simply getting data from configured descriptor 
> ring sizes and rte_eth_rx/tx_queue_count().
>
> PS: For naming conventions, I sought inspiration from the mempool library. 
> Also, I don't see any need to use "descs" as part of the names of the 
> proposed functions.
>
> The driver callback can be either "free count" or "used count", whichever is 
> easier for the drivers to implement, or (preferably) whichever is likelier to 
> be faster to execute in the most common case. I would assume that the TX 
> descriptor "used count" is relatively low most of the time.

I will change driver callback to "used count" to have better synergy
with public ethdev API.


>

Re: [PATCH v3 5/6] examples/qos_sched: fix lcore ID restriction

[Ferruh Yigit]

On 12/20/2023 4:31 PM, Stephen Hemminger wrote:
> On Wed, 20 Dec 2023 07:45:00 +0100
> Sivaprasad Tummala  wrote:
> 
>> diff --git a/examples/qos_sched/args.c b/examples/qos_sched/args.c
>> index e97273152a..22fe76eeb5 100644
>> --- a/examples/qos_sched/args.c
>> +++ b/examples/qos_sched/args.c
>> @@ -182,10 +182,10 @@ app_parse_flow_conf(const char *conf_str)
>>  
>>  pconf->rx_port = vals[0];
>>  pconf->tx_port = vals[1];
>> -pconf->rx_core = (uint8_t)vals[2];
>> -pconf->wt_core = (uint8_t)vals[3];
>> +pconf->rx_core = (uint16_t)vals[2];
>> +pconf->wt_core = (uint16_t)vals[3];
>>  if (ret == 5)
>> -pconf->tx_core = (uint8_t)vals[4];
>> +pconf->tx_core = (uint16_t)vals[4];
>>  else
>>  pconf->tx_core = pconf->wt_core;
>>  
>> -- 
> 
> Not sure why cast is even needed, assigning uint32_t to uint16_t
> is not going to generate a warning with current compiler settings.
>

I was assuming compiler will complain when assigning uint32_t to
uint16_t, but it seems '-Wconversion' compiler flag is required for this
warning.
Enabling this flag for DPDK build causes lots of warnings, I wonder if
we should add a new buildtype in meson that enables this flag.


And except from compiler warning, I think it is good to keep explicit
cast where assignment can cause change of value. This at worst can work
as documentation that assignment between different types done intentionally.

Re: vmxnet3 no longer functional on DPDK 21.11

[Bruce Richardson]

On Tue, Jan 09, 2024 at 09:21:47AM -0600, Lewis Donzis wrote:
> 
> 
> - On Jan 9, 2024, at 8:28 AM, Bruce Richardson bruce.richard...@intel.com 
> wrote:
> 
> > On Tue, Jan 09, 2024 at 07:46:47AM -0600, Lewis Donzis wrote:
> >> Hi, Bruce.
> >> 
> >> I'm even less familiar with it, but we do quite a lot of testing using 
> >> VMs, so
> >> it's been quite handy.
> >> 
> >> Your patch seems very reasonable, however it also produces a warning:
> >> 
> >> ../drivers/net/vmxnet3/vmxnet3_ethdev.c:264:1: warning: unused function
> >> 'vmxnet3_enable_all_intrs' [-Wunused-function]
> >> 
> >> Adding an #ifndef around vmxnet3_enable_all_intrs() eliminates that 
> >> warning.
> > 
> > Right, I should have compile-tested on FreeBSD myself, before sending the
> > suggestion. Patch has now been submitted. Please test and ack if the fix
> > works for your use-cases, thanks.
> 
> I compiled it and ran it just now and it appears to work just fine.  Thanks 
> very much for submitting.
> 
> > In general, yes we try and avoid interrupts on the data-path or fast-path
> > and use polling. However, for some use-cases where traffic levels are low,
> > interrupts may make sense to save power for fast-path. Even if not,
> > interrupts are useful for things like error conditions or for monitoring
> > link-status changes (LSC). Unfortunately, we don't have any interrupt
> > support on BSD, so fixes like this are necessary.
> 
> That makes sense.  Makes me wonder why there's no interrupt support on BSD, 
> i.e., maybe it's better to fix that than to have to fix "avoiding it" in the 
> drivers?

Sadly, interrupt support was never implemented for BSD, which is why it is
missing. If someone has the time and interest to do up the patches to add
it, it would be a welcome addition to DPDK.

> 
> I kind of feel like we're a bit orphaned in the FreeBSD world.  I don't know 
> how many others are using BSD, but it seems like we're in a relatively 
> less-supported environment.
> 

Linux is the dominant environment for DPDK, which does mean it gets most of
the attention. However, we do want to keep supporting BSD, so please
continue to flag issues to the community as you encounter them. Smaller
bugs we can certainly endeavour to fix, but adding things like interrupt
support probably requires someone to explicitly step up and dedicate time
to implementing it properly.

Regards,
/Bruce

[PATCH v7 0/7] dts: Port scatter suite over

[jspewock]

From: Jeremy Spewock 

v7:

Addressed comments and made appropriate changes. Most changes had to do
with grammatical/spelling errors and expanding upon documentation. Also
fixed the typing of eal parameters when creating interactive shells
through the SUT node.

Jeremy Spewock (7):
  dts: add startup verification and forwarding modes to testpmd shell
  dts: limit EAL parameters to DPDK apps and add parameters to all apps
  dts: add optional packet filtering to scapy sniffer
  dts: add pci addresses to EAL parameters
  dts: allow configuring MTU of ports
  dts: add scatter to the yaml schema
  dts: add pmd_buffer_scatter test suite

 dts/framework/config/conf_yaml_schema.json|   3 +-
 dts/framework/exception.py|   7 +
 dts/framework/remote_session/testpmd_shell.py | 149 +-
 dts/framework/test_suite.py   |  15 +-
 dts/framework/testbed_model/linux_session.py  |   8 +
 dts/framework/testbed_model/os_session.py |   9 ++
 dts/framework/testbed_model/sut_node.py   |  28 +++-
 dts/framework/testbed_model/tg_node.py|  14 +-
 .../traffic_generator/__init__.py |   7 +-
 .../capturing_traffic_generator.py|  22 ++-
 .../testbed_model/traffic_generator/scapy.py  |  27 
 dts/tests/TestSuite_pmd_buffer_scatter.py | 132 
 12 files changed, 407 insertions(+), 14 deletions(-)
 create mode 100644 dts/tests/TestSuite_pmd_buffer_scatter.py

-- 
2.43.0

[PATCH v7 1/7] dts: add startup verification and forwarding modes to testpmd shell

[jspewock]

From: Jeremy Spewock 

Added commonly used methods in testpmd such as starting and stopping
packet forwarding, changing forward modes, and verifying link status of
ports so that developers can configure testpmd and start forwarding
through the provided class rather than sending commands to the testpmd
session directly.

Signed-off-by: Jeremy Spewock 
---
 dts/framework/exception.py|   7 +
 dts/framework/remote_session/testpmd_shell.py | 149 +-
 2 files changed, 155 insertions(+), 1 deletion(-)

diff --git a/dts/framework/exception.py b/dts/framework/exception.py
index 658eee2c38..cce1e0231a 100644
--- a/dts/framework/exception.py
+++ b/dts/framework/exception.py
@@ -146,6 +146,13 @@ def __str__(self) -> str:
 return f"Command {self.command} returned a non-zero exit code: 
{self._command_return_code}"
 
 
+class InteractiveCommandExecutionError(DTSError):
+"""An unsuccessful execution of a remote command in an interactive 
environment."""
+
+#:
+severity: ClassVar[ErrorSeverity] = ErrorSeverity.REMOTE_CMD_EXEC_ERR
+
+
 class RemoteDirectoryExistsError(DTSError):
 """A directory that exists on a remote node."""
 
diff --git a/dts/framework/remote_session/testpmd_shell.py 
b/dts/framework/remote_session/testpmd_shell.py
index 0184cc2e71..11c5c7f93c 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -15,9 +15,15 @@
 testpmd_shell.close()
 """
 
+import time
+from enum import auto
 from pathlib import PurePath
 from typing import Callable, ClassVar
 
+from framework.exception import InteractiveCommandExecutionError
+from framework.settings import SETTINGS
+from framework.utils import StrEnum
+
 from .interactive_shell import InteractiveShell
 
 
@@ -43,14 +49,51 @@ def __str__(self) -> str:
 return self.pci_address
 
 
+class TestPmdForwardingModes(StrEnum):
+r"""The supported packet forwarding modes for :class:`~TestPmdShell`\s."""
+
+#:
+io = auto()
+#:
+mac = auto()
+#:
+macswap = auto()
+#:
+flowgen = auto()
+#:
+rxonly = auto()
+#:
+txonly = auto()
+#:
+csum = auto()
+#:
+icmpecho = auto()
+#:
+ieee1588 = auto()
+#:
+noisy = auto()
+#:
+fivetswap = "5tswap"
+#:
+shared_rxq = "shared-rxq"
+#:
+recycle_mbufs = auto()
+
+
 class TestPmdShell(InteractiveShell):
 """Testpmd interactive shell.
 
 The testpmd shell users should never use
 the :meth:`~.interactive_shell.InteractiveShell.send_command` method 
directly, but rather
 call specialized methods. If there isn't one that satisfies a need, it 
should be added.
+
+Attributes:
+number_of_ports: The number of ports which were allowed on the 
command-line when testpmd
+was started.
 """
 
+number_of_ports: int
+
 #: The path to the testpmd executable.
 path: ClassVar[PurePath] = PurePath("app", "dpdk-testpmd")
 
@@ -65,9 +108,66 @@ class TestPmdShell(InteractiveShell):
 _command_extra_chars: ClassVar[str] = "\n"
 
 def _start_application(self, get_privileged_command: Callable[[str], str] 
| None) -> None:
-self._app_args += " -- -i"
+"""Overrides :meth:`~.interactive_shell._start_application`.
+
+Add flags for starting testpmd in interactive mode and disabling 
messages for link state
+change events before starting the application. Link state is verified 
before starting
+packet forwarding and the messages create unexpected newlines in the 
terminal which
+complicates output collection.
+
+Also find the number of pci addresses which were allowed on the 
command line when the app
+was started.
+"""
+self._app_args += " -- -i --mask-event intr_lsc"
+self.number_of_ports = self._app_args.count("-a ")
 super()._start_application(get_privileged_command)
 
+def start(self, verify: bool = True) -> None:
+"""Start packet forwarding with the current configuration.
+
+Args:
+verify: If :data:`True` , a second start command will be sent in 
an attempt to verify
+packet forwarding started as expected.
+
+Raises:
+InteractiveCommandExecutionError: If `verify` is :data:`True` and 
forwarding fails to
+start or ports fail to come up.
+"""
+self.send_command("start")
+if verify:
+# If forwarding was already started, sending "start" again should 
tell us
+start_cmd_output = self.send_command("start")
+if "Packet forwarding already started" not in start_cmd_output:
+self._logger.debug(f"Failed to start packet forwarding: 
\n{start_cmd_output}")
+raise InteractiveCommandExecutionError("Testpmd failed to 
start packet forwarding.")
+
+for port_id in range(self.number_of_ports):
+if not

[PATCH v7 2/7] dts: limit EAL parameters to DPDK apps and add parameters to all apps

[jspewock]

From: Jeremy Spewock 

Changed the factory method for creating interactive apps in the SUT Node
so that EAL parameters would only be passed into DPDK apps since
non-DPDK apps wouldn't be able to process them. Also modified
interactive apps to allow for the ability to pass parameters into the
app on startup so that the applications can be started with certain
configuration steps passed on the command line.

Signed-off-by: Jeremy Spewock 
---
 dts/framework/remote_session/testpmd_shell.py |  2 +-
 dts/framework/testbed_model/sut_node.py   | 16 ++--
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/dts/framework/remote_session/testpmd_shell.py 
b/dts/framework/remote_session/testpmd_shell.py
index 11c5c7f93c..8a9578913b 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -118,7 +118,7 @@ def _start_application(self, get_privileged_command: 
Callable[[str], str] | None
 Also find the number of pci addresses which were allowed on the 
command line when the app
 was started.
 """
-self._app_args += " -- -i --mask-event intr_lsc"
+self._app_args += " -i --mask-event intr_lsc"
 self.number_of_ports = self._app_args.count("-a ")
 super()._start_application(get_privileged_command)
 
diff --git a/dts/framework/testbed_model/sut_node.py 
b/dts/framework/testbed_model/sut_node.py
index c4acea38d1..909394e756 100644
--- a/dts/framework/testbed_model/sut_node.py
+++ b/dts/framework/testbed_model/sut_node.py
@@ -430,7 +430,8 @@ def create_interactive_shell(
 shell_cls: Type[InteractiveShellType],
 timeout: float = SETTINGS.timeout,
 privileged: bool = False,
-eal_parameters: EalParameters | str | None = None,
+app_parameters: str = "",
+eal_parameters: EalParameters | None = None,
 ) -> InteractiveShellType:
 """Extend the factory for interactive session handlers.
 
@@ -449,20 +450,23 @@ def create_interactive_shell(
 eal_parameters: List of EAL parameters to use to launch the app. 
If this
 isn't provided or an empty string is passed, it will default 
to calling
 :meth:`create_eal_parameters`.
+app_parameters: Additional arguments to pass into the application 
on the
+command-line.
 
 Returns:
 An instance of the desired interactive application shell.
 """
-if not eal_parameters:
-eal_parameters = self.create_eal_parameters()
-
-# We need to append the build directory for DPDK apps
+# We need to append the build directory and add EAL parameters for 
DPDK apps
 if shell_cls.dpdk_app:
+if not eal_parameters:
+eal_parameters = self.create_eal_parameters()
+app_parameters = f"{eal_parameters} -- {app_parameters}"
+
 shell_cls.path = self.main_session.join_remote_path(
 self.remote_dpdk_build_dir, shell_cls.path
 )
 
-return super().create_interactive_shell(shell_cls, timeout, 
privileged, str(eal_parameters))
+return super().create_interactive_shell(shell_cls, timeout, 
privileged, app_parameters)
 
 def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
 """Bind all ports on the SUT to a driver.
-- 
2.43.0

[PATCH v7 3/7] dts: add optional packet filtering to scapy sniffer

[jspewock]

From: Jeremy Spewock 

Added the options to filter out LLDP and ARP packets when
sniffing for packets with scapy. This was done using BPF filters to
ensure that the noise these packets provide does not interfere with test
cases.

Signed-off-by: Jeremy Spewock 
---
 dts/framework/test_suite.py   | 15 +--
 dts/framework/testbed_model/tg_node.py| 14 --
 .../traffic_generator/__init__.py |  7 -
 .../capturing_traffic_generator.py| 22 ++-
 .../testbed_model/traffic_generator/scapy.py  | 27 +++
 5 files changed, 79 insertions(+), 6 deletions(-)

diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
index dfb391ffbd..ffea917690 100644
--- a/dts/framework/test_suite.py
+++ b/dts/framework/test_suite.py
@@ -38,6 +38,7 @@
 from .settings import SETTINGS
 from .test_result import BuildTargetResult, Result, TestCaseResult, 
TestSuiteResult
 from .testbed_model import Port, PortLink, SutNode, TGNode
+from .testbed_model.traffic_generator import PacketFilteringConfig
 from .utils import get_packet_summaries
 
 
@@ -208,7 +209,12 @@ def configure_testbed_ipv4(self, restore: bool = False) -> 
None:
 def _configure_ipv4_forwarding(self, enable: bool) -> None:
 self.sut_node.configure_ipv4_forwarding(enable)
 
-def send_packet_and_capture(self, packet: Packet, duration: float = 1) -> 
list[Packet]:
+def send_packet_and_capture(
+self,
+packet: Packet,
+filter_config: PacketFilteringConfig = PacketFilteringConfig(),
+duration: float = 1,
+) -> list[Packet]:
 """Send and receive `packet` using the associated TG.
 
 Send `packet` through the appropriate interface and receive on the 
appropriate interface.
@@ -216,6 +222,7 @@ def send_packet_and_capture(self, packet: Packet, duration: 
float = 1) -> list[P
 
 Args:
 packet: The packet to send.
+filter_config: The filter to use when capturing packets.
 duration: Capture traffic for this amount of time after sending 
`packet`.
 
 Returns:
@@ -223,7 +230,11 @@ def send_packet_and_capture(self, packet: Packet, 
duration: float = 1) -> list[P
 """
 packet = self._adjust_addresses(packet)
 return self.tg_node.send_packet_and_capture(
-packet, self._tg_port_egress, self._tg_port_ingress, duration
+packet,
+self._tg_port_egress,
+self._tg_port_ingress,
+filter_config,
+duration,
 )
 
 def get_expected_packet(self, packet: Packet) -> Packet:
diff --git a/dts/framework/testbed_model/tg_node.py 
b/dts/framework/testbed_model/tg_node.py
index f269d4c585..d3206e87e0 100644
--- a/dts/framework/testbed_model/tg_node.py
+++ b/dts/framework/testbed_model/tg_node.py
@@ -15,7 +15,11 @@
 
 from .node import Node
 from .port import Port
-from .traffic_generator import CapturingTrafficGenerator, 
create_traffic_generator
+from .traffic_generator import (
+CapturingTrafficGenerator,
+PacketFilteringConfig,
+create_traffic_generator,
+)
 
 
 class TGNode(Node):
@@ -53,6 +57,7 @@ def send_packet_and_capture(
 packet: Packet,
 send_port: Port,
 receive_port: Port,
+filter_config: PacketFilteringConfig = PacketFilteringConfig(),
 duration: float = 1,
 ) -> list[Packet]:
 """Send `packet`, return received traffic.
@@ -65,13 +70,18 @@ def send_packet_and_capture(
 packet: The packet to send.
 send_port: The egress port on the TG node.
 receive_port: The ingress port in the TG node.
+filter_config: The filter to use when capturing packets.
 duration: Capture traffic for this amount of time after sending 
`packet`.
 
 Returns:
  A list of received packets. May be empty if no packets are 
captured.
 """
 return self.traffic_generator.send_packet_and_capture(
-packet, send_port, receive_port, duration
+packet,
+send_port,
+receive_port,
+filter_config,
+duration,
 )
 
 def close(self) -> None:
diff --git a/dts/framework/testbed_model/traffic_generator/__init__.py 
b/dts/framework/testbed_model/traffic_generator/__init__.py
index 11e2bd7d97..0eaf0355cd 100644
--- a/dts/framework/testbed_model/traffic_generator/__init__.py
+++ b/dts/framework/testbed_model/traffic_generator/__init__.py
@@ -14,11 +14,16 @@
 and a capturing traffic generator is required.
 """
 
+# pylama:ignore=W0611
+
 from framework.config import ScapyTrafficGeneratorConfig, TrafficGeneratorType
 from framework.exception import ConfigurationError
 from framework.testbed_model.node import Node
 
-from .capturing_traffic_generator import CapturingTrafficGenerator
+from .capturing_traffic_generator import (
+CapturingTrafficGenerator,
+PacketFilteringConfig,
+)
 

[PATCH v7 4/7] dts: add pci addresses to EAL parameters

[jspewock]

From: Jeremy Spewock 

Added allow list to the EAL parameters created in DTS to ensure that
only the relevant PCI devices are considered when launching DPDK
applications.

Signed-off-by: Jeremy Spewock 
---
 dts/framework/testbed_model/sut_node.py | 12 
 1 file changed, 12 insertions(+)

diff --git a/dts/framework/testbed_model/sut_node.py 
b/dts/framework/testbed_model/sut_node.py
index 909394e756..97aa26d419 100644
--- a/dts/framework/testbed_model/sut_node.py
+++ b/dts/framework/testbed_model/sut_node.py
@@ -30,6 +30,7 @@
 from .cpu import LogicalCoreCount, LogicalCoreList
 from .node import Node
 from .os_session import InteractiveShellType, OSSession
+from .port import Port
 from .virtual_device import VirtualDevice
 
 
@@ -46,6 +47,7 @@ def __init__(
 prefix: str,
 no_pci: bool,
 vdevs: list[VirtualDevice],
+ports: list[Port],
 other_eal_param: str,
 ):
 """Initialize the parameters according to inputs.
@@ -63,6 +65,7 @@ def __init__(
 VirtualDevice('net_ring0'),
 VirtualDevice('net_ring1')
 ]
+ports: The list of ports to allow.
 other_eal_param: user defined DPDK EAL parameters, e.g.:
 ``other_eal_param='--single-file-segments'``
 """
@@ -73,6 +76,7 @@ def __init__(
 self._prefix = f"--file-prefix={prefix}"
 self._no_pci = "--no-pci" if no_pci else ""
 self._vdevs = " ".join(f"--vdev {vdev}" for vdev in vdevs)
+self._ports = " ".join(f"-a {port.pci}" for port in ports)
 self._other_eal_param = other_eal_param
 
 def __str__(self) -> str:
@@ -83,6 +87,7 @@ def __str__(self) -> str:
 f"{self._prefix} "
 f"{self._no_pci} "
 f"{self._vdevs} "
+f"{self._ports} "
 f"{self._other_eal_param}"
 )
 
@@ -347,6 +352,7 @@ def create_eal_parameters(
 append_prefix_timestamp: bool = True,
 no_pci: bool = False,
 vdevs: list[VirtualDevice] | None = None,
+ports: list[Port] | None = None,
 other_eal_param: str = "",
 ) -> "EalParameters":
 """Compose the EAL parameters.
@@ -370,6 +376,8 @@ def create_eal_parameters(
 VirtualDevice('net_ring0'),
 VirtualDevice('net_ring1')
 ]
+ports: The list of ports to allow. If :data:`None`, all ports 
listed in `self.ports`
+will be allowed.
 other_eal_param: user defined DPDK EAL parameters, e.g.:
 ``other_eal_param='--single-file-segments'``.
 
@@ -388,12 +396,16 @@ def create_eal_parameters(
 if vdevs is None:
 vdevs = []
 
+if ports is None:
+ports = self.ports
+
 return EalParameters(
 lcore_list=lcore_list,
 memory_channels=self.config.memory_channels,
 prefix=prefix,
 no_pci=no_pci,
 vdevs=vdevs,
+ports=ports,
 other_eal_param=other_eal_param,
 )
 
-- 
2.43.0

[PATCH v7 6/7] dts: add scatter to the yaml schema

[jspewock]

From: Jeremy Spewock 

Allow for scatter to be specified in the configuration file.

Signed-off-by: Jeremy Spewock 
---
 dts/framework/config/conf_yaml_schema.json | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/dts/framework/config/conf_yaml_schema.json 
b/dts/framework/config/conf_yaml_schema.json
index 84e45fe3c2..e6dc50ca7f 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -186,7 +186,8 @@
   "type": "string",
   "enum": [
 "hello_world",
-"os_udp"
+"os_udp",
+"pmd_buffer_scatter"
   ]
 },
 "test_target": {
-- 
2.43.0

[PATCH v7 5/7] dts: allow configuring MTU of ports

[jspewock]

From: Jeremy Spewock 

Adds methods in both os_session and linux session to allow for setting
MTU of port interfaces so that suites that require the sending and
receiving of packets of a specific size, or the rejection of packets
over a certain size, can configure this maximum as needed.

Signed-off-by: Jeremy Spewock 
---
 dts/framework/testbed_model/linux_session.py | 8 
 dts/framework/testbed_model/os_session.py| 9 +
 2 files changed, 17 insertions(+)

diff --git a/dts/framework/testbed_model/linux_session.py 
b/dts/framework/testbed_model/linux_session.py
index 0ab59cef85..5d24030c3d 100644
--- a/dts/framework/testbed_model/linux_session.py
+++ b/dts/framework/testbed_model/linux_session.py
@@ -198,6 +198,14 @@ def configure_port_ip_address(
 verify=True,
 )
 
+def configure_port_mtu(self, mtu: int, port: Port) -> None:
+"""Overrides :meth:`~.os_session.OSSession.configure_port_mtu`."""
+self.send_command(
+f"ip link set dev {port.logical_name} mtu {mtu}",
+privileged=True,
+verify=True,
+)
+
 def configure_ipv4_forwarding(self, enable: bool) -> None:
 """Overrides 
:meth:`~.os_session.OSSession.configure_ipv4_forwarding`."""
 state = 1 if enable else 0
diff --git a/dts/framework/testbed_model/os_session.py 
b/dts/framework/testbed_model/os_session.py
index ac6bb5e112..e42f4d752a 100644
--- a/dts/framework/testbed_model/os_session.py
+++ b/dts/framework/testbed_model/os_session.py
@@ -413,6 +413,15 @@ def configure_port_ip_address(
 delete: If :data:`True`, remove the IP address, otherwise 
configure it.
 """
 
+@abstractmethod
+def configure_port_mtu(self, mtu: int, port: Port) -> None:
+"""Configure `mtu` on `port`.
+
+Args:
+mtu: Desired MTU value.
+port: Port to set `mtu` on.
+"""
+
 @abstractmethod
 def configure_ipv4_forwarding(self, enable: bool) -> None:
 """Enable IPv4 forwarding in the operating system.
-- 
2.43.0

[PATCH v7 7/7] dts: add pmd_buffer_scatter test suite

[jspewock]

From: Jeremy Spewock 

This test suite provides testing of the support of scattered packets by
Poll Mode Drivers using testpmd, verifying the ability to receive and
transmit scattered multi-segment packets made up of multiple
non-contiguous memory buffers. This is tested through 5 different cases
in which the length of the packets sent are less than the mbuf size,
equal to the mbuf size, and 1, 4, and 5 bytes greater than the mbuf size
in order to show both the CRC and the packet data are capable of
existing in the first, second, or both buffers.

Naturally, if the PMD is capable of forwarding scattered packets which
it receives as input, this shows it is capable of both receiving and
transmitting scattered packets.

Signed-off-by: Jeremy Spewock 
---
 dts/tests/TestSuite_pmd_buffer_scatter.py | 132 ++
 1 file changed, 132 insertions(+)
 create mode 100644 dts/tests/TestSuite_pmd_buffer_scatter.py

diff --git a/dts/tests/TestSuite_pmd_buffer_scatter.py 
b/dts/tests/TestSuite_pmd_buffer_scatter.py
new file mode 100644
index 00..9ce1bacabf
--- /dev/null
+++ b/dts/tests/TestSuite_pmd_buffer_scatter.py
@@ -0,0 +1,132 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2023-2024 University of New Hampshire
+
+"""Multi-segment packet scattering testing suite.
+
+This testing suite tests the support of transmitting and receiving scattered 
packets. This is shown
+by the Poll Mode Driver being able to forward scattered multi-segment packets 
composed of multiple
+non-contiguous memory buffers. To ensure the receipt of scattered packets, the 
DMA rings of the
+port's RX queues must be configured with mbuf data buffers whose size is less 
than the maximum
+length.
+
+If it is the case that the Poll Mode Driver can forward scattered packets 
which it receives, then
+this suffices to show the Poll Mode Driver is capable of both receiving and 
transmitting scattered
+packets.
+"""
+
+import struct
+
+from scapy.layers.inet import IP  # type: ignore[import]
+from scapy.layers.l2 import Ether  # type: ignore[import]
+from scapy.packet import Raw  # type: ignore[import]
+from scapy.utils import hexstr  # type: ignore[import]
+
+from framework.remote_session.testpmd_shell import TestPmdForwardingModes, 
TestPmdShell
+from framework.test_suite import TestSuite
+
+
+class PmdBufferScatter(TestSuite):
+"""DPDK PMD packet scattering test suite.
+
+Configure the Rx queues to have mbuf data buffers whose sizes are smaller 
than the maximum
+packet size. Specifically, set mbuf data buffers to have a size of 2048 to 
fit a full 1512-byte
+(CRC included) ethernet frame in a mono-segment packet. The testing of 
scattered packets is
+done by sending a packet whose length is greater than the size of the 
configured size of mbuf
+data buffers. There are a total of 5 packets sent within test cases which 
have lengths less
+than, equal to, and greater than the mbuf size. There are multiple packets 
sent with lengths
+greater than the mbuf size in order to test cases such as:
+
+1. A single byte of the CRC being in a second buffer while the remaining 3 
bytes are stored in
+the first buffer alongside packet data.
+2. The entire CRC being stored in a second buffer while all of the packet 
data is stored in the
+first.
+3. Most of the packet data being stored in the first buffer and a single 
byte of packet data
+stored in a second buffer alongside the CRC.
+"""
+
+def set_up_suite(self) -> None:
+"""Set up the test suite.
+
+Setup:
+Verify that we have at least 2 port links in the current execution 
and increase the MTU
+of both ports on the traffic generator to 9000 to support larger 
packet sizes.
+"""
+self.verify(
+len(self._port_links) > 1,
+"There must be at least two port links to run the scatter test 
suite",
+)
+
+self.tg_node.main_session.configure_port_mtu(9000, 
self._tg_port_egress)
+self.tg_node.main_session.configure_port_mtu(9000, 
self._tg_port_ingress)
+
+def scatter_pktgen_send_packet(self, pktsize: int) -> str:
+"""Generate and send a packet to the SUT then capture what is 
forwarded back.
+
+Generate an IP packet of a specific length and send it to the SUT, 
then capture the
+resulting received packet and extract its payload. The desired length 
of the packet is met
+by packing its payload with the letter "X" in hexadecimal.
+
+Args:
+pktsize: Size of the packet to generate and send.
+
+Returns:
+The payload of the received packet as a string.
+"""
+packet = Ether() / IP() / Raw()
+packet.getlayer(2).load = ""
+payload_len = pktsize - len(packet) - 4
+payload = ["58"] * payload_len
+# pack the payload
+for X_in_hex in payload:
+packet.load += struct.pack("=B", int("

Re: rte_eth_dev_data reorganization needed

[Ferruh Yigit]

On 1/8/2024 5:14 PM, Stephen Hemminger wrote:
> While looking at ethdev (for pdump issues) noticed.
> The rte_eth_dev_data structure layout is unorganized.
> Lots of holes, fields not arranged in logical groups
> and usage could be localized for better cache access.
> 

For reference following is structure layout [1].

Agree that it can benefit from reorganization.

> Obviously, any reorg is ABI break. We should do this for 24.11
>

It may not be an ABI break, since structure moved to ethdev_driver.h
with recent changes, it is not exposed to user directly anymore.


eth_dev_data is device data and it is not directly involved in the
datapath, although I can see 'rx_mbuf_alloc_failed' stats is updated in
datapath, and some drivers access 'dev_private' in datapath, most of
other fields are configuration related.

So, although I expect changes in eth_dev_data doesn't impact the
performance it needs to be tested and verified.

As this release looks mostly quite, perhaps it can be good time to try
this kind of change, what do you think to start reorg in this release?





[1]
struct rte_eth_dev_data {
 char   name[64]; /* 064 */
 /* --- cacheline 1 boundary (64 bytes) --- */
 void * *   rx_queues;/*64 8 */
 void * *   tx_queues;/*72 8 */
 uint16_t   nb_rx_queues; /*80 2 */
 uint16_t   nb_tx_queues; /*82 2 */
 struct rte_eth_dev_sriov   sriov;/*84 6 */

 /* XXX 6 bytes hole, try to pack */

 void * dev_private;  /*96 8 */
 struct rte_eth_linkdev_link __attribute__((__aligned__(8))); /*
  104 8 */

 /* XXX last struct has 2 bytes of padding */

 struct rte_eth_confdev_conf; /*   112  2280 */

 /* XXX last struct has 4 bytes of padding */

 /* --- cacheline 37 boundary (2368 bytes) was 24 bytes ago --- */
 uint16_t   mtu;  /*  2392 2 */

 /* XXX 2 bytes hole, try to pack */

 uint32_t   min_rx_buf_size;  /*  2396 4 */
 uint64_t   rx_mbuf_alloc_failed; /*  2400 8 */
 struct rte_ether_addr *mac_addrs;/*  2408 8 */
 uint64_t   mac_pool_sel[128];/*  2416  1024 */
 /* --- cacheline 53 boundary (3392 bytes) was 48 bytes ago --- */
 struct rte_ether_addr *hash_mac_addrs;   /*  3440 8 */
 uint16_t   port_id;  /*  3448 2 */
 uint8_tpromiscuous:1;/*  3450: 0  1 */
 uint8_tscattered_rx:1;   /*  3450: 1  1 */
 uint8_tall_multicast:1;  /*  3450: 2  1 */
 uint8_tdev_started:1;/*  3450: 3  1 */
 uint8_tlro:1;/*  3450: 4  1 */
 uint8_tdev_configured:1; /*  3450: 5  1 */
 uint8_tflow_configured:1;/*  3450: 6  1 */

 /* XXX 1 bit hole, try to pack */

 uint8_trx_queue_state[1024]; /*  3451  1024 */
 /* --- cacheline 69 boundary (4416 bytes) was 59 bytes ago --- */
 uint8_ttx_queue_state[1024]; /*  4475  1024 */

 /* XXX 1 byte hole, try to pack */

 /* --- cacheline 85 boundary (5440 bytes) was 60 bytes ago --- */
 uint32_t   dev_flags;/*  5500 4 */
 /* --- cacheline 86 boundary (5504 bytes) --- */
 intnuma_node;/*  5504 4 */

 /* XXX 4 bytes hole, try to pack */

 struct rte_vlan_filter_conf vlan_filter_conf;/*  5512   512 */
 /* --- cacheline 94 boundary (6016 bytes) was 8 bytes ago --- */
 struct rte_eth_dev_owner   owner;/*  602472 */
 /* --- cacheline 95 boundary (6080 bytes) was 16 bytes ago --- */
 uint16_t   representor_id;   /*  6096 2 */
 uint16_t   backer_port_id;   /*  6098 2 */

 /* XXX 4 bytes hole, try to pack */

 pthread_mutex_tflow_ops_mutex;   /*  610440 */

 /* size: 6144, cachelines: 96, members: 32 */
 /* sum members: 6126, holes: 5, sum holes: 17 */
 /* sum bitfield members: 7 bits, bit holes: 1, sum bit holes: 1 bits */
 /* paddings: 2, sum paddings: 6 */
 /* forced alignments: 1 */
} __attribute__((__aligned__(64)));

RE: [EXT] [PATCH] cryptodev: remove unused extern

[Akhil Goyal]

> The variable rte_cyptodev_names is unused and misspelled.
> 
> Fixes: c0f87eb5252b ("cryptodev: change burst API to be crypto op oriented")
Cc: sta...@dpdk.org

> Signed-off-by: Stephen Hemminger 
Acked-by: Akhil Goyal 

Applied to dpdk-next-crypto

Thanks for the fix Stephen.

RE: [PATCH 1/2] examples/ipsec-secgw: fix width of variables

[Akhil Goyal]

> 
> > 'rte_eth_rx_burst' returns uint16_t. The same value need to be passed
> > to 'process_packets' functions which performs further processing. Having
> > this function use 'uint8_t' can result in issues when MAX_PKT_BURST is
> > larger.
> >
> > The route functions (route4_pkts & route6_pkts) take uint8_t as the
> > argument. The caller can pass larger values as the field that is passed
> > is of type uint32_t. And the function can work with uint32_t as it loops
> > through the packets and sends it out. Using uint8_t can result in silent
> > packet drops.
> >
> > Fixes: 4fbfa6c7c921 ("examples/ipsec-secgw: update eth header during route
> lookup")

Cc: sta...@dpdk.org

> >
> > Signed-off-by: Anoob Joseph 
Acked-by: Akhil Goyal 

Applied to dpdk-next-crypto
 
> Acked-by: Konstantin Ananyev 
Email Id shows warning in mailmap
Contributor name/email mismatch with .mailmap:
Konstantin Ananyev  is not the primary 
email address

I believe Konstantin is using both email ids, but mailmap is taking the first 
one as primary one.
Konstantin,
Is it possible for you to use a single email ID and update mailmap accordingly?

RE: [PATCH] examples/ipsec-secgw: use bulk free

[Akhil Goyal]

> Subject: [PATCH] examples/ipsec-secgw: use bulk free
> 
> Use rte_pktmbuf_free_bulk() API instead of looping through the packets
> and freeing individually.
> 
> Signed-off-by: Anoob Joseph 
> Suggested-by: Stephen Hemminger 
Acked-by: Akhil Goyal 
Applied to dpdk-next-crypto
Thanks.

Re: [PATCH 2/3] net/nfp: fix free resource problem

[Ferruh Yigit]

On 1/9/2024 7:56 AM, Chaoyong He wrote:
>> On 12/18/2023 1:50 AM, Chaoyong He wrote:
 On 12/14/2023 10:24 AM, Chaoyong He wrote:
> From: Long Wu 
>
> Set the representor array to NULL to avoid that close interface does
> not free some resource.
>
> Fixes: a135bc1644d6 ("net/nfp: fix resource leak for flower
> firmware")
> Cc: chaoyong...@corigine.com
> Cc: sta...@dpdk.org
>
> Signed-off-by: Long Wu 
> Reviewed-by: Chaoyong He 
> Reviewed-by: Peng Zhang 
> ---
>  drivers/net/nfp/flower/nfp_flower_representor.c | 15
> ++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/nfp/flower/nfp_flower_representor.c
> b/drivers/net/nfp/flower/nfp_flower_representor.c
> index 27ea3891bd..5f7c1fa737 100644
> --- a/drivers/net/nfp/flower/nfp_flower_representor.c
> +++ b/drivers/net/nfp/flower/nfp_flower_representor.c
> @@ -294,17 +294,30 @@ nfp_flower_repr_tx_burst(void *tx_queue,
> static int  nfp_flower_repr_uninit(struct rte_eth_dev *eth_dev)  {
> + uint16_t index;
>   struct nfp_flower_representor *repr;
>
>   repr = eth_dev->data->dev_private;
>   rte_ring_free(repr->ring);
>
> + if (repr->repr_type == NFP_REPR_TYPE_PHYS_PORT) {
> + index = NFP_FLOWER_CMSG_PORT_PHYS_PORT_NUM(repr-
> port_id);
> + repr->app_fw_flower->phy_reprs[index] = NULL;
> + } else {
> + index = repr->vf_id;
> + repr->app_fw_flower->vf_reprs[index] = NULL;
> + }
> +
>   return 0;
>  }
>
>  static int
> -nfp_flower_pf_repr_uninit(__rte_unused struct rte_eth_dev *eth_dev)
> +nfp_flower_pf_repr_uninit(struct rte_eth_dev *eth_dev)
>  {
> + struct nfp_flower_representor *repr = eth_dev->data->dev_private;
> +
> + repr->app_fw_flower->pf_repr = NULL;
>

 Here it is assigned to NULL but is it freed? If freed, why not set to
 NULL where it is freed?

 Same for above phy_reprs & vf_reprs.
>>>
>>> The whole invoke view:
>>> rte_eth_dev_close()
>>> --> nfp_flower_repr_dev_close()
>>> --> nfp_flower_repr_free()
>>> --> nfp_flower_pf_repr_uninit()
>>> --> nfp_flower_repr_uninit()
>>>// In these two functions, we just assigned to NULL but not 
>>> freed yet.
>>>// It is still refer by the `eth_dev->data->dev_private`.
>>> --> rte_eth_dev_release_port()
>>> --> rte_free(eth_dev->data->dev_private);
>>> // And here it is really freed (by the rte framework).
>>>
>>
>> 'rte_eth_dev_release_port()' frees the device private data, but not all 
>> pointers,
>> like 'repr->app_fw_flower->pf_repr', in the struct are freed, it is 
>> dev_close() or
>> unint() functions responsibility.
>>
>> Can you please double check if
>> 'eth_dev->data->dev_private->app_fw_flower->pf_repr' freed or not?
> 
> (gdb) b nfp_flower_repr_dev_close
> Breakpoint 1 at 0x7f839a4ad37f: file 
> ../drivers/net/nfp/flower/nfp_flower_representor.c, line 356.
> (gdb) c
> Continuing.
> 
> Thread 1 "dpdk-testpmd" hit Breakpoint 1, nfp_flower_repr_dev_close 
> (dev=0x7f839aed2340 )
> at ../drivers/net/nfp/flower/nfp_flower_representor.c:356
> 356 if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> (gdb) n
> 359 repr = dev->data->dev_private;
> (gdb)
> 360 app_fw_flower = repr->app_fw_flower;
> (gdb)
> 361 hw = app_fw_flower->pf_hw;
> (gdb)
> 362 pf_dev = hw->pf_dev;
> (gdb)
> 368 nfp_net_disable_queues(dev);
> (gdb) p repr
> $1 = (struct nfp_flower_representor *) 0x17c49c800
> (gdb) p dev->data->dev_private
> $2 = (void *) 0x17c49c800
> (gdb) p repr->app_fw_flower->pf_repr
> $3 = (struct nfp_flower_representor *) 0x17c49c800
> 
> As we can see, these three pointers point the same block of memory.
> 

Ahh, I missed that 'repr->app_fw_flower->pf_repr' points to
'dev_private', so your code makes sense.

But if it is 'dev_private', why free it in 'nfp_pf_uninit()' as it will
be freed by 'rte_eth_dev_release_port()'?
Won't removing 'rte_free(pf_dev);' from 'nfp_pf_uninit()' will have the
same effect, instead of setting it NULL in advance?

Re: [PATCH] ethdev: add dump regs for telemetry

[Ferruh Yigit]

On 1/9/2024 2:19 AM, Jie Hai wrote:
> On 2023/12/14 20:49, Ferruh Yigit wrote:
>> On 12/14/2023 1:56 AM, Jie Hai wrote:
>>> The ethdev library now registers a telemetry command for
>>> dump regs.
>>>
>>> An example usage is shown below:
>>> --> /ethdev/regs,test
>>> {
>>>    "/ethdev/regs": {
>>>  "regs_offset": 0,
>>>  "regs_length": 3192,
>>>  "regs_width": 4,
>>>  "device_version": "0x1080f00",
>>>  "regs_file": "port_0_regs_test"
>>>    }
>>> }
> 
>>
>> Above code writes register data to a file.
>>
>> I am not sure about this kind of usage of telemetry command, that it
>> cause data to be written to a file.
>>
>> My understanding is, telemetry usage is based on what telemetry client
>> receives.
>> What do you think just keep the 'reg_info' fields excluding data to the
>> file?
>>
>> .Hi, Ferruh
> 
> I tried to write all register information to telemetry data,
> but gave up because some drivers had too many registers (eg.ixgbe)
> to carry. Therefore, the writing data to file approach is selected.
> 
> When we query a register, the register content is the key.
> The information such as the width and length is only auxiliary
> information. If the register data cannot be obtained, the auxiliary
> information is optional. So I don't think register data should be removed.
> 
> In my opinion, writing a file is a more appropriate way to do it.
> I wonder if there's a better way.
> 
> 

Is there a usecase to get register information from telemetry interface?

Re: [PATCH v1] net/axgbe: read and save the port property register

[Ferruh Yigit]

On 1/9/2024 11:14 AM, Ande, Venkat Kumar wrote:
> [AMD Official Use Only - General]
> 
> Hi Ferruh,
> 
> This change will be refactoring for future code readability of AMD AXGBE 
> driver.
> 

Thanks for clarification, I can update commit log with this info while
merging.


> Regards,
> Venkat
> 
> -Original Message-
> From: Yigit, Ferruh 
> Sent: Tuesday, January 9, 2024 4:05 PM
> To: Ande, Venkat Kumar ; dev@dpdk.org
> Cc: Sebastian, Selwin 
> Subject: Re: [PATCH v1] net/axgbe: read and save the port property register
> 
> On 1/5/2024 11:32 AM, Venkat Kumar Ande wrote:
>> From: Venkat Kumar Ande 
>>
>> Read and save the port property registers once during the device probe
>> and then use the saved values as they are needed.
>>
> 
> Hi Venkat,
> 
> Can you please describe what is the motivation/reason for the change?
> 
> Is it addressing a functional problem or refactoring for coming feature 
> etc...?
> 
>> Signed-off-by: Venkat Kumar Ande 
>> ---
>>  drivers/net/axgbe/axgbe_ethdev.c   | 21 +
>>  drivers/net/axgbe/axgbe_ethdev.h   |  7 +++
>>  drivers/net/axgbe/axgbe_phy_impl.c | 68
>> --
>>  3 files changed, 48 insertions(+), 48 deletions(-)
>>
>> diff --git a/drivers/net/axgbe/axgbe_ethdev.c
>> b/drivers/net/axgbe/axgbe_ethdev.c
>> index f174d46143..3450374535 100644
>> --- a/drivers/net/axgbe/axgbe_ethdev.c
>> +++ b/drivers/net/axgbe/axgbe_ethdev.c
>> @@ -2342,23 +2342,28 @@ eth_axgbe_dev_init(struct rte_eth_dev *eth_dev)
>>   pdata->arcache = AXGBE_DMA_OS_ARCACHE;
>>   pdata->awcache = AXGBE_DMA_OS_AWCACHE;
>>
>> + /* Read the port property registers */
>> + pdata->pp0 = XP_IOREAD(pdata, XP_PROP_0);
>> + pdata->pp1 = XP_IOREAD(pdata, XP_PROP_1);
>> + pdata->pp2 = XP_IOREAD(pdata, XP_PROP_2);
>> + pdata->pp3 = XP_IOREAD(pdata, XP_PROP_3);
>> + pdata->pp4 = XP_IOREAD(pdata, XP_PROP_4);
>> +
> 
> <...>
> 

[Bug 1345] net/i40e packet loss after Rx interrupt disable

[bugzilla]

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

Bug ID: 1345
   Summary: net/i40e packet loss after Rx interrupt disable
   Product: DPDK
   Version: 23.11
  Hardware: All
OS: All
Status: UNCONFIRMED
  Severity: normal
  Priority: Normal
 Component: ethdev
  Assignee: dev@dpdk.org
  Reporter: andrew.rybche...@oktetlabs.ru
  Target Milestone: ---

net/i40e packet loss after Rx interrupt disable

Open-source test suite which has been run at UNH IOL has a number of test
scenarios which check Rx interrupts support.
There is a common pattern of failures in 4 cases. If Rx interrupt is disabled,
the next packet to be received is lost [1], [2], [3] and [4].

[1]
https://ts-factory.io/bublik/v2/log/362398?mode=treeAndlog&focusId=363531&experimental=true&lineNumber=1_75
[2]
https://ts-factory.io/bublik/v2/log/362398?mode=treeAndlog&focusId=363534&experimental=true&lineNumber=1_75
[3]
https://ts-factory.io/bublik/v2/log/362398?mode=treeAndlog&focusId=363535&experimental=true&lineNumber=1_58
[4]
https://ts-factory.io/bublik/v2/log/362398?mode=treeAndlog&focusId=363536&experimental=true&lineNumber=1_79

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

[Bug 1346] net/i40e does not report interrupt for the second packet

[bugzilla]

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

Bug ID: 1346
   Summary: net/i40e does not report interrupt for the second
packet
   Product: DPDK
   Version: 23.11
  Hardware: All
OS: All
Status: UNCONFIRMED
  Severity: normal
  Priority: Normal
 Component: ethdev
  Assignee: dev@dpdk.org
  Reporter: andrew.rybche...@oktetlabs.ru
  Target Milestone: ---

net/i40e does not report interrupt for the second packet

If I understand Rx interrupts functionality correctly, Rx interrupts should be
generated until disabled. However, it does not happen in the case of i40e with
X710 NIC.

The problem is found by open-source test suite which has been run at UNH IOL
[1].

Scenario is simple:
 - enable Rx interrupts
 - send packet from peer
 - check interrupt, receive packet
 - send one more packet
 - check interrupt - failure, no interupt

[1]
https://ts-factory.io/bublik/v2/log/362398?mode=treeAndlog&focusId=363533&experimental=true&lineNumber=1_69

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

[Bug 1347] net/i40e reports unexpected interrupt just after enable

[bugzilla]

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

Bug ID: 1347
   Summary: net/i40e reports unexpected interrupt just after
enable
   Product: DPDK
   Version: 23.11
  Hardware: All
OS: All
Status: UNCONFIRMED
  Severity: normal
  Priority: Normal
 Component: ethdev
  Assignee: dev@dpdk.org
  Reporter: andrew.rybche...@oktetlabs.ru
  Target Milestone: ---

net/i40e reports unexpected interrupt just after enable

Scenario is the following:
 - send packet from peer
 - receive it on DUT
 - enable Rx interrupt
 - check that no Rx interrupt report - failure [1]

[1]
https://ts-factory.io/bublik/v2/log/362398?mode=treeAndlog&focusId=363540&experimental=true&lineNumber=1_59

Above logs are generated by open-source test suite run at UNH IOL.

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

Re: Depends-on patchseries support via git-pw or patchwork

[Adam Hassick]

 I'm not sure yet. I've poked the issue thread about whether they need our
help with anything and what the next steps are.

On Mon, Jan 8, 2024 at 11:18 AM Ferruh Yigit  wrote:

> On 12/22/2023 5:26 PM, Patrick Robb wrote:
> > Hi all,
> >
> > As some of you know from discussions at DPDK CI meetings, Adam from UNH
> > is writing a script which leverages git-pw, and takes as arguments a
> > patch series patchwork id, patchwork project, and pw token, and produces
> > a project artifact for CI testing purposes. Starting in January we will
> > use it for applying patches to DPDK and creating our dpdk.tar.gz
> > artifacts for testing. And, we will submit it to the dpdk-ci repo.
> >
> > Anyways, when we originally discussed the idea, Thomas suggested that we
> > implement the depends-on functionality by contributing to the git-pw
> > project, as opposed to implementing the depend-on support in the create
> > artifact script itself. Adam did create a github issue on the git-pw
> > project in order to poll the community for interest in this feature, and
> > one of the patchwork maintainers chimed in to suggest that rather than
> > implementing the feature on the client side via git-pw, it should simply
> > be implemented for patchwork itself. That way if it's patchwork server
> > side and exposed via the api, other client side tools like pwclient can
> > also receive the benefits.
> >
> > I just wanted to flag this on the ci mailing list so that anyone with
> > thoughts could submit them on the Github issue, which you can find
> > here: https://github.com/getpatchwork/git-pw/issues/71
> > 
> >
> > Thanks Adam for pushing this effort forward.
> >
>
> Thanks Patrick for the update and thanks Adam for driving this.
>
> Implementing support to patchwork sounds good to me, is anything
> expected from our end for this?
>
>

Re: Issues around packet capture when secondary process is doing rx/tx

[Stephen Hemminger]

On Tue, 9 Jan 2024 15:06:47 -0800
Stephen Hemminger  wrote:

> On Mon, 8 Jan 2024 15:13:25 +
> Konstantin Ananyev  wrote:
> 
> > > I have been looking at a problem reported by Sandesh
> > > where packet capture does not work if rx/tx burst is done in secondary 
> > > process.
> > > 
> > > The root cause is that existing rx/tx callback model just doesn't work
> > > unless the process doing the rx/tx burst calls is the same one that
> > > registered the callbacks.
> > > 
> > > An example sequence would be:
> > >   1. dumpcap (or pdump) as secondary tells pdump in primary to register 
> > > callback
> > >   2. secondary process calls rx_burst.
> > >   3. rx_burst sees the callback but it has pointer pdump_rx which is not 
> > > necessarily
> > >  at same location in primary and secondary process.
> > >   4. indirect function call in secondary to bad location likely causes 
> > > crash.
> > 
> > As I remember, RX/TX callbacks were never intended to work over multiple 
> > processes.
> > Right now RX/TX callbacks are private for the process, different process 
> > simply should not
> > see/execute them.
> > I.E. it callbacks list is part of 'struct rte_eth_dev' itself, not the 
> > rte_eth_dev.data that is shared
> > between processes.
> > It should be normal, wehn for the same port/queue you will end-up with 
> > different list of callbacks
> > for different processes.  
> > So, unless I am missing something, I don't see how we can end-up with 3) 
> > and 4) from above:
> > From my understanding secondary process will never see/call primary's 
> > callbacks.
> > 
> > About pdump itself, it was a while when I looked at it last time, but as I 
> > remember to start it to work,
> > server process has to call rte_pdump_init() which in terns register 
> > PDUMP_MP handler.
> > I suppose for the secondary process to act as a 'pdump server' it needs to 
> > call rte_pdump_init() itself,
> > though I am not sure such option is supported right now. 
> >
> 
> Did some more tests with modified testpmd, and reached some conclusions:
> 
> The logical interface would be to allow rte_pdump_init() to be called by
>the process that would be using rx/tx burst API's.
> 
>   This doesn't work as it should because the multi-process socket API
>   assumes that the it only runs the server in primary.  The secondary
>   can start its own MP thread, but it won't work:
> 
>   Primary EAL: Multi-process socket /var/run/dpdk/rte/mp_socket
>   Secondary: EAL: Multi-process socket 
> /var/run/dpdk/rte/mp_socket_6057_1ccd4157fd5
> 
>   The problem is when client (pdump or dumpcap) tries to run, it uses the 
> mp_socket
>   in the primary which causes: EAL: Cannot find action: mp_pdump
> 
>   Looks like the whole MP socket mechanism is just not up to this.
> 
> Maybe pdump needs to have its own socket and control thread?
> Or MP socket needs to have some multicast fanout to all secondaries?
> 
> 
> 
> 
> 
> 
> 
> 2. Fut

[PATCH] doc: add testpmd latencystats option

[Stephen Hemminger]

Option was added but never added to documentation.

Fixes: 62d3216d6194 ("app/testpmd: add latency statistics calculation")
Signed-off-by: Stephen Hemminger 
---
 doc/guides/testpmd_app_ug/run_app.rst | 4 
 1 file changed, 4 insertions(+)

diff --git a/doc/guides/testpmd_app_ug/run_app.rst 
b/doc/guides/testpmd_app_ug/run_app.rst
index 24a086401ee7..e149373cd235 100644
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -422,6 +422,10 @@ The command line options are:
 
 Set the logical core N to perform bitrate calculation.
 
+*   ``--latencystats=N``
+
+Set the logical core N to perform latency calculation.
+
 *   ``--print-event 
``
 
 Enable printing the occurrence of the designated event. Using all will
-- 
2.43.0

Re: Issues around packet capture when secondary process is doing rx/tx

[Stephen Hemminger]

On Mon, 8 Jan 2024 15:13:25 +
Konstantin Ananyev  wrote:

> > I have been looking at a problem reported by Sandesh
> > where packet capture does not work if rx/tx burst is done in secondary 
> > process.
> > 
> > The root cause is that existing rx/tx callback model just doesn't work
> > unless the process doing the rx/tx burst calls is the same one that
> > registered the callbacks.
> > 
> > An example sequence would be:
> > 1. dumpcap (or pdump) as secondary tells pdump in primary to register 
> > callback
> > 2. secondary process calls rx_burst.
> > 3. rx_burst sees the callback but it has pointer pdump_rx which is not 
> > necessarily
> >at same location in primary and secondary process.
> > 4. indirect function call in secondary to bad location likely causes 
> > crash.  
> 
> As I remember, RX/TX callbacks were never intended to work over multiple 
> processes.
> Right now RX/TX callbacks are private for the process, different process 
> simply should not
> see/execute them.
> I.E. it callbacks list is part of 'struct rte_eth_dev' itself, not the 
> rte_eth_dev.data that is shared
> between processes.
> It should be normal, wehn for the same port/queue you will end-up with 
> different list of callbacks
> for different processes.  
> So, unless I am missing something, I don't see how we can end-up with 3) and 
> 4) from above:
> From my understanding secondary process will never see/call primary's 
> callbacks.
> 
> About pdump itself, it was a while when I looked at it last time, but as I 
> remember to start it to work,
> server process has to call rte_pdump_init() which in terns register PDUMP_MP 
> handler.
> I suppose for the secondary process to act as a 'pdump server' it needs to 
> call rte_pdump_init() itself,
> though I am not sure such option is supported right now. 
>  

Did some more tests with modified testpmd, and reached some conclusions:

The logical interface would be to allow rte_pdump_init() to be called by
   the process that would be using rx/tx burst API's.

  This doesn't work as it should because the multi-process socket API
  assumes that the it only runs the server in primary.  The secondary
  can start its own MP thread, but it won't work:

  Primary EAL: Multi-process socket /var/run/dpdk/rte/mp_socket
  Secondary: EAL: Multi-process socket 
/var/run/dpdk/rte/mp_socket_6057_1ccd4157fd5

  The problem is when client (pdump or dumpcap) tries to run, it uses the 
mp_socket
  in the primary which causes: EAL: Cannot find action: mp_pdump

  Looks like the whole MP socket mechanism is just not up to this.

Maybe pdump needs to have its own socket and control thread?
Or MP socket needs to have some multicast fanout to all secondaries?







2. Fut

[PATCH] testpmd: do not print bitrate-stats in help if not configured

[Stephen Hemminger]

Like other #ifdef options, bitrate-stats should not be printed
in help if not configured.

Fixes: e25e6c70fb56 ("app/testpmd: add --bitrate-stats option")
Signed-off-by: Stephen Hemminger 
---
 app/test-pmd/parameters.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index a9ca58339dd7..f7df7d31295f 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -167,8 +167,10 @@ usage(char* progname)
printf("  --disable-device-start: do not automatically start port\n");
printf("  --no-lsc-interrupt: disable link status change interrupt.\n");
printf("  --no-rmv-interrupt: disable device removal interrupt.\n");
+#ifdef RTE_LIB_BITRATESTATS
printf("  --bitrate-stats=N: set the logical core N to perform "
"bit-rate calculation.\n");
+#endif
printf("  --print-event 
:
 "
   "enable print of designated event or all of them.\n");
printf("  --mask-event 
:
 "
-- 
2.43.0

RE: 21.11.6 patches review and test

[Xu, HailinX]

Hi Kevin,

Intel have started testing, I'm sorry for not sent test results. Because some 
personnel were on vacation before and should be able to produce results this 
week.
I will send the test report as soon as possible, I am deeply sorry for any 
inconvenience caused to you.


Regards,
Xu, Hailin

> -Original Message-
> From: Kevin Traynor 
> Sent: Tuesday, January 9, 2024 5:49 PM
> To: sta...@dpdk.org; Mcnamara, John 
> Cc: dev@dpdk.org; Abhishek Marathe ;
> Ali Alnubani ; benjamin.wal...@intel.com; David
> Christensen ; Hemant Agrawal
> ; Stokes, Ian ; Jerin Jacob
> ; Ju-Hyoung Lee ; Luca
> Boccassi ; Pei Zhang ;
> qian.q...@intel.com; Raslan Darawsheh ; Thomas
> Monjalon ; yangh...@redhat.com;
> yuan.p...@intel.com; zhaoyan.c...@intel.com
> Subject: Re: 21.11.6 patches review and test
> 
> On 20/12/2023 13:22, Kevin Traynor wrote:
> > Hi all,
> >
> > Here is a list of patches targeted for stable release 21.11.6.
> >
> > The planned date for the final release is 12 January.
> >
> 
> I have validation reports for Red Hat and Nvidia. Thank you.
> 
> Is there any validation results from Intel, or an ETA ?
> 
> > Please help with testing and validation of your use cases and report
> > any issues/results with reply-all to this mail. For the final release
> > the fixes and reported validations will be added to the release notes.
> >
> > A release candidate tarball can be found at:
> >
> > https://dpdk.org/browse/dpdk-stable/tag/?id=v21.11.6-rc1
> >
> > These patches are located at branch 21.11 of dpdk-stable repo:
> > https://dpdk.org/browse/dpdk-stable/
> >
> > Thanks.
> >
> > Kevin
> >
> 

Re: [PATCH] ethdev: add dump regs for telemetry

[fengchengwen]

Hi Ferruh,

On 2024/1/10 2:06, Ferruh Yigit wrote:
> On 1/9/2024 2:19 AM, Jie Hai wrote:
>> On 2023/12/14 20:49, Ferruh Yigit wrote:
>>> On 12/14/2023 1:56 AM, Jie Hai wrote:
 The ethdev library now registers a telemetry command for
 dump regs.

 An example usage is shown below:
 --> /ethdev/regs,test
 {
    "/ethdev/regs": {
  "regs_offset": 0,
  "regs_length": 3192,
  "regs_width": 4,
  "device_version": "0x1080f00",
  "regs_file": "port_0_regs_test"
    }
 }
>>
>>>
>>> Above code writes register data to a file.
>>>
>>> I am not sure about this kind of usage of telemetry command, that it
>>> cause data to be written to a file.
>>>
>>> My understanding is, telemetry usage is based on what telemetry client
>>> receives.
>>> What do you think just keep the 'reg_info' fields excluding data to the
>>> file?
>>>
>>> .Hi, Ferruh
>>
>> I tried to write all register information to telemetry data,
>> but gave up because some drivers had too many registers (eg.ixgbe)
>> to carry. Therefore, the writing data to file approach is selected.
>>
>> When we query a register, the register content is the key.
>> The information such as the width and length is only auxiliary
>> information. If the register data cannot be obtained, the auxiliary
>> information is optional. So I don't think register data should be removed.
>>
>> In my opinion, writing a file is a more appropriate way to do it.
>> I wonder if there's a better way.
>>
>>
> 
> Is there a usecase to get register information from telemetry interface?

Among the available tools:
1, ethtool/proc-info: should use multi-process mechanism to connect to the main 
process
2, telemetry: easier, lighter load, and it don't need re-probe the ethdev in 
the secondary process,
  and also cost more resource, like hugepage, cores.

>From our users, they prefer use the second 'telemetry', so I think we should 
>move
more status-query-points to telemetry.

As for this question, I think it's okay to get register info from telemetry.



Another question, we have some internal registers, which:
1. Is not suitable expose by xstats, because they may includes configuration
2. Is not suitable expose by dumps, because this dumps is hard to understand 
(because it only has value).

So we plan to add some telemetry points in the driver itself, so we could 
display them like xstats:
"" : 0x1234
"" : 0x100

Will the community accept this kind of telemetry points which limit one driver ?

Thanks.

> 
> .
> 

[PATCH v11] net/iavf: add diagnostic support in TX path

[Mingjin Ye]

Implemented a Tx wrapper to perform a thorough check on mbufs,
categorizing and counting invalid cases by types for diagnostic
purposes. The count of invalid cases is accessible through xstats_get.

Also, the devarg option "mbuf_check" was introduced to configure the
diagnostic parameters to enable the appropriate diagnostic features.

supported cases: mbuf, size, segment, offload.
 1. mbuf: check for corrupted mbuf.
 2. size: check min/max packet length according to hw spec.
 3. segment: check number of mbuf segments not exceed hw limitation.
 4. offload: check any unsupported offload flag.

parameter format: "mbuf_check=" or "mbuf_check=[,]"
eg: dpdk-testpmd -a :81:01.0,mbuf_check=[mbuf,size] -- -i

Signed-off-by: Mingjin Ye 
---
v2: Remove call chain.
---
v3: Optimisation implementation.
---
v4: Fix Windows os compilation error.
---
v5: Split Patch.
---
v6: remove strict.
---
v9: Modify the description document.
---
v10: Modify vf rst document.
---
v11: modify comment log.
---
 doc/guides/nics/intel_vf.rst   | 11 
 drivers/net/iavf/iavf.h| 11 
 drivers/net/iavf/iavf_ethdev.c | 72 +
 drivers/net/iavf/iavf_rxtx.c   | 98 ++
 drivers/net/iavf/iavf_rxtx.h   |  2 +
 5 files changed, 194 insertions(+)

diff --git a/doc/guides/nics/intel_vf.rst b/doc/guides/nics/intel_vf.rst
index ce96c2e1f8..f62bb4233c 100644
--- a/doc/guides/nics/intel_vf.rst
+++ b/doc/guides/nics/intel_vf.rst
@@ -111,6 +111,17 @@ For more detail on SR-IOV, please refer to the following 
documents:
 by setting the ``devargs`` parameter like ``-a 
18:01.0,no-poll-on-link-down=1``
 when IAVF is backed by an Intel\ |reg| E810 device or an Intel\ |reg| 700 
Series Ethernet device.
 
+When IAVF is backed by an Intel\ |reg| E810 device or an Intel\ |reg| 700 
series Ethernet devices.
+Set the ``devargs`` parameter ``mbuf_check`` to enable TX diagnostics. For 
example,
+``-a 18:01.0,mbuf_check=`` or ``-a 
18:01.0,mbuf_check=[,...]``. Also,
+``xstats_get`` can be used to get the error counts, which are collected in 
``tx_mbuf_error_packets``
+xstats. For example, ``testpmd> show port xstats all``. Supported cases:
+
+*   mbuf: Check for corrupted mbuf.
+*   size: Check min/max packet length according to hw spec.
+*   segment: Check number of mbuf segments not exceed hw limitation.
+*   offload: Check any unsupported offload flag.
+
 The PCIE host-interface of Intel Ethernet Switch FM1 Series VF 
infrastructure
 
^
 
diff --git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h
index ab24cb02c3..824ae4aa02 100644
--- a/drivers/net/iavf/iavf.h
+++ b/drivers/net/iavf/iavf.h
@@ -114,9 +114,14 @@ struct iavf_ipsec_crypto_stats {
} ierrors;
 };
 
+struct iavf_mbuf_stats {
+   uint64_t tx_pkt_errors;
+};
+
 struct iavf_eth_xstats {
struct virtchnl_eth_stats eth_stats;
struct iavf_ipsec_crypto_stats ips_stats;
+   struct iavf_mbuf_stats mbuf_stats;
 };
 
 /* Structure that defines a VSI, associated with a adapter. */
@@ -310,6 +315,7 @@ struct iavf_devargs {
uint32_t watchdog_period;
int auto_reset;
int no_poll_on_link_down;
+   uint64_t mbuf_check;
 };
 
 struct iavf_security_ctx;
@@ -353,6 +359,11 @@ enum iavf_tx_burst_type {
IAVF_TX_AVX512_CTX_OFFLOAD,
 };
 
+#define IAVF_MBUF_CHECK_F_TX_MBUF(1ULL << 0)
+#define IAVF_MBUF_CHECK_F_TX_SIZE(1ULL << 1)
+#define IAVF_MBUF_CHECK_F_TX_SEGMENT (1ULL << 2)
+#define IAVF_MBUF_CHECK_F_TX_OFFLOAD (1ULL << 3)
+
 /* Structure to store private data for each VF instance. */
 struct iavf_adapter {
struct iavf_hw hw;
diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index 1fb876e827..fca57b50b3 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -39,6 +40,7 @@
 #define IAVF_RESET_WATCHDOG_ARG"watchdog_period"
 #define IAVF_ENABLE_AUTO_RESET_ARG "auto_reset"
 #define IAVF_NO_POLL_ON_LINK_DOWN_ARG "no-poll-on-link-down"
+#define IAVF_MBUF_CHECK_ARG   "mbuf_check"
 uint64_t iavf_timestamp_dynflag;
 int iavf_timestamp_dynfield_offset = -1;
 int rte_pmd_iavf_tx_lldp_dynfield_offset = -1;
@@ -49,6 +51,7 @@ static const char * const iavf_valid_args[] = {
IAVF_RESET_WATCHDOG_ARG,
IAVF_ENABLE_AUTO_RESET_ARG,
IAVF_NO_POLL_ON_LINK_DOWN_ARG,
+   IAVF_MBUF_CHECK_ARG,
NULL
 };
 
@@ -175,6 +178,7 @@ static const struct rte_iavf_xstats_name_off 
rte_iavf_stats_strings[] = {
{"tx_broadcast_packets", _OFF_OF(eth_stats.tx_broadcast)},
{"tx_dropped_packets", _OFF_OF(eth_stats.tx_discards)},
{"tx_error_packets", _OFF_OF(eth_stats.tx_errors)},
+   {"tx_mbuf_error_packets", _OFF_OF(mbuf_stats.tx_pkt_errors)},
 
{"inline_ipsec

Reminder - DPDK Techboard Meeting Tomorrow, Wed. Jan. 10th, 7am Pacific/10am Eastern/ 1500h UTC

[Nathan Southern]

Good evening DPDK Community,

This is a reminder that the tech board meets tomorrow morning at 7am
Pacific/10am Eastern/1500h UTC.

As always, a read-only copy of the agenda can be found here:

https://annuel.framapad.org/p/r.0c3cc4d1e011214183872a98f6b5c7db



Minutes can be found here:

http://core.dpdk.org/techboard/minutes


And zoom information to follow. I will be absent because I have a
conflicting meeting but will look forward to reading the minutes.

Ways to join meeting:

1. Join from PC, Mac, iPad, or Android

https://zoom-lfx.platform.linuxfoundation.org/meeting/96459488340?password=d808f1f6-0a28-4165-929e-5a5bcae7efeb


2. Join via audio

One tap mobile:
US: +12532158782,,96459488340# or +13462487799,,96459488340

Or dial:
US: +1 253 215 8782 or +1 346 248 7799 or +1 669 900 6833 or +1 301 715
8592 or +1 312 626 6799 or +1 646 374 8656 or 877 369 0926 (Toll Free) or
855 880 1246 (Toll Free)
Canada: +1 647 374 4685 or +1 647 558 0588 or +1 778 907 2071 or +1 204 272
7920 or +1 438 809 7799 or +1 587 328 1099 or 855 703 8985 (Toll Free)

Meeting ID: 96459488340

Meeting Passcode: 699526


International numbers: https://zoom.us/u/alwnPIaVT


Many thanks,

Nathan

Nathan C. Southern, Senior Project Coordinator

Data Plane Development Kit

The Linux Foundation

248.835.4812 (mobile)

nsouth...@linuxfoundation.org

[RFC 0/2] use traffic class PRM field for IPv6 modification

[Gavin Li]

New PRM defined new field OUT_IPV6_TRAFFIC_CLASS for IPv6 which will be
used by both IPv6 ECN and DSCP. To apply the new ID and keep backward
compatibility with different RDMA core and FW releases, 1) detect the
support of the new ID in RDMA core and FW. 2) apply the new ID if possible
otherwise, keep using the old ID.

Gavin Li (2):
  net/mlx5: discover IPv6 traffic class support in RDMA core
  net/mlx5: use traffic class PRM field for IPv6 modification

 drivers/common/mlx5/mlx5_devx_cmds.c |  3 ++
 drivers/common/mlx5/mlx5_devx_cmds.h |  1 +
 drivers/common/mlx5/mlx5_prm.h   |  8 ++-
 drivers/net/mlx5/linux/mlx5_os.c |  5 ++
 drivers/net/mlx5/mlx5.h  |  1 +
 drivers/net/mlx5/mlx5_flow.c | 42 +++
 drivers/net/mlx5/mlx5_flow.h |  4 ++
 drivers/net/mlx5/mlx5_flow_dv.c  | 78 
 drivers/net/mlx5/mlx5_flow_hw.c  |  7 +++
 9 files changed, 138 insertions(+), 11 deletions(-)

-- 
2.39.1

[RFC 1/2] net/mlx5: discover IPv6 traffic class support in RDMA core

[Gavin Li]

Previously, IPv6 traffic class used the same ids of IPv4 DSCP and ECN by
rdam core and firmware. New FW support new IPv6 traffic class id which is
recommended to be used though the old way is still working.

FW exposed a new cap bit to indicate the supporting of the new id while
RDMA core does not have such mechanism.

To fix the backward compatibility issue of combination of RDMA core and FW
of different versions, a new function and a new flag were introduced to
check if the new IPv6 traffic class id is supported by RDMA core.

Signed-off-by: Gavin Li 
---
 drivers/net/mlx5/linux/mlx5_os.c |  4 +++
 drivers/net/mlx5/mlx5.h  |  1 +
 drivers/net/mlx5/mlx5_flow.c | 42 
 drivers/net/mlx5/mlx5_flow.h |  1 +
 4 files changed, 48 insertions(+)

diff --git a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c
index ae82e1e5d8..5ae31c88f4 100644
--- a/drivers/net/mlx5/linux/mlx5_os.c
+++ b/drivers/net/mlx5/linux/mlx5_os.c
@@ -1602,6 +1602,10 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
goto error;
}
rte_rwlock_init(&priv->ind_tbls_lock);
+   if (sh->config.dv_flow_en == 1 &&
+   !priv->sh->ipv6_tc_fallback &&
+   mlx5_flow_discover_ipv6_tc_support(eth_dev))
+   priv->sh->ipv6_tc_fallback = 1;
if (priv->sh->config.dv_flow_en == 2) {
 #ifdef HAVE_MLX5_HWS_SUPPORT
if (priv->sh->config.dv_esw_en) {
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 263ebead7f..779805bcd8 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -1444,6 +1444,7 @@ struct mlx5_dev_ctx_shared {
uint32_t lag_rx_port_affinity_en:1;
/* lag_rx_port_affinity is supported. */
uint32_t hws_max_log_bulk_sz:5;
+   uint32_t ipv6_tc_fallback:1;
/* Log of minimal HWS counters created hard coded. */
uint32_t hws_max_nb_counters; /* Maximal number for HWS counters. */
uint32_t max_port; /* Maximal IB device port index. */
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 85e8c77c81..90b72b7b0a 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -12476,3 +12476,45 @@ mlx5_flow_pick_transfer_proxy(struct rte_eth_dev *dev,
  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
  NULL, "unable to find a proxy port");
 }
+
+/**
+ * Discover ipv6 traffic class id support in rdma core and firmware.
+ *
+ * @param dev
+ *   Ethernet device.
+ *
+ * @return
+ *   0, rdma core is good to work with firmware.
+ *   -EOPNOTSUPP, rdma core could not work with new ipv6 tc id.
+ */
+int
+mlx5_flow_discover_ipv6_tc_support(struct rte_eth_dev *dev)
+{
+   struct rte_flow_action_set_dscp set_dscp;
+   struct rte_flow_attr attr;
+   struct rte_flow_action actions[2];
+   struct rte_flow_item items[3];
+   struct rte_flow_error error;
+   uint32_t flow_idx;
+
+   memset(&attr, 0, sizeof(attr));
+   memset(actions, 0, sizeof(actions));
+   memset(items, 0, sizeof(items));
+   attr.group = 1;
+   attr.egress = 1;
+   items[0].type = RTE_FLOW_ITEM_TYPE_ETH;
+   items[1].type = RTE_FLOW_ITEM_TYPE_IPV6;
+   items[2].type = RTE_FLOW_ITEM_TYPE_END;
+   /* Random value */
+   set_dscp.dscp = 9;
+   actions[0].type = RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP;
+   actions[0].conf = &set_dscp;
+   actions[1].type = RTE_FLOW_ACTION_TYPE_END;
+
+   flow_idx = flow_list_create(dev, MLX5_FLOW_TYPE_GEN, &attr, items, 
actions, true, &error);
+   if (!flow_idx)
+   return -EOPNOTSUPP;
+
+   flow_list_destroy(dev, MLX5_FLOW_TYPE_GEN, flow_idx);
+   return 0;
+}
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 120609c595..33d4a28077 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -2638,6 +2638,7 @@ void mlx5_flow_destroy_sub_policy_with_rxq(struct 
rte_eth_dev *dev,
struct mlx5_flow_meter_policy *mtr_policy);
 int mlx5_flow_dv_discover_counter_offset_support(struct rte_eth_dev *dev);
 int mlx5_flow_discover_dr_action_support(struct rte_eth_dev *dev);
+int mlx5_flow_discover_ipv6_tc_support(struct rte_eth_dev *dev);
 int mlx5_action_handle_attach(struct rte_eth_dev *dev);
 int mlx5_action_handle_detach(struct rte_eth_dev *dev);
 int mlx5_action_handle_flush(struct rte_eth_dev *dev);
-- 
2.39.1

[RFC 2/2] net/mlx5: use traffic class PRM field for IPv6 modification

[Gavin Li]

New PRM defined new field OUT_IPV6_TRAFFIC_CLASS for IPv6 which will be
used by both IPv6 ECN and DSCP. A new cap bit
modify_out_ipv6_traffic_class is added. It can be used to check if the
new field is supported by FW.

However, IPv6 ECN and DSCP starts from different offset in the same byte.
Update SWS and HWS to used the new filed and introduce extra offset for
IPv6 DSCP data and mask to solve the issue.

Signed-off-by: Gavin Li 
---
 drivers/common/mlx5/mlx5_devx_cmds.c |  3 ++
 drivers/common/mlx5/mlx5_devx_cmds.h |  1 +
 drivers/common/mlx5/mlx5_prm.h   |  8 ++-
 drivers/net/mlx5/linux/mlx5_os.c |  5 +-
 drivers/net/mlx5/mlx5_flow.h |  3 ++
 drivers/net/mlx5/mlx5_flow_dv.c  | 78 
 drivers/net/mlx5/mlx5_flow_hw.c  |  7 +++
 7 files changed, 92 insertions(+), 13 deletions(-)

diff --git a/drivers/common/mlx5/mlx5_devx_cmds.c 
b/drivers/common/mlx5/mlx5_devx_cmds.c
index 4d8818924a..3a894f894a 100644
--- a/drivers/common/mlx5/mlx5_devx_cmds.c
+++ b/drivers/common/mlx5/mlx5_devx_cmds.c
@@ -1229,6 +1229,9 @@ mlx5_devx_cmd_query_hca_attr(void *ctx,
attr->modify_outer_ip_ecn = MLX5_GET
(flow_table_nic_cap, hcattr,
 ft_header_modify_nic_receive.outer_ip_ecn);
+   attr->modify_outer_ipv6_traffic_class = MLX5_GET
+   (flow_table_nic_cap, hcattr,
+ft_header_modify_nic_receive.outer_ipv6_traffic_class);
attr->set_reg_c = 0x;
if (attr->nic_flow_table) {
 #define GET_RX_REG_X_BITS \
diff --git a/drivers/common/mlx5/mlx5_devx_cmds.h 
b/drivers/common/mlx5/mlx5_devx_cmds.h
index 7f23e925a5..4a6008dc1a 100644
--- a/drivers/common/mlx5/mlx5_devx_cmds.h
+++ b/drivers/common/mlx5/mlx5_devx_cmds.h
@@ -304,6 +304,7 @@ struct mlx5_hca_attr {
uint32_t set_reg_c:16;
uint32_t nic_flow_table:1;
uint32_t modify_outer_ip_ecn:1;
+   uint32_t modify_outer_ipv6_traffic_class:1;
union {
uint32_t max_flow_counter;
struct {
diff --git a/drivers/common/mlx5/mlx5_prm.h b/drivers/common/mlx5/mlx5_prm.h
index 0d46ba9c40..69404b5ed8 100644
--- a/drivers/common/mlx5/mlx5_prm.h
+++ b/drivers/common/mlx5/mlx5_prm.h
@@ -848,6 +848,7 @@ enum mlx5_modification_field {
MLX5_MODI_META_REG_C_13 = 0x94,
MLX5_MODI_META_REG_C_14 = 0x95,
MLX5_MODI_META_REG_C_15 = 0x96,
+   MLX5_MODI_OUT_IPV6_TRAFFIC_CLASS = 0x11C,
MLX5_MODI_OUT_IPV4_TOTAL_LEN = 0x11D,
MLX5_MODI_OUT_IPV6_PAYLOAD_LEN = 0x11E,
MLX5_MODI_OUT_IPV4_IHL = 0x11F,
@@ -2202,7 +2203,9 @@ struct mlx5_ifc_ft_fields_support_bits {
u8 metadata_reg_c_x[0x8];
}; /* end of DW3 */
/* set_action_field_support_2 */
-   u8 reserved_at_80[0x80];
+   u8 reserved_at_80[0x37];
+   u8 outer_ipv6_traffic_class[0x1];
+   u8 reserved_at_B8[0x48];
/* add_action_field_support */
u8 reserved_at_100[0x80];
/* add_action_field_support_2 */
@@ -2240,7 +2243,8 @@ struct mlx5_ifc_ft_fields_support_2_bits {
u8 inner_l4_checksum_ok[0x1];
u8 outer_ipv4_checksum_ok[0x1];
u8 outer_l4_checksum_ok[0x1]; /* end of DW0 */
-   u8 reserved_at_20[0x18];
+   u8 reserved_at_20[0x17];
+   u8 outer_ipv6_traffic_class[0x1];
union {
struct {
u8 metadata_reg_c_15[0x1];
diff --git a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c
index 5ae31c88f4..6ea0296109 100644
--- a/drivers/net/mlx5/linux/mlx5_os.c
+++ b/drivers/net/mlx5/linux/mlx5_os.c
@@ -1602,9 +1602,10 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
goto error;
}
rte_rwlock_init(&priv->ind_tbls_lock);
-   if (sh->config.dv_flow_en == 1 &&
+   if (!priv->sh->cdev->config.hca_attr.modify_outer_ipv6_traffic_class ||
+   (sh->config.dv_flow_en == 1 &&
!priv->sh->ipv6_tc_fallback &&
-   mlx5_flow_discover_ipv6_tc_support(eth_dev))
+   mlx5_flow_discover_ipv6_tc_support(eth_dev)))
priv->sh->ipv6_tc_fallback = 1;
if (priv->sh->config.dv_flow_en == 2) {
 #ifdef HAVE_MLX5_HWS_SUPPORT
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 33d4a28077..fe4f46724b 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -413,6 +413,9 @@ enum mlx5_feature_name {
 #define IPPROTO_MPLS 137
 #endif
 
+#define MLX5_IPV6_HDR_ECN_MASK 0x3
+#define MLX5_IPV6_HDR_DSCP_SHIFT 2
+
 /* UDP port number for MPLS */
 #define MLX5_UDP_PORT_MPLS 6635
 
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 97f55003c3..ecf86d861d 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -253,6 +253,11 @@ struct field_modify_info modify_ipv6[] = {
{0, 0, 0},
 };
 
+struct field_modify_info modify_ipv6_traffic_class[] = {
+   {1,  0, MLX5_MODI_OUT_IPV6_TRAFFIC_CLASS}