libnm: normalize interface-name for infiniband profiles

NetworkManager does not support changing the interface name for
infiniband interfaces. Consequently, we verify that
"connection.interface-name" is either unset or set to the expected
"$parent.$p_key".  Anything else wouldn't work anyway and is rejected as
invalid configuration. That brings problems however.

Rejecting invalid configuration seems fine at first:

  $ nmcli --offline connection add type infiniband infiniband.parent ib0 infiniband.p-key 0x8010 connection.interface-name xxx
  Error: Error writing connection: connection.interface-name: interface name of software infiniband device must be 'ib0.8010' or unset (instead it is 'xxx')

However, when we modify the p-key, we also get an error message:

  $ nmcli --offline connection add type infiniband infiniband.parent ib0 infiniband.p-key 0x8010 connection.interface-name ib0.8010 |
    nmcli --offline connection modify infiniband.p-key 5
  Error: Error writing connection: connection.interface-name: interface name of software infiniband device must be 'ib0.0005' or unset (instead it is 'ib0.8010')

It's worse, because ifcfg-rh reader will mangle the PKEY_ID with |=0x8000 to set
the full membership flag. That means, if you add a profile like

  $ nmcli --offline connection add type infiniband infiniband.parent ib0 infiniband.p-key 0x0010 connection.interface-name ib0.0010

it gets written to ifcfg-rh file. Then upon reload it's invalid (as the
interface name mismatches).

There are multiple solutions for this. For example, ifcfg-rh reader could also
mangle the connection.interface-name, so that the overall result is valid. Or
we could just not validate at all, and accept any bogus interface-name.

With this patch instead we will just normalize the invalid configuration to
make it right.

  $ nmcli --offline connection add type infiniband infiniband.parent ib0 infiniband.p-key 0x8010 connection.interface-name ib0.8010 |
    nmcli --offline connection modify infiniband.p-key 5
  ...

The downside is that this happens silently, so a user doesn't
notice that configuration is ignored:

  $ nmcli --offline connection add type infiniband infiniband.parent ib0 infiniband.p-key 0x8010 connection.interface-name foo
  ...
  interface-name=ib0.8010

This approach still seems preferable, because setting
"connection.interface-name" for infiniband profiles makes little sense,
so what we care here is to avoid problems.

(cherry picked from commit 4610fd67e6e795131a358b292ec3fc1ba2a2250f)

3 files changed, 63 insertions, 20 deletions

diff --git a/src/libnm-core-impl/nm-connection.c b/src/libnm-core-impl/nm-connection.c
index 3a9eda0e18..b017a6e51c 100644
--- a/src/libnm-core-impl/nm-connection.c
+++ b/src/libnm-core-impl/nm-connection.c
@@ -1332,18 +1332,41 @@ _normalize_ip_config(NMConnection *self, GHashTable *parameters)
 }
 
 static gboolean
-_normalize_infiniband_mtu(NMConnection *self)
+_normalize_infiniband(NMConnection *self)
 {
     NMSettingInfiniband *s_infini = nm_connection_get_setting_infiniband(self);
+    gboolean             changed  = FALSE;
+    const char          *interface_name;
+    int                  p_key;
 
-    if (!s_infini || nm_setting_infiniband_get_mtu(s_infini) <= NM_INFINIBAND_MAX_MTU
-        || !NM_IN_STRSET(nm_setting_infiniband_get_transport_mode(s_infini),
-                         "datagram",
-                         "connected"))
+    if (!s_infini)
         return FALSE;
 
-    g_object_set(s_infini, NM_SETTING_INFINIBAND_MTU, (guint) NM_INFINIBAND_MAX_MTU, NULL);
-    return TRUE;
+    if (nm_setting_infiniband_get_mtu(s_infini) > NM_INFINIBAND_MAX_MTU) {
+        if (NM_IN_STRSET(nm_setting_infiniband_get_transport_mode(s_infini),
+                         "datagram",
+                         "connected")) {
+            g_object_set(s_infini, NM_SETTING_INFINIBAND_MTU, (guint) NM_INFINIBAND_MAX_MTU, NULL);
+            changed = TRUE;
+        }
+    }
+
+    if ((p_key = nm_setting_infiniband_get_p_key(s_infini)) != -1
+        && (interface_name = nm_connection_get_interface_name(self))) {
+        const char *virtual_iface_name;
+
+        virtual_iface_name = nm_setting_infiniband_get_virtual_interface_name(s_infini);
+
+        if (!nm_streq0(interface_name, virtual_iface_name)) {
+            g_object_set(nm_connection_get_setting_connection(self),
+                         NM_SETTING_CONNECTION_INTERFACE_NAME,
+                         virtual_iface_name,
+                         NULL);
+            changed = TRUE;
+        }
+    }
+
+    return changed;
 }
 
 static gboolean
@@ -1986,7 +2009,7 @@ _connection_normalize(NMConnection *connection,
     was_modified |= _normalize_invalid_slave_port_settings(connection);
     was_modified |= _normalize_ip_config(connection, parameters);
     was_modified |= _normalize_ethernet_link_neg(connection);
-    was_modified |= _normalize_infiniband_mtu(connection);
+    was_modified |= _normalize_infiniband(connection);
     was_modified |= _normalize_bond_mode(connection);
     was_modified |= _normalize_bond_options(connection);
     was_modified |= _normalize_wireless_mac_address_randomization(connection);
diff --git a/src/libnm-core-impl/nm-setting-infiniband.c b/src/libnm-core-impl/nm-setting-infiniband.c
index 7ba5720619..f5b5bb3d1f 100644
--- a/src/libnm-core-impl/nm-setting-infiniband.c
+++ b/src/libnm-core-impl/nm-setting-infiniband.c
@@ -181,8 +181,8 @@ nm_setting_infiniband_get_virtual_interface_name(NMSettingInfiniband *setting)
 static gboolean
 verify(NMSetting *setting, NMConnection *connection, GError **error)
 {
-    NMSettingConnection        *s_con = NULL;
-    NMSettingInfinibandPrivate *priv  = NM_SETTING_INFINIBAND_GET_PRIVATE(setting);
+    NMSettingConnection        *s_con;
+    NMSettingInfinibandPrivate *priv = NM_SETTING_INFINIBAND_GET_PRIVATE(setting);
 
     if (priv->mac_address && !nm_utils_hwaddr_valid(priv->mac_address, INFINIBAND_ALEN)) {
         g_set_error_literal(error,
@@ -251,8 +251,10 @@ verify(NMSetting *setting, NMConnection *connection, GError **error)
         }
     }
 
-    if (connection)
-        s_con = nm_connection_get_setting_connection(connection);
+    /* *** errors above here should be always fatal, below NORMALIZABLE_ERROR *** */
+
+    s_con = connection ? nm_connection_get_setting_connection(connection) : NULL;
+
     if (s_con) {
         const char *interface_name = nm_setting_connection_get_interface_name(s_con);
 
@@ -287,13 +289,11 @@ verify(NMSetting *setting, NMConnection *connection, GError **error)
                                "%s.%s: ",
                                NM_SETTING_CONNECTION_SETTING_NAME,
                                NM_SETTING_CONNECTION_INTERFACE_NAME);
-                return FALSE;
+                return NM_SETTING_VERIFY_NORMALIZABLE_ERROR;
             }
         }
     }
 
-    /* *** errors above here should be always fatal, below NORMALIZABLE_ERROR *** */
-
     if (priv->mtu > NM_INFINIBAND_MAX_MTU) {
         /* Traditionally, MTU for "datagram" mode was limited to 2044
          * and for "connected" mode it was 65520.
diff --git a/src/libnm-core-impl/tests/test-general.c b/src/libnm-core-impl/tests/test-general.c
index 8a98265abb..78ef9dcdac 100644
--- a/src/libnm-core-impl/tests/test-general.c
+++ b/src/libnm-core-impl/tests/test-general.c
@@ -6156,16 +6156,17 @@ test_connection_normalize_slave_type_2(void)
 }
 
 static void
-test_connection_normalize_infiniband_mtu(void)
+test_connection_normalize_infiniband(void)
 {
     gs_unref_object NMConnection *con = NULL;
     NMSettingInfiniband          *s_infini;
+    NMSettingConnection          *s_con;
     guint                         mtu_regular = nmtst_rand_select(2044, 2045, 65520);
 
-    con = nmtst_create_minimal_connection("test_connection_normalize_infiniband_mtu",
+    con = nmtst_create_minimal_connection("test_connection_normalize_infiniband",
                                           NULL,
                                           NM_SETTING_INFINIBAND_SETTING_NAME,
-                                          NULL);
+                                          &s_con);
 
     s_infini = nm_connection_get_setting_infiniband(con);
     g_object_set(s_infini, NM_SETTING_INFINIBAND_TRANSPORT_MODE, "connected", NULL);
@@ -6213,6 +6214,25 @@ test_connection_normalize_infiniband_mtu(void)
                                                          NM_CONNECTION_ERROR_INVALID_PROPERTY);
     nmtst_connection_normalize(con);
     g_assert_cmpint(65520, ==, nm_setting_infiniband_get_mtu(s_infini));
+
+    g_object_set(s_infini,
+                 NM_SETTING_INFINIBAND_PARENT,
+                 "foo",
+                 NM_SETTING_INFINIBAND_P_KEY,
+                 0x005c,
+                 NULL);
+    nmtst_assert_connection_verifies_without_normalization(con);
+
+    g_object_set(s_con, NM_SETTING_CONNECTION_INTERFACE_NAME, "foo.005c", NULL);
+    nmtst_assert_connection_verifies_without_normalization(con);
+
+    g_object_set(s_con, NM_SETTING_CONNECTION_INTERFACE_NAME, "foo", NULL);
+    nmtst_assert_connection_verifies_after_normalization(con,
+                                                         NM_CONNECTION_ERROR,
+                                                         NM_CONNECTION_ERROR_INVALID_PROPERTY);
+
+    nmtst_connection_normalize(con);
+    g_assert_cmpstr(nm_connection_get_interface_name(con), ==, "foo.005c");
 }
 
 static void
@@ -11511,8 +11531,8 @@ main(int argc, char **argv)
                     test_connection_normalize_slave_type_1);
     g_test_add_func("/core/general/test_connection_normalize_slave_type_2",
                     test_connection_normalize_slave_type_2);
-    g_test_add_func("/core/general/test_connection_normalize_infiniband_mtu",
-                    test_connection_normalize_infiniband_mtu);
+    g_test_add_func("/core/general/test_connection_normalize_infiniband",
+                    test_connection_normalize_infiniband);
     g_test_add_func("/core/general/test_connection_normalize_gateway_never_default",
                     test_connection_normalize_gateway_never_default);
     g_test_add_func("/core/general/test_connection_normalize_may_fail",