lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ym7us45wytkmibod5fkxoyss3nl4kxzehlchdm4pqnvvnzreey@dvuwn7olusc2>
Date: Thu, 13 Nov 2025 16:31:28 +0100
From: Stefano Garzarella <sgarzare@...hat.com>
To: Bobby Eshleman <bobbyeshleman@...il.com>
Cc: Shuah Khan <shuah@...nel.org>, "David S. Miller" <davem@...emloft.net>, 
	Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, 
	Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>, 
	Stefan Hajnoczi <stefanha@...hat.com>, "Michael S. Tsirkin" <mst@...hat.com>, 
	Jason Wang <jasowang@...hat.com>, Xuan Zhuo <xuanzhuo@...ux.alibaba.com>, 
	Eugenio Pérez <eperezma@...hat.com>, "K. Y. Srinivasan" <kys@...rosoft.com>, 
	Haiyang Zhang <haiyangz@...rosoft.com>, Wei Liu <wei.liu@...nel.org>, Dexuan Cui <decui@...rosoft.com>, 
	Bryan Tan <bryan-bt.tan@...adcom.com>, Vishnu Dasa <vishnu.dasa@...adcom.com>, 
	Broadcom internal kernel review list <bcm-kernel-feedback-list@...adcom.com>, virtualization@...ts.linux.dev, netdev@...r.kernel.org, 
	linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org, kvm@...r.kernel.org, 
	linux-hyperv@...r.kernel.org, Sargun Dhillon <sargun@...gun.me>, berrange@...hat.com, 
	Bobby Eshleman <bobbyeshleman@...a.com>
Subject: Re: [PATCH net-next v9 03/14] vsock/virtio: add netns support to
 virtio transport and virtio common

On Wed, Nov 12, 2025 at 11:32:51AM -0800, Bobby Eshleman wrote:
>On Wed, Nov 12, 2025 at 06:39:22PM +0100, Stefano Garzarella wrote:
>> On Wed, Nov 12, 2025 at 08:13:50AM -0800, Bobby Eshleman wrote:
>> > On Wed, Nov 12, 2025 at 03:18:42PM +0100, Stefano Garzarella wrote:
>> > > On Tue, Nov 11, 2025 at 10:54:45PM -0800, Bobby Eshleman wrote:
>> > > > From: Bobby Eshleman <bobbyeshleman@...a.com>
>
>[...]
>
>> > > If it simplifies, I think we can eventually merge all changes to transports
>> > > that depends on virtio_transport_common in a single commit.
>> > > IMO is better to have working commits than better split.
>> >
>> > That would be so much easier. Much of this patch is just me trying to
>> > find a way to keep total patch size reasonably small for review... if
>> > having them all in one commit is preferred then that makes life easier.
>> >
>> > The answer to all of the above is that I was just trying to make the
>> > virtio_common changes in one place, but not break bisect/build by
>> > failing to update the transport-level call sites. So the placeholder
>> > values are primarily there to compile.
>>
>> In theory, they should compile, but they should also properly behave.
>>
>> BTW I strongly believe that having separate commits is a great thing, but we
>> shouldn't take things to extremes and complicate our lives when things are
>> too closely related, as in this case.
>>
>> There is a clear dependency between these patches, so IMO, if the patch
>> doesn't become huge, it's better to have everything together. (I mean
>> between dependencies with virtio_transport_common).
>
>Sounds good, let's give the combined commit a go, I think the
>transport-specific pieces are small enough for it to not balloon?

Yeah, I think so.

>
>> What we could perhaps do is have an initial commit where you make the
>> changes, but the behavior remains unchanged (continue to use global
>> everywhere, as for virtio_transport.c in this patch), and then specific
>> commits to just enable support for local/global.
>>
>> Not sure if it's doable, but I'd like to remove the placeholders if
>> possibile. Let's discuss more about it if there are issues.
>
>Sounds good, I'll come back to this thread if the combined commit
>approach above balloons. For the combined commit, should the change log
>start at "Changes in v10" with any new changes, mention combining +
>links to the v9 patches that were combined?

Yep, that would be great. Plus exaplaining why we decided to do that (I 
mean just in the changelog).

Thanks,
Stefano


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ