Date:   Mon, 23 Jan 2023 18:54:10 -0600
From:   David Vernet <void@manifault.com>
To:     Jonathan Corbet <corbet@lwn.net>
Cc:     Daniel Borkmann <daniel@iogearbox.net>,
        Alexei Starovoitov <alexei.starovoitov@gmail.com>,
        bpf@vger.kernel.org, ast@kernel.org, andrii@kernel.org,
        martin.lau@linux.dev, song@kernel.org, yhs@meta.com,
        john.fastabend@gmail.com, kpsingh@kernel.org, sdf@google.com,
        haoluo@google.com, jolsa@kernel.org, linux-kernel@vger.kernel.org,
        kernel-team@meta.com, memxor@gmail.com
Subject: Re: [PATCH bpf-next v2 3/3] bpf: Use BPF_KFUNC macro at all kfunc
 definitions
Message-ID: <Y88sMlmrq0wCFSRP@maniforge.lan>
References: <20230123171506.71995-1-void@manifault.com>
 <20230123171506.71995-4-void@manifault.com>
 <20230123183305.2mgoxgw4ca3sfk24@macbook-pro-6.dhcp.thefacebook.com>
 <Y87We/92xiv5/+g+@maniforge.lan>
 <20230123185434.ybfhrmbootcnjuoj@macbook-pro-6.dhcp.thefacebook.com>
 <ebff2166-8a70-af62-b859-6b5c6b008b36@iogearbox.net>
 <87o7qphspq.fsf@meer.lwn.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87o7qphspq.fsf@meer.lwn.net>
User-Agent: Mutt/2.2.9 (2022-11-12)
Precedence: bulk

On Mon, Jan 23, 2023 at 02:00:17PM -0700, Jonathan Corbet wrote:
> Daniel Borkmann <daniel@iogearbox.net> writes:
> 
> > Did you look into making this similar to the EXPORT_SYMBOL() infra? If possible
> > that would look much more natural to developers, e.g. :
> >
> > struct nf_conn *
> > bpf_skb_ct_lookup(struct __sk_buff *skb_ctx, struct bpf_sock_tuple *bpf_tuple,
> >   		  u32 tuple__sz, struct bpf_ct_opts *opts, u32 opts__sz)
> > {
> > 	[...]
> > }
> >
> > EXPORT_BPF_KFUNC(bpf_skb_ct_lookup);
> 
> That was my question too; it's a similar functionality that would be
> nice to express in a similar way.  Even better, if possible, might be to
> fold it into BTF_ID_FLAGS, which I might then rename to EXPORT_KFUNC()
> or some such ... :)

Thanks Daniel and Jon for taking a look. These are good suggestions, and
I agree that using something like EXPORT_BPF_KFUNC or BTF_ID_FLAGS would
be preferred and probably a better and more intuitive UX.

I expect that it's going to require some nontrivial build integration
work to get this looking exactly like we want it to. AFAICT, one
difference between kfuncs and EXPORT_SYMBOL symbols is that
EXPORT_SYMBOL symbol signatures are all exported via headers, as the
intention is of course for them to be linked against by other core
kernel code or modules. For kfuncs, that's not really what we want given
that for many of the kfuncs we may not want non-BPF callers to invoke
them. I don't think any of this is impossible, just a SMOP.

I was perhaps a bit naive to think we could just throw a __bpf_kfunc
macro onto the function signatures and call it a day :-) I think it's
probably best to table this for now, and either I or someone else can
come back to it when we have bandwidth to solve the problem more
appropriately.

> 
> Thanks,
> 
> jon