8000 Fix service id exceeds max limit by haozhangami · Pull Request #37191 · cilium/cilium · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix service id exceeds max limit #37191

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 1 commit into from
Feb 14, 2025
Merged

Fix service id exceeds max limit #37191

merged 1 commit into from
Feb 14, 2025

Conversation

haozhangami
Copy link
Contributor
@haozhangami haozhangami commented Jan 23, 2025 •
edited
Loading

The maximum service id is 65535 due to the limit of __u16 rev_nat_index. the service command line and service api handler do not check the maximum id. this patch checks the maximum service id.

Fixes: #32758

@haozhangami haozhangami requested review from a team as code owners January 23, 2025 07:25
@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 23, 2025
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Jan 23, 2025
@haozhangami haozhangami force-pushed the fix-service-id-exceeds-max-limit branch from acfa80f to 1fd97d7 Compare January 23, 2025 14:06
@joestringer joestringer added the release-note/bug This PR fixes an issue in a previous release of Cilium. label Jan 23, 2025
@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 23, 2025
@joestringer joestringer added affects/v1.14 This issue affects v1.14 branch affects/v1.15 This issue affects v1.15 branch affects/v1.16 This issue affects v1.16 branch needs-backport/1.17 This PR / issue needs backporting to the v1.17 branch labels Jan 23, 2025
derailed
derailed requested changes Feb 3, 2025
View reviewed changes
Copy link
Contributor
@derailed derailed left a comment

Choose a reason for hiding this comment

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

@haozhangami Thank you for this update!

8000
@haozhangami haozhangami force-pushed the fix-service-id-exceeds-max-limit branch 2 times, most recently from ff78acc to d773793 Compare February 5, 2025 06:21
@haozhangami haozhangami requested a review from derailed February 5, 2025 07:13
derailed
derailed approved these changes Feb 11, 2025
View reviewed changes
Copy link
Contributor
@derailed derailed left a comment

Choose a reason for hiding this comment

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

@haozhangami Thank you for the updates.

@haozhangami
Copy link
Contributor Author

/test

@ysksuzuki
Copy link
Member

Thank you for the PR! Your base branch is outdated, which is causing CI failures. Could you rebase your branch onto the latest main? Thanks!

https://github.com/cilium/cilium/actions/runs/13279191498/job/37074148532

+ docker run --network host -v /home/runner/.kube/config:/root/.kube/config -v /home/runner/work/cilium/cilium:/root/app -v /home/runner/.aws:/root/.aws -v /home/runner/.azure:/root/.azure -v /home/runner/.config/gcloud:/root/.config/gcloud quay.io/cilium/cilium-cli-ci:d773793f928a3a9513d8694cdf49bcdeffd3888c cilium connectivity test --include-conn-disrupt-test --include-conn-disrupt-test-ns-traffic --conn-disrupt-test-setup --conn-disrupt-test-restarts-path ./cilium-conn-disrupt-restarts --conn-disrupt-dispatch-interval 0ms
unknown flag: --include-conn-disrupt-test-ns-traffic

@haozhangami haozhangami force-pushed the fix-service-id-exceeds-max-limit branch from d773793 to 2e16fcf Compare February 12, 2025 07:44
@ysksuzuki
Copy link
Member

/test

ysksuzuki
ysksuzuki reviewed Feb 13, 2025
View reviewed changes
@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 13, 2025
@ysksuzuki ysksuzuki removed the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Feb 13, 2025
@haozhangami haozhangami force-pushed the fix-service-id-exceeds-max-limit branch from 2e16fcf to cefd475 Compare February 13, 2025 21:10
@haozhangami
Fix service id exceeds max limit
fb748bc 
The maximum service id is 65535 due to the limit of __u16 rev_nat_index.
the service command line and service api handler do not check the maximum
id. this patch checks the maximum service id.

Fixes: cilium#32758

Signed-off-by: Hao Zhang <hao.zhang.am.i@gmail.com>
@haozhangami haozhangami force-pushed the fix-service-id-exceeds-max-limit branch from cefd475 to fb748bc Compare February 13, 2025 21:12
ysksuzuki
ysksuzuki approved these changes Feb 14, 2025
View reviewed changes
Copy link
Member
@ysksuzuki ysksuzuki left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@ysksuzuki
Copy link
Member

/test

@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 14, 2025
@julianwiedmann julianwiedmann added the area/loadbalancing Impacts load-balancing and Kubernetes service implementations label Feb 14, 2025
@julianwiedmann julianwiedmann added this pull request to the merge queue Feb 14, 2025
Merged via the queue into cilium:main with commit 83847c7 Feb 14, 2025
66 checks passed
@nbusseneau nbusseneau mentioned this pull request Feb 14, 2025
Merged
22 tasks
@nbusseneau nbusseneau added backport-pending/1.17 The backport for Cilium 1.17.x for this PR is in progress. and removed needs-backport/1.17 This PR / issue needs backporting to the v1.17 branch labels Feb 14, 2025
@github-actions github-actions bot added backport-done/1.17 The backport for Cilium 1.17.x for this PR is done. and removed backport-pending/1.17 The backport for Cilium 1.17.x for this PR is in progress. labels Feb 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derailed derailed derailed approved these changes

@ysksuzuki ysksuzuki ysksuzuki approved these changes

Assignees
No one assigned
Labels
affects/v1.14 This issue affects v1.14 branch affects/v1.15 This issue affects v1.15 branch affects/v1.16 This issue affects v1.16 branch area/loadbalancing Impacts load-balancing and Kubernetes service implementations backport-done/1.17 The backport for Cilium 1.17.x for this PR is done. kind/community-contribution This was a contribution made by a community member. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Unexpected behaviour when creating services with IDs > 65535
6 participants
@haozhangami @ysksuzuki @derailed @joestringer @nbusseneau @julianwiedmann
0