From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id B38A83B5; Fri, 28 Apr 2017 15:40:07 +0200 (CEST) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 28 Apr 2017 06:40:04 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.37,388,1488873600"; d="scan'208";a="95005496" Received: from irsmsx105.ger.corp.intel.com ([163.33.3.28]) by fmsmga005.fm.intel.com with ESMTP; 28 Apr 2017 06:40:04 -0700 Received: from irsmsx101.ger.corp.intel.com ([169.254.1.187]) by irsmsx105.ger.corp.intel.com ([169.254.7.163]) with mapi id 14.03.0319.002; Fri, 28 Apr 2017 14:38:05 +0100 From: "Trahe, Fiona" To: "Doherty, Declan" , "dev@dpdk.org" CC: "De Lara Guarch, Pablo" , "stable@dpdk.org" , "Trahe, Fiona" Thread-Topic: [PATCH] lib/cryptodev: fix API digest length comments Thread-Index: AQHSveTriLhO2/eIAUe59lgUb4NDbaHadS4AgABQYuA= Date: Fri, 28 Apr 2017 13:38:04 +0000 Message-ID: <348A99DA5F5B7549AA880327E580B435891ECAE9@IRSMSX101.ger.corp.intel.com> References: <1493139391-9356-1-git-send-email-fiona.trahe@intel.com> <2761b670-22c5-0eda-fe08-7869ba5973d2@intel.com> In-Reply-To: <2761b670-22c5-0eda-fe08-7869ba5973d2@intel.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 10.0.102.7 dlp-reaction: no-action x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] lib/cryptodev: fix API digest length comments X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 28 Apr 2017 13:40:08 -0000 Hi Declan, > -----Original Message----- > From: Doherty, Declan > Sent: Friday, April 28, 2017 10:22 AM > To: Trahe, Fiona ; dev@dpdk.org > Cc: De Lara Guarch, Pablo ; > stable@dpdk.org > Subject: Re: [PATCH] lib/cryptodev: fix API digest length comments >=20 > On 25/04/2017 5:56 PM, Trahe, Fiona wrote: > > Fix misleading comments clarifying setting of digest length. > > > > Fixes: d11b0f30df88 ("cryptodev: introduce API and framework for crypto > devices") > > > > Cc: stable@dpdk.org > > Signed-off-by: Fiona Trahe > > --- > > lib/librte_cryptodev/rte_crypto_sym.h | 11 ++++++----- > > 1 file changed, 6 insertions(+), 5 deletions(-) > > > > diff --git a/lib/librte_cryptodev/rte_crypto_sym.h > b/lib/librte_cryptodev/rte_crypto_sym.h > > index eb7b530..12f1583 100644 > > --- a/lib/librte_cryptodev/rte_crypto_sym.h > > +++ b/lib/librte_cryptodev/rte_crypto_sym.h > > @@ -310,11 +310,10 @@ struct rte_crypto_auth_xform { > > * this specifies the length of the digest to be compared for the > > * session. > > * > > + * It is the caller's responsibility to ensure that the > > + * digest length is compliant with the hash algorithm being used. > > * If the value is less than the maximum length allowed by the hash, > > - * the result shall be truncated. If the value is greater than the > > - * maximum length allowed by the hash then an error will be > generated > > - * by *rte_cryptodev_sym_session_create* or by the > > - * *rte_cryptodev_sym_enqueue_burst* if using session-less APIs. > > + * the result shall be truncated. > > */ >=20 > I don't think this comment change is valid, we already validate many of > the parameters which are passed into session creation, such as key > lengths etc, if we are not validating digest length now I think we > should be, maybe this is a gap in our unit tests. >=20 Neither the API nor any of the PMDs validate the digest_length at present. I agree, they probably should, but it's a bit late to add this in 17.05, as it would be quite a bit of code churn, each PMD would have to check=20 against the range in their Capabilities structure. So the next best thing for this release in my opinion is to remove the comm= ent as it is misleading and out of sync with the implementation. In the next release we should remove the comments saying it's the callers=20 responsibility from both digest_length and auth key length and add=20 the param checks to each PMD. > > > > uint32_t add_auth_data_length; > > @@ -597,7 +596,9 @@ struct rte_crypto_sym_op { > > phys_addr_t phys_addr; > > /**< Physical address of digest */ > > uint16_t length; > > - /**< Length of digest */ > > + /**< Length of digest. This must be the same value as > > + * @ref rte_crypto_auth_xform.digest_length. > > + */ > > } digest; /**< Digest parameters */ > > > > struct { > >