2008-03-14 18:15:42

by Chuck Lever

[permalink] [raw]
Subject: [PATCH 5/5] NFS: NFS_V4 and NFSD_V4 should depend on SUNRPC_GSS

There is a guideline in Documentation/kbuild/kconfig-language.txt that
states "In general use select only for non-visible symbols (no prompts
anywhere) and for symbols with no dependencies." Both CONFIG_NFS_V4 and
CONFIG_NFSD_V4 select RPCSEC_GSS_KRB5, which is visible, and has
dependencies.

For the sake of addressing the kconfig-language recommendation,
"select RPCSEC_GSS_KRB5" to "select SUNRPC_GSS".

The server side requires it to build properly, and the client side has it
to provide support for loading GSS capabilities, which RFC 3530 makes
mandatory for NFS version 4.

Signed-off-by: Chuck Lever <[email protected]>
---

fs/Kconfig | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/Kconfig b/fs/Kconfig
index bbc8e52..28b7ba5 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -1628,7 +1628,7 @@ config NFS_V3_ACL
config NFS_V4
bool "NFS client support for NFS version 4 (EXPERIMENTAL)"
depends on NFS_FS && EXPERIMENTAL
- select RPCSEC_GSS_KRB5
+ select SUNRPC_GSS
help
This option enables support for version 4 of the NFS protocol
(RFC 3530) in the kernel's NFS client.
@@ -1719,7 +1719,7 @@ config NFSD_V4
bool "NFS server support for NFS version 4 (EXPERIMENTAL)"
depends on NFSD && PROC_FS && EXPERIMENTAL
select NFSD_V3
- select RPCSEC_GSS_KRB5
+ select SUNRPC_GSS
help
This option enables support in your system's NFS server for
version 4 of the NFS protocol (RFC 3530).



2008-03-17 16:07:59

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH 5/5] NFS: NFS_V4 and NFSD_V4 should depend on SUNRPC_GSS


On Fri, 2008-03-14 at 14:15 -0400, Chuck Lever wrote:
> There is a guideline in Documentation/kbuild/kconfig-language.txt that
> states "In general use select only for non-visible symbols (no prompts
> anywhere) and for symbols with no dependencies." Both CONFIG_NFS_V4 and
> CONFIG_NFSD_V4 select RPCSEC_GSS_KRB5, which is visible, and has
> dependencies.
>
> For the sake of addressing the kconfig-language recommendation,
> "select RPCSEC_GSS_KRB5" to "select SUNRPC_GSS".
>
> The server side requires it to build properly, and the client side has it
> to provide support for loading GSS capabilities, which RFC 3530 makes
> mandatory for NFS version 4.
>
> Signed-off-by: Chuck Lever <[email protected]>
> ---
>
> fs/Kconfig | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/Kconfig b/fs/Kconfig
> index bbc8e52..28b7ba5 100644
> --- a/fs/Kconfig
> +++ b/fs/Kconfig
> @@ -1628,7 +1628,7 @@ config NFS_V3_ACL
> config NFS_V4
> bool "NFS client support for NFS version 4 (EXPERIMENTAL)"
> depends on NFS_FS && EXPERIMENTAL
> - select RPCSEC_GSS_KRB5
> + select SUNRPC_GSS
> help
> This option enables support for version 4 of the NFS protocol
> (RFC 3530) in the kernel's NFS client.
> @@ -1719,7 +1719,7 @@ config NFSD_V4
> bool "NFS server support for NFS version 4 (EXPERIMENTAL)"
> depends on NFSD && PROC_FS && EXPERIMENTAL
> select NFSD_V3
> - select RPCSEC_GSS_KRB5
> + select SUNRPC_GSS
> help
> This option enables support in your system's NFS server for
> version 4 of the NFS protocol (RFC 3530).

NACK. Selecting SUNRPC_GSS is _not_ equivalent to what we have today.
Currently, if you select NFSv4, it will force compilation of the
KerberosV mechanism too. AFAICS, this patch regresses that behaviour.

Trond

--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com

2008-03-17 17:00:36

by Chuck Lever

[permalink] [raw]
Subject: Re: [PATCH 5/5] NFS: NFS_V4 and NFSD_V4 should depend on SUNRPC_GSS

On Mar 17, 2008, at 12:00 PM, Trond Myklebust wrote:
> On Fri, 2008-03-14 at 14:15 -0400, Chuck Lever wrote:
>> There is a guideline in Documentation/kbuild/kconfig-language.txt
>> that
>> states "In general use select only for non-visible symbols (no
>> prompts
>> anywhere) and for symbols with no dependencies." Both
>> CONFIG_NFS_V4 and
>> CONFIG_NFSD_V4 select RPCSEC_GSS_KRB5, which is visible, and has
>> dependencies.
>>
>> For the sake of addressing the kconfig-language recommendation,
>> "select RPCSEC_GSS_KRB5" to "select SUNRPC_GSS".
>>
>> The server side requires it to build properly, and the client side
>> has it
>> to provide support for loading GSS capabilities, which RFC 3530 makes
>> mandatory for NFS version 4.
>>
>> Signed-off-by: Chuck Lever <[email protected]>
>> ---
>>
>> fs/Kconfig | 4 ++--
>> 1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/Kconfig b/fs/Kconfig
>> index bbc8e52..28b7ba5 100644
>> --- a/fs/Kconfig
>> +++ b/fs/Kconfig
>> @@ -1628,7 +1628,7 @@ config NFS_V3_ACL
>> config NFS_V4
>> bool "NFS client support for NFS version 4 (EXPERIMENTAL)"
>> depends on NFS_FS && EXPERIMENTAL
>> - select RPCSEC_GSS_KRB5
>> + select SUNRPC_GSS
>> help
>> This option enables support for version 4 of the NFS protocol
>> (RFC 3530) in the kernel's NFS client.
>> @@ -1719,7 +1719,7 @@ config NFSD_V4
>> bool "NFS server support for NFS version 4 (EXPERIMENTAL)"
>> depends on NFSD && PROC_FS && EXPERIMENTAL
>> select NFSD_V3
>> - select RPCSEC_GSS_KRB5
>> + select SUNRPC_GSS
>> help
>> This option enables support in your system's NFS server for
>> version 4 of the NFS protocol (RFC 3530).
>
> NACK.

A little discussion first would be more friendly.

> Selecting SUNRPC_GSS is _not_ equivalent to what we have today.
> Currently, if you select NFSv4, it will force compilation of the
> KerberosV mechanism too. AFAICS, this patch regresses that behaviour.


Enforcing that behavior breaks the rules of Kconfig.

The purpose of Kconfig is to allow the kernel to build correctly.
The resulting configuration does not have to make sense from a run-
time perspective.

Normally the way this sort of thing is handled is that a note is left
in the help text that says "Oh, and you will find enabling Kerberos V
support useful since the spec requires it." Adding a "default m &&
(NFSV4 || NFSD_V4)" or something like that under Kerberos GSS might
work as well.

But the point is, what I'm removing is not a build dependency; what
I'm replacing it with is a build dependency.

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

2008-03-17 18:31:59

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH 5/5] NFS: NFS_V4 and NFSD_V4 should depend on SUNRPC_GSS


On Mon, 2008-03-17 at 12:59 -0400, Chuck Lever wrote:
> > Selecting SUNRPC_GSS is _not_ equivalent to what we have today.
> > Currently, if you select NFSv4, it will force compilation of the
> > KerberosV mechanism too. AFAICS, this patch regresses that behaviour.
>
>
> Enforcing that behavior breaks the rules of Kconfig.

The current _implementation_ may break the rules, but the behaviour is
definitely correct and should not be changed.

> The purpose of Kconfig is to allow the kernel to build correctly.
> The resulting configuration does not have to make sense from a run-
> time perspective.

While Kconfig may allow it, that still doesn't make it a good policy.

> Normally the way this sort of thing is handled is that a note is left
> in the help text that says "Oh, and you will find enabling Kerberos V
> support useful since the spec requires it." Adding a "default m &&
> (NFSV4 || NFSD_V4)" or something like that under Kerberos GSS might
> work as well.

config RPCSEC_GSS_KRB5
tristate "Secure RPC: Kerberos V mechanism (EXPERIMENTAL)"
depends on SUNRPC && EXPERIMENTAL
default y if (NFS_V4 && NFS_FS=y) || (NFSD_V4 && NFSD=y)
default m if (NFS_V4 && NFS_FS=m) || (NFSD_V4 && NFSD=m)
select ...

> But the point is, what I'm removing is not a build dependency; what
> I'm replacing it with is a build dependency.

See above. "It compiles but doesn't run" isn't acceptable.

--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com