Re: [PATCH bpf-next v4 4/9] bpf: add the kfunc bpf_fsession_cookie

[Menglong Dong <menglong.dong@linux.dev>]

On 2025/12/19 08:55 Andrii Nakryiko <andrii.nakryiko@gmail.com> write:
> On Wed, Dec 17, 2025 at 1:55 AM Menglong Dong <menglong8.dong@gmail.com> wrote:
> >
> > Implement session cookie for fsession. In order to limit the stack usage,
> > we make 4 as the maximum of the cookie count.
> >
> > The offset of the current cookie is stored in the
> > "(ctx[-1] >> BPF_TRAMP_M_COOKIE) & 0xFF". Therefore, we can get the
> > session cookie with ctx[-offset].
> >
> > The stack will look like this:
> >
> >   return value  -> 8 bytes
> >   argN          -> 8 bytes
> >   ...
> >   arg1          -> 8 bytes
> >   nr_args       -> 8 bytes
> >   ip(optional)  -> 8 bytes
> >   cookie2       -> 8 bytes
> >   cookie1       -> 8 bytes
> >
> > Inline the bpf_fsession_cookie() in the verifer too.
> >
> > Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
> > ---
> > v4:
> > - limit the maximum of the cookie count to 4
> > - store the session cookies before nr_regs in stack
> > ---
> >  include/linux/bpf.h      | 16 ++++++++++++++++
> >  kernel/bpf/trampoline.c  | 14 +++++++++++++-
> >  kernel/bpf/verifier.c    | 20 ++++++++++++++++++--
> >  kernel/trace/bpf_trace.c |  9 +++++++++
> >  4 files changed, 56 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index d165ace5cc9b..0f35c6ab538c 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -1215,6 +1215,7 @@ enum {
> >
> >  #define BPF_TRAMP_M_NR_ARGS    0
> >  #define BPF_TRAMP_M_IS_RETURN  8
> > +#define BPF_TRAMP_M_COOKIE     9
> >
> >  struct bpf_tramp_links {
> >         struct bpf_tramp_link *links[BPF_MAX_TRAMP_LINKS];
> > @@ -1318,6 +1319,7 @@ struct bpf_trampoline {
> >         struct mutex mutex;
> >         refcount_t refcnt;
> >         u32 flags;
> > +       int cookie_cnt;
> 
> can't you just count this each time you need to know instead of
> keeping track of this? it's not that expensive and won't happen that
> frequently (and we keep lock on trampoline, so it's also safe and
> race-free to count)

There is a for-loop below that use the "cookie_cnt" to clear all the
cookie to zero. We limited the maximum of cookie_cnt to 4, so
I guess we can count it directly there. I'll change it in the
next version.

Thanks!
Menglong Dong

> 
> >         u64 key;
> >         struct {
> >                 struct btf_func_model model;
> > @@ -1762,6 +1764,7 @@ struct bpf_prog {
> >                                 enforce_expected_attach_type:1, /* Enforce expected_attach_type checking at attach time */
> >                                 call_get_stack:1, /* Do we call bpf_get_stack() or bpf_get_stackid() */
> >                                 call_get_func_ip:1, /* Do we call get_func_ip() */
> > +                               call_session_cookie:1, /* Do we call bpf_fsession_cookie() */
> >                                 tstamp_type_access:1, /* Accessed __sk_buff->tstamp_type */
> >                                 sleepable:1;    /* BPF program is sleepable */
> >         enum bpf_prog_type      type;           /* Type of BPF program */
> 
> [...]
>