From: Daniel Borkmann <daniel@iogearbox•net>
To: Jakub Kicinski <jakub.kicinski@netronome•com>, netdev@vger•kernel.org
Cc: ast@kernel•org, dinan.gunawardena@netronome•com,
jiri@resnulli•us, john.fastabend@gmail•com, kubakici@wp•pl
Subject: Re: [RFCv2 03/16] net: cls_bpf: limit hardware offload by software-only flag
Date: Mon, 29 Aug 2016 17:06:34 +0200 [thread overview]
Message-ID: <57C44F7A.5060501@iogearbox.net> (raw)
In-Reply-To: <1472234775-29453-4-git-send-email-jakub.kicinski@netronome.com>
On 08/26/2016 08:06 PM, Jakub Kicinski wrote:
> Add cls_bpf support for the TCA_CLS_FLAGS_SKIP_HW flag.
> Unlike U32 and flower cls_bpf already has some netlink
> flags defined. I chose to create a new attribute to be
> able to use the same flag values as the above.
>
> Unknown flags are ignored and not reported upon dump.
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome•com>
[...]
> @@ -55,6 +58,7 @@ struct cls_bpf_prog {
> static const struct nla_policy bpf_policy[TCA_BPF_MAX + 1] = {
> [TCA_BPF_CLASSID] = { .type = NLA_U32 },
> [TCA_BPF_FLAGS] = { .type = NLA_U32 },
> + [TCA_BPF_FLAGS_GEN] = { .type = NLA_U32 },
> [TCA_BPF_FD] = { .type = NLA_U32 },
> [TCA_BPF_NAME] = { .type = NLA_NUL_STRING, .len = CLS_BPF_NAME_LEN },
> [TCA_BPF_OPS_LEN] = { .type = NLA_U16 },
> @@ -156,6 +160,7 @@ static int cls_bpf_offload_cmd(struct tcf_proto *tp, struct cls_bpf_prog *prog,
> bpf_offload.filter = prog->filter;
> bpf_offload.name = prog->bpf_name;
> bpf_offload.exts_integrated = prog->exts_integrated;
> + bpf_offload.gen_flags = prog->gen_flags;
>
> return dev->netdev_ops->ndo_setup_tc(dev, tp->q->handle,
> tp->protocol, &offload);
> @@ -169,14 +174,14 @@ static void cls_bpf_offload(struct tcf_proto *tp, struct cls_bpf_prog *prog,
> enum tc_clsbpf_command cmd;
>
> if (oldprog && oldprog->offloaded) {
> - if (tc_should_offload(dev, tp, 0)) {
> + if (tc_should_offload(dev, tp, prog->gen_flags)) {
> cmd = TC_CLSBPF_REPLACE;
> } else {
> obj = oldprog;
> cmd = TC_CLSBPF_DESTROY;
> }
> } else {
> - if (!tc_should_offload(dev, tp, 0))
> + if (!tc_should_offload(dev, tp, prog->gen_flags))
> return;
> cmd = TC_CLSBPF_ADD;
> }
> @@ -372,6 +377,7 @@ static int cls_bpf_modify_existing(struct net *net, struct tcf_proto *tp,
> {
> bool is_bpf, is_ebpf, have_exts = false;
> struct tcf_exts exts;
> + u32 gen_flags = 0;
> int ret;
>
> is_bpf = tb[TCA_BPF_OPS_LEN] && tb[TCA_BPF_OPS];
> @@ -396,8 +402,16 @@ static int cls_bpf_modify_existing(struct net *net, struct tcf_proto *tp,
>
> have_exts = bpf_flags & TCA_BPF_FLAG_ACT_DIRECT;
> }
> + if (tb[TCA_BPF_FLAGS_GEN]) {
> + gen_flags = nla_get_u32(tb[TCA_BPF_FLAGS_GEN]);
> + /* Make sure dump doesn't report back flags we don't handle */
> + gen_flags &= CLS_BPF_SUPPORTED_GEN_FLAGS;
Instead of above rather ...
if (gen_flags & ~CLS_BPF_SUPPORTED_GEN_FLAGS) {
ret = -EINVAL;
goto errout;
}
... so that we can handle further additions properly like we do with
tb[TCA_BPF_FLAGS]?
> + if (!tc_flags_valid(gen_flags))
> + return -EINVAL;
Shouldn't we: goto errout?
> + }
>
> prog->exts_integrated = have_exts;
> + prog->gen_flags = gen_flags;
>
> ret = is_bpf ? cls_bpf_prog_from_ops(tb, prog) :
> cls_bpf_prog_from_efd(tb, prog, tp);
> @@ -569,6 +583,9 @@ static int cls_bpf_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
> bpf_flags |= TCA_BPF_FLAG_ACT_DIRECT;
> if (bpf_flags && nla_put_u32(skb, TCA_BPF_FLAGS, bpf_flags))
> goto nla_put_failure;
> + if (prog->gen_flags &&
> + nla_put_u32(skb, TCA_BPF_FLAGS_GEN, prog->gen_flags))
> + goto nla_put_failure;
>
> nla_nest_end(skb, nest);
Rest looks good:
Acked-by: Daniel Borkmann <daniel@iogearbox•net>
next prev parent reply other threads:[~2016-08-29 15:07 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-08-26 18:05 [RFCv2 00/16] BPF hardware offload (cls_bpf for now) Jakub Kicinski
2016-08-26 18:06 ` [RFCv2 01/16] add basic register-field manipulation macros Jakub Kicinski
2016-08-29 14:34 ` Daniel Borkmann
2016-08-29 15:07 ` Jakub Kicinski
2016-08-29 15:40 ` Daniel Borkmann
2016-08-26 18:06 ` [RFCv2 02/16] net: cls_bpf: add hardware offload Jakub Kicinski
2016-08-29 14:51 ` Daniel Borkmann
2016-08-26 18:06 ` [RFCv2 03/16] net: cls_bpf: limit hardware offload by software-only flag Jakub Kicinski
2016-08-29 15:06 ` Daniel Borkmann [this message]
2016-08-29 15:15 ` Jakub Kicinski
2016-08-26 18:06 ` [RFCv2 04/16] net: cls_bpf: add support for marking filters as hardware-only Jakub Kicinski
2016-08-29 15:28 ` Daniel Borkmann
2016-08-26 18:06 ` [RFCv2 05/16] bpf: recognize 64bit immediate loads as consts Jakub Kicinski
2016-08-26 18:06 ` [RFCv2 06/16] bpf: verifier: recognize rN ^ rN as load of 0 Jakub Kicinski
2016-08-26 18:06 ` [RFCv2 07/16] bpf: enable non-core use of the verfier Jakub Kicinski
2016-08-26 23:29 ` Alexei Starovoitov
2016-08-27 11:40 ` Jakub Kicinski
2016-08-27 17:32 ` Alexei Starovoitov
2016-08-29 20:13 ` Daniel Borkmann
2016-08-29 20:17 ` Daniel Borkmann
2016-08-30 10:48 ` Jakub Kicinski
2016-08-30 19:07 ` Daniel Borkmann
2016-08-30 20:22 ` Jakub Kicinski
2016-08-30 20:48 ` Alexei Starovoitov
2016-08-30 21:00 ` Daniel Borkmann
2016-08-31 1:18 ` Alexei Starovoitov
2016-08-26 18:06 ` [RFCv2 08/16] bpf: export bpf_prog_clone functions Jakub Kicinski
2016-08-26 18:06 ` [RFCv2 09/16] nfp: add BPF to NFP code translator Jakub Kicinski
2016-08-26 18:06 ` [RFCv2 10/16] nfp: bpf: add hardware bpf offload Jakub Kicinski
2016-08-26 18:06 ` [RFCv2 11/16] net: cls_bpf: allow offloaded filters to update stats Jakub Kicinski
2016-08-29 20:43 ` Daniel Borkmann
2016-08-26 18:06 ` [RFCv2 12/16] net: bpf: " Jakub Kicinski
2016-08-26 18:06 ` [RFCv2 13/16] nfp: bpf: add packet marking support Jakub Kicinski
2016-08-26 18:06 ` [RFCv2 14/16] net: act_mirred: allow statistic updates from offloaded actions Jakub Kicinski
2016-08-26 18:06 ` [RFCv2 15/16] nfp: bpf: add support for legacy redirect action Jakub Kicinski
2016-08-26 18:06 ` [RFCv2 16/16] nfp: bpf: add offload of TC direct action mode Jakub Kicinski
2016-08-29 21:09 ` Daniel Borkmann
2016-08-30 10:52 ` Jakub Kicinski
2016-08-30 20:02 ` Daniel Borkmann
2016-08-30 20:50 ` Jakub Kicinski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=57C44F7A.5060501@iogearbox.net \
--to=daniel@iogearbox$(echo .)net \
--cc=ast@kernel$(echo .)org \
--cc=dinan.gunawardena@netronome$(echo .)com \
--cc=jakub.kicinski@netronome$(echo .)com \
--cc=jiri@resnulli$(echo .)us \
--cc=john.fastabend@gmail$(echo .)com \
--cc=kubakici@wp$(echo .)pl \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox