ethdev: refresh shared memory reference in secondary process

Message ID 20231020120629.2158219-1-david.marchand@redhat.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series ethdev: refresh shared memory reference in secondary process |

Checks

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

Commit Message

David Marchand Oct. 20, 2023, 12:06 p.m. UTC
  In the process of releasing ethdev port in the primary process,
secondary processes are asked to stop referencing such a port.
Doing so, those processes can't predict whether the primary process
will later invalidate/free the shared memory. So they may live with a
reference to an old shared memory memzone and location in memory.

Refresh the shared memory pointer in secondary process when requesting
access to the shared memory: this is the best moment ethdev can check
it because the primary process will (re-)create a shared memory only if
no secondary process is referencing a ethdev port.

Bugzilla ID: 1303
Fixes: 36c46e738120 ("ethdev: cleanup shared data with the last port")

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/ethdev/ethdev_private.c | 46 ++++++++++++++++++++++++-------------
 1 file changed, 30 insertions(+), 16 deletions(-)
  

Comments

Ferruh Yigit Oct. 26, 2023, 1:11 p.m. UTC | #1
On 10/20/2023 1:06 PM, David Marchand wrote:
> In the process of releasing ethdev port in the primary process,
> secondary processes are asked to stop referencing such a port.
> Doing so, those processes can't predict whether the primary process
> will later invalidate/free the shared memory. So they may live with a
> reference to an old shared memory memzone and location in memory.
> 
> Refresh the shared memory pointer in secondary process when requesting
> access to the shared memory: this is the best moment ethdev can check
> it because the primary process will (re-)create a shared memory only if
> no secondary process is referencing a ethdev port.
> 
> Bugzilla ID: 1303
> Fixes: 36c46e738120 ("ethdev: cleanup shared data with the last port")
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
>

Reviewed-by: Ferruh Yigit <ferruh.yigit@amd.com>

Applied to dpdk-next-net/main, thanks.
  

Patch

diff --git a/lib/ethdev/ethdev_private.c b/lib/ethdev/ethdev_private.c
index 7cc7f28296..a78a66bbfd 100644
--- a/lib/ethdev/ethdev_private.c
+++ b/lib/ethdev/ethdev_private.c
@@ -324,17 +324,18 @@  rte_eth_call_tx_callbacks(uint16_t port_id, uint16_t queue_id,
 void *
 eth_dev_shared_data_prepare(void)
 {
-	const unsigned int flags = 0;
 	const struct rte_memzone *mz;
 
-	if (eth_dev_shared_mz == NULL) {
-		if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
-			/* Allocate port data and ownership shared memory. */
-			mz = rte_memzone_reserve(MZ_RTE_ETH_DEV_DATA,
-					sizeof(*eth_dev_shared_data),
-					rte_socket_id(), flags);
-		} else
-			mz = rte_memzone_lookup(MZ_RTE_ETH_DEV_DATA);
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+		const unsigned int flags = 0;
+
+		if (eth_dev_shared_mz != NULL)
+			goto out;
+
+		/* Allocate port data and ownership shared memory. */
+		mz = rte_memzone_reserve(MZ_RTE_ETH_DEV_DATA,
+				sizeof(*eth_dev_shared_data),
+				rte_socket_id(), flags);
 		if (mz == NULL) {
 			RTE_ETHDEV_LOG(ERR, "Cannot allocate ethdev shared data\n");
 			goto out;
@@ -342,14 +343,27 @@  eth_dev_shared_data_prepare(void)
 
 		eth_dev_shared_mz = mz;
 		eth_dev_shared_data = mz->addr;
-		if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
-			eth_dev_shared_data->allocated_owners = 0;
-			eth_dev_shared_data->next_owner_id =
-					RTE_ETH_DEV_NO_OWNER + 1;
-			eth_dev_shared_data->allocated_ports = 0;
-			memset(eth_dev_shared_data->data, 0,
-			       sizeof(eth_dev_shared_data->data));
+		eth_dev_shared_data->allocated_owners = 0;
+		eth_dev_shared_data->next_owner_id =
+			RTE_ETH_DEV_NO_OWNER + 1;
+		eth_dev_shared_data->allocated_ports = 0;
+		memset(eth_dev_shared_data->data, 0,
+		       sizeof(eth_dev_shared_data->data));
+	} else {
+		mz = rte_memzone_lookup(MZ_RTE_ETH_DEV_DATA);
+		if (mz == NULL) {
+			/* Clean remaining any traces of a previous shared mem */
+			eth_dev_shared_mz = NULL;
+			eth_dev_shared_data = NULL;
+			RTE_ETHDEV_LOG(ERR, "Cannot lookup ethdev shared data\n");
+			goto out;
 		}
+		if (mz == eth_dev_shared_mz && mz->addr == eth_dev_shared_data)
+			goto out;
+
+		/* Shared mem changed in primary process, refresh pointers */
+		eth_dev_shared_mz = mz;
+		eth_dev_shared_data = mz->addr;
 	}
 out:
 	return eth_dev_shared_data;