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

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 4118 bytes --]

On Tue, Aug 19, 2025 at 11:09:59PM -0400, Jon Maloy 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>
> ---
>  flow.c | 19 ++++++++++++++++++-
>  flow.h |  2 ++
>  2 files changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/flow.c b/flow.c
> index feefda3..d7b3fd1 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,18 +439,27 @@ 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];

I don't see anywhere you're modifying *ini.  Leftover from an older
draft?

>  	struct flowside *tgt = &f->side[TGTSIDE];
>  	uint8_t tgtpif = PIF_NONE;
> +	int ifi;
>  
>  	ASSERT(flow_new_entry == flow && f->state == FLOW_STATE_INI);
>  	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(f->tap_omac, c->our_tap_mac, ETH_ALEN);
>  
>  	switch (f->pif[INISIDE]) {
>  	case PIF_TAP:
>  		tgtpif = fwd_nat_from_tap(c, proto, ini, tgt);

Attempting to preserve MAC probably only makes sense if tgtpif ==
PIF_HOST (which I think is always true for now, but maybe not
forever).

> +
> +		/* If there is no NAT, the remote host might be on the template

As before, I'm not sure if conditioning this no no NAT makes sense.

> +		 * interface's local network segment. If so, insert its MAC address
> +		 */
> +		ifi = inany_v4(&ini->oaddr) ? c->ifi4 : c->ifi6;
> +		if (!fwd_inany_nat(c, &ini->oaddr))
> +			nl_neigh_mac_get(nl_sock, &ini->oaddr, ifi, f->tap_omac);

It should be tgt->eaddr, rather than ini->oaddr.  They'll usually be
equal, but logically we want to look up based on the host side
address.

As noted in general comments on earlier versions, this lookup will
fail (give our_tap_mac) if the host hasn't contacted this peer before
(so it's not in the neigh table).  Can we defer this lookup until we
get a reply, to avoid that problem?

>  		break;
>  
>  	case PIF_SPLICE:
> @@ -458,6 +468,13 @@ struct flowside *flow_target(const struct ctx *c, union flow *flow,
>  
>  	case PIF_HOST:
>  		tgtpif = fwd_nat_from_host(c, proto, ini, tgt);
> +
> +		/* If there is no NAT, the remote host might be on the template
> +		 * interface's local network segment. If so, insert its MAC address
> +		 */
> +		ifi = inany_v4(&ini->eaddr) ? c->ifi4 : c->ifi6;
> +		if (!fwd_inany_nat(c, &ini->eaddr))
> +			nl_neigh_mac_get(nl_sock, &ini->eaddr, ifi, f->tap_omac);
>  		break;
>  
>  	default:
> diff --git a/flow.h b/flow.h
> index cac618a..29c8bc6 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
>   */
>  struct flow_common {
>  #ifdef __GNUC__
> @@ -192,6 +193,7 @@ struct flow_common {
>  #endif
>  	uint8_t		pif[SIDES];
>  	struct flowside	side[SIDES];
> +	unsigned char tap_omac[6];
>  };
>  
>  #define FLOW_INDEX_BITS		17	/* 128k - 1 */

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]