Skip to content

Commit

Permalink
vsock/virtio: discard packets only when socket is really closed
Browse files Browse the repository at this point in the history
Starting from commit 8692cef ("virtio_vsock: Fix race condition
in virtio_transport_recv_pkt"), we discard packets in
virtio_transport_recv_pkt() if the socket has been released.

When the socket is connected, we schedule a delayed work to wait the
RST packet from the other peer, also if SHUTDOWN_MASK is set in
sk->sk_shutdown.
This is done to complete the virtio-vsock shutdown algorithm, releasing
the port assigned to the socket definitively only when the other peer
has consumed all the packets.

If we discard the RST packet received, the socket will be closed only
when the VSOCK_CLOSE_TIMEOUT is reached.

Sergio discovered the issue while running ab(1) HTTP benchmark using
libkrun [1] and observing a latency increase with that commit.

To avoid this issue, we discard packet only if the socket is really
closed (SOCK_DONE flag is set).
We also set SOCK_DONE in virtio_transport_release() when we don't need
to wait any packets from the other peer (we didn't schedule the delayed
work). In this case we remove the socket from the vsock lists, releasing
the port assigned.

[1] https://github.com/containers/libkrun

Fixes: 8692cef ("virtio_vsock: Fix race condition in virtio_transport_recv_pkt")
Cc: [email protected]
Reported-by: Sergio Lopez <[email protected]>
Tested-by: Sergio Lopez <[email protected]>
Signed-off-by: Stefano Garzarella <[email protected]>
Acked-by: Jia He <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Jakub Kicinski <[email protected]>
  • Loading branch information
stefano-garzarella authored and kuba-moo committed Nov 24, 2020
1 parent 01770a1 commit 3fe356d
Showing 1 changed file with 5 additions and 3 deletions.
8 changes: 5 additions & 3 deletions net/vmw_vsock/virtio_transport_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -841,8 +841,10 @@ void virtio_transport_release(struct vsock_sock *vsk)
virtio_transport_free_pkt(pkt);
}

if (remove_sock)
if (remove_sock) {
sock_set_flag(sk, SOCK_DONE);
vsock_remove_sock(vsk);
}
}
EXPORT_SYMBOL_GPL(virtio_transport_release);

Expand Down Expand Up @@ -1132,8 +1134,8 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,

lock_sock(sk);

/* Check if sk has been released before lock_sock */
if (sk->sk_shutdown == SHUTDOWN_MASK) {
/* Check if sk has been closed before lock_sock */
if (sock_flag(sk, SOCK_DONE)) {
(void)virtio_transport_reset_no_sock(t, pkt);
release_sock(sk);
sock_put(sk);
Expand Down

0 comments on commit 3fe356d

Please sign in to comment.