platform: fetch objects via the event socket

Use the event socket to request object via NLM_F_DUMP.

No longer use 'priv->nlh' socket to fetch objects.
Instead fetch them via the priv->nlh_event socket that also
provides asynchronous events when objects change.

That way, the events are in sync with our explicit requests
and we can directly use the events. Previously, the events were
only used to indicate that a refetch must happen, so that every
event triggered a complete dump of all addresses/routes.

We still use 'priv->nlh' to make synchronous requests such as
adding/changing/deleting objects. That means, after we send a
request, we must make sure that the result manifested itself
at 'nlh_event' socket and the platform cache.
That's why we sometimes still must force a dump to sync changes.
That could be improved by using only one netlink socket so that
we would wait for the ACK of our request.

While not yet perfect, this already significantly reduces the number of
fetches. Additionally, before, whenever requesting a dump of addresses
or routes (which we did much more often, search for "get_kernel_object for type"
log lines), we always dumped IPv4 and IPv6 together. Now only request
the addr-family in question.

https://bugzilla.gnome.org/show_bug.cgi?id=747985
https://bugzilla.redhat.com/show_bug.cgi?id=1211133
This commit is contained in:
Thomas Haller 2015-05-10 10:02:31 +02:00
parent 3377cd7e18
commit 051cf8bbde
5 changed files with 760 additions and 514 deletions

View file

@ -32,6 +32,8 @@
#include "nm-fake-platform.h"
#include "nm-logging.h"
#include "nm-test-utils.h"
#define debug(format, ...) nm_log_dbg (LOGD_PLATFORM, format, __VA_ARGS__)
typedef struct {
@ -50,6 +52,7 @@ typedef struct {
GBytes *address;
int vlan_id;
int ib_p_key;
struct in6_addr ip6_lladdr;
} NMFakePlatformLink;
#define NM_FAKE_PLATFORM_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), NM_TYPE_FAKE_PLATFORM, NMFakePlatformPrivate))
@ -58,6 +61,15 @@ G_DEFINE_TYPE (NMFakePlatform, nm_fake_platform, NM_TYPE_PLATFORM)
/******************************************************************/
static void link_changed (NMPlatform *platform, NMFakePlatformLink *device, gboolean raise_signal);
static gboolean ip6_address_add (NMPlatform *platform, int ifindex,
struct in6_addr addr, struct in6_addr peer_addr,
int plen, guint32 lifetime, guint32 preferred, guint flags);
static gboolean ip6_address_delete (NMPlatform *platform, int ifindex, struct in6_addr addr, int plen);
/******************************************************************/
static gboolean
sysctl_set (NMPlatform *platform, const char *path, const char *value)
{
@ -105,16 +117,21 @@ type_to_type_name (NMLinkType type)
static void
link_init (NMFakePlatformLink *device, int ifindex, int type, const char *name)
{
gs_free char *ip6_lladdr = NULL;
g_assert (!name || strlen (name) < sizeof(device->link.name));
memset (device, 0, sizeof (*device));
ip6_lladdr = ifindex > 0 ? g_strdup_printf ("fe80::fa1e:%0x:%0x", ifindex / 256, ifindex % 256) : NULL;
device->link.ifindex = name ? ifindex : 0;
device->link.type = type;
device->link.kind = type_to_type_name (type);
device->link.driver = type_to_type_name (type);
device->link.udi = device->udi = g_strdup_printf ("fake:%d", ifindex);
device->link.initialized = TRUE;
device->ip6_lladdr = *nmtst_inet6_from_string (ip6_lladdr);
if (name)
strcpy (device->link.name, name);
switch (device->link.type) {
@ -207,9 +224,12 @@ link_add (NMPlatform *platform,
g_array_append_val (priv->links, device);
if (device.link.ifindex)
if (device.link.ifindex) {
g_signal_emit_by_name (platform, NM_PLATFORM_SIGNAL_LINK_CHANGED, device.link.ifindex, &device, NM_PLATFORM_SIGNAL_ADDED, NM_PLATFORM_REASON_INTERNAL);
link_changed (platform, &g_array_index (priv->links, NMFakePlatformLink, priv->links->len - 1), FALSE);
}
if (out_link)
*out_link = device.link;
return TRUE;
@ -305,12 +325,20 @@ link_get_unmanaged (NMPlatform *platform, int ifindex, gboolean *managed)
}
static void
link_changed (NMPlatform *platform, NMFakePlatformLink *device)
link_changed (NMPlatform *platform, NMFakePlatformLink *device, gboolean raise_signal)
{
NMFakePlatformPrivate *priv = NM_FAKE_PLATFORM_GET_PRIVATE (platform);
int i;
g_signal_emit_by_name (platform, NM_PLATFORM_SIGNAL_LINK_CHANGED, device->link.ifindex, &device->link, NM_PLATFORM_SIGNAL_CHANGED, NM_PLATFORM_REASON_INTERNAL);
if (raise_signal)
g_signal_emit_by_name (platform, NM_PLATFORM_SIGNAL_LINK_CHANGED, device->link.ifindex, &device->link, NM_PLATFORM_SIGNAL_CHANGED, NM_PLATFORM_REASON_INTERNAL);
if (device->link.ifindex && !IN6_IS_ADDR_UNSPECIFIED (&device->ip6_lladdr)) {
if (device->link.connected)
ip6_address_add (platform, device->link.ifindex, device->ip6_lladdr, in6addr_any, 64, NM_PLATFORM_LIFETIME_PERMANENT, NM_PLATFORM_LIFETIME_PERMANENT, 0);
else
ip6_address_delete (platform, device->link.ifindex, device->ip6_lladdr, 64);
}
if (device->link.master) {
gboolean connected = FALSE;
@ -328,7 +356,7 @@ link_changed (NMPlatform *platform, NMFakePlatformLink *device)
if (master->link.connected != connected) {
master->link.connected = connected;
link_changed (platform, master);
link_changed (platform, master, TRUE);
}
}
}
@ -362,7 +390,7 @@ link_set_up (NMPlatform *platform, int ifindex)
|| device->link.connected != connected) {
device->link.up = up;
device->link.connected = connected;
link_changed (platform, device);
link_changed (platform, device, TRUE);
}
return TRUE;
@ -380,7 +408,7 @@ link_set_down (NMPlatform *platform, int ifindex)
device->link.up = FALSE;
device->link.connected = FALSE;
link_changed (platform, device);
link_changed (platform, device, TRUE);
}
return TRUE;
@ -396,7 +424,7 @@ link_set_arp (NMPlatform *platform, int ifindex)
device->link.arp = TRUE;
link_changed (platform, device);
link_changed (platform, device, TRUE);
return TRUE;
}
@ -411,7 +439,7 @@ link_set_noarp (NMPlatform *platform, int ifindex)
device->link.arp = FALSE;
link_changed (platform, device);
link_changed (platform, device, TRUE);
return TRUE;
}
@ -450,7 +478,7 @@ link_set_address (NMPlatform *platform, int ifindex, gconstpointer addr, size_t
device->address = g_bytes_new (addr, len);
link_changed (platform, link_get (platform, ifindex));
link_changed (platform, link_get (platform, ifindex), TRUE);
return TRUE;
}
@ -482,7 +510,7 @@ link_set_mtu (NMPlatform *platform, int ifindex, guint32 mtu)
if (device) {
device->link.mtu = mtu;
link_changed (platform, device);
link_changed (platform, device, TRUE);
}
return !!device;
@ -592,7 +620,7 @@ link_enslave (NMPlatform *platform, int master, int slave)
device->link.connected = TRUE;
}
link_changed (platform, device);
link_changed (platform, device, TRUE);
}
return TRUE;
@ -614,8 +642,8 @@ link_release (NMPlatform *platform, int master_idx, int slave_idx)
slave->link.master = 0;
link_changed (platform, slave);
link_changed (platform, master);
link_changed (platform, slave, TRUE);
link_changed (platform, master, TRUE);
return TRUE;
}
@ -1005,8 +1033,10 @@ ip6_address_add (NMPlatform *platform, int ifindex,
if (item->plen != address.plen)
continue;
memcpy (item, &address, sizeof (address));
g_signal_emit_by_name (platform, NM_PLATFORM_SIGNAL_IP6_ADDRESS_CHANGED, ifindex, &address, NM_PLATFORM_SIGNAL_CHANGED, NM_PLATFORM_REASON_INTERNAL);
if (nm_platform_ip6_address_cmp (item, &address) != 0) {
memcpy (item, &address, sizeof (address));
g_signal_emit_by_name (platform, NM_PLATFORM_SIGNAL_IP6_ADDRESS_CHANGED, ifindex, &address, NM_PLATFORM_SIGNAL_CHANGED, NM_PLATFORM_REASON_INTERNAL);
}
return TRUE;
}

File diff suppressed because it is too large Load diff

View file

@ -58,7 +58,7 @@ test_cleanup_internal (void)
routes6 = nm_platform_ip6_route_get_all (NM_PLATFORM_GET, ifindex, NM_PLATFORM_GET_ROUTE_MODE_ALL);
g_assert_cmpint (addresses4->len, ==, 1);
g_assert_cmpint (addresses6->len, ==, 1);
g_assert_cmpint (addresses6->len, ==, 2); /* also has a IPv6 LL address. */
g_assert_cmpint (routes4->len, ==, 3);
g_assert_cmpint (routes6->len, ==, 3);

View file

@ -172,7 +172,7 @@ test_slave (int master, int type, SignalData *master_changed)
g_assert (nm_platform_link_enslave (NM_PLATFORM_GET, master, ifindex)); no_error ();
g_assert_cmpint (nm_platform_link_get_master (NM_PLATFORM_GET, ifindex), ==, master); no_error ();
accept_signal (link_changed);
accept_signals (link_changed, 1, 3);
accept_signals (master_changed, 0, 1);
/* enslaveing brings put the slave */
@ -184,7 +184,7 @@ test_slave (int master, int type, SignalData *master_changed)
/* Set master up */
g_assert (nm_platform_link_set_up (NM_PLATFORM_GET, master));
g_assert (nm_platform_link_is_up (NM_PLATFORM_GET, master));
accept_signal (master_changed);
accept_signals (master_changed, 1, 2);
/* Master with a disconnected slave is disconnected
*
@ -222,7 +222,7 @@ test_slave (int master, int type, SignalData *master_changed)
g_assert (nm_platform_link_set_up (NM_PLATFORM_GET, ifindex)); no_error ();
g_assert (nm_platform_link_is_connected (NM_PLATFORM_GET, ifindex));
g_assert (nm_platform_link_is_connected (NM_PLATFORM_GET, master));
accept_signal (link_changed);
accept_signals (link_changed, 1, 3);
/* NM running, can cause additional change of addrgenmode */
accept_signals (master_changed, 1, 2);
@ -230,8 +230,9 @@ test_slave (int master, int type, SignalData *master_changed)
*
* Gracefully succeed if already enslaved.
*/
g_assert (nm_platform_link_enslave (NM_PLATFORM_GET, master, ifindex)); no_error ();
ensure_no_signal (link_changed);
g_assert (nm_platform_link_enslave (NM_PLATFORM_GET, master, ifindex)); no_error ();
accept_signals (link_changed, 0, 2);
ensure_no_signal (master_changed);
/* Set slave option */
@ -251,21 +252,30 @@ test_slave (int master, int type, SignalData *master_changed)
}
/* Release */
ensure_no_signal (link_changed);
g_assert (nm_platform_link_release (NM_PLATFORM_GET, master, ifindex));
g_assert_cmpint (nm_platform_link_get_master (NM_PLATFORM_GET, ifindex), ==, 0); no_error ();
accept_signal (link_changed);
accept_signals (link_changed, 1, 3);
if (link_type != NM_LINK_TYPE_TEAM)
accept_signals (master_changed, 1, 2);
else
accept_signals (master_changed, 1, 1);
ensure_no_signal (master_changed);
/* Release again */
ensure_no_signal (link_changed);
g_assert (!nm_platform_link_release (NM_PLATFORM_GET, master, ifindex));
error (NM_PLATFORM_ERROR_NOT_SLAVE);
ensure_no_signal (master_changed);
/* Remove */
ensure_no_signal (link_changed);
g_assert (nm_platform_link_delete (NM_PLATFORM_GET, ifindex));
no_error ();
accept_signals (master_changed, 0, 1);
accept_signals (link_changed, 0, 1);
accept_signal (link_removed);
free_signal (link_added);
@ -309,7 +319,7 @@ test_software (NMLinkType link_type, const char *link_typename)
g_assert (nm_platform_link_uses_arp (NM_PLATFORM_GET, ifindex));
g_assert (nm_platform_link_set_noarp (NM_PLATFORM_GET, ifindex));
g_assert (!nm_platform_link_uses_arp (NM_PLATFORM_GET, ifindex));
accept_signal (link_changed);
accept_signals (link_changed, 1, 2);
g_assert (nm_platform_link_set_arp (NM_PLATFORM_GET, ifindex));
g_assert (nm_platform_link_uses_arp (NM_PLATFORM_GET, ifindex));
accept_signal (link_changed);
@ -353,6 +363,7 @@ test_software (NMLinkType link_type, const char *link_typename)
default:
break;
}
free_signal (link_changed);
/* Delete */
g_assert (nm_platform_link_delete (NM_PLATFORM_GET, ifindex));
@ -379,7 +390,6 @@ test_software (NMLinkType link_type, const char *link_typename)
/* No pending signal */
free_signal (link_added);
free_signal (link_changed);
free_signal (link_removed);
}
@ -543,20 +553,13 @@ test_external (void)
wait_signal (link_changed);
g_assert (!nm_platform_link_is_up (NM_PLATFORM_GET, ifindex));
g_assert (!nm_platform_link_is_connected (NM_PLATFORM_GET, ifindex));
/* This test doesn't trigger a netlink event at least on
* 3.8.2-206.fc18.x86_64. Disabling the waiting and checking code
* because of that.
*/
run_command ("ip link set %s arp on", DEVICE_NAME);
#if 0
wait_signal (link_changed);
g_assert (nm_platform_link_uses_arp (ifindex));
#endif
g_assert (nm_platform_link_uses_arp (NM_PLATFORM_GET, ifindex));
run_command ("ip link set %s arp off", DEVICE_NAME);
#if 0
wait_signal (link_changed);
g_assert (!nm_platform_link_uses_arp (ifindex));
#endif
g_assert (!nm_platform_link_uses_arp (NM_PLATFORM_GET, ifindex));
run_command ("ip link del %s", DEVICE_NAME);
wait_signal (link_removed);

View file

@ -417,24 +417,6 @@
# libnl3
###############################################################
{
# fixed by https://github.com/thom311/libnl/commit/d65c32a7205e679c7fc13f0e4565b13e698ba906
libnl_rtnl_link_set_type_01
Memcheck:Leak
match-leak-kinds: definite
fun:calloc
fun:vlan_alloc
fun:rtnl_link_set_type
fun:link_msg_parser
fun:__pickup_answer
fun:nl_cb_call
fun:recvmsgs
fun:nl_recvmsgs_report
fun:nl_recvmsgs
fun:nl_pickup
fun:rtnl_link_get_kernel
...
}
{
# fixed by https://github.com/thom311/libnl/commit/d65c32a7205e679c7fc13f0e4565b13e698ba906
# Same issue as libnl_rtnl_link_set_type_01, but different backtrace by calling nl_msg_parse().