-
Notifications
You must be signed in to change notification settings - Fork 3.4k
datapath: fix NodePort to remote hostns backend with tunnel config #22738
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
datapath: fix NodePort to remote hostns backend with tunnel config #22738
Conversation
/test |
0b6e77f
to
7f165a7
Compare
/test-runtime |
/test-runtime |
c0010a5
to
6cccf16
Compare
/test-runtime |
This pull request has been automatically marked as stale because it |
This pull request has not seen any activity since it was marked stale. |
Hey @julianwiedmann, is this change still valid? This issue has been affecting us for a while now, so just wondering if we can reopen this. Thanks! |
When forwarding external service requests to a remote backend in hostNetwork, the NodePort code in tail_nodeport_nat_egress_ipv4() doesn't redirect into the tunnel (as it's effectively a host-to-host connection). Therefore it uses IPV4_DIRECT_ROUTING as src address. So if we're not populating IPV4_DIRECT_ROUTING, such forwarded requests end up being SNATed with 0.0.0.0. Fixes: cilium#22557 Fixes: 5283ec9 ("datapath: Introduce DirectRoutingDeviceRequired helper") Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
66c288b
to
76837ea
Compare
Heya @michaelasp - yeah sorry, lost track of this :/. IIRC the change was hitting troubles in the agent tests (as it was now expecting the direct-routing device to be defined in a lot more tests). Discussed with @joamaki what would need to change there, but never got to it... want to give it a shot? I actually just hit the same problem in a different PR (your timing is spot-on 😄), so hoping we can get this fixed 👍 . |
6cccf16
to
66c288b
Compare
/test-runtime |
Hi @julianwiedmann, do you have any updates on this commit? Looks like the unit test is failing. |
Yes, that's the fallout in agent tests I mentioned in my previous comment. |
This pull request has been automatically marked as stale because it |
This pull request has not seen any activity since it was marked stale. |
Removing backport labels from closed PR, please re-add if you reopen this. |
Hey @julianwiedmann, with the pr being closed, I assume the issue is still present? I remember that due to test issues it was sidelined. I'd be more than happy to take on the work of trying to fix this if necessary :) |
That would be great @michaelasp ! I honestly don't know at what point I would find the cycles to get this wrapped up :/. |
Sounds good! I'll try and get a PR out in the next couple weeks. |
When forwarding external service requests to a remote backend in
hostNetwork, the NodePort code in tail_nodeport_nat_egress_ipv4() doesn't
redirect into the tunnel (as it's effectively a host-to-host connection).
Therefore it uses IPV4_DIRECT_ROUTING as src address. So if we're not
populating IPV4_DIRECT_ROUTING, such forwarded requests end up being SNATed
with 0.0.0.0.
Fixes: #22557
Fixes: 5283ec9 ("datapath: Introduce DirectRoutingDeviceRequired helper")
Signed-off-by: Julian Wiedmann jwi@isovalent.com