From 91a7369abc7e71cbd0471f2a9b44314dadd69848 Mon Sep 17 00:00:00 2001 From: Jan Leon Date: Mon, 7 Sep 2026 22:47:18 +0200 Subject: [PATCH 1/2] remote: bound updates to MTU-safe UDP datagrams Split oversized updates into existing-format node/station batches without changing the receive protocol. Validate indivisible records before sending and cover the splitter with parser and UDP regression tests. Signed-off-by: Jan Leon --- CMakeLists.txt | 12 +- remote-message.c | 142 ++++++++++++++++++++++ remote-message.h | 18 +++ remote.c | 40 +++++-- tests/README.md | 52 ++++++++ tests/remote-message.c | 264 +++++++++++++++++++++++++++++++++++++++++ 6 files changed, 520 insertions(+), 8 deletions(-) create mode 100644 remote-message.c create mode 100644 remote-message.h create mode 100644 tests/README.md create mode 100644 tests/remote-message.c diff --git a/CMakeLists.txt b/CMakeLists.txt index f889659..2d9bae5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -24,7 +24,7 @@ IF(NOT HAVE_PCAP_H) MESSAGE(FATAL_ERROR "pcap/pcap.h is not found") ENDIF() -SET(SOURCES main.c local_node.c node.c sta.c policy.c ubus.c remote.c parse.c netifd.c timeout.c event.c measurement.c band_steering.c) +SET(SOURCES main.c local_node.c node.c sta.c policy.c ubus.c remote.c remote-message.c parse.c netifd.c timeout.c event.c measurement.c band_steering.c) IF(NL_CFLAGS) ADD_DEFINITIONS(${NL_CFLAGS}) @@ -44,6 +44,16 @@ TARGET_LINK_LIBRARIES(fakeap ubox ubus) ADD_EXECUTABLE(ap-monitor monitor.c parse.c) TARGET_LINK_LIBRARIES(ap-monitor ubox pcap blobmsg_json) +OPTION(BUILD_TESTING "Build remote message regression tests" OFF) +IF(BUILD_TESTING) + ENABLE_TESTING() + ADD_EXECUTABLE(test-remote-message tests/remote-message.c remote-message.c parse.c) + TARGET_INCLUDE_DIRECTORIES(test-remote-message PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) + TARGET_COMPILE_OPTIONS(test-remote-message PRIVATE -UNDEBUG) + TARGET_LINK_LIBRARIES(test-remote-message ubox) + ADD_TEST(NAME remote-message COMMAND test-remote-message) +ENDIF() + SET(CMAKE_INSTALL_PREFIX /usr) INSTALL(TARGETS usteerd diff --git a/remote-message.c b/remote-message.c new file mode 100644 index 0000000..6dd259b --- /dev/null +++ b/remote-message.c @@ -0,0 +1,142 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +#include +#include +#include + +#include "remote-message.h" +#include "remote.h" + +size_t usteer_message_payload_limit(unsigned int mtu, bool ipv6) +{ + unsigned int overhead = ipv6 ? 48 : 28; + + if (mtu <= overhead) + return 0; + + return mtu - overhead < USTEER_REMOTE_MAX_PAYLOAD ? + mtu - overhead : USTEER_REMOTE_MAX_PAYLOAD; +} + +/* The input is a locally constructed message, not an untrusted receive buffer. */ +static struct blob_attr *message_field(struct blob_attr *data, unsigned int id) +{ + struct blob_attr *cur; + int rem; + + blob_for_each_attr(cur, data, rem) + if (blob_id(cur) == id) + return cur; + + return NULL; +} + +static size_t metadata_size(struct blob_attr *data, unsigned int skip) +{ + struct blob_attr *cur; + size_t len = sizeof(*data); + int rem; + + blob_for_each_attr(cur, data, rem) + if (blob_id(cur) != skip) + len += blob_pad_len(cur); + + return len; +} + +static size_t copy_metadata(char *out, struct blob_attr *data, unsigned int skip) +{ + struct blob_attr *cur; + size_t len = sizeof(*data); + int rem; + + memcpy(out, data, sizeof(*data)); + blob_for_each_attr(cur, data, rem) { + if (blob_id(cur) == skip) + continue; + memcpy(out + len, cur, blob_pad_len(cur)); + len += blob_pad_len(cur); + } + + return len; +} + +static int emit_chunk(char *out, size_t len, size_t nodes, size_t node, + size_t stations, usteer_message_emit emit, void *priv) +{ + blob_set_raw_len((struct blob_attr *)out, len); + blob_set_raw_len((struct blob_attr *)(out + nodes), len - nodes); + blob_set_raw_len((struct blob_attr *)(out + node), len - node); + blob_set_raw_len((struct blob_attr *)(out + stations), len - stations); + return emit((struct blob_attr *)out, priv); +} + +int usteer_message_send(struct blob_attr *data, size_t limit, + usteer_message_emit emit, void *priv) +{ + struct blob_attr *nodes, *node, *stations, *sta; + size_t top_len, node_len, overhead, nodes_off, node_off, sta_off, len; + int rem, sta_rem, ret = 0; + char *out; + + if (blob_pad_len(data) <= limit) + return emit(data, priv); + + nodes = message_field(data, APMSG_NODES); + if (!nodes) + return -EINVAL; + + top_len = metadata_size(data, APMSG_NODES) + sizeof(*nodes); + if (top_len > limit) + return -EMSGSIZE; + + /* Validate the entire update before emitting any part of it. Metadata and + * individual station records are indivisible in the existing protocol. + * Never silently truncate them or fall back to IP fragmentation. + */ + blob_for_each_attr(node, nodes, rem) { + stations = message_field(node, APMSG_NODE_STATIONS); + if (!stations) + return -EINVAL; + + node_len = metadata_size(node, APMSG_NODE_STATIONS) + sizeof(*stations); + if (node_len > limit - top_len) + return -EMSGSIZE; + + overhead = top_len + node_len; + blob_for_each_attr(sta, stations, sta_rem) + if (blob_pad_len(sta) > limit - overhead) + return -EMSGSIZE; + } + + out = malloc(limit); + if (!out) + return -ENOMEM; + + nodes_off = copy_metadata(out, data, APMSG_NODES); + memcpy(out + nodes_off, nodes, sizeof(*nodes)); + node_off = nodes_off + sizeof(*nodes); + blob_for_each_attr(node, nodes, rem) { + stations = message_field(node, APMSG_NODE_STATIONS); + sta_off = node_off + copy_metadata(out + node_off, node, APMSG_NODE_STATIONS); + memcpy(out + sta_off, stations, sizeof(*stations)); + overhead = sta_off + sizeof(*stations); + len = overhead; + + blob_for_each_attr(sta, stations, sta_rem) { + if (blob_pad_len(sta) > limit - len) { + ret = emit_chunk(out, len, nodes_off, node_off, sta_off, emit, priv); + if (ret) + goto out; + len = overhead; + } + memcpy(out + len, sta, blob_pad_len(sta)); + len += blob_pad_len(sta); + } + ret = emit_chunk(out, len, nodes_off, node_off, sta_off, emit, priv); + if (ret) + goto out; + } +out: + free(out); + return ret; +} diff --git a/remote-message.h b/remote-message.h new file mode 100644 index 0000000..f3c4e4e --- /dev/null +++ b/remote-message.h @@ -0,0 +1,18 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +#ifndef __USTEER_REMOTE_MESSAGE_H +#define __USTEER_REMOTE_MESSAGE_H + +#include +#include +#include + +/* Leave room for IPv6 and UDP even on a 1280-byte link. */ +#define USTEER_REMOTE_MAX_PAYLOAD 1200 + +typedef int (*usteer_message_emit)(struct blob_attr *data, void *priv); + +size_t usteer_message_payload_limit(unsigned int mtu, bool ipv6); +int usteer_message_send(struct blob_attr *data, size_t limit, + usteer_message_emit emit, void *priv); + +#endif diff --git a/remote.c b/remote.c index bf58ea3..5424bdd 100644 --- a/remote.c +++ b/remote.c @@ -21,6 +21,7 @@ #include #include +#include #include #include #include @@ -32,6 +33,7 @@ #include #include "usteer.h" #include "remote.h" +#include "remote-message.h" #include "node.h" static uint32_t local_id; @@ -485,7 +487,7 @@ static void interface_recv_v6(struct uloop_fd *u, unsigned int events){ } while (1); } -static void interface_send_msg_v4(struct interface *iface, struct blob_attr *data) +static int interface_send_msg_v4(struct interface *iface, struct blob_attr *data) { static size_t cmsg_data[( CMSG_SPACE(sizeof(struct in_pktinfo)) / sizeof(size_t)) + 1]; static struct sockaddr_in a; @@ -518,11 +520,12 @@ static void interface_send_msg_v4(struct interface *iface, struct blob_attr *dat iov.iov_len = blob_pad_len(data); if (sendmsg(remote_fd.fd, &m, 0) < 0) - perror("sendmsg"); + return -errno; + return 0; } -static void interface_send_msg_v6(struct interface *iface, struct blob_attr *data) { +static int interface_send_msg_v6(struct interface *iface, struct blob_attr *data) { static struct sockaddr_in6 groupSock = {}; groupSock.sin6_family = AF_INET6; @@ -532,15 +535,38 @@ static void interface_send_msg_v6(struct interface *iface, struct blob_attr *dat setsockopt(remote_fd.fd, IPPROTO_IPV6, IPV6_MULTICAST_IF, &iface->ifindex, sizeof(iface->ifindex)); if (sendto(remote_fd.fd, data, blob_pad_len(data), 0, (const struct sockaddr *)&groupSock, sizeof(groupSock)) < 0) - perror("sendmsg"); + return -errno; + return 0; } -static void interface_send_msg(struct interface *iface, struct blob_attr *data){ +static int interface_emit_msg(struct blob_attr *data, void *priv) +{ + struct interface *iface = priv; + if (config.ipv6) { - interface_send_msg_v6(iface, data); + return interface_send_msg_v6(iface, data); } else { - interface_send_msg_v4(iface, data); + return interface_send_msg_v4(iface, data); + } +} + +static void interface_send_msg(struct interface *iface, struct blob_attr *data) +{ + struct ifreq ifr = {}; + size_t limit; + int ret; + + snprintf(ifr.ifr_name, sizeof(ifr.ifr_name), "%s", interface_name(iface)); + if (ioctl(remote_fd.fd, SIOCGIFMTU, &ifr) < 0) { + MSG(FATAL, "Cannot read MTU for %s: %s\n", interface_name(iface), strerror(errno)); + return; } + limit = usteer_message_payload_limit(ifr.ifr_mtu > 0 ? ifr.ifr_mtu : 0, + config.ipv6); + ret = usteer_message_send(data, limit, interface_emit_msg, iface); + if (ret) + MSG(FATAL, "Cannot send remote update on %s (payload limit %zu): %s\n", + interface_name(iface), limit, strerror(-ret)); } static void usteer_send_sta_info(struct sta_info *sta) diff --git a/tests/README.md b/tests/README.md new file mode 100644 index 0000000..df71be5 --- /dev/null +++ b/tests/README.md @@ -0,0 +1,52 @@ +# Begrenzte Remote-Updates testen + +Mit den normalen Usteer-Buildabhängigkeiten: + +```sh +cmake -S . -B build-test -DBUILD_TESTING=ON +cmake --build build-test +ctest --test-dir build-test --output-on-failure +``` + +Für ASan/UBSan zusätzlich beim Konfigurieren: + +```sh +-DCMAKE_C_FLAGS="-fsanitize=address,undefined -fno-omit-frame-pointer" \ +-DCMAKE_EXE_LINKER_FLAGS="-fsanitize=address,undefined" +``` + +Der Test nutzt den unveränderten Parser aus `parse.c`. Er prüft die +byteidentische Weitergabe kleiner Nachrichten, eine Grenze exakt auf und +knapp unter der Nachrichtengröße, 1.001 Paketgrenzen von 200 bis 1.200 Byte +mit jeweils 300 Stationen auf zwei APs sowie einen dritten leeren AP. +Stationsdaten dürfen weder fehlen noch doppelt vorkommen. Metadaten, +unbekannte AP-Felder und der Eingabepuffer müssen unverändert bleiben. +Weitere Fälle: unteilbare übergroße Metadaten im letzten AP, übergroße +Stationsdatensätze, zu kleine MTUs, reine Host-Updates und Sendefehler. +Zwei Tests senden und empfangen die Nachrichten über lokale IPv4-/IPv6- +UDP-Sockets; IPv6-Loopback muss in der Testumgebung verfügbar sein. + +## Protokoll und Grenzen + +Die Aufteilung verwendet ausschließlich bestehende Nachrichtenfelder. +Jeder Teil enthält dieselben Host-Metadaten und dieselbe Sequenznummer des +logischen Updates. Der bestehende Empfänger verwendet die Sequenznummer +nicht zur Duplikatunterdrückung und aktualisiert Stationsdatensätze einzeln; +fehlende Stationen werden nicht wegen eines Teilupdates gelöscht. +Der Patch benötigt keine neue Empfangslogik und keinen gleichzeitigen +Versionswechsel aller Gegenstellen. Dies ist durch Parser- und Datentests +gestützt, ersetzt aber keinen Mehr-AP-Betriebstest. + +Das UDP-Nutzlastlimit ist höchstens 1.200 Byte und bei kleineren lokalen +Interface-MTUs zusätzlich um IPv4-/IPv6- und UDP-Header reduziert. Das +ist keine allgemeine Path-MTU-Ermittlung für Tunnel oder fremde Routen. +Die gesamte Nachricht wird vor dem ersten Teil auf Teilbarkeit geprüft. +Ein einzelner zu großer Metadatenblock oder Stationsdatensatz führt zu +`EMSGSIZE` und einem protokollierten Sendefehler, nicht zu Abschneiden oder +einem Rückfall auf übergroße Datagramme. Ein späterer Socket-Sendefehler +kann wie gewöhnlicher UDP-Verlust ein unvollständig empfangenes Update +hinterlassen; die nächste periodische Aktualisierung erfolgt weiterhin. + +Der Patch soll IP-Fragmentierung der Usteer-Updates auf den vorgesehenen +LAN-Interfaces vermeiden. Er ist kein Nachweis einer Behebung sonstiger +WLAN-, Shelly- oder HomePod-Probleme. diff --git a/tests/remote-message.c b/tests/remote-message.c new file mode 100644 index 0000000..70e4787 --- /dev/null +++ b/tests/remote-message.c @@ -0,0 +1,264 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include "remote-message.h" +#include "remote.h" + +static struct blob_buf input; +static unsigned int seen[3][200]; +static unsigned int nodes_seen[3], emitted, fail_at; +static size_t cap; +static bool expect_identical; +static unsigned int station_extra; +static int udp_tx, udp_rx; + +static struct blob_attr *field(struct blob_attr *data, unsigned int id) +{ + struct blob_attr *cur; + int rem; + blob_for_each_attr(cur, data, rem) + if (blob_id(cur) == id) + return cur; + return NULL; +} + +static void make_message(unsigned int count, unsigned int metadata) +{ + unsigned int n, i; + void *nodes, *node, *stations, *station; + char extra[2048] = {}, name[16]; + uint8_t addr[6] = {}; + + assert(metadata <= sizeof(extra)); + blob_buf_init(&input, 0); + blob_put_int32(&input, APMSG_ID, 123); + blob_put_int32(&input, APMSG_SEQ, 456); + blob_put(&input, APMSG_HOST_INFO, NULL, 0); + nodes = blob_nest_start(&input, APMSG_NODES); + for (n = 0; n < 3; n++) { + node = blob_nest_start(&input, 0); + snprintf(name, sizeof(name), "radio%u", n); + blob_put_string(&input, APMSG_NODE_NAME, name); + blob_put_string(&input, APMSG_NODE_SSID, "HomeNET"); + addr[0] = n; + blob_put(&input, APMSG_NODE_BSSID, addr, 6); + blob_put_int32(&input, APMSG_NODE_FREQ, 2412); + blob_put_int32(&input, APMSG_NODE_N_ASSOC, n == 2 ? 0 : count); + blob_put(&input, APMSG_NODE_NODE_INFO, extra, n == 2 ? metadata : 0); + blob_put(&input, 31, "abc", 3); + stations = blob_nest_start(&input, APMSG_NODE_STATIONS); + for (i = 0; n != 2 && i < count; i++) { + station = blob_nest_start(&input, 0); + addr[1] = i; + blob_put(&input, APMSG_STA_ADDR, addr, 6); + blob_put_int32(&input, APMSG_STA_SIGNAL, -40); + blob_put_int32(&input, APMSG_STA_TIMEOUT, 1000); + blob_put_int32(&input, APMSG_STA_SEEN, 10); + blob_put_int32(&input, APMSG_STA_LAST_CONNECTED, 20); + blob_put_int8(&input, APMSG_STA_CONNECTED, 1); + if (station_extra) + blob_put(&input, 31, extra, station_extra); + blob_nest_end(&input, station); + } + blob_nest_end(&input, stations); + blob_nest_end(&input, node); + } + blob_nest_end(&input, nodes); +} + +static int receive_message(struct blob_attr *data, void *priv) +{ + struct apmsg msg; + struct apmsg_node node_msg = {}; + struct apmsg_sta sta_msg; + struct blob_attr *node, *sta, *cur, *original, *original_nodes; + int rem, sta_rem, attr_rem, orig_rem; + unsigned int n; + + assert(priv == &input); + assert(blob_pad_len(data) <= cap); + emitted++; + if (fail_at && emitted == fail_at) + return -EIO; + if (expect_identical) + assert(!memcmp(data, input.head, blob_pad_len(input.head))); + /* Use the unchanged upstream parser, including mandatory-field checks. */ + assert(parse_apmsg(&msg, data)); + assert(msg.id == 123 && msg.seq == 456); + assert(msg.host_info && !blob_len(msg.host_info)); + original_nodes = field(input.head, APMSG_NODES); + blob_for_each_attr(node, msg.nodes, rem) { + assert(parse_apmsg_node(&node_msg, node)); + n = (unsigned char)node_msg.bssid[0]; + assert(n < 3); + nodes_seen[n]++; + original = NULL; + blob_for_each_attr(cur, original_nodes, orig_rem) + if (((unsigned char *)blob_data(field(cur, APMSG_NODE_BSSID)))[0] == n) + original = cur; + assert(original); + blob_for_each_attr(cur, original, attr_rem) { + struct blob_attr *copy; + if (blob_id(cur) == APMSG_NODE_STATIONS) + continue; + copy = field(node, blob_id(cur)); + assert(copy && blob_pad_len(copy) == blob_pad_len(cur)); + assert(!memcmp(copy, cur, blob_pad_len(cur))); + } + blob_for_each_attr(sta, node_msg.stations, sta_rem) { + assert(parse_apmsg_sta(&sta_msg, sta)); + assert(sta_msg.addr[0] == n && sta_msg.addr[1] < 200); + assert(sta_msg.signal == -40 && sta_msg.timeout == 1000); + assert(sta_msg.seen == 10 && sta_msg.last_connected == 20); + assert(sta_msg.connected); + assert(++seen[n][sta_msg.addr[1]] == 1); + } + } + return 0; +} + +static void reset(size_t limit) +{ + memset(seen, 0, sizeof(seen)); + memset(nodes_seen, 0, sizeof(nodes_seen)); + emitted = fail_at = 0; + expect_identical = false; + cap = limit; +} + +static int udp_emit(struct blob_attr *data, void *priv) +{ + unsigned int received[512]; + ssize_t len = blob_pad_len(data); + + assert(send(udp_tx, data, len, 0) == len); + assert(recv(udp_rx, received, sizeof(received), 0) == len); + assert(!memcmp(received, data, len)); + return receive_message((struct blob_attr *)received, priv); +} + +static void test_udp(int family) +{ + struct sockaddr_storage addr = {}; + struct sockaddr_in *v4 = (void *)&addr; + struct sockaddr_in6 *v6 = (void *)&addr; + struct timeval timeout = { .tv_sec = 2 }; + socklen_t len = family == AF_INET ? sizeof(*v4) : sizeof(*v6); + unsigned int n, i; + + addr.ss_family = family; + if (family == AF_INET) + v4->sin_addr.s_addr = htonl(INADDR_LOOPBACK); + else + v6->sin6_addr = in6addr_loopback; + udp_rx = socket(family, SOCK_DGRAM, 0); + udp_tx = socket(family, SOCK_DGRAM, 0); + assert(udp_rx >= 0 && udp_tx >= 0); + assert(!bind(udp_rx, (void *)&addr, len)); + assert(!getsockname(udp_rx, (void *)&addr, &len)); + assert(!connect(udp_tx, (void *)&addr, len)); + assert(!setsockopt(udp_rx, SOL_SOCKET, SO_RCVTIMEO, &timeout, sizeof(timeout))); + make_message(150, 16); + reset(1200); + assert(!usteer_message_send(input.head, cap, udp_emit, &input)); + for (n = 0; n < 2; n++) + for (i = 0; i < 150; i++) + assert(seen[n][i] == 1); + assert(nodes_seen[2]); + close(udp_rx); + close(udp_tx); +} + +int main(void) +{ + unsigned int n, i; + size_t limit; + int ret; + void *original; + + assert(usteer_message_payload_limit(1500, false) == 1200); + assert(usteer_message_payload_limit(1280, true) == 1200); + assert(usteer_message_payload_limit(576, false) == 548); + assert(usteer_message_payload_limit(1000, true) == 952); + assert(!usteer_message_payload_limit(28, false)); + assert(!usteer_message_payload_limit(48, true)); + assert(!usteer_message_payload_limit(0, true)); + + make_message(1, 0); + reset(blob_pad_len(input.head)); + expect_identical = true; + assert(!usteer_message_send(input.head, cap, receive_message, &input)); + assert(emitted == 1); + reset(blob_pad_len(input.head) - 1); + assert(!usteer_message_send(input.head, cap, receive_message, &input)); + assert(emitted > 1); + + make_message(150, 16); + original = malloc(blob_pad_len(input.head)); + assert(original); + memcpy(original, input.head, blob_pad_len(input.head)); + for (limit = 200; limit <= 1200; limit++) { + reset(limit); + assert(!usteer_message_send(input.head, cap, receive_message, &input)); + assert(!memcmp(original, input.head, blob_pad_len(input.head))); + for (n = 0; n < 3; n++) { + assert(nodes_seen[n]); + for (i = 0; i < 150; i++) + assert(seen[n][i] == (n != 2)); + } + } + free(original); + reset(1200); + fail_at = 2; + assert(usteer_message_send(input.head, cap, receive_message, &input) == -EIO); + assert(emitted == 2); + + /* Metadata and individual records cannot be truncated to fit. */ + for (limit = 0; limit < 100; limit++) { + reset(limit); + ret = usteer_message_send(input.head, cap, receive_message, &input); + assert(ret == -EMSGSIZE && !emitted); + } + make_message(1, 1400); + reset(1200); + assert(usteer_message_send(input.head, cap, receive_message, &input) == -EMSGSIZE); + assert(!emitted); + station_extra = 1400; + make_message(1, 0); + reset(1200); + assert(usteer_message_send(input.head, cap, receive_message, &input) == -EMSGSIZE); + assert(!emitted); + station_extra = 0; + + make_message(0, 0); + reset(200); + assert(!usteer_message_send(input.head, cap, receive_message, &input)); + assert(nodes_seen[0] && nodes_seen[1] && nodes_seen[2]); + + /* A host-only update remains a complete, valid old-format message. */ + blob_buf_init(&input, 0); + blob_put_int32(&input, APMSG_ID, 123); + blob_put_int32(&input, APMSG_SEQ, 456); + blob_put(&input, APMSG_HOST_INFO, NULL, 0); + blob_put(&input, APMSG_NODES, NULL, 0); + reset(1200); + expect_identical = true; + assert(!usteer_message_send(input.head, cap, receive_message, &input)); + assert(emitted == 1); + reset(20); + assert(usteer_message_send(input.head, cap, receive_message, &input) == -EMSGSIZE); + assert(!emitted); + + test_udp(AF_INET); + test_udp(AF_INET6); + blob_buf_free(&input); + puts("remote-message: all tests passed"); + return 0; +} From 947969722087281b874fe7b4dbfe36c5b462c1fc Mon Sep 17 00:00:00 2001 From: Jan Leon Date: Mon, 7 Sep 2026 22:47:18 +0200 Subject: [PATCH 2/2] remote: initialize the socket before sending the first update Skip sends on unregistered sockets and handle descriptor zero correctly. Add a socket lifecycle regression test and English test documentation. Signed-off-by: Jan Leon --- CMakeLists.txt | 6 +++ remote.c | 14 ++++--- tests/README.md | 78 +++++++++++++++++++------------------ tests/remote-socket.c | 89 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 144 insertions(+), 43 deletions(-) create mode 100644 tests/remote-socket.c diff --git a/CMakeLists.txt b/CMakeLists.txt index 2d9bae5..a7b8d84 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -52,6 +52,12 @@ IF(BUILD_TESTING) TARGET_COMPILE_OPTIONS(test-remote-message PRIVATE -UNDEBUG) TARGET_LINK_LIBRARIES(test-remote-message ubox) ADD_TEST(NAME remote-message COMMAND test-remote-message) + ADD_EXECUTABLE(test-remote-socket tests/remote-socket.c remote-message.c) + TARGET_INCLUDE_DIRECTORIES(test-remote-socket PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) + TARGET_COMPILE_OPTIONS(test-remote-socket PRIVATE -UNDEBUG -ffunction-sections -fdata-sections) + SET_TARGET_PROPERTIES(test-remote-socket PROPERTIES LINK_FLAGS "-Wl,--gc-sections") + TARGET_LINK_LIBRARIES(test-remote-socket ubox) + ADD_TEST(NAME remote-socket COMMAND test-remote-socket) ENDIF() SET(CMAKE_INSTALL_PREFIX /usr) diff --git a/remote.c b/remote.c index 5424bdd..6ee4447 100644 --- a/remote.c +++ b/remote.c @@ -37,7 +37,7 @@ #include "node.h" static uint32_t local_id; -static struct uloop_fd remote_fd; +static struct uloop_fd remote_fd = { .fd = -1 }; static struct uloop_timeout remote_timer; static struct uloop_timeout reload_timer; @@ -556,6 +556,10 @@ static void interface_send_msg(struct interface *iface, struct blob_attr *data) size_t limit; int ret; + /* Node notifications can arrive before the remote socket is ready. */ + if (!remote_fd.registered) + return; + snprintf(ifr.ifr_name, sizeof(ifr.ifr_name), "%s", interface_name(iface)); if (ioctl(remote_fd.fd, SIOCGIFMTU, &ifr) < 0) { MSG(FATAL, "Cannot read MTU for %s: %s\n", interface_name(iface), strerror(errno)); @@ -770,7 +774,7 @@ static int usteer_create_v6_socket() { static void usteer_reload_timer(struct uloop_timeout *t) { /* Remove uloop descriptor */ - if (remote_fd.fd && remote_fd.registered) { + if (remote_fd.fd >= 0) { uloop_fd_delete(&remote_fd); close(remote_fd.fd); } @@ -794,11 +798,11 @@ int usteer_interface_init(void) if (usteer_init_local_id()) return -1; - remote_timer.cb = usteer_send_update_timer; - remote_timer.cb(&remote_timer); - reload_timer.cb = usteer_reload_timer; reload_timer.cb(&reload_timer); + remote_timer.cb = usteer_send_update_timer; + remote_timer.cb(&remote_timer); + return 0; } diff --git a/tests/README.md b/tests/README.md index df71be5..c9f4ba7 100644 --- a/tests/README.md +++ b/tests/README.md @@ -1,6 +1,6 @@ -# Begrenzte Remote-Updates testen +# Testing bounded remote updates -Mit den normalen Usteer-Buildabhängigkeiten: +With the regular Usteer build dependencies installed: ```sh cmake -S . -B build-test -DBUILD_TESTING=ON @@ -8,45 +8,47 @@ cmake --build build-test ctest --test-dir build-test --output-on-failure ``` -Für ASan/UBSan zusätzlich beim Konfigurieren: +For ASan/UBSan, also pass these options when configuring: ```sh -DCMAKE_C_FLAGS="-fsanitize=address,undefined -fno-omit-frame-pointer" \ -DCMAKE_EXE_LINKER_FLAGS="-fsanitize=address,undefined" ``` -Der Test nutzt den unveränderten Parser aus `parse.c`. Er prüft die -byteidentische Weitergabe kleiner Nachrichten, eine Grenze exakt auf und -knapp unter der Nachrichtengröße, 1.001 Paketgrenzen von 200 bis 1.200 Byte -mit jeweils 300 Stationen auf zwei APs sowie einen dritten leeren AP. -Stationsdaten dürfen weder fehlen noch doppelt vorkommen. Metadaten, -unbekannte AP-Felder und der Eingabepuffer müssen unverändert bleiben. -Weitere Fälle: unteilbare übergroße Metadaten im letzten AP, übergroße -Stationsdatensätze, zu kleine MTUs, reine Host-Updates und Sendefehler. -Zwei Tests senden und empfangen die Nachrichten über lokale IPv4-/IPv6- -UDP-Sockets; IPv6-Loopback muss in der Testumgebung verfügbar sein. - -## Protokoll und Grenzen - -Die Aufteilung verwendet ausschließlich bestehende Nachrichtenfelder. -Jeder Teil enthält dieselben Host-Metadaten und dieselbe Sequenznummer des -logischen Updates. Der bestehende Empfänger verwendet die Sequenznummer -nicht zur Duplikatunterdrückung und aktualisiert Stationsdatensätze einzeln; -fehlende Stationen werden nicht wegen eines Teilupdates gelöscht. -Der Patch benötigt keine neue Empfangslogik und keinen gleichzeitigen -Versionswechsel aller Gegenstellen. Dies ist durch Parser- und Datentests -gestützt, ersetzt aber keinen Mehr-AP-Betriebstest. - -Das UDP-Nutzlastlimit ist höchstens 1.200 Byte und bei kleineren lokalen -Interface-MTUs zusätzlich um IPv4-/IPv6- und UDP-Header reduziert. Das -ist keine allgemeine Path-MTU-Ermittlung für Tunnel oder fremde Routen. -Die gesamte Nachricht wird vor dem ersten Teil auf Teilbarkeit geprüft. -Ein einzelner zu großer Metadatenblock oder Stationsdatensatz führt zu -`EMSGSIZE` und einem protokollierten Sendefehler, nicht zu Abschneiden oder -einem Rückfall auf übergroße Datagramme. Ein späterer Socket-Sendefehler -kann wie gewöhnlicher UDP-Verlust ein unvollständig empfangenes Update -hinterlassen; die nächste periodische Aktualisierung erfolgt weiterhin. - -Der Patch soll IP-Fragmentierung der Usteer-Updates auf den vorgesehenen -LAN-Interfaces vermeiden. Er ist kein Nachweis einer Behebung sonstiger -WLAN-, Shelly- oder HomePod-Probleme. +The test uses the unchanged parser from `parse.c`. It checks byte-identical +forwarding of small messages, limits exactly at and just below the message +size, and 1,001 payload limits from 200 to 1,200 bytes with 300 stations +across two nodes plus a third, empty node. Station records must not be lost +or duplicated. Metadata, unknown node fields and the input buffer must be +preserved. Further cases cover indivisible oversized metadata in the last +node, oversized station records, small MTUs, host-only updates and send +errors. Two tests send and receive messages through actual IPv4/IPv6 +loopback UDP sockets; IPv6 loopback must be available in the test environment. + +`remote-socket` also exercises the actual send path with mocked system +calls: no MTU query before socket registration or after removal, valid +descriptor 0 and a controlled MTU query failure. The initial periodic send +is started only after socket initialization. + +## Protocol and limitations + +Splitting uses only existing message fields. Each chunk contains the same +host metadata and sequence number as the logical update. The existing +receiver does not use the sequence number to suppress duplicates and +updates station records individually; missing stations are not deleted +because an update contains only a subset. No new receive logic or +simultaneous upgrade of all peers is required. Parser and data tests +support this, but do not replace a multi-AP deployment test. + +The UDP payload limit is at most 1,200 bytes and is reduced further for +smaller local interface MTUs, accounting for IPv4/IPv6 and UDP headers. +This is not general path-MTU discovery for tunnels or routed networks. +The entire message is checked for splittability before sending any chunk. +An indivisible oversized metadata block or station record produces +`EMSGSIZE` and a logged send error, not truncation or an oversized fallback. +A later socket send error can still leave a partially received update, +as with ordinary UDP loss; subsequent periodic updates continue normally. + +The patch is intended to avoid IP fragmentation of Usteer updates on the +intended LAN interfaces. It does not establish a fix for unrelated radio +or client connectivity problems. diff --git a/tests/remote-socket.c b/tests/remote-socket.c new file mode 100644 index 0000000..b7ea8ef --- /dev/null +++ b/tests/remote-socket.c @@ -0,0 +1,89 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +#define _GNU_SOURCE +#include +#include +#include +#include + +static int mock_ioctl(int fd, unsigned long request, ...); +static ssize_t mock_sendmsg(int fd, const struct msghdr *msg, int flags); +static ssize_t mock_sendto(int fd, const void *data, size_t len, int flags, + const struct sockaddr *addr, socklen_t addrlen); +#define ioctl mock_ioctl +#define sendmsg mock_sendmsg +#define sendto mock_sendto +#include "../remote.c" +#undef ioctl +#undef sendmsg +#undef sendto + +struct usteer_config config; +static unsigned int mtu_queries, sends, errors; +static bool fail_mtu; + +void debug_msg(int level, const char *func, int line, const char *format, ...) +{ + errors++; +} + +static int mock_ioctl(int fd, unsigned long request, ...) +{ + struct ifreq *ifr; + va_list ap; + + assert(fd == 0 && request == SIOCGIFMTU); + mtu_queries++; + if (fail_mtu) { + errno = ENODEV; + return -1; + } + va_start(ap, request); + ifr = va_arg(ap, struct ifreq *); + ifr->ifr_mtu = 1500; + va_end(ap); + return 0; +} + +static ssize_t mock_sendmsg(int fd, const struct msghdr *msg, int flags) +{ + assert(fd == 0 && msg->msg_iovlen == 1); + sends++; + return msg->msg_iov[0].iov_len; +} + +static ssize_t mock_sendto(int fd, const void *data, size_t len, int flags, + const struct sockaddr *addr, socklen_t addrlen) +{ + assert(0); + return -1; +} + +int main(void) +{ + struct interface iface = {}; + + iface.node.avl.key = "test0"; + blob_buf_init(&buf, 0); + blob_put_int32(&buf, APMSG_ID, 1); + blob_put_int32(&buf, APMSG_SEQ, 1); + blob_put(&buf, APMSG_NODES, NULL, 0); + assert(remote_fd.fd == -1 && !remote_fd.registered); + interface_send_msg(&iface, buf.head); + assert(!mtu_queries && !sends && !errors); + /* Descriptor zero is valid after registration, but not before it. */ + remote_fd.fd = 0; + interface_send_msg(&iface, buf.head); + assert(!mtu_queries && !sends && !errors); + remote_fd.registered = true; + interface_send_msg(&iface, buf.head); + assert(mtu_queries == 1 && sends == 1 && !errors); + fail_mtu = true; + interface_send_msg(&iface, buf.head); + assert(mtu_queries == 2 && sends == 1 && errors == 1); + remote_fd.registered = false; + interface_send_msg(&iface, buf.head); + assert(mtu_queries == 2 && sends == 1 && errors == 1); + blob_buf_free(&buf); + puts("remote-socket: all tests passed"); + return 0; +}