Skip to content

refactor: cleanups in unparallel tests and replace netlink with safenetlink #41363

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Aug 26, 2025
Merged

refactor: cleanups in unparallel tests and replace netlink with safenetlink #41363

merged 5 commits into from
Aug 26, 2025

Conversation

smagnani96
Copy link
Contributor

@smagnani96 smagnani96 commented Aug 25, 2025 •
edited
Loading

This PR is mostly just to please golangci-lint run and editor linters.
Please refer to commits.

@smagnani96 smagnani96 self-assigned this Aug 25, 2025
@smagnani96 smagnani96 added kind/cleanup This includes no functional changes. release-note/misc This PR makes changes that have no direct user impact. labels Aug 25, 2025
@smagnani96 smagnani96 requested a review from pchaigno August 25, 2025 10:36
@smagnani96
refactor:tests: cleanup unused variables in node_linux_test
d467c4c 
Temporarily removing `//go:build unparallel` raised a warning in the
editor about these unused variables. Let's remove them.

Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
@smagnani96
refactor: device controller if-else to switch
351a03f 
This is just to remove a warning from the linter about possible refactorization
of if-else statements to switch statements.

Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
@smagnani96
refactor:ipsec: cleanup unused log variable
847d5d3 
This is to cleanup the unused `log` variable from the `parseSPI` and
`LoadIPSecKeys` functions. No functional changes, just to golangci-lint happy.

Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
@smagnani96
refactor:ipsec: replace netlink.Xfrm{State,Policy}List with safelinknet
01de1fe 
This commit removes the occurrences of netlink.Xfrm{State,Policy}List in
favor of the safenetlink package. This is to overcome an issue returned
by  `golangci-lint run` due to using netlink rather than safelinknet.
An example of error returned:

`pkg/datapath/linux/node_linux_test.go:1195:19: use of `netlink.XfrmPolicyList` forbidden because "Found netlink function which can return ErrDumpInterrupted. Use safenetlink package instead." (forbidigo)`

We do not see this in CI given these test files start with `//go:build unparallel`.
From the moment we remove that line (I did that to have linting during this
patch series) the error is returned.

Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
@smagnani96
refactor:ipsec: remove unused MapUpdateContextWithMap from encryptmap
17bf284 
We no longer use this method, not even in tests, let's remove it.

Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
@smagnani96 smagnani96 force-pushed the pr/smagnani96/misc-cleanups branch from fe80488 to 17bf284 Compare August 25, 2025 11:41
@smagnani96
Copy link
Contributor Author

/test

@smagnani96 smagnani96 marked this pull request as ready for review August 26, 2025 08:50
@smagnani96 smagnani96 requested review from a team as code owners August 26, 2025 08:50
@smagnani96
Copy link
Contributor Author

Checkpatch is failing due to the issue of previous days: images not being available.
A rebase would solve this, let me know @pchaigno if that's preferred (would require to rerun CI).
Running locally (after rebasing) gives no problem:

❯ make -C bpf checkpatch
make: Entering directory '/home/user/cilium/bpf'
docker container run --rm \
	--workdir /workspace \
	--volume /home/user/cilium/bpf/..:/workspace \
	--user "1000:1000" \
	-e GITHUB_REF= -e GITHUB_REPOSITORY= -e GITHUB_TOKEN= \
	--entrypoint /bin/bash quay.io/cilium/cilium-checkpatch:1755701578-b97bd7a@sha256:5c4df794425e29962aac25fd5c005fd39d737232121688ba8d08878f4815b232 /checkpatch/checkpatch.sh -- --ignore MACRO_ARG_REUSE 
Retrieved 5 commits on top of ref upstream/main

=========================================================
[1/5] Running on 24a5d04009f5dc3e25242004a5e6fdb401337932
refactor:ipsec: remove unused MapUpdateContextWithMap from encryptmap
=========================================================

"[PATCH] refactor:ipsec: remove unused MapUpdateContextWithMap from" has no obvious style problems and is ready for submission.

NOTE: Ignored message types: BIT_MACRO C99_COMMENT_TOLERANCE COMMIT_LOG_LONG_LINE COMMIT_MESSAGE COMPLEX_MACRO CONSTANT_CONVERSION CONST_STRUCT EMAIL_SUBJECT FILE_PATH_CHANGES FROM_SIGN_OFF_MISMATCH GIT_COMMIT_ID JIFFIES_COMPARISON LEADING_SPACE LONG_LINE_COMMENT MACRO_ARG_REUSE MACRO_WITH_FLOW_CONTROL MULTISTATEMENT_MACRO_USE_DO_WHILE NOT_UNIFIED_DIFF PRINTK_WITHOUT_KERN_LEVEL TRAILING_SEMICOLON TRAILING_STATEMENTS VOLATILE
=========================================================
[2/5] Running on a0581d7489ed449ab486777dbf90909b73ef0b62
refactor:ipsec: replace netlink.Xfrm{State,Policy}List with safelinknet
=========================================================

"[PATCH] refactor:ipsec: replace netlink.Xfrm{State,Policy}List with" has no obvious style problems and is ready for submission.

NOTE: Ignored message types: BIT_MACRO C99_COMMENT_TOLERANCE COMMIT_LOG_LONG_LINE COMMIT_MESSAGE COMPLEX_MACRO CONSTANT_CONVERSION CONST_STRUCT EMAIL_SUBJECT FILE_PATH_CHANGES FROM_SIGN_OFF_MISMATCH GIT_COMMIT_ID JIFFIES_COMPARISON LEADING_SPACE LONG_LINE_COMMENT MACRO_ARG_REUSE MACRO_WITH_FLOW_CONTROL MULTISTATEMENT_MACRO_USE_DO_WHILE NOT_UNIFIED_DIFF PRINTK_WITHOUT_KERN_LEVEL TRAILING_SEMICOLON TRAILING_STATEMENTS VOLATILE
=========================================================
[3/5] Running on 839d82c644bcd2d78e1a149f5bc7a787cd823167
refactor:ipsec: cleanup unused log variable
=========================================================

"[PATCH] refactor:ipsec: cleanup unused log variable" has no obvious style problems and is ready for submission.

NOTE: Ignored message types: BIT_MACRO C99_COMMENT_TOLERANCE COMMIT_LOG_LONG_LINE COMMIT_MESSAGE COMPLEX_MACRO CONSTANT_CONVERSION CONST_STRUCT EMAIL_SUBJECT FILE_PATH_CHANGES FROM_SIGN_OFF_MISMATCH GIT_COMMIT_ID JIFFIES_COMPARISON LEADING_SPACE LONG_LINE_COMMENT MACRO_ARG_REUSE MACRO_WITH_FLOW_CONTROL MULTISTATEMENT_MACRO_USE_DO_WHILE NOT_UNIFIED_DIFF PRINTK_WITHOUT_KERN_LEVEL TRAILING_SEMICOLON TRAILING_STATEMENTS VOLATILE
=========================================================
[4/5] Running on 327bbcadee811869798e11091c1c625aeed82d7a
refactor: device controller if-else to switch
=========================================================

"[PATCH] refactor: device controller if-else to switch" has no obvious style problems and is ready for submission.

NOTE: Ignored message types: BIT_MACRO C99_COMMENT_TOLERANCE COMMIT_LOG_LONG_LINE COMMIT_MESSAGE COMPLEX_MACRO CONSTANT_CONVERSION CONST_STRUCT EMAIL_SUBJECT FILE_PATH_CHANGES FROM_SIGN_OFF_MISMATCH GIT_COMMIT_ID JIFFIES_COMPARISON LEADING_SPACE LONG_LINE_COMMENT MACRO_ARG_REUSE MACRO_WITH_FLOW_CONTROL MULTISTATEMENT_MACRO_USE_DO_WHILE NOT_UNIFIED_DIFF PRINTK_WITHOUT_KERN_LEVEL TRAILING_SEMICOLON TRAILING_STATEMENTS VOLATILE
=========================================================
[5/5] Running on 1fb93c340c2cf6fad57f9a01e03efa2943071fec
refactor:tests: cleanup unused variables in node_linux_test
=========================================================

"[PATCH] refactor:tests: cleanup unused variables in node_linux_test" has no obvious style problems and is ready for submission.

NOTE: Ignored message types: BIT_MACRO C99_COMMENT_TOLERANCE COMMIT_LOG_LONG_LINE COMMIT_MESSAGE COMPLEX_MACRO CONSTANT_CONVERSION CONST_STRUCT EMAIL_SUBJECT FILE_PATH_CHANGES FROM_SIGN_OFF_MISMATCH GIT_COMMIT_ID JIFFIES_COMPARISON LEADING_SPACE LONG_LINE_COMMENT MACRO_ARG_REUSE MACRO_WITH_FLOW_CONTROL MULTISTATEMENT_MACRO_USE_DO_WHILE NOT_UNIFIED_DIFF PRINTK_WITHOUT_KERN_LEVEL TRAILING_SEMICOLON TRAILING_STATEMENTS VOLATILE
All done
make: Leaving directory '/home/user/cilium/bpf'

pchaigno
pchaigno approved these changes Aug 26, 2025
View reviewed changes
Copy link
Member

@pchaigno pchaigno left a comment

Choose a reason for hiding this comment

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

Thanks, very easy to review thanks to the descriptions!

@pchaigno pchaigno added this pull request to the merge queue Aug 26, 2025
Merged via the queue into main with commit cb307a5 Aug 26, 2025
482 of 491 checks passed
@pchaigno pchaigno deleted the pr/smagnani96/misc-cleanups branch August 26, 2025 12:18
@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 Aug 26, 2025
@smagnani96 smagnani96 mentioned this pull request Aug 26, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pchaigno pchaigno pchaigno approved these changes

Assignees

@smagnani96 smagnani96

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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@smagnani96 @pchaigno