> On Feb 5, 2024, at 2:44 PM, NeilBrown <neilb@xxxxxxx> wrote: > > On Tue, 06 Feb 2024, Lorenzo Bianconi wrote: >>> On Fri, 2024-02-02 at 18:08 +0100, Lorenzo Bianconi wrote: >>>>> The existing rpc.nfsd program was designed during a different time, when >>>>> we just didn't require that much control over how it behaved. It's >>>>> klunky to work with. >>>>> >>>>> In a response to Chuck's recent RFC patch to add knob to disable >>>>> READ_PLUS calls, I mentioned that it might be a good time to make a >>>>> clean break from the past and start a new program for controlling nfsd. >>>>> >>>>> Here's what I'm thinking: >>>>> >>>>> Let's build a swiss-army-knife kind of interface like git or virsh: >>>>> >>>>> # nfsdctl stats <--- fetch the new stats that got merged >>>>> # nfsdctl add_listener <--- add a new listen socket, by address or hostname >>>>> # nfsdctl set v3 on <--- enable NFSv3 >>>>> # nfsdctl set splice_read off <--- disable splice reads (per Chuck's recent patch) >>>>> # nfsdctl set threads 128 <--- spin up the threads >>>>> >>>>> We could start with just the bare minimum for now (the stats interface), >>>>> and then expand on it. Once we're at feature parity with rpc.nfsd, we'd >>>>> want to have systemd preferentially use nfsdctl instead of rpc.nfsd to >>>>> start and stop the server. systemd will also need to fall back to using >>>>> rpc.nfsd if nfsdctl or the netlink program isn't present. >>>>> >>>>> Note that I think this program will have to be a compiled binary vs. a >>>>> python script or the like, given that it'll be involved in system >>>>> startup. >>>>> >>>>> It turns out that Lorenzo already has a C program that has a lot of the >>>>> plumbing we'd need: >>>>> >>>>> https://github.com/LorenzoBianconi/nfsd-netlink >>>> >>>> This is something I developed just for testing the new interface but I agree we >>>> could start from it. >>>> >>>> Regarding the kernel part I addressed the comments I received upstream on v6 and >>>> pushed the code here [0]. >>>> How do you guys prefer to proceed? Is the better to post v7 upstream and continue >>>> the discussion in order to have something usable to develop the user-space part or >>>> do you prefer to have something for the user-space first? >>>> I do not have a strong opinion on it. >>>> >>>> Regards, >>>> Lorenzo >>>> >>>> [0] https://github.com/LorenzoBianconi/nfsd-next/tree/nfsd-next-netlink-new-cmds-public-v7 >>>> >>>> >>> >>> My advice? >>> >>> Step back and spend some time working on the userland bits before >>> posting another revision. Experience has shown that you never realize >>> what sort of warts an interface like this has until you have to work >>> with it. >>> >>> You may find that you want to tweak it some once you do, and it's much >>> easier to do that before we merge anything. This will be part of the >>> kernel ABI, so once it's in a shipping kernel, we're sort of stuck with >>> it. >>> >>> Having a userland program ready to go will allow us to do things like >>> set up the systemd service for this too, which is primarily how this new >>> program will be called. >> >> I agree on it. In order to proceed I guess we should define a list of >> requirements/expected behaviour on this new user-space tool used to >> configure nfsd. I am not so familiar with the user-space requirements >> for nfsd so I am just copying what you suggested, something like: >> >> $ nfsdctl stats <--- fetch the new stats that got merged >> $ nfsdctl xprt add proto <udp|tcp> host <host> [port <port>] <--- add a new listen socket, by address or hostname >> $ nfsdctl proto v3.0 v4.0 v4.1 <--- enable NFSv3 and v4.1 >> $ nfsdctl set threads 128 <--- spin up the threads > > My preference would be: > > nfsdctl start > and > nfsdctl stop > > where nfsdctl reads a config file to discover what setting are required. > I cannot see any credible use case for 'xprt' or 'proto' or 'threads' > commands. > > Possibly nfsctl would accept config on the command line: > nfsdctl start proto=3,4.1 threads=42 proto=tcp:localhost:2049 > or similar. You've got proto= listed twice here. I'm more in favor of having more subcommands, each of which do something simple. Easier to understand, easier to test. > It would also be helpful to have "nfsdinfo" or similar which has "stats" > and "status" commands. Maybe that could be combined with "nfsdctl", but > extracting info is not a form of "control". Or we could find a more > generic verb. For soft-raid I wrote "mdadm" "adm" for "administer" > which seemed less specific than "control" (mdctl). Though probably the > main reason was that I like palindromes and "mdadm" was a bit easier to > say. nfsdadm ?? (see also udevadm, drdbadm, fsadm ....) But maybe I'm > just too fuss. > > In my experience working with our customers and support team, they are > not at all interested in fine control. This is an interface to be used by systemctl. I don't think customers or support teams would ever need to make use of it directly. > "systemctl restart nfs-server" is > their second preference when "reboot" isn't appropriate for some reason. > > You might be able to convince me that "nfdctl reload" was useful - it > could be called from "systemctl reload nfs-server". That would then > justify kernel interfaces to remove xprts. But having all the > fine-control just increases the required testing needed with little > practical gain. > > NeilBrown -- Chuck Lever