-
Notifications
You must be signed in to change notification settings - Fork 3.4k
[v1.16] ipam: Fix IPv4 native routing CIDR auto detection #35611
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
[v1.16] ipam: Fix IPv4 native routing CIDR auto detection #35611
Conversation
BPF Masquerading relies on IPV4_SNAT_EXCLUSION_DST_CIDR datapath config value to know which destination CIDRs should not be SNATed for pod to pod traffic. In case of IPAM modes "eni", "azure" or "alibabacloud", if "ipv4-native-routing-cidr" is not explicitly set, the exclusion destination CIDR is autodetected at runtime, specifically from the relevant cloud provider Status section in the CiliumNode CRD. In commit #50e42f1659 the way that autodetection logic propagates that value was changed and it now relies on the LocalNodeStore to emit an update for the new IPv4 native routing CIDR value. But the datapath still reads the value stored in the global config option variable, thus it ignores the update from the autodetection logic. To let the datapath configure the correct SNAT exclusion CIDR, restore the previous logic to update the global config option variable alongside the propagation of the updated value with LocalNodeStore. Related: 50e42f1 ("node: Add ip{v4,v6} native routing CIDR") Fixes: cilium#34615 Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
/test-backport-1.16 |
Is there a |
Here it is: #35624 |
Have you investigated how much work it would be to change the datapath instead? I do feel a bit uneasy on exposing the detected native routing CIDR that way. But I think the PR as is seems safe, i.e. we mutate the I guess the |
My understanding is that we would need the "full-reconciling" orchestrator to react to a runtime update of the ipv4 native routing CIDR and reinitialize the datapath. Alternatively, it is surely possible to add a custom piece of logic to do that, but I don't think it is the best choice to do that just for v1.16.
Yep, this approach is safe for v1.16 because it is basically restoring what was there before #31372. And luckily, the three steps:
happen in that order, so the change is safe. The approach for main takes advantage of the new orchestrator implementation to propagate the new autodetected native routing CIDR, without the risk of modifying the daemon config after the promise has been resolved. |
BPF Masquerading relies on IPV4_SNAT_EXCLUSION_DST_CIDR datapath config value to know which destination CIDRs should not be SNATed for pod to pod traffic.
In case of IPAM modes "eni", "azure" or "alibabacloud", if "ipv4-native-routing-cidr" is not explicitly set, the exclusion destination CIDR is autodetected at runtime, specifically from the relevant cloud provider Status section in the CiliumNode CRD.
In commit #50e42f1659 the way that autodetection logic propagates that value was changed and it now relies on the LocalNodeStore to emit an update for the new IPv4 native routing CIDR value. But the datapath still reads the value stored in the global config option variable, thus it ignores the update from the autodetection logic.
To let the datapath configure the correct SNAT exclusion CIDR, restore the previous logic to update the global config option variable alongside the propagation of the updated value with LocalNodeStore.
Related: 50e42f1 ("node: Add ip{v4,v6} native routing CIDR")
Fixes: #34615