Re: [dpdk-dev] [PATCH v4 3/4] eventdev: Add eventdev ethernet Rx adapter

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

-----Original Message-----
> Date: Fri, 22 Sep 2017 02:47:13 +0530
> From: Nikhil Rao <nikhil.rao@intel.com>
> To: jerin.jacob@caviumnetworks.com, bruce.richardson@intel.com
> CC: gage.eads@intel.com, dev@dpdk.org, thomas@monjalon.net,
>  harry.van.haaren@intel.com, hemant.agrawal@nxp.com, nipun.gupta@nxp.com,
>  narender.vangati@intel.com, erik.g.carrillo@intel.com,
>  abhinandan.gujjar@intel.com, santosh.shukla@caviumnetworks.com
> Subject: [PATCH v4 3/4] eventdev: Add eventdev ethernet Rx adapter
> X-Mailer: git-send-email 2.7.4
> 
> Add common APIs for configuring packet transfer from ethernet Rx
> queues to event devices across HW & SW packet transfer mechanisms.
> A detailed description of the adapter is contained in the header's
> comments.
> 
> The adapter implementation uses eventdev PMDs to configure the packet
> transfer if HW support is available and if not, it uses an EAL service
> function that reads packets from ethernet Rx queues and injects these
> as events into the event device.
> 
> 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>

Overall it looks good. A few top-level comments to start with,

1) Please split this patch to minimum two
a) Specification header
b) and the implementation

2) Please add a new section in MAINTAINERS files and update the new files
responsibility.

3) The doxygen file is not hooked into the documentation build.
Check the doc/api/doxy-api-index.md file. You can use "make doc-api-html"
to verify the doxygen html output.

4) Since the APIs looks good and if there is no other objection,
Can you add a programmer guide for Rx adapter.
If you are busy it is fine not have in next version. Post RC1 or RC2 is
fine. What do you think?


> ---
>  lib/librte_eventdev/rte_event_eth_rx_adapter.h |  384 ++++++++
>  lib/librte_eventdev/rte_event_eth_rx_adapter.c | 1238 ++++++++++++++++++++++++
>  lib/Makefile                                   |    2 +-
>  lib/librte_eventdev/Makefile                   |    2 +
>  lib/librte_eventdev/rte_eventdev_version.map   |   11 +-
>  5 files changed, 1635 insertions(+), 2 deletions(-)
>  create mode 100644 lib/librte_eventdev/rte_event_eth_rx_adapter.h
>  create mode 100644 lib/librte_eventdev/rte_event_eth_rx_adapter.c
> 
> +#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 application uses the adapter APIs to configure
> + * the packet flow between the ethernet devices and event devices. Depending on
> + * on the capabilties of the eventdev PMD, the adapter may use a EAL service

s/capabilties/capabilities

> + * core function for packet transfer or use internal PMD functions to configure
> + * the packet transfer between the ethernet device and the event device.
> + *
> + * The ethernet Rx event adapter's functions are:
> + *  - rte_event_eth_rx_adapter_create_ext()
> + *  - rte_event_eth_rx_adapter_create()/free()
> + *  - rte_event_eth_rx_adapter_queue_add()/del()
> + *  - rte_event_eth_rx_adapter_start()/stop()
> + *  - rte_event_eth_rx_adapter_stats_get()/reset()
> + *
> + * The applicaton creates an event to ethernet adapter using

How about,
The application creates an ethernet device to event device adapter using

> + * rte_event_eth_rx_adapter_create_ext() or rte_event_eth_rx_adapter_create()
> + * functions.
> + * 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.
> + * The servicing weight parameter in the rte_event_eth_rx_adapter_queue_conf
> + * is applicable when the Rx adapter uses a service core function and is
> + * 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.
> + *
> + * If the adapter uses a rte_service function, then the application is also
> + * required to assign a core to the service function and control the service
> + * core using the rte_service APIs. The rte_event_eth_rx_adapter_service_id_get
> + * function can be used to retrieve the service function ID of the adapter in
> + * this case.
> + *
> + * Note: Interrupt driven receive queues are currentely unimplemented.

s/currentely/currently

> + */
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#include <stdint.h>

A Linefeed is norm here.

> +#include <rte_service.h>
> +
> +#include "rte_eventdev.h"
> +
> +#define RTE_MAX_EVENT_ETH_RX_ADAPTER_INSTANCE 32

Considering the name space, How about to change to
RTE_EVENT_ETH_RX_ADAPTER_MAX_INSTANCE? and fix the missing the doxygen comments

> +
> +/* 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

@see rte_event_eth_rx_adapter_queue_conf::rx_queue_flags
@see rte_event_eth_rx_adapter_queue_conf::ev::flow_id


> + */
> +
> +/**
> + * Function type used for adapter configuration callback. The callback is
> + * used to fill in members of the struct rte_event_eth_rx_adapter_conf, this
> + * callback is invoked when creating a SW service for packet transfer from
> + * ethdev queues to the event device. The SW service is created within the
> + * rte_event_eth_rx_adapter_queue_add() function if packet required.

"if packet is required", does not seem to be correct usage.
I guess, you mean, if packet required to transfer from ethdev queues to
the event device or something like that?

> + *
> + * @param id
> + *  Adapter identifier.
> + *
> + * @param dev_id
> + *  Event device identifier.
> + *
> + * @conf
> + *  Structure that needs to be populated by this callback.
> + *
> + * @arg
> + *  Argument to the callback. This is the same as the conf_arg passed to the
> + *  rte_event_eth_rx_adapter_create_ext()
> + */
> +typedef int (*rx_adapter_conf_cb) (uint8_t id, uint8_t dev_id,
> +			struct rte_event_eth_rx_adapter_conf *conf,
> +			void *arg);

Public symbols should start with rte_

> +
> +/** Rx queue configuration structure */
> +struct rte_event_eth_rx_adapter_queue_conf {
> +	uint32_t rx_queue_flags;
> +	 /**< Flags for handling received packets
> +	  * @see RTE_EVENT_ETH_RX_ADAPTER_QUEUE_FLOW_ID_VALID
> +	  */
> +	uint16_t servicing_weight;
> +	/**< Relative polling frequency of ethernet receive queue, if this
> +	 * is set to zero, the Rx queue is interrupt driven (unless rx queue
> +	 * interrupts are not enabled for the ethernet device)

IMO, You can mentione it as hint and applicable only when using with
SW based Rx adapter or something on similar lines.

> +	 */
> +	struct rte_event ev;
> +	/**<
> +	 *  The values from the following event fields will be used when
> +	 *  enqueuing mbuf events:
> +	 *   - event_queue_id: Targeted event queue ID for received packets.
> +	 *   - event_priority: Event priority of packets from this Rx queue in
> +	 *                     the event queue relative to other events.
> +	 *   - sched_type: Scheduling type for packets from this Rx queue.
> +	 *   - flow_id: If the RTE_ETH_RX_EVENT_ADAPTER_QUEUE_FLOW_ID_VALID bit
> +	 *		is set in rx_queue_flags, this flow_id is used for all
> +	 *		packets received from this queue. Otherwise the flow ID
> +	 *		is set to the RSS hash of the src and dst IPv4/6
> +	 *		address.
> +	 *
> +	 * The event adapter sets ev.event_type to RTE_EVENT_TYPE_ETHDEV in the
> +	 * enqueued event

When we worked on a prototype, we figured out that we need a separate event type
for RX adapter. Probably RTE_EVENT_TYPE_ETHDEV_RX_ADAPTER?
The Reason is:
- In the HW based Rx adapter case, the packet are coming directly to eventdev once it is configured.
- So on a HW implementation of the event dequeue(), CPU needs to convert HW specific
metadata to mbuf
- The event dequeue() is used in two cases
a) octeontx eventdev driver used with any external NIC
b) octeontx eventdev driver used with integrated NIC(without service
core to inject the packet)
We need some identifier to understand case (a) and (b).So, in dequeue(), if the
packet is from RTE_EVENT_TYPE_ETHDEV then we can do "HW specific metadata" to mbuf
conversion and in another case (!RTE_EVENT_TYPE_ETHDEV) result in no mbuf
conversion.

Application can check if it is an Ethernet type event by
ev.event_type == RTE_EVENT_TYPE_ETHDEV || ev.event_type ==
RTE_EVENT_TYPE_ETHDEV_RX_ADAPTER

Thoughts?


> +/**
> + * Add receive queue to an event adapter. After a queue has been
> + * added to the event adapter, the result of the application calling
> + * rte_eth_rx_burst(eth_dev_id, rx_queue_id, ..) is undefined.
> + *
> + * @param id
> + *  Adapter identifier.
> + *
> + * @param eth_dev_id
> + *  Port identifier of Ethernet device.
> + *
> + * @param rx_queue_id
> + *  Ethernet device receive queue index.
> + *  If rx_queue_id is -1, then all Rx queues configured for
> + *  the device are added. If the ethdev Rx queues can only be
> + *  connected to a single event queue then rx_queue_id is
> + *  required to be -1.
> + *
> + * @param conf
> + *  Additonal configuration structure of type *rte_event_eth_rx_adapte_conf*
> + *
> + * @see

You can add @ see to denote the multi event queue enqueue capability

> + * @return
> + *  - 0: Success, Receive queue added correctly.
> + *  - <0: Error code on failure.
> + */
> +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);
> +
> +/**
> + * Delete receive queue from an event adapter.
> + *
> + * @param id
> + *  Adapter identifier.
> + *
> + * @param eth_dev_id
> + *  Port identifier of Ethernet device.
> + *
> + * @param rx_queue_id
> + *  Ethernet device receive queue index.
> + *  If rx_queue_id is -1, then all Rx queues configured for
> + *  the device are deleted. If the ethdev Rx queues can only be
> + *  connected to a single event queue then rx_queue_id is
> + *  required to be -1.

You can add @ see to denote the multi event queue enqueue capability

> + *
> + * @return
> + *  - 0: Success, Receive queue deleted correctly.
> + *  - <0: Error code on failure.
> + */
> +int rte_event_eth_rx_adapter_queue_del(uint8_t id, uint8_t eth_dev_id,
> +				       int32_t rx_queue_id);
> +
> +
> +/**
> + * Retrieve statistics for an adapter
> + *
> + * @param id
> + *  Adapter identifier.
> + *
> + * @param stats

@param [out] stats

> + *  A pointer to structure used to retrieve statistics for an adapter.
> + *
> + * @return
> + *  - 0: Success, retrieved successfully.
> + *  - <0: Error code on failure.
> + */
> +int rte_event_eth_rx_adapter_stats_get(uint8_t id,
> +				struct rte_event_eth_rx_adapter_stats *stats);
> +
> +
> +/**
> + * Retrieve the service ID of an adapter. If the adapter doesn't use
> + * a rte_service function, this function returns -ESRCH
> + *
> + * @param id
> + *  Adapter identifier.

@param [out] service_id

> + *
> + * @return
> + *  - 0: Success, statistics reset successfully.
> + *  - <0: Error code on failure, if the adapter doesn't use a rte_service
> + * function, this function returns -ESRCH.
> + */
> +int rte_event_eth_rx_adapter_service_id_get(uint8_t id, uint32_t *service_id);
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +#endif	/* _RTE_EVENT_ETH_RX_ADAPTER_ */
> diff --git a/lib/librte_eventdev/rte_event_eth_rx_adapter.c b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
> new file mode 100644
> index 000000000..d5b655dae
> --- /dev/null
> +
> +struct rte_event_eth_rx_adapter {
> +	/* event device identifier */

Every elements you can start with capital letter.

> +	uint8_t eventdev_id;
> +	/* per ethernet device structure */
> +	struct eth_device_info *eth_devices;
> +	/* malloc name */
> +	char mem_name[ETH_RX_ADAPTER_MEM_NAME_LEN];
> +	/* socket identifier cached from eventdev */
> +	int socket_id;
> +
> +	/* elements below are used by SW service */
> +
> +	/* event port identifier */
> +	uint8_t event_port_id;
> +	/* per adapter EAL service */
> +	uint32_t service_id;
> +	/* lock to serialize config updates with service function */
> +	rte_spinlock_t rx_lock;
> +	/* max mbufs processed in any service function invocation */
> +	uint32_t max_nb_rx;
> +	/* Receive queues that need to be polled */
> +	struct eth_rx_poll_entry *eth_rx_poll;
> +	/* size of the eth_rx_poll array */
> +	uint16_t num_rx_polled;
> +	/* Weighted round robin schedule */
> +	uint32_t *wrr_sched;
> +	/* wrr_sched[] size */
> +	uint32_t wrr_len;
> +	/* Next entry in wrr[] to begin polling */
> +	uint32_t wrr_pos;
> +	/* Event burst buffer */
> +	struct rte_eth_event_enqueue_buffer event_enqueue_buffer;
> +	/* per adapter stats */
> +	struct rte_event_eth_rx_adapter_stats stats;
> +	/* Block count, counts upto BLOCK_CNT_THRESHOLD */
> +	uint16_t enq_block_count;
> +	/* Block start ts */
> +	uint64_t rx_enq_block_start_ts;
> +	/* Configuration callback for rte_service configuration */
> +	rx_adapter_conf_cb conf_cb;
> +	/* Configuration callback argument */
> +	void *conf_arg;
> +	/* Service initialization state */
> +	uint8_t service_inited;
> +	/* Total count of Rx queues in adapter */
> +	uint32_t nb_queues;
> +} __rte_cache_aligned;
> +
> +/* Per eth device */
> +struct eth_device_info {
> +	struct rte_eth_dev *dev;
> +	struct eth_rx_queue_info *rx_queue;
> +	/* Set if ethdev->eventdev packet transfer uses a
> +	 * hardware mechanism
> +	 */
> +	uint8_t internal_event_port;
> +	/* set if the adapter is processing rx queues for

s/set/Set

> +	 * this eth device and packet processing has been
> +	 * started, allows for the code to know if the PMD
> +	 * rx_adapter_stop callback needs to be invoked
> +	 */
> +	uint8_t dev_rx_started;
> +	/* if nb_dev_queues > 0, the start callback will
> +	 * be invoked if not already invoked
> +	 */
> +	uint16_t nb_dev_queues;
> +};
> +
> +static struct rte_event_eth_rx_adapter **rte_event_eth_rx_adapter;

Avoid using rte for Internal object(**rte_event_eth_rx_adapter)_

> +static struct rte_event_port_conf
> +		create_port_conf[RTE_MAX_EVENT_ETH_RX_ADAPTER_INSTANCE];

IMO, this memory can be stored in adapter memory to avoid global variable.

> +
> +static uint8_t default_rss_key[] = {
> +	0x6d, 0x5a, 0x56, 0xda, 0x25, 0x5b, 0x0e, 0xc2,
> +	0x41, 0x67, 0x25, 0x3d, 0x43, 0xa3, 0x8f, 0xb0,
> +	0xd0, 0xca, 0x2b, 0xcb, 0xae, 0x7b, 0x30, 0xb4,
> +	0x77, 0xcb, 0x2d, 0xa3, 0x80, 0x30, 0xf2, 0x0c,
> +	0x6a, 0x42, 0xb7, 0x3b, 0xbe, 0xac, 0x01, 0xfa,
> +};

Looks like the scope of this array is only for rte_event_eth_rx_adapter_init,
if so please move it to stack.


> +static uint8_t *rss_key_be;

Can we remove this global variable add it in in adapter memory?

> +}
> +
> +static inline void
> +fill_event_buffer(struct rte_event_eth_rx_adapter *rx_adapter,
> +	uint8_t dev_id,
> +	uint16_t rx_queue_id,
> +	struct rte_mbuf **mbufs,
> +	uint16_t num)
> +{
> +	uint32_t i;
> +	struct eth_device_info *eth_device_info =
> +					&rx_adapter->eth_devices[dev_id];
> +	struct eth_rx_queue_info *eth_rx_queue_info =
> +					&eth_device_info->rx_queue[rx_queue_id];
> +
> +	int32_t qid = eth_rx_queue_info->event_queue_id;
> +	uint8_t sched_type = eth_rx_queue_info->sched_type;
> +	uint8_t priority = eth_rx_queue_info->priority;
> +	uint32_t flow_id;
> +	struct rte_event events[BATCH_SIZE];
> +	struct rte_mbuf *m = mbufs[0];
> +	uint32_t rss_mask;
> +	uint32_t rss;
> +	int do_rss;
> +
> +	/* 0xffff ffff if PKT_RX_RSS_HASH is set, otherwise 0 */
> +	rss_mask = ~(((m->ol_flags & PKT_RX_RSS_HASH) != 0) - 1);
> +	do_rss = !rss_mask && !eth_rx_queue_info->flow_id_mask;
> +
> +	for (i = 0; i < num; i++) {
> +		m = mbufs[i];
> +		struct rte_event *ev = &events[i];
> +
> +		rss = do_rss ? do_softrss(m) : m->hash.rss;
> +		flow_id =
> +		    eth_rx_queue_info->flow_id &
> +				eth_rx_queue_info->flow_id_mask;
> +		flow_id |= rss & ~eth_rx_queue_info->flow_id_mask;
> +
> +		ev->flow_id = flow_id;
> +		ev->op = RTE_EVENT_OP_NEW;
> +		ev->sched_type = sched_type;
> +		ev->queue_id = qid;
> +		ev->event_type = RTE_EVENT_TYPE_ETHDEV;

Thoughts on changing to RTE_EVENT_TYPE_ETHDEV_RX_ADAPTER as a solution for the
problem described earlier.


> +		ev->sub_event_type = 0;
> +		ev->priority = priority;
> +		ev->mbuf = m;
> +
> +		buf_event_enqueue(rx_adapter, ev);
> +	}
> +}
> +
> +/*
> + * Polls receive queues added to the event adapter and enqueues received
> + * packets to the event device.
> + *
> + * The receive code enqueues initially to a temporary buffer, the
> + * temporary buffer is drained anytime it holds >= BATCH_SIZE packets
> + *
> + * If there isn't space available in the temporary buffer, packets from the
> + * Rx queue arent dequeued from the eth device, this backpressures the

s/arent/aren't


> + * eth device, in virtual device enviroments this backpressure is relayed to the

s/enviroments/environments

> + * hypervisor's switching layer where adjustments can be made to deal with
> + * it.
> + */
> +static inline uint32_t
> +eth_rx_poll(struct rte_event_eth_rx_adapter *rx_adapter)
> +static int
> +event_eth_rx_adapter_service_func(void *args)
> +{
> +	struct rte_event_eth_rx_adapter *rx_adapter = args;
> +	struct rte_eth_event_enqueue_buffer *buf;
> +
> +	buf = &rx_adapter->event_enqueue_buffer;
> +	if (!rte_spinlock_trylock(&rx_adapter->rx_lock))
> +		return 0;
> +	if (eth_rx_poll(rx_adapter) == 0 && buf->count)
> +		flush_event_buffer(rx_adapter);
> +	rte_spinlock_unlock(&rx_adapter->rx_lock);
> +	return 0;
> +}
> +
> +static int
> +rte_event_eth_rx_adapter_init(void)
> +{
> +	const char *name = "rte_event_eth_rx_adapter_array";
> +	const struct rte_memzone *mz;
> +	unsigned int sz;
> +	unsigned int rss_key_off;
> +
> +	sz = sizeof(*rte_event_eth_rx_adapter) *
> +	    RTE_MAX_EVENT_ETH_RX_ADAPTER_INSTANCE;

I think, you need to use size of struct rte_event_eth_rx_adapter here. if so,
we need **rte_event_eth_rx_adapter here. Right?

test code
struct abc {

        uint64_t a[64];
};

struct abc **k;

int main()
{
	printf("%d %d %d\n", sizeof(k), sizeof(*k), sizeof(**k));

	return 0;
}

$./a.out
8 8 512



> +	sz = RTE_ALIGN(sz, RTE_CACHE_LINE_SIZE);
> +	rss_key_off = sz;
> +	sz = RTE_ALIGN(sz + sizeof(default_rss_key), RTE_CACHE_LINE_SIZE);
> +
> +	mz = rte_memzone_lookup(name);
> +	if (!mz) {
> +		mz = rte_memzone_reserve_aligned(name, sz, rte_socket_id(), 0,
> +						 RTE_CACHE_LINE_SIZE);

How about passing the socket id from the rte_event_dev_socket_id()?
if eventdev is in node !=0  then it may not correct thing?

> +		if (mz) {
> +			rte_convert_rss_key((uint32_t *)default_rss_key,
> +			    (uint32_t *)(uintptr_t)(mz->addr_64 + rss_key_off),
> +			    RTE_DIM(default_rss_key));
> +		} else {
> +			RTE_EDEV_LOG_ERR("failed to reserve memzone err = %"
> +					PRId32, rte_errno);
> +			return -rte_errno;
> +		}
> +	}
> +
> +	rte_event_eth_rx_adapter = mz->addr;
> +	rss_key_be = (uint8_t *)(mz->addr_64 + rss_key_off);
> +	return 0;
> +}
> +
> +static int
> +default_conf_cb(uint8_t id, uint8_t dev_id,
> +		struct rte_event_eth_rx_adapter_conf *conf, void *arg)
> +{
> +
> +	ret = rte_event_port_setup(dev_id, port_id, port_conf);
> +	if (ret) {
> +		RTE_EDEV_LOG_ERR("failed to setup event port %u\n",
> +					port_id);

return or add goto to exit from here to avoid calling rte_event_dev_start below

> +	} else {
> +		conf->event_port_id = port_id;
> +		conf->max_nb_rx = 128;
> +	}
> +
> +	if (started)
> +		rte_event_dev_start(dev_id);
> +	return ret;
> +}
> +
> +static int
> +init_service(struct rte_event_eth_rx_adapter *rx_adapter, uint8_t id)
> +{
> +		&rx_adapter_conf, rx_adapter->conf_arg);
> +	if (ret) {
> +		RTE_EDEV_LOG_ERR("confguration callback failed err = %" PRId32,

s/configuration/configuration

> +			ret);
> +		goto err_done;
> +	}
> +	rx_adapter->event_port_id = rx_adapter_conf.event_port_id;
> +	rx_adapter->max_nb_rx = rx_adapter_conf.max_nb_rx;
> +	rx_adapter->service_inited = 1;
> +	return 0;
> +
> +err_done:
> +	rte_service_component_unregister(rx_adapter->service_id);
> +	return ret;
> +}
> +
> +static void
> +update_queue_info(struct rte_event_eth_rx_adapter *rx_adapter,
> +		struct eth_device_info *dev_info,
> +		int32_t rx_queue_id,
> +		uint8_t add)
> +{
> +	struct eth_rx_queue_info *queue_info;
> +	int enabled;
> +	uint16_t i;
> +
> +	if (!dev_info->rx_queue)
> +		return;
> +
> +	if (rx_queue_id == -1) {
> +		for (i = 0; i < dev_info->dev->data->nb_rx_queues; i++) {
> +			queue_info = &dev_info->rx_queue[i];
> +			enabled = queue_info->queue_enabled;
> +			if (add) {
> +				rx_adapter->nb_queues += !enabled;
> +				dev_info->nb_dev_queues += !enabled;
> +			} else {
> +				rx_adapter->nb_queues -= enabled;
> +				dev_info->nb_dev_queues -= enabled;
> +			}
> +			queue_info->queue_enabled = !!add;

See next comment.

> +		}
> +	} else {
> +		queue_info = &dev_info->rx_queue[rx_queue_id];
> +		enabled = queue_info->queue_enabled;
> +		if (add) {
> +			rx_adapter->nb_queues += !enabled;
> +			dev_info->nb_dev_queues += !enabled;
> +		} else {
> +			rx_adapter->nb_queues -= enabled;
> +			dev_info->nb_dev_queues -= enabled;
> +		}
> +		queue_info->queue_enabled = !!add;

Duplicate code. Worth to make it static inline to avoid duplicate code

> +	}
> +}
> +
> +int
> +rte_event_eth_rx_adapter_create_ext(uint8_t id, uint8_t dev_id,
> +				rx_adapter_conf_cb conf_cb, void *conf_arg)
> +{
> +	struct rte_event_eth_rx_adapter *rx_adapter;
> +	int ret;
> +	int socket_id;
> +	uint8_t i;
> +	char mem_name[ETH_RX_ADAPTER_SERVICE_NAME_LEN];
> +
> +	RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(id, -EINVAL);
> +	RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
> +	if (!conf_cb)

Remove negative logic and change to conf_cb == NULL. Its DPDK coding
standard. There is a lot similar instance in this file.Please fix those

> +		return -EINVAL;
> +
> +	if (rte_event_eth_rx_adapter == NULL) {
> +		ret = rte_event_eth_rx_adapter_init();
> +		if (ret)
> +			return ret;
> +	}
> +
> +	rx_adapter = id_to_rx_adapter(id);
> +	if (rx_adapter != NULL) {
> +int
> +rte_event_eth_rx_adapter_create(uint8_t id, uint8_t dev_id,
> +		struct rte_event_port_conf *port_config)
> +{
> +	if (!port_config)

port_config == NULL

> +		return -EINVAL;
> +	RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(id, -EINVAL);
> +
> +	create_port_conf[id] = *port_config;
> +	return rte_event_eth_rx_adapter_create_ext(id, dev_id,
> +					default_conf_cb,
> +					&create_port_conf[id]);
> +}
> +
> +
> +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 *queue_conf)
> +{
> +	int ret;
> +	uint32_t rx_adapter_cap;
> +	struct rte_event_eth_rx_adapter *rx_adapter;
> +	struct rte_eventdev *dev;
> +	struct eth_device_info *dev_info;
> +	int start_service = 0;

Duplicate store to zero.

> +
> +	RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(id, -EINVAL);
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(eth_dev_id, -EINVAL);
> +
> +	rx_adapter = id_to_rx_adapter(id);
> +	if (!rx_adapter || !queue_conf)
> +		return -EINVAL;
> +
> +	dev = &rte_eventdevs[rx_adapter->eventdev_id];
> +	ret = (*dev->dev_ops->eth_rx_adapter_caps_get)(dev,
> +						&rte_eth_devices[eth_dev_id],
> +						&rx_adapter_cap);
> +	if (ret) {
> +		RTE_EDEV_LOG_ERR("Failed to get adapter caps edev %" PRIu8
> +			"eth port %" PRIu8, id, eth_dev_id);
> +		return ret;
> +	}
> +
> +	if (!(rx_adapter_cap & RTE_EVENT_ETH_RX_ADAPTER_CAP_FLOW_ID) &&
> +		!(queue_conf->rx_queue_flags &
> +			RTE_EVENT_ETH_RX_ADAPTER_QUEUE_FLOW_ID_VALID)) {
> +		RTE_EDEV_LOG_ERR("Flow ID required for configuration,"
> +				" eth port: %" PRIu8 " adapter id: %" PRIu8,
> +				eth_dev_id, id);
> +		return -EINVAL;
> +	}
> +
> +	if ((rx_adapter_cap & RTE_EVENT_ETH_RX_ADAPTER_CAP_SINGLE_EVENTQ) &&
> +		(rx_queue_id != -1)) {
> +		RTE_EDEV_LOG_ERR("Rx queues can only be connected to single "
> +			"event queue id %u eth port %u", id, eth_dev_id);
> +		return -EINVAL;
> +	}
> +
> +	if (rx_queue_id != -1 && (uint16_t)rx_queue_id >=
> +			rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
> +		RTE_EDEV_LOG_ERR("Invalid rx queue_id %" PRIu16,
> +			 (uint16_t)rx_queue_id);
> +		return -EINVAL;
> +	}
> +
> +	start_service = 0;

See above comment.