From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
Subject: Re: [RFC] cryptodev: proposed changes in
	rte_cryptodev_sym_session
Date: Wed, 14 Nov 2018 08:35:17 +0000
Message-ID: <2601191342CEEE43887BDE71AB977258010CEB62FB@IRSMSX106.ger.corp.intel.com>
References: <1535132906-5167-1-git-send-email-konstantin.ananyev@intel.com>
 <348A99DA5F5B7549AA880327E580B43589676DA2@IRSMSX101.ger.corp.intel.com>
 <2601191342CEEE43887BDE71AB977258010CE4AB41@IRSMSX106.ger.corp.intel.com>
 <348A99DA5F5B7549AA880327E580B43589677E12@IRSMSX101.ger.corp.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Cc: "De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>, Akhil Goyal
 <akhil.goyal@nxp.com>, "Doherty, Declan" <declan.doherty@intel.com>, "Ravi
 Kumar" <ravi1.kumar@amd.com>, Jerin Jacob <jerin.jacob@caviumnetworks.com>,
 "Zhang, Roy Fan" <roy.fan.zhang@intel.com>, Tomasz Duszynski
 <tdu@semihalf.com>, Hemant Agrawal <hemant.agrawal@nxp.com>, Natalie Samsonov
 <nsamsono@marvell.com>, Dmitri Epshtein <dima@marvell.com>, Jay Zhou
 <jianjay.zhou@huawei.com>, "Zhang, Roy Fan" <roy.fan.zhang@intel.com>
To: "Trahe, Fiona" <fiona.trahe@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
Return-path: <dev-bounces@dpdk.org>
Received: from mga04.intel.com (mga04.intel.com [192.55.52.120])
 by dpdk.org (Postfix) with ESMTP id 354AF11A4
 for <dev@dpdk.org>; Wed, 14 Nov 2018 09:35:22 +0100 (CET)
In-Reply-To: <348A99DA5F5B7549AA880327E580B43589677E12@IRSMSX101.ger.corp.intel.com>
Content-Language: en-US
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>


> > > > @@ -1301,8 +1346,7 @@ rte_cryptodev_sym_session_free(struct rte_cry=
ptodev_sym_session *sess)
> > > >
> > > >  	/* Check that all device private data has been freed */
> > > >  	for (i =3D 0; i < nb_drivers; i++) {
> > > [Fiona] Use the sess.nb_drivers rather than the global.
> >
> > Ok, though doesn't really matter here.
> > get_sym_session_private_data() will return NULL for invalid index anywa=
y.
> [Fiona] Except that you removed the NULL check below and are using that
> invalid index to deref the array.

Ah yes, you right - have to use sess.nb_drivers here.

>=20
> > > Actually maybe name slightly differently to reduce the chance of that=
 mistake happening, e.g.
> > > rename sess.nb_drivers to sess.num_drivers?
> > >
> > > > -		sess_priv =3D get_sym_session_private_data(sess, i);
> > > > -		if (sess_priv !=3D NULL)
> > > > +		if (sess->sess_data[i].refcnt !=3D 0)
> > > >  			return -EBUSY;
> > > >  	}
> > > >
> > > > @@ -1313,6 +1357,23 @@ rte_cryptodev_sym_session_free(struct rte_cr=
yptodev_sym_session *sess)
> > > >  	return 0;
> > > >  }
> > > >
> > > > +unsigned int
> > > > +rte_cryptodev_sym_session_max_data_size(void)
> > > [Fiona] Suggest renaming this
> > > rte_cryptodev_sym_session_max_array_size()
> >
> > I usually don't care much about names, but that seems just confusing:
> > totally unclear which array we are talking about.
> > Any better naming ideas?
> [Fiona] Isn't it returning the size of the array of structs in the sessio=
n hdr?
> Seems a bit more informative than "data".

Yes, but user doesn't have to know it is an array of pointers or
something different.

> Can't think of anything better, if you find array confusing then I
> don't feel that strongly about it, stick with data.
> Or just get rid of the function altogether and put inside
> rte_cryptodev_sym_session_max_size()
> Unless it's called elsewhere.

Sounds like a good option.
Konstantin