* [PATCH net-next] net/sched: introduce ACT_P_BOUND return code
@ 2023-12-29 13:26 Pedro Tammela
2023-12-29 13:26 ` [PATCH net-next] net/sched: sch_api: conditional netlink notifications Pedro Tammela
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Pedro Tammela @ 2023-12-29 13:26 UTC (permalink / raw)
To: netdev
Cc: davem, edumazet, kuba, pabeni, jhs, xiyou.wangcong, jiri,
Pedro Tammela
Bound actions always return '0' and as of today we rely on '0'
being returned in order to properly skip bound actions in
tcf_idr_insert_many. In order to further improve maintainability,
introduce the ACT_P_BOUND return code.
Actions are updated to return 'ACT_P_BOUND' instead of plain '0'.
tcf_idr_insert_many is then updated to check for 'ACT_P_BOUND'.
Signed-off-by: Pedro Tammela <pctammela@mojatatu•com>
---
include/net/act_api.h | 1 +
net/sched/act_api.c | 2 +-
net/sched/act_bpf.c | 2 +-
net/sched/act_connmark.c | 2 +-
net/sched/act_csum.c | 4 ++--
net/sched/act_ct.c | 2 +-
net/sched/act_ctinfo.c | 2 +-
net/sched/act_gact.c | 2 +-
net/sched/act_gate.c | 2 +-
net/sched/act_ife.c | 2 +-
net/sched/act_mirred.c | 2 +-
net/sched/act_mpls.c | 2 +-
net/sched/act_nat.c | 2 +-
net/sched/act_pedit.c | 2 +-
net/sched/act_police.c | 2 +-
net/sched/act_sample.c | 2 +-
net/sched/act_simple.c | 2 +-
net/sched/act_skbedit.c | 2 +-
net/sched/act_skbmod.c | 2 +-
net/sched/act_tunnel_key.c | 2 +-
net/sched/act_vlan.c | 2 +-
21 files changed, 22 insertions(+), 21 deletions(-)
diff --git a/include/net/act_api.h b/include/net/act_api.h
index ea13e1e4a7c2..447985a45ef6 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -137,6 +137,7 @@ struct tc_action_ops {
#ifdef CONFIG_NET_CLS_ACT
+#define ACT_P_BOUND 0
#define ACT_P_CREATED 1
#define ACT_P_DELETED 1
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index a44c097a880d..ef70d4771811 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -1313,7 +1313,7 @@ void tcf_idr_insert_many(struct tc_action *actions[], int init_res[])
tcf_act_for_each_action(i, a, actions) {
struct tcf_idrinfo *idrinfo;
- if (init_res[i] == 0) /* Bound */
+ if (init_res[i] == ACT_P_BOUND)
continue;
idrinfo = a->idrinfo;
diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c
index b0455fda7d0b..6cfee6658103 100644
--- a/net/sched/act_bpf.c
+++ b/net/sched/act_bpf.c
@@ -318,7 +318,7 @@ static int tcf_bpf_init(struct net *net, struct nlattr *nla,
} else if (ret > 0) {
/* Don't override defaults. */
if (bind)
- return 0;
+ return ACT_P_BOUND;
if (!(flags & TCA_ACT_FLAGS_REPLACE)) {
tcf_idr_release(*act, bind);
diff --git a/net/sched/act_connmark.c b/net/sched/act_connmark.c
index 0d7aee8933c5..f8762756657d 100644
--- a/net/sched/act_connmark.c
+++ b/net/sched/act_connmark.c
@@ -146,7 +146,7 @@ static int tcf_connmark_init(struct net *net, struct nlattr *nla,
} else if (ret > 0) {
ci = to_connmark(*a);
if (bind) {
- err = 0;
+ err = ACT_P_BOUND;
goto out_free;
}
if (!(flags & TCA_ACT_FLAGS_REPLACE)) {
diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
index 8ed285023a40..7f8b1f2f2ed9 100644
--- a/net/sched/act_csum.c
+++ b/net/sched/act_csum.c
@@ -77,8 +77,8 @@ static int tcf_csum_init(struct net *net, struct nlattr *nla,
}
ret = ACT_P_CREATED;
} else if (err > 0) {
- if (bind)/* dont override defaults */
- return 0;
+ if (bind) /* dont override defaults */
+ return ACT_P_BOUND;
if (!(flags & TCA_ACT_FLAGS_REPLACE)) {
tcf_idr_release(*a, bind);
return -EEXIST;
diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c
index f69c47945175..c3e004b5b820 100644
--- a/net/sched/act_ct.c
+++ b/net/sched/act_ct.c
@@ -1349,7 +1349,7 @@ static int tcf_ct_init(struct net *net, struct nlattr *nla,
res = ACT_P_CREATED;
} else {
if (bind)
- return 0;
+ return ACT_P_BOUND;
if (!(flags & TCA_ACT_FLAGS_REPLACE)) {
tcf_idr_release(*a, bind);
diff --git a/net/sched/act_ctinfo.c b/net/sched/act_ctinfo.c
index 4d15b6a6169c..e620f9a84afe 100644
--- a/net/sched/act_ctinfo.c
+++ b/net/sched/act_ctinfo.c
@@ -221,7 +221,7 @@ static int tcf_ctinfo_init(struct net *net, struct nlattr *nla,
ret = ACT_P_CREATED;
} else if (err > 0) {
if (bind) /* don't override defaults */
- return 0;
+ return ACT_P_BOUND;
if (!(flags & TCA_ACT_FLAGS_REPLACE)) {
tcf_idr_release(*a, bind);
return -EEXIST;
diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
index 904ab3d457ef..4af3b7ec249f 100644
--- a/net/sched/act_gact.c
+++ b/net/sched/act_gact.c
@@ -108,7 +108,7 @@ static int tcf_gact_init(struct net *net, struct nlattr *nla,
ret = ACT_P_CREATED;
} else if (err > 0) {
if (bind)/* dont override defaults */
- return 0;
+ return ACT_P_BOUND;
if (!(flags & TCA_ACT_FLAGS_REPLACE)) {
tcf_idr_release(*a, bind);
return -EEXIST;
diff --git a/net/sched/act_gate.c b/net/sched/act_gate.c
index 393b78729216..c681cd011afd 100644
--- a/net/sched/act_gate.c
+++ b/net/sched/act_gate.c
@@ -356,7 +356,7 @@ static int tcf_gate_init(struct net *net, struct nlattr *nla,
return err;
if (err && bind)
- return 0;
+ return ACT_P_BOUND;
if (!err) {
ret = tcf_idr_create_from_flags(tn, index, est, a,
diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index bc7611b0744c..0e867d13beb5 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -548,7 +548,7 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
exists = err;
if (exists && bind) {
kfree(p);
- return 0;
+ return ACT_P_BOUND;
}
if (!exists) {
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index d1f9794ca9b7..12386f590b0f 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -135,7 +135,7 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
return err;
exists = err;
if (exists && bind)
- return 0;
+ return ACT_P_BOUND;
if (tb[TCA_MIRRED_BLOCKID] && parm->ifindex) {
NL_SET_ERR_MSG_MOD(extack,
diff --git a/net/sched/act_mpls.c b/net/sched/act_mpls.c
index 1010dc632874..34b8edb6cc77 100644
--- a/net/sched/act_mpls.c
+++ b/net/sched/act_mpls.c
@@ -195,7 +195,7 @@ static int tcf_mpls_init(struct net *net, struct nlattr *nla,
return err;
exists = err;
if (exists && bind)
- return 0;
+ return ACT_P_BOUND;
if (!exists) {
ret = tcf_idr_create(tn, index, est, a, &act_mpls_ops, bind,
diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
index 4184af5abbf3..a180e724634e 100644
--- a/net/sched/act_nat.c
+++ b/net/sched/act_nat.c
@@ -69,7 +69,7 @@ static int tcf_nat_init(struct net *net, struct nlattr *nla, struct nlattr *est,
ret = ACT_P_CREATED;
} else if (err > 0) {
if (bind)
- return 0;
+ return ACT_P_BOUND;
if (!(flags & TCA_ACT_FLAGS_REPLACE)) {
tcf_idr_release(*a, bind);
return -EEXIST;
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index 1ef8fcfa9997..2ef22969f274 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -202,7 +202,7 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
ret = ACT_P_CREATED;
} else if (err > 0) {
if (bind)
- return 0;
+ return ACT_P_BOUND;
if (!(flags & TCA_ACT_FLAGS_REPLACE)) {
ret = -EEXIST;
goto out_release;
diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index f3121c5a85e9..e119b4a3db9f 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -77,7 +77,7 @@ static int tcf_police_init(struct net *net, struct nlattr *nla,
return err;
exists = err;
if (exists && bind)
- return 0;
+ return ACT_P_BOUND;
if (!exists) {
ret = tcf_idr_create(tn, index, NULL, a,
diff --git a/net/sched/act_sample.c b/net/sched/act_sample.c
index 4c670e7568dc..c5c61efe6db4 100644
--- a/net/sched/act_sample.c
+++ b/net/sched/act_sample.c
@@ -66,7 +66,7 @@ static int tcf_sample_init(struct net *net, struct nlattr *nla,
return err;
exists = err;
if (exists && bind)
- return 0;
+ return ACT_P_BOUND;
if (!exists) {
ret = tcf_idr_create(tn, index, est, a,
diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c
index 4b84514534f3..0a3e92888295 100644
--- a/net/sched/act_simple.c
+++ b/net/sched/act_simple.c
@@ -118,7 +118,7 @@ static int tcf_simp_init(struct net *net, struct nlattr *nla,
return err;
exists = err;
if (exists && bind)
- return 0;
+ return ACT_P_BOUND;
if (tb[TCA_DEF_DATA] == NULL) {
if (exists)
diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index ce7008cf291c..754f78b35bb8 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -209,7 +209,7 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
return err;
exists = err;
if (exists && bind)
- return 0;
+ return ACT_P_BOUND;
if (!flags) {
if (exists)
diff --git a/net/sched/act_skbmod.c b/net/sched/act_skbmod.c
index dffa990a9629..bcb673ab0008 100644
--- a/net/sched/act_skbmod.c
+++ b/net/sched/act_skbmod.c
@@ -157,7 +157,7 @@ static int tcf_skbmod_init(struct net *net, struct nlattr *nla,
return err;
exists = err;
if (exists && bind)
- return 0;
+ return ACT_P_BOUND;
if (!lflags) {
if (exists)
diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c
index 0c8aa7e686ea..300b08aa8283 100644
--- a/net/sched/act_tunnel_key.c
+++ b/net/sched/act_tunnel_key.c
@@ -401,7 +401,7 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
return err;
exists = err;
if (exists && bind)
- return 0;
+ return ACT_P_BOUND;
switch (parm->t_action) {
case TCA_TUNNEL_KEY_ACT_RELEASE:
diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
index 0251442f5f29..836183011a7c 100644
--- a/net/sched/act_vlan.c
+++ b/net/sched/act_vlan.c
@@ -151,7 +151,7 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
return err;
exists = err;
if (exists && bind)
- return 0;
+ return ACT_P_BOUND;
switch (parm->v_action) {
case TCA_VLAN_ACT_POP:
--
2.40.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net-next] net/sched: sch_api: conditional netlink notifications
2023-12-29 13:26 [PATCH net-next] net/sched: introduce ACT_P_BOUND return code Pedro Tammela
@ 2023-12-29 13:26 ` Pedro Tammela
2023-12-31 19:01 ` Jamal Hadi Salim
2024-01-04 3:00 ` patchwork-bot+netdevbpf
2023-12-31 19:00 ` [PATCH net-next] net/sched: introduce ACT_P_BOUND return code Jamal Hadi Salim
2024-01-04 3:00 ` patchwork-bot+netdevbpf
2 siblings, 2 replies; 6+ messages in thread
From: Pedro Tammela @ 2023-12-29 13:26 UTC (permalink / raw)
To: netdev
Cc: davem, edumazet, kuba, pabeni, jhs, xiyou.wangcong, jiri,
Pedro Tammela
Implement conditional netlink notifications for Qdiscs and classes,
which were missing in the initial patches that targeted tc filters and
actions. Notifications will only be built after passing a check for
'rtnl_notify_needed()'.
For both Qdiscs and classes 'get' operations now call a dedicated
notification function as it was not possible to distinguish between
'create' and 'get' before. This distinction is necessary because 'get'
always send a notification.
Signed-off-by: Pedro Tammela <pctammela@mojatatu•com>
---
net/sched/sch_api.c | 79 ++++++++++++++++++++++++++++++++++++++-------
1 file changed, 68 insertions(+), 11 deletions(-)
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 299086bb6205..2a2a48838eb9 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1003,6 +1003,32 @@ static bool tc_qdisc_dump_ignore(struct Qdisc *q, bool dump_invisible)
return false;
}
+static int qdisc_get_notify(struct net *net, struct sk_buff *oskb,
+ struct nlmsghdr *n, u32 clid, struct Qdisc *q,
+ struct netlink_ext_ack *extack)
+{
+ struct sk_buff *skb;
+ u32 portid = oskb ? NETLINK_CB(oskb).portid : 0;
+
+ skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
+ if (!skb)
+ return -ENOBUFS;
+
+ if (!tc_qdisc_dump_ignore(q, false)) {
+ if (tc_fill_qdisc(skb, q, clid, portid, n->nlmsg_seq, 0,
+ RTM_NEWQDISC, extack) < 0)
+ goto err_out;
+ }
+
+ if (skb->len)
+ return rtnetlink_send(skb, net, portid, RTNLGRP_TC,
+ n->nlmsg_flags & NLM_F_ECHO);
+
+err_out:
+ kfree_skb(skb);
+ return -EINVAL;
+}
+
static int qdisc_notify(struct net *net, struct sk_buff *oskb,
struct nlmsghdr *n, u32 clid,
struct Qdisc *old, struct Qdisc *new,
@@ -1011,6 +1037,9 @@ static int qdisc_notify(struct net *net, struct sk_buff *oskb,
struct sk_buff *skb;
u32 portid = oskb ? NETLINK_CB(oskb).portid : 0;
+ if (!rtnl_notify_needed(net, n->nlmsg_flags, RTNLGRP_TC))
+ return 0;
+
skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
if (!skb)
return -ENOBUFS;
@@ -1583,7 +1612,7 @@ static int tc_get_qdisc(struct sk_buff *skb, struct nlmsghdr *n,
if (err != 0)
return err;
} else {
- qdisc_notify(net, skb, n, clid, NULL, q, NULL);
+ qdisc_get_notify(net, skb, n, clid, q, NULL);
}
return 0;
}
@@ -1977,6 +2006,9 @@ static int tclass_notify(struct net *net, struct sk_buff *oskb,
struct sk_buff *skb;
u32 portid = oskb ? NETLINK_CB(oskb).portid : 0;
+ if (!rtnl_notify_needed(net, n->nlmsg_flags, RTNLGRP_TC))
+ return 0;
+
skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
if (!skb)
return -ENOBUFS;
@@ -1990,6 +2022,27 @@ static int tclass_notify(struct net *net, struct sk_buff *oskb,
n->nlmsg_flags & NLM_F_ECHO);
}
+static int tclass_get_notify(struct net *net, struct sk_buff *oskb,
+ struct nlmsghdr *n, struct Qdisc *q,
+ unsigned long cl, struct netlink_ext_ack *extack)
+{
+ struct sk_buff *skb;
+ u32 portid = oskb ? NETLINK_CB(oskb).portid : 0;
+
+ skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
+ if (!skb)
+ return -ENOBUFS;
+
+ if (tc_fill_tclass(skb, q, cl, portid, n->nlmsg_seq, 0, RTM_NEWTCLASS,
+ extack) < 0) {
+ kfree_skb(skb);
+ return -EINVAL;
+ }
+
+ return rtnetlink_send(skb, net, portid, RTNLGRP_TC,
+ n->nlmsg_flags & NLM_F_ECHO);
+}
+
static int tclass_del_notify(struct net *net,
const struct Qdisc_class_ops *cops,
struct sk_buff *oskb, struct nlmsghdr *n,
@@ -2003,14 +2056,18 @@ static int tclass_del_notify(struct net *net,
if (!cops->delete)
return -EOPNOTSUPP;
- skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
- if (!skb)
- return -ENOBUFS;
+ if (rtnl_notify_needed(net, n->nlmsg_flags, RTNLGRP_TC)) {
+ skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
+ if (!skb)
+ return -ENOBUFS;
- if (tc_fill_tclass(skb, q, cl, portid, n->nlmsg_seq, 0,
- RTM_DELTCLASS, extack) < 0) {
- kfree_skb(skb);
- return -EINVAL;
+ if (tc_fill_tclass(skb, q, cl, portid, n->nlmsg_seq, 0,
+ RTM_DELTCLASS, extack) < 0) {
+ kfree_skb(skb);
+ return -EINVAL;
+ }
+ } else {
+ skb = NULL;
}
err = cops->delete(q, cl, extack);
@@ -2019,8 +2076,8 @@ static int tclass_del_notify(struct net *net,
return err;
}
- err = rtnetlink_send(skb, net, portid, RTNLGRP_TC,
- n->nlmsg_flags & NLM_F_ECHO);
+ err = rtnetlink_maybe_send(skb, net, portid, RTNLGRP_TC,
+ n->nlmsg_flags & NLM_F_ECHO);
return err;
}
@@ -2215,7 +2272,7 @@ static int tc_ctl_tclass(struct sk_buff *skb, struct nlmsghdr *n,
tc_bind_tclass(q, portid, clid, 0);
goto out;
case RTM_GETTCLASS:
- err = tclass_notify(net, skb, n, q, cl, RTM_NEWTCLASS, extack);
+ err = tclass_get_notify(net, skb, n, q, cl, extack);
goto out;
default:
err = -EINVAL;
--
2.40.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net/sched: introduce ACT_P_BOUND return code
2023-12-29 13:26 [PATCH net-next] net/sched: introduce ACT_P_BOUND return code Pedro Tammela
2023-12-29 13:26 ` [PATCH net-next] net/sched: sch_api: conditional netlink notifications Pedro Tammela
@ 2023-12-31 19:00 ` Jamal Hadi Salim
2024-01-04 3:00 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 6+ messages in thread
From: Jamal Hadi Salim @ 2023-12-31 19:00 UTC (permalink / raw)
To: Pedro Tammela; +Cc: netdev, davem, edumazet, kuba, pabeni, xiyou.wangcong, jiri
On Fri, Dec 29, 2023 at 8:27 AM Pedro Tammela <pctammela@mojatatu•com> wrote:
>
> Bound actions always return '0' and as of today we rely on '0'
> being returned in order to properly skip bound actions in
> tcf_idr_insert_many. In order to further improve maintainability,
> introduce the ACT_P_BOUND return code.
>
> Actions are updated to return 'ACT_P_BOUND' instead of plain '0'.
> tcf_idr_insert_many is then updated to check for 'ACT_P_BOUND'.
>
> Signed-off-by: Pedro Tammela <pctammela@mojatatu•com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu•com>
cheers,
jamal
> ---
> include/net/act_api.h | 1 +
> net/sched/act_api.c | 2 +-
> net/sched/act_bpf.c | 2 +-
> net/sched/act_connmark.c | 2 +-
> net/sched/act_csum.c | 4 ++--
> net/sched/act_ct.c | 2 +-
> net/sched/act_ctinfo.c | 2 +-
> net/sched/act_gact.c | 2 +-
> net/sched/act_gate.c | 2 +-
> net/sched/act_ife.c | 2 +-
> net/sched/act_mirred.c | 2 +-
> net/sched/act_mpls.c | 2 +-
> net/sched/act_nat.c | 2 +-
> net/sched/act_pedit.c | 2 +-
> net/sched/act_police.c | 2 +-
> net/sched/act_sample.c | 2 +-
> net/sched/act_simple.c | 2 +-
> net/sched/act_skbedit.c | 2 +-
> net/sched/act_skbmod.c | 2 +-
> net/sched/act_tunnel_key.c | 2 +-
> net/sched/act_vlan.c | 2 +-
> 21 files changed, 22 insertions(+), 21 deletions(-)
>
> diff --git a/include/net/act_api.h b/include/net/act_api.h
> index ea13e1e4a7c2..447985a45ef6 100644
> --- a/include/net/act_api.h
> +++ b/include/net/act_api.h
> @@ -137,6 +137,7 @@ struct tc_action_ops {
>
> #ifdef CONFIG_NET_CLS_ACT
>
> +#define ACT_P_BOUND 0
> #define ACT_P_CREATED 1
> #define ACT_P_DELETED 1
>
> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
> index a44c097a880d..ef70d4771811 100644
> --- a/net/sched/act_api.c
> +++ b/net/sched/act_api.c
> @@ -1313,7 +1313,7 @@ void tcf_idr_insert_many(struct tc_action *actions[], int init_res[])
> tcf_act_for_each_action(i, a, actions) {
> struct tcf_idrinfo *idrinfo;
>
> - if (init_res[i] == 0) /* Bound */
> + if (init_res[i] == ACT_P_BOUND)
> continue;
>
> idrinfo = a->idrinfo;
> diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c
> index b0455fda7d0b..6cfee6658103 100644
> --- a/net/sched/act_bpf.c
> +++ b/net/sched/act_bpf.c
> @@ -318,7 +318,7 @@ static int tcf_bpf_init(struct net *net, struct nlattr *nla,
> } else if (ret > 0) {
> /* Don't override defaults. */
> if (bind)
> - return 0;
> + return ACT_P_BOUND;
>
> if (!(flags & TCA_ACT_FLAGS_REPLACE)) {
> tcf_idr_release(*act, bind);
> diff --git a/net/sched/act_connmark.c b/net/sched/act_connmark.c
> index 0d7aee8933c5..f8762756657d 100644
> --- a/net/sched/act_connmark.c
> +++ b/net/sched/act_connmark.c
> @@ -146,7 +146,7 @@ static int tcf_connmark_init(struct net *net, struct nlattr *nla,
> } else if (ret > 0) {
> ci = to_connmark(*a);
> if (bind) {
> - err = 0;
> + err = ACT_P_BOUND;
> goto out_free;
> }
> if (!(flags & TCA_ACT_FLAGS_REPLACE)) {
> diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
> index 8ed285023a40..7f8b1f2f2ed9 100644
> --- a/net/sched/act_csum.c
> +++ b/net/sched/act_csum.c
> @@ -77,8 +77,8 @@ static int tcf_csum_init(struct net *net, struct nlattr *nla,
> }
> ret = ACT_P_CREATED;
> } else if (err > 0) {
> - if (bind)/* dont override defaults */
> - return 0;
> + if (bind) /* dont override defaults */
> + return ACT_P_BOUND;
> if (!(flags & TCA_ACT_FLAGS_REPLACE)) {
> tcf_idr_release(*a, bind);
> return -EEXIST;
> diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c
> index f69c47945175..c3e004b5b820 100644
> --- a/net/sched/act_ct.c
> +++ b/net/sched/act_ct.c
> @@ -1349,7 +1349,7 @@ static int tcf_ct_init(struct net *net, struct nlattr *nla,
> res = ACT_P_CREATED;
> } else {
> if (bind)
> - return 0;
> + return ACT_P_BOUND;
>
> if (!(flags & TCA_ACT_FLAGS_REPLACE)) {
> tcf_idr_release(*a, bind);
> diff --git a/net/sched/act_ctinfo.c b/net/sched/act_ctinfo.c
> index 4d15b6a6169c..e620f9a84afe 100644
> --- a/net/sched/act_ctinfo.c
> +++ b/net/sched/act_ctinfo.c
> @@ -221,7 +221,7 @@ static int tcf_ctinfo_init(struct net *net, struct nlattr *nla,
> ret = ACT_P_CREATED;
> } else if (err > 0) {
> if (bind) /* don't override defaults */
> - return 0;
> + return ACT_P_BOUND;
> if (!(flags & TCA_ACT_FLAGS_REPLACE)) {
> tcf_idr_release(*a, bind);
> return -EEXIST;
> diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
> index 904ab3d457ef..4af3b7ec249f 100644
> --- a/net/sched/act_gact.c
> +++ b/net/sched/act_gact.c
> @@ -108,7 +108,7 @@ static int tcf_gact_init(struct net *net, struct nlattr *nla,
> ret = ACT_P_CREATED;
> } else if (err > 0) {
> if (bind)/* dont override defaults */
> - return 0;
> + return ACT_P_BOUND;
> if (!(flags & TCA_ACT_FLAGS_REPLACE)) {
> tcf_idr_release(*a, bind);
> return -EEXIST;
> diff --git a/net/sched/act_gate.c b/net/sched/act_gate.c
> index 393b78729216..c681cd011afd 100644
> --- a/net/sched/act_gate.c
> +++ b/net/sched/act_gate.c
> @@ -356,7 +356,7 @@ static int tcf_gate_init(struct net *net, struct nlattr *nla,
> return err;
>
> if (err && bind)
> - return 0;
> + return ACT_P_BOUND;
>
> if (!err) {
> ret = tcf_idr_create_from_flags(tn, index, est, a,
> diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
> index bc7611b0744c..0e867d13beb5 100644
> --- a/net/sched/act_ife.c
> +++ b/net/sched/act_ife.c
> @@ -548,7 +548,7 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
> exists = err;
> if (exists && bind) {
> kfree(p);
> - return 0;
> + return ACT_P_BOUND;
> }
>
> if (!exists) {
> diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
> index d1f9794ca9b7..12386f590b0f 100644
> --- a/net/sched/act_mirred.c
> +++ b/net/sched/act_mirred.c
> @@ -135,7 +135,7 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
> return err;
> exists = err;
> if (exists && bind)
> - return 0;
> + return ACT_P_BOUND;
>
> if (tb[TCA_MIRRED_BLOCKID] && parm->ifindex) {
> NL_SET_ERR_MSG_MOD(extack,
> diff --git a/net/sched/act_mpls.c b/net/sched/act_mpls.c
> index 1010dc632874..34b8edb6cc77 100644
> --- a/net/sched/act_mpls.c
> +++ b/net/sched/act_mpls.c
> @@ -195,7 +195,7 @@ static int tcf_mpls_init(struct net *net, struct nlattr *nla,
> return err;
> exists = err;
> if (exists && bind)
> - return 0;
> + return ACT_P_BOUND;
>
> if (!exists) {
> ret = tcf_idr_create(tn, index, est, a, &act_mpls_ops, bind,
> diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
> index 4184af5abbf3..a180e724634e 100644
> --- a/net/sched/act_nat.c
> +++ b/net/sched/act_nat.c
> @@ -69,7 +69,7 @@ static int tcf_nat_init(struct net *net, struct nlattr *nla, struct nlattr *est,
> ret = ACT_P_CREATED;
> } else if (err > 0) {
> if (bind)
> - return 0;
> + return ACT_P_BOUND;
> if (!(flags & TCA_ACT_FLAGS_REPLACE)) {
> tcf_idr_release(*a, bind);
> return -EEXIST;
> diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
> index 1ef8fcfa9997..2ef22969f274 100644
> --- a/net/sched/act_pedit.c
> +++ b/net/sched/act_pedit.c
> @@ -202,7 +202,7 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
> ret = ACT_P_CREATED;
> } else if (err > 0) {
> if (bind)
> - return 0;
> + return ACT_P_BOUND;
> if (!(flags & TCA_ACT_FLAGS_REPLACE)) {
> ret = -EEXIST;
> goto out_release;
> diff --git a/net/sched/act_police.c b/net/sched/act_police.c
> index f3121c5a85e9..e119b4a3db9f 100644
> --- a/net/sched/act_police.c
> +++ b/net/sched/act_police.c
> @@ -77,7 +77,7 @@ static int tcf_police_init(struct net *net, struct nlattr *nla,
> return err;
> exists = err;
> if (exists && bind)
> - return 0;
> + return ACT_P_BOUND;
>
> if (!exists) {
> ret = tcf_idr_create(tn, index, NULL, a,
> diff --git a/net/sched/act_sample.c b/net/sched/act_sample.c
> index 4c670e7568dc..c5c61efe6db4 100644
> --- a/net/sched/act_sample.c
> +++ b/net/sched/act_sample.c
> @@ -66,7 +66,7 @@ static int tcf_sample_init(struct net *net, struct nlattr *nla,
> return err;
> exists = err;
> if (exists && bind)
> - return 0;
> + return ACT_P_BOUND;
>
> if (!exists) {
> ret = tcf_idr_create(tn, index, est, a,
> diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c
> index 4b84514534f3..0a3e92888295 100644
> --- a/net/sched/act_simple.c
> +++ b/net/sched/act_simple.c
> @@ -118,7 +118,7 @@ static int tcf_simp_init(struct net *net, struct nlattr *nla,
> return err;
> exists = err;
> if (exists && bind)
> - return 0;
> + return ACT_P_BOUND;
>
> if (tb[TCA_DEF_DATA] == NULL) {
> if (exists)
> diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
> index ce7008cf291c..754f78b35bb8 100644
> --- a/net/sched/act_skbedit.c
> +++ b/net/sched/act_skbedit.c
> @@ -209,7 +209,7 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
> return err;
> exists = err;
> if (exists && bind)
> - return 0;
> + return ACT_P_BOUND;
>
> if (!flags) {
> if (exists)
> diff --git a/net/sched/act_skbmod.c b/net/sched/act_skbmod.c
> index dffa990a9629..bcb673ab0008 100644
> --- a/net/sched/act_skbmod.c
> +++ b/net/sched/act_skbmod.c
> @@ -157,7 +157,7 @@ static int tcf_skbmod_init(struct net *net, struct nlattr *nla,
> return err;
> exists = err;
> if (exists && bind)
> - return 0;
> + return ACT_P_BOUND;
>
> if (!lflags) {
> if (exists)
> diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c
> index 0c8aa7e686ea..300b08aa8283 100644
> --- a/net/sched/act_tunnel_key.c
> +++ b/net/sched/act_tunnel_key.c
> @@ -401,7 +401,7 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
> return err;
> exists = err;
> if (exists && bind)
> - return 0;
> + return ACT_P_BOUND;
>
> switch (parm->t_action) {
> case TCA_TUNNEL_KEY_ACT_RELEASE:
> diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
> index 0251442f5f29..836183011a7c 100644
> --- a/net/sched/act_vlan.c
> +++ b/net/sched/act_vlan.c
> @@ -151,7 +151,7 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
> return err;
> exists = err;
> if (exists && bind)
> - return 0;
> + return ACT_P_BOUND;
>
> switch (parm->v_action) {
> case TCA_VLAN_ACT_POP:
> --
> 2.40.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net/sched: sch_api: conditional netlink notifications
2023-12-29 13:26 ` [PATCH net-next] net/sched: sch_api: conditional netlink notifications Pedro Tammela
@ 2023-12-31 19:01 ` Jamal Hadi Salim
2024-01-04 3:00 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 6+ messages in thread
From: Jamal Hadi Salim @ 2023-12-31 19:01 UTC (permalink / raw)
To: Pedro Tammela; +Cc: netdev, davem, edumazet, kuba, pabeni, xiyou.wangcong, jiri
On Fri, Dec 29, 2023 at 8:27 AM Pedro Tammela <pctammela@mojatatu•com> wrote:
>
> Implement conditional netlink notifications for Qdiscs and classes,
> which were missing in the initial patches that targeted tc filters and
> actions. Notifications will only be built after passing a check for
> 'rtnl_notify_needed()'.
>
> For both Qdiscs and classes 'get' operations now call a dedicated
> notification function as it was not possible to distinguish between
> 'create' and 'get' before. This distinction is necessary because 'get'
> always send a notification.
>
> Signed-off-by: Pedro Tammela <pctammela@mojatatu•com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu•com>
cheers,
jamal
> ---
> net/sched/sch_api.c | 79 ++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 68 insertions(+), 11 deletions(-)
>
> diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
> index 299086bb6205..2a2a48838eb9 100644
> --- a/net/sched/sch_api.c
> +++ b/net/sched/sch_api.c
> @@ -1003,6 +1003,32 @@ static bool tc_qdisc_dump_ignore(struct Qdisc *q, bool dump_invisible)
> return false;
> }
>
> +static int qdisc_get_notify(struct net *net, struct sk_buff *oskb,
> + struct nlmsghdr *n, u32 clid, struct Qdisc *q,
> + struct netlink_ext_ack *extack)
> +{
> + struct sk_buff *skb;
> + u32 portid = oskb ? NETLINK_CB(oskb).portid : 0;
> +
> + skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
> + if (!skb)
> + return -ENOBUFS;
> +
> + if (!tc_qdisc_dump_ignore(q, false)) {
> + if (tc_fill_qdisc(skb, q, clid, portid, n->nlmsg_seq, 0,
> + RTM_NEWQDISC, extack) < 0)
> + goto err_out;
> + }
> +
> + if (skb->len)
> + return rtnetlink_send(skb, net, portid, RTNLGRP_TC,
> + n->nlmsg_flags & NLM_F_ECHO);
> +
> +err_out:
> + kfree_skb(skb);
> + return -EINVAL;
> +}
> +
> static int qdisc_notify(struct net *net, struct sk_buff *oskb,
> struct nlmsghdr *n, u32 clid,
> struct Qdisc *old, struct Qdisc *new,
> @@ -1011,6 +1037,9 @@ static int qdisc_notify(struct net *net, struct sk_buff *oskb,
> struct sk_buff *skb;
> u32 portid = oskb ? NETLINK_CB(oskb).portid : 0;
>
> + if (!rtnl_notify_needed(net, n->nlmsg_flags, RTNLGRP_TC))
> + return 0;
> +
> skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
> if (!skb)
> return -ENOBUFS;
> @@ -1583,7 +1612,7 @@ static int tc_get_qdisc(struct sk_buff *skb, struct nlmsghdr *n,
> if (err != 0)
> return err;
> } else {
> - qdisc_notify(net, skb, n, clid, NULL, q, NULL);
> + qdisc_get_notify(net, skb, n, clid, q, NULL);
> }
> return 0;
> }
> @@ -1977,6 +2006,9 @@ static int tclass_notify(struct net *net, struct sk_buff *oskb,
> struct sk_buff *skb;
> u32 portid = oskb ? NETLINK_CB(oskb).portid : 0;
>
> + if (!rtnl_notify_needed(net, n->nlmsg_flags, RTNLGRP_TC))
> + return 0;
> +
> skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
> if (!skb)
> return -ENOBUFS;
> @@ -1990,6 +2022,27 @@ static int tclass_notify(struct net *net, struct sk_buff *oskb,
> n->nlmsg_flags & NLM_F_ECHO);
> }
>
> +static int tclass_get_notify(struct net *net, struct sk_buff *oskb,
> + struct nlmsghdr *n, struct Qdisc *q,
> + unsigned long cl, struct netlink_ext_ack *extack)
> +{
> + struct sk_buff *skb;
> + u32 portid = oskb ? NETLINK_CB(oskb).portid : 0;
> +
> + skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
> + if (!skb)
> + return -ENOBUFS;
> +
> + if (tc_fill_tclass(skb, q, cl, portid, n->nlmsg_seq, 0, RTM_NEWTCLASS,
> + extack) < 0) {
> + kfree_skb(skb);
> + return -EINVAL;
> + }
> +
> + return rtnetlink_send(skb, net, portid, RTNLGRP_TC,
> + n->nlmsg_flags & NLM_F_ECHO);
> +}
> +
> static int tclass_del_notify(struct net *net,
> const struct Qdisc_class_ops *cops,
> struct sk_buff *oskb, struct nlmsghdr *n,
> @@ -2003,14 +2056,18 @@ static int tclass_del_notify(struct net *net,
> if (!cops->delete)
> return -EOPNOTSUPP;
>
> - skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
> - if (!skb)
> - return -ENOBUFS;
> + if (rtnl_notify_needed(net, n->nlmsg_flags, RTNLGRP_TC)) {
> + skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
> + if (!skb)
> + return -ENOBUFS;
>
> - if (tc_fill_tclass(skb, q, cl, portid, n->nlmsg_seq, 0,
> - RTM_DELTCLASS, extack) < 0) {
> - kfree_skb(skb);
> - return -EINVAL;
> + if (tc_fill_tclass(skb, q, cl, portid, n->nlmsg_seq, 0,
> + RTM_DELTCLASS, extack) < 0) {
> + kfree_skb(skb);
> + return -EINVAL;
> + }
> + } else {
> + skb = NULL;
> }
>
> err = cops->delete(q, cl, extack);
> @@ -2019,8 +2076,8 @@ static int tclass_del_notify(struct net *net,
> return err;
> }
>
> - err = rtnetlink_send(skb, net, portid, RTNLGRP_TC,
> - n->nlmsg_flags & NLM_F_ECHO);
> + err = rtnetlink_maybe_send(skb, net, portid, RTNLGRP_TC,
> + n->nlmsg_flags & NLM_F_ECHO);
> return err;
> }
>
> @@ -2215,7 +2272,7 @@ static int tc_ctl_tclass(struct sk_buff *skb, struct nlmsghdr *n,
> tc_bind_tclass(q, portid, clid, 0);
> goto out;
> case RTM_GETTCLASS:
> - err = tclass_notify(net, skb, n, q, cl, RTM_NEWTCLASS, extack);
> + err = tclass_get_notify(net, skb, n, q, cl, extack);
> goto out;
> default:
> err = -EINVAL;
> --
> 2.40.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net/sched: introduce ACT_P_BOUND return code
2023-12-29 13:26 [PATCH net-next] net/sched: introduce ACT_P_BOUND return code Pedro Tammela
2023-12-29 13:26 ` [PATCH net-next] net/sched: sch_api: conditional netlink notifications Pedro Tammela
2023-12-31 19:00 ` [PATCH net-next] net/sched: introduce ACT_P_BOUND return code Jamal Hadi Salim
@ 2024-01-04 3:00 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-01-04 3:00 UTC (permalink / raw)
To: Pedro Tammela
Cc: netdev, davem, edumazet, kuba, pabeni, jhs, xiyou.wangcong, jiri
Hello:
This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel•org>:
On Fri, 29 Dec 2023 10:26:41 -0300 you wrote:
> Bound actions always return '0' and as of today we rely on '0'
> being returned in order to properly skip bound actions in
> tcf_idr_insert_many. In order to further improve maintainability,
> introduce the ACT_P_BOUND return code.
>
> Actions are updated to return 'ACT_P_BOUND' instead of plain '0'.
> tcf_idr_insert_many is then updated to check for 'ACT_P_BOUND'.
>
> [...]
Here is the summary with links:
- [net-next] net/sched: introduce ACT_P_BOUND return code
https://git.kernel.org/netdev/net-next/c/c2a67de9bb54
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net/sched: sch_api: conditional netlink notifications
2023-12-29 13:26 ` [PATCH net-next] net/sched: sch_api: conditional netlink notifications Pedro Tammela
2023-12-31 19:01 ` Jamal Hadi Salim
@ 2024-01-04 3:00 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-01-04 3:00 UTC (permalink / raw)
To: Pedro Tammela
Cc: netdev, davem, edumazet, kuba, pabeni, jhs, xiyou.wangcong, jiri
Hello:
This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel•org>:
On Fri, 29 Dec 2023 10:26:42 -0300 you wrote:
> Implement conditional netlink notifications for Qdiscs and classes,
> which were missing in the initial patches that targeted tc filters and
> actions. Notifications will only be built after passing a check for
> 'rtnl_notify_needed()'.
>
> For both Qdiscs and classes 'get' operations now call a dedicated
> notification function as it was not possible to distinguish between
> 'create' and 'get' before. This distinction is necessary because 'get'
> always send a notification.
>
> [...]
Here is the summary with links:
- [net-next] net/sched: sch_api: conditional netlink notifications
https://git.kernel.org/netdev/net-next/c/530496985cea
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-01-04 3:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-29 13:26 [PATCH net-next] net/sched: introduce ACT_P_BOUND return code Pedro Tammela
2023-12-29 13:26 ` [PATCH net-next] net/sched: sch_api: conditional netlink notifications Pedro Tammela
2023-12-31 19:01 ` Jamal Hadi Salim
2024-01-04 3:00 ` patchwork-bot+netdevbpf
2023-12-31 19:00 ` [PATCH net-next] net/sched: introduce ACT_P_BOUND return code Jamal Hadi Salim
2024-01-04 3:00 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox