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: 4877 bytes --]

On Tue, Jul 22, 2025 at 12:12:25PM +1000, David Gibson wrote:
> 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.

Following on from this, having read more of the series:

In this case (unlike the guest ARP handling) the peer's MAC won't
change (as it appears to the guest), because we set it in the flow
then never update it.  At the very least that means that this
basically won't work reliably for any protocol where the guest is
making first contact to the peers (and the host hasn't happened to
communicate with those same neighbours).  They'll all just appear as
our_tap_mac to the guest on the first flow, then may appear either as
that or the correct map on later flows depending on whether the host's
neighbour entry timed out or not.

It also has another weird consequence: we'll keep using the original
MAC we picked for the flow, even if we're concurrently answering ARPs
from the guest with a different MAC.

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