> On Nov 12, 2023, at 6:09 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote: > > On Sun, 2023-11-12 at 11:02 +0100, Lorenzo Bianconi wrote: >>> On Sat, 2023-11-04 at 12:13 +0100, Lorenzo Bianconi wrote: >>>> Introduce write_threads, write_version and write_ports netlink >>>> commands similar to the ones available through the procfs. >>>> >>>> Changes since v3: >>>> - drop write_maxconn and write_maxblksize for the moment >>>> - add write_version and write_ports commands >>>> Changes since v2: >>>> - use u32 to store nthreads in nfsd_nl_threads_set_doit >>>> - rename server-attr in control-plane in nfsd.yaml specs >>>> Changes since v1: >>>> - remove write_v4_end_grace command >>>> - add write_maxblksize and write_maxconn netlink commands >>>> >>>> This patch can be tested with user-space tool reported below: >>>> https://github.com/LorenzoBianconi/nfsd-netlink.git >>>> This series is based on the commit below available in net-next tree >>>> >>>> commit e0fadcffdd172d3a762cb3d0e2e185b8198532d9 >>>> Author: Jakub Kicinski <kuba@xxxxxxxxxx> >>>> Date: Fri Oct 6 06:50:32 2023 -0700 >>>> >>>> tools: ynl-gen: handle do ops with no input attrs >>>> >>>> The code supports dumps with no input attributes currently >>>> thru a combination of special-casing and luck. >>>> Clean up the handling of ops with no inputs. Create empty >>>> Structs, and skip printing of empty types. >>>> This makes dos with no inputs work. >>>> >>>> Lorenzo Bianconi (3): >>>> NFSD: convert write_threads to netlink commands >>>> NFSD: convert write_version to netlink commands >>>> NFSD: convert write_ports to netlink commands >>>> >>>> Documentation/netlink/specs/nfsd.yaml | 83 ++++++++ >>>> fs/nfsd/netlink.c | 54 ++++++ >>>> fs/nfsd/netlink.h | 8 + >>>> fs/nfsd/nfsctl.c | 267 +++++++++++++++++++++++++- >>>> include/uapi/linux/nfsd_netlink.h | 30 +++ >>>> tools/net/ynl/generated/nfsd-user.c | 254 ++++++++++++++++++++++++ >>>> tools/net/ynl/generated/nfsd-user.h | 156 +++++++++++++++ >>>> 7 files changed, 845 insertions(+), 7 deletions(-) >>>> >>> >>> Nice work, Lorenzo! Now comes the bikeshedding... >> >> Hi Jeff, >> >>> >>> With the nfsdfs interface, we sort of had to split things up into >>> multiple files like this, but it has some drawbacks, in particular with >>> weird behavior when people do things out of order. >> >> what do you mean with 'weird behavior'? Something not expected? >> > > Yeah. > > For instance, if you set up sockets but never write anything to the > "threads" file, those sockets will sit around in perpetuity. Granted > most people use rpc.nfsd to start the server, so this generally doesn't > happen often, but it's always been a klunky interface regardless. > >>> >>> Would it make more sense to instead have a single netlink command that >>> sets up ports and versions, and then spawns the requisite amount of >>> threads, all in one fell swoop? >> >> I do not have a strong opinion about it but I would say having a dedicated >> set/get for each paramater allow us to have more granularity (e.g. if you want >> to change just a parameter we do not need to send all of them to the kernel). >> What do you think? >> > > It's pretty rare to need to twiddle settings on the server while it's up > and running. Restarting the server in the face of even minor changes is > not generally a huge problem, so I don't see this as necessary. I don't have a problem creating a single "set" netlink command that takes a number of optional arguments to adjust nfsd's configuration in a single operation. And a matching "get" command to query all of the server settings at one time. > Also, it's always been a bit hit and miss as to which settings take > immediate effect. For instance, if I (e.g.) turn off NFSv4 serving > altogether on a running server, it doesn't purge the existing NFSv4 > state, but v4 RPCs would be immediately rejected. Eventually it would > time out, but it is odd. > > Personally, I think this is amenable to a declarative interface: > > Have userland always send down a complete description of what the server > should look like, and then the kernel can do what it needs to make that > happen (starting/stopping threads, opening/closing sockets, changing > versions served, etc.). > >>> >>> That does presuppose we can send down a variable-length frame though, >>> but I assume that is possible with netlink. >> >> sure, we can do it. > -- > Jeff Layton <jlayton@xxxxxxxxxx> -- Chuck Lever