[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20231207205650.GH50400@kernel.org>
Date: Thu, 7 Dec 2023 20:56:50 +0000
From: Simon Horman <horms@...nel.org>
To: Pedro Tammela <pctammela@...atatu.com>
Cc: netdev@...r.kernel.org, davem@...emloft.net, edumazet@...gle.com,
kuba@...nel.org, pabeni@...hat.com, jhs@...atatu.com,
xiyou.wangcong@...il.com, jiri@...nulli.us,
marcelo.leitner@...il.com, vladbu@...dia.com
Subject: Re: [PATCH net-next v3 4/5] net/sched: act_api: conditional
notification of events
On Wed, Dec 06, 2023 at 01:44:15PM -0300, Pedro Tammela 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@...atatu.com>
Hi Pedro,
a nice optimisation :)
...
> +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 = NULL;
> + int ret;
> +
> + if (!rtnl_notify_needed(net, 0, RTNLGRP_TC))
> + goto skip_msg;
Is there a reason (performance?) to use a goto here
rather than putting the tcf_reoffload_del_notify_msg() call inside
an if condition?
Completely untested!
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);
}
Or perhaps a helper, as this pattern seems to also appear in tcf_add_notify()
> +
> + skb = tcf_reoffload_del_notify_msg(net, action);
> + if (IS_ERR(skb))
> + return PTR_ERR(skb);
> +
> +skip_msg:
> 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);
> }
...
> +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 = NULL;
> +
> + if (!rtnl_notify_needed(net, n->nlmsg_flags, RTNLGRP_TC))
> + goto skip_msg;
> +
> + skb = tcf_add_notify_msg(net, n, actions, portid, attr_size, extack);
> + if (IS_ERR(skb))
> + return PTR_ERR(skb);
> +
> +skip_msg:
> + 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
>
Powered by blists - more mailing lists