On Mon, Jun 02, 2025 at 05:25:25PM +0200, Christoph Hellwig wrote:
> On Sat, May 17, 2025 at 12:45:02PM +0300, Sagi Grimberg wrote:
> >
> >
> > On 16/05/2025 20:03, Jarkko Sakkinen wrote:
> >> On Fri, May 16, 2025 at 02:47:18PM +0300, Sagi Grimberg wrote:
> >>> Reviewed-by: Sagi Grimberg <sagi@xxxxxxxxxxx>
> >> Based on?
> >
> > Based on the same that nvme is doing. The only reason I see to have it
> > is to avoid having the user explicitly set perms on the key for tlshd to be
> > able to load it. nvme creates its own keyring that possessors can use, so
> > makes
> > sense that nfs has this keyring as well.
>
> Jarkoo, can you please state your objections clearly? You've only
> asked this one liner question in response to Sagi's question but not
> even commented the original patch.
OK, I put this in simple terms, so perhaps I learn something from
nvme and nfs code:
1. The code change itself, if this keyring is needed, it looks
reasonable.
2. However, I don't see any callers within the scope of patch set
for this keyring.
I could quite quickly grab the idea how NVME uses nvme_keyring in TLS
handshake code from drivers/nvme/target/{configfs.c,tcp.c}. I guess
similar idea will be used in nfs code but I don't see any use for it
in the patch set.
Thus, it is hard to grasp the idea of having this patch applied without
any supplemental patch set.
BR, Jarkko
