Skip to content

Release/v0.20 fixes#1644

Open
Fredi-raspall wants to merge 17 commits into
release/v0.20from
release/v0.20-fixes
Open

Release/v0.20 fixes#1644
Fredi-raspall wants to merge 17 commits into
release/v0.20from
release/v0.20-fixes

Conversation

@Fredi-raspall

Copy link
Copy Markdown
Contributor

Minimal log fixes for v0.20.2

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>
@Fredi-raspall
Fredi-raspall requested a review from a team as a code owner July 17, 2026 19:59
@Fredi-raspall Fredi-raspall added the ci:+release Enable VLAB release tests label Jul 17, 2026
@Fredi-raspall
Fredi-raspall removed the request for review from a team July 17, 2026 19:59
@Fredi-raspall Fredi-raspall added the ci:+vlab Enable VLAB tests label Jul 17, 2026
@Fredi-raspall
Fredi-raspall requested a review from mvachhar July 17, 2026 19:59
@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 90355969-194e-41c8-904e-edc15706cbf7

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

@qmonnet

qmonnet commented Jul 17, 2026

Copy link
Copy Markdown
Member

Do we have any sort of established process for backports? I guess not?

@qmonnet

qmonnet commented Jul 17, 2026

Copy link
Copy Markdown
Member

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

@Frostman

Frostman commented Jul 17, 2026

Copy link
Copy Markdown
Member

Do we have any sort of established process for backports? I guess not?

we have, it's been used for a while, but it's a big secret 😆

daniel-noland and others added 10 commits July 17, 2026 17:07
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>
daniel-noland and others added 2 commits July 17, 2026 17:30
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>
@mvachhar mvachhar added the dont-merge Do not merge this Pull Request label Jul 20, 2026
@mvachhar

Copy link
Copy Markdown
Contributor

@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?

@mvachhar

Copy link
Copy Markdown
Contributor

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>
@Fredi-raspall

Copy link
Copy Markdown
Contributor Author

Also, with the impending release, is this worth doing at this point?

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:+release Enable VLAB release tests ci:+vlab Enable VLAB tests dont-merge Do not merge this Pull Request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants