* [PATCH net-next v4 1/7] rtnl: add helper to check if rtnl group has listeners
2023-12-08 19:28 [PATCH net-next v4 0/7] net/sched: conditional notification of events for cls and act Pedro Tammela
@ 2023-12-08 19:28 ` Pedro Tammela
2023-12-08 19:28 ` [PATCH net-next v4 2/7] rtnl: add helper to check if a notification is needed Pedro Tammela
` (6 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Pedro Tammela @ 2023-12-08 19:28 UTC (permalink / raw)
To: netdev
Cc: davem, edumazet, kuba, pabeni, jhs, xiyou.wangcong, jiri,
marcelo.leitner, vladbu, horms, Jiri Pirko, Victor Nogueira,
Pedro Tammela
From: Jamal Hadi Salim <jhs@mojatatu.com>
As of today, rtnl code creates a new skb and unconditionally fills and
broadcasts it to the relevant group. For most operations this is okay
and doesn't waste resources in general.
When operations are done without the rtnl_lock, as in tc-flower, such
skb allocation, message fill and no-op broadcasting can happen in all
cores of the system, which contributes to system pressure and wastes
precious cpu cycles when no one will receive the built message.
Introduce this helper so rtnetlink operations can simply check if someone
is listening and then proceed if necessary.
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
---
include/linux/rtnetlink.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index 3d6cf306cd55..a7d757e96c55 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -130,4 +130,11 @@ extern int ndo_dflt_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
extern void rtnl_offload_xstats_notify(struct net_device *dev);
+static inline int rtnl_has_listeners(const struct net *net, u32 group)
+{
+ struct sock *rtnl = net->rtnl;
+
+ return netlink_has_listeners(rtnl, group);
+}
+
#endif /* __LINUX_RTNETLINK_H */
--
2.40.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH net-next v4 2/7] rtnl: add helper to check if a notification is needed
2023-12-08 19:28 [PATCH net-next v4 0/7] net/sched: conditional notification of events for cls and act Pedro Tammela
2023-12-08 19:28 ` [PATCH net-next v4 1/7] rtnl: add helper to check if rtnl group has listeners Pedro Tammela
@ 2023-12-08 19:28 ` Pedro Tammela
2023-12-08 19:28 ` [PATCH net-next v4 3/7] rtnl: add helper to send if skb is not null Pedro Tammela
` (5 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Pedro Tammela @ 2023-12-08 19:28 UTC (permalink / raw)
To: netdev
Cc: davem, edumazet, kuba, pabeni, jhs, xiyou.wangcong, jiri,
marcelo.leitner, vladbu, horms, Victor Nogueira, Jiri Pirko,
Pedro Tammela
From: Victor Nogueira <victor@mojatatu.com>
Building on the rtnl_has_listeners helper, add the rtnl_notify_needed
helper to check if we can bail out early in the notification routines.
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
---
include/linux/rtnetlink.h | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index a7d757e96c55..0cbbbded0331 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -137,4 +137,19 @@ static inline int rtnl_has_listeners(const struct net *net, u32 group)
return netlink_has_listeners(rtnl, group);
}
+/**
+ * rtnl_notify_needed - check if notification is needed
+ * @net: Pointer to the net namespace
+ * @nlflags: netlink ingress message flags
+ * @group: rtnl group
+ *
+ * Based on the ingress message flags and rtnl group, returns true
+ * if a notification is needed, false otherwise.
+ */
+static inline bool
+rtnl_notify_needed(const struct net *net, u16 nlflags, u32 group)
+{
+ return (nlflags & NLM_F_ECHO) || rtnl_has_listeners(net, group);
+}
+
#endif /* __LINUX_RTNETLINK_H */
--
2.40.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH net-next v4 3/7] rtnl: add helper to send if skb is not null
2023-12-08 19:28 [PATCH net-next v4 0/7] net/sched: conditional notification of events for cls and act Pedro Tammela
2023-12-08 19:28 ` [PATCH net-next v4 1/7] rtnl: add helper to check if rtnl group has listeners Pedro Tammela
2023-12-08 19:28 ` [PATCH net-next v4 2/7] rtnl: add helper to check if a notification is needed Pedro Tammela
@ 2023-12-08 19:28 ` Pedro Tammela
2023-12-08 19:28 ` [PATCH net-next v4 4/7] net/sched: act_api: don't open code max() Pedro Tammela
` (4 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Pedro Tammela @ 2023-12-08 19:28 UTC (permalink / raw)
To: netdev
Cc: davem, edumazet, kuba, pabeni, jhs, xiyou.wangcong, jiri,
marcelo.leitner, vladbu, horms, Pedro Tammela, Jiri Pirko
This is a convenience helper for routines handling conditional rtnl
events, that is code that might send a notification depending on
rtnl_has_listeners/rtnl_notify_needed.
Instead of:
if (skb)
rtnetlink_send(...)
Use:
rtnetlink_maybe_send(...)
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
---
include/linux/rtnetlink.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index 0cbbbded0331..6a8543b34e2c 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -10,6 +10,13 @@
#include <uapi/linux/rtnetlink.h>
extern int rtnetlink_send(struct sk_buff *skb, struct net *net, u32 pid, u32 group, int echo);
+
+static inline int rtnetlink_maybe_send(struct sk_buff *skb, struct net *net,
+ u32 pid, u32 group, int echo)
+{
+ return !skb ? 0 : rtnetlink_send(skb, net, pid, group, echo);
+}
+
extern int rtnl_unicast(struct sk_buff *skb, struct net *net, u32 pid);
extern void rtnl_notify(struct sk_buff *skb, struct net *net, u32 pid,
u32 group, const struct nlmsghdr *nlh, gfp_t flags);
--
2.40.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH net-next v4 4/7] net/sched: act_api: don't open code max()
2023-12-08 19:28 [PATCH net-next v4 0/7] net/sched: conditional notification of events for cls and act Pedro Tammela
` (2 preceding siblings ...)
2023-12-08 19:28 ` [PATCH net-next v4 3/7] rtnl: add helper to send if skb is not null Pedro Tammela
@ 2023-12-08 19:28 ` Pedro Tammela
2023-12-10 11:47 ` Jiri Pirko
2023-12-08 19:28 ` [PATCH net-next v4 5/7] net/sched: act_api: conditional notification of events Pedro Tammela
` (3 subsequent siblings)
7 siblings, 1 reply; 12+ messages in thread
From: Pedro Tammela @ 2023-12-08 19:28 UTC (permalink / raw)
To: netdev
Cc: davem, edumazet, kuba, pabeni, jhs, xiyou.wangcong, jiri,
marcelo.leitner, vladbu, horms, Pedro Tammela
Use max() in a couple of places that are open coding it with the
ternary operator.
Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
---
net/sched/act_api.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index abec5c45b5a4..4f295ae4e152 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -1796,8 +1796,7 @@ tcf_reoffload_del_notify(struct net *net, struct tc_action *action)
struct sk_buff *skb;
int ret;
- skb = alloc_skb(attr_size <= NLMSG_GOODSIZE ? NLMSG_GOODSIZE : attr_size,
- GFP_KERNEL);
+ skb = alloc_skb(max(attr_size, NLMSG_GOODSIZE), GFP_KERNEL);
if (!skb)
return -ENOBUFS;
@@ -1882,8 +1881,7 @@ tcf_del_notify(struct net *net, struct nlmsghdr *n, struct tc_action *actions[],
int ret;
struct sk_buff *skb;
- skb = alloc_skb(attr_size <= NLMSG_GOODSIZE ? NLMSG_GOODSIZE : attr_size,
- GFP_KERNEL);
+ skb = alloc_skb(max(attr_size, NLMSG_GOODSIZE), GFP_KERNEL);
if (!skb)
return -ENOBUFS;
@@ -1961,8 +1959,7 @@ tcf_add_notify(struct net *net, struct nlmsghdr *n, struct tc_action *actions[],
{
struct sk_buff *skb;
- skb = alloc_skb(attr_size <= NLMSG_GOODSIZE ? NLMSG_GOODSIZE : attr_size,
- GFP_KERNEL);
+ skb = alloc_skb(max(attr_size, NLMSG_GOODSIZE), GFP_KERNEL);
if (!skb)
return -ENOBUFS;
--
2.40.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH net-next v4 4/7] net/sched: act_api: don't open code max()
2023-12-08 19:28 ` [PATCH net-next v4 4/7] net/sched: act_api: don't open code max() Pedro Tammela
@ 2023-12-10 11:47 ` Jiri Pirko
0 siblings, 0 replies; 12+ messages in thread
From: Jiri Pirko @ 2023-12-10 11:47 UTC (permalink / raw)
To: Pedro Tammela
Cc: netdev, davem, edumazet, kuba, pabeni, jhs, xiyou.wangcong,
marcelo.leitner, vladbu, horms
Fri, Dec 08, 2023 at 08:28:44PM CET, pctammela@mojatatu.com wrote:
>Use max() in a couple of places that are open coding it with the
>ternary operator.
>
>Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH net-next v4 5/7] net/sched: act_api: conditional notification of events
2023-12-08 19:28 [PATCH net-next v4 0/7] net/sched: conditional notification of events for cls and act Pedro Tammela
` (3 preceding siblings ...)
2023-12-08 19:28 ` [PATCH net-next v4 4/7] net/sched: act_api: don't open code max() Pedro Tammela
@ 2023-12-08 19:28 ` Pedro Tammela
2023-12-10 11:49 ` Jiri Pirko
2023-12-08 19:28 ` [PATCH net-next v4 6/7] net/sched: cls_api: remove 'unicast' argument from delete notification Pedro Tammela
` (2 subsequent siblings)
7 siblings, 1 reply; 12+ messages in thread
From: Pedro Tammela @ 2023-12-08 19:28 UTC (permalink / raw)
To: netdev
Cc: davem, edumazet, kuba, pabeni, jhs, xiyou.wangcong, jiri,
marcelo.leitner, vladbu, horms, Pedro Tammela
As of today tc-action events are unconditionally built and sent to
RTNLGRP_TC. As with the introduction of rtnl_notify_needed we can check
before-hand if they are really needed.
Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
---
net/sched/act_api.c | 98 ++++++++++++++++++++++++++++++++++-----------
1 file changed, 75 insertions(+), 23 deletions(-)
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 4f295ae4e152..6611f292b6cb 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -1785,30 +1785,45 @@ static int tcf_action_delete(struct net *net, struct tc_action *actions[])
return 0;
}
-static int
-tcf_reoffload_del_notify(struct net *net, struct tc_action *action)
+static struct sk_buff *tcf_reoffload_del_notify_msg(struct net *net,
+ struct tc_action *action)
{
size_t attr_size = tcf_action_fill_size(action);
struct tc_action *actions[TCA_ACT_MAX_PRIO] = {
[0] = action,
};
- const struct tc_action_ops *ops = action->ops;
struct sk_buff *skb;
- int ret;
skb = alloc_skb(max(attr_size, NLMSG_GOODSIZE), GFP_KERNEL);
if (!skb)
- return -ENOBUFS;
+ return ERR_PTR(-ENOBUFS);
if (tca_get_fill(skb, actions, 0, 0, 0, RTM_DELACTION, 0, 1, NULL) <= 0) {
kfree_skb(skb);
- return -EINVAL;
+ return ERR_PTR(-EINVAL);
+ }
+
+ return skb;
+}
+
+static int tcf_reoffload_del_notify(struct net *net, struct tc_action *action)
+{
+ const struct tc_action_ops *ops = action->ops;
+ struct sk_buff *skb;
+ int ret;
+
+ if (!rtnl_notify_needed(net, 0, RTNLGRP_TC)) {
+ skb = NULL;
+ } else {
+ skb = tcf_reoffload_del_notify_msg(net, action);
+ if (IS_ERR(skb))
+ return PTR_ERR(skb);
}
ret = tcf_idr_release_unsafe(action);
if (ret == ACT_P_DELETED) {
module_put(ops->owner);
- ret = rtnetlink_send(skb, net, 0, RTNLGRP_TC, 0);
+ ret = rtnetlink_maybe_send(skb, net, 0, RTNLGRP_TC, 0);
} else {
kfree_skb(skb);
}
@@ -1874,22 +1889,41 @@ int tcf_action_reoffload_cb(flow_indr_block_bind_cb_t *cb,
return 0;
}
-static int
-tcf_del_notify(struct net *net, struct nlmsghdr *n, struct tc_action *actions[],
- u32 portid, size_t attr_size, struct netlink_ext_ack *extack)
+static struct sk_buff *tcf_del_notify_msg(struct net *net, struct nlmsghdr *n,
+ struct tc_action *actions[],
+ u32 portid, size_t attr_size,
+ struct netlink_ext_ack *extack)
{
- int ret;
struct sk_buff *skb;
skb = alloc_skb(max(attr_size, NLMSG_GOODSIZE), GFP_KERNEL);
if (!skb)
- return -ENOBUFS;
+ return ERR_PTR(-ENOBUFS);
if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, 0, RTM_DELACTION,
0, 2, extack) <= 0) {
NL_SET_ERR_MSG(extack, "Failed to fill netlink TC action attributes");
kfree_skb(skb);
- return -EINVAL;
+ return ERR_PTR(-EINVAL);
+ }
+
+ return skb;
+}
+
+static int tcf_del_notify(struct net *net, struct nlmsghdr *n,
+ struct tc_action *actions[], u32 portid,
+ size_t attr_size, struct netlink_ext_ack *extack)
+{
+ struct sk_buff *skb;
+ int ret;
+
+ if (!rtnl_notify_needed(net, n->nlmsg_flags, RTNLGRP_TC)) {
+ skb = NULL;
+ } else {
+ skb = tcf_del_notify_msg(net, n, actions, portid, attr_size,
+ extack);
+ if (IS_ERR(skb))
+ return PTR_ERR(skb);
}
/* now do the delete */
@@ -1900,9 +1934,8 @@ tcf_del_notify(struct net *net, struct nlmsghdr *n, struct tc_action *actions[],
return ret;
}
- ret = rtnetlink_send(skb, net, portid, RTNLGRP_TC,
- n->nlmsg_flags & NLM_F_ECHO);
- return ret;
+ return rtnetlink_maybe_send(skb, net, portid, RTNLGRP_TC,
+ n->nlmsg_flags & NLM_F_ECHO);
}
static int
@@ -1953,25 +1986,44 @@ tca_action_gd(struct net *net, struct nlattr *nla, struct nlmsghdr *n,
return ret;
}
-static int
-tcf_add_notify(struct net *net, struct nlmsghdr *n, struct tc_action *actions[],
- u32 portid, size_t attr_size, struct netlink_ext_ack *extack)
+static struct sk_buff *tcf_add_notify_msg(struct net *net, struct nlmsghdr *n,
+ struct tc_action *actions[],
+ u32 portid, size_t attr_size,
+ struct netlink_ext_ack *extack)
{
struct sk_buff *skb;
skb = alloc_skb(max(attr_size, NLMSG_GOODSIZE), GFP_KERNEL);
if (!skb)
- return -ENOBUFS;
+ return ERR_PTR(-ENOBUFS);
if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, n->nlmsg_flags,
RTM_NEWACTION, 0, 0, extack) <= 0) {
NL_SET_ERR_MSG(extack, "Failed to fill netlink attributes while adding TC action");
kfree_skb(skb);
- return -EINVAL;
+ return ERR_PTR(-EINVAL);
+ }
+
+ return skb;
+}
+
+static int tcf_add_notify(struct net *net, struct nlmsghdr *n,
+ struct tc_action *actions[], u32 portid,
+ size_t attr_size, struct netlink_ext_ack *extack)
+{
+ struct sk_buff *skb;
+
+ if (!rtnl_notify_needed(net, n->nlmsg_flags, RTNLGRP_TC)) {
+ skb = NULL;
+ } else {
+ skb = tcf_add_notify_msg(net, n, actions, portid, attr_size,
+ extack);
+ if (IS_ERR(skb))
+ return PTR_ERR(skb);
}
- return rtnetlink_send(skb, net, portid, RTNLGRP_TC,
- n->nlmsg_flags & NLM_F_ECHO);
+ return rtnetlink_maybe_send(skb, net, portid, RTNLGRP_TC,
+ n->nlmsg_flags & NLM_F_ECHO);
}
static int tcf_action_add(struct net *net, struct nlattr *nla,
--
2.40.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH net-next v4 5/7] net/sched: act_api: conditional notification of events
2023-12-08 19:28 ` [PATCH net-next v4 5/7] net/sched: act_api: conditional notification of events Pedro Tammela
@ 2023-12-10 11:49 ` Jiri Pirko
0 siblings, 0 replies; 12+ messages in thread
From: Jiri Pirko @ 2023-12-10 11:49 UTC (permalink / raw)
To: Pedro Tammela
Cc: netdev, davem, edumazet, kuba, pabeni, jhs, xiyou.wangcong,
marcelo.leitner, vladbu, horms
Fri, Dec 08, 2023 at 08:28:45PM CET, pctammela@mojatatu.com wrote:
>As of today tc-action events are unconditionally built and sent to
>RTNLGRP_TC. As with the introduction of rtnl_notify_needed we can check
>before-hand if they are really needed.
>
>Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH net-next v4 6/7] net/sched: cls_api: remove 'unicast' argument from delete notification
2023-12-08 19:28 [PATCH net-next v4 0/7] net/sched: conditional notification of events for cls and act Pedro Tammela
` (4 preceding siblings ...)
2023-12-08 19:28 ` [PATCH net-next v4 5/7] net/sched: act_api: conditional notification of events Pedro Tammela
@ 2023-12-08 19:28 ` Pedro Tammela
2023-12-10 11:49 ` Jiri Pirko
2023-12-08 19:28 ` [PATCH net-next v4 7/7] net/sched: cls_api: conditional notification of events Pedro Tammela
2023-12-12 3:20 ` [PATCH net-next v4 0/7] net/sched: conditional notification of events for cls and act patchwork-bot+netdevbpf
7 siblings, 1 reply; 12+ messages in thread
From: Pedro Tammela @ 2023-12-08 19:28 UTC (permalink / raw)
To: netdev
Cc: davem, edumazet, kuba, pabeni, jhs, xiyou.wangcong, jiri,
marcelo.leitner, vladbu, horms, Pedro Tammela
This argument is never called while set to true, so remove it as there's
no need for it.
Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
---
net/sched/cls_api.c | 26 +++++++++-----------------
1 file changed, 9 insertions(+), 17 deletions(-)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 1976bd163986..4050215a532d 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -650,7 +650,7 @@ static void tc_chain_tmplt_del(const struct tcf_proto_ops *tmplt_ops,
static int tc_chain_notify_delete(const struct tcf_proto_ops *tmplt_ops,
void *tmplt_priv, u32 chain_index,
struct tcf_block *block, struct sk_buff *oskb,
- u32 seq, u16 flags, bool unicast);
+ u32 seq, u16 flags);
static void __tcf_chain_put(struct tcf_chain *chain, bool by_act,
bool explicitly_created)
@@ -685,8 +685,7 @@ static void __tcf_chain_put(struct tcf_chain *chain, bool by_act,
if (non_act_refcnt == chain->explicitly_created && !by_act) {
if (non_act_refcnt == 0)
tc_chain_notify_delete(tmplt_ops, tmplt_priv,
- chain->index, block, NULL, 0, 0,
- false);
+ chain->index, block, NULL, 0, 0);
/* Last reference to chain, no need to lock. */
chain->flushing = false;
}
@@ -2075,8 +2074,8 @@ static int tfilter_notify(struct net *net, struct sk_buff *oskb,
static int tfilter_del_notify(struct net *net, struct sk_buff *oskb,
struct nlmsghdr *n, struct tcf_proto *tp,
struct tcf_block *block, struct Qdisc *q,
- u32 parent, void *fh, bool unicast, bool *last,
- bool rtnl_held, struct netlink_ext_ack *extack)
+ u32 parent, void *fh, bool *last, bool rtnl_held,
+ struct netlink_ext_ack *extack)
{
struct sk_buff *skb;
u32 portid = oskb ? NETLINK_CB(oskb).portid : 0;
@@ -2100,11 +2099,8 @@ static int tfilter_del_notify(struct net *net, struct sk_buff *oskb,
return err;
}
- if (unicast)
- err = rtnl_unicast(skb, net, portid);
- else
- err = rtnetlink_send(skb, net, portid, RTNLGRP_TC,
- n->nlmsg_flags & NLM_F_ECHO);
+ err = rtnetlink_send(skb, net, portid, RTNLGRP_TC,
+ n->nlmsg_flags & NLM_F_ECHO);
if (err < 0)
NL_SET_ERR_MSG(extack, "Failed to send filter delete notification");
@@ -2499,9 +2495,8 @@ static int tc_del_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
} else {
bool last;
- err = tfilter_del_notify(net, skb, n, tp, block,
- q, parent, fh, false, &last,
- rtnl_held, extack);
+ err = tfilter_del_notify(net, skb, n, tp, block, q, parent, fh,
+ &last, rtnl_held, extack);
if (err)
goto errout;
@@ -2929,7 +2924,7 @@ static int tc_chain_notify(struct tcf_chain *chain, struct sk_buff *oskb,
static int tc_chain_notify_delete(const struct tcf_proto_ops *tmplt_ops,
void *tmplt_priv, u32 chain_index,
struct tcf_block *block, struct sk_buff *oskb,
- u32 seq, u16 flags, bool unicast)
+ u32 seq, u16 flags)
{
u32 portid = oskb ? NETLINK_CB(oskb).portid : 0;
struct net *net = block->net;
@@ -2945,9 +2940,6 @@ static int tc_chain_notify_delete(const struct tcf_proto_ops *tmplt_ops,
return -EINVAL;
}
- if (unicast)
- return rtnl_unicast(skb, net, portid);
-
return rtnetlink_send(skb, net, portid, RTNLGRP_TC, flags & NLM_F_ECHO);
}
--
2.40.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH net-next v4 6/7] net/sched: cls_api: remove 'unicast' argument from delete notification
2023-12-08 19:28 ` [PATCH net-next v4 6/7] net/sched: cls_api: remove 'unicast' argument from delete notification Pedro Tammela
@ 2023-12-10 11:49 ` Jiri Pirko
0 siblings, 0 replies; 12+ messages in thread
From: Jiri Pirko @ 2023-12-10 11:49 UTC (permalink / raw)
To: Pedro Tammela
Cc: netdev, davem, edumazet, kuba, pabeni, jhs, xiyou.wangcong,
marcelo.leitner, vladbu, horms
Fri, Dec 08, 2023 at 08:28:46PM CET, pctammela@mojatatu.com wrote:
>This argument is never called while set to true, so remove it as there's
>no need for it.
>
>Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH net-next v4 7/7] net/sched: cls_api: conditional notification of events
2023-12-08 19:28 [PATCH net-next v4 0/7] net/sched: conditional notification of events for cls and act Pedro Tammela
` (5 preceding siblings ...)
2023-12-08 19:28 ` [PATCH net-next v4 6/7] net/sched: cls_api: remove 'unicast' argument from delete notification Pedro Tammela
@ 2023-12-08 19:28 ` Pedro Tammela
2023-12-12 3:20 ` [PATCH net-next v4 0/7] net/sched: conditional notification of events for cls and act patchwork-bot+netdevbpf
7 siblings, 0 replies; 12+ messages in thread
From: Pedro Tammela @ 2023-12-08 19:28 UTC (permalink / raw)
To: netdev
Cc: davem, edumazet, kuba, pabeni, jhs, xiyou.wangcong, jiri,
marcelo.leitner, vladbu, horms, Pedro Tammela, Jiri Pirko
As of today tc-filter/chain events are unconditionally built and sent to
RTNLGRP_TC. As with the introduction of rtnl_notify_needed we can check
before-hand if they are really needed. This will help to alleviate
system pressure when filters are concurrently added without the rtnl
lock as in tc-flower.
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
---
net/sched/cls_api.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 4050215a532d..437daebc1fc4 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -2052,6 +2052,9 @@ static int tfilter_notify(struct net *net, struct sk_buff *oskb,
u32 portid = oskb ? NETLINK_CB(oskb).portid : 0;
int err = 0;
+ if (!unicast && !rtnl_notify_needed(net, n->nlmsg_flags, RTNLGRP_TC))
+ return 0;
+
skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
if (!skb)
return -ENOBUFS;
@@ -2081,6 +2084,9 @@ static int tfilter_del_notify(struct net *net, struct sk_buff *oskb,
u32 portid = oskb ? NETLINK_CB(oskb).portid : 0;
int err;
+ if (!rtnl_notify_needed(net, n->nlmsg_flags, RTNLGRP_TC))
+ return tp->ops->delete(tp, fh, last, rtnl_held, extack);
+
skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
if (!skb)
return -ENOBUFS;
@@ -2901,6 +2907,9 @@ static int tc_chain_notify(struct tcf_chain *chain, struct sk_buff *oskb,
struct sk_buff *skb;
int err = 0;
+ if (!unicast && !rtnl_notify_needed(net, flags, RTNLGRP_TC))
+ return 0;
+
skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
if (!skb)
return -ENOBUFS;
@@ -2930,6 +2939,9 @@ static int tc_chain_notify_delete(const struct tcf_proto_ops *tmplt_ops,
struct net *net = block->net;
struct sk_buff *skb;
+ if (!rtnl_notify_needed(net, flags, RTNLGRP_TC))
+ return 0;
+
skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
if (!skb)
return -ENOBUFS;
--
2.40.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH net-next v4 0/7] net/sched: conditional notification of events for cls and act
2023-12-08 19:28 [PATCH net-next v4 0/7] net/sched: conditional notification of events for cls and act Pedro Tammela
` (6 preceding siblings ...)
2023-12-08 19:28 ` [PATCH net-next v4 7/7] net/sched: cls_api: conditional notification of events Pedro Tammela
@ 2023-12-12 3:20 ` patchwork-bot+netdevbpf
7 siblings, 0 replies; 12+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-12-12 3:20 UTC (permalink / raw)
To: Pedro Tammela
Cc: netdev, davem, edumazet, kuba, pabeni, jhs, xiyou.wangcong, jiri,
marcelo.leitner, vladbu, horms
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Fri, 8 Dec 2023 16:28:40 -0300 you wrote:
> This is an optimization we have been leveraging on P4TC but we believe
> it will benefit rtnl users in general.
>
> It's common to allocate an skb, build a notification message and then
> broadcast an event. In the absence of any user space listeners, these
> resources (cpu and memory operations) are wasted. In cases where the subsystem
> is lockless (such as in tc-flower) this waste is more prominent. For the
> scenarios where the rtnl_lock is held it is not as prominent.
>
> [...]
Here is the summary with links:
- [net-next,v4,1/7] rtnl: add helper to check if rtnl group has listeners
https://git.kernel.org/netdev/net-next/c/c5e2a973448d
- [net-next,v4,2/7] rtnl: add helper to check if a notification is needed
https://git.kernel.org/netdev/net-next/c/8439109b76a3
- [net-next,v4,3/7] rtnl: add helper to send if skb is not null
https://git.kernel.org/netdev/net-next/c/ddb6b284bdc3
- [net-next,v4,4/7] net/sched: act_api: don't open code max()
https://git.kernel.org/netdev/net-next/c/c73724bfde09
- [net-next,v4,5/7] net/sched: act_api: conditional notification of events
https://git.kernel.org/netdev/net-next/c/8d4390f51920
- [net-next,v4,6/7] net/sched: cls_api: remove 'unicast' argument from delete notification
https://git.kernel.org/netdev/net-next/c/e522755520ef
- [net-next,v4,7/7] net/sched: cls_api: conditional notification of events
https://git.kernel.org/netdev/net-next/c/93775590b1ee
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] 12+ messages in thread