Re: [RFC PATCH v3 0/4] several updates to virtio/vsock

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Mar 09, 2023 at 07:20:20PM +0300, Arseniy Krasnov wrote:


On 09.03.2023 19:21, Stefano Garzarella wrote:
On Thu, Mar 09, 2023 at 01:10:36PM +0300, Arseniy Krasnov wrote:
Hello,

this patchset evolved from previous v2 version (see link below). It does
several updates to virtio/vsock:
1) Changes 'virtio_transport_inc/dec_rx_pkt()' interface. Now instead of
  using skbuff state ('head' and 'data' pointers) to update 'fwd_cnt'
  and 'rx_bytes', integer value is passed as an input argument. This
  makes code more simple, because in this case we don't need to udpate
  skbuff state before calling 'virtio_transport_inc/dec_rx_pkt()'. In
  more common words - we don't need to change skbuff state to update
  'rx_bytes' and 'fwd_cnt' correctly.
2) For SOCK_STREAM, when copying data to user fails, current skbuff is
  not dropped. Next read attempt will use same skbuff and last offset.
  Instead of 'skb_dequeue()', 'skb_peek()' + '__skb_unlink()' are used.
  This behaviour was implemented before skbuff support.
3) For SOCK_SEQPACKET it removes unneeded 'skb_pull()' call, because for
  this type of socket each skbuff is used only once: after removing it
  from socket's queue, it will be freed anyway.

Test for 2) also added:
Test tries to 'recv()' data to NULL buffer, then does 'recv()' with valid
buffer. For SOCK_STREAM second 'recv()' must return data, because skbuff
must not be dropped, but for SOCK_SEQPACKET skbuff will be dropped by
kernel, and 'recv()' will return EAGAIN.

Link to v1 on lore:
https://lore.kernel.org/netdev/c2d3e204-89d9-88e9-8a15-3fe027e56b4b@xxxxxxxxxxxxxx/

Link to v2 on lore:
https://lore.kernel.org/netdev/a7ab414b-5e41-c7b6-250b-e8401f335859@xxxxxxxxxxxxxx/

Change log:

v1 -> v2:
- For SOCK_SEQPACKET call 'skb_pull()' also in case of copy failure or
  dropping skbuff (when we just waiting message end).
- Handle copy failure for SOCK_STREAM in the same manner (plus free
  current skbuff).
- Replace bug repdroducer with new test in vsock_test.c

v2 -> v3:
- Replace patch which removes 'skb->len' subtraction from function
  'virtio_transport_dec_rx_pkt()' with patch which updates functions
  'virtio_transport_inc/dec_rx_pkt()' by passing integer argument
  instead of skbuff pointer.
- Replace patch which drops skbuff when copying to user fails with
  patch which changes this behaviour by keeping skbuff in queue until
  it has no data.
- Add patch for SOCK_SEQPACKET which removes redundant 'skb_pull()'
  call on read.
- I remove "Fixes" tag from all patches, because all of them now change
  code logic, not only fix something.

Yes, but they solve the problem, so we should use the tag (I think at
least in patch 1 and 3).

We usually use the tag when we are fixing a problem introduced by a
previous change. So we need to backport the patch to the stable branches
as well, and we need the tag to figure out which branches have the patch
or not.
Ahh, sorry. Ok. I see now :)

No problem at all :-)

I think also patch 2 can have the Fixes tag.

Thanks,
Stefano


Thanks, Arseniy

Thanks,
Stefano


Arseniy Krasnov (4):
 virtio/vsock: don't use skbuff state to account credit
 virtio/vsock: remove redundant 'skb_pull()' call
 virtio/vsock: don't drop skbuff on copy failure
 test/vsock: copy to user failure test

net/vmw_vsock/virtio_transport_common.c |  29 +++---
tools/testing/vsock/vsock_test.c        | 118 ++++++++++++++++++++++++
2 files changed, 131 insertions(+), 16 deletions(-)

-- 
2.25.1







[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux