Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
MIME-Version: 1.0
References: <20220518205924.399291-1-benjamin.tissoires@redhat.com>
 <YoX7iHddAd4FkQRQ@infradead.org> <YoX904CAFOAfWeJN@kroah.com>
 <YoYCIhYhzLmhIGxe@infradead.org> <CAO-hwJL4Pj4JaRquoXD1AtegcKnh22_T0Z0VY_peZ8FRko3kZw@mail.gmail.com>
 <87ee0p951b.fsf@toke.dk> <CAO-hwJKwj6H0Nc_gqsN5okT2ipLL3H6fqe23_vpO+xC3PnX5uw@mail.gmail.com>
In-Reply-To: <CAO-hwJKwj6H0Nc_gqsN5okT2ipLL3H6fqe23_vpO+xC3PnX5uw@mail.gmail.com>
From:   Alexei Starovoitov <alexei.starovoitov@gmail.com>
Date:   Fri, 20 May 2022 17:18:22 -0700
Message-ID: <CAADnVQ+Qj9Farf-bp4STpe0P+=7Xr2Hqxm5Tru5bLCyPiBJMqg@mail.gmail.com>
Subject: Re: [PATCH bpf-next v5 00/17] Introduce eBPF support for HID devices
To:     Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc:     =?UTF-8?B?VG9rZSBIw7hpbGFuZC1Kw7hyZ2Vuc2Vu?= <toke@redhat.com>,
        Christoph Hellwig <hch@infradead.org>,
        Greg KH <gregkh@linuxfoundation.org>,
        Jiri Kosina <jikos@kernel.org>,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>,
        Martin KaFai Lau <kafai@fb.com>,
        Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@kernel.org>, Shuah Khan <shuah@kernel.org>,
        Dave Marchevsky <davemarchevsky@fb.com>,
        Joe Stringer <joe@cilium.io>, Jonathan Corbet <corbet@lwn.net>,
        Tero Kristo <tero.kristo@linux.intel.com>,
        lkml <linux-kernel@vger.kernel.org>,
        "open list:HID CORE LAYER" <linux-input@vger.kernel.org>,
        Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>,
        "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@vger.kernel.org>,
        Linux Doc Mailing List <linux-doc@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, May 19, 2022 at 4:56 AM Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
> As Greg mentioned in his reply, report descriptors fixups don't do
> much besides changing a memory buffer at probe time. So we can either
> have udev load the program, pin it and forget about it, or we can also
> have the kernel do that for us.
>
> So I envision the distribution to be hybrid:
> - for plain fixups where no userspace is required, we should
> distribute those programs in the kernel itself, in-tree.
> This series already implements pre-loading of BPF programs for the
> core part of HID-BPF, but I plan on working on some automation of
> pre-loading of these programs from the kernel itself when we need to
> do so.
>
> Ideally, the process would be:
> * user reports a bug
> * developer produces an eBPF program (and maybe compile it if the user
> doesn't have LLVM)
> * user tests/validates the fix without having to recompile anything
> * developer drops the program in-tree
> * some automated magic happens (still unclear exactly how to define
> which HID device needs which eBPF program ATM)
> * when the kernel sees this exact same device (BUS/VID/PID/INTERFACE)
> it loads the fixup
>
> - the other part of the hybrid solution is for when userspace is
> heavily involved (because it exports a new dbus interface for that
> particular feature on this device). We can not really automatically
> preload the BPF program because we might not have the user in front of
> it.
> So in that case, the program would be hosted alongside the
> application, out-of-the-tree, but given that to be able to call kernel
> functions you need to be GPL, some public distribution of the sources
> is required.

Agree with everything you've said earlier.
Just one additional comment:
By default the source code is embedded in bpf objects.
Here is an example.
$ bpftool prog dump jited id 3927008|head -50
void cwnd_event(long long unsigned int * ctx):
bpf_prog_9b9adc0a36a25303_cwnd_event:
; void BPF_STRUCT_OPS(cwnd_event, struct sock* sk, enum tcp_ca_event ev) {
   0:    nopl   0x0(%rax,%rax,1)
   5:    xchg   %ax,%ax
...
; switch (ev) {
  25:    mov    %r14d,%edi
  28:    add    $0xfffffffc,%edi
...
; ca->loss_cwnd = tp->snd_cwnd;
  4a:    mov    %edi,0x18(%r13)
  4e:    mov    $0x2,%edi
; tp->snd_ssthresh = max(tp->snd_cwnd >> 1U, 2U);
  53:    test   %rbx,%rbx
  56:    jne    0x000000000000005c

It's not the full source, of course, but good enough in practice
for a person to figure out what program is doing.