On Aug 21, 2012, at 5:33 PM, J. Bruce Fields wrote:
> On Tue, Aug 21, 2012 at 05:29:16PM -0400, Chuck Lever wrote:
>>
>> On Aug 21, 2012, at 5:24 PM, J. Bruce Fields wrote:
>>
>>> On Tue, Aug 21, 2012 at 05:02:14PM -0400, Chuck Lever wrote:
>>>>
>>>> On Aug 21, 2012, at 4:57 PM, J. Bruce Fields wrote:
>>>>
>>>>> From: "J. Bruce Fields" <bfields@xxxxxxxxxx>
>>>>>
>>>>> How would this happen?
>>>>
>>>> If an unsupported address family is used in the rqstp.
>>>
>>> Right, but that's impossible, isn't it?
>>
>> The point is to catch this case when someone adds support for new address families. It won't happen in current code.
>>
>>>>> In any case, it appears this would be returned all the way up to the
>>>>> caller of svc_recv(), and it's obvious that none of them are equipped to
>>>>> handle it, and not clear what they would want to do with it anyway.
>>>>> Let's just drop this and return -EAGAIN.
>>>>
>>>> EAGAIN is incorrect; the correct error code is EAFNOSUPPORT. If callers are not prepared for this error return, perhaps BUG_ON() would be more appropriate here.
>>>
>>> Yeah. Actually on a quick check this is the only caller that even
>>> checks for this case. So probably the check should go in svc_addr_len.
>>> Maybe we should be nice and make it a warning.
>>
>> I normally find BUG pretty harsh, but in this case it's true a software error, and as you say, should be "impossible": thus it should be a BUG. The code should stop before the zero length is used.
>
> Eh, OK, I guess I can live with that.
>
> commit 751f877b10f8ce0d12b40d2a102f3b42b26dc08d
> Author: J. Bruce Fields <bfields@xxxxxxxxxx>
> Date: Tue Aug 21 17:22:11 2012 -0400
>
> svcrpc: don't bother checking bad svc_addr_len result
>
> None of the callers should see an unsupported address family (only one
> of them even bothers to check for that case), so just check for the
> buggy case in svc_addr_len and don't bother elsewhere.
>
> Signed-off-by: J. Bruce Fields <bfields@xxxxxxxxxx>
Acked-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
>
> diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
> index b3f64b1..4f3a6ab 100644
> --- a/include/linux/sunrpc/svc_xprt.h
> +++ b/include/linux/sunrpc/svc_xprt.h
> @@ -166,8 +166,7 @@ static inline size_t svc_addr_len(const struct sockaddr *sa)
> case AF_INET6:
> return sizeof(struct sockaddr_in6);
> }
> -
> - return 0;
> + BUG();
> }
>
> static inline unsigned short svc_xprt_local_port(const struct svc_xprt *xprt)
> diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
> index 998aa8c..13b005c 100644
> --- a/net/sunrpc/svcsock.c
> +++ b/net/sunrpc/svcsock.c
> @@ -601,8 +601,6 @@ static int svc_udp_recvfrom(struct svc_rqst *rqstp)
> return -EAGAIN;
> }
> len = svc_addr_len(svc_addr(rqstp));
> - if (len == 0)
> - return -EAFNOSUPPORT;
> rqstp->rq_addrlen = len;
> if (skb->tstamp.tv64 == 0) {
> skb->tstamp = ktime_get_real();
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
