DPDK patches and discussions
 help / color / mirror / Atom feed
From: "De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>
To: Pavan Nikhilesh Bhagavatula <pbhagavatula@caviumnetworks.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	"Trahe, Fiona" <fiona.trahe@intel.com>
Cc: "Doherty, Declan" <declan.doherty@intel.com>
Subject: Re: [dpdk-dev] [PATCH] cryptodev: fix cryptodev start return value
Date: Tue, 11 Jul 2017 14:08:28 +0000	[thread overview]
Message-ID: <E115CCD9D858EF4F90C690B0DCB4D8976CBD9F25@IRSMSX108.ger.corp.intel.com> (raw)
In-Reply-To: <20170608081242.GA19599@PBHAGAVATULA-LT>



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Pavan Nikhilesh
> Bhagavatula
> Sent: Thursday, June 8, 2017 9:13 AM
> To: dev@dpdk.org; Trahe, Fiona <fiona.trahe@intel.com>
> Cc: Doherty, Declan <declan.doherty@intel.com>; Pavan Nikhilesh
> <pbhagavatula@caviumnetworks.com>
> Subject: Re: [dpdk-dev] [PATCH] cryptodev: fix cryptodev start return value
> 
> On Wed, Jun 07, 2017 at 03:54:23PM +0000, Trahe, Fiona wrote:
> > Hi Pavan,
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Pavan
> Nikhilesh
> > > Sent: Wednesday, June 7, 2017 11:37 AM
> > > To: dev@dpdk.org
> > > Cc: Doherty, Declan <declan.doherty@intel.com>; Pavan Nikhilesh
> > > Bhagavatula <pbhagavatula@caviumnetworks.com>
> > > Subject: [dpdk-dev] [PATCH] cryptodev: fix cryptodev start return
> > > value
> > >
> > > From: Pavan Nikhilesh Bhagavatula
> <pbhagavatula@caviumnetworks.com>
> > >
> > > If cryptodev has already started it should return -EBUSY instead of
> > > 0 when rte_cryptodev_start is called.
> > >
> > > Fixes: d11b0f30df88 ("cryptodev: introduce API and framework for
> > > crypto devices")
> > >
> > > Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> > > ---
> > >  lib/librte_cryptodev/rte_cryptodev.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/lib/librte_cryptodev/rte_cryptodev.c
> > > b/lib/librte_cryptodev/rte_cryptodev.c
> > > index b65cd9c..c815038 100644
> > > --- a/lib/librte_cryptodev/rte_cryptodev.c
> > > +++ b/lib/librte_cryptodev/rte_cryptodev.c
> > > @@ -1000,7 +1000,7 @@ rte_cryptodev_start(uint8_t dev_id)
> > >  	if (dev->data->dev_started != 0) {
> > >  		CDEV_LOG_ERR("Device with dev_id=%" PRIu8 " already
> started",
> > >  			dev_id);
> > > -		return 0;
> > > +		return -EBUSY;
> > It makes sense to me to return 0/success in this case, as the end
> > result is the same, the device is successfully started.
> > But I don't feel strongly about it if there's a good argument for making the
> change?
> 
> I do agree with this but from an application perspective when the API is
> called again after the device has already started (without calling the stop
> API) it would mean that there is an underlying issue with the application's
> business logic and it would go undetected, so I feel that we should strictly
> enforce this scenario as an error.
> 
> > However, as it is an API change doesn't it need to be flagged in a release
> before the change is made?
> 
> I don't think that this would be an API change as it doesn't deprecate the
> existing ABI.
> Any thoughts about this from the community are welcome as the same
> issue affects multiple core libraries (crytodev, ethdev, eventdev).

Hi Pavan,

As said by Fiona and Thomas (for ethdev), this is an API change, so it would
require a deprecation notice, in order to be changed in the next release.

Thanks,
Pablo
> >
> >
> > }
> > >
> > >  	diag = (*dev->dev_ops->dev_start)(dev);
> > > --
> > > 2.7.4
> >

      reply	other threads:[~2017-07-11 14:08 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-07 10:36 Pavan Nikhilesh
2017-06-07 15:54 ` Trahe, Fiona
2017-06-08  8:12   ` Pavan Nikhilesh Bhagavatula
2017-07-11 14:08     ` De Lara Guarch, Pablo [this message]

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=E115CCD9D858EF4F90C690B0DCB4D8976CBD9F25@IRSMSX108.ger.corp.intel.com \
    --to=pablo.de.lara.guarch@intel.com \
    --cc=declan.doherty@intel.com \
    --cc=dev@dpdk.org \
    --cc=fiona.trahe@intel.com \
    --cc=pbhagavatula@caviumnetworks.com \
    /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).