libnm: don't skip routes in nm_setting_ip_config_add_route() that only differ by attributes

For kernel and NetworkManager's core, route identity is a complicated topic
(see NM_PLATFORM_IP_ROUTE_CMP_TYPE_ID). For example, a route
without explity table is treated identical to "table 254" or "table 0".

It would be complicated to have nm_setting_ip_config_add_route()
implement that logic, especially since libnm offers not public API
to expose kernel's logic.

However, previously nm_setting_ip_config_add_route() would only consider
dest/prefix,next_hop,metric when comparing for equality. Hence, with

  nmcli connection modify "$CON" +ipv4.routes '192.168.5.0/24'
  nmcli connection modify "$CON" +ipv4.routes '192.168.5.0/24 table=42'

the second route was not actually added, although it is a very different
route. Fix that, and consider attributes too. Note that this allows the user
to add two routes that look different to libnm, but are actually idential:

  nmcli connection modify "$CON" +ipv4.routes '192.168.5.0/24'
  nmcli connection modify "$CON" +ipv4.routes '192.168.5.0/24 table=254'

In the above example, the route instances look different, but
sementically they are both the same route in the main table (254).

This also allows the user to add routes that are semantically different, but
are treated as the same route by kernel:

  nmcli connection modify "$CON" +ipv6.routes 'a:b:c::/120'
  nmcli connection modify "$CON" +ipv6.routes 'a:b:c::/120 mtu=600'

I think libnm should allow to add routes as long as they look different
to libnm. Regardless how kernel and NetworkManager-core thinks about
route identity.

This changes API of nm_setting_ip_config_add_route(). However, I think
the previous behavior was just broken.

Same for nm_setting_ip_config_remove_route_by_value().

1 files changed, 12 insertions, 4 deletions

diff --git a/libnm-core/nm-setting-ip-config.c b/libnm-core/nm-setting-ip-config.c
index 8678edfe03..dd09baa62c 100644
--- a/libnm-core/nm-setting-ip-config.c
+++ b/libnm-core/nm-setting-ip-config.c
@@ -2140,8 +2140,14 @@ nm_setting_ip_config_get_route (NMSettingIPConfig *setting, int idx)
  * @setting: the #NMSettingIPConfig
  * @route: the route to add
  *
- * Adds a new route and associated information to the setting.  The
+ * Appends a new route and associated information to the setting.  The
  * given route is duplicated internally and is not changed by this function.
+ * If an identical route (considering attributes as well) already exists, the
+ * route is not added and the function returns %FALSE.
+ *
+ * Note that before 1.10, this function would not consider route attributes
+ * and not add a route that has an existing route with same dest/prefix,next_hop,metric
+ * parameters.
  *
  * Returns: %TRUE if the route was added; %FALSE if the route was already known.
  **/
@@ -2158,7 +2164,7 @@ nm_setting_ip_config_add_route (NMSettingIPConfig *setting,
 
 	priv = NM_SETTING_IP_CONFIG_GET_PRIVATE (setting);
 	for (i = 0; i < priv->routes->len; i++) {
-		if (nm_ip_route_equal (priv->routes->pdata[i], route))
+		if (_nm_ip_route_equal (priv->routes->pdata[i], route, TRUE))
 			return FALSE;
 	}
 
@@ -2193,7 +2199,9 @@ nm_setting_ip_config_remove_route (NMSettingIPConfig *setting, int idx)
  * @setting: the #NMSettingIPConfig
  * @route: the route to remove
  *
- * Removes the route @route.
+ * Removes the first matching route that matches @route.
+ * Note that before 1.10, this function would only compare dest/prefix,next_hop,metric
+ * and ignore route attributes. Now, @route must match exactly.
  *
  * Returns: %TRUE if the route was found and removed; %FALSE if it was not.
  **/
@@ -2209,7 +2217,7 @@ nm_setting_ip_config_remove_route_by_value (NMSettingIPConfig *setting,
 
 	priv = NM_SETTING_IP_CONFIG_GET_PRIVATE (setting);
 	for (i = 0; i < priv->routes->len; i++) {
-		if (nm_ip_route_equal (priv->routes->pdata[i], route)) {
+		if (_nm_ip_route_equal (priv->routes->pdata[i], route, TRUE)) {
 			g_ptr_array_remove_index (priv->routes, i);
 			g_object_notify (G_OBJECT (setting), NM_SETTING_IP_CONFIG_ROUTES);
 			return TRUE;