netdev - Re: [RFC bpf-next] bpf: add new jited info fields in bpf_dev_offload and bpf_prog

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 15 Jan 2018 15:07:25 +0100
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Jiong Wang <jiong.wang@...ronome.com>,
        Jakub Kicinski <jakub.kicinski@...ronome.com>
Cc:     alexei.starovoitov@...il.com, kafai@...com,
        oss-drivers@...ronome.com, netdev@...r.kernel.org,
        francois.theron@...ronome.com
Subject: Re: [RFC bpf-next] bpf: add new jited info fields in bpf_dev_offload
 and bpf_prog_info

On 01/15/2018 01:27 PM, Jiong Wang wrote:
>> On Fri, 12 Jan 2018 12:31:15 +0100, Daniel Borkmann wrote:
[...]
>>> Given we know when ifindex is 0, then its always host JITed and the
>>> current approach with bfd_openr() works fine, and if ifindex is non-0
>>> it is /always/ device offloaded to the one bound in the ifindex so
>>> JIT dump will be specific to that device only and never host one. Not
>>> at all opposed to extending uapi, just a question on my side to get a
>>> better understanding first wrt arch string (maybe rough but complete
>>> patch with nfp + bpftool bits might help too).
>> I was advocating for full separate set of fields, Jiong was trying for
>> a reuse, and we sort of met in the middle :)  Depends on how one looks
>> at the definition of the jited_prog_insn field, old user space is not
>> guaranteed to pay attention to ifindex.  We will drop the arch and reuse
>> host fields if that's the direction you're leaning in.
> 
> I also want to make sure adding new "jited_image" and "jited_len" is the
> correct approach.

I think it's fine.

> Was thinking to reusing exsiting fields for host jit, i.e
> bpf_prog->jited_len and bpf_prog->aux->jited_data, but different offload
> targets might have different structure for "jited_data" that we need new
> call back to parse it, also I feel keep host fields clean for host JIT
> only might help preventing code logic for host and offload interleaving
> with each other, so had gone with adding new fields.

Agree, one thing less to worry since this also ties into kallsyms, etc.

Ok, lets stick with the current RFC, they seem to be the cleanest approach
overall with having offload_arch_name[] in the uapi and only filled by
driver JITs (and not host JITs).

Thanks,
Daniel