From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: passt.top; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=NEHR0D5M; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTPS id CB3F15A0265 for ; Wed, 06 May 2026 10:30:17 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1778056216; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=M9uR3E9j9Hp/8ZvAmiS+WGBcCzzUbi5RXZGVLkEq5go=; b=NEHR0D5MQmd0+d7LiPChGesxfO5PWqDMkeVuDG3LvRXNzxB4QPWFUVRshK1Uyt3SOqzDSj Ob9gTwi35naO+VtWmAbSZph1dFxmnk1jlaM2DR/dOCEVOjxrf77RH3CcvIti0Y2NPEbAzf f+9rNH/BXguUVFY+N0OTsNhHpVktHhI= Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-584-Ow6CGNCsNEyIdIfxJM3VRw-1; Wed, 06 May 2026 04:30:15 -0400 X-MC-Unique: Ow6CGNCsNEyIdIfxJM3VRw-1 X-Mimecast-MFC-AGG-ID: Ow6CGNCsNEyIdIfxJM3VRw_1778056215 Received: by mail-wm1-f71.google.com with SMTP id 5b1f17b1804b1-48d104d1d8aso17213775e9.3 for ; Wed, 06 May 2026 01:30:15 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1778056214; x=1778661014; h=date:content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=M9uR3E9j9Hp/8ZvAmiS+WGBcCzzUbi5RXZGVLkEq5go=; b=FJIX3TSNY59SEYRpwyBBze3E2n+PMOiPf+8+otZS5Fy/oRytBJkVSlRiFPBmTTjDuu DD0z9BJmba00+64FU9khi3kvTuMVWT1BSNDUn0WrUiXCj/SNxmFqFQ78o9rtluekyJhY JIUl3mgTldrP+hufDqv3ZkENf5/BqrCygv0Ynp0EIpf0Qc9CSXQl14rbfX6po6DcpiF8 8Z6eIRGmC8C+0/I4rGw0OTPn9mFR/gu6l3yGVB2X9A8+CI8YNHW5iOAayRqRNjOWtRh1 lv/nR6yiCdvLFv/zt4L0JGXgV+3Vvo8kwXItykLw1nKiPNhFfa7s5NfOmxf6eg6Ki8q6 rzNg== X-Gm-Message-State: AOJu0YyypKrB3OgkI/+Ni6qzYPwPZjMO2hRsGUt7hE0wBDgEyz3dXaUC TipPzfzI2mSsMTBMu4S2Lp+ymABinVJawW8no02y9NsifmdeLqczbw02NBA7b79ElMwDvEV8xqU WcsyDUWbiPzKbShwMfvp94uZZW9l26ryOPu/zWpSty7JpTokF3cIUyQ== X-Gm-Gg: AeBDiesbe4nXLuUzXXkDhO/1S8URlakR8ZB0/jfs2/285yqsyZXe5kK09mmKgg2nU9k /C1ubyWplCvbvUsDfNIZ1kMlkBf9uJoqDw4GbUwmhKBeHJr9QHshoXLAwHFTjAjHKR06TePPC7J ax2yPEiDFZf855BfQ9wj4brTgkDKTx+2awtgjImKaFMMxjqJW965Y+EICmferxktcmhDLeM7Xw6 7t6ONRasTeWwLTXIP0s1b5PMSFsJWP2SUm2PmoSo1hz2IxdvM0+J5+YjdqGyCpV5LQcCqag46Td q8hRaSotuNaQK85XlFXFTrEov/Wy0OsOZnlYmb6u8RrTihC5+eLy+xGyIJPM2zI9Jt6T0xStcyu sw6EoozusFZ4c51n+ZdGwlEFtHDvduDzsDUrak59qs+SUTb6M7fbpOeHKGgrV X-Received: by 2002:a05:600c:1d11:b0:48a:7f90:2231 with SMTP id 5b1f17b1804b1-48e51f364f3mr45002645e9.19.1778056214376; Wed, 06 May 2026 01:30:14 -0700 (PDT) X-Received: by 2002:a05:600c:1d11:b0:48a:7f90:2231 with SMTP id 5b1f17b1804b1-48e51f364f3mr45001825e9.19.1778056213657; Wed, 06 May 2026 01:30:13 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [176.103.220.4]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-48e538a5486sm32576285e9.6.2026.05.06.01.30.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 06 May 2026 01:30:13 -0700 (PDT) From: Stefano Brivio To: David Gibson Subject: Re: [PATCH v8 10/19] pesto, conf: Have pesto connect to passt and check versions Message-ID: <20260506103007.67eb4d11@elisabeth> In-Reply-To: References: <20260505234719.1437340-1-sbrivio@redhat.com> <20260505234719.1437340-11-sbrivio@redhat.com> <20260506095531.0b1387c5@elisabeth> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 Date: Wed, 06 May 2026 10:30:12 +0200 (CEST) X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: m7vxO0O0zUU7R4xhALLiLqlzr5zbGm_DCKY5XFlixA0_1778056215 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: 7SSY6DTWMZVHUOWJBR54USWRLRU2EIJQ X-Message-ID-Hash: 7SSY6DTWMZVHUOWJBR54USWRLRU2EIJQ X-MailFrom: sbrivio@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top, Jon Maloy , Laurent Vivier X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Wed, 6 May 2026 18:21:32 +1000 David Gibson wrote: > On Wed, May 06, 2026 at 09:55:32AM +0200, Stefano Brivio wrote: > > On Wed, 6 May 2026 15:38:30 +1000 > > David Gibson wrote: > > > > > On Wed, May 06, 2026 at 01:47:10AM +0200, Stefano Brivio wrote: > > > > From: David Gibson > > > > > > > > Start implementing pesto in earnest. Create a control/configuration > > > > socket in passt. Have pesto connect to it and retrieve a server greeting > > > > Perform some basic version checking. > > > > > > > > Signed-off-by: David Gibson > > > > [sbrivio: Avoid potential recursive calling between conf_accept() and > > > > conf_close(), reported by clang-tidy] > > > > > > Huh. For some reason that warning didn't trip for me. Although it's > > > technically true they can mutually recurse, I believe they're both > > > tail calls, so it shouldn't eat the stack. > > > > > > > [sbrivio: In conf(), check we're not exceeding sizeof(c->control_path) > > > > instead of sizeof(c->socket_path), and, in pesto's main(), print > > > > argv[optind] instead of argv[1] to indicate an invalid socket path, > > > > both reported by Jon Maloy] > > > > [sbrivio: In pesto's main(), drop unnecessary newline from error > > > > message, reported by Laurent] > > > > [sbrivio: Don't use SOCK_NONBLOCK on accept4(), as that only applies > > > > to the *new* file descriptor, which we don't want -- set O_NONBLOCK > > > > on the listening file descriptor using fcntl()] > > > > > > Making the new (accepted) socket non-blocking was the intended > > > behaviour here. We also want non-blocking for the listening socket, > > > but that was already done in feab892c7 ("tap, repair: Use > > > SOCK_NONBLOCK and SOCK_CLOEXEC on Unix sockets"). > > > > Oops, now that Laurent mentioned it, I realised I dropped it > > accidentally while / after debugging things on v6, and: > > Ah, right. > > > > WIth the current design, I guess we don't want non-blocking on the > > > accepted socket, although I don't think it actually matters very much. > > > > ...this is the issue I was trying to fix: if the accepted socket is > > non-blocking, messages are cut short sometimes, and in general things > > don't work. > > Hrm. I was pretty sure setting it blocking just meant you'd always > get *some* data instead of EAGAIN. I don't believe it prevents either > short reads or short writes. Maybe, but something caused actual problems for me, otherwise I wouldn't have played with it at all. The current behaviour with this patch is something I tested quite heavily by now. > Both sides should already be using {read,write}_all_buf() to handle > short read/writes, so I'm really not sure where it's going wrong if > the accepted socket is non-blocking (other than maybe spinning on > EAGAIN more than we'd like). > > > I don't remember if this was while testing things on Fedora or Debian, > > it only happened in one of the two environments. > > > > So, while it was accidental (I really didn't leave any note for a cover > > letter, so I'm almost certain there was no other reason for me to drop > > it), I think it's actually a good idea to drop it for the following > > reasons: > > > > - O_NONBLOCK on the accepted socket breaks things > > The earlier patch doesn't affect O_NONBLOCK on the accepted socket, > only on the listening socket (it's not inherited). > > > - the rest looks correct to me but fairly out of scope, and I have very > > limited time for testing things in detail right now, so I'd rather > > keep that patch for later > > It's in scope because O_NONBLOCK on the listening socket is essential This can be implemented in two lines like the current version does. The rest is out of scope. > to implementing the "concurrent client blocks" instead of "concurrent > client is rejected" behaviour. Without O_NONBLOCK on the listening > socket, we can't safely call conf_accept() anywhere other than in > response to the epoll event - because looking for additional > connections after we close one could block. > > > Without that patch, and my follow-up change to this patch, we're just > > adding two lines for this specific behaviour, instead of 18. > > > > > We will want non-blocking it when we change this to read out the > > > updated rules incrementally, rather than all at once. > > > > Right, so maybe we can keep that patch for that moment. Or even > > before, > > "that patch" meaning the sock_unix() one? Again, that affects the > listening socket behaviour. What we need for incrementally reading > the rules is about the accepted socket behaviour. Yes. That doesn't just affect the listening socket, it affects a bunch of things, and they can all be checked and revisited, more conveniently, together, at a later point. -- Stefano