fdo-mirrors/NetworkManager
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/NetworkManager/NetworkManager.git synced 2026-02-04 04:50:30 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

core: remove unused tag-less API from nm_netns_watcher*()

Browse source 
The implementation came with two flavors, where watcher could either
specify a tag or no tag. That resulted in different usage patterns and
behavior.

Handles with tag are indexed by a dictionary and de-duplicated. Also the intended
pattern is to delete them with nm_netns_watcher_remove_all(),
Currently, nm_netns_watcher_remove_handle() was not permissible to tag-full handles,
because of the de-duplication and because handles had no ref-counting
implemented (the latter would be fixable, so
nm_netns_watcher_remove_handle() would be made to work).

On the other hand, handles without tag are never de-duplicated. They are
also not indexed, so nm_netns_watcher_remove_all() doesn't work for
them. They could only be removed via nm_netns_watcher_remove_handle().

Currently, the only user of the API will use tag-full handles. Drop the
unused API. This is done as a separate commit, to potentially revert and
restore tag-less handles (after they were already implemented).
This commit is contained in:
Thomas Haller 2023-03-09 07:45:27 +01:00
parent 6d804b149c
commit 71b2d4c33a
No known key found for this signature in database
GPG key ID: 29C2366E4DFC5728
2 changed files with 49 additions and 108 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

138
src/core/nm-netns.c
View file

@ -39,8 +39,7 @@ struct _NMNetnsWatcherHandle {
NMNetnsWatcherCallback callback;
gpointer callback_user_data;
/* If "tag" is NULL, this element is linked to "priv->watcher_no_tag_lst_head".
* Otherwise, it is linked to "WatcherByTag.watcher_by_tag_lst_head" in
/* This is linked to "WatcherByTag.watcher_by_tag_lst_head" in
* "priv->watcher_by_tag_idx". */
CList watcher_tag_lst;
@ -73,8 +72,7 @@ typedef struct {
GHashTable *ecmp_track_by_obj;
GHashTable *ecmp_track_by_ecmpid;
/* Contains the watchers that have a tag. Those without tag are not indexed
* by a dictionary but linked in "watcher_no_tag_lst_head" list. */
/* Indexes the watcher handles. */
GHashTable *watcher_idx;
/* An index of WatcherByTag. It allows to lookup watcher handles by tag.
@ -85,10 +83,6 @@ typedef struct {
* by IP address. */
GHashTable *watcher_ip_data_idx;
/* NMNetnsWatcherHandle without tag are not indexed in "watcher_idx" nor
* "watcher_by_tag_idx". Those are only linked in this list. */
CList watcher_no_tag_lst_head;
CList l3cfg_signal_pending_lst_head;
GSource *signal_pending_idle_source;
} NMNetnsPrivate;
@ -1227,7 +1221,7 @@ _watcher_unregister_handle(NMNetns *self, NMNetnsWatcherHandle *handle)
nm_assert_not_reached();
}
NMNetnsWatcherHandle *
void
nm_netns_watcher_add(NMNetns *self,
NMNetnsWatcherType watcher_type,
const NMNetnsWatcherData *watcher_data,
@ -1240,51 +1234,45 @@ nm_netns_watcher_add(NMNetns *self,
gboolean is_new = FALSE;
char sbuf[500];
g_return_val_if_fail(NM_IS_NETNS(self), NULL);
g_return_val_if_fail(NM_NETNS_WATCHER_TYPE_VALID(watcher_type), NULL);
g_return_val_if_fail(callback, NULL);
g_return_if_fail(NM_IS_NETNS(self));
g_return_if_fail(NM_NETNS_WATCHER_TYPE_VALID(watcher_type));
g_return_if_fail(callback);
g_return_if_fail(tag);
priv = NM_NETNS_GET_PRIVATE(self);
if (tag)
handle = _watcher_lookup_handle(self, watcher_type, watcher_data, tag);
else
handle = NULL;
handle = _watcher_lookup_handle(self, watcher_type, watcher_data, tag);
if (!handle) {
if (G_UNLIKELY(c_list_is_empty(&priv->watcher_no_tag_lst_head)
&& g_hash_table_size(priv->watcher_idx) == 0))
WatcherByTag *watcher_by_tag;
if (G_UNLIKELY(g_hash_table_size(priv->watcher_idx) == 0))
g_object_ref(self);
handle = g_slice_new(NMNetnsWatcherHandle);
_watcher_handle_init(handle, watcher_type, watcher_data, tag);
if (tag) {
WatcherByTag *watcher_by_tag;
if (!g_hash_table_add(priv->watcher_idx, handle))
nm_assert_not_reached();
if (!g_hash_table_add(priv->watcher_idx, handle))
nm_assert_not_reached();
watcher_by_tag = g_hash_table_lookup(priv->watcher_by_tag_idx, &tag);
watcher_by_tag = g_hash_table_lookup(priv->watcher_by_tag_idx, &tag);
if (!watcher_by_tag) {
watcher_by_tag = g_slice_new(WatcherByTag);
*watcher_by_tag = (WatcherByTag){
.tag = tag,
.watcher_by_tag_lst_head = C_LIST_INIT(watcher_by_tag->watcher_by_tag_lst_head),
};
g_hash_table_add(priv->watcher_by_tag_idx, watcher_by_tag);
}
if (!watcher_by_tag) {
watcher_by_tag = g_slice_new(WatcherByTag);
*watcher_by_tag = (WatcherByTag){
.tag = tag,
.watcher_by_tag_lst_head = C_LIST_INIT(watcher_by_tag->watcher_by_tag_lst_head),
};
g_hash_table_add(priv->watcher_by_tag_idx, watcher_by_tag);
}
c_list_link_tail(&watcher_by_tag->watcher_by_tag_lst_head, &handle->watcher_tag_lst);
} else
c_list_link_tail(&priv->watcher_no_tag_lst_head, &handle->watcher_tag_lst);
c_list_link_tail(&watcher_by_tag->watcher_by_tag_lst_head, &handle->watcher_tag_lst);
is_new = TRUE;
} else {
/* Handles with a tag are deduplicated/shared. Hence it is error prone
* (and likely a bug) to provide different callback/user_data. Such
* usage is rejected here.
/* Handles are deduplicated/shared. Hence it is error prone (and likely
* a bug) to provide different callback/user_data. Such usage is
* rejected here.
*
* This could be made to work, for example by now allowing handles to
* be merged or simply requiring the caller to be careful to not get
@ -1308,27 +1296,16 @@ nm_netns_watcher_add(NMNetns *self,
if (is_new)
_watcher_register_handle(self, handle);
if (tag) {
/* Identical handles with tags are deduplicated (via the
* priv->watchers_idx dictionary). The usage pattern with a tag is to
* use nm_netns_watcher_remove_all(), not to delete them one by one via
* nm_netns_watcher_remove_handle(). Consequently, the handles don't
* have a ref-count implemented, and consequently the user cannot use
* nm_netns_watcher_remove_handle() to remove such handles (because
* nm_netns_watcher_add() might return the same handle multiple times).
*
* For that reason, we return NULL here. The user cannot use such
* handles with a tag and must ignore them
*
* We could extend that by adding a ref-count to the handles, but
* it's unnecessary, for now. */
return NULL;
}
return handle;
/* We cannot return a handle here, because handles are deduplicated via the priv->watchers_idx dictionary.
* The usage pattern is to use nm_netns_watcher_remove_all(), and not remove them one by one.
* As nm_netns_watcher_add() can return the same handle more than once, the user
* wouldn't know when it's safe to call nm_netns_watcher_remove_handle().
*
* This could be extended by adding a ref-count to the handles. But that is not
* used currently, so it's not possible to remove watcher by their handle. */
}
void
static void
nm_netns_watcher_remove_handle(NMNetns *self, NMNetnsWatcherHandle *handle)
{
NMNetnsPrivate *priv;
@ -1336,14 +1313,11 @@ nm_netns_watcher_remove_handle(NMNetns *self, NMNetnsWatcherHandle *handle)
g_return_if_fail(NM_IS_NETNS(self));
g_return_if_fail(handle);
nm_assert(handle->tag);
priv = NM_NETNS_GET_PRIVATE(self);
if (NM_MORE_ASSERTS > 5) {
nm_assert(!handle->tag || g_hash_table_lookup(priv->watcher_idx, handle) == handle);
nm_assert(handle->tag
|| c_list_contains(&priv->watcher_no_tag_lst_head, &handle->watcher_tag_lst));
}
nm_assert(g_hash_table_lookup(priv->watcher_idx, handle) == handle);
_LOGT("netns-watcher: %s %s",
"unregister",
@ -1351,41 +1325,21 @@ nm_netns_watcher_remove_handle(NMNetns *self, NMNetnsWatcherHandle *handle)
_watcher_unregister_handle(self, handle);
if (handle->tag) {
if (!g_hash_table_remove(priv->watcher_idx, handle))
nm_assert_not_reached();
if (!g_hash_table_remove(priv->watcher_idx, handle))
nm_assert_not_reached();
if (c_list_is_empty_or_single(&handle->watcher_tag_lst)) {
if (!g_hash_table_remove(priv->watcher_by_tag_idx, &handle->tag))
nm_assert_not_reached();
}
if (c_list_is_empty_or_single(&handle->watcher_tag_lst)) {
if (!g_hash_table_remove(priv->watcher_by_tag_idx, &handle->tag))
nm_assert_not_reached();
}
c_list_unlink_stale(&handle->watcher_tag_lst);
nm_g_slice_free(handle);
if (G_UNLIKELY(c_list_is_empty(&priv->watcher_no_tag_lst_head)
&& g_hash_table_size(priv->watcher_idx) == 0))
if (G_UNLIKELY(g_hash_table_size(priv->watcher_idx) == 0))
g_object_unref(self);
}
void
nm_netns_watcher_remove(NMNetns *self,
NMNetnsWatcherType watcher_type,
const NMNetnsWatcherData *watcher_data,
gconstpointer tag)
{
NMNetnsWatcherHandle *handle;
g_return_if_fail(NM_IS_NETNS(self));
g_return_if_fail(NM_NETNS_WATCHER_TYPE_VALID(watcher_type));
g_return_if_fail(tag);
handle = _watcher_lookup_handle(self, watcher_type, watcher_data, tag);
if (handle)
nm_netns_watcher_remove_handle(self, handle);
}
void
nm_netns_watcher_remove_all(NMNetns *self, gconstpointer tag, gboolean all)
{
@ -1399,11 +1353,7 @@ nm_netns_watcher_remove_all(NMNetns *self, gconstpointer tag, gboolean all)
/* remove-all only works with handles that have a tag associated.
* Since NMNetns can have multiple users that are unknown to each
* other, it makes no sense to have a remove-all function which
* would remove all of them.
*
* Note that if your watch handle has no tag, the only way to
* remove it is by tracking the handle returned from nm_netns_watcher_add()
* and use it with nm_netns_watcher_remove_handle(). */
* would remove all of them. */
g_return_if_fail(tag);
priv = NM_NETNS_GET_PRIVATE(self);
@ -1473,7 +1423,6 @@ nm_netns_init(NMNetns *self)
priv->_self_signal_user_data = self;
c_list_init(&priv->l3cfg_signal_pending_lst_head);
c_list_init(&priv->watcher_no_tag_lst_head);
G_STATIC_ASSERT_EXPR(G_STRUCT_OFFSET(EcmpTrackObj, obj) == 0);
priv->ecmp_track_by_obj =
@ -1571,7 +1520,6 @@ dispose(GObject *object)
nm_assert(nm_g_hash_table_size(priv->watcher_idx) == 0);
nm_assert(nm_g_hash_table_size(priv->watcher_by_tag_idx) == 0);
nm_assert(nm_g_hash_table_size(priv->watcher_ip_data_idx) == 0);
nm_assert(c_list_is_empty(&priv->watcher_no_tag_lst_head));
nm_clear_pointer(&priv->ecmp_track_by_obj, g_hash_table_destroy);
nm_clear_pointer(&priv->ecmp_track_by_ecmpid, g_hash_table_destroy);

19
src/core/nm-netns.h
View file

@ -91,19 +91,12 @@ typedef void (*NMNetnsWatcherCallback)(NMNetns *self,
const NMNetnsWatcherEventData *event_data,
gpointer user_data);
NMNetnsWatcherHandle *nm_netns_watcher_add(NMNetns *self,
NMNetnsWatcherType watcher_type,
const NMNetnsWatcherData *watcher_data,
gconstpointer tag,
NMNetnsWatcherCallback callback,
gpointer user_data);
void nm_netns_watcher_remove_handle(NMNetns *self, NMNetnsWatcherHandle *handle);
void nm_netns_watcher_remove(NMNetns *self,
NMNetnsWatcherType watcher_type,
const NMNetnsWatcherData *watcher_data,
gconstpointer tag);
void nm_netns_watcher_add(NMNetns *self,
NMNetnsWatcherType watcher_type,
const NMNetnsWatcherData *watcher_data,
gconstpointer tag,
NMNetnsWatcherCallback callback,
gpointer user_data);
void
nm_netns_watcher_remove_all(NMNetns *self, gconstpointer tag, gboolean all /* or only dirty */);