NetworkManager/src/devices/nm-device-bond.c

679 lines
22 KiB
C
Raw Normal View History

// SPDX-License-Identifier: GPL-2.0+
/*
* Copyright (C) 2011 - 2018 Red Hat, Inc.
*/
#include "nm-default.h"
#include "nm-device-bond.h"
#include <stdlib.h>
#include <net/if.h>
#include "NetworkManagerUtils.h"
#include "nm-device-private.h"
#include "platform/nm-platform.h"
2014-09-08 10:15:02 -05:00
#include "nm-device-factory.h"
#include "nm-core-internal.h"
#include "nm-ip4-config.h"
#include "nm-device-logging.h"
_LOG_DECLARE_SELF(NMDeviceBond);
/*****************************************************************************/
struct _NMDeviceBond {
NMDevice parent;
};
struct _NMDeviceBondClass {
NMDeviceClass parent;
};
G_DEFINE_TYPE (NMDeviceBond, nm_device_bond, NM_TYPE_DEVICE)
/*****************************************************************************/
static NMDeviceCapabilities
get_generic_capabilities (NMDevice *dev)
{
return NM_DEVICE_CAP_CARRIER_DETECT | NM_DEVICE_CAP_IS_SOFTWARE;
}
static gboolean
complete_connection (NMDevice *device,
NMConnection *connection,
const char *specific_object,
NMConnection *const*existing_connections,
GError **error)
{
NMSettingBond *s_bond;
nm_utils_complete_generic (nm_device_get_platform (device),
connection,
NM_SETTING_BOND_SETTING_NAME,
existing_connections,
NULL,
_("Bond connection"),
"bond",
NULL,
TRUE);
s_bond = nm_connection_get_setting_bond (connection);
if (!s_bond) {
s_bond = (NMSettingBond *) nm_setting_bond_new ();
nm_connection_add_setting (connection, NM_SETTING (s_bond));
}
return TRUE;
}
/*****************************************************************************/
static gboolean
_set_bond_attr (NMDevice *device, const char *attr, const char *value)
{
NMDeviceBond *self = NM_DEVICE_BOND (device);
int ifindex = nm_device_get_ifindex (device);
gboolean ret;
ret = nm_platform_sysctl_master_set_option (nm_device_get_platform (device),
ifindex,
attr,
value);
if (!ret)
_LOGW (LOGD_PLATFORM, "failed to set bonding attribute '%s' to '%s'", attr, value);
return ret;
}
#define _set_bond_attr_take(device, attr, value) \
G_STMT_START { \
gs_free char *_tmp = (value); \
\
_set_bond_attr (device, NM_SETTING_BOND_OPTION_ARP_IP_TARGET, _tmp); \
} G_STMT_END
#define _set_bond_attr_printf(device, attr, fmt, ...) \
_set_bond_attr_take ((device), (attr), g_strdup_printf (fmt, __VA_ARGS__))
static gboolean
ignore_option (NMSettingBond *s_bond, const char *option, const char *value)
{
const char *defvalue;
if (nm_streq0 (option, NM_SETTING_BOND_OPTION_MIIMON)) {
/* The default value for miimon, when missing in the setting, is
* 0 if arp_interval is != 0, and 100 otherwise. So, let's ignore
* miimon=0 (which means that miimon is disabled) and accept any
* other value. Adding miimon=100 does not cause any harm.
*/
defvalue = "0";
} else
defvalue = nm_setting_bond_get_option_default (s_bond, option);
return nm_streq0 (value, defvalue);
}
static void
update_connection (NMDevice *device, NMConnection *connection)
{
NMSettingBond *s_bond = nm_connection_get_setting_bond (connection);
int ifindex = nm_device_get_ifindex (device);
NMBondMode mode = NM_BOND_MODE_UNKNOWN;
const char **options;
if (!s_bond) {
s_bond = (NMSettingBond *) nm_setting_bond_new ();
nm_connection_add_setting (connection, (NMSetting *) s_bond);
}
/* Read bond options from sysfs and update the Bond setting to match */
options = nm_setting_bond_get_valid_options (s_bond);
for (; *options; options++) {
char *p;
gs_free char *value = nm_platform_sysctl_master_get_option (nm_device_get_platform (device),
ifindex,
*options);
if ( value
&& _nm_setting_bond_get_option_type (s_bond, *options) == NM_BOND_OPTION_TYPE_BOTH) {
p = strchr (value, ' ');
if (p)
*p = '\0';
}
if (mode == NM_BOND_MODE_UNKNOWN) {
if (value && nm_streq (*options, NM_SETTING_BOND_OPTION_MODE))
mode = _nm_setting_bond_mode_from_string (value);
if (mode == NM_BOND_MODE_UNKNOWN)
continue;
}
if (!_nm_setting_bond_option_supported (*options, mode))
continue;
if ( value
&& value[0]
&& !ignore_option (s_bond, *options, value)) {
/* Replace " " with "," for arp_ip_targets from the kernel */
if (strcmp (*options, NM_SETTING_BOND_OPTION_ARP_IP_TARGET) == 0) {
for (p = value; *p; p++) {
if (*p == ' ')
*p = ',';
}
}
nm_setting_bond_add_option (s_bond, *options, value);
}
}
}
static gboolean
master_update_slave_connection (NMDevice *self,
NMDevice *slave,
NMConnection *connection,
GError **error)
{
g_object_set (nm_connection_get_setting_connection (connection),
NM_SETTING_CONNECTION_MASTER, nm_device_get_iface (self),
NM_SETTING_CONNECTION_SLAVE_TYPE, NM_SETTING_BOND_SETTING_NAME,
NULL);
return TRUE;
}
static void
set_arp_targets (NMDevice *device,
NMBondMode mode,
const char *cur_arp_ip_target,
const char *new_arp_ip_target)
{
gs_unref_ptrarray GPtrArray *free_list = NULL;
gs_free const char **cur_strv = NULL;
gs_free const char **new_strv = NULL;
gsize cur_len;
gsize new_len;
gsize i;
gsize j;
cur_strv = nm_utils_strsplit_set_full (cur_arp_ip_target, NM_ASCII_SPACES, NM_UTILS_STRSPLIT_SET_FLAGS_STRSTRIP);
new_strv = nm_utils_bond_option_arp_ip_targets_split (new_arp_ip_target);
cur_len = NM_PTRARRAY_LEN (cur_strv);
new_len = NM_PTRARRAY_LEN (new_strv);
if (new_len > 0) {
for (j = 0, i = 0; i < new_len; i++) {
const char *s;
in_addr_t a4;
s = new_strv[i];
if (nm_utils_parse_inaddr_bin (AF_INET, s, NULL, &a4)) {
char sbuf[INET_ADDRSTRLEN];
_nm_utils_inet4_ntop (a4, sbuf);
if (!nm_streq (s, sbuf)) {
if (!free_list)
free_list = g_ptr_array_new_with_free_func (g_free);
s = g_strdup (sbuf);
g_ptr_array_add (free_list, (gpointer) s);
}
}
if (nm_utils_strv_find_first ((char **) new_strv, i, s) < 0)
new_strv[j++] = s;
}
new_strv[j] = NULL;
new_len = j;
}
if ( cur_len == 0
&& new_len == 0)
return;
if (_nm_utils_strv_equal ((char **) cur_strv, (char **) new_strv))
return;
for (i = 0; i < cur_len; i++)
_set_bond_attr_printf (device, NM_SETTING_BOND_OPTION_ARP_IP_TARGET, "-%s", cur_strv[i]);
for (i = 0; i < new_len; i++)
_set_bond_attr_printf (device, NM_SETTING_BOND_OPTION_ARP_IP_TARGET, "+%s", new_strv[i]);
}
/*
* Sets bond attribute stored in the option hashtable or
* the default value if no value was set.
*/
static void
set_bond_attr_or_default (NMDevice *device,
NMSettingBond *s_bond,
const char *opt)
{
NMDeviceBond *self = NM_DEVICE_BOND (device);
const char *value;
value = nm_setting_bond_get_option_or_default (s_bond, opt);
if (!value) {
if ( _LOGT_ENABLED (LOGD_BOND)
&& nm_setting_bond_get_option_by_name (s_bond, opt))
_LOGT (LOGD_BOND, "bond option '%s' not set as it conflicts with other options", opt);
return;
}
_set_bond_attr (device, opt, value);
}
static void
set_bond_attr_active_slave (NMDevice *device, NMSettingBond *s_bond)
{
NMDeviceBond *self = NM_DEVICE_BOND (device);
const NMPlatformLink *plink;
const char *value;
const char *error_reason;
int ifindex;
value = nm_setting_bond_get_option_or_default (s_bond, NM_SETTING_BOND_OPTION_ACTIVE_SLAVE);
if (!value)
return;
if (!nm_str_is_empty (value)) {
ifindex = nm_device_get_ifindex (device);
plink = nm_platform_link_get_by_ifname (nm_device_get_platform (device), value);
if (!plink)
error_reason = "does not exist";
else if (plink->master != ifindex)
error_reason = "is not yet enslaved";
else if (!NM_FLAGS_HAS (plink->n_ifi_flags, IFF_UP))
error_reason = "is not up";
else
error_reason = NULL;
if (error_reason) {
_LOGT (LOGD_BOND, "bond option 'active_slave' not set as device \"%s\" %s", value, error_reason);
return;
}
}
_set_bond_attr (device, NM_SETTING_BOND_OPTION_ACTIVE_SLAVE, value);
}
static gboolean
apply_bonding_config (NMDeviceBond *self)
{
NMDevice *device = NM_DEVICE (self);
int ifindex = nm_device_get_ifindex (device);
NMSettingBond *s_bond;
NMBondMode mode;
const char *mode_str;
gs_free char *cur_arp_ip_target = NULL;
s_bond = nm_device_get_applied_setting (device, NM_TYPE_SETTING_BOND);
g_return_val_if_fail (s_bond, FALSE);
mode_str = nm_setting_bond_get_option_or_default (s_bond, NM_SETTING_BOND_OPTION_MODE);
mode = _nm_setting_bond_mode_from_string (mode_str);
g_return_val_if_fail (mode != NM_BOND_MODE_UNKNOWN, FALSE);
/* Set mode first, as some other options (e.g. arp_interval) are valid
* only for certain modes.
*/
set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_MODE);
set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_MIIMON);
set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_UPDELAY);
set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_DOWNDELAY);
set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_ARP_INTERVAL);
set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_ARP_VALIDATE);
set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_PRIMARY);
/* ARP targets: clear and initialize the list */
cur_arp_ip_target = nm_platform_sysctl_master_get_option (nm_device_get_platform (device),
ifindex,
NM_SETTING_BOND_OPTION_ARP_IP_TARGET);
set_arp_targets (device,
mode,
cur_arp_ip_target,
nm_setting_bond_get_option_or_default (s_bond, NM_SETTING_BOND_OPTION_ARP_IP_TARGET));
set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_AD_ACTOR_SYSTEM);
set_bond_attr_active_slave (device, s_bond);
set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_AD_ACTOR_SYS_PRIO);
set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_AD_SELECT);
set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_AD_USER_PORT_KEY);
set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_ALL_SLAVES_ACTIVE);
set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_ARP_ALL_TARGETS);
set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_FAIL_OVER_MAC);
set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_LACP_RATE);
set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_LP_INTERVAL);
set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_MIN_LINKS);
set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_PACKETS_PER_SLAVE);
set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_PRIMARY_RESELECT);
set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_RESEND_IGMP);
set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_TLB_DYNAMIC_LB);
set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_USE_CARRIER);
set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_XMIT_HASH_POLICY);
set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_NUM_GRAT_ARP);
return TRUE;
}
static NMActStageReturn
act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason)
{
NMDeviceBond *self = NM_DEVICE_BOND (device);
NMActStageReturn ret = NM_ACT_STAGE_RETURN_SUCCESS;
/* Interface must be down to set bond options */
nm_device_take_down (device, TRUE);
if (!apply_bonding_config (self))
ret = NM_ACT_STAGE_RETURN_FAILURE;
else {
if (!nm_device_hw_addr_set_cloned (device,
nm_device_get_applied_connection (device),
FALSE))
ret = NM_ACT_STAGE_RETURN_FAILURE;
}
nm_device_bring_up (device, TRUE, NULL);
return ret;
}
static gboolean
enslave_slave (NMDevice *device,
NMDevice *slave,
NMConnection *connection,
gboolean configure)
{
NMDeviceBond *self = NM_DEVICE_BOND (device);
gboolean success = TRUE;
2012-11-14 14:05:30 -06:00
const char *slave_iface = nm_device_get_ip_iface (slave);
NMConnection *master_con;
nm_device_master_check_slave_physical_port (device, slave, LOGD_BOND);
if (configure) {
nm_device_take_down (slave, TRUE);
success = nm_platform_link_enslave (nm_device_get_platform (device),
platform: add self argument to platform functions Most nm_platform_*() functions operate on the platform singleton nm_platform_get(). That made sense because the NMPlatform instance was mainly to hook fake platform for testing. While the implicit argument saved some typing, I think explicit is better. Especially, because NMPlatform could become a more usable object then just a hook for testing. With this change, NMPlatform instances can be used individually, not only as a singleton instance. Before this change, the constructor of NMLinuxPlatform could not call any nm_platform_*() functions because the singleton was not yet initialized. We could only instantiate an incomplete instance, register it via nm_platform_setup(), and then complete initialization via singleton->setup(). With this change, we can create and fully initialize NMPlatform instances before/without setting them up them as singleton. Also, currently there is no clear distinction between functions that operate on the NMPlatform instance, and functions that can be used stand-alone (e.g. nm_platform_ip4_address_to_string()). The latter can not be mocked for testing. With this change, the distinction becomes obvious. That is also useful because it becomes clearer which functions make use of the platform cache and which not. Inside nm-linux-platform.c, continue the pattern that the self instance is named @platform. That makes sense because its type is NMPlatform, and not NMLinuxPlatform what we would expect from a paramter named @self. This is a major diff that causes some pain when rebasing. Try to rebase to the parent commit of this commit as a first step. Then rebase on top of this commit using merge-strategy "ours".
2015-04-18 12:36:09 +02:00
nm_device_get_ip_ifindex (device),
nm_device_get_ip_ifindex (slave));
nm_device_bring_up (slave, TRUE, NULL);
2012-11-14 14:05:30 -06:00
if (!success)
return FALSE;
_LOGI (LOGD_BOND, "enslaved bond slave %s", slave_iface);
/* The active_slave option can be set only after the interface is enslaved */
master_con = nm_device_get_applied_connection (device);
if (master_con) {
NMSettingBond *s_bond = nm_connection_get_setting_bond (master_con);
const char *active;
if (s_bond) {
active = nm_setting_bond_get_option_or_default (s_bond,
NM_SETTING_BOND_OPTION_ACTIVE_SLAVE);
if (nm_streq0 (active, nm_device_get_iface (slave))) {
nm_platform_sysctl_master_set_option (nm_device_get_platform (device),
nm_device_get_ifindex (device),
NM_SETTING_BOND_OPTION_ACTIVE_SLAVE,
active);
_LOGD (LOGD_BOND, "setting slave %s as active one for master %s",
active, nm_device_get_iface (device));
}
}
}
} else
_LOGI (LOGD_BOND, "bond slave %s was enslaved", slave_iface);
return TRUE;
}
static void
release_slave (NMDevice *device,
NMDevice *slave,
gboolean configure)
{
NMDeviceBond *self = NM_DEVICE_BOND (device);
gboolean success;
gs_free char *address = NULL;
device: fix crash releasing destroyed slave I encountered this on a WIP branch, but I think it can happen under regular conditions. I think there is no error condition here, and we should do nothing if we have no ifindex. <debug> [1561653068.2192] platform: signal: link removed: 1699: test1p <DOWN;broadcast,multicast> mtu 1500 master 1698 arp 1 veth* init addrgenmode none addr D6:14:45:97:06:75 brd FF:FF:FF:FF:FF:FF driver veth rx:0,0 tx:38,5606 ... <info> [1561653068.2617] device (test1): state change: activated -> unmanaged (reason 'unmanaged', sys-iface-state: 'removed') ... <trace> [1561653068.2635] device[0x564058c73750] (test1p): sys-iface-state: external -> removed <debug> [1561653068.2635] device[0x564058c73750] (test1p): unrealize (ifindex 1699) <debug> [1561653068.2636] device[0x564058c73750] (test1p): parent: clear <trace> [1561653068.2636] device[0x564058b98eb0] (vethbr): mtu: commit-mtu... <debug> [1561653068.2639] device[0x564058c73750] (test1p): unmanaged: flags set to [platform-init,!sleeping,!by-type,!user-explicit,!user-settings,!user-udev,!is-slave=0x10/0x1479/unmanaged/unrealized], set-unmanaged [platform-init=0x10]) <debug> [1561653068.2639] device[0x564058c73750] (test1p): unmanaged: flags set to [platform-init,!sleeping,!user-settings=0x10/0x51/unmanaged/unrealized], forget [parent,by-type,user-explicit,user-udev,external-down,is-slave=0x1c2c]) <info> [1561653068.2639] device (test1p): state change: activated -> unmanaged (reason 'unmanaged', sys-iface-state: 'removed') <debug> [1561653068.2640] device[0x564058c73750] (test1p): deactivating device (reason 'unmanaged') [3] <trace> [1561653068.2640] device[0x564058c73750] (test1p): ip4-state: set to 0 (none) <trace> [1561653068.2640] device[0x564058c73750] (test1p): ip6-state: set to 0 (none) <trace> [1561653068.2640] device[0x564058c73750] (test1p): remove_pending_action (0): 'dhcp6' not pending (expected) <trace> [1561653068.2640] device[0x564058c73750] (test1p): remove_pending_action (0): 'autoconf6' not pending (expected) <debug> [1561653068.2640] rules-manager: sync <debug> [1561653068.2640] device[0x564058c73750] (test1p): set metered value 0 <debug> [1561653068.2641] device[0x564058c73750] (test1p): ip4-config: update (commit=1, new-config=(nil)) <debug> [1561653068.2641] device[0x564058c73750] (test1p): ip6-config: update (commit=1, new-config=(nil)) <debug> [1561653068.2644] device[0x564058b98eb0] (vethbr): slave test1p state change 100 (activated) -> 10 (unmanaged) <trace> [1561653068.2644] device[0x564058b98eb0] (vethbr): master: release one slave 0x564058c73750/test1p ((src/platform/nm-platform.c:2002)): assertion '<dropped>' failed backtrace: ... #3 0x0000564057fb713e _nm_g_return_if_fail_warning (NetworkManager) #4 0x000056405808b37c release_slave (NetworkManager) #5 0x0000564058079aef nm_device_master_release_one_slave (NetworkManager) #6 0x00005640580844d7 slave_state_changed (NetworkManager) #7 0x00007efc24833fae ffi_call_unix64 (libffi.so.6) #8 0x00007efc2483396f ffi_call (libffi.so.6) #9 0x00007efc29b836e5 g_cclosure_marshal_generic (libgobject-2.0.so.0) #10 0x00007efc29b82c1d g_closure_invoke (libgobject-2.0.so.0) #11 0x00007efc29b96173 signal_emit_unlocked_R (libgobject-2.0.so.0) #12 0x00007efc29b9f29a g_signal_emit_valist (libgobject-2.0.so.0) #13 0x00007efc29b9f893 g_signal_emit (libgobject-2.0.so.0) #14 0x000056405807ab20 _set_state_full (NetworkManager) #15 0x000056405807d803 nm_device_unrealize (NetworkManager) #16 0x0000564057f6072c _platform_link_cb_idle (NetworkManager) #17 0x00007efc296a01db g_idle_dispatch (libglib-2.0.so.0) ...
2019-06-28 17:31:40 +02:00
int ifindex_slave;
int ifindex;
if (configure) {
ifindex = nm_device_get_ifindex (device);
if ( ifindex <= 0
|| !nm_platform_link_get (nm_device_get_platform (device), ifindex))
configure = FALSE;
}
device: fix crash releasing destroyed slave I encountered this on a WIP branch, but I think it can happen under regular conditions. I think there is no error condition here, and we should do nothing if we have no ifindex. <debug> [1561653068.2192] platform: signal: link removed: 1699: test1p <DOWN;broadcast,multicast> mtu 1500 master 1698 arp 1 veth* init addrgenmode none addr D6:14:45:97:06:75 brd FF:FF:FF:FF:FF:FF driver veth rx:0,0 tx:38,5606 ... <info> [1561653068.2617] device (test1): state change: activated -> unmanaged (reason 'unmanaged', sys-iface-state: 'removed') ... <trace> [1561653068.2635] device[0x564058c73750] (test1p): sys-iface-state: external -> removed <debug> [1561653068.2635] device[0x564058c73750] (test1p): unrealize (ifindex 1699) <debug> [1561653068.2636] device[0x564058c73750] (test1p): parent: clear <trace> [1561653068.2636] device[0x564058b98eb0] (vethbr): mtu: commit-mtu... <debug> [1561653068.2639] device[0x564058c73750] (test1p): unmanaged: flags set to [platform-init,!sleeping,!by-type,!user-explicit,!user-settings,!user-udev,!is-slave=0x10/0x1479/unmanaged/unrealized], set-unmanaged [platform-init=0x10]) <debug> [1561653068.2639] device[0x564058c73750] (test1p): unmanaged: flags set to [platform-init,!sleeping,!user-settings=0x10/0x51/unmanaged/unrealized], forget [parent,by-type,user-explicit,user-udev,external-down,is-slave=0x1c2c]) <info> [1561653068.2639] device (test1p): state change: activated -> unmanaged (reason 'unmanaged', sys-iface-state: 'removed') <debug> [1561653068.2640] device[0x564058c73750] (test1p): deactivating device (reason 'unmanaged') [3] <trace> [1561653068.2640] device[0x564058c73750] (test1p): ip4-state: set to 0 (none) <trace> [1561653068.2640] device[0x564058c73750] (test1p): ip6-state: set to 0 (none) <trace> [1561653068.2640] device[0x564058c73750] (test1p): remove_pending_action (0): 'dhcp6' not pending (expected) <trace> [1561653068.2640] device[0x564058c73750] (test1p): remove_pending_action (0): 'autoconf6' not pending (expected) <debug> [1561653068.2640] rules-manager: sync <debug> [1561653068.2640] device[0x564058c73750] (test1p): set metered value 0 <debug> [1561653068.2641] device[0x564058c73750] (test1p): ip4-config: update (commit=1, new-config=(nil)) <debug> [1561653068.2641] device[0x564058c73750] (test1p): ip6-config: update (commit=1, new-config=(nil)) <debug> [1561653068.2644] device[0x564058b98eb0] (vethbr): slave test1p state change 100 (activated) -> 10 (unmanaged) <trace> [1561653068.2644] device[0x564058b98eb0] (vethbr): master: release one slave 0x564058c73750/test1p ((src/platform/nm-platform.c:2002)): assertion '<dropped>' failed backtrace: ... #3 0x0000564057fb713e _nm_g_return_if_fail_warning (NetworkManager) #4 0x000056405808b37c release_slave (NetworkManager) #5 0x0000564058079aef nm_device_master_release_one_slave (NetworkManager) #6 0x00005640580844d7 slave_state_changed (NetworkManager) #7 0x00007efc24833fae ffi_call_unix64 (libffi.so.6) #8 0x00007efc2483396f ffi_call (libffi.so.6) #9 0x00007efc29b836e5 g_cclosure_marshal_generic (libgobject-2.0.so.0) #10 0x00007efc29b82c1d g_closure_invoke (libgobject-2.0.so.0) #11 0x00007efc29b96173 signal_emit_unlocked_R (libgobject-2.0.so.0) #12 0x00007efc29b9f29a g_signal_emit_valist (libgobject-2.0.so.0) #13 0x00007efc29b9f893 g_signal_emit (libgobject-2.0.so.0) #14 0x000056405807ab20 _set_state_full (NetworkManager) #15 0x000056405807d803 nm_device_unrealize (NetworkManager) #16 0x0000564057f6072c _platform_link_cb_idle (NetworkManager) #17 0x00007efc296a01db g_idle_dispatch (libglib-2.0.so.0) ...
2019-06-28 17:31:40 +02:00
ifindex_slave = nm_device_get_ip_ifindex (slave);
if (ifindex_slave <= 0)
_LOGD (LOGD_BOND, "bond slave %s is already released", nm_device_get_ip_iface (slave));
if (configure) {
/* When the last slave is released the bond MAC will be set to a random
* value by kernel; remember the current one and restore it afterwards.
*/
address = g_strdup (nm_device_get_hw_address (device));
device: fix crash releasing destroyed slave I encountered this on a WIP branch, but I think it can happen under regular conditions. I think there is no error condition here, and we should do nothing if we have no ifindex. <debug> [1561653068.2192] platform: signal: link removed: 1699: test1p <DOWN;broadcast,multicast> mtu 1500 master 1698 arp 1 veth* init addrgenmode none addr D6:14:45:97:06:75 brd FF:FF:FF:FF:FF:FF driver veth rx:0,0 tx:38,5606 ... <info> [1561653068.2617] device (test1): state change: activated -> unmanaged (reason 'unmanaged', sys-iface-state: 'removed') ... <trace> [1561653068.2635] device[0x564058c73750] (test1p): sys-iface-state: external -> removed <debug> [1561653068.2635] device[0x564058c73750] (test1p): unrealize (ifindex 1699) <debug> [1561653068.2636] device[0x564058c73750] (test1p): parent: clear <trace> [1561653068.2636] device[0x564058b98eb0] (vethbr): mtu: commit-mtu... <debug> [1561653068.2639] device[0x564058c73750] (test1p): unmanaged: flags set to [platform-init,!sleeping,!by-type,!user-explicit,!user-settings,!user-udev,!is-slave=0x10/0x1479/unmanaged/unrealized], set-unmanaged [platform-init=0x10]) <debug> [1561653068.2639] device[0x564058c73750] (test1p): unmanaged: flags set to [platform-init,!sleeping,!user-settings=0x10/0x51/unmanaged/unrealized], forget [parent,by-type,user-explicit,user-udev,external-down,is-slave=0x1c2c]) <info> [1561653068.2639] device (test1p): state change: activated -> unmanaged (reason 'unmanaged', sys-iface-state: 'removed') <debug> [1561653068.2640] device[0x564058c73750] (test1p): deactivating device (reason 'unmanaged') [3] <trace> [1561653068.2640] device[0x564058c73750] (test1p): ip4-state: set to 0 (none) <trace> [1561653068.2640] device[0x564058c73750] (test1p): ip6-state: set to 0 (none) <trace> [1561653068.2640] device[0x564058c73750] (test1p): remove_pending_action (0): 'dhcp6' not pending (expected) <trace> [1561653068.2640] device[0x564058c73750] (test1p): remove_pending_action (0): 'autoconf6' not pending (expected) <debug> [1561653068.2640] rules-manager: sync <debug> [1561653068.2640] device[0x564058c73750] (test1p): set metered value 0 <debug> [1561653068.2641] device[0x564058c73750] (test1p): ip4-config: update (commit=1, new-config=(nil)) <debug> [1561653068.2641] device[0x564058c73750] (test1p): ip6-config: update (commit=1, new-config=(nil)) <debug> [1561653068.2644] device[0x564058b98eb0] (vethbr): slave test1p state change 100 (activated) -> 10 (unmanaged) <trace> [1561653068.2644] device[0x564058b98eb0] (vethbr): master: release one slave 0x564058c73750/test1p ((src/platform/nm-platform.c:2002)): assertion '<dropped>' failed backtrace: ... #3 0x0000564057fb713e _nm_g_return_if_fail_warning (NetworkManager) #4 0x000056405808b37c release_slave (NetworkManager) #5 0x0000564058079aef nm_device_master_release_one_slave (NetworkManager) #6 0x00005640580844d7 slave_state_changed (NetworkManager) #7 0x00007efc24833fae ffi_call_unix64 (libffi.so.6) #8 0x00007efc2483396f ffi_call (libffi.so.6) #9 0x00007efc29b836e5 g_cclosure_marshal_generic (libgobject-2.0.so.0) #10 0x00007efc29b82c1d g_closure_invoke (libgobject-2.0.so.0) #11 0x00007efc29b96173 signal_emit_unlocked_R (libgobject-2.0.so.0) #12 0x00007efc29b9f29a g_signal_emit_valist (libgobject-2.0.so.0) #13 0x00007efc29b9f893 g_signal_emit (libgobject-2.0.so.0) #14 0x000056405807ab20 _set_state_full (NetworkManager) #15 0x000056405807d803 nm_device_unrealize (NetworkManager) #16 0x0000564057f6072c _platform_link_cb_idle (NetworkManager) #17 0x00007efc296a01db g_idle_dispatch (libglib-2.0.so.0) ...
2019-06-28 17:31:40 +02:00
if (ifindex_slave > 0) {
success = nm_platform_link_release (nm_device_get_platform (device),
nm_device_get_ip_ifindex (device),
ifindex_slave);
if (success) {
_LOGI (LOGD_BOND, "released bond slave %s",
nm_device_get_ip_iface (slave));
} else {
_LOGW (LOGD_BOND, "failed to release bond slave %s",
nm_device_get_ip_iface (slave));
}
}
nm_platform_process_events (nm_device_get_platform (device));
if (nm_device_update_hw_address (device))
nm_device_hw_addr_set (device, address, "restore", FALSE);
/* Kernel bonding code "closes" the slave when releasing it, (which clears
* IFF_UP), so we must bring it back up here to ensure carrier changes and
* other state is noticed by the now-released slave.
*/
device: fix crash releasing destroyed slave I encountered this on a WIP branch, but I think it can happen under regular conditions. I think there is no error condition here, and we should do nothing if we have no ifindex. <debug> [1561653068.2192] platform: signal: link removed: 1699: test1p <DOWN;broadcast,multicast> mtu 1500 master 1698 arp 1 veth* init addrgenmode none addr D6:14:45:97:06:75 brd FF:FF:FF:FF:FF:FF driver veth rx:0,0 tx:38,5606 ... <info> [1561653068.2617] device (test1): state change: activated -> unmanaged (reason 'unmanaged', sys-iface-state: 'removed') ... <trace> [1561653068.2635] device[0x564058c73750] (test1p): sys-iface-state: external -> removed <debug> [1561653068.2635] device[0x564058c73750] (test1p): unrealize (ifindex 1699) <debug> [1561653068.2636] device[0x564058c73750] (test1p): parent: clear <trace> [1561653068.2636] device[0x564058b98eb0] (vethbr): mtu: commit-mtu... <debug> [1561653068.2639] device[0x564058c73750] (test1p): unmanaged: flags set to [platform-init,!sleeping,!by-type,!user-explicit,!user-settings,!user-udev,!is-slave=0x10/0x1479/unmanaged/unrealized], set-unmanaged [platform-init=0x10]) <debug> [1561653068.2639] device[0x564058c73750] (test1p): unmanaged: flags set to [platform-init,!sleeping,!user-settings=0x10/0x51/unmanaged/unrealized], forget [parent,by-type,user-explicit,user-udev,external-down,is-slave=0x1c2c]) <info> [1561653068.2639] device (test1p): state change: activated -> unmanaged (reason 'unmanaged', sys-iface-state: 'removed') <debug> [1561653068.2640] device[0x564058c73750] (test1p): deactivating device (reason 'unmanaged') [3] <trace> [1561653068.2640] device[0x564058c73750] (test1p): ip4-state: set to 0 (none) <trace> [1561653068.2640] device[0x564058c73750] (test1p): ip6-state: set to 0 (none) <trace> [1561653068.2640] device[0x564058c73750] (test1p): remove_pending_action (0): 'dhcp6' not pending (expected) <trace> [1561653068.2640] device[0x564058c73750] (test1p): remove_pending_action (0): 'autoconf6' not pending (expected) <debug> [1561653068.2640] rules-manager: sync <debug> [1561653068.2640] device[0x564058c73750] (test1p): set metered value 0 <debug> [1561653068.2641] device[0x564058c73750] (test1p): ip4-config: update (commit=1, new-config=(nil)) <debug> [1561653068.2641] device[0x564058c73750] (test1p): ip6-config: update (commit=1, new-config=(nil)) <debug> [1561653068.2644] device[0x564058b98eb0] (vethbr): slave test1p state change 100 (activated) -> 10 (unmanaged) <trace> [1561653068.2644] device[0x564058b98eb0] (vethbr): master: release one slave 0x564058c73750/test1p ((src/platform/nm-platform.c:2002)): assertion '<dropped>' failed backtrace: ... #3 0x0000564057fb713e _nm_g_return_if_fail_warning (NetworkManager) #4 0x000056405808b37c release_slave (NetworkManager) #5 0x0000564058079aef nm_device_master_release_one_slave (NetworkManager) #6 0x00005640580844d7 slave_state_changed (NetworkManager) #7 0x00007efc24833fae ffi_call_unix64 (libffi.so.6) #8 0x00007efc2483396f ffi_call (libffi.so.6) #9 0x00007efc29b836e5 g_cclosure_marshal_generic (libgobject-2.0.so.0) #10 0x00007efc29b82c1d g_closure_invoke (libgobject-2.0.so.0) #11 0x00007efc29b96173 signal_emit_unlocked_R (libgobject-2.0.so.0) #12 0x00007efc29b9f29a g_signal_emit_valist (libgobject-2.0.so.0) #13 0x00007efc29b9f893 g_signal_emit (libgobject-2.0.so.0) #14 0x000056405807ab20 _set_state_full (NetworkManager) #15 0x000056405807d803 nm_device_unrealize (NetworkManager) #16 0x0000564057f6072c _platform_link_cb_idle (NetworkManager) #17 0x00007efc296a01db g_idle_dispatch (libglib-2.0.so.0) ...
2019-06-28 17:31:40 +02:00
if (ifindex_slave > 0) {
if (!nm_device_bring_up (slave, TRUE, NULL))
_LOGW (LOGD_BOND, "released bond slave could not be brought up.");
}
} else {
device: fix crash releasing destroyed slave I encountered this on a WIP branch, but I think it can happen under regular conditions. I think there is no error condition here, and we should do nothing if we have no ifindex. <debug> [1561653068.2192] platform: signal: link removed: 1699: test1p <DOWN;broadcast,multicast> mtu 1500 master 1698 arp 1 veth* init addrgenmode none addr D6:14:45:97:06:75 brd FF:FF:FF:FF:FF:FF driver veth rx:0,0 tx:38,5606 ... <info> [1561653068.2617] device (test1): state change: activated -> unmanaged (reason 'unmanaged', sys-iface-state: 'removed') ... <trace> [1561653068.2635] device[0x564058c73750] (test1p): sys-iface-state: external -> removed <debug> [1561653068.2635] device[0x564058c73750] (test1p): unrealize (ifindex 1699) <debug> [1561653068.2636] device[0x564058c73750] (test1p): parent: clear <trace> [1561653068.2636] device[0x564058b98eb0] (vethbr): mtu: commit-mtu... <debug> [1561653068.2639] device[0x564058c73750] (test1p): unmanaged: flags set to [platform-init,!sleeping,!by-type,!user-explicit,!user-settings,!user-udev,!is-slave=0x10/0x1479/unmanaged/unrealized], set-unmanaged [platform-init=0x10]) <debug> [1561653068.2639] device[0x564058c73750] (test1p): unmanaged: flags set to [platform-init,!sleeping,!user-settings=0x10/0x51/unmanaged/unrealized], forget [parent,by-type,user-explicit,user-udev,external-down,is-slave=0x1c2c]) <info> [1561653068.2639] device (test1p): state change: activated -> unmanaged (reason 'unmanaged', sys-iface-state: 'removed') <debug> [1561653068.2640] device[0x564058c73750] (test1p): deactivating device (reason 'unmanaged') [3] <trace> [1561653068.2640] device[0x564058c73750] (test1p): ip4-state: set to 0 (none) <trace> [1561653068.2640] device[0x564058c73750] (test1p): ip6-state: set to 0 (none) <trace> [1561653068.2640] device[0x564058c73750] (test1p): remove_pending_action (0): 'dhcp6' not pending (expected) <trace> [1561653068.2640] device[0x564058c73750] (test1p): remove_pending_action (0): 'autoconf6' not pending (expected) <debug> [1561653068.2640] rules-manager: sync <debug> [1561653068.2640] device[0x564058c73750] (test1p): set metered value 0 <debug> [1561653068.2641] device[0x564058c73750] (test1p): ip4-config: update (commit=1, new-config=(nil)) <debug> [1561653068.2641] device[0x564058c73750] (test1p): ip6-config: update (commit=1, new-config=(nil)) <debug> [1561653068.2644] device[0x564058b98eb0] (vethbr): slave test1p state change 100 (activated) -> 10 (unmanaged) <trace> [1561653068.2644] device[0x564058b98eb0] (vethbr): master: release one slave 0x564058c73750/test1p ((src/platform/nm-platform.c:2002)): assertion '<dropped>' failed backtrace: ... #3 0x0000564057fb713e _nm_g_return_if_fail_warning (NetworkManager) #4 0x000056405808b37c release_slave (NetworkManager) #5 0x0000564058079aef nm_device_master_release_one_slave (NetworkManager) #6 0x00005640580844d7 slave_state_changed (NetworkManager) #7 0x00007efc24833fae ffi_call_unix64 (libffi.so.6) #8 0x00007efc2483396f ffi_call (libffi.so.6) #9 0x00007efc29b836e5 g_cclosure_marshal_generic (libgobject-2.0.so.0) #10 0x00007efc29b82c1d g_closure_invoke (libgobject-2.0.so.0) #11 0x00007efc29b96173 signal_emit_unlocked_R (libgobject-2.0.so.0) #12 0x00007efc29b9f29a g_signal_emit_valist (libgobject-2.0.so.0) #13 0x00007efc29b9f893 g_signal_emit (libgobject-2.0.so.0) #14 0x000056405807ab20 _set_state_full (NetworkManager) #15 0x000056405807d803 nm_device_unrealize (NetworkManager) #16 0x0000564057f6072c _platform_link_cb_idle (NetworkManager) #17 0x00007efc296a01db g_idle_dispatch (libglib-2.0.so.0) ...
2019-06-28 17:31:40 +02:00
if (ifindex_slave > 0) {
_LOGI (LOGD_BOND, "bond slave %s was released",
nm_device_get_ip_iface (slave));
}
}
}
static gboolean
create_and_realize (NMDevice *device,
NMConnection *connection,
NMDevice *parent,
const NMPlatformLink **out_plink,
GError **error)
{
const char *iface = nm_device_get_iface (device);
int r;
g_assert (iface);
r = nm_platform_link_bond_add (nm_device_get_platform (device), iface, out_plink);
if (r < 0) {
g_set_error (error, NM_DEVICE_ERROR, NM_DEVICE_ERROR_CREATION_FAILED,
"Failed to create bond interface '%s' for '%s': %s",
iface,
nm_connection_get_id (connection),
nm_strerror (r));
return FALSE;
}
return TRUE;
}
static gboolean
check_changed_options (NMSettingBond *s_a, NMSettingBond *s_b, GError **error)
{
const char **option_list;
option_list = nm_setting_bond_get_valid_options (NULL);
for (; *option_list; ++option_list) {
const char *name = *option_list;
/* We support changes to these */
if (NM_IN_STRSET (name,
NM_SETTING_BOND_OPTION_ACTIVE_SLAVE,
NM_SETTING_BOND_OPTION_PRIMARY)) {
continue;
}
/* Reject any other changes */
if (!nm_streq0 (nm_setting_bond_get_option_normalized (s_a, name),
nm_setting_bond_get_option_normalized (s_b, name))) {
g_set_error (error,
NM_DEVICE_ERROR,
NM_DEVICE_ERROR_INCOMPATIBLE_CONNECTION,
"Can't reapply '%s' bond option",
name);
return FALSE;
}
}
return TRUE;
}
static gboolean
can_reapply_change (NMDevice *device,
const char *setting_name,
NMSetting *s_old,
NMSetting *s_new,
GHashTable *diffs,
GError **error)
{
NMDeviceClass *device_class;
/* Only handle bond setting here, delegate other settings to parent class */
if (nm_streq (setting_name, NM_SETTING_BOND_SETTING_NAME)) {
if (!nm_device_hash_check_invalid_keys (diffs,
NM_SETTING_BOND_SETTING_NAME,
error,
NM_SETTING_BOND_OPTIONS))
return FALSE;
return check_changed_options (NM_SETTING_BOND (s_old), NM_SETTING_BOND (s_new), error);
}
device_class = NM_DEVICE_CLASS (nm_device_bond_parent_class);
return device_class->can_reapply_change (device,
setting_name,
s_old,
s_new,
diffs,
error);
}
static void
reapply_connection (NMDevice *device, NMConnection *con_old, NMConnection *con_new)
{
NMDeviceBond *self = NM_DEVICE_BOND (device);
const char *value;
NMSettingBond *s_bond;
NMBondMode mode;
NM_DEVICE_CLASS (nm_device_bond_parent_class)->reapply_connection (device,
con_old,
con_new);
_LOGD (LOGD_BOND, "reapplying bond settings");
s_bond = nm_connection_get_setting_bond (con_new);
g_return_if_fail (s_bond);
value = nm_setting_bond_get_option_or_default (s_bond, NM_SETTING_BOND_OPTION_MODE);
mode = _nm_setting_bond_mode_from_string (value);
g_return_if_fail (mode != NM_BOND_MODE_UNKNOWN);
set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_PRIMARY);
set_bond_attr_active_slave (device, s_bond);
}
/*****************************************************************************/
static void
nm_device_bond_init (NMDeviceBond * self)
{
nm_assert (nm_device_is_master (NM_DEVICE (self)));
}
core/dbus: rework D-Bus implementation to use lower layer GDBusConnection API Previously, we used the generated GDBusInterfaceSkeleton types and glued them via the NMExportedObject base class to our NM types. We also used GDBusObjectManagerServer. Don't do that anymore. The resulting code was more complicated despite (or because?) using generated classes. It was hard to understand, complex, had ordering-issues, and had a runtime and memory overhead. This patch refactors this entirely and uses the lower layer API GDBusConnection directly. It replaces the generated code, GDBusInterfaceSkeleton, and GDBusObjectManagerServer. All this is now done by NMDbusObject and NMDBusManager and static descriptor instances of type GDBusInterfaceInfo. This adds a net plus of more then 1300 lines of hand written code. I claim that this implementation is easier to understand. Note that previously we also required extensive and complex glue code to bind our objects to the generated skeleton objects. Instead, now glue our objects directly to GDBusConnection. The result is more immediate and gets rid of layers of code in between. Now that the D-Bus glue us more under our control, we can address issus and bottlenecks better, instead of adding code to bend the generated skeletons to our needs. Note that the current implementation now only supports one D-Bus connection. That was effectively the case already, although there were places (and still are) where the code pretends it could also support connections from a private socket. We dropped private socket support mainly because it was unused, untested and buggy, but also because GDBusObjectManagerServer could not export the same objects on multiple connections. Now, it would be rather straight forward to fix that and re-introduce ObjectManager on each private connection. But this commit doesn't do that yet, and the new code intentionally supports only one D-Bus connection. Also, the D-Bus startup was simplified. There is no retry, either nm_dbus_manager_start() succeeds, or it detects the initrd case. In the initrd case, bus manager never tries to connect to D-Bus. Since the initrd scenario is not yet used/tested, this is good enough for the moment. It could be easily extended later, for example with polling whether the system bus appears (like was done previously). Also, restart of D-Bus daemon isn't supported either -- just like before. Note how NMDBusManager now implements the ObjectManager D-Bus interface directly. Also, this fixes race issues in the server, by no longer delaying PropertiesChanged signals. NMExportedObject would collect changed properties and send the signal out in idle_emit_properties_changed() on idle. This messes up the ordering of change events w.r.t. other signals and events on the bus. Note that not only NMExportedObject messed up the ordering. Also the generated code would hook into notify() and process change events in and idle handle, exhibiting the same ordering issue too. No longer do that. PropertiesChanged signals will be sent right away by hooking into dispatch_properties_changed(). This means, changing a property in quick succession will no longer be combined and is guaranteed to emit signals for each individual state. Quite possibly we emit now more PropertiesChanged signals then before. However, we are now able to group a set of changes by using standard g_object_freeze_notify()/g_object_thaw_notify(). We probably should make more use of that. Also, now that our signals are all handled in the right order, we might find places where we still emit them in the wrong order. But that is then due to the order in which our GObjects emit signals, not due to an ill behavior of the D-Bus glue. Possibly we need to identify such ordering issues and fix them. Numbers (for contrib/rpm --without debug on x86_64): - the patch changes the code size of NetworkManager by - 2809360 bytes + 2537528 bytes (-9.7%) - Runtime measurements are harder because there is a large variance during testing. In other words, the numbers are not reproducible. Currently, the implementation performs no caching of GVariants at all, but it would be rather simple to add it, if that turns out to be useful. Anyway, without strong claim, it seems that the new form tends to perform slightly better. That would be no surprise. $ time (for i in {1..1000}; do nmcli >/dev/null || break; echo -n .; done) - real 1m39.355s + real 1m37.432s $ time (for i in {1..2000}; do busctl call org.freedesktop.NetworkManager /org/freedesktop org.freedesktop.DBus.ObjectManager GetManagedObjects > /dev/null || break; echo -n .; done) - real 0m26.843s + real 0m25.281s - Regarding RSS size, just looking at the processes in similar conditions, doesn't give a large difference. On my system they consume about 19MB RSS. It seems that the new version has a slightly smaller RSS size. - 19356 RSS + 18660 RSS
2018-02-26 13:51:52 +01:00
static const NMDBusInterfaceInfoExtended interface_info_device_bond = {
.parent = NM_DEFINE_GDBUS_INTERFACE_INFO_INIT (
NM_DBUS_INTERFACE_DEVICE_BOND,
.signals = NM_DEFINE_GDBUS_SIGNAL_INFOS (
&nm_signal_info_property_changed_legacy,
),
.properties = NM_DEFINE_GDBUS_PROPERTY_INFOS (
NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE_L ("HwAddress", "s", NM_DEVICE_HW_ADDRESS),
NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE_L ("Carrier", "b", NM_DEVICE_CARRIER),
NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE_L ("Slaves", "ao", NM_DEVICE_SLAVES),
),
),
.legacy_property_changed = TRUE,
};
static void
nm_device_bond_class_init (NMDeviceBondClass *klass)
{
core/dbus: rework D-Bus implementation to use lower layer GDBusConnection API Previously, we used the generated GDBusInterfaceSkeleton types and glued them via the NMExportedObject base class to our NM types. We also used GDBusObjectManagerServer. Don't do that anymore. The resulting code was more complicated despite (or because?) using generated classes. It was hard to understand, complex, had ordering-issues, and had a runtime and memory overhead. This patch refactors this entirely and uses the lower layer API GDBusConnection directly. It replaces the generated code, GDBusInterfaceSkeleton, and GDBusObjectManagerServer. All this is now done by NMDbusObject and NMDBusManager and static descriptor instances of type GDBusInterfaceInfo. This adds a net plus of more then 1300 lines of hand written code. I claim that this implementation is easier to understand. Note that previously we also required extensive and complex glue code to bind our objects to the generated skeleton objects. Instead, now glue our objects directly to GDBusConnection. The result is more immediate and gets rid of layers of code in between. Now that the D-Bus glue us more under our control, we can address issus and bottlenecks better, instead of adding code to bend the generated skeletons to our needs. Note that the current implementation now only supports one D-Bus connection. That was effectively the case already, although there were places (and still are) where the code pretends it could also support connections from a private socket. We dropped private socket support mainly because it was unused, untested and buggy, but also because GDBusObjectManagerServer could not export the same objects on multiple connections. Now, it would be rather straight forward to fix that and re-introduce ObjectManager on each private connection. But this commit doesn't do that yet, and the new code intentionally supports only one D-Bus connection. Also, the D-Bus startup was simplified. There is no retry, either nm_dbus_manager_start() succeeds, or it detects the initrd case. In the initrd case, bus manager never tries to connect to D-Bus. Since the initrd scenario is not yet used/tested, this is good enough for the moment. It could be easily extended later, for example with polling whether the system bus appears (like was done previously). Also, restart of D-Bus daemon isn't supported either -- just like before. Note how NMDBusManager now implements the ObjectManager D-Bus interface directly. Also, this fixes race issues in the server, by no longer delaying PropertiesChanged signals. NMExportedObject would collect changed properties and send the signal out in idle_emit_properties_changed() on idle. This messes up the ordering of change events w.r.t. other signals and events on the bus. Note that not only NMExportedObject messed up the ordering. Also the generated code would hook into notify() and process change events in and idle handle, exhibiting the same ordering issue too. No longer do that. PropertiesChanged signals will be sent right away by hooking into dispatch_properties_changed(). This means, changing a property in quick succession will no longer be combined and is guaranteed to emit signals for each individual state. Quite possibly we emit now more PropertiesChanged signals then before. However, we are now able to group a set of changes by using standard g_object_freeze_notify()/g_object_thaw_notify(). We probably should make more use of that. Also, now that our signals are all handled in the right order, we might find places where we still emit them in the wrong order. But that is then due to the order in which our GObjects emit signals, not due to an ill behavior of the D-Bus glue. Possibly we need to identify such ordering issues and fix them. Numbers (for contrib/rpm --without debug on x86_64): - the patch changes the code size of NetworkManager by - 2809360 bytes + 2537528 bytes (-9.7%) - Runtime measurements are harder because there is a large variance during testing. In other words, the numbers are not reproducible. Currently, the implementation performs no caching of GVariants at all, but it would be rather simple to add it, if that turns out to be useful. Anyway, without strong claim, it seems that the new form tends to perform slightly better. That would be no surprise. $ time (for i in {1..1000}; do nmcli >/dev/null || break; echo -n .; done) - real 1m39.355s + real 1m37.432s $ time (for i in {1..2000}; do busctl call org.freedesktop.NetworkManager /org/freedesktop org.freedesktop.DBus.ObjectManager GetManagedObjects > /dev/null || break; echo -n .; done) - real 0m26.843s + real 0m25.281s - Regarding RSS size, just looking at the processes in similar conditions, doesn't give a large difference. On my system they consume about 19MB RSS. It seems that the new version has a slightly smaller RSS size. - 19356 RSS + 18660 RSS
2018-02-26 13:51:52 +01:00
NMDBusObjectClass *dbus_object_class = NM_DBUS_OBJECT_CLASS (klass);
NMDeviceClass *device_class = NM_DEVICE_CLASS (klass);
core/dbus: rework D-Bus implementation to use lower layer GDBusConnection API Previously, we used the generated GDBusInterfaceSkeleton types and glued them via the NMExportedObject base class to our NM types. We also used GDBusObjectManagerServer. Don't do that anymore. The resulting code was more complicated despite (or because?) using generated classes. It was hard to understand, complex, had ordering-issues, and had a runtime and memory overhead. This patch refactors this entirely and uses the lower layer API GDBusConnection directly. It replaces the generated code, GDBusInterfaceSkeleton, and GDBusObjectManagerServer. All this is now done by NMDbusObject and NMDBusManager and static descriptor instances of type GDBusInterfaceInfo. This adds a net plus of more then 1300 lines of hand written code. I claim that this implementation is easier to understand. Note that previously we also required extensive and complex glue code to bind our objects to the generated skeleton objects. Instead, now glue our objects directly to GDBusConnection. The result is more immediate and gets rid of layers of code in between. Now that the D-Bus glue us more under our control, we can address issus and bottlenecks better, instead of adding code to bend the generated skeletons to our needs. Note that the current implementation now only supports one D-Bus connection. That was effectively the case already, although there were places (and still are) where the code pretends it could also support connections from a private socket. We dropped private socket support mainly because it was unused, untested and buggy, but also because GDBusObjectManagerServer could not export the same objects on multiple connections. Now, it would be rather straight forward to fix that and re-introduce ObjectManager on each private connection. But this commit doesn't do that yet, and the new code intentionally supports only one D-Bus connection. Also, the D-Bus startup was simplified. There is no retry, either nm_dbus_manager_start() succeeds, or it detects the initrd case. In the initrd case, bus manager never tries to connect to D-Bus. Since the initrd scenario is not yet used/tested, this is good enough for the moment. It could be easily extended later, for example with polling whether the system bus appears (like was done previously). Also, restart of D-Bus daemon isn't supported either -- just like before. Note how NMDBusManager now implements the ObjectManager D-Bus interface directly. Also, this fixes race issues in the server, by no longer delaying PropertiesChanged signals. NMExportedObject would collect changed properties and send the signal out in idle_emit_properties_changed() on idle. This messes up the ordering of change events w.r.t. other signals and events on the bus. Note that not only NMExportedObject messed up the ordering. Also the generated code would hook into notify() and process change events in and idle handle, exhibiting the same ordering issue too. No longer do that. PropertiesChanged signals will be sent right away by hooking into dispatch_properties_changed(). This means, changing a property in quick succession will no longer be combined and is guaranteed to emit signals for each individual state. Quite possibly we emit now more PropertiesChanged signals then before. However, we are now able to group a set of changes by using standard g_object_freeze_notify()/g_object_thaw_notify(). We probably should make more use of that. Also, now that our signals are all handled in the right order, we might find places where we still emit them in the wrong order. But that is then due to the order in which our GObjects emit signals, not due to an ill behavior of the D-Bus glue. Possibly we need to identify such ordering issues and fix them. Numbers (for contrib/rpm --without debug on x86_64): - the patch changes the code size of NetworkManager by - 2809360 bytes + 2537528 bytes (-9.7%) - Runtime measurements are harder because there is a large variance during testing. In other words, the numbers are not reproducible. Currently, the implementation performs no caching of GVariants at all, but it would be rather simple to add it, if that turns out to be useful. Anyway, without strong claim, it seems that the new form tends to perform slightly better. That would be no surprise. $ time (for i in {1..1000}; do nmcli >/dev/null || break; echo -n .; done) - real 1m39.355s + real 1m37.432s $ time (for i in {1..2000}; do busctl call org.freedesktop.NetworkManager /org/freedesktop org.freedesktop.DBus.ObjectManager GetManagedObjects > /dev/null || break; echo -n .; done) - real 0m26.843s + real 0m25.281s - Regarding RSS size, just looking at the processes in similar conditions, doesn't give a large difference. On my system they consume about 19MB RSS. It seems that the new version has a slightly smaller RSS size. - 19356 RSS + 18660 RSS
2018-02-26 13:51:52 +01:00
dbus_object_class->interface_infos = NM_DBUS_INTERFACE_INFOS (&interface_info_device_bond);
device_class->connection_type_supported = NM_SETTING_BOND_SETTING_NAME;
device_class->connection_type_check_compatible = NM_SETTING_BOND_SETTING_NAME;
device_class->link_types = NM_DEVICE_DEFINE_LINK_TYPES (NM_LINK_TYPE_BOND);
device_class->is_master = TRUE;
device_class->get_generic_capabilities = get_generic_capabilities;
device_class->complete_connection = complete_connection;
device_class->update_connection = update_connection;
device_class->master_update_slave_connection = master_update_slave_connection;
device_class->create_and_realize = create_and_realize;
device_class->act_stage1_prepare = act_stage1_prepare;
device_class->get_configured_mtu = nm_device_get_configured_mtu_for_wired;
device_class->enslave_slave = enslave_slave;
device_class->release_slave = release_slave;
device_class->can_reapply_change = can_reapply_change;
device_class->reapply_connection = reapply_connection;
}
2014-09-08 10:15:02 -05:00
/*****************************************************************************/
2014-09-08 10:15:02 -05:00
#define NM_TYPE_BOND_DEVICE_FACTORY (nm_bond_device_factory_get_type ())
#define NM_BOND_DEVICE_FACTORY(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), NM_TYPE_BOND_DEVICE_FACTORY, NMBondDeviceFactory))
2014-09-08 10:15:02 -05:00
static NMDevice *
create_device (NMDeviceFactory *factory,
const char *iface,
const NMPlatformLink *plink,
NMConnection *connection,
gboolean *out_ignore)
2014-09-08 10:15:02 -05:00
{
return (NMDevice *) g_object_new (NM_TYPE_DEVICE_BOND,
2016-01-08 16:11:41 +01:00
NM_DEVICE_IFACE, iface,
NM_DEVICE_DRIVER, "bonding",
NM_DEVICE_TYPE_DESC, "Bond",
NM_DEVICE_DEVICE_TYPE, NM_DEVICE_TYPE_BOND,
NM_DEVICE_LINK_TYPE, NM_LINK_TYPE_BOND,
NULL);
2014-09-08 10:15:02 -05:00
}
NM_DEVICE_FACTORY_DEFINE_INTERNAL (BOND, Bond, bond,
NM_DEVICE_FACTORY_DECLARE_LINK_TYPES (NM_LINK_TYPE_BOND)
NM_DEVICE_FACTORY_DECLARE_SETTING_TYPES (NM_SETTING_BOND_SETTING_NAME),
factory_class->create_device = create_device;
);