From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 2FB3442EDC; Fri, 21 Jul 2023 19:34:04 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0C59A40DDC; Fri, 21 Jul 2023 19:34:04 +0200 (CEST) Received: from agw.arknetworks.am (agw.arknetworks.am [79.141.165.80]) by mails.dpdk.org (Postfix) with ESMTP id 76F9640DD8 for ; Fri, 21 Jul 2023 19:34:02 +0200 (CEST) Received: from debian (unknown [78.109.74.86]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by agw.arknetworks.am (Postfix) with ESMTPSA id 64B06E123E; Fri, 21 Jul 2023 21:34:01 +0400 (+04) Date: Fri, 21 Jul 2023 21:33:54 +0400 (+04) From: Ivan Malov To: Ferruh Yigit cc: Thomas Monjalon , Andrew Rybchenko , dev@dpdk.org, David Marchand , Jie Hai , Song Jiale , Yuan Peng , Raslan Darawsheh , Qiming Yang Subject: Re: [RFC] ethdev: clarify device queue state after start and stop In-Reply-To: <20230721160422.3848154-1-ferruh.yigit@amd.com> Message-ID: References: <20230721160422.3848154-1-ferruh.yigit@amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Hi Ferruh, The commit log says "commit [1]" and "commit [2]" but does not seem to provide the URLs for the [1] and [2]. What are these resources? I'd like to know. Thank you. On Fri, 21 Jul 2023, Ferruh Yigit wrote: > Drivers start/stop device queues on port start/stop, but not all drivers > update queue state accordingly. > > This becomes more visible if a specific queue stopped explicitly and > port stopped/started later, in this case although all queues are > started, the state of that specific queue is stopped and it is > misleading. > > Misrepresentation of queue state became a defect with commit [1] that > does forwarding decision based on queue state and commit [2] that gets > up to date queue state from ethdev/device before forwarding. > > This patch documents that status of all queues of a device should be > `RTE_ETH_QUEUE_STATE_STOPPED` after port stop and their status should > be`RTE_ETH_QUEUE_STATE_STARTED` after port start. > > Also an unit test added to verify drivers. > > Signed-off-by: Ferruh Yigit > --- > Cc: Jie Hai > Cc: Song Jiale > Cc: Yuan Peng > Cc: Raslan Darawsheh > Cc: Qiming Yang > --- > app/test/meson.build | 2 + > app/test/test_ethdev_api.c | 169 +++++++++++++++++++++++++++++++++++++ > lib/ethdev/rte_ethdev.h | 5 ++ > 3 files changed, 176 insertions(+) > create mode 100644 app/test/test_ethdev_api.c > > diff --git a/app/test/meson.build b/app/test/meson.build > index b89cf0368fb5..8e409cf59c35 100644 > --- a/app/test/meson.build > +++ b/app/test/meson.build > @@ -49,6 +49,7 @@ test_sources = files( > 'test_efd_perf.c', > 'test_errno.c', > 'test_ethdev_link.c', > + 'test_ethdev_api.c', > 'test_event_crypto_adapter.c', > 'test_event_eth_rx_adapter.c', > 'test_event_ring.c', > @@ -187,6 +188,7 @@ fast_tests = [ > ['eal_fs_autotest', true, true], > ['errno_autotest', true, true], > ['ethdev_link_status', true, true], > + ['ethdev_api', true, true], > ['event_ring_autotest', true, true], > ['fib_autotest', true, true], > ['fib6_autotest', true, true], > diff --git a/app/test/test_ethdev_api.c b/app/test/test_ethdev_api.c > new file mode 100644 > index 000000000000..1b4569396dda > --- /dev/null > +++ b/app/test/test_ethdev_api.c > @@ -0,0 +1,169 @@ > +/* SPDX-License-Identifier: BSD-3-Clause > + * Copyright (C) 2023, Advanced Micro Devices, Inc. > + */ > + > +#include > +#include > + > +#include > +#include "test.h" > + > +#define NUM_RXQ 2 > +#define NUM_TXQ 2 > +#define NUM_RXD 512 > +#define NUM_TXD 512 > +#define NUM_MBUF 1024 > +#define MBUF_CACHE_SIZE 256 > + > +static int32_t > +ethdev_api_queue_status(void) > +{ > + struct rte_eth_dev_info dev_info; > + struct rte_eth_rxq_info rx_qinfo; > + struct rte_eth_txq_info tx_qinfo; > + struct rte_mempool *mbuf_pool; > + /*struct rte_eth_rxconf rx_conf;*/ > + /*struct rte_eth_txconf tx_conf;*/ > + struct rte_eth_conf eth_conf; > + uint16_t port_id; > + int ret; > + > + mbuf_pool = rte_pktmbuf_pool_create("MBUF_POOL", NUM_MBUF, MBUF_CACHE_SIZE, 0, > + RTE_MBUF_DEFAULT_BUF_SIZE, rte_socket_id()); > + > + RTE_ETH_FOREACH_DEV(port_id) { > + memset(ð_conf, 0, sizeof(dev_info)); > + ret = rte_eth_dev_configure(port_id, NUM_RXQ, NUM_TXQ, ð_conf); > + TEST_ASSERT(ret == 0, > + "Port(%u) failed to configure.\n", port_id); > + > + /* RxQ setup */ > + /*memset(&rx_conf, 0, sizeof(rx_conf));*/ > + for (uint16_t queue_id = 0; queue_id < NUM_RXQ; queue_id++) { > + ret = rte_eth_rx_queue_setup(port_id, queue_id, NUM_RXD, > + rte_socket_id(), NULL, mbuf_pool); > + TEST_ASSERT(ret == 0, > + "Port(%u), queue(%u) failed to setup RxQ.\n", > + port_id, queue_id); > + } > + > + /* TxQ setup */ > + /*memset(&tx_conf, 0, sizeof(tx_conf));*/ > + for (uint16_t queue_id = 0; queue_id < NUM_TXQ; queue_id++) { > + ret = rte_eth_tx_queue_setup(port_id, queue_id, NUM_TXD, > + rte_socket_id(), NULL); > + TEST_ASSERT(ret == 0, > + "Port(%u), queue(%u) failed to setup TxQ.\n", > + port_id, queue_id); > + } > + > + ret = rte_eth_dev_info_get(port_id, &dev_info); > + TEST_ASSERT(ret == 0, > + "Port(%u) failed to get dev info.\n", port_id); > + > + /* Initial RxQ */ > + for (uint16_t queue_id = 0; queue_id < dev_info.nb_rx_queues; queue_id++) { > + ret = rte_eth_rx_queue_info_get(port_id, queue_id, &rx_qinfo); > + TEST_ASSERT(ret == 0, > + "Port(%u), queue(%u) failed to get RxQ info.\n", > + port_id, queue_id); > + > + TEST_ASSERT(rx_qinfo.queue_state == RTE_ETH_QUEUE_STATE_STOPPED, > + "Wrong initial Rx queue(%u) state(%d)\n", > + queue_id, rx_qinfo.queue_state); > + } > + > + /* Initial TxQ */ > + for (uint16_t queue_id = 0; queue_id < dev_info.nb_tx_queues; queue_id++) { > + ret = rte_eth_tx_queue_info_get(port_id, queue_id, &tx_qinfo); > + TEST_ASSERT(ret == 0, > + "Port(%u), queue(%u) failed to get TxQ info.\n", > + port_id, queue_id); > + > + TEST_ASSERT(tx_qinfo.queue_state == RTE_ETH_QUEUE_STATE_STOPPED, > + "Wrong initial Tx queue(%u) state(%d)\n", > + queue_id, tx_qinfo.queue_state); > + } > + > + > + ret = rte_eth_dev_start(port_id); > + TEST_ASSERT(ret == 0, > + "Port(%u) failed to start.\n", port_id); > + > + /* Started RxQ */ > + for (uint16_t queue_id = 0; queue_id < dev_info.nb_rx_queues; queue_id++) { > + ret = rte_eth_rx_queue_info_get(port_id, queue_id, &rx_qinfo); > + TEST_ASSERT(ret == 0, > + "Port(%u), queue(%u) failed to get RxQ info.\n", > + port_id, queue_id); > + > + TEST_ASSERT(rx_qinfo.queue_state == RTE_ETH_QUEUE_STATE_STARTED, > + "Wrong started Rx queue(%u) state(%d)\n", > + queue_id, rx_qinfo.queue_state); > + } > + > + /* Started TxQ */ > + for (uint16_t queue_id = 0; queue_id < dev_info.nb_tx_queues; queue_id++) { > + ret = rte_eth_tx_queue_info_get(port_id, queue_id, &tx_qinfo); > + TEST_ASSERT(ret == 0, > + "Port(%u), queue(%u) failed to get TxQ info.\n", > + port_id, queue_id); > + > + TEST_ASSERT(tx_qinfo.queue_state == RTE_ETH_QUEUE_STATE_STARTED, > + "Wrong started Tx queue(%u) state(%d)\n", > + queue_id, tx_qinfo.queue_state); > + } > + > + > + ret = rte_eth_dev_stop(port_id); > + TEST_ASSERT(ret == 0, > + "Port(%u) failed to stop.\n", port_id); > + > + /* Stopped RxQ */ > + for (uint16_t queue_id = 0; queue_id < dev_info.nb_rx_queues; queue_id++) { > + ret = rte_eth_rx_queue_info_get(port_id, queue_id, &rx_qinfo); > + TEST_ASSERT(ret == 0, > + "Port(%u), queue(%u) failed to get RxQ info.\n", > + port_id, queue_id); > + > + TEST_ASSERT(rx_qinfo.queue_state == RTE_ETH_QUEUE_STATE_STOPPED, > + "Wrong stopped Rx queue(%u) state(%d)\n", > + queue_id, rx_qinfo.queue_state); > + } > + > + /* Stopped TxQ */ > + for (uint16_t queue_id = 0; queue_id < dev_info.nb_tx_queues; queue_id++) { > + ret = rte_eth_tx_queue_info_get(port_id, queue_id, &tx_qinfo); > + TEST_ASSERT(ret == 0, > + "Port(%u), queue(%u) failed to get TxQ info.\n", > + port_id, queue_id); > + > + TEST_ASSERT(tx_qinfo.queue_state == RTE_ETH_QUEUE_STATE_STOPPED, > + "Wrong stopped Tx queue(%u) state(%d)\n", > + queue_id, tx_qinfo.queue_state); > + } > + } > + > + return TEST_SUCCESS; > +} > + > +static struct unit_test_suite ethdev_api_testsuite = { > + .suite_name = "ethdev API tests", > + .setup = NULL, > + .teardown = NULL, > + .unit_test_cases = { > + TEST_CASE(ethdev_api_queue_status), > + TEST_CASES_END() /**< NULL terminate unit test array */ > + } > +}; > + > +static int > +test_ethdev_api(void) > +{ > + rte_log_set_global_level(RTE_LOG_DEBUG); > + rte_log_set_level(RTE_LOGTYPE_EAL, RTE_LOG_DEBUG); > + > + return unit_test_suite_runner(ðdev_api_testsuite); > +} > + > +REGISTER_TEST_COMMAND(ethdev_api, test_ethdev_api); > diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h > index 3d44979b44f7..8f2e0f158cc4 100644 > --- a/lib/ethdev/rte_ethdev.h > +++ b/lib/ethdev/rte_ethdev.h > @@ -2788,6 +2788,9 @@ int rte_eth_dev_tx_queue_stop(uint16_t port_id, uint16_t tx_queue_id); > * Device RTE_ETH_DEV_NOLIVE_MAC_ADDR flag causes MAC address to be set before > * PMD port start callback function is invoked. > * > + * All device queues (except form deferred queues) status should be > + * `RTE_ETH_QUEUE_STATE_STARTED` after start. > + * > * On success, all basic functions exported by the Ethernet API (link status, > * receive/transmit, and so on) can be invoked. > * > @@ -2804,6 +2807,8 @@ int rte_eth_dev_start(uint16_t port_id); > * Stop an Ethernet device. The device can be restarted with a call to > * rte_eth_dev_start() > * > + * All device queues status should be `RTE_ETH_QUEUE_STATE_STOPPED` after stop. > + * > * @param port_id > * The port identifier of the Ethernet device. > * @return > -- > 2.34.1 > >