Re: [dpdk-dev] [PATCH v3] crypto/ccp: enable IOMMU for CCP

[Thomas Monjalon]

28/05/2021 07:02, Somalapuram, Amaranath:
> From: Thomas Monjalon 
> > 27/05/2021 15:24, David Marchand:
> > > On Fri, Dec 25, 2020 at 9:06 AM  wrote:
> > > > From: Amaranath Somalapuram 
> > > > 
> > > > CCP use vdev framework, and vdev framework don’t support IOMMU.
> > > > Adding custom IOMMU support for AMD CCP driver.
> > > 
> > > I am currently looking at pci bus patches/cleanups.
> > > I ended up looking at crypto/ccp.
> > > This driver code contains a lot of features duplicated with the pci bus.
> > > 
> > > Why is the ccp driver not a PCI driver?
> > 
> > Indeed it looks abusing vdev.
> > We should drop all the code duplicating the PCI bus driver.
> > If nothing else is done, it would mean breaking the probing of this
> > driver.
> > 
> > Adding more people in Cc list to have a fix before it is broken, thanks.
> 
> Enable IOMMU for vdev was not supported in DPDK.
> I can remove all the duplicating code after I test the CCP with IOMMU.

I think you didn't get it.
It should not be a vdev.
We want to switch the driver to a true PCI device,
and remove all the code copied from the PCI bus driver.

Re: [dpdk-dev] [PATCH v3] build: fix symlink of drivers for Windows

[Bruce Richardson]

On Mon, Apr 26, 2021 at 11:07:32AM +0100, Nick Connolly wrote:
> The symlink-drivers-solibs.sh script was disabled as part of 'install'
> for Windows because there is no support for shell scripts. However,
> this means that driver related DLLs are not present in the installed
> 'libdir' directory. Add a python script to perform the install and use
> it for Windows if the version of meson supports using an external
> program with add_install_script (>= 0.55.0).
> 
> On Windows, symbolic links are somewhat problematic since the
> SeCreateSymbolicLinkPrivilege is required to be able to create them.
> In addition, different cross-compilation environments handle symbolic
> links differently, e.g. WSL, Msys2, Cygwin. Rather than trying to
> distinguish these scenarios, the python script will perform a file copy
> for any Windows specific names.
> 
> On Windows, the shared library outputs have different names depending
> upon which toolset has been used to build them. The script currently
> handles Clang and GCC.
> 
> On Linux the functionality is unchanged, but could be replaced with the
> python script once the required minimum version of meson is >= 0.55.0.
> 
> Fixes: 5c7d86948764 ("build: fix install on Windows")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Nick Connolly 
> Tested-by: Narcisa Vasile 
> Acked-by: Narcisa Vasile 
> ---
Reviewed-by: Bruce Richardson 

Re: [dpdk-dev] [PATCH v3] build: fix symlink of drivers for Windows

[Bruce Richardson]

On Thu, May 27, 2021 at 06:37:57PM +0100, Nick Connolly wrote:
> Hi Bruce,
> 
> Would you have some time to take a look at this?  It's a replacement for the
> symlink-drivers-solibs.sh script for windows builds.
> 
> Thanks,
> Nick
> 
I've reviewed it now and it looks good. Couldn't even find anything to
nit-pick in it! :-)

Re: [dpdk-dev] [21.08 PATCH v1 1/2] power: invert the monitor check

[Ananyev, Konstantin]

> >
> > On 25-May-21 10:15 AM, Liu, Yong wrote:
> > >
> > >
> > >> -Original Message-
> > >> From: dev  On Behalf Of Anatoly Burakov
> > >> Sent: Tuesday, May 11, 2021 11:32 PM
> > >> To: dev@dpdk.org; McDaniel, Timothy ;
> > Xing,
> > >> Beilei ; Wu, Jingjing ; 
> > >> Yang,
> > >> Qiming ; Zhang, Qi Z ;
> > >> Wang, Haiyue ; Matan Azrad
> > >> ; Shahaf Shuler ; Viacheslav
> > >> Ovsiienko ; Richardson, Bruce
> > >> ; Ananyev, Konstantin
> > >> 
> > >> Cc: Loftus, Ciara 
> > >> Subject: [dpdk-dev] [21.08 PATCH v1 1/2] power: invert the monitor check
> > >>
> > >> Previously, the semantics of power monitor were such that we were
> > >> checking current value against the expected value, and if they matched,
> > >> then the sleep was aborted. This is somewhat inflexible, because it only
> > >> allowed us to check for a specific value.
> > >>
> > >> We can reverse the check, and instead have monitor sleep to be aborted
> > >> if the expected value *doesn't* match what's in memory. This allows us
> > >> to both implement all currently implemented driver code, as well as
> > >> support more use cases which don't easily map to previous semantics
> > >> (such as waiting on writes to AF_XDP counter value).
> > >>
> > >
> > > Hi Anatoly,
> > > In virtio spec, packed formatted descriptor utilizes two bits for 
> > > representing
> > the status. One bit for available status, one bit for used status.
> > > For checking the status more precisely, it is need to check value against 
> > > the
> > expected value.
> > > The monitor function in virtio datapath still can work with new semantics,
> > but it may lead to some useless io call.
> > > Base on that, I'd like to keep previous semantics.
> > >
> > > Regards,
> > > Marvin
> > >
> >
> > Thanks for your feedback! Would making this an option make things
> > better? Because we need the inverted semantics for AF_XDP, it can't work
> > without it. So, we either invert all of them, or we have an option to do
> > regular or inverted check on a per-condition basis. Would that work?
> >
> 
> That will be great if we can select the check type based on input parameter.
> Just in virtio datapath, we need both inverted and original semantics for 
> different ring formats.
> 

Should we probably the consider introducing _check_ callback to be provided by 
PMD?
So we can leave these various check details inside PMD itself. 
And monitor will just read the specified address and call the callback.
Konstantin

[dpdk-dev] [Bug 722] eal_save_args function has memory leak

[bugzilla]

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

Bug ID: 722
   Summary: eal_save_args function has memory leak
   Product: DPDK
   Version: unspecified
  Hardware: x86
OS: Linux
Status: UNCONFIRMED
  Severity: normal
  Priority: Normal
 Component: core
  Assignee: dev@dpdk.org
  Reporter: zhihongx.p...@intel.com
  Target Milestone: ---

Created attachment 157
  --> https://bugs.dpdk.org/attachment.cgi?id=157&action=edit
asan meson patch

Environment
DPDK: 21.05
clang: 9.0.0
test patch: asan meson patch pmdinfogen-allow-padding-after-NUL-terminator.diff

Test Setup:
1. git apply pmdinfogen-allow-padding-after-NUL-terminator.dif
2. CC=clang meson --werror -Denable_kmods=True -Dbuildtype=debug
-Db_lundef=false -Db_sanitize=address x86_64-native-linuxapp-clang
3. ninja -C x86_64-native-linuxapp-clang -j 110
4. ./x86_64-native-linuxapp-clang/app/dpdk-testpmd -c 0x6 -n 4 – -i
5. testpmd> quit
6. expect result
quit normally
actual result
There are some errors here:

Detected memory leaks:
Direct leak of 3 byte(s) in 1 object(s) allocated from:
EAL: recvmsg failed, Bad file descriptor
EAL: recvmsg failed, Bad file descriptor
EAL: recvmsg failed, Bad file descriptor
EAL: recvmsg failed, Bad file descriptor
EAL: recvmsg failed, Bad file descriptor
EAL: recvmsg failed, Bad file descriptor
#0 0x573d94 in strdup
(/home/pzh/main_dpdk/x86_64-native-linuxapp-clang/app/dpdk-testpmd+0x573d94)
EAL: recvmsg failed, Bad file descriptor
EAL: recvmsg failed, Bad file descriptor
EAL: recvmsg failed, Bad file descriptor
#1 0x14f5ca5 in eal_save_args
/home/pzh/main_dpdk/x86_64-native-linuxapp-clang/../lib/eal/common/eal_common_options.c:232:17
EAL: recvmsg failed, Bad file descriptor
EAL: recvmsg failed, Bad file descriptor
EAL: recvmsg failed, Bad file descriptor
#2 0x155266f in rte_eal_init
/home/pzh/main_dpdk/x86_64-native-linuxapp-clang/../lib/eal/linux/eal.c:1002:2
EAL: recvmsg failed, Bad file descriptor
EAL: recvmsg failed, Bad file descriptor
EAL: recvmsg failed, Bad file descriptor
#3 0x80f964 in main
/home/pzh/main_dpdk/x86_64-native-linuxapp-clang/../app/test-pmd/testpmd.c:3752:9
EAL: recvmsg failed, Bad file descriptor
EAL: recvmsg failed, Bad file descriptor
#4 0x7fe863d22b96 in __libc_start_main
/build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310

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

[dpdk-dev] [Bug 723] pci_scan_one function has memory leak

[bugzilla]

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

Bug ID: 723
   Summary: pci_scan_one function has memory leak
   Product: DPDK
   Version: unspecified
  Hardware: x86
OS: Linux
Status: UNCONFIRMED
  Severity: normal
  Priority: Normal
 Component: core
  Assignee: dev@dpdk.org
  Reporter: zhihongx.p...@intel.com
  Target Milestone: ---

Created attachment 158
  --> https://bugs.dpdk.org/attachment.cgi?id=158&action=edit
asan meson patch

Environment
failed on: CVL 25G*4
dpdk: 21.05
clang: 9.0.0
test patch: asan meson patch pmdinfogen-allow-padding-after-NUL-terminator.diff

Test Setup:
1. git apply pmdinfogen-allow-padding-after-NUL-terminator.dif
2. CC=clang meson --werror -Denable_kmods=True -Dbuildtype=debug
-Db_lundef=false -Db_sanitize=address x86_64-native-linuxapp-clang
3. ninja -C x86_64-native-linuxapp-clang -j 110
4. ./x86_64-native-linuxapp-clang/app/dpdk-testpmd -c 0x6 -n 4 – -i
5. testpmd> quit
6. expect result
quit normally
actual result
There are some errors here:

Direct leak of 12379752 byte(s) in 231 object(s) allocated from:
EAL: recvmsg failed, Bad file descriptor
EAL: recvmsg failed, Bad file descriptor
EAL: recvmsg failed, Bad file descriptor
EAL: recvmsg failed, Bad file descriptor
EAL: recvmsg failed, Bad file descriptor
#0 0x58801d in malloc
(/home/pzh/main_dpdk/x86_64-native-linuxapp-clang/app/dpdk-testpmd+0x58801d)
EAL: recvmsg failed, Bad file descriptor
EAL: recvmsg failed, Bad file descriptor
EAL: recvmsg failed, Bad file descriptor
EAL: recvmsg failed, Bad file descriptor
#1 0x1688d9a in pci_scan_one
/home/pzh/main_dpdk/x86_64-native-linuxapp-clang/../drivers/bus/pci/linux/pci.c:225:8
EAL: recvmsg failed, Bad file descriptor
EAL: recvmsg failed, Bad file descriptor
EAL: recvmsg failed, Bad file descriptor
#2 0x16883a3 in rte_pci_scan
/home/pzh/main_dpdk/x86_64-native-linuxapp-clang/../drivers/bus/pci/linux/pci.c:487:7
EAL: recvmsg failed, Bad file descriptor
EAL: recvmsg failed, Bad file descriptor
EAL: recvmsg failed, Bad file descriptor
#3 0x14ce0d6 in rte_bus_scan
/home/pzh/main_dpdk/x86_64-native-linuxapp-clang/../lib/eal/common/eal_common_bus.c:50:9
EAL: recvmsg failed, Bad file descriptor
EAL: recvmsg failed, Bad file descriptor
EAL: recvmsg failed, Bad file descriptor
#4 0x1552c7a in rte_eal_init
/home/pzh/main_dpdk/x86_64-native-linuxapp-clang/../lib/eal/linux/eal.c:1070:6
EAL: recvmsg failed, Bad file descriptor
EAL: recvmsg failed, Bad file descriptor
EAL: recvmsg failed, Bad file descriptor
#5 0x80f964 in main
/home/pzh/main_dpdk/x86_64-native-linuxapp-clang/../app/test-pmd/testpmd.c:3752:9

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

Re: [dpdk-dev] [PATCH v3] crypto/ccp: enable IOMMU for CCP

[Somalapuram, Amaranath]

[Public]



-Original Message-
From: Thomas Monjalon  
Sent: Friday, May 28, 2021 12:51 PM
To: Somalapuram, Amaranath 
Cc: David Marchand ; dev ; Akhil Goyal 
; Kumar, Ravi1 ; Song, Keesang 

Subject: Re: [dpdk-dev] [PATCH v3] crypto/ccp: enable IOMMU for CCP

[CAUTION: External Email]

28/05/2021 07:02, Somalapuram, Amaranath:
> From: Thomas Monjalon 
> > 27/05/2021 15:24, David Marchand:
> > > On Fri, Dec 25, 2020 at 9:06 AM  wrote:
> > > > From: Amaranath Somalapuram 
> > > >
> > > > CCP use vdev framework, and vdev framework don’t support IOMMU.
> > > > Adding custom IOMMU support for AMD CCP driver.
> > >
> > > I am currently looking at pci bus patches/cleanups.
> > > I ended up looking at crypto/ccp.
> > > This driver code contains a lot of features duplicated with the pci bus.
> > >
> > > Why is the ccp driver not a PCI driver?
> >
> > Indeed it looks abusing vdev.
> > We should drop all the code duplicating the PCI bus driver.
> > If nothing else is done, it would mean breaking the probing of this 
> > driver.
> >
> > Adding more people in Cc list to have a fix before it is broken, thanks.
>
> Enable IOMMU for vdev was not supported in DPDK.
> I can remove all the duplicating code after I test the CCP with IOMMU.

I think you didn't get it.
It should not be a vdev.
We want to switch the driver to a true PCI device, and remove all the code 
copied from the PCI bus driver.

We will implement CCP as true PCI device. 

Re: [dpdk-dev] [PATCH] net: introduce IPv4 ihl and version fields

[Ananyev, Konstantin]

 
> > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Gregory Etelson
> > Sent: Thursday, 27 May 2021 17.29
> and version fields
> >
> > RTE IPv4 header definition combines the `version' and `ihl'  fields
> > into a single structure member.
> > This patch introduces dedicated structure members for both `version'
> > and `ihl' IPv4 fields. Separated header fields definitions allow to
> > create simplified code to match on the IHL value in a flow rule.
> > The original `version_ihl' structure member is kept for backward
> > compatibility.
> >
> > Signed-off-by: Gregory Etelson 
> > ---
> >  app/test/test_flow_classify.c |  8 
> >  lib/net/rte_ip.h  | 16 +++-
> >  2 files changed, 19 insertions(+), 5 deletions(-)
> >
> > diff --git a/app/test/test_flow_classify.c
> > b/app/test/test_flow_classify.c
> > index 951606f248..4f64be5357 100644
> > --- a/app/test/test_flow_classify.c
> > +++ b/app/test/test_flow_classify.c
> > @@ -95,7 +95,7 @@ static struct rte_acl_field_def
> > ipv4_defs[NUM_FIELDS_IPV4] = {
> >   *  dst mask 255.255.255.00 / udp src is 32 dst is 33 / end"
> >   */
> >  static struct rte_flow_item_ipv4 ipv4_udp_spec_1 = {
> > -   { 0, 0, 0, 0, 0, 0, IPPROTO_UDP, 0,
> > +   { { .version_ihl = 0}, 0, 0, 0, 0, 0, IPPROTO_UDP, 0,
> >   RTE_IPV4(2, 2, 2, 3), RTE_IPV4(2, 2, 2, 7)}
> >  };
> >  static const struct rte_flow_item_ipv4 ipv4_mask_24 = {
> > @@ -131,7 +131,7 @@ static struct rte_flow_item  end_item = {
> > RTE_FLOW_ITEM_TYPE_END,
> >   *  dst mask 255.255.255.00 / tcp src is 16 dst is 17 / end"
> >   */
> >  static struct rte_flow_item_ipv4 ipv4_tcp_spec_1 = {
> > -   { 0, 0, 0, 0, 0, 0, IPPROTO_TCP, 0,
> > +   { { .version_ihl = 0}, 0, 0, 0, 0, 0, IPPROTO_TCP, 0,
> >   RTE_IPV4(1, 2, 3, 4), RTE_IPV4(5, 6, 7, 8)}
> >  };
> >
> > @@ -150,8 +150,8 @@ static struct rte_flow_item  tcp_item_1 = {
> > RTE_FLOW_ITEM_TYPE_TCP,
> >   *  dst mask 255.255.255.00 / sctp src is 16 dst is 17/ end"
> >   */
> >  static struct rte_flow_item_ipv4 ipv4_sctp_spec_1 = {
> > -   { 0, 0, 0, 0, 0, 0, IPPROTO_SCTP, 0, RTE_IPV4(11, 12, 13, 14),
> > -   RTE_IPV4(15, 16, 17, 18)}
> > +   { { .version_ihl = 0}, 0, 0, 0, 0, 0, IPPROTO_SCTP, 0,
> > +   RTE_IPV4(11, 12, 13, 14), RTE_IPV4(15, 16, 17, 18)}
> >  };
> >
> >  static struct rte_flow_item_sctp sctp_spec_1 = {
> > diff --git a/lib/net/rte_ip.h b/lib/net/rte_ip.h
> > index 4b728969c1..684bb028b2 100644
> > --- a/lib/net/rte_ip.h
> > +++ b/lib/net/rte_ip.h
> > @@ -38,7 +38,21 @@ extern "C" {
> >   * IPv4 Header
> >   */
> >  struct rte_ipv4_hdr {
> > -   uint8_t  version_ihl;   /**< version and header length */
> > +   __extension__
> > +   union {
> > +   uint8_t version_ihl;/**< version and header length */
> > +   struct {
> > +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> > +   uint8_t ihl:4;
> > +   uint8_t version:4;
> > +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> > +   uint8_t version:4;
> > +   uint8_t ihl:4;
> > +#else
> > +#error "setup endian definition"
> > +#endif
> > +   };
> > +   };
> > uint8_t  type_of_service;   /**< type of service */
> > rte_be16_t total_length;/**< length of packet */
> > rte_be16_t packet_id;   /**< packet ID */
> > --
> > 2.31.1
> >
> 
> This does not break the ABI, but it could be discussed if it breaks the API 
> due to the required structure initialization changes shown in
> test_flow_classify.c.

Yep, I guess it might be classified as API change.
Another thing that concerns me - it is not the only place in IPv4 header when 
we unite multiple bit-fields into one field:
type_of_service, fragment_offset.
If we start splitting ipv4 fields into actual bitfields, I suppose we'll end-up 
splitting these ones too.
But I am not sure it will pay off - as compiler not always generates optimal 
code for reading/updating bitfields.
Did you consider just adding extra macros to simplify access to these fields 
(like RTE_IPV4_HDR_(GET_SET)_*),
instead?  

> I think this patch is an improvement, and that such structure modifications 
> should be generally accepted, so:
> 
> Acked-by: Morten Brørup 

[dpdk-dev] [PATCH] vhost: allocate and free packets in bulk in Tx split

[Balazs Nemeth]

Same idea as commit a287ac28919d ("vhost: allocate and free packets
in bulk in Tx packed"), allocate and free packets in bulk. Also remove
the unused function virtio_dev_pktmbuf_alloc.

Signed-off-by: Balazs Nemeth 
---
 lib/vhost/virtio_net.c | 37 -
 1 file changed, 8 insertions(+), 29 deletions(-)

diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
index 8da8a86a10..32aa2c19a9 100644
--- a/lib/vhost/virtio_net.c
+++ b/lib/vhost/virtio_net.c
@@ -2670,32 +2670,6 @@ virtio_dev_pktmbuf_prep(struct virtio_net *dev, struct 
rte_mbuf *pkt,
return -1;
 }
 
-/*
- * Allocate a host supported pktmbuf.
- */
-static __rte_always_inline struct rte_mbuf *
-virtio_dev_pktmbuf_alloc(struct virtio_net *dev, struct rte_mempool *mp,
-uint32_t data_len)
-{
-   struct rte_mbuf *pkt = rte_pktmbuf_alloc(mp);
-
-   if (unlikely(pkt == NULL)) {
-   VHOST_LOG_DATA(ERR,
-   "Failed to allocate memory for mbuf.\n");
-   return NULL;
-   }
-
-   if (virtio_dev_pktmbuf_prep(dev, pkt, data_len)) {
-   /* Data doesn't fit into the buffer and the host supports
-* only linear buffers
-*/
-   rte_pktmbuf_free(pkt);
-   return NULL;
-   }
-
-   return pkt;
-}
-
 __rte_always_inline
 static uint16_t
 virtio_dev_tx_split(struct virtio_net *dev, struct vhost_virtqueue *vq,
@@ -2725,6 +2699,9 @@ virtio_dev_tx_split(struct virtio_net *dev, struct 
vhost_virtqueue *vq,
VHOST_LOG_DATA(DEBUG, "(%d) about to dequeue %u buffers\n",
dev->vid, count);
 
+   if (rte_pktmbuf_alloc_bulk(mbuf_pool, pkts, count))
+   return 0;
+
for (i = 0; i < count; i++) {
struct buf_vector buf_vec[BUF_VECTOR_MAX];
uint16_t head_idx;
@@ -2741,8 +2718,8 @@ virtio_dev_tx_split(struct virtio_net *dev, struct 
vhost_virtqueue *vq,
 
update_shadow_used_ring_split(vq, head_idx, 0);
 
-   pkts[i] = virtio_dev_pktmbuf_alloc(dev, mbuf_pool, buf_len);
-   if (unlikely(pkts[i] == NULL)) {
+   err = virtio_dev_pktmbuf_prep(dev, pkts[i], buf_len);
+   if (unlikely(err)) {
/*
 * mbuf allocation fails for jumbo packets when external
 * buffer allocation is not allowed and linear buffer
@@ -2762,7 +2739,6 @@ virtio_dev_tx_split(struct virtio_net *dev, struct 
vhost_virtqueue *vq,
err = copy_desc_to_mbuf(dev, vq, buf_vec, nr_vec, pkts[i],
mbuf_pool, legacy_ol_flags);
if (unlikely(err)) {
-   rte_pktmbuf_free(pkts[i]);
if (!allocerr_warned) {
VHOST_LOG_DATA(ERR,
"Failed to copy desc to mbuf on %s.\n",
@@ -2775,6 +2751,9 @@ virtio_dev_tx_split(struct virtio_net *dev, struct 
vhost_virtqueue *vq,
}
}
 
+   if (i != count)
+   rte_pktmbuf_free_bulk(&pkts[i - 1], count - i);
+
vq->last_avail_idx += i;
 
do_data_copy_dequeue(vq);
-- 
2.30.2

[dpdk-dev] i40e PMD Linux bonding issue with VFs

[Fried Onions]

I came across the patch for the i40e PMD that allows VF to set the MAC
address even if the PF had already done so in
https://mails.dpdk.org/archives/dev/2020-March/160631.html


I tried backporting it to DPDK 18.11's i40e PMD and it fixes the issue of
allowing the VF to set the MAC address.

However, Linux bonding is still not working because the values don't seem
to be updated at host level.

Would appreciate it if anyone has any idea what other patches are needed
for Linux bonding with VFs to work on the i40e PMD in DPDK18.11 in terms of
patches to the PMD or firmware upgrades on the XL710 card? Or if Linux
bonding can be supported at all with the i40e PMD with VFs?

The kernel's i40e driver has the following known issue with Linux bonding
with VFs (copied below https://github.com/dmarion/i40e/blob/master/README),
but that seems to have been fixed with the 2.15.9 release, and the patch
above seems to resolve the issue of setting the MAC address by the VF for
the PMD, but looks like it's insufficient for it to work when backported to
the DPDK18.11's i40e PMD.

Linux bonding fails with Virtual Functions bound to an Intel(R) Ethernet

Controller 700 series based device



If you bind Virtual Functions (VFs) to an Intel(R) Ethernet Controller 700

series based device, the VF slaves may fail when they become the active
slave.

*If the MAC address of the VF is set by the PF (Physical Function) of the*

*device, when you add a slave, or change the active-backup slave, Linux
bonding*

*tries to sync the backup slave's MAC address to the same MAC address as
the*

*active slave. Linux bonding will fail at this point. *This issue will not
occur

if the VF's MAC address is not set by the PF.

Re: [dpdk-dev] [PATCH v3] build: fix symlink of drivers for Windows

[Nick Connolly]

I've reviewed it now and it looks good. Couldn't even find anything to
nit-pick in it! :-)


Thanks Bruce - appreciated  :-)
Nick

Re: [dpdk-dev] [PATCH] net: introduce IPv4 ihl and version fields

[Morten Brørup]

> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Ananyev,
> Konstantin
> Sent: Friday, 28 May 2021 12.21
> 
> > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Gregory
> Etelson
> > > Sent: Thursday, 27 May 2021 17.29
> > and version fields
> > >
> > > RTE IPv4 header definition combines the `version' and `ihl'  fields
> > > into a single structure member.
> > > This patch introduces dedicated structure members for both
> `version'
> > > and `ihl' IPv4 fields. Separated header fields definitions allow to
> > > create simplified code to match on the IHL value in a flow rule.
> > > The original `version_ihl' structure member is kept for backward
> > > compatibility.
> > >
> > > Signed-off-by: Gregory Etelson 
> > > ---
> > >  app/test/test_flow_classify.c |  8 
> > >  lib/net/rte_ip.h  | 16 +++-
> > >  2 files changed, 19 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/app/test/test_flow_classify.c
> > > b/app/test/test_flow_classify.c
> > > index 951606f248..4f64be5357 100644
> > > --- a/app/test/test_flow_classify.c
> > > +++ b/app/test/test_flow_classify.c
> > > @@ -95,7 +95,7 @@ static struct rte_acl_field_def
> > > ipv4_defs[NUM_FIELDS_IPV4] = {
> > >   *  dst mask 255.255.255.00 / udp src is 32 dst is 33 / end"
> > >   */
> > >  static struct rte_flow_item_ipv4 ipv4_udp_spec_1 = {
> > > - { 0, 0, 0, 0, 0, 0, IPPROTO_UDP, 0,
> > > + { { .version_ihl = 0}, 0, 0, 0, 0, 0, IPPROTO_UDP, 0,
> > > RTE_IPV4(2, 2, 2, 3), RTE_IPV4(2, 2, 2, 7)}
> > >  };
> > >  static const struct rte_flow_item_ipv4 ipv4_mask_24 = {
> > > @@ -131,7 +131,7 @@ static struct rte_flow_item  end_item = {
> > > RTE_FLOW_ITEM_TYPE_END,
> > >   *  dst mask 255.255.255.00 / tcp src is 16 dst is 17 / end"
> > >   */
> > >  static struct rte_flow_item_ipv4 ipv4_tcp_spec_1 = {
> > > - { 0, 0, 0, 0, 0, 0, IPPROTO_TCP, 0,
> > > + { { .version_ihl = 0}, 0, 0, 0, 0, 0, IPPROTO_TCP, 0,
> > > RTE_IPV4(1, 2, 3, 4), RTE_IPV4(5, 6, 7, 8)}
> > >  };
> > >
> > > @@ -150,8 +150,8 @@ static struct rte_flow_item  tcp_item_1 = {
> > > RTE_FLOW_ITEM_TYPE_TCP,
> > >   *  dst mask 255.255.255.00 / sctp src is 16 dst is 17/ end"
> > >   */
> > >  static struct rte_flow_item_ipv4 ipv4_sctp_spec_1 = {
> > > - { 0, 0, 0, 0, 0, 0, IPPROTO_SCTP, 0, RTE_IPV4(11, 12, 13, 14),
> > > - RTE_IPV4(15, 16, 17, 18)}
> > > + { { .version_ihl = 0}, 0, 0, 0, 0, 0, IPPROTO_SCTP, 0,
> > > + RTE_IPV4(11, 12, 13, 14), RTE_IPV4(15, 16, 17, 18)}
> > >  };
> > >
> > >  static struct rte_flow_item_sctp sctp_spec_1 = {
> > > diff --git a/lib/net/rte_ip.h b/lib/net/rte_ip.h
> > > index 4b728969c1..684bb028b2 100644
> > > --- a/lib/net/rte_ip.h
> > > +++ b/lib/net/rte_ip.h
> > > @@ -38,7 +38,21 @@ extern "C" {
> > >   * IPv4 Header
> > >   */
> > >  struct rte_ipv4_hdr {
> > > - uint8_t  version_ihl;   /**< version and header length */
> > > + __extension__
> > > + union {
> > > + uint8_t version_ihl;/**< version and header length */
> > > + struct {
> > > +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> > > + uint8_t ihl:4;
> > > + uint8_t version:4;
> > > +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> > > + uint8_t version:4;
> > > + uint8_t ihl:4;
> > > +#else
> > > +#error "setup endian definition"
> > > +#endif
> > > + };
> > > + };
> > >   uint8_t  type_of_service;   /**< type of service */
> > >   rte_be16_t total_length;/**< length of packet */
> > >   rte_be16_t packet_id;   /**< packet ID */
> > > --
> > > 2.31.1
> > >
> >
> > This does not break the ABI, but it could be discussed if it breaks
> the API due to the required structure initialization changes shown in
> > test_flow_classify.c.
> 
> Yep, I guess it might be classified as API change.
> Another thing that concerns me - it is not the only place in IPv4
> header when we unite multiple bit-fields into one field:
> type_of_service, fragment_offset.
> If we start splitting ipv4 fields into actual bitfields, I suppose
> we'll end-up splitting these ones too.
> But I am not sure it will pay off - as compiler not always generates
> optimal code for reading/updating bitfields.
> Did you consider just adding extra macros to simplify access to these
> fields (like RTE_IPV4_HDR_(GET_SET)_*),
> instead?
> 

Let's please not introduce accessor macros for bitfields. If we don't introduce 
bitfields like these, I would rather stick with the current _MASK, _SHIFT and 
_FLAG defines.

Yes, this change will lead to the introduction of more bitfields, both here and 
in other places. We already accepted it in the eCPRI structure 
(/lib/net/rte_ecpri.h), so why not just generally accept it.

Are modern compilers really worse at handling a bitfield defined like this, 
compared to handling a single uint8_t with hand coding? I consider your concern 
very important, so I'm only asking if it is still relevant, to avoid making 
decisions based on past experience that might be out

[dpdk-dev] [PATCH v2 0/2] extend idxd config script functionality

[Kevin Laatz]

This patchset extends the functionality of the idxd config script, the
additions are described in the individual patches.

Kevin Laatz (2):
  raw/ioat: add pci address handling to python script
  raw/ioat: add device reset to python script

 drivers/raw/ioat/dpdk_idxd_cfg.py | 41 +--
 1 file changed, 39 insertions(+), 2 deletions(-)

-- 
2.30.2

[dpdk-dev] [PATCH v2 2/2] raw/ioat: add device reset to python script

[Kevin Laatz]

Currently once a device is configured, the user does not have the ability
to reset the device via the script.

This patch adds a device reset option to the script. For example
"$dpdk_idxd_cfg.py 0 --reset" would reset device 0.

Signed-off-by: Kevin Laatz 
---
 drivers/raw/ioat/dpdk_idxd_cfg.py | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/raw/ioat/dpdk_idxd_cfg.py 
b/drivers/raw/ioat/dpdk_idxd_cfg.py
index ad8393a645..138b555040 100755
--- a/drivers/raw/ioat/dpdk_idxd_cfg.py
+++ b/drivers/raw/ioat/dpdk_idxd_cfg.py
@@ -29,6 +29,12 @@ def write_values(self, values):
 f.write(str(contents))
 
 
+def reset_device(dsa_id):
+"Reset the DSA device and all its queues"
+drv_dir = SysfsDir("/sys/bus/dsa/drivers/dsa")
+drv_dir.write_values({"unbind": f"dsa{dsa_id}"})
+
+
 def get_pci_dir(pci):
 "Search for the sysfs directory of the PCI device"
 base_dir = '/sys/bus/pci/devices/'
@@ -95,11 +101,18 @@ def main(args):
 arg_p.add_argument('--name-prefix', metavar='prefix', dest='prefix',
default="dpdk",
help="Prefix for workqueue name to mark for DPDK use 
[default: 'dpdk']")
+arg_p.add_argument('--reset', action='store_true',
+   help="Reset DSA device and its queues")
 parsed_args = arg_p.parse_args(args[1:])
 
 dsa_id = parsed_args.dsa_id
 dsa_id = get_dsa_id(dsa_id) if ':' in dsa_id else dsa_id
-configure_dsa(dsa_id, parsed_args.q, parsed_args.prefix)
+if parsed_args.reset:
+print(f"Resetting DSA instance {dsa_id}")
+reset_device(dsa_id)
+else:
+print(f"Configuring DSA instance {dsa_id}")
+configure_dsa(dsa_id, parsed_args.q, parsed_args.prefix)
 
 
 if __name__ == "__main__":
-- 
2.30.2

[dpdk-dev] [PATCH v2 1/2] raw/ioat: add pci address handling to python script

[Kevin Laatz]

Currently the user needs to find the DSA instance number for any DSA device
they would like to configure using this script, which can be cumbersome and
error-prone since the instance numbering changes when changing the binding
of the devices between vfio-pci and idxd.

This patch improved the usability of the script by adding the ability to
specify the DSA device to configure using the device's PCI address instead
of the DSA instance number. For example, "$dpdk_idxd_cfg.py 0" and
"$dpdk_idxd_cfg.py 6a:01.0" are both valid references to the same device
(assuming the numbering).

Signed-off-by: Kevin Laatz 

---
v2:
   - split the device reset change into its own patch
   - changed sysfs dir search
   - improved error case handling
---
 drivers/raw/ioat/dpdk_idxd_cfg.py | 28 ++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/raw/ioat/dpdk_idxd_cfg.py 
b/drivers/raw/ioat/dpdk_idxd_cfg.py
index ff06d9e240..ad8393a645 100755
--- a/drivers/raw/ioat/dpdk_idxd_cfg.py
+++ b/drivers/raw/ioat/dpdk_idxd_cfg.py
@@ -29,6 +29,26 @@ def write_values(self, values):
 f.write(str(contents))
 
 
+def get_pci_dir(pci):
+"Search for the sysfs directory of the PCI device"
+base_dir = '/sys/bus/pci/devices/'
+for path, dirs, files in os.walk(base_dir):
+for dir in dirs:
+if pci in dir:
+return os.path.join(base_dir, dir)
+sys.exit(f"Could not find sysfs directory for device {pci}")
+
+
+def get_dsa_id(pci):
+"Get the DSA instance ID using the PCI address of the device"
+pci_dir = get_pci_dir(pci)
+for path, dirs, files in os.walk(pci_dir):
+for dir in dirs:
+if dir.startswith('dsa') and 'wq' not in dir:
+return int(dir[3:])
+sys.exit(f"Could not get device ID for device {pci}")
+
+
 def configure_dsa(dsa_id, queues, prefix):
 "Configure the DSA instance with appropriate number of queues"
 dsa_dir = SysfsDir(f"/sys/bus/dsa/devices/dsa{dsa_id}")
@@ -68,14 +88,18 @@ def main(args):
 "Main function, does arg parsing and calls config function"
 arg_p = argparse.ArgumentParser(
 description="Configure whole DSA device instance for DPDK use")
-arg_p.add_argument('dsa_id', type=int, help="DSA instance number")
+arg_p.add_argument('dsa_id',
+   help="Specify DSA instance either via DSA instance 
number or PCI address")
 arg_p.add_argument('-q', metavar='queues', type=int, default=255,
help="Number of queues to set up")
 arg_p.add_argument('--name-prefix', metavar='prefix', dest='prefix',
default="dpdk",
help="Prefix for workqueue name to mark for DPDK use 
[default: 'dpdk']")
 parsed_args = arg_p.parse_args(args[1:])
-configure_dsa(parsed_args.dsa_id, parsed_args.q, parsed_args.prefix)
+
+dsa_id = parsed_args.dsa_id
+dsa_id = get_dsa_id(dsa_id) if ':' in dsa_id else dsa_id
+configure_dsa(dsa_id, parsed_args.q, parsed_args.prefix)
 
 
 if __name__ == "__main__":
-- 
2.30.2

[dpdk-dev] [PATCH] net/virtio: fix kernel set features for multi-queue devices

[Thierry Herbelot]

Restore the original code, where VHOST_SET_FEATURES is applied to
all vhostfds of the device.

Fixes: cc0151b34dee ("net/virtio: add virtio-user features ops")
Cc: sta...@dpdk.org
Cc: Maxime Coquelin 
Cc: Chenbo Xia 

Signed-off-by: Thierry Herbelot 
---
 drivers/net/virtio/virtio_user/vhost_kernel.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/net/virtio/virtio_user/vhost_kernel.c 
b/drivers/net/virtio/virtio_user/vhost_kernel.c
index ad46f10a9300..d65f89e1fc16 100644
--- a/drivers/net/virtio/virtio_user/vhost_kernel.c
+++ b/drivers/net/virtio/virtio_user/vhost_kernel.c
@@ -158,6 +158,8 @@ static int
 vhost_kernel_set_features(struct virtio_user_dev *dev, uint64_t features)
 {
struct vhost_kernel_data *data = dev->backend_data;
+   uint32_t i;
+   int ret;
 
/* We don't need memory protection here */
features &= ~(1ULL << VIRTIO_F_IOMMU_PLATFORM);
@@ -166,7 +168,16 @@ vhost_kernel_set_features(struct virtio_user_dev *dev, 
uint64_t features)
features &= ~VHOST_KERNEL_HOST_OFFLOADS_MASK;
features &= ~(1ULL << VIRTIO_NET_F_MQ);
 
-   return vhost_kernel_ioctl(data->vhostfds[0], VHOST_SET_FEATURES, 
&features);
+   for (i = 0; i < dev->max_queue_pairs; ++i) {
+   if (data->vhostfds[i] < 0)
+   continue;
+
+   ret = vhost_kernel_ioctl(data->vhostfds[i], VHOST_SET_FEATURES, 
&features);
+   if (ret < 0)
+   return ret;
+   }
+
+   return 0;
 }
 
 static int
-- 
2.29.2

[dpdk-dev] [PATCH 1/2] common/mlx5: add glue function for duplicate rule ability

[Jiawei Wang]

Add glue function to update the duplicate rule allow/disallow
through rdma-core DV API.

Signed-off-by: Jiawei Wang 
---
 drivers/common/mlx5/linux/meson.build |  2 ++
 drivers/common/mlx5/linux/mlx5_glue.c | 12 
 drivers/common/mlx5/linux/mlx5_glue.h |  1 +
 3 files changed, 15 insertions(+)

diff --git a/drivers/common/mlx5/linux/meson.build 
b/drivers/common/mlx5/linux/meson.build
index 007834a49b..c0bfab1f33 100644
--- a/drivers/common/mlx5/linux/meson.build
+++ b/drivers/common/mlx5/linux/meson.build
@@ -191,6 +191,8 @@ has_sym_args = [
 'mlx5dv_dump_dr_rule' ],
 [ 'HAVE_MLX5_DR_ACTION_ASO_CT', 'infiniband/mlx5dv.h',
 'MLX5DV_DR_ACTION_FLAGS_ASO_CT_DIRECTION_INITIATOR' ],
+   [ 'HAVE_MLX5_DR_ALLOW_DUPLICATE', 'infiniband/mlx5dv.h',
+   'mlx5dv_dr_domain_allow_duplicate_rules' ],
 ]
 config = configuration_data()
 foreach arg:has_sym_args
diff --git a/drivers/common/mlx5/linux/mlx5_glue.c 
b/drivers/common/mlx5/linux/mlx5_glue.c
index d3bd645a5b..145cf83fd9 100644
--- a/drivers/common/mlx5/linux/mlx5_glue.c
+++ b/drivers/common/mlx5/linux/mlx5_glue.c
@@ -1321,6 +1321,17 @@ mlx5_glue_dv_alloc_pp(struct ibv_context *context,
 #endif
 }
 
+static void
+mlx5_glue_dr_allow_duplicate_rules(void *domain, uint32_t allow)
+{
+#ifdef HAVE_MLX5_DR_ALLOW_DUPLICATE
+   mlx5dv_dr_domain_allow_duplicate_rules(domain, allow);
+#else
+   (void)(allow);
+   (void)(domain);
+#endif
+}
+
 static void
 mlx5_glue_dv_free_pp(struct mlx5dv_pp *pp)
 {
@@ -1441,6 +1452,7 @@ const struct mlx5_glue *mlx5_glue = &(const struct 
mlx5_glue) {
mlx5_glue_dr_create_flow_action_sampler,
.dr_create_flow_action_dest_array =
mlx5_glue_dr_action_create_dest_array,
+   .dr_allow_duplicate_rules = mlx5_glue_dr_allow_duplicate_rules,
.devx_query_eqn = mlx5_glue_devx_query_eqn,
.devx_create_event_channel = mlx5_glue_devx_create_event_channel,
.devx_destroy_event_channel = mlx5_glue_devx_destroy_event_channel,
diff --git a/drivers/common/mlx5/linux/mlx5_glue.h 
b/drivers/common/mlx5/linux/mlx5_glue.h
index 97462e9ab8..56246bca18 100644
--- a/drivers/common/mlx5/linux/mlx5_glue.h
+++ b/drivers/common/mlx5/linux/mlx5_glue.h
@@ -336,6 +336,7 @@ struct mlx5_glue {
 struct mlx5dv_devx_async_event_hdr *event_data,
 size_t event_resp_len);
void (*dr_reclaim_domain_memory)(void *domain, uint32_t enable);
+   void (*dr_allow_duplicate_rules)(void *domain, uint32_t allow);
struct mlx5dv_pp *(*dv_alloc_pp)(struct ibv_context *context,
 size_t pp_context_sz,
 const void *pp_context,
-- 
2.18.1

[dpdk-dev] [PATCH 2/2] net/mlx5: control rules with identical pattern behavior

[Jiawei Wang]

In order to allow\disallow configuring rules with identical
patterns, the new device argument 'allow_duplicate_pattern'
is introduced.
If allow, these rules be inserted successfully and only the
first rule take affect.
If disallow, the first rule will be inserted and other rules
be rejected.

The default is to allow.
Set it to 0 if disallow, for example:
-a ,allow_duplicate_pattern=0

Signed-off-by: Jiawei Wang 
---
 doc/guides/nics/mlx5.rst   | 10 ++
 doc/guides/rel_notes/release_21_08.rst |  6 ++
 drivers/net/mlx5/linux/mlx5_os.c   |  7 +++
 drivers/net/mlx5/mlx5.c|  6 ++
 drivers/net/mlx5/mlx5.h|  2 ++
 drivers/net/mlx5/mlx5_flow_dv.c|  3 +++
 6 files changed, 34 insertions(+)

diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index 83299646dd..26f4a25441 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -1058,6 +1058,16 @@ Driver options
 
   By default, the PMD will set this value to 1.
 
+- ``allow_duplicate_pattern`` parameter [int]
+
+  There are two options to choose:
+
+  - 0. Prevent insertion of rules with the same pattern items on non-root 
table.
+
+  - 1. Allow insertion of rules with the same pattern items.
+
+  By default, the PMD will set this value to 1.
+
 .. _mlx5_firmware_config:
 
 Firmware configuration
diff --git a/doc/guides/rel_notes/release_21_08.rst 
b/doc/guides/rel_notes/release_21_08.rst
index a6ecfdf3ce..e6f696a71d 100644
--- a/doc/guides/rel_notes/release_21_08.rst
+++ b/doc/guides/rel_notes/release_21_08.rst
@@ -55,6 +55,12 @@ New Features
  Also, make sure to start the actual text at the margin.
  ===
 
+* **Updated Mellanox mlx5 net driver and common layer.**
+
+  Updated Mellanox mlx5 driver with new features and improvements, including:
+
+  * Added devargs options ``allow_duplicate_pattern``.
+
 
 Removed Items
 -
diff --git a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c
index 534a56a555..7c4384ca77 100644
--- a/drivers/net/mlx5/linux/mlx5_os.c
+++ b/drivers/net/mlx5/linux/mlx5_os.c
@@ -355,6 +355,12 @@ mlx5_alloc_shared_dr(struct mlx5_priv *priv)
mlx5_glue->dr_reclaim_domain_memory(sh->fdb_domain, 1);
}
sh->pop_vlan_action = mlx5_glue->dr_create_flow_action_pop_vlan();
+   if (!priv->config.allow_duplicate_pattern) {
+   mlx5_glue->dr_allow_duplicate_rules(sh->rx_domain, 0);
+   mlx5_glue->dr_allow_duplicate_rules(sh->tx_domain, 0);
+   if (sh->fdb_domain)
+   mlx5_glue->dr_allow_duplicate_rules(sh->fdb_domain, 0);
+   }
 #endif /* HAVE_MLX5DV_DR */
sh->default_miss_action =
mlx5_glue->dr_create_flow_action_default_miss();
@@ -2359,6 +2365,7 @@ mlx5_os_pci_probe_pf(struct rte_pci_device *pci_dev,
dev_config.dv_flow_en = 1;
dev_config.decap_en = 1;
dev_config.log_hp_size = MLX5_ARG_UNSET;
+   dev_config.allow_duplicate_pattern = 1;
list[i].eth_dev = mlx5_dev_spawn(&pci_dev->device,
 &list[i],
 &dev_config,
diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index cf1815cb74..ef15b115d8 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -175,6 +175,9 @@
 /* Decap will be used or not. */
 #define MLX5_DECAP_EN "decap_en"
 
+/* Device parameter to configure allow or prevent duplicate rules pattern. */
+#define MLX5_ALLOW_DUPLICATE_PATTERN "allow_duplicate_pattern"
+
 /* Shared memory between primary and secondary processes. */
 struct mlx5_shared_data *mlx5_shared_data;
 
@@ -1948,6 +1951,8 @@ mlx5_args_check(const char *key, const char *val, void 
*opaque)
config->sys_mem_en = !!tmp;
} else if (strcmp(MLX5_DECAP_EN, key) == 0) {
config->decap_en = !!tmp;
+   } else if (strcmp(MLX5_ALLOW_DUPLICATE_PATTERN, key) == 0) {
+   config->allow_duplicate_pattern = !!tmp;
} else {
DRV_LOG(WARNING, "%s: unknown parameter", key);
rte_errno = EINVAL;
@@ -2007,6 +2012,7 @@ mlx5_args(struct mlx5_dev_config *config, struct 
rte_devargs *devargs)
MLX5_RECLAIM_MEM,
MLX5_SYS_MEM_EN,
MLX5_DECAP_EN,
+   MLX5_ALLOW_DUPLICATE_PATTERN,
NULL,
};
struct rte_kvargs *kvlist;
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 32b2817bf2..63e7779d6f 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -244,6 +244,8 @@ struct mlx5_dev_config {
unsigned int sys_mem_en:1; /* The default memory allocator. */
unsigned int decap_en:1; /* Whether decap will be used or not. */
unsigned int dv_miss_info:1; /* restore pack

Re: [dpdk-dev] [PATCH v2 1/2] raw/ioat: add pci address handling to python script

[Bruce Richardson]

On Fri, May 28, 2021 at 02:19:01PM +0100, Kevin Laatz wrote:
> Currently the user needs to find the DSA instance number for any DSA device
> they would like to configure using this script, which can be cumbersome and
> error-prone since the instance numbering changes when changing the binding
> of the devices between vfio-pci and idxd.
> 
s/changes/may change/
I've found a number of times that after unbinding and rebinding a device the
number did not change.

> This patch improved the usability of the script by adding the ability to
s/improved/improves/

> specify the DSA device to configure using the device's PCI address instead
> of the DSA instance number. For example, "$dpdk_idxd_cfg.py 0" and
> "$dpdk_idxd_cfg.py 6a:01.0" are both valid references to the same device
> (assuming the numbering).
> 
> Signed-off-by: Kevin Laatz 
> 
With the above minor commit-log tweaks

Acked-by: Bruce Richardson 

> ---
> v2:
>- split the device reset change into its own patch
>- changed sysfs dir search
>- improved error case handling
> ---
>  drivers/raw/ioat/dpdk_idxd_cfg.py | 28 ++--
>  1 file changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/raw/ioat/dpdk_idxd_cfg.py 
> b/drivers/raw/ioat/dpdk_idxd_cfg.py
> index ff06d9e240..ad8393a645 100755
> --- a/drivers/raw/ioat/dpdk_idxd_cfg.py
> +++ b/drivers/raw/ioat/dpdk_idxd_cfg.py
> @@ -29,6 +29,26 @@ def write_values(self, values):
>  f.write(str(contents))
>  
>  
> +def get_pci_dir(pci):
> +"Search for the sysfs directory of the PCI device"
> +base_dir = '/sys/bus/pci/devices/'
> +for path, dirs, files in os.walk(base_dir):
> +for dir in dirs:
> +if pci in dir:
> +return os.path.join(base_dir, dir)
> +sys.exit(f"Could not find sysfs directory for device {pci}")
> +
> +
> +def get_dsa_id(pci):
> +"Get the DSA instance ID using the PCI address of the device"
> +pci_dir = get_pci_dir(pci)
> +for path, dirs, files in os.walk(pci_dir):
> +for dir in dirs:
> +if dir.startswith('dsa') and 'wq' not in dir:
> +return int(dir[3:])
> +sys.exit(f"Could not get device ID for device {pci}")
> +
> +
>  def configure_dsa(dsa_id, queues, prefix):
>  "Configure the DSA instance with appropriate number of queues"
>  dsa_dir = SysfsDir(f"/sys/bus/dsa/devices/dsa{dsa_id}")
> @@ -68,14 +88,18 @@ def main(args):
>  "Main function, does arg parsing and calls config function"
>  arg_p = argparse.ArgumentParser(
>  description="Configure whole DSA device instance for DPDK use")
> -arg_p.add_argument('dsa_id', type=int, help="DSA instance number")
> +arg_p.add_argument('dsa_id',
> +   help="Specify DSA instance either via DSA instance 
> number or PCI address")
>  arg_p.add_argument('-q', metavar='queues', type=int, default=255,
> help="Number of queues to set up")
>  arg_p.add_argument('--name-prefix', metavar='prefix', dest='prefix',
> default="dpdk",
> help="Prefix for workqueue name to mark for DPDK use 
> [default: 'dpdk']")
>  parsed_args = arg_p.parse_args(args[1:])
> -configure_dsa(parsed_args.dsa_id, parsed_args.q, parsed_args.prefix)
> +
> +dsa_id = parsed_args.dsa_id
> +dsa_id = get_dsa_id(dsa_id) if ':' in dsa_id else dsa_id
> +configure_dsa(dsa_id, parsed_args.q, parsed_args.prefix)
>  
>  
>  if __name__ == "__main__":
> -- 
> 2.30.2
> 

Re: [dpdk-dev] [PATCH v2 2/2] raw/ioat: add device reset to python script

[Bruce Richardson]

On Fri, May 28, 2021 at 02:19:02PM +0100, Kevin Laatz wrote:
> Currently once a device is configured, the user does not have the ability
> to reset the device via the script.
> 
> This patch adds a device reset option to the script. For example
> "$dpdk_idxd_cfg.py 0 --reset" would reset device 0.
> 
> Signed-off-by: Kevin Laatz 
> ---

I'd tend toward not having the prints for what the script is doing, and
have the script silent by default. However, with or without that tweak:

Acked-by: Bruce Richardson 

[dpdk-dev] [PATCH v3 0/2] extend idxd config script functionality

[Kevin Laatz]

This patchset extends the functionality of the idxd config script, the
additions are described in the individual patches.

v3:
   - minor commit log updates
   - remove printing if the script is running config or reset 

Kevin Laatz (2):
  raw/ioat: add pci address handling to python script
  raw/ioat: add device reset to python script

 drivers/raw/ioat/dpdk_idxd_cfg.py | 39 +--
 1 file changed, 37 insertions(+), 2 deletions(-)

-- 
2.30.2

[dpdk-dev] [PATCH v3 1/2] raw/ioat: add pci address handling to python script

[Kevin Laatz]

Currently the user needs to find the DSA instance number for any DSA device
they would like to configure using this script, which can be cumbersome and
error-prone since the instance numbering may change when changing the
binding of the devices between vfio-pci and idxd.

This patch improves the usability of the script by adding the ability to
specify the DSA device to configure using the device's PCI address instead
of the DSA instance number. For example, "$dpdk_idxd_cfg.py 0" and
"$dpdk_idxd_cfg.py 6a:01.0" are both valid references to the same device
(assuming the numbering).

Signed-off-by: Kevin Laatz 
Acked-by: Bruce Richardson 

---
v2:
   - split the device reset change into its own patch
   - changed sysfs dir search
   - improved error case handling

v3: minor commit log updates
---
 drivers/raw/ioat/dpdk_idxd_cfg.py | 28 ++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/raw/ioat/dpdk_idxd_cfg.py 
b/drivers/raw/ioat/dpdk_idxd_cfg.py
index ff06d9e240..ad8393a645 100755
--- a/drivers/raw/ioat/dpdk_idxd_cfg.py
+++ b/drivers/raw/ioat/dpdk_idxd_cfg.py
@@ -29,6 +29,26 @@ def write_values(self, values):
 f.write(str(contents))
 
 
+def get_pci_dir(pci):
+"Search for the sysfs directory of the PCI device"
+base_dir = '/sys/bus/pci/devices/'
+for path, dirs, files in os.walk(base_dir):
+for dir in dirs:
+if pci in dir:
+return os.path.join(base_dir, dir)
+sys.exit(f"Could not find sysfs directory for device {pci}")
+
+
+def get_dsa_id(pci):
+"Get the DSA instance ID using the PCI address of the device"
+pci_dir = get_pci_dir(pci)
+for path, dirs, files in os.walk(pci_dir):
+for dir in dirs:
+if dir.startswith('dsa') and 'wq' not in dir:
+return int(dir[3:])
+sys.exit(f"Could not get device ID for device {pci}")
+
+
 def configure_dsa(dsa_id, queues, prefix):
 "Configure the DSA instance with appropriate number of queues"
 dsa_dir = SysfsDir(f"/sys/bus/dsa/devices/dsa{dsa_id}")
@@ -68,14 +88,18 @@ def main(args):
 "Main function, does arg parsing and calls config function"
 arg_p = argparse.ArgumentParser(
 description="Configure whole DSA device instance for DPDK use")
-arg_p.add_argument('dsa_id', type=int, help="DSA instance number")
+arg_p.add_argument('dsa_id',
+   help="Specify DSA instance either via DSA instance 
number or PCI address")
 arg_p.add_argument('-q', metavar='queues', type=int, default=255,
help="Number of queues to set up")
 arg_p.add_argument('--name-prefix', metavar='prefix', dest='prefix',
default="dpdk",
help="Prefix for workqueue name to mark for DPDK use 
[default: 'dpdk']")
 parsed_args = arg_p.parse_args(args[1:])
-configure_dsa(parsed_args.dsa_id, parsed_args.q, parsed_args.prefix)
+
+dsa_id = parsed_args.dsa_id
+dsa_id = get_dsa_id(dsa_id) if ':' in dsa_id else dsa_id
+configure_dsa(dsa_id, parsed_args.q, parsed_args.prefix)
 
 
 if __name__ == "__main__":
-- 
2.30.2

[dpdk-dev] [PATCH v3 2/2] raw/ioat: add device reset to python script

[Kevin Laatz]

Currently once a device is configured, the user does not have the ability
to reset the device via the script.

This patch adds a device reset option to the script. For example
"$dpdk_idxd_cfg.py 0 --reset" would reset device 0.

Signed-off-by: Kevin Laatz 
Acked-by: Bruce Richardson 

---
v3: remove printing if the script running config or reset
---
 drivers/raw/ioat/dpdk_idxd_cfg.py | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/raw/ioat/dpdk_idxd_cfg.py 
b/drivers/raw/ioat/dpdk_idxd_cfg.py
index ad8393a645..83ef4817db 100755
--- a/drivers/raw/ioat/dpdk_idxd_cfg.py
+++ b/drivers/raw/ioat/dpdk_idxd_cfg.py
@@ -29,6 +29,12 @@ def write_values(self, values):
 f.write(str(contents))
 
 
+def reset_device(dsa_id):
+"Reset the DSA device and all its queues"
+drv_dir = SysfsDir("/sys/bus/dsa/drivers/dsa")
+drv_dir.write_values({"unbind": f"dsa{dsa_id}"})
+
+
 def get_pci_dir(pci):
 "Search for the sysfs directory of the PCI device"
 base_dir = '/sys/bus/pci/devices/'
@@ -95,11 +101,16 @@ def main(args):
 arg_p.add_argument('--name-prefix', metavar='prefix', dest='prefix',
default="dpdk",
help="Prefix for workqueue name to mark for DPDK use 
[default: 'dpdk']")
+arg_p.add_argument('--reset', action='store_true',
+   help="Reset DSA device and its queues")
 parsed_args = arg_p.parse_args(args[1:])
 
 dsa_id = parsed_args.dsa_id
 dsa_id = get_dsa_id(dsa_id) if ':' in dsa_id else dsa_id
-configure_dsa(dsa_id, parsed_args.q, parsed_args.prefix)
+if parsed_args.reset:
+reset_device(dsa_id)
+else:
+configure_dsa(dsa_id, parsed_args.q, parsed_args.prefix)
 
 
 if __name__ == "__main__":
-- 
2.30.2

Re: [dpdk-dev] [PATCH] net: introduce IPv4 ihl and version fields

[Gregory Etelson]

> > > > RTE IPv4 header definition combines the `version' and `ihl'
> > > > fields into a single structure member.
> > > > This patch introduces dedicated structure members for both
> > `version'
> > > > and `ihl' IPv4 fields. Separated header fields definitions allow
> > > > to create simplified code to match on the IHL value in a flow rule.
> > > > The original `version_ihl' structure member is kept for backward
> > > > compatibility.
> > > >
> > > > Signed-off-by: Gregory Etelson 
> > > > ---
> > > >  app/test/test_flow_classify.c |  8 
> > > >  lib/net/rte_ip.h  | 16 +++-
> > > >  2 files changed, 19 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/app/test/test_flow_classify.c
> > > > b/app/test/test_flow_classify.c index 951606f248..4f64be5357
> > > > 100644
> > > > --- a/app/test/test_flow_classify.c
> > > > +++ b/app/test/test_flow_classify.c
> > > > @@ -95,7 +95,7 @@ static struct rte_acl_field_def
> > > > ipv4_defs[NUM_FIELDS_IPV4] = {
> > > >   *  dst mask 255.255.255.00 / udp src is 32 dst is 33 / end"
> > > >   */
> > > >  static struct rte_flow_item_ipv4 ipv4_udp_spec_1 = {
> > > > - { 0, 0, 0, 0, 0, 0, IPPROTO_UDP, 0,
> > > > + { { .version_ihl = 0}, 0, 0, 0, 0, 0, IPPROTO_UDP, 0,
> > > > RTE_IPV4(2, 2, 2, 3), RTE_IPV4(2, 2, 2, 7)}  };  static const
> > > > struct rte_flow_item_ipv4 ipv4_mask_24 = { @@ -131,7 +131,7 @@
> > > > static struct rte_flow_item  end_item = {
> RTE_FLOW_ITEM_TYPE_END,
> > > >   *  dst mask 255.255.255.00 / tcp src is 16 dst is 17 / end"
> > > >   */
> > > >  static struct rte_flow_item_ipv4 ipv4_tcp_spec_1 = {
> > > > - { 0, 0, 0, 0, 0, 0, IPPROTO_TCP, 0,
> > > > + { { .version_ihl = 0}, 0, 0, 0, 0, 0, IPPROTO_TCP, 0,
> > > > RTE_IPV4(1, 2, 3, 4), RTE_IPV4(5, 6, 7, 8)}  };
> > > >
> > > > @@ -150,8 +150,8 @@ static struct rte_flow_item  tcp_item_1 = {
> > > > RTE_FLOW_ITEM_TYPE_TCP,
> > > >   *  dst mask 255.255.255.00 / sctp src is 16 dst is 17/ end"
> > > >   */
> > > >  static struct rte_flow_item_ipv4 ipv4_sctp_spec_1 = {
> > > > - { 0, 0, 0, 0, 0, 0, IPPROTO_SCTP, 0, RTE_IPV4(11, 12, 13, 14),
> > > > - RTE_IPV4(15, 16, 17, 18)}
> > > > + { { .version_ihl = 0}, 0, 0, 0, 0, 0, IPPROTO_SCTP, 0,
> > > > + RTE_IPV4(11, 12, 13, 14), RTE_IPV4(15, 16, 17, 18)}
> > > >  };
> > > >
> > > >  static struct rte_flow_item_sctp sctp_spec_1 = { diff --git
> > > > a/lib/net/rte_ip.h b/lib/net/rte_ip.h index 4b728969c1..684bb028b2
> > > > 100644
> > > > --- a/lib/net/rte_ip.h
> > > > +++ b/lib/net/rte_ip.h
> > > > @@ -38,7 +38,21 @@ extern "C" {
> > > >   * IPv4 Header
> > > >   */
> > > >  struct rte_ipv4_hdr {
> > > > - uint8_t  version_ihl;   /**< version and header length */
> > > > + __extension__
> > > > + union {
> > > > + uint8_t version_ihl;/**< version and header length */
> > > > + struct {
> > > > +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> > > > + uint8_t ihl:4;
> > > > + uint8_t version:4; #elif RTE_BYTE_ORDER ==
> > > > +RTE_BIG_ENDIAN
> > > > + uint8_t version:4;
> > > > + uint8_t ihl:4;
> > > > +#else
> > > > +#error "setup endian definition"
> > > > +#endif
> > > > + };
> > > > + };
> > > >   uint8_t  type_of_service;   /**< type of service */
> > > >   rte_be16_t total_length;/**< length of packet */
> > > >   rte_be16_t packet_id;   /**< packet ID */
> > > > --
> > > > 2.31.1
> > > >
> > >
> > > This does not break the ABI, but it could be discussed if it breaks
> > the API due to the required structure initialization changes shown in
> > > test_flow_classify.c.
> >
> > Yep, I guess it might be classified as API change.
> > Another thing that concerns me - it is not the only place in IPv4
> > header when we unite multiple bit-fields into one field:
> > type_of_service, fragment_offset.
> > If we start splitting ipv4 fields into actual bitfields, I suppose
> > we'll end-up splitting these ones too.
> > But I am not sure it will pay off - as compiler not always generates
> > optimal code for reading/updating bitfields.
> > Did you consider just adding extra macros to simplify access to these
> > fields (like RTE_IPV4_HDR_(GET_SET)_*), instead?
> >
> 
> Let's please not introduce accessor macros for bitfields. If we don't
> introduce bitfields like these, I would rather stick with the current _MASK,
> _SHIFT and _FLAG defines.
> 
> Yes, this change will lead to the introduction of more bitfields, both here
> and in other places. We already accepted it in the eCPRI structure
> (/lib/net/rte_ecpri.h), so why not just generally accept it.
> 
> Are modern compilers really worse at handling a bitfield defined like this,
> compared to handling a single uint8_t with hand coding? I consider your
> concern very important, so I'm only asking if it is still relevant, to avoid
> making decisions based on past experience that might be outdated. (I admit
> to falling into that trap myself, once in a while.)
> 

Re: [dpdk-dev] [PATCH 3/3] eal/windows: cleanup interrupt resources

[Jie Zhou]

On Sun, May 02, 2021 at 05:33:33AM +0300, Dmitry Kozlyuk wrote:
> Interrupt manager in Windows EAL allocates on IOCP and starts
> a control thread that runs indefinitely. At DPDK cleanup
> this thread was not stopped and IOCP handle was not closed.
> 
> Gracefully stop interrupt-handling in rte_eal_cleanup().
> The thread already closes IOCP handle before exiting.
> 
> Fixes: 5c016fc0205a ("eal/windows: add interrupt thread skeleton")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Dmitry Kozlyuk 

Acked-by: Jie Zhou 
Tested-by: Jie Zhou 

> ---
>  lib/eal/windows/eal.c|  1 +
>  lib/eal/windows/eal_interrupts.c | 26 --
>  lib/eal/windows/eal_windows.h|  5 +
>  3 files changed, 30 insertions(+), 2 deletions(-)

Enabled a subset of Unit test on Windows and when running alarm_autotest, 
system hang at rte_eal_alarm_set. After applying this patch set, no repro any 
more. Also system hang at pmd_perf_autotest and no repro with the patch. It is 
with Intel i40e.
 

[dpdk-dev] [PATCH] doc: fix default burst size

[Ajit Khaparde]

Default burst size in testpmd has been changed from 16 to 32
for some time now. But the documentation had not been updated.

Fixes: 836853d3d4cf7 ("app/testpmd: increase default burst size to 32")
Cc: sta...@dpdk.org
Cc: bruce.richard...@intel.com
Cc: xiaoyun...@intel.com
Cc: ferruh.yi...@intel.com
Cc: andrew.rybche...@oktetlabs.ru

Signed-off-by: Ajit Khaparde 
---
 doc/guides/prog_guide/writing_efficient_code.rst | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/doc/guides/prog_guide/writing_efficient_code.rst 
b/doc/guides/prog_guide/writing_efficient_code.rst
index 7baeaae431..a61e8320ae 100644
--- a/doc/guides/prog_guide/writing_efficient_code.rst
+++ b/doc/guides/prog_guide/writing_efficient_code.rst
@@ -143,20 +143,21 @@ In order to achieve higher throughput,
 the DPDK attempts to aggregate the cost of processing each packet individually 
by processing packets in bursts.
 
 Using the testpmd application as an example,
-the burst size can be set on the command line to a value of 16 (also the 
default value).
-This allows the application to request 16 packets at a time from the PMD.
+the burst size can be set on the command line to a value of 32 (also the 
default value).
+This allows the application to request 32 packets at a time from the PMD.
 The testpmd application then immediately attempts to transmit all the packets 
that were received,
-in this case, all 16 packets.
+in this case, all 32 packets.
 
 The packets are not transmitted until the tail pointer is updated on the 
corresponding TX queue of the network port.
 This behavior is desirable when tuning for high throughput because
-the cost of tail pointer updates to both the RX and TX queues can be spread 
across 16 packets,
+the cost of tail pointer updates to both the RX and TX queues can be spread
+across 32 packets,
 effectively hiding the relatively slow MMIO cost of writing to the PCIe* 
device.
 However, this is not very desirable when tuning for low latency because
-the first packet that was received must also wait for another 15 packets to be 
received.
-It cannot be transmitted until the other 15 packets have also been processed 
because
+the first packet that was received must also wait for another 31 packets to be 
received.
+It cannot be transmitted until the other 31 packets have also been processed 
because
 the NIC will not know to transmit the packets until the TX tail pointer has 
been updated,
-which is not done until all 16 packets have been processed for transmission.
+which is not done until all 32 packets have been processed for transmission.
 
 To consistently achieve low latency, even under heavy system load,
 the application developer should avoid processing packets in bunches.
-- 
2.21.1 (Apple Git-122.3)

[dpdk-dev] [PATCH] net/iavf: enable on Windows

[Pallavi Kadam]

This patch enables building the iAVF PMD on Windows.
- Replace x86intrin.h with rte_vect.h to avoid __m_prefetchw conflicting
  types
- Fix for pointer and integer sign warnings using Clang compiler on
  Windows
- Add extra cflags '-fno-asynchronous-unwind-tables'
  to avoid MinGW build error:
  Error: invalid register for .seh_savexmm
- Update release notes

Signed-off-by: Pallavi Kadam 
Reviewed-by: Ranjit Menon 
---
 doc/guides/rel_notes/release_21_08.rst  |  4 
 drivers/net/iavf/iavf.h |  3 ++-
 drivers/net/iavf/iavf_rxtx_vec_avx2.c   |  2 +-
 drivers/net/iavf/iavf_rxtx_vec_avx512.c |  2 +-
 drivers/net/iavf/meson.build| 10 --
 5 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/doc/guides/rel_notes/release_21_08.rst 
b/doc/guides/rel_notes/release_21_08.rst
index a6ecfdf3c..931f5c322 100644
--- a/doc/guides/rel_notes/release_21_08.rst
+++ b/doc/guides/rel_notes/release_21_08.rst
@@ -55,6 +55,10 @@ New Features
  Also, make sure to start the actual text at the margin.
  ===
 
+* **Updated Intel iavf driver.**
+
+  * Added Intel iavf support on Windows.
+
 
 Removed Items
 -
diff --git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h
index 4f5811ae8..9b46608b6 100644
--- a/drivers/net/iavf/iavf.h
+++ b/drivers/net/iavf/iavf.h
@@ -286,7 +286,8 @@ _clear_cmd(struct iavf_info *vf)
 static inline int
 _atomic_set_cmd(struct iavf_info *vf, enum virtchnl_ops ops)
 {
-   int ret = rte_atomic32_cmpset(&vf->pend_cmd, VIRTCHNL_OP_UNKNOWN, ops);
+   int ret = rte_atomic32_cmpset((volatile uint32_t *)&vf->pend_cmd,
+   VIRTCHNL_OP_UNKNOWN, ops);
 
if (!ret)
PMD_DRV_LOG(ERR, "There is incomplete cmd %d", vf->pend_cmd);
diff --git a/drivers/net/iavf/iavf_rxtx_vec_avx2.c 
b/drivers/net/iavf/iavf_rxtx_vec_avx2.c
index f5646d645..60ff3e356 100644
--- a/drivers/net/iavf/iavf_rxtx_vec_avx2.c
+++ b/drivers/net/iavf/iavf_rxtx_vec_avx2.c
@@ -4,7 +4,7 @@
 
 #include "iavf_rxtx_vec_common.h"
 
-#include 
+#include 
 
 #ifndef __INTEL_COMPILER
 #pragma GCC diagnostic ignored "-Wcast-qual"
diff --git a/drivers/net/iavf/iavf_rxtx_vec_avx512.c 
b/drivers/net/iavf/iavf_rxtx_vec_avx512.c
index d99de2a8b..8669a71ba 100644
--- a/drivers/net/iavf/iavf_rxtx_vec_avx512.c
+++ b/drivers/net/iavf/iavf_rxtx_vec_avx512.c
@@ -4,7 +4,7 @@
 
 #include "iavf_rxtx_vec_common.h"
 
-#include 
+#include 
 
 #ifndef __INTEL_COMPILER
 #pragma GCC diagnostic ignored "-Wcast-qual"
diff --git a/drivers/net/iavf/meson.build b/drivers/net/iavf/meson.build
index 6f222a9e8..a6baa77ce 100644
--- a/drivers/net/iavf/meson.build
+++ b/drivers/net/iavf/meson.build
@@ -1,12 +1,6 @@
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2018 Luca Boccassi 
 
-if is_windows
-build = false
-reason = 'not supported on Windows'
-subdir_done()
-endif
-
 cflags += ['-Wno-strict-aliasing']
 
 includes += include_directories('../../common/iavf')
@@ -24,6 +18,10 @@ sources = files(
 if arch_subdir == 'x86'
 sources += files('iavf_rxtx_vec_sse.c')
 
+if is_windows and cc.get_id() != 'clang'
+cflags += ['-fno-asynchronous-unwind-tables']
+endif
+
 # compile AVX2 version if either:
 # a. we have AVX supported in minimum instruction set baseline
 # b. it's not minimum instruction set, but supported by compiler
-- 
2.18.0.windows.1