On 16-06-01 01:15 PM, Alexei Starovoitov wrote: > 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.kicin...@netronome.com> >>> Reviewed-by: Dinan Gunawardena <dgunaward...@netronome.com> >>> Reviewed-by: Simon Horman <simon.hor...@netronome.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. >
In the default case (no flags to the tc command) the tc filter tries to load itself in the hardware. The ethtool flag is there to enable/disable this default behavior. The alternative to the default load into hardware behavior is to specify it explicitly via userspace using the 'do offload tc+cls_bpf' as you note. This was the default behavior folks wanted at netdev conference so I added it even though for many of my use cases users specify explicitly if they want offload or not. Thanks, John