[dpdk-dev] [PATCH] net/virtio-user: fix dev_init in legacy-mem mode

[Xiao Wang]

In legacy-mem mode, memory event callback registering is not supported,
we should not return error in dev_init on this case.

Fixes: 12ecb2f63b12 ("net/virtio-user: support memory hotplug")

Signed-off-by: Xiao Wang 
Suggested-by: Tiwei Bie 
---
 drivers/net/virtio/virtio_user/virtio_user_dev.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c 
b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 992a68757..bd16fbb60 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -445,8 +445,11 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char 
*path, int queues,
 
if (rte_mem_event_callback_register(VIRTIO_USER_MEM_EVENT_CLB_NAME,
virtio_user_mem_event_cb, dev)) {
-   PMD_INIT_LOG(ERR, "Failed to register mem event callback\n");
-   return -1;
+   if (rte_errno != ENOTSUP) {
+   PMD_INIT_LOG(ERR, "Failed to register mem event"
+   " callback\n");
+   return -1;
+   }
}
 
return 0;
-- 
2.15.1

[dpdk-dev] [PATCH] doc: update release notes for mlx drivers

[Shahaf Shuler]

Signed-off-by: Shahaf Shuler 
---
 doc/guides/rel_notes/release_18_05.rst | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/doc/guides/rel_notes/release_18_05.rst 
b/doc/guides/rel_notes/release_18_05.rst
index 74ca3cf280..27241f3ea4 100644
--- a/doc/guides/rel_notes/release_18_05.rst
+++ b/doc/guides/rel_notes/release_18_05.rst
@@ -59,6 +59,33 @@ New Features
 
   Support to update RSS hash and key has been added to CXGBE PMD.
 
+* **Updated mlx5 driver.**
+
+  Updated the mlx5 driver including the following changes:
+
+  * Introduced Multi-packet Rx. With it, achieved 100Gb/sec with 64B frames.
+  * Supported to be run by non-root users given reduced set of capabilities
+CAP_NET_ADMIN, CAP_NET_RAW and CAP_IPC_LOCK.
+  * Supported TSO and checksum for generic UDP and IP tunnels.
+  * Supported inner checksum and RSS for GRE, VXLAN-GPE, MPLSoGRE
+and MPLSoUDP tunnels.
+  * Accommodate to the new memory hotplug model.
+  * Supported non virtually contiguous mempools.
+  * Supported MAC adding along with allmulti and promiscuous modes from VF.
+  * Supported Mellanox BlueField SoC device.
+  * Supported PMD defaults for queue number and depth to improve the out
+of the box performance.
+
+* **Updated mlx4 driver.**
+
+  Updated the mlx4 driver including the following changes:
+
+  * Supported to be run by non-root users given reduced set of capabilities
+CAP_NET_ADMIN, CAP_NET_RAW and CAP_IPC_LOCK.
+  * Supported CRC strip toggling.
+  * Accommodate to the new memory hotplug model.
+  * Supported non virtually contiguous mempools.
+
 * **Added CXGBE VF PMD.**
 
   CXGBE VF Poll Mode Driver has been added to run DPDK over Chelsio
-- 
2.12.0

Re: [dpdk-dev] [PATCH] net/virtio-user: fix dev_init in legacy-mem mode

[Maxime Coquelin]

Hi Xiao,

Next time, could you please devtools/check-git-log.sh script before
posting.

I tihnk the commit title should be changed to:
net/virtio-user: fix device init in legacy-mem mode

On 05/17/2018 09:35 AM, Xiao Wang wrote:

In legacy-mem mode, memory event callback registering is not supported,
we should not return error in dev_init on this case.

Fixes: 12ecb2f63b12 ("net/virtio-user: support memory hotplug")

Signed-off-by: Xiao Wang 
Suggested-by: Tiwei Bie 


I think the suggested-by line should go above the signed-off one,
as it was suggested before being implemented.


---
  drivers/net/virtio/virtio_user/virtio_user_dev.c | 7 +--
  1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c 
b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 992a68757..bd16fbb60 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -445,8 +445,11 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char 
*path, int queues,
  
  	if (rte_mem_event_callback_register(VIRTIO_USER_MEM_EVENT_CLB_NAME,

virtio_user_mem_event_cb, dev)) {
-   PMD_INIT_LOG(ERR, "Failed to register mem event callback\n");
-   return -1;
+   if (rte_errno != ENOTSUP) {
+   PMD_INIT_LOG(ERR, "Failed to register mem event"
+   " callback\n");
+   return -1;
+   }
}
  
  	return 0;




Apart above minor comments, the patch looks good to me:
Reviewed-by: Maxime Coquelin 

I'll handle the changes when applying.

Thanks,
Maxime

Re: [dpdk-dev] [PATCH V2] librte_hash: new hash del abi to return stored value

[De Lara Guarch, Pablo]

Hi Vijaya,

> -Original Message-
> From: Vijaya Mohan Guvva [mailto:vgu...@caviumnetworks.com]
> Sent: Thursday, May 17, 2018 2:27 AM
> To: Richardson, Bruce ; De Lara Guarch, Pablo
> 
> Cc: dev@dpdk.org; Vijaya Mohan Guvva 
> Subject: [PATCH V2] librte_hash: new hash del abi to return stored value
> 

You are actually adding new API, not ABI, so I would reword the commit message.

"hash: add API to return stored value at deletion" maybe?

I would add some information in the commit message and move the changelog (V1, 
V2 notes)
after the three dashes.
 
> V2:
> Adding another new interface rte_hash_del_key_data to delete key from hash
> table and return stored data.
> 
> V1:
> Add a new key delete interface rte_hash_del_key_with_hash_data to delete the
> key from hash and return the value stored. This is useful for hash users to 
> free
> the data stored in the table after key delete and to avoid maintaining a user 
> data
> array in the dpdk application.
> 
> Signed-off-by: Vijaya Mohan Guvva 
> ---
>  lib/librte_hash/rte_cuckoo_hash.c | 30 +++---
>  lib/librte_hash/rte_hash.h| 45
> +++
>  2 files changed, 72 insertions(+), 3 deletions(-)

...

> +int32_t
> +rte_hash_del_key_with_hash_data(const struct rte_hash *h, const void *key,
> + hash_sig_t sig, void **data);
> +
> +/**
>   * Remove a key from an existing hash table.
>   * This operation is not multi-thread safe
>   * and should only be called from one thread.
> --
> 1.8.3.1

You need to update the version.map file to add the two new functions
(you might need to wait until 18.05 is released, so you can use the 18.08 tag).

Re: [dpdk-dev] [PATCH] net/virtio-user: fix dev_init in legacy-mem mode

[Wang, Xiao W]

Hi,


> -Original Message-
> From: Maxime Coquelin [mailto:maxime.coque...@redhat.com]
> Sent: Thursday, May 17, 2018 4:03 PM
> To: Wang, Xiao W 
> Cc: dev@dpdk.org; Bie, Tiwei ; Yao, Lei A
> 
> Subject: Re: [PATCH] net/virtio-user: fix dev_init in legacy-mem mode
> 
> Hi Xiao,
> 
> Next time, could you please devtools/check-git-log.sh script before
> posting.

OK, thanks!

BRs,
Xiao
> 
> I tihnk the commit title should be changed to:
> net/virtio-user: fix device init in legacy-mem mode
> 
> On 05/17/2018 09:35 AM, Xiao Wang wrote:
> > In legacy-mem mode, memory event callback registering is not supported,
> > we should not return error in dev_init on this case.
> >
> > Fixes: 12ecb2f63b12 ("net/virtio-user: support memory hotplug")
> >
> > Signed-off-by: Xiao Wang 
> > Suggested-by: Tiwei Bie 
> 
> I think the suggested-by line should go above the signed-off one,
> as it was suggested before being implemented.
> 
> > ---
> >   drivers/net/virtio/virtio_user/virtio_user_dev.c | 7 +--
> >   1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > index 992a68757..bd16fbb60 100644
> > --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > @@ -445,8 +445,11 @@ virtio_user_dev_init(struct virtio_user_dev *dev,
> char *path, int queues,
> >
> > if
> (rte_mem_event_callback_register(VIRTIO_USER_MEM_EVENT_CLB_NAME,
> > virtio_user_mem_event_cb, dev)) {
> > -   PMD_INIT_LOG(ERR, "Failed to register mem event
> callback\n");
> > -   return -1;
> > +   if (rte_errno != ENOTSUP) {
> > +   PMD_INIT_LOG(ERR, "Failed to register mem event"
> > +   " callback\n");
> > +   return -1;
> > +   }
> > }
> >
> > return 0;
> >
> 
> Apart above minor comments, the patch looks good to me:
> Reviewed-by: Maxime Coquelin 
> 
> I'll handle the changes when applying.
> 
> Thanks,
> Maxime

[dpdk-dev] [PATCH] eventdev/eth_rx_adapter: update the note field

[Vipin Varghese]

rte_event_eth_rx_adapter_create allocates eth_devices for currently
available eth devices. For dynamically created eth devices a new instance
for rx adapter has to be created.

Signed-off-by: Vipin Varghese 
---
 lib/librte_eventdev/rte_event_eth_rx_adapter.h | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eventdev/rte_event_eth_rx_adapter.h 
b/lib/librte_eventdev/rte_event_eth_rx_adapter.h
index 834eb53..0ef18ce 100644
--- a/lib/librte_eventdev/rte_event_eth_rx_adapter.h
+++ b/lib/librte_eventdev/rte_event_eth_rx_adapter.h
@@ -63,7 +63,10 @@
  * rte_event_eth_rx_adapter_service_id_get() function can be used to retrieve
  * the service function ID of the adapter in this case.
  *
- * Note: Interrupt driven receive queues are currently unimplemented.
+ * Note:
+ * 1) Interrupt driven receive queues are currently unimplemented.
+ * 2) rte_event_eth_rx_adapter_create() is not accounted for dynamic
+ *eth devices.
  */
 
 #ifdef __cplusplus
-- 
2.7.4

Re: [dpdk-dev] [PATCH] net/virtio-user: fix dev_init in legacy-mem mode

[Maxime Coquelin]

On 05/17/2018 09:35 AM, Xiao Wang wrote:

In legacy-mem mode, memory event callback registering is not supported,
we should not return error in dev_init on this case.

Fixes: 12ecb2f63b12 ("net/virtio-user: support memory hotplug")

Signed-off-by: Xiao Wang
Suggested-by: Tiwei Bie
---
  drivers/net/virtio/virtio_user/virtio_user_dev.c | 7 +--
  1 file changed, 5 insertions(+), 2 deletions(-)



Applied to dpdk-next-virtio/master with suggested changes.

Thanks,
Maxime

Re: [dpdk-dev] [PATCH v4 09/23] rte_mbuf.h: explicit casts for int16 to uint16

[Olivier Matz]

On Mon, May 14, 2018 at 01:10:12PM +0800, Andy Green wrote:
> differences to the atomic16 are signed, but the
> atomic16 itself is unsigned.  It needs to be
> made explicit with casts.
> 
> Signed-off-by: Andy Green 
> ---
>  lib/librte_mbuf/rte_mbuf.h |   12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index a2a37a311..c214f1945 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -806,7 +806,7 @@ rte_mbuf_refcnt_read(const struct rte_mbuf *m)
>  static inline void
>  rte_mbuf_refcnt_set(struct rte_mbuf *m, uint16_t new_value)
>  {
> - rte_atomic16_set(&m->refcnt_atomic, new_value);
> + rte_atomic16_set(&m->refcnt_atomic, (int16_t)new_value);
>  }
>  
>  /* internal */
> @@ -837,8 +837,8 @@ rte_mbuf_refcnt_update(struct rte_mbuf *m, int16_t value)
>*/
>   if (likely(rte_mbuf_refcnt_read(m) == 1)) {
>   ++value;
> - rte_mbuf_refcnt_set(m, value);
> - return value;
> + rte_mbuf_refcnt_set(m, (uint16_t)value);
> + return (uint16_t)value;
>   }
>  
>   return __rte_mbuf_refcnt_update(m, value);
> @@ -909,7 +909,7 @@ static inline void
>  rte_mbuf_ext_refcnt_set(struct rte_mbuf_ext_shared_info *shinfo,
>   uint16_t new_value)
>  {
> - rte_atomic16_set(&shinfo->refcnt_atomic, new_value);
> + rte_atomic16_set(&shinfo->refcnt_atomic, (int16_t)new_value);
>  }
>  
>  /**
> @@ -929,8 +929,8 @@ rte_mbuf_ext_refcnt_update(struct 
> rte_mbuf_ext_shared_info *shinfo,
>  {
>   if (likely(rte_mbuf_ext_refcnt_read(shinfo) == 1)) {
>   ++value;
> - rte_mbuf_ext_refcnt_set(shinfo, value);
> - return value;
> + rte_mbuf_ext_refcnt_set(shinfo, (uint16_t)value);
> + return (uint16_t)value;
>   }
>  
>   return (uint16_t)rte_atomic16_add_return(&shinfo->refcnt_atomic, value);
> 

Looking at the API of functions, I think there are some inconsistencies:

  static inline void
  rte_mbuf_refcnt_set(struct rte_mbuf *m, uint16_t new_value)

  /* internal */
  static inline uint16_t
  __rte_mbuf_refcnt_update(struct rte_mbuf *m, int16_t value)

  static inline uint16_t
  rte_mbuf_refcnt_update(struct rte_mbuf *m, int16_t value)

For next version, I think we should change it to be int16_t everywhere,
to avoid some of the explicit casts you are introducing.

If we want it for 18.05:
Acked-by: Olivier Matz 

Re: [dpdk-dev] [PATCH v4 06/23] rte_ring_generic.h: stack declarations before code

[Olivier Matz]

On Mon, May 14, 2018 at 01:09:57PM +0800, Andy Green wrote:
> /projects/lagopus/src/dpdk/build/include/rte_ring_generic.h:
> In function '__rte_ring_move_prod_head':
> /projects/lagopus/src/dpdk/build/include/rte_ring_generic.h:76:3:
> warning: ISO C90 forbids mixed declarations and code
> [-Wdeclaration-after-statement]
>const uint32_t cons_tail = r->cons.tail;
>^
> /projects/lagopus/src/dpdk/build/include/rte_ring_generic.h:
> In function '__rte_ring_move_cons_head':
> /projects/lagopus/src/dpdk/build/include/rte_ring_generic.h:147:3:
> warning: ISO C90 forbids mixed declarations and code
> [-Wdeclaration-after-statement]
>const uint32_t prod_tail = r->prod.tail;
> 
> Signed-off-by: Andy Green 
> Fixes: 0dfc98c507b1 ("ring: separate out head index manipulation")

Acked-by: Olivier Matz 

Re: [dpdk-dev] [PATCH v4 07/23] rte_ring.h: remove signed type flipflopping

[Olivier Matz]

On Mon, May 14, 2018 at 01:10:02PM +0800, Andy Green wrote:
> /projects/lagopus/src/dpdk/build/include/rte_ring.h:350:46:
> warning: conversion to 'uint32_t' {aka 'unsigned int'}
> from 'int' may change the sign of the result
> [-Wsign-conversion]
>   update_tail(&r->prod, prod_head, prod_next, is_sp, 1);
> 
> The visible apis take unsigned int, then call a private
> api taking an int, which finally calls an api taking an
> unsigned int.
> 
> Convert the private api to take unsigned int removing
> 5 x warning similar to that shown above.
> 
> Signed-off-by: Andy Green 

Acked-by: Olivier Matz 

Re: [dpdk-dev] [PATCH v4 08/23] rte_mbuf.h: avoid warnings from inadvertant promotion

[Olivier Matz]

On Mon, May 14, 2018 at 01:10:07PM +0800, Andy Green wrote:
> "1 + value", where value is an uint16_t causes promotion
> to a signed int.  The compiler complained that we are
> shoving an int into a uint16_t return type with different
> size and sign.
> 
> Bumping and returning value directly instead removes the
> promotion and the problem.
> 
> Signed-off-by: Andy Green 

Acked-by: Olivier Matz 

[dpdk-dev] [PATCH v3 0/4] Cryptodev API/ABI deprecation notices

[Pablo de Lara]

v3:
- Added an extra deprecation announcement (replacing rte_pci_device
  with rte_device)
- Rebased against latest DPDK code

v2:
- Added an extra deprecation announcement
- Bonded the other two deprecation notices with the new one in a
  patchset

Pablo de Lara (4):
  doc: announce ABI change for crypto sym info struct
  doc: announce ABI change for crypto info struct
  doc: announce deprecation for attach/detach crypto session
  doc: announce deprecation in crypto queue pair start/stop

 doc/guides/rel_notes/deprecation.rst | 17 +
 lib/librte_cryptodev/rte_cryptodev.h |  4 
 2 files changed, 21 insertions(+)

-- 
2.17.0

[dpdk-dev] [PATCH v3 1/4] doc: announce ABI change for crypto sym info struct

[Pablo de Lara]

Since the API changes made in 17.08, the session mempool
is not created anymore in each crypto device.
Therefore, there is no need to have, in the cryptodev info
structure, the maximum number of sessions supported per device
and per queue pair.

Signed-off-by: Pablo de Lara 
Acked-by: Fiona Trahe 
---
 doc/guides/rel_notes/deprecation.rst | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst 
b/doc/guides/rel_notes/deprecation.rst
index c3b79a22f..d6cccbfde 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -71,3 +71,10 @@ Deprecation Notices
   - ``rte_pdump_set_socket_dir`` will be removed;
   - The parameter, ``path``, of ``rte_pdump_init`` will be removed;
   - The enum ``rte_pdump_socktype`` will be removed.
+
+* cryptodev: The following changes will be made in the library
+  for 18.08:
+
+  - Removal of ``sym`` structure in ``rte_cryptodev_info`` structure,
+containing fields not relevant anymore since the session mempool
+is not internal in the crypto device anymore.
-- 
2.17.0

[dpdk-dev] [PATCH v3 3/4] doc: announce deprecation for attach/detach crypto session

[Pablo de Lara]

Functions rte_cryptodev_queue_pair_attach_sym_session
and rte_cryptodev_queue_pair_detach_sym_sessions
are not really used in any of the crypto drivers
(only one driver implements it and it just return 0).
Therefore, this API can be deprecated from 18.05
and removed in 18.08.

Signed-off-by: Pablo de Lara 
---
 doc/guides/rel_notes/deprecation.rst | 4 
 lib/librte_cryptodev/rte_cryptodev.h | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst 
b/doc/guides/rel_notes/deprecation.rst
index 85945ee72..cd75150a6 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -80,3 +80,7 @@ Deprecation Notices
 is not internal in the crypto device anymore.
   - Replacement of ``pci_dev`` field with the more generic ``rte_device``
 structure.
+  - Functions ``rte_cryptodev_queue_pair_attach_sym_session()`` and
+``rte_cryptodev_queue_pair_dettach_sym_session()`` will be deprecated from
+18.02 and removed in 18.05, as there are no drivers doing anything useful
+with them.
diff --git a/lib/librte_cryptodev/rte_cryptodev.h 
b/lib/librte_cryptodev/rte_cryptodev.h
index 261a359dc..93eb9ffe2 100644
--- a/lib/librte_cryptodev/rte_cryptodev.h
+++ b/lib/librte_cryptodev/rte_cryptodev.h
@@ -986,6 +986,7 @@ unsigned int
 rte_cryptodev_get_private_session_size(uint8_t dev_id);
 
 /**
+ * @deprecated
  * Attach queue pair with sym session.
  *
  * @param  dev_id  Device to which the session will be attached.
@@ -1002,6 +1003,7 @@ rte_cryptodev_queue_pair_attach_sym_session(uint8_t 
dev_id, uint16_t qp_id,
struct rte_cryptodev_sym_session *session);
 
 /**
+ * @deprecated
  * Detach queue pair with sym session.
  *
  * @param  dev_id  Device to which the session is attached.
-- 
2.17.0

[dpdk-dev] [PATCH v3 2/4] doc: announce ABI change for crypto info struct

[Pablo de Lara]

Cryptodev info structure currently contains
a pointer to an rte_pci_device structure.
This field depends on a specific bus type (PCI),
which is not following a bus independent design.
Following the same approach taken in ethdev, the field
will be replaced with a pointer to an rte_device structure.

Signed-off-by: Pablo de Lara 
---
 doc/guides/rel_notes/deprecation.rst | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst 
b/doc/guides/rel_notes/deprecation.rst
index d6cccbfde..85945ee72 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -78,3 +78,5 @@ Deprecation Notices
   - Removal of ``sym`` structure in ``rte_cryptodev_info`` structure,
 containing fields not relevant anymore since the session mempool
 is not internal in the crypto device anymore.
+  - Replacement of ``pci_dev`` field with the more generic ``rte_device``
+structure.
-- 
2.17.0

[dpdk-dev] [PATCH v3 4/4] doc: announce deprecation in crypto queue pair start/stop

[Pablo de Lara]

Functions rte_cryptodev_queue_pair_start/stop
are not really used in any of the crypto drivers
(they all just return 0 or -ENOTSUP).
Therefore, this API can be deprecated from 18.02
and removed in 18.05.

Signed-off-by: Pablo de Lara 
Acked-by: Akhil Goyal 
---
 doc/guides/rel_notes/deprecation.rst | 4 
 lib/librte_cryptodev/rte_cryptodev.h | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst 
b/doc/guides/rel_notes/deprecation.rst
index cd75150a6..6c7978d1b 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -84,3 +84,7 @@ Deprecation Notices
 ``rte_cryptodev_queue_pair_dettach_sym_session()`` will be deprecated from
 18.02 and removed in 18.05, as there are no drivers doing anything useful
 with them.
+  - Functions ``rte_cryptodev_queue_pair_start()`` and
+``rte_cryptodev_queue_pair_stop()`` will be deprecated from 18.02
+and removed in 18.05, as there are no drivers doing anything useful
+with them.
diff --git a/lib/librte_cryptodev/rte_cryptodev.h 
b/lib/librte_cryptodev/rte_cryptodev.h
index 93eb9ffe2..364cc4399 100644
--- a/lib/librte_cryptodev/rte_cryptodev.h
+++ b/lib/librte_cryptodev/rte_cryptodev.h
@@ -602,6 +602,7 @@ rte_cryptodev_queue_pair_setup(uint8_t dev_id, uint16_t 
queue_pair_id,
struct rte_mempool *session_pool);
 
 /**
+ * @deprecated
  * Start a specified queue pair of a device. It is used
  * when deferred_start flag of the specified queue is true.
  *
@@ -619,6 +620,7 @@ extern int
 rte_cryptodev_queue_pair_start(uint8_t dev_id, uint16_t queue_pair_id);
 
 /**
+ * @deprecated
  * Stop specified queue pair of a device
  *
  * @param  dev_id  The identifier of the device
-- 
2.17.0

Re: [dpdk-dev] How to dump DPDK log?

[Ferruh Yigit]

On 5/17/2018 3:41 AM, Sam wrote:
> Oh, it should be user1,8 in dpdk-17.05

Yes, loglevel numbers are converted to more human readable format in this
release, old releases are using numbers.

> 
> OMG
> 
> 2018-05-17 10:26 GMT+08:00 Sam  >:
> 
> How to set this param...
> 
> I use this, but report me bug:
> 
> sudo /usr/local/bin/ovs-vsctl --no-wait set Open_vSwitch .
> other_config:dpdk-extra="--log-level=user1,debug -c 0x40004 -n 4
> --socket-mem 1024 -w :01:00.0 -w :01:00.1"
> 
> Bug:
> 
> 2018-05-17T02:23:21.878Z|00010|dpdk|INFO|EAL ARGS: ovs-vswitchd
> --log-level=user1,debug -c 0x40004 -n 4 --socket-mem 1024 -w :01:00.0 
> -w
> :01:00.1
> 2018-05-17T02:23:21.878Z|00011|dpdk|ERR|EAL: invalid parameters for 
> --log-level
> 2018-05-17T02:23:21.879Z|00012|dpdk|INFO|EAL: Detected 32 lcore(s)
> 2018-05-17T02:23:21.879Z|00013|dpdk|ERR|EAL: invalid parameters for 
> --log-level
> 
> 2018-05-17T02:23:04.374Z|00010|dpdk|INFO|EAL ARGS: ovs-vswitchd
> --log-level="user1,debug" -c 0x40004 -n 4 --socket-mem 1024 -w 
> :01:00.0
> -w :01:00.1
> 2018-05-17T02:23:04.374Z|00011|dpdk|ERR|EAL: invalid parameters for 
> --log-level
> 2018-05-17T02:23:04.375Z|00012|dpdk|INFO|EAL: Detected 32 lcore(s)
> 2018-05-17T02:23:04.375Z|00013|dpdk|ERR|EAL: invalid parameters for 
> --log-level
> 
> 2018-05-16 21:29 GMT+08:00 Ferruh Yigit  >:
> 
> On 5/16/2018 9:30 AM, Sam wrote:
> > I want DPDK VHOST module DEBUG level log
> >
> > 2018-05-16 16:27 GMT+08:00 Sam  >:
> >
> >> Hi all,
> >>
> >> I'm debugging OVS2.9.0 and DPDK18.02, I found I could not dump 
> DPDK log
> >> like vhost module.
> >>
> >> How can I dump DPDK log into files? Thank you~
> >>
> 
> (Unfortunately) vhost module doesn't register its log type but uses
> USER1 log
> type [1].
> 
> Need to set user1 log type to debug, following eal parameter should 
> work:
> --log-level="user1,debug"
> 
> Same can be done with logging related APIs by application
> 
> 
> [1]
> 
> https://dpdk.org/browse/dpdk/tree/lib/librte_vhost/vhost.h?h=v18.05-rc4#n351
> 
> 
> 
> 
> 

Re: [dpdk-dev] [PATCH] maintainers: fix responsibility of flow API bits

[Iremonger, Bernard]

Hi Adrien,



> > > Subject: [PATCH] maintainers: fix responsibility of flow API bits
> > >
> > > The following commits lack MAINTAINERS entries for this mess.
> > >
> > > Fixes: 4d73b6fb9907 ("doc: add generic flow API guide")
> > > Fixes: 19c90af6285c ("app/testpmd: add flow command")
> > > Cc: sta...@dpdk.org
> > >
> > > Signed-off-by: Adrien Mazarguil 
> > > Cc: Bernard Iremonger 
> > > ---
> > >  MAINTAINERS | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/MAINTAINERS b/MAINTAINERS index bd08d4292..187817fff
> > > 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -303,6 +303,8 @@ F: devtools/test-null.sh  Flow API
> > >  M: Adrien Mazarguil 
> > >  T: git://dpdk.org/next/dpdk-next-net
> > > +F: app/test-pmd/cmdline_flow.c
> > > +F: doc/guides/prog_guide/rte_flow.rst
> >
> > doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > Should be added to the list of flow related files.
> 
> I did not add it because there is no way to split responsibility on a
> documentation section basis AFAIK, and only a fraction of this file contains
> information about rte_flow-related stuff.
> 
> --
> Adrien Mazarguil
> 6WIND

There are already several maintainers for testpmd, adding another maintainer 
for the flow parts of this file should be ok (I think).

Regards,

Bernard

Re: [dpdk-dev] [PATCH] bus/vdev: don't double space log messages

[Ferruh Yigit]

On 5/16/2018 10:51 PM, Stephen Hemminger wrote:
> The VDEV_LOG() macro already adds a newline, don't duplicate.
> 
> Signed-off-by: Stephen Hemminger 

Fixes: d22fcb225c24 ("bus/vdev: change log type")

Reviewed-by: Ferruh Yigit 

[dpdk-dev] [PATCH] maintainers: handoff ownership of Tap PMD

[Pascal Mazon]

I have unfortunately no longer time enough for maintaining Tap PMD.
Keith has kindly volunteered to take over maintainership. He's been at
the origin of this PMD and knows well how it works.

Signed-off-by: Pascal Mazon 
Acked-by: Keith Wiles 
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2663f1c037d7..a1b072c0b2d7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -686,7 +686,7 @@ F: doc/guides/nics/pcap_ring.rst
 F: doc/guides/nics/features/pcap.ini
 
 Tap PMD
-M: Pascal Mazon 
+M: Keith Wiles 
 F: drivers/net/tap/
 F: doc/guides/nics/tap.rst
 F: doc/guides/nics/features/tap.ini
-- 
2.16.1.72.g5be1f00a9

[dpdk-dev] [PATCH v2] net/tap: fix device removal when no queues exist

[Ophir Munk]

TAP device is created following its first queue creation. Multiple
queues can be added or removed over time. In Linux terminology those
are file descriptors which are opened or closed over time. As long as
the number of opened file descriptors is positive - TAP device will
appear as a Linux device. In case all queues are released (the
equivalent of all file descriptors being closed) the TAP device will
be removed. This can lead to abnormalities in different scenarios
where the TAP device should exist even if all its queues are released.
In order to make TAP existence independent of its number of queues -
an extra file descriptor is opened on TAP creation and is closed on
TAP closure. It's only purpose is to serve as a keep-alive mechanism
for the TAP device.

Fixes: bf7b7f437b49 ("net/tap: create netdevice during probing")
Cc: sta...@dpdk.org

Signed-off-by: Ophir Munk 
---
v1:
Initial release
v2:
Reword commit message (a fixing patch)

 drivers/net/tap/rte_eth_tap.c | 31 ---
 drivers/net/tap/rte_eth_tap.h |  1 +
 2 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index c006d07..6901edc 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -929,6 +929,15 @@ tap_dev_close(struct rte_eth_dev *dev)
ioctl(internals->ioctl_sock, SIOCSIFFLAGS,
&internals->remote_initial_flags);
}
+
+   if (internals->ka_fd != -1) {
+   close(internals->ka_fd);
+   internals->ka_fd = -1;
+   }
+   /*
+* Since TUN device has no more opened file descriptors
+* it will be removed from kernel
+*/
 }
 
 static void
@@ -1561,13 +1570,18 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, char 
*tap_name,
rte_memcpy(&pmd->eth_addr, mac_addr, sizeof(*mac_addr));
}
 
-   /* Immediately create the netdevice (this will create the 1st queue). */
-   /* rx queue */
-   if (tap_setup_queue(dev, pmd, 0, 1) == -1)
-   goto error_exit;
-   /* tx queue */
-   if (tap_setup_queue(dev, pmd, 0, 0) == -1)
+   /*
+* Allocate a TUN device keep-alive file descriptor that will only be
+* closed when the TUN device itself is closed or removed.
+* This keep-alive file descriptor will guarantee that the TUN device
+* exists even when all of its queues are closed
+*/
+   pmd->ka_fd = tun_alloc(pmd);
+   if (pmd->ka_fd < 0) {
+   pmd->ka_fd = -1;
+   TAP_LOG(ERR, "Unable to create %s interface", tuntap_name);
goto error_exit;
+   }
 
ifr.ifr_mtu = dev->data->mtu;
if (tap_ioctl(pmd, SIOCSIFMTU, &ifr, 1, LOCAL_AND_REMOTE) < 0)
@@ -1961,9 +1975,12 @@ rte_pmd_tap_remove(struct rte_vdev_device *dev)
 
close(internals->ioctl_sock);
rte_free(eth_dev->data->dev_private);
-
rte_eth_dev_release_port(eth_dev);
 
+   if (internals->ka_fd != -1) {
+   close(internals->ka_fd);
+   internals->ka_fd = -1;
+   }
return 0;
 }
 
diff --git a/drivers/net/tap/rte_eth_tap.h b/drivers/net/tap/rte_eth_tap.h
index babe42d..575dce4 100644
--- a/drivers/net/tap/rte_eth_tap.h
+++ b/drivers/net/tap/rte_eth_tap.h
@@ -81,6 +81,7 @@ struct pmd_internals {
struct rx_queue rxq[RTE_PMD_TAP_MAX_QUEUES]; /* List of RX queues */
struct tx_queue txq[RTE_PMD_TAP_MAX_QUEUES]; /* List of TX queues */
struct rte_intr_handle intr_handle;  /* LSC interrupt handle. */
+   int ka_fd;/* keep-alive file descriptor */
 };
 
 /* tap_intr.c */
-- 
2.7.4

[dpdk-dev] [PATCH] test/eventdev: add multi port test to suite

[Vipin Varghese]

Add a new test to enhance the test suite, to allow multiple eth
ports rx queue to be added to rx bridge adapter. Update the test
function to reflect change of port index from 8 to 16 bit

Signed-off-by: Vipin Varghese 
---
 test/test/test_event_eth_rx_adapter.c | 87 +--
 1 file changed, 84 insertions(+), 3 deletions(-)

diff --git a/test/test/test_event_eth_rx_adapter.c 
b/test/test/test_event_eth_rx_adapter.c
index ab55398..0af95be 100644
--- a/test/test/test_event_eth_rx_adapter.c
+++ b/test/test/test_event_eth_rx_adapter.c
@@ -30,7 +30,7 @@ struct event_eth_rx_adapter_test_params {
 static struct event_eth_rx_adapter_test_params default_params;
 
 static inline int
-port_init(uint8_t port, struct rte_mempool *mp)
+port_init(uint16_t port, struct rte_mempool *mp)
 {
static const struct rte_eth_conf port_conf_default = {
.rxmode = {
@@ -107,15 +107,21 @@ port_init(uint8_t port, struct rte_mempool *mp)
 static int
 init_ports(int num_ports)
 {
-   uint8_t portid;
+   uint16_t portid;
int retval;
 
-   default_params.mp = rte_pktmbuf_pool_create("packet_pool",
+   struct rte_mempool *ptr = rte_mempool_lookup("packet_pool");
+
+   if (ptr == NULL)
+   default_params.mp = rte_pktmbuf_pool_create("packet_pool",
NB_MBUFS,
MBUF_CACHE_SIZE,
MBUF_PRIV_SIZE,
RTE_MBUF_DEFAULT_BUF_SIZE,
rte_socket_id());
+   else
+   default_params.mp = ptr;
+
if (!default_params.mp)
return -ENOMEM;
 
@@ -333,6 +339,80 @@ adapter_queue_add_del(void)
 }
 
 static int
+adapter_multi_eth_add_del(void)
+{
+   int err;
+   struct rte_event ev;
+   uint32_t cap;
+
+   char tun_drv_name[20] = "net_null";
+   uint16_t port_index, tun_id = 0;
+   char tun_driver_name[50] = "\0";
+
+   struct rte_event_eth_rx_adapter_queue_conf queue_config;
+
+   err = rte_event_eth_rx_adapter_caps_get(TEST_DEV_ID, TEST_ETHDEV_ID,
+   &cap);
+   TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+
+   ev.queue_id = 0;
+   ev.sched_type = RTE_SCHED_TYPE_ATOMIC;
+   ev.priority = 0;
+
+   queue_config.rx_queue_flags = 0;
+   if (cap & RTE_EVENT_ETH_RX_ADAPTER_CAP_OVERRIDE_FLOW_ID) {
+   ev.flow_id = 1;
+   queue_config.rx_queue_flags =
+   RTE_EVENT_ETH_RX_ADAPTER_QUEUE_FLOW_ID_VALID;
+   }
+   queue_config.ev = ev;
+   queue_config.servicing_weight = 1;
+
+   /* stop eth devices for existing */
+   port_index = 0;
+   for (; port_index < rte_eth_dev_count_total(); port_index += 1)
+   rte_eth_dev_stop(port_index);
+
+   /* add the max port for rx_adapter */
+   port_index = rte_eth_dev_count_total();
+   for (; port_index < RTE_MAX_ETHPORTS; port_index += 1) {
+   sprintf(tun_driver_name, "%s%u", tun_drv_name, tun_id);
+   err = rte_vdev_init(tun_driver_name, NULL);
+   TEST_ASSERT(err == 0, "Failed driver %s got %d",
+   tun_driver_name, err);
+   tun_id += 1;
+   }
+
+   err = init_ports(rte_eth_dev_count_total());
+   TEST_ASSERT(err == 0, "Port initialization failed err %d\n", err);
+
+   /* to work around the issue in rte_event_eth_rx_adapter_create_ext */
+   adapter_free();
+   adapter_create();
+
+   /* eth_rx_adapter_queue_add for n ports */
+   port_index = 0;
+   for (; port_index < rte_eth_dev_count_total(); port_index += 1) {
+   err = rte_event_eth_rx_adapter_queue_add(TEST_INST_ID,
+   port_index, 0,
+   &queue_config);
+   TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+   }
+
+   /* eth_rx_adapter_queue_del n ports */
+   port_index = 0;
+   for (; port_index < rte_eth_dev_count_total(); port_index += 1) {
+   err = rte_event_eth_rx_adapter_queue_del(TEST_INST_ID,
+   port_index, 0);
+   TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+   }
+
+   adapter_free();
+
+   return TEST_SUCCESS;
+}
+
+static int
 adapter_start_stop(void)
 {
int err;
@@ -410,6 +490,7 @@ static struct unit_test_suite service_tests  = {
TEST_CASE_ST(NULL, NULL, adapter_create_free),
TEST_CASE_ST(adapter_create, adapter_free,
adapter_queue_add_del),
+   TEST_CASE_ST(NULL, NULL, adapter_multi_eth_add_del),
TEST_CASE_ST(adapter_create, adapter_free, adapter_start_stop),
TEST_CASE_ST(adapter_create, adapter_free, adapter_stats),
TEST_CASES_END() /**< NULL ter

Re: [dpdk-dev] [PATCH v4 01/23] lib/librte_eal: import libbsd strlcpy

[Bruce Richardson]

On Mon, May 14, 2018 at 01:09:32PM +0800, Andy Green wrote:
> Signed-off-by: Andy Green 
> ---
>  lib/librte_eal/common/eal_common_string_fns.c  |   34 
> 
>  lib/librte_eal/common/include/rte_string_fns.h |7 +
>  2 files changed, 36 insertions(+), 5 deletions(-)
> 

While I'm aware this was suggested by other reviewers, I really don't feel
that it is necessary to actually import the code. If libbsd is present on
the system, we will use it directly. If libbsd is not present, the snprintf
provides an acceptable fallback for strlcpy IMHO. Having the full function
without good justification seems excessive.

/Bruce

Re: [dpdk-dev] [PATCH v4 05/23] /lib/librte_eal: stage cast from uint64 to long

[Bruce Richardson]

On Mon, May 14, 2018 at 01:09:52PM +0800, Andy Green wrote:
> /projects/lagopus/src/dpdk/build/include/rte_random.h:
> In function 'rte_srand':
> /projects/lagopus/src/dpdk/build/include/rte_random.h:34:10:
> warning: conversion to 'long int' from 'long unsigned int'
> may change the sign of the result [-Wsign-conversion]
>   srand48((long unsigned int)seedval);
> 
> /projects/lagopus/src/dpdk/build/include/rte_random.h:51:8:
> warning: conversion to 'uint64_t' {aka 'long unsigned int'}
> from 'long int' may change the sign of the result
> [-Wsign-conversion]
>   val = lrand48();
> ^~~
> /projects/lagopus/src/dpdk/build/include/rte_random.h:53:6:
> warning: conversion to 'long unsigned int' from 'long int'
> may change the sign of the result [-Wsign-conversion]
>   val += lrand48();
> 
> Signed-off-by: Andy Green 
> ---
>  lib/librte_eal/common/include/rte_random.h |6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/librte_eal/common/include/rte_random.h 
> b/lib/librte_eal/common/include/rte_random.h
> index 63bb28088..e30777b83 100644
> --- a/lib/librte_eal/common/include/rte_random.h
> +++ b/lib/librte_eal/common/include/rte_random.h
> @@ -31,7 +31,7 @@ extern "C" {
>  static inline void
>  rte_srand(uint64_t seedval)
>  {
> - srand48((long unsigned int)seedval);
> + srand48((long)(unsigned long)seedval);

Is double-cast necessary here? Can we not just cast straight to long?

>  }
>  
>  /**
> @@ -48,9 +48,9 @@ static inline uint64_t
>  rte_rand(void)
>  {
>   uint64_t val;
> - val = lrand48();
> + val = (uint64_t)lrand48();
>   val <<= 32;
> - val += lrand48();
> + val += (uint64_t)lrand48();
>   return val;
>  }
>  
> 

Apart from the one minor comment above LGTM

Acked-by: Bruce Richardson 

Re: [dpdk-dev] [PATCH v2] net/mlx5: add bluefield device ID

[Ferruh Yigit]

On 5/15/2018 10:28 AM, Nélio Laranjeiro wrote:
> On Tue, May 15, 2018 at 09:12:50AM +0300, Shahaf Shuler wrote:
>> Signed-off-by: Shahaf Shuler 
> 
> Acked-by: Nelio Laranjeiro 

Is web patch coming to introduce the device support in
https://dpdk.org/doc/nics?

Re: [dpdk-dev] [PATCH v4 10/23] rte_mbuf.h: make sure RTE-MIN compares same types

[Bruce Richardson]

On Mon, May 14, 2018 at 01:10:17PM +0800, Andy Green wrote:
> /projects/lagopus/src/dpdk/build/include/rte_common.h:384:2:
> warning: conversion from 'int' to 'uint16_t'
> {aka 'short unsigned int'} may change value [-Wconversion]
>   __extension__ ({ \
>   ^
> /projects/lagopus/src/dpdk/build/include/rte_mbuf.h:1204:16:
> note: in expansion of macro 'RTE_MIN'
>   m->data_off = RTE_MIN(RTE_PKTMBUF_HEADROOM,
> (uint16_t)m->buf_len);
> 
> RTE_PKTMBUF_HEADROOM is typ 128, so it doesn't make trouble.
> 
> Signed-off-by: Andy Green 
> ---
>  lib/librte_mbuf/rte_mbuf.h |3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index c214f1945..a27dbb878 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -1201,7 +1201,8 @@ rte_pktmbuf_priv_size(struct rte_mempool *mp)
>   */
>  static inline void rte_pktmbuf_reset_headroom(struct rte_mbuf *m)
>  {
> - m->data_off = RTE_MIN(RTE_PKTMBUF_HEADROOM, (uint16_t)m->buf_len);
> + m->data_off = (uint16_t)RTE_MIN((uint16_t)RTE_PKTMBUF_HEADROOM,
> + (uint16_t)m->buf_len);
>  }
>  
Not sure the cast before RTE_MIN is necessary, but it's harmless.

Acked-by: Bruce Richardson 

Re: [dpdk-dev] [PATCH v4 11/23] rte_mbuf.h: explicit cast restricting ptrdiff to uint16

[Bruce Richardson]

On Mon, May 14, 2018 at 01:10:22PM +0800, Andy Green wrote:
> /projects/lagopus/src/dpdk/build/include/rte_common.h:141:34:
> warning: conversion from 'long unsigned int' to 'uint16_t'
> {aka 'short unsigned int'} may change value [-Wconversion]
>  #define RTE_PTR_DIFF(ptr1, ptr2) ((uintptr_t)(ptr1) -
> (uintptr_t)(ptr2))
>   ^
> /projects/lagopus/src/dpdk/build/include/rte_mbuf.h:1360:13:
> note: in expansion of macro 'RTE_PTR_DIFF'
>   *buf_len = RTE_PTR_DIFF(shinfo, buf_addr);
> 
> Signed-off-by: Andy Green 
> ---
>  lib/librte_mbuf/rte_mbuf.h |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index a27dbb878..0580ec8a0 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -1358,7 +1358,7 @@ rte_pktmbuf_ext_shinfo_init_helper(void *buf_addr, 
> uint16_t *buf_len,
>   shinfo->fcb_opaque = fcb_opaque;
>   rte_mbuf_ext_refcnt_set(shinfo, 1);
>  
> - *buf_len = RTE_PTR_DIFF(shinfo, buf_addr);
> + *buf_len = (uint16_t)RTE_PTR_DIFF(shinfo, buf_addr);
>   return shinfo;
>  }
Acked-by: Bruce Richardson 

Re: [dpdk-dev] [PATCH v4 12/23] rte_mbuf.h: explicit cast for size type to uint32

[Bruce Richardson]

On Mon, May 14, 2018 at 01:10:27PM +0800, Andy Green wrote:
> Signed-off-by: Andy Green 
> ---
>  lib/librte_mbuf/rte_mbuf.h |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index 0580ec8a0..169f3d3b0 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -1577,7 +1577,7 @@ static inline void rte_pktmbuf_detach(struct rte_mbuf 
> *m)
>   __rte_pktmbuf_free_direct(m);
>  
>   priv_size = rte_pktmbuf_priv_size(mp);
> - mbuf_size = sizeof(struct rte_mbuf) + priv_size;
> + mbuf_size = (uint32_t)sizeof(struct rte_mbuf) + priv_size;
>   buf_len = rte_pktmbuf_data_room_size(mp);
>  
>   m->priv_size = priv_size;
> 
It would be good to include the error message in the commit log.
Also, for safety, is it better to have extra braces to cast the whole
expression, not just the sizeof, i.e.

mbuf_size = (uint32_t)(sizeof(...) + priv_size);

/Bruce

[dpdk-dev] [PATCH] ethdev: update description for functions

[Vipin Varghese]

Change adds extra information on name parameter for API
rte_eth_dev_get_name_by_port and rte_eth_dev_get_port_by_name.

Signed-off-by: Vipin Varghese 
---
 lib/librte_ethdev/rte_ethdev.h | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index f8815e9..7387686 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -3657,7 +3657,10 @@ rte_eth_dev_l2_tunnel_offload_set(uint16_t port_id,
 
 /**
 * Get the port id from pci address or device name
-* Ex: :2:00.0 or vdev name net_pcap0
+* Ex:
+* - PCIe, :2:00.0
+* - SoC, fslgmac0
+* - vdev driver name, net_pcap0
 *
 * @param name
 *  pci address or name of the device
@@ -3672,6 +3675,10 @@ rte_eth_dev_get_port_by_name(const char *name, uint16_t 
*port_id);
 
 /**
 * Get the device name from port id
+* Ex:
+* - PCIe Bus:Domian:Function, :02:00.0
+* - SoC device name, fslgmac0
+* - VDEV driver name, net_pcap0, net_null0, net_tun0 or net_tap0
 *
 * @param port_id
 *   Port identifier of the device.
-- 
2.7.4

Re: [dpdk-dev] [PATCH v4 13/23] rte_mbuf.h: explicit casts to uint16 to avoid warnings

[Bruce Richardson]

On Mon, May 14, 2018 at 01:10:32PM +0800, Andy Green wrote:
> Signed-off-by: Andy Green 
> ---
>  lib/librte_mbuf/rte_mbuf.h |   15 ---
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index 169f3d3b0..3cd76abbc 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -1580,7 +1580,7 @@ static inline void rte_pktmbuf_detach(struct rte_mbuf 
> *m)
>   mbuf_size = (uint32_t)sizeof(struct rte_mbuf) + priv_size;
>   buf_len = rte_pktmbuf_data_room_size(mp);
>  
> - m->priv_size = priv_size;
> + m->priv_size = (uint16_t)priv_size;
>   m->buf_addr = (char *)m + mbuf_size;
>   m->buf_iova = rte_mempool_virt2iova(m) + mbuf_size;
>   m->buf_len = (uint16_t)buf_len;

I think a better fix for this is to change priv_size to be uint16_t. There
is no reason for it to be a 32-bit value.


> @@ -1905,7 +1905,7 @@ static inline char *rte_pktmbuf_prepend(struct rte_mbuf 
> *m,
>   if (unlikely(len > rte_pktmbuf_headroom(m)))
>   return NULL;
>  
> - m->data_off -= len;
> + m->data_off = (uint16_t)(m->data_off - len);
>   m->data_len = (uint16_t)(m->data_len + len);
>   m->pkt_len  = (m->pkt_len + len);
>  

What is the error message here? The cast isn't wrong, it just seems like we
shouldn't need it. I suppose it's implicit promotion being done in the
subtraction operation.

> @@ -1966,7 +1966,7 @@ static inline char *rte_pktmbuf_adj(struct rte_mbuf *m, 
> uint16_t len)
>   return NULL;
>  
>   m->data_len = (uint16_t)(m->data_len - len);
> - m->data_off += len;
> + m->data_off = (uint16_t)(m->data_off + len);
>   m->pkt_len  = (m->pkt_len - len);
>   return (char *)m->buf_addr + m->data_off;
>  }
> @@ -2079,7 +2079,7 @@ static inline int rte_pktmbuf_chain(struct rte_mbuf 
> *head, struct rte_mbuf *tail
>   cur_tail->next = tail;
>  
>   /* accumulate number of segments and total length. */
> - head->nb_segs += tail->nb_segs;
> + head->nb_segs = (uint16_t)(head->nb_segs + tail->nb_segs);
>   head->pkt_len += tail->pkt_len;
>  
>   /* pkt_len is only set in the head */
> @@ -2109,7 +2109,8 @@ rte_validate_tx_offload(const struct rte_mbuf *m)
>   return 0;
>  
>   if (ol_flags & PKT_TX_OUTER_IP_CKSUM)
> - inner_l3_offset += m->outer_l2_len + m->outer_l3_len;
> + inner_l3_offset += (unsigned int)(m->outer_l2_len +
> +   m->outer_l3_len);
>  
>   /* Headers are fragmented */
>   if (rte_pktmbuf_data_len(m) < inner_l3_offset + m->l3_len + m->l4_len)
> @@ -2154,7 +2155,7 @@ rte_validate_tx_offload(const struct rte_mbuf *m)
>  static inline int
>  rte_pktmbuf_linearize(struct rte_mbuf *mbuf)
>  {
> - int seg_len, copy_len;
> + size_t seg_len, copy_len;
>   struct rte_mbuf *m;
>   struct rte_mbuf *m_next;
>   char *buffer;
> @@ -2169,7 +2170,7 @@ rte_pktmbuf_linearize(struct rte_mbuf *mbuf)
>   return -1;
>  
>   buffer = rte_pktmbuf_mtod_offset(mbuf, char *, mbuf->data_len);
> - mbuf->data_len = (uint16_t)(mbuf->pkt_len);
> + mbuf->data_len = (uint16_t)mbuf->pkt_len;
>  
>   /* Append data from next segments to the first one */
>   m = mbuf->next;
> 

Re: [dpdk-dev] [PATCH v4 14/23] rte_ether.h: explicit cast avoiding truncation warning

[Bruce Richardson]

On Mon, May 14, 2018 at 01:10:37PM +0800, Andy Green wrote:
> /projects/lagopus/src/dpdk/build/include/rte_ether.h:213:13:
> warning: conversion from 'int' to 'uint8_t'
> {aka 'unsigned char'} may change value [-Wconversion]
>   addr[0] &= ~ETHER_GROUP_ADDR;
> /* clear multicast bit */
> 
> Signed-off-by: Andy Green 

Acked-by: Bruce Richardson 

Re: [dpdk-dev] [PATCH v4 11/23] rte_mbuf.h: explicit cast restricting ptrdiff to uint16

[Ananyev, Konstantin]

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Bruce Richardson
> Sent: Thursday, May 17, 2018 11:51 AM
> To: Andy Green 
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4 11/23] rte_mbuf.h: explicit cast 
> restricting ptrdiff to uint16
> 
> On Mon, May 14, 2018 at 01:10:22PM +0800, Andy Green wrote:
> > /projects/lagopus/src/dpdk/build/include/rte_common.h:141:34:
> > warning: conversion from 'long unsigned int' to 'uint16_t'
> > {aka 'short unsigned int'} may change value [-Wconversion]
> >  #define RTE_PTR_DIFF(ptr1, ptr2) ((uintptr_t)(ptr1) -
> > (uintptr_t)(ptr2))
> >   ^
> > /projects/lagopus/src/dpdk/build/include/rte_mbuf.h:1360:13:
> > note: in expansion of macro 'RTE_PTR_DIFF'
> >   *buf_len = RTE_PTR_DIFF(shinfo, buf_addr);
> >
> > Signed-off-by: Andy Green 
> > ---
> >  lib/librte_mbuf/rte_mbuf.h |2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> > index a27dbb878..0580ec8a0 100644
> > --- a/lib/librte_mbuf/rte_mbuf.h
> > +++ b/lib/librte_mbuf/rte_mbuf.h
> > @@ -1358,7 +1358,7 @@ rte_pktmbuf_ext_shinfo_init_helper(void *buf_addr, 
> > uint16_t *buf_len,
> > shinfo->fcb_opaque = fcb_opaque;
> > rte_mbuf_ext_refcnt_set(shinfo, 1);
> >
> > -   *buf_len = RTE_PTR_DIFF(shinfo, buf_addr);
> > +   *buf_len = (uint16_t)RTE_PTR_DIFF(shinfo, buf_addr);
> > return shinfo;
> >  }
> Acked-by: Bruce Richardson 


Just as a nit: if all such conversions are unavoidable - might be introduce:
RTE_PTR_DIFF_TYPE(.., type) ((type)(RTE_PTR_DIFF(...))
Might look a bit nicer.
Konstantin

[dpdk-dev] [PATCH v1] net/mlx4: add an RSS hash update callback

[Ophir Munk]

Add an RSS hash update callback to eth_dev_ops.

Signed-off-by: Ophir Munk 
---
 drivers/net/mlx4/Makefile   |  1 +
 drivers/net/mlx4/mlx4.c |  1 +
 drivers/net/mlx4/mlx4.h |  7 ++
 drivers/net/mlx4/mlx4_rss.c | 59 +
 4 files changed, 68 insertions(+)
 create mode 100644 drivers/net/mlx4/mlx4_rss.c

diff --git a/drivers/net/mlx4/Makefile b/drivers/net/mlx4/Makefile
index 73f9d40..eb89f6b 100644
--- a/drivers/net/mlx4/Makefile
+++ b/drivers/net/mlx4/Makefile
@@ -23,6 +23,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_MLX4_PMD) += mlx4_rxq.c
 SRCS-$(CONFIG_RTE_LIBRTE_MLX4_PMD) += mlx4_rxtx.c
 SRCS-$(CONFIG_RTE_LIBRTE_MLX4_PMD) += mlx4_txq.c
 SRCS-$(CONFIG_RTE_LIBRTE_MLX4_PMD) += mlx4_utils.c
+SRCS-$(CONFIG_RTE_LIBRTE_MLX4_PMD) += mlx4_rss.c
 
 ifeq ($(CONFIG_RTE_LIBRTE_MLX4_DLOPEN_DEPS),y)
 INSTALL-$(CONFIG_RTE_LIBRTE_MLX4_PMD)-lib += $(LIB_GLUE)
diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
index 9f8ecd0..7000511 100644
--- a/drivers/net/mlx4/mlx4.c
+++ b/drivers/net/mlx4/mlx4.c
@@ -261,6 +261,7 @@ static const struct eth_dev_ops mlx4_dev_ops = {
.flow_ctrl_get = mlx4_flow_ctrl_get,
.flow_ctrl_set = mlx4_flow_ctrl_set,
.mtu_set = mlx4_mtu_set,
+   .rss_hash_update = mlx4_rss_hash_update,
.filter_ctrl = mlx4_filter_ctrl,
.rx_queue_intr_enable = mlx4_rx_intr_enable,
.rx_queue_intr_disable = mlx4_rx_intr_disable,
diff --git a/drivers/net/mlx4/mlx4.h b/drivers/net/mlx4/mlx4.h
index 300cb4d..6842a71 100644
--- a/drivers/net/mlx4/mlx4.h
+++ b/drivers/net/mlx4/mlx4.h
@@ -50,6 +50,9 @@
 /** Port parameter. */
 #define MLX4_PMD_PORT_KVARG "port"
 
+/** Supported RSS. */
+#define MLX4_RSS_HF_MASK (~(ETH_RSS_IP | ETH_RSS_UDP | ETH_RSS_TCP))
+
 enum {
PCI_VENDOR_ID_MELLANOX = 0x15b3,
 };
@@ -144,4 +147,8 @@ void mlx4_rxq_intr_disable(struct priv *priv);
 int mlx4_rx_intr_disable(struct rte_eth_dev *dev, uint16_t idx);
 int mlx4_rx_intr_enable(struct rte_eth_dev *dev, uint16_t idx);
 
+/* mlx4_rss.c */
+
+int mlx4_rss_hash_update(struct rte_eth_dev *dev,
+struct rte_eth_rss_conf *rss_conf);
 #endif /* RTE_PMD_MLX4_H_ */
diff --git a/drivers/net/mlx4/mlx4_rss.c b/drivers/net/mlx4/mlx4_rss.c
new file mode 100644
index 000..656a00d
--- /dev/null
+++ b/drivers/net/mlx4/mlx4_rss.c
@@ -0,0 +1,59 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright 2015 6WIND S.A.
+ * Copyright 2015 Mellanox Technologies, Ltd
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+/* Verbs headers do not support -pedantic. */
+/* ISO C doesn't support unnamed structs/unions, disabling -pedantic. */
+#ifdef PEDANTIC
+#pragma GCC diagnostic ignored "-Wpedantic"
+#endif
+#include 
+#ifdef PEDANTIC
+#pragma GCC diagnostic error "-Wpedantic"
+#endif
+
+#include "mlx4.h"
+#include "mlx4_flow.h"
+#include "mlx4_utils.h"
+
+/**
+ * DPDK callback to update the RSS hash configuration.
+ *
+ * @param dev
+ *   Pointer to Ethernet device structure.
+ * @param[in] rss_conf
+ *   RSS configuration data.
+ *
+ * @return
+ *   0 on success, a negative errno value otherwise and rte_errno is set.
+ */
+int
+mlx4_rss_hash_update(struct rte_eth_dev *dev,
+struct rte_eth_rss_conf *rss_conf)
+{
+   /*
+* limitation: MLX4 supports all of IP, UDP and TCP hash
+* functions together and not in partial combinations
+* Just make sure no unsupported HF is requested
+*/
+   if (rss_conf->rss_hf & MLX4_RSS_HF_MASK) {
+   rte_errno = EINVAL;
+   return -rte_errno;
+   }
+   if (rss_conf->rss_key && rss_conf->rss_key_len) {
+   /*
+* Currently mlx4 RSS key cannot be updated
+*/
+   ERROR("port %u RSS key cannot be updated",
+   dev->data->port_id);
+   rte_errno = EINVAL;
+   return -rte_errno;
+   }
+   return 0;
+}
-- 
2.7.4

Re: [dpdk-dev] [PATCH v3 3/4] doc: announce deprecation for attach/detach crypto session

[Trahe, Fiona]

Hi Pablo,



> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Pablo de Lara
> Sent: Thursday, May 17, 2018 10:01 AM
> To: Doherty, Declan ; akhil.go...@nxp.com
> Cc: dev@dpdk.org; De Lara Guarch, Pablo 
> Subject: [dpdk-dev] [PATCH v3 3/4] doc: announce deprecation for 
> attach/detach crypto session
> 
> Functions rte_cryptodev_queue_pair_attach_sym_session
> and rte_cryptodev_queue_pair_detach_sym_sessions
> are not really used in any of the crypto drivers
> (only one driver implements it and it just return 0).
> Therefore, this API can be deprecated from 18.05
> and removed in 18.08.
> 
> Signed-off-by: Pablo de Lara 
> ---
>  doc/guides/rel_notes/deprecation.rst | 4 
>  lib/librte_cryptodev/rte_cryptodev.h | 2 ++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst 
> b/doc/guides/rel_notes/deprecation.rst
> index 85945ee72..cd75150a6 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -80,3 +80,7 @@ Deprecation Notices
>  is not internal in the crypto device anymore.
>- Replacement of ``pci_dev`` field with the more generic ``rte_device``
>  structure.
> +  - Functions ``rte_cryptodev_queue_pair_attach_sym_session()`` and
> +``rte_cryptodev_queue_pair_dettach_sym_session()`` will be deprecated 
> from
> +18.02 and removed in 18.05, as there are no drivers doing anything useful
[Fiona] This should read 18.05 / 18.08

Apart from that 
Acked-by: Fiona Trahe 

Re: [dpdk-dev] [PATCH v3 2/4] doc: announce ABI change for crypto info struct

[Trahe, Fiona]

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Pablo de Lara
> Sent: Thursday, May 17, 2018 10:01 AM
> To: Doherty, Declan ; akhil.go...@nxp.com
> Cc: dev@dpdk.org; De Lara Guarch, Pablo 
> Subject: [dpdk-dev] [PATCH v3 2/4] doc: announce ABI change for crypto info 
> struct
> 
> Cryptodev info structure currently contains
> a pointer to an rte_pci_device structure.
> This field depends on a specific bus type (PCI),
> which is not following a bus independent design.
> Following the same approach taken in ethdev, the field
> will be replaced with a pointer to an rte_device structure.
> 
> Signed-off-by: Pablo de Lara 

Acked-by: Fiona Trahe 

Re: [dpdk-dev] [PATCH] maintainers: fix responsibility of flow API bits

[Adrien Mazarguil]

On Thu, May 17, 2018 at 09:26:15AM +, Iremonger, Bernard wrote:
> Hi Adrien,
> 
> 
> 
> > > > Subject: [PATCH] maintainers: fix responsibility of flow API bits
> > > >
> > > > The following commits lack MAINTAINERS entries for this mess.
> > > >
> > > > Fixes: 4d73b6fb9907 ("doc: add generic flow API guide")
> > > > Fixes: 19c90af6285c ("app/testpmd: add flow command")
> > > > Cc: sta...@dpdk.org
> > > >
> > > > Signed-off-by: Adrien Mazarguil 
> > > > Cc: Bernard Iremonger 
> > > > ---
> > > >  MAINTAINERS | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/MAINTAINERS b/MAINTAINERS index bd08d4292..187817fff
> > > > 100644
> > > > --- a/MAINTAINERS
> > > > +++ b/MAINTAINERS
> > > > @@ -303,6 +303,8 @@ F: devtools/test-null.sh  Flow API
> > > >  M: Adrien Mazarguil 
> > > >  T: git://dpdk.org/next/dpdk-next-net
> > > > +F: app/test-pmd/cmdline_flow.c
> > > > +F: doc/guides/prog_guide/rte_flow.rst
> > >
> > > doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > > Should be added to the list of flow related files.
> > 
> > I did not add it because there is no way to split responsibility on a
> > documentation section basis AFAIK, and only a fraction of this file contains
> > information about rte_flow-related stuff.
> > 
> > --
> > Adrien Mazarguil
> > 6WIND
> 
> There are already several maintainers for testpmd, adding another maintainer 
> for the flow parts of this file should be ok (I think).

True, however providing an exact file name instead of a directory or
wildcard makes me the main maintainer/contact for that specific file
(e.g. "app/test-pmd/cmdline_flow.c" vs. "app/test-pmd"), basically I do not
want to be responsible for the entirety of testpmd's documentation.

How about the following instead: another patch that extracts the flow
command documentation in its own file (testpmd_flow.rst?) using the include 
directive (somewhat like cmdline_flow.c). This patch will update MAINTAINERS
accordingly.

-- 
Adrien Mazarguil
6WIND

Re: [dpdk-dev] [PATCH] app/testpmd: add to call detach for vdev when quitting

[Iremonger, Bernard]

Hi Zhiyong,

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of
> zhiyong.y...@intel.com
> Sent: Thursday, May 17, 2018 2:00 PM
> To: dev@dpdk.org
> Cc: maxime.coque...@redhat.com; Yigit, Ferruh ; Bie,
> Tiwei ; Yao, Lei A ; Yang, Zhiyong
> 
> Subject: [dpdk-dev] [PATCH] app/testpmd: add to call detach for vdev when
> quitting
> 
> For vdev, just calling rte_eth_dev_close() isn't enough to free all the 
> resources
> allocated during device probe, e.g. for virtio-user, virtio_user_pmd_remove(),
> i.e. the remove() method of a vdev driver, needs to be called to unlink the 
> socket
> file created during device probe. So this patch calls the 
> rte_eth_dev_detach() for
> vdev when quitting testpmd.
> 
> Cc: maxime.coque...@redhat.com
> Cc: ferruh.yi...@intel.com
> Cc: tiwei@intel.com
> Cc: lei.a@intel.com
> 
> Signed-off-by: Zhiyong Yang 
> ---
>  app/test-pmd/testpmd.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> 134401603..1d308f056 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -2011,6 +2011,8 @@ detach_port(portid_t port_id)  void
>  pmd_test_exit(void)
>  {
> + const struct rte_bus *bus;
> + struct rte_device *device;
>   portid_t pt_id;
>   int ret;
> 
> @@ -2020,10 +2022,14 @@ pmd_test_exit(void)
>   if (ports != NULL) {
>   no_link_check = 1;
>   RTE_ETH_FOREACH_DEV(pt_id) {
> + device = rte_eth_devices[pt_id].device;
> + bus = rte_bus_find_by_device(device);
>   printf("\nShutting down port %d...\n", pt_id);
>   fflush(stdout);
>   stop_port(pt_id);
>   close_port(pt_id);
> + if (bus && !strcmp(bus->name, "vdev"))
> + detach_port(pt_id);
>   }
>   }
> 
> --
> 2.14.3

This appears to be a bug fix patch, if so it should have a fixes line.
Also the commit line should include "fix", for example:
"app/testpmd: fix pmd_test_exit function for vdevs"

Regards,

Bernard.

Re: [dpdk-dev] [PATCH v2] crypto/scheduler: fix multicore rings re-use

[Pattan, Reshma]

Hi,

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Kirill Rybalchenko
> Sent: Wednesday, May 16, 2018 3:25 PM
> To: dev@dpdk.org
> Cc: sta...@dpdk.org; Rybalchenko, Kirill ;
> Zhang, Roy Fan 
> Subject: [dpdk-dev] [PATCH v2] crypto/scheduler: fix multicore rings re-use
> 
> When scheduler mode changed from multicore to roundrobin and back to
> multicore, scheduler tries to create memory rings with the same name and
> fails. The fix allows to lookup and re-use previously allocated memory rings.
> 
> Fixes: 4c07e0552f0a ("crypto/scheduler: add multicore scheduling mode")
> Cc: sta...@dpdk.org
> 
> v2:
>  Rebase to head of dpdk-dev master branch
> 
> Signed-off-by: Kirill Rybalchenko 

Tested-by: Reshma Pattan 

Re: [dpdk-dev] [PATCH] maintainers: fix responsibility of flow API bits

[Iremonger, Bernard]

Hi Adrien,




> > > > > Subject: [PATCH] maintainers: fix responsibility of flow API
> > > > > bits
> > > > >
> > > > > The following commits lack MAINTAINERS entries for this mess.
> > > > >
> > > > > Fixes: 4d73b6fb9907 ("doc: add generic flow API guide")
> > > > > Fixes: 19c90af6285c ("app/testpmd: add flow command")
> > > > > Cc: sta...@dpdk.org
> > > > >
> > > > > Signed-off-by: Adrien Mazarguil 
> > > > > Cc: Bernard Iremonger 
> > > > > ---
> > > > >  MAINTAINERS | 2 ++
> > > > >  1 file changed, 2 insertions(+)
> > > > >
> > > > > diff --git a/MAINTAINERS b/MAINTAINERS index
> > > > > bd08d4292..187817fff
> > > > > 100644
> > > > > --- a/MAINTAINERS
> > > > > +++ b/MAINTAINERS
> > > > > @@ -303,6 +303,8 @@ F: devtools/test-null.sh  Flow API
> > > > >  M: Adrien Mazarguil 
> > > > >  T: git://dpdk.org/next/dpdk-next-net
> > > > > +F: app/test-pmd/cmdline_flow.c
> > > > > +F: doc/guides/prog_guide/rte_flow.rst
> > > >
> > > > doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > > > Should be added to the list of flow related files.
> > >
> > > I did not add it because there is no way to split responsibility on
> > > a documentation section basis AFAIK, and only a fraction of this
> > > file contains information about rte_flow-related stuff.
> > >
> > > --
> > > Adrien Mazarguil
> > > 6WIND
> >
> > There are already several maintainers for testpmd, adding another maintainer
> for the flow parts of this file should be ok (I think).
> 
> True, however providing an exact file name instead of a directory or wildcard
> makes me the main maintainer/contact for that specific file (e.g. "app/test-
> pmd/cmdline_flow.c" vs. "app/test-pmd"), basically I do not want to be
> responsible for the entirety of testpmd's documentation.
> 
> How about the following instead: another patch that extracts the flow
> command documentation in its own file (testpmd_flow.rst?) using the include
> directive (somewhat like cmdline_flow.c). This patch will update MAINTAINERS
> accordingly.
> 
> --
> Adrien Mazarguil
> 6WIND

Sounds good to me.

Regards,

Bernard.

Re: [dpdk-dev] [PATCH v3 4/4] doc: announce deprecation in crypto queue pair start/stop

[Trahe, Fiona]

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Pablo de Lara
> Sent: Thursday, May 17, 2018 10:01 AM
> To: Doherty, Declan ; akhil.go...@nxp.com
> Cc: dev@dpdk.org; De Lara Guarch, Pablo 
> Subject: [dpdk-dev] [PATCH v3 4/4] doc: announce deprecation in crypto queue 
> pair start/stop
> 
> Functions rte_cryptodev_queue_pair_start/stop
> are not really used in any of the crypto drivers
> (they all just return 0 or -ENOTSUP).
> Therefore, this API can be deprecated from 18.02
> and removed in 18.05.
> 
> Signed-off-by: Pablo de Lara 
> Acked-by: Akhil Goyal 
[Fiona] I guess you mean 18.05/18.08. Apart from that 
Acked-by: Fiona Trahe 

Re: [dpdk-dev] [PATCH v3] igb_uio: fail and log if kernel lock down is enabled

[Neil Horman]

On Wed, May 16, 2018 at 03:42:20PM +0100, Ferruh Yigit wrote:
> When EFI secure boot is enabled, it is possible to lock down kernel and
> prevent accessing device BARs and this makes igb_uio unusable.
> 
> Lock down patches are not part of the vanilla kernel but they are
> applied and used by some distros already [1].
> 
> It is not possible to fix this issue, but intention of this patch is to
> detect and log if kernel lock down enabled and don't insert the module
> for that case.
> 
> The challenge is since this feature enabled by distros, they have
> different config options and APIs for it. This patch is done based on
> Fedora and Ubuntu kernel source, may needs to add more distro specific
> support.
> 
I still need to ask, what exactly is the error you're seeing with inserting the
uio module?  The lockdown patch set restricts BAR address changes, but via paths
acessible from user space, igbuio should still insert and initalize just fine
(or so it would seem to me).  Why not fix this by detecting the problem during
the user space library initalization, where you can do so via a standard method
that works accross distributions?

Neil

[dpdk-dev] [PATCH] net/avf: fix AVF traffic blocked issue

[Xiaoyun Li]

When resetting ports, traffic will be blocked. There is a mistake when
getting hw info at avf_dev_stop. This causes the device stop without
stopping queues. This patch fixes this issue.

Fixes: 69dd4c3d0898 ("net/avf: enable queue and device")
Cc: sta...@dpdk.org

Signed-off-by: Xiaoyun Li 
---
 drivers/net/avf/avf_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/avf/avf_ethdev.c b/drivers/net/avf/avf_ethdev.c
index 0ef1f17..ad83a57 100644
--- a/drivers/net/avf/avf_ethdev.c
+++ b/drivers/net/avf/avf_ethdev.c
@@ -475,7 +475,7 @@ avf_dev_stop(struct rte_eth_dev *dev)
 {
struct avf_adapter *adapter =
AVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
-   struct avf_hw *hw = AVF_DEV_PRIVATE_TO_HW(dev);
+   struct avf_hw *hw = AVF_DEV_PRIVATE_TO_HW(dev->data->dev_private);
struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
struct rte_intr_handle *intr_handle = dev->intr_handle;
int ret, i;
-- 
2.7.4

Re: [dpdk-dev] [PATCH v3] vhost: improve dirty pages logging performance

[Maxime Coquelin]

On 05/17/2018 06:50 AM, Tiwei Bie wrote:

On Thu, May 17, 2018 at 06:06:34AM +0300, Michael S. Tsirkin wrote:

On Wed, May 16, 2018 at 06:54:23PM +0200, Maxime Coquelin wrote:

This patch caches all dirty pages logging until the used ring index
is updated.

The goal of this optimization is to fix a performance regression
introduced when the vhost library started to use atomic operations
to set bits in the shared dirty log map. While the fix was valid
as previous implementation wasn't safe against concurent accesses,
contention was induced.

With this patch, during migration, we have:
1. Less atomic operations as only a single atomic OR operation
per 32 or 64 (depending on CPU) pages.
2. Less atomic operations as during a burst, the same page will
be marked dirty only once.
3. Less write memory barriers.

Fixes: 897f13a1f726 ("vhost: make page logging atomic")

Cc: sta...@dpdk.org

Cc: Tiwei Bie 
Suggested-by: Michael S. Tsirkin 
Signed-off-by: Maxime Coquelin 



Deferring updates until GET_BASE would also be possible,
but would increase the chance that a disconnect causes
ring to become inconsistent.


Yeah. The sync of the updates from vhost backend will
be deferred a lot. Another issue is that, it probably
will increase the downtime, as there will be more pages
to sync after the old device is stopped and before the
new device is started.



I'm not sure whether there is a chance of that with this
patch (in case of a crash after used idx updated but
before dirty log update of the used idx), but
at least it's not bigger than before this patch.


The used idx update and the corresponding logging
are two operations instead of one atomic operation.
So theoretically, it should be possible.

I got your point now. Maybe we should add a barrier
between cache sync and the used idx update to ensure
that all dirty pages are logged before they can be
seen by the guest.


Good idea, I'll add a carrier at the end of the sync.

Thanks,
Maxime


Best regards,
Tiwei Bie

[dpdk-dev] [PATCH v4] vhost: improve dirty pages logging performance

[Maxime Coquelin]

This patch caches all dirty pages logging until the used ring index
is updated.

The goal of this optimization is to fix a performance regression
introduced when the vhost library started to use atomic operations
to set bits in the shared dirty log map. While the fix was valid
as previous implementation wasn't safe against concurrent accesses,
contention was induced.

With this patch, during migration, we have:
1. Less atomic operations as only a single atomic OR operation
per 32 or 64 (depending on CPU) pages.
2. Less atomic operations as during a burst, the same page will
be marked dirty only once.
3. Less write memory barriers.

Fixes: 897f13a1f726 ("vhost: make page logging atomic")

Cc: sta...@dpdk.org

Cc: Tiwei Bie 
Suggested-by: Michael S. Tsirkin 
Signed-off-by: Maxime Coquelin 
---
Changes since v2:
- Fix typo in commit message (Tiwei)
- Add write barrier after log cache sync (Tiwei)

 lib/librte_vhost/vhost.h  | 119 +-
 lib/librte_vhost/virtio_net.c |  29 ++
 2 files changed, 137 insertions(+), 11 deletions(-)

diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
index 891978131..58c425a5c 100644
--- a/lib/librte_vhost/vhost.h
+++ b/lib/librte_vhost/vhost.h
@@ -36,6 +36,8 @@
 
 #define BUF_VECTOR_MAX 256
 
+#define VHOST_LOG_CACHE_NR 32
+
 /**
  * Structure contains buffer address, length and descriptor index
  * from vring to do scatter RX.
@@ -69,6 +71,14 @@ struct batch_copy_elem {
uint64_t log_addr;
 };
 
+/*
+ * Structure that contains the info for batched dirty logging.
+ */
+struct log_cache_entry {
+   uint32_t offset;
+   unsigned long val;
+};
+
 /**
  * Structure contains variables relevant to RX/TX virtqueues.
  */
@@ -112,6 +122,9 @@ struct vhost_virtqueue {
struct batch_copy_elem  *batch_copy_elems;
uint16_tbatch_copy_nb_elems;
 
+   struct log_cache_entry log_cache[VHOST_LOG_CACHE_NR];
+   uint16_t log_cache_nb_elem;
+
rte_rwlock_tiotlb_lock;
rte_rwlock_tiotlb_pending_lock;
struct rte_mempool *iotlb_pool;
@@ -309,7 +322,15 @@ struct virtio_net {
 static __rte_always_inline void
 vhost_set_bit(unsigned int nr, volatile uint8_t *addr)
 {
-   __sync_fetch_and_or_8(addr, (1U << nr));
+#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION < 70100)
+   /*
+* __sync_ built-ins are deprecated, but __atomic_ ones
+* are sub-optimized in older GCC versions.
+*/
+   __sync_fetch_and_or_1(addr, (1U << nr));
+#else
+   __atomic_fetch_or(addr, (1U << nr), __ATOMIC_RELAXED);
+#endif
 }
 
 static __rte_always_inline void
@@ -340,6 +361,102 @@ vhost_log_write(struct virtio_net *dev, uint64_t addr, 
uint64_t len)
}
 }
 
+static __rte_always_inline void
+vhost_log_cache_sync(struct virtio_net *dev, struct vhost_virtqueue *vq)
+{
+   unsigned long *log_base;
+   int i;
+
+   if (likely(((dev->features & (1ULL << VHOST_F_LOG_ALL)) == 0) ||
+  !dev->log_base))
+   return;
+
+   log_base = (unsigned long *)(uintptr_t)dev->log_base;
+
+   /*
+* It is expected a write memory barrier has been issued
+* before this function is called.
+*/
+
+   for (i = 0; i < vq->log_cache_nb_elem; i++) {
+   struct log_cache_entry *elem = vq->log_cache + i;
+
+#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION < 70100)
+   /*
+* '__sync' builtins are deprecated, but '__atomic' ones
+* are sub-optimized in older GCC versions.
+*/
+   __sync_fetch_and_or(log_base + elem->offset, elem->val);
+#else
+   __atomic_fetch_or(log_base + elem->offset, elem->val,
+   __ATOMIC_RELAXED);
+#endif
+   }
+
+   rte_smp_wmb();
+
+   vq->log_cache_nb_elem = 0;
+}
+
+static __rte_always_inline void
+vhost_log_cache_page(struct virtio_net *dev, struct vhost_virtqueue *vq,
+   uint64_t page)
+{
+   uint32_t bit_nr = page % (sizeof(unsigned long) << 3);
+   uint32_t offset = page / (sizeof(unsigned long) << 3);
+   int i;
+
+   for (i = 0; i < vq->log_cache_nb_elem; i++) {
+   struct log_cache_entry *elem = vq->log_cache + i;
+
+   if (elem->offset == offset) {
+   elem->val |= (1UL << bit_nr);
+   return;
+   }
+   }
+
+   if (unlikely(i >= VHOST_LOG_CACHE_NR)) {
+   /*
+* No more room for a new log cache entry,
+* so write the dirty log map directly.
+*/
+   rte_smp_wmb();
+   vhost_log_page((uint8_t *)(uintptr_t)dev->log_base, page);
+
+   return;
+   }
+
+   vq->log_cache[i].offset = offset;
+   vq->log_cache[i].val = (1UL << bit_nr);
+}
+
+static __rte_always_inline void
+vhost_log_cache_write(struct virtio_net *dev, struc

Re: [dpdk-dev] [PATCH v4] vhost: improve dirty pages logging performance

[Maxime Coquelin]

Hi Tiwei,

On 05/17/2018 01:44 PM, Maxime Coquelin wrote:

This patch caches all dirty pages logging until the used ring index
is updated.

The goal of this optimization is to fix a performance regression
introduced when the vhost library started to use atomic operations
to set bits in the shared dirty log map. While the fix was valid
as previous implementation wasn't safe against concurrent accesses,
contention was induced.

With this patch, during migration, we have:
1. Less atomic operations as only a single atomic OR operation
per 32 or 64 (depending on CPU) pages.
2. Less atomic operations as during a burst, the same page will
be marked dirty only once.
3. Less write memory barriers.

Fixes: 897f13a1f726 ("vhost: make page logging atomic")

Cc:sta...@dpdk.org

Cc: Tiwei Bie
Suggested-by: Michael S. Tsirkin
Signed-off-by: Maxime Coquelin


I missed to add your:
Reviewed-by: Tiwei Bie 

that you replied to v3, and since the changes in v4 were suggested by
you, I guess it still applies.

Cheers,
Maxime

Re: [dpdk-dev] [PATCH v4] vhost: improve dirty pages logging performance

[Tiwei Bie]

On Thu, May 17, 2018 at 02:01:52PM +0200, Maxime Coquelin wrote:
> Hi Tiwei,
> 
> On 05/17/2018 01:44 PM, Maxime Coquelin wrote:
> > This patch caches all dirty pages logging until the used ring index
> > is updated.
> > 
> > The goal of this optimization is to fix a performance regression
> > introduced when the vhost library started to use atomic operations
> > to set bits in the shared dirty log map. While the fix was valid
> > as previous implementation wasn't safe against concurrent accesses,
> > contention was induced.
> > 
> > With this patch, during migration, we have:
> > 1. Less atomic operations as only a single atomic OR operation
> > per 32 or 64 (depending on CPU) pages.
> > 2. Less atomic operations as during a burst, the same page will
> > be marked dirty only once.
> > 3. Less write memory barriers.
> > 
> > Fixes: 897f13a1f726 ("vhost: make page logging atomic")
> > 
> > Cc:sta...@dpdk.org
> > 
> > Cc: Tiwei Bie
> > Suggested-by: Michael S. Tsirkin
> > Signed-off-by: Maxime Coquelin
> 
> I missed to add your:
> Reviewed-by: Tiwei Bie 
> 
> that you replied to v3, and since the changes in v4 were suggested by
> you, I guess it still applies.

Yeah. Thanks! :)

Best regards,
Tiwei Bie

Re: [dpdk-dev] [PATCH] ethdev: update description for functions

[Ferruh Yigit]

On 5/17/2018 11:59 AM, Vipin Varghese wrote:
> Change adds extra information on name parameter for API
> rte_eth_dev_get_name_by_port and rte_eth_dev_get_port_by_name.
> 
> Signed-off-by: Vipin Varghese 
> ---
>  lib/librte_ethdev/rte_ethdev.h | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> index f8815e9..7387686 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -3657,7 +3657,10 @@ rte_eth_dev_l2_tunnel_offload_set(uint16_t port_id,
>  
>  /**
>  * Get the port id from pci address or device name
> -* Ex: :2:00.0 or vdev name net_pcap0
> +* Ex:
> +* - PCIe, :2:00.0
> +* - SoC, fslgmac0
> +* - vdev driver name, net_pcap0

driver name is "net_pcap", device name is "net_pcap0".
Original "vdev name" seems more accurate, is there a reason to change it to
"driver name"?

>  *
>  * @param name
>  *  pci address or name of the device
> @@ -3672,6 +3675,10 @@ rte_eth_dev_get_port_by_name(const char *name, 
> uint16_t *port_id);
>  
>  /**
>  * Get the device name from port id
> +* Ex:

What do you think say "Ex device name:" here and remove "device name" from 
below:
PCIe, :02:00.0
SoC, fslgmac0
vdev, net_pcap0, net_null0, net_tun0 or net_tap0, ...

> +* - PCIe Bus:Domian:Function, :02:00.0
> +* - SoC device name, fslgmac0
> +* - VDEV driver name, net_pcap0, net_null0, net_tun0 or net_tap0

Why VDEV capitalized?

>  *
>  * @param port_id
>  *   Port identifier of the device.
> 

Re: [dpdk-dev] [dpdk-stable] [PATCH v4] vhost: improve dirty pages logging performance

[Ferruh Yigit]

On 5/17/2018 1:01 PM, Maxime Coquelin wrote:
> Hi Tiwei,
> 
> On 05/17/2018 01:44 PM, Maxime Coquelin wrote:
>> This patch caches all dirty pages logging until the used ring index
>> is updated.
>>
>> The goal of this optimization is to fix a performance regression
>> introduced when the vhost library started to use atomic operations
>> to set bits in the shared dirty log map. While the fix was valid
>> as previous implementation wasn't safe against concurrent accesses,
>> contention was induced.
>>
>> With this patch, during migration, we have:
>> 1. Less atomic operations as only a single atomic OR operation
>> per 32 or 64 (depending on CPU) pages.
>> 2. Less atomic operations as during a burst, the same page will
>> be marked dirty only once.
>> 3. Less write memory barriers.
>>
>> Fixes: 897f13a1f726 ("vhost: make page logging atomic")
>>
>> Cc:sta...@dpdk.org
>>
>> Cc: Tiwei Bie
>> Suggested-by: Michael S. Tsirkin
>> Signed-off-by: Maxime Coquelin
> 
> I missed to add your:
> Reviewed-by: Tiwei Bie 

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

Re: [dpdk-dev] [PATCH v4 01/23] lib/librte_eal: import libbsd strlcpy

[Andy Green]

On 05/17/2018 06:36 PM, Bruce Richardson wrote:

On Mon, May 14, 2018 at 01:09:32PM +0800, Andy Green wrote:

Signed-off-by: Andy Green 
---
  lib/librte_eal/common/eal_common_string_fns.c  |   34 
  lib/librte_eal/common/include/rte_string_fns.h |7 +
  2 files changed, 36 insertions(+), 5 deletions(-)



While I'm aware this was suggested by other reviewers, I really don't feel
that it is necessary to actually import the code. If libbsd is present on
the system, we will use it directly. If libbsd is not present, the snprintf
provides an acceptable fallback for strlcpy IMHO. Having the full function
without good justification seems excessive.


Well, as you can probably guess, I don't really mind either way.

This also implies another patch to export rte_strlcpy since it's no 
longer an inline in the headers this way.


I removed these patches and rebuilt dpdk and then lagopus without it 
with the idea of pasting the compile error.  But I can't reproduce the 
original problem... since then I rebased on current master dpdk, got 
updated to gcc 8.1 and added more patches on lagopus.


So just drop this patch if you don't want the bsd lstrcpy.

-Andy


/Bruce

Re: [dpdk-dev] [PATCH v2] net/mlx5: add bluefield device ID

[Shahaf Shuler]

Thursday, May 17, 2018 1:49 PM, Ferruh Yigit:
> Subject: Re: [dpdk-dev] [PATCH v2] net/mlx5: add bluefield device ID
> 
> On 5/15/2018 10:28 AM, Nélio Laranjeiro wrote:
> > On Tue, May 15, 2018 at 09:12:50AM +0300, Shahaf Shuler wrote:
> >> Signed-off-by: Shahaf Shuler 
> >
> > Acked-by: Nelio Laranjeiro 
> 
> Is web patch coming to introduce the device support in
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdp
> dk.org%2Fdoc%2Fnics&data=02%7C01%7Cshahafs%40mellanox.com%7Cb55
> 153d4e8ff4131d05608d5bbe3c08b%7Ca652971c7d2e4d9ba6a4d149256f461b%
> 7C0%7C0%7C636621509233305840&sdata=j3AVrlQByWOrWslubG1MJyBRhuf
> LuBYFjf9C1ExkQsw%3D&reserved=0?

Done
http://dpdk.org/ml/archives/web/2018-May/000725.html

Re: [dpdk-dev] [PATCH] app/testbbdev: fix unchecked return value

[Mokhtar, Amr]

> -Original Message-
> From: Chalupnik, KamilX
> Sent: Wednesday 16 May 2018 14:57
> To: Mokhtar, Amr ; De Lara Guarch, Pablo
> 
> Cc: dev@dpdk.org; Chalupnik, KamilX 
> Subject: [PATCH] app/testbbdev: fix unchecked return value
> 
> Fixing CHECKED_RETURN issue by checking values returned
> by rte_bbdev_dec_op_alloc_bulk and rte_bbdev_enc_op_alloc_bulk
> functions.
> 
> Fixes: f714a18885a6 ("app/testbbdev: add test application for bbdev")
> Coverity issue: 279447, 279456
> 
> Signed-off-by: Kamil Chalupnik 

Acked-by: Amr Mokhtar 

Re: [dpdk-dev] [PATCH] maintainers: handoff ownership of Tap PMD

[Wiles, Keith]

> On May 17, 2018, at 2:44 AM, Pascal Mazon  wrote:
> 
> I have unfortunately no longer time enough for maintaining Tap PMD.
> Keith has kindly volunteered to take over maintainership. He's been at
> the origin of this PMD and knows well how it works.
> 
> Signed-off-by: Pascal Mazon 
> Acked-by: Keith Wiles 
> ---
> MAINTAINERS | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2663f1c037d7..a1b072c0b2d7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -686,7 +686,7 @@ F: doc/guides/nics/pcap_ring.rst
> F: doc/guides/nics/features/pcap.ini
> 
> Tap PMD
> -M: Pascal Mazon 
> +M: Keith Wiles 
> F: drivers/net/tap/
> F: doc/guides/nics/tap.rst
> F: doc/guides/nics/features/tap.ini
> -- 
> 2.16.1.72.g5be1f00a9
> 

Acked by: Keith Wiles

Regards,
Keith

Re: [dpdk-dev] [PATCH v4 01/23] lib/librte_eal: import libbsd strlcpy

[Bruce Richardson]

On Thu, May 17, 2018 at 08:35:21PM +0800, Andy Green wrote:
> 
> 
> On 05/17/2018 06:36 PM, Bruce Richardson wrote:
> > On Mon, May 14, 2018 at 01:09:32PM +0800, Andy Green wrote:
> > > Signed-off-by: Andy Green 
> > > ---
> > >   lib/librte_eal/common/eal_common_string_fns.c  |   34 
> > > 
> > >   lib/librte_eal/common/include/rte_string_fns.h |7 +
> > >   2 files changed, 36 insertions(+), 5 deletions(-)
> > > 
> > 
> > While I'm aware this was suggested by other reviewers, I really don't feel
> > that it is necessary to actually import the code. If libbsd is present on
> > the system, we will use it directly. If libbsd is not present, the snprintf
> > provides an acceptable fallback for strlcpy IMHO. Having the full function
> > without good justification seems excessive.
> 
> Well, as you can probably guess, I don't really mind either way.
> 
> This also implies another patch to export rte_strlcpy since it's no longer
> an inline in the headers this way.
> 
> I removed these patches and rebuilt dpdk and then lagopus without it with
> the idea of pasting the compile error.  But I can't reproduce the original
> problem... since then I rebased on current master dpdk, got updated to gcc
> 8.1 and added more patches on lagopus.
> 
> So just drop this patch if you don't want the bsd lstrcpy.
> 
Yes, let's drop it from the set for now anyway, if it's not solving any
specific error. We can relook at it in 18.08 anyway.

/Bruce

Re: [dpdk-dev] [PATCH v4 05/23] /lib/librte_eal: stage cast from uint64 to long

[Andy Green]

On 05/17/2018 06:47 PM, Bruce Richardson wrote:

On Mon, May 14, 2018 at 01:09:52PM +0800, Andy Green wrote:

/projects/lagopus/src/dpdk/build/include/rte_random.h:
In function 'rte_srand':
/projects/lagopus/src/dpdk/build/include/rte_random.h:34:10:
warning: conversion to 'long int' from 'long unsigned int'
may change the sign of the result [-Wsign-conversion]
   srand48((long unsigned int)seedval);

/projects/lagopus/src/dpdk/build/include/rte_random.h:51:8:
warning: conversion to 'uint64_t' {aka 'long unsigned int'}
from 'long int' may change the sign of the result
[-Wsign-conversion]
   val = lrand48();
 ^~~
/projects/lagopus/src/dpdk/build/include/rte_random.h:53:6:
warning: conversion to 'long unsigned int' from 'long int'
may change the sign of the result [-Wsign-conversion]
   val += lrand48();

Signed-off-by: Andy Green 
---
  lib/librte_eal/common/include/rte_random.h |6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/common/include/rte_random.h 
b/lib/librte_eal/common/include/rte_random.h
index 63bb28088..e30777b83 100644
--- a/lib/librte_eal/common/include/rte_random.h
+++ b/lib/librte_eal/common/include/rte_random.h
@@ -31,7 +31,7 @@ extern "C" {
  static inline void
  rte_srand(uint64_t seedval)
  {
-   srand48((long unsigned int)seedval);
+   srand48((long)(unsigned long)seedval);


Is double-cast necessary here? Can we not just cast straight to long?


You're right, it can just go straight to long.

I'm going through adding the compiler errors into the patches that have 
them missing and doing this kind of small change, and I'll reissue in an 
hour or two (.


-Andy


  }
  
  /**

@@ -48,9 +48,9 @@ static inline uint64_t
  rte_rand(void)
  {
uint64_t val;
-   val = lrand48();
+   val = (uint64_t)lrand48();
val <<= 32;
-   val += lrand48();
+   val += (uint64_t)lrand48();
return val;
  }
  



Apart from the one minor comment above LGTM

Acked-by: Bruce Richardson 

Re: [dpdk-dev] [PATCH v2] net/tap: fix device removal when no queues exist

[Wiles, Keith]

> On May 17, 2018, at 2:47 AM, Ophir Munk  wrote:
> 
> TAP device is created following its first queue creation. Multiple
> queues can be added or removed over time. In Linux terminology those
> are file descriptors which are opened or closed over time. As long as
> the number of opened file descriptors is positive - TAP device will
> appear as a Linux device. In case all queues are released (the
> equivalent of all file descriptors being closed) the TAP device will
> be removed. This can lead to abnormalities in different scenarios
> where the TAP device should exist even if all its queues are released.
> In order to make TAP existence independent of its number of queues -
> an extra file descriptor is opened on TAP creation and is closed on
> TAP closure. It's only purpose is to serve as a keep-alive mechanism
> for the TAP device.
> 
> Fixes: bf7b7f437b49 ("net/tap: create netdevice during probing")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Ophir Munk 
> ---
> v1:
> Initial release
> v2:
> Reword commit message (a fixing patch)
> 
> drivers/net/tap/rte_eth_tap.c | 31 ---
> drivers/net/tap/rte_eth_tap.h |  1 +
> 2 files changed, 25 insertions(+), 7 deletions(-)

I did not see where ka_fd is set to -1 at startup, just in case we fail before 
the first open attempt and possible hit the close code in the remove routine. I 
did not look at the complete driver, but I think it maybe reasonable to add 
that initial variable setup.

> 
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index c006d07..6901edc 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -929,6 +929,15 @@ tap_dev_close(struct rte_eth_dev *dev)
>   ioctl(internals->ioctl_sock, SIOCSIFFLAGS,
>   &internals->remote_initial_flags);
>   }
> +
> + if (internals->ka_fd != -1) {
> + close(internals->ka_fd);
> + internals->ka_fd = -1;
> + }
> + /*
> +  * Since TUN device has no more opened file descriptors
> +  * it will be removed from kernel
> +  */
> }
> 
> static void
> @@ -1561,13 +1570,18 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, char 
> *tap_name,
>   rte_memcpy(&pmd->eth_addr, mac_addr, sizeof(*mac_addr));
>   }
> 
> - /* Immediately create the netdevice (this will create the 1st queue). */
> - /* rx queue */
> - if (tap_setup_queue(dev, pmd, 0, 1) == -1)
> - goto error_exit;
> - /* tx queue */
> - if (tap_setup_queue(dev, pmd, 0, 0) == -1)
> + /*
> +  * Allocate a TUN device keep-alive file descriptor that will only be
> +  * closed when the TUN device itself is closed or removed.
> +  * This keep-alive file descriptor will guarantee that the TUN device
> +  * exists even when all of its queues are closed
> +  */
> + pmd->ka_fd = tun_alloc(pmd);
> + if (pmd->ka_fd < 0) {
> + pmd->ka_fd = -1;
> + TAP_LOG(ERR, "Unable to create %s interface", tuntap_name);
>   goto error_exit;
> + }
> 
>   ifr.ifr_mtu = dev->data->mtu;
>   if (tap_ioctl(pmd, SIOCSIFMTU, &ifr, 1, LOCAL_AND_REMOTE) < 0)
> @@ -1961,9 +1975,12 @@ rte_pmd_tap_remove(struct rte_vdev_device *dev)
> 
>   close(internals->ioctl_sock);
>   rte_free(eth_dev->data->dev_private);
> -
>   rte_eth_dev_release_port(eth_dev);
> 
> + if (internals->ka_fd != -1) {
> + close(internals->ka_fd);
> + internals->ka_fd = -1;
> + }
>   return 0;
> }
> 
> diff --git a/drivers/net/tap/rte_eth_tap.h b/drivers/net/tap/rte_eth_tap.h
> index babe42d..575dce4 100644
> --- a/drivers/net/tap/rte_eth_tap.h
> +++ b/drivers/net/tap/rte_eth_tap.h
> @@ -81,6 +81,7 @@ struct pmd_internals {
>   struct rx_queue rxq[RTE_PMD_TAP_MAX_QUEUES]; /* List of RX queues */
>   struct tx_queue txq[RTE_PMD_TAP_MAX_QUEUES]; /* List of TX queues */
>   struct rte_intr_handle intr_handle;  /* LSC interrupt handle. */
> + int ka_fd;/* keep-alive file descriptor */
> };
> 
> /* tap_intr.c */
> -- 
> 2.7.4
> 

Regards,
Keith

Re: [dpdk-dev] [PATCH v4 01/23] lib/librte_eal: import libbsd strlcpy

[Andy Green]

On 05/17/2018 08:56 PM, Bruce Richardson wrote:

On Thu, May 17, 2018 at 08:35:21PM +0800, Andy Green wrote:



On 05/17/2018 06:36 PM, Bruce Richardson wrote:

On Mon, May 14, 2018 at 01:09:32PM +0800, Andy Green wrote:

Signed-off-by: Andy Green 
---
   lib/librte_eal/common/eal_common_string_fns.c  |   34 

   lib/librte_eal/common/include/rte_string_fns.h |7 +
   2 files changed, 36 insertions(+), 5 deletions(-)



While I'm aware this was suggested by other reviewers, I really don't feel
that it is necessary to actually import the code. If libbsd is present on
the system, we will use it directly. If libbsd is not present, the snprintf
provides an acceptable fallback for strlcpy IMHO. Having the full function
without good justification seems excessive.


Well, as you can probably guess, I don't really mind either way.

This also implies another patch to export rte_strlcpy since it's no longer
an inline in the headers this way.

I removed these patches and rebuilt dpdk and then lagopus without it with
the idea of pasting the compile error.  But I can't reproduce the original
problem... since then I rebased on current master dpdk, got updated to gcc
8.1 and added more patches on lagopus.

So just drop this patch if you don't want the bsd lstrcpy.


Yes, let's drop it from the set for now anyway, if it's not solving any
specific error. We can relook at it in 18.08 anyway.


Sorry to immediately contradict myself but I forgot a step in the rather 
complicated flow to force the lagopus dpdk submodule HEAD... by default 
making lagopus forces the submodule commit to something else.  It does 
need a much smaller one-liner to avoid


In file included from ./dpdk/dpdk.c:88:
/projects/lagopus/src/dpdk/build/include/rte_string_fns.h: In
function 'rte_strlcpy':
/projects/lagopus/src/dpdk/build/include/rte_string_fns.h:58:9:
warning: conversion to 'size_t' {aka 'long unsigned int'} from
'int' may change the sign of the result [-Wsign-conversion]
  return snprintf(dst, size, "%s", src);
 ^~

It just needs a cast to make the return type of the snprintf compatible 
with the expected return type of strlcpy().


I'll include this in the next send in an hour or two.

-Andy


/Bruce

Re: [dpdk-dev] [PATCH v4 12/23] rte_mbuf.h: explicit cast for size type to uint32

[Andy Green]

On 05/17/2018 06:53 PM, Bruce Richardson wrote:

On Mon, May 14, 2018 at 01:10:27PM +0800, Andy Green wrote:

Signed-off-by: Andy Green 
---
  lib/librte_mbuf/rte_mbuf.h |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 0580ec8a0..169f3d3b0 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -1577,7 +1577,7 @@ static inline void rte_pktmbuf_detach(struct rte_mbuf *m)
__rte_pktmbuf_free_direct(m);
  
  	priv_size = rte_pktmbuf_priv_size(mp);

-   mbuf_size = sizeof(struct rte_mbuf) + priv_size;
+   mbuf_size = (uint32_t)sizeof(struct rte_mbuf) + priv_size;
buf_len = rte_pktmbuf_data_room_size(mp);
  
  	m->priv_size = priv_size;



It would be good to include the error message in the commit log.
Also, for safety, is it better to have extra braces to cast the whole
expression, not just the sizeof, i.e.

mbuf_size = (uint32_t)(sizeof(...) + priv_size);


OK I went back and captured the original error and added it back in, and 
adapted the scope of the cast as you suggested.


-Andy


/Bruce

Re: [dpdk-dev] [PATCH] doc: advise to specify LTS branch when backporting patches

[Thomas Monjalon]

16/05/2018 19:03, Luca Boccassi:
> On Wed, 2018-05-16 at 16:19 +0100, Kevin Traynor wrote:
> > On 05/16/2018 03:31 PM, luca.bocca...@gmail.com wrote:
> > > From: Luca Boccassi 
> > > 
> > > We have many stable branches being maintaned at the same time, and
> > 
> > typo
> > 
> > > sometimes it's not clear which branch a patch is being backported
> > > for.
> > > Note in the guidelines that it should be specified via the cover
> > > letter,
> > > annotation or using --subject-prefix.
> > > Also note to send only to sta...@dpdk.org, not dev@dpdk.org.
> > > 
> > > Signed-off-by: Luca Boccassi 
> > > ---
> > >  doc/guides/contributing/patches.rst | 8 
> > >  1 file changed, 8 insertions(+)
> > > 
> > > diff --git a/doc/guides/contributing/patches.rst
> > > b/doc/guides/contributing/patches.rst
> > > index 2287835f9..1dc623a23 100644
> > > --- a/doc/guides/contributing/patches.rst
> > > +++ b/doc/guides/contributing/patches.rst
> > > @@ -450,6 +450,14 @@ Experienced committers may send patches
> > > directly with ``git send-email`` without
> > >  The options ``--annotate`` and ``confirm = always`` are
> > > recommended for checking patches before sending.
> > >  
> > >  
> > > +Backporting patches for Stable Releases
> > > +~~~
> > > +
> > > +Sometimes a maintainer or contributor wishes, or can be asked, to
> > > send a patch for a stable release rather than mainline. In this
> > > case the patch(es) should be sent to ``sta...@dpdk.org``, not to ``
> > > dev@dpdk.org``.
> > > +
> > > +Given that there are multiple stable releases being maintained at
> > > the same time, please specify exactly which branch the patch is for
> > > in the cover letter, in the annotation or using ``git send-email --
> > > subject-prefix='16.11' ...``
> > > +
> > 
> > s/branch/branch(es)/
> > 
> > How about [16.11] instead of '16.11' ?
> 
> Using --subject-prefix git will already use "[PATCH 16.11]

I think it should be --subject-prefix='PATCH 16.11'

Please wrap the lines after some punctuation signs to avoid really long lines
in the rst file.

Re: [dpdk-dev] [PATCH] net/ixgbe: fix dcb configuration issue

[Zhang, Qi Z]

Hi Qiming

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org]
> Sent: Thursday, May 17, 2018 9:23 PM
> To: dev@dpdk.org
> Cc: Wu, Jingjing ; Lu, Wenzhuo
> ; Yang, Qiming ;
> sta...@dpdk.org
> Subject: [dpdk-dev] [PATCH] net/ixgbe: fix dcb configuration issue
> 
> This patch removed unnecessary check in ixgbe_check_mq_mode, fixed
> historical issue.
> 
> Fixes: 27b609cbd1c6 ("ethdev: move the multi-queue mode check to specific
> drivers")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Qiming Yang 
> ---
>  drivers/net/ixgbe/ixgbe_ethdev.c | 37 -
>  1 file changed, 37 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index f5006bc..9e3ae95 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -2300,43 +2300,6 @@ ixgbe_check_mq_mode(struct rte_eth_dev *dev)
>   }
>   }
> 
> - /* For DCB mode check our configuration before we go further */
> - if (dev_conf->rxmode.mq_mode == ETH_MQ_RX_DCB) {
> - const struct rte_eth_dcb_rx_conf *conf;
> -
> - if (nb_rx_q != IXGBE_DCB_NB_QUEUES) {
> - PMD_INIT_LOG(ERR, "DCB selected, nb_rx_q != 
> %d.",
> -  IXGBE_DCB_NB_QUEUES);
> - return -EINVAL;
> - }
> - conf = &dev_conf->rx_adv_conf.dcb_rx_conf;
> - if (!(conf->nb_tcs == ETH_4_TCS ||
> -conf->nb_tcs == ETH_8_TCS)) {

I guess, we should not remove all check, for nb_tcs, it still valid, right?

> - PMD_INIT_LOG(ERR, "DCB selected, nb_tcs != %d"
> - " and nb_tcs != %d.",
> - ETH_4_TCS, ETH_8_TCS);
> - return -EINVAL;
> - }
> - }
> -
> - if (dev_conf->txmode.mq_mode == ETH_MQ_TX_DCB) {
> - const struct rte_eth_dcb_tx_conf *conf;
> -
> - if (nb_tx_q != IXGBE_DCB_NB_QUEUES) {
> - PMD_INIT_LOG(ERR, "DCB, nb_tx_q != %d.",
> -  IXGBE_DCB_NB_QUEUES);
> - return -EINVAL;
> - }
> - conf = &dev_conf->tx_adv_conf.dcb_tx_conf;
> - if (!(conf->nb_tcs == ETH_4_TCS ||
> -conf->nb_tcs == ETH_8_TCS)) {
Same to rx.
> - PMD_INIT_LOG(ERR, "DCB selected, nb_tcs != %d"
> - " and nb_tcs != %d.",
> - ETH_4_TCS, ETH_8_TCS);
> - return -EINVAL;
> - }
> - }
> -
>   /*
>* When DCB/VT is off, maximum number of queues changes,
>* except for 82598EB, which remains constant.
> --
> 2.9.5

Re: [dpdk-dev] [PATCH] igb_uio: fail and log if kernel lock down is enabled

[Ferruh Yigit]

On 5/16/2018 12:47 PM, Neil Horman wrote:
> On Tue, May 15, 2018 at 05:56:12PM +0100, Ferruh Yigit wrote:
>> When EFI secure boot is enabled, it is possible to lock down kernel and
>> prevent accessing device BARs and this makes igb_uio unusable.
>>
>> Lock down patches are not part of the vanilla kernel but they are
>> applied and used by some distros already [1].
>>
>> It is not possible to fix this issue, but intention of this patch is to
>> detect and log if kernel lock down enabled and don't insert the module
>> for that case.
>>
>> The challenge is since this feature enabled by distros, they have
>> different config options and APIs for it. This patch is done based on
>> Fedora and Ubuntu kernel source, may needs to add more distro specific
>> support.
>>
>> [1]
>> kernel.ubuntu.com/git/ubuntu/ubuntu-artful.git/commit/?id=99f9ef18d5b6
>> And a few more patches to
>>
> What exactly is the error you get when you load the igb_uio module?  I ask
> because, looking at least at the Fedora patches, the BAR registers themselves
> aren't made unwriteable, its only userspace access through very specific
> channels that are gated on (things like /proc/bus/pci/...).  From what I can 
> see
> (again, not having looked at other implementations), kernel modules that load
> successfully should be able to modify bar registers, and otherwise function
> normally (as to weather they are permitted to load is another question).

This patch is based on understanding on the effect of the lockdown patches, that
it will disable hardware access from userspace.
I don't have an environment to test this and indeed I am not very clear about
effects of the lockdown set.

> 
> The reason I ask this is twofold:
> 
> 1) if a specific access is failing, that seems like it could be the trigger to
> use, rather than explicitly checking if the kernel is locked down.  I don't 
> see
> one expressly called, but if you're calling pci_write_config_* somewhere, and
> getting an EPERM error, thats a reason to fail the loading of igb_uio, based 
> on
> the fact that you don't have permission to write to the appropriate hardware.
> 
> 2) Its more than just the igb_uio module that will fail.  Any attempt to pass 
> a
> VF into a guest using user space tools (including the vfio scripts that dpdk
> includes), should fail.  As such, it might be better to have some component in
> user space test one of the aforementioned restricted paths for writeability.
> Such an approach would be more generic, and eliminate the need to assemble a 
> set
> of tests to see if the kernel is locked down.  A more generic error message
> could then be logged and the dpdk could exit gracefully, weather or not 
> igb_uio
> was loaded.

With the existing patches, expectation is vfio will work but it will only effect
igb_uio.

> 
> Its probably also important to note here that, this lockdown patch, from my
> digging, has been carried in Fedora since December of 2016, and its still not
> made it upstream.  Thats not to say that it will never do so, but it suggests
> that, given the 2 years of out of tree updates its received, there its use is
> both very specific and limted to users who understand its implications.  This
> probably isn't something to make significant or hard-to-maintain changes to 
> the
> dpdk (or any other software) over.

Have same expectation that use will be specific and limited, that is why planed
to change only igb_uio to detect the case and return with a log, instead of
updating anything in the dpdk.

in igb_uio the plan was just adding simple check, patches being not upstreamed
added more complexity, but not still I believe it is not significant or
hard-to-maintain change.

> 
> Neil
> 
>> Signed-off-by: Ferruh Yigit 
>> ---
>> Cc: Christian Ehrhardt 
>> Cc: Luca Boccassi 
>> Cc: Maxime Coquelin 
>> Cc: Neil Horman 
>> Cc: Stephen Hemminger 
>> ---
>>  kernel/linux/igb_uio/compat.h  | 24 
>>  kernel/linux/igb_uio/igb_uio.c |  5 +
>>  2 files changed, 25 insertions(+), 4 deletions(-)
>>
>> diff --git a/kernel/linux/igb_uio/compat.h b/kernel/linux/igb_uio/compat.h
>> index d9f4d29fc..774c980c2 100644
>> --- a/kernel/linux/igb_uio/compat.h
>> +++ b/kernel/linux/igb_uio/compat.h
>> @@ -125,10 +125,6 @@ static bool pci_check_and_mask_intx(struct pci_dev 
>> *pdev)
>>  #define HAVE_PCI_IS_BRIDGE_API 1
>>  #endif
>>  
>> -#if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 8, 0)
>> -#define HAVE_ALLOC_IRQ_VECTORS 1
>> -#endif
>> -
>>  #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 3, 0)
>>  #define HAVE_MSI_LIST_IN_GENERIC_DEVICE 1
>>  #endif
>> @@ -136,3 +132,23 @@ static bool pci_check_and_mask_intx(struct pci_dev 
>> *pdev)
>>  #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 5, 0)
>>  #define HAVE_PCI_MSI_MASK_IRQ 1
>>  #endif
>> +
>> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 8, 0)
>> +#define HAVE_ALLOC_IRQ_VECTORS 1
>> +#endif
>> +
>> +static inline bool igbuio_kernel_is_locked_down(void)
>> +{
>> +#ifdef CONFIG_LOCK_DOWN_KERNEL
>> +#if

Re: [dpdk-dev] [PATCH v3] igb_uio: fail and log if kernel lock down is enabled

[Ferruh Yigit]

On 5/17/2018 12:34 PM, Neil Horman wrote:
> On Wed, May 16, 2018 at 03:42:20PM +0100, Ferruh Yigit wrote:
>> When EFI secure boot is enabled, it is possible to lock down kernel and
>> prevent accessing device BARs and this makes igb_uio unusable.
>>
>> Lock down patches are not part of the vanilla kernel but they are
>> applied and used by some distros already [1].
>>
>> It is not possible to fix this issue, but intention of this patch is to
>> detect and log if kernel lock down enabled and don't insert the module
>> for that case.
>>
>> The challenge is since this feature enabled by distros, they have
>> different config options and APIs for it. This patch is done based on
>> Fedora and Ubuntu kernel source, may needs to add more distro specific
>> support.
>>
> I still need to ask, what exactly is the error you're seeing with inserting 
> the
> uio module?  The lockdown patch set restricts BAR address changes, but via 
> paths
> acessible from user space, igbuio should still insert and initalize just fine
> (or so it would seem to me).  Why not fix this by detecting the problem during
> the user space library initalization, where you can do so via a standard 
> method
> that works accross distributions?

I have seen you comment on other thread, this v3 was just to fix a silly
mistake, lets continue discussion in other thread.

Re: [dpdk-dev] [PATCH] net/mlx5: fix default RSS level

[Nélio Laranjeiro]

On Tue, May 15, 2018 at 04:23:24PM +0300, Shahaf Shuler wrote:
> Using inner RSS by default for GRE leads to memory corruption as the
> extra flow items added for the inner RSS are not counted in the flow
> attributes buffer size.
> 
> Fixing by enforcing the default RSS level to be outer. This much
> simplify the flow engine and more robust.
> Future optimization for out of the box RSS can be done on subsequent
> commits.
> 
> Fixes: d4a405186b73 ("net/mlx5: support tunnel RSS level")
> Cc: xuemi...@mellanox.com
> Cc: ma...@mellanox.com
> 
> Signed-off-by: Shahaf Shuler 

Acked-by: Nelio Laranjeiro 

> ---
>  drivers/net/mlx5/mlx5_flow.c | 22 ++
>  1 file changed, 2 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index 0c0d6f99ad..994be05be6 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -805,7 +805,7 @@ mlx5_flow_convert_actions(struct rte_eth_dev *dev,
>   }
>   parser->rss_conf = (struct rte_flow_action_rss){
>   .func = RTE_ETH_HASH_FUNCTION_DEFAULT,
> - .level = rss->level,
> + .level = rss->level ? rss->level : 1,
>   .types = rss->types,
>   .key_len = rss_key_len,
>   .queue_num = rss->queue_num,
> @@ -1166,9 +1166,6 @@ mlx5_flow_convert_rss(struct mlx5_flow_parse *parser)
>   int outer = parser->tunnel && parser->rss_conf.level < 2;
>   uint64_t rss = parser->rss_conf.types;
>  
> - /* Default to outer RSS. */
> - if (!parser->rss_conf.level)
> - parser->rss_conf.level = 1;
>   layer = outer ? parser->out_layer : parser->layer;
>   if (layer == HASH_RXQ_TUNNEL)
>   layer = HASH_RXQ_ETH;
> @@ -1801,9 +1798,6 @@ mlx5_flow_create_vxlan(const struct rte_flow_item *item,
>   parser->tunnel = ptype_ext[PTYPE_IDX(RTE_PTYPE_TUNNEL_VXLAN)];
>   parser->out_layer = parser->layer;
>   parser->layer = HASH_RXQ_TUNNEL;
> - /* Default VXLAN to outer RSS. */
> - if (!parser->rss_conf.level)
> - parser->rss_conf.level = 1;
>   if (spec) {
>   if (!mask)
>   mask = default_mask;
> @@ -1876,9 +1870,6 @@ mlx5_flow_create_vxlan_gpe(const struct rte_flow_item 
> *item,
>   parser->tunnel = ptype_ext[PTYPE_IDX(RTE_PTYPE_TUNNEL_VXLAN_GPE)];
>   parser->out_layer = parser->layer;
>   parser->layer = HASH_RXQ_TUNNEL;
> - /* Default VXLAN-GPE to outer RSS. */
> - if (!parser->rss_conf.level)
> - parser->rss_conf.level = 1;
>   if (spec) {
>   if (!mask)
>   mask = default_mask;
> @@ -1956,9 +1947,6 @@ mlx5_flow_create_gre(const struct rte_flow_item *item,
>   parser->tunnel = ptype_ext[PTYPE_IDX(RTE_PTYPE_TUNNEL_GRE)];
>   parser->out_layer = parser->layer;
>   parser->layer = HASH_RXQ_TUNNEL;
> - /* Default GRE to inner RSS. */
> - if (!parser->rss_conf.level)
> - parser->rss_conf.level = 2;
>  #ifdef HAVE_IBV_DEVICE_MPLS_SUPPORT
>   if (spec) {
>   if (!mask)
> @@ -2052,12 +2040,6 @@ mlx5_flow_create_mpls(const struct rte_flow_item *item,
>   /* parser->out_layer stays as in GRE out_layer. */
>   }
>   parser->layer = HASH_RXQ_TUNNEL;
> - /*
> -  * For MPLS-in-GRE, RSS level should have been set.
> -  * For MPLS-in-UDP, use outer RSS.
> -  */
> - if (!parser->rss_conf.level)
> - parser->rss_conf.level = 1;
>   if (spec) {
>   if (!mask)
>   mask = default_mask;
> @@ -2501,7 +2483,7 @@ mlx5_flow_list_create(struct rte_eth_dev *dev,
>   flow->tunnel = parser.tunnel;
>   flow->rss_conf = (struct rte_flow_action_rss){
>   .func = RTE_ETH_HASH_FUNCTION_DEFAULT,
> - .level = 0,
> + .level = parser.rss_conf.level,
>   .types = parser.rss_conf.types,
>   .key_len = parser.rss_conf.key_len,
>   .queue_num = parser.rss_conf.queue_num,
> -- 
> 2.12.0
> 

-- 
Nélio Laranjeiro
6WIND

Re: [dpdk-dev] [PATCH v4 13/23] rte_mbuf.h: explicit casts to uint16 to avoid warnings

[Andy Green]

On 05/17/2018 06:58 PM, Bruce Richardson wrote:

On Mon, May 14, 2018 at 01:10:32PM +0800, Andy Green wrote:

Signed-off-by: Andy Green 
---
  lib/librte_mbuf/rte_mbuf.h |   15 ---
  1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 169f3d3b0..3cd76abbc 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -1580,7 +1580,7 @@ static inline void rte_pktmbuf_detach(struct rte_mbuf *m)
mbuf_size = (uint32_t)sizeof(struct rte_mbuf) + priv_size;
buf_len = rte_pktmbuf_data_room_size(mp);
  
-	m->priv_size = priv_size;

+   m->priv_size = (uint16_t)priv_size;
m->buf_addr = (char *)m + mbuf_size;
m->buf_iova = rte_mempool_virt2iova(m) + mbuf_size;
m->buf_len = (uint16_t)buf_len;


I think a better fix for this is to change priv_size to be uint16_t. There
is no reason for it to be a 32-bit value.


OK.


@@ -1905,7 +1905,7 @@ static inline char *rte_pktmbuf_prepend(struct rte_mbuf 
*m,
if (unlikely(len > rte_pktmbuf_headroom(m)))
return NULL;
  
-	m->data_off -= len;

+   m->data_off = (uint16_t)(m->data_off - len);
m->data_len = (uint16_t)(m->data_len + len);
m->pkt_len  = (m->pkt_len + len);
  


What is the error message here? The cast isn't wrong, it just seems like we
shouldn't need it. I suppose it's implicit promotion being done in the
subtraction operation.


/projects/lagopus/src/dpdk/build/include/rte_mbuf.h:
In function 'rte_pktmbuf_prepend':
/projects/lagopus/src/dpdk/build/include/rte_mbuf.h:
1908:17: warning: conversion from 'int' to 'uint16_t'
{aka 'short unsigned int'} may change value [-Wconversion]
  m->data_off -= len;
 ^~~

I have added all the original warnings to the patch.

-Andy


@@ -1966,7 +1966,7 @@ static inline char *rte_pktmbuf_adj(struct rte_mbuf *m, 
uint16_t len)
return NULL;
  
  	m->data_len = (uint16_t)(m->data_len - len);

-   m->data_off += len;
+   m->data_off = (uint16_t)(m->data_off + len);
m->pkt_len  = (m->pkt_len - len);
return (char *)m->buf_addr + m->data_off;
  }
@@ -2079,7 +2079,7 @@ static inline int rte_pktmbuf_chain(struct rte_mbuf 
*head, struct rte_mbuf *tail
cur_tail->next = tail;
  
  	/* accumulate number of segments and total length. */

-   head->nb_segs += tail->nb_segs;
+   head->nb_segs = (uint16_t)(head->nb_segs + tail->nb_segs);
head->pkt_len += tail->pkt_len;
  
  	/* pkt_len is only set in the head */

@@ -2109,7 +2109,8 @@ rte_validate_tx_offload(const struct rte_mbuf *m)
return 0;
  
  	if (ol_flags & PKT_TX_OUTER_IP_CKSUM)

-   inner_l3_offset += m->outer_l2_len + m->outer_l3_len;
+   inner_l3_offset += (unsigned int)(m->outer_l2_len +
+ m->outer_l3_len);
  
  	/* Headers are fragmented */

if (rte_pktmbuf_data_len(m) < inner_l3_offset + m->l3_len + m->l4_len)
@@ -2154,7 +2155,7 @@ rte_validate_tx_offload(const struct rte_mbuf *m)
  static inline int
  rte_pktmbuf_linearize(struct rte_mbuf *mbuf)
  {
-   int seg_len, copy_len;
+   size_t seg_len, copy_len;
struct rte_mbuf *m;
struct rte_mbuf *m_next;
char *buffer;
@@ -2169,7 +2170,7 @@ rte_pktmbuf_linearize(struct rte_mbuf *mbuf)
return -1;
  
  	buffer = rte_pktmbuf_mtod_offset(mbuf, char *, mbuf->data_len);

-   mbuf->data_len = (uint16_t)(mbuf->pkt_len);
+   mbuf->data_len = (uint16_t)mbuf->pkt_len;
  
  	/* Append data from next segments to the first one */

m = mbuf->next;

Re: [dpdk-dev] [PATCH] net/ixgbe: fix too many interrupts

[Zhang, Qi Z]

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Wenzhuo Lu
> Sent: Thursday, May 17, 2018 1:34 PM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo ; Luo, Michael
> 
> Subject: [dpdk-dev] [PATCH] net/ixgbe: fix too many interrupts
> 
> To support kernel VF, PBA bit is always set. But it may cause the too many
> interrupts issue on specific Linux kernel, like 4.4.0-116.
> PF host should set the atuo clean, mask and throttling as we always set the
> register for kernel VF.
> 
> Fixes: 6b75183ac4d0 ("net/ixgbe: fix wrong PBA setting")
> 
> Signed-off-by: Michael Luo 
> Signed-off-by: Wenzhuo Lu 

Reviewed-by: Qi Zhang 

> ---
>  drivers/net/ixgbe/ixgbe_ethdev.c | 49
> +++-
>  1 file changed, 28 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index f5006bc..7219f02 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -5800,8 +5800,12 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev
> *dev, bool on)
> 
>   /* won't configure msix register if no mapping is done
>* between intr vector and event fd
> +  * but if misx has been enabled already, need to configure
> +  * auto clean, auto mask and throttling.
>*/
> - if (!rte_intr_dp_is_en(intr_handle))
> + gpie = IXGBE_READ_REG(hw, IXGBE_GPIE);
> + if (!rte_intr_dp_is_en(intr_handle) &&
> + !(gpie & (IXGBE_GPIE_MSIX_MODE | IXGBE_GPIE_PBA_SUPPORT)))
>   return;
> 
>   if (rte_intr_allow_others(intr_handle))
> @@ -5825,27 +5829,30 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev
> *dev, bool on)
>   /* Populate the IVAR table and set the ITR values to the
>* corresponding register.
>*/
> - for (queue_id = 0; queue_id < dev->data->nb_rx_queues;
> -  queue_id++) {
> - /* by default, 1:1 mapping */
> - ixgbe_set_ivar_map(hw, 0, queue_id, vec);
> - intr_handle->intr_vec[queue_id] = vec;
> - if (vec < base + intr_handle->nb_efd - 1)
> - vec++;
> - }
> + if (rte_intr_dp_is_en(intr_handle)) {
> + for (queue_id = 0; queue_id < dev->data->nb_rx_queues;
> + queue_id++) {
> + /* by default, 1:1 mapping */
> + ixgbe_set_ivar_map(hw, 0, queue_id, vec);
> + intr_handle->intr_vec[queue_id] = vec;
> + if (vec < base + intr_handle->nb_efd - 1)
> + vec++;
> + }
> 
> - switch (hw->mac.type) {
> - case ixgbe_mac_82598EB:
> - ixgbe_set_ivar_map(hw, -1, IXGBE_IVAR_OTHER_CAUSES_INDEX,
> -IXGBE_MISC_VEC_ID);
> - break;
> - case ixgbe_mac_82599EB:
> - case ixgbe_mac_X540:
> - case ixgbe_mac_X550:
> - ixgbe_set_ivar_map(hw, -1, 1, IXGBE_MISC_VEC_ID);
> - break;
> - default:
> - break;
> + switch (hw->mac.type) {
> + case ixgbe_mac_82598EB:
> + ixgbe_set_ivar_map(hw, -1,
> +IXGBE_IVAR_OTHER_CAUSES_INDEX,
> +IXGBE_MISC_VEC_ID);
> + break;
> + case ixgbe_mac_82599EB:
> + case ixgbe_mac_X540:
> + case ixgbe_mac_X550:
> + ixgbe_set_ivar_map(hw, -1, 1, IXGBE_MISC_VEC_ID);
> + break;
> + default:
> + break;
> + }
>   }
>   IXGBE_WRITE_REG(hw, IXGBE_EITR(IXGBE_MISC_VEC_ID),
> 
>   IXGBE_EITR_INTERVAL_US(IXGBE_QUEUE_ITR_INTERVAL_DEFAULT)
> --
> 1.9.3

[dpdk-dev] [PATCH v5 02/21] rte_string_fns.h: fix gcc8.1 sign conv warning in lstrcpy

[Andy Green]

In file included from ./dpdk/dpdk.c:88:
/projects/lagopus/src/dpdk/build/include/rte_string_fns.h: In
function 'rte_strlcpy':
/projects/lagopus/src/dpdk/build/include/rte_string_fns.h:58:9:
warning: conversion to 'size_t' {aka 'long unsigned int'} from
'int' may change the sign of the result [-Wsign-conversion]
  return snprintf(dst, size, "%s", src);
 ^~
---
 lib/librte_eal/common/include/rte_string_fns.h |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/include/rte_string_fns.h 
b/lib/librte_eal/common/include/rte_string_fns.h
index fcbb42e00..97597a148 100644
--- a/lib/librte_eal/common/include/rte_string_fns.h
+++ b/lib/librte_eal/common/include/rte_string_fns.h
@@ -55,7 +55,7 @@ rte_strsplit(char *string, int stringlen,
 static inline size_t
 rte_strlcpy(char *dst, const char *src, size_t size)
 {
-   return snprintf(dst, size, "%s", src);
+   return (size_t)snprintf(dst, size, "%s", src);
 }
 
 /* pull in a strlcpy function */

[dpdk-dev] [PATCH v5 00/21] Fixes for GCC8 against lagopus

[Andy Green]

The following series fixes build problems in dpdk master
headers, found when using it as the dpdk subproject in
lagopus.  These errors are coming when you try to use
the dpdk headers, not when you build dpdk itself.

v5 applies list comment from today.  gcc8.1 errors
corresponding to the problem provoking the patch
have been added in where relevant (almost all of them).

v4 has absorbed more list comment and added 6 x patches
at the end, from getting further into lagopus build with
master DPDK.  There are still many warnings and errors
in lagopus I am working through, but so far it looks like
the solutions for those now belong in lagopus, not DPDK
after this series.

(Applies to v3)
Reviewed-by: Stephen Hemminger 

---

Andy Green (21):
  lib/librte_ethdev: change eth-dev-ops API to return int
  rte_string_fns.h: fix gcc8.1 sign conv warning in lstrcpy
  lib/librte_eal: explicit tmp cast
  /lib/librte_eal: stage cast from uint64 to long
  rte_ring_generic.h: stack declarations before code
  rte_ring.h: remove signed type flipflopping
  rte_mbuf.h: avoid warnings from inadvertant promotion
  rte_mbuf.h: explicit casts for int16 to uint16
  rte_mbuf.h: make sure RTE-MIN compares same types
  rte_mbuf.h: explicit cast restricting ptrdiff to uint16
  rte_ether.h: explicit cast avoiding truncation warning
  rte_rwlock.h: gcc8 sign conversion warnings
  rte_ip.h: cast input to bswap16 to be uint16
  rte_ip.h: cast around promotion to int
  rte_ip.h: cast type decided by sizeof to uint32
  rte_ip.h: cast return checksum size to uint16
  rte_ip.h: cast away gcc8 warning on rte_ipv6_phdr_cksum
  rte_mbuf.h: explicit cast for size type to uint32
  rte_mbuf.h: explicit casts to uint16 to avoid warnings
  rte_ethdev.h: align sign and scope of temp var
  rte_byteorder.h: explicit cast for return promotion


 drivers/net/ark/ark_ethdev_rx.c|4 +-
 drivers/net/ark/ark_ethdev_rx.h|3 +-
 drivers/net/avf/avf_rxtx.c |4 +-
 drivers/net/avf/avf_rxtx.h |2 +
 drivers/net/bnxt/bnxt_ethdev.c |5 ++-
 drivers/net/dpaa/dpaa_ethdev.c |4 +-
 drivers/net/dpaa2/dpaa2_ethdev.c   |6 ++-
 drivers/net/e1000/e1000_ethdev.h   |6 +--
 drivers/net/e1000/em_rxtx.c|4 +-
 drivers/net/e1000/igb_rxtx.c   |4 +-
 drivers/net/enic/enic_ethdev.c |9 ++---
 drivers/net/i40e/i40e_rxtx.c   |4 +-
 drivers/net/i40e/i40e_rxtx.h   |3 +-
 drivers/net/ixgbe/ixgbe_ethdev.h   |3 +-
 drivers/net/ixgbe/ixgbe_rxtx.c |4 +-
 drivers/net/nfp/nfp_net.c  |9 ++---
 drivers/net/sfc/sfc_ethdev.c   |4 +-
 drivers/net/thunderx/nicvf_ethdev.c|2 +
 drivers/net/thunderx/nicvf_rxtx.c  |4 +-
 drivers/net/thunderx/nicvf_rxtx.h  |2 +
 drivers/net/vhost/rte_eth_vhost.c  |4 +-
 examples/l3fwd-power/main.c|2 +
 .../common/include/arch/x86/rte_memcpy.h   |8 ++--
 .../common/include/generic/rte_byteorder.h |6 ++-
 lib/librte_eal/common/include/generic/rte_rwlock.h |4 +-
 lib/librte_eal/common/include/rte_random.h |6 ++-
 lib/librte_eal/common/include/rte_string_fns.h |2 +
 lib/librte_ethdev/rte_ethdev.h |   25 --
 lib/librte_ethdev/rte_ethdev_core.h|4 +-
 lib/librte_mbuf/rte_mbuf.h |   37 +++-
 lib/librte_net/rte_ether.h |2 +
 lib/librte_net/rte_ip.h|   14 
 lib/librte_ring/rte_ring.h |4 +-
 lib/librte_ring/rte_ring_c11_mem.h |2 +
 lib/librte_ring/rte_ring_generic.h |   10 ++---
 35 files changed, 108 insertions(+), 108 deletions(-)

--
Signature

[dpdk-dev] [PATCH v5 01/21] lib/librte_ethdev: change eth-dev-ops API to return int

[Andy Green]

Signed-off-by: Andy Green 
---
 drivers/net/ark/ark_ethdev_rx.c |4 ++--
 drivers/net/ark/ark_ethdev_rx.h |3 +--
 drivers/net/avf/avf_rxtx.c  |4 ++--
 drivers/net/avf/avf_rxtx.h  |2 +-
 drivers/net/bnxt/bnxt_ethdev.c  |5 +++--
 drivers/net/dpaa/dpaa_ethdev.c  |4 ++--
 drivers/net/dpaa2/dpaa2_ethdev.c|6 +++---
 drivers/net/e1000/e1000_ethdev.h|6 ++
 drivers/net/e1000/em_rxtx.c |4 ++--
 drivers/net/e1000/igb_rxtx.c|4 ++--
 drivers/net/enic/enic_ethdev.c  |9 +++--
 drivers/net/i40e/i40e_rxtx.c|4 ++--
 drivers/net/i40e/i40e_rxtx.h|3 +--
 drivers/net/ixgbe/ixgbe_ethdev.h|3 +--
 drivers/net/ixgbe/ixgbe_rxtx.c  |4 ++--
 drivers/net/nfp/nfp_net.c   |9 -
 drivers/net/sfc/sfc_ethdev.c|4 ++--
 drivers/net/thunderx/nicvf_ethdev.c |2 +-
 drivers/net/thunderx/nicvf_rxtx.c   |4 ++--
 drivers/net/thunderx/nicvf_rxtx.h   |2 +-
 drivers/net/vhost/rte_eth_vhost.c   |4 ++--
 examples/l3fwd-power/main.c |2 +-
 lib/librte_ethdev/rte_ethdev_core.h |4 ++--
 23 files changed, 44 insertions(+), 52 deletions(-)

diff --git a/drivers/net/ark/ark_ethdev_rx.c b/drivers/net/ark/ark_ethdev_rx.c
index 987d085e2..7f0a6fc52 100644
--- a/drivers/net/ark/ark_ethdev_rx.c
+++ b/drivers/net/ark/ark_ethdev_rx.c
@@ -407,13 +407,13 @@ eth_ark_rx_queue_drain(struct ark_rx_queue *queue)
}
 }
 
-uint32_t
+int
 eth_ark_dev_rx_queue_count(struct rte_eth_dev *dev, uint16_t queue_id)
 {
struct ark_rx_queue *queue;
 
queue = dev->data->rx_queues[queue_id];
-   return (queue->prod_index - queue->cons_index); /* mod arith */
+   return (int)(queue->prod_index - queue->cons_index);/* mod arith */
 }
 
 /* * */
diff --git a/drivers/net/ark/ark_ethdev_rx.h b/drivers/net/ark/ark_ethdev_rx.h
index 146787112..c3f56be3b 100644
--- a/drivers/net/ark/ark_ethdev_rx.h
+++ b/drivers/net/ark/ark_ethdev_rx.h
@@ -47,8 +47,7 @@ int eth_ark_dev_rx_queue_setup(struct rte_eth_dev *dev,
   unsigned int socket_id,
   const struct rte_eth_rxconf *rx_conf,
   struct rte_mempool *mp);
-uint32_t eth_ark_dev_rx_queue_count(struct rte_eth_dev *dev,
-   uint16_t rx_queue_id);
+int eth_ark_dev_rx_queue_count(struct rte_eth_dev *dev, uint16_t rx_queue_id);
 int eth_ark_rx_stop_queue(struct rte_eth_dev *dev, uint16_t queue_id);
 int eth_ark_rx_start_queue(struct rte_eth_dev *dev, uint16_t queue_id);
 uint16_t eth_ark_recv_pkts_noop(void *rx_queue, struct rte_mbuf **rx_pkts,
diff --git a/drivers/net/avf/avf_rxtx.c b/drivers/net/avf/avf_rxtx.c
index e03a136fc..abfca0e85 100644
--- a/drivers/net/avf/avf_rxtx.c
+++ b/drivers/net/avf/avf_rxtx.c
@@ -1839,13 +1839,13 @@ avf_dev_txq_info_get(struct rte_eth_dev *dev, uint16_t 
queue_id,
 }
 
 /* Get the number of used descriptors of a rx queue */
-uint32_t
+int
 avf_dev_rxq_count(struct rte_eth_dev *dev, uint16_t queue_id)
 {
 #define AVF_RXQ_SCAN_INTERVAL 4
volatile union avf_rx_desc *rxdp;
struct avf_rx_queue *rxq;
-   uint16_t desc = 0;
+   int desc = 0;
 
rxq = dev->data->rx_queues[queue_id];
rxdp = &rxq->rx_ring[rxq->rx_tail];
diff --git a/drivers/net/avf/avf_rxtx.h b/drivers/net/avf/avf_rxtx.h
index 297d0776d..fa287a003 100644
--- a/drivers/net/avf/avf_rxtx.h
+++ b/drivers/net/avf/avf_rxtx.h
@@ -185,7 +185,7 @@ void avf_dev_rxq_info_get(struct rte_eth_dev *dev, uint16_t 
queue_id,
  struct rte_eth_rxq_info *qinfo);
 void avf_dev_txq_info_get(struct rte_eth_dev *dev, uint16_t queue_id,
  struct rte_eth_txq_info *qinfo);
-uint32_t avf_dev_rxq_count(struct rte_eth_dev *dev, uint16_t queue_id);
+int avf_dev_rxq_count(struct rte_eth_dev *dev, uint16_t queue_id);
 int avf_dev_rx_desc_status(void *rx_queue, uint16_t offset);
 int avf_dev_tx_desc_status(void *tx_queue, uint16_t offset);
 
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 9edcc7b7d..edee42bb2 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -1610,15 +1610,16 @@ bnxt_dev_led_off_op(struct rte_eth_dev *dev)
return bnxt_hwrm_port_led_cfg(bp, false);
 }
 
-static uint32_t
+static int
 bnxt_rx_queue_count_op(struct rte_eth_dev *dev, uint16_t rx_queue_id)
 {
-   uint32_t desc = 0, raw_cons = 0, cons;
struct bnxt_cp_ring_info *cpr;
+   uint32_t raw_cons = 0, cons;
struct bnxt_rx_queue *rxq;
struct rx_pkt_cmpl *rxcmp;
uint16_t cmp_type;
uint8_t cmp = 1;
+   int desc = 0;
bool valid;
 
rxq = dev->data->rx_queues[rx_queue_id];
diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c
index d014a11aa..70a5b4851 100644

Re: [dpdk-dev] [PATCH v4 01/23] lib/librte_eal: import libbsd strlcpy

[Bruce Richardson]

On Thu, May 17, 2018 at 09:00:25PM +0800, Andy Green wrote:
> 
> 
> On 05/17/2018 08:56 PM, Bruce Richardson wrote:
> > On Thu, May 17, 2018 at 08:35:21PM +0800, Andy Green wrote:
> > > 
> > > 
> > > On 05/17/2018 06:36 PM, Bruce Richardson wrote:
> > > > On Mon, May 14, 2018 at 01:09:32PM +0800, Andy Green wrote:
> > > > > Signed-off-by: Andy Green 
> > > > > ---
> > > > >lib/librte_eal/common/eal_common_string_fns.c  |   34 
> > > > > 
> > > > >lib/librte_eal/common/include/rte_string_fns.h |7 +
> > > > >2 files changed, 36 insertions(+), 5 deletions(-)
> > > > > 
> > > > 
> > > > While I'm aware this was suggested by other reviewers, I really don't 
> > > > feel
> > > > that it is necessary to actually import the code. If libbsd is present 
> > > > on
> > > > the system, we will use it directly. If libbsd is not present, the 
> > > > snprintf
> > > > provides an acceptable fallback for strlcpy IMHO. Having the full 
> > > > function
> > > > without good justification seems excessive.
> > > 
> > > Well, as you can probably guess, I don't really mind either way.
> > > 
> > > This also implies another patch to export rte_strlcpy since it's no longer
> > > an inline in the headers this way.
> > > 
> > > I removed these patches and rebuilt dpdk and then lagopus without it with
> > > the idea of pasting the compile error.  But I can't reproduce the original
> > > problem... since then I rebased on current master dpdk, got updated to gcc
> > > 8.1 and added more patches on lagopus.
> > > 
> > > So just drop this patch if you don't want the bsd lstrcpy.
> > > 
> > Yes, let's drop it from the set for now anyway, if it's not solving any
> > specific error. We can relook at it in 18.08 anyway.
> 
> Sorry to immediately contradict myself but I forgot a step in the rather
> complicated flow to force the lagopus dpdk submodule HEAD... by default
> making lagopus forces the submodule commit to something else.  It does need
> a much smaller one-liner to avoid
> 
> In file included from ./dpdk/dpdk.c:88:
> /projects/lagopus/src/dpdk/build/include/rte_string_fns.h: In
> function 'rte_strlcpy':
> /projects/lagopus/src/dpdk/build/include/rte_string_fns.h:58:9:
> warning: conversion to 'size_t' {aka 'long unsigned int'} from
> 'int' may change the sign of the result [-Wsign-conversion]
>   return snprintf(dst, size, "%s", src);
>  ^~
> 
> It just needs a cast to make the return type of the snprintf compatible with
> the expected return type of strlcpy().
> 
> I'll include this in the next send in an hour or two.
> 
Great, thanks for the all the patches!

[dpdk-dev] [PATCH v5 04/21] /lib/librte_eal: stage cast from uint64 to long

[Andy Green]

/projects/lagopus/src/dpdk/build/include/rte_random.h:
In function 'rte_srand':
/projects/lagopus/src/dpdk/build/include/rte_random.h:34:10:
warning: conversion to 'long int' from 'long unsigned int'
may change the sign of the result [-Wsign-conversion]
  srand48((long unsigned int)seedval);

/projects/lagopus/src/dpdk/build/include/rte_random.h:51:8:
warning: conversion to 'uint64_t' {aka 'long unsigned int'}
from 'long int' may change the sign of the result
[-Wsign-conversion]
  val = lrand48();
^~~
/projects/lagopus/src/dpdk/build/include/rte_random.h:53:6:
warning: conversion to 'long unsigned int' from 'long int'
may change the sign of the result [-Wsign-conversion]
  val += lrand48();

Signed-off-by: Andy Green 
---
 lib/librte_eal/common/include/rte_random.h |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/common/include/rte_random.h 
b/lib/librte_eal/common/include/rte_random.h
index 63bb28088..b2ca1c209 100644
--- a/lib/librte_eal/common/include/rte_random.h
+++ b/lib/librte_eal/common/include/rte_random.h
@@ -31,7 +31,7 @@ extern "C" {
 static inline void
 rte_srand(uint64_t seedval)
 {
-   srand48((long unsigned int)seedval);
+   srand48((long)seedval);
 }
 
 /**
@@ -48,9 +48,9 @@ static inline uint64_t
 rte_rand(void)
 {
uint64_t val;
-   val = lrand48();
+   val = (uint64_t)lrand48();
val <<= 32;
-   val += lrand48();
+   val += (uint64_t)lrand48();
return val;
 }
 

[dpdk-dev] [PATCH v5 05/21] rte_ring_generic.h: stack declarations before code

[Andy Green]

/projects/lagopus/src/dpdk/build/include/rte_ring_generic.h:
In function '__rte_ring_move_prod_head':
/projects/lagopus/src/dpdk/build/include/rte_ring_generic.h:76:3:
warning: ISO C90 forbids mixed declarations and code
[-Wdeclaration-after-statement]
   const uint32_t cons_tail = r->cons.tail;
   ^
/projects/lagopus/src/dpdk/build/include/rte_ring_generic.h:
In function '__rte_ring_move_cons_head':
/projects/lagopus/src/dpdk/build/include/rte_ring_generic.h:147:3:
warning: ISO C90 forbids mixed declarations and code
[-Wdeclaration-after-statement]
   const uint32_t prod_tail = r->prod.tail;

Signed-off-by: Andy Green 
Fixes: 0dfc98c507b1 ("ring: separate out head index manipulation")
---
 lib/librte_ring/rte_ring_generic.h |6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/lib/librte_ring/rte_ring_generic.h 
b/lib/librte_ring/rte_ring_generic.h
index 5b110425f..c2d482bc9 100644
--- a/lib/librte_ring/rte_ring_generic.h
+++ b/lib/librte_ring/rte_ring_generic.h
@@ -73,14 +73,13 @@ __rte_ring_move_prod_head(struct rte_ring *r, int is_sp,
 */
rte_smp_rmb();
 
-   const uint32_t cons_tail = r->cons.tail;
/*
 *  The subtraction is done between two unsigned 32bits value
 * (the result is always modulo 32 bits even if we have
 * *old_head > cons_tail). So 'free_entries' is always between 0
 * and capacity (which is < size).
 */
-   *free_entries = (capacity + cons_tail - *old_head);
+   *free_entries = (capacity + r->cons.tail - *old_head);
 
/* check that we have enough room in ring */
if (unlikely(n > *free_entries))
@@ -144,13 +143,12 @@ __rte_ring_move_cons_head(struct rte_ring *r, int is_sc,
 */
rte_smp_rmb();
 
-   const uint32_t prod_tail = r->prod.tail;
/* The subtraction is done between two unsigned 32bits value
 * (the result is always modulo 32 bits even if we have
 * cons_head > prod_tail). So 'entries' is always between 0
 * and size(ring)-1.
 */
-   *entries = (prod_tail - *old_head);
+   *entries = (r->prod.tail - *old_head);
 
/* Set the actual entries for dequeue */
if (n > *entries)

[dpdk-dev] [PATCH v5 03/21] lib/librte_eal: explicit tmp cast

[Andy Green]

/projects/lagopus/src/dpdk/build/include/rte_memcpy.h:
793:2: note: in expansion of macro 'MOVEUNALIGNED_LEFT47'
  MOVEUNALIGNED_LEFT47(dst, src, n, srcofs);
  ^~~~
/projects/lagopus/src/dpdk/build/include/rte_memcpy.h:
649:51: warning: conversion from 'size_t' {aka 'long
unsigned int'} to 'int' may change value [-Wconversion]
 case 0x0B: MOVEUNALIGNED_LEFT47_IMM(dst, src,
n, 0x0B); break;\
   ^
/projects/lagopus/src/dpdk/build/include/rte_memcpy.h:
616:15: note: in definition of macro 'MOVEUNALIGNED_LEFT47_IMM'
 tmp = len; 
 \
   ^~~
/projects/lagopus/src/dpdk/build/include/rte_memcpy.h:
793:2: note: in expansion of macro 'MOVEUNALIGNED_LEFT47'
  MOVEUNALIGNED_LEFT47(dst, src, n, srcofs);
  ^~~~
/projects/lagopus/src/dpdk/build/include/rte_memcpy.h:
618:13: warning: conversion to 'size_t' {aka 'long
unsigned int'} from 'int' may change the sign of the
result [-Wsign-conversion]
 tmp -= len;
 \
 ^~
/projects/lagopus/src/dpdk/build/include/rte_memcpy.h:
649:16: note: in expansion of macro 'MOVEUNALIGNED_LEFT47_IMM'
 case 0x0B: MOVEUNALIGNED_LEFT47_IMM(dst, src,
n, 0x0B); break;\
^~~~
/projects/lagopus/src/dpdk/build/include/rte_memcpy.h:
793:2: note: in expansion of macro 'MOVEUNALIGNED_LEFT47'
  MOVEUNALIGNED_LEFT47(dst, src, n, srcofs);
  ^~~~

Signed-off-by: Andy Green 
---
 .../common/include/arch/x86/rte_memcpy.h   |8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h 
b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
index 5ead68ab2..f9ea0ab69 100644
--- a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
+++ b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
@@ -597,9 +597,9 @@ __extension__ ({
 _mm_storeu_si128((__m128i *)((uint8_t *)dst + 7 * 16), 
_mm_alignr_epi8(xmm8, xmm7, offset));\
 dst = (uint8_t *)dst + 128;
 \
 }  
 \
-tmp = len; 
 \
+tmp = (int)len;
 \
 len = ((len - 16 + offset) & 127) + 16 - offset;   
 \
-tmp -= len;
 \
+tmp -= (int)len;   
 \
 src = (const uint8_t *)src + tmp;  
 \
 dst = (uint8_t *)dst + tmp;
 \
 if (len >= 32 + 16 - offset) { 
 \
@@ -613,9 +613,9 @@ __extension__ ({
 _mm_storeu_si128((__m128i *)((uint8_t *)dst + 1 * 16), 
_mm_alignr_epi8(xmm2, xmm1, offset));\
 dst = (uint8_t *)dst + 32; 
 \
 }  
 \
-tmp = len; 
 \
+tmp = (int)len;
 \
 len = ((len - 16 + offset) & 31) + 16 - offset;
 \
-tmp -= len;
 \
+tmp -= (int)len;   
 \
 src = (const uint8_t *)src + tmp;  
 \
 dst = (uint8_t *)dst + tmp;
 \
 }  
 \

[dpdk-dev] [PATCH v5 07/21] rte_mbuf.h: avoid warnings from inadvertant promotion

[Andy Green]

"1 + value", where value is an uint16_t causes promotion
to a signed int.  The compiler complained that we are
shoving an int into a uint16_t return type with different
size and sign.

Bumping and returning value directly instead removes the
promotion and the problem.

Signed-off-by: Andy Green 
---
 lib/librte_mbuf/rte_mbuf.h |   10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 4fd9a0d9e..a2a37a311 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -836,8 +836,9 @@ rte_mbuf_refcnt_update(struct rte_mbuf *m, int16_t value)
 * reference counter can occur.
 */
if (likely(rte_mbuf_refcnt_read(m) == 1)) {
-   rte_mbuf_refcnt_set(m, 1 + value);
-   return 1 + value;
+   ++value;
+   rte_mbuf_refcnt_set(m, value);
+   return value;
}
 
return __rte_mbuf_refcnt_update(m, value);
@@ -927,8 +928,9 @@ rte_mbuf_ext_refcnt_update(struct rte_mbuf_ext_shared_info 
*shinfo,
int16_t value)
 {
if (likely(rte_mbuf_ext_refcnt_read(shinfo) == 1)) {
-   rte_mbuf_ext_refcnt_set(shinfo, 1 + value);
-   return 1 + value;
+   ++value;
+   rte_mbuf_ext_refcnt_set(shinfo, value);
+   return value;
}
 
return (uint16_t)rte_atomic16_add_return(&shinfo->refcnt_atomic, value);

[dpdk-dev] [PATCH v5 06/21] rte_ring.h: remove signed type flipflopping

[Andy Green]

/projects/lagopus/src/dpdk/build/include/rte_ring.h:350:46:
warning: conversion to 'uint32_t' {aka 'unsigned int'}
from 'int' may change the sign of the result
[-Wsign-conversion]
  update_tail(&r->prod, prod_head, prod_next, is_sp, 1);

The visible apis take unsigned int, then call a private
api taking an int, which finally calls an api taking an
unsigned int.

Convert the private api to take unsigned int removing
5 x warning similar to that shown above.

Signed-off-by: Andy Green 
---
 lib/librte_ring/rte_ring.h |4 ++--
 lib/librte_ring/rte_ring_c11_mem.h |2 +-
 lib/librte_ring/rte_ring_generic.h |4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h
index d3d3f7f97..124582251 100644
--- a/lib/librte_ring/rte_ring.h
+++ b/lib/librte_ring/rte_ring.h
@@ -335,7 +335,7 @@ void rte_ring_dump(FILE *f, const struct rte_ring *r);
 static __rte_always_inline unsigned int
 __rte_ring_do_enqueue(struct rte_ring *r, void * const *obj_table,
 unsigned int n, enum rte_ring_queue_behavior behavior,
-int is_sp, unsigned int *free_space)
+unsigned int is_sp, unsigned int *free_space)
 {
uint32_t prod_head, prod_next;
uint32_t free_entries;
@@ -377,7 +377,7 @@ __rte_ring_do_enqueue(struct rte_ring *r, void * const 
*obj_table,
 static __rte_always_inline unsigned int
 __rte_ring_do_dequeue(struct rte_ring *r, void **obj_table,
 unsigned int n, enum rte_ring_queue_behavior behavior,
-int is_sc, unsigned int *available)
+unsigned int is_sc, unsigned int *available)
 {
uint32_t cons_head, cons_next;
uint32_t entries;
diff --git a/lib/librte_ring/rte_ring_c11_mem.h 
b/lib/librte_ring/rte_ring_c11_mem.h
index 08825ea5b..cb3f82b1a 100644
--- a/lib/librte_ring/rte_ring_c11_mem.h
+++ b/lib/librte_ring/rte_ring_c11_mem.h
@@ -51,7 +51,7 @@ update_tail(struct rte_ring_headtail *ht, uint32_t old_val, 
uint32_t new_val,
  *   If behavior == RTE_RING_QUEUE_FIXED, this will be 0 or n only.
  */
 static __rte_always_inline unsigned int
-__rte_ring_move_prod_head(struct rte_ring *r, int is_sp,
+__rte_ring_move_prod_head(struct rte_ring *r, unsigned int is_sp,
unsigned int n, enum rte_ring_queue_behavior behavior,
uint32_t *old_head, uint32_t *new_head,
uint32_t *free_entries)
diff --git a/lib/librte_ring/rte_ring_generic.h 
b/lib/librte_ring/rte_ring_generic.h
index c2d482bc9..ea7dbe5b9 100644
--- a/lib/librte_ring/rte_ring_generic.h
+++ b/lib/librte_ring/rte_ring_generic.h
@@ -53,7 +53,7 @@ update_tail(struct rte_ring_headtail *ht, uint32_t old_val, 
uint32_t new_val,
  *   If behavior == RTE_RING_QUEUE_FIXED, this will be 0 or n only.
  */
 static __rte_always_inline unsigned int
-__rte_ring_move_prod_head(struct rte_ring *r, int is_sp,
+__rte_ring_move_prod_head(struct rte_ring *r, unsigned int is_sp,
unsigned int n, enum rte_ring_queue_behavior behavior,
uint32_t *old_head, uint32_t *new_head,
uint32_t *free_entries)
@@ -123,7 +123,7 @@ __rte_ring_move_prod_head(struct rte_ring *r, int is_sp,
  * If behavior == RTE_RING_QUEUE_FIXED, this will be 0 or n only.
  */
 static __rte_always_inline unsigned int
-__rte_ring_move_cons_head(struct rte_ring *r, int is_sc,
+__rte_ring_move_cons_head(struct rte_ring *r, unsigned int is_sc,
unsigned int n, enum rte_ring_queue_behavior behavior,
uint32_t *old_head, uint32_t *new_head,
uint32_t *entries)

[dpdk-dev] [PATCH v5 08/21] rte_mbuf.h: explicit casts for int16 to uint16

[Andy Green]

differences to the atomic16 are signed, but the
atomic16 itself is unsigned.  It needs to be
made explicit with casts.

Signed-off-by: Andy Green 
If we want it for 18.05:
Acked-by: Olivier Matz 
---
 lib/librte_mbuf/rte_mbuf.h |   12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index a2a37a311..c214f1945 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -806,7 +806,7 @@ rte_mbuf_refcnt_read(const struct rte_mbuf *m)
 static inline void
 rte_mbuf_refcnt_set(struct rte_mbuf *m, uint16_t new_value)
 {
-   rte_atomic16_set(&m->refcnt_atomic, new_value);
+   rte_atomic16_set(&m->refcnt_atomic, (int16_t)new_value);
 }
 
 /* internal */
@@ -837,8 +837,8 @@ rte_mbuf_refcnt_update(struct rte_mbuf *m, int16_t value)
 */
if (likely(rte_mbuf_refcnt_read(m) == 1)) {
++value;
-   rte_mbuf_refcnt_set(m, value);
-   return value;
+   rte_mbuf_refcnt_set(m, (uint16_t)value);
+   return (uint16_t)value;
}
 
return __rte_mbuf_refcnt_update(m, value);
@@ -909,7 +909,7 @@ static inline void
 rte_mbuf_ext_refcnt_set(struct rte_mbuf_ext_shared_info *shinfo,
uint16_t new_value)
 {
-   rte_atomic16_set(&shinfo->refcnt_atomic, new_value);
+   rte_atomic16_set(&shinfo->refcnt_atomic, (int16_t)new_value);
 }
 
 /**
@@ -929,8 +929,8 @@ rte_mbuf_ext_refcnt_update(struct rte_mbuf_ext_shared_info 
*shinfo,
 {
if (likely(rte_mbuf_ext_refcnt_read(shinfo) == 1)) {
++value;
-   rte_mbuf_ext_refcnt_set(shinfo, value);
-   return value;
+   rte_mbuf_ext_refcnt_set(shinfo, (uint16_t)value);
+   return (uint16_t)value;
}
 
return (uint16_t)rte_atomic16_add_return(&shinfo->refcnt_atomic, value);

[dpdk-dev] [PATCH v5 09/21] rte_mbuf.h: make sure RTE-MIN compares same types

[Andy Green]

/projects/lagopus/src/dpdk/build/include/rte_common.h:384:2:
warning: conversion from 'int' to 'uint16_t'
{aka 'short unsigned int'} may change value [-Wconversion]
  __extension__ ({ \
  ^
/projects/lagopus/src/dpdk/build/include/rte_mbuf.h:1204:16:
note: in expansion of macro 'RTE_MIN'
  m->data_off = RTE_MIN(RTE_PKTMBUF_HEADROOM,
(uint16_t)m->buf_len);

RTE_PKTMBUF_HEADROOM is typ 128, so it doesn't make trouble.

Signed-off-by: Andy Green 
Acked-by: Bruce Richardson 
---
 lib/librte_mbuf/rte_mbuf.h |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index c214f1945..a27dbb878 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -1201,7 +1201,8 @@ rte_pktmbuf_priv_size(struct rte_mempool *mp)
  */
 static inline void rte_pktmbuf_reset_headroom(struct rte_mbuf *m)
 {
-   m->data_off = RTE_MIN(RTE_PKTMBUF_HEADROOM, (uint16_t)m->buf_len);
+   m->data_off = (uint16_t)RTE_MIN((uint16_t)RTE_PKTMBUF_HEADROOM,
+   (uint16_t)m->buf_len);
 }
 
 /**

[dpdk-dev] [PATCH v5 10/21] rte_mbuf.h: explicit cast restricting ptrdiff to uint16

[Andy Green]

/projects/lagopus/src/dpdk/build/include/rte_common.h:141:34:
warning: conversion from 'long unsigned int' to 'uint16_t'
{aka 'short unsigned int'} may change value [-Wconversion]
 #define RTE_PTR_DIFF(ptr1, ptr2) ((uintptr_t)(ptr1) -
(uintptr_t)(ptr2))
  ^
/projects/lagopus/src/dpdk/build/include/rte_mbuf.h:1360:13:
note: in expansion of macro 'RTE_PTR_DIFF'
  *buf_len = RTE_PTR_DIFF(shinfo, buf_addr);

Signed-off-by: Andy Green 
Acked-by: Bruce Richardson 
---
 lib/librte_mbuf/rte_mbuf.h |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index a27dbb878..0580ec8a0 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -1358,7 +1358,7 @@ rte_pktmbuf_ext_shinfo_init_helper(void *buf_addr, 
uint16_t *buf_len,
shinfo->fcb_opaque = fcb_opaque;
rte_mbuf_ext_refcnt_set(shinfo, 1);
 
-   *buf_len = RTE_PTR_DIFF(shinfo, buf_addr);
+   *buf_len = (uint16_t)RTE_PTR_DIFF(shinfo, buf_addr);
return shinfo;
 }
 

[dpdk-dev] [PATCH v5 11/21] rte_ether.h: explicit cast avoiding truncation warning

[Andy Green]

/projects/lagopus/src/dpdk/build/include/rte_ether.h:213:13:
warning: conversion from 'int' to 'uint8_t'
{aka 'unsigned char'} may change value [-Wconversion]
  addr[0] &= ~ETHER_GROUP_ADDR;
/* clear multicast bit */

Signed-off-by: Andy Green 
Acked-by: Bruce Richardson 
---
 lib/librte_net/rte_ether.h |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h
index 27c919594..bee2b34f0 100644
--- a/lib/librte_net/rte_ether.h
+++ b/lib/librte_net/rte_ether.h
@@ -210,7 +210,7 @@ static inline void eth_random_addr(uint8_t *addr)
uint8_t *p = (uint8_t *)&rand;
 
rte_memcpy(addr, p, ETHER_ADDR_LEN);
-   addr[0] &= ~ETHER_GROUP_ADDR;   /* clear multicast bit */
+   addr[0] &= (uint8_t)~ETHER_GROUP_ADDR;   /* clear multicast bit */
addr[0] |= ETHER_LOCAL_ADMIN_ADDR;  /* set local assignment bit */
 }
 

[dpdk-dev] [PATCH v5 12/21] rte_rwlock.h: gcc8 sign conversion warnings

[Andy Green]

In file included from /projects/lagopus/src/dpdk/
build/include/rte_rwlock.h:12,
 from ./mgr/lock.h:24,
 from ./mgr/sock_io.c:54:
/projects/lagopus/src/dpdk/build/include/generic/
rte_rwlock.h: In function 'rte_rwlock_read_lock':
/projects/lagopus/src/dpdk/build/include/generic/
rte_rwlock.h:74:12: warning: conversion to 'uint32_t'
{aka 'unsigned int'} from 'int32_t' {aka 'int'} may
change the sign of the result [-Wsign-conversion]
x, x + 1);
^
/projects/lagopus/src/dpdk/build/include/generic/
rte_rwlock.h:74:17: warning: conversion to 'uint32_t'
{aka 'unsigned int'} from 'int' may change the sign
of the result [-Wsign-conversion]
x, x + 1);
   ~~^~~
/projects/lagopus/src/dpdk/build/include/generic/
rte_rwlock.h: In function 'rte_rwlock_write_lock':
/projects/lagopus/src/dpdk/build/include/generic/
rte_rwlock.h:110:15: warning: unsigned conversion
from 'int' to 'uint32_t' {aka 'unsigned int'}
changes value from '-1' to '4294967295'
[-Wsign-conversion]
0, -1);
   ^~
Fixes: af75078fec ("first public release")
Signed-off-by: Andy Green 
---
 lib/librte_eal/common/include/generic/rte_rwlock.h |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eal/common/include/generic/rte_rwlock.h 
b/lib/librte_eal/common/include/generic/rte_rwlock.h
index 899e9bc43..5751a0e6d 100644
--- a/lib/librte_eal/common/include/generic/rte_rwlock.h
+++ b/lib/librte_eal/common/include/generic/rte_rwlock.h
@@ -71,7 +71,7 @@ rte_rwlock_read_lock(rte_rwlock_t *rwl)
continue;
}
success = rte_atomic32_cmpset((volatile uint32_t *)&rwl->cnt,
- x, x + 1);
+ (uint32_t)x, (uint32_t)(x + 1));
}
 }
 
@@ -107,7 +107,7 @@ rte_rwlock_write_lock(rte_rwlock_t *rwl)
continue;
}
success = rte_atomic32_cmpset((volatile uint32_t *)&rwl->cnt,
- 0, -1);
+ 0, (uint32_t)-1);
}
 }
 

[dpdk-dev] [PATCH v5 14/21] rte_ip.h: cast around promotion to int

[Andy Green]

/projects/lagopus/src/dpdk/build/include/
rte_ip.h: In function 'rte_ipv4_cksum':
/projects/lagopus/src/dpdk/build/include/
rte_ip.h:256:35: warning: conversion from
'int' to 'uint16_t' {aka 'short unsigned int'}
may change value [-Wconversion]
  return (cksum == 0x) ? cksum : ~cksum;
 ~~^~~~

Signed-off-by: Andy Green 
---
 lib/librte_net/rte_ip.h |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
index b46a0c717..8689e9113 100644
--- a/lib/librte_net/rte_ip.h
+++ b/lib/librte_net/rte_ip.h
@@ -253,7 +253,7 @@ rte_ipv4_cksum(const struct ipv4_hdr *ipv4_hdr)
 {
uint16_t cksum;
cksum = rte_raw_cksum(ipv4_hdr, sizeof(struct ipv4_hdr));
-   return (cksum == 0x) ? cksum : ~cksum;
+   return (cksum == 0x) ? cksum : (uint16_t)~cksum;
 }
 
 /**

[dpdk-dev] [PATCH v5 16/21] rte_ip.h: cast return checksum size to uint16

[Andy Green]

In file included from ./dpdk/worker.c:94:
/projects/lagopus/src/dpdk/build/include/rte_ip.h:
332:9: warning: conversion from 'uint32_t'
{aka 'unsigned int'} to 'uint16_t' {aka 'short
unsigned int'} may change value [-Wconversion]
  return cksum;
 ^

/projects/lagopus/src/dpdk/build/include/rte_ip.h:
In function 'rte_ipv6_udptcp_cksum':
/projects/lagopus/src/dpdk/build/include/rte_ip.h:421:9:
warning: conversion from 'uint32_t' {aka 'unsigned int'}
to 'uint16_t' {aka 'short unsigned int'} may change
value [-Wconversion]
  return cksum;
 ^


Signed-off-by: Andy Green 
---
 lib/librte_net/rte_ip.h |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
index 88dfbaa9a..c46398548 100644
--- a/lib/librte_net/rte_ip.h
+++ b/lib/librte_net/rte_ip.h
@@ -329,7 +329,7 @@ rte_ipv4_udptcp_cksum(const struct ipv4_hdr *ipv4_hdr, 
const void *l4_hdr)
if (cksum == 0)
cksum = 0x;
 
-   return cksum;
+   return (uint16_t)cksum;
 }
 
 /**
@@ -418,7 +418,7 @@ rte_ipv6_udptcp_cksum(const struct ipv6_hdr *ipv6_hdr, 
const void *l4_hdr)
if (cksum == 0)
cksum = 0x;
 
-   return cksum;
+   return (uint16_t)cksum;
 }
 
 #ifdef __cplusplus

[dpdk-dev] [PATCH v5 17/21] rte_ip.h: cast away gcc8 warning on rte_ipv6_phdr_cksum

[Andy Green]

/projects/lagopus/src/dpdk/build/include/rte_ip.h:
In function 'rte_ipv6_phdr_cksum':
/projects/lagopus/src/dpdk/build/include/rte_ip.h:
378:18: warning: conversion to 'uint32_t' {aka
'unsigned int'} from 'int' may change the sign of
the result [-Wsign-conversion]
  psd_hdr.proto = (ipv6_hdr->proto << 24);

Signed-off-by: Andy Green 
---
 lib/librte_net/rte_ip.h |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
index c46398548..72dc2456a 100644
--- a/lib/librte_net/rte_ip.h
+++ b/lib/librte_net/rte_ip.h
@@ -375,7 +375,7 @@ rte_ipv6_phdr_cksum(const struct ipv6_hdr *ipv6_hdr, 
uint64_t ol_flags)
uint32_t proto; /* L4 protocol - top 3 bytes must be zero */
} psd_hdr;
 
-   psd_hdr.proto = (ipv6_hdr->proto << 24);
+   psd_hdr.proto = (uint32_t)(ipv6_hdr->proto << 24);
if (ol_flags & PKT_TX_TCP_SEG) {
psd_hdr.len = 0;
} else {

[dpdk-dev] [PATCH v5 13/21] rte_ip.h: cast input to bswap16 to be uint16

[Andy Green]

In file included from /projects/lagopus/src/dpdk/
build/include/rte_ether.h:24,
 from /projects/lagopus/src/dpdk/
build/include/rte_ethdev.h:158,
 from ./dpdk/worker.c:90:
/projects/lagopus/src/dpdk/build/include/rte_ip.h:
In function 'rte_raw_cksum_mbuf':
/projects/lagopus/src/dpdk/build/include/rte_ip.h:
225:22: warning: conversion from 'uint32_t'
{aka 'unsigned int'} to 'uint16_t' {aka 'short
unsigned int'} may change value [-Wconversion]
tmp = rte_bswap16(tmp);
  ^~~
/projects/lagopus/src/dpdk/build/include/
rte_byteorder.h:53:25: note: in definition of
macro 'rte_bswap16'
rte_arch_bswap16(x)))
 ^

Signed-off-by: Andy Green 
---
 lib/librte_net/rte_ip.h |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
index f32684c66..b46a0c717 100644
--- a/lib/librte_net/rte_ip.h
+++ b/lib/librte_net/rte_ip.h
@@ -222,7 +222,7 @@ rte_raw_cksum_mbuf(const struct rte_mbuf *m, uint32_t off, 
uint32_t len,
for (;;) {
tmp = __rte_raw_cksum(buf, seglen, 0);
if (done & 1)
-   tmp = rte_bswap16(tmp);
+   tmp = rte_bswap16((uint16_t)tmp);
sum += tmp;
done += seglen;
if (done == len)

[dpdk-dev] [PATCH v5 15/21] rte_ip.h: cast type decided by sizeof to uint32

[Andy Green]

/projects/lagopus/src/dpdk/build/include/rte_ip.h:
In function 'rte_ipv4_udptcp_cksum':
/projects/lagopus/src/dpdk/build/include/rte_byteorder.h:
51:24: warning: conversion from 'long unsigned int' to
'uint32_t' {aka 'unsigned int'} may change value
[-Wconversion]
 #define rte_bswap16(x) ((uint16_t)
(__builtin_constant_p(x) ?  \
^
/projects/lagopus/src/dpdk/build/include/rte_byteorder.h:
85:29: note: in expansion of macro 'rte_bswap16'
 #define rte_be_to_cpu_16(x) rte_bswap16(x)
 ^~~
/projects/lagopus/src/dpdk/build/include/rte_ip.h:321:11:
note: in expansion of macro 'rte_be_to_cpu_16'
  l4_len = rte_be_to_cpu_16(ipv4_hdr->total_length) -
   ^~~~

Signed-off-by: Andy Green 
---
 lib/librte_net/rte_ip.h |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
index 8689e9113..88dfbaa9a 100644
--- a/lib/librte_net/rte_ip.h
+++ b/lib/librte_net/rte_ip.h
@@ -318,8 +318,8 @@ rte_ipv4_udptcp_cksum(const struct ipv4_hdr *ipv4_hdr, 
const void *l4_hdr)
uint32_t cksum;
uint32_t l4_len;
 
-   l4_len = rte_be_to_cpu_16(ipv4_hdr->total_length) -
-   sizeof(struct ipv4_hdr);
+   l4_len = (uint32_t)(rte_be_to_cpu_16(ipv4_hdr->total_length) -
+   sizeof(struct ipv4_hdr));
 
cksum = rte_raw_cksum(l4_hdr, l4_len);
cksum += rte_ipv4_phdr_cksum(ipv4_hdr, 0);

[dpdk-dev] [PATCH v5 18/21] rte_mbuf.h: explicit cast for size type to uint32

[Andy Green]

/projects/lagopus/src/dpdk/build/include/rte_mbuf.h:
In function 'rte_pktmbuf_detach':
/projects/lagopus/src/dpdk/build/include/rte_mbuf.h:
1580:14: warning: conversion from 'long unsigned int'
to 'uint32_t' {aka 'unsigned int'} may change
value [-Wconversion]
  mbuf_size = sizeof(struct rte_mbuf) + priv_size;
  ^~

Signed-off-by: Andy Green 
---
 lib/librte_mbuf/rte_mbuf.h |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 0580ec8a0..7849192c6 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -1577,7 +1577,7 @@ static inline void rte_pktmbuf_detach(struct rte_mbuf *m)
__rte_pktmbuf_free_direct(m);
 
priv_size = rte_pktmbuf_priv_size(mp);
-   mbuf_size = sizeof(struct rte_mbuf) + priv_size;
+   mbuf_size = (uint32_t)(sizeof(struct rte_mbuf) + priv_size);
buf_len = rte_pktmbuf_data_room_size(mp);
 
m->priv_size = priv_size;

[dpdk-dev] [PATCH v5 19/21] rte_mbuf.h: explicit casts to uint16 to avoid warnings

[Andy Green]

/projects/lagopus/src/dpdk/build/include/rte_mbuf.h:
In function 'rte_pktmbuf_detach':
/projects/lagopus/src/dpdk/build/include/rte_mbuf.h:
1583:17: warning: conversion from 'uint32_t' {aka
'unsigned int'} to 'uint16_t' {aka
'short unsigned int'} may change value [-Wconversion]
  m->priv_size = priv_size;
 ^
/projects/lagopus/src/dpdk/build/include/rte_mbuf.h:
In function 'rte_pktmbuf_prepend':
/projects/lagopus/src/dpdk/build/include/rte_mbuf.h:
1908:17: warning: conversion from 'int' to 'uint16_t'
{aka 'short unsigned int'} may change value [-Wconversion]
  m->data_off -= len;
 ^~~
/projects/lagopus/src/dpdk/build/include/rte_mbuf.h:
In function 'rte_pktmbuf_adj':
/projects/lagopus/src/dpdk/build/include/rte_mbuf.h:
1969:17: warning: conversion from 'int' to 'uint16_t'
{aka 'short unsigned int'} may change value [-Wconversion]
  m->data_off += len;
 ^~~
/projects/lagopus/src/dpdk/build/include/rte_mbuf.h:
In function 'rte_pktmbuf_chain':
/projects/lagopus/src/dpdk/build/include/rte_mbuf.h:
2082:19: warning: conversion from 'int' to 'uint16_t'
{aka 'short unsigned int'} may change value [-Wconversion]
  head->nb_segs += tail->nb_segs;
   ^~~~
/projects/lagopus/src/dpdk/build/include/rte_mbuf.h:
In function 'rte_validate_tx_offload':
/projects/lagopus/src/dpdk/build/include/rte_mbuf.h:
2112:19: warning: conversion to 'uint64_t'
{aka 'long unsigned int'} from 'int' may change the
sign of the result [-Wsign-conversion]
   inner_l3_offset += m->outer_l2_len + m->outer_l3_len;
   ^~
/projects/lagopus/src/dpdk/build/include/rte_mbuf.h:
In function 'rte_pktmbuf_linearize':
/projects/lagopus/src/dpdk/build/include/rte_mbuf.h:
1873:32: warning: conversion to 'int' from 'uint32_t'
{aka 'unsigned int'} may change the sign of the
result [-Wsign-conversion]
 #define rte_pktmbuf_pkt_len(m) ((m)->pkt_len)
^
/projects/lagopus/src/dpdk/build/include/rte_mbuf.h:
2166:13: note: in expansion of macro 'rte_pktmbuf_pkt_len'
  copy_len = rte_pktmbuf_pkt_len(mbuf) -
rte_pktmbuf_data_len(mbuf);
 ^~~
/projects/lagopus/src/dpdk/build/include/rte_mbuf.h:
2180:51: warning: conversion to 'size_t' {aka
'long unsigned int'} from 'int' may change the
sign of the result [-Wsign-conversion]
   rte_memcpy(buffer, rte_pktmbuf_mtod(m, char *), seg_len);
   ^~~
Signed-off-by: Andy Green 
---
 lib/librte_mbuf/rte_mbuf.h |   16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 7849192c6..4eddf238e 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -1569,7 +1569,8 @@ __rte_pktmbuf_free_direct(struct rte_mbuf *m)
 static inline void rte_pktmbuf_detach(struct rte_mbuf *m)
 {
struct rte_mempool *mp = m->pool;
-   uint32_t mbuf_size, buf_len, priv_size;
+   uint32_t mbuf_size, buf_len;
+   uint16_t priv_size;
 
if (RTE_MBUF_HAS_EXTBUF(m))
__rte_pktmbuf_free_extbuf(m);
@@ -1905,7 +1906,7 @@ static inline char *rte_pktmbuf_prepend(struct rte_mbuf 
*m,
if (unlikely(len > rte_pktmbuf_headroom(m)))
return NULL;
 
-   m->data_off -= len;
+   m->data_off = (uint16_t)(m->data_off - len);
m->data_len = (uint16_t)(m->data_len + len);
m->pkt_len  = (m->pkt_len + len);
 
@@ -1966,7 +1967,7 @@ static inline char *rte_pktmbuf_adj(struct rte_mbuf *m, 
uint16_t len)
return NULL;
 
m->data_len = (uint16_t)(m->data_len - len);
-   m->data_off += len;
+   m->data_off = (uint16_t)(m->data_off + len);
m->pkt_len  = (m->pkt_len - len);
return (char *)m->buf_addr + m->data_off;
 }
@@ -2079,7 +2080,7 @@ static inline int rte_pktmbuf_chain(struct rte_mbuf 
*head, struct rte_mbuf *tail
cur_tail->next = tail;
 
/* accumulate number of segments and total length. */
-   head->nb_segs += tail->nb_segs;
+   head->nb_segs = (uint16_t)(head->nb_segs + tail->nb_segs);
head->pkt_len += tail->pkt_len;
 
/* pkt_len is only set in the head */
@@ -2109,7 +2110,8 @@ rte_validate_tx_offload(const struct rte_mbuf *m)
return 0;
 
if (ol_flags & PKT_TX_OUTER_IP_CKSUM)
-   inner_l3_offset += m->outer_l2_len + m->outer_l3_len;
+   inner_l3_offset += (unsigned int)(m->outer_l2_len +
+ m->outer_l3_len);
 
/* Headers are fragmented */
if (rte_pktmbuf_data_len(m) < inner_l3_offset + m->l3_len + m->l4_len)
@@ -2154,7 +2156,7 @@ rte_validate_tx_offload(const struct rte_mbuf *m)
 static inline int
 rte_pktmbuf_linearize(struct rte_mbuf *mbuf)
 {
-   int seg_len, copy_len;
+   size_t seg_len, copy_len;
struct rte_mbuf *m;
struct rte_mbuf *m_next;
char *buffer;
@@ -2169,7 +21

[dpdk-dev] [PATCH v5 20/21] rte_ethdev.h: align sign and scope of temp var

[Andy Green]

/projects/lagopus/src/dpdk/build/include/rte_ethdev.h:
In function 'rte_eth_rx_burst':
/projects/lagopus/src/dpdk/build/include/rte_ethdev.h:
3836:18: warning: conversion to 'int16_t' {aka 'short
int'} from 'uint16_t' {aka 'short unsigned int'} may
change the sign of the result [-Wsign-conversion]
  int16_t nb_rx = (*dev->rx_pkt_burst)(dev->data->
rx_queues[queue_id],
  ^
/projects/lagopus/src/dpdk/build/include/rte_ethdev.h:
3844:50: warning: conversion to 'uint16_t' {aka 'short
unsigned int'} from 'int16_t' {aka 'short int'} may
change the sign of the result [-Wsign-conversion]
nb_rx = cb->fn.rx(port_id, queue_id, rx_pkts, nb_rx,
  ^
/projects/lagopus/src/dpdk/build/include/rte_ethdev.h:
3844:12: warning: conversion to 'int16_t' {aka 'short
int'} from 'uint16_t' {aka 'short unsigned int'} may
change the sign of the result [-Wsign-conversion]
nb_rx = cb->fn.rx(port_id, queue_id, rx_pkts, nb_rx,
^~
/projects/lagopus/src/dpdk/build/include/rte_ethdev.h:
3851:9: warning: conversion to 'uint16_t' {aka 'short
unsigned int'} from 'int16_t' {aka 'short int'} may
change the sign of the result [-Wsign-conversion]
  return nb_rx;
 ^


Signed-off-by: Andy Green 
---
 lib/librte_ethdev/rte_ethdev.h |   25 +++--
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index f8815e994..7e7cfeee9 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -3823,6 +3823,7 @@ rte_eth_rx_burst(uint16_t port_id, uint16_t queue_id,
 struct rte_mbuf **rx_pkts, const uint16_t nb_pkts)
 {
struct rte_eth_dev *dev = &rte_eth_devices[port_id];
+   uint16_t nb_rx;
 
 #ifdef RTE_LIBRTE_ETHDEV_DEBUG
RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, 0);
@@ -3833,18 +3834,22 @@ rte_eth_rx_burst(uint16_t port_id, uint16_t queue_id,
return 0;
}
 #endif
-   int16_t nb_rx = (*dev->rx_pkt_burst)(dev->data->rx_queues[queue_id],
-   rx_pkts, nb_pkts);
+   nb_rx = (*dev->rx_pkt_burst)(dev->data->rx_queues[queue_id],
+rx_pkts, nb_pkts);
 
 #ifdef RTE_ETHDEV_RXTX_CALLBACKS
-   struct rte_eth_rxtx_callback *cb = dev->post_rx_burst_cbs[queue_id];
-
-   if (unlikely(cb != NULL)) {
-   do {
-   nb_rx = cb->fn.rx(port_id, queue_id, rx_pkts, nb_rx,
-   nb_pkts, cb->param);
-   cb = cb->next;
-   } while (cb != NULL);
+   {
+   struct rte_eth_rxtx_callback *cb =
+   dev->post_rx_burst_cbs[queue_id];
+
+   if (unlikely(cb != NULL)) {
+   do {
+   nb_rx = cb->fn.rx(port_id, queue_id,
+ rx_pkts, nb_rx,
+ nb_pkts, cb->param);
+   cb = cb->next;
+   } while (cb != NULL);
+   }
}
 #endif
 

[dpdk-dev] [PATCH v5 21/21] rte_byteorder.h: explicit cast for return promotion

[Andy Green]

/projects/lagopus/src/dpdk/build/include/generic/
rte_byteorder.h: In function 'rte_constant_bswap16':
/projects/lagopus/src/dpdk/build/include/generic/
rte_byteorder.h:54:45: warning: conversion from
'int' to 'uint16_t' {aka 'short unsigned int'}
may change value [-Wconversion]
  uint16_t)(v) & UINT16_C(0x00ff)) << 8) | \
  ~~~^~~
   (((uint16_t)(v) & UINT16_C(0xff00)) >> 8))
   ~~
/projects/lagopus/src/dpdk/build/include/generic/
rte_byteorder.h:126:9: note: in expansion of macro
'RTE_STATIC_BSWAP16'
  return RTE_STATIC_BSWAP16(x);
 ^~

The other two sizes are going to be afflicted the
same, so get the same fix.

Signed-off-by: Andy Green 
---
 .../common/include/generic/rte_byteorder.h |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/common/include/generic/rte_byteorder.h 
b/lib/librte_eal/common/include/generic/rte_byteorder.h
index 9bed85cca..7d9a1463c 100644
--- a/lib/librte_eal/common/include/generic/rte_byteorder.h
+++ b/lib/librte_eal/common/include/generic/rte_byteorder.h
@@ -123,7 +123,7 @@ typedef uint64_t rte_le64_t; /**< 64-bit little-endian 
value. */
 static inline uint16_t
 rte_constant_bswap16(uint16_t x)
 {
-   return RTE_STATIC_BSWAP16(x);
+   return (uint16_t)RTE_STATIC_BSWAP16(x);
 }
 
 /*
@@ -135,7 +135,7 @@ rte_constant_bswap16(uint16_t x)
 static inline uint32_t
 rte_constant_bswap32(uint32_t x)
 {
-   return RTE_STATIC_BSWAP32(x);
+   return (uint32_t)RTE_STATIC_BSWAP32(x);
 }
 
 /*
@@ -147,7 +147,7 @@ rte_constant_bswap32(uint32_t x)
 static inline uint64_t
 rte_constant_bswap64(uint64_t x)
 {
-   return RTE_STATIC_BSWAP64(x);
+   return (uint64_t)RTE_STATIC_BSWAP64(x);
 }
 
 

[dpdk-dev] [PATCH v3] doc: advise to specify LTS branch when backporting patches

[luca . boccassi]

From: Luca Boccassi 

We have many stable branches being maintained at the same time, and
sometimes it's not clear which branch a patch is being backported for.
Note in the guidelines that it should be specified via the cover letter,
annotation or using --subject-prefix.
Also note to send only to sta...@dpdk.org, not dev@dpdk.org.

Signed-off-by: Luca Boccassi 
---
v3: break long lines, fix subject-prefix argument to include PATH

v2: hint that --subject-prefix is required, fix typos

 doc/guides/contributing/patches.rst | 12 
 1 file changed, 12 insertions(+)

diff --git a/doc/guides/contributing/patches.rst 
b/doc/guides/contributing/patches.rst
index 2287835f9..c077e341a 100644
--- a/doc/guides/contributing/patches.rst
+++ b/doc/guides/contributing/patches.rst
@@ -450,6 +450,18 @@ Experienced committers may send patches directly with 
``git send-email`` without
 The options ``--annotate`` and ``confirm = always`` are recommended for 
checking patches before sending.
 
 
+Backporting patches for Stable Releases
+~~~
+
+Sometimes a maintainer or contributor wishes, or can be asked, to send a patch 
for a stable release
+rather than mainline. In this case the patch(es) should be sent to 
``sta...@dpdk.org``,
+not to ``dev@dpdk.org``.
+
+Given that there are multiple stable releases being maintained at the same 
time, please specify
+exactly which branch(es) the patch is for using ``git send-email 
--subject-prefix='PATCH 16.11' ...``
+and also optionally in the cover letter or in the annotation.
+
+
 The Review Process
 --
 
-- 
2.14.2

Re: [dpdk-dev] [PATCH] doc: advise to specify LTS branch when backporting patches

[Luca Boccassi]

On Thu, 2018-05-17 at 15:12 +0200, Thomas Monjalon wrote:
> 16/05/2018 19:03, Luca Boccassi:
> > On Wed, 2018-05-16 at 16:19 +0100, Kevin Traynor wrote:
> > > On 05/16/2018 03:31 PM, luca.bocca...@gmail.com wrote:
> > > > From: Luca Boccassi 
> > > > 
> > > > We have many stable branches being maintaned at the same time,
> > > > and
> > > 
> > > typo
> > > 
> > > > sometimes it's not clear which branch a patch is being
> > > > backported
> > > > for.
> > > > Note in the guidelines that it should be specified via the
> > > > cover
> > > > letter,
> > > > annotation or using --subject-prefix.
> > > > Also note to send only to sta...@dpdk.org, not dev@dpdk.org.
> > > > 
> > > > Signed-off-by: Luca Boccassi 
> > > > ---
> > > >  doc/guides/contributing/patches.rst | 8 
> > > >  1 file changed, 8 insertions(+)
> > > > 
> > > > diff --git a/doc/guides/contributing/patches.rst
> > > > b/doc/guides/contributing/patches.rst
> > > > index 2287835f9..1dc623a23 100644
> > > > --- a/doc/guides/contributing/patches.rst
> > > > +++ b/doc/guides/contributing/patches.rst
> > > > @@ -450,6 +450,14 @@ Experienced committers may send patches
> > > > directly with ``git send-email`` without
> > > >  The options ``--annotate`` and ``confirm = always`` are
> > > > recommended for checking patches before sending.
> > > >  
> > > >  
> > > > +Backporting patches for Stable Releases
> > > > +~~~
> > > > +
> > > > +Sometimes a maintainer or contributor wishes, or can be asked,
> > > > to
> > > > send a patch for a stable release rather than mainline. In this
> > > > case the patch(es) should be sent to ``sta...@dpdk.org``, not
> > > > to ``
> > > > dev@dpdk.org``.
> > > > +
> > > > +Given that there are multiple stable releases being maintained
> > > > at
> > > > the same time, please specify exactly which branch the patch is
> > > > for
> > > > in the cover letter, in the annotation or using ``git send-
> > > > email --
> > > > subject-prefix='16.11' ...``
> > > > +
> > > 
> > > s/branch/branch(es)/
> > > 
> > > How about [16.11] instead of '16.11' ?
> > 
> > Using --subject-prefix git will already use "[PATCH 16.11]
> 
> I think it should be --subject-prefix='PATCH 16.11'
> 
> Please wrap the lines after some punctuation signs to avoid really
> long lines
> in the rst file.

Ok, done both in v3.

-- 
Kind regards,
Luca Boccassi

Re: [dpdk-dev] [PATCH v4 15/23] rte_ethdev.h: align sign and scope of temp var

[Bruce Richardson]

On Mon, May 14, 2018 at 01:10:43PM +0800, Andy Green wrote:
> Signed-off-by: Andy Green 
> ---
>  lib/librte_ethdev/rte_ethdev.h |   25 +++--
>  1 file changed, 15 insertions(+), 10 deletions(-)
> 

While I dislike the changes below, since I believe it's always more
readable to declare variables at first use, if the changes are needed to
remove compiler errors in apps, then they need to be fixed.

Patch needs a suitable commit log explaining the changes or giving the
error message.


> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> index 49c2ebbd5..2cb5fe3be 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -3801,6 +3801,7 @@ rte_eth_rx_burst(uint16_t port_id, uint16_t queue_id,
>struct rte_mbuf **rx_pkts, const uint16_t nb_pkts)
>  {
>   struct rte_eth_dev *dev = &rte_eth_devices[port_id];
> + uint16_t nb_rx;
>  
>  #ifdef RTE_LIBRTE_ETHDEV_DEBUG
>   RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, 0);
> @@ -3811,18 +3812,22 @@ rte_eth_rx_burst(uint16_t port_id, uint16_t queue_id,
>   return 0;
>   }
>  #endif
> - int16_t nb_rx = (*dev->rx_pkt_burst)(dev->data->rx_queues[queue_id],
> - rx_pkts, nb_pkts);
> + nb_rx = (*dev->rx_pkt_burst)(dev->data->rx_queues[queue_id],
> +  rx_pkts, nb_pkts);
>  
>  #ifdef RTE_ETHDEV_RXTX_CALLBACKS
> - struct rte_eth_rxtx_callback *cb = dev->post_rx_burst_cbs[queue_id];
> -
> - if (unlikely(cb != NULL)) {
> - do {
> - nb_rx = cb->fn.rx(port_id, queue_id, rx_pkts, nb_rx,
> - nb_pkts, cb->param);
> - cb = cb->next;
> - } while (cb != NULL);
> + {
> + struct rte_eth_rxtx_callback *cb =
> + dev->post_rx_burst_cbs[queue_id];
> +
> + if (unlikely(cb != NULL)) {
> + do {
> + nb_rx = cb->fn.rx(port_id, queue_id,
> +   rx_pkts, nb_rx,
> +   nb_pkts, cb->param);
> + cb = cb->next;
> + } while (cb != NULL);
> + }
>   }
>  #endif

Rather than increasing the level of indentation needed with the extra
braces, it's probably best to separate variable definition and assignment
as you did in the first change above.

/Bruce

Re: [dpdk-dev] [PATCH v5 01/21] lib/librte_ethdev: change eth-dev-ops API to return int

[Bruce Richardson]

On Thu, May 17, 2018 at 09:48:57PM +0800, Andy Green wrote:
> Signed-off-by: Andy Green 
> ---

What's the reason for this change of type?

Re: [dpdk-dev] [PATCH v5 02/21] rte_string_fns.h: fix gcc8.1 sign conv warning in lstrcpy

[Bruce Richardson]

On Thu, May 17, 2018 at 09:49:02PM +0800, Andy Green wrote:
> In file included from ./dpdk/dpdk.c:88:
> /projects/lagopus/src/dpdk/build/include/rte_string_fns.h: In
> function 'rte_strlcpy':
> /projects/lagopus/src/dpdk/build/include/rte_string_fns.h:58:9:
> warning: conversion to 'size_t' {aka 'long unsigned int'} from
> 'int' may change the sign of the result [-Wsign-conversion]
>   return snprintf(dst, size, "%s", src);
>  ^~
> ---
>  lib/librte_eal/common/include/rte_string_fns.h |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
Acked-by: Bruce Richardson 

Re: [dpdk-dev] [PATCH] maintainers: handoff ownership of Tap PMD

[Ferruh Yigit]

On 5/17/2018 1:47 PM, Wiles, Keith wrote:
> 
> 
>> On May 17, 2018, at 2:44 AM, Pascal Mazon  wrote:
>>
>> I have unfortunately no longer time enough for maintaining Tap PMD.
>> Keith has kindly volunteered to take over maintainership. He's been at
>> the origin of this PMD and knows well how it works.
>>
>> Signed-off-by: Pascal Mazon 
>> Acked-by: Keith Wiles 

> Acked by: Keith Wiles

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

Thank you Pascal! Thank you Keith!

[dpdk-dev] [PATCH] rte_string_fns.h: fix gcc8.1 sign conv warning in lstrcpy

[Andy Green]

In file included from ./dpdk/dpdk.c:88:
/projects/lagopus/src/dpdk/build/include/rte_string_fns.h: In
function 'rte_strlcpy':
/projects/lagopus/src/dpdk/build/include/rte_string_fns.h:58:9:
warning: conversion to 'size_t' {aka 'long unsigned int'} from
'int' may change the sign of the result [-Wsign-conversion]
  return snprintf(dst, size, "%s", src);
 ^~

Signed-off-by: Andy Green 
---
 lib/librte_eal/common/include/rte_string_fns.h |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/include/rte_string_fns.h 
b/lib/librte_eal/common/include/rte_string_fns.h
index fcbb42e00..97597a148 100644
--- a/lib/librte_eal/common/include/rte_string_fns.h
+++ b/lib/librte_eal/common/include/rte_string_fns.h
@@ -55,7 +55,7 @@ rte_strsplit(char *string, int stringlen,
 static inline size_t
 rte_strlcpy(char *dst, const char *src, size_t size)
 {
-   return snprintf(dst, size, "%s", src);
+   return (size_t)snprintf(dst, size, "%s", src);
 }
 
 /* pull in a strlcpy function */

Re: [dpdk-dev] [PATCH v5 01/21] lib/librte_ethdev: change eth-dev-ops API to return int

[Andy Green]

On 05/17/2018 09:55 PM, Bruce Richardson wrote:

On Thu, May 17, 2018 at 09:48:57PM +0800, Andy Green wrote:

Signed-off-by: Andy Green 
---


What's the reason for this change of type?


I was asked to do it on the list... the original patch I sent was 
triggered by gcc8 warnings and just casted it away.  But the comment was 
that as an int, it could return negative error codes, so better to 
change them all.


It thereby became something in the way of a refactor rather than a gcc 8 
fix any more.


-Andy

Re: [dpdk-dev] [dpdk-stable] [PATCH v3] net/tap: fix isolation mode toggling

[Ferruh Yigit]

On 5/14/2018 11:46 PM, Wiles, Keith wrote:
> 
> 
>> On May 14, 2018, at 5:26 PM, Ophir Munk  wrote:
>>
>> Running testpmd command "flow isolae  0" (i.e. disabling flow
>> isolation) followed by command "flow isolate  1" (i.e. enabling
>> flow isolation) may result in a TAP error:
>> PMD: Kernel refused TC filter rule creation (17): File exists
>>
>> Root cause analysis: when disabling flow isolation we keep the local
>> rule to redirect packets on TX (TAP_REMOTE_TX index) while we add it
>> again when enabling flow isolation. As a result this rule is added
>> two times in a row which results in "File exists" error.
>> The fix is to identify the "File exists" error and silently ignore it.
>>
>> Another issue occurs when enabling isolation mode several times in a
>> row in which case the same tc rules are added consecutively and
>> rte_flow structs are added to a linked list before removing the
>> previous rte_flow structs.
>> The fix is to act upon isolation mode command only when there is a
>> change from "0" to "1" (or vice versa).
>>
>> Fixes: f503d2694825 ("net/tap: support flow API isolated mode")
>> Cc: sta...@dpdk.org
>>
>> Reviewed-by: Raslan Darawsheh 
>> Signed-off-by: Ophir Munk 
>> —
> 
> Acked by: Keith Wiles

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

Re: [dpdk-dev] [PATCH v4 15/23] rte_ethdev.h: align sign and scope of temp var

[Andy Green]

On 05/17/2018 09:54 PM, Bruce Richardson wrote:

On Mon, May 14, 2018 at 01:10:43PM +0800, Andy Green wrote:

Signed-off-by: Andy Green 
---
  lib/librte_ethdev/rte_ethdev.h |   25 +++--
  1 file changed, 15 insertions(+), 10 deletions(-)



While I dislike the changes below, since I believe it's always more
readable to declare variables at first use, if the changes are needed to
remove compiler errors in apps, then they need to be fixed.

Patch needs a suitable commit log explaining the changes or giving the
error message.


It has this in the last push (which overlapped with your comment)... I 
seem to have missed the error about declarations after code not being 
allowed in C90, but I guess that part being an issue is not controversial.





diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 49c2ebbd5..2cb5fe3be 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -3801,6 +3801,7 @@ rte_eth_rx_burst(uint16_t port_id, uint16_t queue_id,
 struct rte_mbuf **rx_pkts, const uint16_t nb_pkts)
  {
struct rte_eth_dev *dev = &rte_eth_devices[port_id];
+   uint16_t nb_rx;
  
  #ifdef RTE_LIBRTE_ETHDEV_DEBUG

RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, 0);
@@ -3811,18 +3812,22 @@ rte_eth_rx_burst(uint16_t port_id, uint16_t queue_id,
return 0;
}
  #endif
-   int16_t nb_rx = (*dev->rx_pkt_burst)(dev->data->rx_queues[queue_id],
-   rx_pkts, nb_pkts);
+   nb_rx = (*dev->rx_pkt_burst)(dev->data->rx_queues[queue_id],
+rx_pkts, nb_pkts);
  
  #ifdef RTE_ETHDEV_RXTX_CALLBACKS

-   struct rte_eth_rxtx_callback *cb = dev->post_rx_burst_cbs[queue_id];
-
-   if (unlikely(cb != NULL)) {
-   do {
-   nb_rx = cb->fn.rx(port_id, queue_id, rx_pkts, nb_rx,
-   nb_pkts, cb->param);
-   cb = cb->next;
-   } while (cb != NULL);
+   {
+   struct rte_eth_rxtx_callback *cb =
+   dev->post_rx_burst_cbs[queue_id];
+
+   if (unlikely(cb != NULL)) {
+   do {
+   nb_rx = cb->fn.rx(port_id, queue_id,
+ rx_pkts, nb_rx,
+ nb_pkts, cb->param);
+   cb = cb->next;
+   } while (cb != NULL);
+   }
}
  #endif


Rather than increasing the level of indentation needed with the extra
braces, it's probably best to separate variable definition and assignment
as you did in the first change above.


IIRC my thinking was I had a choice to repeat the conditional 
compilation stuff around the declaration (because it's in an #ifdef), 
(void)cb; to dodge the unused var warning, or add a basic block for it 
to scope to.  The last one didn't seem so bad.


-Andy


/Bruce

Re: [dpdk-dev] [PATCH v4 15/23] rte_ethdev.h: align sign and scope of temp var

[Bruce Richardson]

On Thu, May 17, 2018 at 10:17:04PM +0800, Andy Green wrote:
> 
> 
> On 05/17/2018 09:54 PM, Bruce Richardson wrote:
> > On Mon, May 14, 2018 at 01:10:43PM +0800, Andy Green wrote:
> > > Signed-off-by: Andy Green 
> > > ---
> > >   lib/librte_ethdev/rte_ethdev.h |   25 +++--
> > >   1 file changed, 15 insertions(+), 10 deletions(-)
> > > 
> > 
> > While I dislike the changes below, since I believe it's always more
> > readable to declare variables at first use, if the changes are needed to
> > remove compiler errors in apps, then they need to be fixed.
> > 
> > Patch needs a suitable commit log explaining the changes or giving the
> > error message.
> 
> It has this in the last push (which overlapped with your comment)... I seem
> to have missed the error about declarations after code not being allowed in
> C90, but I guess that part being an issue is not controversial.
> 
> > 
> > > diff --git a/lib/librte_ethdev/rte_ethdev.h 
> > > b/lib/librte_ethdev/rte_ethdev.h
> > > index 49c2ebbd5..2cb5fe3be 100644
> > > --- a/lib/librte_ethdev/rte_ethdev.h
> > > +++ b/lib/librte_ethdev/rte_ethdev.h
> > > @@ -3801,6 +3801,7 @@ rte_eth_rx_burst(uint16_t port_id, uint16_t 
> > > queue_id,
> > >struct rte_mbuf **rx_pkts, const uint16_t nb_pkts)
> > >   {
> > >   struct rte_eth_dev *dev = &rte_eth_devices[port_id];
> > > + uint16_t nb_rx;
> > >   #ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > >   RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, 0);
> > > @@ -3811,18 +3812,22 @@ rte_eth_rx_burst(uint16_t port_id, uint16_t 
> > > queue_id,
> > >   return 0;
> > >   }
> > >   #endif
> > > - int16_t nb_rx = (*dev->rx_pkt_burst)(dev->data->rx_queues[queue_id],
> > > - rx_pkts, nb_pkts);
> > > + nb_rx = (*dev->rx_pkt_burst)(dev->data->rx_queues[queue_id],
> > > +  rx_pkts, nb_pkts);
> > >   #ifdef RTE_ETHDEV_RXTX_CALLBACKS
> > > - struct rte_eth_rxtx_callback *cb = dev->post_rx_burst_cbs[queue_id];
> > > -
> > > - if (unlikely(cb != NULL)) {
> > > - do {
> > > - nb_rx = cb->fn.rx(port_id, queue_id, rx_pkts, nb_rx,
> > > - nb_pkts, cb->param);
> > > - cb = cb->next;
> > > - } while (cb != NULL);
> > > + {
> > > + struct rte_eth_rxtx_callback *cb =
> > > + dev->post_rx_burst_cbs[queue_id];
> > > +
> > > + if (unlikely(cb != NULL)) {
> > > + do {
> > > + nb_rx = cb->fn.rx(port_id, queue_id,
> > > +   rx_pkts, nb_rx,
> > > +   nb_pkts, cb->param);
> > > + cb = cb->next;
> > > + } while (cb != NULL);
> > > + }
> > >   }
> > >   #endif
> > 
> > Rather than increasing the level of indentation needed with the extra
> > braces, it's probably best to separate variable definition and assignment
> > as you did in the first change above.
> 
> IIRC my thinking was I had a choice to repeat the conditional compilation
> stuff around the declaration (because it's in an #ifdef), (void)cb; to dodge
> the unused var warning, or add a basic block for it to scope to.  The last
> one didn't seem so bad.
> 
Ok, that makes sense. For completeness, I'd add a "do" and "while(0)" to the
braces, but that's not important here, it's ok as-is.

BTW: for your v5, I don't think you kept the previous ack's that myself and
Olivier had given for a number of your patches. It's strongly recommended
that you put any previously-given acks in any new revisions after the
signoff line, so as to make it easier to track for maintainers, and to save
us having to re-ack the same patches multiple times.

Thanks,
/Bruce

Re: [dpdk-dev] [PATCH] app/testpmd: add to call detach for vdev when quitting

[Yang, Zhiyong]

Bernard,

Thanks for your review.

> -Original Message-
> From: Iremonger, Bernard
> Sent: Thursday, May 17, 2018 7:23 PM
> To: Yang, Zhiyong ; dev@dpdk.org
> Cc: maxime.coque...@redhat.com; Yigit, Ferruh ; Bie,
> Tiwei ; Yao, Lei A ; Yang, Zhiyong
> 
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: add to call detach for vdev when
> quitting
> 
> Hi Zhiyong,
> 
> > -Original Message-
> > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of
> > zhiyong.y...@intel.com
> > Sent: Thursday, May 17, 2018 2:00 PM
> > To: dev@dpdk.org
> > Cc: maxime.coque...@redhat.com; Yigit, Ferruh
> > ; Bie, Tiwei ; Yao, Lei A
> > ; Yang, Zhiyong 
> > Subject: [dpdk-dev] [PATCH] app/testpmd: add to call detach for vdev
> > when quitting
> >
> > For vdev, just calling rte_eth_dev_close() isn't enough to free all
> > the resources allocated during device probe, e.g. for virtio-user,
> > virtio_user_pmd_remove(), i.e. the remove() method of a vdev driver,
> > needs to be called to unlink the socket file created during device
> > probe. So this patch calls the rte_eth_dev_detach() for vdev when quitting
> testpmd.
> >
> > Cc: maxime.coque...@redhat.com
> > Cc: ferruh.yi...@intel.com
> > Cc: tiwei@intel.com
> > Cc: lei.a@intel.com
> >
> > Signed-off-by: Zhiyong Yang 
> > ---
> >  app/test-pmd/testpmd.c | 6 ++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > 134401603..1d308f056 100644
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -2011,6 +2011,8 @@ detach_port(portid_t port_id)  void
> >  pmd_test_exit(void)
> >  {
> > +   const struct rte_bus *bus;
> > +   struct rte_device *device;
> > portid_t pt_id;
> > int ret;
> >
> > @@ -2020,10 +2022,14 @@ pmd_test_exit(void)
> > if (ports != NULL) {
> > no_link_check = 1;
> > RTE_ETH_FOREACH_DEV(pt_id) {
> > +   device = rte_eth_devices[pt_id].device;
> > +   bus = rte_bus_find_by_device(device);
> > printf("\nShutting down port %d...\n", pt_id);
> > fflush(stdout);
> > stop_port(pt_id);
> > close_port(pt_id);
> > +   if (bus && !strcmp(bus->name, "vdev"))
> > +   detach_port(pt_id);
> > }
> > }
> >
> > --
> > 2.14.3
> 
> This appears to be a bug fix patch, if so it should have a fixes line.
> Also the commit line should include "fix", for example:
> "app/testpmd: fix pmd_test_exit function for vdevs"
> 

Ok, fix it in next version. 

Thanks
Zhiyong

Re: [dpdk-dev] [PATCH] igb_uio: fail and log if kernel lock down is enabled

[Stephen Hemminger]

On Thu, 17 May 2018 14:23:46 +0100
Ferruh Yigit  wrote:

> On 5/16/2018 12:47 PM, Neil Horman wrote:
> > On Tue, May 15, 2018 at 05:56:12PM +0100, Ferruh Yigit wrote:  
> >> When EFI secure boot is enabled, it is possible to lock down kernel and
> >> prevent accessing device BARs and this makes igb_uio unusable.
> >>
> >> Lock down patches are not part of the vanilla kernel but they are
> >> applied and used by some distros already [1].
> >>
> >> It is not possible to fix this issue, but intention of this patch is to
> >> detect and log if kernel lock down enabled and don't insert the module
> >> for that case.
> >>
> >> The challenge is since this feature enabled by distros, they have
> >> different config options and APIs for it. This patch is done based on
> >> Fedora and Ubuntu kernel source, may needs to add more distro specific
> >> support.
> >>
> >> [1]
> >> kernel.ubuntu.com/git/ubuntu/ubuntu-artful.git/commit/?id=99f9ef18d5b6
> >> And a few more patches to
> >>  
> > What exactly is the error you get when you load the igb_uio module?  I ask
> > because, looking at least at the Fedora patches, the BAR registers 
> > themselves
> > aren't made unwriteable, its only userspace access through very specific
> > channels that are gated on (things like /proc/bus/pci/...).  From what I 
> > can see
> > (again, not having looked at other implementations), kernel modules that 
> > load
> > successfully should be able to modify bar registers, and otherwise function
> > normally (as to weather they are permitted to load is another question).  
> 
> This patch is based on understanding on the effect of the lockdown patches, 
> that
> it will disable hardware access from userspace.
> I don't have an environment to test this and indeed I am not very clear about
> effects of the lockdown set.
> 
> > 
> > The reason I ask this is twofold:
> > 
> > 1) if a specific access is failing, that seems like it could be the trigger 
> > to
> > use, rather than explicitly checking if the kernel is locked down.  I don't 
> > see
> > one expressly called, but if you're calling pci_write_config_* somewhere, 
> > and
> > getting an EPERM error, thats a reason to fail the loading of igb_uio, 
> > based on
> > the fact that you don't have permission to write to the appropriate 
> > hardware.
> > 
> > 2) Its more than just the igb_uio module that will fail.  Any attempt to 
> > pass a
> > VF into a guest using user space tools (including the vfio scripts that dpdk
> > includes), should fail.  As such, it might be better to have some component 
> > in
> > user space test one of the aforementioned restricted paths for writeability.
> > Such an approach would be more generic, and eliminate the need to assemble 
> > a set
> > of tests to see if the kernel is locked down.  A more generic error message
> > could then be logged and the dpdk could exit gracefully, weather or not 
> > igb_uio
> > was loaded.  
> 
> With the existing patches, expectation is vfio will work but it will only 
> effect
> igb_uio.
> 
> > 
> > Its probably also important to note here that, this lockdown patch, from my
> > digging, has been carried in Fedora since December of 2016, and its still 
> > not
> > made it upstream.  Thats not to say that it will never do so, but it 
> > suggests
> > that, given the 2 years of out of tree updates its received, there its use 
> > is
> > both very specific and limted to users who understand its implications.  
> > This
> > probably isn't something to make significant or hard-to-maintain changes to 
> > the
> > dpdk (or any other software) over.  
> 
> Have same expectation that use will be specific and limited, that is why 
> planed
> to change only igb_uio to detect the case and return with a log, instead of
> updating anything in the dpdk.
> 
> in igb_uio the plan was just adding simple check, patches being not upstreamed
> added more complexity, but not still I believe it is not significant or
> hard-to-maintain change.

The  issue is that igb_uio is not secure since it allows userspace to setup
DMA to any physical address. In lockdown mode, even root is not supposed to be
able to peek and poke arbitrary memory.

Actually, it would make more sense to just have code to block all UIO drivers
in uio.c since uio_pci_generic has the same issue.