From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=none 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=AbAIGTKR; 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 3E81D5A0274 for ; Wed, 12 Feb 2025 02:20:15 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1739323214; 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=BsEnVBpQgwufV+dzWSKvUxGVXHoM2Hkvc0huLU47KM0=; b=AbAIGTKRS/zt4yXPvP6wIXtINu2dObQdUoZGWnDnU6sjHGGH8mkWOLcg+YiiRx4GK4k0x+ X+lHLKVFrFZwytPDmFheRegseEEUIw5tCq2GdB4cod5EkY5C/uxYokN+hO/EAMThmqmpcu kjMU3B+qzhxdM6T3xLpLxe3GRAdOLjs= 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-150-SX3A-Ue-Pdqk-KLpVnaUkQ-1; Tue, 11 Feb 2025 20:20:12 -0500 X-MC-Unique: SX3A-Ue-Pdqk-KLpVnaUkQ-1 X-Mimecast-MFC-AGG-ID: SX3A-Ue-Pdqk-KLpVnaUkQ_1739323211 Received: by mail-wm1-f69.google.com with SMTP id 5b1f17b1804b1-4393b6763a3so17139925e9.2 for ; Tue, 11 Feb 2025 17:20:12 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1739323211; x=1739928011; 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=BsEnVBpQgwufV+dzWSKvUxGVXHoM2Hkvc0huLU47KM0=; b=bQJvG6p8FyP2+zO1nzXFavKUXS4TEevphE0KRPQ2d7t+T8Ek9zoxqA9VO/rJ4A3f0Z 54WsWXKzD4p4gPELdUSBqD/XrEfw/imuZgs2AEc1Wi2H4oQuEncPKaEaJXPKJr4Qu4L8 MOw7CiO+aN2fm3OxhLRSvr5Dm4Py6yKcup++Wx0+OVpjLAVDZWiL2MJoasgsBZ/Jm4Aq uX/cNup6j5VO3/SgHTbV/Ovu4IThPUdAQBzX8iJdqfQ4VRiK9APvguCuWIY+o5Ss9lzq dRPIgk8q7MxKHEwGrWED9VLBrjDvu6rwM9MKdpg7OLQHhOBYu8uElOrYVAzyW3E73b1m M4yQ== X-Gm-Message-State: AOJu0YxhTMH/Vu9PzLhN21A9rnQIsz9on3W9b3VaWQITO2D60Z8cDRCx XxdgOANGU5YDCO1aiv+DWnmLI3ywo8DHht3nz3LYd0LpewpcuUJ8ax+/ctcSRJUBzMoaQHxte3Z bkeEDOMHsMJOFNX6MzA9T4nhfaCgPVEUahpOO2ORwAysiUwynbA== X-Gm-Gg: ASbGncuV7THbFlpn9XHuLUhStruak8ru8AefjDuNDZ3SqQltc44upv811asnOwGBxW2 XFX8l9gup9M/pN0QBPbd/xfu8G3RmOYDaWji6VwkLyqnULOZrOxD68+8/h9jjaT2AM33HohYawp ruOwZzwEK7DBQSoIXlhz+gK0bi4lDnAtzoy9dq8in9C/q++TJtBTWFIK0yo/z7P9PArVcizfNc0 bTwktwyOyro+p2kUZv5dY1GrS8cwo/PobABNR+FaDuTYhT3dvfTh9IDFG28+CU8l2KGlCWyIHAu HfZU6mKhphBkwNV5 X-Received: by 2002:a05:600c:a55:b0:434:f0df:9f6 with SMTP id 5b1f17b1804b1-439599258bcmr4067365e9.3.1739323211362; Tue, 11 Feb 2025 17:20:11 -0800 (PST) X-Google-Smtp-Source: AGHT+IFCxjOmyr/zn3jYtPA9EP1Nfd7bOr9izOn/JW8+2FmWgwwYXe34vO2nwfuEXpWm28LpaA0BnA== X-Received: by 2002:a05:600c:a55:b0:434:f0df:9f6 with SMTP id 5b1f17b1804b1-439599258bcmr4067295e9.3.1739323211033; Tue, 11 Feb 2025 17:20:11 -0800 (PST) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-38dc09fc2d9sm15204851f8f.6.2025.02.11.17.20.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 11 Feb 2025 17:20:10 -0800 (PST) Date: Wed, 12 Feb 2025 02:20:09 +0100 From: Stefano Brivio To: David Gibson Subject: Re: [PATCH] tcp: Don't discard window information on keep-alive segments Message-ID: <20250212022009.3734cf73@elisabeth> In-Reply-To: References: <20250211195051.197798-1-sbrivio@redhat.com> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.41; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: 1AqUokxYGHnMzmRAuvwwywNlExEntIdnQvxoEAClb7A_1739323211 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: JGHEAEO5FUBHVBCE6HFQKQCL2DOKSIPU X-Message-ID-Hash: JGHEAEO5FUBHVBCE6HFQKQCL2DOKSIPU 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 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, 12 Feb 2025 11:42:54 +1100 David Gibson wrote: > On Tue, Feb 11, 2025 at 08:50:51PM +0100, Stefano Brivio wrote: > > It looks like a detail, but it's critical if we're dealing with > > somebody, such as near-future self, using TCP_REPAIR to migrate TCP > > connections in the guest or container. > > > > The last packet sent from the 'source' process/guest/container > > typically reports a small window, or zero, because the guest/container > > hadn't been draining it for a while. > > > > The next packet, appearing as the target sets TCP_REPAIR_OFF on the > > migrated socket, is a keep-alive (also called "window probe" in CRIU > > or TCP_REPAIR-related code), and it comes with an updated window > > value, reflecting the pre-migration "regular" value. > > > > If we ignore it, it might take a while/forever before we realise we > > can actually restart sending. > > > > Fixes: 238c69f9af45 ("tcp: Acknowledge keep-alive segments, ignore them for the rest") > > Signed-off-by: Stefano Brivio > > Reviewed-by: David Gibson > > Although... > > > --- > > tcp.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/tcp.c b/tcp.c > > index af6bd95..2addf4a 100644 > > --- a/tcp.c > > +++ b/tcp.c > > @@ -1664,8 +1664,10 @@ static int tcp_data_from_tap(const struct ctx *c, struct tcp_tap_conn *conn, > > tcp_send_flag(c, conn, ACK); > > tcp_timer_ctl(c, conn); > > > > - if (p->count == 1) > > + if (p->count == 1) { > > ... not really this patch, but this condition seems wrong to me. IIUC > it's attempting to detect the last packet in the batch, which isn't > necessarily the same thing as the _only_ packet in the batch. No, not really, I just want to select one-packet batches on purpose. If a keep-alive is part of a batch 1. it's not a keep-alive and 2. it would probably need a more complicated handling which I hadn't really time to think about. See previous discussion on this: https://archives.passt.top/passt-dev/Zz01CDMNyFN-Ze68@zatzit > Admittedly, it probably will be for a keep-alive, but I'm having a > hard time convincing myself it absolutely has to be. It is, because it makes no sense to batch keep-alives... > Should this maybe be (i + 1 == p->count) instead? -- Stefano