From: Stefano Garzarella <sgarzare@redhat.com>
To: Arseniy Krasnov <avkrasnov@sberdevices.ru>
Cc: Bobby Eshleman <bobby.eshleman@bytedance.com>,
kvm@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, oxffffaa@gmail.com,
Eric Dumazet <edumazet@google.com>,
Stefan Hajnoczi <stefanha@redhat.com>,
kernel@sberdevices.ru, Jakub Kicinski <kuba@kernel.org>,
Paolo Abeni <pabeni@redhat.com>,
"David S. Miller" <davem@davemloft.net>
Subject: Re: [RFC PATCH v2 2/3] virtio/vsock: WARN_ONCE() for invalid state of socket
Date: Tue, 28 Mar 2023 11:41:03 +0200 [thread overview]
Message-ID: <CAGxU2F707_88UGt1w9pp_KLrg8EY7-BjceM+N1S6YHXSmBC+eQ@mail.gmail.com> (raw)
In-Reply-To: <d91ac5f0-1f47-58b3-d033-f492d0e17da7@sberdevices.ru>
On Tue, Mar 28, 2023 at 11:35 AM Arseniy Krasnov
<avkrasnov@sberdevices.ru> wrote:
>
>
>
> On 28.03.2023 12:29, Stefano Garzarella wrote:
> > On Sun, Mar 26, 2023 at 01:09:25AM +0300, Arseniy Krasnov wrote:
> >> This adds WARN_ONCE() and return from stream dequeue callback when
> >> socket's queue is empty, but 'rx_bytes' still non-zero.
> >
> > Nit: I would explain why we add this, for example:
> >
> > This allows the detection of potential bugs due to packet merging
> > (see previous patch).
> >
> >>
> >> Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru>
> >> ---
> >> net/vmw_vsock/virtio_transport_common.c | 7 +++++++
> >> 1 file changed, 7 insertions(+)
> >
> >>
> >> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> >> index b9144af71553..ad70531de133 100644
> >> --- a/net/vmw_vsock/virtio_transport_common.c
> >> +++ b/net/vmw_vsock/virtio_transport_common.c
> >> @@ -398,6 +398,13 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk,
> >> u32 free_space;
> >>
> >> spin_lock_bh(&vvs->rx_lock);
> >> +
> >> + if (WARN_ONCE(skb_queue_empty(&vvs->rx_queue) && vvs->rx_bytes,
> >> + "No skbuffs with non-zero 'rx_bytes'\n")) {
> >
> > Nit: I would rephrase it this way:
> > "rx_queue is empty, but rx_bytes is non-zero"
> >
> >> + spin_unlock_bh(&vvs->rx_lock);
> >> + return err;
> >> + }
> >> +
> >> while (total < len && !skb_queue_empty(&vvs->rx_queue)) {
> >> skb = skb_peek(&vvs->rx_queue);
> >>
> >> --
> >> 2.25.1
> >>
> >
> > Anyway the patch LGTM!
>
> Thanks for review! Since only string value and commit message should be
> updated, i can resend it with 'net' (as it is fix) and update two thing
> above in 'net' version?
Yep, sure!
And you can already add my R-b ;-)
Thanks,
Stefano
>
> Thanks, Arseniy
> >
> > Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
> >
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
next prev parent reply other threads:[~2023-03-28 9:41 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <728181e9-6b35-0092-3d01-3d7aff4521b6@sberdevices.ru>
[not found] ` <b5d31a81-a089-146b-d04f-569710e6b14b@sberdevices.ru>
2023-03-28 9:23 ` [RFC PATCH v2 1/3] virtio/vsock: fix header length on skb merging Stefano Garzarella
[not found] ` <30aa2604-77c0-322e-44fd-ff99fc25e388@sberdevices.ru>
2023-03-28 9:29 ` [RFC PATCH v2 2/3] virtio/vsock: WARN_ONCE() for invalid state of socket Stefano Garzarella
[not found] ` <d91ac5f0-1f47-58b3-d033-f492d0e17da7@sberdevices.ru>
2023-03-28 9:41 ` Stefano Garzarella [this message]
[not found] ` <776a30e8-3cd5-35ba-5187-63c9b83eaa44@sberdevices.ru>
2023-03-28 9:30 ` [RFC PATCH v2 3/3] test/vsock: new skbuff appending test Stefano Garzarella
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=CAGxU2F707_88UGt1w9pp_KLrg8EY7-BjceM+N1S6YHXSmBC+eQ@mail.gmail.com \
--to=sgarzare@redhat.com \
--cc=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=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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).