On Mon, 29 Aug 2016 17:06:34 +0200, Daniel Borkmann wrote:
> On 08/26/2016 08:06 PM, Jakub Kicinski wrote:
> > [...]
> > @@ -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!
> > + if (!tc_flags_valid(gen_flags))
> > + return -EINVAL;
>
> Shouldn't we: goto errout?
Ugh, right! I'm missing:
tcf_exts_destroy(&exts);
Thanks!
