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

daemon: Check nodePortMax < ephemeralPortMin in agent #10260

Merged
merged 4 commits into from
Feb 20, 2020

Conversation

brb
Copy link
Member

@brb brb commented Feb 20, 2020

Previously, if the ephermeral range min port was not greater than the nodeport range max port, the compilation of bpf_netdev was failing with a cryptic message:

    level=warning msg="In file included from /var/lib/cilium/bpf/bpf_overlay.c:39:" subsys=daemon
    level=warning msg="/var/lib/cilium/bpf/lib/nodeport.h:898:3: error: array size is negative" subsys=daemon
    level=warning msg="                build_bug_on(!(NODEPORT_PORT_MAX < EPHERMERAL_MIN));" subsys=daemon
    level=warning msg="
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~" subsys=daemon
    level=warning msg="/var/lib/cilium/bpf/lib/utils.h:124:45: note: expanded from macro 'build_bug_on'" subsys=daemon
    level=warning msg="# define build_bug_on(e) ((void)sizeof(char[1 - 2*!!(e)]))" subsys=daemon
    level=warning msg=" ^~~~~~~~~~~" subsys=daemon
    level=warning msg="3 warnings and 1 error generated." subsys=daemon

Fix this by checking the constraint in the agent, and log a helpful message how to fix it. E.g.:

level=fatal msg="Failed to populate NodePortRange." error="NodePort range (30000-32768) max port must be smaller than ephemeral range (32768-60999) min port. Adjust ephemeral range port with \"sysctl -w net.ipv4.ip_local_port_range='MIN MAX'\"" subsys=config

Also, when running in the non-strict mode (kubeProxyReplacement=probe), we should not panic if the constraint is not met, and just disable the feature.

Keeping the build_bug_on(!(NODEPORT_PORT_MAX < EPHERMERAL_MIN)) check in the datapath code, as it helps readers to understand the NodePort/SNAT code.


This change is Reviewable

brb added 2 commits February 20, 2020 14:15
The function can be used for reading sysctl parameters.

Signed-off-by: Martynas Pumputis <[email protected]>
Previously, if the ephermeral range min port was not greater than the
nodeport range max port, the compilation of bpf_netdev was failing with
a cryptic message:

    level=warning msg="In file included from /var/lib/cilium/bpf/bpf_overlay.c:39:" subsys=daemon
    level=warning msg="/var/lib/cilium/bpf/lib/nodeport.h:898:3: error: array size is negative" subsys=daemon
    level=warning msg="                build_bug_on(!(NODEPORT_PORT_MAX < EPHERMERAL_MIN));" subsys=daemon
    level=warning msg="
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~" subsys=daemon
    level=warning msg="/var/lib/cilium/bpf/lib/utils.h:124:45: note: expanded from macro 'build_bug_on'" subsys=daemon
    level=warning msg="# define build_bug_on(e) ((void)sizeof(char[1 - 2*!!(e)]))" subsys=daemon
    level=warning msg=" ^~~~~~~~~~~" subsys=daemon
    level=warning msg="3 warnings and 1 error generated." subsys=daemon

Fix this by checking the constraint in the agent, and log a helpful
message how to fix it.

Signed-off-by: Martynas Pumputis <[email protected]>
@brb brb requested a review from a team February 20, 2020 14:03
@maintainer-s-little-helper
Copy link

Release note label not set, please set the appropriate release note.

2 similar comments
@maintainer-s-little-helper
Copy link

Release note label not set, please set the appropriate release note.

@maintainer-s-little-helper
Copy link

Release note label not set, please set the appropriate release note.

@brb
Copy link
Member Author

brb commented Feb 20, 2020

test-me-please

@brb brb added the release-note/misc This PR makes changes that have no direct user impact. label Feb 20, 2020
It's a ephemeral port range, not a ephermeral port range.

Signed-off-by: Martynas Pumputis <[email protected]>
@brb brb changed the title option: Check nodePortMax < ephermeralPortMin in agent option: Check nodePortMax < ephemeralPortMin in agent Feb 20, 2020
@brb
Copy link
Member Author

brb commented Feb 20, 2020

test-me-please

@coveralls
Copy link

coveralls commented Feb 20, 2020

Coverage Status

Coverage decreased (-0.03%) to 45.522% when pulling 40a7e8d on pr/brb/check-np-ephermeral-ports into f5d7691 on master.

Otherwise, we might see some users unexpectedly seeing the panic, as in
v1.7 we started to enable NodePort by default.

Signed-off-by: Martynas Pumputis <[email protected]>
@brb brb requested a review from a team February 20, 2020 14:41
@brb brb changed the title option: Check nodePortMax < ephemeralPortMin in agent daemon: Check nodePortMax < ephemeralPortMin in agent Feb 20, 2020
@brb
Copy link
Member Author

brb commented Feb 20, 2020

test-me-please

@brb brb added the priority/high This is considered vital to an upcoming release. label Feb 20, 2020
@borkmann borkmann merged commit bc73555 into master Feb 20, 2020
@borkmann borkmann deleted the pr/brb/check-np-ephermeral-ports branch February 20, 2020 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority/high This is considered vital to an upcoming release. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants