From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 089F44C8C for ; Wed, 22 Aug 2018 18:13:07 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 22 Aug 2018 09:13:06 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.53,274,1531810800"; d="scan'208";a="74833450" Received: from nikhilr-mobl1.amr.corp.intel.com (HELO [10.252.84.201]) ([10.252.84.201]) by FMSMGA003.fm.intel.com with ESMTP; 22 Aug 2018 09:13:04 -0700 To: Pavan Nikhilesh , jerin.jacob@caviumnetworks.com, olivier.matz@6wind.com Cc: dev@dpdk.org, "Rao, Nikhil" References: <1534479652-80182-1-git-send-email-nikhil.rao@intel.com> <1534479652-80182-4-git-send-email-nikhil.rao@intel.com> <20180817115515.GA4360@ltp-pvn> From: "Rao, Nikhil" Message-ID: <6cd55ea9-fb68-73a4-43bd-dcd6772247cf@intel.com> Date: Wed, 22 Aug 2018 21:43:02 +0530 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.0 MIME-Version: 1.0 In-Reply-To: <20180817115515.GA4360@ltp-pvn> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v2 4/4] eventdev: add auto test for eth Tx 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: Wed, 22 Aug 2018 16:13:08 -0000 On 8/17/2018 5:25 PM, Pavan Nikhilesh wrote: > Hi Nikhil, > > Few comments inline. > > Nit: Please use --in-reply-to while sending next versions. > (https://core.dpdk.org/contribute/) > > Thanks, > Pavan > > On Fri, Aug 17, 2018 at 09:50:52AM +0530, Nikhil Rao wrote: >> >> This patch adds tests for the eth Tx adapter APIs. It also >> tests the data path for the rte_service function based >> implementation of the APIs. >> >> Signed-off-by: Nikhil Rao >> --- >> test/test/test_event_eth_tx_adapter.c | 676 ++++++++++++++++++++++++++++++++++ >> MAINTAINERS | 1 + >> test/test/Makefile | 1 + >> test/test/meson.build | 2 + >> 4 files changed, 680 insertions(+) >> create mode 100644 test/test/test_event_eth_tx_adapter.c >> >> diff --git a/test/test/test_event_eth_tx_adapter.c b/test/test/test_event_eth_tx_adapter.c >> new file mode 100644 >> index 0000000..2dc487b >> --- /dev/null >> +++ b/test/test/test_event_eth_tx_adapter.c > > > >> +static int >> +testsuite_setup(void) >> +{ >> + int err; >> + uint8_t count; >> + struct rte_event_dev_info dev_info; >> + uint8_t priority; >> + uint8_t queue_id; >> + >> + count = rte_event_dev_count(); >> + if (!count) { >> + printf("Failed to find a valid event device," >> + " testing with event_sw0 device\n"); >> + rte_vdev_init("event_sw0", NULL); >> + event_dev_delete = 1; >> + } >> + >> + struct rte_event_dev_config config = { >> + .nb_event_queues = 1, >> + .nb_event_ports = 1, >> + }; >> + >> + struct rte_event_queue_conf wkr_q_conf = { >> + .schedule_type = RTE_SCHED_TYPE_ORDERED, >> + .priority = RTE_EVENT_DEV_PRIORITY_NORMAL, >> + .nb_atomic_flows = 1024, >> + .nb_atomic_order_sequences = 1024, >> + }; >> + >> + err = rte_event_dev_info_get(TEST_DEV_ID, &dev_info); >> + config.nb_event_queue_flows = dev_info.max_event_queue_flows; >> + config.nb_event_port_dequeue_depth = >> + dev_info.max_event_port_dequeue_depth; >> + config.nb_event_port_enqueue_depth = >> + dev_info.max_event_port_enqueue_depth; >> + config.nb_events_limit = >> + dev_info.max_num_events; >> + >> + rte_log_set_level(0, RTE_LOG_DEBUG); >> + err = rte_event_dev_configure(TEST_DEV_ID, &config); >> + TEST_ASSERT(err == 0, "Event device initialization failed err %d\n", >> + err); >> + if (rte_event_queue_setup(TEST_DEV_ID, 0, &wkr_q_conf) < 0) { >> + printf("%d: error creating qid %d\n", __LINE__, 0); >> + return -1; >> + } >> + if (rte_event_port_setup(TEST_DEV_ID, 0, NULL) < 0) { >> + printf("Error setting up port %d\n", 0); >> + return -1; >> + } >> + >> + priority = RTE_EVENT_DEV_PRIORITY_LOWEST; > > queue_id is uninitilized here, garbage value might be passed. Ok. > >> + if (rte_event_port_link(TEST_DEV_ID, 0, &queue_id, &priority, 1) != 1) { >> + printf("Error linking port\n"); >> + return -1; >> + } >> + >> + err = init_ports(); >> + TEST_ASSERT(err == 0, "Port initialization failed err %d\n", err); >> + >> + err = rte_event_eth_tx_adapter_caps_get(TEST_DEV_ID, TEST_ETHDEV_ID, >> + &default_params.caps); >> + TEST_ASSERT(err == 0, "Failed to get adapter cap err %d\n", >> + err); >> + >> + return err; >> +} >> + > > > >> +static uint32_t eid, tid; >> + >> +static int >> +tx_adapter_single(uint16_t port, uint16_t tx_queue_id, >> + struct rte_mbuf *m, uint8_t qid, >> + uint8_t sched_type) >> +{ >> + struct rte_event event; >> + struct rte_mbuf *r; >> + int ret; >> + unsigned int l; >> + >> + event.queue_id = qid; > > Set event_type to RTE_EVENT_TYPE_CPU so that underlying drivers don't mess up the > packet. Ok. > > > >> + >> +static int >> +tx_adapter_service(void) >> +{ >> + struct rte_event_eth_tx_adapter_stats stats; >> + uint32_t i; >> + int err; >> + uint8_t ev_port, ev_qid; >> + struct rte_mbuf bufs[RING_SIZE]; >> + struct rte_mbuf *pbufs[RING_SIZE]; >> + struct rte_event_dev_info dev_info; >> + struct rte_event_dev_config dev_conf; >> + struct rte_event_queue_conf qconf; >> + uint32_t qcnt, pcnt; >> + uint16_t q; >> + int internal_port; >> + >> + err = rte_event_eth_tx_adapter_caps_get(TEST_DEV_ID, TEST_ETHDEV_ID, >> + &default_params.caps); >> + TEST_ASSERT(err == 0, "Failed to get adapter cap err %d\n", >> + err); >> + >> + internal_port = !!(default_params.caps & >> + RTE_EVENT_ETH_TX_ADAPTER_CAP_INTERNAL_PORT); >> + >> + if (internal_port) >> + return TEST_SUCCESS; >> + >> + err = rte_event_eth_tx_adapter_queue_add(TEST_INST_ID, TEST_ETHDEV_ID, >> + -1); >> + TEST_ASSERT(err == 0, "Expected 0 got %d", err); >> + >> + err = rte_event_eth_tx_adapter_event_port_get(TEST_INST_ID, >> + &ev_port); >> + TEST_ASSERT_SUCCESS(err, "Failed to get event port %d", err); >> + >> + err = rte_event_dev_attr_get(TEST_DEV_ID, RTE_EVENT_DEV_ATTR_PORT_COUNT, >> + &pcnt); >> + TEST_ASSERT_SUCCESS(err, "Port count get failed"); >> + >> + err = rte_event_dev_attr_get(TEST_DEV_ID, >> + RTE_EVENT_DEV_ATTR_QUEUE_COUNT, &qcnt); >> + TEST_ASSERT_SUCCESS(err, "Queue count get failed"); >> + >> + err = rte_event_dev_info_get(TEST_DEV_ID, &dev_info); >> + TEST_ASSERT_SUCCESS(err, "Dev info failed"); >> + > > memset dev_config to avoid invalid values. > Ok. >> + dev_conf.nb_event_queue_flows = dev_info.max_event_queue_flows; >> + dev_conf.nb_event_port_dequeue_depth = >> + dev_info.max_event_port_dequeue_depth; >> + dev_conf.nb_event_port_enqueue_depth = >> + dev_info.max_event_port_enqueue_depth; >> + dev_conf.nb_events_limit = >> + dev_info.max_num_events; >> + dev_conf.nb_event_queues = qcnt + 1; >> + dev_conf.nb_event_ports = pcnt; >> + err = rte_event_dev_configure(TEST_DEV_ID, &dev_conf); >> + TEST_ASSERT(err == 0, "Event device initialization failed err %d\n", >> + err); >> + >> + ev_qid = qcnt; >> + qconf.nb_atomic_flows = dev_info.max_event_queue_flows; >> + qconf.nb_atomic_order_sequences = 32; >> + qconf.schedule_type = RTE_SCHED_TYPE_ATOMIC; >> + qconf.priority = RTE_EVENT_DEV_PRIORITY_HIGHEST; >> + qconf.event_queue_cfg = RTE_EVENT_QUEUE_CFG_SINGLE_LINK; >> + err = rte_event_queue_setup(TEST_DEV_ID, ev_qid, &qconf); >> + TEST_ASSERT_SUCCESS(err, "Failed to setup queue %u", ev_qid); > > On reconfigure, setup all the ports and queues so that the newly configured > values are seen by them. Is that required since dev_conf matches dev_info except for the number of queues ? > >> + >> + err = rte_event_port_link(TEST_DEV_ID, ev_port, &ev_qid, NULL, 1); >> + TEST_ASSERT(err == 1, "Failed to link queue port %u", >> + ev_port); >> + >> + err = rte_event_eth_tx_adapter_start(TEST_INST_ID); >> + TEST_ASSERT(err == 0, "Expected 0 got %d", err); >> + >> + err = rte_event_dev_service_id_get(0, &eid); >> + TEST_ASSERT(err == 0, "Expected 0 got %d", err); > > An event device might not be needing a service core, check the capabilities > before requesting service id above. > The internal_port capability is checked at the beginning of the function, if its not set, I think its Ok to assume that we need a service core ? Thanks, Nikhil