| Message ID | da93402d-920e-c248-a5a1-baf24b70ebee@sberdevices.ru (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fix header length on skb merging | expand |
On Sun, Mar 19, 2023 at 09:52:19PM +0300, Arseniy Krasnov wrote: >This prints WARN() and returns from stream dequeue callback when socket's >queue is empty, but 'rx_bytes' still non-zero. > >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 3c75986e16c2..c35b03adad8d 100644 >--- a/net/vmw_vsock/virtio_transport_common.c >+++ b/net/vmw_vsock/virtio_transport_common.c >@@ -388,6 +388,13 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk, > u32 free_space; > > spin_lock_bh(&vvs->rx_lock); >+ >+ if (skb_queue_empty(&vvs->rx_queue) && vvs->rx_bytes) { >+ WARN(1, "No skbuffs with non-zero 'rx_bytes'\n"); I would use WARN_ONCE, since we can't recover so we will flood the log. And you can put the condition in the first argument, I mean something like this: if (WARN_ONCE(skb_queue_empty(&vvs->rx_queue) && vvs->rx_bytes, "rx_queue is empty, but rx_bytes is non-zero\n")) { Thanks, Stefano >+ 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 >
On 20.03.2023 18:07, Stefano Garzarella wrote: > On Sun, Mar 19, 2023 at 09:52:19PM +0300, Arseniy Krasnov wrote: >> This prints WARN() and returns from stream dequeue callback when socket's >> queue is empty, but 'rx_bytes' still non-zero. >> >> 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 3c75986e16c2..c35b03adad8d 100644 >> --- a/net/vmw_vsock/virtio_transport_common.c >> +++ b/net/vmw_vsock/virtio_transport_common.c >> @@ -388,6 +388,13 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk, >> u32 free_space; >> >> spin_lock_bh(&vvs->rx_lock); >> + >> + if (skb_queue_empty(&vvs->rx_queue) && vvs->rx_bytes) { >> + WARN(1, "No skbuffs with non-zero 'rx_bytes'\n"); > > I would use WARN_ONCE, since we can't recover so we will flood the log. > > And you can put the condition in the first argument, I mean something > like this: > if (WARN_ONCE(skb_queue_empty(&vvs->rx_queue) && vvs->rx_bytes, > "rx_queue is empty, but rx_bytes is non-zero\n")) { I see, ok. > > Thanks, > Stefano > >> + 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 >> >
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c index 3c75986e16c2..c35b03adad8d 100644 --- a/net/vmw_vsock/virtio_transport_common.c +++ b/net/vmw_vsock/virtio_transport_common.c @@ -388,6 +388,13 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk, u32 free_space; spin_lock_bh(&vvs->rx_lock); + + if (skb_queue_empty(&vvs->rx_queue) && vvs->rx_bytes) { + WARN(1, "No skbuffs with non-zero 'rx_bytes'\n"); + spin_unlock_bh(&vvs->rx_lock); + return err; + } + while (total < len && !skb_queue_empty(&vvs->rx_queue)) { skb = skb_peek(&vvs->rx_queue);
This prints WARN() and returns from stream dequeue callback when socket's queue is empty, but 'rx_bytes' still non-zero. Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru> --- net/vmw_vsock/virtio_transport_common.c | 7 +++++++ 1 file changed, 7 insertions(+)