Skip to content

Conversation

julianwiedmann
Copy link
Member

#28920 introduced usage of CB_FROM_TUNNEL in the IPv6 ingress tail-call. But unless all calling programs get upgraded/downgraded in very specific order (so that while this version of the tail-call is installed, all in-flight skbs have their CB_FROM_TUNNEL populated), some packets potentially miss the tunnel-specific processing that tail_ipv6_policy() is meant to apply. In particular their type might not get switched to PACKET_HOST, resulting in drops with SKB_DROP_REASON_OTHERHOST.

So we need to take a two-step approach - first roll out the support for CB_FROM_TUNNEL into all callers, but wait until a subsequent release (1.16) before tail_ipv6_policy() starts using it. Thus even on a downgrade to 1.15, all callers will continue to populate CB_FROM_TUNNEL.

Reported-by: Martynas Pumputis m@lambda.lt

cilium#28920 introduced usage of
CB_FROM_TUNNEL in the IPv6 ingress tail-call. But unless all calling
programs get upgraded/downgraded in *very* specific order (so that while
this version of the tail-call is installed, *all* in-flight skbs have their
CB_FROM_TUNNEL populated), some packets potentially miss the
tunnel-specific processing that tail_ipv6_policy() is meant to apply. In
particular their type might not get switched to PACKET_HOST, resulting in
drops with SKB_DROP_REASON_OTHERHOST.

So we need to take a two-step approach - first roll out the support for
CB_FROM_TUNNEL into all callers, but wait until a subsequent release (1.16)
before tail_ipv6_policy() starts using it. Thus even on a downgrade to
1.15, all callers will continue to populate CB_FROM_TUNNEL.

Reported-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
@julianwiedmann julianwiedmann 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. release-note/misc This PR makes changes that have no direct user impact. feature/ipv6 Relates to IPv6 protocol support labels Nov 21, 2023
@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann julianwiedmann requested a review from brb November 21, 2023 13:28
@julianwiedmann julianwiedmann added the dont-merge/discussion A discussion is ongoing and should be resolved before merging, regardless of reviews & tests status. label Nov 21, 2023
@julianwiedmann julianwiedmann marked this pull request as ready for review November 21, 2023 13:29
@julianwiedmann julianwiedmann requested a review from a team as a code owner November 21, 2023 13:29
brb added a commit that referenced this pull request Nov 21, 2023
Signed-off-by: Martynas Pumputis <m@lambda.lt>
@julianwiedmann julianwiedmann removed the dont-merge/discussion A discussion is ongoing and should be resolved before merging, regardless of reviews & tests status. label Nov 21, 2023
Copy link
Member

@brb brb left a comment

Choose a reason for hiding this comment

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

Thanks!

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 22, 2023
@brb brb merged commit ad12010 into cilium:main Nov 22, 2023
@julianwiedmann julianwiedmann deleted the 1.15-bpf-lxc-updown branch November 22, 2023 07:05
brb added a commit that referenced this pull request Nov 22, 2023
Signed-off-by: Martynas Pumputis <m@lambda.lt>
julianwiedmann added a commit to julianwiedmann/cilium that referenced this pull request Jan 15, 2024
This workaround was added by cilium#29304 to
deal with up-/downgrade troubles between v1.14 and v1.15.

As the comment says, we can now remove this workaround for the 1.16 devel
cycle.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
github-merge-queue bot pushed a commit that referenced this pull request Jan 15, 2024
This workaround was added by #29304 to
deal with up-/downgrade troubles between v1.14 and v1.15.

As the comment says, we can now remove this workaround for the 1.16 devel
cycle.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
derailed pushed a commit to derailed/cilium that referenced this pull request Jan 24, 2024
This workaround was added by cilium#29304 to
deal with up-/downgrade troubles between v1.14 and v1.15.

As the comment says, we can now remove this workaround for the 1.16 devel
cycle.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
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. feature/ipv6 Relates to IPv6 protocol support kind/bug This is a bug in the Cilium logic. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. 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.

2 participants