Re: [dpdk-dev] [PATCH] eventdev: abstract ethdev HW capability to inject packets

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

-----Original Message-----
> Date: Fri, 5 May 2017 19:02:59 +0000
> From: "Eads, Gage" <gage.eads@intel.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, "dev@dpdk.org"
>  <dev@dpdk.org>
> CC: "thomas@monjalon.net" <thomas@monjalon.net>, "Richardson, Bruce"
>  <bruce.richardson@intel.com>, "Van Haaren, Harry"
>  <harry.van.haaren@intel.com>, "hemant.agrawal@nxp.com"
>  <hemant.agrawal@nxp.com>, "nipun.gupta@nxp.com" <nipun.gupta@nxp.com>,
>  "Vangati, Narender" <narender.vangati@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] eventdev: abstract ethdev HW capability to
>  inject packets
> 
> Hi Jerin,

Hi Gage,

> 
> >  -----Original Message-----
> >  From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> >  Sent: Friday, May 5, 2017 8:34 AM
> >  To: dev@dpdk.org
> >  Cc: thomas@monjalon.net; Richardson, Bruce <bruce.richardson@intel.com>;
> >  Van Haaren, Harry <harry.van.haaren@intel.com>; hemant.agrawal@nxp.com;
> >  Eads, Gage <gage.eads@intel.com>; nipun.gupta@nxp.com; Vangati,
> >  Narender <narender.vangati@intel.com>; Jerin Jacob
> >  <jerin.jacob@caviumnetworks.com>
> >  Subject: [dpdk-dev] [PATCH] eventdev: abstract ethdev HW capability to inject
> >  packets
> >  
> >  Some Ethdev Hardware is capable of injecting the events(Ethernet packets) to
> >  eventdev without the need for dedicated service cores on Rx path.
> >  Since eventdev API is device capability agnostic, we need to address three
> >  combinations of ethdev and eventdev PMD drivers.
> >  
> >  1) Ethdev HW is not capable of injecting the packets and SW eventdev driver(All
> >  existing ethdev PMD + drivers/event/sw PMD combination)
> >  2) Ethdev HW is not capable of injecting the packets and not compatible HW
> >  eventdev driver(All existing ethdev PMD + driver/event/octeontx PMD
> >  combination)
> >  3) Ethdev HW is capable of injecting the packet to compatible HW eventdev
> >  driver.
> >  
> >  This patch abstract such capability disparity and have unified way to get the
> >  functionality in the application.
> >  
> >  NOTE: The same infrastructure can be used _if_ an SW PMD wish to create the
> >  default producer as service function with the service core scheme in EAL to
> >  make it completely transparent to the application for the default case. The
> >  selection of service core enablement can be a vdev argument to SW PMD.
> >  
> 
> I'm not sure that a vdev argument/arguments is the right way to go for cases 1 and 2. It'll be rather complicated to specify the service core configuration per event producer on the command line, and that configuration can't use runtime information.

Yes. I didn't meant service core configuration needs to passed through vdev
argument. I meant, only the true/false parameter in vdev command line to enable
default producer service function in PMD _if_ a PMD wish to do that way.
A PMD can choose to return always FALSE for rte_event_dev_has_producer() as it
will have be hook to PMD driver.

Since service core proposal, talked only about "PMD" registering the
service core functions NOT _applications_, I thought you guys need to move
default producer handler in PMD and invoke them through service core
infrastructure in application with application selected service core mask.

if you think, it always makes sense to return FALSE to
rte_event_dev_has_producer() on SW PMD then it is fine.
This patch does not dictate to return TRUE and it is completely in PMD control.
I can remove NOTE: section in git commit section it is confusing.

> 
> What do you think about this approach: We develop a library of device producer code, with interfaces designed to run on service cores, that the app can run if rte_event_dev_has_producer() returns FALSE. The device producer code would, at minimum, have some configuration function (more or less equivalent to the rte_event_queue_producer_conf) and a run function that (at a high level) processes eth/crypto/etc.-dev queues and injects events into an event device.

That fine.

> 
> For example, if rte_event_dev_has_producer() indicates there's no hardware support for connecting an ethdev to the event device, the application configures the ethdev->eventdev producer code and installs the run function with the service core infrastructure.
> 
> Plus, this approach allows an application to plug in their own device producer logic, if they require some app-specific functionality. In that case, the DPDK-provided device producer could serve as a template for the app developer.

Make sense. I believe, which is inline with rte_event_dev_has_producer()
comment in the patch
------------------------------------------
When application needs to setup an event producer to inject the events to
eventdev, application must check the event device producer
capability by invoking this function, On success, application can configure to inject the
events to eventdev by setting up the event queue using rte_event_queue_setup() with checked *conf*.
On failure due to lack of adequate capability in PMD or an application that wish to
                                                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
inject the event through application, it must create
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
additional event port by configuring the eventdev with rte_event_dev_config()
and use rte_event_enqueue_burst() to inject the event through op == RTE_EVENT_OP_NEW operation
-------------------------------------------

> If you agree this approach -- at least at a high level -- I'll put together an RFC the library and a header file for an eth->eventdev block and we can work on the details.

Sure. Please send the RFC.


> 
> >  Detailed comments are added in the header file.
> >  
> >  Example API usage:
> >  
> >  - rte_eth_dev_configure(port,..);
> >  - rte_eth_rx_queue_setup(port,..);
> >  - rte_eth_dev_start(port,..);
> >  
> >  struct rte_event_queue_producer_conf ethdev_conf =  {
> >  	.event_type = RTE_EVENT_TYPE_ETHDEV;
> >  	.sched_type = RTE_SCHED_TYPE_ATOMIC;
> >  	.priority =  RTE_EVENT_DEV_PRIORITY_LOWEST;
> >  	.ethdev.ethdev_port = port;
> >    	.ethdev.rx_queue_id = -1;
> >  };
> >  
> >  - rte_event_dev_has_producer(dev_id, &ethdev_conf); # configure additional
> >  producer ports based on eventdev producer capability.
> >  - rte_event_dev_configure(dev_id,..);
> >  
> >  struct rte_event_queue_conf = conf {
> >  	nb_producers = 1;
> >  	producers = &ethdev_conf;
> >  };
> >  # configure event queue based on eventdev producer capability
> >  - rte_event_queue_setup(dev_id, &conf,..);
> >  - rte_event_port_setup(dev_id,..);
> >  - rte_event_dev_start(dev_id);
> >  
> >  Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> >  ---
> >  This patch is based on the review commets from following RFC
> >  http://dpdk.org/ml/archives/dev/2017-May/065176.html
> >  ---
> >   lib/librte_eventdev/rte_eventdev.h | 110
> >  +++++++++++++++++++++++++++++++++++++
> >   1 file changed, 110 insertions(+)
> >  
> >  diff --git a/lib/librte_eventdev/rte_eventdev.h
> >  b/lib/librte_eventdev/rte_eventdev.h
> >  index 20e7293e0..202d03f61 100644
> >  --- a/lib/librte_eventdev/rte_eventdev.h
> >  +++ b/lib/librte_eventdev/rte_eventdev.h
> >  @@ -396,6 +396,100 @@ struct rte_event_dev_info {  int
> >  rte_event_dev_info_get(uint8_t dev_id, struct rte_event_dev_info *dev_info);
> >  
> >  +/** Eventdev producer configuration structure.
> >  + * The events are injected to event device through *enqueue* operation
> >  +with
> >  + * op == RTE_EVENT_OP_NEW by event producers in the system. If the
> >  +event
> >  + * producer is an Ethernet device then eventdev PMD may operate in
> >  +conjunction
> >  + * with ethdev PMD to injects the events(Ethernet packets) to eventdev.
> >  + * The event injection can happen in HW or SW or the combination of
> >  +these two
> >  + * based on the HW capabilities of target eventdev and ethdev PMDs.
> >  + *
> >  + * @see rte_event_dev_has_producer() rte_event_queue_setup()
> >  + *
> >  + */
> >  +struct rte_event_dev_producer_conf {
> >  +	uint32_t event_type:4;
> >  +	/**< Event type to classify the event source.
> >  +	 * @see RTE_EVENT_TYPE_ETHDEV, (RTE_EVENT_TYPE_*)
> >  +	 */
> >  +	uint8_t sched_type:2;
> >  +	/**< Scheduler synchronization type (RTE_SCHED_TYPE_*)
> >  +	 * associated with flow id on a given event queue for the enqueue
> >  +	 * operation.
> >  +	 */
> >  +	uint8_t priority;
> >  +	/**< Event priority relative to other events in the
> >  +	 * event queue. The requested priority should in the
> >  +	 * range of  [RTE_EVENT_DEV_PRIORITY_HIGHEST,
> >  +	 * RTE_EVENT_DEV_PRIORITY_LOWEST].
> >  +	 * The implementation shall normalize the requested
> >  +	 * priority to supported priority value.
> >  +	 * Valid when the device has
> >  +	 * RTE_EVENT_DEV_CAP_EVENT_QOS capability.
> >  +	 */
> >  +	union {
> >  +		struct rte_event_ethdev_producer {
> >  +			uint16_t ethdev_port;
> >  +			/**< The port identifier of the Ethernet device */
> >  +			int32_t rx_queue_id;
> >  +			/**< The index of the receive queue from which to
> >  +			 * retrieve the input packets and inject to eventdev.
> >  +			 * The value -1 denotes all the Rx queues configured
> >  +			 * for the given ethdev_port are selected for retrieving
> >  +			 * the input packets and then injecting the
> >  +			 * events/packets to eventdev.
> >  +			 * The rte_eth_rx_burst() result is undefined
> >  +			 * if application invokes on bounded ethdev_port and
> >  +			 * rx_queue_id.
> >  +			 */
> >  +		} ethdev; /* RTE_EVENT_TYPE_ETHDEV */
> >  +		/**< Valid when event_type == RTE_EVENT_TYPE_ETHDEV.
> >  +		 * Implementation may use mbuff's rss->hash value as
> >  +		 * flow_id for the enqueue operation.
> >  +		 */
> >  +	};
> >  +};
> >  +
> >  +/**
> >  + * Check for eventdev producer capability for the given eventdev
> >  +producer *conf*
> >  + * on an event device.
> >  + *
> >  + * When application needs to setup an event producer to inject the
> >  +events to
> >  + * eventdev, application must check the event device producer
> >  +capability by
> >  + * invoking this function, On success, application can configure to
> >  +inject the
> >  + * events to eventdev by setting up the event queue using
> >  + * rte_event_queue_setup() with checked *conf*.
> >  + * On failure due to lack of adequate capability in PMD or an
> >  +application that
> >  + * wish to inject the event through application, it must create
> >  +additional
> >  + * event port by configuring the eventdev with rte_event_dev_config()
> >  +and use
> >  + * rte_event_enqueue_burst() to inject the event through
> >  + * op == RTE_EVENT_OP_NEW operation.
> >  + *
> >  + * @param dev_id
> >  + *   The identifier of the device.
> >  + * @param conf
> >  + *   The eventdev producer configuration structure.
> >  + *
> >  + * @see rte_event_queue_setup() rte_event_dev_config()
> >  + *
> >  + * @return
> >  + *   - TRUE (value different than 0) if the eventdev is capable of producing the
> >  + *   events for the given *conf*
> >  + *   - FALSE (value zero) if the eventdev is not capable of producing the events
> >  + *   for the given *conf*
> >  + *
> >  + *   PMD may have partial support for the requested capability.
> >  + *   On return FALSE, PMD sets rte_errno to reflect the partial support.
> >  + *   Possible rte_errno values include:
> >  + *   - -EOPNOTSUPP: The eventdev is not capable of pulling the events from
> >  + *   the specific producer queue. On this errno, The caller may try to
> >  + *   check the capability with rx_queue_id as -1 in *conf*.
> 
> How do we indicate the PMD has zero support? E.g., return FALSE and set rte_errno to 0?

Yes

> 
> >  + *
> >  + */
> >  +int
> >  +rte_event_dev_has_producer(uint8_t dev_id,
> >  +				struct rte_event_dev_producer_conf *conf);
> 
> I interpret this to mean the user queries the presence of a particular type of producer by setting conf->event_type. Is that correct?

For other sources, say(crypto or timer) the events are generated based on
explicit application request(i.e for crypto: application needs to submit the
crypto request and, on completion the event will be produced)
not from wire like ethdev Rx case.

So I think, mostly for ethdev Rx case only we need this scheme on other
cases when we submit the crypto job or timer job we specify where to
inject the event on work completion.
 

> 
> For an ethdev producer, I assume the PMD's response would depend on conf->ethdev_port -- that is, it could return true for a hardware ethdev and but would always return false for a software (e.g. pcap) ethdev.

Yes.

> 
> Thanks,
> Gage