From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 67C699E4 for ; Mon, 18 Jun 2018 15:15:37 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 18 Jun 2018 06:15:35 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,239,1526367600"; d="scan'208";a="58543667" Received: from abaagwal-mobl1.gar.corp.intel.com (HELO [10.252.76.57]) ([10.252.76.57]) by fmsmga002.fm.intel.com with ESMTP; 18 Jun 2018 06:15:33 -0700 To: Jerin Jacob Cc: dev@dpdk.org References: <1528481718-7241-1-git-send-email-nikhil.rao@intel.com> <1528481718-7241-6-git-send-email-nikhil.rao@intel.com> <20180617134958.GF7498@jerin> From: "Rao, Nikhil" Message-ID: Date: Mon, 18 Jun 2018 18:45:32 +0530 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: <20180617134958.GF7498@jerin> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US Subject: Re: [dpdk-dev] [PATCH v1 4/4] eventdev: add interrupt driven queues in Rx event adapter X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 18 Jun 2018 13:15:38 -0000 On 6/17/2018 7:19 PM, Jerin Jacob wrote: > -----Original Message----- >> Date: Fri, 8 Jun 2018 23:45:18 +0530 >> From: Nikhil Rao >> To: jerin.jacob@caviumnetworks.com >> CC: dev@dpdk.org, Nikhil Rao >> 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 >> --- >> 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? OK. >> */ >> >> @@ -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 >> +#include >> #include >> #include >> #include >> @@ -11,6 +13,7 @@ >> #include >> #include >> #include >> +#include >> >> #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? Interrupt functionality works only on Linux (rte_epoll_wait() etc are implemented only in Linux.) or I am not understanding your question correctly ? >> + 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 ? OK. >> # >> 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 >> Thanks for the review. Nikhil