Re: [PATCH v10 5/9] bpf: Add bpf_lookup_*_key() and bpf_key_put() kfuncs

From: "Daniel Müller" <deso@posteo.net>
To: Roberto Sassu <roberto.sassu@huawei.com>
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>,
	"ast@kernel.org" <ast@kernel.org>,
	"daniel@iogearbox.net" <daniel@iogearbox.net>,
	"andrii@kernel.org" <andrii@kernel.org>,
	"martin.lau@linux.dev" <martin.lau@linux.dev>,
	"song@kernel.org" <song@kernel.org>, "yhs@fb.com" <yhs@fb.com>,
	"john.fastabend@gmail.com" <john.fastabend@gmail.com>,
	"kpsingh@kernel.org" <kpsingh@kernel.org>,
	"sdf@google.com" <sdf@google.com>,
	"haoluo@google.com" <haoluo@google.com>,
	"jolsa@kernel.org" <jolsa@kernel.org>,
	"corbet@lwn.net" <corbet@lwn.net>,
	"dhowells@redhat.com" <dhowells@redhat.com>,
	"jarkko@kernel.org" <jarkko@kernel.org>,
	"rostedt@goodmis.org" <rostedt@goodmis.org>,
	"mingo@redhat.com" <mingo@redhat.com>,
	"paul@paul-moore.com" <paul@paul-moore.com>,
	"jmorris@namei.org" <jmorris@namei.org>,
	"serge@hallyn.com" <serge@hallyn.com>,
	"shuah@kernel.org" <shuah@kernel.org>,
	"bpf@vger.kernel.org" <bpf@vger.kernel.org>,
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
	"keyrings@vger.kernel.org" <keyrings@vger.kernel.org>,
	"linux-security-module@vger.kernel.org" 
	<linux-security-module@vger.kernel.org>,
	"linux-kselftest@vger.kernel.org"
	<linux-kselftest@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v10 5/9] bpf: Add bpf_lookup_*_key() and bpf_key_put() kfuncs
Date: Mon, 15 Aug 2022 16:22:27 +0000	[thread overview]
Message-ID: <20220815162227.clqx44cwka7yt2u3@muellerd-fedora-PC2BDTX9> (raw)
In-Reply-To: <bff9efc2121046d78e50f0a270d13dc3@huawei.com>

On Fri, Aug 12, 2022 at 08:11:00AM +0000, Roberto Sassu wrote:
> > From: Daniel Müller [mailto:deso@posteo.net]
> > Sent: Friday, August 12, 2022 1:52 AM
> > On Thu, Aug 11, 2022 at 12:02:57PM +0000, Roberto Sassu wrote:
> > > > From: Roberto Sassu [mailto:roberto.sassu@huawei.com]
> > > > Sent: Thursday, August 11, 2022 9:47 AM
> > > > > From: Alexei Starovoitov [mailto:alexei.starovoitov@gmail.com]
> > > > > Sent: Wednesday, August 10, 2022 11:34 PM
> > > > > On Wed, Aug 10, 2022 at 06:59:28PM +0200, Roberto Sassu wrote:
> > > > > > +
> > > > > > +static int __init bpf_key_sig_kfuncs_init(void)
> > > > > > +{
> > > > > > +	int ret;
> > > > > > +
> > > > > > +	ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING,
> > > > > > +					&bpf_key_sig_kfunc_set);
> > > > > > +	if (!ret)
> > > > > > +		return 0;
> > > > > > +
> > > > > > +	return register_btf_kfunc_id_set(BPF_PROG_TYPE_LSM,
> > > > > > +					 &bpf_key_sig_kfunc_set);
> > > > >
> > > > > Isn't this a watery water ?
> > > > > Don't you have a patch 1 ?
> > > > > What am I missing ?
> > > >
> > > > Uhm, yes. I had doubts too. That was what also KP did.
> > > >
> > > > It makes sense to register once, since we mapped LSM to
> > > > TRACING.
> > > >
> > > > Will resend only this patch. And I will figure out why CI failed.
> > >
> > > Adding in CC Daniel Müller, which worked on this.
> > >
> > > I think the issue is that some kernel options are set to =m.
> > > This causes the CI to miss all kernel modules, since they are
> > > not copied to the virtual machine that executes the tests.
> > >
> > > I'm testing this patch:
> > >
> > > https://github.com/robertosassu/libbpf-
> > ci/commit/b665e001b58c4ddb792a2a68098ea5dc6936b15c
> > 
> > I commented on the pull request. Would it make sense to adjust the
> > kernel configuration in this repository instead? I am worried that
> > otherwise everybody may need a similar work around, depending on how
> > selftests are ultimately run.
> 
> The issue seems specific of the eBPF CI. Others might be able to use
> kernel modules.
> 
> Either choice is fine for me.

I understand that depending on how tests are run, kernel modules may be
available to be loaded. My point is that I am not aware of anything that we
would loose by having the functionality built-in to begin with (others can
correct me). So it seems as if that's an easy way to sidestep any issues of that
sort from the start and, hence, would be my preference.

Thanks,
Daniel