libnm-core: fail verify() for NMSettingVlan for invalid vlan id

Point in case:

    # ip link add link dummy0 name dummy0.vlan type vlan id 4095
    RTNETLINK answers: Numerical result out of range

This potentially causes existing (invalid) connections to disappear
as they now fail verification.

Instead of adjusting the range of the GObject property
NM_SETTING_VLAN_ID, reject it during vlan. This is a bit more
forgiving to an older client that isn't aware of this new restriction,
so he can first set the value without raising a critical warning.

diff --git a/libnm-core/nm-setting-vlan.c b/libnm-core/nm-setting-vlan.c
index 8428a36..92a052e 100644 (file)
--- a/libnm-core/nm-setting-vlan.c
+++ b/libnm-core/nm-setting-vlan.c
@@ -666,6 +666,15 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
                }
        }
 
+       if (priv->id >= 4095) {
+               g_set_error (error,
+                            NM_CONNECTION_ERROR,
+                            NM_CONNECTION_ERROR_INVALID_PROPERTY,
+                            _("the vlan id must be in range 0-4094 but is %u"),
+                            priv->id);
+               g_prefix_error (error, "%s.%s: ", NM_SETTING_VLAN_SETTING_NAME, NM_SETTING_VLAN_ID);
+       }
+
        if (priv->flags & ~NM_VLAN_FLAGS_ALL) {
                g_set_error_literal (error,
                                     NM_CONNECTION_ERROR,
@@ -851,7 +860,7 @@ nm_setting_vlan_class_init (NMSettingVlanClass *setting_class)
         * NMSettingVlan:id:
         *
         * The VLAN identifier that the interface created by this connection should
-        * be assigned.
+        * be assigned. The valid range is from 0 to 4094, without the reserved id 4095.
         **/
        /* ---ifcfg-rh---
         * property: id

diff --git a/libnm-util/nm-setting-vlan.c b/libnm-util/nm-setting-vlan.c
index 310a7da..d101f45 100644 (file)
--- a/libnm-util/nm-setting-vlan.c
+++ b/libnm-util/nm-setting-vlan.c
@@ -593,6 +593,16 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
                return FALSE;
        }
 
+       if (priv->id >= 4095) {
+               g_set_error (error,
+                            NM_SETTING_VLAN_ERROR,
+                            NM_SETTING_VLAN_ERROR_INVALID_PROPERTY,
+                            _("the vlan id must be in range 0-4094 but is %u"),
+                            priv->id);
+               g_prefix_error (error, "%s.%s: ", NM_SETTING_VLAN_SETTING_NAME, NM_SETTING_VLAN_ID);
+               return FALSE;
+       }
+
        /* If interface_name is specified, it must be a valid interface name. We
         * don't check that it matches parent and/or id, because we allow
         * renaming vlans to arbitrary names.

diff --git a/src/NetworkManagerUtils.c b/src/NetworkManagerUtils.c
index dd7467c..49fb8cb 100644 (file)
--- a/src/NetworkManagerUtils.c
+++ b/src/NetworkManagerUtils.c
@@ -2147,10 +2147,9 @@ nm_utils_new_vlan_name (const char *parent_iface, guint32 vlan_id)
 
        g_return_val_if_fail (parent_iface && *parent_iface, NULL);
 
-       if (vlan_id < 10) {
-               g_return_val_if_fail (vlan_id > 0, NULL);
+       if (vlan_id < 10)
                id_len = 2;
-       } else if (vlan_id < 100)
+       else if (vlan_id < 100)
                id_len = 3;
        else if (vlan_id < 1000)
                id_len = 4;

diff --git a/src/tests/test-general-with-expect.c b/src/tests/test-general-with-expect.c
index 31afd62..91f3f33 100644 (file)
--- a/src/tests/test-general-with-expect.c
+++ b/src/tests/test-general-with-expect.c
@@ -879,8 +879,8 @@ test_nm_utils_new_vlan_name (void)
                        gs_free char *vlan_id_s = NULL;
                        guint vlan_id;
 
-                       /* Create a random VLAN id between 1 and 4094 */
-                       vlan_id = 1 + nmtst_get_rand_int () % 4094;
+                       /* Create a random VLAN id between 0 and 4094 */
+                       vlan_id = nmtst_get_rand_int () % 4095;
 
                        vlan_id_s = g_strdup_printf (".%d", vlan_id);