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 7ECCDA04C8; Fri, 18 Sep 2020 14:19:12 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id F0C961D9FD; Fri, 18 Sep 2020 14:19:11 +0200 (CEST) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 9A5821D9F7 for ; Fri, 18 Sep 2020 14:19:10 +0200 (CEST) IronPort-SDR: yGyztaTFYxoh1TOXmh3ITki/DfOSQV40f7qxrT1xvKCcLLMGNlvN0F3Eokv5IyclvcNX6Yt9BN AvOBJwxnTOWg== X-IronPort-AV: E=McAfee;i="6000,8403,9747"; a="221481444" X-IronPort-AV: E=Sophos;i="5.77,274,1596524400"; d="scan'208";a="221481444" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Sep 2020 05:19:09 -0700 IronPort-SDR: 8eyJxq4YB5UtC0UkP4gq0vdGgkz/LbClb50KXvUWexXDtDXSxajoUFXg6KYltM1b++VwYjDP39 /5Ao+YNpc83A== X-IronPort-AV: E=Sophos;i="5.77,274,1596524400"; d="scan'208";a="484182411" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.227.248]) ([10.213.227.248]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Sep 2020 05:19:08 -0700 To: Andrew Rybchenko , Bruce Richardson Cc: david.marchand@redhat.com, dev@dpdk.org References: <20200916164429.244847-1-bruce.richardson@intel.com> <20200918084136.GD1583@bricha3-MOBL.ger.corp.intel.com> <2e1310d7-4e94-40f1-b5eb-ed827645be0b@solarflare.com> From: Ferruh Yigit Message-ID: <1f3efce9-3ee3-9b50-90b0-a377f1618e00@intel.com> Date: Fri, 18 Sep 2020 13:19:04 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.2.2 MIME-Version: 1.0 In-Reply-To: <2e1310d7-4e94-40f1-b5eb-ed827645be0b@solarflare.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [RFC PATCH 0/5] rework feature enabling macros for compatibility 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" On 9/18/2020 9:59 AM, Andrew Rybchenko wrote: > On 9/18/20 11:41 AM, Bruce Richardson wrote: >> On Thu, Sep 17, 2020 at 08:59:26PM +0300, Andrew Rybchenko wrote: >>> On 9/16/20 7:44 PM, Bruce Richardson wrote: >>>> As flagged previously on-list, there are a number of macros used to specify >>>> what libs and drivers are enabled in the build which differ from the >>>> equivalents used with make. This patchset is one possible approach to >>>> fixing these, but as part of the investigation some issues were hit where >>>> I'd like additional input to ensure we are ok with the approach taken in >>>> this set. >>>> >>>> First, a problem statement: >>>> >>>> * While the make build defines generally followed a pattern, there were >>>> many instances where the defines were unique. These can be seen in the >>>> values defined in patch 4. >>>> >>>> * The NIC PMDs had two separate standards for the defines - some (the >>>> physical device drivers) tended to have the _PMD at the end of the >>>> macros, while the virtual drivers had it in the middle. Since the >>>> majority seemed to go with it at the end, meson chose this option. >>>> However, as can be seen from patch 4, a number now need special handling >>>> for compatibility >>>> >>>> * This "_PMD" at the end made its way into other device classes, such as >>>> crypto and event, but it appears that the standard for these classes from >>>> make is in fact the opposite. Therefore we have had for the last 2+ years >>>> conflicting macros for crypto, compression and event classes. >>>> >>>> * There is also the question of how important compatibility for these >>>> macros is, especially since we have had numerous incompatibilities >>>> without it being reported before. There is also the issue of the >>>> deprecation process for macros, since these are not part of any ABI. >>>> >>>> What's done in this set: >>>> >>>> * Firstly, and missing dependencies on apps or examples had to be fixed, >>>> where a dependency was missed because it was never needed due to the code >>>> being stripped out because of a missing macro. >>>> >>>> * Secondly, since it is likely that use of the defines from make is more >>>> widespread than those from meson, the defines for the crypto, compression >>>> and event classes are changed to align with the make values. Just in case >>>> though, temporary code is added to drivers/meson.build to redefine the >>>> old meson values too, and a deprecation notice is added for these. The >>>> hope is that we can then remove this temporary code in the next release, >>>> leaving us with just one macro style for each driver class. >>>> >>>> * Thirdly, we add an additional set of backward compatibility macros for >>>> the ~30 special-cases, where the meson macro template does not match that >>>> defined for make. Again, this is intended to be temporary and a >>>> deprecation notice is given for the macros in that file. >>>> >>>> * Finally, we replace all instances of the old macros in the codebase with >>>> the newer versions. While the work done in the first four patches (steps >>>> 1-3 above) should be ok to backport, this final patch is not suitable for >>>> backporting. However, it is relatively simple to produce a new patch for >>>> backporting which allow either old or new values to be used in macros. >>>> >>>> >>>> Open issues/considerations after this patch: >>>> >>>> * We still have inconsistencies in our driver macro and naming templates. >>>> This is just a nice-to-have, but if people are ok with generally having a >>>> breakage in our macro defines, we could clean this up a lot further. >>>> Notice how 'net', 'regex' and 'vdpa' have '_PMD' at the end, while most >>>> others have it before the name. Notice also that many device classes have >>>> the class at the end of the template, while bbdev has it in the middle. >>>> $ git grep config_flag_fmt -- drivers/*/meson.build >>>> drivers/baseband/meson.build:config_flag_fmt = 'RTE_LIBRTE_PMD_BBDEV_@0@' >>>> drivers/bus/meson.build:config_flag_fmt = 'RTE_LIBRTE_@0@_BUS' >>>> drivers/common/meson.build:config_flag_fmt = 'RTE_LIBRTE_@0@_COMMON' >>>> drivers/compress/meson.build:config_flag_fmt = 'RTE_LIBRTE_PMD_@0@' >>>> drivers/crypto/meson.build:config_flag_fmt = 'RTE_LIBRTE_PMD_@0@' >>>> drivers/event/meson.build:config_flag_fmt = 'RTE_LIBRTE_PMD_@0@_EVENTDEV' >>>> drivers/mempool/meson.build:config_flag_fmt = 'RTE_LIBRTE_@0@_MEMPOOL' >>>> drivers/net/meson.build:config_flag_fmt = 'RTE_LIBRTE_@0@_PMD' >>>> drivers/raw/meson.build:config_flag_fmt = 'RTE_LIBRTE_PMD_@0@_RAWDEV' >>>> drivers/regex/meson.build:config_flag_fmt = 'RTE_LIBRTE_@0@_PMD' >>>> drivers/vdpa/meson.build:config_flag_fmt = 'RTE_LIBRTE_@0@_PMD' >>> >>> As a generic direction I would vote for standard names which are >>> based on directory structure: >>> - RTE_LIBRTE_ETHDEV >>> - RTE_DRIVER_NET_SFC >>> - RTE_DRIVER_COMMON_MLX5 >>> - RTE_DRIVER_BUS_PCI >>> >> >> Definite +1, and it would be good if we standardized the .so names like >> that too. >> >> The open question is how much backward compatibility needs to be maintained >> for macros (and also too for .so names)? With this patchset, I've aimed >> very much to keep strict compatibility for now. > > I think it is really good. Technically it does not look hard to > provide backward compatibility for macros, so I'd keep it up to > but not including 21.11 (next LTS?). Same for .so names, if it > is not a problem. > > If we approve the decision on naming, all new entities must > follow it from the very beginning. > +1