Re: [PATCH] Send an initial ARP request to resolve the guest IP address

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

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

On Sun, Sep 07, 2025 at 01:01:08PM +0200, Volker Diels-Grabsch wrote:
> When restarting passt while QEMU keeps running with a configured
> "reconnect-ms" setting, the port forwardings will stop working until
> the guest sends some outgoing network traffic.
> 
> Reason: Although QEMU reconnects successfully to the unix domain
> socket of the new passt process, that one no longer knows the guest's
> MAC address and uses instead a broadcast MAC address.  However, this
> is ignored by the guest, at least if the guest runs Linux.

Huh... I thought Linux would respond to that.  I wonder if there's
some sysctl config or version difference going on here.

> Only after
> the guest sends some network package on its own initiative, passt will

[Aside: Although "packet" and "package" usually mean the same thing in
 English, it's always "network packets" not "network packages" and
 always "software packages" not "software packets"]

> know the MAC address and will be able to establish forwarded
> connections.
> 
> This change fixes this issue by sending an ARP request to resolve the
> guest's MAC address via its IP address, which we do know, right after
> the unix domain socket (re)connection.

Right.  The guest doesn't necessarily use the IP we give it, but it
usually will, so this will work most of the time.

> The only case where the IP is "wrong" would be if the configuration
> changed, and/or on the very first start right after qemu started.  But
> in those cases, we just wouldn't get an ARP response, and can't do
> anything until we receive the guest's DHCP request - just as before.
> In other words, in the worst case an ARP request would be harmless.

Right.  This seems like a good idea to me.  It won't help in all
cases, but it will help in some, and I can't see any harm it could do.

Some comments on the implementation details below.

> Signed-off-by: Volker Diels-Grabsch <v@njh.eu>
> ---
>  arp.c | 34 ++++++++++++++++++++++++++++++++++
>  arp.h |  1 +
>  tap.c | 15 +++++++++++++--
>  3 files changed, 48 insertions(+), 2 deletions(-)
> 
> diff --git a/arp.c b/arp.c
> index 44677ad..561581a 100644
> --- a/arp.c
> +++ b/arp.c
> @@ -112,3 +112,37 @@ int arp(const struct ctx *c, struct iov_tail *data)
>  
>  	return 1;
>  }
> +
> +/**
> + * send_initial_arp_req() - Send initial ARP request to retrieve guest MAC address

Wrap to 80 columns, please.

Also, since this is exposed from this module the name should start
with 'arp_'.

> + * @c:		Execution context
> + */
> +void send_initial_arp_req(const struct ctx *c)
> +{
> +	struct {
> +		struct ethhdr eh;
> +		struct arphdr ah;
> +		struct arpmsg am;
> +	} __attribute__((__packed__)) req;
> +
> +	/* Ethernet header */
> +	req.eh.h_proto = htons(ETH_P_ARP);
> +	memcpy(req.eh.h_dest, c->guest_mac, sizeof(req.eh.h_dest));

At this point we expect guest_mac to be the broadcast address, but it
seems like explicitly using broadcast would be a little more robust if
that changes at some point.

> +	memcpy(req.eh.h_source, c->our_tap_mac, sizeof(req.eh.h_source));
> +
> +	/* ARP header */
> +	req.ah.ar_op = htons(ARPOP_REQUEST);
> +	req.ah.ar_hrd = htons(ARPHRD_ETHER);
> +	req.ah.ar_pro = htons(ETH_P_IP);
> +	req.ah.ar_hln = ETH_ALEN;
> +	req.ah.ar_pln = 4;
> +
> +	/* ARP message */
> +	memcpy(req.am.sha,	c->our_tap_mac,		sizeof(req.am.sha));
> +	memcpy(req.am.sip,	&c->ip4.our_tap_addr,	sizeof(req.am.sip));
> +	memcpy(req.am.tha,	c->guest_mac,		sizeof(req.am.tha));
> +	memcpy(req.am.tip,	&c->ip4.addr,		sizeof(req.am.tip));
> +
> +	info("sending initial ARP request to retrieve guest MAC address after reconnect");
> +	tap_send_single(c, &req, sizeof(req));
> +}
> diff --git a/arp.h b/arp.h
> index 86bcbf8..5490144 100644
> --- a/arp.h
> +++ b/arp.h
> @@ -21,5 +21,6 @@ struct arpmsg {
>  } __attribute__((__packed__));
>  
>  int arp(const struct ctx *c, struct iov_tail *data);
> +void send_initial_arp_req(const struct ctx *c);
>  
>  #endif /* ARP_H */
> diff --git a/tap.c b/tap.c
> index 7ba6399..47dedd5 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -1355,6 +1355,16 @@ static void tap_start_connection(const struct ctx *c)
>  	ev.events = EPOLLIN | EPOLLRDHUP;
>  	ev.data.u64 = ref.u64;
>  	epoll_ctl(c->epollfd, EPOLL_CTL_ADD, c->fd_tap, &ev);
> +
> +	switch (c->mode) {
> +	case MODE_PASST:
> +		send_initial_arp_req(c);
> +		break;
> +	case MODE_PASTA:
> +		break;
> +	case MODE_VU:
> +		break;
> +	}

I don't think we want to make this conditional on MODE_PASST.  I think
MODE_VU could suffer from the same problem.  So could MODE_PASTA,
although it would take a more unusual setup to trigger it.  In any
case, sending it unconditionally should be at worst harmless and is
simpler.

>  }
>  
>  /**
> @@ -1504,8 +1514,9 @@ void tap_backend_init(struct ctx *c)
>  		tap_sock_unix_init(c);
>  
>  		/* In passt mode, we don't know the guest's MAC address until it
> -		 * sends us packets.  Use the broadcast address so that our
> -		 * first packets will reach it.
> +		 * sends us packets (e.g. responds to our initial ARP request).
> +		 * Until then, use the broadcast address so that our first
> +		 * packets will reach it.
>  		 */
>  		memset(&c->guest_mac, 0xff, sizeof(c->guest_mac));
>  		break;
> -- 
> 2.47.2
> 

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