lldp: only have GHashTable instance for LLDP neighbors when running

When the instance is not running (after creation or after stop), there
is no need to keep the GHashTable around.

Create it when needed (during start) and clear it during stop. This
makes it slightly cheaper to keep a NMLldpListener instance around,
if it's currently not running.

NMDevice already keeps the NMLldpListener around, even after stopping
it. It's not clear whether the instance will be started again, so also
clear the GHashTable. Also, one effect is that if you initially were in
a network with many LLDP neibors, after stop and start, the GHashTable
now gets recreated and may not need to allocate a large internal array
as before.
This commit is contained in:
Thomas Haller
2020-06-08 13:28:24 +02:00
parent a7aa2a5e01
commit 0d41abea2e
2 changed files with 14 additions and 13 deletions

View File

@@ -3845,7 +3845,8 @@ nm_device_update_dynamic_ip_setup (NMDevice *self)
/* FIXME: todo */ /* FIXME: todo */
} }
if (priv->lldp_listener && nm_lldp_listener_is_running (priv->lldp_listener)) { if ( priv->lldp_listener
&& nm_lldp_listener_is_running (priv->lldp_listener)) {
nm_lldp_listener_stop (priv->lldp_listener); nm_lldp_listener_stop (priv->lldp_listener);
if (!nm_lldp_listener_start (priv->lldp_listener, nm_device_get_ifindex (self), &error)) { if (!nm_lldp_listener_start (priv->lldp_listener, nm_device_get_ifindex (self), &error)) {
_LOGD (LOGD_DEVICE, "LLDP listener %p could not be restarted: %s", _LOGD (LOGD_DEVICE, "LLDP listener %p could not be restarted: %s",
@@ -7157,7 +7158,8 @@ lldp_init (NMDevice *self, gboolean restart)
gs_free_error GError *error = NULL; gs_free_error GError *error = NULL;
if (priv->lldp_listener) { if (priv->lldp_listener) {
if (restart && nm_lldp_listener_is_running (priv->lldp_listener)) if ( restart
&& nm_lldp_listener_is_running (priv->lldp_listener))
nm_lldp_listener_stop (priv->lldp_listener); nm_lldp_listener_stop (priv->lldp_listener);
} else { } else {
priv->lldp_listener = nm_lldp_listener_new (); priv->lldp_listener = nm_lldp_listener_new ();

View File

@@ -30,16 +30,15 @@ NM_GOBJECT_PROPERTIES_DEFINE (NMLldpListener,
); );
typedef struct { typedef struct {
char *iface;
int ifindex;
sd_lldp *lldp_handle; sd_lldp *lldp_handle;
GHashTable *lldp_neighbors; GHashTable *lldp_neighbors;
GVariant *variant;
/* the timestamp in nsec until which we delay updates. */ /* the timestamp in nsec until which we delay updates. */
gint64 ratelimit_next_nsec; gint64 ratelimit_next_nsec;
guint ratelimit_id; guint ratelimit_id;
GVariant *variant; int ifindex;
} NMLldpListenerPrivate; } NMLldpListenerPrivate;
struct _NMLldpListener { struct _NMLldpListener {
@@ -724,6 +723,8 @@ process_lldp_neighbor (NMLldpListener *self, sd_lldp_neighbor *neighbor_sd, gboo
g_return_if_fail (priv->lldp_handle); g_return_if_fail (priv->lldp_handle);
g_return_if_fail (neighbor_sd); g_return_if_fail (neighbor_sd);
nm_assert (priv->lldp_neighbors);
p_parse_error = _LOGT_ENABLED () ? &parse_error : NULL; p_parse_error = _LOGT_ENABLED () ? &parse_error : NULL;
neigh = lldp_neighbor_new (neighbor_sd, p_parse_error); neigh = lldp_neighbor_new (neighbor_sd, p_parse_error);
@@ -831,6 +832,10 @@ nm_lldp_listener_start (NMLldpListener *self, int ifindex, GError **error)
goto err; goto err;
} }
priv->lldp_neighbors = g_hash_table_new_full (lldp_neighbor_id_hash,
lldp_neighbor_id_equal,
(GDestroyNotify) lldp_neighbor_free, NULL);
_LOGD ("start"); _LOGD ("start");
return TRUE; return TRUE;
@@ -863,6 +868,7 @@ nm_lldp_listener_stop (NMLldpListener *self)
size = g_hash_table_size (priv->lldp_neighbors); size = g_hash_table_size (priv->lldp_neighbors);
g_hash_table_remove_all (priv->lldp_neighbors); g_hash_table_remove_all (priv->lldp_neighbors);
nm_clear_pointer (&priv->lldp_neighbors, g_hash_table_unref);
if ( size > 0 if ( size > 0
|| priv->ratelimit_id != 0) || priv->ratelimit_id != 0)
changed = TRUE; changed = TRUE;
@@ -897,8 +903,8 @@ nm_lldp_listener_get_neighbors (NMLldpListener *self)
priv = NM_LLDP_LISTENER_GET_PRIVATE (self); priv = NM_LLDP_LISTENER_GET_PRIVATE (self);
if (G_UNLIKELY (!priv->variant)) { if (G_UNLIKELY (!priv->variant)) {
GVariantBuilder array_builder;
gs_free LldpNeighbor **neighbors = NULL; gs_free LldpNeighbor **neighbors = NULL;
GVariantBuilder array_builder;
guint i, n; guint i, n;
g_variant_builder_init (&array_builder, G_VARIANT_TYPE ("aa{sv}")); g_variant_builder_init (&array_builder, G_VARIANT_TYPE ("aa{sv}"));
@@ -932,12 +938,6 @@ get_property (GObject *object, guint prop_id,
static void static void
nm_lldp_listener_init (NMLldpListener *self) nm_lldp_listener_init (NMLldpListener *self)
{ {
NMLldpListenerPrivate *priv = NM_LLDP_LISTENER_GET_PRIVATE (self);
priv->lldp_neighbors = g_hash_table_new_full (lldp_neighbor_id_hash,
lldp_neighbor_id_equal,
(GDestroyNotify) lldp_neighbor_free, NULL);
_LOGT ("lldp listener created"); _LOGT ("lldp listener created");
} }
@@ -962,7 +962,6 @@ finalize (GObject *object)
NMLldpListenerPrivate *priv = NM_LLDP_LISTENER_GET_PRIVATE (self); NMLldpListenerPrivate *priv = NM_LLDP_LISTENER_GET_PRIVATE (self);
nm_lldp_listener_stop (self); nm_lldp_listener_stop (self);
g_hash_table_unref (priv->lldp_neighbors);
nm_clear_g_variant (&priv->variant); nm_clear_g_variant (&priv->variant);