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

[Stefano Brivio <sbrivio@redhat.com>]

On Sat, 27 Sep 2025 15:25:22 -0400
Jon Maloy <jmaloy@redhat.com> 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 <jmaloy@redhat.com>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> 
> ---
> 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.
> ---
>  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..1d99632 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);
>  
> +	/* Try to find true MAC address in ARP/NDP table if needed */

Same as for 4/9: consider using "recorded".

Everything else looks good to me! But I haven't tried using my little
hammer on it, yet.

-- 
Stefano