datapath: fix NodePort to remote hostns backend with tunnel config

[michaelasp]

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.

This is a copy of #22738 by jwi@isovalent.com, reopening to debug test failures.

Fixes: #22557
Fixes: 5283ec9 ("datapath: Introduce DirectRoutingDeviceRequired helper")

[michaelasp]

After rebasing on top of #24677, a nil check introduced in front of device manager allows the tests to pass. Looks like this can be merged with just these changes 😄

[michaelasp]

@julianwiedmann these are basically the changes you made in #22738 but updated with some of the newer changes in device manager. Tested the agent tests, and they pass now 🥳

[joestringer]

/test

[joestringer]

If this is based on #22738, then please ensure that the commit message properly attributes the origins of the source, ie:

Co-authored-by: Julian Wiedmann <jwi@isovalent.com>

[michaelasp]

Thanks @joestringer! Definitely want to make sure credit is given 👍

[joestringer]

I'm seeing a few CI issues that are due to the PR being based on an older version of main. I'd suggest rebasing against the tip of the main branch and updating the commit message while you're at it.

[michaelasp]

Done!

[joestringer]

/test

[michaelasp]

Fixed issue in TestDetect, other issues seem unrelated. I reverted my changes just in case and I could repro the TestWriteNodeConfig failures locally.

[aojea]

@julianwiedmann as previous owner of the PR can you please help us with the reviews and approval process?

[lmb]

@jibi can you review this please?

@michaelasp please respond to or resolve the open conversations.

[michaelasp]

Hey @lmb sorry for the wait! I just got back to my computer today from a long break, responded 😃.

[jibi]

The rationale for the change makes sense to me, but it could have a broader impact than expected as:

• DirectRoutingDeviceRequired() can now return true even when tunneling is not enabled
• because of that (*DeviceManager) Detect() will try to auto-detect a direct routing device even in tunneling mode
• if we fail to detect a device either bpf nodeport gets disabled or the agent returns an error during initialization

so this PR has a potential upgrade impact as it might break some setups that used to work before 🤔

[jibi]

synced internally, I think in practice these changes are fine 👍

[michaelasp]

Awesome! Glad to hear :)

[lmb]

Removing ready-to-merge since @ti-mo needs to approve for cilium/loader. Not sure why the automation decided this was ready.