[dpdk-dev,RFC] eventdev: event tx adapter APIs

Message ID 1527260924-86922-1-git-send-email-nikhil.rao@intel.com (mailing list archive)
State Changes Requested, archived
Delegated to: Jerin Jacob
Headers

Checks

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

Commit Message

Rao, Nikhil May 25, 2018, 3:08 p.m. UTC
  The patch below introduces the event tx adapter APIs that encapsulate common
code for the tx stage of a packet processing pipeline. These APIs allow
the application to configure a rte_service function that reads mbuf events
from an event queue and sends mbufs on the transmit queue and ethernet
port specified in the mbuf private area.

Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
---
 lib/librte_eventdev/rte_event_eth_tx_adapter.h | 272 +++++++++++++++++++++++++
 1 file changed, 272 insertions(+)
 create mode 100644 lib/librte_eventdev/rte_event_eth_tx_adapter.h
  

Comments

Jerin Jacob May 30, 2018, 7:26 a.m. UTC | #1
-----Original Message-----
> Date: Fri, 25 May 2018 20:38:44 +0530
> From: Nikhil Rao <nikhil.rao@intel.com>
> To: jerin.jacob@caviumnetworks.com, hemant.agrawal@nxp.com
> CC: dev@dpdk.org, narender.vangati@intel.com, abhinandan.gujjar@intel.com,
>  gage.eads@intel.com, Nikhil Rao <nikhil.rao@intel.com>
> Subject: [RFC] eventdev: event tx adapter APIs
> X-Mailer: git-send-email 1.8.3.1
> 
> The patch below introduces the event tx adapter APIs that encapsulate common
> code for the tx stage of a packet processing pipeline. These APIs allow
> the application to configure a rte_service function that reads mbuf events
> from an event queue and sends mbufs on the transmit queue and ethernet
> port specified in the mbuf private area.
> 

Hi Nikhil,

I think, it is reasonable to have Tx adapter.

Some top level comments to starts with,

1) Slow path API looks fine. The only comment is, How about changing
rte_event_eth_tx_adapter_queue_add() instead of rte_event_eth_tx_adapter_queue_start()
to get align  with Rx adapter?

2) This my understanding of fastpath

a) Common code will have a extra port(given through adapter create)
where all workers invoke rte_event_enqueue_burst() to that port and then common code 
dequeue from that port and send packet
using rte_eth_tx_burst() and/or using tx buffering APIs

b) If the source queue or sched_type is ORDERED, When it enqueue to the extra port it
will change to atomic/direct to maintain the the ingress order if need.

Couple of issues and a minor change in RFC to fix the issues as a proposal.

Issue 1) I think, Using mbuf private data for such purpose will be a problem as
exiting application already may using for it own needs and there will be additional cache
miss etc on fastpath to get private data.

Issue 2) At least on Cavium, We can do so optimization by introducing
the same RTE_EVENT_ETH_TX_ADAPTER_CAP_INTERNAL_PORT schematics of Rx
adapter. i.e we don't need extra port to converge all traffic from
different workers and transmit.


Considering above two issues2, We would like propose an fastpath API,
which semantics is almost same rte_ethdev_tx_burst().
a) It will get ride of mbuf metadata need
b) New fastpath API gives driver enough possibilities of optimization if
possible.(address the issue (2))

The API can be like,

uint16_t rte_event_eth_tx_adapter_enqueue(uint16_t eth_port_id, uint16_t
eth_queue_id, uint8_t event_port_id, const struct rte_event ev[], uint16_t nb_events);

This API would land in driver as function pointer if device has
RTE_EVENT_ETH_TX_ADAPTER_CAP_INTERNAL_PORT capability.

If device does not have !RTE_EVENT_ETH_TX_ADAPTER_CAP_INTERNAL_PORT then 
common code can choose to ignore event_port_id in
rte_event_eth_tx_adapter_enqueue() and select the port given in adapter
create in common code.

So from the top-level, each worker lcore will look like

while(1)
{
	rte_event_dequeue_burst(..,worker[port],..);
	(process the packets)
	rte_event_eth_tx_adapter_enqueue(... worker[port],...);
}

Thoughts?
  
Rao, Nikhil June 1, 2018, 6:17 p.m. UTC | #2
Hi Jerin,

On 5/30/2018 12:56 PM, Jerin Jacob wrote:
> -----Original Message-----
> Hi Nikhil,
> I think, it is reasonable to have Tx adapter.
>
> Some top level comments to starts with,
>
> 1) Slow path API looks fine. The only comment is, How about changing
> rte_event_eth_tx_adapter_queue_add() instead of rte_event_eth_tx_adapter_queue_start()
> to get align  with Rx adapter?
OK.
>
> 2) This my understanding of fastpath
>
> a) Common code will have a extra port(given through adapter create)
> where all workers invoke rte_event_enqueue_burst() to that port and then common code
> dequeue from that port and send packet
> using rte_eth_tx_burst() and/or using tx buffering APIs
The workers invoke rte_event_enqueue_burst() to their local port not to 
the extra port as you described. The queue ID specified when
enqueuing is linked to the the adapter's port, the adapter reads these 
events and transmits mbufs on the
ethernet port and queue specified in these mbufs. The diagram below 
illustrates what I just described.

+------+
|      |   +----+
|Worker+-->+port+--+
|      |   +----+  |                                         +----+
+------+           |                                     +-->+eth0|
                    |  +---------+            +-------+   |   +----+
                    +--+         |   +----+   |       +---+   +----+
                       |  Queue  +-->+port+-->+Adapter|------>+eth1|
                    +--+         |   +----+   |       +---+   +----+
+------+           |  +---------+            +-------+   |   +----+
|      |   +----+  |                                     +-->+eth2|
|Worker+-->+port+--+                                         +----+
|      |   +----+
+------+

> b) If the source queue or sched_type is ORDERED, When it enqueue to the extra port it
> will change to atomic/direct to maintain the the ingress order if need.
>
> Couple of issues and a minor change in RFC to fix the issues as a proposal.
>
> Issue 1) I think, Using mbuf private data for such purpose will be a problem as
> exiting application already may using for it own needs and there will be additional cache
> miss etc on fastpath to get private data.
Instead of using mbuf private data, the eth port and queue can be 
specified in
the mbuf itself using mbuf:port and mbuf:hash respectively.

> Issue 2) At least on Cavium, We can do so optimization by introducing
> the same RTE_EVENT_ETH_TX_ADAPTER_CAP_INTERNAL_PORT schematics of Rx
> adapter. i.e we don't need extra port to converge all traffic from
> different workers and transmit.
OK.
>
> Considering above two issues2, We would like propose an fastpath API,
> which semantics is almost same rte_ethdev_tx_burst().
> a) It will get ride of mbuf metadata need
> b) New fastpath API gives driver enough possibilities of optimization if
> possible.(address the issue (2))
>
> The API can be like,
>
> uint16_t rte_event_eth_tx_adapter_enqueue(uint16_t eth_port_id, uint16_t
> eth_queue_id, uint8_t event_port_id, const struct rte_event ev[], uint16_t nb_events);
The worker core will receive events pointing to mbufs that need to be 
transmitted to different
ports/queues, as described above. The port and the queue will be 
populated in the mbuf and the
API can be as below

uint16_t rte_event_eth_tx_adapter_enqueue(uint8_t instance_id, uint8_t event_port_id, const struct rte_event ev[], uint16_t nb_events);

Let me know if that works for you.

> This API would land in driver as function pointer if device has
> RTE_EVENT_ETH_TX_ADAPTER_CAP_INTERNAL_PORT capability.
Agreed.
> If device does not have !RTE_EVENT_ETH_TX_ADAPTER_CAP_INTERNAL_PORT then
> common code can choose to ignore event_port_id in
> rte_event_eth_tx_adapter_enqueue() and select the port given in adapter
> create in common code.
The common code will still use the local worker port.
> So from the top-level, each worker lcore will look like
>
> while(1)
> {
> 	rte_event_dequeue_burst(..,worker[port],..);
> 	(process the packets)
> 	rte_event_eth_tx_adapter_enqueue(... worker[port],...);
> }
Agreed, with the API modification above

Nikhil
  
Jerin Jacob June 4, 2018, 5:11 a.m. UTC | #3
-----Original Message-----
> Date: Fri, 1 Jun 2018 23:47:00 +0530
> From: "Rao, Nikhil" <nikhil.rao@intel.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> CC: hemant.agrawal@nxp.com, dev@dpdk.org, narender.vangati@intel.com,
>  abhinandan.gujjar@intel.com, gage.eads@intel.com, nikhil.rao@intel.com
> Subject: Re: [RFC] eventdev: event tx adapter APIs
> User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
>  Thunderbird/52.8.0
> 
> 
> Hi Jerin,

Hi Nikhil,

> 
> On 5/30/2018 12:56 PM, Jerin Jacob wrote:
> > -----Original Message-----
> > Hi Nikhil,
> > I think, it is reasonable to have Tx adapter.
> > 
> > Some top level comments to starts with,
> > 
> > 1) Slow path API looks fine. The only comment is, How about changing
> > rte_event_eth_tx_adapter_queue_add() instead of rte_event_eth_tx_adapter_queue_start()
> > to get align  with Rx adapter?
> OK.
> > 
> > 2) This my understanding of fastpath
> > 
> > a) Common code will have a extra port(given through adapter create)
> > where all workers invoke rte_event_enqueue_burst() to that port and then common code
> > dequeue from that port and send packet
> > using rte_eth_tx_burst() and/or using tx buffering APIs
> The workers invoke rte_event_enqueue_burst() to their local port not to the
> extra port as you described. The queue ID specified when
> enqueuing is linked to the the adapter's port, the adapter reads these
> events and transmits mbufs on the
> ethernet port and queue specified in these mbufs. The diagram below
> illustrates what I just described.
> 
> +------+
> |      |   +----+
> |Worker+-->+port+--+
> |      |   +----+  |                                         +----+
> +------+           |                                     +-->+eth0|
>                    |  +---------+            +-------+   |   +----+
>                    +--+         |   +----+   |       +---+   +----+
>                       |  Queue  +-->+port+-->+Adapter|------>+eth1|
>                    +--+         |   +----+   |       +---+   +----+
> +------+           |  +---------+            +-------+   |   +----+
> |      |   +----+  |                                     +-->+eth2|
> |Worker+-->+port+--+                                         +----+
> |      |   +----+
> +------+


Makes sense. One suggestion here, Since we have ALL type queue and
normal queues, Can we move the queue change or sched_type change code
from the application and move that down to function pointer abstraction(any
way adapter knows which queues to enqueue for), that way we can have same
final stage code for ALL type queues and Normal queues.

> 
> > b) If the source queue or sched_type is ORDERED, When it enqueue to the extra port it
> > will change to atomic/direct to maintain the the ingress order if need.
> > 
> > Couple of issues and a minor change in RFC to fix the issues as a proposal.
> > 
> > Issue 1) I think, Using mbuf private data for such purpose will be a problem as
> > exiting application already may using for it own needs and there will be additional cache
> > miss etc on fastpath to get private data.
> Instead of using mbuf private data, the eth port and queue can be specified
> in
> the mbuf itself using mbuf:port and mbuf:hash respectively.


Looks good to me.

> 
> > Issue 2) At least on Cavium, We can do so optimization by introducing
> > the same RTE_EVENT_ETH_TX_ADAPTER_CAP_INTERNAL_PORT schematics of Rx
> > adapter. i.e we don't need extra port to converge all traffic from
> > different workers and transmit.
> OK.
> > 
> > Considering above two issues2, We would like propose an fastpath API,
> > which semantics is almost same rte_ethdev_tx_burst().
> > a) It will get ride of mbuf metadata need
> > b) New fastpath API gives driver enough possibilities of optimization if
> > possible.(address the issue (2))
> > 
> > The API can be like,
> > 
> > uint16_t rte_event_eth_tx_adapter_enqueue(uint16_t eth_port_id, uint16_t
> > eth_queue_id, uint8_t event_port_id, const struct rte_event ev[], uint16_t nb_events);
> The worker core will receive events pointing to mbufs that need to be
> transmitted to different
> ports/queues, as described above. The port and the queue will be populated
> in the mbuf and the
> API can be as below
> 
> uint16_t rte_event_eth_tx_adapter_enqueue(uint8_t instance_id, uint8_t event_port_id, const struct rte_event ev[], uint16_t nb_events);
> 
> Let me know if that works for you.

Yes. That API works for me. I think, we can leverage "struct
rte_eventdev" area for adding new function pointer. Just like 
enqueue_new_burst, enqueue_forward_burst variants, we can add one more
there, so that we can reuse that hot cacheline for all fastpath function pointer case.
That would translate to adding "uint8_t dev_id" on the above API.


> 
> > This API would land in driver as function pointer if device has
> > RTE_EVENT_ETH_TX_ADAPTER_CAP_INTERNAL_PORT capability.
> Agreed.
> > If device does not have !RTE_EVENT_ETH_TX_ADAPTER_CAP_INTERNAL_PORT then
> > common code can choose to ignore event_port_id in
> > rte_event_eth_tx_adapter_enqueue() and select the port given in adapter
> > create in common code.
> The common code will still use the local worker port.
> > So from the top-level, each worker lcore will look like
> > 
> > while(1)
> > {
> > 	rte_event_dequeue_burst(..,worker[port],..);
> > 	(process the packets)
> > 	rte_event_eth_tx_adapter_enqueue(... worker[port],...);
> > }
> Agreed, with the API modification above

Great.

> 
> Nikhil
  
Rao, Nikhil June 5, 2018, 9:24 a.m. UTC | #4
On 6/4/2018 10:41 AM, Jerin Jacob wrote:
> -----Original Message-----
>> Date: Fri, 1 Jun 2018 23:47:00 +0530
>> From: "Rao, Nikhil" <nikhil.rao@intel.com>
>> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
>> CC: hemant.agrawal@nxp.com, dev@dpdk.org, narender.vangati@intel.com,
>>   abhinandan.gujjar@intel.com, gage.eads@intel.com, nikhil.rao@intel.com
>> Subject: Re: [RFC] eventdev: event tx adapter APIs
>> User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
>>   Thunderbird/52.8.0
>>
>>
>> Hi Jerin,
> 
>
>> The workers invoke rte_event_enqueue_burst() to their local port not to the
>> extra port as you described. The queue ID specified when
>> enqueuing is linked to the the adapter's port, the adapter reads these
>> events and transmits mbufs on the
>> ethernet port and queue specified in these mbufs. The diagram below
>> illustrates what I just described.
>>
>> +------+
>> |      |   +----+
>> |Worker+-->+port+--+
>> |      |   +----+  |                                         +----+
>> +------+           |                                     +-->+eth0|
>>                     |  +---------+            +-------+   |   +----+
>>                     +--+         |   +----+   |       +---+   +----+
>>                        |  Queue  +-->+port+-->+Adapter|------>+eth1|
>>                     +--+         |   +----+   |       +---+   +----+
>> +------+           |  +---------+            +-------+   |   +----+
>> |      |   +----+  |                                     +-->+eth2|
>> |Worker+-->+port+--+                                         +----+
>> |      |   +----+
>> +------+
> 
> 
> Makes sense. One suggestion here, Since we have ALL type queue and
> normal queues, Can we move the queue change or sched_type change code
> from the application and move that down to function pointer abstraction(any
> way adapter knows which queues to enqueue for), that way we can have same
> final stage code for ALL type queues and Normal queues.
> 
Yes, I see the queue/sched type change approach followed in 
pipeline_worker_tx.c, a queue id can be provided in 
rte_event_eth_tx_adapter_conf

+struct rte_event_eth_tx_adapter_conf {
+	uint8_t event_port_id;
+	/**< Event port identifier, the adapter dequeues mbuf events from this
+	 * port.
+	 */
+	uint16_t tx_metadata_off;
+	/**<  Offset of struct rte_event_eth_tx_adapter_meta in the private
+	 * area of the mbuf
+	 */
+	uint32_t max_nb_tx;
+	/**< The adapter can return early if it has processed at least
+	 * max_nb_tx mbufs. This isn't treated as a requirement; batching may
+	 * cause the adapter to process more than max_nb_tx mbufs.
+	 */
+};

</sniped>

>> The worker core will receive events pointing to mbufs that need to be
>> transmitted to different
>> ports/queues, as described above. The port and the queue will be populated
>> in the mbuf and the
>> API can be as below
>>
>> uint16_t rte_event_eth_tx_adapter_enqueue(uint8_t instance_id, uint8_t event_port_id, const struct rte_event ev[], uint16_t nb_events);
>>
>> Let me know if that works for you.
> 
> Yes. That API works for me. I think, we can leverage "struct
> rte_eventdev" area for adding new function pointer. Just like
> enqueue_new_burst, enqueue_forward_burst variants, we can add one more
> there, so that we can reuse that hot cacheline for all fastpath function pointer case.
> That would translate to adding "uint8_t dev_id" on the above API.
The dev_id can be derived from the instance_id, does that work ?

I need some clarification on the configuration API/flow. The 
eventdev_pipeline sample app checks if DEV_TX_OFFLOAD_MT_LOCKFREE flag 
is set on all ethernet devices and if so, uses the pipeline_worker_tx 
path as opposed to the "consumer" function, if we were to use the 
adapter to replace some of the sample code then it seems like the 
RTE_EVENT_ETH_TX_ADAPTER_CAP_INTERNAL_PORT is hardware assist for the 
pipeline worker tx mode, the adapter would support 2 modes (consumer and 
worker_tx, borrowing terminology from the sample), worker_tx would only 
be supported if the eventdev supports 
RTE_EVENT_ETH_TX_ADAPTER_CAP_INTERNAL_PORT (at least in the first version)

Thanks,
Nikhil
  
Jerin Jacob June 10, 2018, 12:05 p.m. UTC | #5
-----Original Message-----
> Date: Tue, 5 Jun 2018 14:54:58 +0530
> From: "Rao, Nikhil" <nikhil.rao@intel.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> CC: hemant.agrawal@nxp.com, dev@dpdk.org, narender.vangati@intel.com,
>  abhinandan.gujjar@intel.com, gage.eads@intel.com, nikhil.rao@intel.com
> Subject: Re: [RFC] eventdev: event tx adapter APIs
> User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
>  Thunderbird/52.8.0
> 
> On 6/4/2018 10:41 AM, Jerin Jacob wrote:
> > -----Original Message-----
> > > Date: Fri, 1 Jun 2018 23:47:00 +0530
> > > From: "Rao, Nikhil" <nikhil.rao@intel.com>
> > > To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > > CC: hemant.agrawal@nxp.com, dev@dpdk.org, narender.vangati@intel.com,
> > >   abhinandan.gujjar@intel.com, gage.eads@intel.com, nikhil.rao@intel.com
> > > Subject: Re: [RFC] eventdev: event tx adapter APIs
> > > User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
> > >   Thunderbird/52.8.0
> > > 
> > > 
> > > Hi Jerin,
> > 
> > 
> > > The workers invoke rte_event_enqueue_burst() to their local port not to the
> > > extra port as you described. The queue ID specified when
> > > enqueuing is linked to the the adapter's port, the adapter reads these
> > > events and transmits mbufs on the
> > > ethernet port and queue specified in these mbufs. The diagram below
> > > illustrates what I just described.
> > > 
> > > +------+
> > > |      |   +----+
> > > |Worker+-->+port+--+
> > > |      |   +----+  |                                         +----+
> > > +------+           |                                     +-->+eth0|
> > >                     |  +---------+            +-------+   |   +----+
> > >                     +--+         |   +----+   |       +---+   +----+
> > >                        |  Queue  +-->+port+-->+Adapter|------>+eth1|
> > >                     +--+         |   +----+   |       +---+   +----+
> > > +------+           |  +---------+            +-------+   |   +----+
> > > |      |   +----+  |                                     +-->+eth2|
> > > |Worker+-->+port+--+                                         +----+
> > > |      |   +----+
> > > +------+
> > 
> > 
> > Makes sense. One suggestion here, Since we have ALL type queue and
> > normal queues, Can we move the queue change or sched_type change code
> > from the application and move that down to function pointer abstraction(any
> > way adapter knows which queues to enqueue for), that way we can have same
> > final stage code for ALL type queues and Normal queues.
> > 
> Yes, I see the queue/sched type change approach followed in
> pipeline_worker_tx.c, a queue id can be provided in
> rte_event_eth_tx_adapter_conf
> 
> +struct rte_event_eth_tx_adapter_conf {
> +	uint8_t event_port_id;
> +	/**< Event port identifier, the adapter dequeues mbuf events from this
> +	 * port.
> +	 */
> +	uint16_t tx_metadata_off;
> +	/**<  Offset of struct rte_event_eth_tx_adapter_meta in the private
> +	 * area of the mbuf
> +	 */
> +	uint32_t max_nb_tx;
> +	/**< The adapter can return early if it has processed at least
> +	 * max_nb_tx mbufs. This isn't treated as a requirement; batching may
> +	 * cause the adapter to process more than max_nb_tx mbufs.
> +	 */
> +};
> 
> </sniped>
> 
> > > The worker core will receive events pointing to mbufs that need to be
> > > transmitted to different
> > > ports/queues, as described above. The port and the queue will be populated
> > > in the mbuf and the
> > > API can be as below
> > > 
> > > uint16_t rte_event_eth_tx_adapter_enqueue(uint8_t instance_id, uint8_t event_port_id, const struct rte_event ev[], uint16_t nb_events);
> > > 
> > > Let me know if that works for you.
> > 
> > Yes. That API works for me. I think, we can leverage "struct
> > rte_eventdev" area for adding new function pointer. Just like
> > enqueue_new_burst, enqueue_forward_burst variants, we can add one more
> > there, so that we can reuse that hot cacheline for all fastpath function pointer case.
> > That would translate to adding "uint8_t dev_id" on the above API.

> The dev_id can be derived from the instance_id, does that work ?

Do we need to that in fastpath?, IMO, if you can do that in slow path then it is fine.

> 
> I need some clarification on the configuration API/flow. The
> eventdev_pipeline sample app checks if DEV_TX_OFFLOAD_MT_LOCKFREE flag is
> set on all ethernet devices and if so, uses the pipeline_worker_tx path as
> opposed to the "consumer" function,

Yes.

> if we were to use the adapter to replace
> some of the sample code then it seems like the
> RTE_EVENT_ETH_TX_ADAPTER_CAP_INTERNAL_PORT is hardware assist for the
> pipeline worker tx mode, the adapter would support 2 modes (consumer and
> worker_tx, borrowing terminology from the sample), worker_tx would only be
> supported if the eventdev supports
> RTE_EVENT_ETH_TX_ADAPTER_CAP_INTERNAL_PORT (at least in the first version)

I think the flow can be

1) rte_event_eth_tx_adapter_enqueue() function should simply call,

struct rte_eventdev *dev = &rte_eventdev[dev_id];
return (*dev->eth_tx_adapter_enqueue)(...);

2) You can expose generic version of "eth_tx_adapter_enqueue" in Tx
adapter. If drivers does not set the "eth_tx_adapter_enqueue" function
pointer or DEV_TX_OFFLOAD_MT_LOCKFREE flag is NOT set on all ethernet devices
_then_ in common code we can assign "eth_tx_adapter_enqueue" function
pointer as your generic Tx adapter function pointer.

3) I think, you can focus only on generic "consumer" case as you can not
test "worker_tx" case. We are planning to add more optimized raw
"worker_tx" case in driver(Point 2 will allow that by having driver
specific "eth_tx_adapter_enqueue" function pointer).

/Jerin

> 
> Thanks,
> Nikhil
>
  
Jerin Jacob June 10, 2018, 12:12 p.m. UTC | #6
-----Original Message-----
> Date: Tue, 5 Jun 2018 14:54:58 +0530
> From: "Rao, Nikhil" <nikhil.rao@intel.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> CC: hemant.agrawal@nxp.com, dev@dpdk.org, narender.vangati@intel.com,
>  abhinandan.gujjar@intel.com, gage.eads@intel.com, nikhil.rao@intel.com
> Subject: Re: [RFC] eventdev: event tx adapter APIs
> User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
>  Thunderbird/52.8.0
> 
> On 6/4/2018 10:41 AM, Jerin Jacob wrote:
> > -----Original Message-----
> > > Date: Fri, 1 Jun 2018 23:47:00 +0530
> > > From: "Rao, Nikhil" <nikhil.rao@intel.com>
> > > To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > > CC: hemant.agrawal@nxp.com, dev@dpdk.org, narender.vangati@intel.com,
> > >   abhinandan.gujjar@intel.com, gage.eads@intel.com, nikhil.rao@intel.com
> > > Subject: Re: [RFC] eventdev: event tx adapter APIs
> > > User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
> > >   Thunderbird/52.8.0
> > > 
> > > 
> > > Hi Jerin,
> > 
> > 
> > > The workers invoke rte_event_enqueue_burst() to their local port not to the
> > > extra port as you described. The queue ID specified when
> > > enqueuing is linked to the the adapter's port, the adapter reads these
> > > events and transmits mbufs on the
> > > ethernet port and queue specified in these mbufs. The diagram below
> > > illustrates what I just described.
> > > 
> > > +------+
> > > |      |   +----+
> > > |Worker+-->+port+--+
> > > |      |   +----+  |                                         +----+
> > > +------+           |                                     +-->+eth0|
> > >                     |  +---------+            +-------+   |   +----+
> > >                     +--+         |   +----+   |       +---+   +----+
> > >                        |  Queue  +-->+port+-->+Adapter|------>+eth1|
> > >                     +--+         |   +----+   |       +---+   +----+
> > > +------+           |  +---------+            +-------+   |   +----+
> > > |      |   +----+  |                                     +-->+eth2|
> > > |Worker+-->+port+--+                                         +----+
> > > |      |   +----+
> > > +------+
> > 
> > 
> > Makes sense. One suggestion here, Since we have ALL type queue and
> > normal queues, Can we move the queue change or sched_type change code
> > from the application and move that down to function pointer abstraction(any
> > way adapter knows which queues to enqueue for), that way we can have same
> > final stage code for ALL type queues and Normal queues.
> > 
> Yes, I see the queue/sched type change approach followed in
> pipeline_worker_tx.c, a queue id can be provided in
> rte_event_eth_tx_adapter_conf
> 
> +struct rte_event_eth_tx_adapter_conf {
> +	uint8_t event_port_id;
> +	/**< Event port identifier, the adapter dequeues mbuf events from this
> +	 * port.
> +	 */
> +	uint16_t tx_metadata_off;
> +	/**<  Offset of struct rte_event_eth_tx_adapter_meta in the private
> +	 * area of the mbuf
> +	 */
> +	uint32_t max_nb_tx;
> +	/**< The adapter can return early if it has processed at least
> +	 * max_nb_tx mbufs. This isn't treated as a requirement; batching may
> +	 * cause the adapter to process more than max_nb_tx mbufs.
> +	 */
> +};
> 
> </sniped>
> 
> > > The worker core will receive events pointing to mbufs that need to be
> > > transmitted to different
> > > ports/queues, as described above. The port and the queue will be populated
> > > in the mbuf and the
> > > API can be as below
> > > 
> > > uint16_t rte_event_eth_tx_adapter_enqueue(uint8_t instance_id, uint8_t event_port_id, const struct rte_event ev[], uint16_t nb_events);
> > > 
> > > Let me know if that works for you.
> > 
> > Yes. That API works for me. I think, we can leverage "struct
> > rte_eventdev" area for adding new function pointer. Just like
> > enqueue_new_burst, enqueue_forward_burst variants, we can add one more
> > there, so that we can reuse that hot cacheline for all fastpath function pointer case.
> > That would translate to adding "uint8_t dev_id" on the above API.

> The dev_id can be derived from the instance_id, does that work ?

Do we need to that in fastpath?, IMO, if you can do that in slow path then it is fine.

> 
> I need some clarification on the configuration API/flow. The
> eventdev_pipeline sample app checks if DEV_TX_OFFLOAD_MT_LOCKFREE flag is
> set on all ethernet devices and if so, uses the pipeline_worker_tx path as
> opposed to the "consumer" function,

Yes

> if we were to use the adapter to replace
> some of the sample code then it seems like the
> RTE_EVENT_ETH_TX_ADAPTER_CAP_INTERNAL_PORT is hardware assist for the
> pipeline worker tx mode, the adapter would support 2 modes (consumer and
> worker_tx, borrowing terminology from the sample), worker_tx would only be
> supported if the eventdev supports
> RTE_EVENT_ETH_TX_ADAPTER_CAP_INTERNAL_PORT (at least in the first version)

Yes.

1) I think, rte_event_eth_tx_adapter_enqueue() function can simply call,

struct rte_eventdev *dev = &rte_eventdevs[dev_id];
return (*dev->eth_tx_adapter_enqueue)(...);

2) You can expose generic version of "eth_tx_adapter_enqueue" in Tx
adapter. If drivers does not set the "eth_tx_adapter_enqueue" function
pointer or DEV_TX_OFFLOAD_MT_LOCKFREE flag is NOT set on all ethernet devices
_then_ in common code we can assign eth_tx_adapter_enqueue function
pointer as your generic Tx adapter function pointer.

3) I think, you can focus only on generic "consumer" case as you can not
test "worker_tx" case. We are planning to add more optimized raw
"worker_tx" case in driver(Point 2 will allow that by having driver
specific "eth_tx_adapter_enqueue" function pointer).

/Jerin

> 
> Thanks,
> Nikhil
>
  

Patch

diff --git a/lib/librte_eventdev/rte_event_eth_tx_adapter.h b/lib/librte_eventdev/rte_event_eth_tx_adapter.h
new file mode 100644
index 0000000..56218fe
--- /dev/null
+++ b/lib/librte_eventdev/rte_event_eth_tx_adapter.h
@@ -0,0 +1,272 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Intel Corporation.
+ */
+
+#ifndef _RTE_EVENT_ETH_TX_ADAPTER_
+#define _RTE_EVENT_ETH_TX_ADAPTER_
+
+/**
+ * @file
+ *
+ * RTE Event Ethernet Tx Adapter
+ *
+ * The event ethernet Tx adapter provides the transmit stage of a
+ * event driven packet processing application. It dequeues mbuf events from
+ * an event queue and sends mbufs on a ethernet device.
+ *
+ * The ethernet Tx event adapter's functions are:
+ *  - rte_event_eth_tx_adapter_create()
+ *  - rte_event_eth_tx_adapter_free()
+ *  - rte_event_eth_tx_adapter_start()
+ *  - rte_event_eth_tx_adapter_stop()
+ *  - rte_event_eth_tx_adapter_queue_start()
+ *  - rte_event_eth_tx_adapter_queue_stop()
+ *  - rte_event_eth_tx_adapter_stats_get()
+ *  - rte_event_eth_tx_adapter_stats_reset()
+ *
+ * The application creates the adapter using
+ * rte_event_eth_tx_adapter_create(). The application is required to
+ * have linked a queue to the event port specified in this call.
+ *
+ * The application sends mbufs to the adapter via this event queue. The
+ * ethernet port and transmit queue index to send the mbuf on are specified
+ * in the mbuf private area and are accessed using
+ * struct rte_event_eth_tx_adapter_meta. The offset to this structure within
+ * the private area is provided when creating the adapter.
+ *
+ * The application can start and stop the adapter using the
+ * rte_event_eth_tx_adapter_start/stop() calls.
+ *
+ * To support dynamic reconfiguration of Tx queues, the application can
+ * call rte_event_eth_tx_adapter_queue_start()/stop() to synchronize
+ * access to the Tx queue with the adapter. For example, if the application
+ * wants to reconfigure a Tx queue that could be concurrently
+ * being accessed by the adapter, it calls rte_event_eth_tx_adapter_queue_stop()
+ * first, reconfigures the queue and then calls
+ * rte_event_eth_tx_adapter_queue_start() which signals to the adapter
+ * that it is safe to resume access to the Tx queue.
+ *
+ * The adapter uses an EAL service function and its execution is controlled
+ * using the rte_service APIs. The rte_event_eth_tx_adapter_service_id_get()
+ * function can be used to retrieve the adapter's service function ID.
+ */
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <stdint.h>
+
+#include "rte_eventdev.h"
+
+#define RTE_EVENT_ETH_TX_ADAPTER_MAX_INSTANCE 32
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Adapter configuration structure
+ */
+struct rte_event_eth_tx_adapter_conf {
+	uint8_t event_port_id;
+	/**< Event port identifier, the adapter dequeues mbuf events from this
+	 * port.
+	 */
+	uint16_t tx_metadata_off;
+	/**<  Offset of struct rte_event_eth_tx_adapter_meta in the private
+	 * area of the mbuf
+	 */
+	uint32_t max_nb_tx;
+	/**< The adapter can return early if it has processed at least
+	 * max_nb_tx mbufs. This isn't treated as a requirement; batching may
+	 * cause the adapter to process more than max_nb_tx mbufs.
+	 */
+};
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * A structure used to retrieve statistics for an eth tx adapter instance.
+ */
+struct rte_event_eth_tx_adapter_stats {
+	uint64_t event_poll_count;
+	/*< Event port poll count */
+	uint64_t tx_packets;
+	/*< Number of packets transmitted */
+	uint64_t tx_dropped;
+	/*< Number of packets dropped */
+};
+
+/*
+ * Tx adapter metadata used to specifiy the ethernet port id and queue id to be
+ * used for transmitting the mbuf
+ */
+struct rte_event_eth_tx_adapter_meta {
+	uint16_t eth_dev_id;
+	/**< Port identifier of the ethernet device */
+	uint16_t eth_queue_id;
+	/**< Transmit queue index */
+};
+	
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Create a new event ethernet Tx adapter with the specified identifier.
+ *
+ * @param id
+ *  The identifier of the event ethernet Tx adapter.
+ * @param dev_id
+ *  The event device identifier.
+ * @param conf
+ * Tx adapter configuration.
+ * @return
+ *   - 0: Success
+ *   - <0: Error code on failure
+ */
+int rte_event_eth_tx_adapter_create(uint8_t id, uint8_t dev_id,
+				struct rte_event_eth_tx_adapter_conf *conf);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Free an event adapter
+ *
+ * @param id
+ *  Adapter identifier.
+ * @return
+ *   - 0: Success
+ *   - <0: Error code on failure, If the adapter still has Tx queues
+ *      added to it, the function returns -EBUSY.
+ */
+int rte_event_eth_tx_adapter_free(uint8_t id);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Start ethernet Tx event adapter
+ *
+ * @param id
+ *  Adapter identifier.
+ * @return
+ *  - 0: Success, Adapter started correctly.
+ *  - <0: Error code on failure.
+ */
+int rte_event_eth_tx_adapter_start(uint8_t id);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Stop ethernet Tx event adapter
+ *
+ * @param id
+ *  Adapter identifier.
+ * @return
+ *  - 0: Success, Adapter started correctly.
+ *  - <0: Error code on failure.
+ */
+int rte_event_eth_tx_adapter_stop(uint8_t id);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Signal the Tx adapter to start processing mbufs for a
+ * Tx queue. A queue value of -1 is used to indicate all
+ * queues within the device.
+ *
+ * @param id
+ *  Adapter identifier.
+ * @param eth_dev_id
+ *  Ethernet Port Identifier.
+ * @param queue
+ *  Tx queue index.
+ * @return
+ *  - 0: Success, Adapter started correctly.
+ *  - <0: Error code on failure.
+ */
+int rte_event_eth_tx_adapter_queue_start(uint8_t id, 
+					uint16_t eth_dev_id,
+					int32_t queue);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Signal the Tx adapter to stop processing mbufs for a
+ * Tx queue. A queue value of -1 is used to indicate all
+ * queues within the device.
+ *
+ * @param id
+ *  Adapter identifier.
+ * @param eth_dev_id
+ *  Ethernet Port Identifier.
+ * @param queue
+ *  Tx queue index.
+ * @return
+ *  - 0: Success, Adapter started correctly.
+ *  - <0: Error code on failure.
+ */
+int rte_event_eth_tx_adapter_queue_stop(uint8_t id, 
+					uint16_t eth_dev_id,
+					int32_t queue);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Retrieve statistics for an adapter
+ *
+ * @param id
+ *  Adapter identifier.
+ * @param [out] stats
+ *  A pointer to structure used to retrieve statistics for an adapter.
+ * @return
+ *  - 0: Success, retrieved successfully.
+ *  - <0: Error code on failure.
+ */
+int rte_event_eth_tx_adapter_stats_get(uint8_t id,
+				struct rte_event_eth_tx_adapter_stats *stats);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Reset statistics for an adapter.
+ *
+ * @param id
+ *  Adapter identifier.
+ * @return
+ *  - 0: Success, statistics reset successfully.
+ *  - <0: Error code on failure.
+ */
+int rte_event_eth_tx_adapter_stats_reset(uint8_t id);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Retrieve the service ID of an adapter. If the adapter doesn't use
+ * a rte_service function, this function returns -ESRCH.
+ *
+ * @param id
+ *  Adapter identifier.
+ * @param [out] service_id
+ *  A pointer to a uint32_t, to be filled in with the service id.
+ * @return
+ *  - 0: Success
+ *  - <0: Error code on failure, if the adapter doesn't use a rte_service
+ * function, this function returns -ESRCH.
+ */
+int rte_event_eth_tx_adapter_service_id_get(uint8_t id, uint32_t *service_id);
+
+#ifdef __cplusplus
+}
+#endif
+#endif	/* _RTE_EVENT_ETH_TX_ADAPTER_ */
-- 
1.8.3.1