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

datapath: Sort VLAN IDs in generated macros #17105

Merged
merged 1 commit into from
Aug 10, 2021

Conversation

jrajahalme
Copy link
Member

Apparently netlink.LinkList() can return VLAN IDs in any order. Sort
them to get consistent generated macros.

Fixes: #17104
Signed-off-by: Jarno Rajahalme jarno@isovalent.com

@jrajahalme jrajahalme added kind/bug This is a bug in the Cilium logic. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. release-note/misc This PR makes changes that have no direct user impact. labels Aug 9, 2021
@jrajahalme jrajahalme requested review from a team as code owners August 9, 2021 17:05
@jrajahalme jrajahalme requested review from a team, nebril, aditighag and twpayne August 9, 2021 17:05
@jrajahalme
Copy link
Member Author

jrajahalme commented Aug 9, 2021

test-me-please

Job 'Cilium-PR-K8s-1.20-kernel-4.19' failed and has not been observed before, so may be related to your PR:

Click to show.

Test Name

K8sServicesTest Checks service across nodes Tests NodePort BPF Tests with direct routing Tests NodePort

Failure Output

FAIL: Request from k8s1 to service tftp://[fd04::11]:31422/hello failed

If it is a flake, comment /mlh new-flake Cilium-PR-K8s-1.20-kernel-4.19 so I can create a new GitHub issue to track it.

Job 'Cilium-PR-K8s-1.16-net-next' hit: #17060 (85.43% similarity)

@jrajahalme
Copy link
Member Author

None of the flakes are related to this PR that fixes another flake.

Copy link
Member

@aditighag aditighag left a comment

Choose a reason for hiding this comment

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

It seems like only the last commit is related to the PR title. Is this supposed to be a draft PR?

@jrajahalme jrajahalme marked this pull request as draft August 10, 2021 10:17
@jrajahalme
Copy link
Member Author

It seems like only the last commit is related to the PR title. Is this supposed to be a draft PR?

Good catch, accidentally based this on another branch rather than master, will rebase!

Apparently netlink.LinkList() can return VLAN IDs in any order. Sort
them to get consistent generated macros.

Fixes: cilium#17104
Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
@jrajahalme jrajahalme marked this pull request as ready for review August 10, 2021 10:19
@jrajahalme
Copy link
Member Author

@aditighag Rebased :-)

@jrajahalme
Copy link
Member Author

test-me-please

@jrajahalme jrajahalme added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Aug 10, 2021
@gandro gandro merged commit c6d25fc into cilium:master Aug 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug This is a bug in the Cilium logic. 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.

CI: RuntimePrivilegedUnitTests Run Tests
5 participants