projects / pve-kernel.git / blame
| Commit | Line | Data |
|---|---|---|
| 9e94988c | 1 | From f490c15a1bd99597f97fcff35c5f528de38d5b1e Mon Sep 17 00:00:00 2001 |
| ddad99c9 FG |
2 | From: Wei Xu <wexu@redhat.com> |
| 3 | Date: Fri, 1 Dec 2017 05:10:36 -0500 | |
| 9e94988c | 4 | Subject: [PATCH 08/10] vhost: fix skb leak in handle_rx() |
| ddad99c9 FG |
5 | MIME-Version: 1.0 |
| 6 | Content-Type: text/plain; charset=UTF-8 | |
| 7 | Content-Transfer-Encoding: 8bit | |
| 8 | ||
| 9 | Matthew found a roughly 40% tcp throughput regression with commit | |
| 10 | c67df11f(vhost_net: try batch dequing from skb array) as discussed | |
| 11 | in the following thread: | |
| 12 | https://www.mail-archive.com/netdev@vger.kernel.org/msg187936.html | |
| 13 | ||
| 14 | Eventually we figured out that it was a skb leak in handle_rx() | |
| 15 | when sending packets to the VM. This usually happens when a guest | |
| 16 | can not drain out vq as fast as vhost fills in, afterwards it sets | |
| 17 | off the traffic jam and leaks skb(s) which occurs as no headcount | |
| 18 | to send on the vq from vhost side. | |
| 19 | ||
| 20 | This can be avoided by making sure we have got enough headcount | |
| 21 | before actually consuming a skb from the batched rx array while | |
| 22 | transmitting, which is simply done by moving checking the zero | |
| 23 | headcount a bit ahead. | |
| 24 | ||
| 25 | Signed-off-by: Wei Xu <wexu@redhat.com> | |
| 26 | Reported-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com> | |
| 27 | Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> | |
| 28 | --- | |
| 29 | drivers/vhost/net.c | 20 ++++++++++---------- | |
| 30 | 1 file changed, 10 insertions(+), 10 deletions(-) | |
| 31 | ||
| 32 | diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c | |
| 33 | index 1c75572f5a3f..010253847022 100644 | |
| 34 | --- a/drivers/vhost/net.c | |
| 35 | +++ b/drivers/vhost/net.c | |
| 36 | @@ -781,16 +781,6 @@ static void handle_rx(struct vhost_net *net) | |
| 37 | /* On error, stop handling until the next kick. */ | |
| 38 | if (unlikely(headcount < 0)) | |
| 39 | goto out; | |
| 40 | - if (nvq->rx_array) | |
| 41 | - msg.msg_control = vhost_net_buf_consume(&nvq->rxq); | |
| 42 | - /* On overrun, truncate and discard */ | |
| 43 | - if (unlikely(headcount > UIO_MAXIOV)) { | |
| 44 | - iov_iter_init(&msg.msg_iter, READ, vq->iov, 1, 1); | |
| 45 | - err = sock->ops->recvmsg(sock, &msg, | |
| 46 | - 1, MSG_DONTWAIT | MSG_TRUNC); | |
| 47 | - pr_debug("Discarded rx packet: len %zd\n", sock_len); | |
| 48 | - continue; | |
| 49 | - } | |
| 50 | /* OK, now we need to know about added descriptors. */ | |
| 51 | if (!headcount) { | |
| 52 | if (unlikely(vhost_enable_notify(&net->dev, vq))) { | |
| 53 | @@ -803,6 +793,16 @@ static void handle_rx(struct vhost_net *net) | |
| 54 | * they refilled. */ | |
| 55 | goto out; | |
| 56 | } | |
| 57 | + if (nvq->rx_array) | |
| 58 | + msg.msg_control = vhost_net_buf_consume(&nvq->rxq); | |
| 59 | + /* On overrun, truncate and discard */ | |
| 60 | + if (unlikely(headcount > UIO_MAXIOV)) { | |
| 61 | + iov_iter_init(&msg.msg_iter, READ, vq->iov, 1, 1); | |
| 62 | + err = sock->ops->recvmsg(sock, &msg, | |
| 63 | + 1, MSG_DONTWAIT | MSG_TRUNC); | |
| 64 | + pr_debug("Discarded rx packet: len %zd\n", sock_len); | |
| 65 | + continue; | |
| 66 | + } | |
| 67 | /* We don't need to be notified again. */ | |
| 68 | iov_iter_init(&msg.msg_iter, READ, vq->iov, in, vhost_len); | |
| 69 | fixup = msg.msg_iter; | |
| 70 | -- | |
| 71 | 2.14.2 | |
| 72 |