lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 2 Aug 2018 17:41:41 +0900 From: Toshiaki Makita <makita.toshiaki@....ntt.co.jp> To: Jason Wang <jasowang@...hat.com>, Tonghao Zhang <xiangxia.m.yue@...il.com> Cc: mst@...hat.com, virtualization@...ts.linux-foundation.org, Linux Kernel Network Developers <netdev@...r.kernel.org> Subject: Re: [PATCH net-next v7 3/4] net: vhost: factor out busy polling logic to vhost_net_busy_poll() On 2018/08/02 17:18, Jason Wang wrote: > On 2018年08月01日 17:52, Tonghao Zhang wrote: >>> +static void vhost_net_busy_poll_check(struct vhost_net *net, >>> + struct vhost_virtqueue *rvq, >>> + struct vhost_virtqueue *tvq, >>> + bool rx) >>> +{ >>> + struct socket *sock = rvq->private_data; >>> + >>> + if (rx) >>> + vhost_net_busy_poll_try_queue(net, tvq); >>> + else if (sock && sk_has_rx_data(sock->sk)) >>> + vhost_net_busy_poll_try_queue(net, rvq); >>> + else { >>> + /* On tx here, sock has no rx data, so we >>> + * will wait for sock wakeup for rx, and >>> + * vhost_enable_notify() is not needed. */ >> >> A possible case is we do have rx data but guest does not refill the rx >> queue. In this case we may lose notifications from guest. > Yes, should consider this case. thanks. I'm a bit confused. Isn't this covered by the previous "else if (sock && sk_has_rx_data(...))" block? >>>> + >>>> + cpu_relax(); >>>> + } >>>> + >>>> + preempt_enable(); >>>> + >>>> + if (!rx) >>>> + vhost_net_enable_vq(net, vq); >>> No need to enable rx virtqueue, if we are sure handle_rx() will be >>> called soon. >> If we disable rx virtqueue in handle_tx and don't send packets from >> guest anymore(handle_tx is not called), so we can wake up for sock rx. >> so the network is broken. > > Not sure I understand here. I mean is we schedule work for handle_rx(), > there's no need to enable it since handle_rx() will do this for us. Looks like in the last "else" block in vhost_net_busy_poll_check() we need to enable vq since in that case we have no rx data and handle_rx() is not scheduled. -- Toshiaki Makita
Powered by blists - more mailing lists