mctpd: Deadline-based recovery

This PR replaces individual recovery timers with u64 deadlines instead.
Add a global time entrypoint that triggers the deadlines.

Signed-off-by: Khang D Nguyen <[email protected]>

Author: khangng-ampere

src/mctp-ops.c

@@ -7,6 +7,8 @@
 
 #define _GNU_SOURCE
 
+#include <sys/time.h>
+#include <sys/timerfd.h>
 #include <unistd.h>
 #include <linux/netlink.h>
 #include <err.h>
@@ -57,6 +59,60 @@ static void mctp_bug_warn(const char *fmt, va_list args)
 	vwarnx(fmt, args);
 }
 
+static inline struct itimerspec itimespec_from_usec(uint64_t u)
+{
+	return (struct itimerspec){
+		.it_interval = { 0 },
+		.it_value.tv_sec = u / 1000000,
+		.it_value.tv_nsec = (u % 1000000) * 1000,
+	};
+}
+static inline uint64_t usec_from_itimespec(struct itimerspec spec)
+{
+	return (spec.it_value.tv_sec * 1000000) +
+	       (spec.it_value.tv_nsec / 1000);
+}
+
+static int mctp_op_timerfd_create()
+{
+	return timerfd_create(CLOCK_MONOTONIC, 0);
+}
+
+static int mctp_op_timerfd_settime(int fd, uint64_t new_value,
+				   uint64_t *old_value)
+{
+	struct itimerspec new = itimespec_from_usec(new_value);
+	struct itimerspec old;
+	int rc;
+
+	rc = timerfd_settime(fd, TFD_TIMER_ABSTIME, &new, &old);
+	if (rc < 0)
+		return rc;
+
+	*old_value = usec_from_itimespec(old);
+
+	return 0;
+}
+
+static int mctp_op_timerfd_gettime(int fd, uint64_t *curr_value)
+{
+	struct itimerspec spec;
+	int rc;
+
+	rc = timerfd_gettime(fd, &spec);
+	if (rc < 0)
+		return rc;
+
+	*curr_value = usec_from_itimespec(spec);
+
+	return 0;
+}
+
+static int mctp_op_timerfd_read(int fd, uint64_t *n)
+{
+	return read(fd, n, sizeof(*n));
+}
+
 const struct mctp_ops mctp_ops = {
 	.mctp = {
 		.socket = mctp_op_mctp_socket,
@@ -74,6 +130,12 @@ const struct mctp_ops mctp_ops = {
 		.recvfrom = mctp_op_recvfrom,
 		.close = mctp_op_close,
 	},
+	.timerfd = {
+        .create = mctp_op_timerfd_create,
+        .gettime = mctp_op_timerfd_gettime,
+        .settime = mctp_op_timerfd_settime,
+        .read = mctp_op_timerfd_read,
+	},
 	.bug_warn = mctp_bug_warn,
 };
 

src/mctp-ops.h

@@ -7,8 +7,10 @@
  */
 #pragma once
 
+#include <stdint.h>
 #include <sys/socket.h>
 #include <stdarg.h>
+#include <sys/timerfd.h>
 
 #define _GNU_SOURCE
 
@@ -24,9 +26,17 @@ struct socket_ops {
 	int (*close)(int sd);
 };
 
+struct timerfd_ops {
+	int (*create)();
+	int (*settime)(int fd, uint64_t new_value, uint64_t *old_value);
+	int (*gettime)(int fd, uint64_t *curr_value);
+	int (*read)(int fd, uint64_t *n);
+};
+
 struct mctp_ops {
 	struct socket_ops mctp;
 	struct socket_ops nl;
+	struct timerfd_ops timerfd;
 	void (*bug_warn)(const char *fmt, va_list args);
 };
 

src/mctpd.c

@@ -189,7 +189,7 @@ struct peer {
 	bool degraded;
 	struct {
 		uint64_t delay;
-		sd_event_source *source;
+		uint64_t deadline;
 		int npolls;
 		mctp_eid_t eid;
 		uint8_t endpoint_type;
@@ -248,6 +248,10 @@ struct ctx {
 
 	//  maximum pool size for assumed MCTP Bridge
 	uint8_t max_pool_size;
+
+	uint64_t now;
+	uint64_t next_deadline;
+	int timerfd;
 };
 
 static int emit_endpoint_added(const struct peer *peer);
@@ -1791,6 +1795,7 @@ static int add_peer(struct ctx *ctx, const dest_phys *dest, mctp_eid_t eid,
 	memcpy(&peer->phys, dest, sizeof(*dest));
 	peer->state = REMOTE;
 	peer->ctx = ctx;
+	peer->recovery.deadline = UINT64_MAX;
 
 	// Update network eid map
 	n->peers[eid] = peer;
@@ -1851,16 +1856,7 @@ static int remove_peer(struct peer *peer)
 
 	// Clear it
 	if (peer->degraded) {
-		int rc;
-
-		rc = sd_event_source_set_enabled(peer->recovery.source,
-						 SD_EVENT_OFF);
-		if (rc < 0) {
-			/* XXX: Fix caller assumptions? */
-			warnx("Failed to stop recovery timer while removing peer: %d",
-			      rc);
-		}
-		sd_event_source_unref(peer->recovery.source);
+		peer->recovery.deadline = UINT64_MAX;
 	}
 
 	n->peers[peer->eid] = NULL;
@@ -3079,10 +3075,19 @@ static int method_endpoint_remove(sd_bus_message *call, void *data,
 	(MCTP_I2C_TSYM_MT1_MAX_US + 2 * MCTP_I2C_TSYM_MT3_MAX_US)
 #define MCTP_I2C_TSYM_MT2_MAX_MS MCTP_I2C_TSYM_MT4_MIN_US
 
-static int peer_endpoint_recover(sd_event_source *s, uint64_t usec,
-				 void *userdata)
+static int update_deadline(struct ctx *ctx, uint64_t new_deadline)
+{
+	uint64_t prev_deadline;
+
+	if (ctx->next_deadline < new_deadline)
+		return 0;
+
+	return mctp_ops.timerfd.settime(ctx->timerfd, new_deadline,
+					&prev_deadline);
+}
+
+static int peer_endpoint_recover(struct peer *peer)
 {
-	struct peer *peer = userdata;
 	struct ctx *ctx = peer->ctx;
 	const char *peer_path;
 	int rc;
@@ -3145,8 +3150,7 @@ static int peer_endpoint_recover(sd_event_source *s, uint64_t usec,
 			/* It's not known to be the same device, allocate a new EID */
 			dest_phys phys = peer->phys;
 
-			assert(sd_event_source_get_enabled(
-				       peer->recovery.source, NULL) == 0);
+			peer->recovery.deadline = UINT64_MAX;
 			remove_peer(peer);
 			/*
 			 * The representation of the old peer is now gone. Set up the new peer,
@@ -3184,28 +3188,21 @@ static int peer_endpoint_recover(sd_event_source *s, uint64_t usec,
 		goto reschedule;
 	}
 
-	assert(sd_event_source_get_enabled(peer->recovery.source, NULL) == 0);
-	sd_event_source_unref(peer->recovery.source);
 	peer->recovery.delay = 0;
-	peer->recovery.source = NULL;
+	peer->recovery.deadline = UINT64_MAX;
 	peer->recovery.npolls = 0;
 
 	return rc;
 
 reschedule:
 	if (peer->recovery.npolls > 0) {
-		rc = sd_event_source_set_time_relative(peer->recovery.source,
-						       peer->recovery.delay);
-		if (rc >= 0) {
-			rc = sd_event_source_set_enabled(peer->recovery.source,
-							 SD_EVENT_ONESHOT);
-		}
+		peer->recovery.deadline = ctx->now + peer->recovery.delay;
+		rc = update_deadline(ctx, peer->recovery.deadline);
 	}
 	if (rc < 0) {
 reclaim:
 		/* Recovery unsuccessful, clean up the peer */
-		assert(sd_event_source_get_enabled(peer->recovery.source,
-						   NULL) == 0);
+		peer->recovery.deadline = UINT64_MAX;
 		remove_peer(peer);
 	}
 	return rc < 0 ? rc : 0;
@@ -3225,18 +3222,13 @@ static int method_endpoint_recover(sd_bus_message *call, void *data,
 
 	if (!previously) {
 		assert(!peer->recovery.delay);
-		assert(!peer->recovery.source);
+		assert(peer->recovery.deadline == UINT64_MAX);
 		assert(!peer->recovery.npolls);
 		peer->recovery.npolls = MCTP_I2C_TSYM_MN1_MIN + 1;
 		peer->recovery.delay =
 			(MCTP_I2C_TSYM_TRECLAIM_MIN_US / 2) - ctx->mctp_timeout;
-		rc = sd_event_add_time_relative(
-			ctx->event, &peer->recovery.source, CLOCK_MONOTONIC, 0,
-			ctx->mctp_timeout, peer_endpoint_recover, peer);
-		if (rc < 0) {
-			goto out;
-		}
-
+		peer->recovery.deadline = ctx->now + ctx->mctp_timeout;
+		update_deadline(ctx, peer->recovery.deadline);
 		peer->degraded = true;
 
 		rc = sd_bus_emit_properties_changed(
@@ -3255,13 +3247,10 @@ static int method_endpoint_recover(sd_bus_message *call, void *data,
 	if (rc < 0 && !previously) {
 		if (peer->degraded) {
 			/* Cleanup the timer if it was setup successfully. */
-			sd_event_source_set_enabled(peer->recovery.source,
-						    SD_EVENT_OFF);
-			sd_event_source_unref(peer->recovery.source);
+			peer->recovery.deadline = UINT64_MAX;
 		}
 		peer->degraded = previously;
 		peer->recovery.delay = 0;
-		peer->recovery.source = NULL;
 		peer->recovery.npolls = 0;
 	}
 	set_berr(ctx, rc, berr);
@@ -4949,6 +4938,52 @@ static int endpoint_allocate_eids(struct peer *peer)
 	return 0;
 }
 
+int global_time_callback(sd_event_source *source, int timerfd, uint revents,
+			 void *userdata)
+{
+	struct ctx *ctx = userdata;
+	uint64_t n_expireds;
+	size_t i;
+
+	mctp_ops.timerfd.read(timerfd, &n_expireds);
+	mctp_ops.timerfd.gettime(timerfd, &ctx->now);
+
+	for (i = 0; i < ctx->num_peers; i++) {
+		struct peer *p = ctx->peers[i];
+		if (p->recovery.deadline <= ctx->now) {
+			peer_endpoint_recover(p);
+		}
+	}
+
+	return 0;
+}
+
+int setup_timers(struct ctx *ctx)
+{
+	int rc;
+
+	ctx->timerfd = mctp_ops.timerfd.create();
+	if (ctx->timerfd < 0) {
+		warn("timerfd_create");
+		return -errno;
+	}
+
+	rc = mctp_ops.timerfd.gettime(ctx->timerfd, &ctx->now);
+	if (rc < 0) {
+		warn("timerfd_gettime");
+		return -errno;
+	}
+
+	ctx->next_deadline = UINT64_MAX;
+
+	rc = sd_event_add_io(ctx->event, NULL, ctx->timerfd, EPOLLIN,
+			     global_time_callback, ctx);
+	if (rc < 0)
+		return rc;
+
+	return 0;
+}
+
 int main(int argc, char **argv)
 {
 	struct ctx ctxi = { 0 }, *ctx = &ctxi;
@@ -4997,6 +5032,10 @@ int main(int argc, char **argv)
 	if (rc < 0)
 		return 1;
 
+	rc = setup_timers(ctx);
+	if (rc < 0)
+		return 1;
+
 	// TODO add net argument?
 	rc = listen_control_msg(ctx, MCTP_NET_ANY);
 	if (rc < 0) {

tests/conftest.py

@@ -1,5 +1,6 @@
 
 import sys
+import math
 
 import pytest
 import asyncdbus
@@ -25,9 +26,12 @@ def config():
     return None
 
 @pytest.fixture
-async def mctpd(nursery, dbus, sysnet, config):
+async def mctpd(nursery, dbus, sysnet, config, autojump_clock):
+    autojump_clock.rate = 1
+    autojump_clock.autojump_threshold = 1
     m = mctpenv.MctpdWrapper(dbus, sysnet, config = config)
     await m.start_mctpd(nursery)
+    autojump_clock.autojump_threshold = math.inf
     yield m
     res = await m.stop_mctpd()
     assert res == 0