Repository navigation
v1.0 magic - #93
Merged
Merged
v1.0 magic#93
Conversation
NUD probes (RFC 4861 §7.7.3) are sent unicast and SHOULD omit SLLAO since the sender already knows the link-layer address. The previous code hard-returned when SLLAO was absent, so ndppd either dropped the probe entirely or responded with an unsolicited NA (SOLICITED=0) to ff02::1. An unsolicited NA does not satisfy a NUD PROBE transition (§7.3.5), causing the Juniper switch to declare the neighbor unreachable. Fix: pass the Ethernet source MAC through ndL_handle_ns and use it as the fallback src_ll when SLLAO is absent. This allows a properly solicited NA to be sent back to the unicast probe source. Also add NULL src_ll guards in session.c for the DAD (unspecified source) path, and fix the PID file write check which compared the return value of write() to 0 instead of the expected byte count, logging a spurious error on every successful daemonize.
The use-kernel config option conditionally called RTM_NEWNEIGH/ RTM_DELNEIGH to maintain kernel neighbor proxy entries alongside ndppd's own proxying. It was never promoted beyond experimental and is not used in any deployed config. The underlying nd_rt_add_neigh/ nd_rt_remove_neigh helpers in rt.c are retained.
iface.c: - Reject NS with multicast target address (RFC 4861 §7.1.1 MUST) - Discard ND packet containing any option with length zero (RFC 4861 §4.6 MUST) - Iterate all NS options to find SLLAO instead of only inspecting the first - BSD/BPF: fix msg pointer to use per-packet offset (was always buf base, corrupting all field reads and silently dropping all but first BPF record) - BSD/BPF: fix plen sanity check to use bpf_hdr->bh_caplen not total read len session.c: - STALE state: queue subscriber and trigger fresh NUD probe rather than immediately responding with OVERRIDE NA against unconfirmed reachability (RFC 4861 §7.3.3 — STALE means reachability is unknown) - VALID state: refresh state_time on incoming NA so gratuitous NAs from the target extend session lifetime (RFC 4861 §7.2.5) - STALE exponential backoff: guard nd_conf_retrans_limit == 0 modulo (config allows min=0, causing division-by-zero UB) - STALE exponential backoff: cap shift at 20 to prevent signed int overflow Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
PACKET_MR_ALLMULTI only enables all-multicast reception; unicast NS frames addressed to a MAC other than the interface's own are dropped by the NIC. This silently breaks proxying whenever an external host holds a stale neighbour-cache entry with an old container veth MAC and sends its NUD probe as a unicast to that address — ndppd never sees the NS and never replies. Running tcpdump worked around the problem because it sets PACKET_MR_PROMISC as a side-effect, making the NIC accept every frame. Switch to PACKET_MR_PROMISC (a strict superset of ALLMULTI) so that all NS packets reach ndppd regardless of their Ethernet destination. The existing BPF filter still limits userspace delivery to ICMPv6 NS/NA only, so there is no meaningful performance impact. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ode) In auto mode, if no route to the container exists when the first NS arrives, nd_session_create() sets state=INVALID immediately. All subsequent NS from the same requester add subscribers to this INVALID session, but nd_session_update() for INVALID only purges them after invalid_ttl — it never retries the probe. The external host gives up long before the TTL expires, producing the symptom of "no reply on new container start." When an NS arrives for an INVALID session that has no interface (i.e. went invalid because of a missing route, not a failed probe), re-check the routing table. If the route has appeared in the meantime, open the downstream interface, transition to INCOMPLETE, and start probing so queued subscribers get an answer once the container replies with NA. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
nd_iface_send_ns() was building the solicited-node Ethernet multicast destination as 33:33:tgt[12:15] — i.e. taking all four last bytes of the TARGET address. For EUI-64-derived container addresses, byte 12 is always 0xfe (the lower byte of the ff:fe EUI-64 marker), so ndppd was sending to 33:33:fe:XX:XX:XX instead of the correct 33:33:ff:XX:XX:XX. The solicited-node multicast Ethernet MAC is derived from the last four bytes of the IPv6 *destination* (ff02::1:ffXX:XXXX), not the target. That address has 0xff at byte 12, not 0xfe. The correct MAC is always 33:33:ff:tgt[13]:tgt[14]:tgt[15]. Practical impact: containers with EUI-64 addresses never received ndppd's NS probe, so they never replied with NA, and ndppd's iface-mode sessions stayed INCOMPLETE indefinitely. Running tcpdump happened to put the bridge/veth into promiscuous mode, which flooded the wrong-MAC frame to all ports and accidentally made it work. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The -v verbosity flag was comparing against ND_LOG_ERROR (0) — always
false — so it never did anything. Fix the comparison to ND_LOG_TRACE.
Default verbosity changed from TRACE to INFO so -vvv reaches trace.
Add nd_log_debug/trace at every silent return path so packet drops are
visible in -vvv output:
- unknown ifindex, short frame, ethertype, plen mismatch in io handler
- hop-by-hop truncation, non-ICMPv6 nxt, ICMPv6 checksum mismatch,
hop-limit != 255 in ndL_handle_msg
- non-proxy iface, short NS, multicast target in ndL_handle_ns
- no rule match in nd_proxy_handle_ns
- no session match for incoming NA in ndL_handle_na
- no src_ll (DAD / no SLLAO) in nd_session_handle_ns
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
After fork(), both parent and child share the same AF_PACKET socket. The parent's atexit handler (nd_iface_cleanup → nd_iface_close) was calling PACKET_DROP_MEMBERSHIP on the shared socket before the parent exited, silently removing the PROMISC membership that the child daemon still needed. Result: the daemon started without promiscuous mode and missed multicast NS packets until tcpdump happened to re-enable promisc. nd_iface_no_restore_flags already existed for exactly this purpose but was never set. Set it in the parent branch of ndL_daemonize() before exit(0), and guard the PACKET_DROP_MEMBERSHIP call in nd_iface_close() behind the flag so the parent's cleanup is a no-op on the socket. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This was referenced Oct 15, 2026
DanielAdolfsson
added a commit
that referenced
this pull request
Oct 15, 2026
Two layers, both emitting TAP 13: 219 unit assertions and 51 integration
assertions.
make check
make check-unit # no network, no privileges, no waiting
make check-integration # a real daemon on real interfaces
Unit tests link the production objects for addr, alloc, conf, log, proxy,
rule and session, and substitute test/unit/fake.c for iface.c, rt.c and
io.c -- the two places ndppd needs a kernel. The code under test stays the
shipped code, while three things become possible that are not possible
against a running daemon: every emitted NS and NA is recorded and can be
asserted on exactly; nd_current_time is an ordinary variable, so a thirty
second TTL transition is tested by assigning to it rather than by sleeping;
and nd_iface_send_ns() tolerates a NULL interface, so a path that would
segfault against the real iface.c is observable instead of fatal.
tap.h and tap.c are 150 lines of C99 and are the entire framework. ndppd is
expected to build with nothing but a compiler on platforms as old as the
ones in #67 and #73, and the tests must not be the reason that stops being
true.
addr.c gets the heaviest treatment -- every prefix length from 0 to 128
checked against an independent bitwise reference, written in a deliberately
different style so the two cannot share a bug. It is where a mistake is
easiest to make and hardest to see, and #67 is a report against exactly
that code.
Integration tests drive the binary, not an API, in three network
namespaces under `unshare -Urn` -- no root. Where unprivileged user
namespaces are unavailable the suite reports 1..0 # SKIP with the reason
rather than failing, and CI fails explicitly on that so a skip cannot
masquerade as a pass. Nothing in test/integration/ touches ndppd's
internals, so run.sh --binary <path> will test any build.
Three behaviours cannot be reached with ping and ip, so lib/ndtool.py is a
stdlib-only packet tool for them: the Ethernet destination of an outgoing
solicitation (a wrong solicited-node MAC still reaches a veth peer, which
has no hardware filter -- on a real bridge it is dropped); a unicast NUD
solicitation that omits the SLLAO as RFC 4861 7.7.3 says it should, which
Linux will never emit; and solicitations ndppd must discard.
Several cases are regression tests for specific fixes in #93, and they
discriminate -- reverting the solicited-node MAC computation to its pre-#93
form turns 080 red with `got 33:33:fe:00:ab:cd`.
Three changes outside test/ were needed to get here:
- The Makefile now carries the warning flags CMakeLists.txt always had.
ndppd reads packets out of __attribute__((packed)) buffers and takes the
address of fields inside them, so without -Wno-address-of-packed-member
the build the README tells you to run has been stopping at src/iface.c on
any current gcc. PR #94 found the same defect independently on master.
- A `uint32_t bits` in ndL_parse_block() was assigned and never read.
Harmless under gcc; under clang -Werror it failed the build outright, so
clang could not compile ndppd at all.
- nd_proxy_find() is split out of nd_proxy_create(), which already did the
search. It is what lets the config tests inspect what the parser built
rather than only whether it returned true.
Writing the suite turned up several real defects. This change fixes none of
them -- that is separate work, and mixing it in would make this
unreviewable. They are reproduced and pinned with TAP's TODO directive, so
they are tracked without turning CI red, and each is paired with an
assertion on *current* behaviour so a bug cannot change shape while it is
open.
The one that matters: a `static` rule segfaults the daemon. A static
session has no downstream interface, but nd_session_update() calls
nd_iface_send_ns(session->iface, ...) unguarded when VALID ages into STALE,
and nd_iface_send_ns() reads iface->lladdr as its first act. It needs
upstream traffic to reach, so a static rule nobody uses survives and one
carrying traffic dies about valid-ttl after it starts -- twelve seconds in
case 070.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
DanielAdolfsson
added a commit
that referenced
this pull request
Oct 15, 2026
…onfig path **README.** The rewrite that came with #93 is the base; this says the things it could not, because it was written for a fork. - A **Status** section that answers the question a visitor actually has. For six years this project's public answer was "experimental, no estimate for a beta", which is why the reporters in #71 concluded it was abandoned and moved to ndpresponder. The honest answer now is that 1.0 is in beta, is running in production, and is being worked on again. - **Credit to totoCZ**, in Status and in a Credits section. The rewrite sat untouched from June 2020 until they took it into production behind a Juniper switch, found everything wrong with it and sent the fixes back. Several are the kind of bug only a real network finds: a multicast MAC wrong by one byte, a socket membership dropped by the wrong process at fork, an NA correct in every respect except the one flag that would let a neighbour cache believe it. That is #93, and it is why this branch is worth finishing. - **A "Limitations" section.** ndppd does not route, does not enable forwarding, does not send RAs, is not a substitute for the kernel's neighbour table on its own host, is Ethernet-only, is defeated by MLD snooping, and needs its interfaces to exist at startup. That list is most of the open tracker -- #55, #69, #72, #77, #79 -- and #60 has been open since 2020 asking for exactly this. - **How to choose a rule mode**, which is the decision people most often get wrong, including what `static` really commits you to: a static /64 claims eighteen quintillion addresses on your provider's link. - `gmake` on BSD, which is #66. The Makefile opens with `ifdef` and the base system's make answers "Need an operator". **Man pages.** ndppd.conf(5) documented `invalid-ttl` as defaulting to 5000 when it is 10000, used `route eth0` where it meant `proxy eth0`, and was missing `keepalive`, `router`, `target` and `rewrite` entirely. Bounds are now stated for every numeric option, because the parser rejects out-of-range values rather than clamping them and the error does not say what the range was. ndppd(8) gained `--netns`, what the verbosity levels actually are, and a note about promiscuous mode and log buffering. **The default configuration path.** ndppd(8) documents /etc/ndppd.conf. The compiled-in default was the *relative* path "../ndppd.conf", and ndppd.service passes no -c at all -- systemd runs a service with its working directory at /, so the shipped unit looked for /ndppd.conf and the daemon exited. The default is now built from SYSCONFDIR so a packager who moves it moves it everywhere, and case 110 asserts the binary and its own man page name the same path, whatever it is. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
DanielAdolfsson
added a commit
that referenced
this pull request
Oct 15, 2026
Two layers, both emitting TAP 13: 219 unit assertions and 51 integration
assertions.
make check
make check-unit # no network, no privileges, no waiting
make check-integration # a real daemon on real interfaces
Unit tests link the production objects for addr, alloc, conf, log, proxy,
rule and session, and substitute test/unit/fake.c for iface.c, rt.c and
io.c -- the two places ndppd needs a kernel. The code under test stays the
shipped code, while three things become possible that are not possible
against a running daemon: every emitted NS and NA is recorded and can be
asserted on exactly; nd_current_time is an ordinary variable, so a thirty
second TTL transition is tested by assigning to it rather than by sleeping;
and nd_iface_send_ns() tolerates a NULL interface, so a path that would
segfault against the real iface.c is observable instead of fatal.
tap.h and tap.c are 150 lines of C99 and are the entire framework. ndppd is
expected to build with nothing but a compiler on platforms as old as the
ones in #67 and #73, and the tests must not be the reason that stops being
true.
addr.c gets the heaviest treatment -- every prefix length from 0 to 128
checked against an independent bitwise reference, written in a deliberately
different style so the two cannot share a bug. It is where a mistake is
easiest to make and hardest to see, and #67 is a report against exactly
that code.
Integration tests drive the binary, not an API, in three network
namespaces under `unshare -Urn` -- no root. Where unprivileged user
namespaces are unavailable the suite reports 1..0 # SKIP with the reason
rather than failing, and CI fails explicitly on that so a skip cannot
masquerade as a pass. Nothing in test/integration/ touches ndppd's
internals, so run.sh --binary <path> will test any build.
Three behaviours cannot be reached with ping and ip, so lib/ndtool.py is a
stdlib-only packet tool for them: the Ethernet destination of an outgoing
solicitation (a wrong solicited-node MAC still reaches a veth peer, which
has no hardware filter -- on a real bridge it is dropped); a unicast NUD
solicitation that omits the SLLAO as RFC 4861 7.7.3 says it should, which
Linux will never emit; and solicitations ndppd must discard.
Several cases are regression tests for specific fixes in #93, and they
discriminate -- reverting the solicited-node MAC computation to its pre-#93
form turns 080 red with `got 33:33:fe:00:ab:cd`.
Three changes outside test/ were needed to get here:
- The Makefile now carries the warning flags CMakeLists.txt always had.
ndppd reads packets out of __attribute__((packed)) buffers and takes the
address of fields inside them, so without -Wno-address-of-packed-member
the build the README tells you to run has been stopping at src/iface.c on
any current gcc. PR #94 found the same defect independently on master.
- A `uint32_t bits` in ndL_parse_block() was assigned and never read.
Harmless under gcc; under clang -Werror it failed the build outright, so
clang could not compile ndppd at all.
- nd_proxy_find() is split out of nd_proxy_create(), which already did the
search. It is what lets the config tests inspect what the parser built
rather than only whether it returned true.
Writing the suite turned up several real defects. This change fixes none of
them -- that is separate work, and mixing it in would make this
unreviewable. They are reproduced and pinned with TAP's TODO directive, so
they are tracked without turning CI red, and each is paired with an
assertion on *current* behaviour so a bug cannot change shape while it is
open.
The one that matters: a `static` rule segfaults the daemon. A static
session has no downstream interface, but nd_session_update() calls
nd_iface_send_ns(session->iface, ...) unguarded when VALID ages into STALE,
and nd_iface_send_ns() reads iface->lladdr as its first act. It needs
upstream traffic to reach, so a static rule nobody uses survives and one
carrying traffic dies about valid-ttl after it starts -- twelve seconds in
case 070.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
DanielAdolfsson
added a commit
that referenced
this pull request
Oct 15, 2026
…onfig path (#97) **README.** The rewrite that came with #93 is the base; this says the things it could not, because it was written for a fork. - A **Status** section that answers the question a visitor actually has. For six years this project's public answer was "experimental, no estimate for a beta", which is why the reporters in #71 concluded it was abandoned and moved to ndpresponder. The honest answer now is that 1.0 is in beta, is running in production, and is being worked on again. - **Credit to totoCZ**, in Status and in a Credits section. The rewrite sat untouched from June 2020 until they took it into production behind a Juniper switch, found everything wrong with it and sent the fixes back. Several are the kind of bug only a real network finds: a multicast MAC wrong by one byte, a socket membership dropped by the wrong process at fork, an NA correct in every respect except the one flag that would let a neighbour cache believe it. That is #93, and it is why this branch is worth finishing. - **A "Limitations" section.** ndppd does not route, does not enable forwarding, does not send RAs, is not a substitute for the kernel's neighbour table on its own host, is Ethernet-only, is defeated by MLD snooping, and needs its interfaces to exist at startup. That list is most of the open tracker -- #55, #69, #72, #77, #79 -- and #60 has been open since 2020 asking for exactly this. - **How to choose a rule mode**, which is the decision people most often get wrong, including what `static` really commits you to: a static /64 claims eighteen quintillion addresses on your provider's link. - `gmake` on BSD, which is #66. The Makefile opens with `ifdef` and the base system's make answers "Need an operator". **Man pages.** ndppd.conf(5) documented `invalid-ttl` as defaulting to 5000 when it is 10000, used `route eth0` where it meant `proxy eth0`, and was missing `keepalive`, `router`, `target` and `rewrite` entirely. Bounds are now stated for every numeric option, because the parser rejects out-of-range values rather than clamping them and the error does not say what the range was. ndppd(8) gained `--netns`, what the verbosity levels actually are, and a note about promiscuous mode and log buffering. **The default configuration path.** ndppd(8) documents /etc/ndppd.conf. The compiled-in default was the *relative* path "../ndppd.conf", and ndppd.service passes no -c at all -- systemd runs a service with its working directory at /, so the shipped unit looked for /ndppd.conf and the daemon exited. The default is now built from SYSCONFDIR so a packager who moves it moves it everywhere, and case 110 asserts the binary and its own man page name the same path, whatever it is. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This makes v1 production ready.