DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Kinsella, Ray" <mdr@ashroe.eu>
To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>,
	Bruce Richardson <bruce.richardson@intel.com>,
	"Akhil.goyal@nxp.com" <akhil.goyal@nxp.com>
Cc: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
	"Gujjar, Abhinandan S" <abhinandan.gujjar@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	"Doherty, Declan" <declan.doherty@intel.com>,
	"techboard@dpdk.org" <techboard@dpdk.org>,
	"Vangati, Narender" <narender.vangati@intel.com>,
	"jerinj@marvell.com" <jerinj@marvell.com>, nd <nd@arm.com>
Subject: Re: [dpdk-dev] [dpdk-techboard] [v4 1/3] cryptodev: support enqueue callback functions
Date: Thu, 29 Oct 2020 14:26:44 +0000	[thread overview]
Message-ID: <b622fd98-0422-7287-3e52-badfab69a7ce@ashroe.eu> (raw)
In-Reply-To: <DBAPR08MB58147711C02EB901451D2AE598170@DBAPR08MB5814.eurprd08.prod.outlook.com>



On 28/10/2020 15:22, Honnappa Nagarahalli wrote:
> + Ray for ABI
> 
> <snip>
> 
>>
>> On Wed, Oct 28, 2020 at 02:28:43PM +0000, Akhil Goyal wrote:
>>>
>>> Hi Konstantin,
>>>
>>>>>> Hi Tech board members,
>>>>>>
>>>>>> I have a doubt about the ABI breakage in below addition of field.
>>>>>> Could you please comment.
>>>>>>
>>>>>>>  /** The data structure associated with each crypto device. */
>>>>>>> struct rte_cryptodev {
>>>>>>>  	dequeue_pkt_burst_t dequeue_burst; @@ -867,6 +922,10
>> @@
>>>>>>> struct rte_cryptodev {
>>>>>>>  	__extension__
>>>>>>>  	uint8_t attached : 1;
>>>>>>>  	/**< Flag indicating the device is attached */
>>>>>>> +
>>>>>>> +	struct rte_cryptodev_enq_cb_rcu *enq_cbs;
>>>>>>> +	/**< User application callback for pre enqueue processing */
>>>>>>> +
>>>>>>>  } __rte_cache_aligned;
>>>>>>
>>>>>> Here rte_cryptodevs is defined in stable API list in map file
>>>>>> which is a pointer To all rte_cryptodev and the above change is
>>>>>> changing the size of the
>>>> structure.
>>>>
>>>> While this patch adds new fields into rte_cryptodev structure, it
>>>> doesn't change the size of it.
>>>> struct rte_cryptodev is cache line aligned, so it's current size:
>>>> 128B for 64-bit systems, and 64B(/128B) for 32-bit systems.
>>>> So for 64-bit we have 47B implicitly reserved, and for 32-bit we
>>>> have 19B reserved.
>>>> That's enough to add two pointers without changing size of this struct.
>>>>
>>>
>>> The structure is cache aligned, and if the cache line size in 32Byte
>>> and the compilation is done on 64bit machine, then we will be left
>>> with 15Bytes which is not sufficient for 2 pointers.
>>> Do we have such systems? Am I missing something?
>>>
>>
>> I don't think we support any such systems, so unless someone can point out
>> a specific case where we need to support 32-byte CLs, I'd tend towards
>> ignoring this as a non-issue.
> Agree. I have not come across 32B cache line.
> 
>>
>>> The reason I brought this into techboard is to have a consensus on
>>> such change As rte_cryptodev is a very popular and stable structure.
>>> Any changes to it may Have impacts which one person cannot judge all use
>> cases.
>>>
>>
>> Haven't been tracking this discussion much, but from what I read here, this
>> doesn't look like an ABI break and should be ok.
> If we are filling the holes in the cache line with new fields, it should not be an ABI break.

Agreed, risk seems minimal  ... it is an ABI Breakage window in anycase.
 
>>
>> Regards,
>> /Bruce

  parent reply	other threads:[~2020-10-29 14:26 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-25  9:44 [dpdk-dev] [v4 0/3] support enqueue callbacks on cryptodev Abhinandan Gujjar
2020-10-25  9:44 ` [dpdk-dev] [v4 1/3] cryptodev: support enqueue callback functions Abhinandan Gujjar
2020-10-27 12:47   ` Ananyev, Konstantin
2020-10-27 17:16     ` Gujjar, Abhinandan S
2020-10-27 17:20       ` Ananyev, Konstantin
2020-10-27 17:22         ` Gujjar, Abhinandan S
2020-10-27 18:19   ` Akhil Goyal
2020-10-27 19:16     ` Gujjar, Abhinandan S
2020-10-27 19:26       ` Akhil Goyal
2020-10-27 19:41         ` Gujjar, Abhinandan S
2020-10-27 18:28   ` Akhil Goyal
2020-10-28  8:20     ` Gujjar, Abhinandan S
2020-10-28 12:55       ` Ananyev, Konstantin
2020-10-28 14:28         ` Akhil Goyal
2020-10-28 14:52           ` Ananyev, Konstantin
2020-10-28 15:11           ` [dpdk-dev] [dpdk-techboard] " Bruce Richardson
2020-10-28 15:22             ` Honnappa Nagarahalli
2020-10-29 13:52               ` Gujjar, Abhinandan S
2020-10-29 14:00                 ` Akhil Goyal
2020-10-30  4:24                   ` Gujjar, Abhinandan S
2020-10-30 17:18                     ` Gujjar, Abhinandan S
2020-10-29 14:26               ` Kinsella, Ray [this message]
2020-10-25  9:44 ` [dpdk-dev] [v4 2/3] test: add testcase for crypto enqueue callback Abhinandan Gujjar
2020-10-25  9:44 ` [dpdk-dev] [v4 3/3] doc: add enqueue callback APIs Abhinandan Gujjar
2020-10-26 19:08   ` Akhil Goyal
2020-10-27  3:52     ` Gujjar, Abhinandan S
2020-10-27 12:51   ` Ananyev, Konstantin
2020-10-27 17:17     ` Gujjar, Abhinandan S

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b622fd98-0422-7287-3e52-badfab69a7ce@ashroe.eu \
    --to=mdr@ashroe.eu \
    --cc=Honnappa.Nagarahalli@arm.com \
    --cc=abhinandan.gujjar@intel.com \
    --cc=akhil.goyal@nxp.com \
    --cc=bruce.richardson@intel.com \
    --cc=declan.doherty@intel.com \
    --cc=dev@dpdk.org \
    --cc=jerinj@marvell.com \
    --cc=konstantin.ananyev@intel.com \
    --cc=narender.vangati@intel.com \
    --cc=nd@arm.com \
    --cc=techboard@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).