-
Notifications
You must be signed in to change notification settings - Fork 9
Apply NAT to headers/packets carried within ICMP messages #736
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
Conversation
Taking this for a spin in a local omicron environment, we can run traceroute from within instances now!
|
Traceroute from outside the rack going in also works.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. It looks like the CI issues are just that we need a new lab image?
Thanks for the review Ry, great to see that traceroute now works as intended.
Yep, I have oxidecomputer/meta#654 open for this. |
* Apply NAT to headers/packets carried within ICMP messages (oxidecomputer/opte#736)
* Apply NAT to headers/packets carried within ICMP messages (oxidecomputer/opte#736)
This PR modifies nested packets in ICMP payloads to ensure that private source/destination addresses are replaced with the correct external IP. This makes use of a body transform to modify the packet contents, which has the downside of downgrading NAT'd ICMP packets to the less-fast-path. Note that this is not performance-critical traffic, and these frames are still taking a read lock over the port at worst.
There's a bit of trouble passing the full header stack back into these body transforms -- we have an arbitrary
T
which can be treated as a byteslice, but ourdyn BodyTransform
s presumably need to be monomorphised. We need to hand at least some knowledge of the parsed headers over, such that we only attempt this transform on the right ICMP types. For now, we achieve this by passing in a reference to the owned version of the inner ULP header.Fixes #369.