Skip to content
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.

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

CGNAT Exit and integration test #1104

Merged
merged 3 commits into from
Feb 19, 2025
Merged

CGNAT Exit and integration test #1104

merged 3 commits into from
Feb 19, 2025

Conversation

ch-iara
Copy link
Contributor

@ch-iara ch-iara commented Feb 13, 2025

right now this is manually adding all possible external ips on the exit subnet, which we're hoping to avoid since that gets clunky real fast.

right now this is manually adding all possible external ips on the exit subnet,
which we're hoping to avoid since that gets clunky real fast.
@ch-iara ch-iara force-pushed the chiara/cgnat-exits branch 2 times, most recently from beba305 to 9569820 Compare February 14, 2025 21:05
Copy link
Member

@jkilpatr jkilpatr left a comment

Choose a reason for hiding this comment

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

This looks good. I only have some very minor comment nits, overall using the build in cgnat is a big improvement in code simplicity!

@@ -368,6 +368,51 @@ pub fn teardown_snat(
Ok(())
}

/// Sets up the CGNAT rules for the exit server run on startup
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice if this comment explicitly noted that we're handing the cgnat tasks off to the kernel to allocate connections between the internal and external subnets.

}
}
}

// gets the range of possible ips for a given subnet. by convention the static assignments in cgnat mode should be
// on the ends of the assignable range to make random assignment less complex. TODO: endpoint to set static ips should
Copy link
Member

Choose a reason for hiding this comment

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

I think this todo is completed and can be cleaned up

@jkilpatr
Copy link
Member

looks like the multi exit test needs a fix.

Removing some extra cruft now that CGNAT mode does not explicitly
assign or keep track of clients <-> external ips
@jkilpatr jkilpatr merged commit 44f5e20 into master Feb 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants