Commit 695176bf authored by Cong Wang's avatar Cong Wang Committed by David S. Miller
Browse files

net_sched: refactor TC action init API 



TC action ->init() API has 10 parameters, it becomes harder
to read. Some of them are just boolean and can be replaced
by flags. Similarly for the internal API tcf_action_init()
and tcf_exts_validate().

This patch converts them to flags and fold them into
the upper 16 bits of "flags", whose lower 16 bits are still
reserved for user-space. More specifically, the following
kernel flags are introduced:

TCA_ACT_FLAGS_POLICE replace 'name' in a few contexts, to
distinguish whether it is compatible with policer.

TCA_ACT_FLAGS_BIND replaces 'bind', to indicate whether
this action is bound to a filter.

TCA_ACT_FLAGS_REPLACE  replaces 'ovr' in most contexts,
means we are replacing an existing action.

TCA_ACT_FLAGS_NO_RTNL replaces 'rtnl_held' but has the
opposite meaning, because we still hold RTNL in most
cases.

The only user-space flag TCA_ACT_FLAGS_NO_PERCPU_STATS is
untouched and still stored as before.

I have tested this patch with tdc and I do not see any
failure related to this patch.

Tested-by: default avatarVlad Buslov <vladbu@nvidia.com>
Acked-by: default avatarJamal Hadi <Salim&lt;jhs@mojatatu.com>
Cc: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: default avatarCong Wang <cong.wang@bytedance.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 451395f7

include/net/act_api.h

+14 −8
Original line number Diff line number Diff line
@@ -58,6 +58,14 @@ struct tc_action { 
#define TCA_ACT_HW_STATS_ANY (TCA_ACT_HW_STATS_IMMEDIATE | \

			      TCA_ACT_HW_STATS_DELAYED)



/* Reserve 16 bits for user-space. See TCA_ACT_FLAGS_NO_PERCPU_STATS. */

#define TCA_ACT_FLAGS_USER_BITS 16

#define TCA_ACT_FLAGS_USER_MASK 0xffff

#define TCA_ACT_FLAGS_POLICE	(1U << TCA_ACT_FLAGS_USER_BITS)

#define TCA_ACT_FLAGS_BIND	(1U << (TCA_ACT_FLAGS_USER_BITS + 1))

#define TCA_ACT_FLAGS_REPLACE	(1U << (TCA_ACT_FLAGS_USER_BITS + 2))

#define TCA_ACT_FLAGS_NO_RTNL	(1U << (TCA_ACT_FLAGS_USER_BITS + 3))



/* Update lastuse only if needed, to avoid dirtying a cache line.

 * We use a temp variable to avoid fetching jiffies twice.

 */
@@ -99,8 +107,8 @@ struct tc_action_ops { 
	void	(*cleanup)(struct tc_action *);

	int     (*lookup)(struct net *net, struct tc_action **a, u32 index);

	int     (*init)(struct net *net, struct nlattr *nla,

			struct nlattr *est, struct tc_action **act, int ovr,

			int bind, bool rtnl_held, struct tcf_proto *tp,

			struct nlattr *est, struct tc_action **act,

			struct tcf_proto *tp,

			u32 flags, struct netlink_ext_ack *extack);

	int     (*walk)(struct net *, struct sk_buff *,

			struct netlink_callback *, int,
@@ -179,18 +187,16 @@ int tcf_action_destroy(struct tc_action *actions[], int bind); 
int tcf_action_exec(struct sk_buff *skb, struct tc_action **actions,

		    int nr_actions, struct tcf_result *res);

int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,

		    struct nlattr *est, char *name, int ovr, int bind,

		    struct nlattr *est,

		    struct tc_action *actions[], int init_res[], size_t *attr_size,

		    bool rtnl_held, struct netlink_ext_ack *extack);

struct tc_action_ops *tc_action_load_ops(char *name, struct nlattr *nla,

		    u32 flags, struct netlink_ext_ack *extack);

struct tc_action_ops *tc_action_load_ops(struct nlattr *nla, bool police,

					 bool rtnl_held,

					 struct netlink_ext_ack *extack);

struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,

				    struct nlattr *nla, struct nlattr *est,

				    char *name, int ovr, int bind,

				    struct tc_action_ops *a_o, int *init_res,

				    bool rtnl_held,

				    struct netlink_ext_ack *extack);

				    u32 flags, struct netlink_ext_ack *extack);

int tcf_action_dump(struct sk_buff *skb, struct tc_action *actions[], int bind,

		    int ref, bool terse);

int tcf_action_dump_old(struct sk_buff *skb, struct tc_action *a, int, int);

include/net/pkt_cls.h

+1 −1
Original line number Diff line number Diff line
@@ -319,7 +319,7 @@ tcf_exts_exec(struct sk_buff *skb, struct tcf_exts *exts, 


int tcf_exts_validate(struct net *net, struct tcf_proto *tp,

		      struct nlattr **tb, struct nlattr *rate_tlv,

		      struct tcf_exts *exts, bool ovr, bool rtnl_held,

		      struct tcf_exts *exts, u32 flags,

		      struct netlink_ext_ack *extack);

void tcf_exts_destroy(struct tcf_exts *exts);

void tcf_exts_change(struct tcf_exts *dst, struct tcf_exts *src);

include/net/sch_generic.h

+1 −1
Original line number Diff line number Diff line
@@ -357,7 +357,7 @@ struct tcf_proto_ops { 
	int			(*change)(struct net *net, struct sk_buff *,

					struct tcf_proto*, unsigned long,

					u32 handle, struct nlattr **,

					void **, bool, bool,

					void **, u32,

					struct netlink_ext_ack *);

	int			(*delete)(struct tcf_proto *tp, void *arg,

					  bool *last, bool rtnl_held,

include/uapi/linux/pkt_cls.h

+1 −0
Original line number Diff line number Diff line
@@ -22,6 +22,7 @@ enum { 
	__TCA_ACT_MAX

};



/* See other TCA_ACT_FLAGS_ * flags in include/net/act_api.h. */

#define TCA_ACT_FLAGS_NO_PERCPU_STATS 1 /* Don't use percpu allocator for

					 * actions stats.

					 */

net/sched/act_api.c

+31 −30
Original line number Diff line number Diff line
@@ -495,7 +495,7 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est, 
	p->tcfa_tm.install = jiffies;

	p->tcfa_tm.lastuse = jiffies;

	p->tcfa_tm.firstuse = 0;

	p->tcfa_flags = flags;

	p->tcfa_flags = flags & TCA_ACT_FLAGS_USER_MASK;

	if (est) {

		err = gen_new_estimator(&p->tcfa_bstats, p->cpu_bstats,

					&p->tcfa_rate_est,
@@ -941,7 +941,7 @@ void tcf_idr_insert_many(struct tc_action *actions[]) 
	}

}



struct tc_action_ops *tc_action_load_ops(char *name, struct nlattr *nla,

struct tc_action_ops *tc_action_load_ops(struct nlattr *nla, bool police,

					 bool rtnl_held,

					 struct netlink_ext_ack *extack)

{
@@ -951,7 +951,7 @@ struct tc_action_ops *tc_action_load_ops(char *name, struct nlattr *nla, 
	struct nlattr *kind;

	int err;



	if (name == NULL) {

	if (!police) {

		err = nla_parse_nested_deprecated(tb, TCA_ACT_MAX, nla,

						  tcf_action_policy, extack);

		if (err < 0)
@@ -967,7 +967,7 @@ struct tc_action_ops *tc_action_load_ops(char *name, struct nlattr *nla, 
			return ERR_PTR(err);

		}

	} else {

		if (strlcpy(act_name, name, IFNAMSIZ) >= IFNAMSIZ) {

		if (strlcpy(act_name, "police", IFNAMSIZ) >= IFNAMSIZ) {

			NL_SET_ERR_MSG(extack, "TC action name too long");

			return ERR_PTR(-EINVAL);

		}
@@ -1004,12 +1004,11 @@ struct tc_action_ops *tc_action_load_ops(char *name, struct nlattr *nla, 


struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,

				    struct nlattr *nla, struct nlattr *est,

				    char *name, int ovr, int bind,

				    struct tc_action_ops *a_o, int *init_res,

				    bool rtnl_held,

				    struct netlink_ext_ack *extack)

				    u32 flags, struct netlink_ext_ack *extack)

{

	struct nla_bitfield32 flags = { 0, 0 };

	bool police = flags & TCA_ACT_FLAGS_POLICE;

	struct nla_bitfield32 userflags = { 0, 0 };

	u8 hw_stats = TCA_ACT_HW_STATS_ANY;

	struct nlattr *tb[TCA_ACT_MAX + 1];

	struct tc_cookie *cookie = NULL;
@@ -1017,7 +1016,7 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp, 
	int err;



	/* backward compatibility for policer */

	if (name == NULL) {

	if (!police) {

		err = nla_parse_nested_deprecated(tb, TCA_ACT_MAX, nla,

						  tcf_action_policy, extack);

		if (err < 0)
@@ -1032,22 +1031,22 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp, 
		}

		hw_stats = tcf_action_hw_stats_get(tb[TCA_ACT_HW_STATS]);

		if (tb[TCA_ACT_FLAGS])

			flags = nla_get_bitfield32(tb[TCA_ACT_FLAGS]);

			userflags = nla_get_bitfield32(tb[TCA_ACT_FLAGS]);



		err = a_o->init(net, tb[TCA_ACT_OPTIONS], est, &a, ovr, bind,

				rtnl_held, tp, flags.value, extack);

		err = a_o->init(net, tb[TCA_ACT_OPTIONS], est, &a, tp,

				userflags.value | flags, extack);

	} else {

		err = a_o->init(net, nla, est, &a, ovr, bind, rtnl_held,

				tp, flags.value, extack);

		err = a_o->init(net, nla, est, &a, tp, userflags.value | flags,

				extack);

	}

	if (err < 0)

		goto err_out;

	*init_res = err;



	if (!name && tb[TCA_ACT_COOKIE])

	if (!police && tb[TCA_ACT_COOKIE])

		tcf_set_action_cookie(&a->act_cookie, cookie);



	if (!name)

	if (!police)

		a->hw_stats = hw_stats;



	return a;
@@ -1063,9 +1062,9 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp, 
/* Returns numbers of initialized actions or negative error. */



int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,

		    struct nlattr *est, char *name, int ovr, int bind,

		    struct tc_action *actions[], int init_res[], size_t *attr_size,

		    bool rtnl_held, struct netlink_ext_ack *extack)

		    struct nlattr *est, struct tc_action *actions[],

		    int init_res[], size_t *attr_size, u32 flags,

		    struct netlink_ext_ack *extack)

{

	struct tc_action_ops *ops[TCA_ACT_MAX_PRIO] = {};

	struct nlattr *tb[TCA_ACT_MAX_PRIO + 1];
@@ -1082,7 +1081,9 @@ int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla, 
	for (i = 1; i <= TCA_ACT_MAX_PRIO && tb[i]; i++) {

		struct tc_action_ops *a_o;



		a_o = tc_action_load_ops(name, tb[i], rtnl_held, extack);

		a_o = tc_action_load_ops(tb[i], flags & TCA_ACT_FLAGS_POLICE,

					 !(flags & TCA_ACT_FLAGS_NO_RTNL),

					 extack);

		if (IS_ERR(a_o)) {

			err = PTR_ERR(a_o);

			goto err_mod;
@@ -1091,9 +1092,8 @@ int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla, 
	}



	for (i = 1; i <= TCA_ACT_MAX_PRIO && tb[i]; i++) {

		act = tcf_action_init_1(net, tp, tb[i], est, name, ovr, bind,

					ops[i - 1], &init_res[i - 1], rtnl_held,

					extack);

		act = tcf_action_init_1(net, tp, tb[i], est, ops[i - 1],

					&init_res[i - 1], flags, extack);

		if (IS_ERR(act)) {

			err = PTR_ERR(act);

			goto err;
@@ -1113,7 +1113,7 @@ int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla, 
	goto err_mod;



err:

	tcf_action_destroy(actions, bind);

	tcf_action_destroy(actions, flags & TCA_ACT_FLAGS_BIND);

err_mod:

	for (i = 0; i < TCA_ACT_MAX_PRIO; i++) {

		if (ops[i])
@@ -1495,7 +1495,7 @@ tcf_add_notify(struct net *net, struct nlmsghdr *n, struct tc_action *actions[], 
}



static int tcf_action_add(struct net *net, struct nlattr *nla,

			  struct nlmsghdr *n, u32 portid, int ovr,

			  struct nlmsghdr *n, u32 portid, u32 flags,

			  struct netlink_ext_ack *extack)

{

	size_t attr_size = 0;
@@ -1504,8 +1504,8 @@ static int tcf_action_add(struct net *net, struct nlattr *nla, 
	int init_res[TCA_ACT_MAX_PRIO] = {};



	for (loop = 0; loop < 10; loop++) {

		ret = tcf_action_init(net, NULL, nla, NULL, NULL, ovr, 0,

				      actions, init_res, &attr_size, true, extack);

		ret = tcf_action_init(net, NULL, nla, NULL, actions, init_res,

				      &attr_size, flags, extack);

		if (ret != -EAGAIN)

			break;

	}
@@ -1535,7 +1535,8 @@ static int tc_ctl_action(struct sk_buff *skb, struct nlmsghdr *n, 
	struct net *net = sock_net(skb->sk);

	struct nlattr *tca[TCA_ROOT_MAX + 1];

	u32 portid = NETLINK_CB(skb).portid;

	int ret = 0, ovr = 0;

	u32 flags = 0;

	int ret = 0;



	if ((n->nlmsg_type != RTM_GETACTION) &&

	    !netlink_capable(skb, CAP_NET_ADMIN))
@@ -1561,8 +1562,8 @@ static int tc_ctl_action(struct sk_buff *skb, struct nlmsghdr *n, 
		 * is zero) then just set this

		 */

		if (n->nlmsg_flags & NLM_F_REPLACE)

			ovr = 1;

		ret = tcf_action_add(net, tca[TCA_ACT_TAB], n, portid, ovr,

			flags = TCA_ACT_FLAGS_REPLACE;

		ret = tcf_action_add(net, tca[TCA_ACT_TAB], n, portid, flags,

				     extack);

		break;

	case RTM_DELACTION: