From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pablo.de.lara.guarch@intel.com>
Received: from mga03.intel.com (mga03.intel.com [134.134.136.65])
 by dpdk.org (Postfix) with ESMTP id 420A81B79B
 for <dev@dpdk.org>; Tue, 24 Oct 2017 13:03:09 +0200 (CEST)
Received: from fmsmga004.fm.intel.com ([10.253.24.48])
 by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
 24 Oct 2017 04:03:08 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.43,427,1503385200"; d="scan'208";a="326993514"
Received: from irsmsx101.ger.corp.intel.com ([163.33.3.153])
 by fmsmga004.fm.intel.com with ESMTP; 24 Oct 2017 04:03:07 -0700
Received: from irsmsx108.ger.corp.intel.com ([169.254.11.167]) by
 IRSMSX101.ger.corp.intel.com ([169.254.1.22]) with mapi id 14.03.0319.002;
 Tue, 24 Oct 2017 12:03:07 +0100
From: "De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>
To: "Doherty, Declan" <declan.doherty@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
CC: "Doherty, Declan" <declan.doherty@intel.com>
Thread-Topic: [dpdk-dev] [PATCH 1/3] cryptodev: add new APIs to assist PMD
 initialisation
Thread-Index: AQHTSeoSkW9IXijgikGz4KfjJ4wRt6Ly02dA
Date: Tue, 24 Oct 2017 11:03:06 +0000
Message-ID: <E115CCD9D858EF4F90C690B0DCB4D8976CC3A8EE@IRSMSX108.ger.corp.intel.com>
References: <20171020212113.4543-1-declan.doherty@intel.com>
 <20171020212113.4543-2-declan.doherty@intel.com>
In-Reply-To: <20171020212113.4543-2-declan.doherty@intel.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNjZhZDIyYWUtNzVjNS00OTE5LWJiOWMtYzkxMDA5OGUzNGIwIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6IlVsNzY5RWFmVkE0dUNQUVhzeVdDaDFKXC9qeWJwRlhqOXE2aDFHK0NZWjFNPSJ9
x-ctpclassification: CTP_IC
dlp-product: dlpe-windows
dlp-version: 11.0.0.116
dlp-reaction: no-action
x-originating-ip: [163.33.239.181]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [dpdk-dev] [PATCH 1/3] cryptodev: add new APIs to assist
	PMD	initialisation
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Tue, 24 Oct 2017 11:03:10 -0000

Hi Declan,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Declan Doherty
> Sent: Friday, October 20, 2017 10:21 PM
> To: dev@dpdk.org
> Cc: Doherty, Declan <declan.doherty@intel.com>
> Subject: [dpdk-dev] [PATCH 1/3] cryptodev: add new APIs to assist PMD
> initialisation
>=20
> Adds new PMD assist functions which are bus independent for driver to
> create and destroy new device instances.
>=20
> Also includes function to parse parameters which can be passed to driver
> on device initialisation.
>=20
> Signed-off-by: Declan Doherty <declan.doherty@intel.com>

...

> +struct rte_cryptodev *
> +rte_cryptodev_pmd_create(const char *name,
> +		struct rte_device *device,
> +		struct rte_cryptodev_pmd_init_params *params) {
> +	struct rte_cryptodev *cryptodev;
> +
> +	if (params->name[0] !=3D '\0') {
> +		CDEV_LOG_INFO("[%s] User specified device name =3D %s\n",
> +				device->driver->name, params->name);
> +		name =3D params->name;
> +	}
> +
> +	CDEV_LOG_INFO("[%s] Creating cryptodev %s\n",
> +			device->driver->name, name);
> +
> +	CDEV_LOG_INFO("[%s] Initialisation parameters [ name: %s, "
> +			"private data size:  %zu, "
> +			"socket id: %d,"
> +			"max queue pairs: %u, "
> +			"max sessions: %u ]\n",
> +			device->driver->name, name, params-
> >private_data_size,
> +			params->socket_id, params->max_nb_queue_pairs,
> +			params->max_nb_sessions);

Testing this, I see the following:

CRYPTODEV: rte_cryptodev_pmd_create() line 140: [crypto_aesni_mb]
Creating cryptodev crypto_aesni_mb

CRYPTODEV: rte_cryptodev_pmd_create() line 149: [crypto_aesni_mb]
Initialisation parameters [ name: crypto_aesni_mb, private data size:  12,
socket id: 1,max queue pairs: 8, max sessions: 2048 ]

It looks too long to me. I would remove the first line (containing the line=
 number),
and also the private data size in the initialization parameters,
as I think they are not giving any useful info for a user.

...

> diff --git a/lib/librte_cryptodev/rte_cryptodev_pmd.h
...

> +/**
> + * @internal
> + *
> + * PMD assist function to provide boiler plate code for crypto driver
> +to
> + * destroy and free resources associated with a crypto PMD device
> instance.
> + *
> + * @param	name	crypto device name.
> + * @param	device	base device instance
> + * @param	params	PMD initialisation parameters
> + *
> + * @return
> + *  - crypto device instance on success
> + *  - NULL on creation failure
> + */
> +int
> +rte_cryptodev_pmd_destroy(struct rte_cryptodev *cryptodev);

Parameters and return value are wrong here.