lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CALnP8ZbTPN2nGVTF12ONZQxvmRR_wT4kch2z1TP_u-BJf730Kg@mail.gmail.com>
Date: Thu, 1 Feb 2024 11:00:15 -0800
From: Marcelo Ricardo Leitner <mleitner@...hat.com>
To: Jamal Hadi Salim <jhs@...atatu.com>
Cc: netdev@...r.kernel.org, deb.chatterjee@...el.com, anjali.singhai@...el.com, 
	namrata.limaye@...el.com, tom@...anda.io, Mahesh.Shirshyad@....com, 
	tomasz.osinski@...el.com, jiri@...nulli.us, xiyou.wangcong@...il.com, 
	davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com, 
	vladbu@...dia.com, horms@...nel.org, khalidm@...dia.com, toke@...hat.com, 
	mattyk@...dia.com, daniel@...earbox.net, bpf@...r.kernel.org
Subject: Re: [PATCH v10 net-next 01/15] net: sched: act_api: Introduce P4
 actions list

On Mon, Jan 22, 2024 at 02:47:47PM -0500, Jamal Hadi Salim wrote:
> In P4 we require to generate new actions "on the fly" based on the
> specified P4 action definition. P4 action kinds, like the pipeline
> they are attached to, must be per net namespace, as opposed to native
> action kinds which are global. For that reason, we chose to create a
> separate structure to store P4 actions.
>
> Co-developed-by: Victor Nogueira <victor@...atatu.com>
> Signed-off-by: Victor Nogueira <victor@...atatu.com>
> Co-developed-by: Pedro Tammela <pctammela@...atatu.com>
> Signed-off-by: Pedro Tammela <pctammela@...atatu.com>
> Signed-off-by: Jamal Hadi Salim <jhs@...atatu.com>
> Reviewed-by: Vlad Buslov <vladbu@...dia.com>

Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@...il.com>

> ---
>  include/net/act_api.h |   8 ++-
>  net/sched/act_api.c   | 123 +++++++++++++++++++++++++++++++++++++-----
>  net/sched/cls_api.c   |   2 +-
>  3 files changed, 116 insertions(+), 17 deletions(-)
>
> diff --git a/include/net/act_api.h b/include/net/act_api.h
> index e1e5e72b9..ab28c2254 100644
> --- a/include/net/act_api.h
> +++ b/include/net/act_api.h
> @@ -105,6 +105,7 @@ typedef void (*tc_action_priv_destructor)(void *priv);
>
>  struct tc_action_ops {
>  	struct list_head head;
> +	struct list_head p4_head;
>  	char    kind[IFNAMSIZ];
>  	enum tca_id  id; /* identifier should match kind */
>  	unsigned int	net_id;
> @@ -199,8 +200,10 @@ int tcf_idr_check_alloc(struct tc_action_net *tn, u32 *index,
>  int tcf_idr_release(struct tc_action *a, bool bind);
>
>  int tcf_register_action(struct tc_action_ops *a, struct pernet_operations *ops);
> +int tcf_register_p4_action(struct net *net, struct tc_action_ops *act);
>  int tcf_unregister_action(struct tc_action_ops *a,
>  			  struct pernet_operations *ops);
> +void tcf_unregister_p4_action(struct net *net, struct tc_action_ops *act);
>  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);
> @@ -208,8 +211,9 @@ int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,
>  		    struct nlattr *est,
>  		    struct tc_action *actions[], int init_res[], size_t *attr_size,
>  		    u32 flags, u32 fl_flags, struct netlink_ext_ack *extack);
> -struct tc_action_ops *tc_action_load_ops(struct nlattr *nla, u32 flags,
> -					 struct netlink_ext_ack *extack);
> +struct tc_action_ops *
> +tc_action_load_ops(struct net *net, struct nlattr *nla,
> +		   u32 flags, 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,
>  				    struct tc_action_ops *a_o, int *init_res,
> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
> index 3e30d7260..e4a1b8f5a 100644
> --- a/net/sched/act_api.c
> +++ b/net/sched/act_api.c
> @@ -57,6 +57,40 @@ static void tcf_free_cookie_rcu(struct rcu_head *p)
>  	kfree(cookie);
>  }
>
> +static unsigned int p4_act_net_id;
> +
> +struct tcf_p4_act_net {
> +	struct list_head act_base;
> +	rwlock_t act_mod_lock;
> +};
> +
> +static __net_init int tcf_p4_act_base_init_net(struct net *net)
> +{
> +	struct tcf_p4_act_net *p4_base_net = net_generic(net, p4_act_net_id);
> +
> +	INIT_LIST_HEAD(&p4_base_net->act_base);
> +	rwlock_init(&p4_base_net->act_mod_lock);
> +
> +	return 0;
> +}
> +
> +static void __net_exit tcf_p4_act_base_exit_net(struct net *net)
> +{
> +	struct tcf_p4_act_net *p4_base_net = net_generic(net, p4_act_net_id);
> +	struct tc_action_ops *ops, *tmp;
> +
> +	list_for_each_entry_safe(ops, tmp, &p4_base_net->act_base, p4_head) {
> +		list_del(&ops->p4_head);
> +	}
> +}
> +
> +static struct pernet_operations tcf_p4_act_base_net_ops = {
> +	.init = tcf_p4_act_base_init_net,
> +	.exit = tcf_p4_act_base_exit_net,
> +	.id = &p4_act_net_id,
> +	.size = sizeof(struct tc_action_ops),
> +};
> +
>  static void tcf_set_action_cookie(struct tc_cookie __rcu **old_cookie,
>  				  struct tc_cookie *new_cookie)
>  {
> @@ -962,6 +996,48 @@ static void tcf_pernet_del_id_list(unsigned int id)
>  	mutex_unlock(&act_id_mutex);
>  }
>
> +static struct tc_action_ops *tc_lookup_p4_action(struct net *net, char *kind)
> +{
> +	struct tcf_p4_act_net *p4_base_net = net_generic(net, p4_act_net_id);
> +	struct tc_action_ops *a, *res = NULL;
> +
> +	read_lock(&p4_base_net->act_mod_lock);
> +	list_for_each_entry(a, &p4_base_net->act_base, p4_head) {
> +		if (strcmp(kind, a->kind) == 0) {
> +			if (try_module_get(a->owner))
> +				res = a;
> +			break;
> +		}
> +	}
> +	read_unlock(&p4_base_net->act_mod_lock);
> +
> +	return res;
> +}
> +
> +void tcf_unregister_p4_action(struct net *net, struct tc_action_ops *act)
> +{
> +	struct tcf_p4_act_net *p4_base_net = net_generic(net, p4_act_net_id);
> +
> +	write_lock(&p4_base_net->act_mod_lock);
> +	list_del(&act->p4_head);
> +	write_unlock(&p4_base_net->act_mod_lock);
> +}
> +EXPORT_SYMBOL(tcf_unregister_p4_action);
> +
> +int tcf_register_p4_action(struct net *net, struct tc_action_ops *act)
> +{
> +	struct tcf_p4_act_net *p4_base_net = net_generic(net, p4_act_net_id);
> +
> +	if (tc_lookup_p4_action(net, act->kind))
> +		return -EEXIST;
> +
> +	write_lock(&p4_base_net->act_mod_lock);
> +	list_add(&act->p4_head, &p4_base_net->act_base);
> +	write_unlock(&p4_base_net->act_mod_lock);
> +
> +	return 0;
> +}
> +
>  int tcf_register_action(struct tc_action_ops *act,
>  			struct pernet_operations *ops)
>  {
> @@ -1032,7 +1108,7 @@ int tcf_unregister_action(struct tc_action_ops *act,
>  EXPORT_SYMBOL(tcf_unregister_action);
>
>  /* lookup by name */
> -static struct tc_action_ops *tc_lookup_action_n(char *kind)
> +static struct tc_action_ops *tc_lookup_action_n(struct net *net, char *kind)
>  {
>  	struct tc_action_ops *a, *res = NULL;
>
> @@ -1040,31 +1116,48 @@ static struct tc_action_ops *tc_lookup_action_n(char *kind)
>  		read_lock(&act_mod_lock);
>  		list_for_each_entry(a, &act_base, head) {
>  			if (strcmp(kind, a->kind) == 0) {
> -				if (try_module_get(a->owner))
> -					res = a;
> -				break;
> +				if (try_module_get(a->owner)) {
> +					read_unlock(&act_mod_lock);
> +					return a;
> +				}
>  			}
>  		}
>  		read_unlock(&act_mod_lock);
> +
> +		return tc_lookup_p4_action(net, kind);
>  	}
> +
>  	return res;
>  }
>
>  /* lookup by nlattr */
> -static struct tc_action_ops *tc_lookup_action(struct nlattr *kind)
> +static struct tc_action_ops *tc_lookup_action(struct net *net,
> +					      struct nlattr *kind)
>  {
> +	struct tcf_p4_act_net *p4_base_net = net_generic(net, p4_act_net_id);
>  	struct tc_action_ops *a, *res = NULL;
>
>  	if (kind) {
>  		read_lock(&act_mod_lock);
>  		list_for_each_entry(a, &act_base, head) {
> +			if (nla_strcmp(kind, a->kind) == 0) {
> +				if (try_module_get(a->owner)) {
> +					read_unlock(&act_mod_lock);
> +					return a;
> +				}
> +			}
> +		}
> +		read_unlock(&act_mod_lock);
> +
> +		read_lock(&p4_base_net->act_mod_lock);
> +		list_for_each_entry(a, &p4_base_net->act_base, p4_head) {
>  			if (nla_strcmp(kind, a->kind) == 0) {
>  				if (try_module_get(a->owner))
>  					res = a;
>  				break;
>  			}
>  		}
> -		read_unlock(&act_mod_lock);
> +		read_unlock(&p4_base_net->act_mod_lock);
>  	}
>  	return res;
>  }
> @@ -1324,8 +1417,9 @@ void tcf_idr_insert_many(struct tc_action *actions[], int init_res[])
>  	}
>  }
>
> -struct tc_action_ops *tc_action_load_ops(struct nlattr *nla, u32 flags,
> -					 struct netlink_ext_ack *extack)
> +struct tc_action_ops *
> +tc_action_load_ops(struct net *net, struct nlattr *nla,
> +		   u32 flags, struct netlink_ext_ack *extack)
>  {
>  	bool police = flags & TCA_ACT_FLAGS_POLICE;
>  	struct nlattr *tb[TCA_ACT_MAX + 1];
> @@ -1356,7 +1450,7 @@ struct tc_action_ops *tc_action_load_ops(struct nlattr *nla, u32 flags,
>  		}
>  	}
>
> -	a_o = tc_lookup_action_n(act_name);
> +	a_o = tc_lookup_action_n(net, act_name);
>  	if (a_o == NULL) {
>  #ifdef CONFIG_MODULES
>  		bool rtnl_held = !(flags & TCA_ACT_FLAGS_NO_RTNL);
> @@ -1367,7 +1461,7 @@ struct tc_action_ops *tc_action_load_ops(struct nlattr *nla, u32 flags,
>  		if (rtnl_held)
>  			rtnl_lock();
>
> -		a_o = tc_lookup_action_n(act_name);
> +		a_o = tc_lookup_action_n(net, act_name);
>
>  		/* We dropped the RTNL semaphore in order to
>  		 * perform the module load.  So, even if we
> @@ -1477,7 +1571,7 @@ 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(tb[i], flags, extack);
> +		a_o = tc_action_load_ops(net, tb[i], flags, extack);
>  		if (IS_ERR(a_o)) {
>  			err = PTR_ERR(a_o);
>  			goto err_mod;
> @@ -1683,7 +1777,7 @@ static struct tc_action *tcf_action_get_1(struct net *net, struct nlattr *nla,
>  	index = nla_get_u32(tb[TCA_ACT_INDEX]);
>
>  	err = -EINVAL;
> -	ops = tc_lookup_action(tb[TCA_ACT_KIND]);
> +	ops = tc_lookup_action(net, tb[TCA_ACT_KIND]);
>  	if (!ops) { /* could happen in batch of actions */
>  		NL_SET_ERR_MSG(extack, "Specified TC action kind not found");
>  		goto err_out;
> @@ -1731,7 +1825,7 @@ static int tca_action_flush(struct net *net, struct nlattr *nla,
>
>  	err = -EINVAL;
>  	kind = tb[TCA_ACT_KIND];
> -	ops = tc_lookup_action(kind);
> +	ops = tc_lookup_action(net, kind);
>  	if (!ops) { /*some idjot trying to flush unknown action */
>  		NL_SET_ERR_MSG(extack, "Cannot flush unknown TC action");
>  		goto err_out;
> @@ -2184,7 +2278,7 @@ static int tc_dump_action(struct sk_buff *skb, struct netlink_callback *cb)
>  		return 0;
>  	}
>
> -	a_o = tc_lookup_action(kind);
> +	a_o = tc_lookup_action(net, kind);
>  	if (a_o == NULL)
>  		return 0;
>
> @@ -2251,6 +2345,7 @@ static int __init tc_action_init(void)
>  	rtnl_register(PF_UNSPEC, RTM_GETACTION, tc_ctl_action, tc_dump_action,
>  		      0);
>
> +	register_pernet_subsys(&tcf_p4_act_base_net_ops);
>  	return 0;
>  }
>
> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> index 92a12e3d0..2fec3f80b 100644
> --- a/net/sched/cls_api.c
> +++ b/net/sched/cls_api.c
> @@ -3323,7 +3323,7 @@ int tcf_exts_validate_ex(struct net *net, struct tcf_proto *tp, struct nlattr **
>  			struct tc_action_ops *a_o;
>
>  			flags |= TCA_ACT_FLAGS_POLICE | TCA_ACT_FLAGS_BIND;
> -			a_o = tc_action_load_ops(tb[exts->police], flags,
> +			a_o = tc_action_load_ops(net, tb[exts->police], flags,
>  						 extack);
>  			if (IS_ERR(a_o))
>  				return PTR_ERR(a_o);
> --
> 2.34.1
>


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ