netdev - Re: [PATCH v4 0/3] convert write_threads, write_version and write

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Sun, 12 Nov 2023 11:02:48 +0100
From: Lorenzo Bianconi <lorenzo@...nel.org>
To: Jeff Layton <jlayton@...nel.org>
Cc: linux-nfs@...r.kernel.org, lorenzo.bianconi@...hat.com, neilb@...e.de,
	chuck.lever@...cle.com, netdev@...r.kernel.org, kuba@...nel.org
Subject: Re: [PATCH v4 0/3] convert write_threads, write_version and
 write_ports to netlink commands

> 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@...nel.org>
> > 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?

> 
> 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?

> 
> 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.

Regards,
Lorenzo

> -- 
> Jeff Layton <jlayton@...nel.org>

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)