From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id 42B2E47D2 for ; Wed, 16 Apr 2014 15:09:03 +0200 (CEST) Received: from nat-pool-rdu-t.redhat.com ([66.187.233.202] helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.63) (envelope-from ) id 1WaPaD-0008Ig-4E; Wed, 16 Apr 2014 09:09:02 -0400 Date: Wed, 16 Apr 2014 09:08:48 -0400 From: Neil Horman To: Thomas Monjalon Message-ID: <20140416130848.GC11887@localhost.localdomain> References: <1397585169-14537-1-git-send-email-nhorman@tuxdriver.com> <1397585169-14537-4-git-send-email-nhorman@tuxdriver.com> <1462763.GWB5SR3fGh@xps13> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1462763.GWB5SR3fGh@xps13> User-Agent: Mutt/1.5.21 (2010-09-15) X-Spam-Score: -2.9 (--) X-Spam-Status: No Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH 03/15] pmd: Add PMD_REGISTER_DRIVER macro X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 16 Apr 2014 13:09:03 -0000 On Wed, Apr 16, 2014 at 01:52:49PM +0200, Thomas Monjalon wrote: > 2014-04-15 14:05, Neil Horman: > > Rather than have each driver have to remember to add a constructor to it to > > make sure its gets registered properly, wrap that process up in a macro to > > make registration a one line affair. This also sets the stage for us to > > make registration of vdev pmds and physical pmds a uniform process > > > > Signed-off-by: Neil Horman > > Could you explain why having a macro is better than an explicit constructor > function? > Because its a one line declaration inside a driver function that points to the structure used to initilze the pmd? Having to append ((__constructor__)) to each initalization function is both error prone during entry and exposes the possibiilty of developers doing "too much" in their constructor. It also allows for easy updating to all drivers, if additional boilerplate work needs to be done in the future for all pmds. > > +enum rte_pmd_driver_type { > > + PMD_VDEV = 1 > > +}; > > + > > +extern void rte_eal_nonpci_dev_init_register(const char *name, int > > (*dev_initfn)(const char *, const char *)); +#define PMD_REGISTER_DRIVER(d, > > t)\ > > +void devinitfn_ ##d(void);\ > > +void __attribute__((constructor, used)) devinitfn_ ##d(void)\ > > +{\ > > + enum rte_pmd_driver_type _t = (t);\ > > + switch(_t)\ > > + {\ > > + case PMD_VDEV:\ > > + rte_eal_vdev_driver_register(&d);\ > > + break;\ > > + };\ > > Are you sure this switch is needed? > You are removing it in patch 7. > I think you answered your own question :). It was needed when this patch was written because the physical pmd regitration differed from the virtual pmd. As you note though, its removed in patch 7 because we merged the registration methods. Could I merge them earlier? sure, but its not at all needed. It works in this invocation, and it works (better in the final version at the end of the series). > If someone else think this macro is a good idea, or not, speak now :) > I'm speaking up, its a good idea :) > -- > Thomas >