dhcp: merge branch 'th/dhcp-client-id-infiniband-rh1658057'

- fix DHCP client-id generatation on infiniband devices
- change default ipv4.dhcp-client-id of internal DHCP plugin to "mac"

https://bugzilla.redhat.com/show_bug.cgi?id=1658057
https://bugzilla.redhat.com/show_bug.cgi?id=1661165

https://github.com/NetworkManager/NetworkManager/pull/263

8 files changed, 99 insertions, 62 deletions

diff --git a/contrib/fedora/rpm/00-server-dhcp-client-id.conf b/contrib/fedora/rpm/00-server-dhcp-client-id.conf
deleted file mode 100644
index 0cd3cd7167..0000000000
--- a/contrib/fedora/rpm/00-server-dhcp-client-id.conf
+++ /dev/null
@@ -1,6 +0,0 @@
-# default the DHCP client-id to MAC (instead of type 255, node-specific RFC 4361).
-# But don't do so for dhclient DHCP plugin, as the default of dhclient may
-# be specified via /etc/dhcp (and anyway defaults to "hardware" already).
-[connection-00-server-dhcp-client-id]
-match-device=*,except:dhcp-plugin:dhclient
-ipv4.dhcp-client-id=mac
diff --git a/contrib/fedora/rpm/NetworkManager.spec b/contrib/fedora/rpm/NetworkManager.spec
index b965ccc89c..c72dec1320 100644
--- a/contrib/fedora/rpm/NetworkManager.spec
+++ b/contrib/fedora/rpm/NetworkManager.spec
@@ -117,7 +117,6 @@ URL: http://www.gnome.org/projects/NetworkManager/
 Source: __SOURCE1__
 Source1: NetworkManager.conf
 Source2: 00-server.conf
-Source3: 00-server-dhcp-client-id.conf
 Source4: 20-connectivity-fedora.conf
 Source5: 20-connectivity-redhat.conf
 
@@ -694,7 +693,6 @@ make install DESTDIR=%{buildroot}
 cp %{SOURCE1} %{buildroot}%{_sysconfdir}/%{name}/
 
 cp %{SOURCE2} %{buildroot}%{nmlibdir}/conf.d/
-cp %{SOURCE3} %{buildroot}%{nmlibdir}/conf.d/
 
 %if %{with connectivity_fedora}
 cp %{SOURCE4} %{buildroot}%{nmlibdir}/conf.d/
@@ -979,7 +977,6 @@ fi
 %dir %{nmlibdir}
 %dir %{nmlibdir}/conf.d
 %{nmlibdir}/conf.d/00-server.conf
-%{nmlibdir}/conf.d/00-server-dhcp-client-id.conf
 
 
 %files dispatcher-routing-rules
diff --git a/contrib/fedora/rpm/build.sh b/contrib/fedora/rpm/build.sh
index 375c97eeec..017aab9c30 100755
--- a/contrib/fedora/rpm/build.sh
+++ b/contrib/fedora/rpm/build.sh
@@ -18,7 +18,6 @@
 #   SOURCE_FROM_GIT=|1|0
 #   SOURCE_NETWORKMANAGER_CONF=
 #   SOURCE_CONFIG_SERVER=
-#   SOURCE_CONFIG_SERVER_DHCP_CLIENT_ID=
 #   SOURCE_CONFIG_CONNECTIVITY_FEDORA=
 #   SOURCE_CONFIG_CONNECTIVITY_REDHAT=
 
@@ -122,7 +121,6 @@ fi
 
 SOURCE_NETWORKMANAGER_CONF="$(abs_path "$SOURCE_NETWORKMANAGER_CONF" "$SCRIPTDIR/NetworkManager.conf")" || die "invalid \$SOURCE_NETWORKMANAGER_CONF argument"
 SOURCE_CONFIG_SERVER="$(abs_path "$SOURCE_CONFIG_SERVER" "$SCRIPTDIR/00-server.conf")" || die "invalid \$SOURCE_CONFIG_SERVER argument"
-SOURCE_CONFIG_SERVER_DHCP_CLIENT_ID="$(abs_path "$SOURCE_CONFIG_SERVER_DHCP_CLIENT_ID" "$SCRIPTDIR/00-server-dhcp-client-id.conf")" || die "invalid \$SOURCE_CONFIG_SERVER_DHCP_CLIENT_ID argument"
 SOURCE_CONFIG_CONNECTIVITY_FEDORA="$(abs_path "$SOURCE_CONFIG_CONNECTIVITY_FEDORA" "$SCRIPTDIR/20-connectivity-fedora.conf")" || die "invalid \$SOURCE_CONFIG_CONNECTIVITY_FEDORA argument"
 SOURCE_CONFIG_CONNECTIVITY_REDHAT="$(abs_path "$SOURCE_CONFIG_CONNECTIVITY_REDHAT" "$SCRIPTDIR/20-connectivity-redhat.conf")" || die "invalid \$SOURCE_CONFIG_CONNECTIVITY_REDHAT argument"
 
@@ -146,7 +144,6 @@ LOG "SOURCE=$SOURCE"
 LOG "SOURCE_FROM_GIT=$SOURCE_FROM_GIT"
 LOG "SOURCE_NETWORKMANAGER_CONF=$SOURCE_NETWORKMANAGER_CONF"
 LOG "SOURCE_CONFIG_SERVER=$SOURCE_CONFIG_SERVER"
-LOG "SOURCE_CONFIG_SERVER_DHCP_CLIENT_ID=$SOURCE_CONFIG_SERVER_DHCP_CLIENT_ID"
 LOG "SOURCE_CONFIG_CONNECTIVITY_FEDORA=$SOURCE_CONFIG_CONNECTIVITY_FEDORA"
 LOG "SOURCE_CONFIG_CONNECTIVITY_REDHAT=$SOURCE_CONFIG_CONNECTIVITY_REDHAT"
 LOG "BUILDTYPE=$BUILDTYPE"
@@ -167,7 +164,6 @@ if [[ "$(dirname "$SOURCE")" != "$TEMP/SOURCES" ]]; then
 fi
 cp "$SOURCE_NETWORKMANAGER_CONF" "$TEMP/SOURCES/NetworkManager.conf" || die "Could not copy source $SOURCE_NETWORKMANAGER_CONF to $TEMP/SOURCES"
 cp "$SOURCE_CONFIG_SERVER" "$TEMP/SOURCES/00-server.conf" || die "Could not copy source $SOURCE_CONFIG_SERVER to $TEMP/SOURCES"
-cp "$SOURCE_CONFIG_SERVER_DHCP_CLIENT_ID" "$TEMP/SOURCES/00-server-dhcp-client-id.conf" || die "Could not copy source $SOURCE_CONFIG_SERVER_DHCP_CLIENT_ID to $TEMP/SOURCES"
 cp "$SOURCE_CONFIG_CONNECTIVITY_FEDORA" "$TEMP/SOURCES/20-connectivity-fedora.conf" || die "Could not copy source $SOURCE_CONFIG_CONNECTIVITY_FEDORA to $TEMP/SOURCES"
 cp "$SOURCE_CONFIG_CONNECTIVITY_REDHAT" "$TEMP/SOURCES/20-connectivity-redhat.conf" || die "Could not copy source $SOURCE_CONFIG_CONNECTIVITY_REDHAT to $TEMP/SOURCES"
 
diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index 9c76a2c887..f03dd1e59c 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -7457,37 +7457,6 @@ get_dhcp_timeout (NMDevice *self, int addr_family)
 	return timeout ?: NM_DHCP_TIMEOUT_DEFAULT;
 }
 
-static void
-_ASSERT_arp_type (guint16 arp_type,
-                  const guint8 *hwaddr,
-                  gsize hwaddr_len)
-{
-	/* we actually only support ethernet and infiniband below. Assert that
-	 * the arp-type and the address length correspond. */
-	nm_assert (NM_IN_SET (arp_type, ARPHRD_ETHER, ARPHRD_INFINIBAND));
-	nm_assert (arp_type <= 255);
-	nm_assert (hwaddr_len > 0);
-	nm_assert (arp_type != ARPHRD_ETHER      || hwaddr_len == ETH_ALEN);
-	nm_assert (arp_type != ARPHRD_INFINIBAND || hwaddr_len == INFINIBAND_ALEN);
-	nm_assert (hwaddr);
-}
-
-static GBytes *
-dhcp4_get_client_id_mac (guint16 arp_type,
-                         const guint8 *hwaddr,
-                         gsize hwaddr_len)
-{
-	guint8 *client_id_buf;
-	const guint8 hwaddr_type = arp_type;
-
-	_ASSERT_arp_type (arp_type, hwaddr, hwaddr_len);
-
-	client_id_buf = g_malloc (hwaddr_len + 1);
-	client_id_buf[0] = hwaddr_type;
-	memcpy (&client_id_buf[1], hwaddr, hwaddr_len);
-	return g_bytes_new_take (client_id_buf, hwaddr_len + 1);
-}
-
 static GBytes *
 dhcp4_get_client_id (NMDevice *self,
                      NMConnection *connection,
@@ -7531,13 +7500,13 @@ dhcp4_get_client_id (NMDevice *self,
 		}
 
 		hwaddr_bin = g_bytes_get_data (hwaddr, &hwaddr_len);
-		arp_type = nm_utils_detect_arp_type_from_addrlen (hwaddr_len);
+		arp_type = nm_utils_arp_type_detect_from_hwaddrlen (hwaddr_len);
 		if (arp_type < 0) {
 			fail_reason = "unsupported link-layer address";
 			goto out_fail;
 		}
 
-		result = dhcp4_get_client_id_mac ((guint16) arp_type, hwaddr_bin, hwaddr_len);
+		result = nm_utils_dhcp_client_id_mac (arp_type, hwaddr_bin, hwaddr_len);
 		goto out_good;
 	}
 
@@ -7553,13 +7522,13 @@ dhcp4_get_client_id (NMDevice *self,
 		if (!_nm_utils_hwaddr_aton (hwaddr_str, hwaddr_bin_buf, sizeof (hwaddr_bin_buf), &hwaddr_len))
 			g_return_val_if_reached (NULL);
 
-		arp_type = nm_utils_detect_arp_type_from_addrlen (hwaddr_len);
+		arp_type = nm_utils_arp_type_detect_from_hwaddrlen (hwaddr_len);
 		if (arp_type < 0) {
 			fail_reason = "unsupported permanent link-layer address";
 			goto out_fail;
 		}
 
-		result = dhcp4_get_client_id_mac ((guint16) arp_type, hwaddr_bin_buf, hwaddr_len);
+		result = nm_utils_dhcp_client_id_mac (arp_type, hwaddr_bin_buf, hwaddr_len);
 		goto out_good;
 	}
 
@@ -8214,7 +8183,7 @@ dhcp6_prefix_delegated (NMDhcpClient *client,
 #define EPOCH_DATETIME_200001010000  946684800
 
 static GBytes *
-generate_duid_llt (guint16 arp_type,
+generate_duid_llt (int arp_type,
                    const guint8 *hwaddr,
                    gsize hwaddr_len,
                    gint64 time)
@@ -8224,7 +8193,8 @@ generate_duid_llt (guint16 arp_type,
 	const guint16 hw_type = htons (arp_type);
 	const guint32 duid_time = htonl (NM_MAX (0, time - EPOCH_DATETIME_200001010000));
 
-	_ASSERT_arp_type (arp_type, hwaddr, hwaddr_len);
+	if (!nm_utils_arp_type_get_hwaddr_relevant_part (arp_type, &hwaddr, &hwaddr_len))
+		nm_assert_not_reached ();
 
 	arr = g_new (guint8, 2 + 2 + 4 + hwaddr_len);
 
@@ -8237,7 +8207,7 @@ generate_duid_llt (guint16 arp_type,
 }
 
 static GBytes *
-generate_duid_ll (guint16 arp_type,
+generate_duid_ll (int arp_type,
                   const guint8 *hwaddr,
                   gsize hwaddr_len)
 {
@@ -8245,7 +8215,8 @@ generate_duid_ll (guint16 arp_type,
 	const guint16 duid_type = htons (3);
 	const guint16 hw_type = htons (arp_type);
 
-	_ASSERT_arp_type (arp_type, hwaddr, hwaddr_len);
+	if (!nm_utils_arp_type_get_hwaddr_relevant_part (arp_type, &hwaddr, &hwaddr_len))
+		nm_assert_not_reached ();
 
 	arr = g_new (guint8, 2 + 2 + hwaddr_len);
 
@@ -8360,7 +8331,7 @@ dhcp6_get_duid (NMDevice *self, NMConnection *connection, GBytes *hwaddr, gboole
 		}
 
 		hwaddr_bin = g_bytes_get_data (hwaddr, &hwaddr_len);
-		arp_type = nm_utils_detect_arp_type_from_addrlen (hwaddr_len);
+		arp_type = nm_utils_arp_type_detect_from_hwaddrlen (hwaddr_len);
 		if (arp_type < 0) {
 			duid_error = "unsupported link-layer address";
 			goto out_fail;
@@ -8418,7 +8389,7 @@ dhcp6_get_duid (NMDevice *self, NMConnection *connection, GBytes *hwaddr, gboole
 				duid_error = "missing link-layer address";
 				goto out_fail;
 			}
-			if ((arp_type = nm_utils_detect_arp_type_from_addrlen (g_bytes_get_size (hwaddr))) < 0) {
+			if ((arp_type = nm_utils_arp_type_detect_from_hwaddrlen (g_bytes_get_size (hwaddr))) < 0) {
 				duid_error = "unsupported link-layer address";
 				goto out_fail;
 			}
diff --git a/src/dhcp/nm-dhcp-manager.c b/src/dhcp/nm-dhcp-manager.c
index a51c6e3847..c13c3043a5 100644
--- a/src/dhcp/nm-dhcp-manager.c
+++ b/src/dhcp/nm-dhcp-manager.c
@@ -239,7 +239,7 @@ client_start (NMDhcpManager *self,
 	 *
 	 * - for IPv4, the calling code may determine a client-id (from NM's connection profile).
 	 *   If present, it is taken. If not present, the DHCP plugin uses a plugin specific default.
-	 *     - for "internal" plugin, the default is just "duid".
+	 *     - for "internal" plugin, the default is just "mac".
 	 *     - for "dhclient", we try to get the configuration from dhclient's /etc/dhcp or fallback
 	 *       to whatever dhclient uses by default.
 	 *   We do it this way, because for dhclient the user may configure a default
diff --git a/src/dhcp/nm-dhcp-systemd.c b/src/dhcp/nm-dhcp-systemd.c
index 614485c4e7..bcbe916f60 100644
--- a/src/dhcp/nm-dhcp-systemd.c
+++ b/src/dhcp/nm-dhcp-systemd.c
@@ -681,7 +681,7 @@ ip4_start (NMDhcpClient *client,
 	hwaddr = nm_dhcp_client_get_hw_addr (client);
 	if (   !hwaddr
 	    || !(hwaddr_arr = g_bytes_get_data (hwaddr, &hwaddr_len))
-	    || (arp_type = nm_utils_detect_arp_type_from_addrlen (hwaddr_len)) < 0) {
+	    || (arp_type = nm_utils_arp_type_detect_from_hwaddrlen (hwaddr_len)) < 0) {
 		nm_utils_error_set_literal (error, NM_UTILS_ERROR_UNKNOWN, "invalid MAC address");
 		return FALSE;
 	}
@@ -725,8 +725,7 @@ ip4_start (NMDhcpClient *client,
 
 	client_id = nm_dhcp_client_get_client_id (client);
 	if (!client_id) {
-		client_id_new = nm_utils_dhcp_client_id_systemd_node_specific (TRUE,
-		                                                               nm_dhcp_client_get_iface (client));
+		client_id_new = nm_utils_dhcp_client_id_mac (arp_type, hwaddr_arr, hwaddr_len);
 		client_id = client_id_new;
 	}
 
@@ -740,6 +739,8 @@ ip4_start (NMDhcpClient *client,
 		return FALSE;
 	}
 
+	/* Note that we always set a client-id. In particular for infiniband that is necessary,
+	 * see https://tools.ietf.org/html/rfc4390#section-2.1 . */
 	r = sd_dhcp_client_set_client_id (sd_client,
 	                                  client_id_arr[0],
 	                                  client_id_arr + 1,
@@ -1032,7 +1033,7 @@ ip6_start (NMDhcpClient *client,
 	hwaddr = nm_dhcp_client_get_hw_addr (client);
 	if (   !hwaddr
 	    || !(hwaddr_arr = g_bytes_get_data (hwaddr, &hwaddr_len))
-	    || (arp_type = nm_utils_detect_arp_type_from_addrlen (hwaddr_len)) < 0) {
+	    || (arp_type = nm_utils_arp_type_detect_from_hwaddrlen (hwaddr_len)) < 0) {
 		nm_utils_error_set_literal (error, NM_UTILS_ERROR_UNKNOWN, "invalid MAC address");
 		return FALSE;
 	}
diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c
index 0ba3a5cb94..89f8f329f8 100644
--- a/src/nm-core-utils.c
+++ b/src/nm-core-utils.c
@@ -2841,7 +2841,7 @@ nm_utils_boot_id_bin (void)
 /*****************************************************************************/
 
 /**
- * nm_utils_detect_arp_type_from_addrlen:
+ * nm_utils_arp_type_detect_from_hwaddrlen:
  * @hwaddr_len: the length of the hardware address in bytes.
  *
  * Detects the arp-type based on the length of the MAC address.
@@ -2852,7 +2852,7 @@ nm_utils_boot_id_bin (void)
  *
  * Returns: the arp-type or negative value on error. */
 int
-nm_utils_detect_arp_type_from_addrlen (gsize hwaddr_len)
+nm_utils_arp_type_detect_from_hwaddrlen (gsize hwaddr_len)
 {
 	switch (hwaddr_len) {
 	case ETH_ALEN:
@@ -2867,6 +2867,51 @@ nm_utils_detect_arp_type_from_addrlen (gsize hwaddr_len)
 	}
 }
 
+gboolean
+nm_utils_arp_type_validate_hwaddr (int arp_type,
+                                   const guint8 *hwaddr,
+                                   gsize hwaddr_len)
+{
+
+	if (!hwaddr)
+		return FALSE;
+
+	if (arp_type == ARPHRD_ETHER) {
+		G_STATIC_ASSERT (ARPHRD_ETHER >= 0 && ARPHRD_ETHER <= 0xFF);
+		if (hwaddr_len != ETH_ALEN)
+			return FALSE;
+	} else if (arp_type == ARPHRD_INFINIBAND) {
+		G_STATIC_ASSERT (ARPHRD_INFINIBAND >= 0 && ARPHRD_INFINIBAND <= 0xFF);
+		if (hwaddr_len != INFINIBAND_ALEN)
+			return FALSE;
+	} else
+		return FALSE;
+
+	nm_assert (arp_type == nm_utils_arp_type_detect_from_hwaddrlen (hwaddr_len));
+	return TRUE;
+}
+
+gboolean
+nm_utils_arp_type_get_hwaddr_relevant_part (int arp_type,
+                                            const guint8 **hwaddr,
+                                            gsize *hwaddr_len)
+{
+	g_return_val_if_fail (   hwaddr
+	                      && hwaddr_len
+	                      && nm_utils_arp_type_validate_hwaddr (arp_type, *hwaddr, *hwaddr_len),
+	                      FALSE);
+
+	/* for infiniband, we only consider the last 8 bytes. */
+	if (arp_type == ARPHRD_INFINIBAND) {
+		*hwaddr += (INFINIBAND_ALEN - 8);
+		*hwaddr_len = 8;
+	}
+
+	return TRUE;
+}
+
+/*****************************************************************************/
+
 /* Returns the "u" (universal/local) bit value for a Modified EUI-64 */
 static gboolean
 get_gre_eui64_u_bit (guint32 addr)
@@ -3514,6 +3559,23 @@ nm_utils_hw_addr_gen_stable_eth (NMUtilsStableType stable_type,
 
 /*****************************************************************************/
 
+GBytes *
+nm_utils_dhcp_client_id_mac (int arp_type,
+                             const guint8 *hwaddr,
+                             gsize hwaddr_len)
+{
+	guint8 *client_id_buf;
+	const guint8 hwaddr_type = arp_type;
+
+	if (!nm_utils_arp_type_get_hwaddr_relevant_part (arp_type, &hwaddr, &hwaddr_len))
+		g_return_val_if_reached (NULL);
+
+	client_id_buf = g_malloc (hwaddr_len + 1);
+	client_id_buf[0] = hwaddr_type;
+	memcpy (&client_id_buf[1], hwaddr, hwaddr_len);
+	return g_bytes_new_take (client_id_buf, hwaddr_len + 1);
+}
+
 #define HASH_KEY ((const guint8[16]) { 0x80, 0x11, 0x8c, 0xc2, 0xfe, 0x4a, 0x03, 0xee, 0x3e, 0xd6, 0x0c, 0x6f, 0x36, 0x39, 0x14, 0x09 })
 
 /**
diff --git a/src/nm-core-utils.h b/src/nm-core-utils.h
index ea9174bd15..a93854a465 100644
--- a/src/nm-core-utils.h
+++ b/src/nm-core-utils.h
@@ -279,7 +279,19 @@ gboolean nm_utils_host_id_get (const guint8 **out_host_id,
                                gsize *out_host_id_len);
 gint64 nm_utils_host_id_get_timestamp_ns (void);
 
-int nm_utils_detect_arp_type_from_addrlen (gsize hwaddr_len);
+/*****************************************************************************/
+
+int nm_utils_arp_type_detect_from_hwaddrlen (gsize hwaddr_len);
+
+gboolean nm_utils_arp_type_validate_hwaddr (int arp_type,
+                                            const guint8 *hwaddr,
+                                            gsize hwaddr_len);
+
+gboolean nm_utils_arp_type_get_hwaddr_relevant_part (int arp_type,
+                                                     const guint8 **hwaddr,
+                                                     gsize *hwaddr_len);
+
+/*****************************************************************************/
 
 /* IPv6 Interface Identifier helpers */
 
@@ -375,6 +387,10 @@ char *nm_utils_hw_addr_gen_stable_eth (NMUtilsStableType stable_type,
 
 /*****************************************************************************/
 
+GBytes *nm_utils_dhcp_client_id_mac (int arp_type,
+                                     const guint8 *hwaddr,
+                                     gsize hwaddr_len);
+
 guint32 nm_utils_create_dhcp_iaid (gboolean legacy_unstable_byteorder,
                                    const guint8 *interface_id,
                                    gsize interface_id_len);