On Thu, Jun 12, 2025 at 05:17:29PM +0200, Stefano Brivio wrote: > On Thu, 12 Jun 2025 00:21:47 -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 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 > > --- > > 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 So... there are several things to unpack here. 1) This series introduces cases where the statement as written above will not be true any more. Specifically it add cases where the (tap side) MACs are: - a real MAC from the host's LAN and - the guest's MAC Neither of these is our_tap_mac. 1a) I'm guessing you meant c->guest_mac, rather than c->our_tap_mac above. In that case it's true for the time being. It would cease to be true if we started supporting multiple bridged guests behind a single passt/pasta instance (in fact c->guest_mac would have to cease to exist to allow that). 2) If this were to go into flowside, it should be "omac" instead of just "mac". It's specifically "our" MAC in the sense that it's for frames going to/from from passt itself, rather than to/from the guest (a.k.a. the endpoint). 3) I don't think putting this in flowside makes sense for a different reason: putting it in flowside implies we need to track it for both the host side and the tap side. There are not 2, but *4* MAC addresses involved in a flow - just as there are 4 IP addresses. The two MACs on the host side, and the two MACs on the tap side. I don't think the host side omac will ever be useful. We can't control it, and it can change without warning due to reconfiguration on the host. Even without reconfiguration, multiple host interfaces could mean the host side omac is non-constant, and non-trivial to determine for a specific packet/flow. In fact if there's multipath routing, it might not even be constant for a single flow. Host side emac is what we're determining by looking at the host ARP table. But all we're doing with it is using it to initialise the tap-side omac, so I don't think we need to track it separately. It could also change during the life of a flow. If the host interface is point to point (e.g. wireguard) there will be no host side MACs at all. If the host interface is something other than ethernet there might be MACs, but they could have a different format. Tap-side omac is the key new concept in this series: we're allowing it to be set per-flow, rather than being a global constant (our_tap_mac). Tap-side emac remains a global constant (guest_mac) even with this series. As above, we'd need to change that to handle multiple bridged guests on a single passt instance. So, to accomplish what we need for this series, we only really need to track tap-side omac. I agree with Stefano that it makes more sense in flow_common than flowside. In fact, it's even more specific than that, since it doesn't make sense for spliced flows (since they go via guest loopback the guest side traffic has no MACs at all). But we don't currently have a location for pif-specific but not protocol-specific flow information, so flow_common is the closest we have. -- 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