[RFC PATCH 0/4] Added virtio-net RSS with eBPF support.

Andrew Melnychenko posted 4 patches 6 months, 3 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20231009061616.1663330-1-andrew@daynix.com
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 +
14 files changed, 349 insertions(+)
[RFC PATCH 0/4] Added virtio-net RSS with eBPF support.
Posted by Andrew Melnychenko 6 months, 3 weeks ago
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).

 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 +
 14 files changed, 349 insertions(+)

-- 
2.42.0
Re: [RFC PATCH 0/4] Added virtio-net RSS with eBPF support.
Posted by Peter Krempa 6 months, 3 weeks ago
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.
Re: [RFC PATCH 0/4] Added virtio-net RSS with eBPF support.
Posted by Peter Krempa 6 months, 3 weeks ago
On Mon, Oct 09, 2023 at 10:15:34 +0200, Peter Krempa wrote:
> 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,
> 

Also what is the status of the qemu patches, specifically the QMP
interface? I see that the patch was in v1 of Jasons 'Net patches' pull
request in September, but was dropped in v2 for some reason.

Specifically our policy is that a feature depending on new qemu
interfaces can be merged to libvirt only after the code was comitted to
qemu.

We do not require that it is released, in fact we follow the development
cycle with the the "qemu capabilities" updates done throughout the the
dev cycle.

Once the qemu code is merged I can do the required update of the
capability test output (tests/qemucapabilitiesdata/...replies) output
file based on your patch so that churn and your manual input is
minimized.
Re: [RFC PATCH 0/4] Added virtio-net RSS with eBPF support.
Posted by Andrew Melnichenko 6 months, 3 weeks ago
Hi all, thank you for your comments.

On Mon, Oct 9, 2023 at 11:22 AM Peter Krempa <pkrempa@redhat.com> wrote:
>
> On Mon, Oct 09, 2023 at 10:15:34 +0200, Peter Krempa wrote:
> > 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,
> >
>
> Also what is the status of the qemu patches, specifically the QMP
> interface? I see that the patch was in v1 of Jasons 'Net patches' pull
> request in September, but was dropped in v2 for some reason.
>

As I know ebpf qmp patches was in queue
(https://lists.gnu.org/archive/html/qemu-devel/2023-08/msg05859.html).
I'll recheck it later.

> Specifically our policy is that a feature depending on new qemu
> interfaces can be merged to libvirt only after the code was comitted to
> qemu.
>
> We do not require that it is released, in fact we follow the development
> cycle with the the "qemu capabilities" updates done throughout the the
> dev cycle.
>
> Once the qemu code is merged I can do the required update of the
> capability test output (tests/qemucapabilitiesdata/...replies) output
> file based on your patch so that churn and your manual input is
> minimized.
>

Yes, these are RFC patches, so when QEMU applies qmp ebpf - the
Libvirt solution should be properly ready.