| Message ID | 20231201204314.220543-4-pctammela@mojatatu.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net/sched: conditional notification of events for cls and act | expand |
On Fri, 1 Dec 2023 17:43:13 -0300 Pedro Tammela wrote: > --- a/net/sched/act_api.c > +++ b/net/sched/act_api.c > @@ -1791,6 +1791,13 @@ tcf_reoffload_del_notify(struct net *net, struct tc_action *action) > struct sk_buff *skb; > int ret; > > + if (!tc_should_notify(net, 0)) { > + ret = tcf_idr_release_unsafe(action); > + if (ret == ACT_P_DELETED) > + module_put(ops->owner); > + return ret; > + } I fell like we can do better than this.. let's refactor this code a bit harder. Maybe factor out the alloc_skb() and fill()? Then add a wrapper around rtnetlink_send() which does nothing if skb is NULL? > skb = alloc_skb(attr_size <= NLMSG_GOODSIZE ? NLMSG_GOODSIZE : attr_size, > GFP_KERNEL); > if (!skb)
On 02/12/2023 16:21, Jakub Kicinski wrote: > On Fri, 1 Dec 2023 17:43:13 -0300 Pedro Tammela wrote: >> --- a/net/sched/act_api.c >> +++ b/net/sched/act_api.c >> @@ -1791,6 +1791,13 @@ tcf_reoffload_del_notify(struct net *net, struct tc_action *action) >> struct sk_buff *skb; >> int ret; >> >> + if (!tc_should_notify(net, 0)) { >> + ret = tcf_idr_release_unsafe(action); >> + if (ret == ACT_P_DELETED) >> + module_put(ops->owner); >> + return ret; >> + } > > I fell like we can do better than this.. let's refactor this code a bit > harder. Maybe factor out the alloc_skb() and fill()? Then add a wrapper > around rtnetlink_send() which does nothing if skb is NULL? Ack
diff --git a/net/sched/act_api.c b/net/sched/act_api.c index c39252d61ebb..2570f9702eeb 100644 --- a/net/sched/act_api.c +++ b/net/sched/act_api.c @@ -1791,6 +1791,13 @@ tcf_reoffload_del_notify(struct net *net, struct tc_action *action) struct sk_buff *skb; int ret; + if (!tc_should_notify(net, 0)) { + ret = tcf_idr_release_unsafe(action); + if (ret == ACT_P_DELETED) + module_put(ops->owner); + return ret; + } + skb = alloc_skb(attr_size <= NLMSG_GOODSIZE ? NLMSG_GOODSIZE : attr_size, GFP_KERNEL); if (!skb) @@ -1877,6 +1884,13 @@ tcf_del_notify(struct net *net, struct nlmsghdr *n, struct tc_action *actions[], int ret; struct sk_buff *skb; + if (!tc_should_notify(net, n->nlmsg_flags)) { + ret = tcf_action_delete(net, actions); + if (ret < 0) + NL_SET_ERR_MSG(extack, "Failed to delete TC action"); + return ret; + } + skb = alloc_skb(attr_size <= NLMSG_GOODSIZE ? NLMSG_GOODSIZE : attr_size, GFP_KERNEL); if (!skb) @@ -1956,6 +1970,9 @@ tcf_add_notify(struct net *net, struct nlmsghdr *n, struct tc_action *actions[], { struct sk_buff *skb; + if (!tc_should_notify(net, n->nlmsg_flags)) + return 0; + skb = alloc_skb(attr_size <= NLMSG_GOODSIZE ? NLMSG_GOODSIZE : attr_size, GFP_KERNEL); if (!skb)
As of today tc-action events are unconditionally built and sent to RTNLGRP_TC. As with the introduction of tc_should_notify we can check before-hand if they are really needed. Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> --- net/sched/act_api.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)