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

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

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

On Sun, Jun 29, 2025 at 01:13:42PM -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>
> 
> ---
> 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.)

Sorry, I probably wasn't entirely clear there.  There are two related
but distinct concepts here.  There's the host facing 'emac' - the real
MAC address of the host neighbour.  Then there's the guest facing
'omac', the source MAC address we use when sending on the tap
interface.

[There's also conceptually a host facing 'omac' - the host's MAC on
 the host interface, and a guest facing 'emac' - the guest's MAC
 address on the tap interface, but those aren't really relevant here]

The whole point of this series is to make guest-omac equal host-emac
when possible - but it's not always possible.  Given that, guest-omac
is the one it makes sense to track. That's the one we actually need to
put into packets.  When the peer isn't in the neighbour table the
guest-omac is well defined (our_tap_mac) whereas the host-emac is
simply unknown (and might or might not become known later).

But now that this is in flow_common, instead of flowside, we do need
to disambiguate which side we're talking about.  So it should probably
be 'tap_omac'.

>     - Adapted to new signature of function nl_mac_get(), now passing
>       it the index of the template interface.
> ---
>  flow.c | 21 ++++++++++++++++++++-
>  flow.h |  2 ++
>  2 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/flow.c b/flow.c
> index da5c813..dcda1a7 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,28 @@ 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;
> +	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->omac, c->our_tap_mac, ETH_ALEN);
>  
>  	switch (f->pif[INISIDE]) {
>  	case PIF_TAP:
>  		tgtpif = fwd_nat_from_tap(c, proto, ini, tgt);
> +
> +		/* If there was no NAT, chances are this is a remote host
> +		 * on the template interface's local network segment.
> +		 * If so, insert its MAC address
> +		 */
> +		ifi = inany_v4(&ini->oaddr) ? c->ifi4 : c->ifi6;
> +		if (inany_equals(&ini->oaddr, &tgt->eaddr))
> +			nl_mac_get(nl_sock, &ini->oaddr, ifi, f->omac);

Again, if this is the first time we're contacting that peer, it won't
be in the ARP table yet.

>  		break;
>  
>  	case PIF_SPLICE:
> @@ -458,6 +469,14 @@ 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 was no NAT, chances are this is a remote host
> +		 * on the template interface's local network segment.
> +		 * If so, insert its MAC address
> +		 */
> +		ifi = inany_v4(&ini->eaddr) ? c->ifi4 : c->ifi6;
> +		if (inany_equals(&ini->eaddr, &tgt->oaddr))
> +			nl_mac_get(nl_sock, &ini->eaddr, ifi, f->omac);

In this case it's fine, since the flow is coming from the host side,
the peer must have contacted the host, so we can safely expect it to
be in the host neighbour table.

>  		break;
>  
>  	default:
> diff --git a/flow.h b/flow.h
> index cac618a..3240fb7 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
> + * @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		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 --]