Re: [PATCH v2 3/8] flow: add mac address of LAN local remote hosts to flow

[Stefano Brivio <sbrivio@redhat.com>]

On Thu, 12 Jun 2025 00:21:47 -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 flowside structure is a convenient
> location for storing that address, so we do that in this commit.
> 
> Note that we don´t add usage of this address in this commit, - that
> will come in later commits.
> 
> Signed-off-by: Jon Maloy <jmaloy@redhat.com>
> ---
>  flow.c | 13 ++++++++++++-
>  flow.h |  2 ++
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/flow.c b/flow.c
> index da5c813..fffc817 100644
> --- a/flow.c
> +++ b/flow.c
> @@ -20,6 +20,7 @@
>  #include "flow.h"
>  #include "flow_table.h"
>  #include "repair.h"
> +#include "netlink.h"
>  
>  const char *flow_state_str[] = {
>  	[FLOW_STATE_FREE]	= "FREE",
> @@ -438,7 +439,7 @@ struct flowside *flow_target(const struct ctx *c, union flow *flow,
>  {
>  	char estr[INANY_ADDRSTRLEN], fstr[INANY_ADDRSTRLEN];
>  	struct flow_common *f = &flow->f;
> -	const struct flowside *ini = &f->side[INISIDE];
> +	struct flowside *ini = &f->side[INISIDE];
>  	struct flowside *tgt = &f->side[TGTSIDE];
>  	uint8_t tgtpif = PIF_NONE;
>  
> @@ -446,10 +447,16 @@ struct flowside *flow_target(const struct ctx *c, union flow *flow,
>  	ASSERT(f->type == FLOW_TYPE_NONE);
>  	ASSERT(f->pif[INISIDE] != PIF_NONE && f->pif[TGTSIDE] == PIF_NONE);
>  	ASSERT(flow->f.state == FLOW_STATE_INI);
> +	memcpy(ini->mac, c->our_tap_mac, ETH_ALEN);
> +	memcpy(tgt->mac, c->our_tap_mac, ETH_ALEN);
>  
>  	switch (f->pif[INISIDE]) {
>  	case PIF_TAP:
>  		tgtpif = fwd_nat_from_tap(c, proto, ini, tgt);
> +
> +		/* If remote host on local network - insert its mac address */
> +		if (!memcmp(&tgt->eaddr, &ini->oaddr, sizeof(ini->oaddr)))
> +			nl_mac_get(nl_sock, &ini->oaddr, ini->mac);
>  		break;
>  
>  	case PIF_SPLICE:
> @@ -458,6 +465,10 @@ struct flowside *flow_target(const struct ctx *c, union flow *flow,
>  
>  	case PIF_HOST:
>  		tgtpif = fwd_nat_from_host(c, proto, ini, tgt);
> +
> +		/* If remote host on local network - insert its mac address */
> +		if (!memcmp(&tgt->oaddr, &ini->eaddr, sizeof(ini->eaddr)))
> +			nl_mac_get(nl_sock, &tgt->oaddr, tgt->mac);
>  		break;
>  
>  	default:
> diff --git a/flow.h b/flow.h
> index cac618a..916951b 100644
> --- a/flow.h
> +++ b/flow.h
> @@ -143,12 +143,14 @@ extern const uint8_t flow_proto[];
>   * @oaddr:	Our address (local address from passt's PoV)
>   * @eport:	Endpoint port
>   * @oport:	Our port
> + * @mac:	MAC address of remote endpoint
>   */
>  struct flowside {
>  	union inany_addr	oaddr;
>  	union inany_addr	eaddr;
>  	in_port_t		oport;
>  	in_port_t		eport;
> +	unsigned char 		mac[6];

We'll never have two MAC addresses that are not c->our_tap_mac in a
single flow, right? If that's the case, shouldn't we move this to
flow_common, so that we have just one instance, reflecting our usage?

>  };
>  
>  /**

-- 
Stefano