DPDK patches and discussions
 help / color / mirror / Atom feed
From: Akhil Goyal <akhil.goyal@nxp.com>
To: "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>,
	"Honnappa.Nagarahalli@arm.com" <Honnappa.Nagarahalli@arm.com>,
	"techboard@dpdk.org" <techboard@dpdk.org>
Cc: "Vangati, Narender" <narender.vangati@intel.com>,
	"jerinj@marvell.com" <jerinj@marvell.com>
Subject: Re: [dpdk-dev] [v4 1/3] cryptodev: support enqueue callback functions
Date: Wed, 28 Oct 2020 14:28:43 +0000
Message-ID: <VI1PR04MB316857318EA0B2299D055488E6170@VI1PR04MB3168.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <BYAPR11MB330121301B54DBA67D329FE29A170@BYAPR11MB3301.namprd11.prod.outlook.com>


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?

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.

> > > IMO, it seems an ABI breakage, but not sure. So wanted to double check.
> > > Now if it is an ABI breakage, then can we allow it? There was no deprecation
> > > notice Prior to this release.
> 
> Yes, there was no deprecation note in advance.
> Though I think the risk is minimal - size of the struct will remain unchanged (see
> above).
> My vote to let it in for 20.11.
> 
> > > Also I think if we are allowing the above change, then we should also add
> > > another Field for deq_cbs also for post crypto processing in this patch only.
> 
> +1 for this.
> I think it was already addressed in v5.
> 
> Konstantin


  reply	other threads:[~2020-10-28 14:28 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 [this message]
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
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=VI1PR04MB316857318EA0B2299D055488E6170@VI1PR04MB3168.eurprd04.prod.outlook.com \
    --to=akhil.goyal@nxp.com \
    --cc=Honnappa.Nagarahalli@arm.com \
    --cc=abhinandan.gujjar@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=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

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git