diff mbox

[nfs-utils,v2,01/12] mount: don't use IPPROTO_UDP for address resolution

Message ID 20170630132120.31578-2-stefanha@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Stefan Hajnoczi June 30, 2017, 1:21 p.m. UTC
Although getaddrinfo(3) with IPPROTO_UDP works fine for AF_INET and
AF_INET6, the AF_VSOCK address family does not support IPPROTO_UDP and
produces an error.

Drop IPPROTO_UDP and use the 0 default (TCP) which works for all address
families.  Modern NFS uses TCP anyway so it's strange to specify UDP.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Jeff Layton <jlayton@redhat.com>
---
 utils/mount/stropts.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Steve Dickson June 30, 2017, 2:34 p.m. UTC | #1
On 06/30/2017 09:21 AM, Stefan Hajnoczi wrote:
> Although getaddrinfo(3) with IPPROTO_UDP works fine for AF_INET and
> AF_INET6, the AF_VSOCK address family does not support IPPROTO_UDP and
> produces an error.
> 
> Drop IPPROTO_UDP and use the 0 default (TCP) which works for all address
> families.  Modern NFS uses TCP anyway so it's strange to specify UDP.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> Reviewed-by: Jeff Layton <jlayton@redhat.com>
> ---
>  utils/mount/stropts.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> index c2a739b..99656dd 100644
> --- a/utils/mount/stropts.c
> +++ b/utils/mount/stropts.c
> @@ -909,9 +909,7 @@ static int nfs_try_mount(struct nfsmount_info *mi)
>  	int result = 0;
>  
>  	if (mi->address == NULL) {
> -		struct addrinfo hint = {
> -			.ai_protocol	= (int)IPPROTO_UDP,
> -		};
> +		struct addrinfo hint = {};
Just curious as to why not simply pass a NULL hints parameter 
verses an empty hints structure?

steved.

>  		int error;
>  		struct addrinfo *address;
>  
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stefan Hajnoczi July 3, 2017, 8:55 a.m. UTC | #2
On Fri, Jun 30, 2017 at 10:34:54AM -0400, Steve Dickson wrote:
> 
> 
> On 06/30/2017 09:21 AM, Stefan Hajnoczi wrote:
> > Although getaddrinfo(3) with IPPROTO_UDP works fine for AF_INET and
> > AF_INET6, the AF_VSOCK address family does not support IPPROTO_UDP and
> > produces an error.
> > 
> > Drop IPPROTO_UDP and use the 0 default (TCP) which works for all address
> > families.  Modern NFS uses TCP anyway so it's strange to specify UDP.
> > 
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > Reviewed-by: Jeff Layton <jlayton@redhat.com>
> > ---
> >  utils/mount/stropts.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> > index c2a739b..99656dd 100644
> > --- a/utils/mount/stropts.c
> > +++ b/utils/mount/stropts.c
> > @@ -909,9 +909,7 @@ static int nfs_try_mount(struct nfsmount_info *mi)
> >  	int result = 0;
> >  
> >  	if (mi->address == NULL) {
> > -		struct addrinfo hint = {
> > -			.ai_protocol	= (int)IPPROTO_UDP,
> > -		};
> > +		struct addrinfo hint = {};
> Just curious as to why not simply pass a NULL hints parameter 
> verses an empty hints structure?

It's not clear from the surrounding unified diff context but the code
does set .ai_family later on:

  hint.ai_family = (int)mi->family;

Would you prefer it if I move that up into the variable definition?

  struct addrinfo hint = {
          .ai_family = (int)mi->family,
  };
Steve Dickson July 3, 2017, 4:35 p.m. UTC | #3
On 07/03/2017 04:55 AM, Stefan Hajnoczi wrote:
> On Fri, Jun 30, 2017 at 10:34:54AM -0400, Steve Dickson wrote:
>>
>>
>> On 06/30/2017 09:21 AM, Stefan Hajnoczi wrote:
>>> Although getaddrinfo(3) with IPPROTO_UDP works fine for AF_INET and
>>> AF_INET6, the AF_VSOCK address family does not support IPPROTO_UDP and
>>> produces an error.
>>>
>>> Drop IPPROTO_UDP and use the 0 default (TCP) which works for all address
>>> families.  Modern NFS uses TCP anyway so it's strange to specify UDP.
>>>
>>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>>> Reviewed-by: Jeff Layton <jlayton@redhat.com>
>>> ---
>>>  utils/mount/stropts.c | 4 +---
>>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>>
>>> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
>>> index c2a739b..99656dd 100644
>>> --- a/utils/mount/stropts.c
>>> +++ b/utils/mount/stropts.c
>>> @@ -909,9 +909,7 @@ static int nfs_try_mount(struct nfsmount_info *mi)
>>>  	int result = 0;
>>>  
>>>  	if (mi->address == NULL) {
>>> -		struct addrinfo hint = {
>>> -			.ai_protocol	= (int)IPPROTO_UDP,
>>> -		};
>>> +		struct addrinfo hint = {};
>> Just curious as to why not simply pass a NULL hints parameter 
>> verses an empty hints structure?
> 
> It's not clear from the surrounding unified diff context but the code
> does set .ai_family later on:
> 
>   hint.ai_family = (int)mi->family;
Ah... I did miss this...

> 
> Would you prefer it if I move that up into the variable definition?
> 
>   struct addrinfo hint = {
>           .ai_family = (int)mi->family,
>   };
> 
It's good as is.

steved.

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index c2a739b..99656dd 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -909,9 +909,7 @@  static int nfs_try_mount(struct nfsmount_info *mi)
 	int result = 0;
 
 	if (mi->address == NULL) {
-		struct addrinfo hint = {
-			.ai_protocol	= (int)IPPROTO_UDP,
-		};
+		struct addrinfo hint = {};
 		int error;
 		struct addrinfo *address;