From ed7378a2a9538634e6bf88822d9e3048e431c41a Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Sat, 31 Aug 2024 09:42:18 +0700 Subject: [PATCH 1/2] tests: Add min_mtu and max_mtu to test Interface Initial MTU is set to max_mtu to match the Linux driver. These are expected in all interfaces reported by the kernel. Tests will fail without this once mctpd starts using IFLA_MIN_MTU and IFLA_MAX_MTU. Signed-off-by: Matt Johnston --- tests/conftest.py | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index ca5fbb6..35a6fcc 100644 --- a/tests/conftest.py +++ b/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): @@ -652,6 +658,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]], @@ -929,15 +937,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)) From 7a3352481620c415f60bc46530a80b568d6fc3c5 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Thu, 2 Mar 2023 13:05:41 +0800 Subject: [PATCH 2/2] mctpd: Set initial route MTU to interface minimum 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 MTU was left at 0, so matching the currently set MTU of the interface. Signed-off-by: Matt Johnston --- CHANGELOG.md | 7 ++++++ src/mctp-netlink.c | 58 +++++++++++++++++++++++++++++++++++----------- src/mctp-netlink.h | 4 ++++ src/mctpd.c | 7 +++++- 4 files changed, 62 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 20a20a0..0416a4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). now associated with the interface object, they no longer take the interface name as their first argument. +6. 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. + ### Fixed 1. mctpd: EID assignments now work in the case where a new endpoint has a diff --git a/src/mctp-netlink.c b/src/mctp-netlink.c index dfbe042..6304dde 100644 --- a/src/mctp-netlink.c +++ b/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; } diff --git a/src/mctp-netlink.h b/src/mctp-netlink.h index 52847c9..26450a4 100644 --- a/src/mctp-netlink.h +++ b/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); diff --git a/src/mctpd.c b/src/mctpd.c index b16e456..0c934fc 100644 --- a/src/mctpd.c +++ b/src/mctpd.c @@ -146,7 +146,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 @@ -2161,6 +2162,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);