Re: [dpdk-dev] [PATCH v1 4/4] eventdev: add interrupt driven queues in Rx event adapter

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

-----Original Message-----
> Date: Fri, 8 Jun 2018 23:45:18 +0530
> From: Nikhil Rao <nikhil.rao@intel.com>
> To: jerin.jacob@caviumnetworks.com
> CC: dev@dpdk.org, Nikhil Rao <nikhil.rao@intel.com>
> Subject: [PATCH v1 4/4] eventdev: add interrupt driven queues in Rx event
>  adapter
> X-Mailer: git-send-email 1.8.3.1
> 
> Add support for interrupt driven queues when eth device is
> configured for rxq interrupts and servicing weight for the
> queue is configured to be zero.
> 
> A interrupt driven packet received counter has been added to
> rte_event_eth_rx_adapter_stats.
> 
> Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
> ---
>  lib/librte_eventdev/rte_event_eth_rx_adapter.h     |    5 +-
>  lib/librte_eventdev/rte_event_eth_rx_adapter.c     | 1049 +++++++++++++++++++-
>  test/test/test_event_eth_rx_adapter.c              |  261 ++++-

Please move the testcase to separate patch.

>  .../prog_guide/event_ethernet_rx_adapter.rst       |   24 +
>  config/common_base                                 |    1 +

This patch creates build issue with meson build.
command to reproduce:
--------------------
export MESON_PARAMS='-Dwerror=true -Dexamples=all'
CC="ccache gcc" meson --default-library=shared $MESON_PARAMS gcc-shared-build
ninja -C gcc-shared-build

log:
---
../lib/librte_eventdev/rte_event_eth_rx_adapter.c: In function
‘rxa_intr_ring_check_avail’:
../lib/librte_eventdev/rte_event_eth_rx_adapter.c:916:5: error:
‘RTE_EVENT_ETH_INTR_RING_SIZE’ undeclared (first use in this function);
did you mean ‘RTE_EVENT_DEV_XSTATS_NAME_SIZE’?
     RTE_EVENT_ETH_INTR_RING_SIZE) {
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
     RTE_EVENT_DEV_XSTATS_NAME_SIZE
../lib/librte_eventdev/rte_event_eth_rx_adapter.c:916:5: note: each
undeclared identifier is reported only once for each function it appears
in
../lib/librte_eventdev/rte_event_eth_rx_adapter.c: In function
‘rxa_intr_thread’:
../lib/librte_eventdev/rte_event_eth_rx_adapter.c:971:8: error:
‘RTE_EVENT_ETH_INTR_RING_SIZE’ undeclared (first use in this function);
did you mean ‘RTE_EVENT_DEV_XSTATS_NAME_SIZE’?
        RTE_EVENT_ETH_INTR_RING_SIZE + 1, -1);
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
>  6 files changed, 1296 insertions(+), 48 deletions(-)
> 
> diff --git a/lib/librte_eventdev/rte_event_eth_rx_adapter.h b/lib/librte_eventdev/rte_event_eth_rx_adapter.h
> index 307b2b5..97f25e9 100644
> --- a/lib/librte_eventdev/rte_event_eth_rx_adapter.h
> +++ b/lib/librte_eventdev/rte_event_eth_rx_adapter.h
> @@ -64,8 +64,7 @@
>   * the service function ID of the adapter in this case.
>   *
>   * Note:
> - * 1) Interrupt driven receive queues are currently unimplemented.
> - * 2) Devices created after an instance of rte_event_eth_rx_adapter_create
> + * 1) Devices created after an instance of rte_event_eth_rx_adapter_create
>   *  should be added to a new instance of the rx adapter.

Can we remove this NOTE and add this check in the code if it is not the
case?

>   */
>  
> @@ -199,6 +198,8 @@ struct rte_event_eth_rx_adapter_stats {
>  	 * block cycles can be used to compute the percentage of
>  	 * cycles the service is blocked by the event device.
>  	 */
> +	uint64_t rx_intr_packets;
> +	/**< Received packet count for interrupt mode Rx queues */
>  };
>  
>  /**
> diff --git a/lib/librte_eventdev/rte_event_eth_rx_adapter.c b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
> index 40e9bc9..d038ee4 100644
> --- a/lib/librte_eventdev/rte_event_eth_rx_adapter.c
> +++ b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
> @@ -2,6 +2,8 @@
>   * Copyright(c) 2017 Intel Corporation.
>   * All rights reserved.
>   */
> +#include <unistd.h>
> +#include <sys/epoll.h>
>  #include <rte_cycles.h>
>  #include <rte_common.h>
>  #include <rte_dev.h>
> @@ -11,6 +13,7 @@
>  #include <rte_malloc.h>
>  #include <rte_service_component.h>
>  #include <rte_thash.h>
> +#include <rte_interrupts.h>
>  
>  #include "rte_eventdev.h"
>  #include "rte_eventdev_pmd.h"
> @@ -24,6 +27,36 @@
>  #define ETH_RX_ADAPTER_MEM_NAME_LEN	32
> +static void *
> +rxa_intr_thread(void *arg)
> +{
> +	struct rte_event_eth_rx_adapter *rx_adapter = arg;
> +	struct rte_epoll_event *epoll_events = rx_adapter->epoll_events;
> +	int n, i;
> +	uint8_t val;
> +	ssize_t bytes_read;
> +
> +	while (1) {
> +		n = rte_epoll_wait(rx_adapter->epd, epoll_events,
> +				   RTE_EVENT_ETH_INTR_RING_SIZE + 1, -1);

Can you check with FreeBSD if everything is fine or not?

> +		if (unlikely(n < 0))
> +			RTE_EDEV_LOG_ERR("rte_epoll_wait returned error %d",
> +					n);
> +		for (i = 0; i < n; i++) {
> +			if (epoll_events[i].fd == rx_adapter->intr_pipe.readfd)
> +				goto done;
> +			rxa_intr_ring_enqueue(rx_adapter,
> +					epoll_events[i].epdata.data);
> +		}
> +	}
> +
> +done:
> +
> +static int
> +rxa_create_intr_thread(struct rte_event_eth_rx_adapter *rx_adapter)
> +{
> +	int err;
> +	uint8_t val;
> +	char thread_name[RTE_MAX_THREAD_NAME_LEN];
> +
> +	if (rx_adapter->intr_ring)
> +		return 0;
> +
> +	rx_adapter->intr_ring = rte_ring_create("intr_ring",
> +					RTE_EVENT_ETH_INTR_RING_SIZE,
> +					rte_socket_id(), 0);
> +	if (!rx_adapter->intr_ring)
> +		return -ENOMEM;
> +
> +	rx_adapter->epoll_events = rte_zmalloc_socket(rx_adapter->mem_name,
> +					(RTE_EVENT_ETH_INTR_RING_SIZE + 1) *
> +					sizeof(struct rte_epoll_event),
> +					RTE_CACHE_LINE_SIZE,
> +					rx_adapter->socket_id);
> +	if (!rx_adapter->epoll_events) {
> +		err = -ENOMEM;
> +		goto error;
> +	}
> +
> +	rte_spinlock_init(&rx_adapter->intr_ring_lock);
> +
> +	snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN,
> +			"rx-intr-thread");
> +	err = pthread_create(&rx_adapter->rx_intr_thread, NULL,
> +			rxa_intr_thread, rx_adapter);


Can you replace the pthread_* with new rte_ctrl_thread_create()
abstraction ?

>  #
> diff --git a/lib/librte_eventdev/Makefile b/lib/librte_eventdev/Makefile
> index b3e2546..e269357 100644
> --- a/lib/librte_eventdev/Makefile
> +++ b/lib/librte_eventdev/Makefile
> @@ -8,14 +8,16 @@ include $(RTE_SDK)/mk/rte.vars.mk
>  LIB = librte_eventdev.a
>  
>  # library version
> -LIBABIVER := 4
> +LIBABIVER := 5
>  
>  # build flags
>  CFLAGS += -DALLOW_EXPERIMENTAL_API
> +CFLAGS += -D_GNU_SOURCE
>  CFLAGS += -O3
>  CFLAGS += $(WERROR_FLAGS)
>  LDLIBS += -lrte_eal -lrte_ring -lrte_ethdev -lrte_hash -lrte_mempool -lrte_timer
>  LDLIBS += -lrte_mbuf -lrte_cryptodev
> +LDLIBS += -lpthread

This may not be required if we add rte_ctrl_thread library.

>  
>  # library source files
>  SRCS-y += rte_eventdev.c
> -- 
> 1.8.3.1
>