Re: [PATCH] fstests: add an option for testing nfs over rdma

[Jeff Layton <jlayton@kernel.org>]

On Fri, 2024-01-26 at 17:54 +0000, Chuck Lever III wrote:
> 
> > On Jan 26, 2024, at 12:45 PM, Jeff Layton <jlayton@kernel.org> wrote:
> > 
> > On Fri, 2024-01-26 at 17:23 +0000, Chuck Lever III wrote:
> > > 
> > > > On Jan 26, 2024, at 11:58 AM, Jeff Layton <jlayton@kernel.org> wrote:
> > > > 
> > > > This adds a new Kconfig option for testing NFS over RDMA.
> > > > 
> > > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > > ---
> > > > playbooks/roles/fstests/templates/nfs/nfs.config | 7 +++++++
> > > > playbooks/roles/nfsd/templates/nfs.conf.j2       | 1 +
> > > > workflows/fstests/nfs/Kconfig                    | 8 ++++++++
> > > > workflows/fstests/nfs/Makefile                   | 3 +++
> > > > 4 files changed, 19 insertions(+)
> > > > 
> > > > diff --git a/playbooks/roles/fstests/templates/nfs/nfs.config b/playbooks/roles/fstests/templates/nfs/nfs.config
> > > > index e2265f3f3ee2..77d583d84bc2 100644
> > > > --- a/playbooks/roles/fstests/templates/nfs/nfs.config
> > > > +++ b/playbooks/roles/fstests/templates/nfs/nfs.config
> > > > @@ -15,6 +15,13 @@ CANON_DEVS=yes
> > > > # Test with default mount options
> > > > [nfs_default]
> > > > {% endif %}
> > > > +{% if fstests_nfs_section_rdma -%}
> > > > +
> > > > +# Test over RDMA
> > > > +[nfs_rdma]
> > > > +TEST_FS_MOUNT_OPTS="-o rdma"
> > > > +MOUNT_OPTIONS="-o rdma"
> > > > +{% endif %}
> > > > {% if fstests_nfs_section_tls -%}
> > > 
> > > I was thinking this kind of adjustment to the mount
> > > options could be done with a += instead of just
> > > setting a fixed value.
> > > 
> > > That would make it easier to test combinations of
> > > settings by enabling more than one Kconfig option.
> > > 
> > 
> > Here, I don't think it matters. The string will be blank before this
> > point anyway and we only set it once for each section. Making the
> > fstests config template more flexible sounds interesting though. How
> > would we structure the kconfig menu options for that?
> 
> grep for ANSIBLE_EXTRA_ARGS -- basically when a bool Kconfig option
> is set, a Makefile somewhere does:
> 
> ANSIBLE_EXTRA_ARGS += "kdevop_option_we_want_to_set = true"
> 
> So, same idea:
> 
> We can set up an NFS_EXTRA_MOUNT_OPTS variable as an empty string,
> and just have Makefile processing that adds "rdma" or "udp" or
> "xprtsec=tls" or whatever. We'd have to deal with adding commas
> too, so maybe collect the options in an Ansible dictionary instead?
> Dunno.
> 
> Then when the test workflow runs it appends NFS_EXTRA_MOUNT_OPTS
> to it's mount option string.
> 

One thing to consider is that the guests that the fstests dedicated
workflow spins up are dependent on config file options named like:

	config_val: "{{ 'CONFIG_FSTESTS_' + fs + '_SECTION_' }}"


That's how I was able to spin up a new guest variant while only touching
a few files. I think if you want to modify how the mount options are
handled, you'll need to ensure that that still works somehow.

That's the part I don't quite get. I think for now we're sort of stuck
with the rigid mount options, but there may be some way around that
eventually.

One idea:

We could add a new freeform string Kconfig option that is "base mount
options that all guests should use" and then have the different
CONFIG_FSTESTS_NFS_SECTION_* options just add their own mount options to
that?

Is that sort of what you were thinking?

> > > > # Test NFS with RPC over TLS
> > > > diff --git a/playbooks/roles/nfsd/templates/nfs.conf.j2 b/playbooks/roles/nfsd/templates/nfs.conf.j2
> > > > index 1be49ca74120..79ca7c0a7a55 100644
> > > > --- a/playbooks/roles/nfsd/templates/nfs.conf.j2
> > > > +++ b/playbooks/roles/nfsd/templates/nfs.conf.j2
> > > > @@ -1,3 +1,4 @@
> > > > [nfsd]
> > > > udp=y
> > > > +rdma=y
> > > > threads={{ nfsd_threads }}
> > > > diff --git a/workflows/fstests/nfs/Kconfig b/workflows/fstests/nfs/Kconfig
> > > > index 7e8731dc4dc1..5bbf0a34a124 100644
> > > > --- a/workflows/fstests/nfs/Kconfig
> > > > +++ b/workflows/fstests/nfs/Kconfig
> > > > @@ -54,6 +54,14 @@ config FSTESTS_NFS_SECTION_DEFAULT
> > > > time of this writing, this makes the client autonegotiate an NFS
> > > > version, starting with v4.2 if it's available.
> > > > 
> > > > +config FSTESTS_NFS_SECTION_RDMA
> > > > + bool "Enable testing section: nfs_rdma"
> > > > + default n
> > > > + help
> > > > +  Enabling this will test with the "rdma" mount option. Unless the
> > > > +  hosts have RDMA hardware already, you probably also want to enable
> > > > +  CONFIG_KDEVOPS_SETUP_SIW as well.
> > > > +
> > > > config FSTESTS_NFS_SECTION_TLS
> > > > bool "Enable testing section: nfs_tls"
> > > > default n
> > > > diff --git a/workflows/fstests/nfs/Makefile b/workflows/fstests/nfs/Makefile
> > > > index 0e5245920ee9..2daa058a5377 100644
> > > > --- a/workflows/fstests/nfs/Makefile
> > > > +++ b/workflows/fstests/nfs/Makefile
> > > > @@ -9,6 +9,9 @@ FSTESTS_ARGS += fstests_nfs_server_host='$(FSTESTS_NFS_SERVER_HOST)'
> > > > ifeq (y,$(CONFIG_FSTESTS_NFS_SECTION_DEFAULT))
> > > > FSTESTS_ARGS += fstests_nfs_section_default=True
> > > > endif
> > > > +ifeq (y,$(CONFIG_FSTESTS_NFS_SECTION_RDMA))
> > > > +FSTESTS_ARGS += fstests_nfs_section_rdma=True
> > > > +endif
> > > > ifeq (y,$(CONFIG_FSTESTS_NFS_SECTION_TLS))
> > > > FSTESTS_ARGS += fstests_nfs_section_tls=True
> > > > endif
> > > > -- 
> > > > 2.43.0
> > > > 
> > > > 
> > > 
> > > --
> > > Chuck Lever
> > > 
> > > 
> > 
> > -- 
> > Jeff Layton <jlayton@kernel.org>
> > 
> 
> --
> Chuck Lever
> 
> 

-- 
Jeff Layton <jlayton@kernel.org>