From patchwork Mon Oct 16 08:31:39 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Mi X-Patchwork-Id: 826188 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3yFs4f549hz9sDB for ; Mon, 16 Oct 2017 19:32:02 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751829AbdJPIbz (ORCPT ); Mon, 16 Oct 2017 04:31:55 -0400 Received: from mail-il-dmz.mellanox.com ([193.47.165.129]:50084 "EHLO mellanox.co.il" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751780AbdJPIbx (ORCPT ); Mon, 16 Oct 2017 04:31:53 -0400 Received: from Internal Mail-Server by MTLPINE1 (envelope-from chrism@mellanox.com) with ESMTPS (AES256-SHA encrypted); 16 Oct 2017 10:31:47 +0200 Received: from dev-r630-03.mtbc.labs.mlnx (dev-r630-03.mtbc.labs.mlnx [10.12.205.13]) by labmailer.mlnx (8.13.8/8.13.8) with ESMTP id v9G8Vhgs017404; Mon, 16 Oct 2017 11:31:46 +0300 From: Chris Mi To: netdev@vger.kernel.org Cc: jhs@mojatatu.com, lucasb@mojatatu.com, xiyou.wangcong@gmail.com, jiri@resnulli.us, davem@davemloft.net Subject: [patch net 1/4] net/sched: Change tc_action refcnt and bindcnt to atomic Date: Mon, 16 Oct 2017 04:31:39 -0400 Message-Id: <1508142702-3312-2-git-send-email-chrism@mellanox.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1508142702-3312-1-git-send-email-chrism@mellanox.com> References: <1508142702-3312-1-git-send-email-chrism@mellanox.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org If many filters share the same action. That action's refcnt and bindcnt could be manipulated by many RCU callbacks at the same time. This patch makes these operations atomic. Fixes commit in pre-git era. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Signed-off-by: Chris Mi --- include/net/act_api.h | 4 ++-- net/sched/act_api.c | 21 +++++++++++---------- net/sched/act_bpf.c | 4 ++-- net/sched/act_connmark.c | 4 ++-- net/sched/act_csum.c | 4 ++-- net/sched/act_gact.c | 4 ++-- net/sched/act_ife.c | 4 ++-- net/sched/act_ipt.c | 4 ++-- net/sched/act_mirred.c | 4 ++-- net/sched/act_nat.c | 4 ++-- net/sched/act_pedit.c | 4 ++-- net/sched/act_police.c | 4 ++-- net/sched/act_sample.c | 4 ++-- net/sched/act_simple.c | 4 ++-- net/sched/act_skbedit.c | 4 ++-- net/sched/act_skbmod.c | 4 ++-- net/sched/act_tunnel_key.c | 4 ++-- net/sched/act_vlan.c | 4 ++-- 18 files changed, 45 insertions(+), 44 deletions(-) diff --git a/include/net/act_api.h b/include/net/act_api.h index b944e0eb..a469ab6 100644 --- a/include/net/act_api.h +++ b/include/net/act_api.h @@ -25,8 +25,8 @@ struct tc_action { struct tcf_idrinfo *idrinfo; u32 tcfa_index; - int tcfa_refcnt; - int tcfa_bindcnt; + atomic_t tcfa_refcnt; + atomic_t tcfa_bindcnt; u32 tcfa_capab; int tcfa_action; struct tcf_t tcfa_tm; diff --git a/net/sched/act_api.c b/net/sched/act_api.c index da6fa82..9c0224d 100644 --- a/net/sched/act_api.c +++ b/net/sched/act_api.c @@ -88,12 +88,13 @@ int __tcf_idr_release(struct tc_action *p, bool bind, bool strict) if (p) { if (bind) - p->tcfa_bindcnt--; - else if (strict && p->tcfa_bindcnt > 0) + atomic_dec(&p->tcfa_bindcnt); + else if (strict && atomic_read(&p->tcfa_bindcnt) > 0) return -EPERM; - p->tcfa_refcnt--; - if (p->tcfa_bindcnt <= 0 && p->tcfa_refcnt <= 0) { + atomic_dec(&p->tcfa_refcnt); + if (atomic_read(&p->tcfa_bindcnt) == 0 && + atomic_read(&p->tcfa_refcnt) == 0) { if (p->ops->cleanup) p->ops->cleanup(p, bind); tcf_idr_remove(p->idrinfo, p); @@ -245,8 +246,8 @@ bool tcf_idr_check(struct tc_action_net *tn, u32 index, struct tc_action **a, if (index && p) { if (bind) - p->tcfa_bindcnt++; - p->tcfa_refcnt++; + atomic_inc(&p->tcfa_bindcnt); + atomic_inc(&p->tcfa_refcnt); *a = p; return true; } @@ -274,9 +275,9 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est, if (unlikely(!p)) return -ENOMEM; - p->tcfa_refcnt = 1; + atomic_set(&p->tcfa_refcnt, 1); if (bind) - p->tcfa_bindcnt = 1; + atomic_set(&p->tcfa_bindcnt, 1); if (cpustats) { p->cpu_bstats = netdev_alloc_pcpu_stats(struct gnet_stats_basic_cpu); @@ -727,7 +728,7 @@ static void cleanup_a(struct list_head *actions, int ovr) return; list_for_each_entry(a, actions, list) - a->tcfa_refcnt--; + atomic_dec(&a->tcfa_refcnt); } int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla, @@ -751,7 +752,7 @@ int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla, } act->order = i; if (ovr) - act->tcfa_refcnt++; + atomic_inc(&act->tcfa_refcnt); list_add_tail(&act->list, actions); } diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c index c0c707e..4ddf281 100644 --- a/net/sched/act_bpf.c +++ b/net/sched/act_bpf.c @@ -141,8 +141,8 @@ static int tcf_bpf_dump(struct sk_buff *skb, struct tc_action *act, struct tcf_bpf *prog = to_bpf(act); struct tc_act_bpf opt = { .index = prog->tcf_index, - .refcnt = prog->tcf_refcnt - ref, - .bindcnt = prog->tcf_bindcnt - bind, + .refcnt = atomic_read(&prog->tcf_refcnt) - ref, + .bindcnt = atomic_read(&prog->tcf_bindcnt) - bind, .action = prog->tcf_action, }; struct tcf_t tm; diff --git a/net/sched/act_connmark.c b/net/sched/act_connmark.c index 10b7a88..d2cf658 100644 --- a/net/sched/act_connmark.c +++ b/net/sched/act_connmark.c @@ -153,8 +153,8 @@ static inline int tcf_connmark_dump(struct sk_buff *skb, struct tc_action *a, struct tc_connmark opt = { .index = ci->tcf_index, - .refcnt = ci->tcf_refcnt - ref, - .bindcnt = ci->tcf_bindcnt - bind, + .refcnt = atomic_read(&ci->tcf_refcnt) - ref, + .bindcnt = atomic_read(&ci->tcf_bindcnt) - bind, .action = ci->tcf_action, .zone = ci->zone, }; diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c index 1c40caa..b9ca424 100644 --- a/net/sched/act_csum.c +++ b/net/sched/act_csum.c @@ -575,8 +575,8 @@ static int tcf_csum_dump(struct sk_buff *skb, struct tc_action *a, int bind, .update_flags = p->update_flags, .index = p->tcf_index, .action = p->tcf_action, - .refcnt = p->tcf_refcnt - ref, - .bindcnt = p->tcf_bindcnt - bind, + .refcnt = atomic_read(&p->tcf_refcnt) - ref, + .bindcnt = atomic_read(&p->tcf_bindcnt) - bind, }; struct tcf_t t; diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c index e29a48e..b1326b7 100644 --- a/net/sched/act_gact.c +++ b/net/sched/act_gact.c @@ -169,8 +169,8 @@ static int tcf_gact_dump(struct sk_buff *skb, struct tc_action *a, struct tcf_gact *gact = to_gact(a); struct tc_gact opt = { .index = gact->tcf_index, - .refcnt = gact->tcf_refcnt - ref, - .bindcnt = gact->tcf_bindcnt - bind, + .refcnt = atomic_read(&gact->tcf_refcnt) - ref, + .bindcnt = atomic_read(&gact->tcf_bindcnt) - bind, .action = gact->tcf_action, }; struct tcf_t t; diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c index 8ccd358..5cdcc87 100644 --- a/net/sched/act_ife.c +++ b/net/sched/act_ife.c @@ -551,8 +551,8 @@ static int tcf_ife_dump(struct sk_buff *skb, struct tc_action *a, int bind, struct tcf_ife_info *ife = to_ife(a); struct tc_ife opt = { .index = ife->tcf_index, - .refcnt = ife->tcf_refcnt - ref, - .bindcnt = ife->tcf_bindcnt - bind, + .refcnt = atomic_read(&ife->tcf_refcnt) - ref, + .bindcnt = atomic_read(&ife->tcf_bindcnt) - bind, .action = ife->tcf_action, .flags = ife->flags, }; diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c index d9e399a..02c14dc 100644 --- a/net/sched/act_ipt.c +++ b/net/sched/act_ipt.c @@ -277,8 +277,8 @@ static int tcf_ipt_dump(struct sk_buff *skb, struct tc_action *a, int bind, if (unlikely(!t)) goto nla_put_failure; - c.bindcnt = ipt->tcf_bindcnt - bind; - c.refcnt = ipt->tcf_refcnt - ref; + c.bindcnt = atomic_read(&ipt->tcf_bindcnt) - bind; + c.refcnt = atomic_read(&ipt->tcf_refcnt) - ref; strcpy(t->u.user.name, ipt->tcfi_t->u.kernel.target->name); if (nla_put(skb, TCA_IPT_TARG, ipt->tcfi_t->u.user.target_size, t) || diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c index 416627c..aeeeb38 100644 --- a/net/sched/act_mirred.c +++ b/net/sched/act_mirred.c @@ -249,8 +249,8 @@ static int tcf_mirred_dump(struct sk_buff *skb, struct tc_action *a, int bind, struct tc_mirred opt = { .index = m->tcf_index, .action = m->tcf_action, - .refcnt = m->tcf_refcnt - ref, - .bindcnt = m->tcf_bindcnt - bind, + .refcnt = atomic_read(&m->tcf_refcnt) - ref, + .bindcnt = atomic_read(&m->tcf_bindcnt) - bind, .eaction = m->tcfm_eaction, .ifindex = m->tcfm_ifindex, }; diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c index c365d01..58fa1ae 100644 --- a/net/sched/act_nat.c +++ b/net/sched/act_nat.c @@ -256,8 +256,8 @@ static int tcf_nat_dump(struct sk_buff *skb, struct tc_action *a, .index = p->tcf_index, .action = p->tcf_action, - .refcnt = p->tcf_refcnt - ref, - .bindcnt = p->tcf_bindcnt - bind, + .refcnt = atomic_read(&p->tcf_refcnt) - ref, + .bindcnt = atomic_read(&p->tcf_bindcnt) - bind, }; struct tcf_t t; diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c index 491fe5de..27b9fea 100644 --- a/net/sched/act_pedit.c +++ b/net/sched/act_pedit.c @@ -391,8 +391,8 @@ static int tcf_pedit_dump(struct sk_buff *skb, struct tc_action *a, opt->nkeys = p->tcfp_nkeys; opt->flags = p->tcfp_flags; opt->action = p->tcf_action; - opt->refcnt = p->tcf_refcnt - ref; - opt->bindcnt = p->tcf_bindcnt - bind; + opt->refcnt = atomic_read(&p->tcf_refcnt) - ref; + opt->bindcnt = atomic_read(&p->tcf_bindcnt) - bind; if (p->tcfp_keys_ex) { tcf_pedit_key_ex_dump(skb, p->tcfp_keys_ex, p->tcfp_nkeys); diff --git a/net/sched/act_police.c b/net/sched/act_police.c index 3bb2ebf..d8a1ac6 100644 --- a/net/sched/act_police.c +++ b/net/sched/act_police.c @@ -272,8 +272,8 @@ static int tcf_act_police_dump(struct sk_buff *skb, struct tc_action *a, .action = police->tcf_action, .mtu = police->tcfp_mtu, .burst = PSCHED_NS2TICKS(police->tcfp_burst), - .refcnt = police->tcf_refcnt - ref, - .bindcnt = police->tcf_bindcnt - bind, + .refcnt = atomic_read(&police->tcf_refcnt) - ref, + .bindcnt = atomic_read(&police->tcf_bindcnt) - bind, }; struct tcf_t t; diff --git a/net/sched/act_sample.c b/net/sched/act_sample.c index ec986ae..64889a4 100644 --- a/net/sched/act_sample.c +++ b/net/sched/act_sample.c @@ -179,8 +179,8 @@ static int tcf_sample_dump(struct sk_buff *skb, struct tc_action *a, struct tc_sample opt = { .index = s->tcf_index, .action = s->tcf_action, - .refcnt = s->tcf_refcnt - ref, - .bindcnt = s->tcf_bindcnt - bind, + .refcnt = atomic_read(&s->tcf_refcnt) - ref, + .bindcnt = atomic_read(&s->tcf_bindcnt) - bind, }; struct tcf_t t; diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c index e7b57e5..ec7e397 100644 --- a/net/sched/act_simple.c +++ b/net/sched/act_simple.c @@ -148,8 +148,8 @@ static int tcf_simp_dump(struct sk_buff *skb, struct tc_action *a, struct tcf_defact *d = to_defact(a); struct tc_defact opt = { .index = d->tcf_index, - .refcnt = d->tcf_refcnt - ref, - .bindcnt = d->tcf_bindcnt - bind, + .refcnt = atomic_read(&d->tcf_refcnt) - ref, + .bindcnt = atomic_read(&d->tcf_bindcnt) - bind, .action = d->tcf_action, }; struct tcf_t t; diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c index 59949d6..2b05e56 100644 --- a/net/sched/act_skbedit.c +++ b/net/sched/act_skbedit.c @@ -172,8 +172,8 @@ static int tcf_skbedit_dump(struct sk_buff *skb, struct tc_action *a, struct tcf_skbedit *d = to_skbedit(a); struct tc_skbedit opt = { .index = d->tcf_index, - .refcnt = d->tcf_refcnt - ref, - .bindcnt = d->tcf_bindcnt - bind, + .refcnt = atomic_read(&d->tcf_refcnt) - ref, + .bindcnt = atomic_read(&d->tcf_bindcnt) - bind, .action = d->tcf_action, }; struct tcf_t t; diff --git a/net/sched/act_skbmod.c b/net/sched/act_skbmod.c index b642ad3..ca1ddf9 100644 --- a/net/sched/act_skbmod.c +++ b/net/sched/act_skbmod.c @@ -201,8 +201,8 @@ static int tcf_skbmod_dump(struct sk_buff *skb, struct tc_action *a, struct tcf_skbmod_params *p = rtnl_dereference(d->skbmod_p); struct tc_skbmod opt = { .index = d->tcf_index, - .refcnt = d->tcf_refcnt - ref, - .bindcnt = d->tcf_bindcnt - bind, + .refcnt = atomic_read(&d->tcf_refcnt) - ref, + .bindcnt = atomic_read(&d->tcf_bindcnt) - bind, .action = d->tcf_action, }; struct tcf_t t; diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c index 30c9627..158d472 100644 --- a/net/sched/act_tunnel_key.c +++ b/net/sched/act_tunnel_key.c @@ -250,8 +250,8 @@ static int tunnel_key_dump(struct sk_buff *skb, struct tc_action *a, struct tcf_tunnel_key_params *params; struct tc_tunnel_key opt = { .index = t->tcf_index, - .refcnt = t->tcf_refcnt - ref, - .bindcnt = t->tcf_bindcnt - bind, + .refcnt = atomic_read(&t->tcf_refcnt) - ref, + .bindcnt = atomic_read(&t->tcf_bindcnt) - bind, }; struct tcf_t tm; diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c index 16eb067..e2d7aab 100644 --- a/net/sched/act_vlan.c +++ b/net/sched/act_vlan.c @@ -208,8 +208,8 @@ static int tcf_vlan_dump(struct sk_buff *skb, struct tc_action *a, struct tcf_vlan *v = to_vlan(a); struct tc_vlan opt = { .index = v->tcf_index, - .refcnt = v->tcf_refcnt - ref, - .bindcnt = v->tcf_bindcnt - bind, + .refcnt = atomic_read(&v->tcf_refcnt) - ref, + .bindcnt = atomic_read(&v->tcf_bindcnt) - bind, .action = v->tcf_action, .v_action = v->tcfv_action, }; From patchwork Mon Oct 16 08:31:40 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Mi X-Patchwork-Id: 826187 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3yFs4Z1sMPz9sDB for ; Mon, 16 Oct 2017 19:31:58 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751889AbdJPIb4 (ORCPT ); Mon, 16 Oct 2017 04:31:56 -0400 Received: from mail-il-dmz.mellanox.com ([193.47.165.129]:50086 "EHLO mellanox.co.il" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751818AbdJPIbx (ORCPT ); Mon, 16 Oct 2017 04:31:53 -0400 Received: from Internal Mail-Server by MTLPINE1 (envelope-from chrism@mellanox.com) with ESMTPS (AES256-SHA encrypted); 16 Oct 2017 10:31:49 +0200 Received: from dev-r630-03.mtbc.labs.mlnx (dev-r630-03.mtbc.labs.mlnx [10.12.205.13]) by labmailer.mlnx (8.13.8/8.13.8) with ESMTP id v9G8Vhgt017404; Mon, 16 Oct 2017 11:31:48 +0300 From: Chris Mi To: netdev@vger.kernel.org Cc: jhs@mojatatu.com, lucasb@mojatatu.com, xiyou.wangcong@gmail.com, jiri@resnulli.us, davem@davemloft.net Subject: [patch net 2/4] net/sched: Use action array instead of action list as parameter Date: Mon, 16 Oct 2017 04:31:40 -0400 Message-Id: <1508142702-3312-3-git-send-email-chrism@mellanox.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1508142702-3312-1-git-send-email-chrism@mellanox.com> References: <1508142702-3312-1-git-send-email-chrism@mellanox.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org When destroying filters, actions should be destroyed first. The pointers of each action are saved in an array. TC doesn't use the array directly, but put all actions in a doubly linked list and use that list as parameter. There is no problem if each filter has its own actions. But if some filters share the same action, when these filters are destroyed, RCU callback fl_destroy_filter() may be called at the same time. That means the same action's 'struct list_head list' could be manipulated at the same time. It may point to an invalid address so that system will panic. This patch uses the action array directly to fix this issue. Fixes commit in pre-git era. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Signed-off-by: Chris Mi --- include/net/act_api.h | 7 ++-- net/sched/act_api.c | 103 +++++++++++++++++++++++++++++++------------------- net/sched/cls_api.c | 18 +++------ 3 files changed, 75 insertions(+), 53 deletions(-) diff --git a/include/net/act_api.h b/include/net/act_api.h index a469ab6..081a313 100644 --- a/include/net/act_api.h +++ b/include/net/act_api.h @@ -148,16 +148,17 @@ static inline int tcf_idr_release(struct tc_action *a, bool bind) int tcf_register_action(struct tc_action_ops *a, struct pernet_operations *ops); int tcf_unregister_action(struct tc_action_ops *a, struct pernet_operations *ops); -int tcf_action_destroy(struct list_head *actions, int bind); +int tcf_action_destroy(struct tc_action **actions, int nr, int bind); int tcf_action_exec(struct sk_buff *skb, struct tc_action **actions, int nr_actions, struct tcf_result *res); int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla, struct nlattr *est, char *name, int ovr, int bind, - struct list_head *actions); + struct tc_action **actions, int *nr); struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp, struct nlattr *nla, struct nlattr *est, char *name, int ovr, int bind); -int tcf_action_dump(struct sk_buff *skb, struct list_head *, int, int); +int tcf_action_dump(struct sk_buff *skb, struct tc_action **actions, int nr, + int bind, int ref); int tcf_action_dump_old(struct sk_buff *skb, struct tc_action *a, int, int); int tcf_action_dump_1(struct sk_buff *skb, struct tc_action *a, int, int); int tcf_action_copy_stats(struct sk_buff *, struct tc_action *, int); diff --git a/net/sched/act_api.c b/net/sched/act_api.c index 9c0224d..391d560 100644 --- a/net/sched/act_api.c +++ b/net/sched/act_api.c @@ -513,13 +513,15 @@ int tcf_action_exec(struct sk_buff *skb, struct tc_action **actions, } EXPORT_SYMBOL(tcf_action_exec); -int tcf_action_destroy(struct list_head *actions, int bind) +int tcf_action_destroy(struct tc_action **actions, int nr, int bind) { const struct tc_action_ops *ops; - struct tc_action *a, *tmp; + struct tc_action *a; int ret = 0; + int i; - list_for_each_entry_safe(a, tmp, actions, list) { + for (i = 0; i < nr; i++) { + a = actions[i]; ops = a->ops; ret = __tcf_idr_release(a, bind, true); if (ret == ACT_P_DELETED) @@ -568,14 +570,16 @@ int tcf_action_destroy(struct list_head *actions, int bind) } EXPORT_SYMBOL(tcf_action_dump_1); -int tcf_action_dump(struct sk_buff *skb, struct list_head *actions, +int tcf_action_dump(struct sk_buff *skb, struct tc_action **actions, int nr, int bind, int ref) { struct tc_action *a; - int err = -EINVAL; struct nlattr *nest; + int err = -EINVAL; + int i; - list_for_each_entry(a, actions, list) { + for (i = 0; i < nr; i++) { + a = actions[i]; nest = nla_nest_start(skb, a->order); if (nest == NULL) goto nla_put_failure; @@ -700,10 +704,7 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp, if (TC_ACT_EXT_CMP(a->tcfa_action, TC_ACT_GOTO_CHAIN)) { err = tcf_action_goto_chain_init(a, tp); if (err) { - LIST_HEAD(actions); - - list_add_tail(&a->list, &actions); - tcf_action_destroy(&actions, bind); + tcf_action_destroy(&a, 1, bind); return ERR_PTR(err); } } @@ -720,23 +721,27 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp, return ERR_PTR(err); } -static void cleanup_a(struct list_head *actions, int ovr) +static void cleanup_a(struct tc_action **actions, int nr, int ovr) { struct tc_action *a; + int i; if (!ovr) return; - list_for_each_entry(a, actions, list) + for (i = 0; i < nr; i++) { + a = actions[i]; atomic_dec(&a->tcfa_refcnt); + } } int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla, struct nlattr *est, char *name, int ovr, int bind, - struct list_head *actions) + struct tc_action **actions, int *nr) { struct nlattr *tb[TCA_ACT_MAX_PRIO + 1]; struct tc_action *act; + int n = 0; int err; int i; @@ -753,17 +758,19 @@ int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla, act->order = i; if (ovr) atomic_inc(&act->tcfa_refcnt); - list_add_tail(&act->list, actions); + actions[n++] = act; } + *nr = n; /* Remove the temp refcnt which was necessary to protect against * destroying an existing action which was being replaced */ - cleanup_a(actions, ovr); + cleanup_a(actions, n, ovr); return 0; err: - tcf_action_destroy(actions, bind); + tcf_action_destroy(actions, n, bind); + *nr = 0; return err; } @@ -811,9 +818,9 @@ int tcf_action_copy_stats(struct sk_buff *skb, struct tc_action *p, return -1; } -static int tca_get_fill(struct sk_buff *skb, struct list_head *actions, - u32 portid, u32 seq, u16 flags, int event, int bind, - int ref) +static int tca_get_fill(struct sk_buff *skb, struct tc_action **actions, + int nr, u32 portid, u32 seq, u16 flags, int event, + int bind, int ref) { struct tcamsg *t; struct nlmsghdr *nlh; @@ -832,7 +839,7 @@ static int tca_get_fill(struct sk_buff *skb, struct list_head *actions, if (nest == NULL) goto out_nlmsg_trim; - if (tcf_action_dump(skb, actions, bind, ref) < 0) + if (tcf_action_dump(skb, actions, nr, bind, ref) < 0) goto out_nlmsg_trim; nla_nest_end(skb, nest); @@ -847,14 +854,14 @@ static int tca_get_fill(struct sk_buff *skb, struct list_head *actions, static int tcf_get_notify(struct net *net, u32 portid, struct nlmsghdr *n, - struct list_head *actions, int event) + struct tc_action **actions, int nr, int event) { struct sk_buff *skb; skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL); if (!skb) return -ENOBUFS; - if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, 0, event, + if (tca_get_fill(skb, actions, nr, portid, n->nlmsg_seq, 0, event, 0, 0) <= 0) { kfree_skb(skb); return -EINVAL; @@ -968,7 +975,8 @@ static int tca_action_flush(struct net *net, struct nlattr *nla, } static int -tcf_del_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions, +tcf_del_notify(struct net *net, struct nlmsghdr *n, + struct tc_action **actions, int nr, u32 portid) { int ret; @@ -978,14 +986,14 @@ static int tca_action_flush(struct net *net, struct nlattr *nla, if (!skb) return -ENOBUFS; - if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, 0, RTM_DELACTION, - 0, 1) <= 0) { + if (tca_get_fill(skb, actions, nr, portid, n->nlmsg_seq, 0, + RTM_DELACTION, 0, 1) <= 0) { kfree_skb(skb); return -EINVAL; } /* now do the delete */ - ret = tcf_action_destroy(actions, 0); + ret = tcf_action_destroy(actions, nr, 0); if (ret < 0) { kfree_skb(skb); return ret; @@ -1002,10 +1010,11 @@ static int tca_action_flush(struct net *net, struct nlattr *nla, tca_action_gd(struct net *net, struct nlattr *nla, struct nlmsghdr *n, u32 portid, int event) { - int i, ret; struct nlattr *tb[TCA_ACT_MAX_PRIO + 1]; + struct tc_action **actions; struct tc_action *act; - LIST_HEAD(actions); + int i, ret; + int nr = 0; ret = nla_parse_nested(tb, TCA_ACT_MAX_PRIO, nla, NULL, NULL); if (ret < 0) @@ -1018,6 +1027,11 @@ static int tca_action_flush(struct net *net, struct nlattr *nla, return -EINVAL; } + actions = kcalloc(TCA_ACT_MAX_PRIO, sizeof(struct tc_action *), + GFP_KERNEL); + if (!actions) + return -ENOMEM; + for (i = 1; i <= TCA_ACT_MAX_PRIO && tb[i]; i++) { act = tcf_action_get_1(net, tb[i], n, portid); if (IS_ERR(act)) { @@ -1025,25 +1039,28 @@ static int tca_action_flush(struct net *net, struct nlattr *nla, goto err; } act->order = i; - list_add_tail(&act->list, &actions); + actions[nr++] = act; } if (event == RTM_GETACTION) - ret = tcf_get_notify(net, portid, n, &actions, event); + ret = tcf_get_notify(net, portid, n, actions, nr, event); else { /* delete */ - ret = tcf_del_notify(net, n, &actions, portid); + ret = tcf_del_notify(net, n, actions, nr, portid); if (ret) goto err; + kfree(actions); return ret; } err: if (event != RTM_GETACTION) - tcf_action_destroy(&actions, 0); + tcf_action_destroy(actions, nr, 0); + kfree(actions); return ret; } static int -tcf_add_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions, +tcf_add_notify(struct net *net, struct nlmsghdr *n, + struct tc_action **actions, int nr, u32 portid) { struct sk_buff *skb; @@ -1053,7 +1070,7 @@ static int tca_action_flush(struct net *net, struct nlattr *nla, if (!skb) return -ENOBUFS; - if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, n->nlmsg_flags, + if (tca_get_fill(skb, actions, nr, portid, n->nlmsg_seq, n->nlmsg_flags, RTM_NEWACTION, 0, 0) <= 0) { kfree_skb(skb); return -EINVAL; @@ -1069,14 +1086,24 @@ static int tca_action_flush(struct net *net, struct nlattr *nla, static int tcf_action_add(struct net *net, struct nlattr *nla, struct nlmsghdr *n, u32 portid, int ovr) { + struct tc_action **actions; int ret = 0; - LIST_HEAD(actions); + int nr; - ret = tcf_action_init(net, NULL, nla, NULL, NULL, ovr, 0, &actions); + actions = kcalloc(TCA_ACT_MAX_PRIO, sizeof(struct tc_action *), + GFP_KERNEL); + if (!actions) + return -ENOMEM; + + ret = tcf_action_init(net, NULL, nla, NULL, NULL, ovr, 0, + actions, &nr); if (ret) - return ret; + goto out; - return tcf_add_notify(net, n, &actions, portid); + ret = tcf_add_notify(net, n, actions, nr, portid); +out: + kfree(actions); + return ret; } static u32 tcaa_root_flags_allowed = TCA_FLAG_LARGE_DUMP_ON; diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index 0b2219a..acaa0c6 100644 --- a/net/sched/cls_api.c +++ b/net/sched/cls_api.c @@ -879,8 +879,7 @@ void tcf_exts_destroy(struct tcf_exts *exts) #ifdef CONFIG_NET_CLS_ACT LIST_HEAD(actions); - tcf_exts_to_list(exts, &actions); - tcf_action_destroy(&actions, TCA_ACT_UNBIND); + tcf_action_destroy(exts->actions, exts->nr_actions, TCA_ACT_UNBIND); kfree(exts->actions); exts->nr_actions = 0; #endif @@ -905,17 +904,14 @@ int tcf_exts_validate(struct net *net, struct tcf_proto *tp, struct nlattr **tb, exts->actions[0] = act; exts->nr_actions = 1; } else if (exts->action && tb[exts->action]) { - LIST_HEAD(actions); - int err, i = 0; + int err; err = tcf_action_init(net, tp, tb[exts->action], rate_tlv, NULL, ovr, TCA_ACT_BIND, - &actions); + exts->actions, + &exts->nr_actions); if (err) return err; - list_for_each_entry(act, &actions, list) - exts->actions[i++] = act; - exts->nr_actions = i; } } #else @@ -961,14 +957,12 @@ int tcf_exts_dump(struct sk_buff *skb, struct tcf_exts *exts) * tc data even if iproute2 was newer - jhs */ if (exts->type != TCA_OLD_COMPAT) { - LIST_HEAD(actions); - nest = nla_nest_start(skb, exts->action); if (nest == NULL) goto nla_put_failure; - tcf_exts_to_list(exts, &actions); - if (tcf_action_dump(skb, &actions, 0, 0) < 0) + if (tcf_action_dump(skb, exts->actions, + exts->nr_actions, 0, 0) < 0) goto nla_put_failure; nla_nest_end(skb, nest); } else if (exts->police) { From patchwork Mon Oct 16 08:31:41 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Mi X-Patchwork-Id: 826190 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3yFs4l11N1z9sDB for ; Mon, 16 Oct 2017 19:32:07 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751915AbdJPIcB (ORCPT ); Mon, 16 Oct 2017 04:32:01 -0400 Received: from mail-il-dmz.mellanox.com ([193.47.165.129]:50132 "EHLO mellanox.co.il" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751818AbdJPIb6 (ORCPT ); Mon, 16 Oct 2017 04:31:58 -0400 Received: from Internal Mail-Server by MTLPINE1 (envelope-from chrism@mellanox.com) with ESMTPS (AES256-SHA encrypted); 16 Oct 2017 10:31:51 +0200 Received: from dev-r630-03.mtbc.labs.mlnx (dev-r630-03.mtbc.labs.mlnx [10.12.205.13]) by labmailer.mlnx (8.13.8/8.13.8) with ESMTP id v9G8Vhgu017404; Mon, 16 Oct 2017 11:31:50 +0300 From: Chris Mi To: netdev@vger.kernel.org Cc: jhs@mojatatu.com, lucasb@mojatatu.com, xiyou.wangcong@gmail.com, jiri@resnulli.us, davem@davemloft.net Subject: [patch net 3/4] selftests: Introduce a new script to generate tc batch file Date: Mon, 16 Oct 2017 04:31:41 -0400 Message-Id: <1508142702-3312-4-git-send-email-chrism@mellanox.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1508142702-3312-1-git-send-email-chrism@mellanox.com> References: <1508142702-3312-1-git-send-email-chrism@mellanox.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org # ./tdc_batch.py -h usage: tdc_batch.py [-h] [-n NUMBER] [-o] [-s] [-p] device file TC batch file generator positional arguments: device device name file batch file name optional arguments: -h, --help show this help message and exit -n NUMBER, --number NUMBER how many lines in batch file -o, --skip_sw skip_sw (offload), by default skip_hw -s, --share_action all filters share the same action -p, --prio all filters have different prio Signed-off-by: Chris Mi --- tools/testing/selftests/tc-testing/tdc_batch.py | 62 +++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100755 tools/testing/selftests/tc-testing/tdc_batch.py diff --git a/tools/testing/selftests/tc-testing/tdc_batch.py b/tools/testing/selftests/tc-testing/tdc_batch.py new file mode 100755 index 0000000..707c6bf --- /dev/null +++ b/tools/testing/selftests/tc-testing/tdc_batch.py @@ -0,0 +1,62 @@ +#!/usr/bin/python3 + +""" +tdc_batch.py - a script to generate TC batch file + +Copyright (C) 2017 Chris Mi +""" + +import argparse + +parser = argparse.ArgumentParser(description='TC batch file generator') +parser.add_argument("device", help="device name") +parser.add_argument("file", help="batch file name") +parser.add_argument("-n", "--number", type=int, + help="how many lines in batch file") +parser.add_argument("-o", "--skip_sw", + help="skip_sw (offload), by default skip_hw", + action="store_true") +parser.add_argument("-s", "--share_action", + help="all filters share the same action", + action="store_true") +parser.add_argument("-p", "--prio", + help="all filters have different prio", + action="store_true") +args = parser.parse_args() + +device = args.device +file = open(args.file, 'w') + +number = 1 +if args.number: + number = args.number + +skip = "skip_hw" +if args.skip_sw: + skip = "skip_sw" + +share_action = "" +if args.share_action: + share_action = "index 1" + +prio = "prio 1" +if args.prio: + prio = "" + if number > 0x4000: + number = 0x4000 + +index = 0 +for i in range(0x100): + for j in range(0x100): + for k in range(0x100): + mac = ("%02x:%02x:%02x" % (i, j, k)) + src_mac = "e4:11:00:" + mac + dst_mac = "e4:12:00:" + mac + cmd = ("filter add dev %s %s protocol ip parent ffff: flower %s " + "src_mac %s dst_mac %s action drop %s" % + (device, prio, skip, src_mac, dst_mac, share_action)) + file.write("%s\n" % cmd) + index += 1 + if index >= number: + file.close() + exit(0) From patchwork Mon Oct 16 08:31:42 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Mi X-Patchwork-Id: 826189 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3yFs4h2Cgwz9sDB for ; Mon, 16 Oct 2017 19:32:04 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751929AbdJPIcC (ORCPT ); Mon, 16 Oct 2017 04:32:02 -0400 Received: from mail-il-dmz.mellanox.com ([193.47.165.129]:50134 "EHLO mellanox.co.il" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751891AbdJPIb6 (ORCPT ); Mon, 16 Oct 2017 04:31:58 -0400 Received: from Internal Mail-Server by MTLPINE1 (envelope-from chrism@mellanox.com) with ESMTPS (AES256-SHA encrypted); 16 Oct 2017 10:31:53 +0200 Received: from dev-r630-03.mtbc.labs.mlnx (dev-r630-03.mtbc.labs.mlnx [10.12.205.13]) by labmailer.mlnx (8.13.8/8.13.8) with ESMTP id v9G8Vhgv017404; Mon, 16 Oct 2017 11:31:52 +0300 From: Chris Mi To: netdev@vger.kernel.org Cc: jhs@mojatatu.com, lucasb@mojatatu.com, xiyou.wangcong@gmail.com, jiri@resnulli.us, davem@davemloft.net Subject: [patch net 4/4] selftests: Introduce a new test case to tc testsuite Date: Mon, 16 Oct 2017 04:31:42 -0400 Message-Id: <1508142702-3312-5-git-send-email-chrism@mellanox.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1508142702-3312-1-git-send-email-chrism@mellanox.com> References: <1508142702-3312-1-git-send-email-chrism@mellanox.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org In this patchset, we fixed a tc bug. This patch adds the test case that reproduces the bug. To run this test case, user should specify an existing NIC device: # sudo ./tdc.py -d enp4s0f0 This test case belongs to category "flower". If user doesn't specify a NIC device, the test cases belong to "flower" will not be run. In this test case, we create 1M filters and all filters share the same action. When destroying all filters, kernel should not panic. It takes about 18s to run it. Signed-off-by: Chris Mi --- .../tc-testing/tc-tests/filters/tests.json | 23 +++++++++++++++++++++- tools/testing/selftests/tc-testing/tdc.py | 20 +++++++++++++++---- tools/testing/selftests/tc-testing/tdc_config.py | 2 ++ 3 files changed, 40 insertions(+), 5 deletions(-) diff --git a/tools/testing/selftests/tc-testing/tc-tests/filters/tests.json b/tools/testing/selftests/tc-testing/tc-tests/filters/tests.json index c727b96..5fa02d8 100644 --- a/tools/testing/selftests/tc-testing/tc-tests/filters/tests.json +++ b/tools/testing/selftests/tc-testing/tc-tests/filters/tests.json @@ -17,5 +17,26 @@ "teardown": [ "$TC qdisc del dev $DEV1 ingress" ] + }, + { + "id": "d052", + "name": "Add 1M filters with the same action", + "category": [ + "filter", + "flower" + ], + "setup": [ + "$TC qdisc add dev $DEV2 ingress", + "./tdc_batch.py $DEV2 $BATCH_FILE --share_action -n 1000000" + ], + "cmdUnderTest": "$TC -b $BATCH_FILE", + "expExitCode": "0", + "verifyCmd": "$TC actions list action gact", + "matchPattern": "action order 0: gact action drop.*index 1 ref 1000000 bind 1000000", + "matchCount": "1", + "teardown": [ + "$TC qdisc del dev $DEV2 ingress", + "/bin/rm $BATCH_FILE" + ] } -] \ No newline at end of file +] diff --git a/tools/testing/selftests/tc-testing/tdc.py b/tools/testing/selftests/tc-testing/tdc.py index cd61b78..5f11f5d 100755 --- a/tools/testing/selftests/tc-testing/tdc.py +++ b/tools/testing/selftests/tc-testing/tdc.py @@ -88,7 +88,7 @@ def prepare_env(cmdlist): exit(1) -def test_runner(filtered_tests): +def test_runner(filtered_tests, args): """ Driver function for the unit tests. @@ -105,6 +105,8 @@ def test_runner(filtered_tests): for tidx in testlist: result = True tresult = "" + if "flower" in tidx["category"] and args.device == None: + continue print("Test " + tidx["id"] + ": " + tidx["name"]) prepare_env(tidx["setup"]) (p, procout) = exec_cmd(tidx["cmdUnderTest"]) @@ -152,6 +154,10 @@ def ns_create(): exec_cmd(cmd, False) cmd = 'ip -s $NS link set $DEV1 up' exec_cmd(cmd, False) + cmd = 'ip link set $DEV2 netns $NS' + exec_cmd(cmd, False) + cmd = 'ip -s $NS link set $DEV2 up' + exec_cmd(cmd, False) def ns_destroy(): @@ -211,7 +217,8 @@ def set_args(parser): help='Execute the single test case with specified ID') parser.add_argument('-i', '--id', action='store_true', dest='gen_id', help='Generate ID numbers for new test cases') - return parser + parser.add_argument('-d', '--device', + help='Execute the test case in flower category') return parser @@ -225,6 +232,8 @@ def check_default_settings(args): if args.path != None: NAMES['TC'] = args.path + if args.device != None: + NAMES['DEV2'] = args.device if not os.path.isfile(NAMES['TC']): print("The specified tc path " + NAMES['TC'] + " does not exist.") exit(1) @@ -381,14 +390,17 @@ def set_operation_mode(args): if (len(alltests) == 0): print("Cannot find a test case with ID matching " + target_id) exit(1) - catresults = test_runner(alltests) + catresults = test_runner(alltests, args) print("All test results: " + "\n\n" + catresults) elif (len(target_category) > 0): + if (target_category == "flower") and args.device == None: + print("Please specify a NIC device (-d) to run category flower") + exit(1) if (target_category not in ucat): print("Specified category is not present in this file.") exit(1) else: - catresults = test_runner(testcases[target_category]) + catresults = test_runner(testcases[target_category], args) print("Category " + target_category + "\n\n" + catresults) ns_destroy() diff --git a/tools/testing/selftests/tc-testing/tdc_config.py b/tools/testing/selftests/tc-testing/tdc_config.py index 0108737..b635251 100644 --- a/tools/testing/selftests/tc-testing/tdc_config.py +++ b/tools/testing/selftests/tc-testing/tdc_config.py @@ -12,6 +12,8 @@ NAMES = { # Name of veth devices to be created for the namespace 'DEV0': 'v0p0', 'DEV1': 'v0p1', + 'DEV2': '', + 'BATCH_FILE': './batch.txt', # Name of the namespace to use 'NS': 'tcut' }