Re: [dpdk-dev] [PATCH v5 3/3] PCI: don't use vfio ioctl call to access PIO resource

[Maxime Coquelin]

On 1/20/21 3:54 PM, 谢华伟(此时此刻） wrote:
> 
> On 2021/1/13 0:58, Maxime Coquelin wrote:
>>
>> On 1/12/21 10:37 AM, Maxime Coquelin wrote:
>>> bus/pci: ...
>>>
>>> On 10/22/20 5:51 PM, 谢华伟(此时此刻） wrote:
 From: "huawei.xhw" 

 VFIO should use the same way to map/read/write PORT IO as UIO, for
 virtio PMD.
>>> Please provide more details in the commit message on why the way VFIO
>>> works today is wrong (The cover letter is lost once applied).
> ok
>>>
 Signed-off-by: huawei.xhw 
>>> Same comment about name format as on previous patches.
>>>
 ---
   drivers/bus/pci/linux/pci.c | 8 
   drivers/bus/pci/linux/pci_uio.c | 4 +++-
   2 files changed, 7 insertions(+), 5 deletions(-)

 diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
 index 0dc99e9..2ed9f2b 100644
 --- a/drivers/bus/pci/linux/pci.c
 +++ b/drivers/bus/pci/linux/pci.c
 @@ -687,7 +687,7 @@ int rte_pci_write_config(const struct
 rte_pci_device *device,
   #ifdef VFIO_PRESENT
   case RTE_PCI_KDRV_VFIO:
   if (pci_vfio_is_enabled())
 -    ret = pci_vfio_ioport_map(dev, bar, p);
 +    ret = pci_uio_ioport_map(dev, bar, p);
>>> Doesn't it create a regression with regards to needed capabilities?
>>> My understanding is that before this patch we don't need to call iopl(),
>>> whereas once applied it is required, correct?
>> I did some testing today, and think it is not a regression with para-
>> virtualized Virtio devices.
>>
>> Indeed, I thought it would be a regression with Legacy devices when
>> IOMMU is enabled and the program is run as non-root (IOMMU enabled
>> just to suport IOVA as VA mode). But it turns out para-virtualized
>> Virtio legacy device and vIOMMU enabled is not a supported configuration
>> by QEMU.
>>
>> Note that when noiommu mode is enabled, the app needs cap_sys_rawio, so
>> same as iopl(). No regression in this case too.
>>
>> That said, with real (non para-virtualized) Virtio device using PIO like
>> yours, doesn't your patch introduce a restriction for your device that
>> it will require cap_sys_rawio whereas it would not be needed?
> 
> I don't catch the regression issue.
> 
> With real virtio device(hardware implemented), if it is using MMIO, no
> cap_sys_rawio is required.
> 
> If it is using PIO, iopl is required always.

My understanding of the Kernel VFIO driver is that cap_sys_rawio is only
necessary in noiommu mode, i.e. when VFIO is loaded with
enable_unsafe_noiommu parameter set. The doc for this parameters seems
to validate my understanding of the code:
"
MODULE_PARM_DESC(enable_unsafe_noiommu_mode, "Enable UNSAFE, no-IOMMU
mode.  This mode provides no device isolation, no DMA translation, no
host kernel protection, cannot be used for device assignment to virtual
machines, requires RAWIO permissions, and will taint the kernel.  If you
do not know what this is for, step away. (default: false)");
"

I think that using inb/outb in the case of VFIO with IOMMU enabled won't
work without cap_sys_rawio, and using it in the case of VFIO with IOMMU
disabled just bypasses VFIO and so is not correct.

In my opinion, what we should do is to add something like this in the
DPDK documentation:

 - MMIO BAR: VFIO with IOMMU enabled recommended. Equivalent performance
as with IGB UIO or VFIO with NOIOMMU. VFIO with IOMMU is recommended for
security reasons.
 - PIO BAR: VFIO with IOMMU enabled is recommended for security reasons,
providing proper isolation and not requiring cap_sys_rawio. However, use
of IOMMU is not always possible in some cases (e.g. para-virtualized
Virtio-net legacy device). Also, performance of using VFIO for PIO BARs
accesses has an impact on performance as it uses pread/pwrite syscalls,
whereas UIO drivers use inb/outb. If security is not a concern or IOMMU
is not available, one might consider using UIO driver in this case for
performance reasons.

What do you think?

Thanks,
Maxime

> 
>> Thanks,
>> Maxime
>>
>>> Regards,
>>> Maxime
>>>
> 

Re: [dpdk-dev] [PATCH v2 08/19] vhost: fix missing header includes

[Maxime Coquelin]

On 1/15/21 12:10 PM, Bruce Richardson wrote:
> The vhost header files were missing definitions from headers to allow
> them to be compiled up individually.
> 
> Fixes: d7280c9fffcb ("vhost: support selective datapath")
> Fixes: a49f758d1170 ("vhost: split vDPA header file")
> Fixes: 939066d96563 ("vhost/crypto: add public function implementation")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Bruce Richardson 
> ---
>  lib/librte_vhost/rte_vdpa.h | 2 ++
>  lib/librte_vhost/rte_vdpa_dev.h | 1 +
>  lib/librte_vhost/rte_vhost_crypto.h | 7 +++
>  3 files changed, 10 insertions(+)
> 
> diff --git a/lib/librte_vhost/rte_vdpa.h b/lib/librte_vhost/rte_vdpa.h
> index f074ec0c4a..1437f400bf 100644
> --- a/lib/librte_vhost/rte_vdpa.h
> +++ b/lib/librte_vhost/rte_vdpa.h
> @@ -11,6 +11,8 @@
>   * Device specific vhost lib
>   */
>  
> +#include 
> +
>  /** Maximum name length for statistics counters */
>  #define RTE_VDPA_STATS_NAME_SIZE 64
>  
> diff --git a/lib/librte_vhost/rte_vdpa_dev.h b/lib/librte_vhost/rte_vdpa_dev.h
> index a60183f780..bfada387b0 100644
> --- a/lib/librte_vhost/rte_vdpa_dev.h
> +++ b/lib/librte_vhost/rte_vdpa_dev.h
> @@ -8,6 +8,7 @@
>  #include 
>  
>  #include "rte_vhost.h"
> +#include "rte_vdpa.h"
>  
>  #define RTE_VHOST_QUEUE_ALL UINT16_MAX
>  
> diff --git a/lib/librte_vhost/rte_vhost_crypto.h 
> b/lib/librte_vhost/rte_vhost_crypto.h
> index c809c46a21..2a27a35892 100644
> --- a/lib/librte_vhost/rte_vhost_crypto.h
> +++ b/lib/librte_vhost/rte_vhost_crypto.h
> @@ -5,6 +5,13 @@
>  #ifndef _VHOST_CRYPTO_H_
>  #define _VHOST_CRYPTO_H_
>  
> +#include 
> +#include 
> +
> +/* pre-declare structs to avoid including full headers */
> +struct rte_mempool;
> +struct rte_crypto_op;
> +
>  #define VHOST_CRYPTO_MBUF_POOL_SIZE  (8192)
>  #define VHOST_CRYPTO_MAX_BURST_SIZE  (64)
>  #define VHOST_CRYPTO_MAX_DATA_SIZE   (4096)
> 

Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [dpdk-dev] [PATCH 1/1] doc: simplify OS support in features matrix

[Andrew Rybchenko]

On 1/21/21 2:03 AM, Thomas Monjalon wrote:
> The networking drivers features matrix had rows to show
> OS and kernel modules support:
>   - BSD nic_uio
>   - Linux UIO
>   - Linux VFIO
>   - Other kdrv
>   - Windows
> 
> The kernel modules details are removed to keep only OS support:
>   - FreeBSD
>   - Linux
>   - Windows
> 
> Signed-off-by: Thomas Monjalon 

Acked-by: Andrew Rybchenko 

Re: [dpdk-dev] [PATCH v5 0/3] support both PIO and MMIO BAR for virtio PMD

[Maxime Coquelin]

On 1/21/21 5:12 AM, 谢华伟(此时此刻） wrote:
> 
> On 2021/1/13 1:37, Maxime Coquelin wrote:
>>
>> On 10/22/20 5:51 PM, 谢华伟(此时此刻） wrote:
>>> From: "huawei.xhw" 
>>>
>>> Legacy virtio-pci only supports PIO BAR resource. As we need to
>>> create lots of
>>> virtio devices and PIO resource on x86 is very limited, we expose
>>> MMIO BAR.
>>>
>>> Kernel supports both PIO  and MMIO BAR for legacy virtio-pci device.
>>> We handles
>>> different type of BAR in the similar way.
>>>
>>> In previous implementation, with igb_uio we get PIO address from igb_uio
>>> sysfs entry; with uio_pci_generic, we get PIO address from
>>> /proc/ioports.
>>> For PIO/MMIO RW, there is different path for different drivers and arch.
>>> For VFIO, PIO/MMIO RW is through syscall, which has big performance
>>> issue.
>> Regarding the performance issue, do you have some numbers to share?
>> AFAICS, it can only have an impact on performance when interrupt mode is
>> used or queue notification is enabled.
>>
>> Does your HW Virtio implementation requires notification?
> 
> Yes, hardware needs notification to tell which queue has more buffer.
> 
> vhost backend also needs notification when it is not running in polling
> mode.
> 
> It is easy for software backend to sync with frontend whether it needs
> notification through memory but a big burden for hardware.

Yes, I understand, thanks for the clarification.

> Anyway, using vfio ioctl isn't needed at all. virtio PMD is only the
> consumer of pci_vfio_ioport_read.

My understanding is that using VFIO read/write ops is required for IOMMU
enabled case without cap_sys_rawio. And anyway, using inb/outb is just
bypassing VFIO. As I suggest in my other reply, it is better to document
that in the case of devices having PIO BARs, the user should consider
using UIO driver if performance is a concern.

> we could consider if we still need pci_vfio_ioport_read related API in
> future.

I disagree. I think the pci_vfio_ioport_* API is required at least for
the IOMMU enabled case.

Documentation is the way to go in my opinion, we can also add a warning
that performance may be degraded compared to UIO in
pci_vfio_ioport_map() when IOMMU is disabled if you think it may help
the users.

Thanks,
Maxime

> /huawei
>>
>> Is performance the only issue to have your HW working with Virtio PMD,
>> or is this series also fixing some functionnal issues?
>>
>> Best regards,
>> Maxime
>>
> 

Re: [dpdk-dev] [PATCH v2 25/44] net/virtio: make Vhost-user req sender consistent

[Xia, Chenbo]

> -Original Message-
> From: Maxime Coquelin 
> Sent: Wednesday, January 20, 2021 5:25 AM
> To: dev@dpdk.org; Xia, Chenbo ; olivier.m...@6wind.com;
> amore...@redhat.com; david.march...@redhat.com
> Cc: Maxime Coquelin 
> Subject: [PATCH v2 25/44] net/virtio: make Vhost-user req sender consistent
> 
> This patch makes vhost_user_write() consistent with
> vhost_user_read(), by passing a Vhost-user message
> pointer instead of a buffer pointer and its length, which
> is now calculated in the function.
> 
> Signed-off-by: Maxime Coquelin 
> ---
>  drivers/net/virtio/virtio_user/vhost_user.c | 10 --
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_user/vhost_user.c
> b/drivers/net/virtio/virtio_user/vhost_user.c
> index 350eed4182..55c81a 100644
> --- a/drivers/net/virtio/virtio_user/vhost_user.c
> +++ b/drivers/net/virtio/virtio_user/vhost_user.c
> @@ -51,7 +51,7 @@ struct vhost_user_msg {
>   (sizeof(struct vhost_user_msg) - VHOST_USER_HDR_SIZE)
> 
>  static int
> -vhost_user_write(int fd, void *buf, int len, int *fds, int fd_num)
> +vhost_user_write(int fd, struct vhost_user_msg *msg, int *fds, int fd_num)
>  {
>   int r;
>   struct msghdr msgh;
> @@ -63,8 +63,8 @@ vhost_user_write(int fd, void *buf, int len, int *fds, int
> fd_num)
>   memset(&msgh, 0, sizeof(msgh));
>   memset(control, 0, sizeof(control));
> 
> - iov.iov_base = (uint8_t *)buf;
> - iov.iov_len = len;
> + iov.iov_base = (uint8_t *)msg;
> + iov.iov_len = VHOST_USER_HDR_SIZE + msg->size;
> 
>   msgh.msg_iov = &iov;
>   msgh.msg_iovlen = 1;
> @@ -259,7 +259,6 @@ vhost_user_sock(struct virtio_user_dev *dev,
>   int has_reply_ack = 0;
>   int fds[VHOST_MEMORY_MAX_NREGIONS];
>   int fd_num = 0;
> - int len;
>   int vhostfd = dev->vhostfd;
> 
>   RTE_SET_USED(m);
> @@ -364,8 +363,7 @@ vhost_user_sock(struct virtio_user_dev *dev,
>   return -1;
>   }
> 
> - len = VHOST_USER_HDR_SIZE + msg.size;
> - if (vhost_user_write(vhostfd, &msg, len, fds, fd_num) < 0) {
> + if (vhost_user_write(vhostfd, &msg, fds, fd_num) < 0) {
>   PMD_DRV_LOG(ERR, "%s failed: %s",
>   vhost_msg_strings[req], strerror(errno));
>   return -1;
> --
> 2.29.2

Reviewed-by: Chenbo Xia 

Re: [dpdk-dev] [PATCH v2 36/44] net/virtio: move Vhost-user reqs to Vhost-user backend

[Xia, Chenbo]

Hi Maxime,

> -Original Message-
> From: Maxime Coquelin 
> Sent: Wednesday, January 20, 2021 5:25 AM
> To: dev@dpdk.org; Xia, Chenbo ; olivier.m...@6wind.com;
> amore...@redhat.com; david.march...@redhat.com
> Cc: Maxime Coquelin 
> Subject: [PATCH v2 36/44] net/virtio: move Vhost-user reqs to Vhost-user

Better use full name 'requests' if the title's not too long?

Same thing could be done for 'net/virtio: make Vhost-user req sender consistent'

With this fixed:

Reviewed-by: Chenbo Xia 

> backend
> 
> Now that we have a proper isolation of the backends,
> we can move Vhost-user requests declaration to the
> Vhost-user backend file.
> 
> Signed-off-by: Maxime Coquelin 
> ---
>  drivers/net/virtio/virtio_user/vhost.h  | 25 -
>  drivers/net/virtio/virtio_user/vhost_user.c | 25 +
>  2 files changed, 25 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_user/vhost.h
> b/drivers/net/virtio/virtio_user/vhost.h
> index 6294b8afee..2aa6b2cb70 100644
> --- a/drivers/net/virtio/virtio_user/vhost.h
> +++ b/drivers/net/virtio/virtio_user/vhost.h
> @@ -63,31 +63,6 @@ struct vhost_vring_addr {
>  #define VHOST_USER_PROTOCOL_F_STATUS 16
>  #endif
> 
> -enum vhost_user_request {
> - VHOST_USER_NONE = 0,
> - VHOST_USER_GET_FEATURES = 1,
> - VHOST_USER_SET_FEATURES = 2,
> - VHOST_USER_SET_OWNER = 3,
> - VHOST_USER_RESET_OWNER = 4,
> - VHOST_USER_SET_MEM_TABLE = 5,
> - VHOST_USER_SET_LOG_BASE = 6,
> - VHOST_USER_SET_LOG_FD = 7,
> - VHOST_USER_SET_VRING_NUM = 8,
> - VHOST_USER_SET_VRING_ADDR = 9,
> - VHOST_USER_SET_VRING_BASE = 10,
> - VHOST_USER_GET_VRING_BASE = 11,
> - VHOST_USER_SET_VRING_KICK = 12,
> - VHOST_USER_SET_VRING_CALL = 13,
> - VHOST_USER_SET_VRING_ERR = 14,
> - VHOST_USER_GET_PROTOCOL_FEATURES = 15,
> - VHOST_USER_SET_PROTOCOL_FEATURES = 16,
> - VHOST_USER_GET_QUEUE_NUM = 17,
> - VHOST_USER_SET_VRING_ENABLE = 18,
> - VHOST_USER_SET_STATUS = 39,
> - VHOST_USER_GET_STATUS = 40,
> - VHOST_USER_MAX
> -};
> -
>  #ifndef VHOST_BACKEND_F_IOTLB_MSG_V2
>  #define VHOST_BACKEND_F_IOTLB_MSG_V2 1
>  #endif
> diff --git a/drivers/net/virtio/virtio_user/vhost_user.c
> b/drivers/net/virtio/virtio_user/vhost_user.c
> index d46e25b64b..fb6fcc82c9 100644
> --- a/drivers/net/virtio/virtio_user/vhost_user.c
> +++ b/drivers/net/virtio/virtio_user/vhost_user.c
> @@ -27,6 +27,31 @@ struct vhost_memory {
>   struct vhost_memory_region regions[VHOST_MEMORY_MAX_NREGIONS];
>  };
> 
> +enum vhost_user_request {
> + VHOST_USER_NONE = 0,
> + VHOST_USER_GET_FEATURES = 1,
> + VHOST_USER_SET_FEATURES = 2,
> + VHOST_USER_SET_OWNER = 3,
> + VHOST_USER_RESET_OWNER = 4,
> + VHOST_USER_SET_MEM_TABLE = 5,
> + VHOST_USER_SET_LOG_BASE = 6,
> + VHOST_USER_SET_LOG_FD = 7,
> + VHOST_USER_SET_VRING_NUM = 8,
> + VHOST_USER_SET_VRING_ADDR = 9,
> + VHOST_USER_SET_VRING_BASE = 10,
> + VHOST_USER_GET_VRING_BASE = 11,
> + VHOST_USER_SET_VRING_KICK = 12,
> + VHOST_USER_SET_VRING_CALL = 13,
> + VHOST_USER_SET_VRING_ERR = 14,
> + VHOST_USER_GET_PROTOCOL_FEATURES = 15,
> + VHOST_USER_SET_PROTOCOL_FEATURES = 16,
> + VHOST_USER_GET_QUEUE_NUM = 17,
> + VHOST_USER_SET_VRING_ENABLE = 18,
> + VHOST_USER_SET_STATUS = 39,
> + VHOST_USER_GET_STATUS = 40,
> + VHOST_USER_MAX
> +};
> +
>  struct vhost_user_msg {
>   enum vhost_user_request request;
> 
> --
> 2.29.2

Re: [dpdk-dev] [PATCH v2 01/44] bus/vdev: add helper to get vdev from eth dev

[Xia, Chenbo]

Hi Maxime,

> -Original Message-
> From: Maxime Coquelin 
> Sent: Wednesday, January 20, 2021 5:24 AM
> To: dev@dpdk.org; Xia, Chenbo ; olivier.m...@6wind.com;
> amore...@redhat.com; david.march...@redhat.com
> Cc: Maxime Coquelin 
> Subject: [PATCH v2 01/44] bus/vdev: add helper to get vdev from eth dev

Sorry that I missed the title in v1..

Better use 'ethdev' or 'eth device'here?

Thanks,
Chenbo

> 
> This patch adds an helper macro to get the rte_vdev_device
> pointer from a rte_eth_dev pointer.
> 
> This is similar to RTE_ETH_DEV_TO_PCI().
> 
> Signed-off-by: Maxime Coquelin 
> Reviewed-by: Chenbo Xia 
> Reviewed-by: David Marchand 
> ---
>  drivers/bus/vdev/rte_bus_vdev.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/bus/vdev/rte_bus_vdev.h b/drivers/bus/vdev/rte_bus_vdev.h
> index d14eeb41b0..f99a41f825 100644
> --- a/drivers/bus/vdev/rte_bus_vdev.h
> +++ b/drivers/bus/vdev/rte_bus_vdev.h
> @@ -34,6 +34,8 @@ struct rte_vdev_device {
>  #define RTE_DEV_TO_VDEV_CONST(ptr) \
>   container_of(ptr, const struct rte_vdev_device, device)
> 
> +#define RTE_ETH_DEV_TO_VDEV(eth_dev) RTE_DEV_TO_VDEV((eth_dev)->device)
> +
>  static inline const char *
>  rte_vdev_device_name(const struct rte_vdev_device *dev)
>  {
> --
> 2.29.2

Re: [dpdk-dev] [dpdk-stable] [PATCH v4] mbuf: fix reset on mbuf free

[Ferruh Yigit]

On 1/19/2021 2:21 PM, Morten Brørup wrote:

From: Ferruh Yigit [mailto:ferruh.yi...@intel.com]
Sent: Tuesday, January 19, 2021 3:03 PM

On 1/19/2021 12:27 PM, Morten Brørup wrote:

From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Ferruh Yigit
Sent: Tuesday, January 19, 2021 1:01 PM

On 1/19/2021 8:53 AM, Morten Brørup wrote:

Could someone at Intel please update the test script to provide

output according to the test plan? Or delegate to the right person.


According to the test plan, the information requested by Olivier

should be in the test output already:





http://git.dpdk.org/tools/dts/tree/test_plans/nic_single_core_perf_test

_plan.rst?h=next


PS: I can't find out who is the maintainer of the test plan, so I'm

randomly pointing my finger at the test plan doc copyright holder.

:-)




Hi Morten,

Ali has a request to update the expected baseline, to be able to

detect

the
performance drops, let me internally figure out who can do this.

And do you have any other request, or asking same thing?



Hi Ferruh,

I am asking for something else:

The test script does not provide the output that its documentation

says that it does.


Apparently, the test script for nic_single_core_perf produces an

output table with these four columns (as seen at
https://lab.dpdk.org/results/dashboard/patchsets/15142/#env-18):


 +++---+--

+

 | Result | frame_size (bytes) | txd/rxd (descriptors) |

throughput Difference (Mpps) |

 +++---+--

+

 | PASS   | 64 | 512   | 1.57100

|

 +++---+--

+

 | PASS   | 64 | 2048  | 1.87500

|

 +++---+--

+


But the test plan documentation (at

http://git.dpdk.org/tools/dts/tree/test_plans/nic_single_core_perf_test
_plan.rst) says that this output should be produced:


 ++-+-+-+-

+

 | Frame Size | TXD/RXD |  Throughput |   Rate  | Expected

Throughput |

 ++-+-+-+-

+

 | 64 |   512   | xx Mpps |   xxx % | xxxMpps

|

 ++-+-+-+-

+

 | 64 |   2048  | xx Mpps |   xxx % | xxxMpps

|

 ++-+-+-+-

+


Olivier and I am saying that only showing the Throughput Difference

(Mpps) does not provide any perspective to the result.


I am requesting that the Expected Throughput (Mpps) should be shown

in the result too, as documented in the test plan.




Ahh, this has a history, when the initial community lab infrastructure
prepared
some vendor(s) didn't want to show the actual throughput numbers.

That is why this diff and baseline introduced, and this is the how
current
infrastructure works. So this is not something related to Intel.

And as you can imagine this is not a technical issue, some companies
seems not
willing to share their performance numbers via community lab, and I
don't know
if something changed here in last a few years.



That explains it!

If those companies still want to keep the community lab performance data hidden 
(which I don't object to), wouldn't it be better if the performance test 
scripts output the deviation from the expected throughput in percent (with one 
or two decimals after the comma) instead of in Mpps?



Sounds reasonable, I assume there is a reason behind it but I don't remember, 
cc'ed lab.







Med venlig hilsen / kind regards
- Morten Brørup


-Original Message-
From: Olivier Matz [mailto:olivier.m...@6wind.com]
Sent: Tuesday, January 19, 2021 9:32 AM
To: Ali Alnubani
Cc: David Marchand; Ferruh Yigit; zhaoyan.c...@intel.com; dev;

Andrew

Rybchenko; Ananyev, Konstantin; Morten Brørup;

ajitkhapa...@gmail.com;

dpdk stable; Ajit Khaparde; Slava Ovsiienko; Alexander Kozyrev
Subject: Re: [dpdk-stable] [PATCH v4] mbuf: fix reset on mbuf free

Hi Ali,


On Mon, Jan 18, 2021 at 05:52:32PM +, Ali Alnubani wrote:

Hi,
(Sorry had to resend this to some recipients due to mail server

problems).


Just confirming that I can still reproduce the regression with

single

core and 64B frames on other servers.

Many thanks for the feedback. Can you please detail what is the

amount

of performance loss in percent, and confirm the test case? (I

suppose

it
is testpmd io forward).

Unfortunatly, I won't be able to spend a lot of time on this soon
(sorry
for that). So I see at least these 2 options:

- postpone the patch again, until I can find more time to analyze
 and optimize
- apply the patch if the performance loss is acceptable compared

t

Re: [dpdk-dev] [dpdk-stable] [PATCH v4] mbuf: fix reset on mbuf free

[Ferruh Yigit]

On 1/15/2021 6:39 PM, Ali Alnubani wrote:

Hi,
Adding Ferruh and Zhaoyan,


Ali,

You reported some performance regression, did you confirm it?
If I get no reply by monday, I'll proceed with this patch.


Sure I'll confirm by Monday.

Doesn't the regression also reproduce on the Lab's Intel servers?
Even though the check iol-intel-Performance isn't failing, I can see that the 
throughput differences from expected for this patch are less than those of 
another patch that was tested only 20 minutes earlier. Both patches were 
applied to the same tree:

https://mails.dpdk.org/archives/test-report/2021-January/173927.html

| 64 | 512 | 1.571   |


https://mails.dpdk.org/archives/test-report/2021-January/173919.html

| 64 | 512 | 2.698   |


Assuming that pw86457 doesn't have an effect on this test, it looks to me that 
this patch caused a regression in Intel hardware as well.

Can someone update the baseline's expected values for the Intel NICs and rerun 
the test on this patch?



Zhaoyan said that the baseline is calculated dynamically,
what I understand is baseline set based on previous days performance result, so 
it shouldn't require updating.


But cc'ed the lab for more details.

Re: [dpdk-dev] [PATCH v2 00/19] ensure headers have correct includes

[David Marchand]

On Fri, Jan 15, 2021 at 12:11 PM Bruce Richardson
 wrote:
>
> As a general principle, each header file should include any other
> headers it needs to provide data type definitions or macros. For
> example, any header using the uintX_t types in structures or function
> prototypes should include "stdint.h" to provide those type definitions.
>
> In practice, while many, but not all, headers in DPDK did include all
> necessary headers, it was never actually checked that each header could
> be included in a C file and compiled without having any compiler errors
> about missing definitions. This patchset fixes any missing includes in
> public headers from the DPDK "lib" folder and then adds a "chkincs" app.
> to verify this on an ongoing basis.
>
> This chkincs app does nothing when run, it's for build-time checking
> only. Its source code consists of one C file per public DPDK header,
> where that C file contains nothing except an include for that header.
> Therefore, if any header is added to the lib folder which fails to
> compile when included alone, the build of chkincs will fail with a
> suitable error message. Since this compile checking is not needed on
> most builds of DPDK, the building of chkincs is disabled by default, but
> can be enabled by the "test_includes" meson option. To catch errors with
> patch submissions, the final patch of this series enables it for a
> single build in test-meson-builds script.
>
> Future work could involve doing similar checks on headers for C++ 
> compatibility,
> for example.
>
> V2:
> * Add maintainers file entry for new app
> * Drop patch for c11 ring header
> * Use build variable "headers_no_chkincs" for tracking exceptions
>
> Bruce Richardson (19):
>   eal: fix missing header inclusion

I split this as two patches since the rte_thread.h only applies to 21.02.


>   telemetry: fix missing header include
>   ethdev: fix missing header include

Patch 3 seems unnecessary, we skip rte_eth_ctrl.h validation later but
I took it anyway.

>   net: fix missing header include
>   mbuf: fix missing header include

Is rte_byteorder.h inclusion in rte_mbuf.h still necessary?


>   bitratestats: fix missing header include
>   rib: fix missing header includes
>   vhost: fix missing header includes
>   ipsec: fix missing header include
>   fib: fix missing header includes
>   table: fix missing header include
>   pipeline: fix missing header includes
>   metrics: fix variable declaration in header
>   node: fix missing header include
>   app: fix extra include paths for app builds

Reviewed and applied patches 1-15, thanks Bruce.

-- 
David Marchand

Re: [dpdk-dev] [PATCH 2/2] test/crypto: support SSL/TLS way of cipher-auth operations

[Kusztal, ArkadiuszX]

Hi Tejasree,

> -Original Message-
> From: dev  On Behalf Of Tejasree Kondoj
> Sent: piątek, 18 grudnia 2020 15:10
> To: Akhil Goyal ; Nicolau, Radu
> 
> Cc: Tejasree Kondoj ; Anoob Joseph
> ; Ankur Dwivedi ;
> dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 2/2] test/crypto: support SSL/TLS way of 
> cipher-auth
> operations
> 
> Adding support for SSL/TLS way of cipher-auth operations order
> - auth generation followed by encryption
> - decryption followed by auth verify
> 
> Signed-off-by: Tejasree Kondoj 
> ---
>  app/test/test_cryptodev_aes_test_vectors.h | 576 +
>  app/test/test_cryptodev_blockcipher.c  |  98 +++-
>  app/test/test_cryptodev_blockcipher.h  |   9 +
>  doc/guides/rel_notes/release_21_02.rst |   6 +
>  4 files changed, 677 insertions(+), 12 deletions(-)
> 
> diff --git a/app/test/test_cryptodev_aes_test_vectors.h
> b/app/test/test_cryptodev_aes_test_vectors.h
> index c192d75a7e..c239c1cb35 100644
> --- a/app/test/test_cryptodev_aes_test_vectors.h
> +++ b/app/test/test_cryptodev_aes_test_vectors.h
> @@ -1093,6 +1093,172 @@ static const uint8_t
> ciphertext512_aes128cbc_aad[] = {
>   0x73, 0x65, 0x72, 0x73, 0x2C, 0x20, 0x73, 0x75  };
> 
> +static const uint8_t plaintext_aes_common_ssl[] = {
> + 0x57, 0x68, 0x61, 0x74, 0x20, 0x61, 0x20, 0x6C,
> + 0x6F, 0x75, 0x73, 0x79, 0x20, 0x65, 0x61, 0x72,
> + 0x74, 0x68, 0x21, 0x20, 0x48, 0x65, 0x20, 0x77,
> + 0x6F, 0x6E, 0x64, 0x65, 0x72, 0x65, 0x64, 0x20,
> + 0x68, 0x6F, 0x77, 0x20, 0x6D, 0x61, 0x6E, 0x79,
> + 0x20, 0x70, 0x65, 0x6F, 0x70, 0x6C, 0x65, 0x20,
> + 0x77, 0x65, 0x72, 0x65, 0x20, 0x64, 0x65, 0x73,
> + 0x74, 0x69, 0x74, 0x75, 0x74, 0x65, 0x20, 0x74,
> + 0x68, 0x61, 0x74, 0x20, 0x73, 0x61, 0x6D, 0x65,
> + 0x20, 0x6E, 0x69, 0x67, 0x68, 0x74, 0x20, 0x65,
> + 0x76, 0x65, 0x6E, 0x20, 0x69, 0x6E, 0x20, 0x68,
> + 0x69, 0x73, 0x20, 0x6F, 0x77, 0x6E, 0x20, 0x70,
> + 0x72, 0x6F, 0x73, 0x70, 0x65, 0x72, 0x6F, 0x75,
> + 0x73, 0x20, 0x63, 0x6F, 0x75, 0x6E, 0x74, 0x72,
> + 0x79, 0x2C, 0x20, 0x68, 0x6F, 0x77, 0x20, 0x6D,
> + 0x61, 0x6E, 0x79, 0x20, 0x68, 0x6F, 0x6D, 0x65,
> + 0x73, 0x20, 0x77, 0x65, 0x72, 0x65, 0x20, 0x73,
> + 0x68, 0x61, 0x6E, 0x74, 0x69, 0x65, 0x73, 0x2C,
> + 0x20, 0x68, 0x6F, 0x77, 0x20, 0x6D, 0x61, 0x6E,
> + 0x79, 0x20, 0x68, 0x75, 0x73, 0x62, 0x61, 0x6E,
> + 0x64, 0x73, 0x20, 0x77, 0x65, 0x72, 0x65, 0x20,
> + 0x64, 0x72, 0x75, 0x6E, 0x6B, 0x20, 0x61, 0x6E,
> + 0x64, 0x20, 0x77, 0x69, 0x76, 0x65, 0x73, 0x20,
> + 0x73, 0x6F, 0x63, 0x6B, 0x65, 0x64, 0x2C, 0x20,
> + 0x61, 0x6E, 0x64, 0x20, 0x68, 0x6F, 0x77, 0x20,
> + 0x6D, 0x61, 0x6E, 0x79, 0x20, 0x63, 0x68, 0x69,
> + 0x6C, 0x64, 0x72, 0x65, 0x6E, 0x20, 0x77, 0x65,
> + 0x72, 0x65, 0x20, 0x62, 0x75, 0x6C, 0x6C, 0x69,
> + 0x65, 0x64, 0x2C, 0x20, 0x61, 0x62, 0x75, 0x73,
> + 0x65, 0x64, 0x2C, 0x20, 0x6F, 0x72, 0x20, 0x61,
> + 0x62, 0x61, 0x6E, 0x64, 0x6F, 0x6E, 0x65, 0x64,
> + 0x2E, 0x20, 0x48, 0x6F, 0x77, 0x20, 0x6D, 0x61,
> + 0x6E, 0x79, 0x20, 0x66, 0x61, 0x6D, 0x69, 0x6C,
> + 0x69, 0x65, 0x73, 0x20, 0x68, 0x75, 0x6E, 0x67,
> + 0x65, 0x72, 0x65, 0x64, 0x20, 0x66, 0x6F, 0x72,
> + 0x20, 0x66, 0x6F, 0x6F, 0x64, 0x20, 0x74, 0x68,
> + 0x65, 0x79, 0x20, 0x63, 0x6F, 0x75, 0x6C, 0x64,
> + 0x20, 0x6E, 0x6F, 0x74, 0x20, 0x61, 0x66, 0x66,
> + 0x6F, 0x72, 0x64, 0x20, 0x74, 0x6F, 0x20, 0x62,
> + 0x75, 0x79, 0x3F, 0x20, 0x48, 0x6F, 0x77, 0x20,
> + 0x6D, 0x61, 0x6E, 0x79, 0x20, 0x68, 0x65, 0x61,
> + 0x72, 0x74, 0x73, 0x20, 0x77, 0x65, 0x72, 0x65,
> + 0x20, 0x62, 0x72, 0x6F, 0x6B, 0x65, 0x6E, 0x3F,
> + 0x20, 0x48, 0x6F, 0x77, 0x20, 0x6D, 0x61, 0x6E,
> + 0x79, 0x20, 0x73, 0x75, 0x69, 0x63, 0x69, 0x64,
> + 0x65, 0x73, 0x20, 0x77, 0x6F, 0x75, 0x6C, 0x64,
> + 0x20, 0x74, 0x61, 0x6B, 0x65, 0x20, 0x70, 0x6C,
> + 0x61, 0x63, 0x65, 0x20, 0x74, 0x68, 0x61, 0x74,
> + 0x20, 0x73, 0x61, 0x6D, 0x65, 0x20, 0x6E, 0x69,
> + 0x67, 0x68, 0x74, 0x2C, 0x20, 0x68, 0x6F, 0x77,
> + 0x20, 0x6D, 0x61, 0x6E, 0x79, 0x20, 0x70, 0x65,
> + 0x6F, 0x70, 0x6C, 0x65, 0x20, 0x77, 0x6F, 0x75,
> + 0x6C, 0x64, 0x20, 0x67, 0x6F, 0x20, 0x69, 0x6E,
> + 0x73, 0x61, 0x6E, 0x65, 0x3F, 0x20, 0x48, 0x6F,
> + 0x77, 0x20, 0x6D, 0x61, 0x6E, 0x79, 0x20, 0x63,
> + 0x6F, 0x63, 0x6B, 0x72, 0x6F, 0x61, 0x63, 0x68,
> + 0x65, 0x73, 0x20, 0x61, 0x6E, 0x64, 0x20, 0x6C,
> + 0x61, 0x6E, 0x64, 0x6C, 0x6F, 0x72, 0x64, 0x73,
> + 0x20, 0x77, 0x6F, 0x75, 0x6C, 0x64, 0x20, 0x74,
> + 0x72, 0x69, 0x75, 0x6D, 0x70, 0x68, 0x3F, 0x20,
> + 0x48, 0x6F, 0x77, 0x20, 0x6D, 0x61, 0x6E, 0x79,
> + 0x20, 0x77, 0x69, 0x6E, 0x6E, 0x65, 0x72, 0x73,
> + 0x20, 0x77, 0x65, 0x72, 0x65, 0x20, 0x6c, 0x6f,
> + 0x73, 0x65, 0x72, 0x73, 0x2c, 0x20, 0x73, 0x75,
> + /* mac */
> + 0xC4, 0xB7, 0x0E, 0x6B, 0xDE, 0xD1, 0xE7, 0x77,
> + 0x7E, 0x2E, 0x8F, 0xFC, 0x48, 0x39, 0x46, 0x17,
> + 0x3F

Re: [dpdk-dev] [dpdk-stable] [PATCH v4] mbuf: fix reset on mbuf free

[Morten Brørup]

> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Ferruh Yigit
> Sent: Thursday, January 21, 2021 10:19 AM
> 
> On 1/15/2021 6:39 PM, Ali Alnubani wrote:
> > Hi,
> > Adding Ferruh and Zhaoyan,
> >
> >> Ali,
> >>
> >> You reported some performance regression, did you confirm it?
> >> If I get no reply by monday, I'll proceed with this patch.
> >
> > Sure I'll confirm by Monday.
> >
> > Doesn't the regression also reproduce on the Lab's Intel servers?
> > Even though the check iol-intel-Performance isn't failing, I can see
> that the throughput differences from expected for this patch are less
> than those of another patch that was tested only 20 minutes earlier.
> Both patches were applied to the same tree:
> >
> > https://mails.dpdk.org/archives/test-report/2021-January/173927.html
> >> | 64 | 512 | 1.571   |
> >
> > https://mails.dpdk.org/archives/test-report/2021-January/173919.html
> >> | 64 | 512 | 2.698   |
> >
> > Assuming that pw86457 doesn't have an effect on this test, it looks
> to me that this patch caused a regression in Intel hardware as well.
> >
> > Can someone update the baseline's expected values for the Intel NICs
> and rerun the test on this patch?
> >
> 
> Zhaoyan said that the baseline is calculated dynamically,
> what I understand is baseline set based on previous days performance
> result, so
> it shouldn't require updating.

That sounds smart!

Perhaps another reference baseline could be added, for informational purposes 
only:
Deviation from the performance of the last official release.

> 
> But cc'ed the lab for more details.

Re: [dpdk-dev] [PATCH v2 00/19] ensure headers have correct includes

[Bruce Richardson]

On Thu, Jan 21, 2021 at 10:25:07AM +0100, David Marchand wrote:
> On Fri, Jan 15, 2021 at 12:11 PM Bruce Richardson
>  wrote:
> >
> > As a general principle, each header file should include any other
> > headers it needs to provide data type definitions or macros. For
> > example, any header using the uintX_t types in structures or function
> > prototypes should include "stdint.h" to provide those type definitions.
> >
> > In practice, while many, but not all, headers in DPDK did include all
> > necessary headers, it was never actually checked that each header could
> > be included in a C file and compiled without having any compiler errors
> > about missing definitions. This patchset fixes any missing includes in
> > public headers from the DPDK "lib" folder and then adds a "chkincs" app.
> > to verify this on an ongoing basis.
> >
> > This chkincs app does nothing when run, it's for build-time checking
> > only. Its source code consists of one C file per public DPDK header,
> > where that C file contains nothing except an include for that header.
> > Therefore, if any header is added to the lib folder which fails to
> > compile when included alone, the build of chkincs will fail with a
> > suitable error message. Since this compile checking is not needed on
> > most builds of DPDK, the building of chkincs is disabled by default, but
> > can be enabled by the "test_includes" meson option. To catch errors with
> > patch submissions, the final patch of this series enables it for a
> > single build in test-meson-builds script.
> >
> > Future work could involve doing similar checks on headers for C++ 
> > compatibility,
> > for example.
> >
> > V2:
> > * Add maintainers file entry for new app
> > * Drop patch for c11 ring header
> > * Use build variable "headers_no_chkincs" for tracking exceptions
> >
> > Bruce Richardson (19):
> >   eal: fix missing header inclusion
> 
> I split this as two patches since the rte_thread.h only applies to 21.02.
> 
> 
> >   telemetry: fix missing header include
> >   ethdev: fix missing header include
> 
> Patch 3 seems unnecessary, we skip rte_eth_ctrl.h validation later but
> I took it anyway.
> 
> >   net: fix missing header include
> >   mbuf: fix missing header include
> 
> Is rte_byteorder.h inclusion in rte_mbuf.h still necessary?
>
Good question. The checks I was doing were only for missing headers.
Checking for superfluous headers is more complicated and not something I've
looked at. I know it was previously suggested to use the
"include-what-you-use" tool on DPDK, if anyone wants to attempt that.

> 
> >   bitratestats: fix missing header include
> >   rib: fix missing header includes
> >   vhost: fix missing header includes
> >   ipsec: fix missing header include
> >   fib: fix missing header includes
> >   table: fix missing header include
> >   pipeline: fix missing header includes
> >   metrics: fix variable declaration in header
> >   node: fix missing header include
> >   app: fix extra include paths for app builds
> 
> Reviewed and applied patches 1-15, thanks Bruce.
> 

Thanks for the bits of rework, David.

Re: [dpdk-dev] [PATCH] app/testpmd: fix RSS key

[Zhang, AlvinX]

Hi Ferruh,

Thanks for your review.
I will modify the patch in V2.

Alvin

> -Original Message-
> From: Ferruh Yigit 
> Sent: Wednesday, January 20, 2021 3:15 AM
> To: Zhang, AlvinX 
> Cc: dev@dpdk.org; sta...@dpdk.org
> Subject: Re: [PATCH] app/testpmd: fix RSS key
> 
> On 1/18/2021 8:59 AM, Zhang,Alvin wrote:
> > From: Alvin Zhang 
> >
> > Since the patch '1848b117' has set the value of key in 'struct
> > rte_flow_action_rss' to NULL, the PMD cannot get the RSS key now.
> >
> > This patch sets offset and size of the key pointer as the first
> > parameter of the token 'key' and copies the start address of the 'HEX'
> > data to the location specified by the first parameter of the token.
> >
> 
> Can you please put the rte_flow command that enables reproducing this defect,
> it may help in the future?
> 
> > Fixes: 1848b117cca1 ("app/testpmd: fix RSS key for flow API RSS rule")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Alvin Zhang 
> > ---
> >   app/test-pmd/cmdline_flow.c | 24 +---
> >   1 file changed, 13 insertions(+), 11 deletions(-)
> >
> > diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> > index 585cab9..6eb46d3 100644
> > --- a/app/test-pmd/cmdline_flow.c
> > +++ b/app/test-pmd/cmdline_flow.c
> > @@ -3403,7 +3403,10 @@ static int comp_set_sample_index(struct context
> *, const struct token *,
> > .name = "key",
> > .help = "RSS hash key",
> > .next = NEXT(action_rss, NEXT_ENTRY(HEX)),
> > -   .args = ARGS(ARGS_ENTRY_ARB(0, 0),
> > +   .args = ARGS(ARGS_ENTRY_ARB
> > +(offsetof(struct action_rss_data, conf) +
> > + offsetof(struct rte_flow_action_rss, key),
> > + sizeof(((struct rte_flow_action_rss *)0)->key)),
> 
> 
> +1, it is required to write the address, and I confirm this enables
> +getting
> 'key' value to the PMD.
> 
> >  ARGS_ENTRY_ARB
> >  (offsetof(struct action_rss_data, conf) +
> >   offsetof(struct rte_flow_action_rss, key_len), @@
> -6495,19
> > +6498,18 @@ static int comp_set_sample_index(struct context *, const
> struct token *,
> > if (ctx->objmask)
> > memset((uint8_t *)ctx->objmask + arg_data->offset,
> > 0xff, hexlen);
> > +
> > /* Save address if requested. */
> > if (arg_addr->size) {
> > -   memcpy((uint8_t *)ctx->object + arg_addr->offset,
> > -  (void *[]){
> > -   (uint8_t *)ctx->object + arg_data->offset
> > -  },
> > -  arg_addr->size);
> > +   if (arg_addr->size < sizeof(void *))
> > +   goto error;
> 
> Above two lines seems only actual change in this function, others are 
> refactoring
> the assignment.
> 
> 1) why this check required, I think we can ignore it.
> 2) What do you think to remove the refactoring to reduce the change to the
> actual fix?
> 
> > +
> > +   *(void **)((uint8_t *)ctx->object + arg_addr->offset) =
> > +   (uint8_t *)ctx->object + arg_data->offset;
> > +
> > if (ctx->objmask)
> > -   memcpy((uint8_t *)ctx->objmask + arg_addr->offset,
> > -  (void *[]){
> > -   (uint8_t *)ctx->objmask + arg_data->offset
> > -  },
> > -  arg_addr->size);
> > +   *(void **)((uint8_t *)ctx->objmask + arg_addr->offset) =
> > +   (uint8_t *)ctx->objmask + arg_data->offset;
> > }
> > return len;
> >   error:
> >

Re: [dpdk-dev] [PATCH v2 00/19] ensure headers have correct includes

[Thomas Monjalon]

21/01/2021 10:33, Bruce Richardson:
> On Thu, Jan 21, 2021 at 10:25:07AM +0100, David Marchand wrote:
> > Is rte_byteorder.h inclusion in rte_mbuf.h still necessary?
> >
> Good question. The checks I was doing were only for missing headers.
> Checking for superfluous headers is more complicated and not something I've
> looked at. I know it was previously suggested to use the
> "include-what-you-use" tool on DPDK, if anyone wants to attempt that.

I would love having an integration of include-what-you-use in our build system.
I don't know whether it's trivial or difficult with meson.

Re: [dpdk-dev] [PATCH v1] eal/arm: fix gcc build for 128-bit atomic compare exchange

[Joyce Kong]

>-Original Message-
>From: David Marchand 
>Sent: Friday, January 15, 2021 11:56 PM
>To: Joyce Kong 
>Cc: jer...@marvell.com; Ruifeng Wang ; Honnappa
>Nagarahalli ; dev ; nd
>; dpdk stable 
>Subject: Re: [PATCH v1] eal/arm: fix gcc build for 128-bit atomic compare
>exchange
>
>On Fri, Jan 15, 2021 at 10:58 AM Joyce Kong  wrote:
>>
>> Compiling with "meson build -Dbuildtype=debug --cross-file
>> config/arm/arm64_thunderx2_linux_gcc" shows the warnings "function
>> returns an aggregate [-Waggregate-return]":
>> ../../dpdk/lib/librte_eal/arm/include/rte_atomic_64.h: In function
>> ‘__cas_128_relaxed’:
>> ../../dpdk/lib/librte_eal/arm/include/rte_atomic_64.h:81:20:
>> error: function returns an aggregate [-Werror=aggregate-return]
>> __ATOMIC128_CAS_OP(__cas_128_relaxed, "casp")
>> ^
>>
>> Fix the compiling issue by defining __ATOMIC128_CAS_OP as a void
>> function and passing the address pointer into it.
>>
>> Fixes: 7e2c3e17fe2c ("eal/arm64: add 128-bit atomic compare exchange")
>> Cc: sta...@dpdk.org
>>
>> Signed-off-by: Joyce Kong 
>
>From my tests, the trigger is when switching to debug.
>The thunderx2 target builds fine for me with debugoptimized.
>I can reproduce the issue too with octeontx2.
>
>What is the common point?
>
>--
>David Marchand

The issue was reported on octeontx2 and discussed in the patch 
https://patchwork.dpdk.org/patch/84613/, I reproduced the issue on thunderx2.
And this may be because '__cas_128_xxx' returns aggregated data type I think. 
Returning two separate uint64_t by taking as the pointer may be a solution.

[dpdk-dev] [PATCH v2] app/testpmd: fix RSS key

[Zhang,Alvin]

From: Alvin Zhang 

Since the patch '1848b117' has initialized the variable 'key' in
'struct rte_flow_action_rss' with 'NULL', the PMD cannot get the
RSS key now. Details as bellow:

testpmd> flow create 0 ingress pattern eth / ipv4 / end actions
 rss types ipv4-other end key
 1234567890123456789012345678901234567890123
 4567890123456789012345678901234567890
 queues end / end
Flow rule #1 created
testpmd> show port 0 rss-hash key
RSS functions:
 all ipv4-other ip
RSS key:
 4439796BB54C5023B675EA5B124F9F30B8A2C03DDFDC4D02A08C9B3
 34AF64A4C05C6FA343958D8557D99583AE138C92E81150366

This patch sets offset and size of the 'key' variable as the first
parameter of the token 'key'. Later, the address of the RSS key will
be copied to 'key' variable.

Fixes: 1848b117cca1 ("app/testpmd: fix RSS key for flow API RSS rule")
Cc: sta...@dpdk.org

Signed-off-by: Alvin Zhang 
---
 app/test-pmd/cmdline_flow.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 0618611..067e120 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -3541,7 +3541,10 @@ static int comp_set_modify_field_id(struct context *, 
const struct token *,
.name = "key",
.help = "RSS hash key",
.next = NEXT(action_rss, NEXT_ENTRY(HEX)),
-   .args = ARGS(ARGS_ENTRY_ARB(0, 0),
+   .args = ARGS(ARGS_ENTRY_ARB
+(offsetof(struct action_rss_data, conf) +
+ offsetof(struct rte_flow_action_rss, key),
+ sizeof(((struct rte_flow_action_rss *)0)->key)),
 ARGS_ENTRY_ARB
 (offsetof(struct action_rss_data, conf) +
  offsetof(struct rte_flow_action_rss, key_len),
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v2 00/19] ensure headers have correct includes

[Bruce Richardson]

On Thu, Jan 21, 2021 at 10:36:18AM +0100, Thomas Monjalon wrote:
> 21/01/2021 10:33, Bruce Richardson:
> > On Thu, Jan 21, 2021 at 10:25:07AM +0100, David Marchand wrote:
> > > Is rte_byteorder.h inclusion in rte_mbuf.h still necessary?
> > >
> > Good question. The checks I was doing were only for missing headers.
> > Checking for superfluous headers is more complicated and not something I've
> > looked at. I know it was previously suggested to use the
> > "include-what-you-use" tool on DPDK, if anyone wants to attempt that.
> 
> I would love having an integration of include-what-you-use in our build 
> system.
> I don't know whether it's trivial or difficult with meson.
> 
+1 to that.

I'm not familiar enough with the tool to comment on how hard or difficult
it would be. I try it once a few years ago, and I don't think I managed to
get it working successfully standalone, let alone as part of a build
system. However, I'm sure IWYU has moved on quite a bit since then...

Re: [dpdk-dev] [PATCH 2/2] test/crypto: support SSL/TLS way of cipher-auth operations

[Tejasree Kondoj]

Hi Arek,

Please see inline.

Thanks
Tejasree

> -Original Message-
> From: Kusztal, ArkadiuszX 
> Sent: Thursday, January 21, 2021 2:56 PM
> To: Tejasree Kondoj ; Akhil Goyal
> ; Nicolau, Radu 
> Cc: Anoob Joseph ; Ankur Dwivedi
> ; dev@dpdk.org
> Subject: [EXT] RE: [dpdk-dev] [PATCH 2/2] test/crypto: support SSL/TLS way
> of cipher-auth operations
> 
> External Email
> 
> --
> Hi Tejasree,
> 
> > -Original Message-
> > From: dev  On Behalf Of Tejasree Kondoj
> > Sent: piątek, 18 grudnia 2020 15:10
> > To: Akhil Goyal ; Nicolau, Radu
> > 
> > Cc: Tejasree Kondoj ; Anoob Joseph
> > ; Ankur Dwivedi ;
> > dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH 2/2] test/crypto: support SSL/TLS way of
> > cipher-auth operations
> >
> > Adding support for SSL/TLS way of cipher-auth operations order
> > - auth generation followed by encryption
> > - decryption followed by auth verify
> >
> > Signed-off-by: Tejasree Kondoj 
> > ---
> >  app/test/test_cryptodev_aes_test_vectors.h | 576
> +
> >  app/test/test_cryptodev_blockcipher.c  |  98 +++-
> >  app/test/test_cryptodev_blockcipher.h  |   9 +
> >  doc/guides/rel_notes/release_21_02.rst |   6 +
> >  4 files changed, 677 insertions(+), 12 deletions(-)
> >
> > diff --git a/app/test/test_cryptodev_aes_test_vectors.h
> > b/app/test/test_cryptodev_aes_test_vectors.h
> > index c192d75a7e..c239c1cb35 100644
> > --- a/app/test/test_cryptodev_aes_test_vectors.h
> > +++ b/app/test/test_cryptodev_aes_test_vectors.h
> > @@ -1093,6 +1093,172 @@ static const uint8_t
> > ciphertext512_aes128cbc_aad[] = {
> > 0x73, 0x65, 0x72, 0x73, 0x2C, 0x20, 0x73, 0x75  };
> >
> > +static const uint8_t plaintext_aes_common_ssl[] = {
> > +   0x57, 0x68, 0x61, 0x74, 0x20, 0x61, 0x20, 0x6C,
> > +   0x6F, 0x75, 0x73, 0x79, 0x20, 0x65, 0x61, 0x72,
> > +   0x74, 0x68, 0x21, 0x20, 0x48, 0x65, 0x20, 0x77,
> > +   0x6F, 0x6E, 0x64, 0x65, 0x72, 0x65, 0x64, 0x20,
> > +   0x68, 0x6F, 0x77, 0x20, 0x6D, 0x61, 0x6E, 0x79,
> > +   0x20, 0x70, 0x65, 0x6F, 0x70, 0x6C, 0x65, 0x20,
> > +   0x77, 0x65, 0x72, 0x65, 0x20, 0x64, 0x65, 0x73,
> > +   0x74, 0x69, 0x74, 0x75, 0x74, 0x65, 0x20, 0x74,
> > +   0x68, 0x61, 0x74, 0x20, 0x73, 0x61, 0x6D, 0x65,
> > +   0x20, 0x6E, 0x69, 0x67, 0x68, 0x74, 0x20, 0x65,
> > +   0x76, 0x65, 0x6E, 0x20, 0x69, 0x6E, 0x20, 0x68,
> > +   0x69, 0x73, 0x20, 0x6F, 0x77, 0x6E, 0x20, 0x70,
> > +   0x72, 0x6F, 0x73, 0x70, 0x65, 0x72, 0x6F, 0x75,
> > +   0x73, 0x20, 0x63, 0x6F, 0x75, 0x6E, 0x74, 0x72,
> > +   0x79, 0x2C, 0x20, 0x68, 0x6F, 0x77, 0x20, 0x6D,
> > +   0x61, 0x6E, 0x79, 0x20, 0x68, 0x6F, 0x6D, 0x65,
> > +   0x73, 0x20, 0x77, 0x65, 0x72, 0x65, 0x20, 0x73,
> > +   0x68, 0x61, 0x6E, 0x74, 0x69, 0x65, 0x73, 0x2C,
> > +   0x20, 0x68, 0x6F, 0x77, 0x20, 0x6D, 0x61, 0x6E,
> > +   0x79, 0x20, 0x68, 0x75, 0x73, 0x62, 0x61, 0x6E,
> > +   0x64, 0x73, 0x20, 0x77, 0x65, 0x72, 0x65, 0x20,
> > +   0x64, 0x72, 0x75, 0x6E, 0x6B, 0x20, 0x61, 0x6E,
> > +   0x64, 0x20, 0x77, 0x69, 0x76, 0x65, 0x73, 0x20,
> > +   0x73, 0x6F, 0x63, 0x6B, 0x65, 0x64, 0x2C, 0x20,
> > +   0x61, 0x6E, 0x64, 0x20, 0x68, 0x6F, 0x77, 0x20,
> > +   0x6D, 0x61, 0x6E, 0x79, 0x20, 0x63, 0x68, 0x69,
> > +   0x6C, 0x64, 0x72, 0x65, 0x6E, 0x20, 0x77, 0x65,
> > +   0x72, 0x65, 0x20, 0x62, 0x75, 0x6C, 0x6C, 0x69,
> > +   0x65, 0x64, 0x2C, 0x20, 0x61, 0x62, 0x75, 0x73,
> > +   0x65, 0x64, 0x2C, 0x20, 0x6F, 0x72, 0x20, 0x61,
> > +   0x62, 0x61, 0x6E, 0x64, 0x6F, 0x6E, 0x65, 0x64,
> > +   0x2E, 0x20, 0x48, 0x6F, 0x77, 0x20, 0x6D, 0x61,
> > +   0x6E, 0x79, 0x20, 0x66, 0x61, 0x6D, 0x69, 0x6C,
> > +   0x69, 0x65, 0x73, 0x20, 0x68, 0x75, 0x6E, 0x67,
> > +   0x65, 0x72, 0x65, 0x64, 0x20, 0x66, 0x6F, 0x72,
> > +   0x20, 0x66, 0x6F, 0x6F, 0x64, 0x20, 0x74, 0x68,
> > +   0x65, 0x79, 0x20, 0x63, 0x6F, 0x75, 0x6C, 0x64,
> > +   0x20, 0x6E, 0x6F, 0x74, 0x20, 0x61, 0x66, 0x66,
> > +   0x6F, 0x72, 0x64, 0x20, 0x74, 0x6F, 0x20, 0x62,
> > +   0x75, 0x79, 0x3F, 0x20, 0x48, 0x6F, 0x77, 0x20,
> > +   0x6D, 0x61, 0x6E, 0x79, 0x20, 0x68, 0x65, 0x61,
> > +   0x72, 0x74, 0x73, 0x20, 0x77, 0x65, 0x72, 0x65,
> > +   0x20, 0x62, 0x72, 0x6F, 0x6B, 0x65, 0x6E, 0x3F,
> > +   0x20, 0x48, 0x6F, 0x77, 0x20, 0x6D, 0x61, 0x6E,
> > +   0x79, 0x20, 0x73, 0x75, 0x69, 0x63, 0x69, 0x64,
> > +   0x65, 0x73, 0x20, 0x77, 0x6F, 0x75, 0x6C, 0x64,
> > +   0x20, 0x74, 0x61, 0x6B, 0x65, 0x20, 0x70, 0x6C,
> > +   0x61, 0x63, 0x65, 0x20, 0x74, 0x68, 0x61, 0x74,
> > +   0x20, 0x73, 0x61, 0x6D, 0x65, 0x20, 0x6E, 0x69,
> > +   0x67, 0x68, 0x74, 0x2C, 0x20, 0x68, 0x6F, 0x77,
> > +   0x20, 0x6D, 0x61, 0x6E, 0x79, 0x20, 0x70, 0x65,
> > +   0x6F, 0x70, 0x6C, 0x65, 0x20, 0x77, 0x6F, 0x75,
> > +   0x6C, 0x64, 0x20, 0x67, 0x6F, 0x20, 0x69, 0x6E,
> > +   0x73, 0x61, 0x6E, 0x65, 0x3F, 0x20, 0x48, 0x6F,
> > +   0x77, 0x20, 0x6D, 0x61, 0x6E, 0x79, 0x20, 0x63,
> > +   0x6F, 0x63, 0x6B, 0x72, 0x6F, 0x61, 0x63, 0x68,
> > +   0x65, 0x73, 0x20, 0x61, 0x6E, 0x64, 0x20, 0x6C,
> > +   0x61, 0

Re: [dpdk-dev] [PATCH v7 1/2] cryptodev: support enqueue and dequeue callback functions

[Kinsella, Ray]

> -Original Message-
> From: Thomas Monjalon 
> Sent: Tuesday 19 January 2021 18:32
> To: Gujjar, Abhinandan S 
> Cc: dev@dpdk.org; Ananyev, Konstantin ;
> Akhil Goyal ; Kinsella, Ray
> ; acon...@redhat.com; david.march...@redhat.com
> Subject: Re: [dpdk-dev] [PATCH v7 1/2] cryptodev: support enqueue and
> dequeue callback functions
> 
> 15/01/2021 17:01, Akhil Goyal:
> > > This patch adds APIs to add/remove callback functions on crypto
> > > enqueue/dequeue burst. The callback function will be called for
> each
> > > burst of crypto ops received/sent on a given crypto device queue
> > > pair.
> > >
> > > Signed-off-by: Abhinandan Gujjar 
> > > Acked-by: Konstantin Ananyev 
> > > ---
> > Series applied to dpdk-next-crypto
> 
> 
> It is missing a rule to ignore the false positive ABI break:
> 
> --- a/devtools/libabigail.abignore
> +++ b/devtools/libabigail.abignore
> @@ -11,3 +11,8 @@
>  ; Explicit ignore for driver-only ABI
>  [suppress_type]
>  name = eth_dev_ops
> +
> +; Ignore fields inserted in cacheline boundary of rte_cryptodev
> +[suppress_type]
> +name = rte_cryptodev
> +has_data_member_inserted_between = {0, 1023}
> 

This is a bit of a blunt instrument as the range quiet large?
{offset_after(attached), end} instead works better - I will send a patch.

> I'll add this change while pulling in the main tree.
> 

BTW - can people use ashroe.eu, not intel.com for ABI stuff. 

Ray K

Re: [dpdk-dev] [PATCH v7 1/2] cryptodev: support enqueue and dequeue callback functions

[Kinsella, Ray]

> -Original Message-
> From: Thomas Monjalon 
> Sent: Wednesday 20 January 2021 13:16
> To: Gujjar, Abhinandan S ; Kinsella, Ray
> ; m...@ashroe.eu
> Cc: dev@dpdk.org; Ananyev, Konstantin ;
> Akhil Goyal ; acon...@redhat.com;
> david.march...@redhat.com
> Subject: Re: [dpdk-dev] [PATCH v7 1/2] cryptodev: support enqueue and
> dequeue callback functions
> 
> 20/01/2021 14:01, Kinsella, Ray:
> > From: Thomas Monjalon 
> > > 15/01/2021 17:01, Akhil Goyal:
> > > > > This patch adds APIs to add/remove callback functions on crypto
> > > > > enqueue/dequeue burst. The callback function will be called for
> > > each
> > > > > burst of crypto ops received/sent on a given crypto device
> queue
> > > > > pair.
> > > > >
> > > > > Signed-off-by: Abhinandan Gujjar 
> > > > > Acked-by: Konstantin Ananyev 
> > > > > ---
> > > > Series applied to dpdk-next-crypto
> > >
> > >
> > > It is missing a rule to ignore the false positive ABI break:
> > >
> > > --- a/devtools/libabigail.abignore
> > > +++ b/devtools/libabigail.abignore
> > > @@ -11,3 +11,8 @@
> > >  ; Explicit ignore for driver-only ABI  [suppress_type]
> > >  name = eth_dev_ops
> > > +
> > > +; Ignore fields inserted in cacheline boundary of rte_cryptodev
> > > +[suppress_type]
> > > +name = rte_cryptodev
> > > +has_data_member_inserted_between = {0, 1023}
> > >
> >
> > This is a bit of a blunt instrument as the range quiet large?
> 
> The range is in bits. It matches the actual size of the struct for 64B
> cacheline.

Ok

> 
> > {offset_after(attached), end} instead works better - I will send a
> patch.
> 
> Yes that's exactly what David told me earlier today.

Makes sense, I think.

> 
> > > I'll add this change while pulling in the main tree.
> 
> Yes please.
> Note: we missed requiring this exception rule in the original patch.

Ok, in the next 20 minutes or so.

Re: [dpdk-dev] [dpdk-stable] [PATCH] vdpa/mlx5: fix configuration mutex cleanup

[Maxime Coquelin]

On 1/14/21 4:23 PM, Matan Azrad wrote:
> 
> 
> From: Maxime Coquelin
>> On 1/14/21 2:09 PM, Matan Azrad wrote:
>>>
>>>
>>> From: Maxime Coquelin
 Hi Matan,

 On 1/14/21 12:49 PM, Matan Azrad wrote:
> Hi Maxime and David
>
> Thank you for Review.
>
> From: David Marchand
>> On Fri, Jan 8, 2021 at 9:48 AM David Marchand
>>  wrote:
 I wonder if it would be possible and cleaner to disable
 cancellation on the thread while the mutex is held?
>
> Yes, we can cause thread to return by some global variable sync.
> It is the same logic.

 No, that was not my suggestion. My suggestion is to block the thread
 cancellation while in the critical section, using pthread_setcancelstate().
>>>
>>> Yes, Generally it is better to let the thread control his cancellation, 
>>> either
>> cancel itself or enabling\disabling cancellations.
>>>
>>> I don't see a reason to wait for the thread in current logic - the critical 
>>> section
>> is not important to be completed here.
>>
>> The reason I see is there are quite a few things done in this critical 
>> section. And
>> if tomorrow someone add new things in it, he may not know the thread can be
>> cancelled at any time, which could cause hard to debug issues.
> 
> As I said, here it is not needed, this thread designed just to cause guest 
> notifications.
> 
> The optional future developer mistake can be done also outside the critical 
> section in in any other place - we cannot protect it.
> 
> The design choice is to close the thread fast.

But why is it so urgent that it cannot been stopped cleanly?
I don't think it would add seconds delay by doing it in a clean way.

Thanks,
Maxime

>>> We just want to close the thread and to clean the mutex.
>>>
>>> +1
>>
>> IEEE Std 1003.1-2001/Cor 2-2004, item XBD/TC2/D6/26 is applied,
>> adding pthread_t to the list of types that are not required to be
>> arithmetic types, thus allowing pthread_t to be defined as a structure.
>>
>> It would be better to leave pthread_t alone and not interpret it:
>>
>> if (priv->timer_tid) {
>> pthread_cancel(priv->timer_tid);
>> pthread_join(priv->timer_tid, &status); }
>> priv->timer_tid = 0;
>
>
> I'm not sure why you think it is better in this specific case.
> The cancellation will close the thread in faster way, no need to
> wait for the
 thread to close itself.
>
>
>>
>> --
>> David Marchand
>
>>>
> 

Re: [dpdk-dev] [PATCH v2] mem: improve parameter checking on memory hotplug

[Burakov, Anatoly]

On 19-Jan-21 11:39 PM, Thomas Monjalon wrote:

18/01/2021 16:41, Anatoly Burakov:

+   /* now that we've validated the request, time for a PSA */


What is PSA?


Public Service Announcement :)




+   eal_memalloc_mem_event_notify(RTE_MEM_EVENT_FREE,
+   m->free_req.addr, m->free_req.len);

[...]


+   /* this is checked by the API, but we need to prevent divide by zero */
+   if (ar->page_sz == 0 || !rte_is_power_of_2(ar->page_sz)) {
+   RTE_LOG(ERR, EAL, "Attempting to allocate with page size\n");


Is there a missing part in the log message?



Seems like it. I'll resubmit.

--
Thanks,
Anatoly

Re: [dpdk-dev] [PATCH] net/bnxt: fix FW version log

[Ajit Khaparde]

On Sun, Jan 17, 2021 at 7:58 PM Kalesh A P
 wrote:
>
> From: Kalesh AP 
>
> Driver is not logging the complete FW version along with HSI version.
> Fix it to indicate complete FW version string.
>
> Fixes: 9a891c1764ea ("net/bnxt: update HWRM to version 1.9.2")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Kalesh AP 
> Reviewed-by: Somnath Kotur 
> Reviewed-by: Ajit Kumar Khaparde 
Patch applied to dpdk-next-net-brcm.

> ---
>  drivers/net/bnxt/bnxt_hwrm.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
> index 6d54b16..4d8ca9e 100644
> --- a/drivers/net/bnxt/bnxt_hwrm.c
> +++ b/drivers/net/bnxt/bnxt_hwrm.c
> @@ -1102,10 +1102,11 @@ int bnxt_hwrm_ver_get(struct bnxt *bp, uint32_t 
> timeout)
> else
> HWRM_CHECK_RESULT();
>
> -   PMD_DRV_LOG(INFO, "%d.%d.%d:%d.%d.%d\n",
> +   PMD_DRV_LOG(INFO, "%d.%d.%d:%d.%d.%d.%d\n",
> resp->hwrm_intf_maj_8b, resp->hwrm_intf_min_8b,
> resp->hwrm_intf_upd_8b, resp->hwrm_fw_maj_8b,
> -   resp->hwrm_fw_min_8b, resp->hwrm_fw_bld_8b);
> +   resp->hwrm_fw_min_8b, resp->hwrm_fw_bld_8b,
> +   resp->hwrm_fw_rsvd_8b);
> bp->fw_ver = (resp->hwrm_fw_maj_8b << 24) |
>  (resp->hwrm_fw_min_8b << 16) |
>  (resp->hwrm_fw_bld_8b << 8) |
> --
> 2.10.1
>

Re: [dpdk-dev] [PATCH] net/bnxt: fix ptype index calculation

[Ajit Khaparde]

On Mon, Jan 18, 2021 at 1:57 PM Lance Richardson
 wrote:
>
> Fix mask to include all four bits of hardware packet type
> field.
>
> Fixes: 97b1db288dd0 ("net/bnxt: use table based packet type translation")
> Cc: sta...@dpdk.org
> Signed-off-by: Lance Richardson 
Patch applied to dpdk-next-net-brcm.

> ---
>  drivers/net/bnxt/bnxt_rxr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
> index a195bf1187..969cae19fc 100644
> --- a/drivers/net/bnxt/bnxt_rxr.c
> +++ b/drivers/net/bnxt/bnxt_rxr.c
> @@ -402,7 +402,7 @@ bnxt_init_ptype_table(void)
>
> ip6 = i & (RX_PKT_CMPL_FLAGS2_IP_TYPE >> 7);
> tun = i & (RX_PKT_CMPL_FLAGS2_T_IP_CS_CALC >> 2);
> -   type = (i & 0x38) << 9;
> +   type = (i & 0x78) << 9;
>
> if (!tun && !ip6)
> l3 = RTE_PTYPE_L3_IPV4_EXT_UNKNOWN;
> --
> 2.25.1
>

[dpdk-dev] Downloading the dpdk-kmods repo as a tarball

[Ferruh Yigit]

Hi Thomas,

There is a user request to be able to download the 'dpdk-kmods' repo as tarball.

The 'dpdk' repo can be downloaded form https://git.dpdk.org/ per each tag, but 
since the 'dpdk-kmods' repo doesn't have any tag, it is not possible.


Also 'dpdk-kmods' is not under the download page, http://static.dpdk.org/rel/.


The 'dpdk-kmods' repo is not version bound, so I don't know what kind of tag can 
be added to the repo, or what can be put under the download page.


Other option can be mirroring the 'dpdk-kmods' repo to the github, which allows 
downloading a branch as tarball.


What do you think?

Thanks,
ferruh

[dpdk-dev] DPDK Release Status Meeting 21/01/2021

[Ferruh Yigit]

Meeting minutes of 21 January 2021
--

Agenda:
* Release Dates
* Highlights
* -rc1 status
* Subtrees
* LTS
* Opens

Participants:
* Arm
* Broadcom
* Debian/Microsoft
* Intel
* Nvidia
* NXP
* Red Hat


Release Dates
-

* v21.02 dates
  * -rc1 is released on Tuesday, 19 January 2021
* http://inbox.dpdk.org/dev/4846307.Pfn0FrbqUJ@thomas/
  * -rc2Friday, 29 January 2021
  * -rc3Friday, 5 February 2021
  * Release pushed to   *Wednesday, 17 February 2021*

  * Release date may conflict with Chinese New Year, we need to discuss and
define the release date offline in the mail list, please comment.


Highlights
--

* Need to finalize the 21.02 release date on the mail list.

* pmdinfogen will be switched to python implementation, CI / testing
  infrastructures should prepare themselves for the 'pyelftools' dependency.
  The patchset to verify the infrastructure in advance:
  * https://patches.dpdk.org/project/dpdk/list/?series=13153


-rc1 status
---

* No testing result received yet.

* Two build errors detected, virtio for Arm and mingw cross build.


Subtrees


* main
  * There are built errors with -rc1
* Arm virtio build error, asked for help
* Mingw cross builds, with older versions of compiler
  * Build related updates can continue for -rc2
* Applied changes were mostly for Arm
* New build options can be added
  * pmdinfogen python rewrite not merged for -rc1, but planned for -rc2
* This may break the CI / test infrastructures because of 'pyelftools'
  dependency
  * This has been called out many times, will merge at this point
  * Intel power management series
* Partially merged, ethdev & eal part merged, power library part is
  remaining
  * power library get a new version
* Thomas has concern about the power library design, it looks like
  designed for a specific case and not generic
  * Currently there is not better suggestion, will proceed if no
there is no objection
  * Header check patchset merged partially
  * ABI checks, some exceptions added
* Exceptions should be reviewed carefully
* We lost Travis automated ABI checks
  * There is github actions checks but it is not sending reports back to
patchwork
* There is a work going on for reporting
  * Authors either check ABI themselves or explicitly check the github
actions test results for it
* Can check automated test from:
  https://github.com/ovsrobot/dpdk/actions
  * Is ring library refactoring work stalled? Arm will check.
* https://patches.dpdk.org/project/dpdk/list/?series=14405

* next-net
  * Following ehtdev patches not able to make the -rc1 and postponed to next
release:
* ethdev: introduce representor type
  * last version sent late for -rc1
* add apistats function
  * Not clear if this is right approach, more comments required
* Also there are some ethdev patches from previous releases, they need to be
  cleaned up, most probably will be done in next release.
  * For -rc2, there are
* octeon_tx endpoint driver
* ionic set
* various driver and testpmd fixes
* patchsets that first version sent after -rc1 will get less priority

* next-crypto
  * There is new compressdev PMD for the -rc2
  * Also an ABI break discussion is going on

* next-eventdev
  * no update

* next-virtio
  * The big refactor set work is going on
* Plan is to merge it for -rc2 if it is ready
  * Intel vhost example review is going on, planned for -rc2
  * There are some concerns on Alibaba's PIO mapping patch
* Not able to test but there is potential issues
  * Struct packing series has less priority against the refactoring sets,
and can wait the refactoring sets to be merged first.

* next-net-mlx
  * -rc1 looks OK
  * A couple of patches already merged for the -rc2
  * A few more is expected

* next-net-brcm
  * A few fixes in the backlog

* next-net-intel
  * Progressing

* next-net-mrvl
  * mvpp2 is expected for the -rc2


LTS
---

* v18.11.11 is released
  * http://inbox.dpdk.org/dev/20210120155818.388598-1-ktray...@redhat.com
  * This is the last release of the 18.11 LTS, thanks to all contributors

* v19.11.7
  * Luca will start working on patches

* v20.11.x
  * Kevin will step down from the 20.11 LTS maintainership, volunteers are
welcome.


Opens
-

* Coverity scans are automated but not able to assign defects

* Milestone doc is still pending
  * https://patches.dpdk.org/patch/86455/



DPDK Release Status Meetings


The DPDK Release Status Meeting is intended for DPDK Committers to discuss the
status of the master tree and sub-trees, and for project managers to track
progress or milestone dates.

The meeting occurs on every Thursdays at 8:30 UTC. on https://meet.jit.si/DPDK

If you wish to attend just send an email to
"John McN

Re: [dpdk-dev] [PATCH] net/mlx5: fix flow split combined with counter

[Raslan Darawsheh]

Hi,

> -Original Message-
> From: dev  On Behalf Of Dekel Peled
> Sent: Sunday, January 17, 2021 11:41 AM
> To: Matan Azrad ; Shahaf Shuler
> ; Slava Ovsiienko 
> Cc: dev@dpdk.org; sta...@dpdk.org
> Subject: [dpdk-dev] [PATCH] net/mlx5: fix flow split combined with counter
> 
> Currently, for a flow containing a count action, if flow is split to
> sub-flows, a new counter will be created for each sub-flow.
> However only the counter created for the last sub-flow will be queried
> on flow query and cleared on flow removal.
> 
> This behavior is wrong, causing a leak of resources.
> Need to create just one counter per flow, and use it for all sub-flows.
> 
> This patch adds the required check to make sure a counter is
> created just once per flow, and used by all sub-flows.
> 
> Fixes: fa2d01c87d2b ("net/mlx5: support flow aging")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Dekel Peled 
> 
Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh

Re: [dpdk-dev] [PATCH] net/mlx5: fix flow split combined with age action

[Raslan Darawsheh]

Hi,

> -Original Message-
> From: dev  On Behalf Of Dekel Peled
> Sent: Sunday, January 17, 2021 11:41 AM
> To: Matan Azrad ; Shahaf Shuler
> ; Slava Ovsiienko 
> Cc: dev@dpdk.org; sta...@dpdk.org
> Subject: [dpdk-dev] [PATCH] net/mlx5: fix flow split combined with age
> action
> 
> Currently, for a flow containing an age action, if flow is split to
> sub-flows, a new age action will be created for each sub-flow.
> However only the action created for the last sub-flow will be queried
> on flow query and cleared on flow removal.
> 
> This behavior is wrong, causing a leak of resources.
> Need to create just one action per flow, and use it for all sub-flows.
> 
> This patch adds the required check to make sure an age action is
> created just once per flow, and used by all sub-flows.
> 
> Fixes: f935ed4b645a ("net/mlx5: support flow hit action for aging")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Dekel Peled 
> ---

Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh

Re: [dpdk-dev] [PATCH v9 1/2] examples/vhost: add ioat ring space count and check

[Maxime Coquelin]

On 1/12/21 5:38 AM, Cheng Jiang wrote:
> Add ioat ring space count and check, if ioat ring space is not enough
> for the next async vhost packet enqueue, then just return to prevent
> enqueue failure. Add rte_ioat_completed_ops() fail handler.
> 
> Signed-off-by: Cheng Jiang 
> Reviewed-by: Jiayu Hu 
> Reviewed-by: Maxime Coquelin 
> ---
>  examples/vhost/ioat.c | 24 +---
>  1 file changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/examples/vhost/ioat.c b/examples/vhost/ioat.c
> index 71d8a1f1f..dbad28d43 100644
> --- a/examples/vhost/ioat.c
> +++ b/examples/vhost/ioat.c
> @@ -17,6 +17,7 @@ struct packet_tracker {
>   unsigned short next_read;
>   unsigned short next_write;
>   unsigned short last_remain;
> + unsigned short ioat_space;
>  };
> 
>  struct packet_tracker cb_tracker[MAX_VHOST_DEVICE];
> @@ -113,7 +114,7 @@ open_ioat(const char *value)
>   goto out;
>   }
>   rte_rawdev_start(dev_id);
> -
> + cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE;
>   dma_info->nr++;
>   i++;
>   }
> @@ -140,13 +141,9 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
>   src = descs[i_desc].src;
>   dst = descs[i_desc].dst;
>   i_seg = 0;
> + if (cb_tracker[dev_id].ioat_space < src->nr_segs)
> + break;
>   while (i_seg < src->nr_segs) {
> - /*
> -  * TODO: Assuming that the ring space of the
> -  * IOAT device is large enough, so there is no
> -  * error here, and the actual error handling
> -  * will be added later.
> -  */
>   rte_ioat_enqueue_copy(dev_id,
>   (uintptr_t)(src->iov[i_seg].iov_base)
>   + src->offset,
> @@ -158,7 +155,8 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
>   i_seg++;
>   }
>   write &= mask;
> - cb_tracker[dev_id].size_track[write] = i_seg;
> + cb_tracker[dev_id].size_track[write] = src->nr_segs;
> + cb_tracker[dev_id].ioat_space -= src->nr_segs;
>   write++;
>   }
>   } else {
> @@ -178,17 +176,21 @@ ioat_check_completed_copies_cb(int vid, uint16_t 
> queue_id,
>  {
>   if (!opaque_data) {
>   uintptr_t dump[255];
> - unsigned short n_seg;
> + int n_seg;
>   unsigned short read, write;
>   unsigned short nb_packet = 0;
>   unsigned short mask = MAX_ENQUEUED_SIZE - 1;
>   unsigned short i;
> +
>   int dev_id = dma_bind[vid].dmas[queue_id * 2
>   + VIRTIO_RXQ].dev_id;
>   n_seg = rte_ioat_completed_ops(dev_id, 255, dump, dump);
> - n_seg += cb_tracker[dev_id].last_remain;
> - if (!n_seg)
> + if (n_seg <= 0)
>   return 0;

In a separate patch, it might make sense to propagate the error if
rte_ioat_completed_ops return -1.

Reviewed-by: Maxime Coquelin 

Maxime
> +
> + cb_tracker[dev_id].ioat_space += n_seg;
> + n_seg += cb_tracker[dev_id].last_remain;
> +
>   read = cb_tracker[dev_id].next_read;
>   write = cb_tracker[dev_id].next_write;
>   for (i = 0; i < max_packets; i++) {
> --
> 2.29.2
> 

Re: [dpdk-dev] [PATCH v9 2/2] examples/vhost: refactor vhost data path

[Maxime Coquelin]

On 1/12/21 5:38 AM, Cheng Jiang wrote:
> Change the vm2vm data path to batch enqueue for better performance.
> Support latest async vhost API, refactor vhost async data path,
> replace rte_atomicNN_xxx to __atomic_XXX and clean some codes.
> 
> Signed-off-by: Cheng Jiang 
> Reviewed-by: Jiayu Hu 
> ---
>  examples/vhost/ioat.h |   2 +-
>  examples/vhost/main.c | 238 ++
>  examples/vhost/main.h |   6 +-
>  3 files changed, 178 insertions(+), 68 deletions(-)
> 


Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

[dpdk-dev] [PATCH] app/testpmd: fix queue reconfig request on Rx split update

[Viacheslav Ovsiienko]

There is the "set rxpkts" command in the testpmd interactive mode,
it configures the segment sizes to split the packet on receiving.
The mentioned segment sizes are provided on the Rx queue setup
as part of queue configuration. Hence, to take the rxpkts command
into effect the Rx queues must be explicitly reconfigured.

The explained above is related to the "set rxoffs" as well.

The patch sets the queue reconfiguration request flag for
all devices once Rx split settings are updated, to take
the changes into effect the port(s) should be restarted.

Fixes: 0f2096d7ab36 ("app/testpmd: add rxpkts commands and parameters")
Fixes: 91c78e090eed ("app/testpmd: add rxoffs commands and parameters")
Cc: sta...@dpdk.org

Signed-off-by: Viacheslav Ovsiienko 
---
 app/test-pmd/cmdline.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 0d2d6aa..3ffc7bf 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -3782,6 +3782,7 @@ struct cmd_set_rxoffs_result {
  MAX_SEGS_BUFFER_SPLIT, seg_offsets, 0);
if (nb_segs > 0)
set_rx_pkt_offsets(seg_offsets, nb_segs);
+   cmd_reconfig_device_queue(RTE_PORT_ALL, 0, 1);
 }
 
 cmdline_parse_token_string_t cmd_set_rxoffs_keyword =
@@ -3828,6 +3829,7 @@ struct cmd_set_rxpkts_result {
  MAX_SEGS_BUFFER_SPLIT, seg_lengths, 0);
if (nb_segs > 0)
set_rx_pkt_segments(seg_lengths, nb_segs);
+   cmd_reconfig_device_queue(RTE_PORT_ALL, 0, 1);
 }
 
 cmdline_parse_token_string_t cmd_set_rxpkts_keyword =
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 1/1] doc: simplify OS support in features matrix

[Ferruh Yigit]

On 1/21/2021 8:38 AM, Andrew Rybchenko wrote:

On 1/21/21 2:03 AM, Thomas Monjalon wrote:

The networking drivers features matrix had rows to show
OS and kernel modules support:
- BSD nic_uio
- Linux UIO
- Linux VFIO
- Other kdrv
- Windows

The kernel modules details are removed to keep only OS support:
- FreeBSD
- Linux
- Windows

Signed-off-by: Thomas Monjalon 


Acked-by: Andrew Rybchenko 



Acked-by: Ferruh Yigit 

Re: [dpdk-dev] [PATCH v2 0/2] net/mlx4: fix PCI probe and remove functions

[Raslan Darawsheh]

Hi,

> -Original Message-
> From: Michael Baum 
> Sent: Wednesday, January 20, 2021 10:15 AM
> To: dev@dpdk.org
> Cc: Matan Azrad ; Raslan Darawsheh
> ; David Marchand 
> Subject: [PATCH v2 0/2] net/mlx4: fix PCI probe and remove functions
> 
> A couple of bug fixes in probing and removing a PCI device.
> 
> v1: Initial version.
> v2: Fix coverity issue.
> 
> Michael Baum (2):
>   net/mlx4: fix device detach
>   net/mlx4: fix PCI probe error flow
> 
>  drivers/net/mlx4/mlx4.c | 62
> +++--
>  1 file changed, 50 insertions(+), 12 deletions(-)
> 
> --
> 1.8.3.1

Series applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh

Re: [dpdk-dev] [PATCH v5 0/3] support both PIO and MMIO BAR for virtio PMD

[谢华伟(此时此刻）]

On 2021/1/21 16:47, Maxime Coquelin wrote:


On 1/21/21 5:12 AM, 谢华伟(此时此刻） wrote:

On 2021/1/13 1:37, Maxime Coquelin wrote:

On 10/22/20 5:51 PM, 谢华伟(此时此刻） wrote:

From: "huawei.xhw" 

Legacy virtio-pci only supports PIO BAR resource. As we need to
create lots of
virtio devices and PIO resource on x86 is very limited, we expose
MMIO BAR.

Kernel supports both PIO  and MMIO BAR for legacy virtio-pci device.
We handles
different type of BAR in the similar way.

In previous implementation, with igb_uio we get PIO address from igb_uio
sysfs entry; with uio_pci_generic, we get PIO address from
/proc/ioports.
For PIO/MMIO RW, there is different path for different drivers and arch.
For VFIO, PIO/MMIO RW is through syscall, which has big performance
issue.

Regarding the performance issue, do you have some numbers to share?
AFAICS, it can only have an impact on performance when interrupt mode is
used or queue notification is enabled.

Does your HW Virtio implementation requires notification?

Yes, hardware needs notification to tell which queue has more buffer.

vhost backend also needs notification when it is not running in polling
mode.

It is easy for software backend to sync with frontend whether it needs
notification through memory but a big burden for hardware.

Yes, I understand, thanks for the clarification.


Anyway, using vfio ioctl isn't needed at all. virtio PMD is only the
consumer of pci_vfio_ioport_read.

My understanding is that using VFIO read/write ops is required for IOMMU
enabled case without cap_sys_rawio. And anyway, using inb/outb is just
bypassing VFIO. As I suggest in my other reply, it is better to document
that in the case of devices having PIO BARs, the user should consider
using UIO driver if performance is a concern.


Get it. so user could read/write PIO using VFIO without iopl permission, 
with some performance penalty.




we could consider if we still need pci_vfio_ioport_read related API in
future.

I disagree. I think the pci_vfio_ioport_* API is required at least for
the IOMMU enabled case.

Documentation is the way to go in my opinion, we can also add a warning
that performance may be degraded compared to UIO in
pci_vfio_ioport_map() when IOMMU is disabled if you think it may help
the users.

Thanks,
Maxime


/huawei

Is performance the only issue to have your HW working with Virtio PMD,
or is this series also fixing some functionnal issues?

Best regards,
Maxime

Re: [dpdk-dev] [PATCH 1/1] doc: simplify OS support in features matrix

[Thomas Monjalon]

> >> The networking drivers features matrix had rows to show
> >> OS and kernel modules support:
> >>- BSD nic_uio
> >>- Linux UIO
> >>- Linux VFIO
> >>- Other kdrv
> >>- Windows
> >>
> >> The kernel modules details are removed to keep only OS support:
> >>- FreeBSD
> >>- Linux
> >>- Windows
> >>
> >> Signed-off-by: Thomas Monjalon 
> > 
> > Acked-by: Ajit Khaparde 
> > Acked-by: Andrew Rybchenko 
> Acked-by: Ferruh Yigit 

Applied

Re: [dpdk-dev] [PATCH] net/mlx5: fix wrong Flow Tag decompression

[Slava Ovsiienko]

> -Original Message-
> From: Alexander Kozyrev 
> Sent: Thursday, January 14, 2021 23:32
> To: dev@dpdk.org
> Cc: sta...@dpdk.org; Raslan Darawsheh ; Slava
> Ovsiienko ; Matan Azrad 
> Subject: [PATCH] net/mlx5: fix wrong Flow Tag decompression
> 
> Packets can get a wrong Flow Tag on x86 architecture with the Flow Tag
> compression format (rxq_cqe_comp_en=2) enabled inside the SSE Rx burst.
> The shuffle mask that extracts a Flow Tag from the pair of compressed CQEs is
> reversed. This leads to the wrong Flow Tag assignment.
> Correct the shuffle mask to get proper bytes for a Flow Tag from miniCQEs.
> 
> Fixes: 54c2d46b160 ("net/mlx5: support flow tag and packet header
> miniCQEs")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Alexander Kozyrev 
Acked-by: Viacheslav Ovsiienko 

[dpdk-dev] [PATCH v1] net/ice: drain out DCF AdminQ command queue

[Haiyue Wang]

The virtchnl command message response is handled by matchiing the opcode
only for limitation.

The DCF AdminQ command with buffer data needs two virtchnl commands, one
is to handle the AdminQ header, the other is to handle AdminQ buffer. If
the AdminQ header command gets the failure response, the AdminQ buffer
command needs to wait for the buffer message response until timeout to
drain out the virtchnl command queue, since both of them are sent to PF,
the PF will handle them one by one, and send back the response. If not,
it will cause the next AdminQ command failure with the stall response.

Fixes: daa714d55c72 ("net/ice: handle AdminQ command by DCF")
Cc: sta...@dpdk.org

Signed-off-by: Haiyue Wang 
---
 drivers/net/ice/ice_dcf.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ice/ice_dcf.c b/drivers/net/ice/ice_dcf.c
index 4a9af3292c..a211797d9e 100644
--- a/drivers/net/ice/ice_dcf.c
+++ b/drivers/net/ice/ice_dcf.c
@@ -506,7 +506,6 @@ ice_dcf_send_aq_cmd(void *dcf_hw, struct ice_aq_desc *desc,
 
do {
if ((!desc_cmd.pending && !buff_cmd.pending) ||
-   (!desc_cmd.pending && desc_cmd.v_ret != IAVF_SUCCESS) ||
(!buff_cmd.pending && buff_cmd.v_ret != IAVF_SUCCESS))
break;
 
-- 
2.30.0

[dpdk-dev] [PATCH v2] net/ice: drain out DCF AdminQ command queue

[Haiyue Wang]

The virtchnl command message response is handled by matching the opcode
only for limitation.

The DCF AdminQ command with buffer data needs two virtchnl commands, one
is to handle the AdminQ header, the other is to handle AdminQ buffer. If
the AdminQ header command gets the failure response, the AdminQ buffer
command needs to wait for the buffer message response until timeout to
drain out the virtchnl command queue, since both of them are sent to PF,
the PF will handle them one by one, and send back the response. If not,
it will cause the next AdminQ command failure with the stall response.

Fixes: daa714d55c72 ("net/ice: handle AdminQ command by DCF")
Cc: sta...@dpdk.org

Signed-off-by: Haiyue Wang 
---
v2: Fix the commit message typo : 'matchiing' should be 'matching'
---
 drivers/net/ice/ice_dcf.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ice/ice_dcf.c b/drivers/net/ice/ice_dcf.c
index 4a9af3292c..a211797d9e 100644
--- a/drivers/net/ice/ice_dcf.c
+++ b/drivers/net/ice/ice_dcf.c
@@ -506,7 +506,6 @@ ice_dcf_send_aq_cmd(void *dcf_hw, struct ice_aq_desc *desc,
 
do {
if ((!desc_cmd.pending && !buff_cmd.pending) ||
-   (!desc_cmd.pending && desc_cmd.v_ret != IAVF_SUCCESS) ||
(!buff_cmd.pending && buff_cmd.v_ret != IAVF_SUCCESS))
break;
 
-- 
2.30.0

Re: [dpdk-dev] [PATCH v5 3/3] PCI: don't use vfio ioctl call to access PIO resource

[谢华伟(此时此刻）]

On 2021/1/21 16:29, Maxime Coquelin wrote:


On 1/20/21 3:54 PM, 谢华伟(此时此刻） wrote:

On 2021/1/13 0:58, Maxime Coquelin wrote:

On 1/12/21 10:37 AM, Maxime Coquelin wrote:

bus/pci: ...

On 10/22/20 5:51 PM, 谢华伟(此时此刻） wrote:

From: "huawei.xhw" 

VFIO should use the same way to map/read/write PORT IO as UIO, for
virtio PMD.

Please provide more details in the commit message on why the way VFIO
works today is wrong (The cover letter is lost once applied).

ok

Signed-off-by: huawei.xhw 

Same comment about name format as on previous patches.


---
   drivers/bus/pci/linux/pci.c | 8 
   drivers/bus/pci/linux/pci_uio.c | 4 +++-
   2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index 0dc99e9..2ed9f2b 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -687,7 +687,7 @@ int rte_pci_write_config(const struct
rte_pci_device *device,
   #ifdef VFIO_PRESENT
   case RTE_PCI_KDRV_VFIO:
   if (pci_vfio_is_enabled())
-    ret = pci_vfio_ioport_map(dev, bar, p);
+    ret = pci_uio_ioport_map(dev, bar, p);

Doesn't it create a regression with regards to needed capabilities?
My understanding is that before this patch we don't need to call iopl(),
whereas once applied it is required, correct?

I did some testing today, and think it is not a regression with para-
virtualized Virtio devices.

Indeed, I thought it would be a regression with Legacy devices when
IOMMU is enabled and the program is run as non-root (IOMMU enabled
just to suport IOVA as VA mode). But it turns out para-virtualized
Virtio legacy device and vIOMMU enabled is not a supported configuration
by QEMU.

Note that when noiommu mode is enabled, the app needs cap_sys_rawio, so
same as iopl(). No regression in this case too.

That said, with real (non para-virtualized) Virtio device using PIO like
yours, doesn't your patch introduce a restriction for your device that
it will require cap_sys_rawio whereas it would not be needed?

I don't catch the regression issue.

With real virtio device(hardware implemented), if it is using MMIO, no
cap_sys_rawio is required.

If it is using PIO, iopl is required always.

My understanding of the Kernel VFIO driver is that cap_sys_rawio is only
necessary in noiommu mode, i.e. when VFIO is loaded with
enable_unsafe_noiommu parameter set. The doc for this parameters seems
to validate my understanding of the code:
"
MODULE_PARM_DESC(enable_unsafe_noiommu_mode, "Enable UNSAFE, no-IOMMU
mode.  This mode provides no device isolation, no DMA translation, no
host kernel protection, cannot be used for device assignment to virtual
machines, requires RAWIO permissions, and will taint the kernel.  If you
do not know what this is for, step away. (default: false)");
"

I think that using inb/outb in the case of VFIO with IOMMU enabled won't
work without cap_sys_rawio, and using it in the case of VFIO with IOMMU
disabled just bypasses VFIO and so is not correct.


Get your concern.

PIO bar:

    HW virtio on HW machine: any vendor implements hardware virtio 
using PIO bar? I think this isn't right. And i dout if vfio doesn't 
check rawio perssion in the syscall in this case.


    Para virtio:  you have no choice to enable unsafe no-iommu mode.  
You must have RAWIO permission.


so with PIO bar, the regression doesn't exist in real world.

Btw, our virtio device is basically MMIO bar, either in hardware machine 
or in pass-throughed virtual machine.



Do you mean we apply or abandon patch 3? I am both OK. The first 
priority to me is to enable MMIO bar support.




In my opinion, what we should do is to add something like this in the
DPDK documentation:

  - MMIO BAR: VFIO with IOMMU enabled recommended. Equivalent performance
as with IGB UIO or VFIO with NOIOMMU. VFIO with IOMMU is recommended for
security reasons.
  - PIO BAR: VFIO with IOMMU enabled is recommended for security reasons,
providing proper isolation and not requiring cap_sys_rawio. However, use
of IOMMU is not always possible in some cases (e.g. para-virtualized
Virtio-net legacy device). Also, performance of using VFIO for PIO BARs
accesses has an impact on performance as it uses pread/pwrite syscalls,
whereas UIO drivers use inb/outb. If security is not a concern or IOMMU
is not available, one might consider using UIO driver in this case for
performance reasons.

What do you think?

Thanks,
Maxime


Regards,
Maxime

Re: [dpdk-dev] [PATCH v5 3/3] PCI: don't use vfio ioctl call to access PIO resource

[谢华伟(此时此刻）]

"

I think that using inb/outb in the case of VFIO with IOMMU enabled won't
work without cap_sys_rawio, and using it in the case of VFIO with IOMMU
disabled just bypasses VFIO and so is not correct.


Get your concern.

PIO bar:

    HW virtio on HW machine: any vendor implements hardware virtio 
using PIO bar? I think this isn't right. And i dout if vfio doesn't 
check rawio perssion in the syscall in this case.


    Para virtio:  you have no choice to enable unsafe no-iommu mode.  
You must have RAWIO permission.

Sorry. typo.  "you have no choice but to enable unsafe no-iommu mode. "


so with PIO bar, the regression doesn't exist in real world.

Btw, our virtio device is basically MMIO bar, either in hardware 
machine or in pass-throughed virtual machine.



Do you mean we apply or abandon patch 3? I am both OK. The first 
priority to me is to enable MMIO bar support.

Re: [dpdk-dev] [PATCH v15 11/12] build: add Arm SoC meson option

[Juraj Linkeš]

> -Original Message-
> From: Thomas Monjalon 
> Sent: Wednesday, January 20, 2021 5:10 PM
> To: Honnappa Nagarahalli ; Bruce
> Richardson ; Juraj Linkeš
> 
> Cc: Ruifeng Wang ; Phil Yang
> ; vcchu...@amazon.com; Dharmik Thakkar
> ; jerinjac...@gmail.com;
> hemant.agra...@nxp.com; Ajit Khaparde (ajit.khapa...@broadcom.com)
> ; ferruh.yi...@intel.com; abo...@pensando.io;
> dev@dpdk.org; nd 
> Subject: Re: [dpdk-dev] [PATCH v15 11/12] build: add Arm SoC meson option
> 
> 20/01/2021 09:41, Juraj Linkeš:
> > From: Honnappa Nagarahalli 
> > > > 20/01/2021 02:04, Honnappa Nagarahalli:
> > > > > > On Tue, Jan 19, 2021 at 04:52:19PM +0100, Thomas Monjalon wrote:
> > > > > > > 19/01/2021 15:56, Juraj Linkeš:
> > > > > > > > From: Thomas Monjalon 
> > > > > > > > > 15/01/2021 14:26, Juraj Linkeš:
> > > > > > > > > > --- a/meson_options.txt
> > > > > > > > > > +++ b/meson_options.txt
> > > > > > > > > > +option('arm_soc', type: 'string', value: '',
> > > > > > > > > > +   description: 'Specify if you want to build for a
> > > > > > > > > > +particular
> > > > > > > > > > +aarch64 Arm SoC when building on an aarch64
> > > > > > > > > > +machine.')
> > > > > > > > >
> > > > > > > > > Why the option is named "arm_soc" and not just "soc"?
> > > > > > > > > The same option could be used by other archs, isn't it?
> > > > > > > >
> > > > > > > > Agree that a more generic name would be better.
> > > > > > > > I'll change it to "soc" if there are no other suggestions.
> > > > > > >
> > > > > > > Another name could be "machine".
> > > > > > > Should it be the same mechanism as compiling for a specific
> > > > > > > x86 CPU from an x86 machine?
> > > > > > >
> > > > > > I'd rather not re-use the term "machine", for a new use,
> > > > > > better to use a new term IMHO.
> > > > > +1, agree. 'soc' sounds good to me.
> > > >
> > > > Another possible word is "platform", as in
> > > > http://doc.dpdk.org/guides/platform/index.html
> > > I am fine with 'platform' too.
> > >
> >
> > 'platform' is likely the best and actually works nicely with
> http://patches.dpdk.org/patch/85956/. Taken together, 'platform' could be
> either 'native', 'generic' or an soc, which is, I believe, exactly what we 
> want.
> 
> I am not sure what we want :)
> We need to specify the instruction set, and the specific target.
> We could deduce the instruction set from the target, but I think it is good 
> to be
> able to overwrite the instruction set in case there can be multiple 
> instruction
> sets for a target.
> 

I think we had this (or similar) discussion here 
http://patches.dpdk.org/patch/85956/. I agree with your summary.

> I think "native" and "generic" should be specified as instruction set, in the
> existing option "machine" or renamed as "instruction_set" or "isa".
> 

Agree, but I would add that we also want "native" and "generic" as valid 
platforms. More below.

> Let's imagine the first option is "isa" and the new second option is 
> "platform".
> We can have a default "isa" per "platform".
> The default "platform" would have a default "isa": native or generic?
> 

In general, yes, but I let me expand the "platform" option a bit.

Let's dig into what "platform" means. I understand it to be a set of 
configuration options, e.g.:
platform=native: use discovered values for all configuration options (where we 
can do that)
platform=generic: use predetermined values to produce a "generic" build that 
would work on most machine of the corresponding type/arch
platform=other: use predetermined values to produce a build tailored to 
platform "other", such as some arm soc.

In all these cases, leave user the option to specify supported options (i.e. 
user can specifying "instruction_set" and that value would be used for machine 
args or "max_lcores" would set max lcores).

The default "instruction_set" would be different per platform:
platform=native => instruction_set=native
platform=generic => the generic instruction_set for the architecture, as 
specified here: https://github.com/DPDK/dpdk/blob/main/config/meson.build#L79
platform=other => the instruction set of the platform

When "instruction_set" is set to its default value (such as auto), the 
per-platform instruction set would be used. If the user specifies anything 
else, that value would be used.

I basically just reworded Bruce's proposal from the other thread, since I agree 
with it.

Using the "platform" option in this commit just extends the supported platforms 
(from "native" and "default") by adding the soc platforms. (or the other patch 
would extend the supported platforms, depending on in which order the patches 
would be applied)

> What else do we need?
> 
> 

I think the above proposal (actually implemented here: 
http://patches.dpdk.org/patch/85956/) gives us what we want, which I believe is 
this (which is basically your summary + maybe some extra thoughts):
1. Arm wants to have the ability to choose a configuration set (i.e. the 
"platform" option). We also refer to th

Re: [dpdk-dev] [PATCH v4 1/3] build: add aarch64 clang to meson cross-compile

[Juraj Linkeš]

> -Original Message-
> From: Thomas Monjalon 
> Sent: Wednesday, January 20, 2021 11:35 AM
> To: Juraj Linkeš 
> Cc: david.march...@redhat.com; acon...@redhat.com;
> maicolgabr...@hotmail.com; jerinjac...@gmail.com; ferruh.yi...@intel.com;
> ruifeng.w...@arm.com; dev@dpdk.org; bruce.richard...@intel.com
> Subject: Re: [dpdk-dev] [PATCH v4 1/3] build: add aarch64 clang to meson 
> cross-
> compile
> 
> 20/01/2021 11:30, Juraj Linkeš:
> > From: Thomas Monjalon 
> > > 20/01/2021 09:24, Juraj Linkeš:
> > > > From: Thomas Monjalon 
> > > > > 19/01/2021 09:33, Juraj Linkeš:
> > > > > > Create meson cross-file arm64_armv8_linux_clang_ubuntu1804.
> > > > >
> > > > > Why is it specific to Ubuntu 18.04?
> > > > > I don't want to add specifc cross files per distributions.
> > > > >
> > > > > > Use clang/LLVM toolchain with sysroot pointing to gcc cross stdlib.
> > > > > >
> > > > > > The sysroot path must be in the cross-file so that Clang can
> > > > > > find the proper headers:
> > > > > > * setting CFLAGS, LDFLAGS or -Dc_args, -Dc_link_args doesn't
> > > > > > affect cross builds (only native builds). Support added in 0.51.0.
> > > > > > * setting pkg-config vars only affects lib searching, not
> > > > > > includes
> > > > > > * splitting the cross-file into two (one with clang info, one
> > > > > > with
> > > > > > paths) doesn't work. Support added in 0.52.0.
> > > > >
> > > > > I don't understand the explanations above.
> > > > > Please explain what is the bug and how it is fixed.
> > > > >
> > > >
> > > > I guess the missing piece is that the sysroot path is the ubuntu 
> > > > specific part.
> > > The explanations illustrate why we can't have a generic cross file
> > > with the current meson version - there's no way to pass the paths to
> > > cross builds. Now that I think about it, the commit message needs a
> > > rewrite - I should've mentioned that clang/llvm doesn't provide it's
> > > own standard c lib, so that has to come from elsewhere (such as gcc) and
> thus we have to provide the paths.
> > >
> > > Can it be done with the option -Dc_args?
> > >
> >
> > Not in Meson 0.47.1, it's explained in the commit msg:
> > * setting CFLAGS, LDFLAGS or -Dc_args, -Dc_link_args doesn't affect
> > cross builds (only native builds). Support added in 0.51.0.
> 
> So I suggest to make it clearer :)
> One proposal:
> 
> * setting CFLAGS/LDFLAGS variables or using -Dc_args/-Dc_link_args options
>   do not work on cross builds until Meson 0.51.0.
> 

Ok, I'll reword the commit message to make it more clearer based on your 
feedback.

> > > > > [...]
> > > > > > +c_args = ['-target', 'aarch64-linux-gnu', '--sysroot',
> > > > > > +'/usr/aarch64-linux-gnu'] c_link_args = ['-target',
> > > > > > +'aarch64-linux-gnu', '-fuse-ld=lld', '--gcc-toolchain=/usr']
> > >
> > >
> > >
> >
> >
> 
> 
> 
> 
> 

Re: [dpdk-dev] [PATCH v1] devtools: update abi ignore for cryptodev

[Dodji Seketeli]

Hello Thomas and others,

Thomas Monjalon  writes:

> Question to an expert, Dodji,

Thanks for the kind words, but I am not an expert in anything, sadly.  I
am just trying to keep learning about these things ;-)

> We have this structure:
>
> struct rte_cryptodev {
>   lot of fields...
>   uint8_t attached : 1;
> } __rte_cache_aligned;
>
> Because of the cache alignment, there is enough padding in the struct
> (no matter the size of the cache line) for adding two more pointers:
>
> struct rte_cryptodev {
>   lot of fields...
>   uint8_t attached : 1;
>   struct rte_cryptodev_cb_rcu *enq_cbs;
>   struct rte_cryptodev_cb_rcu *deq_cbs;
> } __rte_cache_aligned;
>
> We checked manually that the ABI is still compatible.

Right.

I am curious, but normally, libabigail should raise the addition of
structures, but then it'll tell you that there was no size or offset
change between the two structures.  If it doesn't, then that's a bug.  I
hope it does :-)


> Then I've added (quickly) a libabigail exception rule:
>
> [suppress_type]
>   name = rte_cryptodev
>   has_data_member_inserted_between = {0, 1023}
>
> Now we want to improve this rule to restrict the offsets
> to the padding at the end of the struct only,
> so we keep forbidding changes in existing fields,
> and forbidding additions further the current struct size.
> Is this new rule good?
>
>   has_data_member_inserted_between = {offset_after(attached), end}


Yes, this rule should do what you think it says.

> Do you confirm that the keyword "end" means the old reference size?

Yes I do.


> What else do we need to check for adding a new field in a padding?

Actually, that rule will work independantly of it there is enough
padding or not.  It'll shut down the change report, even if the added
data exceeds the padding.

You just made me think of an idea of a new feature there.

Maybe we'd need a new property for the [suppress_type] directive that
would suppress changes only if said changes don't modify the size of the
type or any offset of any member of the type?

Maybe something like:

[suppress_type]
   ; lots of properties can go here.

   ; ...

   ; If the type has any size or offset change
   ; then this suppression directive will fail
   ; and the change report will be emitted
   has_no_size_or_offset_change

Would that be useful to you in this case,

Cheers,

-- 
Dodji

Re: [dpdk-dev] [PATCH v2 00/19] ensure headers have correct includes

[Bruce Richardson]

On Thu, Jan 21, 2021 at 10:36:18AM +0100, Thomas Monjalon wrote:
> 21/01/2021 10:33, Bruce Richardson:
> > On Thu, Jan 21, 2021 at 10:25:07AM +0100, David Marchand wrote:
> > > Is rte_byteorder.h inclusion in rte_mbuf.h still necessary?
> > >
> > Good question. The checks I was doing were only for missing headers.
> > Checking for superfluous headers is more complicated and not something I've
> > looked at. I know it was previously suggested to use the
> > "include-what-you-use" tool on DPDK, if anyone wants to attempt that.
> 
> I would love having an integration of include-what-you-use in our build 
> system.
> I don't know whether it's trivial or difficult with meson.
> 

Did some initial investigation here, and here are some basic instructions
based on how I got it running on my Ubuntu 20.10 system. The positive is
that "iwyu_tool" can run on DPDK build dirs without any extra work on our
part.

1. Install iwyu from Ubuntu packaging.
2. Install clang-9 package:
* This is not a dependency of iwyu, but probably should be
* The version of clang installed *must* match that used to build iwyu
  [I had clang-10 installed which didn't work for this]
* If you get errors about missing standard headers e.g. stddef.h,
  the version mismatch is likely the issue.
3. Configure a clang build of DPDK:
* CC=clang meson  build-clang
4. "iwyu_tool" supports the build database format used by meson, so just
   run: "iwyu_tool -p build-clang"

Be warned, this runs really slowly and produces lots of output!

Regards,
/Bruce

Re: [dpdk-dev] [PATCH v5 3/3] PCI: don't use vfio ioctl call to access PIO resource

[Maxime Coquelin]

On 1/21/21 3:57 PM, 谢华伟(此时此刻） wrote:
> 
> On 2021/1/21 16:29, Maxime Coquelin wrote:
>>
>> On 1/20/21 3:54 PM, 谢华伟(此时此刻） wrote:
>>> On 2021/1/13 0:58, Maxime Coquelin wrote:
 On 1/12/21 10:37 AM, Maxime Coquelin wrote:
> bus/pci: ...
>
> On 10/22/20 5:51 PM, 谢华伟(此时此刻） wrote:
>> From: "huawei.xhw" 
>>
>> VFIO should use the same way to map/read/write PORT IO as UIO, for
>> virtio PMD.
> Please provide more details in the commit message on why the way VFIO
> works today is wrong (The cover letter is lost once applied).
>>> ok
>> Signed-off-by: huawei.xhw 
> Same comment about name format as on previous patches.
>
>> ---
>>    drivers/bus/pci/linux/pci.c | 8 
>>    drivers/bus/pci/linux/pci_uio.c | 4 +++-
>>    2 files changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/bus/pci/linux/pci.c
>> b/drivers/bus/pci/linux/pci.c
>> index 0dc99e9..2ed9f2b 100644
>> --- a/drivers/bus/pci/linux/pci.c
>> +++ b/drivers/bus/pci/linux/pci.c
>> @@ -687,7 +687,7 @@ int rte_pci_write_config(const struct
>> rte_pci_device *device,
>>    #ifdef VFIO_PRESENT
>>    case RTE_PCI_KDRV_VFIO:
>>    if (pci_vfio_is_enabled())
>> -    ret = pci_vfio_ioport_map(dev, bar, p);
>> +    ret = pci_uio_ioport_map(dev, bar, p);
> Doesn't it create a regression with regards to needed capabilities?
> My understanding is that before this patch we don't need to call
> iopl(),
> whereas once applied it is required, correct?
 I did some testing today, and think it is not a regression with para-
 virtualized Virtio devices.

 Indeed, I thought it would be a regression with Legacy devices when
 IOMMU is enabled and the program is run as non-root (IOMMU enabled
 just to suport IOVA as VA mode). But it turns out para-virtualized
 Virtio legacy device and vIOMMU enabled is not a supported
 configuration
 by QEMU.

 Note that when noiommu mode is enabled, the app needs cap_sys_rawio, so
 same as iopl(). No regression in this case too.

 That said, with real (non para-virtualized) Virtio device using PIO
 like
 yours, doesn't your patch introduce a restriction for your device that
 it will require cap_sys_rawio whereas it would not be needed?
>>> I don't catch the regression issue.
>>>
>>> With real virtio device(hardware implemented), if it is using MMIO, no
>>> cap_sys_rawio is required.
>>>
>>> If it is using PIO, iopl is required always.
>> My understanding of the Kernel VFIO driver is that cap_sys_rawio is only
>> necessary in noiommu mode, i.e. when VFIO is loaded with
>> enable_unsafe_noiommu parameter set. The doc for this parameters seems
>> to validate my understanding of the code:
>> "
>> MODULE_PARM_DESC(enable_unsafe_noiommu_mode, "Enable UNSAFE, no-IOMMU
>> mode.  This mode provides no device isolation, no DMA translation, no
>> host kernel protection, cannot be used for device assignment to virtual
>> machines, requires RAWIO permissions, and will taint the kernel.  If you
>> do not know what this is for, step away. (default: false)");
>> "
>>
>> I think that using inb/outb in the case of VFIO with IOMMU enabled won't
>> work without cap_sys_rawio, and using it in the case of VFIO with IOMMU
>> disabled just bypasses VFIO and so is not correct.
> 
> Get your concern.
> 
> PIO bar:
> 
>     HW virtio on HW machine: any vendor implements hardware virtio using
> PIO bar? I think this isn't right. And i dout if vfio doesn't check
> rawio perssion in the syscall in this case.

I checked VFIO code, and it only check for rawio permission if noiommu
mode is enabled.

>     Para virtio:  you have no choice to enable unsafe no-iommu mode. 
> You must have RAWIO permission.
> 
> so with PIO bar, the regression doesn't exist in real world.
>
> 
> Btw, our virtio device is basically MMIO bar, either in hardware machine
> or in pass-throughed virtual machine.

OK, that thing was not clear to me.

> 
> Do you mean we apply or abandon patch 3? I am both OK. The first
> priority to me is to enable MMIO bar support.

OK, so yes, I think we should abandon patch 2 and patch 3.
For patch 1, it looks valid to me, but I'll let Ferruh decide.

For your device, if my understanding is correct, what we need to do is
to support MMIO for legacy devices. Correct?

If so, the change should be in virtio_pci.c. In vtpci_init(), after
modern detection has failed, we should check the the BAR is PIO or MMIO
based on the flag. the result can be saved in struct virtio_pci_dev.


We would introduce new wrappers like vtpci_legacy_read,
vtpci_legacy_write that would either call rte_pci_ioport_read,
rte_pci_ioport_read in case of PIO, or rte_read32, rte_write32 in case
of MMIO.

It is not too late for this release, as the change will not be that
intrusive. But if you prepare such patch, please base it on top of my
virtio rew

[dpdk-dev] [PATCH v5 0/3] aarch64 clang cross compilation

[Juraj Linkeš]

Use clang/LLVM toolchain with gcc stdlib to cross compile aarch64 target.

v3:
Removed AARCH_GCC and AARCH_CLANG variables in favor of CC_FOR_BUILD.

v4:
Rebased.
Updated commit msg in build: add aarch64 clang to meson cross-compile.

v5:
Rebased.
Reworded commit msg in build: add aarch64 clang to meson cross-compile.

Juraj Linkeš (3):
  build: add aarch64 clang to meson cross-compile
  ci: add aarch64 clang cross-compilation builds
  doc: add clang to aarch64 cross build guide

 .ci/linux-build.sh|   6 +-
 .travis.yml   |  18 ++
 config/arm/arm64_armv8_linux_clang_ubuntu1804 |  20 ++
 .../linux_gsg/cross_build_dpdk_for_arm64.rst  | 183 +-
 4 files changed, 175 insertions(+), 52 deletions(-)
 create mode 100644 config/arm/arm64_armv8_linux_clang_ubuntu1804

-- 
2.20.1

[dpdk-dev] [PATCH v5 1/3] build: add aarch64 clang to meson cross-compile

[Juraj Linkeš]

Create distribution specific meson cross-file
arm64_armv8_linux_clang_ubuntu1804. The file is distribution specific
because it contains paths to headers and libs specific to the
distribution. The clang/LLVM toolchain does not provide its own c stdlib
so the paths must be supplied in some manner.

In the current version of meson, v0.47.1, the only place
where the paths can be specified is the cross-file. Other possibilities
do not work:
* setting CFLAGS, LDFLAGS only sets these for non-cross builds.
* setting -Dc_args, -Dc_link_args on the command line also only sets
these for non-cross builds. Support for specifying these for
cross builds was added in v0.51.0 [0].
* the cross-file can't be split into generic clang cross config and
distribution specific config. Support added in v0.52.0 [1].

[0] https://mesonbuild.com/Builtin-options.html#specifying-options-per-machine
[1] https://mesonbuild.com/Machine-files.html#loading-multiple-machine-files

Signed-off-by: Juraj Linkeš 
Reviewed-by: Ruifeng Wang 
---
 config/arm/arm64_armv8_linux_clang_ubuntu1804 | 20 +++
 1 file changed, 20 insertions(+)
 create mode 100644 config/arm/arm64_armv8_linux_clang_ubuntu1804

diff --git a/config/arm/arm64_armv8_linux_clang_ubuntu1804 
b/config/arm/arm64_armv8_linux_clang_ubuntu1804
new file mode 100644
index 0..aa5ee0132
--- /dev/null
+++ b/config/arm/arm64_armv8_linux_clang_ubuntu1804
@@ -0,0 +1,20 @@
+[binaries]
+c = 'clang'
+cpp = 'clang++'
+ar = 'llvm-ar'
+strip = 'llvm-strip'
+llvm-config = 'llvm-config'
+pcap-config = 'llvm-config'
+pkgconfig = 'aarch64-linux-gnu-pkg-config'
+
+[host_machine]
+system = 'linux'
+cpu_family = 'aarch64'
+cpu = 'armv8-a'
+endian = 'little'
+
+[properties]
+implementor_id = 'generic'
+implementor_pn = 'default'
+c_args = ['-target', 'aarch64-linux-gnu', '--sysroot', 
'/usr/aarch64-linux-gnu']
+c_link_args = ['-target', 'aarch64-linux-gnu', '-fuse-ld=lld', 
'--gcc-toolchain=/usr']
-- 
2.20.1

[dpdk-dev] [PATCH v5 2/3] ci: add aarch64 clang cross-compilation builds

[Juraj Linkeš]

Mirror the existing gcc jobs - build static and shared libs.
Use arm64_armv8_linux_clang_ubuntu1804 meson cross file.

Signed-off-by: Juraj Linkeš 
---
 .ci/linux-build.sh |  6 +-
 .travis.yml| 18 ++
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index d2c821adf..afa3689a0 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -32,7 +32,11 @@ install_libabigail() {
 
 if [ "$AARCH64" = "true" ]; then
 # convert the arch specifier
-OPTS="$OPTS --cross-file config/arm/arm64_armv8_linux_gcc"
+if [ "$CC_FOR_BUILD" = "gcc" ]; then
+   OPTS="$OPTS --cross-file config/arm/arm64_armv8_linux_gcc"
+elif [ "$CC_FOR_BUILD" = "clang" ]; then
+   OPTS="$OPTS --cross-file config/arm/arm64_armv8_linux_clang_ubuntu1804"
+fi
 fi
 
 if [ "$BUILD_DOCS" = "true" ]; then
diff --git a/.travis.yml b/.travis.yml
index 5aa7ad49f..ad6e956be 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -21,6 +21,10 @@ _aarch64_packages: &aarch64_packages
   - *required_packages
   - [gcc-aarch64-linux-gnu, libc6-dev-arm64-cross, 
pkg-config-aarch64-linux-gnu]
 
+_aarch64_clang_packages: &aarch64_clang_packages
+  - *required_packages
+  - [libgcc-7-dev-arm64-cross, libatomic1-arm64-cross, libc6-dev-arm64-cross, 
pkg-config-aarch64-linux-gnu]
+
 _libabigail_build_packages: &libabigail_build_packages
   - [autoconf, automake, libtool, pkg-config, libxml2-dev, libdw-dev]
 
@@ -102,6 +106,20 @@ jobs:
   apt:
 packages:
   - *aarch64_packages
+  - env: DEF_LIB="static" AARCH64=true
+arch: amd64
+compiler: clang
+addons:
+  apt:
+packages:
+  - *aarch64_clang_packages
+  - env: DEF_LIB="shared" AARCH64=true
+arch: amd64
+compiler: clang
+addons:
+  apt:
+packages:
+  - *aarch64_clang_packages
   # aarch64 gcc jobs
   - env: DEF_LIB="static"
 arch: arm64
-- 
2.20.1

[dpdk-dev] [PATCH v5 3/3] doc: add clang to aarch64 cross build guide

[Juraj Linkeš]

Reorganize and update the aarch64 cross guide with clang cross
compilation. Update the GNU toolchain version which clang also uses.
Reorganize into common part, GNU part and clang part.

Signed-off-by: Juraj Linkeš 
Acked-by: Ruifeng Wang 
---
 .../linux_gsg/cross_build_dpdk_for_arm64.rst  | 183 +-
 1 file changed, 132 insertions(+), 51 deletions(-)

diff --git a/doc/guides/linux_gsg/cross_build_dpdk_for_arm64.rst 
b/doc/guides/linux_gsg/cross_build_dpdk_for_arm64.rst
index d0d346c83..ed34cdb0f 100644
--- a/doc/guides/linux_gsg/cross_build_dpdk_for_arm64.rst
+++ b/doc/guides/linux_gsg/cross_build_dpdk_for_arm64.rst
@@ -1,104 +1,185 @@
 ..  SPDX-License-Identifier: BSD-3-Clause
-Copyright(c) 2018 ARM Corporation.
+Copyright(c) 2020 ARM Corporation.
 
-Cross compile DPDK for ARM64
-
+Cross compiling DPDK for ARM64
+==
 This chapter describes how to cross compile DPDK for ARM64 from x86 build 
hosts.
 
 .. note::
 
Whilst it is recommended to natively build DPDK on ARM64 (just
-   like with x86), it is also possible to cross-build DPDK for ARM64. An
-   ARM64 cross compile GNU toolchain is used for this.
+   like with x86), it is also possible to cross compile DPDK for ARM64.
+   An ARM64 cross compiler GNU toolchain or an LLVM/clang toolchain
+   may be used for cross-compilation.
 
-Obtain the cross tool chain

-The latest cross compile tool chain can be downloaded from:
+
+Prerequisites
+-
+
+NUMA library
+
+
+NUMA is required by most modern machines, not needed for non-NUMA 
architectures.
+
+.. note::
+
+   For compiling the NUMA lib, run libtool --version to ensure the libtool 
version >= 2.2,
+   otherwise the compilation will fail with errors.
+
+.. code-block:: console
+
+   git clone https://github.com/numactl/numactl.git
+   cd numactl
+   git checkout v2.0.13 -b v2.0.13
+   ./autogen.sh
+   autoconf -i
+   ./configure --host=aarch64-linux-gnu CC= --prefix=
+   make install
+
+.. note::
+
+   The compiler above can be either aarch64-linux-gnu-gcc or clang.
+   See below for information on how to get the compiler of your choice.
+
+The numa header files and lib file is generated in the include and lib folder 
respectively under .
+
+Meson prerequisites
+~~~
+
+Meson depends on pkgconfig to find the dependencies.
+The package ``pkg-config-aarch64-linux-gnu`` is required for aarch64.
+To install it in Ubuntu::
+
+   sudo apt install pkg-config-aarch64-linux-gnu
+
+
+GNU toolchain
+-
+
+.. _obtain_GNU_toolchain:
+
+Obtain the cross toolchain
+~~
+The latest GNU cross compiler toolchain can be downloaded from:
 https://developer.arm.com/open-source/gnu-toolchain/gnu-a/downloads.
 
 It is always recommended to check and get the latest compiler tool from the 
page and use
-it to generate better code. As of this writing 8.3-2019.03 is the newest, the 
following
+it to generate better code. As of this writing 9.2-2019.12 is the newest, the 
following
 description is an example of this version.
 
 .. code-block:: console
 
-   wget 
https://developer.arm.com/-/media/Files/downloads/gnu-a/8.3-2019.03/binrel/gcc-arm-8.3-2019.03-x86_64-aarch64-linux-gnu.tar.xz
+   wget 
https://developer.arm.com/-/media/Files/downloads/gnu-a/9.2-2019.12/binrel/gcc-arm-9.2-2019.12-x86_64-aarch64-none-linux-gnu.tar.xz
 
 Unzip and add into the PATH

+~~~
 
 .. code-block:: console
 
-   tar -xvf gcc-arm-8.3-2019.03-x86_64-aarch64-linux-gnu.tar.xz
-   export 
PATH=$PATH:/gcc-arm-8.3-2019.03-x86_64-aarch64-linux-gnu/bin
+   tar -xvf gcc-arm-9.2-2019.12-x86_64-aarch64-none-linux-gnu.tar.xz
+   export 
PATH=$PATH:/gcc-arm-9.2-2019.12-x86_64-aarch64-none-linux-gnu/bin
 
 .. note::
 
For the host requirements and other info, refer to the release note 
section: https://releases.linaro.org/components/toolchain/binaries/
 
-.. _arm_cross_build_getting_the_prerequisite_library:
+.. _augment_the_gnu_toolchain_with_numa_support:
 
-Getting the prerequisite library
-
-
-NUMA is required by most modern machines, not needed for non-NUMA 
architectures.
+Augment the GNU toolchain with NUMA support
+~~~
 
 .. note::
 
-   For compiling the NUMA lib, run libtool --version to ensure the libtool 
version >= 2.2,
-   otherwise the compilation will fail with errors.
+   This way is optional, an alternative is to use extra CFLAGS and LDFLAGS.
+
+Copy the NUMA header files and lib to the cross compiler's directories:
 
 .. code-block:: console
 
-   git clone https://github.com/numactl/numactl.git
-   cd numactl
-   git checkout v2.0.13 -b v2.0.13
-   ./autogen.sh
-   autoconf -i
-   ./configure --host=aarch64-linux-gnu CC=aarch64-linux-gnu-gcc 
--prefix=
-   make install
+   cp /include/numa*.h 
/gcc-arm-9.2-2019.12-x86_64-aarch64-none-linux-

Re: [dpdk-dev] [PATCH v15 11/12] build: add Arm SoC meson option

[Thomas Monjalon]

21/01/2021 16:02, Juraj Linkeš:
> From: Thomas Monjalon 
> > 20/01/2021 09:41, Juraj Linkeš:
> > > From: Honnappa Nagarahalli 
> > > > > 20/01/2021 02:04, Honnappa Nagarahalli:
> > > > > > > On Tue, Jan 19, 2021 at 04:52:19PM +0100, Thomas Monjalon wrote:
> > > > > > > > 19/01/2021 15:56, Juraj Linkeš:
> > > > > > > > > From: Thomas Monjalon 
> > > > > > > > > > 15/01/2021 14:26, Juraj Linkeš:
> > > > > > > > > > > --- a/meson_options.txt
> > > > > > > > > > > +++ b/meson_options.txt
> > > > > > > > > > > +option('arm_soc', type: 'string', value: '',
> > > > > > > > > > > + description: 'Specify if you want to build for a
> > > > > > > > > > > +particular
> > > > > > > > > > > +aarch64 Arm SoC when building on an aarch64
> > > > > > > > > > > +machine.')
> > > > > > > > > >
> > > > > > > > > > Why the option is named "arm_soc" and not just "soc"?
> > > > > > > > > > The same option could be used by other archs, isn't it?
> > > > > > > > >
> > > > > > > > > Agree that a more generic name would be better.
> > > > > > > > > I'll change it to "soc" if there are no other suggestions.
> > > > > > > >
> > > > > > > > Another name could be "machine".
> > > > > > > > Should it be the same mechanism as compiling for a specific
> > > > > > > > x86 CPU from an x86 machine?
> > > > > > > >
> > > > > > > I'd rather not re-use the term "machine", for a new use,
> > > > > > > better to use a new term IMHO.
> > > > > > +1, agree. 'soc' sounds good to me.
> > > > >
> > > > > Another possible word is "platform", as in
> > > > > http://doc.dpdk.org/guides/platform/index.html
> > > > I am fine with 'platform' too.
> > > >
> > >
> > > 'platform' is likely the best and actually works nicely with
> > http://patches.dpdk.org/patch/85956/. Taken together, 'platform' could be
> > either 'native', 'generic' or an soc, which is, I believe, exactly what we 
> > want.
> > 
> > I am not sure what we want :)
> > We need to specify the instruction set, and the specific target.
> > We could deduce the instruction set from the target, but I think it is good 
> > to be
> > able to overwrite the instruction set in case there can be multiple 
> > instruction
> > sets for a target.
> > 
> 
> I think we had this (or similar) discussion here 
> http://patches.dpdk.org/patch/85956/. I agree with your summary.
> 
> > I think "native" and "generic" should be specified as instruction set, in 
> > the
> > existing option "machine" or renamed as "instruction_set" or "isa".
> > 
> 
> Agree, but I would add that we also want "native" and "generic" as valid 
> platforms. More below.
> 
> > Let's imagine the first option is "isa" and the new second option is 
> > "platform".
> > We can have a default "isa" per "platform".
> > The default "platform" would have a default "isa": native or generic?
> > 
> 
> In general, yes, but I let me expand the "platform" option a bit.
> 
> Let's dig into what "platform" means. I understand it to be a set of 
> configuration options, e.g.:
> platform=native: use discovered values for all configuration options (where 
> we can do that)
> platform=generic: use predetermined values to produce a "generic" build that 
> would work on most machine of the corresponding type/arch

This is where I was disagreeing:
you propose to have 2 special values of platform (native and generic),
I propose to have only 1 default value for platform.

After more thoughts, I think it's fine.


> platform=other: use predetermined values to produce a build tailored to 
> platform "other", such as some arm soc.
> 
> In all these cases, leave user the option to specify supported options (i.e. 
> user can specifying "instruction_set" and that value would be used for 
> machine args or "max_lcores" would set max lcores).
> 
> The default "instruction_set" would be different per platform:

What do you think about calling this option "isa"?

> platform=native => instruction_set=native
> platform=generic => the generic instruction_set for the architecture, as 
> specified here: https://github.com/DPDK/dpdk/blob/main/config/meson.build#L79
> platform=other => the instruction set of the platform
> 
> When "instruction_set" is set to its default value (such as auto), the 
> per-platform instruction set would be used. If the user specifies anything 
> else, that value would be used.

Why auto? This is what we call native.

> I basically just reworded Bruce's proposal from the other thread, since I 
> agree with it.
> 
> Using the "platform" option in this commit just extends the supported 
> platforms (from "native" and "default") by adding the soc platforms. (or the 
> other patch would extend the supported platforms, depending on in which order 
> the patches would be applied)
> 
> > What else do we need?
> > 
> > 
> 
> I think the above proposal (actually implemented here: 
> http://patches.dpdk.org/patch/85956/) gives us what we want, which I believe 
> is this (which is basically your summary + maybe some extra thoughts):
> 1. Arm wants to have the ability

Re: [dpdk-dev] [PATCH v1] devtools: update abi ignore for cryptodev

[Thomas Monjalon]

21/01/2021 16:15, Dodji Seketeli:
> Hello Thomas and others,
> 
> Thomas Monjalon  writes:
> 
> > Question to an expert, Dodji,
> 
> Thanks for the kind words, but I am not an expert in anything, sadly.  I
> am just trying to keep learning about these things ;-)
> 
> > We have this structure:
> >
> > struct rte_cryptodev {
> > lot of fields...
> > uint8_t attached : 1;
> > } __rte_cache_aligned;
> >
> > Because of the cache alignment, there is enough padding in the struct
> > (no matter the size of the cache line) for adding two more pointers:
> >
> > struct rte_cryptodev {
> > lot of fields...
> > uint8_t attached : 1;
> > struct rte_cryptodev_cb_rcu *enq_cbs;
> > struct rte_cryptodev_cb_rcu *deq_cbs;
> > } __rte_cache_aligned;
> >
> > We checked manually that the ABI is still compatible.
> 
> Right.
> 
> I am curious, but normally, libabigail should raise the addition of
> structures, but then it'll tell you that there was no size or offset
> change between the two structures.  If it doesn't, then that's a bug.  I
> hope it does :-)

Yes it was raising a problem, that's why we are adding a rule.


> > Then I've added (quickly) a libabigail exception rule:
> >
> > [suppress_type]
> > name = rte_cryptodev
> > has_data_member_inserted_between = {0, 1023}
> >
> > Now we want to improve this rule to restrict the offsets
> > to the padding at the end of the struct only,
> > so we keep forbidding changes in existing fields,
> > and forbidding additions further the current struct size.
> > Is this new rule good?
> >
> > has_data_member_inserted_between = {offset_after(attached), end}
> 
> 
> Yes, this rule should do what you think it says.
> 
> > Do you confirm that the keyword "end" means the old reference size?
> 
> Yes I do.
> 
> 
> > What else do we need to check for adding a new field in a padding?
> 
> Actually, that rule will work independantly of it there is enough
> padding or not.  It'll shut down the change report, even if the added
> data exceeds the padding.

I don't understand why.
If "end" means the old reference size, then addition after the old size
should be reported, isn't it?


> You just made me think of an idea of a new feature there.
> 
> Maybe we'd need a new property for the [suppress_type] directive that
> would suppress changes only if said changes don't modify the size of the
> type or any offset of any member of the type?
> 
> Maybe something like:
> 
> [suppress_type]
>; lots of properties can go here.
> 
>; ...
> 
>; If the type has any size or offset change
>; then this suppression directive will fail
>; and the change report will be emitted
>has_no_size_or_offset_change
> 
> Would that be useful to you in this case,
> 
> Cheers,

Re: [dpdk-dev] [PATCH v15 11/12] build: add Arm SoC meson option

[Bruce Richardson]

On Thu, Jan 21, 2021 at 04:52:43PM +0100, Thomas Monjalon wrote:
> 21/01/2021 16:02, Juraj Linkeš:
> > From: Thomas Monjalon 
> > > 20/01/2021 09:41, Juraj Linkeš:
> > > > From: Honnappa Nagarahalli 
> > > > > > 20/01/2021 02:04, Honnappa Nagarahalli:
> > > > > > > > On Tue, Jan 19, 2021 at 04:52:19PM +0100, Thomas Monjalon wrote:
> > > > > > > > > 19/01/2021 15:56, Juraj Linkeš:
> > > > > > > > > > From: Thomas Monjalon 
> > > > > > > > > > > 15/01/2021 14:26, Juraj Linkeš:
> > > > > > > > > > > > --- a/meson_options.txt
> > > > > > > > > > > > +++ b/meson_options.txt
> > > > > > > > > > > > +option('arm_soc', type: 'string', value: '',
> > > > > > > > > > > > +   description: 'Specify if you want to build for a
> > > > > > > > > > > > +particular
> > > > > > > > > > > > +aarch64 Arm SoC when building on an aarch64
> > > > > > > > > > > > +machine.')
> > > > > > > > > > >
> > > > > > > > > > > Why the option is named "arm_soc" and not just "soc"?
> > > > > > > > > > > The same option could be used by other archs, isn't it?
> > > > > > > > > >
> > > > > > > > > > Agree that a more generic name would be better.
> > > > > > > > > > I'll change it to "soc" if there are no other suggestions.
> > > > > > > > >
> > > > > > > > > Another name could be "machine".
> > > > > > > > > Should it be the same mechanism as compiling for a specific
> > > > > > > > > x86 CPU from an x86 machine?
> > > > > > > > >
> > > > > > > > I'd rather not re-use the term "machine", for a new use,
> > > > > > > > better to use a new term IMHO.
> > > > > > > +1, agree. 'soc' sounds good to me.
> > > > > >
> > > > > > Another possible word is "platform", as in
> > > > > > http://doc.dpdk.org/guides/platform/index.html
> > > > > I am fine with 'platform' too.
> > > > >
> > > >
> > > > 'platform' is likely the best and actually works nicely with
> > > http://patches.dpdk.org/patch/85956/. Taken together, 'platform' could be
> > > either 'native', 'generic' or an soc, which is, I believe, exactly what 
> > > we want.
> > > 
> > > I am not sure what we want :)
> > > We need to specify the instruction set, and the specific target.
> > > We could deduce the instruction set from the target, but I think it is 
> > > good to be
> > > able to overwrite the instruction set in case there can be multiple 
> > > instruction
> > > sets for a target.
> > > 
> > 
> > I think we had this (or similar) discussion here 
> > http://patches.dpdk.org/patch/85956/. I agree with your summary.
> > 
> > > I think "native" and "generic" should be specified as instruction set, in 
> > > the
> > > existing option "machine" or renamed as "instruction_set" or "isa".
> > > 
> > 
> > Agree, but I would add that we also want "native" and "generic" as valid 
> > platforms. More below.
> > 
> > > Let's imagine the first option is "isa" and the new second option is 
> > > "platform".
> > > We can have a default "isa" per "platform".
> > > The default "platform" would have a default "isa": native or generic?
> > > 
> > 
> > In general, yes, but I let me expand the "platform" option a bit.
> > 
> > Let's dig into what "platform" means. I understand it to be a set of 
> > configuration options, e.g.:
> > platform=native: use discovered values for all configuration options (where 
> > we can do that)
> > platform=generic: use predetermined values to produce a "generic" build 
> > that would work on most machine of the corresponding type/arch
> 
> This is where I was disagreeing:
> you propose to have 2 special values of platform (native and generic),
> I propose to have only 1 default value for platform.
> 
> After more thoughts, I think it's fine.
> 
> 
> > platform=other: use predetermined values to produce a build tailored to 
> > platform "other", such as some arm soc.
> > 
> > In all these cases, leave user the option to specify supported options 
> > (i.e. user can specifying "instruction_set" and that value would be used 
> > for machine args or "max_lcores" would set max lcores).
> > 
> > The default "instruction_set" would be different per platform:
> 
> What do you think about calling this option "isa"?
> 
> > platform=native => instruction_set=native
> > platform=generic => the generic instruction_set for the architecture, as 
> > specified here: 
> > https://github.com/DPDK/dpdk/blob/main/config/meson.build#L79
> > platform=other => the instruction set of the platform
> > 
> > When "instruction_set" is set to its default value (such as auto), the 
> > per-platform instruction set would be used. If the user specifies anything 
> > else, that value would be used.
> 
> Why auto? This is what we call native.
> 

"auto" is a better term. If the platform is selected as "native" then the
default isa will be "native", while if the platform is generic, the default
isa should be something generic too. Since we unfortunately can't have one
option automatically update the value of the other, we need to use a value
of "auto" to allow platform to provide variable defaults for th

Re: [dpdk-dev] [PATCH v11 3/4] raw/ifpga: add OPAE API for OpenStack Cyborg

[Ferruh Yigit]

On 1/21/2021 6:03 AM, Wei Huang wrote:

Cyborg is an OpenStack project that aims to provide a general purpose
management framework for acceleration resources (i.e. various types
of accelerators such as GPU, FPGA, NP, ODP, DPDK/SPDK and so on).
It needs some OPAE type APIs to manage PACs (Programmable Acceleration
Card) with Intel FPGA. Below major functions are added to meets
Cyborg requirements.
1. opae_init() set up OPAE environment.
2. opae_cleanup() clean up OPAE environment.
3. opae_enumerate() searches PAC with specific FPGA.
4. opae_get_property() gets properties of FPGA.
5. opae_partial_reconfigure() perform partial configuration on FPGA.
6. opae_get_image_info() gets information of image file.
7. opae_update_flash() updates FPGA flash with specific image file.
8. opae_cancel_flash_update() cancel process of FPGA flash update.
9. opae_probe_device() manually probe specific FPGA with ifpga driver.
10. opae_remove_device() manually remove specific FPGA from ifpga driver.
11. opae_bind_driver() binds specific FPGA with specified kernel driver.
12. opae_unbind_driver() unbinds specific FPGA from kernel driver.
13. opae_reboot_device() reboots specific FPGA (do reconfiguration).

Signed-off-by: Wei Huang 
Acked-by: Tianfei Zhang 
Acked-by: Rosen Xu 


<...>


+
+RTE_INIT(init_api_env)
+{
+   eal_inited = 0;
+   opae_log_level = OPAE_LOG_ERR;
+   opae_log_file = NULL;
+   ifpga_rawdev_logtype = 0;
+
+   opae_log_info("API environment is initialized\n");
+}
+
+RTE_FINI(clean_api_env)
+{
+   if (opae_log_file) {
+   fclose(opae_log_file);
+   opae_log_file = NULL;
+   }
+   opae_log_info("API environment is cleaned\n");
+}
+


There are called in constructor and destructor, which means all DPDK 
applications have this PMD compiled will run above and print some log, is this 
really what we want here?

Is there a way to limit the initialize to the PMD context, like 'probe()' etc?

Re: [dpdk-dev] [PATCH v11 3/4] raw/ifpga: add OPAE API for OpenStack Cyborg

[Ferruh Yigit]

On 1/21/2021 6:03 AM, Wei Huang wrote:

Cyborg is an OpenStack project that aims to provide a general purpose
management framework for acceleration resources (i.e. various types
of accelerators such as GPU, FPGA, NP, ODP, DPDK/SPDK and so on).
It needs some OPAE type APIs to manage PACs (Programmable Acceleration
Card) with Intel FPGA. Below major functions are added to meets
Cyborg requirements.
1. opae_init() set up OPAE environment.
2. opae_cleanup() clean up OPAE environment.
3. opae_enumerate() searches PAC with specific FPGA.
4. opae_get_property() gets properties of FPGA.
5. opae_partial_reconfigure() perform partial configuration on FPGA.
6. opae_get_image_info() gets information of image file.
7. opae_update_flash() updates FPGA flash with specific image file.
8. opae_cancel_flash_update() cancel process of FPGA flash update.
9. opae_probe_device() manually probe specific FPGA with ifpga driver.
10. opae_remove_device() manually remove specific FPGA from ifpga driver.
11. opae_bind_driver() binds specific FPGA with specified kernel driver.
12. opae_unbind_driver() unbinds specific FPGA from kernel driver.
13. opae_reboot_device() reboots specific FPGA (do reconfiguration).



Hi Wei,

As far as I understand you are adding above public functions which are on top of 
raw/ifpga driver functions, so they are like PMD specific APIs, I think there 
are a few problems with it:


1) Do we really need/want this much PMD specific API? Can't we have them through 
the rawdev abstraction layer?


2) DPDK public APIs are part of API/ABI policy, so there are a few rules they 
have to follow, like:
- They should start with 'rte_' prefix, and the PMD specific APIs should start 
with 'rte_pmd_' prefix

- They should be in the .map file
- They should be experimental at least one release
- They should be fully documented in a doxygen format
  - Header file should be added to index file for API documentation

Please don't update above before 1) is clearified and we are sure new APIs are 
required.


<...>


@@ -13,8 +13,10 @@ objs = [base_objs]
  deps += ['ethdev', 'rawdev', 'pci', 'bus_pci', 'kvargs',
'bus_vdev', 'bus_ifpga', 'net', 'net_i40e', 'net_ipn3ke']
  
-sources = files('ifpga_rawdev.c')

+sources = files('ifpga_rawdev.c', 'ifpga_opae_api.c')
  
  includes += include_directories('base')

  includes += include_directories('../../net/ipn3ke')
  includes += include_directories('../../net/i40e')
+
+install_headers('ifpga_opae_api.h')



There is a 'headers' helper that you can use for meson. Also the header file 
name should start with 'rte_pmd_'.


Even before this patch, isn't application has to include the rawdev PMD header? 
Why that header was not installed?

Re: [dpdk-dev] [dpdk-stable] [PATCH v11 1/4] raw/ifpga: add fpga rsu function

[Ferruh Yigit]

On 1/21/2021 6:03 AM, Wei Huang wrote:

RSU (Remote System Update) depends on secure manager which may be
different on various implementations, so a new secure manager device
is implemented for adapting such difference.
There are three major functions added:
1. ifpga_rawdev_update_flash() updates flash with specific image file.
2. ifpga_rawdev_stop_flash_update() aborts flash update process.
3. ifpga_rawdev_reload() reloads FPGA from updated flash.

Signed-off-by: Wei Huang 
Acked-by: Tianfei Zhang 
Acked-by: Rosen Xu 


<...>


@@ -76,4 +76,9 @@ int
  ifpga_unregister_msix_irq(enum ifpga_irq_type type,
int vec_start, rte_intr_callback_fn handler, void *arg);
  
+int ifpga_rawdev_update_flash(struct rte_rawdev *dev, const char *image,

+   uint64_t *status);
+int ifpga_rawdev_stop_flash_update(struct rte_rawdev *dev, int force);
+int ifpga_rawdev_reload(struct rte_rawdev *dev, int type, int page);
+
  #endif /* _IFPGA_RAWDEV_H_ */



Hi Wei,

Please help me understand the rawdev, who should be calling the above newly 
added functions?

Re: [dpdk-dev] [PATCH v11 3/4] raw/ifpga: add OPAE API for OpenStack Cyborg

[Ferruh Yigit]

On 1/21/2021 6:03 AM, Wei Huang wrote:

+int opae_init(int eal_init_result)
+{
+   int ret = 0;
+
+   if (!check_eal(0))
+   return 0;
+
+   if (eal_init_result < 0) {
+   if (rte_errno == EALREADY) {
+   eal_inited = 1;
+   opae_log_info("EAL already initialized\n");
+   } else {
+   opae_log_err("Cannot initialize EAL\n");
+   ret = -1;
+   }
+   } else {
+   eal_inited = 1;
+   opae_log_info("Initialize EAL done\n");
+   }
+
+   return ret;
+}


Why an PMD API needs to know eal initialization status? This may be pointing a 
bad design, is it possible that most of the 'opae_' API code should go to the 
application?

Re: [dpdk-dev] [dpdklab] RE: [dpdk-stable] [PATCH v4] mbuf: fix reset on mbuf free

[Lincoln Lavoie]

Hi All,

Trying to follow the specific conversation.  It is correct, the lab does
not list the specific throughput values achieved by the hardware, as that
data can be sensitive to the hardware vendors, etc. The purpose of the lab
is to check for degradations caused by patches, so the difference is really
the important factor.  The comparison is against a prior run on the same
hardware, via the DPDK main branch, so any delta should be caused by the
specific patch changes (excluding statistical "wiggle").

If the group would prefer, we could calculate additional references if
desired (i.e. difference from the last official release, or a monthly run
of the current, etc.).  We just need the community to define their needs,
and we can add this to the development queue.

Cheers,
Lincoln


On Thu, Jan 21, 2021 at 4:29 AM Morten Brørup 
wrote:

> > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Ferruh Yigit
> > Sent: Thursday, January 21, 2021 10:19 AM
> >
> > On 1/15/2021 6:39 PM, Ali Alnubani wrote:
> > > Hi,
> > > Adding Ferruh and Zhaoyan,
> > >
> > >> Ali,
> > >>
> > >> You reported some performance regression, did you confirm it?
> > >> If I get no reply by monday, I'll proceed with this patch.
> > >
> > > Sure I'll confirm by Monday.
> > >
> > > Doesn't the regression also reproduce on the Lab's Intel servers?
> > > Even though the check iol-intel-Performance isn't failing, I can see
> > that the throughput differences from expected for this patch are less
> > than those of another patch that was tested only 20 minutes earlier.
> > Both patches were applied to the same tree:
> > >
> > > https://mails.dpdk.org/archives/test-report/2021-January/173927.html
> > >> | 64 | 512 | 1.571   |
> > >
> > > https://mails.dpdk.org/archives/test-report/2021-January/173919.html
> > >> | 64 | 512 | 2.698   |
> > >
> > > Assuming that pw86457 doesn't have an effect on this test, it looks
> > to me that this patch caused a regression in Intel hardware as well.
> > >
> > > Can someone update the baseline's expected values for the Intel NICs
> > and rerun the test on this patch?
> > >
> >
> > Zhaoyan said that the baseline is calculated dynamically,
> > what I understand is baseline set based on previous days performance
> > result, so
> > it shouldn't require updating.
>
> That sounds smart!
>
> Perhaps another reference baseline could be added, for informational
> purposes only:
> Deviation from the performance of the last official release.
>
> >
> > But cc'ed the lab for more details.
>
>

-- 
*Lincoln Lavoie*
Senior Engineer, Broadband Technologies
21 Madbury Rd., Ste. 100, Durham, NH 03824
lylav...@iol.unh.edu
https://www.iol.unh.edu
+1-603-674-2755 (m)

Re: [dpdk-dev] [dpdk-stable] [PATCH v11 4/4] examples/ifpga: add example for opae ifpga API

[Ferruh Yigit]

On 1/21/2021 6:03 AM, Wei Huang wrote:

+get_property command
+
+
+Display property information of specified FPGA. Property type is defined as 
below.
+0 - All properties
+1 - PCI property
+2 - FME property
+4 - port property
+8 - BMC property
+PCI property is always available, other properties can only be displayed after
+ifpga driver is probed to the FPGA.
+
+.. code-block:: console
+
+   opae> get_property 24:00.0 0
+   PCI:
+PCIe s:b:d.f : :24:00.0
+kernel driver: vfio-pci
+   FME:
+platform : Vista Creek
+DCP version  : DCP 1.2
+phase: Beta
+interface: 2x2x25G
+build version: 0.0.2
+ports num: 1
+boot page: user
+pr interface id  : a5d72a3c-c8b0-4939-912c-f715e5dc10ca
+   PORT0:
+access type  : PF
+accelerator id   : 8892c23e-2eed-4b44-8bb6-5c88606e07df
+   BMC:
+MAX10 version: D.2.0.5
+NIOS FW version  : D.2.0.12


Are all commands interacting device need to provide the PCI BDF as parameter to 
select the device? Like '24:00.0' in above sample.


Why not get the unique rawdev identifier, like port_id equivalent of the ethdev, 
and use it in the command line, won't it be easier?

Re: [dpdk-dev] [PATCH v3 01/11] common/mlx5: add DevX attributes for compress

[Tal Shnaiderman]

> Subject: [dpdk-dev] [PATCH v3 01/11] common/mlx5: add DevX attributes
> for compress
> 
> Add the DevX attributes for compress related engines:
>   - compress
>   - decompress
>   - dma
> 
> Signed-off-by: Matan Azrad 
> Acked-by: Viacheslav Ovsiienko 
> ---
>  drivers/common/mlx5/mlx5_devx_cmds.c | 10 ++
> drivers/common/mlx5/mlx5_devx_cmds.h |  7 +++
>  drivers/common/mlx5/mlx5_prm.h   | 18 --
>  3 files changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/common/mlx5/mlx5_devx_cmds.c
> b/drivers/common/mlx5/mlx5_devx_cmds.c
> index d5859c2..33acd73 100644
> --- a/drivers/common/mlx5/mlx5_devx_cmds.c
> +++ b/drivers/common/mlx5/mlx5_devx_cmds.c
> @@ -732,6 +732,16 @@ struct mlx5_devx_obj *
>   attr->log_max_pd = MLX5_GET(cmd_hca_cap, hcattr, log_max_pd);
>   attr->log_max_srq = MLX5_GET(cmd_hca_cap, hcattr, log_max_srq);
>   attr->log_max_srq_sz = MLX5_GET(cmd_hca_cap, hcattr,
> log_max_srq_sz);
> + attr->mmo_dma_en = MLX5_GET(cmd_hca_cap, hcattr, dma_mmo);
> + attr->mmo_compress_en = MLX5_GET(cmd_hca_cap, hcattr,
> compress);
> + attr->mmo_decompress_en = MLX5_GET(cmd_hca_cap, hcattr,
> decompress);
> + attr->compress_min_block_size = MLX5_GET(cmd_hca_cap, hcattr,
> +  compress_min_block_size);
> + attr->log_max_mmo_dma = MLX5_GET(cmd_hca_cap, hcattr,
> log_dma_mmo_size);
> + attr->log_max_mmo_compress = MLX5_GET(cmd_hca_cap, hcattr,
> +   log_compress_mmo_size);
> + attr->log_max_mmo_decompress = MLX5_GET(cmd_hca_cap,
> hcattr,
> + log_decompress_mmo_size);
>   if (attr->qos.sup) {
>   MLX5_SET(query_hca_cap_in, in, op_mod,
>MLX5_GET_HCA_CAP_OP_MOD_QOS_CAP |
> diff --git a/drivers/common/mlx5/mlx5_devx_cmds.h
> b/drivers/common/mlx5/mlx5_devx_cmds.h
> index bf83a90..696a981 100644
> --- a/drivers/common/mlx5/mlx5_devx_cmds.h
> +++ b/drivers/common/mlx5/mlx5_devx_cmds.h
> @@ -130,6 +130,13 @@ struct mlx5_hca_attr {
>   uint32_t log_max_srq;
>   uint32_t log_max_srq_sz;
>   uint32_t rss_ind_tbl_cap;
> + uint32_t mmo_dma_en:1;
> + uint32_t mmo_compress_en:1;
> + uint32_t mmo_decompress_en:1;
> + uint32_t compress_min_block_size:4;
> + uint32_t log_max_mmo_dma:5;
> + uint32_t log_max_mmo_compress:5;
> + uint32_t log_max_mmo_decompress:5;
>  };
> 
>  struct mlx5_devx_wq_attr {
> diff --git a/drivers/common/mlx5/mlx5_prm.h
> b/drivers/common/mlx5/mlx5_prm.h index c9eba22..72c843f 100644
> --- a/drivers/common/mlx5/mlx5_prm.h
> +++ b/drivers/common/mlx5/mlx5_prm.h
> @@ -1127,7 +1127,15 @@ enum {
>  struct mlx5_ifc_cmd_hca_cap_bits {
>   u8 reserved_at_0[0x30];
>   u8 vhca_id[0x10];
> - u8 reserved_at_40[0x40];
> + u8 reserved_at_40[0x20];
> + u8 reserved_at_60[0x3];
> + u8 log_regexp_scatter_gather_size[0x5];
> + u8 reserved_at_68[0x3];
> + u8 log_dma_mmo_size[5];
> + u8 reserved_at_70[0x3];
> + u8 log_compress_mmo_size[5];
> + u8 reserved_at_78[0x3];
> + u8 log_decompress_mmo_size[5];

Small comment, PRM bit array size is always defined in hex format.

>   u8 log_max_srq_sz[0x8];
>   u8 log_max_qp_sz[0x8];
>   u8 reserved_at_90[0x9];
> @@ -1175,7 +1183,13 @@ struct mlx5_ifc_cmd_hca_cap_bits {
>   u8 log_max_ra_res_dc[0x6];
>   u8 reserved_at_140[0xa];
>   u8 log_max_ra_req_qp[0x6];
> - u8 reserved_at_150[0xa];
> + u8 rtr2rts_qp_counters_set_id[1];
> + u8 rts2rts_udp_sport[1];
> + u8 rts2rts_lag_tx_port_affinity[1];
> + u8 dma_mmo[1];
> + u8 compress_min_block_size[4];
> + u8 compress[1];
> + u8 decompress[1];

Same.

>   u8 log_max_ra_res_qp[0x6];
>   u8 end_pad[0x1];
>   u8 cc_query_allowed[0x1];
> --
> 1.8.3.1

Re: [dpdk-dev] [PATCH] net/mlx5:fix storing the synced MAC to internal table

[Slava Ovsiienko]

Hi, Souvik

Thank you for the patch, please see my comments below.
>From: Souvik Dey  
>Sent: Wednesday, December 9, 2020 17:11
>To: Raslan Darawsheh ; Matan Azrad ; 
>Shahaf Shuler >; Slava Ovsiienko 
>Cc: dev@dpdk.org; Souvik Dey 
>Subject: [PATCH] net/mlx5:fix storing the synced MAC to internal table

We have the warnings:
Wrong headline format:
net/mlx5:fix storing the synced MAC to internal table
Wrong headline prefix:
net/mlx5:fix storing the synced MAC to internal table
Missing 'Fixes' tag:
net/mlx5:fix storing the synced MAC to internal table

The patch headline should be:
common/mlx5: fix storing the synched MAC to internal table

- net/ -> common/
- space after mlx5:
- synced -> synched

The patch should contain the Fixes tag: 

Fixes: f22442cb5d42 (“net/mlx5: reduce Netlink commands dependencies”)
Fixes: ccdcba53a3f4 (“net/mlx5: use Netlink to add/remove MAC addresses”)

The patch should contain (to be sent to LTS ML):

Cc: sta...@dpdk.org

>As the internal MAC table is divided into Unicast and Multicast address
>section, we should check the type of synced MAC address before storing
section -> sections
synced -> synched

>it to the internal table. Currently the check is not done, and the
>synced MAC of 33:33:00:00:00:01 gets stored in the unicast section
sync ->synched

>(mostly index 1) which causes all subsequent mlx5_set_mc_addr_list()
reword? :  which causes -> causing  

>to fail with error -EADDRINUSE, as the mac_list contains the MAC
>33:33:00:00:00:01. This denies adding of any new multicast address to
>the internal list and also fails to add the MAC address to the device
>in case of SR-IOV VF case.
typo: case (to remove)

With best regards,
Slava

diff --git a/drivers/common/mlx5/linux/mlx5_nl.c 
b/drivers/common/mlx5/linux/mlx5_nl.c
index 40d8620..ef7a521 100644
--- a/drivers/common/mlx5/linux/mlx5_nl.c
+++ b/drivers/common/mlx5/linux/mlx5_nl.c
@@ -758,11 +758,21 @@ mlx5_nl_mac_addr_sync(int nlsk_fd, unsigned int iface_idx,
break;
if (j != n)
continue;
- /* Find the first entry available. */
- for (j = 0; j != n; ++j) {
- if (rte_is_zero_ether_addr(&mac_addrs[j])) {
- mac_addrs[j] = macs[i];
- break;
+ if (rte_is_multicast_ether_addr(&macs[i])) {
+ /* Find the first entry available. */
+ for (j = MLX5_MAX_UC_MAC_ADDRESSES; j != n; ++j) {
+ if (rte_is_zero_ether_addr(&mac_addrs[j])) {
+ mac_addrs[j] = macs[i];
+ break;
+ }
+ }
+ } else {
+ /* Find the first entry available. */
+ for (j = 0; j != MLX5_MAX_UC_MAC_ADDRESSES; ++j) {
+ if (rte_is_zero_ether_addr(&mac_addrs[j])) {
+ mac_addrs[j] = macs[i];
+ break;
+ }
}
}
}
-- 
2.9.3.windows.1


Notice: This e-mail together with any attachments may contain information of 
Ribbon Communications Inc. that is confidential and/or proprietary for the sole 
use of the intended recipient. Any review, disclosure, reliance or distribution 
by others or forwarding without express permission is strictly prohibited. If 
you are not the intended recipient, please notify the sender immediately and 
then delete all copies, including any attachments.

Re: [dpdk-dev] [PATCH v3 01/11] common/mlx5: add DevX attributes for compress

[Matan Azrad]

From: Tal Shnaiderman
> Sent: Thursday, January 21, 2021 6:52 PM
> To: Matan Azrad ; dev@dpdk.org
> Cc: NBU-Contact-Thomas Monjalon ; Ashish Gupta
> ; Fiona Trahe ;
> akhil.go...@nxp.com
> Subject: RE: [dpdk-dev] [PATCH v3 01/11] common/mlx5: add DevX attributes
> for compress
> 
> > Subject: [dpdk-dev] [PATCH v3 01/11] common/mlx5: add DevX attributes
> > for compress
> >
> > Add the DevX attributes for compress related engines:
> > - compress
> > - decompress
> > - dma
> >
> > Signed-off-by: Matan Azrad 
> > Acked-by: Viacheslav Ovsiienko 
> > ---
> >  drivers/common/mlx5/mlx5_devx_cmds.c | 10 ++
> > drivers/common/mlx5/mlx5_devx_cmds.h |  7 +++
> >  drivers/common/mlx5/mlx5_prm.h   | 18 --
> >  3 files changed, 33 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/common/mlx5/mlx5_devx_cmds.c
> > b/drivers/common/mlx5/mlx5_devx_cmds.c
> > index d5859c2..33acd73 100644
> > --- a/drivers/common/mlx5/mlx5_devx_cmds.c
> > +++ b/drivers/common/mlx5/mlx5_devx_cmds.c
> > @@ -732,6 +732,16 @@ struct mlx5_devx_obj *
> > attr->log_max_pd = MLX5_GET(cmd_hca_cap, hcattr, log_max_pd);
> > attr->log_max_srq = MLX5_GET(cmd_hca_cap, hcattr, log_max_srq);
> > attr->log_max_srq_sz = MLX5_GET(cmd_hca_cap, hcattr,
> > log_max_srq_sz);
> > +   attr->mmo_dma_en = MLX5_GET(cmd_hca_cap, hcattr, dma_mmo);
> > +   attr->mmo_compress_en = MLX5_GET(cmd_hca_cap, hcattr,
> > compress);
> > +   attr->mmo_decompress_en = MLX5_GET(cmd_hca_cap, hcattr,
> > decompress);
> > +   attr->compress_min_block_size = MLX5_GET(cmd_hca_cap, hcattr,
> > +compress_min_block_size);
> > +   attr->log_max_mmo_dma = MLX5_GET(cmd_hca_cap, hcattr,
> > log_dma_mmo_size);
> > +   attr->log_max_mmo_compress = MLX5_GET(cmd_hca_cap, hcattr,
> > + log_compress_mmo_size);
> > +   attr->log_max_mmo_decompress = MLX5_GET(cmd_hca_cap,
> > hcattr,
> > +   log_decompress_mmo_size);
> > if (attr->qos.sup) {
> > MLX5_SET(query_hca_cap_in, in, op_mod,
> >  MLX5_GET_HCA_CAP_OP_MOD_QOS_CAP | diff --git
> > a/drivers/common/mlx5/mlx5_devx_cmds.h
> > b/drivers/common/mlx5/mlx5_devx_cmds.h
> > index bf83a90..696a981 100644
> > --- a/drivers/common/mlx5/mlx5_devx_cmds.h
> > +++ b/drivers/common/mlx5/mlx5_devx_cmds.h
> > @@ -130,6 +130,13 @@ struct mlx5_hca_attr {
> > uint32_t log_max_srq;
> > uint32_t log_max_srq_sz;
> > uint32_t rss_ind_tbl_cap;
> > +   uint32_t mmo_dma_en:1;
> > +   uint32_t mmo_compress_en:1;
> > +   uint32_t mmo_decompress_en:1;
> > +   uint32_t compress_min_block_size:4;
> > +   uint32_t log_max_mmo_dma:5;
> > +   uint32_t log_max_mmo_compress:5;
> > +   uint32_t log_max_mmo_decompress:5;
> >  };
> >
> >  struct mlx5_devx_wq_attr {
> > diff --git a/drivers/common/mlx5/mlx5_prm.h
> > b/drivers/common/mlx5/mlx5_prm.h index c9eba22..72c843f 100644
> > --- a/drivers/common/mlx5/mlx5_prm.h
> > +++ b/drivers/common/mlx5/mlx5_prm.h
> > @@ -1127,7 +1127,15 @@ enum {
> >  struct mlx5_ifc_cmd_hca_cap_bits {
> > u8 reserved_at_0[0x30];
> > u8 vhca_id[0x10];
> > -   u8 reserved_at_40[0x40];
> > +   u8 reserved_at_40[0x20];
> > +   u8 reserved_at_60[0x3];
> > +   u8 log_regexp_scatter_gather_size[0x5];
> > +   u8 reserved_at_68[0x3];
> > +   u8 log_dma_mmo_size[5];
> > +   u8 reserved_at_70[0x3];
> > +   u8 log_compress_mmo_size[5];
> > +   u8 reserved_at_78[0x3];
> > +   u8 log_decompress_mmo_size[5];
> 
> Small comment, PRM bit array size is always defined in hex format.
> 
> > u8 log_max_srq_sz[0x8];
> > u8 log_max_qp_sz[0x8];
> > u8 reserved_at_90[0x9];
> > @@ -1175,7 +1183,13 @@ struct mlx5_ifc_cmd_hca_cap_bits {
> > u8 log_max_ra_res_dc[0x6];
> > u8 reserved_at_140[0xa];
> > u8 log_max_ra_req_qp[0x6];
> > -   u8 reserved_at_150[0xa];
> > +   u8 rtr2rts_qp_counters_set_id[1];
> > +   u8 rts2rts_udp_sport[1];
> > +   u8 rts2rts_lag_tx_port_affinity[1];
> > +   u8 dma_mmo[1];
> > +   u8 compress_min_block_size[4];
> > +   u8 compress[1];
> > +   u8 decompress[1];
> 
> Same.
> 
> > u8 log_max_ra_res_qp[0x6];
> > u8 end_pad[0x1];
> > u8 cc_query_allowed[0x1];
> > --
> > 1.8.3.1

Yes, not look urgent, Akhil let me know if you can take it in integration or 
need to send more version for all the series because of it.

[dpdk-dev] [PATCH v4 0/6] power: fix make build for power apps

[David Hunt]

The guest channel message definitions and functions in guest_channel.h
are needed by applications and need to be made public.

This worked pre-20.11, but now with all the meson/ninja changes, making
these apps externally no longer works. To fix, we need to move the
header file with the API definitions for the channel commands public,
and rename the functions accordingly.

The main change is to rename channel_commands.h to
rte_power_guest_channel.h so that it gets picked up by the installer
and copied to /usr/local/include. Other changes include renaming #defines
to have RTE_ at the beginning instead of CPU_. Finally we refactor the
code to work with those changes.

---
v2 changes
 - re-worked from monolithic patch to a 6 patch patchset for easier review
v3 changes
  - Ensure both functions added to the API are tagged as experimental
v4 changes
  - add @internal tag on 2 functions for Doxygen
  - add @warning EXPERIMENTAL tag on 2 functions for Doxygen
  - improve description of lcore param in API docs
  - Improve maintainability sizeof's
  - add the 2 added functions to the version.map file

[PATCH v4 1/6] power: create guest channel public header file
[PATCH v4 2/6] power: make channel msg functions public
[PATCH v4 3/6] power: rename public structs
[PATCH v4 4/6] power: rename defines
[PATCH v4 5/6] power: add new header file to export list
[PATCH v4 6/6] power: clean up includes

[dpdk-dev] [PATCH v4 1/6] power: create guest channel public header file

[David Hunt]

From: Bruce Richardson 

In preparation for making the header file public, we first rename
channel_commands.h as rte_power_guest_channel.h.

Fixes: 210c383e247b ("power: packet format for vm power management")
Fixes: cd0d5547e873 ("power: vm communication channels in guest")
Cc: sta...@dpdk.org

Signed-off-by: Bruce Richardson 
Signed-off-by: David Hunt 
---
 examples/vm_power_manager/channel_manager.c  | 2 +-
 examples/vm_power_manager/channel_monitor.c  | 2 +-
 examples/vm_power_manager/channel_monitor.h  | 2 +-
 examples/vm_power_manager/guest_cli/vm_power_cli_guest.h | 2 +-
 examples/vm_power_manager/vm_power_cli.c | 2 +-
 lib/librte_power/guest_channel.c | 2 +-
 lib/librte_power/guest_channel.h | 2 +-
 lib/librte_power/power_kvm_vm.c  | 2 +-
 .../{channel_commands.h => rte_power_guest_channel.h}| 9 -
 9 files changed, 12 insertions(+), 13 deletions(-)
 rename lib/librte_power/{channel_commands.h => rte_power_guest_channel.h} (94%)

diff --git a/examples/vm_power_manager/channel_manager.c 
b/examples/vm_power_manager/channel_manager.c
index a26315051..c7d5bf5a8 100644
--- a/examples/vm_power_manager/channel_manager.c
+++ b/examples/vm_power_manager/channel_manager.c
@@ -27,7 +27,7 @@
 #include 
 
 #include "channel_manager.h"
-#include "channel_commands.h"
+#include "rte_power_guest_channel.h"
 #include "channel_monitor.h"
 #include "power_manager.h"
 
diff --git a/examples/vm_power_manager/channel_monitor.c 
b/examples/vm_power_manager/channel_monitor.c
index 228f06803..08306105d 100644
--- a/examples/vm_power_manager/channel_monitor.c
+++ b/examples/vm_power_manager/channel_monitor.c
@@ -35,7 +35,7 @@
 
 #include 
 #include "channel_monitor.h"
-#include "channel_commands.h"
+#include "rte_power_guest_channel.h"
 #include "channel_manager.h"
 #include "power_manager.h"
 #include "oob_monitor.h"
diff --git a/examples/vm_power_manager/channel_monitor.h 
b/examples/vm_power_manager/channel_monitor.h
index 7362a80d2..4a526ff67 100644
--- a/examples/vm_power_manager/channel_monitor.h
+++ b/examples/vm_power_manager/channel_monitor.h
@@ -6,7 +6,7 @@
 #define CHANNEL_MONITOR_H_
 
 #include "channel_manager.h"
-#include "channel_commands.h"
+#include "rte_power_guest_channel.h"
 
 struct core_share {
unsigned int pcpu;
diff --git a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.h 
b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.h
index 6ad14a3de..2299d23dc 100644
--- a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.h
+++ b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.h
@@ -9,7 +9,7 @@
 extern "C" {
 #endif
 
-#include "channel_commands.h"
+#include "rte_power_guest_channel.h"
 
 struct channel_packet *get_policy(void);
 
diff --git a/examples/vm_power_manager/vm_power_cli.c 
b/examples/vm_power_manager/vm_power_cli.c
index ed0623a41..f7e1b596e 100644
--- a/examples/vm_power_manager/vm_power_cli.c
+++ b/examples/vm_power_manager/vm_power_cli.c
@@ -21,7 +21,7 @@
 #include "channel_manager.h"
 #include "channel_monitor.h"
 #include "power_manager.h"
-#include "channel_commands.h"
+#include "rte_power_guest_channel.h"
 
 struct cmd_quit_result {
cmdline_fixed_string_t quit;
diff --git a/lib/librte_power/guest_channel.c b/lib/librte_power/guest_channel.c
index 7b5926e5c..4cb5ae1dd 100644
--- a/lib/librte_power/guest_channel.c
+++ b/lib/librte_power/guest_channel.c
@@ -17,7 +17,7 @@
 #include 
 
 #include "guest_channel.h"
-#include "channel_commands.h"
+#include "rte_power_guest_channel.h"
 
 #define RTE_LOGTYPE_GUEST_CHANNEL RTE_LOGTYPE_USER1
 
diff --git a/lib/librte_power/guest_channel.h b/lib/librte_power/guest_channel.h
index e15db46fc..d3d87f0ae 100644
--- a/lib/librte_power/guest_channel.h
+++ b/lib/librte_power/guest_channel.h
@@ -8,7 +8,7 @@
 extern "C" {
 #endif
 
-#include 
+#include 
 
 /**
  * Check if any Virtio-Serial VM end-points exist in path.
diff --git a/lib/librte_power/power_kvm_vm.c b/lib/librte_power/power_kvm_vm.c
index 409c3e03a..649ebe85c 100644
--- a/lib/librte_power/power_kvm_vm.c
+++ b/lib/librte_power/power_kvm_vm.c
@@ -7,7 +7,7 @@
 #include 
 
 #include "guest_channel.h"
-#include "channel_commands.h"
+#include "rte_power_guest_channel.h"
 #include "power_kvm_vm.h"
 #include "power_common.h"
 
diff --git a/lib/librte_power/channel_commands.h 
b/lib/librte_power/rte_power_guest_channel.h
similarity index 94%
rename from lib/librte_power/channel_commands.h
rename to lib/librte_power/rte_power_guest_channel.h
index adc8e5ca2..ef3b064a8 100644
--- a/lib/librte_power/channel_commands.h
+++ b/lib/librte_power/rte_power_guest_channel.h
@@ -1,9 +1,8 @@
 /* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2010-2014 Intel Corporation
+ * Copyright(c) 2010-2021 Intel Corporation
  */
-
-#ifndef CHANNEL_COMMANDS_H_
-#define CHANNEL_COMMANDS_H_
+#ifndef RTE_POWER_GUEST_CHANNEL_H
+#define RTE_POWER_GUEST_CH

[dpdk-dev] [PATCH v4 2/6] power: make channel msg functions public

[David Hunt]

From: Bruce Richardson 

Move the 2 public functions into rte_power_guest_channel.h

Fixes: 210c383e247b ("power: packet format for vm power management")
Fixes: cd0d5547e873 ("power: vm communication channels in guest")
Cc: sta...@dpdk.org

Signed-off-by: Bruce Richardson 
Signed-off-by: David Hunt 

---
changes in v3
* Mark both added functions as experimental
changes in v4
* add @internal tag for Doxygen
* add @warning EXPERIMENTAL tag for Doxygen
* improve description of lcore param
---
 lib/librte_power/guest_channel.h   | 40 +
 lib/librte_power/rte_power_guest_channel.h | 50 ++
 2 files changed, 51 insertions(+), 39 deletions(-)

diff --git a/lib/librte_power/guest_channel.h b/lib/librte_power/guest_channel.h
index d3d87f0ae..69020b030 100644
--- a/lib/librte_power/guest_channel.h
+++ b/lib/librte_power/guest_channel.h
@@ -65,21 +65,7 @@ void guest_channel_host_disconnect(unsigned int lcore_id);
  */
 int guest_channel_send_msg(struct channel_packet *pkt, unsigned int lcore_id);
 
-/**
- * Send a message contained in pkt over the Virtio-Serial to the host endpoint.
- *
- * @param pkt
- *  Pointer to a populated struct channel_packet
- *
- * @param lcore_id
- *  lcore_id.
- *
- * @return
- *  - 0 on success.
- *  - Negative on error.
- */
-int rte_power_guest_channel_send_msg(struct channel_packet *pkt,
-   unsigned int lcore_id);
+
 
 /**
  * Read a message contained in pkt over the Virtio-Serial
@@ -103,30 +89,6 @@ int power_guest_channel_read_msg(void *pkt,
size_t pkt_len,
unsigned int lcore_id);
 
-/**
- * Receive a message contained in pkt over the Virtio-Serial
- * from the host endpoint.
- *
- * @param pkt
- *  Pointer to channel_packet or
- *  channel_packet_freq_list struct.
- *
- * @param pkt_len
- *  Size of expected data packet.
- *
- * @param lcore_id
- *  lcore_id.
- *
- * @return
- *  - 0 on success.
- *  - Negative on error.
- */
-__rte_experimental
-int
-rte_power_guest_channel_receive_msg(void *pkt,
-   size_t pkt_len,
-   unsigned int lcore_id);
-
 
 #ifdef __cplusplus
 }
diff --git a/lib/librte_power/rte_power_guest_channel.h 
b/lib/librte_power/rte_power_guest_channel.h
index ef3b064a8..c500c0cda 100644
--- a/lib/librte_power/rte_power_guest_channel.h
+++ b/lib/librte_power/rte_power_guest_channel.h
@@ -116,6 +116,56 @@ struct channel_packet_caps_list {
uint8_t num_vcpu;
 };
 
+/**
+ * @internal
+ *
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Send a message contained in pkt over the Virtio-Serial to the host endpoint.
+ *
+ * @param pkt
+ *  Pointer to a populated struct channel_packet.
+ *
+ * @param lcore_id
+ *  Use channel specific to this lcore_id.
+ *
+ * @return
+ *  - 0 on success.
+ *  - Negative on error.
+ */
+__rte_experimental
+int rte_power_guest_channel_send_msg(struct channel_packet *pkt,
+   unsigned int lcore_id);
+
+/**
+ * @internal
+ *
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Receive a message contained in pkt over the Virtio-Serial
+ * from the host endpoint.
+ *
+ * @param pkt
+ *  Pointer to channel_packet or
+ *  channel_packet_freq_list struct.
+ *
+ * @param pkt_len
+ *  Size of expected data packet.
+ *
+ * @param lcore_id
+ *  Use channel specific to this lcore_id.
+ *
+ * @return
+ *  - 0 on success.
+ *  - Negative on error.
+ */
+__rte_experimental
+int rte_power_guest_channel_receive_msg(void *pkt,
+   size_t pkt_len,
+   unsigned int lcore_id);
+
 
 #ifdef __cplusplus
 }
-- 
2.17.1

[dpdk-dev] [PATCH v4 3/6] power: rename public structs

[David Hunt]

From: Bruce Richardson 

rename the public structs to have an rte_power_ prefix and
add them to version.map in experimental section.

Fixes: 210c383e247b ("power: packet format for vm power management")
Fixes: cd0d5547e873 ("power: vm communication channels in guest")
Cc: sta...@dpdk.org

Signed-off-by: Bruce Richardson 
Signed-off-by: David Hunt 

---
changes in v4
* Improve sizeof's
* add the 2 functions to the version.map file
---
 examples/vm_power_manager/channel_monitor.c   | 33 ++--
 examples/vm_power_manager/channel_monitor.h   |  2 +-
 examples/vm_power_manager/guest_cli/main.c|  2 +-
 .../guest_cli/vm_power_cli_guest.c| 38 +++---
 .../guest_cli/vm_power_cli_guest.h|  4 +-
 lib/librte_power/guest_channel.c  |  7 +--
 lib/librte_power/guest_channel.h  |  7 +--
 lib/librte_power/power_kvm_vm.c   |  2 +-
 lib/librte_power/rte_power_guest_channel.h| 51 +--
 lib/librte_power/version.map  |  4 ++
 10 files changed, 77 insertions(+), 73 deletions(-)

diff --git a/examples/vm_power_manager/channel_monitor.c 
b/examples/vm_power_manager/channel_monitor.c
index 08306105d..1b6041b6f 100644
--- a/examples/vm_power_manager/channel_monitor.c
+++ b/examples/vm_power_manager/channel_monitor.c
@@ -108,7 +108,7 @@ str_to_ether_addr(const char *a, struct rte_ether_addr 
*ether_addr)
 }
 
 static int
-set_policy_mac(struct channel_packet *pkt, int idx, char *mac)
+set_policy_mac(struct rte_power_channel_packet *pkt, int idx, char *mac)
 {
union PFID pfid;
int ret;
@@ -165,7 +165,7 @@ get_resource_id_from_vmname(const char *vm_name)
 }
 
 static int
-parse_json_to_pkt(json_t *element, struct channel_packet *pkt,
+parse_json_to_pkt(json_t *element, struct rte_power_channel_packet *pkt,
const char *vm_name)
 {
const char *key;
@@ -173,7 +173,7 @@ parse_json_to_pkt(json_t *element, struct channel_packet 
*pkt,
int ret;
int resource_id;
 
-   memset(pkt, 0, sizeof(struct channel_packet));
+   memset(pkt, 0, sizeof(*pkt));
 
pkt->nb_mac_to_monitor = 0;
pkt->t_boost_status.tbEnabled = false;
@@ -463,7 +463,7 @@ get_pfid(struct policy *pol)
 }
 
 static int
-update_policy(struct channel_packet *pkt)
+update_policy(struct rte_power_channel_packet *pkt)
 {
 
unsigned int updated = 0;
@@ -512,7 +512,7 @@ update_policy(struct channel_packet *pkt)
 }
 
 static int
-remove_policy(struct channel_packet *pkt __rte_unused)
+remove_policy(struct rte_power_channel_packet *pkt __rte_unused)
 {
unsigned int i;
 
@@ -673,7 +673,7 @@ static void
 apply_policy(struct policy *pol)
 {
 
-   struct channel_packet *pkt = &pol->pkt;
+   struct rte_power_channel_packet *pkt = &pol->pkt;
 
/*Check policy to use*/
if (pkt->policy_to_use == TRAFFIC)
@@ -715,12 +715,12 @@ write_binary_packet(void *buffer,
 }
 
 static int
-send_freq(struct channel_packet *pkt,
+send_freq(struct rte_power_channel_packet *pkt,
struct channel_info *chan_info,
bool freq_list)
 {
unsigned int vcore_id = pkt->resource_id;
-   struct channel_packet_freq_list channel_pkt_freq_list;
+   struct rte_power_channel_packet_freq_list channel_pkt_freq_list;
struct vm_info info;
 
if (get_info_vm(pkt->vm_name, &info) != 0)
@@ -751,12 +751,12 @@ send_freq(struct channel_packet *pkt,
 }
 
 static int
-send_capabilities(struct channel_packet *pkt,
+send_capabilities(struct rte_power_channel_packet *pkt,
struct channel_info *chan_info,
bool list_requested)
 {
unsigned int vcore_id = pkt->resource_id;
-   struct channel_packet_caps_list channel_pkt_caps_list;
+   struct rte_power_channel_packet_caps_list channel_pkt_caps_list;
struct vm_info info;
struct rte_power_core_capabilities caps;
int ret;
@@ -805,18 +805,19 @@ send_capabilities(struct channel_packet *pkt,
 }
 
 static int
-send_ack_for_received_cmd(struct channel_packet *pkt,
+send_ack_for_received_cmd(struct rte_power_channel_packet *pkt,
struct channel_info *chan_info,
uint32_t command)
 {
pkt->command = command;
return write_binary_packet(pkt,
-   sizeof(struct channel_packet),
+   sizeof(*pkt),
chan_info);
 }
 
 static int
-process_request(struct channel_packet *pkt, struct channel_info *chan_info)
+process_request(struct rte_power_channel_packet *pkt,
+   struct channel_info *chan_info)
 {
int ret;
 
@@ -988,7 +989,7 @@ channel_monitor_init(void)
 static void
 read_binary_packet(struct channel_info *chan_info)
 {
-   struct channel_packet pkt;
+   struct rte_power_channel_packet pkt;
void *buffer = &pkt;
int buffer_len = sizeof(pkt);
int n_bytes, err = 0;
@@ -1019,7 +1020,7 @@

[dpdk-dev] [PATCH v4 4/6] power: rename defines

[David Hunt]

From: Bruce Richardson 

Rename the #defines to have an RTE_POWER_ prefix

Fixes: 210c383e247b ("power: packet format for vm power management")
Fixes: cd0d5547e873 ("power: vm communication channels in guest")
Cc: sta...@dpdk.org

Signed-off-by: Bruce Richardson 
Signed-off-by: David Hunt 
---
 examples/vm_power_manager/channel_monitor.c   | 116 +-
 examples/vm_power_manager/channel_monitor.h   |   6 +-
 examples/vm_power_manager/guest_cli/main.c|  29 +++--
 .../guest_cli/vm_power_cli_guest.c|  28 ++---
 examples/vm_power_manager/main.c  |   2 +-
 lib/librte_power/guest_channel.c  |   2 +-
 lib/librte_power/power_kvm_vm.c   |  14 +--
 lib/librte_power/rte_power_guest_channel.h|  98 +++
 8 files changed, 155 insertions(+), 140 deletions(-)

diff --git a/examples/vm_power_manager/channel_monitor.c 
b/examples/vm_power_manager/channel_monitor.c
index 1b6041b6f..7bb33e026 100644
--- a/examples/vm_power_manager/channel_monitor.c
+++ b/examples/vm_power_manager/channel_monitor.c
@@ -177,10 +177,10 @@ parse_json_to_pkt(json_t *element, struct 
rte_power_channel_packet *pkt,
 
pkt->nb_mac_to_monitor = 0;
pkt->t_boost_status.tbEnabled = false;
-   pkt->workload = LOW;
-   pkt->policy_to_use = TIME;
-   pkt->command = PKT_POLICY;
-   pkt->core_type = CORE_TYPE_PHYSICAL;
+   pkt->workload = RTE_POWER_WL_LOW;
+   pkt->policy_to_use = RTE_POWER_POLICY_TIME;
+   pkt->command = RTE_POWER_PKT_POLICY;
+   pkt->core_type = RTE_POWER_CORE_TYPE_PHYSICAL;
 
if (vm_name == NULL) {
RTE_LOG(ERR, CHANNEL_MONITOR,
@@ -203,11 +203,11 @@ parse_json_to_pkt(json_t *element, struct 
rte_power_channel_packet *pkt,
char command[32];
strlcpy(command, json_string_value(value), 32);
if (!strcmp(command, "power")) {
-   pkt->command = CPU_POWER;
+   pkt->command = RTE_POWER_CPU_POWER;
} else if (!strcmp(command, "create")) {
-   pkt->command = PKT_POLICY;
+   pkt->command = RTE_POWER_PKT_POLICY;
} else if (!strcmp(command, "destroy")) {
-   pkt->command = PKT_POLICY_REMOVE;
+   pkt->command = RTE_POWER_PKT_POLICY_REMOVE;
} else {
RTE_LOG(ERR, CHANNEL_MONITOR,
"Invalid command received in JSON\n");
@@ -217,13 +217,17 @@ parse_json_to_pkt(json_t *element, struct 
rte_power_channel_packet *pkt,
char command[32];
strlcpy(command, json_string_value(value), 32);
if (!strcmp(command, "TIME")) {
-   pkt->policy_to_use = TIME;
+   pkt->policy_to_use =
+   RTE_POWER_POLICY_TIME;
} else if (!strcmp(command, "TRAFFIC")) {
-   pkt->policy_to_use = TRAFFIC;
+   pkt->policy_to_use =
+   RTE_POWER_POLICY_TRAFFIC;
} else if (!strcmp(command, "WORKLOAD")) {
-   pkt->policy_to_use = WORKLOAD;
+   pkt->policy_to_use =
+   RTE_POWER_POLICY_WORKLOAD;
} else if (!strcmp(command, "BRANCH_RATIO")) {
-   pkt->policy_to_use = BRANCH_RATIO;
+   pkt->policy_to_use =
+   RTE_POWER_POLICY_BRANCH_RATIO;
} else {
RTE_LOG(ERR, CHANNEL_MONITOR,
"Wrong policy_type received in JSON\n");
@@ -233,11 +237,11 @@ parse_json_to_pkt(json_t *element, struct 
rte_power_channel_packet *pkt,
char command[32];
strlcpy(command, json_string_value(value), 32);
if (!strcmp(command, "HIGH")) {
-   pkt->workload = HIGH;
+   pkt->workload = RTE_POWER_WL_HIGH;
} else if (!strcmp(command, "MEDIUM")) {
-   pkt->workload = MEDIUM;
+   pkt->workload = RTE_POWER_WL_MEDIUM;
} else if (!strcmp(command, "LOW")) {
-   pkt->workload = LOW;
+   pkt->workload = RTE_POWER_WL_LOW;
} else {
RTE_LOG(ERR, CHANNEL_MONITOR,
"Wrong workload received in JSON\n");
@@ -283,17 +287,17 @@ 

[dpdk-dev] [PATCH v4 5/6] power: add new header file to export list

[David Hunt]

From: Bruce Richardson 

Adjust meson.build so that 'ninja install' copies the new header
file into the installation directory.

Fixes: 210c383e247b ("power: packet format for vm power management")
Fixes: cd0d5547e873 ("power: vm communication channels in guest")
Cc: sta...@dpdk.org

Signed-off-by: Bruce Richardson 
Signed-off-by: David Hunt 
---
 lib/librte_power/meson.build | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/librte_power/meson.build b/lib/librte_power/meson.build
index 4b4cf1b90..541569528 100644
--- a/lib/librte_power/meson.build
+++ b/lib/librte_power/meson.build
@@ -10,5 +10,6 @@ sources = files('rte_power.c', 'power_acpi_cpufreq.c',
'rte_power_empty_poll.c',
'power_pstate_cpufreq.c',
'power_common.c')
-headers = files('rte_power.h','rte_power_empty_poll.h')
+headers = files('rte_power.h','rte_power_empty_poll.h',
+   'rte_power_guest_channel.h')
 deps += ['timer']
-- 
2.17.1

[dpdk-dev] [PATCH v4 6/6] power: clean up includes

[David Hunt]

From: Bruce Richardson 

re-organise the including of the new public header file and
remove un-needed includes

Fixes: 210c383e247b ("power: packet format for vm power management")
Fixes: cd0d5547e873 ("power: vm communication channels in guest")
Cc: sta...@dpdk.org

Signed-off-by: Bruce Richardson 
Signed-off-by: David Hunt 
---
 examples/vm_power_manager/channel_manager.c  | 1 -
 examples/vm_power_manager/channel_monitor.c  | 1 -
 examples/vm_power_manager/channel_monitor.h  | 2 +-
 examples/vm_power_manager/guest_cli/vm_power_cli_guest.c | 1 -
 examples/vm_power_manager/guest_cli/vm_power_cli_guest.h | 2 --
 examples/vm_power_manager/vm_power_cli.c | 1 -
 lib/librte_power/guest_channel.c | 2 +-
 lib/librte_power/guest_channel.h | 2 --
 lib/librte_power/power_kvm_vm.c  | 2 +-
 lib/librte_power/rte_power.h | 1 +
 lib/librte_power/rte_power_guest_channel.h   | 3 ---
 11 files changed, 4 insertions(+), 14 deletions(-)

diff --git a/examples/vm_power_manager/channel_manager.c 
b/examples/vm_power_manager/channel_manager.c
index c7d5bf5a8..0a28cb643 100644
--- a/examples/vm_power_manager/channel_manager.c
+++ b/examples/vm_power_manager/channel_manager.c
@@ -27,7 +27,6 @@
 #include 
 
 #include "channel_manager.h"
-#include "rte_power_guest_channel.h"
 #include "channel_monitor.h"
 #include "power_manager.h"
 
diff --git a/examples/vm_power_manager/channel_monitor.c 
b/examples/vm_power_manager/channel_monitor.c
index 7bb33e026..99f81544d 100644
--- a/examples/vm_power_manager/channel_monitor.c
+++ b/examples/vm_power_manager/channel_monitor.c
@@ -35,7 +35,6 @@
 
 #include 
 #include "channel_monitor.h"
-#include "rte_power_guest_channel.h"
 #include "channel_manager.h"
 #include "power_manager.h"
 #include "oob_monitor.h"
diff --git a/examples/vm_power_manager/channel_monitor.h 
b/examples/vm_power_manager/channel_monitor.h
index 5d3537b91..9184a8327 100644
--- a/examples/vm_power_manager/channel_monitor.h
+++ b/examples/vm_power_manager/channel_monitor.h
@@ -5,8 +5,8 @@
 #ifndef CHANNEL_MONITOR_H_
 #define CHANNEL_MONITOR_H_
 
+#include 
 #include "channel_manager.h"
-#include "rte_power_guest_channel.h"
 
 struct core_share {
unsigned int pcpu;
diff --git a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c 
b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
index ec6409abd..0bf5774ff 100644
--- a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
+++ b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
@@ -19,7 +19,6 @@
 #include 
 
 #include 
-#include 
 
 #include "vm_power_cli_guest.h"
 
diff --git a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.h 
b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.h
index 5d285ca9d..b578ec072 100644
--- a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.h
+++ b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.h
@@ -9,8 +9,6 @@
 extern "C" {
 #endif
 
-#include "rte_power_guest_channel.h"
-
 struct rte_power_channel_packet *get_policy(void);
 
 int set_policy_mac(int port, int idx);
diff --git a/examples/vm_power_manager/vm_power_cli.c 
b/examples/vm_power_manager/vm_power_cli.c
index f7e1b596e..1a55e553b 100644
--- a/examples/vm_power_manager/vm_power_cli.c
+++ b/examples/vm_power_manager/vm_power_cli.c
@@ -21,7 +21,6 @@
 #include "channel_manager.h"
 #include "channel_monitor.h"
 #include "power_manager.h"
-#include "rte_power_guest_channel.h"
 
 struct cmd_quit_result {
cmdline_fixed_string_t quit;
diff --git a/lib/librte_power/guest_channel.c b/lib/librte_power/guest_channel.c
index 039cb1872..2f7507a03 100644
--- a/lib/librte_power/guest_channel.c
+++ b/lib/librte_power/guest_channel.c
@@ -15,9 +15,9 @@
 
 
 #include 
+#include 
 
 #include "guest_channel.h"
-#include "rte_power_guest_channel.h"
 
 #define RTE_LOGTYPE_GUEST_CHANNEL RTE_LOGTYPE_USER1
 
diff --git a/lib/librte_power/guest_channel.h b/lib/librte_power/guest_channel.h
index 7d3a909d9..24f609df3 100644
--- a/lib/librte_power/guest_channel.h
+++ b/lib/librte_power/guest_channel.h
@@ -8,8 +8,6 @@
 extern "C" {
 #endif
 
-#include 
-
 /**
  * Check if any Virtio-Serial VM end-points exist in path.
  *
diff --git a/lib/librte_power/power_kvm_vm.c b/lib/librte_power/power_kvm_vm.c
index 27f9937aa..ab7d4b8ce 100644
--- a/lib/librte_power/power_kvm_vm.c
+++ b/lib/librte_power/power_kvm_vm.c
@@ -6,8 +6,8 @@
 
 #include 
 
-#include "guest_channel.h"
 #include "rte_power_guest_channel.h"
+#include "guest_channel.h"
 #include "power_kvm_vm.h"
 #include "power_common.h"
 
diff --git a/lib/librte_power/rte_power.h b/lib/librte_power/rte_power.h
index bbbde4dfb..c8086bf6b 100644
--- a/lib/librte_power/rte_power.h
+++ b/lib/librte_power/rte_power.h
@@ -14,6 +14,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #ifdef __cplusplus
 extern "C" {
diff --git a/lib/librte_power/rte_power_gue

Re: [dpdk-dev] [PATCH v15 11/12] build: add Arm SoC meson option

[Thomas Monjalon]

21/01/2021 17:12, Bruce Richardson:
> On Thu, Jan 21, 2021 at 04:52:43PM +0100, Thomas Monjalon wrote:
> > 21/01/2021 16:02, Juraj Linkeš:
> > > From: Thomas Monjalon 
> > > > 20/01/2021 09:41, Juraj Linkeš:
> > > > > From: Honnappa Nagarahalli 
> > > > > > > 20/01/2021 02:04, Honnappa Nagarahalli:
> > > > > > > > > On Tue, Jan 19, 2021 at 04:52:19PM +0100, Thomas Monjalon 
> > > > > > > > > wrote:
> > > > > > > > > > 19/01/2021 15:56, Juraj Linkeš:
> > > > > > > > > > > From: Thomas Monjalon 
> > > > > > > > > > > > 15/01/2021 14:26, Juraj Linkeš:
> > > > > > > > > > > > > --- a/meson_options.txt
> > > > > > > > > > > > > +++ b/meson_options.txt
> > > > > > > > > > > > > +option('arm_soc', type: 'string', value: '',
> > > > > > > > > > > > > + description: 'Specify if you want to build for a
> > > > > > > > > > > > > +particular
> > > > > > > > > > > > > +aarch64 Arm SoC when building on an aarch64
> > > > > > > > > > > > > +machine.')
> > > > > > > > > > > >
> > > > > > > > > > > > Why the option is named "arm_soc" and not just "soc"?
> > > > > > > > > > > > The same option could be used by other archs, isn't it?
> > > > > > > > > > >
> > > > > > > > > > > Agree that a more generic name would be better.
> > > > > > > > > > > I'll change it to "soc" if there are no other suggestions.
> > > > > > > > > >
> > > > > > > > > > Another name could be "machine".
> > > > > > > > > > Should it be the same mechanism as compiling for a specific
> > > > > > > > > > x86 CPU from an x86 machine?
> > > > > > > > > >
> > > > > > > > > I'd rather not re-use the term "machine", for a new use,
> > > > > > > > > better to use a new term IMHO.
> > > > > > > > +1, agree. 'soc' sounds good to me.
> > > > > > >
> > > > > > > Another possible word is "platform", as in
> > > > > > > http://doc.dpdk.org/guides/platform/index.html
> > > > > > I am fine with 'platform' too.
> > > > > >
> > > > >
> > > > > 'platform' is likely the best and actually works nicely with
> > > > http://patches.dpdk.org/patch/85956/. Taken together, 'platform' could 
> > > > be
> > > > either 'native', 'generic' or an soc, which is, I believe, exactly what 
> > > > we want.
> > > > 
> > > > I am not sure what we want :)
> > > > We need to specify the instruction set, and the specific target.
> > > > We could deduce the instruction set from the target, but I think it is 
> > > > good to be
> > > > able to overwrite the instruction set in case there can be multiple 
> > > > instruction
> > > > sets for a target.
> > > > 
> > > 
> > > I think we had this (or similar) discussion here 
> > > http://patches.dpdk.org/patch/85956/. I agree with your summary.
> > > 
> > > > I think "native" and "generic" should be specified as instruction set, 
> > > > in the
> > > > existing option "machine" or renamed as "instruction_set" or "isa".
> > > > 
> > > 
> > > Agree, but I would add that we also want "native" and "generic" as valid 
> > > platforms. More below.
> > > 
> > > > Let's imagine the first option is "isa" and the new second option is 
> > > > "platform".
> > > > We can have a default "isa" per "platform".
> > > > The default "platform" would have a default "isa": native or generic?
> > > > 
> > > 
> > > In general, yes, but I let me expand the "platform" option a bit.
> > > 
> > > Let's dig into what "platform" means. I understand it to be a set of 
> > > configuration options, e.g.:
> > > platform=native: use discovered values for all configuration options 
> > > (where we can do that)
> > > platform=generic: use predetermined values to produce a "generic" build 
> > > that would work on most machine of the corresponding type/arch
> > 
> > This is where I was disagreeing:
> > you propose to have 2 special values of platform (native and generic),
> > I propose to have only 1 default value for platform.
> > 
> > After more thoughts, I think it's fine.
> > 
> > 
> > > platform=other: use predetermined values to produce a build tailored to 
> > > platform "other", such as some arm soc.
> > > 
> > > In all these cases, leave user the option to specify supported options 
> > > (i.e. user can specifying "instruction_set" and that value would be used 
> > > for machine args or "max_lcores" would set max lcores).
> > > 
> > > The default "instruction_set" would be different per platform:
> > 
> > What do you think about calling this option "isa"?
> > 
> > > platform=native => instruction_set=native
> > > platform=generic => the generic instruction_set for the architecture, as 
> > > specified here: 
> > > https://github.com/DPDK/dpdk/blob/main/config/meson.build#L79
> > > platform=other => the instruction set of the platform
> > > 
> > > When "instruction_set" is set to its default value (such as auto), the 
> > > per-platform instruction set would be used. If the user specifies 
> > > anything else, that value would be used.
> > 
> > Why auto? This is what we call native.
> > 
> 
> "auto" is a better term. If the platform is selected as "native" then the
> default

Re: [dpdk-dev] [PATCH] build: force pkg-config for dependency detection

[Daly, Lee]

> -Original Message-
> From: Richardson, Bruce 
> Sent: Monday, January 18, 2021 2:30 PM
> To: dev@dpdk.org
> Cc: Yigit, Ferruh ; Richardson, Bruce
> ; sta...@dpdk.org; Matan Azrad
> ; Shahaf Shuler ; Viacheslav
> Ovsiienko ; Liron Himi ;
> Trahe, Fiona ; Griffin, John
> ; Jain, Deepak K ; Daly,
> Lee ; Ashish Gupta ;
> Sunila Sahu ; Ruifeng Wang
> ; Somalapuram Amaranath
> ; Michael Shamis ;
> Doherty, Declan ; Loftus, Ciara
> ; Zhang, Qi Z ; Rasesh Mody
> ; Shahed Shaikh ; Zyta
> Szpak ; Martin Spinler ; Hunt, David
> ; Ananyev, Konstantin
> 
> Subject: [PATCH] build: force pkg-config for dependency detection
> 
> Meson can use cmake as a fallback for detecting packages, and this can lead
> to picking up 64-libs for 32-bit builds. To work around this, force the use of
> pkg-config only for detecting libcrypto, zlib, jansson and other package
> dependencies.
> 
> CC: sta...@dpdk.org
> 
> Signed-off-by: Bruce Richardson 
> ---
> 
Tested ISA-L driver change. Thanks.
Tested-by: Lee Daly 

Re: [dpdk-dev] Crypto API for AES-XTS cipher algorithm

[Matan Azrad]

Hi Akhil

Yes, you right regards terms of data-unit and block size.
My intention was for data-unit.

AES-XTS algorithm is a mode of AES so by definition the block size is 16B.
So, no need capability for it.

Also in the symmetric operation structure, the `length` field is described as 
next:
/**< The message length, in bytes, of the source buffer
  * on which the cryptographic operation will be
  * computed. This must be a multiple of the block size
  */

It doesn't make sense to limit the user for buffer size which is multiple of 
16B, one of the main reasons of AES with XTS mode is to remove this limitation 
of AES.

Also, the data-unit size is important parameter in AES-XTS, because encryption 
and decryption must use the same size of it - so the PMD cannot just guess it 
from the `length` field.

IMO:
Instead of block size capability it is better to use data-unit size capability.

Instead of block size limitation it is better to limit the buffer to be 
multiple of data-units.


It even will be good also to add this data-unit size as configuration in the 
transformation structure and to let the user know the supported sizes in the 
driver (capability or other mechanism).

What do you think?

Matan

From: Akhil Goyal 
Sent: Monday, January 18, 2021 5:27 PM
To: Matan Azrad ; Declan Doherty 
Cc: dev@dpdk.org
Subject: RE: Crypto API for AES-XTS cipher algorithm

External email: Use caution opening links or attachments

Hi Matan,

Block size is specified in the capability structure and is expected to be same 
for a particular algorithm.
And for AES-XTS it is 16 bytes only if I am not wrong.

As per my understanding, data unit is different from block size.
Data unit is the input data which may or may not be multiple of block size. 
There are different handling of data
Unit defined if it a multiple of block size or not. And I believe there is 
limitation for the max value of data unit
Which the driver can give error if it does not support that particular size.

Regards,
Akhil

From: Matan Azrad mailto:ma...@nvidia.com>>
Sent: Monday, January 18, 2021 8:19 PM
To: Declan Doherty mailto:declan.dohe...@intel.com>>; 
Akhil Goyal mailto:akhil.go...@nxp.com>>
Cc: dev@dpdk.org
Subject: Crypto API for AES-XTS cipher algorithm

Hi Declan, Akhil

We are going to implement mlx5 crypto PMD to support AES-XTS de\encrypt 
operations.

The algorithm defines block size >= 16Bytes (it is called also data-unit)which 
should be known for encryption\decryptions.

I didn't find this parameterin the cypher xform.

How do you suggest to add it? maybe I'm missing something?

Matan

[dpdk-dev] [PATCH v3] net/ice: drain out DCF AdminQ command queue

[Haiyue Wang]

The virtchnl message is handled one by one by checking opcode to match
the response for the request.

The DCF AdminQ command with buffer needs two virtchnl commands, one is
to handle the AdminQ descriptor, the other is to the handle AdminQ
buffer. If both of them are sent to PF successfully, it needs to wait
two responses from PF, even if the AdminQ descriptor command gets the
failure response. Since PF will handle them one by one, and send back
the response for each.

If not wait for the buffer message response until timeout to drain out
the virtchnl command queue, it will cause the next AdminQ command with
buffer to get the stall buffer response from previous.

Fixes: daa714d55c72 ("net/ice: handle AdminQ command by DCF")
Cc: sta...@dpdk.org

Signed-off-by: Haiyue Wang 
---
v3: Reword the commit message, and remove the no needed checking.
v2: Fix the commit message typo : 'matchiing' should be 'matching'
---
 drivers/net/ice/ice_dcf.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ice/ice_dcf.c b/drivers/net/ice/ice_dcf.c
index 4a9af3292c..d947d02c5b 100644
--- a/drivers/net/ice/ice_dcf.c
+++ b/drivers/net/ice/ice_dcf.c
@@ -505,9 +505,7 @@ ice_dcf_send_aq_cmd(void *dcf_hw, struct ice_aq_desc *desc,
}
 
do {
-   if ((!desc_cmd.pending && !buff_cmd.pending) ||
-   (!desc_cmd.pending && desc_cmd.v_ret != IAVF_SUCCESS) ||
-   (!buff_cmd.pending && buff_cmd.v_ret != IAVF_SUCCESS))
+   if (!desc_cmd.pending && !buff_cmd.pending)
break;
 
rte_delay_ms(ICE_DCF_ARQ_CHECK_TIME);
-- 
2.30.0

[dpdk-dev] [PATCH v2] app/testpmd: tx pkt clones parameter in flowgen

[Igor Russkikh]

When testing high performance numbers, it is often that CPU performance
limits the max values device can reach (both in pps and in gbps)

Here instead of recreating each packet separately, we use clones counter
to resend the same mbuf to the line multiple times.

PMDs handle that transparently due to reference counting inside of mbuf.

Reaching max PPS on small packet sizes helps here:
Some data from our 2 port x 50G device. Using 2*6 tx queues, 64b packets,
PowerEdge R7525, AMD EPYC 7452:

./build/app/dpdk-testpmd -l 32-63  -- --forward-mode=flowgen \
  --rxq=6 --txq=6  --disable-crc-strip --burst=512 \
  --flowgen-clones=0 --txd=4096 --stats-period=1 --txpkts=64

Gives ~46MPPS TX output:

  Tx-pps: 22926849  Tx-bps:  11738590176
  Tx-pps: 23642629  Tx-bps:  12105024112

Setting flowgen-clones to 512 pushes TX almost to our device
physical limit (68MPPS) using same 2*6 queues(cores):

  Tx-pps: 34357556  Tx-bps:  17591073696
  Tx-pps: 34353211  Tx-bps:  17588802640

Doing similar measurements per core, I see one core can do
6.9MPPS (without clones) vs 11MPPS (with clones)

Verified on Marvell qede and atlantic PMDs.

v2:
  - fixed warning on uninit var
v1:
  - fixes on Ferruh's comments

rfc v2: http://patchwork.dpdk.org/patch/78800/
  - increment ref counter for each mbuf pointer copy
rfc v1: http://patchwork.dpdk.org/patch/78674/

Signed-off-by: Igor Russkikh 
---
 app/test-pmd/flowgen.c| 107 ++
 app/test-pmd/parameters.c |  10 +++
 app/test-pmd/testpmd.c|   1 +
 app/test-pmd/testpmd.h|   1 +
 doc/guides/testpmd_app_ug/run_app.rst |   7 ++
 5 files changed, 78 insertions(+), 48 deletions(-)

diff --git a/app/test-pmd/flowgen.c b/app/test-pmd/flowgen.c
index acf3e2460..82383f265 100644
--- a/app/test-pmd/flowgen.c
+++ b/app/test-pmd/flowgen.c
@@ -85,7 +85,7 @@ pkt_burst_flow_gen(struct fwd_stream *fs)
unsigned pkt_size = tx_pkt_length - 4;  /* Adjust FCS */
struct rte_mbuf  *pkts_burst[MAX_PKT_BURST];
struct rte_mempool *mbp;
-   struct rte_mbuf  *pkt;
+   struct rte_mbuf  *pkt = NULL;
struct rte_ether_hdr *eth_hdr;
struct rte_ipv4_hdr *ip_hdr;
struct rte_udp_hdr *udp_hdr;
@@ -94,6 +94,7 @@ pkt_burst_flow_gen(struct fwd_stream *fs)
uint16_t nb_rx;
uint16_t nb_tx;
uint16_t nb_pkt;
+   uint16_t nb_clones = nb_pkt_flowgen_clones;
uint16_t i;
uint32_t retry;
uint64_t tx_offloads;
@@ -123,53 +124,63 @@ pkt_burst_flow_gen(struct fwd_stream *fs)
ol_flags |= PKT_TX_MACSEC;
 
for (nb_pkt = 0; nb_pkt < nb_pkt_per_burst; nb_pkt++) {
-   pkt = rte_mbuf_raw_alloc(mbp);
-   if (!pkt)
-   break;
-
-   pkt->data_len = pkt_size;
-   pkt->next = NULL;
-
-   /* Initialize Ethernet header. */
-   eth_hdr = rte_pktmbuf_mtod(pkt, struct rte_ether_hdr *);
-   rte_ether_addr_copy(&cfg_ether_dst, ð_hdr->d_addr);
-   rte_ether_addr_copy(&cfg_ether_src, ð_hdr->s_addr);
-   eth_hdr->ether_type = rte_cpu_to_be_16(RTE_ETHER_TYPE_IPV4);
-
-   /* Initialize IP header. */
-   ip_hdr = (struct rte_ipv4_hdr *)(eth_hdr + 1);
-   memset(ip_hdr, 0, sizeof(*ip_hdr));
-   ip_hdr->version_ihl = RTE_IPV4_VHL_DEF;
-   ip_hdr->type_of_service = 0;
-   ip_hdr->fragment_offset = 0;
-   ip_hdr->time_to_live= IP_DEFTTL;
-   ip_hdr->next_proto_id   = IPPROTO_UDP;
-   ip_hdr->packet_id   = 0;
-   ip_hdr->src_addr= rte_cpu_to_be_32(cfg_ip_src);
-   ip_hdr->dst_addr= rte_cpu_to_be_32(cfg_ip_dst +
-  next_flow);
-   ip_hdr->total_length= RTE_CPU_TO_BE_16(pkt_size -
-  sizeof(*eth_hdr));
-   ip_hdr->hdr_checksum= ip_sum((unaligned_uint16_t *)ip_hdr,
-sizeof(*ip_hdr));
-
-   /* Initialize UDP header. */
-   udp_hdr = (struct rte_udp_hdr *)(ip_hdr + 1);
-   udp_hdr->src_port   = rte_cpu_to_be_16(cfg_udp_src);
-   udp_hdr->dst_port   = rte_cpu_to_be_16(cfg_udp_dst);
-   udp_hdr->dgram_cksum= 0; /* No UDP checksum. */
-   udp_hdr->dgram_len  = RTE_CPU_TO_BE_16(pkt_size -
-  sizeof(*eth_hdr) -
-  sizeof(*ip_hdr));
-   pkt->nb_segs= 1;
-   pkt->pkt_len= pkt_size;
-   pkt->ol_flags   &= EXT_ATTACHED_MBUF;
-   pkt->ol_flags   |= ol_flags;
- 

Re: [dpdk-dev] Crypto API for AES-XTS cipher algorithm

[Akhil Goyal]

Hi Matan,

Can you send an RFC patch for the changes you are looking forward in the 
cryptodev lib.

Regards,
Akhil

From: Matan Azrad 
Sent: Thursday, January 21, 2021 11:16 PM
To: Akhil Goyal ; Declan Doherty 
Cc: dev@dpdk.org
Subject: RE: Crypto API for AES-XTS cipher algorithm

Hi Akhil

Yes, you right regards terms of data-unit and block size.
My intention was for data-unit.

AES-XTS algorithm is a mode of AES so by definition the block size is 16B.
So, no need capability for it.

Also in the symmetric operation structure, the `length` field is described as 
next:
/**< The message length, in bytes, of the source buffer
  * on which the cryptographic operation will be
  * computed. This must be a multiple of the block size
  */

It doesn't make sense to limit the user for buffer size which is multiple of 
16B, one of the main reasons of AES with XTS mode is to remove this limitation 
of AES.

Also, the data-unit size is important parameter in AES-XTS, because encryption 
and decryption must use the same size of it - so the PMD cannot just guess it 
from the `length` field.


IMO:
Instead of block size capability it is better to use data-unit size capability.

Instead of block size limitation it is better to limit the buffer to be 
multiple of data-units.


It even will be good also to add this data-unit size as configuration in the 
transformation structure and to let the user know the supported sizes in the 
driver (capability or other mechanism).

What do you think?

Matan

From: Akhil Goyal mailto:akhil.go...@nxp.com>>
Sent: Monday, January 18, 2021 5:27 PM
To: Matan Azrad mailto:ma...@nvidia.com>>; Declan Doherty 
mailto:declan.dohe...@intel.com>>
Cc: dev@dpdk.org
Subject: RE: Crypto API for AES-XTS cipher algorithm

External email: Use caution opening links or attachments

Hi Matan,

Block size is specified in the capability structure and is expected to be same 
for a particular algorithm.
And for AES-XTS it is 16 bytes only if I am not wrong.

As per my understanding, data unit is different from block size.
Data unit is the input data which may or may not be multiple of block size. 
There are different handling of data
Unit defined if it a multiple of block size or not. And I believe there is 
limitation for the max value of data unit
Which the driver can give error if it does not support that particular size.

Regards,
Akhil

From: Matan Azrad mailto:ma...@nvidia.com>>
Sent: Monday, January 18, 2021 8:19 PM
To: Declan Doherty mailto:declan.dohe...@intel.com>>; 
Akhil Goyal mailto:akhil.go...@nxp.com>>
Cc: dev@dpdk.org
Subject: Crypto API for AES-XTS cipher algorithm

Hi Declan, Akhil

We are going to implement mlx5 crypto PMD to support AES-XTS de\encrypt 
operations.

The algorithm defines block size >= 16Bytes (it is called also data-unit)which 
should be known for encryption\decryptions.

I didn't find this parameterin the cypher xform.

How do you suggest to add it? maybe I'm missing something?

Matan

[dpdk-dev] [PATCH] doc: update i40e PMD to support on windows

[Pallavi Kadam]

Add documentation to support i40e PMD on Windows.
Update the release notes for the same.

Signed-off-by: Pallavi Kadam 
Reviewed-by: Ranjit Menon 
---
 doc/guides/nics/i40e.rst   | 16 ++--
 doc/guides/rel_notes/release_21_02.rst |  4 
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/doc/guides/nics/i40e.rst b/doc/guides/nics/i40e.rst
index 20c998398..7bdaea8c0 100644
--- a/doc/guides/nics/i40e.rst
+++ b/doc/guides/nics/i40e.rst
@@ -46,8 +46,8 @@ Features of the i40e PMD are:
 - Malicious Device Drive event catch and notify
 - Generic flow API
 
-Prerequisites
--
+Linux Prerequisites
+---
 
 - Identifying your adapter using `Intel Support
   `_ and get the latest NVM/FW images.
@@ -75,6 +75,18 @@ Prerequisites
   * In all cases Intel recommends using Intel Ethernet Optics; other 
modules
 may function but are not validated by Intel. Contact Intel for 
supported media types.
 
+Windows Prerequisites
+-
+
+- Follow the DPDK `Getting Started Guide for Windows 
`_ to setup the basic DPDK 
environment.
+
+- Identify the Intel?? Ethernet adapter and get the latest NVM/FW version.
+
+- To access any Intel?? Ethernet hardware, load the UIO driver in place of 
existing built-in (inbox) driver.
+
+- To load UIO driver, follow the steps mentioned in `dpdk-kmods repository
+  
`_.
+
 Recommended Matching List
 -
 
diff --git a/doc/guides/rel_notes/release_21_02.rst 
b/doc/guides/rel_notes/release_21_02.rst
index ae36b6a3f..176c6a66f 100644
--- a/doc/guides/rel_notes/release_21_02.rst
+++ b/doc/guides/rel_notes/release_21_02.rst
@@ -82,6 +82,10 @@ New Features
 
   * Added support for 64B completion queue entries
 
+* **Updated Intel i40e driver.**
+
+  * Added support on Windows.
+
 * **Updated Intel ice driver.**
 
   * Added Double VLAN support for DCF switch QinQ filtering.
-- 
2.25.4

Re: [dpdk-dev] [PATCH v4 02/22] app/testpmd: fix max rx packet length for VLAN packets

[Lance Richardson]

On Mon, Jan 18, 2021 at 2:08 AM Steve Yang  wrote:
>
> When the max rx packet length is smaller than the sum of mtu size and
> ether overhead size, it should be enlarged, otherwise the VLAN packets
> will be dropped.
>
> Removed the rx_offloads assignment for jumbo frame during command line
> parsing, and set the correct jumbo frame flag if MTU size is larger than
> the default value 'RTE_ETHER_MTU' within 'init_config()'.
>
> Fixes: 384161e00627 ("app/testpmd: adjust on the fly VLAN configuration")
> Fixes: 35b2d13fd6fd ("net: add rte prefix to ether defines")
> Fixes: ce17eddefc20 ("ethdev: introduce Rx queue offloads API")
> Fixes: 150c9ac2df13 ("app/testpmd: update Rx offload after setting MTU")
>
> Cc: Wenzhuo Lu 
> Cc: Beilei Xing 
> Cc: Bernard Iremonger 
>
> Signed-off-by: Steve Yang 
> ---
>  app/test-pmd/cmdline.c|  6 --
>  app/test-pmd/config.c |  2 +-
>  app/test-pmd/parameters.c |  7 ++-
>  app/test-pmd/testpmd.c| 18 ++
>  4 files changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 2ccbaa039e..65042fcff5 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -1886,7 +1886,6 @@ cmd_config_max_pkt_len_parsed(void *parsed_result,
>
> RTE_ETH_FOREACH_DEV(pid) {
> struct rte_port *port = &ports[pid];
> -   uint64_t rx_offloads = port->dev_conf.rxmode.offloads;
>
> if (!strcmp(res->name, "max-pkt-len")) {
> if (res->value < RTE_ETHER_MIN_LEN) {
> @@ -1898,11 +1897,6 @@ cmd_config_max_pkt_len_parsed(void *parsed_result,
> return;
>
> port->dev_conf.rxmode.max_rx_pkt_len = res->value;
> -   if (res->value > RTE_ETHER_MAX_LEN)
> -   rx_offloads |= DEV_RX_OFFLOAD_JUMBO_FRAME;
> -   else
> -   rx_offloads &= ~DEV_RX_OFFLOAD_JUMBO_FRAME;
> -   port->dev_conf.rxmode.offloads = rx_offloads;
> } else {
> printf("Unknown parameter\n");
> return;
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 3f6c8642b1..1195f054f3 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -1434,7 +1434,7 @@ port_mtu_set(portid_t port_id, uint16_t mtu)
>  * device supports jumbo frame.
>  */
> eth_overhead = dev_info.max_rx_pktlen - dev_info.max_mtu;
> -   if (mtu > RTE_ETHER_MAX_LEN - eth_overhead) {
> +   if (mtu > RTE_ETHER_MTU) {
> rte_port->dev_conf.rxmode.offloads |=
> DEV_RX_OFFLOAD_JUMBO_FRAME;
> rte_port->dev_conf.rxmode.max_rx_pkt_len =
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> index 414a0068fb..df5eb10d84 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -834,12 +834,9 @@ launch_args_parse(int argc, char** argv)
> }
> if (!strcmp(lgopts[opt_idx].name, "max-pkt-len")) {
> n = atoi(optarg);
> -   if (n >= RTE_ETHER_MIN_LEN) {
> +   if (n >= RTE_ETHER_MIN_LEN)
> rx_mode.max_rx_pkt_len = (uint32_t) n;
> -   if (n > RTE_ETHER_MAX_LEN)
> -   rx_offloads |=
> -   
> DEV_RX_OFFLOAD_JUMBO_FRAME;
> -   } else
> +   else
> rte_exit(EXIT_FAILURE,
>  "Invalid max-pkt-len=%d - 
> should be > %d\n",
>  n, RTE_ETHER_MIN_LEN);
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 2b60f6c5d3..c256e719ae 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -1410,6 +1410,7 @@ init_config(void)
> struct rte_gro_param gro_param;
> uint32_t gso_types;
> uint16_t data_size;
> +   uint16_t eth_overhead;
> bool warning = 0;
> int k;
> int ret;
> @@ -1446,6 +1447,23 @@ init_config(void)
> rte_exit(EXIT_FAILURE,
>  "rte_eth_dev_info_get() failed\n");
>
> +   /* Update the max_rx_pkt_len to have MTU as RTE_ETHER_MTU */
> +   if (port->dev_info.max_mtu != UINT16_MAX &&
> +   port->dev_info.max_rx_pktlen > port->dev_info.max_mtu)
> +   eth_overhead = port->dev_info.max_rx_pktlen -
> +   port->dev_info.max_mtu;
> +   else
> +   

Re: [dpdk-dev] [PATCH v3] net/bnxt: code refactoring changes

[Ajit Khaparde]

On Wed, Jan 20, 2021 at 11:15 PM Ajit Khaparde  wrote:
>
> From: Somnath Kotur 
>
> Move all the individual driver fields allocation routines to one
> routine - bnxt_drv_init(). This houses all such routines where
> memory needs to be allocated once during the driver's lifetime
> and does not need to be torn down during error recovery.
> Rename some function names in accordance with their functionality.
> bnxt_init_board() is doing nothing more than mapping the PCI bars,
> so rename it as such.
> Given that there is a bnxt_shutdown_nic that is called in dev_stop_op,
> rename it's counterpart - bnxt_init_chip() that is called in dev_start_op,
> to bnxt_start_nic. Also helps avoid confusion with some of the other
> bnxt_init_xxx routines.
> Rename bnxt_init_fw() to bnxt_get_config() as that is what that routine
> is doing mostly functionality wise.
>
> Cc: sta...@dpdk.org
> Signed-off-by: Somnath Kotur 
> Reviewed-by: Ajit Khaparde 
Patch applied to dpdk-next-net-brcm.

> ---
> v1->v2: rebased against latest dpdk-next-net.
> v2->v3: rebased against latest dpdk-next-net.
> ---
>  drivers/net/bnxt/bnxt_ethdev.c | 150 +++--
>  1 file changed, 88 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
> index 74b0f3d1d..f439aeee4 100644
> --- a/drivers/net/bnxt/bnxt_ethdev.c
> +++ b/drivers/net/bnxt/bnxt_ethdev.c
> @@ -679,7 +679,7 @@ static int bnxt_update_phy_setting(struct bnxt *bp)
> return rc;
>  }
>
> -static int bnxt_init_chip(struct bnxt *bp)
> +static int bnxt_start_nic(struct bnxt *bp)
>  {
> struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(bp->eth_dev);
> struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
> @@ -1417,7 +1417,7 @@ static int bnxt_dev_start_op(struct rte_eth_dev 
> *eth_dev)
>
> bnxt_enable_int(bp);
>
> -   rc = bnxt_init_chip(bp);
> +   rc = bnxt_start_nic(bp);
> if (rc)
> goto error;
>
> @@ -1464,6 +1464,27 @@ bnxt_uninit_locks(struct bnxt *bp)
> }
>  }
>
> +static void bnxt_drv_uninit(struct bnxt *bp)
> +{
> +   bnxt_free_switch_domain(bp);
> +   bnxt_free_leds_info(bp);
> +   bnxt_free_cos_queues(bp);
> +   bnxt_free_link_info(bp);
> +   bnxt_free_pf_info(bp);
> +   bnxt_free_parent_info(bp);
> +   bnxt_uninit_locks(bp);
> +
> +   rte_memzone_free((const struct rte_memzone *)bp->tx_mem_zone);
> +   bp->tx_mem_zone = NULL;
> +   rte_memzone_free((const struct rte_memzone *)bp->rx_mem_zone);
> +   bp->rx_mem_zone = NULL;
> +
> +   bnxt_hwrm_free_vf_info(bp);
> +
> +   rte_free(bp->grp_info);
> +   bp->grp_info = NULL;
> +}
> +
>  static int bnxt_dev_close_op(struct rte_eth_dev *eth_dev)
>  {
> struct bnxt *bp = eth_dev->data->dev_private;
> @@ -1488,26 +1509,9 @@ static int bnxt_dev_close_op(struct rte_eth_dev 
> *eth_dev)
> if (eth_dev->data->dev_started)
> ret = bnxt_dev_stop(eth_dev);
>
> -   bnxt_free_switch_domain(bp);
> -
> bnxt_uninit_resources(bp, false);
>
> -   bnxt_free_leds_info(bp);
> -   bnxt_free_cos_queues(bp);
> -   bnxt_free_link_info(bp);
> -   bnxt_free_pf_info(bp);
> -   bnxt_free_parent_info(bp);
> -   bnxt_uninit_locks(bp);
> -
> -   rte_memzone_free((const struct rte_memzone *)bp->tx_mem_zone);
> -   bp->tx_mem_zone = NULL;
> -   rte_memzone_free((const struct rte_memzone *)bp->rx_mem_zone);
> -   bp->rx_mem_zone = NULL;
> -
> -   bnxt_hwrm_free_vf_info(bp);
> -
> -   rte_free(bp->grp_info);
> -   bp->grp_info = NULL;
> +   bnxt_drv_uninit(bp);
>
> return ret;
>  }
> @@ -4086,7 +4090,7 @@ bool bnxt_stratus_device(struct bnxt *bp)
> }
>  }
>
> -static int bnxt_init_board(struct rte_eth_dev *eth_dev)
> +static int bnxt_map_pci_bars(struct rte_eth_dev *eth_dev)
>  {
> struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
> struct bnxt *bp = eth_dev->data->dev_private;
> @@ -4723,7 +4727,11 @@ static int bnxt_map_hcomm_fw_status_reg(struct bnxt 
> *bp)
> return 0;
>  }
>
> -static int bnxt_init_fw(struct bnxt *bp)
> +/* This function gets the FW version along with the
> + * capabilities(MAX and current) of the function, vnic,
> + * error recovery, phy and other chip related info
> + */
> +static int bnxt_get_config(struct bnxt *bp)
>  {
> uint16_t mtu;
> int rc = 0;
> @@ -4819,7 +4827,7 @@ static int bnxt_init_resources(struct bnxt *bp, bool 
> reconfig_dev)
>  {
> int rc = 0;
>
> -   rc = bnxt_init_fw(bp);
> +   rc = bnxt_get_config(bp);
> if (rc)
> return rc;
>
> @@ -5266,38 +5274,14 @@ static int bnxt_alloc_switch_domain(struct bnxt *bp)
> return rc;
>  }
>
> -static int
> -bnxt_dev_init(struct rte_eth_dev *eth_dev, void *params __rte_unused)
> +/* Allocate and initialize various fields in bnxt struct that
> + * need to be allocated/d

Re: [dpdk-dev] [PATCH v3] net/bnxt: fix lock handling in stop and close

[Ajit Khaparde]

On Wed, Jan 20, 2021 at 11:17 PM Ajit Khaparde  wrote:
>
> From: Somnath Kotur 
>
> err_recovery_lock needs to be released before returning in
> stop and close_op if FW_RESET flag is set.
>
> Fixes: dd3613560573 ("net/bnxt: check chip reset in dev stop and close")
> Signed-off-by: Somnath Kotur 
> Reviewed-by: Ajit Khaparde 
Patch applied to dpdk-next-net-brcm.

> ---
> v1->v2: rebased to latest dpdk-next-net and updated commit log.
> v2->v3: rebased to latest dpdk-next-net.
> ---
>  drivers/net/bnxt/bnxt_ethdev.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
> index f439aeee4..89e42ef14 100644
> --- a/drivers/net/bnxt/bnxt_ethdev.c
> +++ b/drivers/net/bnxt/bnxt_ethdev.c
> @@ -1374,6 +1374,7 @@ static int bnxt_dev_stop_op(struct rte_eth_dev *eth_dev)
> if (bp->flags & BNXT_FLAG_FW_RESET) {
> PMD_DRV_LOG(ERR,
> "Adapter recovering from error..Please retry\n");
> +   pthread_mutex_unlock(&bp->err_recovery_lock);
> return -EAGAIN;
> }
> pthread_mutex_unlock(&bp->err_recovery_lock);
> @@ -1497,6 +1498,7 @@ static int bnxt_dev_close_op(struct rte_eth_dev 
> *eth_dev)
> if (bp->flags & BNXT_FLAG_FW_RESET) {
> PMD_DRV_LOG(ERR,
> "Adapter recovering from error...Please retry\n");
> +   pthread_mutex_unlock(&bp->err_recovery_lock);
> return -EAGAIN;
> }
> pthread_mutex_unlock(&bp->err_recovery_lock);
> --
> 2.21.1 (Apple Git-122.3)
>

Re: [dpdk-dev] [dpdk-stable] [PATCH] vdpa/mlx5: fix configuration mutex cleanup

[Matan Azrad]

From: Maxime Coquelin
> On 1/14/21 4:23 PM, Matan Azrad wrote:
> >
> >
> > From: Maxime Coquelin
> >> On 1/14/21 2:09 PM, Matan Azrad wrote:
> >>>
> >>>
> >>> From: Maxime Coquelin
>  Hi Matan,
> 
>  On 1/14/21 12:49 PM, Matan Azrad wrote:
> > Hi Maxime and David
> >
> > Thank you for Review.
> >
> > From: David Marchand
> >> On Fri, Jan 8, 2021 at 9:48 AM David Marchand
> >>  wrote:
>  I wonder if it would be possible and cleaner to disable
>  cancellation on the thread while the mutex is held?
> >
> > Yes, we can cause thread to return by some global variable sync.
> > It is the same logic.
> 
>  No, that was not my suggestion. My suggestion is to block the
>  thread cancellation while in the critical section, using
> pthread_setcancelstate().
> >>>
> >>> Yes, Generally it is better to let the thread control his
> >>> cancellation, either
> >> cancel itself or enabling\disabling cancellations.
> >>>
> >>> I don't see a reason to wait for the thread in current logic - the
> >>> critical section
> >> is not important to be completed here.
> >>
> >> The reason I see is there are quite a few things done in this
> >> critical section. And if tomorrow someone add new things in it, he
> >> may not know the thread can be cancelled at any time, which could cause
> hard to debug issues.
> >
> > As I said, here it is not needed, this thread designed just to cause guest
> notifications.
> >
> > The optional future developer mistake can be done also outside the critical
> section in in any other place - we cannot protect it.
> >
> > The design choice is to close the thread fast.
> 
> But why is it so urgent that it cannot been stopped cleanly?
> I don't think it would add seconds delay by doing it in a clean way.

We have system calls there per queue.
No need this optional delay just because of mutex cleaning. 


 
> Thanks,
> Maxime
> 
> >>> We just want to close the thread and to clean the mutex.
> >>>
> >>> +1
> >>
> >> IEEE Std 1003.1-2001/Cor 2-2004, item XBD/TC2/D6/26 is applied,
> >> adding pthread_t to the list of types that are not required to be
> >> arithmetic types, thus allowing pthread_t to be defined as a structure.
> >>
> >> It would be better to leave pthread_t alone and not interpret it:
> >>
> >> if (priv->timer_tid) {
> >> pthread_cancel(priv->timer_tid);
> >> pthread_join(priv->timer_tid, &status); }
> >> priv->timer_tid = 0;
> >
> >
> > I'm not sure why you think it is better in this specific case.
> > The cancellation will close the thread in faster way, no need to
> > wait for the
>  thread to close itself.
> >
> >
> >>
> >> --
> >> David Marchand
> >
> >>>
> >

Re: [dpdk-dev] [PATCH] doc: update i40e PMD to support on windows

[Thomas Monjalon]

21/01/2021 20:09, Pallavi Kadam:
> Add documentation to support i40e PMD on Windows.
> Update the release notes for the same.
> 
> Signed-off-by: Pallavi Kadam 
> Reviewed-by: Ranjit Menon 
> ---
>  doc/guides/nics/i40e.rst   | 16 ++--
>  doc/guides/rel_notes/release_21_02.rst |  4 
>  2 files changed, 18 insertions(+), 2 deletions(-)

The update of doc/guides/nics/features/i40e.ini is missing.
Please see the line "Windows" in mlx5.ini for comparison.

[...]
> +- To access any Intel® Ethernet hardware, load the UIO driver in place of 
> existing built-in (inbox) driver.
> +
> +- To load UIO driver, follow the steps mentioned in `dpdk-kmods repository
> +  
> `_.

I think we should say NetUIO instead of UIO.

Re: [dpdk-dev] [PATCH v2 00/19] ensure headers have correct includes

[Thomas Monjalon]

21/01/2021 16:15, Bruce Richardson:
> On Thu, Jan 21, 2021 at 10:36:18AM +0100, Thomas Monjalon wrote:
> > 21/01/2021 10:33, Bruce Richardson:
> > > On Thu, Jan 21, 2021 at 10:25:07AM +0100, David Marchand wrote:
> > > > Is rte_byteorder.h inclusion in rte_mbuf.h still necessary?
> > > >
> > > Good question. The checks I was doing were only for missing headers.
> > > Checking for superfluous headers is more complicated and not something 
> > > I've
> > > looked at. I know it was previously suggested to use the
> > > "include-what-you-use" tool on DPDK, if anyone wants to attempt that.
> > 
> > I would love having an integration of include-what-you-use in our build 
> > system.
> > I don't know whether it's trivial or difficult with meson.
> > 
> 
> Did some initial investigation here, and here are some basic instructions
> based on how I got it running on my Ubuntu 20.10 system. The positive is
> that "iwyu_tool" can run on DPDK build dirs without any extra work on our
> part.

I had looked at it recently but didn't try thinking meson was not compatible.
Thank you, it works :)

> 1. Install iwyu from Ubuntu packaging.
> 2. Install clang-9 package:
>   * This is not a dependency of iwyu, but probably should be
>   * The version of clang installed *must* match that used to build iwyu
> [I had clang-10 installed which didn't work for this]
>   * If you get errors about missing standard headers e.g. stddef.h,
> the version mismatch is likely the issue.
> 3. Configure a clang build of DPDK:
>   * CC=clang meson  build-clang
> 4. "iwyu_tool" supports the build database format used by meson, so just
>run: "iwyu_tool -p build-clang"
> 
> Be warned, this runs really slowly and produces lots of output!

We need to filter only reports of useless includes
and do the cleaning...

The difficult part is that some includes may be useless on Linux
but required on FreeBSD.

[dpdk-dev] [PATCH 1/1] lib: fix doxygen for parameters of function pointers

[Thomas Monjalon]

Some parameters of typedef'ed function pointers were not properly listed
in the doxygen comments.
The error is seen with doxygen 1.9 which added this specific check:
https://github.com/doxygen/doxygen/commit/d34236ba4037

Cc: sta...@dpdk.org

Signed-off-by: Thomas Monjalon 
---
 lib/librte_compressdev/rte_compressdev_pmd.h |  2 ++
 lib/librte_cryptodev/rte_cryptodev_pmd.h | 12 ++--
 lib/librte_eal/include/rte_keepalive.h   |  2 +-
 lib/librte_eventdev/rte_eventdev_pmd.h   | 12 +++-
 lib/librte_port/rte_port.h   |  2 +-
 lib/librte_port/rte_swx_port.h   |  4 ++--
 lib/librte_rawdev/rte_rawdev_pmd.h   | 18 --
 lib/librte_security/rte_security_driver.h|  7 ---
 lib/librte_table/rte_swx_table.h |  6 --
 lib/librte_table/rte_table.h |  4 ++--
 10 files changed, 41 insertions(+), 28 deletions(-)

diff --git a/lib/librte_compressdev/rte_compressdev_pmd.h 
b/lib/librte_compressdev/rte_compressdev_pmd.h
index d5898a5b71..52e64eab03 100644
--- a/lib/librte_compressdev/rte_compressdev_pmd.h
+++ b/lib/librte_compressdev/rte_compressdev_pmd.h
@@ -138,6 +138,8 @@ typedef void (*compressdev_stats_reset_t)(struct 
rte_compressdev *dev);
  *
  * @param dev
  *   Compress device
+ * @param dev_info
+ *   Compress device informations to populate
  */
 typedef void (*compressdev_info_get_t)(struct rte_compressdev *dev,
struct rte_compressdev_info *dev_info);
diff --git a/lib/librte_cryptodev/rte_cryptodev_pmd.h 
b/lib/librte_cryptodev/rte_cryptodev_pmd.h
index 9a8a7e632b..1274436870 100644
--- a/lib/librte_cryptodev/rte_cryptodev_pmd.h
+++ b/lib/librte_cryptodev/rte_cryptodev_pmd.h
@@ -121,7 +121,7 @@ extern struct rte_cryptodev *rte_cryptodevs;
  * Function used to configure device.
  *
  * @param  dev Crypto device pointer
- * config  Crypto device configurations
+ * @param  config  Crypto device configurations
  *
  * @return Returns 0 on success
  */
@@ -176,7 +176,8 @@ typedef void (*cryptodev_stats_reset_t)(struct 
rte_cryptodev *dev);
 /**
  * Function used to get specific information of a device.
  *
- * @param  dev Crypto device pointer
+ * @param  dev Crypto device pointer
+ * @param  dev_infoPointer to infos structure to populate
  */
 typedef void (*cryptodev_info_get_t)(struct rte_cryptodev *dev,
struct rte_cryptodev_info *dev_info);
@@ -213,7 +214,7 @@ typedef int (*cryptodev_queue_pair_release_t)(struct 
rte_cryptodev *dev,
  *
  * @param  dev Crypto device pointer
  * @param  nb_objs number of sessions objects in mempool
- * @param  obj_cache   l-core object cache size, see *rte_ring_create*
+ * @param  obj_cache_size  l-core object cache size, see *rte_ring_create*
  * @param  socket_id   Socket Id to allocate  mempool on.
  *
  * @return
@@ -253,7 +254,7 @@ typedef unsigned int 
(*cryptodev_asym_get_session_private_size_t)(
  *
  * @param  dev Crypto device pointer
  * @param  xform   Single or chain of crypto xforms
- * @param  priv_sess   Pointer to cryptodev's private session structure
+ * @param  session Pointer to cryptodev's private session structure
  * @param  mp  Mempool where the private session is allocated
  *
  * @return
@@ -271,7 +272,7 @@ typedef int (*cryptodev_sym_configure_session_t)(struct 
rte_cryptodev *dev,
  *
  * @param  dev Crypto device pointer
  * @param  xform   Single or chain of crypto xforms
- * @param  priv_sess   Pointer to cryptodev's private session structure
+ * @param  session Pointer to cryptodev's private session structure
  * @param  mp  Mempool where the private session is allocated
  *
  * @return
@@ -333,7 +334,6 @@ typedef int (*cryptodev_sym_get_raw_dp_ctx_size_t)(struct 
rte_cryptodev *dev);
  *
  * @param  dev Crypto device pointer.
  * @param  qp_id   Crypto device queue pair index.
- * @param  service_typeType of the service requested.
  * @param  ctx The raw data-path context data.
  * @param  sess_type   session type.
  * @param  session_ctx Session context data. If NULL the driver
diff --git a/lib/librte_eal/include/rte_keepalive.h 
b/lib/librte_eal/include/rte_keepalive.h
index 4bda7ca56f..bd25508da8 100644
--- a/lib/librte_eal/include/rte_keepalive.h
+++ b/lib/librte_eal/include/rte_keepalive.h
@@ -52,7 +52,7 @@ typedef void (*rte_keepalive_failure_callback_t)(
  *  @param data Data pointer passed to rte_keepalive_register_relay_callback()
  *  @param id_core ID of the core for which state is being reported
  *  @param core_state The current state of the core
- *  @param Timestamp of when core was last seen alive
+ *  @param last_seen Timestamp of when core was

Re: [dpdk-dev] [PATCH v9 1/2] examples/vhost: add ioat ring space count and check

[Jiang, Cheng1]

Hi,

> -Original Message-
> From: Maxime Coquelin 
> Sent: Thursday, January 21, 2021 8:35 PM
> To: Jiang, Cheng1 ; Xia, Chenbo
> 
> Cc: dev@dpdk.org; Hu, Jiayu ; Yang, YvonneX
> ; Wang, Yinan 
> Subject: Re: [PATCH v9 1/2] examples/vhost: add ioat ring space count and
> check
> 
> 
> 
> On 1/12/21 5:38 AM, Cheng Jiang wrote:
> > Add ioat ring space count and check, if ioat ring space is not enough
> > for the next async vhost packet enqueue, then just return to prevent
> > enqueue failure. Add rte_ioat_completed_ops() fail handler.
> >
> > Signed-off-by: Cheng Jiang 
> > Reviewed-by: Jiayu Hu 
> > Reviewed-by: Maxime Coquelin 
> > ---
> >  examples/vhost/ioat.c | 24 +---
> >  1 file changed, 13 insertions(+), 11 deletions(-)
> >
> > diff --git a/examples/vhost/ioat.c b/examples/vhost/ioat.c index
> > 71d8a1f1f..dbad28d43 100644
> > --- a/examples/vhost/ioat.c
> > +++ b/examples/vhost/ioat.c
> > @@ -17,6 +17,7 @@ struct packet_tracker {
> > unsigned short next_read;
> > unsigned short next_write;
> > unsigned short last_remain;
> > +   unsigned short ioat_space;
> >  };
> >
> >  struct packet_tracker cb_tracker[MAX_VHOST_DEVICE]; @@ -113,7 +114,7
> > @@ open_ioat(const char *value)
> > goto out;
> > }
> > rte_rawdev_start(dev_id);
> > -
> > +   cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE;
> > dma_info->nr++;
> > i++;
> > }
> > @@ -140,13 +141,9 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
> > src = descs[i_desc].src;
> > dst = descs[i_desc].dst;
> > i_seg = 0;
> > +   if (cb_tracker[dev_id].ioat_space < src->nr_segs)
> > +   break;
> > while (i_seg < src->nr_segs) {
> > -   /*
> > -* TODO: Assuming that the ring space of the
> > -* IOAT device is large enough, so there is no
> > -* error here, and the actual error handling
> > -* will be added later.
> > -*/
> > rte_ioat_enqueue_copy(dev_id,
> > (uintptr_t)(src->iov[i_seg].iov_base)
> > + src->offset,
> > @@ -158,7 +155,8 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
> > i_seg++;
> > }
> > write &= mask;
> > -   cb_tracker[dev_id].size_track[write] = i_seg;
> > +   cb_tracker[dev_id].size_track[write] = src->nr_segs;
> > +   cb_tracker[dev_id].ioat_space -= src->nr_segs;
> > write++;
> > }
> > } else {
> > @@ -178,17 +176,21 @@ ioat_check_completed_copies_cb(int vid,
> uint16_t
> > queue_id,  {
> > if (!opaque_data) {
> > uintptr_t dump[255];
> > -   unsigned short n_seg;
> > +   int n_seg;
> > unsigned short read, write;
> > unsigned short nb_packet = 0;
> > unsigned short mask = MAX_ENQUEUED_SIZE - 1;
> > unsigned short i;
> > +
> > int dev_id = dma_bind[vid].dmas[queue_id * 2
> > + VIRTIO_RXQ].dev_id;
> > n_seg = rte_ioat_completed_ops(dev_id, 255, dump, dump);
> > -   n_seg += cb_tracker[dev_id].last_remain;
> > -   if (!n_seg)
> > +   if (n_seg <= 0)
> > return 0;
> 
> In a separate patch, it might make sense to propagate the error if
> rte_ioat_completed_ops return -1.

Sure, I'll send a patch to fix it when this patch set is merged.

Thanks,
Cheng

> 
> Reviewed-by: Maxime Coquelin 
> 
> Maxime
> > +
> > +   cb_tracker[dev_id].ioat_space += n_seg;
> > +   n_seg += cb_tracker[dev_id].last_remain;
> > +
> > read = cb_tracker[dev_id].next_read;
> > write = cb_tracker[dev_id].next_write;
> > for (i = 0; i < max_packets; i++) {
> > --
> > 2.29.2
> >

Re: [dpdk-dev] [dpdk-stable] [PATCH v11 1/4] raw/ifpga: add fpga rsu function

[Huang, Wei]

Hi Ferruh,

Cyborg is an OpenStack project that aims to provide a general purpose 
management framework for acceleration resources (i.e. various types of 
accelerators such as GPU, FPGA, NP, ODP, DPDK/SPDK and so on).

To update the FPGA flash is one of requirements from Cyborg. Originally there 
are no such interfaces, so I added them.

These interfaces use rte_rawdev to identify which FPGA to access, they will be 
called in opae_update_flash(),
opae_cancel_flash_update() and opae_reboot_device() in ifpga_opae_api.c .

These opae_xxx function use PCI address to identify FPGA instead of rte_rawdev, 
so the caller has no need to know the existence of rte_rawdev.
In fact, Cyborg is Python application, these opae_xxx functions will be 
eventually wrapped in a Python module for Cyborg to call.

Thanks,
Wei

-Original Message-
From: Ferruh Yigit  
Sent: Friday, January 22, 2021 00:30
To: Huang, Wei ; dev@dpdk.org; Xu, Rosen 
; Zhang, Qi Z 
Cc: sta...@dpdk.org; Zhang, Tianfei ; Ray Kinsella 

Subject: Re: [dpdk-stable] [PATCH v11 1/4] raw/ifpga: add fpga rsu function

On 1/21/2021 6:03 AM, Wei Huang wrote:
> RSU (Remote System Update) depends on secure manager which may be 
> different on various implementations, so a new secure manager device 
> is implemented for adapting such difference.
> There are three major functions added:
> 1. ifpga_rawdev_update_flash() updates flash with specific image file.
> 2. ifpga_rawdev_stop_flash_update() aborts flash update process.
> 3. ifpga_rawdev_reload() reloads FPGA from updated flash.
> 
> Signed-off-by: Wei Huang 
> Acked-by: Tianfei Zhang 
> Acked-by: Rosen Xu 

<...>

> @@ -76,4 +76,9 @@ int
>   ifpga_unregister_msix_irq(enum ifpga_irq_type type,
>   int vec_start, rte_intr_callback_fn handler, void *arg);
>   
> +int ifpga_rawdev_update_flash(struct rte_rawdev *dev, const char *image,
> + uint64_t *status);
> +int ifpga_rawdev_stop_flash_update(struct rte_rawdev *dev, int 
> +force); int ifpga_rawdev_reload(struct rte_rawdev *dev, int type, int 
> +page);
> +
>   #endif /* _IFPGA_RAWDEV_H_ */
> 

Hi Wei,

Please help me understand the rawdev, who should be calling the above newly 
added functions?

Re: [dpdk-dev] [PATCH v3] net/ice: drain out DCF AdminQ command queue

[Zhang, Qi Z]

> -Original Message-
> From: Wang, Haiyue 
> Sent: Friday, January 22, 2021 1:32 AM
> To: dev@dpdk.org
> Cc: Yang, Qiming ; Zhang, Qi Z
> ; Fu, Qi ; Wang, Haiyue
> ; sta...@dpdk.org
> Subject: [PATCH v3] net/ice: drain out DCF AdminQ command queue
> 
> The virtchnl message is handled one by one by checking opcode to match the
> response for the request.
> 
> The DCF AdminQ command with buffer needs two virtchnl commands, one is to
> handle the AdminQ descriptor, the other is to the handle AdminQ buffer. If 
> both
> of them are sent to PF successfully, it needs to wait two responses from PF,
> even if the AdminQ descriptor command gets the failure response. Since PF will
> handle them one by one, and send back the response for each.
> 
> If not wait for the buffer message response until timeout to drain out the
> virtchnl command queue, it will cause the next AdminQ command with buffer to
> get the stall buffer response from previous.
> 
> Fixes: daa714d55c72 ("net/ice: handle AdminQ command by DCF")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Haiyue Wang 

Acked-by: Qi Zhang 

Applied to dpdk-next-net-intel.

Thanks
Qi

Re: [dpdk-dev] [PATCH] net/ice: fix symmetric rule creating

[Zhang, Qi Z]

> -Original Message-
> From: Ding, Xuan 
> Sent: Thursday, January 21, 2021 3:27 PM
> To: Zhang, Qi Z ; Wu, Jingjing ;
> Xing, Beilei 
> Cc: dev@dpdk.org; Ding, Xuan ; sta...@dpdk.org
> Subject: [PATCH] net/ice: fix symmetric rule creating
> 
> Only allow to create symmetric rule for L3/L4.
> 
> Fixes: 38d632cbdc88("net/ice: refactor PF RSS")
> Cc: sta...@dpdk.org

Fix on current release, no need to CC stable.
> 
> Signed-off-by: Xuan Ding 

Acked-by: Qi Zhang 

Applied to dpdk-next-net-intel.

Thanks
Qi

Re: [dpdk-dev] [PATCH v11 3/4] raw/ifpga: add OPAE API for OpenStack Cyborg

[Huang, Wei]

Hi Ferruh,

These log information will not be output for the default log level is 
OPAE_LOG_ERR, they are output only when log level is set to OPAE_LOG_INFO. I 
can remove them if you think it's necessary.

All these opae_xxx functions will check the input PCI address is valid or not, 
and the rawdev PMD will also filter input PCI device ID. I think it's enough to 
prevent these function to be misused.

Thanks,
Wei

-Original Message-
From: Ferruh Yigit  
Sent: Friday, January 22, 2021 00:30
To: Huang, Wei ; dev@dpdk.org; Xu, Rosen 
; Zhang, Qi Z 
Cc: sta...@dpdk.org; Zhang, Tianfei 
Subject: Re: [dpdk-dev] [PATCH v11 3/4] raw/ifpga: add OPAE API for OpenStack 
Cyborg

On 1/21/2021 6:03 AM, Wei Huang wrote:
> Cyborg is an OpenStack project that aims to provide a general purpose 
> management framework for acceleration resources (i.e. various types of 
> accelerators such as GPU, FPGA, NP, ODP, DPDK/SPDK and so on).
> It needs some OPAE type APIs to manage PACs (Programmable Acceleration
> Card) with Intel FPGA. Below major functions are added to meets Cyborg 
> requirements.
> 1. opae_init() set up OPAE environment.
> 2. opae_cleanup() clean up OPAE environment.
> 3. opae_enumerate() searches PAC with specific FPGA.
> 4. opae_get_property() gets properties of FPGA.
> 5. opae_partial_reconfigure() perform partial configuration on FPGA.
> 6. opae_get_image_info() gets information of image file.
> 7. opae_update_flash() updates FPGA flash with specific image file.
> 8. opae_cancel_flash_update() cancel process of FPGA flash update.
> 9. opae_probe_device() manually probe specific FPGA with ifpga driver.
> 10. opae_remove_device() manually remove specific FPGA from ifpga driver.
> 11. opae_bind_driver() binds specific FPGA with specified kernel driver.
> 12. opae_unbind_driver() unbinds specific FPGA from kernel driver.
> 13. opae_reboot_device() reboots specific FPGA (do reconfiguration).
> 
> Signed-off-by: Wei Huang 
> Acked-by: Tianfei Zhang 
> Acked-by: Rosen Xu 

<...>

> +
> +RTE_INIT(init_api_env)
> +{
> + eal_inited = 0;
> + opae_log_level = OPAE_LOG_ERR;
> + opae_log_file = NULL;
> + ifpga_rawdev_logtype = 0;
> +
> + opae_log_info("API environment is initialized\n"); }
> +
> +RTE_FINI(clean_api_env)
> +{
> + if (opae_log_file) {
> + fclose(opae_log_file);
> + opae_log_file = NULL;
> + }
> + opae_log_info("API environment is cleaned\n"); }
> +

There are called in constructor and destructor, which means all DPDK 
applications have this PMD compiled will run above and print some log, is this 
really what we want here?
Is there a way to limit the initialize to the PMD context, like 'probe()' etc?

Re: [dpdk-dev] [PATCH v1] net/ice/base: add ethertype offset for QinQ dummy pkt

[Zhang, Qi Z]

> -Original Message-
> From: Zhang, Yuying 
> Sent: Tuesday, January 19, 2021 3:13 PM
> To: dev@dpdk.org; Yang, Qiming ; Zhang, Qi Z
> 
> Cc: Wang, Haiyue ; Guo, Junfeng
> ; Zhang, Yuying 
> Subject: [PATCH v1] net/ice/base: add ethertype offset for QinQ dummy pkt
> 
> Add the ethertype offset for QinQ switch rule dummy packet to allow matching
> the corresponding field.
> 
> Signed-off-by: Yuying Zhang 

Acked-by: Qi Zhang 

Applied to dpdk-next-net-intel.

Thanks
Qi

Re: [dpdk-dev] [PATCH v11 3/4] raw/ifpga: add OPAE API for OpenStack Cyborg

[Huang, Wei]

Hi Ferruh,

That's a good question.
The answer is YES, we do need all these functions. We have complete the 
integrated test with Cyborg, there is no redundant function. Let me show you a 
use case in Cyborg.
Cyborg will update FPGA flash and reboot it to make it effective, so they will 
call functions like below.
1. opae_enumerate() to find the target FPGA
2. opae_get_property() to check FPGA version
3. opae_get_image_info() to check the update image
4. opae_update_flash() to update FPGA flash
5. opae_reboot_device() to reboot FPGA
6. After reboot, FPGA kernel driver will not be vfio-pci by default, 
opae_bind_driver() is used to bind vfio-pci kernel driver.
7. opae_probe_device() to attach ifpga PMD to FPGA, then this FPGA can be 
managed by Cyborg again.

These functions will be wrapped in Python package. Cyborg require this Python 
package can be downloaded from PyPI and compiled without DPDK installed.
So there is an independent project which will create a static library file from 
target DPDK. This library is part of Python package and will be used when 
compiling Python module. That's why I didn't export these function in map file.
BTW, the header file ifpga_opae_api.h is also integrated into Python package 
from target DPDK.

Thanks,
Wei

-Original Message-
From: Ferruh Yigit  
Sent: Friday, January 22, 2021 00:30
To: Huang, Wei ; dev@dpdk.org; Xu, Rosen 
; Zhang, Qi Z 
Cc: sta...@dpdk.org; Zhang, Tianfei ; Ray Kinsella 

Subject: Re: [dpdk-dev] [PATCH v11 3/4] raw/ifpga: add OPAE API for OpenStack 
Cyborg

On 1/21/2021 6:03 AM, Wei Huang wrote:
> Cyborg is an OpenStack project that aims to provide a general purpose 
> management framework for acceleration resources (i.e. various types of 
> accelerators such as GPU, FPGA, NP, ODP, DPDK/SPDK and so on).
> It needs some OPAE type APIs to manage PACs (Programmable Acceleration
> Card) with Intel FPGA. Below major functions are added to meets Cyborg 
> requirements.
> 1. opae_init() set up OPAE environment.
> 2. opae_cleanup() clean up OPAE environment.
> 3. opae_enumerate() searches PAC with specific FPGA.
> 4. opae_get_property() gets properties of FPGA.
> 5. opae_partial_reconfigure() perform partial configuration on FPGA.
> 6. opae_get_image_info() gets information of image file.
> 7. opae_update_flash() updates FPGA flash with specific image file.
> 8. opae_cancel_flash_update() cancel process of FPGA flash update.
> 9. opae_probe_device() manually probe specific FPGA with ifpga driver.
> 10. opae_remove_device() manually remove specific FPGA from ifpga driver.
> 11. opae_bind_driver() binds specific FPGA with specified kernel driver.
> 12. opae_unbind_driver() unbinds specific FPGA from kernel driver.
> 13. opae_reboot_device() reboots specific FPGA (do reconfiguration).
> 

Hi Wei,

As far as I understand you are adding above public functions which are on top 
of raw/ifpga driver functions, so they are like PMD specific APIs, I think 
there are a few problems with it:

1) Do we really need/want this much PMD specific API? Can't we have them 
through the rawdev abstraction layer?

2) DPDK public APIs are part of API/ABI policy, so there are a few rules they 
have to follow, like:
- They should start with 'rte_' prefix, and the PMD specific APIs should start 
with 'rte_pmd_' prefix
- They should be in the .map file
- They should be experimental at least one release
- They should be fully documented in a doxygen format
   - Header file should be added to index file for API documentation

Please don't update above before 1) is clearified and we are sure new APIs are 
required.

<...>

> @@ -13,8 +13,10 @@ objs = [base_objs]
>   deps += ['ethdev', 'rawdev', 'pci', 'bus_pci', 'kvargs',
>   'bus_vdev', 'bus_ifpga', 'net', 'net_i40e', 'net_ipn3ke']
>   
> -sources = files('ifpga_rawdev.c')
> +sources = files('ifpga_rawdev.c', 'ifpga_opae_api.c')
>   
>   includes += include_directories('base')
>   includes += include_directories('../../net/ipn3ke')
>   includes += include_directories('../../net/i40e')
> +
> +install_headers('ifpga_opae_api.h')
> 

There is a 'headers' helper that you can use for meson. Also the header file 
name should start with 'rte_pmd_'.

Even before this patch, isn't application has to include the rawdev PMD header? 
Why that header was not installed?

[dpdk-dev] [PATCH] net/iavf: fix symmetric rule creating

[Xuan Ding]

Only allow to create symmetric rule for L3/L4.

Fixes: 91f27b2e39ab("net/iavf: refactor RSS")
Cc: sta...@dpdk.org

Signed-off-by: Xuan Ding 
---
 drivers/net/iavf/iavf_hash.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/iavf/iavf_hash.c b/drivers/net/iavf/iavf_hash.c
index 958c73c4fb..5fa2f3cf59 100644
--- a/drivers/net/iavf/iavf_hash.c
+++ b/drivers/net/iavf/iavf_hash.c
@@ -927,6 +927,13 @@ iavf_any_invalid_rss_type(enum rte_eth_hash_function 
rss_func,
if (rss_type & (ETH_RSS_L3_SRC_ONLY | ETH_RSS_L3_DST_ONLY |
ETH_RSS_L4_SRC_ONLY | ETH_RSS_L4_DST_ONLY))
return true;
+
+   if (!(rss_type &
+  (ETH_RSS_IPV4 | ETH_RSS_IPV6 |
+   ETH_RSS_NONFRAG_IPV4_UDP | ETH_RSS_NONFRAG_IPV6_UDP |
+   ETH_RSS_NONFRAG_IPV4_TCP | ETH_RSS_NONFRAG_IPV6_TCP |
+   ETH_RSS_NONFRAG_IPV4_SCTP | ETH_RSS_NONFRAG_IPV6_SCTP)))
+   return true;
}
 
/* check invalid combination */
-- 
2.17.1

Re: [dpdk-dev] [PATCH v11 3/4] raw/ifpga: add OPAE API for OpenStack Cyborg

[Huang, Wei]

Hi Ferruh,

Most of other APIs depends on EAL environment, they will make sure EAL is ready 
be executing their task.
As replied in previous mail,  these functions will be called in Python 
environment. Python application should initialize EAL explicitly before calling 
other APIs like opae_update_flash. If application forget to do such 
initialization, an error information will be output when calling other APIs.

Thanks,
Wei

-Original Message-
From: Ferruh Yigit  
Sent: Friday, January 22, 2021 00:34
To: Huang, Wei ; dev@dpdk.org; Xu, Rosen 
; Zhang, Qi Z 
Cc: sta...@dpdk.org; Zhang, Tianfei 
Subject: Re: [dpdk-dev] [PATCH v11 3/4] raw/ifpga: add OPAE API for OpenStack 
Cyborg

On 1/21/2021 6:03 AM, Wei Huang wrote:
> +int opae_init(int eal_init_result)
> +{
> + int ret = 0;
> +
> + if (!check_eal(0))
> + return 0;
> +
> + if (eal_init_result < 0) {
> + if (rte_errno == EALREADY) {
> + eal_inited = 1;
> + opae_log_info("EAL already initialized\n");
> + } else {
> + opae_log_err("Cannot initialize EAL\n");
> + ret = -1;
> + }
> + } else {
> + eal_inited = 1;
> + opae_log_info("Initialize EAL done\n");
> + }
> +
> + return ret;
> +}

Why an PMD API needs to know eal initialization status? This may be pointing a 
bad design, is it possible that most of the 'opae_' API code should go to the 
application?

Re: [dpdk-dev] [PATCH v2 26/44] net/virtio: add Virtio-user ops to set owner

[Xia, Chenbo]

> -Original Message-
> From: Maxime Coquelin 
> Sent: Wednesday, January 20, 2021 5:25 AM
> To: dev@dpdk.org; Xia, Chenbo ; olivier.m...@6wind.com;
> amore...@redhat.com; david.march...@redhat.com
> Cc: Maxime Coquelin 
> Subject: [PATCH v2 26/44] net/virtio: add Virtio-user ops to set owner
> 
> This patch implements a dedicated callback for
> sending owner request. All the requests will be
> converted that way so that backends other than
> Vhost-user don't have to work around being it.
> 
> Signed-off-by: Maxime Coquelin 
> ---
>  drivers/net/virtio/virtio_user/vhost.h|  1 +
>  drivers/net/virtio/virtio_user/vhost_kernel.c | 28 +++--
>  drivers/net/virtio/virtio_user/vhost_user.c   | 21 +++--
>  drivers/net/virtio/virtio_user/vhost_vdpa.c   | 30 ---
>  .../net/virtio/virtio_user/virtio_user_dev.c  |  3 +-
>  5 files changed, 72 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_user/vhost.h
> b/drivers/net/virtio/virtio_user/vhost.h
> index 8ec3a6a62c..5413ec6778 100644
> --- a/drivers/net/virtio/virtio_user/vhost.h
> +++ b/drivers/net/virtio/virtio_user/vhost.h
> @@ -109,6 +109,7 @@ struct virtio_user_dev;
> 
>  struct virtio_user_backend_ops {
>   int (*setup)(struct virtio_user_dev *dev);
> + int (*set_owner)(struct virtio_user_dev *dev);
>   int (*send_request)(struct virtio_user_dev *dev,
>   enum vhost_user_request req,
>   void *arg);
> diff --git a/drivers/net/virtio/virtio_user/vhost_kernel.c
> b/drivers/net/virtio/virtio_user/vhost_kernel.c
> index 2c805077af..b79dcad179 100644
> --- a/drivers/net/virtio/virtio_user/vhost_kernel.c
> +++ b/drivers/net/virtio/virtio_user/vhost_kernel.c
> @@ -6,6 +6,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  #include 
> 
> @@ -55,8 +56,28 @@ get_vhost_kernel_max_regions(void)
>   close(fd);
>  }
> 
> +static int
> +vhost_kernel_ioctl(int fd, uint64_t request, void *arg)
> +{
> + int ret;
> +
> + ret = ioctl(fd, request, arg);
> + if (ret) {
> + PMD_DRV_LOG(ERR, "Vhost-kernel ioctl %"PRIu64" failed (%s)",
> + request, strerror(errno));
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static int
> +vhost_kernel_set_owner(struct virtio_user_dev *dev)
> +{
> + return vhost_kernel_ioctl(dev->vhostfds[0], VHOST_SET_OWNER, NULL);
> +}
> +
>  static uint64_t vhost_req_user_to_kernel[] = {
> - [VHOST_USER_SET_OWNER] = VHOST_SET_OWNER,
>   [VHOST_USER_RESET_OWNER] = VHOST_RESET_OWNER,
>   [VHOST_USER_SET_FEATURES] = VHOST_SET_FEATURES,
>   [VHOST_USER_GET_FEATURES] = VHOST_GET_FEATURES,
> @@ -175,7 +196,7 @@ tap_support_features(void)
>  }
> 
>  static int
> -vhost_kernel_ioctl(struct virtio_user_dev *dev,
> +vhost_kernel_send_request(struct virtio_user_dev *dev,
>  enum vhost_user_request req,
>  void *arg)
>  {
> @@ -385,6 +406,7 @@ vhost_kernel_enable_queue_pair(struct virtio_user_dev 
> *dev,
> 
>  struct virtio_user_backend_ops virtio_ops_kernel = {
>   .setup = vhost_kernel_setup,
> - .send_request = vhost_kernel_ioctl,
> + .set_owner = vhost_kernel_set_owner,
> + .send_request = vhost_kernel_send_request,
>   .enable_qp = vhost_kernel_enable_queue_pair
>  };
> diff --git a/drivers/net/virtio/virtio_user/vhost_user.c
> b/drivers/net/virtio/virtio_user/vhost_user.c
> index 55c81a..ea3bd4ca10 100644
> --- a/drivers/net/virtio/virtio_user/vhost_user.c
> +++ b/drivers/net/virtio/virtio_user/vhost_user.c
> @@ -125,6 +125,24 @@ vhost_user_read(int fd, struct vhost_user_msg *msg)
>   return -1;
>  }
> 
> +static int
> +vhost_user_set_owner(struct virtio_user_dev *dev)
> +{
> + int ret;
> + struct vhost_user_msg msg = {
> + .request = VHOST_USER_SET_OWNER,
> + .flags = VHOST_USER_VERSION,
> + };
> +
> + ret = vhost_user_write(dev->vhostfd, &msg, NULL, 0);
> + if (ret < 0) {
> + PMD_DRV_LOG(ERR, "Failed to set owner");
> + return -1;
> + }
> +
> + return 0;
> +}
> +
>  struct walk_arg {
>   struct vhost_memory *vm;
>   int *fds;
> @@ -230,7 +248,6 @@ prepare_vhost_memory_user(struct vhost_user_msg *msg, int
> fds[])
>  static struct vhost_user_msg m;
> 
>  const char * const vhost_msg_strings[] = {
> - [VHOST_USER_SET_OWNER] = "VHOST_SET_OWNER",
>   [VHOST_USER_RESET_OWNER] = "VHOST_RESET_OWNER",
>   [VHOST_USER_SET_FEATURES] = "VHOST_SET_FEATURES",
>   [VHOST_USER_GET_FEATURES] = "VHOST_GET_FEATURES",
> @@ -308,7 +325,6 @@ vhost_user_sock(struct virtio_user_dev *dev,
>   msg.size = sizeof(m.payload.u64);
>   break;
> 
> - case VHOST_USER_SET_OWNER:
>   case VHOST_USER_RESET_OWNER:
>   break;
> 
> @@ -519,6 +535,7 @@ vhost_user_enable_queue_pair(struct virtio_user_dev *dev,
> 
>  struct virtio_user_backend_ops virtio_ops_user =

Re: [dpdk-dev] [dpdk-stable] [PATCH v11 4/4] examples/ifpga: add example for opae ifpga API

[Huang, Wei]

Hi Ferruh,

Yes, all the command need PCI BDF as identifier of FPGA, because Cyborg use PCI 
BDF to call these APIs, Cyborg does not see any DPDK data structures. That's 
why I add a group of middle layer functions in ifpga_opae_api.c to isolate 
Cyborg from DPDK.

Thanks,
Wei

-Original Message-
From: Ferruh Yigit  
Sent: Friday, January 22, 2021 00:49
To: Huang, Wei ; dev@dpdk.org; Xu, Rosen 
; Zhang, Qi Z 
Cc: sta...@dpdk.org; Zhang, Tianfei 
Subject: Re: [dpdk-stable] [PATCH v11 4/4] examples/ifpga: add example for opae 
ifpga API

On 1/21/2021 6:03 AM, Wei Huang wrote:
> +get_property command
> +
> +
> +Display property information of specified FPGA. Property type is defined as 
> below.
> +0 - All properties
> +1 - PCI property
> +2 - FME property
> +4 - port property
> +8 - BMC property
> +PCI property is always available, other properties can only be 
> +displayed after ifpga driver is probed to the FPGA.
> +
> +.. code-block:: console
> +
> +   opae> get_property 24:00.0 0
> +   PCI:
> +PCIe s:b:d.f : :24:00.0
> +kernel driver: vfio-pci
> +   FME:
> +platform : Vista Creek
> +DCP version  : DCP 1.2
> +phase: Beta
> +interface: 2x2x25G
> +build version: 0.0.2
> +ports num: 1
> +boot page: user
> +pr interface id  : a5d72a3c-c8b0-4939-912c-f715e5dc10ca
> +   PORT0:
> +access type  : PF
> +accelerator id   : 8892c23e-2eed-4b44-8bb6-5c88606e07df
> +   BMC:
> +MAX10 version: D.2.0.5
> +NIOS FW version  : D.2.0.12

Are all commands interacting device need to provide the PCI BDF as parameter to 
select the device? Like '24:00.0' in above sample.

Why not get the unique rawdev identifier, like port_id equivalent of the 
ethdev, and use it in the command line, won't it be easier?

Re: [dpdk-dev] [PATCH] net/iavf: fix symmetric rule creating

[Xing, Beilei]

> -Original Message-
> From: Ding, Xuan 
> Sent: Friday, January 22, 2021 11:19 AM
> To: Zhang, Qi Z ; Wu, Jingjing ;
> Xing, Beilei 
> Cc: dev@dpdk.org; Ding, Xuan ; sta...@dpdk.org
> Subject: [PATCH] net/iavf: fix symmetric rule creating
> 
> Only allow to create symmetric rule for L3/L4.
> 
> Fixes: 91f27b2e39ab("net/iavf: refactor RSS")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Xuan Ding 
Acked-by: Beilei Xing 

[dpdk-dev] [PATCH v1 0/2] fix bugs of app eventdev

[Feifei Wang]

Fix bugs of app eventdev

Feifei Wang (2):
  app/eventdev: adjust event count order for pipeline test
  app/eventdev: remove redundant enqueue in burst Tx

 app/test-eventdev/test_pipeline_queue.c | 17 -
 1 file changed, 8 insertions(+), 9 deletions(-)

-- 
2.25.1

[dpdk-dev] [PATCH v1 1/2] app/eventdev: adjust event count order for pipeline test

[Feifei Wang]

For the fwd mode (internal_port = false) in pipeline test,
processed-pkts increment should after enqueue. However, in
multi_stage_fwd and multi_stage_burst_fwd, "w->processed_pkts" is
increased before enqueue.

To fix this, move "w->processed_pkts" increment after enqueue, and then
the main core can load the correct number of processed packets.

Fixes: 314bcf58ca8f ("app/eventdev: add pipeline queue worker functions")
Cc: pbhagavat...@marvell.com
Cc: sta...@dpdk.org

Signed-off-by: Feifei Wang 
Reviewed-by: Ruifeng Wang 
---
 app/test-eventdev/test_pipeline_queue.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/app/test-eventdev/test_pipeline_queue.c 
b/app/test-eventdev/test_pipeline_queue.c
index 7bebac34f..01f33e3b4 100644
--- a/app/test-eventdev/test_pipeline_queue.c
+++ b/app/test-eventdev/test_pipeline_queue.c
@@ -180,13 +180,13 @@ pipeline_queue_worker_multi_stage_fwd(void *arg)
ev.queue_id = tx_queue[ev.mbuf->port];
rte_event_eth_tx_adapter_txq_set(ev.mbuf, 0);
pipeline_fwd_event(&ev, RTE_SCHED_TYPE_ATOMIC);
+   pipeline_event_enqueue(dev, port, &ev);
w->processed_pkts++;
} else {
ev.queue_id++;
pipeline_fwd_event(&ev, sched_type_list[cq_id]);
+   pipeline_event_enqueue(dev, port, &ev);
}
-
-   pipeline_event_enqueue(dev, port, &ev);
}
 
return 0;
@@ -237,6 +237,7 @@ pipeline_queue_worker_multi_stage_burst_fwd(void *arg)
const uint8_t *tx_queue = t->tx_evqueue_id;
 
while (t->done == false) {
+   uint16_t processed_pkts = 0;
uint16_t nb_rx = rte_event_dequeue_burst(dev, port, ev,
BURST_SIZE, 0);
 
@@ -254,7 +255,7 @@ pipeline_queue_worker_multi_stage_burst_fwd(void *arg)
rte_event_eth_tx_adapter_txq_set(ev[i].mbuf, 0);
pipeline_fwd_event(&ev[i],
RTE_SCHED_TYPE_ATOMIC);
-   w->processed_pkts++;
+   processed_pkts++;
} else {
ev[i].queue_id++;
pipeline_fwd_event(&ev[i],
@@ -263,6 +264,7 @@ pipeline_queue_worker_multi_stage_burst_fwd(void *arg)
}
 
pipeline_event_enqueue_burst(dev, port, ev, nb_rx);
+   w->processed_pkts += processed_pkts;
}
 
return 0;
-- 
2.25.1

[dpdk-dev] [PATCH v1 2/2] app/eventdev: remove redundant enqueue in burst Tx

[Feifei Wang]

For eventdev pipeline test, in burst_tx cases, there is no needed to
set ev.op as RTE_EVENT_OP_RELEASE and call pipeline_event_enqueue_burst
to release events. This is because for tx mode(internal_port=true),
the capability "implicit_release" of dev is enabled, and the app can
release events by "rte_event_dequeue_burst" rather than enqueue.

Fixes: 314bcf58ca8f ("app/eventdev: add pipeline queue worker functions")
Cc: pbhagavat...@marvell.com
Cc: sta...@dpdk.org

Signed-off-by: Feifei Wang 
Reviewed-by: Ruifeng Wang 
---
 app/test-eventdev/test_pipeline_queue.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/app/test-eventdev/test_pipeline_queue.c 
b/app/test-eventdev/test_pipeline_queue.c
index 01f33e3b4..9a9febb19 100644
--- a/app/test-eventdev/test_pipeline_queue.c
+++ b/app/test-eventdev/test_pipeline_queue.c
@@ -83,16 +83,15 @@ pipeline_queue_worker_single_stage_burst_tx(void *arg)
rte_prefetch0(ev[i + 1].mbuf);
if (ev[i].sched_type == RTE_SCHED_TYPE_ATOMIC) {
pipeline_event_tx(dev, port, &ev[i]);
-   ev[i].op = RTE_EVENT_OP_RELEASE;
w->processed_pkts++;
} else {
ev[i].queue_id++;
pipeline_fwd_event(&ev[i],
RTE_SCHED_TYPE_ATOMIC);
+   pipeline_event_enqueue_burst(dev, port, ev,
+   nb_rx);
}
}
-
-   pipeline_event_enqueue_burst(dev, port, ev, nb_rx);
}
 
return 0;
@@ -213,7 +212,6 @@ pipeline_queue_worker_multi_stage_burst_tx(void *arg)
 
if (ev[i].queue_id == tx_queue[ev[i].mbuf->port]) {
pipeline_event_tx(dev, port, &ev[i]);
-   ev[i].op = RTE_EVENT_OP_RELEASE;
w->processed_pkts++;
continue;
}
@@ -222,9 +220,8 @@ pipeline_queue_worker_multi_stage_burst_tx(void *arg)
pipeline_fwd_event(&ev[i], cq_id != last_queue ?
sched_type_list[cq_id] :
RTE_SCHED_TYPE_ATOMIC);
+   pipeline_event_enqueue_burst(dev, port, ev, nb_rx);
}
-
-   pipeline_event_enqueue_burst(dev, port, ev, nb_rx);
}
 
return 0;
-- 
2.25.1

Re: [dpdk-dev] [PATCH] net/ixgbe: disable NFS filtering

[Guo, Jia]

Hi, dapeng

> -Original Message-
> From: dapengx...@intel.com 
> Sent: Wednesday, January 20, 2021 5:27 PM
> To: Guo, Jia ; Wang, Haiyue ;
> Yang, Qiming 
> Cc: dev@dpdk.org; Yu, DapengX ; sta...@dpdk.org
> Subject: [PATCH] net/ixgbe: disable NFS filtering
> 
> From: Dapeng Yu 
> 
> Disable NFS header filtering whether NFS packets coalescing are required or
> not.
> 
> This behavior is aligned with ixgbe kernel driver.
> 
> Fixes: b826efba6de4 ("net/ixgbe: align register setting when RSC is disabled")

I suggest to add one more fix line which related with the patch, since below 
patch is what you most change the behaviors.
the Fixes: 8eecb3295aed ("ixgbe: add LRO support ")

And I saw prior fixed patch are also said it would flow the datasheet, could 
you explain why you bring this change? Is it anything change about that in 
kernel driver prior with currently?

> Cc: sta...@dpdk.org
> 
> Signed-off-by: Dapeng Yu 
> ---
>  drivers/net/ixgbe/ixgbe_rxtx.c | 10 +++---
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
> index cc8f70e6d..3fb55c675 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> @@ -4923,15 +4923,11 @@ ixgbe_set_rsc(struct rte_eth_dev *dev)
>   /* RFCTL configuration  */
>   rfctl = IXGBE_READ_REG(hw, IXGBE_RFCTL);
>   if ((rsc_capable) && (rx_conf->offloads &
> DEV_RX_OFFLOAD_TCP_LRO))
> - /*
> -  * Since NFS packets coalescing is not supported - clear
> -  * RFCTL.NFSW_DIS and RFCTL.NFSR_DIS when RSC is
> -  * enabled.
> -  */
> - rfctl &= ~(IXGBE_RFCTL_RSC_DIS | IXGBE_RFCTL_NFSW_DIS
> |
> -IXGBE_RFCTL_NFSR_DIS);
> + rfctl &= ~IXGBE_RFCTL_RSC_DIS;
>   else
>   rfctl |= IXGBE_RFCTL_RSC_DIS;
> + /* disable NFS filtering */
> + rfctl |= (IXGBE_RFCTL_NFSW_DIS | IXGBE_RFCTL_NFSR_DIS);

I thinks the "()" is no need, right?

>   IXGBE_WRITE_REG(hw, IXGBE_RFCTL, rfctl);
> 
>   /* If LRO hasn't been requested - we are done here. */
> --
> 2.27.0

[dpdk-dev] [PATCH] net/iavf: fix port VLAN cfg err for AVF with SVM

[Junfeng Guo]

For AVF with single vlan mode (SVM), port vlan stripping config
has already been disabled by PF. In this scenario, the error of
-ENOTSUP can be ignored.

Fixes: 1c301e8c3cff ("net/iavf: support new VLAN capabilities")
Signed-off-by: Junfeng Guo 
---
 drivers/net/iavf/iavf_ethdev.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index cf6ea0b15..8175c7729 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -1092,6 +1092,9 @@ iavf_dev_vlan_offload_set_v2(struct rte_eth_dev *dev, int 
mask)
enable = !!(rxmode->offloads & DEV_RX_OFFLOAD_VLAN_STRIP);
 
err = iavf_config_vlan_strip_v2(adapter, enable);
+   /* If not support, the stripping is already disabled by PF */
+   if (err == -ENOTSUP && !enable)
+   err = 0;
if (err)
return -EIO;
}
-- 
2.25.1