From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 5C08EA04BC; Fri, 9 Oct 2020 23:13:45 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 881F31D576; Fri, 9 Oct 2020 23:12:39 +0200 (CEST) Received: from mailout2.w1.samsung.com (mailout2.w1.samsung.com [210.118.77.12]) by dpdk.org (Postfix) with ESMTP id A6A371D561 for ; Fri, 9 Oct 2020 23:12:38 +0200 (CEST) Received: from eucas1p1.samsung.com (unknown [182.198.249.206]) by mailout2.w1.samsung.com (KnoxPortal) with ESMTP id 20201009211227euoutp0236812b66e865d66509ce1dc41383a510~8b03ErNeU2798727987euoutp02S for ; Fri, 9 Oct 2020 21:12:27 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout2.w1.samsung.com 20201009211227euoutp0236812b66e865d66509ce1dc41383a510~8b03ErNeU2798727987euoutp02S DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1602277947; bh=RlCIsfbcf4Cxm/j1jo33DauMfL216l50zdiWaNgVyCs=; h=Subject:To:Cc:From:Date:In-Reply-To:References:From; b=FdGScWQp9fg9X1FBYoMMljDc0/7jV0yc0IHzBTW0bKGN+46ixREJnrENXP5aA2o49 B+hCyt5caewpzkCJmGF4jlV5L2JTFUbnUIj2mUofjxxdP9+qXBW1KrhvEfRBl3jsuI F3tNpew0euutkqDlPN0q7tF40kPECLFWHyIOLw58= Received: from eusmges3new.samsung.com (unknown [203.254.199.245]) by eucas1p1.samsung.com (KnoxPortal) with ESMTP id 20201009211221eucas1p191d716c5dc7f801142ab43ec81c598fc~8b0xaejQG1382513825eucas1p1f; Fri, 9 Oct 2020 21:12:21 +0000 (GMT) Received: from eucas1p1.samsung.com ( [182.198.249.206]) by eusmges3new.samsung.com (EUCPMTA) with SMTP id 00.22.06318.532D08F5; Fri, 9 Oct 2020 22:12:21 +0100 (BST) Received: from eusmtrp2.samsung.com (unknown [182.198.249.139]) by eucas1p1.samsung.com (KnoxPortal) with ESMTPA id 20201009211219eucas1p16169350934f824f5ddd66bc85a443d5d~8b0wTO5UF1384113841eucas1p1l; Fri, 9 Oct 2020 21:12:19 +0000 (GMT) Received: from eusmgms2.samsung.com (unknown [182.198.249.180]) by eusmtrp2.samsung.com (KnoxPortal) with ESMTP id 20201009211219eusmtrp2a80c86f6d300311a7232cd3621c78a0a~8b0wSnAV-2652526525eusmtrp2Y; Fri, 9 Oct 2020 21:12:19 +0000 (GMT) X-AuditID: cbfec7f5-38bff700000018ae-f2-5f80d235a8e0 Received: from eusmtip2.samsung.com ( [203.254.199.222]) by eusmgms2.samsung.com (EUCPMTA) with SMTP id 58.BE.06017.332D08F5; Fri, 9 Oct 2020 22:12:19 +0100 (BST) Received: from [106.210.88.70] (unknown [106.210.88.70]) by eusmtip2.samsung.com (KnoxPortal) with ESMTPA id 20201009211219eusmtip2ea58553ae1a654ff4f4e1e6856b24df4~8b0vtvdq90273602736eusmtip2G; Fri, 9 Oct 2020 21:12:19 +0000 (GMT) To: David Hunt Cc: dev@dpdk.org, "\"'Lukasz Wojciechowski'\"," From: Lukasz Wojciechowski Message-ID: <5e68b338-e9ce-9165-bbc6-dbb0cde8b23b@partner.samsung.com> Date: Fri, 9 Oct 2020 23:12:18 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.12.1 MIME-Version: 1.0 In-Reply-To: <4409896c-526d-023c-950b-3fb23db4e7b7@intel.com> Content-Transfer-Encoding: 8bit Content-Language: en-US X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFuphleLIzCtJLcpLzFFi42LZduznOV3TSw3xBh+OMFv0TfrIZPHu03Ym i2c96xgdmD1+LVjK6rF4z0smj4Pv9jAFMEdx2aSk5mSWpRbp2yVwZcy508ZacMCuov/XbLYG xvvGXYycHBICJhLTWq4ydTFycQgJrGCUWH5hGiOE84VRYsvP72wQzmdGiVf9R5m7GDnAWp7O L4CIL2eUWPe1lxXCecsocefLKmaQucIC/hLHm6axg9giAqoS//Z/YgKxmQVCJJ7faWIBsdkE bCWOzPzKCmLzCrhJrPuwig1kAYuAikTvvWSQsKhAnMSEjS0sECWCEidnPgGzOYFav5/bBjVS XqJ562xmCFtc4taT+UwQr/1nk1iw0AzCdpE49KaVDcIWlnh1fAs7hC0j8X/nfLD3JQS2MUpc /f2TEcLZzyhxvXcFVJW1xOF/v8GOYxbQlFi/Sx8i7Chxd/Z3Nkig8EnceCsIcQOfxKRt06Fh xSvR0SYEUa0n8bRnKiPM2j9rn7BMYFSaheSzWUi+mYXkm1kIexcwsqxiFE8tLc5NTy02zkst 1ytOzC0uzUvXS87P3cQITCOn/x3/uoNx35+kQ4wCHIxKPLwNyQ3xQqyJZcWVuYcYJTiYlUR4 nc6ejhPiTUmsrEotyo8vKs1JLT7EKM3BoiTOa7zoZayQQHpiSWp2ampBahFMlomDU6qBUem1 jba9Wmmr8PkXC++F504JFffJeZPDXnVy+a7QHy2q5zJ0heZPrswIyoq5zLHgxI7MGUujZTdH +v/8kDdR82C0X0lN0vQMh+WH/sppdWdZnarZYpfOuCiA42vvj1aPw0Izfmyd/F7zmidHQvYt 098Xz1kefHRL51C3lQ8/J9/5Mo6WRS53lViKMxINtZiLihMBM/3Rzh8DAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprHIsWRmVeSWpSXmKPExsVy+t/xe7rGlxriDT4+M7bom/SRyeLdp+1M Fs961jE6MHv8WrCU1WPxnpdMHgff7WEKYI7SsynKLy1JVcjILy6xVYo2tDDSM7S00DMysdQz NDaPtTIyVdK3s0lJzcksSy3St0vQy5hzp4214IBdRf+v2WwNjPeNuxg5OCQETCSezi/oYuTi EBJYyigx58F1Joi4jMSHSwJdjJxAprDEn2tdbBA1rxklpjdcYQVJCAv4ShxaNYkFxBYRUJX4 t/8TE4jNLBAisXjSOqiGo0wSH9etZQRJsAnYShyZ+RWsmVfATWLdh1VsIMtYBFQkeu8lg4RF BeIkfkzsZYMoEZQ4OfMJ2HxOoNbv57ZBzTeTmLf5ITOELS/RvHU2lC0ucevJfKYJjEKzkLTP QtIyC0nLLCQtCxhZVjGKpJYW56bnFhvpFSfmFpfmpesl5+duYgTGzbZjP7fsYOx6F3yIUYCD UYmHVyOxIV6INbGsuDL3EKMEB7OSCK/T2dNxQrwpiZVVqUX58UWlOanFhxhNgX6byCwlmpwP jOm8knhDU0NzC0tDc2NzYzMLJXHeDoGDMUIC6YklqdmpqQWpRTB9TBycUg2M66dpFGmnd2lP /c8u7nuc/UXDpEUL1bLNV+l+9tV3XJImOLXPvNJju7Ci5T+JJraQV5wr1W94H80oWZj+XOVr rV9a0uGT3QldVdOKdj8oV+ayKuEpX1JwKTlJWr9H5egHocjdZvejAqXjAg+licb9j70elmyY ulT9c2HLsl26z5QLuNRaXZVYijMSDbWYi4oTAUAjJf6xAgAA X-CMS-MailID: 20201009211219eucas1p16169350934f824f5ddd66bc85a443d5d X-Msg-Generator: CA Content-Type: text/plain; charset="utf-8" X-RootMTR: 20201008052346eucas1p15b04bf84cafc2ba52bbe063f57d08c39 X-EPHeader: CA CMS-TYPE: 201P X-CMS-RootMailID: 20201008052346eucas1p15b04bf84cafc2ba52bbe063f57d08c39 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> <4409896c-526d-023c-950b-3fb23db4e7b7@intel.com> 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 David, W dniu 09.10.2020 o 14:50, David Hunt pisze: > 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. > Indentation fixed as you suggested will be fixed in v6 > >> +    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 Thanks Lukasz > > > -- Lukasz Wojciechowski Principal Software Engineer Samsung R&D Institute Poland Samsung Electronics Office +48 22 377 88 25 l.wojciechow@partner.samsung.com