All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arseniy Krasnov <avkrasnov@sberdevices.ru>
To: Stefan Hajnoczi <stefanha@redhat.com>,
	Stefano Garzarella <sgarzare@redhat.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Bobby Eshleman <bobby.eshleman@bytedance.com>
Cc: <kvm@vger.kernel.org>,
	<virtualization@lists.linux-foundation.org>,
	<netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<kernel@sberdevices.ru>, <oxffffaa@gmail.com>,
	<avkrasnov@sberdevices.ru>
Subject: [RFC PATCH v2 0/4] virtio/vsock: fix credit update logic
Date: Sun, 5 Mar 2023 23:04:45 +0300	[thread overview]
Message-ID: <a7ab414b-5e41-c7b6-250b-e8401f335859@sberdevices.ru> (raw)

Hello,

this patchset fixes three things in skbuff handling:
1) Current implementation of 'virtio_transport_dec_rx_pkt()':

   value to update 'rx_bytes' and 'fwd_cnt' is calculated as:

   skb_headroom(skb) - sizeof(struct virtio_vsock_hdr) - skb->len;

   i'm a little bit confused about subtracting 'skb->len'. It is clear,
   that difference between first two components is number of bytes copied
   to user. 'skb_headroom()' is delta between 'data' and 'head'. 'data'
   is incremented on each copy data to user from skb by call 'skb_pull()'
   (at the same moment, 'skb->len' is decremented to the same amount of
   bytes). 'head' points to the header of the packet. But what is purpose
   of 'skb->len' here? For SOCK_STREAM is has no effect because this
   logic is called only when 'skb->len' == 0, but for SOCK_SEQPACKET and
   other future calls i think it is buggy.

2) For SOCK_SEQPACKET all sk_buffs are handled only once - after dequeue
   each sk_buff is removed, so user will never read rest of the data.
   Thus we need to update credit parameters of the socket ('rx_bytes' and
   'fwd_cnt') like whole sk_buff is read - so call 'skb_pull()' for the
   whole buffer.

3) For SOCK_STREAM when 'memcpy_to_msg()' fails it fixes 'rx_bytes'
   update (like in 2)) and frees current skbuff.

Test is also added to vsock_test. It does two attempts to read data from
socket - first attempt to invalid buffer (kernel must drop skb). Second
attempt is performed with valid buffer and MSG_DONTWAIT flag. If socket's
queue will be empty (skbuff was dropped due to 'memcpy_to_msg()' fail
and 'rx_bytes' which controls data waiting set to 0), such call will
return immediately with EAGAIN.

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

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

Arseniy Krasnov (4):
  virtio/vsock: fix 'rx_bytes'/'fwd_cnt' calculation
  virtio/vsock: remove all data from sk_buff
  virtio/vsock: free skb on data copy failure
  test/vsock: invalid buffer tests

 net/vmw_vsock/virtio_transport_common.c |  10 ++-
 tools/testing/vsock/vsock_test.c        | 106 ++++++++++++++++++++++++
 2 files changed, 113 insertions(+), 3 deletions(-)

-- 
2.25.1

             reply	other threads:[~2023-03-05 20:07 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-05 20:04 Arseniy Krasnov [this message]
2023-03-05 20:06 ` [RFC PATCH v2 1/4] virtio/vsock: fix 'rx_bytes'/'fwd_cnt' calculation Arseniy Krasnov
2023-03-06 11:57   ` Stefano Garzarella
2023-03-06 11:57     ` Stefano Garzarella
2023-03-06 15:27     ` Arseniy Krasnov
2023-03-05 20:07 ` [RFC PATCH v2 2/4] virtio/vsock: remove all data from sk_buff Arseniy Krasnov
2023-03-06 12:08   ` Stefano Garzarella
2023-03-06 12:08     ` Stefano Garzarella
2023-03-06 15:31     ` Arseniy Krasnov
2023-03-06 15:51       ` Stefano Garzarella
2023-03-06 15:51         ` Stefano Garzarella
2023-03-06 16:00         ` Arseniy Krasnov
2023-03-06 16:18           ` Stefano Garzarella
2023-03-06 16:18             ` Stefano Garzarella
2023-03-07 23:53             ` Bobby Eshleman
2023-03-05 20:08 ` [RFC PATCH v2 3/4] virtio/vsock: free skb on data copy failure Arseniy Krasnov
2023-03-06 12:07   ` Stefano Garzarella
2023-03-06 12:07     ` Stefano Garzarella
2023-03-06 15:28     ` Arseniy Krasnov
2023-03-05 20:09 ` [RFC PATCH v2 4/4] test/vsock: invalid buffer tests Arseniy Krasnov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a7ab414b-5e41-c7b6-250b-e8401f335859@sberdevices.ru \
    --to=avkrasnov@sberdevices.ru \
    --cc=bobby.eshleman@bytedance.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kernel@sberdevices.ru \
    --cc=kuba@kernel.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=oxffffaa@gmail.com \
    --cc=pabeni@redhat.com \
    --cc=sgarzare@redhat.com \
    --cc=stefanha@redhat.com \
    --cc=virtualization@lists.linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.