DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Kusztal, ArkadiuszX" <arkadiuszx.kusztal@intel.com>
To: Thomas Monjalon <thomas@monjalon.net>,
	"Trahe, Fiona" <fiona.trahe@intel.com>
Cc: David Marchand <david.marchand@redhat.com>,
	"nhorman@tuxdriver.com" <nhorman@tuxdriver.com>,
	"bluca@debian.org" <bluca@debian.org>,
	"ktraynor@redhat.com" <ktraynor@redhat.com>,
	Ray Kinsella <mdr@ashroe.eu>, "dev@dpdk.org" <dev@dpdk.org>,
	Akhil Goyal <akhil.goyal@nxp.com>,
	"Yigit, Ferruh" <ferruh.yigit@intel.com>,
	"Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>, Anoob Joseph <anoobj@marvell.com>,
	"Richardson, Bruce" <bruce.richardson@intel.com>,
	"Mcnamara, John" <john.mcnamara@intel.com>,
	"dodji@seketeli.net" <dodji@seketeli.net>,
	Andrew Rybchenko <arybchenko@solarflare.com>,
	"aconole@redhat.com" <aconole@redhat.com>,
	"Trahe, Fiona" <fiona.trahe@intel.com>
Subject: Re: [dpdk-dev] [PATCH v2 4/4] add ABI checks
Date: Tue, 17 Mar 2020 19:10:40 +0000	[thread overview]
Message-ID: <BL0PR11MB331654A4EEF98E474C756F729FF60@BL0PR11MB3316.namprd11.prod.outlook.com> (raw)
In-Reply-To: <56588879.matp6XCIr4@xps>



> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Tuesday, March 17, 2020 4:10 PM
> To: Trahe, Fiona <fiona.trahe@intel.com>; Kusztal, ArkadiuszX
> <arkadiuszx.kusztal@intel.com>
> Cc: David Marchand <david.marchand@redhat.com>;
> nhorman@tuxdriver.com; bluca@debian.org; ktraynor@redhat.com; Ray
> Kinsella <mdr@ashroe.eu>; dev@dpdk.org; Akhil Goyal
> <akhil.goyal@nxp.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Ananyev,
> Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org; Anoob Joseph
> <anoobj@marvell.com>; Richardson, Bruce <bruce.richardson@intel.com>;
> Mcnamara, John <john.mcnamara@intel.com>; dodji@seketeli.net; Andrew
> Rybchenko <arybchenko@solarflare.com>; aconole@redhat.com; Trahe,
> Fiona <fiona.trahe@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v2 4/4] add ABI checks
> 
> 17/03/2020 14:27, Kusztal, ArkadiuszX:
> > Hi Thomas,
> >
> > > -----Original Message-----
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > Sent: Monday, March 16, 2020 2:09 PM
> > > To: Trahe, Fiona <fiona.trahe@intel.com>
> > > Cc: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; David
> > > Marchand <david.marchand@redhat.com>; nhorman@tuxdriver.com;
> > > bluca@debian.org; ktraynor@redhat.com; Ray Kinsella
> <mdr@ashroe.eu>;
> > > dev@dpdk.org; Akhil Goyal <akhil.goyal@nxp.com>; Yigit, Ferruh
> > > <ferruh.yigit@intel.com>; Ananyev, Konstantin
> > > <konstantin.ananyev@intel.com>; dev@dpdk.org; Anoob Joseph
> > > <anoobj@marvell.com>; Richardson, Bruce
> > > <bruce.richardson@intel.com>; Mcnamara, John
> > > <john.mcnamara@intel.com>; dodji@seketeli.net; Andrew Rybchenko
> > > <arybchenko@solarflare.com>; aconole@redhat.com; Trahe, Fiona
> > > <fiona.trahe@intel.com>
> > > Subject: Re: [dpdk-dev] [PATCH v2 4/4] add ABI checks
> > >
> > > 16/03/2020 13:57, Trahe, Fiona:
> > > > From: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
> > > > > > > > The patch we're working on will provide two versions of
> > > > > > > > rte_cryptodev_info_get(), both call the same PMD function
> > > > > > > > from the
> > > > > > dev_ops info_get fn ptr.
> > > > > > > > The default version operates s as normal, the 19.11
> > > > > > > > version searches through the list returned by the PMD,
> > > > > > > > looking for sym.aead.algo = ChaChaPoly, it needs to strip
> > > > > > > > it from
> > > > > > > the list.
> > > > > > > > As PMDs just pass a ptr to their capabilities list ( it
> > > > > > > > isn't a linked list, but an array with an end marker  =
> > > > > > > > RTE_CRYPTODEV_END_OF_CAPABILITIES_LIST) if the API layer
> > > > > > > > detects Chacha it must allocate some space and store a
> > > > > > > > local copy of the trimmed
> > > > > > list. This must be stored only once per device.
> > > > >
> > > > > [Arek] The problem with this solution is that we need to allocate
> memory.
> > > > > So the question is how to handle unlikely case of malloc error
> > > > > when we operate inside void function rte_cryptodev_info_get?
> > > > > And even if we would pass somehow error condition to the caller
> > > > > then
> > > what to do is another question.
> > > >
> > > > [Fiona] Quick recap: To avoid breaking ABI, we must return a set
> > > > of capabilities with/without ChaChaPoly depending on the appl
> > > > version. To resolve this, within the rte_cryptodev layer, we
> > > > propose to inspect the
> > > capabilities returned by PMD and strip ChaCha if it exists.
> > > > In that case memory for the new trimmed capabilities array has to
> > > > be
> > > malloced by the lib.
> > >
> > > What happens if the capability is removed from the original
> > > capabilities input?
> > >
> > > > All good, except how to handle a malloc fail is yet another API
> > > > breakage as
> > > rte_cryptodev_get_info() returns void.
> > > > We propose to return an empty capability list, i.e. a list with
> > > > only the END element (which can be done without malloc) in this
> > > > corner case of
> > > a corner case.
> > > > Anyone see any issue with this?
> > >
> > > How can we use the feature if it is not advertised in capabilities?
> > What Fiona meant is that empty capability would indicate error condition in
> this case. That's why she asked if you ok with this API breakage.
> 
> Sorry I'm lost.
> Please could you show what would be the usage?
> 
So this case looks more or less like that:
There are two versions of `rte_cryptodev_info_get`

rte_cryptodev_info_get_v20 (versioned)
rte_cryptodev_info_get_v2005 (new default symbol)

Default version works normally as it will be called only by app build with 20.05 version.

When prior to 20.05 app calls `rte_cryptodev_info_get` version v20 is called. This function will remove Chacha Poly from capabilities, but to achieve this we need to get some memory to store `new` set of capabilities per device (without Chacha). So:
new_capability[dev_id] = malloc( (num_of_capabilies - 1 (chacha)) * sizeof(struct rte_cryptodev_capabilities))
The small problem is how to handle malloc error:
If (new_capability[dev_id] == NULL) {
	/* What to do now as rte_cryptodev_info_get is void function, and API does not say anything about error condition */
/*So Fiona suggestion above is to inform user of an error by doing this: */
	dev_info->capabilities = cryptodev_undefined_capabilities;

/* Where 
static const struct rte_cryptodev_capabilities cryptodev_undefined_capabilities[] = {
		RTE_CRYPTODEV_END_OF_CAPABILITIES_LIST()

}; */
}
Sizeof rte_cryptodev_capabilities is 38 bytes, padded to 40. So properly constructed capabilities can take at most 1920 bytes.  No system should ever fail doing that though iam not an expert. Other option could probably be to preallocate this memory. This is how I understand that.

Another question is can something like that be done if API comments of `rte_cryptodev_info_get` function does not say anything about any potential error?

Regards,
Arek







  reply	other threads:[~2020-03-17 19:10 UTC|newest]

Thread overview: 104+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-20 15:20 [dpdk-dev] [PATCH] " David Marchand
2019-12-20 15:32 ` Richardson, Bruce
2019-12-20 16:20   ` Kinsella, Ray
2019-12-20 21:00     ` Thomas Monjalon
2020-01-06 13:17       ` Aaron Conole
2020-01-15 13:07         ` Burakov, Anatoly
2020-01-14 23:19     ` Thomas Monjalon
2020-01-15 11:33       ` Neil Horman
2020-01-15 12:38         ` Thomas Monjalon
2020-01-16 11:52           ` Neil Horman
2020-01-16 14:20             ` Thomas Monjalon
2020-01-16 18:49               ` Neil Horman
2020-01-16 20:01                 ` Thomas Monjalon
2020-01-17 19:01                   ` Neil Horman
2020-01-17 21:26                     ` David Marchand
2019-12-20 20:25 ` Neil Horman
2020-01-29 17:26 ` [dpdk-dev] [PATCH v2 0/4] " David Marchand
2020-01-29 17:26   ` [dpdk-dev] [PATCH v2 1/4] hash: fix meson headers packaging David Marchand
2020-01-30 10:12     ` Luca Boccassi
2020-01-30 10:54       ` David Marchand
2020-01-30 10:56         ` Luca Boccassi
2020-01-29 17:26   ` [dpdk-dev] [PATCH v2 2/4] build: split build helper David Marchand
2020-01-29 17:26   ` [dpdk-dev] [PATCH v2 3/4] build: test meson installation David Marchand
2020-01-29 17:26   ` [dpdk-dev] [PATCH v2 4/4] add ABI checks David Marchand
2020-01-29 17:42     ` Thomas Monjalon
2020-01-29 18:10       ` Anoob Joseph
2020-01-29 20:03         ` David Marchand
2020-01-29 20:13           ` Akhil Goyal
2020-01-30 16:09             ` Ferruh Yigit
2020-01-30 20:18               ` Thomas Monjalon
2020-01-31  9:03                 ` Ferruh Yigit
2020-01-31 10:26                   ` Ananyev, Konstantin
2020-01-31 14:16                 ` Trahe, Fiona
2020-02-02 13:05                   ` Thomas Monjalon
2020-02-02 14:41                     ` Ananyev, Konstantin
2020-02-03  9:30                       ` Ferruh Yigit
2020-02-03 11:50                         ` Neil Horman
2020-02-03 13:09                           ` Ferruh Yigit
2020-02-03 14:00                             ` Dodji Seketeli
2020-02-03 14:46                               ` Ferruh Yigit
2020-02-03 15:08                             ` Trahe, Fiona
2020-02-03 17:09                         ` Thomas Monjalon
2020-02-03 17:34                           ` Thomas Monjalon
2020-02-03 18:55                             ` Ray Kinsella
2020-02-03 21:07                               ` Thomas Monjalon
2020-02-04  9:46                                 ` Ferruh Yigit
2020-02-04 10:24                                 ` Thomas Monjalon
2020-02-04 12:44                                   ` Trahe, Fiona
2020-02-04 15:52                                     ` Trahe, Fiona
2020-02-04 15:59                                       ` Thomas Monjalon
2020-02-04 17:46                                         ` Trahe, Fiona
2020-02-13 14:51                                           ` Kusztal, ArkadiuszX
2020-03-16 12:57                                             ` Trahe, Fiona
2020-03-16 13:09                                               ` Thomas Monjalon
2020-03-17 13:27                                                 ` Kusztal, ArkadiuszX
2020-03-17 15:10                                                   ` Thomas Monjalon
2020-03-17 19:10                                                     ` Kusztal, ArkadiuszX [this message]
2020-02-04 12:57                                   ` Kevin Traynor
2020-02-04 14:44                                   ` Aaron Conole
2020-02-04 19:49                                     ` Neil Horman
2020-02-04  9:51                               ` David Marchand
2020-02-04 10:10                                 ` Trahe, Fiona
2020-02-04 10:38                                   ` Thomas Monjalon
2020-02-05 11:10                                 ` Ray Kinsella
2020-02-03 17:40                           ` Ferruh Yigit
2020-02-03 18:40                             ` Thomas Monjalon
2020-02-04  9:19                               ` Ferruh Yigit
2020-02-04  9:45                                 ` Thomas Monjalon
2020-02-04  9:56                                   ` Ferruh Yigit
2020-02-04 10:08                                     ` Bruce Richardson
2020-02-04 10:17                                     ` Kevin Traynor
2020-02-04 10:16                             ` Akhil Goyal
2020-02-04 10:28                               ` Thomas Monjalon
2020-02-04 10:32                                 ` Akhil Goyal
2020-02-04 11:35                                   ` Bruce Richardson
2020-02-04 22:10                                   ` Neil Horman
2020-02-05  6:16                                     ` [dpdk-dev] [EXT] " Anoob Joseph
2020-02-05 14:33                                       ` Trahe, Fiona
2020-02-04 21:59                               ` [dpdk-dev] " Neil Horman
2020-01-30 13:06         ` Trahe, Fiona
2020-01-30 15:59           ` Thomas Monjalon
2020-01-30 16:42             ` Ferruh Yigit
2020-01-30 23:49             ` Ananyev, Konstantin
2020-01-31  9:07               ` Ferruh Yigit
2020-01-31  9:37                 ` Ananyev, Konstantin
2020-01-30 10:57   ` [dpdk-dev] [PATCH v2 0/4] " Luca Boccassi
2020-01-30 16:00 ` [dpdk-dev] [PATCH v3 " David Marchand
2020-01-30 16:00   ` [dpdk-dev] [PATCH v3 1/4] hash: fix meson headers packaging David Marchand
2020-01-30 18:01     ` Wang, Yipeng1
2020-01-30 18:40       ` Honnappa Nagarahalli
2020-02-05 19:51         ` Wang, Yipeng1
2020-01-30 16:00   ` [dpdk-dev] [PATCH v3 2/4] build: split build helper David Marchand
2020-01-30 16:00   ` [dpdk-dev] [PATCH v3 3/4] build: test meson installation David Marchand
2020-01-30 22:17     ` Thomas Monjalon
2020-01-30 16:00   ` [dpdk-dev] [PATCH v3 4/4] add ABI checks David Marchand
2020-01-30 22:32     ` Thomas Monjalon
2020-02-01 15:29       ` David Marchand
2020-01-30 22:44     ` Thomas Monjalon
2020-02-02 21:08 ` [dpdk-dev] [PATCH v4 0/3] " David Marchand
2020-02-02 21:08   ` [dpdk-dev] [PATCH v4 1/3] hash: fix meson headers packaging David Marchand
2020-02-05 19:53     ` Wang, Yipeng1
2020-02-02 21:08   ` [dpdk-dev] [PATCH v4 2/3] build: split build helper David Marchand
2020-02-02 21:08   ` [dpdk-dev] [PATCH v4 3/3] add ABI checks David Marchand
2020-02-05 14:13   ` [dpdk-dev] [PATCH v4 0/3] " Thomas Monjalon

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=BL0PR11MB331654A4EEF98E474C756F729FF60@BL0PR11MB3316.namprd11.prod.outlook.com \
    --to=arkadiuszx.kusztal@intel.com \
    --cc=aconole@redhat.com \
    --cc=akhil.goyal@nxp.com \
    --cc=anoobj@marvell.com \
    --cc=arybchenko@solarflare.com \
    --cc=bluca@debian.org \
    --cc=bruce.richardson@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=dodji@seketeli.net \
    --cc=ferruh.yigit@intel.com \
    --cc=fiona.trahe@intel.com \
    --cc=john.mcnamara@intel.com \
    --cc=konstantin.ananyev@intel.com \
    --cc=ktraynor@redhat.com \
    --cc=mdr@ashroe.eu \
    --cc=nhorman@tuxdriver.com \
    --cc=thomas@monjalon.net \
    /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).