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

route: Also compare ip rule mask for lookupRule #31700

Merged
merged 1 commit into from
Apr 3, 2024

Conversation

jschwinger233
Copy link
Member

@jschwinger233 jschwinger233 commented Apr 1, 2024

Previously lookupRule("fwmark 0xa00/0xe00 lookup 2005") doesn't work when "fwmark 0xa00/0xf00 lookup 2005" is installed because lookupRule() doesn't check rule mask. This commit fixes it.

@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 Apr 1, 2024
@jschwinger233 jschwinger233 added the release-note/misc This PR makes changes that have no direct user impact. label Apr 1, 2024
@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 Apr 1, 2024
@jschwinger233 jschwinger233 added the sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. label Apr 1, 2024
@jschwinger233
Copy link
Member Author

/test

@jschwinger233
Copy link
Member Author

/test

@jschwinger233 jschwinger233 marked this pull request as ready for review April 2, 2024 11:52
@jschwinger233 jschwinger233 requested a review from a team as a code owner April 2, 2024 11:52
@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 Apr 3, 2024
@julianwiedmann
Copy link
Member

julianwiedmann commented Apr 3, 2024

it's s/lookupRoute/lookupRule in the title & description, right? I was scratching my head here for a second 🙂

@jschwinger233 jschwinger233 changed the title route: Also compare ip rule mask for lookupRoute route: Also compare ip rule mask for lookupRule Apr 3, 2024
@jschwinger233
Copy link
Member Author

@julianwiedmann Yeah 😨 I have to revise my commit message and re-run CI checks 😿

Previously lookupRule("fwmark 0xa00/0xe00 lookup 2005") doesn't work
when "fwmark 0xa00/0xf00 lookup 2005" is installed because lookupRule()
doesn't check rule mask. This commit fixes it.

Signed-off-by: Zhichuan Liang <gray.liang@isovalent.com>
@julianwiedmann
Copy link
Member

The last push only touched the patch description, no code change. CI was green before -> merging.

@julianwiedmann julianwiedmann merged commit c7dd28e into cilium:main Apr 3, 2024
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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