From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id ACC594410E; Thu, 30 May 2024 13:52:26 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 3925740608; Thu, 30 May 2024 13:52:26 +0200 (CEST) Received: from dkmailrelay1.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id 6DD13402E4 for ; Thu, 30 May 2024 13:52:24 +0200 (CEST) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesys.local [192.168.4.10]) by dkmailrelay1.smartsharesystems.com (Postfix) with ESMTP id 49E9720904; Thu, 30 May 2024 13:52:24 +0200 (CEST) Content-class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Subject: RE: [EXTERNAL] [PATCH v2 1/2] crypto: fix build issues on unsetting crypto callbacks macro X-MimeOLE: Produced By Microsoft Exchange V6.5 Date: Thu, 30 May 2024 13:52:20 +0200 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35E9F4D9@smartserver.smartshare.dk> In-Reply-To: X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [EXTERNAL] [PATCH v2 1/2] crypto: fix build issues on unsetting crypto callbacks macro Thread-Index: AQHasdYqBA7BiFARTUmhLk6iVntHPbGvZbfQgAA31SCAAAr8AIAAAGqQgAACFyA= References: <20240416081222.3002268-1-ganapati.kundapura@intel.com> <20240529144025.4089318-1-ganapati.kundapura@intel.com> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Akhil Goyal" , "Kundapura, Ganapati" , , "Gujjar, Abhinandan S" , , , "Richardson, Bruce" , , X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org > From: Akhil Goyal [mailto:gakhil@marvell.com] > Sent: Thursday, 30 May 2024 13.47 >=20 > > > > #if may not be needed in application. > > > > Test should be skipped if API is not available/supported. > > > > > > It's needed otherwise application developer has to check the = implementation > for > > supported/not supported or else > > run the application to get to know whether api is supported or not. > > >=20 > Application is always required to check the return value > or else it will miss the other errors that the API can return. >=20 > > > > > diff --git a/lib/cryptodev/rte_cryptodev.c > > > > > b/lib/cryptodev/rte_cryptodev.c index 886eb7a..2e0890f 100644 > > > > > --- a/lib/cryptodev/rte_cryptodev.c > > > > > +++ b/lib/cryptodev/rte_cryptodev.c > > > > > @@ -628,6 +628,7 @@ > > > rte_cryptodev_asym_xform_capability_check_hash( > > > > > return ret; > > > > > } > > > > > > > > > > +#if RTE_CRYPTO_CALLBACKS > > > > > /* spinlock for crypto device enq callbacks */ static > > > > > rte_spinlock_t rte_cryptodev_callback_lock =3D > > > > RTE_SPINLOCK_INITIALIZER; > > > > > > > > > > @@ -744,6 +745,7 @@ cryptodev_cb_init(struct rte_cryptodev = *dev) > > > > > cryptodev_cb_cleanup(dev); > > > > > return -ENOMEM; > > > > > } > > > > > +#endif /* RTE_CRYPTO_CALLBACKS */ > > > > > > > > > > > > > @@ -1485,6 +1491,7 @@ rte_cryptodev_queue_pair_setup(uint8_t > > > dev_id, > > > > > uint16_t queue_pair_id, > > > > > socket_id); > > > > > } > > > > > > > > > > +#if RTE_CRYPTO_CALLBACKS > > > > > struct rte_cryptodev_cb * > > > > > rte_cryptodev_add_enq_callback(uint8_t dev_id, > > > > > uint16_t qp_id, > > > > > @@ -1763,6 +1770,7 @@ = rte_cryptodev_remove_deq_callback(uint8_t > > > dev_id, > > > > > rte_spinlock_unlock(&rte_cryptodev_callback_lock); > > > > > return ret; > > > > > } > > > > > +#endif /* RTE_CRYPTO_CALLBACKS */ > > > > > > > > There is an issue here. > > > > The APIs are visible in .h file and are available for = application to > use. > > > > But the API implementation is compiled out. > > > > Rather, you should add a return ENOTSUP from the beginning of = the APIs > > > > if RTE_CRYPTO_CALLBACKS is enabled. > > > > With this approach application will not need to put #if in its = code. > > API declarations wrapped under the macro changes in next patch. >=20 > No, that is not the correct way. Application should check the return = value. > And we cannot force it to add ifdefs. Agree; the function must always be present in DPDK, but fail if built = without callbacks. The ethdev function to add a callback does this [1]: const struct rte_eth_rxtx_callback * rte_eth_add_rx_callback(uint16_t port_id, uint16_t queue_id, rte_rx_callback_fn fn, void *user_param) { #ifndef RTE_ETHDEV_RXTX_CALLBACKS rte_errno =3D ENOTSUP; return NULL; #endif struct rte_eth_dev *dev; [...] [1]: = https://elixir.bootlin.com/dpdk/v24.03/source/lib/ethdev/rte_ethdev.c#L57= 29