Skip to content

fix underlay forwarding #1209

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
qmonnet merged 10 commits into from
Jan 20, 2026
Merged

fix underlay forwarding #1209

qmonnet merged 10 commits into from
Jan 20, 2026

Conversation

@Fredi-raspall
Copy link
Contributor

@Fredi-raspall Fredi-raspall commented Jan 19, 2026 •
edited
Loading

This goes on top of #1198 although it is not too related.
However, I needed to add it there to be able to test the latest changes around defaults.
Fixes #1210

@Fredi-raspall Fredi-raspall requested a review from a team as a code owner January 19, 2026 21:50
@Fredi-raspall Fredi-raspall changed the title fix underlay routing fix underlay forwarding Jan 19, 2026
qmonnet
qmonnet approved these changes Jan 19, 2026
View reviewed changes
Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, I like the last commit in particular.

@Fredi-raspall
Copy link
Contributor Author

Fredi-raspall commented Jan 20, 2026

Nice, I like the last commit in particular.

Thanks 😍 .

I've added two clean-up commits. They are no too related (I can make another PR if needed), which, if approved, should be merged soon to avoid conflicts.

qmonnet
qmonnet reviewed Jan 20, 2026
View reviewed changes
Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I've got mixed feelings about the crate renaming, flow-entry makes sense to me but I fear there will be confusion between flow-entry and flow-info. But I suppose we can always rename again in the future if it's too confusing. It will be nice indeed to have the two closer to each other in the list.

Cargo.toml Outdated
net = { path = "./net", package = "dataplane-net", features = [] }
pipeline = { path = "./pipeline", package = "dataplane-pipeline", features = [] }
pkt-meta = { path = "./pkt-meta", package = "dataplane-pkt-meta", features = [] }
flow-entry = { path = "./flow-entry", package = "dataplane-flow-entry", features = [] }
Copy link
Member

@qmonnet qmonnet Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Move up in the list, to keep them sorted

Copy link
Contributor Author

@Fredi-raspall Fredi-raspall Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@qmonnet
Copy link
Member

qmonnet commented Jan 20, 2026

CI complains about a few remaining includes referring to pkt-meta, it seems

@Fredi-raspall
Copy link
Contributor Author

Fredi-raspall commented Jan 20, 2026

CI complains about a few remaining includes referring to pkt-meta, it seems

Yup, just saw it. Fixing ...

@Fredi-raspall
Copy link
Contributor Author

Fredi-raspall commented Jan 20, 2026

Looks good. I've got mixed feelings about the crate renaming, flow-entry makes sense to me but I fear there will be confusion between flow-entry and flow-info. But I suppose we can always rename again in the future if it's too confusing. It will be nice indeed to have the two closer to each other in the list.

Agreed, I wonder if we should merge them. We could also rename flow-entry to flow-table.

@qmonnet
Copy link
Member

qmonnet commented Jan 20, 2026

I wonder if we should merge them.

I thought about it, but ask @mvachhar first, I think he had a reason for keeping them split at the time.

flow-table might avoid some of the confusion, it's easier to distinguish between information for the flows and a whole table, compared to information and just entries, I find

@Fredi-raspall Fredi-raspall force-pushed the pr/fredi/fix_underlay branch 2 times, most recently from 2ee5a19 to f35885f Compare January 20, 2026 14:02
@Fredi-raspall
Copy link
Contributor Author

Fredi-raspall commented Jan 20, 2026 •
edited
Loading

I wonder if we should merge them.

I thought about it, but ask @mvachhar first, I think he had a reason for keeping them split at the time.

flow-table might avoid some of the confusion, it's easier to distinguish between information for the flows and a whole table, compared to information and just entries, I find

Yeah, although it has a submodule called flow-table. If nothing else needs to go in that crate I'd remove the submodule and call it flow-table. Let's see what @mvachhar thinks about it. Probably the reason not to include the flow-info was to avoid some circular dependency?

Base automatically changed from pr/fredi/expose_refined to main January 20, 2026 15:41
@Fredi-raspall
feat(net): annotate overlay packets
6f3d7ec 
Add a flag "is_overlay" to the packet metadata flags to indicate
that the packet is an overlay one; i.e. a packet that has been
decapsulated.

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
@Fredi-raspall
feat(dataplane): revisit vrf/fib logic & fix log
b54c62e 
Fix a misleading log in ip forward regarding the vrf to use,
that referred to older logic. Also, move the vrf check inside the
process() method so that the overal logic is clearer and take into
account if packet is from the overlay when failing.

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
@Fredi-raspall
feat(flow-filter): ignore packets that are not overlay
591c83a 
The flow-filter should only act on overlay packets. Skip processing
packets that are 'done' or not overlay.

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
@Fredi-raspall
feat(pkt-meta): create flows for overlay packets only
067a254 
We only need (atm) flow entry lookups for overlay packets. Let the
stage to create / lookup flow entries ignore done packets or
packets that are not in the overlay.

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
@Fredi-raspall
feat(net,dependants): rename and repurpose NAT flag
b860e79 
Given that we annotate overlay packets as such and that all of
those packets may be subjected to NAT, there some semantic
overlap between flags "is_underlay" and "nat". We can't remove
the nat flag as it is currently used to prevent statefully NATing
a packet that has been statelessly NATed. So, repurpose the flag to
indicate that a packet has been NATed, which helps in diagnostics
(packet dumps) and preventing double NATs that are currently
not supported.

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
@Fredi-raspall
feat(net,dependants): rename metadata getters
7c83e53 
Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
@Fredi-raspall
feat(net,dependants): make Packet meta pub crate
9a9c80a 
Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
@Fredi-raspall
chore(pkt-meta): rename pkt-meta crate
7d4fc55 
Rename pkt-meta crate (which is a bit confusing) to flow-entry
(closer to the purpose). The rename also keeps it closer to related
crates in the IDE ;-)

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
@Fredi-raspall
feat(mgmt,routing): allow multipath in fabric
b0fe047 
Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
@Fredi-raspall
feat(config): make best path as-path relax the default
7b6ad38 
The BGP instance in the default VRF should be configured
with as-path relax in order to allow multipath, given the
eBGP configuration in fabric. The BGP config for the default
VRF should come from user and be configurable. Since, with the
removal of the gRPC, that degree of configurability was lost,
enable it by default for the time being until we can configure it.

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
@Fredi-raspall Fredi-raspall added the ci:-upgrade Disable VLAB upgrade tests label Jan 20, 2026
@qmonnet qmonnet added this pull request to the merge queue Jan 20, 2026
@qmonnet qmonnet mentioned this pull request Jan 20, 2026
Draft
Merged via the queue into main with commit a2c8292 Jan 20, 2026
23 checks passed
@qmonnet qmonnet deleted the pr/fredi/fix_underlay branch January 20, 2026 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@qmonnet qmonnet qmonnet approved these changes

@mvachhar mvachhar Awaiting requested review from mvachhar

Assignees

No one assigned

Labels

ci:-upgrade Disable VLAB upgrade tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Fix underlay forwarding & routing

3 participants

@Fredi-raspall @qmonnet