Re: [dpdk-dev] [PATCH v2] ethdev: introduce lock-free txq capability flag

[Thomas Monjalon <thomas@monjalon.net>]

Hi Jerin,

Thanks for the update.
I think we can add this new flag in 17.08.
I prefer waiting John's review, especially for doc wording,
before applying it. I consider it does not hurt to add it post-rc1.

See below for my first comment on the doc.

06/07/2017 08:21, Jerin Jacob:
> Introducing the DEV_TX_OFFLOAD_MT_LOCKFREE TX capability flag.
> if a PMD advertises DEV_TX_OFFLOAD_MT_LOCKFREE capable, multiple threads
> can invoke rte_eth_tx_burst() concurrently on the same tx queue without
> SW lock. This PMD feature will be useful in the following use cases and
> found in the OCTEON family of NPUs.
> 
> 1) Remove explicit spinlock in some applications where lcores
> to TX queues are not mapped 1:1.
> example: OVS has such instance
> https://github.com/openvswitch/ovs/blob/master/lib/netdev-dpdk.c#L299
> https://github.com/openvswitch/ovs/blob/master/lib/netdev-dpdk.c#L1859
> See the the usage of tx_lock spinlock.
> 
> 2) In the eventdev use case, Avoid dedicating a separate TX core for
> transmitting and thus enables more scaling as all workers can
> send the packets.
> 
> v2:
> - Changed the flag name to DEV_TX_OFFLOAD_MT_LOCKFREE(Thomas)
> - Updated the documentation in doc/guides/prog_guide/poll_mode_drv.rst
> and rte_eth_tx_burst() doxgen comments(Thomas)
> 
> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> ---
[...]
> +If the PMD is ``DEV_TX_OFFLOAD_MT_LOCKFREE`` capable, multiple threads can invoke ``rte_eth_tx_burst()``
> +concurrently on the same tx queue without SW lock.This PMD feature found in some NICs and

A space is missing after the dot.
Note: my preference is to start next sentence on a new line (in RST source).

> +useful in the following use cases if PMD supports it. See `Hardware Offload`_ for details.

This sentence is confusing. I would remove "if PMD supports it".
After "following use cases", should we add a colon?
The relation with `Hardware Offload`_ is not obvious.

> +*  Remove explicit spinlock in some applications where lcores to TX queues are not mapped 1:1.

Can we reword "lcores to TX queues"?
I suggest "lcores are not mapped to Tx queues with 1:1 relation".

> +*  In the eventdev use case, Avoid dedicating a separate TX core for transmitting and thus

Uppercase in the middle of the sentence spotted.

> +   enables more scaling as all workers can send the packets.