Merge remote-tracking branch 'origin/pr/initial-min-mtu'

CHANGELOG.md

@@ -16,6 +16,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 1. We now enforce IID checks on MCTP control protocol responses; this
    prevents odd behaviour from delayed or invalid responses.
 
+2. In mctpd the initial route MTU for an endpoint is now set to the minimum MTU
+   of the interface. This allows better compatibility with devices that 
+   have a low initial allowed packet size and require application negotiation
+   to increase that packet size. Previously the initial MTU was left as the
+   interface default (normally the maximum MTU).
+   The .SetMTU method can be used to set the endpoint route MTU.
+
 ## [2.0] - 2024-09-19
 
 ### Added

src/mctp-netlink.c

@@ -26,6 +26,9 @@ struct linkmap_entry {
 	int	net;
 	bool 	up;
 
+	uint32_t min_mtu;
+	uint32_t max_mtu;
+
 	mctp_eid_t *local_eids;
 	size_t num_local;
 
@@ -54,7 +57,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, int net,
-		bool up);
+		bool up, uint32_t min_mtu, uint32_t max_mtu);
 static struct linkmap_entry *entry_byindex(const mctp_nl *nl,
 	int index);
 
@@ -678,10 +681,10 @@ static int parse_getlink_dump(mctp_nl *nl, struct nlmsghdr *nlh, uint32_t len)
 	struct ifinfomsg *info;
 
 	for (; NLMSG_OK(nlh, len); nlh = NLMSG_NEXT(nlh, len)) {
-		struct rtattr *rta, *rt_nest, *rt_mctp;
-		char *ifname;
+		struct rtattr *rta = NULL, *rt_nest = NULL, *rt_mctp = NULL;
+		char *ifname = NULL;
 		size_t ifname_len, rlen, nlen, mlen;
-		uint32_t net;
+		uint32_t net, min_mtu = 0, max_mtu = 0;
 		bool up;
 
 		if (nlh->nlmsg_type == NLMSG_DONE)
@@ -700,7 +703,6 @@ static int parse_getlink_dump(mctp_nl *nl, struct nlmsghdr *nlh, uint32_t len)
 		rta = (void *)(info + 1);
 		rlen = NLMSG_PAYLOAD(nlh, sizeof(*info));
 
-		rt_mctp = false;
 		rt_nest = mctp_get_rtnlmsg_attr(IFLA_AF_SPEC, rta, rlen, &nlen);
 		if (rt_nest) {
 			rt_mctp = mctp_get_rtnlmsg_attr(AF_MCTP, rt_nest, nlen, &mlen);
@@ -709,21 +711,33 @@ static int parse_getlink_dump(mctp_nl *nl, struct nlmsghdr *nlh, uint32_t len)
 			/* Skip non-MCTP interfaces */
 			continue;
 		}
-		if (!mctp_get_rtnlmsg_attr_u32(IFLA_MCTP_NET, rt_mctp, mlen, &net)) {
-			warnx("Missing IFLA_MCTP_NET");
-			continue;
-		}
-
-		/* TODO: media type */
 
 		ifname = mctp_get_rtnlmsg_attr(IFLA_IFNAME, rta, rlen, &ifname_len);
 		if (!ifname) {
 			warnx("no ifname?");
 			continue;
 		}
+
 		ifname_len = strnlen(ifname, ifname_len);
+		if (!mctp_get_rtnlmsg_attr_u32(IFLA_MCTP_NET, rt_mctp, mlen, &net)) {
+			warnx("Missing IFLA_MCTP_NET for %s", ifname);
+			continue;
+		}
+
+		if (!mctp_get_rtnlmsg_attr_u32(IFLA_MIN_MTU, rta, rlen, &min_mtu)) {
+			warnx("Missing IFLA_MIN_MTU for %s", ifname);
+			continue;
+		}
+
+		if (!mctp_get_rtnlmsg_attr_u32(IFLA_MAX_MTU, rta, rlen, &max_mtu)) {
+			warnx("Missing IFLA_MAX_MTU for %s", ifname);
+			continue;
+		}
+
+		/* TODO: media type */
+
 		up = info->ifi_flags & IFF_UP;
-		linkmap_add_entry(nl, info, ifname, ifname_len, net, up);
+		linkmap_add_entry(nl, info, ifname, ifname_len, net, up, min_mtu, max_mtu);
 	}
 	// Not done.
 	return 1;
@@ -973,6 +987,22 @@ bool mctp_nl_up_byindex(const mctp_nl *nl, int index)
 	return false;
 }
 
+uint32_t mctp_nl_min_mtu_byindex(const mctp_nl *nl, int index)
+{
+	struct linkmap_entry *entry = entry_byindex(nl, index);
+	if (entry)
+		return entry->min_mtu;
+	return 0;
+}
+
+uint32_t mctp_nl_max_mtu_byindex(const mctp_nl *nl, int index)
+{
+	struct linkmap_entry *entry = entry_byindex(nl, index);
+	if (entry)
+		return entry->max_mtu;
+	return 0;
+}
+
 mctp_eid_t *mctp_nl_addrs_byindex(const mctp_nl *nl, int index,
 	size_t *ret_num)
 {
@@ -1055,7 +1085,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, int net,
-		bool up)
+		bool up, uint32_t min_mtu, uint32_t max_mtu)
 {
 	struct linkmap_entry *entry;
 	size_t newsz;
@@ -1091,6 +1121,8 @@ static int linkmap_add_entry(mctp_nl *nl, struct ifinfomsg *info,
 	entry->ifindex = info->ifi_index;
 	entry->net = net;
 	entry->up = up;
+	entry->max_mtu = max_mtu;
+	entry->min_mtu = min_mtu;
 	return 0;
 }
 

src/mctp-netlink.h

@@ -64,6 +64,10 @@ int mctp_nl_ifindex_byname(const mctp_nl *nl, const char *ifname);
 const char* mctp_nl_if_byindex(const mctp_nl *nl, int index);
 int mctp_nl_net_byindex(const mctp_nl *nl, int index);
 bool mctp_nl_up_byindex(const mctp_nl *nl, int index);
+/* Returns interface min_mtu, or 0 if bad 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);
 /* Caller to free */
 mctp_eid_t *mctp_nl_addrs_byindex(const mctp_nl *nl, int index,
 	size_t *ret_num);

src/mctpd.c

@@ -147,7 +147,8 @@ struct peer {
 	bool have_neigh;
 	bool have_route;
 
-	// set by SetMTU method, 0 otherwise
+	// MTU for the route. Set to the interface's minimum MTU initially,
+	// or changed by .SetMTU method
 	uint32_t mtu;
 
 	// malloc()ed list of supported message types, from Get Message Type
@@ -2269,6 +2270,10 @@ static int setup_added_peer(peer *peer)
 {
 	int rc;
 
+	// Set minimum MTU by default for compatibility. Clients can increase
+	// this with .SetMTU as needed
+	peer->mtu = mctp_nl_min_mtu_byindex(peer->ctx->nl_query, peer->phys.ifindex);
+
 	// add route before querying
 	add_peer_route(peer);
 

tests/conftest.py

@@ -40,12 +40,18 @@ def to_nlmsg(self, seq = 0):
 
 class System:
     class Interface:
-        def __init__(self, name, ifindex, net, lladdr, mtu, up = False):
+        """Interface constructor.
+
+        Initial mtu is set to max_mtu.
+        """
+        def __init__(self, name, ifindex, net, lladdr, min_mtu, max_mtu, up = False):
             self.name = name
             self.ifindex = ifindex
             self.net = net
             self.lladdr = lladdr,
-            self.mtu = mtu
+            self.min_mtu = min_mtu
+            self.max_mtu = max_mtu
+            self.mtu = max_mtu
             self.up = up
 
         def __str__(self):
@@ -654,6 +660,8 @@ async def _handle_getlink(self, msg):
                 ['IFLA_IFNAME', iface.name],
                 ['IFLA_ADDRESS', iface.lladdr],
                 ['IFLA_MTU', iface.mtu],
+                ['IFLA_MIN_MTU', iface.min_mtu],
+                ['IFLA_MAX_MTU', iface.max_mtu],
                 ['IFLA_AF_SPEC', {
                     'attrs': [['AF_MCTP', {
                         'attrs': [['IFLA_MCTP_NET', iface.net]],
@@ -931,15 +939,15 @@ def name_owner_changed(name, new_owner, old_owner):
 
 Sysnet = namedtuple('SysNet', ['system', 'network'])
 
+"""Simple system & network.
+
+Contains one interface (lladdr 0x10, local EID 8), and one endpoint (lladdr
+0x1d), that reports support for MCTP control and PLDM.
+"""
 @pytest.fixture
 async def sysnet():
-    """Simple system & network.
-
-    Contains one interface (lladdr 0x10, local EID 8), and one endpoint (lladdr
-    0x1d), that reports support for MCTP control and PLDM.
-    """
     system = System()
-    iface = System.Interface('mctp0', 1, 1, bytes([0x10]), 68, True)
+    iface = System.Interface('mctp0', 1, 1, bytes([0x10]), 68, 254, True)
     system.add_interface(iface)
     system.add_address(System.Address(iface, 8))