1. Patchew
  2. Libvirt
  3. View series

[PATCH] rpc: 'getaddrinfo' function support both IPv4 and IPv6

Zhimin Feng posted 1 patch 4 years, 1 month ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20200302082651.1932-1-fengzhimin1@huawei.com
src/rpc/virnetsocket.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] rpc: 'getaddrinfo' function support both IPv4 and IPv6
Posted by Zhimin Feng 4 years, 1 month ago 
If only IPv6 is configured and nscd is started, getaddrinfo
function reture value is -9. So hints.ai_flags should include
the AI_V4MAPPED flag.

The following is the partial implementation of getaddrinfo(glibc):

    if (req->ai_family == AF_UNSPEC || req->ai_family == AF_INET)
        at->family = AF_INET;
    else if (req->ai_family == AF_INET6 && (req->ai_flags & AI_V4MAPPED))
    {
        at->addr[3] = at->addr[0];
	at->addr[2] = htonl (0xffff);
	at->addr[1] = 0;
	at->addr[0] = 0;
	at->family = AF_INET6;
    }
    else
    {
         result = -EAI_ADDRFAMILY;
         goto free_and_return;
    }

Signed-off-by: Zhimin Feng <fengzhimin1@huawei.com>
---
 src/rpc/virnetsocket.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index a217404fa6..c547acefc0 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -575,7 +575,7 @@ int virNetSocketNewConnectTCP(const char *nodename,
 
     memset(&hints, 0, sizeof(hints));
     hints.ai_family = family;
-    hints.ai_flags = AI_PASSIVE | AI_ADDRCONFIG;
+    hints.ai_flags = AI_PASSIVE | AI_ADDRCONFIG | AI_V4MAPPED;
     hints.ai_socktype = SOCK_STREAM;
 
     int e = getaddrinfo(nodename, service, &hints, &ai);
-- 
2.19.1
Re: [PATCH] rpc: 'getaddrinfo' function support both IPv4 and IPv6
Posted by Ján Tomko 4 years, 1 month ago 
The commit summary is too generic.
How about:
rpc: getaddrinfo: also accept IPv4-mapped IPv6 addresses

On a Monday in 2020, Zhimin Feng wrote:
>If only IPv6 is configured and nscd is started, getaddrinfo

Is ncsd required to reproduce this?

Looking at glibc source:
libvirt passes AI_ADDRCONFIG and if getifaddrs returs only one family,
it is placed into req->ai_family and the code below is hit.

>function reture value is -9. So hints.ai_flags should include
>the AI_V4MAPPED flag.
>
>The following is the partial implementation of getaddrinfo(glibc):
>
>    if (req->ai_family == AF_UNSPEC || req->ai_family == AF_INET)
>        at->family = AF_INET;
>    else if (req->ai_family == AF_INET6 && (req->ai_flags & AI_V4MAPPED))
>    {
>        at->addr[3] = at->addr[0];
>	at->addr[2] = htonl (0xffff);
>	at->addr[1] = 0;
>	at->addr[0] = 0;
>	at->family = AF_INET6;
>    }
>    else
>    {
>         result = -EAI_ADDRFAMILY;
>         goto free_and_return;
>    }

However small, I'd rather not include GPL code in our commit messages.
Proposed commit message:

If only IPv6 is configured on the host, getaddrinfo with AI_ADDRCONFIG
in hints would return EAI_ADDRFAMILY for nodenames that resolve to IPv4.

Also pass AI_V4MAPPED to accept IPv4-mapped addresses on IPv6-only
systems.

>
>Signed-off-by: Zhimin Feng <fengzhimin1@huawei.com>
>---
> src/rpc/virnetsocket.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
>index a217404fa6..c547acefc0 100644
>--- a/src/rpc/virnetsocket.c
>+++ b/src/rpc/virnetsocket.c
>@@ -575,7 +575,7 @@ int virNetSocketNewConnectTCP(const char *nodename,
>
>     memset(&hints, 0, sizeof(hints));
>     hints.ai_family = family;
>-    hints.ai_flags = AI_PASSIVE | AI_ADDRCONFIG;
>+    hints.ai_flags = AI_PASSIVE | AI_ADDRCONFIG | AI_V4MAPPED;
>     hints.ai_socktype = SOCK_STREAM;
>
>     int e = getaddrinfo(nodename, service, &hints, &ai);

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
Re: [PATCH] rpc: 'getaddrinfo' function support both IPv4 and IPv6
Posted by Daniel P. Berrangé 4 years, 1 month ago 
On Mon, Mar 02, 2020 at 05:54:42PM +0100, Ján Tomko wrote:
> The commit summary is too generic.
> How about:
> rpc: getaddrinfo: also accept IPv4-mapped IPv6 addresses
> 
> On a Monday in 2020, Zhimin Feng wrote:
> > If only IPv6 is configured and nscd is started, getaddrinfo
> 
> Is ncsd required to reproduce this?
> 
> Looking at glibc source:
> libvirt passes AI_ADDRCONFIG and if getifaddrs returs only one family,
> it is placed into req->ai_family and the code below is hit.
> 
> > function reture value is -9. So hints.ai_flags should include
> > the AI_V4MAPPED flag.
> > 
> > The following is the partial implementation of getaddrinfo(glibc):
> > 
> >    if (req->ai_family == AF_UNSPEC || req->ai_family == AF_INET)
> >        at->family = AF_INET;
> >    else if (req->ai_family == AF_INET6 && (req->ai_flags & AI_V4MAPPED))
> >    {
> >        at->addr[3] = at->addr[0];
> > 	at->addr[2] = htonl (0xffff);
> > 	at->addr[1] = 0;
> > 	at->addr[0] = 0;
> > 	at->family = AF_INET6;
> >    }
> >    else
> >    {
> >         result = -EAI_ADDRFAMILY;
> >         goto free_and_return;
> >    }
> 
> However small, I'd rather not include GPL code in our commit messages.
> Proposed commit message:
> 
> If only IPv6 is configured on the host, getaddrinfo with AI_ADDRCONFIG
> in hints would return EAI_ADDRFAMILY for nodenames that resolve to IPv4.
> 
> Also pass AI_V4MAPPED to accept IPv4-mapped addresses on IPv6-only
> systems.
> 
> > 
> > Signed-off-by: Zhimin Feng <fengzhimin1@huawei.com>
> > ---
> > src/rpc/virnetsocket.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
> > index a217404fa6..c547acefc0 100644
> > --- a/src/rpc/virnetsocket.c
> > +++ b/src/rpc/virnetsocket.c
> > @@ -575,7 +575,7 @@ int virNetSocketNewConnectTCP(const char *nodename,
> > 
> >     memset(&hints, 0, sizeof(hints));
> >     hints.ai_family = family;
> > -    hints.ai_flags = AI_PASSIVE | AI_ADDRCONFIG;
> > +    hints.ai_flags = AI_PASSIVE | AI_ADDRCONFIG | AI_V4MAPPED;
> >     hints.ai_socktype = SOCK_STREAM;
> > 
> >     int e = getaddrinfo(nodename, service, &hints, &ai);
> 
> Reviewed-by: Ján Tomko <jtomko@redhat.com>

I noticed that glibc man pages say if ai_flags is 0, then it defaults
to AI_ADDRCONFIG | AI_V4MAPPED.  Libvirt had to set ai_flags to get the
AI_PASSIVE flag, and so given that adding AI_V4MAPPED makes sense to
regain the default behaviour we're loosing.

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.