security: remove get_userdata function pointer

Message ID 20220812085836.3174870-1-schalla@marvell.com (mailing list archive)
State Accepted, archived
Delegated to: akhil goyal
Headers
Series security: remove get_userdata function pointer |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/github-robot: build success github build: passed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/intel-Testing success Testing PASS

Commit Message

Srujana Challa Aug. 12, 2022, 8:58 a.m. UTC
  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 <schalla@marvell.com>
---
 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(-)
  

Comments

Akhil Goyal Aug. 25, 2022, 8:04 a.m. UTC | #1
> Subject: [PATCH] security: remove get_userdata function pointer
> 
> 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 <schalla@marvell.com>
Acked-by: Akhil Goyal <gakhil@marvell.com>
  
Akhil Goyal Sept. 27, 2022, 7 p.m. UTC | #2
> Subject: [PATCH] security: remove get_userdata function pointer
> 
> 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 <schalla@marvell.com>
Applied to dpdk-next-crypto

Thanks.
  
Akhil Goyal Sept. 29, 2022, 4:49 a.m. UTC | #3
> > Subject: [PATCH] security: remove get_userdata function pointer
> >
> > 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 <schalla@marvell.com>
> Applied to dpdk-next-crypto
> 
Release notes added for API changes introduced in this patch.
Updated patch title and description as well.
  

Patch

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;