Message ID | 1468303282-2806-6-git-send-email-shreyansh.jain@nxp.com (mailing list archive) |
---|---|
State | Superseded, archived |
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 7E387532C; Tue, 12 Jul 2016 08:01:18 +0200 (CEST) Received: from NAM03-BY2-obe.outbound.protection.outlook.com (mail-by2nam03on0083.outbound.protection.outlook.com [104.47.42.83]) by dpdk.org (Postfix) with ESMTP id 5D01D530A for <dev@dpdk.org>; Tue, 12 Jul 2016 08:01:17 +0200 (CEST) Received: from DM2PR03CA0034.namprd03.prod.outlook.com (10.141.96.33) by CY1PR0301MB0777.namprd03.prod.outlook.com (10.160.160.13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.528.16; Tue, 12 Jul 2016 06:01:16 +0000 Received: from BY2FFO11FD042.protection.gbl (2a01:111:f400:7c0c::144) by DM2PR03CA0034.outlook.office365.com (2a01:111:e400:2428::33) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.528.16 via Frontend Transport; Tue, 12 Jul 2016 06:01:16 +0000 Authentication-Results: spf=fail (sender IP is 192.88.168.50) smtp.mailfrom=nxp.com; 6wind.com; dkim=none (message not signed) header.d=none; 6wind.com; dmarc=fail action=none header.from=nxp.com; Received-SPF: Fail (protection.outlook.com: domain of nxp.com does not designate 192.88.168.50 as permitted sender) receiver=protection.outlook.com; client-ip=192.88.168.50; helo=tx30smr01.am.freescale.net; Received: from tx30smr01.am.freescale.net (192.88.168.50) by BY2FFO11FD042.mail.protection.outlook.com (10.1.14.227) with Microsoft SMTP Server (TLS) id 15.1.534.7 via Frontend Transport; Tue, 12 Jul 2016 06:01:14 +0000 Received: from Tophie.ap.freescale.net (Tophie.ap.freescale.net [10.232.14.199]) by tx30smr01.am.freescale.net (8.14.3/8.14.0) with ESMTP id u6C60qWd006202; Mon, 11 Jul 2016 23:01:12 -0700 From: Shreyansh Jain <shreyansh.jain@nxp.com> To: <dev@dpdk.org> CC: <viktorin@rehivetech.com>, <thomas.monjalon@6wind.com>, <david.marchand@6wind.com> Date: Tue, 12 Jul 2016 11:31:10 +0530 Message-ID: <1468303282-2806-6-git-send-email-shreyansh.jain@nxp.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1468303282-2806-1-git-send-email-shreyansh.jain@nxp.com> References: <1466510566-9240-1-git-send-email-shreyansh.jain@nxp.com> <1468303282-2806-1-git-send-email-shreyansh.jain@nxp.com> X-EOPAttributedMessage: 0 X-Matching-Connectors: 131127768748396309; (91ab9b29-cfa4-454e-5278-08d120cd25b8); () X-Forefront-Antispam-Report: CIP:192.88.168.50; IPV:NLI; CTRY:US; EFV:NLI; SFV:NSPM; SFS:(10009020)(6009001)(7916002)(2980300002)(1110001)(1109001)(339900001)(199003)(189002)(19580405001)(86362001)(36756003)(5003940100001)(77096005)(305945005)(6806005)(229853001)(19580395003)(105606002)(2351001)(106466001)(81166006)(48376002)(4326007)(81156014)(76176999)(50986999)(189998001)(97736004)(50226002)(50466002)(7846002)(33646002)(8676002)(92566002)(2906002)(87936001)(356003)(8936002)(68736007)(47776003)(586003)(110136002)(11100500001)(2950100001)(85426001)(104016004); DIR:OUT; SFP:1101; SCL:1; SRVR:CY1PR0301MB0777; H:tx30smr01.am.freescale.net; FPR:; SPF:Fail; PTR:InfoDomainNonexistent; A:1; MX:1; LANG:en; X-Microsoft-Exchange-Diagnostics: 1; BY2FFO11FD042; 1:+HP+rkVL91Cjo41za2mKjx17oKCud87J/aLgFoYEnzxJP8P89vvT2+buDXoC0fSePXApdKE5XRHaSP3aMFDdJSop5aoOHNtL7t7kumUy4i3fqgPF4odMBUCWow/rcRgKd2EwHYbVAmfmBG9YlMnlhLFUkVFk9D1Y8Hsus5HApgPJAUFZgFlO9EQ0UZP0qRq0G8tFAxrAQJnvi5hu+jBNl8a1Pw5VW9KpiXhxK5Vn1CN0LbQyOFbxKBUowYKOsyZoX3K3ptQIAYE1f0KSXo61RczQqc40bNiI6kUn8+OcaLUYZgivKEqVMXqZ8BYVRoR+USBdHzKRPfrl2y2FMZLrd5G5wXj4qfFxBs99fzqtyaE9giI3C1rfZP4L+f4U35FdXfZAdLxDpw8A+PX4TcYt20TcLorNbE8+F1oeJfM5jwRP4h0ZesfLoxIk7STcN2nWXfFH6h1F6gHbieynUuAVQxnXwCH1z0h/gUp/8GBMzJRK8cdRvo7VY3JSe8jUzYNsYaddcR25qwNV3uWMechsziLg/b7vJUHUH3Ka7WBbAHzVfSJQEDzbXLVDa+ccBZVNNqk7+hLoXj2yV4OD+/buh0AwCIfXH9Xp+9LtgzqUOr77RE1j2B2yjghiv19nObip MIME-Version: 1.0 Content-Type: text/plain X-MS-Office365-Filtering-Correlation-Id: 878a7f0b-05ba-47f1-a2ac-08d3aa19eed1 X-Microsoft-Exchange-Diagnostics: 1; CY1PR0301MB0777; 2:GLzyBXw4vlqIL9IuAhn7fXV8wCo6LsGtQqCAtyAIQvxUuSYQdXQmbpeMbN2mP0KjcQgwwBENcv4TIE3Li4Dax+ShMuzLXYrPl+RAmew69t+f5KKx2qbzoQ88MDZkhKS3G41BRacMij8VLKouXRDdCqmt4es2GntImIQOkfjPZYSu0X2EkXs21iab+RmlcSs3; 3:80jafjnM4B2uAL/J+CyKxXzw2WZCUYKU6a1kqby5roju1uROAEIZaj+i1wstQbYN5ZjbmBub7amGEf+oQDgjOoa1MYOeCJ99EBJlIrAiqcHDfw7u+M/j0oErgisfLCdL391xvx79lykx2LHd+l+EacEa98YJoWwF+XHmV0IZsBWd0PTrFlnFSZKucwc7DStrBiMbUTzKK8H1zmUkRWYsVsaLWkvFzoJjAxeiLuU0DhQ=; 25:elI6DYVIGDo8jqa85HXs8gSR+7KF9SvGkhuT6W3s83U7MWFCQIXUNXr/nvP6ct1zDqsHNJqbjYxmApp9/YYqUG13vPIY4CIgX8rB5e51gbd2AUWo3QE/FQrp66pnS1cSspNyKbLAjDoABcscZ1zyEX+rKqtmtVEmvr4QsFN3hAG2jqFe+/9/kW/BrcUhRaMWdKASAdZ/jWgx+QZNol4AjyWDg/c6EMA40E4OxubHGlnaovfQsq/2UCYV54PMS7PuZ80GdIW8fEGL76D4QvFFpxuFFM4Y24Hau5zSGMO8r8gjkMrRnRN5SppX5cBuTi+bOxtOFG2ihIcxlV9+/U59O0zSL/agEXlO2sWB7BX+S06DL0QGAHIw9nNxda2Pvr4Dbc+u1NI5gDfgE4Qp0r3F627qaDJVz4fBXcZm3TVBy8U= X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:CY1PR0301MB0777; X-Microsoft-Exchange-Diagnostics: 1; CY1PR0301MB0777; 31:68mZUsAbgDXaXyyZhOU9w4VK0QnTWFdrZv14+jMwrHwSqdN3k6CsMm1m/vMmK23FRrFFFxHboqwjFlWWHh8bI/fe1DE8L0EaXpWzRngRhPJkOMc2LsrINat2vRCtTe2H4jCvmsbyLCbl7Kuyz6hAHHKR9owuKUS/sbXXkF2lVIU8gJvwIDV87LzKwkYU4CdI5EYqixs5aacsRt4Olgbqrg==; 4:zTLFgmrjAg+4Z6ASFR49pSzEw6W53YVJvKMO9ofvvWojKJ2yl/BDuwGaJgQvsyXptWsFMATmpJvnu6PiCN1fXhFGo2eLp0IT72HxqASOV967bgS7B/YQwNvG7e3QU2Qr19WON//jzhHmtNg8Pi2wbQI66/0jl2KJpTgl2mfrF4kCBgE7XQXYCCpZm9XZ1pOfARPP4TdPUKF5+1ekfTLb5+aD03irkxM1RmD4SUAAXhW5bC9smW0lnzfmGXa+FkjgMcDH5qEYdzQAxBZqy+GegQlc2oiGtdlUeHJ6+oM3TS7QGAjVaocKGR/0HWvfgudGMEj878OlTKp2lC07Fe/ojdy7605mDECdRRaTiVpaZvSaWd7jryxxZk9ZzLbrD3AVZ0ExAodOV6yAZB7r9nWKQ3B5ITdFUQQdpGYGF5M7zG6IgO+SrsMhHwBVQvml0kmErhuXeGBT9FjqgBoF3CvWJ3etnHAwfTEs0suMl7E82f5FmoPpgNhyRhpknPu3jmyQo5+S3cTuyslfcnGo+7RoFA== X-Microsoft-Antispam-PRVS: <CY1PR0301MB077755FAD170826170DCC01A90300@CY1PR0301MB0777.namprd03.prod.outlook.com> X-Exchange-Antispam-Report-Test: UriScan:(185117386973197); X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0; RULEID:(601004)(2401047)(13017025)(13024025)(13023025)(13018025)(5005006)(8121501046)(13015025)(3002001)(10201501046)(6055026); SRVR:CY1PR0301MB0777; BCL:0; PCL:0; RULEID:(400006); SRVR:CY1PR0301MB0777; X-Forefront-PRVS: 0001227049 X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1; CY1PR0301MB0777; 23:G7I/L7QCdvzikcq5REimTevG5dp7kcUPyA3Vi7B?= =?us-ascii?Q?KWpzZWxF6VrUkzqX5KHwbR04+EIL/nWKMTfKCZgCjpE4HQabsB+AEsNFlanS?= =?us-ascii?Q?ai0ek6gxcOuRP5AGfcCon/jC5LDzzG/bdxzJvIt3y/L03gIVnwjZrb3wWO8c?= =?us-ascii?Q?i7pZ0S8W0U1UURB+BSB1/sJbiOg36KLXA6PX08s7+2HzRb4Vg8tgZ5c+tlUR?= =?us-ascii?Q?JPKAYVbZuQVQOfwbUFepAZFRWLiM//7qhWWJi5P48UG7WDchHqGWqMkxi1UE?= =?us-ascii?Q?3J7n08DgUDUeY2QCqf3M4v8BT/mFXm2fMxYvzk7A/9FO74EwmhAkIDw/CsYt?= =?us-ascii?Q?8gqSpKpclHHRwFyvrZS735B/tdVyj1B/T66YmTYON+N45tAhBs9e6AWM39lG?= =?us-ascii?Q?eFrOXxFInW+Oeh/+B12AjLHWXulIerZFfsq/P14MV5R8z5hjj+AKhIbzlPFb?= =?us-ascii?Q?NY4F5Rcsz1cy05zRHV3MV813gecYnv+KaW6wYmG9hACP7YFFxzh49CQnsOb3?= =?us-ascii?Q?97U3pVB7J1TJEoiOnmkpZoQvTeQ2OP2Sjox4YysH+PQNq4wexyBc6+ebWkIk?= =?us-ascii?Q?UFFM9ov+rAb50mbcYJw1k7KfHDWEGwJ04UD/CIr0jHFeE/vEAyW5f+db1J8y?= =?us-ascii?Q?eabXWaTbvei/Tra50Iu99kTpgVKTjJSFBWS26sMNQCsSJK9NON/LRERMzMqh?= =?us-ascii?Q?YH9gJ72EUKFg7XkQ1uenIwsGcFlU2Vii8kY8YkUOSY+P8O5c8gNDkYonCt/V?= =?us-ascii?Q?qGWlH3BfP6uba7QkDbQL6lkcXn6/Jky0bkER4FMTYjy8n9CDEDz245DsYJQI?= =?us-ascii?Q?wO8E1xvzUGOm0QVOQa4oJOzhr/HOjXrNrBAG4a49zhKz2vl6pJbG56WFYFvE?= =?us-ascii?Q?X9Vxqhl53X0J8bfxeJ6PDjW19UptndNMMrRJNoVG9wvkfMq9QzfMaEdEXFwM?= =?us-ascii?Q?ef3DChxHWsXpS6jhNF//6NQx5p6ygb/51Vcip50hRBIW1TdsrvXVf2vMykfy?= =?us-ascii?Q?h1R9v/w2rcKJOI/ZMOcxTmYXlm59jjbCB/b/nQf5FPoyCcMEQdEFlAYzBnIy?= =?us-ascii?Q?bsg071QTAye5oieGJR/apaRjIshyLAyyqsPKo/rxbt8OUFnpW2F9tSYY9Kt4?= =?us-ascii?Q?Pv/rksEDTTfk=3D?= X-Microsoft-Exchange-Diagnostics: 1; CY1PR0301MB0777; 6:6P0kAt2ISnyEdIQGzpQdTnyAFI+RGNSFugG03D8wH6wHsKvEXkrgA5NLVDD4GIiHQ8nLJcDb+L5sOF1iIVSia6XRoW2do1I8sJDlwOKEyezU/OEdLkjEDKj0+UUbB571qWhB8DXHY2Dv7lT6ArOgh+tg8y7z5pBAZjHDdZiif86pmv2hT5Gs0C+63RDkKGgHmmSvjGhkNV6zC/N9KZCpYdNM/lTkI9s7qCwy2jK+k8DpULn1sLsyR8DRynvZdMCBr1AVqWOQmPu/p3UzZMkFSR/HNn52vvE+AdPQW8vTl5s=; 5:593/8ccbfy251w2i2KxwPaHGOIiPbQUQzFlY/cfbxdO8TVcWUQnWsPI8gPi0YZ7N09n6+WmSZFTtl3asNePKVHPVdoahSEhJ1gCHrjwA/OVFEvWcqNvZwO5UDjjuwVVDQPAWgm73zZRsetkIKaacoc6kuSgb8n09S+HCguv2d7s=; 24:9xqxBea62p4IiemFo5rN0M/oB7ji2OSFqRBrDB0zJyKfJquCBiud3hbQcpxGqIVjpYcZjJl4VwJfXUe+SrSZnJ+skLAKV4ltB+QKrwxfrw0=; 7:/AunCz8AbX0rBwfMctK2/sBQkxwibtKDcCtgGfxd0y3NZIgwUq7x8a+lpHkHJStYTP3GS0vhPcMqqSXEtMRyhiqdnkOjFc6pCbr/0jfhF+ab/i6p0mUq9QqC3rnRk2sXZBMYJBX+U/WijviytB5VHS2/XRFXkdodOT22HYcnTlYI0EOaCsSmYPNgVYoFJgvPz7pG6r3FOAEb57qRhXEGUnh0JFHQ6sbzskOsiXd56QASX1JsvZi8I9SwcJL1fGmp SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Jul 2016 06:01:14.5120 (UTC) X-MS-Exchange-CrossTenant-Id: 5afe0b00-7697-4969-b663-5eab37d5f47e X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=5afe0b00-7697-4969-b663-5eab37d5f47e; Ip=[192.88.168.50]; Helo=[tx30smr01.am.freescale.net] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY1PR0301MB0777 Subject: [dpdk-dev] [PATCH v6 05/17] eal: introduce init macros X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Shreyansh Jain
July 12, 2016, 6:01 a.m. UTC
Introduce a RTE_INIT macro used to mark an init function as a constructor. Current eal macros have been converted to use this (no functional impact). DRIVER_REGISTER_PCI is added as a helper for pci drivers. Suggested-by: Jan Viktorin <viktorin@rehivetech.com> Signed-off-by: David Marchand <david.marchand@6wind.com> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com> --- lib/librte_eal/common/include/rte_dev.h | 4 ++-- lib/librte_eal/common/include/rte_eal.h | 3 +++ lib/librte_eal/common/include/rte_pci.h | 8 ++++++++ lib/librte_eal/common/include/rte_tailq.h | 4 ++-- 4 files changed, 15 insertions(+), 4 deletions(-)
Comments
Hello Shreyansh, On Tue, 12 Jul 2016 11:31:10 +0530 Shreyansh Jain <shreyansh.jain@nxp.com> wrote: > Introduce a RTE_INIT macro used to mark an init function as a constructor. > Current eal macros have been converted to use this (no functional impact). > DRIVER_REGISTER_PCI is added as a helper for pci drivers. > > Suggested-by: Jan Viktorin <viktorin@rehivetech.com> > Signed-off-by: David Marchand <david.marchand@6wind.com> > Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com> > --- [...] > +#define RTE_INIT(func) \ > +static void __attribute__((constructor, used)) func(void) > + > #ifdef __cplusplus > } > #endif > diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h > index fa74962..3027adf 100644 > --- a/lib/librte_eal/common/include/rte_pci.h > +++ b/lib/librte_eal/common/include/rte_pci.h > @@ -470,6 +470,14 @@ void rte_eal_pci_dump(FILE *f); > */ > void rte_eal_pci_register(struct rte_pci_driver *driver); > > +/** Helper for PCI device registeration from driver (eth, crypto) instance */ > +#define DRIVER_REGISTER_PCI(nm, drv) \ > +RTE_INIT(pciinitfn_ ##nm); \ > +static void pciinitfn_ ##nm(void) \ > +{ \ You are missing setting the name here like PMD_REGISTER_DRIVER does now. Or should I include it in my patch set? (drv).name = RTE_STR(nm); > + rte_eal_pci_register(&drv.pci_drv); \ > +} > + > /** > * Unregister a PCI driver. > * > diff --git a/lib/librte_eal/common/include/rte_tailq.h b/lib/librte_eal/common/include/rte_tailq.h > index 4a686e6..71ed3bb 100644 > --- a/lib/librte_eal/common/include/rte_tailq.h > +++ b/lib/librte_eal/common/include/rte_tailq.h > @@ -148,8 +148,8 @@ struct rte_tailq_head *rte_eal_tailq_lookup(const char *name); > int rte_eal_tailq_register(struct rte_tailq_elem *t); > > #define EAL_REGISTER_TAILQ(t) \ > -void tailqinitfn_ ##t(void); \ > -void __attribute__((constructor, used)) tailqinitfn_ ##t(void) \ > +RTE_INIT(tailqinitfn_ ##t); \ > +static void tailqinitfn_ ##t(void) \ > { \ > if (rte_eal_tailq_register(&t) < 0) \ > rte_panic("Cannot initialize tailq: %s\n", t.name); \
On Wed, 13 Jul 2016 11:20:43 +0200 Jan Viktorin <viktorin@rehivetech.com> wrote: > Hello Shreyansh, > > On Tue, 12 Jul 2016 11:31:10 +0530 > Shreyansh Jain <shreyansh.jain@nxp.com> wrote: > > > Introduce a RTE_INIT macro used to mark an init function as a constructor. > > Current eal macros have been converted to use this (no functional impact). > > DRIVER_REGISTER_PCI is added as a helper for pci drivers. > > > > Suggested-by: Jan Viktorin <viktorin@rehivetech.com> > > Signed-off-by: David Marchand <david.marchand@6wind.com> > > Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com> > > --- > > [...] > > > +#define RTE_INIT(func) \ > > +static void __attribute__((constructor, used)) func(void) > > + > > #ifdef __cplusplus > > } > > #endif > > diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h > > index fa74962..3027adf 100644 > > --- a/lib/librte_eal/common/include/rte_pci.h > > +++ b/lib/librte_eal/common/include/rte_pci.h > > @@ -470,6 +470,14 @@ void rte_eal_pci_dump(FILE *f); > > */ > > void rte_eal_pci_register(struct rte_pci_driver *driver); > > > > +/** Helper for PCI device registeration from driver (eth, crypto) instance */ > > +#define DRIVER_REGISTER_PCI(nm, drv) \ > > +RTE_INIT(pciinitfn_ ##nm); \ > > +static void pciinitfn_ ##nm(void) \ > > +{ \ > > You are missing setting the name here like PMD_REGISTER_DRIVER does > now. Or should I include it in my patch set? > > (drv).name = RTE_STR(nm); Moreover, it should accept the rte_pci_driver *, shouldn't it? Here, it expects a wrapper around it (eth_driver)... I now, my SoC patches were supposing the some... but I think it is wrong. The original David's patch set contains calls like this: RTE_EAL_PCI_REGISTER(bnx2xvf, rte_bnx2xvf_pmd.pci_drv); So, I think, we should go the original way. Jan > > > + rte_eal_pci_register(&drv.pci_drv); \ > > +} > > + > > /** > > * Unregister a PCI driver. > > * > > diff --git a/lib/librte_eal/common/include/rte_tailq.h b/lib/librte_eal/common/include/rte_tailq.h > > index 4a686e6..71ed3bb 100644 > > --- a/lib/librte_eal/common/include/rte_tailq.h > > +++ b/lib/librte_eal/common/include/rte_tailq.h > > @@ -148,8 +148,8 @@ struct rte_tailq_head *rte_eal_tailq_lookup(const char *name); > > int rte_eal_tailq_register(struct rte_tailq_elem *t); > > > > #define EAL_REGISTER_TAILQ(t) \ > > -void tailqinitfn_ ##t(void); \ > > -void __attribute__((constructor, used)) tailqinitfn_ ##t(void) \ > > +RTE_INIT(tailqinitfn_ ##t); \ > > +static void tailqinitfn_ ##t(void) \ > > { \ > > if (rte_eal_tailq_register(&t) < 0) \ > > rte_panic("Cannot initialize tailq: %s\n", t.name); \ > > >
Hi Jan, On Wednesday 13 July 2016 11:04 PM, Jan Viktorin wrote: > On Wed, 13 Jul 2016 11:20:43 +0200 > Jan Viktorin <viktorin@rehivetech.com> wrote: > >> Hello Shreyansh, >> >> On Tue, 12 Jul 2016 11:31:10 +0530 >> Shreyansh Jain <shreyansh.jain@nxp.com> wrote: >> >>> Introduce a RTE_INIT macro used to mark an init function as a constructor. >>> Current eal macros have been converted to use this (no functional impact). >>> DRIVER_REGISTER_PCI is added as a helper for pci drivers. >>> >>> Suggested-by: Jan Viktorin <viktorin@rehivetech.com> >>> Signed-off-by: David Marchand <david.marchand@6wind.com> >>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com> >>> --- >> >> [...] >> >>> +#define RTE_INIT(func) \ >>> +static void __attribute__((constructor, used)) func(void) >>> + >>> #ifdef __cplusplus >>> } >>> #endif >>> diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h >>> index fa74962..3027adf 100644 >>> --- a/lib/librte_eal/common/include/rte_pci.h >>> +++ b/lib/librte_eal/common/include/rte_pci.h >>> @@ -470,6 +470,14 @@ void rte_eal_pci_dump(FILE *f); >>> */ >>> void rte_eal_pci_register(struct rte_pci_driver *driver); >>> >>> +/** Helper for PCI device registeration from driver (eth, crypto) instance */ >>> +#define DRIVER_REGISTER_PCI(nm, drv) \ >>> +RTE_INIT(pciinitfn_ ##nm); \ >>> +static void pciinitfn_ ##nm(void) \ >>> +{ \ >> >> You are missing setting the name here like PMD_REGISTER_DRIVER does >> now. Or should I include it in my patch set? >> >> (drv).name = RTE_STR(nm); That is a miss from my side. I will publish v7 with this. You want this right away or should I wait a little while (more reviews, or any pending additions as per Thomas's notes) before publishing? > > Moreover, it should accept the rte_pci_driver *, shouldn't it? Here, it > expects a wrapper around it (eth_driver)... I now, my SoC patches were > supposing the some... but I think it is wrong. > > The original David's patch set contains calls like this: > > RTE_EAL_PCI_REGISTER(bnx2xvf, rte_bnx2xvf_pmd.pci_drv); > > So, I think, we should go the original way. I have a slightly different opinion of the above. IMO, aim of the helpers is to hide the PCI details and continue to make driver consider itself as a generic ETH driver. In that case, dereferencing pci_drv would be done by macro. Also, considering that in future pci_drv would also have soc_drv, the helpers can effectively hide the intra-structure naming of these. It would help when more such device types (would there be?) are introduced - in which case, driver framework has a consistent coding convention. But, I am ok switching back to David's way as well - I don't have any strong argument against that. > > Jan > >> >>> + rte_eal_pci_register(&drv.pci_drv); \ >>> +} >>> + >>> /** >>> * Unregister a PCI driver. >>> * [...] - Shreyansh
On Thu, 14 Jul 2016 10:57:55 +0530 Shreyansh jain <shreyansh.jain@nxp.com> wrote: > Hi Jan, > > On Wednesday 13 July 2016 11:04 PM, Jan Viktorin wrote: > > On Wed, 13 Jul 2016 11:20:43 +0200 > > Jan Viktorin <viktorin@rehivetech.com> wrote: > > > >> Hello Shreyansh, > >> > >> On Tue, 12 Jul 2016 11:31:10 +0530 > >> Shreyansh Jain <shreyansh.jain@nxp.com> wrote: > >> > >>> Introduce a RTE_INIT macro used to mark an init function as a constructor. > >>> Current eal macros have been converted to use this (no functional impact). > >>> DRIVER_REGISTER_PCI is added as a helper for pci drivers. > >>> > >>> Suggested-by: Jan Viktorin <viktorin@rehivetech.com> > >>> Signed-off-by: David Marchand <david.marchand@6wind.com> > >>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com> > >>> --- > >> > >> [...] > >> > >>> +#define RTE_INIT(func) \ > >>> +static void __attribute__((constructor, used)) func(void) > >>> + > >>> #ifdef __cplusplus > >>> } > >>> #endif > >>> diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h > >>> index fa74962..3027adf 100644 > >>> --- a/lib/librte_eal/common/include/rte_pci.h > >>> +++ b/lib/librte_eal/common/include/rte_pci.h > >>> @@ -470,6 +470,14 @@ void rte_eal_pci_dump(FILE *f); > >>> */ > >>> void rte_eal_pci_register(struct rte_pci_driver *driver); > >>> > >>> +/** Helper for PCI device registeration from driver (eth, crypto) instance */ > >>> +#define DRIVER_REGISTER_PCI(nm, drv) \ > >>> +RTE_INIT(pciinitfn_ ##nm); \ > >>> +static void pciinitfn_ ##nm(void) \ > >>> +{ \ > >> > >> You are missing setting the name here like PMD_REGISTER_DRIVER does > >> now. Or should I include it in my patch set? > >> > >> (drv).name = RTE_STR(nm); > > That is a miss from my side. > I will publish v7 with this. You want this right away or should I wait a little while (more reviews, or any pending additions as per Thomas's notes) before publishing? Please. The time is almost gone. 18/7/2016 is the release (according to the roadmap)... I have to fix it in my patchset, otherwise it does not build (after moving the .name from rte_pci_driver to rte_driver). > > > > > Moreover, it should accept the rte_pci_driver *, shouldn't it? Here, it > > expects a wrapper around it (eth_driver)... I now, my SoC patches were > > supposing the some... but I think it is wrong. > > > > The original David's patch set contains calls like this: > > > > RTE_EAL_PCI_REGISTER(bnx2xvf, rte_bnx2xvf_pmd.pci_drv); > > > > So, I think, we should go the original way. > > I have a slightly different opinion of the above. > IMO, aim of the helpers is to hide the PCI details and continue to make driver consider itself as a generic ETH driver. In that case, dereferencing pci_drv would be done by macro. In this case, I'd prefer to see DRIVER_REGISTER_PCI_ETH. At first, this was also my way of thinking. But I've changed my mind. I find it to be a bit overdesigned. > > Also, considering that in future pci_drv would also have soc_drv, the helpers can effectively hide the intra-structure naming of these. It would help when more such device types (would there be?) are introduced - in which case, driver framework has a consistent coding convention. Hide? I am afraid, I don't understand clearly what you mean. > > But, I am ok switching back to David's way as well - I don't have any strong argument against that. I'd like to preserve the clear semantics. That is DRIVER_REGISTER_PCI -> give a pci device. Has anybody a different opinion? David? Thomas? > > > > > Jan > > > >> > >>> + rte_eal_pci_register(&drv.pci_drv); \ > >>> +} > >>> + > >>> /** > >>> * Unregister a PCI driver. > >>> * > [...] > > - > Shreyansh >
On Thursday 14 July 2016 09:27 PM, Jan Viktorin wrote: > On Thu, 14 Jul 2016 10:57:55 +0530 > Shreyansh jain <shreyansh.jain@nxp.com> wrote: > >> Hi Jan, >> >> On Wednesday 13 July 2016 11:04 PM, Jan Viktorin wrote: >>> On Wed, 13 Jul 2016 11:20:43 +0200 >>> Jan Viktorin <viktorin@rehivetech.com> wrote: >>> >>>> Hello Shreyansh, >>>> >>>> On Tue, 12 Jul 2016 11:31:10 +0530 >>>> Shreyansh Jain <shreyansh.jain@nxp.com> wrote: >>>> >>>>> Introduce a RTE_INIT macro used to mark an init function as a constructor. >>>>> Current eal macros have been converted to use this (no functional impact). >>>>> DRIVER_REGISTER_PCI is added as a helper for pci drivers. >>>>> >>>>> Suggested-by: Jan Viktorin <viktorin@rehivetech.com> >>>>> Signed-off-by: David Marchand <david.marchand@6wind.com> >>>>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com> >>>>> --- >>>> >>>> [...] >>>> >>>>> +#define RTE_INIT(func) \ >>>>> +static void __attribute__((constructor, used)) func(void) >>>>> + >>>>> #ifdef __cplusplus >>>>> } >>>>> #endif >>>>> diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h >>>>> index fa74962..3027adf 100644 >>>>> --- a/lib/librte_eal/common/include/rte_pci.h >>>>> +++ b/lib/librte_eal/common/include/rte_pci.h >>>>> @@ -470,6 +470,14 @@ void rte_eal_pci_dump(FILE *f); >>>>> */ >>>>> void rte_eal_pci_register(struct rte_pci_driver *driver); >>>>> >>>>> +/** Helper for PCI device registeration from driver (eth, crypto) instance */ >>>>> +#define DRIVER_REGISTER_PCI(nm, drv) \ >>>>> +RTE_INIT(pciinitfn_ ##nm); \ >>>>> +static void pciinitfn_ ##nm(void) \ >>>>> +{ \ >>>> >>>> You are missing setting the name here like PMD_REGISTER_DRIVER does >>>> now. Or should I include it in my patch set? >>>> >>>> (drv).name = RTE_STR(nm); >> >> That is a miss from my side. >> I will publish v7 with this. You want this right away or should I wait a little while (more reviews, or any pending additions as per Thomas's notes) before publishing? > > Please. The time is almost gone. 18/7/2016 is the release (according > to the roadmap)... I have to fix it in my patchset, otherwise it > does not build (after moving the .name from rte_pci_driver to > rte_driver). > I didn't consider 18/Jul. Please go ahead. I will continue to send v7 _without_ the above change so that your patchset doesn't break. This way you will not get blocked because of me. >> >>> >>> Moreover, it should accept the rte_pci_driver *, shouldn't it? Here, it >>> expects a wrapper around it (eth_driver)... I now, my SoC patches were >>> supposing the some... but I think it is wrong. >>> >>> The original David's patch set contains calls like this: >>> >>> RTE_EAL_PCI_REGISTER(bnx2xvf, rte_bnx2xvf_pmd.pci_drv); >>> >>> So, I think, we should go the original way. >> >> I have a slightly different opinion of the above. >> IMO, aim of the helpers is to hide the PCI details and continue to make driver consider itself as a generic ETH driver. In that case, dereferencing pci_drv would be done by macro. > > In this case, I'd prefer to see DRIVER_REGISTER_PCI_ETH. > > At first, this was also my way of thinking. But I've changed my mind. I > find it to be a bit overdesigned. There is: DRIVER_REGISTER_PCI(...) DRIVER_REGISTER_PCI_TABLE(...) Wouldn't DRIVER_REGISTER_PCI_ETH look out-of-place? > >> >> Also, considering that in future pci_drv would also have soc_drv, the helpers can effectively hide the intra-structure naming of these. It would help when more such device types (would there be?) are introduced - in which case, driver framework has a consistent coding convention. > > Hide? I am afraid, I don't understand clearly what you mean. DRIVER_REGISTER_PCI(eth_driver) DRIVER_REGISTER_SOC(eth_driver) DRIVER_REGISTER_XXX(eth_driver) ... In either case, the caller always creates the eth_driver and populates internal specific driver structure (pci_drv) as a sub-part of eth_driver specification. Macro 'hides' the internal structure name (pci_drv, soc_drv...). But again, nothing critical. Just a way of usage. We might not even have a 'XXX' in near future. > >> >> But, I am ok switching back to David's way as well - I don't have any strong argument against that. > > I'd like to preserve the clear semantics. That is DRIVER_REGISTER_PCI > -> give a pci device. > > Has anybody a different opinion? David? Thomas? Yes please. Or else, if nothing comes up soon, I will simply go ahead and change to DRIVER_REGISTER_PCI(eth_driver.pci_drv) as this trivial issue shouldn't hold back this series. > >> >>> >>> Jan >>> >>>> >>>>> + rte_eal_pci_register(&drv.pci_drv); \ >>>>> +} >>>>> + >>>>> /** >>>>> * Unregister a PCI driver. >>>>> * >> [...] >> >> - >> Shreyansh >> > > >
Hi guys, 2016-07-15 16:18, Shreyansh jain: > On Thursday 14 July 2016 09:27 PM, Jan Viktorin wrote: > > Please. The time is almost gone. 18/7/2016 is the release (according > > to the roadmap)... I have to fix it in my patchset, otherwise it > > does not build (after moving the .name from rte_pci_driver to > > rte_driver). > > I didn't consider 18/Jul. > Please go ahead. I will continue to send v7 _without_ the above change > so that your patchset doesn't break. This way you will not get blocked > because of me. [...] > > I'd like to preserve the clear semantics. That is DRIVER_REGISTER_PCI > > -> give a pci device. > > > > Has anybody a different opinion? David? Thomas? > > Yes please. > Or else, if nothing comes up soon, I will simply go ahead and change to > DRIVER_REGISTER_PCI(eth_driver.pci_drv) as this trivial issue shouldn't > hold back this series. I'm sorry I have no time to review these series shortly. I think it is too late to consider an integration in 16.07 unfortunately. I feel you are doing a great job and we can target to have these changes in the early days of 16.11 (in August). As this is a big refactoring, it is probably a good idea to integrate it in the beginning of the next release integration cycle.
Hi Jan, On Friday 15 July 2016 04:18 PM, Shreyansh jain wrote: > On Thursday 14 July 2016 09:27 PM, Jan Viktorin wrote: >> On Thu, 14 Jul 2016 10:57:55 +0530 >> Shreyansh jain <shreyansh.jain@nxp.com> wrote: >> >>> Hi Jan, >>> >>> On Wednesday 13 July 2016 11:04 PM, Jan Viktorin wrote: >>>> On Wed, 13 Jul 2016 11:20:43 +0200 >>>> Jan Viktorin <viktorin@rehivetech.com> wrote: >>>> >>>>> Hello Shreyansh, >>>>> >>>>> On Tue, 12 Jul 2016 11:31:10 +0530 >>>>> Shreyansh Jain <shreyansh.jain@nxp.com> wrote: >>>>> >>>>>> Introduce a RTE_INIT macro used to mark an init function as a constructor. >>>>>> Current eal macros have been converted to use this (no functional impact). >>>>>> DRIVER_REGISTER_PCI is added as a helper for pci drivers. >>>>>> >>>>>> Suggested-by: Jan Viktorin <viktorin@rehivetech.com> >>>>>> Signed-off-by: David Marchand <david.marchand@6wind.com> >>>>>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com> >>>>>> --- >>>>> >>>>> [...] >>>>> >>>>>> +#define RTE_INIT(func) \ >>>>>> +static void __attribute__((constructor, used)) func(void) >>>>>> + >>>>>> #ifdef __cplusplus >>>>>> } >>>>>> #endif >>>>>> diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h >>>>>> index fa74962..3027adf 100644 >>>>>> --- a/lib/librte_eal/common/include/rte_pci.h >>>>>> +++ b/lib/librte_eal/common/include/rte_pci.h >>>>>> @@ -470,6 +470,14 @@ void rte_eal_pci_dump(FILE *f); >>>>>> */ >>>>>> void rte_eal_pci_register(struct rte_pci_driver *driver); >>>>>> >>>>>> +/** Helper for PCI device registeration from driver (eth, crypto) instance */ >>>>>> +#define DRIVER_REGISTER_PCI(nm, drv) \ >>>>>> +RTE_INIT(pciinitfn_ ##nm); \ >>>>>> +static void pciinitfn_ ##nm(void) \ >>>>>> +{ \ >>>>> >>>>> You are missing setting the name here like PMD_REGISTER_DRIVER does >>>>> now. Or should I include it in my patch set? >>>>> >>>>> (drv).name = RTE_STR(nm); >>> >>> That is a miss from my side. >>> I will publish v7 with this. You want this right away or should I wait a little while (more reviews, or any pending additions as per Thomas's notes) before publishing? >> >> Please. The time is almost gone. 18/7/2016 is the release (according >> to the roadmap)... I have to fix it in my patchset, otherwise it >> does not build (after moving the .name from rte_pci_driver to >> rte_driver). >> > > I didn't consider 18/Jul. > Please go ahead. I will continue to send v7 _without_ the above change so that your patchset doesn't break. This way you will not get blocked because of me. > Now that we have already skipped the 16.07, I will fix this in my code and release an updated version as soon as 16.07 is officially available. Is that OK? Also, I have fixed most review comments except [1]. I am still not sure of the impact of this change. So, I will publish the v7 without this and then probably a v8 in case this change has any impact. That way we can have your patchset not blocked because of this series. [1] http://dpdk.org/ml/archives/dev/2016-July/044004.html [...] - Shreyansh
On Thu, 28 Jul 2016 15:06:10 +0530 Shreyansh Jain <shreyansh.jain@nxp.com> wrote: > Hi Jan, > > On Friday 15 July 2016 04:18 PM, Shreyansh jain wrote: > > On Thursday 14 July 2016 09:27 PM, Jan Viktorin wrote: [...] > >>>>> (drv).name = RTE_STR(nm); > >>> > >>> That is a miss from my side. > >>> I will publish v7 with this. You want this right away or should I wait a little while (more reviews, or any pending additions as per Thomas's notes) before publishing? > >> > >> Please. The time is almost gone. 18/7/2016 is the release (according > >> to the roadmap)... I have to fix it in my patchset, otherwise it > >> does not build (after moving the .name from rte_pci_driver to > >> rte_driver). > >> > > > > I didn't consider 18/Jul. > > Please go ahead. I will continue to send v7 _without_ the above change so that your patchset doesn't break. This way you will not get blocked because of me. > > > > Now that we have already skipped the 16.07, I will fix this in my code and release an updated version as soon as 16.07 is officially available. Is that OK? Sure :). The thing is that during August-September, I've got significantly less time for this then in June-July :/. That's why I was trying to fit in the 16.07. > > Also, I have fixed most review comments except [1]. I am still not sure of the impact of this change. So, I will publish the v7 without this and then probably a v8 in case this change has any impact. I have no idea about this. Hopefully, it's not a very serious thing. > That way we can have your patchset not blocked because of this series. Thank you! > > [1] http://dpdk.org/ml/archives/dev/2016-July/044004.html > > [...] > > > - > Shreyansh
Hi Jan, On Saturday 30 July 2016 01:44 PM, Jan Viktorin wrote: > On Thu, 28 Jul 2016 15:06:10 +0530 > Shreyansh Jain <shreyansh.jain@nxp.com> wrote: > >> Hi Jan, >> >> On Friday 15 July 2016 04:18 PM, Shreyansh jain wrote: >>> On Thursday 14 July 2016 09:27 PM, Jan Viktorin wrote: > > [...] > >>>>>>> (drv).name = RTE_STR(nm); >>>>> >>>>> That is a miss from my side. >>>>> I will publish v7 with this. You want this right away or should I wait a little while (more reviews, or any pending additions as per Thomas's notes) before publishing? >>>> >>>> Please. The time is almost gone. 18/7/2016 is the release (according >>>> to the roadmap)... I have to fix it in my patchset, otherwise it >>>> does not build (after moving the .name from rte_pci_driver to >>>> rte_driver). >>>> >>> >>> I didn't consider 18/Jul. >>> Please go ahead. I will continue to send v7 _without_ the above change so that your patchset doesn't break. This way you will not get blocked because of me. >>> >> >> Now that we have already skipped the 16.07, I will fix this in my code and release an updated version as soon as 16.07 is officially available. Is that OK? > > Sure :). The thing is that during August-September, I've got significantly > less time for this then in June-July :/. That's why I was trying to fit in > the 16.07. Well, then lets try and get things out for review as soon as we can. I have sent across v7 of rte_driver set. Unfortunately, I forgot to add your sign-off. If the set works for you, can you bulk ack? > >> >> Also, I have fixed most review comments except [1]. I am still not sure of the impact of this change. So, I will publish the v7 without this and then probably a v8 in case this change has any impact. > > I have no idea about this. Hopefully, it's not a very serious thing. > >> That way we can have your patchset not blocked because of this series. > > Thank you! > >> >> [1] http://dpdk.org/ml/archives/dev/2016-July/044004.html > >> >> [...] >> >> >> - >> Shreyansh > > >
diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h index 95789f9..994650b 100644 --- a/lib/librte_eal/common/include/rte_dev.h +++ b/lib/librte_eal/common/include/rte_dev.h @@ -185,8 +185,8 @@ static const char DRIVER_EXPORT_NAME_ARRAY(this_pmd_name, idx) \ __attribute__((used)) = RTE_STR(name) #define PMD_REGISTER_DRIVER(drv, nm)\ -void devinitfn_ ##drv(void);\ -void __attribute__((constructor, used)) devinitfn_ ##drv(void)\ +RTE_INIT(devinitfn_ ##drv);\ +static void devinitfn_ ##drv(void)\ {\ (drv).name = RTE_STR(nm);\ rte_eal_driver_register(&drv);\ diff --git a/lib/librte_eal/common/include/rte_eal.h b/lib/librte_eal/common/include/rte_eal.h index a71d6f5..186f3c6 100644 --- a/lib/librte_eal/common/include/rte_eal.h +++ b/lib/librte_eal/common/include/rte_eal.h @@ -252,6 +252,9 @@ static inline int rte_gettid(void) return RTE_PER_LCORE(_thread_id); } +#define RTE_INIT(func) \ +static void __attribute__((constructor, used)) func(void) + #ifdef __cplusplus } #endif diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h index fa74962..3027adf 100644 --- a/lib/librte_eal/common/include/rte_pci.h +++ b/lib/librte_eal/common/include/rte_pci.h @@ -470,6 +470,14 @@ void rte_eal_pci_dump(FILE *f); */ void rte_eal_pci_register(struct rte_pci_driver *driver); +/** Helper for PCI device registeration from driver (eth, crypto) instance */ +#define DRIVER_REGISTER_PCI(nm, drv) \ +RTE_INIT(pciinitfn_ ##nm); \ +static void pciinitfn_ ##nm(void) \ +{ \ + rte_eal_pci_register(&drv.pci_drv); \ +} + /** * Unregister a PCI driver. * diff --git a/lib/librte_eal/common/include/rte_tailq.h b/lib/librte_eal/common/include/rte_tailq.h index 4a686e6..71ed3bb 100644 --- a/lib/librte_eal/common/include/rte_tailq.h +++ b/lib/librte_eal/common/include/rte_tailq.h @@ -148,8 +148,8 @@ struct rte_tailq_head *rte_eal_tailq_lookup(const char *name); int rte_eal_tailq_register(struct rte_tailq_elem *t); #define EAL_REGISTER_TAILQ(t) \ -void tailqinitfn_ ##t(void); \ -void __attribute__((constructor, used)) tailqinitfn_ ##t(void) \ +RTE_INIT(tailqinitfn_ ##t); \ +static void tailqinitfn_ ##t(void) \ { \ if (rte_eal_tailq_register(&t) < 0) \ rte_panic("Cannot initialize tailq: %s\n", t.name); \