[dpdk-dev] [PATCH] net/virtio: improve logs in Vhost-vDPA DMA mapping

[Maxime Coquelin]

This patch adds debug logs in vhost_vdpa_dma_map() and
vhost_vdpa_dma_unmap() to ease debugging.

Signed-off-by: Maxime Coquelin 
---
 drivers/net/virtio/virtio_user/vhost_vdpa.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/virtio/virtio_user/vhost_vdpa.c 
b/drivers/net/virtio/virtio_user/vhost_vdpa.c
index c7b9349fc8..528ff60f29 100644
--- a/drivers/net/virtio/virtio_user/vhost_vdpa.c
+++ b/drivers/net/virtio/virtio_user/vhost_vdpa.c
@@ -93,6 +93,9 @@ vhost_vdpa_dma_map(struct virtio_user_dev *dev, void *addr,
msg.iotlb.size = len;
msg.iotlb.perm = VHOST_ACCESS_RW;
 
+   PMD_DRV_LOG(DEBUG, "%s: iova: 0x%" PRIx64 ", addr: %p, len: 0x%" PRIx64,
+   __func__, iova, addr, len);
+
if (write(dev->vhostfd, &msg, sizeof(msg)) != sizeof(msg)) {
PMD_DRV_LOG(ERR, "Failed to send IOTLB update (%s)",
strerror(errno));
@@ -113,6 +116,9 @@ vhost_vdpa_dma_unmap(struct virtio_user_dev *dev, 
__rte_unused void *addr,
msg.iotlb.iova = iova;
msg.iotlb.size = len;
 
+   PMD_DRV_LOG(DEBUG, "%s: iova: 0x%" PRIx64 ", len: 0x%" PRIx64,
+   __func__, iova, len);
+
if (write(dev->vhostfd, &msg, sizeof(msg)) != sizeof(msg)) {
PMD_DRV_LOG(ERR, "Failed to send IOTLB invalidate (%s)",
strerror(errno));
-- 
2.26.2

Re: [dpdk-dev] [PATCH v1 3/4] net/ixgbe: implement good checksum flag for NEON vector

[Wang, Haiyue]

> -Original Message-
> From: Feifei Wang 
> Sent: Wednesday, November 18, 2020 15:56
> To: Wang, Haiyue ; jer...@marvell.com; Ruifeng Wang 
> ; Guo,
> Jia 
> Cc: dev@dpdk.org; nd ; nd 
> Subject: 回复: [PATCH v1 3/4] net/ixgbe: implement good checksum flag for NEON 
> vector
> 
> Hi, Haiyue
> 
> > -邮件原件-
> > 发件人: Wang, Haiyue 
> > 发送时间: 2020年11月18日 15:22
> > 收件人: Feifei Wang ; jer...@marvell.com; Ruifeng
> > Wang ; Guo, Jia 
> > 抄送: dev@dpdk.org; nd 
> > 主题: RE: [PATCH v1 3/4] net/ixgbe: implement good checksum flag for NEON
> > vector
> >
> > > -Original Message-
> > > From: Feifei Wang 
> > > Sent: Wednesday, November 18, 2020 14:29
> > > To: Jerin Jacob ; Ruifeng Wang
> > > ; Guo, Jia ; Wang, Haiyue
> > > 
> > > Cc: dev@dpdk.org; n...@arm.com; feiwan02 
> > > Subject: [PATCH v1 3/4] net/ixgbe: implement good checksum flag for
> > > NEON vector
> > >
> > > From: feiwan02 
> > >
> > > Add CKSUM_GOOD flag to distinguish a good checksum from an unknown
> > one
> > > in neon vertor RX function.
> > >
> > > Test Results:
> > > NICs: 82599(igb)
> > > Dirver: ixgbe(vector)
> > > Packet: IPv4_checksum = correct value && UDP_checksum = correct value
> > >
> > > $:./app/dpdk-testpmd -c 0x3 -w 0002:f9:00.0 -- -i
> > > --port-topology=chained
> > > test-pmd> set fwd rxonly
> > > test-pmd> set verbose 1
> > > test-pmd> start
> > >
> > > With this patch:
> > > testpmd> port 0/queue 0: received 1 packets
> > > src=00:00:00:00:00:02 - dst=00:00:00:00:00:01 - type=0x0800 -
> > > length=70 - nb_segs=1
> > > ol_flags: PKT_RX_L4_CKSUM_GOOD PKT_RX_IP_CKSUM_GOOD
> > >
> > > Without this patch:
> > > testpmd> port 0/queue 0: received 1 packets
> > > src=00:00:00:00:00:02 - dst=00:00:00:00:00:01 - type=0x0800 -
> > > length=70 - nb_segs=1
> > > ol_flags: PKT_RX_L4_CKSUM_UNKNOWN PKT_RX_IP_CKSUM_UNKNOWN
> > >
> >
> > How about to merge "1 & 2" and "3 & 4"? Then you will have the above test
> > result. ;-)
> Do you mean that combine patch 1 and patch 2 into one patch to support vlan 
> stripping and bad checksum
> flag,
> Then also do the same work for patch 3 and patch 4 to support good checksum 
> flag and enable IXGBE neon
> vector when DEV_RX_OFFLOAD_CHECKSUM is set?
> 
> Actually, the test results are achieved without patch 4, due to that testpmd 
> does not check rxmode-
> >offloads, it will call IXGBE NEON vector function directly. And this change
> is for the default config of l3fwd example.
> 

I was messed by the vector code, sorry. Patch b is for bad checksum,
update the commit message to align your design. Then the patch set
is OF for me, thanks!

Feifei Wang (4):
  a). net/ixgbe: add VLAN stripping support for Arm
  b). net/ixgbe: support checksum flags for NEON vector
  c). net/ixgbe: implement good checksum flag for NEON vector
  d). net/ixgbe: enable IXGBE NEON vector PMD when CHECKSUM is set

> Best Regards
> Feifei
> >
> > > Signed-off-by: Feifei Wang 
> > > Reviewed-by: Ruifeng Wang 
> > > ---
> > > 2.17.1

[dpdk-dev] 回复: [PATCH v1 3/4] net/ixgbe: implement good checksum flag for NEON vector

[Feifei Wang]

> -邮件原件-
> 发件人: Wang, Haiyue 
> 发送时间: 2020年11月18日 16:15
> 收件人: Feifei Wang ; jer...@marvell.com; Ruifeng
> Wang ; Guo, Jia 
> 抄送: dev@dpdk.org; nd ; nd 
> 主题: RE: [PATCH v1 3/4] net/ixgbe: implement good checksum flag for NEON
> vector
> 
> > -Original Message-
> > From: Feifei Wang 
> > Sent: Wednesday, November 18, 2020 15:56
> > To: Wang, Haiyue ; jer...@marvell.com; Ruifeng
> > Wang ; Guo, Jia 
> > Cc: dev@dpdk.org; nd ; nd 
> > Subject: 回复: [PATCH v1 3/4] net/ixgbe: implement good checksum flag
> > for NEON vector
> >
> > Hi, Haiyue
> >
> > > -邮件原件-
> > > 发件人: Wang, Haiyue 
> > > 发送时间: 2020年11月18日 15:22
> > > 收件人: Feifei Wang ; jer...@marvell.com;
> Ruifeng
> > > Wang ; Guo, Jia 
> > > 抄送: dev@dpdk.org; nd 
> > > 主题: RE: [PATCH v1 3/4] net/ixgbe: implement good checksum flag for
> > > NEON vector
> > >
> > > > -Original Message-
> > > > From: Feifei Wang 
> > > > Sent: Wednesday, November 18, 2020 14:29
> > > > To: Jerin Jacob ; Ruifeng Wang
> > > > ; Guo, Jia ; Wang, Haiyue
> > > > 
> > > > Cc: dev@dpdk.org; n...@arm.com; feiwan02 
> > > > Subject: [PATCH v1 3/4] net/ixgbe: implement good checksum flag
> > > > for NEON vector
> > > >
> > > > From: feiwan02 
> > > >
> > > > Add CKSUM_GOOD flag to distinguish a good checksum from an unknown
> > > one
> > > > in neon vertor RX function.
> > > >
> > > > Test Results:
> > > > NICs: 82599(igb)
> > > > Dirver: ixgbe(vector)
> > > > Packet: IPv4_checksum = correct value && UDP_checksum = correct
> > > > value
> > > >
> > > > $:./app/dpdk-testpmd -c 0x3 -w 0002:f9:00.0 -- -i
> > > > --port-topology=chained
> > > > test-pmd> set fwd rxonly
> > > > test-pmd> set verbose 1
> > > > test-pmd> start
> > > >
> > > > With this patch:
> > > > testpmd> port 0/queue 0: received 1 packets
> > > > src=00:00:00:00:00:02 - dst=00:00:00:00:00:01 - type=0x0800 -
> > > > length=70 - nb_segs=1
> > > > ol_flags: PKT_RX_L4_CKSUM_GOOD PKT_RX_IP_CKSUM_GOOD
> > > >
> > > > Without this patch:
> > > > testpmd> port 0/queue 0: received 1 packets
> > > > src=00:00:00:00:00:02 - dst=00:00:00:00:00:01 - type=0x0800 -
> > > > length=70 - nb_segs=1
> > > > ol_flags: PKT_RX_L4_CKSUM_UNKNOWN PKT_RX_IP_CKSUM_UNKNOWN
> > > >
> > >
> > > How about to merge "1 & 2" and "3 & 4"? Then you will have the above
> > > test result. ;-)
> > Do you mean that combine patch 1 and patch 2 into one patch to support
> > vlan stripping and bad checksum flag, Then also do the same work for
> > patch 3 and patch 4 to support good checksum flag and enable IXGBE
> > neon vector when DEV_RX_OFFLOAD_CHECKSUM is set?
> >
> > Actually, the test results are achieved without patch 4, due to that
> > testpmd does not check rxmode-
> > >offloads, it will call IXGBE NEON vector function directly. And this
> > >change
> > is for the default config of l3fwd example.
> >
> 
> I was messed by the vector code, sorry. Patch b is for bad checksum, update 
> the
> commit message to align your design. Then the patch set is OF for me, thanks!
> 
> Feifei Wang (4):
>   a). net/ixgbe: add VLAN stripping support for Arm
>   b). net/ixgbe: support checksum flags for NEON vector
>   c). net/ixgbe: implement good checksum flag for NEON vector
>   d). net/ixgbe: enable IXGBE NEON vector PMD when CHECKSUM is set
> 
That's all right. I will update the commit message and then upload the new 
version.
Thanks for your attention.
> > Best Regards
> > Feifei
> > >
> > > > Signed-off-by: Feifei Wang 
> > > > Reviewed-by: Ruifeng Wang 
> > > > ---
> > > > 2.17.1

Re: [dpdk-dev] [PATCH v1 1/4] net/ixgbe: add VLAN stripping support for Arm

[Wang, Haiyue]

Hi Feifei,

> -Original Message-
> From: Feifei Wang 
> Sent: Wednesday, November 18, 2020 15:35
> To: Jerin Jacob ; Ruifeng Wang ; 
> Guo, Jia
> ; Wang, Haiyue 
> Cc: dev@dpdk.org; n...@arm.com; Feifei Wang 
> Subject: [PATCH v1 1/4] net/ixgbe: add VLAN stripping support for Arm
> 

And the commit message can be more descriptive like:
http://git.dpdk.org/dpdk/commit/?id=6d28c53bf7a9063caa3197c6cc481e2a69e3be96

No need to add the test result, you can put the result into cover letter.

BR,
Haiyue

> Implemented PKT_RX_VLAN_STRIPPED for vector PMD on Arm platform.
> 
> Test Results:
> NICs: 82599(igb)
> Driver: ixgbe(vector)
> Packet: vlan-id=1
> 
> $:./app/dpdk-testpmd -c 0x3 -w 0002:f9:00.0 -- -i --port-topology=chained \
> to enable vlan stripping: --enable-hw-vlan-strip
> test-pmd> set fwd rxonly
> test-pmd> set verbose 1
> test-pmd> start
> 
> With this patch:
> %enable vlan stripping:
> testpmd> port 0/queue 0: received 1 packets
> src=00:00:00:00:00:02 - dst=00:00:00:00:00:01 - type=0x0800 - length=70 - 
> nb_segs=1 - VLAN tci=0x1
> ol_flags: PKT_RX_VLAN PKT_RX_VLAN_STRIPPED
> 
> %disable vlan stripping:
> testpmd> port 0/queue 0: received 1 packets
> src=00:00:00:00:00:02 - dst=00:00:00:00:00:01 - type=0x8100 - length=74 - 
> nb_segs=1 - VLAN tci=0x0
> ol_flags: PKT_RX_VLAN
> 
> Without this patch:
> %enable vlan stripping:
> testpmd> port 0/queue 0: received 1 packets
> src=00:00:00:00:00:02 - dst=00:00:00:00:00:01 - type=0x0800 - length=70 - 
> nb_segs=1 - VLAN tci=0x1
> ol_flags: PKT_RX_VLAN
> 
> %disable vlan stripping:
> testpmd> port 0/queue 0: received 1 packets
> src=00:00:00:00:00:02 - dst=00:00:00:00:00:01 - type=0x8100 - length=74 - 
> nb_segs=1 - VLAN tci=0x0
> ol_flags: PKT_RX_VLAN
> 


> Signed-off-by: Feifei Wang 
> Reviewed-by: Ruifeng Wang 
> ---
>  drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c | 36 -
>  1 file changed, 23 insertions(+), 13 deletions(-)
> 


> 2.17.1

[dpdk-dev] [PATCH] event/octeontx2: unlink queues during port release

[Shijith Thotton]

Unlinking queues from port should be done during port release. Doing it
during device re-configuration could result in segfault as ports array
is re-allocated based on new number of ports.

Fixes: f7ac8b66b23c ("event/octeontx2: support linking queues to ports")
Cc: sta...@dpdk.org

Signed-off-by: Shijith Thotton 
Signed-off-by: Pavan Nikhilesh 
---
 drivers/event/octeontx2/otx2_evdev.c | 98 +---
 drivers/event/octeontx2/otx2_evdev.h | 12 
 2 files changed, 71 insertions(+), 39 deletions(-)

diff --git a/drivers/event/octeontx2/otx2_evdev.c 
b/drivers/event/octeontx2/otx2_evdev.c
index b31c26e95..ed29b8325 100644
--- a/drivers/event/octeontx2/otx2_evdev.c
+++ b/drivers/event/octeontx2/otx2_evdev.c
@@ -689,7 +689,36 @@ sso_lf_cfg(struct otx2_sso_evdev *dev, struct otx2_mbox 
*mbox,
 static void
 otx2_sso_port_release(void *port)
 {
-   rte_free(port);
+   struct otx2_ssogws_cookie *gws_cookie = ssogws_get_cookie(port);
+   struct otx2_sso_evdev *dev;
+   int i;
+
+   if (!gws_cookie->configured)
+   goto free;
+
+   dev = sso_pmd_priv(gws_cookie->event_dev);
+   if (dev->dual_ws) {
+   struct otx2_ssogws_dual *ws = port;
+
+   for (i = 0; i < dev->nb_event_queues; i++) {
+   sso_port_link_modify((struct otx2_ssogws *)
+&ws->ws_state[0], i, false);
+   sso_port_link_modify((struct otx2_ssogws *)
+&ws->ws_state[1], i, false);
+   }
+   memset(ws, 0, sizeof(*ws));
+   } else {
+   struct otx2_ssogws *ws = port;
+
+   for (i = 0; i < dev->nb_event_queues; i++)
+   sso_port_link_modify(ws, i, false);
+   memset(ws, 0, sizeof(*ws));
+   }
+
+   memset(gws_cookie, 0, sizeof(*gws_cookie));
+
+free:
+   rte_free(gws_cookie);
 }
 
 static void
@@ -699,33 +728,6 @@ otx2_sso_queue_release(struct rte_eventdev *event_dev, 
uint8_t queue_id)
RTE_SET_USED(queue_id);
 }
 
-static void
-sso_clr_links(const struct rte_eventdev *event_dev)
-{
-   struct otx2_sso_evdev *dev = sso_pmd_priv(event_dev);
-   int i, j;
-
-   for (i = 0; i < dev->nb_event_ports; i++) {
-   if (dev->dual_ws) {
-   struct otx2_ssogws_dual *ws;
-
-   ws = event_dev->data->ports[i];
-   for (j = 0; j < dev->nb_event_queues; j++) {
-   sso_port_link_modify((struct otx2_ssogws *)
-   &ws->ws_state[0], j, false);
-   sso_port_link_modify((struct otx2_ssogws *)
-   &ws->ws_state[1], j, false);
-   }
-   } else {
-   struct otx2_ssogws *ws;
-
-   ws = event_dev->data->ports[i];
-   for (j = 0; j < dev->nb_event_queues; j++)
-   sso_port_link_modify(ws, j, false);
-   }
-   }
-}
-
 static void
 sso_restore_links(const struct rte_eventdev *event_dev)
 {
@@ -803,6 +805,7 @@ sso_configure_dual_ports(const struct rte_eventdev 
*event_dev)
}
 
for (i = 0; i < dev->nb_event_ports; i++) {
+   struct otx2_ssogws_cookie *gws_cookie;
struct otx2_ssogws_dual *ws;
uintptr_t base;
 
@@ -811,14 +814,20 @@ sso_configure_dual_ports(const struct rte_eventdev 
*event_dev)
} else {
/* Allocate event port memory */
ws = rte_zmalloc_socket("otx2_sso_ws",
-   sizeof(struct otx2_ssogws_dual),
+   sizeof(struct otx2_ssogws_dual) +
+   RTE_CACHE_LINE_SIZE,
RTE_CACHE_LINE_SIZE,
event_dev->data->socket_id);
-   }
-   if (ws == NULL) {
-   otx2_err("Failed to alloc memory for port=%d", i);
-   rc = -ENOMEM;
-   break;
+   if (ws == NULL) {
+   otx2_err("Failed to alloc memory for port=%d",
+i);
+   rc = -ENOMEM;
+   break;
+   }
+
+   /* First cache line reserved for cookie */
+   ws = (struct otx2_ssogws_dual *)
+   ((uint8_t *)ws + RTE_CACHE_LINE_SIZE);
}
 
ws->port = i;
@@ -830,6 +839,10 @@ sso_configure_dual_ports(const struct rte_eventdev 
*event_dev)
sso_set_port_ops((struct otx2_ssogws *)&ws->ws_state[1], base);
vws++;
 
+ 

Re: [dpdk-dev] [PATCH] doc: flow rule removal on port stop

[Gregory Etelson]

Hello Andrew,

> On 11/17/20 10:18 PM, Gregory Etelson wrote:
> > There is a discrepancy between RTE ETHDEV API and flow rules guide
> > regarding flow rules maintenance after port stop.  RTE ETHDEV API in
> > librte_ethdev.h declares that flow rules will not be stored in PMD
> > after port stop:
> >   > Quite start
> >   Please note that some configuration is not stored between calls to
> >   rte_eth_dev_stop()/rte_eth_dev_start(). The following configuration
> >   will be retained:
> >
> >   - MTU
> >   - flow control settings
> >   - receive mode configuration (promiscuous mode, all-multicast mode,
> > hardware checksum mode, RSS/VMDQ settings etc.)
> >   - VLAN filtering configuration
> >   - default MAC address
> >   - MAC addresses supplied to MAC address array
> >   - flow director filtering mode (but not filtering rules)
> >   - NIC queue statistics mappings
> >    Quote end
> >
> > PMD cannot always correctly restore flow rules after port stop / port
> > start because application may alter port configuration after port stop
> > without PMD knowledge about undergoing changes.  Consider the
> > following scenario:
> > application configures 2 queues 0 and 1 and creates a flow rule with
> > 'queue index 1' action. After that application stops the port and
> > removes queue 1.
> > Although PMD can implement flow rule shadow copy to be used for
> > restore after port start, attempt to restore flow rule from shadow
> > will fail in example above and PMD could not notify application about
> > that failure.  As the result, flow rules map in HW will differ from
> > what application expects.  In addition, flow rules shadow copy used
> > for port start restore consumes considerable amount of system memory,
> > especially in systems with millions of flow rules.
> >
> > Signed-off-by: Gregory Etelson 
> > Acked-by: Ori Kam 
> > ---
> >   doc/guides/prog_guide/rte_flow.rst | 5 ++---
> >   1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/doc/guides/prog_guide/rte_flow.rst
> > b/doc/guides/prog_guide/rte_flow.rst
> > index 944e8242d6..dfe5a40f8e 100644
> > --- a/doc/guides/prog_guide/rte_flow.rst
> > +++ b/doc/guides/prog_guide/rte_flow.rst
> > @@ -3055,10 +3055,9 @@ Caveats
> > temporarily replacing the burst function pointers), an appropriate
> error
> > code must be returned (``EBUSY``).
> >
> > -- PMDs, not applications, are responsible for maintaining flow rules
> > +- Applications, not PMDs, are responsible for maintaining flow rules
> > configuration when stopping and restarting a port or performing
> > other
> > -  actions which may affect them. They can only be destroyed
> > explicitly by
> > -  applications.
> > +  actions which may affect them.
> >
> >   For devices exposing multiple ports sharing global settings affected
> by flow
> >   rules:
> >
> 
> Re-reading it, it still looks vague. What happens on:
>   - port stop without removal of flow rule before
>   - port close without removal of flow rules before
>   - port reset (which could be stop/start, e.g. to recover from error
> condition)

PMD should remove all flows related to hardware resource that was invalidated.

  

[dpdk-dev] [PATCH 1/3] net/mlx5: fix unfreed memory on ASO age close

[Dekel Peled]

Recent patch introduced the use of ASO flow hit action for age action.
The relevant management struct uses dynamically allocated memory.
This memory was not freed on closing.

This patch adds memory freeing as needed.

Fixes: f935ed4b645a ("net/mlx5: support flow hit action for aging")

Signed-off-by: Dekel Peled 
Acked-by: Matan Azrad 
---
 drivers/net/mlx5/mlx5.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index ede5fd44ab..627e511b12 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -419,7 +419,7 @@ mlx5_flow_aso_age_mng_close(struct mlx5_dev_ctx_shared *sh)
}
mlx5_free(sh->aso_age_mng->pools);
}
-   memset(&sh->aso_age_mng, 0, sizeof(sh->aso_age_mng));
+   mlx5_free(sh->aso_age_mng);
 }
 
 /**
-- 
2.25.1

[dpdk-dev] [PATCH 3/3] common/mlx5: move to formal ASO action API

[Dekel Peled]

Existing code uses the previous API offered by rdma-core in order
to create ASO Flow Hit action.

A general API is now formally released, to create ASO action of any type.
This patch moves the MLX5 PMD code to use the formal API.

Signed-off-by: Dekel Peled 
Acked-by: Matan Azrad 
---
 drivers/common/mlx5/linux/meson.build |  4 +--
 drivers/common/mlx5/linux/mlx5_glue.c | 38 ---
 drivers/common/mlx5/linux/mlx5_glue.h |  6 ++---
 drivers/net/mlx5/linux/mlx5_os.c  |  4 +--
 drivers/net/mlx5/mlx5_flow_dv.c   | 11 +---
 5 files changed, 36 insertions(+), 27 deletions(-)

diff --git a/drivers/common/mlx5/linux/meson.build 
b/drivers/common/mlx5/linux/meson.build
index 87f7bfda51..63b78e4bce 100644
--- a/drivers/common/mlx5/linux/meson.build
+++ b/drivers/common/mlx5/linux/meson.build
@@ -181,8 +181,8 @@ has_sym_args = [
[ 'HAVE_MLX5_DR_CREATE_ACTION_DEST_ARRAY', 'infiniband/mlx5dv.h',
'mlx5dv_dr_action_create_dest_array'],
[ 'HAVE_DEVLINK', 'linux/devlink.h', 'DEVLINK_GENL_NAME' ],
-[ 'HAVE_MLX5DV_DR_ACTION_FLOW_HIT', 'infiniband/mlx5dv.h',
-'mlx5dv_dr_action_create_flow_hit'],
+[ 'HAVE_MLX5_DR_CREATE_ACTION_ASO', 'infiniband/mlx5dv.h',
+'mlx5dv_dr_action_create_aso' ],
 ]
 config = configuration_data()
 foreach arg:has_sym_args
diff --git a/drivers/common/mlx5/linux/mlx5_glue.c 
b/drivers/common/mlx5/linux/mlx5_glue.c
index cc6670c5bd..8146c79287 100644
--- a/drivers/common/mlx5/linux/mlx5_glue.c
+++ b/drivers/common/mlx5/linux/mlx5_glue.c
@@ -811,6 +811,27 @@ mlx5_glue_dv_modify_flow_action_meter(void *action,
 #endif
 }
 
+static void *
+mlx5_glue_dv_create_flow_action_aso(struct mlx5dv_dr_domain *domain,
+   void *aso_obj,
+   uint32_t offset,
+   uint32_t flags,
+   uint8_t return_reg_c)
+{
+#if defined(HAVE_MLX5DV_DR) && defined(HAVE_MLX5_DR_CREATE_ACTION_ASO)
+   return mlx5dv_dr_action_create_aso(domain, aso_obj, offset,
+  flags, return_reg_c);
+#else
+   (void)domain;
+   (void)aso_obj;
+   (void)offset;
+   (void)flags;
+   (void)return_reg_c;
+   errno = ENOTSUP;
+   return NULL;
+#endif
+}
+
 static void *
 mlx5_glue_dr_create_flow_action_default_miss(void)
 {
@@ -1281,21 +1302,6 @@ mlx5_glue_dv_free_pp(struct mlx5dv_pp *pp)
 #endif
 }
 
-static void *
-mlx5_glue_dr_action_create_flow_hit(struct mlx5dv_devx_obj *devx_obj,
-   uint32_t offset, uint8_t reg_c_index)
-{
-#ifdef HAVE_MLX5DV_DR_ACTION_FLOW_HIT
-   return mlx5dv_dr_action_create_flow_hit(devx_obj, offset, reg_c_index);
-#else
-   (void)(devx_obj);
-   (void)(offset);
-   (void)(reg_c_index);
-   errno = ENOTSUP;
-   return NULL;
-#endif
-}
-
 __rte_cache_aligned
 const struct mlx5_glue *mlx5_glue = &(const struct mlx5_glue) {
.version = MLX5_GLUE_VERSION,
@@ -1379,6 +1385,7 @@ const struct mlx5_glue *mlx5_glue = &(const struct 
mlx5_glue) {
.dv_create_flow_action_tag =  mlx5_glue_dv_create_flow_action_tag,
.dv_create_flow_action_meter = mlx5_glue_dv_create_flow_action_meter,
.dv_modify_flow_action_meter = mlx5_glue_dv_modify_flow_action_meter,
+   .dv_create_flow_action_aso = mlx5_glue_dv_create_flow_action_aso,
.dr_create_flow_action_default_miss =
mlx5_glue_dr_create_flow_action_default_miss,
.dv_destroy_flow = mlx5_glue_dv_destroy_flow,
@@ -1415,5 +1422,4 @@ const struct mlx5_glue *mlx5_glue = &(const struct 
mlx5_glue) {
.dv_free_var = mlx5_glue_dv_free_var,
.dv_alloc_pp = mlx5_glue_dv_alloc_pp,
.dv_free_pp = mlx5_glue_dv_free_pp,
-   .dr_action_create_flow_hit = mlx5_glue_dr_action_create_flow_hit,
 };
diff --git a/drivers/common/mlx5/linux/mlx5_glue.h 
b/drivers/common/mlx5/linux/mlx5_glue.h
index c5d7853ea9..8be446a902 100644
--- a/drivers/common/mlx5/linux/mlx5_glue.h
+++ b/drivers/common/mlx5/linux/mlx5_glue.h
@@ -344,9 +344,9 @@ struct mlx5_glue {
(void *domain,
 size_t num_dest,
 struct mlx5dv_dr_action_dest_attr *dests[]);
-   void *(*dr_action_create_flow_hit)(struct mlx5dv_devx_obj *devx_obj,
-  uint32_t offset,
-  uint8_t reg_c_index);
+   void *(*dv_create_flow_action_aso)
+   (struct mlx5dv_dr_domain *domain, void *aso_obj,
+uint32_t offset, uint32_t flags, uint8_t return_reg_c);
 };
 
 extern const struct mlx5_glue *mlx5_glue;
diff --git a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c
index 468ef3356f..2dfed4b16f 100644
--- a/drivers/net/mlx5/linux/mlx5_os.c
+++ b/drivers/net/mlx5/linux/mlx5_os.c
@@ -1200,7 +1200,7 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
  

[dpdk-dev] [PATCH 2/3] net/mlx5: fix input register for ASO object

[Dekel Peled]

Existing code uses the hard-coded value REG_C_5 as input for function
mlx5dv_dr_action_create_flow_hit().

This patch updates function mlx5_flow_get_reg_id() to return the
selected REG_C value for ASO Flow Hit operation.
The returned value is used, after reducing offset REG_C_0, as input
for function mlx5dv_dr_action_create_flow_hit().

Fixes: f935ed4b645a ("net/mlx5: support flow hit action for aging")

Signed-off-by: Dekel Peled 
Acked-by: Matan Azrad 
---
 drivers/net/mlx5/linux/mlx5_os.c | 36 +++---
 drivers/net/mlx5/mlx5_flow.c |  1 +
 drivers/net/mlx5/mlx5_flow.h |  1 +
 drivers/net/mlx5/mlx5_flow_dv.c  | 38 +---
 4 files changed, 55 insertions(+), 21 deletions(-)

diff --git a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c
index 4b7fff4eff..468ef3356f 100644
--- a/drivers/net/mlx5/linux/mlx5_os.c
+++ b/drivers/net/mlx5/linux/mlx5_os.c
@@ -1135,17 +1135,6 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
err = -err;
goto error;
}
-#ifdef HAVE_MLX5DV_DR_ACTION_FLOW_HIT
-   if (config->hca_attr.flow_hit_aso) {
-   sh->flow_hit_aso_en = 1;
-   err = mlx5_flow_aso_age_mng_init(sh);
-   if (err) {
-   err = -err;
-   goto error;
-   }
-   DRV_LOG(DEBUG, "Flow Hit ASO is supported.");
-   }
-#endif /* HAVE_MLX5DV_DR_ACTION_FLOW_HIT */
/* Check relax ordering support. */
if (!haswell_broadwell_cpu) {
sh->cmng.relaxed_ordering_write =
@@ -1192,8 +1181,17 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
DRV_LOG(WARNING, "No available register for"
" meter.");
} else {
-   priv->mtr_color_reg = ffs(reg_c_mask) - 1 +
- REG_C_0;
+   /*
+* The meter color register is used by the
+* flow-hit feature as well.
+* The flow-hit feature must use REG_C_3
+* Prefer REG_C_3 if it is available.
+*/
+   if (reg_c_mask & (1 << (REG_C_3 - REG_C_0)))
+   priv->mtr_color_reg = REG_C_3;
+   else
+   priv->mtr_color_reg = ffs(reg_c_mask)
+ - 1 + REG_C_0;
priv->mtr_en = 1;
priv->mtr_reg_share =
  config->hca_attr.qos.flow_meter_reg_share;
@@ -1202,6 +1200,18 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
}
}
 #endif
+#ifdef HAVE_MLX5DV_DR_ACTION_FLOW_HIT
+   if (config->hca_attr.flow_hit_aso &&
+   priv->mtr_color_reg == REG_C_3) {
+   sh->flow_hit_aso_en = 1;
+   err = mlx5_flow_aso_age_mng_init(sh);
+   if (err) {
+   err = -err;
+   goto error;
+   }
+   DRV_LOG(DEBUG, "Flow Hit ASO is supported.");
+   }
+#endif /* HAVE_MLX5DV_DR_ACTION_FLOW_HIT */
 #if defined(HAVE_MLX5DV_DR) && defined(HAVE_MLX5_DR_CREATE_ACTION_FLOW_SAMPLE)
if (config->hca_attr.log_max_ft_sampler_num > 0  &&
config->dv_flow_en) {
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 33dbbd9eef..bd2e588187 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -777,6 +777,7 @@ mlx5_flow_get_reg_id(struct rte_eth_dev *dev,
return priv->mtr_color_reg != REG_C_2 ? REG_C_2 :
   REG_C_3;
case MLX5_MTR_COLOR:
+   case MLX5_ASO_FLOW_HIT: /* Both features use the same REG_C. */
MLX5_ASSERT(priv->mtr_color_reg != REG_NON);
return priv->mtr_color_reg;
case MLX5_COPY_MARK:
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index afddcfc12c..0322db9adc 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -80,6 +80,7 @@ enum mlx5_feature_name {
MLX5_COPY_MARK,
MLX5_MTR_COLOR,
MLX5_MTR_SFX,
+   MLX5_ASO_FLOW_HIT,
 };
 
 /* Default queue number. */
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 3d69957da7..ae8967ec58 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -9449,12 +9449,14 @@ flow_dv_age_

[dpdk-dev] [PATCH 0/3] ASO age action fixes

[Dekel Peled]

The age action using ASO was recently introduced in MLX5 PMD.
This series includes fixes for this feature, repairing issues found
and updating it to the released rdma-core API.

Dekel Peled (3):
  net/mlx5: fix unfreed memory on ASO age close
  net/mlx5: fix input register for ASO object
  common/mlx5: move to formal ASO action API

 drivers/common/mlx5/linux/meson.build |  4 +--
 drivers/common/mlx5/linux/mlx5_glue.c | 38 --
 drivers/common/mlx5/linux/mlx5_glue.h |  6 ++--
 drivers/net/mlx5/linux/mlx5_os.c  | 36 +
 drivers/net/mlx5/mlx5.c   |  2 +-
 drivers/net/mlx5/mlx5_flow.c  |  1 +
 drivers/net/mlx5/mlx5_flow.h  |  1 +
 drivers/net/mlx5/mlx5_flow_dv.c   | 45 +--
 8 files changed, 88 insertions(+), 45 deletions(-)

-- 
2.25.1

Re: [dpdk-dev] [PATCH] doc: flow rule removal on port stop

[Andrew Rybchenko]

On 11/18/20 11:59 AM, Gregory Etelson wrote:
> Hello Andrew,
> 
>> On 11/17/20 10:18 PM, Gregory Etelson wrote:
>>> There is a discrepancy between RTE ETHDEV API and flow rules guide
>>> regarding flow rules maintenance after port stop.  RTE ETHDEV API in
>>> librte_ethdev.h declares that flow rules will not be stored in PMD
>>> after port stop:
>>>   > Quite start
>>>   Please note that some configuration is not stored between calls to
>>>   rte_eth_dev_stop()/rte_eth_dev_start(). The following configuration
>>>   will be retained:
>>>
>>>   - MTU
>>>   - flow control settings
>>>   - receive mode configuration (promiscuous mode, all-multicast mode,
>>> hardware checksum mode, RSS/VMDQ settings etc.)
>>>   - VLAN filtering configuration
>>>   - default MAC address
>>>   - MAC addresses supplied to MAC address array
>>>   - flow director filtering mode (but not filtering rules)
>>>   - NIC queue statistics mappings
>>>    Quote end
>>>
>>> PMD cannot always correctly restore flow rules after port stop / port
>>> start because application may alter port configuration after port stop
>>> without PMD knowledge about undergoing changes.  Consider the
>>> following scenario:
>>> application configures 2 queues 0 and 1 and creates a flow rule with
>>> 'queue index 1' action. After that application stops the port and
>>> removes queue 1.
>>> Although PMD can implement flow rule shadow copy to be used for
>>> restore after port start, attempt to restore flow rule from shadow
>>> will fail in example above and PMD could not notify application about
>>> that failure.  As the result, flow rules map in HW will differ from
>>> what application expects.  In addition, flow rules shadow copy used
>>> for port start restore consumes considerable amount of system memory,
>>> especially in systems with millions of flow rules.
>>>
>>> Signed-off-by: Gregory Etelson 
>>> Acked-by: Ori Kam 
>>> ---
>>>   doc/guides/prog_guide/rte_flow.rst | 5 ++---
>>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/doc/guides/prog_guide/rte_flow.rst
>>> b/doc/guides/prog_guide/rte_flow.rst
>>> index 944e8242d6..dfe5a40f8e 100644
>>> --- a/doc/guides/prog_guide/rte_flow.rst
>>> +++ b/doc/guides/prog_guide/rte_flow.rst
>>> @@ -3055,10 +3055,9 @@ Caveats
>>> temporarily replacing the burst function pointers), an appropriate
>> error
>>> code must be returned (``EBUSY``).
>>>
>>> -- PMDs, not applications, are responsible for maintaining flow rules
>>> +- Applications, not PMDs, are responsible for maintaining flow rules
>>> configuration when stopping and restarting a port or performing
>>> other
>>> -  actions which may affect them. They can only be destroyed
>>> explicitly by
>>> -  applications.
>>> +  actions which may affect them.
>>>
>>>   For devices exposing multiple ports sharing global settings affected
>> by flow
>>>   rules:
>>>
>>
>> Re-reading it, it still looks vague. What happens on:
>>   - port stop without removal of flow rule before
>>   - port close without removal of flow rules before
>>   - port reset (which could be stop/start, e.g. to recover from error
>> condition)
> 
> PMD should remove all flows related to hardware resource that was invalidated.

Stop? Close? I agree and documentation should say so in a bit
clear way.

Re: [dpdk-dev] [PATCH] doc: flow rule removal on port stop

[Gregory Etelson]

> >> On 11/17/20 10:18 PM, Gregory Etelson wrote:
> >>> There is a discrepancy between RTE ETHDEV API and flow rules guide
> >>> regarding flow rules maintenance after port stop.  RTE ETHDEV API in
> >>> librte_ethdev.h declares that flow rules will not be stored in PMD
> >>> after port stop:
> >>>   > Quite start
> >>>   Please note that some configuration is not stored between calls to
> >>>   rte_eth_dev_stop()/rte_eth_dev_start(). The following configuration
> >>>   will be retained:
> >>>
> >>>   - MTU
> >>>   - flow control settings
> >>>   - receive mode configuration (promiscuous mode, all-multicast mode,
> >>> hardware checksum mode, RSS/VMDQ settings etc.)
> >>>   - VLAN filtering configuration
> >>>   - default MAC address
> >>>   - MAC addresses supplied to MAC address array
> >>>   - flow director filtering mode (but not filtering rules)
> >>>   - NIC queue statistics mappings
> >>>    Quote end
> >>>
> >>> PMD cannot always correctly restore flow rules after port stop /
> >>> port start because application may alter port configuration after
> >>> port stop without PMD knowledge about undergoing changes.  Consider
> >>> the following scenario:
> >>> application configures 2 queues 0 and 1 and creates a flow rule with
> >>> 'queue index 1' action. After that application stops the port and
> >>> removes queue 1.
> >>> Although PMD can implement flow rule shadow copy to be used for
> >>> restore after port start, attempt to restore flow rule from shadow
> >>> will fail in example above and PMD could not notify application
> >>> about that failure.  As the result, flow rules map in HW will differ
> >>> from what application expects.  In addition, flow rules shadow copy
> >>> used for port start restore consumes considerable amount of system
> >>> memory, especially in systems with millions of flow rules.
> >>>
> >>> Signed-off-by: Gregory Etelson 
> >>> Acked-by: Ori Kam 
> >>> ---
> >>>   doc/guides/prog_guide/rte_flow.rst | 5 ++---
> >>>   1 file changed, 2 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/doc/guides/prog_guide/rte_flow.rst
> >>> b/doc/guides/prog_guide/rte_flow.rst
> >>> index 944e8242d6..dfe5a40f8e 100644
> >>> --- a/doc/guides/prog_guide/rte_flow.rst
> >>> +++ b/doc/guides/prog_guide/rte_flow.rst
> >>> @@ -3055,10 +3055,9 @@ Caveats
> >>> temporarily replacing the burst function pointers), an
> >>> appropriate
> >> error
> >>> code must be returned (``EBUSY``).
> >>>
> >>> -- PMDs, not applications, are responsible for maintaining flow
> >>> rules
> >>> +- Applications, not PMDs, are responsible for maintaining flow
> >>> +rules
> >>> configuration when stopping and restarting a port or performing
> >>> other
> >>> -  actions which may affect them. They can only be destroyed
> >>> explicitly by
> >>> -  applications.
> >>> +  actions which may affect them.
> >>>
> >>>   For devices exposing multiple ports sharing global settings
> >>> affected
> >> by flow
> >>>   rules:
> >>>
> >>
> >> Re-reading it, it still looks vague. What happens on:
> >>   - port stop without removal of flow rule before
> >>   - port close without removal of flow rules before
> >>   - port reset (which could be stop/start, e.g. to recover from error
> >> condition)
> >
> > PMD should remove all flows related to hardware resource that was
> invalidated.
> 
> Stop? Close? I agree and documentation should say so in a bit clear way.

I'll post updated document patch.

Re: [dpdk-dev] [EXT] Re: [PATCH] maintainers: Update for OcteonTx2 DMA and EP

[Thomas Monjalon]

18/11/2020 05:15, Radha Mohan:
> On Mon, Nov 16, 2020 at 12:28 PM Thomas Monjalon  wrote:
> > 16/11/2020 19:28, Radha Mohan:
> > > On Fri, Nov 13, 2020 at 2:39 PM Thomas Monjalon  
> > > wrote:
> > > > 13/11/2020 20:18, Radha Mohan:
> > > > > On Tue, Nov 10, 2020 at 11:57 PM Mahipal Challa  
> > > > > wrote:
> > > > > From: Radha Mohan 
> > > > > Sent: Tuesday, November 10, 2020 11:44 PM
> > > > > > On Mon, Nov 9, 2020 at 4:20 PM Radha Mohan Chintakuntla 
> > > > > >  wrote:
> > > > > > >
> > > > > > > Replace the maintainers for OcteonTx2 DMA and EP drivers.
> > > > > > >
> > > > > > > Signed-off-by: Radha Mohan Chintakuntla 
> > > > [...]
> > > > > > >Adding previous maintainers to ack.
> > > > > > Acked-by: Mahipal Challa  > > > > >
> > > > > > Thanks,
> > > > > > Mahipal
> > > > >
> > > > > Hi Thomas,
> > > > > Could you please pick this patch ?
> > > >
> > > > This low-priority patch has been sent 4 days ago,
> > > > and was acked by only 1 maintainer 3 days ago.
> > > >
> > >
> > > It was acked by both the old maintainers Satha Koteshwar and Mahipal
> > > Challa. Maybe both of those emails didn't come on top of each other.
> >
> > I missed one.
> >
> > > > Why pushing? What is your fear exactly?
> > >
> > > Just want to make the transfer of ownership official in the mainline.
> > > There's no fear I just normally wanted to send a reminder to you to
> > > pick as it might be missed inclusion. If there's a guarantee that all
> > > ack'ed patches will go automatically then I won't be sending any
> > > reminder emails in future.
> >
> > It's OK to send reminder, but please wait at least a week.
> >
> > > > Why the new maintainers candidates have 0 and 2 contributions
> > > > in git history?
> > > >
> > > These drivers requires some maintenance and likely expanded to support
> > > future chips from Marvell. So you will not see contributions right
> > > away.
> > > So to become a maintainer one has to have prior contributions ?
> >
> > Yes: 
> > http://doc.dpdk.org/guides/contributing/patches.html#maintainers-and-sub-trees
> > "
> > Maintainers should have demonstrated a reasonable level of contributions or 
> > reviews to the component area. The maintainer should be confirmed by an ack 
> > from an established contributor.
> > "
> 
> How was this done without prior contributions?
> http://git.dpdk.org/dpdk/commit/MAINTAINERS?id=238e3167ca869abf44fa50ead022d7fc3b99605b

I asked a confirmation:
http://inbox.dpdk.org/dev/1985242.AXpOyGb66D@thomas/


> The commit log says that he is a "new developer". So am I technically
> to the community.
> 
> Also the thing that puzzles me most is its a driver specific to
> Marvell and both the previous maintainers are ok with the transfer but
> a strange rule that probably applies to generic things of dpdk code
> (which really makes sense there) comes as a blocker.

Did I say it is a blocker?

> And how does new driver go?

For new drivers, the new maintainers arrive by contributing new code.

> Lets say if either myself or veerasena have pushed a new driver now
> who has to ack it for inclusion ? Isn't that like a chicken and egg
> problem.

Nobody has to ack for a new driver.

> I am looking for clarifications as things are different in other open
> source communities.
> 
> > > Kind of makes odd sense as these drivers are specific to our chip and we
> > > are changing ownership for maintaining them.
> > > I do not understand why you have an issue here?
> > > You don't like reminding then I understand.
> >
> > I don't like how you push new unconfirmed maintainers.
> 
> Could you please explain above commit made into new maintainer?

Listen: I say I don't like how pushy you are, that's all.
In general it's very good to have new contributors.
But please think how you can help instead of just requesting.
We have a lot more important things to care at the moment
to close the big release 20.11. If you want to help, you are welcome.

[dpdk-dev] [PATCH] net/mlx5: fix RSS queue types validation

[Dekel Peled]

Recent patch fixed the RSS action validation, making sure hairpin queues
and standard queues are not used together in the same RSS action.
The variable used for comparison was declared and initialized within the
check loop, making the queue type comparison wrong.

This patch moves the variable declaration to the start of the function,
outside of the check loop.

Fixes: cb8a079aee5d ("net/mlx5: fix validate RSS queues types")

Signed-off-by: Dekel Peled 
Acked-by: Ori Kam 
Acked-by: Jack Min 
---
 drivers/net/mlx5/mlx5_flow.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 33dbbd9eef..236610c8fc 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -1411,6 +1411,7 @@ mlx5_validate_action_rss(struct rte_eth_dev *dev,
 {
struct mlx5_priv *priv = dev->data->dev_private;
const struct rte_flow_action_rss *rss = action->conf;
+   enum mlx5_rxq_type rxq_type = MLX5_RXQ_TYPE_UNDEFINED;
unsigned int i;
 
if (rss->func != RTE_ETH_HASH_FUNCTION_DEFAULT &&
@@ -1476,7 +1477,6 @@ mlx5_validate_action_rss(struct rte_eth_dev *dev,
  RTE_FLOW_ERROR_TYPE_ACTION_CONF,
  NULL, "No queues configured");
for (i = 0; i != rss->queue_num; ++i) {
-   enum mlx5_rxq_type rxq_type = MLX5_RXQ_TYPE_UNDEFINED;
struct mlx5_rxq_ctrl *rxq_ctrl;
 
if (rss->queue[i] >= priv->rxqs_n)
-- 
2.25.1

[dpdk-dev] [Bug 578] i40evf_check_api_version(): PF/VF API version mismatch:(0.0)-(1.1) with multiple DPDK instances using different VFs with the same PF

[bugzilla]

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

Bug ID: 578
   Summary: i40evf_check_api_version(): PF/VF API version
mismatch:(0.0)-(1.1) with multiple DPDK instances
using different VFs with the same PF
   Product: DPDK
   Version: 20.08
  Hardware: All
OS: Linux
Status: UNCONFIRMED
  Severity: normal
  Priority: Normal
 Component: ethdev
  Assignee: dev@dpdk.org
  Reporter: juraj.lin...@pantheon.tech
  Target Milestone: ---

ISSUE DESCRIPTION:
VPP uses DPDK during startup with these init args:
EAL init args: -c 2 -n 4 --in-memory --log-level debug --file-prefix
vpp -w :91:03.0 -w :91:03.1 --master-lcore 33

When there's only one VPP starting up, the initialization is successful. When
we add multiple VPPs the initialization sometimes fails:
2020/11/16 08:58:49:660 notice dpdk   EAL: Detected 64 lcore(s)
2020/11/16 08:58:49:660 notice dpdk   EAL: Detected 2 NUMA nodes
2020/11/16 08:58:49:660 notice dpdk   EAL: Selected IOVA mode 'VA'
2020/11/16 08:58:49:660 notice dpdk   EAL: Probing VFIO support...
2020/11/16 08:58:49:660 notice dpdk   EAL: VFIO support initialized
2020/11/16 08:58:49:660 notice dpdk   EAL:   using IOMMU type 1
(Type 1)
2020/11/16 08:58:49:660 notice dpdk   EAL: Probe PCI driver:
net_i40e_vf (8086:154c) device: :91:03.0 (socket 1)
2020/11/16 08:58:49:660 notice dpdk   i40evf_check_api_version():
PF/VF API version mismatch:(0.0)-(1.1)
2020/11/16 08:58:49:660 notice dpdk   i40evf_init_vf(): check_api
version failed
2020/11/16 08:58:49:660 notice dpdk   i40evf_dev_init(): Init vf
failed
2020/11/16 08:58:49:660 notice dpdk   EAL: Releasing pci mapped
resource for :91:03.0
2020/11/16 08:58:49:660 notice dpdk   EAL: Calling
pci_unmap_resource for :91:03.0 at 0x210100
2020/11/16 08:58:49:660 notice dpdk   EAL: Calling
pci_unmap_resource for :91:03.0 at 0x210101
2020/11/16 08:58:49:660 notice dpdk   EAL: Requested device
:91:03.0 cannot be used
2020/11/16 08:58:49:660 notice dpdk   EAL:   using IOMMU type 1
(Type 1)


EXPANDED SETUP/FAILURE EXPLANATION:
There is one testsuite running tests serially in one Jenkins job. Each test
starts VPP anew, so the DPDK initialization happens multiple times during a
testsuite run. When there is only one Jenkins job running, no failure is
observed. With multiple Jenkins jobs (and multiple testsuites running in
parallel), there is a small likelihood that a small number (usually just 1) of
initialization failures will occur, suggesting that when multiple DPDK
instances are retrieving PF/VF version there's a race condition resulting in PF
version 0.0 being retrieved.
Each Jenkins job uses two different VFs from the same PF.

An illustration of what likely happens:
VPP1 in job1 starts and DPDK initializes VF1 and VF2 from a PF.
VPP2 in job2 starts and DPDK initializes VF3 and VF4 from the same PF.
One or more of VF1, VF2, VF3 or VF4 don't get properly initialized because of:
i40evf_check_api_version(): PF/VF API version mismatch:(0.0)-(1.1)

NIC:
Intel Ethernet Converged Network Adapter XL710-Q2
cat /sys/class/net/enp5s0f0/device/device 
0x1583
cat /sys/class/net/enp5s0f0/device/vendor 
0x8086
cat /sys/class/net/enp5s0f0/device/subsystem_device 
0x0001
cat /sys/class/net/enp5s0f0/device/subsystem_vendor 
0x8086

DRIVER/FIRMWARE VERSION:
driver: i40e
version: 2.1.14-k
firmware-version: 6.01 0x800035da 1.1747.0
expansion-rom-version: 
bus-info: :05:00.0
supports-statistics: yes
supports-test: yes
supports-eeprom-access: yes
supports-register-dump: yes
supports-priv-flags: yes

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

Re: [dpdk-dev] [PATCH] bus/pci: fix comment explaining device naming

[David Marchand]

On Mon, Nov 16, 2020 at 11:12 AM Gaetan Rivet  wrote:
>
> The original triple negative was hard to read and the attempt
> to improve the formulation was commendable, unfortunately the new
> comment is the inverse of correct.
>
> Fixes: a65a34a85ebf ("eal: replace usage of blacklist/whitelist in enums")
> Cc: step...@networkplumber.org
> Signed-off-by: Gaetan Rivet 
> ---
>
> No Cc:stable as it was not yet released.
>
>  drivers/bus/pci/pci_common.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
> index b24c069713..d55e5a38cf 100644
> --- a/drivers/bus/pci/pci_common.c
> +++ b/drivers/bus/pci/pci_common.c
> @@ -68,7 +68,9 @@ pci_name_set(struct rte_pci_device *dev)
> devargs = pci_devargs_lookup(&dev->addr);
> dev->device.devargs = devargs;
>
> -   /* If the device is blocked, no rte_devargs exists for it. */
> +   /* When using a block-list, only blocked devices will have

Nit: I don't think we need a -, I would go with blocklist.

> +* an rte_devargs. Allowed devices won't have one.
> +*/
> if (devargs != NULL)
> /* If an rte_devargs exists, the generic rte_device uses the
>  * given name as its name.
> --
> 2.29.2
>

Reviewed-by: David Marchand 


-- 
David Marchand

Re: [dpdk-dev] [PATCH 4/4] app/procinfo: remove useless assignment

[Ferruh Yigit]

On 11/17/2020 6:04 PM, Varghese, Vipin wrote:

Hi Ferruh,

Thanks for the update

snipped

show_mempool(char *name)  {
-uint64_t flags = 0;
+uint64_t flags;



Checking the current code base it makes more sense to move the code inside `if` 
condition check. Sample code shared below

```
-uint64_t flags = 0;

snprintf(bdr_str, MAX_STRING_LEN, " show - MEMPOOL ");
STATS_BDR_STR(10, bdr_str);

if (name != NULL) {
struct rte_mempool *ptr = rte_mempool_lookup(name);
if (ptr != NULL) {
struct rte_mempool_ops *ops;

+unsigned int flags = ptr->flags;
ops = rte_mempool_get_ops(ptr->ops_index);
```

But it is ok



I think both are OK, this is trivial, let me send a quick v2.

[dpdk-dev] [PATCH v2 0/4] Enable Checksum Offloading for NEON vector

[Feifei Wang]

This patch series are mainly to enable checksum offloading for IXGBE NEON
vector PMD, including good and bad checksum flags. In the meanwhile, the
first patch enable VLAN stripping flag for Arm.
Following are the test results for the patches:

NICs: 82599(igb)
Driver: ixgbe(vector)
$:./app/dpdk-testpmd -c 0x3 -w 0002:f9:00.0 -- -i --port-topology=chained
test-pmd> set fwd rxonly
test-pmd> set verbose 1
test-pmd> start

With Patch a:
enable vlan stripping:
src=00:00:00:00:00:02 - dst=00:00:00:00:00:01 - type=0x0800 - length=70 - 
nb_segs=1 - VLAN tci=0x1
ol_flags: PKT_RX_VLAN PKT_RX_VLAN_STRIPPED

With Patch b:
Packet: IPv4_checksum = 0xee && UDP_checksum = 0xee
src=00:00:00:00:00:02 - dst=00:00:00:00:00:01 - type=0x0800 - length=70 - 
nb_segs=1
ol_flags: PKT_RX_L4_CKSUM_BAD PKT_RX_IP_CKSUM_BAD

With Patch c:
Packet: IPv4_checksum = correct value && UDP_checksum = correct value
ol_flags: PKT_RX_L4_CKSUM_GOOD PKT_RX_IP_CKSUM_GOOD

v2:
1. update the commit message title to align the design (Wang, Haiyue)
2. make the commit message more descriptive and move the test results
   to the cover letter (Wang, Haiyue)

Feifei Wang (4):
 a). net/ixgbe: add new flag of stripped VLAN for NEON vector
 b). net/ixgbe: support bad checksum flag for NEON vector
 c). net/ixgbe: support good checksum flag for NEON vector
 d). net/ixgbe: enable IXGBE NEON vector when need to checksum

 drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c | 94 ++---
 1 file changed, 68 insertions(+), 26 deletions(-)

-- 
2.17.1

[dpdk-dev] [PATCH v2 1/4] net/ixgbe: add new flag of stripped VLAN for NEON vector

[Feifei Wang]

For NEON vector of IXGBE PMD, introduce new flag PKT_RX_VLAN_STRIPPED to
show the case that the VLAN is stripped from the VLAN tagged packet.

This is because that the old flag PKT_RX_VLAN_PKT only indicates that the
packet is VLAN tagged, but cannot show whether VLAN is in m->vlan_tci or
in the packet at present. So add new flag to show the vlan has been
stripped by the hardware and its tci is saved in m->vlan_tci when vlan
stripping is enabled in the RX configuration of the IXGBE PMD.

Signed-off-by: Feifei Wang 
Reviewed-by: Ruifeng Wang 
---
 drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c | 36 -
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c 
b/drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c
index 4c81ae9dc..e6d877af9 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c
@@ -81,11 +81,9 @@ ixgbe_rxq_rearm(struct ixgbe_rx_queue *rxq)
IXGBE_PCI_REG_WRITE(rxq->rdt_reg_addr, rx_id);
 }
 
-#define VTAG_SHIFT (3)
-
 static inline void
 desc_to_olflags_v(uint8x16x2_t sterr_tmp1, uint8x16x2_t sterr_tmp2,
- uint8x16_t staterr, struct rte_mbuf **rx_pkts)
+ uint8x16_t staterr, uint8_t vlan_flags, struct rte_mbuf 
**rx_pkts)
 {
uint8x16_t ptype;
uint8x16_t vtag;
@@ -95,13 +93,6 @@ desc_to_olflags_v(uint8x16x2_t sterr_tmp1, uint8x16x2_t 
sterr_tmp2,
uint32_t word;
} vol;
 
-   const uint8x16_t pkttype_msk = {
-   PKT_RX_VLAN, PKT_RX_VLAN,
-   PKT_RX_VLAN, PKT_RX_VLAN,
-   0x00, 0x00, 0x00, 0x00,
-   0x00, 0x00, 0x00, 0x00,
-   0x00, 0x00, 0x00, 0x00};
-
const uint8x16_t rsstype_msk = {
0x0F, 0x0F, 0x0F, 0x0F,
0x00, 0x00, 0x00, 0x00,
@@ -114,12 +105,26 @@ desc_to_olflags_v(uint8x16x2_t sterr_tmp1, uint8x16x2_t 
sterr_tmp2,
PKT_RX_RSS_HASH, 0, 0, 0,
0, 0, 0, PKT_RX_FDIR};
 
+   const uint8x16_t vlan_msk = {
+   IXGBE_RXD_STAT_VP, IXGBE_RXD_STAT_VP,
+   IXGBE_RXD_STAT_VP, IXGBE_RXD_STAT_VP,
+   0, 0, 0, 0,
+   0, 0, 0, 0,
+   0, 0, 0, 0};
+
+   const uint8x16_t vlan_map = {
+   0, 0, 0, 0,
+   0, 0, 0, 0,
+   vlan_flags, 0, 0, 0,
+   0, 0, 0, 0};
+
ptype = vzipq_u8(sterr_tmp1.val[0], sterr_tmp2.val[0]).val[0];
ptype = vandq_u8(ptype, rsstype_msk);
ptype = vqtbl1q_u8(rss_flags, ptype);
 
-   vtag = vshrq_n_u8(staterr, VTAG_SHIFT);
-   vtag = vandq_u8(vtag, pkttype_msk);
+   /* extract vlan_flags from IXGBE_RXD_STAT_VP bits of staterr */
+   vtag = vandq_u8(staterr, vlan_msk);
+   vtag = vqtbl1q_u8(vlan_map, vtag);
vtag = vorrq_u8(ptype, vtag);
 
vol.word = vgetq_lane_u32(vreinterpretq_u32_u8(vtag), 0);
@@ -221,6 +226,7 @@ _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq, struct 
rte_mbuf **rx_pkts,
};
uint16x8_t crc_adjust = {0, 0, rxq->crc_len, 0,
 rxq->crc_len, 0, 0, 0};
+   uint8_t vlan_flags;
 
/* nb_pkts has to be floor-aligned to RTE_IXGBE_DESCS_PER_LOOP */
nb_pkts = RTE_ALIGN_FLOOR(nb_pkts, RTE_IXGBE_DESCS_PER_LOOP);
@@ -250,6 +256,10 @@ _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq, struct 
rte_mbuf **rx_pkts,
 */
sw_ring = &rxq->sw_ring[rxq->rx_tail];
 
+   /* ensure these 2 flags are in the lower 8 bits */
+   RTE_BUILD_BUG_ON((PKT_RX_VLAN | PKT_RX_VLAN_STRIPPED) > UINT8_MAX);
+   vlan_flags = rxq->vlan_flags & UINT8_MAX;
+
/* A. load 4 packet in one loop
 * B. copy 4 mbuf point from swring to rx_pkts
 * C. calc the number of DD bits among the 4 packets
@@ -311,7 +321,7 @@ _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq, struct 
rte_mbuf **rx_pkts,
staterr = vzipq_u8(sterr_tmp1.val[1], sterr_tmp2.val[1]).val[0];
 
/* set ol_flags with vlan packet type */
-   desc_to_olflags_v(sterr_tmp1, sterr_tmp2, staterr,
+   desc_to_olflags_v(sterr_tmp1, sterr_tmp2, staterr, vlan_flags,
  &rx_pkts[pos]);
 
/* D.2 pkt 3,4 set in_port/nb_seg and remove crc */
-- 
2.17.1

[dpdk-dev] [PATCH v2 2/4] net/ixgbe: support bad checksum flag for NEON vector

[Feifei Wang]

Updated desc_to_olflags_v() to support PKT_RX_IP_CKSUM_BAD and
PKT_RX_L4_CKSUM_BAD in the ol_flags of the mbuf.

And then the NEON vector RX function can be called with hw_ip_checksum
enabled.

Signed-off-by: Feifei Wang 
Reviewed-by: Ruifeng Wang 
---
 drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c | 47 +++--
 1 file changed, 36 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c 
b/drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c
index e6d877af9..4d6f057e7 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c
@@ -87,6 +87,8 @@ desc_to_olflags_v(uint8x16x2_t sterr_tmp1, uint8x16x2_t 
sterr_tmp2,
 {
uint8x16_t ptype;
uint8x16_t vtag;
+   uint8x16_t temp_csum;
+   uint32x4_t csum = {0, 0, 0, 0};
 
union {
uint8_t e[4];
@@ -105,26 +107,51 @@ desc_to_olflags_v(uint8x16x2_t sterr_tmp1, uint8x16x2_t 
sterr_tmp2,
PKT_RX_RSS_HASH, 0, 0, 0,
0, 0, 0, PKT_RX_FDIR};
 
-   const uint8x16_t vlan_msk = {
+   /* mask everything except vlan present and l4/ip csum error */
+   const uint8x16_t vlan_csum_msk = {
IXGBE_RXD_STAT_VP, IXGBE_RXD_STAT_VP,
IXGBE_RXD_STAT_VP, IXGBE_RXD_STAT_VP,
0, 0, 0, 0,
0, 0, 0, 0,
-   0, 0, 0, 0};
-
-   const uint8x16_t vlan_map = {
+   (IXGBE_RXDADV_ERR_TCPE | IXGBE_RXDADV_ERR_IPE) >> 24,
+   (IXGBE_RXDADV_ERR_TCPE | IXGBE_RXDADV_ERR_IPE) >> 24,
+   (IXGBE_RXDADV_ERR_TCPE | IXGBE_RXDADV_ERR_IPE) >> 24,
+   (IXGBE_RXDADV_ERR_TCPE | IXGBE_RXDADV_ERR_IPE) >> 24};
+
+   /* map vlan present (0x8), IPE (0x2), L4E (0x1) to ol_flags */
+   const uint8x16_t vlan_csum_map = {
+   0,
+   PKT_RX_L4_CKSUM_BAD,
+   PKT_RX_IP_CKSUM_BAD,
+   PKT_RX_IP_CKSUM_BAD | PKT_RX_L4_CKSUM_BAD,
0, 0, 0, 0,
-   0, 0, 0, 0,
-   vlan_flags, 0, 0, 0,
+   vlan_flags,
+   vlan_flags | PKT_RX_L4_CKSUM_BAD,
+   vlan_flags | PKT_RX_IP_CKSUM_BAD,
+   vlan_flags | PKT_RX_IP_CKSUM_BAD | PKT_RX_L4_CKSUM_BAD,
0, 0, 0, 0};
 
ptype = vzipq_u8(sterr_tmp1.val[0], sterr_tmp2.val[0]).val[0];
ptype = vandq_u8(ptype, rsstype_msk);
ptype = vqtbl1q_u8(rss_flags, ptype);
 
-   /* extract vlan_flags from IXGBE_RXD_STAT_VP bits of staterr */
-   vtag = vandq_u8(staterr, vlan_msk);
-   vtag = vqtbl1q_u8(vlan_map, vtag);
+   /* extract vlan_flags and csum_error from staterr */
+   vtag = vandq_u8(staterr, vlan_csum_msk);
+
+   /* csum bits are in the most significant, to use shuffle we need to
+* shift them. Change mask from 0xc0 to 0x03.
+*/
+   temp_csum = vshrq_n_u8(vtag, 6);
+
+   /* 'OR' the most significant 32 bits containing the checksum
+* flags with the vlan present flags
+* Then bits layout of each lane(8bits) will be ',VP,x,IPE,L4E'
+*/
+   csum = vsetq_lane_u32(vgetq_lane_u32(vreinterpretq_u32_u8(temp_csum), 
3), csum, 0);
+   vtag = vorrq_u8(vreinterpretq_u8_u32(csum), vtag);
+
+   /* convert VP, IPE, L4E to ol_flags */
+   vtag = vqtbl1q_u8(vlan_csum_map, vtag);
vtag = vorrq_u8(ptype, vtag);
 
vol.word = vgetq_lane_u32(vreinterpretq_u32_u8(vtag), 0);
@@ -391,7 +418,6 @@ _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq, struct 
rte_mbuf **rx_pkts,
  * Notice:
  * - nb_pkts < RTE_IXGBE_DESCS_PER_LOOP, just return no packet
  * - floor align nb_pkts to a RTE_IXGBE_DESC_PER_LOOP power-of-two
- * - don't support ol_flags for rss and csum err
  */
 uint16_t
 ixgbe_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
@@ -404,7 +430,6 @@ ixgbe_recv_pkts_vec(void *rx_queue, struct rte_mbuf 
**rx_pkts,
  * vPMD receive routine that reassembles scattered packets
  *
  * Notice:
- * - don't support ol_flags for rss and csum err
  * - nb_pkts < RTE_IXGBE_DESCS_PER_LOOP, just return no packet
  * - floor align nb_pkts to a RTE_IXGBE_DESC_PER_LOOP power-of-two
  */
-- 
2.17.1

[dpdk-dev] [PATCH v2 3/4] net/ixgbe: support good checksum flag for NEON vector

[Feifei Wang]

Add CKSUM_GOOD flag to distinguish a good checksum from an unknown one
in neon vertor RX function.

Signed-off-by: Feifei Wang 
Reviewed-by: Ruifeng Wang 
---
 drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c | 37 +
 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c 
b/drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c
index 4d6f057e7..b2bee2228 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c
@@ -86,13 +86,13 @@ desc_to_olflags_v(uint8x16x2_t sterr_tmp1, uint8x16x2_t 
sterr_tmp2,
  uint8x16_t staterr, uint8_t vlan_flags, struct rte_mbuf 
**rx_pkts)
 {
uint8x16_t ptype;
-   uint8x16_t vtag;
+   uint8x16_t vtag_lo, vtag_hi, vtag;
uint8x16_t temp_csum;
uint32x4_t csum = {0, 0, 0, 0};
 
union {
-   uint8_t e[4];
-   uint32_t word;
+   uint16_t e[4];
+   uint64_t word;
} vol;
 
const uint8x16_t rsstype_msk = {
@@ -119,18 +119,26 @@ desc_to_olflags_v(uint8x16x2_t sterr_tmp1, uint8x16x2_t 
sterr_tmp2,
(IXGBE_RXDADV_ERR_TCPE | IXGBE_RXDADV_ERR_IPE) >> 24};
 
/* map vlan present (0x8), IPE (0x2), L4E (0x1) to ol_flags */
-   const uint8x16_t vlan_csum_map = {
-   0,
-   PKT_RX_L4_CKSUM_BAD,
+   const uint8x16_t vlan_csum_map_lo = {
+   PKT_RX_IP_CKSUM_GOOD,
+   PKT_RX_IP_CKSUM_GOOD | PKT_RX_L4_CKSUM_BAD,
PKT_RX_IP_CKSUM_BAD,
PKT_RX_IP_CKSUM_BAD | PKT_RX_L4_CKSUM_BAD,
0, 0, 0, 0,
-   vlan_flags,
-   vlan_flags | PKT_RX_L4_CKSUM_BAD,
+   vlan_flags | PKT_RX_IP_CKSUM_GOOD,
+   vlan_flags | PKT_RX_IP_CKSUM_GOOD | PKT_RX_L4_CKSUM_BAD,
vlan_flags | PKT_RX_IP_CKSUM_BAD,
vlan_flags | PKT_RX_IP_CKSUM_BAD | PKT_RX_L4_CKSUM_BAD,
0, 0, 0, 0};
 
+   const uint8x16_t vlan_csum_map_hi = {
+   PKT_RX_L4_CKSUM_GOOD >> sizeof(uint8_t), 0,
+   PKT_RX_L4_CKSUM_GOOD >> sizeof(uint8_t), 0,
+   0, 0, 0, 0,
+   PKT_RX_L4_CKSUM_GOOD >> sizeof(uint8_t), 0,
+   PKT_RX_L4_CKSUM_GOOD >> sizeof(uint8_t), 0,
+   0, 0, 0, 0};
+
ptype = vzipq_u8(sterr_tmp1.val[0], sterr_tmp2.val[0]).val[0];
ptype = vandq_u8(ptype, rsstype_msk);
ptype = vqtbl1q_u8(rss_flags, ptype);
@@ -150,11 +158,16 @@ desc_to_olflags_v(uint8x16x2_t sterr_tmp1, uint8x16x2_t 
sterr_tmp2,
csum = vsetq_lane_u32(vgetq_lane_u32(vreinterpretq_u32_u8(temp_csum), 
3), csum, 0);
vtag = vorrq_u8(vreinterpretq_u8_u32(csum), vtag);
 
-   /* convert VP, IPE, L4E to ol_flags */
-   vtag = vqtbl1q_u8(vlan_csum_map, vtag);
-   vtag = vorrq_u8(ptype, vtag);
+   /* convert L4 checksum correct type to vtag_hi */
+   vtag_hi = vqtbl1q_u8(vlan_csum_map_hi, vtag);
+   vtag_hi = vshrq_n_u8(vtag_hi, 7);
+
+   /* convert VP, IPE, L4E to vtag_lo */
+   vtag_lo = vqtbl1q_u8(vlan_csum_map_lo, vtag);
+   vtag_lo = vorrq_u8(ptype, vtag_lo);
 
-   vol.word = vgetq_lane_u32(vreinterpretq_u32_u8(vtag), 0);
+   vtag = vzipq_u8(vtag_lo, vtag_hi).val[0];
+   vol.word = vgetq_lane_u64(vreinterpretq_u64_u8(vtag), 0);
 
rx_pkts[0]->ol_flags = vol.e[0];
rx_pkts[1]->ol_flags = vol.e[1];
-- 
2.17.1

[dpdk-dev] [PATCH v2 4/4] net/ixgbe: enable IXGBE NEON vector when need to checksum

[Feifei Wang]

IXGBE NEON vector PMD now supports checksum offloading, hence can be used
when DEV_RX_OFFLOAD_CHECKSUM is set.

Signed-off-by: Feifei Wang 
Reviewed-by: Ruifeng Wang 
---
 drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c 
b/drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c
index b2bee2228..a5a5b2167 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c
@@ -638,11 +638,5 @@ ixgbe_txq_vec_setup(struct ixgbe_tx_queue *txq)
 int __rte_cold
 ixgbe_rx_vec_dev_conf_condition_check(struct rte_eth_dev *dev)
 {
-   struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
-
-   /* no csum error report support */
-   if (rxmode->offloads & DEV_RX_OFFLOAD_CHECKSUM)
-   return -1;
-
return ixgbe_rx_vec_dev_conf_condition_check_default(dev);
 }
-- 
2.17.1

[dpdk-dev] 回复: [PATCH v1 1/4] net/ixgbe: add VLAN stripping support for Arm

[Feifei Wang]

Hi, Haiyue

> -邮件原件-
> 发件人: Wang, Haiyue 
> 发送时间: 2020年11月18日 16:34
> 收件人: Feifei Wang ; jer...@marvell.com; Ruifeng
> Wang ; Guo, Jia 
> 抄送: dev@dpdk.org; nd 
> 主题: RE: [PATCH v1 1/4] net/ixgbe: add VLAN stripping support for Arm
> 
> Hi Feifei,
> 
> > -Original Message-
> > From: Feifei Wang 
> > Sent: Wednesday, November 18, 2020 15:35
> > To: Jerin Jacob ; Ruifeng Wang
> > ; Guo, Jia ; Wang, Haiyue
> > 
> > Cc: dev@dpdk.org; n...@arm.com; Feifei Wang 
> > Subject: [PATCH v1 1/4] net/ixgbe: add VLAN stripping support for Arm
> >
> 
> And the commit message can be more descriptive like:
> http://git.dpdk.org/dpdk/commit/?id=6d28c53bf7a9063caa3197c6cc481e2a69e
> 3be96
> 
> No need to add the test result, you can put the result into cover letter.
> 
> BR,
> Haiyue

Thanks very much for your advice, the commit message has been changed in the 
new version.

Best Regards
Feifei
> 
> > Implemented PKT_RX_VLAN_STRIPPED for vector PMD on Arm platform.
> >
> > Test Results:
> > NICs: 82599(igb)
> > Driver: ixgbe(vector)
> > Packet: vlan-id=1
> >
> > $:./app/dpdk-testpmd -c 0x3 -w 0002:f9:00.0 -- -i
> > --port-topology=chained \ to enable vlan stripping:
> > --enable-hw-vlan-strip
> > test-pmd> set fwd rxonly
> > test-pmd> set verbose 1
> > test-pmd> start
> >
> > With this patch:
> > %enable vlan stripping:
> > testpmd> port 0/queue 0: received 1 packets
> > src=00:00:00:00:00:02 - dst=00:00:00:00:00:01 - type=0x0800 -
> > length=70 - nb_segs=1 - VLAN tci=0x1
> > ol_flags: PKT_RX_VLAN PKT_RX_VLAN_STRIPPED
> >
> > %disable vlan stripping:
> > testpmd> port 0/queue 0: received 1 packets
> > src=00:00:00:00:00:02 - dst=00:00:00:00:00:01 - type=0x8100 -
> > length=74 - nb_segs=1 - VLAN tci=0x0
> > ol_flags: PKT_RX_VLAN
> >
> > Without this patch:
> > %enable vlan stripping:
> > testpmd> port 0/queue 0: received 1 packets
> > src=00:00:00:00:00:02 - dst=00:00:00:00:00:01 - type=0x0800 -
> > length=70 - nb_segs=1 - VLAN tci=0x1
> > ol_flags: PKT_RX_VLAN
> >
> > %disable vlan stripping:
> > testpmd> port 0/queue 0: received 1 packets
> > src=00:00:00:00:00:02 - dst=00:00:00:00:00:01 - type=0x8100 -
> > length=74 - nb_segs=1 - VLAN tci=0x0
> > ol_flags: PKT_RX_VLAN
> >
> 
> 
> > Signed-off-by: Feifei Wang 
> > Reviewed-by: Ruifeng Wang 
> > ---
> >  drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c | 36
> > -
> >  1 file changed, 23 insertions(+), 13 deletions(-)
> >
> 
> 
> > 2.17.1

Re: [dpdk-dev] [PATCH v2 1/1] devtools: rename build test verbosity variables

[David Marchand]

On Tue, Nov 17, 2020 at 11:38 AM Thomas Monjalon  wrote:
>
> The verbosity was meant to be set with options -v and -vv,
> or possibly with the environment variables TEST_MESON_BUILD_VERBOSE
> and TEST_MESON_BUILD_VERY_VERBOSE.
>
> It is decided to keep only the options -v and -vv,
> so the variables are renamed with lower case, marking them as privates.
>
> The handling of the verbosity level is also moved upper in the script,
> closer to other initializations.
>
> Signed-off-by: Thomas Monjalon 
> ---
> v2: make the variables private to the script
> ---
>  devtools/test-meson-builds.sh | 38 ++-
>  1 file changed, 20 insertions(+), 18 deletions(-)
>
> diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh
> index 3ce49368cf..7280b7a93d 100755
> --- a/devtools/test-meson-builds.sh
> +++ b/devtools/test-meson-builds.sh
> @@ -43,6 +43,24 @@ default_cflags=$CFLAGS
>  default_ldflags=$LDFLAGS
>  default_meson_options=$DPDK_MESON_OPTIONS
>
> +opt_verbose=
> +opt_vverbose=
> +if [ "$1" = "-v" ] ; then
> +   opt_verbose=y
> +elif [ "$1" = "-vv" ] ; then
> +   opt_verbose=y
> +   opt_vverbose=y
> +fi
> +# we can't use plain verbose when we don't have pipefail option so up-level
> +if [ -z "$PIPEFAIL" -a -n "$opt_verbose" ] ; then
> +   echo "# Missing pipefail shell option, changing VERBOSE to 
> VERY_VERBOSE"
> +   opt_vverbose=y
> +fi
> +[ -n "$opt_verbose" ] && exec 8>&1 || exec 8>/dev/null
> +verbose=8
> +[ -n "$opt_vverbose" ] && exec 9>&1 || exec 9>/dev/null
> +veryverbose=9
> +
>  check_cc_flags () #   ...
>  {
> echo 'int main(void) { return 0; }' |
> @@ -108,11 +126,11 @@ config () #   
>  compile () # 
>  {
> builddir=$1
> -   if [ -n "$TEST_MESON_BUILD_VERY_VERBOSE" ] ; then
> +   if [ -n "$opt_vverbose" ] ; then
> # for full output from ninja use "-v"
> echo "$ninja_cmd -v -C $builddir"
> $ninja_cmd -v -C $builddir
> -   elif [ -n "$TEST_MESON_BUILD_VERBOSE" ] ; then
> +   elif [ -n "$opt_verbose" ] ; then
> # for keeping the history of short cmds, pipe through cat
> echo "$ninja_cmd -C $builddir | cat"
> $ninja_cmd -C $builddir | cat
> @@ -180,22 +198,6 @@ build () #   
> 
> fi
>  }
>
> -if [ "$1" = "-vv" ] ; then
> -   TEST_MESON_BUILD_VERY_VERBOSE=1
> -   TEST_MESON_BUILD_VERBOSE=1
> -elif [ "$1" = "-v" ] ; then
> -   TEST_MESON_BUILD_VERBOSE=1
> -fi
> -# we can't use plain verbose when we don't have pipefail option so up-level
> -if [ -z "$PIPEFAIL" -a -n "$TEST_MESON_BUILD_VERBOSE" ] ; then
> -   echo "# Missing pipefail shell option, changing VERBOSE to 
> VERY_VERBOSE"
> -   TEST_MESON_BUILD_VERY_VERBOSE=1
> -fi
> -[ -n "$TEST_MESON_BUILD_VERBOSE" ] && exec 8>&1 || exec 8>/dev/null
> -verbose=8
> -[ -n "$TEST_MESON_BUILD_VERY_VERBOSE" ] && exec 9>&1 || exec 9>/dev/null
> -veryverbose=9
> -
>  # shared and static linked builds with gcc and clang
>  for c in gcc clang ; do
> command -v $c >/dev/null 2>&1 || continue
> --
> 2.28.0
>

Reviewed-by: David Marchand 


-- 
David Marchand

[dpdk-dev] [PATCH v2 0/7] cppcheck

[Ferruh Yigit]

Fix a few of issues identified by cppcheck

Ferruh Yigit (7):
  app/procinfo: fix redundant condition
  app/procinfo: fix negative check on unsigned variable
  app/procinfo: remove suspicious sizeof
  app/procinfo: remove useless assignment
  net/pcap: remove local variable shadown outer one
  net/af_xdp: remove useless assignment
  net/bnxt: fix redundant return

 app/proc-info/main.c| 16 +---
 drivers/net/af_xdp/rte_eth_af_xdp.c |  1 -
 drivers/net/bnxt/tf_core/tf_em_common.c |  1 -
 drivers/net/pcap/rte_eth_pcap.c |  2 +-
 4 files changed, 6 insertions(+), 14 deletions(-)

-- 
2.26.2

[dpdk-dev] [PATCH v2 1/7] app/procinfo: fix redundant condition

[Ferruh Yigit]

'_filters' is compared twice, second one will be always false, removing
it using the message more relevant to the '_filters'.

Fixes: 2deb6b5246d7 ("app/procinfo: add collectd format and host id")
Cc: sta...@dpdk.org

Signed-off-by: Ferruh Yigit 
---
 app/proc-info/main.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index d743209f0d..35e5b596eb 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -420,11 +420,9 @@ static void collectd_resolve_cnt_type(char *cnt_type, 
size_t cnt_type_len,
} else if ((type_end != NULL) &&
   (strncmp(cnt_name, "flow_", strlen("flow_"))) == 0) {
if (strncmp(type_end, "_filters", strlen("_filters")) == 0)
-   strlcpy(cnt_type, "operations", cnt_type_len);
+   strlcpy(cnt_type, "filter_result", cnt_type_len);
else if (strncmp(type_end, "_errors", strlen("_errors")) == 0)
strlcpy(cnt_type, "errors", cnt_type_len);
-   else if (strncmp(type_end, "_filters", strlen("_filters")) == 0)
-   strlcpy(cnt_type, "filter_result", cnt_type_len);
} else if ((type_end != NULL) &&
   (strncmp(cnt_name, "mac_", strlen("mac_"))) == 0) {
if (strncmp(type_end, "_errors", strlen("_errors")) == 0)
-- 
2.26.2

[dpdk-dev] [PATCH v2 2/7] app/procinfo: fix negative check on unsigned variable

[Ferruh Yigit]

'parse_xstats_ids()' return 'int'. The return value is assigned to
'nb_xstats_ids' unsigned value, later negative check on this variable is
wrong.

Adding interim 'int' variable for negative check.

Fixes: 7ac16a3660c0 ("app/proc-info: support xstats by ID and by name")
Cc: sta...@dpdk.org

Signed-off-by: Ferruh Yigit 
---
 app/proc-info/main.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index 35e5b596eb..c11fe25af4 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -301,14 +301,13 @@ proc_info_parse_args(int argc, char **argv)
} else if (!strncmp(long_option[option_index].name,
"xstats-ids",
MAX_LONG_OPT_SZ))   {
-   nb_xstats_ids = parse_xstats_ids(optarg,
+   int ret = parse_xstats_ids(optarg,
xstats_ids, MAX_NB_XSTATS_IDS);
-
-   if (nb_xstats_ids <= 0) {
+   if (ret <= 0) {
printf("xstats-id list parse error.\n");
return -1;
}
-
+   nb_xstats_ids = (uint32_t)ret;
}
break;
default:
-- 
2.26.2

[dpdk-dev] [PATCH v2 3/7] app/procinfo: remove suspicious sizeof

[Ferruh Yigit]

The intention with the "sizeof(0)" usage is not clear, but the 'stats'
already 'memset' by 'rte_cryptodev_stats_get()' API, removing 'memset'
in application.

Fixes: fe773600fe3e ("app/procinfo: add --show-crypto")
Cc: sta...@dpdk.org

Signed-off-by: Ferruh Yigit 
---
 app/proc-info/main.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index c11fe25af4..dc5cc92209 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -1207,7 +1207,6 @@ show_crypto(void)
 
display_crypto_feature_info(dev_info.feature_flags);
 
-   memset(&stats, 0, sizeof(0));
if (rte_cryptodev_stats_get(i, &stats) == 0) {
printf("\t  -- stats\n");
printf("\t\t  + enqueue count (%"PRIu64")"
-- 
2.26.2

[dpdk-dev] [PATCH v2 4/7] app/procinfo: remove useless assignment

[Ferruh Yigit]

'flag' is initialized to '0' but it is overwritten later, moving the
declaration where it is used and initialize with actual value.

Fixes: 0101a0ec6217 ("app/procinfo: add --show-mempool")
Cc: sta...@dpdk.org

Signed-off-by: Ferruh Yigit 
---
 app/proc-info/main.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index dc5cc92209..b891622ccb 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -1264,8 +1264,6 @@ show_ring(char *name)
 static void
 show_mempool(char *name)
 {
-   uint64_t flags = 0;
-
snprintf(bdr_str, MAX_STRING_LEN, " show - MEMPOOL ");
STATS_BDR_STR(10, bdr_str);
 
@@ -1273,8 +1271,8 @@ show_mempool(char *name)
struct rte_mempool *ptr = rte_mempool_lookup(name);
if (ptr != NULL) {
struct rte_mempool_ops *ops;
+   uint64_t flags = ptr->flags;
 
-   flags = ptr->flags;
ops = rte_mempool_get_ops(ptr->ops_index);
printf("  - Name: %s on socket %d\n"
"  - flags:\n"
-- 
2.26.2

[dpdk-dev] [PATCH v2 5/7] net/pcap: remove local variable shadown outer one

[Ferruh Yigit]

'ret' is already defined in the function scope, removing the 'ret' in
the block scope.

Fixes: c9507cd0cada ("net/pcap: support physical interface MAC address")
Cc: sta...@dpdk.org

Signed-off-by: Ferruh Yigit 
---
Cc: juhamatti.kuusisa...@coriant.com
---
 drivers/net/pcap/rte_eth_pcap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 4930d7d382..dd9ef33c85 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -1324,7 +1324,7 @@ eth_from_pcaps(struct rte_vdev_device *vdev,
 
/* phy_mac arg is applied only only if "iface" devarg is 
provided */
if (rx_queues->phy_mac) {
-   int ret = eth_pcap_update_mac(rx_queues->queue[0].name,
+   ret = eth_pcap_update_mac(rx_queues->queue[0].name,
eth_dev, vdev->device.numa_node);
if (ret == 0)
internals->phy_mac = 1;
-- 
2.26.2

[dpdk-dev] [PATCH v2 6/7] net/af_xdp: remove useless assignment

[Ferruh Yigit]

Assignment of function parameter 'umem' removed.

Fixes: f0ce7af0e182 ("net/af_xdp: remove resources when port is closed")
Cc: sta...@dpdk.org

Signed-off-by: Ferruh Yigit 
---
 drivers/net/af_xdp/rte_eth_af_xdp.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c 
b/drivers/net/af_xdp/rte_eth_af_xdp.c
index 2c7892bd7e..7fc70df713 100644
--- a/drivers/net/af_xdp/rte_eth_af_xdp.c
+++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
@@ -840,7 +840,6 @@ xdp_umem_destroy(struct xsk_umem_info *umem)
 #endif
 
rte_free(umem);
-   umem = NULL;
 }
 
 static int
-- 
2.26.2

[dpdk-dev] [PATCH v2 7/7] net/bnxt: fix redundant return

[Ferruh Yigit]

Removing useless 'return' statement.

Fixes: b2da02480cb7 ("net/bnxt: support EEM system memory")
Cc: sta...@dpdk.org

Signed-off-by: Ferruh Yigit 
---
Cc: peter.spreadboro...@broadcom.com
---
 drivers/net/bnxt/tf_core/tf_em_common.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/bnxt/tf_core/tf_em_common.c 
b/drivers/net/bnxt/tf_core/tf_em_common.c
index ad92cbdc75..c96c21c2e9 100644
--- a/drivers/net/bnxt/tf_core/tf_em_common.c
+++ b/drivers/net/bnxt/tf_core/tf_em_common.c
@@ -307,7 +307,6 @@ tf_em_page_tbl_pgcnt(uint32_t num_pages,
 {
return roundup(num_pages, MAX_PAGE_PTRS(page_size)) /
   MAX_PAGE_PTRS(page_size);
-   return 0;
 }
 
 /**
-- 
2.26.2

Re: [dpdk-dev] [PATCH v2 1/4] net/ixgbe: add new flag of stripped VLAN for NEON vector

[Wang, Haiyue]

> -Original Message-
> From: Feifei Wang 
> Sent: Wednesday, November 18, 2020 18:49
> To: Jerin Jacob ; Ruifeng Wang ; 
> Guo, Jia
> ; Wang, Haiyue 
> Cc: dev@dpdk.org; n...@arm.com; Feifei Wang 
> Subject: [PATCH v2 1/4] net/ixgbe: add new flag of stripped VLAN for NEON 
> vector
> 
> For NEON vector of IXGBE PMD, introduce new flag PKT_RX_VLAN_STRIPPED to
> show the case that the VLAN is stripped from the VLAN tagged packet.
> 
> This is because that the old flag PKT_RX_VLAN_PKT only indicates that the
> packet is VLAN tagged, but cannot show whether VLAN is in m->vlan_tci or
> in the packet at present. So add new flag to show the vlan has been
> stripped by the hardware and its tci is saved in m->vlan_tci when vlan
> stripping is enabled in the RX configuration of the IXGBE PMD.
> 
> Signed-off-by: Feifei Wang 
> Reviewed-by: Ruifeng Wang 
> ---
>  drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c | 36 -
>  1 file changed, 23 insertions(+), 13 deletions(-)
> 


Acked-by: Haiyue Wang 


> --
> 2.17.1

Re: [dpdk-dev] [PATCH v2 2/4] net/ixgbe: support bad checksum flag for NEON vector

[Wang, Haiyue]

> -Original Message-
> From: Feifei Wang 
> Sent: Wednesday, November 18, 2020 18:49
> To: Jerin Jacob ; Ruifeng Wang ; 
> Guo, Jia
> ; Wang, Haiyue 
> Cc: dev@dpdk.org; n...@arm.com; Feifei Wang 
> Subject: [PATCH v2 2/4] net/ixgbe: support bad checksum flag for NEON vector
> 
> Updated desc_to_olflags_v() to support PKT_RX_IP_CKSUM_BAD and
> PKT_RX_L4_CKSUM_BAD in the ol_flags of the mbuf.
> 
> And then the NEON vector RX function can be called with hw_ip_checksum
> enabled.
> 
> Signed-off-by: Feifei Wang 
> Reviewed-by: Ruifeng Wang 
> ---
>  drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c | 47 +++--
>  1 file changed, 36 insertions(+), 11 deletions(-)

Acked-by: Haiyue Wang 

> --
> 2.17.1

Re: [dpdk-dev] [PATCH v2 3/4] net/ixgbe: support good checksum flag for NEON vector

[Wang, Haiyue]

> -Original Message-
> From: Feifei Wang 
> Sent: Wednesday, November 18, 2020 18:49
> To: Jerin Jacob ; Ruifeng Wang ; 
> Guo, Jia
> ; Wang, Haiyue 
> Cc: dev@dpdk.org; n...@arm.com; Feifei Wang 
> Subject: [PATCH v2 3/4] net/ixgbe: support good checksum flag for NEON vector
> 
> Add CKSUM_GOOD flag to distinguish a good checksum from an unknown one
> in neon vertor RX function.
> 
> Signed-off-by: Feifei Wang 
> Reviewed-by: Ruifeng Wang 
> ---
>  drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c | 37 +
>  1 file changed, 25 insertions(+), 12 deletions(-)
> 

Acked-by: Haiyue Wang 

> --
> 2.17.1

Re: [dpdk-dev] [PATCH v2 4/4] net/ixgbe: enable IXGBE NEON vector when need to checksum

[Wang, Haiyue]

> -Original Message-
> From: Feifei Wang 
> Sent: Wednesday, November 18, 2020 18:49
> To: Jerin Jacob ; Ruifeng Wang ; 
> Guo, Jia
> ; Wang, Haiyue 
> Cc: dev@dpdk.org; n...@arm.com; Feifei Wang 
> Subject: [PATCH v2 4/4] net/ixgbe: enable IXGBE NEON vector when need to 
> checksum
> 
> IXGBE NEON vector PMD now supports checksum offloading, hence can be used
> when DEV_RX_OFFLOAD_CHECKSUM is set.
> 
> Signed-off-by: Feifei Wang 
> Reviewed-by: Ruifeng Wang 
> ---
>  drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c | 6 --
>  1 file changed, 6 deletions(-)

Acked-by: Haiyue Wang 

> --
> 2.17.1

[dpdk-dev] [PATCH] doc: update ice user guide

[Qi Zhang]

Add link for firmware/OOT kernel driver/DDP download
Add matching List.

Signed-off-by: Qi Zhang 
---
 doc/guides/nics/ice.rst | 28 +---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/doc/guides/nics/ice.rst b/doc/guides/nics/ice.rst
index c5a76a2a21..6e28b20cac 100644
--- a/doc/guides/nics/ice.rst
+++ b/doc/guides/nics/ice.rst
@@ -5,9 +5,8 @@ ICE Poll Mode Driver
 ==
 
 The ice PMD (**librte_net_ice**) provides poll mode driver support for
-10/25/50/100 Gbps Intel® Ethernet 810 Series Network Adapters based on
-the Intel Ethernet Controller E810.
-
+10/25/50/100 Gbps Intel® Ethernet 800 Series Network Adapters based on
+the Intel Ethernet Controller E810 and Intel Ethernet Connection E822/E823.
 
 Prerequisites
 -
@@ -17,6 +16,29 @@ Prerequisites
 - To get better performance on Intel platforms, please follow the "How to get 
best performance with NICs on Intel platforms"
   section of the :ref:`Getting Started Guide for Linux `.
 
+- Please follow the matching list to download specific kernel driver, firmware 
and DDP package from
+  
`https://www.intel.com/content/www/us/en/search.html?ws=text#q=e810&t=Downloads&layout=table`.
+
+- To understand what is DDP package and how it works, please review `Intel® 
Ethernet Controller E810 Dynamic
+  Device Personalization (DDP) for Telecommunications Technology Guide 
`_.
+
+- To understand DDP for COMMs usage with DPDK, please review `Intel® Ethernet 
800 Series Telecommunication (Comms)
+  Dynamic Device Personalization (DDP) Package 
`_.
+
+
+Recommended Matching List
+-
+
+It is highly recommended to upgrade the ice kernel driver, firmware and DDP 
package
+to avoid the compatibility issues with ice PMD. Here is the suggested matching 
list which has been tested and verified.
+The detailed information can refer to chapter Tested Platforms/Tested NICs in 
release notes.
+
+   +---+---+-+---+---+
+   |DPDK   | Kernel Driver | OS Default DDP  | COMMS DDP | Firmware  |
+   +===+===+=+===+===+
+   |20.11  | 1.3.0 |  1.3.19 |  1.3.23   |2.3|
+   +---+---+-+---+---+
+
 Pre-Installation Configuration
 --
 
-- 
2.26.2

Re: [dpdk-dev] [PATCH v2 2/2] app/testpmd: support extended RSS offload types

[Ferruh Yigit]

On 11/18/2020 1:34 AM, Simei Su wrote:

This patch adds testpmd cmdline support for eCPRI.

Signed-off-by: Simei Su 


Reviewed-by: Ferruh Yigit 

Re: [dpdk-dev] [PATCH v2 0/2] extend RSS offload types

[Ferruh Yigit]

On 11/18/2020 1:34 AM, Simei Su wrote:

[PATCH v2 1/2] ethdev: add RSS offload types.
[PATCH v2 2/2] app/testpmd: add cmdline support for RSS types.

v2:
* Update cmd_config_rss.help_str and cmd_help_long_parsed() to add the ecpri.
* Update the documentation to add the ecpri.

Simei Su (2):
   ethdev: add eCPRI RSS offload types
   app/testpmd: support extended RSS offload types



Series applied to dpdk-next-net/main, thanks.

Re: [dpdk-dev] [PATCH v2] net/af_xdp: document 32-bit OS kernel requirement

[Ferruh Yigit]

On 11/17/2020 2:07 PM, Ciara Loftus wrote:

AF_XDP will not work on 32-bit kernels before version 5.4.
Document this restriction in the driver guide.

Signed-off-by: Ciara Loftus 


Applied to dpdk-next-net/main, thanks.

Re: [dpdk-dev] [PATCH v12 09/14] build: optional NUMA and cpu counts detection

[Juraj Linkeš]

> -Original Message-
> From: Thomas Monjalon 
> Sent: Monday, November 16, 2020 8:38 AM
> To: Juraj Linkeš ;
> honnappa.nagaraha...@arm.com
> Cc: dev@dpdk.org; bruce.richard...@intel.com; ruifeng.w...@arm.com;
> phil.y...@arm.com; vcchu...@amazon.com; dharmik.thak...@arm.com;
> jerinjac...@gmail.com; hemant.agra...@nxp.com;
> ajit.khapa...@broadcom.com; ferruh.yi...@intel.com;
> david.march...@redhat.com
> Subject: Re: [dpdk-dev] [PATCH v12 09/14] build: optional NUMA and cpu counts
> detection
> 
> 14/11/2020 14:16, Thomas Monjalon:
> > I didn't think this series is changing options for all archs.
> > It was supposed to be an Arm-only rework.
> > Clearly it is too late for such change in 20.11.
> 
> I recommend splitting the series for 21.02.
> Do not change global stuff in a series named "Arm build options rework".
> It will be easier to absorb as smaller items. Step by step.
> 
> 

Yes, this makes sense. This commit first started with arm specific 
parts/considerations, went through a lot of discussion and ended up here. I'll 
send it separately.

[dpdk-dev] [PATCH] ip_frag: recalculate data length of fragment

[Yicai Lu]

In some situations, we would get several ip fragments， which total
data length is less than minimum frame(64) and padding with zeros.
Examples: Second Fragment is "a0a1 a2a3 a4a5 a6a7   ..."
and Third Fragment is "a8a9 aaab acad aeaf b0b1 b2b3 ...".
And then, we would reassemble Second and Third Fragment like this
"a0a1 a2a3 a4a5 a6a7   ... a8a9 aaab acad aeaf b0b1 ...",
which is not correct! So, we need recalculate the data length 
of fragment to remove invalid padings(zero)!

Signed-off-by: Yicai Lu 
---
 lib/librte_ip_frag/ip_frag_internal.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/librte_ip_frag/ip_frag_internal.c 
b/lib/librte_ip_frag/ip_frag_internal.c
index b436a4c93..7bdd98090 100644
--- a/lib/librte_ip_frag/ip_frag_internal.c
+++ b/lib/librte_ip_frag/ip_frag_internal.c
@@ -155,6 +155,7 @@ ip_frag_process(struct ip_frag_pkt *fp, struct 
rte_ip_frag_death_row *dr,
 
fp->frags[idx].ofs = ofs;
fp->frags[idx].len = len;
+   mb->data_len = mb->l2_len + mb->l3_len + len;
fp->frags[idx].mb = mb;
 
mb = NULL;
-- 
2.28.0.windows.1

Re: [dpdk-dev] [PATCH v3] net/mlx5: fix header reformat action hash key

[Raslan Darawsheh]

Hi,

> -Original Message-
> From: Suanming Mou 
> Sent: Wednesday, November 18, 2020 4:20 AM
> To: Slava Ovsiienko ; Matan Azrad
> 
> Cc: dev@dpdk.org; Raslan Darawsheh 
> Subject: [PATCH v3] net/mlx5: fix header reformat action hash key
> 
> Currently, header reformat action uses the hash list 32-bit key generated
> in header reformat register function directly. The key will not be
> recalculated in the hash list function.
> 
> As the 64-bit key is composed of the 32-bit attributes and 32-bit reformat
> buffer csum, the hash list function only gets 32-bit key directly will take
> the attribute part only, csum part will be ignored. For different header
> reformat actions, the attributes can be the same, while the buffer will
> be different. Only take the attribute part causes lots of the conflicts.
> 
> This commits adds the attribute part and the significant different csum
> part for the key.
> 
> Fixes: f961fd490fd4 ("net/mlx5: make header reformat action thread safe")
> 
> Signed-off-by: Suanming Mou 
> Acked-by: Matan Azrad 
> ---
> 
> v3:
>  - add the missing ack.
> 
> v2:
>  - update key generate.
> 
> ---
>  drivers/net/mlx5/mlx5_flow.h| 12 
>  drivers/net/mlx5/mlx5_flow_dv.c | 31 ---
>  2 files changed, 24 insertions(+), 19 deletions(-)
> 

Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh

Re: [dpdk-dev] [PATCH] net/mlx5: fix raw encap/decap limit constant

[Raslan Darawsheh]

Hi,

> -Original Message-
> From: Viacheslav Ovsiienko 
> Sent: Wednesday, November 18, 2020 9:38 AM
> To: dev@dpdk.org
> Cc: Raslan Darawsheh ; Matan Azrad
> ; Ori Kam ; Suanming Mou
> ; sta...@dpdk.org
> Subject: [PATCH] net/mlx5: fix raw encap/decap limit constant
> 
> The MLX5_ENCAPSULATION_DECISION_SIZE constant is used
> to check the raw encap/decap actions for the raw header
> size. The header is constructed of the rte_xxx_hdr
> structures instead of rte items. Hence, constant
> must be defined with rte_xxx_hdr structure sizes.
> 
> Fixes: 50f576d657d7 ("net/mlx5: fix VLAN actions in meter")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Viacheslav Ovsiienko 
> Acked-by: Ori Kam 
> Acked-by: Suanming Mou 
> ---
>  drivers/net/mlx5/mlx5_flow.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh

Re: [dpdk-dev] [PATCH v2 1/7] app/procinfo: fix redundant condition

[David Marchand]

On Wed, Nov 18, 2020 at 12:46 PM Ferruh Yigit  wrote:
>
> '_filters' is compared twice, second one will be always false, removing
> it using the message more relevant to the '_filters'.
>
> Fixes: 2deb6b5246d7 ("app/procinfo: add collectd format and host id")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Ferruh Yigit 
> ---
>  app/proc-info/main.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/app/proc-info/main.c b/app/proc-info/main.c
> index d743209f0d..35e5b596eb 100644
> --- a/app/proc-info/main.c
> +++ b/app/proc-info/main.c
> @@ -420,11 +420,9 @@ static void collectd_resolve_cnt_type(char *cnt_type, 
> size_t cnt_type_len,
> } else if ((type_end != NULL) &&
>(strncmp(cnt_name, "flow_", strlen("flow_"))) == 0) {
> if (strncmp(type_end, "_filters", strlen("_filters")) == 0)
> -   strlcpy(cnt_type, "operations", cnt_type_len);
> +   strlcpy(cnt_type, "filter_result", cnt_type_len);

Do you know what impact this change of type could have?


-- 
David Marchand

Re: [dpdk-dev] [dpdk-stable] [PATCH v2 2/7] app/procinfo: fix negative check on unsigned variable

[David Marchand]

On Wed, Nov 18, 2020 at 12:46 PM Ferruh Yigit  wrote:
>
> 'parse_xstats_ids()' return 'int'. The return value is assigned to
> 'nb_xstats_ids' unsigned value, later negative check on this variable is
> wrong.
>
> Adding interim 'int' variable for negative check.
>
> Fixes: 7ac16a3660c0 ("app/proc-info: support xstats by ID and by name")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Ferruh Yigit 
> ---
>  app/proc-info/main.c | 7 +++
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/app/proc-info/main.c b/app/proc-info/main.c
> index 35e5b596eb..c11fe25af4 100644
> --- a/app/proc-info/main.c
> +++ b/app/proc-info/main.c
> @@ -301,14 +301,13 @@ proc_info_parse_args(int argc, char **argv)
> } else if (!strncmp(long_option[option_index].name,
> "xstats-ids",
> MAX_LONG_OPT_SZ))   {
> -   nb_xstats_ids = parse_xstats_ids(optarg,
> +   int ret = parse_xstats_ids(optarg,
> xstats_ids, 
> MAX_NB_XSTATS_IDS);
> -
> -   if (nb_xstats_ids <= 0) {
> +   if (ret <= 0) {
> printf("xstats-id list parse 
> error.\n");
> return -1;
> }
> -
> +   nb_xstats_ids = (uint32_t)ret;

The cast is unneeded.

Reviewed-by: David Marchand 


-- 
David Marchand

Re: [dpdk-dev] [dpdk-stable] [PATCH v2 3/7] app/procinfo: remove suspicious sizeof

[David Marchand]

On Wed, Nov 18, 2020 at 12:46 PM Ferruh Yigit  wrote:
>
> The intention with the "sizeof(0)" usage is not clear, but the 'stats'
> already 'memset' by 'rte_cryptodev_stats_get()' API, removing 'memset'
> in application.
>
> Fixes: fe773600fe3e ("app/procinfo: add --show-crypto")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Ferruh Yigit 
> ---
>  app/proc-info/main.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/app/proc-info/main.c b/app/proc-info/main.c
> index c11fe25af4..dc5cc92209 100644
> --- a/app/proc-info/main.c
> +++ b/app/proc-info/main.c
> @@ -1207,7 +1207,6 @@ show_crypto(void)
>
> display_crypto_feature_info(dev_info.feature_flags);
>
> -   memset(&stats, 0, sizeof(0));
> if (rte_cryptodev_stats_get(i, &stats) == 0) {
> printf("\t  -- stats\n");
> printf("\t\t  + enqueue count (%"PRIu64")"
> --
> 2.26.2
>

Reviewed-by: David Marchand 


-- 
David Marchand

Re: [dpdk-dev] [PATCH v2 4/7] app/procinfo: remove useless assignment

[David Marchand]

On Wed, Nov 18, 2020 at 12:47 PM Ferruh Yigit  wrote:
>
> 'flag' is initialized to '0' but it is overwritten later, moving the
> declaration where it is used and initialize with actual value.
>
> Fixes: 0101a0ec6217 ("app/procinfo: add --show-mempool")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Ferruh Yigit 
> ---
>  app/proc-info/main.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/app/proc-info/main.c b/app/proc-info/main.c
> index dc5cc92209..b891622ccb 100644
> --- a/app/proc-info/main.c
> +++ b/app/proc-info/main.c
> @@ -1264,8 +1264,6 @@ show_ring(char *name)
>  static void
>  show_mempool(char *name)
>  {
> -   uint64_t flags = 0;
> -
> snprintf(bdr_str, MAX_STRING_LEN, " show - MEMPOOL ");
> STATS_BDR_STR(10, bdr_str);
>
> @@ -1273,8 +1271,8 @@ show_mempool(char *name)
> struct rte_mempool *ptr = rte_mempool_lookup(name);
> if (ptr != NULL) {
> struct rte_mempool_ops *ops;
> +   uint64_t flags = ptr->flags;

Do we really need a temp storage?

But otherwise,
Reviewed-by: David Marchand 

>
> -   flags = ptr->flags;
> ops = rte_mempool_get_ops(ptr->ops_index);
> printf("  - Name: %s on socket %d\n"
> "  - flags:\n"
> --
> 2.26.2
>

-- 
David Marchand

Re: [dpdk-dev] [PATCH v2 5/7] net/pcap: remove local variable shadown outer one

[David Marchand]

On Wed, Nov 18, 2020 at 12:47 PM Ferruh Yigit  wrote:
>
> 'ret' is already defined in the function scope, removing the 'ret' in
> the block scope.
>
> Fixes: c9507cd0cada ("net/pcap: support physical interface MAC address")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Ferruh Yigit 
> ---
> Cc: juhamatti.kuusisa...@coriant.com
> ---
>  drivers/net/pcap/rte_eth_pcap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
> index 4930d7d382..dd9ef33c85 100644
> --- a/drivers/net/pcap/rte_eth_pcap.c
> +++ b/drivers/net/pcap/rte_eth_pcap.c
> @@ -1324,7 +1324,7 @@ eth_from_pcaps(struct rte_vdev_device *vdev,
>
> /* phy_mac arg is applied only only if "iface" devarg is 
> provided */
> if (rx_queues->phy_mac) {
> -   int ret = 
> eth_pcap_update_mac(rx_queues->queue[0].name,
> +   ret = eth_pcap_update_mac(rx_queues->queue[0].name,
> eth_dev, vdev->device.numa_node);
> if (ret == 0)
> internals->phy_mac = 1;

It is not used after, why not simply check eth_pcap_update_mac() == 0 ?


-- 
David Marchand

Re: [dpdk-dev] [PATCH v12 09/14] build: optional NUMA and cpu counts detection

[Juraj Linkeš]

> -Original Message-
> From: Thomas Monjalon 
> Sent: Monday, November 16, 2020 10:24 AM
> To: Bruce Richardson 
> Cc: Juraj Linkeš ; ruifeng.w...@arm.com;
> honnappa.nagaraha...@arm.com; phil.y...@arm.com;
> vcchu...@amazon.com; dharmik.thak...@arm.com; jerinjac...@gmail.com;
> hemant.agra...@nxp.com; ajit.khapa...@broadcom.com;
> ferruh.yi...@intel.com; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v12 09/14] build: optional NUMA and cpu counts
> detection
> 
> 16/11/2020 10:13, Bruce Richardson:
> > On Mon, Nov 16, 2020 at 08:24:48AM +0100, Thomas Monjalon wrote:
> > > 13/11/2020 15:31, Juraj Linkeš:
> > > > +option('max_lcores', type: 'integer', value: 0,
> > > > +   description: 'maximum number of cores/threads supported by EAL.
> > > > +Set to positive integer to overwrite per-arch or cross-compilation
> defaults. Set to -1 to detect the number of cores on the build machine.')
> option('max_numa_nodes', type: 'integer', value: 0,
> > > > +   description: 'maximum number of NUMA nodes supported by EAL. Set
> > > > +to positive integer to overwrite per-arch or cross-compilation
> > > > +defaults. Set to -1 to detect the number of numa nodes on the
> > > > +build machine.')
> > >
> > > First comment: I don't like having so long description.
> > > Second: I don't understand.
> > >
> > > It is said the default value is 0 so I expect it means automatic 
> > > detection.
> > > But later it is said -1 is for detection. So ?
> > >
> > Zero is for the "per-arch or cross-compilation default". This was
> > discussed quite a bit in previous versions and this was te best
> > compromise we could come up with. Having a default of auto-detect is
> > definitely not something I think we should go with - just thinking of
> > all the build CI jobs running on
> > 2 or 4 core VMs! However, Juraj really felt there was value in having
> > auto-detection, so it's set as a -1 value, which I'm ok with.
> 
> The problem is that I don't understand what 0 means.
> 

There are three pieces of information which we need to convey:
1. The default value (0) indicates that per-arch or cross-compilation defaults 
will be used.
2. Positive integer values will be used instead of these defaults.
3. Detected values will be used for native build when the value is -1.

The description is a bit longer because of this and I've already tried to 
shorten it by combining 1 and 2 into one sentence. But maybe we can shorten it 
while making it a bit clearer. I'll think about it.

> > As I flagged in a previous mail thread, the whole config of DPDK
> > builds is something we need to take a bigger look at in 21.02 and beyond.
> 
> Yes definitely.
> 
> 

Re: [dpdk-dev] [PATCH v12 09/14] build: optional NUMA and cpu counts detection

[Juraj Linkeš]

> -Original Message-
> From: Thomas Monjalon 
> Sent: Monday, November 16, 2020 8:20 AM
> To: Juraj Linkeš 
> Cc: bruce.richard...@intel.com; ruifeng.w...@arm.com;
> honnappa.nagaraha...@arm.com; phil.y...@arm.com;
> vcchu...@amazon.com; dharmik.thak...@arm.com; jerinjac...@gmail.com;
> hemant.agra...@nxp.com; ajit.khapa...@broadcom.com;
> ferruh.yi...@intel.com; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v12 09/14] build: optional NUMA and cpu counts
> detection
> 
> 16/11/2020 08:15, Juraj Linkeš:
> > From: Thomas Monjalon 
> > > 13/11/2020 15:31, Juraj Linkeš:
> > > > Add an option to automatically discover the host's numa and cpu
> > > > counts and use those values for a non cross-build.
> > > > Give users the option to override the per-arch default values or
> > > > values from cross files by specifying them on the command line
> > > > with -Dmax_lcores and -Dmax_numa_nodes.
> > > >
> > > > Signed-off-by: Juraj Linkeš 
> > > > Reviewed-by: Honnappa Nagarahalli 
> > > [...]
> > > >  create mode 100644 buildtools/get_cpu_count.py  create mode
> > > > 100644 buildtools/get_numa_count.py
> > >
> > > These new files should be added in the file MAINTAINERS.
> >
> > Should I add a new section? I just add them under Build System, making
> > Bruce the maintainer of these?
> 
> If Bruce is OK, it looks fine.
> 

Bruce, may I add the files to MAINTAINERS under your name?

Re: [dpdk-dev] [PATCH v12 01/14] build: alias default build as generic

[Juraj Linkeš]

> -Original Message-
> From: Thomas Monjalon 
> Sent: Tuesday, November 17, 2020 10:58 AM
> To: Bruce Richardson 
> Cc: Juraj Linkeš ; Honnappa Nagarahalli
> ; 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; dev@dpdk.org; david.march...@redhat.com; nd
> 
> Subject: Re: [dpdk-dev] [PATCH v12 01/14] build: alias default build as 
> generic
> 
> 17/11/2020 10:15, Bruce Richardson:
> > On Tue, Nov 17, 2020 at 08:49:45AM +0100, Thomas Monjalon wrote:
> > > 17/11/2020 03:46, Honnappa Nagarahalli:
> > > > 
> > > >
> > > > >
> > > > > 16/11/2020 17:16, Bruce Richardson:
> > > > > > On Mon, Nov 16, 2020 at 03:50:31PM +, Juraj Linkeš wrote:
> > > > > > > From: Thomas Monjalon 
> > > > > > > > 13/11/2020 15:31, Juraj Linkeš:
> > > > > > > > > The current machine='default' build name is not
> > > > > > > > > descriptive. The actual default build is
> > > > > > > > > machine='native'. Add an alternative string which does
> > > > > > > > > the same build and better describes what we're
> > > > > building:
> > > > > > > > > machine='generic'. Leave machine='default' for backwards
> > > > > compatibility.
> > > > > > > >
> > > > > > > > What?
> > > > > > > >
> > > > > > > > "generic" means... nothing.
> > > > > > > >
> > > > > > >
> > > > > > > An absence of anything means nothing. Generic means
> > > > > > > "characteristic of
> > > > > or relating to a class or group of things; not specific", which
> > > > > is pretty much what we're looking for.
> > > > > > >
> > > > > > > > "default" should be the most common set of options to make
> > > > > > > > a build work everywhere.
> > > > > > >
> > > > > > > What we want is a value of machine that would "be the most
> > > > > > > common
> > > > > set of options to make a build work everywhere" and using the
> > > > > above definition of generic, it fits very well.
> > > > > > > The reason I said the actual default build is
> > > > > > > machine='native' is because
> > > > > that's how the machine option is defined in meson_options.txt.
> > > > > It follows from what default actually means - "a preselected
> > > > > option adopted by a computer program or other mechanism when no
> > > > > alternative is specified by the user or programmer". Default
> > > > > then means no user input, which means machine='native', which means
> the default build is the default build.
> > > > > > >
> > > > > > > What ""default" should mean" looks like an attempt at
> > > > > > > redefining what
> > > > > the word actually means and leads to confusion, in my
> > > > > experience. Hence an attempt to remove the potential ambiguity.
> > > > > > >
> > > > > >
> > > > > > I would tend to agree that "generic" is probably a better term
> > > > > > than "default" for what we use it for here in the config.
> > > > >
> > > > > In the past, we had a different definition with make config.
> > > > > I am just trying to be consistent.
> > > > > Even with meson, default means "minimal CPU instructions".
> > > > >
> > > > > Example in devtools/test-meson-builds.sh:
> > > > > "test compilation with minimal x86 instruction set"
> > > > > is called build-x86-default.
> > > > >
> > > > > In config/meson.build:
> > > > > "
> > > > > machine type 'default' is special, it defaults to the per arch
> > > > > agreed common minimal baseline needed for DPDK.
> > > > > That might not be the most optimized, but the most portable
> > > > > version while still being able to support the CPU features required 
> > > > > for
> DPDK.
> > > > > This can be bumped up by the DPDK project, but it can never be
> > > > > an invariant like 'native'
> > > > > "
> > > > >
> > > > > So, why this definition is called "generic" in meson Arm config?
> > > > The explanation above is for a build type 'default'. Whereas meson by
> default builds for build type 'native'. Also when you look at the
> config/arm/meson.build the word 'default' was used where it was not related to
> the build type default. It created lot of confusion.
> > > >
> > > > From the dictionary 'default' - "a preselected option adopted by a
> computer program or other mechanism when no alternative is specified by the
> user or programmer." But, if one had to do build of type default, they have to
> mention -Dmachine=default. If nothing is mentioned, it is a build type 
> 'native',
> which does not go along with the definition of 'default'.
> > > >
> > > > But for 'generic' - "characteristic of or relating to a class or group 
> > > > of things;
> not specific". IMO, it better suits the explanation you have provided above. 
> So,
> separating this machine type to 'generic' to cover the same definition makes
> more sense.
> > > >
> > > > However, 'default' is still supported for backward compatibility.
> > >
> > > So? Are you going to change the DPDK definitions we had for years?
> > >
> >
> > I think we should, or at least 

Re: [dpdk-dev] [PATCH v12 09/14] build: optional NUMA and cpu counts detection

[Bruce Richardson]

On Wed, Nov 18, 2020 at 02:20:53PM +, Juraj Linkeš wrote:
> 
> 
> > -Original Message-
> > From: Thomas Monjalon 
> > Sent: Monday, November 16, 2020 8:20 AM
> > To: Juraj Linkeš 
> > Cc: bruce.richard...@intel.com; ruifeng.w...@arm.com;
> > honnappa.nagaraha...@arm.com; phil.y...@arm.com;
> > vcchu...@amazon.com; dharmik.thak...@arm.com; jerinjac...@gmail.com;
> > hemant.agra...@nxp.com; ajit.khapa...@broadcom.com;
> > ferruh.yi...@intel.com; dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v12 09/14] build: optional NUMA and cpu 
> > counts
> > detection
> > 
> > 16/11/2020 08:15, Juraj Linkeš:
> > > From: Thomas Monjalon 
> > > > 13/11/2020 15:31, Juraj Linkeš:
> > > > > Add an option to automatically discover the host's numa and cpu
> > > > > counts and use those values for a non cross-build.
> > > > > Give users the option to override the per-arch default values or
> > > > > values from cross files by specifying them on the command line
> > > > > with -Dmax_lcores and -Dmax_numa_nodes.
> > > > >
> > > > > Signed-off-by: Juraj Linkeš 
> > > > > Reviewed-by: Honnappa Nagarahalli 
> > > > [...]
> > > > >  create mode 100644 buildtools/get_cpu_count.py  create mode
> > > > > 100644 buildtools/get_numa_count.py
> > > >
> > > > These new files should be added in the file MAINTAINERS.
> > >
> > > Should I add a new section? I just add them under Build System, making
> > > Bruce the maintainer of these?
> > 
> > If Bruce is OK, it looks fine.
> > 
> 
> Bruce, may I add the files to MAINTAINERS under your name?

Yes, that is ok for these simple scripts.

Re: [dpdk-dev] [PATCH v12 09/14] build: optional NUMA and cpu counts detection

[Thomas Monjalon]

18/11/2020 15:19, Juraj Linkeš:
> From: Thomas Monjalon 
> > 16/11/2020 10:13, Bruce Richardson:
> > > On Mon, Nov 16, 2020 at 08:24:48AM +0100, Thomas Monjalon wrote:
> > > > 13/11/2020 15:31, Juraj Linkeš:
> > > > > +option('max_lcores', type: 'integer', value: 0,
> > > > > + description: 'maximum number of cores/threads supported by EAL.
> > > > > +Set to positive integer to overwrite per-arch or cross-compilation
> > defaults. Set to -1 to detect the number of cores on the build machine.')
> > option('max_numa_nodes', type: 'integer', value: 0,
> > > > > + description: 'maximum number of NUMA nodes supported by EAL. Set
> > > > > +to positive integer to overwrite per-arch or cross-compilation
> > > > > +defaults. Set to -1 to detect the number of numa nodes on the
> > > > > +build machine.')
> > > >
> > > > First comment: I don't like having so long description.
> > > > Second: I don't understand.
> > > >
> > > > It is said the default value is 0 so I expect it means automatic 
> > > > detection.
> > > > But later it is said -1 is for detection. So ?
> > > >
> > > Zero is for the "per-arch or cross-compilation default". This was
> > > discussed quite a bit in previous versions and this was te best
> > > compromise we could come up with. Having a default of auto-detect is
> > > definitely not something I think we should go with - just thinking of
> > > all the build CI jobs running on
> > > 2 or 4 core VMs! However, Juraj really felt there was value in having
> > > auto-detection, so it's set as a -1 value, which I'm ok with.
> > 
> > The problem is that I don't understand what 0 means.
> > 
> 
> There are three pieces of information which we need to convey:
> 1. The default value (0) indicates that per-arch or cross-compilation 
> defaults will be used.
> 2. Positive integer values will be used instead of these defaults.

Where these positive values come from?

> 3. Detected values will be used for native build when the value is -1.

Why not detect for any native build set up with 0 (default)?

Re: [dpdk-dev] [PATCH v2 6/7] net/af_xdp: remove useless assignment

[David Marchand]

On Wed, Nov 18, 2020 at 12:47 PM Ferruh Yigit  wrote:
>
> Assignment of function parameter 'umem' removed.
>
> Fixes: f0ce7af0e182 ("net/af_xdp: remove resources when port is closed")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Ferruh Yigit 
> ---
>  drivers/net/af_xdp/rte_eth_af_xdp.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c 
> b/drivers/net/af_xdp/rte_eth_af_xdp.c
> index 2c7892bd7e..7fc70df713 100644
> --- a/drivers/net/af_xdp/rte_eth_af_xdp.c
> +++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
> @@ -840,7 +840,6 @@ xdp_umem_destroy(struct xsk_umem_info *umem)
>  #endif
>
> rte_free(umem);
> -   umem = NULL;
>  }
>
>  static int

Reviewed-by: David Marchand 

-- 
David Marchand

Re: [dpdk-dev] [dpdk-stable] [PATCH v2 7/7] net/bnxt: fix redundant return

[David Marchand]

On Wed, Nov 18, 2020 at 12:48 PM Ferruh Yigit  wrote:
>
> Removing useless 'return' statement.
>
> Fixes: b2da02480cb7 ("net/bnxt: support EEM system memory")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Ferruh Yigit 
> ---
> Cc: peter.spreadboro...@broadcom.com
> ---
>  drivers/net/bnxt/tf_core/tf_em_common.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/net/bnxt/tf_core/tf_em_common.c 
> b/drivers/net/bnxt/tf_core/tf_em_common.c
> index ad92cbdc75..c96c21c2e9 100644
> --- a/drivers/net/bnxt/tf_core/tf_em_common.c
> +++ b/drivers/net/bnxt/tf_core/tf_em_common.c
> @@ -307,7 +307,6 @@ tf_em_page_tbl_pgcnt(uint32_t num_pages,
>  {
> return roundup(num_pages, MAX_PAGE_PTRS(page_size)) /
>MAX_PAGE_PTRS(page_size);
> -   return 0;
>  }
>
>  /**

Reviewed-by: David Marchand 

-- 
David Marchand

Re: [dpdk-dev] [PATCH v12 09/14] build: optional NUMA and cpu counts detection

[Bruce Richardson]

On Wed, Nov 18, 2020 at 03:42:36PM +0100, Thomas Monjalon wrote:
> 18/11/2020 15:19, Juraj Linkeš:
> > From: Thomas Monjalon 
> > > 16/11/2020 10:13, Bruce Richardson:
> > > > On Mon, Nov 16, 2020 at 08:24:48AM +0100, Thomas Monjalon wrote:
> > > > > 13/11/2020 15:31, Juraj Linkeš:
> > > > > > +option('max_lcores', type: 'integer', value: 0,
> > > > > > +   description: 'maximum number of cores/threads supported by EAL.
> > > > > > +Set to positive integer to overwrite per-arch or cross-compilation
> > > defaults. Set to -1 to detect the number of cores on the build machine.')
> > > option('max_numa_nodes', type: 'integer', value: 0,
> > > > > > +   description: 'maximum number of NUMA nodes supported by EAL. Set
> > > > > > +to positive integer to overwrite per-arch or cross-compilation
> > > > > > +defaults. Set to -1 to detect the number of numa nodes on the
> > > > > > +build machine.')
> > > > >
> > > > > First comment: I don't like having so long description.
> > > > > Second: I don't understand.
> > > > >
> > > > > It is said the default value is 0 so I expect it means automatic 
> > > > > detection.
> > > > > But later it is said -1 is for detection. So ?
> > > > >
> > > > Zero is for the "per-arch or cross-compilation default". This was
> > > > discussed quite a bit in previous versions and this was te best
> > > > compromise we could come up with. Having a default of auto-detect is
> > > > definitely not something I think we should go with - just thinking of
> > > > all the build CI jobs running on
> > > > 2 or 4 core VMs! However, Juraj really felt there was value in having
> > > > auto-detection, so it's set as a -1 value, which I'm ok with.
> > > 
> > > The problem is that I don't understand what 0 means.
> > > 
> > 
> > There are three pieces of information which we need to convey:
> > 1. The default value (0) indicates that per-arch or cross-compilation 
> > defaults will be used.
> > 2. Positive integer values will be used instead of these defaults.
> 
> Where these positive values come from?
> 
> > 3. Detected values will be used for native build when the value is -1.
> 
> Why not detect for any native build set up with 0 (default)?
> 
That was one of the original suggestions, but I strongly disagreed with
that, because many builds are done on hardware very different from the
final deployment. It would mean that any builds done in e.g. jenkins or
travis, with a 2-core vm, would be limited to running with two cores only.

Re: [dpdk-dev] [PATCH v12 09/14] build: optional NUMA and cpu counts detection

[Thomas Monjalon]

18/11/2020 15:54, Bruce Richardson:
> On Wed, Nov 18, 2020 at 03:42:36PM +0100, Thomas Monjalon wrote:
> > 18/11/2020 15:19, Juraj Linkeš:
> > > From: Thomas Monjalon 
> > > > 16/11/2020 10:13, Bruce Richardson:
> > > > > On Mon, Nov 16, 2020 at 08:24:48AM +0100, Thomas Monjalon wrote:
> > > > > > 13/11/2020 15:31, Juraj Linkeš:
> > > > > > > +option('max_lcores', type: 'integer', value: 0,
> > > > > > > + description: 'maximum number of cores/threads supported by EAL.
> > > > > > > +Set to positive integer to overwrite per-arch or 
> > > > > > > cross-compilation
> > > > defaults. Set to -1 to detect the number of cores on the build 
> > > > machine.')
> > > > option('max_numa_nodes', type: 'integer', value: 0,
> > > > > > > + description: 'maximum number of NUMA nodes supported by EAL. Set
> > > > > > > +to positive integer to overwrite per-arch or cross-compilation
> > > > > > > +defaults. Set to -1 to detect the number of numa nodes on the
> > > > > > > +build machine.')
> > > > > >
> > > > > > First comment: I don't like having so long description.
> > > > > > Second: I don't understand.
> > > > > >
> > > > > > It is said the default value is 0 so I expect it means automatic 
> > > > > > detection.
> > > > > > But later it is said -1 is for detection. So ?
> > > > > >
> > > > > Zero is for the "per-arch or cross-compilation default". This was
> > > > > discussed quite a bit in previous versions and this was te best
> > > > > compromise we could come up with. Having a default of auto-detect is
> > > > > definitely not something I think we should go with - just thinking of
> > > > > all the build CI jobs running on
> > > > > 2 or 4 core VMs! However, Juraj really felt there was value in having
> > > > > auto-detection, so it's set as a -1 value, which I'm ok with.
> > > > 
> > > > The problem is that I don't understand what 0 means.
> > > > 
> > > 
> > > There are three pieces of information which we need to convey:
> > > 1. The default value (0) indicates that per-arch or cross-compilation 
> > > defaults will be used.
> > > 2. Positive integer values will be used instead of these defaults.
> > 
> > Where these positive values come from?
> > 
> > > 3. Detected values will be used for native build when the value is -1.
> > 
> > Why not detect for any native build set up with 0 (default)?
> > 
> That was one of the original suggestions, but I strongly disagreed with
> that, because many builds are done on hardware very different from the
> final deployment. It would mean that any builds done in e.g. jenkins or
> travis, with a 2-core vm, would be limited to running with two cores only.

Yes that's the difference between native and cross build:
native build is not for running on a different machine.
I feel you have a different understanding of native build?

Re: [dpdk-dev] [PATCH v12 09/14] build: optional NUMA and cpu counts detection

[Juraj Linkeš]

> -Original Message-
> From: Thomas Monjalon 
> Sent: Wednesday, November 18, 2020 3:43 PM
> To: Bruce Richardson ; Juraj Linkeš
> 
> Cc: ruifeng.w...@arm.com; honnappa.nagaraha...@arm.com;
> phil.y...@arm.com; vcchu...@amazon.com; dharmik.thak...@arm.com;
> jerinjac...@gmail.com; hemant.agra...@nxp.com;
> ajit.khapa...@broadcom.com; ferruh.yi...@intel.com; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v12 09/14] build: optional NUMA and cpu counts
> detection
> 
> 18/11/2020 15:19, Juraj Linkeš:
> > From: Thomas Monjalon 
> > > 16/11/2020 10:13, Bruce Richardson:
> > > > On Mon, Nov 16, 2020 at 08:24:48AM +0100, Thomas Monjalon wrote:
> > > > > 13/11/2020 15:31, Juraj Linkeš:
> > > > > > +option('max_lcores', type: 'integer', value: 0,
> > > > > > +   description: 'maximum number of cores/threads supported by
> EAL.
> > > > > > +Set to positive integer to overwrite per-arch or
> > > > > > +cross-compilation
> > > defaults. Set to -1 to detect the number of cores on the build
> > > machine.') option('max_numa_nodes', type: 'integer', value: 0,
> > > > > > +   description: 'maximum number of NUMA nodes supported by
> EAL.
> > > > > > +Set to positive integer to overwrite per-arch or
> > > > > > +cross-compilation defaults. Set to -1 to detect the number of
> > > > > > +numa nodes on the build machine.')
> > > > >
> > > > > First comment: I don't like having so long description.
> > > > > Second: I don't understand.
> > > > >
> > > > > It is said the default value is 0 so I expect it means automatic 
> > > > > detection.
> > > > > But later it is said -1 is for detection. So ?
> > > > >
> > > > Zero is for the "per-arch or cross-compilation default". This was
> > > > discussed quite a bit in previous versions and this was te best
> > > > compromise we could come up with. Having a default of auto-detect
> > > > is definitely not something I think we should go with - just
> > > > thinking of all the build CI jobs running on
> > > > 2 or 4 core VMs! However, Juraj really felt there was value in
> > > > having auto-detection, so it's set as a -1 value, which I'm ok with.
> > >
> > > The problem is that I don't understand what 0 means.
> > >
> >
> > There are three pieces of information which we need to convey:
> > 1. The default value (0) indicates that per-arch or cross-compilation 
> > defaults
> will be used.
> > 2. Positive integer values will be used instead of these defaults.
> 
> Where these positive values come from?
> 

From the user - they will have the option to set it to whatever the like if 
they don't want to use defaults.

> > 3. Detected values will be used for native build when the value is -1.
> 
> Why not detect for any native build set up with 0 (default)?
> 

I'll let Bruce explain this, but I'll just say that we wanted to make the 
detection the default for native builds, so we're in agreement.

Re: [dpdk-dev] [PATCH] net/mlx5: fix RSS queue types validation

[Raslan Darawsheh]

Hi,

> -Original Message-
> From: dev  On Behalf Of Dekel Peled
> Sent: Wednesday, November 18, 2020 11:24 AM
> To: Slava Ovsiienko ; Shahaf Shuler
> ; Matan Azrad 
> Cc: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] net/mlx5: fix RSS queue types validation
> 
> Recent patch fixed the RSS action validation, making sure hairpin queues
> and standard queues are not used together in the same RSS action.
> The variable used for comparison was declared and initialized within the
> check loop, making the queue type comparison wrong.
> 
> This patch moves the variable declaration to the start of the function,
> outside of the check loop.
> 
> Fixes: cb8a079aee5d ("net/mlx5: fix validate RSS queues types")
> 
> Signed-off-by: Dekel Peled 
> Acked-by: Ori Kam 
> Acked-by: Jack Min 
> ---

Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh

[dpdk-dev] [PATCH v2 1/4] doc: move VFIO driver to be first

[Anatoly Burakov]

Currently, the Linux GSG mentions UIO drivers first. This is not ideal
as for the longest time, the recommended way to use DPDK with hardware
devices has been to use VFIO driver.

This commit simply moves UIO section after VFIO, with minor edits.

Signed-off-by: Anatoly Burakov 
---
 doc/guides/linux_gsg/linux_drivers.rst | 86 +-
 1 file changed, 43 insertions(+), 43 deletions(-)

diff --git a/doc/guides/linux_gsg/linux_drivers.rst 
b/doc/guides/linux_gsg/linux_drivers.rst
index 080b44955a..69ef4ee275 100644
--- a/doc/guides/linux_gsg/linux_drivers.rst
+++ b/doc/guides/linux_gsg/linux_drivers.rst
@@ -12,51 +12,10 @@ Different PMDs may require different kernel drivers in 
order to work properly.
 Depends on the PMD being used, a corresponding kernel driver should be load
 and bind to the network ports.
 
-UIO

-
-A small kernel module to set up the device, map device memory to user-space 
and register interrupts.
-In many cases, the standard ``uio_pci_generic`` module included in the Linux 
kernel
-can provide the uio capability. This module can be loaded using the command:
-
-.. code-block:: console
-
-sudo modprobe uio_pci_generic
-
-.. note::
-
-``uio_pci_generic`` module doesn't support the creation of virtual 
functions.
-
-As an alternative to the ``uio_pci_generic``, there is the ``igb_uio`` module
-which can be found in the repository `dpdk-kmods 
`_.
-It can be loaded as shown below:
-
-.. code-block:: console
-
-sudo modprobe uio
-sudo insmod igb_uio.ko
-
-.. note::
-
-   If UEFI secure boot is enabled, the Linux kernel may disallow the use of
-   UIO on the system. Therefore, devices for use by DPDK should be bound to the
-   ``vfio-pci`` kernel module rather than any UIO-based module.
-   For more details see :ref:`linux_gsg_binding_kernel` below.
-
-.. note::
-
-   If the devices used for DPDK are bound to the ``uio_pci_generic`` kernel 
module,
-   please make sure that the IOMMU is disabled or passthrough. One can add
-   ``intel_iommu=off`` or ``amd_iommu=off`` or ``intel_iommu=on iommu=pt`` in 
GRUB
-   command line on x86_64 systems, or add ``iommu.passthrough=1`` on aarch64 
system.
-
-Since DPDK release 1.7 onward provides VFIO support, use of UIO is optional
-for platforms that support using VFIO.
-
 VFIO
 
 
-A more robust and secure driver in compare to the ``UIO``, relying on IOMMU 
protection.
+VFIO is a robust and secure driver that relies on IOMMU protection.
 To make use of VFIO, the ``vfio-pci`` module must be loaded:
 
 .. code-block:: console
@@ -111,7 +70,48 @@ This can be done by using the DPDK setup script (called 
dpdk-setup.sh and locate
 
 .. note::
 
-VFIO can be used without IOMMU. While this is just as unsafe as using UIO, 
it does make it possible for the user to keep the degree of device access and 
programming that VFIO has, in situations where IOMMU is not available.
+VFIO can be used without IOMMU. While this is unsafe, it does make it 
possible for the user to keep the degree of device access and programming that 
VFIO has, in situations where IOMMU is not available.
+
+UIO
+---
+
+In situations where using VFIO is not an option, there are alternative drivers 
one can use.
+In many cases, the standard ``uio_pci_generic`` module included in the Linux 
kernel
+can provide the uio capability. This module can be loaded using the command:
+
+.. code-block:: console
+
+sudo modprobe uio_pci_generic
+
+.. note::
+
+``uio_pci_generic`` module doesn't support the creation of virtual 
functions.
+
+As an alternative to the ``uio_pci_generic``, there is the ``igb_uio`` module
+which can be found in the repository `dpdk-kmods 
`_.
+It can be loaded as shown below:
+
+.. code-block:: console
+
+sudo modprobe uio
+sudo insmod igb_uio.ko
+
+.. note::
+
+   If UEFI secure boot is enabled, the Linux kernel may disallow the use of
+   UIO on the system. Therefore, devices for use by DPDK should be bound to the
+   ``vfio-pci`` kernel module rather than any UIO-based module.
+   For more details see :ref:`linux_gsg_binding_kernel` below.
+
+.. note::
+
+   If the devices used for DPDK are bound to the ``uio_pci_generic`` kernel 
module,
+   please make sure that the IOMMU is disabled or passthrough. One can add
+   ``intel_iommu=off`` or ``amd_iommu=off`` or ``intel_iommu=on iommu=pt`` in 
GRUB
+   command line on x86_64 systems, or add ``iommu.passthrough=1`` on aarch64 
system.
+
+Since DPDK release 1.7 onward provides VFIO support, use of UIO is optional
+for platforms that support using VFIO.
 
 .. _bifurcated_driver:
 
-- 
2.17.1

[dpdk-dev] [PATCH v2 2/4] doc: reword VFIO and UIO Linux GSG section

[Anatoly Burakov]

Make sure that we always prioritize VFIO over UIO. Also, minor wording
corrections and improvements.

Signed-off-by: Anatoly Burakov 
---

Notes:
v2:
- Fix formatting for VF token section
- Some more typo and formatting fixes

 doc/guides/linux_gsg/linux_drivers.rst | 131 +++--
 1 file changed, 80 insertions(+), 51 deletions(-)

diff --git a/doc/guides/linux_gsg/linux_drivers.rst 
b/doc/guides/linux_gsg/linux_drivers.rst
index 69ef4ee275..a3f443db2c 100644
--- a/doc/guides/linux_gsg/linux_drivers.rst
+++ b/doc/guides/linux_gsg/linux_drivers.rst
@@ -9,8 +9,8 @@ Linux Drivers
 =
 
 Different PMDs may require different kernel drivers in order to work properly.
-Depends on the PMD being used, a corresponding kernel driver should be load
-and bind to the network ports.
+Depending on the PMD being used, a corresponding kernel driver should be 
loaded,
+and network ports should be bound to that driver.
 
 VFIO
 
@@ -22,51 +22,77 @@ To make use of VFIO, the ``vfio-pci`` module must be loaded:
 
 sudo modprobe vfio-pci
 
-Note that in order to use VFIO, your kernel must support it.
-VFIO kernel modules have been included in the Linux kernel since version 3.6.0 
and are usually present by default,
-however please consult your distributions documentation to make sure that is 
the case.
+VFIO kernel is usually present by default in all distributions, however please
+consult your distributions documentation to make sure that is the case.
 
-The ``vfio-pci`` module since Linux version 5.7 supports the creation of 
virtual
-functions. After the PF is bound to vfio-pci module, the user can create the 
VFs
-by sysfs interface, and these VFs are bound to vfio-pci module automatically.
+Since Linux version 5.7, the ``vfio-pci`` module supports the creation of 
virtual functions.
+After the PF is bound to ``vfio-pci`` module, the user can create the VFs 
using the ``sysfs`` interface,
+and these VFs will be bound to ``vfio-pci`` module automatically.
 
-When the PF is bound to vfio-pci, it has initial VF token generated by random. 
For
-security reason, this token is write only, the user can't read it from the 
kernel
-directly. To access the VF, the user needs to start the PF with token 
parameter to
-setup a VF token in UUID format, then the VF can be accessed with this new 
token.
+When the PF is bound to ``vfio-pci``, by default it will have a randomly
+generated VF token. For security reasons, this token is write only, so the user
+cannot read it from the kernel directly. To access the VFs, the user needs to
+create a new token, and use it to initialize both VF and PF devices. The tokens
+are in UUID format, so any UUID generation tool can be used to create a new
+token.
 
-Since the ``vfio-pci`` module uses the VF token as internal data to provide the
-collaboration between SR-IOV PF and VFs, so DPDK can use the same VF token for 
all
-PF devices which bound to one application. This VF token can be specified by 
the EAL
-parameter ``--vfio-vf-token``.
+This VF token can be passed to DPDK by using EAL parameter ``--vfio-vf-token``.
+The token will be used for all PF and VF ports within the application.
 
-.. code-block:: console
+1. Generate the VF token by uuid command
 
-1. Generate the VF token by uuid command
-14d63f20-8445-11ea-8900-1f9ce7d5650d
+   .. code-block:: console
 
-2. sudo modprobe vfio-pci enable_sriov=1
+  14d63f20-8445-11ea-8900-1f9ce7d5650d
 
-2. ./usertools/dpdk-devbind.py -b vfio-pci :86:00.0
+2. Load the ``vfio-pci`` module with ``enable_sriov`` parameter set
 
-3. echo 2 > /sys/bus/pci/devices/:86:00.0/sriov_numvfs
+   .. code-block:: console
 
-4. Start the PF:
-/app/dpdk-testpmd -l 22-25 -n 4 -w 86:00.0 \
- --vfio-vf-token=14d63f20-8445-11ea-8900-1f9ce7d5650d --file-prefix=pf 
-- -i
+  sudo modprobe vfio-pci enable_sriov=1
 
-5. Start the VF:
-/app/dpdk-testpmd -l 26-29 -n 4 -w 86:02.0 \
- --vfio-vf-token=14d63f20-8445-11ea-8900-1f9ce7d5650d 
--file-prefix=vf0 -- -i
+3. Bind the PCI devices to ``vfio-pci`` driver
 
-Also, to use VFIO, both kernel and BIOS must support and be configured to use 
IO virtualization (such as Intel® VT-d).
+   .. code-block:: console
+
+  ./usertools/dpdk-devbind.py -b vfio-pci :86:00.0
+
+4. Create the desired number of VF devices
+
+   .. code-block:: console
+
+  echo 2 > /sys/bus/pci/devices/:86:00.0/sriov_numvfs
+
+5. Start the DPDK application that will manage the PF device
+
+   .. code-block:: console
+
+  /app/dpdk-testpmd -l 22-25 -n 4 -w 86:00.0 \
+  --vfio-vf-token=14d63f20-8445-11ea-8900-1f9ce7d5650d --file-prefix=pf -- 
-i
+
+6. Start the DPDK application that will manage the VF device
+
+   .. code-block:: console
+
+  /app/dpdk-testpmd -l 26-29 -n 4 -w 86:02.0 \
+  --vfio-vf-token=14d63f20-8445-11ea-8900-1f9ce7d5650d --file-prefix=vf0 
-- -i
+
+To make use of full VFIO functionality, both ke

[dpdk-dev] [PATCH v2 3/4] doc: add VFIO no-IOMMU Linux GSG section

[Anatoly Burakov]

Currently, we have no documentation on how to use VFIO in no-IOMMU mode.
Add such documentation.

Signed-off-by: Anatoly Burakov 
---

Notes:
v2:
- Fixed the noiommu parameter name

 doc/guides/linux_gsg/linux_drivers.rst | 28 +-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/doc/guides/linux_gsg/linux_drivers.rst 
b/doc/guides/linux_gsg/linux_drivers.rst
index a3f443db2c..741b17a644 100644
--- a/doc/guides/linux_gsg/linux_drivers.rst
+++ b/doc/guides/linux_gsg/linux_drivers.rst
@@ -94,9 +94,35 @@ To make use of full VFIO functionality, both kernel and BIOS 
must support and be
 For proper operation of VFIO when running DPDK applications as a 
non-privileged user, correct permissions should also be set up.
 This can be done by using the DPDK setup script (called ``dpdk-setup.sh`` and 
located in the usertools directory).
 
+VFIO no-IOMMU mode
+--
+
+If there is no IOMMU available on the system, VFIO can still be used, but it 
has
+to be loaded with an additional module parameter:
+
+.. code-block:: console
+
+modprobe vfio enable_unsafe_noiommu_mode=1
+
+Alternatively, one can also enable this option in an already loaded kernel 
module:
+
+.. code-block:: console
+
+echo 1 > /sys/module/vfio/parameters/enable_unsafe_noiommu_mode
+
+After that, VFIO can be used with hardware devices as usual.
+
 .. note::
 
-VFIO can be used without IOMMU. While this is unsafe, it does make it 
possible for the user to keep the degree of device access and programming that 
VFIO has, in situations where IOMMU is not available.
+It may be required to unload all VFIO related-modules before probing the
+module again with ``enable_unsafe_noiommu_mode=1`` parameter.
+
+.. warning::
+
+Since no-IOMMU mode forgoes IOMMU protection, it is inherently unsafe. That
+said, it does make it possible for the user to keep the degree of device
+access and programming that VFIO has, in situations where IOMMU is not
+available.
 
 UIO
 ---
-- 
2.17.1

[dpdk-dev] [PATCH v2 4/4] doc: add VFIO troubleshooting section

[Anatoly Burakov]

There are common problems with VFIO that get asked over and over on the
mailing list. Document common problems with VFIO and how to fix them or
at least figure out what went wrong.

Signed-off-by: Anatoly Burakov 
---
 doc/guides/linux_gsg/linux_drivers.rst | 43 ++
 1 file changed, 43 insertions(+)

diff --git a/doc/guides/linux_gsg/linux_drivers.rst 
b/doc/guides/linux_gsg/linux_drivers.rst
index 741b17a644..57001c67ee 100644
--- a/doc/guides/linux_gsg/linux_drivers.rst
+++ b/doc/guides/linux_gsg/linux_drivers.rst
@@ -271,3 +271,46 @@ To restore device ``82:00.0`` to its original kernel 
binding:
 .. code-block:: console
 
 ./usertools/dpdk-devbind.py --bind=ixgbe 82:00.0
+
+Troubleshooting VFIO
+
+
+In certain situations, using ``dpdk-devbind.py`` script to bing a device to 
VFIO
+driver may fail. The first place to check is the kernel messages:
+
+.. code-block:: console
+
+# dmesg | tail
+...
+[ 1297.875090] vfio-pci: probe of :31:00.0 failed with error -22
+...
+
+In most cases, the ``error -22`` indicates that the VFIO subsystem couldn't be
+enabled because there is no IOMMU support. To check whether the kernel has been
+booted with correct parameters, one can check the kernel command-line:
+
+.. code-block:: console
+
+cat /proc/cmdline
+
+Please refer to earlier sections on how to configure kernel parameters 
correctly
+for your system.
+
+If the kernel is configured correctly, one also has to make sure that the BIOS
+configuration has virtualization features (such as Intel® VT-d). There is no
+standard way to check if the platform is configured correctly, so please check
+with your platform documentation to see if it has such features, and how to
+enable them.
+
+In certain distributions, default kernel configuration is such that the 
no-IOMMU
+mode is disabled altogether at compile time. This can be checked in the boot
+configuration of your system:
+
+.. code-block:: console
+
+# cat /boot/config-$(uname -r) | grep NOIOMMU
+# CONFIG_VFIO_NOIOMMU is not set
+
+If ``CONFIG_VFIO_NOIOMMU`` is not enabled in the kernel configuration, VFIO
+driver will not support the no-IOMMU mode, and other alternatives (such as UIO
+drivers) will have to be used.
-- 
2.17.1

Re: [dpdk-dev] [PATCH v12 09/14] build: optional NUMA and cpu counts detection

[Bruce Richardson]

On Wed, Nov 18, 2020 at 04:04:25PM +0100, Thomas Monjalon wrote:
> 18/11/2020 15:54, Bruce Richardson:
> > On Wed, Nov 18, 2020 at 03:42:36PM +0100, Thomas Monjalon wrote:
> > > 18/11/2020 15:19, Juraj Linkeš:
> > > > From: Thomas Monjalon 
> > > > > 16/11/2020 10:13, Bruce Richardson:
> > > > > > On Mon, Nov 16, 2020 at 08:24:48AM +0100, Thomas Monjalon wrote:
> > > > > > > 13/11/2020 15:31, Juraj Linkeš:
> > > > > > > > +option('max_lcores', type: 'integer', value: 0,
> > > > > > > > +   description: 'maximum number of cores/threads supported 
> > > > > > > > by EAL.
> > > > > > > > +Set to positive integer to overwrite per-arch or 
> > > > > > > > cross-compilation
> > > > > defaults. Set to -1 to detect the number of cores on the build 
> > > > > machine.')
> > > > > option('max_numa_nodes', type: 'integer', value: 0,
> > > > > > > > +   description: 'maximum number of NUMA nodes supported by 
> > > > > > > > EAL. Set
> > > > > > > > +to positive integer to overwrite per-arch or cross-compilation
> > > > > > > > +defaults. Set to -1 to detect the number of numa nodes on the
> > > > > > > > +build machine.')
> > > > > > >
> > > > > > > First comment: I don't like having so long description.
> > > > > > > Second: I don't understand.
> > > > > > >
> > > > > > > It is said the default value is 0 so I expect it means automatic 
> > > > > > > detection.
> > > > > > > But later it is said -1 is for detection. So ?
> > > > > > >
> > > > > > Zero is for the "per-arch or cross-compilation default". This was
> > > > > > discussed quite a bit in previous versions and this was te best
> > > > > > compromise we could come up with. Having a default of auto-detect is
> > > > > > definitely not something I think we should go with - just thinking 
> > > > > > of
> > > > > > all the build CI jobs running on
> > > > > > 2 or 4 core VMs! However, Juraj really felt there was value in 
> > > > > > having
> > > > > > auto-detection, so it's set as a -1 value, which I'm ok with.
> > > > > 
> > > > > The problem is that I don't understand what 0 means.
> > > > > 
> > > > 
> > > > There are three pieces of information which we need to convey:
> > > > 1. The default value (0) indicates that per-arch or cross-compilation 
> > > > defaults will be used.
> > > > 2. Positive integer values will be used instead of these defaults.
> > > 
> > > Where these positive values come from?
> > > 
> > > > 3. Detected values will be used for native build when the value is -1.
> > > 
> > > Why not detect for any native build set up with 0 (default)?
> > > 
> > That was one of the original suggestions, but I strongly disagreed with
> > that, because many builds are done on hardware very different from the
> > final deployment. It would mean that any builds done in e.g. jenkins or
> > travis, with a 2-core vm, would be limited to running with two cores only.
> 
> Yes that's the difference between native and cross build:
> native build is not for running on a different machine.
> I feel you have a different understanding of native build?
>
A cross-build is for running on a different architecture, not a different
machine. I can build on my laptop to run on a Xeon server and it's still a
native build rather than a cross build. For this discussion, a cross-build
is considered something using a cross-file.

/Bruce

[dpdk-dev] [PATCH v2] doc: flow rule removal on port stop

[Gregory Etelson]

There is a discrepancy between RTE ETHDEV API and flow rules guide
regarding flow rules maintenance after port stop.  RTE ETHDEV API in
librte_ethdev.h declares that flow rules will not be stored in PMD
after port stop:
 > Quite start
 Please note that some configuration is not stored between calls to
 rte_eth_dev_stop()/rte_eth_dev_start(). The following configuration
 will be retained:

 - MTU
 - flow control settings
 - receive mode configuration (promiscuous mode, all-multicast mode,
   hardware checksum mode, RSS/VMDQ settings etc.)
 - VLAN filtering configuration
 - default MAC address
 - MAC addresses supplied to MAC address array
 - flow director filtering mode (but not filtering rules)
 - NIC queue statistics mappings
  Quote end

PMD cannot always correctly restore flow rules after port stop / port
start because application may alter port configuration after port stop
without PMD knowledge about undergoing changes.  Consider the
following scenario:
application configures 2 queues 0 and 1 and creates a flow rule with
'queue index 1' action. After that application stops the port and
removes queue 1.
Although PMD can implement flow rule shadow copy to be used for
restore after port start, attempt to restore flow rule from shadow
will fail in example above and PMD could not notify application about
that failure.  As the result, flow rules map in HW will differ from
what application expects.  In addition, flow rules shadow copy used
for port start restore consumes considerable amount of system memory,
especially in systems with millions of flow rules.

Signed-off-by: Gregory Etelson 
Acked-by: Ori Kam 
---
 doc/guides/prog_guide/rte_flow.rst | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/doc/guides/prog_guide/rte_flow.rst 
b/doc/guides/prog_guide/rte_flow.rst
index ea203e0ca4..4cff9332fa 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -3229,10 +3229,12 @@ Caveats
   temporarily replacing the burst function pointers), an appropriate error
   code must be returned (``EBUSY``).
 
-- PMDs, not applications, are responsible for maintaining flow rules
-  configuration when stopping and restarting a port or performing other
-  actions which may affect them. They can only be destroyed explicitly by
-  applications.
+- Applications, not PMDs, are responsible for maintaining flow rules
+  configuration when closing, stopping or restarting a port or performing other
+  actions which may affect them.
+  Applications must assume that after port close, stop or restart all flows
+  related to that port are not valid, hardware rules are destroyed and relevant
+  PMD resources are released.
 
 For devices exposing multiple ports sharing global settings affected by flow
 rules:
-- 
2.29.2

Re: [dpdk-dev] [PATCH v2] usertools/devbind: fix binding for built-in 1kernel drivers

[Burakov, Anatoly]

On 18-Nov-20 2:58 AM, Yongxin Liu wrote:

In commit 681a67288655 ("usertools: check if module is loaded before
binding"), script will exit if no driver is found in /sys/module/.

However, for built-in kernel driver, /sys/module/MODULENAME only
shows up if it has a version or at least one parameter. Take ixgbe
for example, after kernel commit 34a2a3b83e2c ("net/intel: remove
driver versions from Intel drivers"), and if ixgbe is built directly
into kernel, there is no ixgbe folder in /sys/module. So the devbind
script should not exit.

Signed-off-by: Yongxin Liu 
---

v2:
  - fix git commit description style in commit log
  - fix typo spelling

---
  usertools/dpdk-devbind.py | 4 
  1 file changed, 4 deletions(-)

diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
index 99112b7ab..f3c0d9814 100755
--- a/usertools/dpdk-devbind.py
+++ b/usertools/dpdk-devbind.py
@@ -530,10 +530,6 @@ def bind_all(dev_list, driver, force=False):
  # driver generated error - it's not a valid device ID, so all is well
  pass
  
-# check if we're attempting to bind to a driver that isn't loaded

-if not module_is_loaded(driver.replace('-','_')):
-sys.exit("Error: Driver '%s' is not loaded." % driver)
-


I believe there is a way to check if module is built-in, can't we use 
that? We could keep a list of built-in modules of interest that we can 
get from:


cat /lib/modules/$(uname -r)/modules.builtin

It's a bit more changes, but this is better than just removing the error 
check.



  try:
  dev_list = map(dev_id_from_dev_name, dev_list)
  except ValueError as ex:




--
Thanks,
Anatoly

[dpdk-dev] [PATCH 2/7] regex/mlx5: fix iterator type in RXP engines management

[Michael Baum]

The mlx5_regex_rules_db_import function goes over all engines in the
loop and program rxp rules.

The iterator of the loop is called id and the variable representing the
number of engines is called priv->nb_engines.
The id variable is of uint8_t type while the priv->nb_engines variable
is of uint32_t type. The size of the priv->nb_engines variable is much
larger than the number of iterations allowed by the id type.
Theoretically there may be a situation where the value of the
priv->nb_engines will be greater than can be represented by 8 bits and
the loop will never end.

Change the type of id to uint32_t.

Fixes: b34d816363b5 ("regex/mlx5: support rules import")
Cc: sta...@dpdk.org

Signed-off-by: Michael Baum 
Acked-by: Ori Kam 
---
 drivers/regex/mlx5/mlx5_rxp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regex/mlx5/mlx5_rxp.c b/drivers/regex/mlx5/mlx5_rxp.c
index 7936a52..e54d2b8 100644
--- a/drivers/regex/mlx5/mlx5_rxp.c
+++ b/drivers/regex/mlx5/mlx5_rxp.c
@@ -921,7 +921,7 @@
 {
struct mlx5_regex_priv *priv = dev->data->dev_private;
struct mlx5_rxp_ctl_rules_pgm *rules = NULL;
-   uint8_t id;
+   uint32_t id;
int ret;
 
if (priv->prog_mode == MLX5_RXP_MODE_NOT_DEFINED) {
-- 
1.8.3.1

[dpdk-dev] [PATCH 5/7] regex/mlx5: improve error messages in RXP rules flush

[Michael Baum]

During the rules flush, the rxp_poll_csr_for_value function is called
twice. The rxp_poll_csr_for_value function can fail for two reasons:
1. It could not read the value from register, in which case the
function returns -1.
2. It read a value, but not the value it expected to receive. In this
case it returns -EBUZY.

When the function fails it prints an error message that is relevant only
for a second type of failure. Moreover, for failure of the first type it
prints a value of an uninitialized variable.
In case of success, the function prints a debug message about the number
of cycles it took. This line was probably copied by mistake, since the
variable it reads from, is always equal to 0 and is not an indicator of
the number of cycles.

Remove the incorrect line about the cycles, and reduce the error print
only for the relevant error.

Fixes: b34d816363b5 ("regex/mlx5: support rules import")
Cc: sta...@dpdk.org

Signed-off-by: Michael Baum 
Acked-by: Ori Kam 
---
 drivers/regex/mlx5/mlx5_rxp.c | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/regex/mlx5/mlx5_rxp.c b/drivers/regex/mlx5/mlx5_rxp.c
index ba78cc0..fcbc766 100644
--- a/drivers/regex/mlx5/mlx5_rxp.c
+++ b/drivers/regex/mlx5/mlx5_rxp.c
@@ -179,12 +179,14 @@
 count, ~0,
 MLX5_RXP_POLL_CSR_FOR_VALUE_TIMEOUT, id);
if (ret < 0) {
-   DRV_LOG(ERR, "Rules not rx by RXP: credit: %d, depth: %d", val,
-   fifo_depth);
+   if (ret == -EBUSY)
+   DRV_LOG(ERR, "Rules not rx by RXP: credit: %d, depth:"
+   " %d", val, fifo_depth);
+   else
+   DRV_LOG(ERR, "CSR poll failed, can't read value!");
return ret;
}
DRV_LOG(DEBUG, "RTRU FIFO depth: 0x%x", fifo_depth);
-   DRV_LOG(DEBUG, "Rules flush took %d cycles.", ret);
ret = mlx5_devx_regex_register_read(ctx, id, MLX5_RXP_RTRU_CSR_CTRL,
&val);
if (ret) {
@@ -203,10 +205,12 @@
 MLX5_RXP_RTRU_CSR_STATUS_UPDATE_DONE,
 MLX5_RXP_POLL_CSR_FOR_VALUE_TIMEOUT, id);
if (ret < 0) {
-   DRV_LOG(ERR, "Rules update timeout: 0x%08X", val);
+   if (ret == -EBUSY)
+   DRV_LOG(ERR, "Rules update timeout: 0x%08X", val);
+   else
+   DRV_LOG(ERR, "CSR poll failed, can't read value!");
return ret;
}
-   DRV_LOG(DEBUG, "Rules update took %d cycles", ret);
if (mlx5_devx_regex_register_read(ctx, id, MLX5_RXP_RTRU_CSR_CTRL,
  &val)) {
DRV_LOG(ERR, "CSR read failed!");
@@ -215,7 +219,7 @@
val &= ~(MLX5_RXP_RTRU_CSR_CTRL_GO);
if (mlx5_devx_regex_register_write(ctx, id, MLX5_RXP_RTRU_CSR_CTRL,
   val)) {
-   DRV_LOG(ERR, "CSR write write failed!");
+   DRV_LOG(ERR, "CSR write failed!");
return -1;
}
 
-- 
1.8.3.1

[dpdk-dev] [PATCH 6/7] regex/mlx5: improve constants type in QP buffers creation

[Michael Baum]

The constant representing the size of the metadata is defined as a
regular number (32-bit signed), even though all of its uses request an
unsigned int variable.
Similarly the constant representing the maximal output is also defined
as a regular number, even though all of its uses request an unsigned int
variable.

Change the type of the above constants to unsigned.

Fixes: 5f41b66d12cd ("regex/mlx5: setup fast path")
Cc: sta...@dpdk.org

Signed-off-by: Michael Baum 
Acked-by: Ori Kam 
---
 drivers/regex/mlx5/mlx5_regex_fastpath.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/regex/mlx5/mlx5_regex_fastpath.c 
b/drivers/regex/mlx5/mlx5_regex_fastpath.c
index 2549547..32ba19f 100644
--- a/drivers/regex/mlx5/mlx5_regex_fastpath.c
+++ b/drivers/regex/mlx5/mlx5_regex_fastpath.c
@@ -3,6 +3,8 @@
  */
 
 #include 
+#include 
+#include 
 #include 
 
 #include 
@@ -17,15 +19,14 @@
 #include 
 #include 
 #include 
-#include 
 
 #include "mlx5_regex_utils.h"
 #include "mlx5_rxp.h"
 #include "mlx5_regex.h"
 
 #define MLX5_REGEX_MAX_WQE_INDEX 0x
-#define MLX5_REGEX_METADATA_SIZE 64
-#define MLX5_REGEX_MAX_OUTPUT (1 << 11)
+#define MLX5_REGEX_METADATA_SIZE UINT32_C(64)
+#define MLX5_REGEX_MAX_OUTPUT (UINT32_C(1) << 11)
 #define MLX5_REGEX_WQE_CTRL_OFFSET 12
 #define MLX5_REGEX_WQE_METADATA_OFFSET 16
 #define MLX5_REGEX_WQE_GATHER_OFFSET 32
-- 
1.8.3.1

[dpdk-dev] [PATCH 3/7] regex/mlx5: fix unnecessary init in RXP handle

[Michael Baum]

The rxp_poll_csr_for_value function defines a variable named ret. It is
the return value of the function, and It is updated to 0 by default
later in the function.
Similarly the rxp_init_rtru function also defines a variable named ret.
The function assigns into it return values from functions during the
function.

In both functions they initialize the ret variable when defining it.
however, in both cases they do not use any ret variable before assigning
into them different values, so the initializations are unnecessary.

Clean the aforementioned unnecessary initializations.

Fixes: e3dbbf718ebc ("regex/mlx5: support configuration")
Fixes: b34d816363b5 ("regex/mlx5: support rules import")
Cc: sta...@dpdk.org

Signed-off-by: Michael Baum 
Acked-by: Ori Kam 
---
 drivers/regex/mlx5/mlx5_rxp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/regex/mlx5/mlx5_rxp.c b/drivers/regex/mlx5/mlx5_rxp.c
index e54d2b8..41fbae7 100644
--- a/drivers/regex/mlx5/mlx5_rxp.c
+++ b/drivers/regex/mlx5/mlx5_rxp.c
@@ -225,7 +225,7 @@
   uint32_t expected_mask, uint32_t timeout_ms, uint8_t id)
 {
unsigned int i;
-   int ret = 0;
+   int ret;
 
ret = -EBUSY;
for (i = 0; i < timeout_ms; i++) {
@@ -276,7 +276,7 @@
uint32_t poll_value;
uint32_t expected_value;
uint32_t expected_mask;
-   int ret = 0;
+   int ret;
 
/* Read the rtru ctrl CSR. */
ret = mlx5_devx_regex_register_read(ctx, id, MLX5_RXP_RTRU_CSR_CTRL,
-- 
1.8.3.1

[dpdk-dev] [PATCH 4/7] regex/mlx5: fix unchecked return value in RXP handle

[Michael Baum]

The rxp_flush_rules function tries to read and write to the register
several times using DevX API, and when it fails the function returns an
error.
Similarly the rxp_init_eng function also tries to write to the register
several times, and if writing is failed, it returns an error too.

Both functions have one write that the function does not check if it
succeeded, overriding the return value from the write function without
using it.

Add a check for this writing, and return an error in case of failure.

Fixes: b34d816363b5 ("regex/mlx5: support rules import")
Fixes: e3dbbf718ebc ("regex/mlx5: support configuration")
Cc: sta...@dpdk.org

Signed-off-by: Michael Baum 
Acked-by: Ori Kam 
---
 drivers/regex/mlx5/mlx5_rxp.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/regex/mlx5/mlx5_rxp.c b/drivers/regex/mlx5/mlx5_rxp.c
index 41fbae7..ba78cc0 100644
--- a/drivers/regex/mlx5/mlx5_rxp.c
+++ b/drivers/regex/mlx5/mlx5_rxp.c
@@ -194,6 +194,10 @@
val |= MLX5_RXP_RTRU_CSR_CTRL_GO;
ret = mlx5_devx_regex_register_write(ctx, id, MLX5_RXP_RTRU_CSR_CTRL,
 val);
+   if (ret) {
+   DRV_LOG(ERR, "CSR write failed!");
+   return -1;
+   }
ret = rxp_poll_csr_for_value(ctx, &val, MLX5_RXP_RTRU_CSR_STATUS,
 MLX5_RXP_RTRU_CSR_STATUS_UPDATE_DONE,
 MLX5_RXP_RTRU_CSR_STATUS_UPDATE_DONE,
@@ -554,6 +558,8 @@
return ret;
ctrl &= ~MLX5_RXP_CSR_CTRL_INIT;
ret = mlx5_devx_regex_register_write(ctx, id, MLX5_RXP_CSR_CTRL, ctrl);
+   if (ret)
+   return ret;
rte_delay_us(2);
ret = rxp_poll_csr_for_value(ctx, &ctrl, MLX5_RXP_CSR_STATUS,
 MLX5_RXP_CSR_STATUS_INIT_DONE,
-- 
1.8.3.1

[dpdk-dev] [PATCH 1/7] regex/mlx5: fix jump to the wrong label

[Michael Baum]

The mlx5_regex_pci_probe function allocates a mlx5_regex_priv structure
using rte_zmalloc.

If the allocation fails, the function jumps to the dev_error label in
order to release previously allocated resources in the function.
However, in the dev_error label it attempts to refer to the internal
fields of the priv structure and if its allocation fails (as in this
case) it is actually dereferencing to NULL.

Replace the jump with an error label.

Fixes: 1db6ebd4ef58 ("regex/mlx5: fix crash on initialization failure")
Cc: sta...@dpdk.org

Signed-off-by: Michael Baum 
Acked-by: Ori Kam 
---
 drivers/regex/mlx5/mlx5_regex.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regex/mlx5/mlx5_regex.c b/drivers/regex/mlx5/mlx5_regex.c
index 05048e7..c91c444 100644
--- a/drivers/regex/mlx5/mlx5_regex.c
+++ b/drivers/regex/mlx5/mlx5_regex.c
@@ -157,7 +157,7 @@
if (!priv) {
DRV_LOG(ERR, "Failed to allocate private memory.");
rte_errno = ENOMEM;
-   goto error;
+   goto dev_error;
}
priv->ctx = ctx;
priv->nb_engines = 2; /* attr.regexp_num_of_engines */
-- 
1.8.3.1

[dpdk-dev] [PATCH 7/7] regex/mlx5: fix QP setuping error flow

[Michael Baum]

In regex QP setup, the PMD creates some SQ objects.

When SQ object creation is failed, the previous SQ objects memory were
not freed what caused a memory leak.

Free them.

Fixes: 54fa1f6a67d7 ("regex/mlx5: add teardown for fastpath buffers")

Signed-off-by: Michael Baum 
Acked-by: Ori Kam 
---
 drivers/regex/mlx5/mlx5_regex_control.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/regex/mlx5/mlx5_regex_control.c 
b/drivers/regex/mlx5/mlx5_regex_control.c
index 88b3d1a..d6f452b 100644
--- a/drivers/regex/mlx5/mlx5_regex_control.c
+++ b/drivers/regex/mlx5/mlx5_regex_control.c
@@ -336,6 +336,7 @@
struct mlx5_regex_priv *priv = dev->data->dev_private;
struct mlx5_regex_qp *qp;
int i;
+   int nb_sq_config = 0;
int ret;
uint16_t log_desc;
 
@@ -364,8 +365,9 @@
ret = regex_ctrl_create_sq(priv, qp, i, log_desc);
if (ret) {
DRV_LOG(ERR, "Can't create sq.");
-   goto err_sq;
+   goto err_btree;
}
+   nb_sq_config++;
}
 
ret = mlx5_mr_btree_init(&qp->mr_ctrl.cache_bh, MLX5_MR_BTREE_CACHE_N,
@@ -385,9 +387,8 @@
 err_fp:
mlx5_mr_btree_free(&qp->mr_ctrl.cache_bh);
 err_btree:
-   for (i = 0; i < qp->nb_obj; i++)
+   for (i = 0; i < nb_sq_config; i++)
regex_ctrl_destroy_sq(priv, qp, i);
-err_sq:
regex_ctrl_destroy_cq(priv, &qp->cq);
 err_cq:
rte_free(qp->sqs);
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v2] app/testpmd: fix testpmd packets dump overlapping

[Jiawei(Jonny) Wang]

Hi Ferruh,

> -Original Message-
> From: Ferruh Yigit 
> Sent: Tuesday, November 17, 2020 8:07 PM
> To: Jiawei(Jonny) Wang ; wenzhuo...@intel.com;
> beilei.x...@intel.com; bernard.iremon...@intel.com; Ori Kam
> ; NBU-Contact-Thomas Monjalon
> ; Raslan Darawsheh 
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] app/testpmd: fix testpmd packets dump
> overlapping
> 
> On 11/14/2020 12:01 PM, Jiawei Wang wrote:
> > When testpmd enabled the verbosity for the received packets, if two
> > packets was received at the same time, for example, sampling packet
> > and normal packet, the dump output of these packets may be overlapping
> > due to multiple core handled the multiple queues simultaneously.
> >
> > The patch uses one string buffer that collects all the packet dump
> > output into this buffer and then printout it at last, that guarantee
> > to printout separately the dump output per packet.
> >
> > Fixes: d862c45 ("app/testpmd: move dumping packets to a separate
> > function")
> >
> > Signed-off-by: Jiawei Wang 
> > ---
> > v2:
> > * Print dump output of per packet instead of per burst.
> > * Add the checking for return value of 'snprintf' and exit if required size
> exceed the print buffer size.
> > * Update the log messages.
> > ---
> >   app/test-pmd/util.c | 378
> 
> >   1 file changed, 295 insertions(+), 83 deletions(-)
> >
> > diff --git a/app/test-pmd/util.c b/app/test-pmd/util.c index
> > 649bf8f..ffae590 100644
> > --- a/app/test-pmd/util.c
> > +++ b/app/test-pmd/util.c
> > @@ -12,15 +12,20 @@
> >   #include 
> >   #include 
> >   #include 
> > +#include 
> >
> >   #include "testpmd.h"
> >
> > -static inline void
> > -print_ether_addr(const char *what, const struct rte_ether_addr
> > *eth_addr)
> > +#define MAX_STRING_LEN 8192
> 
> Isn't '8192' too large for a single packet, what do you think for '2048'?
> 
2K is ok for the normal case, here consider the case that registered mbuf 
dynamic flags names, 
Then total maximum length can reach to   64* RTE_MBUF_DYN_NAMESIZE = 4096
 // char dynf_names[64][RTE_MBUF_DYN_NAMESIZE];
So 2048 is not enough if all dynf_names be configured as maximum length of 
dyn_names.

How about the 5K? I think should be enough for now.
> <...>
> 
> > @@ -93,95 +103,269 @@
> > is_encapsulation = RTE_ETH_IS_TUNNEL_PKT(packet_type);
> > ret = rte_flow_get_restore_info(port_id, mb, &info, &error);
> > if (!ret) {
> > -   printf("restore info:");
> > +   cur_len += snprintf(print_buf + cur_len,
> > +   buf_size - cur_len,
> > +   "restore info:");
> 
> What do you think having a macro like below to simplfy the code:
> 
>   #define FOO(buf, buf_size, cur_len, ...) \
>   do { \
>  if (cur_len >= buf_size) break; \
>  cur_len += snprintf(buf + cur_len, buf_size - cur_len, __VA_ARGS__); 
> \
>   } while (0)
> 
> It can be used as:
>   FOO(print_buf, buf_size, cur_len, "restore info:");
> 
Agree, will move these common code into a MARCO, 
I will use the  "MKDUMPSTR" as MARCO name,  means that  making a dump string 
buffer.

> > +   if (cur_len >= buf_size)
> > +   goto error;
> 
> This 'goto' goes out of the loop, not sure about breking the loop and not
> processing all packets if buffer is out of space for one of the packets.
> Anyway if you switch to macro above, the 'goto' is removed completely.
> 
Use 'break' only break the do{} while and continue to following processing, but 
won't filled anymore into printbuf since it will break firstly in the MARCO 
checking.
Or could use 'goto lable' instead 'break' in the marco?
> <...>
> 
> > +   if (cur_len >= buf_size)
> > +   goto error;
> > +   TESTPMD_LOG(INFO, "%s", print_buf);
> 
> Using 'TESTPMD_LOG' appends "testpmd: " at the beggining of the each line,
> for this case it is noise I think, what do you think turning back to printf() 
> as
> done originally?
> 
ok, will use the printf.
> > +   cur_len = 0;
> > }
> > +   return;
> > +error:
> > +   TESTPMD_LOG(INFO, "%s the output was truncated ...\n", print_buf);
> 
> What do you think having something shorter to notify the truncation, I think
> some special chars can work better, something like "...", "@@", "-->", "???" ?
> 
ok, I will use simple chars  for truncated case like below:
if (cur_len >= buf_size)
printf("%s ...\n", print_buf);

> >   }
> >
> >   uint16_t
> >

Thanks for your comments, I will do the changes and send V3 patch.

Thanks.
Jonny

[dpdk-dev] [PATCH v2 0/2] build: add Wformat to fix gcc compile warnings and format fixes

[Conor Walsh]

On some systems Wformat-nonliteral and Wformat-security could not be
checked without Wformat also being specified this causes a compile
warning on these systems. This patchset adds Wformat to
config/meson.build and fixes some format issues that this exposed.

---

v2:
- expand explaination of patches
- suppress false postives in icc
- add fixes for format issues

Conor Walsh (2):
  build: fix gcc compile warnings by adding wformat
  net/bnxt: fix format characters for unsigned values

 config/meson.build | 3 ++-
 drivers/net/bnxt/tf_core/tf_core.c | 8 
 2 files changed, 6 insertions(+), 5 deletions(-)

-- 
2.25.1

[dpdk-dev] [PATCH v2 1/2] build: fix gcc compile warnings by adding wformat

[Conor Walsh]

On some CentOS/RHEL systems using gcc 8.3.1 to compile dpdk, gcc shows a
warning on every build step saying that -Wformat-nonliteral and
-Wformat-security warnings will be ignored unless -Wformat is
also specified as a compiler flag. When the build is run with -werror
the build will fail due to these warnings.

Exact warning returned:
cc1: error: -Wformat-nonliteral ignored without -Wformat
[-Werror=format-nonliteral]
cc1: error: -Wformat-security ignored without -Wformat
[-Werror=format-security]
cc1: all warnings being treated as errors

This patch adds the -Wformat flag to config/meson.build. The warning id
181 has also been suppressed in icc as icc was showing false positives
with -Wformat enabled.

Fixes: 524a0d5d66b9 ("build: enable extra warnings with meson")
Cc: bruce.richard...@intel.com

Signed-off-by: Conor Walsh 
Tested-by: Chen, LingliX 
---
 config/meson.build | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/config/meson.build b/config/meson.build
index a29693b883..c02802c18e 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -194,6 +194,7 @@ warning_flags = [
# additional warnings in alphabetical order
'-Wcast-qual',
'-Wdeprecated',
+   '-Wformat',
'-Wformat-nonliteral',
'-Wformat-security',
'-Wmissing-declarations',
@@ -220,7 +221,7 @@ if not dpdk_conf.get('RTE_ARCH_64')
warning_flags += '-Wno-pointer-to-int-cast'
 endif
 if cc.get_id() == 'intel'
-   warning_ids = [188, 2203, 2279, 2557, 3179, 3656]
+   warning_ids = [181, 188, 2203, 2279, 2557, 3179, 3656]
foreach i:warning_ids
warning_flags += '-diag-disable=@0@'.format(i)
endforeach
-- 
2.25.1

[dpdk-dev] [PATCH v2 2/2] net/bnxt: fix format characters for unsigned values

[Conor Walsh]

&device requires the %u format specifer not the %d specifier, as
&device is unsigned.

Fixes: a46bbb57605b ("net/bnxt: update multi device design")
Cc: michael.wi...@broadcom.com

Signed-off-by: Conor Walsh 
---
 drivers/net/bnxt/tf_core/tf_core.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bnxt/tf_core/tf_core.c 
b/drivers/net/bnxt/tf_core/tf_core.c
index 0f49a00256..24d49096a7 100644
--- a/drivers/net/bnxt/tf_core/tf_core.c
+++ b/drivers/net/bnxt/tf_core/tf_core.c
@@ -44,7 +44,7 @@ tf_open_session(struct tf *tfp,
 
/* Verify control channel and build the beginning of session_id */
rc = sscanf(parms->ctrl_chan_name,
-   "%x:%x:%x.%d",
+   "%x:%x:%x.%u",
&domain,
&bus,
&slot,
@@ -57,7 +57,7 @@ tf_open_session(struct tf *tfp,
 
/* Check parsing of bus/slot/device */
rc = sscanf(parms->ctrl_chan_name,
-   "%x:%x.%d",
+   "%x:%x.%u",
&bus,
&slot,
&device);
@@ -102,7 +102,7 @@ tf_attach_session(struct tf *tfp,
 
/* Verify control channel */
rc = sscanf(parms->ctrl_chan_name,
-   "%x:%x:%x.%d",
+   "%x:%x:%x.%u",
&domain,
&bus,
&slot,
@@ -115,7 +115,7 @@ tf_attach_session(struct tf *tfp,
 
/* Verify 'attach' channel */
rc = sscanf(parms->attach_chan_name,
-   "%x:%x:%x.%d",
+   "%x:%x:%x.%u",
&domain,
&bus,
&slot,
-- 
2.25.1

[dpdk-dev] [PATCH] net/mlx5: fix restore info in non-tunnel traffic.

[Gregory Etelson]

Tunnel offload API provides applications with ability to restore
packet outer headers after partial offload. Exact feature execution
depends on hardware abilities and PMD implementation. Hardware that is
supported by MLX5 PMD places a mark on a packet after partial offload.
PMD decodes that mark and provides application with required
information.
Application can call the restore API for packets that are part of
offloaded tunnel and not. It's up to a PMD to provide correct
information.
Current MLX5 tunnel offload implementation does not allow applications
to use flow MARK actions. It is restricted to tunnel offload use only.
This fault was triggered by application that did not activate tunnel
offload and called the restore API with a marked packet. The PMD tried
to decode the mark value and crashed. The patch decodes mark value
only if tunnel offload is active.

Fixes: 4ec6360de37d ("net/mlx5: implement tunnel offload")

Signed-off-by: Gregory Etelson 
Acked-by: Viacheslav Ovsiienko 
---
 drivers/net/mlx5/mlx5_flow.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index f650ac0f1a..601008d9b2 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -7819,6 +7819,11 @@ mlx5_flow_tunnel_get_restore_info(struct rte_eth_dev 
*dev,
const struct mlx5_flow_tbl_data_entry *tble;
const uint64_t mask = PKT_RX_FDIR | PKT_RX_FDIR_ID;
 
+   if (!is_tunnel_offload_active(dev)) {
+   info->flags = 0;
+   return 0;
+   }
+
if ((ol_flags & mask) != mask)
goto err;
tble = tunnel_mark_decode(dev, m->hash.fdir.hi);
-- 
2.29.2

Re: [dpdk-dev] [PATCH v2 1/4] doc: move VFIO driver to be first

[David Marchand]

On Wed, Nov 18, 2020 at 4:38 PM Anatoly Burakov
 wrote:
>
> Currently, the Linux GSG mentions UIO drivers first. This is not ideal
> as for the longest time, the recommended way to use DPDK with hardware
> devices has been to use VFIO driver.
>
> This commit simply moves UIO section after VFIO, with minor edits.
>
> Signed-off-by: Anatoly Burakov 

This series does not apply on rc4, could you rebase it?
Thanks.


-- 
David Marchand

Re: [dpdk-dev] [PATCH v2 0/2] build: add Wformat to fix gcc compile warnings and format fixes

[Luca Boccassi]

On Wed, 2020-11-18 at 18:11 +, Conor Walsh wrote:
> On some systems Wformat-nonliteral and Wformat-security could not be
> checked without Wformat also being specified this causes a compile
> warning on these systems. This patchset adds Wformat to
> config/meson.build and fixes some format issues that this exposed.
> 
> ---
> 
> v2:
> - expand explaination of patches
> - suppress false postives in icc
> - add fixes for format issues
> 
> Conor Walsh (2):
>   build: fix gcc compile warnings by adding wformat
>   net/bnxt: fix format characters for unsigned values
> 
>  config/meson.build | 3 ++-
>  drivers/net/bnxt/tf_core/tf_core.c | 8 
>  2 files changed, 6 insertions(+), 5 deletions(-)

Looks good to me, but it should be cc'ed to stable as well I think

-- 
Kind regards,
Luca Boccassi

Re: [dpdk-dev] [PATCH v12 09/14] build: optional NUMA and cpu counts detection

[Honnappa Nagarahalli]

> 
> On Wed, Nov 18, 2020 at 04:04:25PM +0100, Thomas Monjalon wrote:
> > 18/11/2020 15:54, Bruce Richardson:
> > > On Wed, Nov 18, 2020 at 03:42:36PM +0100, Thomas Monjalon wrote:
> > > > 18/11/2020 15:19, Juraj Linkeš:
> > > > > From: Thomas Monjalon 
> > > > > > 16/11/2020 10:13, Bruce Richardson:
> > > > > > > On Mon, Nov 16, 2020 at 08:24:48AM +0100, Thomas Monjalon
> wrote:
> > > > > > > > 13/11/2020 15:31, Juraj Linkeš:
> > > > > > > > > +option('max_lcores', type: 'integer', value: 0,
> > > > > > > > > + description: 'maximum number of cores/threads
> supported by EAL.
> > > > > > > > > +Set to positive integer to overwrite per-arch or
> > > > > > > > > +cross-compilation
> > > > > > defaults. Set to -1 to detect the number of cores on the build
> > > > > > machine.') option('max_numa_nodes', type: 'integer', value: 0,
> > > > > > > > > + description: 'maximum number of NUMA nodes
> supported
> > > > > > > > > +by EAL. Set to positive integer to overwrite per-arch
> > > > > > > > > +or cross-compilation defaults. Set to -1 to detect the
> > > > > > > > > +number of numa nodes on the build machine.')
> > > > > > > >
> > > > > > > > First comment: I don't like having so long description.
> > > > > > > > Second: I don't understand.
> > > > > > > >
> > > > > > > > It is said the default value is 0 so I expect it means automatic
> detection.
> > > > > > > > But later it is said -1 is for detection. So ?
> > > > > > > >
> > > > > > > Zero is for the "per-arch or cross-compilation default".
> > > > > > > This was discussed quite a bit in previous versions and this
> > > > > > > was te best compromise we could come up with. Having a
> > > > > > > default of auto-detect is definitely not something I think
> > > > > > > we should go with - just thinking of all the build CI jobs
> > > > > > > running on
> > > > > > > 2 or 4 core VMs! However, Juraj really felt there was value
> > > > > > > in having auto-detection, so it's set as a -1 value, which I'm ok
> with.
> > > > > >
> > > > > > The problem is that I don't understand what 0 means.
> > > > > >
> > > > >
> > > > > There are three pieces of information which we need to convey:
> > > > > 1. The default value (0) indicates that per-arch or cross-compilation
> defaults will be used.
> > > > > 2. Positive integer values will be used instead of these defaults.
> > > >
> > > > Where these positive values come from?
> > > >
> > > > > 3. Detected values will be used for native build when the value is -1.
> > > >
> > > > Why not detect for any native build set up with 0 (default)?
> > > >
> > > That was one of the original suggestions, but I strongly disagreed
> > > with that, because many builds are done on hardware very different
> > > from the final deployment. It would mean that any builds done in
> > > e.g. jenkins or travis, with a 2-core vm, would be limited to running with
> two cores only.
> >
> > Yes that's the difference between native and cross build:
> > native build is not for running on a different machine.
> > I feel you have a different understanding of native build?
> >
> A cross-build is for running on a different architecture, not a different
> machine. I can build on my laptop to run on a Xeon server and it's still a 
> native
> build rather than a cross build. For this discussion, a cross-build is 
> considered
> something using a cross-file.
> 
This requires that the CPU on the laptop and the Xeon server support the same 
ISA version (I am not sure what it is called in x86). For ex: if the laptop 
supports AVX512, the Xeon server also needs to support AVX512. So, the user 
needs to be aware of all the instructions (or ISA version) on which the binary 
was built to be able to run it on another x86 machine. I am not sure if this is 
called out or understood by the users. In this regard, I tend to agree with 
Thomas that a native build is running on the same machine where it was built.

> /Bruce

Re: [dpdk-dev] [dpdk-stable] [PATCH v2 7/7] net/bnxt: fix redundant return

[Ajit Khaparde]

On Wed, Nov 18, 2020 at 6:43 AM David Marchand
 wrote:
>
> On Wed, Nov 18, 2020 at 12:48 PM Ferruh Yigit  wrote:
> >
> > Removing useless 'return' statement.
> >
> > Fixes: b2da02480cb7 ("net/bnxt: support EEM system memory")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Ferruh Yigit 
> > ---
> > Cc: peter.spreadboro...@broadcom.com
> > ---
> >  drivers/net/bnxt/tf_core/tf_em_common.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/net/bnxt/tf_core/tf_em_common.c 
> > b/drivers/net/bnxt/tf_core/tf_em_common.c
> > index ad92cbdc75..c96c21c2e9 100644
> > --- a/drivers/net/bnxt/tf_core/tf_em_common.c
> > +++ b/drivers/net/bnxt/tf_core/tf_em_common.c
> > @@ -307,7 +307,6 @@ tf_em_page_tbl_pgcnt(uint32_t num_pages,
> >  {
> > return roundup(num_pages, MAX_PAGE_PTRS(page_size)) /
> >MAX_PAGE_PTRS(page_size);
> > -   return 0;
> >  }
> >
> >  /**
>
> Reviewed-by: David Marchand 

Acked-by: Ajit Khaparde 

>
>
> --
> David Marchand
>

Re: [dpdk-dev] [EXT] Re: [PATCH] maintainers: Update for OcteonTx2 DMA and EP

[Radha Mohan]

On Wed, Nov 18, 2020 at 1:15 AM Thomas Monjalon  wrote:
>
> 18/11/2020 05:15, Radha Mohan:
> > On Mon, Nov 16, 2020 at 12:28 PM Thomas Monjalon  
> > wrote:
> > > 16/11/2020 19:28, Radha Mohan:
> > > > On Fri, Nov 13, 2020 at 2:39 PM Thomas Monjalon  
> > > > wrote:
> > > > > 13/11/2020 20:18, Radha Mohan:
> > > > > > On Tue, Nov 10, 2020 at 11:57 PM Mahipal Challa 
> > > > > >  wrote:
> > > > > > From: Radha Mohan 
> > > > > > Sent: Tuesday, November 10, 2020 11:44 PM
> > > > > > > On Mon, Nov 9, 2020 at 4:20 PM Radha Mohan Chintakuntla 
> > > > > > >  wrote:
> > > > > > > >
> > > > > > > > Replace the maintainers for OcteonTx2 DMA and EP drivers.
> > > > > > > >
> > > > > > > > Signed-off-by: Radha Mohan Chintakuntla 
> > > > > [...]
> > > > > > > >Adding previous maintainers to ack.
> > > > > > > Acked-by: Mahipal Challa  > > > > > >
> > > > > > > Thanks,
> > > > > > > Mahipal
> > > > > >
> > > > > > Hi Thomas,
> > > > > > Could you please pick this patch ?
> > > > >
> > > > > This low-priority patch has been sent 4 days ago,
> > > > > and was acked by only 1 maintainer 3 days ago.
> > > > >
> > > >
> > > > It was acked by both the old maintainers Satha Koteshwar and Mahipal
> > > > Challa. Maybe both of those emails didn't come on top of each other.
> > >
> > > I missed one.
> > >
> > > > > Why pushing? What is your fear exactly?
> > > >
> > > > Just want to make the transfer of ownership official in the mainline.
> > > > There's no fear I just normally wanted to send a reminder to you to
> > > > pick as it might be missed inclusion. If there's a guarantee that all
> > > > ack'ed patches will go automatically then I won't be sending any
> > > > reminder emails in future.
> > >
> > > It's OK to send reminder, but please wait at least a week.
> > >
> > > > > Why the new maintainers candidates have 0 and 2 contributions
> > > > > in git history?
> > > > >
> > > > These drivers requires some maintenance and likely expanded to support
> > > > future chips from Marvell. So you will not see contributions right
> > > > away.
> > > > So to become a maintainer one has to have prior contributions ?
> > >
> > > Yes: 
> > > http://doc.dpdk.org/guides/contributing/patches.html#maintainers-and-sub-trees
> > > "
> > > Maintainers should have demonstrated a reasonable level of contributions 
> > > or reviews to the component area. The maintainer should be confirmed by 
> > > an ack from an established contributor.
> > > "
> >
> > How was this done without prior contributions?
> > http://git.dpdk.org/dpdk/commit/MAINTAINERS?id=238e3167ca869abf44fa50ead022d7fc3b99605b
>
> I asked a confirmation:
> http://inbox.dpdk.org/dev/1985242.AXpOyGb66D@thomas/

Right. But in this case I got different.

>
>
> > The commit log says that he is a "new developer". So am I technically
> > to the community.
> >
> > Also the thing that puzzles me most is its a driver specific to
> > Marvell and both the previous maintainers are ok with the transfer but
> > a strange rule that probably applies to generic things of dpdk code
> > (which really makes sense there) comes as a blocker.
>
> Did I say it is a blocker?

Maybe I got that impression from your comments.

>
> > And how does new driver go?
>
> For new drivers, the new maintainers arrive by contributing new code.
>
> > Lets say if either myself or veerasena have pushed a new driver now
> > who has to ack it for inclusion ? Isn't that like a chicken and egg
> > problem.
>
> Nobody has to ack for a new driver.
>
> > I am looking for clarifications as things are different in other open
> > source communities.
> >
> > > > Kind of makes odd sense as these drivers are specific to our chip and we
> > > > are changing ownership for maintaining them.
> > > > I do not understand why you have an issue here?
> > > > You don't like reminding then I understand.
> > >
> > > I don't like how you push new unconfirmed maintainers.
> >
> > Could you please explain above commit made into new maintainer?
>
> Listen: I say I don't like how pushy you are, that's all.
> In general it's very good to have new contributors.
> But please think how you can help instead of just requesting.
> We have a lot more important things to care at the moment
> to close the big release 20.11. If you want to help, you are welcome.
>

ok fair enough. For my defence i wasn't being pushy. So hopefully this
updation can be taken once you are able to do after 20.11.

>

Re: [dpdk-dev] [PATCH 3/5] net/bnxt: fix protocol size for VXLAN encap copy

[Ajit Khaparde]

On Tue, Nov 17, 2020 at 10:37 PM Andrew Rybchenko
 wrote:
>
> On 11/18/20 3:34 AM, Ajit Khaparde wrote:
> > On Mon, Nov 16, 2020 at 8:13 AM Ferruh Yigit  wrote:
> >
> >> On 11/16/2020 7:55 AM, Xiaoyu Min wrote:
> >>> From: Xiaoyu Min 
> >>>
> >>> The rte_flow_item_eth and rte_flow_item_vlan items are refined.
> >>> The structs do not exactly represent the packet bits captured on the
> >>> wire anymore so should only copy real header instead of the whole struct.
> >>>
> >>> Replace the rte_flow_item_* with the existing corresponding rte_*_hdr.
> >>>
> >>> Fixes: 09315fc83861 ("ethdev: add VLAN attributes to ethernet and VLAN
> >> items")
> >>>
> >>> Signed-off-by: Xiaoyu Min 
> >>
> >> <...>
> >>
> >>> @@ -1726,7 +1727,7 @@ ulp_rte_vxlan_encap_act_handler(const struct
> >> rte_flow_action *action_item,
> >>>   BNXT_TF_DBG(ERR, "vxlan encap does not have vni\n");
> >>>   return BNXT_TF_RC_ERROR;
> >>>   }
> >>> - vxlan_size = sizeof(struct rte_flow_item_vxlan);
> >>> + vxlan_size = sizeof(struct rte_vxlan_hdr);
> >>>   /* copy the vxlan details */
> >>>   memcpy(&vxlan_spec, item->spec, vxlan_size);
> >>>   vxlan_spec.flags = 0x08;
> >>>
> >>
> >> 'vxlan_size' seems used both to copy rt_flow_item [1] and to header [2].
> >> Also
> >> ''vxlan_size' is used to copy the 'vxlan_spec'.
> >>
> >> Since both "struct rte_flow_item_vxlan" & "struct rte_vxlan_hdr" size is
> >> same,
> >> this should work fine, but I guess it may be broken if sizes of those two
> >> structures changes in the future.
> >>
> >> [1]
> >> memcpy(&vxlan_spec, item->spec, vxlan_size);
> >>
> >> [2]
> >> ulp_encap_buffer_copy(buff, (const uint8_t *)&vxlan_spec,
> >> vxlan_size, ULP_BUFFER_ALIGN_8_BYTE);
> >>
> > Also, I feel rte_flow_item_vxlan is a control plane structure while
> > rte_vlan_hdr is more for dataplane.
> > It's better to keep them separate to allow refining them independently.
>
> I disagree with the point. It is typical duplication which
> should be fixed. VXLAN item should consist of rte_vxlan_hdr
> plus extra non header fields. In fact, similar thing should
> be done for Ethernet flow item spec.
>
> These items are used for VXLAN encap action and in current
> state of definitions it is tricky to use VXLAN and Ethernet
> items to construct header to be prepended. It is required
> to copy field by field since it is bad to rely on the fact
> that flow item starts from protocol header if it is not
> specified in corresponding C type definition.
I took a look at the change again. I had an impression that
it is changing from specific protocol header to rte flow item,
not the other way around. This is fine.
Note that the commit is changing VXLAN and VLAN. But the log
suggests only VXLAN. Other than that,

Acked-by: Ajit Khaparde 

[dpdk-dev] [PATCH v2] doc: update ice user guide

[Qi Zhang]

Add link for firmware/OOT kernel driver/DDP download
Add matching List.

Signed-off-by: Qi Zhang 
---

v2:
- correct the DDP version number

 doc/guides/nics/ice.rst | 28 +---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/doc/guides/nics/ice.rst b/doc/guides/nics/ice.rst
index c5a76a2a21..1406b8aa88 100644
--- a/doc/guides/nics/ice.rst
+++ b/doc/guides/nics/ice.rst
@@ -5,9 +5,8 @@ ICE Poll Mode Driver
 ==
 
 The ice PMD (**librte_net_ice**) provides poll mode driver support for
-10/25/50/100 Gbps Intel® Ethernet 810 Series Network Adapters based on
-the Intel Ethernet Controller E810.
-
+10/25/50/100 Gbps Intel® Ethernet 800 Series Network Adapters based on
+the Intel Ethernet Controller E810 and Intel Ethernet Connection E822/E823.
 
 Prerequisites
 -
@@ -17,6 +16,29 @@ Prerequisites
 - To get better performance on Intel platforms, please follow the "How to get 
best performance with NICs on Intel platforms"
   section of the :ref:`Getting Started Guide for Linux `.
 
+- Please follow the matching list to download specific kernel driver, firmware 
and DDP package from
+  
`https://www.intel.com/content/www/us/en/search.html?ws=text#q=e810&t=Downloads&layout=table`.
+
+- To understand what is DDP package and how it works, please review `Intel® 
Ethernet Controller E810 Dynamic
+  Device Personalization (DDP) for Telecommunications Technology Guide 
`_.
+
+- To understand DDP for COMMs usage with DPDK, please review `Intel® Ethernet 
800 Series Telecommunication (Comms)
+  Dynamic Device Personalization (DDP) Package 
`_.
+
+
+Recommended Matching List
+-
+
+It is highly recommended to upgrade the ice kernel driver, firmware and DDP 
package
+to avoid the compatibility issues with ice PMD. Here is the suggested matching 
list which has been tested and verified.
+The detailed information can refer to chapter Tested Platforms/Tested NICs in 
release notes.
+
+   +---+---+-+---+---+
+   |DPDK   | Kernel Driver | OS Default DDP  | COMMS DDP | Firmware  |
+   +===+===+=+===+===+
+   |20.11  | 1.3.0 |  1.3.20 |  1.3.24   |2.3|
+   +---+---+-+---+---+
+
 Pre-Installation Configuration
 --
 
-- 
2.26.2

Re: [dpdk-dev] [PATCH] doc: update the OS versions

[Ajit Khaparde]

On Mon, Nov 16, 2020 at 9:11 PM Ajit Khaparde
 wrote:
>
> Update the OS versions. Remove some old OS versions.
>
> Signed-off-by: Ajit Khaparde 
Patch applied to dpdk-next-net-brcm.

> ---
>  doc/guides/nics/bnxt.rst | 8 +++-
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/doc/guides/nics/bnxt.rst b/doc/guides/nics/bnxt.rst
> index d9a7d87930..898c71ef1e 100644
> --- a/doc/guides/nics/bnxt.rst
> +++ b/doc/guides/nics/bnxt.rst
> @@ -29,17 +29,16 @@ Operating Systems supported:
>
>  * Red Hat Enterprise Linux release 8.1 (Ootpa)
>  * Red Hat Enterprise Linux release 8.0 (Ootpa)
> +* Red Hat Enterprise Linux Server release 7.8 (Maipo)
>  * Red Hat Enterprise Linux Server release 7.7 (Maipo)
>  * Red Hat Enterprise Linux Server release 7.6 (Maipo)
>  * Red Hat Enterprise Linux Server release 7.5 (Maipo)
> -* Red Hat Enterprise Linux Server release 7.4 (Maipo)
> -* Red Hat Enterprise Linux Server release 7.3 (Maipo)
> -* Red Hat Enterprise Linux Server release 7.2 (Maipo)
> +* CentOS Linux release 8.1
>  * CentOS Linux release 8.0
> +* CentOS Linux release 7.8
>  * CentOS Linux release 7.7
>  * CentOS Linux release 7.6.1810
>  * CentOS Linux release 7.5.1804
> -* CentOS Linux release 7.4.1708
>  * Fedora 31
>  * FreeBSD 12.1
>  * Suse 15SP1
> @@ -47,7 +46,6 @@ Operating Systems supported:
>  * Ubuntu 18.04
>  * Ubuntu 16.10
>  * Ubuntu 16.04
> -* Ubuntu 14.04
>
>  The BNXT PMD supports operating with:
>
> --
> 2.21.1 (Apple Git-122.3)
>

Re: [dpdk-dev] [PATCH v2] net/mlx5: make flow operation thread safe

[Suanming Mou]

Hi,

(Seems I replied to the wrong version yesterday.)
Since the issue happens only in the flow flush when the port status change.
The root cause is the LSC callback should not be called before the port start 
is done.

BR,
SuanmingMou

> -Original Message-
> From: dev  On Behalf Of Weifeng Li
> Sent: Sunday, November 8, 2020 3:54 PM
> To: Matan Azrad 
> Cc: dev@dpdk.org; Weifeng Li 
> Subject: [dpdk-dev] [PATCH v2] net/mlx5: make flow operation thread safe
> 
> Does it need a lock for flow about below scene.
> Thread1: flow_list_destroyflow_list_create
> Thread2: -flow_list_destroy
> Maybe the same flow can be operate at the same time.
> 
> When i start mlx5 bond and trigger LSC at the same time.
> It is possible to assert in mlx5_rx_queue_release func and print "port 4 Rx 
> queue
> 0 is still used by a flow and cannot be removed". I use dpdk-testpmd to 
> simulate
> the test.
> 
> Signed-off-by: Weifeng Li 
> ---
> v2: adjust coding style issue.
> ---
>  drivers/net/mlx5/linux/mlx5_os.c |  1 +
>  drivers/net/mlx5/mlx5.h  |  1 +
>  drivers/net/mlx5/mlx5_flow.c | 13 +++--
>  3 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/mlx5/linux/mlx5_os.c 
> b/drivers/net/mlx5/linux/mlx5_os.c
> index c78d56f..59c074e 100644
> --- a/drivers/net/mlx5/linux/mlx5_os.c
> +++ b/drivers/net/mlx5/linux/mlx5_os.c
> @@ -1426,6 +1426,7 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
> MLX5_MAX_MAC_ADDRESSES);
>   priv->flows = 0;
>   priv->ctrl_flows = 0;
> + rte_spinlock_init(&priv->flow_lock);
>   rte_spinlock_init(&priv->flow_list_lock);
>   TAILQ_INIT(&priv->flow_meters);
>   TAILQ_INIT(&priv->flow_meter_profiles);
> diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index
> b43a8c9..860bf2f 100644
> --- a/drivers/net/mlx5/mlx5.h
> +++ b/drivers/net/mlx5/mlx5.h
> @@ -963,6 +963,7 @@ struct mlx5_priv {
>   unsigned int reta_idx_n; /* RETA index size. */
>   struct mlx5_drop drop_queue; /* Flow drop queues. */
>   uint32_t flows; /* RTE Flow rules. */
> + rte_spinlock_t flow_lock;
>   uint32_t ctrl_flows; /* Control flow rules. */
>   rte_spinlock_t flow_list_lock;
>   struct mlx5_obj_ops obj_ops; /* HW objects operations. */ diff --git
> a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c index
> e9c0ddd..69d8159 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -5577,6 +5577,7 @@ flow_list_create(struct rte_eth_dev *dev, uint32_t
> *list,
>   external, hairpin_flow, error);
>   if (ret < 0)
>   goto error_before_hairpin_split;
> + rte_spinlock_lock(&priv->flow_lock);
>   flow = mlx5_ipool_zmalloc(priv->sh->ipool[MLX5_IPOOL_RTE_FLOW],
> &idx);
>   if (!flow) {
>   rte_errno = ENOMEM;
> @@ -5598,8 +5599,10 @@ flow_list_create(struct rte_eth_dev *dev, uint32_t
> *list,
>   memset(rss_desc, 0, offsetof(struct mlx5_flow_rss_desc, queue));
>   rss = flow_get_rss_action(p_actions_rx);
>   if (rss) {
> - if (flow_rss_workspace_adjust(wks, rss_desc, rss->queue_num))
> + if (flow_rss_workspace_adjust(wks, rss_desc, rss->queue_num))
> {
> + rte_spinlock_unlock(&priv->flow_lock);
>   return 0;
> + }
>   /*
>* The following information is required by
>* mlx5_flow_hashfields_adjust() in advance.
> @@ -5723,6 +5726,7 @@ flow_list_create(struct rte_eth_dev *dev, uint32_t
> *list,
>   __atomic_add_fetch(&tunnel->refctn, 1, __ATOMIC_RELAXED);
>   mlx5_free(default_miss_ctx.queue);
>   }
> + rte_spinlock_unlock(&priv->flow_lock);
>   return idx;
>  error:
>   MLX5_ASSERT(flow);
> @@ -5738,6 +5742,7 @@ flow_list_create(struct rte_eth_dev *dev, uint32_t
> *list,
>   wks->flow_nested_idx = 0;
>  error_before_hairpin_split:
>   rte_free(translated_actions);
> + rte_spinlock_unlock(&priv->flow_lock);
>   return 0;
>  }
> 
> @@ -5877,11 +5882,14 @@ flow_list_destroy(struct rte_eth_dev *dev,
> uint32_t *list,
> uint32_t flow_idx)
>  {
>   struct mlx5_priv *priv = dev->data->dev_private;
> + rte_spinlock_lock(&priv->flow_lock);
>   struct rte_flow *flow = mlx5_ipool_get(priv->sh->ipool
>  [MLX5_IPOOL_RTE_FLOW],
> flow_idx);
> 
> - if (!flow)
> + if (!flow) {
> + rte_spinlock_unlock(&priv->flow_lock);
>   return;
> + }
>   /*
>* Update RX queue flags only if port is started, otherwise it is
>* already clean.
> @@ -5908,6 +5916,7 @@ flow_list_destroy(struct rte_eth_dev *dev, uint32_t
> *list,
>   }
>   flow_mreg_del_copy_action(dev, flow);
>   mlx5_ipool_free(priv->sh->ipool[MLX5_IPOOL_RTE_FLOW], flow_idx);
> + rte_spinlock_unlock(&priv

[dpdk-dev] [PATCH v2] net/mlx5: fix sample and mirror use incorrect devices

[Suanming Mou]

The sample and mirror action objects are maintained on the list
shared between the ports belonging to the same multiport Infiniband
device(between representors).

The actions in the NIC steering domains might contain the references
to the sub-flow action objects created over the given port. The action
deletion might happen in the context of the different port and on the
deletion of referenced objects the incorrect port might be specified.
To avoid this we should save the port on what the sub-flow actions
were created and then use this saved port for sub-flow action release.

This commit saves the create device in the sample and mirror actions
struct to avoid using the incorrect port device in releasing.

Fixes: 19784141692e ("net/mlx5: make sample and mirror action thread safe")

Signed-off-by: Suanming Mou 
Reviewed-by: Jiawei Wang 
Acked-by: Viacheslav Ovsiienko 
---

v2:
 - Commit message updated.

---
 drivers/net/mlx5/linux/mlx5_os.c |  6 ++
 drivers/net/mlx5/mlx5_flow.h |  2 ++
 drivers/net/mlx5/mlx5_flow_dv.c  | 14 --
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c
index 4b7fff4..00f793b 100644
--- a/drivers/net/mlx5/linux/mlx5_os.c
+++ b/drivers/net/mlx5/linux/mlx5_os.c
@@ -250,15 +250,13 @@
 flow_dv_push_vlan_remove_cb);
/* Init sample action cache list. */
snprintf(s, sizeof(s), "%s_sample_action_cache", sh->ibdev_name);
-   mlx5_cache_list_init(&sh->sample_action_list, s, 0,
-&rte_eth_devices[priv->dev_data->port_id],
+   mlx5_cache_list_init(&sh->sample_action_list, s, 0, sh,
 flow_dv_sample_create_cb,
 flow_dv_sample_match_cb,
 flow_dv_sample_remove_cb);
/* Init dest array action cache list. */
snprintf(s, sizeof(s), "%s_dest_array_cache", sh->ibdev_name);
-   mlx5_cache_list_init(&sh->dest_array_list, s, 0,
-&rte_eth_devices[priv->dev_data->port_id],
+   mlx5_cache_list_init(&sh->dest_array_list, s, 0, sh,
 flow_dv_dest_array_create_cb,
 flow_dv_dest_array_match_cb,
 flow_dv_dest_array_remove_cb);
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index bccb973..70fa028 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -563,6 +563,7 @@ struct mlx5_flow_dv_sample_resource {
void *verbs_action; /**< Verbs sample action object. */
void **sub_actions; /**< Sample sub-action array. */
};
+   struct rte_eth_dev *dev; /**< Device registers the action. */
uint32_t idx; /** Sample object index. */
uint8_t ft_type; /** Flow Table Type */
uint32_t ft_id; /** Flow Table Level */
@@ -584,6 +585,7 @@ struct mlx5_flow_dv_dest_array_resource {
uint32_t idx; /** Destination array action object index. */
uint8_t ft_type; /** Flow Table Type */
uint8_t num_of_dest; /**< Number of destination actions. */
+   struct rte_eth_dev *dev; /**< Device registers the action. */
void *action; /**< Pointer to the rdma core action. */
struct mlx5_flow_sub_actions_idx sample_idx[MLX5_MAX_DEST_NUM];
/**< Action index resources. */
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 1f0a2ab..ee3a172 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -8757,6 +8757,7 @@ struct mlx5_cache_entry *
goto error;
}
cache_resource->idx = idx;
+   cache_resource->dev = dev;
return &cache_resource->entry;
 error:
if (cache_resource->ft_type == MLX5DV_FLOW_TABLE_TYPE_FDB &&
@@ -8919,6 +8920,7 @@ struct mlx5_cache_entry *
goto error;
}
cache_resource->idx = res_idx;
+   cache_resource->dev = dev;
for (idx = 0; idx < resource->num_of_dest; idx++)
mlx5_free(dest_attr[idx]);
return &cache_resource->entry;
@@ -11045,13 +11047,13 @@ struct mlx5_cache_entry *
 }
 
 void
-flow_dv_sample_remove_cb(struct mlx5_cache_list *list,
+flow_dv_sample_remove_cb(struct mlx5_cache_list *list __rte_unused,
 struct mlx5_cache_entry *entry)
 {
-   struct rte_eth_dev *dev = list->ctx;
-   struct mlx5_priv *priv = dev->data->dev_private;
struct mlx5_flow_dv_sample_resource *cache_resource =
container_of(entry, typeof(*cache_resource), entry);
+   struct rte_eth_dev *dev = cache_resource->dev;
+   struct mlx5_priv *priv = dev->data->dev_private;
 
if (cache_resource->verbs_action)
claim_zero(mlx5_glue->destroy_flow_action
@@ -11100,13 +11102,13 @@ struct mlx5_cache_entry *
 }
 
 void
-flow_dv_dest_array_remove_cb(struc

[dpdk-dev] [PATCH] net/mlx5: fix eCPRI item value with mask

[Bing Zhao]

When creating a flow with eCPRI item, the mask and the value are both
needed in order to build the matching criteria.

In the current implementation, the unused value bits clear operation
was missed when filling the mask and value fields. For the value, the
bits not required were not masked with the mask provided. Indeed,
this action is not mandatory. But when creating a flow in the root
table, the kernel driver got involved and a check would prevent this
flow from being created. The same flow could be created successfully
with the userspace rdma-core on the non-root tables.

An AND operation needs to be added to clear the unused bits in the
value when building the matching criteria. Then the same flow can be
created successfully no matter with kernel driver or with rdma-core.

Fixes: daa38a8924a0 ("net/mlx5: add flow translation of eCPRI header")
Cc: sta...@dpdk.org

Signed-off-by: Bing Zhao 
Acked-by: Viacheslav Ovsiienko 
---
 drivers/net/mlx5/mlx5_flow_dv.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 1f0a2ab..c7840da 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -7875,7 +7875,7 @@ flow_dv_translate_item_ecpri(struct rte_eth_dev *dev, 
void *matcher,
prog_sample_field_value_0);
/* Already big endian (network order) in the header. */
*(uint32_t *)dw_m = ecpri_m->hdr.common.u32;
-   *(uint32_t *)dw_v = ecpri_v->hdr.common.u32;
+   *(uint32_t *)dw_v = ecpri_v->hdr.common.u32 & ecpri_m->hdr.common.u32;
/* Sample#0, used for matching type, offset 0. */
MLX5_SET(fte_match_set_misc4, misc4_m,
 prog_sample_field_id_0, samples[0]);
@@ -7897,7 +7897,8 @@ flow_dv_translate_item_ecpri(struct rte_eth_dev *dev, 
void *matcher,
dw_v = MLX5_ADDR_OF(fte_match_set_misc4, misc4_v,
prog_sample_field_value_1);
*(uint32_t *)dw_m = ecpri_m->hdr.dummy[0];
-   *(uint32_t *)dw_v = ecpri_v->hdr.dummy[0];
+   *(uint32_t *)dw_v = ecpri_v->hdr.dummy[0] &
+   ecpri_m->hdr.dummy[0];
/* Sample#1, to match message body, offset 4. */
MLX5_SET(fte_match_set_misc4, misc4_m,
 prog_sample_field_id_1, samples[1]);
-- 
2.5.5

[dpdk-dev] ovs-vswitchd with DPDK crashed when guest VM restarts network service

[Alex Yeh (ayeh)]

Hi,
   We are seeing a ovs-vswitchd service crash with segfault in the 
librte_vhost library when a DPDK application within a guest VM is stopped.

   We are using OVS 2.11.1 on CentOS 7.6 (3.10.0-1062 Linux kernel) 
with DPDK 18.11.2.

   We are using OVS-DPDK on the host and the guest VM is running a 
DPDK application. With some traffic, if the application service within the VM 
is restarted, then OVS crashes.

   This crash is not seen if the guest VM is restarted (instead of 
stopping the application within the VM).

   The crash trackback (attached below) points to the 
rte_memcpy_generic() function in rte_memcpy.h. It looks like the crash occurs 
when vhost is trying to dequeue the packets from the guest VM (as the 
application in the guest VM has stopped and the huge pages are returned to the 
guest kernel).

   We have tried enabling iommu in ovs by setting
"other_config:vhost-iommu-support=true" and enabling iommu in qemu using the 
following configuration in the guest domain XML:



   With iommu enabled ovs-vswitchd still crashes when guest VM 
restarts the network service.

   Is this a known problem? Anyone else seen a crash like this?  
How can we protect the ovs-vswitchd from crashing when a guest VM restarts the 
network application or service?

Thanks
Alex


Log:
Oct 7 19:54:16 Branch81-Bravo kernel: [2245909.596635] pmd16[25721]: segfault 
at 7f4d1d733000 ip 7f4d2ae5d066 sp 7f4d1ce65618 error 4 in 
librte_vhost.so.4[7f4d2ae52000+1a000]
Oct 7 19:54:19 Branch81-Bravo systemd[1]: ovs-vswitchd.service: main process 
exited, code=killed, status=11/SEGV

Environment:
CentOs 7.6.1810
openvswitch-2.11.1-1.el7.centos.x86_64
openvswitch-kmod-2.11.1-1.el7.centos.x86_64
dpdk-18.11-2.el7.centos.x86_64
3.10.0-1062.4.1.el7.x86_64
qemu-kvm-ev-2.12.0-18.el7.centos_6.1.1

Core dump trace:
(gdb) bt
#-1 0x7205602e in rte_memcpy_generic (dst=,
src=0x7fffcef3607c, n=)
at /usr/src/debug/dpdk-18.11/x86_64-native-linuxapp-gcc/include/rte_memcpy.h:793
Backtrace stopped: Cannot access memory at address 0x720558f0

(gdb) list *0x7205602e
0x7205602e is in rte_memcpy_generic 
(/usr/src/debug/dpdk-18.11/x86_64-native-linuxapp-gcc/include/rte_memcpy.h:793).
788 }
789
790 /**
791 * For copy with unaligned load
792 */
793 MOVEUNALIGNED_LEFT47(dst, src, n, srcofs);
794
795 /**
796 * Copy whatever left
797 */

(gdb) list *0x7205c192
0x7205c192 is in rte_vhost_dequeue_burst 
(/usr/src/debug/dpdk-18.11/lib/librte_vhost/virtio_net.c:1192).
1187 * In zero copy mode, one mbuf can only reference data
1188 * for one or partial of one desc buff.
1189 */
1190 mbuf_avail = cpy_len;
1191 } else {
1192 if (likely(cpy_len > MAX_BATCH_LEN ||
1193 vq->batch_copy_nb_elems >= vq->size ||
1194 (hdr && cur == m))) {
1195 rte_memcpy(rte_pktmbuf_mtod_offset(cur, void *,
1196 mbuf_offset),
(gdb)

Re: [dpdk-dev] [PATCH][v2] net/af_xdp: avoid to unnecessary allocation and free mbuf in rx path

[Li,Rongqing]

> 
> Apologies for the delay, I missed your reply Li.
> With the data you've provided I think the patch is justified.
> I think the rollback requires some explanation in the code as it may not be
> immediately clear what is happening.
> I suggest a v3 with either a comment above the rollback, or a new function as
> described in my previous mail, also with a comment.
> 

Ok, we will send V3

Thanks

-Li


> Thanks for the patch.
> 
> Ciara

[dpdk-dev] Question about recent change to rte_mbuf struct - user data and udata64 feels (breaks SPDK)

[Luse, Paul E]

Hi,

Recently this patch 
https://github.com/DPDK/dpdk/commit/5284adad3e95025f9901869f581c8c04ea642d32  
made the following change:

* mbuf: Removed the unioned fields ``userdata`` and ``udata64``
  from the structure ``rte_mbuf``. It is replaced with dynamic fields.

Which breaks the SPDK project’s crypto and compression capabilities as we use 
userdata to store context for our operation so it can be retrieved upcon 
completion of the operation.  It’s not clear to me that we are safe to use the 
fields that were added:

uint64_t dynfield1[2]; /**< Reserved for dynamic fields. */
uint64_t dynfield1[3]; /**< Reserved for dynamic fields. */

based on the comments.  Can someone please advise, why was this done and can we 
use these fields?

Thanks,
Paul

Re: [dpdk-dev] [PATCH v3] gso: add VXLAN UDP GSO support

[Hu, Jiayu]

> -Original Message-
> From: yang_y...@163.com 
> Sent: Monday, November 16, 2020 9:11 AM
> To: dev@dpdk.org
> Cc: Hu, Jiayu ; Ananyev, Konstantin
> ; tho...@monjalon.net;
> yangy...@inspur.com; yang_y...@163.com
> Subject: [PATCH v3] gso: add VXLAN UDP GSO support
> 
> From: Yi Yang 
> 
> Many NICs can't offload VXLAN UFO, so it is very important
> to do VXLAN UDP GSO by software to improve VM-to-VM UDP
> performance, especially for the case that VM MTU is just
> 1500 but not 9000.
> 
> With this enabled in DPDK, OVS DPDK can leverage it to
> improve VM-to-VM UDP performance, performance gain is very
> huge, over 2 times.

GSO means software segmentation, so we don't say "VXLAN UDP GSO
by software".

I think it's better to make the commit message and log more accurate.
For example:
gso: add VxLAN UDP/IPv4 support

As some NICs do not support segmentation for VxLAN-encapsulated
UDP/IPv4 packets, this patch adds VxLAN UDP/IPv4 GSO support.
With leveraging VxLAN UDP/IPv4 GSO, OVS DPDK can significantly
improve performance for VxLAN UDP/IPv4 traffic.

Thanks,
Jiayu

> 
> Signed-off-by: Yi Yang 
> ---
> Changelog:
> 
> v2 -> v3:
>   - Correct gso type check for UDP TSO.
> 
> v1 -> v2:
>   - Remove condition check for outer udp header because it
> is always true for VXLAN.
>   - Remove inner udp header update because it is wrong and
> unnecessary.
> 
> ---
>  .../generic_segmentation_offload_lib.rst   | 14 ++--
>  doc/guides/rel_notes/release_20_11.rst |  4 +
>  lib/librte_gso/gso_common.h|  5 ++
>  lib/librte_gso/gso_tunnel_udp4.c   | 97 
> ++
>  lib/librte_gso/gso_tunnel_udp4.h   | 44 ++
>  lib/librte_gso/meson.build |  2 +-
>  lib/librte_gso/rte_gso.c   |  8 ++
>  7 files changed, 166 insertions(+), 8 deletions(-)
>  create mode 100644 lib/librte_gso/gso_tunnel_udp4.c
>  create mode 100644 lib/librte_gso/gso_tunnel_udp4.h
> 
> diff --git a/doc/guides/prog_guide/generic_segmentation_offload_lib.rst
> b/doc/guides/prog_guide/generic_segmentation_offload_lib.rst
> index ad91c6e..c1d06e1 100644
> --- a/doc/guides/prog_guide/generic_segmentation_offload_lib.rst
> +++ b/doc/guides/prog_guide/generic_segmentation_offload_lib.rst
> @@ -46,7 +46,7 @@ Limitations
>   - TCP
>   - UDP
>   - VxLAN
> - - GRE
> + - GRE TCP
> 
>See `Supported GSO Packet Types`_ for further details.
> 
> @@ -157,14 +157,14 @@ does not modify it during segmentation. Therefore,
> after UDP GSO, only the
>  first output packet has the original UDP header, and others just have l2
>  and l3 headers.
> 
> -VxLAN GSO
> -~
> +VxLAN IPv4 GSO
> +~~
>  VxLAN packets GSO supports segmentation of suitably large VxLAN packets,
> -which contain an outer IPv4 header, inner TCP/IPv4 headers, and optional
> -inner and/or outer VLAN tag(s).
> +which contain an outer IPv4 header, inner TCP/IPv4 or UDP/IPv4 headers,
> and
> +optional inner and/or outer VLAN tag(s).
> 
> -GRE GSO
> -~~~
> +GRE TCP/IPv4 GSO
> +
>  GRE GSO supports segmentation of suitably large GRE packets, which
> contain
>  an outer IPv4 header, inner TCP/IPv4 headers, and an optional VLAN tag.
> 
> diff --git a/doc/guides/rel_notes/release_20_11.rst
> b/doc/guides/rel_notes/release_20_11.rst
> index 24cedba..bd13c5f 100644
> --- a/doc/guides/rel_notes/release_20_11.rst
> +++ b/doc/guides/rel_notes/release_20_11.rst
> @@ -422,6 +422,10 @@ New Features
>leverage IOAT DMA channel with vhost asynchronous APIs.
>See the :doc:`../sample_app_ug/vhost` for more details.
> 
> +* **Added VxLAN UDP/IPv4 GSO support.**
> +
> +  Added inner UDP/IPv4 support for VxLAN IPv4 GSO.
> +
> 
>  Removed Items
>  -
> diff --git a/lib/librte_gso/gso_common.h b/lib/librte_gso/gso_common.h
> index a0b8343..4d5f303 100644
> --- a/lib/librte_gso/gso_common.h
> +++ b/lib/librte_gso/gso_common.h
> @@ -26,6 +26,11 @@
>   (PKT_TX_TCP_SEG | PKT_TX_IPV4 | PKT_TX_OUTER_IPV4 | \
>PKT_TX_TUNNEL_VXLAN))
> 
> +#define IS_IPV4_VXLAN_UDP4(flag) (((flag) & (PKT_TX_UDP_SEG |
> PKT_TX_IPV4 | \
> + PKT_TX_OUTER_IPV4 |
> PKT_TX_TUNNEL_MASK)) == \
> + (PKT_TX_UDP_SEG | PKT_TX_IPV4 | PKT_TX_OUTER_IPV4 | \
> +  PKT_TX_TUNNEL_VXLAN))
> +
>  #define IS_IPV4_GRE_TCP4(flag) (((flag) & (PKT_TX_TCP_SEG | PKT_TX_IPV4
> | \
>   PKT_TX_OUTER_IPV4 |
> PKT_TX_TUNNEL_MASK)) == \
>   (PKT_TX_TCP_SEG | PKT_TX_IPV4 | PKT_TX_OUTER_IPV4 | \
> diff --git a/lib/librte_gso/gso_tunnel_udp4.c
> b/lib/librte_gso/gso_tunnel_udp4.c
> new file mode 100644
> index 000..1fc7a8d
> --- /dev/null
> +++ b/lib/librte_gso/gso_tunnel_udp4.c
> @@ -0,0 +1,97 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2020 Inspur Corporation
> + */
> +
> +#include "gso_common.h"
> +#include "gso_tunnel_udp4.h"
> +
> +#defi

Re: [dpdk-dev] [PATCH] net/i40e: fix conflict with multi-driver

[Guo, Jia]

Hi, beilei

> -Original Message-
> From: Xing, Beilei 
> Sent: Thursday, November 19, 2020 2:16 PM
> To: dev@dpdk.org
> Cc: Guo, Jia ; Xing, Beilei ;
> sta...@dpdk.org
> Subject: [PATCH] net/i40e: fix conflict with multi-driver
> 

Seems that this patch both handle multi-driver and none multi-driver.
I am not sure if it need to rename to a better name, you choice.
" net/i40e: fix global register recovery"?

> From: Beilei Xing 
> 
> PMD configures the global register I40E_GLINT_CTL during device
> initialization to work around the Rx write back issue. But when a device is
> bound from DPDK to kernel, the global register is not recovered to the
> original state, it will cause kernel driver performance drop issue.
> This patch fixes this issue.
> 
> Fixes: be6c228d4da3 ("i40e: support Rx interrupt")

If the issue is root cause that the miss-pair automask configure and automask 
clear, 
do you think it will be better to add one more fixes tag  as below? 
Fixes: 4ab831449a1c ("net/i40e: fix interrupt conflict with multi-driver ")

> Cc: sta...@dpdk.org
> 
> Signed-off-by: Beilei Xing 
> ---
>  drivers/net/i40e/i40e_ethdev.c | 17 +
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index f54769c29d..2cb18ecc03 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -763,6 +763,21 @@ static inline void i40e_config_automask(struct
> i40e_pf *pf)
>   I40E_WRITE_REG(hw, I40E_GLINT_CTL, val);  }
> 
> +static inline void i40e_clear_automask(struct i40e_pf *pf) {
> + struct i40e_hw *hw = I40E_PF_TO_HW(pf);
> + uint32_t val;
> +
> + val = I40E_READ_REG(hw, I40E_GLINT_CTL);
> + val &= ~(I40E_GLINT_CTL_DIS_AUTOMASK_PF0_MASK |
> +  I40E_GLINT_CTL_DIS_AUTOMASK_VF0_MASK);
> +
> + if (!pf->support_multi_driver)
> + val &= ~I40E_GLINT_CTL_DIS_AUTOMASK_N_MASK;
> +
> + I40E_WRITE_REG(hw, I40E_GLINT_CTL, val); }
> +
>  #define I40E_FLOW_CONTROL_ETHERTYPE  0x8808
> 
>  /*
> @@ -2741,6 +2756,8 @@ i40e_dev_close(struct rte_eth_dev *dev)
>   /* Remove all Traffic Manager configuration */
>   i40e_tm_conf_uninit(dev);
> 
> + i40e_clear_automask(pf);
> +
>   hw->adapter_closed = 1;
>   return ret;
>  }
> --
> 2.26.2

[dpdk-dev] [PATCH v4] gso: add VXLAN UDP/IPv4 support

[yang_y_yi]

From: Yi Yang 

As most NICs do not support segmentation for VXLAN-encapsulated
UDP/IPv4 packets, this patch adds VXLAN UDP/IPv4 GSO support.
OVS DPDK can significantly improve VXLAN UDP/IPv4 performance by
VXLAN UDP/IPv4 GSO.

Signed-off-by: Yi Yang 
---
Changelog:

v3 -> v4:
  - Use more precise commit subject and log
  - Correct VxLAN to VXLAN (https://tools.ietf.org/html/rfc7348)

v2 -> v3:
  - Correct gso type check for UDP TSO.

v1 -> v2:
  - Remove condition check for outer udp header because it
is always true for VXLAN.
  - Remove inner udp header update because it is wrong and
unnecessary.

---
 .../generic_segmentation_offload_lib.rst   | 18 ++--
 doc/guides/rel_notes/release_20_11.rst |  4 +
 lib/librte_gso/gso_common.h|  5 ++
 lib/librte_gso/gso_tunnel_udp4.c   | 97 ++
 lib/librte_gso/gso_tunnel_udp4.h   | 44 ++
 lib/librte_gso/meson.build |  2 +-
 lib/librte_gso/rte_gso.c   |  8 ++
 7 files changed, 168 insertions(+), 10 deletions(-)
 create mode 100644 lib/librte_gso/gso_tunnel_udp4.c
 create mode 100644 lib/librte_gso/gso_tunnel_udp4.h

diff --git a/doc/guides/prog_guide/generic_segmentation_offload_lib.rst 
b/doc/guides/prog_guide/generic_segmentation_offload_lib.rst
index ad91c6e..7bff0ae 100644
--- a/doc/guides/prog_guide/generic_segmentation_offload_lib.rst
+++ b/doc/guides/prog_guide/generic_segmentation_offload_lib.rst
@@ -45,8 +45,8 @@ Limitations
 
  - TCP
  - UDP
- - VxLAN
- - GRE
+ - VXLAN
+ - GRE TCP
 
   See `Supported GSO Packet Types`_ for further details.
 
@@ -157,14 +157,14 @@ does not modify it during segmentation. Therefore, after 
UDP GSO, only the
 first output packet has the original UDP header, and others just have l2
 and l3 headers.
 
-VxLAN GSO
-~
-VxLAN packets GSO supports segmentation of suitably large VxLAN packets,
-which contain an outer IPv4 header, inner TCP/IPv4 headers, and optional
-inner and/or outer VLAN tag(s).
+VXLAN IPv4 GSO
+~~
+VXLAN packets GSO supports segmentation of suitably large VXLAN packets,
+which contain an outer IPv4 header, inner TCP/IPv4 or UDP/IPv4 headers, and
+optional inner and/or outer VLAN tag(s).
 
-GRE GSO
-~~~
+GRE TCP/IPv4 GSO
+
 GRE GSO supports segmentation of suitably large GRE packets, which contain
 an outer IPv4 header, inner TCP/IPv4 headers, and an optional VLAN tag.
 
diff --git a/doc/guides/rel_notes/release_20_11.rst 
b/doc/guides/rel_notes/release_20_11.rst
index 24cedba..04aba33 100644
--- a/doc/guides/rel_notes/release_20_11.rst
+++ b/doc/guides/rel_notes/release_20_11.rst
@@ -422,6 +422,10 @@ New Features
   leverage IOAT DMA channel with vhost asynchronous APIs.
   See the :doc:`../sample_app_ug/vhost` for more details.
 
+* **Added VXLAN UDP/IPv4 GSO support.**
+
+  Added inner UDP/IPv4 support for VXLAN IPv4 GSO.
+
 
 Removed Items
 -
diff --git a/lib/librte_gso/gso_common.h b/lib/librte_gso/gso_common.h
index a0b8343..4d5f303 100644
--- a/lib/librte_gso/gso_common.h
+++ b/lib/librte_gso/gso_common.h
@@ -26,6 +26,11 @@
(PKT_TX_TCP_SEG | PKT_TX_IPV4 | PKT_TX_OUTER_IPV4 | \
 PKT_TX_TUNNEL_VXLAN))
 
+#define IS_IPV4_VXLAN_UDP4(flag) (((flag) & (PKT_TX_UDP_SEG | PKT_TX_IPV4 | \
+   PKT_TX_OUTER_IPV4 | PKT_TX_TUNNEL_MASK)) == \
+   (PKT_TX_UDP_SEG | PKT_TX_IPV4 | PKT_TX_OUTER_IPV4 | \
+PKT_TX_TUNNEL_VXLAN))
+
 #define IS_IPV4_GRE_TCP4(flag) (((flag) & (PKT_TX_TCP_SEG | PKT_TX_IPV4 | \
PKT_TX_OUTER_IPV4 | PKT_TX_TUNNEL_MASK)) == \
(PKT_TX_TCP_SEG | PKT_TX_IPV4 | PKT_TX_OUTER_IPV4 | \
diff --git a/lib/librte_gso/gso_tunnel_udp4.c b/lib/librte_gso/gso_tunnel_udp4.c
new file mode 100644
index 000..1fc7a8d
--- /dev/null
+++ b/lib/librte_gso/gso_tunnel_udp4.c
@@ -0,0 +1,97 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2020 Inspur Corporation
+ */
+
+#include "gso_common.h"
+#include "gso_tunnel_udp4.h"
+
+#define IPV4_HDR_MF_BIT (1U << 13)
+
+static void
+update_tunnel_ipv4_udp_headers(struct rte_mbuf *pkt, struct rte_mbuf **segs,
+  uint16_t nb_segs)
+{
+   struct rte_ipv4_hdr *ipv4_hdr;
+   uint16_t outer_id, inner_id, tail_idx, i, length;
+   uint16_t outer_ipv4_offset, inner_ipv4_offset;
+   uint16_t outer_udp_offset;
+   uint16_t frag_offset = 0, is_mf;
+
+   outer_ipv4_offset = pkt->outer_l2_len;
+   outer_udp_offset = outer_ipv4_offset + pkt->outer_l3_len;
+   inner_ipv4_offset = outer_udp_offset + pkt->l2_len;
+
+   /* Outer IPv4 header. */
+   ipv4_hdr = (struct rte_ipv4_hdr *)(rte_pktmbuf_mtod(pkt, char *) +
+   outer_ipv4_offset);
+   outer_id = rte_be_to_cpu_16(ipv4_hdr->packet_id);
+
+   /* Inner IPv4 header. */
+   ipv4_hdr = (struct rte_ipv4_hdr

Re: [dpdk-dev] [PATCH v4] gso: add VXLAN UDP/IPv4 support

[Hu, Jiayu]

Acked-by: Jiayu Hu 

> -Original Message-
> From: yang_y...@163.com 
> Sent: Thursday, November 19, 2020 2:44 PM
> To: dev@dpdk.org
> Cc: Hu, Jiayu ; tho...@monjalon.net;
> yangy...@inspur.com; yang_y...@163.com
> Subject: [PATCH v4] gso: add VXLAN UDP/IPv4 support
> 
> From: Yi Yang 
> 
> As most NICs do not support segmentation for VXLAN-encapsulated
> UDP/IPv4 packets, this patch adds VXLAN UDP/IPv4 GSO support.
> OVS DPDK can significantly improve VXLAN UDP/IPv4 performance by
> VXLAN UDP/IPv4 GSO.
> 
> Signed-off-by: Yi Yang 
> ---
> Changelog:
> 
> v3 -> v4:
>   - Use more precise commit subject and log
>   - Correct VxLAN to VXLAN (https://tools.ietf.org/html/rfc7348)
> 
> v2 -> v3:
>   - Correct gso type check for UDP TSO.
> 
> v1 -> v2:
>   - Remove condition check for outer udp header because it
> is always true for VXLAN.
>   - Remove inner udp header update because it is wrong and
> unnecessary.
> 
> ---
>  .../generic_segmentation_offload_lib.rst   | 18 ++--
>  doc/guides/rel_notes/release_20_11.rst |  4 +
>  lib/librte_gso/gso_common.h|  5 ++
>  lib/librte_gso/gso_tunnel_udp4.c   | 97 
> ++
>  lib/librte_gso/gso_tunnel_udp4.h   | 44 ++
>  lib/librte_gso/meson.build |  2 +-
>  lib/librte_gso/rte_gso.c   |  8 ++
>  7 files changed, 168 insertions(+), 10 deletions(-)
>  create mode 100644 lib/librte_gso/gso_tunnel_udp4.c
>  create mode 100644 lib/librte_gso/gso_tunnel_udp4.h
> 
> diff --git a/doc/guides/prog_guide/generic_segmentation_offload_lib.rst
> b/doc/guides/prog_guide/generic_segmentation_offload_lib.rst
> index ad91c6e..7bff0ae 100644
> --- a/doc/guides/prog_guide/generic_segmentation_offload_lib.rst
> +++ b/doc/guides/prog_guide/generic_segmentation_offload_lib.rst
> @@ -45,8 +45,8 @@ Limitations
> 
>   - TCP
>   - UDP
> - - VxLAN
> - - GRE
> + - VXLAN
> + - GRE TCP
> 
>See `Supported GSO Packet Types`_ for further details.
> 
> @@ -157,14 +157,14 @@ does not modify it during segmentation. Therefore,
> after UDP GSO, only the
>  first output packet has the original UDP header, and others just have l2
>  and l3 headers.
> 
> -VxLAN GSO
> -~
> -VxLAN packets GSO supports segmentation of suitably large VxLAN packets,
> -which contain an outer IPv4 header, inner TCP/IPv4 headers, and optional
> -inner and/or outer VLAN tag(s).
> +VXLAN IPv4 GSO
> +~~
> +VXLAN packets GSO supports segmentation of suitably large VXLAN packets,
> +which contain an outer IPv4 header, inner TCP/IPv4 or UDP/IPv4 headers,
> and
> +optional inner and/or outer VLAN tag(s).
> 
> -GRE GSO
> -~~~
> +GRE TCP/IPv4 GSO
> +
>  GRE GSO supports segmentation of suitably large GRE packets, which
> contain
>  an outer IPv4 header, inner TCP/IPv4 headers, and an optional VLAN tag.
> 
> diff --git a/doc/guides/rel_notes/release_20_11.rst
> b/doc/guides/rel_notes/release_20_11.rst
> index 24cedba..04aba33 100644
> --- a/doc/guides/rel_notes/release_20_11.rst
> +++ b/doc/guides/rel_notes/release_20_11.rst
> @@ -422,6 +422,10 @@ New Features
>leverage IOAT DMA channel with vhost asynchronous APIs.
>See the :doc:`../sample_app_ug/vhost` for more details.
> 
> +* **Added VXLAN UDP/IPv4 GSO support.**
> +
> +  Added inner UDP/IPv4 support for VXLAN IPv4 GSO.
> +
> 
>  Removed Items
>  -
> diff --git a/lib/librte_gso/gso_common.h b/lib/librte_gso/gso_common.h
> index a0b8343..4d5f303 100644
> --- a/lib/librte_gso/gso_common.h
> +++ b/lib/librte_gso/gso_common.h
> @@ -26,6 +26,11 @@
>   (PKT_TX_TCP_SEG | PKT_TX_IPV4 | PKT_TX_OUTER_IPV4 | \
>PKT_TX_TUNNEL_VXLAN))
> 
> +#define IS_IPV4_VXLAN_UDP4(flag) (((flag) & (PKT_TX_UDP_SEG |
> PKT_TX_IPV4 | \
> + PKT_TX_OUTER_IPV4 |
> PKT_TX_TUNNEL_MASK)) == \
> + (PKT_TX_UDP_SEG | PKT_TX_IPV4 | PKT_TX_OUTER_IPV4 | \
> +  PKT_TX_TUNNEL_VXLAN))
> +
>  #define IS_IPV4_GRE_TCP4(flag) (((flag) & (PKT_TX_TCP_SEG | PKT_TX_IPV4
> | \
>   PKT_TX_OUTER_IPV4 |
> PKT_TX_TUNNEL_MASK)) == \
>   (PKT_TX_TCP_SEG | PKT_TX_IPV4 | PKT_TX_OUTER_IPV4 | \
> diff --git a/lib/librte_gso/gso_tunnel_udp4.c
> b/lib/librte_gso/gso_tunnel_udp4.c
> new file mode 100644
> index 000..1fc7a8d
> --- /dev/null
> +++ b/lib/librte_gso/gso_tunnel_udp4.c
> @@ -0,0 +1,97 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2020 Inspur Corporation
> + */
> +
> +#include "gso_common.h"
> +#include "gso_tunnel_udp4.h"
> +
> +#define IPV4_HDR_MF_BIT (1U << 13)
> +
> +static void
> +update_tunnel_ipv4_udp_headers(struct rte_mbuf *pkt, struct rte_mbuf
> **segs,
> +uint16_t nb_segs)
> +{
> + struct rte_ipv4_hdr *ipv4_hdr;
> + uint16_t outer_id, inner_id, tail_idx, i, length;
> + uint16_t outer_ipv4_offset, inner_ipv4_offset;
> + uint16_t 

[dpdk-dev] [PATCH v3] usertools/devbind: fix binding for built-in kernel drivers

[Yongxin Liu]

A driver can be loaded as a dynamic module or a built-in module.
In commit 681a67288655 ("usertools: check if module is loaded
before binding"), script only checks modules in /sys/module/.

However, for built-in kernel driver, it only shows up in /sys/module/,
if it has a version or at least one parameter. So add check for
modules in /lib/modules/$(uname -r)/modules.builtin.

Thanks for Anatoly Burakov's advice.

Signed-off-by: Yongxin Liu 
---

v3:
 - Add built-in module list in loaded_modules for checking
   instead of removing error check.

v2:
 - fix git commit description style in commit log
 - fix typo spelling

---
 usertools/dpdk-devbind.py | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
index 99112b7ab..5b34ccd2a 100755
--- a/usertools/dpdk-devbind.py
+++ b/usertools/dpdk-devbind.py
@@ -181,7 +181,13 @@ def module_is_loaded(module):
 
 loaded_modules = sysfs_mods
 
-return module in sysfs_mods
+# add built-in modules as loaded
+builtin_mods = subprocess.check_output(["cat /lib/modules/$(uname 
-r)/modules.builtin"], shell=True).splitlines()
+for mod in builtin_mods:
+mod_name = os.path.basename(mod.decode("utf8")).split(".ko", 1)
+loaded_modules.append(mod_name[0])
+
+return module in loaded_modules
 
 
 def check_modules():
-- 
2.14.4

Re: [dpdk-dev] [RFC] remove unused functions

[Xu, Rosen]

Hi,

> -Original Message-
> From: Yigit, Ferruh 
> Sent: Thursday, November 19, 2020 11:53
> To: Jerin Jacob ; Dumitrescu, Cristian
> ; Hemant Agrawal
> ; Sachin Saxena ;
> Ray Kinsella ; Neil Horman ; Xu,
> Rosen ; Wu, Jingjing ; Xing,
> Beilei ; Nithin Dabilpuram
> ; Ajit Khaparde
> ; Raveendra Padasalagi
> ; Vikas Gupta
> ; Gagandeep Singh ;
> Somalapuram Amaranath ; Akhil Goyal
> ; Jay Zhou ; McDaniel,
> Timothy ; Ma, Liang J ;
> Mccarthy, Peter ; Shepard Siegel
> ; Ed Czeck ;
> John Miller ; Igor Russkikh
> ; Pavel Belous ;
> Rasesh Mody ; Shahed Shaikh
> ; Somnath Kotur
> ; Chas Williams ; Min Hu
> (Connor) ; Rahul Lakkireddy
> ; Guo, Jia ; Wang,
> Haiyue ; Marcin Wojtas ;
> Michal Krawczyk ; Guy Tzalik ;
> Evgeny Schemeilin ; Igor Chauskin
> ; Zhang, Qi Z ; Wang, Xiao W
> ; Yang, Qiming ; Alfredo
> Cardigliano ; Matan Azrad ;
> Shahaf Shuler ; Viacheslav Ovsiienko
> ; Zyta Szpak ; Liron Himi
> ; Stephen Hemminger ; K.
> Y. Srinivasan ; Haiyang Zhang
> ; Long Li ; Heinrich Kuhn
> ; Harman Kalra ;
> Kiran Kumar K ; Andrew Rybchenko
> ; Singh, Jasvinder
> ; Jiawen Wu ; Jian
> Wang ; Zhang, Tianfei
> ; Ori Kam ; Guy Kaneti
> ; Burakov, Anatoly ;
> Maxime Coquelin ; Xia, Chenbo
> 
> Cc: Yigit, Ferruh ; dev@dpdk.org
> Subject: [RFC] remove unused functions
> 
> Removing unused functions, reported by cppcheck.
> 
> Easy way to remove clutter, since the code is already in the git repo,
> they can be added back when needed.
> 
> Signed-off-by: Ferruh Yigit 
> ---
>  drivers/bus/ifpga/ifpga_common.c  |   23 -
>  drivers/bus/ifpga/ifpga_common.h  |3 -
> 

> diff --git a/drivers/bus/ifpga/ifpga_common.c
> b/drivers/bus/ifpga/ifpga_common.c
> index 78e2eaee4e..7281b169d0 100644
> --- a/drivers/bus/ifpga/ifpga_common.c
> +++ b/drivers/bus/ifpga/ifpga_common.c
> @@ -52,29 +52,6 @@ int rte_ifpga_get_integer32_arg(const char *key
> __rte_unused,
> 
>   return 0;
>  }
> -int ifpga_get_integer64_arg(const char *key __rte_unused,
> - const char *value, void *extra_args)
> -{
> - if (!value || !extra_args)
> - return -EINVAL;
> -
> - *(uint64_t *)extra_args = strtoull(value, NULL, 0);
> -
> - return 0;
> -}
> -int ifpga_get_unsigned_long(const char *str, int base)
> -{
> - unsigned long num;
> - char *end = NULL;
> -
> - errno = 0;
> -
> - num = strtoul(str, &end, base);
> - if ((str[0] == '\0') || (end == NULL) || (*end != '\0') || (errno != 0))
> - return -1;
> -
> - return num;
> -}
> 
>  int ifpga_afu_id_cmp(const struct rte_afu_id *afu_id0,
>   const struct rte_afu_id *afu_id1)
> diff --git a/drivers/bus/ifpga/ifpga_common.h
> b/drivers/bus/ifpga/ifpga_common.h
> index f9254b9d5d..44381eb78d 100644
> --- a/drivers/bus/ifpga/ifpga_common.h
> +++ b/drivers/bus/ifpga/ifpga_common.h
> @@ -9,9 +9,6 @@ int rte_ifpga_get_string_arg(const char *key
> __rte_unused,
>   const char *value, void *extra_args);
>  int rte_ifpga_get_integer32_arg(const char *key __rte_unused,
>   const char *value, void *extra_args);
> -int ifpga_get_integer64_arg(const char *key __rte_unused,
> - const char *value, void *extra_args);
> -int ifpga_get_unsigned_long(const char *str, int base);
>  int ifpga_afu_id_cmp(const struct rte_afu_id *afu_id0,
>   const struct rte_afu_id *afu_id1);
> 
> 2.26.2

Reviewed-by: Rosen Xu 

Re: [dpdk-dev] [PATCH v2] usertools/devbind: fix binding for built-in 1kernel drivers

[Liu, Yongxin]

> -Original Message-
> From: Burakov, Anatoly 
> Sent: Thursday, November 19, 2020 00:28
> To: Liu, Yongxin ; dev@dpdk.org;
> tho...@monjalon.net
> Subject: Re: [dpdk-dev] [PATCH v2] usertools/devbind: fix binding for
> built-in 1kernel drivers
> 
> 
> On 18-Nov-20 2:58 AM, Yongxin Liu wrote:
> > In commit 681a67288655 ("usertools: check if module is loaded before
> > binding"), script will exit if no driver is found in /sys/module/.
> >
> > However, for built-in kernel driver, /sys/module/MODULENAME only shows
> > up if it has a version or at least one parameter. Take ixgbe for
> > example, after kernel commit 34a2a3b83e2c ("net/intel: remove driver
> > versions from Intel drivers"), and if ixgbe is built directly into
> > kernel, there is no ixgbe folder in /sys/module. So the devbind script
> > should not exit.
> >
> > Signed-off-by: Yongxin Liu 
> > ---
> >
> > v2:
> >   - fix git commit description style in commit log
> >   - fix typo spelling
> >
> > ---
> >   usertools/dpdk-devbind.py | 4 
> >   1 file changed, 4 deletions(-)
> >
> > diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
> > index 99112b7ab..f3c0d9814 100755
> > --- a/usertools/dpdk-devbind.py
> > +++ b/usertools/dpdk-devbind.py
> > @@ -530,10 +530,6 @@ def bind_all(dev_list, driver, force=False):
> >   # driver generated error - it's not a valid device ID, so all
> is well
> >   pass
> >
> > -# check if we're attempting to bind to a driver that isn't loaded
> > -if not module_is_loaded(driver.replace('-','_')):
> > -sys.exit("Error: Driver '%s' is not loaded." % driver)
> > -
> 
> I believe there is a way to check if module is built-in, can't we use that?
> We could keep a list of built-in modules of interest that we can get from:
> 
> cat /lib/modules/$(uname -r)/modules.builtin
> 
> It's a bit more changes, but this is better than just removing the error
> check.

Thanks Anatoly for your advice.
I have sent v3. Please review.

/Yongxin

> 
> >   try:
> >   dev_list = map(dev_id_from_dev_name, dev_list)
> >   except ValueError as ex:
> >
> 
> 
> --
> Thanks,
> Anatoly