diff mbox

[dpdk-dev] ethdev: add lock-less txq capability flag

Message ID 20170421122223.24194-1-jerin.jacob@caviumnetworks.com (mailing list archive)
State Changes Requested, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Jerin Jacob April 21, 2017, 12:22 p.m. UTC 
  if this flag is advertised by a PMD, Multiple threads can
invoke rte_eth_tx_burst() concurrently on the same tx queue
without SW lock. This is an HW feature found in some NICs
and useful in the following use cases if HW supports it.

1) Remove explicit spinlock in some application where cores
to tx queues are not mapped 1:1.
example: OVS has such instance
https://github.com/openvswitch/ovs/blob/master/lib/netdev-dpdk.c#L292
https://github.com/openvswitch/ovs/blob/master/lib/netdev-dpdk.c#L1821

2) In eventdev use case, Avoid dedicating a separate tx core
for transmit and thus enables more scaling as all workers
can transmit the packets.

Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
---
 lib/librte_ether/rte_ethdev.h | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Thomas Monjalon April 24, 2017, 12:34 p.m. UTC | #1 
21/04/2017 14:22, Jerin Jacob:
> if this flag is advertised by a PMD, Multiple threads can
> invoke rte_eth_tx_burst() concurrently on the same tx queue
> without SW lock. This is an HW feature found in some NICs
> and useful in the following use cases if HW supports it.

Which hardware supports it?

[...]
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> +#define DEV_TX_OFFLOAD_TXQ_MT_LOCKFREE	0x00004000
> +/**< Multiple threads can invoke rte_eth_tx_burst() concurrently on the
> same + * tx queue without SW lock.
> + */

Why TXQ in the name? DEV_TX_OFFLOAD_MT_LOCKFREE would be enough.
I wonder whether "lock free" wording is confusing because
the locks are probably handled in HW.
I think the good wording is "offloaded multi-thread capability",
maybe with a naming like DEV_TX_OFFLOAD_MT.

Anyway we should reference this flag in rte_eth_tx_burst()
and give more details in doc/guides/prog_guide/poll_mode_drv.rst.

Should we wait a first hardware PoC to add this flag?
Candidate for 17.08?
Jerin Jacob April 27, 2017, 11 a.m. UTC | #2 
-----Original Message-----
> Date: Mon, 24 Apr 2017 14:34:59 +0200
> From: Thomas Monjalon <thomas@monjalon.net>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> Cc: dev@dpdk.org, bruce.richardson@intel.com, harry.van.haaren@intel.com,
>  hemant.agrawal@nxp.com, gage.eads@intel.com, nipun.gupta@nxp.com,
>  santosh.shukla@caviumnetworks.com
> Subject: Re: [dpdk-dev]  [PATCH] ethdev: add lock-less txq capability flag
> 
> 21/04/2017 14:22, Jerin Jacob:
> > if this flag is advertised by a PMD, Multiple threads can
> > invoke rte_eth_tx_burst() concurrently on the same tx queue
> > without SW lock. This is an HW feature found in some NICs
> > and useful in the following use cases if HW supports it.
> 
> Which hardware supports it?

Cavium OCTEONTX Packet transmission HW block(PKO).

> 
> [...]
> > --- a/lib/librte_ether/rte_ethdev.h
> > +++ b/lib/librte_ether/rte_ethdev.h
> > +#define DEV_TX_OFFLOAD_TXQ_MT_LOCKFREE	0x00004000
> > +/**< Multiple threads can invoke rte_eth_tx_burst() concurrently on the
> > same + * tx queue without SW lock.
> > + */
> 
> Why TXQ in the name? DEV_TX_OFFLOAD_MT_LOCKFREE would be enough.

OK

> I wonder whether "lock free" wording is confusing because
> the locks are probably handled in HW.

Yes. Another reason why it possible because HW is not using ring like scheme(head and tail pointers)
for Tx. "lock free" wording is more from software perspective.

> I think the good wording is "offloaded multi-thread capability",
> maybe with a naming like DEV_TX_OFFLOAD_MT.

I think SW lock free is the capability here.IMO, it better to reflect the
capability in the name(DEV_TX_OFFLOAD_MT_LOCKFREE).

> 
> Anyway we should reference this flag in rte_eth_tx_burst()
> and give more details in doc/guides/prog_guide/poll_mode_drv.rst.

OK. Will address in v2.

> 
> Should we wait a first hardware PoC to add this flag?

If we are in agreement for method expose this feature through capability
flag then should we really need to wait for driver implementation to
accept the patch? Thoughts ?
This flag has impact on how we handle the even dev applications if HW support available.
We are in the process of up-streaming OCTEONTX ethdev driver, but there
are few thing like with external pool manager and eventdev integration
needs to be sorted out cleanly before that.

> Candidate for 17.08?
Thomas Monjalon July 5, 2017, 5:46 p.m. UTC | #3 
27/04/2017 13:00, Jerin Jacob:
> -----Original Message-----
> > Date: Mon, 24 Apr 2017 14:34:59 +0200
> > From: Thomas Monjalon <thomas@monjalon.net>
> > To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > Cc: dev@dpdk.org, bruce.richardson@intel.com, harry.van.haaren@intel.com,
> >  hemant.agrawal@nxp.com, gage.eads@intel.com, nipun.gupta@nxp.com,
> >  santosh.shukla@caviumnetworks.com
> > Subject: Re: [dpdk-dev]  [PATCH] ethdev: add lock-less txq capability flag
> > 
> > 21/04/2017 14:22, Jerin Jacob:
> > > if this flag is advertised by a PMD, Multiple threads can
> > > invoke rte_eth_tx_burst() concurrently on the same tx queue
> > > without SW lock. This is an HW feature found in some NICs
> > > and useful in the following use cases if HW supports it.
> > 
> > Which hardware supports it?
> 
> Cavium OCTEONTX Packet transmission HW block(PKO).
> 
> > 
> > [...]
> > > --- a/lib/librte_ether/rte_ethdev.h
> > > +++ b/lib/librte_ether/rte_ethdev.h
> > > +#define DEV_TX_OFFLOAD_TXQ_MT_LOCKFREE	0x00004000
> > > +/**< Multiple threads can invoke rte_eth_tx_burst() concurrently on the
> > > same + * tx queue without SW lock.
> > > + */
> > 
> > Why TXQ in the name? DEV_TX_OFFLOAD_MT_LOCKFREE would be enough.
> 
> OK
> 
> > I wonder whether "lock free" wording is confusing because
> > the locks are probably handled in HW.
> 
> Yes. Another reason why it possible because HW is not using ring like scheme(head and tail pointers)
> for Tx. "lock free" wording is more from software perspective.
> 
> > I think the good wording is "offloaded multi-thread capability",
> > maybe with a naming like DEV_TX_OFFLOAD_MT.
> 
> I think SW lock free is the capability here.IMO, it better to reflect the
> capability in the name(DEV_TX_OFFLOAD_MT_LOCKFREE).
> 
> > 
> > Anyway we should reference this flag in rte_eth_tx_burst()
> > and give more details in doc/guides/prog_guide/poll_mode_drv.rst.
> 
> OK. Will address in v2.
> 
> > 
> > Should we wait a first hardware PoC to add this flag?
> 
> If we are in agreement for method expose this feature through capability
> flag then should we really need to wait for driver implementation to
> accept the patch? Thoughts ?
> This flag has impact on how we handle the even dev applications if HW support available.
> We are in the process of up-streaming OCTEONTX ethdev driver, but there
> are few thing like with external pool manager and eventdev integration
> needs to be sorted out cleanly before that.
> 
> > Candidate for 17.08?

Any news of a v2?
Jerin Jacob July 6, 2017, 6 a.m. UTC | #4 
-----Original Message-----
> Date: Wed, 05 Jul 2017 19:46:54 +0200
> From: Thomas Monjalon <thomas@monjalon.net>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> Cc: dev@dpdk.org, bruce.richardson@intel.com, harry.van.haaren@intel.com,
>  hemant.agrawal@nxp.com, gage.eads@intel.com, nipun.gupta@nxp.com,
>  santosh.shukla@caviumnetworks.com
> Subject: Re: [dpdk-dev] [PATCH] ethdev: add lock-less txq capability flag
> 
> 27/04/2017 13:00, Jerin Jacob:
> > -----Original Message-----
> > > Which hardware supports it?
> > 
> > Cavium OCTEONTX Packet transmission HW block(PKO).
> > 
> > > 
> > > [...]
> > > > --- a/lib/librte_ether/rte_ethdev.h
> > > > +++ b/lib/librte_ether/rte_ethdev.h
> > > > +#define DEV_TX_OFFLOAD_TXQ_MT_LOCKFREE	0x00004000
> > > > +/**< Multiple threads can invoke rte_eth_tx_burst() concurrently on the
> > > > same + * tx queue without SW lock.
> > > > + */
> > > 
> > > Why TXQ in the name? DEV_TX_OFFLOAD_MT_LOCKFREE would be enough.
> > 
> > OK
> > 
> > > I wonder whether "lock free" wording is confusing because
> > > the locks are probably handled in HW.
> > 
> > Yes. Another reason why it possible because HW is not using ring like scheme(head and tail pointers)
> > for Tx. "lock free" wording is more from software perspective.
> > 
> > > I think the good wording is "offloaded multi-thread capability",
> > > maybe with a naming like DEV_TX_OFFLOAD_MT.
> > 
> > I think SW lock free is the capability here.IMO, it better to reflect the
> > capability in the name(DEV_TX_OFFLOAD_MT_LOCKFREE).
> > 
> > > 
> > > Anyway we should reference this flag in rte_eth_tx_burst()
> > > and give more details in doc/guides/prog_guide/poll_mode_drv.rst.
> > 
> > OK. Will address in v2.
> > 
> > > 
> > > Should we wait a first hardware PoC to add this flag?
> > 
> > If we are in agreement for method expose this feature through capability
> > flag then should we really need to wait for driver implementation to
> > accept the patch? Thoughts ?
> > This flag has impact on how we handle the even dev applications if HW support available.
> > We are in the process of up-streaming OCTEONTX ethdev driver, but there
> > are few thing like with external pool manager and eventdev integration
> > needs to be sorted out cleanly before that.
> > 
> > > Candidate for 17.08?
> 
> Any news of a v2?

I will send the v2.

>
diff mbox

Patch

diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index f9de9a2b6..696bc9d44 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -899,6 +899,10 @@  struct rte_eth_conf {
 #define DEV_TX_OFFLOAD_IPIP_TNL_TSO     0x00000800    /**< Used for tunneling packet. */
 #define DEV_TX_OFFLOAD_GENEVE_TNL_TSO   0x00001000    /**< Used for tunneling packet. */
 #define DEV_TX_OFFLOAD_MACSEC_INSERT    0x00002000
+#define DEV_TX_OFFLOAD_TXQ_MT_LOCKFREE	0x00004000
+/**< Multiple threads can invoke rte_eth_tx_burst() concurrently on the same
+ * tx queue without SW lock.
+ */
 
 struct rte_pci_device;