Re: [PATCH v11 5/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: 2661 bytes --]

On Tue, Sep 30, 2025 at 11:29:32PM +0200, Stefano Brivio wrote:
> 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.

Good point, I'd forgotten about that.

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

We already have MAC_BROADCAST.

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