/test

MBOI @julianwiedmann

Changes LGTM. Could you apply the following:

diff --git a/bpf/tests/inter_cluster_snat_clusterip_client_lxc.c b/bpf/tests/inter_cluster_snat_clusterip_client_lxc.c
index 378cbb4dc0..846071d1b5 100644
--- a/bpf/tests/inter_cluster_snat_clusterip_client_lxc.c
+++ b/bpf/tests/inter_cluster_snat_clusterip_client_lxc.c
@@ -60,6 +60,8 @@
 /* Set the LXC source address to be the address of pod one */
 ASSIGN_CONFIG(union v4addr, endpoint_ipv4, { .be32 = CLIENT_IP})

+ASSIGN_CONFIG(__u32, security_label, 0x10042)
+
 #include "lib/ipcache.h"
 #include "lib/lb.h"
 #include "lib/policy.h"
@@ -158,6 +160,7 @@ int lxc_to_overlay_syn_check(struct __ctx_buff *ctx)
        struct iphdr *l3;
        struct ipv4_ct_tuple tuple;
        struct ct_entry *entry;
+       __u32 cluster_id;

        test_init();

@@ -232,6 +235,11 @@ int lxc_to_overlay_syn_check(struct __ctx_buff *ctx)
        if (!entry)
                test_fatal("couldn't find egress conntrack entry");

+       cluster_id = ctx_get_cluster_id_mark(ctx);
+       if (cluster_id != BACKEND_CLUSTER_ID)
+               test_fatal("ctx->mark cluster_id should be %u, got %u",
+                          BACKEND_CLUSTER_ID, cluster_id);
+
        test_finish();

Currently, it fails when running on the main branch with make run_bpf_tests BPF_TEST="inter_cluster_snat_clusterip_client_lxc":

--- FAIL: TestBPF/inter_cluster_snat_clusterip_client_lxc.o/01_lxc_to_overlay_syn (0.00s)

    bpf_test.go:558: inter_cluster_snat_clusterip_client_lxc.c:241: ctx->mark cluster_id should be 2, got 3

Could you apply the following:

Added, thanks a lot!

/test

Rather than wiping out the entire content, we preserve the "magic" part, which is not expected to interfere.

@giorio94
I think that's incorrect - you should be absolutely making sure that the magic part is set to exactly the desired value. And that the rest of the skb->mark is consistent with the semantics of that magic value.

Looking at ctx_get_cluster_id_mark(), this actually feels like the root cause of the problem - this should be using MARK_MAGIC_HOST_MASK as mask, to avoid aliasing with bit-wise overlapping magic values. Right now it's extracting a cluster_id from MARK_MAGIC_IDENTITY marks (and many other magic values), when it should bail out instead.

Rather than wiping out the entire content, we preserve the "magic" part, which is not expected to interfere.

@giorio94 I think that's incorrect - you should be absolutely making sure that the magic part is set to exactly the desired value. And that the rest of the skb->mark is consistent with the semantics of that magic value.

Looking at ctx_get_cluster_id_mark(), this actually feels like the root cause of the problem - this should be using MARK_MAGIC_HOST_MASK as mask, to avoid aliasing with bit-wise overlapping magic values. Right now it's extracting a cluster_id from MARK_MAGIC_IDENTITY marks (and many other magic values), when it should bail out instead.

Thanks a lot for the feedback. I've took a stab at fixing this in #43258. Let me know if that makes sense to you (I don't have much context in this area).