Skip to content

Conversation

julianwiedmann
Copy link
Member

@julianwiedmann julianwiedmann commented Nov 26, 2024

As follow-on to #35636, add support for additional types of ICMP messages that potentially get reflected back by an DEST_UNREACH packet.

When processing DEST_UNREACH messages with an ICMP payload of unsupported
type, return DROP_UNKNOWN_ICMP_CODE instead of DROP_INVALID.

This helps to understand the actual problem, and allows for ignoring such
drops in the cilium CLI tests.

Reported-by: Marco Iorio <marco.iorio@isovalent.com>
Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
Now that the NAT path supports a wider range of DEST_UNREACH code points,
we potentially see more types of embedded ICMP messages.

In the outbound NAT path, we need to reflect ICMP_ECHO messages back
(which for example addressed an unreachable port on the local host).

In the inbound RevNAT path we can potentially see an ICMP_ECHOREPLY
reflected back to the local node, if the addressed port is no longer
reachable on the remote node.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
@julianwiedmann julianwiedmann added area/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. release-note/misc This PR makes changes that have no direct user impact. feature/snat Relates to SNAT or Masquerading of traffic labels Nov 26, 2024
@julianwiedmann julianwiedmann changed the title 1.17 bpf snat icmp invalid bpf: nat: support more embedded ICMP types for DEST_UNREACH packet Nov 26, 2024
@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann julianwiedmann requested a review from jibi November 26, 2024 09:32
@julianwiedmann julianwiedmann marked this pull request as ready for review November 26, 2024 09:32
@julianwiedmann julianwiedmann requested a review from a team as a code owner November 26, 2024 09:32
@julianwiedmann julianwiedmann added this pull request to the merge queue Nov 28, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 28, 2024
Merged via the queue into cilium:main with commit 2caefb4 Nov 28, 2024
70 checks passed
@julianwiedmann julianwiedmann deleted the 1.17-bpf-snat-icmp-invalid branch November 28, 2024 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. feature/snat Relates to SNAT or Masquerading of traffic ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants