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

[Andrew Melnichenko]

Hi all,
Thank you for your comments. I'll check the error/warning.

On Fri, Jun 30, 2023 at 11:55 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> 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.

Need to check if it is necessary to support the eBPF on an old kernel
that doesn't support mmap for eBPF.
eBPF RSS program itself would require kernel 5.5+.
If I am not mistaken, the mmap for eBPF was added in 5.5
I'll recheck it.

>
>
> 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 :|
>