Fix mctpd deleted interface handling

README.md

@@ -126,6 +126,11 @@ cd obj
 pytest
 ```
 
+To run without an existing dbus session:
+```sh
+dbus-run-session env DBUS_STARTER_BUS_TYPE=user pytest
+```
+
 The test infrastructure depends on a few python packages, including the pytest
 binary. You can use a python `venv` to provide these:
 

src/mctp-netlink.c

@@ -28,6 +28,7 @@ struct linkmap_entry {
 
 	uint32_t min_mtu;
 	uint32_t max_mtu;
+	uint32_t hwaddr_len;
 
 	mctp_eid_t *local_eids;
 	size_t num_local;
@@ -57,7 +58,7 @@ static int fill_linkmap(mctp_nl *nl);
 static void sort_linkmap(mctp_nl *nl);
 static int linkmap_add_entry(mctp_nl *nl, struct ifinfomsg *info,
 		const char *ifname, size_t ifname_len, uint32_t net,
-		bool up, uint32_t min_mtu, uint32_t max_mtu);
+		bool up, uint32_t min_mtu, uint32_t max_mtu, size_t hwaddr_len);
 static struct linkmap_entry *entry_byindex(const mctp_nl *nl,
 	int index);
 
@@ -330,13 +331,13 @@ void mctp_nl_changes_dump(mctp_nl *nl, mctp_nl_change *changes, size_t num_chang
 		"ADD_EID", "DEL_EID",
 	};
 
-	printf("%zu changes:\n", num_changes);
+	fprintf(stderr, "%zu changes:\n", num_changes);
 	for (size_t i = 0; i < num_changes; i++) {
 		mctp_nl_change *ch = &changes[i];
 		const char* ifname = mctp_nl_if_byindex(nl, ch->ifindex);
 		if (!ifname)
 			ifname = "deleted";
-		printf("%3zd %-12s ifindex %3d (%-20s) eid %3d old_net %4d old_up %d\n",
+		fprintf(stderr, "%3zd %-12s ifindex %3d (%-20s) eid %3d old_net %4d old_up %d\n",
 			i, ops[ch->op], ch->ifindex, ifname, ch->eid,
 			ch->old_net, ch->old_up);
 	}
@@ -683,7 +684,7 @@ static int parse_getlink_dump(mctp_nl *nl, struct nlmsghdr *nlh, uint32_t len)
 	for (; NLMSG_OK(nlh, len); nlh = NLMSG_NEXT(nlh, len)) {
 		struct rtattr *rta = NULL, *rt_nest = NULL, *rt_mctp = NULL;
 		char *ifname = NULL;
-		size_t ifname_len, rlen, nlen, mlen;
+		size_t ifname_len, rlen, nlen, mlen, hwaddr_len;
 		uint32_t net, min_mtu = 0, max_mtu = 0;
 		bool up;
 
@@ -734,10 +735,15 @@ static int parse_getlink_dump(mctp_nl *nl, struct nlmsghdr *nlh, uint32_t len)
 			continue;
 		}
 
+		/* Treat missing address as 0 length */
+		hwaddr_len = 0;
+		mctp_get_rtnlmsg_attr(IFLA_ADDRESS, rta, rlen, &hwaddr_len);
+
 		/* TODO: media type */
 
 		up = info->ifi_flags & IFF_UP;
-		linkmap_add_entry(nl, info, ifname, ifname_len, net, up, min_mtu, max_mtu);
+		linkmap_add_entry(nl, info, ifname, ifname_len, net, up,
+			min_mtu, max_mtu, hwaddr_len);
 	}
 	// Not done.
 	return 1;
@@ -859,7 +865,7 @@ static int fill_local_addrs(mctp_nl *nl)
 
 		entry = entry_byindex(nl, ifa->ifa_index);
 		if (!entry) {
-			warnx("kernel returned address for unknown if");
+			warnx("kernel returned address for unknown if %d", ifa->ifa_index);
 			continue;
 		}
 		tmp = realloc(entry->local_eids,
@@ -904,19 +910,19 @@ void mctp_nl_linkmap_dump(const mctp_nl *nl)
 {
 	size_t i, j;
 
-	printf("linkmap\n");
+	fprintf(stderr, "linkmap\n");
 	for (i = 0; i < nl->linkmap_count; i++) {
 		struct linkmap_entry *entry = &nl->linkmap[i];
 		const char* updown = entry->up ? "up" : "DOWN";
-		printf("  %2d: %s, net %d %s local addrs [",
+		fprintf(stderr, "  %2d: %s, net %d %s local addrs [",
 			entry->ifindex, entry->ifname,
 			entry->net, updown);
 		for (j = 0; j < entry->num_local; j++) {
 			if (j != 0)
-				printf(", ");
-			printf("%d", entry->local_eids[j]);
+				fprintf(stderr, ", ");
+			fprintf(stderr, "%d", entry->local_eids[j]);
 		}
-		printf("]\n");
+		fprintf(stderr, "]\n");
 	}
 }
 
@@ -1003,6 +1009,16 @@ uint32_t mctp_nl_max_mtu_byindex(const mctp_nl *nl, int index)
 	return 0;
 }
 
+int mctp_nl_hwaddr_len_byindex(const mctp_nl *nl, int index, size_t *ret_hwaddr_len)
+{
+	struct linkmap_entry *entry = entry_byindex(nl, index);
+	if (!entry) {
+		return -ENOENT;
+	}
+	*ret_hwaddr_len = entry->hwaddr_len;
+	return 0;
+}
+
 mctp_eid_t *mctp_nl_addrs_byindex(const mctp_nl *nl, int index,
 	size_t *ret_num)
 {
@@ -1082,7 +1098,7 @@ int *mctp_nl_if_list(const mctp_nl *nl, size_t *ret_num_ifs)
 
 static int linkmap_add_entry(mctp_nl *nl, struct ifinfomsg *info,
 		const char *ifname, size_t ifname_len, uint32_t net,
-		bool up, uint32_t min_mtu, uint32_t max_mtu)
+		bool up, uint32_t min_mtu, uint32_t max_mtu, size_t hwaddr_len)
 {
 	struct linkmap_entry *entry;
 	size_t newsz;
@@ -1120,6 +1136,7 @@ static int linkmap_add_entry(mctp_nl *nl, struct ifinfomsg *info,
 	entry->up = up;
 	entry->max_mtu = max_mtu;
 	entry->min_mtu = min_mtu;
+	entry->hwaddr_len = hwaddr_len;
 	return 0;
 }
 

src/mctp-netlink.h

@@ -68,6 +68,8 @@ bool mctp_nl_up_byindex(const mctp_nl *nl, int index);
 uint32_t mctp_nl_min_mtu_byindex(const mctp_nl *nl, int index);
 /* Returns interface max_mtu, or 0 if bad index */
 uint32_t mctp_nl_max_mtu_byindex(const mctp_nl *nl, int index);
+/* Returns negative errno on failure */
+int mctp_nl_hwaddr_len_byindex(const mctp_nl *nl, int index, size_t *ret_hwaddr_len);
 /* Caller to free */
 mctp_eid_t *mctp_nl_addrs_byindex(const mctp_nl *nl, int index,
 	size_t *ret_num);

src/mctp-ops.c

@@ -9,6 +9,7 @@
 
 #include <unistd.h>
 #include <linux/netlink.h>
+#include <err.h>
 
 #include "mctp.h"
 #include "mctp-ops.h"
@@ -51,7 +52,12 @@ static int mctp_op_close(int sd)
 	return close(sd);
 }
 
-struct mctp_ops mctp_ops = {
+static void mctp_bug_warn(const char* fmt, va_list args)
+{
+	vwarnx(fmt, args);
+}
+
+const struct mctp_ops mctp_ops = {
 	.mctp = {
 		.socket = mctp_op_mctp_socket,
 		.setsockopt = mctp_op_setsockopt,
@@ -68,6 +74,7 @@ struct mctp_ops mctp_ops = {
 		.recvfrom = mctp_op_recvfrom,
 		.close = mctp_op_close,
 	},
+	.bug_warn = mctp_bug_warn,
 };
 
 void mctp_ops_init(void) { }

src/mctp-ops.h

@@ -8,6 +8,7 @@
 #pragma once
 
 #include <sys/socket.h>
+#include <stdarg.h>
 
 #define _GNU_SOURCE
 
@@ -26,8 +27,9 @@ struct socket_ops {
 struct mctp_ops {
 	struct socket_ops mctp;
 	struct socket_ops nl;
+	void (*bug_warn)(const char* fmt, va_list args);
 };
 
-extern struct mctp_ops mctp_ops;
+extern const struct mctp_ops mctp_ops;
 
 void mctp_ops_init(void);