From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id EAAC7A052A; Mon, 25 Jan 2021 11:46:32 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 6B631140EC9; Mon, 25 Jan 2021 11:46:32 +0100 (CET) Received: from dal2relay136.mxroute.com (dal2relay136.mxroute.com [64.40.26.136]) by mails.dpdk.org (Postfix) with ESMTP id 81E38140EC8 for ; Mon, 25 Jan 2021 11:46:30 +0100 (CET) Received: from filter004.mxroute.com ([149.28.56.236] 149.28.56.236.vultr.com) (Authenticated sender: mN4UYu2MZsgR) by dal2relay136.mxroute.com (ZoneMTA) with ESMTPSA id 1773926e4ed000d98d.001 for (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES128-GCM-SHA256); Mon, 25 Jan 2021 10:46:26 +0000 X-Zone-Loop: 3dffa7f5bb6c38f2e90f39776ecdbed8bb81549f1418 X-Originating-IP: [149.28.56.236] Received: from echo.mxrouting.net (echo.mxrouting.net [116.202.222.109]) by filter004.mxroute.com (Postfix) with ESMTPS id B71773EC7A; Mon, 25 Jan 2021 10:46:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=ashroe.eu; s=x; h=Content-Transfer-Encoding:Content-Type:In-Reply-To:MIME-Version:Date: Message-ID:From:References:Cc:To:Subject:Sender:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=R3llULyyCibMQ6FjPC1YcPsuPbZOPMzA8iNi6WAmsps=; b=pmvehOFXLmXlO79Nlr9/9EMAFs bU2IgSV+56HZmbSOE1xN1YxCx211QfnAJVjbvaAcdMOdLDhpa5oy6ALXbsPF7g99U/D28OYJTGTbo tS8VxN50KFW2wrDlLCqjVWuHHPxOZ8FVXyfp+HV3HAZCLR4vhzouAsU8KONRKfwOvs3OPTT1goUkv HvZCMsYs5DmmtbLiZhTfLWQ1agB3G4nYflEbU4FYBW7osRXyXhBp0cBa9tTVVBnoT0IgKRYisaScv F3rINcJ61WkDKna5Sxv/s2a1F0QW8+0gR2MR0+02ESCLHZrIum1/dQuVWTdHtmoTK5MRNgd+BLAqx svyv0p3Q==; To: David Marchand Cc: dev , Thomas Monjalon , Dmitry Kozlyuk References: <20201014183136.22239-1-dmitry.kozliuk@gmail.com> <5444857.Q0qPc8oPp2@thomas> <20210123012403.1bdb35d0@sovereign> <14261305.cmbqrkYmuW@thomas> From: "Kinsella, Ray" Message-ID: <3e6d199a-57ee-8e6a-0176-d04c1fc870be@ashroe.eu> Date: Mon, 25 Jan 2021 10:46:22 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.6.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-AuthUser: mdr@ashroe.eu Subject: Re: [dpdk-dev] [PATCH v8 2/3] build: use Python pmdinfogen X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 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 25/01/2021 10:29, David Marchand wrote: > On Mon, Jan 25, 2021 at 11:01 AM Kinsella, Ray wrote: >> >> >> >> On 25/01/2021 09:25, Kinsella, Ray wrote: >>> >>> >>> On 23/01/2021 11:38, Thomas Monjalon wrote: >>>> 22/01/2021 23:24, Dmitry Kozlyuk: >>>>> On Fri, 22 Jan 2021 21:57:15 +0100, Thomas Monjalon wrote: >>>>>> 22/01/2021 21:31, Dmitry Kozlyuk: >>>>>>> On Wed, 20 Jan 2021 11:24:21 +0100, Thomas Monjalon wrote: >>>>>>>> 20/01/2021 08:23, Dmitry Kozlyuk: >>>>>>>>> On Wed, 20 Jan 2021 01:05:59 +0100, Thomas Monjalon wrote: >>>>>>>>>> This is now the right timeframe to introduce this change >>>>>>>>>> with the new Python module dependency. >>>>>>>>>> Unfortunately, the ABI check is returning an issue: >>>>>>>>>> >>>>>>>>>> 'const char mlx5_common_pci_pmd_info[62]' was changed >>>>>>>>>> to 'const char mlx5_common_pci_pmd_info[60]' at rte_common_mlx5.pmd.c >>>>>>>>> >>>>>>>>> Will investigate and fix ASAP. >>>>>>> >>>>>>> Now that I think of it: strings like this change every time new PCI IDs are >>>>>>> added to a PMD, but AFAIK adding PCI IDs is not considered an ABI breakage, >>>>>>> is it? One example is 28c9a7d7b48e ("net/mlx5: add ConnectX-6 Lx device ID") >>>>>>> added 2020-07-08, i.e. clearly outside of ABI change window. >>>>>> >>>>>> You're right. >>>>>> >>>>>>> "xxx_pmd_info" changes are due to JSON formatting (new is more canonical), >>>>>>> which can be worked around easily, if the above is wrong. >>>>>> >>>>>> If the new format is better, please keep it. >>>>>> What we need is an exception for the pmdinfo symbols >>>>>> in the file devtools/libabigail.abignore. >>>>>> You can probably use a regex for these symbols. >>>>> >>>>> This would allow real breakages to pass ABI check, abidiff doesn't analyze >>>>> variable content and it's not easy to compare. Maybe later a script can be >>>>> added that checks lines with RTE_DEVICE_IN in patches. There are at most 32 of >>>>> 5494 relevant commits between 19.11 and 20.11, though. >>>>> >>>>> To verify there are no meaningful changes I ensured empty diff between >>>>> results of the following command for "main" and the branch: >>>>> >>>>> find build/drivers -name '*.so' -exec usertools/dpdk-pmdinfo.py >>>> >>>> For now we cannot do such check as part of the ABI checker. >>>> And we cannot merge this patch if the ABI check fails. >>>> I think the only solution is to allow any change in the pmdinfo variables. >>>> >>> >>> So my 2c on this is that this is an acceptable work-around for the v21 (DPDK v20.11) ABI. >>> However we are going to end up carrying this rule in libabigail.ignore indefinitely. >>> >>> Would it make sense to just fix the size of _pmd_info to some reasonably large value - >>> say 128 bytes, to allow us to drop the rule in the DPDK 21.11 v22 release? >>> >>> Ray K >> >> >> Another point is - shouldn't _pmd_info probably live in "INTERNAL" is anycase? > > The symbol itself can be hidden from the ABeyes. > It is only a placeholder for the PMD_INFO_STRING= string used by > usertools/dpdk-pmdinfo.py and maybe some other parsing tool. > > I guess a static symbol would be enough: > > diff --git a/buildtools/pmdinfogen/pmdinfogen.c > b/buildtools/pmdinfogen/pmdinfogen.c > index a68d1ea999..14bf7d9f42 100644 > --- a/buildtools/pmdinfogen/pmdinfogen.c > +++ b/buildtools/pmdinfogen/pmdinfogen.c > @@ -393,7 +393,7 @@ static void output_pmd_info_string(struct elf_info > *info, char *outfile) > drv = info->drivers; > > while (drv) { > - fprintf(ofd, "const char %s_pmd_info[] __attribute__((used)) = " > + fprintf(ofd, "static const char %s_pmd_info[] > __attribute__((used)) = " > "\"PMD_INFO_STRING= {", > drv->name); > fprintf(ofd, "\\\"name\\\" : \\\"%s\\\", ", drv->name); > > > We will need an exception for the v21 ABI though. > Good suggestion +1