platform: support creating non-persistant TUN/TAP devicesth/platform-tun

For completeness, extend the API to support non-persistant
device. That requires that nm_platform_link_tun_add()
returns the file descriptor.

While NetworkManager doesn't create such devices itself,
it recognizes the IFLA_TUN_PERSIST / IFF_PERSIST flag.
Since ip-tuntap (obviously) cannot create such devices,
we cannot add a test for how non-persistent devices look
in the platform cache. Well, we could instead add them
with ioctl directly, but instead, just extend the platform
API to allow for that.

Also, use the function from test-lldp.c to (optionally) use
nm_platform_link_tun_add() to create the tap device.

8 files changed, 88 insertions, 35 deletions

diff --git a/src/devices/nm-device-tun.c b/src/devices/nm-device-tun.c
index 123455f16d..c3ce4b7357 100644
--- a/src/devices/nm-device-tun.c
+++ b/src/devices/nm-device-tun.c
@@ -264,7 +264,8 @@ create_and_realize (NMDevice *device,
 	plerr = nm_platform_link_tun_add (nm_device_get_platform (device),
 	                                  iface,
 	                                  &props,
-	                                  out_plink);
+	                                  out_plink,
+	                                  NULL);
 	if (plerr != NM_PLATFORM_ERROR_SUCCESS) {
 		g_set_error (error, NM_DEVICE_ERROR, NM_DEVICE_ERROR_CREATION_FAILED,
 		             "Failed to create TUN/TAP interface '%s' for '%s': %s",
diff --git a/src/devices/tests/test-lldp.c b/src/devices/tests/test-lldp.c
index 655f26ecad..5d0625f80a 100644
--- a/src/devices/tests/test-lldp.c
+++ b/src/devices/tests/test-lldp.c
@@ -347,8 +347,7 @@ static void
 _test_recv_fixture_setup (TestRecvFixture *fixture, gconstpointer user_data)
 {
 	const NMPlatformLink *link;
-	struct ifreq ifr = { };
-	int fd, s;
+	nm_auto_close int fd = -1;
 
 	fd = open ("/dev/net/tun", O_RDWR | O_CLOEXEC);
 	if (fd == -1) {
@@ -357,20 +356,45 @@ _test_recv_fixture_setup (TestRecvFixture *fixture, gconstpointer user_data)
 		return;
 	}
 
-	ifr.ifr_flags = IFF_TAP | IFF_NO_PI;
-	nm_utils_ifname_cpy (ifr.ifr_name, TEST_IFNAME);
-	g_assert (ioctl (fd, TUNSETIFF, &ifr) >= 0);
-
-	/* Bring the interface up */
-	s = socket (AF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0);
-	g_assert (s >= 0);
-	ifr.ifr_flags |= IFF_UP;
-	g_assert (ioctl (s, SIOCSIFFLAGS, &ifr) >= 0);
-	nm_close (s);
+	if (nmtst_get_rand_bool ()) {
+		const NMPlatformLnkTun lnk = {
+			.type = IFF_TAP,
+			.pi = FALSE,
+			.vnet_hdr = FALSE,
+			.multi_queue = FALSE,
+			.persist = FALSE,
+		};
+
+		nm_close (nm_steal_fd (&fd));
+
+		link = nmtstp_link_tun_add (NM_PLATFORM_GET,
+		                            FALSE,
+		                            TEST_IFNAME,
+		                            &lnk,
+		                            &fd);
+		g_assert (link);
+		nmtstp_link_set_updown (NM_PLATFORM_GET, -1, link->ifindex, TRUE);
+		link = nmtstp_assert_wait_for_link (NM_PLATFORM_GET, TEST_IFNAME, NM_LINK_TYPE_TUN, 0);
+	} else {
+		int s;
+		struct ifreq ifr = { };
+
+		ifr.ifr_flags = IFF_TAP | IFF_NO_PI;
+		nm_utils_ifname_cpy (ifr.ifr_name, TEST_IFNAME);
+		g_assert (ioctl (fd, TUNSETIFF, &ifr) >= 0);
+
+		/* Bring the interface up */
+		s = socket (AF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0);
+		g_assert (s >= 0);
+		ifr.ifr_flags |= IFF_UP;
+		g_assert (ioctl (s, SIOCSIFFLAGS, &ifr) >= 0);
+		nm_close (s);
+
+		link = nmtstp_assert_wait_for_link (NM_PLATFORM_GET, TEST_IFNAME, NM_LINK_TYPE_TUN, 100);
+	}
 
-	link = nmtstp_assert_wait_for_link (NM_PLATFORM_GET, TEST_IFNAME, NM_LINK_TYPE_TUN, 100);
 	fixture->ifindex = link->ifindex;
-	fixture->fd = fd;
+	fixture->fd = nm_steal_fd (&fd);
 	memcpy (fixture->mac, link->addr.data, ETH_ALEN);
 }
 
diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index 8923708657..0ed8fa06fb 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -5596,15 +5596,15 @@ static gboolean
 link_tun_add (NMPlatform *platform,
               const char *name,
               const NMPlatformLnkTun *props,
-              const NMPlatformLink **out_link)
+              const NMPlatformLink **out_link,
+              int *out_fd)
 {
 	const NMPObject *obj;
 	struct ifreq ifr = { };
 	nm_auto_close int fd = -1;
 
-	if (   !NM_IN_SET (props->type, IFF_TAP, IFF_TUN)
-	    || !props->persist)
-		g_return_val_if_reached (FALSE);
+	nm_assert (NM_IN_SET (props->type, IFF_TAP, IFF_TUN));
+	nm_assert (props->persist || out_fd);
 
 	fd = open ("/dev/net/tun", O_RDWR | O_CLOEXEC);
 	if (fd < 0)
@@ -5629,18 +5629,23 @@ link_tun_add (NMPlatform *platform,
 			return FALSE;
 	}
 
-	if (ioctl (fd, TUNSETPERSIST, (int) !!props->persist))
-		return FALSE;
+	if (props->persist) {
+		if (ioctl (fd, TUNSETPERSIST, 1))
+			return FALSE;
+	}
 
 	do_request_link (platform, 0, name);
 	obj = nmp_cache_lookup_link_full (nm_platform_get_cache (platform),
 	                                  0, name, FALSE,
 	                                  NM_LINK_TYPE_TUN,
 	                                  NULL, NULL);
-	if (out_link)
-		*out_link = obj ? &obj->link : NULL;
 
-	return !!obj;
+	if (!obj)
+		return FALSE;
+
+	NM_SET_OUT (out_link, &obj->link);
+	NM_SET_OUT (out_fd, nm_steal_fd (&fd));
+	return TRUE;
 }
 
 static gboolean
diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c
index e264324aa0..976039def4 100644
--- a/src/platform/nm-platform.c
+++ b/src/platform/nm-platform.c
@@ -2046,6 +2046,11 @@ nm_platform_link_vxlan_add (NMPlatform *self,
  * @vnet_hdr: whether to set the IFF_VNET_HDR flag
  * @multi_queue: whether to set the IFF_MULTI_QUEUE flag
  * @out_link: on success, the link object
+ * @out_fd: (allow-none): if give, return the file descriptor for the
+ *   created device. Note that when creating a non-persistent device,
+ *   this argument is mandatory, otherwise it makes no sense
+ *   to create such an interface.
+ *   The caller is responsible for closing this file descriptor.
  *
  * Create a TUN or TAP interface.
  */
@@ -2053,7 +2058,8 @@ NMPlatformError
 nm_platform_link_tun_add (NMPlatform *self,
                           const char *name,
                           const NMPlatformLnkTun *props,
-                          const NMPlatformLink **out_link)
+                          const NMPlatformLink **out_link,
+                          int *out_fd)
 {
 	char b[255];
 	NMPlatformError plerr;
@@ -2062,6 +2068,13 @@ nm_platform_link_tun_add (NMPlatform *self,
 
 	g_return_val_if_fail (name, NM_PLATFORM_ERROR_BUG);
 	g_return_val_if_fail (props, NM_PLATFORM_ERROR_BUG);
+	g_return_val_if_fail (NM_IN_SET (props->type, IFF_TUN, IFF_TAP), NM_PLATFORM_ERROR_BUG);
+
+	/* creating a non-persistant device requires that the caller handles
+	 * the file descriptor. */
+	g_return_val_if_fail (props->persist || out_fd, NM_PLATFORM_ERROR_BUG);
+
+	NM_SET_OUT (out_fd, -1);
 
 	plerr = _link_add_check_existing (self, name, NM_LINK_TYPE_TUN, out_link);
 	if (plerr != NM_PLATFORM_ERROR_SUCCESS)
@@ -2069,7 +2082,7 @@ nm_platform_link_tun_add (NMPlatform *self,
 
 	_LOGD ("link: adding tun '%s' %s",
 	       name, nm_platform_lnk_tun_to_string (props, b, sizeof (b)));
-	if (!klass->link_tun_add (self, name, props, out_link))
+	if (!klass->link_tun_add (self, name, props, out_link, out_fd))
 		return NM_PLATFORM_ERROR_UNSPECIFIED;
 	return NM_PLATFORM_ERROR_SUCCESS;
 }
diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h
index 824735083a..e6cef63bae 100644
--- a/src/platform/nm-platform.h
+++ b/src/platform/nm-platform.h
@@ -853,7 +853,8 @@ typedef struct {
 	gboolean (*link_tun_add) (NMPlatform *platform,
 	                          const char *name,
 	                          const NMPlatformLnkTun *props,
-	                          const NMPlatformLink **out_link);
+	                          const NMPlatformLink **out_link,
+	                          int *out_fd);
 
 	gboolean (*infiniband_partition_add) (NMPlatform *, int parent, int p_key, const NMPlatformLink **out_link);
 	gboolean (*infiniband_partition_delete) (NMPlatform *, int parent, int p_key);
@@ -1286,7 +1287,8 @@ NMPlatformError nm_platform_link_sit_add (NMPlatform *self,
 NMPlatformError nm_platform_link_tun_add (NMPlatform *self,
                                           const char *name,
                                           const NMPlatformLnkTun *props,
-                                          const NMPlatformLink **out_link);
+                                          const NMPlatformLink **out_link,
+                                          int *out_fd);
 
 const NMPlatformIP6Address *nm_platform_ip6_address_get (NMPlatform *self, int ifindex, struct in6_addr address);
 
diff --git a/src/platform/tests/test-common.c b/src/platform/tests/test-common.c
index f54c0dd4c1..8724fedc86 100644
--- a/src/platform/tests/test-common.c
+++ b/src/platform/tests/test-common.c
@@ -1472,7 +1472,8 @@ const NMPlatformLink *
 nmtstp_link_tun_add (NMPlatform *platform,
                      gboolean external_command,
                      const char *name,
-                     const NMPlatformLnkTun *lnk)
+                     const NMPlatformLnkTun *lnk,
+                     int *out_fd)
 {
 	const NMPlatformLink *pllink = NULL;
 	NMPlatformError plerr;
@@ -1481,6 +1482,7 @@ nmtstp_link_tun_add (NMPlatform *platform,
 	g_assert (nm_utils_is_valid_iface_name (name, NULL));
 	g_assert (lnk);
 	g_assert (NM_IN_SET (lnk->type, IFF_TUN, IFF_TAP));
+	g_assert (!out_fd || *out_fd == -1);
 
 	if (!lnk->persist) {
 		/* ip tuntap does not support non-persistent devices.
@@ -1522,7 +1524,8 @@ nmtstp_link_tun_add (NMPlatform *platform,
 		else
 			g_error ("failure to add tun/tap device via ip-route");
 	} else {
-		plerr = nm_platform_link_tun_add (platform, name, lnk, &pllink);
+		g_assert (lnk->persist || out_fd);
+		plerr = nm_platform_link_tun_add (platform, name, lnk, &pllink, out_fd);
 		g_assert_cmpint (plerr, ==, NM_PLATFORM_ERROR_SUCCESS);
 	}
 
diff --git a/src/platform/tests/test-common.h b/src/platform/tests/test-common.h
index e7e8b127b5..bd02b0d7d3 100644
--- a/src/platform/tests/test-common.h
+++ b/src/platform/tests/test-common.h
@@ -313,7 +313,8 @@ const NMPlatformLink *nmtstp_link_sit_add (NMPlatform *platform,
 const NMPlatformLink *nmtstp_link_tun_add (NMPlatform *platform,
                                            gboolean external_command,
                                            const char *name,
-                                           const NMPlatformLnkTun *lnk);
+                                           const NMPlatformLnkTun *lnk,
+                                           int *out_fd);
 const NMPlatformLink *nmtstp_link_vxlan_add (NMPlatform *platform,
                                              gboolean external_command,
                                              const char *name,
diff --git a/src/platform/tests/test-link.c b/src/platform/tests/test-link.c
index a44936e5dc..dcd600ee64 100644
--- a/src/platform/tests/test-link.c
+++ b/src/platform/tests/test-link.c
@@ -699,6 +699,7 @@ test_software_detect (gconstpointer user_data)
 	guint i_step;
 	const gboolean ext = test_data->external_command;
 	NMPlatformLnkTun lnk_tun;
+	nm_auto_close int tun_fd = -1;
 
 	nmtstp_run_command_check ("ip link add %s type dummy", PARENT_NAME);
 	ifindex_parent = nmtstp_assert_wait_for_link (NM_PLATFORM_GET, PARENT_NAME, NM_LINK_TYPE_DUMMY, 100)->ifindex;
@@ -910,9 +911,9 @@ test_software_detect (gconstpointer user_data)
 				.vnet_hdr = nmtst_get_rand_bool (),
 				.multi_queue = nmtst_get_rand_bool (),
 
-				/* arguably, adding non-persistant devices from within NetworkManager makes
-				 * no sense. */
-				.persist = TRUE,
+				/* if we add the device via iproute2 (external), we can only
+				 * create persistent devices. */
+				.persist = (ext == 1) ? TRUE : nmtst_get_rand_bool (),
 			};
 			break;
 		default:
@@ -920,7 +921,10 @@ test_software_detect (gconstpointer user_data)
 			break;
 		}
 
-		g_assert (nmtstp_link_tun_add (NULL, ext, DEVICE_NAME, &lnk_tun));
+		g_assert (nmtstp_link_tun_add (NULL, ext, DEVICE_NAME, &lnk_tun,
+		                               (!lnk_tun.persist || nmtst_get_rand_bool ())
+		                                 ? &tun_fd
+		                                 : NULL));
 		break;
 	}
 	default: