| Message ID | 5b0eff4e3ef9bf9621f5095189933f60def40f0d.1690569488.git.lorenzo@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | add rpc_status handler in nfsd debug filesystem | expand |
On Sat, 29 Jul 2023, Lorenzo Bianconi wrote: > Introduce verbose parameter to utility routine in order to reduce output > verbosity. This is a preliminary patch to add rpc_status entry in nfsd > debug filesystem in order to dump pending RPC requests debugging > information. > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > --- > include/linux/sunrpc/svc_xprt.h | 12 ++++++------ > net/sunrpc/svc_xprt.c | 2 +- > 2 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h > index a6b12631db21..1715d4c6bd6b 100644 > --- a/include/linux/sunrpc/svc_xprt.h > +++ b/include/linux/sunrpc/svc_xprt.h > @@ -209,21 +209,21 @@ static inline unsigned short svc_xprt_remote_port(const struct svc_xprt *xprt) > } > > static inline char *__svc_print_addr(const struct sockaddr *addr, > - char *buf, const size_t len) > + char *buf, const size_t len, > + bool verbose) > { > const struct sockaddr_in *sin = (const struct sockaddr_in *)addr; > const struct sockaddr_in6 *sin6 = (const struct sockaddr_in6 *)addr; > > switch (addr->sa_family) { > case AF_INET: > - snprintf(buf, len, "%pI4, port=%u", &sin->sin_addr, > - ntohs(sin->sin_port)); > + snprintf(buf, len, "%pI4, %s%u", &sin->sin_addr, > + verbose ? "port=" : "", ntohs(sin->sin_port)); I would move the "," into the verbose part too. so verbose ? ", port=" : " " Other than that, I like this approach. NeilBrown > break; > > case AF_INET6: > - snprintf(buf, len, "%pI6, port=%u", > - &sin6->sin6_addr, > - ntohs(sin6->sin6_port)); > + snprintf(buf, len, "%pI6, %s%u", &sin6->sin6_addr, > + verbose ? "port=" : "", ntohs(sin6->sin6_port)); > break; > > default: > diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c > index 62c7919ea610..16b794d291a4 100644 > --- a/net/sunrpc/svc_xprt.c > +++ b/net/sunrpc/svc_xprt.c > @@ -386,7 +386,7 @@ EXPORT_SYMBOL_GPL(svc_xprt_copy_addrs); > */ > char *svc_print_addr(struct svc_rqst *rqstp, char *buf, size_t len) > { > - return __svc_print_addr(svc_addr(rqstp), buf, len); > + return __svc_print_addr(svc_addr(rqstp), buf, len, true); > } > EXPORT_SYMBOL_GPL(svc_print_addr); > > -- > 2.41.0 > >
On Sat, 2023-07-29 at 08:13 +1000, NeilBrown wrote: > On Sat, 29 Jul 2023, Lorenzo Bianconi wrote: > > Introduce verbose parameter to utility routine in order to reduce output > > verbosity. This is a preliminary patch to add rpc_status entry in nfsd > > debug filesystem in order to dump pending RPC requests debugging > > information. > > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > > --- > > include/linux/sunrpc/svc_xprt.h | 12 ++++++------ > > net/sunrpc/svc_xprt.c | 2 +- > > 2 files changed, 7 insertions(+), 7 deletions(-) > > > > diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h > > index a6b12631db21..1715d4c6bd6b 100644 > > --- a/include/linux/sunrpc/svc_xprt.h > > +++ b/include/linux/sunrpc/svc_xprt.h > > @@ -209,21 +209,21 @@ static inline unsigned short svc_xprt_remote_port(const struct svc_xprt *xprt) > > } > > > > static inline char *__svc_print_addr(const struct sockaddr *addr, > > - char *buf, const size_t len) > > + char *buf, const size_t len, > > + bool verbose) > > { > > const struct sockaddr_in *sin = (const struct sockaddr_in *)addr; > > const struct sockaddr_in6 *sin6 = (const struct sockaddr_in6 *)addr; > > > > switch (addr->sa_family) { > > case AF_INET: > > - snprintf(buf, len, "%pI4, port=%u", &sin->sin_addr, > > - ntohs(sin->sin_port)); > > + snprintf(buf, len, "%pI4, %s%u", &sin->sin_addr, > > + verbose ? "port=" : "", ntohs(sin->sin_port)); > > I would move the "," into the verbose part too. > so > verbose ? ", port=" : " " > > Other than that, I like this approach. > If we're separating fields in the main seqfile by ' ', then we probably want to use a different delimiter here in the !verbose case. Maybe ':' or ',' instead? Also, ntohs is going to return a short, so the format should probably be using "%hu" for the port. > > > break; > > > > case AF_INET6: > > - snprintf(buf, len, "%pI6, port=%u", > > - &sin6->sin6_addr, > > - ntohs(sin6->sin6_port)); > > + snprintf(buf, len, "%pI6, %s%u", &sin6->sin6_addr, > > + verbose ? "port=" : "", ntohs(sin6->sin6_port)); > > break; > > > > default: > > diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c > > index 62c7919ea610..16b794d291a4 100644 > > --- a/net/sunrpc/svc_xprt.c > > +++ b/net/sunrpc/svc_xprt.c > > @@ -386,7 +386,7 @@ EXPORT_SYMBOL_GPL(svc_xprt_copy_addrs); > > */ > > char *svc_print_addr(struct svc_rqst *rqstp, char *buf, size_t len) > > { > > - return __svc_print_addr(svc_addr(rqstp), buf, len); > > + return __svc_print_addr(svc_addr(rqstp), buf, len, true); > > } > > EXPORT_SYMBOL_GPL(svc_print_addr); > > > > -- > > 2.41.0 > > > > >
On Tue, 01 Aug 2023, Jeff Layton wrote: > On Sat, 2023-07-29 at 08:13 +1000, NeilBrown wrote: > > On Sat, 29 Jul 2023, Lorenzo Bianconi wrote: > > > Introduce verbose parameter to utility routine in order to reduce output > > > verbosity. This is a preliminary patch to add rpc_status entry in nfsd > > > debug filesystem in order to dump pending RPC requests debugging > > > information. > > > > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > > > --- > > > include/linux/sunrpc/svc_xprt.h | 12 ++++++------ > > > net/sunrpc/svc_xprt.c | 2 +- > > > 2 files changed, 7 insertions(+), 7 deletions(-) > > > > > > diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h > > > index a6b12631db21..1715d4c6bd6b 100644 > > > --- a/include/linux/sunrpc/svc_xprt.h > > > +++ b/include/linux/sunrpc/svc_xprt.h > > > @@ -209,21 +209,21 @@ static inline unsigned short svc_xprt_remote_port(const struct svc_xprt *xprt) > > > } > > > > > > static inline char *__svc_print_addr(const struct sockaddr *addr, > > > - char *buf, const size_t len) > > > + char *buf, const size_t len, > > > + bool verbose) > > > { > > > const struct sockaddr_in *sin = (const struct sockaddr_in *)addr; > > > const struct sockaddr_in6 *sin6 = (const struct sockaddr_in6 *)addr; > > > > > > switch (addr->sa_family) { > > > case AF_INET: > > > - snprintf(buf, len, "%pI4, port=%u", &sin->sin_addr, > > > - ntohs(sin->sin_port)); > > > + snprintf(buf, len, "%pI4, %s%u", &sin->sin_addr, > > > + verbose ? "port=" : "", ntohs(sin->sin_port)); > > > > I would move the "," into the verbose part too. > > so > > verbose ? ", port=" : " " > > > > Other than that, I like this approach. > > > > If we're separating fields in the main seqfile by ' ', then we probably > want to use a different delimiter here in the !verbose case. Maybe ':' > or ',' instead? Aren't the address and the port two different fields? Colon is normal for separating host and port, but as IPv6 addresses contain colons, you would need [IP::V6]:port which is probably isn't really an improvement. I would leave address and port as separate fields. > > Also, ntohs is going to return a short, so the format should probably be > using "%hu" for the port. Probably. Thanks, NeilBrown > > > > > > break; > > > > > > case AF_INET6: > > > - snprintf(buf, len, "%pI6, port=%u", > > > - &sin6->sin6_addr, > > > - ntohs(sin6->sin6_port)); > > > + snprintf(buf, len, "%pI6, %s%u", &sin6->sin6_addr, > > > + verbose ? "port=" : "", ntohs(sin6->sin6_port)); > > > break; > > > > > > default: > > > diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c > > > index 62c7919ea610..16b794d291a4 100644 > > > --- a/net/sunrpc/svc_xprt.c > > > +++ b/net/sunrpc/svc_xprt.c > > > @@ -386,7 +386,7 @@ EXPORT_SYMBOL_GPL(svc_xprt_copy_addrs); > > > */ > > > char *svc_print_addr(struct svc_rqst *rqstp, char *buf, size_t len) > > > { > > > - return __svc_print_addr(svc_addr(rqstp), buf, len); > > > + return __svc_print_addr(svc_addr(rqstp), buf, len, true); > > > } > > > EXPORT_SYMBOL_GPL(svc_print_addr); > > > > > > -- > > > 2.41.0 > > > > > > > > > > -- > Jeff Layton <jlayton@kernel.org> >
diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h index a6b12631db21..1715d4c6bd6b 100644 --- a/include/linux/sunrpc/svc_xprt.h +++ b/include/linux/sunrpc/svc_xprt.h @@ -209,21 +209,21 @@ static inline unsigned short svc_xprt_remote_port(const struct svc_xprt *xprt) } static inline char *__svc_print_addr(const struct sockaddr *addr, - char *buf, const size_t len) + char *buf, const size_t len, + bool verbose) { const struct sockaddr_in *sin = (const struct sockaddr_in *)addr; const struct sockaddr_in6 *sin6 = (const struct sockaddr_in6 *)addr; switch (addr->sa_family) { case AF_INET: - snprintf(buf, len, "%pI4, port=%u", &sin->sin_addr, - ntohs(sin->sin_port)); + snprintf(buf, len, "%pI4, %s%u", &sin->sin_addr, + verbose ? "port=" : "", ntohs(sin->sin_port)); break; case AF_INET6: - snprintf(buf, len, "%pI6, port=%u", - &sin6->sin6_addr, - ntohs(sin6->sin6_port)); + snprintf(buf, len, "%pI6, %s%u", &sin6->sin6_addr, + verbose ? "port=" : "", ntohs(sin6->sin6_port)); break; default: diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c index 62c7919ea610..16b794d291a4 100644 --- a/net/sunrpc/svc_xprt.c +++ b/net/sunrpc/svc_xprt.c @@ -386,7 +386,7 @@ EXPORT_SYMBOL_GPL(svc_xprt_copy_addrs); */ char *svc_print_addr(struct svc_rqst *rqstp, char *buf, size_t len) { - return __svc_print_addr(svc_addr(rqstp), buf, len); + return __svc_print_addr(svc_addr(rqstp), buf, len, true); } EXPORT_SYMBOL_GPL(svc_print_addr);
Introduce verbose parameter to utility routine in order to reduce output verbosity. This is a preliminary patch to add rpc_status entry in nfsd debug filesystem in order to dump pending RPC requests debugging information. Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> --- include/linux/sunrpc/svc_xprt.h | 12 ++++++------ net/sunrpc/svc_xprt.c | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-)