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

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

On 2025/12/19 09:31, Menglong Dong wrote:
> 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.

Sorry I messed it up with the 5th patch. I'll remove this cookie_cnt
and count it directly in __bpf_trampoline_link_prog(). And for the
other comments in the patch, all ACK.

Thanks!
Menglong Dong

> 
> 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 */
> > 
> > [...]
> > 
> 
> 
> 
> 
>