Re: [PATCH v11 5/9] flow: add MAC address of LAN local remote hosts to flow

[Stefano Brivio <sbrivio@redhat.com>]

On Sat, 27 Sep 2025 15:25:18 -0400
Jon Maloy <jmaloy@redhat.com> wrote:

> When communicating with remote hosts on the local network, some guest
> applications want to see the real MAC address of that host instead
> of PASST/PASTA's own tap address. The flow_common structure is a
> convenient location for storing that address, so we do that in this
> commit.
> 
> Note that we don´t add actual usage of this address here, that will
> be done in later commits.
> 
> Signed-off-by: Jon Maloy <jmaloy@redhat.com>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> 
> ---
> v3: - Moved the remote host macaddress from struct flowside to
>       struct flow_common. I chose to call it 'omac' as suggested
>       by David, although in my understanding the correct name would be
>       'emac'. (In general I find the address naming scheme confusing.)
>     - Adapted to new signature of function nl_mac_get(), now passing
>       it the index of the template interface.
> v4: - Renamed flow_commeon->omac to flow_common->tap_omac to make is
>       role in the code clearer
> v5: - Modified the criteria for ARP/NDP table lookup like in the
>       previous commits.
>     - Removed the PIF_TAP lookup case, as David suggested, and did
>       instead give the flow->tap_omac field a value marking it as
>       non-initialized.
>     - Calling the cache table instead of netlink for ARP/NDP lookup.
>     - Unconditionally using the potentially translated IP address
>       in the lookup, instead of only if NAT really was applied.
> v6: - Using MAC_ZERO instead of own definitions
> ---
>  flow.c | 2 ++
>  flow.h | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/flow.c b/flow.c
> index feefda3..510f3c5 100644
> --- a/flow.c
> +++ b/flow.c
> @@ -449,6 +449,7 @@ struct flowside *flow_target(const struct ctx *c, union flow *flow,
>  
>  	switch (f->pif[INISIDE]) {
>  	case PIF_TAP:
> +		memcpy(f->tap_omac, MAC_ZERO, ETH_ALEN);

I see in the next patch that this is needed as an invalid value for
f->tap_omac, but MAC_ZERO is actually a valid, usable MAC address.

I guess we should use ff:ff:ff:ff:ff:ff, instead, as MAC_ONES, or
MAC_UNSPEC.

>  		tgtpif = fwd_nat_from_tap(c, proto, ini, tgt);
>  		break;
>  
> @@ -458,6 +459,7 @@ struct flowside *flow_target(const struct ctx *c, union flow *flow,
>  
>  	case PIF_HOST:
>  		tgtpif = fwd_nat_from_host(c, proto, ini, tgt);
> +		fwd_neigh_mac_get(c, &ini->eaddr, f->tap_omac);
>  		break;
>  
>  	default:
> diff --git a/flow.h b/flow.h
> index cac618a..f342895 100644
> --- a/flow.h
> +++ b/flow.h
> @@ -177,6 +177,7 @@ int flowside_connect(const struct ctx *c, int s,
>   * @type:	Type of packet flow
>   * @pif[]:	Interface for each side of the flow
>   * @side[]:	Information for each side of the flow
> + * @tap_omac: MAC address of remote endpoint as seen from the guest

The descriptions of the other fields are aligned with tabs, this has a
single space, so it's not aligned.

>   */
>  struct flow_common {
>  #ifdef __GNUC__
> @@ -192,6 +193,7 @@ struct flow_common {
>  #endif
>  	uint8_t		pif[SIDES];
>  	struct flowside	side[SIDES];
> +	uint8_t		tap_omac[6];
>  };
>  
>  #define FLOW_INDEX_BITS		17	/* 128k - 1 */

-- 
Stefano