From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Gibson To: passt-dev@passt.top Subject: Re: [PATCH 2/2] test/passt_in_ns: Consistent sleep commands before starting socat client Date: Mon, 19 Sep 2022 17:00:23 +1000 Message-ID: In-Reply-To: <20220919083915.475e0295@elisabeth> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1388993954496080649==" --===============1388993954496080649== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable On Mon, Sep 19, 2022 at 08:41:50AM +0200, Stefano Brivio wrote: > On Mon, 19 Sep 2022 11:48:33 +1000 > David Gibson wrote: >=20 > > On Sat, Sep 17, 2022 at 01:28:42PM +0200, Stefano Brivio wrote: > > > On Sat, 17 Sep 2022 20:19:21 +1000 > > > David Gibson wrote: > > > =20 > > > > On Sat, Sep 17, 2022 at 10:44:41AM +0200, Stefano Brivio wrote: =20 > > > > > On Sat, 17 Sep 2022 13:32:45 +1000 > > > > > David Gibson wrote: > > > > > =20 > > > > > > On Sat, Sep 17, 2022 at 01:55:34AM +0200, Stefano Brivio wrote: = =20 > > > > > > > There are some 'sleep 1' commands between starting the socat se= rver > > > > > > > and its corresponding client to avoid races due to the server n= ot > > > > > > > being ready as we start sending data. > > > > > > >=20 > > > > > > > However, those don't cover all the cases where we might need th= em, > > > > > > > and in some cases the sleep command actually ended up being bef= ore > > > > > > > the server even starts. > > > > > > >=20 > > > > > > > This fixes occasional failures in TCP and UDP simple transfer t= ests, > > > > > > > that became apparent with the new command dispatch mechanism. > > > > > > >=20 > > > > > > > Signed-off-by: Stefano Brivio =20 > > > > > >=20 > > > > > > Heh, I have a very similar patch in my upcoming series. I used s= leep > > > > > > 0.1 though, to avoid taking so long, which seems to be sufficient= in > > > > > > practice. =20 > > > > >=20 > > > > > Just mind POSIX only specifies integers as argument for sleep(1), a > > > > > pattern I commonly use is: > > > > >=20 > > > > > sleep 0.1 || sleep 1 =20 > > > >=20 > > > > Ah, right. > > > > =20 > > > > > > I did look for a proper way to wait for the socat server to be re= ady, > > > > > > but I couldn't find anything workable. =20 > > > > >=20 > > > > > I guess we could enable logging and look for "starting accept loop", > > > > > from _xioopen_listen() in xio-listen.c. =20 > > > >=20 > > > > Yeah, I suppose. > > > > =20 > > > > > Anyway, right now, I'm just trying to get the tests to complete with > > > > > all your pending series, because it's been a while. This doesn't lo= ok > > > > > too bad, we can try to improve it later. =20 > > > >=20 > > > > Yeah, fair enough. When I rebase I'll see if there's any refinements > > > > I can make relatively easily. =20 > > >=20 > > > Okay, thanks. > > > =20 > > > > > > I thought I could retry on the > > > > > > client side, but that doesn't work (at least for host to guest > > > > > > connections) because passt is always listening on the forwarded p= orts, > > > > > > so the client won't see a connection refused from the actual serv= er. > > > > > > I don't think there's any sane way to propagate connection refuse= d in > > > > > > that way, although we could and probably should forward connection > > > > > > resets outwards. =20 > > > > >=20 > > > > > They should be already, and (manual) retries actually worked for me= , =20 > > > >=20 > > > > I'm not entirely sure what you mean by manual retries. I was trying > > > > to use socat's retry option, but that only seems to fire on > > > > ECONNREFUSED. =20 > > >=20 > > > I was watching the text execution, then as I saw the socat server > > > process getting stuck, I just re-run the client with ssh -F ... with the > > > original command, and that worked for me every time. =20 > >=20 > > Ah, yeah, I'd expect that to work. The difficulty is automatically > > detecting the failure. > >=20 > > > So, the behaviour I'm inferring is: > > >=20 > > > - server not listening, no retry option: connect() goes > > > through, but anything else will get ECONNRESET (RST from passt), plus > > > there should be EPOLLERR if socat checks that: client exits =20 > >=20 > > That doesn't seem to be the case. We *eventually* get an ECONNRESET, > > but it seems like we can send some data before it happens - sometimes > > quite a lot. I'm assuming this is because quite a lot can fit into > > socket buffers before the RST makes its way back. >=20 > Well, yes, it might simply be that socat relies on ECONNRESET from > send(), and the first send()-like call includes the full buffer. Yeah, that appears to be the case. When it did fail, it seemed to be because it was getting an ECONNRESET on one of the send()s or write()s. > Or, even if it's already checking for EPOLLERR at that point, it could > be delivered too late. >=20 > > > - server not listening, retry option: client exits without retry because > > > it doesn't get ECONNREFUSED =20 > >=20 > > Right. > >=20 > > > ...I guess a cleaner behaviour on passt's side would be to delay the > > > accept() (that's in tcp_conn_from_sock()) until we get SYN, ACK from > > > the guest. But: =20 > >=20 > > I thought about that, but I don't think its workable. For starters, I > > don't think there's a way of explicitly refusing a connection from > > userspace. Not accept()ing, then closing and re-opening the listening > > socket *might* do it. > >=20 > > But.. more fundamentally, due to the connection backlog, I think by > > the time we get the notification in userspace, the kernel has probably > > already completed the TCP handshake, so it's too late to signal a > > connection refused to the client. >=20 > Oops, right, listen() by itself already warrants a handshake, so I > guess there's no point in trying if we can't ensure a given behaviour. >=20 > In real-world applications: >=20 > - the user forwards a set of ports because they expect a server to be > reachable on those ports, fine >=20 > - or all ports are forwarded (this is the case of KubeVirt) so that the > user doesn't have to configure them. In that case, we can always have > handshake, client sending data, and receiving a RST before any of it > is acknowledged -- this is guaranteed by passt as it won't dequeue > data from buffers before the real server accepted the connection Uuuhh.. does that actually guarantee that? Does the kernel send an ack only once userspace has read the data, or just once it is queued in-kernel? > ...the only alternatives I see sound a lot like a port scan, which is > definitely not desirable. Right. > I guess we just have to... accept this behaviour. Yeah, I think so. > The important fact, I > think, is that we don't acknowledge data. >=20 > Whether closing and re-opening the listening socket causes ICMP > messages to be sent: from kernel code I'd say it's not the case, but it > might be worth an actual test, as that would be a marginal improvement. Ok. It's not really clear to me what ICMP messages would be sent to where in that scenario. --=20 David Gibson | 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 --===============1388993954496080649== Content-Type: application/pgp-signature Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="signature.asc" MIME-Version: 1.0 LS0tLS1CRUdJTiBQR1AgU0lHTkFUVVJFLS0tLS0KCmlRSXpCQUVCQ0FBZEZpRUVvVUx4V3U0L1dz MGRCK1h0Z3lwWTRnRXdZU0lGQW1Nb0Uyc0FDZ2tRZ3lwWTRnRXcKWVNKUlpBLytOUlNEdlZ1ZmRz UGNyaU1WaGVlZG1jc3hia1dPQTJCWmk5eXcyai8wbEFhQkdFZmpoM1RQaFRregoveWFic3k4R0Fq SE5JZ1J6R1paOGdCbitGemlxNkk1SnJaM1Q1N2dMbVdiOWZ1dHdTcC9UeW5FUzUxLzJhSVdkCjRj SjFicHhlRE16ZW5XK0FZTldyU0R4SWZ3cDV5NGhSTDZrbTdqRmxXeVdXSWp2R21JcVB1M3NTNmhH ckUxTHcKUTRLby9SNTJQSU9LRm5RWTQ4V0tyYW9UbVZLWC9jdWxwSDBYUGpNU3lLMkMrRUs0emR5 QjRLQk9HMENqQkVpMgo3VFBGSE9UZXJ4Qk9xUUF4Z2Q0SlNNMWZuemxFRUtsZ1pBMWtXanBNUVBI Mm12UDJTWS83OVZObkVMNnBWOVYrCldrbVRJcm5iWmRxWXV3dUdpaXZpYVd0L2lxYlJoaHlIeEZC RXVndkZ4Nm56V0lRSFhiN0NtcDhIaklJcVdhLzQKL0VHczhuK1lnVnhuQ2RMWnQrYTIyVE16MFdZ Wk56Rk9uajdleGR6cEtzR2dOaExqSkwxcVFDTTRNbW44RUNNWgozWWw5YXdRc2NTQzY3b09zWnNK UlcyQVlMaUNHWE53QnJSOGtEbm9xNVNQVXNPdE1sVG1yN3hJSHJMaTNUNER4CndKSEJnVTNuWWIy R0VkZHVkRzNvZm15YjF3YUtlaWx2ZGxZMmUxZzkvQXRPUjNiaE5hTXRHVW5xTlE2R05Vc1oKaFQx dEZBVVB0U2puTmFPLzVtTXBLWXZZRjV3U2ZqZFU5RjJ2Yk5veXVVL3dhak5MaElOR2NxL1F3djB0 dmZkRApYMk9jR2w3TjhTQzk0MHg0T2p0M1dzT2hkVGdGdmxlSGpiSnQyaFNDZjVmZWNuU1JkeFk9 Cj0wUHZyCi0tLS0tRU5EIFBHUCBTSUdOQVRVUkUtLS0tLQo= --===============1388993954496080649==--