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 A6D18A04BC; Fri, 9 Oct 2020 10:05:40 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 641A11C1A8; Fri, 9 Oct 2020 10:05:38 +0200 (CEST) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 5B36A1C1A0 for ; Fri, 9 Oct 2020 10:05:36 +0200 (CEST) IronPort-SDR: PL83zoBmHioQVV3PZelMUJCkAr45MMywjTlqnYle9s0tCyuglUs+5bH4y7JsRipncQ2ZjOiP66 AuSQnGTVDvGQ== X-IronPort-AV: E=McAfee;i="6000,8403,9768"; a="152376846" X-IronPort-AV: E=Sophos;i="5.77,354,1596524400"; d="scan'208";a="152376846" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Oct 2020 01:05:34 -0700 IronPort-SDR: FGNthVPpT4BuBCZJcVy1cY2WdRFuMrFA9kiM6vR75QMPPfnak1jyTSQAEZoUHPhzpsBPg2Fa/e lxi+nEyEyjGQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.77,354,1596524400"; d="scan'208";a="328834606" Received: from fmsmsx601.amr.corp.intel.com ([10.18.126.81]) by orsmga002.jf.intel.com with ESMTP; 09 Oct 2020 01:05:33 -0700 Received: from fmsmsx601.amr.corp.intel.com (10.18.126.81) by fmsmsx601.amr.corp.intel.com (10.18.126.81) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Fri, 9 Oct 2020 01:05:33 -0700 Received: from fmsedg602.ED.cps.intel.com (10.1.192.136) by fmsmsx601.amr.corp.intel.com (10.18.126.81) 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, 9 Oct 2020 01:05:33 -0700 Received: from NAM04-CO1-obe.outbound.protection.outlook.com (104.47.45.51) by edgegateway.intel.com (192.55.55.71) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.1713.5; Fri, 9 Oct 2020 01:05:33 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=G4zauSTZXjKDUc6l3IDCEXAxKH5V0uuDl5ngpJAW1JOZHlYWuvya62gQ95uABlPP9WZIq8n3pnvDXEuI4qHbYbbDH5E1YxWwgWrASGdacFYF/9YIEHKsCU16WZgoe3FGbBnc8pEEejGs5ByjloZMPOUMm7GL3KeycpYOyaKjhl0MM4dTaZtOhXyWupqQMqPjC/GgYJf1lpKAtq2Z+nU8GBcwoDH1jEHBrlFF7FgD3xA3B8CMXtvAILN1XLRti/U/USvCJK+45IBQSGgZ2fH0Edy+pTMBXcq3yeNl8NWY5wxmyxcZSfCHLzt7wJiPmsaGz4Q/YXA8+N5FPohF5uq3QA== 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=WcJQml4QQzGIj2qcEOE6jjH7ILDyE2zaa3lXC4cnY+U=; b=Dv02bwpyrXaaeilsTtytVoOll0vpyHfxgch9802S2+cOKKlEzQxMCfDggNxmBLxW/e7UB+GFUOhsRnxOPohiaz6CgtgdRY+oQGkr6cUkY4Sv0nW7Ht5Svn1teYutcbAeebdPVwKZFPmgSMa31lMNXoaT9hVLSLWqZjZr1Z8Wz/wtzIhYcO8ArfWGv3vcBdnEpETy7J5/Zo/tksBTFz3N1SN/ytXFav62DqGmX/RCLrXrQZMSFR45yQ+2LS+Uw3OMJtiWUQFLPvDUg2Na1z9c/a1ZUAheFDzA3JXzX2wiu5W0IvOoOeBts44GggZEH7tFjz904xMLl0MNpsxYC4rnng== 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=WcJQml4QQzGIj2qcEOE6jjH7ILDyE2zaa3lXC4cnY+U=; b=h0Odca/8d8T596s67++mUuQe0men8JShV3jkBUrc21SLKoNfc3oLYx43x+rF92PKSUIYSDgg6H9L6BzYE/3UBCw5VXdppKbcuO5SsXQcebfDfWKDcT/pRyl5+jOdWAA91pcJci/eftWFW2yU+o6kbqxxpXE9KWiKyZvmjLKp4tw= Received: from BYAPR11MB3301.namprd11.prod.outlook.com (2603:10b6:a03:7f::26) by BY5PR11MB4228.namprd11.prod.outlook.com (2603:10b6:a03:1bd::31) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3455.21; Fri, 9 Oct 2020 08:05:30 +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.3455.023; Fri, 9 Oct 2020 08:05:29 +0000 From: "Ananyev, Konstantin" To: Olivier Matz , Honnappa Nagarahalli CC: "dev@dpdk.org" , "david.marchand@redhat.com" , nd Thread-Topic: [RFC v2 1/1] lib/ring: add scatter gather APIs Thread-Index: AQHWm+TAMRnLEGUYBEqkMDYffUIcWqmLz6WAgAJgIYCAALV2AIAABhsg Date: Fri, 9 Oct 2020 08:05:29 +0000 Message-ID: References: <20200224203931.21256-1-honnappa.nagarahalli@arm.com> <20201006132905.46205-1-honnappa.nagarahalli@arm.com> <20201006132905.46205-2-honnappa.nagarahalli@arm.com> <20201007082739.GL21395@platinum> <20201009073342.GZ21395@platinum> In-Reply-To: <20201009073342.GZ21395@platinum> 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: 6wind.com; dkim=none (message not signed) header.d=none;6wind.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: 62987818-fb0e-4a1d-781e-08d86c2a16c8 x-ms-traffictypediagnostic: BY5PR11MB4228: x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:9508; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: axs8lxOM7BXp3nElO4bmGN0dILulosxe+bxHGAuYdo7Q0zVgAdtCoJlUth5kVWSm448IyUqTzKEzwo5sGoWRoiLWXt/zbWMhdzZea+chUtBYjyOsoaIomq3zoJJ32DCxK1yFgfCJx8XDfQ99akySgdcTjxI/Y7rWMNUqEwhJ/aR85uq7j7Jv7S5vlQSYAF1ND5cVK8IYdsH63/J2uGpAnrtxNaZB0neJO2SZnkzd/D8jigSikHC1d2aaDgNGHzIda6Q9cafVM2LuNN+FL0TvJJs1jLXQDrBdHAq4+qbY9liVtmXMkypHkXs/FaeNOJCY7x+5BGpsl+TxEBjDAzO1Ru5owhq6T+uq/X5iUd5C0FD4tO9HGihdjNhRW+KC1eo1J1Wi4afa/P7xwWWAPyiXRA== 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)(396003)(366004)(39860400002)(376002)(346002)(136003)(4326008)(83080400001)(186003)(66556008)(2906002)(8936002)(83380400001)(26005)(76116006)(66946007)(66446008)(9686003)(64756008)(33656002)(86362001)(54906003)(7696005)(8676002)(478600001)(52536014)(55016002)(66476007)(71200400001)(5660300002)(6506007)(316002)(110136005)(966005); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata: u0yPPDGlmEvAunrNBJVd2bA7m17Rvo8XZORMVhEZy2vI+thVexiBGR3CGM9WdcA3afQu59OU1/Xp1n0EY4xt3NVQGKBqXRzKWtOYwvMDAPdIQsBoOUwtXQw+N5FGqoGcFRALT1CtEbh7eW4jVhi0Gj8th8FURhgdqVStcliGv6C1haSh9jo2FGmuEp3lynmKv+bK/SrAU6VT/wTesp9BkLOAJHW9qhJntNl+EreOJ1tjCiuiUZzfCZCWrWgv++ZzEDkBuMwRVXkumIFyLoNxb8hl/I/m5g5NHcGlVJ7vvHXZ2Xx3zyh10pvI2rRX32uCfZJEOLDRpErNGlPlLzu1MbNsc/wQCrWjzv+yGKWqKr3HXgajxjmBVkBswERRZGK3Y9DAcbH/zSszEgik+PE1hJ5m436+WOCs7m79euNp/xKe6oYzkelp6yLSWQJzlBhpH0BY96gOSPrBs3wEPn8kzWaHfCE+mF0LDg3/zkIT2u0aUGbWHpXDQF+8neNJwMC0JI9Qz07+Adi0a+NEyzpx5NxSGFovs33pMkxKUhUm5/UNmAe+Lw7eXHvlBQkdwanRjmpyBHlD23FpwIZrkkpgIaVwo5LhlAcGoiQZn7HqqgIVICQeLnse718VcqIf0X4D3XBGQM1F+lJT07gHSOlXMg== 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: 62987818-fb0e-4a1d-781e-08d86c2a16c8 X-MS-Exchange-CrossTenant-originalarrivaltime: 09 Oct 2020 08:05:29.7809 (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: kSxONdJN5BPNujdcXEWcppKekgBX/lbS5HKta5fn6yUWlnwF1kgDlokeA0ue4anYleECFCqaozCfjkY0FK9TDHc2XmOvEr6z/bCaQNlnlts= X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR11MB4228 X-OriginatorOrg: intel.com Subject: Re: [dpdk-dev] [RFC v2 1/1] lib/ring: add scatter gather 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" Hi lads, > On Thu, Oct 08, 2020 at 08:44:13PM +0000, Honnappa Nagarahalli wrote: > > > > > > > > > > Hi Honnappa, > > > > > > From a quick walkthrough, I have some questions/comments, please see > > > below. > > Hi Olivier, appreciate your input. > > > > > > > > On Tue, Oct 06, 2020 at 08:29:05AM -0500, Honnappa Nagarahalli wrote: > > > > Add scatter gather APIs to avoid intermediate memcpy. Use cases tha= t > > > > involve copying large amount of data to/from the ring can benefit f= rom > > > > these APIs. > > > > > > > > Signed-off-by: Honnappa Nagarahalli > > > > --- > > > > lib/librte_ring/meson.build | 3 +- > > > > lib/librte_ring/rte_ring_elem.h | 1 + > > > > lib/librte_ring/rte_ring_peek_sg.h | 552 > > > > +++++++++++++++++++++++++++++ > > > > 3 files changed, 555 insertions(+), 1 deletion(-) create mode 100= 644 > > > > lib/librte_ring/rte_ring_peek_sg.h > > > > > > > > diff --git a/lib/librte_ring/meson.build b/lib/librte_ring/meson.bu= ild > > > > index 31c0b4649..377694713 100644 > > > > --- a/lib/librte_ring/meson.build > > > > +++ b/lib/librte_ring/meson.build > > > > @@ -12,4 +12,5 @@ headers =3D files('rte_ring.h', > > > > 'rte_ring_peek.h', > > > > 'rte_ring_peek_c11_mem.h', > > > > 'rte_ring_rts.h', > > > > - 'rte_ring_rts_c11_mem.h') > > > > + 'rte_ring_rts_c11_mem.h', > > > > + 'rte_ring_peek_sg.h') > > > > diff --git a/lib/librte_ring/rte_ring_elem.h > > > > b/lib/librte_ring/rte_ring_elem.h index 938b398fc..7d3933f15 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, > > > > void *obj_table, > > > > > > > > #ifdef ALLOW_EXPERIMENTAL_API > > > > #include > > > > +#include > > > > #endif > > > > > > > > #include > > > > diff --git a/lib/librte_ring/rte_ring_peek_sg.h > > > > b/lib/librte_ring/rte_ring_peek_sg.h > > > > new file mode 100644 > > > > index 000000000..97d5764a6 > > > > --- /dev/null > > > > +++ b/lib/librte_ring/rte_ring_peek_sg.h > > > > @@ -0,0 +1,552 @@ > > > > +/* SPDX-License-Identifier: BSD-3-Clause > > > > + * > > > > + * Copyright (c) 2020 Arm > > > > + * 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_SG_H_ > > > > +#define _RTE_RING_PEEK_SG_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 Scatter Gather APIs > > > > > > I am not fully convinced by the API name. To me, "scatter/gather" is > > > associated to iovecs, like for instance in [1]. The wikipedia definit= ion [2] may > > > be closer though. > > > > > > [1] > > > https://www.gnu.org/software/libc/manual/html_node/Scatter_002dGathe > > > r.html > > > [2] https://en.wikipedia.org/wiki/Gather-scatter_(vector_addressing) > > The way I understand scatter-gather is, the data to be sent to somethin= g (like a device) is coming from multiple sources. It would require > copying to put the data together to be contiguous. If the device supports= scatter-gather, such copying is not required. The device can > collect data from multiple locations and make it contiguous. > > > > In the case I was looking at, one part of the data was coming from the = user of the API and another was generated by the API itself. If > these two pieces of information have to be enqueued as a single object on= the ring, I had to create an intermediate copy. But by exposing > the ring memory to the user, the intermediate copy is avoided. Hence I ca= lled it scatter-gather. > > > > > > > > What about "zero-copy"? > > I think no-copy (nc for short) or user-copy (uc for short) would convey= the meaning better. These would indicate that the rte_ring APIs are > not copying the objects and it is left to the user to do the actual copy. > > > > > > > > Also, the "peek" term looks also a bit confusing to me, but it existe= d before > > > your patch. I understand it for dequeue, but not for enqueue. > > I kept 'peek' there because the API still offers the 'peek' API capabil= ities. I am also not sure on what 'peek' means for enqueue API. The > enqueue 'peek' API was provided to be symmetric with dequeue peek API. > > > > > > > > Or, what about replacing the existing experimental peek API by this o= ne? > > > They look quite similar to me. > > I agree, scatter gather APIs provide the peek capability and the no-cop= y benefits. > > Konstantin, any opinions here? Sorry, didn't have time yet, to look at this RFC properly. Will try to do it next week, as I understand that's for 21.02 anyway? > > > > > > > + * Introduction of rte_ring with scatter gather serialized > > > > + producer/consumer > > > > + * (HTS sync mode) makes it possible to split public enqueue/deque= ue > > > > + API > > > > + * into 3 phases: > > > > + * - enqueue/dequeue start > > > > + * - copy data to/from the ring > > > > + * - enqueue/dequeue finish > > > > + * Along with the advantages of the peek APIs, these APIs provide = the > > > > + ability > > > > + * to avoid copying of the data to temporary area. > > > > + * > > > > + * Note that right now this new API is available only for two sync= modes: > > > > + * 1) Single Producer/Single Consumer (RTE_RING_SYNC_ST) > > > > + * 2) Serialized Producer/Serialized Consumer (RTE_RING_SYNC_MT_HT= S). > > > > + * It is a user responsibility to create/init ring with appropriat= e > > > > + sync > > > > + * modes selected. > > > > + * > > > > + * Example usage: > > > > + * // read 1 elem from the ring: > > > > > > Comment should be "prepare enqueuing 32 objects" > > > > > > > + * n =3D rte_ring_enqueue_sg_bulk_start(ring, 32, &sgd, NULL); > > > > + * if (n !=3D 0) { > > > > + * //Copy objects in the ring > > > > + * memcpy (sgd->ptr1, obj, sgd->n1 * sizeof(uintptr_t)); > > > > + * if (n !=3D sgd->n1) > > > > + * //Second memcpy because of wrapround > > > > + * n2 =3D n - sgd->n1; > > > > + * memcpy (sgd->ptr2, obj[n2], n2 * sizeof(uintptr_t)); > > > > > > Missing { } > > > > > > > + * rte_ring_dequeue_sg_finish(ring, n); > > > > > > Should be enqueue > > > > > Thanks, will correct them. > > > > > > + * } > > > > + * > > > > + * Note that between _start_ and _finish_ none other thread can > > > > + proceed > > > > + * with enqueue(/dequeue) operation till _finish_ completes. > > > > + */ > > > > + > > > > +#ifdef __cplusplus > > > > +extern "C" { > > > > +#endif > > > > + > > > > +#include > > > > + > > > > +/* Rock that needs to be passed between reserve and commit APIs */ > > > > +struct rte_ring_sg_data { > > > > + /* Pointer to the first space in the ring */ > > > > + void **ptr1; > > > > + /* Pointer to the second space in the ring if there is wrap-aroun= d */ > > > > + 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; > > > > +}; > > > > > > Would it be possible to simply return the offset instead of this stru= cture? > > > The wrap could be managed by a __rte_ring_enqueue_elems() function. > > Trying to use __rte_ring_enqueue_elems() will force temporary copy. See= below. > > > > > > > > I mean something like this: > > > > > > uint32_t start; > > > n =3D rte_ring_enqueue_sg_bulk_start(ring, 32, &start, NULL); > > > if (n !=3D 0) { > > > /* Copy objects in the ring. */ > > > __rte_ring_enqueue_elems(ring, start, obj, sizeof(uintptr_t), > > > n); > > For ex: 'obj' here is temporary copy. > > > > > rte_ring_enqueue_sg_finish(ring, n); > > > } > > > > > > It would require to slightly change __rte_ring_enqueue_elems() to sup= port > > > to be called with prod_head >=3D size, and wrap in that case. > > > > > The alternate solution I can think of requires 3 things 1) the base add= ress of the ring 2) Index to where to copy 3) the mask. With these 3 > things one could write the code like below: > > for (i =3D 0; i < n; i++) { > > ring_addr[(index + i) & mask] =3D obj[i]; // ANDing with mask will tak= e care of wrap-around. > > } > > > > However, I think this does not allow for passing the address in the rin= g to another function/API to copy the data (It is possible, but the user > has to calculate the actual address, worry about the wrap-around, second = pointer etc). > > > > The current approach hides some details and provides flexibility to the= application to use the pointer the way it wants. >=20 > I agree that doing the access + masking manually looks too complex. >=20 > However I'm not sure to get why using __rte_ring_enqueue_elems() would > result in an additional copy. I suppose the object that you want to > enqueue is already stored somewhere? >=20 > For instance, let's say you have 10 objects to enqueue, located at > different places: >=20 > void *obj_0_to_3 =3D ; > void *obj_4_to_7 =3D ...; > void *obj_8_to_9 =3D ...; > uint32_t start; > n =3D rte_ring_enqueue_sg_bulk_start(ring, 10, &start, NULL); > if (n !=3D 0) { > __rte_ring_enqueue_elems(ring, start, obj_0_to_3, > sizeof(uintptr_t), 4); > __rte_ring_enqueue_elems(ring, start + 4, obj_4_to_7, > sizeof(uintptr_t), 4); > __rte_ring_enqueue_elems(ring, start + 8, obj_8_to_9, > sizeof(uintptr_t), 2); > rte_ring_enqueue_sg_finish(ring, 10); > } >=20 As I understand, It is not about different objects stored in different plac= es, it is about: a) object is relatively big (16B+ ?) b) You compose objects from values stored in few different places. Let say you have: struct elem_obj {uint64_t a; uint32_t b, c;}; And then you'd like to copy 'a' value from one location, 'b' from second, and 'c' from third one. Konstantin