On Fri, 13 Sep 2024, Pali Rohár wrote: > NFSv4.1 OP_EXCHANGE_ID response from server may contain server > implementation details (domain, name and build time) in optional > nfs_impl_id4 field. Currently nfsd does not fill this field. > > NFSv4.1 OP_EXCHANGE_ID call request from client may contain client > implementation details and Linux NFSv4.1 client is already filling these > information based on runtime module param "nfs.send_implementation_id" and > build time Kconfig option "NFS_V4_1_IMPLEMENTATION_ID_DOMAIN". Module param > send_implementation_id specify whether to fill implementation fields and > Kconfig option "NFS_V4_1_IMPLEMENTATION_ID_DOMAIN" specify the domain > string. > > Do same in nfsd, introduce new runtime param "nfsd.send_implementation_id" > and build time Kconfig option "NFSD_V4_1_IMPLEMENTATION_ID_DOMAIN" and > based on them fill NFSv4.1 server implementation details in OP_EXCHANGE_ID > response. Logic in nfsd is exactly same as in nfs. > > This aligns Linux NFSv4.1 server logic with Linux NFSv4.1 client logic. > > NFSv4.1 client and server implementation fields are useful for statistic > purposes or for identifying type of clients and servers. > > Signed-off-by: Pali Rohár <pali@xxxxxxxxxx> > --- > fs/nfsd/Kconfig | 12 +++++++++++ > fs/nfsd/nfs4xdr.c | 55 +++++++++++++++++++++++++++++++++++++++++++++-- > 2 files changed, 65 insertions(+), 2 deletions(-) > > diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig > index ec2ab6429e00..70067c29316e 100644 > --- a/fs/nfsd/Kconfig > +++ b/fs/nfsd/Kconfig > @@ -136,6 +136,18 @@ config NFSD_FLEXFILELAYOUT > > If unsure, say N. > > +config NFSD_V4_1_IMPLEMENTATION_ID_DOMAIN > + string "NFSv4.1 Implementation ID Domain" > + depends on NFSD_V4 > + default "kernel.org" > + help > + This option defines the domain portion of the implementation ID that > + may be sent in the NFS exchange_id operation. The value must be in > + the format of a DNS domain name and should be set to the DNS domain > + name of the distribution. > + If the NFS server is unchanged from the upstream kernel, this > + option should be set to the default "kernel.org". > + > config NFSD_V4_2_INTER_SSC > bool "NFSv4.2 inter server to server COPY" > depends on NFSD_V4 && NFS_V4_2 > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c > index b45ea5757652..5e89f999d4c7 100644 > --- a/fs/nfsd/nfs4xdr.c > +++ b/fs/nfsd/nfs4xdr.c > @@ -62,6 +62,9 @@ > #include <linux/security.h> > #endif > > +static bool send_implementation_id = true; > +module_param(send_implementation_id, bool, 0644); > +MODULE_PARM_DESC(send_implementation_id, "Send implementation ID with NFSv4.1 exchange_id"); > > #define NFSDDBG_FACILITY NFSDDBG_XDR > > @@ -4833,6 +4836,53 @@ nfsd4_encode_server_owner4(struct xdr_stream *xdr, struct svc_rqst *rqstp) > return nfsd4_encode_opaque(xdr, nn->nfsd_name, strlen(nn->nfsd_name)); > } > > +#define IMPL_NAME_LIMIT (sizeof(utsname()->sysname) + sizeof(utsname()->release) + \ > + sizeof(utsname()->version) + sizeof(utsname()->machine) + 8) This "+8" seems strange. In the xdr_reserve_space() call below you are very thorough about explaining the magic numbers - which is great. Here that is this unexplained 8. I don't think you need +8 at all. sizeof(string) will give room to print the string plus a trailing space or nul, and that is all you need. Otherwise the patch looks OK. Thanks, NeilBrown > + > +static __be32 > +nfsd4_encode_server_impl_id(struct xdr_stream *xdr) > +{ > + char impl_name[IMPL_NAME_LIMIT]; > + int impl_name_len; > + __be32 *p; > + > + impl_name_len = 0; > + if (send_implementation_id && > + sizeof(CONFIG_NFSD_V4_1_IMPLEMENTATION_ID_DOMAIN) > 1 && > + sizeof(CONFIG_NFSD_V4_1_IMPLEMENTATION_ID_DOMAIN) <= NFS4_OPAQUE_LIMIT) > + impl_name_len = snprintf(impl_name, sizeof(impl_name), "%s %s %s %s", > + utsname()->sysname, utsname()->release, > + utsname()->version, utsname()->machine); > + > + if (impl_name_len <= 0) { > + if (xdr_stream_encode_u32(xdr, 0) != XDR_UNIT) > + return nfserr_resource; > + return nfs_ok; > + } > + > + if (xdr_stream_encode_u32(xdr, 1) != XDR_UNIT) > + return nfserr_resource; > + > + p = xdr_reserve_space(xdr, > + 4 /* nii_domain.len */ + > + (XDR_QUADLEN(sizeof(CONFIG_NFSD_V4_1_IMPLEMENTATION_ID_DOMAIN) - 1) * 4) + > + 4 /* nii_name.len */ + > + (XDR_QUADLEN(impl_name_len) * 4) + > + 8 /* nii_time.tv_sec */ + > + 4 /* nii_time.tv_nsec */); > + if (!p) > + return nfserr_resource; > + > + p = xdr_encode_opaque(p, CONFIG_NFSD_V4_1_IMPLEMENTATION_ID_DOMAIN, > + sizeof(CONFIG_NFSD_V4_1_IMPLEMENTATION_ID_DOMAIN) - 1); > + p = xdr_encode_opaque(p, impl_name, impl_name_len); > + /* just send zeros for nii_date - the date is in nii_name */ > + p = xdr_encode_hyper(p, 0); /* tv_sec */ > + *p++ = cpu_to_be32(0); /* tv_nsec */ > + > + return nfs_ok; > +} > + > static __be32 > nfsd4_encode_exchange_id(struct nfsd4_compoundres *resp, __be32 nfserr, > union nfsd4_op_u *u) > @@ -4867,8 +4917,9 @@ nfsd4_encode_exchange_id(struct nfsd4_compoundres *resp, __be32 nfserr, > if (nfserr != nfs_ok) > return nfserr; > /* eir_server_impl_id<1> */ > - if (xdr_stream_encode_u32(xdr, 0) != XDR_UNIT) > - return nfserr_resource; > + nfserr = nfsd4_encode_server_impl_id(xdr); > + if (nfserr != nfs_ok) > + return nfserr; > > return nfs_ok; > } > -- > 2.20.1 > >