Re: [RFCv2 03/16] net: cls_bpf: limit hardware offload by software-only flag

On Mon, 29 Aug 2016 17:06:34 +0200, Daniel Borkmann wrote:

On 08/26/2016 08:06 PM, Jakub Kicinski wrote:

quoted

[...]

@@ -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]?

Sure!

quoted

+		if (!tc_flags_valid(gen_flags))
+			return -EINVAL;  

Shouldn't we: goto errout?

Ugh, right!  I'm missing:

	tcf_exts_destroy(&exts);

Thanks!