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: minor CT cleanups #23718

Merged
merged 2 commits into from
Feb 17, 2023
Merged

Conversation

julianwiedmann
Copy link
Member

@julianwiedmann julianwiedmann commented Feb 13, 2023

Just some minor CT cleanups that have accumulated.

Remove some dead code.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
We currently have IPv4 and IPv6 variants of several helpers that update a
CT entry. These only differ in the CT tuple parameter, which then gets used
as key for a map lookup.

To avoid this duplication, take ct_update_nodeport() as example and turn
the CT tuple into an opaque parameter.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
@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 Feb 13, 2023
@julianwiedmann julianwiedmann changed the title 1.14 bpf ct cleanups bpf: minor CT cleanups Feb 13, 2023
@julianwiedmann julianwiedmann added sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. kind/cleanup This includes no functional changes. release-note/misc This PR makes changes that have no direct user impact. labels Feb 13, 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 Feb 13, 2023
@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann julianwiedmann marked this pull request as ready for review February 13, 2023 18:53
@julianwiedmann julianwiedmann requested a review from a team as a code owner February 13, 2023 18:53
Copy link
Member

@dylandreimerink dylandreimerink left a comment

Choose a reason for hiding this comment

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

Very nice, love to see it!

@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 Feb 16, 2023
@julianwiedmann
Copy link
Member Author

/test-1.16-4.19

@pchaigno pchaigno removed the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Feb 16, 2023
@@ -935,7 +935,7 @@ static __always_inline int lb6_local(const void *map, struct __ctx_buff *ctx,
/* See lb4_local comment */
if (state->rev_nat_index == 0) {
state->rev_nat_index = svc->rev_nat_index;
ct_update6_rev_nat_index(map, tuple, state);
Copy link
Member

Choose a reason for hiding this comment

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

FYI: even if we don't rely on the compiler anymore to check the types, we could easily rely on Coccinelle instead to check that the second parameter is one of the two expected types.

@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 Feb 16, 2023
@pchaigno pchaigno merged commit ef8d9d9 into cilium:master Feb 17, 2023
@julianwiedmann julianwiedmann deleted the 1.14-bpf-ct-cleanups branch February 17, 2023 11:46
@julianwiedmann julianwiedmann added the backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. label Apr 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. 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