| 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
| ||
|
Message-ID: <9eff9a51-a945-48f6-9d14-a484b7c0d04c@linux.dev>
Date: Thu, 29 Feb 2024 22:53:01 -0800
From: Martin KaFai Lau <martin.lau@...ux.dev>
To: Jamal Hadi Salim <jhs@...atatu.com>
Cc: deb.chatterjee@...el.com, anjali.singhai@...el.com,
namrata.limaye@...el.com, tom@...anda.io, mleitner@...hat.com,
Mahesh.Shirshyad@....com, Vipin.Jain@....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, daniel@...earbox.net,
victor@...atatu.com, pctammela@...atatu.com, bpf@...r.kernel.org,
netdev@...r.kernel.org
Subject: Re: [PATCH net-next v12 14/15] p4tc: add set of P4TC table kfuncs
On 2/25/24 8:54 AM, Jamal Hadi Salim wrote:
> +struct p4tc_table_entry_act_bpf_params {
Will this struct be extended in the future?
> + u32 pipeid;
> + u32 tblid;
> +};
> +
> +struct p4tc_table_entry_create_bpf_params {
> + u32 profile_id;
> + u32 pipeid;
> + u32 tblid;
> +};
> +
[ ... ]
> diff --git a/include/net/tc_act/p4tc.h b/include/net/tc_act/p4tc.h
> index c5256d821..155068de0 100644
> --- a/include/net/tc_act/p4tc.h
> +++ b/include/net/tc_act/p4tc.h
> @@ -13,10 +13,26 @@ struct tcf_p4act_params {
> u32 tot_params_sz;
> };
>
> +#define P4TC_MAX_PARAM_DATA_SIZE 124
> +
> +struct p4tc_table_entry_act_bpf {
> + u32 act_id;
> + u32 hit:1,
> + is_default_miss_act:1,
> + is_default_hit_act:1;
> + u8 params[P4TC_MAX_PARAM_DATA_SIZE];
> +} __packed;
> +
> +struct p4tc_table_entry_act_bpf_kern {
> + struct rcu_head rcu;
> + struct p4tc_table_entry_act_bpf act_bpf;
> +};
> +
> struct tcf_p4act {
> struct tc_action common;
> /* Params IDR reference passed during runtime */
> struct tcf_p4act_params __rcu *params;
> + struct p4tc_table_entry_act_bpf_kern __rcu *act_bpf;
> u32 p_id;
> u32 act_id;
> struct list_head node;
> @@ -24,4 +40,39 @@ struct tcf_p4act {
>
> #define to_p4act(a) ((struct tcf_p4act *)a)
>
> +static inline struct p4tc_table_entry_act_bpf *
> +p4tc_table_entry_act_bpf(struct tc_action *action)
> +{
> + struct p4tc_table_entry_act_bpf_kern *act_bpf;
> + struct tcf_p4act *p4act = to_p4act(action);
> +
> + act_bpf = rcu_dereference(p4act->act_bpf);
> +
> + return &act_bpf->act_bpf;
> +}
> +
> +static inline int
> +p4tc_table_entry_act_bpf_change_flags(struct tc_action *action, u32 hit,
> + u32 dflt_miss, u32 dflt_hit)
> +{
> + struct p4tc_table_entry_act_bpf_kern *act_bpf, *act_bpf_old;
> + struct tcf_p4act *p4act = to_p4act(action);
> +
> + act_bpf = kzalloc(sizeof(*act_bpf), GFP_KERNEL);
[ ... ]
> +__bpf_kfunc static struct p4tc_table_entry_act_bpf *
> +bpf_p4tc_tbl_read(struct __sk_buff *skb_ctx,
The argument could be "struct sk_buff *skb" instead of __sk_buff. Take a look at
commit 2f4643934670.
> + struct p4tc_table_entry_act_bpf_params *params,
> + void *key, const u32 key__sz)
> +{
> + struct sk_buff *skb = (struct sk_buff *)skb_ctx;
> + struct net *caller_net;
> +
> + caller_net = skb->dev ? dev_net(skb->dev) : sock_net(skb->sk);
> +
> + return __bpf_p4tc_tbl_read(caller_net, params, key, key__sz);
> +}
> +
> +__bpf_kfunc static struct p4tc_table_entry_act_bpf *
> +xdp_p4tc_tbl_read(struct xdp_md *xdp_ctx,
> + struct p4tc_table_entry_act_bpf_params *params,
> + void *key, const u32 key__sz)
> +{
> + struct xdp_buff *ctx = (struct xdp_buff *)xdp_ctx;
> + struct net *caller_net;
> +
> + caller_net = dev_net(ctx->rxq->dev);
> +
> + return __bpf_p4tc_tbl_read(caller_net, params, key, key__sz);
> +}
> +
> +static int
> +__bpf_p4tc_entry_create(struct net *net,
> + struct p4tc_table_entry_create_bpf_params *params,
> + void *key, const u32 key__sz,
> + struct p4tc_table_entry_act_bpf *act_bpf)
> +{
> + struct p4tc_table_entry_key *entry_key = key;
> + struct p4tc_pipeline *pipeline;
> + struct p4tc_table *table;
> +
> + if (!params || !key)
> + return -EINVAL;
> + if (key__sz != P4TC_ENTRY_KEY_SZ_BYTES(entry_key->keysz))
> + return -EINVAL;
> +
> + pipeline = p4tc_pipeline_find_byid(net, params->pipeid);
> + if (!pipeline)
> + return -ENOENT;
> +
> + table = p4tc_tbl_cache_lookup(net, params->pipeid, params->tblid);
> + if (!table)
> + return -ENOENT;
> +
> + if (entry_key->keysz != table->tbl_keysz)
> + return -EINVAL;
> +
> + return p4tc_table_entry_create_bpf(pipeline, table, entry_key, act_bpf,
> + params->profile_id);
My understanding is this kfunc will allocate a "struct
p4tc_table_entry_act_bpf_kern" object. If the bpf_p4tc_entry_delete() kfunc is
never called and the bpf prog is unloaded, how the act_bpf object will be
cleaned up?
> +}
> +
> +__bpf_kfunc static int
> +bpf_p4tc_entry_create(struct __sk_buff *skb_ctx,
> + struct p4tc_table_entry_create_bpf_params *params,
> + void *key, const u32 key__sz,
> + struct p4tc_table_entry_act_bpf *act_bpf)
> +{
> + struct sk_buff *skb = (struct sk_buff *)skb_ctx;
> + struct net *net;
> +
> + net = skb->dev ? dev_net(skb->dev) : sock_net(skb->sk);
> +
> + return __bpf_p4tc_entry_create(net, params, key, key__sz, act_bpf);
> +}
> +
> +__bpf_kfunc static int
> +xdp_p4tc_entry_create(struct xdp_md *xdp_ctx,
> + struct p4tc_table_entry_create_bpf_params *params,
> + void *key, const u32 key__sz,
> + struct p4tc_table_entry_act_bpf *act_bpf)
> +{
> + struct xdp_buff *ctx = (struct xdp_buff *)xdp_ctx;
> + struct net *net;
> +
> + net = dev_net(ctx->rxq->dev);
> +
> + return __bpf_p4tc_entry_create(net, params, key, key__sz, act_bpf);
> +}
> +
> +__bpf_kfunc static int
> +bpf_p4tc_entry_create_on_miss(struct __sk_buff *skb_ctx,
> + struct p4tc_table_entry_create_bpf_params *params,
> + void *key, const u32 key__sz,
> + struct p4tc_table_entry_act_bpf *act_bpf)
> +{
> + struct sk_buff *skb = (struct sk_buff *)skb_ctx;
> + struct net *net;
> +
> + net = skb->dev ? dev_net(skb->dev) : sock_net(skb->sk);
> +
> + return __bpf_p4tc_entry_create(net, params, key, key__sz, act_bpf);
> +}
> +
> +__bpf_kfunc static int
> +xdp_p4tc_entry_create_on_miss(struct xdp_md *xdp_ctx,
Same here. "struct xdp_buff *xdp".
> + struct p4tc_table_entry_create_bpf_params *params,
> + void *key, const u32 key__sz,
> + struct p4tc_table_entry_act_bpf *act_bpf)
> +{
> + struct xdp_buff *ctx = (struct xdp_buff *)xdp_ctx;
> + struct net *net;
> +
> + net = dev_net(ctx->rxq->dev);
> +
> + return __bpf_p4tc_entry_create(net, params, key, key__sz, act_bpf);
> +}
> +
[ ... ]
> +__bpf_kfunc static int
> +bpf_p4tc_entry_delete(struct __sk_buff *skb_ctx,
> + struct p4tc_table_entry_create_bpf_params *params,
> + void *key, const u32 key__sz)
> +{
> + struct sk_buff *skb = (struct sk_buff *)skb_ctx;
> + struct net *net;
> +
> + net = skb->dev ? dev_net(skb->dev) : sock_net(skb->sk);
> +
> + return __bpf_p4tc_entry_delete(net, params, key, key__sz);
> +}
> +
> +__bpf_kfunc static int
> +xdp_p4tc_entry_delete(struct xdp_md *xdp_ctx,
> + struct p4tc_table_entry_create_bpf_params *params,
> + void *key, const u32 key__sz)
> +{
> + struct xdp_buff *ctx = (struct xdp_buff *)xdp_ctx;
> + struct net *net;
> +
> + net = dev_net(ctx->rxq->dev);
> +
> + return __bpf_p4tc_entry_delete(net, params, key, key__sz);
> +}
> +
> +BTF_SET8_START(p4tc_kfunc_check_tbl_set_skb)
This soon will be broken with the latest change in bpf-next. It is replaced by
BTF_KFUNCS_START. commit a05e90427ef6.
What is the plan on the selftest ?
> +BTF_ID_FLAGS(func, bpf_p4tc_tbl_read, KF_RET_NULL);
> +BTF_ID_FLAGS(func, bpf_p4tc_entry_create);
> +BTF_ID_FLAGS(func, bpf_p4tc_entry_create_on_miss);
> +BTF_ID_FLAGS(func, bpf_p4tc_entry_update);
> +BTF_ID_FLAGS(func, bpf_p4tc_entry_delete);
> +BTF_SET8_END(p4tc_kfunc_check_tbl_set_skb)
> +
> +static const struct btf_kfunc_id_set p4tc_kfunc_tbl_set_skb = {
> + .owner = THIS_MODULE,
> + .set = &p4tc_kfunc_check_tbl_set_skb,
> +};
> +
> +BTF_SET8_START(p4tc_kfunc_check_tbl_set_xdp)
> +BTF_ID_FLAGS(func, xdp_p4tc_tbl_read, KF_RET_NULL);
> +BTF_ID_FLAGS(func, xdp_p4tc_entry_create);
> +BTF_ID_FLAGS(func, xdp_p4tc_entry_create_on_miss);
> +BTF_ID_FLAGS(func, xdp_p4tc_entry_update);
> +BTF_ID_FLAGS(func, xdp_p4tc_entry_delete);
> +BTF_SET8_END(p4tc_kfunc_check_tbl_set_xdp)
Powered by blists - more mailing lists