Interface-manager part 2 by daniel-noland · Pull Request #392 · githedgehog/dataplane

daniel-noland · 2025-04-07T22:43:08Z

Cleanup finally finished.

Don't merge until #419 lands

qmonnet · 2025-04-09T11:30:25Z

Rebased on top of main to discard redundant commits and solve merge conflicts. Backup branch at https://github.com/githedgehog/dataplane/tree/pr/daniel-noland/interface-manager.backup.

I've not cleaned up the PR, though.

qmonnet

I do understand that this is still in progress. I flagged a few things I noticed, but I know you probably plan to address a chunk of them already.

Apart from the VPC manager, there are no tests in the PR. Do you plan to add more?

qmonnet · 2025-04-09T12:06:07Z

+	"irekon",
 	"net",
 	"pipeline",
+	"rekon",
 	"routing",
+	"vpc-manager",


We'll have mgmt from Fredi's PR soon. This PR brings three more new crates - are we sure we need all of them to be independent crates?

I like the idea of merging vpc-manager with mgmt

Fredi-raspall · 2025-04-09T13:56:02Z

@daniel-noland , nice work, I still need to go through some parts of it to understand it better. Some early feedback, given that the PR is in DRAFT:

please, open separate PRs for refactors / cleanups not directly related to the present PR. I think this is useful (and takes little time) because it does not add noise to the PR, allows having a separate discussion and helps those clean-ups / refactors make it sooner to the base; so, if there are other outstanding PRs, rebases are easier. No need to change this in this PR, just a recommendation for subsequent ones.
I'm not too sure that the interface manager code should live in net. Maybe there's a reason, but wouldn't it make more sense to move it out of there? I'm fine if it has, as dependencies, other smaller crates, but I think the interface manager is pretty unrelated to all the Packet abstraction and parsing?
Similarly the route submodule in net: I think it is unrelated to net. Also, shouldn't it be part of the VRF interface? Is it used elsewhere?
I see you have a number of definitions for Interface, etc... which are identical to the ones in routing. I wish we'd had the time to unite those.
Regarding the vpc_manager crate, is there any chance to make it test module? I think it is great to validate the whole interface creation and reconciliation work (and should be by all means kept), but it may clash with other work aiming at configuring the GW as a whole. I think that this is indeed the reason for it? (Correct me if I am wrong). Otherwise, I'd : 1) not make it a crate 2) just for clarity not use the term VPC there but s/t like "Sample" something. Finally, can we make the reconcile_test test be such that it does not leave any interfaces when done?

This describes the relations between interfaces. For example, membership in bridges, bonds, or vrfs. Signed-off-by: Daniel Noland <daniel@githedgehog.com>

This is the spec for interface type specific properties. Signed-off-by: Daniel Noland <daniel@githedgehog.com>

Signed-off-by: Daniel Noland <daniel@githedgehog.com>

This allows us to update the properties of a vtep (vxlan interface). Unfortunately, linux won't let you change much here without destroy and re-create of the whole interface. Signed-off-by: Daniel Noland <daniel@githedgehog.com>

This allows us to change the name of a network interface (if needed). Signed-off-by: Daniel Noland <daniel@githedgehog.com>

This allows us to change the association of a network interface. For example, to put a network interface in (or remove one from) a bond or bridge or vrf. Signed-off-by: Daniel Noland <daniel@githedgehog.com>

This allows us to change the interface-type-specific properties of a network interface. Sometimes this involves the destruction and re-creation of that interface. Signed-off-by: Daniel Noland <daniel@githedgehog.com>

This allows us to change the mac of a network interface. Signed-off-by: Daniel Noland <daniel@githedgehog.com>

Signed-off-by: Daniel Noland <daniel@githedgehog.com>

This is the spec for interface type specific properties. Signed-off-by: Daniel Noland <daniel@githedgehog.com>

Signed-off-by: Daniel Noland <daniel@githedgehog.com>

The netns mounts in test-runner.sh are set to nodev. In retrospect this is silly, since the netns mechanism uses dev mounts. Signed-off-by: Daniel Noland <daniel@githedgehog.com>

This never truly belonged here. Signed-off-by: Daniel Noland <daniel@githedgehog.com>

the vpc-manager crate is responsible for calling the interface manager to meet the goals of the dataplane's config. Signed-off-by: Daniel Noland <daniel@githedgehog.com>

This is the rebranded and updated version of the same crate. Signed-off-by: Daniel Noland <daniel@githedgehog.com>

qmonnet

I'm still not convinced that we need so many crates (we had mentioned merging the vpc-manager with mgmt, for example), but this is something we can revisit later if necessary.

Awesome work!

qmonnet · 2025-04-23T09:45:46Z

@@ -0,0 +1,7 @@
+# Rekon
+
+Rekon is a [facade] for an abstract reconciliation engine.


My French screams at the absence of cedilla (“façade”). I'll try to deal with it 🫣

daniel-noland · 2025-04-23T17:01:55Z

I'm still not convinced that we need so many crates (we had mentioned merging the vpc-manager with mgmt, for example), but this is something we can revisit later if necessary.

Awesome work!

I'm completely with you on this one. I want to do a lot of cleanup and de-dupliation after this demo is done

qmonnet force-pushed the pr/daniel-noland/interface-manager branch from 68c2e4e to ab3f1bd Compare April 9, 2025 11:29

qmonnet force-pushed the pr/daniel-noland/interface-manager branch from ab3f1bd to ae2cfd3 Compare April 9, 2025 11:32

qmonnet reviewed Apr 9, 2025

View reviewed changes

This comment was marked as outdated.

Sign in to view

qmonnet reviewed Apr 9, 2025

View reviewed changes

Comment thread Cargo.lock Outdated

This comment was marked as outdated.

Sign in to view

This was referenced Apr 10, 2025

Consider switching to syn 2 lun3x/multi_index_map#53

Closed

Consider switching to syn 2 lun3x/multi_index_map#54

Open

daniel-noland force-pushed the pr/daniel-noland/interface-manager branch 15 times, most recently from 4b151be to bd6567e Compare April 19, 2025 17:35

This comment was marked as outdated.

Sign in to view

daniel-noland force-pushed the pr/daniel-noland/interface-manager branch 2 times, most recently from a9d8bc3 to 4018aa7 Compare April 19, 2025 17:52

daniel-noland changed the title ~~DRAFT: interface manager~~ Interface manager part 2 Apr 19, 2025

daniel-noland added 15 commits April 22, 2025 20:31

feat(interface-manager): interface association module

8cb24ca

This describes the relations between interfaces. For example, membership in bridges, bonds, or vrfs. Signed-off-by: Daniel Noland <daniel@githedgehog.com>

feat(interface-manager): InterfacePropertiesSpec

2f691f6

This is the spec for interface type specific properties. Signed-off-by: Daniel Noland <daniel@githedgehog.com>

feat(interface-manager): impl AsRequirement for Interface

927fd76

Signed-off-by: Daniel Noland <daniel@githedgehog.com>

feat(interface-manager): impl Create for Manager<Interface>

b433698

Signed-off-by: Daniel Noland <daniel@githedgehog.com>

feat(interface-manager): impl Remove for Manager<Interface>

8a524ae

Signed-off-by: Daniel Noland <daniel@githedgehog.com>

feat(interface-manager): impl Update for Manager<InterfaceName>

bd339a5

This allows us to change the name of a network interface (if needed). Signed-off-by: Daniel Noland <daniel@githedgehog.com>

feat(interface-manager): impl Update for Manager<InterfaceAssociation>

ce142a9

This allows us to change the association of a network interface. For example, to put a network interface in (or remove one from) a bond or bridge or vrf. Signed-off-by: Daniel Noland <daniel@githedgehog.com>

feat(interface-manager): impl Update for Manager<InterfaceProperties>

3e5f74b

This allows us to change the interface-type-specific properties of a network interface. Sometimes this involves the destruction and re-creation of that interface. Signed-off-by: Daniel Noland <daniel@githedgehog.com>

feat(interface-manager): impl Update for Manager<SourceMac>

b1b66c9

This allows us to change the mac of a network interface. Signed-off-by: Daniel Noland <daniel@githedgehog.com>

feat(interface-manager): impl Update for Manager<AdminState>

d75b65d

Signed-off-by: Daniel Noland <daniel@githedgehog.com>

feat(interface-manager): impl Update for Manager<Interface>

bf324aa

Signed-off-by: Daniel Noland <daniel@githedgehog.com>

feat(interface-manager): introduce InterfacePropertiesSpec

925f061

This is the spec for interface type specific properties. Signed-off-by: Daniel Noland <daniel@githedgehog.com>

feat(interface-manager): impl Reconcile for Manager<Interface>

155383b

Signed-off-by: Daniel Noland <daniel@githedgehog.com>

feat(interface-manager): basic tests

4dc3044

Signed-off-by: Daniel Noland <daniel@githedgehog.com>

daniel-noland force-pushed the pr/daniel-noland/interface-manager branch from d8be331 to 560084a Compare April 23, 2025 02:31

daniel-noland removed the dont-merge Do not merge this Pull Request label Apr 23, 2025

daniel-noland requested a review from qmonnet April 23, 2025 02:37

daniel-noland added 5 commits April 22, 2025 20:51

feat(interface-manager): introduce netns management

97b883d

Signed-off-by: Daniel Noland <daniel@githedgehog.com>

fix(interface-manager): netns mounts require dev

a0baa03

The netns mounts in test-runner.sh are set to nodev. In retrospect this is silly, since the netns mechanism uses dev mounts. Signed-off-by: Daniel Noland <daniel@githedgehog.com>

fix(interface-manager): move netlink logic out of net

1a82a5d

This never truly belonged here. Signed-off-by: Daniel Noland <daniel@githedgehog.com>

feat(vpc-manager): introduce vpc-manager

87d1800

the vpc-manager crate is responsible for calling the interface manager to meet the goals of the dataplane's config. Signed-off-by: Daniel Noland <daniel@githedgehog.com>

fix: replace default-net with netdev

6d94a36

This is the rebranded and updated version of the same crate. Signed-off-by: Daniel Noland <daniel@githedgehog.com>

daniel-noland force-pushed the pr/daniel-noland/interface-manager branch from 560084a to 6d94a36 Compare April 23, 2025 02:51

qmonnet approved these changes Apr 23, 2025

View reviewed changes

daniel-noland added this pull request to the merge queue Apr 23, 2025

Merged via the queue into main with commit a6dc173 Apr 23, 2025
16 checks passed

daniel-noland deleted the pr/daniel-noland/interface-manager branch April 23, 2025 17:07

		@@ -0,0 +1,7 @@
		# Rekon

		Rekon is a [facade] for an abstract reconciliation engine.

Conversation

daniel-noland commented Apr 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qmonnet commented Apr 9, 2025

Uh oh!

qmonnet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

qmonnet Apr 9, 2025

Choose a reason for hiding this comment

Uh oh!

daniel-noland Apr 22, 2025

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Fredi-raspall commented Apr 9, 2025

Uh oh!

This comment was marked as outdated.

qmonnet left a comment

Choose a reason for hiding this comment

Uh oh!

qmonnet Apr 23, 2025

Choose a reason for hiding this comment

Uh oh!

daniel-noland commented Apr 23, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

daniel-noland commented Apr 7, 2025 •

edited

Loading