From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202510 header.b=BP0ZhFDK; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id BEBEE5A061A for ; Wed, 22 Oct 2025 04:23:49 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202510; t=1761099825; bh=eVmMDRV1JsuPKuQZkaqrP6vVf13mJuKGvYBZBgBMhSo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=BP0ZhFDKLl1q2YztR3QNjWvPPlW1WFjuP2n3SJp4cuF5N/ofXDIXEAdvpmWpYFTFl R6BIl0IuFI3+i5IvDY1EgCGKuBfV2YkoxKXisnvftzIvQw9e92tMXS0IfTYFVBi/TL ApORQ1adGKwr//XWv3foGzZ2hv2WO3S/LMB6jgtKBwKvMts8bk2DSCvtcMWFGozMAq HT7UfpQZMmMP7Xs5pAjVsRS6lZHoMtbtG8KrB6sGvrl7lmf7Ujs/DMvYqFd2F5A2AK tYzV4Z3FHN2YOBoUtYb62zXNMdh0EL+GG8BU93c9VgKFUcI+wARoyghurzVc0UG/kA V9fqoSrW6I8ZQ== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4crtJK6VHpz4wCX; Wed, 22 Oct 2025 13:23:45 +1100 (AEDT) Date: Wed, 22 Oct 2025 13:23:40 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v3 4/4] tcp: Update data retransmission timeout Message-ID: References: <20251014073836.18150-1-yuhuang@redhat.com> <20251014073836.18150-5-yuhuang@redhat.com> <20251017202812.173e9352@elisabeth> <20251021012046.6a1aa634@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="fJdUOflcS29okPIN" Content-Disposition: inline In-Reply-To: <20251021012046.6a1aa634@elisabeth> Message-ID-Hash: O3RRYNKLQCRU2YTUZP5AYXMVRKEL2VC6 X-Message-ID-Hash: O3RRYNKLQCRU2YTUZP5AYXMVRKEL2VC6 X-MailFrom: dgibson@gandalf.ozlabs.org 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: Yumei Huang , 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: --fJdUOflcS29okPIN Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Oct 21, 2025 at 01:20:46AM +0200, Stefano Brivio wrote: > On Mon, 20 Oct 2025 18:57:45 +0800 > Yumei Huang wrote: > > On Sat, Oct 18, 2025 at 2:28=E2=80=AFAM Stefano Brivio wrote: [snip] > > > because we can surely move faster than that, but three versions in a > > > day obviously before I get any chance to have a look means a > > > substantial overhead for me, and I might miss the meaning and context= of > > > comments of other reviewers (David in this case). There are no > > > changelogs in cover letters either. > > > > > > I plan to skip to v6 but don't expect a review soon, because of that > > > overhead I just mentioned. =20 > >=20 > > Sorry for the overhead I brought. It's just so different from what we > > do with MRs or PRs(at least within our team), which we are supposed to > > update as soon as possible, so reviewers could review again at any > > time they are available. And it's always the latest code (with less > > "problematic" code) there for review, not the outdated ones. A lot of the differences from the workflow on a forge are a question of degree, rather than qualitative. If I see v4, v5 & v6 in my inbox and I haven't looked at any yet, I'll delete v4 & v5 and go straight to v6. It gets trickier when there's already a discussion thread started on one of the earlier versions (by my or someone else), or if I've already started reviewing an earlier version. The higher latencies of an email flow make those sorts of collision more likely. Although not as much as you might think, because most of the latency is not from the technology, but from when people are awake / available. > Oh, I see now. >=20 > I also have some experience with contributing via git forges, and I > think it's a serious limitation (at least on GitHub) coming from the > fact that you don't have (proper) threading. You have it on discussions > and issues/tickets but not on code reviews. >=20 > You lose one dimension of discussion there, because it becomes entirely > "linear", and while you can see differences between revisions, it's not > really practical to review or discuss them. There's also no space to > record and describe changes, if you just force push a branch. >=20 > I think code quality suffers because if the author of the change and > just one reviewer are fast enough, the point of view of everybody else > will be ignored. >=20 > Other points of view can be re-evaluated later, but in this case you'll > waste more time writing yet another revision, which might now ignore a > previous comment (that you addressed, previously) because it's not > visible anymore. I largely agree. I find there are things to like about the forge method. Tracking comments according to the source line they are relevant to can be nice in terms of automatically invalidating them once the code is updated. But it can also be a trap if the comment is a broader point / discussion that was just attached to that source line for want of a better place. I find the forges sometimes encourage review at the level of a the total diff of the MR, rather than patch-by-patch. Patch-by-patch often makes it easier to follow - or at least gives the contributor more scope to make the change clearer by the way they split it up. > * * * >=20 > Let's pick this practical example here: we were in the middle of a > discussion about whether we need to properly size a buffer to read out > sysctl values (David's idea), or if we can go for a larger buffer in any > case to keep things simpler (my proposal). >=20 > Before I had the chance to follow up with the discussion, you posted > another revision. And then another one. >=20 > On GitHub, it would be impossible for me to re-open that discussion, so > I would start a new one, and now David might miss the fact it's the > same discussion. Maybe he was right, but it doesn't matter anymore. >=20 > With email, I can do that because we have threading and persistence, but > if the outcome of the discussion now changes, you wasted time with > another revision. >=20 > Or maybe I see that you're at v7 now and I forget that that discussion > was still open, so my previous point, even if valid, is now effectively > ignored and forgotten by everybody. >=20 > The workflow you have on GitHub works well if you have one author and > one reviewer, or more reviewers who are always right and always agree > between each other, but that's a quite unrealistic expectation. >=20 > I guess it also works well if code quantity is more important than > quality, because it's merged faster that way, and because it's harder > to discuss about it (no real threading). But here we're trying to have > less code and less bugs, not more. >=20 > > I thought > > it's the same with patches in emails, that outdated versions are no > > longer useful. >=20 > They are, but they're not so practical to have a discussion about, so > not so useful as the current one, which is why discussions should have > a chance to complete. >=20 > You'll just be busy writing new revisions otherwise, instead of having > time for something else in parallel. Btw, it's perfectly ok to apply some fixes from review in your local tree for things that are simple, but wait a while before re-posting for longer discussions to complete or for additional reviewers to contribute. I do this all the time. There's not a lot of hard and fast rules here, it's a question of weighing the reasons to post early against the reasons to wait for the next spin. Taking a while to tune that is expected for any new developer. > And reviewers have other stuff to review too, so we don't really gain > time if you re-post fast. >=20 > It's different if we have a critical issue affecting many users and we > want to fix it fast for them. But usually it's a small patch/series in > that case and we don't care so much about discussing the best approach > as long as it's fixed and released quickly. >=20 > > Apparently I got it wrong. I will keep it in mind and > > not send too many versions in a short time, and add changelogs in > > cover letters when necessary. >=20 > It's not always necessary I think, and sometimes you can keep things > short if they're obvious to everybody. Right. As a reviewer, a detailed changelog is very useful. But as a developer I know that maintaining that changelog can be a lot of work. So again, it's a matter of finding a balance. > These are the biggest series > ever posted for passt, in terms of number of patches: >=20 > [PATCH v2 00/32] Use dual stack sockets to listen for inbound > TCP connections > https://archives.passt.top/passt-dev/20221117055908.2782981-1-david@gib= son.dropbear.id.au/ >=20 > [PATCH v11 00/30] Introduce discontiguous frames management > https://archives.passt.top/passt-dev/20250902075253.990038-1-lvivier@re= dhat.com/ >=20 > ...you'll see that, for some revisions, changes are very briefly > summarised. That's enough, especially if there was a single reviewer > for a given revision. >=20 > But with this series it's doable and there are a few specific changes > between each revision, so I think you should, because it helps > reviewers to understand what you're doing. >=20 > --=20 > Stefano >=20 --=20 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 --fJdUOflcS29okPIN Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmj4QCwACgkQzQJF27ox 2GcFzw/+JdBw1HKmWVSgpfUQb9ct3DbTbh5V6XtWKrYKmQxdJ/M/kz5ZXqgsSRFS yeA5q+34JtHY+C+PWZRksPDFYznifzo/8a9I6VpWbFomN4YoWBs3f9hoKIh/7Kyl cgpfkyasTHfsxGmuZQjrYZFBviG3942IvAeUI63cSo8e2485PvI32pHUPjkkV0Me jnext8GofWu8j35Wsi+rHzuqSwffFqNyYYalhBucGiC8I/W1zgp5szFXgBzKh5dm HSZO/H52qcFpHWJTa42a3y2Ugxtm9nrutw+hVtotNbls7kvk3QMKt27E90hxiT9d kQaiy0YNzds2VqKWHMuVxKEkHAcct6kgeqn8fD0VrxoETGXej8INkKd2gePsddRE Zw7ueWa81P7ANyClWdFGy673uh3R+92W6u3cWqtu3/T5X3K+jtUwBRt5pK3OuL+Y iSkvf1tuB2ZUOqACOYAozaRp11u+aPZDqPHO9i1JS9Oydivcx5wIFO7KmvRGr0V7 NSPQKzCo00MhlgE9MMnnxeGF8C0RAE55MILPR7+wgP8G0Q4pEZzibBgRKhnRgo+P HX8Vvm2PX0zDDkzgq+RpLV3C8XQ9bdnMh52PGKwJIlzjXPGYo56/DT2vg/7RiOoZ wX3A8kwOGd1JKKtTc6gr33emWr7R1tGUs0zJV29CR4EgvIEvI2Q= =BqYs -----END PGP SIGNATURE----- --fJdUOflcS29okPIN--