Subject: Re: [PATCH bpf-next v3 1/2] bpf, sockmap: Introduce tracing capability for sockmap

From: "Jiayuan Chen" <[email-masked]>
Date: Tue, 15 Apr 2025 11:04:36 +0000

April 15, 2025 at 05:25, "Cong Wang" <[email-masked]> wrote:

> 
> On Tue, Apr 15, 2025 at 12:11:45AM +0800, Jiayuan Chen wrote:
> 
> > 
> > +#ifndef __TRACE_SOCKMAP_HELPER_ONCE_ONLY
> >  +#define __TRACE_SOCKMAP_HELPER_ONCE_ONLY
> >  +
> >  +enum sockmap_direct_type {
> >  + SOCKMAP_REDIR_NONE = 0,
> >  + SOCKMAP_REDIR_INGRESS,
> >  + SOCKMAP_REDIR_EGRESS,
> >  +};
> > 
> 
> I am curious why you need to define them here since you already pass
> 'ingress' as a parameter? Is it possible to reuse the BPF_F_INGRESS bit?
> Thanks!
>

The lowest bit of skb->_redir being 0 indicates EGRESS, so we cannot use
the built-in __print_flag for output in this case, since it requires the
corresponding bit to be set to 1.

We could certainly do this instead:
'''
if (act != REDIRECT)
    redir = "none"
else if (flag & BPF_F_INGRESS)
    redir = "ingress"
else
    redir = "egress"
'''
However, as Steven mentioned earlier, using an enum instead would be better
here for trace_event.

Of course, we could directly print the hexadecimal value of _redir, but
that would result in poor readability.

Thanks~