patches for DPDK stable branches
 help / color / mirror / Atom feed
* [PATCH] net/virtio: fix socket nonblocking mode affects initialization
@ 2022-06-17  2:42 Yuan Wang
  2022-06-17  3:05 ` Stephen Hemminger
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Yuan Wang @ 2022-06-17  2:42 UTC (permalink / raw)
  To: maxime.coquelin, chenbo.xia, dev
  Cc: jiayu.hu, xingguang.he, Yuan Wang, stable

The virtio-user initialization requires unix socket to receive backend
messages in block mode. However, vhost_user_update_link_state() sets
the same socket to nonblocking via fcntl, which affects all threads.
Enabling the rxq interrupt can causes both of these behaviors to occur
concurrently, with the result that the initialization may fail
because no messages are received in nonblocking socket.

Thread 1:
virtio_init_device()
--> virtio_user_start_device()
	--> vhost_user_set_memory_table()
		--> vhost_user_check_reply_ack()

Thread 2:
virtio_interrupt_handler()
--> vhost_user_update_link_state()

Fix that by replacing O_NONBLOCK with the recv per-call option
MSG_DONTWAIT.

Fixes: ef53b6030039 ("net/virtio-user: support LSC")
Cc: stable@dpdk.org

Signed-off-by: Yuan Wang <yuanx.wang@intel.com>
---
 drivers/net/virtio/virtio_user/vhost_user.c | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/drivers/net/virtio/virtio_user/vhost_user.c b/drivers/net/virtio/virtio_user/vhost_user.c
index 7d1749114d..198bd63d3c 100644
--- a/drivers/net/virtio/virtio_user/vhost_user.c
+++ b/drivers/net/virtio/virtio_user/vhost_user.c
@@ -940,15 +940,8 @@ vhost_user_update_link_state(struct virtio_user_dev *dev)
 
 	if (data->vhostfd >= 0) {
 		int r;
-		int flags;
 
-		flags = fcntl(data->vhostfd, F_GETFL);
-		if (fcntl(data->vhostfd, F_SETFL, flags | O_NONBLOCK) == -1) {
-			PMD_DRV_LOG(ERR, "error setting O_NONBLOCK flag");
-			return -1;
-		}
-
-		r = recv(data->vhostfd, buf, 128, MSG_PEEK);
+		r = recv(data->vhostfd, buf, 128, MSG_PEEK | MSG_DONTWAIT);
 		if (r == 0 || (r < 0 && errno != EAGAIN)) {
 			dev->net_status &= (~VIRTIO_NET_S_LINK_UP);
 			PMD_DRV_LOG(ERR, "virtio-user port %u is down", dev->hw.port_id);
@@ -963,12 +956,6 @@ vhost_user_update_link_state(struct virtio_user_dev *dev)
 		} else {
 			dev->net_status |= VIRTIO_NET_S_LINK_UP;
 		}
-
-		if (fcntl(data->vhostfd, F_SETFL,
-					flags & ~O_NONBLOCK) == -1) {
-			PMD_DRV_LOG(ERR, "error clearing O_NONBLOCK flag");
-			return -1;
-		}
 	} else if (dev->is_server) {
 		dev->net_status &= (~VIRTIO_NET_S_LINK_UP);
 		if (virtio_user_dev_server_reconnect(dev) >= 0)
-- 
2.25.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] net/virtio: fix socket nonblocking mode affects initialization
  2022-06-17  2:42 [PATCH] net/virtio: fix socket nonblocking mode affects initialization Yuan Wang
@ 2022-06-17  3:05 ` Stephen Hemminger
  2022-07-01 10:11 ` Xia, Chenbo
  2022-07-01 13:53 ` Maxime Coquelin
  2 siblings, 0 replies; 4+ messages in thread
From: Stephen Hemminger @ 2022-06-17  3:05 UTC (permalink / raw)
  To: Yuan Wang
  Cc: maxime.coquelin, chenbo.xia, dev, jiayu.hu, xingguang.he, stable

On Fri, 17 Jun 2022 10:42:29 +0800
Yuan Wang <yuanx.wang@intel.com> wrote:

> The virtio-user initialization requires unix socket to receive backend
> messages in block mode. However, vhost_user_update_link_state() sets
> the same socket to nonblocking via fcntl, which affects all threads.
> Enabling the rxq interrupt can causes both of these behaviors to occur
> concurrently, with the result that the initialization may fail
> because no messages are received in nonblocking socket.
> 
> Thread 1:
> virtio_init_device()
> --> virtio_user_start_device()
> 	--> vhost_user_set_memory_table()
> 		--> vhost_user_check_reply_ack()  
> 
> Thread 2:
> virtio_interrupt_handler()
> --> vhost_user_update_link_state()  
> 
> Fix that by replacing O_NONBLOCK with the recv per-call option
> MSG_DONTWAIT.
> 
> Fixes: ef53b6030039 ("net/virtio-user: support LSC")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Yuan Wang <yuanx.wang@intel.com>

Looks good and saves a few system calls.

Acked-by: Stephen Hemminger <stephen@networkplumber.org>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH] net/virtio: fix socket nonblocking mode affects initialization
  2022-06-17  2:42 [PATCH] net/virtio: fix socket nonblocking mode affects initialization Yuan Wang
  2022-06-17  3:05 ` Stephen Hemminger
@ 2022-07-01 10:11 ` Xia, Chenbo
  2022-07-01 13:53 ` Maxime Coquelin
  2 siblings, 0 replies; 4+ messages in thread
From: Xia, Chenbo @ 2022-07-01 10:11 UTC (permalink / raw)
  To: Wang, YuanX, maxime.coquelin, dev; +Cc: Hu, Jiayu, He, Xingguang, stable

> -----Original Message-----
> From: Wang, YuanX <yuanx.wang@intel.com>
> Sent: Friday, June 17, 2022 10:42 AM
> To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>;
> dev@dpdk.org
> Cc: Hu, Jiayu <jiayu.hu@intel.com>; He, Xingguang <xingguang.he@intel.com>;
> Wang, YuanX <yuanx.wang@intel.com>; stable@dpdk.org
> Subject: [PATCH] net/virtio: fix socket nonblocking mode affects
> initialization
> 
> The virtio-user initialization requires unix socket to receive backend
> messages in block mode. However, vhost_user_update_link_state() sets
> the same socket to nonblocking via fcntl, which affects all threads.
> Enabling the rxq interrupt can causes both of these behaviors to occur
> concurrently, with the result that the initialization may fail
> because no messages are received in nonblocking socket.
> 
> Thread 1:
> virtio_init_device()
> --> virtio_user_start_device()
> 	--> vhost_user_set_memory_table()
> 		--> vhost_user_check_reply_ack()
> 
> Thread 2:
> virtio_interrupt_handler()
> --> vhost_user_update_link_state()
> 
> Fix that by replacing O_NONBLOCK with the recv per-call option
> MSG_DONTWAIT.
> 
> Fixes: ef53b6030039 ("net/virtio-user: support LSC")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Yuan Wang <yuanx.wang@intel.com>
> ---
>  drivers/net/virtio/virtio_user/vhost_user.c | 15 +--------------
>  1 file changed, 1 insertion(+), 14 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_user/vhost_user.c
> b/drivers/net/virtio/virtio_user/vhost_user.c
> index 7d1749114d..198bd63d3c 100644
> --- a/drivers/net/virtio/virtio_user/vhost_user.c
> +++ b/drivers/net/virtio/virtio_user/vhost_user.c
> @@ -940,15 +940,8 @@ vhost_user_update_link_state(struct virtio_user_dev
> *dev)
> 
>  	if (data->vhostfd >= 0) {
>  		int r;
> -		int flags;
> 
> -		flags = fcntl(data->vhostfd, F_GETFL);
> -		if (fcntl(data->vhostfd, F_SETFL, flags | O_NONBLOCK) == -1) {
> -			PMD_DRV_LOG(ERR, "error setting O_NONBLOCK flag");
> -			return -1;
> -		}
> -
> -		r = recv(data->vhostfd, buf, 128, MSG_PEEK);
> +		r = recv(data->vhostfd, buf, 128, MSG_PEEK | MSG_DONTWAIT);
>  		if (r == 0 || (r < 0 && errno != EAGAIN)) {
>  			dev->net_status &= (~VIRTIO_NET_S_LINK_UP);
>  			PMD_DRV_LOG(ERR, "virtio-user port %u is down", dev-
> >hw.port_id);
> @@ -963,12 +956,6 @@ vhost_user_update_link_state(struct virtio_user_dev
> *dev)
>  		} else {
>  			dev->net_status |= VIRTIO_NET_S_LINK_UP;
>  		}
> -
> -		if (fcntl(data->vhostfd, F_SETFL,
> -					flags & ~O_NONBLOCK) == -1) {
> -			PMD_DRV_LOG(ERR, "error clearing O_NONBLOCK flag");
> -			return -1;
> -		}
>  	} else if (dev->is_server) {
>  		dev->net_status &= (~VIRTIO_NET_S_LINK_UP);
>  		if (virtio_user_dev_server_reconnect(dev) >= 0)
> --
> 2.25.1

Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] net/virtio: fix socket nonblocking mode affects initialization
  2022-06-17  2:42 [PATCH] net/virtio: fix socket nonblocking mode affects initialization Yuan Wang
  2022-06-17  3:05 ` Stephen Hemminger
  2022-07-01 10:11 ` Xia, Chenbo
@ 2022-07-01 13:53 ` Maxime Coquelin
  2 siblings, 0 replies; 4+ messages in thread
From: Maxime Coquelin @ 2022-07-01 13:53 UTC (permalink / raw)
  To: Yuan Wang, chenbo.xia, dev; +Cc: jiayu.hu, xingguang.he, stable



On 6/17/22 04:42, Yuan Wang wrote:
> The virtio-user initialization requires unix socket to receive backend
> messages in block mode. However, vhost_user_update_link_state() sets
> the same socket to nonblocking via fcntl, which affects all threads.
> Enabling the rxq interrupt can causes both of these behaviors to occur
> concurrently, with the result that the initialization may fail
> because no messages are received in nonblocking socket.
> 
> Thread 1:
> virtio_init_device()
> --> virtio_user_start_device()
> 	--> vhost_user_set_memory_table()
> 		--> vhost_user_check_reply_ack()
> 
> Thread 2:
> virtio_interrupt_handler()
> --> vhost_user_update_link_state()
> 
> Fix that by replacing O_NONBLOCK with the recv per-call option
> MSG_DONTWAIT.
> 
> Fixes: ef53b6030039 ("net/virtio-user: support LSC")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Yuan Wang <yuanx.wang@intel.com>
> ---
>   drivers/net/virtio/virtio_user/vhost_user.c | 15 +--------------
>   1 file changed, 1 insertion(+), 14 deletions(-)
> 

Applied to dpdk-next-virtio/main.

Thanks,
Maxime


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-07-01 13:54 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-17  2:42 [PATCH] net/virtio: fix socket nonblocking mode affects initialization Yuan Wang
2022-06-17  3:05 ` Stephen Hemminger
2022-07-01 10:11 ` Xia, Chenbo
2022-07-01 13:53 ` Maxime Coquelin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).