2018-06-28 14:34:24

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH v2] Handle GARBAGE_ARGS response in rpcbind query

We have a report that some commercial NFS file servers still do not
support rpcbind v4 correctly. They return GARBAGE_ARGS instead of
PROG_MISMATCH or PROG_UNAVAIL, so our rpcbind client now errors out
immediately instead of trying a lower rpcbind version.

To address this, convert the "if () else if () else if ()" to a
switch statement to make it straightforward to add new status codes
to the error processing logic. Then, add a case for
RPC_CANTDECODEARGS (the API status code used when the remote
responds with GARBAGE_ARGS).

Reported-by: Yuan-Yao Sung <[email protected]>
Fixes: 5e7b57bc20bd ("rpcinfo: change order of version to be ... ")
Signed-off-by: Chuck Lever <[email protected]>
Tested-by: Yuan-Yao Sung <[email protected]>
---
src/rpcb_clnt.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/rpcb_clnt.c b/src/rpcb_clnt.c
index 4b44364..d6fefd0 100644
--- a/src/rpcb_clnt.c
+++ b/src/rpcb_clnt.c
@@ -846,6 +846,7 @@ __rpcb_findaddr_timed(program, version, nconf, host, clpp, tp)
struct netbuf *address = NULL;
rpcvers_t start_vers = RPCBVERS4;
struct netbuf servaddr;
+ struct rpc_err rpcerr;

/* parameter checking */
if (nconf == NULL) {
@@ -902,7 +903,8 @@ __rpcb_findaddr_timed(program, version, nconf, host, clpp, tp)
clnt_st = CLNT_CALL(client, (rpcproc_t)RPCBPROC_GETADDR,
(xdrproc_t) xdr_rpcb, (char *)(void *)&parms,
(xdrproc_t) xdr_wrapstring, (char *)(void *) &ua, *tp);
- if (clnt_st == RPC_SUCCESS) {
+ switch (clnt_st) {
+ case RPC_SUCCESS:
if ((ua == NULL) || (ua[0] == 0)) {
/* address unknown */
rpc_createerr.cf_stat = RPC_PROGNOTREGISTERED;
@@ -924,12 +926,15 @@ __rpcb_findaddr_timed(program, version, nconf, host, clpp, tp)
(char *)(void *)&servaddr);
__rpc_fixup_addr(address, &servaddr);
goto done;
- } else if (clnt_st == RPC_PROGVERSMISMATCH) {
- struct rpc_err rpcerr;
+ case RPC_PROGVERSMISMATCH:
clnt_geterr(client, &rpcerr);
if (rpcerr.re_vers.low > RPCBVERS4)
goto error; /* a new version, can't handle */
- } else if (clnt_st != RPC_PROGUNAVAIL) {
+ /* Try the next lower version */
+ case RPC_PROGUNAVAIL:
+ case RPC_CANTDECODEARGS:
+ break;
+ default:
/* Cant handle this error */
rpc_createerr.cf_stat = clnt_st;
clnt_geterr(client, &rpc_createerr.cf_error);



2018-07-10 18:40:53

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH v2] Handle GARBAGE_ARGS response in rpcbind query



On 06/28/2018 10:34 AM, Chuck Lever wrote:
> We have a report that some commercial NFS file servers still do not
> support rpcbind v4 correctly. They return GARBAGE_ARGS instead of
> PROG_MISMATCH or PROG_UNAVAIL, so our rpcbind client now errors out
> immediately instead of trying a lower rpcbind version.
>
> To address this, convert the "if () else if () else if ()" to a
> switch statement to make it straightforward to add new status codes
> to the error processing logic. Then, add a case for
> RPC_CANTDECODEARGS (the API status code used when the remote
> responds with GARBAGE_ARGS).
>
> Reported-by: Yuan-Yao Sung <[email protected]>
> Fixes: 5e7b57bc20bd ("rpcinfo: change order of version to be ... ")
> Signed-off-by: Chuck Lever <[email protected]>
> Tested-by: Yuan-Yao Sung <[email protected]>
Committed...

steved.

> ---
> src/rpcb_clnt.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/src/rpcb_clnt.c b/src/rpcb_clnt.c
> index 4b44364..d6fefd0 100644
> --- a/src/rpcb_clnt.c
> +++ b/src/rpcb_clnt.c
> @@ -846,6 +846,7 @@ __rpcb_findaddr_timed(program, version, nconf, host, clpp, tp)
> struct netbuf *address = NULL;
> rpcvers_t start_vers = RPCBVERS4;
> struct netbuf servaddr;
> + struct rpc_err rpcerr;
>
> /* parameter checking */
> if (nconf == NULL) {
> @@ -902,7 +903,8 @@ __rpcb_findaddr_timed(program, version, nconf, host, clpp, tp)
> clnt_st = CLNT_CALL(client, (rpcproc_t)RPCBPROC_GETADDR,
> (xdrproc_t) xdr_rpcb, (char *)(void *)&parms,
> (xdrproc_t) xdr_wrapstring, (char *)(void *) &ua, *tp);
> - if (clnt_st == RPC_SUCCESS) {
> + switch (clnt_st) {
> + case RPC_SUCCESS:
> if ((ua == NULL) || (ua[0] == 0)) {
> /* address unknown */
> rpc_createerr.cf_stat = RPC_PROGNOTREGISTERED;
> @@ -924,12 +926,15 @@ __rpcb_findaddr_timed(program, version, nconf, host, clpp, tp)
> (char *)(void *)&servaddr);
> __rpc_fixup_addr(address, &servaddr);
> goto done;
> - } else if (clnt_st == RPC_PROGVERSMISMATCH) {
> - struct rpc_err rpcerr;
> + case RPC_PROGVERSMISMATCH:
> clnt_geterr(client, &rpcerr);
> if (rpcerr.re_vers.low > RPCBVERS4)
> goto error; /* a new version, can't handle */
> - } else if (clnt_st != RPC_PROGUNAVAIL) {
> + /* Try the next lower version */
> + case RPC_PROGUNAVAIL:
> + case RPC_CANTDECODEARGS:
> + break;
> + default:
> /* Cant handle this error */
> rpc_createerr.cf_stat = clnt_st;
> clnt_geterr(client, &rpc_createerr.cf_error);
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>