public inbox for linux-nfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Chuck Lever <chuck.lever@oracle.com>
To: NeilBrown <neilb@suse.de>
Cc: Jeff Layton <jlayton@kernel.org>,
	Lorenzo Bianconi <lorenzo@kernel.org>,
	linux-nfs@vger.kernel.org, lorenzo.bianconi@redhat.com
Subject: Re: [PATCH] NFSD: add version field to nfsd_rpc_status_show handler
Date: Tue, 8 Aug 2023 21:04:29 -0400	[thread overview]
Message-ID: <ZNLmHZ/leArMDsEE@tissot.1015granger.net> (raw)
In-Reply-To: <169153110624.32308.3596310364486971122@noble.neil.brown.name>

On Wed, Aug 09, 2023 at 07:45:06AM +1000, NeilBrown wrote:
> On Wed, 09 Aug 2023, Chuck Lever wrote:
> > On Tue, Aug 08, 2023 at 10:20:44AM -0400, Jeff Layton wrote:
> > > 
> > > It would probably be fairly simple to output well-formed yaml instead.
> > > JSON and XML are a bit more of a pain.
> > 
> > If folks don't mind, I would like more structured output like one of
> > these self-documenting formats. (I know I said I didn't care before,
> > but I'm beginning to care now ;-)
> 
> Lustre, which I am somewhat involved with, uses YAML for various things.
> If someone else introduced yaml-producing sysfs files to the kernel
> first, that might make the path for lustre smoother :-)

It worries me that there isn't yet kernel infrastructure for
formating yaml in sysfs files. That broadens the scope of this
work significantly.


> Another option is netlink which lustre is stating to use for
> configuration and stats.  It is a self-describing format.  The code
> looks verbose, but it is widely used in the kernel and so well supported.

I just spent the last 6 months building a netlink upcall to handle
TLS handshake requests for in-kernel TLS consumers. It is built on
the recently-added yaml netlink specs and code generator. The yaml
netlink specs are kept under:

  Documentation/netlink/specs/

Using netlink would give us a lot of infrastructure for this
facility, but I'm not sure it's worth the extra complexity. And it
would /require/ the use of user space tooling (ie, not 'cat') to get
to the information exported from the kernel. <shrug>


> > I'm also wondering if we really ought not add another file under
> > /proc, which is essentially obsolete. Would /sys/fs/nfsd/yada be
> > better for this facility?
> 
> It is only under /proc because that is where it is mounted by default :-)
> I think it might be sensible to create a node under /sys where all the
> content of the nfsd filesystem also appears.

There are things in the nfsd filesystem that really belong under
/proc/net/rpc or elsewhere, so IMO such migration needs to be
handled on a case-by-case basis -- different project for another
time.


> I'm not keen on /sys/fs/nfsd because nfsd isn't a filesystem, it is a
> service.

How about /sys/module/nfsd ?


> > I hesitate to even mention network namespaces...
> 
> Please do mention them - I find them too easy to forget about.
> /proc/fs/nfsd/ inherits the network namespace from whoever mounts it.
> So this can work perfectly.
> If we created a mirror in /sys/ we would presumably use the namespace of
> the process that opens the file.

I agree: the network namespace of the process that opens the
rpc_status file is just what we want to limit access to in-flight
requests. The current network namespace of each thread is available
via SVC_NET(rqst), so it should be quite simple to display only
in-flight requests that match the opener's namespace.


-- 
Chuck Lever

  reply	other threads:[~2023-08-09  1:04 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-08  8:21 [PATCH] NFSD: add version field to nfsd_rpc_status_show handler Lorenzo Bianconi
2023-08-08 11:33 ` NeilBrown
2023-08-08 13:24   ` Chuck Lever
2023-08-08 13:48     ` Jeff Layton
2023-08-08 14:03       ` Chuck Lever
2023-08-08 14:20         ` Jeff Layton
2023-08-08 15:18           ` Chuck Lever
2023-08-08 21:45             ` NeilBrown
2023-08-09  1:04               ` Chuck Lever [this message]
2023-08-09  1:29                 ` NeilBrown
2023-08-08 17:56           ` Chuck Lever
2023-08-09  7:49             ` Lorenzo Bianconi
2023-08-09 12:53               ` Chuck Lever
2023-08-08 21:32     ` NeilBrown
2023-08-08 11:41 ` Jeff Layton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZNLmHZ/leArMDsEE@tissot.1015granger.net \
    --to=chuck.lever@oracle.com \
    --cc=jlayton@kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=lorenzo.bianconi@redhat.com \
    --cc=lorenzo@kernel.org \
    --cc=neilb@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox