Release/v0.20 fixes#1644
Conversation
Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
Some smart nics are known to accept frames larger than the standard jumbo size. Enlarge the reception buffer in the kernel driver to accommodate for that. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
|
Do we have any sort of established process for backports? I guess not? |
|
We'll need to figure out something for CI, in the future 😕 This change is simple enough, but generally, not being able to test backports in CI is annoying |
we have, it's been used for a while, but it's a big secret 😆 |
nixpkgs added the cmake-allow-overriding-sysusers.d-install-directory patch, which turns SYSUSERS_DIR into a CMake cache variable. Our pinned githedgehog/rdma-core fork already cherry-picked that exact upstream commit, so applying the nixpkgs patch on top fails patchPhase with "Reversed (or previously applied) patch detected" and aborts the build. The breakage is profile-independent but only surfaces on uncached derivations; common profiles were served from the binary cache and never re-ran patchPhase, so it first showed up under 'just profile=fuzz sanitize=thread test'. Filter that one patch out by name (rather than clearing the whole list) so any future nixpkgs patches we do want still apply. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
The just language changed to make our previous || hack illegal. The fix is very simple. Signed-off-by: Daniel Noland <daniel@githedgehog.com>
We had the following error for dependency updates:
$ bash scripts/bump.sh
++ dirname scripts/bump.sh
+ pushd scripts/..
/var/home/qmo/dev/dataplane /var/home/qmo/dev/dataplane
+ npins update
Error: Failed to deserialize sources.json
Caused by:
Version 7 is too old, you need to run upgrade
To address it, we simply ran the following command and committed the
changes:
$ npins upgrade
Signed-off-by: Quentin Monnet <qmo@qmon.net>
Backporting bug fixes / security fixes to this release cause some minor build failures in cargo-llvm-cov. We rebuild cargo-llvm-cov only to swap it onto our pinned rust-toolchain; the upstream package is already tested in nixpkgs. Its bundled test suite is sandbox-fragile against our newer cargo: `clean_ws` trips cargo >=1.97's `CACHEDIR.TAG` guard on `cargo clean`, and the `ui`/`trybuild` tests need a `trybuild` dep that isn't in the offline vendor dir. None of these reflect a defect in the built binary, so skip the check phase for this override. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
These were addressed in the previous two bumps. Signed-off-by: Daniel Noland <daniel@githedgehog.com>
We only ship on Linux (and a wasm32-wasip1 build), so pin cargo-deny's graph to those targets. This keeps Windows/macOS-only crates out of the advisory, license, and ban checks where they are dangerous noise. With the graph scoped, the instant (RUSTSEC-2024-0384) advisory no longer fires, so drop its now-unnecessary ignore entry. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
CI currently blocks on the "cargo deny check" step because of advisory RUSTSEC-2026-0173: dependency proc-macro-error2 is now unmaintained. This is not a direct dependency, it's pulled by multi_index_map_derive and our own fixin crate. Until both have transitioned to an alternative solution, we'll ignore this advisory. Link: https://rustsec.org/advisories/RUSTSEC-2026-0173 Link: githedgehog/fixin#2 Link: lun3x/multi_index_map#54 (comment) Signed-off-by: Quentin Monnet <qmo@qmon.net>
Just pulling in security / bug fixes from upstream Signed-off-by: Daniel Noland <daniel@githedgehog.com>
We're about to update Rust to v1.97.0, and apparently this comes with
new Clippy warnings:
error: truncating to zero length
--> cli/bin/main.rs:40:9
|
40 | answer.truncate(0);
| ^^^^^^^^^^^^^^^^^^
= help: use `clear()` instead
Backport of upstream commit fb468b3. The original commit also fixed a
`useless_borrows_in_formatting` report in config/src/display.rs, but that
line has since been rewritten on this branch and no longer triggers the
lint, so only the cli hunk applies here.
Signed-off-by: Quentin Monnet <qmo@qmon.net>
(cherry picked from commit fb468b3)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Drop manual_assert_eq, redundant references in formatting, and implicit format-arg captures flagged under `-D warnings`. `#[allow(dead_code)]` the unused egress helpers (kept for the future egress NF rework). Precursor for the threading rewrite stack so `just lint` is green throughout. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com> (cherry picked from commit da9ca1d) Signed-off-by: Daniel Noland <daniel@githedgehog.com>
3877cb3 to
a9e62aa
Compare
Rust 1.97's clippy::manual_option_zip flags the `ip_source().and_then(|src| ip_destination().map(|dst| (src, dst)))` pattern. Behavior-preserving rewrite to `ip_source().zip(ip_destination())`. Unlike the other 1.97 lint fixes on this branch, there is no upstream commit to cherry-pick: main rewrote the stateless NAT module entirely (static_nat/masquerade split) and this code path no longer exists there, so this fix is local to release/v0.20-fixes with no maintenance drift. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Backport of upstream commit ddae59e. Our branch pinned FRR 10.5 while dplane-plugin (rev 19d17cdba) already targets the 10.6 API, causing the frr container build to fail: hh_dp_msg.c:100: error: incompatible pointer types initializing 'const struct ipaddr *' with an expression of type 'const struct in_addr *' const struct ipaddr *vtep_ip = dplane_ctx_mac_get_vtep_ip(ctx); Bumps frr (stable/10.6), frr-dp (hh-master-10.6), and relabels the dplane-plugin pin branch to match. The frr FRRouting pin was resolved to main's 10.6 revision (our 10.5 revision had floated independently). Verified locally: containers.frr.dataplane builds and hh_dplane compiles. Signed-off-by: Fredi Raspall <fredi@githedgehog.com> (cherry picked from commit ddae59e) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
a9e62aa to
768c64c
Compare
|
@Frostman had some concerns about backporting some of these fixes, can we please revisit and have the minimal set of fixes that we are comfortable backporting to the release branch? |
|
Also, with the impending release, is this worth doing at this point? |
Router macs are learnt separately from evpn routes. These are sent before the routes using them. The same occurs on deletions, e.g. when the bgp session of a leaf switch is torn down. Dataplane resolves next-hops, not routes, and does it pretty agressively to keep a correct state, even on route deletions, because next-hops are shared. When a leaf-switch bgp session (even one the gateway does not participate in) flaps or is torn down, the router macs get deleted, so are the routes. This means that dataplane may attempt to resolve next-hops (with encap info) referring to a vtep/irb for which the router mac is no longer known, producing multiple warnings. Omitting the resolution on route deletions is not viable since our resolution is per next-hop, not per route. Solve this by keeping the router macs for some time interval, (instead of deleting them immediately), so that next-hops (which may end up disapparing anyway) are still correctly resolved. This should not cause any operational issue because if router macs are gone, so are the routes using them later on. So, temporarily keeping the macs does not harm, and up-to-date router macs may be re-established when a (far) bgp session comes up again. The solution marks router macs as "stale" temporarily until they get removed or restored. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
Invalidate next-hops if they have vxlan encapsulation and we fail to resolve the rmac. Such next-hops are retained and used. However, their packet instructions are set to drop the traffic. We should very rarely see an invalid next-hop. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
Impl Display for Mac such that formatting is preserved. We use ArrayString in stack so that no allocation is needed, using the fact that a mac, as a hex string, has a fixed length. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
The condition to maintain v0.20 was "if it is not too costly". In addition to the idea of just changing the bare minimum, I even wonder if we should pin the rust compiler. It appears that several commits were just needed due to stricter lints in a more recent compiler. That requires extra effort to address them and introduces more changes. |
Minimal log fixes for v0.20.2