[v1.9] bpf: Skip policy enforcements for service loopback case #15709

pchaigno · 2021-04-15T13:03:57Z

bpf: Skip policy enforcements for service loopback case #15321 -- bpf: Skip policy enforcements for service loopback case (@aditighag)
Fix for complexity issue in v1.9 from bpf: Skip policy enforcements for service loopback case #15321
- This will be forward-ported to master (as long as it doesn't cause regressions)
test: Extend the clusterIP tests with policy #15928 -- test: Extend the clusterIP tests with policy (@aditighag)

Once this PR is merged, you can update the PR labels via:

$ for pr in 15321 15928; do contrib/backporting/set-labels.py $pr done 1.9; done

pchaigno · 2021-04-15T13:32:52Z

Smoke test / conformance-test is failing because of a complexity issue.

[ upstream commit 52cd6da ] When an endpoint connects to itself via service clusterIP, we hairpin the flow using a loopback IP address (configured using ipv4-service-loopback-address). The destination clusterIP (on egress) and loopback IP (on ingress) map to unexpected identities. As a result, policy enforcement fails and the packet is dropped. This is visible in the cilium monitor output: <- endpoint 1844 flow 0x96c8d52 identity 55108->unknown state new ifindex 0 orig-ip 0.0.0.0: 10.12.0.123:58242 -> 172.20.0.130:80 tcp SYN Policy verdict log: flow 0x96c8d52 local EP ID 1844, remote ID world, proto 6, egress, action deny, match none, 169.254.42.1:58242 -> 10.12.0.123:80 tcp SYN Since we don't want to enforce policies anyway for the loopback traffic, this commit skips policy enforcements in that case. Co-authored-by: Paul Chaignon <paul@cilium.io> Signed-off-by: Aditi Ghag <aditi@cilium.io> Signed-off-by: Chris Tarazi <chris@isovalent.com>

Upon code inspection, the remote endpoint lookup that retrieves the destination ID is useless because the destination ID is not used when `hairpin_flow` is true. We can skip over this code in hopes that it simplifies the code complexity. Signed-off-by: Chris Tarazi <chris@isovalent.com>

[ upstream commit 69f10ed ] Test for PR #15321 - tests the case where a pod connects to itself via service clusterIP when selected by a policy. Signed-off-by: Aditi Ghag <aditi@cilium.io>

christarazi · 2021-05-03T17:27:27Z

test-backport-1.9

christarazi · 2021-05-03T17:44:40Z

Updated the PR description to convert this PR into a backport PR, now that the complexity issue has been resolved on this branch. Opening for reviews.

Edit: I can't request reviews from Paul as he opened the PR, but I've been pushing to it

aditighag

Nice work! Thank you. 🚀

bpf/bpf_lxc.c

pchaigno

LGTM! I can't leave an approving review because I opened the PR, but that's my intent 🙂

Thanks @christarazi and @aditighag! 🙏

pchaigno added kind/backports This PR provides functionality previously merged into master. backport/1.9 labels Apr 15, 2021

christarazi force-pushed the pr/v1.9-backport-2021-04-15 branch from 4249b51 to 33eb488 Compare April 21, 2021 06:17

maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Apr 21, 2021

christarazi force-pushed the pr/v1.9-backport-2021-04-15 branch from 33eb488 to a0f2bca Compare April 21, 2021 06:19

maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Apr 21, 2021

christarazi force-pushed the pr/v1.9-backport-2021-04-15 branch from a0f2bca to 7ffb743 Compare April 21, 2021 06:56

This comment has been minimized.

Sign in to view

christarazi force-pushed the pr/v1.9-backport-2021-04-15 branch from 7ffb743 to ab506af Compare April 22, 2021 20:38

This comment has been minimized.

Sign in to view

christarazi added the priority/high This is considered vital to an upcoming release. label Apr 26, 2021

christarazi force-pushed the pr/v1.9-backport-2021-04-15 branch 2 times, most recently from 9647f4a to 1b0a3d7 Compare April 27, 2021 16:28

christarazi temporarily deployed to release-developer-images April 27, 2021 16:33 Inactive

christarazi changed the title ~~bpf: Skip policy enforcements for service loopback case~~ [v1.9] bpf: Skip policy enforcements for service loopback case Apr 27, 2021

christarazi force-pushed the pr/v1.9-backport-2021-04-15 branch from 1b0a3d7 to 5100ffb Compare April 27, 2021 21:14

maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Apr 27, 2021

christarazi force-pushed the pr/v1.9-backport-2021-04-15 branch from 5100ffb to ba909fa Compare April 27, 2021 23:13

cilium deleted a comment from maintainer-s-little-helper bot Apr 27, 2021

cilium deleted a comment from maintainer-s-little-helper bot Apr 28, 2021

christarazi force-pushed the pr/v1.9-backport-2021-04-15 branch from 76f2c38 to a4b9c96 Compare May 3, 2021 17:20

christarazi and others added 2 commits May 3, 2021 10:20

test: Extend the hairpin flow test with policy

ce442fb

[ upstream commit 69f10ed ] Test for PR #15321 - tests the case where a pod connects to itself via service clusterIP when selected by a policy. Signed-off-by: Aditi Ghag <aditi@cilium.io>

christarazi force-pushed the pr/v1.9-backport-2021-04-15 branch from a4b9c96 to ce442fb Compare May 3, 2021 17:22

maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label May 3, 2021

cilium deleted a comment from maintainer-s-little-helper bot May 3, 2021

christarazi marked this pull request as ready for review May 3, 2021 17:44

christarazi requested a review from a team as a code owner May 3, 2021 17:44

christarazi requested review from aditighag, a team and borkmann and removed request for a team May 3, 2021 17:45

maintainer-s-little-helper bot assigned aditighag and borkmann May 3, 2021

aditighag approved these changes May 3, 2021

View reviewed changes

maintainer-s-little-helper bot unassigned aditighag May 3, 2021

pchaigno self-assigned this May 3, 2021

michi-covalent added the release-blocker/1.9 label May 3, 2021

aditighag reviewed May 3, 2021

View reviewed changes

bpf/bpf_lxc.c Show resolved Hide resolved

pchaigno commented May 3, 2021

View reviewed changes

pchaigno removed their assignment May 3, 2021

pchaigno added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 5, 2021

ti-mo merged commit a2e844f into v1.9 May 5, 2021

ti-mo deleted the pr/v1.9-backport-2021-04-15 branch May 5, 2021 09:02

christarazi unassigned borkmann May 5, 2021

pchaigno mentioned this pull request May 6, 2021

[v1.9 HF] bpf: Skip policy enforcements for service loopback case #15888

Closed

joestringer mentioned this pull request May 13, 2021

Prepare for release v1.9.7 #16131

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[v1.9] bpf: Skip policy enforcements for service loopback case #15709

[v1.9] bpf: Skip policy enforcements for service loopback case #15709

pchaigno commented Apr 15, 2021 •

edited by christarazi

pchaigno commented Apr 15, 2021

This comment has been minimized.

This comment has been minimized.

christarazi commented May 3, 2021

christarazi commented May 3, 2021 •

edited

aditighag left a comment

pchaigno left a comment

[v1.9] bpf: Skip policy enforcements for service loopback case #15709

[v1.9] bpf: Skip policy enforcements for service loopback case #15709

Conversation

pchaigno commented Apr 15, 2021 • edited by christarazi

pchaigno commented Apr 15, 2021

This comment has been minimized.

This comment has been minimized.

christarazi commented May 3, 2021

christarazi commented May 3, 2021 • edited

aditighag left a comment

Choose a reason for hiding this comment

pchaigno left a comment

Choose a reason for hiding this comment

pchaigno commented Apr 15, 2021 •

edited by christarazi

christarazi commented May 3, 2021 •

edited