Re: [dpdk-dev] [PATCH v2 2/6] ethdev: add port ownership

[Matan Azrad]

Hi Konstantin
From: Ananyev, Konstantin, Monday, January 15, 2018 8:44 PM
> Hi Matan,
> > Hi Konstantin
> > From: Ananyev, Konstantin, Monday, January 15, 2018 1:45 PM
> > > Hi Matan,
> > > > Hi Konstantin
> > > > From: Ananyev, Konstantin, Friday, January 12, 2018 2:02 AM
> > > > > Hi Matan,
> > > > > > Hi Konstantin
> > > > > > From: Ananyev, Konstantin, Thursday, January 11, 2018 2:40 PM
> > > > > > > Hi Matan,
> > > > > > > > Hi Konstantin
> > > > > > > > From: Ananyev, Konstantin, Wednesday, January 10, 2018
> > > > > > > > 3:36 PM
> > > > > > > > > Hi Matan,
 
> > > > > > > > > It is good to see that now scanning/updating
> > > > > > > > > rte_eth_dev_data[] is lock protected, but it might be
> > > > > > > > > not very plausible to protect both data[] and
> > > > > > > > > next_owner_id using the
> > > same lock.
> > > > > > > >
> > > > > > > > I guess you mean to the owner structure in
> > > rte_eth_dev_data[port_id].
> > > > > > > > The next_owner_id is read by ownership APIs(for owner
> > > > > > > > validation), so it
> > > > > > > makes sense to use the same lock.
> > > > > > > > Actually, why not?
> > > > > > >
> > > > > > > Well to me next_owner_id and rte_eth_dev_data[] are not
> > > > > > > directly
> > > > > related.
> > > > > > > You may create new owner_id but it doesn't mean you would
> > > > > > > update rte_eth_dev_data[] immediately.
> > > > > > > And visa-versa - you might just want to update
> > > > > > > rte_eth_dev_data[].name or .owner_id.
> > > > > > > It is not very good coding practice to use same lock for
> > > > > > > non-related data structures.
> > > > > > >
> > > > > > I see the relation like next:
> > > > > > Since the ownership mechanism synchronization is in ethdev
> > > > > > responsibility, we must protect against user mistakes as much
> > > > > > as we can by
> > > > > using the same lock.
> > > > > > So, if user try to set by invalid owner (exactly the ID which
> > > > > > currently is
> > > > > allocated) we can protect on it.
> > > > >
> > > > > Hmm, not sure why you can't do same checking with different lock
> > > > > or atomic variable?
> > > > >
> > > > The set ownership API is protected by ownership lock and checks
> > > > the owner ID validity By reading the next owner ID.
> > > > So, the owner ID allocation and set API should use the same atomic
> > > mechanism.
> > >
> > > Sure but all you are doing for checking validity, is  check that
> > > owner_id > 0 &&& owner_id < next_ownwe_id, right?
> > > As you don't allow owner_id overlap (16/3248 bits) you can safely do
> > > same check with just atomic_get(&next_owner_id).
> > >
> > It will not protect it, scenario:
> > - current next_id is X.
> > - call set ownership of port A with owner id X by thread 0(by user mistake).
> > - context switch 
> > - allocate new id by thread 1 and get X and change next_id to X+1
> atomically.
> > -  context switch
> > - Thread 0 validate X by atomic_read and succeed to take ownership.
> > - The system loosed the port(or will be managed by two entities) - crash.
> 
> 
> Ok, and how using lock will protect you with such scenario?

The owner set API validation by thread 0 should fail because the owner 
validation is included in the protected section.

> I don't think you can protect yourself against such scenario with or without
> locking.
> Unless you'll make it harder for the mis-behaving thread to guess valid
> owner_id, or add some extra logic here.
> 
> >
> >
> > > > The set(and others) ownership APIs already uses the ownership lock
> > > > so I
> > > think it makes sense to use the same lock also in ID allocation.
> > > >
> > > > > > > > > In fact, for next_owner_id, you don't need a lock - just
> > > > > > > > > rte_atomic_t should be enough.
> > > > > > > >
> > > > > > > > I don't think so, it is problematic in next_owner_id
> > > > > > > > wraparound and may
> > > > > > > complicate the code in other places which read it.
> > > > > > >
> > > > > > > IMO it is not that complicated, something like that should work I
> think.
> > > > > > >
> > > > > > > /* init to 0 at startup*/
> > > > > > > rte_atomic32_t *owner_id;
> > > > > > >
> > > > > > > int new_owner_id(void)
> > > > > > > {
> > > > > > > int32_t x;
> > > > > > > x = rte_atomic32_add_return(&owner_id, 1);
> > > > > > > if (x > UINT16_MAX) {
> > > > > > >rte_atomic32_dec(&owner_id);
> > > > > > >return -EOVERWLOW;
> > > > > > > } else
> > > > > > > return x;
> > > > > > > }
> > > > > > >
> > > > > > >
> > > > > > > > Why not just to keep it simple and using the same lock?
> > > > > > >
> > > > > > > Lock is also fine, I just think it better be a separate one
> > > > > > > - that would protext just next_owner_id.
> > > > > > > Though if you are going to use uuid here - all that probably
> > > > > > > not relevant any more.
> > > > > > >
> > > > > >
> > > > > > I agree about the uuid but still think the same lock should be
> > > > > > used for
> > > both.
> > > > >
> > > > > B

Re: [dpdk-dev] [PATCH 4/4] net/mlx5: remove MR refcnt

[Nélio Laranjeiro]

Hi Xueming,

On Mon, Jan 15, 2018 at 02:54:20PM +0800, Xueming Li wrote:
> MRs are registered to priv at device start or on the fly, destroyied
> when device stop.
> No mR registration to perticular TXQ, MR references cache in TXQ just
> part of device MR list, no reason to keep MR refcnt.
> 
> Signed-off-by: Xueming Li 
> ---
>  drivers/net/mlx5/mlx5.h  |  1 -
>  drivers/net/mlx5/mlx5_mr.c   | 51 
> +---
>  drivers/net/mlx5/mlx5_rxq.c  |  1 -
>  drivers/net/mlx5/mlx5_rxtx.h |  2 --
>  drivers/net/mlx5/mlx5_txq.c  | 18 
>  5 files changed, 5 insertions(+), 68 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
> index a6cd1474c..df6a70d96 100644
> --- a/drivers/net/mlx5/mlx5.h
> +++ b/drivers/net/mlx5/mlx5.h
> @@ -318,7 +318,6 @@ int priv_socket_connect(struct priv *priv);
>  
>  struct mlx5_mr *priv_mr_new(struct priv *priv, struct rte_mempool *mp,
>   uintptr_t addr);
> -struct mlx5_mr *priv_mr_get(struct priv *, struct rte_mempool *);
>  int priv_mr_release(struct priv *, struct mlx5_mr *);
>  int priv_mr_verify(struct priv *);
>  struct mlx5_mr *priv_mr_lookup(struct priv *priv, uintptr_t addr);
> diff --git a/drivers/net/mlx5/mlx5_mr.c b/drivers/net/mlx5/mlx5_mr.c
> index 8d8fabea1..bd1be5606 100644
> --- a/drivers/net/mlx5/mlx5_mr.c
> +++ b/drivers/net/mlx5/mlx5_mr.c
> @@ -88,15 +88,11 @@ mlx5_mempool_register_cb(struct rte_mempool *mp, void 
> *opaque,
>   cb->error = -1;
>   return;
>   }
> - mr->mp = mp;
>   DEBUG("mempool %p:%u area start=%p size=%zu lkey=0x%08x",
> (void *)mp, mem_idx, memhdr->addr, memhdr->len, mr->mr->lkey);
>   mr->lkey = rte_cpu_to_be_32(mr->mr->lkey);
>   mr->start = (uintptr_t)memhdr->addr;
>   mr->end = (uintptr_t)mr->mr->addr + mr->mr->length;
> - rte_atomic32_inc(&mr->refcnt);
> - DEBUG("%p: new Memory Region %p refcnt: %d", (void *)priv,
> -   (void *)mr, rte_atomic32_read(&mr->refcnt));

Please keep the debug message.

>   LIST_INSERT_HEAD(&priv->mr, mr, next);
>   cb->mr = mr;
>  }
> @@ -121,11 +117,8 @@ mlx5_mp2mr(struct priv *priv, struct rte_mempool *mp, 
> uintptr_t addr)
>  
>   priv_lock(priv);
>   mr = priv_mr_lookup(priv, addr);
> - if (!mr) {
> + if (!mr)
>   mr = priv_mr_new(priv, mp, addr);
> - if (mr)
> - rte_atomic32_inc(&mr->refcnt);
> - }
>   priv_unlock(priv);
>   return mr;
>  }
> @@ -217,35 +210,6 @@ priv_mr_new(struct priv *priv, struct rte_mempool *mp, 
> uintptr_t addr)
>  }
>  
>  /**
> - * Search the memory region object in the memory region list.
> - *
> - * @param  priv
> - *   Pointer to private structure.
> - * @param mp
> - *   Pointer to the memory pool to register.
> - * @return
> - *   The memory region on success.
> - */
> -struct mlx5_mr*
> -priv_mr_get(struct priv *priv, struct rte_mempool *mp)
> -{
> - struct mlx5_mr *mr;
> -
> - assert(mp);
> - if (LIST_EMPTY(&priv->mr))
> - return NULL;
> - LIST_FOREACH(mr, &priv->mr, next) {
> - if (mr->mp == mp) {
> - rte_atomic32_inc(&mr->refcnt);
> - DEBUG("Memory Region %p refcnt: %d",
> -   (void *)mr, rte_atomic32_read(&mr->refcnt));
> - return mr;
> - }
> - }
> - return NULL;
> -}
> -
> -/**
>   * Release the memory region object.
>   *
>   * @param  mr
> @@ -259,15 +223,10 @@ priv_mr_release(struct priv *priv, struct mlx5_mr *mr)
>  {
>   (void)priv;
>   assert(mr);
> - DEBUG("Memory Region %p refcnt: %d",
> -   (void *)mr, rte_atomic32_read(&mr->refcnt));
> - if (rte_atomic32_dec_and_test(&mr->refcnt)) {
> - claim_zero(ibv_dereg_mr(mr->mr));
> - LIST_REMOVE(mr, next);
> - rte_free(mr);
> - return 0;
> - }
> - return EBUSY;
> + claim_zero(ibv_dereg_mr(mr->mr));
> + LIST_REMOVE(mr, next);
> + rte_free(mr);
> + return 0;
>  }
>  
>  /**
> diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
> index a581a2d61..461b4ff91 100644
> --- a/drivers/net/mlx5/mlx5_rxq.c
> +++ b/drivers/net/mlx5/mlx5_rxq.c
> @@ -852,7 +852,6 @@ mlx5_priv_rxq_ibv_get(struct priv *priv, uint16_t idx)
>   return NULL;
>   rxq_ctrl = container_of(rxq_data, struct mlx5_rxq_ctrl, rxq);
>   if (rxq_ctrl->ibv) {
> - priv_mr_get(priv, rxq_data->mp);
>   rte_atomic32_inc(&rxq_ctrl->ibv->refcnt);
>   DEBUG("%p: Verbs Rx queue %p: refcnt %d", (void *)priv,
> (void *)rxq_ctrl->ibv,
> diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h
> index 6589a662e..0b8332cc0 100644
> --- a/drivers/net/mlx5/mlx5_rxtx.h
> +++ b/drivers/net/mlx5/mlx5_rxtx.h
> @@ -85,12 +85,10 @@ struct priv;
>  /* Memory region queue object. */
>  s

Re: [dpdk-dev] [PATCH v2 3/4] eal: add synchronous multi-process communication

[Tan, Jianfeng]

Thank you, Konstantin and Anatoly firstly. Other comments are well 
received and I'll send out a new version.



On 1/16/2018 8:00 AM, Ananyev, Konstantin wrote:



We need the synchronous way for multi-process communication,
i.e., blockingly waiting for reply message when we send a request
to the peer process.

We add two APIs rte_eal_mp_request() and rte_eal_mp_reply() for
such use case. By invoking rte_eal_mp_request(), a request message
is sent out, and then it waits there for a reply message. The
timeout is hard-coded 5 Sec. And the replied message will be copied
in the parameters of this API so that the caller can decide how
to translate those information (including params and fds). Note
if a primary process owns multiple secondary processes, this API
will fail.

The API rte_eal_mp_reply() is always called by an mp action handler.
Here we add another parameter for rte_eal_mp_t so that the action
handler knows which peer address to reply.

We use mutex in rte_eal_mp_request() to guarantee that only one
request is on the fly for one pair of processes.

You don't need to do things in such strange and restrictive way.
Instead you can do something like that:
1) Introduce new struct, list for it and mutex
  struct sync_request {
   int reply_received;
   char dst[PATH_MAX];
   char reply[...];
   LIST_ENTRY(sync_request) next;
};

static struct
 LIST_HEAD(list, sync_request);
 pthread_mutex_t lock;
pthead_cond_t cond;
} sync_requests;

2) then at request() call:
   Grab sync_requests.lock
   Check do we already have a pending request for that destination,
   If yes - the release the lock and returns with error.
   - allocate and init new sync_request struct, set reply_received=0
   - do send_msg()
   -then in a cycle:
   pthread_cond_timed_wait(&sync_requests.cond, &sync_request.lock, ×pec);
   - at return from it check if sync_request.reply_received == 1, if not
check if timeout expired and either return a failure or go to the start of the 
cycle.

3) at mp_handler() if REPLY received - grab sync_request.lock,
 search through sync_requests.list for dst[] ,
if found, then set it's reply_received=1, copy the received message into 
reply
and call pthread_cond_braodcast((&sync_requests.cond);


The only benefit I can see is that now the sender can request to 
multiple receivers at the same time. And it makes things more 
complicated. Do we really need this?


Thanks,
Jianfeng

Re: [dpdk-dev] [PATCH v2 6/6] app/testpmd: adjust ethdev port ownership

[Matan Azrad]

Hi Lu
From: Lu, Wenzhuo, Tuesday, January 16, 2018 7:54 AM
> Hi Matan,
> 
> 
> > -Original Message-
> > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Matan Azrad
> > Sent: Sunday, January 7, 2018 5:46 PM
> > To: Thomas Monjalon ; Gaetan Rivet
> > ; Wu, Jingjing 
> > Cc: dev@dpdk.org; Neil Horman ; Richardson,
> > Bruce ; Ananyev, Konstantin
> > 
> > Subject: [dpdk-dev] [PATCH v2 6/6] app/testpmd: adjust ethdev port
> > ownership
> >
> > Testpmd should not use ethdev ports which are managed by other DPDK
> > entities.
> >
> > Set Testpmd ownership to each port which is not used by other entity
> > and prevent any usage of ethdev ports which are not owned by Testpmd.
> Sorry I don't follow all the discussion as there's too much. So it may be a 
> silly
> question.

No problem, I'm here for any question :)

> Testpmd already has the parameter " --pci-whitelist" to only use the assigned
> devices.

It is an EAL parameter. No? just say to EAL which devices to create..

> When using this parameter, all the devices are owned by the current
> APP.

No, what's about vdev? vdevs may manage devices(even whitlist PCI devices) by 
themselves and want to prevent any app to use these devices(see fail-safe PMD).

 > So I don't know why need to set/check the ownership.
> BTW, in this patch, seem all the change is for ownership checking. I don't 
> find
> the setting code. Do I miss something?

Yes, see in main function (the first FOREACH).

Thanks, Matan.

Re: [dpdk-dev] [PATCH 3/3] app/testpmd: set preferred mempool as default pktpool

[Pavan Nikhilesh]

Hi Lu,

On Tue, Jan 16, 2018 at 06:06:50AM +, Lu, Wenzhuo wrote:
> Hi Pavan,
>
> > -Original Message-
> > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Pavan Nikhilesh
> > Sent: Thursday, December 14, 2017 3:56 AM
> > To: jerin.ja...@caviumnetworks.com; Wu, Jingjing ;
> > Richardson, Bruce ; hemant.agra...@nxp.com;
> > Yigit, Ferruh ; tho...@monjalon.net;
> > olivier.m...@6wind.com
> > Cc: dev@dpdk.org; Pavan Nikhilesh 
> > Subject: [dpdk-dev] [PATCH 3/3] app/testpmd: set preferred mempool as
> > default pktpool
> >
> > Set the mempool preferred by the ethernet devices as default mbuf
> > mempool before creating the pktpool.
> >
> > Signed-off-by: Pavan Nikhilesh 
> > ---
> >  app/test-pmd/testpmd.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > c3ab44849..bfea35613 100644
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -533,6 +533,8 @@ mbuf_pool_create(uint16_t mbuf_seg_size, unsigned
> > nb_mbuf,
> > rte_mempool_obj_iter(rte_mp, rte_pktmbuf_init, NULL);
> > } else {
> > /* wrapper to rte_mempool_create() */
> > +   RTE_LOG(INFO, USER1, "preferred mempool ops %s
> > selected\n",
> > +   rte_eth_dev_get_preferred_pool_name(1));
> This '1'  looks like hardcode. May I suggest to change it to 'true'?

This patchset will be superseeded by http://dpdk.org/dev/patchwork/patch/33716/
I will mark the same.

Thanks,
Pavan.

Re: [dpdk-dev] [PATCH] net/mlx5: fix handling link status change event

[Nélio Laranjeiro]

Hi Yongseok,

Small nips.

On Mon, Jan 15, 2018 at 03:48:00PM -0800, Yongseok Koh wrote:
> Even though link of a port gets down, device still can receive traffic.
> That is the reason why mlx5_set_link_up/down() switches rx/tx_pkt_burst().
> However, if link gets down by an external command (e.g. ifconfig), it isn't
> effective. It is better to change burst functions when link status change
> is detected.
> 
> Fixes: 62072098b54e ("mlx5: support setting link up or down")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Yongseok Koh 
> ---
>  drivers/net/mlx5/mlx5.c |   1 -
>  drivers/net/mlx5/mlx5.h |   1 +
>  drivers/net/mlx5/mlx5_ethdev.c  | 112 
> +++-
>  drivers/net/mlx5/mlx5_trigger.c |  23 ++---
>  4 files changed, 93 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
> index 1c95f3520..fc2d59fee 100644
> --- a/drivers/net/mlx5/mlx5.c
> +++ b/drivers/net/mlx5/mlx5.c
> @@ -869,7 +869,6 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv, struct 
> rte_pci_device *pci_dev)
>   /* Bring Ethernet device up. */
>   DEBUG("forcing Ethernet interface up");
>   priv_set_flags(priv, ~IFF_UP, IFF_UP);
> - mlx5_link_update(priv->dev, 1);
>   /* Store device configuration on private structure. */
>   priv->config = config;
>   continue;
> diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
> index e740a4e77..9a4a59bd4 100644
> --- a/drivers/net/mlx5/mlx5.h
> +++ b/drivers/net/mlx5/mlx5.h
> @@ -225,6 +225,7 @@ int priv_set_flags(struct priv *, unsigned int, unsigned 
> int);
>  int mlx5_dev_configure(struct rte_eth_dev *);
>  void mlx5_dev_infos_get(struct rte_eth_dev *, struct rte_eth_dev_info *);
>  const uint32_t *mlx5_dev_supported_ptypes_get(struct rte_eth_dev *dev);
> +int priv_link_update(struct priv *, int);
>  int mlx5_link_update(struct rte_eth_dev *, int);
>  int mlx5_dev_set_mtu(struct rte_eth_dev *, uint16_t);
>  int mlx5_dev_get_flow_ctrl(struct rte_eth_dev *, struct rte_eth_fc_conf *);
> diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
> index 6f78adc4a..16377a8cb 100644
> --- a/drivers/net/mlx5/mlx5_ethdev.c
> +++ b/drivers/net/mlx5/mlx5_ethdev.c
> @@ -885,7 +885,49 @@ mlx5_link_update_unlocked_gs(struct rte_eth_dev *dev, 
> int wait_to_complete)
>  }
>  
>  /**
> - * DPDK callback to retrieve physical link information.
> + * Enable receiving and transmitting traffic.
> + *
> + * @param dev
> + *   Pointer to Ethernet device structure.
> + */
> +static void
> +priv_link_start(struct priv *priv)

Comments says dev but parameter is priv.

> +{
> + struct rte_eth_dev *dev = priv->dev;
> + int err;
> +
> + dev->tx_pkt_burst = priv_select_tx_function(priv, dev);
> + dev->rx_pkt_burst = priv_select_rx_function(priv, dev);
> + err = priv_dev_traffic_enable(priv, dev);
> + if (err)
> + ERROR("%p: error occurred while configuring control flows: %s",
> +   (void *)priv, strerror(err));
> + err = priv_flow_start(priv, &priv->flows);
> + if (err)
> + ERROR("%p: error occurred while configuring flows: %s",
> +   (void *)priv, strerror(err));
> +}
> +
> +/**
> + * Disable receiving and transmitting traffic.
> + *
> + * @param dev
> + *   Pointer to Ethernet device structure.
> + */
> +static void
> +priv_link_stop(struct priv *priv)

Same here.

> +{
> + struct rte_eth_dev *dev = priv->dev;
> +
> + priv_flow_stop(priv, &priv->flows);
> + priv_dev_traffic_disable(priv, dev);
> + dev->rx_pkt_burst = removed_rx_burst;
> + dev->tx_pkt_burst = removed_tx_burst;
> +}
> +
> +/**
> + * Retrieve physical link information and update rx/tx_pkt_burst callbacks
> + * accordingly.
>   *
>   * @param dev
>   *   Pointer to Ethernet device structure.
> @@ -893,17 +935,54 @@ mlx5_link_update_unlocked_gs(struct rte_eth_dev *dev, 
> int wait_to_complete)
>   *   Wait for request completion (ignored).
>   */
>  int
> -mlx5_link_update(struct rte_eth_dev *dev, int wait_to_complete)
> +priv_link_update(struct priv *priv, int wait_to_complete)


Seems to be also the case here.

Thanks,

-- 
Nélio Laranjeiro
6WIND

Re: [dpdk-dev] [RFC] tunnel endpoint hw acceleration enablement

[Shahaf Shuler]

Thursday, January 11, 2018 11:45 PM, John Daley:
> Hi Declan and Shahaf,
> 
> > I can't see how the existing
> > ethdev API could be used for statistics as a single ethdev could be
> > supporting may concurrent TEPs, therefore we would either need to use
> > the extended stats with many entries, one for each TEP, or if we treat
> > a TEP as an attribute of a port in a similar manner to the way
> > rte_security manages an IPsec SA, the state of each TEP can be
> > monitored and managed independently of both the overall port or the
> flows being transported on that endpoint.
> 
> Assuming we can define one rte_flow rule per TEP, does what you propose
> give us anything more than just using the COUNT action?

I agree with John here, and I also not sure we need such assumption. 

If I get it right, the API proposed here is to have a tunnel endpoint which is 
a logical port on top of ethdev port. the TEP is able to receive and monitor 
some specific tunneled traffic, for example VXLAN, GENEVE and more.
For example, VXLAN TEP can have multiple flows with different VNIs all under 
the same context. 

Now, with the current rte_flow APIs, we can do exactly the same and give the 
application the full flexibility to group the tunnel flows into logical TEP. 
On this suggestion application will:
1. Create rte_flow rules for the pattern it want to receive.
2. In case it is interested in counting, a COUNT action will be added to the 
flow.
3. In case header manipulation is required, a DECAP/ENCAP/REWRITE action will 
be added to the flow. 
4. Grouping of flows into a logical TEP will be done on the application layer 
simply by keeping the relevant rte_flow rules in some dedicated struct. With 
it, create/destroy TEP can be translated to create/destroy the flow rules. 
Statistics query can be done be querying each flow count and sum. Note that 
some devices can support the same counter for multiple flows. Even though it is 
not yet exposed in rte_flow this can be an interesting optimization. 

> >
> > > As for the capabilities - what specifically you had in mind? The
> > > current
> > usage you show with tep is with rte_flow rules. There are no
> > capabilities currently for rte_flow supported actions/pattern. To
> > check such capabilities application uses rte_flow_validate.
> >
> > I envisaged that the application should be able to see if an ethdev
> > can support TEP in the rx/tx offloads, and then the
> > rte_tep_capabilities would allow applications to query what tunnel
> > endpoint protocols are supported etc. I would like a simple mechanism
> > to allow users to see if a particular tunnel endpoint type is
> > supported without having to build actual flows to validate.
> 
> I can see the value of that, but in the end wouldn't the API call
> rte_flow_validate anyways? Maybe we don't add the layer now or maybe it
> doesn't really belong in DPDK? I'm in favor of deferring the capabilities API
> until we know it's really needed.  I hate to see special capabilities APIs 
> start
> sneaking in after we decided to go the rte_flow_validate route and users are
> starting to get used to it.

I don't see how it is different from any other rte_flow creation.
We don't hold caps for device ability to filter packets according to VXLAN or 
GENEVE items. Why we should start now?

We have already the rte_flow_veirfy. I think part of the reasons for it was 
that the number of different capabilities possible with rte_flow is huge. I 
think this also the case with the TEP capabilities (even though It is still not 
clear to me what exactly they will include). 

> >
> > > Regarding the creation/destroy of tep. Why not simply use rte_flow
> > > API
> > and avoid this extra control?
> > > For example - with 17.11 APIs, application can put the port in
> > > isolate mode,
> > and insert a flow_rule to catch only IPv4 VXLAN traffic and direct to
> > some queue/do RSS. Such operation, per my understanding, will create a
> > tunnel endpoint. What are the down sides of doing it with the current
> APIs?
> >
> > That doesn't enable encapsulation and decapsulation of the outer
> > tunnel endpoint in the hw as far as I know. Apart from the inability
> > to monitor the endpoint statistics I mentioned above. It would also
> > require that you redefine the endpoints parameters ever time to you
> > wish to add a new flow to it. I think the having the rte_tep object
> > semantics should also simplify the ability to enable a full vswitch
> > offload of TEP where the hw is handling both encap/decap and switching to
> a particular port.
> 
> If we have the ingress/decap and egress/encap actions and 1 rte_flow rule
> per TEP and use the COUNT action, I think we get all but the last bit. For 
> that,
> perhaps the application could keep  ingress and egress rte_flow template for
> each tunnel type (VxLAN, GRE, ..). Then copying the template and filling in
> the outer packet info and tunnel Id is all that would be required. We could
> also define these in rte_fl

Re: [dpdk-dev] [PATCH 1/3] app/testpmd: Moved cmdline_flow to librte_cmdline

[Olivier Matz]

Hi Georgios,

On Mon, Jan 15, 2018 at 01:30:35AM +, Lu, Wenzhuo wrote:
> Hi,
> 
> > -Original Message-
> > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Georgios Katsikas
> > Sent: Saturday, January 13, 2018 5:01 AM
> > To: olivier.m...@6wind.com
> > Cc: dev@dpdk.org; Georgios Katsikas 
> > Subject: [dpdk-dev] [PATCH 1/3] app/testpmd: Moved cmdline_flow to
> > librte_cmdline
> > 
> > Signed-off-by: Georgios Katsikas 
> Looks like a good idea to move this code to the lib.
> cc Adrien the author of this file, app/test-pmd/cmdline_flow.c.

If the command line parsing of rte_flow is something that has some
chances to be shared among multiple applications, I agree it makes sense
to move it in a library.

However, my opinion is that it would be better to have a specific
library for it, like librte_flow_cmdline, because I'm not sure that
people linking with librte_cmdline always want to pull the rte_flow
parsing code.

Re: [dpdk-dev] [PATCH 1/3] app/testpmd: Moved cmdline_flow to librte_cmdline

[george . dit]

Hi Lu, Oliver,

Thanks for your feedback!
You have a point here, flow commands are only a subset of the parser.
Do you want me to create this new library and send another patch?
I guess I have to use librte_cmdline as a template/example for creating the
librte_flow_cmdline library.

Best regards,
Georgios

On Tue, Jan 16, 2018 at 9:40 AM Olivier Matz  wrote:

> Hi Georgios,
>
> On Mon, Jan 15, 2018 at 01:30:35AM +, Lu, Wenzhuo wrote:
> > Hi,
> >
> > > -Original Message-
> > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Georgios Katsikas
> > > Sent: Saturday, January 13, 2018 5:01 AM
> > > To: olivier.m...@6wind.com
> > > Cc: dev@dpdk.org; Georgios Katsikas 
> > > Subject: [dpdk-dev] [PATCH 1/3] app/testpmd: Moved cmdline_flow to
> > > librte_cmdline
> > >
> > > Signed-off-by: Georgios Katsikas 
> > Looks like a good idea to move this code to the lib.
> > cc Adrien the author of this file, app/test-pmd/cmdline_flow.c.
>
> If the command line parsing of rte_flow is something that has some
> chances to be shared among multiple applications, I agree it makes sense
> to move it in a library.
>
> However, my opinion is that it would be better to have a specific
> library for it, like librte_flow_cmdline, because I'm not sure that
> people linking with librte_cmdline always want to pull the rte_flow
> parsing code.
>
>

Re: [dpdk-dev] [PATCH] vhost: do deep copy while reallocate vq

[Maxime Coquelin]

On 01/15/2018 12:32 PM, Junjie Chen wrote:

When vhost reallocate dev and vq for NUMA enabled case, it doesn't perform
deep copy, which lead to 1) zmbuf list not valid 2) remote memory access.
This patch is to re-initlize the zmbuf list and also do the deep copy.

Signed-off-by: Junjie Chen 
---
  lib/librte_vhost/vhost_user.c | 31 +++
  1 file changed, 31 insertions(+)



Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [dpdk-dev] [PATCH v10 3/5] net: add a helper for making RARP packet

[Olivier Matz]

Hi Xiao,

Please find few comments below.

On Wed, Jan 10, 2018 at 09:23:54AM +0800, Xiao Wang wrote:
> Suggested-by: Maxime Coquelin 
> Signed-off-by: Xiao Wang 
> Reviewed-by: Maxime Coquelin 
> ---
>  lib/librte_net/Makefile|  1 +
>  lib/librte_net/rte_arp.c   | 42 
> ++
>  lib/librte_net/rte_arp.h   | 17 +++
>  lib/librte_net/rte_net_version.map |  6 ++
>  4 files changed, 66 insertions(+)
>  create mode 100644 lib/librte_net/rte_arp.c
> 
> diff --git a/lib/librte_net/Makefile b/lib/librte_net/Makefile
> index 5e8a76b68..ab290c382 100644
> --- a/lib/librte_net/Makefile
> +++ b/lib/librte_net/Makefile
> @@ -13,6 +13,7 @@ LIBABIVER := 1
>  
>  SRCS-$(CONFIG_RTE_LIBRTE_NET) := rte_net.c
>  SRCS-$(CONFIG_RTE_LIBRTE_NET) += rte_net_crc.c
> +SRCS-$(CONFIG_RTE_LIBRTE_NET) += rte_arp.c
>  
>  # install includes
>  SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include := rte_ip.h rte_tcp.h rte_udp.h 
> rte_esp.h
> diff --git a/lib/librte_net/rte_arp.c b/lib/librte_net/rte_arp.c
> new file mode 100644
> index 0..d7223b044
> --- /dev/null
> +++ b/lib/librte_net/rte_arp.c
> @@ -0,0 +1,42 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2018 Intel Corporation
> + */
> +
> +#include 
> +
> +#include 
> +
> +#define RARP_PKT_SIZE64
> +int
> +rte_net_make_rarp_packet(struct rte_mbuf *mbuf, const struct ether_addr *mac)
> +{
> + struct ether_hdr *eth_hdr;
> + struct arp_hdr *rarp;
> +
> + if (mbuf->buf_len < RARP_PKT_SIZE)
> + return -1;
> +
> + /* Ethernet header. */
> + eth_hdr = rte_pktmbuf_mtod(mbuf, struct ether_hdr *);
> + memset(eth_hdr->d_addr.addr_bytes, 0xff, ETHER_ADDR_LEN);
> + ether_addr_copy(mac, ð_hdr->s_addr);
> + eth_hdr->ether_type = htons(ETHER_TYPE_RARP);
> +
> + /* RARP header. */
> + rarp = (struct arp_hdr *)(eth_hdr + 1);
> + rarp->arp_hrd = htons(ARP_HRD_ETHER);
> + rarp->arp_pro = htons(ETHER_TYPE_IPv4);
> + rarp->arp_hln = ETHER_ADDR_LEN;
> + rarp->arp_pln = 4;
> + rarp->arp_op  = htons(ARP_OP_REVREQUEST);
> +
> + ether_addr_copy(mac, &rarp->arp_data.arp_sha);
> + ether_addr_copy(mac, &rarp->arp_data.arp_tha);
> + memset(&rarp->arp_data.arp_sip, 0x00, 4);
> + memset(&rarp->arp_data.arp_tip, 0x00, 4);
> +
> + mbuf->data_len = RARP_PKT_SIZE;
> + mbuf->pkt_len = RARP_PKT_SIZE;
> +
> + return 0;
> +}

You don't check that there is enough tailroom to write the packet data.
Also, nothing verifies that the mbuf passed to the function is empty.
I suggest to do the allocation in this function, what do you think?

You can also use rte_pktmbuf_append() to check for the tailroom and
update data_len/pkt_len:

m = rte_pktmbuf_alloc();
if (m == NULL)
return NULL;
eth_hdr = rte_pktmbuf_append(m, RARP_PKT_SIZE);
if (eth_hdr == NULL) {
m_freem(m);
return NULL;
}
eth_hdr->... = ...;
...
rarp = (struct arp_hdr *)(eth_hdr + 1);
rarp->... = ...;
...

return m;

Re: [dpdk-dev] [PATCH 1/2] lib/cryptodev: add support to set session private data

[Gujjar, Abhinandan S]

Hi Akhil,

> -Original Message-
> From: Akhil Goyal [mailto:akhil.go...@nxp.com]
> Sent: Tuesday, January 16, 2018 12:56 PM
> To: Gujjar, Abhinandan S ; Doherty, Declan
> ; Jacob, Jerin
> 
> Cc: dev@dpdk.org; Vangati, Narender ; Rao,
> Nikhil 
> Subject: Re: [PATCH 1/2] lib/cryptodev: add support to set session private 
> data
> 
> Hi Abhinandan,
> On 1/16/2018 12:35 PM, Gujjar, Abhinandan S wrote:
> > Hi Akhil,
> >
> >> -Original Message-
> >> From: Akhil Goyal [mailto:akhil.go...@nxp.com]
> >> Sent: Tuesday, January 16, 2018 11:55 AM
> >> To: Gujjar, Abhinandan S ; Doherty,
> >> Declan 
> >> Cc: dev@dpdk.org; Vangati, Narender ;
> >> Rao, Nikhil 
> >> Subject: Re: [PATCH 1/2] lib/cryptodev: add support to set session
> >> private data
> >>
> >> Hi Abhinandan,
> >> On 1/16/2018 11:39 AM, Gujjar, Abhinandan S wrote:
> > diff --git a/lib/librte_cryptodev/rte_crypto.h
> > b/lib/librte_cryptodev/rte_crypto.h
> > index bbc510d..3a98cbf 100644
> > --- a/lib/librte_cryptodev/rte_crypto.h
> > +++ b/lib/librte_cryptodev/rte_crypto.h
> > @@ -62,6 +62,18 @@ enum rte_crypto_op_sess_type {
> > RTE_CRYPTO_OP_SECURITY_SESSION  /**< Security session
> crypto
>  operation */
> > };
> >
> > +/** Private data types for cryptographic operation
> > + * @see rte_crypto_op::private_data_type */ enum
> > +rte_crypto_op_private_data_type {
> > +   RTE_CRYPTO_OP_PRIVATE_DATA_NONE,
> > +   /**< No private data */
> > +   RTE_CRYPTO_OP_PRIVATE_DATA_OP,
> > +   /**< Private data is part of rte_crypto_op and indicated by
> > +* private_data_offset */
> > +   RTE_CRYPTO_OP_PRIVATE_DATA_SESSION
> > +   /**< Private data is available at session */ };
> > +
>  We may get away with this enum. If private_data_offset is "0", then
>  we can check with the session if it has priv data or not.
> >>> Right now,  Application uses 'rte_crypto_op_private_data_type' to
> >>> indicate rte_cryptodev_sym_session_set_private_data()
> >>> was called to set the private data. Otherwise, how do you indicate
> >>> there is a
> >> private data associated with the session?
> >>> Any suggestions?
> >> For session based flows, the first choice to store the private data
> >> should be in the session. So RTE_CRYPTO_OP_WITH_SESSION or
> >> RTE_CRYPTO_OP_SECURITY_SESSION can be used to call
> >> rte_cryptodev_sym_session_set_private_data or
> >> rte_security_session_set_private_data.
> > Case 1: private_data_offset is "0" and sess_type =
> > RTE_CRYPTO_OP_WITH_SESSION -> usual case Case 2: private_data_offset
> > is "0" and sess_type = RTE_CRYPTO_OP_WITH_SESSION + event case (access
> > private data) Differentiating between case 1 & 2 will be done by checking
> rte_crypto_op_private_data_type ==
> RTE_CRYPTO_OP_PRIVATE_DATA_SESSION.
> 
> Consider this:
> if (sess_type == RTE_CRYPTO_OP_WITH_SESSION &&
>   rte_cryptodev_sym_session_get_private_data == NULL)
>   usual case.
> else if (sess_type = RTE_CRYPTO_OP_WITH_SESSION &&
>   rte_cryptodev_sym_session_get_private_data != NULL)
>   event case.
> else if (sess_type == RTE_CRYPTO_OP_SESSIONLESS &&
>   private_data_offset != 0)
>   event case for sessionless op.
> 
> I hope all cases can be handled in this way.
Memory allocated for private data will be continuation of session memory.
I think, rte_cryptodev_sym_session_get_private_data() will return a valid 
pointer.
It could be pointer to private data, in case application has allocated mempool 
with session + private data.
It could be again a pointer to a location(may be next session),  in case 
application has allocated mempool with session only.
Unless, there is a flag in the session data which will be set by 
rte_cryptodev_sym_session_set_private_data()
If this flag is not set, rte_cryptodev_sym_session_get_private_data() will 
return NULL.
I am not claiming, I have complete knowledge of different usage case of mempool 
setup for crypto.
I am wondering, whether I am missing anything here. Please let me know.
> 
> 
> -Akhil
-Abhinandan

[dpdk-dev] [PATCH v3 1/2] doc: add guidelines for coverity tags

[Marko Kovacevic]

 Added contribution guideline for adding tags
 when sending patches that have been raised by
 coverity

Signed-off-by: Marko Kovacevic 
Acked-by: Hemant Agrawal 
---
 doc/guides/contributing/patches.rst | 21 +
 1 file changed, 21 insertions(+)

diff --git a/doc/guides/contributing/patches.rst 
b/doc/guides/contributing/patches.rst
index 64408e7..59d2aaf 100644
--- a/doc/guides/contributing/patches.rst
+++ b/doc/guides/contributing/patches.rst
@@ -257,6 +257,27 @@ tags for who reported, suggested, tested and reviewed the 
patch being
 posted. Please refer to the `Tested, Acked and Reviewed by`_ section.
 
 
+Coverity Related Patch Fixes:
+~
+
+`Coverity 
`_ is a 
tool for
+static code analysis. It is used as a cloud-based service used to scan the
+DPDK source code, and alert developers of any potential defects in the source 
code.
+When fixing an issue found by Coverity, the patch must contain a Coverity 
issue ID in the
+body of the commit message. For example::
+
+
+ doc: fix some parameter description
+
+ Update the docs, fixing description of some parameter.
+
+ Coverity issue: 12345
+ Fixes: abcdefgh1234 ("doc: add some parameter")
+ Cc: aut...@example.com
+
+ Signed-off-by: Alex Smith 
+
+
 Creating Patches
 
 
-- 
2.9.5

[dpdk-dev] [PATCH v3 1/2] doc: add guidelines for coverity tags

[Marko Kovacevic]

 Added contribution guideline for adding tags
 when sending patches that have been raised by
 coverity

Signed-off-by: Marko Kovacevic 
Acked-by: Hemant Agrawal 
---
 doc/guides/contributing/patches.rst | 21 +
 1 file changed, 21 insertions(+)

diff --git a/doc/guides/contributing/patches.rst 
b/doc/guides/contributing/patches.rst
index 64408e7..59d2aaf 100644
--- a/doc/guides/contributing/patches.rst
+++ b/doc/guides/contributing/patches.rst
@@ -257,6 +257,27 @@ tags for who reported, suggested, tested and reviewed the 
patch being
 posted. Please refer to the `Tested, Acked and Reviewed by`_ section.
 
 
+Coverity Related Patch Fixes:
+~
+
+`Coverity 
`_ is a 
tool for
+static code analysis. It is used as a cloud-based service used to scan the
+DPDK source code, and alert developers of any potential defects in the source 
code.
+When fixing an issue found by Coverity, the patch must contain a Coverity 
issue ID in the
+body of the commit message. For example::
+
+
+ doc: fix some parameter description
+
+ Update the docs, fixing description of some parameter.
+
+ Coverity issue: 12345
+ Fixes: abcdefgh1234 ("doc: add some parameter")
+ Cc: aut...@example.com
+
+ Signed-off-by: Alex Smith 
+
+
 Creating Patches
 
 
-- 
2.9.5

Re: [dpdk-dev] [PATCH v2 1/8] eal: introduce DMA memory barriers

[Jianbo Liu]

The 01/16/2018 10:49, Andrew Rybchenko wrote:
> On 01/16/2018 04:10 AM, Yongseok Koh wrote:
> >This commit introduces rte_dma_wmb() and rte_dma_rmb(), in order to
> >guarantee the ordering of coherent shared memory between the CPU and a DMA
> >capable device.
> >
> >Signed-off-by: Yongseok Koh 
> >---
> >  lib/librte_eal/common/include/generic/rte_atomic.h | 18 ++
> >  1 file changed, 18 insertions(+)
> >
> >diff --git a/lib/librte_eal/common/include/generic/rte_atomic.h 
> >b/lib/librte_eal/common/include/generic/rte_atomic.h
> >index 16af5ca57..2e0503ce6 100644
> >--- a/lib/librte_eal/common/include/generic/rte_atomic.h
> >+++ b/lib/librte_eal/common/include/generic/rte_atomic.h
> >@@ -98,6 +98,24 @@ static inline void rte_io_wmb(void);
> >   */
> >  static inline void rte_io_rmb(void);
> >+/**
> >+ * Write memory barrier for coherent memory between lcore and IO device
> >+ *
> >+ * Guarantees that the STORE operations on coherent memory that
> >+ * precede the rte_dma_wmb() call are visible to I/O device before the
> >+ * STORE operations that follow it.
> >+ */
> >+static inline void rte_dma_wmb(void);
> >+
> >+/**
> >+ * Read memory barrier for coherent memory between lcore and IO device
> >+ *
> >+ * Guarantees that the LOAD operations on coherent memory updated by
> >+ * IO device that precede the rte_dma_rmb() call are visible to CPU
> >+ * before the LOAD operations that follow it.
> >+ */
> >+static inline void rte_dma_rmb(void);
> >+
> >  #endif /* __DOXYGEN__ */
> >  /**
>
> I'm not an ARMv8 expert so, my comments could be a bit ignorant.
> I'd like to understand the difference between io and added here dma
> barriers.
> The difference should be clearly explained. Otherwise we'll constantly hit
> on incorrect choice of barrier type.
> Also I don't understand why "dma" name is chosen taking into account
> that definition is bound to coherent memory between lcore and IO device.

A good explanation can be found here.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1077fa36f23e259858caf6f269a47393a5aff523

--
IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.

[dpdk-dev] [PATCH v3 1/2] doc: add guidelines for coverity tags

[Marko Kovacevic]

 Added contribution guideline for adding tags
 when sending patches that have been raised by
 coverity

Signed-off-by: Marko Kovacevic 
Acked-by: Hemant Agrawal 
---
 doc/guides/contributing/patches.rst | 21 +
 1 file changed, 21 insertions(+)

diff --git a/doc/guides/contributing/patches.rst 
b/doc/guides/contributing/patches.rst
index 64408e7..59d2aaf 100644
--- a/doc/guides/contributing/patches.rst
+++ b/doc/guides/contributing/patches.rst
@@ -257,6 +257,27 @@ tags for who reported, suggested, tested and reviewed the 
patch being
 posted. Please refer to the `Tested, Acked and Reviewed by`_ section.
 
 
+Coverity Related Patch Fixes:
+~
+
+`Coverity 
`_ is a 
tool for
+static code analysis. It is used as a cloud-based service used to scan the
+DPDK source code, and alert developers of any potential defects in the source 
code.
+When fixing an issue found by Coverity, the patch must contain a Coverity 
issue ID in the
+body of the commit message. For example::
+
+
+ doc: fix some parameter description
+
+ Update the docs, fixing description of some parameter.
+
+ Coverity issue: 12345
+ Fixes: abcdefgh1234 ("doc: add some parameter")
+ Cc: aut...@example.com
+
+ Signed-off-by: Alex Smith 
+
+
 Creating Patches
 
 
-- 
2.9.5

[dpdk-dev] [PATCH v3 2/2] doc: add guidelines for stable tags

[Marko Kovacevic]

   Added contribution guideline for adding stable
   tags when sending patches all fix patches to the
   master branch that are candidates for backporting

Signed-off-by: Marko Kovacevic 
Acked-by: Hemant Agrawal 
---
 doc/guides/contributing/patches.rst | 17 +
 1 file changed, 17 insertions(+)

diff --git a/doc/guides/contributing/patches.rst 
b/doc/guides/contributing/patches.rst
index 59d2aaf..cef9776 100644
--- a/doc/guides/contributing/patches.rst
+++ b/doc/guides/contributing/patches.rst
@@ -277,6 +277,23 @@ body of the commit message. For example::
 
  Signed-off-by: Alex Smith 
 
+Patch for Stable Releases:
+~~
+
+All fix patches to the master branch that are candidates for backporting
+should also be CCed to the `sta...@dpdk.org 
`_
+mailing list. In the commit message body the Cc: sta...@dpdk.org should be 
inserted as follows::
+
+ doc: fix some parameter description
+
+ Update the docs, fixing description of some parameter.
+
+ Fixes: abcdefgh1234 ("doc: add some parameter")
+ Cc: sta...@dpdk.org
+
+ Signed-off-by: Alex Smith 
+
+For further information on stable contribution you can go here :doc:`Stable 
Contribution`
 
 Creating Patches
 
-- 
2.9.5

[dpdk-dev] [PATCH v2 1/2] lib/net: add IPv6 header fields macros

[Shahaf Shuler]

From: Shachar Beiser 

Support IPv6 header vtc_flow fields : tc , flow_label

Signed-off-by: Shachar Beiser 
---
Sending on behalf of Shachar.

On v2:
 - Addressed Stephen comments on the coding style.

---
 lib/librte_net/rte_ip.h | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
index 9a62ff667..f32684c66 100644
--- a/lib/librte_net/rte_ip.h
+++ b/lib/librte_net/rte_ip.h
@@ -344,6 +344,12 @@ struct ipv6_hdr {
uint8_t  dst_addr[16]; /**< IP address of destination host(s). */
 } __attribute__((__packed__));
 
+/* IPv6 vtc_flow: IPv / TC / flow_label */
+#define IPV6_HDR_FL_SHIFT 0
+#define IPV6_HDR_TC_SHIFT 20
+#define IPV6_HDR_FL_MASK ((1u << IPV6_HDR_TC_SHIFT) - 1)
+#define IPV6_HDR_TC_MASK (0xf << IPV6_HDR_TC_SHIFT)
+
 /**
  * Process the pseudo-header checksum of an IPv6 header.
  *
-- 
2.12.0

[dpdk-dev] [PATCH v2 2/2] net/mlx5: fix IPv6 header fields

[Shahaf Shuler]

From: Shachar Beiser 

There are parameters that are not copy from
spec to verbs structure in the vtc_label

Fixes: 43e9d97 ("net/mlx5: support upstream rdma-core")
Cc: sta...@dpdk.org

Signed-off-by: Shachar Beiser 
Acked-by: Yongseok Koh 
---
Sending on behalf of Shachar.

On v2:
 - No changes.

---
 drivers/net/mlx5/mlx5_flow.c | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index ff9fd78d5..4396ea852 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -48,6 +48,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "mlx5.h"
 #include "mlx5_defs.h"
@@ -1410,6 +1411,8 @@ mlx5_flow_create_ipv6(const struct rte_flow_item *item,
parser->layer = HASH_RXQ_IPV6;
if (spec) {
unsigned int i;
+   uint32_t vtc_flow_val;
+   uint32_t vtc_flow_mask;
 
if (!mask)
mask = default_mask;
@@ -1421,7 +1424,20 @@ mlx5_flow_create_ipv6(const struct rte_flow_item *item,
   RTE_DIM(ipv6.mask.src_ip));
memcpy(&ipv6.mask.dst_ip, mask->hdr.dst_addr,
   RTE_DIM(ipv6.mask.dst_ip));
-   ipv6.mask.flow_label = mask->hdr.vtc_flow;
+   vtc_flow_val = rte_be_to_cpu_32(spec->hdr.vtc_flow);
+   vtc_flow_mask = rte_be_to_cpu_32(mask->hdr.vtc_flow);
+   ipv6.val.flow_label =
+   rte_cpu_to_be_32((vtc_flow_val & IPV6_HDR_FL_MASK) >>
+IPV6_HDR_FL_SHIFT);
+   ipv6.val.traffic_class = (vtc_flow_val & IPV6_HDR_TC_MASK) >>
+IPV6_HDR_TC_SHIFT;
+   ipv6.val.next_hdr = spec->hdr.proto;
+   ipv6.val.hop_limit = spec->hdr.hop_limits;
+   ipv6.mask.flow_label =
+   rte_cpu_to_be_32((vtc_flow_mask & IPV6_HDR_FL_MASK) >>
+IPV6_HDR_FL_SHIFT);
+   ipv6.mask.traffic_class = (vtc_flow_mask & IPV6_HDR_TC_MASK) >>
+ IPV6_HDR_TC_SHIFT;
ipv6.mask.next_hdr = mask->hdr.proto;
ipv6.mask.hop_limit = mask->hdr.hop_limits;
/* Remove unwanted bits from values. */
@@ -1430,6 +1446,7 @@ mlx5_flow_create_ipv6(const struct rte_flow_item *item,
ipv6.val.dst_ip[i] &= ipv6.mask.dst_ip[i];
}
ipv6.val.flow_label &= ipv6.mask.flow_label;
+   ipv6.val.traffic_class &= ipv6.mask.traffic_class;
ipv6.val.next_hdr &= ipv6.mask.next_hdr;
ipv6.val.hop_limit &= ipv6.mask.hop_limit;
}
-- 
2.12.0

Re: [dpdk-dev] [PATCH 1/2] lib/cryptodev: add support to set session private data

[Akhil Goyal]

On 1/16/2018 2:33 PM, Gujjar, Abhinandan S wrote:

Hi Akhil,


-Original Message-
From: Akhil Goyal [mailto:akhil.go...@nxp.com]
Sent: Tuesday, January 16, 2018 12:56 PM
To: Gujjar, Abhinandan S ; Doherty, Declan
; Jacob, Jerin

Cc: dev@dpdk.org; Vangati, Narender ; Rao,
Nikhil 
Subject: Re: [PATCH 1/2] lib/cryptodev: add support to set session private data

Hi Abhinandan,
On 1/16/2018 12:35 PM, Gujjar, Abhinandan S wrote:

Hi Akhil,


-Original Message-
From: Akhil Goyal [mailto:akhil.go...@nxp.com]
Sent: Tuesday, January 16, 2018 11:55 AM
To: Gujjar, Abhinandan S ; Doherty,
Declan 
Cc: dev@dpdk.org; Vangati, Narender ;
Rao, Nikhil 
Subject: Re: [PATCH 1/2] lib/cryptodev: add support to set session
private data

Hi Abhinandan,
On 1/16/2018 11:39 AM, Gujjar, Abhinandan S wrote:

diff --git a/lib/librte_cryptodev/rte_crypto.h
b/lib/librte_cryptodev/rte_crypto.h
index bbc510d..3a98cbf 100644
--- a/lib/librte_cryptodev/rte_crypto.h
+++ b/lib/librte_cryptodev/rte_crypto.h
@@ -62,6 +62,18 @@ enum rte_crypto_op_sess_type {
RTE_CRYPTO_OP_SECURITY_SESSION  /**< Security session

crypto

operation */

 };

+/** Private data types for cryptographic operation
+ * @see rte_crypto_op::private_data_type */ enum
+rte_crypto_op_private_data_type {
+   RTE_CRYPTO_OP_PRIVATE_DATA_NONE,
+   /**< No private data */
+   RTE_CRYPTO_OP_PRIVATE_DATA_OP,
+   /**< Private data is part of rte_crypto_op and indicated by
+* private_data_offset */
+   RTE_CRYPTO_OP_PRIVATE_DATA_SESSION
+   /**< Private data is available at session */ };
+

We may get away with this enum. If private_data_offset is "0", then
we can check with the session if it has priv data or not.

Right now,  Application uses 'rte_crypto_op_private_data_type' to
indicate rte_cryptodev_sym_session_set_private_data()
was called to set the private data. Otherwise, how do you indicate
there is a

private data associated with the session?

Any suggestions?

For session based flows, the first choice to store the private data
should be in the session. So RTE_CRYPTO_OP_WITH_SESSION or
RTE_CRYPTO_OP_SECURITY_SESSION can be used to call
rte_cryptodev_sym_session_set_private_data or
rte_security_session_set_private_data.

Case 1: private_data_offset is "0" and sess_type =
RTE_CRYPTO_OP_WITH_SESSION -> usual case Case 2: private_data_offset
is "0" and sess_type = RTE_CRYPTO_OP_WITH_SESSION + event case (access
private data) Differentiating between case 1 & 2 will be done by checking

rte_crypto_op_private_data_type ==
RTE_CRYPTO_OP_PRIVATE_DATA_SESSION.

Consider this:
if (sess_type == RTE_CRYPTO_OP_WITH_SESSION &&
rte_cryptodev_sym_session_get_private_data == NULL)
usual case.
else if (sess_type = RTE_CRYPTO_OP_WITH_SESSION &&
rte_cryptodev_sym_session_get_private_data != NULL)
event case.
else if (sess_type == RTE_CRYPTO_OP_SESSIONLESS &&
private_data_offset != 0)
event case for sessionless op.

I hope all cases can be handled in this way.

Memory allocated for private data will be continuation of session memory.
I think, rte_cryptodev_sym_session_get_private_data() will return a valid 
pointer.
It could be pointer to private data, in case application has allocated mempool 
with session + private data.
It could be again a pointer to a location(may be next session),  in case 
application has allocated mempool with session only.
Unless, there is a flag in the session data which will be set by 
rte_cryptodev_sym_session_set_private_data()
If this flag is not set, rte_cryptodev_sym_session_get_private_data() will 
return NULL.
I am not claiming, I have complete knowledge of different usage case of mempool 
setup for crypto.
I am wondering, whether I am missing anything here. Please let me know.


It depends on the implementation of the get/set API. We can set NULL, if 
it is not filled in the set API. If it is set then we have a valid pointer.


-Akhil

Re: [dpdk-dev] [PATCH 1/3] app/testpmd: Moved cmdline_flow to librte_cmdline

[Olivier Matz]

Hi,

> On Tue, Jan 16, 2018 at 9:40 AM Olivier Matz  wrote:
>
> On Tue, Jan 16, 2018 at 08:45:32AM +, george@gmail.com wrote:
> > Hi Georgios,
> >
> > On Mon, Jan 15, 2018 at 01:30:35AM +, Lu, Wenzhuo wrote:
> > > Hi,
> > >
> > > > -Original Message-
> > > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Georgios Katsikas
> > > > Sent: Saturday, January 13, 2018 5:01 AM
> > > > To: olivier.m...@6wind.com
> > > > Cc: dev@dpdk.org; Georgios Katsikas 
> > > > Subject: [dpdk-dev] [PATCH 1/3] app/testpmd: Moved cmdline_flow to
> > > > librte_cmdline
> > > >
> > > > Signed-off-by: Georgios Katsikas 
> > > Looks like a good idea to move this code to the lib.
> > > cc Adrien the author of this file, app/test-pmd/cmdline_flow.c.
> >
> > If the command line parsing of rte_flow is something that has some
> > chances to be shared among multiple applications, I agree it makes sense
> > to move it in a library.
> >
> > However, my opinion is that it would be better to have a specific
> > library for it, like librte_flow_cmdline, because I'm not sure that
> > people linking with librte_cmdline always want to pull the rte_flow
> > parsing code.
> >
> >
> Hi Lu, Oliver,
> 
> Thanks for your feedback!
> You have a point here, flow commands are only a subset of the parser.
> Do you want me to create this new library and send another patch?

Let's first wait for Adrien's feedback, he can have counter-arguments.

> I guess I have to use librte_cmdline as a template/example for creating the
> librte_flow_cmdline library.

It can be used as an example for Makefile and .map file.

Re: [dpdk-dev] [PATCH v4 05/15] net/dpaa2: use rte_eth_linkstatus_set

[Shreyansh Jain]

On Thursday 11 January 2018 10:36 PM, Ferruh Yigit wrote:

From: Stephen Hemminger 

Use new helper function to update the link status.
As a good side effect this fixes a but because this driver was not

  ^^
  needs rephrasing


returning correct status (should be -1 in link_status changed).

Signed-off-by: Stephen Hemminger 
---
  drivers/net/dpaa2/dpaa2_ethdev.c | 65 +---
  1 file changed, 7 insertions(+), 58 deletions(-)


Other than the change in commit message highlighted above:

Acked-by: Shreyansh Jain 

Thanks.

Re: [dpdk-dev] [PATCH] net/thunderx: Convert ThunderX VNIC PMD to new offload API

[Maciej Czekaj]

-- Oryginal message --

On 1/15/2018 2:49 PM, Maciej Czekaj wrote:




-- Oryginal message --

On 1/3/2018 1:12 PM, maciej.cze...@caviumnetworks.com wrote:

From: Maciej Czekaj 

This patch removes all references to old-style offload API
replacing them with new offload flags.

Signed-off-by: Maciej Czekaj 

<...>

   
   	dev_info->default_txconf = (struct rte_eth_txconf) {

.tx_free_thresh = NICVF_DEFAULT_TX_FREE_THRESH,
-   .txq_flags =
-   ETH_TXQ_FLAGS_NOMULTSEGS  |
-   ETH_TXQ_FLAGS_NOREFCOUNT  |
-   ETH_TXQ_FLAGS_NOMULTMEMP  |
-   ETH_TXQ_FLAGS_NOVLANOFFL  |
-   ETH_TXQ_FLAGS_NOXSUMSCTP,
+   .txq_flags = ETH_TXQ_FLAGS_IGNORE,

I am not sure about this, Shahafs may comment better, shouldn't application
decide to set "c" or not, instead of having this in default
configuration?

I think of it as a safeguard against a legacy application that is using
old-style txq_flags.

There is an assymetry in API, since
   - rte_eth_tx_queue setup() and rte_eth_rx_queue_setup() convert flags
   - rte_eth_dev_info_get() - does not convert them.

The scenario leading to issues is as follows:

1.Application reads default txq_flags from the rte_eth_dev_info_get().
   - dev_info.txconf.offloads contains flags but it is ignored by legacy code
   - dev_info.txq_flags may be:
    a) txq_flags == 0
        b) txq_flags ==  ETH_TXQ_FLAGS_IGNORE
    c) txq_flags == new-style flags converted to old-style flags


2. Application uses default txq_flags field to rte_eth_tx_queue_setup().
Now, depending on txq_flags:

    a) txq_flags == 0, ethdev layer __clears__ all offloads, see *
    b) txq_flags ==  ETH_TXQ_FLAGS_IGNORE, ethdev layer converts offloads
to txq_flags, but leaves orignal offloads, so PMD can still use them
    c) txq_flags == old-style flags, ethdev layer converts txq_flags to
offloads, destroying default offloads


* relevant code snippet from rte_eth_tx_queue_setup() with comments:

      if (tx_conf->txq_flags & ETH_TXQ_FLAGS_IGNORE) {
      // ==> converts offloads to txq_flags but LEAVES offloads, too
          rte_eth_convert_txq_offloads(tx_conf->offloads,
                       &local_conf.txq_flags);
          /* Keep the ignore flag. */
          local_conf.txq_flags |= ETH_TXQ_FLAGS_IGNORE;
      } else {
      // ==> converts txq_flags to offloads but DESTROYS original
offloads
rte_eth_convert_txq_flags(tx_conf->txq_flags,
                        &local_conf.offloads);
      }


So, out of 3 options:

a) does not work for legacy code
b) will work for legacy code
c) will work but defeats the purpose of removing old-style flags, since
dev_info() callback has to setup both old-style and new-style default flags

I chose b) as the simplest way to work-around the issue. I could post a
patch to ethdev API if you think it is important.

What I understand is txq_flags should be supported because of legacy apps. That
is why ethdev layer converts old txq_flags to offloads when new
ETH_TXQ_FLAGS_IGNORE flag is missing. So that PMD can only use "offloads"
variable for both legacy and new applications.

To support the case application uses defaults from PMD, the one you mentioned
above, I think we should go with option c). For implementation you can use only
new "offloads", so it is not both old-style and new-style, just new-style, but
as default yes should keep both.

And new applications will set ETH_TXQ_FLAGS_IGNORE to say "offloads" has valid
information. Currently this also converted to the txq_flags but this will go
away when all PMDs support new method.

Think  about a case where a legacy application gets defaults from PMD and sets a
few more values in txq_flags and configure the device. When you set
ETH_TXQ_FLAGS_IGNORE as part of defaults, ethdev layer will think only
"offloads" is valid and it will overwrite the txq_flags value with offloads one
and the updates to the txq_flags will be lost.

At one point ETH_TXQ_FLAGS_IGNORE will also go away and applications also will
need to support new method. When it is removed than we can get rid of the
txq_flags defaults from PMDs until than I guess we need to live with them.


So you suggest that dev_info callback should provide default txq_flags 
for a moment?


E.g.

.txq_flags =
ETH_TXQ_FLAGS_NOMULTSEGS  |
ETH_TXQ_FLAGS_NOREFCOUNT  |
ETH_TXQ_FLAGS_NOMULTMEMP  |
ETH_TXQ_FLAGS_NOVLANOFFL  |
ETH_TXQ_FLAGS_NOXSUMSCTP,


That is OK with me. We'll wipe it out later whet it all go away.

Will fix in v2.


<...>


+   if ((conf_tx_offloads & tx_offload_capa) != conf_tx_offloads) {
+   PMD_INIT_LOG(ERR, "Some Tx offloads are not supported "
+ "requested 0x%lx supported 0x%lx\n",
+ conf_tx_offloads, tx_offload_capa);

This is broken for 32bits, using PRIx64 instead of "lx" makes your code more
portable.

Will fix in v

Re: [dpdk-dev] [PATCH v4 05/15] net/dpaa2: use rte_eth_linkstatus_set

[Shreyansh Jain]

On Tuesday 16 January 2018 03:14 PM, Shreyansh Jain wrote:

On Thursday 11 January 2018 10:36 PM, Ferruh Yigit wrote:

From: Stephen Hemminger 

Use new helper function to update the link status.
As a good side effect this fixes a but because this driver was not

   ^^
   needs rephrasing


returning correct status (should be -1 in link_status changed).

Signed-off-by: Stephen Hemminger 
---
  drivers/net/dpaa2/dpaa2_ethdev.c | 65 
+---

  1 file changed, 7 insertions(+), 58 deletions(-)


Other than the change in commit message highlighted above:

Acked-by: Shreyansh Jain 

Thanks.



Also, I forgot to add in previous email, I am assuming 
_rte_eth_linkstatus_get would be rte_eth_linkstatus_get eventually.


I concur with your comments in [1] - it certainly would be better to 
have either @internal or _rte - and not both.


http://dpdk.org/ml/archives/dev/2018-January/086742.html

Re: [dpdk-dev] [PATCH v10 3/5] net: add a helper for making RARP packet

[Wang, Xiao W]

Hi Olivier,

> -Original Message-
> From: Olivier Matz [mailto:olivier.m...@6wind.com]
> Sent: Tuesday, January 16, 2018 5:01 PM
> To: Wang, Xiao W 
> Cc: y...@fridaylinux.org; tho...@monjalon.net; Bie, Tiwei
> ; dev@dpdk.org; step...@networkplumber.org;
> maxime.coque...@redhat.com
> Subject: Re: [dpdk-dev] [PATCH v10 3/5] net: add a helper for making RARP
> packet
> 
> Hi Xiao,
> 
> Please find few comments below.
> 
> On Wed, Jan 10, 2018 at 09:23:54AM +0800, Xiao Wang wrote:
> > Suggested-by: Maxime Coquelin 
> > Signed-off-by: Xiao Wang 
> > Reviewed-by: Maxime Coquelin 
> > ---
> >  lib/librte_net/Makefile|  1 +
> >  lib/librte_net/rte_arp.c   | 42
> ++
> >  lib/librte_net/rte_arp.h   | 17 +++
> >  lib/librte_net/rte_net_version.map |  6 ++
> >  4 files changed, 66 insertions(+)
> >  create mode 100644 lib/librte_net/rte_arp.c
> >
> > diff --git a/lib/librte_net/Makefile b/lib/librte_net/Makefile
> > index 5e8a76b68..ab290c382 100644
> > --- a/lib/librte_net/Makefile
> > +++ b/lib/librte_net/Makefile
> > @@ -13,6 +13,7 @@ LIBABIVER := 1
> >
> >  SRCS-$(CONFIG_RTE_LIBRTE_NET) := rte_net.c
> >  SRCS-$(CONFIG_RTE_LIBRTE_NET) += rte_net_crc.c
> > +SRCS-$(CONFIG_RTE_LIBRTE_NET) += rte_arp.c
> >
> >  # install includes
> >  SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include := rte_ip.h rte_tcp.h
> rte_udp.h rte_esp.h
> > diff --git a/lib/librte_net/rte_arp.c b/lib/librte_net/rte_arp.c
> > new file mode 100644
> > index 0..d7223b044
> > --- /dev/null
> > +++ b/lib/librte_net/rte_arp.c
> > @@ -0,0 +1,42 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright(c) 2018 Intel Corporation
> > + */
> > +
> > +#include 
> > +
> > +#include 
> > +
> > +#define RARP_PKT_SIZE  64
> > +int
> > +rte_net_make_rarp_packet(struct rte_mbuf *mbuf, const struct ether_addr
> *mac)
> > +{
> > +   struct ether_hdr *eth_hdr;
> > +   struct arp_hdr *rarp;
> > +
> > +   if (mbuf->buf_len < RARP_PKT_SIZE)
> > +   return -1;
> > +
> > +   /* Ethernet header. */
> > +   eth_hdr = rte_pktmbuf_mtod(mbuf, struct ether_hdr *);
> > +   memset(eth_hdr->d_addr.addr_bytes, 0xff, ETHER_ADDR_LEN);
> > +   ether_addr_copy(mac, ð_hdr->s_addr);
> > +   eth_hdr->ether_type = htons(ETHER_TYPE_RARP);
> > +
> > +   /* RARP header. */
> > +   rarp = (struct arp_hdr *)(eth_hdr + 1);
> > +   rarp->arp_hrd = htons(ARP_HRD_ETHER);
> > +   rarp->arp_pro = htons(ETHER_TYPE_IPv4);
> > +   rarp->arp_hln = ETHER_ADDR_LEN;
> > +   rarp->arp_pln = 4;
> > +   rarp->arp_op  = htons(ARP_OP_REVREQUEST);
> > +
> > +   ether_addr_copy(mac, &rarp->arp_data.arp_sha);
> > +   ether_addr_copy(mac, &rarp->arp_data.arp_tha);
> > +   memset(&rarp->arp_data.arp_sip, 0x00, 4);
> > +   memset(&rarp->arp_data.arp_tip, 0x00, 4);
> > +
> > +   mbuf->data_len = RARP_PKT_SIZE;
> > +   mbuf->pkt_len = RARP_PKT_SIZE;
> > +
> > +   return 0;
> > +}
> 
> You don't check that there is enough tailroom to write the packet data.

Yes, tailroom can be used.

> Also, nothing verifies that the mbuf passed to the function is empty.
> I suggest to do the allocation in this function, what do you think?
>

I agree to allocate in this function and let it do all the checks.
 
> You can also use rte_pktmbuf_append() to check for the tailroom and
> update data_len/pkt_len:
> 
>   m = rte_pktmbuf_alloc();
>   if (m == NULL)
>   return NULL;
>   eth_hdr = rte_pktmbuf_append(m, RARP_PKT_SIZE);

When data_len is not enough, we need to rte_pktmbuf_append(m, RARP_PKT_SIZE - 
m->data_len);

>   if (eth_hdr == NULL) {
>   m_freem(m);
>   return NULL;
>   }
>   eth_hdr->... = ...;
>   ...
>   rarp = (struct arp_hdr *)(eth_hdr + 1);
>   rarp->... = ...;
>   ...
> 
>   return m;
> 

Will change it in next version, thanks for the comments.

BRs,
Xiao

Re: [dpdk-dev] [PATCH v3] ethdev: increase flow type limit from 32 to 64

[Rybalchenko, Kirill]

Hi Thomas,

> -Original Message-
> From: Thomas Monjalon [mailto:tho...@monjalon.net]
> Sent: Monday 15 January 2018 21:27
> To: Rybalchenko, Kirill 
> Cc: dev@dpdk.org; Chilikin, Andrey ; Yigit,
> Ferruh 
> Subject: Re: [PATCH v3] ethdev: increase flow type limit from 32 to 64
> 
> 15/01/2018 18:33, Kirill Rybalchenko:
> > --- a/lib/librte_ether/rte_eth_ctrl.h
> > +++ b/lib/librte_ether/rte_eth_ctrl.h
> > @@ -662,9 +662,9 @@ enum rte_fdir_mode {
> > RTE_FDIR_MODE_PERFECT_TUNNEL,   /**< Enable FDIR filter mode -
> tunnel. */
> >  };
> >
> > -#define UINT32_BIT (CHAR_BIT * sizeof(uint32_t))
> > +#define UINT64_BIT (CHAR_BIT * sizeof(uint64_t))
> >  #define RTE_FLOW_MASK_ARRAY_SIZE \
> > -   (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT32_BIT)/UINT32_BIT)
> > +   (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT64_BIT)/UINT64_BIT)
> >
> >  /**
> >   * A structure used to get the information of flow director filter.
> > @@ -681,7 +681,7 @@ struct rte_eth_fdir_info {
> > uint32_t guarant_spc; /**< Guaranteed spaces.*/
> > uint32_t best_spc; /**< Best effort spaces.*/
> > /** Bit mask for every supported flow type. */
> > -   uint32_t flow_types_mask[RTE_FLOW_MASK_ARRAY_SIZE];
> > +   uint64_t flow_types_mask[RTE_FLOW_MASK_ARRAY_SIZE];
> > uint32_t max_flexpayload; /**< Total flex payload in bytes. */
> > /** Flexible payload unit in bytes. Size and alignments of all flex
> > payload segments should be multiplies of this value. */ @@
> > -774,7 +774,7 @@ enum rte_eth_hash_function {  };
> >
> >  #define RTE_SYM_HASH_MASK_ARRAY_SIZE \
> > -   (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT32_BIT)/UINT32_BIT)
> > +   (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT64_BIT)/UINT64_BIT)
> >  /**
> >   * A structure used to set or get global hash function configurations which
> >   * include symmetric hash enable per flow type and hash function type.
> > @@ -787,9 +787,9 @@ enum rte_eth_hash_function {  struct
> > rte_eth_hash_global_conf {
> > enum rte_eth_hash_function hash_func; /**< Hash function type */
> > /** Bit mask for symmetric hash enable per flow type */
> > -   uint32_t
> sym_hash_enable_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> > +   uint64_t
> sym_hash_enable_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> > /** Bit mask indicates if the corresponding bit is valid */
> > -   uint32_t valid_bit_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> > +   uint64_t valid_bit_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> >  };
> 
> This is still changing the ABI.
> Am I missing something?
> 
We change size of structures rte_eth_fdir_info and rte_eth_hash_filter_info.
Application can use these structures for DPDK library API call only in
rte_eth_dev_filter_ctrl() function call. In the patch this function is
modified in the way that it will be compatible with user binary applications
compiled with previous versions of DPDK library.

Thanks,
Kirill.

Re: [dpdk-dev] [PATCH v3] ethdev: increase flow type limit from 32 to 64

[Thomas Monjalon]

16/01/2018 10:44, Rybalchenko, Kirill:
> Hi Thomas,
> 
> From: Thomas Monjalon [mailto:tho...@monjalon.net]
> > 15/01/2018 18:33, Kirill Rybalchenko:
> > > --- a/lib/librte_ether/rte_eth_ctrl.h
> > > +++ b/lib/librte_ether/rte_eth_ctrl.h
> > > @@ -662,9 +662,9 @@ enum rte_fdir_mode {
> > > RTE_FDIR_MODE_PERFECT_TUNNEL,   /**< Enable FDIR filter mode -
> > tunnel. */
> > >  };
> > >
> > > -#define UINT32_BIT (CHAR_BIT * sizeof(uint32_t))
> > > +#define UINT64_BIT (CHAR_BIT * sizeof(uint64_t))
> > >  #define RTE_FLOW_MASK_ARRAY_SIZE \
> > > -   (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT32_BIT)/UINT32_BIT)
> > > +   (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT64_BIT)/UINT64_BIT)
> > >
> > >  /**
> > >   * A structure used to get the information of flow director filter.
> > > @@ -681,7 +681,7 @@ struct rte_eth_fdir_info {
> > > uint32_t guarant_spc; /**< Guaranteed spaces.*/
> > > uint32_t best_spc; /**< Best effort spaces.*/
> > > /** Bit mask for every supported flow type. */
> > > -   uint32_t flow_types_mask[RTE_FLOW_MASK_ARRAY_SIZE];
> > > +   uint64_t flow_types_mask[RTE_FLOW_MASK_ARRAY_SIZE];
> > > uint32_t max_flexpayload; /**< Total flex payload in bytes. */
> > > /** Flexible payload unit in bytes. Size and alignments of all 
> > > flex
> > > payload segments should be multiplies of this value. */ @@
> > > -774,7 +774,7 @@ enum rte_eth_hash_function {  };
> > >
> > >  #define RTE_SYM_HASH_MASK_ARRAY_SIZE \
> > > -   (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT32_BIT)/UINT32_BIT)
> > > +   (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT64_BIT)/UINT64_BIT)
> > >  /**
> > >   * A structure used to set or get global hash function configurations 
> > > which
> > >   * include symmetric hash enable per flow type and hash function type.
> > > @@ -787,9 +787,9 @@ enum rte_eth_hash_function {  struct
> > > rte_eth_hash_global_conf {
> > > enum rte_eth_hash_function hash_func; /**< Hash function type */
> > > /** Bit mask for symmetric hash enable per flow type */
> > > -   uint32_t
> > sym_hash_enable_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> > > +   uint64_t
> > sym_hash_enable_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> > > /** Bit mask indicates if the corresponding bit is valid */
> > > -   uint32_t valid_bit_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> > > +   uint64_t valid_bit_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> > >  };
> > 
> > This is still changing the ABI.
> > Am I missing something?
> > 
> We change size of structures rte_eth_fdir_info and rte_eth_hash_filter_info.

Yes, and these structures are allocated and read by the application?
So it is an ABI break.

> Application can use these structures for DPDK library API call only in
> rte_eth_dev_filter_ctrl() function call. In the patch this function is
> modified in the way that it will be compatible with user binary applications
> compiled with previous versions of DPDK library.

Have you tried to use a patched DPDK with a binary compiled with DPDK 17.11?

Re: [dpdk-dev] [PATCH 1/3] app/testpmd: Moved cmdline_flow to librte_cmdline

[Gaëtan Rivet]

Hi Olivier, George, thanks for following up on that,

On Tue, Jan 16, 2018 at 10:24:25AM +0100, Olivier Matz wrote:
> Hi,
> 
> > On Tue, Jan 16, 2018 at 9:40 AM Olivier Matz  wrote:
> >
> > On Tue, Jan 16, 2018 at 08:45:32AM +, george@gmail.com wrote:
> > > Hi Georgios,
> > >
> > > On Mon, Jan 15, 2018 at 01:30:35AM +, Lu, Wenzhuo wrote:
> > > > Hi,
> > > >
> > > > > -Original Message-
> > > > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Georgios Katsikas
> > > > > Sent: Saturday, January 13, 2018 5:01 AM
> > > > > To: olivier.m...@6wind.com
> > > > > Cc: dev@dpdk.org; Georgios Katsikas 
> > > > > Subject: [dpdk-dev] [PATCH 1/3] app/testpmd: Moved cmdline_flow to
> > > > > librte_cmdline
> > > > >
> > > > > Signed-off-by: Georgios Katsikas 
> > > > Looks like a good idea to move this code to the lib.
> > > > cc Adrien the author of this file, app/test-pmd/cmdline_flow.c.
> > >
> > > If the command line parsing of rte_flow is something that has some
> > > chances to be shared among multiple applications, I agree it makes sense
> > > to move it in a library.
> > >
> > > However, my opinion is that it would be better to have a specific
> > > library for it, like librte_flow_cmdline, because I'm not sure that
> > > people linking with librte_cmdline always want to pull the rte_flow
> > > parsing code.
> > >

Why not a .config option to enable its build, instead of a separate
library?

> > >
> > Hi Lu, Oliver,
> > 
> > Thanks for your feedback!
> > You have a point here, flow commands are only a subset of the parser.
> > Do you want me to create this new library and send another patch?
> 
> Let's first wait for Adrien's feedback, he can have counter-arguments.
> 
> > I guess I have to use librte_cmdline as a template/example for creating the
> > librte_flow_cmdline library.
> 
> It can be used as an example for Makefile and .map file.

-- 
Gaëtan Rivet
6WIND

Re: [dpdk-dev] [PATCH v1 5/5] test: support for rawdev testcases

[Shreyansh Jain]

On Monday 15 January 2018 04:28 AM, Thomas Monjalon wrote:

02/01/2018 13:57, Shreyansh Jain:

Patch introduces rawdev unit testcase for validation against the
Skeleton rawdev dummy PMD implementation.

Signed-off-by: Shreyansh Jain 
---
  test/test/Makefile  |   4 +
  test/test/test_rawdev.c | 376 
  2 files changed, 380 insertions(+)


As it is being done for test_eventdev_octeontx.c, I think it is
a good idea to move the PMD unit tests in the PMD directory.
Please check the patches from Pavan, applied in dpdk-next-eventdev.




I saw the changes done by Pavan in eventdev-next.

Issue I see here is that rte_test.h is currently in eventdev tree. That 
would mean either I don't use those macros, or use it with this series 
as dependency on master absorbing eventdev-next in near rc.


I am assuming that post rc1 rte_test.h would be in master and thus it 
would be OK to apply this series over it during rc2. Is it OK with you?


-
Shreyansh

Re: [dpdk-dev] [PATCH v1 3/5] drivers/raw: introduce skeleton rawdev driver

[Shreyansh Jain]

On Monday 15 January 2018 04:24 AM, Thomas Monjalon wrote:

02/01/2018 13:57, Shreyansh Jain:

+struct skeleton_firmware {
+   struct skeleton_firmware_version_info firmware_version;
+   /**< Device firmware information */
+   enum skeleton_firmware_state firmware_state;
+   /**< Device state */
+};
+
+#define SKELETON_MAX_ATTRIBUTES 10
+#define SKELETON_ATTRIBUTE_NAME_MAX 20
+
+struct skeleton_rawdev_attributes {
+   char *name;
+   /**< Name of the attribute */
+   uint64_t value;
+   /**< Value or reference of value of attribute */
+};
+
+#define SKELETON_CAPA_FW_LOAD  0x0001
+/**< Device supports firmware loading/unloading */
+#define SKELETON_CAPA_FW_RESET  0x0002
+/**< Device supports firmware reset */
+#define SKELETON_CAPA_QUEUES0x0004
+/**< Device support queue based communication */


Comment about the style. The style is important :)
You are always writing comments after the item.


Yes, I was trying to stick to single method - postfix.


When comments are on a separate line, I think it is preferred to write
them *before* the item they describe.



Consider this:

struct dummy {
int a;  /**< a postfix comment */
/**< a prefix comment */
int b;
};

Personally, Even I prefer prefix when it comes to full line comments - 
but mixing prefix and postfix can lead to non-readable code.


Anyways - I was referring [1]

[1] 
http://dpdk.org/doc/guides/contributing/documentation.html#doxygen-guidelines


and it seems that I should change all to prefix in case any comment in 
the structure is not fitting into a single line.

I will do that.

Regards,
Shreyansh

Re: [dpdk-dev] [PATCH v2] vfio: noiommu check error handling

[Burakov, Anatoly]

On 15-Jan-18 12:22 PM, Jonas Pfefferle wrote:


  On Sat, 13 Jan 2018 23:49:30 +0100
  Thomas Monjalon  wrote:

13/01/2018 13:15, Burakov, Anatoly:

On 11-Jan-18 11:45 PM, Thomas Monjalon wrote:
> 07/11/2017 10:50, Jonas Pfefferle1:
>>> Is there something urgent for 17.11?
>>> Or can it be refined in 18.02?
>>
>> Nothing urgent. We can refine this for 18.02.
>>
>>> Anatoly, any thought?
> > Anatoly, Jonas, how do you want to proceed with this patch?
>
I don't see anything to be refined here, it's a simple bug fix - code 
assumes noiommu mode support is always available, when it might not 
be the case on older kernels.


As a bug fix, the title must start with "fix" and a tag "Fixes:"
must be added to help with backport.
At the same time, the explanation of the bug must be added in
the commit log please.

Thanks


It's not really a bug fix since it does not change the semantic of the 
function but just adds nicer error handling.


Well, as far as i can tell, it *does* change semantics - previously, if 
noiommu mode file was not found, we returned -1, now we return 0.



--
Thanks,
Anatoly

Re: [dpdk-dev] [PATCH] net/thunderx: Convert ThunderX VNIC PMD to new offload API

[Ferruh Yigit]

On 1/16/2018 9:32 AM, Maciej Czekaj wrote:
> 
> 
> 
> 
> -- Oryginal message --
>> On 1/15/2018 2:49 PM, Maciej Czekaj wrote:
>>>
>>>
>>>
>>> -- Oryginal message --
 On 1/3/2018 1:12 PM, maciej.cze...@caviumnetworks.com wrote:
> From: Maciej Czekaj 
>
> This patch removes all references to old-style offload API
> replacing them with new offload flags.
>
> Signed-off-by: Maciej Czekaj 
 <...>

>
>   dev_info->default_txconf = (struct rte_eth_txconf) {
>   .tx_free_thresh = NICVF_DEFAULT_TX_FREE_THRESH,
> - .txq_flags =
> - ETH_TXQ_FLAGS_NOMULTSEGS  |
> - ETH_TXQ_FLAGS_NOREFCOUNT  |
> - ETH_TXQ_FLAGS_NOMULTMEMP  |
> - ETH_TXQ_FLAGS_NOVLANOFFL  |
> - ETH_TXQ_FLAGS_NOXSUMSCTP,
> + .txq_flags = ETH_TXQ_FLAGS_IGNORE,
 I am not sure about this, Shahafs may comment better, shouldn't application
 decide to set "c" or not, instead of having this in default
 configuration?
>>> I think of it as a safeguard against a legacy application that is using
>>> old-style txq_flags.
>>>
>>> There is an assymetry in API, since
>>>    - rte_eth_tx_queue setup() and rte_eth_rx_queue_setup() convert flags
>>>    - rte_eth_dev_info_get() - does not convert them.
>>>
>>> The scenario leading to issues is as follows:
>>>
>>> 1.Application reads default txq_flags from the rte_eth_dev_info_get().
>>>    - dev_info.txconf.offloads contains flags but it is ignored by legacy 
>>> code
>>>    - dev_info.txq_flags may be:
>>>     a) txq_flags == 0
>>>         b) txq_flags ==  ETH_TXQ_FLAGS_IGNORE
>>>     c) txq_flags == new-style flags converted to old-style flags
>>>
>>>
>>> 2. Application uses default txq_flags field to rte_eth_tx_queue_setup().
>>> Now, depending on txq_flags:
>>>
>>>     a) txq_flags == 0, ethdev layer __clears__ all offloads, see *
>>>     b) txq_flags ==  ETH_TXQ_FLAGS_IGNORE, ethdev layer converts offloads
>>> to txq_flags, but leaves orignal offloads, so PMD can still use them
>>>     c) txq_flags == old-style flags, ethdev layer converts txq_flags to
>>> offloads, destroying default offloads
>>>
>>>
>>> * relevant code snippet from rte_eth_tx_queue_setup() with comments:
>>>
>>>       if (tx_conf->txq_flags & ETH_TXQ_FLAGS_IGNORE) {
>>>       // ==> converts offloads to txq_flags but LEAVES offloads, too
>>>           rte_eth_convert_txq_offloads(tx_conf->offloads,
>>>                        &local_conf.txq_flags);
>>>           /* Keep the ignore flag. */
>>>           local_conf.txq_flags |= ETH_TXQ_FLAGS_IGNORE;
>>>       } else {
>>>       // ==> converts txq_flags to offloads but DESTROYS original
>>> offloads
>>> rte_eth_convert_txq_flags(tx_conf->txq_flags,
>>>                         &local_conf.offloads);
>>>       }
>>>
>>>
>>> So, out of 3 options:
>>>
>>> a) does not work for legacy code
>>> b) will work for legacy code
>>> c) will work but defeats the purpose of removing old-style flags, since
>>> dev_info() callback has to setup both old-style and new-style default flags
>>>
>>> I chose b) as the simplest way to work-around the issue. I could post a
>>> patch to ethdev API if you think it is important.
>> What I understand is txq_flags should be supported because of legacy apps. 
>> That
>> is why ethdev layer converts old txq_flags to offloads when new
>> ETH_TXQ_FLAGS_IGNORE flag is missing. So that PMD can only use "offloads"
>> variable for both legacy and new applications.
>>
>> To support the case application uses defaults from PMD, the one you mentioned
>> above, I think we should go with option c). For implementation you can use 
>> only
>> new "offloads", so it is not both old-style and new-style, just new-style, 
>> but
>> as default yes should keep both.
>>
>> And new applications will set ETH_TXQ_FLAGS_IGNORE to say "offloads" has 
>> valid
>> information. Currently this also converted to the txq_flags but this will go
>> away when all PMDs support new method.
>>
>> Think  about a case where a legacy application gets defaults from PMD and 
>> sets a
>> few more values in txq_flags and configure the device. When you set
>> ETH_TXQ_FLAGS_IGNORE as part of defaults, ethdev layer will think only
>> "offloads" is valid and it will overwrite the txq_flags value with offloads 
>> one
>> and the updates to the txq_flags will be lost.
>>
>> At one point ETH_TXQ_FLAGS_IGNORE will also go away and applications also 
>> will
>> need to support new method. When it is removed than we can get rid of the
>> txq_flags defaults from PMDs until than I guess we need to live with them.
> 
> So you suggest that dev_info callback should provide default txq_flags 
> for a moment?
> 
> E.g.
> 
> .txq_flags =
>   ETH_TXQ_FLAGS_NOMULTSEGS  |
>   ETH_TXQ_FLAGS_NOREFCOUNT  |
>   ETH_TXQ_FLAGS_NOMULTMEMP  |
>   ETH_TXQ_FLAGS_NOVLANOFFL  |
>   ETH_TXQ_FLAGS_NOXSUMSCTP,
> 
>

Re: [dpdk-dev] [PATCH v3 1/8] net/failsafe: fix invalid free

[Gaëtan Rivet]

Hi Matan,

On Tue, Jan 09, 2018 at 02:47:26PM +, Matan Azrad wrote:
> From: Adrien Mazarguil 
> 
> rte_free() is not supposed to work with pointers returned by calloc().
> 
> Fixes: a0194d828100 ("net/failsafe: add flexible device definition")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Adrien Mazarguil 
Acked-by: Gaetan Rivet 
> ---
>  drivers/net/failsafe/failsafe_args.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/failsafe/failsafe_args.c 
> b/drivers/net/failsafe/failsafe_args.c
> index cfc83e3..ec63ac9 100644
> --- a/drivers/net/failsafe/failsafe_args.c
> +++ b/drivers/net/failsafe/failsafe_args.c
> @@ -407,7 +407,7 @@ typedef int (parse_cb)(struct rte_eth_dev *dev, const 
> char *params,
>   uint8_t i;
>  
>   FOREACH_SUBDEV(sdev, i, dev) {
> - rte_free(sdev->cmdline);
> + free(sdev->cmdline);
>   sdev->cmdline = NULL;
>   free(sdev->devargs.args);
>   sdev->devargs.args = NULL;
> -- 
> 1.8.3.1
> 

-- 
Gaëtan Rivet
6WIND

Re: [dpdk-dev] [PATCH v3] ethdev: increase flow type limit from 32 to 64

[Rybalchenko, Kirill]

> -Original Message-
> From: Thomas Monjalon [mailto:tho...@monjalon.net]
> Sent: Tuesday 16 January 2018 09:48
> To: Rybalchenko, Kirill 
> Cc: dev@dpdk.org; Chilikin, Andrey ; Yigit,
> Ferruh 
> Subject: Re: [PATCH v3] ethdev: increase flow type limit from 32 to 64
> 
> 16/01/2018 10:44, Rybalchenko, Kirill:
> > Hi Thomas,
> >
> > From: Thomas Monjalon [mailto:tho...@monjalon.net]
> > > 15/01/2018 18:33, Kirill Rybalchenko:
> > > > --- a/lib/librte_ether/rte_eth_ctrl.h
> > > > +++ b/lib/librte_ether/rte_eth_ctrl.h
> > > > @@ -662,9 +662,9 @@ enum rte_fdir_mode {
> > > > RTE_FDIR_MODE_PERFECT_TUNNEL,   /**< Enable FDIR filter mode
> -
> > > tunnel. */
> > > >  };
> > > >
> > > > -#define UINT32_BIT (CHAR_BIT * sizeof(uint32_t))
> > > > +#define UINT64_BIT (CHAR_BIT * sizeof(uint64_t))
> > > >  #define RTE_FLOW_MASK_ARRAY_SIZE \
> > > > -   (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT32_BIT)/UINT32_BIT)
> > > > +   (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT64_BIT)/UINT64_BIT)
> > > >
> > > >  /**
> > > >   * A structure used to get the information of flow director filter.
> > > > @@ -681,7 +681,7 @@ struct rte_eth_fdir_info {
> > > > uint32_t guarant_spc; /**< Guaranteed spaces.*/
> > > > uint32_t best_spc; /**< Best effort spaces.*/
> > > > /** Bit mask for every supported flow type. */
> > > > -   uint32_t flow_types_mask[RTE_FLOW_MASK_ARRAY_SIZE];
> > > > +   uint64_t flow_types_mask[RTE_FLOW_MASK_ARRAY_SIZE];
> > > > uint32_t max_flexpayload; /**< Total flex payload in bytes. */
> > > > /** Flexible payload unit in bytes. Size and alignments of all 
> > > > flex
> > > > payload segments should be multiplies of this value.
> > > > */ @@
> > > > -774,7 +774,7 @@ enum rte_eth_hash_function {  };
> > > >
> > > >  #define RTE_SYM_HASH_MASK_ARRAY_SIZE \
> > > > -   (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT32_BIT)/UINT32_BIT)
> > > > +   (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT64_BIT)/UINT64_BIT)
> > > >  /**
> > > >   * A structure used to set or get global hash function configurations
> which
> > > >   * include symmetric hash enable per flow type and hash function type.
> > > > @@ -787,9 +787,9 @@ enum rte_eth_hash_function {  struct
> > > > rte_eth_hash_global_conf {
> > > > enum rte_eth_hash_function hash_func; /**< Hash function type
> */
> > > > /** Bit mask for symmetric hash enable per flow type */
> > > > -   uint32_t
> > > sym_hash_enable_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> > > > +   uint64_t
> > > sym_hash_enable_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> > > > /** Bit mask indicates if the corresponding bit is valid */
> > > > -   uint32_t valid_bit_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> > > > +   uint64_t valid_bit_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> > > >  };
> > >
> > > This is still changing the ABI.
> > > Am I missing something?
> > >
> > We change size of structures rte_eth_fdir_info and
> rte_eth_hash_filter_info.
> 
> Yes, and these structures are allocated and read by the application?
> So it is an ABI break.
If application binary was compiled with previous version of DPDK it makes
no difference if these structures were used internally there - it still will 
work.
If application binary was recompiled with new version of DPDK - again,
It will work. 
The only issue is if application binary was compiled with old version of DPDK
library, but used with new version of DPDK shared library and uses those
structures to call functions from this DPDK library. But it can be done
only by   rte_eth_dev_filter_ctrl() function, which handles this case properly.
 
> 
> > Application can use these structures for DPDK library API call only in
> > rte_eth_dev_filter_ctrl() function call. In the patch this function is
> > modified in the way that it will be compatible with user binary
> > applications compiled with previous versions of DPDK library.
> 
> Have you tried to use a patched DPDK with a binary compiled with DPDK
> 17.11?

Yes, actually, I did run testpmd from 17.08 with patched DPDK shared library. 
It works fine, as described.

Thanks,
Kirill.

Re: [dpdk-dev] [PATCH v1 5/5] test: support for rawdev testcases

[Thomas Monjalon]

16/01/2018 11:07, Shreyansh Jain:
> On Monday 15 January 2018 04:28 AM, Thomas Monjalon wrote:
> > 02/01/2018 13:57, Shreyansh Jain:
> >> Patch introduces rawdev unit testcase for validation against the
> >> Skeleton rawdev dummy PMD implementation.
> >>
> >> Signed-off-by: Shreyansh Jain 
> >> ---
> >>   test/test/Makefile  |   4 +
> >>   test/test/test_rawdev.c | 376 
> >> 
> >>   2 files changed, 380 insertions(+)
> > 
> > As it is being done for test_eventdev_octeontx.c, I think it is
> > a good idea to move the PMD unit tests in the PMD directory.
> > Please check the patches from Pavan, applied in dpdk-next-eventdev.
> > 
> > 
> 
> I saw the changes done by Pavan in eventdev-next.
> 
> Issue I see here is that rte_test.h is currently in eventdev tree. That 
> would mean either I don't use those macros, or use it with this series 
> as dependency on master absorbing eventdev-next in near rc.
> 
> I am assuming that post rc1 rte_test.h would be in master and thus it 
> would be OK to apply this series over it during rc2. Is it OK with you?

Yes, you can assume next-eventdev will be pulled in master.

Re: [dpdk-dev] [PATCH v1 3/5] drivers/raw: introduce skeleton rawdev driver

[Thomas Monjalon]

16/01/2018 11:21, Shreyansh Jain:
> On Monday 15 January 2018 04:24 AM, Thomas Monjalon wrote:
> > 02/01/2018 13:57, Shreyansh Jain:
> >> +struct skeleton_firmware {
> >> +  struct skeleton_firmware_version_info firmware_version;
> >> +  /**< Device firmware information */
> >> +  enum skeleton_firmware_state firmware_state;
> >> +  /**< Device state */
> >> +};
> >> +
> >> +#define SKELETON_MAX_ATTRIBUTES 10
> >> +#define SKELETON_ATTRIBUTE_NAME_MAX 20
> >> +
> >> +struct skeleton_rawdev_attributes {
> >> +  char *name;
> >> +  /**< Name of the attribute */
> >> +  uint64_t value;
> >> +  /**< Value or reference of value of attribute */
> >> +};
> >> +
> >> +#define SKELETON_CAPA_FW_LOAD 0x0001
> >> +/**< Device supports firmware loading/unloading */
> >> +#define SKELETON_CAPA_FW_RESET  0x0002
> >> +/**< Device supports firmware reset */
> >> +#define SKELETON_CAPA_QUEUES0x0004
> >> +/**< Device support queue based communication */
> > 
> > Comment about the style. The style is important :)
> > You are always writing comments after the item.
> 
> Yes, I was trying to stick to single method - postfix.
> 
> > When comments are on a separate line, I think it is preferred to write
> > them *before* the item they describe.
> > 
> 
> Consider this:
> 
> struct dummy {
>  int a;  /**< a postfix comment */
>  /**< a prefix comment */
>  int b;
> };
> 
> Personally, Even I prefer prefix when it comes to full line comments - 
> but mixing prefix and postfix can lead to non-readable code.
> 
> Anyways - I was referring [1]
> 
> [1] 
> http://dpdk.org/doc/guides/contributing/documentation.html#doxygen-guidelines
> 
> and it seems that I should change all to prefix in case any comment in 
> the structure is not fitting into a single line.
> I will do that.

OK thanks

Re: [dpdk-dev] [PATCH v2 01/15] examples/eventdev: add Rx adapter support

[Van Haaren, Harry]

> From: Pavan Nikhilesh [mailto:pbhagavat...@caviumnetworks.com]
> Sent: Wednesday, January 10, 2018 11:10 AM
> To: Eads, Gage ; jerin.jacobkollanukka...@cavium.com;
> Van Haaren, Harry ; hemant.agra...@nxp.com; Ma,
> Liang J ; santosh.shu...@caviumnetworks.com
> Cc: dev@dpdk.org; Pavan Nikhilesh 
> Subject: [dpdk-dev] [PATCH v2 01/15] examples/eventdev: add Rx adapter
> support
> 
> Use event Rx adapter for packets Rx instead of explicit producer logic.
> Use service run iter function for granular control instead of using
> dedicated service lcore.
> 
> Signed-off-by: Pavan Nikhilesh 
> ---
> 
>  v2 Changes:
>   - split work funtion into delay cycles and excange_mac
>   - add option to configure mempool size
>   - remove prod_data structure(Gage)
>   - simplifly locks used while calling producer and scheduler(Gage)

Series-Acked-By: Harry van Haaren 

Re: [dpdk-dev] [PATCH v3] ethdev: increase flow type limit from 32 to 64

[Thomas Monjalon]

16/01/2018 11:31, Rybalchenko, Kirill:
> From: Thomas Monjalon [mailto:tho...@monjalon.net]
> > 16/01/2018 10:44, Rybalchenko, Kirill:
> > > Hi Thomas,
> > >
> > > From: Thomas Monjalon [mailto:tho...@monjalon.net]
> > > > 15/01/2018 18:33, Kirill Rybalchenko:
> > > > > --- a/lib/librte_ether/rte_eth_ctrl.h
> > > > > +++ b/lib/librte_ether/rte_eth_ctrl.h
> > > > > @@ -662,9 +662,9 @@ enum rte_fdir_mode {
> > > > > RTE_FDIR_MODE_PERFECT_TUNNEL,   /**< Enable FDIR filter mode
> > -
> > > > tunnel. */
> > > > >  };
> > > > >
> > > > > -#define UINT32_BIT (CHAR_BIT * sizeof(uint32_t))
> > > > > +#define UINT64_BIT (CHAR_BIT * sizeof(uint64_t))
> > > > >  #define RTE_FLOW_MASK_ARRAY_SIZE \
> > > > > -   (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT32_BIT)/UINT32_BIT)
> > > > > +   (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT64_BIT)/UINT64_BIT)
> > > > >
> > > > >  /**
> > > > >   * A structure used to get the information of flow director filter.
> > > > > @@ -681,7 +681,7 @@ struct rte_eth_fdir_info {
> > > > > uint32_t guarant_spc; /**< Guaranteed spaces.*/
> > > > > uint32_t best_spc; /**< Best effort spaces.*/
> > > > > /** Bit mask for every supported flow type. */
> > > > > -   uint32_t flow_types_mask[RTE_FLOW_MASK_ARRAY_SIZE];
> > > > > +   uint64_t flow_types_mask[RTE_FLOW_MASK_ARRAY_SIZE];
> > > > > uint32_t max_flexpayload; /**< Total flex payload in bytes. */
> > > > > /** Flexible payload unit in bytes. Size and alignments of 
> > > > > all flex
> > > > > payload segments should be multiplies of this value.
> > > > > */ @@
> > > > > -774,7 +774,7 @@ enum rte_eth_hash_function {  };
> > > > >
> > > > >  #define RTE_SYM_HASH_MASK_ARRAY_SIZE \
> > > > > -   (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT32_BIT)/UINT32_BIT)
> > > > > +   (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT64_BIT)/UINT64_BIT)
> > > > >  /**
> > > > >   * A structure used to set or get global hash function configurations
> > which
> > > > >   * include symmetric hash enable per flow type and hash function 
> > > > > type.
> > > > > @@ -787,9 +787,9 @@ enum rte_eth_hash_function {  struct
> > > > > rte_eth_hash_global_conf {
> > > > > enum rte_eth_hash_function hash_func; /**< Hash function type
> > */
> > > > > /** Bit mask for symmetric hash enable per flow type */
> > > > > -   uint32_t
> > > > sym_hash_enable_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> > > > > +   uint64_t
> > > > sym_hash_enable_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> > > > > /** Bit mask indicates if the corresponding bit is valid */
> > > > > -   uint32_t valid_bit_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> > > > > +   uint64_t valid_bit_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> > > > >  };
> > > >
> > > > This is still changing the ABI.
> > > > Am I missing something?
> > > >
> > > We change size of structures rte_eth_fdir_info and
> > rte_eth_hash_filter_info.
> > 
> > Yes, and these structures are allocated and read by the application?
> > So it is an ABI break.
> If application binary was compiled with previous version of DPDK it makes
> no difference if these structures were used internally there - it still will 
> work.
> If application binary was recompiled with new version of DPDK - again,
> It will work. 
> The only issue is if application binary was compiled with old version of DPDK
> library, but used with new version of DPDK shared library and uses those
> structures to call functions from this DPDK library. But it can be done
> only by   rte_eth_dev_filter_ctrl() function, which handles this case 
> properly.

OK got it.
Thanks for your patience :)

> > > Application can use these structures for DPDK library API call only in
> > > rte_eth_dev_filter_ctrl() function call. In the patch this function is
> > > modified in the way that it will be compatible with user binary
> > > applications compiled with previous versions of DPDK library.
> > 
> > Have you tried to use a patched DPDK with a binary compiled with DPDK
> > 17.11?
> 
> Yes, actually, I did run testpmd from 17.08 with patched DPDK shared library. 
> It works fine, as described.
> 
> Thanks,
> Kirill.

Re: [dpdk-dev] [PATCH v10 3/5] net: add a helper for making RARP packet

[Olivier Matz]

Hi Xiao,

On Tue, Jan 16, 2018 at 09:43:43AM +, Wang, Xiao W wrote:
> Hi Olivier,
> > You can also use rte_pktmbuf_append() to check for the tailroom and
> > update data_len/pkt_len:
> > 
> > m = rte_pktmbuf_alloc();
> > if (m == NULL)
> > return NULL;
> > eth_hdr = rte_pktmbuf_append(m, RARP_PKT_SIZE);
> 
> When data_len is not enough, we need to rte_pktmbuf_append(m, RARP_PKT_SIZE - 
> m->data_len);

Sorry, I don't get your point here.

Re: [dpdk-dev] [PATCH v3 2/8] net/failsafe: add "fd" parameter

[Gaëtan Rivet]

Hi Matam, Adrien,

On Tue, Jan 09, 2018 at 02:47:27PM +, Matan Azrad wrote:
> From: Adrien Mazarguil 
> 
> This parameter enables applications to provide device definitions through
> an arbitrary file descriptor number.

Ok on the principle,



> @@ -161,6 +165,73 @@ typedef int (parse_cb)(struct rte_eth_dev *dev, const 
> char *params,
>  }
>  
>  static int
> +fs_read_fd(struct sub_device *sdev, char *fd_str)
> +{
> +FILE *fp = NULL;
> +int fd = -1;
> +/* store possible newline as well */
> +char output[DEVARGS_MAXLEN + 1];
> +int err = -ENODEV;
> +int ret;

ret is used as flag older, line counter and then error reporting.
err should be the only variable used for reading errors from function
and reporting it.

It would be clearer to use descriptive names, such as "oflags" and "nl"
or "lcount". I don't really care about one additional variable in this
function, for the sake of expressiveness.

> +
> +RTE_ASSERT(fd_str != NULL || sdev->fd_str != NULL);
> +if (sdev->fd_str == NULL) {
> +sdev->fd_str = strdup(fd_str);
> +if (sdev->fd_str == NULL) {
> +ERROR("Command line allocation failed");
> +return -ENOMEM;
> +}
> +}
> +errno = 0;
> +fd = strtol(fd_str, &fd_str, 0);
> +if (errno || *fd_str || fd < 0) {
> +ERROR("Parsing FD number failed");
> +goto error;
> +}
> +/* Fiddle with copy of file descriptor */
> +fd = dup(fd);
> +if (fd == -1)
> +goto error;
> +ret = fcntl(fd, F_GETFL);

oflags = fcntl(...);

> +if (ret == -1)
> +goto error;
> +ret = fcntl(fd, F_SETFL, fd | O_NONBLOCK);

err = fcntl(fd, F_SETFL, oflags | O_NONBLOCK);
Using (fd | O_NONBLOCK) is probably a mistake.

> +if (ret == -1)
> +goto error;
> +fp = fdopen(fd, "r");
> +if (!fp)
> +goto error;
> +fd = -1;
> +/* Only take the last line into account */
> +ret = 0;
> +while (fgets(output, sizeof(output), fp))
> +++ret;

   lcount = 0;
   while (fgets(output, sizeof(output), fp))
   ++lcount;


> +if (feof(fp)) {
> +if (!ret)
> +goto error;
> +} else if (ferror(fp)) {
> +if (errno != EAGAIN || !ret)
> +goto error;
> +} else if (!ret) {
> +goto error;
> +}

These branches seems needlessly complicated:

if (lcount == 0)
goto error;
else if (ferror(fp) && errno != EAGAIN)
goto error;

> +/* Line must end with a newline character */
> +fs_sanitize_cmdline(output);
> +if (output[0] == '\0')
> +goto error;
> +ret = fs_parse_device(sdev, output);
> +if (ret)
> +ERROR("Parsing device '%s' failed", output);
> +err = ret;

no need to use ret instead of err here?

   err = fs_parse_device(sdev, output);
   if (err)
   ERROR("Parsing device '%s' failed", output);

Thus allowing to remove the "ret" variable completely.

> +error:
> +if (fp)
> +fclose(fp);
> +if (fd != -1)
> +close(fd);
> +return err;
> +}
> +
> +static int
>  fs_parse_device_param(struct rte_eth_dev *dev, const char *param,
>  uint8_t head)
>  {
> @@ -202,6 +273,14 @@ typedef int (parse_cb)(struct rte_eth_dev *dev, const 
> char *params,
>  }
>  if (ret)
>  goto free_args;
> +} else if (strncmp(param, "fd", 2) == 0) {

How about strncmp(param, "fd(", 3) == 0 here?
I think I made a mistake for dev and exec device types, no reason at
this point to reiterate for fd as well.

> +ret = fs_read_fd(sdev, args);
> +if (ret == -ENODEV) {
> +DEBUG("Reading device info from FD failed");
> +ret = 0;
> +}
> +if (ret)
> +goto free_args;
>  } else {
>  ERROR("Unrecognized device type: %.*s", (int)b, param);
>  return -EINVAL;
> @@ -409,6 +488,8 @@ typedef int (parse_cb)(struct rte_eth_dev *dev, const 
> char *params,
>  FOREACH_SUBDEV(sdev, i, dev) {
>  free(sdev->cmdline);
>  sdev->cmdline = NULL;
> +free(sdev->fd_str);
> +sdev->fd_str = NULL;
>  free(sdev->devargs.args);
>  sdev->devargs.args = NULL;
>  }
> @@ -424,7 +505,8 @@ typedef int (parse_cb)(struct rte_eth_dev *dev, const 
> char *params,
>  param[b] != '\0')
>  b++;
> 

Re: [dpdk-dev] [PATCH] examples/ipsec-secgw: update incremental checksum

[Nicolau, Radu]

> -Original Message-
> From: Akhil Goyal [mailto:akhil.go...@nxp.com]
> Sent: Tuesday, January 16, 2018 6:30 AM
> To: Nicolau, Radu ; dev@dpdk.org
> Cc: De Lara Guarch, Pablo ;
> hemant.agra...@nxp.com
> Subject: Re: [PATCH] examples/ipsec-secgw: update incremental checksum
> 
> Hi Radu,
> On 1/15/2018 8:10 PM, Nicolau, Radu wrote:
> >
> >
> >> -Original Message-
> >> From: Akhil Goyal [mailto:akhil.go...@nxp.com]
> >> Sent: Monday, January 15, 2018 12:48 PM
> >> To: dev@dpdk.org
> >> Cc: De Lara Guarch, Pablo ;
> >> hemant.agra...@nxp.com; Nicolau, Radu 
> >> Subject: Re: [PATCH] examples/ipsec-secgw: update incremental
> >> checksum
> >>
> >> On 1/15/2018 6:12 PM, Akhil Goyal wrote:
> >>> When TTL is decremented or ecn is updated in IP header before
> >>> forwarding the packet, checksum needs to be updated.
> >>>
> >>> In this patch an incremental checksum is added for ipv4 case.
> >>>
> >>> Signed-off-by: Akhil Goyal 
> >>> ---
> >> This patch is an update to a very old patch which was rejected earlier.
> >> http://dpdk.org/dev/patchwork/patch/16113/
> >>
> >>>examples/ipsec-secgw/ipip.h | 19 ++-
> >>>1 file changed, 18 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/examples/ipsec-secgw/ipip.h
> >>> b/examples/ipsec-secgw/ipip.h index fb6a6fa..13b8455 100644
> >>> --- a/examples/ipsec-secgw/ipip.h
> >>> +++ b/examples/ipsec-secgw/ipip.h
> >>> @@ -27,6 +27,10 @@ ipip_outbound(struct rte_mbuf *m, uint32_t
> >>> offset,
> >> uint32_t is_ipv6,
> >>>   if (inip4->ip_v == IPVERSION) {
> >>>   /* XXX This should be done by the forwarding engine 
> >>> instead
> >> */
> >>>   inip4->ip_ttl -= 1;
> >>> + if (inip4->ip_sum >= rte_cpu_to_be_16(0x - 0x100))
> >>> + inip4->ip_sum += rte_cpu_to_be_16(0x100) + 1;
> >>> + else
> >>> + inip4->ip_sum += rte_cpu_to_be_16(0x100);
> >>>   ds_ecn = inip4->ip_tos;
> >>>   } else {
> >>>   inip6 = (struct ip6_hdr *)inip4; @@ -95,8 +99,17 @@
> >>> ip6ip_outbound(struct rte_mbuf *m, uint32_t offset,
> >>>static inline void
> >>>ip4_ecn_setup(struct ip *ip4)
> >>>{
> >>> - if (ip4->ip_tos & IPTOS_ECN_MASK)
> >>> + if (ip4->ip_tos & IPTOS_ECN_MASK) {
> >>> + unsigned long sum;
> >>> + uint8_t old;
> >>> +
> >>> + old = ip4->ip_tos;
> >>>   ip4->ip_tos |= IPTOS_ECN_CE;
> >>> + sum = old + (~(*(uint8_t *)&ip4->ip_tos) & 0xff);
> >>> + sum += rte_be_to_cpu_16(ip4->ip_sum);
> >>> + sum = (sum & 0x) + (sum >> 16);
> >>> + ip4->ip_sum = rte_cpu_to_be_16(sum + (sum >> 16));
> >>> + }
> >>>}
> >>>
> >>>static inline void
> >>> @@ -140,6 +153,10 @@ ipip_inbound(struct rte_mbuf *m, uint32_t
> offset)
> >>>   ip4_ecn_setup(inip4);
> >>>   /* XXX This should be done by the forwarding engine 
> >>> instead
> >> */
> >>>   inip4->ip_ttl -= 1;
> >>> + if (inip4->ip_sum >= rte_cpu_to_be_16(0x - 0x100))
> >>> + inip4->ip_sum += rte_cpu_to_be_16(0x100) + 1;
> >>> + else
> >>> + inip4->ip_sum += rte_cpu_to_be_16(0x100);
> >>>   m->packet_type &= ~RTE_PTYPE_L4_MASK;
> >>>   if (inip4->ip_p == IPPROTO_UDP)
> >>>   m->packet_type |= RTE_PTYPE_L4_UDP;
> >>>
> >
> > I think instead of manipulating the checksum in this way it will be better 
> > to
> use rte_ipv4_cksum to re-compute it, unless the performance penalty is too
> significant.
> >
> There would be unnecessary wastage of cycles. This way of updating the
> checksum is implemented as per the RFC1141
> https://tools.ietf.org/html/rfc1141
> Do you see any issue in this way of updating the checksum?
No, I just tought that it will make the code look nicer.

Acked-by: Radu Nicolau 

Re: [dpdk-dev] [PATCH v6 2/2] examples/ipsec-secgw: add support for inline protocol

[Nicolau, Radu]

> -Original Message-
> From: Anoob Joseph [mailto:anoob.jos...@caviumnetworks.com]
> Sent: Monday, December 18, 2017 7:15 AM
> To: Akhil Goyal ; Doherty, Declan
> ; Nicolau, Radu ;
> Gonzalez Monroy, Sergio 
> Cc: Anoob Joseph ; Jerin Jacob
> ; Narayana Prasad
> ; dev@dpdk.org
> Subject: [PATCH v6 2/2] examples/ipsec-secgw: add support for inline
> protocol
> 
> Adding support for inline protocol processing
> 
> In ingress side, application will receive regular IP packets, without any 
> IPsec
> related info. Application will do a selector check (SP-SA
> check) by making use of the metadata from the packet. The device-specific
> metadata in mbuf would aid in determing the security session which
> processed the packet.
> 
> In egress side, the plain packet would be submitted to the driver. The packet
> will have optional metadata, which could be used to identify the security
> session associated with the packet.
> 
> Signed-off-by: Anoob Joseph 
> ---
Acked-by: Radu Nicolau 

Re: [dpdk-dev] [PATCH] examples/ipsec-secgw: fix corner case for spi value

[Nicolau, Radu]

> -Original Message-
> From: Akhil Goyal [mailto:akhil.go...@nxp.com]
> Sent: Thursday, January 11, 2018 11:56 AM
> To: dev@dpdk.org
> Cc: De Lara Guarch, Pablo ;
> hemant.agra...@nxp.com; Gonzalez Monroy, Sergio
> ; Nicolau, Radu
> ; Akhil Goyal 
> Subject: [PATCH] examples/ipsec-secgw: fix corner case for spi value
> 
> application is using index 0 of SA table as error, with current value of
> IPSEC_SA_MAX_ENTRIES(128) it can not support SA with spi = 128, as it uses
> sa_idx = 0 in the SA table.
> 
> With this patch, sa_idx = 0 can also be used.
> 
> PS: spi = 0 is an invalid SPI and application throws error for it.
> 
> Fixes: d299106e8e31 ("examples/ipsec-secgw: add IPsec sample application")
> 
> Signed-off-by: Akhil Goyal 
> ---
Acked-by: Radu Nicolau 

Re: [dpdk-dev] [PATCH v10 3/5] net: add a helper for making RARP packet

[Wang, Xiao W]

Hi Olivier,

> -Original Message-
> From: Olivier Matz [mailto:olivier.m...@6wind.com]
> Sent: Tuesday, January 16, 2018 6:43 PM
> To: Wang, Xiao W 
> Cc: y...@fridaylinux.org; tho...@monjalon.net; Bie, Tiwei
> ; dev@dpdk.org; step...@networkplumber.org;
> maxime.coque...@redhat.com
> Subject: Re: [dpdk-dev] [PATCH v10 3/5] net: add a helper for making RARP
> packet
> 
> Hi Xiao,
> 
> On Tue, Jan 16, 2018 at 09:43:43AM +, Wang, Xiao W wrote:
> > Hi Olivier,
> > > You can also use rte_pktmbuf_append() to check for the tailroom and
> > > update data_len/pkt_len:
> > >
> > >   m = rte_pktmbuf_alloc();

I just realized that if we let this function to allocate mbuf, it may restrict 
this api's applicability.
E.g. the caller just has a mbuf, without a mempool.
How do you think?

> > >   if (m == NULL)
> > >   return NULL;
> > >   eth_hdr = rte_pktmbuf_append(m, RARP_PKT_SIZE);
> >
> > When data_len is not enough, we need to rte_pktmbuf_append(m,
> RARP_PKT_SIZE - m->data_len);
> 
> Sorry, I don't get your point here.

I mean we just need to extend the data_len by "RARP_PKT_SIZE - m->data_len" 
when the room is not big enough.

BRs,
Xiao

Re: [dpdk-dev] [PATCH v3 3/8] net/failsafe: support probed sub-devices getting

[Gaëtan Rivet]

Hi Matan,

I'n not fond of the commit title, how about:

[PATCH v3 3/8] net/failsafe: add probed etherdev capture

?

On Tue, Jan 09, 2018 at 02:47:28PM +, Matan Azrad wrote:
> Previous fail-safe code didn't support getting probed sub-devices and
> failed when it tried to probe them.
> 
> Skip fail-safe sub-device probing when it already was probed.
> 
> Signed-off-by: Matan Azrad 
> Cc: Gaetan Rivet 
> ---
>  doc/guides/nics/fail_safe.rst   |  5 
>  drivers/net/failsafe/failsafe_eal.c | 60 
> -
>  2 files changed, 45 insertions(+), 20 deletions(-)
> 
> diff --git a/doc/guides/nics/fail_safe.rst b/doc/guides/nics/fail_safe.rst
> index 5b1b47e..b89e53b 100644
> --- a/doc/guides/nics/fail_safe.rst
> +++ b/doc/guides/nics/fail_safe.rst
> @@ -115,6 +115,11 @@ Fail-safe command line parameters
>order to take only the last line into account (unlike ``exec()``) at every
>probe attempt.
>  
> +.. note::
> +
> +   In case of whitelist sub-device probed by EAL, fail-safe PMD will take 
> the device
> +   as is, which means that EAL device options are taken in this case.
> +
>  - **mac** parameter [MAC address]
>  
>This parameter allows the user to set a default MAC address to the 
> fail-safe
> diff --git a/drivers/net/failsafe/failsafe_eal.c 
> b/drivers/net/failsafe/failsafe_eal.c
> index 19d26f5..7bc7453 100644
> --- a/drivers/net/failsafe/failsafe_eal.c
> +++ b/drivers/net/failsafe/failsafe_eal.c
> @@ -36,39 +36,59 @@
>  #include "failsafe_private.h"
>  
>  static int
> +fs_get_port_by_device_name(const char *name, uint16_t *port_id)

The naming convention for the failsafe driver is

  namespace_object_sub-object_action()

With an ordering of objects by their scope (std, rte, failsafe, file).
Also, "get" as an action is not descriptive enough.

static int
fs_ethdev_capture(const char *name, uint16_t *port_id);

> +{
> + uint16_t pid;
> + size_t len;
> +
> + if (name == NULL) {
> + DEBUG("Null pointer is specified\n");
> + return -EINVAL;
> + }
> + len = strlen(name);
> + RTE_ETH_FOREACH_DEV(pid) {
> + if (!strncmp(name, rte_eth_devices[pid].device->name, len)) {
> + *port_id = pid;
> + return 0;
> + }
> + }
> + return -ENODEV;
> +}
> +
> +static int
>  fs_bus_init(struct rte_eth_dev *dev)
>  {
>   struct sub_device *sdev;
>   struct rte_devargs *da;
>   uint8_t i;
> - uint16_t j;
> + uint16_t pid;
>   int ret;
>  
>   FOREACH_SUBDEV(sdev, i, dev) {
>   if (sdev->state != DEV_PARSED)
>   continue;
>   da = &sdev->devargs;
> - ret = rte_eal_hotplug_add(da->bus->name,
> -   da->name,
> -   da->args);
> - if (ret) {
> - ERROR("sub_device %d probe failed %s%s%s", i,
> -   rte_errno ? "(" : "",
> -   rte_errno ? strerror(rte_errno) : "",
> -   rte_errno ? ")" : "");
> - continue;
> - }
> - RTE_ETH_FOREACH_DEV(j) {
> - if (strcmp(rte_eth_devices[j].device->name,
> - da->name) == 0) {
> - ETH(sdev) = &rte_eth_devices[j];
> - break;
> + if (fs_get_port_by_device_name(da->name, &pid) != 0) {
> + ret = rte_eal_hotplug_add(da->bus->name,
> +   da->name,
> +   da->args);
> + if (ret) {
> + ERROR("sub_device %d probe failed %s%s%s", i,
> +   rte_errno ? "(" : "",
> +   rte_errno ? strerror(rte_errno) : "",
> +   rte_errno ? ")" : "");
> + continue;
>   }
> + if (fs_get_port_by_device_name(da->name, &pid) != 0) {
> + ERROR("sub_device %d init went wrong", i);
> + return -ENODEV;
> + }
> + } else {
> + /* Take control of device probed by EAL options. */
> + DEBUG("Taking control of a probed sub device"
> +   " %d named %s", i, da->name);

In this case, the devargs of the probed device must be copied within the
sub-device definition and removed from the EAL using the proper
rte_devargs API.

Note that there is no rte_devargs copy function. You can use
rte_devargs_parse instead, "parsing" again the original devargs into the
sub-device one. It is necessary for complying with internal rte_devargs
requirements (da->args being malloc-ed, at the moment, but may evolve).

The

Re: [dpdk-dev] [PATCH v3 15/39] examples/ipsec-secgw: convert to new ethdev offloads API

[Nicolau, Radu]

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Shahaf Shuler
> Sent: Tuesday, December 26, 2017 9:23 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v3 15/39] examples/ipsec-secgw: convert to
> new ethdev offloads API
> 
> Ethdev offloads API has changed since:
> 
> commit ce17eddefc20 ("ethdev: introduce Rx queue offloads API") commit
> cba7f53b717d ("ethdev: introduce Tx queue offloads API")
> 
> This commit support the new API.
> 
> Signed-off-by: Shahaf Shuler 
> ---
> 

Acked-by: Radu Nicolau 

[dpdk-dev] [PATCH] crypto/dpaa: retire fq while detaching with session

[akhil.goyal]

From: Alok Makhariya 

Signed-off-by: Alok Makhariya 
Acked-by: Akhil Goyal 
---
This patch is dependent on a patch which is already merged on net-next.
4b60b1e2f8a7 ("bus/dpaa: add support for static queues")

 drivers/crypto/dpaa_sec/dpaa_sec.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/crypto/dpaa_sec/dpaa_sec.c 
b/drivers/crypto/dpaa_sec/dpaa_sec.c
index d7b6f39..9b02c86 100644
--- a/drivers/crypto/dpaa_sec/dpaa_sec.c
+++ b/drivers/crypto/dpaa_sec/dpaa_sec.c
@@ -1248,6 +1248,8 @@ dpaa_sec_detach_rxq(struct dpaa_sec_dev_private *qi, 
struct qman_fq *fq)
 
for (i = 0; i < qi->max_nb_sessions; i++) {
if (&qi->inq[i] == fq) {
+   qman_retire_fq(fq, NULL);
+   qman_oos_fq(fq);
qi->inq_attach[i] = 0;
return 0;
}
-- 
2.9.3

Re: [dpdk-dev] [PATCH v2 3/4] eal: add synchronous multi-process communication

[Ananyev, Konstantin]

Hi Jianfeng,

> -Original Message-
> From: Tan, Jianfeng
> Sent: Tuesday, January 16, 2018 8:11 AM
> To: Ananyev, Konstantin ; dev@dpdk.org; 
> Burakov, Anatoly 
> Cc: Richardson, Bruce ; tho...@monjalon.net
> Subject: Re: [PATCH v2 3/4] eal: add synchronous multi-process communication
> 
> Thank you, Konstantin and Anatoly firstly. Other comments are well
> received and I'll send out a new version.
> 
> 
> On 1/16/2018 8:00 AM, Ananyev, Konstantin wrote:
> >
> >> We need the synchronous way for multi-process communication,
> >> i.e., blockingly waiting for reply message when we send a request
> >> to the peer process.
> >>
> >> We add two APIs rte_eal_mp_request() and rte_eal_mp_reply() for
> >> such use case. By invoking rte_eal_mp_request(), a request message
> >> is sent out, and then it waits there for a reply message. The
> >> timeout is hard-coded 5 Sec. And the replied message will be copied
> >> in the parameters of this API so that the caller can decide how
> >> to translate those information (including params and fds). Note
> >> if a primary process owns multiple secondary processes, this API
> >> will fail.
> >>
> >> The API rte_eal_mp_reply() is always called by an mp action handler.
> >> Here we add another parameter for rte_eal_mp_t so that the action
> >> handler knows which peer address to reply.
> >>
> >> We use mutex in rte_eal_mp_request() to guarantee that only one
> >> request is on the fly for one pair of processes.
> > You don't need to do things in such strange and restrictive way.
> > Instead you can do something like that:
> > 1) Introduce new struct, list for it and mutex
> >   struct sync_request {
> >int reply_received;
> >char dst[PATH_MAX];
> >char reply[...];
> >LIST_ENTRY(sync_request) next;
> > };
> >
> > static struct
> >  LIST_HEAD(list, sync_request);
> >  pthread_mutex_t lock;
> > pthead_cond_t cond;
> > } sync_requests;
> >
> > 2) then at request() call:
> >Grab sync_requests.lock
> >Check do we already have a pending request for that destination,
> >If yes - the release the lock and returns with error.
> >- allocate and init new sync_request struct, set reply_received=0
> >- do send_msg()
> >-then in a cycle:
> >pthread_cond_timed_wait(&sync_requests.cond, &sync_request.lock, 
> > ×pec);
> >- at return from it check if sync_request.reply_received == 1, if not
> > check if timeout expired and either return a failure or go to the start of 
> > the cycle.
> >
> > 3) at mp_handler() if REPLY received - grab sync_request.lock,
> >  search through sync_requests.list for dst[] ,
> > if found, then set it's reply_received=1, copy the received message 
> > into reply
> > and call pthread_cond_braodcast((&sync_requests.cond);
> 
> The only benefit I can see is that now the sender can request to
> multiple receivers at the same time. And it makes things more
> complicated. Do we really need this?

The benefit is that one thread is blocked waiting for response,
your mp_handler can still receive and handle other messages.
Plus as you said - other threads can keep sending messages.
Konstantin 

> 
> Thanks,
> Jianfeng

Re: [dpdk-dev] [PATCH] ethdev: increase flow type limit from 32 to 64

[Adrien Mazarguil]

On Tue, Jan 09, 2018 at 03:16:13PM +, Rybalchenko, Kirill wrote:
> > -Original Message-
> > From: Adrien Mazarguil [mailto:adrien.mazarg...@6wind.com]
> > Sent: Monday 4 December 2017 17:43
> > To: Rybalchenko, Kirill 
> > Cc: dev@dpdk.org; Wu, Jingjing ; Xing, Beilei
> > ; johnd...@cisco.com; neesc...@cisco.com;
> > nelio.laranje...@6wind.com; ys...@mellanox.com; Lu, Wenzhuo
> > ; Ananyev, Konstantin
> > ; Chilikin, Andrey
> > 
> > Subject: Re: [PATCH] ethdev: increase flow type limit from 32 to 64
> > 
> > Hi Kirill,
> > 
> > On Mon, Nov 27, 2017 at 12:29:47PM +, Kirill Rybalchenko wrote:
> > > Increase the internal limit for flow types from 32 to 64 to support
> > > future flow type extensions.
> > > Change type of variables from uint32_t[] to uint64_t[]:
> > >   rte_eth_fdir_info.flow_types_mask
> > >   rte_eth_hash_global_conf.sym_hash_enable_mask
> > >   rte_eth_hash_global_conf.valid_bit_mask
> > >
> > > This modification affects the following components:
> > >   net/i40e
> > >   net/enic
> > >   net/mlx5
> > >   net/ixgbe
> > >   app/testpmd
> > >
> > > Signed-off-by: Kirill Rybalchenko 
> > 
> > Can you elaborate a bit on the need for these changes?
> > Have you considered implementing those future extensions through
> > rte_flow instead?
> 
> Hi Adrien, this is not a new feature but rather fix of existing limitation.
> In current implementation the symmetric hash mask and flow mask are
> represented by 32-bit variable, while hardware bitmask has 64 bits.
> Unfortunately, this modification changes ABI of the library as it changes size
> of rte_eth_fdir_info structure. All related PMDs (listed above) had to be 
> modified
> accordingly.  

OK, no problem with this change. I assume you'll re-submit it since you sent
a deprecation notice, we'll review/ack subsequent mlx5 patches.

-- 
Adrien Mazarguil
6WIND

Re: [dpdk-dev] [PATCH] doc: remove UTF-8 BOM from programmer's guide

[Burakov, Anatoly]

On 21-Dec-17 3:28 PM, Anatoly Burakov wrote:

Fixes: 55694b2a9f64 ("doc: add membership documentation")
Cc: yipeng1.w...@intel.com
Signed-off-by: Anatoly Burakov 
---


Ping?

--
Thanks,
Anatoly

Re: [dpdk-dev] [PATCH v3 2/8] net/failsafe: add "fd" parameter

[Gaëtan Rivet]

Hi again,

made a mistake in reviewing, see below.

On Tue, Jan 16, 2018 at 11:54:43AM +0100, Gaëtan Rivet wrote:
> Hi Matam, Adrien,
> 
> On Tue, Jan 09, 2018 at 02:47:27PM +, Matan Azrad wrote:
> > From: Adrien Mazarguil 
> > 
> > This parameter enables applications to provide device definitions through
> > an arbitrary file descriptor number.
> 
> Ok on the principle,
> 
> 
> 
> > @@ -161,6 +165,73 @@ typedef int (parse_cb)(struct rte_eth_dev *dev, const 
> > char *params,
> >  }
> >  
> >  static int
> > +fs_read_fd(struct sub_device *sdev, char *fd_str)
> > +{
> > +FILE *fp = NULL;
> > +int fd = -1;
> > +/* store possible newline as well */
> > +char output[DEVARGS_MAXLEN + 1];
> > +int err = -ENODEV;
> > +int ret;
> 
> ret is used as flag older, line counter and then error reporting.
> err should be the only variable used for reading errors from function
> and reporting it.
> 
> It would be clearer to use descriptive names, such as "oflags" and "nl"
> or "lcount". I don't really care about one additional variable in this
> function, for the sake of expressiveness.
> 
> > +
> > +RTE_ASSERT(fd_str != NULL || sdev->fd_str != NULL);
> > +if (sdev->fd_str == NULL) {
> > +sdev->fd_str = strdup(fd_str);
> > +if (sdev->fd_str == NULL) {
> > +ERROR("Command line allocation failed");
> > +return -ENOMEM;
> > +}
> > +}
> > +errno = 0;
> > +fd = strtol(fd_str, &fd_str, 0);
> > +if (errno || *fd_str || fd < 0) {
> > +ERROR("Parsing FD number failed");
> > +goto error;
> > +}
> > +/* Fiddle with copy of file descriptor */
> > +fd = dup(fd);
> > +if (fd == -1)
> > +goto error;
> > +ret = fcntl(fd, F_GETFL);
> 
> oflags = fcntl(...);
> 
> > +if (ret == -1)
> > +goto error;
> > +ret = fcntl(fd, F_SETFL, fd | O_NONBLOCK);
> 
> err = fcntl(fd, F_SETFL, oflags | O_NONBLOCK);
> Using (fd | O_NONBLOCK) is probably a mistake.
> 

This is sneaky. err is -ENODEV and would change to -1 on error, losing
some meaning.

> > +if (ret == -1)
> > +goto error;
> > +fp = fdopen(fd, "r");
> > +if (!fp)
> > +goto error;
> > +fd = -1;
> > +/* Only take the last line into account */
> > +ret = 0;
> > +while (fgets(output, sizeof(output), fp))
> > +++ret;
> 
>lcount = 0;
>while (fgets(output, sizeof(output), fp))
>++lcount;
> 
> 
> > +if (feof(fp)) {
> > +if (!ret)
> > +goto error;
> > +} else if (ferror(fp)) {
> > +if (errno != EAGAIN || !ret)
> > +goto error;
> > +} else if (!ret) {
> > +goto error;
> > +}
> 
> These branches seems needlessly complicated:
> 
> if (lcount == 0)
> goto error;
> else if (ferror(fp) && errno != EAGAIN)
> goto error;
> 

Here err would have been set to 0 previously with the fcntl call,
meaning that jumping to error would return 0 as well.

I know Adrien wanted to avoid the usual ugly

if (error) {
err = -ENODEV;
goto error;
}

But this kind of sneakiness is not easy to parse and maintain. If
someone adds a new path of error later, this kind of subtlety *will* be
lost.

So between ugliness and maintainability, I choose maintainability (being
the maintainer, of course).

-- 
Gaëtan Rivet
6WIND

Re: [dpdk-dev] [PATCH] vhost: support UDP Fragmentation Offload

[Thomas Monjalon]

08/01/2018 15:27, Yuanhan Liu:
> On Tue, Nov 21, 2017 at 02:56:52PM +0800, Jiayu Hu wrote:
> > In virtio, UDP Fragmentation Offload (UFO) includes two parts: host UFO
> > and guest UFO. Guest UFO means the frontend can receive large UDP packets,
> > and host UFO means the backend can receive large UDP packets. This patch
> > supports host UFO and guest UFO for vhost-user.
> > 
> > Signed-off-by: Jiayu Hu 
> 
> Applied to dpdk-next-virtio.

Olivier Matz, mbuf maintainer, was not Cc'ed in this patch.
Olivier, can you confirm this new mbuf flag is OK?

Re: [dpdk-dev] [PATCH] doc: remove UTF-8 BOM from programmer's guide

[Burakov, Anatoly]

On 16-Jan-18 11:17 AM, Burakov, Anatoly wrote:

On 21-Dec-17 3:28 PM, Anatoly Burakov wrote:

Fixes: 55694b2a9f64 ("doc: add membership documentation")
Cc: yipeng1.w...@intel.com
Signed-off-by: Anatoly Burakov 
---


Ping?


Oh, mu apologies, didn't notice it was already acked.

--
Thanks,
Anatoly

Re: [dpdk-dev] [PATCH] doc: add queue region feature info to release notes

[Thomas Monjalon]

21/12/2017 19:10, Ferruh Yigit:
> On 12/20/2017 7:52 PM, Wei Zhao wrote:
> > This patch add inforation about i40e queue region
> > realted to release notes, it has been missed before.
> > 
> > Signed-off-by: Wei Zhao 
> > ---
> >  doc/guides/rel_notes/release_17_11.rst | 17 +
> 
> I think we shouldn't update release notes once it has been released.
> 
> Perhaps it can be an option to mention from this in latest release notes with 
> a
> note that says actual support added in v17.11?

I disagree.
It is really confusing to add 17.11 feature in 18.02 release notes.
It is better to add it in 17.11 release notes and backport it.

Please Ferruh, could you remove the patch v2 updating release_18_02.rst
from next-net?

Re: [dpdk-dev] [PATCH v2] net/tap: allow user mac to be passed as args

[Ferruh Yigit]

On 12/21/2017 4:01 PM, Vipin Varghese wrote:
> Added fixes for user TAP user MAC
> 1) user format to RTE_PMD_REGISTER_PARAM_STRING
> 2) TAP to the RTE_LOG in absence of dynamic RTE_LOG
> 3) Boundary case for MAC string added
> > ---
> 
> Other Details:
> 1) not to extract "string to mac" conversion to its own function
> 2) set_mac_type does not take any pmd or device name
> 3) Segault for boundary cases 'mac="01:01:01:01:01:01', not found
> 4) Added user MAC format string

Hi Vipin,

Patch version changes shouldn't be part of git commit. But it is good to put
them after "---" in commit log. Can you please update the commit log?

Also please add maintainer to the cc/to .

> 
> Signed-off-by: Vipin Varghese 

<...>

> - strncpy(macTemp, value, 18);
> -
> - macByte = strtok(macTemp, ":");
> - while ((macByte != NULL) &&
> - (strspn(macByte, 
> "0123456789ABCDEFabcdef")) &&
> - (strspn((macByte + 1), 
> "0123456789ABCDEFabcdef")) &&
> - (strlen(macByte) == 2)) {
> - user_mac[index] = strtoul(macByte, 
> NULL, 16);
> - macByte = strtok(NULL, ":");
> + strncpy(mac_temp, value, 18);
> + mac_temp[19] = "\0";

This cause a build error [1], should be used '\0':

[1]
...drivers/net/tap/rte_eth_tap.c: In function ‘set_mac_type’:
...drivers/net/tap/rte_eth_tap.c:1467:18: error: assignment makes integer from
pointer without a cast [-Werror=int-conversion]
 mac_temp[19] = "\0";
  ^
<>

> -   ETH_TAP_IFACE_ARG "= "
> -   ETH_TAP_SPEED_ARG "= "
> -   ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_FIXED " "
> -   ETH_TAP_REMOTE_ARG "=");
> + ETH_TAP_IFACE_ARG "= "
> + ETH_TAP_SPEED_ARG "= "
> + ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_FIXED "|" ETH_TAP_USER_MAC_FMT " "
> + ETH_TAP_REMOTE_ARG "=");

This also build a build error [2] because of how ETH_TAP_USER_MAC_FMT defined 
[3].

[2]
.../drivers/net/tap/rte_eth_tap.c: At top level:
.../drivers/net/tap/rte_eth_tap.c:45:33: error: called object is not a function
or function pointer
 #define ETH_TAP_IFACE_ARG   "iface"
 ^
.../build/include/rte_dev.h:265:25: note: in definition of macro
‘RTE_PMD_REGISTER_PARAM_STRING’
 __attribute__((used)) = str
 ^~~
.../drivers/net/tap/rte_eth_tap.c:1628:2: note: in expansion of macro
‘ETH_TAP_IFACE_ARG’
  ETH_TAP_IFACE_ARG "= "
  ^
.../drivers/net/tap/rte_eth_tap.c:1630:65: error: expected ‘,’ or ‘;’ before
string constant
  ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_FIXED "|" ETH_TAP_USER_MAC_FMT " "
 ^
.../build/include/rte_dev.h:265:25: note: in definition of macro
‘RTE_PMD_REGISTER_PARAM_STRING’
 __attribute__((used)) = str
 ^~~

[3]
#define ETH_TAP_USER_MAC_FMT ("xx:xx:xx:xx:xx:xx")
parenthesis ...

Re: [dpdk-dev] [PATCH v2 15/15] doc: update example eventdev pipeline

[Kovacevic, Marko]

> Signed-off-by: Pavan Nikhilesh 
> Acked-by: Kevin Laatz 

>  .../{eventdev_pipeline_sw_pmd.rst => eventdev_pipeline.rst} | 6 
> +++---
>  doc/guides/sample_app_ug/index.rst  | 2 +-
>  2 files changed, 4 insertions(+), 4 deletions(-)  rename
> doc/guides/sample_app_ug/{eventdev_pipeline_sw_pmd.rst =>
> eventdev_pipeline.rst} (97%)

Acked-by: Marko Kovacevic 

Re: [dpdk-dev] [PATCH 1/2] lib/cryptodev: add support to set session private data

[Gujjar, Abhinandan S]

Hi Akhil,

> -Original Message-
> From: Akhil Goyal [mailto:akhil.go...@nxp.com]
> Sent: Tuesday, January 16, 2018 2:52 PM
> To: Gujjar, Abhinandan S ; Doherty, Declan
> ; Jacob, Jerin
> 
> Cc: dev@dpdk.org; Vangati, Narender ; Rao,
> Nikhil 
> Subject: Re: [PATCH 1/2] lib/cryptodev: add support to set session private 
> data
> 
> On 1/16/2018 2:33 PM, Gujjar, Abhinandan S wrote:
> > Hi Akhil,
> >
> >> -Original Message-
> >> From: Akhil Goyal [mailto:akhil.go...@nxp.com]
> >> Sent: Tuesday, January 16, 2018 12:56 PM
> >> To: Gujjar, Abhinandan S ; Doherty,
> >> Declan ; Jacob, Jerin
> >> 
> >> Cc: dev@dpdk.org; Vangati, Narender ;
> >> Rao, Nikhil 
> >> Subject: Re: [PATCH 1/2] lib/cryptodev: add support to set session
> >> private data
> >>
> >> Hi Abhinandan,
> >> On 1/16/2018 12:35 PM, Gujjar, Abhinandan S wrote:
> >>> Hi Akhil,
> >>>
>  -Original Message-
>  From: Akhil Goyal [mailto:akhil.go...@nxp.com]
>  Sent: Tuesday, January 16, 2018 11:55 AM
>  To: Gujjar, Abhinandan S ; Doherty,
>  Declan 
>  Cc: dev@dpdk.org; Vangati, Narender ;
>  Rao, Nikhil 
>  Subject: Re: [PATCH 1/2] lib/cryptodev: add support to set session
>  private data
> 
>  Hi Abhinandan,
>  On 1/16/2018 11:39 AM, Gujjar, Abhinandan S wrote:
> >>> diff --git a/lib/librte_cryptodev/rte_crypto.h
> >>> b/lib/librte_cryptodev/rte_crypto.h
> >>> index bbc510d..3a98cbf 100644
> >>> --- a/lib/librte_cryptodev/rte_crypto.h
> >>> +++ b/lib/librte_cryptodev/rte_crypto.h
> >>> @@ -62,6 +62,18 @@ enum rte_crypto_op_sess_type {
> >>>   RTE_CRYPTO_OP_SECURITY_SESSION  /**< Security session
> >> crypto
> >> operation */
> >>>  };
> >>>
> >>> +/** Private data types for cryptographic operation
> >>> + * @see rte_crypto_op::private_data_type */ enum
> >>> +rte_crypto_op_private_data_type {
> >>> + RTE_CRYPTO_OP_PRIVATE_DATA_NONE,
> >>> + /**< No private data */
> >>> + RTE_CRYPTO_OP_PRIVATE_DATA_OP,
> >>> + /**< Private data is part of rte_crypto_op and indicated by
> >>> +  * private_data_offset */
> >>> + RTE_CRYPTO_OP_PRIVATE_DATA_SESSION
> >>> + /**< Private data is available at session */ };
> >>> +
> >> We may get away with this enum. If private_data_offset is "0",
> >> then we can check with the session if it has priv data or not.
> > Right now,  Application uses 'rte_crypto_op_private_data_type' to
> > indicate rte_cryptodev_sym_session_set_private_data()
> > was called to set the private data. Otherwise, how do you indicate
> > there is a
>  private data associated with the session?
> > Any suggestions?
>  For session based flows, the first choice to store the private data
>  should be in the session. So RTE_CRYPTO_OP_WITH_SESSION or
>  RTE_CRYPTO_OP_SECURITY_SESSION can be used to call
>  rte_cryptodev_sym_session_set_private_data or
>  rte_security_session_set_private_data.
> >>> Case 1: private_data_offset is "0" and sess_type =
> >>> RTE_CRYPTO_OP_WITH_SESSION -> usual case Case 2: private_data_offset
> >>> is "0" and sess_type = RTE_CRYPTO_OP_WITH_SESSION + event case
> >>> (access private data) Differentiating between case 1 & 2 will be
> >>> done by checking
> >> rte_crypto_op_private_data_type ==
> >> RTE_CRYPTO_OP_PRIVATE_DATA_SESSION.
> >>
> >> Consider this:
> >> if (sess_type == RTE_CRYPTO_OP_WITH_SESSION &&
> >>rte_cryptodev_sym_session_get_private_data == NULL)
> >>usual case.
> >> else if (sess_type = RTE_CRYPTO_OP_WITH_SESSION &&
> >>rte_cryptodev_sym_session_get_private_data != NULL)
> >>event case.
> >> else if (sess_type == RTE_CRYPTO_OP_SESSIONLESS &&
> >>private_data_offset != 0)
> >>event case for sessionless op.
> >>
> >> I hope all cases can be handled in this way.
> > Memory allocated for private data will be continuation of session memory.
> > I think, rte_cryptodev_sym_session_get_private_data() will return a valid
> pointer.
> > It could be pointer to private data, in case application has allocated 
> > mempool
> with session + private data.
> > It could be again a pointer to a location(may be next session),  in case
> application has allocated mempool with session only.
> > Unless, there is a flag in the session data which will be set by
> > rte_cryptodev_sym_session_set_private_data()
> > If this flag is not set, rte_cryptodev_sym_session_get_private_data() will
> return NULL.
> > I am not claiming, I have complete knowledge of different usage case of
> mempool setup for crypto.
> > I am wondering, whether I am missing anything here. Please let me know.
> 
> It depends on the implementation of the get/set API. We can set NULL, if it is
> not filled in the set API. If it is set then we have a valid pointer.
The plan is to store private data after "sess * nb_drivers ".
As you said, if it is implementation specific, flag may be

Re: [dpdk-dev] [PATCH v10 3/5] net: add a helper for making RARP packet

[Wang, Xiao W]

> -Original Message-
> From: Wang, Xiao W
> Sent: Tuesday, January 16, 2018 7:03 PM
> To: 'Olivier Matz' 
> Cc: y...@fridaylinux.org; tho...@monjalon.net; Bie, Tiwei
> ; dev@dpdk.org; step...@networkplumber.org;
> maxime.coque...@redhat.com
> Subject: RE: [dpdk-dev] [PATCH v10 3/5] net: add a helper for making RARP
> packet
> 
> Hi Olivier,
> 
> > -Original Message-
> > From: Olivier Matz [mailto:olivier.m...@6wind.com]
> > Sent: Tuesday, January 16, 2018 6:43 PM
> > To: Wang, Xiao W 
> > Cc: y...@fridaylinux.org; tho...@monjalon.net; Bie, Tiwei
> > ; dev@dpdk.org; step...@networkplumber.org;
> > maxime.coque...@redhat.com
> > Subject: Re: [dpdk-dev] [PATCH v10 3/5] net: add a helper for making RARP
> > packet
> >
> > Hi Xiao,
> >
> > On Tue, Jan 16, 2018 at 09:43:43AM +, Wang, Xiao W wrote:
> > > Hi Olivier,
> > > > You can also use rte_pktmbuf_append() to check for the tailroom and
> > > > update data_len/pkt_len:
> > > >
> > > > m = rte_pktmbuf_alloc();
> 
> I just realized that if we let this function to allocate mbuf, it may 
> restrict this
> api's applicability.
> E.g. the caller just has a mbuf, without a mempool.
> How do you think?
> 
> > > > if (m == NULL)
> > > > return NULL;
> > > > eth_hdr = rte_pktmbuf_append(m, RARP_PKT_SIZE);
> > >
> > > When data_len is not enough, we need to rte_pktmbuf_append(m,
> > RARP_PKT_SIZE - m->data_len);
> >
> > Sorry, I don't get your point here.
> 
> I mean we just need to extend the data_len by "RARP_PKT_SIZE - m-
> >data_len" when the room is not big enough.

OK, in your sample code, you rte_pktmbuf_alloc() a mbuf, it's reset already, so 
we just append RARP_PKT_SIZE. I got you~

For the mbuf allocation, we can let this function do allocation and content 
filling. If the app needs special need, e.g. chained mbuf,
then let the app fill it by itself.

> 
> BRs,
> Xiao

[dpdk-dev] [PATCH 00/12 v3] event/dpaa: Support for eventdev

[Nipun Gupta]

Event device support for atomic and parallel queues.

These patches are based on dpdk-event-next (commit ID fb79a5525fb9)
rebased on top of dpdk-net-next, as there is dependency on both the
trees for this patchset.
on dpdk-event-next this series depends on applied patch:
https://dpdk.org/dev/patchwork/patch/33485/
on dpdk-net-next this series depend on DPAA net patches:
https://dpdk.org/dev/patchwork/patch/33444/

This patch set includes following changes:
  1. Configuration of atomic and parallel queues with given event device.
  2. Also maintains previous dequeue method, via poll mode queues.
  3. Added Rx functions to dequeue data from portal.
  4. DCA consumption logic for atomic queues.
  5. Dynamic Logging macros for event device

Changes in v2:
  More logial splitting of the patches

Changes in v3:
  Added new capabilities with DPAA PMD
  Removed CONFIG_RTE_LIBRTE_PMD_DPAA_EVENTDEV_DEBUG flag from doc.
  Rebased on dpdk-event-next (which is locally rebased over
dpdk-net-next as there are dependencies on both the trees).
  Added a warning print when eventdev for DPAA is configured with
push mode already enabled in DPAA PMD.

Sunil Kumar Kori (12):
  config: enabling compilation of DPAA eventdev PMD
  bus/dpaa: add event dequeue and consumption support
  bus/dpaa: add dpaa eventdev dynamic log support
  net/dpaa: ethdev Rx queue configurations with eventdev
  event/dpaa: add eventdev PMD
  event/dpaa: add event queue config get/set support
  event/dpaa: add event port config get/set support
  event/dpaa: add dequeue timeout conversion support
  event/dpaa: add eth rx adapter queue config support
  event/dpaa: add eventdev enqueue/dequeue support
  config: add eventdev library to application
  doc: add DPAA eventdev guide

 MAINTAINERS   |   6 +
 config/common_base|   3 +
 config/defconfig_arm64-dpaa-linuxapp-gcc  |   3 +
 doc/guides/eventdevs/dpaa.rst | 140 +
 doc/guides/eventdevs/index.rst|   1 +
 drivers/bus/dpaa/base/qbman/qman.c|  91 ++-
 drivers/bus/dpaa/dpaa_bus.c   |   6 +
 drivers/bus/dpaa/include/fsl_qman.h   |  26 +-
 drivers/bus/dpaa/rte_bus_dpaa_version.map |   6 +
 drivers/bus/dpaa/rte_dpaa_bus.h   |  14 +
 drivers/bus/dpaa/rte_dpaa_logs.h  |  16 +
 drivers/event/Makefile|   1 +
 drivers/event/dpaa/Makefile   |  37 ++
 drivers/event/dpaa/dpaa_eventdev.c| 653 ++
 drivers/event/dpaa/dpaa_eventdev.h|  81 +++
 drivers/event/dpaa/rte_pmd_dpaa_event_version.map |   4 +
 drivers/net/dpaa/Makefile |   2 +
 drivers/net/dpaa/dpaa_ethdev.c| 115 +++-
 drivers/net/dpaa/dpaa_ethdev.h|  29 +
 drivers/net/dpaa/dpaa_rxtx.c  |  79 ++-
 drivers/net/dpaa/rte_pmd_dpaa_version.map |   2 +
 mk/rte.app.mk |   1 +
 22 files changed, 1299 insertions(+), 17 deletions(-)
 create mode 100644 doc/guides/eventdevs/dpaa.rst
 create mode 100644 drivers/event/dpaa/Makefile
 create mode 100644 drivers/event/dpaa/dpaa_eventdev.c
 create mode 100644 drivers/event/dpaa/dpaa_eventdev.h
 create mode 100644 drivers/event/dpaa/rte_pmd_dpaa_event_version.map

-- 
1.9.1

[dpdk-dev] [PATCH 01/12 v3] config: enabling compilation of DPAA eventdev PMD

[Nipun Gupta]

From: Sunil Kumar Kori 

Signed-off-by: Sunil Kumar Kori 
Acked-by: Hemant Agrawal 
---
 config/common_base   | 3 +++
 config/defconfig_arm64-dpaa-linuxapp-gcc | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/config/common_base b/config/common_base
index 3ddb6f0..9603723 100644
--- a/config/common_base
+++ b/config/common_base
@@ -325,6 +325,9 @@ CONFIG_RTE_LIBRTE_DPAA_BUS=n
 CONFIG_RTE_LIBRTE_DPAA_MEMPOOL=n
 CONFIG_RTE_LIBRTE_DPAA_PMD=n
 
+# Compile software NXP DPAA Event Dev PMD
+CONFIG_RTE_LIBRTE_PMD_DPAA_EVENTDEV=n
+
 #
 # Compile burst-oriented Cavium OCTEONTX network PMD driver
 #
diff --git a/config/defconfig_arm64-dpaa-linuxapp-gcc 
b/config/defconfig_arm64-dpaa-linuxapp-gcc
index 5f882ca..c2ca16a 100644
--- a/config/defconfig_arm64-dpaa-linuxapp-gcc
+++ b/config/defconfig_arm64-dpaa-linuxapp-gcc
@@ -30,6 +30,9 @@ CONFIG_RTE_MBUF_DEFAULT_MEMPOOL_OPS="dpaa"
 # Compile software NXP DPAA PMD
 CONFIG_RTE_LIBRTE_DPAA_PMD=y
 
+# Compile software NXP DPAA Event Dev PMD
+CONFIG_RTE_LIBRTE_PMD_DPAA_EVENTDEV=y
+
 #
 # FSL DPAA caam - crypto driver
 #
-- 
1.9.1

[dpdk-dev] [PATCH 02/12 v3] bus/dpaa: add event dequeue and consumption support

[Nipun Gupta]

From: Sunil Kumar Kori 

To receive events from given event port, corresponding
function needs to be added which receives events
from portal. Also added function to consume received
events based on entry index.

Signed-off-by: Sunil Kumar Kori 
Acked-by: Hemant Agrawal 
---
 drivers/bus/dpaa/base/qbman/qman.c| 91 +--
 drivers/bus/dpaa/dpaa_bus.c   |  1 +
 drivers/bus/dpaa/include/fsl_qman.h   | 26 +++--
 drivers/bus/dpaa/rte_bus_dpaa_version.map |  5 ++
 drivers/bus/dpaa/rte_dpaa_bus.h   | 14 +
 drivers/net/dpaa/dpaa_rxtx.c  |  1 +
 6 files changed, 129 insertions(+), 9 deletions(-)

diff --git a/drivers/bus/dpaa/base/qbman/qman.c 
b/drivers/bus/dpaa/base/qbman/qman.c
index e171356..609bc76 100644
--- a/drivers/bus/dpaa/base/qbman/qman.c
+++ b/drivers/bus/dpaa/base/qbman/qman.c
@@ -8,6 +8,8 @@
 #include "qman.h"
 #include 
 #include 
+#include 
+#include 
 
 /* Compilation constants */
 #define DQRR_MAXFILL   15
@@ -1115,6 +1117,74 @@ unsigned int qman_portal_poll_rx(unsigned int poll_limit,
return limit;
 }
 
+u32 qman_portal_dequeue(struct rte_event ev[], unsigned int poll_limit,
+   void **bufs)
+{
+   const struct qm_dqrr_entry *dq;
+   struct qman_fq *fq;
+   enum qman_cb_dqrr_result res;
+   unsigned int limit = 0;
+   struct qman_portal *p = get_affine_portal();
+#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
+   struct qm_dqrr_entry *shadow;
+#endif
+   unsigned int rx_number = 0;
+
+   do {
+   qm_dqrr_pvb_update(&p->p);
+   dq = qm_dqrr_current(&p->p);
+   if (!dq)
+   break;
+#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
+   /*
+* If running on an LE system the fields of the
+* dequeue entry must be swapper.  Because the
+* QMan HW will ignore writes the DQRR entry is
+* copied and the index stored within the copy
+*/
+   shadow = &p->shadow_dqrr[DQRR_PTR2IDX(dq)];
+   *shadow = *dq;
+   dq = shadow;
+   shadow->fqid = be32_to_cpu(shadow->fqid);
+   shadow->contextB = be32_to_cpu(shadow->contextB);
+   shadow->seqnum = be16_to_cpu(shadow->seqnum);
+   hw_fd_to_cpu(&shadow->fd);
+#endif
+
+  /* SDQCR: context_b points to the FQ */
+#ifdef CONFIG_FSL_QMAN_FQ_LOOKUP
+   fq = get_fq_table_entry(dq->contextB);
+#else
+   fq = (void *)(uintptr_t)dq->contextB;
+#endif
+   /* Now let the callback do its stuff */
+   res = fq->cb.dqrr_dpdk_cb(&ev[rx_number], p, fq,
+dq, &bufs[rx_number]);
+   rx_number++;
+   /* Interpret 'dq' from a driver perspective. */
+   /*
+* Parking isn't possible unless HELDACTIVE was set. NB,
+* FORCEELIGIBLE implies HELDACTIVE, so we only need to
+* check for HELDACTIVE to cover both.
+*/
+   DPAA_ASSERT((dq->stat & QM_DQRR_STAT_FQ_HELDACTIVE) ||
+   (res != qman_cb_dqrr_park));
+   if (res != qman_cb_dqrr_defer)
+   qm_dqrr_cdc_consume_1ptr(&p->p, dq,
+res == qman_cb_dqrr_park);
+   /* Move forward */
+   qm_dqrr_next(&p->p);
+   /*
+* Entry processed and consumed, increment our counter.  The
+* callback can request that we exit after consuming the
+* entry, and we also exit if we reach our processing limit,
+* so loop back only if neither of these conditions is met.
+*/
+   } while (++limit < poll_limit);
+
+   return limit;
+}
+
 struct qm_dqrr_entry *qman_dequeue(struct qman_fq *fq)
 {
struct qman_portal *p = get_affine_portal();
@@ -1233,13 +1303,20 @@ u32 qman_static_dequeue_get(struct qman_portal *qp)
return p->sdqcr;
 }
 
-void qman_dca(struct qm_dqrr_entry *dq, int park_request)
+void qman_dca(const struct qm_dqrr_entry *dq, int park_request)
 {
struct qman_portal *p = get_affine_portal();
 
qm_dqrr_cdc_consume_1ptr(&p->p, dq, park_request);
 }
 
+void qman_dca_index(u8 index, int park_request)
+{
+   struct qman_portal *p = get_affine_portal();
+
+   qm_dqrr_cdc_consume_1(&p->p, index, park_request);
+}
+
 /* Frame queue API */
 static const char *mcr_result_str(u8 result)
 {
@@ -2088,8 +2165,8 @@ int qman_enqueue(struct qman_fq *fq, const struct qm_fd 
*fd, u32 flags)
 }
 
 int qman_enqueue_multi(struct qman_fq *fq,
-  const struct qm_fd *fd,
-  int frames_to_send)
+  const struct qm_fd *fd, u32 *flags,
+   int frames_to_send)
 {
struct qman_portal *p = get_affine_

[dpdk-dev] [PATCH 03/12 v3] bus/dpaa: add dpaa eventdev dynamic log support

[Nipun Gupta]

From: Sunil Kumar Kori 

Signed-off-by: Sunil Kumar Kori 
Acked-by: Hemant Agrawal 
---
 drivers/bus/dpaa/dpaa_bus.c   |  5 +
 drivers/bus/dpaa/rte_bus_dpaa_version.map |  1 +
 drivers/bus/dpaa/rte_dpaa_logs.h  | 16 
 3 files changed, 22 insertions(+)

diff --git a/drivers/bus/dpaa/dpaa_bus.c b/drivers/bus/dpaa/dpaa_bus.c
index 78d60be..4c2854f 100644
--- a/drivers/bus/dpaa/dpaa_bus.c
+++ b/drivers/bus/dpaa/dpaa_bus.c
@@ -44,6 +44,7 @@
 int dpaa_logtype_bus;
 int dpaa_logtype_mempool;
 int dpaa_logtype_pmd;
+int dpaa_logtype_eventdev;
 
 struct rte_dpaa_bus rte_dpaa_bus;
 struct netcfg_info *dpaa_netcfg;
@@ -536,4 +537,8 @@ struct rte_dpaa_bus rte_dpaa_bus = {
dpaa_logtype_pmd = rte_log_register("pmd.dpaa");
if (dpaa_logtype_pmd >= 0)
rte_log_set_level(dpaa_logtype_pmd, RTE_LOG_NOTICE);
+
+   dpaa_logtype_eventdev = rte_log_register("eventdev.dpaa");
+   if (dpaa_logtype_eventdev >= 0)
+   rte_log_set_level(dpaa_logtype_eventdev, RTE_LOG_NOTICE);
 }
diff --git a/drivers/bus/dpaa/rte_bus_dpaa_version.map 
b/drivers/bus/dpaa/rte_bus_dpaa_version.map
index f5c291f..925cf91 100644
--- a/drivers/bus/dpaa/rte_bus_dpaa_version.map
+++ b/drivers/bus/dpaa/rte_bus_dpaa_version.map
@@ -68,6 +68,7 @@ DPDK_17.11 {
 DPDK_18.02 {
global:
 
+   dpaa_logtype_eventdev;
dpaa_svr_family;
per_lcore_held_bufs;
qm_channel_pool1;
diff --git a/drivers/bus/dpaa/rte_dpaa_logs.h b/drivers/bus/dpaa/rte_dpaa_logs.h
index 5429864..0fd70bb 100644
--- a/drivers/bus/dpaa/rte_dpaa_logs.h
+++ b/drivers/bus/dpaa/rte_dpaa_logs.h
@@ -12,6 +12,7 @@
 extern int dpaa_logtype_bus;
 extern int dpaa_logtype_mempool;
 extern int dpaa_logtype_pmd;
+extern int dpaa_logtype_eventdev;
 
 #define DPAA_BUS_LOG(level, fmt, args...) \
rte_log(RTE_LOG_ ## level, dpaa_logtype_bus, "%s(): " fmt "\n", \
@@ -74,6 +75,21 @@
 #define DPAA_PMD_WARN(fmt, args...) \
DPAA_PMD_LOG(WARNING, fmt, ## args)
 
+#define DPAA_EVENTDEV_LOG(level, fmt, args...) \
+   rte_log(RTE_LOG_ ## level, dpaa_logtype_eventdev, "%s(): " fmt "\n", \
+   __func__, ##args)
+
+#define EVENTDEV_INIT_FUNC_TRACE() DPAA_EVENTDEV_LOG(DEBUG, " >>")
+
+#define DPAA_EVENTDEV_DEBUG(fmt, args...) \
+   DPAA_EVENTDEV_LOG(DEBUG, fmt, ## args)
+#define DPAA_EVENTDEV_ERR(fmt, args...) \
+   DPAA_EVENTDEV_LOG(ERR, fmt, ## args)
+#define DPAA_EVENTDEV_INFO(fmt, args...) \
+   DPAA_EVENTDEV_LOG(INFO, fmt, ## args)
+#define DPAA_EVENTDEV_WARN(fmt, args...) \
+   DPAA_EVENTDEV_LOG(WARNING, fmt, ## args)
+
 /* DP Logs, toggled out at compile time if level lower than current level */
 #define DPAA_DP_LOG(level, fmt, args...) \
RTE_LOG_DP(level, PMD, fmt, ## args)
-- 
1.9.1

[dpdk-dev] [PATCH 04/12 v3] net/dpaa: ethdev Rx queue configurations with eventdev

[Nipun Gupta]

From: Sunil Kumar Kori 

Given ethernet Rx queues can be attached with event queue in
parallel or atomic mode. Patch imlmplements Rx queue
configuration, attachment/detachment with given event queue and their
corresponding callbacks to handle events from respective queues.

Signed-off-by: Sunil Kumar Kori 
Signed-off-by: Nipun Gupta 
Acked-by: Hemant Agrawal 
---
 drivers/net/dpaa/Makefile |   2 +
 drivers/net/dpaa/dpaa_ethdev.c| 115 --
 drivers/net/dpaa/dpaa_ethdev.h|  29 
 drivers/net/dpaa/dpaa_rxtx.c  |  80 -
 drivers/net/dpaa/rte_pmd_dpaa_version.map |   2 +
 5 files changed, 219 insertions(+), 9 deletions(-)

diff --git a/drivers/net/dpaa/Makefile b/drivers/net/dpaa/Makefile
index b1fc5a0..9c2a5ea 100644
--- a/drivers/net/dpaa/Makefile
+++ b/drivers/net/dpaa/Makefile
@@ -17,7 +17,9 @@ CFLAGS += -I$(RTE_SDK_DPAA)/
 CFLAGS += -I$(RTE_SDK_DPAA)/include
 CFLAGS += -I$(RTE_SDK)/drivers/bus/dpaa
 CFLAGS += -I$(RTE_SDK)/drivers/bus/dpaa/include/
+CFLAGS += -I$(RTE_SDK)/drivers/bus/dpaa/base/qbman
 CFLAGS += -I$(RTE_SDK)/drivers/mempool/dpaa
+CFLAGS += -I$(RTE_SDK)/drivers/event/dpaa
 CFLAGS += -I$(RTE_SDK)/lib/librte_eal/common/include
 CFLAGS += -I$(RTE_SDK)/lib/librte_eal/linuxapp/eal/include
 
diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c
index 444c122..09f0a73 100644
--- a/drivers/net/dpaa/dpaa_ethdev.c
+++ b/drivers/net/dpaa/dpaa_ethdev.c
@@ -95,6 +95,21 @@ struct rte_dpaa_xstats_name_off {
 
 static struct rte_dpaa_driver rte_dpaa_pmd;
 
+static inline void
+dpaa_poll_queue_default_config(struct qm_mcc_initfq *opts)
+{
+   memset(opts, 0, sizeof(struct qm_mcc_initfq));
+   opts->we_mask = QM_INITFQ_WE_FQCTRL | QM_INITFQ_WE_CONTEXTA;
+   opts->fqd.fq_ctrl = QM_FQCTRL_AVOIDBLOCK | QM_FQCTRL_CTXASTASHING |
+  QM_FQCTRL_PREFERINCACHE;
+   opts->fqd.context_a.stashing.exclusive = 0;
+   if (dpaa_svr_family != SVR_LS1046A_FAMILY)
+   opts->fqd.context_a.stashing.annotation_cl =
+   DPAA_IF_RX_ANNOTATION_STASH;
+   opts->fqd.context_a.stashing.data_cl = DPAA_IF_RX_DATA_STASH;
+   opts->fqd.context_a.stashing.context_cl = DPAA_IF_RX_CONTEXT_STASH;
+}
+
 static int
 dpaa_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 {
@@ -533,6 +548,97 @@ int dpaa_eth_rx_queue_setup(struct rte_eth_dev *dev, 
uint16_t queue_idx,
return 0;
 }
 
+int dpaa_eth_eventq_attach(const struct rte_eth_dev *dev,
+  int eth_rx_queue_id,
+   u16 ch_id,
+   const struct rte_event_eth_rx_adapter_queue_conf *queue_conf)
+{
+   int ret;
+   u32 flags = 0;
+   struct dpaa_if *dpaa_intf = dev->data->dev_private;
+   struct qman_fq *rxq = &dpaa_intf->rx_queues[eth_rx_queue_id];
+   struct qm_mcc_initfq opts = {0};
+
+   if (dpaa_push_mode_max_queue)
+   DPAA_PMD_WARN("PUSH mode already enabled for first %d queues.\n"
+ "To disable set DPAA_PUSH_QUEUES_NUMBER to 0\n",
+ dpaa_push_mode_max_queue);
+
+   dpaa_poll_queue_default_config(&opts);
+
+   switch (queue_conf->ev.sched_type) {
+   case RTE_SCHED_TYPE_ATOMIC:
+   opts.fqd.fq_ctrl |= QM_FQCTRL_HOLDACTIVE;
+   /* Reset FQCTRL_AVOIDBLOCK bit as it is unnecessary
+* configuration with HOLD_ACTIVE setting
+*/
+   opts.fqd.fq_ctrl &= (~QM_FQCTRL_AVOIDBLOCK);
+   rxq->cb.dqrr_dpdk_cb = dpaa_rx_cb_atomic;
+   break;
+   case RTE_SCHED_TYPE_ORDERED:
+   DPAA_PMD_ERR("Ordered queue schedule type is not supported\n");
+   return -1;
+   default:
+   opts.fqd.fq_ctrl |= QM_FQCTRL_AVOIDBLOCK;
+   rxq->cb.dqrr_dpdk_cb = dpaa_rx_cb_parallel;
+   break;
+   }
+
+   opts.we_mask = opts.we_mask | QM_INITFQ_WE_DESTWQ;
+   opts.fqd.dest.channel = ch_id;
+   opts.fqd.dest.wq = queue_conf->ev.priority;
+
+   if (dpaa_intf->cgr_rx) {
+   opts.we_mask |= QM_INITFQ_WE_CGID;
+   opts.fqd.cgid = dpaa_intf->cgr_rx[eth_rx_queue_id].cgrid;
+   opts.fqd.fq_ctrl |= QM_FQCTRL_CGE;
+   }
+
+   flags = QMAN_INITFQ_FLAG_SCHED;
+
+   ret = qman_init_fq(rxq, flags, &opts);
+   if (ret) {
+   DPAA_PMD_ERR("Channel/Queue association failed. fqid %d ret:%d",
+rxq->fqid, ret);
+   return ret;
+   }
+
+   /* copy configuration which needs to be filled during dequeue */
+   memcpy(&rxq->ev, &queue_conf->ev, sizeof(struct rte_event));
+   dev->data->rx_queues[eth_rx_queue_id] = rxq;
+
+   return ret;
+}
+
+int dpaa_eth_eventq_detach(const struct rte_eth_dev *dev,
+  int eth_rx_queue_id)
+{
+   struct qm_mcc_init

[dpdk-dev] [PATCH 05/12 v3] event/dpaa: add eventdev PMD

[Nipun Gupta]

From: Sunil Kumar Kori 

Signed-off-by: Sunil Kumar Kori 
Acked-by: Hemant Agrawal 
---
 MAINTAINERS   |   5 +
 drivers/event/Makefile|   1 +
 drivers/event/dpaa/Makefile   |  37 +++
 drivers/event/dpaa/dpaa_eventdev.c| 269 ++
 drivers/event/dpaa/dpaa_eventdev.h|  81 +++
 drivers/event/dpaa/rte_pmd_dpaa_event_version.map |   4 +
 6 files changed, 397 insertions(+)
 create mode 100644 drivers/event/dpaa/Makefile
 create mode 100644 drivers/event/dpaa/dpaa_eventdev.c
 create mode 100644 drivers/event/dpaa/dpaa_eventdev.h
 create mode 100644 drivers/event/dpaa/rte_pmd_dpaa_event_version.map

diff --git a/MAINTAINERS b/MAINTAINERS
index dbfd3f7..fff842e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -704,6 +704,11 @@ M: Nipun Gupta 
 F: drivers/event/dpaa2/
 F: doc/guides/eventdevs/dpaa2.rst
 
+NXP DPAA eventdev
+M: Hemant Agrawal 
+M: Sunil Kumar Kori 
+F: drivers/event/dpaa/
+
 Software Eventdev PMD
 M: Harry van Haaren 
 F: drivers/event/sw/
diff --git a/drivers/event/Makefile b/drivers/event/Makefile
index 7f68440..0cbf2e4 100644
--- a/drivers/event/Makefile
+++ b/drivers/event/Makefile
@@ -9,5 +9,6 @@ DIRS-$(CONFIG_RTE_LIBRTE_PMD_SW_EVENTDEV) += sw
 DIRS-$(CONFIG_RTE_LIBRTE_PMD_OCTEONTX_SSOVF) += octeontx
 DIRS-$(CONFIG_RTE_LIBRTE_PMD_DPAA2_EVENTDEV) += dpaa2
 DIRS-$(CONFIG_RTE_LIBRTE_PMD_OPDL_EVENTDEV) += opdl
+DIRS-$(CONFIG_RTE_LIBRTE_PMD_DPAA_EVENTDEV) += dpaa
 
 include $(RTE_SDK)/mk/rte.subdir.mk
diff --git a/drivers/event/dpaa/Makefile b/drivers/event/dpaa/Makefile
new file mode 100644
index 000..bd0b6c9
--- /dev/null
+++ b/drivers/event/dpaa/Makefile
@@ -0,0 +1,37 @@
+#   SPDX-License-Identifier:BSD-3-Clause
+#   Copyright 2017 NXP
+#
+
+include $(RTE_SDK)/mk/rte.vars.mk
+RTE_SDK_DPAA=$(RTE_SDK)/drivers/net/dpaa
+
+#
+# library name
+#
+LIB = librte_pmd_dpaa_event.a
+
+CFLAGS := -I$(SRCDIR) $(CFLAGS)
+CFLAGS += -O3 $(WERROR_FLAGS)
+CFLAGS += -Wno-pointer-arith
+CFLAGS += -I$(RTE_SDK_DPAA)/
+CFLAGS += -I$(RTE_SDK_DPAA)/include
+CFLAGS += -I$(RTE_SDK)/drivers/bus/dpaa
+CFLAGS += -I$(RTE_SDK)/drivers/bus/dpaa/include/
+CFLAGS += -I$(RTE_SDK)/drivers/mempool/dpaa
+CFLAGS += -I$(RTE_SDK)/lib/librte_eal/common/include
+CFLAGS += -I$(RTE_SDK)/lib/librte_eal/linuxapp/eal/include
+
+EXPORT_MAP := rte_pmd_dpaa_event_version.map
+
+LIBABIVER := 1
+
+# Interfaces with DPDK
+SRCS-$(CONFIG_RTE_LIBRTE_PMD_DPAA_EVENTDEV) += dpaa_eventdev.c
+
+LDLIBS += -lrte_bus_dpaa
+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
+
+include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/event/dpaa/dpaa_eventdev.c 
b/drivers/event/dpaa/dpaa_eventdev.c
new file mode 100644
index 000..0bd74bb
--- /dev/null
+++ b/drivers/event/dpaa/dpaa_eventdev.c
@@ -0,0 +1,269 @@
+/*   SPDX-License-Identifier:BSD-3-Clause
+ *   Copyright 2017 NXP
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include "dpaa_eventdev.h"
+#include 
+
+/*
+ * Clarifications
+ * Evendev = Virtual Instance for SoC
+ * Eventport = Portal Instance
+ * Eventqueue = Channel Instance
+ * 1 Eventdev can have N Eventqueue
+ */
+
+static int
+dpaa_event_dequeue_timeout_ticks(struct rte_eventdev *dev, uint64_t ns,
+uint64_t *timeout_ticks)
+{
+   uint64_t cycles_per_second;
+
+   EVENTDEV_DRV_FUNC_TRACE();
+
+   RTE_SET_USED(dev);
+
+   cycles_per_second = rte_get_timer_hz();
+   *timeout_ticks = ns * (cycles_per_second / NS_PER_S);
+
+   return 0;
+}
+
+static void
+dpaa_event_dev_info_get(struct rte_eventdev *dev,
+   struct rte_event_dev_info *dev_info)
+{
+   EVENTDEV_DRV_FUNC_TRACE();
+
+   RTE_SET_USED(dev);
+   dev_info->driver_name = "event_dpaa";
+   dev_info->min_dequeue_timeout_ns =
+   DPAA_EVENT_MIN_DEQUEUE_TIMEOUT;
+   dev_info->max_dequeue_timeout_ns =
+   DPAA_EVENT_MAX_DEQUEUE_TIMEOUT;
+   dev_info->dequeue_timeout_ns =
+   DPAA_EVENT_MIN_DEQUEUE_TIMEOUT;
+   dev_info->max_event_queues =
+   DPAA_EVENT_MAX_QUEUES;
+   dev_info->max_event_queue_flows =
+   DPAA_EVENT_MAX_QUEUE_FLOWS;
+   dev_info->max_event_queue_priority_levels =
+   DPAA_EVENT_MAX_QUEUE_PRIORITY_LEVELS;
+   dev_info->max_event_priority_levels =
+   DPAA_EVENT_MAX_EVENT_PRIORITY_LEVELS;
+   dev_info->max_event_ports =
+   DPAA_EVENT_MAX_EVENT_PORT;
+   dev_info->max_event_port_dequeue_depth =
+   DPAA_

[dpdk-dev] [PATCH 07/12 v3] event/dpaa: add event port config get/set support

[Nipun Gupta]

From: Sunil Kumar Kori 

Signed-off-by: Sunil Kumar Kori 
Acked-by: Hemant Agrawal 
---
 drivers/event/dpaa/dpaa_eventdev.c | 105 +
 1 file changed, 105 insertions(+)

diff --git a/drivers/event/dpaa/dpaa_eventdev.c 
b/drivers/event/dpaa/dpaa_eventdev.c
index f1db711..82362d5 100644
--- a/drivers/event/dpaa/dpaa_eventdev.c
+++ b/drivers/event/dpaa/dpaa_eventdev.c
@@ -247,6 +247,106 @@
RTE_SET_USED(queue_id);
 }
 
+static void
+dpaa_event_port_default_conf_get(struct rte_eventdev *dev, uint8_t port_id,
+struct rte_event_port_conf *port_conf)
+{
+   EVENTDEV_DRV_FUNC_TRACE();
+
+   RTE_SET_USED(dev);
+   RTE_SET_USED(port_id);
+
+   port_conf->new_event_threshold = DPAA_EVENT_MAX_NUM_EVENTS;
+   port_conf->dequeue_depth = DPAA_EVENT_MAX_PORT_DEQUEUE_DEPTH;
+   port_conf->enqueue_depth = DPAA_EVENT_MAX_PORT_ENQUEUE_DEPTH;
+}
+
+static int
+dpaa_event_port_setup(struct rte_eventdev *dev, uint8_t port_id,
+ const struct rte_event_port_conf *port_conf)
+{
+   struct dpaa_eventdev *eventdev = dev->data->dev_private;
+
+   EVENTDEV_DRV_FUNC_TRACE();
+
+   RTE_SET_USED(port_conf);
+   dev->data->ports[port_id] = &eventdev->ports[port_id];
+
+   return 0;
+}
+
+static void
+dpaa_event_port_release(void *port)
+{
+   EVENTDEV_DRV_FUNC_TRACE();
+
+   RTE_SET_USED(port);
+}
+
+static int
+dpaa_event_port_link(struct rte_eventdev *dev, void *port,
+const uint8_t queues[], const uint8_t priorities[],
+uint16_t nb_links)
+{
+   struct dpaa_eventdev *priv = dev->data->dev_private;
+   struct dpaa_port *event_port = (struct dpaa_port *)port;
+   struct dpaa_eventq *event_queue;
+   uint8_t eventq_id;
+   int i;
+
+   RTE_SET_USED(dev);
+   RTE_SET_USED(priorities);
+
+   /* First check that input configuration are valid */
+   for (i = 0; i < nb_links; i++) {
+   eventq_id = queues[i];
+   event_queue = &priv->evq_info[eventq_id];
+   if ((event_queue->event_queue_cfg
+   & RTE_EVENT_QUEUE_CFG_SINGLE_LINK)
+   && (event_queue->event_port)) {
+   return -EINVAL;
+   }
+   }
+
+   for (i = 0; i < nb_links; i++) {
+   eventq_id = queues[i];
+   event_queue = &priv->evq_info[eventq_id];
+   event_port->evq_info[i].event_queue_id = eventq_id;
+   event_port->evq_info[i].ch_id = event_queue->ch_id;
+   event_queue->event_port = port;
+   }
+
+   event_port->num_linked_evq = event_port->num_linked_evq + i;
+
+   return (int)i;
+}
+
+static int
+dpaa_event_port_unlink(struct rte_eventdev *dev, void *port,
+  uint8_t queues[], uint16_t nb_links)
+{
+   int i;
+   uint8_t eventq_id;
+   struct dpaa_eventq *event_queue;
+   struct dpaa_eventdev *priv = dev->data->dev_private;
+   struct dpaa_port *event_port = (struct dpaa_port *)port;
+
+   if (!event_port->num_linked_evq)
+   return nb_links;
+
+   for (i = 0; i < nb_links; i++) {
+   eventq_id = queues[i];
+   event_port->evq_info[eventq_id].event_queue_id = -1;
+   event_port->evq_info[eventq_id].ch_id = 0;
+   event_queue = &priv->evq_info[eventq_id];
+   event_queue->event_port = NULL;
+   }
+
+   event_port->num_linked_evq = event_port->num_linked_evq - i;
+
+   return (int)i;
+}
+
 static const struct rte_eventdev_ops dpaa_eventdev_ops = {
.dev_infos_get= dpaa_event_dev_info_get,
.dev_configure= dpaa_event_dev_configure,
@@ -256,6 +356,11 @@
.queue_def_conf   = dpaa_event_queue_def_conf,
.queue_setup  = dpaa_event_queue_setup,
.queue_release  = dpaa_event_queue_release,
+   .port_def_conf= dpaa_event_port_default_conf_get,
+   .port_setup   = dpaa_event_port_setup,
+   .port_release   = dpaa_event_port_release,
+   .port_link= dpaa_event_port_link,
+   .port_unlink  = dpaa_event_port_unlink,
 };
 
 static int
-- 
1.9.1

[dpdk-dev] [PATCH 06/12 v3] event/dpaa: add event queue config get/set support

[Nipun Gupta]

From: Sunil Kumar Kori 

Signed-off-by: Sunil Kumar Kori 
Acked-by: Hemant Agrawal 
---
 drivers/event/dpaa/dpaa_eventdev.c | 47 ++
 1 file changed, 47 insertions(+)

diff --git a/drivers/event/dpaa/dpaa_eventdev.c 
b/drivers/event/dpaa/dpaa_eventdev.c
index 0bd74bb..f1db711 100644
--- a/drivers/event/dpaa/dpaa_eventdev.c
+++ b/drivers/event/dpaa/dpaa_eventdev.c
@@ -201,7 +201,51 @@
return 0;
 }
 
+static void
+dpaa_event_queue_def_conf(struct rte_eventdev *dev, uint8_t queue_id,
+ struct rte_event_queue_conf *queue_conf)
+{
+   EVENTDEV_DRV_FUNC_TRACE();
+
+   RTE_SET_USED(dev);
+   RTE_SET_USED(queue_id);
+
+   memset(queue_conf, 0, sizeof(struct rte_event_queue_conf));
+   queue_conf->schedule_type = RTE_SCHED_TYPE_PARALLEL;
+   queue_conf->priority = RTE_EVENT_DEV_PRIORITY_HIGHEST;
+}
+
+static int
+dpaa_event_queue_setup(struct rte_eventdev *dev, uint8_t queue_id,
+  const struct rte_event_queue_conf *queue_conf)
+{
+   struct dpaa_eventdev *priv = dev->data->dev_private;
+   struct dpaa_eventq *evq_info = &priv->evq_info[queue_id];
+
+   EVENTDEV_DRV_FUNC_TRACE();
+
+   switch (queue_conf->schedule_type) {
+   case RTE_SCHED_TYPE_PARALLEL:
+   case RTE_SCHED_TYPE_ATOMIC:
+   break;
+   case RTE_SCHED_TYPE_ORDERED:
+   EVENTDEV_DRV_ERR("Schedule type is not supported.");
+   return -1;
+   }
+   evq_info->event_queue_cfg = queue_conf->event_queue_cfg;
+   evq_info->event_queue_id = queue_id;
+
+   return 0;
+}
 
+static void
+dpaa_event_queue_release(struct rte_eventdev *dev, uint8_t queue_id)
+{
+   EVENTDEV_DRV_FUNC_TRACE();
+
+   RTE_SET_USED(dev);
+   RTE_SET_USED(queue_id);
+}
 
 static const struct rte_eventdev_ops dpaa_eventdev_ops = {
.dev_infos_get= dpaa_event_dev_info_get,
@@ -209,6 +253,9 @@
.dev_start= dpaa_event_dev_start,
.dev_stop = dpaa_event_dev_stop,
.dev_close= dpaa_event_dev_close,
+   .queue_def_conf   = dpaa_event_queue_def_conf,
+   .queue_setup  = dpaa_event_queue_setup,
+   .queue_release  = dpaa_event_queue_release,
 };
 
 static int
-- 
1.9.1

[dpdk-dev] [PATCH 08/12 v3] event/dpaa: add dequeue timeout conversion support

[Nipun Gupta]

From: Sunil Kumar Kori 

Signed-off-by: Sunil Kumar Kori 
Acked-by: Hemant Agrawal 
---
 drivers/event/dpaa/dpaa_eventdev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/event/dpaa/dpaa_eventdev.c 
b/drivers/event/dpaa/dpaa_eventdev.c
index 82362d5..b4b7aed 100644
--- a/drivers/event/dpaa/dpaa_eventdev.c
+++ b/drivers/event/dpaa/dpaa_eventdev.c
@@ -361,6 +361,7 @@
.port_release   = dpaa_event_port_release,
.port_link= dpaa_event_port_link,
.port_unlink  = dpaa_event_port_unlink,
+   .timeout_ticks= dpaa_event_dequeue_timeout_ticks,
 };
 
 static int
-- 
1.9.1

[dpdk-dev] [PATCH 09/12 v3] event/dpaa: add eth rx adapter queue config support

[Nipun Gupta]

From: Sunil Kumar Kori 

Signed-off-by: Sunil Kumar Kori 
Acked-by: Hemant Agrawal 
---
 drivers/event/dpaa/dpaa_eventdev.c | 117 +
 1 file changed, 117 insertions(+)

diff --git a/drivers/event/dpaa/dpaa_eventdev.c 
b/drivers/event/dpaa/dpaa_eventdev.c
index b4b7aed..0d9037e 100644
--- a/drivers/event/dpaa/dpaa_eventdev.c
+++ b/drivers/event/dpaa/dpaa_eventdev.c
@@ -347,6 +347,118 @@
return (int)i;
 }
 
+static int
+dpaa_event_eth_rx_adapter_caps_get(const struct rte_eventdev *dev,
+  const struct rte_eth_dev *eth_dev,
+  uint32_t *caps)
+{
+   const char *ethdev_driver = eth_dev->device->driver->name;
+
+   EVENTDEV_DRV_FUNC_TRACE();
+
+   RTE_SET_USED(dev);
+
+   if (!strcmp(ethdev_driver, "net_dpaa"))
+   *caps = RTE_EVENT_ETH_RX_ADAPTER_DPAA_CAP;
+   else
+   *caps = RTE_EVENT_ETH_RX_ADAPTER_SW_CAP;
+
+   return 0;
+}
+
+static int
+dpaa_event_eth_rx_adapter_queue_add(
+   const struct rte_eventdev *dev,
+   const struct rte_eth_dev *eth_dev,
+   int32_t rx_queue_id,
+   const struct rte_event_eth_rx_adapter_queue_conf *queue_conf)
+{
+   struct dpaa_eventdev *eventdev = dev->data->dev_private;
+   uint8_t ev_qid = queue_conf->ev.queue_id;
+   u16 ch_id = eventdev->evq_info[ev_qid].ch_id;
+   struct dpaa_if *dpaa_intf = eth_dev->data->dev_private;
+   int ret, i;
+
+   EVENTDEV_DRV_FUNC_TRACE();
+
+   if (rx_queue_id == -1) {
+   for (i = 0; i < dpaa_intf->nb_rx_queues; i++) {
+   ret = dpaa_eth_eventq_attach(eth_dev, i, ch_id,
+queue_conf);
+   if (ret) {
+   EVENTDEV_DRV_ERR(
+   "Event Queue attach failed:%d\n", ret);
+   goto detach_configured_queues;
+   }
+   }
+   return 0;
+   }
+
+   ret = dpaa_eth_eventq_attach(eth_dev, rx_queue_id, ch_id, queue_conf);
+   if (ret)
+   EVENTDEV_DRV_ERR("dpaa_eth_eventq_attach failed:%d\n", ret);
+   return ret;
+
+detach_configured_queues:
+
+   for (i = (i - 1); i >= 0 ; i--)
+   dpaa_eth_eventq_detach(eth_dev, i);
+
+   return ret;
+}
+
+static int
+dpaa_event_eth_rx_adapter_queue_del(const struct rte_eventdev *dev,
+   const struct rte_eth_dev *eth_dev,
+   int32_t rx_queue_id)
+{
+   int ret, i;
+   struct dpaa_if *dpaa_intf = eth_dev->data->dev_private;
+
+   EVENTDEV_DRV_FUNC_TRACE();
+
+   RTE_SET_USED(dev);
+   if (rx_queue_id == -1) {
+   for (i = 0; i < dpaa_intf->nb_rx_queues; i++) {
+   ret = dpaa_eth_eventq_detach(eth_dev, i);
+   if (ret)
+   EVENTDEV_DRV_ERR(
+   "Event Queue detach failed:%d\n", ret);
+   }
+
+   return 0;
+   }
+
+   ret = dpaa_eth_eventq_detach(eth_dev, rx_queue_id);
+   if (ret)
+   EVENTDEV_DRV_ERR("dpaa_eth_eventq_detach failed:%d\n", ret);
+   return ret;
+}
+
+static int
+dpaa_event_eth_rx_adapter_start(const struct rte_eventdev *dev,
+   const struct rte_eth_dev *eth_dev)
+{
+   EVENTDEV_DRV_FUNC_TRACE();
+
+   RTE_SET_USED(dev);
+   RTE_SET_USED(eth_dev);
+
+   return 0;
+}
+
+static int
+dpaa_event_eth_rx_adapter_stop(const struct rte_eventdev *dev,
+  const struct rte_eth_dev *eth_dev)
+{
+   EVENTDEV_DRV_FUNC_TRACE();
+
+   RTE_SET_USED(dev);
+   RTE_SET_USED(eth_dev);
+
+   return 0;
+}
+
 static const struct rte_eventdev_ops dpaa_eventdev_ops = {
.dev_infos_get= dpaa_event_dev_info_get,
.dev_configure= dpaa_event_dev_configure,
@@ -362,6 +474,11 @@
.port_link= dpaa_event_port_link,
.port_unlink  = dpaa_event_port_unlink,
.timeout_ticks= dpaa_event_dequeue_timeout_ticks,
+   .eth_rx_adapter_caps_get = dpaa_event_eth_rx_adapter_caps_get,
+   .eth_rx_adapter_queue_add = dpaa_event_eth_rx_adapter_queue_add,
+   .eth_rx_adapter_queue_del = dpaa_event_eth_rx_adapter_queue_del,
+   .eth_rx_adapter_start = dpaa_event_eth_rx_adapter_start,
+   .eth_rx_adapter_stop = dpaa_event_eth_rx_adapter_stop,
 };
 
 static int
-- 
1.9.1

[dpdk-dev] [PATCH 10/12 v3] event/dpaa: add eventdev enqueue/dequeue support

[Nipun Gupta]

From: Sunil Kumar Kori 

Signed-off-by: Sunil Kumar Kori 
Acked-by: Hemant Agrawal 
---
 drivers/event/dpaa/dpaa_eventdev.c | 114 +
 1 file changed, 114 insertions(+)

diff --git a/drivers/event/dpaa/dpaa_eventdev.c 
b/drivers/event/dpaa/dpaa_eventdev.c
index 0d9037e..44e6314 100644
--- a/drivers/event/dpaa/dpaa_eventdev.c
+++ b/drivers/event/dpaa/dpaa_eventdev.c
@@ -60,6 +60,116 @@
 }
 
 static void
+dpaa_eventq_portal_add(u16 ch_id)
+{
+   uint32_t sdqcr;
+
+   sdqcr = QM_SDQCR_CHANNELS_POOL_CONV(ch_id);
+   qman_static_dequeue_add(sdqcr, NULL);
+}
+
+static uint16_t
+dpaa_event_enqueue_burst(void *port, const struct rte_event ev[],
+uint16_t nb_events)
+{
+   uint16_t i;
+   struct rte_mbuf *mbuf;
+
+   RTE_SET_USED(port);
+   /*Release all the contexts saved previously*/
+   for (i = 0; i < nb_events; i++) {
+   switch (ev[i].op) {
+   case RTE_EVENT_OP_RELEASE:
+   qman_dca_index(ev[i].impl_opaque, 0);
+   mbuf = DPAA_PER_LCORE_DQRR_MBUF(i);
+   mbuf->seqn = DPAA_INVALID_MBUF_SEQN;
+   DPAA_PER_LCORE_DQRR_HELD &= ~(1 << i);
+   DPAA_PER_LCORE_DQRR_SIZE--;
+   break;
+   default:
+   break;
+   }
+   }
+
+   return nb_events;
+}
+
+static uint16_t
+dpaa_event_enqueue(void *port, const struct rte_event *ev)
+{
+   return dpaa_event_enqueue_burst(port, ev, 1);
+}
+
+static uint16_t
+dpaa_event_dequeue_burst(void *port, struct rte_event ev[],
+uint16_t nb_events, uint64_t timeout_ticks)
+{
+   int ret;
+   u16 ch_id;
+   void *buffers[8];
+   u32 num_frames, i;
+   uint64_t wait_time, cur_ticks, start_ticks;
+   struct dpaa_port *portal = (struct dpaa_port *)port;
+   struct rte_mbuf *mbuf;
+
+   /* Affine current thread context to a qman portal */
+   ret = rte_dpaa_portal_init((void *)0);
+   if (ret) {
+   DPAA_EVENTDEV_ERR("Unable to initialize portal");
+   return ret;
+   }
+
+   if (unlikely(!portal->is_port_linked)) {
+   /*
+* Affine event queue for current thread context
+* to a qman portal.
+*/
+   for (i = 0; i < portal->num_linked_evq; i++) {
+   ch_id = portal->evq_info[i].ch_id;
+   dpaa_eventq_portal_add(ch_id);
+   }
+   portal->is_port_linked = true;
+   }
+
+   /* Check if there are atomic contexts to be released */
+   i = 0;
+   while (DPAA_PER_LCORE_DQRR_SIZE) {
+   if (DPAA_PER_LCORE_DQRR_HELD & (1 << i)) {
+   qman_dca_index(i, 0);
+   mbuf = DPAA_PER_LCORE_DQRR_MBUF(i);
+   mbuf->seqn = DPAA_INVALID_MBUF_SEQN;
+   DPAA_PER_LCORE_DQRR_HELD &= ~(1 << i);
+   DPAA_PER_LCORE_DQRR_SIZE--;
+   }
+   i++;
+   }
+   DPAA_PER_LCORE_DQRR_HELD = 0;
+
+   if (portal->timeout == DPAA_EVENT_PORT_DEQUEUE_TIMEOUT_INVALID)
+   wait_time = timeout_ticks;
+   else
+   wait_time = portal->timeout;
+
+   /* Lets dequeue the frames */
+   start_ticks = rte_get_timer_cycles();
+   wait_time += start_ticks;
+   do {
+   num_frames = qman_portal_dequeue(ev, nb_events, buffers);
+   if (num_frames != 0)
+   break;
+   cur_ticks = rte_get_timer_cycles();
+   } while (cur_ticks < wait_time);
+
+   return num_frames;
+}
+
+static uint16_t
+dpaa_event_dequeue(void *port, struct rte_event *ev, uint64_t timeout_ticks)
+{
+   return dpaa_event_dequeue_burst(port, ev, 1, timeout_ticks);
+}
+
+static void
 dpaa_event_dev_info_get(struct rte_eventdev *dev,
struct rte_event_dev_info *dev_info)
 {
@@ -496,6 +606,10 @@
}
 
eventdev->dev_ops   = &dpaa_eventdev_ops;
+   eventdev->enqueue   = dpaa_event_enqueue;
+   eventdev->enqueue_burst = dpaa_event_enqueue_burst;
+   eventdev->dequeue   = dpaa_event_dequeue;
+   eventdev->dequeue_burst = dpaa_event_dequeue_burst;
 
/* For secondary processes, the primary has done all the work */
if (rte_eal_process_type() != RTE_PROC_PRIMARY)
-- 
1.9.1

[dpdk-dev] [PATCH 12/12 v3] doc: add DPAA eventdev guide

[Nipun Gupta]

From: Sunil Kumar Kori 

Signed-off-by: Sunil Kumar Kori 
Acked-by: Hemant Agrawal 
---
 MAINTAINERS|   1 +
 doc/guides/eventdevs/dpaa.rst  | 140 +
 doc/guides/eventdevs/index.rst |   1 +
 3 files changed, 142 insertions(+)
 create mode 100644 doc/guides/eventdevs/dpaa.rst

diff --git a/MAINTAINERS b/MAINTAINERS
index fff842e..66633e8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -708,6 +708,7 @@ NXP DPAA eventdev
 M: Hemant Agrawal 
 M: Sunil Kumar Kori 
 F: drivers/event/dpaa/
+F: doc/guides/eventdevs/dpaa.rst
 
 Software Eventdev PMD
 M: Harry van Haaren 
diff --git a/doc/guides/eventdevs/dpaa.rst b/doc/guides/eventdevs/dpaa.rst
new file mode 100644
index 000..44ed128
--- /dev/null
+++ b/doc/guides/eventdevs/dpaa.rst
@@ -0,0 +1,140 @@
+.. SPDX-License-Identifier:BSD-3-Clause
+   Copyright 2017 NXP
+
+NXP DPAA Eventdev Driver
+=
+
+The dpaa eventdev is an implementation of the eventdev API, that provides a
+wide range of the eventdev features. The eventdev relies on a dpaa based
+platform to perform event scheduling.
+
+More information can be found at `NXP Official Website
+`_.
+
+Features
+
+
+The DPAA EVENTDEV implements many features in the eventdev API;
+
+- Hardware based event scheduler
+- 4 event ports
+- 4 event queues
+- Parallel flows
+- Atomic flows
+
+Supported DPAA SoCs
+
+
+- LS1046A
+- LS1043A
+
+Prerequisites
+-
+
+There are following pre-requisites for executing EVENTDEV on a DPAA compatible
+platform:
+
+1. **ARM 64 Tool Chain**
+
+  For example, the `*aarch64* Linaro Toolchain 
`_.
+
+2. **Linux Kernel**
+
+   It can be obtained from `NXP's Github hosting 
`_.
+
+3. **Rootfile System**
+
+   Any *aarch64* supporting filesystem can be used. For example,
+   Ubuntu 15.10 (Wily) or 16.04 LTS (Xenial) userland which can be obtained
+   from `here 
`_.
+
+As an alternative method, DPAA EVENTDEV can also be executed using images 
provided
+as part of SDK from NXP. The SDK includes all the above prerequisites necessary
+to bring up a DPAA board.
+
+The following dependencies are not part of DPDK and must be installed
+separately:
+
+- **NXP Linux SDK**
+
+  NXP Linux software development kit (SDK) includes support for family
+  of QorIQ?? ARM-Architecture-based system on chip (SoC) processors
+  and corresponding boards.
+
+  It includes the Linux board support packages (BSPs) for NXP SoCs,
+  a fully operational tool chain, kernel and board specific modules.
+
+  SDK and related information can be obtained from:  `NXP QorIQ SDK  
`_.
+
+- **DPDK Extra Scripts**
+
+  DPAA based resources can be configured easily with the help of ready to use
+  xml files as provided in the DPDK Extra repository.
+
+  `DPDK Extras Scripts `_.
+
+Currently supported by DPDK:
+
+- NXP SDK **2.0+** or LSDK **17.09+**
+- Supported architectures:  **arm64 LE**.
+
+- Follow the DPDK :ref:`Getting Started Guide for Linux ` to setup 
the basic DPDK environment.
+
+Pre-Installation Configuration
+--
+
+Config File Options
+~~~
+
+The following options can be modified in the ``config`` file.
+Please note that enabling debugging options may affect system performance.
+
+- ``CONFIG_RTE_LIBRTE_PMD_DPAA_EVENTDEV`` (default ``y``)
+
+  Toggle compilation of the ``librte_pmd_dpaa_event`` driver.
+
+Driver Compilation
+~~
+
+To compile the DPAA EVENTDEV PMD for Linux arm64 gcc target, run the
+following ``make`` command:
+
+.. code-block:: console
+
+   cd 
+   make config T=arm64-dpaa-linuxapp-gcc install
+
+Initialization
+--
+
+The dpaa eventdev is exposed as a vdev device which consists of a set of 
channels
+and queues. On EAL initialization, dpaa components will be
+probed and then vdev device can be created from the application code by
+
+* Invoking ``rte_vdev_init("event_dpaa")`` from the application
+
+* Using ``--vdev="event_dpaa"`` in the EAL options, which will call
+  rte_vdev_init() internally
+
+Example:
+
+.. code-block:: console
+
+./your_eventdev_application --vdev="event_dpaa"
+
+Limitations
+---
+
+1. DPAA eventdev can not work with DPAA PUSH mode queues configured for ethdev.
+   Please configure export DPAA_NUM_PUSH_QUEUES=0
+
+Platform Requirement
+
+
+DPAA drivers for DPDK can only work on NXP SoCs as listed in the
+``Supported DPAA SoCs``.
+
+Port-core Binding
+~~~

[dpdk-dev] [PATCH 11/12 v3] config: add eventdev library to application

[Nipun Gupta]

From: Sunil Kumar Kori 

Signed-off-by: Sunil Kumar Kori 
Acked-by: Hemant Agrawal 
---
 mk/rte.app.mk | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index 7dfab67..7d09a4c 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -199,6 +199,7 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_SKELETON_EVENTDEV) += 
-lrte_pmd_skeleton_event
 _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_SW_EVENTDEV) += -lrte_pmd_sw_event
 _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_OCTEONTX_SSOVF) += -lrte_pmd_octeontx_ssovf
 _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_DPAA2_EVENTDEV) += -lrte_pmd_dpaa2_event
+_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_DPAA_EVENTDEV) += -lrte_pmd_dpaa_event
 _LDLIBS-$(CONFIG_RTE_LIBRTE_OCTEONTX_MEMPOOL) += -lrte_mempool_octeontx
 _LDLIBS-$(CONFIG_RTE_LIBRTE_OCTEONTX_PMD) += -lrte_pmd_octeontx
 _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_OPDL_EVENTDEV) += -lrte_pmd_opdl_event
-- 
1.9.1

Re: [dpdk-dev] [PATCH] doc: add queue region feature info to release notes

[Ferruh Yigit]

On 1/16/2018 11:27 AM, Thomas Monjalon wrote:
> 21/12/2017 19:10, Ferruh Yigit:
>> On 12/20/2017 7:52 PM, Wei Zhao wrote:
>>> This patch add inforation about i40e queue region
>>> realted to release notes, it has been missed before.
>>>
>>> Signed-off-by: Wei Zhao 
>>> ---
>>>  doc/guides/rel_notes/release_17_11.rst | 17 +
>>
>> I think we shouldn't update release notes once it has been released.
>>
>> Perhaps it can be an option to mention from this in latest release notes 
>> with a
>> note that says actual support added in v17.11?
> 
> I disagree.
> It is really confusing to add 17.11 feature in 18.02 release notes.
> It is better to add it in 17.11 release notes and backport it.
> 
> Please Ferruh, could you remove the patch v2 updating release_18_02.rst
> from next-net?

Updating a release notes after release looks wrong to me. But I will update the
repo to move this into 17.11 release notes, no patch required.

And there is one more similar update, I guess that should be updated too, I will
check it as well.

Re: [dpdk-dev] [PATCH 12/12 v3] doc: add DPAA eventdev guide

[Jerin Jacob]

-Original Message-
> Date: Tue, 16 Jan 2018 23:28:05 +0530
> From: Nipun Gupta 
> To: jerin.ja...@caviumnetworks.com
> CC: dev@dpdk.org, sunil.k...@nxp.com, hemant.agra...@nxp.com
> Subject: [PATCH 12/12 v3] doc: add DPAA eventdev guide
> X-Mailer: git-send-email 1.9.1
> 
> From: Sunil Kumar Kori 
> 
> Signed-off-by: Sunil Kumar Kori 
> Acked-by: Hemant Agrawal 


CC: marko.kovace...@intel.com

Re: [dpdk-dev] [PATCH 08/12 v3] event/dpaa: add dequeue timeout conversion support

[Jerin Jacob]

-Original Message-
> Date: Tue, 16 Jan 2018 23:28:01 +0530
> From: Nipun Gupta 
> To: jerin.ja...@caviumnetworks.com
> CC: dev@dpdk.org, sunil.k...@nxp.com, hemant.agra...@nxp.com
> Subject: [PATCH 08/12 v3] event/dpaa: add dequeue timeout conversion support
> X-Mailer: git-send-email 1.9.1
> 
> From: Sunil Kumar Kori 
> 
> Signed-off-by: Sunil Kumar Kori 
> Acked-by: Hemant Agrawal 
> ---
>  drivers/event/dpaa/dpaa_eventdev.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/event/dpaa/dpaa_eventdev.c 
> b/drivers/event/dpaa/dpaa_eventdev.c
> index 82362d5..b4b7aed 100644
> --- a/drivers/event/dpaa/dpaa_eventdev.c
> +++ b/drivers/event/dpaa/dpaa_eventdev.c
> @@ -361,6 +361,7 @@
>   .port_release   = dpaa_event_port_release,
>   .port_link= dpaa_event_port_link,
>   .port_unlink  = dpaa_event_port_unlink,
> + .timeout_ticks= dpaa_event_dequeue_timeout_ticks,

This patch is almost empty. Please define dpaa_event_dequeue_timeout_ticks() in 
this patch

>  };
>  
>  static int
> -- 
> 1.9.1
> 

Re: [dpdk-dev] [PATCH 11/12 v3] config: add eventdev library to application

[Jerin Jacob]

-Original Message-
> Date: Tue, 16 Jan 2018 23:28:04 +0530
> From: Nipun Gupta 
> To: jerin.ja...@caviumnetworks.com
> CC: dev@dpdk.org, sunil.k...@nxp.com, hemant.agra...@nxp.com
> Subject: [PATCH 11/12 v3] config: add eventdev library to application
> X-Mailer: git-send-email 1.9.1
> 
> From: Sunil Kumar Kori 
> 
> Signed-off-by: Sunil Kumar Kori 
> Acked-by: Hemant Agrawal 
> ---
>  mk/rte.app.mk | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/mk/rte.app.mk b/mk/rte.app.mk
> index 7dfab67..7d09a4c 100644
> --- a/mk/rte.app.mk
> +++ b/mk/rte.app.mk
> @@ -199,6 +199,7 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_SKELETON_EVENTDEV) += 
> -lrte_pmd_skeleton_event
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_SW_EVENTDEV) += -lrte_pmd_sw_event
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_OCTEONTX_SSOVF) += -lrte_pmd_octeontx_ssovf
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_DPAA2_EVENTDEV) += -lrte_pmd_dpaa2_event
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_DPAA_EVENTDEV) += -lrte_pmd_dpaa_event
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_OCTEONTX_MEMPOOL) += -lrte_mempool_octeontx
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_OCTEONTX_PMD) += -lrte_pmd_octeontx
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_OPDL_EVENTDEV) += -lrte_pmd_opdl_event

This patch is mostly empty. This can be squashed with 5/12.

> -- 
> 1.9.1
> 

[dpdk-dev] [PATCH 0/2] net/ena: convert to new offloads API

[Rafal Kozik]

Ethdev offloads API has changed since:

commit cba7f53b717d ("ethdev: introduce Tx queue offloads API")
commit ce17eddefc20 ("ethdev: introduce Rx queue offloads API")

Those patches support new offloads API.

Rafal Kozik (2):
  net/ena: convert to new Tx offloads API
  net/ena: convert to new Rx offloads API

 drivers/net/ena/ena_ethdev.c | 109 ---
 drivers/net/ena/ena_ethdev.h |   5 ++
 2 files changed, 97 insertions(+), 17 deletions(-)

-- 
2.7.4

[dpdk-dev] [PATCH 1/2] net/ena: convert to new Tx offloads API

[Rafal Kozik]

Ethdev Tx offloads API has changed since:

commit cba7f53b717d ("ethdev: introduce Tx queue offloads API")

This commit support the new Tx offloads API. Queue configuration
is stored in ena_ring.offloads. During preparing mbufs for tx, offloads are
allowed only if appropriate flags in this field are set.

Signed-off-by: Rafal Kozik 
---
 drivers/net/ena/ena_ethdev.c | 73 +++-
 drivers/net/ena/ena_ethdev.h |  3 ++
 2 files changed, 61 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 22db895..6473776 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -164,6 +164,14 @@ static const struct ena_stats ena_stats_ena_com_strings[] 
= {
 #define ENA_STATS_ARRAY_RX ARRAY_SIZE(ena_stats_rx_strings)
 #define ENA_STATS_ARRAY_ENA_COMARRAY_SIZE(ena_stats_ena_com_strings)
 
+#define QUEUE_OFFLOADS (DEV_TX_OFFLOAD_TCP_CKSUM |\
+   DEV_TX_OFFLOAD_UDP_CKSUM |\
+   DEV_TX_OFFLOAD_IPV4_CKSUM |\
+   DEV_TX_OFFLOAD_TCP_TSO)
+#define MBUF_OFFLOADS (PKT_TX_L4_MASK |\
+  PKT_TX_IP_CKSUM |\
+  PKT_TX_TCP_SEG)
+
 /** Vendor ID used by Amazon devices */
 #define PCI_VENDOR_ID_AMAZON 0x1D0F
 /** Amazon devices */
@@ -227,6 +235,8 @@ static int ena_rss_reta_query(struct rte_eth_dev *dev,
  struct rte_eth_rss_reta_entry64 *reta_conf,
  uint16_t reta_size);
 static int ena_get_sset_count(struct rte_eth_dev *dev, int sset);
+static bool ena_are_tx_queue_offloads_allowed(struct ena_adapter *adapter,
+ uint64_t offloads);
 
 static const struct eth_dev_ops ena_dev_ops = {
.dev_configure= ena_dev_configure,
@@ -280,21 +290,24 @@ static inline void ena_rx_mbuf_prepare(struct rte_mbuf 
*mbuf,
 }
 
 static inline void ena_tx_mbuf_prepare(struct rte_mbuf *mbuf,
-  struct ena_com_tx_ctx *ena_tx_ctx)
+  struct ena_com_tx_ctx *ena_tx_ctx,
+  uint64_t queue_offloads)
 {
struct ena_com_tx_meta *ena_meta = &ena_tx_ctx->ena_meta;
 
-   if (mbuf->ol_flags &
-   (PKT_TX_L4_MASK | PKT_TX_IP_CKSUM | PKT_TX_TCP_SEG)) {
+   if ((mbuf->ol_flags & MBUF_OFFLOADS) &&
+   (queue_offloads & QUEUE_OFFLOADS)) {
/* check if TSO is required */
-   if (mbuf->ol_flags & PKT_TX_TCP_SEG) {
+   if ((mbuf->ol_flags & PKT_TX_TCP_SEG) &&
+   (queue_offloads & DEV_TX_OFFLOAD_TCP_TSO)) {
ena_tx_ctx->tso_enable = true;
 
ena_meta->l4_hdr_len = GET_L4_HDR_LEN(mbuf);
}
 
/* check if L3 checksum is needed */
-   if (mbuf->ol_flags & PKT_TX_IP_CKSUM)
+   if ((mbuf->ol_flags & PKT_TX_IP_CKSUM) &&
+   (queue_offloads & DEV_TX_OFFLOAD_IPV4_CKSUM))
ena_tx_ctx->l3_csum_enable = true;
 
if (mbuf->ol_flags & PKT_TX_IPV6) {
@@ -310,19 +323,17 @@ static inline void ena_tx_mbuf_prepare(struct rte_mbuf 
*mbuf,
}
 
/* check if L4 checksum is needed */
-   switch (mbuf->ol_flags & PKT_TX_L4_MASK) {
-   case PKT_TX_TCP_CKSUM:
+   if ((mbuf->ol_flags & PKT_TX_TCP_CKSUM) &&
+   (queue_offloads & DEV_TX_OFFLOAD_TCP_CKSUM)) {
ena_tx_ctx->l4_proto = ENA_ETH_IO_L4_PROTO_TCP;
ena_tx_ctx->l4_csum_enable = true;
-   break;
-   case PKT_TX_UDP_CKSUM:
+   } else if ((mbuf->ol_flags & PKT_TX_UDP_CKSUM) &&
+  (queue_offloads & DEV_TX_OFFLOAD_UDP_CKSUM)) {
ena_tx_ctx->l4_proto = ENA_ETH_IO_L4_PROTO_UDP;
ena_tx_ctx->l4_csum_enable = true;
-   break;
-   default:
+   } else {
ena_tx_ctx->l4_proto = ENA_ETH_IO_L4_PROTO_UNKNOWN;
ena_tx_ctx->l4_csum_enable = false;
-   break;
}
 
ena_meta->mss = mbuf->tso_segsz;
@@ -945,7 +956,7 @@ static int ena_tx_queue_setup(struct rte_eth_dev *dev,
  uint16_t queue_idx,
  uint16_t nb_desc,
  __rte_unused unsigned int socket_id,
- __rte_unused const struct rte_eth_txconf *tx_conf)
+ const struct rte_eth_txconf *tx_conf)
 {
struct ena_com_create_io_ctx ctx =
/* policy set to _HOST just to satisfy icc compiler */
@@ -982,6 +993,11 @@ static int ena_tx_queue_setup(struct rte_eth_dev *dev,
return -EINVAL;
}
 
+   

[dpdk-dev] [PATCH 2/2] net/ena: convert to new Rx offloads API

[Rafal Kozik]

Ethdev Rx offloads API has changed since:

commit ce17eddefc20 ("ethdev: introduce Rx queue offloads API")

This commit support the new Rx offloads API.

Signed-off-by: Rafal Kozik 
---
 drivers/net/ena/ena_ethdev.c | 36 ++--
 drivers/net/ena/ena_ethdev.h |  2 ++
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 6473776..f069ca8 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -237,6 +237,8 @@ static int ena_rss_reta_query(struct rte_eth_dev *dev,
 static int ena_get_sset_count(struct rte_eth_dev *dev, int sset);
 static bool ena_are_tx_queue_offloads_allowed(struct ena_adapter *adapter,
  uint64_t offloads);
+static bool ena_are_rx_queue_offloads_allowed(struct ena_adapter *adapter,
+ uint64_t offloads);
 
 static const struct eth_dev_ops ena_dev_ops = {
.dev_configure= ena_dev_configure,
@@ -766,7 +768,8 @@ static uint32_t ena_get_mtu_conf(struct ena_adapter 
*adapter)
 {
uint32_t max_frame_len = adapter->max_mtu;
 
-   if (adapter->rte_eth_dev_data->dev_conf.rxmode.jumbo_frame == 1)
+   if (adapter->rte_eth_dev_data->dev_conf.rxmode.offloads &
+   DEV_RX_OFFLOAD_JUMBO_FRAME)
max_frame_len =

adapter->rte_eth_dev_data->dev_conf.rxmode.max_rx_pkt_len;
 
@@ -1065,7 +1068,7 @@ static int ena_rx_queue_setup(struct rte_eth_dev *dev,
  uint16_t queue_idx,
  uint16_t nb_desc,
  __rte_unused unsigned int socket_id,
- __rte_unused const struct rte_eth_rxconf *rx_conf,
+ const struct rte_eth_rxconf *rx_conf,
  struct rte_mempool *mp)
 {
struct ena_com_create_io_ctx ctx =
@@ -1101,6 +1104,11 @@ static int ena_rx_queue_setup(struct rte_eth_dev *dev,
return -EINVAL;
}
 
+   if (!ena_are_rx_queue_offloads_allowed(adapter, rx_conf->offloads)) {
+   RTE_LOG(ERR, PMD, "Unsupported queue offloads\n");
+   return -EINVAL;
+   }
+
ena_qid = ENA_IO_RXQ_IDX(queue_idx);
 
ctx.qid = ena_qid;
@@ -1405,6 +1413,7 @@ static int ena_dev_configure(struct rte_eth_dev *dev)
struct ena_adapter *adapter =
(struct ena_adapter *)(dev->data->dev_private);
uint64_t tx_offloads = dev->data->dev_conf.txmode.offloads;
+   uint64_t rx_offloads = dev->data->dev_conf.rxmode.offloads;
 
if ((tx_offloads & adapter->tx_supported_offloads) != tx_offloads) {
RTE_LOG(ERR, PMD, "Some Tx offloads are not supported "
@@ -1413,6 +1422,13 @@ static int ena_dev_configure(struct rte_eth_dev *dev)
return -ENOTSUP;
}
 
+   if ((rx_offloads & adapter->rx_supported_offloads) != rx_offloads) {
+   RTE_LOG(ERR, PMD, "Some Rx offloads are not supported "
+   "requested 0x%lx supported 0x%lx\n",
+   rx_offloads, adapter->rx_supported_offloads);
+   return -ENOTSUP;
+   }
+
if (!(adapter->state == ENA_ADAPTER_STATE_INIT ||
  adapter->state == ENA_ADAPTER_STATE_STOPPED)) {
PMD_INIT_LOG(ERR, "Illegal adapter state: %d",
@@ -1434,6 +1450,7 @@ static int ena_dev_configure(struct rte_eth_dev *dev)
}
 
adapter->tx_selected_offloads = tx_offloads;
+   adapter->rx_selected_offloads = rx_offloads;
return 0;
 }
 
@@ -1475,6 +1492,19 @@ static bool ena_are_tx_queue_offloads_allowed(struct 
ena_adapter *adapter,
return true;
 }
 
+static bool ena_are_rx_queue_offloads_allowed(struct ena_adapter *adapter,
+ uint64_t offloads)
+{
+   uint64_t port_offloads = adapter->rx_selected_offloads;
+
+   /* Check if port supports all requested offloads.
+* True if all offloads selected for queue are set for port.
+*/
+   if ((offloads & port_offloads) != offloads)
+   return false;
+   return true;
+}
+
 static void ena_infos_get(struct rte_eth_dev *dev,
  struct rte_eth_dev_info *dev_info)
 {
@@ -1529,6 +1559,7 @@ static void ena_infos_get(struct rte_eth_dev *dev,
 
/* Inform framework about available features */
dev_info->rx_offload_capa = rx_feat;
+   dev_info->rx_queue_offload_capa = rx_feat;
dev_info->tx_offload_capa = tx_feat;
dev_info->tx_queue_offload_capa = tx_feat;
 
@@ -1541,6 +1572,7 @@ static void ena_infos_get(struct rte_eth_dev *dev,
dev_info->reta_size = ENA_RX_RSS_TABLE_SIZE;
 
adapter->tx_supported_offloads = tx_feat;
+   adapter->rx_supported_offloads = rx_feat;
 }
 
 static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,

Re: [dpdk-dev] [PATCH v4 04/13] app/eventdev: add pipeline opt dump and check functions

[Van Haaren, Harry]

> From: Pavan Nikhilesh [mailto:pbhagavat...@caviumnetworks.com]
> Sent: Friday, January 12, 2018 4:44 PM
> To: jerin.ja...@caviumnetworks.com; santosh.shu...@caviumnetworks.com; Van
> Haaren, Harry ; Eads, Gage
> ; hemant.agra...@nxp.com; nipun.gu...@nxp.com; Ma,
> Liang J 
> Cc: dev@dpdk.org; Pavan Nikhilesh 
> Subject: [dpdk-dev] [PATCH v4 04/13] app/eventdev: add pipeline opt dump and
> check functions
> 
> Signed-off-by: Pavan Nikhilesh 

Acked-by: Harry van Haaren 

Re: [dpdk-dev] [PATCH v4 05/13] app/eventdev: add pipeline ethport setup and destroy

[Van Haaren, Harry]

> From: Pavan Nikhilesh [mailto:pbhagavat...@caviumnetworks.com]
> Sent: Friday, January 12, 2018 4:44 PM
> To: jerin.ja...@caviumnetworks.com; santosh.shu...@caviumnetworks.com; Van
> Haaren, Harry ; Eads, Gage
> ; hemant.agra...@nxp.com; nipun.gu...@nxp.com; Ma,
> Liang J 
> Cc: dev@dpdk.org; Pavan Nikhilesh 
> Subject: [dpdk-dev] [PATCH v4 05/13] app/eventdev: add pipeline ethport
> setup and destroy
> 
> Add common ethdev port setup and destroy along with event dev destroy.
> 
> Signed-off-by: Pavan Nikhilesh 

Acked-by: Harry van Haaren 

Re: [dpdk-dev] [PATCH v4 06/13] app/eventdev: add event port setup and Rx adapter setup

[Van Haaren, Harry]

> From: Pavan Nikhilesh [mailto:pbhagavat...@caviumnetworks.com]
> Sent: Friday, January 12, 2018 4:44 PM
> To: jerin.ja...@caviumnetworks.com; santosh.shu...@caviumnetworks.com; Van
> Haaren, Harry ; Eads, Gage
> ; hemant.agra...@nxp.com; nipun.gu...@nxp.com; Ma,
> Liang J 
> Cc: dev@dpdk.org; Pavan Nikhilesh 
> Subject: [dpdk-dev] [PATCH v4 06/13] app/eventdev: add event port setup and
> Rx adapter setup
> 
> Setup one port per worker and link to all queues and setup producer port
> based on Rx adapter capabilities.
> 
> Signed-off-by: Pavan Nikhilesh 

Acked-by: Harry van Haaren 

Re: [dpdk-dev] [PATCH v5] sched: make RED scaling configurable

[Dumitrescu, Cristian]

> > > Hi Alan,
> > >
> > > What is the intention of the new rte_red_set_scaling() function?
> > >   1. Is it to be called only once, before any RED object gets created?
> > >   2. Is it possible to call it post-init, but in this case any RED
> > > object already created are not impacted (they continue to work)?
> > >
> > > If the answer is 2, then yes, we could simply drop the
> > > __rte_red_reset() and do the RED globals reset as part of the
> > > rte_red_set_scaling() function transparently.
> > >
> > > If the answer is 1, then we probably need to keep your approach: we
> > > need a global rte_red_init_done flag, and rte_red_set_scaling() could
> > > only be called at init time before any red objects are created.
> > >
> > > I probably need to spend more time assessing all the code implications.
> > >
> > > Regards,
> > > Cristian
> >
> > Hi Alan,
> >
> > After talking to Tomasz, we agreed that 2. Is not an option, as any
> previously created red object will be broken.
> >
> > So 1. Is the right answer, therefore this function can be called only once
> and only before any red objects are created. So, IMO we should do this:
> -when rte_red_init_done is true, we need to return error -when
> rte_red_init_done is false, we need to perform red initialization and set this
> flag
> >
> > Agree?
> 
> Yes, as long as you're happy with keeping __rte_red_reset for the unit-tests.
> 

OK

Re: [dpdk-dev] [PATCH v2] eal: add function to return number of detected sockets

[Burakov, Anatoly]

On 12-Jan-18 11:50 AM, Thomas Monjalon wrote:

12/01/2018 12:44, Burakov, Anatoly:

On 11-Jan-18 10:20 PM, Thomas Monjalon wrote:

22/12/2017 13:41, Anatoly Burakov:

During lcore scan, find maximum socket ID and store it.

Signed-off-by: Anatoly Burakov 
---
--- a/lib/librte_eal/common/include/rte_eal.h
+++ b/lib/librte_eal/common/include/rte_eal.h
@@ -83,6 +83,7 @@ enum rte_proc_type_t {
   struct rte_config {
uint32_t master_lcore;   /**< Id of the master lcore */
uint32_t lcore_count;/**< Number of available logical cores. */
+   uint32_t numa_node_count;/**< Number of detected NUMA nodes. */
uint32_t service_lcore_count;/**< Number of available service cores. */
enum rte_lcore_role_t lcore_role[RTE_MAX_LCORE]; /**< State of cores. */


isn't it breaking the ABI?




Yep, you're right, forgot to add that. I didn't expect this to get
merged in 18.02 anyway, so v2 will follow.


Please write 18.05 in the subject to show your expectation.
Thanks



Does it have to be an ABI change though? We can put numa_node_count 
after pointer to mem_config, in which case it won't be an ABI break. 
Would that be better?


--
Thanks,
Anatoly

Re: [dpdk-dev] [PATCH v4 07/13] app/eventdev: add Tx service setup

[Van Haaren, Harry]

> From: Pavan Nikhilesh [mailto:pbhagavat...@caviumnetworks.com]
> Sent: Friday, January 12, 2018 4:44 PM
> To: jerin.ja...@caviumnetworks.com; santosh.shu...@caviumnetworks.com; Van
> Haaren, Harry ; Eads, Gage
> ; hemant.agra...@nxp.com; nipun.gu...@nxp.com; Ma,
> Liang J 
> Cc: dev@dpdk.org; Pavan Nikhilesh 
> Subject: [dpdk-dev] [PATCH v4 07/13] app/eventdev: add Tx service setup
> 
> Setup one port event port for Tx and link the respective event queue.
> Register the Tx function as a service to be called from a service core.
> The Tx function dequeues the events from the event queue and transmits
> the packet to its respective ethernet port.
> 
> Signed-off-by: Pavan Nikhilesh 

Just a comment below - no changes required.

Acked-by: Harry van Haaren 


> +int
> +pipeline_event_tx_service_setup(struct evt_test *test, struct evt_options
> *opt,
> + uint8_t tx_queue_id, uint8_t tx_port_id,
> + const struct rte_event_port_conf p_conf)
> +{

> + /* Register Tx service */
> + memset(&serv, 0, sizeof(struct rte_service_spec));
> + snprintf(serv.name, sizeof(serv.name), "Tx_service");
> +
> + if (evt_has_burst_mode(opt->dev_id))
> + serv.callback = pipeline_event_tx_burst_service_func;
> + else
> + serv.callback = pipeline_event_tx_service_func;
> +
> + serv.callback_userdata = (void *)tx;
> + ret = rte_service_component_register(&serv, &tx->service_id);
> + if (ret) {
> + evt_err("failed to register Tx service");
> + return ret;
> + }
> +
> + ret = evt_service_setup(tx->service_id);
> + if (ret) {
> + evt_err("Failed to setup service core for Tx service\n");
> + return ret;
> + }
> +
> + rte_service_runstate_set(tx->service_id, 1);


It looks like the code above never sets the "component runstate" to indicate 
that the TX service itself is in a runnable state.

This setting of the runstate is performed later in the setup process, when 
launching cores.

Re: [dpdk-dev] [PATCH v4 08/13] app/eventdev: launch pipeline lcores

[Van Haaren, Harry]

> From: Pavan Nikhilesh [mailto:pbhagavat...@caviumnetworks.com]
> Sent: Friday, January 12, 2018 4:44 PM
> To: jerin.ja...@caviumnetworks.com; santosh.shu...@caviumnetworks.com; Van
> Haaren, Harry ; Eads, Gage
> ; hemant.agra...@nxp.com; nipun.gu...@nxp.com; Ma,
> Liang J 
> Cc: dev@dpdk.org; Pavan Nikhilesh 
> Subject: [dpdk-dev] [PATCH v4 08/13] app/eventdev: launch pipeline lcores
> 
> The event master lcore's test termination and the logic to print the mpps
> are common for the queue and all types queue test.
> 
> Move them as the common function.
> 
> Signed-off-by: Pavan Nikhilesh 

Acked-by: Harry van Haaren 

Re: [dpdk-dev] [PATCH v4 09/13] app/eventdev: add pipeline queue test

[Van Haaren, Harry]

> From: Pavan Nikhilesh [mailto:pbhagavat...@caviumnetworks.com]
> Sent: Friday, January 12, 2018 4:44 PM
> To: jerin.ja...@caviumnetworks.com; santosh.shu...@caviumnetworks.com; Van
> Haaren, Harry ; Eads, Gage
> ; hemant.agra...@nxp.com; nipun.gu...@nxp.com; Ma,
> Liang J 
> Cc: dev@dpdk.org; Pavan Nikhilesh 
> Subject: [dpdk-dev] [PATCH v4 09/13] app/eventdev: add pipeline queue test
> 
> This is a pipeline queue test case that aims at testing the following:
> 1. Measure the end-to-end performance of an event dev with a ethernet dev.
> 2. Maintain packet ordering from Rx to Tx.
> 
> The pipeline queue test configures the eventdev with Q queues and P ports,
> where Q is (nb_ethdev * nb_stages) + nb_ethdev and P is nb_workers.
> 
> The user can choose the number of workers and number of stages through the
> --wlcores and the --stlist application command line arguments respectively.
> The probed ethernet devices act as producer(s) for this application.
> 
> The ethdevs are configured as event Rx adapters that enables them to
> injects events to eventdev based the first stage schedule type list
> requested by the user through --stlist the command line argument.
> 
> Based on the number of stages to process(selected through --stlist),
> the application forwards the event to next upstream queue and when it
> reaches last stage in the pipeline if the event type is ATOMIC it is
> enqueued onto ethdev Tx queue else to maintain ordering the event type is
> set to ATOMIC and enqueued onto the last stage queue.
> On packet Tx, application increments the number events processed and print
> periodically in one second to get the number of events processed in one
> second.
> 
> Note: The --prod_type_ethdev is mandatory for running the application.
> 
> Example command to run pipeline queue test:
> sudo build/app/dpdk-test-eventdev -c 0xf -s 0x8 --vdev=event_sw0 -- \
> --test=pipeline_queue --wlcore=1 --prod_type_ethdev --stlist=ao
> 
> Signed-off-by: Pavan Nikhilesh 

Acked-by: Harry van Haaren 

Re: [dpdk-dev] [PATCH 1/2] lib/cryptodev: add support to set session private data

[Akhil Goyal]

Hi Abhinandan,
On 1/16/2018 5:06 PM, Gujjar, Abhinandan S wrote:

Hi Akhil,


-Original Message-
From: Akhil Goyal [mailto:akhil.go...@nxp.com]
Sent: Tuesday, January 16, 2018 2:52 PM
To: Gujjar, Abhinandan S ; Doherty, Declan
; Jacob, Jerin

Cc: dev@dpdk.org; Vangati, Narender ; Rao,
Nikhil 
Subject: Re: [PATCH 1/2] lib/cryptodev: add support to set session private data

On 1/16/2018 2:33 PM, Gujjar, Abhinandan S wrote:

Hi Akhil,


-Original Message-
From: Akhil Goyal [mailto:akhil.go...@nxp.com]
Sent: Tuesday, January 16, 2018 12:56 PM
To: Gujjar, Abhinandan S ; Doherty,
Declan ; Jacob, Jerin

Cc: dev@dpdk.org; Vangati, Narender ;
Rao, Nikhil 
Subject: Re: [PATCH 1/2] lib/cryptodev: add support to set session
private data

Hi Abhinandan,
On 1/16/2018 12:35 PM, Gujjar, Abhinandan S wrote:

Hi Akhil,


-Original Message-
From: Akhil Goyal [mailto:akhil.go...@nxp.com]
Sent: Tuesday, January 16, 2018 11:55 AM
To: Gujjar, Abhinandan S ; Doherty,
Declan 
Cc: dev@dpdk.org; Vangati, Narender ;
Rao, Nikhil 
Subject: Re: [PATCH 1/2] lib/cryptodev: add support to set session
private data

Hi Abhinandan,
On 1/16/2018 11:39 AM, Gujjar, Abhinandan S wrote:

diff --git a/lib/librte_cryptodev/rte_crypto.h
b/lib/librte_cryptodev/rte_crypto.h
index bbc510d..3a98cbf 100644
--- a/lib/librte_cryptodev/rte_crypto.h
+++ b/lib/librte_cryptodev/rte_crypto.h
@@ -62,6 +62,18 @@ enum rte_crypto_op_sess_type {
RTE_CRYPTO_OP_SECURITY_SESSION  /**< Security session

crypto

operation */

  };

+/** Private data types for cryptographic operation
+ * @see rte_crypto_op::private_data_type */ enum
+rte_crypto_op_private_data_type {
+   RTE_CRYPTO_OP_PRIVATE_DATA_NONE,
+   /**< No private data */
+   RTE_CRYPTO_OP_PRIVATE_DATA_OP,
+   /**< Private data is part of rte_crypto_op and indicated by
+* private_data_offset */
+   RTE_CRYPTO_OP_PRIVATE_DATA_SESSION
+   /**< Private data is available at session */ };
+

We may get away with this enum. If private_data_offset is "0",
then we can check with the session if it has priv data or not.

Right now,  Application uses 'rte_crypto_op_private_data_type' to
indicate rte_cryptodev_sym_session_set_private_data()
was called to set the private data. Otherwise, how do you indicate
there is a

private data associated with the session?

Any suggestions?

For session based flows, the first choice to store the private data
should be in the session. So RTE_CRYPTO_OP_WITH_SESSION or
RTE_CRYPTO_OP_SECURITY_SESSION can be used to call
rte_cryptodev_sym_session_set_private_data or
rte_security_session_set_private_data.

Case 1: private_data_offset is "0" and sess_type =
RTE_CRYPTO_OP_WITH_SESSION -> usual case Case 2: private_data_offset
is "0" and sess_type = RTE_CRYPTO_OP_WITH_SESSION + event case
(access private data) Differentiating between case 1 & 2 will be
done by checking

rte_crypto_op_private_data_type ==
RTE_CRYPTO_OP_PRIVATE_DATA_SESSION.

Consider this:
if (sess_type == RTE_CRYPTO_OP_WITH_SESSION &&
rte_cryptodev_sym_session_get_private_data == NULL)
usual case.
else if (sess_type = RTE_CRYPTO_OP_WITH_SESSION &&
rte_cryptodev_sym_session_get_private_data != NULL)
event case.
else if (sess_type == RTE_CRYPTO_OP_SESSIONLESS &&
private_data_offset != 0)
event case for sessionless op.

I hope all cases can be handled in this way.

Memory allocated for private data will be continuation of session memory.
I think, rte_cryptodev_sym_session_get_private_data() will return a valid

pointer.

It could be pointer to private data, in case application has allocated mempool

with session + private data.

It could be again a pointer to a location(may be next session),  in case

application has allocated mempool with session only.

Unless, there is a flag in the session data which will be set by
rte_cryptodev_sym_session_set_private_data()
If this flag is not set, rte_cryptodev_sym_session_get_private_data() will

return NULL.

I am not claiming, I have complete knowledge of different usage case of

mempool setup for crypto.

I am wondering, whether I am missing anything here. Please let me know.


It depends on the implementation of the get/set API. We can set NULL, if it is
not filled in the set API. If it is set then we have a valid pointer.

The plan is to store private data after "sess * nb_drivers ".
As you said, if it is implementation specific, flag may be again required at
struct rte_cryptodev_sym_session

I think my previous statement was not clear.
My point is that whatever we set in the 
rte_cryptodev_sym_session_set_private_data() is a valid value when we 
call this API explicitly. And before calling the set API, the values are 
zero or any invalid value. So if application calls the get API before 
setting it with set API, it will get an invalid value(may be NULL or 
zero or whatever).



OR
If it is planned to store at PMD's sess_private_data, it requires addi

Re: [dpdk-dev] [PATCH v4 10/13] app/eventdev: add pipeline queue worker functions

[Van Haaren, Harry]

> From: Pavan Nikhilesh [mailto:pbhagavat...@caviumnetworks.com]
> Sent: Friday, January 12, 2018 4:44 PM
> To: jerin.ja...@caviumnetworks.com; santosh.shu...@caviumnetworks.com; Van
> Haaren, Harry ; Eads, Gage
> ; hemant.agra...@nxp.com; nipun.gu...@nxp.com; Ma,
> Liang J 
> Cc: dev@dpdk.org; Pavan Nikhilesh 
> Subject: [dpdk-dev] [PATCH v4 10/13] app/eventdev: add pipeline queue worker
> functions
> 
> Signed-off-by: Pavan Nikhilesh 

Some of the code feels like duplication - but there are differences in each 
implementation as far as I can see. The common initialization parts are 
"macro-ed" to ensure minimal duplication. In short, I'm not aware of a better 
solution/implementation.

Please see note on branch-optimization in the code below.

Acked-by: Harry van Haaren 


>  static int
>  worker_wrapper(void *arg)
>  {
> - RTE_SET_USED(arg);
> + struct worker_data *w  = arg;
> + struct evt_options *opt = w->t->opt;
> + const bool burst = evt_has_burst_mode(w->dev_id);
> + const bool mt_safe = !w->t->mt_unsafe;
> + const uint8_t nb_stages = opt->nb_stages;
> + RTE_SET_USED(opt);
> +
> + /* allow compiler to optimize */
> + if (nb_stages == 1) {
> + if (!burst && mt_safe)
> + return pipeline_queue_worker_single_stage_tx(arg);
> + else if (!burst && !mt_safe)
> + return pipeline_queue_worker_single_stage_fwd(arg);
> + else if (burst && mt_safe)
> + return pipeline_queue_worker_single_stage_burst_tx(arg);
> + else if (burst && !mt_safe)
> + return pipeline_queue_worker_single_stage_burst_fwd(
> + arg);
> + } else {
> + if (!burst && mt_safe)
> + return pipeline_queue_worker_multi_stage_tx(arg);
> + else if (!burst && !mt_safe)
> + return pipeline_queue_worker_multi_stage_fwd(arg);
> + else if (burst && mt_safe)
> + return pipeline_queue_worker_multi_stage_burst_tx(arg);
> + else if (burst && !mt_safe)
> + return pipeline_queue_worker_multi_stage_burst_fwd(arg);
> +
> + }


I don't think the compiler will optimize the below, as the nb_stages value 
comes from opt, which is a uint8 originating from a pointer-dereference... 

As far as I know, an optimizing compiler will do "constant propagation" through 
function calls, but I would be amazed if it found a pointer initialized to a 
specific value, and optimized the branches away here based on the contents of 
that pointer deref.

So I see 2 options;
1) accept as is - I'll ack this patch here
2) Rework so that the worker_wrapper() accepts nb_stages, burst and mt_safe as 
function arguments, allowing constant propagation for optimizing away the 
branches.

@Pavan, I'll leave that choice up to you!

Re: [dpdk-dev] [PATCH v4 11/13] app/eventdev: add pipeline atq test

[Van Haaren, Harry]

> From: Pavan Nikhilesh [mailto:pbhagavat...@caviumnetworks.com]
> Sent: Friday, January 12, 2018 4:44 PM
> To: jerin.ja...@caviumnetworks.com; santosh.shu...@caviumnetworks.com; Van
> Haaren, Harry ; Eads, Gage
> ; hemant.agra...@nxp.com; nipun.gu...@nxp.com; Ma,
> Liang J 
> Cc: dev@dpdk.org; Pavan Nikhilesh 
> Subject: [dpdk-dev] [PATCH v4 11/13] app/eventdev: add pipeline atq test
> 
> This is a pipeline test case that aims at testing the following with
> ``all types queue`` eventdev scheme.
> 1. Measure the end-to-end performance of an event dev with a ethernet dev.
> 2. Maintain packet ordering from Rx to Tx.
> 
> The atq queue test functions as same as ``pipeline_queue`` test.
> The difference is, It uses, ``all type queue scheme`` instead of separate
> queues for each stage and thus reduces the number of queues required to
> realize the use case.
> 
> Note: The --prod_type_ethdev is mandatory for running the application.
> 
> Example command to run pipeline atq test:
> sudo build/app/dpdk-test-eventdev -c 0xf -s 0x8 --vdev=event_sw0 -- \
> --test=pipeline_atq --wlcore=1 --prod_type_ethdev --stlist=ao
> 
> Signed-off-by: Pavan Nikhilesh 

Acked-by: Harry van Haaren 

Re: [dpdk-dev] [PATCH v4 12/13] app/eventdev: add pipeline atq worker functions

[Van Haaren, Harry]

> From: Pavan Nikhilesh [mailto:pbhagavat...@caviumnetworks.com]
> Sent: Friday, January 12, 2018 4:44 PM
> To: jerin.ja...@caviumnetworks.com; santosh.shu...@caviumnetworks.com; Van
> Haaren, Harry ; Eads, Gage
> ; hemant.agra...@nxp.com; nipun.gu...@nxp.com; Ma,
> Liang J 
> Cc: dev@dpdk.org; Pavan Nikhilesh 
> Subject: [dpdk-dev] [PATCH v4 12/13] app/eventdev: add pipeline atq worker
> functions
> 
> Signed-off-by: Pavan Nikhilesh 

Same comments about duplication as 2 patches previous, and this re-uses the 
previous defines.

Acked-by: Harry van Haaren 

> ---
>  app/test-eventdev/test_pipeline_atq.c | 281
> +-
>  1 file changed, 280 insertions(+), 1 deletion(-)
> 
> diff --git a/app/test-eventdev/test_pipeline_atq.c b/app/test-
> eventdev/test_pipeline_atq.c
> index 6c9ac6119..f4cfd3cc7 100644
> --- a/app/test-eventdev/test_pipeline_atq.c
> +++ b/app/test-eventdev/test_pipeline_atq.c
> @@ -15,10 +15,289 @@ pipeline_atq_nb_event_queues(struct evt_options *opt)
>   return rte_eth_dev_count();
>  }
> 
> +static int
> +pipeline_atq_worker_single_stage_tx(void *arg)
> +{
> + PIPELINE_WROKER_SINGLE_STAGE_INIT;
> +
> + while (t->done == false) {
> + uint16_t event = rte_event_dequeue_burst(dev, port, &ev, 1, 0);
> +
> + if (!event) {
> + rte_pause();
> + continue;
> + }
> +
> + if (ev.sched_type == RTE_SCHED_TYPE_ATOMIC) {
> + pipeline_tx_pkt(ev.mbuf);
> + w->processed_pkts++;
> + continue;
> + }
> + pipeline_fwd_event(&ev, RTE_SCHED_TYPE_ATOMIC);
> + pipeline_event_enqueue(dev, port, &ev);
> + }
> +
> + return 0;
> +}
> +
> +static int
> +pipeline_atq_worker_single_stage_fwd(void *arg)
> +{
> + PIPELINE_WROKER_SINGLE_STAGE_INIT;
> + const uint8_t tx_queue = t->tx_service.queue_id;
> +
> + while (t->done == false) {
> + uint16_t event = rte_event_dequeue_burst(dev, port, &ev, 1, 0);
> +
> + if (!event) {
> + rte_pause();
> + continue;
> + }
> +
> + w->processed_pkts++;
> + ev.queue_id = tx_queue;
> + pipeline_fwd_event(&ev, RTE_SCHED_TYPE_ATOMIC);
> + pipeline_event_enqueue(dev, port, &ev);
> + }
> +
> + return 0;
> +}
> +
> +static int
> +pipeline_atq_worker_single_stage_burst_tx(void *arg)
> +{
> + PIPELINE_WROKER_SINGLE_STAGE_BURST_INIT;
> +
> + while (t->done == false) {
> + uint16_t nb_rx = rte_event_dequeue_burst(dev, port, ev,
> + BURST_SIZE, 0);
> +
> + if (!nb_rx) {
> + rte_pause();
> + continue;
> + }
> +
> + for (i = 0; i < nb_rx; i++) {
> + rte_prefetch0(ev[i + 1].mbuf);
> + if (ev[i].sched_type == RTE_SCHED_TYPE_ATOMIC) {
> +
> + pipeline_tx_pkt(ev[i].mbuf);
> + ev[i].op = RTE_EVENT_OP_RELEASE;
> + w->processed_pkts++;
> + } else
> + pipeline_fwd_event(&ev[i],
> + RTE_SCHED_TYPE_ATOMIC);
> + }
> +
> + pipeline_event_enqueue_burst(dev, port, ev, nb_rx);
> + }
> +
> + return 0;
> +}
> +
> +static int
> +pipeline_atq_worker_single_stage_burst_fwd(void *arg)
> +{
> + PIPELINE_WROKER_SINGLE_STAGE_BURST_INIT;
> + const uint8_t tx_queue = t->tx_service.queue_id;
> +
> + while (t->done == false) {
> + uint16_t nb_rx = rte_event_dequeue_burst(dev, port, ev,
> + BURST_SIZE, 0);
> +
> + if (!nb_rx) {
> + rte_pause();
> + continue;
> + }
> +
> + for (i = 0; i < nb_rx; i++) {
> + rte_prefetch0(ev[i + 1].mbuf);
> + ev[i].queue_id = tx_queue;
> + pipeline_fwd_event(&ev[i], RTE_SCHED_TYPE_ATOMIC);
> + w->processed_pkts++;
> + }
> +
> + pipeline_event_enqueue_burst(dev, port, ev, nb_rx);
> + }
> +
> + return 0;
> +}
> +
> +static int
> +pipeline_atq_worker_multi_stage_tx(void *arg)
> +{
> + PIPELINE_WROKER_MULTI_STAGE_INIT;
> + const uint8_t nb_stages = t->opt->nb_stages;
> +
> +
> + while (t->done == false) {
> + uint16_t event = rte_event_dequeue_burst(dev, port, &ev, 1, 0);
> +
> + if (!event) {
> + rte_pause();
> + continue;
> + }
> +
> + cq_id = ev.sub_event_type % nb_stages;
> +
> + if (cq_id == last_queue) {
> + if (ev.sched_type == RTE_SCHED_TYPE_ATOMIC) {
> +
> + pipeline_t

Re: [dpdk-dev] [PATCH v2] eal: add function to return number of detected sockets

[Thomas Monjalon]

16/01/2018 12:56, Burakov, Anatoly:
> On 12-Jan-18 11:50 AM, Thomas Monjalon wrote:
> > 12/01/2018 12:44, Burakov, Anatoly:
> >> On 11-Jan-18 10:20 PM, Thomas Monjalon wrote:
> >>> 22/12/2017 13:41, Anatoly Burakov:
>  During lcore scan, find maximum socket ID and store it.
> 
>  Signed-off-by: Anatoly Burakov 
>  ---
>  --- a/lib/librte_eal/common/include/rte_eal.h
>  +++ b/lib/librte_eal/common/include/rte_eal.h
>  @@ -83,6 +83,7 @@ enum rte_proc_type_t {
> struct rte_config {
>   uint32_t master_lcore;   /**< Id of the master lcore */
>   uint32_t lcore_count;/**< Number of available logical 
>  cores. */
>  +uint32_t numa_node_count;/**< Number of detected NUMA 
>  nodes. */
>   uint32_t service_lcore_count;/**< Number of available service 
>  cores. */
>   enum rte_lcore_role_t lcore_role[RTE_MAX_LCORE]; /**< State of 
>  cores. */
> >>>
> >>> isn't it breaking the ABI?
> >>>
> >>>
> >>
> >> Yep, you're right, forgot to add that. I didn't expect this to get
> >> merged in 18.02 anyway, so v2 will follow.
> > 
> > Please write 18.05 in the subject to show your expectation.
> > Thanks
> > 
> 
> Does it have to be an ABI change though? We can put numa_node_count 
> after pointer to mem_config, in which case it won't be an ABI break. 
> Would that be better?

Changing the size of a struct which is allocated by the app,
is an ABI break.
Is your solution changing the size?

Re: [dpdk-dev] [PATCH] vhost: support UDP Fragmentation Offload

[Olivier Matz]

On Tue, Jan 16, 2018 at 12:20:48PM +0100, Thomas Monjalon wrote:
> 08/01/2018 15:27, Yuanhan Liu:
> > On Tue, Nov 21, 2017 at 02:56:52PM +0800, Jiayu Hu wrote:
> > > In virtio, UDP Fragmentation Offload (UFO) includes two parts: host UFO
> > > and guest UFO. Guest UFO means the frontend can receive large UDP packets,
> > > and host UFO means the backend can receive large UDP packets. This patch
> > > supports host UFO and guest UFO for vhost-user.
> > > 
> > > Signed-off-by: Jiayu Hu 
> > 
> > Applied to dpdk-next-virtio.
> 
> Olivier Matz, mbuf maintainer, was not Cc'ed in this patch.
> Olivier, can you confirm this new mbuf flag is OK?

Yuanhan CC'ed me, but I was quite busy at that time.

Even if it's to late:
Acked-by: Olivier Matz 

Re: [dpdk-dev] [PATCH v3 3/8] net/failsafe: support probed sub-devices getting

[Matan Azrad]

Hi Gaetan

From: Gaëtan Rivet, Tuesday, January 16, 2018 1:09 PM
> Hi Matan,
> 
> I'n not fond of the commit title, how about:
> 
> [PATCH v3 3/8] net/failsafe: add probed etherdev capture
> 
> ?
> 
OK, no problem.

> On Tue, Jan 09, 2018 at 02:47:28PM +, Matan Azrad wrote:
> > Previous fail-safe code didn't support getting probed sub-devices and
> > failed when it tried to probe them.
> >
> > Skip fail-safe sub-device probing when it already was probed.
> >
> > Signed-off-by: Matan Azrad 
> > Cc: Gaetan Rivet 
> > ---
> >  doc/guides/nics/fail_safe.rst   |  5 
> >  drivers/net/failsafe/failsafe_eal.c | 60
> > -
> >  2 files changed, 45 insertions(+), 20 deletions(-)
> >
> > diff --git a/doc/guides/nics/fail_safe.rst
> > b/doc/guides/nics/fail_safe.rst index 5b1b47e..b89e53b 100644
> > --- a/doc/guides/nics/fail_safe.rst
> > +++ b/doc/guides/nics/fail_safe.rst
> > @@ -115,6 +115,11 @@ Fail-safe command line parameters
> >order to take only the last line into account (unlike ``exec()``) at 
> > every
> >probe attempt.
> >
> > +.. note::
> > +
> > +   In case of whitelist sub-device probed by EAL, fail-safe PMD will take 
> > the
> device
> > +   as is, which means that EAL device options are taken in this case.
> > +
> >  - **mac** parameter [MAC address]
> >
> >This parameter allows the user to set a default MAC address to the
> > fail-safe diff --git a/drivers/net/failsafe/failsafe_eal.c
> > b/drivers/net/failsafe/failsafe_eal.c
> > index 19d26f5..7bc7453 100644
> > --- a/drivers/net/failsafe/failsafe_eal.c
> > +++ b/drivers/net/failsafe/failsafe_eal.c
> > @@ -36,39 +36,59 @@
> >  #include "failsafe_private.h"
> >
> >  static int
> > +fs_get_port_by_device_name(const char *name, uint16_t *port_id)
> 
> The naming convention for the failsafe driver is
> 
>   namespace_object_sub-object_action()
> 
OK.
> With an ordering of objects by their scope (std, rte, failsafe, file).
> Also, "get" as an action is not descriptive enough.
> 
Isn't "get by device name" descriptive? 
> static int
> fs_ethdev_capture(const char *name, uint16_t *port_id);
> 
You miss here the main reason why we need this function instead of using 
rte_eth_dev_get_port_by_name.
The reason we need this function is because we want to find the device by the 
device name and not ethdev name.
What's about  fs_port_capture_by_device_name?

Maybe comparing it to device->devargs->name is better, What do you think?

> > +{
> > +   uint16_t pid;
> > +   size_t len;
> > +
> > +   if (name == NULL) {
> > +   DEBUG("Null pointer is specified\n");
> > +   return -EINVAL;
> > +   }
> > +   len = strlen(name);
> > +   RTE_ETH_FOREACH_DEV(pid) {
> > +   if (!strncmp(name, rte_eth_devices[pid].device->name,
> len)) {
> > +   *port_id = pid;
> > +   return 0;
> > +   }
> > +   }
> > +   return -ENODEV;
> > +}
> > +
> > +static int
> >  fs_bus_init(struct rte_eth_dev *dev)
> >  {
> > struct sub_device *sdev;
> > struct rte_devargs *da;
> > uint8_t i;
> > -   uint16_t j;
> > +   uint16_t pid;
> > int ret;
> >
> > FOREACH_SUBDEV(sdev, i, dev) {
> > if (sdev->state != DEV_PARSED)
> > continue;
> > da = &sdev->devargs;
> > -   ret = rte_eal_hotplug_add(da->bus->name,
> > - da->name,
> > - da->args);
> > -   if (ret) {
> > -   ERROR("sub_device %d probe failed %s%s%s", i,
> > - rte_errno ? "(" : "",
> > - rte_errno ? strerror(rte_errno) : "",
> > - rte_errno ? ")" : "");
> > -   continue;
> > -   }
> > -   RTE_ETH_FOREACH_DEV(j) {
> > -   if (strcmp(rte_eth_devices[j].device->name,
> > -   da->name) == 0) {
> > -   ETH(sdev) = &rte_eth_devices[j];
> > -   break;
> > +   if (fs_get_port_by_device_name(da->name, &pid) != 0) {
> > +   ret = rte_eal_hotplug_add(da->bus->name,
> > + da->name,
> > + da->args);
> > +   if (ret) {
> > +   ERROR("sub_device %d probe failed
> %s%s%s", i,
> > + rte_errno ? "(" : "",
> > + rte_errno ? strerror(rte_errno) : "",
> > + rte_errno ? ")" : "");
> > +   continue;
> > }
> > +   if (fs_get_port_by_device_name(da->name, &pid)
> != 0) {
> > +   ERROR("sub_device %d init went wrong", i);
> > +   return -ENODEV;
> > +   }
> > +   } else {
> > +   /* Take control of dev

Re: [dpdk-dev] [PATCH 1/2] lib/cryptodev: add support to set session private data

[Gujjar, Abhinandan S]

Hi Akhil,

> -Original Message-
> From: Akhil Goyal [mailto:akhil.go...@nxp.com]
> Sent: Tuesday, January 16, 2018 5:30 PM
> To: Gujjar, Abhinandan S ; Doherty, Declan
> ; Jacob, Jerin
> 
> Cc: dev@dpdk.org; Vangati, Narender ; Rao,
> Nikhil 
> Subject: Re: [PATCH 1/2] lib/cryptodev: add support to set session private 
> data
> 
> Hi Abhinandan,
> On 1/16/2018 5:06 PM, Gujjar, Abhinandan S wrote:
> > Hi Akhil,
> >
> >> -Original Message-
> >> From: Akhil Goyal [mailto:akhil.go...@nxp.com]
> >> Sent: Tuesday, January 16, 2018 2:52 PM
> >> To: Gujjar, Abhinandan S ; Doherty,
> >> Declan ; Jacob, Jerin
> >> 
> >> Cc: dev@dpdk.org; Vangati, Narender ;
> >> Rao, Nikhil 
> >> Subject: Re: [PATCH 1/2] lib/cryptodev: add support to set session
> >> private data
> >>
> >> On 1/16/2018 2:33 PM, Gujjar, Abhinandan S wrote:
> >>> Hi Akhil,
> >>>
>  -Original Message-
>  From: Akhil Goyal [mailto:akhil.go...@nxp.com]
>  Sent: Tuesday, January 16, 2018 12:56 PM
>  To: Gujjar, Abhinandan S ; Doherty,
>  Declan ; Jacob, Jerin
>  
>  Cc: dev@dpdk.org; Vangati, Narender ;
>  Rao, Nikhil 
>  Subject: Re: [PATCH 1/2] lib/cryptodev: add support to set session
>  private data
> 
>  Hi Abhinandan,
>  On 1/16/2018 12:35 PM, Gujjar, Abhinandan S wrote:
> > Hi Akhil,
> >
> >> -Original Message-
> >> From: Akhil Goyal [mailto:akhil.go...@nxp.com]
> >> Sent: Tuesday, January 16, 2018 11:55 AM
> >> To: Gujjar, Abhinandan S ; Doherty,
> >> Declan 
> >> Cc: dev@dpdk.org; Vangati, Narender ;
> >> Rao, Nikhil 
> >> Subject: Re: [PATCH 1/2] lib/cryptodev: add support to set
> >> session private data
> >>
> >> Hi Abhinandan,
> >> On 1/16/2018 11:39 AM, Gujjar, Abhinandan S wrote:
> > diff --git a/lib/librte_cryptodev/rte_crypto.h
> > b/lib/librte_cryptodev/rte_crypto.h
> > index bbc510d..3a98cbf 100644
> > --- a/lib/librte_cryptodev/rte_crypto.h
> > +++ b/lib/librte_cryptodev/rte_crypto.h
> > @@ -62,6 +62,18 @@ enum rte_crypto_op_sess_type {
> > RTE_CRYPTO_OP_SECURITY_SESSION  /**< Security session
>  crypto
>  operation */
> >   };
> >
> > +/** Private data types for cryptographic operation
> > + * @see rte_crypto_op::private_data_type */ enum
> > +rte_crypto_op_private_data_type {
> > +   RTE_CRYPTO_OP_PRIVATE_DATA_NONE,
> > +   /**< No private data */
> > +   RTE_CRYPTO_OP_PRIVATE_DATA_OP,
> > +   /**< Private data is part of rte_crypto_op and indicated by
> > +* private_data_offset */
> > +   RTE_CRYPTO_OP_PRIVATE_DATA_SESSION
> > +   /**< Private data is available at session */ };
> > +
>  We may get away with this enum. If private_data_offset is "0",
>  then we can check with the session if it has priv data or not.
> >>> Right now,  Application uses 'rte_crypto_op_private_data_type'
> >>> to indicate rte_cryptodev_sym_session_set_private_data()
> >>> was called to set the private data. Otherwise, how do you
> >>> indicate there is a
> >> private data associated with the session?
> >>> Any suggestions?
> >> For session based flows, the first choice to store the private
> >> data should be in the session. So RTE_CRYPTO_OP_WITH_SESSION or
> >> RTE_CRYPTO_OP_SECURITY_SESSION can be used to call
> >> rte_cryptodev_sym_session_set_private_data or
> >> rte_security_session_set_private_data.
> > Case 1: private_data_offset is "0" and sess_type =
> > RTE_CRYPTO_OP_WITH_SESSION -> usual case Case 2:
> > private_data_offset is "0" and sess_type =
> > RTE_CRYPTO_OP_WITH_SESSION + event case (access private data)
> > Differentiating between case 1 & 2 will be done by checking
>  rte_crypto_op_private_data_type ==
>  RTE_CRYPTO_OP_PRIVATE_DATA_SESSION.
> 
>  Consider this:
>  if (sess_type == RTE_CRYPTO_OP_WITH_SESSION &&
>   rte_cryptodev_sym_session_get_private_data == NULL)
>   usual case.
>  else if (sess_type = RTE_CRYPTO_OP_WITH_SESSION &&
>   rte_cryptodev_sym_session_get_private_data != NULL)
>   event case.
>  else if (sess_type == RTE_CRYPTO_OP_SESSIONLESS &&
>   private_data_offset != 0)
>   event case for sessionless op.
> 
>  I hope all cases can be handled in this way.
> >>> Memory allocated for private data will be continuation of session memory.
> >>> I think, rte_cryptodev_sym_session_get_private_data() will return a
> >>> valid
> >> pointer.
> >>> It could be pointer to private data, in case application has
> >>> allocated mempool
> >> with session + private data.
> >>> It could be again a pointer to a location(may be next session),  in
> >>> case
> >> application has allocated mempool with session only.
> >>> Unless, there is a flag in the ses

Re: [dpdk-dev] [PATCH v4 10/13] app/eventdev: add pipeline queue worker functions

[Pavan Nikhilesh]

Hi Harry,

Thanks for the review.

On Tue, Jan 16, 2018 at 12:05:48PM +, Van Haaren, Harry wrote:
> > From: Pavan Nikhilesh [mailto:pbhagavat...@caviumnetworks.com]
> > Sent: Friday, January 12, 2018 4:44 PM
> > To: jerin.ja...@caviumnetworks.com; santosh.shu...@caviumnetworks.com; Van
> > Haaren, Harry ; Eads, Gage
> > ; hemant.agra...@nxp.com; nipun.gu...@nxp.com; Ma,
> > Liang J 
> > Cc: dev@dpdk.org; Pavan Nikhilesh 
> > Subject: [dpdk-dev] [PATCH v4 10/13] app/eventdev: add pipeline queue worker
> > functions
> >
> > Signed-off-by: Pavan Nikhilesh 
>
> Some of the code feels like duplication - but there are differences in each 
> implementation as far as I can see. The common initialization parts are 
> "macro-ed" to ensure minimal duplication. In short, I'm not aware of a better 
> solution/implementation.
>
> Please see note on branch-optimization in the code below.
>
> Acked-by: Harry van Haaren 
>
>
> >  static int
> >  worker_wrapper(void *arg)
> >  {
> > -   RTE_SET_USED(arg);
> > +   struct worker_data *w  = arg;
> > +   struct evt_options *opt = w->t->opt;
> > +   const bool burst = evt_has_burst_mode(w->dev_id);
> > +   const bool mt_safe = !w->t->mt_unsafe;
> > +   const uint8_t nb_stages = opt->nb_stages;
> > +   RTE_SET_USED(opt);
> > +
> > +   /* allow compiler to optimize */
> > +   if (nb_stages == 1) {
> > +   if (!burst && mt_safe)
> > +   return pipeline_queue_worker_single_stage_tx(arg);
> > +   else if (!burst && !mt_safe)
> > +   return pipeline_queue_worker_single_stage_fwd(arg);
> > +   else if (burst && mt_safe)
> > +   return pipeline_queue_worker_single_stage_burst_tx(arg);
> > +   else if (burst && !mt_safe)
> > +   return pipeline_queue_worker_single_stage_burst_fwd(
> > +   arg);
> > +   } else {
> > +   if (!burst && mt_safe)
> > +   return pipeline_queue_worker_multi_stage_tx(arg);
> > +   else if (!burst && !mt_safe)
> > +   return pipeline_queue_worker_multi_stage_fwd(arg);
> > +   else if (burst && mt_safe)
> > +   return pipeline_queue_worker_multi_stage_burst_tx(arg);
> > +   else if (burst && !mt_safe)
> > +   return pipeline_queue_worker_multi_stage_burst_fwd(arg);
> > +
> > +   }
>
>
> I don't think the compiler will optimize the below, as the nb_stages value 
> comes from opt, which is a uint8 originating from a pointer-dereference...
>
> As far as I know, an optimizing compiler will do "constant propagation" 
> through function calls, but I would be amazed if it found a pointer 
> initialized to a specific value, and optimized the branches away here based 
> on the contents of that pointer deref.
>
> So I see 2 options;
> 1) accept as is - I'll ack this patch here
> 2) Rework so that the worker_wrapper() accepts nb_stages, burst and mt_safe 
> as function arguments, allowing constant propagation for optimizing away the 
> branches.
>
> @Pavan, I'll leave that choice up to you!

As the worker functions are not affected, I will keep the conditions as is and
remove the comment for now.

Cheers,
Pavan.
>

Re: [dpdk-dev] [PATCH v5] app/testpmd: add option ring-bind-lcpu to bind Q with CPU

[Ananyev, Konstantin]

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of wei.guo.si...@gmail.com
> Sent: Saturday, January 13, 2018 2:35 AM
> To: Lu, Wenzhuo 
> Cc: dev@dpdk.org; Thomas Monjalon ; Simon Guo 
> 
> Subject: [dpdk-dev] [PATCH v5] app/testpmd: add option ring-bind-lcpu to bind 
> Q with CPU
> 
> From: Simon Guo 
> 
> Currently the rx/tx queue is allocated from the buffer pool on socket of:
> - port's socket if --port-numa-config specified
> - or ring-numa-config setting per port
> 
> All the above will "bind" queue to single socket per port configuration.
> But it can actually archieve better performance if one port's queue can
> be spread across multiple NUMA nodes, and the rx/tx queue is allocated
> per lcpu socket.
> 
> This patch adds a new option "--ring-bind-lcpu"(no parameter).  With
> this, testpmd can utilize the PCI-e bus bandwidth on another NUMA
> nodes.
> 
> When --port-numa-config or --ring-numa-config option is specified, this
> --ring-bind-lcpu option will be suppressed.

Instead of introducing one more option - wouldn't it be better to 
allow user manually to define flows and assign them to particular lcores?
Then the user will be able to create any FWD configuration he/she likes.
Something like:
lcore X add flow rxq N,Y txq M,Z

Which would mean - on lcore X recv packets from port=N, rx_queue=Y,
and send them through port=M,tx_queue=Z.

Konstantin
 



> 
> Test result:
> 64bytes package, running in PowerPC with Mellanox
> CX-4 card, single port(100G), with 8 cores, fw mode:
> - Without this patch:  52.5Mpps throughput
> - With this patch: 66Mpps throughput
>   ~25% improvement
> 
> Signed-off-by: Simon Guo 
> ---
>  app/test-pmd/parameters.c |  14 -
>  app/test-pmd/testpmd.c| 112 
> --
>  app/test-pmd/testpmd.h|   7 +++
>  doc/guides/testpmd_app_ug/run_app.rst |   6 ++
>  4 files changed, 107 insertions(+), 32 deletions(-)
> 
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> index 304b98d..1dba92e 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -104,6 +104,10 @@
>  "(flag: 1 for RX; 2 for TX; 3 for RX and TX).\n");
>   printf("  --socket-num=N: set socket from which all memory is allocated 
> "
>  "in NUMA mode.\n");
> + printf("  --ring-bind-lcpu: "
> + "specify TX/RX rings will be allocated on local socket of lcpu."
> + "It will be ignored if ring-numa-config or port-numa-config is 
> used. "
> + "As a result, it allows one port binds to multiple NUMA 
> nodes.\n");
>   printf("  --mbuf-size=N: set the data size of mbuf to N bytes.\n");
>   printf("  --total-num-mbufs=N: set the number of mbufs to be allocated "
>  "in mbuf pools.\n");
> @@ -544,6 +548,7 @@
>   { "interactive",0, 0, 0 },
>   { "cmdline-file",   1, 0, 0 },
>   { "auto-start", 0, 0, 0 },
> + { "ring-bind-lcpu", 0, 0, 0 },
>   { "eth-peers-configfile",   1, 0, 0 },
>   { "eth-peer",   1, 0, 0 },
>  #endif
> @@ -676,6 +681,10 @@
>   stats_period = n;
>   break;
>   }
> + if (!strcmp(lgopts[opt_idx].name, "ring-bind-lcpu")) {
> + ring_bind_lcpu |= RBL_BIND_LOCAL_MASK;
> + break;
> + }
>   if (!strcmp(lgopts[opt_idx].name,
>   "eth-peers-configfile")) {
>   if (init_peer_eth_addrs(optarg) != 0)
> @@ -739,11 +748,14 @@
>   if (parse_portnuma_config(optarg))
>   rte_exit(EXIT_FAILURE,
>  "invalid port-numa configuration\n");
> + ring_bind_lcpu |= RBL_PORT_NUMA_MASK;
>   }
> - if (!strcmp(lgopts[opt_idx].name, "ring-numa-config"))
> + if (!strcmp(lgopts[opt_idx].name, "ring-numa-config")) {
>   if (parse_ringnuma_config(optarg))
>   rte_exit(EXIT_FAILURE,
>  "invalid ring-numa configuration\n");
> + ring_bind_lcpu |= RBL_RING_NUMA_MASK;
> + }
>   if (!strcmp(lgopts[opt_idx].name, "socket-num")) {
>   n = atoi(optarg);
>   if (!new_socket_id((uint8_t)n)) {
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 9414d0e..e9e89d0 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -68,6 +68,9 @@
>  uint8_t interactive = 0;
>  uint8_t au

Re: [dpdk-dev] [PATCH v2] lib/cmdline: init CLI parsing memory

[Olivier Matz]

Hi Xueming,

Sorry for the late response.

On Tue, Dec 26, 2017 at 12:57:41PM +, Xueming(Steven) Li wrote:
> HI Olivier,
> 
> By reading p1 comments carefully, looks like the pointer to result buffer 
> issue
> not resolved by result copy. How about this:
> 
> @@ -263,6 +263,7 @@
>  #ifdef RTE_LIBRTE_CMDLINE_DEBUG
>   char debug_buf[BUFSIZ];
>  #endif
> + char *result_buf = result.buf;
>  
>   if (!cl || !buf)
>   return CMDLINE_PARSE_BAD_ARGS;
> @@ -312,16 +313,13 @@
>   debug_printf("INST %d\n", inst_num);
>  
>   /* fully parsed */
> - tok = match_inst(inst, buf, 0, tmp_result.buf,
> -  sizeof(tmp_result.buf));
> + tok = match_inst(inst, buf, 0, result_buf, sizeof(result.buf));

If we don't use tmp_result, it is maybe better to also replace
sizeof(result.buf) by CMDLINE_PARSE_RESULT_BUFSIZE

>  
>   if (tok > 0) /* we matched at least one token */
>   err = CMDLINE_PARSE_BAD_ARGS;
>  
>   else if (!tok) {
>   debug_printf("INST fully parsed\n");
> - memcpy(&result, &tmp_result,
> -sizeof(result));
>   /* skip spaces */
>   while (isblank2(*curbuf)) {
>   curbuf++;
> @@ -332,6 +330,7 @@
>   if (!f) {
>   memcpy(&f, &inst->f, sizeof(f));
>   memcpy(&data, &inst->data, 
> sizeof(data));
> + result_buf = tmp_result.buf;
>   }
>   else {
>   /* more than 1 inst matches */
> 


I guess the issue you are talking about is the one described in
"cmdline: fix dynamic tokens parsing" in my previous description?

I think this patch is ok, and is probably better than the initial
suggestion, because it avoids to copy the buffer. However, I don't
understand why the previous patch was wrong, can you detail?

Thanks
Olivier

Re: [dpdk-dev] [PATCH 1/2] lib/cryptodev: add support to set session private data

[Akhil Goyal]

On 1/16/2018 5:59 PM, Gujjar, Abhinandan S wrote:

Hi Akhil,


-Original Message-
From: Akhil Goyal [mailto:akhil.go...@nxp.com]
Sent: Tuesday, January 16, 2018 5:30 PM
To: Gujjar, Abhinandan S ; Doherty, Declan
; Jacob, Jerin

Cc: dev@dpdk.org; Vangati, Narender ; Rao,
Nikhil 
Subject: Re: [PATCH 1/2] lib/cryptodev: add support to set session private data

Hi Abhinandan,
On 1/16/2018 5:06 PM, Gujjar, Abhinandan S wrote:

Hi Akhil,


-Original Message-
From: Akhil Goyal [mailto:akhil.go...@nxp.com]
Sent: Tuesday, January 16, 2018 2:52 PM
To: Gujjar, Abhinandan S ; Doherty,
Declan ; Jacob, Jerin

Cc: dev@dpdk.org; Vangati, Narender ;
Rao, Nikhil 
Subject: Re: [PATCH 1/2] lib/cryptodev: add support to set session
private data

On 1/16/2018 2:33 PM, Gujjar, Abhinandan S wrote:

Hi Akhil,


-Original Message-
From: Akhil Goyal [mailto:akhil.go...@nxp.com]
Sent: Tuesday, January 16, 2018 12:56 PM
To: Gujjar, Abhinandan S ; Doherty,
Declan ; Jacob, Jerin

Cc: dev@dpdk.org; Vangati, Narender ;
Rao, Nikhil 
Subject: Re: [PATCH 1/2] lib/cryptodev: add support to set session
private data

Hi Abhinandan,
On 1/16/2018 12:35 PM, Gujjar, Abhinandan S wrote:

Hi Akhil,


-Original Message-
From: Akhil Goyal [mailto:akhil.go...@nxp.com]
Sent: Tuesday, January 16, 2018 11:55 AM
To: Gujjar, Abhinandan S ; Doherty,
Declan 
Cc: dev@dpdk.org; Vangati, Narender ;
Rao, Nikhil 
Subject: Re: [PATCH 1/2] lib/cryptodev: add support to set
session private data

Hi Abhinandan,
On 1/16/2018 11:39 AM, Gujjar, Abhinandan S wrote:

diff --git a/lib/librte_cryptodev/rte_crypto.h
b/lib/librte_cryptodev/rte_crypto.h
index bbc510d..3a98cbf 100644
--- a/lib/librte_cryptodev/rte_crypto.h
+++ b/lib/librte_cryptodev/rte_crypto.h
@@ -62,6 +62,18 @@ enum rte_crypto_op_sess_type {
RTE_CRYPTO_OP_SECURITY_SESSION  /**< Security session

crypto

operation */

   };

+/** Private data types for cryptographic operation
+ * @see rte_crypto_op::private_data_type */ enum
+rte_crypto_op_private_data_type {
+   RTE_CRYPTO_OP_PRIVATE_DATA_NONE,
+   /**< No private data */
+   RTE_CRYPTO_OP_PRIVATE_DATA_OP,
+   /**< Private data is part of rte_crypto_op and indicated by
+* private_data_offset */
+   RTE_CRYPTO_OP_PRIVATE_DATA_SESSION
+   /**< Private data is available at session */ };
+

We may get away with this enum. If private_data_offset is "0",
then we can check with the session if it has priv data or not.

Right now,  Application uses 'rte_crypto_op_private_data_type'
to indicate rte_cryptodev_sym_session_set_private_data()
was called to set the private data. Otherwise, how do you
indicate there is a

private data associated with the session?

Any suggestions?

For session based flows, the first choice to store the private
data should be in the session. So RTE_CRYPTO_OP_WITH_SESSION or
RTE_CRYPTO_OP_SECURITY_SESSION can be used to call
rte_cryptodev_sym_session_set_private_data or
rte_security_session_set_private_data.

Case 1: private_data_offset is "0" and sess_type =
RTE_CRYPTO_OP_WITH_SESSION -> usual case Case 2:
private_data_offset is "0" and sess_type =
RTE_CRYPTO_OP_WITH_SESSION + event case (access private data)
Differentiating between case 1 & 2 will be done by checking

rte_crypto_op_private_data_type ==
RTE_CRYPTO_OP_PRIVATE_DATA_SESSION.

Consider this:
if (sess_type == RTE_CRYPTO_OP_WITH_SESSION &&
rte_cryptodev_sym_session_get_private_data == NULL)
usual case.
else if (sess_type = RTE_CRYPTO_OP_WITH_SESSION &&
rte_cryptodev_sym_session_get_private_data != NULL)
event case.
else if (sess_type == RTE_CRYPTO_OP_SESSIONLESS &&
private_data_offset != 0)
event case for sessionless op.

I hope all cases can be handled in this way.

Memory allocated for private data will be continuation of session memory.
I think, rte_cryptodev_sym_session_get_private_data() will return a
valid

pointer.

It could be pointer to private data, in case application has
allocated mempool

with session + private data.

It could be again a pointer to a location(may be next session),  in
case

application has allocated mempool with session only.

Unless, there is a flag in the session data which will be set by
rte_cryptodev_sym_session_set_private_data()
If this flag is not set,
rte_cryptodev_sym_session_get_private_data() will

return NULL.

I am not claiming, I have complete knowledge of different usage case
of

mempool setup for crypto.

I am wondering, whether I am missing anything here. Please let me know.


It depends on the implementation of the get/set API. We can set NULL,
if it is not filled in the set API. If it is set then we have a valid pointer.

The plan is to store private data after "sess * nb_drivers ".
As you said, if it is implementation specific, flag may be again
required at struct rte_cryptodev_sym_session

I think my previous statement was not clear.
My point is that whatever we set in the
rte_cryptodev

Re: [dpdk-dev] [PATCH v1 6/6] net: fix rte_ether conflicts with libc

[Olivier Matz]

Hi Adrien,

On Fri, Dec 22, 2017 at 03:25:27PM +0100, Adrien Mazarguil wrote:
> On Fri, Dec 22, 2017 at 02:34:21PM +0100, Olivier MATZ wrote:
> > On Thu, Dec 21, 2017 at 02:00:06PM +0100, Adrien Mazarguil wrote:

...

> > > +#if defined(__FreeBSD__)
> > > +#define addr_bytes octet
> > > +#else
> > > +#define addr_bytes ether_addr_octet
> > > +#endif
> > > +
> > >  #define ETHER_LOCAL_ADMIN_ADDR 0x02 /**< Locally assigned Eth. address. 
> > > */
> > >  #define ETHER_GROUP_ADDR   0x01 /**< Multicast or broadcast Eth. 
> > > address. */
> > 
> > This kind of #define looks a bit dangerous to me: it can trigger
> > strange bugs because it will replace all occurences of addr_bytes
> > after this header is included.
> 
> Understandable, I checked before settling on this macro though, there's no
> other usage of addr_bytes inside DPDK.
> 
> As for applications, there's no way to be completely sure. If we consider
> they have to explicitly include rte_ether.h to get this definition, there
> are chances addr_bytes is exclusively used with MAC addresses.
> 
> This change results in an API change (addr_bytes now documented as a
> reserved macro) but has no ABI impact. I think it's a rather harmless
> workaround pending your next suggestion:
> 
> > Wouldn't it be a good opportunity to think about adding the rte_ prefix
> > to all variables/functions of rte_ether.h?
> 
> That would be ideal, however since almost all definitions in librte_net
> (rte_ip.h, rte_udp.h etc.) also lack this prefix and can still technically
> trigger conflicts with outside definitions (I'm aware work was done to avoid
> that, but it doesn't change the fact they're not in the official DPDK
> namespace), a massive API change would be needed for consistency.
> 
> Such a change is unlikely to be accepted for 18.02 in any case, therefore if
> the proposed workaround is deemed too risky, I offer to remove this patch
> from the series. What do you suggest?

I think the only good long term solution is to have a proper namespace
for all rte_net structures and definitions. If there is no blocking issue
requiring this patch now, we can consider the following:

- 18.02: announce an api change for 18.05
- 18.05:
  - add new net structures and definitions with rte_ prefix
  - mark the old ones as deprecated
  - removes the structures or definitions that conflicts with system headers
- 18.08: remove the old structures and definitions

[dpdk-dev] [PATCH v11 0/5] net/virtio: support GUEST ANNOUNCE

[Xiao Wang]

When live migration is finished, the backup VM needs to proactively announce
its new location. DPDK vhost has implemented VHOST_USER_PROTOCOL_F_RARP to
generate a RARP packet to switch in dequeue path. Another method is to let
the guest proactively send out RARP packet using VIRTIO_NET_F_GUEST_ANNOUNCE
feature.

This patch set enables this feature in virtio pmd, to support VM running virtio
pmd be migrated without vhost supporting RARP generation.

v11:
- Add check for parameter and tailroom in rte_net_make_rarp_packet.
- Allocate mbuf in rte_net_make_rarp_packet.

v10:
- Add a bold doxygen comment for the experimental function.

v9:
- Introduce function with the experimental state.

v8:
- Add a helper in lib/librte_net to make rarp packet, it's used by
  both vhost and virtio.

v7:
- Improve comment for state_lock.
- Rename spinlock variable 'sl' to 'lock'.

v6:
- Use rte_pktmbuf_alloc() instead of rte_mbuf_raw_alloc().
- Remove the 'len' parameter in calling virtio_send_command().
- Remove extra space between typo and var.
- Improve comment and alignment.
- Remove the unnecessary header file.
- A better usage of 'unlikely' indication.

v5:
- Remove txvq parameter in virtio_inject_pkts.
- Zero hw->special_buf after using it.
- Return the retval of tx_pkt_burst().
- Allocate a mbuf pointer on stack directly.

v4:
- Move spinlock lock/unlock into dev_pause/resume.
- Separate out a patch for packet injection.

v3:
- Remove Tx function code duplication, use a special pointer for rarp
  injection.
- Rename function generate_rarp to virtio_notify_peers, replace
  'virtnet_' with 'virtio_'.
- Add comment for state_lock.
- Typo fix and comment improvement.

v2:
- Use spaces instead of tabs between the code and comments.
- Remove unnecessary parentheses.
- Use rte_pktmbuf_mtod directly to get eth_hdr addr.
- Fix virtio_dev_pause return value check.

Xiao Wang (5):
  net/virtio: make control queue thread-safe
  net/virtio: add packet injection method
  net: add a helper for making RARP packet
  vhost: use lib API to make RARP packet
  net/virtio: support GUEST ANNOUNCE

 drivers/net/virtio/virtio_ethdev.c  | 113 +++-
 drivers/net/virtio/virtio_ethdev.h  |   6 ++
 drivers/net/virtio/virtio_pci.h |   7 ++
 drivers/net/virtio/virtio_rxtx.c|   3 +-
 drivers/net/virtio/virtio_rxtx.h|   1 +
 drivers/net/virtio/virtio_rxtx_simple.c |   2 +-
 drivers/net/virtio/virtqueue.h  |  11 
 lib/Makefile|   3 +-
 lib/librte_net/Makefile |   1 +
 lib/librte_net/rte_arp.c|  50 ++
 lib/librte_net/rte_arp.h|  18 +
 lib/librte_net/rte_net_version.map  |   6 ++
 lib/librte_vhost/Makefile   |   2 +-
 lib/librte_vhost/virtio_net.c   |  51 +-
 14 files changed, 219 insertions(+), 55 deletions(-)
 create mode 100644 lib/librte_net/rte_arp.c

-- 
2.15.1