On Mon, 4 Mar 2019 14:24:05 +0000
Vlad Buslov <vla...@mellanox.com> wrote:
> On Fri 01 Mar 2019 at 23:51, Stefano Brivio <sbri...@redhat.com> wrote:
> > Hi Vlad,
> >
> > On Wed, 27 Feb 2019 12:12:18 +0200
> > Vlad Buslov <vla...@mellanox.com> wrote:
> >
> >> -static bool __fl_delete(struct tcf_proto *tp, struct cls_fl_filter *f,
> >> - struct netlink_ext_ack *extack)
> >> +static int __fl_delete(struct tcf_proto *tp, struct cls_fl_filter *f,
> >> + bool *last, struct netlink_ext_ack *extack)
> >> {
> >> struct cls_fl_head *head = fl_head_dereference(tp);
> >> bool async = tcf_exts_get_net(&f->exts);
> >> - bool last;
> >>
> >> + *last = false;
> >> +
> >> + if (f->deleted)
> >> + return -ENOENT;
> >> +
> >> + f->deleted = true;
> >
> > Now that I can read this more easily :) I have a doubt: you say this
> > flag "prevent[s] double deletion of filter by concurrent tasks".
> >
> > However, if this has no further protections (which I can't readily
> > see), I think this is racy:
> >
> > task 1 task 2
> > if (f->deleted) [false]
> > if (f->deleted) [false]
> > f->deleted = true; f->deleted = true;
> >
> > what am I missing here?
>
> Of course! Lock is added in "[PATCH net-next v2 10/12] net: sched:
> flower: protect flower classifier state with spinlock". This is safe to
> do because everything is still protected by rtnl mutex until last patch
> in this series sets the TCF_PROTO_OPS_DOIT_UNLOCKED flag for flower.
Indeed! My bad, I forgot about 10/12 here.
--
Stefano
