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

bpf: Consistent usage of MARK_MAGIC_ constants #23125

Merged
merged 1 commit into from Jan 19, 2023

Conversation

pchaigno
Copy link
Member

We currently use 8 bits of the packet mark to indicate that SNAT was already performed. That is more than necessary and inconsistent with other users of the packet mark for similar purposes.

This pull request changes that to only use 4 bits under the reserved MARK_MAGIC_HOST_MASK mask.

Also worth noting that the IPsec comment here is incorrect: MARK_MAGIC_KEY_ID is currently unused so it wasn't conflicting with the SNAT mark.

@pchaigno pchaigno added sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. kind/cleanup This includes no functional changes. labels Jan 16, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jan 16, 2023
@pchaigno pchaigno added the release-note/misc This PR makes changes that have no direct user impact. label Jan 16, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jan 16, 2023
We currently use 8 bits of the packet mark to indicate that SNAT was
already performed. That is more than necessary and inconsistent with
other users of the packet mark for similar purposes.

This commit changes that to only use 4 bits under the reserved
MARK_MAGIC_HOST_MASK mask.

Also worth noting that the IPsec comment here is incorrect:
MARK_MAGIC_KEY_ID is currently unused so it wasn't conflicting with the
SNAT mark.

Signed-off-by: Paul Chaignon <paul@cilium.io>
@pchaigno
Copy link
Member Author

/test

@pchaigno pchaigno requested a review from brb January 17, 2023 10:01
@pchaigno pchaigno marked this pull request as ready for review January 17, 2023 10:01
@pchaigno pchaigno requested a review from a team as a code owner January 17, 2023 10:01
Copy link
Member

@brb brb left a comment

Choose a reason for hiding this comment

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

Nice one! LGTM.

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jan 18, 2023
@ldelossa ldelossa merged commit 7eebe58 into cilium:master Jan 19, 2023
@pchaigno pchaigno deleted the consistent-magic-mark-usage branch January 19, 2023 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/cleanup This includes no functional changes. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants