[lxc-devel] [PATCH] Add get_interfaces to the API - v2

Stéphane Graber stgraber at ubuntu.com
Mon Sep 16 19:04:45 UTC 2013


On Mon, Sep 16, 2013 at 02:54:44PM -0400, S.Çağlar Onur wrote:
> On Mon, Sep 16, 2013 at 2:39 PM, Stéphane Graber <stgraber at ubuntu.com>wrote:
> 
> > On Mon, Sep 16, 2013 at 02:26:47PM -0400, S.Çağlar Onur wrote:
> > > get_ips accepts an interface name as a parameter but there was no
> > > way to get the interfaces names from the container. This patch
> > > introduces a new get_interfaces call to the API so that users
> > > can obtain the name of the interfaces.
> > >
> > > Signed-off-by: S.Çağlar Onur <caglar at 10ur.org>
> >
> > I think I'm fine with this version:
> >
> > One quick comment though, wouldn't we expect get_interfaces() to return
> > all the interfaces by default, including loopback, including those
> > without addresses and those that aren't standard packet based IP
> > interfaces?
> >
> > I still think we don't want to list loopback addresses in get_ips(), at
> > least by default, but I think I'd expect get_interfaces to return me all
> > the existing interfaces without filtering. Does that make sense?
> 
> 
> Guess so. I've no objection including loopback in get_interfaces. I
> excluded it thinking that there is no point returning it as get_ips will
> ignore it. What about something like below as a replacement to v2 (this
> version starts to use if_nameindex instead of getifaddrs)?

I'd rather not use if_nameindex unless someone re-implements a bionic
version of it under lxc/includes/ as I just confirmed it doesn't exist
on bionic and so commiting that change would break our Android builds (I
know bionic is a real pain).

It took me a couple of days to find an slightly adapt a reasonable
implementation of getifaddrs for bionic, so I'd like not to have to go
through that again (but don't mind it if someone else contributes a
bionic compatible implementation).

So just to clarify what I think should happen in a perfect worls :)
 1) get_interfaces() would always return all the network interfaces
    visible by the guest, so I'd expect those to match the entries from
    ifconfig -a (or ip link show)
 2) get_ips() should default to only listing global IP addresses, so by
    default ignore the loopback device and only return scope-global for
    IPv6 (its current behaviour)
 3) If specifically passed interface="lo", then I'd expect get_ips to
    return the loopback IP addresses (not currently the case, but
    probably should be the case)

Does that make sense?

> 
> +static char** lxcapi_get_interfaces(struct lxc_container *c)
> +{
> +       int count = 0;
> +       struct if_nameindex *if_ni = NULL, *i;
> +       char **interfaces = NULL, **temp;
> +       int old_netns = -1, new_netns = -1;
> +
> +       if (!enter_to_ns(c, &old_netns, &new_netns))
> +               goto out;
> +
> +       if_ni = if_nameindex();
> +       if (if_ni == NULL) {
> +               SYSERROR("failed to get interfaces list");
> +               goto out;
> +       }
> +
> +    for (i = if_ni; ! (i->if_index == 0 && i->if_name == NULL); i++) {
> +        count += 1;
> +        temp = realloc(interfaces, count * sizeof(*interfaces));
> +        if (!temp) {
> +            count -= 1;
> +            goto out;
> +        }
> +        interfaces = temp;
> +        interfaces[count - 1] = strdup(i->if_name);
> +    }
> +
> +out:
> +       if_freenameindex(if_ni);
> +
> +       exit_from_ns(c, &old_netns, &new_netns);
> +
> +       /* Append NULL to the array */
> +       interfaces = (char **)lxc_append_null_to_array((void **)interfaces,
> count);
> +
> +       return interfaces;
> +}
> 
> And if we need get_ips to return ips then I guess we can always add a flag
> to get_ips as you suggested before?
> 
>  > ---
> > >  src/lxc/lxccontainer.c | 117
> > ++++++++++++++++++++++++++++++++++---------------
> > >  src/lxc/lxccontainer.h |   1 +
> > >  src/lxc/utils.c        |  22 +++++++++-
> > >  src/lxc/utils.h        |   1 +
> > >  4 files changed, 105 insertions(+), 36 deletions(-)
> > >
> > > diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> > > index 79237df..6aa59fc 100644
> > > --- a/src/lxc/lxccontainer.c
> > > +++ b/src/lxc/lxccontainer.c
> > > @@ -1179,23 +1179,26 @@ static bool lxcapi_clear_config_item(struct
> > lxc_container *c, const char *key)
> > >       return ret == 0;
> > >  }
> > >
> > > -char** lxcapi_get_ips(struct lxc_container *c, char* interface, char*
> > family, int scope)
> > > -{
> > > -     int count = 0;
> > > -     struct ifaddrs *interfaceArray = NULL, *tempIfAddr = NULL;
> > > -     char addressOutputBuffer[INET6_ADDRSTRLEN];
> > > -     void *tempAddrPtr = NULL;
> > > -     char **addresses = NULL, **temp;
> > > -     char *address = NULL;
> > > +static inline void exit_from_ns(struct lxc_container *c, int
> > *old_netns, int *new_netns) {
> > > +     /* Switch back to original netns */
> > > +     if (*old_netns >= 0 && setns(*old_netns, CLONE_NEWNET))
> > > +             SYSERROR("failed to setns");
> > > +     if (*new_netns >= 0)
> > > +             close(*new_netns);
> > > +     if (*old_netns >= 0)
> > > +             close(*old_netns);
> > > +}
> > > +
> > > +static inline bool enter_to_ns(struct lxc_container *c, int *old_netns,
> > int *new_netns) {
> > > +     int ret = 0;
> > >       char new_netns_path[MAXPATHLEN];
> > > -     int old_netns = -1, new_netns = -1, ret = 0;
> > >
> > >       if (!c->is_running(c))
> > >               goto out;
> > >
> > >       /* Save reference to old netns */
> > > -     old_netns = open("/proc/self/ns/net", O_RDONLY);
> > > -     if (old_netns < 0) {
> > > +     *old_netns = open("/proc/self/ns/net", O_RDONLY);
> > > +     if (*old_netns < 0) {
> > >               SYSERROR("failed to open /proc/self/ns/net");
> > >               goto out;
> > >       }
> > > @@ -1205,16 +1208,78 @@ char** lxcapi_get_ips(struct lxc_container *c,
> > char* interface, char* family, in
> > >       if (ret < 0 || ret >= MAXPATHLEN)
> > >               goto out;
> > >
> > > -     new_netns = open(new_netns_path, O_RDONLY);
> > > -     if (new_netns < 0) {
> > > +     *new_netns = open(new_netns_path, O_RDONLY);
> > > +     if (*new_netns < 0) {
> > >               SYSERROR("failed to open %s", new_netns_path);
> > >               goto out;
> > >       }
> > >
> > > -     if (setns(new_netns, CLONE_NEWNET)) {
> > > +     if (setns(*new_netns, CLONE_NEWNET)) {
> > >               SYSERROR("failed to setns");
> > >               goto out;
> > >       }
> > > +     return true;
> > > +out:
> > > +     exit_from_ns(c, old_netns, new_netns);
> > > +     return false;
> > > +}
> > > +
> > > +static char** lxcapi_get_interfaces(struct lxc_container *c)
> > > +{
> > > +     int count = 0;
> > > +     struct ifaddrs *interfaceArray = NULL, *tempIfAddr = NULL;
> > > +     char **interfaces = NULL, **temp;
> > > +     int old_netns = -1, new_netns = -1;
> > > +
> > > +     if (!enter_to_ns(c, &old_netns, &new_netns))
> > > +             goto out;
> > > +
> > > +     /* Grab the list of interfaces */
> > > +     if (getifaddrs(&interfaceArray)) {
> > > +             SYSERROR("failed to get interfaces list");
> > > +             goto out;
> > > +     }
> > > +
> > > +     /* Iterate through the interfaces */
> > > +     for (tempIfAddr = interfaceArray; tempIfAddr != NULL; tempIfAddr =
> > tempIfAddr->ifa_next) {
> > > +             /* filter out loopback interface */
> > > +             if(tempIfAddr->ifa_addr == NULL ||
> > tempIfAddr->ifa_addr->sa_family != AF_PACKET || strcmp("lo",
> > tempIfAddr->ifa_name) == 0)
> > > +                     continue;
> > > +
> > > +        count += 1;
> > > +        temp = realloc(interfaces, count * sizeof(*interfaces));
> > > +        if (!temp) {
> > > +            count -= 1;
> > > +            goto out;
> > > +        }
> > > +        interfaces = temp;
> > > +        interfaces[count - 1] = strdup(tempIfAddr->ifa_name);
> > > +    }
> > > +
> > > +out:
> > > +     if(interfaceArray)
> > > +             freeifaddrs(interfaceArray);
> > > +
> > > +     exit_from_ns(c, &old_netns, &new_netns);
> > > +
> > > +     /* Append NULL to the array */
> > > +     interfaces = (char **)lxc_append_null_to_array((void
> > **)interfaces, count);
> > > +
> > > +     return interfaces;
> > > +}
> > > +
> > > +static char** lxcapi_get_ips(struct lxc_container *c, char* interface,
> > char* family, int scope)
> > > +{
> > > +     int count = 0;
> > > +     struct ifaddrs *interfaceArray = NULL, *tempIfAddr = NULL;
> > > +     char addressOutputBuffer[INET6_ADDRSTRLEN];
> > > +     void *tempAddrPtr = NULL;
> > > +     char **addresses = NULL, **temp;
> > > +     char *address = NULL;
> > > +     int old_netns = -1, new_netns = -1;
> > > +
> > > +     if (!enter_to_ns(c, &old_netns, &new_netns))
> > > +             goto out;
> > >
> > >       /* Grab the list of interfaces */
> > >       if (getifaddrs(&interfaceArray)) {
> > > @@ -1243,7 +1308,6 @@ char** lxcapi_get_ips(struct lxc_container *c,
> > char* interface, char* family, in
> > >                       continue;
> > >               else if (!interface && strcmp("lo", tempIfAddr->ifa_name)
> > == 0)
> > >                       continue;
> > > -
> > >               address = (char
> > *)inet_ntop(tempIfAddr->ifa_addr->sa_family,
> > >                                          tempAddrPtr,
> > >                                          addressOutputBuffer,
> > > @@ -1265,28 +1329,10 @@ out:
> > >       if(interfaceArray)
> > >               freeifaddrs(interfaceArray);
> > >
> > > -     /* Switch back to original netns */
> > > -     if (old_netns >= 0 && setns(old_netns, CLONE_NEWNET))
> > > -             SYSERROR("failed to setns");
> > > -     if (new_netns >= 0)
> > > -             close(new_netns);
> > > -     if (old_netns >= 0)
> > > -             close(old_netns);
> > > +     exit_from_ns(c, &old_netns, &new_netns);
> > >
> > >       /* Append NULL to the array */
> > > -     if (count) {
> > > -             count++;
> > > -             temp = realloc(addresses, count * sizeof(*addresses));
> > > -             if (!temp) {
> > > -                     int i;
> > > -                     for (i = 0; i < count-1; i++)
> > > -                             free(addresses[i]);
> > > -                     free(addresses);
> > > -                     return NULL;
> > > -             }
> > > -             addresses = temp;
> > > -             addresses[count - 1] = NULL;
> > > -     }
> > > +     addresses = (char **)lxc_append_null_to_array((void **)addresses,
> > count);
> > >
> > >       return addresses;
> > >  }
> > > @@ -2576,6 +2622,7 @@ struct lxc_container *lxc_container_new(const char
> > *name, const char *configpath
> > >       c->get_config_path = lxcapi_get_config_path;
> > >       c->set_config_path = lxcapi_set_config_path;
> > >       c->clone = lxcapi_clone;
> > > +     c->get_interfaces = lxcapi_get_interfaces;
> > >       c->get_ips = lxcapi_get_ips;
> > >       c->attach = lxcapi_attach;
> > >       c->attach_run_wait = lxcapi_attach_run_wait;
> > > diff --git a/src/lxc/lxccontainer.h b/src/lxc/lxccontainer.h
> > > index f9ae43b..f17d975 100644
> > > --- a/src/lxc/lxccontainer.h
> > > +++ b/src/lxc/lxccontainer.h
> > > @@ -90,6 +90,7 @@ struct lxc_container {
> > >        * the length which was our would be printed. */
> > >       int (*get_config_item)(struct lxc_container *c, const char *key,
> > char *retv, int inlen);
> > >       int (*get_keys)(struct lxc_container *c, const char *key, char
> > *retv, int inlen);
> > > +     char** (*get_interfaces)(struct lxc_container *c);
> > >       char** (*get_ips)(struct lxc_container *c, char* interface, char*
> > family, int scope);
> > >       /*
> > >        * get_cgroup_item returns the number of bytes read, or an error
> > (<0).
> > > diff --git a/src/lxc/utils.c b/src/lxc/utils.c
> > > index 78b234d..924cc19 100644
> > > --- a/src/lxc/utils.c
> > > +++ b/src/lxc/utils.c
> > > @@ -852,7 +852,7 @@ int lxc_write_to_file(const char *filename, const
> > void* buf, size_t count, bool
> > >               return -1;
> > >       ret = lxc_write_nointr(fd, buf, count);
> > >       if (ret < 0)
> > > -             goto out_error;
> > > +             goto out_error;
> > >       if ((size_t)ret != count)
> > >               goto out_error;
> > >       if (add_newline) {
> > > @@ -899,3 +899,23 @@ int lxc_read_from_file(const char *filename, void*
> > buf, size_t count)
> > >       errno = saved_errno;
> > >       return ret;
> > >  }
> > > +
> > > +void **lxc_append_null_to_array(void **array, size_t count)
> > > +{
> > > +     void **temp;
> > > +
> > > +     /* Append NULL to the array */
> > > +     if (count) {
> > > +             temp = realloc(array, (count + 1) * sizeof(*array));
> > > +             if (!temp) {
> > > +                     int i;
> > > +                     for (i = 0; i < count; i++)
> > > +                             free(array[i]);
> > > +                     free(array);
> > > +                     return NULL;
> > > +             }
> > > +             array = temp;
> > > +             array[count] = NULL;
> > > +     }
> > > +     return array;
> > > +}
> > > diff --git a/src/lxc/utils.h b/src/lxc/utils.h
> > > index ba7cfb3..55f98fa 100644
> > > --- a/src/lxc/utils.h
> > > +++ b/src/lxc/utils.h
> > > @@ -236,4 +236,5 @@ extern void lxc_free_array(void **array, lxc_free_fn
> > element_free_fn);
> > >  extern size_t lxc_array_len(void **array);
> > >  extern void **lxc_dup_array(void **array, lxc_dup_fn element_dup_fn,
> > lxc_free_fn element_free_fn);
> > >
> > > +extern void **lxc_append_null_to_array(void **array, size_t count);
> > >  #endif
> > > --
> > > 1.8.1.2
> > >
> > >
> > >
> > ------------------------------------------------------------------------------
> > > LIMITED TIME SALE - Full Year of Microsoft Training For Just $49.99!
> > > 1,500+ hours of tutorials including VisualStudio 2012, Windows 8,
> > SharePoint
> > > 2013, SQL 2012, MVC 4, more. BEST VALUE: New Multi-Library Power Pack
> > includes
> > > Mobile, Cloud, Java, and UX Design. Lowest price ever! Ends 9/20/13.
> > >
> > http://pubads.g.doubleclick.net/gampad/clk?id=58041151&iu=/4140/ostg.clktrk
> > > _______________________________________________
> > > Lxc-devel mailing list
> > > Lxc-devel at lists.sourceforge.net
> > > https://lists.sourceforge.net/lists/listinfo/lxc-devel
> >
> > --
> > Stéphane Graber
> > Ubuntu developer
> > http://www.ubuntu.com
> >
> 
> 
> 
> -- 
> S.Çağlar Onur <caglar at 10ur.org>

-- 
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20130916/56778d24/attachment.pgp>


More information about the lxc-devel mailing list