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=IxKhxw/a; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by passt.top (Postfix) with ESMTPS id 5D60C5A026F for ; Mon, 27 Oct 2025 07:49:27 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1761547766; 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=swHFPWRDrTzhxFHfBGVT/+WF8Kuu8bTCqZrBM9eV01Q=; b=IxKhxw/a/3pyi9RMnN0jpNcC/Y71MRmuM0a1cTHP/YshZcMmC7okR9Q80OIsHRKaPd+HKH b2GZ5hLWTtnL0d89fZ/PoRe6hifSNP+qOpzm9QL/0NBTKgENXE1G9tQSeaDhNQaLVN2ZYu BVVqdduuen27QnJH0RWiEtWOTKCs9I0= Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-115-lcxfttO-NvS01oZxf-HE1w-1; Mon, 27 Oct 2025 02:49:24 -0400 X-MC-Unique: lcxfttO-NvS01oZxf-HE1w-1 X-Mimecast-MFC-AGG-ID: lcxfttO-NvS01oZxf-HE1w_1761547763 Received: by mail-wm1-f69.google.com with SMTP id 5b1f17b1804b1-46e47d14dceso21104505e9.2 for ; Sun, 26 Oct 2025 23:49:24 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1761547763; x=1762152563; h=content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=iy/mf7LR56Xg4Y/1kpbw9FZeAL0cp90isjKGYhnZWVU=; b=dW3XIc79+GJDY63KSq049zaWjkfJ0utZyRSll2oT3fFqZTolUxRacNm44M6ncZ9B/V zymLvSHWaDz7f/Z+KkC4lAmL5L17EHZJVoOknSC2LF7ODtQAxToMIqqW22Anwu4oO2pv uIr8SHrQwKfpN9l3sIrXEAzj+v986DzfTN4Sxxgc1zJE/juBN7N8er3XrqV4Y489Szoc piXH79an0k2eDRiMwhs9V3qOVsLVN9ehp/iTcKAY4IgYsH7dnCaG8GtY+hIKOx7fW5S6 c0bBx3+gFFuA86bWCCWz2bUAQNA+eUW+fVkekgCMsak9oh5DVgbYa7cnAtg0K08aZbL/ nBLA== X-Forwarded-Encrypted: i=1; AJvYcCUcIyTYuhCeOl5rqWD6qtOMe2jg2Wbda630uLuKZXkV2ZD0AgnxjAvxY5sjMfbjlhFjn+RB8iIzHF0=@passt.top X-Gm-Message-State: AOJu0Yx3903OPCHIAcJq5k3L2aK97MEgljzJrdclYv3M6/9pcWNdF3Po GxYzgptbcKL0juYrc4yOwbQ6fB1+/uIIVkGeY2/U/v30KMuPOp2bc+MWyorTujF0f44z/1onX5f JtC2QBNqk3rN5Da2kx1S3HKKUNKG6b8IIAS0hRsDW4lz4zTUAKuzDLA== X-Gm-Gg: ASbGncvbsBX2hzNtaj/ZJViqggEQHL6by5LAyrYLK/lIFEFhrKPKamXv9aiWpoZ1f7X htxNfvstVVahIPPk6sUgECJkx2ZvcfcI6I+CaeIODo+9oPtPQtiQD+ty9Dj4hGj0zHDyYMq8x3E i7I9qnhBDRrVlYRX9195tSguRW1nH5ZZrZkzRTKW7BHnZFSWWG52pS6+iprILwO7cpktBbS+49n jEuZxp9wWnR8A1GnEMfjd9L/GpzlhiL0zpPatXCHPAajtBtSNyO73HsPGWtL/Tol62t0j5utvvb EI7/oMD+7d0hdkdN2tcSw5bw6G81J/k6oabEDDGu5D28o7/ckArD92JEN3rkEXi0TbNzPHQ4+s0 ZZlmomW9TMw== X-Received: by 2002:a05:6000:3101:b0:3ec:db87:e5f4 with SMTP id ffacd0b85a97d-4298a0406efmr10052949f8f.7.1761547763261; Sun, 26 Oct 2025 23:49:23 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGqZQFSCSZRCEvkcqQcMbTsqsyzEJP+1xKyMWrBco2XTIiqqNVGBmBinLa146Y7JWTnwRUcQA== X-Received: by 2002:a05:6000:3101:b0:3ec:db87:e5f4 with SMTP id ffacd0b85a97d-4298a0406efmr10052928f8f.7.1761547762727; Sun, 26 Oct 2025 23:49:22 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-429952e2e06sm12402888f8f.46.2025.10.26.23.49.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 26 Oct 2025 23:49:22 -0700 (PDT) Date: Mon, 27 Oct 2025 07:49:20 +0100 From: Stefano Brivio To: Yumei Huang Subject: Re: [PATCH v6 3/4] tcp: Resend SYN for inbound connections Message-ID: <20251027074920.7ca3948a@elisabeth> In-Reply-To: References: <20251017062838.21041-1-yuhuang@redhat.com> <20251017062838.21041-4-yuhuang@redhat.com> <20251024010431.4329a843@elisabeth> <20251024103717.715fe49e@elisabeth> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: iUwkZg8VsTJF7upXAcnP5Njhv-ECfrwDWpnILqDXMuM_1761547763 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Message-ID-Hash: UBEGFVJDRPOXP3YJKRJIRLFFKBMVE4CO X-Message-ID-Hash: UBEGFVJDRPOXP3YJKRJIRLFFKBMVE4CO 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: David Gibson , passt-dev@passt.top 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 Mon, 27 Oct 2025 11:37:20 +0800 Yumei Huang wrote: > On Fri, Oct 24, 2025 at 6:56=E2=80=AFPM David Gibson > wrote: > > > > On Fri, Oct 24, 2025 at 10:37:17AM +0200, Stefano Brivio wrote: =20 > > > On Fri, 24 Oct 2025 14:30:09 +1100 > > > David Gibson wrote: =20 > > > > On Fri, Oct 24, 2025 at 01:04:31AM +0200, Stefano Brivio wrote: =20 > > > > > On Fri, 17 Oct 2025 14:28:37 +0800 > > > > > Yumei Huang wrote: =20 > > [snip] =20 > > > > > > @@ -2409,8 +2419,17 @@ void tcp_timer_handler(const struct ctx = *c, union epoll_ref ref) > > > > > > tcp_timer_ctl(c, conn); > > > > > > } else if (conn->flags & ACK_FROM_TAP_DUE) { > > > > > > if (!(conn->events & ESTABLISHED)) { > > > > > > - flow_dbg(conn, "handshake timeout"); > > > > > > - tcp_rst(c, conn); > > > > > > + if (conn->retries >=3D TCP_MAX_RETRIES = || > > > > > > + conn->retries >=3D (c->tcp.tcp_syn_= retries + > > > > > > + c->tcp.syn_linear= _timeouts)) { > > > > > > + flow_dbg(conn, "handshake timeo= ut"); > > > > > > + tcp_rst(c, conn); > > > > > > + } else { > > > > > > + flow_trace(conn, "SYN timeout, = retry"); > > > > > > + tcp_send_flag(c, conn, SYN); > > > > > > + conn->retries++; =20 > > > > > > > > > > I think I already raised this point on a previous revision: this = needs > > > > > to be zeroed as the connection is established, but I don't see th= at in > > > > > the current version. =20 > > > > > > > > Yes, you raised that, but then I realised it's already handled. I > > > > think I put that in the thread, not just direct to Yumei, but maybe > > > > not? Or it just got lost in the minutiae. =20 > > > > > > Yes, here: > > > > > > https://archives.passt.top/passt-dev/aOxFRfJjPWy0ZW0M@zatzit > > > > > > this is another example of what I meant about (potential) advantages = of > > > a fully threaded (email) workflow. > > > > > > In this case, I didn't review v2, which came before you could post th= is > > > to my comment on v1, but in a normal case, we could have settled this > > > earlier, once for all. =20 > > > > Ah, right, that'd do it. > > =20 > > > > When we receive a SYN-ACK, it will have th->ack_seq advanced a byte > > > > acknowledging the SYN. tcp_tap_handler() calls > > > > tcp_update_seqack_from_tap() in the !ESTABLISHED case which will se= e > > > > the new ack_seq and clear retries (retrans before this series). =20 > > > > > > It doesn't look obvious at all to me. =20 > > > > Oh, it's definitely not obvious, but I'm pretty confident it's > > correct. Fwiw, I spotted this because I thought the explicit handling > > in v2 wasn't at quite the right point logically (though close enough > > to be fine in practice). I went looking for the precise right point - > > when we receive the SYN-ACK - and there it was, already handled. > > > > It does make a kind of logical sense. The RFCs don't generally treat > > SYN (or SYN-ACK, or FIN) retransmits any differently from data > > retransmits. We do treat them differently, but less so after this > > series, which is a good thing, I think. > > =20 > > > We're unlikely to break it in the future, so I don't think it's fragi= le > > > in the long term, but... can one of you double check that it's actual= ly > > > the case with a manual one-off test? =20 > > > > Yeah, I guess that's wise. Easiest way is probably to add a temporary > > debug message here, and try it against a qemu guest that's temporarily > > suspended. Yumei, I can walk you through this, too. =20 >=20 > Thank you, I've verified this by adding debug messages in the if block: >=20 > if (th->ack && !(conn->events & ESTABLISHED)) > tcp_update_seqack_from_tap(c, conn, ntohl(th->ack_seq)); >=20 > conn->retries gets reset in tcp_update_seqack_from_tap(). Great, thanks for checking. Maybe we could mention this in a comment, but I'm not exactly sure where we would add this comment, so unless somebody has a good idea we can also skip that. I don't have further comments on v6 by the way, the rest of the series looks good to me. --=20 Stefano