From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-11.5 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 97AFEC433DF for ; Fri, 9 Oct 2020 12:50:32 +0000 (UTC) Received: from dpdk.org (dpdk.org [92.243.14.124]) by mail.kernel.org (Postfix) with ESMTP id 0521822284 for ; Fri, 9 Oct 2020 12:50:31 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0521822284 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=dev-bounces@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 4D0771D637; Fri, 9 Oct 2020 14:50:30 +0200 (CEST) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 698971D635 for ; Fri, 9 Oct 2020 14:50:27 +0200 (CEST) IronPort-SDR: sur64NJBoLHCUU/z2uc1BY2B2hlQ7IsWb0xy02qJKeiCYNOkpQJoIDUEC1gEl0fMr8dfVdyZOq tmQMGDlzp1jQ== X-IronPort-AV: E=McAfee;i="6000,8403,9768"; a="164690111" X-IronPort-AV: E=Sophos;i="5.77,355,1596524400"; d="scan'208";a="164690111" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Oct 2020 05:50:25 -0700 IronPort-SDR: 7VDDQ1tnG6D3rnCKVjqBPXWG18donXl+Jd4lk0DHrcPhbe34qXYLXJGlFaSYrsQAm9SNTb3koh MDlAdhvzWAwQ== X-IronPort-AV: E=Sophos;i="5.77,355,1596524400"; d="scan'208";a="528917638" Received: from dhunt5-mobl5.ger.corp.intel.com (HELO [10.249.34.11]) ([10.249.34.11]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Oct 2020 05:50:24 -0700 To: Lukasz Wojciechowski Cc: dev@dpdk.org References: <20200925224209.12173-1-l.wojciechow@partner.samsung.com> <20201008052323.11547-1-l.wojciechow@partner.samsung.com> <20201008052323.11547-14-l.wojciechow@partner.samsung.com> From: David Hunt Message-ID: <4409896c-526d-023c-950b-3fb23db4e7b7@intel.com> Date: Fri, 9 Oct 2020 13:50:22 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Thunderbird/68.12.1 MIME-Version: 1.0 In-Reply-To: <20201008052323.11547-14-l.wojciechow@partner.samsung.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-GB Subject: Re: [dpdk-dev] [PATCH v5 13/15] test/distributor: add test with packets marking 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Lukasz, On 8/10/2020 6:23 AM, Lukasz Wojciechowski wrote: > All of the former tests analyzed only statistics > of packets processed by all workers. > The new test verifies also if packets are processed > on workers as expected. > Every packets processed by the worker is marked > and analyzed after it is returned to distributor. > > This test allows finding issues in matching algorithms. > > Signed-off-by: Lukasz Wojciechowski > --- > app/test/test_distributor.c | 141 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 141 insertions(+) > > diff --git a/app/test/test_distributor.c b/app/test/test_distributor.c > index 1e0a079ff..0404e463a 100644 > --- a/app/test/test_distributor.c > +++ b/app/test/test_distributor.c > @@ -542,6 +542,141 @@ test_flush_with_worker_shutdown(struct worker_params *wp, > return 0; > } > > +static int > +handle_and_mark_work(void *arg) > +{ > + struct rte_mbuf *buf[8] __rte_cache_aligned; > + struct worker_params *wp = arg; > + struct rte_distributor *db = wp->dist; > + unsigned int num, i; > + unsigned int id = __atomic_fetch_add(&worker_idx, 1, __ATOMIC_RELAXED); > + num = rte_distributor_get_pkt(db, id, buf, NULL, 0); > + while (!quit) { > + __atomic_fetch_add(&worker_stats[id].handled_packets, num, > + __ATOMIC_ACQ_REL); > + for (i = 0; i < num; i++) > + buf[i]->udata64 += id + 1; > + num = rte_distributor_get_pkt(db, id, > + buf, buf, num); > + } > + __atomic_fetch_add(&worker_stats[id].handled_packets, num, > + __ATOMIC_ACQ_REL); > + rte_distributor_return_pkt(db, id, buf, num); > + return 0; > +} > + > +/* sanity_mark_test sends packets to workers which mark them. > + * Every packet has also encoded sequence number. > + * The returned packets are sorted and verified if they were handled > + * by proper workers. > + */ > +static int > +sanity_mark_test(struct worker_params *wp, struct rte_mempool *p) > +{ > + const unsigned int buf_count = 24; > + const unsigned int burst = 8; > + const unsigned int shift = 12; > + const unsigned int seq_shift = 10; > + > + struct rte_distributor *db = wp->dist; > + struct rte_mbuf *bufs[buf_count]; > + struct rte_mbuf *returns[buf_count]; > + unsigned int i, count, id; > + unsigned int sorted[buf_count], seq; > + unsigned int failed = 0; > + > + printf("=== Marked packets test ===\n"); > + clear_packet_count(); > + if (rte_mempool_get_bulk(p, (void *)bufs, buf_count) != 0) { > + printf("line %d: Error getting mbufs from pool\n", __LINE__); > + return -1; > + } > + > +/* bufs' hashes will be like these below, but shifted left. > + * The shifting is for avoiding collisions with backlogs > + * and in-flight tags left by previous tests. > + * [1, 1, 1, 1, 1, 1, 1, 1 > + * 1, 1, 1, 1, 2, 2, 2, 2 > + * 2, 2, 2, 2, 1, 1, 1, 1] > + */ I would suggest indenting the comments to the same indent level as the code, this would make the flow easier to read. Same with additional comments below. > + for (i = 0; i < burst; i++) { > + bufs[0 * burst + i]->hash.usr = 1 << shift; > + bufs[1 * burst + i]->hash.usr = ((i < burst / 2) ? 1 : 2) > + << shift; > + bufs[2 * burst + i]->hash.usr = ((i < burst / 2) ? 2 : 1) > + << shift; > + } > +/* Assign a sequence number to each packet. The sequence is shifted, > + * so that lower bits of the udate64 will hold mark from worker. > + */ > + for (i = 0; i < buf_count; i++) > + bufs[i]->udata64 = i << seq_shift; > + > + count = 0; > + for (i = 0; i < buf_count/burst; i++) { > + rte_distributor_process(db, &bufs[i * burst], burst); > + count += rte_distributor_returned_pkts(db, &returns[count], > + buf_count - count); > + } > + > + do { > + rte_distributor_flush(db); > + count += rte_distributor_returned_pkts(db, &returns[count], > + buf_count - count); > + } while (count < buf_count); > + > + for (i = 0; i < rte_lcore_count() - 1; i++) > + printf("Worker %u handled %u packets\n", i, > + __atomic_load_n(&worker_stats[i].handled_packets, > + __ATOMIC_ACQUIRE)); > + > +/* Sort returned packets by sent order (sequence numbers). */ > + for (i = 0; i < buf_count; i++) { > + seq = returns[i]->udata64 >> seq_shift; > + id = returns[i]->udata64 - (seq << seq_shift); > + sorted[seq] = id; > + } > + > +/* Verify that packets [0-11] and [20-23] were processed > + * by the same worker > + */ > + for (i = 1; i < 12; i++) { > + if (sorted[i] != sorted[0]) { > + printf("Packet number %u processed by worker %u," > + " but should be processes by worker %u\n", > + i, sorted[i], sorted[0]); > + failed = 1; > + } > + } > + for (i = 20; i < 24; i++) { > + if (sorted[i] != sorted[0]) { > + printf("Packet number %u processed by worker %u," > + " but should be processes by worker %u\n", > + i, sorted[i], sorted[0]); > + failed = 1; > + } > + } > +/* And verify that packets [12-19] were processed > + * by the another worker > + */ > + for (i = 13; i < 20; i++) { > + if (sorted[i] != sorted[12]) { > + printf("Packet number %u processed by worker %u," > + " but should be processes by worker %u\n", > + i, sorted[i], sorted[12]); > + failed = 1; > + } > + } > + > + rte_mempool_put_bulk(p, (void *)bufs, buf_count); > + > + if (failed) > + return -1; > + > + printf("Marked packets test passed\n"); > + return 0; > +} > + > static > int test_error_distributor_create_name(void) > { > @@ -726,6 +861,12 @@ test_distributor(void) > goto err; > quit_workers(&worker_params, p); > > + rte_eal_mp_remote_launch(handle_and_mark_work, > + &worker_params, SKIP_MASTER); > + if (sanity_mark_test(&worker_params, p) < 0) > + goto err; > + quit_workers(&worker_params, p); > + > } else { > printf("Too few cores to run worker shutdown test\n"); > } Checking the flows go to the correct workers is areally good test to have. Thanks for the effort here. Apart from the comment indentation nit: the rest looks good to me. Acked-by: David Hunt