Re: [dpdk-dev] [PATCH v5 1/3] ethdev: add generic TTL rewrite actions

[Andrew Rybchenko]

On 13.10.2018 06:24, Jack Min wrote:

rewrite TTL by decrease or just set it directly
it's not necessary to check if the final result
is zero or not

This is slightly different from the one defined
by openflow and more generic

Signed-off-by: Xiaoyu Min 
Acked-by: Yongseok Koh 
---


[...]


diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index 68bbf57d0..f102e6939 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -1579,6 +1579,26 @@ enum rte_flow_action_type {
 * No associated configuration structure.
 */
RTE_FLOW_ACTION_TYPE_MAC_SWAP,
+
+   /**
+* Decrease TTL value directly
+*
+* If flow pattern doesn't define a valid RTE_FLOW_ITEM_TYPE_IPV4, or
+* RTE_FLOW_ITEM_TYPE_IPV6, the PMD should return a
+* RTE_FLOW_ERROR_TYPE_ACTION error.
+*/
+   RTE_FLOW_ACTION_TYPE_DEC_TTL,
+
+   /**
+* Set TTL value
+*
+* If flow pattern doesn't define a valid RTE_FLOW_ITEM_TYPE_IPV4, or
+* RTE_FLOW_ITEM_TYPE_IPV6, the PMD should return a
+* RTE_FLOW_ERROR_TYPE_ACTION error.
+*
+* See struct rte_flow_action_set_ttl
+*/
+   RTE_FLOW_ACTION_TYPE_SET_TTL,
  };
  
  /**


As I understand it is one more case where the following note from Adrien
is applicable [1]:



Another problem is that you must not require actions to rely on specific
pattern content:

 [...]
 *
 * Decapsulate outer most tunnel from matched flow.
 *
 * The flow pattern must have a valid tunnel header
 */
 RTE_FLOW_ACTION_TYPE_TUNNEL_DECAP,

For maximum flexibility, all actions should be usable on their own on empty
pattern. On the other hand, you can document undefined behavior when
performing some action on traffic that doesn't contain something.

Reason is that invalid traffic may have already been removed by other flow
rules (or whatever happens) before such a rule is reached; it's a user's
responsibility to provide such guarantee.

When parsing an action, a PMD is not supposed to look at the pattern. Action
list should contain all the needed info, otherwise it means the API is badly
defined.

I'm aware the above makes it tough to implement something like
RTE_FLOW_ACTION_TYPE_TUNNEL_DECAP as defined in this series, but that's
unfortunately why I think it must not be defined like that.



[1] https://mails.dpdk.org/archives/dev/2018-October/115267.html

Re: [dpdk-dev] [pull-request] next-pipeline 18.11 PRE-RC1

[Thomas Monjalon]

12/10/2018 13:07, Dumitrescu, Cristian:
> From: Thomas Monjalon [mailto:tho...@monjalon.net]
> > 01/10/2018 19:30, Cristian Dumitrescu:
> > >   http://dpdk.org/git/next/dpdk-next-pipeline
> > 
> > Please, could you fix the names in author fields and signed-off, etc?
> > Example: "Zhang, Roy Fan" should be "Fan Zhang
> > "
> > The idea is to keep the same exact name along the git history.
> > 
> > Thanks
> 
> Done, thanks Thomas.

Thanks for fixing compilation and nits.

next-pipeline is pulled in master.

Re: [dpdk-dev] [PATCH v2 1/2] net/mlx5: refactor TC-flow infrastructure

[Shahaf Shuler]

Hi Moty,

Thursday, October 11, 2018 4:05 PM, Mordechay Haimovsky:
> Subject: [PATCH v2 1/2] net/mlx5: refactor TC-flow infrastructure
> 
> This commit refactors tc_flow as a preparation to coming commits that sends
> different type of messages and expect differ type of replies while still using
> the same underlying routines.
> 
> Signed-off-by: Moti Haimovsky 
> ---
>  drivers/net/mlx5/mlx5.c  |  18 +++
>  drivers/net/mlx5/mlx5.h  |   4 +-
>  drivers/net/mlx5/mlx5_flow.h |   8 +--
>  drivers/net/mlx5/mlx5_flow_tcf.c | 113
> ++-
>  4 files changed, 102 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c index
> 7425e2f..20adf88 100644
> --- a/drivers/net/mlx5/mlx5.c
> +++ b/drivers/net/mlx5/mlx5.c
> @@ -286,8 +286,8 @@
>   close(priv->nl_socket_route);
>   if (priv->nl_socket_rdma >= 0)
>   close(priv->nl_socket_rdma);
> - if (priv->mnl_socket)
> - mlx5_flow_tcf_socket_destroy(priv->mnl_socket);
> + if (priv->tcf_context)
> + mlx5_flow_tcf_context_destroy(priv->tcf_context);
>   ret = mlx5_hrxq_ibv_verify(dev);
>   if (ret)
>   DRV_LOG(WARNING, "port %u some hash Rx queue still
> remain", @@ -1137,8 +1137,8 @@
>   claim_zero(mlx5_mac_addr_add(eth_dev, &mac, 0, 0));
>   if (vf && config.vf_nl_en)
>   mlx5_nl_mac_addr_sync(eth_dev);
> - priv->mnl_socket = mlx5_flow_tcf_socket_create();
> - if (!priv->mnl_socket) {
> + priv->tcf_context = mlx5_flow_tcf_context_create();
> + if (!priv->tcf_context) {
>   err = -rte_errno;
>   DRV_LOG(WARNING,
>   "flow rules relying on switch offloads will not be"
> @@ -1153,7 +1153,7 @@
>   error.message =
>   "cannot retrieve network interface index";
>   } else {
> - err = mlx5_flow_tcf_init(priv->mnl_socket, ifindex,
> + err = mlx5_flow_tcf_init(priv->tcf_context, ifindex,
>   &error);
>   }
>   if (err) {
> @@ -1161,8 +1161,8 @@
>   "flow rules relying on switch offloads will"
>   " not be supported: %s: %s",
>   error.message, strerror(rte_errno));
> - mlx5_flow_tcf_socket_destroy(priv->mnl_socket);
> - priv->mnl_socket = NULL;
> + mlx5_flow_tcf_context_destroy(priv->tcf_context);
> + priv->tcf_context = NULL;
>   }
>   }
>   TAILQ_INIT(&priv->flows);
> @@ -1217,8 +1217,8 @@
>   close(priv->nl_socket_route);
>   if (priv->nl_socket_rdma >= 0)
>   close(priv->nl_socket_rdma);
> - if (priv->mnl_socket)
> - mlx5_flow_tcf_socket_destroy(priv->mnl_socket);
> + if (priv->tcf_context)
> + mlx5_flow_tcf_context_destroy(priv->tcf_context);
>   if (own_domain_id)
>   claim_zero(rte_eth_switch_domain_free(priv-
> >domain_id));
>   rte_free(priv);
> diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index
> 2dec88a..d14239c 100644
> --- a/drivers/net/mlx5/mlx5.h
> +++ b/drivers/net/mlx5/mlx5.h
> @@ -169,7 +169,7 @@ struct mlx5_drop {
>   struct mlx5_rxq_ibv *rxq; /* Verbs Rx queue. */  };
> 
> -struct mnl_socket;
> +struct mlx5_flow_tcf_context;
> 
>  struct priv {
>   LIST_ENTRY(priv) mem_event_cb; /* Called by memory event
> callback. */ @@ -236,7 +236,7 @@ struct priv {
>   rte_spinlock_t uar_lock[MLX5_UAR_PAGE_NUM_MAX];
>   /* UAR same-page access control required in 32bit implementations.
> */  #endif
> - struct mnl_socket *mnl_socket; /* Libmnl socket. */
> + struct mlx5_flow_tcf_context *tcf_context; /* TC flower context. */
>  };
> 
>  #define PORT_ID(priv) ((priv)->dev_data->port_id) diff --git
> a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h index
> fee05f0..41d55e8 100644
> --- a/drivers/net/mlx5/mlx5_flow.h
> +++ b/drivers/net/mlx5/mlx5_flow.h
> @@ -338,9 +338,9 @@ int mlx5_flow_validate_item_vxlan_gpe(const struct
> rte_flow_item *item,
> 
>  /* mlx5_flow_tcf.c */
> 
> -int mlx5_flow_tcf_init(struct mnl_socket *nl, unsigned int ifindex,
> -struct rte_flow_error *error);
> -struct mnl_socket *mlx5_flow_tcf_socket_create(void);
> -void mlx5_flow_tcf_socket_destroy(struct mnl_socket *nl);
> +int mlx5_flow_tcf_init(struct mlx5_flow_tcf_context *ctx,
> +unsigned int ifindex, struct rte_flow_error *error); 
> struct
> +mlx5_flow_tcf_context *mlx5_flow_tcf_context_create(void);
> +void mlx5_flow_tcf_context_destroy(struct mlx5_flow_tcf_context *ctx);
> 
>  #endif /* RTE_PMD_MLX5_FLOW_H_ */
> diff --git a/drivers/net/mlx5/m

Re: [dpdk-dev] [PATCH v2 2/2] net/mlx5: support e-switch flow count action

[Shahaf Shuler]

Hi Moty, 

In addition to below, the prefix for each tc flow function should be 
flow_tcf_*. Make sure to update your functions. 

Thursday, October 11, 2018 4:05 PM, Mordechay Haimovsky:
> Subject: [PATCH v2 2/2] net/mlx5: support e-switch flow count action
> 
> This commit adds support for configuring flows destined to the mlx5 eswitch
> with 'count' action and for querying these counts at runtime.
> 

Log from here...

> It is possible to offload an interface flow rules to the hardware using DPDK
> flow commands.
> With mlx5 it is also possible to offload a limited set of flow rules to the 
> mlxsw
> (or e-switch) using the same DPDK flow commands using the 'transfer'
> attribute in the flow rule creation command.
> The commands destined for the switch are transposed to TC flower rules and
> are sent, as Netlink messages, to the mlx5 driver (or more precisely to the
> netdev which represent the mlxsw port).

Till here is not needed. 

> Each flow rule configured by the mlx5 driver is also assigned with a set of 
> flow

With TC, each flow rule ...

Also a set or a single counter?

> counters implicitly. These counters can be retrieved when querying the flow
> rule via Netlink, they can be found in each flow action section of the reply.

No need for log from here...

> Currently the limited set of eswitch flow rules does not contain the 'count'
> action but since every rule contains a count we can still retrieve these 
> values
> as if we configured a 'count' action.

Till here..

> 
> Supporting the 'count' action in the flow configuration command is straight-
> forward. 

Hence, supporting the 'count' action...


When transposing the command to a tc flower Netlink message we
> just ignore it instead of rejecting it.
> So the following two commands will have the same affect and behavior:
>   testpmd> flow create 0 transfer ingress pattern eth src is
>11:22:33:44:55:77 dst is 11:22:33:44:55:88 / end
>actions drop / end
>   testpmd> flow create 0 transfer ingress pattern eth src is
>11:22:33:44:55:77 dst is 11:22:33:44:55:88 / end
>actions count / drop / end

Right, but it is better the user will explicitly provide count action anyway. 

> In the flow query side, the command now also returns the counts the above

Rephrase "the counts the above", maybe the counters or the above counters. 

> flow via using tc Netlink query command.
> Special care was taken in order to prevent Netlink messages truncation due
> to short buffers by using MNL_SOCKET_BUFFER_SIZE buffers which are pre-
> allocate per port instance.

I don't understand this part. 
Isn't it a fixed size of response when querying the packet and bytes counters 
of a single flow? 
What is the size? 

> 
> Signed-off-by: Moti Haimovsky 
> ---
> v2:
>  * Rebase on top of 3f4722ee01e7 ("net/mlx5: refactor TC-flow
> infrastructure")
> ---
>  drivers/net/mlx5/mlx5_flow.c   |  29 +++-
>  drivers/net/mlx5/mlx5_flow.h   |  14 +-
>  drivers/net/mlx5/mlx5_flow_dv.c|   1 +
>  drivers/net/mlx5/mlx5_flow_tcf.c   | 286
> -
>  drivers/net/mlx5/mlx5_flow_verbs.c |   1 +
>  5 files changed, 325 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index 6b2698a..7c6ece1 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -1649,6 +1649,17 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,  {  }
> 
> +int
> +flow_null_query(struct rte_eth_dev *dev __rte_unused,
> + struct rte_flow *flow __rte_unused,
> + enum rte_flow_action_type type __rte_unused,
> + void *data __rte_unused,
> + struct rte_flow_error *error __rte_unused) {
> + rte_errno = ENOTSUP;
> + return -rte_errno;
> +}
> +
>  /* Void driver to protect from null pointer reference. */  const struct
> mlx5_flow_driver_ops mlx5_flow_null_drv_ops = {
>   .validate = flow_null_validate,
> @@ -1657,6 +1668,7 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
>   .apply = flow_null_apply,
>   .remove = flow_null_remove,
>   .destroy = flow_null_destroy,
> + .query = flow_null_query,
>  };
> 

You added the query function pointer to the driver ops which is good. 

>  /**
> @@ -2349,10 +2361,19 @@ struct rte_flow *
>   *   0 on success, a negative errno value otherwise and rte_errno is set.
>   */
>  static int
> -mlx5_flow_query_count(struct rte_flow *flow __rte_unused,
> -   void *data __rte_unused,
> +mlx5_flow_query_count(struct rte_eth_dev *dev,

Why not calling it flow_drv_query, and to have the function body just like the 
other function pointers:
Get the driver type from dev and then call its query op. 

You will probably need to move the current implementation to the query function 
of the verbs. 

> +   struct rte_flow *flow,
> +   void *data,

[dpdk-dev] [PATCH v5 1/7] net/mlx5: remove useless driver name comparison

[Thomas Monjalon]

The function mlx5_dev_to_port_id() is returning all the ports
associated to a rte_device.
It was comparing driver names while already comparing rte_device pointers.
If two devices are the same, they will have the same driver.
So the useless driver name comparison is removed.

Signed-off-by: Thomas Monjalon 
---
 drivers/net/mlx5/mlx5_ethdev.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index 61eda537b..cacdf8e18 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -1305,10 +1305,7 @@ mlx5_dev_to_port_id(const struct rte_device *dev, 
uint16_t *port_list,
RTE_ETH_FOREACH_DEV(id) {
struct rte_eth_dev *ldev = &rte_eth_devices[id];
 
-   if (!ldev->device ||
-   !ldev->device->driver ||
-   strcmp(ldev->device->driver->name, MLX5_DRIVER_NAME) ||
-   ldev->device != dev)
+   if (ldev->device != dev)
continue;
if (n < port_list_n)
port_list[n] = id;
-- 
2.19.0

[dpdk-dev] [PATCH v5 3/7] cryptodev: remove driver name from logs

[Thomas Monjalon]

The logs printed by CDEV_LOG_* were prefixed with the driver name.

In order to avoid assigning the driver before the end of the probing,
the driver name is removed from the cryptodev library logs.

Signed-off-by: Thomas Monjalon 
---
 lib/librte_cryptodev/rte_cryptodev_pmd.c | 23 ---
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/lib/librte_cryptodev/rte_cryptodev_pmd.c 
b/lib/librte_cryptodev/rte_cryptodev_pmd.c
index 2088ac3f3..f03bdbd5e 100644
--- a/lib/librte_cryptodev/rte_cryptodev_pmd.c
+++ b/lib/librte_cryptodev/rte_cryptodev_pmd.c
@@ -93,24 +93,20 @@ rte_cryptodev_pmd_create(const char *name,
struct rte_cryptodev *cryptodev;
 
if (params->name[0] != '\0') {
-   CDEV_LOG_INFO("[%s] User specified device name = %s\n",
-   device->driver->name, params->name);
+   CDEV_LOG_INFO("User specified device name = %s\n", 
params->name);
name = params->name;
}
 
-   CDEV_LOG_INFO("[%s] - Creating cryptodev %s\n",
-   device->driver->name, name);
+   CDEV_LOG_INFO("Creating cryptodev %s\n", name);
 
-   CDEV_LOG_INFO("[%s] - Initialisation parameters - name: %s,"
+   CDEV_LOG_INFO("Initialisation parameters - name: %s,"
"socket id: %d, max queue pairs: %u",
-   device->driver->name, name,
-   params->socket_id, params->max_nb_queue_pairs);
+   name, params->socket_id, params->max_nb_queue_pairs);
 
/* allocate device structure */
cryptodev = rte_cryptodev_pmd_allocate(name, params->socket_id);
if (cryptodev == NULL) {
-   CDEV_LOG_ERR("[%s] Failed to allocate crypto device for %s",
-   device->driver->name, name);
+   CDEV_LOG_ERR("Failed to allocate crypto device for %s", name);
return NULL;
}
 
@@ -123,9 +119,8 @@ rte_cryptodev_pmd_create(const char *name,
params->socket_id);
 
if (cryptodev->data->dev_private == NULL) {
-   CDEV_LOG_ERR("[%s] Cannot allocate memory for "
-   "cryptodev %s private data",
-   device->driver->name, name);
+   CDEV_LOG_ERR("Cannot allocate memory for cryptodev %s"
+   " private data", name);
 
rte_cryptodev_pmd_release_device(cryptodev);
return NULL;
@@ -145,9 +140,7 @@ rte_cryptodev_pmd_destroy(struct rte_cryptodev *cryptodev)
 {
int retval;
 
-   CDEV_LOG_INFO("[%s] Closing crypto device %s",
-   cryptodev->device->driver->name,
-   cryptodev->device->name);
+   CDEV_LOG_INFO("Closing crypto device %s", cryptodev->device->name);
 
/* free crypto device */
retval = rte_cryptodev_pmd_release_device(cryptodev);
-- 
2.19.0

[dpdk-dev] [PATCH v5 2/7] ethdev: rename memzones allocated for DMA

[Thomas Monjalon]

The helper rte_eth_dma_zone_reserve() is called by PMDs
when probing a new port.
It creates a new memzone with an unique name.
The name of this memzone was using the name of the driver
doing the probe.

In order to avoid assigning the driver before the end of the probing,
the driver name is removed from these memzone names.
The ethdev name (data->name) is not used because it may be too long
and may be not set at this stage of probing.

Syntax of old name: ___
Syntax of new name: eth_p_q_

Signed-off-by: Thomas Monjalon 
Tested-by: Andrew Rybchenko 
---
 app/test-pmd/config.c|  4 ++--
 drivers/net/bnx2x/bnx2x_rxtx.c   | 15 ++-
 drivers/net/cxgbe/sge.c  | 24 ++--
 drivers/net/vmxnet3/vmxnet3_ethdev.c |  4 ++--
 lib/librte_ethdev/rte_ethdev.c   |  5 ++---
 5 files changed, 18 insertions(+), 34 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index bf3cd0a8c..a6db5df0c 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1479,8 +1479,8 @@ ring_dma_zone_lookup(const char *ring_name, portid_t 
port_id, uint16_t q_id)
char mz_name[RTE_MEMZONE_NAMESIZE];
const struct rte_memzone *mz;
 
-   snprintf(mz_name, sizeof(mz_name), "%s_%s_%d_%d",
-ports[port_id].dev_info.driver_name, ring_name, port_id, q_id);
+   snprintf(mz_name, sizeof(mz_name), "eth_p%d_q%d_%s",
+   port_id, q_id, ring_name);
mz = rte_memzone_lookup(mz_name);
if (mz == NULL)
printf("%s ring memory zoneof (port %d, queue %d) not"
diff --git a/drivers/net/bnx2x/bnx2x_rxtx.c b/drivers/net/bnx2x/bnx2x_rxtx.c
index 589735ecd..ca28aaccf 100644
--- a/drivers/net/bnx2x/bnx2x_rxtx.c
+++ b/drivers/net/bnx2x/bnx2x_rxtx.c
@@ -12,19 +12,8 @@ static const struct rte_memzone *
 ring_dma_zone_reserve(struct rte_eth_dev *dev, const char *ring_name,
  uint16_t queue_id, uint32_t ring_size, int socket_id)
 {
-   char z_name[RTE_MEMZONE_NAMESIZE];
-   const struct rte_memzone *mz;
-
-   snprintf(z_name, sizeof(z_name), "%s_%s_%d_%d",
-   dev->device->driver->name, ring_name,
-   dev->data->port_id, queue_id);
-
-   mz = rte_memzone_lookup(z_name);
-   if (mz)
-   return mz;
-
-   return rte_memzone_reserve_aligned(z_name, ring_size, socket_id,
-   RTE_MEMZONE_IOVA_CONTIG, BNX2X_PAGE_SIZE);
+   return rte_eth_dma_zone_reserve(dev, ring_name, queue_id,
+   ring_size, BNX2X_PAGE_SIZE, socket_id);
 }
 
 static void
diff --git a/drivers/net/cxgbe/sge.c b/drivers/net/cxgbe/sge.c
index 4ea40d191..f9d2d48a0 100644
--- a/drivers/net/cxgbe/sge.c
+++ b/drivers/net/cxgbe/sge.c
@@ -1873,10 +1873,9 @@ int t4_sge_alloc_rxq(struct adapter *adap, struct 
sge_rspq *iq, bool fwevtq,
/* Size needs to be multiple of 16, including status entry. */
iq->size = cxgbe_roundup(iq->size, 16);
 
-   snprintf(z_name, sizeof(z_name), "%s_%s_%d_%d",
-eth_dev->device->driver->name,
-fwevtq ? "fwq_ring" : "rx_ring",
-eth_dev->data->port_id, queue_id);
+   snprintf(z_name, sizeof(z_name), "eth_p%d_q%d_%s",
+   eth_dev->data->port_id, queue_id,
+   fwevtq ? "fwq_ring" : "rx_ring");
snprintf(z_name_sw, sizeof(z_name_sw), "%s_sw_ring", z_name);
 
iq->desc = alloc_ring(iq->size, iq->iqe_len, 0, &iq->phys_addr, NULL, 0,
@@ -1938,10 +1937,9 @@ int t4_sge_alloc_rxq(struct adapter *adap, struct 
sge_rspq *iq, bool fwevtq,
fl->size = s->fl_starve_thres - 1 + 2 * 8;
fl->size = cxgbe_roundup(fl->size, 8);
 
-   snprintf(z_name, sizeof(z_name), "%s_%s_%d_%d",
-eth_dev->device->driver->name,
-fwevtq ? "fwq_ring" : "fl_ring",
-eth_dev->data->port_id, queue_id);
+   snprintf(z_name, sizeof(z_name), "eth_p%d_q%d_%s",
+   eth_dev->data->port_id, queue_id,
+   fwevtq ? "fwq_ring" : "fl_ring");
snprintf(z_name_sw, sizeof(z_name_sw), "%s_sw_ring", z_name);
 
fl->desc = alloc_ring(fl->size, sizeof(__be64),
@@ -2144,9 +2142,8 @@ int t4_sge_alloc_eth_txq(struct adapter *adap, struct 
sge_eth_txq *txq,
/* Add status entries */
nentries = txq->q.size + s->stat_len / sizeof(struct tx_desc);
 
-   snprintf(z_name, sizeof(z_name), "%s_%s_%d_%d",
-eth_dev->device->driver->name, "tx_ring",
-eth_dev->data->port_id, queue_id);
+   snprintf(z_name, sizeof(z_name), "eth_p%d_q%d_%s",
+   eth_dev->data->port_id, queue_id, "tx_ring");
snprintf(z_name_sw, sizeof(z_name_sw), "%s_sw_ring", z_name);
 
txq->q.desc = alloc_ring(txq->q.size, sizeof(struct tx_desc),
@@ -2223,9 +2220

[dpdk-dev] [PATCH v5 7/7] eal: allow probing a device again

[Thomas Monjalon]

In the devargs syntax for device representors, it is possible to add
several devices at once: -w dbdf,representor=[0-3]
It will become a more frequent case when introducing wildcards
and ranges in the new devargs syntax.

If a devargs string is provided for probing, and updated with a bigger
range for a new probing, then we do not want it to fail because
part of this range was already probed previously.
There can be new ports to create from an existing rte_device.

That's why the check for an already probed device
is moved as bus responsibility.
In the case of vdev, a global check is kept in insert_vdev(),
assuming that a vdev will always have only one port.
In the case of ifpga and vmbus, already probed devices are checked.
In the case of NXP buses, the probing is done only once (no hotplug),
though a check is added at bus level for consistency.
In the case of PCI, a driver flag is added to allow PMD probing again.
Only the PMD knows the ports attached to one rte_device.

As another consequence of being able to probe in several steps,
the field rte_device.devargs must not be considered as a full
representation of the rte_device, but only the latest probing args.
Anyway, the field rte_device.devargs is used only for probing.

Signed-off-by: Thomas Monjalon 
Reviewed-by: Andrew Rybchenko 
Tested-by: Andrew Rybchenko 
---
 drivers/bus/dpaa/dpaa_bus.c |  3 +++
 drivers/bus/fslmc/fslmc_bus.c   |  3 +++
 drivers/bus/ifpga/ifpga_bus.c   | 14 ++-
 drivers/bus/pci/pci_common.c| 33 -
 drivers/bus/pci/rte_bus_pci.h   |  4 ++-
 drivers/bus/vdev/vdev.c |  5 
 lib/librte_eal/common/eal_common_dev.c  |  7 ++
 lib/librte_eal/common/include/rte_dev.h |  2 +-
 8 files changed, 47 insertions(+), 24 deletions(-)

diff --git a/drivers/bus/dpaa/dpaa_bus.c b/drivers/bus/dpaa/dpaa_bus.c
index 138e0f98d..89d1e415d 100644
--- a/drivers/bus/dpaa/dpaa_bus.c
+++ b/drivers/bus/dpaa/dpaa_bus.c
@@ -555,6 +555,9 @@ rte_dpaa_bus_probe(void)
if (ret)
continue;
 
+   if (rte_dev_is_probed(&dev->device))
+   continue;
+
if (!drv->probe ||
(dev->device.devargs &&
dev->device.devargs->policy == RTE_DEV_BLACKLISTED))
diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
index 960f55071..7ebd980aa 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -386,6 +386,9 @@ rte_fslmc_probe(void)
if (!drv->probe)
continue;
 
+   if (rte_dev_is_probed(&dev->device))
+   continue;
+
if (dev->device.devargs &&
  dev->device.devargs->policy == RTE_DEV_BLACKLISTED) {
DPAA2_BUS_LOG(DEBUG, "%s Blacklisted, skipping",
diff --git a/drivers/bus/ifpga/ifpga_bus.c b/drivers/bus/ifpga/ifpga_bus.c
index 2ca1efa72..5f23ed8b4 100644
--- a/drivers/bus/ifpga/ifpga_bus.c
+++ b/drivers/bus/ifpga/ifpga_bus.c
@@ -301,8 +301,11 @@ ifpga_probe_all_drivers(struct rte_afu_device *afu_dev)
return -1;
 
/* Check if a driver is already loaded */
-   if (rte_dev_is_probed(&afu_dev->device))
-   return 0;
+   if (rte_dev_is_probed(&afu_dev->device)) {
+   IFPGA_BUS_DEBUG("Device %s is already probed\n",
+   rte_ifpga_device_name(afu_dev));
+   return -EEXIST;
+   }
 
TAILQ_FOREACH(drv, &ifpga_afu_drv_list, next) {
if (ifpga_probe_one_driver(drv, afu_dev)) {
@@ -325,14 +328,13 @@ ifpga_probe(void)
int ret = 0;
 
TAILQ_FOREACH(afu_dev, &ifpga_afu_dev_list, next) {
-   if (rte_dev_is_probed(&afu_dev->device))
-   continue;
-
ret = ifpga_probe_all_drivers(afu_dev);
+   if (ret == -EEXIST)
+   continue;
if (ret < 0)
IFPGA_BUS_ERR("failed to initialize %s device\n",
rte_ifpga_device_name(afu_dev));
-   }
+   }
 
return ret;
 }
diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
index 62b17fba9..11c5da587 100644
--- a/drivers/bus/pci/pci_common.c
+++ b/drivers/bus/pci/pci_common.c
@@ -6,6 +6,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -121,6 +122,7 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr,
 struct rte_pci_device *dev)
 {
int ret;
+   bool already_probed;
struct rte_pci_addr *loc;
 
if ((dr == NULL) || (dev == NULL))
@@ -151,6 +153,13 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr,
dev->device.numa_node = 0;
}
 
+  

[dpdk-dev] [PATCH v5 6/7] eal: add function to query device status

[Thomas Monjalon]

The function rte_dev_is_probed() is added in order to improve semantic
and enforce proper check of the probing status of a device.

It will answer this rte_device query:
Is it already successfully probed or not?

Signed-off-by: Thomas Monjalon 
Reviewed-by: Andrew Rybchenko 
Tested-by: Andrew Rybchenko 
---
 drivers/bus/ifpga/ifpga_bus.c   |  4 ++--
 drivers/bus/pci/pci_common.c|  2 +-
 drivers/bus/vdev/vdev.c |  2 +-
 drivers/bus/vmbus/vmbus_common.c|  2 +-
 lib/librte_eal/common/eal_common_dev.c  |  9 -
 lib/librte_eal/common/include/rte_dev.h | 14 ++
 lib/librte_eal/rte_eal_version.map  |  1 +
 7 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/drivers/bus/ifpga/ifpga_bus.c b/drivers/bus/ifpga/ifpga_bus.c
index fca2dbace..2ca1efa72 100644
--- a/drivers/bus/ifpga/ifpga_bus.c
+++ b/drivers/bus/ifpga/ifpga_bus.c
@@ -301,7 +301,7 @@ ifpga_probe_all_drivers(struct rte_afu_device *afu_dev)
return -1;
 
/* Check if a driver is already loaded */
-   if (afu_dev->device.driver != NULL)
+   if (rte_dev_is_probed(&afu_dev->device))
return 0;
 
TAILQ_FOREACH(drv, &ifpga_afu_drv_list, next) {
@@ -325,7 +325,7 @@ ifpga_probe(void)
int ret = 0;
 
TAILQ_FOREACH(afu_dev, &ifpga_afu_dev_list, next) {
-   if (afu_dev->device.driver)
+   if (rte_dev_is_probed(&afu_dev->device))
continue;
 
ret = ifpga_probe_all_drivers(afu_dev);
diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
index d63e68045..62b17fba9 100644
--- a/drivers/bus/pci/pci_common.c
+++ b/drivers/bus/pci/pci_common.c
@@ -243,7 +243,7 @@ pci_probe_all_drivers(struct rte_pci_device *dev)
return -1;
 
/* Check if a driver is already loaded */
-   if (dev->device.driver != NULL)
+   if (rte_dev_is_probed(&dev->device))
return 0;
 
FOREACH_DRIVER_ON_PCIBUS(dr) {
diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index 3f27f3510..f666099e6 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -481,7 +481,7 @@ vdev_probe(void)
 * we call each driver probe.
 */
 
-   if (dev->device.driver)
+   if (rte_dev_is_probed(&dev->device))
continue;
 
if (vdev_probe_all_drivers(dev)) {
diff --git a/drivers/bus/vmbus/vmbus_common.c b/drivers/bus/vmbus/vmbus_common.c
index de5548aa4..48a219f73 100644
--- a/drivers/bus/vmbus/vmbus_common.c
+++ b/drivers/bus/vmbus/vmbus_common.c
@@ -143,7 +143,7 @@ vmbus_probe_all_drivers(struct rte_vmbus_device *dev)
int rc;
 
/* Check if a driver is already loaded */
-   if (dev->device.driver != NULL) {
+   if (rte_dev_is_probed(&dev->device)) {
VMBUS_LOG(DEBUG, "VMBUS driver already loaded");
return 0;
}
diff --git a/lib/librte_eal/common/eal_common_dev.c 
b/lib/librte_eal/common/eal_common_dev.c
index 7663eaa3f..7e8a9b260 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -74,6 +74,13 @@ static int cmp_dev_name(const struct rte_device *dev, const 
void *_name)
return strcmp(dev->name, name);
 }
 
+int __rte_experimental
+rte_dev_is_probed(const struct rte_device *dev)
+{
+   /* The field driver should be set only when the probe is successful. */
+   return dev->driver != NULL;
+}
+
 int rte_eal_dev_attach(const char *name, const char *devargs)
 {
struct rte_bus *bus;
@@ -189,7 +196,7 @@ rte_dev_probe(const char *devargs)
goto err_devarg;
}
 
-   if (dev->driver != NULL) {
+   if (rte_dev_is_probed(dev)) {
RTE_LOG(ERR, EAL, "Device is already plugged\n");
return -EEXIST;
}
diff --git a/lib/librte_eal/common/include/rte_dev.h 
b/lib/librte_eal/common/include/rte_dev.h
index 5084c645b..9f169e3b3 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -162,6 +162,20 @@ struct rte_device {
struct rte_devargs *devargs;  /**< Device user arguments */
 };
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Query status of a device.
+ *
+ * @param dev
+ *   Generic device pointer.
+ * @return
+ *   (int)true if already probed successfully, 0 otherwise.
+ */
+__rte_experimental
+int rte_dev_is_probed(const struct rte_device *dev);
+
 /**
  * Attach a device to a registered driver.
  *
diff --git a/lib/librte_eal/rte_eal_version.map 
b/lib/librte_eal/rte_eal_version.map
index e968edc2e..e628c3930 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -279,6 +279,7 @@ EXPERIMENTAL {
rte_dev_event_callback_unregister;
rte_dev_event_monitor_start;
rte_dev_event_monitor_stop;
+   rte_de

[dpdk-dev] [PATCH v5 5/7] drivers/bus: move driver assignment to end of probing

[Thomas Monjalon]

The PCI mapping requires to know the PCI driver to use,
even before the probing is done. That's why the PCI driver is
referenced early inside the PCI device structure. See
commit 1d20a073fa5e ("bus/pci: reference driver structure before mapping")

However the rte_driver does not need to be referenced in rte_device
before the device probing is done.
By moving back this assignment at the end of the device probing,
it becomes possible to make clear the status of a rte_device.

Signed-off-by: Thomas Monjalon 
Reviewed-by: Andrew Rybchenko 
Tested-by: Andrew Rybchenko 
---
 drivers/bus/ifpga/ifpga_bus.c   | 9 -
 drivers/bus/pci/pci_common.c| 7 +++
 drivers/bus/pci/rte_bus_pci.h   | 2 +-
 drivers/bus/vdev/vdev.c | 5 ++---
 drivers/bus/vmbus/vmbus_common.c| 5 +++--
 drivers/net/i40e/i40e_vf_representor.c  | 3 ---
 drivers/net/mlx4/mlx4.c | 1 -
 drivers/net/mlx5/mlx5.c | 1 -
 drivers/net/szedata2/rte_eth_szedata2.c | 6 +++---
 lib/librte_eal/common/include/rte_dev.h | 2 +-
 10 files changed, 17 insertions(+), 24 deletions(-)

diff --git a/drivers/bus/ifpga/ifpga_bus.c b/drivers/bus/ifpga/ifpga_bus.c
index 80663328a..fca2dbace 100644
--- a/drivers/bus/ifpga/ifpga_bus.c
+++ b/drivers/bus/ifpga/ifpga_bus.c
@@ -280,14 +280,13 @@ ifpga_probe_one_driver(struct rte_afu_driver *drv,
 
/* reference driver structure */
afu_dev->driver = drv;
-   afu_dev->device.driver = &drv->driver;
 
/* call the driver probe() function */
ret = drv->probe(afu_dev);
-   if (ret) {
+   if (ret)
afu_dev->driver = NULL;
-   afu_dev->device.driver = NULL;
-   }
+   else
+   afu_dev->device.driver = &drv->driver;
 
return ret;
 }
@@ -302,7 +301,7 @@ ifpga_probe_all_drivers(struct rte_afu_device *afu_dev)
return -1;
 
/* Check if a driver is already loaded */
-   if (afu_dev->driver != NULL)
+   if (afu_dev->device.driver != NULL)
return 0;
 
TAILQ_FOREACH(drv, &ifpga_afu_drv_list, next) {
diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
index c7695d108..d63e68045 100644
--- a/drivers/bus/pci/pci_common.c
+++ b/drivers/bus/pci/pci_common.c
@@ -160,14 +160,12 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr,
 * driver flags for adjusting configuration.
 */
dev->driver = dr;
-   dev->device.driver = &dr->driver;
 
if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
/* map resources for devices that use igb_uio */
ret = rte_pci_map_device(dev);
if (ret != 0) {
dev->driver = NULL;
-   dev->device.driver = NULL;
return ret;
}
}
@@ -176,7 +174,6 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr,
ret = dr->probe(dr, dev);
if (ret) {
dev->driver = NULL;
-   dev->device.driver = NULL;
if ((dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) &&
/* Don't unmap if device is unsupported and
 * driver needs mapped resources.
@@ -184,6 +181,8 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr,
!(ret > 0 &&
(dr->drv_flags & RTE_PCI_DRV_KEEP_MAPPED_RES)))
rte_pci_unmap_device(dev);
+   } else {
+   dev->device.driver = &dr->driver;
}
 
return ret;
@@ -244,7 +243,7 @@ pci_probe_all_drivers(struct rte_pci_device *dev)
return -1;
 
/* Check if a driver is already loaded */
-   if (dev->driver != NULL)
+   if (dev->device.driver != NULL)
return 0;
 
FOREACH_DRIVER_ON_PCIBUS(dr) {
diff --git a/drivers/bus/pci/rte_bus_pci.h b/drivers/bus/pci/rte_bus_pci.h
index 0d1955ffe..984df2b37 100644
--- a/drivers/bus/pci/rte_bus_pci.h
+++ b/drivers/bus/pci/rte_bus_pci.h
@@ -62,7 +62,7 @@ struct rte_pci_device {
struct rte_mem_resource mem_resource[PCI_MAX_RESOURCE];
/**< PCI Memory Resource */
struct rte_intr_handle intr_handle; /**< Interrupt handle */
-   struct rte_pci_driver *driver;  /**< Associated driver */
+   struct rte_pci_driver *driver;  /**< PCI driver used in probing */
uint16_t max_vfs;   /**< sriov enable if not zero */
enum rte_kernel_driver kdrv;/**< Kernel driver passthrough */
char name[PCI_PRI_STR_SIZE+1];  /**< PCI location (ASCII) */
diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index 0142fb2c8..3f27f3510 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -150,10 +150,9 @@ vdev_probe_all_drivers(struct rte_vdev_device *dev)
 
if (vdev_parse(name, &driver))
ret

[dpdk-dev] [PATCH v5 4/7] compressdev: remove driver name from logs

[Thomas Monjalon]

The logs printed by COMPRESSDEV_LOG were prefixed with the driver name.

In order to avoid assigning the driver before the end of the probing,
the driver name is removed from the compressdev library logs.

Signed-off-by: Thomas Monjalon 
---
 lib/librte_compressdev/rte_compressdev_pmd.c | 23 
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/lib/librte_compressdev/rte_compressdev_pmd.c 
b/lib/librte_compressdev/rte_compressdev_pmd.c
index 7de4f339e..95beb26ab 100644
--- a/lib/librte_compressdev/rte_compressdev_pmd.c
+++ b/lib/librte_compressdev/rte_compressdev_pmd.c
@@ -92,24 +92,20 @@ rte_compressdev_pmd_create(const char *name,
struct rte_compressdev *compressdev;
 
if (params->name[0] != '\0') {
-   COMPRESSDEV_LOG(INFO, "[%s] User specified device name = %s\n",
-   device->driver->name, params->name);
+   COMPRESSDEV_LOG(INFO, "User specified device name = %s\n",
+   params->name);
name = params->name;
}
 
-   COMPRESSDEV_LOG(INFO, "[%s] - Creating compressdev %s\n",
-   device->driver->name, name);
+   COMPRESSDEV_LOG(INFO, "Creating compressdev %s\n", name);
 
-   COMPRESSDEV_LOG(INFO,
-   "[%s] - Init parameters - name: %s, socket id: %d",
-   device->driver->name, name,
-   params->socket_id);
+   COMPRESSDEV_LOG(INFO, "Init parameters - name: %s, socket id: %d",
+   name, params->socket_id);
 
/* allocate device structure */
compressdev = rte_compressdev_pmd_allocate(name, params->socket_id);
if (compressdev == NULL) {
-   COMPRESSDEV_LOG(ERR, "[%s] Failed to allocate comp device %s",
-   device->driver->name, name);
+   COMPRESSDEV_LOG(ERR, "Failed to allocate comp device %s", name);
return NULL;
}
 
@@ -123,8 +119,8 @@ rte_compressdev_pmd_create(const char *name,
 
if (compressdev->data->dev_private == NULL) {
COMPRESSDEV_LOG(ERR,
-   "[%s] Cannot allocate memory for compressdev %s private data",
-   device->driver->name, name);
+   "Cannot allocate memory for compressdev"
+   " %s private data", name);
 
rte_compressdev_pmd_release_device(compressdev);
return NULL;
@@ -141,8 +137,7 @@ rte_compressdev_pmd_destroy(struct rte_compressdev 
*compressdev)
 {
int retval;
 
-   COMPRESSDEV_LOG(INFO, "[%s] Closing comp device %s",
-   compressdev->device->driver->name,
+   COMPRESSDEV_LOG(INFO, "Closing comp device %s",
compressdev->device->name);
 
/* free comp device */
-- 
2.19.0

[dpdk-dev] [PATCH v5 0/7] allow hotplug to skip an already probed device

[Thomas Monjalon]

This is a follow-up of an idea presented at Dublin
during the "hotplug talk".

The idea is to ease probing of range of ports attached
to the same rte_device.
I becomes possible to allow probing again the same device
but with a bigger range of ports in the devargs.

Instead of adding a parameter to the hotplug functions, as in the RFC,
the check of an already probed device is moved to the bus and enabled PMDs.
It gives flexibility to drivers for managing range of ports.


Changes in v5 - after Andrew's review:
  - remove driver name usage in mlx5/szedata2 probing and crypto/compress logs
  - update DMA ring names in bnx2x, cxgbe, vmxnet3 and testpmd lookup

Changes in v4 - after Andrew's review:
  - remove access to rte_device.driver during probing (patch 1)

Changes in v3 - after Declan's review:
  - return error in all buses but PCI
  - add a PCI driver flag to enable feature per driver
  - return -EEXIST if probing again is not supported


Thomas Monjalon (7):
  net/mlx5: remove useless driver name comparison
  ethdev: rename memzones allocated for DMA
  cryptodev: remove driver name from logs
  compressdev: remove driver name from logs
  drivers/bus: move driver assignment to end of probing
  eal: add function to query device status
  eal: allow probing a device again

 app/test-pmd/config.c|  4 +--
 drivers/bus/dpaa/dpaa_bus.c  |  3 ++
 drivers/bus/fslmc/fslmc_bus.c|  3 ++
 drivers/bus/ifpga/ifpga_bus.c| 21 +--
 drivers/bus/pci/pci_common.c | 38 
 drivers/bus/pci/rte_bus_pci.h|  6 ++--
 drivers/bus/vdev/vdev.c  | 12 ---
 drivers/bus/vmbus/vmbus_common.c |  5 +--
 drivers/net/bnx2x/bnx2x_rxtx.c   | 15 ++--
 drivers/net/cxgbe/sge.c  | 24 ++---
 drivers/net/i40e/i40e_vf_representor.c   |  3 --
 drivers/net/mlx4/mlx4.c  |  1 -
 drivers/net/mlx5/mlx5.c  |  1 -
 drivers/net/mlx5/mlx5_ethdev.c   |  5 +--
 drivers/net/szedata2/rte_eth_szedata2.c  |  6 ++--
 drivers/net/vmxnet3/vmxnet3_ethdev.c |  4 +--
 lib/librte_compressdev/rte_compressdev_pmd.c | 23 +---
 lib/librte_cryptodev/rte_cryptodev_pmd.c | 23 +---
 lib/librte_eal/common/eal_common_dev.c   | 14 +---
 lib/librte_eal/common/include/rte_dev.h  | 18 --
 lib/librte_eal/rte_eal_version.map   |  1 +
 lib/librte_ethdev/rte_ethdev.c   |  5 ++-
 22 files changed, 121 insertions(+), 114 deletions(-)

-- 
2.19.0

Re: [dpdk-dev] [PATCH v5 1/7] net/mlx5: remove useless driver name comparison

[Thomas Monjalon]

+Cc Yongseok and Shahaf

14/10/2018 22:47, Thomas Monjalon:
> The function mlx5_dev_to_port_id() is returning all the ports
> associated to a rte_device.
> It was comparing driver names while already comparing rte_device pointers.
> If two devices are the same, they will have the same driver.
> So the useless driver name comparison is removed.
> 
> Signed-off-by: Thomas Monjalon 
> ---
>  drivers/net/mlx5/mlx5_ethdev.c | 5 +
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
> index 61eda537b..cacdf8e18 100644
> --- a/drivers/net/mlx5/mlx5_ethdev.c
> +++ b/drivers/net/mlx5/mlx5_ethdev.c
> @@ -1305,10 +1305,7 @@ mlx5_dev_to_port_id(const struct rte_device *dev, 
> uint16_t *port_list,
>   RTE_ETH_FOREACH_DEV(id) {
>   struct rte_eth_dev *ldev = &rte_eth_devices[id];
>  
> - if (!ldev->device ||
> - !ldev->device->driver ||
> - strcmp(ldev->device->driver->name, MLX5_DRIVER_NAME) ||
> - ldev->device != dev)
> + if (ldev->device != dev)
>   continue;
>   if (n < port_list_n)
>   port_list[n] = id;
> 

Re: [dpdk-dev] [PATCH v5 5/7] drivers/bus: move driver assignment to end of probing

[Thomas Monjalon]

+Cc vmbus, ifpga, i40e, mlx and szedata2 maintainers

14/10/2018 22:47, Thomas Monjalon:
> The PCI mapping requires to know the PCI driver to use,
> even before the probing is done. That's why the PCI driver is
> referenced early inside the PCI device structure. See
> commit 1d20a073fa5e ("bus/pci: reference driver structure before mapping")
> 
> However the rte_driver does not need to be referenced in rte_device
> before the device probing is done.
> By moving back this assignment at the end of the device probing,
> it becomes possible to make clear the status of a rte_device.
> 
> Signed-off-by: Thomas Monjalon 
> Reviewed-by: Andrew Rybchenko 
> Tested-by: Andrew Rybchenko 
> ---
>  drivers/bus/ifpga/ifpga_bus.c   | 9 -
>  drivers/bus/pci/pci_common.c| 7 +++
>  drivers/bus/pci/rte_bus_pci.h   | 2 +-
>  drivers/bus/vdev/vdev.c | 5 ++---
>  drivers/bus/vmbus/vmbus_common.c| 5 +++--
>  drivers/net/i40e/i40e_vf_representor.c  | 3 ---
>  drivers/net/mlx4/mlx4.c | 1 -
>  drivers/net/mlx5/mlx5.c | 1 -
>  drivers/net/szedata2/rte_eth_szedata2.c | 6 +++---
>  lib/librte_eal/common/include/rte_dev.h | 2 +-
>  10 files changed, 17 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/bus/ifpga/ifpga_bus.c b/drivers/bus/ifpga/ifpga_bus.c
> index 80663328a..fca2dbace 100644
> --- a/drivers/bus/ifpga/ifpga_bus.c
> +++ b/drivers/bus/ifpga/ifpga_bus.c
> @@ -280,14 +280,13 @@ ifpga_probe_one_driver(struct rte_afu_driver *drv,
>  
>   /* reference driver structure */
>   afu_dev->driver = drv;
> - afu_dev->device.driver = &drv->driver;
>  
>   /* call the driver probe() function */
>   ret = drv->probe(afu_dev);
> - if (ret) {
> + if (ret)
>   afu_dev->driver = NULL;
> - afu_dev->device.driver = NULL;
> - }
> + else
> + afu_dev->device.driver = &drv->driver;
>  
>   return ret;
>  }
> @@ -302,7 +301,7 @@ ifpga_probe_all_drivers(struct rte_afu_device *afu_dev)
>   return -1;
>  
>   /* Check if a driver is already loaded */
> - if (afu_dev->driver != NULL)
> + if (afu_dev->device.driver != NULL)
>   return 0;
>  
>   TAILQ_FOREACH(drv, &ifpga_afu_drv_list, next) {
> diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
> index c7695d108..d63e68045 100644
> --- a/drivers/bus/pci/pci_common.c
> +++ b/drivers/bus/pci/pci_common.c
> @@ -160,14 +160,12 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr,
>* driver flags for adjusting configuration.
>*/
>   dev->driver = dr;
> - dev->device.driver = &dr->driver;
>  
>   if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
>   /* map resources for devices that use igb_uio */
>   ret = rte_pci_map_device(dev);
>   if (ret != 0) {
>   dev->driver = NULL;
> - dev->device.driver = NULL;
>   return ret;
>   }
>   }
> @@ -176,7 +174,6 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr,
>   ret = dr->probe(dr, dev);
>   if (ret) {
>   dev->driver = NULL;
> - dev->device.driver = NULL;
>   if ((dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) &&
>   /* Don't unmap if device is unsupported and
>* driver needs mapped resources.
> @@ -184,6 +181,8 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr,
>   !(ret > 0 &&
>   (dr->drv_flags & RTE_PCI_DRV_KEEP_MAPPED_RES)))
>   rte_pci_unmap_device(dev);
> + } else {
> + dev->device.driver = &dr->driver;
>   }
>  
>   return ret;
> @@ -244,7 +243,7 @@ pci_probe_all_drivers(struct rte_pci_device *dev)
>   return -1;
>  
>   /* Check if a driver is already loaded */
> - if (dev->driver != NULL)
> + if (dev->device.driver != NULL)
>   return 0;
>  
>   FOREACH_DRIVER_ON_PCIBUS(dr) {
> diff --git a/drivers/bus/pci/rte_bus_pci.h b/drivers/bus/pci/rte_bus_pci.h
> index 0d1955ffe..984df2b37 100644
> --- a/drivers/bus/pci/rte_bus_pci.h
> +++ b/drivers/bus/pci/rte_bus_pci.h
> @@ -62,7 +62,7 @@ struct rte_pci_device {
>   struct rte_mem_resource mem_resource[PCI_MAX_RESOURCE];
>   /**< PCI Memory Resource */
>   struct rte_intr_handle intr_handle; /**< Interrupt handle */
> - struct rte_pci_driver *driver;  /**< Associated driver */
> + struct rte_pci_driver *driver;  /**< PCI driver used in probing */
>   uint16_t max_vfs;   /**< sriov enable if not zero */
>   enum rte_kernel_driver kdrv;/**< Kernel driver passthrough */
>   char name[PCI_PRI_STR_SIZE+1];  /**< PCI location (ASCII) */
> diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
> index 0142fb2c8..3f27f35

[dpdk-dev] [PATCH v3 0/2] ethdev port freeing

[Thomas Monjalon]

The function rte_eth_dev_detach() is freeing a port and its underlying
rte_device object. The issue is that we may have several ports
associated to the same rte_device.

The right replacement is to free the port, and free the rte_device
if no more ports.
At ethdev level, the public function for port freeing is rte_eth_dev_close().
The only miss is rte_eth_dev_release_port() to free all port resources.


Changes in v3:
  - free queues, MAC addresses and private structure
in rte_eth_dev_release_port.


Thomas Monjalon (2):
  ethdev: free all common data when releasing port
  ethdev: complete closing of port

 drivers/net/af_packet/rte_eth_af_packet.c |  2 --
 drivers/net/ark/ark_ethdev.c  |  1 -
 drivers/net/avf/avf_ethdev.c  |  3 ---
 drivers/net/avp/avp_ethdev.c  |  5 
 drivers/net/axgbe/axgbe_ethdev.c  |  3 ---
 drivers/net/bnxt/bnxt_ethdev.c|  8 --
 drivers/net/bonding/rte_eth_bond_pmd.c|  7 +
 drivers/net/cxgbe/cxgbe_main.c| 16 ++--
 drivers/net/cxgbe/cxgbevf_main.c  |  9 +--
 drivers/net/dpaa/dpaa_ethdev.c| 10 ---
 drivers/net/dpaa2/dpaa2_ethdev.c  | 10 ---
 drivers/net/e1000/em_ethdev.c |  3 ---
 drivers/net/e1000/igb_ethdev.c|  6 -
 drivers/net/enetc/enetc_ethdev.c  |  4 +--
 drivers/net/failsafe/failsafe.c   |  3 ---
 drivers/net/fm10k/fm10k_ethdev.c  |  8 --
 drivers/net/i40e/i40e_ethdev.c|  3 ---
 drivers/net/i40e/i40e_ethdev_vf.c |  3 ---
 drivers/net/ixgbe/ixgbe_ethdev.c  |  9 ---
 drivers/net/kni/rte_eth_kni.c |  2 --
 drivers/net/liquidio/lio_ethdev.c |  3 ---
 drivers/net/mlx4/mlx4.c   |  3 +--
 drivers/net/mlx5/mlx5.c   |  5 +---
 drivers/net/mvneta/mvneta_ethdev.c| 16 
 drivers/net/mvpp2/mrvl_ethdev.c   | 16 
 drivers/net/netvsc/hn_ethdev.c| 11 
 drivers/net/null/rte_eth_null.c   |  2 --
 drivers/net/octeontx/octeontx_ethdev.c| 29 
 drivers/net/pcap/rte_eth_pcap.c   |  7 -
 drivers/net/qede/qede_ethdev.c|  5 
 drivers/net/ring/rte_eth_ring.c   |  4 ---
 drivers/net/sfc/sfc_ethdev.c  |  3 ---
 drivers/net/softnic/rte_eth_softnic.c |  6 ++---
 drivers/net/szedata2/rte_eth_szedata2.c   |  1 -
 drivers/net/tap/rte_eth_tap.c |  1 -
 drivers/net/vhost/rte_eth_vhost.c | 13 +++--
 drivers/net/virtio/virtio_ethdev.c|  3 ---
 drivers/net/virtio/virtio_user_ethdev.c   |  3 ---
 drivers/net/vmxnet3/vmxnet3_ethdev.c  |  3 ---
 lib/librte_ethdev/rte_ethdev.c| 32 +--
 lib/librte_ethdev/rte_ethdev.h|  3 +--
 lib/librte_ethdev/rte_ethdev_pci.h| 11 
 42 files changed, 47 insertions(+), 248 deletions(-)

-- 
2.19.0

[dpdk-dev] [PATCH v3 1/2] ethdev: free all common data when releasing port

[Thomas Monjalon]

This is a clean-up of common ethdev data freeing.
All data freeing are moved to rte_eth_dev_release_port()
and done only in case of primary process.

It is probably fixing some memory leaks for PMDs which were
not freeing all data.

Signed-off-by: Thomas Monjalon 
---
 drivers/net/af_packet/rte_eth_af_packet.c |  2 --
 drivers/net/ark/ark_ethdev.c  |  1 -
 drivers/net/avf/avf_ethdev.c  |  3 ---
 drivers/net/avp/avp_ethdev.c  |  5 
 drivers/net/axgbe/axgbe_ethdev.c  |  3 ---
 drivers/net/bnxt/bnxt_ethdev.c|  4 
 drivers/net/bonding/rte_eth_bond_pmd.c|  7 +-
 drivers/net/cxgbe/cxgbe_main.c| 16 ++---
 drivers/net/cxgbe/cxgbevf_main.c  |  9 +--
 drivers/net/dpaa/dpaa_ethdev.c| 10 
 drivers/net/dpaa2/dpaa2_ethdev.c  | 10 
 drivers/net/e1000/em_ethdev.c |  3 ---
 drivers/net/e1000/igb_ethdev.c|  6 -
 drivers/net/enetc/enetc_ethdev.c  |  4 +---
 drivers/net/failsafe/failsafe.c   |  3 ---
 drivers/net/fm10k/fm10k_ethdev.c  |  8 ---
 drivers/net/i40e/i40e_ethdev.c|  3 ---
 drivers/net/i40e/i40e_ethdev_vf.c |  3 ---
 drivers/net/ixgbe/ixgbe_ethdev.c  |  9 ---
 drivers/net/kni/rte_eth_kni.c |  2 --
 drivers/net/liquidio/lio_ethdev.c |  3 ---
 drivers/net/mlx4/mlx4.c   |  3 +--
 drivers/net/mlx5/mlx5.c   |  5 +---
 drivers/net/mvneta/mvneta_ethdev.c| 16 -
 drivers/net/mvpp2/mrvl_ethdev.c   | 16 -
 drivers/net/netvsc/hn_ethdev.c| 11 -
 drivers/net/null/rte_eth_null.c   |  2 --
 drivers/net/octeontx/octeontx_ethdev.c| 29 +--
 drivers/net/pcap/rte_eth_pcap.c   |  7 --
 drivers/net/qede/qede_ethdev.c|  5 
 drivers/net/ring/rte_eth_ring.c   |  4 
 drivers/net/sfc/sfc_ethdev.c  |  3 ---
 drivers/net/softnic/rte_eth_softnic.c |  6 ++---
 drivers/net/szedata2/rte_eth_szedata2.c   |  1 -
 drivers/net/tap/rte_eth_tap.c |  1 -
 drivers/net/vhost/rte_eth_vhost.c |  9 +++
 drivers/net/virtio/virtio_ethdev.c|  3 ---
 drivers/net/virtio/virtio_user_ethdev.c   |  3 ---
 drivers/net/vmxnet3/vmxnet3_ethdev.c  |  3 ---
 lib/librte_ethdev/rte_ethdev.c| 23 --
 lib/librte_ethdev/rte_ethdev_pci.h| 11 -
 41 files changed, 43 insertions(+), 232 deletions(-)

diff --git a/drivers/net/af_packet/rte_eth_af_packet.c 
b/drivers/net/af_packet/rte_eth_af_packet.c
index bc7daed5e..74073ecf6 100644
--- a/drivers/net/af_packet/rte_eth_af_packet.c
+++ b/drivers/net/af_packet/rte_eth_af_packet.c
@@ -994,8 +994,6 @@ rte_pmd_af_packet_remove(struct rte_vdev_device *dev)
}
free(internals->if_name);
 
-   rte_free(eth_dev->data->dev_private);
-
rte_eth_dev_release_port(eth_dev);
 
return 0;
diff --git a/drivers/net/ark/ark_ethdev.c b/drivers/net/ark/ark_ethdev.c
index 552ca01a6..70d62ec7f 100644
--- a/drivers/net/ark/ark_ethdev.c
+++ b/drivers/net/ark/ark_ethdev.c
@@ -506,7 +506,6 @@ eth_ark_dev_uninit(struct rte_eth_dev *dev)
dev->dev_ops = NULL;
dev->rx_pkt_burst = NULL;
dev->tx_pkt_burst = NULL;
-   rte_free(dev->data->mac_addrs);
return 0;
 }
 
diff --git a/drivers/net/avf/avf_ethdev.c b/drivers/net/avf/avf_ethdev.c
index e56d57c43..13eec1b45 100644
--- a/drivers/net/avf/avf_ethdev.c
+++ b/drivers/net/avf/avf_ethdev.c
@@ -1303,9 +1303,6 @@ avf_dev_uninit(struct rte_eth_dev *dev)
rte_free(vf->aq_resp);
vf->aq_resp = NULL;
 
-   rte_free(dev->data->mac_addrs);
-   dev->data->mac_addrs = NULL;
-
if (vf->rss_lut) {
rte_free(vf->rss_lut);
vf->rss_lut = NULL;
diff --git a/drivers/net/avp/avp_ethdev.c b/drivers/net/avp/avp_ethdev.c
index 0f8ccb954..09388d05f 100644
--- a/drivers/net/avp/avp_ethdev.c
+++ b/drivers/net/avp/avp_ethdev.c
@@ -1036,11 +1036,6 @@ eth_avp_dev_uninit(struct rte_eth_dev *eth_dev)
return ret;
}
 
-   if (eth_dev->data->mac_addrs != NULL) {
-   rte_free(eth_dev->data->mac_addrs);
-   eth_dev->data->mac_addrs = NULL;
-   }
-
return 0;
 }
 
diff --git a/drivers/net/axgbe/axgbe_ethdev.c b/drivers/net/axgbe/axgbe_ethdev.c
index 4b84b50c0..e89c0ec2c 100644
--- a/drivers/net/axgbe/axgbe_ethdev.c
+++ b/drivers/net/axgbe/axgbe_ethdev.c
@@ -718,9 +718,6 @@ eth_axgbe_dev_uninit(struct rte_eth_dev *eth_dev)
return 0;
 
pci_dev = RTE_DEV_TO_PCI(eth_dev->device);
-   /*Free macaddres*/
-   rte_free(eth_dev->data->mac_addrs);
-   eth_dev->data->mac_addrs = NULL;
eth_dev->dev_ops = NULL;
eth_dev->rx_pkt_burst = NULL;
eth_dev->tx_pkt_burst = NULL;
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/n

[dpdk-dev] [PATCH v3 2/2] ethdev: complete closing of port

[Thomas Monjalon]

After closing a port, it cannot be restarted.
So there is no reason to not free all associated resources.

The last step was done with rte_eth_dev_detach() which is deprecated.
Instead of blindly removing the associated rte_device, the driver should
check if no more port (ethdev, cryptodev, etc) is open for the device.

The last ethdev freeing which were done by rte_eth_dev_detach(),
are now done at the end of rte_eth_dev_close().

If the driver is trying to free the port again, the function
rte_eth_dev_release_port() will abort with -ENODEV error.

Signed-off-by: Thomas Monjalon 
---
 drivers/net/bnxt/bnxt_ethdev.c| 4 
 drivers/net/vhost/rte_eth_vhost.c | 4 
 lib/librte_ethdev/rte_ethdev.c| 9 +++--
 lib/librte_ethdev/rte_ethdev.h| 3 +--
 4 files changed, 4 insertions(+), 16 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index ff41cb0fa..183b40821 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -712,10 +712,6 @@ static void bnxt_dev_close_op(struct rte_eth_dev *eth_dev)
if (bp->dev_stopped == 0)
bnxt_dev_stop_op(eth_dev);
 
-   if (eth_dev->data->mac_addrs != NULL) {
-   rte_free(eth_dev->data->mac_addrs);
-   eth_dev->data->mac_addrs = NULL;
-   }
if (bp->grp_info != NULL) {
rte_free(bp->grp_info);
bp->grp_info = NULL;
diff --git a/drivers/net/vhost/rte_eth_vhost.c 
b/drivers/net/vhost/rte_eth_vhost.c
index 986bf9633..b9cefbbe5 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -998,12 +998,8 @@ eth_dev_close(struct rte_eth_dev *dev)
for (i = 0; i < dev->data->nb_tx_queues; i++)
rte_free(dev->data->tx_queues[i]);
 
-   rte_free(dev->data->mac_addrs);
free(internal->dev_name);
free(internal->iface_name);
-   rte_free(internal);
-
-   dev->data->dev_private = NULL;
 }
 
 static int
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 178800a5b..987ba5ab1 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -367,6 +367,8 @@ rte_eth_dev_release_port(struct rte_eth_dev *eth_dev)
 {
if (eth_dev == NULL)
return -EINVAL;
+   if (eth_dev->state == RTE_ETH_DEV_UNUSED)
+   return -ENODEV;
 
rte_eth_dev_shared_data_prepare();
 
@@ -1384,12 +1386,7 @@ rte_eth_dev_close(uint16_t port_id)
dev->data->dev_started = 0;
(*dev->dev_ops->dev_close)(dev);
 
-   dev->data->nb_rx_queues = 0;
-   rte_free(dev->data->rx_queues);
-   dev->data->rx_queues = NULL;
-   dev->data->nb_tx_queues = 0;
-   rte_free(dev->data->tx_queues);
-   dev->data->tx_queues = NULL;
+   rte_eth_dev_release_port(dev);
 }
 
 int
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index fb40c89e0..dcdeb184b 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -1802,8 +1802,7 @@ int rte_eth_dev_set_link_down(uint16_t port_id);
 
 /**
  * Close a stopped Ethernet device. The device cannot be restarted!
- * The function frees all resources except for needed by the
- * closed state. To free these resources, call rte_eth_dev_detach().
+ * The function frees all port resources.
  *
  * @param port_id
  *   The port identifier of the Ethernet device.
-- 
2.19.0

[dpdk-dev] [PATCH] net/mlx5: fix compiling issue of zero-size array

[Jack Min]

If the build environment hasn't 'linux/tc_act/tc_pedit.h',
compiler will use needed structs defined in mlx5_flow_tcf.c.

However, there is a zero-size array defined in one struct and
ISO C forbids this when -Wpedantic is set by debug mode.

Simply put __extension__ keyword before the struct in question.

Fixes: e696b2051c8c ("net/mlx5: rewrite IP address UDP/TCP port by E-Switch")

Signed-off-by: Xiaoyu Min 
---
 drivers/net/mlx5/mlx5_flow_tcf.c | 19 ++-
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c b/drivers/net/mlx5/mlx5_flow_tcf.c
index aaafaf52d..da4bc632d 100644
--- a/drivers/net/mlx5/mlx5_flow_tcf.c
+++ b/drivers/net/mlx5/mlx5_flow_tcf.c
@@ -93,19 +93,20 @@ enum pedit_cmd {
 };
 
 struct tc_pedit_key {
-   __u32   mask;  /* AND */
-   __u32   val;   /*XOR */
-   __u32   off;  /*offset */
-   __u32   at;
-   __u32   offmask;
-   __u32   shift;
+   __u32 mask; /* AND */
+   __u32 val; /*XOR */
+   __u32 off; /*offset */
+   __u32 at;
+   __u32 offmask;
+   __u32 shift;
 };
 
+__extension__
 struct tc_pedit_sel {
tc_gen;
-   unsigned char   nkeys;
-   unsigned char   flags;
-   struct tc_pedit_key keys[0];
+   unsigned char nkeys;
+   unsigned char flags;
+   struct tc_pedit_key keys[0];
 };
 
 #endif /* HAVE_TC_ACT_VLAN */
-- 
2.17.1

Re: [dpdk-dev] [PATCH v3] doc: add known issue about legacy intr mode for ixgbe

[Li, Xiaoyun]

Hi

> > +Do not bind the ``uio_pci_generic`` module in X550 NICs.
> > +Do not bind ``igb_uio`` with legacy mode in X550 NICs.
> > +Before using ``vfio`` with legacy mode in X550 NICs, using ``modprobe
> > +vfio `` ``nointxmask=1`` to bind ``vfio``.
> 
> I have couple questions here.
> If noinitxmask=1 is set, does that mean, in vfio interrupt handler we will not
> check intr mask? but what if the intx is shared by another devices?
> Does that mean we will also handle interrupt from other devices which is not
> expected?

If noinitxmask=1 is not set, vfio will check if it has broken intx msking 
(which is hardcoded. Only i40e is considered as broken).
If it is broken, vfio_intx_handler will disable_irq_nosync and mask this irq 
and then return the efd to userspace.
But if it is not broken, vfio_intx_handler will call 
pci_check_and_set_intx_mask. In pci_check_and_set_intx_mask, the interrupt 
status bit is checked.
And for X550, vfio thinks it's intx masking is not broken and will check its 
interrupt status bit. But this bit is not enabled in X550 in fact. Then vfio 
will think the interrupt isn't triggered and will not deal with it. Then 
"nobody cared" issue occurs.

But when with nointxmask=1, vfio will think the device has broken intx masking. 
Then will disable_irq_nosync and mask this irq and then return the efd to 
userspace.

Since it will disable_irq_nosync, the irq will be affected if it is shared by 
another device.
> 
> Not sure if we should add some statement like "if the intx is not shared with
> other device " ?
Yes. I will add it.

> 
> >
> >  Inline crypto processing support
> >  
> > --
> > 2.7.4

Re: [dpdk-dev] [PATCH] net/ixgbe: fix busy polling while fiber link update

[Zhao1, Wei]

Hi， Ilya Maximets

> -Original Message-
> From: Ilya Maximets [mailto:i.maxim...@samsung.com]
> Sent: Friday, October 12, 2018 6:15 PM
> To: Zhao1, Wei ; Zhang, Qi Z ;
> Laurent Hardy 
> Cc: Lu, Wenzhuo ; Ananyev, Konstantin
> ; sta...@dpdk.org; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: fix busy polling while fiber link
> update
> 
> On 12.10.2018 12:19, Zhao1, Wei wrote:
> > Hi，
> >
> >> -Original Message-
> >> From: Ilya Maximets [mailto:i.maxim...@samsung.com]
> >> Sent: Thursday, October 11, 2018 6:27 PM
> >> To: Zhao1, Wei ; Zhang, Qi Z
> >> ; Laurent Hardy 
> >> Cc: Lu, Wenzhuo ; Ananyev, Konstantin
> >> ; sta...@dpdk.org; dev@dpdk.org
> >> Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: fix busy polling while
> >> fiber link update
> >>
> >> On 11.10.2018 12:56, Zhao1, Wei wrote:
> >>> Hi,  Ilya Maximets AND laurent.hardy
> >>
> >> Hi, thanks for sharing your thoughts.
> >> Comments inline.
> >>
> >>>
> >>>
>  -Original Message-
>  From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Ilya Maximets
>  Sent: Wednesday, September 12, 2018 4:05 PM
>  To: Zhang, Qi Z ; dev@dpdk.org
>  Cc: Lu, Wenzhuo ; Ananyev, Konstantin
>  ; Laurent Hardy
>  ; Dai, Wei ;
>  sta...@dpdk.org
>  Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: fix busy polling while
>  fiber link update
> 
>  On 12.09.2018 09:49, Zhang, Qi Z wrote:
> >
> >
> >> -Original Message-
> >> From: Ilya Maximets [mailto:i.maxim...@samsung.com]
> >> Sent: Monday, September 10, 2018 11:09 PM
> >> To: Zhang, Qi Z ; dev@dpdk.org
> >> Cc: Lu, Wenzhuo ; Ananyev, Konstantin
> >> ; Laurent Hardy
> >> ; Dai, Wei ;
> >> sta...@dpdk.org
> >> Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: fix busy polling while
> >> fiber link update
> >>
> >> On 04.09.2018 09:08, Zhang, Qi Z wrote:
> >>> Hi Ilya:
> >>>
>  -Original Message-
>  From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Ilya
>  Maximets
>  Sent: Friday, August 31, 2018 8:40 PM
>  To: dev@dpdk.org
>  Cc: Lu, Wenzhuo ; Ananyev, Konstantin
>  ; Laurent Hardy
>  ; Dai, Wei ; Ilya
>  Maximets ; sta...@dpdk.org
>  Subject: [dpdk-dev] [PATCH] net/ixgbe: fix busy polling while
>  fiber link update
> 
>  If the multispeed fiber link is in DOWN state, ixgbe_setup_link
>  could take around a second of busy polling. This is highly
>  inconvenient for the case where single thread periodically
>  checks the
>  link statuses.
>  For example, OVS main thread periodically updates the link
>  statuses and hangs for a really long time busy waiting on
>  ixgbe_setup_link() for a DOWN fiber ports. For case with 3 down
>  ports it hangs for a 3 seconds and unable to do anything
>  including
> >> packet processing.
>  Fix that by shifting that workaround to a separate thread by
>  alarm handler that will try to set up link if it is DOWN.
> >>>
> >>> Does that mean we will block the interrupt thread for 3 seconds?
> >>
> >> Three times for one second. Other work could be scheduled
> between.
> >> IMHO, it's much better than blocking usual caller for 3 seconds.
> >>
> >>> Also, can we guarantee there will not be any race condition if
> >>> we call
> >> ixgbe_setup_link at another thread, the base code API is not
> >> assumed to be thread-safe as I know.
> >>
> >> The only user of 'ixgbe_setup_link' is 'ixgbe_dev_start', but it
> >> could be called only if device stopped. 'ixgbe_dev_stop' cancels
> >> the
> >> alarm.
> >> Race with 'link_update' avoided by
> 'IXGBE_FLAG_NEED_LINK_CONFIG'
>  flag.
> >
> > I guess, it' not only about when ixgb_setup_link race with itself,
> > but also
>  when it race with other APIs.
> > Also the concern is, even in current version, we can prove there
> > is no issue,
>  how can we guarantee we are safe for future base code update? It's
>  not designed as thread-safe.
> > For my option, the change is risky.
> 
>  In current implementation interrupt handler already calls the
>  'ixgbe_dev_link_update' which subsequently calls 'ixgbe_setup_link'
>  in our case if LSC interrupts enabled. So, my change makes the
>  driver even safer by moving 'ixgbe_setup_link' to the same interrupt
> thread.
>  Otherwise two threads (interrupts handler and the link status
>  checking
>  thread) could call 'ixgbe_setup_link' simultaneously.
> 
> >
> > Btw, since ixgbe support LSC, it is not necessary for "single
> > thread
>  periodically checks the link statuses", right?
> 
>  In current implementation it will take at least 5 seconds (4 + 1)
>  for the interrupt handler to detect DOWN link state for ixgbe
>  mult

Re: [dpdk-dev] [PATCH] net/ixgbe: put 5tuple check in front to jump over ntuple filter case

[Zhang, Qi Z]

Hi Wei:

> -Original Message-
> From: Zhao1, Wei
> Sent: Thursday, October 11, 2018 1:10 AM
> To: Zhang, Qi Z ; mocan 
> Cc: dev@dpdk.org; Lu, Wenzhuo 
> Subject: RE: Re:RE: [dpdk-dev] [PATCH] net/ixgbe: put 5tuple check in front
> to jump over ntuple filter case
> 
> Hi, qi
> 
> > -Original Message-
> > From: Zhang, Qi Z
> > Sent: Thursday, October 11, 2018 2:36 AM
> > To: Zhao1, Wei ; mocan 
> > Cc: dev@dpdk.org; Lu, Wenzhuo 
> > Subject: RE: Re:RE: [dpdk-dev] [PATCH] net/ixgbe: put 5tuple check in
> > front to jump over ntuple filter case
> >
> >
> >
> > > -Original Message-
> > > From: Zhao1, Wei
> > > Sent: Monday, October 8, 2018 2:46 AM
> > > To: Zhang, Qi Z ; mocan 
> > > Cc: dev@dpdk.org; Lu, Wenzhuo 
> > > Subject: RE: Re:RE: [dpdk-dev] [PATCH] net/ixgbe: put 5tuple check
> > > in front to jump over ntuple filter case
> > >
> > > Hi,
> > >
> > >
> > > > -Original Message-
> > > > From: Zhang, Qi Z
> > > > Sent: Wednesday, September 26, 2018 7:14 PM
> > > > To: mocan ; Zhao1, Wei
> 
> > > > Cc: dev@dpdk.org
> > > > Subject: RE: Re:RE: [dpdk-dev] [PATCH] net/ixgbe: put 5tuple check
> > > > in front to jump over ntuple filter case
> > > >
> > > > OK, got your point. We should not reject a possible valid fdir
> > > > flow at n-tuple flow check stage.
> > > >
> > > > Review-by: Qi Zhang 
> > >
> > >
> > > I agree with the point of " We should not reject a possible valid
> > > fdir flow at n-tuple flow check stage".
> > > But, I think the fix patch should be more generic for all types
> > > filter of this problem.
> > > Maybe, we should delete all " goto out"  in function
> ixgbe_flow_create().
> > > Then, it will go to ntuple filter and  ethertype filter, syn filter
> > > and fdir filter ,l2_tn_filter one by one.
> > > And aslo, we should code as
> > >
> > > {
> > >
> > > Ntuple:
> > > ..
> > > if(ret)
> > > Goto ethertype
> > > ..
> > >
> > > Ethertype:
> > >
> > > ..
> > > if(ret)
> > > Goto fdir filter
> > > .
> > >
> > > fdir filter:
> > >
> > > if(ret)
> > >   Goto l2_tn_filter
> > >
> > > l2_tn_filter:
> > >
> > > .
> > >
> > > }
> > >
> > > This fix patch only solve the problem of  ntuple and fdir.
> > > Qi, What do you think of this?
> >
> > I'm not the author of this part of code, so my understanding of
> > current implementation is:
> > It assume a flow will not be ambiguous which means if it match to some
> > filter parser (ixgbe_parse_xxx_filter), it is not necessary to match
> > on a different filter.
> > But I'm not sure if the assumption is correct or not, (this depends on
> > the knowledge of the device capability), So do you mean the assumption
> > is not correct? If you think a generic fix is necessary, I have below
> > comments
> 
> Yes, the assumption is may cause bug, this patch is an evidence, maybe this
> user has encountered this problem.
> 
> >
> > 1. it is better be done by Intel people with enough validation
> 
> I agree with you, I will commit a generic fix patch later.
> 
> >2. two options  for patch submit.
> > Submit a v2 with the generic fix, and it will be captured in this 
> > release.
> > If it is not urgent, we can just accept current one first, then have
> >a  separate patch in next release.
> 
> Ok, If someone supply a v2 with the generic fix, I will ack.
> 

Just want to confirm with you , are you agree to merge this patch?
Or you think v2 with generic fix is necessary?
>From my view, the patch can be accepted, since it just add more strict check 
>in cons_parse_ntuple_filter, it does not break anything, and it fix the 
>specific issue.

Thanks
Qi

Re: [dpdk-dev] [PATCH] doc/proc_info: update support for vdev stats

[Varghese, Vipin]

Hi Ferruh,

Please find the answers embedded to the email. 
 


> 
> On 10/12/2018 12:35 PM, Vipin Varghese wrote:
> > With the latest release, support for vdev ethernet devices like pcap,
> > tun and tap are been added. Hence collecting statistics using
> > proc-info is possible now.
> 
> This support is enabled when device shared between primary & secondary. The
> capability to share virtual devices added that is correct, 

The document update is for procinfo tool, which is secondary process to collect 
stats for NIC interfaces under primary process.
Earlier documentation states since DPDK multi process has limitation in sharing 
virtual interface stats. The correction updates the same.

but still user may prefer
> to use virtual device as process only.

Proc info info tool purpose is collect stats from primary. Hence whether the 
primary process uses physical or virtual it is user discretion. But collecting 
stats is possible for any NIC device via proc info tool.

> Not sure if it worth mentioning stats retrieving supported when virtual 
> device is
> shared between processes, what do you think?
> 

With the above reasoning, I believe proc info tool is state it can collect 
stats from both virtual and physical NIC is correct.

[dpdk-dev] [PATCH v4] doc: add known issue about legacy intr mode for ixgbe

[Xiaoyun Li]

When using uio_pci_generic module or using legacy interrupt mode of
igb_uio or vfio, X550 cannot get interrupts. Because the Interrupt
Status bit is not implemented, then the irq cannot be handled correctly
and cannot report the event fd to DPDK apps.

Add this hw limitation and details into ixgbe known issue.

Signed-off-by: Xiaoyun Li 
---
v4:
 * Clarify that the vfio workaround only works when the intx is not shared
   with other devices.
v3:
 * Correct a misspelling.
v2:
 * Correct the link of X550 spec update.
 * Polish the known issue title.
---
 doc/guides/nics/ixgbe.rst | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/doc/guides/nics/ixgbe.rst b/doc/guides/nics/ixgbe.rst
index 16d6390..1c294b0 100644
--- a/doc/guides/nics/ixgbe.rst
+++ b/doc/guides/nics/ixgbe.rst
@@ -200,6 +200,33 @@ There is no RTE API to add a VF's MAC address from the PF. 
On ixgbe, the
 ``rte_eth_dev_mac_addr_add()`` function can be used to add a VF's MAC address,
 as a workaround.
 
+X550 does not support legacy interrupt mode
+~~~
+
+Desccription
+
+X550 cannot get interrupts if using ``uio_pci_generic`` module or using legacy
+interrupt mode of ``igb_uio`` or ``vfio``. Because the errata of X550 states
+that the Interrupt Status bit is not implemented. The errata is the item #22
+from `X550 spec update `_
+
+Implication
+^^^
+When using ``uio_pci_generic`` module or using legacy interrupt mode of
+``igb_uio`` or ``vfio``, the Interrupt Status bit would be checked if the
+interrupt is coming. Since the bit is not implemented in X550, the irq cannot
+be handled correctly and cannot report the event fd to DPDK apps. Then apps
+cannot get interrupts and ``dmesg`` will show messages like ``irq #No.: ``
+``nobody cared.``
+
+Workaround
+^^
+Do not bind the ``uio_pci_generic`` module in X550 NICs.
+Do not bind ``igb_uio`` with legacy mode in X550 NICs.
+Before binding ``vfio`` with legacy mode in X550 NICs, use ``modprobe vfio ``
+``nointxmask=1`` to load ``vfio`` module if the intx is not shared with other
+devices.
 
 Inline crypto processing support
 
-- 
2.7.4

Re: [dpdk-dev] [PATCH] net/mlx5: fix compiling issue of zero-size array

[Shahaf Shuler]

Monday, October 15, 2018 4:45 AM, Jack Min:
> Subject: [PATCH] net/mlx5: fix compiling issue of zero-size array
> 
> If the build environment hasn't 'linux/tc_act/tc_pedit.h', compiler will use
> needed structs defined in mlx5_flow_tcf.c.
> 
> However, there is a zero-size array defined in one struct and ISO C forbids
> this when -Wpedantic is set by debug mode.
> 
> Simply put __extension__ keyword before the struct in question.
> 
> Fixes: e696b2051c8c ("net/mlx5: rewrite IP address UDP/TCP port by E-
> Switch")
> 
> Signed-off-by: Xiaoyu Min 

Applied to next-net-mlx, thanks.

Ferruh, you will probably want to squash this to the relevant commit. 

Re: [dpdk-dev] [PATCH v5 1/7] net/mlx5: remove useless driver name comparison

[Shahaf Shuler]

Sunday, October 14, 2018 11:49 PM, Thomas Monjalon:
> 14/10/2018 22:47, Thomas Monjalon:
> > The function mlx5_dev_to_port_id() is returning all the ports
> > associated to a rte_device.
> > It was comparing driver names while already comparing rte_device
> pointers.
> > If two devices are the same, they will have the same driver.
> > So the useless driver name comparison is removed.
> >
> > Signed-off-by: Thomas Monjalon 
> > ---
> >  drivers/net/mlx5/mlx5_ethdev.c | 5 +
> >  1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/mlx5/mlx5_ethdev.c
> > b/drivers/net/mlx5/mlx5_ethdev.c index 61eda537b..cacdf8e18 100644
> > --- a/drivers/net/mlx5/mlx5_ethdev.c
> > +++ b/drivers/net/mlx5/mlx5_ethdev.c
> > @@ -1305,10 +1305,7 @@ mlx5_dev_to_port_id(const struct rte_device
> *dev, uint16_t *port_list,
> > RTE_ETH_FOREACH_DEV(id) {
> > struct rte_eth_dev *ldev = &rte_eth_devices[id];
> >
> > -   if (!ldev->device ||
> > -   !ldev->device->driver ||
> > -   strcmp(ldev->device->driver->name,
> MLX5_DRIVER_NAME) ||
> > -   ldev->device != dev)
> > +   if (ldev->device != dev)
> > continue;
> > if (n < port_list_n)
> > port_list[n] = id;
> >

Acked-By: Shahaf Shuler 

Re: [dpdk-dev] [PATCH v5 5/7] drivers/bus: move driver assignment to end of probing

[Xu, Rosen]

> -Original Message-
> From: Thomas Monjalon [mailto:tho...@monjalon.net]
> Sent: Monday, October 15, 2018 4:54
> To: dev@dpdk.org
> Cc: gaetan.ri...@6wind.com; ophi...@mellanox.com; Zhang, Qi Z
> ; Yigit, Ferruh ;
> re...@netcope.com; Xing, Beilei ;
> shah...@mellanox.com; ys...@mellanox.com; Xu, Rosen
> ; step...@networkplumber.org
> Subject: Re: [dpdk-dev] [PATCH v5 5/7] drivers/bus: move driver assignment
> to end of probing
> 
> +Cc vmbus, ifpga, i40e, mlx and szedata2 maintainers
> 
> 14/10/2018 22:47, Thomas Monjalon:
> > The PCI mapping requires to know the PCI driver to use, even before
> > the probing is done. That's why the PCI driver is referenced early
> > inside the PCI device structure. See commit 1d20a073fa5e ("bus/pci:
> > reference driver structure before mapping")
> >
> > However the rte_driver does not need to be referenced in rte_device
> > before the device probing is done.
> > By moving back this assignment at the end of the device probing, it
> > becomes possible to make clear the status of a rte_device.
> >
> > Signed-off-by: Thomas Monjalon 
> > Reviewed-by: Andrew Rybchenko 
> > Tested-by: Andrew Rybchenko 
> > ---
> >  drivers/bus/ifpga/ifpga_bus.c   | 9 -
> >  drivers/bus/pci/pci_common.c| 7 +++
> >  drivers/bus/pci/rte_bus_pci.h   | 2 +-
> >  drivers/bus/vdev/vdev.c | 5 ++---
> >  drivers/bus/vmbus/vmbus_common.c| 5 +++--
> >  drivers/net/i40e/i40e_vf_representor.c  | 3 ---
> >  drivers/net/mlx4/mlx4.c | 1 -
> >  drivers/net/mlx5/mlx5.c | 1 -
> >  drivers/net/szedata2/rte_eth_szedata2.c | 6 +++---
> > lib/librte_eal/common/include/rte_dev.h | 2 +-
> >  10 files changed, 17 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/bus/ifpga/ifpga_bus.c
> > b/drivers/bus/ifpga/ifpga_bus.c index 80663328a..fca2dbace 100644
> > --- a/drivers/bus/ifpga/ifpga_bus.c
> > +++ b/drivers/bus/ifpga/ifpga_bus.c
> > @@ -280,14 +280,13 @@ ifpga_probe_one_driver(struct rte_afu_driver
> > *drv,
> >
> > /* reference driver structure */
> > afu_dev->driver = drv;
> > -   afu_dev->device.driver = &drv->driver;
> >
> > /* call the driver probe() function */
> > ret = drv->probe(afu_dev);
> > -   if (ret) {
> > +   if (ret)
> > afu_dev->driver = NULL;
> > -   afu_dev->device.driver = NULL;
> > -   }
> > +   else
> > +   afu_dev->device.driver = &drv->driver;
> >
> > return ret;
> >  }
> > @@ -302,7 +301,7 @@ ifpga_probe_all_drivers(struct rte_afu_device
> *afu_dev)
> > return -1;
> >
> > /* Check if a driver is already loaded */
> > -   if (afu_dev->driver != NULL)
> > +   if (afu_dev->device.driver != NULL)
> > return 0;
> >
> > TAILQ_FOREACH(drv, &ifpga_afu_drv_list, next) { diff --git

 Reviewed-by: Rosen Xu 

> > a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c index
> > c7695d108..d63e68045 100644
> > --- a/drivers/bus/pci/pci_common.c
> > +++ b/drivers/bus/pci/pci_common.c
> > @@ -160,14 +160,12 @@ rte_pci_probe_one_driver(struct rte_pci_driver
> *dr,
> >  * driver flags for adjusting configuration.
> >  */
> > dev->driver = dr;
> > -   dev->device.driver = &dr->driver;
> >
> > if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
> > /* map resources for devices that use igb_uio */
> > ret = rte_pci_map_device(dev);
> > if (ret != 0) {
> > dev->driver = NULL;
> > -   dev->device.driver = NULL;
> > return ret;
> > }
> > }
> > @@ -176,7 +174,6 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr,
> > ret = dr->probe(dr, dev);
> > if (ret) {
> > dev->driver = NULL;
> > -   dev->device.driver = NULL;
> > if ((dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) &&
> > /* Don't unmap if device is unsupported and
> >  * driver needs mapped resources.
> > @@ -184,6 +181,8 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr,
> > !(ret > 0 &&
> > (dr->drv_flags &
> RTE_PCI_DRV_KEEP_MAPPED_RES)))
> > rte_pci_unmap_device(dev);
> > +   } else {
> > +   dev->device.driver = &dr->driver;
> > }
> >
> > return ret;
> > @@ -244,7 +243,7 @@ pci_probe_all_drivers(struct rte_pci_device *dev)
> > return -1;
> >
> > /* Check if a driver is already loaded */
> > -   if (dev->driver != NULL)
> > +   if (dev->device.driver != NULL)
> > return 0;
> >
> > FOREACH_DRIVER_ON_PCIBUS(dr) {
> > diff --git a/drivers/bus/pci/rte_bus_pci.h
> > b/drivers/bus/pci/rte_bus_pci.h index 0d1955ffe..984df2b37 100644
> > --- a/drivers/bus/pci/rte_bus_pci.h
> > +++ b/drivers/bus/pci/rte_bus_pci.h
> > @@ -62,7 +62,7 @@ struct rte_pci_device {
> > struct rte_mem_resource mem_resource[PCI_MAX_RESOURCE];
> > 

Re: [dpdk-dev] [PATCH v3 1/5] bus/fslmc: fix physical addressing check

[Shreyansh Jain]

On Saturday 13 October 2018 09:38 PM, Pavan Nikhilesh wrote:
> On Sat, Oct 13, 2018 at 05:51:26PM +0530, Shreyansh Jain wrote:
>> In case RTE_LIBRTE_DPAA2_USE_PHYS_IOVA is enabled, only supported
>> class is RTE_IOVA_PA.
>>
>> Fixes: f7768afac101 ("bus/fslmc: support dynamic IOVA")
>> Cc: hemant.agra...@nxp.com
>>
>> Signed-off-by: Shreyansh Jain 
>> ---
>>   drivers/bus/fslmc/fslmc_bus.c | 4 
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
>> index 960f55071..19e33caf1 100644
>> --- a/drivers/bus/fslmc/fslmc_bus.c
>> +++ b/drivers/bus/fslmc/fslmc_bus.c
>> @@ -493,6 +493,10 @@ rte_dpaa2_get_iommu_class(void)
>>  bool is_vfio_noiommu_enabled = 1;
>>  bool has_iova_va;
>>
>> +#ifdef RTE_LIBRTE_DPAA2_USE_PHYS_IOVA
>> +   return RTE_IOVA_PA;
>> +#endif
>> +
> 
> I think you forgot to move it below the device list check as discussed in
> previous patchset? :).

Yes, :(. Sorry.
I mixed up my internal branches.

> 
>>  if (TAILQ_EMPTY(&rte_fslmc_bus.device_list))
>>  return RTE_IOVA_DC;
>>
>> --
>> 2.17.1
>>

Re: [dpdk-dev] [PATCH v4 0/8] FIPS validation capability

[Kusztal, ArkadiuszX]

> -Original Message-
> From: Kovacevic, Marko
> Sent: Friday, October 12, 2018 4:45 PM
> To: dev@dpdk.org
> Cc: Zhang, Roy Fan ; akhil.go...@nxp.com;
> Kusztal, ArkadiuszX ; Kovacevic, Marko
> 
> Subject: [PATCH v4 0/8] FIPS validation capability
> 
> This sample application is made for the purpose so that users of DPDK who
> wish to get FIPS certification for their platforms, this sample app enables
> users to parse test vectors that is gotten from NIST and be able to get a
> generated response file which they can then verify and be sure their system
> will pass FIPS certification.
> 
> Marko Kovacevic (8):
> v4:
> - Added Limitation
> - Changed TDES BLOCK SIZE from 16 -> 8
>   as DES block size is 64bits (main.c)
> v3:
> - Fixed a no-testing bug
> - Fixed some code style issue
> 
> v2:
> - Refactor the code.
> - Move the code from test to sample applcation
> 
>   examples: add fips validation into examples
>   examples: add aes parser and enablement for test types
>   examples: add hmac parser
>   examples: add TDES parser and enablement for test types
>   examples: add gcm parser
>   examples: add cmac parser and enablement for test types
>   examples: add ccm parser and enablement for test types
>   doc: add guides for fips validation
> 
>  doc/guides/rel_notes/release_18_11.rst |6 +
>  doc/guides/sample_app_ug/fips_validation.rst   |  105 ++
>  doc/guides/sample_app_ug/index.rst |1 +
>  examples/cryptodev_fips_validate/Makefile  |   75 ++
>  .../cryptodev_fips_parse_3des.c|  259 +
>  .../cryptodev_fips_parse_aes.c |  188 +++
>  .../cryptodev_fips_parse_ccm.c |  272 +
>  .../cryptodev_fips_parse_cmac.c|  116 ++
>  .../cryptodev_fips_parse_gcm.c |  125 ++
>  .../cryptodev_fips_parse_hmac.c|  105 ++
>  .../cryptodev_fips_parse_validate.c|  593 ++
>  .../cryptodev_fips_validate.h  |  234 
>  examples/cryptodev_fips_validate/main.c| 1221
> 
>  examples/cryptodev_fips_validate/meson.build   |   20 +
>  14 files changed, 3320 insertions(+)
>  create mode 100644 doc/guides/sample_app_ug/fips_validation.rst
>  create mode 100644 examples/cryptodev_fips_validate/Makefile
>  create mode 100644
> examples/cryptodev_fips_validate/cryptodev_fips_parse_3des.c
>  create mode 100644
> examples/cryptodev_fips_validate/cryptodev_fips_parse_aes.c
>  create mode 100644
> examples/cryptodev_fips_validate/cryptodev_fips_parse_ccm.c
>  create mode 100644
> examples/cryptodev_fips_validate/cryptodev_fips_parse_cmac.c
>  create mode 100644
> examples/cryptodev_fips_validate/cryptodev_fips_parse_gcm.c
>  create mode 100644
> examples/cryptodev_fips_validate/cryptodev_fips_parse_hmac.c
>  create mode 100644
> examples/cryptodev_fips_validate/cryptodev_fips_parse_validate.c
>  create mode 100644
> examples/cryptodev_fips_validate/cryptodev_fips_validate.h
>  create mode 100644 examples/cryptodev_fips_validate/main.c
>  create mode 100644 examples/cryptodev_fips_validate/meson.build
> 
> --
> 2.9.5

Series-acked-by: Arek Kusztal 

[dpdk-dev] [PATCH v4 4/5] dpaa: enable dpaax library

[Shreyansh Jain]

With this patch, dpaa bus and ethernet devices on this bus
would start using the physical-virtual library interfaces.

This patch impacts mempool/dpaa, event/dpaa and net/dpaa as
they are dependent on the bus/dpaa and thus impact linkage of
libraries.

Signed-off-by: Shreyansh Jain 
---
 drivers/bus/dpaa/Makefile   | 1 +
 drivers/bus/dpaa/dpaa_bus.c | 4 
 drivers/bus/dpaa/meson.build| 2 +-
 drivers/bus/dpaa/rte_dpaa_bus.h | 6 ++
 drivers/crypto/dpaa_sec/Makefile| 1 +
 drivers/crypto/dpaa_sec/dpaa_sec.c  | 6 ++
 drivers/event/dpaa/Makefile | 1 +
 drivers/mempool/dpaa/Makefile   | 1 +
 drivers/mempool/dpaa/dpaa_mempool.c | 4 
 drivers/mempool/dpaa/dpaa_mempool.h | 4 +---
 drivers/net/dpaa/Makefile   | 1 +
 mk/rte.app.mk   | 1 +
 12 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/drivers/bus/dpaa/Makefile b/drivers/bus/dpaa/Makefile
index 9337b5f92..381a5c659 100644
--- a/drivers/bus/dpaa/Makefile
+++ b/drivers/bus/dpaa/Makefile
@@ -48,5 +48,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_DPAA_BUS) += \
 LDLIBS += -lpthread
 LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring
 LDLIBS += -lrte_ethdev
+LDLIBS += -lrte_common_dpaax
 
 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/bus/dpaa/dpaa_bus.c b/drivers/bus/dpaa/dpaa_bus.c
index 138e0f98d..381c3b17c 100644
--- a/drivers/bus/dpaa/dpaa_bus.c
+++ b/drivers/bus/dpaa/dpaa_bus.c
@@ -34,6 +34,7 @@
 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -548,6 +549,9 @@ rte_dpaa_bus_probe(void)
fclose(svr_file);
}
 
+   /* And initialize the PA->VA translation table */
+   dpaax_iova_table_populate();
+
/* For each registered driver, and device, call the driver->probe */
TAILQ_FOREACH(dev, &rte_dpaa_bus.device_list, next) {
TAILQ_FOREACH(drv, &rte_dpaa_bus.driver_list, next) {
diff --git a/drivers/bus/dpaa/meson.build b/drivers/bus/dpaa/meson.build
index 5e7705571..11a3c9499 100644
--- a/drivers/bus/dpaa/meson.build
+++ b/drivers/bus/dpaa/meson.build
@@ -7,7 +7,7 @@ if host_machine.system() != 'linux'
 build = false
 endif
 
-deps += ['eventdev']
+deps += ['common_dpaax', 'eventdev']
 sources = files('base/fman/fman.c',
'base/fman/fman_hw.c',
'base/fman/netcfg_layer.c',
diff --git a/drivers/bus/dpaa/rte_dpaa_bus.h b/drivers/bus/dpaa/rte_dpaa_bus.h
index 15dc6a4ac..1d580a000 100644
--- a/drivers/bus/dpaa/rte_dpaa_bus.h
+++ b/drivers/bus/dpaa/rte_dpaa_bus.h
@@ -8,6 +8,7 @@
 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -110,6 +111,11 @@ extern struct dpaa_memseg_list rte_dpaa_memsegs;
 static inline void *rte_dpaa_mem_ptov(phys_addr_t paddr)
 {
struct dpaa_memseg *ms;
+   void *va;
+
+   va = dpaax_iova_table_get_va(paddr);
+   if (likely(va != NULL))
+   return va;
 
/* Check if the address is already part of the memseg list internally
 * maintained by the dpaa driver.
diff --git a/drivers/crypto/dpaa_sec/Makefile b/drivers/crypto/dpaa_sec/Makefile
index 9be447041..674a7a398 100644
--- a/drivers/crypto/dpaa_sec/Makefile
+++ b/drivers/crypto/dpaa_sec/Makefile
@@ -38,5 +38,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_PMD_DPAA_SEC) += dpaa_sec.c
 
 LDLIBS += -lrte_bus_dpaa
 LDLIBS += -lrte_mempool_dpaa
+LDLIBS += -lrte_common_dpaax
 
 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/crypto/dpaa_sec/dpaa_sec.c 
b/drivers/crypto/dpaa_sec/dpaa_sec.c
index 7c0459f9f..54f1913f2 100644
--- a/drivers/crypto/dpaa_sec/dpaa_sec.c
+++ b/drivers/crypto/dpaa_sec/dpaa_sec.c
@@ -107,6 +107,12 @@ dpaa_mem_vtop(void *vaddr)
 static inline void *
 dpaa_mem_ptov(rte_iova_t paddr)
 {
+   void *va;
+
+   va = (void *)dpaax_iova_table_get_va(paddr);
+   if (likely(va))
+   return va;
+
return rte_mem_iova2virt(paddr);
 }
 
diff --git a/drivers/event/dpaa/Makefile b/drivers/event/dpaa/Makefile
index ddd855227..6f93e7f40 100644
--- a/drivers/event/dpaa/Makefile
+++ b/drivers/event/dpaa/Makefile
@@ -34,5 +34,6 @@ LDLIBS += -lrte_mempool_dpaa
 LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring
 LDLIBS += -lrte_ethdev -lrte_net -lrte_kvargs
 LDLIBS += -lrte_eventdev -lrte_pmd_dpaa -lrte_bus_vdev
+LDLIBS += -lrte_common_dpaax
 
 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/mempool/dpaa/Makefile b/drivers/mempool/dpaa/Makefile
index da8da1e90..9cf36856c 100644
--- a/drivers/mempool/dpaa/Makefile
+++ b/drivers/mempool/dpaa/Makefile
@@ -31,5 +31,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_DPAA_MEMPOOL) += dpaa_mempool.c
 
 LDLIBS += -lrte_bus_dpaa
 LDLIBS += -lrte_eal -lrte_mempool -lrte_ring
+LDLIBS += -lrte_common_dpaax
 
 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/mempool/dpaa/dpaa_mempool.c 
b/drivers/mempool/dpaa/dpaa_mempool.c
index 1c121223b..b05fb7b9d 100644
--- a/drivers/mempool/dpaa/dpaa_mempool.c
+++ b/drivers/mempool/dpaa/dpaa_mempool.c
@@ -26,6 +26,7 @@
 #include 
 
 #include 
+#include 
 

[dpdk-dev] [PATCH v4 3/5] common/dpaax: add library for PA VA translation table

[Shreyansh Jain]

A common library, valid for dpaaX drivers, which is used to maintain
a local copy of PA->VA translations.

In case of physical addressing mode (one of the option for FSLMC, and
only option for DPAA bus), the addresses of descriptors Rx'd are
physical. These need to be converted into equivalent VA for rte_mbuf
and other similar calls.

Using the rte_mem_virt2iova or rte_mem_virt2phy is expensive. This
library is an attempt to reduce the overall cost associated with
this translation.

A small table is maintained, containing continuous entries
representing a continguous physical range. Each of these entries
stores the equivalent VA, which is fed during mempool creation, or
memory allocation/deallocation callbacks.

Signed-off-by: Shreyansh Jain 
---
 config/common_base|   5 +
 config/common_linuxapp|   5 +
 drivers/common/Makefile   |   4 +
 drivers/common/dpaax/Makefile |  31 ++
 drivers/common/dpaax/dpaax_iova_table.c   | 461 ++
 drivers/common/dpaax/dpaax_iova_table.h   | 103 
 drivers/common/dpaax/dpaax_logs.h |  39 ++
 drivers/common/dpaax/meson.build  |  12 +
 .../common/dpaax/rte_common_dpaax_version.map |  11 +
 drivers/common/meson.build|   2 +-
 10 files changed, 672 insertions(+), 1 deletion(-)
 create mode 100644 drivers/common/dpaax/Makefile
 create mode 100644 drivers/common/dpaax/dpaax_iova_table.c
 create mode 100644 drivers/common/dpaax/dpaax_iova_table.h
 create mode 100644 drivers/common/dpaax/dpaax_logs.h
 create mode 100644 drivers/common/dpaax/meson.build
 create mode 100644 drivers/common/dpaax/rte_common_dpaax_version.map

diff --git a/config/common_base b/config/common_base
index 8c7ead68d..7f10f7215 100644
--- a/config/common_base
+++ b/config/common_base
@@ -139,6 +139,11 @@ CONFIG_RTE_ETHDEV_PROFILE_WITH_VTUNE=n
 #
 CONFIG_RTE_ETHDEV_TX_PREPARE_NOOP=n
 
+#
+# Common libraries, before Bus/PMDs
+#
+CONFIG_RTE_LIBRTE_COMMON_DPAAX=n
+
 #
 # Compile the Intel FPGA bus
 #
diff --git a/config/common_linuxapp b/config/common_linuxapp
index 485e1467d..76b884c48 100644
--- a/config/common_linuxapp
+++ b/config/common_linuxapp
@@ -29,6 +29,11 @@ CONFIG_RTE_PROC_INFO=y
 CONFIG_RTE_LIBRTE_VMBUS=y
 CONFIG_RTE_LIBRTE_NETVSC_PMD=y
 
+#
+# Common libraries, before Bus/PMDs
+#
+CONFIG_RTE_LIBRTE_COMMON_DPAAX=y
+
 # NXP DPAA BUS and drivers
 CONFIG_RTE_LIBRTE_DPAA_BUS=y
 CONFIG_RTE_LIBRTE_DPAA_MEMPOOL=y
diff --git a/drivers/common/Makefile b/drivers/common/Makefile
index b498c238f..6392a3412 100644
--- a/drivers/common/Makefile
+++ b/drivers/common/Makefile
@@ -14,4 +14,8 @@ ifneq (,$(findstring y,$(MVEP-y)))
 DIRS-y += mvep
 endif
 
+ifeq ($(CONFIG_RTE_LIBRTE_COMMON_DPAAX),y)
+DIRS-y += dpaax
+endif
+
 include $(RTE_SDK)/mk/rte.subdir.mk
diff --git a/drivers/common/dpaax/Makefile b/drivers/common/dpaax/Makefile
new file mode 100644
index 0..94d2cf0ce
--- /dev/null
+++ b/drivers/common/dpaax/Makefile
@@ -0,0 +1,31 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright 2018 NXP
+#
+
+include $(RTE_SDK)/mk/rte.vars.mk
+
+#
+# library name
+#
+LIB = librte_common_dpaax.a
+
+CFLAGS += -DALLOW_EXPERIMENTAL_API
+CFLAGS += -O3
+CFLAGS += $(WERROR_FLAGS)
+
+# versioning export map
+EXPORT_MAP := rte_common_dpaax_version.map
+
+# library version
+LIBABIVER := 1
+
+#
+# all source are stored in SRCS-y
+#
+SRCS-y += dpaax_iova_table.c
+
+LDLIBS += -lrte_eal
+
+SYMLINK-y-include += dpaax_iova_table.h
+
+include $(RTE_SDK)/mk/rte.lib.mk
\ No newline at end of file
diff --git a/drivers/common/dpaax/dpaax_iova_table.c 
b/drivers/common/dpaax/dpaax_iova_table.c
new file mode 100644
index 0..d54267bb7
--- /dev/null
+++ b/drivers/common/dpaax/dpaax_iova_table.c
@@ -0,0 +1,461 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright 2018 NXP
+ */
+
+#include 
+
+#include "dpaax_iova_table.h"
+#include "dpaax_logs.h"
+
+/* Global dpaax logger identifier */
+int dpaax_logger;
+
+/* Global table reference */
+struct dpaax_iova_table *dpaax_iova_table_p;
+
+static int dpaax_handle_memevents(void);
+
+/* A structure representing the device-tree node available in 
/proc/device-tree.
+ */
+struct reg_node {
+   phys_addr_t addr;
+   size_t len;
+};
+
+/* A ntohll equivalent routine
+ * XXX: This is only applicable for 64 bit environment.
+ */
+static void
+rotate_8(unsigned char *arr)
+{
+   uint32_t temp;
+   uint32_t *first_half;
+   uint32_t *second_half;
+
+   first_half = (uint32_t *)(arr);
+   second_half = (uint32_t *)(arr + 4);
+
+   temp = *first_half;
+   *first_half = *second_half;
+   *second_half = temp;
+
+   *first_half = ntohl(*first_half);
+   *second_half = ntohl(*second_half);
+}
+
+/* read_memory_nodes
+ * Memory layout for DPAAx platforms (LS1043, LS1046, LS1088, LS2088, LX2160)
+ * are populated by Uboot and available in device tree:
+ * /proc/device-tree/memory@/reg <= register.

[dpdk-dev] [PATCH v4 2/5] drivers: common as dependency for bus

[Shreyansh Jain]

Prior to this patch, bus and common compiled parallel. But, post this
dependency is created.

This is especially important for the DPAA/FSLMC buses which are going
to use the common/dpaax library.

Signed-off-by: Shreyansh Jain 
---
 drivers/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/Makefile b/drivers/Makefile
index 75660765e..7d5da5d9f 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -5,6 +5,7 @@ include $(RTE_SDK)/mk/rte.vars.mk
 
 DIRS-y += common
 DIRS-y += bus
+DEPDIRS-bus := common
 DIRS-y += mempool
 DEPDIRS-mempool := common bus
 DIRS-y += net
-- 
2.17.1

[dpdk-dev] [PATCH v4 1/5] bus/fslmc: fix physical addressing check

[Shreyansh Jain]

In case RTE_LIBRTE_DPAA2_USE_PHYS_IOVA is enabled, only supported
class is RTE_IOVA_PA.

Fixes: f7768afac101 ("bus/fslmc: support dynamic IOVA")
Cc: hemant.agra...@nxp.com

Signed-off-by: Shreyansh Jain 
---
 drivers/bus/fslmc/fslmc_bus.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
index 960f55071..2bc9457bc 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -496,6 +496,10 @@ rte_dpaa2_get_iommu_class(void)
if (TAILQ_EMPTY(&rte_fslmc_bus.device_list))
return RTE_IOVA_DC;
 
+#ifdef RTE_LIBRTE_DPAA2_USE_PHYS_IOVA
+   return RTE_IOVA_PA;
+#endif
+
/* check if all devices on the bus support Virtual addressing or not */
has_iova_va = fslmc_all_device_support_iova();
 
-- 
2.17.1

[dpdk-dev] [PATCH v4 0/5] Add a PA-VA Translation table for DPAAx

[Shreyansh Jain]

::Background::
After the restructuring of memory in last release(s), one of the
major impact on fslmc/dpaa bus (and its devices) was the performance
drop when using physical addressing.

Previously, it was assumed that physical range was contiguous for
any given request for hugepage memory. That way, whenever a
virtual address was returned, it was easy to fetch physical
equivalent, in almost constant time.
But, with memory hotplug series, that assumption was negated.
Every call that device drivers made for rte_mem_virt2iova or
rte_mem_virt2phy were expensive. (Using IOVA_CONTIG is an app
dependency which is not a practical option).

For fslmc, working on Physical or Virtual (IOMMU supported) address
is an optional thing. For dpaa bus, it is not optional and only
physical addressing is supported. Thus, it impacted dpaa bus
the most.

::DPAAX PA-VA Table::
- A simple table containing entries for all physical memory range
  available on a particular SoC (in this case, NXP's LS104x and
  LS20xx series, which are handled by dpaa and fslmc bus,
  respectively). As of now, this is SoC dependent for fetching
  range.
- We populate the table either through the mempool handler (for
  mempool pinned memory) or through the memory event callbacks
  (for cases where working memory is allocated by application).
- Though aim is only to translate addresses for descriptors
  which are Rx'd from devices, this is a generic layer which
  should work in other cases as well (though, not the target of
  current testing).

::About patches::
Patch 1: There was an issue in existing PA/VA mode reporting being
 done by fslmc bus. This patch fixes it.
Patch 2: Common libraries/commponents can be dependency for the bus
 thus, blocking parallel compilation
Patch 3: Add the library in common/dpaax. This is a single patch
 as functions are mostly inter-linked.
Patch 4~5: Add support in dpaa and fslmc bus, respectively.
 It is not possible to unlink the bus and device drivers,
 thus, these patches have blanket change across all drivers.

::Next Steps::
- Some optimization are required to tune the access pattern of the
  table. These would be posted as additional patches.
- In case there is any possible split of patches, I will post another
  version. But until then, this is the layout.

::Version History::
v3->v4:
 - Fixed missing rework against review comment from Pavan: shift the
   IOVA mode detection code in bus/fslmc
 - Rebased over master (abe92131c92)

v2->v3:
 - Added back del operation (update) for mem-events, which was removed
   in v2
 - Change IOMMU(PA) detection for FSLMC Bus (review comment: Pavan)
 - Rebase on master (6673fe0ce2)

v1->v2:
 - Rework of review comments on v1
 - Removed dpaax_iova_table_del API - that is redundant
 - Changed paax_iova_table_add to paax_iova_table_update to make
   it more relevant
 - Previous patch removed an advertised API (rte_dpaa2_memsegs). This is
   fixed. A deprecation notice would now be sent for removal in next
   release.
 - Rebase on master (5f73c2670f); Also verified on net-next/mater (317f8b01f)

Shreyansh Jain (5):
  bus/fslmc: fix physical addressing check
  drivers: common as dependency for bus
  common/dpaax: add library for PA VA translation table
  dpaa: enable dpaax library
  fslmc: enable dpaax library

 config/common_base|   5 +
 config/common_linuxapp|   5 +
 drivers/Makefile  |   1 +
 drivers/bus/dpaa/Makefile |   1 +
 drivers/bus/dpaa/dpaa_bus.c   |   4 +
 drivers/bus/dpaa/meson.build  |   2 +-
 drivers/bus/dpaa/rte_dpaa_bus.h   |   6 +
 drivers/bus/fslmc/Makefile|   1 +
 drivers/bus/fslmc/fslmc_bus.c |  24 +
 drivers/bus/fslmc/meson.build |   2 +-
 drivers/bus/fslmc/portal/dpaa2_hw_pvt.h   |  21 +-
 drivers/common/Makefile   |   4 +
 drivers/common/dpaax/Makefile |  31 ++
 drivers/common/dpaax/dpaax_iova_table.c   | 461 ++
 drivers/common/dpaax/dpaax_iova_table.h   | 103 
 drivers/common/dpaax/dpaax_logs.h |  39 ++
 drivers/common/dpaax/meson.build  |  12 +
 .../common/dpaax/rte_common_dpaax_version.map |  11 +
 drivers/common/meson.build|   2 +-
 drivers/crypto/dpaa2_sec/Makefile |   1 +
 drivers/crypto/dpaa_sec/Makefile  |   1 +
 drivers/crypto/dpaa_sec/dpaa_sec.c|   6 +
 drivers/event/dpaa/Makefile   |   1 +
 drivers/event/dpaa2/Makefile  |   1 +
 drivers/mempool/dpaa/Makefile |   1 +
 drivers/mempool/dpaa/dpaa_mempool.c   |   4 +
 drivers/mempool/dpaa/dpaa_mempool.h   |   4 +-
 drivers/mempool/dpaa2/Makefile|   1 +
 drivers/mempool/dpaa2/dpaa2_hw_mempool.c  |  29 +-
 drivers/net/dpaa/Makefile   

[dpdk-dev] [PATCH v4 5/5] fslmc: enable dpaax library

[Shreyansh Jain]

With this patch, fslmc bus and ethernet devices on this bus
would start using the physical-virtual library interfaces.

This patch impacts mempool/dpaa2, event/dpaa2, net/dpaa2,
raw/dpaa2_cmdif and raw/dpaa2_qdma as they are dependent
on the bus/fslmc and thus impact linkage of libraries.

Signed-off-by: Shreyansh Jain 
---
 drivers/bus/fslmc/Makefile   |  1 +
 drivers/bus/fslmc/fslmc_bus.c| 20 
 drivers/bus/fslmc/meson.build|  2 +-
 drivers/bus/fslmc/portal/dpaa2_hw_pvt.h  | 21 -
 drivers/crypto/dpaa2_sec/Makefile|  1 +
 drivers/event/dpaa2/Makefile |  1 +
 drivers/mempool/dpaa2/Makefile   |  1 +
 drivers/mempool/dpaa2/dpaa2_hw_mempool.c | 29 
 drivers/net/dpaa2/Makefile   |  1 +
 drivers/raw/dpaa2_cmdif/Makefile |  1 +
 drivers/raw/dpaa2_qdma/Makefile  |  1 +
 mk/rte.app.mk|  1 +
 12 files changed, 43 insertions(+), 37 deletions(-)

diff --git a/drivers/bus/fslmc/Makefile b/drivers/bus/fslmc/Makefile
index e95551980..218d9bd28 100644
--- a/drivers/bus/fslmc/Makefile
+++ b/drivers/bus/fslmc/Makefile
@@ -19,6 +19,7 @@ CFLAGS += -I$(RTE_SDK)/drivers/bus/fslmc/qbman/include
 CFLAGS += -I$(RTE_SDK)/lib/librte_eal/common
 LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring
 LDLIBS += -lrte_ethdev
+LDLIBS += -lrte_common_dpaax
 
 # versioning export map
 EXPORT_MAP := rte_bus_fslmc_version.map
diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
index 2bc9457bc..5ba5ce96b 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -20,6 +20,8 @@
 #include 
 #include "fslmc_logs.h"
 
+#include 
+
 int dpaa2_logtype_bus;
 
 #define VFIO_IOMMU_GROUP_PATH "/sys/kernel/iommu_groups"
@@ -377,6 +379,19 @@ rte_fslmc_probe(void)
 
probe_all = rte_fslmc_bus.bus.conf.scan_mode != RTE_BUS_SCAN_WHITELIST;
 
+   /* In case of PA, the FD addresses returned by qbman APIs are physical
+* addresses, which need conversion into equivalent VA address for
+* rte_mbuf. For that, a table (a serial array, in memory) is used to
+* increase translation efficiency.
+* This has to be done before probe as some device initialization
+* (during) probe allocate memory (dpaa2_sec) which needs to be pinned
+* to this table.
+*/
+   ret = dpaax_iova_table_populate();
+   if (ret) {
+   DPAA2_BUS_WARN("PA->VA Translation table not available;");
+   }
+
TAILQ_FOREACH(dev, &rte_fslmc_bus.device_list, next) {
TAILQ_FOREACH(drv, &rte_fslmc_bus.driver_list, next) {
ret = rte_fslmc_match(drv, dev);
@@ -456,6 +471,11 @@ rte_fslmc_driver_unregister(struct rte_dpaa2_driver 
*driver)
 
fslmc_bus = driver->fslmc_bus;
 
+   /* Cleanup the PA->VA Translation table; From whereever this function
+* is called from.
+*/
+   dpaax_iova_table_depopulate();
+
TAILQ_REMOVE(&fslmc_bus->driver_list, driver, next);
/* Update Bus references */
driver->fslmc_bus = NULL;
diff --git a/drivers/bus/fslmc/meson.build b/drivers/bus/fslmc/meson.build
index 54ca92d0c..18c45495b 100644
--- a/drivers/bus/fslmc/meson.build
+++ b/drivers/bus/fslmc/meson.build
@@ -7,7 +7,7 @@ if host_machine.system() != 'linux'
 build = false
 endif
 
-deps += ['eventdev', 'kvargs']
+deps += ['common_dpaax', 'eventdev', 'kvargs']
 sources = files('fslmc_bus.c',
'fslmc_vfio.c',
'mc/dpbp.c',
diff --git a/drivers/bus/fslmc/portal/dpaa2_hw_pvt.h 
b/drivers/bus/fslmc/portal/dpaa2_hw_pvt.h
index 820759360..678ee34b8 100644
--- a/drivers/bus/fslmc/portal/dpaa2_hw_pvt.h
+++ b/drivers/bus/fslmc/portal/dpaa2_hw_pvt.h
@@ -9,6 +9,7 @@
 #define _DPAA2_HW_PVT_H_
 
 #include 
+#include 
 
 #include 
 #include 
@@ -275,28 +276,26 @@ extern struct dpaa2_memseg_list rte_dpaa2_memsegs;
 #ifdef RTE_LIBRTE_DPAA2_USE_PHYS_IOVA
 extern uint8_t dpaa2_virt_mode;
 static void *dpaa2_mem_ptov(phys_addr_t paddr) __attribute__((unused));
-/* todo - this is costly, need to write a fast coversion routine */
+
 static void *dpaa2_mem_ptov(phys_addr_t paddr)
 {
-   struct dpaa2_memseg *ms;
+   void *va;
 
if (dpaa2_virt_mode)
return (void *)(size_t)paddr;
 
-   /* Check if the address is already part of the memseg list internally
-* maintained by the dpaa2 driver.
-*/
-   TAILQ_FOREACH(ms, &rte_dpaa2_memsegs, next) {
-   if (paddr >= ms->iova && paddr <
-   ms->iova + ms->len)
-   return RTE_PTR_ADD(ms->vaddr, (uintptr_t)(paddr - 
ms->iova));
-   }
+   va = (void *)dpaax_iova_table_get_va(paddr);
+   if (likely(va != NULL))
+   return va;
 
/* If not, Fallback to full memseg list searching */
-   return rte_mem_iova2virt(paddr);
+   va =