[PATCH 0/2] eal: notify secondaries on primary exit

[PATCH 0/2] eal: notify secondaries on primary exit

[Stephen Hemminger]

Bugzilla 1942: when a primary process exits cleanly, secondary
processes other than testpmd do not get notified.  The notification
mechanism added in 25.11 was placed in testpmd and used
rte_mp_request_sync() with a testpmd-specific action name, so any
non-testpmd secondary (dpdk-dumpcap, dpdk-pdump, dpdk-procinfo, or
out-of-tree consumers) would log "Cannot find action: mp_testpmd"
and the primary would block on the 5 second request timeout.

Putting application-specific IPC actions on a broadcast request path
is the wrong layer.  Notification of primary exit is something every
secondary needs and should come from EAL, not from each application.

Patch 1 reverts the testpmd-side mechanism (commit f96273c8e9d3).
The secondary-side primary alive monitor (enable_primary_monitor)
is preserved and continues to handle detection of primary exit via
the existing alarm-based polling of rte_eal_primary_proc_alive().

Patch 2 adds a generic EAL-level notification.  On primary cleanup,
rte_mp_channel_cleanup() broadcasts an MP_REQ_QUIT message to all
known secondaries via rte_mp_sendmsg().  Secondaries register an
internal action handler that tears down their own MP channel on
receipt.  No new public API; no application changes required for
any secondary, in-tree or out.

This is the minimum fix suitable for backport to 25.11 stable.
It addresses the clean exit case.  The crash case (primary killed
or signaled) continues to be handled by the existing
rte_eal_primary_proc_alive() polling on the secondary side, which
detects the primary's release of the config file lock.

A more complete solution using a connected socket type (SOCK_SEQPACKET)
is planned since that can handle both planned and forced exiting
of the primary.

Tested with testpmd (primary) and dpdk-dumpcap, dpdk-pdump,
dpdk-procinfo (secondaries).


Stephen Hemminger (2):
  Revert "app/testpmd: stop forwarding in secondary process"
  eal: notify secondary on primary exit

 app/test-pmd/testpmd.c           | 103 ++-----------------------------
 lib/eal/common/eal_common_proc.c |  51 ++++++++++++++-
 2 files changed, 53 insertions(+), 101 deletions(-)

-- 
2.53.0

[PATCH 1/2] Revert "app/testpmd: stop forwarding in secondary process"

[Stephen Hemminger]

Testpmd must not assume that the secondary process
is testpmd. The notification needs to be done by EAL.

This reverts commit f96273c8e9d39e472bb07acc05e493b1e712e51b.

Bugzilla ID: 1942
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test-pmd/testpmd.c | 103 ++---------------------------------------
 1 file changed, 4 insertions(+), 99 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index e2569d9e30..e54a3981e9 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -3716,83 +3716,6 @@ detach_devargs(char *identifier)
 	rte_devargs_reset(&da);
 }
 
-#ifndef RTE_EXEC_ENV_WINDOWS
-
-enum testpmd_req_type {
-	TESTPMD_REQ_TYPE_EXIT,
-};
-
-struct testpmd_mp_req {
-	enum testpmd_req_type t;
-};
-
-struct testpmd_mp_resp {
-	int result;
-};
-
-#define TESTPMD_MP	"mp_testpmd"
-
-/* Send reply to this peer when testpmd exits */
-static RTE_ATOMIC(const char *) primary_name;
-
-static void
-reply_to_primary(const char *peer, int result)
-{
-	struct rte_mp_msg reply = { };
-	struct testpmd_mp_resp *resp = (struct testpmd_mp_resp *) &reply.param;
-
-	strlcpy(reply.name, TESTPMD_MP, RTE_MP_MAX_NAME_LEN);
-	reply.len_param = sizeof(*resp);
-	resp->result = result;
-
-	printf("Replying %d to primary\n", result);
-	fflush(stdout);
-
-	if (rte_mp_reply(&reply, peer) < 0)
-		printf("Failed to send response to primary:%s", strerror(rte_errno));
-}
-
-/* Primary process is exiting, stop secondary process */
-static void
-pmd_notify_secondary(void)
-{
-	struct testpmd_mp_req request = {
-		.t = TESTPMD_REQ_TYPE_EXIT,
-	};
-	struct rte_mp_msg mp_req = {
-		.name = TESTPMD_MP,
-		.len_param = sizeof(request),
-	};
-	struct rte_mp_reply reply;
-	struct timespec ts = {.tv_sec = 5, .tv_nsec = 0};
-
-	printf("\nPrimary: Sending 'stop_req' request to secondary...\n");
-	fflush(stdout);
-
-	memcpy(mp_req.param, &request, sizeof(request));
-	rte_mp_request_sync(&mp_req, &reply, &ts);
-}
-
-static int
-handle_testpmd_request(const struct rte_mp_msg *request, const void *peer)
-{
-	const struct testpmd_mp_req *req = (const struct testpmd_mp_req *)request->param;
-
-	if (req->t == TESTPMD_REQ_TYPE_EXIT) {
-		printf("\nReceived notification of primary exiting\n");
-		fflush(stdout);
-
-		/* Response is sent after forwarding loop exits */
-		rte_atomic_store_explicit(&primary_name, peer, rte_memory_order_relaxed);
-
-		kill(getpid(), SIGINT);
-	} else {
-		reply_to_primary(peer, -EINVAL);
-	}
-	return 0;
-}
-#endif
-
 void
 pmd_test_exit(void)
 {
@@ -3804,10 +3727,6 @@ pmd_test_exit(void)
 		stop_packet_forwarding();
 
 #ifndef RTE_EXEC_ENV_WINDOWS
-	/* Tell secondary to exit */
-	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
-		pmd_notify_secondary();
-
 	for (i = 0 ; i < RTE_DIM(mempools) ; i++) {
 		if (mempools[i]) {
 			if (mp_alloc_type == MP_ALLOC_ANON)
@@ -4623,12 +4542,9 @@ main(int argc, char** argv)
 			 rte_strerror(rte_errno));
 
 #ifndef RTE_EXEC_ENV_WINDOWS
-	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
-		if (enable_primary_monitor() < 0)
-			rte_exit(EXIT_FAILURE, "Cannot setup primary monitor");
-		if (rte_mp_action_register(TESTPMD_MP, handle_testpmd_request) < 0)
-			rte_exit(EXIT_FAILURE, "Failed to register message action\n");
-	}
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
+	    enable_primary_monitor() < 0)
+		rte_exit(EXIT_FAILURE, "Cannot setup primary monitor");
 #endif
 
 	/* allocate port structures, and init them */
@@ -4830,23 +4746,12 @@ main(int argc, char** argv)
 	}
 
 #ifndef RTE_EXEC_ENV_WINDOWS
-	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
 		disable_primary_monitor();
-		rte_mp_action_unregister(TESTPMD_MP);
-	}
 #endif
 
 	pmd_test_exit();
 
-#ifndef RTE_EXEC_ENV_WINDOWS
-	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
-		const char *peer = rte_atomic_exchange_explicit(&primary_name, NULL,
-				rte_memory_order_relaxed);
-		if (peer)
-			reply_to_primary(peer, 0);
-	}
-#endif
-
 #ifdef RTE_LIB_PDUMP
 	/* uninitialize packet capture framework */
 	rte_pdump_uninit();
-- 
2.53.0

[PATCH 2/2] eal: notify secondary on primary exit

[Stephen Hemminger]

When primary process exits, it should notify the secondary
processes. This allows for clean shutdown and eliminates need
for MP handling in applications (such as test-pmd).

Bugzilla ID: 1942
Fixes: 85d6815fa6d0 ("eal: close multi-process socket during cleanup")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/common/eal_common_proc.c | 51 ++++++++++++++++++++++++++++++--
 1 file changed, 49 insertions(+), 2 deletions(-)

diff --git a/lib/eal/common/eal_common_proc.c b/lib/eal/common/eal_common_proc.c
index 06f151818c..4369892e44 100644
--- a/lib/eal/common/eal_common_proc.c
+++ b/lib/eal/common/eal_common_proc.c
@@ -60,6 +60,15 @@ enum mp_type {
 	MP_IGN, /* Response telling requester to ignore this response */
 };
 
+/* Message sent from primary EAL to secondary EAL */
+#define EAL_MP "mp_eal"
+struct eal_mp_req {
+	enum {
+		MP_REQ_NOP = 0,
+		MP_REQ_QUIT,
+	} type;
+};
+
 struct mp_msg_internal {
 	int type;
 	struct rte_mp_msg msg;
@@ -614,6 +623,25 @@ close_socket_fd(int fd)
 		unlink(path);
 }
 
+/* callback in secondary when primary has exited */
+static int
+mp_primary_exited(const struct rte_mp_msg *msg, const void *peer __rte_unused)
+{
+	const struct eal_mp_req *req;
+
+	if (msg->len_param != sizeof(*req)) {
+		EAL_LOG(ERR, "invalid request from primary");
+		return -EINVAL;
+	}
+
+	req = (const struct eal_mp_req *)msg->param;
+	if (req->type == MP_REQ_QUIT)
+		rte_mp_channel_cleanup();
+
+	/* no reply needed */
+	return 0;
+}
+
 int
 rte_mp_channel_init(void)
 {
@@ -661,6 +689,10 @@ rte_mp_channel_init(void)
 		return -1;
 	}
 
+	/* listen for quit message from primary */
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
+		rte_mp_action_register(EAL_MP, mp_primary_exited);
+
 	if (rte_thread_create_internal_control(&mp_handle_tid, "mp-msg",
 			mp_handle, NULL) < 0) {
 		EAL_LOG(ERR, "failed to create mp thread: %s",
@@ -682,12 +714,27 @@ rte_mp_channel_cleanup(void)
 {
 	int fd;
 
+	/* Primary exiting, tell all secondary processes */
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+		struct rte_mp_msg msg = { .name = EAL_MP };
+		struct eal_mp_req req = { .type = MP_REQ_QUIT };
+
+		memcpy(&msg.param, &req, sizeof(req));
+		msg.len_param = sizeof(req);
+
+		rte_mp_sendmsg(&msg);
+	} else {
+		rte_mp_action_unregister(EAL_MP);
+	}
+
 	fd = rte_atomic_exchange_explicit(&mp_fd, -1, rte_memory_order_relaxed);
 	if (fd < 0)
 		return;
 
-	pthread_cancel((pthread_t)mp_handle_tid.opaque_id);
-	rte_thread_join(mp_handle_tid, NULL);
+	if (pthread_self() != (pthread_t)mp_handle_tid.opaque_id) {
+		pthread_cancel((pthread_t)mp_handle_tid.opaque_id);
+		rte_thread_join(mp_handle_tid, NULL);
+	}
 	close_socket_fd(fd);
 }
 
-- 
2.53.0