On Fri, Feb 07, 2025 at 07:51:15AM +0100, Stefano Brivio wrote:
> On Fri, 7 Feb 2025 17:26:35 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > It kind of seemed like we were sendmsg()ing "and from guest 2" and it
> > was bouncing straight back to our socket, instead of being delivered
> > to the outer pasta.
> 
> Oops.
> 
> diff --git a/tcp.c b/tcp.c
> index 0f05011..b7f5169 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -2796,6 +2796,12 @@ static int tcp_flow_repair_queues(int s,
>  		debug("Read socket %i receive queue: %zi bytes", s, rc);
>  	}
>  
> +	v = TCP_NO_QUEUE;
> +	if (setsockopt(s, SOL_TCP, TCP_REPAIR_QUEUE, &v, sizeof(v))) {
> +		err_perror("Setting TCP_NO_QUEUE on socket %i", s);
> +		return -errno;
> +	}
> +
>  	return 0;
>  }
>  
> 
> otherwise, I guess, there's a time window in which we might be writing
> that message to our queue instead of writing it on the socket, even
> with repair mode off. The write might be pending or something.

Huh.  It somehow never occurred to me to think that repair stuff might
be happening after we turned repair mode off.  Is that actually the
case, or could it be that the passt-repair protocol bug meant repair
mode sometimes wasn't turned off?

If repair mode stuff is happening after turning it off, I'd say that
is a kernel bug, although a much less nasty and more easily worked
around one than I'd feared (I was contemplating whether repair mode
connect() might be looking something us wrong and wiring things up to
the wrong place).

> With this, the stray packet is gone. I spotted this case now:
> 
> $ tshark -r test/test_logs/passt_2.pcap 
>     1   0.000000 88.198.0.164 → 169.254.1.1  TCP 71 58150 → 10006 [PSH, ACK] Seq=1 Ack=1 Win=1024 Len=17
>     2   0.000031 88.198.0.164 → 169.254.1.1  TCP 54 58150 → 10006 [FIN, ACK] Seq=18 Ack=1 Win=1024 Len=0
>     3   0.000059  169.254.1.1 → 88.198.0.164 TCP 54 10006 → 58150 [RST, ACK] Seq=1 Ack=1 Win=256 Len=0
>     4   0.026443  169.254.1.1 → 88.198.0.164 TCP 74 48150 → 10001 [PSH, ACK] Seq=1 Ack=1 Win=256 Len=20
>     5   0.026538 88.198.0.164 → 169.254.1.1  TCP 54 10001 → 48150 [ACK] Seq=1 Ack=21 Win=1023 Len=0
>     6   0.026557  169.254.1.1 → 88.198.0.164 TCP 54 48150 → 10001 [FIN, ACK] Seq=21 Ack=1 Win=256 Len=0
>     7   0.026656 88.198.0.164 → 169.254.1.1  TCP 54 10001 → 48150 [FIN, ACK] Seq=1 Ack=22 Win=1024 Len=0
>     8   0.026675  169.254.1.1 → 88.198.0.164 TCP 54 48150 → 10001 [ACK] Seq=22 Ack=2 Win=256 Len=0
>     9 318.959707      fe80::1 → ff02::1      ICMPv6 158 Router Advertisement from 9a:55:9a:55:9a:55
> 
> That RST we send as frame #3 looks unwarranted. I wonder if we get
> packets from the target guest before we have a chance to set up flows.

I don't think that should be possible: the target guest shouldn't be
resumed until after we ack the check_device_state.

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