Re: [PATCH v3 0/6] eBPF RSS through QMP support.

[Daniel P . Berrangé]

On Fri, Jun 30, 2023 at 04:21:02PM +0800, Jason Wang wrote:
> On Fri, Jun 30, 2023 at 4:04 PM Daniel P. Berrangé <berrange@redhat.com> 
> wrote:
> >
> > On Fri, Jun 30, 2023 at 01:06:22PM +0800, Jason Wang wrote:
> > > On Thu, Jun 15, 2023 at 6:29 AM Andrew Melnychenko <andrew@daynix.com> 
> > > wrote:
> > > >
> > > > This series of patches provides the ability to retrieve eBPF program
> > > > through qmp, so management application may load bpf blob with proper 
> > > > capabilities.
> > > > Now, virtio-net devices can accept eBPF programs and maps through 
> > > > properties
> > > > as external file descriptors. Access to the eBPF map is direct through 
> > > > mmap()
> > > > call, so it should not require additional capabilities to bpf* calls.
> > > > eBPF file descriptors can be passed to QEMU from parent process or by 
> > > > unix
> > > > socket with sendfd() qmp command.
> > > >
> > > > Possible solution for libvirt may look like this: 
> > > > https://github.com/daynix/libvirt/tree/RSS_eBPF (WIP)
> > > >
> > > > Changes since v2:
> > > >  * moved/refactored QMP command
> > > >  * refactored virtio-net
> > >
> > > I've queued this series, but a question left:
> > >
> > > mmap() support for eBPF maps is not supported from day0, should we
> > > fallback to syscall for the OS that doesn't support that?
> >
> > How recent is mmap() support ?
> 
> I don't check.
> 
> > Is it difficult to do a fallback ?
> 
> Nope, but it requires privilege if we go with a syscall.
> 
> >
> > As since is a new feature,
> 
> But it modifies the old rss loading code, no?
> 
> -    for (; i < len; ++i) {
> -        if (bpf_map_update_elem(ctx->map_indirections_table, &i,
> -                                indirections_table + i, 0) < 0) {
> -            return false;
> -        }
> -    }
> +    memcpy(ctx->mmap_indirections_table, indirections_table,
> +            sizeof(*indirections_table) * len);
> 
> > there's no inherant expectation of support
> > for arbitrary old platforms. So only worth investing in a fallback if
> > it is easy, or there's a very compelling reason to support certain
> > old platforms.
> 
> The reason is that we support eBPF RSS with syscall based map updating
> in the past if Qemu was running with privilege. With this series, it
> won't work if the kernel doesn't support mmap.

Oh right, yes, I missed that aspect. So yeah, we do need fallback.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|