NetworkManager/src/core/devices/nm-device-infiniband.c

Ignoring revisions in .git-blame-ignore-revs. Click here to bypass and see the normal blame view.

491 lines
17 KiB
C
Raw Normal View History

/* SPDX-License-Identifier: GPL-2.0-or-later */
/*
* Copyright (C) 2011 - 2018 Red Hat, Inc.
*/
#include "src/core/nm-default-daemon.h"
#include "nm-device-infiniband.h"
#include <linux/if.h>
#include <linux/if_infiniband.h>
#include "NetworkManagerUtils.h"
#include "nm-device-private.h"
#include "nm-act-request.h"
#include "libnm-platform/nm-platform.h"
#include "nm-device-factory.h"
#include "libnm-core-aux-intern/nm-libnm-core-utils.h"
#include "libnm-core-intern/nm-core-internal.h"
#define NM_DEVICE_INFINIBAND_IS_PARTITION "is-partition"
/*****************************************************************************/
NM_GOBJECT_PROPERTIES_DEFINE_BASE(PROP_IS_PARTITION, );
typedef struct {
gboolean is_partition;
int parent_ifindex;
int p_key;
} NMDeviceInfinibandPrivate;
struct _NMDeviceInfiniband {
NMDevice parent;
NMDeviceInfinibandPrivate _priv;
};
struct _NMDeviceInfinibandClass {
NMDeviceClass parent;
};
G_DEFINE_TYPE(NMDeviceInfiniband, nm_device_infiniband, NM_TYPE_DEVICE)
#define NM_DEVICE_INFINIBAND_GET_PRIVATE(self) \
_NM_GET_PRIVATE(self, NMDeviceInfiniband, NM_IS_DEVICE_INFINIBAND, NMDevice)
/*****************************************************************************/
static NMDeviceCapabilities
get_generic_capabilities(NMDevice *device)
{
guint32 caps = NM_DEVICE_CAP_CARRIER_DETECT;
if (NM_DEVICE_INFINIBAND_GET_PRIVATE(device)->is_partition)
caps |= NM_DEVICE_CAP_IS_SOFTWARE;
return caps;
}
static NMActStageReturn
act_stage1_prepare(NMDevice *device, NMDeviceStateReason *out_failure_reason)
{
nm_auto_close int dirfd = -1;
NMSettingInfiniband *s_infiniband;
char ifname_verified[IFNAMSIZ];
const char *transport_mode;
gboolean ok;
s_infiniband = nm_device_get_applied_setting(device, NM_TYPE_SETTING_INFINIBAND);
g_return_val_if_fail(s_infiniband, NM_ACT_STAGE_RETURN_FAILURE);
transport_mode = nm_setting_infiniband_get_transport_mode(s_infiniband);
dirfd = nm_platform_sysctl_open_netdir(nm_device_get_platform(device),
nm_device_get_ifindex(device),
ifname_verified);
if (dirfd < 0) {
2019-08-22 07:41:34 +02:00
if (nm_streq(transport_mode, "datagram"))
return NM_ACT_STAGE_RETURN_SUCCESS;
else {
NM_SET_OUT(out_failure_reason, NM_DEVICE_STATE_REASON_INFINIBAND_MODE);
return NM_ACT_STAGE_RETURN_FAILURE;
}
}
/* With some drivers the interface must be down to set transport mode */
nm_device_take_down(device, TRUE);
ok = nm_platform_sysctl_set(nm_device_get_platform(device),
NMP_SYSCTL_PATHID_NETDIR(dirfd, ifname_verified, "mode"),
transport_mode);
nm_device_bring_up(device, TRUE, NULL);
if (!ok) {
NM_SET_OUT(out_failure_reason, NM_DEVICE_STATE_REASON_CONFIG_FAILED);
return NM_ACT_STAGE_RETURN_FAILURE;
}
return NM_ACT_STAGE_RETURN_SUCCESS;
}
static guint32
get_configured_mtu(NMDevice *device, NMDeviceMtuSource *out_source, gboolean *out_force)
{
return nm_device_get_configured_mtu_from_connection(device,
NM_TYPE_SETTING_INFINIBAND,
out_source);
}
static gboolean
check_connection_compatible(NMDevice *device, NMConnection *connection, GError **error)
{
NMSettingInfiniband *s_infiniband;
if (!NM_DEVICE_CLASS(nm_device_infiniband_parent_class)
->check_connection_compatible(device, connection, error))
return FALSE;
if (nm_device_is_real(device)) {
const char *mac;
const char *hw_addr;
s_infiniband = nm_connection_get_setting_infiniband(connection);
mac = nm_setting_infiniband_get_mac_address(s_infiniband);
if (mac) {
hw_addr = nm_device_get_permanent_hw_address(device);
if (!hw_addr || !nm_utils_hwaddr_matches(mac, -1, hw_addr, -1)) {
nm_utils_error_set_literal(error,
NM_UTILS_ERROR_CONNECTION_AVAILABLE_TEMPORARY,
"MAC address mismatches");
return FALSE;
}
}
}
return TRUE;
}
static gboolean
complete_connection(NMDevice *device,
NMConnection *connection,
const char *specific_object,
NMConnection *const *existing_connections,
GError **error)
{
NMSettingInfiniband *s_infiniband;
s_infiniband = _nm_connection_ensure_setting(connection, NM_TYPE_SETTING_INFINIBAND);
nm_utils_complete_generic(
nm_device_get_platform(device),
connection,
NM_SETTING_INFINIBAND_SETTING_NAME,
existing_connections,
NULL,
_("InfiniBand connection"),
NULL,
nm_setting_infiniband_get_mac_address(s_infiniband) ? NULL : nm_device_get_iface(device),
TRUE);
if (!nm_setting_infiniband_get_transport_mode(s_infiniband))
g_object_set(G_OBJECT(s_infiniband),
NM_SETTING_INFINIBAND_TRANSPORT_MODE,
"datagram",
NULL);
return TRUE;
}
static void
update_connection(NMDevice *device, NMConnection *connection)
{
NMSettingInfiniband *s_infiniband =
_nm_connection_ensure_setting(connection, NM_TYPE_SETTING_INFINIBAND);
const char *mac = nm_device_get_permanent_hw_address(device);
const char *transport_mode = "datagram";
int ifindex;
if (mac && !nm_utils_hwaddr_matches(mac, -1, NULL, INFINIBAND_ALEN))
g_object_set(s_infiniband, NM_SETTING_INFINIBAND_MAC_ADDRESS, mac, NULL);
ifindex = nm_device_get_ifindex(device);
if (ifindex > 0) {
if (!nm_platform_link_infiniband_get_properties(nm_device_get_platform(device),
ifindex,
NULL,
NULL,
&transport_mode))
transport_mode = "datagram";
}
g_object_set(G_OBJECT(s_infiniband),
NM_SETTING_INFINIBAND_TRANSPORT_MODE,
transport_mode,
NULL);
}
static gboolean
can_reapply_change(NMDevice *device,
const char *setting_name,
NMSetting *s_old,
NMSetting *s_new,
GHashTable *diffs,
GError **error)
{
NMDeviceClass *device_class;
if (nm_streq(setting_name, NM_SETTING_INFINIBAND_SETTING_NAME)) {
return nm_device_hash_check_invalid_keys(
diffs,
NM_SETTING_INFINIBAND_SETTING_NAME,
error,
NM_SETTING_INFINIBAND_MTU); /* reapplied with IP config */
}
device_class = NM_DEVICE_CLASS(nm_device_infiniband_parent_class);
return device_class->can_reapply_change(device, setting_name, s_old, s_new, diffs, error);
}
static gboolean
create_and_realize(NMDevice *device,
NMConnection *connection,
NMDevice *parent,
const NMPlatformLink **out_plink,
GError **error)
{
NMDeviceInfinibandPrivate *priv = NM_DEVICE_INFINIBAND_GET_PRIVATE(device);
NMSettingInfiniband *s_infiniband;
int r;
s_infiniband = nm_connection_get_setting_infiniband(connection);
g_assert(s_infiniband);
/* Can only create partitions at this time */
priv->p_key = nm_setting_infiniband_get_p_key(s_infiniband);
if (priv->p_key < 0) {
g_set_error_literal(error,
NM_DEVICE_ERROR,
NM_DEVICE_ERROR_FAILED,
"only InfiniBand partitions can be created");
return FALSE;
}
device: check for a parent device The device creation can be attempted if the name can be determined. It alone is doesn't mean that there's a parent device -- the name could just have been hardcoded in the connection. NetworkManager[21519]: nm_device_get_ifindex: assertion 'NM_IS_DEVICE (self)' failed Program received signal SIGTRAP, Trace/breakpoint trap. g_logv (log_domain=0x5555557fb2e5 "NetworkManager", log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>, args=args@entry=0x7fffffffd3d0) at gmessages.c:1046 1046 g_private_set (&g_log_depth, GUINT_TO_POINTER (depth)); (gdb) bt #0 0x00007ffff4ec88c3 in g_logv (log_domain=0x5555557fb2e5 "NetworkManager", log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>, args=args@entry=0x7fffffffd3d0) at gmessages.c:1046 #1 0x00007ffff4ec8a3f in g_log (log_domain=<optimized out>, log_level=<optimized out>, format=<optimized out>) at gmessages.c:1079 #2 0x00005555555d2090 in nm_device_get_ifindex (self=0x0) at devices/nm-device.c:562 #3 0x00005555555ef77a in nm_device_supports_vlans (self=0x0) at devices/nm-device.c:9865 #4 0x00005555555bf2f9 in create_and_realize (device=0x555555c549b0 [NMDeviceVlan], connection=0x555555b451e0, parent=0x0, out_plink=0x7fffffffd5f8, error=0x7fffffffd700) at devices/nm-device-vlan.c:225 #5 0x00005555555d5757 in nm_device_create_and_realize (self=0x555555c549b0 [NMDeviceVlan], connection=0x555555b451e0, parent=0x0, error=0x7fffffffd700) at devices/nm-device.c:1783 #6 0x0000555555688601 in system_create_virtual_device (self=0x555555af51c0 [NMManager], connection=0x555555b451e0) at nm-manager.c:1120 #7 0x000055555568894e in connection_changed (settings=0x555555ae8220 [NMSettings], connection=0x555555b451e0, manager=0x555555af51c0 [NMManager]) at nm-manager.c:1172 #8 0x0000555555693448 in nm_manager_start (self=0x555555af51c0 [NMManager], error=0x7fffffffda30) at nm-manager.c:4466 #9 0x00005555555d166f in main (argc=1, argv=0x7fffffffdba8) at main.c:454 (gdb) Fixes: 332994f1b19ded7cb343ef573443916bab05aaec
2016-04-19 12:02:02 +02:00
if (!parent) {
g_set_error(error,
NM_DEVICE_ERROR,
NM_DEVICE_ERROR_MISSING_DEPENDENCIES,
device: check for a parent device The device creation can be attempted if the name can be determined. It alone is doesn't mean that there's a parent device -- the name could just have been hardcoded in the connection. NetworkManager[21519]: nm_device_get_ifindex: assertion 'NM_IS_DEVICE (self)' failed Program received signal SIGTRAP, Trace/breakpoint trap. g_logv (log_domain=0x5555557fb2e5 "NetworkManager", log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>, args=args@entry=0x7fffffffd3d0) at gmessages.c:1046 1046 g_private_set (&g_log_depth, GUINT_TO_POINTER (depth)); (gdb) bt #0 0x00007ffff4ec88c3 in g_logv (log_domain=0x5555557fb2e5 "NetworkManager", log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>, args=args@entry=0x7fffffffd3d0) at gmessages.c:1046 #1 0x00007ffff4ec8a3f in g_log (log_domain=<optimized out>, log_level=<optimized out>, format=<optimized out>) at gmessages.c:1079 #2 0x00005555555d2090 in nm_device_get_ifindex (self=0x0) at devices/nm-device.c:562 #3 0x00005555555ef77a in nm_device_supports_vlans (self=0x0) at devices/nm-device.c:9865 #4 0x00005555555bf2f9 in create_and_realize (device=0x555555c549b0 [NMDeviceVlan], connection=0x555555b451e0, parent=0x0, out_plink=0x7fffffffd5f8, error=0x7fffffffd700) at devices/nm-device-vlan.c:225 #5 0x00005555555d5757 in nm_device_create_and_realize (self=0x555555c549b0 [NMDeviceVlan], connection=0x555555b451e0, parent=0x0, error=0x7fffffffd700) at devices/nm-device.c:1783 #6 0x0000555555688601 in system_create_virtual_device (self=0x555555af51c0 [NMManager], connection=0x555555b451e0) at nm-manager.c:1120 #7 0x000055555568894e in connection_changed (settings=0x555555ae8220 [NMSettings], connection=0x555555b451e0, manager=0x555555af51c0 [NMManager]) at nm-manager.c:1172 #8 0x0000555555693448 in nm_manager_start (self=0x555555af51c0 [NMManager], error=0x7fffffffda30) at nm-manager.c:4466 #9 0x00005555555d166f in main (argc=1, argv=0x7fffffffdba8) at main.c:454 (gdb) Fixes: 332994f1b19ded7cb343ef573443916bab05aaec
2016-04-19 12:02:02 +02:00
"InfiniBand partitions can not be created without a parent interface");
return FALSE;
}
if (!NM_IS_DEVICE_INFINIBAND(parent)) {
g_set_error(error,
NM_DEVICE_ERROR,
NM_DEVICE_ERROR_MISSING_DEPENDENCIES,
"Parent interface %s must be an InfiniBand interface",
nm_device_get_iface(parent));
return FALSE;
}
priv->parent_ifindex = nm_device_get_ifindex(parent);
if (priv->parent_ifindex <= 0) {
g_set_error(error,
NM_DEVICE_ERROR,
NM_DEVICE_ERROR_MISSING_DEPENDENCIES,
"failed to get InfiniBand parent %s ifindex",
nm_device_get_iface(parent));
return FALSE;
}
r = nm_platform_link_infiniband_add(nm_device_get_platform(device),
priv->parent_ifindex,
priv->p_key,
out_plink);
if (r < 0) {
g_set_error(error,
NM_DEVICE_ERROR,
NM_DEVICE_ERROR_CREATION_FAILED,
"Failed to create InfiniBand P_Key interface '%s' for '%s': %s",
nm_device_get_iface(device),
nm_connection_get_id(connection),
nm_strerror(r));
return FALSE;
}
priv->is_partition = TRUE;
return TRUE;
}
static gboolean
unrealize(NMDevice *device, GError **error)
{
NMDeviceInfinibandPrivate *priv;
int r;
g_return_val_if_fail(NM_IS_DEVICE_INFINIBAND(device), FALSE);
priv = NM_DEVICE_INFINIBAND_GET_PRIVATE(device);
if (priv->p_key < 0) {
g_set_error(error,
NM_DEVICE_ERROR,
NM_DEVICE_ERROR_FAILED,
"Only InfiniBand partitions can be removed");
return FALSE;
}
r = nm_platform_link_infiniband_delete(nm_device_get_platform(device),
priv->parent_ifindex,
priv->p_key);
if (r < 0) {
g_set_error(error,
NM_DEVICE_ERROR,
NM_DEVICE_ERROR_CREATION_FAILED,
"Failed to remove InfiniBand P_Key interface '%s': %s",
nm_device_get_iface(device),
nm_strerror(r));
return FALSE;
}
return TRUE;
}
/*****************************************************************************/
static void
get_property(GObject *object, guint prop_id, GValue *value, GParamSpec *pspec)
{
switch (prop_id) {
case PROP_IS_PARTITION:
g_value_set_boolean(value, NM_DEVICE_INFINIBAND_GET_PRIVATE(object)->is_partition);
break;
default:
G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
break;
}
}
static void
set_property(GObject *object, guint prop_id, const GValue *value, GParamSpec *pspec)
{
switch (prop_id) {
case PROP_IS_PARTITION:
NM_DEVICE_INFINIBAND_GET_PRIVATE(object)->is_partition = g_value_get_boolean(value);
break;
default:
G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
break;
}
}
/*****************************************************************************/
static void
nm_device_infiniband_init(NMDeviceInfiniband *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_infiniband = {
.parent = NM_DEFINE_GDBUS_INTERFACE_INFO_INIT(
NM_DBUS_INTERFACE_DEVICE_INFINIBAND,
.properties = NM_DEFINE_GDBUS_PROPERTY_INFOS(
NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE("HwAddress", "s", NM_DEVICE_HW_ADDRESS),
NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE("Carrier", "b", NM_DEVICE_CARRIER), ), ),
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 void
nm_device_infiniband_class_init(NMDeviceInfinibandClass *klass)
{
GObjectClass *object_class = G_OBJECT_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
NMDBusObjectClass *dbus_object_class = NM_DBUS_OBJECT_CLASS(klass);
NMDeviceClass *device_class = NM_DEVICE_CLASS(klass);
object_class->get_property = get_property;
object_class->set_property = set_property;
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_infiniband);
device_class->connection_type_supported = NM_SETTING_INFINIBAND_SETTING_NAME;
device_class->connection_type_check_compatible = NM_SETTING_INFINIBAND_SETTING_NAME;
device_class->link_types = NM_DEVICE_DEFINE_LINK_TYPES(NM_LINK_TYPE_INFINIBAND);
device_class->can_reapply_change = can_reapply_change;
device_class->create_and_realize = create_and_realize;
device_class->unrealize = unrealize;
device_class->get_generic_capabilities = get_generic_capabilities;
device_class->check_connection_compatible = check_connection_compatible;
device_class->complete_connection = complete_connection;
device_class->update_connection = update_connection;
device_class->act_stage1_prepare = act_stage1_prepare;
device_class->get_configured_mtu = get_configured_mtu;
obj_properties[PROP_IS_PARTITION] =
g_param_spec_boolean(NM_DEVICE_INFINIBAND_IS_PARTITION,
"",
"",
FALSE,
G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY | G_PARAM_STATIC_STRINGS);
g_object_class_install_properties(object_class, _PROPERTY_ENUMS_LAST, obj_properties);
}
/*****************************************************************************/
#define NM_TYPE_INFINIBAND_DEVICE_FACTORY (nm_infiniband_device_factory_get_type())
#define NM_INFINIBAND_DEVICE_FACTORY(obj) \
(G_TYPE_CHECK_INSTANCE_CAST((obj), \
NM_TYPE_INFINIBAND_DEVICE_FACTORY, \
NMInfinibandDeviceFactory))
static NMDevice *
create_device(NMDeviceFactory *factory,
const char *iface,
const NMPlatformLink *plink,
NMConnection *connection,
gboolean *out_ignore)
{
gboolean is_partition = FALSE;
if (plink)
is_partition = (plink->parent > 0 || plink->parent == NM_PLATFORM_LINK_OTHER_NETNS);
else if (connection) {
NMSettingInfiniband *s_infiniband;
s_infiniband = nm_connection_get_setting_infiniband(connection);
g_return_val_if_fail(s_infiniband, NULL);
is_partition = !!nm_setting_infiniband_get_parent(s_infiniband)
|| (nm_setting_infiniband_get_p_key(s_infiniband) >= 0
&& nm_setting_infiniband_get_mac_address(s_infiniband));
}
return g_object_new(NM_TYPE_DEVICE_INFINIBAND,
NM_DEVICE_IFACE,
iface,
NM_DEVICE_TYPE_DESC,
"InfiniBand",
NM_DEVICE_DEVICE_TYPE,
NM_DEVICE_TYPE_INFINIBAND,
NM_DEVICE_LINK_TYPE,
NM_LINK_TYPE_INFINIBAND,
/* NOTE: Partition should probably be a different link type! */
NM_DEVICE_INFINIBAND_IS_PARTITION,
is_partition,
NULL);
}
static const char *
get_connection_parent(NMDeviceFactory *factory, NMConnection *connection)
{
NMSettingInfiniband *s_infiniband;
g_return_val_if_fail(nm_connection_is_type(connection, NM_SETTING_INFINIBAND_SETTING_NAME),
NULL);
s_infiniband = nm_connection_get_setting_infiniband(connection);
g_assert(s_infiniband);
return nm_setting_infiniband_get_parent(s_infiniband);
}
static char *
get_connection_iface(NMDeviceFactory *factory, NMConnection *connection, const char *parent_iface)
{
NMSettingInfiniband *s_infiniband;
g_return_val_if_fail(nm_connection_is_type(connection, NM_SETTING_INFINIBAND_SETTING_NAME),
NULL);
s_infiniband = nm_connection_get_setting_infiniband(connection);
g_assert(s_infiniband);
if (!parent_iface)
return NULL;
g_return_val_if_fail(g_strcmp0(parent_iface, nm_setting_infiniband_get_parent(s_infiniband))
== 0,
NULL);
return g_strdup(nm_setting_infiniband_get_virtual_interface_name(s_infiniband));
}
NM_DEVICE_FACTORY_DEFINE_INTERNAL(
INFINIBAND,
Infiniband,
infiniband,
NM_DEVICE_FACTORY_DECLARE_LINK_TYPES(NM_LINK_TYPE_INFINIBAND)
NM_DEVICE_FACTORY_DECLARE_SETTING_TYPES(NM_SETTING_INFINIBAND_SETTING_NAME),
factory_class->create_device = create_device;
factory_class->get_connection_parent = get_connection_parent;
factory_class->get_connection_iface = get_connection_iface;);