diff mbox series

[v7,12/20] SUNRPC: remove call_allocate() BUG_ON if p_arglen=0 to allow RPC with void arg

Message ID 20240624162741.68216-13-snitzer@kernel.org (mailing list archive)
State New
Headers show
Series nfs/nfsd: add support for localio | expand

Commit Message

Mike Snitzer June 24, 2024, 4:27 p.m. UTC
This is needed for the LOCALIO protocol's GETUUID RPC which takes a
void arg.  The LOCALIO protocol spec in rpcgen syntax is:

/* raw RFC 9562 UUID */
typedef u8 uuid_t<UUID_SIZE>;

program NFS_LOCALIO_PROGRAM {
    version LOCALIO_V1 {
        void
            NULL(void) = 0;

        uuid_t
            GETUUID(void) = 1;
    } = 1;
} = 400122;

Signed-off-by: Mike Snitzer <snitzer@kernel.org>
---
 net/sunrpc/clnt.c | 1 -
 1 file changed, 1 deletion(-)

Comments

NeilBrown June 25, 2024, 11:19 p.m. UTC | #1
On Tue, 25 Jun 2024, Mike Snitzer wrote:
> This is needed for the LOCALIO protocol's GETUUID RPC which takes a
> void arg.  The LOCALIO protocol spec in rpcgen syntax is:
> 
> /* raw RFC 9562 UUID */
> typedef u8 uuid_t<UUID_SIZE>;
> 
> program NFS_LOCALIO_PROGRAM {
>     version LOCALIO_V1 {
>         void
>             NULL(void) = 0;
> 
>         uuid_t
>             GETUUID(void) = 1;
>     } = 1;
> } = 400122;
> 
> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
> ---
>  net/sunrpc/clnt.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> index cfd1b1bf7e35..2d7f96103f08 100644
> --- a/net/sunrpc/clnt.c
> +++ b/net/sunrpc/clnt.c
> @@ -1894,7 +1894,6 @@ call_allocate(struct rpc_task *task)
>  		return;
>  
>  	if (proc->p_proc != 0) {
> -		BUG_ON(proc->p_arglen == 0);
>  		if (proc->p_decode != NULL)
>  			BUG_ON(proc->p_replen == 0);
>  	}

I would be in favour get rid of all 5 lines.
I wonder if p_decode is ever NULL.  It would cause
rpcauth_unwrap_resp_decode() problems.

NeilBrown


> -- 
> 2.44.0
> 
>
Mike Snitzer June 26, 2024, 4:53 p.m. UTC | #2
On Wed, Jun 26, 2024 at 09:19:43AM +1000, NeilBrown wrote:
> On Tue, 25 Jun 2024, Mike Snitzer wrote:
> > This is needed for the LOCALIO protocol's GETUUID RPC which takes a
> > void arg.  The LOCALIO protocol spec in rpcgen syntax is:
> > 
> > /* raw RFC 9562 UUID */
> > typedef u8 uuid_t<UUID_SIZE>;
> > 
> > program NFS_LOCALIO_PROGRAM {
> >     version LOCALIO_V1 {
> >         void
> >             NULL(void) = 0;
> > 
> >         uuid_t
> >             GETUUID(void) = 1;
> >     } = 1;
> > } = 400122;
> > 
> > Signed-off-by: Mike Snitzer <snitzer@kernel.org>
> > ---
> >  net/sunrpc/clnt.c | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> > index cfd1b1bf7e35..2d7f96103f08 100644
> > --- a/net/sunrpc/clnt.c
> > +++ b/net/sunrpc/clnt.c
> > @@ -1894,7 +1894,6 @@ call_allocate(struct rpc_task *task)
> >  		return;
> >  
> >  	if (proc->p_proc != 0) {
> > -		BUG_ON(proc->p_arglen == 0);
> >  		if (proc->p_decode != NULL)
> >  			BUG_ON(proc->p_replen == 0);
> >  	}
> 
> I would be in favour get rid of all 5 lines.
> I wonder if p_decode is ever NULL.  It would cause
> rpcauth_unwrap_resp_decode() problems.

OK, but that broader cleanup can happen with an incremental follow-up
patch.
diff mbox series

Patch

diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index cfd1b1bf7e35..2d7f96103f08 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -1894,7 +1894,6 @@  call_allocate(struct rpc_task *task)
 		return;
 
 	if (proc->p_proc != 0) {
-		BUG_ON(proc->p_arglen == 0);
 		if (proc->p_decode != NULL)
 			BUG_ON(proc->p_replen == 0);
 	}