Re: [PATCH v12 9/9] icmp: let icmp use mac address from flowside structure

On Thu, Oct 02, 2025 at 08:34:12PM -0400, Jon Maloy wrote:

Even ICMP needs to be updated to use the external MAC address instead of just the own tap address when applicable. We do that here.

Signed-off-by: Jon Maloy Reviewed-by: David Gibson

--- v3: - Adapted to the move of external MAC address from struct flowside to struct flow_common v4: - Adapted to name changes in previous commits in this series v5: - Added conditional lookup in ARP/NDP if the flow's tap_omac is undefined v6: - Looking up MAC of ICMP generating node in udp_send_tap_icmp4/6() when available, instead trusting the contents of flow->tap_omac. v12: - Using MAC_IS_UNDEF() instead of MAC_IS_ZERO() - Comment update after feedback from Stefano --- icmp.c | 8 ++++++-- ndp.c | 2 +- tap.c | 10 ++++++---- tap.h | 4 ++-- udp.c | 12 ++++++++++-- 5 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/icmp.c b/icmp.c index 6dffafb..93b394a 100644 --- a/icmp.c +++ b/icmp.c @@ -125,17 +125,21 @@ void icmp_sock_handler(const struct ctx *c, union epoll_ref ref) flow_dbg(pingf, "echo reply to tap, ID: %"PRIu16", seq: %"PRIu16, ini->eport, seq);

+ /* Find if neighbour table has a recorded MAC address */ + if (MAC_IS_UNDEF(pingf->f.tap_omac)) + fwd_neigh_mac_get(c, &ini->oaddr, pingf->f.tap_omac);

Again a lookup by guest address. Actually looking at these later patches, having the table stored in terms of guest address probably makes more sense than my earlier suggestions. That means the NAT needs to move where you populate the table from host updates. -- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson