From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-f68.google.com (mail-wm1-f68.google.com [209.85.128.68]) by dpdk.org (Postfix) with ESMTP id 7646B1B108 for ; Thu, 27 Sep 2018 10:44:20 +0200 (CEST) Received: by mail-wm1-f68.google.com with SMTP id b19-v6so5002460wme.3 for ; Thu, 27 Sep 2018 01:44:20 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=6iNAFqFVod6ungu0y0F1BbGsgUAqlsCGOYoXCLevjDY=; b=KhUlssjlpiWiwzYm8OlL+TkStGTiJnGJdtEtLPzxKXBovUwq5+6Rs7HQvuOsSQHzWw l5D1dBm8N3kvLUCi0k83BGn+01NYd/co7PhMKWLulBhQNddL/EkZ56JCblsZOBOMbS6Z SRGK/ghyKW8EKGljvLtFrlkZToKMYX7Ise76Rbe11lXM6rcVhOCWjdWyiHIDs9Y3GZ1v H436qN4GaxMZ3ngqNoVhqQMorvr6u2r0IrtBCB9QeiXLFWTZMc62zMFTCmXiAs+hLS7Z YSSYesRxsGDA+eSY6f3SoRYcl9yf8xxMlXRHsSD5JnVu4BFcU0TIhWBzLgP4itX4l3Yn WVAQ== X-Gm-Message-State: ABuFfojoUeehe8B2m3md3OLoOozJXvG21vxsRFbVmsJlq2DQpX4EPIDK +hOV8OL98RohWLy0X1wyhhA= X-Google-Smtp-Source: ACcGV624yQGv1INB95TJn2kO9DZ28o2C1XPESQPzXmsJAneufdXlVoFF7L09UQRC7MX1jpgxgfKA3Q== X-Received: by 2002:a1c:a9ce:: with SMTP id s197-v6mr5196598wme.77.1538037860099; Thu, 27 Sep 2018 01:44:20 -0700 (PDT) Received: from localhost ([2a01:4b00:f419:6f00:8361:8946:ba2b:d556]) by smtp.gmail.com with ESMTPSA id 198-v6sm2738825wmm.0.2018.09.27.01.44.18 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 27 Sep 2018 01:44:18 -0700 (PDT) From: Luca Boccassi To: Ilya Maximets Cc: Maxime Coquelin , dpdk stable Date: Thu, 27 Sep 2018 09:44:00 +0100 Message-Id: <20180927084403.19646-5-bluca@debian.org> X-Mailer: git-send-email 2.18.0 In-Reply-To: <20180927084403.19646-1-bluca@debian.org> References: <20180927084403.19646-1-bluca@debian.org> Subject: [dpdk-stable] patch 'vhost-user: drop connection on message handling failures' has been queued to LTS release 16.11.9 X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 27 Sep 2018 08:44:20 -0000 Hi, FYI, your patch has been queued to LTS release 16.11.9 Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet. It will be pushed if I get no objections before 09/27/18. So please shout if anyone has objections. Also note that after the patch there's a diff of the upstream commit vs the patch applied to the branch. If the code is different (ie: not only metadata diffs), due for example to a change in context or macro names, please double check it. Thanks. Luca Boccassi --- >>From 1f663861f80ab4de51216ae5e1c0a3a88d52dd85 Mon Sep 17 00:00:00 2001 From: Ilya Maximets Date: Mon, 3 Sep 2018 13:12:24 +0300 Subject: [PATCH] vhost-user: drop connection on message handling failures [ upstream commit 0d7853a4da3bd681005ecb64ef1183c59356eeea ] There are a lot of cases where vhost-user massage handling could fail and end up in a fully not recoverable state. For example, allocation failures of shadow used ring and batched copy array are not recoverable and leads to the segmentation faults like this on the receiving/transmission path: Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x7f913fecf0 (LWP 43625)] in copy_desc_to_mbuf () at /lib/librte_vhost/virtio_net.c:760 760 batch_copy[vq->batch_copy_nb_elems].dst = This could be easily reproduced in case of low memory or big number of vhost-user ports. Fix that by propagating error to the upper layer which will end up with disconnection in case we can not report to the message sender when the error happens. Fixes: f689586bc060 ("vhost: shadow used ring update") Signed-off-by: Ilya Maximets Reviewed-by: Maxime Coquelin --- lib/librte_vhost/vhost_user.c | 48 +++++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c index 04c92ceb30..f3b286e72f 100644 --- a/lib/librte_vhost/vhost_user.c +++ b/lib/librte_vhost/vhost_user.c @@ -741,7 +741,7 @@ vhost_user_set_vring_call(struct virtio_net *dev, struct VhostUserMsg *pmsg) * In vhost-user, when we receive kick message, will test whether virtio * device is ready for packet processing. */ -static void +static int vhost_user_set_vring_kick(struct virtio_net *dev, struct VhostUserMsg *pmsg) { struct vhost_vring_file file; @@ -769,6 +769,8 @@ vhost_user_set_vring_kick(struct virtio_net *dev, struct VhostUserMsg *pmsg) if (notify_ops->new_device(dev->vid) == 0) dev->flags |= VIRTIO_DEV_RUNNING; } + + return 0; } static void @@ -847,14 +849,19 @@ vhost_user_set_vring_enable(struct virtio_net *dev, return 0; } -static void +static int vhost_user_set_protocol_features(struct virtio_net *dev, uint64_t protocol_features) { - if (protocol_features & ~VHOST_USER_PROTOCOL_FEATURES) - return; + if (protocol_features & ~VHOST_USER_PROTOCOL_FEATURES) { + RTE_LOG(ERR, VHOST_CONFIG, + "(%d) received invalid protocol features.\n", + dev->vid); + return -1; + } dev->protocol_features = protocol_features; + return 0; } static int @@ -1087,7 +1094,7 @@ vhost_user_msg_handler(int vid, int fd) send_vhost_message(fd, &msg); break; case VHOST_USER_SET_FEATURES: - vhost_user_set_features(dev, msg.payload.u64); + ret = vhost_user_set_features(dev, msg.payload.u64); break; case VHOST_USER_GET_PROTOCOL_FEATURES: @@ -1096,14 +1103,14 @@ vhost_user_msg_handler(int vid, int fd) send_vhost_message(fd, &msg); break; case VHOST_USER_SET_PROTOCOL_FEATURES: - vhost_user_set_protocol_features(dev, msg.payload.u64); + ret = vhost_user_set_protocol_features(dev, msg.payload.u64); break; case VHOST_USER_SET_OWNER: - vhost_user_set_owner(); + ret = vhost_user_set_owner(); break; case VHOST_USER_RESET_OWNER: - vhost_user_reset_owner(dev); + ret = vhost_user_reset_owner(dev); break; case VHOST_USER_SET_MEM_TABLE: @@ -1111,8 +1118,9 @@ vhost_user_msg_handler(int vid, int fd) break; case VHOST_USER_SET_LOG_BASE: - vhost_user_set_log_base(dev, &msg); - + ret = vhost_user_set_log_base(dev, &msg); + if (ret) + break; /* it needs a reply */ msg.size = sizeof(msg.payload.u64); send_vhost_message(fd, &msg); @@ -1123,23 +1131,25 @@ vhost_user_msg_handler(int vid, int fd) break; case VHOST_USER_SET_VRING_NUM: - vhost_user_set_vring_num(dev, &msg.payload.state); + ret = vhost_user_set_vring_num(dev, &msg.payload.state); break; case VHOST_USER_SET_VRING_ADDR: - vhost_user_set_vring_addr(&dev, &msg.payload.addr); + ret = vhost_user_set_vring_addr(&dev, &msg.payload.addr); break; case VHOST_USER_SET_VRING_BASE: - vhost_user_set_vring_base(dev, &msg.payload.state); + ret = vhost_user_set_vring_base(dev, &msg.payload.state); break; case VHOST_USER_GET_VRING_BASE: ret = vhost_user_get_vring_base(dev, &msg.payload.state); + if (ret) + break; msg.size = sizeof(msg.payload.state); send_vhost_message(fd, &msg); break; case VHOST_USER_SET_VRING_KICK: - vhost_user_set_vring_kick(dev, &msg); + ret = vhost_user_set_vring_kick(dev, &msg); break; case VHOST_USER_SET_VRING_CALL: vhost_user_set_vring_call(dev, &msg); @@ -1158,10 +1168,10 @@ vhost_user_msg_handler(int vid, int fd) break; case VHOST_USER_SET_VRING_ENABLE: - vhost_user_set_vring_enable(dev, &msg.payload.state); + ret = vhost_user_set_vring_enable(dev, &msg.payload.state); break; case VHOST_USER_SEND_RARP: - vhost_user_send_rarp(dev, &msg); + ret = vhost_user_send_rarp(dev, &msg); break; default: @@ -1172,5 +1182,11 @@ vhost_user_msg_handler(int vid, int fd) if (unlock_required) vhost_user_unlock_all_queue_pairs(dev); + if (ret) { + RTE_LOG(ERR, VHOST_CONFIG, + "vhost message handling failed.\n"); + return -1; + } + return 0; } -- 2.18.0 --- Diff of the applied patch vs upstream commit (please double-check if non-empty: --- --- - 2018-09-25 13:26:56.888449909 +0100 +++ 0005-vhost-user-drop-connection-on-message-handling-failu.patch 2018-09-25 13:26:56.775424702 +0100 @@ -1,8 +1,10 @@ -From 0d7853a4da3bd681005ecb64ef1183c59356eeea Mon Sep 17 00:00:00 2001 +From 1f663861f80ab4de51216ae5e1c0a3a88d52dd85 Mon Sep 17 00:00:00 2001 From: Ilya Maximets Date: Mon, 3 Sep 2018 13:12:24 +0300 Subject: [PATCH] vhost-user: drop connection on message handling failures +[ upstream commit 0d7853a4da3bd681005ecb64ef1183c59356eeea ] + There are a lot of cases where vhost-user massage handling could fail and end up in a fully not recoverable state. For example, allocation failures of shadow used ring and batched @@ -22,46 +24,37 @@ the message sender when the error happens. Fixes: f689586bc060 ("vhost: shadow used ring update") -Cc: stable@dpdk.org Signed-off-by: Ilya Maximets Reviewed-by: Maxime Coquelin --- - lib/librte_vhost/vhost_user.c | 51 +++++++++++++++++++++-------------- - 1 file changed, 31 insertions(+), 20 deletions(-) + lib/librte_vhost/vhost_user.c | 48 +++++++++++++++++++++++------------ + 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c -index 9aa1ce1181..63d145b2d6 100644 +index 04c92ceb30..f3b286e72f 100644 --- a/lib/librte_vhost/vhost_user.c +++ b/lib/librte_vhost/vhost_user.c -@@ -1014,7 +1014,7 @@ vhost_user_set_vring_call(struct virtio_net *dev, struct VhostUserMsg *pmsg) - vq->callfd = file.fd; - } - +@@ -741,7 +741,7 @@ vhost_user_set_vring_call(struct virtio_net *dev, struct VhostUserMsg *pmsg) + * In vhost-user, when we receive kick message, will test whether virtio + * device is ready for packet processing. + */ -static void +static int - vhost_user_set_vring_kick(struct virtio_net **pdev, struct VhostUserMsg *pmsg) + vhost_user_set_vring_kick(struct virtio_net *dev, struct VhostUserMsg *pmsg) { struct vhost_vring_file file; -@@ -1032,7 +1032,7 @@ vhost_user_set_vring_kick(struct virtio_net **pdev, struct VhostUserMsg *pmsg) - /* Interpret ring addresses only when ring is started. */ - dev = translate_ring_addresses(dev, file.index); - if (!dev) -- return; -+ return -1; - - *pdev = dev; - -@@ -1049,6 +1049,7 @@ vhost_user_set_vring_kick(struct virtio_net **pdev, struct VhostUserMsg *pmsg) - if (vq->kickfd >= 0) - close(vq->kickfd); - vq->kickfd = file.fd; +@@ -769,6 +769,8 @@ vhost_user_set_vring_kick(struct virtio_net *dev, struct VhostUserMsg *pmsg) + if (notify_ops->new_device(dev->vid) == 0) + dev->flags |= VIRTIO_DEV_RUNNING; + } ++ + return 0; } static void -@@ -1172,14 +1173,19 @@ vhost_user_get_protocol_features(struct virtio_net *dev, - msg->size = sizeof(msg->payload.u64); +@@ -847,14 +849,19 @@ vhost_user_set_vring_enable(struct virtio_net *dev, + return 0; } -static void @@ -83,17 +76,17 @@ } static int -@@ -1657,8 +1663,6 @@ vhost_user_msg_handler(int vid, int fd) +@@ -1087,7 +1094,7 @@ vhost_user_msg_handler(int vid, int fd) + send_vhost_message(fd, &msg); break; case VHOST_USER_SET_FEATURES: - ret = vhost_user_set_features(dev, msg.payload.u64); -- if (ret) -- return -1; +- vhost_user_set_features(dev, msg.payload.u64); ++ ret = vhost_user_set_features(dev, msg.payload.u64); break; case VHOST_USER_GET_PROTOCOL_FEATURES: -@@ -1666,14 +1670,14 @@ vhost_user_msg_handler(int vid, int fd) - send_vhost_reply(fd, &msg); +@@ -1096,14 +1103,14 @@ vhost_user_msg_handler(int vid, int fd) + send_vhost_message(fd, &msg); break; case VHOST_USER_SET_PROTOCOL_FEATURES: - vhost_user_set_protocol_features(dev, msg.payload.u64); @@ -110,7 +103,7 @@ break; case VHOST_USER_SET_MEM_TABLE: -@@ -1681,8 +1685,9 @@ vhost_user_msg_handler(int vid, int fd) +@@ -1111,8 +1118,9 @@ vhost_user_msg_handler(int vid, int fd) break; case VHOST_USER_SET_LOG_BASE: @@ -118,74 +111,65 @@ - + ret = vhost_user_set_log_base(dev, &msg); + if (ret) -+ goto skip_to_reply; ++ break; /* it needs a reply */ msg.size = sizeof(msg.payload.u64); - send_vhost_reply(fd, &msg); -@@ -1693,23 +1698,25 @@ vhost_user_msg_handler(int vid, int fd) + send_vhost_message(fd, &msg); +@@ -1123,23 +1131,25 @@ vhost_user_msg_handler(int vid, int fd) break; case VHOST_USER_SET_VRING_NUM: -- vhost_user_set_vring_num(dev, &msg); -+ ret = vhost_user_set_vring_num(dev, &msg); +- vhost_user_set_vring_num(dev, &msg.payload.state); ++ ret = vhost_user_set_vring_num(dev, &msg.payload.state); break; case VHOST_USER_SET_VRING_ADDR: -- vhost_user_set_vring_addr(&dev, &msg); -+ ret = vhost_user_set_vring_addr(&dev, &msg); +- vhost_user_set_vring_addr(&dev, &msg.payload.addr); ++ ret = vhost_user_set_vring_addr(&dev, &msg.payload.addr); break; case VHOST_USER_SET_VRING_BASE: -- vhost_user_set_vring_base(dev, &msg); -+ ret = vhost_user_set_vring_base(dev, &msg); +- vhost_user_set_vring_base(dev, &msg.payload.state); ++ ret = vhost_user_set_vring_base(dev, &msg.payload.state); break; case VHOST_USER_GET_VRING_BASE: -- vhost_user_get_vring_base(dev, &msg); -+ ret = vhost_user_get_vring_base(dev, &msg); + ret = vhost_user_get_vring_base(dev, &msg.payload.state); + if (ret) -+ goto skip_to_reply; ++ break; msg.size = sizeof(msg.payload.state); - send_vhost_reply(fd, &msg); + send_vhost_message(fd, &msg); break; case VHOST_USER_SET_VRING_KICK: -- vhost_user_set_vring_kick(&dev, &msg); -+ ret = vhost_user_set_vring_kick(&dev, &msg); +- vhost_user_set_vring_kick(dev, &msg); ++ ret = vhost_user_set_vring_kick(dev, &msg); break; case VHOST_USER_SET_VRING_CALL: vhost_user_set_vring_call(dev, &msg); -@@ -1728,10 +1735,10 @@ vhost_user_msg_handler(int vid, int fd) +@@ -1158,10 +1168,10 @@ vhost_user_msg_handler(int vid, int fd) break; case VHOST_USER_SET_VRING_ENABLE: -- vhost_user_set_vring_enable(dev, &msg); -+ ret = vhost_user_set_vring_enable(dev, &msg); +- vhost_user_set_vring_enable(dev, &msg.payload.state); ++ ret = vhost_user_set_vring_enable(dev, &msg.payload.state); break; case VHOST_USER_SEND_RARP: - vhost_user_send_rarp(dev, &msg); + ret = vhost_user_send_rarp(dev, &msg); break; - case VHOST_USER_NET_SET_MTU: -@@ -1752,7 +1759,7 @@ vhost_user_msg_handler(int vid, int fd) - } + default: +@@ -1172,5 +1182,11 @@ vhost_user_msg_handler(int vid, int fd) + if (unlock_required) + vhost_user_unlock_all_queue_pairs(dev); - skip_to_post_handle: -- if (dev->extern_ops.post_msg_handle) { -+ if (!ret && dev->extern_ops.post_msg_handle) { - uint32_t need_reply; - - ret = (*dev->extern_ops.post_msg_handle)( -@@ -1772,6 +1779,10 @@ skip_to_reply: - msg.payload.u64 = !!ret; - msg.size = sizeof(msg.payload.u64); - send_vhost_reply(fd, &msg); -+ } else if (ret) { ++ if (ret) { + RTE_LOG(ERR, VHOST_CONFIG, + "vhost message handling failed.\n"); + return -1; - } - - if (!(dev->flags & VIRTIO_DEV_RUNNING) && virtio_is_ready(dev)) { ++ } ++ + return 0; + } -- 2.18.0