From 3d32f4c548bd8f3af58c59521bee9be127f3e87d Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Wed, 1 Aug 2018 12:36:55 +0200 Subject: net: sched: change name of zombie chain to "held_by_acts_only" As mentioned by Cong and Jakub during the review process, it is a bit odd to sometimes (act flow) create a new chain which would be immediately a "zombie". So just rename it to "held_by_acts_only". Signed-off-by: Jiri Pirko Suggested-by: Cong Wang Suggested-by: Jakub Kicinski Acked-by: Cong Wang Signed-off-by: David S. Miller --- net/sched/cls_api.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index e20aad1987b8..2f78341f2888 100644 --- a/net/sched/cls_api.c +++ b/net/sched/cls_api.c @@ -272,11 +272,10 @@ static void tcf_chain_release_by_act(struct tcf_chain *chain) --chain->action_refcnt; } -static bool tcf_chain_is_zombie(struct tcf_chain *chain) +static bool tcf_chain_held_by_acts_only(struct tcf_chain *chain) { /* In case all the references are action references, this - * chain is a zombie and should not be listed in the chain - * dump list. + * chain should not be shown to the user. */ return chain->refcnt == chain->action_refcnt; } @@ -1838,10 +1837,9 @@ replay: chain = tcf_chain_lookup(block, chain_index); if (n->nlmsg_type == RTM_NEWCHAIN) { if (chain) { - if (tcf_chain_is_zombie(chain)) { + if (tcf_chain_held_by_acts_only(chain)) { /* The chain exists only because there is - * some action referencing it, meaning it - * is a zombie. + * some action referencing it. */ tcf_chain_hold(chain); } else { @@ -1860,7 +1858,7 @@ replay: } } } else { - if (!chain || tcf_chain_is_zombie(chain)) { + if (!chain || tcf_chain_held_by_acts_only(chain)) { NL_SET_ERR_MSG(extack, "Cannot find specified filter chain"); return -EINVAL; } @@ -1988,7 +1986,7 @@ static int tc_dump_chain(struct sk_buff *skb, struct netlink_callback *cb) index++; continue; } - if (tcf_chain_is_zombie(chain)) + if (tcf_chain_held_by_acts_only(chain)) continue; err = tc_chain_fill_node(chain, net, skb, block, NETLINK_CB(cb->skb).portid, -- cgit v1.2.3 From 5368140730e4a67169303edd3a13e31fd9b9d355 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Wed, 1 Aug 2018 12:36:56 +0200 Subject: net: sched: fix notifications for action-held chains Chains that only have action references serve as placeholders. Until a non-action reference is created, user should not be aware of the chain. Also he should not receive any notifications about it. So send notifications for the new chain only in case the chain gets the first non-action reference. Symmetrically to that, when the last non-action reference is dropped, send the notification about deleted chain. Reported-by: Cong Wang Signed-off-by: Jiri Pirko Acked-by: Cong Wang v1->v2: - made __tcf_chain_{get,put}() static as suggested by Cong Signed-off-by: David S. Miller --- net/sched/cls_api.c | 71 ++++++++++++++++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 28 deletions(-) diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index 2f78341f2888..b194a5abfc6a 100644 --- a/net/sched/cls_api.c +++ b/net/sched/cls_api.c @@ -262,16 +262,6 @@ static void tcf_chain_hold(struct tcf_chain *chain) ++chain->refcnt; } -static void tcf_chain_hold_by_act(struct tcf_chain *chain) -{ - ++chain->action_refcnt; -} - -static void tcf_chain_release_by_act(struct tcf_chain *chain) -{ - --chain->action_refcnt; -} - static bool tcf_chain_held_by_acts_only(struct tcf_chain *chain) { /* In case all the references are action references, this @@ -295,52 +285,77 @@ static struct tcf_chain *tcf_chain_lookup(struct tcf_block *block, static int tc_chain_notify(struct tcf_chain *chain, struct sk_buff *oskb, u32 seq, u16 flags, int event, bool unicast); -struct tcf_chain *tcf_chain_get(struct tcf_block *block, u32 chain_index, - bool create) +static struct tcf_chain *__tcf_chain_get(struct tcf_block *block, + u32 chain_index, bool create, + bool by_act) { struct tcf_chain *chain = tcf_chain_lookup(block, chain_index); if (chain) { tcf_chain_hold(chain); - return chain; + } else { + if (!create) + return NULL; + chain = tcf_chain_create(block, chain_index); + if (!chain) + return NULL; } - if (!create) - return NULL; - chain = tcf_chain_create(block, chain_index); - if (!chain) - return NULL; - tc_chain_notify(chain, NULL, 0, NLM_F_CREATE | NLM_F_EXCL, - RTM_NEWCHAIN, false); + if (by_act) + ++chain->action_refcnt; + + /* Send notification only in case we got the first + * non-action reference. Until then, the chain acts only as + * a placeholder for actions pointing to it and user ought + * not know about them. + */ + if (chain->refcnt - chain->action_refcnt == 1 && !by_act) + tc_chain_notify(chain, NULL, 0, NLM_F_CREATE | NLM_F_EXCL, + RTM_NEWCHAIN, false); + return chain; } + +struct tcf_chain *tcf_chain_get(struct tcf_block *block, u32 chain_index, + bool create) +{ + return __tcf_chain_get(block, chain_index, create, false); +} EXPORT_SYMBOL(tcf_chain_get); struct tcf_chain *tcf_chain_get_by_act(struct tcf_block *block, u32 chain_index) { - struct tcf_chain *chain = tcf_chain_get(block, chain_index, true); - - tcf_chain_hold_by_act(chain); - return chain; + return __tcf_chain_get(block, chain_index, true, true); } EXPORT_SYMBOL(tcf_chain_get_by_act); static void tc_chain_tmplt_del(struct tcf_chain *chain); -void tcf_chain_put(struct tcf_chain *chain) +static void __tcf_chain_put(struct tcf_chain *chain, bool by_act) { - if (--chain->refcnt == 0) { + if (by_act) + chain->action_refcnt--; + chain->refcnt--; + + /* The last dropped non-action reference will trigger notification. */ + if (chain->refcnt - chain->action_refcnt == 0 && !by_act) tc_chain_notify(chain, NULL, 0, 0, RTM_DELCHAIN, false); + + if (chain->refcnt == 0) { tc_chain_tmplt_del(chain); tcf_chain_destroy(chain); } } + +void tcf_chain_put(struct tcf_chain *chain) +{ + __tcf_chain_put(chain, false); +} EXPORT_SYMBOL(tcf_chain_put); void tcf_chain_put_by_act(struct tcf_chain *chain) { - tcf_chain_release_by_act(chain); - tcf_chain_put(chain); + __tcf_chain_put(chain, true); } EXPORT_SYMBOL(tcf_chain_put_by_act); -- cgit v1.2.3 From 290b1c8b1a902c0902df9ec05577ab209296f345 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Wed, 1 Aug 2018 12:36:57 +0200 Subject: net: sched: make tcf_chain_{get,put}() static These are no longer used outside of cls_api.c so make them static. Move tcf_chain_flush() to avoid fwd declaration of tcf_chain_put(). Signed-off-by: Jiri Pirko v1->v2: - new patch Signed-off-by: David S. Miller --- include/net/pkt_cls.h | 3 --- net/sched/cls_api.c | 34 ++++++++++++++++------------------ 2 files changed, 16 insertions(+), 21 deletions(-) diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h index 22bfc3a13c25..ef727f71336e 100644 --- a/include/net/pkt_cls.h +++ b/include/net/pkt_cls.h @@ -40,11 +40,8 @@ struct tcf_block_cb; bool tcf_queue_work(struct rcu_work *rwork, work_func_t func); #ifdef CONFIG_NET_CLS -struct tcf_chain *tcf_chain_get(struct tcf_block *block, u32 chain_index, - bool create); struct tcf_chain *tcf_chain_get_by_act(struct tcf_block *block, u32 chain_index); -void tcf_chain_put(struct tcf_chain *chain); void tcf_chain_put_by_act(struct tcf_chain *chain); void tcf_block_netif_keep_dst(struct tcf_block *block); int tcf_block_get(struct tcf_block **p_block, diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index b194a5abfc6a..e8b0bbd0883f 100644 --- a/net/sched/cls_api.c +++ b/net/sched/cls_api.c @@ -232,19 +232,6 @@ static void tcf_chain0_head_change(struct tcf_chain *chain, tcf_chain_head_change_item(item, tp_head); } -static void tcf_chain_flush(struct tcf_chain *chain) -{ - struct tcf_proto *tp = rtnl_dereference(chain->filter_chain); - - tcf_chain0_head_change(chain, NULL); - while (tp) { - RCU_INIT_POINTER(chain->filter_chain, tp->next); - tcf_proto_destroy(tp, NULL); - tp = rtnl_dereference(chain->filter_chain); - tcf_chain_put(chain); - } -} - static void tcf_chain_destroy(struct tcf_chain *chain) { struct tcf_block *block = chain->block; @@ -316,12 +303,11 @@ static struct tcf_chain *__tcf_chain_get(struct tcf_block *block, return chain; } -struct tcf_chain *tcf_chain_get(struct tcf_block *block, u32 chain_index, - bool create) +static struct tcf_chain *tcf_chain_get(struct tcf_block *block, u32 chain_index, + bool create) { return __tcf_chain_get(block, chain_index, create, false); } -EXPORT_SYMBOL(tcf_chain_get); struct tcf_chain *tcf_chain_get_by_act(struct tcf_block *block, u32 chain_index) { @@ -347,11 +333,10 @@ static void __tcf_chain_put(struct tcf_chain *chain, bool by_act) } } -void tcf_chain_put(struct tcf_chain *chain) +static void tcf_chain_put(struct tcf_chain *chain) { __tcf_chain_put(chain, false); } -EXPORT_SYMBOL(tcf_chain_put); void tcf_chain_put_by_act(struct tcf_chain *chain) { @@ -365,6 +350,19 @@ static void tcf_chain_put_explicitly_created(struct tcf_chain *chain) tcf_chain_put(chain); } +static void tcf_chain_flush(struct tcf_chain *chain) +{ + struct tcf_proto *tp = rtnl_dereference(chain->filter_chain); + + tcf_chain0_head_change(chain, NULL); + while (tp) { + RCU_INIT_POINTER(chain->filter_chain, tp->next); + tcf_proto_destroy(tp, NULL); + tp = rtnl_dereference(chain->filter_chain); + tcf_chain_put(chain); + } +} + static bool tcf_block_offload_in_use(struct tcf_block *block) { return block->offloadcnt; -- cgit v1.2.3