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:   Fri, 12 Jan 2018 14:17:53 -0800
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Daniel Borkmann <daniel@...earbox.net>
Cc:     alexei.starovoitov@...il.com, kafai@...com,
        oss-drivers@...ronome.com, netdev@...r.kernel.org,
        francois.theron@...ronome.com,
        Jiong Wang <jiong.wang@...ronome.com>
Subject: Re: [RFC bpf-next] bpf: add new jited info fields in
 bpf_dev_offload and bpf_prog_info

On Fri, 12 Jan 2018 12:31:15 +0100, Daniel Borkmann wrote:
> On 01/12/2018 03:17 AM, Jakub Kicinski wrote:
> > On Thu, 11 Jan 2018 16:47:47 -0800, Jakub Kicinski wrote:  
> >> Hi!
> >>
> >> Jiong is working on dumping JITed NFP image via bpftool, Francois will be
> >> submitting support for NFP in binutils soon (whoop! :)).
> >>
> >> We would appreciate if you could weigh in on the uAPI.  Is it OK to reuse
> >> the existing jited_prog_len/jited_prog_insns or should we add separate
> >> 2 new fields (plus the arch name) to avoid confusing old user space?  
> > 
> > Ah, I skipped one chunk of Jiong's patch here, this would also be
> > necessary if we reuse fields:  
>
> What kind of string would sit in jited_arch_name for nfp? Given you know
> the {ifindex, netns_dev, netns_ino} 3‑tuple, isn't it possible to infer
> the driver info from ethtool (e.g. nfp_net_get_drvinfo()) already and do
> the mapping for binutils? 

Right, the inference can certainly work.  Probably by matching the PCI
ID of the device?  Or we can just assume it's the NFP since there is
only one upstream BPF offload :)

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