Re: [dpdk-dev] [RFC] [PATCH v2] libeventdev: event driven programming model framework for DPDK

[Jerin Jacob <jerin.jacob@caviumnetworks.com>]

On Mon, Oct 17, 2016 at 08:26:33PM +0000, Eads, Gage wrote:
> 
> 
> >  -----Original Message-----
> >  From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> >  Sent: Sunday, October 16, 2016 11:18 PM
> >  To: Eads, Gage <gage.eads@intel.com>
> >  Cc: dev@dpdk.org; thomas.monjalon@6wind.com; Richardson, Bruce
> >  <bruce.richardson@intel.com>; Vangati, Narender
> >  <narender.vangati@intel.com>; hemant.agrawal@nxp.com
> >  Subject: Re: [dpdk-dev] [RFC] [PATCH v2] libeventdev: event driven
> >  programming model framework for DPDK
> >  
> >  On Fri, Oct 14, 2016 at 03:00:57PM +0000, Eads, Gage wrote:
> >  > Thanks Jerin, this looks good. I've put a few notes/questions inline.
> >  
> >  Thanks Gage.
> >  
> >  >
> >  > >  +
> >  > >  +/**
> >  > >  + * Get the device identifier for the named event device.
> >  > >  + *
> >  > >  + * @param name
> >  > >  + *   Event device name to select the event device identifier.
> >  > >  + *
> >  > >  + * @return
> >  > >  + *   Returns event device identifier on success.
> >  > >  + *   - <0: Failure to find named event device.
> >  > >  + */
> >  > >  +extern uint8_t
> >  > >  +rte_event_dev_get_dev_id(const char *name);
> >  >
> >  > This return type should be int8_t, or some signed type, to support the failure
> >  case.
> >  
> >  Makes sense. I will change to int to make consistent with
> >  rte_cryptodev_get_dev_id()
> >  
> >  >
> >  > >  +};
> >  > >  +
> >  > >  +/**
> >  > >  + * Schedule one or more events in the event dev.
> >  > >  + *
> >  > >  + * An event dev implementation may define this is a NOOP, for
> >  > > instance if  + * the event dev performs its scheduling in hardware.
> >  > >  + *
> >  > >  + * @param dev_id
> >  > >  + *   The identifier of the device.
> >  > >  + */
> >  > >  +extern void
> >  > >  +rte_event_schedule(uint8_t dev_id);
> >  >
> >  > One idea: Have the function return the number of scheduled packets (or 0 for
> >  implementations that do scheduling in hardware). This could be a helpful
> >  diagnostic for the software scheduler.
> >  
> >  How about returning an implementation specific value ?
> >  Rather than defining certain function associated with returned value.
> >  Just to  make sure it works with all HW/SW implementations. Something like
> >  below,
> >  
> >  /**
> >   * Schedule one or more events in the event dev.
> >   *
> >   * An event dev implementation may define this is a NOOP, for instance if
> >   * the event dev performs its scheduling in hardware.
> >   *
> >   * @param dev_id
> >   *   The identifier of the device.
> >   * @return
> >   *   Implementation specific value from the event driver for diagnostic purpose
> >   */
> >  extern int
> >  rte_event_schedule(uint8_t dev_id);
> >  
> >  
> 
> That's fine by me.

OK. I will change it in v3

> 
> I also had a comment on the return value of rte_event_dev_info_get() in my previous email: "I'm wondering if this return type should be int, so we can return an error if the dev_id is invalid."
> 
> What do you think?

The void return was based on cryptodev_info_get().I think, it makes
sense to return "int". I will change it in v3.


> 
> Thanks,
> Gage
> 
> >  
> >