Skip to content

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. area/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
@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
area/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. kind/bug This is a bug in the Cilium logic. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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