From eb1da31be749210a1e3527519d7a57e5d7d96a98 Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Mon, 28 Sep 2026 06:22:24 +0200 Subject: [PATCH 01/11] bin: rename without checking the files first Coverity Scan flags a time-of-check to time-of-use race (CID 564389) in rename: both files are checked with access(), and then renamed. The destination can appear after the check and be overwritten without asking, and the source can go away before the rename. Let the rename itself do the checking. RENAME_NOREPLACE fails with EEXIST if the destination exists, so we ask before overwriting, and a missing source is reported from ENOENT. Also guard the strrchr() in the path completion of files. It cannot return NULL for an absolute path, but Coverity cannot see that (CID 564393). Signed-off-by: Joachim Wiberg --- src/bin/files.c | 2 ++ src/bin/rename.c | 25 +++++++++++++++---------- 2 files changed, 17 insertions(+), 10 deletions(-) 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 From ffd2157f4ac5dfa252a023e8c1e10caae9d6bd3d Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Mon, 28 Sep 2026 06:22:24 +0200 Subject: [PATCH 02/11] confd: check chmod() of the support directory Coverity Scan reports two defects in the support-collect RPC: - CID 564390: the return value of chmod() on /var/lib/support is not checked, so a failure to set the mode goes unnoticed - CID 564391: identical branches, the REGISTER_RPC macro jumps to a fail label that is the next statement anyway Log a warning if chmod() fails, and return register_rpc() directly. Signed-off-by: Joachim Wiberg --- src/confd/src/support.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) 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); } From 5c37763d78673160414e42cd957a18b934bacbe4 Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Mon, 28 Sep 2026 06:22:25 +0200 Subject: [PATCH 03/11] statd: bound the interface name from netlink The name in IFLA_IFNAME was used as a string straight from the receive buffer, relying on the kernel to NUL terminate it. Coverity Scan reports it as an unterminated string (CID 564392). Copy it into an IFNAMSIZ buffer, bounded by the attribute payload. Signed-off-by: Joachim Wiberg --- src/statd/iface.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) 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) { From 0c901759782775c5ff67a23832630f08cc010557 Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Mon, 28 Sep 2026 06:28:41 +0200 Subject: [PATCH 04/11] confd: register the factory RPCs directly Both factory RPC init functions logged a failure to subscribe twice, once in register_rpc() and again after the jump to their fail label. Return register_rpc() directly, like the support-collect RPC. Signed-off-by: Joachim Wiberg --- src/confd/src/factory-default.c | 9 ++------- src/confd/src/factory.c | 8 ++------ 2 files changed, 4 insertions(+), 13 deletions(-) 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); } From 3f01a2784d08f789359908882de310f1d6087ade Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Mon, 28 Sep 2026 06:28:41 +0200 Subject: [PATCH 05/11] confd: check mode and owner of container script and backup dir A container script that could not be made executable went unnoticed until the container failed to set up: setup: /run/containers/NAME.sh does not exist or is not executable. Open the script with fopenfp(), which sets the mode and logs failure. Also warn if the backup directory for a configuration migration cannot be created or given to root:wheel. Signed-off-by: Joachim Wiberg --- src/confd/src/containers.c | 3 +-- src/confd/src/main.c | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) 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/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); From 7b3776530aa3d171f9fa61433ac21ac441d7da1e Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Mon, 28 Sep 2026 06:28:42 +0200 Subject: [PATCH 06/11] cli: remove known_hosts if it cannot be given to the user The ssh command creates ~/.ssh/known_hosts as root, then hands it over to the user. If fchown() failed, the file stayed owned by root, ssh could never add a host key to it, and it was never created again. Remove it on failure, so the next ssh command can try again. Signed-off-by: Joachim Wiberg --- src/klish-plugin-infix/src/infix.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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); } From 15681d95c89f81687552c15984eb9f14afa43135 Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Tue, 29 Sep 2026 18:26:10 +0200 Subject: [PATCH 07/11] confd: infer IPv4 prefix length without copying the xpath Coverity Scan reports the strrchr() that strips the ip leaf from a copy of the address xpath as a possible NULL dereference (CID 564405). It cannot be, the fnmatch() above only lets xpaths ending in /ip through. Drop the copy and print the parent xpath with a length limit instead. Signed-off-by: Joachim Wiberg --- src/confd/src/ip.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) 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; } From 60ca42322ed2a38e6a65c6a834ea41b8cd176284 Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Tue, 29 Sep 2026 19:22:09 +0200 Subject: [PATCH 08/11] test: lag_failure: wait for LACP to synchronize before breaking links The failure sequence started about a second after the aggregate was created, while LACP was still negotiating the second link. In QEMU that takes about nine seconds, so the first failover measured the rest of the negotiation: 7 to 9 s in a normal run, and past the 30 s ping budget in two CI runs on 2026-09-28: # block | forward | FAIL after 30.01s The second link never carried traffic before that either, which is why blocking it always passed instantly. Wait for both member links to be collecting and distributing on both DUTs before the first connectivity check. Failover then completes in about a second, and blocking the second link is a real failover. Signed-off-by: Joachim Wiberg --- test/case/interfaces/lag_failure/test.adoc | 1 + test/case/interfaces/lag_failure/test.py | 6 +++++- test/infamy/lag.py | 17 ++++++++++++++++- 3 files changed, 22 insertions(+), 2 deletions(-) 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/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 From 5c4a0b8e35458a2dc88839e6c96bc420d6e8ad07 Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Tue, 29 Sep 2026 19:53:08 +0200 Subject: [PATCH 09/11] test: adjust imagesdir for support collect Found with "make test-spec" Signed-off-by: Joachim Wiberg --- test/case/misc/support_collect/test.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 From c646dc490c3e39fb50bc2176208235b851c0f138 Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Tue, 29 Sep 2026 20:35:58 +0200 Subject: [PATCH 10/11] test: describe how to reach the DUTs of a local QEMU run The test skill said where the logs are, not how to get at a DUT that make test-sh leaves running, so debugging a failure meant finding the container, the mgmt address and the ssh incantation again every time. Signed-off-by: Joachim Wiberg --- test/infix-test-skill.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) 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. From 1a95529b4df663156b250ddebd630119e3d7a462 Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Tue, 29 Sep 2026 22:12:41 +0200 Subject: [PATCH 11/11] frr: OSPF learns no routes over an interface left at network type Null About one run in thirty of the OSPF tests in QEMU, one router had a full adjacency and a complete link state database but no routes from its neighbor, and the neighbor none from it. Its router-LSA lacked the link, and show ip ospf interface said "Network Type Null" for it. ospfd names the interface in its config, so it exists as a placeholder before ospfd connects to zebra. Zebra sends interface state and address events to every client as they happen, also to one that has not yet received the interface dump. A state event finds the placeholder by name and gives it the ifindex without running the hook that sets the default network type, the address event that follows creates the OSPF interface with type 0, and the dump arrives too late. A link flap during an ospfd restart does it, which is what applying a routing configuration involves. FRRouting/frr#4178. Treat the first state event for a placeholder like the interface add it stands in for. With the dump held back to keep the window open, 19 of 20 restarts hit it before, none after. Signed-off-by: Joachim Wiberg --- doc/ChangeLog.md | 4 ++ ...nfig-created-interface-on-its-first-.patch | 62 +++++++++++++++++++ 2 files changed, 66 insertions(+) create mode 100644 patches/frr/10.5.5/0004-lib-realize-a-config-created-interface-on-its-first-.patch 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 +