Re: [PATCH 2/8] tap: Move all-ones initialisation of mac_guest to tap_sock_init()

["Richard W.M. Jones" <rjones@redhat.com>]

On Thu, May 23, 2024 at 10:59:32AM +0100, Richard W.M. Jones wrote:
> On Wed, May 22, 2024 at 10:59:05PM +0200, Stefano Brivio wrote:
> > It has nothing to do with tap_sock_unix_init(). It used to be there as
> > that function could be called multiple times per passt instance, but
> > it's not the case anymore.
> > 
> > This also takes care of the fact that, with --fd, we wouldn't set the
> > initial MAC address, so we would need to wait for the guest to send us
> > an ARP packet before we could exchange data.
> > 
> > Fixes: 6b4e68383c66 ("passt, tap: Add --fd option")
> > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> > ---
> >  tap.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/tap.c b/tap.c
> > index 91fd2e2..177fe26 100644
> > --- a/tap.c
> > +++ b/tap.c
> > @@ -1111,12 +1111,6 @@ static void tap_sock_unix_init(struct ctx *c)
> >  	if (fd < 0)
> >  		die("UNIX socket: %s", strerror(errno));
> >  
> > -	/* In passt mode, we don't know the guest's MAC until it sends
> > -	 * us packets.  Use the broadcast address so our first packets
> > -	 * will reach it.
> > -	 */
> > -	memset(&c->mac_guest, 0xff, sizeof(c->mac_guest));
> > -
> >  	for (i = 1; i < UNIX_SOCK_MAX; i++) {
> >  		char *path = addr.sun_path;
> >  		int ex, ret;
> > @@ -1312,6 +1306,12 @@ void tap_sock_init(struct ctx *c)
> >  	if (c->mode == MODE_PASST) {
> >  		if (c->fd_tap_listen == -1)
> >  			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.
> > +		 */
> > +		memset(&c->mac_guest, 0xff, sizeof(c->mac_guest));
> >  	} else {
> >  		tap_sock_tun_init(c);
> >  	}
> 
> Reading tap.c, the effect of this is that memset will also be called
> when c->fd_tap_listen is set (the --fd option).  As c cannot be NULL

Reading the next patch, I see that fd_tap_listen *isn't* the --fd
option ...  However that doesn't change the analysis.

> and c->mac_guest exists, this seems safe.
> 
> Acked-by: Richard W.M. Jones <rjones@redhat.com>

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
nbdkit - Flexible, fast NBD server with plugins
https://gitlab.com/nbdkit/nbdkit