Stefano Garzarella
2023-Mar-28 09:41 UTC
[RFC PATCH v2 2/3] virtio/vsock: WARN_ONCE() for invalid state of socket
On Tue, Mar 28, 2023 at 11:35?AM Arseniy Krasnov <avkrasnov at 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 at 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 at redhat.com> > > >