From a9bc3eecc66abe6befc60dba97543b2a8ef81bec Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 9 Dec 2020 10:41:05 +0100 Subject: [PATCH 1/2] libnm: move detection/normalization of "connection.slave-type" to a separate function We allow normalizing the slave-type, but sometimes, we may want to validate a profile according to the set slave-type. For example, a "ovs-external-ids" setting should only be allowed for "connection.slave-type=ovs-interface". But during verify, the slave-type may be missing and not yet normalized. We need to be able to obtain the actually used slave-type. --- libnm-core/nm-connection-private.h | 8 ++ libnm-core/nm-setting-connection.c | 160 +++++++++++++++++------------ 2 files changed, 104 insertions(+), 64 deletions(-) diff --git a/libnm-core/nm-connection-private.h b/libnm-core/nm-connection-private.h index 4c05c36b92..4824d5ecce 100644 --- a/libnm-core/nm-connection-private.h +++ b/libnm-core/nm-connection-private.h @@ -17,6 +17,14 @@ NMSetting *_nm_connection_find_base_type_setting(NMConnection *connection); const char *_nm_connection_detect_slave_type(NMConnection *connection, NMSetting **out_s_port); +gboolean _nm_connection_detect_slave_type_full(NMSettingConnection *s_con, + NMConnection * connection, + const char ** out_slave_type, + const char ** out_normerr_slave_setting_type, + const char ** out_normerr_missing_slave_type, + const char **out_normerr_missing_slave_type_port, + GError ** error); + const char *_nm_connection_detect_bluetooth_type(NMConnection *self); gboolean _nm_connection_verify_required_interface_name(NMConnection *connection, GError **error); diff --git a/libnm-core/nm-setting-connection.c b/libnm-core/nm-setting-connection.c index f82309279f..87268ad1f2 100644 --- a/libnm-core/nm-setting-connection.c +++ b/libnm-core/nm-setting-connection.c @@ -969,12 +969,95 @@ _set_error_missing_base_setting(GError **error, const char *type) g_prefix_error(error, "%s: ", type); } +gboolean +_nm_connection_detect_slave_type_full(NMSettingConnection *s_con, + NMConnection * connection, + const char ** out_slave_type, + const char ** out_normerr_slave_setting_type, + const char ** out_normerr_missing_slave_type, + const char ** out_normerr_missing_slave_type_port, + GError ** error) +{ + NMSettingConnectionPrivate *priv = NM_SETTING_CONNECTION_GET_PRIVATE(s_con); + gboolean is_slave; + const char * slave_setting_type; + const char * slave_type; + const char * normerr_slave_setting_type = NULL; + const char * normerr_missing_slave_type = NULL; + const char * normerr_missing_slave_type_port = NULL; + + is_slave = FALSE; + slave_setting_type = NULL; + slave_type = priv->slave_type; + if (slave_type) { + is_slave = _nm_setting_slave_type_is_valid(slave_type, &slave_setting_type); + if (!is_slave) { + g_set_error(error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + _("Unknown slave type '%s'"), + slave_type); + g_prefix_error(error, + "%s.%s: ", + NM_SETTING_CONNECTION_SETTING_NAME, + NM_SETTING_CONNECTION_SLAVE_TYPE); + return FALSE; + } + } + + if (is_slave) { + if (!priv->master) { + g_set_error(error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_MISSING_PROPERTY, + _("Slave connections need a valid '%s' property"), + NM_SETTING_CONNECTION_MASTER); + g_prefix_error(error, + "%s.%s: ", + NM_SETTING_CONNECTION_SETTING_NAME, + NM_SETTING_CONNECTION_MASTER); + return FALSE; + } + if (slave_setting_type && connection + && !nm_connection_get_setting_by_name(connection, slave_setting_type)) + normerr_slave_setting_type = slave_setting_type; + } else { + nm_assert(!slave_type); + if (priv->master) { + NMSetting *s_port; + + if (connection + && (slave_type = _nm_connection_detect_slave_type(connection, &s_port))) { + normerr_missing_slave_type = slave_type; + normerr_missing_slave_type_port = nm_setting_get_name(s_port); + } else { + g_set_error(error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_MISSING_PROPERTY, + _("Cannot set '%s' without '%s'"), + NM_SETTING_CONNECTION_MASTER, + NM_SETTING_CONNECTION_SLAVE_TYPE); + g_prefix_error(error, + "%s.%s: ", + NM_SETTING_CONNECTION_SETTING_NAME, + NM_SETTING_CONNECTION_SLAVE_TYPE); + return FALSE; + } + } + } + + NM_SET_OUT(out_slave_type, slave_type); + NM_SET_OUT(out_normerr_slave_setting_type, normerr_slave_setting_type); + NM_SET_OUT(out_normerr_missing_slave_type, normerr_missing_slave_type); + NM_SET_OUT(out_normerr_missing_slave_type_port, normerr_missing_slave_type_port); + return TRUE; +} + static gboolean verify(NMSetting *setting, NMConnection *connection, GError **error) { - NMSettingConnectionPrivate *priv = NM_SETTING_CONNECTION_GET_PRIVATE(setting); - gboolean is_slave; - const char * slave_setting_type; + NMSettingConnection * self = NM_SETTING_CONNECTION(setting); + NMSettingConnectionPrivate *priv = NM_SETTING_CONNECTION_GET_PRIVATE(self); NMSetting * normerr_base_type = NULL; const char * type; const char * slave_type; @@ -1146,68 +1229,17 @@ verify(NMSetting *setting, NMConnection *connection, GError **error) } after_interface_name: - is_slave = FALSE; - slave_setting_type = NULL; - slave_type = priv->slave_type; - if (slave_type) { - is_slave = _nm_setting_slave_type_is_valid(slave_type, &slave_setting_type); - if (!is_slave) { - g_set_error(error, - NM_CONNECTION_ERROR, - NM_CONNECTION_ERROR_INVALID_PROPERTY, - _("Unknown slave type '%s'"), - slave_type); - g_prefix_error(error, - "%s.%s: ", - NM_SETTING_CONNECTION_SETTING_NAME, - NM_SETTING_CONNECTION_SLAVE_TYPE); - return FALSE; - } - } + if (!_nm_connection_detect_slave_type_full(self, + connection, + &slave_type, + &normerr_slave_setting_type, + &normerr_missing_slave_type, + &normerr_missing_slave_type_port, + error)) + return FALSE; - if (is_slave) { - if (!priv->master) { - g_set_error(error, - NM_CONNECTION_ERROR, - NM_CONNECTION_ERROR_MISSING_PROPERTY, - _("Slave connections need a valid '%s' property"), - NM_SETTING_CONNECTION_MASTER); - g_prefix_error(error, - "%s.%s: ", - NM_SETTING_CONNECTION_SETTING_NAME, - NM_SETTING_CONNECTION_MASTER); - return FALSE; - } - if (slave_setting_type && connection - && !nm_connection_get_setting_by_name(connection, slave_setting_type)) - normerr_slave_setting_type = slave_setting_type; - } else { - nm_assert(!slave_type); - if (priv->master) { - NMSetting *s_port; - - if (connection - && (slave_type = _nm_connection_detect_slave_type(connection, &s_port))) { - normerr_missing_slave_type = slave_type; - normerr_missing_slave_type_port = nm_setting_get_name(s_port); - } else { - g_set_error(error, - NM_CONNECTION_ERROR, - NM_CONNECTION_ERROR_MISSING_PROPERTY, - _("Cannot set '%s' without '%s'"), - NM_SETTING_CONNECTION_MASTER, - NM_SETTING_CONNECTION_SLAVE_TYPE); - g_prefix_error(error, - "%s.%s: ", - NM_SETTING_CONNECTION_SETTING_NAME, - NM_SETTING_CONNECTION_SLAVE_TYPE); - return FALSE; - } - } - } - - if (strcmp(type, NM_SETTING_OVS_PORT_SETTING_NAME) == 0 && slave_type - && strcmp(slave_type, NM_SETTING_OVS_BRIDGE_SETTING_NAME) != 0) { + if (nm_streq(type, NM_SETTING_OVS_PORT_SETTING_NAME) && slave_type + && !nm_streq(slave_type, NM_SETTING_OVS_BRIDGE_SETTING_NAME)) { g_set_error(error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_MISSING_PROPERTY, From 9cc242596db21f37609e6b396dc852d6a0651834 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 9 Dec 2020 10:46:16 +0100 Subject: [PATCH 2/2] libnm: allow OVS external-ids also for system interface Note that reapply currently does not work for OVS system interface. That is, because the code does not make it easy to implement that. --- libnm-core/nm-setting-ovs-external-ids.c | 39 ++++++++++++++++-------- src/devices/nm-device.c | 5 ++- 2 files changed, 31 insertions(+), 13 deletions(-) diff --git a/libnm-core/nm-setting-ovs-external-ids.c b/libnm-core/nm-setting-ovs-external-ids.c index 9371c281b5..7dc8f78ad2 100644 --- a/libnm-core/nm-setting-ovs-external-ids.c +++ b/libnm-core/nm-setting-ovs-external-ids.c @@ -345,7 +345,9 @@ verify(NMSetting *setting, NMConnection *connection, GError **error) } if (connection) { - const char *type; + NMSettingConnection *s_con; + const char * type; + const char * slave_type; type = nm_connection_get_connection_type(connection); if (!type) { @@ -355,18 +357,31 @@ verify(NMSetting *setting, NMConnection *connection, GError **error) if (s_base) type = nm_setting_get_name(s_base); } - if (!NM_IN_STRSET(type, - NM_SETTING_OVS_BRIDGE_SETTING_NAME, - NM_SETTING_OVS_PORT_SETTING_NAME, - NM_SETTING_OVS_INTERFACE_SETTING_NAME)) { - g_set_error_literal(error, - NM_CONNECTION_ERROR, - NM_CONNECTION_ERROR_INVALID_PROPERTY, - _("OVS external IDs can only be added to a profile of type OVS " - "bridge/port/interface")); - return FALSE; - } + if (NM_IN_STRSET(type, + NM_SETTING_OVS_BRIDGE_SETTING_NAME, + NM_SETTING_OVS_PORT_SETTING_NAME, + NM_SETTING_OVS_INTERFACE_SETTING_NAME)) + goto connection_type_is_good; + + if ((s_con = nm_connection_get_setting_connection(connection)) + && _nm_connection_detect_slave_type_full(s_con, + connection, + &slave_type, + NULL, + NULL, + NULL, + NULL) + && nm_streq0(slave_type, NM_SETTING_OVS_PORT_SETTING_NAME)) + goto connection_type_is_good; + + g_set_error_literal(error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + _("OVS external IDs can only be added to a profile of type OVS " + "bridge/port/interface or to OVS system interface")); + return FALSE; } +connection_type_is_good: return TRUE; } diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index ec01d3af6b..38be8c8002 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -12442,8 +12442,11 @@ can_reapply_change(NMDevice * self, } if (nm_streq(setting_name, NM_SETTING_OVS_EXTERNAL_IDS_SETTING_NAME) - && NM_DEVICE_GET_CLASS(self)->can_reapply_change_ovs_external_ids) + && NM_DEVICE_GET_CLASS(self)->can_reapply_change_ovs_external_ids) { + /* TODO: this means, you cannot reapply changes to the external-ids for + * OVS system interfaces. */ return TRUE; + } out_fail: g_set_error(error,