Hello,
DESCRIPTION
This patchset fixes old problem with hungup of both rx/tx sides and adds
test for it. This happens due to non-default SO_RCVLOWAT value and
deferred credit update in virtio/vsock. Link to previous old patchset:
https://lore.kernel.org/netdev/[email protected]/
Here is what happens step by step:
TEST
INITIAL CONDITIONS
1) Vsock buffer size is 128KB.
2) Maximum packet size is also 64KB as defined in header (yes it is
hardcoded, just to remind about that value).
3) SO_RCVLOWAT is default, e.g. 1 byte.
STEPS
SENDER RECEIVER
1) sends 128KB + 1 byte in a
single buffer. 128KB will
be sent, but for 1 byte
sender will wait for free
space at peer. Sender goes
to sleep.
2) reads 64KB, credit update not sent
3) sets SO_RCVLOWAT to 64KB + 1
4) poll() -> wait forever, there is
only 64KB available to read.
So in step 4) receiver also goes to sleep, waiting for enough data or
connection shutdown message from the sender. Idea to fix it is that rx
kicks tx side to continue transmission (and may be close connection)
when rx changes number of bytes to be woken up (e.g. SO_RCVLOWAT) and
this value is bigger than number of available bytes to read.
I've added small test for this, but not sure as it uses hardcoded value
for maximum packet length, this value is defined in kernel header and
used to control deferred credit update. And as this is not available to
userspace, I can't control test parameters correctly (if one day this
define will be changed - test may become useless).
Head for this patchset is:
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=021b0c952f226236f2edf89c737efb9a28d1422d
Link to v1:
https://lore.kernel.org/netdev/[email protected]/
Link to v2:
https://lore.kernel.org/netdev/[email protected]/
Link to v3:
https://lore.kernel.org/netdev/[email protected]/
Link to v4:
https://lore.kernel.org/netdev/[email protected]/
Link to v5:
https://lore.kernel.org/netdev/[email protected]/
Link to v6:
https://lore.kernel.org/netdev/[email protected]/
Link to v7:
https://lore.kernel.org/netdev/[email protected]/
Changelog:
v1 -> v2:
* Patchset rebased and tested on new HEAD of net-next (see hash above).
* New patch is added as 0001 - it removes return from SO_RCVLOWAT set
callback in 'af_vsock.c' when transport callback is set - with that
we can set 'sk_rcvlowat' only once in 'af_vsock.c' and in future do
not copy-paste it to every transport. It was discussed in v1.
* See per-patch changelog after ---.
v2 -> v3:
* See changelog after --- in 0003 only (0001 and 0002 still same).
v3 -> v4:
* Patchset rebased and tested on new HEAD of net-next (see hash above).
* See per-patch changelog after ---.
v4 -> v5:
* Change patchset tag 'RFC' -> 'net-next'.
* See per-patch changelog after ---.
v5 -> v6:
* New patch 0003 which sends credit update during reading bytes from
socket.
* See per-patch changelog after ---.
v6 -> v7:
* Patchset rebased and tested on new HEAD of net-next (see hash above).
* See per-patch changelog after ---.
v7 -> v8:
* See per-patch changelog after ---.
Arseniy Krasnov (4):
vsock: update SO_RCVLOWAT setting callback
virtio/vsock: send credit update during setting SO_RCVLOWAT
virtio/vsock: fix logic which reduces credit update messages
vsock/test: two tests to check credit update logic
drivers/vhost/vsock.c | 1 +
include/linux/virtio_vsock.h | 1 +
include/net/af_vsock.h | 2 +-
net/vmw_vsock/af_vsock.c | 9 +-
net/vmw_vsock/hyperv_transport.c | 4 +-
net/vmw_vsock/virtio_transport.c | 1 +
net/vmw_vsock/virtio_transport_common.c | 43 +++++-
net/vmw_vsock/vsock_loopback.c | 1 +
tools/testing/vsock/vsock_test.c | 175 ++++++++++++++++++++++++
9 files changed, 229 insertions(+), 8 deletions(-)
--
2.25.1
On Tue, Dec 12, 2023 at 12:16:54AM +0300, Arseniy Krasnov wrote:
> Hello,
>
> DESCRIPTION
>
> This patchset fixes old problem with hungup of both rx/tx sides and adds
> test for it. This happens due to non-default SO_RCVLOWAT value and
> deferred credit update in virtio/vsock. Link to previous old patchset:
> https://lore.kernel.org/netdev/[email protected]/
Patchset:
Acked-by: Michael S. Tsirkin <[email protected]>
But I worry whether we actually need 3/8 in net not in net-next.
Thanks!
> Here is what happens step by step:
>
> TEST
>
> INITIAL CONDITIONS
>
> 1) Vsock buffer size is 128KB.
> 2) Maximum packet size is also 64KB as defined in header (yes it is
> hardcoded, just to remind about that value).
> 3) SO_RCVLOWAT is default, e.g. 1 byte.
>
>
> STEPS
>
> SENDER RECEIVER
> 1) sends 128KB + 1 byte in a
> single buffer. 128KB will
> be sent, but for 1 byte
> sender will wait for free
> space at peer. Sender goes
> to sleep.
>
>
> 2) reads 64KB, credit update not sent
> 3) sets SO_RCVLOWAT to 64KB + 1
> 4) poll() -> wait forever, there is
> only 64KB available to read.
>
> So in step 4) receiver also goes to sleep, waiting for enough data or
> connection shutdown message from the sender. Idea to fix it is that rx
> kicks tx side to continue transmission (and may be close connection)
> when rx changes number of bytes to be woken up (e.g. SO_RCVLOWAT) and
> this value is bigger than number of available bytes to read.
>
> I've added small test for this, but not sure as it uses hardcoded value
> for maximum packet length, this value is defined in kernel header and
> used to control deferred credit update. And as this is not available to
> userspace, I can't control test parameters correctly (if one day this
> define will be changed - test may become useless).
>
> Head for this patchset is:
> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=021b0c952f226236f2edf89c737efb9a28d1422d
>
> Link to v1:
> https://lore.kernel.org/netdev/[email protected]/
> Link to v2:
> https://lore.kernel.org/netdev/[email protected]/
> Link to v3:
> https://lore.kernel.org/netdev/[email protected]/
> Link to v4:
> https://lore.kernel.org/netdev/[email protected]/
> Link to v5:
> https://lore.kernel.org/netdev/[email protected]/
> Link to v6:
> https://lore.kernel.org/netdev/[email protected]/
> Link to v7:
> https://lore.kernel.org/netdev/[email protected]/
>
> Changelog:
> v1 -> v2:
> * Patchset rebased and tested on new HEAD of net-next (see hash above).
> * New patch is added as 0001 - it removes return from SO_RCVLOWAT set
> callback in 'af_vsock.c' when transport callback is set - with that
> we can set 'sk_rcvlowat' only once in 'af_vsock.c' and in future do
> not copy-paste it to every transport. It was discussed in v1.
> * See per-patch changelog after ---.
> v2 -> v3:
> * See changelog after --- in 0003 only (0001 and 0002 still same).
> v3 -> v4:
> * Patchset rebased and tested on new HEAD of net-next (see hash above).
> * See per-patch changelog after ---.
> v4 -> v5:
> * Change patchset tag 'RFC' -> 'net-next'.
> * See per-patch changelog after ---.
> v5 -> v6:
> * New patch 0003 which sends credit update during reading bytes from
> socket.
> * See per-patch changelog after ---.
> v6 -> v7:
> * Patchset rebased and tested on new HEAD of net-next (see hash above).
> * See per-patch changelog after ---.
> v7 -> v8:
> * See per-patch changelog after ---.
>
> Arseniy Krasnov (4):
> vsock: update SO_RCVLOWAT setting callback
> virtio/vsock: send credit update during setting SO_RCVLOWAT
> virtio/vsock: fix logic which reduces credit update messages
> vsock/test: two tests to check credit update logic
>
> drivers/vhost/vsock.c | 1 +
> include/linux/virtio_vsock.h | 1 +
> include/net/af_vsock.h | 2 +-
> net/vmw_vsock/af_vsock.c | 9 +-
> net/vmw_vsock/hyperv_transport.c | 4 +-
> net/vmw_vsock/virtio_transport.c | 1 +
> net/vmw_vsock/virtio_transport_common.c | 43 +++++-
> net/vmw_vsock/vsock_loopback.c | 1 +
> tools/testing/vsock/vsock_test.c | 175 ++++++++++++++++++++++++
> 9 files changed, 229 insertions(+), 8 deletions(-)
>
> --
> 2.25.1
On 12.12.2023 18:54, Michael S. Tsirkin wrote:
> On Tue, Dec 12, 2023 at 12:16:54AM +0300, Arseniy Krasnov wrote:
>> Hello,
>>
>> DESCRIPTION
>>
>> This patchset fixes old problem with hungup of both rx/tx sides and adds
>> test for it. This happens due to non-default SO_RCVLOWAT value and
>> deferred credit update in virtio/vsock. Link to previous old patchset:
>> https://lore.kernel.org/netdev/[email protected]/
>
>
> Patchset:
>
> Acked-by: Michael S. Tsirkin <[email protected]>
Thanks!
>
>
> But I worry whether we actually need 3/8 in net not in net-next.
Because of "Fixes" tag ? I think this problem is not critical and reproducible
only in special cases, but i'm not familiar with netdev process so good, so I don't
have strong opinion. I guess @Stefano knows better.
Thanks, Arseniy
>
> Thanks!
>
>> Here is what happens step by step:
>>
>> TEST
>>
>> INITIAL CONDITIONS
>>
>> 1) Vsock buffer size is 128KB.
>> 2) Maximum packet size is also 64KB as defined in header (yes it is
>> hardcoded, just to remind about that value).
>> 3) SO_RCVLOWAT is default, e.g. 1 byte.
>>
>>
>> STEPS
>>
>> SENDER RECEIVER
>> 1) sends 128KB + 1 byte in a
>> single buffer. 128KB will
>> be sent, but for 1 byte
>> sender will wait for free
>> space at peer. Sender goes
>> to sleep.
>>
>>
>> 2) reads 64KB, credit update not sent
>> 3) sets SO_RCVLOWAT to 64KB + 1
>> 4) poll() -> wait forever, there is
>> only 64KB available to read.
>>
>> So in step 4) receiver also goes to sleep, waiting for enough data or
>> connection shutdown message from the sender. Idea to fix it is that rx
>> kicks tx side to continue transmission (and may be close connection)
>> when rx changes number of bytes to be woken up (e.g. SO_RCVLOWAT) and
>> this value is bigger than number of available bytes to read.
>>
>> I've added small test for this, but not sure as it uses hardcoded value
>> for maximum packet length, this value is defined in kernel header and
>> used to control deferred credit update. And as this is not available to
>> userspace, I can't control test parameters correctly (if one day this
>> define will be changed - test may become useless).
>>
>> Head for this patchset is:
>> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=021b0c952f226236f2edf89c737efb9a28d1422d
>>
>> Link to v1:
>> https://lore.kernel.org/netdev/[email protected]/
>> Link to v2:
>> https://lore.kernel.org/netdev/[email protected]/
>> Link to v3:
>> https://lore.kernel.org/netdev/[email protected]/
>> Link to v4:
>> https://lore.kernel.org/netdev/[email protected]/
>> Link to v5:
>> https://lore.kernel.org/netdev/[email protected]/
>> Link to v6:
>> https://lore.kernel.org/netdev/[email protected]/
>> Link to v7:
>> https://lore.kernel.org/netdev/[email protected]/
>>
>> Changelog:
>> v1 -> v2:
>> * Patchset rebased and tested on new HEAD of net-next (see hash above).
>> * New patch is added as 0001 - it removes return from SO_RCVLOWAT set
>> callback in 'af_vsock.c' when transport callback is set - with that
>> we can set 'sk_rcvlowat' only once in 'af_vsock.c' and in future do
>> not copy-paste it to every transport. It was discussed in v1.
>> * See per-patch changelog after ---.
>> v2 -> v3:
>> * See changelog after --- in 0003 only (0001 and 0002 still same).
>> v3 -> v4:
>> * Patchset rebased and tested on new HEAD of net-next (see hash above).
>> * See per-patch changelog after ---.
>> v4 -> v5:
>> * Change patchset tag 'RFC' -> 'net-next'.
>> * See per-patch changelog after ---.
>> v5 -> v6:
>> * New patch 0003 which sends credit update during reading bytes from
>> socket.
>> * See per-patch changelog after ---.
>> v6 -> v7:
>> * Patchset rebased and tested on new HEAD of net-next (see hash above).
>> * See per-patch changelog after ---.
>> v7 -> v8:
>> * See per-patch changelog after ---.
>>
>> Arseniy Krasnov (4):
>> vsock: update SO_RCVLOWAT setting callback
>> virtio/vsock: send credit update during setting SO_RCVLOWAT
>> virtio/vsock: fix logic which reduces credit update messages
>> vsock/test: two tests to check credit update logic
>>
>> drivers/vhost/vsock.c | 1 +
>> include/linux/virtio_vsock.h | 1 +
>> include/net/af_vsock.h | 2 +-
>> net/vmw_vsock/af_vsock.c | 9 +-
>> net/vmw_vsock/hyperv_transport.c | 4 +-
>> net/vmw_vsock/virtio_transport.c | 1 +
>> net/vmw_vsock/virtio_transport_common.c | 43 +++++-
>> net/vmw_vsock/vsock_loopback.c | 1 +
>> tools/testing/vsock/vsock_test.c | 175 ++++++++++++++++++++++++
>> 9 files changed, 229 insertions(+), 8 deletions(-)
>>
>> --
>> 2.25.1
>
On Tue, Dec 12, 2023 at 06:59:03PM +0300, Arseniy Krasnov wrote:
>
>
> On 12.12.2023 18:54, Michael S. Tsirkin wrote:
> > On Tue, Dec 12, 2023 at 12:16:54AM +0300, Arseniy Krasnov wrote:
> >> Hello,
> >>
> >> DESCRIPTION
> >>
> >> This patchset fixes old problem with hungup of both rx/tx sides and adds
> >> test for it. This happens due to non-default SO_RCVLOWAT value and
> >> deferred credit update in virtio/vsock. Link to previous old patchset:
> >> https://lore.kernel.org/netdev/[email protected]/
> >
> >
> > Patchset:
> >
> > Acked-by: Michael S. Tsirkin <[email protected]>
>
> Thanks!
>
> >
> >
> > But I worry whether we actually need 3/8 in net not in net-next.
>
> Because of "Fixes" tag ? I think this problem is not critical and reproducible
> only in special cases, but i'm not familiar with netdev process so good, so I don't
> have strong opinion. I guess @Stefano knows better.
>
> Thanks, Arseniy
Fixes means "if you have that other commit then you need this commit
too". I think as a minimum you need to rearrange patches to make the
fix go in first. We don't want a regression followed by a fix.
> >
> > Thanks!
> >
> >> Here is what happens step by step:
> >>
> >> TEST
> >>
> >> INITIAL CONDITIONS
> >>
> >> 1) Vsock buffer size is 128KB.
> >> 2) Maximum packet size is also 64KB as defined in header (yes it is
> >> hardcoded, just to remind about that value).
> >> 3) SO_RCVLOWAT is default, e.g. 1 byte.
> >>
> >>
> >> STEPS
> >>
> >> SENDER RECEIVER
> >> 1) sends 128KB + 1 byte in a
> >> single buffer. 128KB will
> >> be sent, but for 1 byte
> >> sender will wait for free
> >> space at peer. Sender goes
> >> to sleep.
> >>
> >>
> >> 2) reads 64KB, credit update not sent
> >> 3) sets SO_RCVLOWAT to 64KB + 1
> >> 4) poll() -> wait forever, there is
> >> only 64KB available to read.
> >>
> >> So in step 4) receiver also goes to sleep, waiting for enough data or
> >> connection shutdown message from the sender. Idea to fix it is that rx
> >> kicks tx side to continue transmission (and may be close connection)
> >> when rx changes number of bytes to be woken up (e.g. SO_RCVLOWAT) and
> >> this value is bigger than number of available bytes to read.
> >>
> >> I've added small test for this, but not sure as it uses hardcoded value
> >> for maximum packet length, this value is defined in kernel header and
> >> used to control deferred credit update. And as this is not available to
> >> userspace, I can't control test parameters correctly (if one day this
> >> define will be changed - test may become useless).
> >>
> >> Head for this patchset is:
> >> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=021b0c952f226236f2edf89c737efb9a28d1422d
> >>
> >> Link to v1:
> >> https://lore.kernel.org/netdev/[email protected]/
> >> Link to v2:
> >> https://lore.kernel.org/netdev/[email protected]/
> >> Link to v3:
> >> https://lore.kernel.org/netdev/[email protected]/
> >> Link to v4:
> >> https://lore.kernel.org/netdev/[email protected]/
> >> Link to v5:
> >> https://lore.kernel.org/netdev/[email protected]/
> >> Link to v6:
> >> https://lore.kernel.org/netdev/[email protected]/
> >> Link to v7:
> >> https://lore.kernel.org/netdev/[email protected]/
> >>
> >> Changelog:
> >> v1 -> v2:
> >> * Patchset rebased and tested on new HEAD of net-next (see hash above).
> >> * New patch is added as 0001 - it removes return from SO_RCVLOWAT set
> >> callback in 'af_vsock.c' when transport callback is set - with that
> >> we can set 'sk_rcvlowat' only once in 'af_vsock.c' and in future do
> >> not copy-paste it to every transport. It was discussed in v1.
> >> * See per-patch changelog after ---.
> >> v2 -> v3:
> >> * See changelog after --- in 0003 only (0001 and 0002 still same).
> >> v3 -> v4:
> >> * Patchset rebased and tested on new HEAD of net-next (see hash above).
> >> * See per-patch changelog after ---.
> >> v4 -> v5:
> >> * Change patchset tag 'RFC' -> 'net-next'.
> >> * See per-patch changelog after ---.
> >> v5 -> v6:
> >> * New patch 0003 which sends credit update during reading bytes from
> >> socket.
> >> * See per-patch changelog after ---.
> >> v6 -> v7:
> >> * Patchset rebased and tested on new HEAD of net-next (see hash above).
> >> * See per-patch changelog after ---.
> >> v7 -> v8:
> >> * See per-patch changelog after ---.
> >>
> >> Arseniy Krasnov (4):
> >> vsock: update SO_RCVLOWAT setting callback
> >> virtio/vsock: send credit update during setting SO_RCVLOWAT
> >> virtio/vsock: fix logic which reduces credit update messages
> >> vsock/test: two tests to check credit update logic
> >>
> >> drivers/vhost/vsock.c | 1 +
> >> include/linux/virtio_vsock.h | 1 +
> >> include/net/af_vsock.h | 2 +-
> >> net/vmw_vsock/af_vsock.c | 9 +-
> >> net/vmw_vsock/hyperv_transport.c | 4 +-
> >> net/vmw_vsock/virtio_transport.c | 1 +
> >> net/vmw_vsock/virtio_transport_common.c | 43 +++++-
> >> net/vmw_vsock/vsock_loopback.c | 1 +
> >> tools/testing/vsock/vsock_test.c | 175 ++++++++++++++++++++++++
> >> 9 files changed, 229 insertions(+), 8 deletions(-)
> >>
> >> --
> >> 2.25.1
> >
On 12.12.2023 19:12, Michael S. Tsirkin wrote:
> On Tue, Dec 12, 2023 at 06:59:03PM +0300, Arseniy Krasnov wrote:
>>
>>
>> On 12.12.2023 18:54, Michael S. Tsirkin wrote:
>>> On Tue, Dec 12, 2023 at 12:16:54AM +0300, Arseniy Krasnov wrote:
>>>> Hello,
>>>>
>>>> DESCRIPTION
>>>>
>>>> This patchset fixes old problem with hungup of both rx/tx sides and adds
>>>> test for it. This happens due to non-default SO_RCVLOWAT value and
>>>> deferred credit update in virtio/vsock. Link to previous old patchset:
>>>> https://lore.kernel.org/netdev/[email protected]/
>>>
>>>
>>> Patchset:
>>>
>>> Acked-by: Michael S. Tsirkin <[email protected]>
>>
>> Thanks!
>>
>>>
>>>
>>> But I worry whether we actually need 3/8 in net not in net-next.
>>
>> Because of "Fixes" tag ? I think this problem is not critical and reproducible
>> only in special cases, but i'm not familiar with netdev process so good, so I don't
>> have strong opinion. I guess @Stefano knows better.
>>
>> Thanks, Arseniy
>
> Fixes means "if you have that other commit then you need this commit
> too". I think as a minimum you need to rearrange patches to make the
> fix go in first. We don't want a regression followed by a fix.
I see, ok, @Stefano WDYT? I think rearrange doesn't break anything, because this
patch fixes problem that is not related with the new patches from this patchset.
Thanks, Arseniy
>
>>>
>>> Thanks!
>>>
>>>> Here is what happens step by step:
>>>>
>>>> TEST
>>>>
>>>> INITIAL CONDITIONS
>>>>
>>>> 1) Vsock buffer size is 128KB.
>>>> 2) Maximum packet size is also 64KB as defined in header (yes it is
>>>> hardcoded, just to remind about that value).
>>>> 3) SO_RCVLOWAT is default, e.g. 1 byte.
>>>>
>>>>
>>>> STEPS
>>>>
>>>> SENDER RECEIVER
>>>> 1) sends 128KB + 1 byte in a
>>>> single buffer. 128KB will
>>>> be sent, but for 1 byte
>>>> sender will wait for free
>>>> space at peer. Sender goes
>>>> to sleep.
>>>>
>>>>
>>>> 2) reads 64KB, credit update not sent
>>>> 3) sets SO_RCVLOWAT to 64KB + 1
>>>> 4) poll() -> wait forever, there is
>>>> only 64KB available to read.
>>>>
>>>> So in step 4) receiver also goes to sleep, waiting for enough data or
>>>> connection shutdown message from the sender. Idea to fix it is that rx
>>>> kicks tx side to continue transmission (and may be close connection)
>>>> when rx changes number of bytes to be woken up (e.g. SO_RCVLOWAT) and
>>>> this value is bigger than number of available bytes to read.
>>>>
>>>> I've added small test for this, but not sure as it uses hardcoded value
>>>> for maximum packet length, this value is defined in kernel header and
>>>> used to control deferred credit update. And as this is not available to
>>>> userspace, I can't control test parameters correctly (if one day this
>>>> define will be changed - test may become useless).
>>>>
>>>> Head for this patchset is:
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=021b0c952f226236f2edf89c737efb9a28d1422d
>>>>
>>>> Link to v1:
>>>> https://lore.kernel.org/netdev/[email protected]/
>>>> Link to v2:
>>>> https://lore.kernel.org/netdev/[email protected]/
>>>> Link to v3:
>>>> https://lore.kernel.org/netdev/[email protected]/
>>>> Link to v4:
>>>> https://lore.kernel.org/netdev/[email protected]/
>>>> Link to v5:
>>>> https://lore.kernel.org/netdev/[email protected]/
>>>> Link to v6:
>>>> https://lore.kernel.org/netdev/[email protected]/
>>>> Link to v7:
>>>> https://lore.kernel.org/netdev/[email protected]/
>>>>
>>>> Changelog:
>>>> v1 -> v2:
>>>> * Patchset rebased and tested on new HEAD of net-next (see hash above).
>>>> * New patch is added as 0001 - it removes return from SO_RCVLOWAT set
>>>> callback in 'af_vsock.c' when transport callback is set - with that
>>>> we can set 'sk_rcvlowat' only once in 'af_vsock.c' and in future do
>>>> not copy-paste it to every transport. It was discussed in v1.
>>>> * See per-patch changelog after ---.
>>>> v2 -> v3:
>>>> * See changelog after --- in 0003 only (0001 and 0002 still same).
>>>> v3 -> v4:
>>>> * Patchset rebased and tested on new HEAD of net-next (see hash above).
>>>> * See per-patch changelog after ---.
>>>> v4 -> v5:
>>>> * Change patchset tag 'RFC' -> 'net-next'.
>>>> * See per-patch changelog after ---.
>>>> v5 -> v6:
>>>> * New patch 0003 which sends credit update during reading bytes from
>>>> socket.
>>>> * See per-patch changelog after ---.
>>>> v6 -> v7:
>>>> * Patchset rebased and tested on new HEAD of net-next (see hash above).
>>>> * See per-patch changelog after ---.
>>>> v7 -> v8:
>>>> * See per-patch changelog after ---.
>>>>
>>>> Arseniy Krasnov (4):
>>>> vsock: update SO_RCVLOWAT setting callback
>>>> virtio/vsock: send credit update during setting SO_RCVLOWAT
>>>> virtio/vsock: fix logic which reduces credit update messages
>>>> vsock/test: two tests to check credit update logic
>>>>
>>>> drivers/vhost/vsock.c | 1 +
>>>> include/linux/virtio_vsock.h | 1 +
>>>> include/net/af_vsock.h | 2 +-
>>>> net/vmw_vsock/af_vsock.c | 9 +-
>>>> net/vmw_vsock/hyperv_transport.c | 4 +-
>>>> net/vmw_vsock/virtio_transport.c | 1 +
>>>> net/vmw_vsock/virtio_transport_common.c | 43 +++++-
>>>> net/vmw_vsock/vsock_loopback.c | 1 +
>>>> tools/testing/vsock/vsock_test.c | 175 ++++++++++++++++++++++++
>>>> 9 files changed, 229 insertions(+), 8 deletions(-)
>>>>
>>>> --
>>>> 2.25.1
>>>
>
On Tue, Dec 12, 2023 at 08:43:07PM +0300, Arseniy Krasnov wrote:
>
>
>On 12.12.2023 19:12, Michael S. Tsirkin wrote:
>> On Tue, Dec 12, 2023 at 06:59:03PM +0300, Arseniy Krasnov wrote:
>>>
>>>
>>> On 12.12.2023 18:54, Michael S. Tsirkin wrote:
>>>> On Tue, Dec 12, 2023 at 12:16:54AM +0300, Arseniy Krasnov wrote:
>>>>> Hello,
>>>>>
>>>>> DESCRIPTION
>>>>>
>>>>> This patchset fixes old problem with hungup of both rx/tx sides and adds
>>>>> test for it. This happens due to non-default SO_RCVLOWAT value and
>>>>> deferred credit update in virtio/vsock. Link to previous old patchset:
>>>>> https://lore.kernel.org/netdev/[email protected]/
>>>>
>>>>
>>>> Patchset:
>>>>
>>>> Acked-by: Michael S. Tsirkin <[email protected]>
>>>
>>> Thanks!
>>>
>>>>
>>>>
>>>> But I worry whether we actually need 3/8 in net not in net-next.
>>>
>>> Because of "Fixes" tag ? I think this problem is not critical and reproducible
>>> only in special cases, but i'm not familiar with netdev process so good, so I don't
>>> have strong opinion. I guess @Stefano knows better.
>>>
>>> Thanks, Arseniy
>>
>> Fixes means "if you have that other commit then you need this commit
>> too". I think as a minimum you need to rearrange patches to make the
>> fix go in first. We don't want a regression followed by a fix.
>
>I see, ok, @Stefano WDYT? I think rearrange doesn't break anything,
>because this
>patch fixes problem that is not related with the new patches from this patchset.
I agree, patch 3 is for sure net material (I'm fine with both
rearrangement or send it separately), but IMHO also patch 2 could be.
I think with the same fixes tag, since before commit b89d882dc9fc
("vsock/virtio: reduce credit update messages") we sent a credit update
for every bytes we read, so we should not have this problem, right?
So, maybe all the series could be "net".
Thanks,
Stefano
On 13.12.2023 11:43, Stefano Garzarella wrote:
> On Tue, Dec 12, 2023 at 08:43:07PM +0300, Arseniy Krasnov wrote:
>>
>>
>> On 12.12.2023 19:12, Michael S. Tsirkin wrote:
>>> On Tue, Dec 12, 2023 at 06:59:03PM +0300, Arseniy Krasnov wrote:
>>>>
>>>>
>>>> On 12.12.2023 18:54, Michael S. Tsirkin wrote:
>>>>> On Tue, Dec 12, 2023 at 12:16:54AM +0300, Arseniy Krasnov wrote:
>>>>>> Hello,
>>>>>>
>>>>>> DESCRIPTION
>>>>>>
>>>>>> This patchset fixes old problem with hungup of both rx/tx sides and adds
>>>>>> test for it. This happens due to non-default SO_RCVLOWAT value and
>>>>>> deferred credit update in virtio/vsock. Link to previous old patchset:
>>>>>> https://lore.kernel.org/netdev/[email protected]/
>>>>>
>>>>>
>>>>> Patchset:
>>>>>
>>>>> Acked-by: Michael S. Tsirkin <[email protected]>
>>>>
>>>> Thanks!
>>>>
>>>>>
>>>>>
>>>>> But I worry whether we actually need 3/8 in net not in net-next.
>>>>
>>>> Because of "Fixes" tag ? I think this problem is not critical and reproducible
>>>> only in special cases, but i'm not familiar with netdev process so good, so I don't
>>>> have strong opinion. I guess @Stefano knows better.
>>>>
>>>> Thanks, Arseniy
>>>
>>> Fixes means "if you have that other commit then you need this commit
>>> too". I think as a minimum you need to rearrange patches to make the
>>> fix go in first. We don't want a regression followed by a fix.
>>
>> I see, ok, @Stefano WDYT? I think rearrange doesn't break anything, because this
>> patch fixes problem that is not related with the new patches from this patchset.
>
> I agree, patch 3 is for sure net material (I'm fine with both rearrangement or send it separately), but IMHO also patch 2 could be.
> I think with the same fixes tag, since before commit b89d882dc9fc ("vsock/virtio: reduce credit update messages") we sent a credit update
> for every bytes we read, so we should not have this problem, right?
Agree for 2, so I think I can rearrange: two fixes go first, then current 0001, and then tests. And send it as V9 for 'net' only ?
Thanks, Arseniy
>
> So, maybe all the series could be "net".
>
> Thanks,
> Stefano
>
On Wed, Dec 13, 2023 at 12:08:27PM +0300, Arseniy Krasnov wrote:
>
>
>On 13.12.2023 11:43, Stefano Garzarella wrote:
>> On Tue, Dec 12, 2023 at 08:43:07PM +0300, Arseniy Krasnov wrote:
>>>
>>>
>>> On 12.12.2023 19:12, Michael S. Tsirkin wrote:
>>>> On Tue, Dec 12, 2023 at 06:59:03PM +0300, Arseniy Krasnov wrote:
>>>>>
>>>>>
>>>>> On 12.12.2023 18:54, Michael S. Tsirkin wrote:
>>>>>> On Tue, Dec 12, 2023 at 12:16:54AM +0300, Arseniy Krasnov wrote:
>>>>>>> Hello,
>>>>>>>
>>>>>>> ?????????????????????????????? DESCRIPTION
>>>>>>>
>>>>>>> This patchset fixes old problem with hungup of both rx/tx sides and adds
>>>>>>> test for it. This happens due to non-default SO_RCVLOWAT value and
>>>>>>> deferred credit update in virtio/vsock. Link to previous old patchset:
>>>>>>> https://lore.kernel.org/netdev/[email protected]/
>>>>>>
>>>>>>
>>>>>> Patchset:
>>>>>>
>>>>>> Acked-by: Michael S. Tsirkin <[email protected]>
>>>>>
>>>>> Thanks!
>>>>>
>>>>>>
>>>>>>
>>>>>> But I worry whether we actually need 3/8 in net not in net-next.
>>>>>
>>>>> Because of "Fixes" tag ? I think this problem is not critical and reproducible
>>>>> only in special cases, but i'm not familiar with netdev process so good, so I don't
>>>>> have strong opinion. I guess @Stefano knows better.
>>>>>
>>>>> Thanks, Arseniy
>>>>
>>>> Fixes means "if you have that other commit then you need this commit
>>>> too". I think as a minimum you need to rearrange patches to make the
>>>> fix go in first. We don't want a regression followed by a fix.
>>>
>>> I see, ok, @Stefano WDYT? I think rearrange doesn't break anything, because this
>>> patch fixes problem that is not related with the new patches from this patchset.
>>
>> I agree, patch 3 is for sure net material (I'm fine with both rearrangement or send it separately), but IMHO also patch 2 could be.
>> I think with the same fixes tag, since before commit b89d882dc9fc ("vsock/virtio: reduce credit update messages") we sent a credit update
>> for every bytes we read, so we should not have this problem, right?
>
>Agree for 2, so I think I can rearrange: two fixes go first, then current 0001, and then tests. And send it as V9 for 'net' only ?
Maybe you can add this to patch 1 if we want it on net:
Fixes: e38f22c860ed ("vsock: SO_RCVLOWAT transport set callback")
Then I think that patch should go before patch 2, so we don't need to
touch that code multiple times.
so, IMHO the order should be the actual order or 3 - 1 - 2 - 4.
Another option is to send just 2 & 3 to net, and the rest (1 & 4) to
net-next. IMHO should be fine to send the entire series to net with the
fixes tag also in patch 1.
Net maintainers and Michael might have a different advice.
Thanks,
Stefano
On 13.12.2023 12:41, Stefano Garzarella wrote:
> On Wed, Dec 13, 2023 at 12:08:27PM +0300, Arseniy Krasnov wrote:
>>
>>
>> On 13.12.2023 11:43, Stefano Garzarella wrote:
>>> On Tue, Dec 12, 2023 at 08:43:07PM +0300, Arseniy Krasnov wrote:
>>>>
>>>>
>>>> On 12.12.2023 19:12, Michael S. Tsirkin wrote:
>>>>> On Tue, Dec 12, 2023 at 06:59:03PM +0300, Arseniy Krasnov wrote:
>>>>>>
>>>>>>
>>>>>> On 12.12.2023 18:54, Michael S. Tsirkin wrote:
>>>>>>> On Tue, Dec 12, 2023 at 12:16:54AM +0300, Arseniy Krasnov wrote:
>>>>>>>> Hello,
>>>>>>>>
>>>>>>>> DESCRIPTION
>>>>>>>>
>>>>>>>> This patchset fixes old problem with hungup of both rx/tx sides and adds
>>>>>>>> test for it. This happens due to non-default SO_RCVLOWAT value and
>>>>>>>> deferred credit update in virtio/vsock. Link to previous old patchset:
>>>>>>>> https://lore.kernel.org/netdev/[email protected]/
>>>>>>>
>>>>>>>
>>>>>>> Patchset:
>>>>>>>
>>>>>>> Acked-by: Michael S. Tsirkin <[email protected]>
>>>>>>
>>>>>> Thanks!
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> But I worry whether we actually need 3/8 in net not in net-next.
>>>>>>
>>>>>> Because of "Fixes" tag ? I think this problem is not critical and reproducible
>>>>>> only in special cases, but i'm not familiar with netdev process so good, so I don't
>>>>>> have strong opinion. I guess @Stefano knows better.
>>>>>>
>>>>>> Thanks, Arseniy
>>>>>
>>>>> Fixes means "if you have that other commit then you need this commit
>>>>> too". I think as a minimum you need to rearrange patches to make the
>>>>> fix go in first. We don't want a regression followed by a fix.
>>>>
>>>> I see, ok, @Stefano WDYT? I think rearrange doesn't break anything, because this
>>>> patch fixes problem that is not related with the new patches from this patchset.
>>>
>>> I agree, patch 3 is for sure net material (I'm fine with both rearrangement or send it separately), but IMHO also patch 2 could be.
>>> I think with the same fixes tag, since before commit b89d882dc9fc ("vsock/virtio: reduce credit update messages") we sent a credit update
>>> for every bytes we read, so we should not have this problem, right?
>>
>> Agree for 2, so I think I can rearrange: two fixes go first, then current 0001, and then tests. And send it as V9 for 'net' only ?
>
> Maybe you can add this to patch 1 if we want it on net:
>
> Fixes: e38f22c860ed ("vsock: SO_RCVLOWAT transport set callback")
>
> Then I think that patch should go before patch 2, so we don't need to
> touch that code multiple times.
>
> so, IMHO the order should be the actual order or 3 - 1 - 2 - 4.
>
> Another option is to send just 2 & 3 to net, and the rest (1 & 4) to net-next. IMHO should be fine to send the entire series to net with the fixes tag also in patch 1.
Ok, agree that it is good to send whole patchset to net without splitting it.
>
> Net maintainers and Michael might have a different advice.
Ok
>
> Thanks,
> Stefano
>
On Wed, Dec 13, 2023 at 12:08:27PM +0300, Arseniy Krasnov wrote:
>
>
> On 13.12.2023 11:43, Stefano Garzarella wrote:
> > On Tue, Dec 12, 2023 at 08:43:07PM +0300, Arseniy Krasnov wrote:
> >>
> >>
> >> On 12.12.2023 19:12, Michael S. Tsirkin wrote:
> >>> On Tue, Dec 12, 2023 at 06:59:03PM +0300, Arseniy Krasnov wrote:
> >>>>
> >>>>
> >>>> On 12.12.2023 18:54, Michael S. Tsirkin wrote:
> >>>>> On Tue, Dec 12, 2023 at 12:16:54AM +0300, Arseniy Krasnov wrote:
> >>>>>> Hello,
> >>>>>>
> >>>>>> ?????????????????????????????? DESCRIPTION
> >>>>>>
> >>>>>> This patchset fixes old problem with hungup of both rx/tx sides and adds
> >>>>>> test for it. This happens due to non-default SO_RCVLOWAT value and
> >>>>>> deferred credit update in virtio/vsock. Link to previous old patchset:
> >>>>>> https://lore.kernel.org/netdev/[email protected]/
> >>>>>
> >>>>>
> >>>>> Patchset:
> >>>>>
> >>>>> Acked-by: Michael S. Tsirkin <[email protected]>
> >>>>
> >>>> Thanks!
> >>>>
> >>>>>
> >>>>>
> >>>>> But I worry whether we actually need 3/8 in net not in net-next.
> >>>>
> >>>> Because of "Fixes" tag ? I think this problem is not critical and reproducible
> >>>> only in special cases, but i'm not familiar with netdev process so good, so I don't
> >>>> have strong opinion. I guess @Stefano knows better.
> >>>>
> >>>> Thanks, Arseniy
> >>>
> >>> Fixes means "if you have that other commit then you need this commit
> >>> too". I think as a minimum you need to rearrange patches to make the
> >>> fix go in first. We don't want a regression followed by a fix.
> >>
> >> I see, ok, @Stefano WDYT? I think rearrange doesn't break anything, because this
> >> patch fixes problem that is not related with the new patches from this patchset.
> >
> > I agree, patch 3 is for sure net material (I'm fine with both rearrangement or send it separately), but IMHO also patch 2 could be.
> > I think with the same fixes tag, since before commit b89d882dc9fc ("vsock/virtio: reduce credit update messages") we sent a credit update
> > for every bytes we read, so we should not have this problem, right?
>
> Agree for 2, so I think I can rearrange: two fixes go first, then current 0001, and then tests. And send it as V9 for 'net' only ?
>
> Thanks, Arseniy
hmm why not net-next?
> >
> > So, maybe all the series could be "net".
> >
> > Thanks,
> > Stefano
> >
On Wed, Dec 13, 2023 at 10:05:44AM -0500, Michael S. Tsirkin wrote:
> On Wed, Dec 13, 2023 at 12:08:27PM +0300, Arseniy Krasnov wrote:
> >
> >
> > On 13.12.2023 11:43, Stefano Garzarella wrote:
> > > On Tue, Dec 12, 2023 at 08:43:07PM +0300, Arseniy Krasnov wrote:
> > >>
> > >>
> > >> On 12.12.2023 19:12, Michael S. Tsirkin wrote:
> > >>> On Tue, Dec 12, 2023 at 06:59:03PM +0300, Arseniy Krasnov wrote:
> > >>>>
> > >>>>
> > >>>> On 12.12.2023 18:54, Michael S. Tsirkin wrote:
> > >>>>> On Tue, Dec 12, 2023 at 12:16:54AM +0300, Arseniy Krasnov wrote:
> > >>>>>> Hello,
> > >>>>>>
> > >>>>>> ?????????????????????????????? DESCRIPTION
> > >>>>>>
> > >>>>>> This patchset fixes old problem with hungup of both rx/tx sides and adds
> > >>>>>> test for it. This happens due to non-default SO_RCVLOWAT value and
> > >>>>>> deferred credit update in virtio/vsock. Link to previous old patchset:
> > >>>>>> https://lore.kernel.org/netdev/[email protected]/
> > >>>>>
> > >>>>>
> > >>>>> Patchset:
> > >>>>>
> > >>>>> Acked-by: Michael S. Tsirkin <[email protected]>
> > >>>>
> > >>>> Thanks!
> > >>>>
> > >>>>>
> > >>>>>
> > >>>>> But I worry whether we actually need 3/8 in net not in net-next.
> > >>>>
> > >>>> Because of "Fixes" tag ? I think this problem is not critical and reproducible
> > >>>> only in special cases, but i'm not familiar with netdev process so good, so I don't
> > >>>> have strong opinion. I guess @Stefano knows better.
> > >>>>
> > >>>> Thanks, Arseniy
> > >>>
> > >>> Fixes means "if you have that other commit then you need this commit
> > >>> too". I think as a minimum you need to rearrange patches to make the
> > >>> fix go in first. We don't want a regression followed by a fix.
> > >>
> > >> I see, ok, @Stefano WDYT? I think rearrange doesn't break anything, because this
> > >> patch fixes problem that is not related with the new patches from this patchset.
> > >
> > > I agree, patch 3 is for sure net material (I'm fine with both rearrangement or send it separately), but IMHO also patch 2 could be.
> > > I think with the same fixes tag, since before commit b89d882dc9fc ("vsock/virtio: reduce credit update messages") we sent a credit update
> > > for every bytes we read, so we should not have this problem, right?
> >
> > Agree for 2, so I think I can rearrange: two fixes go first, then current 0001, and then tests. And send it as V9 for 'net' only ?
> >
> > Thanks, Arseniy
>
>
> hmm why not net-next?
Oh I missed your previous discussion. I think everything in net-next is
safer. Having said that, I won't nack it net, either.
> > >
> > > So, maybe all the series could be "net".
> > >
> > > Thanks,
> > > Stefano
> > >
On Wed, Dec 13, 2023 at 08:11:57PM +0300, Arseniy Krasnov wrote:
>
>
> On 13.12.2023 18:13, Michael S. Tsirkin wrote:
> > On Wed, Dec 13, 2023 at 10:05:44AM -0500, Michael S. Tsirkin wrote:
> >> On Wed, Dec 13, 2023 at 12:08:27PM +0300, Arseniy Krasnov wrote:
> >>>
> >>>
> >>> On 13.12.2023 11:43, Stefano Garzarella wrote:
> >>>> On Tue, Dec 12, 2023 at 08:43:07PM +0300, Arseniy Krasnov wrote:
> >>>>>
> >>>>>
> >>>>> On 12.12.2023 19:12, Michael S. Tsirkin wrote:
> >>>>>> On Tue, Dec 12, 2023 at 06:59:03PM +0300, Arseniy Krasnov wrote:
> >>>>>>>
> >>>>>>>
> >>>>>>> On 12.12.2023 18:54, Michael S. Tsirkin wrote:
> >>>>>>>> On Tue, Dec 12, 2023 at 12:16:54AM +0300, Arseniy Krasnov wrote:
> >>>>>>>>> Hello,
> >>>>>>>>>
> >>>>>>>>> ?????????????????????????????? DESCRIPTION
> >>>>>>>>>
> >>>>>>>>> This patchset fixes old problem with hungup of both rx/tx sides and adds
> >>>>>>>>> test for it. This happens due to non-default SO_RCVLOWAT value and
> >>>>>>>>> deferred credit update in virtio/vsock. Link to previous old patchset:
> >>>>>>>>> https://lore.kernel.org/netdev/[email protected]/
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Patchset:
> >>>>>>>>
> >>>>>>>> Acked-by: Michael S. Tsirkin <[email protected]>
> >>>>>>>
> >>>>>>> Thanks!
> >>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> But I worry whether we actually need 3/8 in net not in net-next.
> >>>>>>>
> >>>>>>> Because of "Fixes" tag ? I think this problem is not critical and reproducible
> >>>>>>> only in special cases, but i'm not familiar with netdev process so good, so I don't
> >>>>>>> have strong opinion. I guess @Stefano knows better.
> >>>>>>>
> >>>>>>> Thanks, Arseniy
> >>>>>>
> >>>>>> Fixes means "if you have that other commit then you need this commit
> >>>>>> too". I think as a minimum you need to rearrange patches to make the
> >>>>>> fix go in first. We don't want a regression followed by a fix.
> >>>>>
> >>>>> I see, ok, @Stefano WDYT? I think rearrange doesn't break anything, because this
> >>>>> patch fixes problem that is not related with the new patches from this patchset.
> >>>>
> >>>> I agree, patch 3 is for sure net material (I'm fine with both rearrangement or send it separately), but IMHO also patch 2 could be.
> >>>> I think with the same fixes tag, since before commit b89d882dc9fc ("vsock/virtio: reduce credit update messages") we sent a credit update
> >>>> for every bytes we read, so we should not have this problem, right?
> >>>
> >>> Agree for 2, so I think I can rearrange: two fixes go first, then current 0001, and then tests. And send it as V9 for 'net' only ?
> >>>
> >>> Thanks, Arseniy
> >>
> >>
> >> hmm why not net-next?
> >
> > Oh I missed your previous discussion. I think everything in net-next is
> > safer. Having said that, I won't nack it net, either.
>
> So, summarizing all above:
> 1) This patchset entirely goes to net-next as v9
> 2) I reorder patches like 3 - 2 - 1 - 4, e.g. two fixes goes first with Fixes tag
> 3) Add Acked-by: Michael S. Tsirkin <[email protected]> to each patch
>
> @Michael, @Stefano ?
>
> Thanks, Arseniy
Fine by me.
> >
> >>>>
> >>>> So, maybe all the series could be "net".
> >>>>
> >>>> Thanks,
> >>>> Stefano
> >>>>
> >
On 13.12.2023 18:13, Michael S. Tsirkin wrote:
> On Wed, Dec 13, 2023 at 10:05:44AM -0500, Michael S. Tsirkin wrote:
>> On Wed, Dec 13, 2023 at 12:08:27PM +0300, Arseniy Krasnov wrote:
>>>
>>>
>>> On 13.12.2023 11:43, Stefano Garzarella wrote:
>>>> On Tue, Dec 12, 2023 at 08:43:07PM +0300, Arseniy Krasnov wrote:
>>>>>
>>>>>
>>>>> On 12.12.2023 19:12, Michael S. Tsirkin wrote:
>>>>>> On Tue, Dec 12, 2023 at 06:59:03PM +0300, Arseniy Krasnov wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 12.12.2023 18:54, Michael S. Tsirkin wrote:
>>>>>>>> On Tue, Dec 12, 2023 at 12:16:54AM +0300, Arseniy Krasnov wrote:
>>>>>>>>> Hello,
>>>>>>>>>
>>>>>>>>> DESCRIPTION
>>>>>>>>>
>>>>>>>>> This patchset fixes old problem with hungup of both rx/tx sides and adds
>>>>>>>>> test for it. This happens due to non-default SO_RCVLOWAT value and
>>>>>>>>> deferred credit update in virtio/vsock. Link to previous old patchset:
>>>>>>>>> https://lore.kernel.org/netdev/[email protected]/
>>>>>>>>
>>>>>>>>
>>>>>>>> Patchset:
>>>>>>>>
>>>>>>>> Acked-by: Michael S. Tsirkin <[email protected]>
>>>>>>>
>>>>>>> Thanks!
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> But I worry whether we actually need 3/8 in net not in net-next.
>>>>>>>
>>>>>>> Because of "Fixes" tag ? I think this problem is not critical and reproducible
>>>>>>> only in special cases, but i'm not familiar with netdev process so good, so I don't
>>>>>>> have strong opinion. I guess @Stefano knows better.
>>>>>>>
>>>>>>> Thanks, Arseniy
>>>>>>
>>>>>> Fixes means "if you have that other commit then you need this commit
>>>>>> too". I think as a minimum you need to rearrange patches to make the
>>>>>> fix go in first. We don't want a regression followed by a fix.
>>>>>
>>>>> I see, ok, @Stefano WDYT? I think rearrange doesn't break anything, because this
>>>>> patch fixes problem that is not related with the new patches from this patchset.
>>>>
>>>> I agree, patch 3 is for sure net material (I'm fine with both rearrangement or send it separately), but IMHO also patch 2 could be.
>>>> I think with the same fixes tag, since before commit b89d882dc9fc ("vsock/virtio: reduce credit update messages") we sent a credit update
>>>> for every bytes we read, so we should not have this problem, right?
>>>
>>> Agree for 2, so I think I can rearrange: two fixes go first, then current 0001, and then tests. And send it as V9 for 'net' only ?
>>>
>>> Thanks, Arseniy
>>
>>
>> hmm why not net-next?
>
> Oh I missed your previous discussion. I think everything in net-next is
> safer. Having said that, I won't nack it net, either.
So, summarizing all above:
1) This patchset entirely goes to net-next as v9
2) I reorder patches like 3 - 2 - 1 - 4, e.g. two fixes goes first with Fixes tag
3) Add Acked-by: Michael S. Tsirkin <[email protected]> to each patch
@Michael, @Stefano ?
Thanks, Arseniy
>
>>>>
>>>> So, maybe all the series could be "net".
>>>>
>>>> Thanks,
>>>> Stefano
>>>>
>
On Wed, Dec 13, 2023 at 08:11:57PM +0300, Arseniy Krasnov wrote:
>
>
>On 13.12.2023 18:13, Michael S. Tsirkin wrote:
>> On Wed, Dec 13, 2023 at 10:05:44AM -0500, Michael S. Tsirkin wrote:
>>> On Wed, Dec 13, 2023 at 12:08:27PM +0300, Arseniy Krasnov wrote:
>>>>
>>>>
>>>> On 13.12.2023 11:43, Stefano Garzarella wrote:
>>>>> On Tue, Dec 12, 2023 at 08:43:07PM +0300, Arseniy Krasnov wrote:
>>>>>>
>>>>>>
>>>>>> On 12.12.2023 19:12, Michael S. Tsirkin wrote:
>>>>>>> On Tue, Dec 12, 2023 at 06:59:03PM +0300, Arseniy Krasnov wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 12.12.2023 18:54, Michael S. Tsirkin wrote:
>>>>>>>>> On Tue, Dec 12, 2023 at 12:16:54AM +0300, Arseniy Krasnov wrote:
>>>>>>>>>> Hello,
>>>>>>>>>>
>>>>>>>>>> ?????????????????????????????? DESCRIPTION
>>>>>>>>>>
>>>>>>>>>> This patchset fixes old problem with hungup of both rx/tx sides and adds
>>>>>>>>>> test for it. This happens due to non-default SO_RCVLOWAT value and
>>>>>>>>>> deferred credit update in virtio/vsock. Link to previous old patchset:
>>>>>>>>>> https://lore.kernel.org/netdev/[email protected]/
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Patchset:
>>>>>>>>>
>>>>>>>>> Acked-by: Michael S. Tsirkin <[email protected]>
>>>>>>>>
>>>>>>>> Thanks!
>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> But I worry whether we actually need 3/8 in net not in net-next.
>>>>>>>>
>>>>>>>> Because of "Fixes" tag ? I think this problem is not critical and reproducible
>>>>>>>> only in special cases, but i'm not familiar with netdev process so good, so I don't
>>>>>>>> have strong opinion. I guess @Stefano knows better.
>>>>>>>>
>>>>>>>> Thanks, Arseniy
>>>>>>>
>>>>>>> Fixes means "if you have that other commit then you need this commit
>>>>>>> too". I think as a minimum you need to rearrange patches to make the
>>>>>>> fix go in first. We don't want a regression followed by a fix.
>>>>>>
>>>>>> I see, ok, @Stefano WDYT? I think rearrange doesn't break anything, because this
>>>>>> patch fixes problem that is not related with the new patches from this patchset.
>>>>>
>>>>> I agree, patch 3 is for sure net material (I'm fine with both rearrangement or send it separately), but IMHO also patch 2 could be.
>>>>> I think with the same fixes tag, since before commit b89d882dc9fc ("vsock/virtio: reduce credit update messages") we sent a credit update
>>>>> for every bytes we read, so we should not have this problem, right?
>>>>
>>>> Agree for 2, so I think I can rearrange: two fixes go first, then current 0001, and then tests. And send it as V9 for 'net' only ?
>>>>
>>>> Thanks, Arseniy
>>>
>>>
>>> hmm why not net-next?
>>
>> Oh I missed your previous discussion. I think everything in net-next is
>> safer. Having said that, I won't nack it net, either.
>
>So, summarizing all above:
>1) This patchset entirely goes to net-next as v9
>2) I reorder patches like 3 - 2 - 1 - 4, e.g. two fixes goes first with Fixes tag
>3) Add Acked-by: Michael S. Tsirkin <[email protected]> to each patch
>
>@Michael, @Stefano ?
Okay, let's do that ;-)
Stefano