all: standardize on NMSettingWired:mac-address for all VLANs

Currently, ethernet-based VLANs can specify the hardware address of
the parent device (and, in theory, the cloned hardware address and MTU
of the VLAN device) by using an NMSettingWired in addition to the
NMSettingVlan.

The theory was that non-ethernet-based VLANs, when we eventually
supported them, would likewise use the setting type corresponding to
their parent device. However, this turns out to be both complicated
(the settings plugins and connection editor would have a
hard-to-impossible time figuring out which setting type to use in some
cases) and incorrect (for most L2 settings [eg, BSSID, bond mode,
etc], the VLAN can't have its own values separate from the parent
device).

What we should have done was just have :mac-address,
:cloned-mac-address, and :mtu properties on NMSettingVlan. However, at
this point, for backward-compatibility, we will just stick with using
a combination of NMSettingVlan and NMSettingWired, but we will use
NMSettingWired regardless of the underlying hardware type.
This commit is contained in:
Dan Winship 2013-09-09 09:40:40 -04:00
parent 2688ae4950
commit 066b592241
5 changed files with 84 additions and 43 deletions

View file

@ -156,7 +156,10 @@ connection_compatible (NMDevice *device, NMConnection *connection, GError **erro
{
NMSettingConnection *s_con;
NMSettingVlan *s_vlan;
NMSettingWired *s_wired;
const char *ctype, *dev_iface_name, *vlan_iface_name;
const GByteArray *mac_address;
char *mac_address_str;
s_con = nm_connection_get_setting_connection (connection);
g_assert (s_con);
@ -189,6 +192,21 @@ connection_compatible (NMDevice *device, NMConnection *connection, GError **erro
return FALSE;
}
s_wired = nm_connection_get_setting_wired (connection);
if (s_wired)
mac_address = nm_setting_wired_get_mac_address (s_wired);
else
mac_address = NULL;
if (mac_address) {
mac_address_str = nm_utils_hwaddr_ntoa_len (mac_address->data, mac_address->len);
if (!g_strcmp0 (mac_address_str, NM_DEVICE_VLAN_GET_PRIVATE (device)->hw_address)) {
g_set_error (error, NM_DEVICE_VLAN_ERROR, NM_DEVICE_VLAN_ERROR_MAC_MISMATCH,
"The hardware address of the device and the connection didn't match.");
g_free (mac_address_str);
}
g_free (mac_address_str);
}
return NM_DEVICE_CLASS (nm_device_vlan_parent_class)->connection_compatible (device, connection, error);
}

View file

@ -41,6 +41,7 @@ G_BEGIN_DECLS
* @NM_DEVICE_VLAN_ERROR_INVALID_VLAN_CONNECTION: the VLAN connection was invalid
* @NM_DEVICE_VLAN_ERROR_ID_MISMATCH: the VLAN identifiers of the connection and the device mismatched
* @NM_DEVICE_VLAN_ERROR_INTERFACE_MISMATCH: the interfaces of the connection and the device mismatched
* @NM_DEVICE_VLAN_ERROR_MAC_MISMATCH: the MACs of the connection and the device mismatched
*/
typedef enum {
NM_DEVICE_VLAN_ERROR_UNKNOWN = 0, /*< nick=UnknownError >*/
@ -48,6 +49,7 @@ typedef enum {
NM_DEVICE_VLAN_ERROR_INVALID_VLAN_CONNECTION, /*< nick=InvalidVlanConnection >*/
NM_DEVICE_VLAN_ERROR_ID_MISMATCH, /*< nick=IdMismatch >*/
NM_DEVICE_VLAN_ERROR_INTERFACE_MISMATCH, /*< nick=InterfaceMismatch >*/
NM_DEVICE_VLAN_ERROR_MAC_MISMATCH, /*< nick=MacMismatch >*/
} NMDeviceVlanError;
#define NM_DEVICE_VLAN_ERROR nm_device_vlan_error_quark ()

View file

@ -681,8 +681,8 @@ nm_setting_vlan_class_init (NMSettingVlanClass *setting_class)
* If given, specifies the kernel name of the VLAN interface. If not given,
* a default name will be constructed from the interface described by the
* parent interface and the #NMSettingVlan:id , ex 'eth2.1'. The parent
* interface may be given by the #NMSettingVlan:parent property or by a
* hardware address property, eg #NMSettingWired:mac-address.
* interface may be given by the #NMSettingVlan:parent property or by the
* #NMSettingWired:mac-address property of an #NMSettingWired.
**/
g_object_class_install_property
(object_class, PROP_IFACE_NAME,
@ -693,9 +693,8 @@ nm_setting_vlan_class_init (NMSettingVlanClass *setting_class)
"constructed from the interface described by the "
"parent interface and the 'id' property, ex "
"'eth2.1'. The parent interface may be given by "
"the 'parent' property or by a hardware address "
"property, eg the 'wired' settings' 'mac-address' "
"property.",
"the 'parent' property or by the 'mac-address' "
"property of a 'wired' setting.",
NULL,
G_PARAM_READWRITE | G_PARAM_CONSTRUCT | NM_SETTING_PARAM_SERIALIZE));
@ -704,8 +703,8 @@ nm_setting_vlan_class_init (NMSettingVlanClass *setting_class)
*
* If given, specifies the parent interface name or parent connection UUID
* from which this VLAN interface should be created. If this property is
* not specified, the connection must contain a hardware address in a
* hardware-specific setting, like #NMSettingWired:mac-address.
* not specified, the connection must contain a #NMSettingWired:mac-address
* in an #NMSettingWired setting.
**/
g_object_class_install_property
(object_class, PROP_PARENT,
@ -715,8 +714,7 @@ nm_setting_vlan_class_init (NMSettingVlanClass *setting_class)
"parent connection UUID from which this VLAN "
"interface should be created. If this property is "
"not specified, the connection must contain a "
"hardware address in a hardware-specific setting, "
"like the 'wired' settings' 'mac-address' property.",
"'wired' setting with a 'mac-address' property.",
NULL,
G_PARAM_READWRITE | G_PARAM_CONSTRUCT | NM_SETTING_PARAM_SERIALIZE));

View file

@ -144,6 +144,28 @@ match_parent (NMDeviceVlan *self, const char *parent, GError **error)
return TRUE;
}
static gboolean
match_hwaddr (NMDevice *device, NMConnection *connection, gboolean fail_if_no_hwaddr)
{
NMSettingWired *s_wired;
const GByteArray *mac;
const guint8 *device_mac;
guint device_mac_len;
s_wired = nm_connection_get_setting_wired (connection);
if (!s_wired)
return !fail_if_no_hwaddr;
mac = nm_setting_wired_get_mac_address (s_wired);
if (!mac)
return !fail_if_no_hwaddr;
device_mac = nm_device_get_hw_address (device, &device_mac_len);
return ( mac->len == device_mac_len
&& memcmp (mac->data, device_mac, device_mac_len) == 0);
}
static gboolean
check_connection_compatible (NMDevice *device,
NMConnection *connection,
@ -175,8 +197,8 @@ check_connection_compatible (NMDevice *device,
if (!match_parent (NM_DEVICE_VLAN (device), parent, error))
return FALSE;
} else {
/* Parent could be a MAC address in a hardware-specific setting */
if (!nm_device_hwaddr_matches (priv->parent, connection, NULL, 0, TRUE)) {
/* Parent could be a MAC address in an NMSettingWired */
if (!match_hwaddr (device, connection, TRUE)) {
g_set_error (error, NM_VLAN_ERROR, NM_VLAN_ERROR_CONNECTION_INVALID,
"Failed to match the VLAN parent interface via hardware address.");
return FALSE;
@ -206,7 +228,6 @@ complete_connection (NMDevice *device,
const GSList *existing_connections,
GError **error)
{
NMDeviceVlanPrivate *priv = NM_DEVICE_VLAN_GET_PRIVATE (device);
NMSettingVlan *s_vlan;
nm_utils_complete_generic (connection,
@ -226,15 +247,11 @@ complete_connection (NMDevice *device,
/* If there's no VLAN interface, no parent, and no hardware address in the
* settings, then there's not enough information to complete the setting.
*/
if (!nm_setting_vlan_get_parent (s_vlan)) {
if (!nm_device_hwaddr_matches (priv->parent, connection, NULL, 0, TRUE)) {
/* FIXME: put hw_addr into the connection in the appropriate
* hardware-specific setting.
*/
g_set_error_literal (error, NM_VLAN_ERROR, NM_VLAN_ERROR_CONNECTION_INVALID,
"The 'vlan' setting had no interface name, parent, or hardware address.");
return FALSE;
}
if ( !nm_setting_vlan_get_parent (s_vlan)
&& !match_hwaddr (device, connection, TRUE)) {
g_set_error_literal (error, NM_VLAN_ERROR, NM_VLAN_ERROR_CONNECTION_INVALID,
"The 'vlan' setting had no interface name, parent, or hardware address.");
return FALSE;
}
return TRUE;
@ -243,11 +260,8 @@ complete_connection (NMDevice *device,
static gboolean
match_l2_config (NMDevice *device, NMConnection *connection)
{
NMDeviceVlanPrivate *priv = NM_DEVICE_VLAN_GET_PRIVATE (device);
NMSettingVlan *s_vlan;
gboolean fail_if_no_hwaddr = FALSE;
const guint8 *hw_addr;
guint hw_addr_len;
s_vlan = nm_connection_get_setting_vlan (connection);
g_assert (s_vlan);
@ -267,8 +281,7 @@ match_l2_config (NMDevice *device, NMConnection *connection)
* address will be in. The VLAN device shouldn't have to know what kind
* of interface the parent is.
*/
hw_addr = nm_device_get_hw_address (device, &hw_addr_len);
if (!nm_device_hwaddr_matches (priv->parent, connection, hw_addr, hw_addr_len, fail_if_no_hwaddr))
if (!match_hwaddr (device, connection, fail_if_no_hwaddr))
return FALSE;
/* FIXME: any more L2 checks? */

View file

@ -1087,37 +1087,49 @@ pending_activation_destroy (PendingActivation *pending,
/*******************************************************************/
static NMDevice *
get_device_from_hwaddr (NMManager *self, NMConnection *connection)
get_device_from_hwaddr (NMManager *self, const GByteArray *setting_mac)
{
NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self);
const guint8 *device_mac;
guint device_mac_len;
GSList *iter;
if (!setting_mac)
return NULL;
for (iter = priv->devices; iter; iter = g_slist_next (iter)) {
if (nm_device_hwaddr_matches (NM_DEVICE (iter->data), connection, NULL, 0, TRUE))
return iter->data;
NMDevice *device = iter->data;
device_mac = nm_device_get_hw_address (iter->data, &device_mac_len);
if ( setting_mac->len == device_mac_len
&& memcmp (setting_mac->data, device_mac, device_mac_len) == 0)
return device;
}
return NULL;
}
static NMDevice*
static NMDevice *
find_vlan_parent (NMManager *self,
NMConnection *connection,
gboolean check_hwaddr)
NMConnection *connection)
{
NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self);
NMSettingVlan *s_vlan;
NMSettingWired *s_wired;
NMConnection *parent_connection;
const char *parent_iface;
NMDevice *parent = NULL;
const GByteArray *setting_mac;
GSList *iter;
/* The 'parent' property could be either an interface name, a connection
* UUID, or even given by the MAC address of the connection's ethernet
* or WiFi setting.
/* The 'parent' property could be given by an interface name, a
* connection UUID, or the MAC address of an NMSettingWired.
*/
s_vlan = nm_connection_get_setting_vlan (connection);
g_return_val_if_fail (s_vlan != NULL, NULL);
s_wired = nm_connection_get_setting_wired (connection);
setting_mac = s_wired ? nm_setting_wired_get_mac_address (s_wired) : NULL;
parent_iface = nm_setting_vlan_get_parent (s_vlan);
if (parent_iface) {
parent = find_device_by_ip_iface (self, parent_iface);
@ -1142,18 +1154,14 @@ find_vlan_parent (NMManager *self,
}
/* Check the hardware address of the parent connection */
if (check_hwaddr)
return get_device_from_hwaddr (self, parent_connection);
return get_device_from_hwaddr (self, setting_mac);
}
return NULL;
}
}
/* Try the hardware address from the VLAN connection's hardware setting */
if (check_hwaddr)
return get_device_from_hwaddr (self, connection);
return NULL;
return get_device_from_hwaddr (self, setting_mac);
}
static NMDevice *
@ -1163,6 +1171,7 @@ find_infiniband_parent (NMManager *self,
NMSettingInfiniband *s_infiniband;
const char *parent_iface;
NMDevice *parent = NULL;
const GByteArray *setting_mac;
s_infiniband = nm_connection_get_setting_infiniband (connection);
g_return_val_if_fail (s_infiniband != NULL, NULL);
@ -1174,7 +1183,8 @@ find_infiniband_parent (NMManager *self,
return parent;
}
return get_device_from_hwaddr (self, connection);
setting_mac = nm_setting_infiniband_get_mac_address (s_infiniband);
return get_device_from_hwaddr (self, setting_mac);
}
/**
@ -1217,7 +1227,7 @@ get_virtual_iface_name (NMManager *self,
s_vlan = nm_connection_get_setting_vlan (connection);
g_return_val_if_fail (s_vlan != NULL, NULL);
parent = find_vlan_parent (self, connection, TRUE);
parent = find_vlan_parent (self, connection);
if (parent) {
ifname = nm_connection_get_virtual_iface_name (connection);