kdevops.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Chuck Lever III <chuck.lever@oracle.com>
To: Jeff Layton <jlayton@kernel.org>
Cc: "kdevops@lists.linux.dev" <kdevops@lists.linux.dev>
Subject: Re: [PATCH] fstests: add an option for testing nfs over rdma
Date: Fri, 26 Jan 2024 18:32:24 +0000	[thread overview]
Message-ID: <9AE610FB-4614-42B8-8938-BAC7614F1078@oracle.com> (raw)
In-Reply-To: <5a283b346a6e2b40ef0ebdd3f9bbeb1b7b81e869.camel@kernel.org>



> On Jan 26, 2024, at 1:14 PM, Jeff Layton <jlayton@kernel.org> wrote:
> 
> 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.

I thought "./check" pulled the mount options either from a
config file or from environment variables. But, OK, I haven't
looked closely at exactly what the fstests workflow does.


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

That would basically enable us to add a common set of mount
options to test to each SECTION, so I guess it would amount
to the same thing as I was 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>


--
Chuck Lever



  reply	other threads:[~2024-01-26 18:32 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-26 16:58 [PATCH] fstests: add an option for testing nfs over rdma Jeff Layton
2024-01-26 17:23 ` Chuck Lever III
2024-01-26 17:45   ` Jeff Layton
2024-01-26 17:54     ` Chuck Lever III
2024-01-26 18:14       ` Jeff Layton
2024-01-26 18:32         ` Chuck Lever III [this message]
  -- strict thread matches above, loose matches on Subject: below --
2024-03-01 20:39 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=9AE610FB-4614-42B8-8938-BAC7614F1078@oracle.com \
    --to=chuck.lever@oracle.com \
    --cc=jlayton@kernel.org \
    --cc=kdevops@lists.linux.dev \
    /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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).