core: reverse the order of active connections in the manager

When a new active connection is created, it gets added at the
beginning of manager's list. This means that the list contains most
recently activated connections first. Since the list is doubly-linked,
it is possible to efficiently iterate in both directions, so the order
of the list is mostly a matter of convention.

I think it is preferable to have oldest active connections at the
beginning of the list; let's reverse the order.

In most places where the list is iterated, the order doesn't
matter. Where it does, use the *_prev() variant to maintain the old
iteration order.
This commit is contained in:
Beniamino Galvani 2020-11-04 13:53:57 +01:00
parent 740191f7c0
commit dc6ec6ce7b
4 changed files with 26 additions and 6 deletions

View file

@ -109,6 +109,7 @@ ForEachMacros: ['c_list_for_each',
'nm_l3_config_data_iter_ip6_route_for_each',
'nm_l3_config_data_iter_obj_for_each',
'nm_manager_for_each_active_connection',
'nm_manager_for_each_active_connection_prev',
'nm_manager_for_each_active_connection_safe',
'nm_manager_for_each_device',
'nm_manager_for_each_device_safe',

View file

@ -142,7 +142,7 @@ typedef struct {
GArray *capabilities;
CList active_connections_lst_head;
CList active_connections_lst_head; /* Oldest ACs at the beginning */
CList async_op_lst_head;
guint ac_cleanup_id;
NMActiveConnection *primary_connection;
@ -941,7 +941,7 @@ active_connection_add(NMManager *self, NMActiveConnection *active)
nm_assert(NM_IS_ACTIVE_CONNECTION(active));
nm_assert(!c_list_is_linked(&active->active_connections_lst));
c_list_link_front(&priv->active_connections_lst_head, &active->active_connections_lst);
c_list_link_tail(&priv->active_connections_lst_head, &active->active_connections_lst);
g_object_ref(active);
g_signal_connect(active,
@ -7867,7 +7867,9 @@ get_property(GObject *object, guint prop_id, GValue *value, GParamSpec *pspec)
break;
case PROP_ACTIVE_CONNECTIONS:
ptrarr = g_ptr_array_new();
c_list_for_each_entry (ac, &priv->active_connections_lst_head, active_connections_lst) {
c_list_for_each_entry_prev (ac,
&priv->active_connections_lst_head,
active_connections_lst) {
path = nm_dbus_object_get_path(NM_DBUS_OBJECT(ac));
if (path)
g_ptr_array_add(ptrarr, g_strdup(path));

View file

@ -72,6 +72,7 @@ NMState nm_manager_get_state(NMManager *manager);
const CList *nm_manager_get_active_connections(NMManager *manager);
/* From least recently activated */
#define nm_manager_for_each_active_connection(manager, iter, tmp_list) \
for (tmp_list = nm_manager_get_active_connections(manager), \
iter = c_list_entry(tmp_list->next, NMActiveConnection, active_connections_lst); \
@ -86,6 +87,22 @@ const CList *nm_manager_get_active_connections(NMManager *manager);
NMActiveConnection, \
active_connections_lst))
/* From most recently activated */
#define nm_manager_for_each_active_connection_prev(manager, iter, tmp_list) \
for (tmp_list = nm_manager_get_active_connections(manager), \
iter = c_list_entry(tmp_list->prev, NMActiveConnection, active_connections_lst); \
({ \
const gboolean _has_prev = (&iter->active_connections_lst != tmp_list); \
\
if (!_has_prev) \
iter = NULL; \
_has_prev; \
}); \
iter = c_list_entry(iter->active_connections_lst.prev, \
NMActiveConnection, \
active_connections_lst))
/* From least recently activated */
#define nm_manager_for_each_active_connection_safe(manager, iter, tmp_list, iter_safe) \
for (tmp_list = nm_manager_get_active_connections(manager), iter_safe = tmp_list->next; ({ \
if (iter_safe != tmp_list) { \

View file

@ -354,11 +354,11 @@ device_ip6_prefix_delegated(NMDevice *device, NMPlatformIP6Address *prefix, gpoi
delegation->device = device;
delegation->prefix = *prefix;
/* The newly activated connections are added to the list beginning,
* so traversing it from the beginning makes it likely for newly
/* The newly activated connections are added to the end of the list,
* so traversing it from the end makes it likely for newly
* activated connections that have no subnet assigned to be served
* first. That is a simple yet fair policy, which is good. */
nm_manager_for_each_active_connection (priv->manager, ac, tmp_list) {
nm_manager_for_each_active_connection_prev (priv->manager, ac, tmp_list) {
NMDevice *to_device;
to_device = nm_active_connection_get_device(ac);