netdev - Re: [RFC 05/12] nfp: add BPF to NFP code translator

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:	Wed, 1 Jun 2016 13:15:48 -0700
From:	Alexei Starovoitov <alexei.starovoitov@...il.com>
To:	Daniel Borkmann <daniel@...earbox.net>
Cc:	Jakub Kicinski <jakub.kicinski@...ronome.com>,
	netdev@...r.kernel.org, ast@...nel.org,
	dinan.gunawardena@...ronome.com
Subject: Re: [RFC 05/12] nfp: add BPF to NFP code translator

On Wed, Jun 01, 2016 at 10:03:04PM +0200, Daniel Borkmann wrote:
> On 06/01/2016 06:50 PM, Jakub Kicinski wrote:
> >Add translator for JITing eBPF to operations which
> >can be executed on NFP's programmable engines.
> >
> >Signed-off-by: Jakub Kicinski <jakub.kicinski@...ronome.com>
> >Reviewed-by: Dinan Gunawardena <dgunawardena@...ronome.com>
> >Reviewed-by: Simon Horman <simon.horman@...ronome.com>
> [...]
> >+int
> >+nfp_bpf_jit(struct bpf_prog *filter, void *prog_mem, unsigned int prog_start,
> >+	    unsigned int tgt_out, unsigned int tgt_abort,
> >+	    unsigned int prog_sz, struct nfp_bpf_result *res)
> >+{
> >+	struct nfp_prog *nfp_prog;
> >+	int ret;
> >+
> >+	/* TODO: maybe make this dependent on bpf_jit_enable? */
> 
> Probably makes sense to leave it independent from this.
> 
> Maybe that would rather be an ethtool flag/setting?

Agree that it should be independent of bpf_jit_enable,
since that's very different JIT. The whole point of hw offload
is that bpf is translated into something hw understand natively.
Gating it by sysctl or another flag doesn't make much sense to me.
In this case the user will say 'do offload tc+cls_bpf into a nic'
and nic should either do it or not. No need for ethtool flag either.
One can argue that that bpf_jit_enable=2 was useful for debugging
of JIT itself, but looks like it was only used by jit developers
like us, but we would be fine with temp printk while debugging.
At least there was never a case where jit had a bug and we would
ask a person reporting a bug to send us back jit_enable=2 output.
We cannot remove it now, but I wouldn't simply copy the behavior here.
So I'm suggesting not to use bpf_jit_enable either 1 or 2 at all.