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

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

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

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.

Thanks,
Nipun

> 
> Thanks,
> Nikhil