netdev - Re: [RFC 06/12] nfp: add hardware cls

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]

Message-ID: <20160601215146.GA24671@ast-mbp.thefacebook.com>
Date:	Wed, 1 Jun 2016 14:51:48 -0700
From:	Alexei Starovoitov <alexei.starovoitov@...il.com>
To:	Jakub Kicinski <jakub.kicinski@...ronome.com>
Cc:	Daniel Borkmann <daniel@...earbox.net>, netdev@...r.kernel.org,
	ast@...nel.org, dinan.gunawardena@...ronome.com
Subject: Re: [RFC 06/12] nfp: add hardware cls_bpf offload

On Wed, Jun 01, 2016 at 10:15:57PM +0100, Jakub Kicinski wrote:
> On Wed, 1 Jun 2016 13:52:01 -0700, Alexei Starovoitov wrote:
> > On Wed, Jun 01, 2016 at 10:20:54PM +0200, Daniel Borkmann wrote:
> > > On 06/01/2016 06:50 PM, Jakub Kicinski wrote:  
> > > >Add hardware cls_bpf offload on our smart NICs.  Detect if
> > > >capable firmware is loaded and use it to load the code JITed
> > > >with just added translator onto 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>  
> > > [...]  
> > > >+static int
> > > >+nfp_net_bpf_offload_prepare(struct nfp_net *nn,
> > > >+			    struct tc_cls_bpf_offload *cls_bpf,
> > > >+			    struct nfp_bpf_result *res,
> > > >+			    void **code, dma_addr_t *dma_addr, u16 max_instr)
> > > >+{
> > > >+	unsigned int code_sz = max_instr * sizeof(u64);
> > > >+	u16 start_off, tgt_out, tgt_abort;
> > > >+	const struct tc_action *a;
> > > >+	int err;
> > > >+
> > > >+	if (tc_no_actions(cls_bpf->exts))
> > > >+		return -EINVAL;
> > > >+
> > > >+	tc_for_each_action(a, cls_bpf->exts) {
> > > >+		if (!is_tcf_gact_shot(a))
> > > >+			return -EINVAL;
> > > >+	}
> > > >+
> > > >+	if (cls_bpf->exts_integrated)
> > > >+		return -EINVAL;  
> > > 
> > > So cls_bpf has two working modes as mentioned: da (direct-action) and non-da.
> > > Direct-action is I would say the most typical way to run cls_bpf as it allows
> > > you to more naturally and efficiently code programs in the sense that classification
> > > and action is already combined in a single program, so there's no additional
> > > overhead of a linear action chain required, and a single program can already
> > > have multiple action code outcomes (TC_ACT_OK, TC_ACT_SHOT, ...), so that it is
> > > usually enough to run a single cls_bpf instance, for example, on sch_clsact
> > > ingress or egress parent, nothing more than that to get the job done. I think
> > > the cls_bpf->exts_integrated test could probably come first and if it's false,
> > > we'd need to walk the actions?  
> > 
> > I think it makes sense to offload da mode only. Doing tc_for_each_action
> > walk like above is ok, but the number of bpf programs with only separate
> > gact is diminishingly small and we don't recommend to use it anymore.
> > That's the stuff we used when da wasn't available.
>  
> Let me make sure I understand - to get da offloaded I would have to
> check that all return values are either OK or SHOT?  That was my

yes, but jit need to check all return values anyway and do even
more checks depending on how actions are configured, what is default
result, then re-jit the whole thing if default and/or act is changed, etc.
imo da is actually much simpler to jit.