From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id D6A05A059F; Fri, 10 Apr 2020 16:34:31 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id AED6E1D558; Fri, 10 Apr 2020 16:34:31 +0200 (CEST) Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by dpdk.org (Postfix) with ESMTP id BB9A01D557 for ; Fri, 10 Apr 2020 16:34:29 +0200 (CEST) IronPort-SDR: OkV4Sl3KAvEKWV59U4ZbE/+itrcMbJCF1wWyF5P7oMjKuhRQr+TqA+08OMMPuBcRbS9e3wFul/ 1Iu80mBHIi0A== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Apr 2020 07:34:28 -0700 IronPort-SDR: 4rhqSfvXOZywbdnrhiRutgYZssSPrbvlGKxozzoEpmSUBQKLDVTMymWSHcw51GSZHvv8oD5Dnr WM7f/+8sM+mg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,367,1580803200"; d="scan'208";a="240956196" Received: from orsmsx107.amr.corp.intel.com ([10.22.240.5]) by orsmga007.jf.intel.com with ESMTP; 10 Apr 2020 07:34:28 -0700 Received: from ORSEDG002.ED.cps.intel.com (10.7.248.5) by ORSMSX107.amr.corp.intel.com (10.22.240.5) with Microsoft SMTP Server (TLS) id 14.3.439.0; Fri, 10 Apr 2020 07:34:28 -0700 Received: from NAM10-DM6-obe.outbound.protection.outlook.com (104.47.58.100) by edgegateway.intel.com (134.134.137.101) with Microsoft SMTP Server (TLS) id 14.3.439.0; Fri, 10 Apr 2020 07:34:28 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=TpQksXxM+Eq9nl+PCZciBnZsHohAmdsjhpF9LYu3tz/bxS2zTwVays+w7b/Sz6xgMcM2xDCNnihYeEHVvhyxgFmmoyhgFss2ZTuRb61by1qe+tg3n9mJ7VfcD7SeICVCYo1g0mnPUf/TJhpfniDNPMQLUwl8CX6+u+vFAsL4tbmx8Hg4GfYeUV3TljfDhqL5VLy0WBCTT+Wc8qbc4n0WdDmBnU+drrLEPspXelucGgMfDvtB8384gv4+BmN5xth21yCgrl5wkA8vCIECC2+soQHiJZa4ULmjr+jope/1Dos76R2DJ3x7ultA3zoTY46AXpsbI9QAo89XT1SCi9kxKg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=34fUNTgdLlmJQue193h4opUe/ijkivrwkLw+k5sKMek=; b=MyM/XyRO3VldPh7VFgl3LoMF+wYdjasVK4/B6w5AvrbgKhPZTKB9KNO5z22vWI82qII00tiMA/hng6ywlz7iQuyP5LCRQawbJNXruGKkEJD3XfN+CdVzN9xuB1MMN5+wtfpVUXI8s6po0wJ3sOlN9A5BHPNEbhU3cH0V55ZkPfud21nRF/lcV1mFy1m4l3nUVgvSVKizsY4QrSI5jd4cu/KZsoPTmPMXsn5iM2IW9bXGUsGmXINvo9wHJz+bxy+iClKRKX+IRgLtQynkyT36xj3BI2T/HWmQE2UaEUWbk8dGbNCsYpIZeV1dLUF87iHYuGjlApmJpnJh7ygac/alig== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=34fUNTgdLlmJQue193h4opUe/ijkivrwkLw+k5sKMek=; b=OZazQjLM2JAtCP++IOKJqktzBrNSuH7ChnoZKOj47GUmFbZafHD9lKQUb4fo6s2cG5sMKgLd3mFFyXylt+lF39lBI86j5p9f767V1S0EkUtgZsdIypOAXeAsQug64wPrhbbhWE/Ejq7eLrF+nW7K/pmFaE9tijOU9rbCfApx1oo= Received: from MN2PR11MB3550.namprd11.prod.outlook.com (2603:10b6:208:ee::21) by MN2PR11MB4510.namprd11.prod.outlook.com (2603:10b6:208:17b::22) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2878.21; Fri, 10 Apr 2020 14:34:26 +0000 Received: from MN2PR11MB3550.namprd11.prod.outlook.com ([fe80::b945:c80d:ab08:cccf]) by MN2PR11MB3550.namprd11.prod.outlook.com ([fe80::b945:c80d:ab08:cccf%7]) with mapi id 15.20.2878.018; Fri, 10 Apr 2020 14:34:26 +0000 From: "Coyle, David" To: "De Lara Guarch, Pablo" , "dev@dpdk.org" CC: "Doherty, Declan" , "Trahe, Fiona" , "Ryan, Brendan" , "shreyansh.jain@nxp.com" , "hemant.agrawal@nxp.com" , "O'loingsigh, Mairtin" Thread-Topic: [PATCH v2 2/4] raw/aesni_mb: add aesni_mb raw device Thread-Index: AQHWCdfYXfVJC4xeSkaSgVensLTynKhuB2AAgAMbhcA= Date: Fri, 10 Apr 2020 14:34:26 +0000 Message-ID: References: <20200403163656.60545-1-david.coyle@intel.com> <20200403163656.60545-3-david.coyle@intel.com> In-Reply-To: Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-reaction: no-action dlp-version: 11.2.0.6 dlp-product: dlpe-windows authentication-results: spf=none (sender IP is ) smtp.mailfrom=david.coyle@intel.com; x-originating-ip: [192.198.151.188] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 84153e6f-517b-4cb4-91c5-08d7dd5c453d x-ms-traffictypediagnostic: MN2PR11MB4510: x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-forefront-prvs: 0369E8196C x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:MN2PR11MB3550.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFTY:; SFS:(10019020)(346002)(396003)(136003)(39860400002)(376002)(366004)(66446008)(4326008)(81156014)(86362001)(64756008)(66556008)(55016002)(9686003)(186003)(8676002)(2906002)(33656002)(66476007)(107886003)(66946007)(316002)(71200400001)(6506007)(8936002)(54906003)(26005)(52536014)(110136005)(478600001)(76116006)(7696005)(5660300002); DIR:OUT; SFP:1102; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: HE67h/0yymopBuTR2X1FCLwaiAc43ZGxdrD7Y/v73U4zow80TZ3HsHFTPcqYMLiXfmUWnr1Anb8mJo24LJy07FGlHxOl7vnQ0FbdUIh9+vtZast/ySj+QmRTvAhVPb7LHZRLmQLNj9/7vD1LyWW9LHAsxpv2ZDafM5Zlftgzh9EwxijstoinuLaaejQVadBnQsAmTSgDz16DKCR/975JnuYYRD9I3mahBlEzpTjV5m37wuLS9wFDp46VuLLs6v4S2p8elYcQ40w9jNa7hJ4SC44bcBAbwfS+WlhgqDecw7J917rkRAi5VsTsGQ6N9eAHI65niZiRFlwe0qZUeC7tA0PH9ICwmQm++uAvqGzd0cyreI+t+ZZMO3bdk5txq41MHIOlBMsSkF2/vBUv+z++jRJxVV2Z6iy0CuJqg5i8ENLmnT/Kuh5dXEGTVJ+HmwuP x-ms-exchange-antispam-messagedata: V8zyrDKTLgRz5EVwRu0TxWpTOjjJVp1ORjDYc4ISqbMP8o74RHoqooj6rQZcfwnU20A9H1njOz3MZ0G9rzpHJAMUeeF9H6ZjVKhoEFeF4wmjL4U004DEsPkJcQ9UU5jzZmgfco60Er39AamggW+WKw== Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-Network-Message-Id: 84153e6f-517b-4cb4-91c5-08d7dd5c453d X-MS-Exchange-CrossTenant-originalarrivaltime: 10 Apr 2020 14:34:26.3487 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: FGMKSAhb0YlilrY2hqheL0tLz7KR+GfLlMgOEHwKaTVx4qlSNEuZ4l7SisaNrvWLb8KkJTOV6BKiGCJ8HVgp7Q== X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR11MB4510 X-OriginatorOrg: intel.com Subject: Re: [dpdk-dev] [PATCH v2 2/4] raw/aesni_mb: add aesni_mb raw device 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Pablo Thank you for reviewing and the comments - see below for resolutions. The changes will be available in v3 shortly David > -----Original Message----- > From: De Lara Guarch, Pablo > Sent: Tuesday, April 7, 2020 7:51 PM >=20 > Hi David, >=20 > > -----Original Message----- > > From: Coyle, David > > Sent: Friday, April 3, 2020 5:37 PM > > > > Adding an AESNI-MB raw device, thereby exposing AESNI-MB to the > rawdev > > API. The AESNI-MB raw device will use the multi-function interface to > > allow combined operations be sent to the AESNI-MB software library. > > > > Signed-off-by: David Coyle > > Signed-off-by: Mairtin o Loingsigh > > --- > > config/common_base | 6 + > > drivers/raw/Makefile | 2 + > > drivers/raw/aesni_mb/Makefile | 47 + > > drivers/raw/aesni_mb/aesni_mb_rawdev.c | 1536 > +++++++++++++++++ > > drivers/raw/aesni_mb/aesni_mb_rawdev.h | 112 ++ > > drivers/raw/aesni_mb/aesni_mb_rawdev_test.c | 1102 ++++++++++++ > > .../aesni_mb/aesni_mb_rawdev_test_vectors.h | 1183 +++++++++++++ > > drivers/raw/aesni_mb/meson.build | 26 + > > .../aesni_mb/rte_rawdev_aesni_mb_version.map | 3 + > > drivers/raw/meson.build | 3 +- > > mk/rte.app.mk | 2 + >=20 > You missed adding the PMD to the MAINTAINERS file. [DC] Added the new directories to MAINTAINERS file >=20 > > 11 files changed, 4021 insertions(+), 1 deletion(-) create mode > > 100644 drivers/raw/aesni_mb/Makefile create mode 100644 > > drivers/raw/aesni_mb/aesni_mb_rawdev.c > > create mode 100644 drivers/raw/aesni_mb/aesni_mb_rawdev.h > > create mode 100644 drivers/raw/aesni_mb/aesni_mb_rawdev_test.c > > create mode 100644 > > drivers/raw/aesni_mb/aesni_mb_rawdev_test_vectors.h > > create mode 100644 drivers/raw/aesni_mb/meson.build create mode > > 100644 drivers/raw/aesni_mb/rte_rawdev_aesni_mb_version.map > > ... > > diff --git a/drivers/raw/aesni_mb/aesni_mb_rawdev.c > > b/drivers/raw/aesni_mb/aesni_mb_rawdev.c > > new file mode 100644 > > index 000000000..946bdd871 > > --- /dev/null > > +++ b/drivers/raw/aesni_mb/aesni_mb_rawdev.c > > @@ -0,0 +1,1536 @@ > > +/* SPDX-License-Identifier: BSD-3-Clause > > + * Copyright(c) 2020 Intel Corporation. > > + */ > > + > > +#include > > + > > +#include > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include >=20 > No need for , , , > and . > I think is missing, though, for "rte_crypto_sym_xform". [DC] Removed only hexdump and dev. All the others would seem to be needed. rte_cpu_get_flag_enabled() is called from cpuflags And lots of stuff from multi_fn is used throughout this file >=20 > ... >=20 > > +static bool > > +docsis_crc_crypto_encrypt_check(struct rte_multi_fn_xform *xform) { > > + struct rte_crypto_sym_xform *crypto_sym; > > + struct rte_multi_fn_err_detect_xform *err_detect; > > + struct rte_multi_fn_xform *next; > > + > > + if (xform->type =3D=3D RTE_MULTI_FN_XFORM_TYPE_ERR_DETECT) { > > + > > + err_detect =3D &xform->err_detect; > > + next =3D xform->next; > > + > > + if (err_detect->algo =3D=3D > > + RTE_MULTI_FN_ERR_DETECT_CRC32_ETH > && > > + err_detect->op =3D=3D > > + RTE_MULTI_FN_ERR_DETECT_OP_GENERATE > > && >=20 > I don't think leading spaces are allowed. Generally, double tab is used i= n > multi-line if's. Same applies in other parts of the code. [DC] Indentation of multi-line if statements have been fixed here and in ot= her patches >=20 > > + next !=3D NULL && > > + next->type =3D=3D > RTE_MULTI_FN_XFORM_TYPE_CRYPTO_SYM) { > > + >=20 > ... >=20 > > +static bool > > +docsis_crypto_decrypt_crc_check(struct rte_multi_fn_xform *xform) { > > + struct rte_crypto_sym_xform *crypto_sym; > > + struct rte_multi_fn_err_detect_xform *err_detect; > > + struct rte_multi_fn_xform *next; > > + > > + if (xform->type =3D=3D RTE_MULTI_FN_XFORM_TYPE_CRYPTO_SYM) { >=20 > I think in order to reduce this many indentation levels, you can check fo= r the > opposite here and return false. [DC] This was a good suggestion. Have use opposite logic throughout all the= se "check" functions which greatly reduces indentation >=20 > If (xform->type !=3D RTE...) > return false > ... >=20 > > + > > +static bool > > +pon_crc_crypto_encrypt_bip_check(struct rte_multi_fn_xform *xform) { > > + struct rte_crypto_sym_xform *crypto_sym; > > + struct rte_multi_fn_err_detect_xform *err_detect; > > + struct rte_multi_fn_xform *next; > > + > > + if (xform->type =3D=3D RTE_MULTI_FN_XFORM_TYPE_ERR_DETECT) { > > + > > + err_detect =3D &xform->err_detect; > > + next =3D xform->next; >=20 > Same as above here. [DC] Same fix as above >=20 > > + > > + if (err_detect->algo =3D=3D > > + RTE_MULTI_FN_ERR_DETECT_CRC32_ETH > && > > + err_detect->op =3D=3D >=20 > > +static bool > > +pon_bip_crypto_decrypt_crc_check(struct rte_multi_fn_xform *xform) { > > + struct rte_crypto_sym_xform *crypto_sym; > > + struct rte_multi_fn_err_detect_xform *err_detect; > > + struct rte_multi_fn_xform *next; > > + > > + if (xform->type =3D=3D RTE_MULTI_FN_XFORM_TYPE_ERR_DETECT) { > > + > > + err_detect =3D &xform->err_detect; > > + next =3D xform->next; >=20 > Same as above. [DC] And same here too >=20 > > +} > > + > > +static enum aesni_mb_rawdev_op > > +session_support_check(struct rte_multi_fn_xform *xform) { > > + enum aesni_mb_rawdev_op op =3D > > AESNI_MB_RAWDEV_OP_NOT_SUPPORTED; > > + >=20 > ... >=20 > > +static inline int > > +docsis_crypto_crc_check(struct rte_multi_fn_op *first_op, > > + struct rte_multi_fn_op *cipher_op, > > + struct rte_multi_fn_op *crc_op) > > +{ > > + struct rte_multi_fn_op *err_op =3D NULL; > > + uint8_t err_op_status; > > + const uint32_t offset_diff =3D DOCSIS_CIPHER_CRC_OFFSET_DIFF; > > + > > + if (cipher_op->crypto_sym.cipher.data.length && > > + crc_op->err_detect.data.length) { > > + /* Cipher offset must be at least 12 greater than CRC offset > */ > > + if (cipher_op->crypto_sym.cipher.data.offset < > > + ((uint32_t)crc_op->err_detect.data.offset + offset_diff)) { > > + err_op =3D crc_op; > > + err_op_status =3D > > RTE_MULTI_FN_ERR_DETECT_OP_STATUS_ERROR; > > + /* > > + * Cipher length must be at least 8 less than CRC length, > taking > > + * known differences of what is ciphered and what is crc'ed > into > > + * account > > + */ > > + } else if ((cipher_op->crypto_sym.cipher.data.length + > > + DOCSIS_CIPHER_CRC_LENGTH_DIFF) > >=20 > For consistency, you can use offset_diff here too, instead of the macro. [DC] This check here is a LENGTH_DIFF so re-using offset_diff would work so= well I only added the offset_diff variable to get around a compiler error, so I = would have used the macro there too if I could have. Decided not to make any change here >=20 > > + crc_op->err_detect.data.length) { > > + err_op =3D crc_op; > > + err_op_status =3D > > RTE_MULTI_FN_ERR_DETECT_OP_STATUS_ERROR; > > + } > > + } >=20 > ... >=20 > > +static inline int > > +mb_job_params_set(JOB_AES_HMAC *job, > > + struct aesni_mb_rawdev_qp *qp, > > + struct rte_multi_fn_op *op, > > + uint8_t *output_idx) > > +{ > > + struct rte_mbuf *m_src, *m_dst; > > + struct rte_multi_fn_op *cipher_op; > > + struct rte_multi_fn_op *crc_op; > > + struct rte_multi_fn_op *bip_op; > > + uint32_t cipher_offset; > > + struct aesni_mb_rawdev_session *session; > > + >=20 > ... >=20 > > + cipher_op->crypto_sym.cipher.data.length; >=20 > I would declare a variable like sym_op to reduce the length of these. > Maybe also for err_dectect. [DC] Have added variables like you suggested which does improve things alri= ght in terms of line length >=20 > > + > > + switch (session->op) { > > + case AESNI_MB_RAWDEV_OP_DOCSIS_CRC_CRYPTO: > > + case AESNI_MB_RAWDEV_OP_DOCSIS_CRYPTO_CRC: > > + job->hash_start_src_offset_in_bytes =3D > > + crc_op- >=20 > ... >=20 >=20 > > +++ b/drivers/raw/aesni_mb/aesni_mb_rawdev_test.c > > @@ -0,0 +1,1102 @@ > > +/* SPDX-License-Identifier: BSD-3-Clause > > + * Copyright(c) 2020 Intel Corporation. >=20 > Could this be moved under the test app? Looks odd here. [DC] As you already spotted yourself, this is how rawdev define there tests= . When we add the QAT multi-function PMD, we will have to look at this though= as both will need the same set of tests. May have to add the tests directly under the test app then >=20 > Thanks, > Pablo