[dpdk-dev] [PATCH] net/vhost: restore pseudo TSO support

[David Marchand]

The net/vhost pmd does not comply with the ethdev offload API as it does
not report rx/tx offload capabilities wrt TSO and checksum offloading.
On the other hand, the net/vhost pmd lets guest negotiates TSO and
checksum offloading.

Changing the behavior for rx/tx offload flags handling won't
improve/fix this situation and will break applications that might have
been relying on implicit support of TSO in this driver.

Revert this behavior change until we have a complete fix.

Fixes: ca7036b4af3a ("vhost: fix offload flags in Rx path")

Signed-off-by: David Marchand 
---
The full fix requires:
- reporting rx/tx_offload_capa,
- supporting sw TSO and checksums for the case when a virtual machine
  negotiates this feature, but the application does not configure
  DEV_[RT]X_OFFLOAD_* appropriate flags,

---
 drivers/net/vhost/rte_eth_vhost.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/vhost/rte_eth_vhost.c 
b/drivers/net/vhost/rte_eth_vhost.c
index 8524898661..a202931e9a 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -1505,7 +1505,7 @@ rte_pmd_vhost_probe(struct rte_vdev_device *dev)
int ret = 0;
char *iface_name;
uint16_t queues;
-   uint64_t flags = RTE_VHOST_USER_NET_COMPLIANT_OL_FLAGS;
+   uint64_t flags = 0;
uint64_t disable_flags = 0;
int client_mode = 0;
int iommu_support = 0;
-- 
2.23.0

Re: [dpdk-dev] [PATCH] net/vhost: restore pseudo TSO support

[Maxime Coquelin]

Hi David,

On 5/18/21 9:07 AM, David Marchand wrote:
> The net/vhost pmd does not comply with the ethdev offload API as it does
> not report rx/tx offload capabilities wrt TSO and checksum offloading.
> On the other hand, the net/vhost pmd lets guest negotiates TSO and
> checksum offloading.
> 
> Changing the behavior for rx/tx offload flags handling won't
> improve/fix this situation and will break applications that might have
> been relying on implicit support of TSO in this driver.
> 
> Revert this behavior change until we have a complete fix.
> 
> Fixes: ca7036b4af3a ("vhost: fix offload flags in Rx path")
> 
> Signed-off-by: David Marchand 
> ---
> The full fix requires:
> - reporting rx/tx_offload_capa,
> - supporting sw TSO and checksums for the case when a virtual machine
>   negotiates this feature, but the application does not configure
>   DEV_[RT]X_OFFLOAD_* appropriate flags,
> 
> ---
>  drivers/net/vhost/rte_eth_vhost.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/vhost/rte_eth_vhost.c 
> b/drivers/net/vhost/rte_eth_vhost.c
> index 8524898661..a202931e9a 100644
> --- a/drivers/net/vhost/rte_eth_vhost.c
> +++ b/drivers/net/vhost/rte_eth_vhost.c
> @@ -1505,7 +1505,7 @@ rte_pmd_vhost_probe(struct rte_vdev_device *dev)
>   int ret = 0;
>   char *iface_name;
>   uint16_t queues;
> - uint64_t flags = RTE_VHOST_USER_NET_COMPLIANT_OL_FLAGS;
> + uint64_t flags = 0;
>   uint64_t disable_flags = 0;
>   int client_mode = 0;
>   int iommu_support = 0;
> 

As we discussed yesterday, I agree that's the best thing to do while
proper TSO support is implemented.

Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [dpdk-dev] [dpdk-announce] release candidate 21.05-rc3

[Jiang, YuX]

All,

Update the test status for Intel part. Till now dpdk21.05-rc3 test is almost 
finished and no critical issue blocked the test.

# Basic Intel(R) NIC testing
* Build or compile:
*Build: cover the build test combination with latest 
GCC/Clang/ICC version and the popular OS revision such as Ubuntu20.04, CentOS 
Stream 8, etc.
- All passed except "crypto_zuc build failed on 
Fedora34 with gcc 11". 3 patches have been provided to fix the gcc11 build 
issue and verified passed by validation team.
*Compile: cover the CFLAGES(O0/O1/O2/O3) with popular OS such 
as Ubuntu20.04 and CentOS Stream 8.
- All passed.

* PF(i40e, ixgbe and igb): test scenarios including 
RTE_FLOW/TSO/Jumboframe/checksum offload/VLAN/VXLAN, etc.
- All tests are done, find 1 new bug about 
unit_tests_power_cpufreq: unit test failed, Intel dev is investigating.
- 1 bug is reported based on rc1 in bugzilla. Patch has 
provided but need other patch supports, all of them will be fixed in next 
release.
> * https://bugs.dpdk.org/show_bug.cgi?id=687

* VF(i40e, ixgbe and igb): test scenarios including 
VF-RTE_FLOW/TSO/Jumboframe/checksum offload/VLAN/VXLAN, etc.
- All tests are done. No new issue is found.

* PF/VF(ICE): test scenarios including /Switch features/Package 
Management/Flow Director/Advanced Tx/Advanced RSS/ACL/DCF/Share code 
update/Flexible Descriptor, etc.
- All tests are done. No new issue is found except known issues.

* Intel NIC single core/NIC performance: test scenarios including PF/VF 
single core performance test, RFC2544 Zero packet loss performance test, etc.
- All passed. No new issue is found.

* Power and IPsec:
* Power: test scenarios including bi-direction/Telemetry/Empty 
Poll Lib/Priority Base Frequency and so on.
- All passed. No new issue is found.
* IPsec: test scenarios including ipsec/ipsec-secgw
- All passed. No new issue is found.

# Basic cryptodev and virtio testing
* Virtio: both function and performance test are covered. Such as 
PVP/Virtio_loopback/virtio-user loopback/virtio-net VM2VM perf testing/VMAWARE 
ESXI 7.0u2, etc.
- All tests are done.
- No new issue is found except known issues which are found in 
rc2.
- 1 bug is reported based on rc2 in bugzilla. Patch has been 
provided and verified passed by validation team.
> * https://bugs.dpdk.org/show_bug.cgi?id=699

* Cryptodev:
*Function test: test scenarios including Cryptodev API 
testing/CompressDev ISA-L/QAT/ZLIB PMD Testing/FIPS, etc.
- All tests are done. No new bug is found except known 
issue.
*Performance test: test scenarios including Thoughput 
Performance /Cryptodev Latency, etc.
- All tests are done. No big perf drop.

Best regards,
Yu Jiang

>-Original Message-
>From: Jiang, YuX
>Sent: Friday, May 14, 2021 6:07 PM
>To: 'Thomas Monjalon' ; dev (dev@dpdk.org)
>
>Cc: Devlin, Michelle ; Mcnamara, John
>; Yigit, Ferruh 
>Subject: RE: [dpdk-dev] [dpdk-announce] release candidate 21.05-rc3
>
>Update the test status for Intel part. Till now 70% is executed, 70% is
>debugged and no critical issue blocked the test.
>
># Basic Intel(R) NIC testing
>   * Build or compile:
>   *Build: cover the build test combination with latest
>GCC/Clang/ICC version and the popular OS revision such as Ubuntu20.04,
>CentOS Stream 8, etc.
>   - All passed.
>   *Compile: cover the CFLAGES(O0/O1/O2/O3) with popular OS
>such as Ubuntu20.04 and CentOS Stream 8.
>   - All passed.
>
>   * PF(i40e, ixgbe and igb): test scenarios including
>RTE_FLOW/TSO/Jumboframe/checksum offload/VLAN/VXLAN, etc.
>   - Execution rate: 100%, debug rate: 80%.
>   - Fixed RC2's critical bad commit id, issue DPDK PF+DPDK
>VF:launch testpmd failed by using VF, bugzilla688, bugzilla693.
>   - 1 bug is reported in bugzilla still investigating.
>   > * https://bugs.dpdk.org/show_bug.cgi?id=687
>
>   * VF(i40e, ixgbe and igb): test scenarios including VF-
>RTE_FLOW/TSO/Jumboframe/checksum offload/VLAN/VXLAN, etc.
>   - Execution rate: 100%, debug rate: 90%. No new issue is
>found.
>
>   * PF/VF(ICE): test scenarios including /Switch features/Package
>Management/Flow Director/Advanced Tx/Advanced RSS/ACL/DCF/Share
>code update/Flexible Descriptor, etc.
>   - Execution rate: 80%, debug rate: 60%. No new issue is found.
>   - 2 bugs about rss_pppoe add rss cfg failed and
>ip_fragment_pf_rss hash issues are found in RC2 have been fixed in RC3.
>
>  

[dpdk-dev] [PATCH] doc: announce transition to vDPA port close function

[Thomas Monjalon]

There is a layer violation in the vDPA API which encourages to destroy
a full device with rte_dev_remove() instead of just closing the port.
The plan is to introduce a new function in 21.08, promote in 21.11,
and deprecate rte_vdpa_get_rte_device() in 21.11.

Signed-off-by: Thomas Monjalon 
---
 doc/guides/rel_notes/deprecation.rst | 8 
 1 file changed, 8 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst 
b/doc/guides/rel_notes/deprecation.rst
index 9584d6bfd7..30f84403eb 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -126,6 +126,14 @@ Deprecation Notices
   can still be used if users specify the devarg "driver=i40evf". I40evf will
   be deleted in DPDK 21.11.
 
+* vdpa: The vDPA API should not try to manipulate or export
+  any ``rte_device`` object, which belongs to the bus layer.
+  The function ``rte_vdpa_get_rte_device()`` will be deprecated in 21.11,
+  when its usage will be replaced with a function ``rte_vdpa_close()``.
+  The new function should enter in 21.08 and get promoted to stable in 21.11.
+  A port close function will allow to close a single port without destroying
+  the rest of the device.
+
 * eventdev: The structure ``rte_event_eth_rx_adapter_queue_conf`` will be
   extended to include ``rte_event_eth_rx_adapter_event_vector_config`` elements
   and the function ``rte_event_eth_rx_adapter_queue_event_vector_config`` will
-- 
2.31.1

Re: [dpdk-dev] [PATCH] doc: announce transition to vDPA port close function

[Andrew Rybchenko]

On 5/18/21 10:34 AM, Thomas Monjalon wrote:
> There is a layer violation in the vDPA API which encourages to destroy
> a full device with rte_dev_remove() instead of just closing the port.
> The plan is to introduce a new function in 21.08, promote in 21.11,
> and deprecate rte_vdpa_get_rte_device() in 21.11.
> 
> Signed-off-by: Thomas Monjalon 

Acked-by: Andrew Rybchenko 

Re: [dpdk-dev] [PATCH] vhost: fix invalid use of stored last used index

[Xia, Chenbo]

> -Original Message-
> From: dev  On Behalf Of Balazs Nemeth
> Sent: Saturday, May 15, 2021 12:02 AM
> To: bnem...@redhat.com; dev@dpdk.org; Ling, WeiX 
> Subject: [dpdk-dev] [PATCH] vhost: fix invalid use of stored last used index
> 
> The optimization introduced by commit d18db8049c7c ("vhost: read last
> used index once") didn't account for the fact that
> vhost_flush_enqueue_shadow_packed increments the last_used_idx. For this
> reason, store last_used_idx after the potential call to
> vhost_flush_enqueue_shadow_packed.
> 
> Fixes: d18db8049c7c ("vhost: read last used index once")
> Signed-off-by: Balazs Nemeth 
> ---
> 2.30.2

Applied to next-virtio/main, thanks

Re: [dpdk-dev] [PATCH] vhost: restore IOTLB mempool allocation

[Xia, Chenbo]

> -Original Message-
> From: David Marchand 
> Sent: Monday, May 17, 2021 5:00 PM
> To: dev@dpdk.org
> Cc: tho...@monjalon.net; Yigit, Ferruh ; Maxime
> Coquelin ; Xia, Chenbo ;
> Zhihong Wang ; Junjie Wan
> 
> Subject: [PATCH] vhost: restore IOTLB mempool allocation
> 
> As explained by Chenbo, IOTLB messages will be sent when some queues
> are not enabled. If we initialize IOTLB in vhost_user_set_vring_num,
> it could happen that IOTLB update comes when IOTLB pool of disabled
> queues are not initialized.
> 
> Fixes: 968bbc7e2e50 ("vhost: avoid IOTLB mempool allocation while IOMMU
> disabled")
> 
> Signed-off-by: David Marchand 
> ---
> Summary of a discussion with Maxime:
> 
> To keep the mempool allocation optimization, we could try to initialise
> the per-vring mempools at reception of the first IOTLB message.
> Since those pools are used as caches, it is not an issue if some vrings
> received more IOTLB updates than others.
> 
> But looking/testing this now is too late for 21.05, hence reverting is
> the safer.
> 
> ---
>  lib/vhost/vhost.c  | 5 +++--
>  lib/vhost/vhost_user.c | 6 +-
>  2 files changed, 4 insertions(+), 7 deletions(-)
> --
> 2.23.0

Applied to next-virtio/main. Thanks

Re: [dpdk-dev] [PATCH] net/vhost: restore pseudo TSO support

[Xia, Chenbo]

> -Original Message-
> From: David Marchand 
> Sent: Tuesday, May 18, 2021 3:07 PM
> To: dev@dpdk.org
> Cc: tho...@monjalon.net; Yigit, Ferruh ; Maxime
> Coquelin ; Xia, Chenbo 
> Subject: [PATCH] net/vhost: restore pseudo TSO support
> 
> The net/vhost pmd does not comply with the ethdev offload API as it does
> not report rx/tx offload capabilities wrt TSO and checksum offloading.
> On the other hand, the net/vhost pmd lets guest negotiates TSO and
> checksum offloading.
> 
> Changing the behavior for rx/tx offload flags handling won't
> improve/fix this situation and will break applications that might have
> been relying on implicit support of TSO in this driver.
> 
> Revert this behavior change until we have a complete fix.
> 
> Fixes: ca7036b4af3a ("vhost: fix offload flags in Rx path")
> 
> Signed-off-by: David Marchand 
> ---
> 2.23.0

Applied to next-virtio/main. Thanks.

Re: [dpdk-dev] [dpdk-stable] [PATCH v2] vdpa/mlx5: fix device unplug

[Xia, Chenbo]

> -Original Message-
> From: stable  On Behalf Of Matan Azrad
> Sent: Friday, May 14, 2021 2:40 AM
> To: dev@dpdk.org
> Cc: maxime.coque...@redhat.com; sta...@dpdk.org; Eli Britstein
> ; Xueming Li 
> Subject: [dpdk-stable] [PATCH v2] vdpa/mlx5: fix device unplug
> 
> The vDPA PCI device unplug process should release all the private
> device resources and also to unregister the device.
> 
> The device unregistration was missed what remained the device data
> invalid in the rte_vhost library.
> 
> Unregister the device in unplug process via the remove operation.
> 
> Fixes: 95276abaaf0a ("vdpa/mlx5: introduce Mellanox vDPA driver")
> Cc: sta...@dpdk.org
> 
> Reported-by: Eli Britstein 
> Signed-off-by: Matan Azrad 
> Tested-by: Eli Britstein 
> Acked-by: Xueming Li 
> ---
>  drivers/vdpa/mlx5/mlx5_vdpa.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> v2:
> fix spelling and email format
> 
> diff --git a/drivers/vdpa/mlx5/mlx5_vdpa.c b/drivers/vdpa/mlx5/mlx5_vdpa.c
> index 40db28b6db..e5e03e6582 100644
> --- a/drivers/vdpa/mlx5/mlx5_vdpa.c
> +++ b/drivers/vdpa/mlx5/mlx5_vdpa.c
> @@ -787,6 +787,8 @@ mlx5_vdpa_pci_remove(struct rte_pci_device *pci_dev)
>   mlx5_glue->dv_free_var(priv->var);
>   priv->var = NULL;
>   }
> + if (priv->vdev)
> + rte_vdpa_unregister_device(priv->vdev);
>   mlx5_glue->close_device(priv->ctx);
>   pthread_mutex_destroy(&priv->vq_config_lock);
>   rte_free(priv);
> --
> 2.25.1

Reviewed-by: Chenbo Xia 

Re: [dpdk-dev] [dpdk-stable] [PATCH v2] vdpa/mlx5: fix device unplug

[Xia, Chenbo]

> -Original Message-
> From: stable  On Behalf Of Matan Azrad
> Sent: Friday, May 14, 2021 2:40 AM
> To: dev@dpdk.org
> Cc: maxime.coque...@redhat.com; sta...@dpdk.org; Eli Britstein
> ; Xueming Li 
> Subject: [dpdk-stable] [PATCH v2] vdpa/mlx5: fix device unplug
> 
> The vDPA PCI device unplug process should release all the private
> device resources and also to unregister the device.
> 
> The device unregistration was missed what remained the device data
> invalid in the rte_vhost library.
> 
> Unregister the device in unplug process via the remove operation.
> 
> Fixes: 95276abaaf0a ("vdpa/mlx5: introduce Mellanox vDPA driver")
> Cc: sta...@dpdk.org
> 
> Reported-by: Eli Britstein 
> Signed-off-by: Matan Azrad 
> Tested-by: Eli Britstein 
> Acked-by: Xueming Li 
> ---
> 2.25.1

Applied to next-virtio/main. Thanks.

Re: [dpdk-dev] [PATCH v3] net/mlx5: fix loopback for DV queue

[Thomas Monjalon]

17/05/2021 17:18, Bing Zhao:
> In the past, all the queues and other hardware objects were created
> through Verbs interface. Currently, most of the objects creation are
> migrated to Devx interface by default, including queues. Only when
> the DV is disabled by device arg or eswitch is enabled, all or some
> of the objects are created through Verbs interface.
> 
> When using Devx interface to create queues, the kernel driver
> behavior is different from the case using Verbs. The Tx loopback
> cannot work properly even if the Tx and Rx queues are configured
> with loopback attribute. To fix the support self loopback for Tx, a
> Verbs dummy queue pair needs to be created to trigger the kernel to
> enable the global loopback capability.
> 
> This is only required when TIR is created for Rx and loopback is
> needed. Only CQ and QP are needed for this case, no WQ(RQ) needs to
> be created.
> 
> This requirement comes from bugzilla 645, more details can be found
> in the bugzilla link.
> 
> Bugzilla ID: 645
> 
> Fixes: 6deb19e1b2d2 ("net/mlx5: separate Rx queue object creations")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Bing Zhao 
> Acked-by: Viacheslav Ovsiienko 

Applied to next-net-mlx, thanks.

Re: [dpdk-dev] [PATCH v3] net/mlx5: fix loopback for DV queue

[Bing Zhao]

Thanks a lot, Thomas.

> -Original Message-
> From: Thomas Monjalon 
> Sent: Tuesday, May 18, 2021 4:33 PM
> To: Bing Zhao 
> Cc: Slava Ovsiienko ; Matan Azrad
> ; dev@dpdk.org; Ori Kam ; Raslan
> Darawsheh ; sta...@dpdk.org; Tal Shnaiderman
> 
> Subject: Re: [dpdk-dev] [PATCH v3] net/mlx5: fix loopback for DV
> queue
> 
> External email: Use caution opening links or attachments
> 
> 
> 17/05/2021 17:18, Bing Zhao:
> > In the past, all the queues and other hardware objects were
> created
> > through Verbs interface. Currently, most of the objects creation
> are
> > migrated to Devx interface by default, including queues. Only when
> the
> > DV is disabled by device arg or eswitch is enabled, all or some of
> the
> > objects are created through Verbs interface.
> >
> > When using Devx interface to create queues, the kernel driver
> behavior
> > is different from the case using Verbs. The Tx loopback cannot
> work
> > properly even if the Tx and Rx queues are configured with loopback
> > attribute. To fix the support self loopback for Tx, a Verbs dummy
> > queue pair needs to be created to trigger the kernel to enable the
> > global loopback capability.
> >
> > This is only required when TIR is created for Rx and loopback is
> > needed. Only CQ and QP are needed for this case, no WQ(RQ) needs
> to be
> > created.
> >
> > This requirement comes from bugzilla 645, more details can be
> found in
> > the bugzilla link.
> >
> > Bugzilla ID: 645
> >
> > Fixes: 6deb19e1b2d2 ("net/mlx5: separate Rx queue object
> creations")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Bing Zhao 
> > Acked-by: Viacheslav Ovsiienko 
> 
> Applied to next-net-mlx, thanks.
> 
> 

[dpdk-dev] [PATCH v3 0/2] remove wmb for net/mlx

[Feifei Wang]

For net/mlx4 and net/mlx5, remove unnecessary wmb for Memory Region
cache.

v2:
1. keep the order of dev_gen and global cache (Slava Ovsiienko)
2. remove the wmb at last instead of moving it forward
3. remove atomic_thread_fence patches

v3:
1. commit message rewording (Slava Ovsiienko)

Feifei Wang (2):
  net/mlx4: remove unnecessary wmb for Memory Region cache
  net/mlx5: remove unnecessary wmb for Memory Region cache

 drivers/net/mlx4/mlx4_mr.c | 11 +++
 drivers/net/mlx5/mlx5_mr.c | 22 ++
 2 files changed, 9 insertions(+), 24 deletions(-)

-- 
2.25.1

[dpdk-dev] [PATCH v3 1/2] net/mlx4: remove unnecessary wmb for Memory Region cache

[Feifei Wang]

'dev_gen' is a variable to trigger all cores to flush their local caches
once the global MR cache has been rebuilt.

This is due to MR cache's R/W lock can maintain synchronization between
threads:

1. dev_gen and global cache updating ordering inside the lock protected
section does not matter. Because other threads cannot take the lock
until global cache has been updated. Thus, in out of order platform,
even if other agents firstly observe updated dev_gen but global does
not update, they still have to wait the lock. As a result, it is
unnecessary to add a wmb between global cache rebuilding and updating
the dev_gen to keep the memory store order.

2. Store-Release of unlock provides the implicit wmb at the level
visible by software. This makes 'rebuilding global cache' and 'updating
dev_gen' be observed before local_cache starts to be updated by other
agents. Thus, wmb after 'updating dev_gen' can be removed.

Suggested-by: Ruifeng Wang 
Signed-off-by: Feifei Wang 
Reviewed-by: Ruifeng Wang 
---
 drivers/net/mlx4/mlx4_mr.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/net/mlx4/mlx4_mr.c b/drivers/net/mlx4/mlx4_mr.c
index 6b2f0cf187..2274b5df19 100644
--- a/drivers/net/mlx4/mlx4_mr.c
+++ b/drivers/net/mlx4/mlx4_mr.c
@@ -948,18 +948,13 @@ mlx4_mr_mem_event_free_cb(struct rte_eth_dev *dev, const 
void *addr, size_t len)
if (rebuild) {
mr_rebuild_dev_cache(dev);
/*
-* Flush local caches by propagating invalidation across cores.
-* rte_smp_wmb() is enough to synchronize this event. If one of
-* freed memsegs is seen by other core, that means the memseg
-* has been allocated by allocator, which will come after this
-* free call. Therefore, this store instruction (incrementing
-* generation below) will be guaranteed to be seen by other core
-* before the core sees the newly allocated memory.
+* No explicit wmb is needed after updating dev_gen due to
+* store-release ordering in unlock that provides the
+* implicit barrier at the software visible level.
 */
++priv->mr.dev_gen;
DEBUG("broadcasting local cache flush, gen=%d",
  priv->mr.dev_gen);
-   rte_smp_wmb();
}
rte_rwlock_write_unlock(&priv->mr.rwlock);
 #ifdef RTE_LIBRTE_MLX4_DEBUG
-- 
2.25.1

[dpdk-dev] [PATCH v3 2/2] net/mlx5: remove unnecessary wmb for Memory Region cache

[Feifei Wang]

'dev_gen' is a variable to trigger all cores to flush their local caches
once the global MR cache has been rebuilt.

This is due to MR cache's R/W lock can maintain synchronization between
threads:

1. dev_gen and global cache updating ordering inside the lock protected
section does not matter. Because other threads cannot take the lock
until global cache has been updated. Thus, in out of order platform,
even if other agents firstly observe updated dev_gen but global does
not update, they also have to wait the lock. As a result, it is
unnecessary to add a wmb between global cache rebuilding and updating
the dev_gen to keep the memory store order.

2. Store-Release of unlock provides the implicit wmb at the level
visible by software. This makes 'rebuilding global cache' and 'updating
dev_gen' be observed before local_cache starts to be updated by other
agents. Thus, wmb after 'updating dev_gen' can be removed.

Suggested-by: Ruifeng Wang 
Signed-off-by: Feifei Wang 
Reviewed-by: Ruifeng Wang 
---
 drivers/net/mlx5/mlx5_mr.c | 22 ++
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_mr.c b/drivers/net/mlx5/mlx5_mr.c
index e791b6338d..0c5403e493 100644
--- a/drivers/net/mlx5/mlx5_mr.c
+++ b/drivers/net/mlx5/mlx5_mr.c
@@ -107,18 +107,13 @@ mlx5_mr_mem_event_free_cb(struct mlx5_dev_ctx_shared *sh,
if (rebuild) {
mlx5_mr_rebuild_cache(&sh->share_cache);
/*
-* Flush local caches by propagating invalidation across cores.
-* rte_smp_wmb() is enough to synchronize this event. If one of
-* freed memsegs is seen by other core, that means the memseg
-* has been allocated by allocator, which will come after this
-* free call. Therefore, this store instruction (incrementing
-* generation below) will be guaranteed to be seen by other core
-* before the core sees the newly allocated memory.
+* No explicit wmb is needed after updating dev_gen due to
+* store-release ordering in unlock that provides the
+* implicit barrier at the software visible level.
 */
++sh->share_cache.dev_gen;
DRV_LOG(DEBUG, "broadcasting local cache flush, gen=%d",
  sh->share_cache.dev_gen);
-   rte_smp_wmb();
}
rte_rwlock_write_unlock(&sh->share_cache.rwlock);
 }
@@ -411,18 +406,13 @@ mlx5_dma_unmap(struct rte_pci_device *pdev, void *addr,
  (void *)mr);
mlx5_mr_rebuild_cache(&sh->share_cache);
/*
-* Flush local caches by propagating invalidation across cores.
-* rte_smp_wmb() is enough to synchronize this event. If one of
-* freed memsegs is seen by other core, that means the memseg
-* has been allocated by allocator, which will come after this
-* free call. Therefore, this store instruction (incrementing
-* generation below) will be guaranteed to be seen by other core
-* before the core sees the newly allocated memory.
+* No explicit wmb is needed after updating dev_gen due to
+* store-release ordering in unlock that provides the
+* implicit barrier at the software visible level.
 */
++sh->share_cache.dev_gen;
DRV_LOG(DEBUG, "broadcasting local cache flush, gen=%d",
  sh->share_cache.dev_gen);
-   rte_smp_wmb();
rte_rwlock_read_unlock(&sh->share_cache.rwlock);
return 0;
 }
-- 
2.25.1

[dpdk-dev] 回复: [PATCH v2 2/2] net/mlx5: remove unnecessary wmb for Memory Region cache

[Feifei Wang]

Hi， Slava

> -邮件原件-
> 发件人: Slava Ovsiienko 
> 发送时间: 2021年5月17日 22:15
> 收件人: Feifei Wang ; Matan Azrad
> ; Shahaf Shuler 
> 抄送: dev@dpdk.org; nd ; Ruifeng Wang
> 
> 主题: RE: [PATCH v2 2/2] net/mlx5: remove unnecessary wmb for Memory
> Region cache
> 
> Hi, Feifei
> 
> Thanks you for the patch.
> Please, see my notes below about typos and minor commit message
> rewording.

Thanks very much for your very careful reviewing.
I will apply these in the next version.























































 

Re: [dpdk-dev] [PATCH] test/table: fix build with GCC 11

[Thomas Monjalon]

17/05/2021 17:57, Ferruh Yigit:
> Build error:
> ../app/test/test_table_tables.c: In function ‘test_table_stub’:
> ../app/test/test_table_tables.c:31:9:
>   warning: ‘memset’ offset [0, 31] is out of the bounds [0, 0]
>   [-Warray-bounds]
>  memset((uint8_t *)mbuf + sizeof(struct rte_mbuf) + 32, 0, 32); \
>  ^
> ../app/test/test_table_tables.c:151:25:
>   note: in expansion of macro ‘PREPARE_PACKET’
>   151 | PREPARE_PACKET(mbufs[i], 0xadadadad);
>   | ^~
> 
> 'key' points to mbuf header + 32 bytes, and memset clears next 32 bytes
> of 'key', so overall there needs to be 64 bytes after mbuf header.
> Adding a mbuf size check before memset.
> 
> The original code has an assumption that mbuf data buffer follows mbuf
> header, this patch accepts same assumption.
> 
> Bugzilla ID: 677
> Fixes: 5205954791cb ("app/test: packet framework unit tests")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Ferruh Yigit 
> ---
> Cc: cristian.dumitre...@intel.com
> Cc: Kevin Traynor 
> 
> Not exactly clear why compiler complains about, compiler can't know the
> bounds of the memory we try to memset here.
> But adding a size check seems logic thing to do also fixes the compiler
> warning.

Applied, thanks

Re: [dpdk-dev] [PATCH] doc: update QAT compression PMD in 21.05 rel notes

[Dybkowski, AdamX]

Thanks, Akhil.

Adam

> -Original Message-
> From: Akhil Goyal 
> Sent: Monday, 17 May, 2021 21:39
> To: Dybkowski, AdamX ; Thomas Monjalon
> 
> Cc: dev@dpdk.org; Zhang, Roy Fan ; Mcnamara,
> John ; Yigit, Ferruh 
> Subject: RE: [dpdk-dev] [PATCH] doc: update QAT compression PMD in
> 21.05 rel notes
> 
> > Hi.
> >
> > This release notes update describes that patch:
> > http://patches.dpdk.org/project/dpdk/patch/20210428144142.85929-2-
> adamx.dybkow...@intel.com/
> >
> > Adam
> >
> Added Fixes tag in commit log.
> Also updated the patch title and the location of adding the compression
> PMD is also updated.
> 
> Please take care in future.
> 
> Applied to dpdk-next-crypto

[dpdk-dev] 回复: [PATCH] app/eventdev: remove unnecessary barrier for order test

[Feifei Wang]

Hi, Jerin

Sorry to disturb you. Would you please help me review this patch, thanks very 
much.


Best Regards
Feifei

> -Original Message-
> From: Feifei Wang 
> Sent: Monday, May 10, 2021 2:12 PM
> To: jer...@marvell.com
> Cc: dev@dpdk.org; nd ; Feifei Wang
> ; Ruifeng Wang ;
> Honnappa Nagarahalli 
> Subject: [PATCH] app/eventdev: remove unnecessary barrier for order test
> 
> For "order_launch_lcores" function, wmb after that the main lcore updates
> the variable "t->err", which represents the end of the test signal, is
> unnecessary. Because after the main lcore updates this siginal variable, it 
> will
> jump out of the launch function loop, and wait other lcores stop or return
> error in the main function(evt_main.c).
> During this time, there is no storing operation and thus no need for wmb.
> 
> Signed-off-by: Feifei Wang 
> Reviewed-by: Ruifeng Wang 
> Reviewed-by: Honnappa Nagarahalli 
> ---
>  app/test-eventdev/test_order_common.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/app/test-eventdev/test_order_common.c b/app/test-
> eventdev/test_order_common.c
> index 04456d56db..d7760061ba 100644
> --- a/app/test-eventdev/test_order_common.c
> +++ b/app/test-eventdev/test_order_common.c
> @@ -308,7 +308,6 @@ order_launch_lcores(struct evt_test *test, struct
> evt_options *opt,
>   rte_event_dev_dump(opt->dev_id, stdout);
>   evt_err("No schedules for seconds,
> deadlock");
>   t->err = true;
> - rte_smp_wmb();
>   break;
>   }
>   old_remaining = remaining;
> --
> 2.25.1
> 

Re: [dpdk-dev] [PATCH] test/table: fix build with GCC 11

[Kevin Traynor]

On 18/05/2021 09:55, Thomas Monjalon wrote:
> 17/05/2021 17:57, Ferruh Yigit:
>> Build error:
>> ../app/test/test_table_tables.c: In function ‘test_table_stub’:
>> ../app/test/test_table_tables.c:31:9:
>>  warning: ‘memset’ offset [0, 31] is out of the bounds [0, 0]
>>  [-Warray-bounds]
>>  memset((uint8_t *)mbuf + sizeof(struct rte_mbuf) + 32, 0, 32); \
>>  ^
>> ../app/test/test_table_tables.c:151:25:
>>  note: in expansion of macro ‘PREPARE_PACKET’
>>   151 | PREPARE_PACKET(mbufs[i], 0xadadadad);
>>   | ^~
>>
>> 'key' points to mbuf header + 32 bytes, and memset clears next 32 bytes
>> of 'key', so overall there needs to be 64 bytes after mbuf header.
>> Adding a mbuf size check before memset.
>>
>> The original code has an assumption that mbuf data buffer follows mbuf
>> header, this patch accepts same assumption.
>>
>> Bugzilla ID: 677
>> Fixes: 5205954791cb ("app/test: packet framework unit tests")
>> Cc: sta...@dpdk.org
>>
>> Signed-off-by: Ferruh Yigit 
>> ---
>> Cc: cristian.dumitre...@intel.com
>> Cc: Kevin Traynor 
>>
>> Not exactly clear why compiler complains about, compiler can't know the
>> bounds of the memory we try to memset here.
>> But adding a size check seems logic thing to do also fixes the compiler
>> warning.
> 
> Applied, thanks
> 

Was just testing this. fwiw, passing build on F34/gcc11 and LGTM.

> 
> 

Re: [dpdk-dev] [PATCH] net/mlx5: check meta register width for modify field

[Slava Ovsiienko]

> -Original Message-
> From: Alexander Kozyrev 
> Sent: Thursday, May 13, 2021 22:55
> To: dev@dpdk.org
> Cc: sta...@dpdk.org; Raslan Darawsheh ; Slava
> Ovsiienko ; Matan Azrad 
> Subject: [PATCH] net/mlx5: check meta register width for modify field
> 
> The modify_field Flow API assumes that the META item is 32 bits wide.
> But the C Register that is used for Meta item can be 16 or 32 bits wide
> depending on kernel and firmware configurations.
> Take this into consideration and use the appropriate META width.
> 
> Fixes: 641dbe4fb0 ("net/mlx5: support modify field flow action")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Alexander Kozyrev 
Acked-by: Viacheslav Ovsiienko 

Re: [dpdk-dev] [PATCH] net/mlx5: check meta register width for modify field

[Thomas Monjalon]

> > The modify_field Flow API assumes that the META item is 32 bits wide.
> > But the C Register that is used for Meta item can be 16 or 32 bits wide
> > depending on kernel and firmware configurations.
> > Take this into consideration and use the appropriate META width.
> > 
> > Fixes: 641dbe4fb0 ("net/mlx5: support modify field flow action")
> > Cc: sta...@dpdk.org
> > 
> > Signed-off-by: Alexander Kozyrev 
> Acked-by: Viacheslav Ovsiienko 

Applied in next-net-mlx, thanks.

[dpdk-dev] [Bug 713] Failure in latest DPDK build: Cannot init any port in AWS ENA

[bugzilla]

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

Bug ID: 713
   Summary: Failure in latest DPDK build: Cannot init any port in
AWS ENA
   Product: DPDK
   Version: 20.11
  Hardware: All
OS: All
Status: UNCONFIRMED
  Severity: normal
  Priority: Normal
 Component: ethdev
  Assignee: dev@dpdk.org
  Reporter: kapraka...@gmail.com
  Target Milestone: ---

Created attachment 156
  --> https://bugs.dpdk.org/attachment.cgi?id=156&action=edit
Snapshot of issue

Issue Description
-
With AWS running Ubuntu 20.04 agains latest DPDK version. We are not able to
init the ports
(More info: testpmd is able to detect and load all ports, but
dpdk_skeleton(basic packet forwarding) always fails to init ports)

Steps to reproduce
--
Below is the issue we are facing, while trying to start the basic dpdk packet
forwarding,
(1) The core allocation was successful
(2) The interface binding was successful
(3) Port init is failing

__SnapShot__of__issue__
===

ubuntu@ip-172-31-23-251:~/dpdk-21.02/build/examples$ sudo ./dpdk-skeleton -l1
-n4
EAL: Detected 4 lcore(s)
EAL: Detected 1 NUMA nodes
EAL: Detected static linkage of DPDK
EAL: Multi-process socket /var/run/dpdk/rte/mp_socket
EAL: Selected IOVA mode 'PA'
EAL: No available 1048576 kB hugepages reported
EAL: Probing VFIO support...
EAL:   Invalid NUMA socket, default to 0
EAL:   Invalid NUMA socket, default to 0
EAL: Probe PCI driver: net_ena (1d0f:ec20) device: :00:06.0 (socket 0)
EAL:   Invalid NUMA socket, default to 0
EAL: Probe PCI driver: net_ena (1d0f:ec20) device: :00:07.0 (socket 0)
EAL: No legacy callbacks, legacy socket not created
Port 0 MAC: 06 66 dc 11 e8 30
EAL: Error - exiting with code: 1
  Cause: Cannot init port 0

ubuntu@ip-172-31-23-251:~/dpdk-21.02/build/examples$ cd ../..

Network devices using DPDK-compatible driver

:00:06.0 'Elastic Network Adapter (ENA) ec20' drv=igb_uio unused=ena
:00:07.0 'Elastic Network Adapter (ENA) ec20' drv=igb_uio unused=ena

Network devices using kernel driver
===
:00:05.0 'Elastic Network Adapter (ENA) ec20' if=ens5 drv=ena
unused=igb_uio *Active*

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

Re: [dpdk-dev] [PATCH v3 2/2] net/mlx5: remove unnecessary wmb for Memory Region cache

[Slava Ovsiienko]

> -Original Message-
> From: Feifei Wang 
> Sent: Tuesday, May 18, 2021 11:51
> To: Matan Azrad ; Shahaf Shuler
> ; Slava Ovsiienko 
> Cc: dev@dpdk.org; n...@arm.com; Feifei Wang ;
> Ruifeng Wang 
> Subject: [PATCH v3 2/2] net/mlx5: remove unnecessary wmb for Memory
> Region cache
> 
> 'dev_gen' is a variable to trigger all cores to flush their local caches once 
> the
> global MR cache has been rebuilt.
> 
> This is due to MR cache's R/W lock can maintain synchronization between
> threads:
> 
> 1. dev_gen and global cache updating ordering inside the lock protected
> section does not matter. Because other threads cannot take the lock until
> global cache has been updated. Thus, in out of order platform, even if other
> agents firstly observe updated dev_gen but global does not update, they
> also have to wait the lock. As a result, it is unnecessary to add a wmb
> between global cache rebuilding and updating the dev_gen to keep the
> memory store order.
> 
> 2. Store-Release of unlock provides the implicit wmb at the level visible by
> software. This makes 'rebuilding global cache' and 'updating dev_gen' be
> observed before local_cache starts to be updated by other agents. Thus,
> wmb after 'updating dev_gen' can be removed.
> 
> Suggested-by: Ruifeng Wang 
> Signed-off-by: Feifei Wang 
> Reviewed-by: Ruifeng Wang 
Acked-by: Viacheslav Ovsiienko 

Thanks a lot for patience and cooperation.
With best regards,
Slava

Re: [dpdk-dev] [PATCH v5 3/3] devtools: check flow API doc tables

[Thomas Monjalon]

14/05/2021 12:51, Ferruh Yigit:
> On 4/7/2021 11:33 PM, Thomas Monjalon wrote:
> > The script check-doc-vs-code.sh may be used to add
> > some automatic checks of the doc.
> > 
> > If run without any argument, a complete check is done.
> > The optional argument is a git history reference point
> > to check faster only what has changed since this commit.
> > 
> > In this commit, the only check is for rte_flow tables,
> > achieved through the script parse-flow-support.sh.
> > If run without a .ini reference, it prints rte_flow tables.
> > Note: detected features are marked with the value Y,
> > while the real .ini file could have special values like I.
> > The script allow parsing exceptions (exclude or include),
> > like for bnxt code which lists unsupported items and actions.
> >
> 
> Overall great to be able to generate and check document against code, also 
> good
> to have this by relatively small/simple scripts, thanks for the work.
> This helps to remove the maintenance concerns I had.
> 
> > Signed-off-by: Thomas Monjalon 
> > ---
> >  devtools/check-doc-vs-code.sh  | 79 ++
> >  devtools/parse-flow-support.sh | 76 
> 
> Will there be automated checks as part of the build system? Presumably in
> 'developer_mode'?

I think we should discuss what could enter in developer mode or not.
I'm afraid it will become a mini-CI and will make compilation longer for 
everybody.

> btw, scripts points out some new features not documented in .ini files, those
> are the recently added ones, patch requires a rebase on top of latest code.

OK I am rebasing.

> > +rte_flow_support() # 
> > +{
> > +   title="rte_flow $1s"
> > +   pattern=$(echo "RTE_FLOW_$1_TYPE_" | awk '{print toupper($0)}')
> > +   list "$title" "$pattern" | grep -vwE 'void|end'
> 
> Should 'RTE_FLOW_ITEM_TYPE_ANY' also excluded, does it have benefit to have it
> as listed?

This item may be unsupported by some PMDs,
so I think we need to report it.

[dpdk-dev] [PATCH] net/nfp: remove compile time log

[Ferruh Yigit]

Logging should be converted to dynamic log.

Signed-off-by: Ferruh Yigit 
---
 drivers/net/nfp/nfp_net_logs.h | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/drivers/net/nfp/nfp_net_logs.h b/drivers/net/nfp/nfp_net_logs.h
index 27dd87611b94..76cc94cb6565 100644
--- a/drivers/net/nfp/nfp_net_logs.h
+++ b/drivers/net/nfp/nfp_net_logs.h
@@ -30,14 +30,7 @@ extern int nfp_logtype_init;
 #define ASSERT(x) do { } while (0)
 #endif
 
-#define RTE_LIBRTE_NFP_NET_DEBUG_CPP
-
-#ifdef RTE_LIBRTE_NFP_NET_DEBUG_CPP
-#define PMD_CPP_LOG(level, fmt, args...) \
-   RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
-#else
 #define PMD_CPP_LOG(level, fmt, args...) do { } while (0)
-#endif
 
 extern int nfp_logtype_driver;
 #define PMD_DRV_LOG(level, fmt, args...) \
-- 
2.31.1

Re: [dpdk-dev] [PATCH 0/6] bugfix for hns3 PMD

[Ferruh Yigit]

On 5/15/2021 1:52 AM, Min Hu (Connor) wrote:
> This patch set contains six bugfixes for hns3 PMD.
> 
> Huisong Li (6):
>   net/hns3: fix check of Rx/Tx queue numbers
>   net/hns3: fix requested FC mode roolback
>   net/hns3: remove meaningless packet buffer rollback
>   net/hns3: fix DCB configuration
>   net/hns3: fix DCB cannot be reconfigured
>   net/hns3: fix link speed when VF device is down
> 

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

[dpdk-dev] [Bug 713] Cannot init ENA port in skeleton example

[bugzilla]

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

kalyan (kapraka...@gmail.com) changed:

   What|Removed |Added

 Resolution|--- |WORKSFORME
 Status|UNCONFIRMED |RESOLVED

--- Comment #2 from kalyan (kapraka...@gmail.com) ---
Hi David,

Thanks much! After I masked that check it works(snapshot below),

Besides, could you throw some light on the future implications of masking this
check in AWS ENA? (And also the reason behind introducing this check) 

__Snap shot__
oot@ip-172-31-23-251:/home/ubuntu/dpdk-stable-20.11.1/build/examples#
./dpdk-skeleton -l 1 -n 2
EAL: Detected 4 lcore(s)
EAL: Detected 1 NUMA nodes
EAL: Detected static linkage of DPDK
EAL: Multi-process socket /var/run/dpdk/rte/mp_socket
EAL: Selected IOVA mode 'PA'
EAL: No available hugepages reported in hugepages-1048576kB
EAL: Probing VFIO support...
EAL:   Invalid NUMA socket, default to 0
EAL:   Invalid NUMA socket, default to 0
EAL: Probe PCI driver: net_ena (1d0f:ec20) device: :00:06.0 (socket 0)
EAL:   Invalid NUMA socket, default to 0
EAL: Probe PCI driver: net_ena (1d0f:ec20) device: :00:07.0 (socket 0)
EAL: No legacy callbacks, legacy socket not created
Port 0 MAC: 06 66 dc 11 e8 30
Port 1 MAC: 06 b7 8f e2 7e 6e

Core 1 forwarding packets.

Thanks,
Kalyan

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

[dpdk-dev] [PATCH] app/acl: add script for automate testing

[Konstantin Ananyev]

The purpose of this script is to help automate ACL library functional
testing using test-acl app.
Sample input files are also provided.

Signed-off-by: Konstantin Ananyev 
---
 app/test-acl/input/acl1v4_5_rule  |  5 ++
 app/test-acl/input/acl1v4_5_trace |  5 ++
 app/test-acl/input/acl1v6_1_rule  |  1 +
 app/test-acl/input/acl1v6_1_trace |  2 +
 app/test-acl/test-acl.sh  | 92 +++
 5 files changed, 105 insertions(+)
 create mode 100644 app/test-acl/input/acl1v4_5_rule
 create mode 100644 app/test-acl/input/acl1v4_5_trace
 create mode 100644 app/test-acl/input/acl1v6_1_rule
 create mode 100644 app/test-acl/input/acl1v6_1_trace
 create mode 100644 app/test-acl/test-acl.sh

diff --git a/app/test-acl/input/acl1v4_5_rule b/app/test-acl/input/acl1v4_5_rule
new file mode 100644
index 00..45301f3a3d
--- /dev/null
+++ b/app/test-acl/input/acl1v4_5_rule
@@ -0,0 +1,5 @@
+@16.32.1.1/32  2.1.0.0/16  0 : 65535   0 : 65535 0/0
+@16.32.1.0/24  2.1.1.0/24  0 : 65535   0 : 65535 0/0
+@2.1.0.0/1616.32.1.1/320 : 65535   0 : 65535 0/0
+@2.1.0.0/1616.32.1.0/241000 : 2000 0 : 65535 0/0
+@2.1.1.0/2416.32.1.0/240 : 65535   0 : 65535 0/0
diff --git a/app/test-acl/input/acl1v4_5_trace 
b/app/test-acl/input/acl1v4_5_trace
new file mode 100644
index 00..bfbcd31f88
--- /dev/null
+++ b/app/test-acl/input/acl1v4_5_trace
@@ -0,0 +1,5 @@
+0x10200101 0x02010304 100 100 6 0
+0x10200103 0x02010104 100 100 6 1
+0x02010304 0x10200101 100 100 6 2
+0x02010104 0x10200103 100 100 6 4
+0x02010104 0x10200101 100 100 6 2
diff --git a/app/test-acl/input/acl1v6_1_rule b/app/test-acl/input/acl1v6_1_rule
new file mode 100644
index 00..2ea9856b27
--- /dev/null
+++ b/app/test-acl/input/acl1v6_1_rule
@@ -0,0 +1 @@
+@9baa:cead:8000::e300:00ff:fe04:c5a8/33
d1b5:2feb:::6600:00ff:fed9:aeaa/33  0 : 65535   1526 : 1526 
0x00/0x00
diff --git a/app/test-acl/input/acl1v6_1_trace 
b/app/test-acl/input/acl1v6_1_trace
new file mode 100644
index 00..6e86741edf
--- /dev/null
+++ b/app/test-acl/input/acl1v6_1_trace
@@ -0,0 +1,2 @@
+9baa:cead:8000::::: 
d1b5:2feb:::::: 37826 1526 6 0
+2eba:cc41:::::: 
16ef:6cdb:::::: 25946 1525 0 4294967295
diff --git a/app/test-acl/test-acl.sh b/app/test-acl/test-acl.sh
new file mode 100644
index 00..348962c540
--- /dev/null
+++ b/app/test-acl/test-acl.sh
@@ -0,0 +1,92 @@
+#! /bin/bash
+# SPDX-License-Identifier: BSD-3-Clause
+
+# Usage:
+# /bin/bash

+# Expected file-naming conventions:
+#   - for rules: 'acl[0-9]v[4,6]_[0-9,a-z]+_rule'
+#   - for traces: 'acl[0-9]v[4,6]_[0-9,a-z]+_trace'
+# Each rule file expects to have exactly one trace file.
+# test-acl app follows classbench file format.
+# Each line defines exactly one rule/trace.
+# rules record format:
+# '@''/' \
+# '/' \
+# ":" \
+# ":" \
+# '/'
+# trace record format:
+#  \
+# ...
+#
+# As an example:
+# /bin/bash app/test-acl/test-acl.sh build/app/dpdk-test-acl \
+# app/test-acl/input scalar 32
+#
+# Refer to test-acl app for more information about rules/trace files format,
+# available test-acl command-line options, etc.
+
+TACL_PATH=$1
+TACL_DIR=$2
+TACL_ALG=$3
+TACL_STEP=$4
+
+if [[ ! -x ${TACL_PATH} ]]; then
+   echo "invalid TACL_PATH=${TACL_PATH}"
+   exit 127
+fi
+
+if [[ ! -d ${TACL_DIR} ]]; then
+   echo "invalid TACL_DIR=${TACL_DIR}"
+   exit 127
+fi
+
+V4F=`find ${TACL_DIR} -type f | egrep -e 'acl[0-9]v4_[0-9,a-z]+_rule$'`
+V6F=`find ${TACL_DIR} -type f | egrep -e 'acl[0-9]v6_[0-9,a-z]+_rule$'`
+
+run_test()
+{
+   i=$1
+   n=`basename ${i}`
+
+   TRACEF=`echo ${i} | sed -e 's/_rule$/_trace/'`
+   if [[ ! -f ${TRACEF} ]]; then
+   echo "${TRACEF} not found"
+   echo "test ${n} FAILED"
+   exit 127
+   fi
+
+   OUTF=`mktemp ${n}_XX`
+   echo "start test ${n} with alg ${TACL_ALG}, burst-size ${TACL_STEP}"
+   ${TACL_PATH} -l 0 -n 4 --log-level="acl,debug" \
+   --force-max-simd-bitwidth=0 --no-pci -- \
+   ${XPRM} --tracenum=20 --rulesf=${i} --tracef=${TRACEF} \
+   --tracestep=${TACL_STEP} --alg=${TACL_ALG} \
+   > ${OUTF}
+   grep 'result:' ${OUTF} | awk '{print $(NF);}' > ${OUTF}.out
+   sed -e '/^#/d' -e 's/[[:space:]]*$//g' ${TRACEF} | \
+   awk '{print $(NF);}' > ${OUTF}.chk
+   diff -u ${OUTF}.chk ${OUTF}.out
+   st=$?
+   if [[ $st -ne 0 ]]; then
+   echo "test ${n} FAILED"
+   echo "output files:"
+   ls ${OUTF}*
+   cat ${OUTF}*
+   exit 127
+   fi
+   rm -f ${OUTF}*
+   echo "test ${n} OK"
+}
+
+for i in ${V4F}; do
+   run_test $i
+done
+
+for i in ${V6F}; do
+   XPRM='--ipv6'
+   run_test $i
+   unset XPRM
+done
+
+echo "All tests have ended successfully"
-- 
2.26.3

Re: [dpdk-dev] [PATCH] net/mlx5: check meta register width for modify field

[Asaf Penso]

I didn't plan to have it in this release since it's not critical and I'm afraid 
of such changes... I didn't mention this in the status mail today.
@Raslan Darawsheh can we check tomorrow regression 
result?
If not good we'll need to revert it.

Regards,
Asaf Penso

From: Thomas Monjalon 
Sent: Tuesday, May 18, 2021 12:43:34 PM
To: Alexander Kozyrev 
Cc: dev@dpdk.org ; sta...@dpdk.org ; Raslan 
Darawsheh ; Matan Azrad ; Slava Ovsiienko 
; Asaf Penso 
Subject: Re: [dpdk-dev] [PATCH] net/mlx5: check meta register width for modify 
field

> > The modify_field Flow API assumes that the META item is 32 bits wide.
> > But the C Register that is used for Meta item can be 16 or 32 bits wide
> > depending on kernel and firmware configurations.
> > Take this into consideration and use the appropriate META width.
> >
> > Fixes: 641dbe4fb0 ("net/mlx5: support modify field flow action")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Alexander Kozyrev 
> Acked-by: Viacheslav Ovsiienko 

Applied in next-net-mlx, thanks.

Re: [dpdk-dev] [PATCH v6] doc: add release milestones definition

[Ferruh Yigit]

On 3/28/2021 8:00 PM, Thomas Monjalon wrote:
> From: Asaf Penso 
> 
> Adding more information about the release milestones.
> This includes the scope of change, expectations, etc.
> 
> Signed-off-by: Asaf Penso 
> Signed-off-by: Thomas Monjalon 
> Acked-by: John McNamara 
> ---
> v2: fix styling format and add content in the commit message
> v3: change punctuation and avoid plural form when unneeded
> v4: avoid abbreviations, "Priority" in -rc, and reword as John suggests
> v5: note that release candidates may vary
> v6: merge RFC and proposal deadline, add roadmap link and reduce duplication
> ---
>  doc/guides/contributing/patches.rst | 69 +
>  1 file changed, 69 insertions(+)
> 
> diff --git a/doc/guides/contributing/patches.rst 
> b/doc/guides/contributing/patches.rst
> index 6dbbd5f8d1..1277443620 100644
> --- a/doc/guides/contributing/patches.rst
> +++ b/doc/guides/contributing/patches.rst
> @@ -177,6 +177,8 @@ Make your planned changes in the cloned ``dpdk`` repo. 
> Here are some guidelines
>  * Add documentation, if relevant, in the form of Doxygen comments or a User 
> Guide in RST format.
>See the :ref:`Documentation Guidelines `.
>  
> +* Code and related documentation must be updated atomically in the same 
> patch.
> +
>  Once the changes have been made you should commit them to your local repo.
>  
>  For small changes, that do not require specific explanations, it is better 
> to keep things together in the
> @@ -660,3 +662,70 @@ patch accepted. The general cycle for patch review and 
> acceptance is:
>   than rework of the original.
> * Trivial patches may be merged sooner than described above at the tree 
> committer's
>   discretion.
> +
> +
> +Milestones definition
> +-
> +
> +Each DPDK release has milestones that help everyone to converge to the 
> release date.
> +The following is a list of these milestones
> +together with concrete definitions and expectations,
> +for a typical release cycle (3 months ending after 4 release candidates).
> +The number and expectations of release candidates might vary slightly.
> +The schedule is updated in the `roadmap 
> `_.
> +
> +Roadmap
> +~~~
> +
> +* Announce new features in libraries, drivers, applications, and examples.
> +* To be published before the first day of the release cycle.
> +
> +Proposal Deadline
> +~
> +
> +* Must send an RFC or a complete v1 patch.
> +* Early RFC gives time for design review before complete implementation.
> +* Should include at least the API changes in libraries and applications.
> +* Library code should be quite complete at the deadline.
> +* Nice to have: driver implementation (full or draft), example code, and 
> documentation.
> +
> +rc1
> +~~~
> +
> +* Priority: new or updated API.

Can we just say API or libraries?

Overall what is the intention for the 'priority' information? Should we really
split release candidates for libraries, driver and applications?
We merge all as much as possible before -rc1.

Can we say this other-way around, API/library features can't be merged after 
-rc1.

And similarly driver features shouldn't be merged after -rc2, application
changes shouldn't merge after -rc3.
Fixes can be merged anytime before -rc4. After -rc4 only critical fixes and
documentation changes.

Just I want to highlight that for example we merge documentation updates
anytime, it doesn't have to wait -rc4, but below listing looks like different
part only allocated for different -rc, which is wrong as far as I know.

> +* New API should be defined and implemented in libraries.> +* The API should 
> include Doxygen documentation

s/should/must

> +  and be part of the relevant .rst files (library-specific and release 
> notes).
> +* API should be used in a test application (``/app``).
> +* At least one PMD should implement the API.
> +  It can be a draft but must be sent as a separate series.

I am not sure if "must be sent as a separate series" needs to be highlighted,
having all in the same series has a benefit to see bigger picture. If the driver
patches acked/reviewed by its maintainers, I think it can be merged in single
series.

> +* The above should be sent to the mailing list at least 2 weeks before -rc1.
> +* Nice to have: example code (``/examples``)
> +
> +rc2
> +~~~
> +
> +* Priority: drivers.
> +* New features should be implemented in drivers.

I already mentioned above, but this can cause misunderstanding. We want all
driver implementation to be ready for proposal deadline, same as other patches.
But because of its reduced scope (they don't affect all project but only
specific vendor), we are flexible to get driver features for -rc2 and -rc3 too.

Please check number of driver patches merged for a release, it is impossible to
manage them within period between -rc1 & -rc2.
Also some driver features are complex and big, they should be sent before
proposal deadline so that they can 

Re: [dpdk-dev] [PATCH] net/memif: fix missing Tx-bps stats for zero-copy

[Ferruh Yigit]

On 4/27/2021 7:30 AM, Tianyu Li wrote:
> Hi Jakub,
> 
> Any comments about the patch?
> 

+Damjan.

Hi Damjan,

Is Jakub still maintaining the net/memif?

> -Original Message-
> From: Ferruh Yigit  
> Sent: Wednesday, April 14, 2021 4:13 PM
> To: Tianyu Li ; Jakub Grajciar 
> Cc: dev@dpdk.org; nd ; sta...@dpdk.org
> Subject: Re: [PATCH] net/memif: fix missing Tx-bps stats for zero-copy
> 
> On 4/12/2021 9:22 AM, Tianyu Li wrote:
>> Fix the missing Tx-bps counter for memif zero-copy mode Before
>>Rx-pps:  6891450  Rx-bps:   3528438928
>>Tx-pps:  6891482  Tx-bps:0
>> After
>>Throughput (since last show)
>>Rx-pps: 11157056  Rx-bps:   5712413016
>>Tx-pps: 11157056  Tx-bps:   5712413016
>>
>> Fixes: 43b815d88188 ("net/memif: support zero-copy slave")
>> Cc: sta...@dpdk.org
>>
>> Signed-off-by: Tianyu Li 
>> ---
>>   drivers/net/memif/rte_eth_memif.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/net/memif/rte_eth_memif.c 
>> b/drivers/net/memif/rte_eth_memif.c
>> index 77c95bcb7..dd2825968 100644
>> --- a/drivers/net/memif/rte_eth_memif.c
>> +++ b/drivers/net/memif/rte_eth_memif.c
>> @@ -706,6 +706,7 @@ memif_tx_one_zc(struct pmd_process_private 
>> *proc_private, struct memif_queue *mq
>>  /* populate descriptor */
>>  d0 = &ring->desc[slot & mask];
>>  d0->length = rte_pktmbuf_data_len(mbuf);
>> +mq->n_bytes += rte_pktmbuf_data_len(mbuf);
>>  /* FIXME: get region index */
>>  d0->region = 1;
>>  d0->offset = rte_pktmbuf_mtod(mbuf, uint8_t *) -
>>
> 
> Looks good to me, but let's wait Jakub's review for it.
> 

Re: [dpdk-dev] [PATCH v3 1/2] net/mlx4: remove unnecessary wmb for Memory Region cache

[Slava Ovsiienko]

> -Original Message-
> From: dev  On Behalf Of Feifei Wang
> Sent: Tuesday, May 18, 2021 11:51
> To: Matan Azrad ; Shahaf Shuler
> 
> Cc: dev@dpdk.org; n...@arm.com; Feifei Wang ;
> Ruifeng Wang 
> Subject: [dpdk-dev] [PATCH v3 1/2] net/mlx4: remove unnecessary wmb for
> Memory Region cache
> 
> 'dev_gen' is a variable to trigger all cores to flush their local caches once 
> the
> global MR cache has been rebuilt.
> 
> This is due to MR cache's R/W lock can maintain synchronization between
> threads:
> 
> 1. dev_gen and global cache updating ordering inside the lock protected
> section does not matter. Because other threads cannot take the lock until
> global cache has been updated. Thus, in out of order platform, even if other
> agents firstly observe updated dev_gen but global does not update, they still
> have to wait the lock. As a result, it is unnecessary to add a wmb between
> global cache rebuilding and updating the dev_gen to keep the memory store
> order.
> 
> 2. Store-Release of unlock provides the implicit wmb at the level visible by
> software. This makes 'rebuilding global cache' and 'updating dev_gen' be
> observed before local_cache starts to be updated by other agents. Thus,
> wmb after 'updating dev_gen' can be removed.
> 
> Suggested-by: Ruifeng Wang 
> Signed-off-by: Feifei Wang 
> Reviewed-by: Ruifeng Wang 
Acked-by: Viacheslav Ovsiienko 

Re: [dpdk-dev] [PATCH v6] doc: add release milestones definition

[Thomas Monjalon]

18/05/2021 13:57, Ferruh Yigit:
> On 3/28/2021 8:00 PM, Thomas Monjalon wrote:
> > From: Asaf Penso 
> > 
> > Adding more information about the release milestones.
> > This includes the scope of change, expectations, etc.
[...]
> > +rc1
> > +~~~
> > +
> > +* Priority: new or updated API.
> 
> Can we just say API or libraries?

Yes

> Overall what is the intention for the 'priority' information? Should we really
> split release candidates for libraries, driver and applications?
> We merge all as much as possible before -rc1.

The idea is to simply reflect the priority
in case time is limited. But yes we always merge as much as possible.

> Can we say this other-way around, API/library features can't be merged after 
> -rc1.

Correct

> And similarly driver features shouldn't be merged after -rc2, application
> changes shouldn't merge after -rc3.
> Fixes can be merged anytime before -rc4. After -rc4 only critical fixes and
> documentation changes.
> 
> Just I want to highlight that for example we merge documentation updates
> anytime, it doesn't have to wait -rc4, but below listing looks like different
> part only allocated for different -rc, which is wrong as far as I know.

I understand the confusion and will try to make it clearer in next revision.

> > +* New API should be defined and implemented in libraries.
> > +* The API should include Doxygen documentation
> 
> s/should/must

OK

> > +  and be part of the relevant .rst files (library-specific and release 
> > notes).
> > +* API should be used in a test application (``/app``).
> > +* At least one PMD should implement the API.
> > +  It can be a draft but must be sent as a separate series.
> 
> I am not sure if "must be sent as a separate series" needs to be highlighted,
> having all in the same series has a benefit to see bigger picture. If the 
> driver
> patches acked/reviewed by its maintainers, I think it can be merged in single
> series.

That's not the same kind of review for driver and API,
not the same time constraint, and not the same iterations.
I think it is more practical to suggest separate,
but it should not be "must".

> > +* The above should be sent to the mailing list at least 2 weeks before 
> > -rc1.
> > +* Nice to have: example code (``/examples``)
> > +
> > +rc2
> > +~~~
> > +
> > +* Priority: drivers.
> > +* New features should be implemented in drivers.
> 
> I already mentioned above, but this can cause misunderstanding. We want all
> driver implementation to be ready for proposal deadline, same as other 
> patches.
> But because of its reduced scope (they don't affect all project but only
> specific vendor), we are flexible to get driver features for -rc2 and -rc3 
> too.

-rc3 really? It should be exceptional so not mentioned here.

> Please check number of driver patches merged for a release, it is impossible 
> to
> manage them within period between -rc1 & -rc2.
> Also some driver features are complex and big, they should be sent before
> proposal deadline so that they can be reviewed for the release.

Yes sooner is better. The doc is about deadline + priorities,
showing the no-go limits, without warranty of merge if all good.
Is there a contradiction?

> > +* A driver change should include documentation
> 
> s/should/must

Sometimes there is no doc to change. Is "must" confusing?

> > +  in the relevant .rst files (driver-specific and release notes).
> > +* The above should be sent to the mailing list at least 2 weeks before 
> > -rc2.
> > +
> > +rc3
> > +~~~
> > +
> > +* Priority: applications.
> > +* New functionality that does not depend on libraries update
> > +  can be integrated as part of -rc3.
> 
> Again for same issue, let me share my understanding,
> the -rc1 has been tested widely, after that each -rc gets less and less tests.
> So the -rc1 should have API/library changes, so that they will be tested more
> and will have more time to fix any issues, since library changes has biggest
> impact for the project.
> 
> Next biggest impact is drivers.
> 
> Applications and unit tests are internal to DPDK, they have no user impact, 
> that
> is why we can get more risk with them and they can be merged even as late as 
> rc3.
> 
> And documentation doesn't have anything related to testing, or they don't
> introduce any risk for specific release, so they are merged until last stage 
> of
> the release.

Yes

> > +* The application should include documentation in the relevant .rst files
> > +  (application-specific and release notes if significant).
> 
> s/should/must
> 
> > +* It may be the last opportunity for miscellaneous changes.
> 
> This is very vague, what does misch changes mean?

Scripts, code cleanup, yes it is vague, we can remove.

> > +* Libraries and drivers cleanup are allowed.
> > +* Small driver reworks.
> > +* Critical and minor bug fixes.
> > +
> > +rc4
> > +~~~
> > +
> > +* Documentation updates.
> > +* Critical bug fixes.

Re: [dpdk-dev] [PATCH v5 2/2] net/hns3: refactor SVE code compile method

[fengchengwen]

Hi, Thomas, Ferruh

This patch is part of the hns3 SVE compilation solution and can work 
independently. Could you review it ?

Best regards

On 2021/5/14 22:12, Honnappa Nagarahalli wrote:
> 
> 
>>
>> Currently, the SVE code is compiled only when -march supports SVE (e.g. '-
>> march=armv8.2a+sve'), there maybe some problem[1] with this approach.
>>
>> The solution:
>> a. If the minimum instruction set support SVE then compiles it.
>> b. Else if the compiler support SVE then compiles it.
>> c. Otherwise don't compile it.
>>
>> [1] https://mails.dpdk.org/archives/dev/2021-April/208189.html
>>
>> Fixes: 8c25b02b082a ("net/hns3: fix enabling SVE Rx/Tx")
>> Fixes: 952ebacce4f2 ("net/hns3: support SVE Rx")
>> Cc: sta...@dpdk.org
>>
>> Signed-off-by: Chengwen Feng 
> Looks good to me.
> Reviewed-by: Honnappa Nagarahalli 
> 
>> ---
>>  drivers/net/hns3/hns3_rxtx.c |  2 +-
>>  drivers/net/hns3/meson.build | 13 +
>>  2 files changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/hns3/hns3_rxtx.c b/drivers/net/hns3/hns3_rxtx.c
>> index 1d7a769..4ef20c6 100644
>> --- a/drivers/net/hns3/hns3_rxtx.c
>> +++ b/drivers/net/hns3/hns3_rxtx.c
>> @@ -2808,7 +2808,7 @@ hns3_get_default_vec_support(void)
>>  static bool
>>  hns3_get_sve_support(void)
>>  {
>> -#if defined(RTE_ARCH_ARM64) && defined(__ARM_FEATURE_SVE)
>> +#if defined(CC_SVE_SUPPORT)
>>  if (rte_vect_get_max_simd_bitwidth() < RTE_VECT_SIMD_256)
>>  return false;
>>  if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SVE))
>> diff --git a/drivers/net/hns3/meson.build b/drivers/net/hns3/meson.build
>> index 53c7df7..8563d70 100644
>> --- a/drivers/net/hns3/meson.build
>> +++ b/drivers/net/hns3/meson.build
>> @@ -35,7 +35,20 @@ deps += ['hash']
>>
>>  if arch_subdir == 'arm' and dpdk_conf.get('RTE_ARCH_64')
>>  sources += files('hns3_rxtx_vec.c')
>> +
>> +# compile SVE when:
>> +# a. support SVE in minimum instruction set baseline
>> +# b. it's not minimum instruction set, but compiler support
>>  if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != ''
>> +cflags += ['-DCC_SVE_SUPPORT']
>>  sources += files('hns3_rxtx_vec_sve.c')
>> +elif cc.has_argument('-march=armv8.2-a+sve')
>> +cflags += ['-DCC_SVE_SUPPORT']
>> +hns3_sve_lib = static_library('hns3_sve_lib',
>> +'hns3_rxtx_vec_sve.c',
>> +dependencies: [static_rte_ethdev],
>> +include_directories: includes,
>> +c_args: [cflags, '-march=armv8.2-a+sve'])
>> +objs += hns3_sve_lib.extract_objects('hns3_rxtx_vec_sve.c')
>>  endif
>>  endif
>> --
>> 2.8.1
> 
> 
> .
> 

Re: [dpdk-dev] [PATCH] net/mlx5: check meta register width for modify field

[Slava Ovsiienko]

Yes, It is palliative, it is not perfect, and, IMO, it is crucial - with always 
applying 32-bit wide values we can destroy the kernel part of reg_c0
in some extended metadata modes and this would lead to steering malfunction.

With best regards,
Slava

From: Asaf Penso 
Sent: Tuesday, May 18, 2021 14:33
To: NBU-Contact-Thomas Monjalon ; Alexander Kozyrev 
; Raslan Darawsheh 
Cc: dev@dpdk.org; sta...@dpdk.org; Raslan Darawsheh ; Matan 
Azrad ; Slava Ovsiienko 
Subject: Re: [dpdk-dev] [PATCH] net/mlx5: check meta register width for modify 
field

I didn't plan to have it in this release since it's not critical and I'm afraid 
of such changes... I didn't mention this in the status mail today.
@Raslan Darawsheh can we check tomorrow regression 
result?
If not good we'll need to revert it.

Regards,
Asaf Penso

From: Thomas Monjalon mailto:tho...@monjalon.net>>
Sent: Tuesday, May 18, 2021 12:43:34 PM
To: Alexander Kozyrev mailto:akozy...@nvidia.com>>
Cc: dev@dpdk.org mailto:dev@dpdk.org>>; 
sta...@dpdk.org 
mailto:sta...@dpdk.org>>; Raslan Darawsheh 
mailto:rasl...@nvidia.com>>; Matan Azrad 
mailto:ma...@nvidia.com>>; Slava Ovsiienko 
mailto:viachesl...@nvidia.com>>; Asaf Penso 
mailto:as...@nvidia.com>>
Subject: Re: [dpdk-dev] [PATCH] net/mlx5: check meta register width for modify 
field

> > The modify_field Flow API assumes that the META item is 32 bits wide.
> > But the C Register that is used for Meta item can be 16 or 32 bits wide
> > depending on kernel and firmware configurations.
> > Take this into consideration and use the appropriate META width.
> >
> > Fixes: 641dbe4fb0 ("net/mlx5: support modify field flow action")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Alexander Kozyrev 
> > mailto:akozy...@nvidia.com>>
> Acked-by: Viacheslav Ovsiienko 
> mailto:viachesl...@nvidia.com>>

Applied in next-net-mlx, thanks.

Re: [dpdk-dev] [PATCH v5 3/3] devtools: check flow API doc tables

[Thomas Monjalon]

13/05/2021 20:40, Ferruh Yigit:
> On 4/7/2021 11:33 PM, Thomas Monjalon wrote:
> > +changed_files()
> > +{
> > +   [ -n "$files" ] ||
> > +   files=$(git diff-tree --name-only -r $trusted_commit..)
> > +   echo "$files"
> > +}
> > +
> > +has_code_change() # 
> > +{
> > +   test -n "$(git log --format='%h' -S"$1" $trusted_commit..)"
> > +}
> > +
> > +has_file_change() # 
> > +{
> > +   changed_files | grep -q "$1"
> > +}
> > +
> > +changed_net_drivers()
> > +{
> > +   net_paths='drivers/net/|doc/guides/nics/features/'
> > +   [ -n "$drivers" ] ||
> > +   drivers=$(changed_files |
> > +   sed -rn "s,^($net_paths)([^./]*).*,\2,p")
> > +   echo "$drivers"
> > +}
> 
> I will not reviewed in details yet, but first observation,
> when 'trusted_commit' argument is used, the drivers list has many duplicated
> entries which makes the output redundant and makes script take too much time.
> Getting only unique list may help on it.

Yes good catch, thanks.

[dpdk-dev] [PATCH] doc: add new feature for connection tracking

[Bing Zhao]

The feature support of connection tracking is added in the feature
table of networking drivers. The feature overview part is also
updated.

Signed-off-by: Bing Zhao 
---
 doc/guides/nics/features.rst | 12 
 doc/guides/nics/features/default.ini |  1 +
 doc/guides/nics/features/mlx5.ini|  1 +
 3 files changed, 14 insertions(+)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index f6d30d0af3..a7b8d250a6 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -923,6 +923,18 @@ Supports to get Rx/Tx packet burst mode information.
 * **[implements] eth_dev_ops**: ``rx_burst_mode_get``, ``tx_burst_mode_get``.
 * **[related] API**: ``rte_eth_rx_burst_mode_get()``, 
``rte_eth_tx_burst_mode_get()``.
 
+.. _nic_features_connection_tracking:
+
+Connection tracking
+---
+
+Supports conntrack offload by HW module.
+
+* **[uses]   rte_flow_item**: ``rte_flow_item_conntrack``.
+* **[uses]   rte_flow_action**: ``rte_flow_action_conntrack``.
+* **[uses]   user config**: ``rte_flow_modify_conntrack``.
+* **[related] API**: ``rte_flow_action_handle_create()``, 
``rte_flow_action_handle_destroy()``, ``rte_flow_action_handle_update()``, 
``rte_flow_action_handle_query()``.
+
 .. _nic_features_other:
 
 Other dev ops not represented by a Feature
diff --git a/doc/guides/nics/features/default.ini 
b/doc/guides/nics/features/default.ini
index 8046bd121e..0deb4ef547 100644
--- a/doc/guides/nics/features/default.ini
+++ b/doc/guides/nics/features/default.ini
@@ -66,6 +66,7 @@ Module EEPROM dump   =
 Registers dump   =
 LED  =
 Multiprocess aware   =
+Connection tracking  =
 FreeBSD  =
 Linux=
 Windows  =
diff --git a/doc/guides/nics/features/mlx5.ini 
b/doc/guides/nics/features/mlx5.ini
index ddd131da16..45dbe75d07 100644
--- a/doc/guides/nics/features/mlx5.ini
+++ b/doc/guides/nics/features/mlx5.ini
@@ -45,6 +45,7 @@ Stats per queue  = Y
 FW version   = Y
 Module EEPROM dump   = Y
 Multiprocess aware   = Y
+Connection tracking  = Y
 Linux= Y
 Windows  = P
 ARMv8= Y
-- 
2.27.0

Re: [dpdk-dev] [PATCH v7 17/17] doc: update mlx5 support for conntrack

[Bing Zhao]

Hi Ferruh & Thomas,

> -Original Message-
> From: Ferruh Yigit 
> Sent: Thursday, May 6, 2021 2:21 AM
> To: Bing Zhao ; Slava Ovsiienko
> ; Matan Azrad ; NBU-
> Contact-Thomas Monjalon 
> Cc: dev@dpdk.org; Ori Kam ; Raslan Darawsheh
> 
> Subject: Re: [dpdk-dev] [PATCH v7 17/17] doc: update mlx5 support
> for conntrack
> 
> External email: Use caution opening links or attachments
> 
> 
> On 5/5/2021 1:23 PM, Bing Zhao wrote:
> > In the release notes and mlx5 NIC document, the support and
> limitation
> > of connection tracking are added.
> >
> 
> A new NIC feature seems added, better to be acked beyond the mlx5
> scope.
> 
> > Signed-off-by: Bing Zhao 
> > Acked-by: Viacheslav Ovsiienko 
> > ---
> >  doc/guides/nics/features/default.ini   |  1 +
> >  doc/guides/nics/features/mlx5.ini  |  1 +
> >  doc/guides/nics/mlx5.rst   | 14 ++
> >  doc/guides/rel_notes/release_21_05.rst |  2 ++
> >  4 files changed, 18 insertions(+)
> >
> > diff --git a/doc/guides/nics/features/default.ini
> > b/doc/guides/nics/features/default.ini
> > index 8046bd121e..0deb4ef547 100644
> > --- a/doc/guides/nics/features/default.ini
> > +++ b/doc/guides/nics/features/default.ini
> > @@ -66,6 +66,7 @@ Module EEPROM dump   =
> >  Registers dump   =
> >  LED  =
> >  Multiprocess aware   =
> > +Connection tracking  =
> >  FreeBSD  =
> >  Linux=
> >  Windows  =
> 
> Need to update 'doc/guides/nics/features.rst' too, to describe new
> feature.
> 
> I will drop new feature part in next-net, it can be sent as separate
> patch later. Will keep driver and release notes documentation.

http://patches.dpdk.org/project/dpdk/patch/20210518125235.90185-1-bi...@nvidia.com/

A new patch with the updated description is in the link below.
I checked the file of the "features.rst" again, it seems that only the API for 
ethdev are
described.
Even if conntrack is a new feature and provided by new HW, but the only way to 
operate on it
is to use rte_flow API now.

So I am not quite sure if it is possible and proper to list the feature in the 
driver feature
lists table. What do you think?

BR. Bing

Re: [dpdk-dev] [PATCH v5 2/2] net/hns3: refactor SVE code compile method

[Honnappa Nagarahalli]

> 
> Hi, Thomas, Ferruh
> 
> This patch is part of the hns3 SVE compilation solution and can work
> independently. Could you review it ?
I believe this patch is targeted for 20.08 release (as 20.05 is already close 
to completion), is my understanding correct?
If it is targeted for 20.08, it will give us sometime to do few experiments 
with the generic approach?

> 
> Best regards
> 
> On 2021/5/14 22:12, Honnappa Nagarahalli wrote:
> > 
> >
> >>
> >> Currently, the SVE code is compiled only when -march supports SVE
> >> (e.g. '- march=armv8.2a+sve'), there maybe some problem[1] with this
> approach.
> >>
> >> The solution:
> >> a. If the minimum instruction set support SVE then compiles it.
> >> b. Else if the compiler support SVE then compiles it.
> >> c. Otherwise don't compile it.
> >>
> >> [1] https://mails.dpdk.org/archives/dev/2021-April/208189.html
> >>
> >> Fixes: 8c25b02b082a ("net/hns3: fix enabling SVE Rx/Tx")
> >> Fixes: 952ebacce4f2 ("net/hns3: support SVE Rx")
> >> Cc: sta...@dpdk.org
> >>
> >> Signed-off-by: Chengwen Feng 
> > Looks good to me.
> > Reviewed-by: Honnappa Nagarahalli 
> >
> >> ---
> >>  drivers/net/hns3/hns3_rxtx.c |  2 +-  drivers/net/hns3/meson.build |
> >> 13 +
> >>  2 files changed, 14 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/net/hns3/hns3_rxtx.c
> >> b/drivers/net/hns3/hns3_rxtx.c index 1d7a769..4ef20c6 100644
> >> --- a/drivers/net/hns3/hns3_rxtx.c
> >> +++ b/drivers/net/hns3/hns3_rxtx.c
> >> @@ -2808,7 +2808,7 @@ hns3_get_default_vec_support(void)
> >>  static bool
> >>  hns3_get_sve_support(void)
> >>  {
> >> -#if defined(RTE_ARCH_ARM64) && defined(__ARM_FEATURE_SVE)
> >> +#if defined(CC_SVE_SUPPORT)
> >>if (rte_vect_get_max_simd_bitwidth() < RTE_VECT_SIMD_256)
> >>return false;
> >>if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SVE))
> >> diff --git a/drivers/net/hns3/meson.build
> >> b/drivers/net/hns3/meson.build index 53c7df7..8563d70 100644
> >> --- a/drivers/net/hns3/meson.build
> >> +++ b/drivers/net/hns3/meson.build
> >> @@ -35,7 +35,20 @@ deps += ['hash']
> >>
> >>  if arch_subdir == 'arm' and dpdk_conf.get('RTE_ARCH_64')
> >>  sources += files('hns3_rxtx_vec.c')
> >> +
> >> +# compile SVE when:
> >> +# a. support SVE in minimum instruction set baseline
> >> +# b. it's not minimum instruction set, but compiler support
> >>  if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != ''
> >> +cflags += ['-DCC_SVE_SUPPORT']
> >>  sources += files('hns3_rxtx_vec_sve.c')
> >> +elif cc.has_argument('-march=armv8.2-a+sve')
> >> +cflags += ['-DCC_SVE_SUPPORT']
> >> +hns3_sve_lib = static_library('hns3_sve_lib',
> >> +'hns3_rxtx_vec_sve.c',
> >> +dependencies: [static_rte_ethdev],
> >> +include_directories: includes,
> >> +c_args: [cflags, '-march=armv8.2-a+sve'])
> >> +objs += hns3_sve_lib.extract_objects('hns3_rxtx_vec_sve.c')
> >>  endif
> >>  endif
> >> --
> >> 2.8.1
> >
> >
> > .
> >

Re: [dpdk-dev] [PATCH v5 2/2] net/hns3: refactor SVE code compile method

[Honnappa Nagarahalli]

> 
> >
> > Hi, Thomas, Ferruh
> >
> > This patch is part of the hns3 SVE compilation solution and can work
> > independently. Could you review it ?
> I believe this patch is targeted for 20.08 release (as 20.05 is already close 
> to
> completion), is my understanding correct?
> If it is targeted for 20.08, it will give us sometime to do few experiments 
> with
> the generic approach?
Apologies, please ignore my comments, I misunderstood the patch. I do not have 
any issues with this patch.

> 
> >
> > Best regards
> >
> > On 2021/5/14 22:12, Honnappa Nagarahalli wrote:
> > > 
> > >
> > >>
> > >> Currently, the SVE code is compiled only when -march supports SVE
> > >> (e.g. '- march=armv8.2a+sve'), there maybe some problem[1] with
> > >> this
> > approach.
> > >>
> > >> The solution:
> > >> a. If the minimum instruction set support SVE then compiles it.
> > >> b. Else if the compiler support SVE then compiles it.
> > >> c. Otherwise don't compile it.
> > >>
> > >> [1] https://mails.dpdk.org/archives/dev/2021-April/208189.html
> > >>
> > >> Fixes: 8c25b02b082a ("net/hns3: fix enabling SVE Rx/Tx")
> > >> Fixes: 952ebacce4f2 ("net/hns3: support SVE Rx")
> > >> Cc: sta...@dpdk.org
> > >>
> > >> Signed-off-by: Chengwen Feng 
> > > Looks good to me.
> > > Reviewed-by: Honnappa Nagarahalli 
> > >
> > >> ---
> > >>  drivers/net/hns3/hns3_rxtx.c |  2 +-  drivers/net/hns3/meson.build
> > >> |
> > >> 13 +
> > >>  2 files changed, 14 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/drivers/net/hns3/hns3_rxtx.c
> > >> b/drivers/net/hns3/hns3_rxtx.c index 1d7a769..4ef20c6 100644
> > >> --- a/drivers/net/hns3/hns3_rxtx.c
> > >> +++ b/drivers/net/hns3/hns3_rxtx.c
> > >> @@ -2808,7 +2808,7 @@ hns3_get_default_vec_support(void)
> > >>  static bool
> > >>  hns3_get_sve_support(void)
> > >>  {
> > >> -#if defined(RTE_ARCH_ARM64) && defined(__ARM_FEATURE_SVE)
> > >> +#if defined(CC_SVE_SUPPORT)
> > >>  if (rte_vect_get_max_simd_bitwidth() < RTE_VECT_SIMD_256)
> > >>  return false;
> > >>  if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SVE))
> > >> diff --git a/drivers/net/hns3/meson.build
> > >> b/drivers/net/hns3/meson.build index 53c7df7..8563d70 100644
> > >> --- a/drivers/net/hns3/meson.build
> > >> +++ b/drivers/net/hns3/meson.build
> > >> @@ -35,7 +35,20 @@ deps += ['hash']
> > >>
> > >>  if arch_subdir == 'arm' and dpdk_conf.get('RTE_ARCH_64')
> > >>  sources += files('hns3_rxtx_vec.c')
> > >> +
> > >> +# compile SVE when:
> > >> +# a. support SVE in minimum instruction set baseline
> > >> +# b. it's not minimum instruction set, but compiler support
> > >>  if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != ''
> > >> +cflags += ['-DCC_SVE_SUPPORT']
> > >>  sources += files('hns3_rxtx_vec_sve.c')
> > >> +elif cc.has_argument('-march=armv8.2-a+sve')
> > >> +cflags += ['-DCC_SVE_SUPPORT']
> > >> +hns3_sve_lib = static_library('hns3_sve_lib',
> > >> +'hns3_rxtx_vec_sve.c',
> > >> +dependencies: [static_rte_ethdev],
> > >> +include_directories: includes,
> > >> +c_args: [cflags, '-march=armv8.2-a+sve'])
> > >> +objs +=
> > >> + hns3_sve_lib.extract_objects('hns3_rxtx_vec_sve.c')
> > >>  endif
> > >>  endif
> > >> --
> > >> 2.8.1
> > >
> > >
> > > .
> > >

Re: [dpdk-dev] [PATCH v6] doc: add release milestones definition

[Ferruh Yigit]

On 5/18/2021 1:25 PM, Thomas Monjalon wrote:
> 18/05/2021 13:57, Ferruh Yigit:
>> On 3/28/2021 8:00 PM, Thomas Monjalon wrote:
>>> From: Asaf Penso 
>>>
>>> Adding more information about the release milestones.
>>> This includes the scope of change, expectations, etc.
> [...]
>>> +rc1
>>> +~~~
>>> +
>>> +* Priority: new or updated API.
>>
>> Can we just say API or libraries?
> 
> Yes
> 
>> Overall what is the intention for the 'priority' information? Should we 
>> really
>> split release candidates for libraries, driver and applications?
>> We merge all as much as possible before -rc1.
> 
> The idea is to simply reflect the priority
> in case time is limited. But yes we always merge as much as possible.
> 
>> Can we say this other-way around, API/library features can't be merged after 
>> -rc1.
> 
> Correct
> 
>> And similarly driver features shouldn't be merged after -rc2, application
>> changes shouldn't merge after -rc3.
>> Fixes can be merged anytime before -rc4. After -rc4 only critical fixes and
>> documentation changes.
>>
>> Just I want to highlight that for example we merge documentation updates
>> anytime, it doesn't have to wait -rc4, but below listing looks like different
>> part only allocated for different -rc, which is wrong as far as I know.
> 
> I understand the confusion and will try to make it clearer in next revision.
> 
>>> +* New API should be defined and implemented in libraries.
>>> +* The API should include Doxygen documentation
>>
>> s/should/must
> 
> OK
> 
>>> +  and be part of the relevant .rst files (library-specific and release 
>>> notes).
>>> +* API should be used in a test application (``/app``).
>>> +* At least one PMD should implement the API.
>>> +  It can be a draft but must be sent as a separate series.
>>
>> I am not sure if "must be sent as a separate series" needs to be highlighted,
>> having all in the same series has a benefit to see bigger picture. If the 
>> driver
>> patches acked/reviewed by its maintainers, I think it can be merged in single
>> series.
> 
> That's not the same kind of review for driver and API,
> not the same time constraint, and not the same iterations.
> I think it is more practical to suggest separate,
> but it should not be "must".
> 
>>> +* The above should be sent to the mailing list at least 2 weeks before 
>>> -rc1.
>>> +* Nice to have: example code (``/examples``)
>>> +
>>> +rc2
>>> +~~~
>>> +
>>> +* Priority: drivers.
>>> +* New features should be implemented in drivers.
>>
>> I already mentioned above, but this can cause misunderstanding. We want all
>> driver implementation to be ready for proposal deadline, same as other 
>> patches.
>> But because of its reduced scope (they don't affect all project but only
>> specific vendor), we are flexible to get driver features for -rc2 and -rc3 
>> too.
> 
> -rc3 really? It should be exceptional so not mentioned here.
> 

In practice we are having it, but agree to have it exceptional and not mention
in the guide.

>> Please check number of driver patches merged for a release, it is impossible 
>> to
>> manage them within period between -rc1 & -rc2.
>> Also some driver features are complex and big, they should be sent before
>> proposal deadline so that they can be reviewed for the release.
> 
> Yes sooner is better. The doc is about deadline + priorities,
> showing the no-go limits, without warranty of merge if all good.
> Is there a contradiction?
> 

My concern is document can be read as, it is normal/expected to send driver
patches after -rc1, because this documents as -rc2 task is driver patches.

I am OK with it if it is clear that deadline is -rc2, but normal/expected is to
have driver patches also before proposal deadline.

>>> +* A driver change should include documentation
>>
>> s/should/must
> 
> Sometimes there is no doc to change. Is "must" confusing?
> 

I believe we can improve our documentation, there are some new features driver
or library, not documented at all.

But you are right, there may be driver features that may not require any
documentation, but if there is a feature big enough for documentation, I am for
having documentation as a 'must', not sure how to clearly document this.

>>> +  in the relevant .rst files (driver-specific and release notes).
>>> +* The above should be sent to the mailing list at least 2 weeks before 
>>> -rc2.
>>> +
>>> +rc3
>>> +~~~
>>> +
>>> +* Priority: applications.
>>> +* New functionality that does not depend on libraries update
>>> +  can be integrated as part of -rc3.
>>
>> Again for same issue, let me share my understanding,
>> the -rc1 has been tested widely, after that each -rc gets less and less 
>> tests.
>> So the -rc1 should have API/library changes, so that they will be tested more
>> and will have more time to fix any issues, since library changes has biggest
>> impact for the project.
>>
>> Next biggest impact is drivers.
>>
>> Applications and unit tests are internal to DPDK, they have no user impact, 
>> that
>> is why we

Re: [dpdk-dev] [PATCH] doc: add new feature for connection tracking

[Ferruh Yigit]

On 5/18/2021 1:52 PM, Bing Zhao wrote:
> The feature support of connection tracking is added in the feature
> table of networking drivers. The feature overview part is also
> updated.
> 
> Signed-off-by: Bing Zhao 

Hi Bing,

Thomas is suggestion another table to list flow API item/actions in separate
table, can this change can be replaced by it?

Re: [dpdk-dev] [PATCH] doc: add new feature for connection tracking

[Thomas Monjalon]

18/05/2021 15:16, Ferruh Yigit:
> On 5/18/2021 1:52 PM, Bing Zhao wrote:
> > The feature support of connection tracking is added in the feature
> > table of networking drivers. The feature overview part is also
> > updated.
> > 
> > Signed-off-by: Bing Zhao 
> 
> Hi Bing,
> 
> Thomas is suggestion another table to list flow API item/actions in separate
> table, can this change can be replaced by it?

Correct, I am adding conntrack as part of rte_flow features table.
Please don't add it in the global table.

Re: [dpdk-dev] [EXT] Re: [PATCH] config/arm: add ability to express arch extensions

[Honnappa Nagarahalli]

> > > >
> > > >>
> > > >> >> >>
> > > >> >> >> >
> > > >> >> >> > The patch still holds true for CRC though as it is listed
> > > >> >> >> > separately below
> > > >> >> >> > https://urldefense.proofpoint.com/v2/url?u=https-
> > > >> >> >3A__developer.arm.com_architectures_cpu-2Darchitecture_a-
> > > >> >>
> > > >>2D&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=E3SgYMjtKCMVsB-
> > > >> >> >fmvgGV3o-
> > > >> >>
> > > >>
> > > >>>g_fjLhk5Pupi9ijohpc&m=i3kC8htMiHjXMoJWUn6QlDVZQCblbFrIJyMc
> > > >> >W
> > > >> >>
> > > >>
> > > >>>d9nAmM&s=fA4SM6O3iC2HXIK1qSbOHzxVeHoYqcfUebEOwioHC7c&
> > > >e
> > > >> >=
> > > >> >> >> > profile/exploration-tools/feature-names-for-a-profile
> > > >> >> >CRC is mandatory starting in V8.1, refer to Arm-ARM document.
> > > >> >> >
> > > >> >> >> >
> > > >> >> >> > Also, looks like sve2 support in n2 core might be
> > > >> >> >> > optional as per
> > > >> >> >above doc?
> > > >> >> >> I need to check on this. Some of the info here might not be
> > > >public
> > > >> >yet.
> > > >> >> >I found [1]. SVE2 is mandatory feature.
> > > >> >> >
> > > >> >>
> > > >> >> I see thanks for the info I will remove extension from cnxk.
> > > >> >>
> > > >> >> Do you think the extension infra is still useful for other cases? 
> > > >> >> i.e.
> > > >> >older cores
> > > >> >> or cases where vendor wants to enable some extensions by
> > > >default?
> > > >> >>
> > > >> >> I found a document[1] which describes about extensions not
> > > >enabled
> > > >> >by
> > > >> >> default but supported by a given march.
> > > >> >> In case of n2 I think memory tagging is one such feature
> > > >> >I think the reference is providing a different information than
> > > >> >what you are trying to achieve here.
> > > >> >
> > > >> >It looks like you are trying to address a use case where in the
> > > >> >same CPU IP has different features enabled/disabled on different
> SoCs.
> > > >> >This is a valid use case from crypto perspective (due to export
> > > >control
> > > >> >reasons) where-in 2 different SoCs might have crypto
> > > >enabled/disabled.
> > > >> >I am not sure if other features can be enabled/disabled. But,
> > > >> >Crypto feature is a good enough reason to address such a use case.
> > > >>
> > > >> Yes, that's my intension apologies if the commit log doesn't
> > > >> clarify it
> > > >properly.
> > > >>
> > > >> >
> > > >> >IMO,  we should capture the SoC specific details in SoC specific
> > > >> >files, in this case in 'arm64_cn10k_linux_gcc'. I believe there
> > > >> >were some challenges in doing this.
> > > >>
> > > >> Since, all the flags are populated through soc_* variable and
> > > >> arm64_cn10k_linux_gcc also translates to soc_cn10k I believe the
> > > >extensions
> > > >> should be reported through
> > > >> soc_* variables.
> > > >IMO, there will be more SoCs in the future. I prefer to not grow
> > > >meson.build.
> > >
> > > Problem is native build wouldn't read arm64_*_linux_gcc, it will be
> > > really hard to parse it and read extensions if they are placed there.
> > >
> > Since our minimum meson version for DPDK is >0.49, would native-build
> > files[1] for meson offer a solution here?
> >
> 
> We need a place to define SoC specific configuration that would be accessible
> to both native and cross builds. A separate file for each SoC would be great
> and I've thought about native files in the past where we'd have:
> 1. an SoC specific file such as soc_armada_config 2. a cross file for each
> compiler, such as arm64_linux_gcc
> 
> This we'd we could use the first file in native builds (and we wouldn't need 
> the
> platform parameter) and we'd use both files in cross builds.
> 
> I have a hazy memory of trying something similar in 0.47.1 (splitting the 
> cross
> file into SoC config and the rest), but it didn't work, both of the files 
> needed
> all of the mandatory sections and I suspect this is still true judging from 
> the
> docs (even for the latest Meson).
Are we concluding there is no solution?

> 
> > /Bruce
> >
> > [1] https://mesonbuild.com/Native-environments.html
> 

Re: [dpdk-dev] [PATCH v5 2/2] net/hns3: refactor SVE code compile method

[Ferruh Yigit]

On 5/18/2021 1:41 PM, fengchengwen wrote:
> Hi, Thomas, Ferruh
> 
> This patch is part of the hns3 SVE compilation solution and can work 
> independently. Could you review it ?
> 

Hi Chengwen,

This patch has been missed as being part of the config file set. As far as I
understand you want this change for v20.05, let me check it right now as today
is the -rc4 release day.

> Best regards
> 
> On 2021/5/14 22:12, Honnappa Nagarahalli wrote:
>> 
>>
>>>
>>> Currently, the SVE code is compiled only when -march supports SVE (e.g. '-
>>> march=armv8.2a+sve'), there maybe some problem[1] with this approach.
>>>
>>> The solution:
>>> a. If the minimum instruction set support SVE then compiles it.
>>> b. Else if the compiler support SVE then compiles it.
>>> c. Otherwise don't compile it.
>>>
>>> [1] https://mails.dpdk.org/archives/dev/2021-April/208189.html
>>>
>>> Fixes: 8c25b02b082a ("net/hns3: fix enabling SVE Rx/Tx")
>>> Fixes: 952ebacce4f2 ("net/hns3: support SVE Rx")
>>> Cc: sta...@dpdk.org
>>>
>>> Signed-off-by: Chengwen Feng 
>> Looks good to me.
>> Reviewed-by: Honnappa Nagarahalli 
>>
>>> ---
>>>  drivers/net/hns3/hns3_rxtx.c |  2 +-
>>>  drivers/net/hns3/meson.build | 13 +
>>>  2 files changed, 14 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/hns3/hns3_rxtx.c b/drivers/net/hns3/hns3_rxtx.c
>>> index 1d7a769..4ef20c6 100644
>>> --- a/drivers/net/hns3/hns3_rxtx.c
>>> +++ b/drivers/net/hns3/hns3_rxtx.c
>>> @@ -2808,7 +2808,7 @@ hns3_get_default_vec_support(void)
>>>  static bool
>>>  hns3_get_sve_support(void)
>>>  {
>>> -#if defined(RTE_ARCH_ARM64) && defined(__ARM_FEATURE_SVE)
>>> +#if defined(CC_SVE_SUPPORT)
>>> if (rte_vect_get_max_simd_bitwidth() < RTE_VECT_SIMD_256)
>>> return false;
>>> if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SVE))
>>> diff --git a/drivers/net/hns3/meson.build b/drivers/net/hns3/meson.build
>>> index 53c7df7..8563d70 100644
>>> --- a/drivers/net/hns3/meson.build
>>> +++ b/drivers/net/hns3/meson.build
>>> @@ -35,7 +35,20 @@ deps += ['hash']
>>>
>>>  if arch_subdir == 'arm' and dpdk_conf.get('RTE_ARCH_64')
>>>  sources += files('hns3_rxtx_vec.c')
>>> +
>>> +# compile SVE when:
>>> +# a. support SVE in minimum instruction set baseline
>>> +# b. it's not minimum instruction set, but compiler support
>>>  if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != ''
>>> +cflags += ['-DCC_SVE_SUPPORT']
>>>  sources += files('hns3_rxtx_vec_sve.c')
>>> +elif cc.has_argument('-march=armv8.2-a+sve')
>>> +cflags += ['-DCC_SVE_SUPPORT']
>>> +hns3_sve_lib = static_library('hns3_sve_lib',
>>> +'hns3_rxtx_vec_sve.c',
>>> +dependencies: [static_rte_ethdev],
>>> +include_directories: includes,
>>> +c_args: [cflags, '-march=armv8.2-a+sve'])
>>> +objs += hns3_sve_lib.extract_objects('hns3_rxtx_vec_sve.c')
>>>  endif
>>>  endif
>>> --
>>> 2.8.1
>>
>>
>> .
>>
> 

Re: [dpdk-dev] [PATCH v4 1/2] config/arm: select most suitable -march for kunpeng soc

[Honnappa Nagarahalli]

> >>
> >> On Thu, May 13, 2021 at 6:41 PM Chengwen Feng
> >>  wrote:
> >>>
> >>> Currently, the soc_kunpeng930 declares
> >>> '-march=armv8.2-a+crypto+sve', but some compiler doesn't recognize
> >>> the march because it doesn't support sve.
> >>>
> >>> To solve this bug we use the following scheme:
> >>> 1. Define 'march_base' tuple which defines support march, it should
> >>> arrange from lower to higher.
> >>> e.g. 'march_base' : ['-march=armv8-a', '-march=armv8.2-a'] 2. Define
> >>> 'march_feature' tuple which defines support feature.
> >>> e.g. 'march_feature' : ['crypto', 'sve'] 3. Select the most suitable
> >>> march+feature combination based on 'march_base' and 'march_feature'
> >>> tuples.
> >>> 4. Use the selected march+feature combination as the default
> >>> machine_args.
> >>>
> >>> Fixes: 7cf32a22b240 ("config/arm: add Hisilicon kunpeng")
> >>>
> >>> Signed-off-by: Chengwen Feng 
> >>> ---
> >>>  config/arm/meson.build | 27 +--
> >>>  1 file changed, 25 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/config/arm/meson.build b/config/arm/meson.build index
> >>> 3f34ec9..8551a80 100644
> >>> --- a/config/arm/meson.build
> >>> +++ b/config/arm/meson.build
> >>> @@ -149,7 +149,9 @@ implementer_hisilicon = {
> >>>  ],
> >>>  'part_number_config': {
> >>>  '0xd01': {
> >>> -'machine_args': ['-march=armv8.2-a+crypto', '-mtune=tsv110'],
> >>> +'march_base' : ['-march=armv8-a', '-march=armv8.2-a'],
> > If the compiler supports armv8.1-a, you need to choose armv8.1-a.
> >
> >>
> >> Another target has the same issue. Could you fix that all together as
> >> it is generic problem in the existing infrastructure.
> > I think this needs to be more generic solution. IMO, the requirement is as
> follows:
> >
> > 1) We need to pick the most closest march_base supported by the
> > compiler. For ex: If the SoC support v8.2 and the compiler supports
> > v8.1, we need to pick v8.1
> > 2) SoCs are allowed to support a base march version + hand picked features
> from the next 1 base marchs. i.e. armv8.x compliant implementation can
> include any features from armv8.(x + 1). Please refer to  Arm-ARM section A2
> for more details. So, it is possible that the compiler supports a base march
> and a bunch of optional features from the next version. We need to test all
> the features allowed by the architecture and pick the ones that are supported
> in the compiler.
> >
> 
> I try to add 'march_base' : ['-march=armv8-a', '-march=armv8.5-a'] to cn10k,
> and then find it can't work with ['RTE_ARM_FEATURE_ATOMICS', true] when
> using gcc7.2:
>   [268/2250] Compiling C object
> lib/librte_stack.a.p/stack_rte_stack_lf.c.o
>   FAILED: lib/librte_stack.a.p/stack_rte_stack_lf.c.o
>   ...
>   {standard input}: Assembler messages:
>   {standard input}:13: Error: selected processor does not support `caspl
> x0,x1,x2,x3,[x5]'
>   [347/2250] Compiling C object
> lib/librte_hash.a.p/hash_rte_cuckoo_hash.c.o
>   ninja: build stopped: subcommand failed.
>   make: *** [cn10k] Error 1
> It seem can't simplely add '-march=armv8-a' in 'march_base'.
> And it require a lot of testing to get the right 'march_base' and
> 'march_feature' parameters.
> So for v5, I just modify the kunpeng930's config which was well tested.
> 
> I think the 'march_base' and 'march_feature' could well solve the gcc's minor-
> version problem.
> Note: the minor-version means a few version which are closes, not big gap,
> like gcc5.4 and gcc10.2
> 
> For that old gcc which could not support the 'march' that defined in
> 'machine_args' or 'march_base'
> I think it better use the 'generic' profile else it will compile fail which 
> showed
> above.
> 
> So how about add new tuple: fallback_generic? eg:
>   '0xd02': {
> 'march_base': ['-march=armv8.2-a'],
> 'march_feature': ['crypto', 'sve'],
> 'machine_args': [],
> 'flags': [
> ['RTE_MACHINE', '"Kunpeng 930"'],
> ['RTE_ARM_FEATURE_ATOMICS', true],
> ['RTE_MAX_LCORE', 1280],
> ['RTE_MAX_NUMA_NODES', 16]
> ],
>   'fallback_generic': true
> }
> PS: The premise is that the 'generic' profile is tested.
Jerin, how big of a problem is this (i.e. having to compile the code with an 
older version of the compiler)? Is it just one old version of the compiler or 
there are several of them? Do they all need to be captured in the meson.build 
file? I am just trying to understand the need for a generic approach.

> 
> 
> >>
> >>
> >>> +'march_feature' : ['crypto'],
> >>> +'machine_args': ['-mtune=tsv110'],
> >>>  'flags': [
> >>>  ['RTE_MACHINE', '"Kunpeng 920"'],
> >>>  ['RTE_ARM_FEATURE_ATOMICS', true], @@ -158,7 +160,9
> >>> @@ implementer_hisilicon = {
> >>>  ]
> >>>  },
> >>>  '0xd02': {
> >>> -

[dpdk-dev] [PATCH v6 0/3] rte_flow doc matrix

[Thomas Monjalon]

This is an improvement of the rte_flow documentation:
it makes possible to quickly read which items and actions
may be accepted by the drivers in a DPDK release.

A script is provided for CI to make sure the matrix is updated,
while allowing some manual tuning.

Thomas Monjalon (3):
  doc: rename sfc features file
  doc: add flow API features tables
  devtools: check flow API doc tables

 .gitignore|   2 +
 devtools/check-doc-vs-code.sh |  84 +
 devtools/parse-flow-support.sh|  78 
 doc/guides/conf.py|  18 +--
 doc/guides/nics/features.rst  |  11 --
 doc/guides/nics/features/bnxt.ini |  26 +++-
 doc/guides/nics/features/cxgbe.ini|  31 -
 doc/guides/nics/features/default.ini  | 118 +-
 doc/guides/nics/features/dpaa2.ini|  19 ++-
 doc/guides/nics/features/e1000.ini|  14 +++
 doc/guides/nics/features/enic.ini |  29 -
 doc/guides/nics/features/failsafe.ini |   1 -
 doc/guides/nics/features/hinic.ini|  16 ++-
 doc/guides/nics/features/hns3.ini |  23 +++-
 doc/guides/nics/features/hns3_vf.ini  |   1 -
 doc/guides/nics/features/i40e.ini |  31 -
 doc/guides/nics/features/iavf.ini |  30 -
 doc/guides/nics/features/ice.ini  |  34 -
 doc/guides/nics/features/ice_dcf.ini  |   1 -
 doc/guides/nics/features/igb.ini  |   1 -
 doc/guides/nics/features/igc.ini  |  12 +-
 doc/guides/nics/features/ipn3ke.ini   |  15 ++-
 doc/guides/nics/features/ixgbe.ini|  24 +++-
 doc/guides/nics/features/mlx4.ini |  13 +-
 doc/guides/nics/features/mlx5.ini |  73 ++-
 doc/guides/nics/features/mvpp2.ini|  14 +++
 doc/guides/nics/features/octeontx2.ini|  43 ++-
 doc/guides/nics/features/octeontx2_vec.ini|   1 -
 doc/guides/nics/features/octeontx2_vf.ini |   1 -
 doc/guides/nics/features/qede.ini |  11 +-
 .../nics/features/{sfc_efx.ini => sfc.ini}|  35 +-
 doc/guides/nics/features/tap.ini  |  15 ++-
 doc/guides/nics/features/txgbe.ini|  24 +++-
 doc/guides/nics/overview.rst  |   8 ++
 34 files changed, 813 insertions(+), 44 deletions(-)
 create mode 100755 devtools/check-doc-vs-code.sh
 create mode 100755 devtools/parse-flow-support.sh
 rename doc/guides/nics/features/{sfc_efx.ini => sfc.ini} (57%)

-- 
2.31.1

[dpdk-dev] [PATCH v6 1/3] doc: rename sfc features file

[Thomas Monjalon]

The driver directory is drivers/net/sfc
but the features file was doc/guides/nics/features/sfc_efx.ini.

sfc_efx.ini is renamed sfc.ini to match the driver directory name.
It will help automatic checks of this file.

Signed-off-by: Thomas Monjalon 
---
 doc/guides/nics/features/{sfc_efx.ini => sfc.ini} | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename doc/guides/nics/features/{sfc_efx.ini => sfc.ini} (100%)

diff --git a/doc/guides/nics/features/sfc_efx.ini 
b/doc/guides/nics/features/sfc.ini
similarity index 100%
rename from doc/guides/nics/features/sfc_efx.ini
rename to doc/guides/nics/features/sfc.ini
-- 
2.31.1

[dpdk-dev] [PATCH v6 2/3] doc: add flow API features tables

[Thomas Monjalon]

The NICs overview table lists all supported features per driver.
There was a single row for "Flow API",
although rte_flow is composed of many items and actions.

The row "Flow API" is replaced with two new tables for items and actions.

Also, since rte_flow is not implemented in all drivers,
it would be ugly to add empty sections in some files.
That's why the error message for missing INI section is removed.

The lists are sorted alphabetically.
The extra files for some VF and vectorized data paths are not filled.

Signed-off-by: Asaf Penso 
Signed-off-by: Thomas Monjalon 
Acked-by: Kiran Kumar Kokkilagadda 
---
v6 changes:
- rebase/update
- remove deprecated shared action
---
 .gitignore |   2 +
 doc/guides/conf.py |  18 ++--
 doc/guides/nics/features.rst   |  11 --
 doc/guides/nics/features/bnxt.ini  |  26 -
 doc/guides/nics/features/cxgbe.ini |  31 +-
 doc/guides/nics/features/default.ini   | 118 -
 doc/guides/nics/features/dpaa2.ini |  19 +++-
 doc/guides/nics/features/e1000.ini |  14 +++
 doc/guides/nics/features/enic.ini  |  29 -
 doc/guides/nics/features/failsafe.ini  |   1 -
 doc/guides/nics/features/hinic.ini |  16 ++-
 doc/guides/nics/features/hns3.ini  |  23 +++-
 doc/guides/nics/features/hns3_vf.ini   |   1 -
 doc/guides/nics/features/i40e.ini  |  31 +-
 doc/guides/nics/features/iavf.ini  |  30 +-
 doc/guides/nics/features/ice.ini   |  34 +-
 doc/guides/nics/features/ice_dcf.ini   |   1 -
 doc/guides/nics/features/igb.ini   |   1 -
 doc/guides/nics/features/igc.ini   |  12 ++-
 doc/guides/nics/features/ipn3ke.ini|  15 ++-
 doc/guides/nics/features/ixgbe.ini |  24 -
 doc/guides/nics/features/mlx4.ini  |  13 ++-
 doc/guides/nics/features/mlx5.ini  |  73 -
 doc/guides/nics/features/mvpp2.ini |  14 +++
 doc/guides/nics/features/octeontx2.ini |  43 +++-
 doc/guides/nics/features/octeontx2_vec.ini |   1 -
 doc/guides/nics/features/octeontx2_vf.ini  |   1 -
 doc/guides/nics/features/qede.ini  |  11 +-
 doc/guides/nics/features/sfc.ini   |  35 +-
 doc/guides/nics/features/tap.ini   |  15 ++-
 doc/guides/nics/features/txgbe.ini |  24 -
 doc/guides/nics/overview.rst   |   8 ++
 32 files changed, 651 insertions(+), 44 deletions(-)

diff --git a/.gitignore b/.gitignore
index f73d93ca53..b19c0717e6 100644
--- a/.gitignore
+++ b/.gitignore
@@ -3,6 +3,8 @@
 
 # ignore generated documentation tables
 doc/guides/nics/overview_table.txt
+doc/guides/nics/rte_flow_actions_table.txt
+doc/guides/nics/rte_flow_items_table.txt
 doc/guides/cryptodevs/overview_feature_table.txt
 doc/guides/cryptodevs/overview_cipher_table.txt
 doc/guides/cryptodevs/overview_auth_table.txt
diff --git a/doc/guides/conf.py b/doc/guides/conf.py
index 894d81ca75..67d2dd62c7 100644
--- a/doc/guides/conf.py
+++ b/doc/guides/conf.py
@@ -176,14 +176,8 @@ def generate_overview_table(output_filename, table_id, 
section, table_name, titl
 # Initialize the dict with the default.ini value.
 ini_data[ini_filename] = valid_features.copy()
 
-# Check for a valid ini section.
+# Check for a section.
 if not config.has_section(section):
-print("{}: File '{}' has no [{}] secton".format(warning,
-ini_filename,
-section),
-file=stderr)
-if stop_on_error:
-raise Exception('Warning is treated as a failure')
 continue
 
 # Check for valid features names.
@@ -339,6 +333,16 @@ def setup(app):
 'Features',
 'Features availability in networking drivers',
 'Feature')
+table_file = dirname(__file__) + '/nics/rte_flow_items_table.txt'
+generate_overview_table(table_file, 2,
+'rte_flow items',
+'rte_flow items availability in networking 
drivers',
+'Item')
+table_file = dirname(__file__) + '/nics/rte_flow_actions_table.txt'
+generate_overview_table(table_file, 3,
+'rte_flow actions',
+'rte_flow actions availability in networking 
drivers',
+'Action')
 table_file = dirname(__file__) + '/cryptodevs/overview_feature_table.txt'
 generate_overview_table(table_file, 1,
 'Features',
diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index f6d30d0af3..403c2b03a3 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features

[dpdk-dev] [PATCH v6 3/3] devtools: check flow API doc tables

[Thomas Monjalon]

The script check-doc-vs-code.sh may be used to add
some automatic checks of the doc.

If run without any argument, a complete check is done.
The optional argument is a git history reference point
to check faster only what has changed since this commit.

In this commit, the only check is for rte_flow tables,
achieved through the script parse-flow-support.sh.
If run without a .ini reference, it prints rte_flow tables.
Note: detected features are marked with the value Y,
while the real .ini file could have special values like I.
The script allow parsing exceptions (exclude or include),
like for bnxt code which lists unsupported items and actions.

Signed-off-by: Thomas Monjalon 
---
v6 changes:
- fix redundant drivers
- ignore indirect action
- prefix misses with a category (item or action)
---
 devtools/check-doc-vs-code.sh  | 84 ++
 devtools/parse-flow-support.sh | 78 +++
 2 files changed, 162 insertions(+)
 create mode 100755 devtools/check-doc-vs-code.sh
 create mode 100755 devtools/parse-flow-support.sh

diff --git a/devtools/check-doc-vs-code.sh b/devtools/check-doc-vs-code.sh
new file mode 100755
index 00..c58c239c87
--- /dev/null
+++ b/devtools/check-doc-vs-code.sh
@@ -0,0 +1,84 @@
+#! /bin/sh -e
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright 2021 Mellanox Technologies, Ltd
+
+# Check whether doc & code are in sync.
+# Optional argument: check only what changed since a commit.
+trusted_commit=$1 # example: origin/main
+
+selfdir=$(dirname $(readlink -f $0))
+rootdir=$(readlink -f $selfdir/..)
+
+# speed up by ignoring Unicode details
+export LC_COLLATE=C
+
+result=0
+error() # 
+{
+   echo "$*"
+   result=$(($result + 1))
+}
+
+changed_files()
+{
+   [ -n "$files" ] ||
+   files=$(git diff-tree --name-only -r $trusted_commit..)
+   echo "$files"
+}
+
+has_code_change() # 
+{
+   test -n "$(git log --format='%h' -S"$1" $trusted_commit..)"
+}
+
+has_file_change() # 
+{
+   changed_files | grep -q "$1"
+}
+
+changed_net_drivers()
+{
+   net_paths='drivers/net/|doc/guides/nics/features/'
+   [ -n "$drivers" ] ||
+   drivers=$(changed_files |
+   sed -rn "s,^($net_paths)([^./]*).*,\2,p" |
+   sort -u)
+   echo "$drivers"
+}
+
+all_net_drivers()
+{
+   find $rootdir/drivers/net -mindepth 1 -maxdepth 1 -type d |
+   sed 's,.*/,,' |
+   sort
+}
+
+check_rte_flow() # 
+{
+   code=$rootdir/drivers/net/$1
+   doc=$rootdir/doc/guides/nics/features/$1.ini
+   [ -d $code ] || return 0
+   [ -f $doc ] || return 0
+   report=$($selfdir/parse-flow-support.sh $code $doc)
+   if [ -n "$report" ]; then
+   error "rte_flow doc out of sync for $1"
+   echo "$report" | sed 's,^,\t,'
+   fi
+}
+
+if [ -z "$trusted_commit" ]; then
+   # check all
+   for driver in $(all_net_drivers); do
+   check_rte_flow $driver
+   done
+   exit $result
+fi
+
+# find what changed and check
+if has_code_change 'RTE_FLOW_.*_TYPE_' ||
+   has_file_change 'doc/guides/nics/features'; then
+   for driver in $(changed_net_drivers); do
+   check_rte_flow $driver
+   done
+fi
+exit $result
diff --git a/devtools/parse-flow-support.sh b/devtools/parse-flow-support.sh
new file mode 100755
index 00..aeed76e55f
--- /dev/null
+++ b/devtools/parse-flow-support.sh
@@ -0,0 +1,78 @@
+#! /bin/sh -e
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright 2021 Mellanox Technologies, Ltd
+
+# Parse rte_flow support of a driver directory,
+# and optionally show difference with a doc file in .ini format.
+
+dir=$1 # drivers/net/foo
+ref=$2 # doc/guides/nics/features/foo.ini
+
+if [ -z "$dir" ]; then
+   echo "directory argument is required" >&2
+   exit 1
+fi
+
+# sorting order
+export LC_COLLATE=C
+
+# exclude exceptions
+exclude() # 
+{
+   case $(basename $dir) in
+   bnxt)
+   filter=$(sed -n "/$1/{N;/TYPE_NOT_SUPPORTED/P;}" \
+   $dir/tf_ulp/ulp_template_db{,_tbl}.c |
+   grep -wo "$1[[:alnum:]_]*" | sort -u |
+   tr '\n' '|' | sed 's,.$,\n,')
+   grep -vE "$filter";;
+   *) cat
+   esac
+}
+
+# include exceptions
+include() # 
+{
+   case $(basename $dir) in
+   esac
+}
+
+# generate INI section
+list() #  
+{
+   echo "[$1]"
+   git grep -who "$2[[:alnum:]_]*" $dir |
+   (exclude $2; include $2) | sort -u |
+   awk 'sub(/'$2'/, "") {printf "%-20s = Y\n", tolower($0)}'
+}
+
+rte_flow_support() # 
+{
+   title="rte_flow $1s"
+   pattern=$(echo "RTE_FLOW_$1_TYPE_" | awk '{print toupper($0)}')
+   list "$title" "$pattern" | grep -vwE 'void|indirect|end'
+}
+
+if [ -z "$ref" ]; then # generate full tables
+   rte_flow_support item
+   e

Re: [dpdk-dev] [PATCH] doc: add new feature for connection tracking

[Bing Zhao]

Hi Ferruh,

> -Original Message-
> From: Ferruh Yigit 
> Sent: Tuesday, May 18, 2021 9:16 PM
> To: Bing Zhao ; Ori Kam ; Slava
> Ovsiienko ; Matan Azrad ;
> NBU-Contact-Thomas Monjalon 
> Cc: dev@dpdk.org; Raslan Darawsheh ;
> andrew.rybche...@oktetlabs.ru
> Subject: Re: [PATCH] doc: add new feature for connection tracking
> 
> External email: Use caution opening links or attachments
> 
> 
> On 5/18/2021 1:52 PM, Bing Zhao wrote:
> > The feature support of connection tracking is added in the feature
> > table of networking drivers. The feature overview part is also
> > updated.
> >
> > Signed-off-by: Bing Zhao 
> 
> Hi Bing,
> 
> Thomas is suggestion another table to list flow API item/actions in
> separate table, can this change can be replaced by it?

It is OK for me. But I might need the comments from Thomas, Ori and ETHDEV 
maintainers since you all are much more familiar with the documents and their 
scopes.

@Thomas, is it OK?

Thanks

Re: [dpdk-dev] [PATCH] doc: add new feature for connection tracking

[Thomas Monjalon]

18/05/2021 15:33, Bing Zhao:
> From: Ferruh Yigit 
> > On 5/18/2021 1:52 PM, Bing Zhao wrote:
> > > The feature support of connection tracking is added in the feature
> > > table of networking drivers. The feature overview part is also
> > > updated.
> > >
> > > Signed-off-by: Bing Zhao 
> > 
> > Hi Bing,
> > 
> > Thomas is suggestion another table to list flow API item/actions in
> > separate table, can this change can be replaced by it?
> 
> It is OK for me. But I might need the comments from Thomas, Ori and ETHDEV 
> maintainers since you all are much more familiar with the documents and their 
> scopes.
> 
> @Thomas, is it OK?

Yes, I mark this patch as rejected.

See how it is added in the new table:
https://patches.dpdk.org/project/dpdk/patch/20210518132844.3779728-3-tho...@monjalon.net/

Re: [dpdk-dev] [PATCH v4 1/2] config/arm: select most suitable -march for kunpeng soc

[Jerin Jacob]

On Tue, May 18, 2021 at 6:55 PM Honnappa Nagarahalli
 wrote:
>
> 
>
> > >>
> > >> On Thu, May 13, 2021 at 6:41 PM Chengwen Feng
> > >>  wrote:
> > >>>
> > >>> Currently, the soc_kunpeng930 declares
> > >>> '-march=armv8.2-a+crypto+sve', but some compiler doesn't recognize
> > >>> the march because it doesn't support sve.
> > >>>
> > >>> To solve this bug we use the following scheme:
> > >>> 1. Define 'march_base' tuple which defines support march, it should
> > >>> arrange from lower to higher.
> > >>> e.g. 'march_base' : ['-march=armv8-a', '-march=armv8.2-a'] 2. Define
> > >>> 'march_feature' tuple which defines support feature.
> > >>> e.g. 'march_feature' : ['crypto', 'sve'] 3. Select the most suitable
> > >>> march+feature combination based on 'march_base' and 'march_feature'
> > >>> tuples.
> > >>> 4. Use the selected march+feature combination as the default
> > >>> machine_args.
> > >>>
> > >>> Fixes: 7cf32a22b240 ("config/arm: add Hisilicon kunpeng")
> > >>>
> > >>> Signed-off-by: Chengwen Feng 
> > >>> ---
> > >>>  config/arm/meson.build | 27 +--
> > >>>  1 file changed, 25 insertions(+), 2 deletions(-)
> > >>>
> > >>> diff --git a/config/arm/meson.build b/config/arm/meson.build index
> > >>> 3f34ec9..8551a80 100644
> > >>> --- a/config/arm/meson.build
> > >>> +++ b/config/arm/meson.build
> > >>> @@ -149,7 +149,9 @@ implementer_hisilicon = {
> > >>>  ],
> > >>>  'part_number_config': {
> > >>>  '0xd01': {
> > >>> -'machine_args': ['-march=armv8.2-a+crypto', 
> > >>> '-mtune=tsv110'],
> > >>> +'march_base' : ['-march=armv8-a', '-march=armv8.2-a'],
> > > If the compiler supports armv8.1-a, you need to choose armv8.1-a.
> > >
> > >>
> > >> Another target has the same issue. Could you fix that all together as
> > >> it is generic problem in the existing infrastructure.
> > > I think this needs to be more generic solution. IMO, the requirement is as
> > follows:
> > >
> > > 1) We need to pick the most closest march_base supported by the
> > > compiler. For ex: If the SoC support v8.2 and the compiler supports
> > > v8.1, we need to pick v8.1
> > > 2) SoCs are allowed to support a base march version + hand picked features
> > from the next 1 base marchs. i.e. armv8.x compliant implementation can
> > include any features from armv8.(x + 1). Please refer to  Arm-ARM section A2
> > for more details. So, it is possible that the compiler supports a base march
> > and a bunch of optional features from the next version. We need to test all
> > the features allowed by the architecture and pick the ones that are 
> > supported
> > in the compiler.
> > >
> >
> > I try to add 'march_base' : ['-march=armv8-a', '-march=armv8.5-a'] to cn10k,
> > and then find it can't work with ['RTE_ARM_FEATURE_ATOMICS', true] when
> > using gcc7.2:
> >   [268/2250] Compiling C object
> > lib/librte_stack.a.p/stack_rte_stack_lf.c.o
> >   FAILED: lib/librte_stack.a.p/stack_rte_stack_lf.c.o
> >   ...
> >   {standard input}: Assembler messages:
> >   {standard input}:13: Error: selected processor does not support `caspl
> > x0,x1,x2,x3,[x5]'
> >   [347/2250] Compiling C object
> > lib/librte_hash.a.p/hash_rte_cuckoo_hash.c.o
> >   ninja: build stopped: subcommand failed.
> >   make: *** [cn10k] Error 1
> > It seem can't simplely add '-march=armv8-a' in 'march_base'.
> > And it require a lot of testing to get the right 'march_base' and
> > 'march_feature' parameters.
> > So for v5, I just modify the kunpeng930's config which was well tested.
> >
> > I think the 'march_base' and 'march_feature' could well solve the gcc's 
> > minor-
> > version problem.
> > Note: the minor-version means a few version which are closes, not big gap,
> > like gcc5.4 and gcc10.2
> >
> > For that old gcc which could not support the 'march' that defined in
> > 'machine_args' or 'march_base'
> > I think it better use the 'generic' profile else it will compile fail which 
> > showed
> > above.
> >
> > So how about add new tuple: fallback_generic? eg:
> >   '0xd02': {
> > 'march_base': ['-march=armv8.2-a'],
> > 'march_feature': ['crypto', 'sve'],
> > 'machine_args': [],
> > 'flags': [
> > ['RTE_MACHINE', '"Kunpeng 930"'],
> > ['RTE_ARM_FEATURE_ATOMICS', true],
> > ['RTE_MAX_LCORE', 1280],
> > ['RTE_MAX_NUMA_NODES', 16]
> > ],
> >   'fallback_generic': true
> > }
> > PS: The premise is that the 'generic' profile is tested.
> Jerin, how big of a problem is this (i.e. having to compile the code with an 
> older version of the compiler)? Is it just one old version of the compiler or 
> there are several of them? Do they all need to be captured in the meson.build 
> file? I am just trying to understand the need for a generic approach.

IMO, We are supporting from gcc 4.7. So a lot of combinations are possible.


>
> >
> >
> > >>
> > 

Re: [dpdk-dev] [PATCH] doc: add new feature for connection tracking

[Bing Zhao]

Thanks a lot to you all.

> -Original Message-
> From: Thomas Monjalon 
> Sent: Tuesday, May 18, 2021 9:19 PM
> To: Bing Zhao ; Ferruh Yigit
> 
> Cc: Ori Kam ; Slava Ovsiienko
> ; Matan Azrad ;
> dev@dpdk.org; Raslan Darawsheh ;
> andrew.rybche...@oktetlabs.ru
> Subject: Re: [PATCH] doc: add new feature for connection tracking
> 
> External email: Use caution opening links or attachments
> 
> 
> 18/05/2021 15:16, Ferruh Yigit:
> > On 5/18/2021 1:52 PM, Bing Zhao wrote:
> > > The feature support of connection tracking is added in the
> feature
> > > table of networking drivers. The feature overview part is also
> > > updated.
> > >
> > > Signed-off-by: Bing Zhao 
> >
> > Hi Bing,
> >
> > Thomas is suggestion another table to list flow API item/actions
> in
> > separate table, can this change can be replaced by it?
> 
> Correct, I am adding conntrack as part of rte_flow features table.
> Please don't add it in the global table.
> 

Re: [dpdk-dev] 回复: [PATCH] app/eventdev: remove unnecessary barrier for order test

[Jerin Jacob]

On Tue, May 18, 2021 at 2:57 PM Feifei Wang  wrote:
>
> Hi, Jerin
>
> Sorry to disturb you. Would you please help me review this patch, thanks very 
> much.

In slowpath, I thought of having this barrier. But I agree that it can
be removed.
Since this is not bug. We will merge this patch in next release.



>
>
> Best Regards
> Feifei
>
> > -Original Message-
> > From: Feifei Wang 
> > Sent: Monday, May 10, 2021 2:12 PM
> > To: jer...@marvell.com
> > Cc: dev@dpdk.org; nd ; Feifei Wang
> > ; Ruifeng Wang ;
> > Honnappa Nagarahalli 
> > Subject: [PATCH] app/eventdev: remove unnecessary barrier for order test
> >
> > For "order_launch_lcores" function, wmb after that the main lcore updates
> > the variable "t->err", which represents the end of the test signal, is
> > unnecessary. Because after the main lcore updates this siginal variable, it 
> > will
> > jump out of the launch function loop, and wait other lcores stop or return
> > error in the main function(evt_main.c).
> > During this time, there is no storing operation and thus no need for wmb.
> >
> > Signed-off-by: Feifei Wang 
> > Reviewed-by: Ruifeng Wang 
> > Reviewed-by: Honnappa Nagarahalli 
> > ---
> >  app/test-eventdev/test_order_common.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/app/test-eventdev/test_order_common.c b/app/test-
> > eventdev/test_order_common.c
> > index 04456d56db..d7760061ba 100644
> > --- a/app/test-eventdev/test_order_common.c
> > +++ b/app/test-eventdev/test_order_common.c
> > @@ -308,7 +308,6 @@ order_launch_lcores(struct evt_test *test, struct
> > evt_options *opt,
> >   rte_event_dev_dump(opt->dev_id, stdout);
> >   evt_err("No schedules for seconds,
> > deadlock");
> >   t->err = true;
> > - rte_smp_wmb();
> >   break;
> >   }
> >   old_remaining = remaining;
> > --
> > 2.25.1
> >
>

[dpdk-dev] [PATCH] test/crypto: fix test status when PMD isn't loaded

[Ciara Power]

The return value for a test when the required PMD is not loaded should
be TEST_SKIPPED, rather than TEST_FAILED.

Fixes: 8bfdd8a7f0f1 ("test/crypto: refactor to use sub test suites")

Signed-off-by: Ciara Power 
---
 app/test/test_cryptodev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 736952a9e6..c68684b80b 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -14504,7 +14504,7 @@ run_cryptodev_testsuite(const char *pmd_name)
 
if (gbl_driver_id == -1) {
RTE_LOG(ERR, USER1, "%s PMD must be loaded.\n", pmd_name);
-   return TEST_FAILED;
+   return TEST_SKIPPED;
}
 
ts.unit_test_suites = malloc(sizeof(struct unit_test_suite *) *
-- 
2.25.1

Re: [dpdk-dev] [PATCH] doc/guides: add details for new test structure

[Power, Ciara]

Hi Aaron,


>-Original Message-
>From: Aaron Conole 
>Sent: Monday 17 May 2021 19:02
>To: Power, Ciara 
>Cc: dev@dpdk.org; Zhang, Roy Fan ; Doherty,
>Declan 
>Subject: Re: [PATCH] doc/guides: add details for new test structure
>
>Ciara Power  writes:
>
>> The testing guide is now updated to include details about using
>> sub-testsuites. Some example code is given to demonstrate how they can
>> be used.
>>
>> A note is also added to highlight the need for using vdev EAL args
>> when running cryptodev tests.
>>
>> Depends-on: patch-88751 ("guides: add a guide for developing unit
>> tests")
>>
>> Signed-off-by: Ciara Power 
>> ---
>>  doc/guides/contributing/testing.rst | 89
>> -
>>  1 file changed, 87 insertions(+), 2 deletions(-)
>>
>> diff --git a/doc/guides/contributing/testing.rst
>> b/doc/guides/contributing/testing.rst
>> index 0757d71ad0..1188d63a97 100644
>> --- a/doc/guides/contributing/testing.rst
>> +++ b/doc/guides/contributing/testing.rst
>> @@ -114,13 +114,21 @@ for interacting with the test harness:
>>
>>2. unit_test_suite_runner(struct unit_test_suite \*)
>>   Returns a runner for a full test suite object, which contains
>> - a test suite name, setup, tear down, and vector of unit test
>> - cases.
>> + a test suite name, setup, tear down, a pointer to a list of
>> + sub-testsuites, and vector of unit test cases.
>>
>>  Each test suite has a setup and tear down function that runs at the
>> beginning and end of the test suite execution.  Each unit test has  a
>> similar function for test case setup and tear down.
>>
>> +Each testsuite may use a nested list of sub-testsuites, which are
>> +iterated by the unit_test_suite_runner.
>> +This support allows for better granularity when designing test suites.
>> +The sub-testsuites list can also be used in parallel with the vector
>> +of testcases, in this case the testcases will be run, and then each
>> +sub-testsuite is executed. To see an example of a testsuite using
>> +sub-testsuites, see *app/test/test_cryptodev.c*.
>> +
>>  Test cases are added to the `.unit_test_cases` element of the unit
>> test suite structure.  Ex:
>>
>> @@ -165,6 +173,70 @@ test suite structure.  Ex:
>>  The above code block is a small example that can be used to create a
>> complete test suite with test case.
>>
>> +Sub-testsuites can be added to the `.unit_test_suites` element of the
>> +unit test suite structure.  Ex:
>> +
>> +.. code-block:: c
>> +   :linenos:
>> +
>> +   static int testsuite_setup(void) { return TEST_SUCCESS; }
>> +   static void testsuite_teardown(void) { }
>> +
>> +   static int ut_setup(void) { return TEST_SUCCESS; }
>> +   static void ut_teardown(void) { }
>> +
>> +   static int test_case_first(void) { return TEST_SUCCESS; }
>> +
>> +   static struct unit_test_suite example_parent_testsuite = {
>> +  .suite_name = "EXAMPLE PARENT TEST SUITE",
>> +  .setup = testsuite_setup,
>> +  .teardown = testsuite_teardown,
>> +  .unit_test_cases = {TEST_CASES_END()}
>> +   };
>> +
>> +   static int sub_testsuite_setup(void) { return TEST_SUCCESS; }
>> +   static void sub_testsuite_teardown(void) { }
>> +
>> +   static struct unit_test_suite example_sub_testsuite = {
>> +  .suite_name = "EXAMPLE SUB TEST SUITE",
>> +  .setup = sub_testsuite_setup,
>> +  .teardown = sub_testsuite_teardown,
>> +  .unit_test_cases = {
>> +   TEST_CASE_ST(ut_setup, ut_teardown, test_case_first),
>> +
>> +   TEST_CASES_END(), /**< NULL terminate unit test array */
>> +  },
>> +   };
>> +
>> +   static struct unit_test_suite end_testsuite = {
>> +  .suite_name = NULL,
>> +  .setup = NULL,
>> +  .teardown = NULL,
>> +  .unit_test_suites = NULL
>> +   };
>> +
>> +   static int example_tests()
>> +   {
>> +   uint8_t ret, i = 0;
>> +   struct unit_test_suite *sub_suites[] = {
>> +  &example_sub_testsuite,
>> +  &end_testsuite /**< NULL test suite to indicate end of list */
>> +};
>> +
>> +   example_parent_testsuite.unit_test_suites =
>> +   malloc(sizeof(struct unit_test_suite *) *
>> + RTE_DIM(sub_suites));
>> +
>> +   for (i = 0; i < RTE_DIM(sub_suites); i++)
>> +   example_parent_testsuite.unit_test_suites[i] =
>> + sub_suites[i];
>> +
>> +   ret = unit_test_suite_runner(&example_parent_testsuite);
>> +   free(example_parent_testsuite.unit_test_suites);
>> +
>> +   return ret;
>> +   }
>> +
>> +   REGISTER_TEST_COMMAND(example_autotest, example_tests);
>> +
>>
>>  Designing a test
>>  
>> @@ -241,3 +313,16 @@ be run as part of the appropriate class (fast, perf,
>driver, etc.).
>>  Some of these default test suites are run during continuous
>> integration  tests, making regression checking automatic for new
>> patches submitted  to the project.
>> +
>> +
>> +Running Cryptodev Tests
>> +---

[dpdk-dev] Meson configuration for pkg-config for drivers' shared objects

[Jakub Palider]

Hi,

I experience a problem while building out-of-tree application against drivers' 
shared libraries from dpdk that are _not_ accessed by means of lib/ interface. 
This looks a bit like a lack of a link between the framework and what certain 
drivers deliver (rte_pmd API). Employing pkg-config in current shape does not 
satisfy the dependencies.

Please, have a look at the example below which materializes this problem of an 
app.c:

#include 
#include 

int main()
{
rte_ether_format_addr(NULL, 0, NULL);
rte_pmd_i40e_set_vf_mac_addr(0, 0, NULL);
}

I want to use two symbols, one from librte_net and one from i40e net rte pmd.

1. PLAIN compilation: complains about both of them, as expected:

$ ${CROSS}-g++ -march=armv8.1-a -o bin-app app.c
/tmp/ccUac7fS.o: In function `main':
app.c:(.text+0x114): undefined reference to `rte_ether_format_addr'
app.c:(.text+0x124): undefined reference to `rte_pmd_i40e_set_vf_mac_addr'
collect2: error: ld returned 1 exit status

2. PKGCONFIG asked to fill dependencies adds them for dpdk lib/ but i40e rte 
pmd is still missing:

${CROSS}-g++ -march=armv8.1-a -o bin-app app.c $(pkg-config --libs libdpdk)
/tmp/ccYhiA3K.o: In function `main':
app.c:(.text+0x124): undefined reference to `rte_pmd_i40e_set_vf_mac_addr'
collect2: error: ld returned 1 exit status

3. WORKADOUND: which adds explicitly dependency for linker:

${CROSS}-g++ -march=armv8.1-a -o bin-app app.c $(pkg-config --libs libdpdk) 
-lrte_net_i40e

The borderline is that I don't want to add them explicitly, but rather 
pkg-config do this for me. This would be realized if driver's lib (which 
actually gets built as .so) was added to meson's dpdk_libraries variable:
my_lib_name = '_'.join(['rte', class, name])
shared_lib = shared_library(my_lib_name)
dpdk_libraries = [shared_lib] + dpdk_libraries
In such case $(pkg-config --libs libdpdk) would satisfy all dependencies.

My questions are:
1. Is there a better approach to this problem than one mentioned by me? Am I 
missing something?
2. Shall RTE PMD APIs of certain drivers be a part of dpdk default pkg-config 
configuration?

I have learned there used to be libdpdk.so but this is no longer applicable, 
also this is not a plugin-alike issue. So I hope I am not duplicating topics - 
I could not find anything resembling this one.

Regards,
Jakub

Re: [dpdk-dev] [PATCH] test/crypto: fix test status when PMD isn't loaded

[Zhang, Roy Fan]

> -Original Message-
> From: Power, Ciara 
> Sent: Tuesday, May 18, 2021 3:20 PM
> To: dev@dpdk.org
> Cc: Zhang, Roy Fan ; Doherty, Declan
> ; acon...@redhat.com; gak...@marvell.com;
> Power, Ciara 
> Subject: [PATCH] test/crypto: fix test status when PMD isn't loaded
> 
> The return value for a test when the required PMD is not loaded should
> be TEST_SKIPPED, rather than TEST_FAILED.
> 
> Fixes: 8bfdd8a7f0f1 ("test/crypto: refactor to use sub test suites")
> 
> Signed-off-by: Ciara Power 
> ---
> 2.25.1

Acked-by: Fan Zhang 

Re: [dpdk-dev] [PATCH v5 2/2] net/hns3: refactor SVE code compile method

[Ferruh Yigit]

On 5/14/2021 10:53 AM, Chengwen Feng wrote:
> Currently, the SVE code is compiled only when -march supports SVE
> (e.g. '-march=armv8.2a+sve'), there maybe some problem[1] with this
> approach.
> 
> The solution:
> a. If the minimum instruction set support SVE then compiles it.
> b. Else if the compiler support SVE then compiles it.
> c. Otherwise don't compile it.
> 
> [1] https://mails.dpdk.org/archives/dev/2021-April/208189.html
> 

Hi Chengwen,

As far as I understand from above problem statement, you want to produce a
binary that can run in two different platforms, one supports only NEON
instructions, other supports NEON + SVE.

For this driver should be compiled in a way to support min instruction set,
which is NEON.

There are two build items,

1) hns3_rxtx_vec_sve.c
2) rest of the library

There is already runtime checks to select Rx/Tx functions, so it is safe to
build (1) as long as compiler supports. If the platform doesn't support SVE, the
SVE path won't be selected during runtime.

For (2), it should be build to support NEON only, if it is compiled to support
SVE, it won't run on the platform that only supports NEON.

So, in below, if '__ARM_FEATURE_SVE' is supported, all driver is build with SVE
support, won't this cause a problem on the NEON platform?

What do you think to only keep the else leg of the below check, which is if
compiler supports SVE, set '-DCC_SVE_SUPPORT' flag and only build (1) with SVE 
flag?

> Fixes: 8c25b02b082a ("net/hns3: fix enabling SVE Rx/Tx")
> Fixes: 952ebacce4f2 ("net/hns3: support SVE Rx")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Chengwen Feng 
> ---
>  drivers/net/hns3/hns3_rxtx.c |  2 +-
>  drivers/net/hns3/meson.build | 13 +
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/hns3/hns3_rxtx.c b/drivers/net/hns3/hns3_rxtx.c
> index 1d7a769..4ef20c6 100644
> --- a/drivers/net/hns3/hns3_rxtx.c
> +++ b/drivers/net/hns3/hns3_rxtx.c
> @@ -2808,7 +2808,7 @@ hns3_get_default_vec_support(void)
>  static bool
>  hns3_get_sve_support(void)
>  {
> -#if defined(RTE_ARCH_ARM64) && defined(__ARM_FEATURE_SVE)
> +#if defined(CC_SVE_SUPPORT)
>   if (rte_vect_get_max_simd_bitwidth() < RTE_VECT_SIMD_256)
>   return false;
>   if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SVE))
> diff --git a/drivers/net/hns3/meson.build b/drivers/net/hns3/meson.build
> index 53c7df7..8563d70 100644
> --- a/drivers/net/hns3/meson.build
> +++ b/drivers/net/hns3/meson.build
> @@ -35,7 +35,20 @@ deps += ['hash']
>  
>  if arch_subdir == 'arm' and dpdk_conf.get('RTE_ARCH_64')
>  sources += files('hns3_rxtx_vec.c')
> +
> +# compile SVE when:
> +# a. support SVE in minimum instruction set baseline
> +# b. it's not minimum instruction set, but compiler support
>  if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != ''
> +cflags += ['-DCC_SVE_SUPPORT']
>  sources += files('hns3_rxtx_vec_sve.c')
> +elif cc.has_argument('-march=armv8.2-a+sve')
> +cflags += ['-DCC_SVE_SUPPORT']
> +hns3_sve_lib = static_library('hns3_sve_lib',
> +'hns3_rxtx_vec_sve.c',
> +dependencies: [static_rte_ethdev],
> +include_directories: includes,
> +c_args: [cflags, '-march=armv8.2-a+sve'])
> +objs += hns3_sve_lib.extract_objects('hns3_rxtx_vec_sve.c')
>  endif
>  endif
> 

Re: [dpdk-dev] [PATCH 1/1] event/dlb2: fix vector based dequeue

[Jerin Jacob]

On Wed, May 12, 2021 at 11:55 PM McDaniel, Timothy
 wrote:
>
> From: Timothy McDaniel 
>
> This commit fixes the following bugs in the vector based
> dequeue path:
> - extract hw sched type
> - update xstats
>
> The default mode of operation was also changed from vector
> optimized mode to scalar mode.
>
> Fixes: 000a7b8e7582 ("event/dlb2: optimize dequeue operation")
> Cc: timothy.mcdan...@intel.com

Removed this CC
>
> Signed-off-by: Timothy McDaniel 

@Thomas Monjalon  Could you merge this patch as this is one only patch
for the final release from eventdev.
This patch is passing my sanity build tests.

[dpdk-dev] [PATCH 1/2] common/sfc_efx/base: limit reported MCDI response length

[Ivan Malov]

From: Andy Moreton 

MCDI helper routines in libefx include length checks for response
messages, to ensure that short replies and optional fields are
handled correctly.

If the MCDI response message from the firmware is larger than the
caller's buffer then the response length reported to the caller
should be limited to the buffer size. Otherwise length checks in
the caller may allow reading past the end of the buffer.

Fixes: 6f619653b9b1 ("net/sfc/base: import MCDI implementation")
Cc: sta...@dpdk.org

Signed-off-by: Andy Moreton 
Signed-off-by: Ivan Malov 
Reviewed-by: Andrew Rybchenko 
---
 drivers/common/sfc_efx/base/efx_mcdi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/common/sfc_efx/base/efx_mcdi.c 
b/drivers/common/sfc_efx/base/efx_mcdi.c
index ff676f8a0..f4e1384d0 100644
--- a/drivers/common/sfc_efx/base/efx_mcdi.c
+++ b/drivers/common/sfc_efx/base/efx_mcdi.c
@@ -516,6 +516,9 @@ efx_mcdi_finish_response(
bytes = MIN(emrp->emr_out_length_used, emrp->emr_out_length);
efx_mcdi_read_response(enp, emrp->emr_out_buf, resp_off, bytes);
 
+   /* Report bytes copied to caller (response message may be larger) */
+   emrp->emr_out_length_used = bytes;
+
 #if EFSYS_OPT_MCDI_LOGGING
if (emtp->emt_logger != NULL) {
emtp->emt_logger(emtp->emt_context,
-- 
2.20.1

[dpdk-dev] [PATCH 2/2] common/sfc_efx/base: add missing MCDI response length checks

[Ivan Malov]

From: Andy Moreton 

Fixes: 6f619653b9b1 ("net/sfc/base: import MCDI implementation")
Fixes: e7cd430c864f ("net/sfc/base: import SFN7xxx family support")
Fixes: 94190e3543bf ("net/sfc/base: import SFN8xxx family support")
Fixes: 34285fd0891d ("common/sfc_efx/base: add match spec validate API")
Fixes: e61baa82e64b ("common/sfc_efx/base: add MAE action set provisioning 
APIs")
Fixes: b4fac34715f2 ("common/sfc_efx/base: add MAE action rule provisioning 
APIs")
Fixes: ed15d7f8e064 ("common/sfc_efx/base: validate and compare outer match 
specs")
Fixes: 7a673e1a4a05 ("common/sfc_efx/base: support outer rule provisioning")
Cc: sta...@dpdk.org

Signed-off-by: Andy Moreton 
Signed-off-by: Ivan Malov 
Reviewed-by: Andrew Rybchenko 
---
 drivers/common/sfc_efx/base/ef10_filter.c | 11 -
 drivers/common/sfc_efx/base/ef10_nic.c| 10 -
 drivers/common/sfc_efx/base/efx_mae.c | 52 +++
 drivers/common/sfc_efx/base/efx_mcdi.c|  7 +++
 4 files changed, 69 insertions(+), 11 deletions(-)

diff --git a/drivers/common/sfc_efx/base/ef10_filter.c 
b/drivers/common/sfc_efx/base/ef10_filter.c
index 0c99d4b74..ac6006c9b 100644
--- a/drivers/common/sfc_efx/base/ef10_filter.c
+++ b/drivers/common/sfc_efx/base/ef10_filter.c
@@ -1225,20 +1225,25 @@ efx_mcdi_get_parser_disp_info(
goto fail1;
}
 
+   if (req.emr_out_length_used < MC_CMD_GET_PARSER_DISP_INFO_OUT_LENMIN) {
+   rc = EMSGSIZE;
+   goto fail2;
+   }
+
matches_count = MCDI_OUT_DWORD(req,
GET_PARSER_DISP_INFO_OUT_NUM_SUPPORTED_MATCHES);
 
if (req.emr_out_length_used <
MC_CMD_GET_PARSER_DISP_INFO_OUT_LEN(matches_count)) {
rc = EMSGSIZE;
-   goto fail2;
+   goto fail3;
}
 
*list_lengthp = matches_count;
 
if (buffer_length < matches_count) {
rc = ENOSPC;
-   goto fail3;
+   goto fail4;
}
 
/*
@@ -1258,6 +1263,8 @@ efx_mcdi_get_parser_disp_info(
 
return (0);
 
+fail4:
+   EFSYS_PROBE(fail4);
 fail3:
EFSYS_PROBE(fail3);
 fail2:
diff --git a/drivers/common/sfc_efx/base/ef10_nic.c 
b/drivers/common/sfc_efx/base/ef10_nic.c
index 531365e42..eda0ad306 100644
--- a/drivers/common/sfc_efx/base/ef10_nic.c
+++ b/drivers/common/sfc_efx/base/ef10_nic.c
@@ -491,11 +491,17 @@ efx_mcdi_get_rxdp_config(
req.emr_out_length = MC_CMD_GET_RXDP_CONFIG_OUT_LEN;
 
efx_mcdi_execute(enp, &req);
+
if (req.emr_rc != 0) {
rc = req.emr_rc;
goto fail1;
}
 
+   if (req.emr_out_length_used < MC_CMD_GET_RXDP_CONFIG_OUT_LEN) {
+   rc = EMSGSIZE;
+   goto fail2;
+   }
+
if (MCDI_OUT_DWORD_FIELD(req, GET_RXDP_CONFIG_OUT_DATA,
GET_RXDP_CONFIG_OUT_PAD_HOST_DMA) == 0) {
/* RX DMA end padding is disabled */
@@ -514,7 +520,7 @@ efx_mcdi_get_rxdp_config(
break;
default:
rc = ENOTSUP;
-   goto fail2;
+   goto fail3;
}
}
 
@@ -522,6 +528,8 @@ efx_mcdi_get_rxdp_config(
 
return (0);
 
+fail3:
+   EFSYS_PROBE(fail3);
 fail2:
EFSYS_PROBE(fail2);
 fail1:
diff --git a/drivers/common/sfc_efx/base/efx_mae.c 
b/drivers/common/sfc_efx/base/efx_mae.c
index 80fe155d0..c1784211e 100644
--- a/drivers/common/sfc_efx/base/efx_mae.c
+++ b/drivers/common/sfc_efx/base/efx_mae.c
@@ -109,17 +109,22 @@ efx_mae_get_outer_rule_caps(
goto fail2;
}
 
+   if (req.emr_out_length_used < MC_CMD_MAE_GET_OR_CAPS_OUT_LENMIN) {
+   rc = EMSGSIZE;
+   goto fail3;
+   }
+
mcdi_field_ncaps = MCDI_OUT_DWORD(req, MAE_GET_OR_CAPS_OUT_COUNT);
 
if (req.emr_out_length_used <
MC_CMD_MAE_GET_OR_CAPS_OUT_LEN(mcdi_field_ncaps)) {
rc = EMSGSIZE;
-   goto fail3;
+   goto fail4;
}
 
if (mcdi_field_ncaps > field_ncaps) {
rc = EMSGSIZE;
-   goto fail4;
+   goto fail5;
}
 
for (i = 0; i < mcdi_field_ncaps; ++i) {
@@ -147,6 +152,8 @@ efx_mae_get_outer_rule_caps(
 
return (0);
 
+fail5:
+   EFSYS_PROBE(fail5);
 fail4:
EFSYS_PROBE(fail4);
 fail3:
@@ -191,17 +198,22 @@ efx_mae_get_action_rule_caps(
goto fail2;
}
 
-   mcdi_field_ncaps = MCDI_OUT_DWORD(req, MAE_GET_OR_CAPS_OUT_COUNT);
+   if (req.emr_out_length_used < MC_CMD_MAE_GET_AR_CAPS_OUT_LENMIN) {
+   rc = EMSGSIZE;
+   goto fail3;
+   }
+
+   mcdi_field_ncaps = MCDI_OUT_DWORD(req, MAE_GET_AR_CAPS_OUT_COUNT);
 
if (req.emr_out_length_used <
MC_CMD_MAE_GET_AR_CAPS_OUT_LEN(mcdi_field_ncaps)) {
rc = EMSGSIZE;
-   goto fail3;
+   

Re: [dpdk-dev] [PATCH v5 2/2] net/hns3: refactor SVE code compile method

[Bruce Richardson]

On Tue, May 18, 2021 at 03:40:18PM +0100, Ferruh Yigit wrote:
> On 5/14/2021 10:53 AM, Chengwen Feng wrote:
> > Currently, the SVE code is compiled only when -march supports SVE
> > (e.g. '-march=armv8.2a+sve'), there maybe some problem[1] with this
> > approach.
> > 
> > The solution:
> > a. If the minimum instruction set support SVE then compiles it.
> > b. Else if the compiler support SVE then compiles it.
> > c. Otherwise don't compile it.
> > 
> > [1] https://mails.dpdk.org/archives/dev/2021-April/208189.html
> > 
> 
> Hi Chengwen,
> 
> As far as I understand from above problem statement, you want to produce a
> binary that can run in two different platforms, one supports only NEON
> instructions, other supports NEON + SVE.
> 
> For this driver should be compiled in a way to support min instruction set,
> which is NEON.
> 
> There are two build items,
> 
> 1) hns3_rxtx_vec_sve.c
> 2) rest of the library
> 
> There is already runtime checks to select Rx/Tx functions, so it is safe to
> build (1) as long as compiler supports. If the platform doesn't support SVE, 
> the
> SVE path won't be selected during runtime.
> 
> For (2), it should be build to support NEON only, if it is compiled to support
> SVE, it won't run on the platform that only supports NEON.
> 
> So, in below, if '__ARM_FEATURE_SVE' is supported, all driver is build with 
> SVE
> support, won't this cause a problem on the NEON platform?
> 
In that case, the rest of DPDK is being build with SVE so having one driver
neon-only doesn't really make sense.

Overall, the patch looks to mirror what we do for AVX2/AVX512 support in
the Intel drivers, so looks ok to me.

/Bruce

Re: [dpdk-dev] [PATCH v5 2/2] net/hns3: refactor SVE code compile method

[Honnappa Nagarahalli]

> 
> On 5/14/2021 10:53 AM, Chengwen Feng wrote:
> > Currently, the SVE code is compiled only when -march supports SVE
> > (e.g. '-march=armv8.2a+sve'), there maybe some problem[1] with this
> > approach.
> >
> > The solution:
> > a. If the minimum instruction set support SVE then compiles it.
> > b. Else if the compiler support SVE then compiles it.
> > c. Otherwise don't compile it.
> >
> > [1] https://mails.dpdk.org/archives/dev/2021-April/208189.html
> >
> 
> Hi Chengwen,
> 
> As far as I understand from above problem statement, you want to produce a
> binary that can run in two different platforms, one supports only NEON
> instructions, other supports NEON + SVE.
> 
> For this driver should be compiled in a way to support min instruction set,
> which is NEON.
> 
> There are two build items,
> 
> 1) hns3_rxtx_vec_sve.c
> 2) rest of the library
> 
> There is already runtime checks to select Rx/Tx functions, so it is safe to 
> build
> (1) as long as compiler supports. If the platform doesn't support SVE, the SVE
> path won't be selected during runtime.
> 
> For (2), it should be build to support NEON only, if it is compiled to support
> SVE, it won't run on the platform that only supports NEON.
> 
> So, in below, if '__ARM_FEATURE_SVE' is supported, all driver is build with
> SVE support, won't this cause a problem on the NEON platform?
The first if statement checks if the user has enabled SVE during compilation 
which indicates that the user will run the binary on a platform that has SVE 
(the minimum ISA level supported by this binary), hence it is ok to compile all 
the code with SVE.

If the user has not enabled SVE during compilation which indicates the user 
might run the binary on a platform that does not have SVE, the second if 
statement, checks if the compiler supports SVE. If yes, it will compile the SVE 
version of the driver as well and the run time checks choose the correct 
version.

> 
> What do you think to only keep the else leg of the below check, which is if
> compiler supports SVE, set '-DCC_SVE_SUPPORT' flag and only build (1) with
> SVE flag?
> 
> > Fixes: 8c25b02b082a ("net/hns3: fix enabling SVE Rx/Tx")
> > Fixes: 952ebacce4f2 ("net/hns3: support SVE Rx")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Chengwen Feng 
> > ---
> >  drivers/net/hns3/hns3_rxtx.c |  2 +-
> >  drivers/net/hns3/meson.build | 13 +
> >  2 files changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/hns3/hns3_rxtx.c
> > b/drivers/net/hns3/hns3_rxtx.c index 1d7a769..4ef20c6 100644
> > --- a/drivers/net/hns3/hns3_rxtx.c
> > +++ b/drivers/net/hns3/hns3_rxtx.c
> > @@ -2808,7 +2808,7 @@ hns3_get_default_vec_support(void)
> >  static bool
> >  hns3_get_sve_support(void)
> >  {
> > -#if defined(RTE_ARCH_ARM64) && defined(__ARM_FEATURE_SVE)
> > +#if defined(CC_SVE_SUPPORT)
> > if (rte_vect_get_max_simd_bitwidth() < RTE_VECT_SIMD_256)
> > return false;
> > if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SVE))
> > diff --git a/drivers/net/hns3/meson.build
> > b/drivers/net/hns3/meson.build index 53c7df7..8563d70 100644
> > --- a/drivers/net/hns3/meson.build
> > +++ b/drivers/net/hns3/meson.build
> > @@ -35,7 +35,20 @@ deps += ['hash']
> >
> >  if arch_subdir == 'arm' and dpdk_conf.get('RTE_ARCH_64')
> >  sources += files('hns3_rxtx_vec.c')
> > +
> > +# compile SVE when:
> > +# a. support SVE in minimum instruction set baseline
> > +# b. it's not minimum instruction set, but compiler support
> >  if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != ''
> > +cflags += ['-DCC_SVE_SUPPORT']
> >  sources += files('hns3_rxtx_vec_sve.c')
> > +elif cc.has_argument('-march=armv8.2-a+sve')
> > +cflags += ['-DCC_SVE_SUPPORT']
> > +hns3_sve_lib = static_library('hns3_sve_lib',
> > +'hns3_rxtx_vec_sve.c',
> > +dependencies: [static_rte_ethdev],
> > +include_directories: includes,
> > +c_args: [cflags, '-march=armv8.2-a+sve'])
> > +objs += hns3_sve_lib.extract_objects('hns3_rxtx_vec_sve.c')
> >  endif
> >  endif
> >

Re: [dpdk-dev] [PATCH v5 2/2] net/hns3: refactor SVE code compile method

[Ferruh Yigit]

On 5/18/2021 4:48 PM, Honnappa Nagarahalli wrote:
> 
>>
>> On 5/14/2021 10:53 AM, Chengwen Feng wrote:
>>> Currently, the SVE code is compiled only when -march supports SVE
>>> (e.g. '-march=armv8.2a+sve'), there maybe some problem[1] with this
>>> approach.
>>>
>>> The solution:
>>> a. If the minimum instruction set support SVE then compiles it.
>>> b. Else if the compiler support SVE then compiles it.
>>> c. Otherwise don't compile it.
>>>
>>> [1] https://mails.dpdk.org/archives/dev/2021-April/208189.html
>>>
>>
>> Hi Chengwen,
>>
>> As far as I understand from above problem statement, you want to produce a
>> binary that can run in two different platforms, one supports only NEON
>> instructions, other supports NEON + SVE.
>>
>> For this driver should be compiled in a way to support min instruction set,
>> which is NEON.
>>
>> There are two build items,
>>
>> 1) hns3_rxtx_vec_sve.c
>> 2) rest of the library
>>
>> There is already runtime checks to select Rx/Tx functions, so it is safe to 
>> build
>> (1) as long as compiler supports. If the platform doesn't support SVE, the 
>> SVE
>> path won't be selected during runtime.
>>
>> For (2), it should be build to support NEON only, if it is compiled to 
>> support
>> SVE, it won't run on the platform that only supports NEON.
>>
>> So, in below, if '__ARM_FEATURE_SVE' is supported, all driver is build with
>> SVE support, won't this cause a problem on the NEON platform?
> The first if statement checks if the user has enabled SVE during compilation 
> which indicates that the user will run the binary on a platform that has SVE 
> (the minimum ISA level supported by this binary), hence it is ok to compile 
> all the code with SVE.
> 

So it is related to the what user provided (I assume as compiler flag), instead
of host HW capability.

> If the user has not enabled SVE during compilation which indicates the user 
> might run the binary on a platform that does not have SVE, the second if 
> statement, checks if the compiler supports SVE. If yes, it will compile the 
> SVE version of the driver as well and the run time checks choose the correct 
> version.
> 

OK, this sounds good, thanks for clarification.

>>
>> What do you think to only keep the else leg of the below check, which is if
>> compiler supports SVE, set '-DCC_SVE_SUPPORT' flag and only build (1) with
>> SVE flag?
>>
>>> Fixes: 8c25b02b082a ("net/hns3: fix enabling SVE Rx/Tx")
>>> Fixes: 952ebacce4f2 ("net/hns3: support SVE Rx")
>>> Cc: sta...@dpdk.org
>>>
>>> Signed-off-by: Chengwen Feng 
>>> ---
>>>  drivers/net/hns3/hns3_rxtx.c |  2 +-
>>>  drivers/net/hns3/meson.build | 13 +
>>>  2 files changed, 14 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/hns3/hns3_rxtx.c
>>> b/drivers/net/hns3/hns3_rxtx.c index 1d7a769..4ef20c6 100644
>>> --- a/drivers/net/hns3/hns3_rxtx.c
>>> +++ b/drivers/net/hns3/hns3_rxtx.c
>>> @@ -2808,7 +2808,7 @@ hns3_get_default_vec_support(void)
>>>  static bool
>>>  hns3_get_sve_support(void)
>>>  {
>>> -#if defined(RTE_ARCH_ARM64) && defined(__ARM_FEATURE_SVE)
>>> +#if defined(CC_SVE_SUPPORT)
>>> if (rte_vect_get_max_simd_bitwidth() < RTE_VECT_SIMD_256)
>>> return false;
>>> if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SVE))
>>> diff --git a/drivers/net/hns3/meson.build
>>> b/drivers/net/hns3/meson.build index 53c7df7..8563d70 100644
>>> --- a/drivers/net/hns3/meson.build
>>> +++ b/drivers/net/hns3/meson.build
>>> @@ -35,7 +35,20 @@ deps += ['hash']
>>>
>>>  if arch_subdir == 'arm' and dpdk_conf.get('RTE_ARCH_64')
>>>  sources += files('hns3_rxtx_vec.c')
>>> +
>>> +# compile SVE when:
>>> +# a. support SVE in minimum instruction set baseline
>>> +# b. it's not minimum instruction set, but compiler support
>>>  if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != ''
>>> +cflags += ['-DCC_SVE_SUPPORT']
>>>  sources += files('hns3_rxtx_vec_sve.c')
>>> +elif cc.has_argument('-march=armv8.2-a+sve')
>>> +cflags += ['-DCC_SVE_SUPPORT']
>>> +hns3_sve_lib = static_library('hns3_sve_lib',
>>> +'hns3_rxtx_vec_sve.c',
>>> +dependencies: [static_rte_ethdev],
>>> +include_directories: includes,
>>> +c_args: [cflags, '-march=armv8.2-a+sve'])
>>> +objs += hns3_sve_lib.extract_objects('hns3_rxtx_vec_sve.c')
>>>  endif
>>>  endif
>>>
> 

Re: [dpdk-dev] [PATCH v5 2/2] net/hns3: refactor SVE code compile method

[Ferruh Yigit]

On 5/18/2021 4:15 PM, Bruce Richardson wrote:
> On Tue, May 18, 2021 at 03:40:18PM +0100, Ferruh Yigit wrote:
>> On 5/14/2021 10:53 AM, Chengwen Feng wrote:
>>> Currently, the SVE code is compiled only when -march supports SVE
>>> (e.g. '-march=armv8.2a+sve'), there maybe some problem[1] with this
>>> approach.
>>>
>>> The solution:
>>> a. If the minimum instruction set support SVE then compiles it.
>>> b. Else if the compiler support SVE then compiles it.
>>> c. Otherwise don't compile it.
>>>
>>> [1] https://mails.dpdk.org/archives/dev/2021-April/208189.html
>>>
>>
>> Hi Chengwen,
>>
>> As far as I understand from above problem statement, you want to produce a
>> binary that can run in two different platforms, one supports only NEON
>> instructions, other supports NEON + SVE.
>>
>> For this driver should be compiled in a way to support min instruction set,
>> which is NEON.
>>
>> There are two build items,
>>
>> 1) hns3_rxtx_vec_sve.c
>> 2) rest of the library
>>
>> There is already runtime checks to select Rx/Tx functions, so it is safe to
>> build (1) as long as compiler supports. If the platform doesn't support SVE, 
>> the
>> SVE path won't be selected during runtime.
>>
>> For (2), it should be build to support NEON only, if it is compiled to 
>> support
>> SVE, it won't run on the platform that only supports NEON.
>>
>> So, in below, if '__ARM_FEATURE_SVE' is supported, all driver is build with 
>> SVE
>> support, won't this cause a problem on the NEON platform?
>>
> In that case, the rest of DPDK is being build with SVE so having one driver
> neon-only doesn't really make sense.
> 
> Overall, the patch looks to mirror what we do for AVX2/AVX512 support in
> the Intel drivers, so looks ok to me.
> 

I agree there is no point to make driver specific change if whole DPDK build
with SVE, and that seems controlled by user as Honnappa clarified.
So I will proceed with patch, thanks.

Re: [dpdk-dev] [PATCH v5 2/2] net/hns3: refactor SVE code compile method

[Honnappa Nagarahalli]

> > 
> >>
> >> On 5/14/2021 10:53 AM, Chengwen Feng wrote:
> >>> Currently, the SVE code is compiled only when -march supports SVE
> >>> (e.g. '-march=armv8.2a+sve'), there maybe some problem[1] with this
> >>> approach.
> >>>
> >>> The solution:
> >>> a. If the minimum instruction set support SVE then compiles it.
> >>> b. Else if the compiler support SVE then compiles it.
> >>> c. Otherwise don't compile it.
> >>>
> >>> [1] https://mails.dpdk.org/archives/dev/2021-April/208189.html
> >>>
> >>
> >> Hi Chengwen,
> >>
> >> As far as I understand from above problem statement, you want to
> >> produce a binary that can run in two different platforms, one
> >> supports only NEON instructions, other supports NEON + SVE.
> >>
> >> For this driver should be compiled in a way to support min
> >> instruction set, which is NEON.
> >>
> >> There are two build items,
> >>
> >> 1) hns3_rxtx_vec_sve.c
> >> 2) rest of the library
> >>
> >> There is already runtime checks to select Rx/Tx functions, so it is
> >> safe to build
> >> (1) as long as compiler supports. If the platform doesn't support
> >> SVE, the SVE path won't be selected during runtime.
> >>
> >> For (2), it should be build to support NEON only, if it is compiled
> >> to support SVE, it won't run on the platform that only supports NEON.
> >>
> >> So, in below, if '__ARM_FEATURE_SVE' is supported, all driver is
> >> build with SVE support, won't this cause a problem on the NEON platform?
> > The first if statement checks if the user has enabled SVE during compilation
> which indicates that the user will run the binary on a platform that has SVE
> (the minimum ISA level supported by this binary), hence it is ok to compile 
> all
> the code with SVE.
> >
> 
> So it is related to the what user provided (I assume as compiler flag), 
> instead
> of host HW capability.
It is the HW host capability as provided in the compiler flag. It is coming 
from config/arm/meson.build.

> 
> > If the user has not enabled SVE during compilation which indicates the user
> might run the binary on a platform that does not have SVE, the second if
> statement, checks if the compiler supports SVE. If yes, it will compile the 
> SVE
> version of the driver as well and the run time checks choose the correct
> version.
> >
> 
> OK, this sounds good, thanks for clarification.
> 
> >>
> >> What do you think to only keep the else leg of the below check, which
> >> is if compiler supports SVE, set '-DCC_SVE_SUPPORT' flag and only
> >> build (1) with SVE flag?
> >>
> >>> Fixes: 8c25b02b082a ("net/hns3: fix enabling SVE Rx/Tx")
> >>> Fixes: 952ebacce4f2 ("net/hns3: support SVE Rx")
> >>> Cc: sta...@dpdk.org
> >>>
> >>> Signed-off-by: Chengwen Feng 
> >>> ---
> >>>  drivers/net/hns3/hns3_rxtx.c |  2 +-  drivers/net/hns3/meson.build
> >>> | 13 +
> >>>  2 files changed, 14 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/net/hns3/hns3_rxtx.c
> >>> b/drivers/net/hns3/hns3_rxtx.c index 1d7a769..4ef20c6 100644
> >>> --- a/drivers/net/hns3/hns3_rxtx.c
> >>> +++ b/drivers/net/hns3/hns3_rxtx.c
> >>> @@ -2808,7 +2808,7 @@ hns3_get_default_vec_support(void)
> >>>  static bool
> >>>  hns3_get_sve_support(void)
> >>>  {
> >>> -#if defined(RTE_ARCH_ARM64) && defined(__ARM_FEATURE_SVE)
> >>> +#if defined(CC_SVE_SUPPORT)
> >>>   if (rte_vect_get_max_simd_bitwidth() < RTE_VECT_SIMD_256)
> >>>   return false;
> >>>   if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SVE))
> >>> diff --git a/drivers/net/hns3/meson.build
> >>> b/drivers/net/hns3/meson.build index 53c7df7..8563d70 100644
> >>> --- a/drivers/net/hns3/meson.build
> >>> +++ b/drivers/net/hns3/meson.build
> >>> @@ -35,7 +35,20 @@ deps += ['hash']
> >>>
> >>>  if arch_subdir == 'arm' and dpdk_conf.get('RTE_ARCH_64')
> >>>  sources += files('hns3_rxtx_vec.c')
> >>> +
> >>> +# compile SVE when:
> >>> +# a. support SVE in minimum instruction set baseline
> >>> +# b. it's not minimum instruction set, but compiler support
> >>>  if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != ''
> >>> +cflags += ['-DCC_SVE_SUPPORT']
> >>>  sources += files('hns3_rxtx_vec_sve.c')
> >>> +elif cc.has_argument('-march=armv8.2-a+sve')
> >>> +cflags += ['-DCC_SVE_SUPPORT']
> >>> +hns3_sve_lib = static_library('hns3_sve_lib',
> >>> +'hns3_rxtx_vec_sve.c',
> >>> +dependencies: [static_rte_ethdev],
> >>> +include_directories: includes,
> >>> +c_args: [cflags, '-march=armv8.2-a+sve'])
> >>> +objs += hns3_sve_lib.extract_objects('hns3_rxtx_vec_sve.c')
> >>>  endif
> >>>  endif
> >>>
> >

Re: [dpdk-dev] [PATCH v5 2/2] net/hns3: refactor SVE code compile method

[Ferruh Yigit]

On 5/14/2021 3:12 PM, Honnappa Nagarahalli wrote:
> 
> 
>>
>> Currently, the SVE code is compiled only when -march supports SVE (e.g. '-
>> march=armv8.2a+sve'), there maybe some problem[1] with this approach.
>>
>> The solution:
>> a. If the minimum instruction set support SVE then compiles it.
>> b. Else if the compiler support SVE then compiles it.
>> c. Otherwise don't compile it.
>>
>> [1] https://mails.dpdk.org/archives/dev/2021-April/208189.html
>>
>> Fixes: 8c25b02b082a ("net/hns3: fix enabling SVE Rx/Tx")
>> Fixes: 952ebacce4f2 ("net/hns3: support SVE Rx")
>> Cc: sta...@dpdk.org
>>
>> Signed-off-by: Chengwen Feng 
> Looks good to me.
> Reviewed-by: Honnappa Nagarahalli 
> 

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

(Only this patch, 2/2, applied, not whole set)

Re: [dpdk-dev] [PATCH v5 2/2] net/hns3: refactor SVE code compile method

[Ferruh Yigit]

On 5/18/2021 5:12 PM, Honnappa Nagarahalli wrote:
>>> 

 On 5/14/2021 10:53 AM, Chengwen Feng wrote:
> Currently, the SVE code is compiled only when -march supports SVE
> (e.g. '-march=armv8.2a+sve'), there maybe some problem[1] with this
> approach.
>
> The solution:
> a. If the minimum instruction set support SVE then compiles it.
> b. Else if the compiler support SVE then compiles it.
> c. Otherwise don't compile it.
>
> [1] https://mails.dpdk.org/archives/dev/2021-April/208189.html
>

 Hi Chengwen,

 As far as I understand from above problem statement, you want to
 produce a binary that can run in two different platforms, one
 supports only NEON instructions, other supports NEON + SVE.

 For this driver should be compiled in a way to support min
 instruction set, which is NEON.

 There are two build items,

 1) hns3_rxtx_vec_sve.c
 2) rest of the library

 There is already runtime checks to select Rx/Tx functions, so it is
 safe to build
 (1) as long as compiler supports. If the platform doesn't support
 SVE, the SVE path won't be selected during runtime.

 For (2), it should be build to support NEON only, if it is compiled
 to support SVE, it won't run on the platform that only supports NEON.

 So, in below, if '__ARM_FEATURE_SVE' is supported, all driver is
 build with SVE support, won't this cause a problem on the NEON platform?
>>> The first if statement checks if the user has enabled SVE during compilation
>> which indicates that the user will run the binary on a platform that has SVE
>> (the minimum ISA level supported by this binary), hence it is ok to compile 
>> all
>> the code with SVE.
>>>
>>
>> So it is related to the what user provided (I assume as compiler flag), 
>> instead
>> of host HW capability.
> It is the HW host capability as provided in the compiler flag. It is coming 
> from config/arm/meson.build.
> 

Is this patch has dependency to 1/2, that updates 'config/arm/meson.build'?

What I understand is, if user provides compiler argument to request SVE,
something like '-march=armv8.2-a+sve', and host HW supports it, whole driver
will be built with SVE support.

If user not request SVE, driver won't be compiled with SVE support even if HW
support it, but only 'hns3_rxtx_vec_sve.c' will be compiled if compiler supports
SVE.

Is above correct and does it have any dependency to first patch, I thought this
is independent from first patch.


>>
>>> If the user has not enabled SVE during compilation which indicates the user
>> might run the binary on a platform that does not have SVE, the second if
>> statement, checks if the compiler supports SVE. If yes, it will compile the 
>> SVE
>> version of the driver as well and the run time checks choose the correct
>> version.
>>>
>>
>> OK, this sounds good, thanks for clarification.
>>

 What do you think to only keep the else leg of the below check, which
 is if compiler supports SVE, set '-DCC_SVE_SUPPORT' flag and only
 build (1) with SVE flag?

> Fixes: 8c25b02b082a ("net/hns3: fix enabling SVE Rx/Tx")
> Fixes: 952ebacce4f2 ("net/hns3: support SVE Rx")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Chengwen Feng 
> ---
>  drivers/net/hns3/hns3_rxtx.c |  2 +-  drivers/net/hns3/meson.build
> | 13 +
>  2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/hns3/hns3_rxtx.c
> b/drivers/net/hns3/hns3_rxtx.c index 1d7a769..4ef20c6 100644
> --- a/drivers/net/hns3/hns3_rxtx.c
> +++ b/drivers/net/hns3/hns3_rxtx.c
> @@ -2808,7 +2808,7 @@ hns3_get_default_vec_support(void)
>  static bool
>  hns3_get_sve_support(void)
>  {
> -#if defined(RTE_ARCH_ARM64) && defined(__ARM_FEATURE_SVE)
> +#if defined(CC_SVE_SUPPORT)
>   if (rte_vect_get_max_simd_bitwidth() < RTE_VECT_SIMD_256)
>   return false;
>   if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SVE))
> diff --git a/drivers/net/hns3/meson.build
> b/drivers/net/hns3/meson.build index 53c7df7..8563d70 100644
> --- a/drivers/net/hns3/meson.build
> +++ b/drivers/net/hns3/meson.build
> @@ -35,7 +35,20 @@ deps += ['hash']
>
>  if arch_subdir == 'arm' and dpdk_conf.get('RTE_ARCH_64')
>  sources += files('hns3_rxtx_vec.c')
> +
> +# compile SVE when:
> +# a. support SVE in minimum instruction set baseline
> +# b. it's not minimum instruction set, but compiler support
>  if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != ''
> +cflags += ['-DCC_SVE_SUPPORT']
>  sources += files('hns3_rxtx_vec_sve.c')
> +elif cc.has_argument('-march=armv8.2-a+sve')
> +cflags += ['-DCC_SVE_SUPPORT']
> +hns3_sve_lib = static_library('hns3_sve_lib',
> +'hns3_rxtx_vec_sve.c',
> +

Re: [dpdk-dev] [PATCH 1/2] common/sfc_efx/base: limit reported MCDI response length

[Ferruh Yigit]

On 5/18/2021 4:10 PM, Ivan Malov wrote:
> From: Andy Moreton 
> 
> MCDI helper routines in libefx include length checks for response
> messages, to ensure that short replies and optional fields are
> handled correctly.
> 
> If the MCDI response message from the firmware is larger than the
> caller's buffer then the response length reported to the caller
> should be limited to the buffer size. Otherwise length checks in
> the caller may allow reading past the end of the buffer.
> 
> Fixes: 6f619653b9b1 ("net/sfc/base: import MCDI implementation")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Andy Moreton 
> Signed-off-by: Ivan Malov 
> Reviewed-by: Andrew Rybchenko 

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

[dpdk-dev] [PATCH v7] doc: add release milestones definition

[Thomas Monjalon]

From: Asaf Penso 

Adding more information about the release milestones.
This includes the scope of change, expectations, etc.

Signed-off-by: Asaf Penso 
Signed-off-by: Thomas Monjalon 
Acked-by: John McNamara 
Acked-by: Ajit Khaparde 
---
v2: fix styling format and add content in the commit message
v3: change punctuation and avoid plural form when unneeded
v4: avoid abbreviations, "Priority" in -rc, and reword as John suggests
v5: note that release candidates may vary
v6: merge RFC and proposal deadline, add roadmap link and reduce duplication
v7: make expectations clearer and stricter
---
 doc/guides/contributing/patches.rst | 72 +
 1 file changed, 72 insertions(+)

diff --git a/doc/guides/contributing/patches.rst 
b/doc/guides/contributing/patches.rst
index 6dbbd5f8d1..4d70e326c5 100644
--- a/doc/guides/contributing/patches.rst
+++ b/doc/guides/contributing/patches.rst
@@ -177,6 +177,8 @@ Make your planned changes in the cloned ``dpdk`` repo. Here 
are some guidelines
 * Add documentation, if relevant, in the form of Doxygen comments or a User 
Guide in RST format.
   See the :ref:`Documentation Guidelines `.
 
+* Code and related documentation must be updated atomically in the same patch.
+
 Once the changes have been made you should commit them to your local repo.
 
 For small changes, that do not require specific explanations, it is better to 
keep things together in the
@@ -660,3 +662,73 @@ patch accepted. The general cycle for patch review and 
acceptance is:
  than rework of the original.
* Trivial patches may be merged sooner than described above at the tree 
committer's
  discretion.
+
+
+Milestones definition
+-
+
+Each DPDK release has milestones that help everyone to converge to the release 
date.
+The following is a list of these milestones
+together with concrete definitions and expectations,
+for a typical release cycle (3 months ending after 4 release candidates).
+The number and expectations of release candidates might vary slightly.
+The schedule is updated in the `roadmap 
`_.
+
+.. note::
+   Sooner is always better. Deadlines are not ideal dates.
+
+   Integration is never guaranteed but everyone can help.
+
+Roadmap
+~~~
+
+* Announce new features in libraries, drivers, applications, and examples.
+* To be published before the first day of the release cycle.
+
+Proposal Deadline
+~
+
+* Must send an RFC or a complete v1 patch.
+* Early RFC gives time for design review before complete implementation.
+* Should include at least the API changes in libraries and applications.
+* Library code should be quite complete at the deadline.
+* Nice to have: driver implementation (full or draft), example code, and 
documentation.
+
+rc1
+~~~
+
+* Priority: libraries. No library feature should be accepted after -rc1.
+* New API must be defined and implemented in libraries.
+* The API must include Doxygen documentation
+  and be part of the relevant .rst files (library-specific and release notes).
+* API should be used in a test application (``/app``).
+* At least one PMD should implement the API.
+  It may be a draft sent in a separate series.
+* The above should be sent to the mailing list at least 2 weeks before -rc1.
+* Nice to have: example code (``/examples``)
+
+rc2
+~~~
+
+* Priority: drivers. No driver feature should be accepted after -rc2.
+* A driver change must include documentation
+  in the relevant .rst files (driver-specific and release notes).
+* The above should be sent to the mailing list at least 2 weeks before -rc2.
+
+rc3
+~~~
+
+* Priority: applications. No application feature should be accepted after -rc3.
+* New functionality that does not depend on libraries update
+  can be integrated as part of -rc3.
+* The application change must include documentation in the relevant .rst files
+  (application-specific and release notes if significant).
+* Libraries and drivers cleanup are allowed.
+* Small driver reworks.
+* Critical and minor bug fixes.
+
+rc4
+~~~
+
+* Documentation updates.
+* Critical bug fixes.
-- 
2.31.1

Re: [dpdk-dev] [PATCH v6 0/3] rte_flow doc matrix

[Thomas Monjalon]

18/05/2021 15:28, Thomas Monjalon:
> This is an improvement of the rte_flow documentation:
> it makes possible to quickly read which items and actions
> may be accepted by the drivers in a DPDK release.
> 
> A script is provided for CI to make sure the matrix is updated,
> while allowing some manual tuning.
> 
> Thomas Monjalon (3):
>   doc: rename sfc features file
>   doc: add flow API features tables
>   devtools: check flow API doc tables

Applied

Re: [dpdk-dev] [PATCH] net/memif: fix missing Tx-bps stats for zero-copy

[Jakub Grajciar -X (jgrajcia - PANTHEON TECH SRO at Cisco)]

Hi,

Sorry for the inconvenience, the thread got filtered so I only found it in spam 
after Damjan notified me.

The patch looks good. Thanks for the fix!

Reviewd-By: Jakub Grajciar 

> -Original Message-
> From: Ferruh Yigit 
> Sent: Tuesday, May 18, 2021 2:09 PM
> To: Tianyu Li ; Jakub Grajciar -X (jgrajcia - PANTHEON
> TECH SRO at Cisco) ; Damjan Marion (damarion)
> 
> Cc: dev@dpdk.org; nd ; sta...@dpdk.org
> Subject: Re: [PATCH] net/memif: fix missing Tx-bps stats for zero-copy
> 
> On 4/27/2021 7:30 AM, Tianyu Li wrote:
> > Hi Jakub,
> >
> > Any comments about the patch?
> >
> 
> +Damjan.
> 
> Hi Damjan,
> 
> Is Jakub still maintaining the net/memif?
> 
> > -Original Message-
> > From: Ferruh Yigit 
> > Sent: Wednesday, April 14, 2021 4:13 PM
> > To: Tianyu Li ; Jakub Grajciar 
> > Cc: dev@dpdk.org; nd ; sta...@dpdk.org
> > Subject: Re: [PATCH] net/memif: fix missing Tx-bps stats for zero-copy
> >
> > On 4/12/2021 9:22 AM, Tianyu Li wrote:
> >> Fix the missing Tx-bps counter for memif zero-copy mode Before
> >>Rx-pps:  6891450  Rx-bps:   3528438928
> >>Tx-pps:  6891482  Tx-bps:0
> >> After
> >>Throughput (since last show)
> >>Rx-pps: 11157056  Rx-bps:   5712413016
> >>Tx-pps: 11157056  Tx-bps:   5712413016
> >>
> >> Fixes: 43b815d88188 ("net/memif: support zero-copy slave")
> >> Cc: sta...@dpdk.org
> >>
> >> Signed-off-by: Tianyu Li 
> >> ---
> >>   drivers/net/memif/rte_eth_memif.c | 1 +
> >>   1 file changed, 1 insertion(+)
> >>
> >> diff --git a/drivers/net/memif/rte_eth_memif.c
> >> b/drivers/net/memif/rte_eth_memif.c
> >> index 77c95bcb7..dd2825968 100644
> >> --- a/drivers/net/memif/rte_eth_memif.c
> >> +++ b/drivers/net/memif/rte_eth_memif.c
> >> @@ -706,6 +706,7 @@ memif_tx_one_zc(struct pmd_process_private
> *proc_private, struct memif_queue *mq
> >>/* populate descriptor */
> >>d0 = &ring->desc[slot & mask];
> >>d0->length = rte_pktmbuf_data_len(mbuf);
> >> +  mq->n_bytes += rte_pktmbuf_data_len(mbuf);
> >>/* FIXME: get region index */
> >>d0->region = 1;
> >>d0->offset = rte_pktmbuf_mtod(mbuf, uint8_t *) -
> >>
> >
> > Looks good to me, but let's wait Jakub's review for it.
> >

Re: [dpdk-dev] [PATCH] dpdk-kmods/windows: Add Intel 100GbE Ethernet adapter device IDs to netuio INF

[Narcisa Ana Maria Vasile]

On Wed, Apr 21, 2021 at 05:05:03PM -0700, Pallavi Kadam wrote:
> Add Intel 100GbE Ethernet adapter device IDs to netuio inf file
> in order to enable them on Windows.
> 
> Signed-off-by: Pallavi Kadam 
> Reviewed-by: Ranjit Menon 
> ---
>  windows/netuio/netuio.inf | 12 
>  1 file changed, 12 insertions(+)
> 
Successfully installed netuio and tested by running l2fwd.

Acked-by: Narcisa Vasile 

Re: [dpdk-dev] [PATCH] ppc64le: fix build without glibc and using Clang

[David Christensen]

On 5/16/21 5:46 PM, Piotr Kubaj wrote:

__ppc_get_timebase() is only present when glibc is used.

Signed-off-by: Piotr Kubaj 
---
  lib/eal/ppc/include/rte_altivec.h |  3 +++
  lib/eal/ppc/include/rte_cycles.h  | 12 
  lib/eal/ppc/rte_cycles.c  | 16 
  3 files changed, 31 insertions(+)

diff --git a/lib/eal/ppc/include/rte_altivec.h 
b/lib/eal/ppc/include/rte_altivec.h
index 1551a94544..3fcc819c11 100644
--- a/lib/eal/ppc/include/rte_altivec.h
+++ b/lib/eal/ppc/include/rte_altivec.h
@@ -7,6 +7,9 @@
  #define _RTE_ALTIVEC_H_

  /* To include altivec.h, GCC version must be >= 4.8 */
+#ifdef __clang__
+#define vector __vector
+#endif
  #include 

  /*
diff --git a/lib/eal/ppc/include/rte_cycles.h b/lib/eal/ppc/include/rte_cycles.h
index 5585f9273c..a8307ceaff 100644
--- a/lib/eal/ppc/include/rte_cycles.h
+++ b/lib/eal/ppc/include/rte_cycles.h
@@ -10,7 +10,13 @@
  extern "C" {
  #endif

+#ifdef linux
+#include 
+#endif
+
+#ifdef __GLIBC__
  #include 
+#endif

  #include "generic/rte_cycles.h"

@@ -26,7 +32,13 @@ extern "C" {
  static inline uint64_t
  rte_rdtsc(void)
  {
+#ifdef __GLIBC__
return __ppc_get_timebase();
+#else
+   uint64_t __tb;
+   __asm__ volatile ("mfspr %0, 268" : "=r" (__tb));
+   return __tb;
+#endif
  }

  static inline uint64_t
diff --git a/lib/eal/ppc/rte_cycles.c b/lib/eal/ppc/rte_cycles.c
index 3180adb0ff..48545c4d67 100644
--- a/lib/eal/ppc/rte_cycles.c
+++ b/lib/eal/ppc/rte_cycles.c
@@ -2,12 +2,28 @@
   * Copyright (C) IBM Corporation 2019.
   */

+#ifdef linux
+#include 
+#elif defined(__FreeBSD__)
+#include 
+#include 
+#endif
+
+#ifdef __GLIBC__
  #include 
+#endif

  #include "eal_private.h"

  uint64_t
  get_tsc_freq_arch(void)
  {
+#ifdef __GLIBC__
return __ppc_get_timebase_freq();
+#elif defined(__FreeBSD__)
+   uint64_t freq;
+   size_t length = sizeof(freq);
+   sysctlbyname("kern.timecounter.tc.timebase.frequency", &freq, &length, 
NULL, 0);
+   return freq;
+#endif
  }



Reviewed-by: David Christensen 

Re: [dpdk-dev] [PATCH v6] doc: add release milestones definition

[Ajit Khaparde]

>
>
> >> I already mentioned above, but this can cause misunderstanding. We want
> all
> >> driver implementation to be ready for proposal deadline, same as other
> patches.
> >> But because of its reduced scope (they don't affect all project but only
> >> specific vendor), we are flexible to get driver features for -rc2 and
> -rc3 too.
> >
> > -rc3 really? It should be exceptional so not mentioned here.
>
Agree. Let's keep it to -rc2.


> >
>
> In practice we are having it, but agree to have it exceptional and not
> mention
> in the guide.
>
> >> Please check number of driver patches merged for a release, it is
> impossible to
> >> manage them within period between -rc1 & -rc2.
> >> Also some driver features are complex and big, they should be sent
> before
> >> proposal deadline so that they can be reviewed for the release.
> >
> > Yes sooner is better. The doc is about deadline + priorities,
> > showing the no-go limits, without warranty of merge if all good.
> > Is there a contradiction?
> >
>
> My concern is document can be read as, it is normal/expected to send driver
> patches after -rc1, because this documents as -rc2 task is driver patches.
>
> I am OK with it if it is clear that deadline is -rc2, but normal/expected
> is to
> have driver patches also before proposal deadline.
>
+1


>
>
>
>

Re: [dpdk-dev] [PATCH v5 2/2] net/hns3: refactor SVE code compile method

[fengchengwen]

On 2021/5/19 0:37, Ferruh Yigit wrote:
> On 5/18/2021 5:12 PM, Honnappa Nagarahalli wrote:
 
>
> On 5/14/2021 10:53 AM, Chengwen Feng wrote:
>> Currently, the SVE code is compiled only when -march supports SVE
>> (e.g. '-march=armv8.2a+sve'), there maybe some problem[1] with this
>> approach.
>>
>> The solution:
>> a. If the minimum instruction set support SVE then compiles it.
>> b. Else if the compiler support SVE then compiles it.
>> c. Otherwise don't compile it.
>>
>> [1] https://mails.dpdk.org/archives/dev/2021-April/208189.html
>>
>
> Hi Chengwen,
>
> As far as I understand from above problem statement, you want to
> produce a binary that can run in two different platforms, one
> supports only NEON instructions, other supports NEON + SVE.
>
> For this driver should be compiled in a way to support min
> instruction set, which is NEON.
>
> There are two build items,
>
> 1) hns3_rxtx_vec_sve.c
> 2) rest of the library
>
> There is already runtime checks to select Rx/Tx functions, so it is
> safe to build
> (1) as long as compiler supports. If the platform doesn't support
> SVE, the SVE path won't be selected during runtime.
>
> For (2), it should be build to support NEON only, if it is compiled
> to support SVE, it won't run on the platform that only supports NEON.
>
> So, in below, if '__ARM_FEATURE_SVE' is supported, all driver is
> build with SVE support, won't this cause a problem on the NEON platform?
 The first if statement checks if the user has enabled SVE during 
 compilation
>>> which indicates that the user will run the binary on a platform that has SVE
>>> (the minimum ISA level supported by this binary), hence it is ok to compile 
>>> all
>>> the code with SVE.

>>>
>>> So it is related to the what user provided (I assume as compiler flag), 
>>> instead
>>> of host HW capability.
>> It is the HW host capability as provided in the compiler flag. It is coming 
>> from config/arm/meson.build.
>>
> 
> Is this patch has dependency to 1/2, that updates 'config/arm/meson.build'?
> 
> What I understand is, if user provides compiler argument to request SVE,
> something like '-march=armv8.2-a+sve', and host HW supports it, whole driver
> will be built with SVE support.
> 
> If user not request SVE, driver won't be compiled with SVE support even if HW
> support it, but only 'hns3_rxtx_vec_sve.c' will be compiled if compiler 
> supports
> SVE.
> 
> Is above correct and does it have any dependency to first patch, I thought 
> this
> is independent from first patch.
> 

Yes, you are right, it's independent from first patch.

> 
>>>
 If the user has not enabled SVE during compilation which indicates the user
>>> might run the binary on a platform that does not have SVE, the second if
>>> statement, checks if the compiler supports SVE. If yes, it will compile the 
>>> SVE
>>> version of the driver as well and the run time checks choose the correct
>>> version.

>>>
>>> OK, this sounds good, thanks for clarification.
>>>
>
> What do you think to only keep the else leg of the below check, which
> is if compiler supports SVE, set '-DCC_SVE_SUPPORT' flag and only
> build (1) with SVE flag?
>
>> Fixes: 8c25b02b082a ("net/hns3: fix enabling SVE Rx/Tx")
>> Fixes: 952ebacce4f2 ("net/hns3: support SVE Rx")
>> Cc: sta...@dpdk.org
>>
>> Signed-off-by: Chengwen Feng 
>> ---
>>  drivers/net/hns3/hns3_rxtx.c |  2 +-  drivers/net/hns3/meson.build
>> | 13 +
>>  2 files changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/hns3/hns3_rxtx.c
>> b/drivers/net/hns3/hns3_rxtx.c index 1d7a769..4ef20c6 100644
>> --- a/drivers/net/hns3/hns3_rxtx.c
>> +++ b/drivers/net/hns3/hns3_rxtx.c
>> @@ -2808,7 +2808,7 @@ hns3_get_default_vec_support(void)
>>  static bool
>>  hns3_get_sve_support(void)
>>  {
>> -#if defined(RTE_ARCH_ARM64) && defined(__ARM_FEATURE_SVE)
>> +#if defined(CC_SVE_SUPPORT)
>>  if (rte_vect_get_max_simd_bitwidth() < RTE_VECT_SIMD_256)
>>  return false;
>>  if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SVE))
>> diff --git a/drivers/net/hns3/meson.build
>> b/drivers/net/hns3/meson.build index 53c7df7..8563d70 100644
>> --- a/drivers/net/hns3/meson.build
>> +++ b/drivers/net/hns3/meson.build
>> @@ -35,7 +35,20 @@ deps += ['hash']
>>
>>  if arch_subdir == 'arm' and dpdk_conf.get('RTE_ARCH_64')
>>  sources += files('hns3_rxtx_vec.c')
>> +
>> +# compile SVE when:
>> +# a. support SVE in minimum instruction set baseline
>> +# b. it's not minimum instruction set, but compiler support
>>  if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != ''
>> +cflags += ['-DCC_SVE_SUPPORT']
>> 

Re: [dpdk-dev] [PATCH v5 2/2] net/hns3: refactor SVE code compile method

[fengchengwen]

Thanks a lot, Ferruh & Honnappa.

On 2021/5/19 0:27, Ferruh Yigit wrote:
> On 5/14/2021 3:12 PM, Honnappa Nagarahalli wrote:
>> 
>>
>>>
>>> Currently, the SVE code is compiled only when -march supports SVE (e.g. '-
>>> march=armv8.2a+sve'), there maybe some problem[1] with this approach.
>>>
>>> The solution:
>>> a. If the minimum instruction set support SVE then compiles it.
>>> b. Else if the compiler support SVE then compiles it.
>>> c. Otherwise don't compile it.
>>>
>>> [1] https://mails.dpdk.org/archives/dev/2021-April/208189.html
>>>
>>> Fixes: 8c25b02b082a ("net/hns3: fix enabling SVE Rx/Tx")
>>> Fixes: 952ebacce4f2 ("net/hns3: support SVE Rx")
>>> Cc: sta...@dpdk.org
>>>
>>> Signed-off-by: Chengwen Feng 
>> Looks good to me.
>> Reviewed-by: Honnappa Nagarahalli 
>>
> 
> Applied to dpdk-next-net/main, thanks.
> 
> (Only this patch, 2/2, applied, not whole set)
> 
> .
> 

[dpdk-dev] 回复: [PATCH v3 2/2] net/mlx5: remove unnecessary wmb for Memory Region cache

[Feifei Wang]

Also thanks for your patient communication and explanation.
A wonderful discussion~

Best Regards
Feifei
> -邮件原件-
> 发件人: Slava Ovsiienko 
> 发送时间: 2021年5月18日 18:18
> 收件人: Feifei Wang ; Matan Azrad
> ; Shahaf Shuler 
> 抄送: dev@dpdk.org; nd ; Ruifeng Wang
> 
> 主题: RE: [PATCH v3 2/2] net/mlx5: remove unnecessary wmb for Memory
> Region cache
> 
> > -Original Message-
> > From: Feifei Wang 
> > Sent: Tuesday, May 18, 2021 11:51
> > To: Matan Azrad ; Shahaf Shuler
> > ; Slava Ovsiienko 
> > Cc: dev@dpdk.org; n...@arm.com; Feifei Wang ;
> > Ruifeng Wang 
> > Subject: [PATCH v3 2/2] net/mlx5: remove unnecessary wmb for Memory
> > Region cache
> >
> > 'dev_gen' is a variable to trigger all cores to flush their local
> > caches once the global MR cache has been rebuilt.
> >
> > This is due to MR cache's R/W lock can maintain synchronization
> > between
> > threads:
> >
> > 1. dev_gen and global cache updating ordering inside the lock
> > protected section does not matter. Because other threads cannot take
> > the lock until global cache has been updated. Thus, in out of order
> > platform, even if other agents firstly observe updated dev_gen but
> > global does not update, they also have to wait the lock. As a result,
> > it is unnecessary to add a wmb between global cache rebuilding and
> > updating the dev_gen to keep the memory store order.
> >
> > 2. Store-Release of unlock provides the implicit wmb at the level
> > visible by software. This makes 'rebuilding global cache' and
> > 'updating dev_gen' be observed before local_cache starts to be updated
> > by other agents. Thus, wmb after 'updating dev_gen' can be removed.
> >
> > Suggested-by: Ruifeng Wang 
> > Signed-off-by: Feifei Wang 
> > Reviewed-by: Ruifeng Wang 
> Acked-by: Viacheslav Ovsiienko 
> 
> Thanks a lot for patience and cooperation.
> With best regards,
> Slava

[dpdk-dev] 回复: 回复: [PATCH] app/eventdev: remove unnecessary barrier for order test

[Feifei Wang]

> -邮件原件-
> 发件人: Jerin Jacob 
> 发送时间: 2021年5月18日 22:05
> 收件人: Feifei Wang 
> 抄送: jer...@marvell.com; Ruifeng Wang ;
> Honnappa Nagarahalli ; dev@dpdk.org;
> nd 
> 主题: Re: [dpdk-dev] 回复: [PATCH] app/eventdev: remove unnecessary
> barrier for order test
> 
> On Tue, May 18, 2021 at 2:57 PM Feifei Wang 
> wrote:
> >
> > Hi, Jerin
> >
> > Sorry to disturb you. Would you please help me review this patch, thanks
> very much.
> 
> In slowpath, I thought of having this barrier. But I agree that it can be
> removed.
> Since this is not bug. We will merge this patch in next release.

That's Ok. Thanks very much for your reviewing.

Best Regards
Feifei
> 
> 
> 
> >
> >
> > Best Regards
> > Feifei
> >
> > > -Original Message-
> > > From: Feifei Wang 
> > > Sent: Monday, May 10, 2021 2:12 PM
> > > To: jer...@marvell.com
> > > Cc: dev@dpdk.org; nd ; Feifei Wang
> > > ; Ruifeng Wang ;
> > > Honnappa Nagarahalli 
> > > Subject: [PATCH] app/eventdev: remove unnecessary barrier for order
> > > test
> > >
> > > For "order_launch_lcores" function, wmb after that the main lcore
> > > updates the variable "t->err", which represents the end of the test
> > > signal, is unnecessary. Because after the main lcore updates this
> > > siginal variable, it will jump out of the launch function loop, and
> > > wait other lcores stop or return error in the main function(evt_main.c).
> > > During this time, there is no storing operation and thus no need for wmb.
> > >
> > > Signed-off-by: Feifei Wang 
> > > Reviewed-by: Ruifeng Wang 
> > > Reviewed-by: Honnappa Nagarahalli 
> > > ---
> > >  app/test-eventdev/test_order_common.c | 1 -
> > >  1 file changed, 1 deletion(-)
> > >
> > > diff --git a/app/test-eventdev/test_order_common.c b/app/test-
> > > eventdev/test_order_common.c index 04456d56db..d7760061ba 100644
> > > --- a/app/test-eventdev/test_order_common.c
> > > +++ b/app/test-eventdev/test_order_common.c
> > > @@ -308,7 +308,6 @@ order_launch_lcores(struct evt_test *test,
> > > struct evt_options *opt,
> > >   rte_event_dev_dump(opt->dev_id, stdout);
> > >   evt_err("No schedules for seconds,
> > > deadlock");
> > >   t->err = true;
> > > - rte_smp_wmb();
> > >   break;
> > >   }
> > >   old_remaining = remaining;
> > > --
> > > 2.25.1
> > >
> >

[dpdk-dev] [PATCH v1] net/i40e: fix flow director does not work

[Steve Yang]

When user configured the flow rule with raw packet via command
"flow_director_filter", it would reset all previous fdir input set
flags with "i40e_flow_set_fdir_inset()".

Ignore to configure the flow input set with raw packet rule used.

Fixes: ff04964ea6d5 ("net/i40e: fix flow director for common pctypes")

Signed-off-by: Steve Yang 
---
 drivers/net/i40e/i40e_fdir.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
index ac0e09bfdd..3c7cf1ba90 100644
--- a/drivers/net/i40e/i40e_fdir.c
+++ b/drivers/net/i40e/i40e_fdir.c
@@ -1768,7 +1768,8 @@ i40e_flow_add_del_fdir_filter(struct rte_eth_dev *dev,
 
if (add) {
/* configure the input set for common PCTYPEs*/
-   if (!filter->input.flow_ext.customized_pctype) {
+   if (!filter->input.flow_ext.customized_pctype &&
+   !filter->input.flow_ext.pkt_template) {
ret = i40e_flow_set_fdir_inset(pf, pctype,
filter->input.flow_ext.input_set);
if (ret < 0)
-- 
2.27.0

Re: [dpdk-dev] [dpdk-announce] release candidate 21.05-rc3

[Thinh Tran]

DPDK 21.05 RC3 on IBM Power result

* DPDK: v21.05-rc3-0-g7989b7e7d
* OS:   RHEL 8.3 kernel level: 4.18.0-240.el8.ppc64le
* GCC:  version 8.3.1 20191121 (Red Hat 8.3.1-5)
* Basic PF on Mellanox: No new issues or regressions were seen.
* Performance: not tested.

System tested:
- IBM Power9 Model 8335-101 CPU: 2.2 (pvr 004e 1202)
Tested NICs:
- Mellanox Technologies MT28800 Family [ConnectX-5 Ex]
- MLNX_OFED_LINUX-5.2-1.0.4.1
- Firmware level: 16.29.1017

Regards,
Thinh Tran

On 5/12/2021 3:57 PM, Thomas Monjalon wrote:

A new DPDK release candidate is ready for testing:
https://git.dpdk.org/dpdk/tag/?id=v21.05-rc3

There are 114 new patches in this snapshot.

Release notes:
https://doc.dpdk.org/guides/rel_notes/release_21_05.html

Please test and report issues on bugs.dpdk.org.
You may share some release validation results
by replying to this message at dev@dpdk.org.

DPDK 21.05-rc4 will be out during next week for the final touch.

Thank you everyone

[dpdk-dev] DTS Workgroup: MoM 05/12/2021

[Honnappa Nagarahalli]

Attendees:
Aaron Conole
Daniel Martin Buckley
Honnappa Nagarahalli
Thomas Monjalon
Lijuan Tu
Juraj Linkes
Lincoln Lavoie
Owen Hilyard

The meeting announcements are sent to dev@dpdk.org.

Minutes:

1) Action items from the meeting held on 4/28
a) Juraj Linkes - Understand and discuss the patches that DTS applies 
on DPDK in the next call.
app/test-pmd/macfwd.c - a printf statement, can be provided by 
test-pmd 'show fdwstats' command.
  fm10K - Might not be used anymore
  ABI patch is not used in DTS - Documentation needs to be 
updated.
Many of the test cases use sed scripts to modify the code. 
These need to be explored further.

2) Agreed to create two lists of test cases - i) reviewed by DTS working group 
ii) non-reviewed test cases. This will help in introducing the test cases in 
stages without having to review the entire list.

3) Agreed to set up the meeting for the next 4 weeks.

4) Action Items
Honnappa - Validate test-pmd to make sure the mac-fwd printf statement output 
is available in 'show fwd stats'
Lijuan Tu - Check if the fm10K test case is still valid
Juraj Linkes - Explore the code modified using sed commands in various test 
cases.

5) The work item related discussions are captured in [1]


[1] 
https://docs.google.com/document/d/1c5S0_mZzFvzZfYkqyORLT2-qNvUb-fBdjA6DGusy4yM/edit?usp=sharing

[dpdk-dev] [PATCH] eal/hotplug: suppress one error log on primary process

[Changpeng Liu]

This is a normal case that the primary process already
owned one PCI device while the secondary process try to
attach it, so suppress the error log here to exclude
this case.

Signed-off-by: Changpeng Liu 
---
 lib/eal/common/hotplug_mp.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/lib/eal/common/hotplug_mp.c b/lib/eal/common/hotplug_mp.c
index ae6010e..bf55056 100644
--- a/lib/eal/common/hotplug_mp.c
+++ b/lib/eal/common/hotplug_mp.c
@@ -98,10 +98,9 @@ static int cmp_dev_name(const struct rte_device *dev, const 
void *_name)
memset(&da, 0, sizeof(da));
if (req->t == EAL_DEV_REQ_TYPE_ATTACH) {
ret = local_dev_probe(req->devargs, &dev);
-   if (ret != 0) {
+   if (ret != 0 && ret != -EEXIST) {
RTE_LOG(ERR, EAL, "Failed to hotplug add device on 
primary\n");
-   if (ret != -EEXIST)
-   goto finish;
+   goto finish;
}
ret = eal_dev_hotplug_request_to_secondary(&tmp_req);
if (ret != 0) {
-- 
1.9.3

Re: [dpdk-dev] [PATCH] examples/l3fwd-power: fix assignment errors

[Thomas Monjalon]

13/05/2021 11:47, David Hunt:
> On 13/5/2021 10:09 AM, Pattan, Reshma wrote:
> >> 21/04/2021 05:53, Min Hu (Connor):
> >>> From: HongBo Zheng 
> >>>
> >>> +   ep_hgh_edpi = EMPTY_POLL_HGH_THRESHOLD;
> > 
> >>> +   if (hgh_edpi > 0)
> >>>  ep_hgh_edpi = hgh_edpi;
> >>>
> > Changes looks good to me.
> >
> > Reviewed-by: Reshma Pattan 
> >
> >
> > Thanks,
> > Reshma
> 
> 
> +1
> 
> Acked-by: David Hunt 

Applied, thanks