-----Original Message-----
> Date: Fri, 1 Jun 2018 23:47:00 +0530
> From: "Rao, Nikhil" <nikhil....@intel.com>
> To: Jerin Jacob <jerin.ja...@caviumnetworks.com>
> CC: hemant.agra...@nxp.com, dev@dpdk.org, narender.vang...@intel.com,
>  abhinandan.guj...@intel.com, gage.e...@intel.com, nikhil....@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

Reply via email to