BPF List
 help / color / mirror / Atom feed
From: Martin KaFai Lau <martin.lau@linux.dev>
To: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Cc: Amery Hung <amery.hung@bytedance.com>,
	bpf@vger.kernel.org, magnus.karlsson@intel.com,
	sreedevi.joshi@intel.com, ast@kernel.org
Subject: Re: [External] Storing sk_buffs as kptrs in map
Date: Fri, 6 Dec 2024 16:36:03 -0800	[thread overview]
Message-ID: <176ad1b2-034b-4b10-93cd-f03391820e24@linux.dev> (raw)
In-Reply-To: <Z1MlVa3OXQJw0VXm@boxer>

On 12/6/24 8:24 AM, Maciej Fijalkowski wrote:
>> I think we can remove the projection_of call from the
>> bpf_is_prog_ctx_type() such that it honors the exact argument
>> type written in the kernel source code. Add this particular projection_of
>> check (renamed to bpf_is_kern_ctx in the diff) to the other callers for
>> backward compat such that the caller can selectively translate
>> the argument of a subprog to the corresponding prog ctx type.
>>
>> Lightly tested only:
> I tried the kernel diff on my side and it addressed my needs. Will you
> send a patch?

There is no real kfunc taking the "struct sk_buff *" now. It is better to make 
this change together with your skb acquire/release kfunc introduction. You can 
include this patch in your future set.

> 
>> diff --git i/kernel/bpf/btf.c w/kernel/bpf/btf.c
>> index e7a59e6462a9..2d39f91617fb 100644
>> --- i/kernel/bpf/btf.c
>> +++ w/kernel/bpf/btf.c
>> @@ -5914,6 +5914,26 @@ bool btf_is_projection_of(const char *pname, const char *tname)
>>   	return false;
>>   }
>> +static bool btf_is_kern_ctx(const struct btf *btf,
>> +			    const struct btf_type *t,
>> +			    enum bpf_prog_type prog_type)
>> +{
>> +	const struct btf_type *ctx_type;
>> +	const char *tname, *ctx_tname;
>> +
>> +	t = btf_type_skip_modifiers(btf, t->type, NULL);
>> +	if (!btf_type_is_struct(t))
>> +		return false;
>> +
>> +	tname = btf_name_by_offset(btf, t->name_off);
>> +	if (!tname)
>> +		return false;
>> +
>> +	ctx_type = find_canonical_prog_ctx_type(prog_type);
>> +	ctx_tname = btf_name_by_offset(btf_vmlinux, ctx_type->name_off);
>> +	return btf_is_projection_of(ctx_tname, tname);
> We're sort of doubling the work that btf_is_prog_ctx_type() is doing also,
> maybe add a flag to btf_is_prog_ctx_type() that will allow us to skip
> btf_is_projection_of() call when needed? e.g. in get_kfunc_ptr_arg_type().

It is pretty cheap to do the btf_is_kern_ctx().

I don't have a strong opinion on either way. may be a "bool check_kern_ctx".

  reply	other threads:[~2024-12-07  0:36 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-26 17:05 Storing sk_buffs as kptrs in map Maciej Fijalkowski
2024-11-26 19:56 ` [External] " Amery Hung
2024-11-26 20:47   ` Martin KaFai Lau
2024-11-27 19:07     ` Maciej Fijalkowski
2024-11-27 20:54       ` Martin KaFai Lau
2024-12-03 20:46         ` Maciej Fijalkowski
2024-12-04 23:24           ` Martin KaFai Lau
2024-12-06 16:24             ` Maciej Fijalkowski
2024-12-07  0:36               ` Martin KaFai Lau [this message]
2024-12-09 13:17                 ` Maciej Fijalkowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=176ad1b2-034b-4b10-93cd-f03391820e24@linux.dev \
    --to=martin.lau@linux.dev \
    --cc=amery.hung@bytedance.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=maciej.fijalkowski@intel.com \
    --cc=magnus.karlsson@intel.com \
    --cc=sreedevi.joshi@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox