Skip to content

L2 externals #1139

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
Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

L2 externals #1139

wants to merge 7 commits into from

Conversation

@edipascale
Copy link
Contributor

@edipascale edipascale commented Oct 16, 2025 •
edited
Loading

Fix #1140

@edipascale edipascale requested review from a team as code owners October 16, 2025 15:22
@edipascale edipascale marked this pull request as draft October 16, 2025 15:22
@edipascale edipascale changed the base branch from master to ema/externals-as-vpcs October 16, 2025 15:23
@edipascale edipascale force-pushed the ema/externals-as-vpcs branch from 0e89f55 to d94cbdd Compare October 16, 2025 15:24
@edipascale edipascale added the ci:-vlab Disable VLAB tests label Oct 16, 2025
@github-actions
Copy link

github-actions bot commented Oct 16, 2025

🚀 Temp artifacts published: v0-1919c7f62 🚀

@github-actions
Copy link

github-actions bot commented Oct 16, 2025

🚀 Temp artifacts published: v0-518464ba2 🚀

@edipascale edipascale changed the title Ema/l2 externals L2 externals Oct 17, 2025
@github-actions
Copy link

github-actions bot commented Oct 17, 2025

🚀 Temp artifacts published: v0-975dc8f31 🚀

@github-actions
Copy link

github-actions bot commented Oct 17, 2025

🚀 Temp artifacts published: v0-fa21c3511 🚀

@github-actions
Copy link

github-actions bot commented Oct 20, 2025

🚀 Temp artifacts published: v0-94e1827e3 🚀

@github-actions
Copy link

github-actions bot commented Oct 20, 2025

🚀 Temp artifacts published: v0-9a49db650 🚀

@edipascale
Copy link
Contributor Author

edipascale commented Oct 20, 2025

other than the horrible names for some of the new API objects, this should be good to be reviewed - I can configure and remove config with both subinterfaces and the parent interface (i.e. no vlan).

@edipascale edipascale marked this pull request as ready for review October 20, 2025 14:01
@github-actions
Copy link

github-actions bot commented Oct 21, 2025

🚀 Temp artifacts published: v0-df86cbe03 🚀

@edipascale
Copy link
Contributor Author

edipascale commented Oct 21, 2025

other than the horrible names for some of the new API objects, this should be good to be reviewed - I can configure and remove config with both subinterfaces and the parent interface (i.e. no vlan).

I did a first pass on the l2 external attachment API and used hopefully less horrible names for some of these things

@github-actions
Copy link

github-actions bot commented Oct 21, 2025

🚀 Temp artifacts published: v0-7f9c57d5f 🚀

@github-actions
Copy link

github-actions bot commented Oct 23, 2025

🚀 Temp artifacts published: v0-019baba74 🚀

@edipascale edipascale marked this pull request as draft October 23, 2025 08:45
@edipascale edipascale added the ci:-upgrade Disable VLAB upgrade tests label Oct 23, 2025
@edipascale
Copy link
Contributor Author

edipascale commented Oct 23, 2025

I'm converting this to draft since we may not need the static ARP + virtual IP trick, but we want to verify this on a physical env before going ahead with the changes

@github-actions
Copy link

github-actions bot commented Oct 23, 2025

🚀 Temp artifacts published: v0-019baba74 🚀

@github-actions
Copy link

github-actions bot commented Oct 23, 2025

🚀 Temp artifacts published: v0-ece0450b4 🚀

@github-actions
Copy link

github-actions bot commented Oct 23, 2025

🚀 Temp artifacts published: v0-f64059c8d 🚀

@github-actions
Copy link

github-actions bot commented Oct 23, 2025

🚀 Temp artifacts published: v0-10c634ab4 🚀

@edipascale
Copy link
Contributor Author

edipascale commented Oct 29, 2025

I'm converting this to draft since we may not need the static ARP + virtual IP trick, but we want to verify this on a physical env before going ahead with the changes

I was able to try this today on env-5 and, with the appropriate configuration on the DS2000 that we use as the external, I could verify external connectivity from a VPC to Cloudflare via a NATted peering using the gateway and the simplified L2 externals (i.e. with no static ARP entry)

@edipascale edipascale marked this pull request as ready for review October 29, 2025 11:19
@edipascale edipascale force-pushed the ema/externals-as-vpcs branch from d94cbdd to 5630063 Compare October 29, 2025 12:16
@Frostman Frostman force-pushed the ema/externals-as-vpcs branch 2 times, most recently from abafe6f to d69beae Compare November 3, 2025 07:40
@github-actions
Copy link

github-actions bot commented Nov 3, 2025

🚀 Temp artifacts published: v0-8d7139adc 🚀

Base automatically changed from ema/externals-as-vpcs to master November 3, 2025 15:31
@github-actions
Copy link

github-actions bot commented Nov 3, 2025

🚀 Temp artifacts published: v0-6ec7cdddf 🚀

@github-actions
Copy link

github-actions bot commented Nov 3, 2025

🚀 Temp artifacts published: v0-cdc782680 🚀

Frostman
Frostman requested changes Nov 3, 2025
View reviewed changes
Copy link
Member

@Frostman Frostman left a comment

Choose a reason for hiding this comment

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

Few comments on API side, didn't review the plan/gnmi part yet

@edipascale
feat(agent): support for proxy-arp
33e4eae 
Signed-off-by: Emanuele Di Pascale <emanuele@githedgehog.com>
@edipascale
feat(agent): support for static ARP entries
ef63339 
Signed-off-by: Emanuele Di Pascale <emanuele@githedgehog.com>
@edipascale
feat: L2 external API
6142b84 
introduces an L2 external as a subtype of the existing BGP
speaker one. Ideally we would move the existing BGP params
in a sub-struct, but for now we want to avoid having to deal
with non backward-compatible changes.

this will need an equivalent L2 externalAttachment in order
to work; I will introduce it in a separate commit for ease
of review.

Signed-off-by: Emanuele Di Pascale <emanuele@githedgehog.com>
@edipascale
feat: L2 externalAttachment API
a182865 
continues from the previous commit to add externalAttachment
for L2 externals.

Signed-off-by: Emanuele Di Pascale <emanuele@githedgehog.com>
@edipascale
fix: simplify l2 externalAttchment
fe4395b 
remove the virtual IPs and static ARP trick, use a direct
route instead

note that we do not need the static ARP config handler with
this solution, but I've kept it anyway as we might want to
use it in the future for some other feature.

Signed-off-by: Emanuele Di Pascale <emanuele@githedgehog.com>
@edipascale
feat(agent): support direct static routes
e3a46ec 
Signed-off-by: Emanuele Di Pascale <emanuele@githedgehog.com>
@edipascale
chore: revert spurious omitempty changes
b7a3438 
and rename ExternalL2Spec to ExternalL2 as requested

Signed-off-by: Emanuele Di Pascale <emanuele@githedgehog.com>
@edipascale edipascale requested a review from Frostman November 4, 2025 07:40
@edipascale edipascale removed ci:-vlab Disable VLAB tests ci:-upgrade Disable VLAB upgrade tests labels Nov 4, 2025
@github-actions
Copy link

github-actions bot commented Nov 4, 2025

🚀 Temp artifacts published: v0-b7a34382c 🚀

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

Reviewers

@Frostman Frostman Awaiting requested review from Frostman

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Extend external API to cover the L2 adjacency case

3 participants

@edipascale @Frostman