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

[Nipun Gupta <nipun.gupta@nxp.com>]

> -----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.

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);
> >> > > >> +  }
> >> > > >> +}