linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "J. Bruce Fields" <bfields@redhat.com>
To: NeilBrown <neilb@suse.com>
Cc: Steve Dickson <SteveD@redhat.com>, linux-nfs@vger.kernel.org
Subject: Re: [PATCH 03/15] Add /etc/nfs.conf support to rpc.nfsd
Date: Fri, 9 Dec 2016 17:43:50 -0500	[thread overview]
Message-ID: <20161209224349.GD13651@parsley.fieldses.org> (raw)
In-Reply-To: <877f7bs55d.fsf@notabene.neil.brown.name>

On Thu, Dec 08, 2016 at 10:14:54AM +1100, NeilBrown wrote:
> On Thu, Dec 08 2016, "J. Bruce Fields" <bfields@redhat.com> wrote:
> 
> > On Wed, Dec 07, 2016 at 09:44:25AM -0500, Steve Dickson wrote:
> >> > 
> >> > I do wonder if this is ever valid though.  Why do we allow minor
> >> > versions to be enabled/disabled?
> >> IDK... I think Trond did this... you know...
> >> when in doubt... blame Trond! 8-) 
> >
> > Or Benny, 8daf220a6a83 "nfsd41: control nfsv4.1 svc via
> > /proc/fs/nfsd/versions".
> >
> >> > Does it make any sense to enable a non-contiguous set of minor versions?
> >> I don't think so... Talk about handing people rope! ;-)
> >
> > I can't think of a reason either.
> >
> >> > Should we just have a maximum NFSv4 minor version?
> >> Maybe.. 
> >
> > If you do that then I'd allow a minimum too.
> 
> Why?  I'm trying to understand why you would ever want to turn disable a
> particular minor version.  I could equally well ask myself "why specify
> a maximum"...

In the past setting a default maximum that's lower than the maximum
supported by the code has been a way to make new features available for
testers and early adopters while keeping other users safe.

For a minimum--I think "1" would be the most likely minimum, just
because there were some major changes made between 4.0 and 4.1, and
people might eventually want to enforce a policy of supporting only
versions >=4.1 in their environements.

And eventually, I don't know, maybe you really don't want clients
mounting unless they support labeled NFS, or something?  Though you
can't really force them to use new features.  I haven't really thought
it through.

> Benny's patch only suggests that it removes the need for
> CONFIG_NFSD_V4_1, but why was that needed?
> Maybe it is just to disable buggy code if some version is found to have
> an inconvenient bug?  The client, by default, tries 4.2, then 4.1, then
> 4.0.  Older clients might start elsewhere.  So just disabling unreliable
> versions individually does seem to make sense.
> 
> I note that Benny's original patch disabled v4.1 by default, and
> required it to be explicitly enabled.  The current kernel enables all by
> default, and requires them be explicitly disabled is required.
> My current nfs.conf code will disabled unwanted minor versions, but not
> enable any that are already disabled ... I guess I could fix that.

I think what that means is that the defaults are determined by the
kernel version you're running as opposed to by userspace.

The main reason for disabling minor versions has been immature code in
higher versions, and in that case I think it makes sense for the kernel
to provide the default: if you don't have a particular preference, I
think the choice of whether to run 4.1 or 4.0 probably depends on how
new your kernel is.

--b.

> .... it is a shame that we can only enable/disable minor versions
> when there are no nfsd threads running.  The justification for
> failing if nfsd is active is fairly lame, and only applies
> to major versions.  If minor versions could be changed at any time,
> then it could be a simple function of rpc.nfsd, and the config
> file would never need to enable things, only disable them.
> 
> Is there any other reason to disable minor versions, other than to avoid
> buggy code (either in server or client)??
> 
> 
> Actually.... I have a related question that I keep meaning to ask, but
> haven't.
> 
> What are the circumstances where NFSv3 should be preferred over NFSv4
> (assuming up-to-date kernels on server and client)?
> 
> I can think of:
>  - NFSv3 is safer if you might need to support loop-back mounts
>    as I don't think NFSv4 state management can reliably make forward
>    progress against memory pressure.  In particular, creating the
>    state-manager thread can deadlock waiting for an NFS write.
> 
>  - If you want fast-failover from one server to another,
>    NFSv3 is probably faster as it only imposes the grace-time on
>    lock requests.  NFSv4 imposes it on OPEN and so READ/WRITE
>    as well.
> 
>  - NFSv3 is preferred if (for some weird reason) you need
>    to use UDP.
> 
> Are there any others?  Are there any similar reasons to prefer a smaller
> NFSv4 minor version over a larger one?
> 
> Thanks,
> NeilBrown
> 
> >
> > --b.
> >
> >> > I was trying to duplicate the current functionality as closely as
> >> > convenient.  That might not be best in this case.
> >> You did a good job... this is definitely a nit.
> >> 
> >> steved.
> >> 



  parent reply	other threads:[~2016-12-09 22:43 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-02  3:58 [RFC PATCH 00/15] Enhance /etc/nfs.conf usage and remove nfs-config.service NeilBrown
2016-12-02  3:58 ` [PATCH 04/15] Add /etc/nfs.conf support for mountd NeilBrown
2016-12-02  3:58 ` [PATCH 05/15] Add /etc/nfs.conf support for statd NeilBrown
2016-12-02  3:58 ` [PATCH 02/15] conffile: add bool support NeilBrown
2016-12-02  3:58 ` [PATCH 01/15] Add man-page describing /etc/nfs.conf NeilBrown
2016-12-02  3:58 ` [PATCH 03/15] Add /etc/nfs.conf support to rpc.nfsd NeilBrown
2016-12-05 22:27   ` J. Bruce Fields
2016-12-05 22:42     ` NeilBrown
2016-12-06 17:52   ` Steve Dickson
2016-12-06 22:30     ` NeilBrown
2016-12-07 14:34       ` Steve Dickson
2016-12-06 18:51   ` Steve Dickson
2016-12-06 22:36     ` NeilBrown
2016-12-07 14:44       ` Steve Dickson
2016-12-07 18:08         ` J. Bruce Fields
2016-12-07 23:14           ` NeilBrown
2016-12-08  0:38             ` Steve Dickson
2016-12-09 22:43             ` J. Bruce Fields [this message]
2016-12-20 23:22       ` NeilBrown
2016-12-21  1:55         ` J. Bruce Fields
2016-12-02  3:58 ` [PATCH 11/15] conffile: ignore setting of empty string NeilBrown
2016-12-02  3:58 ` [PATCH 06/15] Add /etc/nfs.conf support for sm-notify NeilBrown
2016-12-02  3:58 ` [PATCH 09/15] conffile: add support for include files NeilBrown
2016-12-02  3:58 ` [PATCH 07/15] conffile: free image of config file after parsing NeilBrown
2016-12-02  3:58 ` [PATCH 08/15] conffile: split loading of file into a separate function NeilBrown
2016-12-02  3:58 ` [PATCH 10/15] conffile: strip "quotes" from values in conf file NeilBrown
2016-12-02  3:58 ` [PATCH 13/15] statd: allow --no-notify to be passed via environment variable NeilBrown
2016-12-02  3:58 ` [PATCH 12/15] conffile: allow $name expansion of tag values NeilBrown
2016-12-02  3:58 ` [PATCH 15/15] Add nfs.systemd man page NeilBrown
2016-12-02  3:58 ` [PATCH 14/15] systemd: Remove the nfs-config.service NeilBrown
2016-12-02 15:56 ` [RFC PATCH 00/15] Enhance /etc/nfs.conf usage and remove nfs-config.service J. Bruce Fields
2016-12-06 16:55 ` Steve Dickson
2016-12-06 22:38   ` NeilBrown
2016-12-07 14:24     ` Steve Dickson
2016-12-06 17:26 ` J. Bruce Fields
2016-12-06 22:47   ` NeilBrown
2016-12-07 14:19     ` J. Bruce Fields
2016-12-06 19:25 ` Steve Dickson
2016-12-06 22:51   ` NeilBrown
2016-12-07 14:21     ` Steve Dickson
2016-12-20 18:33 ` Steve Dickson

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=20161209224349.GD13651@parsley.fieldses.org \
    --to=bfields@redhat.com \
    --cc=SteveD@redhat.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neilb@suse.com \
    /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 NNTP newsgroup(s).