Re: [PATCH 1/2] eventdev: add event adapter for ethernet Rx queues

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

Hi Nikhil/Edas,

> -----Original Message-----
> From: Nikhil Rao [mailto:nikhil.rao@intel.com]
> Sent: Friday, July 07, 2017 3:23
> To: jerin.jacob@caviumnetworks.com
> Cc: gage.eads@intel.com; dev@dpdk.org; thomas@monjalon.net;
> bruce.richardson@intel.com; harry.van.haaren@intel.com; Hemant Agrawal
> <hemant.agrawal@nxp.com>; Nipun Gupta <nipun.gupta@nxp.com>;
> narender.vangati@intel.com; Nikhil Rao <nikhil.rao@intel.com>; Abhinandan
> Gujjar <abhinandan.gujjar@intel.com>
> Subject: [PATCH 1/2] eventdev: add event adapter for ethernet Rx queues
> 
> Eventdev-based networking applications require a component to dequeue
> packets from NIC Rx queues and inject them into eventdev queues[1]. While
> some platforms (e.g. Cavium Octeontx) do this operation in hardware, other
> platforms use software.
> 
> This patchset introduces an ethernet Rx event adapter that dequeues packets
> from ethernet devices and enqueues them to event devices. It is based on
> a previous RFC[2].
> 
> The adapter is designed to work with the EAL service core[3]. If
> an application determines that the adapter is required, it can register and
> launch it on a service core. Alternatively, this adapter can serve as a
> template for applications to design customer ethernet Rx event adapters
> better suited to their needs.
> 
> The adapter can service multiple ethernet devices and queues. Each queue is
> configured with a servicing weight to control the relative frequency with
> which the adapter polls the queue, and the event fields to use when
> constructing packet events. The adapter has two modes for programming an
> event's flow ID: use a static per-queue user-specified value or use the RSS
> hash.
> 
> A detailed description of the adapter is contained in the header's
> comments.
> 
> [1] http://dpdk.org/ml/archives/dev/2017-May/065341.html
> [2] http://dpdk.org/ml/archives/dev/2017-May/065539.html
> [3] http://dpdk.org/ml/archives/dev/2017-July/069782.html
> 
> Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
> Signed-off-by: Gage Eads <gage.eads@intel.com>
> Signed-off-by: Abhinandan Gujjar <abhinandan.gujjar@intel.com>
> ---
> 
> v2:
> Thanks Jerin for review - below is a list of changes you
> suggested.
> 
> - all public symbols are started with rte_event_.
> - Add Doxygen reference with @see.
> - Mention setting of ev.event_type.
> - Mention adapter to service function mapping.
> - Remove rte_eth_rx_event_adapter_dev_add/del().
> - Change rx_queuee_id to int32_t and use -1 to denote all Rx queues.
> - Add rte_eth_event_rx_queue_del().
> 
> Other changes
> - Remove adapter's run function (rte_event_eth_rx_adapter_run()) from
>   the public interface. The adapter internally uses it to create a
>   service.
> - Add a blocked cycle count to stats. Further description is contained
>   in the header.
> - Minor struct renames rte_event_eth_rx_adapter_config -> .._conf
> ---
>  lib/librte_eventdev/rte_event_eth_rx_adapter.h | 301 +++++++++
>  lib/librte_eventdev/rte_event_eth_rx_adapter.c | 825
> +++++++++++++++++++++++++
>  lib/librte_eventdev/rte_eventdev_version.map   |  13 +
>  lib/Makefile                                   |   2 +-
>  lib/librte_eventdev/Makefile                   |   2 +
>  5 files changed, 1142 insertions(+), 1 deletion(-)
>  create mode 100644 lib/librte_eventdev/rte_event_eth_rx_adapter.h
>  create mode 100644 lib/librte_eventdev/rte_event_eth_rx_adapter.c
> 
> diff --git a/lib/librte_eventdev/rte_event_eth_rx_adapter.h
> b/lib/librte_eventdev/rte_event_eth_rx_adapter.h
> new file mode 100644
> index 000000000..5ccd0bd24
> --- /dev/null
> +++ b/lib/librte_eventdev/rte_event_eth_rx_adapter.h
> @@ -0,0 +1,301 @@
> +/*
> + *   Copyright(c) 2017 Intel Corporation. All rights reserved.
> + *   All rights reserved.
> + *
> + *   Redistribution and use in source and binary forms, with or without
> + *   modification, are permitted provided that the following conditions
> + *   are met:
> + *
> + *     * Redistributions of source code must retain the above copyright
> + *       notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copyright
> + *       notice, this list of conditions and the following disclaimer in
> + *       the documentation and/or other materials provided with the
> + *       distribution.
> + *     * Neither the name of Intel Corporation nor the names of its
> + *       contributors may be used to endorse or promote products derived
> + *       from this software without specific prior written permission.
> + *
> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> CONTRIBUTORS
> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
> FITNESS FOR
> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> COPYRIGHT
> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> INCIDENTAL,
> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> NOT
> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
> OF USE,
> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
> ON ANY
> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
> TORT
> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
> THE USE
> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> DAMAGE.
> + */
> +
> +#ifndef _RTE_EVENT_ETH_RX_ADAPTER_
> +#define _RTE_EVENT_ETH_RX_ADAPTER_
> +
> +/**
> + * @file
> + *
> + * RTE Event Ethernet Rx Adapter
> + *
> + * An eventdev-based packet processing application enqueues/dequeues mbufs
> + * to/from the event device. The ethernet Rx event adapter's role is to transfer
> + * mbufs from the ethernet receive queues managed by DPDK to an event
> device.
> + * The application uses the adapter APIs to configure the packet flow between
> + * the ethernet devices and event devices. The adapter is designed to work with
> + * the EAL service cores. The adapter's work can be parallelized by dividing the
> + * NIC Rx queues among multiple adapter services that run in parallel.

First of all, apologies for commenting late on this. I am going through this patchset and have some concerns over this.
It is mentioned that "The adapter is designed to work with * the EAL service cores"
Does this mean that the adapter cannot function without service core?
In the case where Ethdev HW is capable of injecting the packet to compatible HW eventdev driver, there is no service
required. It seems this patchset does not take care of this use-case or maybe I am missing something here?

> + *
> + * Before using the adapter, the application needs to enumerate and configure
> + * the ethernet devices that it wishes to use. This is typically done using the
> + * following DPDK ethdev functions:
> + *  - rte_eth_dev_configure()
> + *  - rte_eth_tx_queue_setup()
> + *  - rte_eth_rx_queue_setup()
> + *  - rte_eth_dev_start()
> + *
> + * The application also configures an event device and creates event ports
> + * to interface with the event device. In addition to the event ports used by
> + * its packet processing functions, the application creates an event port
> + * to be used by this adapter.
> + *
> + * The ethernet Rx event adapter's functions are:
> + *  - rte_event_eth_rx_adapter_create()
> + *  - rte_event_eth_rx_adapter_free()
> + *  - rte_event_eth_rx_adapter_queue_add()
> + *  - rte_event_eth_rx_adapter_queue_del()
> + *  - rte_event_eth_rx_adapter_stats_get()
> + *  - rte_event_eth_rx_adapter_stats_reset()
> + *
> + * The applicaton creates an event to ethernet adapter using
> + * rte_event_eth_rx_adapter_create(). The event device and event port
> + * identifiers are passed to this function.
> + *
> + * The adapter needs to know which ethernet rx queues to poll for mbufs as
> well
> + * as event device parameters such as the event queue identifier, event
> + * priority and scheduling type that the adapter should use when constructing
> + * events. The rte_event_eth_rx_adapter_queue_add() function is provided for
> + * this purpose.
> + *
> + * At the time of adding an ethernet device receive queue, the application can
> + * also specify a static event flow id and set the
> + * RTE_ETH_RX_EVENT_ADAPTER_QUEUE_FLOW_ID_VALID bit of the
> rx_queue_flags
> + * member of the rte_event_eth_rx_adapter_queue_conf structure. If the
> + * RTE_ETH_RX_EVENT_ADAPTER_QUEUE_FLOW_ID_VALID isn't set, the flow
> id is
> + * assigned the value of the RSS hash. The adapter generates the RSS hash if it
> + * hasn't been already computed by the NIC, based on source and destination
> + * IPv4/6 addresses, using the rte_softrss_be() routine included in the DPDK.
> + *
> + * The servicing weight parameter in the
> rte_event_eth_rx_adapter_queue_conf
> + * intended to provide application control of the polling frequency of ethernet
> + * device receive queues, for example, the application may want to poll higher
> + * priority queues with a higher frequency but at the same time not starve
> + * lower priority queues completely. If this parameter is zero and the receive
> + * interrupt is enabled when configuring the device, the receive queue is
> + * interrupt driven; else, the queue is assigned a servicing weight of one.
> + *
> + * Note: Interrupt driven receive queues are currentely unimplemented.
> + */
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#include <stdint.h>
> +#include <rte_service.h>
> +
> +#include "rte_eventdev.h"
> +
> +
> +#define RTE_MAX_EVENT_ETH_RX_ADAPTER_INSTANCE 32
> +
> +/* struct rte_event_eth_rx_adapter_queue_conf flags definitions */
> +#define RTE_EVENT_ETH_RX_ADAPTER_QUEUE_FLOW_ID_VALID	0x1
> +/**< This flag indicates the flow identifier is valid
> + * @see rte_event_eth_rx_adapter_queue_conf
> + */

<snip>

> +
> +static int
> +_rte_event_eth_rx_adapter_queue_add(struct rte_event_eth_rx_adapter
> *rx_adapter,
> +				    struct eth_device_info *dev_info,
> +				    uint16_t rx_queue_id,
> +				    const struct
> rte_event_eth_rx_adapter_queue_conf *conf)
> +{
> +	int ret;
> +	struct eth_rx_queue_info *queue_info;
> +	const struct rte_event *ev;
> +
> +	if (rx_queue_id >= dev_info->dev->data->nb_rx_queues)
> +		return -EINVAL;
> +
> +	queue_info = &dev_info->rx_queue[rx_queue_id];
> +	if (queue_info->queue_enabled)
> +		return -EEXIST;
> +
> +	ev = &conf->ev;
> +	memset(queue_info, 0, sizeof(*queue_info));
> +	queue_info->event_queue_id = ev->queue_id;
> +	queue_info->sched_type = ev->sched_type;
> +	queue_info->priority = ev->priority;
> +	queue_info->wt = conf->servicing_weight;
> +
> +	if (queue_info->wt == 0) {
> +		struct rte_eth_dev_data *data = dev_info->dev->data;
> +
> +		/* If Rx interrupts are disabled set wt = 1 */
> +		queue_info->wt = !data->dev_conf.intr_conf.rxq;
> +	}
> +
> +	if (conf->
> +	    rx_queue_flags &
> RTE_EVENT_ETH_RX_ADAPTER_QUEUE_FLOW_ID_VALID) {
> +		queue_info->flow_id = ev->flow_id;
> +		queue_info->flow_id_mask = ~0;
> +	}
> +
> +	queue_info->queue_enabled = true;
> +	rx_adapter->num_rx_polled++;
> +	ret = eth_poll_wrr_calc(rx_adapter);
> +	if (ret) {
> +		rx_adapter->num_rx_polled--;
> +		queue_info->queue_enabled = false;
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +int
> +rte_event_eth_rx_adapter_queue_add(uint8_t id,
> +				   uint8_t eth_dev_id,
> +				   int32_t rx_queue_id,
> +				   const struct
> rte_event_eth_rx_adapter_queue_conf *conf)
> +{
> +	int ret = 0;
> +	struct rte_event_eth_rx_adapter *rx_adapter;
> +	struct eth_device_info *dev_info;
> +	uint32_t i, j;
> +
> +	rx_adapter = rte_event_eth_rx_adapter[id];
> +	if (!valid_id(id) || !rte_event_eth_rx_adapter || !rx_adapter ||
> +	    eth_dev_id >= rte_eth_dev_count() || !conf)
> +		return -EINVAL;
> +
> +	rte_spinlock_lock(&rx_adapter->rx_lock);
> +
> +	dev_info = &rx_adapter->eth_devices[eth_dev_id];
> +	if (rx_queue_id == -1) {
> +		for (i = 0; i < dev_info->dev->data->nb_rx_queues; i++) {
> +			ret =
> +			    _rte_event_eth_rx_adapter_queue_add(rx_adapter,
> dev_info, i,
> +								conf);
> +			if (ret) {
> +				for (j = 0; j < i; j++)
> +
> 	_rte_event_eth_rx_adapter_queue_del(rx_adapter,
> +
> dev_info,
> +									    j);
> +			}
> +		}
> +
> +	} else {
> +		ret = _rte_event_eth_rx_adapter_queue_add(rx_adapter,
> dev_info,
> +
> (uint16_t)rx_queue_id,
> +							  conf);
> +	}
> +
> +	rte_spinlock_unlock(&rx_adapter->rx_lock);
> +
> +	return ret;
> +}

Looking at the rte_event_eth_rx_adapter_queue_add & event_eth_rx_adapter_service_func
it seems that this indeed will not fit with the cases where ethdev is capable of enqueing packets
to the eventdev (as was mentioned in Jerin's first RFC).

In case of hardware based eventdev and queues, these function should also invoke respective PMD
HW configs. e.g. In queue case - rte_eventdev and rte_ethdev - both PMDs at hw level shall be configured.

A typical eventdev hardware will require queues of eth devices will be configured to directly attach to
eventdev in the hardware.

Mapping it to NXP eventdev, enabling this functionality requires some configuration where dev private
information of both the devices (event dev and eth dev) is required at the same time,
and the final configuration is provided via eth device to H/W. So, this require inter device communication in DPDK.

Jerin,

I have an impression that Cavium hardware has H/W capability to inject packets from Ethernet
devices to event devices? If yes, how do you plan to support it?

Thanks,
Nipun