Connection Manager

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

  1. ptp-default-gateway
  2. Merge into trunk
Proposed by Kalle Valo
Status: Merged
Merged at revision: 2272
Proposed branch: lp:~kvalo/connman/ptp-default-gateway
Merge into: lp:~indicator-network-developers/connman/trunk
Diff against target: 150 lines (+72/-9)
4 files modified
include/inet.h (+1/-0)
src/connection.c (+14/-7)
src/inet.c (+45/-0)
src/rtnl.c (+12/-2)
To merge this branch: bzr merge lp:~kvalo/connman/ptp-default-gateway
Reviewer Review Type Date Requested Status
David Barth (community) Approve
Review via email: mp+26694@code.launchpad.net

Description of the change

Fix for default interface problem.

To post a comment you must log in.
Revision history for this message
David Barth (dbarth) wrote :

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
Revision history for this message
Kalle Valo (kvalo) wrote :

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.

Revision history for this message
David Barth (dbarth) wrote :

Ok +1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'include/inet.h'
2--- include/inet.h 2010-02-11 04:33:05 +0000
3+++ include/inet.h 2010-06-03 12:42:23 +0000
4@@ -50,6 +50,7 @@
5 int connman_inet_set_gateway_address(int index, const char *gateway);
6 int connman_inet_clear_gateway_address(int index, const char *gateway);
7 int connman_inet_set_gateway_interface(int index);
8+int connman_inet_clear_gateway_interface(int index);
9
10 #ifdef __cplusplus
11 }
12
13=== modified file 'src/connection.c'
14--- src/connection.c 2010-05-06 05:32:30 +0000
15+++ src/connection.c 2010-06-03 12:42:23 +0000
16@@ -66,18 +66,21 @@
17
18 static int del_routes(struct gateway_data *data)
19 {
20- const char *address;
21+ DBG("gateway %s", data->gateway);
22
23 if (data->vpn) {
24 if (data->vpn_phy_index >= 0)
25 connman_inet_del_host_route(data->vpn_phy_index,
26 data->gateway);
27- address = data->vpn_ip;
28+ return connman_inet_clear_gateway_address(data->index,
29+ data->vpn_ip);
30+ } else if (g_strcmp0(data->gateway, "0.0.0.0") == 0) {
31+ return connman_inet_clear_gateway_interface(data->index);
32 } else {
33 connman_inet_del_host_route(data->index, data->gateway);
34- address = data->gateway;
35+ return connman_inet_clear_gateway_address(data->index,
36+ data->gateway);
37 }
38- return connman_inet_clear_gateway_address(data->index, address);
39 }
40
41 static void find_element(struct connman_element *element, gpointer user_data)
42@@ -104,6 +107,13 @@
43 if (data == NULL)
44 return NULL;
45
46+ /*
47+ * If gateway is NULL, it's a point to point link and the default
48+ * gateway is 0.0.0.0, meaning the interface.
49+ */
50+ if (gateway == NULL)
51+ gateway = "0.0.0.0";
52+
53 data->index = index;
54 data->gateway = g_strdup(gateway);
55 data->active = FALSE;
56@@ -272,9 +282,6 @@
57
58 connman_element_set_enabled(element, TRUE);
59
60- if (gateway == NULL)
61- return 0;
62-
63 active_gateway = find_active_gateway();
64 new_gateway = add_gateway(element->index, gateway);
65
66
67=== modified file 'src/inet.c'
68--- src/inet.c 2010-03-07 23:01:29 +0000
69+++ src/inet.c 2010-06-03 12:42:23 +0000
70@@ -899,3 +899,48 @@
71
72 return err;
73 }
74+
75+int connman_inet_clear_gateway_interface(int index)
76+{
77+ struct ifreq ifr;
78+ struct rtentry rt;
79+ struct sockaddr_in addr;
80+ int sk, err;
81+
82+ DBG("");
83+
84+ sk = socket(PF_INET, SOCK_DGRAM, 0);
85+ if (sk < 0)
86+ return -1;
87+
88+ memset(&ifr, 0, sizeof(ifr));
89+ ifr.ifr_ifindex = index;
90+
91+ if (ioctl(sk, SIOCGIFNAME, &ifr) < 0) {
92+ close(sk);
93+ return -1;
94+ }
95+
96+ DBG("ifname %s", ifr.ifr_name);
97+
98+ memset(&rt, 0, sizeof(rt));
99+ rt.rt_flags = RTF_UP;
100+
101+ memset(&addr, 0, sizeof(addr));
102+ addr.sin_family = AF_INET;
103+ addr.sin_addr.s_addr = INADDR_ANY;
104+
105+ memcpy(&rt.rt_genmask, &addr, sizeof(rt.rt_genmask));
106+ memcpy(&rt.rt_dst, &addr, sizeof(rt.rt_dst));
107+ memcpy(&rt.rt_gateway, &addr, sizeof(rt.rt_gateway));
108+
109+ rt.rt_dev = ifr.ifr_name;
110+
111+ err = ioctl(sk, SIOCDELRT, &rt);
112+ if (err < 0)
113+ connman_error("Removing default interface route failed (%s)",
114+ strerror(errno));
115+ close(sk);
116+
117+ return err;
118+}
119
120=== modified file 'src/rtnl.c'
121--- src/rtnl.c 2010-05-18 21:48:47 +0000
122+++ src/rtnl.c 2010-06-03 12:42:23 +0000
123@@ -485,7 +485,12 @@
124
125 __connman_ipconfig_newroute(index, scope, dststr, gatewaystr);
126
127- if (scope != RT_SCOPE_UNIVERSE || dst.s_addr != INADDR_ANY)
128+ /* skip host specific routes */
129+ if (scope != RT_SCOPE_UNIVERSE &&
130+ !(scope == RT_SCOPE_LINK && dst.s_addr == INADDR_ANY))
131+ return;
132+
133+ if (dst.s_addr != INADDR_ANY)
134 return;
135
136 for (list = rtnl_list; list; list = list->next) {
137@@ -514,7 +519,12 @@
138
139 __connman_ipconfig_delroute(index, scope, dststr, gatewaystr);
140
141- if (scope != RT_SCOPE_UNIVERSE || dst.s_addr != INADDR_ANY)
142+ /* skip host specific routes */
143+ if (scope != RT_SCOPE_UNIVERSE &&
144+ !(scope == RT_SCOPE_LINK && dst.s_addr == INADDR_ANY))
145+ return;
146+
147+ if (dst.s_addr != INADDR_ANY)
148 return;
149
150 for (list = rtnl_list; list; list = list->next) {

Subscribers

People subscribed via source and target branches