From patchwork Fri Aug 12 08:58:36 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Srujana Challa X-Patchwork-Id: 114857 X-Patchwork-Delegate: gakhil@marvell.com Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 9121DA0543; Fri, 12 Aug 2022 10:58:46 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 341D640A7D; Fri, 12 Aug 2022 10:58:45 +0200 (CEST) Received: from mx0b-0016f401.pphosted.com (mx0a-0016f401.pphosted.com [67.231.148.174]) by mails.dpdk.org (Postfix) with ESMTP id 8F93B406A2 for ; Fri, 12 Aug 2022 10:58:43 +0200 (CEST) Received: from pps.filterd (m0045849.ppops.net [127.0.0.1]) by mx0a-0016f401.pphosted.com (8.17.1.5/8.17.1.5) with ESMTP id 27C46UFN028001; Fri, 12 Aug 2022 01:58:42 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=marvell.com; h=from : to : cc : subject : date : message-id : mime-version : content-transfer-encoding : content-type; s=pfpt0220; bh=5HP3k6Xlkt5mpLUzjeKHmrqGi9Inia/bJFNaiul02EA=; b=auqLSpc4NmcPDR6EEoUOVgqNtyp5F2VkGkqZeEmDnNLSk7jVaMEza8aHbwKDMiQG+WJC tZSqlwaPtCa41nxA4XZWFMsrvSiYbDVCEu4nxg+gSMMZItEXTQ8Kl2e2u3g5fijDP3af YFgmR2d5nBWM6xWMP1U86ekrV7MeJGoQAQJh1gZH83Mcg2vWTRJPazcWgdo3YDhWJ5ri vBak/WsgyNyupN/hX/bY+vVIC0zAJFKQQSXJKM/mBwQOvh9Hfu4EnXELXn4U1QoKUEaf wbaFqfz0qQgUCEUTuz/uNOaUQ/rCtyMl6p83FyTwRBhm62vDuogD9WoPXyMQJ3acCOPY xw== Received: from dc5-exch02.marvell.com ([199.233.59.182]) by mx0a-0016f401.pphosted.com (PPS) with ESMTPS id 3hwfj0guss-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=NOT); Fri, 12 Aug 2022 01:58:41 -0700 Received: from DC5-EXCH01.marvell.com (10.69.176.38) by DC5-EXCH02.marvell.com (10.69.176.39) with Microsoft SMTP Server (TLS) id 15.0.1497.18; Fri, 12 Aug 2022 01:58:40 -0700 Received: from maili.marvell.com (10.69.176.80) by DC5-EXCH01.marvell.com (10.69.176.38) with Microsoft SMTP Server id 15.0.1497.2 via Frontend Transport; Fri, 12 Aug 2022 01:58:40 -0700 Received: from localhost.localdomain (unknown [10.28.36.175]) by maili.marvell.com (Postfix) with ESMTP id 623043F7058; Fri, 12 Aug 2022 01:58:37 -0700 (PDT) From: Srujana Challa To: , , CC: , , , , Subject: [PATCH] security: remove get_userdata function pointer Date: Fri, 12 Aug 2022 14:28:36 +0530 Message-ID: <20220812085836.3174870-1-schalla@marvell.com> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 X-Proofpoint-ORIG-GUID: CcK8m4NuEeE0I-FcPn_eSSatSXfsl0RT X-Proofpoint-GUID: CcK8m4NuEeE0I-FcPn_eSSatSXfsl0RT X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.883,Hydra:6.0.517,FMLib:17.11.122.1 definitions=2022-08-12_06,2022-08-11_01,2022-06-22_01 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 Removes get_userdata function pointer as it is being unused and make fast accessing method that uses dynamic field to get userdata as default for optimizing security path. Signed-off-by: Srujana Challa Acked-by: Akhil Goyal --- app/test/test_security.c | 166 ----------------------- doc/guides/nics/features.rst | 2 +- doc/guides/prog_guide/rte_security.rst | 5 - drivers/crypto/cnxk/cnxk_cryptodev_sec.c | 1 - drivers/net/cnxk/cnxk_ethdev.c | 3 +- drivers/net/iavf/iavf_ipsec_crypto.c | 1 - examples/ipsec-secgw/ipsec-secgw.c | 32 +---- examples/ipsec-secgw/ipsec_worker.h | 6 +- lib/security/rte_security.c | 16 --- lib/security/rte_security.h | 43 +----- lib/security/rte_security_driver.h | 18 --- lib/security/version.map | 1 - 12 files changed, 11 insertions(+), 283 deletions(-) diff --git a/app/test/test_security.c b/app/test/test_security.c index 059731b65d..23f3f09254 100644 --- a/app/test/test_security.c +++ b/app/test/test_security.c @@ -438,43 +438,6 @@ mock_set_pkt_metadata(void *device, return mock_set_pkt_metadata_exp.ret; } -/** - * get_userdata mockup - * - * Verified parameters: device, md. - * The userdata parameter works as an output parameter, so a passed address - * is verified not to be NULL and filled with userdata stored in structure. - */ -static struct mock_get_userdata_data { - void *device; - uint64_t md; - void *userdata; - - int ret; - - int called; - int failed; -} mock_get_userdata_exp = {NULL, 0UL, NULL, 0, 0, 0}; - -static int -mock_get_userdata(void *device, - uint64_t md, - void **userdata) -{ - mock_get_userdata_exp.called++; - - MOCK_TEST_ASSERT_POINTER_PARAMETER(mock_get_userdata_exp, device); - MOCK_TEST_ASSERT_U64_PARAMETER(mock_get_userdata_exp, md); - - MOCK_TEST_ASSERT_NOT_NULL(mock_get_userdata_exp.failed, - userdata, - "Expecting parameter userdata not to be NULL but it's %p", - userdata); - *userdata = mock_get_userdata_exp.userdata; - - return mock_get_userdata_exp.ret; -} - /** * capabilities_get mockup * @@ -518,7 +481,6 @@ struct rte_security_ops mock_ops = { .session_stats_get = mock_session_stats_get, .session_destroy = mock_session_destroy, .set_pkt_metadata = mock_set_pkt_metadata, - .get_userdata = mock_get_userdata, .capabilities_get = mock_capabilities_get, }; @@ -638,7 +600,6 @@ ut_setup(void) mock_session_stats_get_exp.called = 0; mock_session_destroy_exp.called = 0; mock_set_pkt_metadata_exp.called = 0; - mock_get_userdata_exp.called = 0; mock_capabilities_get_exp.called = 0; mock_session_create_exp.failed = 0; @@ -647,7 +608,6 @@ ut_setup(void) mock_session_stats_get_exp.failed = 0; mock_session_destroy_exp.failed = 0; mock_set_pkt_metadata_exp.failed = 0; - mock_get_userdata_exp.failed = 0; mock_capabilities_get_exp.failed = 0; return TEST_SUCCESS; @@ -1688,121 +1648,6 @@ test_set_pkt_metadata_success(void) return TEST_SUCCESS; } - -/** - * rte_security_get_userdata tests - */ - -/** - * Test execution of rte_security_get_userdata with NULL instance - */ -static int -test_get_userdata_inv_context(void) -{ -#ifdef RTE_DEBUG - uint64_t md = 0xDEADBEEF; - - void *ret = rte_security_get_userdata(NULL, md); - TEST_ASSERT_MOCK_FUNCTION_CALL_RET(rte_security_get_userdata, - ret, NULL, "%p"); - TEST_ASSERT_MOCK_CALLS(mock_get_userdata_exp, 0); - - return TEST_SUCCESS; -#else - return TEST_SKIPPED; -#endif -} - -/** - * Test execution of rte_security_get_userdata with invalid - * security operations structure (NULL) - */ -static int -test_get_userdata_inv_context_ops(void) -{ -#ifdef RTE_DEBUG - struct security_unittest_params *ut_params = &unittest_params; - uint64_t md = 0xDEADBEEF; - ut_params->ctx.ops = NULL; - - void *ret = rte_security_get_userdata(&ut_params->ctx, md); - TEST_ASSERT_MOCK_FUNCTION_CALL_RET(rte_security_get_userdata, - ret, NULL, "%p"); - TEST_ASSERT_MOCK_CALLS(mock_get_userdata_exp, 0); - - return TEST_SUCCESS; -#else - return TEST_SKIPPED; -#endif -} - -/** - * Test execution of rte_security_get_userdata with empty - * security operations - */ -static int -test_get_userdata_inv_context_ops_fun(void) -{ - struct security_unittest_params *ut_params = &unittest_params; - uint64_t md = 0xDEADBEEF; - ut_params->ctx.ops = &empty_ops; - - void *ret = rte_security_get_userdata(&ut_params->ctx, md); - TEST_ASSERT_MOCK_FUNCTION_CALL_RET(rte_security_get_userdata, - ret, NULL, "%p"); - TEST_ASSERT_MOCK_CALLS(mock_get_userdata_exp, 0); - - return TEST_SUCCESS; -} - -/** - * Test execution of rte_security_get_userdata when get_userdata - * security operation fails - */ -static int -test_get_userdata_ops_failure(void) -{ - struct security_unittest_params *ut_params = &unittest_params; - uint64_t md = 0xDEADBEEF; - void *userdata = (void *)0x7E577E57; - - mock_get_userdata_exp.device = NULL; - mock_get_userdata_exp.md = md; - mock_get_userdata_exp.userdata = userdata; - mock_get_userdata_exp.ret = -1; - - void *ret = rte_security_get_userdata(&ut_params->ctx, md); - TEST_ASSERT_MOCK_FUNCTION_CALL_RET(rte_security_get_userdata, - ret, NULL, "%p"); - TEST_ASSERT_MOCK_CALLS(mock_get_userdata_exp, 1); - - return TEST_SUCCESS; -} - -/** - * Test execution of rte_security_get_userdata in successful execution path - */ -static int -test_get_userdata_success(void) -{ - struct security_unittest_params *ut_params = &unittest_params; - uint64_t md = 0xDEADBEEF; - void *userdata = (void *)0x7E577E57; - - mock_get_userdata_exp.device = NULL; - mock_get_userdata_exp.md = md; - mock_get_userdata_exp.userdata = userdata; - mock_get_userdata_exp.ret = 0; - - void *ret = rte_security_get_userdata(&ut_params->ctx, md); - TEST_ASSERT_MOCK_FUNCTION_CALL_RET(rte_security_get_userdata, - ret, userdata, "%p"); - TEST_ASSERT_MOCK_CALLS(mock_get_userdata_exp, 1); - - return TEST_SUCCESS; -} - - /** * rte_security_capabilities_get tests */ @@ -2569,17 +2414,6 @@ static struct unit_test_suite security_testsuite = { TEST_CASE_ST(ut_setup_with_session, ut_teardown, test_set_pkt_metadata_success), - TEST_CASE_ST(ut_setup_with_session, ut_teardown, - test_get_userdata_inv_context), - TEST_CASE_ST(ut_setup_with_session, ut_teardown, - test_get_userdata_inv_context_ops), - TEST_CASE_ST(ut_setup_with_session, ut_teardown, - test_get_userdata_inv_context_ops_fun), - TEST_CASE_ST(ut_setup_with_session, ut_teardown, - test_get_userdata_ops_failure), - TEST_CASE_ST(ut_setup_with_session, ut_teardown, - test_get_userdata_success), - TEST_CASE_ST(ut_setup_with_session, ut_teardown, test_capabilities_get_inv_context), TEST_CASE_ST(ut_setup_with_session, ut_teardown, diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst index 7f6cb914a5..56812c86bb 100644 --- a/doc/guides/nics/features.rst +++ b/doc/guides/nics/features.rst @@ -433,7 +433,7 @@ protocol operations. See security library and PMD documentation for more details * **[uses] rte_eth_txconf,rte_eth_txmode**: ``offloads:RTE_ETH_TX_OFFLOAD_SECURITY``. * **[uses] mbuf**: ``mbuf.l2_len``. * **[implements] rte_security_ops**: ``session_create``, ``session_update``, - ``session_stats_get``, ``session_destroy``, ``set_pkt_metadata``, ``get_userdata``, + ``session_stats_get``, ``session_destroy``, ``set_pkt_metadata``, ``capabilities_get``. * **[provides] rte_eth_dev_info**: ``rx_offload_capa,rx_queue_offload_capa:RTE_ETH_RX_OFFLOAD_SECURITY``, ``tx_offload_capa,tx_queue_offload_capa:RTE_ETH_TX_OFFLOAD_SECURITY``. diff --git a/doc/guides/prog_guide/rte_security.rst b/doc/guides/prog_guide/rte_security.rst index 72ca0bd330..cd31948067 100644 --- a/doc/guides/prog_guide/rte_security.rst +++ b/doc/guides/prog_guide/rte_security.rst @@ -571,11 +571,6 @@ For Inline Crypto and Inline protocol offload, device specific defined metadata updated in the mbuf using ``rte_security_set_pkt_metadata()`` if ``RTE_ETH_TX_OFFLOAD_SEC_NEED_MDATA`` is set. -For inline protocol offloaded ingress traffic, the application can register a -pointer, ``userdata`` , in the security session. When the packet is received, -``rte_security_get_userdata()`` would return the userdata registered for the -security session which processed the packet. - .. note:: In case of inline processed packets, ``RTE_SECURITY_DYNFIELD_NAME`` field diff --git a/drivers/crypto/cnxk/cnxk_cryptodev_sec.c b/drivers/crypto/cnxk/cnxk_cryptodev_sec.c index e5a5d2dddd..93553644f0 100644 --- a/drivers/crypto/cnxk/cnxk_cryptodev_sec.c +++ b/drivers/crypto/cnxk/cnxk_cryptodev_sec.c @@ -17,7 +17,6 @@ struct rte_security_ops cnxk_sec_ops = { .session_get_size = NULL, .session_stats_get = NULL, .set_pkt_metadata = NULL, - .get_userdata = NULL, .capabilities_get = cnxk_crypto_sec_capabilities_get }; diff --git a/drivers/net/cnxk/cnxk_ethdev.c b/drivers/net/cnxk/cnxk_ethdev.c index 24182909f1..a8a7ecfb69 100644 --- a/drivers/net/cnxk/cnxk_ethdev.c +++ b/drivers/net/cnxk/cnxk_ethdev.c @@ -1684,8 +1684,7 @@ cnxk_eth_dev_init(struct rte_eth_dev *eth_dev) return -ENOMEM; sec_ctx->device = eth_dev; sec_ctx->ops = &cnxk_eth_sec_ops; - sec_ctx->flags = - (RTE_SEC_CTX_F_FAST_SET_MDATA | RTE_SEC_CTX_F_FAST_GET_UDATA); + sec_ctx->flags = RTE_SEC_CTX_F_FAST_SET_MDATA; eth_dev->security_ctx = sec_ctx; /* For secondary processes, the primary has done all the work */ diff --git a/drivers/net/iavf/iavf_ipsec_crypto.c b/drivers/net/iavf/iavf_ipsec_crypto.c index 75f05ee558..02fdcc8b5f 100644 --- a/drivers/net/iavf/iavf_ipsec_crypto.c +++ b/drivers/net/iavf/iavf_ipsec_crypto.c @@ -1481,7 +1481,6 @@ static struct rte_security_ops iavf_ipsec_crypto_ops = { .session_stats_get = iavf_ipsec_crypto_session_stats_get, .session_destroy = iavf_ipsec_crypto_session_destroy, .set_pkt_metadata = iavf_ipsec_crypto_pkt_metadata_set, - .get_userdata = NULL, .capabilities_get = iavf_ipsec_crypto_capabilities_get, }; diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c index 815b9254ae..f30694a380 100644 --- a/examples/ipsec-secgw/ipsec-secgw.c +++ b/examples/ipsec-secgw/ipsec-secgw.c @@ -2188,39 +2188,15 @@ pool_init(struct socket_ctx *ctx, int32_t socket_id, int portid, printf("Allocated mbuf pool on socket %d\n", socket_id); } -static inline int -inline_ipsec_event_esn_overflow(struct rte_security_ctx *ctx, uint64_t md) -{ - struct ipsec_sa *sa; - - /* For inline protocol processing, the metadata in the event will - * uniquely identify the security session which raised the event. - * Application would then need the userdata it had registered with the - * security session to process the event. - */ - - sa = (struct ipsec_sa *)rte_security_get_userdata(ctx, md); - - if (sa == NULL) { - /* userdata could not be retrieved */ - return -1; - } - - /* Sequence number over flow. SA need to be re-established */ - RTE_SET_USED(sa); - return 0; -} - static int inline_ipsec_event_callback(uint16_t port_id, enum rte_eth_event_type type, void *param, void *ret_param) { uint64_t md; struct rte_eth_event_ipsec_desc *event_desc = NULL; - struct rte_security_ctx *ctx = (struct rte_security_ctx *) - rte_eth_dev_get_sec_ctx(port_id); RTE_SET_USED(param); + RTE_SET_USED(port_id); if (type != RTE_ETH_EVENT_IPSEC) return -1; @@ -2233,8 +2209,10 @@ inline_ipsec_event_callback(uint16_t port_id, enum rte_eth_event_type type, md = event_desc->metadata; - if (event_desc->subtype == RTE_ETH_EVENT_IPSEC_ESN_OVERFLOW) - return inline_ipsec_event_esn_overflow(ctx, md); + if (event_desc->subtype == RTE_ETH_EVENT_IPSEC_ESN_OVERFLOW) { + if (md == 0) + return -1; + } else if (event_desc->subtype >= RTE_ETH_EVENT_IPSEC_MAX) { printf("Invalid IPsec event reported\n"); return -1; diff --git a/examples/ipsec-secgw/ipsec_worker.h b/examples/ipsec-secgw/ipsec_worker.h index 315f3d67c7..33722a4ed3 100644 --- a/examples/ipsec-secgw/ipsec_worker.h +++ b/examples/ipsec-secgw/ipsec_worker.h @@ -212,11 +212,7 @@ prepare_one_packet(struct rte_security_ctx *ctx, struct rte_mbuf *pkt, struct ipsec_sa *sa; struct ipsec_mbuf_metadata *priv; - /* Retrieve the userdata registered. Here, the userdata - * registered is the SA pointer. - */ - sa = (struct ipsec_sa *)rte_security_get_userdata(ctx, - *rte_security_dynfield(pkt)); + sa = *(struct ipsec_sa **)rte_security_dynfield(pkt); if (sa == NULL) { /* userdata could not be retrieved */ return; diff --git a/lib/security/rte_security.c b/lib/security/rte_security.c index 4f5e4b4d49..74fa513861 100644 --- a/lib/security/rte_security.c +++ b/lib/security/rte_security.c @@ -136,22 +136,6 @@ __rte_security_set_pkt_metadata(struct rte_security_ctx *instance, sess, m, params); } -void * -__rte_security_get_userdata(struct rte_security_ctx *instance, uint64_t md) -{ - void *userdata = NULL; - -#ifdef RTE_DEBUG - RTE_PTR_OR_ERR_RET(instance, NULL); - RTE_PTR_OR_ERR_RET(instance->ops, NULL); -#endif - RTE_FUNC_PTR_OR_ERR_RET(*instance->ops->get_userdata, NULL); - if (instance->ops->get_userdata(instance->device, md, &userdata)) - return NULL; - - return userdata; -} - const struct rte_security_capability * rte_security_capabilities_get(struct rte_security_ctx *instance) { diff --git a/lib/security/rte_security.h b/lib/security/rte_security.h index 675db940eb..3e8cd29082 100644 --- a/lib/security/rte_security.h +++ b/lib/security/rte_security.h @@ -78,12 +78,9 @@ struct rte_security_ctx { }; #define RTE_SEC_CTX_F_FAST_SET_MDATA 0x00000001 -/**< Driver uses fast metadata update without using driver specific callback */ - -#define RTE_SEC_CTX_F_FAST_GET_UDATA 0x00000002 -/**< Driver provides udata using fast method without using driver specific - * callback. For fast mdata and udata, mbuf dynamic field would be registered - * by driver via rte_security_dynfield_register(). +/**< Driver uses fast metadata update without using driver specific callback. + * For fast mdata, mbuf dynamic field would be registered by driver + * via rte_security_dynfield_register(). */ /** @@ -664,40 +661,6 @@ rte_security_set_pkt_metadata(struct rte_security_ctx *instance, return __rte_security_set_pkt_metadata(instance, sess, mb, params); } -/** Function to call PMD specific function pointer get_userdata() */ -__rte_experimental -extern void *__rte_security_get_userdata(struct rte_security_ctx *instance, - uint64_t md); - -/** - * Get userdata associated with the security session. Device specific metadata - * provided would be used to uniquely identify the security session being - * referred to. This userdata would be registered while creating the session, - * and application can use this to identify the SA etc. - * - * Device specific metadata would be set in mbuf for inline processed inbound - * packets. In addition, the same metadata would be set for IPsec events - * reported by rte_eth_event framework. - * - * @param instance security instance - * @param md device-specific metadata - * - * @return - * - On success, userdata - * - On failure, NULL - */ -__rte_experimental -static inline void * -rte_security_get_userdata(struct rte_security_ctx *instance, uint64_t md) -{ - /* Fast Path */ - if (instance->flags & RTE_SEC_CTX_F_FAST_GET_UDATA) - return (void *)(uintptr_t)md; - - /* Jump to PMD specific function pointer */ - return __rte_security_get_userdata(instance, md); -} - /** * Attach a session to a symmetric crypto operation * diff --git a/lib/security/rte_security_driver.h b/lib/security/rte_security_driver.h index b0253e962e..0063a66524 100644 --- a/lib/security/rte_security_driver.h +++ b/lib/security/rte_security_driver.h @@ -108,22 +108,6 @@ typedef int (*security_set_pkt_metadata_t)(void *device, struct rte_security_session *sess, struct rte_mbuf *mb, void *params); -/** - * Get application specific userdata associated with the security session. - * Device specific metadata provided would be used to uniquely identify - * the security session being referred to. - * - * @param device Crypto/eth device pointer - * @param md Metadata - * @param userdata Pointer to receive userdata - * - * @return - * - Returns 0 if userdata is retrieved successfully. - * - Returns -ve value for errors. - */ -typedef int (*security_get_userdata_t)(void *device, - uint64_t md, void **userdata); - /** * Get security capabilities of the device. * @@ -150,8 +134,6 @@ struct rte_security_ops { /**< Clear a security sessions private data. */ security_set_pkt_metadata_t set_pkt_metadata; /**< Update mbuf metadata. */ - security_get_userdata_t get_userdata; - /**< Get userdata associated with session which processed the packet. */ security_capabilities_get_t capabilities_get; /**< Get security capabilities. */ }; diff --git a/lib/security/version.map b/lib/security/version.map index c770b2e8f8..85ca7921e7 100644 --- a/lib/security/version.map +++ b/lib/security/version.map @@ -13,7 +13,6 @@ DPDK_23 { EXPERIMENTAL { global: - __rte_security_get_userdata; __rte_security_set_pkt_metadata; rte_security_dynfield_offset; rte_security_session_stats_get;