[lxc-devel] [PATCH] Add get_interfaces to the API and start not to filter out loopback interface

Stéphane Graber stgraber at ubuntu.com
Fri Sep 13 22:32:32 UTC 2013


On Fri, Sep 13, 2013 at 06:21:20PM -0400, S.Çağlar Onur wrote:
> Signed-off-by: S.Çağlar Onur <caglar at 10ur.org>

The loopback filtering was there so that all the software using get_ips
to wait for the container to be reachable would work.
Your change will essentially force everyone to do two calls, one for all
interfaces and one just for lo, then substract the second set from the
first, that seems suboptimal.

I guess your plan is that most API users will iterate through
get_interfaces then call get_ips for the interfaces they want, though
that's just making things harder for little benefit.


So I see two ways not to regress in that regard:
 1) Make scope filtering work with IPv4, so that get_ips() user can pass
    scope=0 and only get global IPv4 and IPv6 addresses.
 2) Add a no_loopback argument to get_ips.

As it's:
Nacked-by: Stéphane Graber <stgraber at ubuntu.com>

> ---
>  src/lxc/lxccontainer.c | 119 +++++++++++++++++++++++++++++++++++++++----------
>  src/lxc/lxccontainer.h |   1 +
>  2 files changed, 97 insertions(+), 23 deletions(-)
> 
> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> index 79237df..14b6942 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 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 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,93 @@ 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;
> +}
> +
> +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) {
> +        if (tempIfAddr->ifa_addr == NULL)
> +            continue;
> +
> +		if(tempIfAddr->ifa_addr->sa_family != AF_PACKET) {
> +            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 */
> +	if (count) {
> +		count++;
> +		temp = realloc(interfaces, count * sizeof(*interfaces));
> +		if (!temp) {
> +			int i;
> +			for (i = 0; i < count-1; i++)
> +				free(interfaces[i]);
> +			free(interfaces);
> +			return NULL;
> +		}
> +		interfaces = temp;
> +		interfaces[count - 1] = NULL;
> +	}
> +
> +	return interfaces;
> +}
> +
> +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)) {
> @@ -1241,8 +1321,6 @@ char** lxcapi_get_ips(struct lxc_container *c, char* interface, char* family, in
>  
>  		if (interface && strcmp(interface, tempIfAddr->ifa_name))
>  			continue;
> -		else if (!interface && strcmp("lo", tempIfAddr->ifa_name) == 0)
> -			continue;
>  
>  		address = (char *)inet_ntop(tempIfAddr->ifa_addr->sa_family,
>  					   tempAddrPtr,
> @@ -1265,13 +1343,7 @@ 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) {
> @@ -2576,6 +2648,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).
> -- 
> 1.8.1.2
> 
> 
> ------------------------------------------------------------------------------
> How ServiceNow helps IT people transform IT departments:
> 1. Consolidate legacy IT systems to a single system of record for IT
> 2. Standardize and globalize service processes across IT
> 3. Implement zero-touch automation to replace manual, redundant tasks
> http://pubads.g.doubleclick.net/gampad/clk?id=51271111&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
-------------- 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/20130913/17365ae5/attachment.pgp>


More information about the lxc-devel mailing list