Re: [patch net-next] virtio_net: add support for Byte Queue Limits

Jason Wang
2024-06-06T07:57:11Z

On Thu, Jun 6, 2024 at 2:05 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Thu, Jun 06, 2024 at 12:25:15PM +0800, Jason Wang wrote:
> > > If the codes of orphan mode don't have an impact when you enable
> > > napi_tx mode, please keep it if you can.
> >
> > For example, it complicates BQL implementation.
> >
> > Thanks
>
> I very much doubt sending interrupts to a VM can
> *on all benchmarks* compete with not sending interrupts.

It should not differ too much from the physical NIC. We can have one
more round of benchmarks to see the difference.

But if NAPI mode needs to win all of the benchmarks in order to get
rid of orphan, that would be very difficult. Considering various bugs
will be fixed by dropping skb_orphan(), it would be sufficient if most
of the benchmark doesn't show obvious differences.

Looking at git history, there're commits that removes skb_orphan(), for example:

commit 8112ec3b8722680251aecdcc23dfd81aa7af6340
Author: Eric Dumazet <edumazet@google.com>
Date: Fri Sep 28 07:53:26 2012 +0000

mlx4: dont orphan skbs in mlx4_en_xmit()

After commit e22979d96a55d (mlx4_en: Moving to Interrupts for TX
completions) we no longer need to orphan skbs in mlx4_en_xmit()
since skb wont stay a long time in TX ring before their release.

Orphaning skbs in ndo_start_xmit() should be avoided as much as
possible, since it breaks TCP Small Queue or other flow control
mechanisms (per socket limits)

Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Yevgeny Petrilin <yevgenyp@mellanox.com>
Cc: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

>
> So yea, it's great if napi and hardware are advanced enough
> that the default can be changed, since this way virtio
> is closer to a regular nic and more or standard
> infrastructure can be used.
>
> But dropping it will go against *no breaking userspace* rule.
> Complicated? Tough.

I don't know what kind of userspace is broken by this. Or why it is
not broken since the day we enable NAPI mode by default.

Thanks

>
> --
> MST
>