diff mbox series

SUNRPC: use request size to initialize bio_vec in svc_udp_sendto()

Message ID 20240117210628.1534046-1-l.stach@pengutronix.de (mailing list archive)
State Not Applicable
Delegated to: Netdev Maintainers
Headers show
Series SUNRPC: use request size to initialize bio_vec in svc_udp_sendto() | expand

Checks

Context Check Description
netdev/series_format warning Single patches do not need cover letters; Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1092 this patch: 1092
netdev/cc_maintainers success CCed 0 of 0 maintainers
netdev/build_clang success Errors and warnings before: 1107 this patch: 1107
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 1107 this patch: 1107
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 14 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Lucas Stach Jan. 17, 2024, 9:06 p.m. UTC
Use the proper size when setting up the bio_vec, as otherwise only
zero-length UDP packets will be sent.

Fixes: baabf59c2414 ("SUNRPC: Convert svc_udp_sendto() to use the per-socket bio_vec array")
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 net/sunrpc/svcsock.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Chuck Lever Jan. 17, 2024, 10:03 p.m. UTC | #1
On Wed, Jan 17, 2024 at 10:06:28PM +0100, Lucas Stach wrote:
> Use the proper size when setting up the bio_vec, as otherwise only
> zero-length UDP packets will be sent.
> 
> Fixes: baabf59c2414 ("SUNRPC: Convert svc_udp_sendto() to use the per-socket bio_vec array")
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  net/sunrpc/svcsock.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
> index 998687421fa6..e0ce4276274b 100644
> --- a/net/sunrpc/svcsock.c
> +++ b/net/sunrpc/svcsock.c
> @@ -717,12 +717,12 @@ static int svc_udp_sendto(struct svc_rqst *rqstp)
>  				ARRAY_SIZE(rqstp->rq_bvec), xdr);
>  
>  	iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, rqstp->rq_bvec,
> -		      count, 0);
> +		      count, rqstp->rq_res.len);
>  	err = sock_sendmsg(svsk->sk_sock, &msg);
>  	if (err == -ECONNREFUSED) {
>  		/* ICMP error on earlier request. */
>  		iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, rqstp->rq_bvec,
> -			      count, 0);
> +			      count, rqstp->rq_res.len);
>  		err = sock_sendmsg(svsk->sk_sock, &msg);
>  	}
>  
> -- 
> 2.43.0
> 

I can't fathom why I would have chosen zero for the @count argument.

We currently have zero test coverage for UDP. I'll look into that.

I've applied this to the nfsd-fixes branch. It should appear in
v6.8-rc if I can get it tested.
Lucas Stach Jan. 18, 2024, 8:42 a.m. UTC | #2
Am Mittwoch, dem 17.01.2024 um 17:03 -0500 schrieb Chuck Lever:
> On Wed, Jan 17, 2024 at 10:06:28PM +0100, Lucas Stach wrote:
> > Use the proper size when setting up the bio_vec, as otherwise only
> > zero-length UDP packets will be sent.
> > 
> > Fixes: baabf59c2414 ("SUNRPC: Convert svc_udp_sendto() to use the per-socket bio_vec array")
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > ---
> >  net/sunrpc/svcsock.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
> > index 998687421fa6..e0ce4276274b 100644
> > --- a/net/sunrpc/svcsock.c
> > +++ b/net/sunrpc/svcsock.c
> > @@ -717,12 +717,12 @@ static int svc_udp_sendto(struct svc_rqst *rqstp)
> >  				ARRAY_SIZE(rqstp->rq_bvec), xdr);
> >  
> >  	iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, rqstp->rq_bvec,
> > -		      count, 0);
> > +		      count, rqstp->rq_res.len);
> >  	err = sock_sendmsg(svsk->sk_sock, &msg);
> >  	if (err == -ECONNREFUSED) {
> >  		/* ICMP error on earlier request. */
> >  		iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, rqstp->rq_bvec,
> > -			      count, 0);
> > +			      count, rqstp->rq_res.len);
> >  		err = sock_sendmsg(svsk->sk_sock, &msg);
> >  	}
> >  
> > -- 
> > 2.43.0
> > 
> 
> I can't fathom why I would have chosen zero for the @count argument.
> 
> We currently have zero test coverage for UDP. I'll look into that.
> 
> I've applied this to the nfsd-fixes branch. It should appear in
> v6.8-rc if I can get it tested.

Thanks. For what it is worth, this fix didn't come out of pure code
inspection, but fixes a real world setup for me. While I can't claim
that I have any kind of comprehensive testing, this fix has at least
shown to fix the issue introduced in the referenced commit in my setup.

Regards,
Lucas
diff mbox series

Patch

diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index 998687421fa6..e0ce4276274b 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -717,12 +717,12 @@  static int svc_udp_sendto(struct svc_rqst *rqstp)
 				ARRAY_SIZE(rqstp->rq_bvec), xdr);
 
 	iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, rqstp->rq_bvec,
-		      count, 0);
+		      count, rqstp->rq_res.len);
 	err = sock_sendmsg(svsk->sk_sock, &msg);
 	if (err == -ECONNREFUSED) {
 		/* ICMP error on earlier request. */
 		iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, rqstp->rq_bvec,
-			      count, 0);
+			      count, rqstp->rq_res.len);
 		err = sock_sendmsg(svsk->sk_sock, &msg);
 	}