From ac626c689251409fb4731071e0208b12e30d80df Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 2 Nov 2020 18:27:50 +0100 Subject: [PATCH 01/35] libnm: fix assertion in nm_setting_user_set_data() to check input argument --- libnm-core/nm-setting-user.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libnm-core/nm-setting-user.c b/libnm-core/nm-setting-user.c index 3f0e03fb2a..dd8600f752 100644 --- a/libnm-core/nm-setting-user.c +++ b/libnm-core/nm-setting-user.c @@ -288,7 +288,7 @@ nm_setting_user_set_data(NMSettingUser *setting, const char *key, const char *va NMSettingUserPrivate *priv; gboolean changed = FALSE; - g_return_val_if_fail(NM_IS_SETTING(self), FALSE); + g_return_val_if_fail(NM_IS_SETTING_USER(self), FALSE); g_return_val_if_fail(!error || !*error, FALSE); if (!nm_setting_user_check_key(key, error)) From c4d981959eac8c3beeecba8e8436ebda707f553c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 6 Nov 2020 16:05:26 +0100 Subject: [PATCH 02/35] shared: add nm_utils_strdup_reset_take() helper --- shared/nm-glib-aux/nm-shared-utils.h | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/shared/nm-glib-aux/nm-shared-utils.h b/shared/nm-glib-aux/nm-shared-utils.h index e8aba6104c..1e2c482549 100644 --- a/shared/nm-glib-aux/nm-shared-utils.h +++ b/shared/nm-glib-aux/nm-shared-utils.h @@ -2204,6 +2204,25 @@ nm_utils_strdup_reset(char **dst, const char *src) return TRUE; } +static inline gboolean +nm_utils_strdup_reset_take(char **dst, char *src) +{ + char *old; + + nm_assert(dst); + nm_assert(src != *dst); + + if (nm_streq0(*dst, src)) { + if (src) + g_free(src); + return FALSE; + } + old = *dst; + *dst = src; + g_free(old); + return TRUE; +} + void nm_indirect_g_free(gpointer arg); /*****************************************************************************/ From 571aeec933ac8bd62ee4888948bea4361316dd8b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 6 Nov 2020 16:29:41 +0100 Subject: [PATCH 03/35] shared: add nm_utils_named_value_clear_with_g_free() helper --- shared/nm-glib-aux/nm-shared-utils.c | 12 ++++++++++++ shared/nm-glib-aux/nm-shared-utils.h | 2 ++ 2 files changed, 14 insertions(+) diff --git a/shared/nm-glib-aux/nm-shared-utils.c b/shared/nm-glib-aux/nm-shared-utils.c index 7f766871f6..b1a557a28e 100644 --- a/shared/nm-glib-aux/nm-shared-utils.c +++ b/shared/nm-glib-aux/nm-shared-utils.c @@ -3066,6 +3066,18 @@ nm_utils_fd_read_loop_exact(int fd, void *buf, size_t nbytes, bool do_poll) /*****************************************************************************/ +void +nm_utils_named_value_clear_with_g_free(NMUtilsNamedValue *val) +{ + if (val) { + gs_free gpointer x_name = NULL; + gs_free gpointer x_value = NULL; + + x_name = (gpointer) g_steal_pointer(&val->name); + x_value = g_steal_pointer(&val->value_ptr); + } +} + G_STATIC_ASSERT(G_STRUCT_OFFSET(NMUtilsNamedValue, name) == 0); NMUtilsNamedValue * diff --git a/shared/nm-glib-aux/nm-shared-utils.h b/shared/nm-glib-aux/nm-shared-utils.h index 1e2c482549..df3f55195c 100644 --- a/shared/nm-glib-aux/nm-shared-utils.h +++ b/shared/nm-glib-aux/nm-shared-utils.h @@ -1460,6 +1460,8 @@ void nm_utils_named_value_list_sort(NMUtilsNamedValue *arr, GCompareDataFunc compare_func, gpointer user_data); +void nm_utils_named_value_clear_with_g_free(NMUtilsNamedValue *val); + /*****************************************************************************/ gpointer *nm_utils_hash_keys_to_array(GHashTable * hash, From 0cf4250021eb6f942baa4a6245668d769e0b67eb Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 7 Nov 2020 19:18:39 +0100 Subject: [PATCH 04/35] shared: add nm_utils_strdict_clone() helper --- shared/nm-glib-aux/nm-shared-utils.c | 18 ++++++++++++++++++ shared/nm-glib-aux/nm-shared-utils.h | 2 ++ 2 files changed, 20 insertions(+) diff --git a/shared/nm-glib-aux/nm-shared-utils.c b/shared/nm-glib-aux/nm-shared-utils.c index b1a557a28e..05e8caf3ad 100644 --- a/shared/nm-glib-aux/nm-shared-utils.c +++ b/shared/nm-glib-aux/nm-shared-utils.c @@ -434,6 +434,24 @@ nm_g_variant_singleton_u_0(void) /*****************************************************************************/ +GHashTable * +nm_utils_strdict_clone(GHashTable *src) +{ + GHashTable * dst; + GHashTableIter iter; + const char * key; + const char * val; + + if (!src) + return NULL; + + dst = g_hash_table_new_full(nm_str_hash, g_str_equal, g_free, g_free); + g_hash_table_iter_init(&iter, src); + while (g_hash_table_iter_next(&iter, (gpointer *) &key, (gpointer *) &val)) + g_hash_table_insert(dst, g_strdup(key), g_strdup(val)); + return dst; +} + /* Convert a hash table with "char *" keys and values to an "a{ss}" GVariant. * The keys will be sorted asciibetically. * Returns a floating reference. diff --git a/shared/nm-glib-aux/nm-shared-utils.h b/shared/nm-glib-aux/nm-shared-utils.h index df3f55195c..b736086387 100644 --- a/shared/nm-glib-aux/nm-shared-utils.h +++ b/shared/nm-glib-aux/nm-shared-utils.h @@ -320,6 +320,8 @@ gboolean nm_utils_gbytes_equal_mem(GBytes *bytes, gconstpointer mem_data, gsize GVariant *nm_utils_gbytes_to_variant_ay(GBytes *bytes); +GHashTable *nm_utils_strdict_clone(GHashTable *src); + GVariant *nm_utils_strdict_to_variant_ass(GHashTable *strdict); GVariant *nm_utils_strdict_to_variant_asv(GHashTable *strdict); From f4d472beab6d2cec4a0a3b8b26a4be254c9e4b9c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 6 Nov 2020 16:40:24 +0100 Subject: [PATCH 05/35] shared: add nm_g_array_unref() helper --- shared/nm-glib-aux/nm-shared-utils.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/shared/nm-glib-aux/nm-shared-utils.h b/shared/nm-glib-aux/nm-shared-utils.h index b736086387..a8b9abc772 100644 --- a/shared/nm-glib-aux/nm-shared-utils.h +++ b/shared/nm-glib-aux/nm-shared-utils.h @@ -1538,6 +1538,13 @@ nm_g_array_len(const GArray *arr) return arr ? arr->len : 0u; } +static inline void +nm_g_array_unref(GArray *arr) +{ + if (arr) + g_array_unref(arr); +} + #define nm_g_array_append_new(arr, type) \ ({ \ GArray *const _arr = (arr); \ From d52c3b3c94507c5dd2738c8d16782735886a53fe Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 2 Nov 2020 19:04:52 +0100 Subject: [PATCH 06/35] shared: use GEqualFunc instead of NMUtilsHashTableEqualFunc typedef --- shared/nm-glib-aux/nm-shared-utils.c | 8 ++++---- shared/nm-glib-aux/nm-shared-utils.h | 10 ++++------ 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/shared/nm-glib-aux/nm-shared-utils.c b/shared/nm-glib-aux/nm-shared-utils.c index 05e8caf3ad..060631a84a 100644 --- a/shared/nm-glib-aux/nm-shared-utils.c +++ b/shared/nm-glib-aux/nm-shared-utils.c @@ -3877,10 +3877,10 @@ nm_utils_array_find_binary_search(gconstpointer list, * Returns: %TRUE, if both dictionaries have the same content. */ gboolean -nm_utils_hash_table_equal(const GHashTable * a, - const GHashTable * b, - gboolean treat_null_as_empty, - NMUtilsHashTableEqualFunc equal_func) +nm_utils_hash_table_equal(const GHashTable *a, + const GHashTable *b, + gboolean treat_null_as_empty, + GEqualFunc equal_func) { guint n; GHashTableIter iter; diff --git a/shared/nm-glib-aux/nm-shared-utils.h b/shared/nm-glib-aux/nm-shared-utils.h index a8b9abc772..a7e894de89 100644 --- a/shared/nm-glib-aux/nm-shared-utils.h +++ b/shared/nm-glib-aux/nm-shared-utils.h @@ -1700,12 +1700,10 @@ gssize nm_utils_array_find_binary_search(gconstpointer list, /*****************************************************************************/ -typedef gboolean (*NMUtilsHashTableEqualFunc)(gconstpointer a, gconstpointer b); - -gboolean nm_utils_hash_table_equal(const GHashTable * a, - const GHashTable * b, - gboolean treat_null_as_empty, - NMUtilsHashTableEqualFunc equal_func); +gboolean nm_utils_hash_table_equal(const GHashTable *a, + const GHashTable *b, + gboolean treat_null_as_empty, + GEqualFunc equal_func); /*****************************************************************************/ From a3aa3725e5faf707d76eaa1009a71152c37564dc Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 2 Nov 2020 18:27:26 +0100 Subject: [PATCH 07/35] shared,all: cleanup nm_utils_hashtable_equal*() functions We have: - nm_utils_hashtable_cmp(): this does a full cmp of two hash tables, with the intent to provide a stable sort order. It thus takes a GCompareDataFunc() argument. - nm_utils_hashtable_cmp_equal(): this is like nm_utils_hashtable_cmp(), except that the caller won't get a compare value, only a boolean value that indicates equality. This was previously called nm_utils_hashtable_equal(). - nm_utils_hashtable_equal(): this takes a GEqualFunc function for comparing the values for equality. It takes thus a different kind of predicate, but otherwise is similar to nm_utils_hashtable_cmp_equal(). This was previously called nm_utils_hash_table_equal(). Unify the naming of these functions. --- libnm-core/nm-setting-user.c | 10 +- libnm-core/nm-setting.c | 8 +- shared/nm-glib-aux/nm-shared-utils.c | 124 +++++++++--------- shared/nm-glib-aux/nm-shared-utils.h | 18 ++- .../nm-glib-aux/tests/test-shared-general.c | 10 +- 5 files changed, 84 insertions(+), 86 deletions(-) diff --git a/libnm-core/nm-setting-user.c b/libnm-core/nm-setting-user.c index dd8600f752..ad3010e65c 100644 --- a/libnm-core/nm-setting-user.c +++ b/libnm-core/nm-setting-user.c @@ -416,11 +416,11 @@ compare_property(const NMSettInfoSetting *sett_info, priv = NM_SETTING_USER_GET_PRIVATE(NM_SETTING_USER(set_a)); pri2 = NM_SETTING_USER_GET_PRIVATE(NM_SETTING_USER(set_b)); - return nm_utils_hash_table_equal(priv->data, pri2->data, TRUE, g_str_equal) - && nm_utils_hash_table_equal(priv->data_invalid, - pri2->data_invalid, - TRUE, - g_str_equal); + return nm_utils_hashtable_equal(priv->data, pri2->data, TRUE, g_str_equal) + && nm_utils_hashtable_equal(priv->data_invalid, + pri2->data_invalid, + TRUE, + g_str_equal); } return NM_SETTING_CLASS(nm_setting_user_parent_class) diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c index 0f62128908..28f1fedb06 100644 --- a/libnm-core/nm-setting.c +++ b/libnm-core/nm-setting.c @@ -1481,10 +1481,10 @@ _nm_setting_compare(NMConnection * con_a, GenData *a_gendata = _gendata_hash(a, FALSE); GenData *b_gendata = _gendata_hash(b, FALSE); - return nm_utils_hash_table_equal(a_gendata ? a_gendata->hash : NULL, - b_gendata ? b_gendata->hash : NULL, - TRUE, - g_variant_equal); + return nm_utils_hashtable_equal(a_gendata ? a_gendata->hash : NULL, + b_gendata ? b_gendata->hash : NULL, + TRUE, + g_variant_equal); } for (i = 0; i < sett_info->property_infos_len; i++) { diff --git a/shared/nm-glib-aux/nm-shared-utils.c b/shared/nm-glib-aux/nm-shared-utils.c index 060631a84a..b46e0c486e 100644 --- a/shared/nm-glib-aux/nm-shared-utils.c +++ b/shared/nm-glib-aux/nm-shared-utils.c @@ -3292,6 +3292,62 @@ nm_utils_hash_values_to_array(GHashTable * hash, return arr; } +/*****************************************************************************/ + +/** + * nm_utils_hashtable_equal: + * @a: one #GHashTable + * @b: other #GHashTable + * @treat_null_as_empty: if %TRUE, when either @a or @b is %NULL, it is + * treated like an empty hash. It means, a %NULL hash will compare equal + * to an empty hash. + * @equal_func: the equality function, for comparing the values. + * If %NULL, the values are not compared. In that case, the function + * only checks, if both dictionaries have the same keys -- according + * to @b's key equality function. + * Note that the values of @a will be passed as first argument + * to @equal_func. + * + * Compares two hash tables, whether they have equal content. + * This only makes sense, if @a and @b have the same key types and + * the same key compare-function. + * + * Returns: %TRUE, if both dictionaries have the same content. + */ +gboolean +nm_utils_hashtable_equal(const GHashTable *a, + const GHashTable *b, + gboolean treat_null_as_empty, + GEqualFunc equal_func) +{ + guint n; + GHashTableIter iter; + gconstpointer key, v_a, v_b; + + if (a == b) + return TRUE; + if (!treat_null_as_empty) { + if (!a || !b) + return FALSE; + } + + n = a ? g_hash_table_size((GHashTable *) a) : 0; + if (n != (b ? g_hash_table_size((GHashTable *) b) : 0)) + return FALSE; + + if (n > 0) { + g_hash_table_iter_init(&iter, (GHashTable *) a); + while (g_hash_table_iter_next(&iter, (gpointer *) &key, (gpointer *) &v_a)) { + if (!g_hash_table_lookup_extended((GHashTable *) b, key, NULL, (gpointer *) &v_b)) + return FALSE; + if (equal_func && !equal_func(v_a, v_b)) + return FALSE; + } + } + + return TRUE; +} + static gboolean _utils_hashtable_equal(GHashTable * hash_a, GHashTable * hash_b, @@ -3331,7 +3387,7 @@ _utils_hashtable_equal(GHashTable * hash_a, } /** - * nm_utils_hashtable_equal: + * nm_utils_hashtable_cmp_equal: * @a: (allow-none): the hash table or %NULL * @b: (allow-none): the other hash table or %NULL * @cmp_values: (allow-none): if %NULL, only the keys @@ -3346,10 +3402,10 @@ _utils_hashtable_equal(GHashTable * hash_a, * @cmp_values is given) all values are the same. */ gboolean -nm_utils_hashtable_equal(const GHashTable *a, - const GHashTable *b, - GCompareDataFunc cmp_values, - gpointer user_data) +nm_utils_hashtable_cmp_equal(const GHashTable *a, + const GHashTable *b, + GCompareDataFunc cmp_values, + gpointer user_data) { GHashTable *hash_a = (GHashTable *) a; GHashTable *hash_b = (GHashTable *) b; @@ -3404,7 +3460,7 @@ _hashtable_cmp_func(gconstpointer a, gconstpointer b, gpointer user_data) * @a: (allow-none): the hash to compare. May be %NULL. * @b: (allow-none): the other hash to compare. May be %NULL. * @do_fast_precheck: if %TRUE, assume that the hashes are equal - * and that it is worth calling nm_utils_hashtable_equal() first. + * and that it is worth calling nm_utils_hashtable_cmp_equal() first. * That requires, that both hashes have the same equals function * which is compatible with the @cmp_keys function. * @cmp_keys: the compare function for keys. Usually, the hash/equal function @@ -3856,62 +3912,6 @@ nm_utils_array_find_binary_search(gconstpointer list, /*****************************************************************************/ -/** - * nm_utils_hash_table_equal: - * @a: one #GHashTable - * @b: other #GHashTable - * @treat_null_as_empty: if %TRUE, when either @a or @b is %NULL, it is - * treated like an empty hash. It means, a %NULL hash will compare equal - * to an empty hash. - * @equal_func: the equality function, for comparing the values. - * If %NULL, the values are not compared. In that case, the function - * only checks, if both dictionaries have the same keys -- according - * to @b's key equality function. - * Note that the values of @a will be passed as first argument - * to @equal_func. - * - * Compares two hash tables, whether they have equal content. - * This only makes sense, if @a and @b have the same key types and - * the same key compare-function. - * - * Returns: %TRUE, if both dictionaries have the same content. - */ -gboolean -nm_utils_hash_table_equal(const GHashTable *a, - const GHashTable *b, - gboolean treat_null_as_empty, - GEqualFunc equal_func) -{ - guint n; - GHashTableIter iter; - gconstpointer key, v_a, v_b; - - if (a == b) - return TRUE; - if (!treat_null_as_empty) { - if (!a || !b) - return FALSE; - } - - n = a ? g_hash_table_size((GHashTable *) a) : 0; - if (n != (b ? g_hash_table_size((GHashTable *) b) : 0)) - return FALSE; - - if (n > 0) { - g_hash_table_iter_init(&iter, (GHashTable *) a); - while (g_hash_table_iter_next(&iter, (gpointer *) &key, (gpointer *) &v_a)) { - if (!g_hash_table_lookup_extended((GHashTable *) b, key, NULL, (gpointer *) &v_b)) - return FALSE; - if (equal_func && !equal_func(v_a, v_b)) - return FALSE; - } - } - - return TRUE; -} - -/*****************************************************************************/ - /** * nm_utils_get_start_time_for_pid: * @pid: the process identifier diff --git a/shared/nm-glib-aux/nm-shared-utils.h b/shared/nm-glib-aux/nm-shared-utils.h index a7e894de89..cf01169d1c 100644 --- a/shared/nm-glib-aux/nm-shared-utils.h +++ b/shared/nm-glib-aux/nm-shared-utils.h @@ -1487,13 +1487,18 @@ nm_utils_strdict_get_keys(const GHashTable *hash, gboolean sorted, guint *out_le gboolean nm_utils_hashtable_equal(const GHashTable *a, const GHashTable *b, - GCompareDataFunc cmp_values, - gpointer user_data); + gboolean treat_null_as_empty, + GEqualFunc equal_func); + +gboolean nm_utils_hashtable_cmp_equal(const GHashTable *a, + const GHashTable *b, + GCompareDataFunc cmp_values, + gpointer user_data); static inline gboolean nm_utils_hashtable_same_keys(const GHashTable *a, const GHashTable *b) { - return nm_utils_hashtable_equal(a, b, NULL, NULL); + return nm_utils_hashtable_cmp_equal(a, b, NULL, NULL); } int nm_utils_hashtable_cmp(const GHashTable *a, @@ -1700,13 +1705,6 @@ gssize nm_utils_array_find_binary_search(gconstpointer list, /*****************************************************************************/ -gboolean nm_utils_hash_table_equal(const GHashTable *a, - const GHashTable *b, - gboolean treat_null_as_empty, - GEqualFunc equal_func); - -/*****************************************************************************/ - void _nm_utils_strv_sort(const char **strv, gssize len); #define nm_utils_strv_sort(strv, len) _nm_utils_strv_sort(NM_CAST_STRV_MC(strv), len) diff --git a/shared/nm-glib-aux/tests/test-shared-general.c b/shared/nm-glib-aux/tests/test-shared-general.c index dbb8ed275e..13dd26f240 100644 --- a/shared/nm-glib-aux/tests/test-shared-general.c +++ b/shared/nm-glib-aux/tests/test-shared-general.c @@ -1190,8 +1190,8 @@ test_utils_hashtable_cmp(void) } g_assert(nm_utils_hashtable_same_keys(h1, h2)); - g_assert(nm_utils_hashtable_equal(h1, h2, NULL, NULL)); - g_assert(nm_utils_hashtable_equal(h1, h2, func_val_cmp, NULL)); + g_assert(nm_utils_hashtable_cmp_equal(h1, h2, NULL, NULL)); + g_assert(nm_utils_hashtable_cmp_equal(h1, h2, func_val_cmp, NULL)); g_assert(nm_utils_hashtable_cmp(h1, h2, FALSE, func_key_cmp, NULL, NULL) == 0); g_assert(nm_utils_hashtable_cmp(h1, h2, TRUE, func_key_cmp, NULL, NULL) == 0); g_assert(nm_utils_hashtable_cmp(h1, h2, FALSE, func_key_cmp, func_val_cmp, NULL) == 0); @@ -1221,16 +1221,16 @@ again: if (has_same_keys) { g_assert(nm_utils_hashtable_same_keys(h1, h2)); - g_assert(nm_utils_hashtable_equal(h1, h2, NULL, NULL)); + g_assert(nm_utils_hashtable_cmp_equal(h1, h2, NULL, NULL)); g_assert(nm_utils_hashtable_cmp(h1, h2, FALSE, func_key_cmp, NULL, NULL) == 0); g_assert(nm_utils_hashtable_cmp(h1, h2, TRUE, func_key_cmp, NULL, NULL) == 0); } else { g_assert(!nm_utils_hashtable_same_keys(h1, h2)); - g_assert(!nm_utils_hashtable_equal(h1, h2, NULL, NULL)); + g_assert(!nm_utils_hashtable_cmp_equal(h1, h2, NULL, NULL)); g_assert(nm_utils_hashtable_cmp(h1, h2, FALSE, func_key_cmp, NULL, NULL) != 0); g_assert(nm_utils_hashtable_cmp(h1, h2, TRUE, func_key_cmp, NULL, NULL) != 0); } - g_assert(!nm_utils_hashtable_equal(h1, h2, func_val_cmp, NULL)); + g_assert(!nm_utils_hashtable_cmp_equal(h1, h2, func_val_cmp, NULL)); g_assert(nm_utils_hashtable_cmp(h1, h2, FALSE, func_key_cmp, func_val_cmp, NULL) != 0); g_assert(nm_utils_hashtable_cmp(h1, h2, TRUE, func_key_cmp, func_val_cmp, NULL) != 0); } From 336270edd5a6237f2569a1260d11afdde6bec629 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 5 Nov 2020 14:00:46 +0100 Subject: [PATCH 08/35] shared/strbuf: add nm_str_buf_get_char() and nm_str_buf_get_str_at_unsafe() helpers --- shared/nm-glib-aux/nm-str-buf.h | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/shared/nm-glib-aux/nm-str-buf.h b/shared/nm-glib-aux/nm-str-buf.h index 062630a5d2..f5e0bd8fe9 100644 --- a/shared/nm-glib-aux/nm-str-buf.h +++ b/shared/nm-glib-aux/nm-str-buf.h @@ -31,7 +31,7 @@ typedef struct _NMStrBuf { /*****************************************************************************/ static inline void -_nm_str_buf_assert(NMStrBuf *strbuf) +_nm_str_buf_assert(const NMStrBuf *strbuf) { nm_assert(strbuf); nm_assert((!!strbuf->_priv_str) == (strbuf->_priv_allocated > 0)); @@ -361,6 +361,31 @@ nm_str_buf_get_str_unsafe(NMStrBuf *strbuf) return strbuf->_priv_str; } +static inline char * +nm_str_buf_get_str_at_unsafe(NMStrBuf *strbuf, gsize index) +{ + _nm_str_buf_assert(strbuf); + + /* it is acceptable to ask for a pointer at the end of the buffer -- even + * if there is no data there. The caller is anyway required to take care + * of the length (that's the "unsafe" part), and in that case, the length + * is merely zero. */ + nm_assert(index <= strbuf->allocated); + + if (!strbuf->_priv_str) + return NULL; + + return &strbuf->_priv_str[index]; +} + +static inline char +nm_str_buf_get_char(const NMStrBuf *strbuf, gsize index) +{ + _nm_str_buf_assert(strbuf); + nm_assert(index < strbuf->allocated); + return strbuf->_priv_str[index]; +} + /** * nm_str_buf_finalize: * @strbuf: an initilized #NMStrBuf From 0d083f5dabe10a8bbd32fdfce032394b877c5bee Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 4 Nov 2020 16:53:25 +0100 Subject: [PATCH 09/35] libnm: add nm_utils_print() function libnm supports verbose debug logging by setting "LIBNM_CLIENT_DEBUG" environment variable. That mechanism uses g_printerr() (or g_print()). When testing an application it's useful to combine printf debugging with this debug logging. However, python's print() statement is additionally buffered and not in sync with the logging functions that libnm uses. As far as I see, g_print() and g_printerr() is not accessible via introspections/pygobject, probably because these are variadic functions. Add nm_utils_print() to libnm. This ensures to use the same logging mechanism as libnm. --- libnm/libnm.ver | 1 + libnm/nm-client.h | 5 +++++ libnm/nm-libnm-utils.c | 39 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+) diff --git a/libnm/libnm.ver b/libnm/libnm.ver index 552e550646..baf1e06f86 100644 --- a/libnm/libnm.ver +++ b/libnm/libnm.ver @@ -1766,4 +1766,5 @@ global: nm_keyfile_read; nm_keyfile_warn_severity_get_type; nm_keyfile_write; + nm_utils_print; } libnm_1_28_0; diff --git a/libnm/nm-client.h b/libnm/nm-client.h index 86e9486e29..befd454f96 100644 --- a/libnm/nm-client.h +++ b/libnm/nm-client.h @@ -480,6 +480,11 @@ void nm_client_dbus_set_property(NMClient * client, NM_AVAILABLE_IN_1_24 gboolean nm_client_dbus_set_property_finish(NMClient *client, GAsyncResult *result, GError **error); +/*****************************************************************************/ + +NM_AVAILABLE_IN_1_30 +void nm_utils_print(int output_mode, const char *msg); + G_END_DECLS #endif /* __NM_CLIENT_H__ */ diff --git a/libnm/nm-libnm-utils.c b/libnm/nm-libnm-utils.c index 9c27f64de8..775aebf803 100644 --- a/libnm/nm-libnm-utils.c +++ b/libnm/nm-libnm-utils.c @@ -869,3 +869,42 @@ nm_utils_g_param_spec_is_default(const GParamSpec *pspec) * strictly asserts and only support argument types that we expect. */ g_return_val_if_reached(FALSE); } + +/*****************************************************************************/ + +/** + * nm_utils_print: + * @output_mode: if 1 it uses g_print(). If 2, it uses g_printerr(). + * If 0, it uses either g_print() or g_printerr(), depending + * on LIBNM_CLIENT_DEBUG (and the "stdout" flag). + * @msg: the message to print. The function does not append + * a trailing newline. + * + * The only purpose of this function is to give access to g_print() + * or g_printerr() from pygobject. libnm can do debug logging by + * setting LIBNM_CLIENT_DEBUG and uses thereby g_printerr() or + * g_print(). A plain "print()" function in python is not in sync + * with these functions (it implements additional buffering). By + * using nm_utils_print(), the same logging mechanisms can be used. + * + * Since: 1.30 + */ +void +nm_utils_print(int output_mode, const char *msg) +{ + gboolean use_stdout; + + g_return_if_fail(msg); + + if (output_mode == 0) { + nml_dbus_log_enabled_full(NML_DBUS_LOG_LEVEL_ANY, &use_stdout); + output_mode = use_stdout ? 1 : 2; + } + + if (output_mode == 1) + g_print("%s", msg); + else if (output_mode == 2) + g_printerr("%s", msg); + else + g_return_if_reached(); +} From 6100b52e5ccc80b9c11e35907c0537d930f50da6 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 2 Nov 2020 17:29:26 +0100 Subject: [PATCH 10/35] libnm: add NMSettingOvsExternalIDs --- Makefile.am | 2 + .../generate-docs-nm-settings-nmcli.xml.in | 2 + clients/common/nm-meta-setting-desc.c | 2 + clients/common/settings-docs.h.in | 1 + docs/libnm/libnm-docs.xml | 1 + libnm-core/meson.build | 6 +- libnm-core/nm-core-internal.h | 4 + libnm-core/nm-core-types.h | 1 + libnm-core/nm-keyfile/nm-keyfile.c | 107 +++- .../nm-libnm-core-utils.h | 4 + libnm-core/nm-setting-ovs-external-ids.c | 536 ++++++++++++++++++ libnm-core/nm-setting-ovs-external-ids.h | 68 +++ libnm/libnm.ver | 7 + po/POTFILES.in | 1 + shared/nm-meta-setting.c | 8 + shared/nm-meta-setting.h | 1 + 16 files changed, 747 insertions(+), 4 deletions(-) create mode 100644 libnm-core/nm-setting-ovs-external-ids.c create mode 100644 libnm-core/nm-setting-ovs-external-ids.h diff --git a/Makefile.am b/Makefile.am index 473a763d74..dce16fe8c8 100644 --- a/Makefile.am +++ b/Makefile.am @@ -954,6 +954,7 @@ libnm_core_lib_h_pub_real = \ libnm-core/nm-setting-olpc-mesh.h \ libnm-core/nm-setting-ovs-bridge.h \ libnm-core/nm-setting-ovs-dpdk.h \ + libnm-core/nm-setting-ovs-external-ids.h \ libnm-core/nm-setting-ovs-interface.h \ libnm-core/nm-setting-ovs-patch.h \ libnm-core/nm-setting-ovs-port.h \ @@ -1027,6 +1028,7 @@ libnm_core_lib_c_settings_real = \ libnm-core/nm-setting-olpc-mesh.c \ libnm-core/nm-setting-ovs-bridge.c \ libnm-core/nm-setting-ovs-dpdk.c \ + libnm-core/nm-setting-ovs-external-ids.c \ libnm-core/nm-setting-ovs-interface.c \ libnm-core/nm-setting-ovs-patch.c \ libnm-core/nm-setting-ovs-port.c \ diff --git a/clients/cli/generate-docs-nm-settings-nmcli.xml.in b/clients/cli/generate-docs-nm-settings-nmcli.xml.in index 25675a1ec7..f2f589fe8d 100644 --- a/clients/cli/generate-docs-nm-settings-nmcli.xml.in +++ b/clients/cli/generate-docs-nm-settings-nmcli.xml.in @@ -784,6 +784,8 @@ + + diff --git a/clients/common/nm-meta-setting-desc.c b/clients/common/nm-meta-setting-desc.c index 2609c706fe..86b555fe5f 100644 --- a/clients/common/nm-meta-setting-desc.c +++ b/clients/common/nm-meta-setting-desc.c @@ -7946,6 +7946,7 @@ _setting_init_fcn_wireless (ARGS_SETTING_INIT_FCN) #define SETTING_PRETTY_NAME_OLPC_MESH N_("OLPC Mesh connection") #define SETTING_PRETTY_NAME_OVS_BRIDGE N_("Open vSwitch bridge settings") #define SETTING_PRETTY_NAME_OVS_DPDK N_("Open vSwitch DPDK interface settings") +#define SETTING_PRETTY_NAME_OVS_EXTERNAL_IDS N_("OVS External IDs") #define SETTING_PRETTY_NAME_OVS_INTERFACE N_("Open vSwitch interface settings") #define SETTING_PRETTY_NAME_OVS_PATCH N_("Open vSwitch patch interface settings") #define SETTING_PRETTY_NAME_OVS_PORT N_("Open vSwitch port settings") @@ -8129,6 +8130,7 @@ const NMMetaSettingInfoEditor nm_meta_setting_infos_editor[] = { ), ), SETTING_INFO (OVS_DPDK), + SETTING_INFO_EMPTY (OVS_EXTERNAL_IDS), SETTING_INFO (OVS_INTERFACE, .valid_parts = NM_META_SETTING_VALID_PARTS ( NM_META_SETTING_VALID_PART_ITEM (CONNECTION, TRUE), diff --git a/clients/common/settings-docs.h.in b/clients/common/settings-docs.h.in index 5b93627245..78198ec973 100644 --- a/clients/common/settings-docs.h.in +++ b/clients/common/settings-docs.h.in @@ -294,6 +294,7 @@ #define DESCRIBE_DOC_NM_SETTING_OVS_BRIDGE_RSTP_ENABLE N_("Enable or disable RSTP.") #define DESCRIBE_DOC_NM_SETTING_OVS_BRIDGE_STP_ENABLE N_("Enable or disable STP.") #define DESCRIBE_DOC_NM_SETTING_OVS_DPDK_DEVARGS N_("Open vSwitch DPDK device arguments.") +#define DESCRIBE_DOC_NM_SETTING_OVS_EXTERNAL_IDS_DATA N_("A dictionary of key/value pairs with exernal-ids for OVS.") #define DESCRIBE_DOC_NM_SETTING_OVS_INTERFACE_TYPE N_("The interface type. Either \"internal\", \"system\", \"patch\", \"dpdk\", or empty.") #define DESCRIBE_DOC_NM_SETTING_OVS_PATCH_PEER N_("Specifies the name of the interface for the other side of the patch. The patch on the other side must also set this interface as peer.") #define DESCRIBE_DOC_NM_SETTING_OVS_PORT_BOND_DOWNDELAY N_("The time port must be inactive in order to be considered down.") diff --git a/docs/libnm/libnm-docs.xml b/docs/libnm/libnm-docs.xml index 47b1fa1c08..d0be0eb475 100644 --- a/docs/libnm/libnm-docs.xml +++ b/docs/libnm/libnm-docs.xml @@ -338,6 +338,7 @@ print ("NetworkManager version " + client.get_version())]]> + diff --git a/libnm-core/meson.build b/libnm-core/meson.build index aa8823c991..7b59a8c204 100644 --- a/libnm-core/meson.build +++ b/libnm-core/meson.build @@ -43,8 +43,9 @@ libnm_core_headers = files( 'nm-setting-match.h', 'nm-setting-olpc-mesh.h', 'nm-setting-ovs-bridge.h', - 'nm-setting-ovs-interface.h', 'nm-setting-ovs-dpdk.h', + 'nm-setting-ovs-external-ids.h', + 'nm-setting-ovs-interface.h', 'nm-setting-ovs-patch.h', 'nm-setting-ovs-port.h', 'nm-setting-ppp.h', @@ -143,8 +144,9 @@ libnm_core_settings_sources = files( 'nm-setting-match.c', 'nm-setting-olpc-mesh.c', 'nm-setting-ovs-bridge.c', - 'nm-setting-ovs-interface.c', 'nm-setting-ovs-dpdk.c', + 'nm-setting-ovs-external-ids.c', + 'nm-setting-ovs-interface.c', 'nm-setting-ovs-patch.c', 'nm-setting-ovs-port.c', 'nm-setting-ppp.c', diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h index f296a77fa6..fcc38565d3 100644 --- a/libnm-core/nm-core-internal.h +++ b/libnm-core/nm-core-internal.h @@ -476,6 +476,10 @@ GSList *_nm_vpn_plugin_info_list_load_dir(const char * dirname, /*****************************************************************************/ +GHashTable *_nm_setting_ovs_external_ids_get_data(NMSettingOvsExternalIDs *self); + +/*****************************************************************************/ + typedef struct { const char *name; gboolean numeric; diff --git a/libnm-core/nm-core-types.h b/libnm-core/nm-core-types.h index c2ac48e618..64d6464e2b 100644 --- a/libnm-core/nm-core-types.h +++ b/libnm-core/nm-core-types.h @@ -39,6 +39,7 @@ typedef struct _NMSettingMatch NMSettingMatch; typedef struct _NMSettingOlpcMesh NMSettingOlpcMesh; typedef struct _NMSettingOvsBridge NMSettingOvsBridge; typedef struct _NMSettingOvsDpdk NMSettingOvsDpdk; +typedef struct _NMSettingOvsExternalIDs NMSettingOvsExternalIDs; typedef struct _NMSettingOvsInterface NMSettingOvsInterface; typedef struct _NMSettingOvsPatch NMSettingOvsPatch; typedef struct _NMSettingOvsPort NMSettingOvsPort; diff --git a/libnm-core/nm-keyfile/nm-keyfile.c b/libnm-core/nm-keyfile/nm-keyfile.c index 1042d2d604..d3a1357281 100644 --- a/libnm-core/nm-keyfile/nm-keyfile.c +++ b/libnm-core/nm-keyfile/nm-keyfile.c @@ -24,9 +24,14 @@ #include "nm-core-internal.h" #include "nm-keyfile.h" #include "nm-setting-user.h" +#include "nm-setting-ovs-external-ids.h" #include "nm-keyfile-utils.h" +#define ETHERNET_S390_OPTIONS_GROUP_NAME "ethernet-s390-options" + +#define OVS_EXTERNAL_IDS_DATA_PREFIX "data." + /*****************************************************************************/ typedef struct _ParseInfoProperty ParseInfoProperty; @@ -989,6 +994,44 @@ ip_routing_rule_parser_full(KeyfileReaderInfo * info, } } +static void +_parser_full_ovs_external_ids_data(KeyfileReaderInfo * info, + const NMMetaSettingInfo * setting_info, + const NMSettInfoProperty *property_info, + const ParseInfoProperty * pip, + NMSetting * setting) +{ + const char * setting_name = NM_SETTING_OVS_EXTERNAL_IDS_SETTING_NAME; + gs_strfreev char **keys = NULL; + gsize n_keys; + gsize i; + + nm_assert(NM_IS_SETTING_OVS_EXTERNAL_IDS(setting)); + nm_assert(nm_streq(property_info->name, NM_SETTING_OVS_EXTERNAL_IDS_DATA)); + nm_assert(nm_streq(setting_name, setting_info->setting_name)); + nm_assert(nm_streq(setting_name, nm_setting_get_name(setting))); + + keys = nm_keyfile_plugin_kf_get_keys(info->keyfile, setting_name, &n_keys, NULL); + + for (i = 0; i < n_keys; i++) { + const char * key = keys[i]; + gs_free char *name_to_free = NULL; + gs_free char *value = NULL; + const char * name; + + if (!NM_STR_HAS_PREFIX(key, OVS_EXTERNAL_IDS_DATA_PREFIX)) + continue; + + value = nm_keyfile_plugin_kf_get_string(info->keyfile, setting_name, key, NULL); + if (!value) + continue; + + name = &key[NM_STRLEN(OVS_EXTERNAL_IDS_DATA_PREFIX)]; + name = nm_keyfile_key_decode(name, &name_to_free); + nm_setting_ovs_external_ids_set_data(NM_SETTING_OVS_EXTERNAL_IDS(setting), name, value); + } +} + static void ip_dns_parser(KeyfileReaderInfo *info, NMSetting *setting, const char *key) { @@ -2214,8 +2257,6 @@ bridge_vlan_writer(KeyfileWriterInfo *info, } } -#define ETHERNET_S390_OPTIONS_GROUP_NAME "ethernet-s390-options" - static void wired_s390_options_parser_full(KeyfileReaderInfo * info, const NMMetaSettingInfo * setting_info, @@ -2360,6 +2401,60 @@ tfilter_writer(KeyfileWriterInfo *info, NMSetting *setting, const char *key, con } } +static void +_writer_full_ovs_external_ids_data(KeyfileWriterInfo * info, + const NMMetaSettingInfo * setting_info, + const NMSettInfoProperty *property_info, + const ParseInfoProperty * pip, + NMSetting * setting) +{ + GHashTable * hash; + NMUtilsNamedValue data_static[300u / sizeof(NMUtilsNamedValue)]; + gs_free NMUtilsNamedValue *data_free = NULL; + const NMUtilsNamedValue * data; + guint data_len; + char full_key_static[NM_STRLEN(OVS_EXTERNAL_IDS_DATA_PREFIX) + 300u]; + guint i; + + nm_assert(NM_IS_SETTING_OVS_EXTERNAL_IDS(setting)); + nm_assert(nm_streq(property_info->name, NM_SETTING_OVS_EXTERNAL_IDS_DATA)); + + hash = _nm_setting_ovs_external_ids_get_data(NM_SETTING_OVS_EXTERNAL_IDS(setting)); + if (!hash) + return; + + data = nm_utils_named_values_from_strdict(hash, &data_len, data_static, &data_free); + if (data_len == 0) + return; + + memcpy(full_key_static, OVS_EXTERNAL_IDS_DATA_PREFIX, NM_STRLEN(OVS_EXTERNAL_IDS_DATA_PREFIX)); + + for (i = 0; i < data_len; i++) { + const char * key = data[i].name; + const char * val = data[i].value_str; + gs_free char *escaped_key_to_free = NULL; + const char * escaped_key; + gsize len; + gs_free char *full_key_free = NULL; + char * full_key = full_key_static; + + escaped_key = nm_keyfile_key_encode(key, &escaped_key_to_free); + + len = strlen(escaped_key) + 1u; + if (len >= G_N_ELEMENTS(full_key_static) - NM_STRLEN(OVS_EXTERNAL_IDS_DATA_PREFIX)) { + full_key_free = g_new(char, NM_STRLEN(OVS_EXTERNAL_IDS_DATA_PREFIX) + len); + full_key = full_key_free; + memcpy(full_key, OVS_EXTERNAL_IDS_DATA_PREFIX, NM_STRLEN(OVS_EXTERNAL_IDS_DATA_PREFIX)); + } + memcpy(&full_key[NM_STRLEN(OVS_EXTERNAL_IDS_DATA_PREFIX)], escaped_key, len); + + nm_keyfile_plugin_kf_set_string(info->keyfile, + NM_SETTING_OVS_EXTERNAL_IDS_SETTING_NAME, + full_key, + val); + } +} + static void write_hash_of_string(GKeyFile *file, NMSetting *setting, const char *key, const GValue *value) { @@ -2799,6 +2894,14 @@ static const ParseInfoSetting *const parse_infos[_NM_META_SETTING_TYPE_NUM] = { .writer_full = ip_routing_rule_writer_full, .has_parser_full = TRUE, .has_writer_full = TRUE, ), ), ), + PARSE_INFO_SETTING( + NM_META_SETTING_TYPE_OVS_EXTERNAL_IDS, + PARSE_INFO_PROPERTIES(PARSE_INFO_PROPERTY(NM_SETTING_OVS_EXTERNAL_IDS_DATA, + .parser_no_check_key = TRUE, + .parser_full = _parser_full_ovs_external_ids_data, + .writer_full = _writer_full_ovs_external_ids_data, + .has_parser_full = TRUE, + .has_writer_full = TRUE, ), ), ), PARSE_INFO_SETTING(NM_META_SETTING_TYPE_SERIAL, PARSE_INFO_PROPERTIES(PARSE_INFO_PROPERTY(NM_SETTING_SERIAL_PARITY, .parser = parity_parser, ), ), ), diff --git a/libnm-core/nm-libnm-core-intern/nm-libnm-core-utils.h b/libnm-core/nm-libnm-core-intern/nm-libnm-core-utils.h index ca29eade24..2361bf8c30 100644 --- a/libnm-core/nm-libnm-core-intern/nm-libnm-core-utils.h +++ b/libnm-core/nm-libnm-core-intern/nm-libnm-core-utils.h @@ -98,6 +98,10 @@ gboolean nm_utils_vlan_priority_map_parse_str(NMVlanPriorityMap map_type, /*****************************************************************************/ +#define NM_OVS_EXTERNAL_ID_NM_PREFIX "NM." + +/*****************************************************************************/ + static inline int nm_setting_ip_config_get_addr_family(NMSettingIPConfig *s_ip) { diff --git a/libnm-core/nm-setting-ovs-external-ids.c b/libnm-core/nm-setting-ovs-external-ids.c new file mode 100644 index 0000000000..90fda48317 --- /dev/null +++ b/libnm-core/nm-setting-ovs-external-ids.c @@ -0,0 +1,536 @@ +/* SPDX-License-Identifier: LGPL-2.1+ */ +/* + * Copyright (C) 2017 - 2020 Red Hat, Inc. + */ + +#include "nm-default.h" + +#include "nm-setting-ovs-external-ids.h" + +#include "nm-setting-private.h" +#include "nm-utils-private.h" +#include "nm-connection-private.h" + +#define MAX_NUM_KEYS 256 + +/*****************************************************************************/ + +/** + * SECTION:nm-setting-ovs-external-ids + * @short_description: External-IDs for OVS database + * + * The #NMSettingOvsExternalIDs object is a #NMSetting subclass that allow to + * configure external ids for OVS. + **/ + +/*****************************************************************************/ + +NM_GOBJECT_PROPERTIES_DEFINE(NMSettingOvsExternalIDs, PROP_DATA, ); + +typedef struct { + GHashTable * data; + const char **data_keys; +} NMSettingOvsExternalIDsPrivate; + +/** + * NMSettingOvsExternalIDs: + * + * OVS External IDs Settings + */ +struct _NMSettingOvsExternalIDs { + NMSetting parent; + NMSettingOvsExternalIDsPrivate _priv; +}; + +struct _NMSettingOvsExternalIDsClass { + NMSettingClass parent; +}; + +G_DEFINE_TYPE(NMSettingOvsExternalIDs, nm_setting_ovs_external_ids, NM_TYPE_SETTING) + +#define NM_SETTING_OVS_EXTERNAL_IDS_GET_PRIVATE(self) \ + _NM_GET_PRIVATE(self, NMSettingOvsExternalIDs, NM_IS_SETTING_OVS_EXTERNAL_IDS) + +/*****************************************************************************/ + +static gboolean +_exid_key_char_is_regular(char ch) +{ + /* allow words of printable characters, plus some + * special characters, for example to support base64 encoding. */ + return (ch >= 'a' && ch <= 'z') || (ch >= 'A' && ch <= 'Z') || (ch >= '0' && ch <= '9') + || NM_IN_SET(ch, '-', '_', '+', '/', '=', '.'); +} + +/** + * nm_setting_ovs_external_ids_check_key: + * @key: (allow-none): the key to check + * @error: a #GError, %NULL to ignore. + * + * Checks whether @key is a valid key for OVS' external-ids. + * This means, the key cannot be %NULL, not too large and valid ASCII. + * Also, only digits and numbers are allowed with a few special + * characters. They key must also not start with "NM.". + * + * Since: 1.30 + * + * Returns: %TRUE if @key is a valid user data key. + */ +gboolean +nm_setting_ovs_external_ids_check_key(const char *key, GError **error) +{ + gsize len; + + g_return_val_if_fail(!error || !*error, FALSE); + + if (!key || !key[0]) { + g_set_error_literal(error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + _("missing key")); + return FALSE; + } + len = strlen(key); + if (len > 255u) { + g_set_error_literal(error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + _("key is too long")); + return FALSE; + } + if (!g_utf8_validate(key, len, NULL)) { + g_set_error_literal(error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + _("key must be UTF8")); + return FALSE; + } + if (!NM_STRCHAR_ALL(key, ch, _exid_key_char_is_regular(ch))) { + /* Probably OVS is more forgiving about what makes a valid key for + * an external-id. However, we are strict (at least, for now). */ + g_set_error_literal(error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + _("key contains invalid characters")); + return FALSE; + } + + if (NM_STR_HAS_PREFIX(key, NM_OVS_EXTERNAL_ID_NM_PREFIX)) { + /* these keys are reserved. */ + g_set_error_literal(error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + _("key cannot start with \"NM.\"")); + return FALSE; + } + + return TRUE; +} + +/** + * nm_setting_ovs_external_ids_check_val: + * @val: (allow-none): the value to check + * @error: a #GError, %NULL to ignore. + * + * Checks whether @val is a valid user data value. This means, + * value is not %NULL, not too large and valid UTF-8. + * + * Since: 1.30 + * + * Returns: %TRUE if @val is a valid user data value. + */ +gboolean +nm_setting_ovs_external_ids_check_val(const char *val, GError **error) +{ + gsize len; + + g_return_val_if_fail(!error || !*error, FALSE); + + if (!val) { + g_set_error_literal(error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + _("value is missing")); + return FALSE; + } + + len = strlen(val); + if (len > (8u * 1024u)) { + g_set_error_literal(error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + _("value is too large")); + return FALSE; + } + + if (!g_utf8_validate(val, len, NULL)) { + g_set_error_literal(error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + _("value is not valid UTF8")); + return FALSE; + } + + return TRUE; +} + +/*****************************************************************************/ + +static GHashTable * +_create_data_hash(void) +{ + return g_hash_table_new_full(nm_str_hash, g_str_equal, g_free, g_free); +} + +GHashTable * +_nm_setting_ovs_external_ids_get_data(NMSettingOvsExternalIDs *self) +{ + return NM_SETTING_OVS_EXTERNAL_IDS_GET_PRIVATE(self)->data; +} + +/** + * nm_setting_ovs_external_ids_get_data_keys: + * @setting: the #NMSettingOvsExternalIDs + * @out_len: (out): the length of the returned array + * + * Returns: (array length=out_len) (transfer none): a + * %NULL-terminated array containing each key from the table. + **/ +const char *const * +nm_setting_ovs_external_ids_get_data_keys(NMSettingOvsExternalIDs *setting, guint *out_len) +{ + NMSettingOvsExternalIDs * self = setting; + NMSettingOvsExternalIDsPrivate *priv; + + g_return_val_if_fail(NM_IS_SETTING_OVS_EXTERNAL_IDS(self), NULL); + + priv = NM_SETTING_OVS_EXTERNAL_IDS_GET_PRIVATE(self); + + if (priv->data_keys) { + NM_SET_OUT(out_len, g_hash_table_size(priv->data)); + return priv->data_keys; + } + + priv->data_keys = nm_utils_strdict_get_keys(priv->data, TRUE, out_len); + + /* don't return %NULL, but hijack the @data_keys fields as a pseudo + * empty strv array. */ + return priv->data_keys ?: ((const char **) &priv->data_keys); +} + +/*****************************************************************************/ + +/** + * nm_setting_ovs_external_ids_get_data: + * @setting: the #NMSettingOvsExternalIDs instance + * @key: the external-id to lookup + * + * Since: 1.30 + * + * Returns: (transfer none): the value associated with @key or %NULL if no such + * value exists. + */ +const char * +nm_setting_ovs_external_ids_get_data(NMSettingOvsExternalIDs *setting, const char *key) +{ + NMSettingOvsExternalIDs * self = setting; + NMSettingOvsExternalIDsPrivate *priv; + + g_return_val_if_fail(NM_IS_SETTING_OVS_EXTERNAL_IDS(self), NULL); + g_return_val_if_fail(key, NULL); + + priv = NM_SETTING_OVS_EXTERNAL_IDS_GET_PRIVATE(self); + + if (!priv->data) + return NULL; + + return g_hash_table_lookup(priv->data, key); +} + +/** + * nm_setting_ovs_external_ids_set_data: + * @setting: the #NMSettingOvsExternalIDs instance + * @key: the key to set + * @val: (allow-none): the value to set or %NULL to clear a key. + * + * Since: 1.30 + */ +void +nm_setting_ovs_external_ids_set_data(NMSettingOvsExternalIDs *setting, + const char * key, + const char * val) +{ + NMSettingOvsExternalIDs * self = setting; + NMSettingOvsExternalIDsPrivate *priv; + + g_return_if_fail(NM_IS_SETTING_OVS_EXTERNAL_IDS(self)); + + priv = NM_SETTING_OVS_EXTERNAL_IDS_GET_PRIVATE(self); + + if (!val) { + if (priv->data && g_hash_table_remove(priv->data, key)) + goto out_changed; + return; + } + + if (priv->data) { + const char *val2; + + if (g_hash_table_lookup_extended(priv->data, key, NULL, (gpointer *) &val2)) { + if (nm_streq(val, val2)) + return; + } + } else + priv->data = _create_data_hash(); + + g_hash_table_insert(priv->data, g_strdup(key), g_strdup(val)); + +out_changed: + nm_clear_g_free(&priv->data_keys); + _notify(self, PROP_DATA); +} + +/*****************************************************************************/ + +static gboolean +verify(NMSetting *setting, NMConnection *connection, GError **error) +{ + NMSettingOvsExternalIDs * self = NM_SETTING_OVS_EXTERNAL_IDS(setting); + NMSettingOvsExternalIDsPrivate *priv = NM_SETTING_OVS_EXTERNAL_IDS_GET_PRIVATE(self); + + if (priv->data) { + gs_free_error GError *local = NULL; + GHashTableIter iter; + const char * key; + const char * val; + + g_hash_table_iter_init(&iter, priv->data); + while (g_hash_table_iter_next(&iter, (gpointer *) &key, (gpointer *) &val)) { + if (!nm_setting_ovs_external_ids_check_key(key, &local)) { + g_set_error(error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_FAILED, + _("invalid key \"%s\": %s"), + key, + local->message); + } else if (!nm_setting_ovs_external_ids_check_val(val, &local)) { + g_set_error(error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_FAILED, + _("invalid value for \"%s\": %s"), + key, + local->message); + } else + continue; + g_prefix_error(error, + "%s.%s: ", + NM_SETTING_OVS_EXTERNAL_IDS_SETTING_NAME, + NM_SETTING_OVS_EXTERNAL_IDS_DATA); + return FALSE; + } + } + + if (priv->data && g_hash_table_size(priv->data) > MAX_NUM_KEYS) { + g_set_error(error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + _("maximum number of user data entries reached (%u instead of %u)"), + g_hash_table_size(priv->data), + (unsigned) MAX_NUM_KEYS); + g_prefix_error(error, + "%s.%s: ", + NM_SETTING_OVS_EXTERNAL_IDS_SETTING_NAME, + NM_SETTING_OVS_EXTERNAL_IDS_DATA); + return FALSE; + } + + if (connection) { + const char *type; + + type = nm_connection_get_connection_type(connection); + if (!type) { + NMSetting *s_base; + + s_base = _nm_connection_find_base_type_setting(connection); + if (s_base) + type = nm_setting_get_name(s_base); + } + if (!NM_IN_STRSET(type, + NM_SETTING_OVS_BRIDGE_SETTING_NAME, + NM_SETTING_OVS_PORT_SETTING_NAME, + NM_SETTING_OVS_INTERFACE_SETTING_NAME)) { + g_set_error_literal(error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + _("OVS external IDs can only be added to a profile of type OVS " + "bridge/port/interface")); + return FALSE; + } + } + + return TRUE; +} + +static NMTernary +compare_property(const NMSettInfoSetting *sett_info, + guint property_idx, + NMConnection * con_a, + NMSetting * set_a, + NMConnection * con_b, + NMSetting * set_b, + NMSettingCompareFlags flags) +{ + NMSettingOvsExternalIDsPrivate *priv; + NMSettingOvsExternalIDsPrivate *pri2; + + if (nm_streq(sett_info->property_infos[property_idx].name, NM_SETTING_OVS_EXTERNAL_IDS_DATA)) { + if (NM_FLAGS_HAS(flags, NM_SETTING_COMPARE_FLAG_INFERRABLE)) + return NM_TERNARY_DEFAULT; + + if (!set_b) + return TRUE; + + priv = NM_SETTING_OVS_EXTERNAL_IDS_GET_PRIVATE(NM_SETTING_OVS_EXTERNAL_IDS(set_a)); + pri2 = NM_SETTING_OVS_EXTERNAL_IDS_GET_PRIVATE(NM_SETTING_OVS_EXTERNAL_IDS(set_b)); + return nm_utils_hashtable_equal(priv->data, pri2->data, TRUE, g_str_equal); + } + + return NM_SETTING_CLASS(nm_setting_ovs_external_ids_parent_class) + ->compare_property(sett_info, property_idx, con_a, set_a, con_b, set_b, flags); +} + +/*****************************************************************************/ + +static void +get_property(GObject *object, guint prop_id, GValue *value, GParamSpec *pspec) +{ + NMSettingOvsExternalIDs * self = NM_SETTING_OVS_EXTERNAL_IDS(object); + NMSettingOvsExternalIDsPrivate *priv = NM_SETTING_OVS_EXTERNAL_IDS_GET_PRIVATE(self); + GHashTableIter iter; + GHashTable * data; + const char * key; + const char * val; + + switch (prop_id) { + case PROP_DATA: + data = _create_data_hash(); + if (priv->data) { + g_hash_table_iter_init(&iter, priv->data); + while (g_hash_table_iter_next(&iter, (gpointer *) &key, (gpointer *) &val)) + g_hash_table_insert(data, g_strdup(key), g_strdup(val)); + } + g_value_take_boxed(value, data); + 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) +{ + NMSettingOvsExternalIDs * self = NM_SETTING_OVS_EXTERNAL_IDS(object); + NMSettingOvsExternalIDsPrivate *priv = NM_SETTING_OVS_EXTERNAL_IDS_GET_PRIVATE(self); + + switch (prop_id) { + case PROP_DATA: + { + gs_unref_hashtable GHashTable *old = NULL; + GHashTableIter iter; + GHashTable * data; + const char * key; + const char * val; + + nm_clear_g_free(&priv->data_keys); + + old = g_steal_pointer(&priv->data); + + data = g_value_get_boxed(value); + if (nm_g_hash_table_size(data) <= 0) + return; + + priv->data = _create_data_hash(); + g_hash_table_iter_init(&iter, data); + while (g_hash_table_iter_next(&iter, (gpointer *) &key, (gpointer *) &val)) + g_hash_table_insert(priv->data, g_strdup(key), g_strdup(val)); + break; + } + default: + G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); + break; + } +} + +/*****************************************************************************/ + +static void +nm_setting_ovs_external_ids_init(NMSettingOvsExternalIDs *self) +{} + +/** + * nm_setting_ovs_external_ids_new: + * + * Creates a new #NMSettingOvsExternalIDs object with default values. + * + * Returns: (transfer full) (type NMSettingOvsExternalIDs): the new empty + * #NMSettingOvsExternalIDs object + * + * Since: 1.30 + */ +gpointer +nm_setting_ovs_external_ids_new(void) +{ + return g_object_new(NM_TYPE_SETTING_OVS_EXTERNAL_IDS, NULL); +} + +static void +finalize(GObject *object) +{ + NMSettingOvsExternalIDs * self = NM_SETTING_OVS_EXTERNAL_IDS(object); + NMSettingOvsExternalIDsPrivate *priv = NM_SETTING_OVS_EXTERNAL_IDS_GET_PRIVATE(self); + + g_free(priv->data_keys); + if (priv->data) + g_hash_table_unref(priv->data); + + G_OBJECT_CLASS(nm_setting_ovs_external_ids_parent_class)->finalize(object); +} + +static void +nm_setting_ovs_external_ids_class_init(NMSettingOvsExternalIDsClass *klass) +{ + GObjectClass * object_class = G_OBJECT_CLASS(klass); + NMSettingClass *setting_class = NM_SETTING_CLASS(klass); + GArray * properties_override = _nm_sett_info_property_override_create_array(); + + object_class->get_property = get_property; + object_class->set_property = set_property; + object_class->finalize = finalize; + + setting_class->compare_property = compare_property; + setting_class->verify = verify; + + /** + * NMSettingOvsExternalIDs:data: (type GHashTable(utf8,utf8)) + * + * A dictionary of key/value pairs with exernal-ids for OVS. + * + * Since: 1.30 + **/ + obj_properties[PROP_DATA] = g_param_spec_boxed(NM_SETTING_OVS_EXTERNAL_IDS_DATA, + "", + "", + G_TYPE_HASH_TABLE, + G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS); + _nm_properties_override_gobj(properties_override, + obj_properties[PROP_DATA], + &nm_sett_info_propert_type_strdict); + + g_object_class_install_properties(object_class, _PROPERTY_ENUMS_LAST, obj_properties); + + _nm_setting_class_commit_full(setting_class, + NM_META_SETTING_TYPE_OVS_EXTERNAL_IDS, + NULL, + properties_override); +} diff --git a/libnm-core/nm-setting-ovs-external-ids.h b/libnm-core/nm-setting-ovs-external-ids.h new file mode 100644 index 0000000000..0f2442b48f --- /dev/null +++ b/libnm-core/nm-setting-ovs-external-ids.h @@ -0,0 +1,68 @@ +/* SPDX-License-Identifier: LGPL-2.1+ */ +/* + * Copyright (C) 2017 - 2020 Red Hat, Inc. + */ + +#ifndef __NM_SETTING_OVS_EXTERNAL_IDS_H__ +#define __NM_SETTING_OVS_EXTERNAL_IDS_H__ + +#if !defined(__NETWORKMANAGER_H_INSIDE__) && !defined(NETWORKMANAGER_COMPILATION) + #error "Only can be included directly." +#endif + +#include "nm-setting.h" + +G_BEGIN_DECLS + +#define NM_TYPE_SETTING_OVS_EXTERNAL_IDS (nm_setting_ovs_external_ids_get_type()) +#define NM_SETTING_OVS_EXTERNAL_IDS(obj) \ + (G_TYPE_CHECK_INSTANCE_CAST((obj), NM_TYPE_SETTING_OVS_EXTERNAL_IDS, NMSettingOvsExternalIDs)) +#define NM_SETTING_OVS_EXTERNAL_IDS_CLASS(klass) \ + (G_TYPE_CHECK_CLASS_CAST((klass), \ + NM_TYPE_SETTING_OVS_EXTERNAL_IDS, \ + NMSettingOvsExternalIDsClass)) +#define NM_IS_SETTING_OVS_EXTERNAL_IDS(obj) \ + (G_TYPE_CHECK_INSTANCE_TYPE((obj), NM_TYPE_SETTING_OVS_EXTERNAL_IDS)) +#define NM_IS_SETTING_OVS_EXTERNAL_IDS_CLASS(klass) \ + (G_TYPE_CHECK_CLASS_TYPE((klass), NM_TYPE_SETTING_OVS_EXTERNAL_IDS)) +#define NM_SETTING_OVS_EXTERNAL_IDS_GET_CLASS(obj) \ + (G_TYPE_INSTANCE_GET_CLASS((obj), \ + NM_TYPE_SETTING_OVS_EXTERNAL_IDS, \ + NMSettingOvsExternalIDsClass)) + +#define NM_SETTING_OVS_EXTERNAL_IDS_SETTING_NAME "ovs-external-ids" + +#define NM_SETTING_OVS_EXTERNAL_IDS_DATA "data" + +typedef struct _NMSettingOvsExternalIDsClass NMSettingOvsExternalIDsClass; + +NM_AVAILABLE_IN_1_30 +GType nm_setting_ovs_external_ids_get_type(void); + +NM_AVAILABLE_IN_1_30 +void *nm_setting_ovs_external_ids_new(void); + +/*****************************************************************************/ + +NM_AVAILABLE_IN_1_30 +const char *const *nm_setting_ovs_external_ids_get_data_keys(NMSettingOvsExternalIDs *setting, + guint * out_len); + +NM_AVAILABLE_IN_1_30 +const char *nm_setting_ovs_external_ids_get_data(NMSettingOvsExternalIDs *setting, const char *key); + +NM_AVAILABLE_IN_1_30 +void nm_setting_ovs_external_ids_set_data(NMSettingOvsExternalIDs *setting, + const char * key, + const char * val); + +/*****************************************************************************/ + +NM_AVAILABLE_IN_1_30 +gboolean nm_setting_ovs_external_ids_check_key(const char *key, GError **error); +NM_AVAILABLE_IN_1_30 +gboolean nm_setting_ovs_external_ids_check_val(const char *val, GError **error); + +G_END_DECLS + +#endif /* __NM_SETTING_OVS_EXTERNAL_IDS_H__ */ diff --git a/libnm/libnm.ver b/libnm/libnm.ver index baf1e06f86..0086fc01d2 100644 --- a/libnm/libnm.ver +++ b/libnm/libnm.ver @@ -1766,5 +1766,12 @@ global: nm_keyfile_read; nm_keyfile_warn_severity_get_type; nm_keyfile_write; + nm_setting_ovs_external_ids_check_key; + nm_setting_ovs_external_ids_check_val; + nm_setting_ovs_external_ids_get_data; + nm_setting_ovs_external_ids_get_data_keys; + nm_setting_ovs_external_ids_get_type; + nm_setting_ovs_external_ids_new; + nm_setting_ovs_external_ids_set_data; nm_utils_print; } libnm_1_28_0; diff --git a/po/POTFILES.in b/po/POTFILES.in index ea2eafa3f3..f2718ac648 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -82,6 +82,7 @@ libnm-core/nm-setting-macvlan.c libnm-core/nm-setting-match.c libnm-core/nm-setting-olpc-mesh.c libnm-core/nm-setting-ovs-bridge.c +libnm-core/nm-setting-ovs-external-ids.c libnm-core/nm-setting-ovs-interface.c libnm-core/nm-setting-ovs-patch.c libnm-core/nm-setting-ovs-port.c diff --git a/shared/nm-meta-setting.c b/shared/nm-meta-setting.c index 51b90e6f05..5356b9a5de 100644 --- a/shared/nm-meta-setting.c +++ b/shared/nm-meta-setting.c @@ -33,6 +33,7 @@ #include "nm-setting-ovs-bridge.h" #include "nm-setting-ovs-interface.h" #include "nm-setting-ovs-dpdk.h" +#include "nm-setting-ovs-external-ids.h" #include "nm-setting-ovs-patch.h" #include "nm-setting-ovs-port.h" #include "nm-setting-ppp.h" @@ -312,6 +313,13 @@ const NMMetaSettingInfo nm_meta_setting_infos[] = { .setting_name = NM_SETTING_OVS_DPDK_SETTING_NAME, .get_setting_gtype = nm_setting_ovs_dpdk_get_type, }, + [NM_META_SETTING_TYPE_OVS_EXTERNAL_IDS] = + { + .meta_type = NM_META_SETTING_TYPE_OVS_EXTERNAL_IDS, + .setting_priority = NM_SETTING_PRIORITY_AUX, + .setting_name = NM_SETTING_OVS_EXTERNAL_IDS_SETTING_NAME, + .get_setting_gtype = nm_setting_ovs_external_ids_get_type, + }, [NM_META_SETTING_TYPE_OVS_INTERFACE] = { .meta_type = NM_META_SETTING_TYPE_OVS_INTERFACE, diff --git a/shared/nm-meta-setting.h b/shared/nm-meta-setting.h index dadf2f7217..9d268e0ec2 100644 --- a/shared/nm-meta-setting.h +++ b/shared/nm-meta-setting.h @@ -128,6 +128,7 @@ typedef enum { NM_META_SETTING_TYPE_MATCH, NM_META_SETTING_TYPE_OVS_BRIDGE, NM_META_SETTING_TYPE_OVS_DPDK, + NM_META_SETTING_TYPE_OVS_EXTERNAL_IDS, NM_META_SETTING_TYPE_OVS_INTERFACE, NM_META_SETTING_TYPE_OVS_PATCH, NM_META_SETTING_TYPE_OVS_PORT, From 7e39e23f64184ddd12a7bb4d34b4d4ae616c059d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 3 Nov 2020 13:47:00 +0100 Subject: [PATCH 11/35] examples: add "ovs-external-ids.py" example script And example script for getting and setting OVS external-ids. Since currently there is no nmcli support for these properties yet, the script becomes more interesting. This "example" is rather long, and it showcases less the usage of libnm (which is rather trivial, with respect to configuring NMSettingOvsExternalIDs). Instead, it aims to provide a useful command line tool for debugging. Hence, it's mostly concerned with an elaborate command line syntax and useful print output. --- Makefile.examples | 1 + examples/python/gi/ovs-external-ids.py | 425 +++++++++++++++++++++++++ 2 files changed, 426 insertions(+) create mode 100755 examples/python/gi/ovs-external-ids.py diff --git a/Makefile.examples b/Makefile.examples index 3ac292fdd8..3cb46293bb 100644 --- a/Makefile.examples +++ b/Makefile.examples @@ -181,6 +181,7 @@ EXTRA_DIST += \ examples/python/gi/nm-keyfile.py \ examples/python/gi/nm-update2.py \ examples/python/gi/nm-wg-set \ + examples/python/gi/ovs-external-ids.py \ examples/python/gi/setting-user-data.py \ examples/python/gi/show-wifi-networks.py \ examples/python/gi/update-ip4-method.py \ diff --git a/examples/python/gi/ovs-external-ids.py b/examples/python/gi/ovs-external-ids.py new file mode 100755 index 0000000000..e80da51120 --- /dev/null +++ b/examples/python/gi/ovs-external-ids.py @@ -0,0 +1,425 @@ +#!/usr/bin/env python +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright (C) 2017, 2020 Red Hat, Inc. +# + +# +# set and show OVS external-ids for a connection: +# + +import sys +import os +import re +import pprint + +import gi + +gi.require_version("NM", "1.0") +from gi.repository import GLib, NM + +MODE_GET = "get" +MODE_SET = "set" + + +def pr(v): + pprint.pprint(v, indent=4, depth=5, width=60) + + +HAS_LIBNM_DEBUG = os.getenv("LIBNM_CLIENT_DEBUG") is not None + + +def _print(msg=""): + if HAS_LIBNM_DEBUG: + # we want to use the same logging mechanism as libnm's debug + # logging with "LIBNM_CLIENT_DEBUG=trace,stdout". + NM.utils_print(0, msg + "\n") + return + print(msg) + + +def mainloop_run(timeout_msec=0, mainloop=None): + if mainloop is None: + mainloop = GLib.MainLoop() + + timeout_id = None + timeout_reached = [] + + if timeout_msec > 0: + + def _timeout_cb(unused): + # it can happen that the caller already quit the mainloop + # otherwise. In that case, we don't want to signal a timeout. + if mainloop.is_running(): + timeout_reached.append(1) + mainloop.quit() + return True + + timeout_id = GLib.timeout_add(timeout_msec, _timeout_cb, None) + + mainloop.run() + if timeout_id: + GLib.source_remove(timeout_id) + return not timeout_reached + + +def usage(): + _print("%s g[et] PROFILE [ GETTER ]" % (sys.argv[0])) + _print("%s s[et] [--test] PROFILE SETTER" % (sys.argv[0])) + _print( + " PROFILE := [id | uuid | type] STRING | [ ~id | ~type ] REGEX_STRING | STRING" + ) + _print(" GETTER := ( KEY | ~REGEX_KEY ) [... GETTER]") + _print(" SETTER := ( + | - | -KEY | [+]KEY VALUE ) [... SETTER]") + + +def die(msg, show_usage=False): + _print("FAILED: %s" % (msg)) + if show_usage: + usage() + sys.exit(1) + + +def die_usage(msg): + die(msg, show_usage=True) + + +def parse_args(argv): + had_dash_dash = False + args = { + "mode": MODE_GET, + "profile_arg": None, + "ids_arg": [], + "do_test": False, + } + i = 1 + while i < len(argv): + a = argv[i] + + if i == 1: + if a in ["s", "set"]: + args["mode"] = MODE_SET + elif a in ["g", "get"]: + args["mode"] = MODE_GET + else: + die_usage("unexpected mode argument '%s'" % (a)) + i += 1 + continue + + if a == "--test": + args["do_test"] = True + i += 1 + continue + + if args["profile_arg"] is None: + if a in ["id", "~id", "uuid", "type", "~type"]: + if i + 1 >= len(argv): + die_usage("'%s' requires an argument'" % (a)) + args["profile_arg"] = (a, argv[i + 1]) + i += 2 + continue + + if a == "*": + a = None + args["profile_arg"] = ("*", a) + i += 1 + continue + + if args["mode"] == MODE_GET: + args["ids_arg"].append(a) + i += 1 + continue + + if not a: + die_usage("argument should specify a external-id but is empty string") + + if a[0] == "-": + v = (a, None) + i += 1 + elif a == "+": + v = (a, None) + i += 1 + else: + if a[0] != "+": + a = "+" + a + if i + 1 >= len(argv): + die_usage("'%s' requires an argument'" % (a)) + v = (a, argv[i + 1]) + i += 2 + + args["ids_arg"].append(v) + + if args["mode"] == MODE_SET: + if not args["ids_arg"]: + die_usage("Requires one or more external-ids to set or delete") + + return args + + +def connection_to_str(connection, show_type=False): + if show_type: + return "%s (%s, %s)" % ( + connection.get_id(), + connection.get_uuid(), + connection.get_connection_type(), + ) + return "%s (%s)" % (connection.get_id(), connection.get_uuid()) + + +def connections_filter(connections, profile_arg): + connections = list(sorted(connections, key=connection_to_str)) + if not profile_arg: + return connections + # we preserve the order of the selected connections. And + # if connections are selected multiple times, we return + # them multiple times. + l = [] + f = profile_arg + for c in connections: + if f[0] == "id": + if f[1] == c.get_id(): + l.append(c) + elif f[0] == "~id": + if re.match(f[1], c.get_id()): + l.append(c) + elif f[0] == "uuid": + if f[1] == c.get_uuid(): + l.append(c) + elif f[0] == "type": + if f[1] == c.get_connection_type(): + l.append(c) + elif f[0] == "~type": + if re.match(f[1], c.get_connection_type()): + l.append(c) + else: + assert f[0] == "*" + if f[1] is None: + l.append(c) + else: + if f[1] in [c.get_uuid(), c.get_id()]: + l.append(c) + return l + + +def ids_select(ids, mode, ids_arg): + ids = list(ids) + if not ids_arg: + return (ids, []) + + keys = set() + requested = [] + for d in ids_arg: + if mode == MODE_GET: + if d[0] == "~": + r = re.compile(d[1:]) + keys.update([k for k in ids if r.match(k)]) + else: + keys.update([k for k in ids if k == d]) + if d not in requested: + requested.append(d) + else: + d2 = d[0] + assert d2[0] in ["-", "+"] + d3 = d2[1:] + if d3 in ids: + keys.add(d3) + return (list([k for k in ids if k in keys]), requested) + + +def connection_print(connection, mode, ids_arg, dbus_path, prefix=""): + sett = connection.get_setting(NM.SettingOvsExternalIDs) + + if sett is not None: + all_ids = list(sett.get_data_keys()) + keys, requested = ids_select(all_ids, mode, ids_arg) + num_str = "%s" % (len(all_ids)) + else: + keys = [] + requested = [] + num_str = "none" + + _print( + "%s%s [%s]" % (prefix, connection_to_str(connection, show_type=True), num_str) + ) + _print("%s %s" % (prefix, dbus_path)) + if sett is not None: + dd = sett.get_property(NM.SETTING_OVS_EXTERNAL_IDS_DATA) + else: + dd = {} + for k in keys: + v = sett.get_data(k) + assert v is not None + assert v == dd.get(k, None) + _print('%s "%s" = "%s"' % (prefix, k, v)) + for k in requested: + _print('%s "%s" = ' % (prefix, k)) + + +def do_get(connections, ids_arg): + first_line = True + for c in connections: + if first_line: + first_line = False + else: + _print() + connection_print(c, MODE_GET, ids_arg, dbus_path=c.get_path()) + + +def do_set(nmc, connection, ids_arg, do_test): + + remote_connection = connection + connection = NM.SimpleConnection.new_clone(remote_connection) + + connection_print( + connection, MODE_SET, [], remote_connection.get_path(), prefix="BEFORE: " + ) + _print() + + sett = connection.get_setting(NM.SettingOvsExternalIDs) + + for d in ids_arg: + op = d[0][0] + key = d[0][1:] + val = d[1] + + oldval = None + if sett is not None: + oldval = sett.get_data(key) + + if op == "-": + assert val is None + if key == "": + if sett is None: + _print(" DEL: setting (ovs-external-ids group was not present)") + else: + connection.remove_setting(NM.SettingOvsExternalIDs) + sett = None + _print(" DEL: setting") + continue + + if sett is None: + _print(' DEL: "%s" (ovs-external-ids group was not present)' % (key)) + continue + if oldval is None: + _print(' DEL: "%s" (id was unset)' % (key)) + continue + _print(' DEL: "%s" (id was set to"%s")' % (key, oldval)) + sett.set_data(key, None) + continue + + if key == "": + assert val is None + if sett is None: + sett = NM.SettingOvsExternalIDs.new() + connection.add_setting(sett) + _print(" SET: setting (external-ids group was added)") + continue + + _print(" SET: setting (external-ids group was present)") + continue + + assert val is not None + + if sett is None: + sett = NM.SettingOvsExternalIDs.new() + connection.add_setting(sett) + _print( + ' SET: "%s" = "%s" (external-ids group was not present)' % (key, val) + ) + elif oldval is None: + _print(' SET: "%s" = "%s" (new)' % (key, val)) + elif oldval != val: + _print(' SET: "%s" = "%s" (was "%s")' % (key, val, oldval)) + else: + _print(' SET: "%s" = "%s" (unchanged)' % (key, val)) + sett.set_data(key, val) + + if do_test: + _print() + _print("Only show. Run without --test to set") + return + + mainloop = GLib.MainLoop() + result_error = [] + + def callback(c, result): + try: + c.update2_finish(result) + except Exception as e: + result_error.append(e) + mainloop.quit() + + remote_connection.update2( + connection.to_dbus(NM.ConnectionSerializationFlags.ALL), + NM.SettingsUpdate2Flags.NO_REAPPLY, + None, + None, + callback, + ) + + mainloop_run(mainloop=mainloop) + + if result_error: + _print() + _print("FAILURE to commit connection: %s" % (result_error[0])) + return + + # NMClient received the completion of Update2() call. It also received + # a property changed signal that the profile changed, and it is about + # to fetch the new value. However, that value is not yet here. + # + # libnm should provide a better API for this. For example, not signal + # completion of update2() until the profile was refetched. Or, indicate + # that the settings are dirty, so we would know how long to wait. + # + # Add an ugly workaround here and wait a bit. + _print() + _print("WORKAROUND: wait for connection to change") + mainloop_run(timeout_msec=500) + + if remote_connection is not nmc.get_object_by_path(remote_connection.get_path()): + _print() + _print( + "Connection %s no longer exists after commit" + % (remote_connection.get_path()) + ) + return + + _print() + connection_print( + remote_connection, MODE_SET, [], remote_connection.get_path(), prefix="AFTER: " + ) + + _print() + if remote_connection.compare(connection, NM.SettingCompareFlags.EXACT): + _print("resulting connection is as expected") + else: + _print("WARNING: resulting connection is not as expected") + + +############################################################################### + +if __name__ == "__main__": + + args = parse_args(sys.argv) + + nmc = NM.Client.new(None) + + connections = connections_filter(nmc.get_connections(), args["profile_arg"]) + + if args["mode"] == MODE_SET: + if len(connections) != 1: + _print( + "To set the external-ids of a connection, exactly one connection must be selected via id|uuid. Instead, %s connection matched ([%s])" + % ( + len(connections), + ", ".join([connection_to_str(c) for c in connections]), + ) + ) + die_usage("Select unique connection to set") + do_set(nmc, connections[0], args["ids_arg"], do_test=args["do_test"]) + else: + if len(connections) < 1: + _print("No connection selected for printing the external ids") + die_usage("Select connection to get") + do_get(connections, args["ids_arg"]) From 7d5ec103df5209ca7fbf74309df53af87dd42e5e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 5 Nov 2020 20:27:59 +0100 Subject: [PATCH 12/35] format: mark json_{object,array}_foreach() macors as ForEachMacros for clang-format --- .clang-format | 2 ++ src/devices/ovs/nm-ovsdb.c | 18 ++++++------------ 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/.clang-format b/.clang-format index f84ea591c3..f0331adefc 100644 --- a/.clang-format +++ b/.clang-format @@ -85,6 +85,8 @@ ForEachMacros: ['c_list_for_each', 'c_rbtree_for_each_safe', 'c_rbtree_for_each_safe_postorder', 'c_rbtree_for_each_safe_postorder_unlink', + 'json_array_foreach', + 'json_object_foreach', 'json_object_foreach_safe', 'ndp_msg_opt_dnssl_for_each_domain', 'ndp_msg_opt_for_each_offset', diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c index 9826c1b7c5..f3ef54a1b0 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -1101,8 +1101,7 @@ _uuids_to_array(GPtrArray *array, const json_t *items) if (g_strcmp0(key, "uuid") == 0 && json_is_string(value)) { g_ptr_array_add(array, g_strdup(json_string_value(value))); } else if (g_strcmp0(key, "set") == 0 && json_is_array(value)) { - json_array_foreach(value, set_index, set_value) - { + json_array_foreach (value, set_index, set_value) { _uuids_to_array(array, set_value); } } @@ -1118,8 +1117,7 @@ _connection_uuid_from_external_ids(json_t *external_ids) if (g_strcmp0("map", json_string_value(json_array_get(external_ids, 0))) != 0) return NULL; - json_array_foreach(json_array_get(external_ids, 1), index, value) - { + json_array_foreach (json_array_get(external_ids, 1), index, value) { if (g_strcmp0("NM.connection.uuid", json_string_value(json_array_get(value, 0))) == 0) return g_strdup(json_string_value(json_array_get(value, 1))); } @@ -1180,8 +1178,7 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) } /* Interfaces */ - json_object_foreach(interface, key, value) - { + json_object_foreach (interface, key, value) { json_t * error = NULL; gboolean old = FALSE; gboolean new = FALSE; @@ -1269,8 +1266,7 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) } /* Ports */ - json_object_foreach(port, key, value) - { + json_object_foreach (port, key, value) { gboolean old = FALSE; gboolean new = FALSE; @@ -1333,8 +1329,7 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) } /* Bridges */ - json_object_foreach(bridge, key, value) - { + json_object_foreach (bridge, key, value) { gboolean old = FALSE; gboolean new = FALSE; @@ -1838,8 +1833,7 @@ _transact_cb(NMOvsdb *self, json_t *result, GError *error, gpointer user_data) if (error) goto out; - json_array_foreach(result, index, value) - { + json_array_foreach (result, index, value) { if (json_unpack(value, "{s:s, s:s}", "error", &err, "details", &err_details) == 0) { g_set_error(&error, G_IO_ERROR, From cc35dc3bdf5f76cac5ed37127ca89eef18bb9998 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 6 Nov 2020 18:22:11 +0100 Subject: [PATCH 13/35] device: improve "nm-device-logging.h" to support a self pointer of NMDevice type "nm-device-logging.h" defines logging macros for a NMDevice instance. It also expects a "self" variable in the call environment, and that variable had to be in the type of NMDevice or the NMDevice subclass. Extend the macro foo, so that @self can be either a NMDevice* pointer or a NMDevice$SUBTYPE. Of course, that would have always been possible, if we would simply cast to "(NMDevice *)" where we need it. The trick is that the macro only works if @self is one of the two expected types, and not some arbitrary unrelated type. --- src/devices/adsl/nm-device-adsl.c | 2 +- src/devices/bluetooth/nm-device-bt.c | 2 +- src/devices/nm-device-6lowpan.c | 2 +- src/devices/nm-device-bond.c | 2 +- src/devices/nm-device-bridge.c | 2 +- src/devices/nm-device-dummy.c | 2 +- src/devices/nm-device-ethernet.c | 2 +- src/devices/nm-device-ip-tunnel.c | 2 +- src/devices/nm-device-logging.h | 58 ++++++++++++----------- src/devices/nm-device-macsec.c | 2 +- src/devices/nm-device-macvlan.c | 2 +- src/devices/nm-device-ppp.c | 2 +- src/devices/nm-device-tun.c | 2 +- src/devices/nm-device-veth.c | 2 +- src/devices/nm-device-vlan.c | 2 +- src/devices/nm-device-vrf.c | 2 +- src/devices/nm-device-vxlan.c | 2 +- src/devices/nm-device-wireguard.c | 2 +- src/devices/nm-device-wpan.c | 2 +- src/devices/nm-device.c | 1 - src/devices/ovs/nm-device-ovs-bridge.c | 2 +- src/devices/ovs/nm-device-ovs-interface.c | 2 +- src/devices/ovs/nm-device-ovs-port.c | 2 +- src/devices/team/nm-device-team.c | 2 +- src/devices/wifi/nm-device-iwd.c | 2 +- src/devices/wifi/nm-device-olpc-mesh.c | 2 +- src/devices/wifi/nm-device-wifi-p2p.c | 2 +- src/devices/wifi/nm-device-wifi.c | 2 +- src/devices/wwan/nm-device-modem.c | 2 +- 29 files changed, 58 insertions(+), 55 deletions(-) diff --git a/src/devices/adsl/nm-device-adsl.c b/src/devices/adsl/nm-device-adsl.c index 3261562399..556dfd829b 100644 --- a/src/devices/adsl/nm-device-adsl.c +++ b/src/devices/adsl/nm-device-adsl.c @@ -23,8 +23,8 @@ #include "nm-setting-adsl.h" #include "nm-utils.h" +#define _NMLOG_DEVICE_TYPE NMDeviceAdsl #include "devices/nm-device-logging.h" -_LOG_DECLARE_SELF(NMDeviceAdsl); /*****************************************************************************/ diff --git a/src/devices/bluetooth/nm-device-bt.c b/src/devices/bluetooth/nm-device-bt.c index c31961df92..ca2a59731d 100644 --- a/src/devices/bluetooth/nm-device-bt.c +++ b/src/devices/bluetooth/nm-device-bt.c @@ -30,8 +30,8 @@ #include "devices/wwan/nm-modem-manager.h" #include "devices/wwan/nm-modem.h" +#define _NMLOG_DEVICE_TYPE NMDeviceBt #include "devices/nm-device-logging.h" -_LOG_DECLARE_SELF(NMDeviceBt); /*****************************************************************************/ diff --git a/src/devices/nm-device-6lowpan.c b/src/devices/nm-device-6lowpan.c index bf63fb7337..5ab0b933f6 100644 --- a/src/devices/nm-device-6lowpan.c +++ b/src/devices/nm-device-6lowpan.c @@ -14,8 +14,8 @@ #include "nm-setting-6lowpan.h" #include "nm-utils.h" +#define _NMLOG_DEVICE_TYPE NMDevice6Lowpan #include "nm-device-logging.h" -_LOG_DECLARE_SELF(NMDevice6Lowpan); /*****************************************************************************/ diff --git a/src/devices/nm-device-bond.c b/src/devices/nm-device-bond.c index 43f2a92ef4..0fa4be9158 100644 --- a/src/devices/nm-device-bond.c +++ b/src/devices/nm-device-bond.c @@ -17,8 +17,8 @@ #include "nm-core-internal.h" #include "nm-ip4-config.h" +#define _NMLOG_DEVICE_TYPE NMDeviceBond #include "nm-device-logging.h" -_LOG_DECLARE_SELF(NMDeviceBond); /*****************************************************************************/ diff --git a/src/devices/nm-device-bridge.c b/src/devices/nm-device-bridge.c index 48dcec1b1d..930c93489f 100644 --- a/src/devices/nm-device-bridge.c +++ b/src/devices/nm-device-bridge.c @@ -15,8 +15,8 @@ #include "nm-device-factory.h" #include "nm-core-internal.h" +#define _NMLOG_DEVICE_TYPE NMDeviceBridge #include "nm-device-logging.h" -_LOG_DECLARE_SELF(NMDeviceBridge); /*****************************************************************************/ diff --git a/src/devices/nm-device-dummy.c b/src/devices/nm-device-dummy.c index 4f6b8cc234..364a634647 100644 --- a/src/devices/nm-device-dummy.c +++ b/src/devices/nm-device-dummy.c @@ -18,8 +18,8 @@ #include "nm-setting-dummy.h" #include "nm-core-internal.h" +#define _NMLOG_DEVICE_TYPE NMDeviceDummy #include "nm-device-logging.h" -_LOG_DECLARE_SELF(NMDeviceDummy); /*****************************************************************************/ diff --git a/src/devices/nm-device-ethernet.c b/src/devices/nm-device-ethernet.c index 2569c4262c..a1ea1cf93a 100644 --- a/src/devices/nm-device-ethernet.c +++ b/src/devices/nm-device-ethernet.c @@ -35,8 +35,8 @@ #include "NetworkManagerUtils.h" #include "nm-udev-aux/nm-udev-utils.h" +#define _NMLOG_DEVICE_TYPE NMDeviceEthernet #include "nm-device-logging.h" -_LOG_DECLARE_SELF(NMDeviceEthernet); /*****************************************************************************/ diff --git a/src/devices/nm-device-ip-tunnel.c b/src/devices/nm-device-ip-tunnel.c index e01cc7ff74..21307cab9e 100644 --- a/src/devices/nm-device-ip-tunnel.c +++ b/src/devices/nm-device-ip-tunnel.c @@ -22,8 +22,8 @@ #include "nm-act-request.h" #include "nm-ip4-config.h" +#define _NMLOG_DEVICE_TYPE NMDeviceIPTunnel #include "nm-device-logging.h" -_LOG_DECLARE_SELF(NMDeviceIPTunnel); /*****************************************************************************/ diff --git a/src/devices/nm-device-logging.h b/src/devices/nm-device-logging.h index d12910207e..ff3d4180dc 100644 --- a/src/devices/nm-device-logging.h +++ b/src/devices/nm-device-logging.h @@ -8,36 +8,40 @@ #include "nm-device.h" -#define _LOG_DECLARE_SELF(t) \ - _nm_unused static inline NMDevice *_nm_device_log_self_to_device(t *self) \ - { \ - return (NMDevice *) self; \ - } +#if !_NM_CC_SUPPORT_GENERIC + #define _NM_DEVICE_CAST(self) ((NMDevice *) (self)) +#elif !defined(_NMLOG_DEVICE_TYPE) + #define _NM_DEVICE_CAST(self) _NM_ENSURE_TYPE(NMDevice *, self) +#else + #define _NM_DEVICE_CAST(self) \ + _Generic((self), _NMLOG_DEVICE_TYPE * \ + : ((NMDevice *) (self)), NMDevice * \ + : ((NMDevice *) (self))) +#endif #undef _NMLOG_ENABLED #define _NMLOG_ENABLED(level, domain) (nm_logging_enabled((level), (domain))) -#define _NMLOG(level, domain, ...) \ - G_STMT_START \ - { \ - const NMLogLevel _level = (level); \ - const NMLogDomain _domain = (domain); \ - \ - if (nm_logging_enabled(_level, _domain)) { \ - typeof(*self) *const _self = (self); \ - const char *const _ifname = \ - _nm_device_get_iface(_nm_device_log_self_to_device(_self)); \ - \ - nm_log_obj(_level, \ - _domain, \ - _ifname, \ - NULL, \ - _self, \ - "device", \ - "%s%s%s: " _NM_UTILS_MACRO_FIRST(__VA_ARGS__), \ - NM_PRINT_FMT_QUOTED(_ifname, "(", _ifname, ")", "[null]") \ - _NM_UTILS_MACRO_REST(__VA_ARGS__)); \ - } \ - } \ +#define _NMLOG(level, domain, ...) \ + G_STMT_START \ + { \ + const NMLogLevel _level = (level); \ + const NMLogDomain _domain = (domain); \ + \ + if (nm_logging_enabled(_level, _domain)) { \ + typeof(*self) *const _self = (self); \ + const char *const _ifname = _nm_device_get_iface(_NM_DEVICE_CAST(_self)); \ + \ + nm_log_obj(_level, \ + _domain, \ + _ifname, \ + NULL, \ + _self, \ + "device", \ + "%s%s%s: " _NM_UTILS_MACRO_FIRST(__VA_ARGS__), \ + NM_PRINT_FMT_QUOTED(_ifname, "(", _ifname, ")", "[null]") \ + _NM_UTILS_MACRO_REST(__VA_ARGS__)); \ + } \ + } \ G_STMT_END #endif /* __NETWORKMANAGER_DEVICE_LOGGING_H__ */ diff --git a/src/devices/nm-device-macsec.c b/src/devices/nm-device-macsec.c index 7d738b4243..3e3ed30f4d 100644 --- a/src/devices/nm-device-macsec.c +++ b/src/devices/nm-device-macsec.c @@ -18,8 +18,8 @@ #include "supplicant/nm-supplicant-interface.h" #include "supplicant/nm-supplicant-config.h" +#define _NMLOG_DEVICE_TYPE NMDeviceMacsec #include "nm-device-logging.h" -_LOG_DECLARE_SELF(NMDeviceMacsec); /*****************************************************************************/ diff --git a/src/devices/nm-device-macvlan.c b/src/devices/nm-device-macvlan.c index d9e71c8ca8..12c53a3b08 100644 --- a/src/devices/nm-device-macvlan.c +++ b/src/devices/nm-device-macvlan.c @@ -21,8 +21,8 @@ #include "nm-ip4-config.h" #include "nm-utils.h" +#define _NMLOG_DEVICE_TYPE NMDeviceMacvlan #include "nm-device-logging.h" -_LOG_DECLARE_SELF(NMDeviceMacvlan); /*****************************************************************************/ diff --git a/src/devices/nm-device-ppp.c b/src/devices/nm-device-ppp.c index 82861bdafc..0cdb7ac935 100644 --- a/src/devices/nm-device-ppp.c +++ b/src/devices/nm-device-ppp.c @@ -18,8 +18,8 @@ #include "ppp/nm-ppp-manager-call.h" #include "ppp/nm-ppp-status.h" +#define _NMLOG_DEVICE_TYPE NMDevicePpp #include "nm-device-logging.h" -_LOG_DECLARE_SELF(NMDevicePpp); /*****************************************************************************/ diff --git a/src/devices/nm-device-tun.c b/src/devices/nm-device-tun.c index 9480525502..ae1550809b 100644 --- a/src/devices/nm-device-tun.c +++ b/src/devices/nm-device-tun.c @@ -19,8 +19,8 @@ #include "nm-setting-tun.h" #include "nm-core-internal.h" +#define _NMLOG_DEVICE_TYPE NMDeviceTun #include "nm-device-logging.h" -_LOG_DECLARE_SELF(NMDeviceTun); /*****************************************************************************/ diff --git a/src/devices/nm-device-veth.c b/src/devices/nm-device-veth.c index f61bc99f2a..7982a7faa4 100644 --- a/src/devices/nm-device-veth.c +++ b/src/devices/nm-device-veth.c @@ -13,8 +13,8 @@ #include "platform/nm-platform.h" #include "nm-device-factory.h" +#define _NMLOG_DEVICE_TYPE NMDeviceVeth #include "nm-device-logging.h" -_LOG_DECLARE_SELF(NMDeviceVeth); /*****************************************************************************/ diff --git a/src/devices/nm-device-vlan.c b/src/devices/nm-device-vlan.c index 8a743d1e62..5f036cd54b 100644 --- a/src/devices/nm-device-vlan.c +++ b/src/devices/nm-device-vlan.c @@ -22,8 +22,8 @@ #include "nm-core-internal.h" #include "platform/nmp-object.h" +#define _NMLOG_DEVICE_TYPE NMDeviceVlan #include "nm-device-logging.h" -_LOG_DECLARE_SELF(NMDeviceVlan); /*****************************************************************************/ diff --git a/src/devices/nm-device-vrf.c b/src/devices/nm-device-vrf.c index ae0ecc60aa..9c3927e146 100644 --- a/src/devices/nm-device-vrf.c +++ b/src/devices/nm-device-vrf.c @@ -12,8 +12,8 @@ #include "platform/nm-platform.h" #include "settings/nm-settings.h" +#define _NMLOG_DEVICE_TYPE NMDeviceVrf #include "nm-device-logging.h" -_LOG_DECLARE_SELF(NMDeviceVrf); /*****************************************************************************/ diff --git a/src/devices/nm-device-vxlan.c b/src/devices/nm-device-vxlan.c index 2ed66a644d..e9dd969886 100644 --- a/src/devices/nm-device-vxlan.c +++ b/src/devices/nm-device-vxlan.c @@ -19,8 +19,8 @@ #include "nm-ip4-config.h" #include "nm-core-internal.h" +#define _NMLOG_DEVICE_TYPE NMDeviceVxlan #include "nm-device-logging.h" -_LOG_DECLARE_SELF(NMDeviceVxlan); /*****************************************************************************/ diff --git a/src/devices/nm-device-wireguard.c b/src/devices/nm-device-wireguard.c index 9bd23e6564..c8bc4bfcb2 100644 --- a/src/devices/nm-device-wireguard.c +++ b/src/devices/nm-device-wireguard.c @@ -22,8 +22,8 @@ #include "nm-act-request.h" #include "dns/nm-dns-manager.h" +#define _NMLOG_DEVICE_TYPE NMDeviceWireGuard #include "nm-device-logging.h" -_LOG_DECLARE_SELF(NMDeviceWireGuard); /*****************************************************************************/ diff --git a/src/devices/nm-device-wpan.c b/src/devices/nm-device-wpan.c index 195603dcde..f8e2b641ca 100644 --- a/src/devices/nm-device-wpan.c +++ b/src/devices/nm-device-wpan.c @@ -20,8 +20,8 @@ #include "nm-setting-wpan.h" #include "nm-core-internal.h" +#define _NMLOG_DEVICE_TYPE NMDeviceWpan #include "nm-device-logging.h" -_LOG_DECLARE_SELF(NMDeviceWpan); /*****************************************************************************/ diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 6e53048bb4..df3669aceb 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -74,7 +74,6 @@ #include "nm-device-wireguard.h" #include "nm-device-logging.h" -_LOG_DECLARE_SELF(NMDevice); /*****************************************************************************/ diff --git a/src/devices/ovs/nm-device-ovs-bridge.c b/src/devices/ovs/nm-device-ovs-bridge.c index a7b7a539e2..6c92544c9c 100644 --- a/src/devices/ovs/nm-device-ovs-bridge.c +++ b/src/devices/ovs/nm-device-ovs-bridge.c @@ -14,8 +14,8 @@ #include "nm-setting-connection.h" #include "nm-setting-ovs-bridge.h" +#define _NMLOG_DEVICE_TYPE NMDeviceOvsBridge #include "devices/nm-device-logging.h" -_LOG_DECLARE_SELF(NMDeviceOvsBridge); /*****************************************************************************/ diff --git a/src/devices/ovs/nm-device-ovs-interface.c b/src/devices/ovs/nm-device-ovs-interface.c index 16b6e04da0..0084c8bc58 100644 --- a/src/devices/ovs/nm-device-ovs-interface.c +++ b/src/devices/ovs/nm-device-ovs-interface.c @@ -14,8 +14,8 @@ #include "nm-setting-ovs-interface.h" #include "nm-setting-ovs-port.h" +#define _NMLOG_DEVICE_TYPE NMDeviceOvsInterface #include "devices/nm-device-logging.h" -_LOG_DECLARE_SELF(NMDeviceOvsInterface); /*****************************************************************************/ diff --git a/src/devices/ovs/nm-device-ovs-port.c b/src/devices/ovs/nm-device-ovs-port.c index de98e29e06..d3a462753a 100644 --- a/src/devices/ovs/nm-device-ovs-port.c +++ b/src/devices/ovs/nm-device-ovs-port.c @@ -15,8 +15,8 @@ #include "nm-setting-ovs-port.h" #include "nm-setting-ovs-port.h" +#define _NMLOG_DEVICE_TYPE NMDeviceOvsPort #include "devices/nm-device-logging.h" -_LOG_DECLARE_SELF(NMDeviceOvsPort); /*****************************************************************************/ diff --git a/src/devices/team/nm-device-team.c b/src/devices/team/nm-device-team.c index 27aa300430..65fd70c78a 100644 --- a/src/devices/team/nm-device-team.c +++ b/src/devices/team/nm-device-team.c @@ -25,8 +25,8 @@ #include "nm-ip4-config.h" #include "nm-std-aux/nm-dbus-compat.h" +#define _NMLOG_DEVICE_TYPE NMDeviceTeam #include "devices/nm-device-logging.h" -_LOG_DECLARE_SELF(NMDeviceTeam); /*****************************************************************************/ diff --git a/src/devices/wifi/nm-device-iwd.c b/src/devices/wifi/nm-device-iwd.c index c479b3d453..e59dafdefe 100644 --- a/src/devices/wifi/nm-device-iwd.c +++ b/src/devices/wifi/nm-device-iwd.c @@ -28,8 +28,8 @@ #include "settings/nm-settings.h" #include "supplicant/nm-supplicant-types.h" +#define _NMLOG_DEVICE_TYPE NMDeviceIwd #include "devices/nm-device-logging.h" -_LOG_DECLARE_SELF(NMDeviceIwd); /*****************************************************************************/ diff --git a/src/devices/wifi/nm-device-olpc-mesh.c b/src/devices/wifi/nm-device-olpc-mesh.c index 22e94a6dcc..4b9c640149 100644 --- a/src/devices/wifi/nm-device-olpc-mesh.c +++ b/src/devices/wifi/nm-device-olpc-mesh.c @@ -30,8 +30,8 @@ #include "nm-manager.h" #include "platform/nm-platform.h" +#define _NMLOG_DEVICE_TYPE NMDeviceOlpcMesh #include "devices/nm-device-logging.h" -_LOG_DECLARE_SELF(NMDeviceOlpcMesh); /*****************************************************************************/ diff --git a/src/devices/wifi/nm-device-wifi-p2p.c b/src/devices/wifi/nm-device-wifi-p2p.c index 2e5317fd95..0780d80b34 100644 --- a/src/devices/wifi/nm-device-wifi-p2p.c +++ b/src/devices/wifi/nm-device-wifi-p2p.c @@ -27,8 +27,8 @@ #include "platform/nmp-object.h" #include "settings/nm-settings.h" +#define _NMLOG_DEVICE_TYPE NMDeviceWifiP2P #include "devices/nm-device-logging.h" -_LOG_DECLARE_SELF(NMDeviceWifiP2P); /*****************************************************************************/ diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 7c749f1e3c..24de0cdeca 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -41,8 +41,8 @@ #include "nm-core-internal.h" #include "nm-config.h" +#define _NMLOG_DEVICE_TYPE NMDeviceWifi #include "devices/nm-device-logging.h" -_LOG_DECLARE_SELF(NMDeviceWifi); #define SCAN_INTERVAL_SEC_MIN 3 #define SCAN_INTERVAL_SEC_STEP 20 diff --git a/src/devices/wwan/nm-device-modem.c b/src/devices/wwan/nm-device-modem.c index f77de85aac..5b8b3bd29d 100644 --- a/src/devices/wwan/nm-device-modem.c +++ b/src/devices/wwan/nm-device-modem.c @@ -16,8 +16,8 @@ #include "NetworkManagerUtils.h" #include "nm-core-internal.h" +#define _NMLOG_DEVICE_TYPE NMDeviceModem #include "devices/nm-device-logging.h" -_LOG_DECLARE_SELF(NMDeviceModem); /*****************************************************************************/ From d75c31afd02b0b68bb925109d146c95150474035 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 6 Nov 2020 17:19:33 +0100 Subject: [PATCH 14/35] device: refactor NMDevice's can_reapply_change() to return early Don't have if-else-if structure, if we can always return from an "if" block, once we matched the setting-name. --- src/devices/nm-device.c | 48 +++++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index df3669aceb..b77d0fd36a 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -12387,28 +12387,34 @@ can_reapply_change(NMDevice * self, NM_SETTING_CONNECTION_LLDP, NM_SETTING_CONNECTION_MDNS, NM_SETTING_CONNECTION_LLMNR); - } else if (NM_IN_STRSET(setting_name, - NM_SETTING_USER_SETTING_NAME, - NM_SETTING_PROXY_SETTING_NAME, - NM_SETTING_IP4_CONFIG_SETTING_NAME, - NM_SETTING_IP6_CONFIG_SETTING_NAME)) { - return TRUE; - } else if (nm_streq(setting_name, NM_SETTING_WIRED_SETTING_NAME) - && NM_IN_SET(NM_DEVICE_GET_CLASS(self)->get_configured_mtu, - nm_device_get_configured_mtu_wired_parent, - nm_device_get_configured_mtu_for_wired)) { - return nm_device_hash_check_invalid_keys(diffs, - NM_SETTING_WIRED_SETTING_NAME, - error, - NM_SETTING_WIRED_MTU); - } else { - g_set_error(error, - NM_DEVICE_ERROR, - NM_DEVICE_ERROR_INCOMPATIBLE_CONNECTION, - "Can't reapply any changes to '%s' setting", - setting_name); - return FALSE; } + + if (NM_IN_STRSET(setting_name, + NM_SETTING_USER_SETTING_NAME, + NM_SETTING_PROXY_SETTING_NAME, + NM_SETTING_IP4_CONFIG_SETTING_NAME, + NM_SETTING_IP6_CONFIG_SETTING_NAME)) + return TRUE; + + if (nm_streq(setting_name, NM_SETTING_WIRED_SETTING_NAME)) { + if (NM_IN_SET(NM_DEVICE_GET_CLASS(self)->get_configured_mtu, + nm_device_get_configured_mtu_wired_parent, + nm_device_get_configured_mtu_for_wired)) { + return nm_device_hash_check_invalid_keys(diffs, + NM_SETTING_WIRED_SETTING_NAME, + error, + NM_SETTING_WIRED_MTU); + } + goto out_fail; + } + +out_fail: + g_set_error(error, + NM_DEVICE_ERROR, + NM_DEVICE_ERROR_INCOMPATIBLE_CONNECTION, + "Can't reapply any changes to '%s' setting", + setting_name); + return FALSE; } static void From 46e0a3374b39f40213b20a665c2313b2d08a947f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 5 Nov 2020 10:03:21 +0100 Subject: [PATCH 15/35] core/trivial: add FIXME comment about immutable applied-connection --- src/devices/nm-device.c | 2 ++ src/devices/ovs/nm-ovsdb.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index b77d0fd36a..b905855cf5 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -12557,6 +12557,8 @@ check_and_reapply_connection(NMDevice * self, con_old = applied_clone = nm_simple_connection_new_clone(applied); con_new = applied; + /* FIXME(applied-connection-immutable): we should not modify the applied + * connection but replace it with a new (immutable) instance. */ nm_connection_replace_settings_from_connection(applied, connection_clean); nm_connection_clear_secrets(applied); } else diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c index f3ef54a1b0..56b7421182 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -216,6 +216,8 @@ ovsdb_call_method(NMOvsdb * self, case OVSDB_MONITOR: break; case OVSDB_ADD_INTERFACE: + /* FIXME(applied-connection-immutable): we should not modify the applied + * connection, consequently there is no need to clone the connections. */ call->bridge = nm_simple_connection_new_clone(bridge); call->port = nm_simple_connection_new_clone(port); call->interface = nm_simple_connection_new_clone(interface); From 609b08e2eb6a10ca1ca87725207eafa5ac4f4b83 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 6 Nov 2020 18:53:57 +0100 Subject: [PATCH 16/35] core/ovs: fix leak of "NMOvsdbPrivate.db_uuid Also, never update the value to %NULL. If the current message does not contain a UUID, keep the previous one. Fixes: 830a5a14cb29 ('device: add support for OpenVSwitch devices') --- src/devices/ovs/nm-ovsdb.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c index 56b7421182..5a18c32734 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -1175,8 +1175,12 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) } if (ovs) { - iter = json_object_iter(ovs); - priv->db_uuid = iter ? g_strdup(json_object_iter_key(iter)) : NULL; + const char *s; + + iter = json_object_iter(ovs); + s = json_object_iter_key(iter); + if (s) + nm_utils_strdup_reset(&priv->db_uuid, s); } /* Interfaces */ From e05edcfd7ed5c2f396fc2bace4144b140bd285dc Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 5 Nov 2020 15:10:52 +0100 Subject: [PATCH 17/35] core/ovs: cleanup handling of call id for OVS commands - rename "id" to something more distinct: "call_id". - consistently use guint64 type. We don't want nor need to handle negative values. For CALL_ID_UNSPEC we can use G_MAXUINT64. - don't use "i" format string for the call id. That expects an "int", so it's not clear how this was working correctly previously. Also, "int" has a smaller range than our 64bits. Use instead "json_int_t" and cast properly in the variadic arguments of json_pack(). --- src/devices/ovs/nm-ovsdb.c | 56 ++++++++++++++++++++++++++------------ 1 file changed, 38 insertions(+), 18 deletions(-) diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c index 5a18c32734..b52cf0b645 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -53,7 +53,7 @@ typedef struct { size_t bufp; /* Last decoded byte in the input buffer. */ GString * input; /* JSON stream waiting for decoding. */ GString * output; /* JSON stream to be sent. */ - gint64 seq; + guint64 call_id_counter; GArray * calls; /* Method calls waiting for a response. */ GHashTable * interfaces; /* interface uuid => OpenvswitchInterface */ GHashTable * ports; /* port uuid => OpenvswitchPort */ @@ -104,9 +104,10 @@ typedef enum { OVSDB_SET_INTERFACE_MTU, } OvsdbCommand; +#define CALL_ID_UNSPEC G_MAXUINT64 + typedef struct { - gint64 id; -#define COMMAND_PENDING -1 /* id not yet assigned */ + guint64 call_id; OvsdbCommand command; OvsdbMethodCallback callback; gpointer user_data; @@ -207,7 +208,7 @@ ovsdb_call_method(NMOvsdb * self, g_array_set_size(priv->calls, priv->calls->len + 1); call = &g_array_index(priv->calls, OvsdbMethodCall, priv->calls->len - 1); } - call->id = COMMAND_PENDING; + call->call_id = CALL_ID_UNSPEC; call->command = command; call->callback = callback; call->user_data = user_data; @@ -975,20 +976,21 @@ ovsdb_next_command(NMOvsdb *self) if (!priv->calls->len) return; call = &g_array_index(priv->calls, OvsdbMethodCall, 0); - if (call->id != COMMAND_PENDING) + if (call->call_id != CALL_ID_UNSPEC) return; - call->id = priv->seq++; + + call->call_id = ++priv->call_id_counter; switch (call->command) { case OVSDB_MONITOR: - msg = json_pack("{s:i, s:s, s:[s, n, {" + msg = json_pack("{s:I, s:s, s:[s, n, {" " s:[{s:[s, s, s]}]," " s:[{s:[s, s, s]}]," " s:[{s:[s, s, s, s]}]," " s:[{s:[]}]" "}]}", "id", - call->id, + (json_int_t) call->call_id, "method", "monitor", "params", @@ -1025,7 +1027,13 @@ ovsdb_next_command(NMOvsdb *self) call->bridge_device, call->interface_device); - msg = json_pack("{s:i, s:s, s:o}", "id", call->id, "method", "transact", "params", params); + msg = json_pack("{s:I, s:s, s:o}", + "id", + (json_int_t) call->call_id, + "method", + "transact", + "params", + params); break; case OVSDB_DEL_INTERFACE: params = json_array(); @@ -1034,7 +1042,13 @@ ovsdb_next_command(NMOvsdb *self) _delete_interface(self, params, call->ifname); - msg = json_pack("{s:i, s:s, s:o}", "id", call->id, "method", "transact", "params", params); + msg = json_pack("{s:I, s:s, s:o}", + "id", + (json_int_t) call->call_id, + "method", + "transact", + "params", + params); break; case OVSDB_SET_INTERFACE_MTU: params = json_array(); @@ -1055,7 +1069,13 @@ ovsdb_next_command(NMOvsdb *self) "==", call->ifname)); - msg = json_pack("{s:i, s:s, s:o}", "id", call->id, "method", "transact", "params", params); + msg = json_pack("{s:I, s:s, s:o}", + "id", + (json_int_t) call->call_id, + "method", + "transact", + "params", + params); break; } @@ -1437,7 +1457,7 @@ ovsdb_got_msg(NMOvsdb *self, json_t *msg) 0, }; json_t * json_id = NULL; - gint64 id = -1; + json_int_t id = (json_int_t) -1; const char * method = NULL; json_t * params = NULL; json_t * result = NULL; @@ -1490,18 +1510,18 @@ ovsdb_got_msg(NMOvsdb *self, json_t *msg) return; } - if (id > -1) { + if (id >= 0) { /* This is a response to a method call. */ if (!priv->calls->len) { - _LOGE("there are no queued calls expecting response %" G_GUINT64_FORMAT, id); + _LOGE("there are no queued calls expecting response %" G_GUINT64_FORMAT, (guint64) id); ovsdb_disconnect(self, FALSE, FALSE); return; } call = &g_array_index(priv->calls, OvsdbMethodCall, 0); - if (call->id != id) { + if (call->call_id != id) { _LOGE("expected a response to call %" G_GUINT64_FORMAT ", not %" G_GUINT64_FORMAT, - call->id, - id); + call->call_id, + (guint64) id); ovsdb_disconnect(self, FALSE, FALSE); return; } @@ -1705,7 +1725,7 @@ ovsdb_disconnect(NMOvsdb *self, gboolean retry, gboolean is_disposing) if (retry) { if (priv->calls->len != 0) - g_array_index(priv->calls, OvsdbMethodCall, 0).id = COMMAND_PENDING; + g_array_index(priv->calls, OvsdbMethodCall, 0).call_id = CALL_ID_UNSPEC; } else { nm_utils_error_set_cancelled(&error, is_disposing, "NMOvsdb"); From 4cad3cfe88e71ee61ca0d0ea780741cdb86e8416 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 5 Nov 2020 15:15:21 +0100 Subject: [PATCH 18/35] core/ovs: fix using unsigned "mtu" value to json_pack() Of course, in practice "mtu" is much smaller than 2^31, and also is sizeof(int) >= sizeof(uint32_t) (on our systems). Hence, this was correct. Still, it feels ugly to pass a unsigned integer where not the entire range is covered. --- src/devices/ovs/nm-ovsdb.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c index b52cf0b645..2044f3629f 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -1056,14 +1056,14 @@ ovsdb_next_command(NMOvsdb *self) json_array_append_new(params, _inc_next_cfg(priv->db_uuid)); json_array_append_new(params, - json_pack("{s:s, s:s, s:{s: i}, s:[[s, s, s]]}", + json_pack("{s:s, s:s, s:{s: I}, s:[[s, s, s]]}", "op", "update", "table", "Interface", "row", "mtu_request", - call->mtu, + (json_int_t) call->mtu, "where", "name", "==", From 7738955c2f49460dfa223bce6a39bfaecd92b9c0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 5 Nov 2020 20:24:09 +0100 Subject: [PATCH 19/35] core/ovs: cleanup uses of g_slice_*() in "nm-ovsdb.c" --- src/devices/ovs/nm-ovsdb.c | 47 ++++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c index 2044f3629f..6ef1288d60 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -1251,10 +1251,12 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) } if (new) { - ovs_interface = g_slice_new(OpenvswitchInterface); - ovs_interface->name = g_strdup(name); - ovs_interface->type = g_strdup(type); - ovs_interface->connection_uuid = _connection_uuid_from_external_ids(external_ids); + ovs_interface = g_slice_new(OpenvswitchInterface); + *ovs_interface = (OpenvswitchInterface){ + .name = g_strdup(name), + .type = g_strdup(type), + .connection_uuid = _connection_uuid_from_external_ids(external_ids), + }; g_hash_table_insert(priv->interfaces, g_strdup(key), ovs_interface); if (old) { _LOGT("changed an '%s' interface: %s%s%s", @@ -1329,10 +1331,12 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) } if (new) { - ovs_port = g_slice_new(OpenvswitchPort); - ovs_port->name = g_strdup(name); - ovs_port->connection_uuid = _connection_uuid_from_external_ids(external_ids); - ovs_port->interfaces = g_ptr_array_new_with_free_func(g_free); + ovs_port = g_slice_new(OpenvswitchPort); + *ovs_port = (OpenvswitchPort){ + .name = g_strdup(name), + .connection_uuid = _connection_uuid_from_external_ids(external_ids), + .interfaces = g_ptr_array_new_with_free_func(g_free), + }; _uuids_to_array(ovs_port->interfaces, items); g_hash_table_insert(priv->ports, g_strdup(key), ovs_port); if (old) { @@ -1392,10 +1396,12 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) } if (new) { - ovs_bridge = g_slice_new(OpenvswitchBridge); - ovs_bridge->name = g_strdup(name); - ovs_bridge->connection_uuid = _connection_uuid_from_external_ids(external_ids); - ovs_bridge->ports = g_ptr_array_new_with_free_func(g_free); + ovs_bridge = g_slice_new(OpenvswitchBridge); + *ovs_bridge = (OpenvswitchBridge){ + .name = g_strdup(name), + .connection_uuid = _connection_uuid_from_external_ids(external_ids), + .ports = g_ptr_array_new_with_free_func(g_free), + }; _uuids_to_array(ovs_bridge->ports, items); g_hash_table_insert(priv->bridges, g_strdup(key), ovs_bridge); if (old) { @@ -1873,7 +1879,7 @@ _transact_cb(NMOvsdb *self, json_t *result, GError *error, gpointer user_data) out: call->callback(error, call->user_data); - g_slice_free(OvsdbCall, call); + nm_g_slice_free(call); } static OvsdbCall * @@ -1881,10 +1887,11 @@ ovsdb_call_new(NMOvsdbCallback callback, gpointer user_data) { OvsdbCall *call; - call = g_slice_new(OvsdbCall); - call->callback = callback; - call->user_data = user_data; - + call = g_slice_new(OvsdbCall); + *call = (OvsdbCall){ + .callback = callback, + .user_data = user_data, + }; return call; } @@ -1985,7 +1992,7 @@ _free_bridge(gpointer data) g_free(ovs_bridge->name); g_free(ovs_bridge->connection_uuid); g_ptr_array_free(ovs_bridge->ports, TRUE); - g_slice_free(OpenvswitchBridge, ovs_bridge); + nm_g_slice_free(ovs_bridge); } static void @@ -1996,7 +2003,7 @@ _free_port(gpointer data) g_free(ovs_port->name); g_free(ovs_port->connection_uuid); g_ptr_array_free(ovs_port->interfaces, TRUE); - g_slice_free(OpenvswitchPort, ovs_port); + nm_g_slice_free(ovs_port); } static void @@ -2007,7 +2014,7 @@ _free_interface(gpointer data) g_free(ovs_interface->name); g_free(ovs_interface->connection_uuid); g_free(ovs_interface->type); - g_slice_free(OpenvswitchInterface, ovs_interface); + nm_g_slice_free(ovs_interface); } static void From 5d5b35285ec730f5f48d3fde7dc8720d628b345c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 5 Nov 2020 20:41:53 +0100 Subject: [PATCH 20/35] core/ovs: use streq() instead of strcmp() --- src/devices/ovs/nm-ovsdb.c | 40 ++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c index 6ef1288d60..9b50b1aa74 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -767,8 +767,8 @@ _add_interface(NMOvsdb * self, while (g_hash_table_iter_next(&iter, (gpointer) &bridge_uuid, (gpointer) &ovs_bridge)) { json_array_append_new(bridges, json_pack("[s, s]", "uuid", bridge_uuid)); - if (g_strcmp0(ovs_bridge->name, bridge_name) != 0 - || g_strcmp0(ovs_bridge->connection_uuid, nm_connection_get_uuid(bridge)) != 0) + if (!nm_streq0(ovs_bridge->name, bridge_name) + || !nm_streq0(ovs_bridge->connection_uuid, nm_connection_get_uuid(bridge))) continue; for (pi = 0; pi < ovs_bridge->ports->len; pi++) { @@ -781,8 +781,8 @@ _add_interface(NMOvsdb * self, /* This would be a violation of ovsdb's reference integrity (a bug). */ _LOGW("Unknown port '%s' in bridge '%s'", port_uuid, bridge_uuid); continue; - } else if (strcmp(ovs_port->name, port_name) != 0 - || g_strcmp0(ovs_port->connection_uuid, nm_connection_get_uuid(port)) != 0) { + } else if (!nm_streq(ovs_port->name, port_name) + || !nm_streq0(ovs_port->connection_uuid, nm_connection_get_uuid(port))) { continue; } @@ -795,12 +795,10 @@ _add_interface(NMOvsdb * self, if (!ovs_interface) { /* This would be a violation of ovsdb's reference integrity (a bug). */ _LOGW("Unknown interface '%s' in port '%s'", interface_uuid, port_uuid); - } else if (strcmp(ovs_interface->name, interface_name) == 0 - && g_strcmp0(ovs_interface->connection_uuid, - nm_connection_get_uuid(interface)) - == 0) { + } else if (nm_streq(ovs_interface->name, interface_name) + && nm_streq0(ovs_interface->connection_uuid, + nm_connection_get_uuid(interface))) has_interface = TRUE; - } } break; @@ -911,7 +909,7 @@ _delete_interface(NMOvsdb *self, json_t *params, const char *ifname) json_array_append_new(interfaces, json_pack("[s,s]", "uuid", interface_uuid)); if (ovs_interface) { - if (strcmp(ovs_interface->name, ifname) == 0) { + if (nm_streq(ovs_interface->name, ifname)) { /* skip the interface */ interfaces_changed = TRUE; continue; @@ -1120,9 +1118,9 @@ _uuids_to_array(GPtrArray *array, const json_t *items) if (!value) return; - if (g_strcmp0(key, "uuid") == 0 && json_is_string(value)) { + if (nm_streq0(key, "uuid") && json_is_string(value)) { g_ptr_array_add(array, g_strdup(json_string_value(value))); - } else if (g_strcmp0(key, "set") == 0 && json_is_array(value)) { + } else if (nm_streq0(key, "set") && json_is_array(value)) { json_array_foreach (value, set_index, set_value) { _uuids_to_array(array, set_value); } @@ -1136,11 +1134,11 @@ _connection_uuid_from_external_ids(json_t *external_ids) json_t *value; size_t index; - if (g_strcmp0("map", json_string_value(json_array_get(external_ids, 0))) != 0) + if (!nm_streq0("map", json_string_value(json_array_get(external_ids, 0)))) return NULL; json_array_foreach (json_array_get(external_ids, 1), index, value) { - if (g_strcmp0("NM.connection.uuid", json_string_value(json_array_get(value, 0))) == 0) + if (nm_streq0("NM.connection.uuid", json_string_value(json_array_get(value, 0)))) return g_strdup(json_string_value(json_array_get(value, 1))); } @@ -1230,14 +1228,14 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) ovs_interface = g_hash_table_lookup(priv->interfaces, key); if (!ovs_interface) { _LOGW("Interface '%s' was not seen", key); - } else if (!new || strcmp(ovs_interface->name, name) != 0) { + } else if (!new || !nm_streq(ovs_interface->name, name)) { old = FALSE; _LOGT("removed an '%s' interface: %s%s%s", ovs_interface->type, ovs_interface->name, ovs_interface->connection_uuid ? ", " : "", ovs_interface->connection_uuid ?: ""); - if (g_strcmp0(ovs_interface->type, "internal") == 0) { + if (nm_streq0(ovs_interface->type, "internal")) { /* Currently, the factory only creates NMDevices for * internal interfaces. Ignore the rest. */ g_signal_emit(self, @@ -1270,7 +1268,7 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) ovs_interface->name, ovs_interface->connection_uuid ? ", " : "", ovs_interface->connection_uuid ?: ""); - if (g_strcmp0(ovs_interface->type, "internal") == 0) { + if (nm_streq0(ovs_interface->type, "internal")) { /* Currently, the factory only creates NMDevices for * internal interfaces. Ignore the rest. */ g_signal_emit(self, @@ -1315,7 +1313,7 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) if (old) { ovs_port = g_hash_table_lookup(priv->ports, key); - if (!new || g_strcmp0(ovs_port->name, name) != 0) { + if (!new || !nm_streq0(ovs_port->name, name)) { old = FALSE; _LOGT("removed a port: %s%s%s", ovs_port->name, @@ -1380,7 +1378,7 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) if (old) { ovs_bridge = g_hash_table_lookup(priv->bridges, key); - if (!new || g_strcmp0(ovs_bridge->name, name) != 0) { + if (!new || !nm_streq0(ovs_bridge->name, name)) { old = FALSE; _LOGT("removed a bridge: %s%s%s", ovs_bridge->name, @@ -1504,10 +1502,10 @@ ovsdb_got_msg(NMOvsdb *self, json_t *msg) return; } - if (g_strcmp0(method, "update") == 0) { + if (nm_streq0(method, "update")) { /* This is a update method call. */ ovsdb_got_update(self, json_array_get(params, 1)); - } else if (g_strcmp0(method, "echo") == 0) { + } else if (nm_streq0(method, "echo")) { /* This is an echo request. */ ovsdb_got_echo(self, id, params); } else { From 8d78f8effb67a6c5dca93dbbaa45efd4a0196ecb Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 5 Nov 2020 21:23:11 +0100 Subject: [PATCH 21/35] core/ovs: avoid possible crash in _add_interface() --- src/devices/ovs/nm-ovsdb.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c index 9b50b1aa74..b1d09ce5b8 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -1313,7 +1313,7 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) if (old) { ovs_port = g_hash_table_lookup(priv->ports, key); - if (!new || !nm_streq0(ovs_port->name, name)) { + if (!new || (ovs_port && !nm_streq0(ovs_port->name, name))) { old = FALSE; _LOGT("removed a port: %s%s%s", ovs_port->name, @@ -1378,7 +1378,7 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) if (old) { ovs_bridge = g_hash_table_lookup(priv->bridges, key); - if (!new || !nm_streq0(ovs_bridge->name, name)) { + if (!new || (ovs_bridge && !nm_streq0(ovs_bridge->name, name))) { old = FALSE; _LOGT("removed a bridge: %s%s%s", ovs_bridge->name, From 263e92bf491aaaaf8e917f525e544f01a478aa3b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 5 Nov 2020 21:23:46 +0100 Subject: [PATCH 22/35] core/ovs: minor cleanup of logic in _add_interface() --- src/devices/ovs/nm-ovsdb.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c index b1d09ce5b8..8c59b40262 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -781,11 +781,12 @@ _add_interface(NMOvsdb * self, /* This would be a violation of ovsdb's reference integrity (a bug). */ _LOGW("Unknown port '%s' in bridge '%s'", port_uuid, bridge_uuid); continue; - } else if (!nm_streq(ovs_port->name, port_name) - || !nm_streq0(ovs_port->connection_uuid, nm_connection_get_uuid(port))) { - continue; } + if (!nm_streq(ovs_port->name, port_name) + || !nm_streq0(ovs_port->connection_uuid, nm_connection_get_uuid(port))) + continue; + for (ii = 0; ii < ovs_port->interfaces->len; ii++) { interface_uuid = g_ptr_array_index(ovs_port->interfaces, ii); ovs_interface = g_hash_table_lookup(priv->interfaces, interface_uuid); @@ -795,9 +796,10 @@ _add_interface(NMOvsdb * self, if (!ovs_interface) { /* This would be a violation of ovsdb's reference integrity (a bug). */ _LOGW("Unknown interface '%s' in port '%s'", interface_uuid, port_uuid); - } else if (nm_streq(ovs_interface->name, interface_name) - && nm_streq0(ovs_interface->connection_uuid, - nm_connection_get_uuid(interface))) + continue; + } + if (nm_streq(ovs_interface->name, interface_name) + && nm_streq0(ovs_interface->connection_uuid, nm_connection_get_uuid(interface))) has_interface = TRUE; } From 2094cbb5d11b2d0d89d2851e23fc2597f5186398 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 5 Nov 2020 21:16:58 +0100 Subject: [PATCH 23/35] core/ovs: track key for OpenvswitchBridge in same struct GHashTable is optimized for data that has no separate value pointer. We can use the OpenvswitchBridge structs as key themselves, by having the id as first field of the structure and only use g_hash_table_add(). --- src/devices/ovs/nm-ovsdb.c | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c index 8c59b40262..993e6fd0b6 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -28,6 +28,7 @@ typedef struct { } OpenvswitchPort; typedef struct { + char * bridge_uuid; char * name; char * connection_uuid; GPtrArray *ports; /* port uuids */ @@ -694,7 +695,6 @@ _add_interface(NMOvsdb * self, { NMOvsdbPrivate * priv = NM_OVSDB_GET_PRIVATE(self); GHashTableIter iter; - const char * bridge_uuid; const char * port_uuid; const char * interface_uuid; const char * bridge_name; @@ -764,8 +764,8 @@ _add_interface(NMOvsdb * self, } g_hash_table_iter_init(&iter, priv->bridges); - while (g_hash_table_iter_next(&iter, (gpointer) &bridge_uuid, (gpointer) &ovs_bridge)) { - json_array_append_new(bridges, json_pack("[s, s]", "uuid", bridge_uuid)); + while (g_hash_table_iter_next(&iter, (gpointer) &ovs_bridge, NULL)) { + json_array_append_new(bridges, json_pack("[s, s]", "uuid", ovs_bridge->bridge_uuid)); if (!nm_streq0(ovs_bridge->name, bridge_name) || !nm_streq0(ovs_bridge->connection_uuid, nm_connection_get_uuid(bridge))) @@ -779,7 +779,7 @@ _add_interface(NMOvsdb * self, if (!ovs_port) { /* This would be a violation of ovsdb's reference integrity (a bug). */ - _LOGW("Unknown port '%s' in bridge '%s'", port_uuid, bridge_uuid); + _LOGW("Unknown port '%s' in bridge '%s'", port_uuid, ovs_bridge->bridge_uuid); continue; } @@ -856,7 +856,6 @@ _delete_interface(NMOvsdb *self, json_t *params, const char *ifname) { NMOvsdbPrivate * priv = NM_OVSDB_GET_PRIVATE(self); GHashTableIter iter; - char * bridge_uuid; char * port_uuid; char * interface_uuid; OpenvswitchBridge * ovs_bridge; @@ -875,7 +874,7 @@ _delete_interface(NMOvsdb *self, json_t *params, const char *ifname) bridges_changed = FALSE; g_hash_table_iter_init(&iter, priv->bridges); - while (g_hash_table_iter_next(&iter, (gpointer) &bridge_uuid, (gpointer) &ovs_bridge)) { + while (g_hash_table_iter_next(&iter, (gpointer) &ovs_bridge, NULL)) { nm_auto_decref_json json_t *ports = NULL; nm_auto_decref_json json_t *new_ports = NULL; @@ -883,7 +882,7 @@ _delete_interface(NMOvsdb *self, json_t *params, const char *ifname) new_ports = json_array(); ports_changed = FALSE; - json_array_append_new(bridges, json_pack("[s,s]", "uuid", bridge_uuid)); + json_array_append_new(bridges, json_pack("[s,s]", "uuid", ovs_bridge->bridge_uuid)); for (pi = 0; pi < ovs_bridge->ports->len; pi++) { nm_auto_decref_json json_t *interfaces = NULL; @@ -900,7 +899,7 @@ _delete_interface(NMOvsdb *self, json_t *params, const char *ifname) if (!ovs_port) { /* This would be a violation of ovsdb's reference integrity (a bug). */ - _LOGW("Unknown port '%s' in bridge '%s'", port_uuid, bridge_uuid); + _LOGW("Unknown port '%s' in bridge '%s'", port_uuid, ovs_bridge->bridge_uuid); continue; } @@ -942,7 +941,7 @@ _delete_interface(NMOvsdb *self, json_t *params, const char *ifname) _expect_bridge_ports(params, ovs_bridge->name, ports); _set_bridge_ports(params, ovs_bridge->name, new_ports); } - json_array_append_new(new_bridges, json_pack("[s,s]", "uuid", bridge_uuid)); + json_array_append_new(new_bridges, json_pack("[s,s]", "uuid", ovs_bridge->bridge_uuid)); } } @@ -1379,7 +1378,7 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) new = TRUE; if (old) { - ovs_bridge = g_hash_table_lookup(priv->bridges, key); + ovs_bridge = g_hash_table_lookup(priv->bridges, &key); if (!new || (ovs_bridge && !nm_streq0(ovs_bridge->name, name))) { old = FALSE; _LOGT("removed a bridge: %s%s%s", @@ -1392,18 +1391,19 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) ovs_bridge->name, NM_DEVICE_TYPE_OVS_BRIDGE); } - g_hash_table_remove(priv->bridges, key); + g_hash_table_remove(priv->bridges, &key); } if (new) { ovs_bridge = g_slice_new(OpenvswitchBridge); *ovs_bridge = (OpenvswitchBridge){ + .bridge_uuid = g_strdup(key), .name = g_strdup(name), .connection_uuid = _connection_uuid_from_external_ids(external_ids), .ports = g_ptr_array_new_with_free_func(g_free), }; _uuids_to_array(ovs_bridge->ports, items); - g_hash_table_insert(priv->bridges, g_strdup(key), ovs_bridge); + g_hash_table_add(priv->bridges, ovs_bridge); if (old) { _LOGT("changed a bridge: %s%s%s", ovs_bridge->name, @@ -1989,6 +1989,7 @@ _free_bridge(gpointer data) { OpenvswitchBridge *ovs_bridge = data; + g_free(ovs_bridge->bridge_uuid); g_free(ovs_bridge->name); g_free(ovs_bridge->connection_uuid); g_ptr_array_free(ovs_bridge->ports, TRUE); @@ -2026,7 +2027,7 @@ nm_ovsdb_init(NMOvsdb *self) g_array_set_clear_func(priv->calls, _clear_call); priv->input = g_string_new(NULL); priv->output = g_string_new(NULL); - priv->bridges = g_hash_table_new_full(nm_str_hash, g_str_equal, g_free, _free_bridge); + priv->bridges = g_hash_table_new_full(nm_pstr_hash, nm_pstr_equal, _free_bridge, NULL); priv->ports = g_hash_table_new_full(nm_str_hash, g_str_equal, g_free, _free_port); priv->interfaces = g_hash_table_new_full(nm_str_hash, g_str_equal, g_free, _free_interface); From 51495e4e9ad4a1d463388e44e07fa79595b2d95d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 5 Nov 2020 21:16:58 +0100 Subject: [PATCH 24/35] core/ovs: track key for OpenvswitchPort in same struct --- src/devices/ovs/nm-ovsdb.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c index 993e6fd0b6..d25e40b0ba 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -22,6 +22,7 @@ #endif typedef struct { + char * port_uuid; char * name; char * connection_uuid; GPtrArray *interfaces; /* interface uuids */ @@ -773,7 +774,7 @@ _add_interface(NMOvsdb * self, for (pi = 0; pi < ovs_bridge->ports->len; pi++) { port_uuid = g_ptr_array_index(ovs_bridge->ports, pi); - ovs_port = g_hash_table_lookup(priv->ports, port_uuid); + ovs_port = g_hash_table_lookup(priv->ports, &port_uuid); json_array_append_new(ports, json_pack("[s, s]", "uuid", port_uuid)); @@ -891,7 +892,7 @@ _delete_interface(NMOvsdb *self, json_t *params, const char *ifname) interfaces = json_array(); new_interfaces = json_array(); port_uuid = g_ptr_array_index(ovs_bridge->ports, pi); - ovs_port = g_hash_table_lookup(priv->ports, port_uuid); + ovs_port = g_hash_table_lookup(priv->ports, &port_uuid); json_array_append_new(ports, json_pack("[s,s]", "uuid", port_uuid)); @@ -1313,7 +1314,7 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) new = TRUE; if (old) { - ovs_port = g_hash_table_lookup(priv->ports, key); + ovs_port = g_hash_table_lookup(priv->ports, &key); if (!new || (ovs_port && !nm_streq0(ovs_port->name, name))) { old = FALSE; _LOGT("removed a port: %s%s%s", @@ -1326,18 +1327,19 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) ovs_port->name, NM_DEVICE_TYPE_OVS_PORT); } - g_hash_table_remove(priv->ports, key); + g_hash_table_remove(priv->ports, &key); } if (new) { ovs_port = g_slice_new(OpenvswitchPort); *ovs_port = (OpenvswitchPort){ + .port_uuid = g_strdup(key), .name = g_strdup(name), .connection_uuid = _connection_uuid_from_external_ids(external_ids), .interfaces = g_ptr_array_new_with_free_func(g_free), }; _uuids_to_array(ovs_port->interfaces, items); - g_hash_table_insert(priv->ports, g_strdup(key), ovs_port); + g_hash_table_add(priv->ports, ovs_port); if (old) { _LOGT("changed a port: %s%s%s", ovs_port->name, @@ -2001,6 +2003,7 @@ _free_port(gpointer data) { OpenvswitchPort *ovs_port = data; + g_free(ovs_port->port_uuid); g_free(ovs_port->name); g_free(ovs_port->connection_uuid); g_ptr_array_free(ovs_port->interfaces, TRUE); @@ -2028,7 +2031,7 @@ nm_ovsdb_init(NMOvsdb *self) priv->input = g_string_new(NULL); priv->output = g_string_new(NULL); priv->bridges = g_hash_table_new_full(nm_pstr_hash, nm_pstr_equal, _free_bridge, NULL); - priv->ports = g_hash_table_new_full(nm_str_hash, g_str_equal, g_free, _free_port); + priv->ports = g_hash_table_new_full(nm_pstr_hash, nm_pstr_equal, _free_port, NULL); priv->interfaces = g_hash_table_new_full(nm_str_hash, g_str_equal, g_free, _free_interface); ovsdb_try_connect(self); From e403f7654461b116c2318746b135e21a6b40d1a3 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 5 Nov 2020 21:16:58 +0100 Subject: [PATCH 25/35] core/ovs: track key for OpenvswitchInterface in same struct --- src/devices/ovs/nm-ovsdb.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c index d25e40b0ba..181633fae8 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -36,6 +36,7 @@ typedef struct { } OpenvswitchBridge; typedef struct { + char *interface_uuid; char *name; char *type; char *connection_uuid; @@ -790,7 +791,7 @@ _add_interface(NMOvsdb * self, for (ii = 0; ii < ovs_port->interfaces->len; ii++) { interface_uuid = g_ptr_array_index(ovs_port->interfaces, ii); - ovs_interface = g_hash_table_lookup(priv->interfaces, interface_uuid); + ovs_interface = g_hash_table_lookup(priv->interfaces, &interface_uuid); json_array_append_new(interfaces, json_pack("[s, s]", "uuid", interface_uuid)); @@ -906,7 +907,7 @@ _delete_interface(NMOvsdb *self, json_t *params, const char *ifname) for (ii = 0; ii < ovs_port->interfaces->len; ii++) { interface_uuid = g_ptr_array_index(ovs_port->interfaces, ii); - ovs_interface = g_hash_table_lookup(priv->interfaces, interface_uuid); + ovs_interface = g_hash_table_lookup(priv->interfaces, &interface_uuid); json_array_append_new(interfaces, json_pack("[s,s]", "uuid", interface_uuid)); @@ -1227,7 +1228,7 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) new = TRUE; if (old) { - ovs_interface = g_hash_table_lookup(priv->interfaces, key); + ovs_interface = g_hash_table_lookup(priv->interfaces, &key); if (!ovs_interface) { _LOGW("Interface '%s' was not seen", key); } else if (!new || !nm_streq(ovs_interface->name, name)) { @@ -1247,17 +1248,18 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) NM_DEVICE_TYPE_OVS_INTERFACE); } } - g_hash_table_remove(priv->interfaces, key); + g_hash_table_remove(priv->interfaces, &key); } if (new) { ovs_interface = g_slice_new(OpenvswitchInterface); *ovs_interface = (OpenvswitchInterface){ + .interface_uuid = g_strdup(key), .name = g_strdup(name), .type = g_strdup(type), .connection_uuid = _connection_uuid_from_external_ids(external_ids), }; - g_hash_table_insert(priv->interfaces, g_strdup(key), ovs_interface); + g_hash_table_add(priv->interfaces, ovs_interface); if (old) { _LOGT("changed an '%s' interface: %s%s%s", type, @@ -2015,6 +2017,7 @@ _free_interface(gpointer data) { OpenvswitchInterface *ovs_interface = data; + g_free(ovs_interface->interface_uuid); g_free(ovs_interface->name); g_free(ovs_interface->connection_uuid); g_free(ovs_interface->type); @@ -2032,7 +2035,7 @@ nm_ovsdb_init(NMOvsdb *self) priv->output = g_string_new(NULL); priv->bridges = g_hash_table_new_full(nm_pstr_hash, nm_pstr_equal, _free_bridge, NULL); priv->ports = g_hash_table_new_full(nm_pstr_hash, nm_pstr_equal, _free_port, NULL); - priv->interfaces = g_hash_table_new_full(nm_str_hash, g_str_equal, g_free, _free_interface); + priv->interfaces = g_hash_table_new_full(nm_pstr_hash, nm_pstr_equal, _free_interface, NULL); ovsdb_try_connect(self); } From cb3b6a2417e19614526f9f4c18c8143dea9ccaa5 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 6 Nov 2020 09:40:04 +0100 Subject: [PATCH 26/35] core/ovs: move code in "nm-ovsdb.c" around to have simple helpers at the top --- src/devices/ovs/nm-ovsdb.c | 126 ++++++++++++++++++++----------------- 1 file changed, 67 insertions(+), 59 deletions(-) diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c index 181633fae8..9fab5a2601 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -17,6 +17,10 @@ /*****************************************************************************/ +#define OVSDB_MAX_FAILURES 3 + +/*****************************************************************************/ + #if JANSSON_VERSION_HEX < 0x020400 #warning "requires at least libjansson 2.4" #endif @@ -129,7 +133,7 @@ typedef struct { }; } OvsdbMethodCall; -#define OVSDB_MAX_FAILURES 3 +/*****************************************************************************/ static void _LOGT_call_do(const char *comment, OvsdbMethodCall *call, json_t *msg) @@ -178,6 +182,68 @@ _LOGT_call_do(const char *comment, OvsdbMethodCall *call, json_t *msg) } \ G_STMT_END +/*****************************************************************************/ + +static void +_clear_call(gpointer data) +{ + OvsdbMethodCall *call = data; + + switch (call->command) { + case OVSDB_MONITOR: + break; + case OVSDB_ADD_INTERFACE: + g_clear_object(&call->bridge); + g_clear_object(&call->port); + g_clear_object(&call->interface); + g_clear_object(&call->bridge_device); + g_clear_object(&call->interface_device); + break; + case OVSDB_DEL_INTERFACE: + case OVSDB_SET_INTERFACE_MTU: + nm_clear_g_free(&call->ifname); + break; + } +} + +static void +_free_bridge(gpointer data) +{ + OpenvswitchBridge *ovs_bridge = data; + + g_free(ovs_bridge->bridge_uuid); + g_free(ovs_bridge->name); + g_free(ovs_bridge->connection_uuid); + g_ptr_array_free(ovs_bridge->ports, TRUE); + nm_g_slice_free(ovs_bridge); +} + +static void +_free_port(gpointer data) +{ + OpenvswitchPort *ovs_port = data; + + g_free(ovs_port->port_uuid); + g_free(ovs_port->name); + g_free(ovs_port->connection_uuid); + g_ptr_array_free(ovs_port->interfaces, TRUE); + nm_g_slice_free(ovs_port); +} + +static void +_free_interface(gpointer data) +{ + OpenvswitchInterface *ovs_interface = data; + + g_free(ovs_interface->interface_uuid); + g_free(ovs_interface->name); + g_free(ovs_interface->connection_uuid); + g_free(ovs_interface->type); + nm_g_slice_free(ovs_interface); +} + +/*****************************************************************************/ + /** * ovsdb_call_method: * @@ -1966,64 +2032,6 @@ nm_ovsdb_set_interface_mtu(NMOvsdb * self, /*****************************************************************************/ -static void -_clear_call(gpointer data) -{ - OvsdbMethodCall *call = data; - - switch (call->command) { - case OVSDB_MONITOR: - break; - case OVSDB_ADD_INTERFACE: - g_clear_object(&call->bridge); - g_clear_object(&call->port); - g_clear_object(&call->interface); - g_clear_object(&call->bridge_device); - g_clear_object(&call->interface_device); - break; - case OVSDB_DEL_INTERFACE: - case OVSDB_SET_INTERFACE_MTU: - nm_clear_g_free(&call->ifname); - break; - } -} - -static void -_free_bridge(gpointer data) -{ - OpenvswitchBridge *ovs_bridge = data; - - g_free(ovs_bridge->bridge_uuid); - g_free(ovs_bridge->name); - g_free(ovs_bridge->connection_uuid); - g_ptr_array_free(ovs_bridge->ports, TRUE); - nm_g_slice_free(ovs_bridge); -} - -static void -_free_port(gpointer data) -{ - OpenvswitchPort *ovs_port = data; - - g_free(ovs_port->port_uuid); - g_free(ovs_port->name); - g_free(ovs_port->connection_uuid); - g_ptr_array_free(ovs_port->interfaces, TRUE); - nm_g_slice_free(ovs_port); -} - -static void -_free_interface(gpointer data) -{ - OpenvswitchInterface *ovs_interface = data; - - g_free(ovs_interface->interface_uuid); - g_free(ovs_interface->name); - g_free(ovs_interface->connection_uuid); - g_free(ovs_interface->type); - nm_g_slice_free(ovs_interface); -} - static void nm_ovsdb_init(NMOvsdb *self) { From 7dc4d0c666c37f0d018f3373b3a291eabe7b65c7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 6 Nov 2020 09:52:37 +0100 Subject: [PATCH 27/35] core/ovs: use helper functions to emit NM_OVSDB_* signals --- src/devices/ovs/nm-ovs-factory.c | 18 +++----- src/devices/ovs/nm-ovsdb.c | 73 ++++++++++++++++---------------- 2 files changed, 44 insertions(+), 47 deletions(-) diff --git a/src/devices/ovs/nm-ovs-factory.c b/src/devices/ovs/nm-ovs-factory.c index e1feacbd02..e25f94bf89 100644 --- a/src/devices/ovs/nm-ovs-factory.c +++ b/src/devices/ovs/nm-ovs-factory.c @@ -106,12 +106,10 @@ new_device_from_type(const char *name, NMDeviceType device_type) } static void -ovsdb_device_added(NMOvsdb * ovsdb, - const char * name, - NMDeviceType device_type, - NMDeviceFactory *self) +ovsdb_device_added(NMOvsdb *ovsdb, const char *name, guint device_type_i, NMDeviceFactory *self) { - NMDevice *device = NULL; + const NMDeviceType device_type = device_type_i; + NMDevice * device; device = new_device_from_type(name, device_type); if (!device) @@ -122,13 +120,11 @@ ovsdb_device_added(NMOvsdb * ovsdb, } static void -ovsdb_device_removed(NMOvsdb * ovsdb, - const char * name, - NMDeviceType device_type, - NMDeviceFactory *self) +ovsdb_device_removed(NMOvsdb *ovsdb, const char *name, guint device_type_i, NMDeviceFactory *self) { - NMDevice * device; - NMDeviceState device_state; + const NMDeviceType device_type = device_type_i; + NMDevice * device; + NMDeviceState device_state; device = nm_manager_get_device(NM_MANAGER_GET, name, device_type); if (!device) diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c index 9fab5a2601..4d15e7c8ef 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -244,6 +244,29 @@ _free_interface(gpointer data) /*****************************************************************************/ +static void +_signal_emit_device_added(NMOvsdb *self, const char *name, NMDeviceType device_type) +{ + g_signal_emit(self, signals[DEVICE_ADDED], 0, name, (guint) device_type); +} + +static void +_signal_emit_device_removed(NMOvsdb *self, const char *name, NMDeviceType device_type) +{ + g_signal_emit(self, signals[DEVICE_REMOVED], 0, name, (guint) device_type); +} + +static void +_signal_emit_interface_failed(NMOvsdb * self, + const char *name, + const char *connection_uuid, + const char *error) +{ + g_signal_emit(self, signals[INTERFACE_FAILED], 0, name, connection_uuid, error); +} + +/*****************************************************************************/ + /** * ovsdb_call_method: * @@ -1307,11 +1330,9 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) if (nm_streq0(ovs_interface->type, "internal")) { /* Currently, the factory only creates NMDevices for * internal interfaces. Ignore the rest. */ - g_signal_emit(self, - signals[DEVICE_REMOVED], - 0, - ovs_interface->name, - NM_DEVICE_TYPE_OVS_INTERFACE); + _signal_emit_device_removed(self, + ovs_interface->name, + NM_DEVICE_TYPE_OVS_INTERFACE); } } g_hash_table_remove(priv->interfaces, &key); @@ -1341,22 +1362,18 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) if (nm_streq0(ovs_interface->type, "internal")) { /* Currently, the factory only creates NMDevices for * internal interfaces. Ignore the rest. */ - g_signal_emit(self, - signals[DEVICE_ADDED], - 0, - ovs_interface->name, - NM_DEVICE_TYPE_OVS_INTERFACE); + _signal_emit_device_added(self, + ovs_interface->name, + NM_DEVICE_TYPE_OVS_INTERFACE); } } /* The error is a string. No error is indicated by an empty set, * because why the fuck not: [ "set": [] ] */ if (error && json_is_string(error)) { - g_signal_emit(self, - signals[INTERFACE_FAILED], - 0, - ovs_interface->name, - ovs_interface->connection_uuid, - json_string_value(error)); + _signal_emit_interface_failed(self, + ovs_interface->name, + ovs_interface->connection_uuid, + json_string_value(error)); } } } @@ -1389,11 +1406,7 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) ovs_port->name, ovs_port->connection_uuid ? ", " : "", ovs_port->connection_uuid ?: ""); - g_signal_emit(self, - signals[DEVICE_REMOVED], - 0, - ovs_port->name, - NM_DEVICE_TYPE_OVS_PORT); + _signal_emit_device_removed(self, ovs_port->name, NM_DEVICE_TYPE_OVS_PORT); } g_hash_table_remove(priv->ports, &key); } @@ -1418,11 +1431,7 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) ovs_port->name, ovs_port->connection_uuid ? ", " : "", ovs_port->connection_uuid ?: ""); - g_signal_emit(self, - signals[DEVICE_ADDED], - 0, - ovs_port->name, - NM_DEVICE_TYPE_OVS_PORT); + _signal_emit_device_added(self, ovs_port->name, NM_DEVICE_TYPE_OVS_PORT); } } } @@ -1455,11 +1464,7 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) ovs_bridge->name, ovs_bridge->connection_uuid ? ", " : "", ovs_bridge->connection_uuid ?: ""); - g_signal_emit(self, - signals[DEVICE_REMOVED], - 0, - ovs_bridge->name, - NM_DEVICE_TYPE_OVS_BRIDGE); + _signal_emit_device_removed(self, ovs_bridge->name, NM_DEVICE_TYPE_OVS_BRIDGE); } g_hash_table_remove(priv->bridges, &key); } @@ -1484,11 +1489,7 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) ovs_bridge->name, ovs_bridge->connection_uuid ? ", " : "", ovs_bridge->connection_uuid ?: ""); - g_signal_emit(self, - signals[DEVICE_ADDED], - 0, - ovs_bridge->name, - NM_DEVICE_TYPE_OVS_BRIDGE); + _signal_emit_device_added(self, ovs_bridge->name, NM_DEVICE_TYPE_OVS_BRIDGE); } } } From f6d3b5f5f41ec2480db073c3163fd7c3cabf5d1f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 6 Nov 2020 09:43:54 +0100 Subject: [PATCH 28/35] core/ovs: change function signature of _free_{bridge,port,interface} We will call the function directly as well. Lets aim to get the types right. Also the compiler would warn if the cast to (GDestroyNotify) would be to a fundamtally different function signature. --- src/devices/ovs/nm-ovsdb.c | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c index 4d15e7c8ef..62146fef46 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -207,10 +207,8 @@ _clear_call(gpointer data) } static void -_free_bridge(gpointer data) +_free_bridge(OpenvswitchBridge *ovs_bridge) { - OpenvswitchBridge *ovs_bridge = data; - g_free(ovs_bridge->bridge_uuid); g_free(ovs_bridge->name); g_free(ovs_bridge->connection_uuid); @@ -219,10 +217,8 @@ _free_bridge(gpointer data) } static void -_free_port(gpointer data) +_free_port(OpenvswitchPort *ovs_port) { - OpenvswitchPort *ovs_port = data; - g_free(ovs_port->port_uuid); g_free(ovs_port->name); g_free(ovs_port->connection_uuid); @@ -231,10 +227,8 @@ _free_port(gpointer data) } static void -_free_interface(gpointer data) +_free_interface(OpenvswitchInterface *ovs_interface) { - OpenvswitchInterface *ovs_interface = data; - g_free(ovs_interface->interface_uuid); g_free(ovs_interface->name); g_free(ovs_interface->connection_uuid); @@ -2040,11 +2034,14 @@ nm_ovsdb_init(NMOvsdb *self) priv->calls = g_array_new(FALSE, TRUE, sizeof(OvsdbMethodCall)); g_array_set_clear_func(priv->calls, _clear_call); - priv->input = g_string_new(NULL); - priv->output = g_string_new(NULL); - priv->bridges = g_hash_table_new_full(nm_pstr_hash, nm_pstr_equal, _free_bridge, NULL); - priv->ports = g_hash_table_new_full(nm_pstr_hash, nm_pstr_equal, _free_port, NULL); - priv->interfaces = g_hash_table_new_full(nm_pstr_hash, nm_pstr_equal, _free_interface, NULL); + priv->input = g_string_new(NULL); + priv->output = g_string_new(NULL); + priv->bridges = + g_hash_table_new_full(nm_pstr_hash, nm_pstr_equal, (GDestroyNotify) _free_bridge, NULL); + priv->ports = + g_hash_table_new_full(nm_pstr_hash, nm_pstr_equal, (GDestroyNotify) _free_port, NULL); + priv->interfaces = + g_hash_table_new_full(nm_pstr_hash, nm_pstr_equal, (GDestroyNotify) _free_interface, NULL); ovsdb_try_connect(self); } From 7cf1f7fe02d8f1a1b00f7c1263bf40f417495cea Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 6 Nov 2020 14:19:23 +0100 Subject: [PATCH 29/35] core/ovs: cleanup logic in update handling of ovsdb_got_update() ovsdb sends monitor updates, with "new" and "old" values that indicate whether this is an addition, and update, or a removal. Since we also cache the entries, we might not agree with what ovsdb says. E.g. if ovsdb says this is an update, but we didn't have the interface in our cache, we should rather pretend that the interface was added. Even if this possibly indicates some inconsistency between what OVS says and what we have cached, we should make the best of it. Rework the code. On update, we compare the result with our cache and care less about the "new" / "old" values. --- src/devices/ovs/nm-ovsdb.c | 415 ++++++++++++++++++++++++------------- 1 file changed, 266 insertions(+), 149 deletions(-) diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c index 62146fef46..b78bb8a804 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -236,6 +236,14 @@ _free_interface(OpenvswitchInterface *ovs_interface) nm_g_slice_free(ovs_interface); } +static gboolean +_openvswitch_interface_should_emit_signal(const OpenvswitchInterface *ovs_interface) +{ + /* Currently, the factory only creates NMDevices for + * internal interfaces. We ignore the rest. */ + return nm_streq0(ovs_interface->type, "internal"); +} + /*****************************************************************************/ static void @@ -1187,7 +1195,7 @@ ovsdb_next_command(NMOvsdb *self) * [ "uuid", "185c93f6-0b39-424e-8587-77d074aa7ce0" ], ... ] ] */ static void -_uuids_to_array(GPtrArray *array, const json_t *items) +_uuids_to_array_inplace(GPtrArray *array, const json_t *items) { const char *key; json_t * value; @@ -1201,19 +1209,36 @@ _uuids_to_array(GPtrArray *array, const json_t *items) value = json_array_get(items, index); index++; - if (!value) + if (!value || !key) return; - if (nm_streq0(key, "uuid") && json_is_string(value)) { - g_ptr_array_add(array, g_strdup(json_string_value(value))); - } else if (nm_streq0(key, "set") && json_is_array(value)) { - json_array_foreach (value, set_index, set_value) { - _uuids_to_array(array, set_value); + if (nm_streq(key, "uuid")) { + if (json_is_string(value)) + g_ptr_array_add(array, g_strdup(json_string_value(value))); + continue; + } + if (nm_streq(key, "set")) { + if (json_is_array(value)) { + json_array_foreach (value, set_index, set_value) + _uuids_to_array_inplace(array, set_value); } + continue; } } } +static GPtrArray * +_uuids_to_array(const json_t *items) +{ + GPtrArray *array; + + array = g_ptr_array_new_with_free_func(g_free); + _uuids_to_array_inplace(array, items); + return array; +} + +/*****************************************************************************/ + static char * _connection_uuid_from_external_ids(json_t *external_ids) { @@ -1251,14 +1276,11 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) json_error_t json_error = { 0, }; - void * iter; - const char * name; - const char * key; - const char * type; - json_t * value; - OpenvswitchBridge * ovs_bridge; - OpenvswitchPort * ovs_port; - OpenvswitchInterface *ovs_interface; + void * iter; + const char *name; + const char *key; + const char *type; + json_t * value; if (json_unpack_ex(msg, &json_error, @@ -1287,16 +1309,13 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) nm_utils_strdup_reset(&priv->db_uuid, s); } - /* Interfaces */ json_object_foreach (interface, key, value) { - json_t * error = NULL; - gboolean old = FALSE; - gboolean new = FALSE; + OpenvswitchInterface *ovs_interface; + gs_free char * connection_uuid = NULL; + json_t * error = NULL; + int r; - if (json_unpack(value, "{s:{}}", "old") == 0) - old = TRUE; - - if (json_unpack(value, + r = json_unpack(value, "{s:{s:s, s:s, s?:o, s:o}}", "new", "name", @@ -1306,81 +1325,109 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) "error", &error, "external_ids", - &external_ids) - == 0) - new = TRUE; + &external_ids); + if (r != 0) { + gpointer unused; - if (old) { - ovs_interface = g_hash_table_lookup(priv->interfaces, &key); - if (!ovs_interface) { - _LOGW("Interface '%s' was not seen", key); - } else if (!new || !nm_streq(ovs_interface->name, name)) { - old = FALSE; - _LOGT("removed an '%s' interface: %s%s%s", - ovs_interface->type, - ovs_interface->name, - ovs_interface->connection_uuid ? ", " : "", - ovs_interface->connection_uuid ?: ""); - if (nm_streq0(ovs_interface->type, "internal")) { - /* Currently, the factory only creates NMDevices for - * internal interfaces. Ignore the rest. */ - _signal_emit_device_removed(self, - ovs_interface->name, - NM_DEVICE_TYPE_OVS_INTERFACE); - } + r = json_unpack(value, "{s:{}}", "old"); + if (r != 0) + continue; + + if (!g_hash_table_steal_extended(priv->interfaces, + &key, + (gpointer *) &ovs_interface, + &unused)) + continue; + + _LOGT("obj[iface:%s]: removed an '%s' interface: %s%s%s", + key, + ovs_interface->type, + ovs_interface->name, + NM_PRINT_FMT_QUOTED2(ovs_interface->connection_uuid, + ", ", + ovs_interface->connection_uuid, + "")); + if (_openvswitch_interface_should_emit_signal(ovs_interface)) { + _signal_emit_device_removed(self, + ovs_interface->name, + NM_DEVICE_TYPE_OVS_INTERFACE); } - g_hash_table_remove(priv->interfaces, &key); + _free_interface(ovs_interface); + continue; } - if (new) { + ovs_interface = g_hash_table_lookup(priv->interfaces, &key); + + if (ovs_interface + && (!nm_streq0(ovs_interface->name, name) || !nm_streq0(ovs_interface->type, type))) { + if (!g_hash_table_steal(priv->interfaces, ovs_interface)) + nm_assert_not_reached(); + if (_openvswitch_interface_should_emit_signal(ovs_interface)) { + _signal_emit_device_removed(self, + ovs_interface->name, + NM_DEVICE_TYPE_OVS_INTERFACE); + } + nm_clear_pointer(&ovs_interface, _free_interface); + } + + connection_uuid = _connection_uuid_from_external_ids(external_ids); + + if (ovs_interface) { + gboolean changed = FALSE; + + nm_assert(nm_streq0(ovs_interface->name, name)); + + changed |= nm_utils_strdup_reset(&ovs_interface->type, type); + changed |= nm_utils_strdup_reset_take(&ovs_interface->connection_uuid, + g_steal_pointer(&connection_uuid)); + if (changed) { + _LOGT("obj[iface:%s]: changed an '%s' interface: %s%s%s", + key, + type, + ovs_interface->name, + NM_PRINT_FMT_QUOTED2(ovs_interface->connection_uuid, + ", ", + ovs_interface->connection_uuid, + "")); + } + } else { ovs_interface = g_slice_new(OpenvswitchInterface); *ovs_interface = (OpenvswitchInterface){ .interface_uuid = g_strdup(key), .name = g_strdup(name), .type = g_strdup(type), - .connection_uuid = _connection_uuid_from_external_ids(external_ids), + .connection_uuid = g_steal_pointer(&connection_uuid), }; g_hash_table_add(priv->interfaces, ovs_interface); - if (old) { - _LOGT("changed an '%s' interface: %s%s%s", - type, - ovs_interface->name, - ovs_interface->connection_uuid ? ", " : "", - ovs_interface->connection_uuid ?: ""); - } else { - _LOGT("added an '%s' interface: %s%s%s", - ovs_interface->type, - ovs_interface->name, - ovs_interface->connection_uuid ? ", " : "", - ovs_interface->connection_uuid ?: ""); - if (nm_streq0(ovs_interface->type, "internal")) { - /* Currently, the factory only creates NMDevices for - * internal interfaces. Ignore the rest. */ - _signal_emit_device_added(self, - ovs_interface->name, - NM_DEVICE_TYPE_OVS_INTERFACE); - } - } - /* The error is a string. No error is indicated by an empty set, - * because why the fuck not: [ "set": [] ] */ - if (error && json_is_string(error)) { - _signal_emit_interface_failed(self, - ovs_interface->name, - ovs_interface->connection_uuid, - json_string_value(error)); - } + _LOGT("obj[iface:%s]: added an '%s' interface: %s%s%s", + key, + ovs_interface->type, + ovs_interface->name, + NM_PRINT_FMT_QUOTED2(ovs_interface->connection_uuid, + ", ", + ovs_interface->connection_uuid, + "")); + if (_openvswitch_interface_should_emit_signal(ovs_interface)) + _signal_emit_device_added(self, ovs_interface->name, NM_DEVICE_TYPE_OVS_INTERFACE); + } + + /* The error is a string. No error is indicated by an empty set, + * Why not: [ "set": [] ] ? */ + if (error && json_is_string(error)) { + _signal_emit_interface_failed(self, + ovs_interface->name, + ovs_interface->connection_uuid, + json_string_value(error)); } } - /* Ports */ json_object_foreach (port, key, value) { - gboolean old = FALSE; - gboolean new = FALSE; + gs_unref_ptrarray GPtrArray *interfaces = NULL; + OpenvswitchPort * ovs_port; + gs_free char * connection_uuid = NULL; + int r; - if (json_unpack(value, "{s:{}}", "old") == 0) - old = TRUE; - - if (json_unpack(value, + r = json_unpack(value, "{s:{s:s, s:o, s:o}}", "new", "name", @@ -1388,57 +1435,89 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) "external_ids", &external_ids, "interfaces", - &items) - == 0) - new = TRUE; + &items); + if (r != 0) { + gpointer unused; - if (old) { - ovs_port = g_hash_table_lookup(priv->ports, &key); - if (!new || (ovs_port && !nm_streq0(ovs_port->name, name))) { - old = FALSE; - _LOGT("removed a port: %s%s%s", - ovs_port->name, - ovs_port->connection_uuid ? ", " : "", - ovs_port->connection_uuid ?: ""); - _signal_emit_device_removed(self, ovs_port->name, NM_DEVICE_TYPE_OVS_PORT); - } - g_hash_table_remove(priv->ports, &key); + r = json_unpack(value, "{s:{}}", "old"); + if (r != 0) + continue; + + if (!g_hash_table_steal_extended(priv->ports, &key, (gpointer *) &ovs_port, &unused)) + continue; + + _LOGT("obj[port:%s]: removed a port: %s%s%s", + key, + ovs_port->name, + NM_PRINT_FMT_QUOTED2(ovs_port->connection_uuid, + ", ", + ovs_port->connection_uuid, + "")); + _signal_emit_device_removed(self, ovs_port->name, NM_DEVICE_TYPE_OVS_PORT); + _free_port(ovs_port); + continue; } - if (new) { + ovs_port = g_hash_table_lookup(priv->ports, &key); + + if (ovs_port && !nm_streq0(ovs_port->name, name)) { + if (!g_hash_table_steal(priv->ports, ovs_port)) + nm_assert_not_reached(); + _signal_emit_device_removed(self, ovs_port->name, NM_DEVICE_TYPE_OVS_PORT); + nm_clear_pointer(&ovs_port, _free_port); + } + + connection_uuid = _connection_uuid_from_external_ids(external_ids); + interfaces = _uuids_to_array(items); + + if (ovs_port) { + gboolean changed = FALSE; + + nm_assert(nm_streq0(ovs_port->name, name)); + + changed |= nm_utils_strdup_reset(&ovs_port->name, name); + changed |= nm_utils_strdup_reset_take(&ovs_port->connection_uuid, + g_steal_pointer(&connection_uuid)); + if (nm_strv_ptrarray_cmp(ovs_port->interfaces, interfaces) != 0) { + NM_SWAP(&ovs_port->interfaces, &interfaces); + changed = TRUE; + } + if (changed) { + _LOGT("obj[port:%s]: changed a port: %s%s%s", + key, + ovs_port->name, + NM_PRINT_FMT_QUOTED2(ovs_port->connection_uuid, + ", ", + ovs_port->connection_uuid, + "")); + } + } else { ovs_port = g_slice_new(OpenvswitchPort); *ovs_port = (OpenvswitchPort){ .port_uuid = g_strdup(key), .name = g_strdup(name), - .connection_uuid = _connection_uuid_from_external_ids(external_ids), - .interfaces = g_ptr_array_new_with_free_func(g_free), + .connection_uuid = g_steal_pointer(&connection_uuid), + .interfaces = g_steal_pointer(&interfaces), }; - _uuids_to_array(ovs_port->interfaces, items); g_hash_table_add(priv->ports, ovs_port); - if (old) { - _LOGT("changed a port: %s%s%s", - ovs_port->name, - ovs_port->connection_uuid ? ", " : "", - ovs_port->connection_uuid ?: ""); - } else { - _LOGT("added a port: %s%s%s", - ovs_port->name, - ovs_port->connection_uuid ? ", " : "", - ovs_port->connection_uuid ?: ""); - _signal_emit_device_added(self, ovs_port->name, NM_DEVICE_TYPE_OVS_PORT); - } + _LOGT("obj[port:%s]: added a port: %s%s%s", + key, + ovs_port->name, + NM_PRINT_FMT_QUOTED2(ovs_port->connection_uuid, + ", ", + ovs_port->connection_uuid, + "")); + _signal_emit_device_added(self, ovs_port->name, NM_DEVICE_TYPE_OVS_PORT); } } - /* Bridges */ json_object_foreach (bridge, key, value) { - gboolean old = FALSE; - gboolean new = FALSE; + gs_unref_ptrarray GPtrArray *ports = NULL; + OpenvswitchBridge * ovs_bridge; + gs_free char * connection_uuid = NULL; + int r; - if (json_unpack(value, "{s:{}}", "old") == 0) - old = TRUE; - - if (json_unpack(value, + r = json_unpack(value, "{s:{s:s, s:o, s:o}}", "new", "name", @@ -1446,45 +1525,83 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) "external_ids", &external_ids, "ports", - &items) - == 0) - new = TRUE; + &items); - if (old) { - ovs_bridge = g_hash_table_lookup(priv->bridges, &key); - if (!new || (ovs_bridge && !nm_streq0(ovs_bridge->name, name))) { - old = FALSE; - _LOGT("removed a bridge: %s%s%s", - ovs_bridge->name, - ovs_bridge->connection_uuid ? ", " : "", - ovs_bridge->connection_uuid ?: ""); - _signal_emit_device_removed(self, ovs_bridge->name, NM_DEVICE_TYPE_OVS_BRIDGE); - } - g_hash_table_remove(priv->bridges, &key); + if (r != 0) { + gpointer unused; + + r = json_unpack(value, "{s:{}}", "old"); + if (r != 0) + continue; + + if (!g_hash_table_steal_extended(priv->bridges, + &key, + (gpointer *) &ovs_bridge, + &unused)) + continue; + + _LOGT("obj[bridge:%s]: removed a bridge: %s%s%s", + key, + ovs_bridge->name, + NM_PRINT_FMT_QUOTED2(ovs_bridge->connection_uuid, + ", ", + ovs_bridge->connection_uuid, + "")); + _signal_emit_device_removed(self, ovs_bridge->name, NM_DEVICE_TYPE_OVS_BRIDGE); + _free_bridge(ovs_bridge); + continue; } - if (new) { + ovs_bridge = g_hash_table_lookup(priv->bridges, &key); + + if (ovs_bridge && !nm_streq0(ovs_bridge->name, name)) { + if (!g_hash_table_steal(priv->bridges, ovs_bridge)) + nm_assert_not_reached(); + _signal_emit_device_removed(self, ovs_bridge->name, NM_DEVICE_TYPE_OVS_BRIDGE); + nm_clear_pointer(&ovs_bridge, _free_bridge); + } + + connection_uuid = _connection_uuid_from_external_ids(external_ids); + ports = _uuids_to_array(items); + + if (ovs_bridge) { + gboolean changed = FALSE; + + nm_assert(nm_streq0(ovs_bridge->name, name)); + + changed = nm_utils_strdup_reset(&ovs_bridge->name, name); + changed = nm_utils_strdup_reset_take(&ovs_bridge->connection_uuid, + g_steal_pointer(&connection_uuid)); + if (nm_strv_ptrarray_cmp(ovs_bridge->ports, ports) != 0) { + NM_SWAP(&ovs_bridge->ports, &ports); + changed = TRUE; + } + if (changed) { + _LOGT("obj[bridge:%s]: changed a bridge: %s%s%s", + key, + ovs_bridge->name, + NM_PRINT_FMT_QUOTED2(ovs_bridge->connection_uuid, + ", ", + ovs_bridge->connection_uuid, + "")); + } + } else { ovs_bridge = g_slice_new(OpenvswitchBridge); *ovs_bridge = (OpenvswitchBridge){ .bridge_uuid = g_strdup(key), .name = g_strdup(name), - .connection_uuid = _connection_uuid_from_external_ids(external_ids), - .ports = g_ptr_array_new_with_free_func(g_free), + .connection_uuid = g_steal_pointer(&connection_uuid), + .ports = g_steal_pointer(&ports), }; - _uuids_to_array(ovs_bridge->ports, items); g_hash_table_add(priv->bridges, ovs_bridge); - if (old) { - _LOGT("changed a bridge: %s%s%s", - ovs_bridge->name, - ovs_bridge->connection_uuid ? ", " : "", - ovs_bridge->connection_uuid ?: ""); - } else { - _LOGT("added a bridge: %s%s%s", - ovs_bridge->name, - ovs_bridge->connection_uuid ? ", " : "", - ovs_bridge->connection_uuid ?: ""); - _signal_emit_device_added(self, ovs_bridge->name, NM_DEVICE_TYPE_OVS_BRIDGE); - } + _LOGT("obj[bridge:%s]: added a bridge: %s%s%s", + key, + ovs_bridge->name, + NM_PRINT_FMT_QUOTED2(ovs_bridge->connection_uuid, + ", ", + ovs_bridge->connection_uuid, + "")); + _signal_emit_device_added(self, ovs_bridge->name, NM_DEVICE_TYPE_OVS_BRIDGE); } } } From 1eeca3c606aba71937d129c0a45f38d88f4d7178 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 6 Nov 2020 16:32:01 +0100 Subject: [PATCH 30/35] core/ovs: track external-ids for cached ovsdb objects We will need them later. --- .../nm-libnm-core-utils.h | 3 +- src/devices/ovs/nm-ovsdb.c | 163 +++++++++++++----- 2 files changed, 120 insertions(+), 46 deletions(-) diff --git a/libnm-core/nm-libnm-core-intern/nm-libnm-core-utils.h b/libnm-core/nm-libnm-core-intern/nm-libnm-core-utils.h index 2361bf8c30..ac7b3cc8e8 100644 --- a/libnm-core/nm-libnm-core-intern/nm-libnm-core-utils.h +++ b/libnm-core/nm-libnm-core-intern/nm-libnm-core-utils.h @@ -98,7 +98,8 @@ gboolean nm_utils_vlan_priority_map_parse_str(NMVlanPriorityMap map_type, /*****************************************************************************/ -#define NM_OVS_EXTERNAL_ID_NM_PREFIX "NM." +#define NM_OVS_EXTERNAL_ID_NM_PREFIX "NM." +#define NM_OVS_EXTERNAL_ID_NM_CONNECTION_UUID "NM.connection.uuid" /*****************************************************************************/ diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c index b78bb8a804..f197dcf362 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -30,6 +30,7 @@ typedef struct { char * name; char * connection_uuid; GPtrArray *interfaces; /* interface uuids */ + GArray * external_ids; } OpenvswitchPort; typedef struct { @@ -37,13 +38,15 @@ typedef struct { char * name; char * connection_uuid; GPtrArray *ports; /* port uuids */ + GArray * external_ids; } OpenvswitchBridge; typedef struct { - char *interface_uuid; - char *name; - char *type; - char *connection_uuid; + char * interface_uuid; + char * name; + char * type; + char * connection_uuid; + GArray *external_ids; } OpenvswitchInterface; /*****************************************************************************/ @@ -213,6 +216,7 @@ _free_bridge(OpenvswitchBridge *ovs_bridge) g_free(ovs_bridge->name); g_free(ovs_bridge->connection_uuid); g_ptr_array_free(ovs_bridge->ports, TRUE); + nm_g_array_unref(ovs_bridge->external_ids); nm_g_slice_free(ovs_bridge); } @@ -223,6 +227,7 @@ _free_port(OpenvswitchPort *ovs_port) g_free(ovs_port->name); g_free(ovs_port->connection_uuid); g_ptr_array_free(ovs_port->interfaces, TRUE); + nm_g_array_unref(ovs_port->external_ids); nm_g_slice_free(ovs_port); } @@ -233,6 +238,7 @@ _free_interface(OpenvswitchInterface *ovs_interface) g_free(ovs_interface->name); g_free(ovs_interface->connection_uuid); g_free(ovs_interface->type); + nm_g_array_unref(ovs_interface->external_ids); nm_g_slice_free(ovs_interface); } @@ -593,7 +599,7 @@ _insert_interface(json_t * params, options, "external_ids", "map", - "NM.connection.uuid", + NM_OVS_EXTERNAL_ID_NM_CONNECTION_UUID, nm_connection_get_uuid(interface)); if (cloned_mac) @@ -659,10 +665,12 @@ _insert_port(json_t *params, NMConnection *port, json_t *new_interfaces) json_object_set_new(row, "name", json_string(nm_connection_get_interface_name(port))); json_object_set_new(row, "interfaces", json_pack("[s, O]", "set", new_interfaces)); - json_object_set_new( - row, - "external_ids", - json_pack("[s, [[s, s]]]", "map", "NM.connection.uuid", nm_connection_get_uuid(port))); + json_object_set_new(row, + "external_ids", + json_pack("[s, [[s, s]]]", + "map", + NM_OVS_EXTERNAL_ID_NM_CONNECTION_UUID, + nm_connection_get_uuid(port))); /* Create a new one. */ json_array_append_new(params, @@ -722,10 +730,12 @@ _insert_bridge(json_t * params, json_object_set_new(row, "name", json_string(nm_connection_get_interface_name(bridge))); json_object_set_new(row, "ports", json_pack("[s, O]", "set", new_ports)); - json_object_set_new( - row, - "external_ids", - json_pack("[s, [[s, s]]]", "map", "NM.connection.uuid", nm_connection_get_uuid(bridge))); + json_object_set_new(row, + "external_ids", + json_pack("[s, [[s, s]]]", + "map", + NM_OVS_EXTERNAL_ID_NM_CONNECTION_UUID, + nm_connection_get_uuid(bridge))); if (cloned_mac) { json_object_set_new(row, @@ -1237,25 +1247,72 @@ _uuids_to_array(const json_t *items) return array; } -/*****************************************************************************/ - -static char * -_connection_uuid_from_external_ids(json_t *external_ids) +static void +_external_ids_extract(json_t *external_ids, GArray **out_array, const char **out_connection_uuid) { + json_t *array; json_t *value; - size_t index; + gsize index; + + nm_assert(out_array && !*out_array); + nm_assert(!out_connection_uuid || !*out_connection_uuid); if (!nm_streq0("map", json_string_value(json_array_get(external_ids, 0)))) - return NULL; + return; - json_array_foreach (json_array_get(external_ids, 1), index, value) { - if (nm_streq0("NM.connection.uuid", json_string_value(json_array_get(value, 0)))) - return g_strdup(json_string_value(json_array_get(value, 1))); + array = json_array_get(external_ids, 1); + + json_array_foreach (array, index, value) { + const char * key = json_string_value(json_array_get(value, 0)); + const char * val = json_string_value(json_array_get(value, 1)); + NMUtilsNamedValue *v; + + if (!key || !val) + continue; + + if (!*out_array) { + *out_array = g_array_new(FALSE, FALSE, sizeof(NMUtilsNamedValue)); + g_array_set_clear_func(*out_array, + (GDestroyNotify) nm_utils_named_value_clear_with_g_free); + } + + v = nm_g_array_append_new(*out_array, NMUtilsNamedValue); + *v = (NMUtilsNamedValue){ + .name = g_strdup(key), + .value_str = g_strdup(val), + }; + + if (out_connection_uuid && nm_streq(v->name, NM_OVS_EXTERNAL_ID_NM_CONNECTION_UUID)) { + *out_connection_uuid = v->value_str; + out_connection_uuid = NULL; + } } - - return NULL; } +static gboolean +_external_ids_equal(const GArray *arr1, const GArray *arr2) +{ + guint n; + guint i; + + n = nm_g_array_len(arr1); + + if (n != nm_g_array_len(arr2)) + return FALSE; + for (i = 0; i < n; i++) { + const NMUtilsNamedValue *n1 = &g_array_index(arr1, NMUtilsNamedValue, i); + const NMUtilsNamedValue *n2 = &g_array_index(arr2, NMUtilsNamedValue, i); + + if (!nm_streq0(n1->name, n2->name)) + return FALSE; + if (!nm_streq0(n1->value_str, n2->value_str)) + return FALSE; + } + return TRUE; +} + +/*****************************************************************************/ + /** * ovsdb_got_update: * @@ -1311,9 +1368,10 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) json_object_foreach (interface, key, value) { OpenvswitchInterface *ovs_interface; - gs_free char * connection_uuid = NULL; - json_t * error = NULL; - int r; + gs_unref_array GArray *external_ids_arr = NULL; + const char * connection_uuid = NULL; + json_t * error = NULL; + int r; r = json_unpack(value, "{s:{s:s, s:s, s?:o, s:o}}", @@ -1370,7 +1428,7 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) nm_clear_pointer(&ovs_interface, _free_interface); } - connection_uuid = _connection_uuid_from_external_ids(external_ids); + _external_ids_extract(external_ids, &external_ids_arr, &connection_uuid); if (ovs_interface) { gboolean changed = FALSE; @@ -1378,8 +1436,11 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) nm_assert(nm_streq0(ovs_interface->name, name)); changed |= nm_utils_strdup_reset(&ovs_interface->type, type); - changed |= nm_utils_strdup_reset_take(&ovs_interface->connection_uuid, - g_steal_pointer(&connection_uuid)); + changed |= nm_utils_strdup_reset(&ovs_interface->connection_uuid, connection_uuid); + if (!_external_ids_equal(ovs_interface->external_ids, external_ids_arr)) { + NM_SWAP(&ovs_interface->external_ids, &external_ids_arr); + changed = TRUE; + } if (changed) { _LOGT("obj[iface:%s]: changed an '%s' interface: %s%s%s", key, @@ -1396,7 +1457,8 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) .interface_uuid = g_strdup(key), .name = g_strdup(name), .type = g_strdup(type), - .connection_uuid = g_steal_pointer(&connection_uuid), + .connection_uuid = g_strdup(connection_uuid), + .external_ids = g_steal_pointer(&external_ids_arr), }; g_hash_table_add(priv->interfaces, ovs_interface); _LOGT("obj[iface:%s]: added an '%s' interface: %s%s%s", @@ -1424,8 +1486,9 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) json_object_foreach (port, key, value) { gs_unref_ptrarray GPtrArray *interfaces = NULL; OpenvswitchPort * ovs_port; - gs_free char * connection_uuid = NULL; - int r; + gs_unref_array GArray *external_ids_arr = NULL; + const char * connection_uuid = NULL; + int r; r = json_unpack(value, "{s:{s:s, s:o, s:o}}", @@ -1467,8 +1530,8 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) nm_clear_pointer(&ovs_port, _free_port); } - connection_uuid = _connection_uuid_from_external_ids(external_ids); - interfaces = _uuids_to_array(items); + _external_ids_extract(external_ids, &external_ids_arr, &connection_uuid); + interfaces = _uuids_to_array(items); if (ovs_port) { gboolean changed = FALSE; @@ -1476,12 +1539,15 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) nm_assert(nm_streq0(ovs_port->name, name)); changed |= nm_utils_strdup_reset(&ovs_port->name, name); - changed |= nm_utils_strdup_reset_take(&ovs_port->connection_uuid, - g_steal_pointer(&connection_uuid)); + changed |= nm_utils_strdup_reset(&ovs_port->connection_uuid, g_strdup(connection_uuid)); if (nm_strv_ptrarray_cmp(ovs_port->interfaces, interfaces) != 0) { NM_SWAP(&ovs_port->interfaces, &interfaces); changed = TRUE; } + if (!_external_ids_equal(ovs_port->external_ids, external_ids_arr)) { + NM_SWAP(&ovs_port->external_ids, &external_ids_arr); + changed = TRUE; + } if (changed) { _LOGT("obj[port:%s]: changed a port: %s%s%s", key, @@ -1496,8 +1562,9 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) *ovs_port = (OpenvswitchPort){ .port_uuid = g_strdup(key), .name = g_strdup(name), - .connection_uuid = g_steal_pointer(&connection_uuid), + .connection_uuid = g_strdup(connection_uuid), .interfaces = g_steal_pointer(&interfaces), + .external_ids = g_steal_pointer(&external_ids_arr), }; g_hash_table_add(priv->ports, ovs_port); _LOGT("obj[port:%s]: added a port: %s%s%s", @@ -1514,8 +1581,9 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) json_object_foreach (bridge, key, value) { gs_unref_ptrarray GPtrArray *ports = NULL; OpenvswitchBridge * ovs_bridge; - gs_free char * connection_uuid = NULL; - int r; + gs_unref_array GArray *external_ids_arr = NULL; + const char * connection_uuid = NULL; + int r; r = json_unpack(value, "{s:{s:s, s:o, s:o}}", @@ -1561,8 +1629,8 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) nm_clear_pointer(&ovs_bridge, _free_bridge); } - connection_uuid = _connection_uuid_from_external_ids(external_ids); - ports = _uuids_to_array(items); + _external_ids_extract(external_ids, &external_ids_arr, &connection_uuid); + ports = _uuids_to_array(items); if (ovs_bridge) { gboolean changed = FALSE; @@ -1570,12 +1638,16 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) nm_assert(nm_streq0(ovs_bridge->name, name)); changed = nm_utils_strdup_reset(&ovs_bridge->name, name); - changed = nm_utils_strdup_reset_take(&ovs_bridge->connection_uuid, - g_steal_pointer(&connection_uuid)); + changed = + nm_utils_strdup_reset(&ovs_bridge->connection_uuid, g_strdup(connection_uuid)); if (nm_strv_ptrarray_cmp(ovs_bridge->ports, ports) != 0) { NM_SWAP(&ovs_bridge->ports, &ports); changed = TRUE; } + if (!_external_ids_equal(ovs_bridge->external_ids, external_ids_arr)) { + NM_SWAP(&ovs_bridge->external_ids, &external_ids_arr); + changed = TRUE; + } if (changed) { _LOGT("obj[bridge:%s]: changed a bridge: %s%s%s", key, @@ -1590,8 +1662,9 @@ ovsdb_got_update(NMOvsdb *self, json_t *msg) *ovs_bridge = (OpenvswitchBridge){ .bridge_uuid = g_strdup(key), .name = g_strdup(name), - .connection_uuid = g_steal_pointer(&connection_uuid), + .connection_uuid = g_strdup(connection_uuid), .ports = g_steal_pointer(&ports), + .external_ids = g_steal_pointer(&external_ids_arr), }; g_hash_table_add(priv->bridges, ovs_bridge); _LOGT("obj[bridge:%s]: added a bridge: %s%s%s", From 2d8c5e9efad1d4ea6fbbd330db68d562ba886359 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 6 Nov 2020 19:33:02 +0100 Subject: [PATCH 31/35] core/ovs: cleanup debug logging for OVS command - always print the JSON string as last (if present). Previously that didn't happen with OVSDB_SET_INTERFACE_MTU. - introduce _QUOTE_MSG() macro. --- src/devices/ovs/nm-ovsdb.c | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c index f197dcf362..2a17f08666 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -141,14 +141,14 @@ typedef struct { static void _LOGT_call_do(const char *comment, OvsdbMethodCall *call, json_t *msg) { - gs_free char *str = NULL; + gs_free char *msg_as_str = NULL; - if (msg) - str = json_dumps(msg, 0); +#define _QUOTE_MSG(msg, msg_as_str) \ + (msg) ? ": " : "", (msg) ? (msg_as_str = json_dumps((msg), 0)) : "" switch (call->command) { case OVSDB_MONITOR: - _LOGT("%s: monitor%s%s", comment, msg ? ": " : "", msg ? str : ""); + _LOGT("%s: monitor%s%s", comment, _QUOTE_MSG(msg, msg_as_str)); break; case OVSDB_ADD_INTERFACE: _LOGT("%s: add-iface bridge=%s port=%s interface=%s%s%s", @@ -156,23 +156,17 @@ _LOGT_call_do(const char *comment, OvsdbMethodCall *call, json_t *msg) nm_connection_get_interface_name(call->bridge), nm_connection_get_interface_name(call->port), nm_connection_get_interface_name(call->interface), - msg ? ": " : "", - msg ? str : ""); + _QUOTE_MSG(msg, msg_as_str)); break; case OVSDB_DEL_INTERFACE: - _LOGT("%s: del-iface interface=%s%s%s", - comment, - call->ifname, - msg ? ": " : "", - msg ? str : ""); + _LOGT("%s: del-iface interface=%s%s%s", comment, call->ifname, _QUOTE_MSG(msg, msg_as_str)); break; case OVSDB_SET_INTERFACE_MTU: - _LOGT("%s: set-iface-mtu interface=%s%s%s mtu=%u", + _LOGT("%s: set-iface-mtu interface=%s mtu=%u%s%s", comment, call->ifname, - msg ? ": " : "", - msg ? str : "", - call->mtu); + call->mtu, + _QUOTE_MSG(msg, msg_as_str)); break; } } From 487c78733e01c488f32ab99f1f5d6cd378e269c8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 6 Nov 2020 19:41:25 +0100 Subject: [PATCH 32/35] core/ovs: name union fields in OvsdbMethodCall As we add more command types, the union gets more members. Name each union field explicitly to match the OvsdbCommand type. --- src/devices/ovs/nm-ovsdb.c | 74 +++++++++++++++++++++----------------- 1 file changed, 41 insertions(+), 33 deletions(-) diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c index 2a17f08666..c706df4663 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -122,17 +122,20 @@ typedef struct { OvsdbMethodCallback callback; gpointer user_data; union { - struct { - char * ifname; - guint32 mtu; - }; struct { NMConnection *bridge; NMConnection *port; NMConnection *interface; NMDevice * bridge_device; NMDevice * interface_device; - }; + } add_interface; + struct { + char *ifname; + } del_interface; + struct { + char * ifname; + guint32 mtu; + } set_interface_mtu; }; } OvsdbMethodCall; @@ -153,19 +156,22 @@ _LOGT_call_do(const char *comment, OvsdbMethodCall *call, json_t *msg) case OVSDB_ADD_INTERFACE: _LOGT("%s: add-iface bridge=%s port=%s interface=%s%s%s", comment, - nm_connection_get_interface_name(call->bridge), - nm_connection_get_interface_name(call->port), - nm_connection_get_interface_name(call->interface), + nm_connection_get_interface_name(call->add_interface.bridge), + nm_connection_get_interface_name(call->add_interface.port), + nm_connection_get_interface_name(call->add_interface.interface), _QUOTE_MSG(msg, msg_as_str)); break; case OVSDB_DEL_INTERFACE: - _LOGT("%s: del-iface interface=%s%s%s", comment, call->ifname, _QUOTE_MSG(msg, msg_as_str)); + _LOGT("%s: del-iface interface=%s%s%s", + comment, + call->del_interface.ifname, + _QUOTE_MSG(msg, msg_as_str)); break; case OVSDB_SET_INTERFACE_MTU: _LOGT("%s: set-iface-mtu interface=%s mtu=%u%s%s", comment, - call->ifname, - call->mtu, + call->set_interface_mtu.ifname, + call->set_interface_mtu.mtu, _QUOTE_MSG(msg, msg_as_str)); break; } @@ -190,15 +196,17 @@ _clear_call(gpointer data) case OVSDB_MONITOR: break; case OVSDB_ADD_INTERFACE: - g_clear_object(&call->bridge); - g_clear_object(&call->port); - g_clear_object(&call->interface); - g_clear_object(&call->bridge_device); - g_clear_object(&call->interface_device); + g_clear_object(&call->add_interface.bridge); + g_clear_object(&call->add_interface.port); + g_clear_object(&call->add_interface.interface); + g_clear_object(&call->add_interface.bridge_device); + g_clear_object(&call->add_interface.interface_device); break; case OVSDB_DEL_INTERFACE: + nm_clear_g_free(&call->del_interface.ifname); + break; case OVSDB_SET_INTERFACE_MTU: - nm_clear_g_free(&call->ifname); + nm_clear_g_free(&call->set_interface_mtu.ifname); break; } } @@ -313,18 +321,18 @@ ovsdb_call_method(NMOvsdb * self, case OVSDB_ADD_INTERFACE: /* FIXME(applied-connection-immutable): we should not modify the applied * connection, consequently there is no need to clone the connections. */ - call->bridge = nm_simple_connection_new_clone(bridge); - call->port = nm_simple_connection_new_clone(port); - call->interface = nm_simple_connection_new_clone(interface); - call->bridge_device = g_object_ref(bridge_device); - call->interface_device = g_object_ref(interface_device); + call->add_interface.bridge = nm_simple_connection_new_clone(bridge); + call->add_interface.port = nm_simple_connection_new_clone(port); + call->add_interface.interface = nm_simple_connection_new_clone(interface); + call->add_interface.bridge_device = g_object_ref(bridge_device); + call->add_interface.interface_device = g_object_ref(interface_device); break; case OVSDB_DEL_INTERFACE: - call->ifname = g_strdup(ifname); + call->del_interface.ifname = g_strdup(ifname); break; case OVSDB_SET_INTERFACE_MTU: - call->ifname = g_strdup(ifname); - call->mtu = mtu; + call->set_interface_mtu.ifname = g_strdup(ifname); + call->set_interface_mtu.mtu = mtu; break; } @@ -1117,11 +1125,11 @@ ovsdb_next_command(NMOvsdb *self) _add_interface(self, params, - call->bridge, - call->port, - call->interface, - call->bridge_device, - call->interface_device); + call->add_interface.bridge, + call->add_interface.port, + call->add_interface.interface, + call->add_interface.bridge_device, + call->add_interface.interface_device); msg = json_pack("{s:I, s:s, s:o}", "id", @@ -1136,7 +1144,7 @@ ovsdb_next_command(NMOvsdb *self) json_array_append_new(params, json_string("Open_vSwitch")); json_array_append_new(params, _inc_next_cfg(priv->db_uuid)); - _delete_interface(self, params, call->ifname); + _delete_interface(self, params, call->del_interface.ifname); msg = json_pack("{s:I, s:s, s:o}", "id", @@ -1159,11 +1167,11 @@ ovsdb_next_command(NMOvsdb *self) "Interface", "row", "mtu_request", - (json_int_t) call->mtu, + (json_int_t) call->set_interface_mtu.mtu, "where", "name", "==", - call->ifname)); + call->set_interface_mtu.ifname)); msg = json_pack("{s:I, s:s, s:o}", "id", From 81863c959b82ca84df8775d16c0ba73a3d6ede9c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 6 Nov 2020 19:44:55 +0100 Subject: [PATCH 33/35] core/ovs: rename logging output for _LOGT_call() The text should match the OvsdbCommand enum. If the enum value is named OVSDB_ADD_INTERFACE, then we should print "add-interface". Or alternatively, if you think spelling out interface is too long, then the enum should be renamed. I don't care, but name should correspond. --- src/devices/ovs/nm-ovsdb.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c index c706df4663..2593b9b741 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -154,7 +154,7 @@ _LOGT_call_do(const char *comment, OvsdbMethodCall *call, json_t *msg) _LOGT("%s: monitor%s%s", comment, _QUOTE_MSG(msg, msg_as_str)); break; case OVSDB_ADD_INTERFACE: - _LOGT("%s: add-iface bridge=%s port=%s interface=%s%s%s", + _LOGT("%s: add-interface bridge=%s port=%s interface=%s%s%s", comment, nm_connection_get_interface_name(call->add_interface.bridge), nm_connection_get_interface_name(call->add_interface.port), @@ -162,13 +162,13 @@ _LOGT_call_do(const char *comment, OvsdbMethodCall *call, json_t *msg) _QUOTE_MSG(msg, msg_as_str)); break; case OVSDB_DEL_INTERFACE: - _LOGT("%s: del-iface interface=%s%s%s", + _LOGT("%s: del-interface interface=%s%s%s", comment, call->del_interface.ifname, _QUOTE_MSG(msg, msg_as_str)); break; case OVSDB_SET_INTERFACE_MTU: - _LOGT("%s: set-iface-mtu interface=%s mtu=%u%s%s", + _LOGT("%s: set-interface-mtu interface=%s mtu=%u%s%s", comment, call->set_interface_mtu.ifname, call->set_interface_mtu.mtu, From 9b5bb3e45cc6a63301e185f31d65d4bb9b2907bd Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 6 Nov 2020 21:24:48 +0100 Subject: [PATCH 34/35] core/ovs: split payload out of OvsdbMethodCall struct Before, ovsdb_call_method() has a long list of arguments to account for all possible commands. That does not scale. Instead, introduce a separate OvsdbMethodPayload type and only add a macro to allow passing the right parameters. --- src/devices/ovs/nm-ovsdb.c | 223 +++++++++++++++++++++---------------- 1 file changed, 125 insertions(+), 98 deletions(-) diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c index 2593b9b741..e4d6e23808 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -116,27 +116,69 @@ typedef enum { #define CALL_ID_UNSPEC G_MAXUINT64 +typedef union { + struct { + } monitor; + struct { + NMConnection *bridge; + NMConnection *port; + NMConnection *interface; + NMDevice * bridge_device; + NMDevice * interface_device; + } add_interface; + struct { + char *ifname; + } del_interface; + struct { + char * ifname; + guint32 mtu; + } set_interface_mtu; +} OvsdbMethodPayload; + +#define OVSDB_METHOD_PAYLOAD_MONITOR() \ + (&((const OvsdbMethodPayload){ \ + .monitor = {}, \ + })) + +#define OVSDB_METHOD_PAYLOAD_ADD_INTERFACE(xbridge, \ + xport, \ + xinterface, \ + xbridge_device, \ + xinterface_device) \ + (&((const OvsdbMethodPayload){ \ + .add_interface = \ + { \ + .bridge = (xbridge), \ + .port = (xport), \ + .interface = (xinterface), \ + .bridge_device = (xbridge_device), \ + .interface_device = (xinterface_device), \ + }, \ + })) + +#define OVSDB_METHOD_PAYLOAD_DEL_INTERFACE(xifname) \ + (&((const OvsdbMethodPayload){ \ + .del_interface = \ + { \ + .ifname = (char *) NM_CONSTCAST(char, (xifname)), \ + }, \ + })) + +#define OVSDB_METHOD_PAYLOAD_SET_INTERFACE_MTU(xifname, xmtu) \ + (&((const OvsdbMethodPayload){ \ + .set_interface_mtu = \ + { \ + .ifname = (char *) NM_CONSTCAST(char, (xifname)), \ + .mtu = (xmtu), \ + }, \ + })) + typedef struct { guint64 call_id; OvsdbCommand command; OvsdbMethodCallback callback; gpointer user_data; - union { - struct { - NMConnection *bridge; - NMConnection *port; - NMConnection *interface; - NMDevice * bridge_device; - NMDevice * interface_device; - } add_interface; - struct { - char *ifname; - } del_interface; - struct { - char * ifname; - guint32 mtu; - } set_interface_mtu; - }; + OvsdbMethodPayload payload; } OvsdbMethodCall; /*****************************************************************************/ @@ -156,22 +198,22 @@ _LOGT_call_do(const char *comment, OvsdbMethodCall *call, json_t *msg) case OVSDB_ADD_INTERFACE: _LOGT("%s: add-interface bridge=%s port=%s interface=%s%s%s", comment, - nm_connection_get_interface_name(call->add_interface.bridge), - nm_connection_get_interface_name(call->add_interface.port), - nm_connection_get_interface_name(call->add_interface.interface), + nm_connection_get_interface_name(call->payload.add_interface.bridge), + nm_connection_get_interface_name(call->payload.add_interface.port), + nm_connection_get_interface_name(call->payload.add_interface.interface), _QUOTE_MSG(msg, msg_as_str)); break; case OVSDB_DEL_INTERFACE: _LOGT("%s: del-interface interface=%s%s%s", comment, - call->del_interface.ifname, + call->payload.del_interface.ifname, _QUOTE_MSG(msg, msg_as_str)); break; case OVSDB_SET_INTERFACE_MTU: _LOGT("%s: set-interface-mtu interface=%s mtu=%u%s%s", comment, - call->set_interface_mtu.ifname, - call->set_interface_mtu.mtu, + call->payload.set_interface_mtu.ifname, + call->payload.set_interface_mtu.mtu, _QUOTE_MSG(msg, msg_as_str)); break; } @@ -196,21 +238,23 @@ _clear_call(gpointer data) case OVSDB_MONITOR: break; case OVSDB_ADD_INTERFACE: - g_clear_object(&call->add_interface.bridge); - g_clear_object(&call->add_interface.port); - g_clear_object(&call->add_interface.interface); - g_clear_object(&call->add_interface.bridge_device); - g_clear_object(&call->add_interface.interface_device); + g_clear_object(&call->payload.add_interface.bridge); + g_clear_object(&call->payload.add_interface.port); + g_clear_object(&call->payload.add_interface.interface); + g_clear_object(&call->payload.add_interface.bridge_device); + g_clear_object(&call->payload.add_interface.interface_device); break; case OVSDB_DEL_INTERFACE: - nm_clear_g_free(&call->del_interface.ifname); + nm_clear_g_free(&call->payload.del_interface.ifname); break; case OVSDB_SET_INTERFACE_MTU: - nm_clear_g_free(&call->set_interface_mtu.ifname); + nm_clear_g_free(&call->payload.set_interface_mtu.ifname); break; } } +/*****************************************************************************/ + static void _free_bridge(OpenvswitchBridge *ovs_bridge) { @@ -284,18 +328,12 @@ _signal_emit_interface_failed(NMOvsdb * self, * there's no command pending completion. */ static void -ovsdb_call_method(NMOvsdb * self, - OvsdbCommand command, - const char * ifname, - NMConnection * bridge, - NMConnection * port, - NMConnection * interface, - NMDevice * bridge_device, - NMDevice * interface_device, - guint32 mtu, - OvsdbMethodCallback callback, - gpointer user_data, - gboolean add_first) +ovsdb_call_method(NMOvsdb * self, + OvsdbMethodCallback callback, + gpointer user_data, + gboolean add_first, + OvsdbCommand command, + const OvsdbMethodPayload *payload) { NMOvsdbPrivate * priv = NM_OVSDB_GET_PRIVATE(self); OvsdbMethodCall *call; @@ -306,33 +344,42 @@ ovsdb_call_method(NMOvsdb * self, if (add_first) { g_array_prepend_val(priv->calls, (OvsdbMethodCall){}); call = &g_array_index(priv->calls, OvsdbMethodCall, 0); - } else { - g_array_set_size(priv->calls, priv->calls->len + 1); - call = &g_array_index(priv->calls, OvsdbMethodCall, priv->calls->len - 1); - } + } else + call = nm_g_array_append_new(priv->calls, OvsdbMethodCall); + call->call_id = CALL_ID_UNSPEC; call->command = command; call->callback = callback; call->user_data = user_data; - switch (call->command) { + /* Mmigrate the arguments from @payload to @call->payload. Technically, + * this is not a plain copy, because + * - call->payload is not initialized (thus no need to free the previous data). + * - payload does not own the data. It is merely initialized using the + * OVSDB_METHOD_PAYLOAD_*() macros. */ + switch (command) { case OVSDB_MONITOR: break; case OVSDB_ADD_INTERFACE: /* FIXME(applied-connection-immutable): we should not modify the applied * connection, consequently there is no need to clone the connections. */ - call->add_interface.bridge = nm_simple_connection_new_clone(bridge); - call->add_interface.port = nm_simple_connection_new_clone(port); - call->add_interface.interface = nm_simple_connection_new_clone(interface); - call->add_interface.bridge_device = g_object_ref(bridge_device); - call->add_interface.interface_device = g_object_ref(interface_device); + call->payload.add_interface.bridge = + nm_simple_connection_new_clone(payload->add_interface.bridge); + call->payload.add_interface.port = + nm_simple_connection_new_clone(payload->add_interface.port); + call->payload.add_interface.interface = + nm_simple_connection_new_clone(payload->add_interface.interface); + call->payload.add_interface.bridge_device = + g_object_ref(payload->add_interface.bridge_device); + call->payload.add_interface.interface_device = + g_object_ref(payload->add_interface.interface_device); break; case OVSDB_DEL_INTERFACE: - call->del_interface.ifname = g_strdup(ifname); + call->payload.del_interface.ifname = g_strdup(payload->del_interface.ifname); break; case OVSDB_SET_INTERFACE_MTU: - call->set_interface_mtu.ifname = g_strdup(ifname); - call->set_interface_mtu.mtu = mtu; + call->payload.set_interface_mtu.ifname = g_strdup(payload->set_interface_mtu.ifname); + call->payload.set_interface_mtu.mtu = payload->set_interface_mtu.mtu; break; } @@ -1125,11 +1172,11 @@ ovsdb_next_command(NMOvsdb *self) _add_interface(self, params, - call->add_interface.bridge, - call->add_interface.port, - call->add_interface.interface, - call->add_interface.bridge_device, - call->add_interface.interface_device); + call->payload.add_interface.bridge, + call->payload.add_interface.port, + call->payload.add_interface.interface, + call->payload.add_interface.bridge_device, + call->payload.add_interface.interface_device); msg = json_pack("{s:I, s:s, s:o}", "id", @@ -1144,7 +1191,7 @@ ovsdb_next_command(NMOvsdb *self) json_array_append_new(params, json_string("Open_vSwitch")); json_array_append_new(params, _inc_next_cfg(priv->db_uuid)); - _delete_interface(self, params, call->del_interface.ifname); + _delete_interface(self, params, call->payload.del_interface.ifname); msg = json_pack("{s:I, s:s, s:o}", "id", @@ -1167,11 +1214,11 @@ ovsdb_next_command(NMOvsdb *self) "Interface", "row", "mtu_request", - (json_int_t) call->set_interface_mtu.mtu, + (json_int_t) call->payload.set_interface_mtu.mtu, "where", "name", "==", - call->set_interface_mtu.ifname)); + call->payload.set_interface_mtu.ifname)); msg = json_pack("{s:I, s:s, s:o}", "id", @@ -2087,17 +2134,11 @@ ovsdb_try_connect(NMOvsdb *self) /* Queue a monitor call before any other command, ensuring that we have an up * to date view of existing bridged that we need for add and remove ops. */ ovsdb_call_method(self, - OVSDB_MONITOR, - NULL, - NULL, - NULL, - NULL, - NULL, - NULL, - 0, _monitor_bridges_cb, NULL, - TRUE); + TRUE, + OVSDB_MONITOR, + OVSDB_METHOD_PAYLOAD_MONITOR()); } /*****************************************************************************/ @@ -2163,17 +2204,15 @@ nm_ovsdb_add_interface(NMOvsdb * self, gpointer user_data) { ovsdb_call_method(self, - OVSDB_ADD_INTERFACE, - NULL, - bridge, - port, - interface, - bridge_device, - interface_device, - 0, _transact_cb, ovsdb_call_new(callback, user_data), - FALSE); + FALSE, + OVSDB_ADD_INTERFACE, + OVSDB_METHOD_PAYLOAD_ADD_INTERFACE(bridge, + port, + interface, + bridge_device, + interface_device)); } void @@ -2183,17 +2222,11 @@ nm_ovsdb_del_interface(NMOvsdb * self, gpointer user_data) { ovsdb_call_method(self, - OVSDB_DEL_INTERFACE, - ifname, - NULL, - NULL, - NULL, - NULL, - NULL, - 0, _transact_cb, ovsdb_call_new(callback, user_data), - FALSE); + FALSE, + OVSDB_DEL_INTERFACE, + OVSDB_METHOD_PAYLOAD_DEL_INTERFACE(ifname)); } void @@ -2204,17 +2237,11 @@ nm_ovsdb_set_interface_mtu(NMOvsdb * self, gpointer user_data) { ovsdb_call_method(self, - OVSDB_SET_INTERFACE_MTU, - ifname, - NULL, - NULL, - NULL, - NULL, - NULL, - mtu, _transact_cb, ovsdb_call_new(callback, user_data), - FALSE); + FALSE, + OVSDB_SET_INTERFACE_MTU, + OVSDB_METHOD_PAYLOAD_SET_INTERFACE_MTU(ifname, mtu)); } /*****************************************************************************/ From bac5dc99d7a983f6323aeb4c3484c178c3d90241 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 9 Nov 2020 10:07:55 +0100 Subject: [PATCH 35/35] core/ovs: refactor duplicate code in ovsdb_next_command() --- src/devices/ovs/nm-ovsdb.c | 93 ++++++++++++++++---------------------- 1 file changed, 40 insertions(+), 53 deletions(-) diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c index e4d6e23808..bd55a8b341 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -1117,15 +1117,15 @@ static void ovsdb_next_command(NMOvsdb *self) { NMOvsdbPrivate * priv = NM_OVSDB_GET_PRIVATE(self); - OvsdbMethodCall * call = NULL; + OvsdbMethodCall * call; char * cmd; nm_auto_decref_json json_t *msg = NULL; - json_t * params; if (!priv->conn) return; if (!priv->calls->len) return; + call = &g_array_index(priv->calls, OvsdbMethodCall, 0); if (call->call_id != CALL_ID_UNSPEC) return; @@ -1165,60 +1165,46 @@ ovsdb_next_command(NMOvsdb *self) "Open_vSwitch", "columns"); break; - case OVSDB_ADD_INTERFACE: + default: + { + json_t *params = NULL; + params = json_array(); json_array_append_new(params, json_string("Open_vSwitch")); json_array_append_new(params, _inc_next_cfg(priv->db_uuid)); - _add_interface(self, - params, - call->payload.add_interface.bridge, - call->payload.add_interface.port, - call->payload.add_interface.interface, - call->payload.add_interface.bridge_device, - call->payload.add_interface.interface_device); - - msg = json_pack("{s:I, s:s, s:o}", - "id", - (json_int_t) call->call_id, - "method", - "transact", - "params", - params); - break; - case OVSDB_DEL_INTERFACE: - params = json_array(); - json_array_append_new(params, json_string("Open_vSwitch")); - json_array_append_new(params, _inc_next_cfg(priv->db_uuid)); - - _delete_interface(self, params, call->payload.del_interface.ifname); - - msg = json_pack("{s:I, s:s, s:o}", - "id", - (json_int_t) call->call_id, - "method", - "transact", - "params", - params); - break; - case OVSDB_SET_INTERFACE_MTU: - params = json_array(); - json_array_append_new(params, json_string("Open_vSwitch")); - json_array_append_new(params, _inc_next_cfg(priv->db_uuid)); - - json_array_append_new(params, - json_pack("{s:s, s:s, s:{s: I}, s:[[s, s, s]]}", - "op", - "update", - "table", - "Interface", - "row", - "mtu_request", - (json_int_t) call->payload.set_interface_mtu.mtu, - "where", - "name", - "==", - call->payload.set_interface_mtu.ifname)); + switch (call->command) { + case OVSDB_ADD_INTERFACE: + _add_interface(self, + params, + call->payload.add_interface.bridge, + call->payload.add_interface.port, + call->payload.add_interface.interface, + call->payload.add_interface.bridge_device, + call->payload.add_interface.interface_device); + break; + case OVSDB_DEL_INTERFACE: + _delete_interface(self, params, call->payload.del_interface.ifname); + break; + case OVSDB_SET_INTERFACE_MTU: + json_array_append_new(params, + json_pack("{s:s, s:s, s:{s: I}, s:[[s, s, s]]}", + "op", + "update", + "table", + "Interface", + "row", + "mtu_request", + (json_int_t) call->payload.set_interface_mtu.mtu, + "where", + "name", + "==", + call->payload.set_interface_mtu.ifname)); + break; + default: + nm_assert_not_reached(); + break; + } msg = json_pack("{s:I, s:s, s:o}", "id", @@ -1229,11 +1215,12 @@ ovsdb_next_command(NMOvsdb *self) params); break; } + } g_return_if_fail(msg); _LOGT_call("send", call, msg); - cmd = json_dumps(msg, 0); + cmd = json_dumps(msg, 0); g_string_append(priv->output, cmd); free(cmd);