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: Fix back-edge in bpf_sock for older kernels #11739

Merged
merged 1 commit into from May 28, 2020

Conversation

brb
Copy link
Member

@brb brb commented May 28, 2020

The commit 4fa26a4 ("datapath: Enable sessionAffinity for older
kernels") enabled the session affinity feature in bpf_sock when running
on older kernels (e.g. 4.19.57). However, the feature was broken on such
kernels due to the back-edge detected by the BPF verifier:

msg="+ tc exec bpf pin /sys/fs/bpf/tc/globals/cilium_cgroups_connect6 obj bpf_sock.o type sockaddr attach_type connect6 sec connect6" subsys=datapath-loader
subsys=datapath-loader
msg="Prog section 'connect6' rejected: Invalid argument (22)!" subsys=datapath-loader
msg=" - Type:         18" subsys=datapath-loader
msg=" - Attach Type:  11" subsys=datapath-loader
msg=" - Instructions: 740 (0 over limit)" subsys=datapath-loader
msg=" - License:      GPL" subsys=datapath-loader
subsys=datapath-loader
msg="Verifier analysis:" subsys=datapath-loader
subsys=datapath-loader
msg="back-edge from insn 624 to 570" subsys=datapath-loader
subsys=datapath-loader
msg="Error fetching program/map!" subsys=datapath-loader

This was happening, as in the backend reselection case (when a backend from
affinity cannot be found) we goto to previous lines in the code.

Fix it by immediately checking whether the backend from affinity exists.

Fixes: 4fa26a4 ("datapath: Enable sessionAffinity for older kernels")
Reported-by: Paul Chaignon paul@cilium.io

Fix #11731

The commit 4fa26a4 ("datapath: Enable sessionAffinity for older
kernels") enabled the session affinity feature in bpf_sock when running
on older kernels (e.g. 4.19.57). However, the feature was broken on such
kernels due to the back-edge detected by the BPF verifier:

msg="+ tc exec bpf pin /sys/fs/bpf/tc/globals/cilium_cgroups_connect6 obj
bpf_sock.o type sockaddr attach_type connect6 sec connect6" subsys=datapath-loader
subsys=datapath-loader
msg="Prog section 'connect6' rejected: Invalid argument (22)!" subsys=datapath-loader
msg=" - Type:         18" subsys=datapath-loader
msg=" - Attach Type:  11" subsys=datapath-loader
msg=" - Instructions: 740 (0 over limit)" subsys=datapath-loader
msg=" - License:      GPL" subsys=datapath-loader
subsys=datapath-loader
msg="Verifier analysis:" subsys=datapath-loader
subsys=datapath-loader
msg="back-edge from insn 624 to 570" subsys=datapath-loader
subsys=datapath-loader
msg="Error fetching program/map!" subsys=datapath-loader

This was happening, as in the backend reselection case (when a backend from
affinity cannot be found) we goto to previous lines in the code.

Fix it by immediately checking whether the backend from affinity exists.

Fixes: 4fa26a4 ("datapath: Enable sessionAffinity for older kernels")
Reported-by: Paul Chaignon <paul@cilium.io>
Signed-off-by: Martynas Pumputis <m@lambda.lt>
@brb brb 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. needs-backport/1.8 labels May 28, 2020
@brb brb requested a review from a team May 28, 2020 06:29
@maintainer-s-little-helper
Copy link

Please set the appropriate release note label.

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

Please set the appropriate release note label.

@maintainer-s-little-helper
Copy link

Please set the appropriate release note label.

@maintainer-s-little-helper
Copy link

Please set the appropriate release note label.

@brb
Copy link
Member Author

brb commented May 28, 2020

retest-4.19

@brb
Copy link
Member Author

brb commented May 28, 2020

retest-net-next

@brb
Copy link
Member Author

brb commented May 28, 2020

retest-runtime

@brb brb requested a review from borkmann May 28, 2020 06:31
@brb brb added the release-note/bug This PR fixes an issue in a previous release of Cilium. label May 28, 2020
Copy link
Member

@borkmann borkmann left a comment

Choose a reason for hiding this comment

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

lgtm

@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 36.963% when pulling 2b5f3b2 on pr/brb/fix-back-edge-bpf-sock into 3cf91ac on master.

@aanm aanm merged commit 1b95d35 into master May 28, 2020
1.8.0 automation moved this from In progress to Merged May 28, 2020
@aanm aanm deleted the pr/brb/fix-back-edge-bpf-sock branch May 28, 2020 08:09
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. release-note/bug This PR fixes an issue in a previous release of Cilium. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages.
Projects
No open projects
1.8.0
  
Merged
Development

Successfully merging this pull request may close these issues.

CI: Verifier error in connect6: back-edge from insn 624 to 570
6 participants