Re: [dpdk-dev] [PATCH v4 08/10] examples/l2fwd-event: add eventdev main loop

[Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>]

>-----Original Message-----
>From: dev <dev-bounces@dpdk.org> On Behalf Of Nipun Gupta
>Sent: Monday, September 30, 2019 11:21 PM
>To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Jerin
>Jacob <jerinjacobk@gmail.com>
>Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
>bruce.richardson@intel.com; Akhil Goyal <akhil.goyal@nxp.com>;
>Marko Kovacevic <marko.kovacevic@intel.com>; Ori Kam
><orika@mellanox.com>; Radu Nicolau <radu.nicolau@intel.com>;
>Tomasz Kantecki <tomasz.kantecki@intel.com>; Sunil Kumar Kori
><skori@marvell.com>; dev@dpdk.org; Hemant Agrawal
><hemant.agrawal@nxp.com>
>Subject: Re: [dpdk-dev] [PATCH v4 08/10] examples/l2fwd-event: add
>eventdev main loop
>
>
>
>> -----Original Message-----
>> From: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
>> Sent: Monday, September 30, 2019 1:39 PM
>> To: Nipun Gupta <nipun.gupta@nxp.com>; Jerin Jacob
><jerinjacobk@gmail.com>
>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
>> bruce.richardson@intel.com; Akhil Goyal <akhil.goyal@nxp.com>;
>Marko
>> Kovacevic <marko.kovacevic@intel.com>; Ori Kam
><orika@mellanox.com>;
>> Radu Nicolau <radu.nicolau@intel.com>; Tomasz Kantecki
>> <tomasz.kantecki@intel.com>; Sunil Kumar Kori
><skori@marvell.com>;
>> dev@dpdk.org
>> Subject: RE: [dpdk-dev] [PATCH v4 08/10] examples/l2fwd-event: add
>eventdev
>> main loop
>>
>>
>>
>> >-----Original Message-----
>> >From: Nipun Gupta <nipun.gupta@nxp.com>
>> >Sent: Monday, September 30, 2019 1:17 PM
>> >To: Jerin Jacob <jerinjacobk@gmail.com>
>> >Cc: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Jerin
>> >Jacob Kollanukkaran <jerinj@marvell.com>;
>> >bruce.richardson@intel.com; Akhil Goyal <akhil.goyal@nxp.com>;
>> >Marko Kovacevic <marko.kovacevic@intel.com>; Ori Kam
>> ><orika@mellanox.com>; Radu Nicolau <radu.nicolau@intel.com>;
>> >Tomasz Kantecki <tomasz.kantecki@intel.com>; Sunil Kumar Kori
>> ><skori@marvell.com>; dev@dpdk.org
>> >Subject: RE: [dpdk-dev] [PATCH v4 08/10] examples/l2fwd-event:
>add
>> >eventdev main loop
>> >
>> >
>> >
>> >> -----Original Message-----
>> >> From: Jerin Jacob <jerinjacobk@gmail.com>
>> >> Sent: Monday, September 30, 2019 12:08 PM
>> >> To: Nipun Gupta <nipun.gupta@nxp.com>
>> >> Cc: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>;
>Jerin
>> >Jacob
>> >> Kollanukkaran <jerinj@marvell.com>;
>bruce.richardson@intel.com;
>> >Akhil
>> >> Goyal <akhil.goyal@nxp.com>; Marko Kovacevic
>> >> <marko.kovacevic@intel.com>; Ori Kam <orika@mellanox.com>;
>> >Radu
>> >> Nicolau <radu.nicolau@intel.com>; Tomasz Kantecki
>> >> <tomasz.kantecki@intel.com>; Sunil Kumar Kori
>> ><skori@marvell.com>;
>> >> dev@dpdk.org
>> >> Subject: Re: [dpdk-dev] [PATCH v4 08/10] examples/l2fwd-event:
>add
>> >> eventdev main loop
>> >>
>> >> On Mon, Sep 30, 2019 at 11:08 AM Nipun Gupta
>> ><nipun.gupta@nxp.com>
>> >> wrote:
>> >> >
>> >> >
>> >> >
>> >> > > -----Original Message-----
>> >> > > From: Pavan Nikhilesh Bhagavatula
><pbhagavatula@marvell.com>
>> >> > > Sent: Friday, September 27, 2019 8:05 PM
>> >> > > To: Nipun Gupta <nipun.gupta@nxp.com>; Jerin Jacob
>> >Kollanukkaran
>> >> > > <jerinj@marvell.com>; bruce.richardson@intel.com; Akhil
>Goyal
>> >> > > <akhil.goyal@nxp.com>; Marko Kovacevic
>> ><marko.kovacevic@intel.com>;
>> >> > > Ori Kam <orika@mellanox.com>; Radu Nicolau
>> ><radu.nicolau@intel.com>;
>> >> > > Tomasz Kantecki <tomasz.kantecki@intel.com>; Sunil Kumar
>Kori
>> >> > > <skori@marvell.com>
>> >> > > Cc: dev@dpdk.org
>> >> > > Subject: RE: [dpdk-dev] [PATCH v4 08/10] examples/l2fwd-
>event:
>> >add
>> >> > > eventdev main loop
>> >> > >
>> >> > > >>
>> >> > > >> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>> >> > > >>
>> >> > > >> Add event dev main loop based on enabled l2fwd options
>and
>> >> > > >eventdev
>> >> > > >> capabilities.
>> >> > > >>
>> >> > > >> Signed-off-by: Pavan Nikhilesh
><pbhagavatula@marvell.com>
>> >> > > >> ---
>> >> > > >
>> >> > > ><snip>
>> >> > > >
>> >> > > >> +          if (flags & L2FWD_EVENT_TX_DIRECT) {
>> >> > > >> +                  rte_event_eth_tx_adapter_txq_set(mbuf, 0);
>> >> > > >> +                  while
>> >> > > >> (!rte_event_eth_tx_adapter_enqueue(event_d_id,
>> >> > > >> +                                                          port_id,
>> >> > > >> +                                                          &ev, 1)
>> >> > > >&&
>> >> > > >> +                                  !*done)
>> >> > > >> +                          ;
>> >> > > >> +          }
>> >> > > >
>> >> > > >In the TX direct mode we can send packets directly to the
>> >ethernet
>> >> > > >device using ethdev
>> >> > > >API's. This will save unnecessary indirections and event
>unfolds
>> >within
>> >> > > >the driver.
>> >> > >
>> >> > > How would we guarantee atomicity of access to Tx queues?
>> >Between
>> >> cores
>> >> > > as we can only use one Tx queue.
>> >> > > Also, if SCHED_TYPE is ORDERED how would we guarantee flow
>> >ordering?
>> >> > > The capability of MT_LOCKFREE and flow ordering is abstracted
>> >through `
>> >> > > rte_event_eth_tx_adapter_enqueue `.
>> >> >
>> >> > I understand your objective here. Probably in your case the
>DIRECT
>> >is
>> >> equivalent
>> >> > to giving the packet to the scheduler, which will pass on the
>packet
>> >to the
>> >> destined device.
>> >> > On NXP platform, DIRECT implies sending the packet directly to
>the
>> >device
>> >> (eth/crypto),
>> >> > and scheduler will internally pitch in.
>> >> > Here we will need another option to send it directly to the
>device.
>> >> > We can set up a call to discuss the same, or send patch regarding
>this
>> >to you
>> >> to incorporate
>> >> > the same in your series.
>> >>
>> >> Yes. Sending the patch will make us understand better.
>> >>
>> >> Currently, We have two different means for abstracting Tx adapter
>> >fast
>> >> path changes,
>> >> a) SINGLE LINK QUEUE
>> >> b) rte_event_eth_tx_adapter_enqueue()
>> >>
>> >> Could you please share why any of the above schemes do not
>work
>> >for NXP
>> >> HW?
>> >> If there is no additional functionality in
>> >> rte_event_eth_tx_adapter_enqueue(), you could
>> >> simply call direct ethdev tx burst function pointer to make
>> >> abstraction  intact to avoid
>> >> one more code flow in the fast path.
>> >>
>> >> If I guess it right since NXP HW supports MT_LOCKFREE and only
>> >atomic, due
>> >> to
>> >> that, calling eth_dev_tx_burst will be sufficient. But abstracting
>> >> over rte_event_eth_tx_adapter_enqueue()
>> >> makes application life easy. You can call the low level DPPA2 Tx
>> >function in
>> >> rte_event_eth_tx_adapter_enqueue() to avoid any performance
>> >> impact(We
>> >> are doing the same).
>> >
>> >Yes, that’s correct regarding our H/W capability.
>> >Agree that the application will become complex by adding more
>code
>> >flow,
>> >but calling Tx functions internally may lead to additional CPU cycles.
>> >Give us a couple of days to analyze the performance impact, and as
>you
>> >also say, I too
>> >don't think it would be much. We should be able to manage it in
>within
>> >our driver.
>>
>> When application calls rte_event_eth_tx_adapter_queue_add()
>based on
>> the eth_dev_id the underlying eventdevice can set
>> set rte_event_eth_tx_adapter_enqueue() to directly call a function
>which
>> does the platform specific Tx.
>>
>> i.e if eth_dev is net/dpaa and event dev is also net/dpaa we need
>_not_ call
>> `rte_eth_tx_burst()` in ` rte_event_eth_tx_adapter_enqueue()` it
>can directly
>> Invoke the platform specific Rx function which would avoid function
>pointer
>> indirection.
>
>I have some performance concern regarding the burst mode; not w.r.t
>the
>function call sequence, but w.r.t the burst functionality.
>
>The API `rte_event_eth_tx_adapter_enqueue()` is called with `nb_rx`
>events. In case we
>are calling the Ethernet API's directly from within the adapter, we will
>still need to send
>all of them separately to the Ethernet device rather than in burst (or
>scan and separate
>the packets internally for ethernet device, queue pair). This separation
>in the driver is
>more complex than in the application, as application is aware of the Eth
>dev and queues
>it is using and thus can easily bifurcate the events.
>
>I suggest to have a flag in the `rte_event_eth_tx_adapter_enqueue()`
>API to determine
>if the application is sending all the packets in a particular API call for a
>single destination,
>so that driver can act smartly and send the burst to Eth Tx function, on
>the basis of fields
>set in the first mbuf.
>

We could have a flag for the above but the application still needs to segregate 
packets based on port_id as `rte_event_dequeue_burst` doesn’t guarantee 
that all the packets arrive from the same ethernet port/queue. 

I think since the application is setting mbuf->port and 
`rte_event_eth_tx_adapter_txq_set`, the segregation should be done at 
`rte_event_eth_tx_adapter_enqueue` as it would be the same logic for every 
application and reduces application complexity.

Regards,
Pavan.

>Seems fine to you guys? I plan to send the patch regarding this soon.
>
>Regards,
>Nipun
>
>>
>> >
>> >>
>> >>
>> >> >
>> >> > >
>> >> > > @see examples/eventdev_pipeline and app/test-
>> >> eventdev/test_pipeline_*.
>> >> >
>> >> > Yes, we are aware of that, They are one way of representing,
>how
>> >to build
>> >> a complete eventdev pipeline.
>> >> > They don't work on NXP HW.
>> >> > We plan to send patches for them to fix them for NXP HW soon.
>> >> >
>> >> > Regards,
>> >> > Nipun
>> >> >
>> >> > >
>> >> > > >
>> >> > > >> +
>> >> > > >> +          if (timer_period > 0)
>> >> > > >> +                  __atomic_fetch_add(&eventdev_rsrc-
>> >> > > >>stats[mbuf-
>> >> > > >> >port].tx,
>> >> > > >> +                                     1, __ATOMIC_RELAXED);
>> >> > > >> +  }
>> >> > > >> +}