diff --git a/doc/ChangeLog.md b/doc/ChangeLog.md index 7911e476e..e66857e30 100644 --- a/doc/ChangeLog.md +++ b/doc/ChangeLog.md @@ -121,6 +121,10 @@ All notable changes to the project are documented in this file. - WebUI: a WiFi interface can be switched between station, access point and mesh point from the interface editor. The mode used to be fixed when the interface was created +- OSPF sometimes learned no routes over a link after a routing change + was applied while the link went down and up: the adjacency came up + but the interface was stuck at network type Null and left out of the + router's LSA. Seen about once in thirty runs on virtual machines [relsup]: https://github.com/kernelkit/infix/blob/main/doc/releases.md [snmp]: https://www.kernelkit.org/infix/latest/snmp/ diff --git a/patches/frr/10.5.5/0004-lib-realize-a-config-created-interface-on-its-first-.patch b/patches/frr/10.5.5/0004-lib-realize-a-config-created-interface-on-its-first-.patch new file mode 100644 index 000000000..04e7c55ef --- /dev/null +++ b/patches/frr/10.5.5/0004-lib-realize-a-config-created-interface-on-its-first-.patch @@ -0,0 +1,62 @@ +From 13d16871ee1da466d42ab2bb72de3516de2ebb5d Mon Sep 17 00:00:00 2001 +From: Joachim Wiberg +Date: Tue, 29 Sep 2026 20:46:52 +0200 +Subject: [PATCH 4/4] lib: realize a config-created interface on its first + state update +Organization: Wires + +A daemon that names an interface in its config, e.g. ospfd with an +"interface e7" stanza, creates it as a placeholder before connecting +to zebra. Zebra sends INTERFACE_UP/DOWN and ADDRESS_ADD to every +client as they happen, also to one that has not yet received the +interface dump it asked for. zebra_interface_state_read() finds the +placeholder by name and gives it the ifindex, but only +zebra_interface_add_read() calls if_new_via_zapi(), so the if_real +hook does not run. An ADDRESS_ADD then finds the interface by index +and the daemon acts on an interface it was never told is real. + +For ospfd this leaves the ospf_interface with network type 0, "Null" +in show ip ospf interface: the type is filled in by the if_real hook +and only copied to the ospf_interface when it is created. Its +router-LSA then lacks the link, so no routes are learned across it +although the adjacency reaches Full. Seen when a link flap coincides +with an ospfd restart, FRRouting/frr#4178. + +A state update carries the same data as an add, so treat the first one +for a placeholder as the add. A delete for a placeholder then reads as +add and delete, which is what the dump would have given. + +Signed-off-by: Joachim Wiberg +--- + lib/zclient.c | 6 ++++++ + 1 file changed, 6 insertions(+) + +diff --git a/lib/zclient.c b/lib/zclient.c +index 48fb42872a..9a444dc041 100644 +--- a/lib/zclient.c ++++ b/lib/zclient.c +@@ -2851,6 +2851,7 @@ struct interface *zebra_interface_state_read(struct stream *s, vrf_id_t vrf_id) + { + struct interface *ifp; + char ifname_tmp[IFNAMSIZ + 1] = {}; ++ bool unreal; + + /* Read interface name. */ + STREAM_GET(ifname_tmp, s, IFNAMSIZ); +@@ -2864,8 +2865,13 @@ struct interface *zebra_interface_state_read(struct stream *s, vrf_id_t vrf_id) + return NULL; + } + ++ unreal = ifp->ifindex == IFINDEX_INTERNAL; + zebra_interface_if_set_value(s, ifp); + ++ /* Created from config ahead of INTERFACE_ADD, see FRRouting/frr#4178 */ ++ if (unreal && ifp->ifindex != IFINDEX_INTERNAL) ++ if_new_via_zapi(ifp); ++ + return ifp; + stream_failure: + return NULL; +-- +2.43.0 + diff --git a/src/bin/files.c b/src/bin/files.c index 5897fb64c..a44e91ef8 100644 --- a/src/bin/files.c +++ b/src/bin/files.c @@ -105,6 +105,8 @@ static int complete(const char *word) strlcpy(dir, word, sizeof(dir)); slash = strrchr(dir, '/'); + if (!slash) + return 0; base = word + (slash - dir) + 1; slash[1] = 0; diff --git a/src/bin/rename.c b/src/bin/rename.c index 4217e1bda..361e3115d 100644 --- a/src/bin/rename.c +++ b/src/bin/rename.c @@ -2,13 +2,13 @@ #include "config.h" #include +#include #include #include #include #include #include #include -#include #include "util.h" @@ -43,13 +43,12 @@ static int mkparent(const char *path) static int do_rename(const char *from, const char *to) { char *src = NULL, *dst = NULL; + int rc = 1, err; mode_t mode; - int rc = 1; src = cfg_adjust(from, NULL, sanitize); - if (!src || access(src, F_OK)) { - fprintf(stderr, "%s: %s: no such file, or not an allowed path\n", - prognm, from); + if (!src) { + fprintf(stderr, "%s: %s: not an allowed path\n", prognm, from); goto out; } @@ -59,17 +58,23 @@ static int do_rename(const char *from, const char *to) goto out; } - if (!force && !access(dst, F_OK) && !yorn("Overwrite existing file %s", dst)) - goto out; - if (mkparent(dst)) { fprintf(stderr, "%s: failed creating directory for %s: %s\n", prognm, dst, strerror(errno)); goto out; } - if (rename(src, dst)) { - if (errno == EXDEV) + err = renameat2(AT_FDCWD, src, AT_FDCWD, dst, force ? 0 : RENAME_NOREPLACE); + if (err && errno == EEXIST) { + if (!yorn("Overwrite existing file %s", dst)) + goto out; + err = rename(src, dst); + } + + if (err) { + if (errno == ENOENT) + fprintf(stderr, "%s: %s: no such file\n", prognm, from); + else if (errno == EXDEV) fprintf(stderr, "%s: %s and %s are on different file systems," " use copy and remove\n", prognm, src, dst); else diff --git a/src/confd/src/containers.c b/src/confd/src/containers.c index 8dabcc690..3bace13d9 100644 --- a/src/confd/src/containers.c +++ b/src/confd/src/containers.c @@ -80,7 +80,7 @@ static int add(const char *name, struct lyd_node *cif) FILE *fp, *ap; snprintf(script, sizeof(script), "%s.sh", name); - fp = fopenf("w", "%s/%s", _PATH_CONT, script); + fp = fopenfp(0700, NULL, "%s/%s", _PATH_CONT, script); if (!fp) { ERRNO("Failed creating container script %s/%s", _PATH_CONT, script); return SR_ERR_SYS; @@ -319,7 +319,6 @@ static int add(const char *name, struct lyd_node *cif) fprintf(fp, " %s", string); fprintf(fp, "\n"); - fchmod(fileno(fp), 0700); fclose(fp); if (lydx_is_enabled(cif, "enabled")) { diff --git a/src/confd/src/factory-default.c b/src/confd/src/factory-default.c index a78e83f0e..7af521ebf 100644 --- a/src/confd/src/factory-default.c +++ b/src/confd/src/factory-default.c @@ -18,11 +18,6 @@ static int factory_reset(sr_session_ctx_t *session, uint32_t sub_id, const char int factory_default_rpc_init(struct confd *confd) { - int rc; - - REGISTER_RPC(confd->session, "/ietf-factory-default:factory-reset", factory_reset, NULL, &confd->fsub); - return SR_ERR_OK; -fail: - ERROR("failed: %s", sr_strerror(rc)); - return rc; + return register_rpc(confd->session, "/ietf-factory-default:factory-reset", + factory_reset, NULL, &confd->fsub); } diff --git a/src/confd/src/factory.c b/src/confd/src/factory.c index 450163e9f..35730c382 100644 --- a/src/confd/src/factory.c +++ b/src/confd/src/factory.c @@ -35,10 +35,6 @@ static int rpc(sr_session_ctx_t *session, uint32_t sub_id, const char *xpath, int factory_rpc_init(struct confd *confd) { - int rc; - REGISTER_RPC(confd->session, "/infix-factory-default:factory-default", rpc, NULL, &confd->fsub); - return SR_ERR_OK; -fail: - ERROR("failed: %s", sr_strerror(rc)); - return rc; + return register_rpc(confd->session, "/infix-factory-default:factory-default", + rpc, NULL, &confd->fsub); } diff --git a/src/confd/src/ip.c b/src/confd/src/ip.c index c451e50f2..ce6a36d7f 100644 --- a/src/confd/src/ip.c +++ b/src/confd/src/ip.c @@ -19,8 +19,8 @@ int ifchange_cand_infer_ipv4_prefix(sr_session_ctx_t *session, const sr_val_t *v sr_error_t err = SR_ERR_OK; struct in_addr ina; uint32_t addr; - char *xpath; size_t cnt; + int len; if (!strstr(val->xpath, ":ipv4/address[") || fnmatch("*]/ip", val->xpath, 0)) return SR_ERR_OK; @@ -37,16 +37,12 @@ int ifchange_cand_infer_ipv4_prefix(sr_session_ctx_t *session, const sr_val_t *v else return SR_ERR_OK; /* class D/E, no default */ - xpath = strdup(val->xpath); - if (!xpath) - return SR_ERR_SYS; - *strrchr(xpath, '/') = 0; - - err = srx_nitems(session, &cnt, "%s/prefix-length", xpath); + /* Strip the /ip matched above to get the address list entry */ + len = strlen(val->xpath) - strlen("/ip"); + err = srx_nitems(session, &cnt, "%.*s/prefix-length", len, val->xpath); if (!err && !cnt) - err = srx_set_item(session, &inferred, 0, "%s/prefix-length", xpath); + err = srx_set_item(session, &inferred, 0, "%.*s/prefix-length", len, val->xpath); - free(xpath); return err; } diff --git a/src/confd/src/main.c b/src/confd/src/main.c index 1d0e9f137..12480ee54 100644 --- a/src/confd/src/main.c +++ b/src/confd/src/main.c @@ -421,8 +421,8 @@ static int maybe_migrate(const char *path) NOTE("%s config version %s vs confd %s, migrating ...", path, file_ver, CONFD_VERSION); - mkpath(backup_dir, 0770); - chown(backup_dir, 0, 10); /* root:wheel */ + if (mkpath(backup_dir, 0770) || chown(backup_dir, 0, 10)) /* root:wheel */ + WARN("Cannot create %s: %m", backup_dir); snprintf(backup, sizeof(backup), "%s/%s", backup_dir, basenm(path)); rc = systemf("migrate -i -b \"%s\" \"%s\"", backup, path); diff --git a/src/confd/src/support.c b/src/confd/src/support.c index 664d98ea9..089e0f711 100644 --- a/src/confd/src/support.c +++ b/src/confd/src/support.c @@ -71,9 +71,10 @@ static void strip_lf(unsigned char *str) /* Same mode as the tool gives it, our umask is stricter */ static const char *workdir(void) { - if (!mkdir(SUPPORT_DIR, 0755)) - chmod(SUPPORT_DIR, 0755); - else if (errno != EEXIST) + if (!mkdir(SUPPORT_DIR, 0755)) { + if (chmod(SUPPORT_DIR, 0755)) + WARN("Cannot set mode of %s: %s", SUPPORT_DIR, strerror(errno)); + } else if (errno != EEXIST) return SUPPORT_TMP; if (access(SUPPORT_DIR, W_OK)) @@ -375,10 +376,6 @@ static int rpc_collect(sr_session_ctx_t *session, uint32_t sub_id, const char *p int support_rpc_init(struct confd *confd) { - int rc = 0; - - REGISTER_RPC(confd->session, "/infix-system:support-collect", - rpc_collect, NULL, &confd->sub); -fail: - return rc; + return register_rpc(confd->session, "/infix-system:support-collect", + rpc_collect, NULL, &confd->sub); } diff --git a/src/klish-plugin-infix/src/infix.c b/src/klish-plugin-infix/src/infix.c index 5f506c7d6..57b8619aa 100644 --- a/src/klish-plugin-infix/src/infix.c +++ b/src/klish-plugin-infix/src/infix.c @@ -666,7 +666,8 @@ static void ensure_known_hosts(const struct passwd *pw) if (fd < 0) return; /* Already exists, or unrecoverable error */ - fchown(fd, pw->pw_uid, pw->pw_gid); + if (fchown(fd, pw->pw_uid, pw->pw_gid)) + unlink(path); close(fd); } diff --git a/src/statd/iface.c b/src/statd/iface.c index 8c5e55d45..5c772148f 100644 --- a/src/statd/iface.c +++ b/src/statd/iface.c @@ -99,14 +99,15 @@ static void iface_parse(struct iface_ctx *ctx, struct nlmsghdr *nlh, int dump) struct ifinfomsg *ifi = NLMSG_DATA(nlh); int len = nlh->nlmsg_len - NLMSG_LENGTH(sizeof(*ifi)); uint8_t operstate = IF_OPER_UNKNOWN; - const char *name = NULL; + char name[IFNAMSIZ] = ""; struct rtattr *rta; struct iface *l; for (rta = IFLA_RTA(ifi); RTA_OK(rta, len); rta = RTA_NEXT(rta, len)) { switch (rta->rta_type & NLA_TYPE_MASK) { case IFLA_IFNAME: - name = RTA_DATA(rta); + snprintf(name, sizeof(name), "%.*s", (int)RTA_PAYLOAD(rta), + (char *)RTA_DATA(rta)); break; case IFLA_OPERSTATE: operstate = *(uint8_t *)RTA_DATA(rta); @@ -114,7 +115,7 @@ static void iface_parse(struct iface_ctx *ctx, struct nlmsghdr *nlh, int dump) } } - if (!name) + if (!name[0]) return; if (nlh->nlmsg_type == RTM_DELLINK) { diff --git a/test/case/interfaces/lag_failure/test.adoc b/test/case/interfaces/lag_failure/test.adoc index eed973208..41b39ba8e 100644 --- a/test/case/interfaces/lag_failure/test.adoc +++ b/test/case/interfaces/lag_failure/test.adoc @@ -21,6 +21,7 @@ image::topology.svg[LACP Aggregate w/ Degraded Link topology, align=center, scal . Set up topology and attach to target DUTs . Set up link aggregate, lag0, between dut1 and dut2 +. Wait for LACP to synchronize both links . Initial connectivity check ... . Verify failure modes diff --git a/test/case/interfaces/lag_failure/test.py b/test/case/interfaces/lag_failure/test.py index c397d138a..09ab0e2b3 100755 --- a/test/case/interfaces/lag_failure/test.py +++ b/test/case/interfaces/lag_failure/test.py @@ -15,7 +15,7 @@ import infamy import infamy.lag from infamy.netns import TPMR -from infamy.util import parallel +from infamy.util import parallel, until IPH = "192.168.2.1" IP1 = "192.168.2.41" @@ -135,6 +135,10 @@ def dut_init(dut, addr, peer): parallel(lambda: dut_init(dut1, IP1, IP2), lambda: dut_init(dut2, IP2, IP1)) + with test.step("Wait for LACP to synchronize both links"): + until(lambda: all(infamy.lag.lacp_synced(dut, dut["link1"], dut["link2"]) + for dut in (dut1, dut2)), attempts=60) + with test.step("Initial connectivity check ..."): ns.must_reach(IP2, timeout=30) diff --git a/test/case/misc/support_collect/test.adoc b/test/case/misc/support_collect/test.adoc index 14e68ce05..037047d8c 100644 --- a/test/case/misc/support_collect/test.adoc +++ b/test/case/misc/support_collect/test.adoc @@ -1,6 +1,6 @@ === Support Data Collection -ifdef::topdoc[:imagesdir: {topdoc}../../misc/support_collect] +ifdef::topdoc[:imagesdir: {topdoc}../../test/case/misc/support_collect] ==== Description diff --git a/test/infamy/lag.py b/test/infamy/lag.py index 7a2cf5b13..02d2becc0 100644 --- a/test/infamy/lag.py +++ b/test/infamy/lag.py @@ -1,4 +1,4 @@ -from . import topology +from . import iface, topology def edge_mappings(les, pes): """Specialized topology edge mapper for LAG tests @@ -24,3 +24,18 @@ def links_compatible(candidate): for candidate in topology.edge_mappings(les, pes): if links_compatible(candidate): yield candidate + + +def lacp_synced(target, *ports): + """True when all ports are collecting and distributing, as seen by both ends""" + for port in ports: + data = target.get_data(iface.get_xpath(port)) or {} + for entry in data.get("interfaces", {}).get("interface", []): + # netconf presents lag-port, restconf prefixes it with the model + lagport = entry.get("lag-port") or entry.get("infix-interfaces:lag-port") or {} + lacp = lagport.get("lacp", {}) + for state in (lacp.get("actor-state", []), lacp.get("partner-state", [])): + if not {"collecting", "distributing"} <= set(state): + return False + + return True diff --git a/test/infix-test-skill.md b/test/infix-test-skill.md index a6a5f53a4..6fa981d64 100644 --- a/test/infix-test-skill.md +++ b/test/infix-test-skill.md @@ -15,3 +15,26 @@ Info about the Infix regression tests. and `provides` attributes, and prints the mapping in the log. A test is skipped when no mapping fits. - Logs: `test/.log//output/`. Full guide: `doc/testing.md`. + +## Debugging a local QEMU run + +- `make test-sh` keeps the DUTs and the `infamy0` container running after + a failure, `make test` tears them down. The DUTs are QEMU guests with + 384 MB RAM, one host tap per port (`d2a` is port a of dut2, and so on). +- The log maps the test's logical names to DUTs (`R1: dut2`) and prints + the mgmt address it connected to, e.g. `fe80::2a0:85ff:fe00:201%d2a`. +- Run commands on a DUT over SSH from inside the container, admin/admin: + + podman exec infamy0 sshpass -p admin ssh -o StrictHostKeyChecking=no \ + -o UserKnownHostsFile=/dev/null admin@fe80::2a0:85ff:fe00:201%d2a \ + 'vtysh -c "show ip ospf neighbor"' + + `admin` can `sudo -n` and is in `frrvty`, so `vtysh`, `/var/log/messages`, + `dmesg`, `initctl status` and `sysrepocfg -X -d operational -x ` + are all reachable this way. `test/console dut2` attaches to the serial + console instead. The system is Finit and sysklogd, there is no journal. +- Run a subset: list the tests in a yaml under `test/case/` with `case:` + paths relative to that directory, then + `make test TESTS=$PWD/test/case/subset.yaml`. Repeat an entry under + different names to loop a flaky test. `INFAMY_ARGS=--transport=netconf` + (or `restconf`) forces the transport, otherwise it is picked per run.