Merge lp:~kvalo/connman/ptp-default-gateway into lp:~indicator-network-developers/connman/trunk

[Kalle Valo]

Fix for default interface problem.

[David Barth (dbarth)]

Comments and needs info:
- the del_routes changes seem correct to me
- connman_inet_clear_gateway_interface looks good to me
- rntl: sounds ok, but i'm not sure how safe it is to skip those host-specific routes

My only worry is about swapping a NULL for a "0.0.0.0" string. You've done the changes locally, but are there other parts of the code (or of plugins) where gateway != NULL (because it's now "0.0.0.0") would be mis-interpreted? Have you grepped for that in the rest of the code?

review: Needs Information

[Kalle Valo (kvalo)]

Fortunately struct gateway_data is only used inside src/connections.c and I reviewed the cases in that file and they look to be ok.

[David Barth (dbarth)]

Ok +1

review: Approve

=== modified file 'include/inet.h'
--- include/inet.h	2010-02-11 04:33:05 +0000
+++ include/inet.h	2010-06-03 12:42:23 +0000
@@ -50,6 +50,7 @@
 int connman_inet_set_gateway_address(int index, const char *gateway);
 int connman_inet_clear_gateway_address(int index, const char *gateway);
 int connman_inet_set_gateway_interface(int index);
+int connman_inet_clear_gateway_interface(int index);
 
 #ifdef __cplusplus
 }

=== modified file 'src/connection.c'
--- src/connection.c	2010-05-06 05:32:30 +0000
+++ src/connection.c	2010-06-03 12:42:23 +0000
@@ -66,18 +66,21 @@
 
 static int del_routes(struct gateway_data *data)
 {
-	const char *address;
+	DBG("gateway %s", data->gateway);
 
 	if (data->vpn) {
 		if (data->vpn_phy_index >= 0)
 			connman_inet_del_host_route(data->vpn_phy_index,
 							data->gateway);
-		address = data->vpn_ip;
+		return connman_inet_clear_gateway_address(data->index,
+							data->vpn_ip);
+	} else if (g_strcmp0(data->gateway, "0.0.0.0") == 0) {
+		return connman_inet_clear_gateway_interface(data->index);
 	} else {
 		connman_inet_del_host_route(data->index, data->gateway);
-		address = data->gateway;
+		return connman_inet_clear_gateway_address(data->index,
+							data->gateway);
 	}
-	return connman_inet_clear_gateway_address(data->index, address);
 }
 
 static void find_element(struct connman_element *element, gpointer user_data)
@@ -104,6 +107,13 @@
 	if (data == NULL)
 		return NULL;
 
+	/*
+	 * If gateway is NULL, it's a point to point link and the default
+	 * gateway is 0.0.0.0, meaning the interface.
+	 */
+	if (gateway == NULL)
+		gateway = "0.0.0.0";
+
 	data->index = index;
 	data->gateway = g_strdup(gateway);
 	data->active = FALSE;
@@ -272,9 +282,6 @@
 
 	connman_element_set_enabled(element, TRUE);
 
-	if (gateway == NULL)
-		return 0;
-
 	active_gateway = find_active_gateway();
 	new_gateway = add_gateway(element->index, gateway);
 

=== modified file 'src/inet.c'
--- src/inet.c	2010-03-07 23:01:29 +0000
+++ src/inet.c	2010-06-03 12:42:23 +0000
@@ -899,3 +899,48 @@
 
 	return err;
 }
+
+int connman_inet_clear_gateway_interface(int index)
+{
+	struct ifreq ifr;
+	struct rtentry rt;
+	struct sockaddr_in addr;
+	int sk, err;
+
+	DBG("");
+
+	sk = socket(PF_INET, SOCK_DGRAM, 0);
+	if (sk < 0)
+		return -1;
+
+	memset(&ifr, 0, sizeof(ifr));
+	ifr.ifr_ifindex = index;
+
+	if (ioctl(sk, SIOCGIFNAME, &ifr) < 0) {
+		close(sk);
+		return -1;
+	}
+
+	DBG("ifname %s", ifr.ifr_name);
+
+	memset(&rt, 0, sizeof(rt));
+	rt.rt_flags = RTF_UP;
+
+	memset(&addr, 0, sizeof(addr));
+	addr.sin_family = AF_INET;
+	addr.sin_addr.s_addr = INADDR_ANY;
+
+	memcpy(&rt.rt_genmask, &addr, sizeof(rt.rt_genmask));
+	memcpy(&rt.rt_dst, &addr, sizeof(rt.rt_dst));
+	memcpy(&rt.rt_gateway, &addr, sizeof(rt.rt_gateway));
+
+	rt.rt_dev = ifr.ifr_name;
+
+	err = ioctl(sk, SIOCDELRT, &rt);
+	if (err < 0)
+		connman_error("Removing default interface route failed (%s)",
+							strerror(errno));
+	close(sk);
+
+	return err;
+}

=== modified file 'src/rtnl.c'
--- src/rtnl.c	2010-05-18 21:48:47 +0000
+++ src/rtnl.c	2010-06-03 12:42:23 +0000
@@ -485,7 +485,12 @@
 
 	__connman_ipconfig_newroute(index, scope, dststr, gatewaystr);
 
-	if (scope != RT_SCOPE_UNIVERSE || dst.s_addr != INADDR_ANY)
+	/* skip host specific routes */
+	if (scope != RT_SCOPE_UNIVERSE &&
+			!(scope == RT_SCOPE_LINK && dst.s_addr == INADDR_ANY))
+		return;
+
+	if (dst.s_addr != INADDR_ANY)
 		return;
 
 	for (list = rtnl_list; list; list = list->next) {
@@ -514,7 +519,12 @@
 
 	__connman_ipconfig_delroute(index, scope, dststr, gatewaystr);
 
-	if (scope != RT_SCOPE_UNIVERSE || dst.s_addr != INADDR_ANY)
+	/* skip host specific routes */
+	if (scope != RT_SCOPE_UNIVERSE &&
+			!(scope == RT_SCOPE_LINK && dst.s_addr == INADDR_ANY))
+		return;
+
+	if (dst.s_addr != INADDR_ANY)
 		return;
 
 	for (list = rtnl_list; list; list = list->next) {