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 4610fd67e6)
This commit is contained in:
Thomas Haller 2023-05-24 17:32:19 +02:00
parent 2945254e29
commit 30f0429d12
No known key found for this signature in database
GPG key ID: 29C2366E4DFC5728
3 changed files with 63 additions and 20 deletions

View file

@ -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);

View file

@ -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.

View file

@ -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",