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 EB0F7A053A; Wed, 5 Aug 2020 11:18:47 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 2299A1C02E; Wed, 5 Aug 2020 11:18:47 +0200 (CEST) Received: from qrelay196.mxroute.com (qrelay196.mxroute.com [172.82.139.196]) by dpdk.org (Postfix) with ESMTP id E05561BFFA for ; Wed, 5 Aug 2020 11:18:45 +0200 (CEST) Received: from filter003.mxroute.com ([168.235.111.26] 168-235-111-26.cloud.ramnode.com) (Authenticated sender: mN4UYu2MZsgR) by qrelay196.mxroute.com (ZoneMTA) with ESMTPA id 173bdea70960002bdc.001 for ; Wed, 05 Aug 2020 09:18:44 +0000 X-Zone-Loop: 5c0f93dfa6ed487f4a56dddcb28c093e5ea55a41b9f0 X-Originating-IP: [168.235.111.26] Received: from echo.mxrouting.net (echo.mxrouting.net [116.202.222.109]) by filter003.mxroute.com (Postfix) with ESMTPS id 3576A60007; Wed, 5 Aug 2020 09:18:44 +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=/1TFhpYpTH/FztWEnCJHncgP3oUtGgemRMp+NgRD/o0=; b=PVuT8eYSCm4MI9ZzzTiUU2P4hz pRCfpNDfHNhL0GWufIeHVYKL5irgT+SK0aWCkSwbcEJ9CqSoWqFNtzcjtP5awRpD5iJOMHPIEQACG EUHkCJ5aFBAFoXJFZW4EUkcvmpsL2CBk/IldXRWbNHd1pCrBg1Jq5mOYdGE8t+1hPZjQYkOOlqTit MfI2X953Qf5UfFCjIKBvVzKOLlVSJvLpNsEFWDBg1uKOVOPci/S9Sqaoguh5S6vuZxaR9f/LLmXyx L3UuTPzHqct1jVM2QD7DTQ4hhi5CkIxhuE4mg8pK4P0xuYusyTnhsF0mMomtuQkXhZcVrCB+jJb2V sKtwM2xg==; To: Jerin Jacob , Bruce Richardson Cc: Pavan Nikhilesh , Jerin Jacob , Neil Horman , John McNamara , Marko Kovacevic , dpdk-dev , Thomas Monjalon , David Marchand References: <20200802105137.1666-1-pbhagavatula@marvell.com> <20200803072903.1209-1-pbhagavatula@marvell.com> <20200804104153.GA1464@bricha3-MOBL.ger.corp.intel.com> <20200804142451.GA1704@bricha3-MOBL.ger.corp.intel.com> <20200804162410.GD1704@bricha3-MOBL.ger.corp.intel.com> From: "Kinsella, Ray" Autocrypt: addr=mdr@ashroe.eu; keydata= mQINBFv8B3wBEAC+5ImcgbIvadt3axrTnt7Sxch3FsmWTTomXfB8YiuHT8KL8L/bFRQSL1f6 ASCHu3M89EjYazlY+vJUWLr0BhK5t/YI7bQzrOuYrl9K94vlLwzD19s/zB/g5YGGR5plJr0s JtJsFGEvF9LL3e+FKMRXveQxBB8A51nAHfwG0WSyx53d61DYz7lp4/Y4RagxaJoHp9lakn8j HV2N6rrnF+qt5ukj5SbbKWSzGg5HQF2t0QQ5tzWhCAKTfcPlnP0GymTBfNMGOReWivi3Qqzr S51Xo7hoGujUgNAM41sxpxmhx8xSwcQ5WzmxgAhJ/StNV9cb3HWIoE5StCwQ4uXOLplZNGnS uxNdegvKB95NHZjRVRChg/uMTGpg9PqYbTIFoPXjuk27sxZLRJRrueg4tLbb3HM39CJwSB++ YICcqf2N+GVD48STfcIlpp12/HI+EcDSThzfWFhaHDC0hyirHxJyHXjnZ8bUexI/5zATn/ux TpMbc/vicJxeN+qfaVqPkCbkS71cHKuPluM3jE8aNCIBNQY1/j87k5ELzg3qaesLo2n1krBH bKvFfAmQuUuJT84/IqfdVtrSCTabvDuNBDpYBV0dGbTwaRfE7i+LiJJclUr8lOvHUpJ4Y6a5 0cxEPxm498G12Z3NoY/mP5soItPIPtLR0rA0fage44zSPwp6cQARAQABtBxSYXkgS2luc2Vs bGEgPG1kckBhc2hyb2UuZXU+iQJUBBMBCAA+FiEEcDUDlKDJaDuJlfZfdJdaH/sCCpsFAlv8 B3wCGyMFCQlmAYAFCwkIBwIGFQoJCAsCBBYCAwECHgECF4AACgkQdJdaH/sCCptdtRAAl0oE msa+djBVYLIsax+0f8acidtWg2l9f7kc2hEjp9h9aZCpPchQvhhemtew/nKavik3RSnLTAyn B3C/0GNlmvI1l5PFROOgPZwz4xhJKGN7jOsRrbkJa23a8ly5UXwF3Vqnlny7D3z+7cu1qq/f VRK8qFyWkAb+xgqeZ/hTcbJUWtW+l5Zb+68WGEp8hB7TuJLEWb4+VKgHTpQ4vElYj8H3Z94a 04s2PJMbLIZSgmKDASnyrKY0CzTpPXx5rSJ1q+B1FCsfepHLqt3vKSALa3ld6bJ8fSJtDUJ7 JLiU8dFZrywgDIVme01jPbjJtUScW6jONLvhI8Z2sheR71UoKqGomMHNQpZ03ViVWBEALzEt TcjWgJFn8yAmxqM4nBnZ+hE3LbMo34KCHJD4eg18ojDt3s9VrDLa+V9fNxUHPSib9FD9UX/1 +nGfU/ZABmiTuUDM7WZdXri7HaMpzDRJUKI6b+/uunF8xH/h/MHW16VuMzgI5dkOKKv1LejD dT5mA4R+2zBS+GsM0oa2hUeX9E5WwjaDzXtVDg6kYq8YvEd+m0z3M4e6diFeLS77/sAOgaYL 92UcoKD+Beym/fVuC6/55a0e12ksTmgk5/ZoEdoNQLlVgd2INtvnO+0k5BJcn66ZjKn3GbEC VqFbrnv1GnA58nEInRCTzR1k26h9nmS5Ag0EW/wHfAEQAMth1vHr3fOZkVOPfod3M6DkQir5 xJvUW5EHgYUjYCPIa2qzgIVVuLDqZgSCCinyooG5dUJONVHj3nCbITCpJp4eB3PI84RPfDcC hf/V34N/Gx5mTeoymSZDBmXT8YtvV/uJvn+LvHLO4ZJdvq5ZxmDyxfXFmkm3/lLw0+rrNdK5 pt6OnVlCqEU9tcDBezjUwDtOahyV20XqxtUttN4kQWbDRkhT+HrA9WN9l2HX91yEYC+zmF1S OhBqRoTPLrR6g4sCWgFywqztpvZWhyIicJipnjac7qL/wRS+wrWfsYy6qWLIV80beN7yoa6v ccnuy4pu2uiuhk9/edtlmFE4dNdoRf7843CV9k1yRASTlmPkU59n0TJbw+okTa9fbbQgbIb1 pWsAuicRHyLUIUz4f6kPgdgty2FgTKuPuIzJd1s8s6p2aC1qo+Obm2gnBTduB+/n1Jw+vKpt 07d+CKEKu4CWwvZZ8ktJJLeofi4hMupTYiq+oMzqH+V1k6QgNm0Da489gXllU+3EFC6W1qKj tkvQzg2rYoWeYD1Qn8iXcO4Fpk6wzylclvatBMddVlQ6qrYeTmSbCsk+m2KVrz5vIyja0o5Y yfeN29s9emXnikmNfv/dA5fpi8XCANNnz3zOfA93DOB9DBf0TQ2/OrSPGjB3op7RCfoPBZ7u AjJ9dM7VABEBAAGJAjwEGAEIACYWIQRwNQOUoMloO4mV9l90l1of+wIKmwUCW/wHfAIbDAUJ CWYBgAAKCRB0l1of+wIKm3KlD/9w/LOG5rtgtCUWPl4B3pZvGpNym6XdK8cop9saOnE85zWf u+sKWCrxNgYkYP7aZrYMPwqDvilxhbTsIJl5HhPgpTO1b0i+c0n1Tij3EElj5UCg3q8mEc17 c+5jRrY3oz77g7E3oPftAjaq1ybbXjY4K32o3JHFR6I8wX3m9wJZJe1+Y+UVrrjY65gZFxcA thNVnWKErarVQGjeNgHV4N1uF3pIx3kT1N4GSnxhoz4Bki91kvkbBhUgYfNflGURfZT3wIKK +d50jd7kqRouXUCzTdzmDh7jnYrcEFM4nvyaYu0JjSS5R672d9SK5LVIfWmoUGzqD4AVmUW8 pcv461+PXchuS8+zpltR9zajl72Q3ymlT4BTAQOlCWkD0snBoKNUB5d2EXPNV13nA0qlm4U2 GpROfJMQXjV6fyYRvttKYfM5xYKgRgtP0z5lTAbsjg9WFKq0Fndh7kUlmHjuAIwKIV4Tzo75 QO2zC0/NTaTjmrtiXhP+vkC4pcrOGNsbHuaqvsc/ZZ0siXyYsqbctj/sCd8ka2r94u+c7o4l BGaAm+FtwAfEAkXHu4y5Phuv2IRR+x1wTey1U1RaEPgN8xq0LQ1OitX4t2mQwjdPihZQBCnZ wzOrkbzlJMNrMKJpEgulmxAHmYJKgvZHXZXtLJSejFjR0GdHJcL5rwVOMWB8cg== Message-ID: Date: Wed, 5 Aug 2020 10:18:41 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-AuthUser: mdr@ashroe.eu Subject: Re: [dpdk-dev] [PATCH v2] doc: add reserve fields to eventdev public structures 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 04/08/2020 18:18, Jerin Jacob wrote: > On Tue, Aug 4, 2020 at 9:54 PM Bruce Richardson > wrote: >> >> On Tue, Aug 04, 2020 at 09:33:14PM +0530, Jerin Jacob wrote: >>> On Tue, Aug 4, 2020 at 7:55 PM Bruce Richardson >>> wrote: >>>> >>>> On Tue, Aug 04, 2020 at 05:07:12PM +0530, Jerin Jacob wrote: >>>>> On Tue, Aug 4, 2020 at 4:12 PM Bruce Richardson >>>>> wrote: >>>>>> >>>>>> On Mon, Aug 03, 2020 at 12:59:03PM +0530, pbhagavatula@marvell.com wrote: >>>>>>> From: Pavan Nikhilesh >>>>>>> >>>>>>> Add 64 byte padding at the end of event device public structure to allow >>>>>>> future extensions. >>>>>>> >>>>>>> Signed-off-by: Pavan Nikhilesh >>>>>>> Acked-by: Jerin Jacob >>>>>>> --- >>>>>>> v2 Changes: >>>>>>> - Modify commit title. >>>>>>> - Add patch reference to doc. >>>>>>> >>>>>>> doc/guides/rel_notes/deprecation.rst | 11 +++++++++++ >>>>>>> 1 file changed, 11 insertions(+) >>>>>>> >>>>>>> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst >>>>>>> index ea4cfa7a4..ec5db68e9 100644 >>>>>>> --- a/doc/guides/rel_notes/deprecation.rst >>>>>>> +++ b/doc/guides/rel_notes/deprecation.rst >>>>>>> @@ -151,3 +151,14 @@ Deprecation Notices >>>>>>> Python 2 support will be completely removed in 20.11. >>>>>>> In 20.08, explicit deprecation warnings will be displayed when running >>>>>>> scripts with Python 2. >>>>>>> + >>>>>>> +* eventdev: A 64 byte padding is added at the end of the following structures >>>>>>> + in event device library to support future extensions: >>>>>>> + ``rte_event_crypto_adapter_conf``, ``rte_event_eth_rx_adapter_conf``, >>>>>>> + ``rte_event_eth_rx_adapter_queue_conf``, ``rte_event_eth_tx_adapter_conf``, >>>>>>> + ``rte_event_timer_adapter_conf``, ``rte_event_timer_adapter_info``, >>>>>>> + ``rte_event_dev_info``, ``rte_event_dev_config``, ``rte_event_queue_conf``, >>>>>>> + ``rte_event_port_conf``, ``rte_event_timer_adapter``, >>>>>>> + ``rte_event_timer_adapter_data``. >>>>>>> + Reference: >>>>>>> + http://patches.dpdk.org/project/dpdk/list/?series=10728&archive=both&state=* >>>>>>> -- >>>>>> >>>>>> I don't like this idea of adding lots of padding to the ends of these >>>>>> structures. For some structures, such as the public arrays for devices it >>>>>> may be necessary, but for all the conf structures passed as parameters to >>>>>> functions I think we can do better. Since these structures are passed by >>>>>> the user to various functions, function versioning can be used to ensure >>>>>> that the correct function in eventdev is always called. From there to the >>>>>> individual PMDs, we can implement ABI compatibility by either: >>>>>> 1. including the length of the struct as a parameter to the driver. (This is >>>>>> a bit similar to my proposal for rawdev [1]) >>>>>> 2. including the ABI version as a parameter to the driver. >>>>> >>>>> But, Will the above solution work if the application is dependent on >>>>> struct size? >>>>> i.e change of s1 size will change offset of s3 i.e >>>>> app_sepecific_struct_s3. Right? >>>>> i.e DPDK version should not change the offset of s3. Right? >>>>> >>>>> example, >>>>> struct app_struct { >>>>> struct dpdk_public_struct_s1 s1; >>>>> struct dpdk_public_struct_s2 s2; >>>>> struct app_sepecific_struct_s3 s3; >>>>> } >>>>> >>>> Not sure what exactly you mean here. The actual offsets and sizes of the >>>> structs will obviously change as you change the struct, but the end >>>> compiled app has no idea of structs, all it knows of is offsets, which is >>>> why you provide ABI compatible versions of the functions which use "legacy" >>>> copies of the structs to ensure correct offsets. It's pretty much standard >>>> practice for ABI versioning. >>> >>> Currently, We have only function versioning(not structure versioning). >>> Are you suggesting having structure versioning? >>> Will it complicate the code in terms of readability and supporting multiple >>> structure versions aginst its support functions. >>> >> >> We don't, and can't version structures, only functions are versioned. >> Even if we do what you suggest and add a block of 64-bytes expansion room >> at the end of the structures, how is the function you are calling expected >> to know what the structure actually contains? For example, if you add a >> field to the end, and reduce the padding by 8 bytes, your structure is >> still the same size, and how does the called function know whether X or X+8 >> bytes are valid in it. Basically, you still need to version all >> functions using the structure, just as if you didn't bother extending the >> struct. > > Yes. We need function versioning for sure if we change the behavior of > the function. > Is function version + reserved field enough to decode the correct value from > the reserved filed from the structure. > > My concern is, Let say, we are making the change in structure a->b and > function c->d > assisted with it. > > In the reserved filed case: > - struct a remains same(we will adding the fields in reserved filed) > - the function will have c and d version and both using struct a > > In another scheme: > - The application needs to change where versioned function(c or d) need to > give associate structure manually. Right? If it is manually, it will > be huge change > in application. Right? > > How an application can express the correct structure mapping? > Will it it be like > rte_substrem_v21(struct rte_subsystem_conf_v21 *config)? > vs > rte_substrem_v21(struct rte_subsystem_conf *config)?> where rte_subsystem_conf has reserved filed. So the ABI policy approach for doing this is rte_substrem_v21(struct rte_subsystem_conf_v21 *config) instead of rte_substrem_v21(struct rte_subsystem_conf *config) (with extension padding). There are benefits and drawbacks with each approach, these include ... The padding approach assumes you are always happy to tack whatever field you want onto the end of the structure, when in many cases it's more natural home is usually in the middle or beginning. Then there is also dead/unused and uninitialized memory, to be conscious of. However what you say is completely correct, if I have a v21 version of the function, only it should be looking at the new field/additional bytes in the structure. The alternative is to version the structure along with the function. And this is what is described in the ABI policy. The obvious drawback with this approach is that if you have a structure that is used across a large number of functions ... it can be a real headache as they all need to be versioned. The benefit with this approach is that it is completely explicit, which function and structure versions are associated. > >> >>>> >>>> The real complication arises because the actual eventdev driver functions >>>> are not called directly with the linker resolving symbol versioning. >>>> Instead they are called using function pointers from the code. This is why >>>> one needs to add in the additional parameter to the driver APIs so that the >>>> ABI info - be it struct size or version - can be passed from the versioned >>>> eventdev library function through to the driver. >>> >>> If I understand it correctly, it is easy for rawdev subsystem as >>> config structure as _opaque_. >>> But in the case for ethdev, cryptodev, eventdev, config structure are >>> not opaque. >>> >> >> Well, actually it's more complicated for rawdev because the structures are >> opaque, so the value passed could be anything. With eventdev, they will >> ever only be one or two possible values, so you just need enough info to >> distinguish if it's the structure from version X, or the structure from >> version Y you are passing. >> >>> If we take structure versioning, Is n't call for adding structure >>> version change to >>> functions that's been associated with(Kind of M x N case to make a >>> structure change. Here M means >>> offending structure to make change and N means function associated >>> with structure) >>> >> Yes, to version any structure you basically do a new version of every >> function using it, or using the changed fields. However, adding padding is >> not going to remove that need. > > See above. > >> >> Regards, >> /Bruce