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 99EB9A0A0A; Fri, 22 Jan 2021 13:12:09 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5259E140F4B; Fri, 22 Jan 2021 13:12:09 +0100 (CET) Received: from dal1relay167.mxroute.com (dal1relay167.mxroute.com [199.181.239.167]) by mails.dpdk.org (Postfix) with ESMTP id 0BD9C140F3F for ; Fri, 22 Jan 2021 13:12:07 +0100 (CET) Received: from filter004.mxroute.com ([149.28.56.236] 149.28.56.236.vultr.com) (Authenticated sender: mN4UYu2MZsgR) by dal1relay167.mxroute.com (ZoneMTA) with ESMTPSA id 1772a02327f000d98d.001 for (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES128-GCM-SHA256); Fri, 22 Jan 2021 12:12:03 +0000 X-Zone-Loop: a3e82c452dbdb73362a3fe7968a9022e10ed3ac0cc90 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 AC6D43EADC; Fri, 22 Jan 2021 12:12:01 +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=KO73KiJ7O8XeXIgihDcCRfBXe3RJaNDlvrUwYES1BvE=; b=YaJBrlPfNfOleObIWQCZGrIAix 7EJSWRUCjReor9a7weup9ig7CAOFSFntDKhEm61f/G+aQkYNkwJEfoKtlhSRUtISctnqCOG4T0Ej/ lDrbV6zcyp3grR5CyHS/8sWehK2pRNlZqLoxcXVJZIZtJiPyXMcefElEZ6V7mxcY0G260GVXIb83X VdYrPgl7OAy+dNO9kweEGl82/3cWAKty7hqNNuIapCpWwNWG6tr9PWHSuCUdiZWwvMOzCEJ8keGAL cFphIvNIVn1XQCuH+J9UBk228bhVbc6X8rb7Q8ZvzcCSIM8+J9QbMg+8y0xm6TQ88BaAFdrnaPbFJ N3gZJEJw==; To: Thomas Monjalon , Dodji Seketeli Cc: Neil Horman , Akhil Goyal , Konstantin Ananyev , Abhinandan Gujjar , dev@dpdk.org, david.marchand@redhat.com References: <20210120142558.4120552-1-mdr@ashroe.eu> <15493004.lGub55pDpk@thomas> <86sg6uiamx.fsf@redhat.com> <2579086.B9NNOlTe4A@thomas> From: "Kinsella, Ray" Message-ID: Date: Fri, 22 Jan 2021 12:11:59 +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: <2579086.B9NNOlTe4A@thomas> 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 v1] devtools: update abi ignore for cryptodev 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 21/01/2021 15:58, Thomas Monjalon wrote: > 21/01/2021 16:15, Dodji Seketeli: >> Hello Thomas and others, >> >> Thomas Monjalon writes: >> >>> Question to an expert, Dodji, >> >> Thanks for the kind words, but I am not an expert in anything, sadly. I >> am just trying to keep learning about these things ;-) >> >>> We have this structure: >>> >>> struct rte_cryptodev { >>> lot of fields... >>> uint8_t attached : 1; >>> } __rte_cache_aligned; >>> >>> Because of the cache alignment, there is enough padding in the struct >>> (no matter the size of the cache line) for adding two more pointers: >>> >>> struct rte_cryptodev { >>> lot of fields... >>> uint8_t attached : 1; >>> struct rte_cryptodev_cb_rcu *enq_cbs; >>> struct rte_cryptodev_cb_rcu *deq_cbs; >>> } __rte_cache_aligned; >>> >>> We checked manually that the ABI is still compatible. >> >> Right. >> >> I am curious, but normally, libabigail should raise the addition of >> structures, but then it'll tell you that there was no size or offset >> change between the two structures. If it doesn't, then that's a bug. I >> hope it does :-) > > Yes it was raising a problem, that's why we are adding a rule. > > >>> Then I've added (quickly) a libabigail exception rule: >>> >>> [suppress_type] >>> name = rte_cryptodev >>> has_data_member_inserted_between = {0, 1023} >>> >>> Now we want to improve this rule to restrict the offsets >>> to the padding at the end of the struct only, >>> so we keep forbidding changes in existing fields, >>> and forbidding additions further the current struct size. >>> Is this new rule good? >>> >>> has_data_member_inserted_between = {offset_after(attached), end} >> >> >> Yes, this rule should do what you think it says. >> >>> Do you confirm that the keyword "end" means the old reference size? >> >> Yes I do. >> >> >>> What else do we need to check for adding a new field in a padding? >> >> Actually, that rule will work independantly of it there is enough >> padding or not. It'll shut down the change report, even if the added >> data exceeds the padding. > > I don't understand why. > If "end" means the old reference size, then addition after the old size > should be reported, isn't it? yes - this comment confuses me also. If "end" refers to the size original data-structure (position of the end), which in this case had some padding. If the additions fall fully within the padding I would expect this rule to work - as long as the data-structure size is still the same. However if the additions fall beyond the size of the original data-structure, the data-structure's size will have changed, I would not expect this rule to condone a change in the size of the data-structure. > > >> You just made me think of an idea of a new feature there. >> >> Maybe we'd need a new property for the [suppress_type] directive that >> would suppress changes only if said changes don't modify the size of the >> type or any offset of any member of the type? >> >> Maybe something like: >> >> [suppress_type] >> ; lots of properties can go here. >> >> ; ... >> >> ; If the type has any size or offset change >> ; then this suppression directive will fail >> ; and the change report will be emitted >> has_no_size_or_offset_change >> >> Would that be useful to you in this case, >> >> Cheers, > > >