Skip to content

Conversation

julianwiedmann
Copy link
Member

Provide easy access to the security identity which is embedded into Cilium's overlay traffic. And start making use of it in the encrypted-overlay path, to avoid some manual packet parsing.

@julianwiedmann julianwiedmann added area/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. area/encryption Impacts encryption support such as IPSec, WireGuard, or kTLS. release-note/misc This PR makes changes that have no direct user impact. feature/ipsec Relates to Cilium's IPsec feature labels Jun 6, 2024
@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann julianwiedmann marked this pull request as ready for review June 6, 2024 19:51
@julianwiedmann julianwiedmann requested review from a team as code owners June 6, 2024 19:51
@julianwiedmann julianwiedmann requested a review from pchaigno June 6, 2024 19:51
@julianwiedmann
Copy link
Member Author

oho! CI says that we sometimes can't access the tunnel_key in to-overlay. Now why would that be ... back to draft we go.

@julianwiedmann julianwiedmann marked this pull request as draft June 6, 2024 20:37
@ldelossa
Copy link
Contributor

ldelossa commented Jun 7, 2024

@julianwiedmann

Cilium is registering drops before the CI connectivity tests are even running:

root@cilium-testing-control-plane:/home/cilium# cilium metrics list | grep drop
cilium_drop_bytes_total                                  direction="EGRESS" reason="Error retrieving tunnel key"                                440.000000
cilium_drop_bytes_total                                  direction="EGRESS" reason="Unsupported L3 protocol"                                    1698.000000
cilium_drop_count_total                                  direction="EGRESS" reason="Error retrieving tunnel key"                                4.000000
cilium_drop_count_total                                  direction="EGRESS" reason="Unsupported L3 protocol"                                    21.000000

These are the metrics the tests look for, but I grabbed them on a local cluster before any connectivity tests run.
We are dropping this cilium_vxlan traffic pretty early right after a fresh cluster is setup.

Successive runs of the connectivity tests do not bump this counter.

Can we think of any reasons vxlan traffic without a tunnel key would be routed to cilium-vxlan device during the early phases of Cilium deployment? Maybe some half-state during cilium-vxlan device setup?

@ldelossa
Copy link
Contributor

ldelossa commented Jun 7, 2024

@julianwiedmann

So I wanted to catch what traffic was bumping the drop counters.

I created a dummy interface on the Cilium host before cilium started and let TCPDUMP just sit there.
I updated the patch to do this:

	if (unlikely(ctx_get_tunnel_key(ctx, &tunnel_key,
					TUNNEL_KEY_WITHOUT_SRC_IP, 0) < 0)) {
		ret = ctx_redirect(ctx, 999, 0);
		/* ret = DROP_NO_TUNNEL_KEY; */
		goto out;
	}

Where 999 is my dummy interface.

I did wind up catching the traffic:

root@cilium-testing-control-plane:/# tcpdump -nnn -i debug
tcpdump: verbose output suppressed, use -v[v]... for full protocol decode
listening on debug, link-type EN10MB (Ethernet), snapshot length 262144 bytes
18:32:15.033109 IP6 :: > ff02::1:ff75:c503: ICMP6, neighbor solicitation, who has fe80::f412:28ff:fe75:c503, length 32
18:32:15.160062 IP6 :: > ff02::16: HBH ICMP6, multicast listener report v2, 2 group record(s), length 48
18:32:16.056217 IP6 fe80::f412:28ff:fe75:c503 > ff02::16: HBH ICMP6, multicast listener report v2, 3 group record(s), length 68
18:32:16.060055 IP6 fe80::f412:28ff:fe75:c503 > ff02::16: HBH ICMP6, multicast listener report v2, 1 group record(s), length 28
18:32:16.065082 IP6 fe80::f412:28ff:fe75:c503 > ff02::16: HBH ICMP6, multicast listener report v2, 3 group record(s), length 68
18:32:16.888174 IP6 fe80::f412:28ff:fe75:c503 > ff02::16: HBH ICMP6, multicast listener report v2, 1 group record(s), length 28

Looks like we are catching IPv6 related broadcast/NDP traffic, I'm assuming when the interface gets a link-local address assigned for IPv6... Pretty fun 😆

cilium_vxlan: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default 
    link/ether f6:12:28:75:c5:03 brd ff:ff:ff:ff:ff:ff
    inet6 fe80::f412:28ff:fe75:c503/64 scope link 
       valid_lft forever preferred_lft forever

I'm kinda assuming that these packets are generated when we set the link up. Maybe, we are recording metrics a bit too early?

@julianwiedmann
Copy link
Member Author

@ldelossa 🙏 heh, so it was Control plane traffic. Nice!

Think we can just tolerate the missing tunnel_key then for now - both vxlan and geneve would end up dropping such traffic later. With the minor difference that vxlan.ko doesn't increment the tx_dropped counter ...

Long-term we could maybe also just set the disable_ipv6 sysctl on the overlay interface, to deal with this specific case.

@julianwiedmann julianwiedmann force-pushed the 1.16-bpf-overlay-identity branch 2 times, most recently from a9a1867 to c31c96b Compare June 10, 2024 14:57
@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann julianwiedmann marked this pull request as ready for review June 10, 2024 16:09
Provide easy access to the security identity which is embedded into
Cilium's overlay traffic. And start making use of it in the
encrypted-overlay path, to avoid some manual packet parsing.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
@julianwiedmann julianwiedmann force-pushed the 1.16-bpf-overlay-identity branch from c31c96b to eec5b87 Compare June 10, 2024 16:11
@julianwiedmann
Copy link
Member Author

/test

Copy link
Contributor

@ldelossa ldelossa left a comment

Choose a reason for hiding this comment

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

Changes LGTM.

@pchaigno pchaigno requested review from rgo3 and removed request for pchaigno June 12, 2024 10:09
@julianwiedmann julianwiedmann added this pull request to the merge queue Jun 13, 2024
Merged via the queue into cilium:main with commit b429b08 Jun 13, 2024
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. area/encryption Impacts encryption support such as IPSec, WireGuard, or kTLS. feature/ipsec Relates to Cilium's IPsec feature 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.

3 participants