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@xxxxxxxxxxxx>
> Signed-off-by: Victor Nogueira <victor@xxxxxxxxxxxx>
> Co-developed-by: Pedro Tammela <pctammela@xxxxxxxxxxxx>
> Signed-off-by: Pedro Tammela <pctammela@xxxxxxxxxxxx>
> Signed-off-by: Jamal Hadi Salim <jhs@xxxxxxxxxxxx>
> Reviewed-by: Vlad Buslov <vladbu@xxxxxxxxxx>
Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@xxxxxxxxx>
> ---
> 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
>
