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

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Fri, 7 Jun 2024 08:40:16 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: Jason Wang <jasowang@...hat.com>
Cc: Jason Xing <kerneljasonxing@...il.com>,
	Heng Qi <hengqi@...ux.alibaba.com>, davem@...emloft.net,
	edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
	mst@...hat.com, xuanzhuo@...ux.alibaba.com,
	virtualization@...ts.linux.dev, ast@...nel.org,
	daniel@...earbox.net, hawk@...nel.org, john.fastabend@...il.com,
	netdev@...r.kernel.org
Subject: Re: [patch net-next] virtio_net: add support for Byte Queue Limits

Fri, Jun 07, 2024 at 08:22:31AM CEST, jasowang@...hat.com wrote:
>On Thu, Jun 6, 2024 at 9:41 PM Jiri Pirko <jiri@...nulli.us> wrote:
>>
>> Thu, Jun 06, 2024 at 06:25:15AM CEST, jasowang@...hat.com wrote:
>> >On Thu, Jun 6, 2024 at 10:59 AM Jason Xing <kerneljasonxing@...il.com> wrote:
>> >>
>> >> Hello Jason,
>> >>
>> >> On Thu, Jun 6, 2024 at 8:21 AM Jason Wang <jasowang@...hat.com> wrote:
>> >> >
>> >> > On Wed, Jun 5, 2024 at 7:51 PM Heng Qi <hengqi@...ux.alibaba.com> wrote:
>> >> > >
>> >> > > On Wed, 5 Jun 2024 13:30:51 +0200, Jiri Pirko <jiri@...nulli.us> wrote:
>> >> > > > Mon, May 20, 2024 at 02:48:15PM CEST, jiri@...nulli.us wrote:
>> >> > > > >Fri, May 10, 2024 at 09:11:16AM CEST, hengqi@...ux.alibaba.com wrote:
>> >> > > > >>On Thu,  9 May 2024 13:46:15 +0200, Jiri Pirko <jiri@...nulli.us> wrote:
>> >> > > > >>> From: Jiri Pirko <jiri@...dia.com>
>> >> > > > >>>
>> >> > > > >>> Add support for Byte Queue Limits (BQL).
>> >> > > > >>
>> >> > > > >>Historically both Jason and Michael have attempted to support BQL
>> >> > > > >>for virtio-net, for example:
>> >> > > > >>
>> >> > > > >>https://lore.kernel.org/netdev/21384cb5-99a6-7431-1039-b356521e1bc3@redhat.com/
>> >> > > > >>
>> >> > > > >>These discussions focus primarily on:
>> >> > > > >>
>> >> > > > >>1. BQL is based on napi tx. Therefore, the transfer of statistical information
>> >> > > > >>needs to rely on the judgment of use_napi. When the napi mode is switched to
>> >> > > > >>orphan, some statistical information will be lost, resulting in temporary
>> >> > > > >>inaccuracy in BQL.
>> >> > > > >>
>> >> > > > >>2. If tx dim is supported, orphan mode may be removed and tx irq will be more
>> >> > > > >>reasonable. This provides good support for BQL.
>> >> > > > >
>> >> > > > >But when the device does not support dim, the orphan mode is still
>> >> > > > >needed, isn't it?
>> >> > > >
>> >> > > > Heng, is my assuption correct here? Thanks!
>> >> > > >
>> >> > >
>> >> > > Maybe, according to our cloud data, napi_tx=on works better than orphan mode in
>> >> > > most scenarios. Although orphan mode performs better in specific benckmark,
>> >> >
>> >> > For example pktgen (I meant even if the orphan mode can break pktgen,
>> >> > it can finish when there's a new packet that needs to be sent after
>> >> > pktgen is completed).
>> >> >
>> >> > > perf of napi_tx can be enhanced through tx dim. Then, there is no reason not to
>> >> > > support dim for devices that want the best performance.
>> >> >
>> >> > Ideally, if we can drop orphan mode, everything would be simplified.
>> >>
>> >> Please please don't do this. Orphan mode still has its merits. In some
>> >> cases which can hardly be reproduced in production, we still choose to
>> >> turn off the napi_tx mode because the delay of freeing a skb could
>> >> cause lower performance in the tx path,
>> >
>> >Well, it's probably just a side effect and it depends on how to define
>> >performance here.
>> >
>> >> which is, I know, surely
>> >> designed on purpose.
>> >
>> >I don't think so and no modern NIC uses that. It breaks a lot of things.
>> >
>> >>
>> >> 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.
>>
>> Well, bql could be disabled when napi is not used. It is just a matter
>> of one "if" in the xmit path.
>
>Maybe, care to post a patch?
>
>The trick part is, a skb is queued when BQL is enabled but sent when
>BQL is disabled as discussed here:
>
>https://lore.kernel.org/netdev/21384cb5-99a6-7431-1039-b356521e1bc3@redhat.com/
>
>Thanks

Will try to go in orphan removal direction first.

>
>>
>>
>> >
>> >Thanks
>> >
>> >>
>> >> Thank you.
>> >>
>> >
>>
>