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 0F63DA04CC; Mon, 21 Sep 2020 12:59:43 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id B790D1D93B; Mon, 21 Sep 2020 12:59:42 +0200 (CEST) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by dpdk.org (Postfix) with ESMTP id 497BF1D93B for ; Mon, 21 Sep 2020 12:59:41 +0200 (CEST) IronPort-SDR: N/czLgBLg+BKVcW2ZGOvw1nKlUcet/D0WZhM2zUI3gW6yTHaDR2jyZoa7CPG8xIb1pIkb9vL/9 PO92UkYQcZ1g== X-IronPort-AV: E=McAfee;i="6000,8403,9750"; a="139850143" X-IronPort-AV: E=Sophos;i="5.77,286,1596524400"; d="scan'208";a="139850143" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Sep 2020 03:59:37 -0700 IronPort-SDR: K8UMP+Bv0EF00jMcvVmwrjTp7GUA8mlqar2PPOX7ZCJO8lmJAXr8N39Rzq+QaYchOm3mH4TjXa flp/0tnmYPlA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.77,286,1596524400"; d="scan'208";a="334339215" Received: from orsmsx602.amr.corp.intel.com ([10.22.229.15]) by fmsmga004.fm.intel.com with ESMTP; 21 Sep 2020 03:59:36 -0700 Received: from orsmsx608.amr.corp.intel.com (10.22.229.21) by ORSMSX602.amr.corp.intel.com (10.22.229.15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Mon, 21 Sep 2020 03:59:36 -0700 Received: from orsmsx601.amr.corp.intel.com (10.22.229.14) by ORSMSX608.amr.corp.intel.com (10.22.229.21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Mon, 21 Sep 2020 03:59:35 -0700 Received: from ORSEDG602.ED.cps.intel.com (10.7.248.7) by orsmsx601.amr.corp.intel.com (10.22.229.14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5 via Frontend Transport; Mon, 21 Sep 2020 03:59:35 -0700 Received: from NAM12-DM6-obe.outbound.protection.outlook.com (104.47.59.177) by edgegateway.intel.com (134.134.137.103) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.1713.5; Mon, 21 Sep 2020 03:59:32 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=hUyyk7gn1LfjzR0VTyVHs3jwF4Tj0rUgA2szYCf2fXVt9TkTBA6vlKNleEO6ACNwgYqYXE4dRDFuR2B3dyt2/sVoqOZ7UpiQ/d/dWOmyCQE7ZXkQjwnkyGJ9dSfn4+Noi45R0V289WZEpcnPhnx2RYhlUZDbZeeCVh7FPgBlkjJZ4x5jvK6aazqXJMMrcYG9cnThDAGEHBFtHKza1637U+wq/EfOfrIWbMpqDHbPpdECY7mVPIWAjrztU0ONZ3yW36i+TwDXdzQxte1IrHg027OHHuO4OdVTcz/YhCnGDC6EBZCgBopRqpqzx/Le36EEmT02MOHYbILNurIFhq9nYg== 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=s6nGwkKs+qXmUM4qfjEEFtxJYVsg0tnqPGeKi0bmJfI=; b=DIRGuG2QDcABtNs8XHR6VTTNlZfkBAexlnPWfz7N1ohjIseLDPRWC4dBvO4z3xwVNWWJcIfxNELq0ZLD1iM62jwhIQy3DNTTwV1XK9V4k5Y3OsQIjY3fxlfyCppnjQkikLSuUWgGB55WAi7jTAJ5l1NohDXtmMgMxI2xM+91A8Wk+ulDspkoyJVBnh06hm4EZwCKR0Q5XqYTo1g8KDwx1dOmvgliL7KorhpUzY33GnIqZC3tFLf3+hEdSE+nDNE3U4tp7AymT4Ca38Jl8GhAEvHTnlJWW4PWvd/Pa9R6RGfjX+DNKQka6k4KulfeUw7557JyribGLM+7KI1j45L6JQ== 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=s6nGwkKs+qXmUM4qfjEEFtxJYVsg0tnqPGeKi0bmJfI=; b=yTXjm5s/W6aIUY9IJYdWNq92nEQ+7GZE3Fjsj12ActNlW2hraCGze1508URk/qspT4QSO7XaCRnJJP4EsIlZbT6vZn1hoaxW2i1WYxOZH+WP3hNqiH4FGha4UXDpyluOHKouSFAhe1MQ6P6yHdukYnQP2kbKUwH+m0JGKaRYBSk= Received: from MWHPR11MB1838.namprd11.prod.outlook.com (2603:10b6:300:10c::11) by MWHPR11MB1775.namprd11.prod.outlook.com (2603:10b6:300:10e::14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3391.14; Mon, 21 Sep 2020 10:59:27 +0000 Received: from MWHPR11MB1838.namprd11.prod.outlook.com ([fe80::34c5:6550:a098:209f]) by MWHPR11MB1838.namprd11.prod.outlook.com ([fe80::34c5:6550:a098:209f%12]) with mapi id 15.20.3391.011; Mon, 21 Sep 2020 10:59:27 +0000 From: "Gujjar, Abhinandan S" To: "Ananyev, Konstantin" , "dev@dpdk.org" , "Doherty, Declan" , "Honnappa Nagarahalli" CC: "jerinj@marvell.com" , "akhil.goyal@nxp.com" , "Vangati, Narender" Thread-Topic: [dpdk-dev] [v2 1/2] cryptodev: support enqueue callback functions Thread-Index: AQHWhsTNpueHJn2Z1E+jIJevuNB9zqlrUBQAgAARrrCAAXx3gIAGGrIA Date: Mon, 21 Sep 2020 10:59:26 +0000 Message-ID: References: <1599549024-195051-1-git-send-email-abhinandan.gujjar@intel.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-Mentions: declan.doherty@intel.com,Honnappa.Nagarahalli@arm.com X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-reaction: no-action dlp-version: 11.5.1.3 authentication-results: intel.com; dkim=none (message not signed) header.d=none;intel.com; dmarc=none action=none header.from=intel.com; x-originating-ip: [103.5.135.70] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 4a59e6d0-fa45-48c8-e0f4-08d85e1d6858 x-ms-traffictypediagnostic: MWHPR11MB1775: 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-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: yUTLCHiBgf3oMZ+/X/QwyPnCf9l+ViwnPB86EBGD2iN/y9E5Pvx8w2fZNVb3+j65ER0D3zTTbrhRqXK4ZD7YTa91TQ0uFbue+NE8SIzDrQ8KJVgYNmiWQqTKyme9YKwftvsCoYpNAN/ZtxILmrRaNzUnobMCR7fRT4z6v/oOe097RmKEQz7ZTshxuGufqtAMpO+Vof2U1vpbxgrMc61ZfFX162SCo/PwATmYA8A/SrNTBc1dL80ScfsoIfCUNhvt7huJxSSyQTCwWXZCcMOZ13NIVxZv93UtovrPhMQqeRdCa1Ey7jF6B1DQ1eFCWWRzrNMjR0Pfdbbka58GIw2hMoLXW7yly92v9rbpS7cQhUrIlm+tKXLNYFMpr+CEre6L56DIP60qt8nih8tlweeJGbqAc1WnLechMVAQYi1/jopVaROHyijYrmJoVCWcr32+b5/DWjVYcuBoLpYVe3XBxw== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:MWHPR11MB1838.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(396003)(136003)(366004)(376002)(346002)(39860400002)(83380400001)(66946007)(966005)(107886003)(8676002)(4326008)(76116006)(33656002)(7696005)(64756008)(8936002)(66556008)(66476007)(66446008)(86362001)(30864003)(52536014)(5660300002)(186003)(316002)(71200400001)(6506007)(55236004)(2906002)(110136005)(54906003)(9686003)(53546011)(478600001)(55016002)(26005)(579004); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata: XClYn1jBc9JZBjajoKChP75QU+r57jWYkRE18MTVv74mCvlmvsQLdDuFlOtF930wrivu0HjsUBSNTtuBW6LPEmYVzRp7FJtiPEBUCV3l9HH8y5id0fKwYiwvjNogsU1Q0tRST858uB0YHCJZhIj+SlMoeJ+tAK95ezplua4Vz9qC20mQZcTtT4VTp0smk364BWm1lyZLuXen8M9BFIBKly1Sn5glr43nUCxBUlNsEBHoTTketTR2MAdOiiCFBijFhUsTw1AvugobeSHAAIpWLxRZSDh7xNX+qhIbewJ2ey/kGJC0lXb+JtPavzPxXJTptmYncSyzT3tAYBR6e6/t3RUn9YZQxP4cHnJRCoQlim5XhnNS/nKL47WF+VxU8reicnwVj20CAkJ5JMKWYPR0fYqkqhVVx2tlGp5Aoyup9uZOlwF4pTZrz3pNCopIExj/qIY6pwlRKVelgPf1V2YLk/TxPi31iW5HsWWCtvcL5V8l9ZiiiCQvkM/hAbSR2mExVUMNdnifKReFaMx+w1k/KMiFNsmQI6XxqDddQ8+EmmjFoJLHWgnsPKIHjEF0GKBLFL/g/Y1T6YLBE0u5Y3zUnM6R/gcDlmSSGfSwthyPxZlxoq4ZatHV92KKduYNLmzT7NKUUJcVC/0rQ74C92wZmA== Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: MWHPR11MB1838.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 4a59e6d0-fa45-48c8-e0f4-08d85e1d6858 X-MS-Exchange-CrossTenant-originalarrivaltime: 21 Sep 2020 10:59:26.9523 (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: PP76YV2fmyVmfF+rad5o8ymGgenXmRs0oVYouGym+AxZufGrbF4veMqSDBZ/3eSGzolFAZSRxmE+Hh4IlQjuPot6yZTnKYEpIRyi0O03n9U= X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR11MB1775 X-OriginatorOrg: intel.com Subject: Re: [dpdk-dev] [v2 1/2] cryptodev: support enqueue callback functions 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 Konstantin, Please find comments inline. @Doherty, Declan & @Honnappa Nagarahalli, Can you please chime in? Regards Abhinandan > -----Original Message----- > From: Ananyev, Konstantin > Sent: Thursday, September 17, 2020 6:55 PM > To: Gujjar, Abhinandan S ; dev@dpdk.org > Cc: Doherty, Declan ; jerinj@marvell.com; > akhil.goyal@nxp.com; Vangati, Narender > Subject: RE: [dpdk-dev] [v2 1/2] cryptodev: support enqueue callback > functions >=20 >=20 >=20 > > > > > > > > In an eventdev world, multiple workers (with ordered queue) will > > > > be working on IPsec ESP processing. The ESP header's sequence > > > > number is unique and has to be sequentially incremented in an order= ly > manner. > > > > This rises a need for incrementing sequence number in crypto stage > > > > especially in event crypto adapter. By adding a user callback to > > > > cryptodev at enqueue burst, the user callback will get executed in > > > > the context of event crypto adapter. This helps the application to > > > > increment the ESP sequence number atomically and orderly manner. > > > > > > > > This patch adds APIs to add/remove callback functions. The > > > > callback function will be called for each burst of crypto ops > > > > received on a given crypto device queue pair. > > > > > > > > v1->v2: > > > > Moved callback related members to the end of cryptodev struct > > > > Added support for RCU > > > > > > > > Signed-off-by: Abhinandan Gujjar > > > > --- > > > > config/common_base | 1 + > > > > lib/librte_cryptodev/Makefile | 2 +- > > > > lib/librte_cryptodev/rte_cryptodev.c | 157 > > > +++++++++++++++++++++++++ > > > > lib/librte_cryptodev/rte_cryptodev.h | 154 > > > +++++++++++++++++++++++- > > > > lib/librte_cryptodev/rte_cryptodev_version.map | 6 + > > > > 5 files changed, 318 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/config/common_base b/config/common_base index > > > > fbf0ee7..f5ebde4 100644 > > > > --- a/config/common_base > > > > +++ b/config/common_base > > > > @@ -599,6 +599,7 @@ > > > CONFIG_RTE_LIBRTE_PMD_BBDEV_FPGA_5GNR_FEC=3Dy > > > > # > > > > CONFIG_RTE_LIBRTE_CRYPTODEV=3Dy > > > > CONFIG_RTE_CRYPTO_MAX_DEVS=3D64 > > > > +CONFIG_RTE_CRYPTODEV_CALLBACKS=3Dy > > > > > > > > # > > > > # Compile PMD for ARMv8 Crypto device diff --git > > > > a/lib/librte_cryptodev/Makefile b/lib/librte_cryptodev/Makefile > > > > index > > > > 73e77a2..514d552 100644 > > > > --- a/lib/librte_cryptodev/Makefile > > > > +++ b/lib/librte_cryptodev/Makefile > > > > @@ -10,7 +10,7 @@ LIB =3D librte_cryptodev.a CFLAGS +=3D -O3 CFLA= GS > > > > +=3D > > > > $(WERROR_FLAGS) LDLIBS +=3D -lrte_eal -lrte_mempool -lrte_ring > > > > -lrte_mbuf -LDLIBS +=3D -lrte_kvargs > > > > +LDLIBS +=3D -lrte_kvargs -lrte_rcu > > > > > > > > # library source files > > > > SRCS-y +=3D rte_cryptodev.c rte_cryptodev_pmd.c > > > > cryptodev_trace_points.c diff --git > > > > a/lib/librte_cryptodev/rte_cryptodev.c > > > > b/lib/librte_cryptodev/rte_cryptodev.c > > > > index 1dd795b..2fb3e35 100644 > > > > --- a/lib/librte_cryptodev/rte_cryptodev.c > > > > +++ b/lib/librte_cryptodev/rte_cryptodev.c > > > > @@ -38,6 +38,7 @@ > > > > #include > > > > #include > > > > #include > > > > +#include > > > > > > > > #include "rte_crypto.h" > > > > #include "rte_cryptodev.h" > > > > @@ -499,6 +500,10 @@ struct > > > rte_cryptodev_sym_session_pool_private_data { > > > > return 0; > > > > } > > > > > > > > +#ifdef RTE_CRYPTODEV_CALLBACKS > > > > +/* spinlock for crypto device enq callbacks */ static > > > > +rte_spinlock_t rte_cryptodev_enq_cb_lock =3D > > > > +RTE_SPINLOCK_INITIALIZER; #endif > > > > > > > > const char * > > > > rte_cryptodev_get_feature_name(uint64_t flag) @@ -1449,6 > > > > +1454,158 > > > @@ > > > > struct rte_cryptodev * > > > > rte_spinlock_unlock(&rte_cryptodev_cb_lock); > > > > } > > > > > > > > +#ifdef RTE_CRYPTODEV_CALLBACKS > > > > +int > > > > +rte_cryptodev_rcu_qsbr_add(uint8_t dev_id, struct rte_rcu_qsbr > > > > +*qsbr) { > > > > + > > > > + struct rte_cryptodev *dev; > > > > + > > > > + if (!rte_cryptodev_pmd_is_valid_dev(dev_id)) { > > > > + CDEV_LOG_ERR("Invalid dev_id=3D%" PRIu8, dev_id); > > > > + return -EINVAL; > > > > + } > > > > + > > > > + dev =3D &rte_crypto_devices[dev_id]; > > > > + dev->qsbr =3D qsbr; > > > > + return 0; > > > > +} > > > > > > So if I understand your patch correctly you propose a new working > > > model for > > > crypto-devs: > > > 1. Control-plane has to allocate/setup rcu_qsbr and do > > > rte_cryptodev_rcu_qsbr_add(). > > > 2. Data-plane has somehow to obtain pointer to that rcu_qsbr and > > > wrap > > > cryptodev_enqueue() > > > with rcu_qsbr_quiescent() or rcu_qsbr_online()/rcu_qsbr_offline()= . > > Yes. I think, it is not a new model. It is same as RCU integration with= LPM. > > Please refer: https://patches.dpdk.org/cover/73673/ >=20 > I am talking about new working model for crypto-dev enqueue/dequeue. > As I said above now it becomes data-plane thread responsibility to: > -somehow to obtain pointer to that rcu_qsbr for each cryptodev it is usi= ng. > -call rcu sync functions (quiescent/online/offline) on a regular basis. It is not on regular basis. When data plane comes up, they report online. They report quiescent when they are done with critical section or shared st= ructure. All though, there is some dataplane changes involved here, I don't think, i= t is major. =20 > Note that now data-plane thread would have to do that always - even if > there are now callbacks installed for that cryptodev queue right now. > All that changes behaviour of existing apps and I presume would reduce > adoption of that fature. There is always trade off involved! In the previous patch, you suggested that some lazy app may not free up the memory allocated by add cb. For such apps, this patch has sync mechanism with some additional cost of CP & DP changes. > I still think all this callback mechanism should be totally opaque to dat= a-plane > threads - user shouldn't change his app code depending on would some > enqueue/dequeue callbacks be installed or not. I am not sure, how that can be implemented with existing RCU design. @Honnappa Nagarahalli, Do you have any suggestions? >=20 > > > > > > > > That seems quite a big change and I don't think it is acceptable for > > > most users. > > > From my perspective adding/installing call-backs to the dev has to > > > be opaque to the data-plane code. > > > Also note that different callbacks can be installed by different > > > entities (libs) and might have no idea about each other. > > > That's why I thought it would be better to make all this RCU stuff > > > internal inside cryptodev: > > > hide all this rcu_qsbr allocation/setup inside cryptod somehow to > obtain pointer to that rcu_qsbr ev init/queue setup > > > invoke rcu_qsbr_online()/rcu_qsbr_offline() inside > cryptodev_enqueue(). > > I have already tried exploring above stuffs. There are too many constra= ints. > > The changes don't fit in, as per RCU design. >=20 > Hmm could you be more specific here - what constraints are you referring > to? >=20 > > Moreover, having rcu api under enqueue_burst() will affect the > performance. >=20 > It most likely will. Though my expectation it will affect performance onl= y > when some callbacks are installed. My thought here: > callback function by itself will affect cryptdev_enqueue performance > anyway, With existing callback design, I have measured the performance(with crypto = perf test) on xeon. It was almost negligible and same was shared with Declan. That is one of the reasons, I didn't want to add to many stuffs in to the c= allback. The best part of existing design is crypto lib is not much modified. The changes are either pushed to CP or DP. so adding extra overhead for sync is probably ok here. > Though for situation when no callbacks are installed - perfomance should = be > left unaffected (or impact should be as small as possible). >=20 > > The changes are more on control plane side, which is one time. > > The data plane changes are minimal. >=20 > I still think upper layer data-plane code should stay unaffected (zero > changes). >=20 > > > > > > > + > > > > +struct rte_cryptodev_enq_callback * > > > > +rte_cryptodev_add_enq_callback(uint8_t dev_id, > > > > + uint16_t qp_id, > > > > + rte_cryptodev_enq_cb_fn cb_fn, > > > > + void *cb_arg) > > > > +{ > > > > + struct rte_cryptodev *dev; > > > > + struct rte_cryptodev_enq_callback *cb, *tail; > > > > + > > > > + if (!cb_fn) > > > > + return NULL; > > > > + > > > > + if (!rte_cryptodev_pmd_is_valid_dev(dev_id)) { > > > > + CDEV_LOG_ERR("Invalid dev_id=3D%" PRIu8, dev_id); > > > > + return NULL; > > > > + } > > > > + > > > > + dev =3D &rte_crypto_devices[dev_id]; > > > > + if (qp_id >=3D dev->data->nb_queue_pairs) { > > > > + CDEV_LOG_ERR("Invalid queue_pair_id=3D%d", qp_id); > > > > + return NULL; > > > > + } > > > > + > > > > + cb =3D rte_zmalloc(NULL, sizeof(*cb), 0); > > > > + if (cb =3D=3D NULL) { > > > > + CDEV_LOG_ERR("Failed to allocate memory for callback on " > > > > + "dev=3D%d, queue_pair_id=3D%d", dev_id, qp_id); > > > > + rte_errno =3D ENOMEM; > > > > + return NULL; > > > > + } > > > > + > > > > + cb->fn =3D cb_fn; > > > > + cb->arg =3D cb_arg; > > > > + > > > > + rte_spinlock_lock(&rte_cryptodev_enq_cb_lock); > > > > + if (dev->enq_cbs =3D=3D NULL) { > > > > + dev->enq_cbs =3D rte_zmalloc(NULL, sizeof(cb) * > > > > + dev->data->nb_queue_pairs, 0); > > > > + if (dev->enq_cbs =3D=3D NULL) { > > > > + CDEV_LOG_ERR("Failed to allocate memory for > > > callbacks"); > > > > + rte_errno =3D ENOMEM; > > > > + rte_free(cb); > > > > + return NULL; > > > > + } > > > > + } > > > > + > > > > + /* Add the callbacks in fifo order. */ > > > > + tail =3D dev->enq_cbs[qp_id]; > > > > + if (tail) { > > > > + while (tail->next) > > > > + tail =3D tail->next; > > > > + tail->next =3D cb; > > > > + } else > > > > + dev->enq_cbs[qp_id] =3D cb; > > > > + > > > > + rte_spinlock_unlock(&rte_cryptodev_enq_cb_lock); > > > > + > > > > + return cb; > > > > +} > > > > + > > > > +int > > > > +rte_cryptodev_remove_enq_callback(uint8_t dev_id, > > > > + uint16_t qp_id, > > > > + struct rte_cryptodev_enq_callback *cb) { > > > > + struct rte_cryptodev *dev; > > > > + struct rte_cryptodev_enq_callback **prev_cb, *curr_cb; > > > > + uint16_t qp; > > > > + int free_mem; > > > > + int ret; > > > > + > > > > + free_mem =3D 1; > > > > + > > > > + if (!cb) { > > > > + CDEV_LOG_ERR("cb is NULL"); > > > > + return -EINVAL; > > > > + } > > > > + > > > > + if (!rte_cryptodev_pmd_is_valid_dev(dev_id)) { > > > > + CDEV_LOG_ERR("Invalid dev_id=3D%" PRIu8, dev_id); > > > > + return -EINVAL; > > > > + } > > > > + > > > > + dev =3D &rte_crypto_devices[dev_id]; > > > > + if (qp_id >=3D dev->data->nb_queue_pairs) { > > > > + CDEV_LOG_ERR("Invalid queue_pair_id=3D%d", qp_id); > > > > + return -EINVAL; > > > > + } > > > > + > > > > + if (!dev->qsbr) { > > > > + CDEV_LOG_ERR("Rcu qsbr is NULL"); > > > > + return -EINVAL; > > > > + } > > > > + > > > > + rte_spinlock_lock(&rte_cryptodev_enq_cb_lock); > > > > + if (dev->enq_cbs =3D=3D NULL) { > > > > + rte_spinlock_unlock(&rte_cryptodev_enq_cb_lock); > > > > + return -EINVAL; > > > > + } > > > > + > > > > + prev_cb =3D &dev->enq_cbs[qp_id]; > > > > + for (; *prev_cb !=3D NULL; prev_cb =3D &curr_cb->next) { > > > > + curr_cb =3D *prev_cb; > > > > + if (curr_cb =3D=3D cb) { > > > > + /* Remove the user cb from the callback list. */ > > > > + *prev_cb =3D curr_cb->next; > > > > + ret =3D 0; > > > > + break; > > > > + } > > > > + } > > > > + > > > > + if (!ret) { > > > > + /* Call sync with invalid thread id as this is part of > > > > + * control plane API */ > > > > + rte_rcu_qsbr_synchronize(dev->qsbr, > > > > + RTE_QSBR_THRID_INVALID); > > > > + rte_free(cb); > > > > + } > > > > + > > > > + for (qp =3D 0; qp < dev->data->nb_queue_pairs; qp++) > > > > + if (dev->enq_cbs[qp] !=3D NULL) { > > > > > > Some reference count (number of callbacks) seems like a better > > > approach here. > > Ok. > > > > > > > + free_mem =3D 0; > > > > + break; > > > > + } > > > > + > > > > + if (free_mem) { > > > > + rte_free(dev->enq_cbs); > > > > + dev->enq_cbs =3D NULL; > > > > + } > > > > + > > > > + rte_spinlock_unlock(&rte_cryptodev_enq_cb_lock); > > > > + > > > > + return ret; > > > > +} > > > > +#endif > > > > > > > > int > > > > rte_cryptodev_sym_session_init(uint8_t dev_id, diff --git > > > > a/lib/librte_cryptodev/rte_cryptodev.h > > > > b/lib/librte_cryptodev/rte_cryptodev.h > > > > index 7b3ebc2..2c7a47b 100644 > > > > --- a/lib/librte_cryptodev/rte_cryptodev.h > > > > +++ b/lib/librte_cryptodev/rte_cryptodev.h > > > > @@ -530,6 +530,32 @@ struct rte_cryptodev_qp_conf { }; > > > > > > > > /** > > > > + * Function type used for pre processing crypto ops when enqueue > > > > +burst is > > > > + * called. > > > > + * > > > > + * The callback function is called on enqueue burst immediately > > > > + * before the crypto ops are put onto the hardware queue for > processing. > > > > + * > > > > + * @param dev_id The identifier of the device. > > > > + * @param qp_id The index of the queue pair in which > ops are > > > > + * to be enqueued for processing. The > value > > > > + * must be in the range [0, > nb_queue_pairs - 1] > > > > + * previously supplied to > > > > + * *rte_cryptodev_configure*. > > > > + * @param ops The address of an array of *nb_ops* > pointers > > > > + * to *rte_crypto_op* structures which > contain > > > > + * the crypto operations to be > processed. > > > > + * @param nb_ops The number of operations to process. > > > > + * @param user_param The arbitrary user parameter passed > in by the > > > > + * application when the callback was > originally > > > > + * registered. > > > > + * @return The number of ops to be enqueued > to the > > > > + * crypto device. > > > > + */ > > > > +typedef uint16_t (*rte_cryptodev_enq_cb_fn)(uint16_t dev_id, > > > > +uint16_t > > > qp_id, > > > > + struct rte_crypto_op **ops, uint16_t nb_ops, void > > > *user_param); > > > > + > > > > +/** > > > > * Typedef for application callback function to be registered by > application > > > > * software for notification of device events > > > > * > > > > @@ -853,7 +879,6 @@ struct rte_cryptodev_config { > > > > enum rte_cryptodev_event_type event, > > > > rte_cryptodev_cb_fn cb_fn, void *cb_arg); > > > > > > > > - > > > > typedef uint16_t (*dequeue_pkt_burst_t)(void *qp, > > > > struct rte_crypto_op **ops, uint16_t nb_ops); > > > > /**< Dequeue processed packets from queue pair of a device. */ @@ > > > > -870,6 +895,17 @@ typedef uint16_t (*enqueue_pkt_burst_t)(void > > > > *qp, > > > > /** Structure to keep track of registered callbacks */ > > > > TAILQ_HEAD(rte_cryptodev_cb_list, rte_cryptodev_callback); > > > > > > > > +/** > > > > + * @internal > > > > + * Structure used to hold information about the callbacks to be > > > > +called for a > > > > + * queue pair on enqueue. > > > > + */ > > > > +struct rte_cryptodev_enq_callback { > > > > + struct rte_cryptodev_enq_callback *next; > > > > + rte_cryptodev_enq_cb_fn fn; > > > > + void *arg; > > > > +}; > > > > + > > > > /** The data structure associated with each crypto device. */ > > > > struct rte_cryptodev { > > > > dequeue_pkt_burst_t dequeue_burst; @@ -898,6 +934,14 @@ > struct > > > > rte_cryptodev { > > > > __extension__ > > > > uint8_t attached : 1; > > > > /**< Flag indicating the device is attached */ > > > > + > > > > +#ifdef RTE_CRYPTODEV_CALLBACKS > > > > + struct rte_cryptodev_enq_callback **enq_cbs; > > > > + /**< User application callback for pre enqueue processing */ > > > > + > > > > + struct rte_rcu_qsbr *qsbr; > > > > + /** < RCU QSBR variable for rte_cryptodev_enq_callback */ > > > > > > Probably better to have both these fields per queue. > > > Space for them can be allocated at dev_configure() or so. > > enq_cbs is allocated during callback add. > > Unlike ethdev, each cryptodev have their own max queue pair. There is n= o > macro for that. > > I think, single RCU should be good enough, as it has mechanism to track= all > its reporting threads. > > > > > BTW, wouldn't it make sense to have ability to add callback for deque= ue > too? > > As mentioned in the commit message, this patch was driven by a > requirement. > > If required, callback for the dequeue can be added in a separate patch. > > > > > > > +#endif > > > > } __rte_cache_aligned; > > > > > > > > void * > > > > @@ -1019,6 +1063,18 @@ struct rte_cryptodev_data { > > > > struct rte_crypto_op **ops, uint16_t nb_ops) { > > > > struct rte_cryptodev *dev =3D &rte_cryptodevs[dev_id]; > > > > +#ifdef RTE_CRYPTODEV_CALLBACKS > > > > + if (unlikely(dev->enq_cbs !=3D NULL && dev->enq_cbs[qp_id] !=3D > > > NULL)) { > > > > + struct rte_cryptodev_enq_callback *cb =3D > > > > + dev->enq_cbs[qp_id]; > > > > + > > > > + do { > > > > + nb_ops =3D cb->fn(dev_id, qp_id, ops, nb_ops, > > > > + cb->arg); > > > > + cb =3D cb->next; > > > > + } while (cb !=3D NULL); > > > > + } > > > > +#endif > > > > > > > > rte_cryptodev_trace_enqueue_burst(dev_id, qp_id, (void **)ops, > > > nb_ops); > > > > return (*dev->enqueue_burst)( > > > > @@ -1351,6 +1407,102 @@ struct rte_cryptodev_asym_session * > > > > struct rte_cryptodev_sym_session *sess, union rte_crypto_sym_ofs > > > ofs, > > > > struct rte_crypto_sym_vec *vec); > > > > > > > > +#ifdef RTE_CRYPTODEV_CALLBACKS > > > > +/** > > > > + * @warning > > > > + * @b EXPERIMENTAL: this API may change without prior notice > > > > + * > > > > + * Add a user callback for a given crypto device and queue pair > > > > +which will be > > > > + * called on crypto ops enqueue. > > > > + * > > > > + * This API configures a function to be called for each burst of > > > > +crypto ops > > > > + * received on a given crypto device queue pair. The return value > > > > +is a pointer > > > > + * that can be used later to remove the callback using > > > > + * rte_cryptodev_remove_enq_callback(). > > > > + * > > > > + * Multiple functions are called in the order that they are added. > > > > + * > > > > + * @param dev_id The identifier of the device. > > > > + * @param qp_id The index of the queue pair in which > ops are > > > > + * to be enqueued for processing. The > value > > > > + * must be in the range [0, > nb_queue_pairs - 1] > > > > + * previously supplied to > > > > + * *rte_cryptodev_configure*. > > > > + * @param cb_fn The callback function > > > > + * @param cb_arg A generic pointer parameter which > will be > > > passed > > > > + * to each invocation of the callback > function on > > > > + * this crypto device and queue pair. > > > > + * > > > > + * @return > > > > + * NULL on error. > > > > + * On success, a pointer value which can later be used to remove= the > > > callback. > > > > + */ > > > > + > > > > +__rte_experimental > > > > +struct rte_cryptodev_enq_callback * > > > > +rte_cryptodev_add_enq_callback(uint8_t dev_id, > > > > + uint16_t qp_id, > > > > + rte_cryptodev_enq_cb_fn cb_fn, > > > > + void *cb_arg); > > > > + > > > > + > > > > +/** > > > > + * @warning > > > > + * @b EXPERIMENTAL: this API may change without prior notice > > > > + * > > > > + * Remove a user callback function for given crypto device and que= ue > pair. > > > > + * > > > > + * This function is used to removed callbacks that were added to > > > > +a crypto > > > > + * device queue pair using rte_cryptodev_add_enq_callback(). > > > > + * > > > > + * Note: The callback expects a RCU QSBR to be configured to > > > > +synchronize > > > > + * to free the memory. Application is expected to configure RCU > > > > +QSBR after > > > > + * adding an enqueue callback. > > > > + * > > > > + * > > > > + * @param dev_id The identifier of the device. > > > > + * @param qp_id The index of the queue pair in which > ops are > > > > + * to be enqueued for processing. The > value > > > > + * must be in the range [0, > nb_queue_pairs - 1] > > > > + * previously supplied to > > > > + * *rte_cryptodev_configure*. > > > > + * @param cb Pointer to user supplied callback > created via > > > > + * rte_cryptodev_add_enq_callback(). > > > > + * > > > > + * @return > > > > + * - 0: Success. Callback was removed. > > > > + * - -EINVAL: The dev_id or the qp_id is out of range, or the c= allback > > > > + * is NULL or not found for the crypto device queue = pair. > > > > + */ > > > > + > > > > +__rte_experimental > > > > +int rte_cryptodev_remove_enq_callback(uint8_t dev_id, > > > > + uint16_t qp_id, > > > > + struct rte_cryptodev_enq_callback *cb); > > > > + > > > > + > > > > +/** > > > > + * @warning > > > > + * @b EXPERIMENTAL: this API may change without prior notice > > > > + * > > > > + * Associate RCU QSBR variable with a cryptodev. > > > > + * > > > > + * This function is used to add RCU QSBR to a crypto device. > > > > + * The purpose of RCU is to help multiple threads to synchronize > > > > + * with each other before initiating adding/removing callback > > > > + * while dataplane threads are running enqueue callbacks. > > > > + * > > > > + * @param dev_id The identifier of the device. > > > > + * @param qsr RCU QSBR configuration > > > > + * @return > > > > + * On success - 0 > > > > + * On error - EINVAL. > > > > + */ > > > > + > > > > +__rte_experimental > > > > +int rte_cryptodev_rcu_qsbr_add(uint8_t dev_id, struct > > > > +rte_rcu_qsbr *qsbr); #endif > > > > + > > > > #ifdef __cplusplus > > > > } > > > > #endif > > > > diff --git a/lib/librte_cryptodev/rte_cryptodev_version.map > > > > b/lib/librte_cryptodev/rte_cryptodev_version.map > > > > index 02f6dcf..46de3ca 100644 > > > > --- a/lib/librte_cryptodev/rte_cryptodev_version.map > > > > +++ b/lib/librte_cryptodev/rte_cryptodev_version.map > > > > @@ -64,6 +64,7 @@ DPDK_20.0 { > > > > rte_cryptodev_sym_capability_get; }; > > > > > > > > + > > > > EXPERIMENTAL { > > > > global: > > > > > > > > @@ -105,4 +106,9 @@ EXPERIMENTAL { > > > > > > > > # added in 20.08 > > > > rte_cryptodev_get_qp_status; > > > > + > > > > + # added in 20.11 > > > > + rte_cryptodev_add_enq_callback; > > > > + rte_cryptodev_remove_enq_callback; > > > > + rte_cryptodev_rcu_qsbr_add; > > > > }; > > > > -- > > > > 1.9.1