dpdk init - #1822
Draft
daniel-noland wants to merge 29 commits into
Draft
dpdk init#1822daniel-noland wants to merge 29 commits into
daniel-noland wants to merge 29 commits into
Conversation
init cleared the environment before starting the dataplane, passing only
RUST_BACKTRACE. Sealing the *configuration* into a memfd is what stops the
dataplane being reconfigured behind our back; it says nothing about the
ambient environment, and the dataplane needs that environment to work.
Putting init in the DaemonSet's `Command` made this visible immediately.
Three things broke at once, all quietly:
- `KUBERNETES_SERVICE_HOST` and `KUBERNETES_SERVICE_PORT` are how an
in-cluster client finds the API server. Without them the k8s client
could infer no configuration at all:
Failed to infer configuration: in-cluster: (failed to read an
incluster environment variable: environment variable not found)
Ten retries, then a gateway that never reports its status.
- `HOME` is how it finds a kubeconfig to fall back on, so the same error
named `/var/empty/.kube/config`, a path belonging to nobody, which
reads like a misconfiguration rather than a missing variable.
- `DATAPLANE_PYROSCOPE_URL` exists exactly because a controller owns
argv here -- a flag with no environment fallback is a flag nobody can
set -- so clearing the environment removed the only way to turn
profiling on.
RUST_BACKTRACE is now set only if the launcher did not set it, so an
operator who picked a level keeps it. The gateway DaemonSet sets FULL.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…runtime
The control plane runs in a network namespace of its own so that its taps
can carry the configured interface names and FRR can only see what the
dataplane means it to see. But not everything the dataplane does is
control-plane traffic: it watches a Kubernetes API server, serves a metrics
endpoint something outside scrapes, and pushes profiles to Pyroscope. A
private namespace is a place with no route anywhere, so until now all three
were unreachable and `--datapath-netns` with a control namespace was
`--config-dir` only.
`dataplane-init` now opens `/proc/self/ns/net` *before* it enters the
control namespace -- afterwards that path means the namespace it just
joined, and the way back is gone -- and passes it to the dataplane at
descriptor 60, beside the datapath's at 50.
The dataplane builds a second runtime whose threads `setns` into it in
`on_thread_start`, and runs the k8s client, the metrics endpoint and the
Pyroscope agent there. A namespace belongs to a *thread*, not to a future,
and tokio moves futures between threads as it pleases, so the unit of
placement has to be "every thread that could ever poll this", which is what
`on_thread_start` gives -- blocking-pool threads included, so
`spawn_blocking` is covered too.
Two places needed more than a different handle:
- `run_k8s` is reached through `Handle::block_on`, which polls its future
on the *calling* thread. The first poll opens the connection, so the
socket would have been created in the control namespace no matter which
runtime was named. Init is spawned onto the host runtime and awaited.
- The Pyroscope agent creates its own threads, and a thread inherits the
namespace of whichever thread made it. Building it on the main thread
put the entire agent in the control namespace, so it is built from a
host-runtime blocking thread instead.
`on_thread_start` cannot fail a runtime -- it returns nothing, and a thread
whose `setns` failed carries on serving from the wrong namespace. That is
silent, and its symptoms (connections that time out, a metrics endpoint
nothing can scrape) point nowhere near a namespace. So the runtime is asked
where it actually is, via a spawned task rather than `block_on` for exactly
the reason above, and a mismatch is fatal at startup where it can still be
read.
Where there is no split -- a dataplane run by hand, or one whose devices
stayed put -- no descriptor arrives, `host_handle` is the mgmt runtime's,
and every arrangement above is the one it always had.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three tests, under `#[n_vm::test]` because `setns` needs `CAP_SYS_ADMIN`
that a workstation test run does not have.
The first two assert an inequality as well as an equality. Checking only
that a task ran in the target namespace would also pass if `setns` had done
nothing and the two namespaces were the same to begin with -- which is the
failure being guarded against, so the test has to establish that the target
is somewhere else first.
The second covers `spawn_blocking` specifically. Its threads come from a
different pool than the workers, and the Pyroscope agent is built on one of
them, so a `on_thread_start` that missed that pool would leave the agent
pushing from the control namespace while every worker looked right.
The third is the reverse: handed the namespace the caller is already in,
the runtime must report agreement rather than mistake it for a failed
`setns`. It is the one case where a no-op and a success are genuinely
indistinguishable, and it should stay that way.
Break-tested by replacing the `setns` with a no-op: two of the three fail,
the third passes by design, and the failure arrives from `host_runtime`'s
own verification rather than from an assertion --
the host runtime's threads are in network namespace net:[4026531833],
not net:[4026532064]; setns did not take effect, so anything reaching
outside the fabric would be sent from the control namespace
which is the message an operator would get, so the break test checks the
diagnostic as well as the detection.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…idge
The control-plane bridge was DPDK-only, and that asymmetry was an accident
of which driver needed it first rather than a design. `cpbridge` has no
DPDK imports at all -- it moves from `drivers/dpdk/` to `drivers/` as a
pure relocation.
Under DPDK the separation is forced: on `vfio-pci` there is no netdev, and
on a bifurcated driver it was moved away. The kernel driver could leave its
interfaces where the control plane sees them and let `AF_PACKET` hand the
dataplane a copy, which is what it did. Three things argue against it:
- An interface the kernel still owns is one the kernel will route
through, answer ARP on and terminate connections on, with no dataplane
involvement. Keeping VXLAN traffic away from the host stack took
netfilter rules; moving the device out of that namespace removes the
thing those rules defend against.
- One punt policy instead of two. Both drivers now ask `disposition`, so
a new `DoneReason` forces the decision once rather than in two places
that can drift.
- The control plane stops caring which driver is running.
`dataplane-init` moves the interfaces with one `RTM_NEWLINK` carrying
`IFLA_NET_NS_FD` -- no driver reinit, no devlink instance to find, no RDMA
subsystem with an opinion. They lose their addresses and come back down,
which is correct: the dataplane drives them with `AF_PACKET` and does not
want the kernel configuring them, and the addresses the control plane cares
about belong on the taps that take their names.
Three placements matter, and each is a namespace-per-thread problem:
- Discovery and link setup run on a scratch thread with
`enter_with_sysfs`, because `netdev::get_interfaces` reads
`/sys/class/net` and sysfs is tagged with the namespace it was
*mounted* in. A scratch thread because `setns` and `unshare` are
per-thread and irreversible -- doing either in place would strand the
main thread, which the control plane still needs where it is.
- Each worker enters with a plain `setns` before it opens anything: an
`AF_PACKET` socket belongs to the namespace of the thread that made it.
No sysfs there; the workers bind by ifindex.
- Only worker 0 takes the bridge ends. Two workers draining one injection
queue would interleave a peering session's frames and reorder them.
`Kif` now carries the MAC and MTU the kernel reports, which the bridge
needs: a tap whose MAC does not match answers ARP with the wrong address,
and the peer's frames then arrive with a destination the interface will not
accept.
The gate is the namespace, not the driver. Either driver whose interfaces
moved needs the bridge; either driver whose interfaces stayed put must not
have one, because the taps would be created on top of the very devices they
are named after.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…place `--datapath-netns` moved the control plane into a fresh namespace of its own every time. That is right once init starts FRR, and wrong before then. FRR has to see the taps. It is configured by looking each interface up in the kernel, and it peers through them. When init starts FRR the namespace is shared automatically, because FRR inherits it. When FRR is a container of its own -- which is how it runs today, a separate DaemonSet on `HostNetwork` -- moving out of the shared namespace leaves it looking at an empty one. Config applies fail with "Unable to find kernel interface", the routing table never learns the interface, and no session ever comes up. So the rule is now explicit: a fresh control namespace when `--supervise-frr` says FRR is ours, the namespace `--control-netns` names when an operator arranged one, and otherwise stay where we started. Staying put does not make the datapath namespace pointless -- the two answer different questions. The datapath's takes the interfaces away from the host stack, which is what stops the kernel routing and answering ARP behind the dataplane's back, and it works on its own: the taps take the names the real interfaces have just vacated. The control plane's separates FRR from the rest of the host, which only starts mattering once FRR is ours. Staying also means there is no way back to ask for, so `host_netns` is `None` and the dataplane builds no second runtime -- which is exactly right, since it is already where its outward-facing work belongs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every packet was rejected as `InterfaceUnknown` the moment the control
plane's namespace was not a freshly created one. Found in vlab:
drops_by_reason{reason="interface_unknown"} 63
datapath namespace: enp2s1 = 3, enp2s2 = 4
control namespace: enp2s1 = 15, enp2s2 = 16 (the taps)
Interface indices are per-namespace. The port has one where the datapath
runs; the tap standing in for it has an unrelated one where the control
plane runs. Everything above the driver -- the configuration, the interface
table the ingress stage looks packets up in, the `oif` the router picks --
is built by looking interfaces up **by name in the control namespace**, so
all of it speaks in tap indices. Both drivers were stamping their own.
Neither had been caught, and the reason is worth recording: two freshly
created namespaces each number upwards from `lo`, so a single-port
dataplane gets `2` on both sides and works by accident. That is exactly the
shape of the hardware validation this design was signed off on. It only
fails once the control plane is somewhere that already has interfaces in it
-- the host's namespace, say, which is where it stays while FRR is a
container of its own.
So the bridge now learns each tap's index when it creates it, and both
drivers take their `iif`, their `oif` and their interface-table keys from
that. Where there is no bridge nothing moved, and a port's own index is
what the control plane saw too.
`if_nametoindex`, not netlink: `CpBridge::create` runs with the management
runtime *entered* on the calling thread, and `Handle::enter` alone is
enough to make `Handle::block_on` panic with "Cannot start a runtime from
within a runtime". A synchronous syscall has no such problem and resolves
in the calling thread's namespace, which is the one the tap was just made
in. The existing round-trip test caught that immediately.
Two things also had to be split rather than handed to one worker. Every
worker needs the tap's index, because every worker stamps received packets
with it, and every worker needs the punt sender, because any of them may
receive a frame the kernel should see. Only one may hold the injection
receiver. `BridgedPort` makes that split explicit instead of leaving it to
whoever calls `DatapathEnds::take` first.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
FRR is given these addresses -- they are rendered into its configuration as
`ip address` lines -- and zebra installs them faithfully on a physical
interface. It does **not** install them on a *tap* of the same name.
Measured in vlab, twice, including with an FRR started after the taps
already existed, so it is not a question of ordering:
physical enp2s1 in the host namespace: 172.30.128.9/31
tap enp2s1 standing in for it: fe80::.../64 only
Once the dataplane's interfaces move into a namespace of their own, every
interface the control plane can see is a tap, so that is every interface.
Without an address there is no connected route, nothing is recognised as
locally destined, and packets addressed to us are *forwarded* instead --
and since eBGP between directly connected peers uses a TTL of 1, they then
expire. What the lab shows is `hop_limit_exceeded` climbing and a session
stuck in `Active`. Nothing in that chain of symptoms mentions an address.
`InterfaceConfig.addresses` has been carried through the whole
configuration pipeline with **no consumer at all** -- populated from the
Gateway CR, rendered into FRR's config, and otherwise unread. This gives it
one, in the component that already creates the interfaces in question.
Applying the same addresses to a physical interface is harmless: they are
the ones zebra would install anyway, and an address already present comes
back `EEXIST` and is skipped.
It adds and never removes. An address the configuration has stopped
mentioning stays until the interface goes away, which for a tap is the next
restart. Removing would mean deciding that any address not in the
configuration is ours to delete, and on an interface the kernel still owns
that is not true; doing it properly means teaching the reconciler about
addresses, which is where this belongs in the end.
The interfaces are collected before the first await. The multi-index
iterators are not `Send`, and holding one across an await makes the whole
config processor's future non-`Send` -- a compile error reported a long way
from its cause.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three things the supervisor needed that only running it in a real image would have shown. `frr-agent`'s defaults for the reloader and for vtysh's directory -- `/hedgehog/frr-reload.py` and `/usr/local/bin` -- describe a Debian FRR container and name nothing that exists here. The `DaemonSet` being replaced passed both explicitly; the supervisor did not. It is a quiet failure: the agent starts, binds its socket, reports ready, and then fails every reload it is ever asked for. FRR's state directory is the second. Every path in the image is laid down read-only and owned by root -- `dataplane.tar` is tarred with `--mode='ugo-sw'` -- and FRR is root only long enough to drop to `frr`. After that it has to create `<state>/<daemon>.vty`, which is both how `watchfrr` decides a daemon is up and how the supervisor decides FRR is. So init creates and chowns it, which is a thing that can only be true at runtime: an image cannot carry an ownership it has no `chown` to apply, and a container runtime will not apply one either. The third is the sweep the `init-frr` container did. The state directory is a host path that outlives the pod, and a *stale* `zebra.vty` is worse than a missing one: FRR is declared up the instant it starts, the agent is released to configure a zebra that is not listening, and it surfaces as a configuration that did not apply rather than as a startup problem. Only the top level, and only FRR's own suffixes -- `hh/` below it holds the control-plane socket the dataplane has already bound by the time this runs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The gateway is one process tree now, so it has to be one image. `dataplane-init` supervises the dataplane, `watchfrr` and `frr-agent` together and enforces shared fate between them; it cannot fork a binary that lives in another container. FRR is laid down as a tree rather than reached through the store because it is configured with absolute image paths and nothing else will do: `--bindir=/bin`, `--libdir=/lib`, `--sbindir=/libexec/frr`, `--sysconfdir=/etc`, `--localstatedir=/run/frr`, `--with-moduledir=/lib/frr/modules`. zebra looks for `hh_dplane` at `/lib/frr/modules` and will not look anywhere else. A `buildEnv` composes it so the collisions between busybox's applets, coreutils and FRR's own binaries are resolved once, the same way `containers.frr.dataplane` already does -- less `tini`, since init is pid 1 here and reaps its own orphans. The tar's inputs become one deduplicated list. FRR's closure and the dataplane's overlap, and a path named twice is archived twice; `closureInfo` is also what knows about the python interpreter, which nothing links against and which `frr-reload.py` reaches only through its `#!` line. That interpreter is what `closure-check` was built to keep out, so this adds an exemption mechanism rather than widening the rule. The pattern still matches, the canary that proves it works still fires, and anything else matching it still fails the build -- what changes is that this one name is permitted, with the reason recorded beside it. Every exemption must be *used* or the build fails: an exemption nobody needs is a hole nobody is watching, and if FRR ever stops reaching for an interpreter this should be deleted at that commit rather than left to cover the next thing that reaches for one. Break-tested both ways: adding an exemption that matches nothing fails with "stale exemption", and removing this one fails with the interpreter forbidden. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The image's entrypoint was `/bin/dataplane`, from when it held one binary. It holds FRR now, and `dataplane-init` is what starts the whole thing. This also removes a landing-order constraint rather than adding one. init takes the same arguments as the dataplane and execs it when there is nothing else to do, so an orchestrator that names the dataplane's argv and no command -- which is every controller that predates this work -- gets exactly what it asked for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The gateway is one pod now, and that is not a change this repository can make on its own. Three repositories have to move together: * this one, for the merged image and init's supervision of FRR; * githedgehog/fabric#1608, for the controller that stops creating a second DaemonSet and deletes the old one; * githedgehog/fabricator, for a readiness check that no longer waits on it. The fabricator half is not optional and its absence is not obvious. `StatusFRR` looks up `gw--<node>--frr`, which no longer exists, so it reports NotFound for every gateway node forever. `IsGatewayReady` demands Ready. Nothing errors -- `IsReady` stays true, because it only asks for "not Unknown" -- and `hhfab vlab up --ready` sits printing `gatewayReady=false` until the job times out an hour later. Upgrade runs are parked. An upgrade from 26.03 starts with the old two-DaemonSet gateway and a host-namespace zebra, so it exercises the transition on top of everything else that is new. One variable at a time; a clean install first. All of this is experimental scaffolding pointing at branches. It reverts as one commit once the three halves land. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The lab jobs pin `<version>-release`, and `container-profiles` only reached for the release image on a pull request carrying `ci:+vlab` or `ci:+hlab`. A manual dispatch is not skipped by the vlab job's own logic, so it would start a lab against an image tag nothing ever built. It fails late and in the wrong place: the build matrix is green, and hhfab dies mirroring a missing tag, which reads as a registry problem rather than a missing build. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…o run a lab" This reverts commit 08ea3c9. The premise was wrong. `ci-gate` turns *on* for `push | merge_group | workflow_dispatch` before it ever looks at labels, so `container-profiles` emits its `on-value` -- `["debug", "release"]` -- on a dispatch already. The `off-value` expression is only consulted for pull requests, which makes the `workflow_dispatch` clause I added unreachable. Confirmed on the dispatched run: `check/release` and `coverage/fuzz` both scheduled, so `profiles` resolved to all three and `container-profiles` to both. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`containers.lgtm` and `containers.vlab` were not marked `source-volatile`, so their tarballs would have been pushed to Cachix -- a telemetry sink and a developer's toolbox, neither of which anybody would pull from there. Caught by `check-push-filter`, which is the gate for exactly this and had never run against this branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An SPDX header on `scripts/bf3-eswitch-reset.sh`, and MD049 emphasis style in `init/README.md`. Both are first-run findings: nothing on this branch had been through CI until now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`lint` still runs and still reports. It just no longer gates `build`, and so no longer gates vlab. The question this branch is asking is whether the merged gateway comes up in a CI lab. A markdown emphasis style or a missing SPDX header cannot answer it, and finding one costs a full round trip through a lab runner. The commits here are going to be rewritten wholesale before any of this lands, so the formality buys nothing now that it would not buy later, more cheaply. Experimental scaffolding; restore the dependency with the rest of it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`dpdk` denies `rustdoc::all`, and `PortLifecycle` is private, so the two links to it from `Stage` and `DevState::STAGE` -- both public -- fail the docs build. The name is still worth saying; it just has to be said in plain backticks. Another first-run finding: nothing on this branch had been through `check-docs`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The dataplane binds its control-plane socket *inside* that directory, at
`<state>/hh`, and it starts first. Preparing the directory afterwards means it
never starts at all:
router: Opening UNIX sock; target bind point is /var/run/frr/hh/dataplane.sock
panicked: failed to start router: InvalidPath("/var/run/frr/hh/dataplane.sock")
dataplane exited before it was ready (killed by SIGABRT)
Only a lab built from scratch could show this. `/run/frr` is a host path that
outlives the pod, and any machine that has run the old two-container gateway
already has `hh/` in it, left behind by the `init-frr` container this work
replaces. Roll a new image onto such a machine and it works; install onto a
fresh one and the gateway crashloops.
Reading the daemon list moves up with it, which is worth having for its own
sake: an FRR install missing zebra should be a refusal to start rather than a
discovery made after the datapath is already up.
The bind error keeps its errno now. `map_err(|_| InvalidPath(path))` threw it
away, so a missing parent directory, a permission problem and an address already
in use all printed as the path and nothing else -- which is what turned a
one-line diagnosis into a log dig.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four changes, all of them about turning a dispatch into "build an image and run a lab" instead of "run everything this repository knows how to run". `ci-gate` no longer treats `workflow_dispatch` as a deep run. That one line was turning on miri, the sanitizers, cross, concurrency, wasm and all three profiles before the lab it is actually asking about even started. Off, a dispatch looks like an unlabelled pull request. `check` and `coverage` are parked. Neither can answer whether the merged gateway comes up. Last run: 1973/1973 tests passed, `check-docs` failed on a private intra-doc link since fixed, and `coverage` died in an unrelated `n-vm-initramfs` zstd race that wants chasing on its own. The lab runs debug images now. A release build's panic report is seventeen frames of `<unknown source file>`, which is how the last failure cost a log dig rather than a glance. The `validator`/`debug` exclusion is lifted with it, because fabricator derives the validator's tag from `Versions.Gateway.Dataplane` -- one field for both -- so a `-debug` dataplane sends hhfab looking for a validator tag nothing built. And the matrix throttle takes a dispatch in its fast arm. It exists so pull requests cannot crowd out merge gating; that is not a concern here, and serialised matrices were most of the wall clock between a dispatch and a result. All of it reverts with the rest of the experimental scaffolding. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Proof-of-concept work to de-risk the DPDK driver's path to production and to make a fair kernel-vs-DPDK benchmark possible. One WIP commit covering five separable pieces; it wants splitting into a series before it is reviewable. - RSS was off, so every frame landed on receive queue 0 and one worker did all the work however many were configured. Measured on a BlueField-3: 4 workers, 40k frames, 9941/10050/10054/9955. - `Dev::stats()` was read only by a hardware test, so `imissed`/`rx_nombuf` were invisible. Eight `port_*` counters are now polled by the DPDK supervisor and exported. - Workers were plain threads reporting `LCORE_ID_ANY`, for which `rte_mempool_default_cache` returns NULL -- so every alloc and free went to the shared ring under atomics. Registration is now a `!Send + !Sync` capability token gating the APIs that require it. - NAT checksums are updated incrementally (RFC 1624) rather than by re-summing the payload. This half is backported separately as it benefits the kernel driver equally. - Six hardware probes, so these findings fail loudly on a DPDK bump rather than rotting. Signed-off-by: Daniel Noland <daniel@githedgehog.com>
DPDK is NUMA-aware -- the EAL links libnuma and calls numa_set_preferred before faulting each hugepage -- but it can only consume pages the kernel has already reserved. It cannot create them. That gap matters more than it sounds, because numa_set_preferred is MPOL_PREFERRED: when the preferred node has no free pages the kernel satisfies the allocation from another node in silence. Nothing fails and nothing warns, and the datapath then runs with its mbufs a hop away from the NIC. For a benchmark that is the worst available outcome, because the result looks like a result. So dataplane-init now reserves the pages itself, once it knows which node the configured devices are on: compact, try 4x1GiB, and on a shortfall compact again and fall back to 4GiB of 2MiB pages. What it actually secured rides to the dataplane in the launch configuration, which asks the EAL for exactly that with --numa-mem. A silent cross-node fallback becomes a loud startup failure. Every step is best-effort by design. Without CAP_SYS_ADMIN the sysfs files are read-only, and on a host that has been up for a while a 1GiB reservation usually fails however much memory is free, since a gigabyte page needs a gigabyte of physically contiguous, gigabyte-aligned memory. Neither is worth refusing to start over: the dataplane runs on 2MiB pages, and --in-memory means it needs no mount at all. What is owed is an honest log line, and a warning naming hugepagesz=1G on the kernel command line as the reliable fix. Two details the reservation depends on. The pool is grown by the shortfall rather than set to the requested figure, because nr_hugepages is the total and writing the bare number could shrink a pool something else is holding. And free_hugepages is read back afterwards, because the kernel gives less than asked without saying so, which makes a successful write no evidence at all. --numa-mem is the DPDK 26.07 spelling; --socket-mem survives as an alias. init/examples/hugepage_probe is read-only and reports what this host would offer, so the reservation can be checked on a benchmark machine before it is relied on. Signed-off-by: Daniel Noland <daniel@githedgehog.com>
hugetlbfs treats `size=` as a hard ceiling on the mount, not as a reservation. The 2 MiB mount carried `size=128M`, so however many pages the kernel had, only 128 MiB of them were reachable through it -- and the symptom is an allocation failure on a host with thousands of free pages, which reads as the host being short rather than as a mount option. Both caps are dropped. The pool is the only limit that should apply. Also adds DATAPLANE_HUGEPAGE_RESERVE=off. The reservation touches host state and pins the EAL to a figure, so when a lab run dies somewhere in memory setup the first question is whether this is the cause; answering that should not need a rebuild. The probe added here measures what --numa-mem actually does, and the answer corrects the claim made when it was introduced: asking for 8192 MiB against a 4096 MiB pool still returns success, because the EAL reserves address space and faults pages in lazily. --numa-mem does not make a short pool a startup failure. It still places the memory on the right node, which is what it is for, but it is not the loud gate it was described as. Signed-off-by: Daniel Noland <daniel@githedgehog.com>
The lab failed with `EAL: Cannot init memory` after this chose 1 GiB pages on a host whose gigabyte pages it was never allowed to touch. The log said "preallocate 4096 MiB in 1 GiB pages" on a machine set up with 2 MiB ones. The mistake was the oracle. `free_hugepages` reports the *host* pool, and inside a container that is not what binds: Kubernetes treats hugepages as a scheduled resource, so a pod that asked for hugepages-2Mi gets hugetlb.1GB.max = 0 and may not touch a gigabyte page however many the host has free. Reading sysfs, seeing four, and reserving nothing because they already looked free is how a size the EAL cannot use got chosen, and DPDK reports that as an out-of-memory on a machine with thousands of pages to spare. So ask the kernel instead of a counter. page_size_is_usable creates a hugetlb-backed memfd of that size, maps it and faults one byte -- the same path DPDK takes under --in-memory, and the point where a cgroup limit, an empty pool or a namespace actually says no. A size that fails is skipped before any counter is consulted. Two smaller corrections fall out. A short result at a large page size now falls back to a smaller one instead of being accepted; taking it was what let a partially-satisfiable figure reach the EAL. And the probe releases what it takes, which a test asserts by reading free_hugepages either side -- a probe that leaked a page per call would drain the pool it measures. Verified on a host with both pools free: both sizes report usable and neither leaks. The cgroup-forbidden case, which is the one that broke the lab, cannot be reproduced here because this machine has no hugetlb controller enabled. Signed-off-by: Daniel Noland <daniel@githedgehog.com>
The first version passed an empty device list, so reserve_for returned None because there were no nodes to reserve on -- whichever way the hatch behaved. Disabling the hatch entirely left both tests green. They now pass a device, and a third test asserts that without the variable the same call *does* produce a plan. That one is the guard: if this host ever stops having usable hugepages, the other two stop being able to tell the hatch from the absence of pages, and it says so rather than passing. Signed-off-by: Daniel Noland <daniel@githedgehog.com>
`move_devices_to_netns` documented this precondition and claimed to check it, but the only mention was a hint inside the error raised when the devlink reload *fails*. Shared mode is precisely the case where the reload succeeds: `_ib_alloc_device` discards the requested net, so the devlink instance moves while the RDMA device -- the half the mlx5 PMD attaches through -- stays in init_net. The message never printed in the situation it described, and the symptom was an empty device list much later, which is also what absent hardware looks like. The mode is now read from ib_core's netns_mode parameter before any device is moved, and shared mode is a refusal naming ib_core.netns_mode=0. The refusal says why a runtime change is not an option: `rdma system set netns exclusive` is permitted only while no network namespace but the initial one exists, which on a node running containers is never. An unreadable parameter warns rather than refuses -- failing to start over something we cannot read would be worse than the empty list. Parsing is split from the read so both answers are testable. A machine can only be in one mode, and the arm that matters is `Y`: it is the kernel default, so it is what a host nobody configured reports. A break test in that arm passed until this split, because the workstation reads `N`. The hugepage tests are also folded into one. They observe and mutate the kernel's pool, which nextest's process-per-test does not isolate: a second test calling reserve_for takes a page while the first is counting, and the leak assertion fails for an unrelated reason. That flake passed when run alone, which is how it was found. Signed-off-by: Daniel Noland <daniel@githedgehog.com>
`try_reserve` returned `free_now` when the pool already held enough, so the figure handed to the EAL was however many pages the host happened to have free rather than the four gigabytes the datapath wanted. A lab node with 3584 free 2 MiB pages produced `--numa-mem=7168` against a cgroup that permitted 4096, and DPDK failed in rte_eal_memory_init asking for memory it was never going to be given. Every return from `try_reserve` now goes through `claimable`, which is `min` and exists mostly to be testable: the over-claim is invisible on a host whose free pool happens to match what was wanted, which is every machine I can run this on. Also bounds the request by the hugetlb cgroup limit where one is legible. free_hugepages describes the host; inside a container Kubernetes hands the pod a slice of that, and the pod may not exceed it however much the host has spare -- so a plan built from sysfs alone can be honest about the machine and still wrong about this process. An unreadable or `max` limit leaves the host figures standing on their own. Signed-off-by: Daniel Noland <daniel@githedgehog.com>
`--numa-mem` places memory across NUMA nodes. With one node there is nowhere to place it wrongly, so the flag can only constrain the EAL and never help it -- while every failure this code has caused was in computing the figure it carries. On one node there is nothing to weigh against that, so the EAL is left to allocate lazily within whatever the cgroup permits. The decision moves into `skip_reason`, which names why rather than returning a bare `None`. `None` from `reserve_for` covers several unrelated situations, and a test that only sees `None` cannot tell the escape hatch from a machine with nothing to place -- which is the vacuity that let a broken hatch pass its own test once already. The hatch is checked first so it works on any machine, including one where the reservation would have been skipped anyway. Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Only CI can push images, so every one-line fix to hugepage or namespace handling costs a full round trip before it can be tried against the hardware that is the only thing able to falsify it. Three things were being paid for on each of those trips and none of them answered the question being asked. Images are release-only. The matrix was building debug as well, doubling the wait for an artifact this loop does not install. Note the tag changes with it: the images are `v0-<sha>-release`, and anything pointing at `-debug` will find nothing. `lint` is parked, like `check` and `coverage` before it. It already did not gate the images; this is about the runner it occupies. These commits will be rewritten wholesale before any of this lands, so the lint that matters is the one run over the rewritten series. `vlab` is parked, which deserves a reason on a branch named for it: the question has moved. vlab answers whether the gateway comes up in a virtual lab, and the failures in hand -- a hugetlb cgroup limit, the RDMA namespace mode -- are properties of a physical host that a virtual lab does not have and cannot reproduce. Until the physical run gets past EAL startup, a pass would tell us nothing and a failure would be about something else. All three are scaffolding and are marked as such. `summary` already treats a skipped job as success, so nothing starts reporting green that was not. Signed-off-by: Daniel Noland <daniel@githedgehog.com>
The refusal added a commit ago was synthetic. It rested on an inference from a doc comment, not on a measurement, and the inference did not carry as far as the refusal did. What the comment establishes is that in shared mode `_ib_alloc_device` discards the requested net, so the devlink instance moves while the RDMA device stays in init_net. That is a claim about where the device lives, not about whether it can be reached. Shared mode also means every RDMA device is visible from every namespace -- so if the kernel does not namespace-tag them in that mode, the datapath finds the device exactly as it would have in init_net and the arrangement works. Both readings fit what is written down. Measured what was measurable: on an exclusive-mode host, a fresh network namespace with a fresh sysfs lists no infiniband devices at all, so the class is tagged there and visibility follows the device's net. Whether that tagging still applies under shared mode is the part that decides this, and it cannot be answered on a host that is not in shared mode. A hard refusal would have stopped the only kind of machine that could settle the question from starting at all, in order to prevent a failure that is now understood and clearly reported when it occurs. So it warns, names what to look for, and lets the run proceed. Signed-off-by: Daniel Noland <daniel@githedgehog.com>
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueComment |
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
1 participant
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.
scratch: don't yet review please
🤖 Generated with Claude Code