Re: [dpdk-dev] [PATCH v4] eventdev: flag to identify same destined packets enqueue

[Jerin Jacob <jerinjacobk@gmail.com>]

On Thu, Oct 10, 2019 at 6:13 PM Nipun Gupta <nipun.gupta@nxp.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Rao, Nikhil <nikhil.rao@intel.com>
> > Sent: Thursday, October 10, 2019 3:36 PM
> > To: Nipun Gupta <nipun.gupta@nxp.com>; dev@dpdk.org
> > Cc: jerinj@marvell.com; aconole@redhat.com; pbhagavatula@marvell.com;
> > skori@marvell.com; Hemant Agrawal <hemant.agrawal@nxp.com>;
> > Richardson, Bruce <bruce.richardson@intel.com>; Kovacevic, Marko
> > <marko.kovacevic@intel.com>; orika@mellanox.com; Nicolau, Radu
> > <radu.nicolau@intel.com>; Kantecki, Tomasz <tomasz.kantecki@intel.com>;
> > Van Haaren, Harry <harry.van.haaren@intel.com>
> > Subject: RE: [PATCH v4] eventdev: flag to identify same destined packets
> > enqueue
> >
> > Hi Nipun,
> >
> >
> > > -----Original Message-----
> > > From: Nipun Gupta [mailto:nipun.gupta@nxp.com]
> > > Sent: Wednesday, October 9, 2019 1:03 PM
> > > To: dev@dpdk.org
> > > Cc: jerinj@marvell.com; aconole@redhat.com; pbhagavatula@marvell.com;
> > > skori@marvell.com; hemant.agrawal@nxp.com; Richardson, Bruce
> > > <bruce.richardson@intel.com>; Kovacevic, Marko
> > > <marko.kovacevic@intel.com>; orika@mellanox.com; Nicolau, Radu
> > > <radu.nicolau@intel.com>; Kantecki, Tomasz
> > <tomasz.kantecki@intel.com>;
> > > Van Haaren, Harry <harry.van.haaren@intel.com>; Rao, Nikhil
> > > <nikhil.rao@intel.com>; Nipun Gupta <nipun.gupta@nxp.com>
> > > Subject: [PATCH v4] eventdev: flag to identify same destined packets
> > enqueue
> > >
> > > This patch introduces a `flag` in the Eth TX adapter enqueue API.
> > > Some drivers may support burst functionality only with the packets having
> > > same destination device and queue.
> > >
> > > The flag `RTE_EVENT_ETH_TX_ADAPTER_ENQUEUE_SAME_DEST` can be
> > used
> > > to indicate this so the underlying driver, for drivers to utilize burst
> > functionality
> > > appropriately.
> > >
> > > Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
> > > Acked-by: Jerin Jacob <jerinj@marvell.com>
> > > ---
> > >
> > > Changes in v4:
> > >  - Update rel note specifying the API change
> > >  - Remove redundant rte_event_tx_adapter_enqueue_same_dest API
> > >
> > </snip>
> >
> > >  /**
> > >   * Enqueue a burst of events objects or an event object supplied in
> > *rte_event*
> > >   * structure on an  event device designated by its *dev_id* through the
> > event
> > > @@ -324,6 +329,10 @@
> > rte_event_eth_tx_adapter_event_port_get(uint8_t
> > > id, uint8_t *event_port_id);
> > >   *  The number of event objects to enqueue, typically number of
> > >   *  rte_event_port_attr_get(...RTE_EVENT_PORT_ATTR_ENQ_DEPTH...)
> > >   *  available for this port.
> > > + * @param flags
> > > + *  RTE_EVENT_ETH_TX_ADAPTER_ENQUEUE_ flags.
> > > + *  #RTE_EVENT_ETH_TX_ADAPTER_ENQUEUE_SAME_DEST signifies that
> > all
> > > the
> > > + packets
> > > + *  which are enqueued are destined for the same Ethernet port & Tx
> > queue.
> > >   *
> > >   * @return
> > >   *   The number of event objects actually enqueued on the event device.
> > The
> > > @@ -343,7 +352,8 @@ static inline uint16_t
> > > rte_event_eth_tx_adapter_enqueue(uint8_t dev_id,
> > >                             uint8_t port_id,
> > >                             struct rte_event ev[],
> > > -                           uint16_t nb_events)
> > > +                           uint16_t nb_events,
> > > +                           const uint8_t flags)
> > >  {
> > >     const struct rte_eventdev *dev = &rte_eventdevs[dev_id];
> > >
> > > @@ -359,7 +369,12 @@ rte_event_eth_tx_adapter_enqueue(uint8_t
> > dev_id,
> > >             return 0;
> > >     }
> > >  #endif
> > > -   return dev->txa_enqueue(dev->data->ports[port_id], ev,
> > nb_events);
> > > +   if (flags)
> > > +           return dev->txa_enqueue_same_dest(dev->data-
> > > >ports[port_id],
> > > +                                             ev, nb_events);
> > > +   else
> > > +           return dev->txa_enqueue(dev->data->ports[port_id], ev,
> > > +                                   nb_events);
> > >  }
> >
> > For the if (flags) condition to work for the PMDs that support
> > RTE_EVENT_ETH_TX_ADAPTER_CAP_INTERNAL_PORT but do not have a
> > txa_enqueue_same_dest callback, doesn't the dev-
> > >txa_enqueue_same_dest pointer need to be set to the same value as dev-
> > >txa_enqueue ?
>
> Something like below would work, but it would be unnecessary additional cost
>         if (flags && dev->txa_enqueue_same_dest)

Yes

> Instead the drivers can have both the function pointers pointing to the same
> function in such cases. Seems legitimate?
> Ill add the function pointers in the octeontx and octeontx2 drivers in the next spin.

OK. Please make the change as slow path.

>
> Thanks,
> Nipun
>
> >
> > Thanks,
> > Nikhil
>