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 696D3A04DD; Fri, 23 Oct 2020 15:59:37 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 3C294A964; Fri, 23 Oct 2020 15:59:36 +0200 (CEST) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 30D06A962 for ; Fri, 23 Oct 2020 15:59:33 +0200 (CEST) IronPort-SDR: TN9HGhJVckoASG7dekjWTdeXfLxjFO3xJYERfH/QygmUPwKZKdC7i73snFkv1atMJ5BLHYkkty aiFcgNJ+v34Q== X-IronPort-AV: E=McAfee;i="6000,8403,9782"; a="252376442" X-IronPort-AV: E=Sophos;i="5.77,408,1596524400"; d="scan'208";a="252376442" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 Oct 2020 06:59:26 -0700 IronPort-SDR: 7IQogD4/1qORmZf6W1PVlUPkfJ8ug1nTnmLFhxwtFGed+Ms4LVqPYq9RVU0i3z0msr2QQSECqt gUSLJRFO5mOQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.77,408,1596524400"; d="scan'208";a="333312880" Received: from orsmsx605.amr.corp.intel.com ([10.22.229.18]) by orsmga002.jf.intel.com with ESMTP; 23 Oct 2020 06:59:25 -0700 Received: from orsmsx607.amr.corp.intel.com (10.22.229.20) by ORSMSX605.amr.corp.intel.com (10.22.229.18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Fri, 23 Oct 2020 06:59:25 -0700 Received: from orsmsx611.amr.corp.intel.com (10.22.229.24) by ORSMSX607.amr.corp.intel.com (10.22.229.20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Fri, 23 Oct 2020 06:59:25 -0700 Received: from ORSEDG602.ED.cps.intel.com (10.7.248.7) by orsmsx611.amr.corp.intel.com (10.22.229.24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5 via Frontend Transport; Fri, 23 Oct 2020 06:59:25 -0700 Received: from NAM10-BN7-obe.outbound.protection.outlook.com (104.47.70.104) by edgegateway.intel.com (134.134.137.103) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.1713.5; Fri, 23 Oct 2020 06:59:24 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=VAFBJc6t75LZ8P/o0IYbImyEzk2UZKo3jndeArT4oxMQcd1r7hXB/VJ+kTBDrppCCfg8Kg1snQQr68AOiGneNiEhdIEPFDXE5Q7I9Dk4xDtVAtF/2mFxUCI66e33h5+cAQ/0CaLq/XA5FNLYegBZiiE1rS4NbRszf5jhbANieuYzbgWBKviQaqzqREPKTxfDPluJ1WTm7tkqx6/a8XoToOHqGh9b1X+ghNcpZPNK5B5ki/6P9lIXRL4iQ4QLv/8jrthiS9OxmwX6XevL172bKl6B8kco4g9fh3n994WpU4v/EX+y/obmWfxDtT0iG30PHe3qs6bLsn//3OJIM+JkHQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=qUq1Gll/+Sf4ru1QBFydZILc/NWhKVTn9JZM0hEDa6c=; b=gbOFzVhpfDcqPro3FQ+WRVSH08pZlfoc6xk73RmSnmHiscof4KKJDQR5AVqXcK6iFVaopB7fIdGMHuL2htb+fbkWi4Q4H2gsr8tB9Zph4BQYjoyaHUEMbQ3hXGV/+pZu8JfYby1oPZ5DpTqJG3GK8bSuo6gwRJaEQ5V+1Nq5JK+o2JQgzWHfRm2OHN139VW6WoXw2vKmh8QGYUn9XXxqh2/A6hZYHVEcTT2TYU0qFlKs3N+iBq35wf7mjiy0XgE2urIgx8o4boFU5C4NGTdiqnsEp6TxzLEZsCk9C1Srr3+8ZL1X3R56C9WEi6E+MiWoOKzBps2pcpCJuSJyMoeF+A== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=qUq1Gll/+Sf4ru1QBFydZILc/NWhKVTn9JZM0hEDa6c=; b=ESbaEp2iP18Pp+pF4s56AmixsjKI1XKw/38ZXicOQZ4HXG1laQ2zNAZNg596TEmJAAXFlhd8V2I721iPl7JUEJ5nlWJ/kyHrOFVji+jmO6kWg9mUZQV9yEK0aDltv65j5PYAmNvpRGIkBDJS1Op/Gb+MHhxWNfVesyuASE/kJA4= Received: from BYAPR11MB3301.namprd11.prod.outlook.com (2603:10b6:a03:7f::26) by SJ0PR11MB4846.namprd11.prod.outlook.com (2603:10b6:a03:2d8::24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3477.21; Fri, 23 Oct 2020 13:59:23 +0000 Received: from BYAPR11MB3301.namprd11.prod.outlook.com ([fe80::f5a4:3f6b:ade3:296b]) by BYAPR11MB3301.namprd11.prod.outlook.com ([fe80::f5a4:3f6b:ade3:296b%3]) with mapi id 15.20.3499.018; Fri, 23 Oct 2020 13:59:23 +0000 From: "Ananyev, Konstantin" To: Honnappa Nagarahalli , "dev@dpdk.org" CC: "olivier.matz@6wind.com" , "david.marchand@redhat.com" , "dharmik.thakkar@arm.com" , "ruifeng.wang@arm.com" , "nd@arm.com" Thread-Topic: [PATCH v3 2/5] lib/ring: add zero copy APIs Thread-Index: AQHWqPcwQno5Vvsh50iPfK7J+o5iEamlLsNw Date: Fri, 23 Oct 2020 13:59:22 +0000 Message-ID: References: <20200224203931.21256-1-honnappa.nagarahalli@arm.com> <20201023044343.13462-1-honnappa.nagarahalli@arm.com> <20201023044343.13462-3-honnappa.nagarahalli@arm.com> In-Reply-To: <20201023044343.13462-3-honnappa.nagarahalli@arm.com> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-reaction: no-action dlp-version: 11.5.1.3 authentication-results: arm.com; dkim=none (message not signed) header.d=none;arm.com; dmarc=none action=none header.from=intel.com; x-originating-ip: [46.7.39.127] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: ae8257e7-c09c-441e-7b2f-08d8775bd882 x-ms-traffictypediagnostic: SJ0PR11MB4846: x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: NZnyCJsTvm/BwtZ6/uJwCAJHb6I/YgzPvFRdThTLtLQygagp2UBr43YOy/b4JSIc1PnA4gLe/lNDEw1TLWiqsKo2ntELPrbzXsz5B4dKcj4WGoSEm1NQyWg5+heRxBHOlVv/XIpapOfxtGPWqCE9txFeYbGqkQhkW+u+6CIxIMAPrfg6ilfQsWHN+TErI3ueRxX1aUekXlXQJBYwEuoQ+wEkb6tClaPcqowN3MN+FhQhQl7+DJJSfCojA4nKN+hm3r1Fqge7tcMNZxp9zRSf9uReSZ96mv8Bl4TN91KMtoPnakUC0p0P7XI+ykXdiVs/ x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:BYAPR11MB3301.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(136003)(346002)(376002)(39860400002)(366004)(396003)(33656002)(71200400001)(66946007)(64756008)(66476007)(66556008)(66446008)(86362001)(55016002)(5660300002)(4326008)(6506007)(52536014)(8676002)(110136005)(76116006)(83380400001)(8936002)(478600001)(316002)(7696005)(26005)(9686003)(186003)(2906002)(54906003); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata: Pw3DCi/ohwBoOnVdfMcIWI2efSwrNIdNIByQq30pt6bD9TpZO4pRgyMIWMDqAmdzPz3f/RAKnFC6FCGscZl0I6jUYCgx6y/T2S5tGlu5c2G/EU25xWNIToPuvcEzdcQK+6VrM6VD4gGl4gYBd6UFNaPmiy2hj1JNMUrytJhNEM+9B1/ilJZ3xVda3a1nVv4S1VApbu7n90oTibDmkjM9OvPPZO4KVg/n5Iq/33En9mGb8EQ6uckBCsUcf6mnO9BbTjhMLm8PloO3VrxADfz2RelsiXRymZtvQf4cEOJFMCMeU/U0oxz5YcyPtfFPzoUech96tOgmPUeT7LbTxeSq6wCG7nf7hKgeoNCOoewscRP/kVqRu/hO3cC+Hije/rTuORm+2YcubmXxbqqaAFu3YpzUcPFCL3sjGDUHUw6nH4NRx0nAOY2SjCmEmyg7MVRAVX5r9DS/lZENxE0Cd+fzA1Ne4SB6WgSzX3yx5njvI+RdF0xf5tPJupReJIBbCw2Di/lrslDDUP+pRt/aPxBvnqQCE6YSr4zP0mvF+ABmjOE07BjWgh8LwvqUwihOHdRGsFIaJaUxDGcb3ftqR1QQPK3KQHX/rgEwFcbwf9fG1S7mnJQmjBodiShZI9q836ZP69i3jXcZ9l2KE+5g2XY73g== x-ms-exchange-transport-forked: True Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: BYAPR11MB3301.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: ae8257e7-c09c-441e-7b2f-08d8775bd882 X-MS-Exchange-CrossTenant-originalarrivaltime: 23 Oct 2020 13:59:23.0054 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: WoUIXoN3KMVEAdHcSq1EoAHhbd1Xg3ZLnFJuOqsPd3XuKE6tt/FVJ+LgWbbtENg/5QUYMJ117muMftUkM2oS4e70nu3ypyRV0F2omr74XdA= X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR11MB4846 X-OriginatorOrg: intel.com Subject: Re: [dpdk-dev] [PATCH v3 2/5] lib/ring: add zero copy APIs 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" >=20 > Add zero-copy APIs. These APIs provide the capability to > copy the data to/from the ring memory directly, without > having a temporary copy (for ex: an array of mbufs on > the stack). Use cases that involve copying large amount > of data to/from the ring can benefit from these APIs. LGTM in general. Few nits, see below. >=20 > Signed-off-by: Honnappa Nagarahalli > Reviewed-by: Dharmik Thakkar > --- > lib/librte_ring/meson.build | 1 + > lib/librte_ring/rte_ring_elem.h | 1 + > lib/librte_ring/rte_ring_peek_zc.h | 542 +++++++++++++++++++++++++++++ > 3 files changed, 544 insertions(+) > create mode 100644 lib/librte_ring/rte_ring_peek_zc.h Need to update documentation: PG and RN. >=20 > diff --git a/lib/librte_ring/meson.build b/lib/librte_ring/meson.build > index 31c0b4649..36fdcb6a5 100644 > --- a/lib/librte_ring/meson.build > +++ b/lib/librte_ring/meson.build > @@ -11,5 +11,6 @@ headers =3D files('rte_ring.h', > 'rte_ring_hts_c11_mem.h', > 'rte_ring_peek.h', > 'rte_ring_peek_c11_mem.h', > + 'rte_ring_peek_zc.h', > 'rte_ring_rts.h', > 'rte_ring_rts_c11_mem.h') > diff --git a/lib/librte_ring/rte_ring_elem.h b/lib/librte_ring/rte_ring_e= lem.h > index 938b398fc..7034d29c0 100644 > --- a/lib/librte_ring/rte_ring_elem.h > +++ b/lib/librte_ring/rte_ring_elem.h > @@ -1079,6 +1079,7 @@ rte_ring_dequeue_burst_elem(struct rte_ring *r, voi= d *obj_table, >=20 > #ifdef ALLOW_EXPERIMENTAL_API > #include > +#include > #endif >=20 > #include > diff --git a/lib/librte_ring/rte_ring_peek_zc.h b/lib/librte_ring/rte_rin= g_peek_zc.h > new file mode 100644 > index 000000000..9db2d343f > --- /dev/null > +++ b/lib/librte_ring/rte_ring_peek_zc.h > @@ -0,0 +1,542 @@ > +/* SPDX-License-Identifier: BSD-3-Clause > + * > + * Copyright (c) 2020 Arm Limited > + * Copyright (c) 2007-2009 Kip Macy kmacy@freebsd.org > + * All rights reserved. > + * Derived from FreeBSD's bufring.h > + * Used as BSD-3 Licensed with permission from Kip Macy. > + */ > + > +#ifndef _RTE_RING_PEEK_ZC_H_ > +#define _RTE_RING_PEEK_ZC_H_ > + > +/** > + * @file > + * @b EXPERIMENTAL: this API may change without prior notice > + * It is not recommended to include this file directly. > + * Please include instead. > + * > + * Ring Peek Zero Copy APIs > + * These APIs make it possible to split public enqueue/dequeue API > + * into 3 parts: > + * - enqueue/dequeue start > + * - copy data to/from the ring > + * - enqueue/dequeue finish > + * Along with the advantages of the peek APIs, these APIs provide the ab= ility > + * to avoid copying of the data to temporary area (for ex: array of mbuf= s > + * on the stack). > + * > + * Note that currently these APIs are available only for two sync modes: > + * 1) Single Producer/Single Consumer (RTE_RING_SYNC_ST) > + * 2) Serialized Producer/Serialized Consumer (RTE_RING_SYNC_MT_HTS). > + * It is user's responsibility to create/init ring with appropriate sync > + * modes selected. > + * > + * Following are some examples showing the API usage. > + * 1) > + * struct elem_obj {uint64_t a; uint32_t b, c;}; > + * struct elem_obj *obj; > + * > + * // Create ring with sync type RTE_RING_SYNC_ST or RTE_RING_SYNC_MT_HT= S > + * // Reserve space on the ring > + * n =3D rte_ring_enqueue_zc_bulk_elem_start(r, sizeof(elem_obj), 1, &zc= d, NULL); > + * > + * // Produce the data directly on the ring memory > + * obj =3D (struct elem_obj *)zcd->ptr1; > + * obj.a =3D rte_get_a(); As obj is a pointer, should be obj->a =3D ... Same for b and c. > + * obj.b =3D rte_get_b(); > + * obj.c =3D rte_get_c(); > + * rte_ring_enqueue_zc_elem_finish(ring, n); > + * > + * 2) > + * // Create ring with sync type RTE_RING_SYNC_ST or RTE_RING_SYNC_MT_HT= S > + * // Reserve space on the ring > + * n =3D rte_ring_enqueue_zc_burst_start(r, 32, &zcd, NULL); > + * > + * // Pkt I/O core polls packets from the NIC > + * if (n =3D=3D 32) > + * nb_rx =3D rte_eth_rx_burst(portid, queueid, zcd->ptr1, 32); > + * else > + * nb_rx =3D rte_eth_rx_burst(portid, queueid, zcd->ptr1, zcd->n1); Hmm, that doesn't look exactly correct to me. It could be that n =3D=3D 32, but we still need to do wrap-around. Shouldn't it be: If (n !=3D 0) { nb_rx =3D rte_eth_rx_burst(portid, queueid, zcd->ptr1, zcd->n1); if (nb_rx =3D=3D zcd->n1 && nb_rx !=3D n) nb_rx +=3D rte_eth_rx_burst(portid, queueid, zcd->ptr2, n - nb_rx); } > + * > + * // Provide packets to the packet processing cores > + * rte_ring_enqueue_zc_finish(r, nb_rx); > + * > + * Note that between _start_ and _finish_ none other thread can proceed > + * with enqueue/dequeue operation till _finish_ completes. > + */ > + > +#ifdef __cplusplus > +extern "C" { > +#endif > + > +#include > + > +/** > + * Ring zero-copy information structure. > + * > + * This structure contains the pointers and length of the space > + * reserved on the ring storage. > + */ > +struct rte_ring_zc_data { > + /* Pointer to the first space in the ring */ > + void **ptr1; Why not just 'void *ptr1;'? Same for ptr2. > + /* Pointer to the second space in the ring if there is wrap-around */ > + void **ptr2; > + /* Number of elements in the first pointer. If this is equal to > + * the number of elements requested, then ptr2 is NULL. > + * Otherwise, subtracting n1 from number of elements requested > + * will give the number of elements available at ptr2. > + */ > + unsigned int n1; > +} __rte_cache_aligned; > + > +static __rte_always_inline void > +__rte_ring_get_elem_addr(struct rte_ring *r, uint32_t head, > + uint32_t esize, uint32_t num, void **dst1, uint32_t *n1, void **dst2) > +{ > + uint32_t idx, scale, nr_idx; > + uint32_t *ring =3D (uint32_t *)&r[1]; > + > + /* Normalize to uint32_t */ > + scale =3D esize / sizeof(uint32_t); > + idx =3D head & r->mask; > + nr_idx =3D idx * scale; > + > + *dst1 =3D ring + nr_idx; > + *n1 =3D num; > + > + if (idx + num > r->size) { > + *n1 =3D r->size - idx; > + *dst2 =3D ring; > + } Seems like missing: else {*dst2 =3D NULL;} > +} > + > +/** > + * @internal This function moves prod head value. > + */ > +static __rte_always_inline unsigned int > +__rte_ring_do_enqueue_zc_elem_start(struct rte_ring *r, unsigned int esi= ze, > + uint32_t n, enum rte_ring_queue_behavior behavior, > + struct rte_ring_zc_data *zcd, unsigned int *free_space) > +{ > + uint32_t free, head, next; > + > + switch (r->prod.sync_type) { > + case RTE_RING_SYNC_ST: > + n =3D __rte_ring_move_prod_head(r, RTE_RING_SYNC_ST, n, > + behavior, &head, &next, &free); > + __rte_ring_get_elem_addr(r, head, esize, n, (void **)&zcd->ptr1, If you change ptr1, ptr2 to be just 'void *', then probably no extra type-c= ast will be needed here.=20 > + &zcd->n1, (void **)&zcd->ptr2); > + break; > + case RTE_RING_SYNC_MT_HTS: > + n =3D __rte_ring_hts_move_prod_head(r, n, behavior, &head, &free); > + __rte_ring_get_elem_addr(r, head, esize, n, (void **)&zcd->ptr1, > + &zcd->n1, (void **)&zcd->ptr2); > + break; > + case RTE_RING_SYNC_MT: > + case RTE_RING_SYNC_MT_RTS: > + default: > + /* unsupported mode, shouldn't be here */ > + RTE_ASSERT(0); > + n =3D 0; > + free =3D 0; > + } Would it make sense to move __rte_ring_get_elem_addr() here and do it only when n !=3D 0? I.E: if (n !=3D 0) __rte_ring_get_elem_addr(...); =09 Same comments for _dequeue_ analog. > + > + if (free_space !=3D NULL) > + *free_space =3D free - n; > + return n; > +} > +