On Mon, Oct 09, 2023 at 09:16:10 +0300, Andrew Melnychenko wrote:
> This series of rfc patches adds support for loading the RSS eBPF program and passing it to the QEMU.
> Comments and suggestions would be useful.
>
> QEMU with vhost may work with RSS through eBPF. To load eBPF,
> the capabilities required that Libvirt may provide.
> eBPF program and maps may be unique for particular QEMU and
> Libvirt retrieves eBPF through qapi.
> For now, there is only "RSS" eBPF object in QEMU, in the future,
> there may be another one(g.e. network filters).
> That's why in Libvirt added logic to load and store any
> eBPF object that QEMU provides using qapi schema.
>
> For virtio-net RSS, the document has not changed.
> ```
> <interface type="network">
> <model type="virtio"/>
> <driver queues="4" rss="on" rss_hash_report="off"/>
> <interface type="network">
> ```
>
> Simplified routine for RSS:
> * Libvirt retrieves eBPF "RSS" and load it.
> * Libvirt passes file descriptors to virtio-net with property "ebpf_rss_fds" ("rss" property should be "on" too).
> * if fds was provided - QEMU using eBPF RSS implementation.
> * if fds was not provided - QEMU tries to load eBPF RSS in own context and use it.
> * if eBPF RSS was not loaded - QEMU uses "in-qemu" RSS(vhost not supported).
Hi,
please note that in my review following this mail I'll be mostly
commenting about general problems, coding style problems and the
capabilies probing an caching.
I'm not familiar with what this feature is supposed to be doing as I'm
involved more with storage and I maintain the qemu capability code.
Thus input of others may be needed in terms of the actual feature.
> meson.build | 6 ++
> meson_options.txt | 1 +
> src/qemu/meson.build | 1 +
> src/qemu/qemu_capabilities.c | 181 +++++++++++++++++++++++++++++++++++
> src/qemu/qemu_capabilities.h | 4 +
> src/qemu/qemu_command.c | 53 ++++++++++
> src/qemu/qemu_domain.c | 4 +
> src/qemu/qemu_domain.h | 3 +
> src/qemu/qemu_interface.c | 42 ++++++++
> src/qemu/qemu_interface.h | 4 +
> src/qemu/qemu_monitor.c | 23 +++++
> src/qemu/qemu_monitor.h | 3 +
> src/qemu/qemu_monitor_json.c | 21 ++++
> src/qemu/qemu_monitor_json.h | 3 +
During a quick look at the patches I've noticed few repeated coding
style problems.
Note that our prefered function declaration/definition header is
virReturnType *
virFunctionName(virFirstArg argname,
virSecondArg *argval,
virEtc etc)
{
}
We also keep two line spacing between functions.
Additionally we also require C90-style variable declarations (variable
declaration must not mix with code inside a block).
Other comments inline.
