On Sun, 2024-02-25 at 11:54 -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 77ee0c657..f22be14bb 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,10 +200,12 @@ 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);
> #define NET_ACT_ALIAS_PREFIX "net-act-"
> #define MODULE_ALIAS_NET_ACT(kind) MODULE_ALIAS(NET_ACT_ALIAS_PREFIX kind)
> +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);
> @@ -210,8 +213,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 9ee622fb1..23ef394f2 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;
Note that rwlock in networking code is discouraged, as they have to be
unfair, see commit 0daf07e527095e64ee8927ce297ab626643e9f51.
In this specific case I think there should be no problems, as is
extremely hard/impossible to have serious contention on the write
side,. Also there is already an existing rwlock nearby, no not a
blocker but IMHO worthy to be noted.
Cheers,
Paolo
