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 2067F4591A; Fri, 6 Sep 2024 08:33:03 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id DCFD24025D; Fri, 6 Sep 2024 08:33:02 +0200 (CEST) Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) by mails.dpdk.org (Postfix) with ESMTP id 388604025C for ; Fri, 6 Sep 2024 08:33:01 +0200 (CEST) Received: from mail.maildlp.com (unknown [172.19.88.194]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4X0RFM2WT9zpVQR; Fri, 6 Sep 2024 14:31:03 +0800 (CST) Received: from dggpeml500024.china.huawei.com (unknown [7.185.36.10]) by mail.maildlp.com (Postfix) with ESMTPS id BEE0914022F; Fri, 6 Sep 2024 14:32:58 +0800 (CST) Received: from [10.67.121.161] (10.67.121.161) by dggpeml500024.china.huawei.com (7.185.36.10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.39; Fri, 6 Sep 2024 14:32:58 +0800 Subject: Re: [PATCH] [RFC] cryptodev: replace LIST_END enumerators with APIs To: =?UTF-8?Q?Morten_Br=c3=b8rup?= , Akhil Goyal , CC: , , , , , , , , , , , , , , , , , References: <20240905101438.3888274-1-gakhil@marvell.com> <98CBD80474FA8B44BF855DF32C47DC35E9F6A8@smartserver.smartshare.dk> From: fengchengwen Message-ID: <1d17f91a-fbfe-229b-9772-c149ed9d335a@huawei.com> Date: Fri, 6 Sep 2024 14:32:58 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 MIME-Version: 1.0 In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35E9F6A8@smartserver.smartshare.dk> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.121.161] X-ClientProxiedBy: dggems701-chm.china.huawei.com (10.3.19.178) To dggpeml500024.china.huawei.com (7.185.36.10) 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 On 2024/9/5 23:09, Morten Brørup wrote: >> +++ b/app/test/test_cryptodev_asym.c >> @@ -581,7 +581,7 @@ static inline void print_asym_capa( >> rte_cryptodev_asym_get_xform_string(capa->xform_type)); >> printf("operation supported -"); >> >> - for (i = 0; i < RTE_CRYPTO_ASYM_OP_LIST_END; i++) { >> + for (i = 0; i < rte_crypto_asym_op_list_end(); i++) { > >> +++ b/lib/cryptodev/rte_crypto_asym.h >> +static inline int >> +rte_crypto_asym_xform_type_list_end(void) >> +{ >> + return RTE_CRYPTO_ASYM_XFORM_SM2 + 1; >> +} >> + >> /** >> * Asymmetric crypto operation type variants >> + * Note: Update rte_crypto_asym_op_list_end for every new type added. >> */ >> enum rte_crypto_asym_op_type { >> RTE_CRYPTO_ASYM_OP_ENCRYPT, >> @@ -135,9 +141,14 @@ enum rte_crypto_asym_op_type { >> /**< Signature Generation operation */ >> RTE_CRYPTO_ASYM_OP_VERIFY, >> /**< Signature Verification operation */ >> - RTE_CRYPTO_ASYM_OP_LIST_END >> }; >> >> +static inline int >> +rte_crypto_asym_op_list_end(void) >> +{ >> + return RTE_CRYPTO_ASYM_OP_VERIFY + 1; >> +} > > I like the concept of replacing an "last enum value" with a "last enum function" for API/ABI compatibility purposes. +1 There are many such define in DPDK, e.g. RTE_ETH_EVENT_MAX > > Here's an idea... > > We can introduce a generic design pattern where we keep the _LIST_END enum value at the end, somehow marking it private (and not part of the API/ABI), and move the _list_end() function inside the C file, so it uses the _LIST_END enum value that the library was built with. E.g. like this: > > > In the header file: > > enum rte_crypto_asym_op_type { > RTE_CRYPTO_ASYM_OP_VERIFY, > /**< Signature Verification operation */ > #if RTE_BUILDING_INTERNAL > __RTE_CRYPTO_ASYM_OP_LIST_END /* internal */ > #endif > } > > int rte_crypto_asym_op_list_end(void); > > > And in the associated library code file, when including rte_crypto_asym.h: > > #define RTE_BUILDING_INTERNAL > #include > > int > rte_crypto_asym_op_list_end(void) > { > return __RTE_CRYPTO_ASYM_OP_LIST_END; > } It's more generic, and also keep LIST_END in the define, we just add new enum before it. But based on my understanding of ABI compatibility, from the point view of application, this API should return old-value even with the new library, but it will return new-value with new library. It could also break ABI. So this API should force inline, just as this patch did. But it seem can't work if move this API to header file and add static inline. > > . >