From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (xvm-189-124.dc0.ghst.net [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 1E36AA09FF;
	Wed,  6 Jan 2021 17:04:47 +0100 (CET)
Received: from [217.70.189.124] (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id 8F3DD1609D9;
	Wed,  6 Jan 2021 17:04:46 +0100 (CET)
Received: from nat-hk.nvidia.com (nat-hk.nvidia.com [203.18.50.4])
 by mails.dpdk.org (Postfix) with ESMTP id C43B01609D2
 for <dev@dpdk.org>; Wed,  6 Jan 2021 17:04:43 +0100 (CET)
Received: from HKMAIL101.nvidia.com (Not Verified[10.18.92.100]) by
 nat-hk.nvidia.com (using TLS: TLSv1.2, AES256-SHA)
 id <B5ff5df9a0000>; Thu, 07 Jan 2021 00:04:42 +0800
Received: from HKMAIL103.nvidia.com (10.18.16.12) by HKMAIL101.nvidia.com
 (10.18.16.10) with Microsoft SMTP Server (TLS) id 15.0.1473.3; Wed, 6 Jan
 2021 16:04:36 +0000
Received: from NAM12-DM6-obe.outbound.protection.outlook.com (104.47.59.170)
 by HKMAIL103.nvidia.com (10.18.16.12) with Microsoft SMTP Server (TLS) id
 15.0.1473.3 via Frontend Transport; Wed, 6 Jan 2021 16:04:36 +0000
ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none;
 b=BD4eCAnn5NNSiq21v8UydTve4ehKRfGuBH4mFaN84VNBmHm6LiuWWZ3jc9LMQWhP22VBjFiSl3eeeS1i4hwvKev6m/iTflsZVSnPwKbqvpBO5YY9YLBZSrn6xAGbygUHsP5bFaXFk++RvLh/HPv0j+7ah7kmpWlJgKfjUz5BqMGX17qIo5qooJ4XjQ/1FzHchPOrFH2Tdg9jVENGg+9wvqPKcu6mhOAziWfvGkeKk6u0NH+o94jRuu1711xiQllKwoaJj9t7tWWGmnaI8NKgV74xufwMPX3dgFGlcyzFTB7C6c2iSmlgVV76WuhuOYep3fHGtVUIgyqucmMCGrnGdQ==
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=e/OK6q6ubS7HAcyUxMA8QkTQzc4QPR4JJuPEwzOKC28=;
 b=lI/LbuPzF7e2jYh6CR4EYf1we2oGM8ruy9wtfIHwaY6TJ5DstEI3oPIudF0riGtQmncsG2sRUdbRbipXc/8X20CFlI54j2uJobJ91g3IKLB0c57JvSYLbIPAsyuZP7Q6LxtL+mpcrlG8n9sEj/zJ+zqAyepCBPbtIXK5kqG6/ZBIBMCt6PHNtnKoiG3GFjzJJWRV/DMhWfRB2t2AYRFeST+IVPgYVChaCi8SOprSALRRG+TRPUPZUyLxJRQB8VQf/7HsK9Co/8rDb89nQ+n9fgh712i5SQzsbig+/xtJIBDq1TELgAsjTLSV/AjQnaPO0lUHWw6HvXXUXqPn64tdcw==
ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass
 smtp.mailfrom=nvidia.com; dmarc=pass action=none header.from=nvidia.com;
 dkim=pass header.d=nvidia.com; arc=none
Received: from DM6PR12MB3945.namprd12.prod.outlook.com (2603:10b6:5:1c2::27)
 by DM6PR12MB5005.namprd12.prod.outlook.com (2603:10b6:5:1be::23) with
 Microsoft SMTP Server (version=TLS1_2,
 cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3721.20; Wed, 6 Jan
 2021 16:04:34 +0000
Received: from DM6PR12MB3945.namprd12.prod.outlook.com
 ([fe80::d1dc:9fb2:4724:53df]) by DM6PR12MB3945.namprd12.prod.outlook.com
 ([fe80::d1dc:9fb2:4724:53df%7]) with mapi id 15.20.3721.024; Wed, 6 Jan 2021
 16:04:34 +0000
From: Tal Shnaiderman <talshn@nvidia.com>
To: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
CC: "dev@dpdk.org" <dev@dpdk.org>, NBU-Contact-Thomas Monjalon
 <thomas@monjalon.net>, "pallavi.kadam@intel.com" <pallavi.kadam@intel.com>,
 "navasile@linux.microsoft.com" <navasile@linux.microsoft.com>,
 "dmitrym@microsoft.com" <dmitrym@microsoft.com>, "david.marchand@redhat.com"
 <david.marchand@redhat.com>
Thread-Topic: [PATCH v7 2/2] eal: add generic thread-local-storage functions
Thread-Index: AQHW5D1kaZA1Ekx2HEqiTbFhJusoKKoawn3w
Date: Wed, 6 Jan 2021 16:04:33 +0000
Message-ID: <DM6PR12MB39450855FD40253AE55BC178A4D00@DM6PR12MB3945.namprd12.prod.outlook.com>
References: <20201226160848.9824-1-talshn@nvidia.com>
 <20210105170635.6212-1-talshn@nvidia.com>
 <20210105170635.6212-3-talshn@nvidia.com> <20210106180525.3f4dd1a3@sovereign>
In-Reply-To: <20210106180525.3f4dd1a3@sovereign>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
authentication-results: gmail.com; dkim=none (message not signed)
 header.d=none;gmail.com; dmarc=none action=none header.from=nvidia.com;
x-originating-ip: [77.137.141.17]
x-ms-publictraffictype: Email
x-ms-office365-filtering-correlation-id: edf3e4ff-f1d1-4ed9-a056-08d8b25cc25e
x-ms-traffictypediagnostic: DM6PR12MB5005:
x-ld-processed: 43083d15-7273-40c1-b7db-39efd9ccc17a,ExtAddr
x-microsoft-antispam-prvs: <DM6PR12MB5005AE99719C39295949431EA4D00@DM6PR12MB5005.namprd12.prod.outlook.com>
x-ms-oob-tlc-oobclassifiers: OLM:8882;
x-ms-exchange-senderadcheck: 1
x-microsoft-antispam: BCL:0;
x-microsoft-antispam-message-info: SohFBkJPebYl/z+XJZZJlEMClsI8oG+LkOlCpR83vo/Lo2wCj1ZDNmeEjXcxvAWElIkXzF5y86jN+VrvrXPwlJPte+pGTKXawMQ2MirFnKHz4ZQnagjpGaUE/FBsvaJYfjBRe6I7WU4nxmVq4G7Sjd1qZZDBiyMgMU2a4ZG0YyCtSWjunrQu8Fc4uprIv+yu9GRAwj2JPzlhftvFB9gzJ1zswamv9xEK20HmPVIHc1SVu2ymB6SSEDMiv731aWcLQNTkrspq/6kbBDs6HhRc4caP6GGTzvDIvlBfrecK1TrhIzBuW64IraaKPCxSPZyuJ2Ev1Mv13o23l2vvFW5x3HY/qBAlEUft0EQ3tBM1KMk5pDR8d6ITIo+wqxrVA33Q2bUbTtd8pRfNzgAvoPnviqdqI7Xtm1/dCpWtRh7NXn5+OSCPzxGLxR7Tu4Ch/Ivf
x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:;
 IPV:NLI; SFV:NSPM; H:DM6PR12MB3945.namprd12.prod.outlook.com; PTR:; CAT:NONE;
 SFS:(4636009)(136003)(396003)(366004)(376002)(346002)(39860400002)(83380400001)(478600001)(316002)(86362001)(54906003)(7696005)(6916009)(4326008)(9686003)(55016002)(71200400001)(2906002)(8936002)(5660300002)(26005)(64756008)(66476007)(66446008)(76116006)(8676002)(66556008)(186003)(6506007)(66946007)(52536014)(33656002)(41533002);
 DIR:OUT; SFP:1101; 
x-ms-exchange-antispam-messagedata: =?us-ascii?Q?gtxgcUQdgk5vfVPzMbi4xCAV8DYfa5B7mfTmO0HifbhTfTWWPvT90dR309re?=
 =?us-ascii?Q?9RE98y1Q2DMnfBywknmrGGt7LYAVOy0eWkpix+b8apYIb9xoV+FDpf/f3NNt?=
 =?us-ascii?Q?2FEfjfmNltHI5tWISFL4Xclbj4OyqE+IlGJIqPO7QhMWr7L3MEeZ/b7/mq1Y?=
 =?us-ascii?Q?yHsW66vqUKPMeskf7LSrYOPrM2YJ2Gq5DR1/yRNYu7jXadICJKwGKTTxyWz6?=
 =?us-ascii?Q?L42KO/s2hcPqBzSPDKjl4ke92RhFT5yl9Tgj6Shk6OcHHtDkwL8KaWdbkJEc?=
 =?us-ascii?Q?5F5IE3vrENMriodLUxtAahBPL2Be7Ys62rzH2QIOP2FLVI6iA6qZyN8gDo0j?=
 =?us-ascii?Q?2S+lt7lljlpWgFCj2YD97CfBPldc7Jw4/HLLuk2L8mp4Dt1uoByjpMWkCe4e?=
 =?us-ascii?Q?95NhmsDz6roCRt4yv8DzB9QGHKgcyK4VCEGQeBOG3wAN29NXT7D08ADcUsDq?=
 =?us-ascii?Q?FRMQ82YXQxd1irOZWkH6nMdjY4BJtV4Zcor3zfJCjKXzM7blKPiEVVhgIdBw?=
 =?us-ascii?Q?8Zq5eGbAe4VGHBrclSXPSZVMaJ2WaBRsFXXcFGDKkJzpNAwPcjWpB7pbqwTZ?=
 =?us-ascii?Q?EAeQHG9TLApzbCADZfDmnLRTqRKla+bE1hA+xC3aptRVU+6QyOtX8XXsX2gJ?=
 =?us-ascii?Q?+Ej1cfVML2OeXkJA8qV2LBE5bibxCPxLS/khJGnsqx6rFmmGPd2cvERjuhEr?=
 =?us-ascii?Q?vPaBOipJ6G14yIhz4rIiC+RFrCrQ3sRe5uVqCwtJBPqRzuifsd8FDwbFWZwp?=
 =?us-ascii?Q?3MUH4tnY5KYHu1OArgjMXxat0g+zx9ww0sevPQoOXlTJtsbH6DL64w9QCSfG?=
 =?us-ascii?Q?M+nLC8q7A9dXuQ+jA8nIHM9/FCOQTE/yw8ob/Jub6lIRmuWHX6nokjtGw0pF?=
 =?us-ascii?Q?cfvvadxyI3bWNnYbwXsgd1vbE04fa2D0mPmmyyWQ/040ZbYxHA0Y3w2FqT59?=
 =?us-ascii?Q?mYvhrb3kkk4zVUm3K3RovH9Pnbq9nmWzS3RCWn68aYI=3D?=
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: DM6PR12MB3945.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: edf3e4ff-f1d1-4ed9-a056-08d8b25cc25e
X-MS-Exchange-CrossTenant-originalarrivaltime: 06 Jan 2021 16:04:33.9352 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: FCzKJrj/RzJFBiUPlKIfzDBQq4LhQNtAzZ3hn1LUJIenra19Rg9c5o5NHDipxC5PqOy4Wmf9+/vh4brTFExS7A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB5005
X-OriginatorOrg: Nvidia.com
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nvidia.com; s=n1;
 t=1609949082; bh=e/OK6q6ubS7HAcyUxMA8QkTQzc4QPR4JJuPEwzOKC28=;
 h=ARC-Seal:ARC-Message-Signature:ARC-Authentication-Results:From:To:
 CC:Subject:Thread-Topic:Thread-Index:Date:Message-ID:References:
 In-Reply-To:Accept-Language:Content-Language:X-MS-Has-Attach:
 X-MS-TNEF-Correlator:authentication-results:x-originating-ip:
 x-ms-publictraffictype:x-ms-office365-filtering-correlation-id:
 x-ms-traffictypediagnostic:x-ld-processed:
 x-microsoft-antispam-prvs:x-ms-oob-tlc-oobclassifiers:
 x-ms-exchange-senderadcheck:x-microsoft-antispam:
 x-microsoft-antispam-message-info:x-forefront-antispam-report:
 x-ms-exchange-antispam-messagedata:x-ms-exchange-transport-forked:
 Content-Type:Content-Transfer-Encoding:MIME-Version:
 X-MS-Exchange-CrossTenant-AuthAs:
 X-MS-Exchange-CrossTenant-AuthSource:
 X-MS-Exchange-CrossTenant-Network-Message-Id:
 X-MS-Exchange-CrossTenant-originalarrivaltime:
 X-MS-Exchange-CrossTenant-fromentityheader:
 X-MS-Exchange-CrossTenant-id:X-MS-Exchange-CrossTenant-mailboxtype:
 X-MS-Exchange-CrossTenant-userprincipalname:
 X-MS-Exchange-Transport-CrossTenantHeadersStamped:X-OriginatorOrg;
 b=kORf3Ka5umrFhKf9tuCM/CDZYgJ9CnkZ+tnNF7SdQpkfh4NuMMF2PrhN6kdkDhDoF
 ieGXU1Kb9P3vkNZxyFd2qjHIwQeax9rqGO6jD6slgcf74Ip+dTd+pktgP4pe1MGRfU
 QPbNS3sDoPN5hxFLKfGwfqnaqGwZ0VirT2dzstGEhqAivXsm4cS7so2JOYmWPoVeZt
 5i4odUHEu+odbe+zxPv13e8CiwXysIlEDBZ2hszjgq53Lto9vgUe67ftctd/1eSDEN
 qA767zpL7nZGKgU4lqXWPpp0lNKQvixl8olQt7LUqd3p+4H6onEzFMnT7VKjZVmIkT
 x8VHmkD77KBwQ==
Subject: Re: [dpdk-dev] [PATCH v7 2/2] eal: add generic thread-local-storage
 functions
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

> Subject: Re: [PATCH v7 2/2] eal: add generic thread-local-storage functio=
ns
>=20
> External email: Use caution opening links or attachments
>=20
>=20
> On Tue,  5 Jan 2021 19:06:35 +0200, Tal Shnaiderman wrote:
> > Add support for TLS functionality in EAL.
> >
> > The following functions are added:
> > rte_thread_tls_key_create - function to create a TLS data key.
> > rte_thread_tls_key_delete - function to delete a TLS data key.
> > rte_thread_tls_value_set - function to set value bound to the TLS key
> > rte_thread_tls_value_get - function to get value bound to the TLS key
>=20
> "Function to" is redundant both here and in Doxygen comments.
>=20
> > TLS key will be defined by the new type rte_tls_key.
>=20
> Please use present tense when describing patch content.
>=20
> > The API Allocates the thread local storage (TLS) key.
>=20
> Typo: "allocates".
>=20
> > Any thread of the process can subsequently use this key to store and
> > retrieve values that are local to the thread.
> >
> > Those functions are added in addition to TLS capability in
> > rte_per_lcore.h to allow user to use the thread reasouce destruction
> > capability in rte_thread_tls_key_create.
>=20
> Err, I assumed we're abstracting pthread, otherwise the capability has
> already been there. Also, a typo: "resource".
>=20
> > Windows implementation is under librte_eal/windows and implemented
> > using WIN32 API for Windows only.
> >
> > common implementation is under librte_eal/common and implemented
> using
> > pthread for UNIX compilation.
>=20
> librte_eal/unix
>=20
> [...]
> > +/**
> > + * Opaque pointer for TLS key.
> > + */
> > +typedef struct eal_tls_key *rte_tls_key;
>=20
> Suggesting "TLS key type, an opaque pointer". Now it's unclear if there's=
 a
> TLS key and a pointer to it or TLS key is an opaque pointer itself (which=
 is the
> case, API-wise).
>=20
> > +
> >  /**
> >   * Set core affinity of the current thread.
> >   * Support both EAL and non-EAL thread and update TLS.
> > @@ -36,4 +41,67 @@ int rte_thread_set_affinity(rte_cpuset_t *cpusetp);
> >   */
> >  void rte_thread_get_affinity(rte_cpuset_t *cpusetp);
> >
> > +/**
> > + * Function to create a TLS data key visible to all threads in the
> > +process
> > + * function need to be called once to create a key usable by all threa=
ds.
>=20
> The sentences repeat each other and are not separated.
>=20
> > + * rte_tls_key is an opaque pointer used to store the allocated key.
> > + * which is later used to get/set a value.
> > + * and optional destructor can be set to be called when a thread expir=
es.
>=20
> "expires" -> "exits", here and below.
>=20
> No need to repeat parameter description in function description.
>=20
> > + *
> > + * @param key
> > + *   Pointer to store the allocated rte_tls_key
> > + * @param destructor
> > + *   The function to be called when the thread expires.
> > + *   Ignored on Windows OS.
> > + *
> > + * @return
> > + *   On success, zero.
> > + *   On failure, a negative number.
> > + */
> > +
> > +__rte_experimental
> > +int rte_thread_tls_key_create(rte_tls_key *key, void
> > +(*destructor)(void *));
>=20
> > +
> > +/**
> > + * Function to delete a TLS data key visible to all threads in the
> > +process
> > + * rte_tls_key is the opaque pointer allocated by
> rte_thread_tls_key_create.
> > + *
> > + * @param key
> > + *   The rte_tls_key will contain the allocated key
>=20
> "Will"? It's an rte_tls_key key allocated by rte_thread_tls_key_create().
>=20
> > + *
> > + * @return
> > + *   On success, zero.
> > + *   On failure, a negative number.
> > + */
> > +__rte_experimental
> > +int rte_thread_tls_key_delete(rte_tls_key key);
>=20
> > +/**
> > + * Function to set value bound to the tls key on behalf of the
> > +calling thread
>=20
> "tls" -> "TLS"
>=20
> > + *
> > + * @param key
> > + *   The rte_tls_key key allocated by rte_thread_tls_key_create.
> > + * @param value
> > + *   The value bound to the rte_tls_key key for the calling thread.
> > + *
> > + * @return
> > + *   On success, zero.
> > + *   On failure, a negative number.
> > + */
> > +__rte_experimental
> > +int rte_thread_tls_value_set(rte_tls_key key, const void *value);
> > +
> > +/**
> > + * Function to get value bound to the tls key on behalf of the
> > +calling thread
>=20
> "tls" -> "TLS"
>=20
> > + *
> > + * @param key
> > + *   The rte_tls_key key allocated by rte_thread_tls_key_create.
> > + *
> > + * @return
> > + *   On success, value data pointer (can also be NULL).
> > + *   On failure, NULL and an error number is set in rte_errno.
> > + */
> > +__rte_experimental
> > +void *rte_thread_tls_value_get(rte_tls_key key);
> > +
> >  #endif /* _RTE_THREAD_H_ */
>=20
> > diff --git a/lib/librte_eal/windows/rte_thread.c
> > b/lib/librte_eal/windows/rte_thread.c
> > new file mode 100644
> > index 0000000000..4ced283c62
> > --- /dev/null
> > +++ b/lib/librte_eal/windows/rte_thread.c
> > @@ -0,0 +1,83 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright 2021 Mellanox Technologies, Ltd  */
> > +
> > +#include <rte_common.h>
> > +#include <rte_errno.h>
> > +#include <rte_thread.h>
> > +#include <rte_windows.h>
> > +
> > +struct eal_tls_key {
> > +     DWORD thread_index;
> > +};
> > +
> > +int
> > +rte_thread_tls_key_create(rte_tls_key *key,
> > +             __rte_unused void (*destructor)(void *)) {
> > +     *key =3D malloc(sizeof(struct eal_tls_key));
>=20
> sizeof(*key)
>=20
> > +     if ((*key) =3D=3D NULL) {
> > +             RTE_LOG(DEBUG, EAL, "Cannot allocate tls key.");
>=20
> "tls" -> "TLS"
>=20
> > +             return -1;
> > +     }
> > +     (*key)->thread_index =3D TlsAlloc();
> > +     if ((*key)->thread_index =3D=3D TLS_OUT_OF_INDEXES) {
> > +             RTE_LOG_WIN32_ERR("TlsAlloc()");
> > +             free(*key);
> > +             return -1;
> > +     }
> > +     return 0;
> > +}
> > +
> > +int
> > +rte_thread_tls_key_delete(rte_tls_key key) {
> > +     if (!key) {
> > +             RTE_LOG(DEBUG, EAL, "invalid tls key passed to
> > +function.\n");
>=20
> "tls" -> "TLS"
> Messages should start with a capital letter.
> To which "function"?
>=20
> > +             return -1;
> > +     }
> > +     if (!TlsFree(key->thread_index)) {
> > +             RTE_LOG_WIN32_ERR("TlsFree()");
> > +             free(key);
> > +             return -1;
> > +     }
> > +     free(key);
> > +     return 0;
> > +}

Thanks for the review Dmitry, I'll fix and resend both patches.