bpf:nat: restore a NAT entry if its REV NAT is not found
NAT Map LRU behavior could evict any entry in the MAP. This change restore a reverse NAT entry when it is expected but not found. Previously, we were only relying on existence of forwarding NAT to determine if the NAT pair exists or not.
This fix is verified by using the same repro steps in #34833. After the fix, we saw much less timeout from the client, but the case like eviction happens between TCP SYN and TCP SYN ACK will still drop the returning packets.
Fixes: #34833
/test
Commit 50ad1eb429bee80a7480fdeb58c04cc20dc37fd4 does not match "(?m)^Signed-off-by:".
Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin
Hi @joestringer, let me know if I need to add anything else to this PR.
/test
@gentoo-root Do you have intentions to review this PR or can you find someone else from @cilium/sig-datapath ?
Could you please also add the IPv6 part of the fix?
Agree with Tom that this needs tests. I could see a
bpf_hosttest underbpf/tests/that walks through the desired packet flow, and reproduces the whole scenario you're fixing:
- transmit a packet, have it SNATed
- receive a reply for that connection, check that it gets revSNATed
- manually delete the revSNAT entry
- receive a second reply, check that it no longer gets revSNATed
- transmit a second packet, validate that it re-creates the missing state
- receive a third reply, check that it gets revSNATed again
@julianwiedmann There are existing flows for 1 and 2. Adding following tests to cover 3 to 6. And I also added the support for ipv6.
Commit 4fd8da215d6d5b463e1a980cfbaba05556cc2330 does not match "(?m)^Signed-off-by:".
Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin
/test
/test
Commit f2261bdb10469d13bfe68cad4217ccee28cd4af3 does not match "(?m)^Signed-off-by:".
Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin
Thanks, these changes all look good to me.
@julianwiedmann do you mind taking another look as well?
lgtm, thank you! I still feel this is a bit hack-ish, and hope we can find a solution to this issue that covers the whole problem space.
@julianwiedmann +1. I remember we have a on-going effort to combine fwd NAT and rev NAT into a single entry? That could eliminate all the complexity here.
/test
Commit eb6cf24c65fdc6ecbac7f56070487a1d091f776c does not match "(?m)^Signed-off-by:".
Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin
/test