On Mon, Aug 28, 2017 at 02:41:16AM -0400, Chris Mi wrote: > Currently, all filters with the same priority are linked in a doubly > linked list. Every filter should have a unique handle. To make the > handle unique, we need to iterate the list every time to see if the > handle exists or not when inserting a new filter. It is time-consuming. > For example, it takes about 5m3.169s to insert 64K rules. > > This patch changes cls_flower to use IDR. With this patch, it > takes about 0m1.127s to insert 64K rules. The improvement is huge.
Very nice :) > But please note that in this testing, all filters share the same action. > If every filter has a unique action, that is another bottleneck. > Follow-up patch in this patchset addresses that. > > Signed-off-by: Chris Mi <chr...@mellanox.com> > Signed-off-by: Jiri Pirko <j...@mellanox.com> > --- > net/sched/cls_flower.c | 55 > +++++++++++++++++++++----------------------------- > 1 file changed, 23 insertions(+), 32 deletions(-) > > diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c > index bd9dab4..3d041d2 100644 > --- a/net/sched/cls_flower.c > +++ b/net/sched/cls_flower.c ... > @@ -890,6 +870,7 @@ static int fl_change(struct net *net, struct sk_buff > *in_skb, > struct cls_fl_filter *fnew; > struct nlattr **tb; > struct fl_flow_mask mask = {}; > + unsigned long idr_index; > int err; > > if (!tca[TCA_OPTIONS]) > @@ -920,13 +901,21 @@ static int fl_change(struct net *net, struct sk_buff > *in_skb, > goto errout; > > if (!handle) { > - handle = fl_grab_new_handle(tp, head); > - if (!handle) { > - err = -EINVAL; > + err = idr_alloc_ext(&head->handle_idr, fnew, &idr_index, > + 1, 0x80000000, GFP_KERNEL); > + if (err) > goto errout; > - } > + fnew->handle = idr_index; > + } > + > + /* user specifies a handle and it doesn't exist */ > + if (handle && !fold) { > + err = idr_alloc_ext(&head->handle_idr, fnew, &idr_index, > + handle, handle + 1, GFP_KERNEL); > + if (err) > + goto errout; > + fnew->handle = idr_index; > } > - fnew->handle = handle; > > if (tb[TCA_FLOWER_FLAGS]) { > fnew->flags = nla_get_u32(tb[TCA_FLOWER_FLAGS]); > @@ -980,6 +969,8 @@ static int fl_change(struct net *net, struct sk_buff > *in_skb, > *arg = fnew; > > if (fold) { > + fnew->handle = handle; Can it be the case that fold is non-NULL and handle is zero? The handling of that case seem to have changed in this patch. > + idr_replace_ext(&head->handle_idr, fnew, fnew->handle); > list_replace_rcu(&fold->list, &fnew->list); > tcf_unbind_filter(tp, &fold->res); > call_rcu(&fold->rcu, fl_destroy_filter); > -- > 1.8.3.1 >