| 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
| ||
|
Message-ID: <CACGkMEueUcag6ETNqjPeCaKAird8E9P8JutSmtyOPrGeyaMtpg@mail.gmail.com>
Date: Fri, 21 Feb 2025 08:55:30 +0800
From: Jason Wang <jasowang@...hat.com>
To: "Michael S. Tsirkin" <mst@...hat.com>
Cc: andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com,
pabeni@...hat.com, xuanzhuo@...ux.alibaba.com, eperezma@...hat.com,
virtualization@...ts.linux.dev, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next] virtio-net: tweak for better TX performance in
NAPI mode
On Fri, Feb 21, 2025 at 5:25 AM Michael S. Tsirkin <mst@...hat.com> wrote:
>
> On Tue, Feb 18, 2025 at 10:39:08AM +0800, Jason Wang wrote:
> > There are several issues existed in start_xmit():
> >
> > - Transmitted packets need to be freed before sending a packet, this
> > introduces delay and increases the average packets transmit
> > time. This also increase the time that spent in holding the TX lock.
> > - Notification is enabled after free_old_xmit_skbs() which will
> > introduce unnecessary interrupts if TX notification happens on the
> > same CPU that is doing the transmission now (actually, virtio-net
> > driver are optimized for this case).
> >
> > So this patch tries to avoid those issues by not cleaning transmitted
> > packets in start_xmit() when TX NAPI is enabled and disable
> > notifications even more aggressively. Notification will be since the
> > beginning of the start_xmit(). But we can't enable delayed
> > notification after TX is stopped as we will lose the
> > notifications. Instead, the delayed notification needs is enabled
> > after the virtqueue is kicked for best performance.
> >
> > Performance numbers:
> >
> > 1) single queue 2 vcpus guest with pktgen_sample03_burst_single_flow.sh
> > (burst 256) + testpmd (rxonly) on the host:
> >
> > - When pinning TX IRQ to pktgen VCPU: split virtqueue PPS were
> > increased 55% from 6.89 Mpps to 10.7 Mpps and 32% TX interrupts were
> > eliminated. Packed virtqueue PPS were increased 50% from 7.09 Mpps to
> > 10.7 Mpps, 99% TX interrupts were eliminated.
> >
> > - When pinning TX IRQ to VCPU other than pktgen: split virtqueue PPS
> > were increased 96% from 5.29 Mpps to 10.4 Mpps and 45% TX interrupts
> > were eliminated; Packed virtqueue PPS were increased 78% from 6.12
> > Mpps to 10.9 Mpps and 99% TX interrupts were eliminated.
> >
> > 2) single queue 1 vcpu guest + vhost-net/TAP on the host: single
> > session netperf from guest to host shows 82% improvement from
> > 31Gb/s to 58Gb/s, %stddev were reduced from 34.5% to 1.9% and 88%
> > of TX interrupts were eliminated.
> >
> > Signed-off-by: Jason Wang <jasowang@...hat.com>
>
>
> okay
>
> Acked-by: Michael S. Tsirkin <mst@...hat.com>
>
> but tell me something, would it be even better to schedule
> napi once, and have that deal with enabling notifications?
I'm not sure I will get here, if we know a NAPI is scheduled, any
reason for enabling notifications?
Thanks
>
> > ---
> > drivers/net/virtio_net.c | 45 ++++++++++++++++++++++++++++------------
> > 1 file changed, 32 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 7646ddd9bef7..ac26a6201c44 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -1088,11 +1088,10 @@ static bool is_xdp_raw_buffer_queue(struct virtnet_info *vi, int q)
> > return false;
> > }
> >
> > -static void check_sq_full_and_disable(struct virtnet_info *vi,
> > - struct net_device *dev,
> > - struct send_queue *sq)
> > +static bool tx_may_stop(struct virtnet_info *vi,
> > + struct net_device *dev,
> > + struct send_queue *sq)
> > {
> > - bool use_napi = sq->napi.weight;
> > int qnum;
> >
> > qnum = sq - vi->sq;
> > @@ -1114,6 +1113,25 @@ static void check_sq_full_and_disable(struct virtnet_info *vi,
> > u64_stats_update_begin(&sq->stats.syncp);
> > u64_stats_inc(&sq->stats.stop);
> > u64_stats_update_end(&sq->stats.syncp);
> > +
> > + return true;
> > + }
> > +
> > + return false;
> > +}
> > +
> > +static void check_sq_full_and_disable(struct virtnet_info *vi,
> > + struct net_device *dev,
> > + struct send_queue *sq)
> > +{
> > + bool use_napi = sq->napi.weight;
> > + int qnum;
> > +
> > + qnum = sq - vi->sq;
> > +
> > + if (tx_may_stop(vi, dev, sq)) {
> > + struct netdev_queue *txq = netdev_get_tx_queue(dev, qnum);
> > +
> > if (use_napi) {
> > if (unlikely(!virtqueue_enable_cb_delayed(sq->vq)))
> > virtqueue_napi_schedule(&sq->napi, sq->vq);
> > @@ -3253,15 +3271,10 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
> > bool use_napi = sq->napi.weight;
> > bool kick;
> >
> > - /* Free up any pending old buffers before queueing new ones. */
> > - do {
> > - if (use_napi)
> > - virtqueue_disable_cb(sq->vq);
> > -
> > + if (!use_napi)
> > free_old_xmit(sq, txq, false);
> > -
> > - } while (use_napi && !xmit_more &&
> > - unlikely(!virtqueue_enable_cb_delayed(sq->vq)));
> > + else
> > + virtqueue_disable_cb(sq->vq);
> >
> > /* timestamp packet in software */
> > skb_tx_timestamp(skb);
> > @@ -3287,7 +3300,10 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
> > nf_reset_ct(skb);
> > }
> >
> > - check_sq_full_and_disable(vi, dev, sq);
> > + if (use_napi)
> > + tx_may_stop(vi, dev, sq);
> > + else
> > + check_sq_full_and_disable(vi, dev,sq);
> >
> > kick = use_napi ? __netdev_tx_sent_queue(txq, skb->len, xmit_more) :
> > !xmit_more || netif_xmit_stopped(txq);
> > @@ -3299,6 +3315,9 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
> > }
> > }
> >
> > + if (use_napi && kick && unlikely(!virtqueue_enable_cb_delayed(sq->vq)))
> > + virtqueue_napi_schedule(&sq->napi, sq->vq);
> > +
> > return NETDEV_TX_OK;
> > }
> >
> > --
> > 2.34.1
>
Powered by blists - more mailing lists