All of lore.kernel.org
 help / color / mirror / Atom feed
From: Martin KaFai Lau <martin.lau@linux.dev>
To: Amery Hung <ameryhung@gmail.com>
Cc: alexei.starovoitov@gmail.com, andrii@kernel.org,
	daniel@iogearbox.net, tj@kernel.org, martin.lau@kernel.org,
	kernel-team@meta.com, bpf@vger.kernel.org
Subject: Re: [RFC bpf-next v1 2/4] bpf: Support cookie for linked-based struct_ops attachment
Date: Thu, 10 Jul 2025 12:47:39 -0700	[thread overview]
Message-ID: <2d1b45f3-3bde-415d-8568-eb4c2a7dd219@linux.dev> (raw)
In-Reply-To: <CAMB2axO3Ma7jYa00fbSzB8ZFZyekS13BNJ87rsTfbfcSZhpc6w@mail.gmail.com>

On 7/10/25 11:39 AM, Amery Hung wrote:
>> On 7/8/25 4:08 PM, Amery Hung wrote:
>>> @@ -906,6 +904,10 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
>>>                goto unlock;
>>>        }
>>>
>>> +     err = bpf_struct_ops_prepare_attach(st_map, 0);
>> A follow-up on the "using the map->id as the cookie" comment in the cover
>> letter. I meant to use the map->id here instead of 0. If the cookie is intended
>> to identify a particular struct_ops instance (i.e., the struct_ops map), then
>> map->id should be a good fit, and it is automatically generated by the kernel
>> during the map creation. As a result, I suspect that most of the changes in
>> patch 1 and patch 2 will not be needed.
>>
> Do you mean keep using cookie as the mechanism to associate programs,
> but for struct_ops the cookie will be map->id (i.e.,
> bpf_get_attah_cookie() in struct_ops will return map->id)?

I meant to use the map->id as the bpf_cookie stored in the bpf_tramp_run_ctx. 
Then there is no need for user space to generate a unique cookie during 
link_create. The kernel has already generated a unique ID in the map->id. The 
map->id is available during the bpf_struct_ops_map_update_elem(). Then there is 
also no need to distinguish between SEC(".struct_ops") vs 
SEC(".struct_ops.link"). Most of the patch 1 and patch 2 will not be needed.

A minor detail: note that the same struct ops program can be used in different 
trampolines. Thus, to be specific, the bpf cookie is stored in the trampoline.

If the question is about bpf global variable vs bpf cookie, yeah, I think using 
a bpf global variable should also work. The global variable can be initialized 
before libbpf's bpf_map__attach_struct_ops(). At that time, the map->id should 
be known already. I don't have a strong opinion on reusing the bpf cookie in the 
struct ops trampoline. No one is using it now, so it is available to be used. 
Exposing BPF_FUNC_get_attach_cookie for struct ops programs is pretty cheap 
also. Using bpf cookie to allow the struct ops program to tell which struct_ops 
map is calling it seems to fit well also after sleeping on it a bit. bpf global 
variable will also break if a bpf_prog.o has more than one SEC(".struct_ops").

For tracing program, the bpf cookie seems to be an existing mechanism that can 
have any value (?). Thus, user space is free to store the map->id in it also. It 
can also choose to store the map->id in a bpf global variable if it has other 
uses for the bpf cookie. I think both should work similarly.

  reply	other threads:[~2025-07-10 19:47 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-08 23:08 [RFC bpf-next v1 0/4] Support cookie for link-based struct_ops attachment Amery Hung
2025-07-08 23:08 ` [RFC bpf-next v1 1/4] bpf: Factor out bpf_struct_ops_prepare_attach() Amery Hung
2025-07-08 23:08 ` [RFC bpf-next v1 2/4] bpf: Support cookie for linked-based struct_ops attachment Amery Hung
2025-07-09 22:13   ` Martin KaFai Lau
2025-07-10 18:26     ` Martin KaFai Lau
2025-07-10 18:39     ` Amery Hung
2025-07-10 19:47       ` Martin KaFai Lau [this message]
2025-07-10 21:00         ` Amery Hung
2025-07-11 18:41           ` Andrii Nakryiko
2025-07-11 19:29             ` Amery Hung
2025-07-11 20:21               ` Alexei Starovoitov
2025-07-11 21:38                 ` Amery Hung
2025-07-14 20:46                   ` Andrii Nakryiko
2025-07-14 21:02                     ` Amery Hung
2025-07-14 22:51                       ` Martin KaFai Lau
2025-07-11 21:55                 ` Martin KaFai Lau
2025-07-08 23:08 ` [RFC bpf-next v1 3/4] libbpf: Support link-based struct_ops attach with options Amery Hung
2025-07-08 23:08 ` [RFC bpf-next v1 4/4] selftests/bpf: Test bpf_get_attach_cookie() in struct_ops program Amery Hung

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=2d1b45f3-3bde-415d-8568-eb4c2a7dd219@linux.dev \
    --to=martin.lau@linux.dev \
    --cc=alexei.starovoitov@gmail.com \
    --cc=ameryhung@gmail.com \
    --cc=andrii@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kernel-team@meta.com \
    --cc=martin.lau@kernel.org \
    --cc=tj@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.