Re: [dpdk-dev] [PATCH v4] vhost: add config change slave msg support
On Thu, Dec 19, 2019 at 04:54:06PM +0800, Li Feng wrote:
> This msg is used to notify qemu that should get the config of backend.
>
> For example, vhost-user-blk uses this msg to notify guest os the
> capacity of backend has changed.
>
> The need_reply flag is not mandatory because it will block the sender
> thread and master process will send get_config message to fetch the
> configuration, this need an extra thread to process the vhost message.
>
> Signed-off-by: Li Feng
> ---
> v4:
> * Fix type and code style.
> * Add need_reply support.
>
> v3:
> * Move the declare to rte_vhost.h
> * Add the symbol in rte_vhost_version.map
>
> v2:
> * Fix a little log typo.
>
> lib/librte_vhost/rte_vhost.h | 15 +++
> lib/librte_vhost/rte_vhost_version.map | 1 +
> lib/librte_vhost/vhost_user.c | 35
> ++
> lib/librte_vhost/vhost_user.h | 1 +
> 4 files changed, 52 insertions(+)
>
> diff --git a/lib/librte_vhost/rte_vhost.h b/lib/librte_vhost/rte_vhost.h
> index 7b5dc87c2..c7b619ae0 100644
> --- a/lib/librte_vhost/rte_vhost.h
> +++ b/lib/librte_vhost/rte_vhost.h
> @@ -10,6 +10,7 @@
> * Interface to vhost-user
> */
>
> +#include
> #include
> #include
>
> @@ -977,6 +978,20 @@ __rte_experimental
> int
> rte_vhost_get_vdpa_device_id(int vid);
>
> +/**
> + * Notify the guest that should get virtio configuration space from backend.
> + *
> + * @param vid
> + * vhost device ID
> + * @param need_reply
> + * wait for the master response the status of this operation
> + * @return
> + * 0 on success, < 0 on failure
> + */
> +__rte_experimental
> +int
> +rte_vhost_slave_config_change(int vid, bool need_reply);
> +
> #ifdef __cplusplus
> }
> #endif
> diff --git a/lib/librte_vhost/rte_vhost_version.map
> b/lib/librte_vhost/rte_vhost_version.map
> index c512377fe..051d08c12 100644
> --- a/lib/librte_vhost/rte_vhost_version.map
> +++ b/lib/librte_vhost/rte_vhost_version.map
> @@ -65,4 +65,5 @@ EXPERIMENTAL {
> rte_vhost_clr_inflight_desc_packed;
> rte_vhost_get_vhost_ring_inflight;
> rte_vhost_get_vring_base_from_inflight;
> + rte_vhost_slave_config_change;
> };
> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
> index 0cfb8b792..aed1210b1 100644
> --- a/lib/librte_vhost/vhost_user.c
> +++ b/lib/librte_vhost/vhost_user.c
> @@ -2840,6 +2840,41 @@ vhost_user_iotlb_miss(struct virtio_net *dev, uint64_t
> iova, uint8_t perm)
> return 0;
> }
>
> +static int
> +vhost_user_slave_config_change(struct virtio_net *dev, bool need_reply)
> +{
> + int ret;
> + uint32_t flags = VHOST_USER_VERSION;
> + if (need_reply)
> + flags |= VHOST_USER_NEED_REPLY;
> + struct VhostUserMsg msg = {
> + .request.slave = VHOST_USER_SLAVE_CONFIG_CHANGE_MSG,
> + .flags = flags,
> + .size = 0,
> + };
Normally, we don't mix the declarations and code.
Something like this looks better to me:
struct VhostUserMsg msg = {
.request.slave = VHOST_USER_SLAVE_CONFIG_CHANGE_MSG,
.flags = VHOST_USER_VERSION,
};
if (need_reply)
msg.flags |= VHOST_USER_NEED_REPLY;
> +
> + ret = send_vhost_slave_message(dev, &msg);
> + if (ret < 0) {
> + RTE_LOG(ERR, VHOST_CONFIG,
> + "Failed to send config change (%d)\n",
> + ret);
> + return ret;
> + }
> + if (need_reply)
> + return process_slave_message_reply(dev, &msg);
We can call process_slave_message_reply() unconditionally.
https://github.com/DPDK/dpdk/blob/f26c2b39b271/lib/librte_vhost/vhost_user.c#L2794-L2795
> + return 0;
> +}
> +
> +int
> +rte_vhost_slave_config_change(int vid, bool need_reply)
> +{
> + struct virtio_net *dev;
Looks better to add an empty line here.
> + dev = get_device(vid);
> + if (!dev)
> + return -ENODEV;
> + return vhost_user_slave_config_change(dev, need_reply);
> +}
> +
> static int vhost_user_slave_set_vring_host_notifier(struct virtio_net *dev,
> int index, int fd,
> uint64_t offset,
> diff --git a/lib/librte_vhost/vhost_user.h b/lib/librte_vhost/vhost_user.h
> index 6563f7315..86c364a93 100644
> --- a/lib/librte_vhost/vhost_user.h
> +++ b/lib/librte_vhost/vhost_user.h
> @@ -62,6 +62,7 @@ typedef enum VhostUserRequest {
> typedef enum VhostUserSlaveRequest {
> VHOST_USER_SLAVE_NONE = 0,
> VHOST_USER_SLAVE_IOTLB_MSG = 1,
> + VHOST_USER_SLAVE_CONFIG_CHANGE_MSG = 2,
> VHOST_USER_SLAVE_VRING_HOST_NOTIFIER_MSG = 3,
> VHOST_USER_SLAVE_MAX
> } VhostUserSlaveRequest;
> --
> 2.11.0
>
>
> --
> The SmartX email address is only for business purpose. Any sent message
> that is not related to the business is not authorized or permitted by
> SmartX.
> 本邮箱为北京志凌海
Re: [dpdk-dev] [PATCH v2 2/3] net/virtio: virtual PCI requires smp barriers
On Fri, Dec 20, 2019 at 11:09:50AM +0800, Gavin Hu wrote:
> Other than real PCI reads and writes to the device memory requiring
> the io barriers, virtual pci memories are normal memory in the smp
> configuration, which requires the smp barriers.
>
> Since the smp barriers and io barriers are identical on x86 and PPC,
> this change has only effect on aarch64.
>
> As far as peripheral coherence order for ‘virtual’ devices, the arch
> intent is that the Hypervisor view of things takes precedence – since
> translations are made in holistic manner as the full stage1+stage2
> regime, there is no such thing as a transaction taking on “EL1”
> mapping as far as ordering. If the Hypervisor maps stage2 as Normal
> but the OS at EL1 maps it as Device-nGnRE, then it’s Normal memory and
> follows the ordering rules for Normal memory.
>
> Signed-off-by: Gavin Hu
> ---
> drivers/net/virtio/virtio_pci.c | 108
> +---
> 1 file changed, 78 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/net/virtio/virtio_pci.c b/drivers/net/virtio/virtio_pci.c
> index 4468e89..64aa0a0 100644
> --- a/drivers/net/virtio/virtio_pci.c
> +++ b/drivers/net/virtio/virtio_pci.c
> @@ -24,6 +24,54 @@
> #define PCI_CAP_ID_VNDR 0x09
> #define PCI_CAP_ID_MSIX 0x11
>
> +static __rte_always_inline uint8_t
> +virtio_pci_read8(const volatile void *addr)
> +{
> + uint8_t val;
> + val = rte_read8_relaxed(addr);
> + rte_smp_rmb();
> + return val;
> +}
> +
> +static __rte_always_inline uint16_t
> +virtio_pci_read16(const volatile void *addr)
> +{
> + uint16_t val;
> + val = rte_read16_relaxed(addr);
> + rte_smp_rmb();
> + return val;
> +}
> +
> +static __rte_always_inline uint32_t
> +virtio_pci_read32(const volatile void *addr)
> +{
> + uint32_t val;
> + val = rte_read32_relaxed(addr);
> + rte_smp_rmb();
> + return val;
> +}
> +
> +static __rte_always_inline void
> +virtio_pci_write8(uint8_t value, volatile void *addr)
> +{
> + rte_smp_wmb();
> + rte_write8_relaxed(value, addr);
> +}
> +
> +static __rte_always_inline void
> +virtio_pci_write16(uint16_t value, volatile void *addr)
> +{
> + rte_smp_wmb();
> + rte_write16_relaxed(value, addr);
> +}
> +
> +static __rte_always_inline void
> +virtio_pci_write32(uint32_t value, volatile void *addr)
> +{
> + rte_smp_wmb();
> + rte_write32_relaxed(value, addr);
> +}
We can't assume that virtio device is software running
in an SMP configuration unless VIRTIO_F_ORDER_PLATFORM
isn't negotiated.
https://github.com/oasis-tcs/virtio-spec/blob/94520b3af19c/content.tex#L5788
> +
[dpdk-dev] [PATCH v5] vhost: add config change slave msg support
This msg is used to notify qemu that should get the config of backend.
For example, vhost-user-blk uses this msg to notify guest os the
capacity of backend has changed.
The need_reply flag is not mandatory because it will block the sender
thread and master process will send get_config message to fetch the
configuration, this need an extra thread to process the vhost message.
Signed-off-by: Li Feng
---
v5:
* Fix code style.
v4:
* Fix type and code style.
* Add need_reply support.
v3:
* Move the declare to rte_vhost.h
* Add the symbol in rte_vhost_version.map
v2:
* Fix a little log typo.
lib/librte_vhost/rte_vhost.h | 15 ++
lib/librte_vhost/rte_vhost_version.map | 1 +
lib/librte_vhost/vhost_user.c | 36 ++
lib/librte_vhost/vhost_user.h | 1 +
4 files changed, 53 insertions(+)
diff --git a/lib/librte_vhost/rte_vhost.h b/lib/librte_vhost/rte_vhost.h
index 7b5dc87c2..c7b619ae0 100644
--- a/lib/librte_vhost/rte_vhost.h
+++ b/lib/librte_vhost/rte_vhost.h
@@ -10,6 +10,7 @@
* Interface to vhost-user
*/
+#include
#include
#include
@@ -977,6 +978,20 @@ __rte_experimental
int
rte_vhost_get_vdpa_device_id(int vid);
+/**
+ * Notify the guest that should get virtio configuration space from backend.
+ *
+ * @param vid
+ * vhost device ID
+ * @param need_reply
+ * wait for the master response the status of this operation
+ * @return
+ * 0 on success, < 0 on failure
+ */
+__rte_experimental
+int
+rte_vhost_slave_config_change(int vid, bool need_reply);
+
#ifdef __cplusplus
}
#endif
diff --git a/lib/librte_vhost/rte_vhost_version.map
b/lib/librte_vhost/rte_vhost_version.map
index c512377fe..051d08c12 100644
--- a/lib/librte_vhost/rte_vhost_version.map
+++ b/lib/librte_vhost/rte_vhost_version.map
@@ -65,4 +65,5 @@ EXPERIMENTAL {
rte_vhost_clr_inflight_desc_packed;
rte_vhost_get_vhost_ring_inflight;
rte_vhost_get_vring_base_from_inflight;
+ rte_vhost_slave_config_change;
};
diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 0cfb8b792..defd82a19 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -2840,6 +2840,42 @@ vhost_user_iotlb_miss(struct virtio_net *dev, uint64_t
iova, uint8_t perm)
return 0;
}
+static int
+vhost_user_slave_config_change(struct virtio_net *dev, bool need_reply)
+{
+ int ret;
+ struct VhostUserMsg msg = {
+ .request.slave = VHOST_USER_SLAVE_CONFIG_CHANGE_MSG,
+ .flags = VHOST_USER_VERSION,
+ .size = 0,
+ };
+
+ if (need_reply)
+ msg.flags |= VHOST_USER_NEED_REPLY;
+
+ ret = send_vhost_slave_message(dev, &msg);
+ if (ret < 0) {
+ RTE_LOG(ERR, VHOST_CONFIG,
+ "Failed to send config change (%d)\n",
+ ret);
+ return ret;
+ }
+
+ return process_slave_message_reply(dev, &msg);
+}
+
+int
+rte_vhost_slave_config_change(int vid, bool need_reply)
+{
+ struct virtio_net *dev;
+
+ dev = get_device(vid);
+ if (!dev)
+ return -ENODEV;
+
+ return vhost_user_slave_config_change(dev, need_reply);
+}
+
static int vhost_user_slave_set_vring_host_notifier(struct virtio_net *dev,
int index, int fd,
uint64_t offset,
diff --git a/lib/librte_vhost/vhost_user.h b/lib/librte_vhost/vhost_user.h
index 6563f7315..86c364a93 100644
--- a/lib/librte_vhost/vhost_user.h
+++ b/lib/librte_vhost/vhost_user.h
@@ -62,6 +62,7 @@ typedef enum VhostUserRequest {
typedef enum VhostUserSlaveRequest {
VHOST_USER_SLAVE_NONE = 0,
VHOST_USER_SLAVE_IOTLB_MSG = 1,
+ VHOST_USER_SLAVE_CONFIG_CHANGE_MSG = 2,
VHOST_USER_SLAVE_VRING_HOST_NOTIFIER_MSG = 3,
VHOST_USER_SLAVE_MAX
} VhostUserSlaveRequest;
--
2.11.0
--
The SmartX email address is only for business purpose. Any sent message
that is not related to the business is not authorized or permitted by
SmartX.
本邮箱为北京志凌海纳科技有限公司（SmartX）工作邮箱. 如本邮箱发出的邮件与工作无关,该邮件未得到本公司任何的明示或默示的授权.
Re: [dpdk-dev] [PATCH v1] net/ice: add new device IDs
On 12/20/2019 7:01 AM, Xu, Ting wrote:
>
>
>> -Original Message-
>> From: Yigit, Ferruh
>> Sent: Thursday, December 19, 2019 8:14 PM
>> To: Ye, Xiaolong ; Xu, Ting
>> Cc: dev@dpdk.org; Lu, Wenzhuo ; Yang, Qiming
>> ; Zhang, Qi Z
>> Subject: Re: [dpdk-dev] [PATCH v1] net/ice: add new device IDs
>>
>> On 12/19/2019 2:43 AM, Ye Xiaolong wrote:
>>> On 12/18, Ting Xu wrote:
This patch added new device IDs for C822N.
Signed-off-by: Ting Xu
---
drivers/net/ice/ice_ethdev.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ice/ice_ethdev.c
b/drivers/net/ice/ice_ethdev.c index de189daba..2cbd82c94 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -163,6 +163,9 @@ static const struct rte_pci_id pci_id_ice_map[] = {
{ RTE_PCI_DEVICE(ICE_INTEL_VENDOR_ID,
>> ICE_DEV_ID_E810_XXV_BACKPLANE) },
{ RTE_PCI_DEVICE(ICE_INTEL_VENDOR_ID,
>> ICE_DEV_ID_E810_XXV_QSFP) },
{ RTE_PCI_DEVICE(ICE_INTEL_VENDOR_ID,
>> ICE_DEV_ID_E810_XXV_SFP) },
+ { RTE_PCI_DEVICE(ICE_INTEL_VENDOR_ID,
>> ICE_DEV_ID_C822N_BACKPLANE) },
+ { RTE_PCI_DEVICE(ICE_INTEL_VENDOR_ID, ICE_DEV_ID_C822N_QSFP) },
+ { RTE_PCI_DEVICE(ICE_INTEL_VENDOR_ID, ICE_DEV_ID_C822N_SFP) },
{ .vendor_id = 0, /* sentinel */ },
};
--
2.17.1
>>>
>>> Acked-by: Xiaolong Ye
>>>
>>> Applied to dpdk-next-net-intel, Thanks.
>>>
>>
>> Just to double check, if this requires any update in the NIC documentation?
>
> Hi, Ferruh, I will update docs for ice supported NICs to add C822 later.
>
Thanks.
>
> And is it necessary to update the 20.02 release note as well?
>
Optional, up to you, but why not put a brief single line to mention from it.
[dpdk-dev] [PATCH v2 1/2] ci: add travis ci support for aarch64
Add Travis compilation jobs for aarch64. gcc/clang compilations for
static/shared libraries are added.
Some limitations for current aarch64 Travis support:
1. Container is used. Huge page is not available due to security reason.
2. Missing kernel header package in Xenial distribution.
Solutions to address the limitations:
1. Not to add unit test for now. And run tests with no-huge in future.
2. Use Bionic distribution for all aarch64 jobs.
Signed-off-by: Ruifeng Wang
Reviewed-by: Gavin Hu
---
.ci/linux-setup.sh | 11 +++
.travis.yml| 37 -
2 files changed, 43 insertions(+), 5 deletions(-)
diff --git a/.ci/linux-setup.sh b/.ci/linux-setup.sh
index dfb9d4a20..a92978037 100755
--- a/.ci/linux-setup.sh
+++ b/.ci/linux-setup.sh
@@ -3,7 +3,10 @@
# need to install as 'root' since some of the unit tests won't run without it
sudo python3 -m pip install --upgrade meson
-# setup hugepages
-cat /proc/meminfo
-sudo sh -c 'echo 1024 > /proc/sys/vm/nr_hugepages'
-cat /proc/meminfo
+# hugepage settings are skipped on aarch64 due to environment limitation
+if [ "$TRAVIS_ARCH" != "aarch64" ]; then
+# setup hugepages
+cat /proc/meminfo
+sudo sh -c 'echo 1024 > /proc/sys/vm/nr_hugepages'
+cat /proc/meminfo
+fi
diff --git a/.travis.yml b/.travis.yml
index 8f90d06f2..048cb6ba5 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -115,6 +115,41 @@ matrix:
apt:
packages:
- *extra_packages
-
+ - env: DEF_LIB="static"
+arch: arm64
+compiler: gcc
+addons:
+ apt:
+packages:
+ - *required_packages
+ - env: DEF_LIB="shared"
+arch: arm64
+compiler: gcc
+addons:
+ apt:
+packages:
+ - *required_packages
+ - env: DEF_LIB="static"
+arch: arm64
+compiler: clang
+addons:
+ apt:
+packages:
+ - *required_packages
+ - env: DEF_LIB="shared"
+arch: arm64
+compiler: clang
+addons:
+ apt:
+packages:
+ - *required_packages
+ - env: DEF_LIB="shared" OPTS="-Denable_kmods=false" BUILD_DOCS=1
+arch: arm64
+compiler: gcc
+addons:
+ apt:
+packages:
+ - *required_packages
+ - *doc_packages
script: ./.ci/${TRAVIS_OS_NAME}-build.sh
--
2.17.1
[dpdk-dev] [PATCH v2 0/2] add travis ci support for aarch64
This patch set is to enable native aarch64 build in Travis CI. It leverages Travis CI multi arch support. As the first step, compilation jobs are added. Unit test is not added for now due to service limitation. We are planning to run unit test with no-huge in future. This patch set has dependency on: http://patches.dpdk.org/patch/63969/ v2: Removed dist designation from matrix since base dist is changing. Add explanation for library path issue. Ruifeng Wang (2): ci: add travis ci support for aarch64 devtools: add path to additional shared object files .ci/linux-setup.sh| 11 +++ .travis.yml | 37 - devtools/test-null.sh | 2 +- 3 files changed, 44 insertions(+), 6 deletions(-) -- 2.17.1
[dpdk-dev] [PATCH v2 2/2] devtools: add path to additional shared object files
Drivers librte_mempool_ring.so and librte_pmd_null.so are loaded by librte_eal.so when running testpmd. In Ubuntu Xenial, driver path is installed to RPATH on testpmd. This allows librte_eal.so to find drivers by using the RPATH. However, in Ubuntu Bionic, driver path is installed to RUNPATH instead. The RUNPATH on testpmd is not available by librte_eal.so and therefore lead to driver load failure: EAL: Detected 32 lcore(s) EAL: Detected 1 NUMA nodes EAL: librte_mempool_ring.so: cannot open shared object file: No such file or directory EAL: FATAL: Cannot init plugins EAL: Cannot init plugins Add 'drivers' into LD_LIBRARY_PATH so that testpmd can find and make use of these shared libraries. Signed-off-by: Ruifeng Wang Reviewed-by: Gavin Hu --- devtools/test-null.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/devtools/test-null.sh b/devtools/test-null.sh index f39af2c06..548de8113 100755 --- a/devtools/test-null.sh +++ b/devtools/test-null.sh @@ -20,7 +20,7 @@ if [ ! -f "$testpmd" ] ; then fi if ldd $testpmd | grep -q librte_ ; then - export LD_LIBRARY_PATH=$build/lib:$LD_LIBRARY_PATH + export LD_LIBRARY_PATH=$build/drivers:$build/lib:$LD_LIBRARY_PATH libs='-d librte_mempool_ring.so -d librte_pmd_null.so' else libs= -- 2.17.1
Re: [dpdk-dev] [PATCH v2 2/3] net/virtio: virtual PCI requires smp barriers
Hi Tiwei,
Thanks for review.
> -Original Message-
> From: Tiwei Bie
> Sent: Friday, December 20, 2019 4:18 PM
> To: Gavin Hu
> Cc: dev@dpdk.org; nd ; david.march...@redhat.com;
> tho...@monjalon.net; rasl...@mellanox.com;
> maxime.coque...@redhat.com; hemant.agra...@nxp.com;
> jer...@marvell.com; pbhagavat...@marvell.com; Honnappa Nagarahalli
> ; Ruifeng Wang
> ; Phil Yang ; Joyce Kong
> ; Steve Capper
> Subject: Re: [PATCH v2 2/3] net/virtio: virtual PCI requires smp barriers
>
> On Fri, Dec 20, 2019 at 11:09:50AM +0800, Gavin Hu wrote:
> > Other than real PCI reads and writes to the device memory requiring
> > the io barriers, virtual pci memories are normal memory in the smp
> > configuration, which requires the smp barriers.
> >
> > Since the smp barriers and io barriers are identical on x86 and PPC,
> > this change has only effect on aarch64.
> >
> > As far as peripheral coherence order for ‘virtual’ devices, the arch
> > intent is that the Hypervisor view of things takes precedence – since
> > translations are made in holistic manner as the full stage1+stage2
> > regime, there is no such thing as a transaction taking on “EL1”
> > mapping as far as ordering. If the Hypervisor maps stage2 as Normal
> > but the OS at EL1 maps it as Device-nGnRE, then it’s Normal memory and
> > follows the ordering rules for Normal memory.
> >
> > Signed-off-by: Gavin Hu
> > ---
> > drivers/net/virtio/virtio_pci.c | 108 +---
>
> > 1 file changed, 78 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/net/virtio/virtio_pci.c
> > b/drivers/net/virtio/virtio_pci.c
> > index 4468e89..64aa0a0 100644
> > --- a/drivers/net/virtio/virtio_pci.c
> > +++ b/drivers/net/virtio/virtio_pci.c
> > @@ -24,6 +24,54 @@
> > #define PCI_CAP_ID_VNDR0x09
> > #define PCI_CAP_ID_MSIX0x11
> >
> > +static __rte_always_inline uint8_t
> > +virtio_pci_read8(const volatile void *addr)
> > +{
> > + uint8_t val;
> > + val = rte_read8_relaxed(addr);
> > + rte_smp_rmb();
> > + return val;
> > +}
> > +
> > +static __rte_always_inline uint16_t
> > +virtio_pci_read16(const volatile void *addr)
> > +{
> > + uint16_t val;
> > + val = rte_read16_relaxed(addr);
> > + rte_smp_rmb();
> > + return val;
> > +}
> > +
> > +static __rte_always_inline uint32_t
> > +virtio_pci_read32(const volatile void *addr)
> > +{
> > + uint32_t val;
> > + val = rte_read32_relaxed(addr);
> > + rte_smp_rmb();
> > + return val;
> > +}
> > +
> > +static __rte_always_inline void
> > +virtio_pci_write8(uint8_t value, volatile void *addr)
> > +{
> > + rte_smp_wmb();
> > + rte_write8_relaxed(value, addr);
> > +}
> > +
> > +static __rte_always_inline void
> > +virtio_pci_write16(uint16_t value, volatile void *addr)
> > +{
> > + rte_smp_wmb();
> > + rte_write16_relaxed(value, addr);
> > +}
> > +
> > +static __rte_always_inline void
> > +virtio_pci_write32(uint32_t value, volatile void *addr)
> > +{
> > + rte_smp_wmb();
> > + rte_write32_relaxed(value, addr);
> > +}
>
> We can't assume that virtio device is software running
> in an SMP configuration unless VIRTIO_F_ORDER_PLATFORM
> isn't negotiated.
That's true, rte_smp is sufficient for *non*- VIRTIO_F_ORDER_PLATFORM,
As in the previous patch, rte_io is relaxed to a compiler barrier, rte_smp is
stronger than that, put another way, rte_smp is also sufficient for *
VIRTIO_F_ORDER_PLATFORM* case.
As X86 and PPC make no difference about rte_smp and rte_io, so everything goes
fine?
Anyway, I will update the commit log to reflect the above points.
>
> https://github.com/oasis-tcs/virtio-
> spec/blob/94520b3af19c/content.tex#L5788
>
> > +
Re: [dpdk-dev] [PATCH v6 00/11] Add ABI compatibility checks to the meson build
19/12/2019 22:58, David Marchand: > On Fri, Dec 13, 2019 at 5:41 PM Kevin Laatz wrote: > > The proposed solution works as follows: > > 1. Generate the ABI dump of the baseline. This can be done with the new > >script added in this set. This step will only need to be done when the > >ABI version changes (so once a year) and can be added to master so it > >exists by default. This step can be skipped if the dump files for the > >baseline already exist. > > 2. Build with meson. If there is an ABI incompatibility, the build will > >fail and print the incompatibility information. [...] > - I asked for the dump files, but I can see that it is impractical. > > The dump files are huge. I did not expect that :-). > The dump files are architecture specific and maintaining multi arch > dumps would be even bigger than just what you sent for x86_64. > (not even considering the changes in ABI if some configuration items > have an impact...). I agree that dump files are better managed outside of git. > As you pointed out, people who don't have all dependencies won't > create/update those dump files. > Dealing with ABI updates (thinking of bumping the ABI version) is > likely a maintainer job, but it will be a source of issues and we > (maintainers) might miss some updates especially for drivers we can't > build. > > > - Why do we restrict the checks to meson? > The make build framework is going to disappear ok, but we can't leave > it untested. > People still rely on it. > > Checking the ABI is orthogonal to building DPDK. > Dumping the ABI and checking it against objects can be done externally. I agree we should not rely on meson for running the check. > - For those reasons, I have been trying an alternate approach [1]: in > Travis, generate a reference dump based on the first ancestor tag, > then build the proposed patches. > All contributions get picked up by Aaron robot and would have to pass > through this check. > > As an exercise, I tried to integrate Eelco patch [2], that moves > symbols from EXPERIMENTAL to a stable ABI. The check passes fine. > I also tried to bump the ABI major version. The check fails, as > expected, but I prepared a way to bypass such failures for the > releases where we will explicitely break ABI. > > For maintainers that integrate patches or developers that get a CI > failure and want to fix it, we would need to help them to: > * generate dumps on a reference version, so I would tend to write some > documentation since playing with the current sources would be too > dangerous from my pov, > * run the checks, like adding the check in the > devtools/test-*-build.sh scripts that already exist, with a new > configuration item to point at the dumps per target, We can start with a documented process, and write a separate script later if we feel it helps. > Those last two points are still to be done. > > WDYT? > > > 1: > https://github.com/david-marchand/dpdk/commit/f18de2ec157f3cc1e7b319cb19700e1b5e9cecde > 2: https://patchwork.dpdk.org/patch/63970/ Thanks to both of you trying some approaches and making progress.
[dpdk-dev] [PATCH] net/mlx5: fix ConnectX-4LX Tx burst routines set
The tx_burst routine supporting multi-segment packets with
legacy MPW and without inline was missed, and there was no
valid selection for these options, patch adds the missing
routine.
Fixes: 82e75f8323bf ("net/mlx5: fix legacy multi-packet Tx descriptors")
Cc: sta...@dpdk.org
Signed-off-by: Viacheslav Ovsiienko
---
drivers/net/mlx5/mlx5_rxtx.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
index a7f3bff..57804f5 100644
--- a/drivers/net/mlx5/mlx5_rxtx.c
+++ b/drivers/net/mlx5/mlx5_rxtx.c
@@ -4984,6 +4984,10 @@ enum mlx5_txcmp_code {
MLX5_TXOFF_CONFIG_INLINE | MLX5_TXOFF_CONFIG_EMPW |
MLX5_TXOFF_CONFIG_MPW)
+MLX5_TXOFF_DECL(mc_mpw,
+ MLX5_TXOFF_CONFIG_MULTI | MLX5_TXOFF_CONFIG_CSUM |
+ MLX5_TXOFF_CONFIG_EMPW | MLX5_TXOFF_CONFIG_MPW)
+
MLX5_TXOFF_DECL(i_mpw,
MLX5_TXOFF_CONFIG_INLINE | MLX5_TXOFF_CONFIG_EMPW |
MLX5_TXOFF_CONFIG_MPW)
@@ -5140,6 +5144,10 @@ enum mlx5_txcmp_code {
MLX5_TXOFF_CONFIG_INLINE | MLX5_TXOFF_CONFIG_EMPW |
MLX5_TXOFF_CONFIG_MPW)
+MLX5_TXOFF_INFO(mc_mpw,
+ MLX5_TXOFF_CONFIG_MULTI | MLX5_TXOFF_CONFIG_CSUM |
+ MLX5_TXOFF_CONFIG_EMPW | MLX5_TXOFF_CONFIG_MPW)
+
MLX5_TXOFF_INFO(i_mpw,
MLX5_TXOFF_CONFIG_INLINE | MLX5_TXOFF_CONFIG_EMPW |
MLX5_TXOFF_CONFIG_MPW)
@@ -5297,6 +5305,7 @@ enum mlx5_txcmp_code {
DRV_LOG(DEBUG, "port %u has no selected Tx function"
" for requested offloads %04X",
dev->data->port_id, olx);
+ assert(false);
return NULL;
}
DRV_LOG(DEBUG, "port %u has selected Tx function"
--
1.8.3.1
Re: [dpdk-dev] [PATCH] mempool: fix mempool obj alignment for nonx86
> -Original Message- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Jerin Jacob > Sent: Friday, December 20, 2019 4:45 AM > > On Fri, Dec 20, 2019 at 8:56 AM Gavin Hu wrote: > > > > Hi Jerin, > > > > It got two coding style warnings, otherwise, > > Reviewed-by: Gavin Hu > > Thanks Gavin for review. Since the existing code is using "unsigned" > in that file, I thought of not change by this patch. > If someone thinks, It is better to change then I can send v2 by fixing > "unsigned" to "unsigned int" across the file as a first patch in the > series. > The use of the type "unsigned" is a general issue with older DPDK libraries. Anyone touching any of these libraries will get these warnings. They should all be fixed. I'm not saying you should do it; I'm only suggesting that someone should create a dedicated patch to fix them all. > > > > WARNING:UNSPECIFIED_INT: Prefer 'unsigned int' to bare use of > 'unsigned' > > #144: FILE: lib/librte_mempool/rte_mempool.c:84: > > +arch_mem_object_align(unsigned obj_size) > > > > WARNING:UNSPECIFIED_INT: Prefer 'unsigned int' to bare use of > 'unsigned' > > #154: FILE: lib/librte_mempool/rte_mempool.c:106: > > +arch_mem_object_align(unsigned obj_size)
Re: [dpdk-dev] [PATCH v6 00/11] Add ABI compatibility checks to the meson build
On Thu, Dec 19, 2019 at 10:58:35PM +0100, David Marchand wrote: > Hello Kevin, > > On Fri, Dec 13, 2019 at 5:41 PM Kevin Laatz wrote: > > > > With the recent changes made to stabilize ABI versioning in DPDK, it will > > become increasingly important to check patches for ABI compatibility. We > > propose adding the ABI compatibility checking to be done as part of the > > build. > > > > The advantages to adding the ABI compatibility checking to the build are > > two-fold. Firstly, developers can easily check their patches to make sure > > they don’t break the ABI without adding any extra steps. Secondly, it > > makes the integration into existing CI seamless since there are no extra > > scripts to make the CI run. The build will run as usual and if an > > incompatibility is detected in the ABI, the build will fail and show the > > incompatibility. As an added bonus, enabling the ABI compatibility checks > > does not impact the build speed. > > > > The proposed solution works as follows: > > 1. Generate the ABI dump of the baseline. This can be done with the new > >script added in this set. This step will only need to be done when the > >ABI version changes (so once a year) and can be added to master so it > >exists by default. This step can be skipped if the dump files for the > >baseline already exist. > > 2. Build with meson. If there is an ABI incompatibility, the build will > >fail and print the incompatibility information. > > > > The patches in this set include the ABI dump file generating script, the > > dump files for drivers and libs, the meson option required to > > enable/disable the checks, and the required meson changes to run the > > compatibility checks during the build. > > > > Note: This patch set depends on: http://patches.dpdk.org/patch/63765/. The > > generated .dump files in this patch set are based on the changes in the > > patch "build: fix soname info for 19.11 compatibility". If a decision is > > made to use a different format for the sonames, then a new version of this > > patch set will be required as the .dump files will need to be regenerated. > > > > Note: The following driver dump files are not included in these patches: > > common/mvep:missing dependency, "libmusdk" > > net/mvneta: missing dependency, "libmusdk" > > net/mvpp2: missing dependency, "libmusdk" > > net/nfb:missing dependency, "libnfb" > > crypto/mvsam: missing dependency, "libmusdk" > > > > They have not been included as I do not have access to these dependencies. > > Please feel free to add them if you can! (Maintainers of the above Cc'ed). > > - I asked for the dump files, but I can see that it is impractical. > > The dump files are huge. I did not expect that :-). Yes, they are big alright, but on the other hand, they also don't change very much (we hope!) > The dump files are architecture specific and maintaining multi arch > dumps would be even bigger than just what you sent for x86_64. > (not even considering the changes in ABI if some configuration items > have an impact...). Good point, we missed that when looking at this. > > As you pointed out, people who don't have all dependencies won't > create/update those dump files. Well, the creation should be a once-off, the comparison is what is done regularly and needs the build tools. > Dealing with ABI updates (thinking of bumping the ABI version) is > likely a maintainer job, but it will be a source of issues and we > (maintainers) might miss some updates especially for drivers we can't > build. > > > - Why do we restrict the checks to meson? > The make build framework is going to disappear ok, but we can't leave > it untested. > People still rely on it. > Because as you point out below, checking the ABI is technically orthogonal to building the DPDK, so we didn't see the payoff in adding support to two build systems as being worth the additional effort. > Checking the ABI is orthogonal to building DPDK. > Dumping the ABI and checking it against objects can be done externally. > True, but the advantage of doing so as part of each and every build is that any ABI break is caught by the original developer before he even submits his patch to the CI. As with so many things, the earlier in the process that something can be run the better it is. > > - For those reasons, I have been trying an alternate approach [1]: in > Travis, generate a reference dump based on the first ancestor tag, > then build the proposed patches. > All contributions get picked up by Aaron robot and would have to pass > through this check. Yes, the alternative to having the checks done at build time is to have them done as part of the CI, though I'd personally perfer the former. > > As an exercise, I tried to integrate Eelco patch [2], that moves > symbols from EXPERIMENTAL to a stable ABI. The check passes fine. > I also tried to bump the ABI major version. The check fails, as > expected
[dpdk-dev] [dpdk-announce] Accepted talks in FOSDEM 2020
The comittee for SDN devroom at FOSDEM is pleased to announce the schedule is available. Six talks mentioning DPDK are accepted: - Fundamental Technologies We Need to Work on for Cloud-Native Networking https://fosdem.org/2020/schedule/event/fundamental_technologies_we_need_to_work_on_for_cloud_native_networking/ - Skydive - A real time network topology and protocols analyzer https://fosdem.org/2020/schedule/event/real_time_network_topology_and_protocols_analyzer/ - Do you really see what’s happening on your NFV infrastructure? https://fosdem.org/2020/schedule/event/do_you_really_see_whats_happening_on_your_nfv_infra/ - Analyzing DPDK applications with eBPF https://fosdem.org/2020/schedule/event/analyzing_dpdk_applications_with_ebpf/ - Mixing kool-aids! Accelerate the internet with AF_XDP & DPDK https://fosdem.org/2020/schedule/event/mixing_kool_aids/ - Dial your Networking Code up to 11 https://fosdem.org/2020/schedule/event/dial_your_networking_code_up_to_11/ You will probably be interested by the other talks of the track: https://fosdem.org/2020/schedule/track/software_defined_networking/ FOSDEM welcomes a large number of devrooms during two days (Feb 1-2), allowing to learn, meet and discuss about a lot of topics. See you there!
Re: [dpdk-dev] [dpdk-announce] Accepted talks in FOSDEM 2020
+1, it is going to be _fabulous_ Ray K On 20/12/2019 11:13, Thomas Monjalon wrote: > The comittee for SDN devroom at FOSDEM is pleased to announce > the schedule is available. > > Six talks mentioning DPDK are accepted: > > - Fundamental Technologies We Need to Work on for Cloud-Native Networking > https://fosdem.org/2020/schedule/event/fundamental_technologies_we_need_to_work_on_for_cloud_native_networking/ > > - Skydive - A real time network topology and protocols analyzer > https://fosdem.org/2020/schedule/event/real_time_network_topology_and_protocols_analyzer/ > > - Do you really see what’s happening on your NFV infrastructure? > https://fosdem.org/2020/schedule/event/do_you_really_see_whats_happening_on_your_nfv_infra/ > > - Analyzing DPDK applications with eBPF > https://fosdem.org/2020/schedule/event/analyzing_dpdk_applications_with_ebpf/ > > - Mixing kool-aids! Accelerate the internet with AF_XDP & DPDK > https://fosdem.org/2020/schedule/event/mixing_kool_aids/ > > - Dial your Networking Code up to 11 > https://fosdem.org/2020/schedule/event/dial_your_networking_code_up_to_11/ > > You will probably be interested by the other talks of the track: > https://fosdem.org/2020/schedule/track/software_defined_networking/ > > FOSDEM welcomes a large number of devrooms during two days (Feb 1-2), > allowing to learn, meet and discuss about a lot of topics. > See you there! > >
[dpdk-dev] [PATCH] test/crypto: add operation status checks
This patch adds checking of the symmetric crypto operation status
that was silently skipped before. It fixes the wireless algorithms
session creation (SNOW3G, KASUMI, ZUC) and passing of the digest
data for the verification by PMD.
Signed-off-by: Adam Dybkowski
---
app/test/test_cryptodev.c | 96 +--
1 file changed, 52 insertions(+), 44 deletions(-)
diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 1b561456d..241a1f97a 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -143,7 +143,7 @@ static struct rte_crypto_op *
process_crypto_request(uint8_t dev_id, struct rte_crypto_op *op)
{
if (rte_cryptodev_enqueue_burst(dev_id, 0, &op, 1) != 1) {
- printf("Error sending packet for encryption");
+ printf("Error sending packet for encryption\n");
return NULL;
}
@@ -152,6 +152,11 @@ process_crypto_request(uint8_t dev_id, struct
rte_crypto_op *op)
while (rte_cryptodev_dequeue_burst(dev_id, 0, &op, 1) == 0)
rte_pause();
+ if (op->status != RTE_CRYPTO_OP_STATUS_SUCCESS) {
+ RTE_LOG(DEBUG, USER1, "Operation status %d\n", op->status);
+ return NULL;
+ }
+
return op;
}
@@ -2823,9 +2828,18 @@ create_wireless_algo_auth_cipher_session(uint8_t dev_id,
ut_params->sess = rte_cryptodev_sym_session_create(
ts_params->session_mpool);
- status = rte_cryptodev_sym_session_init(dev_id, ut_params->sess,
- &ut_params->auth_xform,
- ts_params->session_priv_mpool);
+ if (cipher_op == RTE_CRYPTO_CIPHER_OP_DECRYPT) {
+ ut_params->auth_xform.next = NULL;
+ ut_params->cipher_xform.next = &ut_params->auth_xform;
+ status = rte_cryptodev_sym_session_init(dev_id, ut_params->sess,
+ &ut_params->cipher_xform,
+ ts_params->session_priv_mpool);
+
+ } else
+ status = rte_cryptodev_sym_session_init(dev_id, ut_params->sess,
+ &ut_params->auth_xform,
+ ts_params->session_priv_mpool);
+
TEST_ASSERT_EQUAL(status, 0, "session init failed");
TEST_ASSERT_NOT_NULL(ut_params->sess, "Session creation failed");
@@ -3018,13 +3032,14 @@ create_wireless_algo_cipher_hash_operation(const
uint8_t *auth_tag,
}
static int
-create_wireless_algo_auth_cipher_operation(unsigned int auth_tag_len,
+create_wireless_algo_auth_cipher_operation(
+ const uint8_t *auth_tag, unsigned int auth_tag_len,
const uint8_t *cipher_iv, uint8_t cipher_iv_len,
const uint8_t *auth_iv, uint8_t auth_iv_len,
unsigned int data_pad_len,
unsigned int cipher_len, unsigned int cipher_offset,
unsigned int auth_len, unsigned int auth_offset,
- uint8_t op_mode, uint8_t do_sgl)
+ uint8_t op_mode, uint8_t do_sgl, uint8_t verify)
{
struct crypto_testsuite_params *ts_params = &testsuite_params;
struct crypto_unittest_params *ut_params = &unittest_params;
@@ -3081,6 +3096,10 @@ create_wireless_algo_auth_cipher_operation(unsigned int
auth_tag_len,
}
}
+ /* Copy digest for the verification */
+ if (verify)
+ memcpy(sym_op->auth.digest.data, auth_tag, auth_tag_len);
+
/* Copy cipher and auth IVs at the end of the crypto operation */
uint8_t *iv_ptr = rte_crypto_op_ctod_offset(
ut_params->op, uint8_t *, IV_OFFSET);
@@ -4643,7 +4662,7 @@ test_snow3g_auth_cipher(const struct snow3g_test_data
*tdata,
/* Create SNOW 3G operation */
retval = create_wireless_algo_auth_cipher_operation(
- tdata->digest.len,
+ tdata->digest.data, tdata->digest.len,
tdata->cipher_iv.data, tdata->cipher_iv.len,
tdata->auth_iv.data, tdata->auth_iv.len,
(tdata->digest.offset_bytes == 0 ?
@@ -4653,7 +4672,7 @@ test_snow3g_auth_cipher(const struct snow3g_test_data
*tdata,
tdata->cipher.offset_bits,
tdata->validAuthLenInBits.len,
tdata->auth.offset_bits,
- op_mode, 0);
+ op_mode, 0, verify);
if (retval < 0)
return retval;
@@ -4819,7 +4838,7 @@ test_snow3g_auth_cipher_sgl(const struct snow3g_test_data
*tdata,
/* Create SNOW 3G operation */
retval = create_wireless_algo_auth_cipher_operation(
- tdata->digest.len,
+ tdata->digest.data, tdata->digest.len,
tdata->cipher_iv.data, tdata->cipher_iv.len,
tdata->auth_iv.data, tdata->auth_iv.len,
(tdata->digest.offset_bytes == 0 ?
@@ -4829,7 +4848,7 @@ test_snow3g_auth_cipher_sg
Re: [dpdk-dev] [PATCH] test/crypto: add operation status checks
Hi Adam,
> -Original Message-
> From: Dybkowski, AdamX
> Sent: Friday, December 20, 2019 11:50 AM
> To: dev@dpdk.org; Trahe, Fiona ; akhil.go...@nxp.com;
> Doherty, Declan
>
> Cc: Dybkowski, AdamX
> Subject: [PATCH] test/crypto: add operation status checks
>
> This patch adds checking of the symmetric crypto operation status
> that was silently skipped before. It fixes the wireless algorithms
> session creation (SNOW3G, KASUMI, ZUC) and passing of the digest
> data for the verification by PMD.
>
[Fiona] This should be marked as a fix for backporting
> Signed-off-by: Adam Dybkowski
> ---
> app/test/test_cryptodev.c | 96 +--
> 1 file changed, 52 insertions(+), 44 deletions(-)
>
> diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
> index 1b561456d..241a1f97a 100644
> --- a/app/test/test_cryptodev.c
> +++ b/app/test/test_cryptodev.c
> @@ -143,7 +143,7 @@ static struct rte_crypto_op *
> process_crypto_request(uint8_t dev_id, struct rte_crypto_op *op)
> {
> if (rte_cryptodev_enqueue_burst(dev_id, 0, &op, 1) != 1) {
> - printf("Error sending packet for encryption");
> + printf("Error sending packet for encryption\n");
[Fiona] Can you replace this with RTE_LOG while you're modifying it please
> return NULL;
> }
>
> @@ -152,6 +152,11 @@ process_crypto_request(uint8_t dev_id, struct
> rte_crypto_op *op)
> while (rte_cryptodev_dequeue_burst(dev_id, 0, &op, 1) == 0)
> rte_pause();
>
> + if (op->status != RTE_CRYPTO_OP_STATUS_SUCCESS) {
> + RTE_LOG(DEBUG, USER1, "Operation status %d\n", op->status);
> + return NULL;
> + }
> +
[Fiona] are there any negative tests - e.g. that expect to see an auth verify
failure or invalid session -
that would be affected by this? If so should the actual status be returned?
[dpdk-dev] [PATCH v1] net/axgbe: enhance device stats reporting
From: Chandu Babu N
Implements eth dev ops xstats_get, xstats_reset, xstats_get_names,
xstats_get_names_by_id, xstats_get_by_id
Signed-off-by: Chandu Babu N
---
drivers/net/axgbe/axgbe_dev.c| 15 ++
drivers/net/axgbe/axgbe_ethdev.c | 408 +++
drivers/net/axgbe/axgbe_ethdev.h | 49
drivers/net/axgbe/axgbe_rxtx.c | 1 +
drivers/net/axgbe/axgbe_rxtx.h | 1 +
5 files changed, 474 insertions(+)
diff --git a/drivers/net/axgbe/axgbe_dev.c b/drivers/net/axgbe/axgbe_dev.c
index b1f0bbc8e..83089f20d 100644
--- a/drivers/net/axgbe/axgbe_dev.c
+++ b/drivers/net/axgbe/axgbe_dev.c
@@ -1036,6 +1036,20 @@ static void axgbe_config_checksum_offload(struct
axgbe_port *pdata)
AXGMAC_IOWRITE_BITS(pdata, MAC_RCR, IPC, 0);
}
+static void axgbe_config_mmc(struct axgbe_port *pdata)
+{
+ struct axgbe_mmc_stats *stats = &pdata->mmc_stats;
+
+ /* Reset stats */
+ memset(stats, 0, sizeof(*stats));
+
+ /* Set counters to reset on read */
+ AXGMAC_IOWRITE_BITS(pdata, MMC_CR, ROR, 1);
+
+ /* Reset the counters */
+ AXGMAC_IOWRITE_BITS(pdata, MMC_CR, CR, 1);
+}
+
static int axgbe_init(struct axgbe_port *pdata)
{
int ret;
@@ -1078,6 +1092,7 @@ static int axgbe_init(struct axgbe_port *pdata)
axgbe_config_flow_control(pdata);
axgbe_config_mac_speed(pdata);
axgbe_config_checksum_offload(pdata);
+ axgbe_config_mmc(pdata);
return 0;
}
diff --git a/drivers/net/axgbe/axgbe_ethdev.c b/drivers/net/axgbe/axgbe_ethdev.c
index d1f160e79..750a768e4 100644
--- a/drivers/net/axgbe/axgbe_ethdev.c
+++ b/drivers/net/axgbe/axgbe_ethdev.c
@@ -24,9 +24,79 @@ static int axgbe_dev_link_update(struct rte_eth_dev *dev,
static int axgbe_dev_stats_get(struct rte_eth_dev *dev,
struct rte_eth_stats *stats);
static int axgbe_dev_stats_reset(struct rte_eth_dev *dev);
+static int axgbe_dev_xstats_get(struct rte_eth_dev *dev,
+ struct rte_eth_xstat *stats,
+ unsigned int n);
+static int
+axgbe_dev_xstats_get_names(struct rte_eth_dev *dev,
+ struct rte_eth_xstat_name *xstats_names,
+ unsigned int size);
+static int
+axgbe_dev_xstats_get_by_id(struct rte_eth_dev *dev,
+ const uint64_t *ids,
+ uint64_t *values,
+ unsigned int n);
+static int
+axgbe_dev_xstats_get_names_by_id(struct rte_eth_dev *dev,
+struct rte_eth_xstat_name *xstats_names,
+const uint64_t *ids,
+unsigned int size);
+static int axgbe_dev_xstats_reset(struct rte_eth_dev *dev);
static int axgbe_dev_info_get(struct rte_eth_dev *dev,
struct rte_eth_dev_info *dev_info);
+struct axgbe_xstats {
+ char name[RTE_ETH_XSTATS_NAME_SIZE];
+ int offset;
+};
+
+#define AXGMAC_MMC_STAT(_string, _var) \
+ { _string, \
+ offsetof(struct axgbe_mmc_stats, _var), \
+ }
+
+static const struct axgbe_xstats axgbe_xstats_strings[] = {
+ AXGMAC_MMC_STAT("tx_bytes", txoctetcount_gb),
+ AXGMAC_MMC_STAT("tx_packets", txframecount_gb),
+ AXGMAC_MMC_STAT("tx_unicast_packets", txunicastframes_gb),
+ AXGMAC_MMC_STAT("tx_broadcast_packets", txbroadcastframes_gb),
+ AXGMAC_MMC_STAT("tx_multicast_packets", txmulticastframes_gb),
+ AXGMAC_MMC_STAT("tx_vlan_packets", txvlanframes_g),
+ AXGMAC_MMC_STAT("tx_64_byte_packets", tx64octets_gb),
+ AXGMAC_MMC_STAT("tx_65_to_127_byte_packets", tx65to127octets_gb),
+ AXGMAC_MMC_STAT("tx_128_to_255_byte_packets", tx128to255octets_gb),
+ AXGMAC_MMC_STAT("tx_256_to_511_byte_packets", tx256to511octets_gb),
+ AXGMAC_MMC_STAT("tx_512_to_1023_byte_packets", tx512to1023octets_gb),
+ AXGMAC_MMC_STAT("tx_1024_to_max_byte_packets", tx1024tomaxoctets_gb),
+ AXGMAC_MMC_STAT("tx_underflow_errors", txunderflowerror),
+ AXGMAC_MMC_STAT("tx_pause_frames", txpauseframes),
+
+ AXGMAC_MMC_STAT("rx_bytes", rxoctetcount_gb),
+ AXGMAC_MMC_STAT("rx_packets", rxframecount_gb),
+ AXGMAC_MMC_STAT("rx_unicast_packets", rxunicastframes_g),
+ AXGMAC_MMC_STAT("rx_broadcast_packets", rxbroadcastframes_g),
+ AXGMAC_MMC_STAT("rx_multicast_packets", rxmulticastframes_g),
+ AXGMAC_MMC_STAT("rx_vlan_packets", rxvlanframes_gb),
+ AXGMAC_MMC_STAT("rx_64_byte_packets", rx64octets_gb),
+ AXGMAC_MMC_STAT("rx_65_to_127_byte_packets", rx65to127octets_gb),
+ AXGMAC_MMC_STAT("rx_128_to_255_byte_packets", rx128to255octets_gb),
+ AXGMAC_MMC_STAT("rx_256_to_511_byte_packets", rx256to511octets_gb),
+ AXGMAC_MMC_STAT("rx_512_to_1023_byte_packets", rx512to1023octets_gb),
+ AXGMAC_MMC_STAT("
Re: [dpdk-dev] [PATCH] test/crypto: add operation status checks
Hi Fiona.
Answers inline below.
> -Original Message-
> From: Trahe, Fiona
> Sent: Friday, 20 December, 2019 13:39
> To: Dybkowski, AdamX ; dev@dpdk.org;
> akhil.go...@nxp.com; Doherty, Declan
> Cc: Trahe, Fiona
> Subject: RE: [PATCH] test/crypto: add operation status checks
>
> Hi Adam,
>
>
> > -Original Message-
> > From: Dybkowski, AdamX
> > Sent: Friday, December 20, 2019 11:50 AM
> > To: dev@dpdk.org; Trahe, Fiona ;
> > akhil.go...@nxp.com; Doherty, Declan
> > Cc: Dybkowski, AdamX
> > Subject: [PATCH] test/crypto: add operation status checks
> >
> > This patch adds checking of the symmetric crypto operation status that
> > was silently skipped before. It fixes the wireless algorithms session
> > creation (SNOW3G, KASUMI, ZUC) and passing of the digest data for the
> > verification by PMD.
> >
> [Fiona] This should be marked as a fix for backporting
[Adam] OK, will do in v2.
>
> > Signed-off-by: Adam Dybkowski
> > ---
> > app/test/test_cryptodev.c | 96
> > +--
> > 1 file changed, 52 insertions(+), 44 deletions(-)
> >
> > diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
> > index 1b561456d..241a1f97a 100644
> > --- a/app/test/test_cryptodev.c
> > +++ b/app/test/test_cryptodev.c
> > @@ -143,7 +143,7 @@ static struct rte_crypto_op *
> > process_crypto_request(uint8_t dev_id, struct rte_crypto_op *op) {
> > if (rte_cryptodev_enqueue_burst(dev_id, 0, &op, 1) != 1) {
> > -printf("Error sending packet for encryption");
> > +printf("Error sending packet for encryption\n");
> [Fiona] Can you replace this with RTE_LOG while you're modifying it please
[Adam] OK, will do in v2.
> > return NULL;
> > }
> >
> > @@ -152,6 +152,11 @@ process_crypto_request(uint8_t dev_id, struct
> > rte_crypto_op *op) while (rte_cryptodev_dequeue_burst(dev_id, 0, &op,
> > 1) == 0) rte_pause();
> >
> > +if (op->status != RTE_CRYPTO_OP_STATUS_SUCCESS) { RTE_LOG(DEBUG,
> > +USER1, "Operation status %d\n", op->status); return NULL; }
> > +
> [Fiona] are there any negative tests - e.g. that expect to see an auth verify
> failure or invalid session - that would be affected by this? If so should the
> actual status be returned?
[Adam] Few negative tests had to be updated because now in the case of any
op error, the process request function returns NULL. The negative tests now
check
for the NULL instead of checking for nonzero op status. This was done in
test_authenticated_decryption_fail_when_corruption,
test_authentication_verify_GMAC_fail_when_corruption and
test_authentication_verify_fail_when_data_corruption.
[dpdk-dev] [PATCH v2 1/1] test/crypto: fix missing operation status check
This patch adds checking of the symmetric crypto operation status
that was silently skipped before. It fixes the wireless algorithms
session creation (SNOW3G, KASUMI, ZUC) and passing of the digest
data for the verification by PMD. Also fixed the missing aad padding
issue revealed after op status checking was introduced.
Fixes: c0f87eb5252b ("cryptodev: change burst API to be crypto op oriented")
Fixes: 77a217a19bb7 ("test/crypto: add AES-CCM tests")
Signed-off-by: Adam Dybkowski
---
app/test/test_cryptodev.c | 96 +--
1 file changed, 52 insertions(+), 44 deletions(-)
diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 1b561456d..79ced809d 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -143,7 +143,7 @@ static struct rte_crypto_op *
process_crypto_request(uint8_t dev_id, struct rte_crypto_op *op)
{
if (rte_cryptodev_enqueue_burst(dev_id, 0, &op, 1) != 1) {
- printf("Error sending packet for encryption");
+ RTE_LOG(ERR, USER1, "Error sending packet for encryption\n");
return NULL;
}
@@ -152,6 +152,11 @@ process_crypto_request(uint8_t dev_id, struct
rte_crypto_op *op)
while (rte_cryptodev_dequeue_burst(dev_id, 0, &op, 1) == 0)
rte_pause();
+ if (op->status != RTE_CRYPTO_OP_STATUS_SUCCESS) {
+ RTE_LOG(DEBUG, USER1, "Operation status %d\n", op->status);
+ return NULL;
+ }
+
return op;
}
@@ -2823,9 +2828,18 @@ create_wireless_algo_auth_cipher_session(uint8_t dev_id,
ut_params->sess = rte_cryptodev_sym_session_create(
ts_params->session_mpool);
- status = rte_cryptodev_sym_session_init(dev_id, ut_params->sess,
- &ut_params->auth_xform,
- ts_params->session_priv_mpool);
+ if (cipher_op == RTE_CRYPTO_CIPHER_OP_DECRYPT) {
+ ut_params->auth_xform.next = NULL;
+ ut_params->cipher_xform.next = &ut_params->auth_xform;
+ status = rte_cryptodev_sym_session_init(dev_id, ut_params->sess,
+ &ut_params->cipher_xform,
+ ts_params->session_priv_mpool);
+
+ } else
+ status = rte_cryptodev_sym_session_init(dev_id, ut_params->sess,
+ &ut_params->auth_xform,
+ ts_params->session_priv_mpool);
+
TEST_ASSERT_EQUAL(status, 0, "session init failed");
TEST_ASSERT_NOT_NULL(ut_params->sess, "Session creation failed");
@@ -3018,13 +3032,14 @@ create_wireless_algo_cipher_hash_operation(const
uint8_t *auth_tag,
}
static int
-create_wireless_algo_auth_cipher_operation(unsigned int auth_tag_len,
+create_wireless_algo_auth_cipher_operation(
+ const uint8_t *auth_tag, unsigned int auth_tag_len,
const uint8_t *cipher_iv, uint8_t cipher_iv_len,
const uint8_t *auth_iv, uint8_t auth_iv_len,
unsigned int data_pad_len,
unsigned int cipher_len, unsigned int cipher_offset,
unsigned int auth_len, unsigned int auth_offset,
- uint8_t op_mode, uint8_t do_sgl)
+ uint8_t op_mode, uint8_t do_sgl, uint8_t verify)
{
struct crypto_testsuite_params *ts_params = &testsuite_params;
struct crypto_unittest_params *ut_params = &unittest_params;
@@ -3081,6 +3096,10 @@ create_wireless_algo_auth_cipher_operation(unsigned int
auth_tag_len,
}
}
+ /* Copy digest for the verification */
+ if (verify)
+ memcpy(sym_op->auth.digest.data, auth_tag, auth_tag_len);
+
/* Copy cipher and auth IVs at the end of the crypto operation */
uint8_t *iv_ptr = rte_crypto_op_ctod_offset(
ut_params->op, uint8_t *, IV_OFFSET);
@@ -4643,7 +4662,7 @@ test_snow3g_auth_cipher(const struct snow3g_test_data
*tdata,
/* Create SNOW 3G operation */
retval = create_wireless_algo_auth_cipher_operation(
- tdata->digest.len,
+ tdata->digest.data, tdata->digest.len,
tdata->cipher_iv.data, tdata->cipher_iv.len,
tdata->auth_iv.data, tdata->auth_iv.len,
(tdata->digest.offset_bytes == 0 ?
@@ -4653,7 +4672,7 @@ test_snow3g_auth_cipher(const struct snow3g_test_data
*tdata,
tdata->cipher.offset_bits,
tdata->validAuthLenInBits.len,
tdata->auth.offset_bits,
- op_mode, 0);
+ op_mode, 0, verify);
if (retval < 0)
return retval;
@@ -4819,7 +4838,7 @@ test_snow3g_auth_cipher_sgl(const struct snow3g_test_data
*tdata,
/* Create SNOW 3G operation */
retval = create_wireless_algo_auth_cipher_operation(
- tdata->digest.len,
+ tdata->digest.data,
[dpdk-dev] [PATCH v2 0/1] test/crypto: fix missing operation status check
This patch adds checking of the symmetric crypto operation status that was silently skipped before. It fixes the wireless algorithms session creation (SNOW3G, KASUMI, ZUC) and passing of the digest data for the verification by PMD. Also fixed the missing aad padding issue revealed after op status checking was introduced. --- v2: * update the commit message, add missing "Fixes" tags * use RTE_LOG instead of printf Adam Dybkowski (1): test/crypto: fix missing operation status check app/test/test_cryptodev.c | 96 +-- 1 file changed, 52 insertions(+), 44 deletions(-) -- 2.17.1
[dpdk-dev] [PATCH v1 1/1] net/octeontx2: allow vec to process pkts not multiple of 4
From: Vamsi Attunuru
Current vector mode implementation floor-aligns pkt count
with NIX_DESCS_PER_LOOP and process that many packets.
Patch addresses the case where pkt count modulo NIX_DESCS_PER_LOOP
is non-zero, after the vector mode processing, scalar routine is
used to process if there are any leftover packets. Scalar routine
is also used when descriptor head is about to wrap and turn out to
be unaligned.
Signed-off-by: Vamsi Attunuru
Signed-off-by: Nithin Dabilpuram
---
drivers/net/octeontx2/otx2_rx.c | 18 ++
drivers/net/octeontx2/otx2_tx.c | 18 +-
2 files changed, 27 insertions(+), 9 deletions(-)
diff --git a/drivers/net/octeontx2/otx2_rx.c b/drivers/net/octeontx2/otx2_rx.c
index 48565db..8e6452a 100644
--- a/drivers/net/octeontx2/otx2_rx.c
+++ b/drivers/net/octeontx2/otx2_rx.c
@@ -130,16 +130,22 @@ nix_recv_pkts_vector(void *rx_queue, struct rte_mbuf
**rx_pkts,
const uintptr_t desc = rxq->desc;
uint8x16_t f0, f1, f2, f3;
uint32_t head = rxq->head;
+ uint16_t pkts_left;
pkts = nix_rx_nb_pkts(rxq, wdata, pkts, qmask);
+ pkts_left = pkts & (NIX_DESCS_PER_LOOP - 1);
+
/* Packets has to be floor-aligned to NIX_DESCS_PER_LOOP */
pkts = RTE_ALIGN_FLOOR(pkts, NIX_DESCS_PER_LOOP);
while (packets < pkts) {
- /* Get the CQ pointers, since the ring size is multiple of
-* 4, We can avoid checking the wrap around of head
-* value after the each access unlike scalar version.
-*/
+ /* Exit loop if head is about to wrap and become unaligned */
+ if (((head + NIX_DESCS_PER_LOOP - 1) & qmask) <
+ NIX_DESCS_PER_LOOP) {
+ pkts_left += (pkts - packets);
+ break;
+ }
+
const uintptr_t cq0 = desc + CQE_SZ(head);
/* Prefetch N desc ahead */
@@ -301,6 +307,10 @@ nix_recv_pkts_vector(void *rx_queue, struct rte_mbuf
**rx_pkts,
/* Free all the CQs that we've processed */
otx2_write64((rxq->wdata | packets), rxq->cq_door);
+ if (unlikely(pkts_left))
+ packets += nix_recv_pkts(rx_queue, &rx_pkts[packets],
+pkts_left, flags);
+
return packets;
}
diff --git a/drivers/net/octeontx2/otx2_tx.c b/drivers/net/octeontx2/otx2_tx.c
index fa53300..96be92a 100644
--- a/drivers/net/octeontx2/otx2_tx.c
+++ b/drivers/net/octeontx2/otx2_tx.c
@@ -97,7 +97,7 @@ nix_xmit_pkts_mseg(void *tx_queue, struct rte_mbuf **tx_pkts,
#define NIX_DESCS_PER_LOOP 4
static __rte_always_inline uint16_t
nix_xmit_pkts_vector(void *tx_queue, struct rte_mbuf **tx_pkts,
-uint16_t pkts, const uint16_t flags)
+uint16_t pkts, uint64_t *cmd, const uint16_t flags)
{
uint64x2_t dataoff_iova0, dataoff_iova1, dataoff_iova2, dataoff_iova3;
uint64x2_t len_olflags0, len_olflags1, len_olflags2, len_olflags3;
@@ -118,11 +118,13 @@ nix_xmit_pkts_vector(void *tx_queue, struct rte_mbuf
**tx_pkts,
uint64x2_t cmd20, cmd21;
uint64x2_t cmd30, cmd31;
uint64_t lmt_status, i;
-
- pkts = RTE_ALIGN_FLOOR(pkts, NIX_DESCS_PER_LOOP);
+ uint16_t pkts_left;
NIX_XMIT_FC_OR_RETURN(txq, pkts);
+ pkts_left = pkts & (NIX_DESCS_PER_LOOP - 1);
+ pkts = RTE_ALIGN_FLOOR(pkts, NIX_DESCS_PER_LOOP);
+
/* Reduce the cached count */
txq->fc_cache_pkts -= pkts;
@@ -929,17 +931,21 @@ nix_xmit_pkts_vector(void *tx_queue, struct rte_mbuf
**tx_pkts,
} while (lmt_status == 0);
}
+ if (unlikely(pkts_left))
+ pkts += nix_xmit_pkts(tx_queue, tx_pkts, pkts_left, cmd, flags);
+
return pkts;
}
#else
static __rte_always_inline uint16_t
nix_xmit_pkts_vector(void *tx_queue, struct rte_mbuf **tx_pkts,
-uint16_t pkts, const uint16_t flags)
+uint16_t pkts, uint64_t *cmd, const uint16_t flags)
{
RTE_SET_USED(tx_queue);
RTE_SET_USED(tx_pkts);
RTE_SET_USED(pkts);
+ RTE_SET_USED(cmd);
RTE_SET_USED(flags);
return 0;
}
@@ -985,12 +991,14 @@ static uint16_t __rte_noinline__hot
\
otx2_nix_xmit_pkts_vec_ ## name(void *tx_queue,
\
struct rte_mbuf **tx_pkts, uint16_t pkts) \
{ \
+ uint64_t cmd[sz]; \
+ \
/* VLAN, TSTMP, TSO is not supported by vec */ \
if ((flags) & NIX_TX_OFFLOAD_VLAN_QINQ_F || \
(flags) & NIX_TX_OFFLOAD_TSTAMP_F ||\
(flags) & NIX_TX_OFFLOA
Re: [dpdk-dev] [PATCH v2 1/1] test/crypto: fix missing operation status check
> -Original Message-
> From: Dybkowski, AdamX
> Sent: Friday, December 20, 2019 12:59 PM
> To: dev@dpdk.org; Trahe, Fiona ; akhil.go...@nxp.com;
> Doherty, Declan
> ; sta...@dpdk.org
> Cc: Dybkowski, AdamX
> Subject: [PATCH v2 1/1] test/crypto: fix missing operation status check
>
> This patch adds checking of the symmetric crypto operation status
> that was silently skipped before. It fixes the wireless algorithms
> session creation (SNOW3G, KASUMI, ZUC) and passing of the digest
> data for the verification by PMD. Also fixed the missing aad padding
> issue revealed after op status checking was introduced.
>
> Fixes: c0f87eb5252b ("cryptodev: change burst API to be crypto op oriented")
> Fixes: 77a217a19bb7 ("test/crypto: add AES-CCM tests")
>
> Signed-off-by: Adam Dybkowski
Acked-by: Fiona Trahe
Re: [dpdk-dev] [PATCH v6 00/11] Add ABI compatibility checks to the meson build
On Fri, Dec 20, 2019 at 12:04 PM Bruce Richardson wrote: > > For maintainers that integrate patches or developers that get a CI > > failure and want to fix it, we would need to help them to: > > * generate dumps on a reference version, so I would tend to write some > > documentation since playing with the current sources would be too > > dangerous from my pov, > > This should be a one-off reference dump archived somewhere. Each maintainer > should not have his own copy, I think. This is not a one-off thing. We maintain ABI for some time (1/2 year(s)), and we expect to bump ABI. When doing this, in size, the diff will be at least the same than what we have here. If you disable libraries, features etc... you get a new ABI. What would be the reference*s* then? Builds with default options from meson for each architecture? > > * run the checks, like adding the check in the > > devtools/test-*-build.sh scripts that already exist, with a new > > configuration item to point at the dumps per target, > > > > Where do we store the dumps then? Do they get stored in a separate git > repo? Creating a separate git repo is just adding more pain to submitters (/maintainers): they would have to submit (/apply) patches against two trees. -- David Marchand
Re: [dpdk-dev] [PATCH v2 0/2] add travis ci support for aarch64
On Fri, Dec 20, 2019 at 10:38 AM Ruifeng Wang wrote: > > This patch set is to enable native aarch64 build in Travis CI. > It leverages Travis CI multi arch support. > > As the first step, compilation jobs are added. > Unit test is not added for now due to service limitation. We are > planning to run unit test with no-huge in future. > > This patch set has dependency on: > http://patches.dpdk.org/patch/63969/ > > > v2: > Removed dist designation from matrix since base dist is changing. *If* (I did not think about the implications yet, hence the *If*) we switch the base distribution to Bionic, we can't push it anyway without your second patch. Patches order must be reversed. And for the jobs in travis, the distribution for aarch64 native jobs must be explicitely bionic distribution (you did not update the commitlog in the v2 but not a problem if you restore the dist: bionic entries). We can then take your series. Kevin patch that upgrades globally from xenial to bionic, can then be integrated after yours. > Add explanation for library path issue. Thanks for this. -- David Marchand
[dpdk-dev] [PATCH v2] test/common: fix log2 check
We recently started to get random failures on the common_autotest ut with
clang on Ubuntu 16.04.6.
Example: https://travis-ci.com/DPDK/dpdk/jobs/263177424
Wrong rte_log2_u64(0) val 0, expected
Test Failed
The ut passes 0 to log2() to get an expected value.
Quoting log2 / log(3) manual:
If x is zero, then a pole error occurs, and the functions return
-HUGE_VAL, -HUGE_VALF, or -HUGE_VALL, respectively.
rte_log2_uXX helpers handle 0 as a special value and return 0.
Let's have dedicated tests for this case.
Fixes: 05c4345ef5c2 ("test: add unit test for integer log2 function")
Cc: sta...@dpdk.org
Signed-off-by: David Marchand
Acked-by: Aaron Conole
---
Changelog since v1:
- added comment in rte_log2_uXX API descriptions,
---
app/test/test_common.c | 14 +-
lib/librte_eal/common/include/rte_common.h | 6 ++
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/app/test/test_common.c b/app/test/test_common.c
index 2b856f8ba5..12bd1cad90 100644
--- a/app/test/test_common.c
+++ b/app/test/test_common.c
@@ -216,7 +216,19 @@ test_log2(void)
const uint32_t max = 0x1;
const uint32_t step = 1;
- for (i = 0; i < max; i = i + step) {
+ compare = rte_log2_u32(0);
+ if (compare != 0) {
+ printf("Wrong rte_log2_u32(0) val %x, expected 0\n", compare);
+ return TEST_FAILED;
+ }
+
+ compare = rte_log2_u64(0);
+ if (compare != 0) {
+ printf("Wrong rte_log2_u64(0) val %x, expected 0\n", compare);
+ return TEST_FAILED;
+ }
+
+ for (i = 1; i < max; i = i + step) {
uint64_t i64;
/* extend range for 64-bit */
diff --git a/lib/librte_eal/common/include/rte_common.h
b/lib/librte_eal/common/include/rte_common.h
index 459d082d14..7a98071ffe 100644
--- a/lib/librte_eal/common/include/rte_common.h
+++ b/lib/librte_eal/common/include/rte_common.h
@@ -538,6 +538,9 @@ rte_bsf32_safe(uint64_t v, uint32_t *pos)
/**
* Return the rounded-up log2 of a integer.
*
+ * @note Contrary to the logarithm mathematical operation,
+ * rte_log2_u32(0) == 0 and not -inf.
+ *
* @param v
* The input parameter.
* @return
@@ -632,6 +635,9 @@ rte_fls_u64(uint64_t x)
/**
* Return the rounded-up log2 of a 64-bit integer.
*
+ * @note Contrary to the logarithm mathematical operation,
+ * rte_log2_u32(0) == 0 and not -inf.
+ *
* @param v
* The input parameter.
* @return
--
2.23.0
Re: [dpdk-dev] [dpdk-stable] [PATCH v2] test/common: fix log2 check
On Fri, Dec 20, 2019 at 3:02 PM David Marchand wrote: > diff --git a/lib/librte_eal/common/include/rte_common.h > b/lib/librte_eal/common/include/rte_common.h > index 459d082d14..7a98071ffe 100644 > --- a/lib/librte_eal/common/include/rte_common.h > +++ b/lib/librte_eal/common/include/rte_common.h > @@ -538,6 +538,9 @@ rte_bsf32_safe(uint64_t v, uint32_t *pos) > /** > * Return the rounded-up log2 of a integer. > * > + * @note Contrary to the logarithm mathematical operation, > + * rte_log2_u32(0) == 0 and not -inf. > + * > * @param v > * The input parameter. > * @return > @@ -632,6 +635,9 @@ rte_fls_u64(uint64_t x) > /** > * Return the rounded-up log2 of a 64-bit integer. > * > + * @note Contrary to the logarithm mathematical operation, > + * rte_log2_u32(0) == 0 and not -inf. _u64* Will fix while applying. -- David Marchand
Re: [dpdk-dev] [PATCH v6 00/11] Add ABI compatibility checks to the meson build
On Fri, Dec 20, 2019 at 02:19:13PM +0100, David Marchand wrote: > On Fri, Dec 20, 2019 at 12:04 PM Bruce Richardson > wrote: > > > For maintainers that integrate patches or developers that get a CI > > > failure and want to fix it, we would need to help them to: > > > * generate dumps on a reference version, so I would tend to write some > > > documentation since playing with the current sources would be too > > > dangerous from my pov, > > > > This should be a one-off reference dump archived somewhere. Each maintainer > > should not have his own copy, I think. > > This is not a one-off thing. > We maintain ABI for some time (1/2 year(s)), and we expect to bump ABI. > When doing this, in size, the diff will be at least the same than what > we have here. > I don't think it will be quite that big, but ok, it may be significant, so I will concede that these are too big to store in the main git repo. > > If you disable libraries, features etc... you get a new ABI. > What would be the reference*s* then? > Builds with default options from meson for each architecture? > On the project level API, yes, removing libraries/drivers does affect things. However, the specific checks are done on the individual .so level, so having some drivers off in the build should not be a problem. Even with features - all public functions need to correspond with map file entries, so we can't conditionally remove them without providing a stub AFAIK. Therefore, having one master reference of the DPDK_20 ABI is perfectly feasible. > > > > * run the checks, like adding the check in the > > > devtools/test-*-build.sh scripts that already exist, with a new > > > configuration item to point at the dumps per target, > > > > > > > Where do we store the dumps then? Do they get stored in a separate git > > repo? > > Creating a separate git repo is just adding more pain to submitters > (/maintainers): they would have to submit (/apply) patches against two > trees. > Well, the official ABI dumps need to be stored somewhere, because if it's an official ABI, then it is exactly that. There cannot be different people with different versions of the ABI to check against. Everyone should check against the one common, official reference. Regards, /Bruce
[dpdk-dev] [PATCH] examples/l2fwd-event: fix event device config
From: Pavan Nikhilesh
Always enable implicit release since we don't support explicit release
in datapath.
Master lcore is used only for printing stats so don't allocate event
port for it.
Fix service launch for event device without distributed scheduling.
Fixes: bcb6f841d42a ("examples/l2fwd-event: setup service core")
Cc: sta...@dpdk.org
Signed-off-by: Pavan Nikhilesh
---
examples/l2fwd-event/l2fwd_event.c | 2 +-
examples/l2fwd-event/l2fwd_event_generic.c | 9 ++---
examples/l2fwd-event/l2fwd_event_internal_port.c | 11 +--
3 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/examples/l2fwd-event/l2fwd_event.c
b/examples/l2fwd-event/l2fwd_event.c
index 0379c580d..38d590c14 100644
--- a/examples/l2fwd-event/l2fwd_event.c
+++ b/examples/l2fwd-event/l2fwd_event.c
@@ -67,7 +67,7 @@ l2fwd_event_service_setup(struct l2fwd_resources *rsrc)
int ret, i;
rte_event_dev_info_get(evt_rsrc->event_d_id, &evdev_info);
- if (evdev_info.event_dev_cap & RTE_EVENT_DEV_CAP_DISTRIBUTED_SCHED) {
+ if (!(evdev_info.event_dev_cap & RTE_EVENT_DEV_CAP_DISTRIBUTED_SCHED)) {
ret = rte_event_dev_service_id_get(evt_rsrc->event_d_id,
&service_id);
if (ret != -ESRCH && ret != 0)
diff --git a/examples/l2fwd-event/l2fwd_event_generic.c
b/examples/l2fwd-event/l2fwd_event_generic.c
index b7e467c1e..b07306a17 100644
--- a/examples/l2fwd-event/l2fwd_event_generic.c
+++ b/examples/l2fwd-event/l2fwd_event_generic.c
@@ -42,8 +42,10 @@ l2fwd_event_device_setup_generic(struct l2fwd_resources
*rsrc)
/* Event device configurtion */
rte_event_dev_info_get(event_d_id, &dev_info);
- evt_rsrc->disable_implicit_release = !!(dev_info.event_dev_cap &
- RTE_EVENT_DEV_CAP_IMPLICIT_RELEASE_DISABLE);
+
+ /* Enable implicit release */
+ if (dev_info.event_dev_cap & RTE_EVENT_DEV_CAP_IMPLICIT_RELEASE_DISABLE)
+ evt_rsrc->disable_implicit_release = 0;
if (dev_info.event_dev_cap & RTE_EVENT_DEV_CAP_QUEUE_ALL_TYPES)
event_queue_cfg |= RTE_EVENT_QUEUE_CFG_ALL_TYPES;
@@ -70,7 +72,8 @@ l2fwd_event_device_setup_generic(struct l2fwd_resources *rsrc)
event_d_conf.nb_event_port_enqueue_depth =
dev_info.max_event_port_enqueue_depth;
- num_workers = rte_lcore_count() - rte_service_lcore_count();
+ /* Ignore Master core and service cores. */
+ num_workers = rte_lcore_count() - 1 - rte_service_lcore_count();
if (dev_info.max_event_ports < num_workers)
num_workers = dev_info.max_event_ports;
diff --git a/examples/l2fwd-event/l2fwd_event_internal_port.c
b/examples/l2fwd-event/l2fwd_event_internal_port.c
index b382763dd..5e6e8598a 100644
--- a/examples/l2fwd-event/l2fwd_event_internal_port.c
+++ b/examples/l2fwd-event/l2fwd_event_internal_port.c
@@ -27,7 +27,6 @@ l2fwd_event_device_setup_internal_port(struct l2fwd_resources
*rsrc)
.nb_event_port_enqueue_depth = 128
};
struct rte_event_dev_info dev_info;
- uint8_t disable_implicit_release;
const uint8_t event_d_id = 0; /* Always use first event device only */
uint32_t event_queue_cfg = 0;
uint16_t ethdev_count = 0;
@@ -44,10 +43,9 @@ l2fwd_event_device_setup_internal_port(struct
l2fwd_resources *rsrc)
/* Event device configurtion */
rte_event_dev_info_get(event_d_id, &dev_info);
- disable_implicit_release = !!(dev_info.event_dev_cap &
- RTE_EVENT_DEV_CAP_IMPLICIT_RELEASE_DISABLE);
- evt_rsrc->disable_implicit_release =
- disable_implicit_release;
+ /* Enable implicit release */
+ if (dev_info.event_dev_cap & RTE_EVENT_DEV_CAP_IMPLICIT_RELEASE_DISABLE)
+ evt_rsrc->disable_implicit_release = 0;
if (dev_info.event_dev_cap & RTE_EVENT_DEV_CAP_QUEUE_ALL_TYPES)
event_queue_cfg |= RTE_EVENT_QUEUE_CFG_ALL_TYPES;
@@ -73,7 +71,8 @@ l2fwd_event_device_setup_internal_port(struct l2fwd_resources
*rsrc)
event_d_conf.nb_event_port_enqueue_depth =
dev_info.max_event_port_enqueue_depth;
- num_workers = rte_lcore_count();
+ /* Ignore Master core. */
+ num_workers = rte_lcore_count() - 1;
if (dev_info.max_event_ports < num_workers)
num_workers = dev_info.max_event_ports;
--
2.17.1
Re: [dpdk-dev] [PATCH v2] service: don't walk out of bounds when checking services
On Wed, Dec 4, 2019 at 9:34 AM David Marchand wrote:
>
> On Wed, Dec 4, 2019 at 9:33 AM David Marchand
> wrote:
> >
> > On Tue, Dec 3, 2019 at 10:15 PM Aaron Conole wrote:
> > >
> > > The service_valid call is used without properly bounds checking the
> > > input parameter. Almost all instances of the service_valid call are
> > > inside a for() loop that prevents excessive walks, but some of the
> > > public APIs don't bounds check and will pass invalid arguments.
> > >
> > > Prevent this by using SERVICE_GET_OR_ERR_RET where it makes sense,
> > > and adding a bounds check to one service_valid() use.
> > >
> > > Fixes: 8d39d3e237c2 ("service: fix race in service on app lcore function")
> > > Fixes: e9139a32f6e8 ("service: add function to run on app lcore")
> > > Fixes: e30dd31847d2 ("service: add mechanism for quiescing")
> Cc: sta...@dpdk.org
>
> > > Signed-off-by: Aaron Conole
> >
> > Reviewed-by: David Marchand
Applied, thanks.
--
David Marchand
Re: [dpdk-dev] [dpdk-stable] [PATCH v2] test/common: fix log2 check
On Fri, Dec 20, 2019 at 3:02 PM David Marchand
wrote:
>
> We recently started to get random failures on the common_autotest ut with
> clang on Ubuntu 16.04.6.
>
> Example: https://travis-ci.com/DPDK/dpdk/jobs/263177424
>
> Wrong rte_log2_u64(0) val 0, expected
> Test Failed
>
> The ut passes 0 to log2() to get an expected value.
>
> Quoting log2 / log(3) manual:
> If x is zero, then a pole error occurs, and the functions return
> -HUGE_VAL, -HUGE_VALF, or -HUGE_VALL, respectively.
>
> rte_log2_uXX helpers handle 0 as a special value and return 0.
> Let's have dedicated tests for this case.
>
> Fixes: 05c4345ef5c2 ("test: add unit test for integer log2 function")
> Cc: sta...@dpdk.org
>
> Signed-off-by: David Marchand
> Acked-by: Aaron Conole
Applied with mentioned fix.
--
David Marchand
Re: [dpdk-dev] [dpdk-stable] [PATCH v2] test/common: fix log2 check
David Marchand writes:
> On Fri, Dec 20, 2019 at 3:02 PM David Marchand
> wrote:
>>
>> We recently started to get random failures on the common_autotest ut with
>> clang on Ubuntu 16.04.6.
>>
>> Example: https://travis-ci.com/DPDK/dpdk/jobs/263177424
>>
>> Wrong rte_log2_u64(0) val 0, expected
>> Test Failed
>>
>> The ut passes 0 to log2() to get an expected value.
>>
>> Quoting log2 / log(3) manual:
>> If x is zero, then a pole error occurs, and the functions return
>> -HUGE_VAL, -HUGE_VALF, or -HUGE_VALL, respectively.
>>
>> rte_log2_uXX helpers handle 0 as a special value and return 0.
>> Let's have dedicated tests for this case.
>>
>> Fixes: 05c4345ef5c2 ("test: add unit test for integer log2 function")
>> Cc: sta...@dpdk.org
>>
>> Signed-off-by: David Marchand
>> Acked-by: Aaron Conole
>
> Applied with mentioned fix.
Thanks!
[dpdk-dev] [PATCH v2 0/1] eal: user notification improvement
v2: - fixed coding style issue Signed-off-by: Artur Trybula Artur Trybula (1): eal: improve user notification for too low memzone segments lib/librte_eal/common/eal_common_memzone.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) -- 2.17.1
[dpdk-dev] [PATCH v2 1/1] eal: improve user notification for too low memzone segments
In case of too low number of memzone segements user notification
was misleading. This patch improves the description by providing
better explanation about the cause.
Signed-off-by: Artur Trybula
---
lib/librte_eal/common/eal_common_memzone.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/lib/librte_eal/common/eal_common_memzone.c
b/lib/librte_eal/common/eal_common_memzone.c
index 99b8d6531..86f61369b 100644
--- a/lib/librte_eal/common/eal_common_memzone.c
+++ b/lib/librte_eal/common/eal_common_memzone.c
@@ -71,7 +71,9 @@ memzone_reserve_aligned_thread_unsafe(const char *name,
size_t len,
/* no more room in config */
if (arr->count >= arr->len) {
- RTE_LOG(ERR, EAL, "%s(): No more room in config\n", __func__);
+ RTE_LOG(ERR, EAL,
+ "%s(): Number of requested memzone segments exceeds
RTE_MAX_MEMZONE\n",
+ __func__);
rte_errno = ENOSPC;
return NULL;
}
--
2.17.1
[dpdk-dev] [PATCH] add ABI checks
Starting from Kevin and Bruce idea of using libabigail, here is an
alternate approach to implement ABI checks.
By default, those checks are disabled and enabling them requires a
manual step that generates the ABI dumps on a reference version for a
set of configurations.
Those checks are enabled in the CI by default for the default meson
options on x86 and aarch64 so that proposed patches are validated.
A cache of the ABI is stored in travis jobs.
Checks can be only informational by setting ABI_CHECKS_WARN_ONLY when
breaking the ABI in a future release.
For advanced developers and maintainers, the contributing guide details
the higher level scripts that are quite close to the existing devtools
scripts.
Signed-off-by: David Marchand
---
.ci/linux-build.sh | 43 +++
.travis.yml | 20 +++--
devtools/check-abi-dump.sh | 46 +
devtools/check-abi-reference.sh | 27 +
devtools/dpdk.abignore | 2 ++
devtools/gen-abi-dump.sh| 29 ++
devtools/gen-abi-reference.sh | 24 +++
devtools/test-build.sh | 13 ++--
devtools/test-meson-builds.sh | 6
doc/guides/contributing/patches.rst | 25
10 files changed, 230 insertions(+), 5 deletions(-)
create mode 100755 devtools/check-abi-dump.sh
create mode 100755 devtools/check-abi-reference.sh
create mode 100644 devtools/dpdk.abignore
create mode 100755 devtools/gen-abi-dump.sh
create mode 100755 devtools/gen-abi-reference.sh
diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index ccc3a7ccd..345dba264 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -30,8 +30,51 @@ fi
OPTS="$OPTS --default-library=$DEF_LIB"
meson build --werror -Dexamples=all $OPTS
+
+if [ "$ABI_CHECKS" = "1" ]; then
+git remote add ref ${REF_GIT_REPO:-https://dpdk.org/git/dpdk}
+git fetch --tags ref ${REF_GIT_BRANCH:-master}
+
+head=$(git describe --all)
+tag=$(git describe --abbrev=0)
+
+if [ "$(cat reference/VERSION 2>/dev/null)" != "$tag" ]; then
+rm -rf reference
+fi
+
+if [ ! -d reference ]; then
+gen_abi_dump=$(mktemp -t gen-abi-dump-XXX.sh)
+cp -a devtools/gen-abi-dump.sh $gen_abi_dump
+
+git checkout -qf $tag
+ninja -C build
+$gen_abi_dump build reference
+
+if [ "$AARCH64" != "1" ]; then
+mkdir -p reference/app
+cp -a build/app/dpdk-testpmd reference/app/
+
+export LD_LIBRARY_PATH=$(pwd)/build/lib:$(pwd)/build/drivers
+devtools/test-null.sh reference/app/dpdk-testpmd
+unset LD_LIBRARY_PATH
+fi
+echo $tag > reference/VERSION
+
+git checkout -qf $head
+fi
+fi
+
ninja -C build
+if [ "$ABI_CHECKS" = "1" ]; then
+devtools/check-abi-dump.sh build reference ${ABI_CHECKS_WARN_ONLY:-}
+if [ "$AARCH64" != "1" ]; then
+export LD_LIBRARY_PATH=$(pwd)/build/lib:$(pwd)/build/drivers
+devtools/test-null.sh reference/app/dpdk-testpmd
+unset LD_LIBRARY_PATH
+fi
+fi
+
if [ "$AARCH64" != "1" ]; then
devtools/test-null.sh
fi
diff --git a/.travis.yml b/.travis.yml
index 8f90d06f2..bbb060fa2 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,5 +1,8 @@
language: c
-cache: ccache
+cache:
+ ccache: true
+ directories:
+- reference
compiler:
- gcc
- clang
@@ -21,7 +24,7 @@ aarch64_packages: &aarch64_packages
extra_packages: &extra_packages
- *required_packages
- - [libbsd-dev, libpcap-dev, libcrypto++-dev, libjansson4]
+ - [libbsd-dev, libpcap-dev, libcrypto++-dev, libjansson4, abigail-tools]
build_32b_packages: &build_32b_packages
- *required_packages
@@ -59,6 +62,13 @@ matrix:
apt:
packages:
- *aarch64_packages
+ - env: DEF_LIB="shared" EXTRA_PACKAGES=1 ABI_CHECKS=1 AARCH64=1
+compiler: gcc
+addons:
+ apt:
+packages:
+ - *aarch64_packages
+ - *extra_packages
- env: DEF_LIB="static" EXTRA_PACKAGES=1
compiler: gcc
addons:
@@ -72,6 +82,12 @@ matrix:
packages:
- *extra_packages
- *doc_packages
+ - env: DEF_LIB="shared" EXTRA_PACKAGES=1 ABI_CHECKS=1
+compiler: gcc
+addons:
+ apt:
+packages:
+ - *extra_packages
- env: DEF_LIB="static" OPTS="-Denable_kmods=false" EXTRA_PACKAGES=1
compiler: gcc
addons:
diff --git a/devtools/check-abi-dump.sh b/devtools/check-abi-dump.sh
new file mode 100755
index 0..f48a2ae7e
--- /dev/null
+++ b/devtools/check-abi-dump.sh
@@ -0,0 +1,46 @@
+#!/bin/sh -e
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright (c) 2019 Red Hat, Inc.
+
+if [ $# != 2 ] && [ $# != 3 ]; then
+ echo "Usage: $0 builddir dumpdir [warnonly]"
+ exit 1
+fi
+
+builddir=$1
+dumpdir=$2
+warnonly=${3:-}
+if [ ! -d $builddir ]; then
+ echo "Error: build directory
Re: [dpdk-dev] [PATCH] add ABI checks
> -Original Message-
> From: David Marchand
> Sent: Friday, December 20, 2019 3:21 PM
> To: dev@dpdk.org
> Cc: tho...@monjalon.net; Richardson, Bruce ;
> Laatz, Kevin ; acon...@redhat.com;
> nhor...@tuxdriver.com; Michael Santana ;
> Mcnamara, John ; Kovacevic, Marko
>
> Subject: [PATCH] add ABI checks
>
> Starting from Kevin and Bruce idea of using libabigail, here is an
> alternate approach to implement ABI checks.
>
> By default, those checks are disabled and enabling them requires a manual
> step that generates the ABI dumps on a reference version for a set of
> configurations.
>
> Those checks are enabled in the CI by default for the default meson
> options on x86 and aarch64 so that proposed patches are validated.
> A cache of the ABI is stored in travis jobs.
> Checks can be only informational by setting ABI_CHECKS_WARN_ONLY when
> breaking the ABI in a future release.
>
> For advanced developers and maintainers, the contributing guide details
> the higher level scripts that are quite close to the existing devtools
> scripts.
>
> Signed-off-by: David Marchand
> ---
> .ci/linux-build.sh | 43 +++
> .travis.yml | 20 +++--
> devtools/check-abi-dump.sh | 46 +
> devtools/check-abi-reference.sh | 27 +
> devtools/dpdk.abignore | 2 ++
> devtools/gen-abi-dump.sh| 29 ++
> devtools/gen-abi-reference.sh | 24 +++
> devtools/test-build.sh | 13 ++--
> devtools/test-meson-builds.sh | 6
> doc/guides/contributing/patches.rst | 25
> 10 files changed, 230 insertions(+), 5 deletions(-) create mode 100755
> devtools/check-abi-dump.sh create mode 100755 devtools/check-abi-
> reference.sh create mode 100644 devtools/dpdk.abignore create mode
> 100755 devtools/gen-abi-dump.sh create mode 100755 devtools/gen-abi-
> reference.sh
>
> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh index
> ccc3a7ccd..345dba264 100755
> --- a/.ci/linux-build.sh
> +++ b/.ci/linux-build.sh
> @@ -30,8 +30,51 @@ fi
>
> OPTS="$OPTS --default-library=$DEF_LIB"
> meson build --werror -Dexamples=all $OPTS
> +
> +if [ "$ABI_CHECKS" = "1" ]; then
> +git remote add ref ${REF_GIT_REPO:-https://dpdk.org/git/dpdk}
> +git fetch --tags ref ${REF_GIT_BRANCH:-master}
> +
> +head=$(git describe --all)
> +tag=$(git describe --abbrev=0)
> +
> +if [ "$(cat reference/VERSION 2>/dev/null)" != "$tag" ]; then
> +rm -rf reference
> +fi
> +
> +if [ ! -d reference ]; then
> +gen_abi_dump=$(mktemp -t gen-abi-dump-XXX.sh)
> +cp -a devtools/gen-abi-dump.sh $gen_abi_dump
> +
> +git checkout -qf $tag
> +ninja -C build
> +$gen_abi_dump build reference
> +
> +if [ "$AARCH64" != "1" ]; then
> +mkdir -p reference/app
> +cp -a build/app/dpdk-testpmd reference/app/
> +
> +export LD_LIBRARY_PATH=$(pwd)/build/lib:$(pwd)/build/drivers
> +devtools/test-null.sh reference/app/dpdk-testpmd
> +unset LD_LIBRARY_PATH
> +fi
> +echo $tag > reference/VERSION
> +
> +git checkout -qf $head
> +fi
> +fi
> +
> ninja -C build
>
> +if [ "$ABI_CHECKS" = "1" ]; then
> +devtools/check-abi-dump.sh build reference ${ABI_CHECKS_WARN_ONLY:-}
> +if [ "$AARCH64" != "1" ]; then
> +export LD_LIBRARY_PATH=$(pwd)/build/lib:$(pwd)/build/drivers
> +devtools/test-null.sh reference/app/dpdk-testpmd
> +unset LD_LIBRARY_PATH
> +fi
> +fi
> +
> if [ "$AARCH64" != "1" ]; then
> devtools/test-null.sh
> fi
> diff --git a/.travis.yml b/.travis.yml
> index 8f90d06f2..bbb060fa2 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -1,5 +1,8 @@
> language: c
> -cache: ccache
> +cache:
> + ccache: true
> + directories:
> +- reference
> compiler:
>- gcc
>- clang
> @@ -21,7 +24,7 @@ aarch64_packages: &aarch64_packages
>
> extra_packages: &extra_packages
>- *required_packages
> - - [libbsd-dev, libpcap-dev, libcrypto++-dev, libjansson4]
> + - [libbsd-dev, libpcap-dev, libcrypto++-dev, libjansson4,
> + abigail-tools]
>
> build_32b_packages: &build_32b_packages
>- *required_packages
> @@ -59,6 +62,13 @@ matrix:
>apt:
> packages:
>- *aarch64_packages
> + - env: DEF_LIB="shared" EXTRA_PACKAGES=1 ABI_CHECKS=1 AARCH64=1
> +compiler: gcc
> +addons:
> + apt:
> +packages:
> + - *aarch64_packages
> + - *extra_packages
>- env: DEF_LIB="static" EXTRA_PACKAGES=1
> compiler: gcc
> addons:
> @@ -72,6 +82,12 @@ matrix:
> packages:
>- *extra_packages
>- *doc_packages
> + - env: DEF_LIB="shared" EXTRA_PACKAGES=1 ABI_CHECKS=1
> +compiler: gcc
> +addons:
> + apt:
> +packages:
> + - *extra_packages
Re: [dpdk-dev] [PATCH] mempool: fix mempool obj alignment for non x86
>
> From: Jerin Jacob
>
> The exiting optimize_object_size() function address the memory object
> alignment constraint on x86 for better performance.
>
> Different (Mirco) architecture may have different memory alignment
> constraint for better performance and it not same as the existing
> optimize_object_size() function. Some use, XOR(kind of CRC) scheme to
> enable DRAM channel distribution based on the address and some may have
> a different formula.
If I understand correctly, address interleaving is the characteristic of the
memory controller and not the CPU.
For ex: different SoCs using the same Arm architecture might have different
memory controllers. So, the solution should not be architecture specific, but
SoC specific.
>
> Introducing arch_mem_object_align() function to abstract the differences in
> different (mirco) architectures and avoid wasting memory for mempool
> object alignment for the architecture the existing optimize_object_size() is
> not valid.
>
> Additional details:
> https://www.mail-archive.com/dev@dpdk.org/msg149157.html
>
> Fixes: af75078fece3 ("first public release")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Jerin Jacob
> ---
> doc/guides/prog_guide/mempool_lib.rst | 6 +++---
> lib/librte_mempool/rte_mempool.c | 17 +
> 2 files changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/doc/guides/prog_guide/mempool_lib.rst
> b/doc/guides/prog_guide/mempool_lib.rst
> index 3bb84b0a6..eea7a2906 100644
> --- a/doc/guides/prog_guide/mempool_lib.rst
> +++ b/doc/guides/prog_guide/mempool_lib.rst
> @@ -27,10 +27,10 @@ In debug mode
> (CONFIG_RTE_LIBRTE_MEMPOOL_DEBUG is enabled), statistics about get
> from/put in the pool are stored in the mempool structure.
> Statistics are per-lcore to avoid concurrent access to statistics counters.
>
> -Memory Alignment Constraints
> -
> +Memory Alignment Constraints on X86 architecture
> +
>
> -Depending on hardware memory configuration, performance can be greatly
> improved by adding a specific padding between objects.
> +Depending on hardware memory configuration on X86 architecture,
> performance can be greatly improved by adding a specific padding between
> objects.
> The objective is to ensure that the beginning of each object starts on a
> different channel and rank in memory so that all channels are equally loaded.
>
> This is particularly true for packet buffers when doing L3 forwarding or flow
> classification.
> diff --git a/lib/librte_mempool/rte_mempool.c
> b/lib/librte_mempool/rte_mempool.c
> index 78d8eb941..871894525 100644
> --- a/lib/librte_mempool/rte_mempool.c
> +++ b/lib/librte_mempool/rte_mempool.c
> @@ -45,6 +45,7 @@ EAL_REGISTER_TAILQ(rte_mempool_tailq)
> #define CALC_CACHE_FLUSHTHRESH(c)\
> ((typeof(c))((c) * CACHE_FLUSHTHRESH_MULTIPLIER))
>
> +#if defined(RTE_ARCH_X86)
> /*
> * return the greatest common divisor between a and b (fast algorithm)
> *
> @@ -74,12 +75,13 @@ static unsigned get_gcd(unsigned a, unsigned b) }
>
> /*
> - * Depending on memory configuration, objects addresses are spread
> + * Depending on memory configuration on x86 arch, objects addresses are
> + spread
> * between channels and ranks in RAM: the pool allocator will add
> * padding between objects. This function return the new size of the
> * object.
> */
> -static unsigned optimize_object_size(unsigned obj_size)
> +static unsigned
> +arch_mem_object_align(unsigned obj_size)
> {
> unsigned nrank, nchan;
> unsigned new_obj_size;
> @@ -99,6 +101,13 @@ static unsigned optimize_object_size(unsigned
> obj_size)
> new_obj_size++;
> return new_obj_size * RTE_MEMPOOL_ALIGN; }
> +#else
This applies to add Arm (PPC as well) SoCs which might have different schemes
depending on the memory controller. IMO, this should not be architecture
specific.
> +static unsigned
> +arch_mem_object_align(unsigned obj_size) {
> + return obj_size;
> +}
> +#endif
>
> struct pagesz_walk_arg {
> int socket_id;
> @@ -234,8 +243,8 @@ rte_mempool_calc_obj_size(uint32_t elt_size,
> uint32_t flags,
>*/
> if ((flags & MEMPOOL_F_NO_SPREAD) == 0) {
> unsigned new_size;
> - new_size = optimize_object_size(sz->header_size + sz-
> >elt_size +
> - sz->trailer_size);
> + new_size = arch_mem_object_align
> + (sz->header_size + sz->elt_size + sz->trailer_size);
> sz->trailer_size = new_size - sz->header_size - sz->elt_size;
> }
>
> --
> 2.24.1
Re: [dpdk-dev] [PATCH 1/4] lib/crypto: add support for ECDSA
Hi Anoob,
Few suggestions inline.
> ;
> [Asymmetric]
> -RSA =
> -DSA =
> -Modular Exponentiation =
> -Modular Inversion =
> -Diffie-hellman =
> \ No newline at end of file
> +RSA =
> +DSA =
> +Modular Exponentiation =
> +Modular Inversion =
> +Diffie-hellman =
> +ECDSA =
> diff --git a/lib/librte_cryptodev/rte_crypto_asym.h
> b/lib/librte_cryptodev/rte_crypto_asym.h
> index 0d34ce8..dd5e6e3 100644
> --- a/lib/librte_cryptodev/rte_crypto_asym.h
> +++ b/lib/librte_cryptodev/rte_crypto_asym.h
> @@ -81,6 +81,10 @@ enum rte_crypto_asym_xform_type {
> /**< Modular Exponentiation
>* Perform Modular Exponentiation b^e mod n
>*/
> + RTE_CRYPTO_ASYM_XFORM_ECDSA,
> + /**< Elliptic Curve Digital Signature Algorithm
> + * Perform Signature Generation and Verification.
> + */
> RTE_CRYPTO_ASYM_XFORM_TYPE_LIST_END
> /**< End of list */
> };
> @@ -319,6 +323,46 @@ struct rte_crypto_dsa_xform { };
>
> /**
> + * TLS named curves
> + * https://www.iana.org/assignments/tls-parameters/
> + * tls-parameters.xhtml#tls-parameters-8
> + * secp192r1 = 19,
> + * secp224r1 = 21,
> + * secp256r1 = 23,
> + * secp384r1 = 24,
> + * secp521r1 = 25,
> + */
> +enum rte_crypto_ec_group {
> + RTE_CRYPTO_EC_GROUP_UNKNOWN = 0,
> + RTE_CRYPTO_EC_GROUP_NISTP192 = 19,
> + RTE_CRYPTO_EC_GROUP_NISTP224 = 21,
> + RTE_CRYPTO_EC_GROUP_NISTP256 = 23,
> + RTE_CRYPTO_EC_GROUP_NISTP384 = 24,
> + RTE_CRYPTO_EC_GROUP_NISTP521 = 25,
> +};
[Arek] Since in comment we use SECG naming maybe enum should follow to avoid
confusion?
> +
> +/**
> + * Structure for elliptic curve point
> + */
> +struct rte_crypto_ec_point {
> + rte_crypto_param x;
> + /**< X coordinate */
> + rte_crypto_param y;
> + /**< Y coordinate */
> +};
> +
> +/**
> + * Asymmetric elliptic curve transform data
> + *
> + * Structure describing all EC based xform params
> + *
> + */
> +struct rte_crypto_ec_xform {
> + enum rte_crypto_ec_group curve_id;
> + /**< Pre-defined ec groups */
> +};
> +
> +/**
> * Operations params for modular operations:
> * exponentiation and multiplicative inverse
> *
> @@ -372,6 +416,11 @@ struct rte_crypto_asym_xform {
>
> struct rte_crypto_dsa_xform dsa;
> /**< DSA xform parameters */
> +
> + struct rte_crypto_ec_xform ec;
> + /**< EC xform parameters, used by elliptic curve based
> + * operations.
> + */
> };
> };
>
> @@ -516,6 +565,39 @@ struct rte_crypto_dsa_op_param { };
>
> /**
> + * ECDSA operation params
> + */
> +struct rte_crypto_ecdsa_op_param {
> + enum rte_crypto_asym_op_type op_type;
> + /**< Signature generation or verification */
> +
> + rte_crypto_param pkey;
> + /**< Private key of the signer for signature generation */
[Arek] - for DSA we have private key in xform, why this inconsistency?
> +
> + struct rte_crypto_ec_point q;
> + /**< Public key of the signer for verification */
> +
> + rte_crypto_param message;
> + /**< Input message to be signed or verified */
[Arek] - This I expect should be message digest instead of message itself?
> +
> + rte_crypto_param k;
> + /**< The ECDSA per-message secret number, which is an integer
> + * in the interval (1, n-1)
> + */
[Arek] - If pmd can generate 'k' internally we could do something like:
'if k.data == NULL => PMD will generate 'k' internally, k.data remains
untouched.'
Another option is to provide user with some callback function to generate CSRN
which could be useful for RSA PSS, OAEP as well (we already discussed that
internally in Intel, I will elaborate on this bit more in different thread).
> +
> + rte_crypto_param r;
> + /**< r component of elliptic curve signature
> + * output : for signature generation
> + * input : for signature verification
> + */
> + rte_crypto_param s;
> + /**< s component of elliptic curve signature
> + * output : for signature generation
> + * input : for signature verification
> + */
[Arek] - Do we want to add any constraints like 'this field should be big
enough to hold...'
> +};
> +
> +/**
> * Asymmetric Cryptographic Operation.
> *
> * Structure describing asymmetric crypto operation params.
> @@ -537,6 +619,7 @@ struct rte_crypto_asym_op {
> struct rte_crypto_mod_op_param modinv;
> struct rte_crypto_dh_op_param dh;
> struct rte_crypto_dsa_op_param dsa;
> + struct rte_crypto_ecdsa_op_param ecdsa;
> };
> };
>
> diff --git a/lib/librte_cryptodev/rte_cryptodev.c
> b/lib/librte_cryptodev/rte_cryptodev.c
> index 89aa2ed..0d6babb 100644
> --- a/lib/librte_cryptodev/rte_cryptodev.c
> +++ b/lib/librte_cryptodev/rte_cryptodev.c
> @@ -173,6 +173,7 @@ const char *rte_crypto_asym_xform_strings[] = {
> [RTE_CRYPTO_
[dpdk-dev] DPDK techboard minutes @18/12/2019
Minutes of Technical Board Meeting, 2019-12-18 Members Attending - -Bruce -Ferruh -Hemant -Honnappa -Jerin -Kevin -Konstantin (Chair) -Maxime -Olivier -Stephen -Thomas NOTE: The technical board meetings every second Wednesday on IRC channel #dpdk-board, at 3pm UTC. Meetings are public and DPDK community members are welcome to attend. NOTE: Next meeting will be on Wednesday 2020-01-15 @3pm UTC, and will be chaired by Maxime Coquelin 1) drivers/vdpa introduction discussion. Currently, at dpdk.org we have only IFC vDPA driver available (located in drivers/net), but more drivers are arriving (Mellanox, Virtio vDPA in v20.02). Might be some others are expected in later releases. Mellanox proposal is to introduce a new vDPA subsystem (drivers/vdpa), and move code common to both net and vdpa PMDs to the drivers/common directory. drivers/net/ifc will be moved entirely into drivers/vdpa/ifc While it seems to provide better code layout for drivers in long run, in short term it probably would need significant code refactoring for both mlx and virtio PMDs, plus can cause some difficulties for stable release maintainers. AR to Maxime (virtio) and Matan (mlx) to estimate how big code refactoring will be required for such move. AR to stable release maintainers (Kevin and Luca) to estimate impact on stable releases. Decision at next TB meeting based on provided estimations/feedback. 2) AR from Thomas to other TB members to submit missing notes for previous TB meeting. 3) AR for Thomas to add the approved email IDs to the security pre-release mailing list. Reply from Thomas: done.
Re: [dpdk-dev] [PATCH] mempool: fix mempool obj alignment for non x86
On Fri, Dec 20, 2019 at 9:25 PM Honnappa Nagarahalli
wrote:
>
>
>
> >
> > From: Jerin Jacob
> >
> > The exiting optimize_object_size() function address the memory object
> > alignment constraint on x86 for better performance.
> >
> > Different (Mirco) architecture may have different memory alignment
> > constraint for better performance and it not same as the existing
> > optimize_object_size() function. Some use, XOR(kind of CRC) scheme to
> > enable DRAM channel distribution based on the address and some may have
> > a different formula.
> If I understand correctly, address interleaving is the characteristic of the
> memory controller and not the CPU.
> For ex: different SoCs using the same Arm architecture might have different
> memory controllers. So, the solution should not be architecture specific, but
> SoC specific.
Yes. See below.
> > -static unsigned optimize_object_size(unsigned obj_size)
> > +static unsigned
> > +arch_mem_object_align(unsigned obj_size)
> > {
> > unsigned nrank, nchan;
> > unsigned new_obj_size;
> > @@ -99,6 +101,13 @@ static unsigned optimize_object_size(unsigned
> > obj_size)
> > new_obj_size++;
> > return new_obj_size * RTE_MEMPOOL_ALIGN; }
> > +#else
> This applies to add Arm (PPC as well) SoCs which might have different schemes
> depending on the memory controller. IMO, this should not be architecture
> specific.
I agree in principle.
I will summarize the
https://www.mail-archive.com/dev@dpdk.org/msg149157.html feedback:
1) For x86 arch, it is architecture-specific
2) For power PC arch, It is architecture-specific
3) For the ARM case, it will be the memory controller specific.
4) For the ARM case, The memory controller is not using the existing
x86 arch formula.
5) If it is memory/arch-specific, Can userspace code find the optimal
alignment? In the case of octeontx2/arm64,
the memory controller does XOR on PA address which userspace code
doesn't have much control.
This patch address the known case of (1), (2), (4) and (5). (2) can
be added to this framework when POWER9 folks want it.
We can extend this patch to address (3) if there is a case. Without
the actual requirement(If some can share the formula of alignment
which is the memory
controller specific and it does not come under (4))) then we can
create extra layer for the memory controller and abstraction to probe
it.
Again there is no standard way of probing the memory controller in
userspace and we need platform #define, which won't work for
distribution build.
So solution needs to be arch-specific and then fine-tune to memory
controller if possible.
I can work on creating an extra layer of code if some can provide the
details of the memory controller and probing mechanism or this
patch be extended to support such case if it arises in future.
Thoughts?
>
> > +static unsigned
> > +arch_mem_object_align(unsigned obj_size) {
> > + return obj_size;
> > +}
> > +#endif
> >
> > struct pagesz_walk_arg {
> > int socket_id;
> > @@ -234,8 +243,8 @@ rte_mempool_calc_obj_size(uint32_t elt_size,
> > uint32_t flags,
> >*/
> > if ((flags & MEMPOOL_F_NO_SPREAD) == 0) {
> > unsigned new_size;
> > - new_size = optimize_object_size(sz->header_size + sz-
> > >elt_size +
> > - sz->trailer_size);
> > + new_size = arch_mem_object_align
> > + (sz->header_size + sz->elt_size +
> > sz->trailer_size);
> > sz->trailer_size = new_size - sz->header_size - sz->elt_size;
> > }
> >
> > --
> > 2.24.1
>
Re: [dpdk-dev] [PATCH] add ABI checks
On Fri, Dec 20, 2019 at 04:20:58PM +0100, David Marchand wrote:
> Starting from Kevin and Bruce idea of using libabigail, here is an
> alternate approach to implement ABI checks.
>
> By default, those checks are disabled and enabling them requires a
> manual step that generates the ABI dumps on a reference version for a
> set of configurations.
>
> Those checks are enabled in the CI by default for the default meson
> options on x86 and aarch64 so that proposed patches are validated.
> A cache of the ABI is stored in travis jobs.
Are they? I'm looking here:
https://travis-ci.com/DPDK/dpdk
And can't for the life of me see where those cached ABI files are pulled from,
or where the checks are encoded at all. Am I missing something (which is the
far greater likelyhood).
assuming they are there, and I'm just not seeing them, do we have external
access to them? Could we download them as part of the abi check process on
local trees?
Neil
> Checks can be only informational by setting ABI_CHECKS_WARN_ONLY when
> breaking the ABI in a future release.
>
> For advanced developers and maintainers, the contributing guide details
> the higher level scripts that are quite close to the existing devtools
> scripts.
>
> Signed-off-by: David Marchand
> ---
> .ci/linux-build.sh | 43 +++
> .travis.yml | 20 +++--
> devtools/check-abi-dump.sh | 46 +
> devtools/check-abi-reference.sh | 27 +
> devtools/dpdk.abignore | 2 ++
> devtools/gen-abi-dump.sh| 29 ++
> devtools/gen-abi-reference.sh | 24 +++
> devtools/test-build.sh | 13 ++--
> devtools/test-meson-builds.sh | 6
> doc/guides/contributing/patches.rst | 25
> 10 files changed, 230 insertions(+), 5 deletions(-)
> create mode 100755 devtools/check-abi-dump.sh
> create mode 100755 devtools/check-abi-reference.sh
> create mode 100644 devtools/dpdk.abignore
> create mode 100755 devtools/gen-abi-dump.sh
> create mode 100755 devtools/gen-abi-reference.sh
>
> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
> index ccc3a7ccd..345dba264 100755
> --- a/.ci/linux-build.sh
> +++ b/.ci/linux-build.sh
> @@ -30,8 +30,51 @@ fi
>
> OPTS="$OPTS --default-library=$DEF_LIB"
> meson build --werror -Dexamples=all $OPTS
> +
> +if [ "$ABI_CHECKS" = "1" ]; then
> +git remote add ref ${REF_GIT_REPO:-https://dpdk.org/git/dpdk}
> +git fetch --tags ref ${REF_GIT_BRANCH:-master}
> +
> +head=$(git describe --all)
> +tag=$(git describe --abbrev=0)
> +
> +if [ "$(cat reference/VERSION 2>/dev/null)" != "$tag" ]; then
> +rm -rf reference
> +fi
> +
> +if [ ! -d reference ]; then
> +gen_abi_dump=$(mktemp -t gen-abi-dump-XXX.sh)
> +cp -a devtools/gen-abi-dump.sh $gen_abi_dump
> +
> +git checkout -qf $tag
> +ninja -C build
> +$gen_abi_dump build reference
> +
> +if [ "$AARCH64" != "1" ]; then
> +mkdir -p reference/app
> +cp -a build/app/dpdk-testpmd reference/app/
> +
> +export LD_LIBRARY_PATH=$(pwd)/build/lib:$(pwd)/build/drivers
> +devtools/test-null.sh reference/app/dpdk-testpmd
> +unset LD_LIBRARY_PATH
> +fi
> +echo $tag > reference/VERSION
> +
> +git checkout -qf $head
> +fi
> +fi
> +
> ninja -C build
>
> +if [ "$ABI_CHECKS" = "1" ]; then
> +devtools/check-abi-dump.sh build reference ${ABI_CHECKS_WARN_ONLY:-}
> +if [ "$AARCH64" != "1" ]; then
> +export LD_LIBRARY_PATH=$(pwd)/build/lib:$(pwd)/build/drivers
> +devtools/test-null.sh reference/app/dpdk-testpmd
> +unset LD_LIBRARY_PATH
> +fi
> +fi
> +
> if [ "$AARCH64" != "1" ]; then
> devtools/test-null.sh
> fi
> diff --git a/.travis.yml b/.travis.yml
> index 8f90d06f2..bbb060fa2 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -1,5 +1,8 @@
> language: c
> -cache: ccache
> +cache:
> + ccache: true
> + directories:
> +- reference
> compiler:
>- gcc
>- clang
> @@ -21,7 +24,7 @@ aarch64_packages: &aarch64_packages
>
> extra_packages: &extra_packages
>- *required_packages
> - - [libbsd-dev, libpcap-dev, libcrypto++-dev, libjansson4]
> + - [libbsd-dev, libpcap-dev, libcrypto++-dev, libjansson4, abigail-tools]
>
> build_32b_packages: &build_32b_packages
>- *required_packages
> @@ -59,6 +62,13 @@ matrix:
>apt:
> packages:
>- *aarch64_packages
> + - env: DEF_LIB="shared" EXTRA_PACKAGES=1 ABI_CHECKS=1 AARCH64=1
> +compiler: gcc
> +addons:
> + apt:
> +packages:
> + - *aarch64_packages
> + - *extra_packages
>- env: DEF_LIB="static" EXTRA_PACKAGES=1
> compiler: gcc
> addons:
> @@ -72,6 +82,12 @@ matrix:
> packages:
>- *extra_packages
>- *doc_packages
Re: [dpdk-dev] [PATCH] add ABI checks
20/12/2019 17:20, Kinsella, Ray:
> > -Original Message-
> > From: Richardson, Bruce
> > Sent: Friday 20 December 2019 15:32
> > To: David Marchand ; dev@dpdk.org
> > Cc: tho...@monjalon.net; Laatz, Kevin ;
> > acon...@redhat.com; nhor...@tuxdriver.com; Michael Santana
> > ; Mcnamara, John ;
> > Kovacevic, Marko ; Kinsella, Ray
> >
> > Subject: RE: [PATCH] add ABI checks
> >
> >
> >
> > > -Original Message-
> > > From: David Marchand
> > > Sent: Friday, December 20, 2019 3:21 PM
> > > To: dev@dpdk.org
> > > Cc: tho...@monjalon.net; Richardson, Bruce
> > > ; Laatz, Kevin ;
> > > acon...@redhat.com; nhor...@tuxdriver.com; Michael Santana
> > > ; Mcnamara, John
> > ;
> > > Kovacevic, Marko
> > > Subject: [PATCH] add ABI checks
> > >
> > > Starting from Kevin and Bruce idea of using libabigail, here is an
> > > alternate approach to implement ABI checks.
> > >
> > > By default, those checks are disabled and enabling them requires a
> > > manual step that generates the ABI dumps on a reference version for a
> > > set of configurations.
> > >
> > > Those checks are enabled in the CI by default for the default meson
> > > options on x86 and aarch64 so that proposed patches are validated.
> > > A cache of the ABI is stored in travis jobs.
> > > Checks can be only informational by setting ABI_CHECKS_WARN_ONLY when
> > > breaking the ABI in a future release.
> > >
> > > For advanced developers and maintainers, the contributing guide
> > > details the higher level scripts that are quite close to the existing
> > > devtools scripts.
> > >
> > > Signed-off-by: David Marchand
> > > ---
> > > .ci/linux-build.sh | 43 +++
> > > .travis.yml | 20 +++--
> > > devtools/check-abi-dump.sh | 46
> > +
> > > devtools/check-abi-reference.sh | 27 +
> > > devtools/dpdk.abignore | 2 ++
> > > devtools/gen-abi-dump.sh| 29 ++
> > > devtools/gen-abi-reference.sh | 24 +++
> > > devtools/test-build.sh | 13 ++--
> > > devtools/test-meson-builds.sh | 6
> > > doc/guides/contributing/patches.rst | 25
> > > 10 files changed, 230 insertions(+), 5 deletions(-) create mode
> > > 100755 devtools/check-abi-dump.sh create mode 100755
> > > devtools/check-abi- reference.sh create mode 100644
> > > devtools/dpdk.abignore create mode
> > > 100755 devtools/gen-abi-dump.sh create mode 100755 devtools/gen-abi-
> > > reference.sh
> > >
> > > diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh index
> > > ccc3a7ccd..345dba264 100755
> > > --- a/.ci/linux-build.sh
> > > +++ b/.ci/linux-build.sh
> > > @@ -30,8 +30,51 @@ fi
> > >
> > > OPTS="$OPTS --default-library=$DEF_LIB"
> > > meson build --werror -Dexamples=all $OPTS
> > > +
> > > +if [ "$ABI_CHECKS" = "1" ]; then
> > > +git remote add ref ${REF_GIT_REPO:-https://dpdk.org/git/dpdk}
> > > +git fetch --tags ref ${REF_GIT_BRANCH:-master}
> > > +
> > > +head=$(git describe --all)
> > > +tag=$(git describe --abbrev=0)
> > > +
> > > +if [ "$(cat reference/VERSION 2>/dev/null)" != "$tag" ]; then
> > > +rm -rf reference
> > > +fi
> > > +
> > > +if [ ! -d reference ]; then
> > > +gen_abi_dump=$(mktemp -t gen-abi-dump-XXX.sh)
> > > +cp -a devtools/gen-abi-dump.sh $gen_abi_dump
> > > +
> > > +git checkout -qf $tag
> > > +ninja -C build
> > > +$gen_abi_dump build reference
> > > +
> > > +if [ "$AARCH64" != "1" ]; then
> > > +mkdir -p reference/app
> > > +cp -a build/app/dpdk-testpmd reference/app/
> > > +
> > > +export
> > LD_LIBRARY_PATH=$(pwd)/build/lib:$(pwd)/build/drivers
> > > +devtools/test-null.sh reference/app/dpdk-testpmd
> > > +unset LD_LIBRARY_PATH
> > > +fi
> > > +echo $tag > reference/VERSION
> > > +
> > > +git checkout -qf $head
> > > +fi
> > > +fi
> > > +
> > > ninja -C build
> > >
> > > +if [ "$ABI_CHECKS" = "1" ]; then
> > > +devtools/check-abi-dump.sh build reference
> > ${ABI_CHECKS_WARN_ONLY:-}
> > > +if [ "$AARCH64" != "1" ]; then
> > > +export LD_LIBRARY_PATH=$(pwd)/build/lib:$(pwd)/build/drivers
> > > +devtools/test-null.sh reference/app/dpdk-testpmd
> > > +unset LD_LIBRARY_PATH
> > > +fi
> > > +fi
> > > +
> > > if [ "$AARCH64" != "1" ]; then
> > > devtools/test-null.sh
> > > fi
> > > diff --git a/.travis.yml b/.travis.yml index 8f90d06f2..bbb060fa2
> > > 100644
> > > --- a/.travis.yml
> > > +++ b/.travis.yml
> > > @@ -1,5 +1,8 @@
> > > language: c
> > > -cache: ccache
> > > +cache:
> > > + ccache: true
> > > + directories:
> > > +- reference
> > > compiler:
> > >- gcc
> > >- clang
> > > @@ -21,7 +24,7 @@ aarch64_packages: &aarch64_packages
> > >
> > > extra_packages: &extra_packages
> > >- *
Re: [dpdk-dev] [PATCH] mempool: fix mempool obj alignment for non x86
> > > From: Jerin Jacob
> > >
> > > The exiting optimize_object_size() function address the memory
> > > object alignment constraint on x86 for better performance.
> > >
> > > Different (Mirco) architecture may have different memory alignment
> > > constraint for better performance and it not same as the existing
> > > optimize_object_size() function. Some use, XOR(kind of CRC) scheme
> > > to enable DRAM channel distribution based on the address and some
> > > may have a different formula.
> > If I understand correctly, address interleaving is the characteristic of the
> memory controller and not the CPU.
> > For ex: different SoCs using the same Arm architecture might have different
> memory controllers. So, the solution should not be architecture specific, but
> SoC specific.
>
> Yes. See below.
>
> > > -static unsigned optimize_object_size(unsigned obj_size)
> > > +static unsigned
> > > +arch_mem_object_align(unsigned obj_size)
> > > {
> > > unsigned nrank, nchan;
> > > unsigned new_obj_size;
> > > @@ -99,6 +101,13 @@ static unsigned optimize_object_size(unsigned
> > > obj_size)
> > > new_obj_size++;
> > > return new_obj_size * RTE_MEMPOOL_ALIGN; }
> > > +#else
> > This applies to add Arm (PPC as well) SoCs which might have different
> schemes depending on the memory controller. IMO, this should not be
> architecture specific.
>
> I agree in principle.
> I will summarize the
> https://www.mail-archive.com/dev@dpdk.org/msg149157.html feedback:
>
> 1) For x86 arch, it is architecture-specific
> 2) For power PC arch, It is architecture-specific
> 3) For the ARM case, it will be the memory controller specific.
> 4) For the ARM case, The memory controller is not using the existing
> x86 arch formula.
> 5) If it is memory/arch-specific, Can userspace code find the optimal
> alignment? In the case of octeontx2/arm64, the memory controller does XOR
> on PA address which userspace code doesn't have much control.
>
> This patch address the known case of (1), (2), (4) and (5). (2) can be added
> to
> this framework when POWER9 folks want it.
>
> We can extend this patch to address (3) if there is a case. Without the actual
> requirement(If some can share the formula of alignment which is the
> memory controller specific and it does not come under (4))) then we can
> create extra layer for the memory controller and abstraction to probe it.
> Again there is no standard way of probing the memory controller in
> userspace and we need platform #define, which won't work for distribution
> build.
> So solution needs to be arch-specific and then fine-tune to memory controller
> if possible.
>
> I can work on creating an extra layer of code if some can provide the details
> of the memory controller and probing mechanism or this patch be extended
Inputs for BlueField, DPAAx, ThunderX2 would be helpful.
> to support such case if it arises in future.
>
> Thoughts?
How much memory will this save for your platform? Is it affecting performance?
>
> >
> > > +static unsigned
> > > +arch_mem_object_align(unsigned obj_size) {
> > > + return obj_size;
> > > +}
> > > +#endif
> > >
> > > struct pagesz_walk_arg {
> > > int socket_id;
> > > @@ -234,8 +243,8 @@ rte_mempool_calc_obj_size(uint32_t elt_size,
> > > uint32_t flags,
> > >*/
> > > if ((flags & MEMPOOL_F_NO_SPREAD) == 0) {
> > > unsigned new_size;
> > > - new_size = optimize_object_size(sz->header_size + sz-
> > > >elt_size +
> > > - sz->trailer_size);
> > > + new_size = arch_mem_object_align
> > > + (sz->header_size + sz->elt_size +
> > > + sz->trailer_size);
> > > sz->trailer_size = new_size - sz->header_size -
> > > sz->elt_size;
> > > }
> > >
> > > --
> > > 2.24.1
> >
[dpdk-dev] KNI portmask bugs
KNI example has a couple of bugs that can cause surprises if using failsafe (or bonding). The portmask argument to kni does not check that device is not a sub device (already owned) and allows any device. While your at it, the portmask for kni is only 32 bits which is not big enough on some systems.
Re: [dpdk-dev] [PATCH v1] rte_timer: add rte_timer_next_ticks
Hi Stephen,
I added a comment in-line. With that change, it looks good to me:
Acked-by: Erik Gabriel Carrillo
Regards,
Erik
> -Original Message-
> From: Stephen Hemminger
> Sent: Monday, December 16, 2019 6:55 PM
> To: dev@dpdk.org
> Cc: Carrillo, Erik G ; Stephen Hemminger
>
> Subject: [PATCH v1] rte_timer: add rte_timer_next_ticks
>
> It is useful to know when the next timer will expire when using
> rte_epoll_wait (or sleep when idle). This experimental API provides a hook
> to query the number of ticks remaining.
>
> Signed-off-by: Stephen Hemminger
> ---
> v1 - incorporate feedback from old RFC
>
> lib/librte_timer/rte_timer.c | 27 ++
> lib/librte_timer/rte_timer.h | 16 +++
> lib/librte_timer/rte_timer_version.map | 1 +
> 3 files changed, 44 insertions(+)
>
> diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c index
> ca88454ff68a..a1ed186cf591 100644
> --- a/lib/librte_timer/rte_timer.c
> +++ b/lib/librte_timer/rte_timer.c
> @@ -978,6 +978,33 @@ rte_timer_stop_all(uint32_t timer_data_id,
> unsigned int *walk_lcores,
> return 0;
> }
>
> +int64_t
> +rte_timer_next_ticks(void)
> +{
> + unsigned int lcore_id = rte_lcore_id();
> + struct rte_timer_data *timer_data;
> + struct priv_timer *priv_timer;
> + const struct rte_timer *tm;
> + uint64_t cur_time;
> + int64_t left = -ENOENT;
> +
> + TIMER_DATA_VALID_GET_OR_ERR_RET(default_data_id,
> timer_data, -EINVAL);
> +
> + priv_timer = timer_data->priv_timer;
> + cur_time = rte_get_timer_cycles();
> +
> + rte_spinlock_lock(&priv_timer[lcore_id].list_lock);
> + tm = priv_timer[lcore_id].pending_head.sl_next[0];
> + if (tm) {
> + left = tm->expire - cur_time;
> + if (left < 0)
> + left = 0;
> + }
> + rte_spinlock_unlock(&priv_timer[lcore_id].list_lock);
> +
> + return left;
> +}
> +
> /* dump statistics about timers */
> static void
> __rte_timer_dump_stats(struct rte_timer_data *timer_data __rte_unused,
> FILE *f) diff --git a/lib/librte_timer/rte_timer.h
> b/lib/librte_timer/rte_timer.h
> index 9dc5fc309249..c5031d0448ba 100644
> --- a/lib/librte_timer/rte_timer.h
> +++ b/lib/librte_timer/rte_timer.h
> @@ -331,6 +331,22 @@ void rte_timer_stop_sync(struct rte_timer *tim);
> */
> int rte_timer_pending(struct rte_timer *tim);
>
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Time until the next timer
s/next timer/next timer on the current lcore
> + * This function gives the ticks until the next timer will be active.
> + *
> + * @return
> + * - -EINVAL: invalid timer data instance identifier
> + * - -ENOENT: no timer pending
> + * - 0: a timer is pending and will run at next rte_timer_manage()
> + * - >0: ticks until the next timer is ready
> + */
> +__rte_experimental
> +int64_t rte_timer_next_ticks(void);
> +
> /**
> * Manage the timer list and execute callback functions.
> *
> diff --git a/lib/librte_timer/rte_timer_version.map
> b/lib/librte_timer/rte_timer_version.map
> index 2a59d3f081c4..4471cef92be5 100644
> --- a/lib/librte_timer/rte_timer_version.map
> +++ b/lib/librte_timer/rte_timer_version.map
> @@ -23,6 +23,7 @@ EXPERIMENTAL {
> rte_timer_alt_stop;
> rte_timer_data_alloc;
> rte_timer_data_dealloc;
> + rte_timer_next_ticks;
> rte_timer_stop_all;
> rte_timer_subsystem_finalize;
> };
> --
> 2.20.1
[dpdk-dev] [PATCH] kni: rename variable
All global variables in kernel should be prefixed by the same
to avoid any symbol conflics. Rename dflt_carrier to kni_default_carrier.
Fixes: 89397a01ce4a ("kni: set default carrier state of interface")
Cc: d...@adax.com
Cc: sta...@dpdk.org
Signed-off-by: Stephen Hemminger
---
kernel/linux/kni/kni_dev.h | 2 +-
kernel/linux/kni/kni_misc.c | 10 +-
kernel/linux/kni/kni_net.c | 2 +-
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h
index 5e75c6371f22..ca5f92a47b70 100644
--- a/kernel/linux/kni/kni_dev.h
+++ b/kernel/linux/kni/kni_dev.h
@@ -32,7 +32,7 @@
#define MBUF_BURST_SZ 32
/* Default carrier state for created KNI network interfaces */
-extern uint32_t dflt_carrier;
+extern uint32_t kni_dflt_carrier;
/**
* A structure describing the private information for a kni device.
diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
index cda71bde08e1..2b464c438113 100644
--- a/kernel/linux/kni/kni_misc.c
+++ b/kernel/linux/kni/kni_misc.c
@@ -39,7 +39,7 @@ static uint32_t multiple_kthread_on;
/* Default carrier state for created KNI network interfaces */
static char *carrier;
-uint32_t dflt_carrier;
+uint32_t kni_dflt_carrier;
#define KNI_DEV_IN_USE_BIT_NUM 0 /* Bit number for device in use */
@@ -554,14 +554,14 @@ static int __init
kni_parse_carrier_state(void)
{
if (!carrier) {
- dflt_carrier = 0;
+ kni_dflt_carrier = 0;
return 0;
}
if (strcmp(carrier, "off") == 0)
- dflt_carrier = 0;
+ kni_dflt_carrier = 0;
else if (strcmp(carrier, "on") == 0)
- dflt_carrier = 1;
+ kni_dflt_carrier = 1;
else
return -1;
@@ -588,7 +588,7 @@ kni_init(void)
return -EINVAL;
}
- if (dflt_carrier == 0)
+ if (kni_dflt_carrier == 0)
pr_debug("Default carrier state set to off.\n");
else
pr_debug("Default carrier state set to on.\n");
diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c
index 1ba9b1b99f66..97fe85be9ac6 100644
--- a/kernel/linux/kni/kni_net.c
+++ b/kernel/linux/kni/kni_net.c
@@ -158,7 +158,7 @@ kni_net_open(struct net_device *dev)
struct kni_dev *kni = netdev_priv(dev);
netif_start_queue(dev);
- if (dflt_carrier == 1)
+ if (kni_dflt_carrier == 1)
netif_carrier_on(dev);
else
netif_carrier_off(dev);
--
2.20.1
[dpdk-dev] [PATCH] net/qede/base: fix number of ports per engine
Fix the way in which the number of ports per engine of an adapter is
determined by reading port mode register. Set default value to 1.
Fixes: 3b307c55f2ac ("net/qede/base: update FW to 8.40.25.0")
Cc: sta...@dpdk.com
Signed-off-by: Rasesh Mody
---
drivers/net/qede/base/ecore_dev.c | 38 ++-
1 file changed, 27 insertions(+), 11 deletions(-)
diff --git a/drivers/net/qede/base/ecore_dev.c
b/drivers/net/qede/base/ecore_dev.c
index 9d1db14590..f33b9910c0 100644
--- a/drivers/net/qede/base/ecore_dev.c
+++ b/drivers/net/qede/base/ecore_dev.c
@@ -5253,7 +5253,6 @@ static void ecore_emul_hw_info_port_num(struct ecore_hwfn
*p_hwfn,
/* MISCS_REG_ECO_RESERVED[15:12]: num of ports in an engine */
eco_reserved = ecore_rd(p_hwfn, p_ptt, MISCS_REG_ECO_RESERVED);
-
switch ((eco_reserved & 0xf000) >> 12) {
case 1:
p_dev->num_ports_in_engine = 1;
@@ -5268,7 +5267,7 @@ static void ecore_emul_hw_info_port_num(struct ecore_hwfn
*p_hwfn,
DP_NOTICE(p_hwfn, false,
"Emulation: Unknown port mode [ECO_RESERVED
0x%08x]\n",
eco_reserved);
- p_dev->num_ports_in_engine = 2; /* Default to something */
+ p_dev->num_ports_in_engine = 1; /* Default to something */
break;
}
@@ -5281,8 +5280,8 @@ static void ecore_emul_hw_info_port_num(struct ecore_hwfn
*p_hwfn,
static void ecore_hw_info_port_num(struct ecore_hwfn *p_hwfn,
struct ecore_ptt *p_ptt)
{
+ u32 addr, global_offsize, global_addr, port_mode;
struct ecore_dev *p_dev = p_hwfn->p_dev;
- u32 addr, global_offsize, global_addr;
#ifndef ASIC_ONLY
if (CHIP_REV_IS_TEDIBEAR(p_dev)) {
@@ -5304,15 +5303,32 @@ static void ecore_hw_info_port_num(struct ecore_hwfn
*p_hwfn,
return;
}
- addr = SECTION_OFFSIZE_ADDR(p_hwfn->mcp_info->public_base,
- PUBLIC_GLOBAL);
- global_offsize = ecore_rd(p_hwfn, p_ptt, addr);
- global_addr = SECTION_ADDR(global_offsize, 0);
- addr = global_addr + OFFSETOF(struct public_global, max_ports);
- p_dev->num_ports = (u8)ecore_rd(p_hwfn, p_ptt, addr);
+ /* Determine the number of ports per engine */
+ port_mode = ecore_rd(p_hwfn, p_ptt, MISC_REG_PORT_MODE);
+ switch (port_mode) {
+ case 0x0:
+ p_dev->num_ports_in_engine = 1;
+ break;
+ case 0x1:
+ p_dev->num_ports_in_engine = 2;
+ break;
+ case 0x2:
+ p_dev->num_ports_in_engine = 4;
+ break;
+ default:
+ DP_NOTICE(p_hwfn, false, "Unknown port mode 0x%08x\n",
+ port_mode);
+ p_dev->num_ports_in_engine = 1; /* Default to something */
+ break;
+ }
- p_dev->num_ports_in_engine = p_dev->num_ports >>
-(ecore_device_num_engines(p_dev) - 1);
+ /* Get the total number of ports of the device */
+ addr = SECTION_OFFSIZE_ADDR(p_hwfn->mcp_info->public_base,
+ PUBLIC_GLOBAL);
+ global_offsize = ecore_rd(p_hwfn, p_ptt, addr);
+ global_addr = SECTION_ADDR(global_offsize, 0);
+ addr = global_addr + OFFSETOF(struct public_global, max_ports);
+ p_dev->num_ports = (u8)ecore_rd(p_hwfn, p_ptt, addr);
}
static void ecore_mcp_get_eee_caps(struct ecore_hwfn *p_hwfn,
--
2.18.0
[dpdk-dev] [PATCH] net/bnx2x: add support for secondary process
Skip the device re-initialization for secondary process.
Cc: sta...@dpdk.com
Signed-off-by: Rasesh Mody
---
drivers/net/bnx2x/bnx2x_ethdev.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/net/bnx2x/bnx2x_ethdev.c b/drivers/net/bnx2x/bnx2x_ethdev.c
index 20b045ff87..7864b5b80a 100644
--- a/drivers/net/bnx2x/bnx2x_ethdev.c
+++ b/drivers/net/bnx2x/bnx2x_ethdev.c
@@ -598,6 +598,11 @@ bnx2x_common_dev_init(struct rte_eth_dev *eth_dev, int
is_vf)
eth_dev->dev_ops = is_vf ? &bnx2xvf_eth_dev_ops : &bnx2x_eth_dev_ops;
+ if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
+ PMD_DRV_LOG(ERR, sc, "Skipping device init from secondary
process");
+ return 0;
+ }
+
rte_eth_copy_pci_info(eth_dev, pci_dev);
sc->pcie_bus= pci_dev->addr.bus;
--
2.18.0
[dpdk-dev] [PATCH v2 1/5] net/bnxt: fix link failure during port toggle
From: Santoshkumar Karanappa Rastapur
We need to wait for up to 500ms to receive async event notification after
forcing link down. Similarly we need to wait for up to 10s for link to
come up after configuring the hardware for link up.
Fixes: c09f57b49c13 ("net/bnxt: add start/stop/link update operations")
Cc: sta...@dpdk.org
Signed-off-by: Santoshkumar Karanappa Rastapur
Signed-off-by: Somnath Kotur
---
drivers/net/bnxt/bnxt.h| 10 ++
drivers/net/bnxt/bnxt_cpr.c| 2 +-
drivers/net/bnxt/bnxt_ethdev.c | 18 +-
3 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index ab18e8acd..ab0b8dde1 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -231,9 +231,10 @@ struct bnxt_pf_info {
uint8_t evb_mode;
};
-/* Max wait time is 10 * 100ms = 1s */
-#define BNXT_LINK_WAIT_CNT 10
-#define BNXT_LINK_WAIT_INTERVAL100
+/* Max wait time for link up is 10s and link down is 500ms */
+#define BNXT_LINK_UP_WAIT_CNT 200
+#define BNXT_LINK_DOWN_WAIT_CNT10
+#define BNXT_LINK_WAIT_INTERVAL50
struct bnxt_link_info {
uint32_tphy_flags;
uint8_t mac_type;
@@ -656,7 +657,8 @@ struct bnxt {
};
int bnxt_mtu_set_op(struct rte_eth_dev *eth_dev, uint16_t new_mtu);
-int bnxt_link_update_op(struct rte_eth_dev *eth_dev, int wait_to_complete);
+int bnxt_link_update(struct rte_eth_dev *eth_dev, int wait_to_complete,
+bool exp_link_status);
int bnxt_rcv_msg_from_vf(struct bnxt *bp, uint16_t vf_id, void *msg);
int is_bnxt_in_error(struct bnxt *bp);
uint16_t bnxt_rss_ctxts(const struct bnxt *bp);
diff --git a/drivers/net/bnxt/bnxt_cpr.c b/drivers/net/bnxt/bnxt_cpr.c
index 2c3129fe2..bb316b9e0 100644
--- a/drivers/net/bnxt/bnxt_cpr.c
+++ b/drivers/net/bnxt/bnxt_cpr.c
@@ -63,7 +63,7 @@ void bnxt_handle_async_event(struct bnxt *bp,
case HWRM_ASYNC_EVENT_CMPL_EVENT_ID_LINK_SPEED_CHANGE:
case HWRM_ASYNC_EVENT_CMPL_EVENT_ID_LINK_SPEED_CFG_CHANGE:
/* FALLTHROUGH */
- bnxt_link_update_op(bp->eth_dev, 0);
+ bnxt_link_update(bp->eth_dev, 0, ETH_LINK_UP);
break;
case HWRM_ASYNC_EVENT_CMPL_EVENT_ID_PF_DRVR_UNLOAD:
PMD_DRV_LOG(INFO, "Async event: PF driver unloaded\n");
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 1b4ed293d..88df82b86 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -856,7 +856,7 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
eth_dev->data->scattered_rx = bnxt_scattered_rx(eth_dev);
- bnxt_link_update_op(eth_dev, 1);
+ bnxt_link_update(eth_dev, 1, ETH_LINK_UP);
if (rx_offloads & DEV_RX_OFFLOAD_VLAN_FILTER)
vlan_mask |= ETH_VLAN_FILTER_MASK;
@@ -940,7 +940,7 @@ static void bnxt_dev_stop_op(struct rte_eth_dev *eth_dev)
* During reset recovery, there is no need to wait
*/
if (!is_bnxt_in_error(bp))
- rte_delay_ms(BNXT_LINK_WAIT_INTERVAL * 2);
+ bnxt_link_update(eth_dev, 1, ETH_LINK_DOWN);
/* Clean queue intr-vector mapping */
rte_intr_efd_disable(intr_handle);
@@ -1086,12 +1086,14 @@ static int bnxt_mac_addr_add_op(struct rte_eth_dev
*eth_dev,
return rc;
}
-int bnxt_link_update_op(struct rte_eth_dev *eth_dev, int wait_to_complete)
+int bnxt_link_update(struct rte_eth_dev *eth_dev, int wait_to_complete,
+bool exp_link_status)
{
int rc = 0;
struct bnxt *bp = eth_dev->data->dev_private;
struct rte_eth_link new;
- unsigned int cnt = BNXT_LINK_WAIT_CNT;
+ int cnt = exp_link_status ? BNXT_LINK_UP_WAIT_CNT :
+ BNXT_LINK_DOWN_WAIT_CNT;
rc = is_bnxt_in_error(bp);
if (rc)
@@ -1109,7 +,7 @@ int bnxt_link_update_op(struct rte_eth_dev *eth_dev, int
wait_to_complete)
goto out;
}
- if (!wait_to_complete || new.link_status)
+ if (!wait_to_complete || new.link_status == exp_link_status)
break;
rte_delay_ms(BNXT_LINK_WAIT_INTERVAL);
@@ -1131,6 +1133,12 @@ int bnxt_link_update_op(struct rte_eth_dev *eth_dev, int
wait_to_complete)
return rc;
}
+static int bnxt_link_update_op(struct rte_eth_dev *eth_dev,
+ int wait_to_complete)
+{
+ return bnxt_link_update(eth_dev, wait_to_complete, ETH_LINK_UP);
+}
+
static int bnxt_promiscuous_enable_op(struct rte_eth_dev *eth_dev)
{
struct bnxt *bp = eth_dev->data->dev_private;
--
2.21.0 (Apple Git-122.2)
[dpdk-dev] [PATCH v2 2/5] net/bnxt: fix to use first valid profile
From: Somnath Kotur
In the case when CoS classification is disabled, driver was iterating
looking for only lossy profiles as that is what is expected to be used
for regular NIC operations. But in certain custom profiles, there were
no lossy profiles configured, only lossless profiles instead.
To handle such cases, it is better to fallback to using the first valid
profile.
Fixes: 698aa7e95325 ("net/bnxt: add code to determine the Tx COS queue")
Cc: sta...@dpdk.org
Signed-off-by: Somnath Kotur
Reviewed-by: Ajit Khaparde
---
drivers/net/bnxt/bnxt_hwrm.c | 44 +---
drivers/net/bnxt/bnxt_hwrm.h | 3 +++
2 files changed, 39 insertions(+), 8 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index d0dcd561c..bee4c154f 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -1210,6 +1210,35 @@ static int bnxt_hwrm_port_phy_qcfg(struct bnxt *bp,
return rc;
}
+static bool bnxt_find_lossy_profile(struct bnxt *bp)
+{
+ int i = 0;
+
+ for (i = BNXT_COS_QUEUE_COUNT - 1; i >= 0; i--) {
+ if (bp->tx_cos_queue[i].profile ==
+ HWRM_QUEUE_SERVICE_PROFILE_LOSSY) {
+ bp->tx_cosq_id[0] = bp->tx_cos_queue[i].id;
+ return true;
+ }
+ }
+ return false;
+}
+
+static void bnxt_find_first_valid_profile(struct bnxt *bp)
+{
+ int i = 0;
+
+ for (i = BNXT_COS_QUEUE_COUNT - 1; i >= 0; i--) {
+ if (bp->tx_cos_queue[i].profile !=
+ HWRM_QUEUE_SERVICE_PROFILE_UNKNOWN &&
+ bp->tx_cos_queue[i].id !=
+ HWRM_QUEUE_SERVICE_PROFILE_UNKNOWN) {
+ bp->tx_cosq_id[0] = bp->tx_cos_queue[i].id;
+ break;
+ }
+ }
+}
+
int bnxt_hwrm_queue_qportcfg(struct bnxt *bp)
{
int rc = 0;
@@ -1269,14 +1298,13 @@ int bnxt_hwrm_queue_qportcfg(struct bnxt *bp)
bp->tx_cos_queue[i].id;
}
} else {
- for (i = BNXT_COS_QUEUE_COUNT - 1; i >= 0; i--) {
- if (bp->tx_cos_queue[i].profile ==
- HWRM_QUEUE_SERVICE_PROFILE_LOSSY) {
- bp->tx_cosq_id[0] =
- bp->tx_cos_queue[i].id;
- break;
- }
- }
+ /* When CoS classification is disabled, for normal NIC
+* operations, ideally we should look to use LOSSY.
+* If not found, fallback to the first valid profile
+*/
+ if (!bnxt_find_lossy_profile(bp))
+ bnxt_find_first_valid_profile(bp);
+
}
}
diff --git a/drivers/net/bnxt/bnxt_hwrm.h b/drivers/net/bnxt/bnxt_hwrm.h
index abe5de9db..d8d1360f9 100644
--- a/drivers/net/bnxt/bnxt_hwrm.h
+++ b/drivers/net/bnxt/bnxt_hwrm.h
@@ -35,6 +35,9 @@ struct bnxt_cp_ring_info;
#define HWRM_QUEUE_SERVICE_PROFILE_LOSSY \
HWRM_QUEUE_QPORTCFG_OUTPUT_QUEUE_ID0_SERVICE_PROFILE_LOSSY
+#define HWRM_QUEUE_SERVICE_PROFILE_UNKNOWN \
+ HWRM_QUEUE_QPORTCFG_OUTPUT_QUEUE_ID0_SERVICE_PROFILE_UNKNOWN
+
#define HWRM_FUNC_RESOURCE_QCAPS_OUTPUT_VF_RESV_STRATEGY_MINIMAL_STATIC \
HWRM_FUNC_RESOURCE_QCAPS_OUTPUT_VF_RESERVATION_STRATEGY_MINIMAL_STATIC
#define HWRM_FUNC_RESOURCE_QCAPS_OUTPUT_VF_RESV_STRATEGY_MAXIMAL \
--
2.21.0 (Apple Git-122.2)
[dpdk-dev] [PATCH v2 0/5] bnxt patchset
Fixes for bnxt PMD. v1->v2: - Updated Fixes tag and commit logs - Rebased patch 4,5 - Squashed patch 5 and 6 Santoshkumar Karanappa Rastapur (2): net/bnxt: fix link failure during port toggle net/bnxt: fix non matching flow hitting filter rule Somnath Kotur (3): net/bnxt: fix to use first valid profile net/bnxt: fix flow flush to sync with flow destroy net/bnxt: fix to reuse an L2 filter drivers/net/bnxt/bnxt.h| 10 +- drivers/net/bnxt/bnxt_cpr.c| 2 +- drivers/net/bnxt/bnxt_ethdev.c | 18 +++- drivers/net/bnxt/bnxt_filter.h | 4 + drivers/net/bnxt/bnxt_flow.c | 163 + drivers/net/bnxt/bnxt_hwrm.c | 64 ++--- drivers/net/bnxt/bnxt_hwrm.h | 3 + 7 files changed, 143 insertions(+), 121 deletions(-) -- 2.21.0 (Apple Git-122.2)
[dpdk-dev] [PATCH v2 3/5] net/bnxt: fix flow flush to sync with flow destroy
From: Somnath Kotur
Sync flow flush routine with flow destroy so that the operations
performed per flow during a flush are the same as that are done for an
individual flow destroy by having a common function to call for both.
One of the things that was missed in the flow flush routine was the
deletion of the L2 filter that would have been created as part of an
n-tuple filter.
Also, decrement the l2_ref_cnt for a filter in the case of a filter
update as it would've bumped up previously in validate_and_parse_flow()
Fixes: 89278c59d97c58 ("net/bnxt: fix flow flush handling")
Cc: sta...@dpdk.org
Signed-off-by: Somnath Kotur
Reviewed-by: Santoshkumar Karanappa Rastapur
---
drivers/net/bnxt/bnxt_flow.c | 132 ---
1 file changed, 46 insertions(+), 86 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index 7bd6811f1..7343b7e7b 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -1536,10 +1536,13 @@ bnxt_update_filter(struct bnxt *bp, struct
bnxt_filter_info *old_filter,
* filter which points to the new destination queue and so we clear
* the previous L2 filter. For ntuple filters, we are going to reuse
* the old L2 filter and create new NTUPLE filter with this new
-* destination queue subsequently during bnxt_flow_create.
+* destination queue subsequently during bnxt_flow_create. So we
+* decrement the ref cnt of the L2 filter that would've been bumped
+* up previously in bnxt_validate_and_parse_flow as the old n-tuple
+* filter that was referencing it will be deleted now.
*/
+ bnxt_hwrm_clear_l2_filter(bp, old_filter);
if (new_filter->filter_type == HWRM_CFA_L2_FILTER) {
- bnxt_hwrm_clear_l2_filter(bp, old_filter);
bnxt_hwrm_set_l2_filter(bp, new_filter->dst_id, new_filter);
} else {
if (new_filter->filter_type == HWRM_CFA_EM_FILTER)
@@ -1816,46 +1819,24 @@ static int bnxt_handle_tunnel_redirect_destroy(struct
bnxt *bp,
}
static int
-bnxt_flow_destroy(struct rte_eth_dev *dev,
- struct rte_flow *flow,
- struct rte_flow_error *error)
+_bnxt_flow_destroy(struct bnxt *bp,
+ struct rte_flow *flow,
+ struct rte_flow_error *error)
{
- struct bnxt *bp = dev->data->dev_private;
struct bnxt_filter_info *filter;
struct bnxt_vnic_info *vnic;
int ret = 0;
- bnxt_acquire_flow_lock(bp);
- if (!flow) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
- "Invalid flow: failed to destroy flow.");
- bnxt_release_flow_lock(bp);
- return -EINVAL;
- }
-
filter = flow->filter;
vnic = flow->vnic;
- if (!filter) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
- "Invalid flow: failed to destroy flow.");
- bnxt_release_flow_lock(bp);
- return -EINVAL;
- }
-
if (filter->filter_type == HWRM_CFA_TUNNEL_REDIRECT_FILTER &&
filter->enables == filter->tunnel_type) {
- ret = bnxt_handle_tunnel_redirect_destroy(bp,
- filter,
- error);
- if (!ret) {
+ ret = bnxt_handle_tunnel_redirect_destroy(bp, filter, error);
+ if (!ret)
goto done;
- } else {
- bnxt_release_flow_lock(bp);
+ else
return ret;
- }
}
ret = bnxt_match_filter(bp, filter);
@@ -1902,7 +1883,36 @@ bnxt_flow_destroy(struct rte_eth_dev *dev,
"Failed to destroy flow.");
}
+ return ret;
+}
+
+static int
+bnxt_flow_destroy(struct rte_eth_dev *dev,
+ struct rte_flow *flow,
+ struct rte_flow_error *error)
+{
+ struct bnxt *bp = dev->data->dev_private;
+ int ret = 0;
+
+ bnxt_acquire_flow_lock(bp);
+ if (!flow) {
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
+ "Invalid flow: failed to destroy flow.");
+ bnxt_release_flow_lock(bp);
+ return -EINVAL;
+ }
+
+ if (!flow->filter) {
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
+ "Invalid flow: failed to destroy flow.");
+ bnxt_release_flow_lock(bp);
+ return -EINVAL;
+ }
+ ret = _bnxt_flow_destr
[dpdk-dev] [PATCH v2 5/5] net/bnxt: fix to reuse an L2 filter
From: Somnath Kotur
The software L2 filter was being released back to the free pool,
though it was created in HW and the filter corresponding to an actual
'flow' would have reference to the HW L2 filter.
But if this 'flow were to be deleted, then this HW L2 filter also
would be gone.
Fix this by storing the L2 filter created originally either for an
n-tuple flow or otherwise as part of the vnic's filter list.
This would require the filter_info struct to have a backptr to the
vnic which it came from.
Now that L2 filters can be re-used for an n-tuple filter(s), delete
L2 filter as well so the reference count of an L2 filter (if reused)
can be decremented appropriately.
Fixes: 5c1171c97216 ("net/bnxt: refactor filter/flow")
Cc: sta...@dpdk.org
Signed-off-by: Somnath Kotur
Reviewed-by: Ajit Khaparde
---
drivers/net/bnxt/bnxt_filter.h | 4
drivers/net/bnxt/bnxt_flow.c | 20 +---
drivers/net/bnxt/bnxt_hwrm.c | 20 +++-
3 files changed, 28 insertions(+), 16 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_filter.h b/drivers/net/bnxt/bnxt_filter.h
index 9db3e7487..fc40f112b 100644
--- a/drivers/net/bnxt/bnxt_filter.h
+++ b/drivers/net/bnxt/bnxt_filter.h
@@ -77,6 +77,10 @@ struct bnxt_filter_info {
uint16_tip_addr_type;
uint16_tethertype;
uint32_tpriority;
+ /* Backptr to vnic. As of now, used only by an L2 filter
+* to remember which vnic it was created on
+*/
+ struct bnxt_vnic_info *vnic;
};
struct bnxt_filter_info *bnxt_alloc_filter(struct bnxt *bp);
diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index 855994a6b..2bd492ea3 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -746,10 +746,9 @@ bnxt_find_matching_l2_filter(struct bnxt *bp, struct
bnxt_filter_info *nf)
{
struct bnxt_filter_info *mf, *f0;
struct bnxt_vnic_info *vnic0;
- struct rte_flow *flow;
int i;
- vnic0 = &bp->vnic_info[0];
+ vnic0 = BNXT_GET_DEFAULT_VNIC(bp);
f0 = STAILQ_FIRST(&vnic0->filter);
/* This flow has same DST MAC as the port/l2 filter. */
@@ -762,8 +761,7 @@ bnxt_find_matching_l2_filter(struct bnxt *bp, struct
bnxt_filter_info *nf)
if (vnic->fw_vnic_id == INVALID_VNIC_ID)
continue;
- STAILQ_FOREACH(flow, &vnic->flow_list, next) {
- mf = flow->filter;
+ STAILQ_FOREACH(mf, &vnic->filter, next) {
if (mf->matching_l2_fltr_ptr)
continue;
@@ -798,6 +796,8 @@ bnxt_create_l2_filter(struct bnxt *bp, struct
bnxt_filter_info *nf,
if (filter1 == NULL)
return NULL;
+ memcpy(filter1, nf, sizeof(*filter1));
+
filter1->flags = HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_XDP_DISABLE;
filter1->flags |= HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_PATH_RX;
if (nf->valid_flags & BNXT_FLOW_L2_SRC_VALID_FLAG ||
@@ -880,11 +880,14 @@ bnxt_get_l2_filter(struct bnxt *bp, struct
bnxt_filter_info *nf,
l2_filter = bnxt_find_matching_l2_filter(bp, nf);
if (l2_filter) {
l2_filter->l2_ref_cnt++;
- nf->matching_l2_fltr_ptr = l2_filter;
} else {
l2_filter = bnxt_create_l2_filter(bp, nf, vnic);
- nf->matching_l2_fltr_ptr = NULL;
+ if (l2_filter) {
+ STAILQ_INSERT_TAIL(&vnic->filter, l2_filter, next);
+ l2_filter->vnic = vnic;
+ }
}
+ nf->matching_l2_fltr_ptr = l2_filter;
return l2_filter;
}
@@ -1429,11 +1432,6 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
goto ret;
}
- if (filter1 && !filter->matching_l2_fltr_ptr) {
- bnxt_free_filter(bp, filter1);
- filter1->fw_l2_filter_id = -1;
- }
-
done:
act = bnxt_flow_non_void_action(++act);
if (act->type != RTE_FLOW_ACTION_TYPE_END) {
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index bee4c154f..8178213e5 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -363,10 +363,11 @@ int bnxt_hwrm_cfa_vlan_antispoof_cfg(struct bnxt *bp,
uint16_t fid,
}
int bnxt_hwrm_clear_l2_filter(struct bnxt *bp,
- struct bnxt_filter_info *filter)
+struct bnxt_filter_info *filter)
{
int rc = 0;
struct bnxt_filter_info *l2_filter = filter;
+ struct bnxt_vnic_info *vnic = NULL;
struct hwrm_cfa_l2_filter_free_input req = {.req_type = 0 };
struct hwrm_cfa_l2_filter_free_output *resp = bp->hwrm_cmd_resp_addr;
@@ -379,6 +380,9 @@ int bnxt_hwrm_clear_l2_filter(struct bnxt *bp,
PMD_DRV_LOG(DEBUG, "filter: %p l2_filter: %p ref_cnt: %d\n",
[dpdk-dev] [PATCH v2 4/5] net/bnxt: fix non matching flow hitting filter rule
From: Santoshkumar Karanappa Rastapur
As part of ntuple filter, we were creating L2 filter with the ntuple
redirect queue resulting in any L2 matching flow getting steered to
this queue. For ntuple filters, we need to create the L2 filter with
default queue. The user specified redirect queue will be set while
creating the ntuple filter in hardware.
Fixes: 5c1171c97216 ("net/bnxt: refactor filter/flow")
Cc: sta...@dpdk.org
Signed-off-by: Santoshkumar Karanappa Rastapur
Signed-off-by: Somnath Kotur
---
drivers/net/bnxt/bnxt_flow.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index 7343b7e7b..855994a6b 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -1126,7 +1126,16 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
PMD_DRV_LOG(DEBUG,
"Setting vnic ff_idx %d\n", vnic->ff_pool_idx);
filter->dst_id = vnic->fw_vnic_id;
- filter1 = bnxt_get_l2_filter(bp, filter, vnic);
+
+ /* For ntuple filter, create the L2 filter with default VNIC.
+* The user specified redirect queue will be set while creating
+* the ntuple filter in hardware.
+*/
+ vnic0 = BNXT_GET_DEFAULT_VNIC(bp);
+ if (use_ntuple)
+ filter1 = bnxt_get_l2_filter(bp, filter, vnic0);
+ else
+ filter1 = bnxt_get_l2_filter(bp, filter, vnic);
if (filter1 == NULL) {
rte_flow_error_set(error,
ENOSPC,
--
2.21.0 (Apple Git-122.2)
[dpdk-dev] [PATCH v2] net/bnxt: add support for flow mark action
Add support for RTE_FLOW_ACTION_TYPE_MARK.
Use the flow_id provided by FW during flow creation to lookup the
mark id provided by the application.
Signed-off-by: Ajit Khaparde
Reviewed-by: Lance Richardson
---
drivers/net/bnxt/bnxt.h| 5 +++
drivers/net/bnxt/bnxt_ethdev.c | 16 -
drivers/net/bnxt/bnxt_filter.h | 3 ++
drivers/net/bnxt/bnxt_flow.c | 66 --
drivers/net/bnxt/bnxt_hwrm.c | 2 ++
drivers/net/bnxt/bnxt_rxr.c| 41 -
drivers/net/bnxt/bnxt_rxr.h| 11 ++
7 files changed, 132 insertions(+), 12 deletions(-)
diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index ab0b8dde1..21ca059b8 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -515,6 +515,7 @@ struct bnxt {
#define BNXT_FLAG_INIT_DONEBIT(21)
#define BNXT_FLAG_FW_CAP_ONE_STEP_TX_TSBIT(22)
#define BNXT_FLAG_ADV_FLOW_MGMTBIT(23)
+#define BNXT_FLAG_RX_VECTOR_PKT_MODE BIT(24)
#define BNXT_PF(bp)(!((bp)->flags & BNXT_FLAG_VF))
#define BNXT_VF(bp)((bp)->flags & BNXT_FLAG_VF)
#define BNXT_NPAR(bp) ((bp)->port_partition_type)
@@ -654,6 +655,10 @@ struct bnxt {
/* Struct to hold adapter error recovery related info */
struct bnxt_error_recovery_info *recovery_info;
+#define BNXT_MARK_TABLE_SZ (sizeof(uint32_t) * 64 * 1024)
+/* TCAM and EM should be 16-bit only. Other modes not supported. */
+#define BNXT_FLOW_ID_MASK 0x
+ uint32_t*mark_table;
};
int bnxt_mtu_set_op(struct rte_eth_dev *eth_dev, uint16_t new_mtu);
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 88df82b86..7b5df9ac1 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -458,6 +458,10 @@ static int bnxt_init_chip(struct bnxt *bp)
}
bnxt_print_link_info(bp->eth_dev);
+ bp->mark_table = rte_zmalloc("bnxt_mark_table", BNXT_MARK_TABLE_SZ, 0);
+ if (!bp->mark_table)
+ PMD_DRV_LOG(ERR, "Allocation of mark table failed\n");
+
return 0;
err_free:
@@ -740,8 +744,10 @@ static int bnxt_scattered_rx(struct rte_eth_dev *eth_dev)
}
static eth_rx_burst_t
-bnxt_receive_function(__rte_unused struct rte_eth_dev *eth_dev)
+bnxt_receive_function(struct rte_eth_dev *eth_dev)
{
+ struct bnxt *bp = eth_dev->data->dev_private;
+
#ifdef RTE_ARCH_X86
#ifndef RTE_LIBRTE_IEEE1588
/*
@@ -762,6 +768,7 @@ bnxt_receive_function(__rte_unused struct rte_eth_dev
*eth_dev)
DEV_RX_OFFLOAD_VLAN_FILTER))) {
PMD_DRV_LOG(INFO, "Using vector mode receive for port %d\n",
eth_dev->data->port_id);
+ bp->flags |= BNXT_FLAG_RX_VECTOR_PKT_MODE;
return bnxt_recv_pkts_vec;
}
PMD_DRV_LOG(INFO, "Vector mode receive disabled for port %d\n",
@@ -773,6 +780,7 @@ bnxt_receive_function(__rte_unused struct rte_eth_dev
*eth_dev)
eth_dev->data->dev_conf.rxmode.offloads);
#endif
#endif
+ bp->flags &= ~BNXT_FLAG_RX_VECTOR_PKT_MODE;
return bnxt_recv_pkts;
}
@@ -956,6 +964,8 @@ static void bnxt_dev_stop_op(struct rte_eth_dev *eth_dev)
bnxt_int_handler(eth_dev);
bnxt_shutdown_nic(bp);
bnxt_hwrm_if_change(bp, 0);
+ memset(bp->mark_table, 0, BNXT_MARK_TABLE_SZ);
+ bp->flags &= ~BNXT_FLAG_RX_VECTOR_PKT_MODE;
bp->dev_stopped = 1;
bp->rx_cosq_cnt = 0;
}
@@ -976,6 +986,9 @@ static void bnxt_dev_close_op(struct rte_eth_dev *eth_dev)
bp->grp_info = NULL;
}
+ rte_free(bp->mark_table);
+ bp->mark_table = NULL;
+
bnxt_dev_uninit(eth_dev);
}
@@ -4775,6 +4788,7 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev)
bp = eth_dev->data->dev_private;
bp->dev_stopped = 1;
+ bp->flags &= ~BNXT_FLAG_RX_VECTOR_PKT_MODE;
if (bnxt_vf_pciid(pci_dev->id.device_id))
bp->flags |= BNXT_FLAG_VF;
diff --git a/drivers/net/bnxt/bnxt_filter.h b/drivers/net/bnxt/bnxt_filter.h
index fc40f112b..8f8a4c13b 100644
--- a/drivers/net/bnxt/bnxt_filter.h
+++ b/drivers/net/bnxt/bnxt_filter.h
@@ -23,9 +23,11 @@ struct bnxt;
#define BNXT_FLOW_L2_INNER_DST_VALID_FLAG BIT(4)
#define BNXT_FLOW_L2_DROP_FLAG BIT(5)
#define BNXT_FLOW_PARSE_INNER_FLAG BIT(6)
+#define BNXT_FLOW_MARK_FLAGBIT(7)
struct bnxt_filter_info {
STAILQ_ENTRY(bnxt_filter_info) next;
+ uint32_tflow_id;
uint64_tfw_l2_filter_id;
struct bnxt_filter_info *matching_l2_fltr_ptr;
uint64_tfw_em_filter_id;
@@ -81,6 +83,7 @@ struct bnxt_filter_info {
* to remember which vnic it was created on
*/
struct bnxt_vnic_info *vnic;
+ uint32_t
[dpdk-dev] [PATCH] net/bnxt: fix to not overwrite error message
In some cases when flow creation fails, we overwrite the specific
error message with a generic error message. This patch fixes it.
Fixes: d24610f7bfda ("net/bnxt: allow flow creation when RSS is enabled")
Cc: sta...@dpdk.org
Signed-off-by: Ajit Khaparde
Reviewed-by: Lance Richardson
---
drivers/net/bnxt/bnxt_flow.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index 707aedcec..cde1fa41c 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -1485,7 +1485,7 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
if (rxq && !vnic->rx_queue_cnt)
rxq->vnic = &bp->vnic_info[0];
}
- return rc;
+ return -rte_errno;
}
static
@@ -1815,7 +1815,7 @@ bnxt_flow_create(struct rte_eth_dev *dev,
rte_flow_error_set(error, 0,
RTE_FLOW_ERROR_TYPE_NONE, NULL,
"Flow with pattern exists, updating
destination queue");
- else
+ else if (!rte_errno)
rte_flow_error_set(error, -ret,
RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
"Failed to create flow.");
--
2.21.0 (Apple Git-122.2)
Re: [dpdk-dev] [PATCH v2 0/5] bnxt patchset
On Fri, Dec 20, 2019 at 6:31 PM Ajit Khaparde wrote: > Fixes for bnxt PMD. > > v1->v2: > - Updated Fixes tag and commit logs > - Rebased patch 4,5 > - Squashed patch 5 and 6 > > Santoshkumar Karanappa Rastapur (2): > net/bnxt: fix link failure during port toggle > net/bnxt: fix non matching flow hitting filter rule > > Somnath Kotur (3): > net/bnxt: fix to use first valid profile > net/bnxt: fix flow flush to sync with flow destroy > net/bnxt: fix to reuse an L2 filter > > drivers/net/bnxt/bnxt.h| 10 +- > drivers/net/bnxt/bnxt_cpr.c| 2 +- > drivers/net/bnxt/bnxt_ethdev.c | 18 +++- > drivers/net/bnxt/bnxt_filter.h | 4 + > drivers/net/bnxt/bnxt_flow.c | 163 + > drivers/net/bnxt/bnxt_hwrm.c | 64 ++--- > drivers/net/bnxt/bnxt_hwrm.h | 3 + > 7 files changed, 143 insertions(+), 121 deletions(-) > Patchset applied to dpdk-next-net-brcm Thanks > > -- > 2.21.0 (Apple Git-122.2) > >
Re: [dpdk-dev] [PATCH v2] net/bnxt: add support for flow mark action
On Fri, Dec 20, 2019 at 6:39 PM Ajit Khaparde
wrote:
> Add support for RTE_FLOW_ACTION_TYPE_MARK.
> Use the flow_id provided by FW during flow creation to lookup the
> mark id provided by the application.
>
> Signed-off-by: Ajit Khaparde
> Reviewed-by: Lance Richardson
>
Patch applied to dpdk-next-net-brcm
> ---
> drivers/net/bnxt/bnxt.h| 5 +++
> drivers/net/bnxt/bnxt_ethdev.c | 16 -
> drivers/net/bnxt/bnxt_filter.h | 3 ++
> drivers/net/bnxt/bnxt_flow.c | 66 --
> drivers/net/bnxt/bnxt_hwrm.c | 2 ++
> drivers/net/bnxt/bnxt_rxr.c| 41 -
> drivers/net/bnxt/bnxt_rxr.h| 11 ++
> 7 files changed, 132 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
> index ab0b8dde1..21ca059b8 100644
> --- a/drivers/net/bnxt/bnxt.h
> +++ b/drivers/net/bnxt/bnxt.h
> @@ -515,6 +515,7 @@ struct bnxt {
> #define BNXT_FLAG_INIT_DONEBIT(21)
> #define BNXT_FLAG_FW_CAP_ONE_STEP_TX_TSBIT(22)
> #define BNXT_FLAG_ADV_FLOW_MGMTBIT(23)
> +#define BNXT_FLAG_RX_VECTOR_PKT_MODE BIT(24)
> #define BNXT_PF(bp)(!((bp)->flags & BNXT_FLAG_VF))
> #define BNXT_VF(bp)((bp)->flags & BNXT_FLAG_VF)
> #define BNXT_NPAR(bp) ((bp)->port_partition_type)
> @@ -654,6 +655,10 @@ struct bnxt {
>
> /* Struct to hold adapter error recovery related info */
> struct bnxt_error_recovery_info *recovery_info;
> +#define BNXT_MARK_TABLE_SZ (sizeof(uint32_t) * 64 * 1024)
> +/* TCAM and EM should be 16-bit only. Other modes not supported. */
> +#define BNXT_FLOW_ID_MASK 0x
> + uint32_t*mark_table;
> };
>
> int bnxt_mtu_set_op(struct rte_eth_dev *eth_dev, uint16_t new_mtu);
> diff --git a/drivers/net/bnxt/bnxt_ethdev.c
> b/drivers/net/bnxt/bnxt_ethdev.c
> index 88df82b86..7b5df9ac1 100644
> --- a/drivers/net/bnxt/bnxt_ethdev.c
> +++ b/drivers/net/bnxt/bnxt_ethdev.c
> @@ -458,6 +458,10 @@ static int bnxt_init_chip(struct bnxt *bp)
> }
> bnxt_print_link_info(bp->eth_dev);
>
> + bp->mark_table = rte_zmalloc("bnxt_mark_table",
> BNXT_MARK_TABLE_SZ, 0);
> + if (!bp->mark_table)
> + PMD_DRV_LOG(ERR, "Allocation of mark table failed\n");
> +
> return 0;
>
> err_free:
> @@ -740,8 +744,10 @@ static int bnxt_scattered_rx(struct rte_eth_dev
> *eth_dev)
> }
>
> static eth_rx_burst_t
> -bnxt_receive_function(__rte_unused struct rte_eth_dev *eth_dev)
> +bnxt_receive_function(struct rte_eth_dev *eth_dev)
> {
> + struct bnxt *bp = eth_dev->data->dev_private;
> +
> #ifdef RTE_ARCH_X86
> #ifndef RTE_LIBRTE_IEEE1588
> /*
> @@ -762,6 +768,7 @@ bnxt_receive_function(__rte_unused struct rte_eth_dev
> *eth_dev)
> DEV_RX_OFFLOAD_VLAN_FILTER))) {
> PMD_DRV_LOG(INFO, "Using vector mode receive for port
> %d\n",
> eth_dev->data->port_id);
> + bp->flags |= BNXT_FLAG_RX_VECTOR_PKT_MODE;
> return bnxt_recv_pkts_vec;
> }
> PMD_DRV_LOG(INFO, "Vector mode receive disabled for port %d\n",
> @@ -773,6 +780,7 @@ bnxt_receive_function(__rte_unused struct rte_eth_dev
> *eth_dev)
> eth_dev->data->dev_conf.rxmode.offloads);
> #endif
> #endif
> + bp->flags &= ~BNXT_FLAG_RX_VECTOR_PKT_MODE;
> return bnxt_recv_pkts;
> }
>
> @@ -956,6 +964,8 @@ static void bnxt_dev_stop_op(struct rte_eth_dev
> *eth_dev)
> bnxt_int_handler(eth_dev);
> bnxt_shutdown_nic(bp);
> bnxt_hwrm_if_change(bp, 0);
> + memset(bp->mark_table, 0, BNXT_MARK_TABLE_SZ);
> + bp->flags &= ~BNXT_FLAG_RX_VECTOR_PKT_MODE;
> bp->dev_stopped = 1;
> bp->rx_cosq_cnt = 0;
> }
> @@ -976,6 +986,9 @@ static void bnxt_dev_close_op(struct rte_eth_dev
> *eth_dev)
> bp->grp_info = NULL;
> }
>
> + rte_free(bp->mark_table);
> + bp->mark_table = NULL;
> +
> bnxt_dev_uninit(eth_dev);
> }
>
> @@ -4775,6 +4788,7 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev)
> bp = eth_dev->data->dev_private;
>
> bp->dev_stopped = 1;
> + bp->flags &= ~BNXT_FLAG_RX_VECTOR_PKT_MODE;
>
> if (bnxt_vf_pciid(pci_dev->id.device_id))
> bp->flags |= BNXT_FLAG_VF;
> diff --git a/drivers/net/bnxt/bnxt_filter.h
> b/drivers/net/bnxt/bnxt_filter.h
> index fc40f112b..8f8a4c13b 100644
> --- a/drivers/net/bnxt/bnxt_filter.h
> +++ b/drivers/net/bnxt/bnxt_filter.h
> @@ -23,9 +23,11 @@ struct bnxt;
> #define BNXT_FLOW_L2_INNER_DST_VALID_FLAG BIT(4)
> #define BNXT_FLOW_L2_DROP_FLAG BIT(5)
> #define BNXT_FLOW_PARSE_INNER_FLAG BIT(6)
> +#define BNXT_FLOW_MARK_FLAGBIT(7)
>
> struct bnxt_filter_info {
> STAILQ_ENTRY(bnxt_filter_info) next;
> + uint32_tflow_id;
> u
Re: [dpdk-dev] [PATCH] net/bnxt: fix to not overwrite error message
On Fri, Dec 20, 2019 at 6:42 PM Ajit Khaparde
wrote:
> In some cases when flow creation fails, we overwrite the specific
> error message with a generic error message. This patch fixes it.
>
> Fixes: d24610f7bfda ("net/bnxt: allow flow creation when RSS is enabled")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Ajit Khaparde
> Reviewed-by: Lance Richardson
>
Patch applied to dpdk-next-net-brcm
> ---
> drivers/net/bnxt/bnxt_flow.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
> index 707aedcec..cde1fa41c 100644
> --- a/drivers/net/bnxt/bnxt_flow.c
> +++ b/drivers/net/bnxt/bnxt_flow.c
> @@ -1485,7 +1485,7 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
> if (rxq && !vnic->rx_queue_cnt)
> rxq->vnic = &bp->vnic_info[0];
> }
> - return rc;
> + return -rte_errno;
> }
>
> static
> @@ -1815,7 +1815,7 @@ bnxt_flow_create(struct rte_eth_dev *dev,
> rte_flow_error_set(error, 0,
>RTE_FLOW_ERROR_TYPE_NONE, NULL,
>"Flow with pattern exists, updating
> destination queue");
> - else
> + else if (!rte_errno)
> rte_flow_error_set(error, -ret,
>RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
>"Failed to create flow.");
> --
> 2.21.0 (Apple Git-122.2)
>
>
Re: [dpdk-dev] [PATCH] mempool: fix mempool obj alignment for non x86
On Sat, Dec 21, 2019 at 2:37 AM Honnappa Nagarahalli
wrote:
>
>
> > > > From: Jerin Jacob
> > > >
> > > > The exiting optimize_object_size() function address the memory
> > > > object alignment constraint on x86 for better performance.
> > > >
> > > > Different (Mirco) architecture may have different memory alignment
> > > > constraint for better performance and it not same as the existing
> > > > optimize_object_size() function. Some use, XOR(kind of CRC) scheme
> > > > to enable DRAM channel distribution based on the address and some
> > > > may have a different formula.
> > > If I understand correctly, address interleaving is the characteristic of
> > > the
> > memory controller and not the CPU.
> > > For ex: different SoCs using the same Arm architecture might have
> > > different
> > memory controllers. So, the solution should not be architecture specific,
> > but
> > SoC specific.
> >
> > Yes. See below.
> >
> > > > -static unsigned optimize_object_size(unsigned obj_size)
> > > > +static unsigned
> > > > +arch_mem_object_align(unsigned obj_size)
> > > > {
> > > > unsigned nrank, nchan;
> > > > unsigned new_obj_size;
> > > > @@ -99,6 +101,13 @@ static unsigned optimize_object_size(unsigned
> > > > obj_size)
> > > > new_obj_size++;
> > > > return new_obj_size * RTE_MEMPOOL_ALIGN; }
> > > > +#else
> > > This applies to add Arm (PPC as well) SoCs which might have different
> > schemes depending on the memory controller. IMO, this should not be
> > architecture specific.
> >
> > I agree in principle.
> > I will summarize the
> > https://www.mail-archive.com/dev@dpdk.org/msg149157.html feedback:
> >
> > 1) For x86 arch, it is architecture-specific
> > 2) For power PC arch, It is architecture-specific
> > 3) For the ARM case, it will be the memory controller specific.
> > 4) For the ARM case, The memory controller is not using the existing
> > x86 arch formula.
> > 5) If it is memory/arch-specific, Can userspace code find the optimal
> > alignment? In the case of octeontx2/arm64, the memory controller does XOR
> > on PA address which userspace code doesn't have much control.
> >
> > This patch address the known case of (1), (2), (4) and (5). (2) can be
> > added to
> > this framework when POWER9 folks want it.
> >
> > We can extend this patch to address (3) if there is a case. Without the
> > actual
> > requirement(If some can share the formula of alignment which is the
> > memory controller specific and it does not come under (4))) then we can
> > create extra layer for the memory controller and abstraction to probe it.
> > Again there is no standard way of probing the memory controller in
> > userspace and we need platform #define, which won't work for distribution
> > build.
> > So solution needs to be arch-specific and then fine-tune to memory
> > controller
> > if possible.
> >
> > I can work on creating an extra layer of code if some can provide the
> > details
> > of the memory controller and probing mechanism or this patch be extended
> Inputs for BlueField, DPAAx, ThunderX2 would be helpful.
Yes. Probably memory controller used in n1sdp SoC also.
>
> > to support such case if it arises in future.
> >
> > Thoughts?
> How much memory will this save for your platform? Is it affecting performance?
No performance difference.
The existing code adding the tailer for each objs.
Additional space/Trailer space will be function of number of objects
in mempool and its obj_size, its alignment and selected
rte_memory_get_nchannel() and rte_memory_get_nrank()
I will wait for inputs from Bluefield, DPAAx, ThunderX2 and n1sdp(if
any) for any rework on the patch.
>
> >
> > >
> > > > +static unsigned
> > > > +arch_mem_object_align(unsigned obj_size) {
> > > > + return obj_size;
> > > > +}
> > > > +#endif
> > > >
> > > > struct pagesz_walk_arg {
> > > > int socket_id;
> > > > @@ -234,8 +243,8 @@ rte_mempool_calc_obj_size(uint32_t elt_size,
> > > > uint32_t flags,
> > > >*/
> > > > if ((flags & MEMPOOL_F_NO_SPREAD) == 0) {
> > > > unsigned new_size;
> > > > - new_size = optimize_object_size(sz->header_size + sz-
> > > > >elt_size +
> > > > - sz->trailer_size);
> > > > + new_size = arch_mem_object_align
> > > > + (sz->header_size + sz->elt_size +
> > > > + sz->trailer_size);
> > > > sz->trailer_size = new_size - sz->header_size -
> > > > sz->elt_size;
> > > > }
> > > >
> > > > --
> > > > 2.24.1
> > >
