* [patch net-next 0/2] net: sched: couple of adjustments/fixes
@ 2018-07-31 12:07 Jiri Pirko
2018-07-31 12:07 ` [patch net-next 1/2] net: sched: change name of zombie chain to "held_by_acts_only" Jiri Pirko
2018-07-31 12:07 ` [patch net-next 2/2] net: sched: fix notifications for action-held chains Jiri Pirko
0 siblings, 2 replies; 6+ messages in thread
From: Jiri Pirko @ 2018-07-31 12:07 UTC (permalink / raw)
To: netdev; +Cc: davem, jhs, xiyou.wangcong, jakub.kicinski, mlxsw
From: Jiri Pirko <jiri@mellanox.com>
Jiri Pirko (2):
net: sched: change name of zombie chain to "held_by_acts_only"
net: sched: fix notifications for action-held chains
net/sched/cls_api.c | 84 ++++++++++++++++++++++++++++++-----------------------
1 file changed, 48 insertions(+), 36 deletions(-)
--
2.14.4
^ permalink raw reply [flat|nested] 6+ messages in thread
* [patch net-next 1/2] net: sched: change name of zombie chain to "held_by_acts_only"
2018-07-31 12:07 [patch net-next 0/2] net: sched: couple of adjustments/fixes Jiri Pirko
@ 2018-07-31 12:07 ` Jiri Pirko
2018-08-01 4:26 ` Cong Wang
2018-07-31 12:07 ` [patch net-next 2/2] net: sched: fix notifications for action-held chains Jiri Pirko
1 sibling, 1 reply; 6+ messages in thread
From: Jiri Pirko @ 2018-07-31 12:07 UTC (permalink / raw)
To: netdev; +Cc: davem, jhs, xiyou.wangcong, jakub.kicinski, mlxsw
From: Jiri Pirko <jiri@mellanox.com>
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 <jiri@mellanox.com>
Suggested-by: Cong Wang <xiyou.wangcong@gmail.com>
Suggested-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
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 @@ static int tc_ctl_chain(struct sk_buff *skb, struct nlmsghdr *n,
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 @@ static int tc_ctl_chain(struct sk_buff *skb, struct nlmsghdr *n,
}
}
} 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,
--
2.14.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [patch net-next 2/2] net: sched: fix notifications for action-held chains
2018-07-31 12:07 [patch net-next 0/2] net: sched: couple of adjustments/fixes Jiri Pirko
2018-07-31 12:07 ` [patch net-next 1/2] net: sched: change name of zombie chain to "held_by_acts_only" Jiri Pirko
@ 2018-07-31 12:07 ` Jiri Pirko
2018-08-01 4:27 ` Cong Wang
1 sibling, 1 reply; 6+ messages in thread
From: Jiri Pirko @ 2018-07-31 12:07 UTC (permalink / raw)
To: netdev; +Cc: davem, jhs, xiyou.wangcong, jakub.kicinski, mlxsw
From: Jiri Pirko <jiri@mellanox.com>
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 <xiyou.wangcong@gmail.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
net/sched/cls_api.c | 70 ++++++++++++++++++++++++++++++++---------------------
1 file changed, 42 insertions(+), 28 deletions(-)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 2f78341f2888..ac9b4148878d 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,76 @@ 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)
+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)
+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);
--
2.14.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [patch net-next 1/2] net: sched: change name of zombie chain to "held_by_acts_only"
2018-07-31 12:07 ` [patch net-next 1/2] net: sched: change name of zombie chain to "held_by_acts_only" Jiri Pirko
@ 2018-08-01 4:26 ` Cong Wang
0 siblings, 0 replies; 6+ messages in thread
From: Cong Wang @ 2018-08-01 4:26 UTC (permalink / raw)
To: Jiri Pirko
Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
Jakub Kicinski, mlxsw
On Tue, Jul 31, 2018 at 5:10 AM Jiri Pirko <jiri@resnulli.us> wrote:
>
> From: Jiri Pirko <jiri@mellanox.com>
>
> 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 <jiri@mellanox.com>
> Suggested-by: Cong Wang <xiyou.wangcong@gmail.com>
> Suggested-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
Thanks for the update!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch net-next 2/2] net: sched: fix notifications for action-held chains
2018-07-31 12:07 ` [patch net-next 2/2] net: sched: fix notifications for action-held chains Jiri Pirko
@ 2018-08-01 4:27 ` Cong Wang
2018-08-01 9:24 ` Jiri Pirko
0 siblings, 1 reply; 6+ messages in thread
From: Cong Wang @ 2018-08-01 4:27 UTC (permalink / raw)
To: Jiri Pirko
Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
Jakub Kicinski, mlxsw
On Tue, Jul 31, 2018 at 5:10 AM Jiri Pirko <jiri@resnulli.us> wrote:
>
> From: Jiri Pirko <jiri@mellanox.com>
>
> 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 <xiyou.wangcong@gmail.com>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
I think __tcf_chain_{get,put}() can be static.
Other than that,
Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch net-next 2/2] net: sched: fix notifications for action-held chains
2018-08-01 4:27 ` Cong Wang
@ 2018-08-01 9:24 ` Jiri Pirko
0 siblings, 0 replies; 6+ messages in thread
From: Jiri Pirko @ 2018-08-01 9:24 UTC (permalink / raw)
To: Cong Wang
Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
Jakub Kicinski, mlxsw
Wed, Aug 01, 2018 at 06:27:28AM CEST, xiyou.wangcong@gmail.com wrote:
>On Tue, Jul 31, 2018 at 5:10 AM Jiri Pirko <jiri@resnulli.us> wrote:
>>
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> 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 <xiyou.wangcong@gmail.com>
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>
>I think __tcf_chain_{get,put}() can be static.
In fact, tcf_chain_get/put could be now static too. Will send v2.
Thanks!
>
>Other than that,
>
>Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
>
>Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-08-01 11:12 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-07-31 12:07 [patch net-next 0/2] net: sched: couple of adjustments/fixes Jiri Pirko
2018-07-31 12:07 ` [patch net-next 1/2] net: sched: change name of zombie chain to "held_by_acts_only" Jiri Pirko
2018-08-01 4:26 ` Cong Wang
2018-07-31 12:07 ` [patch net-next 2/2] net: sched: fix notifications for action-held chains Jiri Pirko
2018-08-01 4:27 ` Cong Wang
2018-08-01 9:24 ` Jiri Pirko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).