flimflam: Add L2TP/IPsec VPN plugin

plugins/vpn.c

Index: plugins/vpn.c
diff --git a/plugins/vpn.c b/plugins/vpn.c
index af0e082c7276f9424023b3ad2274df21e5867fe2..fd96dddcfae840d585ee26cbaa41e261998f4bcc 100644
--- a/plugins/vpn.c
+++ b/plugins/vpn.c
@@ -25,7 +25,6 @@
  * vpn plugin is responsible for launching the external process and
  * handling notification callbacks to clock the provider state machine.
  *
- * TODO(sleffler) currently assumes extern vpn service uses tun but not all do
  * TODO(sleffler) seems to make more sense in src than plugins
  */
 
@@ -77,27 +76,40 @@ struct vpn_driver_data {
 
 static GHashTable *driver_hash = NULL;
 
-static int kill_tun(char *tun_name)
+static int kill_tun(struct connman_provider *provider)
 {
+	struct vpn_data *data = connman_provider_get_data(provider);
+	struct vpn_driver_data *vpn_driver_data;
+	const char *name;
 	struct ifreq ifr;
 	int fd, err;
 
+	if (data == NULL)
+		return -1;
+
+	name = connman_provider_get_driver_name(provider);
+	vpn_driver_data = g_hash_table_lookup(driver_hash, name);
+
+	if (vpn_driver_data != NULL && vpn_driver_data->vpn_driver !=NULL &&
+		vpn_driver_data->vpn_driver->flags == VPN_FLAG_NO_TUN)
+		return 0;
+
 	memset(&ifr, 0, sizeof(ifr));
 	ifr.ifr_flags = IFF_TUN | IFF_NO_PI;
-	strncpy(ifr.ifr_name, tun_name, sizeof(ifr.ifr_name));
+	strncpy(ifr.ifr_name, data->if_name, sizeof(ifr.ifr_name));

[thutt, 2011/03/17 15:02:21]

(nit) It looks like you are adding tabs in a file which does not have them.
Or, if it does, then lines 81..96 do not use tabs.

(That is, if my interpretation of the red chevrons indicate tabs)

[kmixter1, 2011/03/17 17:52:20]

It's the latter.  The file is mostly tabs but I didn't add them consistently.
Running tabify.

 
 	fd = open("/dev/net/tun", O_RDWR);
 	if (fd < 0) {
 		err = -errno;
 		connman_error("Failed to open /dev/net/tun to device %s: %s",
-			      tun_name, strerror(errno));
+			      data->if_name, strerror(errno));
 		return err;
 	}
 
 	if (ioctl(fd, TUNSETIFF, (void *)&ifr)) {
 		err = -errno;
 		connman_error("Failed to TUNSETIFF for device %s to it: %s",
-			      tun_name, strerror(errno));
+			      data->if_name, strerror(errno));
 		close(fd);
 		return err;
 	}
@@ -105,12 +117,12 @@ static int kill_tun(char *tun_name)
 	if (ioctl(fd, TUNSETPERSIST, 0)) {
 		err = -errno;
 		connman_error("Failed to set tun device %s nonpersistent: %s",
-			      tun_name, strerror(errno));
+			      data->if_name, strerror(errno));
 		close(fd);
 		return err;
 	}
 	close(fd);
-	_DBG_VPN("Killed tun device %s", tun_name);
+	_DBG_VPN("Killed tun device %s", data->if_name);
 	return 0;
 }
 
@@ -125,7 +137,7 @@ void vpn_died(struct connman_task *task, void *user_data)
 	if (!data)
 		goto vpn_exit;
 
-	kill_tun(data->if_name);
+	kill_tun(provider);
 	connman_provider_set_data(provider, NULL);
 	connman_rtnl_remove_watch(data->watch);
 
@@ -144,6 +156,36 @@ vpn_exit:
 	connman_task_destroy(task);
 }
 
+int vpn_set_ifname(struct connman_provider *provider, const char *ifname)
+{
+	struct vpn_data *data = connman_provider_get_data(provider);
+	int index;
+
+	if (data == NULL) {
+		_DBG_VPN("%s: provider data not accessible", __func__);
+		return  -EIO;
+        }
+
+	if (ifname == NULL) {
+		_DBG_VPN("%s: ifname not provided", __func__);
+		return  -EIO;
+        }
+
+	index = connman_inet_ifindex(ifname);
+	if (index < 0) {
+		_DBG_VPN("%s: could not get ifindex from %s", __func__, ifname);
+		return  -EIO;
+	}
+
+	data->if_name = (char *)g_strdup(ifname);
+	connman_provider_set_index(provider, index);
+
+	/* Set connect state to retry creating ipconfig with index above. */
+	connman_provider_set_state(provider, CONNMAN_PROVIDER_STATE_CONNECT);
+
+	return 0;
+}
+
 static void vpn_newlink(const char *ifname, unsigned flags, unsigned change,				void *user_data)
 {
 	struct connman_provider *provider = user_data;
@@ -159,7 +201,7 @@ static void vpn_newlink(const char *ifname, unsigned flags, unsigned change,
 	data->flags = flags;
 }
 
-static void vpn_notify(struct connman_task *task,
+static DBusMessage *vpn_notify(struct connman_task *task,

[thutt, 2011/03/17 15:02:21]

I don't understand why you changed this function to return a DBusMessage*, but
then always return NULL.

[kmixter1, 2011/03/17 17:52:20]

This is the default notification function if the provider doesn't override it. 
The change makes notification functions be able to return dbus responses, but
this one doesn't need to.

 			DBusMessage *msg, void *user_data)
 {
 	struct connman_provider *provider = user_data;
@@ -173,7 +215,7 @@ static void vpn_notify(struct connman_task *task,
 	name = connman_provider_get_driver_name(provider);
 	vpn_driver_data = g_hash_table_lookup(driver_hash, name);
 	if (vpn_driver_data == NULL)
-		return;
+		return NULL;
 
 	state = vpn_driver_data->vpn_driver->notify(msg, provider);
 	switch (state) {
@@ -193,34 +235,19 @@ static void vpn_notify(struct connman_task *task,
 					CONNMAN_PROVIDER_STATE_DISCONNECT);
 		break;
 	}
+
+	return NULL;
 }
 
-static int vpn_connect(struct connman_provider *provider)
+static int vpn_create_tun(struct connman_provider *provider)
 {
 	struct vpn_data *data = connman_provider_get_data(provider);
-	struct vpn_driver_data *vpn_driver_data;
 	struct ifreq ifr;
-	const char *name;
 	int i, fd, index;
 	int ret = 0;
 
-	if (data != NULL)
-		return -EISCONN;
-
-	data = g_try_new0(struct vpn_data, 1);
 	if (data == NULL)

[thutt, 2011/03/17 15:02:21]

I'm not a networking person, but is this returning an error stating that you're
already connected if you don't have any data?

[kmixter1, 2011/03/17 17:52:20]

Done and added a message.

-		return -ENOMEM;
-
-	data->provider = connman_provider_ref(provider);
-	data->watch = 0;
-	data->flags = 0;
-	data->task = NULL;
-	data->state = VPN_STATE_IDLE;
-
-	connman_provider_set_data(provider, data);
-
-	name = connman_provider_get_driver_name(provider);
-	vpn_driver_data = g_hash_table_lookup(driver_hash, name);
+		return -EISCONN;
 
 	fd = open("/dev/net/tun", O_RDWR);
 	if (fd < 0) {
@@ -269,24 +296,65 @@ static int vpn_connect(struct connman_provider *provider)
 	index = connman_inet_ifindex(data->if_name);
 	if (index < 0) {
 		connman_error("%s: failed to get tun ifindex", __func__);
-		kill_tun(data->if_name);
+		kill_tun(provider);
 		ret = -EIO;
 		goto exist_err;
 	}
 	connman_provider_set_index(provider, index);
 
+        return 0;
+
+exist_err:
+        return ret;
+}
+
+static int vpn_connect(struct connman_provider *provider)
+{
+	struct vpn_data *data = connman_provider_get_data(provider);
+	struct vpn_driver_data *vpn_driver_data;
+	const char *name;
+	int ret = 0;
+
+	if (data != NULL) {

[thutt, 2011/03/17 15:02:21]

I'm puzzled by the condition -vs- the message here.  If you have data, you
report an error that you could not access the data.  Maybe it's just that I
don't this stuff very much, but the condition & the message seem to contradict
each other.

It's particularly confusing because line 249 seems to do the opposite.

[kmixter1, 2011/03/17 17:52:20]

Updated the error message - I copy and pasted wrong when adding error messages
in response to an earlier review.

+		_DBG_VPN("%s: unable to access provider data", __func__);
+		return -EISCONN;
+	}
+
+	data = g_try_new0(struct vpn_data, 1);
+	if (data == NULL)
+		return -ENOMEM;
+
+	data->provider = connman_provider_ref(provider);
+	data->watch = 0;
+	data->flags = 0;
+	data->task = NULL;
+	data->state = VPN_STATE_IDLE;
+
+	connman_provider_set_data(provider, data);
+
+	name = connman_provider_get_driver_name(provider);
+	vpn_driver_data = g_hash_table_lookup(driver_hash, name);
+
+	if (vpn_driver_data != NULL && vpn_driver_data->vpn_driver != NULL &&
+		vpn_driver_data->vpn_driver->flags != VPN_FLAG_NO_TUN) {
+
+		ret = vpn_create_tun(provider);
+		if (ret < 0)
+			goto exist_err;
+	}
+
 	data->task = connman_task_create(vpn_driver_data->program);
 
 	if (data->task == NULL) {
 		ret = -ENOMEM;
-		kill_tun(data->if_name);
+		kill_tun(provider);
 		goto exist_err;
 	}
 
 	if (connman_task_set_notify(data->task, "notify",
 					vpn_notify, provider)) {
 		ret = -ENOMEM;
-		kill_tun(data->if_name);
+		kill_tun(provider);
 		connman_task_destroy(data->task);
 		data->task = NULL;
 		goto exist_err;
@@ -295,7 +363,7 @@ static int vpn_connect(struct connman_provider *provider)
 	ret = vpn_driver_data->vpn_driver->connect(provider, data->task,
 							data->if_name);
 	if (ret < 0) {
-		kill_tun(data->if_name);
+		kill_tun(provider);
 		connman_task_destroy(data->task);
 		data->task = NULL;
 		goto exist_err;
@@ -355,7 +423,6 @@ static int vpn_remove(struct connman_provider *provider)
 	struct vpn_data *data;
 
 	data = connman_provider_get_data(provider);
-	connman_provider_set_data(provider, NULL);
 	if (data == NULL)
 		return 0;
 
@@ -365,7 +432,8 @@ static int vpn_remove(struct connman_provider *provider)
 	connman_task_stop(data->task);
 
 	g_usleep(G_USEC_PER_SEC);
-	kill_tun(data->if_name);
+	kill_tun(provider);
+	connman_provider_set_data(provider, NULL);
 	return 0;
 }