On Feb 16, 2012, at 3:40 PM, Myklebust, Trond wrote:
> On Thu, 2012-02-16 at 11:17 -0500, Weston Andros Adamson wrote:
>> Send the nfs implementation id in EXCHANGE_ID requests unless the module
>> parameter nfs.send_implementation_id is 0.
>>
>> This adds a CONFIG variable for the nii_domain that defaults to "kernel.org".
>>
>> Signed-off-by: Weston Andros Adamson <dros@xxxxxxxxxx>
>> ---
>> Documentation/kernel-parameters.txt | 9 ++++++++
>> fs/nfs/Kconfig | 8 +++++++
>> fs/nfs/nfs4xdr.c | 38 +++++++++++++++++++++++++++++++++-
>> 3 files changed, 53 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>> index 1d369c6..7bae0fd 100644
>> --- a/Documentation/kernel-parameters.txt
>> +++ b/Documentation/kernel-parameters.txt
>> @@ -1678,6 +1678,15 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>> back to using the idmapper.
>> To turn off this behaviour, set the value to '0'.
>>
>> + nfs.send_implementation_id =
>> + [NFSv4.1] Send client implementation identification
>> + information in exchange_id requests.
>> + If zero, no implementation identification information
>> + will be sent.
>> + The default is to send the implementation identification
>> + information.
>> +
>> +
>> nmi_debug= [KNL,AVR32,SH] Specify one or more actions to take
>> when a NMI is triggered.
>> Format: [state][,regs][,debounce][,die]
>> diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig
>> index ee86cfc..09f9e38 100644
>> --- a/fs/nfs/Kconfig
>> +++ b/fs/nfs/Kconfig
>> @@ -99,6 +99,14 @@ config PNFS_OBJLAYOUT
>> depends on NFS_FS && NFS_V4_1 && SCSI_OSD_ULD
>> default m
>>
>> +config NFS_V4_1_IMPLEMENTATION_ID_DOMAIN
>> + string "NFSv4.1 Implementation ID Domain"
>> + depends on NFS_V4_1
>> + default "kernel.org"
>> + help
>> + This value will be used in EXCHANGE_ID compounds to help identify
>> + the client implementation.
>
> This needs more fleshing out in order to explain what the purpose is
> (identify the DNS domainname of the distribution), and what format the
> string needs to take (DNS domainname). You might also follow up with a
> recommendation that if the NFS client is unchanged from the upstream
> kernel, then people should use the default 'kernel.org'.
Understood.
>
>> +
>> config ROOT_NFS
>> bool "Root file system on NFS"
>> depends on NFS_FS=y && IP_PNP
>> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
>> index ae78343..70ded94 100644
>> --- a/fs/nfs/nfs4xdr.c
>> +++ b/fs/nfs/nfs4xdr.c
>> @@ -44,6 +44,8 @@
>> #include <linux/pagemap.h>
>> #include <linux/proc_fs.h>
>> #include <linux/kdev_t.h>
>> +#include <linux/module.h>
>> +#include <linux/utsname.h>
>> #include <linux/sunrpc/clnt.h>
>> #include <linux/sunrpc/msg_prot.h>
>> #include <linux/sunrpc/gss_api.h>
>> @@ -271,7 +273,12 @@ static int nfs4_stat_to_errno(int);
>> 1 /* flags */ + \
>> 1 /* spa_how */ + \
>> 0 /* SP4_NONE (for now) */ + \
>> - 1 /* zero implemetation id array */)
>> + 1 /* implemetation id array of size 1 */ + \
>> + 1 /* nii_domain */ + \
>> + XDR_QUADLEN(NFS4_OPAQUE_LIMIT) + \
>> + 1 /* nii_name */ + \
>> + XDR_QUADLEN(NFS4_OPAQUE_LIMIT) + \
>> + 3 /* nii_date */)
>> #define decode_exchange_id_maxsz (op_decode_hdr_maxsz + \
>> 2 /* eir_clientid */ + \
>> 1 /* eir_sequenceid */ + \
>> @@ -838,6 +845,12 @@ const u32 nfs41_maxread_overhead = ((RPC_MAX_HEADER_WITH_AUTH +
>> XDR_UNIT);
>> #endif /* CONFIG_NFS_V4_1 */
>>
>> +static unsigned short send_implementation_id = 1;
>> +
>> +module_param(send_implementation_id, ushort, 0644);
>> +MODULE_PARM_DESC(send_implementation_id,
>> + "Send implementation ID with NFSv4.1 exchange_id");
>> +
>> static const umode_t nfs_type2fmt[] = {
>> [NF4BAD] = 0,
>> [NF4REG] = S_IFREG,
>> @@ -1766,6 +1779,8 @@ static void encode_exchange_id(struct xdr_stream *xdr,
>> struct compound_hdr *hdr)
>> {
>> __be32 *p;
>> + char impl_name[260]; /* 4 strs max 64 each, spaces and null */
> ^^^^ Shouldn't you be putting this in terms of
> NFS4_OPAQUE_LIMIT etc like you did above?
The NFS4_OPAQUE_LIMIT is from the spec - that is, a server could make an nii_name string that big.
Creating the client's nii_name is bound by utsname() size limits (__NEW_UTS_LEN = 64).
If you'd prefer me to just use NFS4_OPAQUE_LIMIT, I have no problem with it.
>
>> + int len = 0;
>>
>> p = reserve_space(xdr, 4 + sizeof(args->verifier->data));
>> *p++ = cpu_to_be32(OP_EXCHANGE_ID);
>> @@ -1776,7 +1791,26 @@ static void encode_exchange_id(struct xdr_stream *xdr,
>> p = reserve_space(xdr, 12);
>> *p++ = cpu_to_be32(args->flags);
>> *p++ = cpu_to_be32(0); /* zero length state_protect4_a */
>> - *p = cpu_to_be32(0); /* zero length implementation id array */
>> +
>> + if (send_implementation_id)
>> + len = snprintf(impl_name, sizeof(impl_name), "%s %s %s %s",
>> + utsname()->sysname, utsname()->release,
>> + utsname()->version, utsname()->machine);
>> +
>
> Can we put in a compile-time check on
> CONFIG_NFS_V4_1_IMPLEMENTATION_ID_DOMAIN here to make sure that the
> length is bounded?
Yes, good call. It should also check to make sure its not an empty string -- some servers (pynfs at least) blow up when you send a string of size 0 here.
>
>> + if (len > 0) {
>> + *p = cpu_to_be32(1); /* implementation id array length=1 */
>> +
>> + encode_string(xdr,
>> + strlen(CONFIG_NFS_V4_1_IMPLEMENTATION_ID_DOMAIN),
>> + CONFIG_NFS_V4_1_IMPLEMENTATION_ID_DOMAIN);
>> + encode_string(xdr, len, impl_name);
>> + /* just send zeros for nii_date - the date is in nii_name */
>> + p = reserve_space(xdr, 12);
>> + p = xdr_encode_hyper(p, 0);
>> + *p = cpu_to_be32(0);
>> + } else
>> + *p = cpu_to_be32(0); /* implementation id array length=0 */
>> +
>> hdr->nops++;
>> hdr->replen += decode_exchange_id_maxsz;
>> }
Thanks,
-drosAttachment:
smime.p7s
Description: S/MIME cryptographic signature
