core: simplify hardware address reading

We no longer need a class method for reading the hardware address
length, for a couple reasons:

1) Using the IP interface hardware address for IP operations now makes
NMDevice's priv->hw_addr_len constant.  So there's no reason to re-read
it all the time.

2) get_hw_address_length() is now only used for determining whether the
hardware address is permanent, and that only mattered for Bluetooth.
Since Bluetooth interfaces have a bogus interface name, they will never
have a valid ifindex, and thus nm_platform_link_get_address() would be
useless.  So instead of using the 'permanent' stuff, just don't bother
updating the hardware address if the NMDevice's ifindex isn't valid,
and let subclasses pass the initial hardware address at device creation.

This also works correctly for NMDevice classes that previously
implemented get_hw_address_length() like ADSL and WWAN, since those
too will never have a valid ifindex or a valid hardware address.

3) Reading the device's hardware address length just ended up calling
nm_platform_link_get_address() for most devices anyway, so
nm_device_update_hw_address() would effectively read the link address
twice (once to read the length, the second time to read the actual
address).  Let's just read the address once.
This commit is contained in:
Dan Williams 2014-07-14 17:59:45 -05:00
parent ac4fafe7a4
commit 27f91d054c
6 changed files with 43 additions and 120 deletions

View file

@ -52,10 +52,6 @@ constructor (GType type,
static void
constructed (GObject *object)
{
NMDevice *device = NM_DEVICE (object);
nm_device_update_hw_address (device);
g_object_class->constructed (object);
}
@ -71,19 +67,10 @@ finalize (GObject *object)
g_object_class->finalize (object);
}
static guint
get_hw_address_length (NMDevice *dev, gboolean *out_permanent)
{
if (out_permanent)
*out_permanent = TRUE;
return ETH_ALEN;
}
static void
nm_test_device_class_init (NMTestDeviceClass *klass)
{
GObjectClass *object_class = G_OBJECT_CLASS (klass);
NMDeviceClass *device_class = NM_DEVICE_CLASS (klass);
g_object_class = g_type_class_peek (G_TYPE_OBJECT);
@ -91,8 +78,6 @@ nm_test_device_class_init (NMTestDeviceClass *klass)
object_class->constructed = constructed;
object_class->dispose = dispose;
object_class->finalize = finalize;
device_class->get_hw_address_length = get_hw_address_length;
}
NMDevice *

View file

@ -483,12 +483,6 @@ deactivate (NMDevice *device)
/**************************************************************/
static guint
get_hw_address_length (NMDevice *device, gboolean *out_permanent)
{
return 0;
}
static gboolean
carrier_update_cb (gpointer user_data)
{
@ -621,7 +615,6 @@ nm_device_adsl_class_init (NMDeviceAdslClass *klass)
parent_class->check_connection_compatible = check_connection_compatible;
parent_class->complete_connection = complete_connection;
parent_class->get_hw_address_length = get_hw_address_length;
parent_class->act_stage2_config = act_stage2_config;
parent_class->act_stage3_ip4_config_start = act_stage3_ip4_config_start;
parent_class->deactivate = deactivate;

View file

@ -111,15 +111,6 @@ guint32 nm_device_bt_get_capabilities (NMDeviceBt *self)
return NM_DEVICE_BT_GET_PRIVATE (self)->capabilities;
}
static guint
get_hw_address_length (NMDevice *device, gboolean *out_permanent)
{
/* HW address is the Bluetooth HW address of the remote device */
if (out_permanent)
*out_permanent = TRUE; /* the bdaddr of the remote device will never change */
return ETH_ALEN;
}
static guint32
get_connection_bt_type (NMConnection *connection)
{
@ -1207,7 +1198,6 @@ nm_device_bt_class_init (NMDeviceBtClass *klass)
object_class->dispose = dispose;
object_class->finalize = finalize;
device_class->get_hw_address_length = get_hw_address_length;
device_class->can_auto_connect = can_auto_connect;
device_class->deactivate = deactivate;
device_class->act_stage2_config = act_stage2_config;

View file

@ -324,7 +324,7 @@ static void _set_state_full (NMDevice *device,
NMDeviceStateReason reason,
gboolean quitting);
static gboolean nm_device_update_hw_address (NMDevice *dev);
static void nm_device_update_hw_address (NMDevice *dev);
/***********************************************************/
@ -6934,12 +6934,6 @@ nm_device_get_state (NMDevice *device)
/***********************************************************/
/* NMConfigDevice interface related stuff */
static guint
nm_device_get_hw_address_length (NMDevice *dev, gboolean *out_permanent)
{
return NM_DEVICE_GET_CLASS (dev)->get_hw_address_length (dev, out_permanent);
}
const guint8 *
nm_device_get_hw_address (NMDevice *dev, guint *out_len)
{
@ -6951,68 +6945,46 @@ nm_device_get_hw_address (NMDevice *dev, guint *out_len)
if (out_len)
*out_len = priv->hw_addr_len;
if (priv->hw_addr_len == 0)
return NULL;
else
return priv->hw_addr;
return priv->hw_addr_len ? priv->hw_addr : NULL;
}
static gboolean
static void
nm_device_update_hw_address (NMDevice *dev)
{
NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (dev);
gboolean changed = FALSE, permanent = FALSE;
int ifindex = nm_device_get_ifindex (dev);
const char *iface = nm_device_get_iface (dev);
const guint8 *hwaddr;
gsize hwaddrlen = 0;
priv->hw_addr_len = nm_device_get_hw_address_length (dev, &permanent);
if (ifindex <= 0)
return;
/* If the address can't be changed, don't bother trying */
if (permanent)
return FALSE;
hwaddr = nm_platform_link_get_address (ifindex, &hwaddrlen);
g_assert (hwaddrlen <= sizeof (priv->hw_addr));
if (hwaddrlen) {
if (hwaddrlen != priv->hw_addr_len || memcmp (priv->hw_addr, hwaddr, hwaddrlen)) {
memcpy (priv->hw_addr, hwaddr, hwaddrlen);
if (priv->hw_addr_len) {
int ifindex = nm_device_get_ifindex (dev);
gsize addrlen;
const guint8 *binaddr;
if (nm_logging_enabled (LOGL_DEBUG, LOGD_HW | LOGD_DEVICE)) {
char *addrstr = nm_utils_hwaddr_ntoa_len (hwaddr, hwaddrlen);
g_return_val_if_fail (ifindex > 0, FALSE);
binaddr = nm_platform_link_get_address (ifindex, &addrlen);
if (addrlen != priv->hw_addr_len) {
nm_log_err (LOGD_HW | LOGD_DEVICE,
"(%s): hardware address is wrong length (got %zd, expected %d)",
nm_device_get_iface (dev), addrlen, priv->hw_addr_len);
} else {
changed = !!memcmp (priv->hw_addr, binaddr, addrlen);
if (changed) {
char *addrstr = nm_utils_hwaddr_ntoa_len (binaddr, priv->hw_addr_len);
memcpy (priv->hw_addr, binaddr, addrlen);
nm_log_dbg (LOGD_HW | LOGD_DEVICE,
"(%s): hardware address is %s",
nm_device_get_iface (dev), addrstr);
nm_log_dbg (LOGD_HW | LOGD_DEVICE, "(%s): hardware address now %s", iface, addrstr);
g_free (addrstr);
g_object_notify (G_OBJECT (dev), NM_DEVICE_HW_ADDRESS);
}
g_object_notify (G_OBJECT (dev), NM_DEVICE_HW_ADDRESS);
}
} else {
int i;
/* hw_addr_len is now 0; see if hw_addr was already empty */
for (i = 0; i < sizeof (priv->hw_addr) && !changed; i++) {
if (priv->hw_addr[i])
changed = TRUE;
}
if (changed) {
/* Invalid or no hardware address */
if (priv->hw_addr_len != 0) {
memset (priv->hw_addr, 0, sizeof (priv->hw_addr));
nm_log_dbg (LOGD_HW | LOGD_DEVICE,
"(%s): previous hardware address is no longer valid",
nm_device_get_iface (dev));
iface);
g_object_notify (G_OBJECT (dev), NM_DEVICE_HW_ADDRESS);
}
}
return changed;
priv->hw_addr_len = hwaddrlen;
}
gboolean
@ -7118,17 +7090,6 @@ spec_match_list (NMDevice *device, const GSList *specs)
return matched;
}
static guint
get_hw_address_length (NMDevice *dev, gboolean *out_permanent)
{
size_t len;
if (nm_platform_link_get_address (nm_device_get_ifindex (dev), &len))
return len;
else
return 0;
}
/***********************************************************/
#define DEFAULT_AUTOCONNECT TRUE
@ -7375,9 +7336,9 @@ set_property (GObject *object, guint prop_id,
{
NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (object);
NMPlatformLink *platform_device;
const char *hw_addr;
guint hw_addr_len;
const char *hw_addr, *p;
guint count;
switch (prop_id) {
case PROP_PLATFORM_DEVICE:
platform_device = g_value_get_pointer (value);
@ -7455,18 +7416,21 @@ set_property (GObject *object, guint prop_id,
priv->is_master = g_value_get_boolean (value);
break;
case PROP_HW_ADDRESS:
hw_addr_len = nm_device_get_hw_address_length (NM_DEVICE (object), NULL);
g_return_if_fail (hw_addr_len <= NM_UTILS_HWADDR_LEN_MAX);
priv->hw_addr_len = hw_addr_len;
/* construct only */
p = hw_addr = g_value_get_string (value);
hw_addr = g_value_get_string (value);
if (!hw_addr)
break;
if (priv->hw_addr_len == 0) {
g_warn_if_fail (*hw_addr == '\0');
/* Hardware address length is the number of ':' plus 1 */
count = 1;
while (p && *p) {
if (*p++ == ':')
count++;
}
if (count < ETH_ALEN || count > NM_UTILS_HWADDR_LEN_MAX) {
g_warn_if_fail (!hw_addr || *hw_addr == '\0');
break;
}
priv->hw_addr_len = count;
if (!nm_utils_hwaddr_aton_len (hw_addr, priv->hw_addr, priv->hw_addr_len)) {
g_warning ("Could not parse hw-address '%s'", hw_addr);
memset (priv->hw_addr, 0, sizeof (priv->hw_addr));
@ -7647,7 +7611,6 @@ nm_device_class_init (NMDeviceClass *klass)
klass->bring_up = bring_up;
klass->take_down = take_down;
klass->carrier_changed = carrier_changed;
klass->get_hw_address_length = get_hw_address_length;
/* Properties */
g_object_class_install_property

View file

@ -55,12 +55,13 @@
#define NM_DEVICE_AVAILABLE_CONNECTIONS "available-connections"
#define NM_DEVICE_PHYSICAL_PORT_ID "physical-port-id"
#define NM_DEVICE_MTU "mtu"
#define NM_DEVICE_TYPE_DESC "type-desc" /* Internal only */
#define NM_DEVICE_RFKILL_TYPE "rfkill-type" /* Internal only */
#define NM_DEVICE_IFINDEX "ifindex" /* Internal only */
#define NM_DEVICE_IS_MASTER "is-master" /* Internal only */
#define NM_DEVICE_MASTER "master" /* Internal only */
#define NM_DEVICE_HW_ADDRESS "hw-address" /* Internal only */
#define NM_DEVICE_HW_ADDRESS "hw-address"
#define NM_DEVICE_TYPE_DESC "type-desc" /* Internal only */
#define NM_DEVICE_RFKILL_TYPE "rfkill-type" /* Internal only */
#define NM_DEVICE_IFINDEX "ifindex" /* Internal only */
#define NM_DEVICE_IS_MASTER "is-master" /* Internal only */
#define NM_DEVICE_MASTER "master" /* Internal only */
#define NM_DEVICE_HAS_PENDING_ACTION "has-pending-action" /* Internal only */
/* Internal signals */
@ -115,10 +116,8 @@ typedef struct {
/* Carrier state (IFF_LOWER_UP) */
void (*carrier_changed) (NMDevice *, gboolean carrier);
void (* update_hw_address) (NMDevice *self);
void (* update_permanent_hw_address) (NMDevice *self);
void (* update_initial_hw_address) (NMDevice *self);
guint (* get_hw_address_length) (NMDevice *self, gboolean *out_permanent);
guint32 (* get_generic_capabilities) (NMDevice *self);

View file

@ -290,12 +290,6 @@ device_state_changed (NMDevice *device,
}
}
static guint
get_hw_address_length (NMDevice *device, gboolean *out_permanent)
{
return 0;
}
static gboolean
check_connection_compatible (NMDevice *device, NMConnection *connection)
{
@ -596,7 +590,6 @@ nm_device_modem_class_init (NMDeviceModemClass *mclass)
object_class->get_property = get_property;
object_class->set_property = set_property;
device_class->get_hw_address_length = get_hw_address_length;
device_class->check_connection_compatible = check_connection_compatible;
device_class->check_connection_available = check_connection_available;
device_class->complete_connection = complete_connection;