> On Apr 20, 2025, at 3:32 AM, Michael S. Tsirkin <m...@redhat.com> wrote: > > !-------------------------------------------------------------------| > CAUTION: External Email > > |-------------------------------------------------------------------! > > On Sat, Apr 19, 2025 at 06:05:18PM -0700, Jon Kohler wrote: >> In handle_tx_copy, TX batching processes packets below ~PAGE_SIZE and >> batches up to 64 messages before calling sock->sendmsg. >> >> Currently, when there are no more messages on the ring to dequeue, >> handle_tx_copy re-enables kicks on the ring *before* firing off the >> batch sendmsg. However, sock->sendmsg incurs a non-zero delay, >> especially if it needs to wake up a thread (e.g., another vhost worker). >> >> If the guest submits additional messages immediately after the last ring >> check and disablement, it triggers an EPT_MISCONFIG vmexit to attempt to >> kick the vhost worker. This may happen while the worker is still >> processing the sendmsg, leading to wasteful exit(s). >> >> This is particularly problematic for single-threaded guest submission >> threads, as they must exit, wait for the exit to be processed >> (potentially involving a TTWU), and then resume. >> >> In scenarios like a constant stream of UDP messages, this results in a >> sawtooth pattern where the submitter frequently vmexits, and the >> vhost-net worker alternates between sleeping and waking. >> >> A common solution is to configure vhost-net busy polling via userspace >> (e.g., qemu poll-us). However, treating the sendmsg as the "busy" >> period by keeping kicks disabled during the final sendmsg and >> performing one additional ring check afterward provides a significant >> performance improvement without any excess busy poll cycles. >> >> If messages are found in the ring after the final sendmsg, requeue the >> TX handler. This ensures fairness for the RX handler and allows >> vhost_run_work_list to cond_resched() as needed. >> >> Test Case >> TX VM: taskset -c 2 iperf3 -c rx-ip-here -t 60 -p 5200 -b 0 -u -i 5 >> RX VM: taskset -c 2 iperf3 -s -p 5200 -D >> 6.12.0, each worker backed by tun interface with IFF_NAPI setup. >> Note: TCP side is largely unchanged as that was copy bound >> >> 6.12.0 unpatched >> EPT_MISCONFIG/second: 5411 >> Datagrams/second: ~382k >> Interval Transfer Bitrate Lost/Total Datagrams >> 0.00-30.00 sec 15.5 GBytes 4.43 Gbits/sec 0/11481630 (0%) sender >> >> 6.12.0 patched >> EPT_MISCONFIG/second: 58 (~93x reduction) >> Datagrams/second: ~650k (~1.7x increase) >> Interval Transfer Bitrate Lost/Total Datagrams >> 0.00-30.00 sec 26.4 GBytes 7.55 Gbits/sec 0/19554720 (0%) sender >> >> Acked-by: Jason Wang <jasow...@redhat.com> >> Signed-off-by: Jon Kohler <j...@nutanix.com> > > sounds like the right approach. > > Acked-by: Michael S. Tsirkin <m...@redhat.com> > > > >> --- > > > in the future, pls put the changelog here as you progress v1->v2->v3. > Thanks!
Thanks for checking out the patch, sorry about the lack of changelog. For posterity, v2 was only sending the patch to net-next as I forgot to add the prefix on v1. v1 patch with Jason’s ack is here: https://lore.kernel.org/kvm/20250401043230.790419-1-...@nutanix.com/T/ > >> drivers/vhost/net.c | 19 +++++++++++++++---- >> 1 file changed, 15 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c >> index b9b9e9d40951..9b04025eea66 100644 >> --- a/drivers/vhost/net.c >> +++ b/drivers/vhost/net.c >> @@ -769,13 +769,17 @@ static void handle_tx_copy(struct vhost_net *net, >> struct socket *sock) >> break; >> /* Nothing new? Wait for eventfd to tell us they refilled. */ >> if (head == vq->num) { >> + /* If interrupted while doing busy polling, requeue >> + * the handler to be fair handle_rx as well as other >> + * tasks waiting on cpu >> + */ >> if (unlikely(busyloop_intr)) { >> vhost_poll_queue(&vq->poll); >> - } else if (unlikely(vhost_enable_notify(&net->dev, >> - vq))) { >> - vhost_disable_notify(&net->dev, vq); >> - continue; >> } >> + /* Kicks are disabled at this point, break loop and >> + * process any remaining batched packets. Queue will >> + * be re-enabled afterwards. >> + */ >> break; >> } >> >> @@ -825,7 +829,14 @@ static void handle_tx_copy(struct vhost_net *net, >> struct socket *sock) >> ++nvq->done_idx; >> } while (likely(!vhost_exceeds_weight(vq, ++sent_pkts, total_len))); >> >> + /* Kicks are still disabled, dispatch any remaining batched msgs. */ >> vhost_tx_batch(net, nvq, sock, &msg); >> + >> + /* All of our work has been completed; however, before leaving the >> + * TX handler, do one last check for work, and requeue handler if >> + * necessary. If there is no work, queue will be reenabled. >> + */ >> + vhost_net_busy_poll_try_queue(net, vq); >> } >> >> static void handle_tx_zerocopy(struct vhost_net *net, struct socket *sock) >> -- >> 2.43.0 >
