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

Fix clustermesh policy with endpoint-routes mode #12694

Merged

Conversation

joestringer
Copy link
Member

@joestringer joestringer commented Jul 28, 2020

In endpoint-routes mode, we encode the source identity in the ctx->mark
when locally routing the packet to the destination device for ingress
policy assessment. Previously we only encoded the local cluster
identity in the mark, thereby omitting the original cluster portion of
the identity.

Found by code inspection.

Fixes: 654303a ("bpf: Skip ingress policy at egress of source if egress prog is in use")

This unlikely to affect many users as by default users do not typically configure clustermesh + endpoint-routes (or ENI) mode at the same time.

@joestringer joestringer added release-note/bug This PR fixes an issue in a previous release of Cilium. needs-backport/1.6 labels Jul 28, 2020
@joestringer joestringer requested review from tgraf and a team July 28, 2020 19:44
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.8.3 Jul 28, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.7.7 Jul 28, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.6.11 Jul 28, 2020
@joestringer joestringer changed the title Fix clustermesh policy with endpoint-routes Fix clustermesh policy with endpoint-routes mode Jul 28, 2020
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.05%) to 37.161% when pulling 1dcdd6c5da8965e24c5f302fd77a080cf4448898 on joestringer:submit/identity-clustermesh-fix into 56a9c1f on cilium:master.

@maintainer-s-little-helper maintainer-s-little-helper bot removed this from Needs backport from master in 1.7.7 Jul 28, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot removed this from Needs backport from master in 1.6.11 Jul 28, 2020
@joestringer
Copy link
Member Author

I dropped the older backport labels as this is unlikely to affect existing users due to the unusual combination of configurations.

@nathanjsweet
Copy link
Member

retest-4.9

@nathanjsweet
Copy link
Member

retest-4.19

@nathanjsweet
Copy link
Member

retest-netnext

@joestringer
Copy link
Member Author

Kernel-4.9 passed: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.19-kernel-4.9/53/
Kernel 4.19 failed, flake #7721: https://jenkins.cilium.io/job/Cilium-PR-Ginkgo-Tests-Kernel/2678/

I'll rekick the entire CI since I forgot before and the trigger attempts from Nate above don't seem to be working.

@joestringer
Copy link
Member Author

test-me-please

@joestringer
Copy link
Member Author

VM provisioning failed on net-next, seems like it timed out for some weird reason: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.12-net-next/66/flowGraphTable/

@joestringer
Copy link
Member Author

retest-net-next

@nathanjsweet
Copy link
Member

retest-gke

@tklauser
Copy link
Member

Looks like this PR needs a rebase to get commit 4ea0ea4 which will fix the Go-related checks / lint action.

@tklauser tklauser added the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Aug 19, 2020
In endpoint-routes mode, we encode the source identity in the ctx->mark
when locally routing the packet to the destination device for ingress
policy assessment. Previously we only encoded the local cluster
identity in the mark, thereby omitting the original cluster portion of
the identity.

Found by code inspection.

Fixes: 654303a ("bpf: Skip ingress policy at egress of source if egress prog is in use")
Signed-off-by: Joe Stringer <joe@cilium.io>
@joestringer joestringer force-pushed the submit/identity-clustermesh-fix branch from 1dcdd6c to e394e8f Compare August 19, 2020 22:18
@joestringer joestringer removed the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Aug 19, 2020
@joestringer
Copy link
Member Author

test-me-please

bpf/lib/l3.h Show resolved Hide resolved
@aanm aanm added this to Needs backport from master in 1.8.4 Sep 4, 2020
@aanm aanm removed this from Needs backport from master in 1.8.3 Sep 4, 2020
@joestringer joestringer merged commit d7433cf into cilium:master Sep 9, 2020
@joestringer joestringer deleted the submit/identity-clustermesh-fix branch September 9, 2020 17:17
@joestringer joestringer moved this from Needs backport from master to Backport done to v1.8 in 1.8.4 Sep 30, 2020
joestringer added a commit to joestringer/cilium that referenced this pull request Oct 26, 2020
When this flag was set to false (not the default), it failed to compile
due to the typo. Fix it.

The bpf build_all target also did not pick up on this, add these options
to the permutations for building.

Fixes: d7433cf ("bpf: Fix clustermesh policy with endpoint-routes")
Fixes: cilium#12694
Signed-off-by: Joe Stringer <joe@cilium.io>
joestringer added a commit that referenced this pull request Oct 27, 2020
When this flag was set to false (not the default), it failed to compile
due to the typo. Fix it.

The bpf build_all target also did not pick up on this, add these options
to the permutations for building.

Fixes: d7433cf ("bpf: Fix clustermesh policy with endpoint-routes")
Fixes: #12694
Signed-off-by: Joe Stringer <joe@cilium.io>
aditighag pushed a commit to aditighag/cilium that referenced this pull request Oct 29, 2020
[ upstream commit e784604 ]

When this flag was set to false (not the default), it failed to compile
due to the typo. Fix it.

The bpf build_all target also did not pick up on this, add these options
to the permutations for building.

Fixes: d7433cf ("bpf: Fix clustermesh policy with endpoint-routes")
Fixes: cilium#12694
Signed-off-by: Joe Stringer <joe@cilium.io>
Signed-off-by: Aditi Ghag <aditi@cilium.io>
joestringer added a commit that referenced this pull request Nov 2, 2020
[ upstream commit e784604 ]

When this flag was set to false (not the default), it failed to compile
due to the typo. Fix it.

The bpf build_all target also did not pick up on this, add these options
to the permutations for building.

Fixes: d7433cf ("bpf: Fix clustermesh policy with endpoint-routes")
Fixes: #12694
Signed-off-by: Joe Stringer <joe@cilium.io>
Signed-off-by: Aditi Ghag <aditi@cilium.io>
tklauser pushed a commit that referenced this pull request Nov 3, 2020
[ upstream commit e784604 ]

When this flag was set to false (not the default), it failed to compile
due to the typo. Fix it.

The bpf build_all target also did not pick up on this, add these options
to the permutations for building.

Fixes: d7433cf ("bpf: Fix clustermesh policy with endpoint-routes")
Fixes: #12694
Signed-off-by: Joe Stringer <joe@cilium.io>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
joestringer added a commit that referenced this pull request Nov 4, 2020
[ upstream commit e784604 ]

When this flag was set to false (not the default), it failed to compile
due to the typo. Fix it.

The bpf build_all target also did not pick up on this, add these options
to the permutations for building.

Fixes: d7433cf ("bpf: Fix clustermesh policy with endpoint-routes")
Fixes: #12694
Signed-off-by: Joe Stringer <joe@cilium.io>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
No open projects
1.8.4
Backport done to v1.8
Development

Successfully merging this pull request may close these issues.

None yet

5 participants