platform,libnm: cleanup handling of TOS for routes

- kernel ignores rtm_tos for IPv6 routes. While iproute2 accepts it,
  let libnm reject TOS attribute for routes as well.

- move the tos field from NMPlatformIPRoute to NMPlatformIP4Route.

- the tos field is part of the weak-id of an IPv4 route. Meaning,
  `ip route add` can add routes that only differ by their TOS.

7 files changed, 35 insertions, 32 deletions

diff --git a/libnm-core/nm-setting-ip-config.c b/libnm-core/nm-setting-ip-config.c
index 113b8f84c9..ac23ede0dc 100644
--- a/libnm-core/nm-setting-ip-config.c
+++ b/libnm-core/nm-setting-ip-config.c
@@ -1188,7 +1188,7 @@ nm_ip_route_set_attribute (NMIPRoute *route, const char *name, GVariant *value)
 static const NMVariantAttributeSpec * const ip_route_attribute_spec[] = {
 	ATTR_SPEC_PTR (NM_IP_ROUTE_ATTRIBUTE_SRC,             G_VARIANT_TYPE_STRING,   TRUE,  TRUE, 'a'),
 	ATTR_SPEC_PTR (NM_IP_ROUTE_ATTRIBUTE_FROM,            G_VARIANT_TYPE_STRING,   FALSE, TRUE, 'p'),
-	ATTR_SPEC_PTR (NM_IP_ROUTE_ATTRIBUTE_TOS,             G_VARIANT_TYPE_BYTE,     TRUE,  TRUE,  0 ),
+	ATTR_SPEC_PTR (NM_IP_ROUTE_ATTRIBUTE_TOS,             G_VARIANT_TYPE_BYTE,     TRUE,  FALSE, 0 ),
 	ATTR_SPEC_PTR (NM_IP_ROUTE_ATTRIBUTE_WINDOW,          G_VARIANT_TYPE_UINT32,   TRUE,  TRUE,  0 ),
 	ATTR_SPEC_PTR (NM_IP_ROUTE_ATTRIBUTE_CWND,            G_VARIANT_TYPE_UINT32,   TRUE,  TRUE,  0 ),
 	ATTR_SPEC_PTR (NM_IP_ROUTE_ATTRIBUTE_INITCWND,        G_VARIANT_TYPE_UINT32,   TRUE,  TRUE,  0 ),
diff --git a/src/nm-ip6-config.c b/src/nm-ip6-config.c
index dafc63cbfc..2197e96dc2 100644
--- a/src/nm-ip6-config.c
+++ b/src/nm-ip6-config.c
@@ -569,7 +569,6 @@ merge_route_attributes (NMIPRoute *s_route, NMPlatformIP6Route *r)
 	if (variant && g_variant_is_of_type (variant, G_VARIANT_TYPE_ ## variant_type)) \
 		r->field = g_variant_get_ ## type (variant);
 
-	GET_ATTR (NM_IP_ROUTE_ATTRIBUTE_TOS,            tos,            BYTE,     byte);
 	GET_ATTR (NM_IP_ROUTE_ATTRIBUTE_WINDOW,         window,         UINT32,   uint32);
 	GET_ATTR (NM_IP_ROUTE_ATTRIBUTE_CWND,           cwnd,           UINT32,   uint32);
 	GET_ATTR (NM_IP_ROUTE_ATTRIBUTE_INITCWND,       initcwnd,       UINT32,   uint32);
diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index da45a859b9..3722c90319 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -2018,7 +2018,9 @@ _new_from_nl_route (struct nlmsghdr *nlh, gboolean id_only)
 			memcpy (&obj->ip6_route.pref_src, nla_data (tb[RTA_PREFSRC]), addr_len);
 	}
 
-	if (!is_v4) {
+	if (is_v4)
+		obj->ip4_route.tos = rtm->rtm_tos;
+	else {
 		if (tb[RTA_SRC]) {
 			_check_addr_or_errout (tb, RTA_SRC, addr_len);
 			memcpy (&obj->ip6_route.src, nla_data (tb[RTA_SRC]), addr_len);
@@ -2032,7 +2034,6 @@ _new_from_nl_route (struct nlmsghdr *nlh, gboolean id_only)
 	obj->ip_route.initcwnd = initcwnd;
 	obj->ip_route.initrwnd = initrwnd;
 	obj->ip_route.mtu = mtu;
-	obj->ip_route.tos = rtm->rtm_tos;
 	obj->ip_route.lock_window   = NM_FLAGS_HAS (lock, 1 << RTAX_WINDOW);
 	obj->ip_route.lock_cwnd     = NM_FLAGS_HAS (lock, 1 << RTAX_CWND);
 	obj->ip_route.lock_initcwnd = NM_FLAGS_HAS (lock, 1 << RTAX_INITCWND);
@@ -2438,7 +2439,9 @@ _nl_msg_new_route (int nlmsg_type,
 	const guint32 lock = ip_route_get_lock_flag (NMP_OBJECT_CAST_IP_ROUTE (obj));
 	struct rtmsg rtmsg = {
 		.rtm_family = klass->addr_family,
-		.rtm_tos = obj->ip_route.tos,
+		.rtm_tos = is_v4
+		           ? obj->ip4_route.tos
+		           : 0,
 		.rtm_table = RT_TABLE_MAIN, /* omit setting RTA_TABLE attribute */
 		.rtm_protocol = nmp_utils_ip_config_source_coerce_to_rtprot (obj->ip_route.rt_source),
 		.rtm_scope = is_v4
diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c
index 1f1c73b4a0..0386981eba 100644
--- a/src/platform/nm-platform.c
+++ b/src/platform/nm-platform.c
@@ -4184,8 +4184,6 @@ nm_platform_ip4_route_to_string (const NMPlatformIP4Route *route, char *buf, gsi
 
 	_to_string_dev (NULL, route->ifindex, str_dev, sizeof (str_dev));
 
-	if (route->tos)
-		nm_sprintf_buf (str_tos, " tos 0x%x", (unsigned) route->tos);
 
 	g_snprintf (buf, len,
 	            "%s/%d"
@@ -4216,7 +4214,7 @@ nm_platform_ip4_route_to_string (const NMPlatformIP4Route *route, char *buf, gsi
 	            route->scope_inv ? (nm_platform_route_scope2str (nm_platform_route_scope_inv (route->scope_inv), str_scope, sizeof (str_scope))) : "",
 	            route->pref_src ? " pref-src " : "",
 	            route->pref_src ? inet_ntop (AF_INET, &route->pref_src, s_pref_src, sizeof(s_pref_src)) : "",
-	            route->tos ? str_tos : "",
+	            route->tos ? nm_sprintf_buf (str_tos, " tos 0x%x", (unsigned) route->tos) : "",
 	            route->window   || route->lock_window   ? nm_sprintf_buf (str_window,   " window %s%"G_GUINT32_FORMAT,   route->lock_window   ? "lock " : "", route->window)   : "",
 	            route->cwnd     || route->lock_cwnd     ? nm_sprintf_buf (str_cwnd,     " cwnd %s%"G_GUINT32_FORMAT,     route->lock_cwnd     ? "lock " : "", route->cwnd)     : "",
 	            route->initcwnd || route->lock_initcwnd ? nm_sprintf_buf (str_initcwnd, " initcwnd %s%"G_GUINT32_FORMAT, route->lock_initcwnd ? "lock " : "", route->initcwnd) : "",
@@ -4243,7 +4241,7 @@ nm_platform_ip6_route_to_string (const NMPlatformIP6Route *route, char *buf, gsi
 	char s_network[INET6_ADDRSTRLEN], s_gateway[INET6_ADDRSTRLEN], s_pref_src[INET6_ADDRSTRLEN];
 	char s_src[INET6_ADDRSTRLEN];
 	char str_dev[TO_STRING_DEV_BUF_SIZE], s_source[50];
-	char str_tos[32], str_window[32], str_cwnd[32], str_initcwnd[32], str_initrwnd[32], str_mtu[32];
+	char str_window[32], str_cwnd[32], str_initcwnd[32], str_initrwnd[32], str_mtu[32];
 
 	if (!nm_utils_to_string_buffer_init_null (route, &buf, &len))
 		return buf;
@@ -4259,9 +4257,6 @@ nm_platform_ip6_route_to_string (const NMPlatformIP6Route *route, char *buf, gsi
 
 	_to_string_dev (NULL, route->ifindex, str_dev, sizeof (str_dev));
 
-	if (route->tos)
-		nm_sprintf_buf (str_tos, " tos 0x%x", (unsigned) route->tos);
-
 	g_snprintf (buf, len,
 	            "%s/%d"
 	            " via %s"
@@ -4272,7 +4267,6 @@ nm_platform_ip6_route_to_string (const NMPlatformIP6Route *route, char *buf, gsi
 	            " src %s/%u" /* source */
 	            "%s" /* cloned */
 	            "%s%s" /* pref-src */
-	            "%s" /* tos */
 	            "%s" /* window */
 	            "%s" /* cwnd */
 	            "%s" /* initcwnd */
@@ -4290,7 +4284,6 @@ nm_platform_ip6_route_to_string (const NMPlatformIP6Route *route, char *buf, gsi
 	            route->rt_cloned ? " cloned" : "",
 	            s_pref_src[0] ? " pref-src " : "",
 	            s_pref_src[0] ? s_pref_src : "",
-	            route->tos ? str_tos : "",
 	            route->window   || route->lock_window   ? nm_sprintf_buf (str_window,   " window %s%"G_GUINT32_FORMAT,   route->lock_window   ? "lock " : "", route->window)   : "",
 	            route->cwnd     || route->lock_cwnd     ? nm_sprintf_buf (str_cwnd,     " cwnd %s%"G_GUINT32_FORMAT,     route->lock_cwnd     ? "lock " : "", route->cwnd)     : "",
 	            route->initcwnd || route->lock_initcwnd ? nm_sprintf_buf (str_initcwnd, " initcwnd %s%"G_GUINT32_FORMAT, route->lock_initcwnd ? "lock " : "", route->initcwnd) : "",
@@ -4733,6 +4726,7 @@ nm_platform_ip4_route_hash (const NMPlatformIP4Route *obj, NMPlatformIPRouteCmpT
 			h = NM_HASH_COMBINE (h, nm_utils_ip4_address_clear_host_address (obj->network, obj->plen));
 			h = NM_HASH_COMBINE (h, obj->plen);
 			h = NM_HASH_COMBINE (h, obj->metric);
+			h = NM_HASH_COMBINE (h, obj->tos);
 			if (cmp_type == NM_PLATFORM_IP_ROUTE_CMP_TYPE_ID) {
 				h = NM_HASH_COMBINE (h, obj->ifindex);
 				h = NM_HASH_COMBINE (h, obj->rt_source);
@@ -4804,6 +4798,7 @@ nm_platform_ip4_route_cmp (const NMPlatformIP4Route *a, const NMPlatformIP4Route
 		NM_CMP_DIRECT_IN4ADDR_SAME_PREFIX (a->network, b->network, MIN (a->plen, b->plen));
 		NM_CMP_FIELD (a, b, plen);
 		NM_CMP_FIELD (a, b, metric);
+		NM_CMP_FIELD (a, b, tos);
 		if (cmp_type == NM_PLATFORM_IP_ROUTE_CMP_TYPE_ID) {
 			NM_CMP_FIELD (a, b, ifindex);
 			NM_CMP_FIELD (a, b, rt_source);
@@ -4904,7 +4899,6 @@ nm_platform_ip6_route_hash (const NMPlatformIP6Route *obj, NMPlatformIPRouteCmpT
 			h = NM_HASH_COMBINE (h, obj->rt_source);
 			h = NM_HASH_COMBINE (h, obj->mss);
 			h = NM_HASH_COMBINE (h, obj->rt_cloned);
-			h = NM_HASH_COMBINE (h, obj->tos);
 			h = NM_HASH_COMBINE (h, obj->lock_window);
 			h = NM_HASH_COMBINE (h, obj->lock_cwnd);
 			h = NM_HASH_COMBINE (h, obj->lock_initcwnd);
@@ -4969,7 +4963,6 @@ nm_platform_ip6_route_cmp (const NMPlatformIP6Route *a, const NMPlatformIP6Route
 		NM_CMP_FIELD (a, b, rt_source);
 		NM_CMP_FIELD (a, b, mss);
 		NM_CMP_FIELD_UNSAFE (a, b, rt_cloned);
-		NM_CMP_FIELD (a, b, tos);
 		NM_CMP_FIELD_UNSAFE (a, b, lock_window);
 		NM_CMP_FIELD_UNSAFE (a, b, lock_cwnd);
 		NM_CMP_FIELD_UNSAFE (a, b, lock_initcwnd);
diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h
index ddd1f11e18..9f04d08917 100644
--- a/src/platform/nm-platform.h
+++ b/src/platform/nm-platform.h
@@ -392,8 +392,8 @@ typedef union {
 	guint32 mtu; \
 	\
 	\
+	/* RTA_PRIORITY (iproute2: metric) */ \
 	guint32 metric; \
-	guint32 tos; \
 	\
 	/*end*/
 
@@ -422,6 +422,13 @@ struct _NMPlatformIP4Route {
 	 * pref_src must match, unless set to 0.0.0.0 to match any. */
 	in_addr_t pref_src;
 
+	/* rtm_tos (iproute2: tos)
+	 *
+	 * For IPv4, tos is part of the weak-id (like metric).
+	 *
+	 * For IPv6, tos is ignored by kernel.  */
+	guint8 tos;
+
 	/* The bitwise inverse of the route scope rtm_scope. It is inverted so that the
 	 * default value (RT_SCOPE_NOWHERE) is zero. Use nm_platform_route_scope_inv()
 	 * to convert back and forth between the inverese representation and the
diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
index c222945420..d605acf81e 100644
--- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
+++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
@@ -467,23 +467,25 @@ parse_route_options (NMIPRoute *route, int family, const char *line, GError **er
 	}
 
 	/* tos */
-	regex = g_regex_new ("(?:\\s|^)tos\\s+(\\S+)(?:$|\\s)", 0, 0, NULL);
-	g_regex_match (regex, line, 0, &match_info);
-	if (g_match_info_matches (match_info)) {
-		gs_free char *str = g_match_info_fetch (match_info, 1);
-		gint64 num = _nm_utils_ascii_str_to_int64 (str, 16, 0, G_MAXUINT8, -1);
+	if (family == AF_INET) {
+		regex = g_regex_new ("(?:\\s|^)tos\\s+(\\S+)(?:$|\\s)", 0, 0, NULL);
+		g_regex_match (regex, line, 0, &match_info);
+		if (g_match_info_matches (match_info)) {
+			gs_free char *str = g_match_info_fetch (match_info, 1);
+			gint64 num = _nm_utils_ascii_str_to_int64 (str, 16, 0, G_MAXUINT8, -1);
 
-		if (num == -1) {
-			g_match_info_free (match_info);
-			g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION,
-			             "Invalid route %s '%s'", "tos", str);
-			goto out;
+			if (num == -1) {
+				g_match_info_free (match_info);
+				g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION,
+				             "Invalid route %s '%s'", "tos", str);
+				goto out;
+			}
+			nm_ip_route_set_attribute (route, NM_IP_ROUTE_ATTRIBUTE_TOS,
+			                           g_variant_new_byte ((guchar) num));
 		}
-		nm_ip_route_set_attribute (route, NM_IP_ROUTE_ATTRIBUTE_TOS,
-		                           g_variant_new_byte ((guchar) num));
+		g_clear_pointer (&regex, g_regex_unref);
+		g_clear_pointer (&match_info, g_match_info_free);
 	}
-	g_clear_pointer (&regex, g_regex_unref);
-	g_clear_pointer (&match_info, g_match_info_free);
 
 	/* from */
 	if (family == AF_INET6) {
diff --git a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
index 27cb74b69e..6833c65cbf 100644
--- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
+++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
@@ -4108,7 +4108,6 @@ test_write_wired_static (void)
 
 	route6 = nm_ip_route_new (AF_INET6, "::", 128, "2222:aaaa::9999", 1, &error);
 	g_assert_no_error (error);
-	nm_ip_route_set_attribute (route6, NM_IP_ROUTE_ATTRIBUTE_TOS, g_variant_new_byte (0xb8));
 	nm_ip_route_set_attribute (route6, NM_IP_ROUTE_ATTRIBUTE_CWND, g_variant_new_uint32 (100));
 	nm_ip_route_set_attribute (route6, NM_IP_ROUTE_ATTRIBUTE_MTU, g_variant_new_uint32 (1280));
 	nm_ip_route_set_attribute (route6, NM_IP_ROUTE_ATTRIBUTE_LOCK_CWND, g_variant_new_boolean (TRUE));