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 6E030A052A; Thu, 9 Jul 2020 19:55:44 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id A26A21E924; Thu, 9 Jul 2020 19:55:43 +0200 (CEST) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by dpdk.org (Postfix) with ESMTP id 31ADF1E916 for ; Thu, 9 Jul 2020 19:55:41 +0200 (CEST) IronPort-SDR: GBXg2l2bXT0EZAwSE/N/mqAWaROWVEb+nCChqVkvnXvt1O9JdBlcHiid3aK+njsvJF+kmGLLlO FAedQEiY+bhw== X-IronPort-AV: E=McAfee;i="6000,8403,9677"; a="135527369" X-IronPort-AV: E=Sophos;i="5.75,332,1589266800"; d="scan'208";a="135527369" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Jul 2020 10:55:39 -0700 IronPort-SDR: fsXJjPO0yl5VitkiNKR4dYostMx5LfZUhlYItBigY4FlCB8gWr4maxsWvsOELj6BXj+Aux+t++ nETrlz1K+V3Q== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.75,332,1589266800"; d="scan'208";a="428296552" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by orsmga004.jf.intel.com with ESMTP; 09 Jul 2020 10:55:39 -0700 Received: from fmsmsx112.amr.corp.intel.com (10.18.116.6) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 9 Jul 2020 10:55:37 -0700 Received: from FMSEDG002.ED.cps.intel.com (10.1.192.134) by FMSMSX112.amr.corp.intel.com (10.18.116.6) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 9 Jul 2020 10:55:37 -0700 Received: from NAM12-MW2-obe.outbound.protection.outlook.com (104.47.66.43) by edgegateway.intel.com (192.55.55.69) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 9 Jul 2020 10:55:32 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=R4VGYbw0WNJWJLFtaL+TX2w6cVr+KGFMRgRFJBb8l/yhFHGVFZa5BYIMcoNFb/bjVdFiSc7sOwNt3yuXp6IotjUR31t8y9JalYmatt/2TQ9C39cXOHytmWYD/tDJl/QweYTYAr0BNfPhoD49BJ7c1wA62rc7JjUgJO3Sbj+Ys17O42EQ+l0FCVLZyzHcDT416rXbvigTEaYsE8X0KQHQ2TQP4Pv8x1Ae+5sKY+I0FyJlFVT3tW6r0CHo+NbL6HLEPcbsyChdrHYMxII0OH+a14iO3klrOJnugcNfv8uaG/oRGATZ7NrnLuPoYHOuHDhlRJWcDywuJmE7lgvHG81TbQ== 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=ZqlImI8jV0vDVjHiEhgn5f/HxoMoR74Z3J0v+nEC3yw=; b=UBTwWXuKja5L2fqlFOUzYGPF6hCF5jzqE1NQyR7VNTB4sLnIBYQDDoLooRaVBXY533kJebfFYwkMBM+xoV5eDAqUuAp1waLrNsFimp6uQYvAVclWOUdm23lQZmX2M6OY92iTTfuil1mQiObR6oDqDBn4CEZmM6vIgYD2guYpDcGKSCLQphWwGt6Uh3QaEmcYeH89GPeOYPANNAK727+1xkrwIfxYFZYpj99QSXsv0D+tfEPM8RBohN8Mvdrf4m1acg6vD+rvebNl3bu3LzLrnnQXCVJkD19JY30UQ6mWuPS8Eki2wJStzYNRdqqK2/UVJ6dJ1H/RId5ogQod8oPnCg== 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=ZqlImI8jV0vDVjHiEhgn5f/HxoMoR74Z3J0v+nEC3yw=; b=BL+7IDuDkxepIeHEUSq43CUiDwLbTpGsef/R1fH0PIk6Uc3/S5JH1teDBFtLd3dLm9Lhv19suDkOM5gY32YsnMqHt2F4nu8K3mHlFb7gpiVNgThuDiJ+7g+HmQgOlIfS/NtnRloa5XE4ufyg4RQmaVCIyrKN7Lu8RVRu6TpK3uM= Received: from BYAPR11MB3301.namprd11.prod.outlook.com (2603:10b6:a03:7f::26) by BYAPR11MB3254.namprd11.prod.outlook.com (2603:10b6:a03:7c::19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3153.28; Thu, 9 Jul 2020 17:55:31 +0000 Received: from BYAPR11MB3301.namprd11.prod.outlook.com ([fe80::f160:29ab:b8f9:4189]) by BYAPR11MB3301.namprd11.prod.outlook.com ([fe80::f160:29ab:b8f9:4189%6]) with mapi id 15.20.3153.032; Thu, 9 Jul 2020 17:55:31 +0000 From: "Ananyev, Konstantin" To: Olivier Matz CC: "dev@dpdk.org" , "arybchenko@solarflare.com" , "jielong.zjl@antfin.com" , "Eads, Gage" Thread-Topic: [PATCH v2] mempool/ring: add support for new ring sync modes Thread-Index: AQHWTi/h8GtyGouqIUabQ46ZzlWz+aj/fMmAgAAHuBA= Date: Thu, 9 Jul 2020 17:55:30 +0000 Message-ID: References: <20200521132027.28219-1-konstantin.ananyev@intel.com> <20200629161024.29059-1-konstantin.ananyev@intel.com> <20200709161829.GV5869@platinum> In-Reply-To: <20200709161829.GV5869@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.2.0.6 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: [192.198.151.172] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: c7655272-36d3-47ce-2a98-08d82431458a x-ms-traffictypediagnostic: BYAPR11MB3254: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:9508; x-forefront-prvs: 04599F3534 x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: 5jYeWnMc2tZggyvfLWvqyqXRHBEfx9Y+N6s9frIx6tKsHh/YKCxCnNehqSlTS4oi8dQt7+UtJLQFHWaTvn0moKekB8uWJqv6GbVUleR+xniNY1XoBXebzXilFrOSHqnxrjPQ2+duV2yyRpSYHMmv0kN8anzLJFu3wKVUiLyx/KgVBgMJ3autXvjs2ARoBxxHPCxXh7BrEXeGbXW89khugNJuZHjI/D0JwnznAy2q8EOs4Fg6Zs5xiPOWaKoV4S8YUUJ5XBsAl78ZJv00gF1WGtprmZOh43lymffLgB9ITbj8eX8nIk0AsJ75GCDXz+znKr0K+fYpUJi3tVxEH4bMIQ== 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; SFTY:; SFS:(4636009)(396003)(376002)(136003)(39860400002)(346002)(366004)(66946007)(52536014)(8936002)(8676002)(66476007)(107886003)(71200400001)(76116006)(6916009)(86362001)(64756008)(2906002)(83380400001)(478600001)(26005)(6506007)(186003)(7696005)(66556008)(54906003)(9686003)(33656002)(316002)(66446008)(4326008)(5660300002)(55016002); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata: qT2LD5Jar/0115fapGqMKoV7f6M7oIUczFEUb5h+IsT0r97WQ0WVDz0CRtVrsPBQzJBK0RXXcz01HYMp7mF+srwx7xFDWHawyzuZJT52TP9aOZWb4I+NpgVF7YnkTAFD1lLEfmw2E7KqM0w9zoCzN4R2mqwqEcnyDoBI2PU6Bv5a/CAHv5yxlm617YkfMa/BYqNNvasNIUj1SVZyxVbzvS9UVJhWd0Q7bn4eJfTNHJKgEEzJ7/MCxhpY+SuP9rXw0wFYAJTmxSFeyePmrA4UYckukwXxES9Ltjutm3v/wb7H4gVLdUaRrPIwmR8vvOSfGTcpGX+qnKF8vkV872pL5sSRmM6wlx2L4HAS59epnM5a/zmhYzib4zDlYpxOr2dLPaEzCkgnx4+/glL1d1xIlvvinY6hkUjej90VNpyk99r8xRl06VvxhksE1e2yvmoZLNsRBypsyBHmh7srWsUBFdBtjzthsxgHuzHIVis84ckB8VMBKnH1DwXzI7XcyOmn 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: c7655272-36d3-47ce-2a98-08d82431458a X-MS-Exchange-CrossTenant-originalarrivaltime: 09 Jul 2020 17:55:31.0785 (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: HcSN/WsS6XAP9LWaO+F9A7X737nAUC1qw6C0ECwBlGOMJ2KMYVGUN5CabHKUbC5x8LPTUjKncE5f32pRz0wZstnL4rYq6thkiKxE2JE2G6o= X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR11MB3254 X-OriginatorOrg: intel.com Subject: Re: [dpdk-dev] [PATCH v2] mempool/ring: add support for new ring sync modes 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 Olivier, =20 > Hi Konstantin, >=20 > On Mon, Jun 29, 2020 at 05:10:24PM +0100, Konstantin Ananyev wrote: > > v2: > > - update Release Notes (as per comments) > > > > Two new sync modes were introduced into rte_ring: > > relaxed tail sync (RTS) and head/tail sync (HTS). > > This change provides user with ability to select these > > modes for ring based mempool via mempool ops API. > > > > Signed-off-by: Konstantin Ananyev > > Acked-by: Gage Eads > > --- > > doc/guides/rel_notes/release_20_08.rst | 6 ++ > > drivers/mempool/ring/rte_mempool_ring.c | 97 ++++++++++++++++++++++--- > > 2 files changed, 94 insertions(+), 9 deletions(-) > > > > diff --git a/doc/guides/rel_notes/release_20_08.rst b/doc/guides/rel_no= tes/release_20_08.rst > > index eaaf11c37..7bdcf3aac 100644 > > --- a/doc/guides/rel_notes/release_20_08.rst > > +++ b/doc/guides/rel_notes/release_20_08.rst > > @@ -84,6 +84,12 @@ New Features > > * Dump ``rte_flow`` memory consumption. > > * Measure packet per second forwarding. > > > > +* **Added support for new sync modes into mempool ring driver.** > > + > > + Added ability to select new ring synchronisation modes: > > + ``relaxed tail sync (ring_mt_rts)`` and ``head/tail sync (ring_mt_ht= s)`` > > + via mempool ops API. > > + > > > > Removed Items > > ------------- > > diff --git a/drivers/mempool/ring/rte_mempool_ring.c b/drivers/mempool/= ring/rte_mempool_ring.c > > index bc123fc52..15ec7dee7 100644 > > --- a/drivers/mempool/ring/rte_mempool_ring.c > > +++ b/drivers/mempool/ring/rte_mempool_ring.c > > @@ -25,6 +25,22 @@ common_ring_sp_enqueue(struct rte_mempool *mp, void = * const *obj_table, > > obj_table, n, NULL) =3D=3D 0 ? -ENOBUFS : 0; > > } > > > > +static int > > +rts_ring_mp_enqueue(struct rte_mempool *mp, void * const *obj_table, > > + unsigned int n) > > +{ > > + return rte_ring_mp_rts_enqueue_bulk(mp->pool_data, > > + obj_table, n, NULL) =3D=3D 0 ? -ENOBUFS : 0; > > +} > > + > > +static int > > +hts_ring_mp_enqueue(struct rte_mempool *mp, void * const *obj_table, > > + unsigned int n) > > +{ > > + return rte_ring_mp_hts_enqueue_bulk(mp->pool_data, > > + obj_table, n, NULL) =3D=3D 0 ? -ENOBUFS : 0; > > +} > > + > > static int > > common_ring_mc_dequeue(struct rte_mempool *mp, void **obj_table, unsig= ned n) > > { > > @@ -39,17 +55,30 @@ common_ring_sc_dequeue(struct rte_mempool *mp, void= **obj_table, unsigned n) > > obj_table, n, NULL) =3D=3D 0 ? -ENOBUFS : 0; > > } > > > > +static int > > +rts_ring_mc_dequeue(struct rte_mempool *mp, void **obj_table, unsigned= int n) > > +{ > > + return rte_ring_mc_rts_dequeue_bulk(mp->pool_data, > > + obj_table, n, NULL) =3D=3D 0 ? -ENOBUFS : 0; > > +} > > + > > +static int > > +hts_ring_mc_dequeue(struct rte_mempool *mp, void **obj_table, unsigned= int n) > > +{ > > + return rte_ring_mc_hts_dequeue_bulk(mp->pool_data, > > + obj_table, n, NULL) =3D=3D 0 ? -ENOBUFS : 0; > > +} > > + > > static unsigned > > common_ring_get_count(const struct rte_mempool *mp) > > { > > return rte_ring_count(mp->pool_data); > > } > > > > - > > static int > > -common_ring_alloc(struct rte_mempool *mp) > > +ring_alloc(struct rte_mempool *mp, uint32_t rg_flags) > > { > > - int rg_flags =3D 0, ret; > > + int ret; > > char rg_name[RTE_RING_NAMESIZE]; > > struct rte_ring *r; > > > > @@ -60,12 +89,6 @@ common_ring_alloc(struct rte_mempool *mp) > > return -rte_errno; > > } > > > > - /* ring flags */ > > - if (mp->flags & MEMPOOL_F_SP_PUT) > > - rg_flags |=3D RING_F_SP_ENQ; > > - if (mp->flags & MEMPOOL_F_SC_GET) > > - rg_flags |=3D RING_F_SC_DEQ; > > - > > /* > > * Allocate the ring that will be used to store objects. > > * Ring functions will return appropriate errors if we are > > @@ -82,6 +105,40 @@ common_ring_alloc(struct rte_mempool *mp) > > return 0; > > } > > > > +static int > > +common_ring_alloc(struct rte_mempool *mp) > > +{ > > + uint32_t rg_flags; > > + > > + rg_flags =3D 0; >=20 > Maybe it could go on the same line >=20 > > + > > + /* ring flags */ >=20 > Not sure we need to keep this comment >=20 > > + if (mp->flags & MEMPOOL_F_SP_PUT) > > + rg_flags |=3D RING_F_SP_ENQ; > > + if (mp->flags & MEMPOOL_F_SC_GET) > > + rg_flags |=3D RING_F_SC_DEQ; > > + > > + return ring_alloc(mp, rg_flags); > > +} > > + > > +static int > > +rts_ring_alloc(struct rte_mempool *mp) > > +{ > > + if ((mp->flags & (MEMPOOL_F_SP_PUT | MEMPOOL_F_SC_GET)) !=3D 0) > > + return -EINVAL; >=20 > Why do we need this? It is a problem to allow sc/sp in this mode (even > if it's not optimal)? These new sync modes (RTS, HTS) are for MT. For SP/SC - there is simply no point to use MT sync modes. I suppose there are few choices: 1. Make F_SP_PUT/F_SC_GET flags silently override expected ops behaviour and create actual ring with ST sync mode for prod/cons.=20 2. Report an error. 3. Silently ignore these flags. As I can see for "ring_mp_mc" ops, we doing #1,=20 while for "stack" we are doing #3. For RTS/HTS I chosoe #2, as it seems cleaner to me. Any thoughts from your side what preferable behaviour should be? >=20 > > + > > + return ring_alloc(mp, RING_F_MP_RTS_ENQ | RING_F_MC_RTS_DEQ); > > +} > > + > > +static int > > +hts_ring_alloc(struct rte_mempool *mp) > > +{ > > + if ((mp->flags & (MEMPOOL_F_SP_PUT | MEMPOOL_F_SC_GET)) !=3D 0) > > + return -EINVAL; > > + > > + return ring_alloc(mp, RING_F_MP_HTS_ENQ | RING_F_MC_HTS_DEQ); > > +} > > + > > static void > > common_ring_free(struct rte_mempool *mp) > > { > > @@ -130,7 +187,29 @@ static const struct rte_mempool_ops ops_sp_mc =3D = { > > .get_count =3D common_ring_get_count, > > }; > > > > +/* ops for mempool with ring in MT_RTS sync mode */ > > +static const struct rte_mempool_ops ops_mt_rts =3D { > > + .name =3D "ring_mt_rts", > > + .alloc =3D rts_ring_alloc, > > + .free =3D common_ring_free, > > + .enqueue =3D rts_ring_mp_enqueue, > > + .dequeue =3D rts_ring_mc_dequeue, > > + .get_count =3D common_ring_get_count, > > +}; > > + > > +/* ops for mempool with ring in MT_HTS sync mode */ > > +static const struct rte_mempool_ops ops_mt_hts =3D { > > + .name =3D "ring_mt_hts", > > + .alloc =3D hts_ring_alloc, > > + .free =3D common_ring_free, > > + .enqueue =3D hts_ring_mp_enqueue, > > + .dequeue =3D hts_ring_mc_dequeue, > > + .get_count =3D common_ring_get_count, > > +}; > > + > > MEMPOOL_REGISTER_OPS(ops_mp_mc); > > MEMPOOL_REGISTER_OPS(ops_sp_sc); > > MEMPOOL_REGISTER_OPS(ops_mp_sc); > > MEMPOOL_REGISTER_OPS(ops_sp_mc); > > +MEMPOOL_REGISTER_OPS(ops_mt_rts); > > +MEMPOOL_REGISTER_OPS(ops_mt_hts); =20 > Not really related to your patch, but I think we need a function to > dump the name of available mempool ops. We could even add a description. > The problem we have is that a user does not know on which criteria is > should use a driver or another (except for platform drivers). Agree, it will be usefull. Though it probably subject for a separate patch.