From e9b066ff924a8dc001b097e244f15058f368639c Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Wed, 1 May 2024 16:10:48 +0200 Subject: [PATCH 1/3] test: Follow symlinks when copying with rsync We have e.g. 25-default.link in test-network/ which becomes a broken symlink when installed so let's not copy the symlinks but follow them instead so they don't become broken. --- test/meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/meson.build b/test/meson.build index 7290b98f0dc..705762d5335 100644 --- a/test/meson.build +++ b/test/meson.build @@ -29,7 +29,7 @@ if install_tests # 'follow_symlinks' once we require Meson 1.3.0 or greater, see: # https://github.com/mesonbuild/meson/commit/0af126fec798d6dbb0d1ad52168cc1f3f1758acd if rsync.found() - rsync_r = rsync.full_path() + ' -rlpt --exclude .gitattributes -- "@0@" "${DESTDIR:-}@1@"' + rsync_r = rsync.full_path() + ' -rLpt --exclude .gitattributes -- "@0@" "${DESTDIR:-}@1@"' meson.add_install_script(sh, '-c', rsync_r.format(meson.current_source_dir() / subdir, testdata_dir)) else From f2adc1de8963030d4c9965e79572ac8db68a553c Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Wed, 1 May 2024 16:11:56 +0200 Subject: [PATCH 2/3] test-network: Make source directory optional --- test/test-network/systemd-networkd-tests.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 7924775e9d1..b6c16372706 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -365,12 +365,15 @@ def clear_networkd_conf_dropins(): rm_rf(networkd_conf_dropin_dir) def setup_systemd_udev_rules(): - if not build_dir: + if not build_dir and not source_dir: return mkdir_p(udev_rules_dir) for path in [build_dir, source_dir]: + if not path: + continue + path = os.path.join(path, "rules.d") print(f"Copying udev rules from {path} to {udev_rules_dir}") @@ -450,7 +453,7 @@ def create_service_dropin(service, command, additional_settings=None): create_unit_dropin(f'{service}.service', drop_in) def setup_system_units(): - if build_dir: + if build_dir or source_dir: mkdir_p('/run/systemd/system/') for unit in [ @@ -462,6 +465,9 @@ def setup_system_units(): 'systemd-udevd.service', ]: for path in [build_dir, source_dir]: + if not path: + continue + fullpath = os.path.join(os.path.join(path, "units"), unit) if os.path.exists(fullpath): print(f"Copying unit file from {fullpath} to /run/systemd/system/") @@ -7753,9 +7759,11 @@ if __name__ == '__main__': if ns.source_dir: source_dir = ns.source_dir - else: + assert os.path.exists(os.path.join(source_dir, "meson_options.txt")), f"{source_dir} doesn't appear to be a systemd source tree." + elif os.path.exists(os.path.normpath(os.path.join(os.path.dirname(os.path.abspath(__file__)), "../../meson_options.txt"))): source_dir = os.path.normpath(os.path.join(os.path.dirname(os.path.abspath(__file__)), "../../")) - assert os.path.exists(os.path.join(source_dir, "meson_options.txt")), f"{source_dir} doesn't appear to be a systemd source tree." + else: + source_dir = None use_valgrind = ns.use_valgrind enable_debug = ns.enable_debug From ee8f605ded4fea6b93aae018415efae877c26ed2 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Wed, 1 May 2024 14:41:41 +0200 Subject: [PATCH 3/3] network/tc: Avoid concurrent set modification in tclass_drop()/qdisc_drop() With the current algorithm, we can end up removing entries from the qdisc/tclass sets while having multiple open iterators over the sets at various positions which leads to assertion failures in the hashmap logic as it's only safe to remove the "current" entry. To avoid the problem, let's split up marking and dropping of tclasses and qdiscs. First, we recursively iterate tclasses/qdiscs and mark all that need to be removed. Next, we iterate once over tclasses and qdiscs and remove all marked entries. Fixes 632d321050f58fe1b5bed7cfe769d212377c0301 --- src/network/tc/qdisc.c | 56 ++++++++++++++++++++++++++++------------- src/network/tc/qdisc.h | 2 ++ src/network/tc/tclass.c | 55 +++++++++++++++++++++++++++------------- src/network/tc/tclass.h | 2 ++ 4 files changed, 79 insertions(+), 36 deletions(-) diff --git a/src/network/tc/qdisc.c b/src/network/tc/qdisc.c index 55ce16600a7..38dee2c0015 100644 --- a/src/network/tc/qdisc.c +++ b/src/network/tc/qdisc.c @@ -285,37 +285,57 @@ int link_find_qdisc(Link *link, uint32_t handle, const char *kind, QDisc **ret) return -ENOENT; } -QDisc* qdisc_drop(QDisc *qdisc) { +void qdisc_mark_recursive(QDisc *qdisc) { TClass *tclass; - Link *link; assert(qdisc); + assert(qdisc->link); - link = ASSERT_PTR(qdisc->link); + if (qdisc_is_marked(qdisc)) + return; - qdisc_mark(qdisc); /* To avoid stack overflow. */ - - /* also drop all child classes assigned to the qdisc. */ - SET_FOREACH(tclass, link->tclasses) { - if (tclass_is_marked(tclass)) - continue; + qdisc_mark(qdisc); + /* also mark all child classes assigned to the qdisc. */ + SET_FOREACH(tclass, qdisc->link->tclasses) { if (TC_H_MAJ(tclass->classid) != qdisc->handle) continue; - tclass_drop(tclass); + tclass_mark_recursive(tclass); } +} - qdisc_unmark(qdisc); - qdisc_enter_removed(qdisc); +void link_qdisc_drop_marked(Link *link) { + QDisc *qdisc; - if (qdisc->state == 0) { - log_qdisc_debug(qdisc, link, "Forgetting"); - qdisc = qdisc_free(qdisc); - } else - log_qdisc_debug(qdisc, link, "Removed"); + assert(link); - return qdisc; + SET_FOREACH(qdisc, link->qdiscs) { + if (!qdisc_is_marked(qdisc)) + continue; + + qdisc_unmark(qdisc); + qdisc_enter_removed(qdisc); + + if (qdisc->state == 0) { + log_qdisc_debug(qdisc, link, "Forgetting"); + qdisc_free(qdisc); + } else + log_qdisc_debug(qdisc, link, "Removed"); + } +} + +QDisc* qdisc_drop(QDisc *qdisc) { + assert(qdisc); + assert(qdisc->link); + + qdisc_mark_recursive(qdisc); + + /* link_qdisc_drop_marked() may invalidate qdisc, so run link_tclass_drop_marked() first. */ + link_tclass_drop_marked(qdisc->link); + link_qdisc_drop_marked(qdisc->link); + + return NULL; } static int qdisc_handler(sd_netlink *rtnl, sd_netlink_message *m, Request *req, Link *link, QDisc *qdisc) { diff --git a/src/network/tc/qdisc.h b/src/network/tc/qdisc.h index a62b9413eca..cbba1bef711 100644 --- a/src/network/tc/qdisc.h +++ b/src/network/tc/qdisc.h @@ -77,7 +77,9 @@ DEFINE_NETWORK_CONFIG_STATE_FUNCTIONS(QDisc, qdisc); QDisc* qdisc_free(QDisc *qdisc); int qdisc_new_static(QDiscKind kind, Network *network, const char *filename, unsigned section_line, QDisc **ret); +void qdisc_mark_recursive(QDisc *qdisc); QDisc* qdisc_drop(QDisc *qdisc); +void link_qdisc_drop_marked(Link *link); int link_find_qdisc(Link *link, uint32_t handle, const char *kind, QDisc **qdisc); diff --git a/src/network/tc/tclass.c b/src/network/tc/tclass.c index 63229ec6e8d..fcbe8cbcf46 100644 --- a/src/network/tc/tclass.c +++ b/src/network/tc/tclass.c @@ -252,37 +252,56 @@ static void log_tclass_debug(TClass *tclass, Link *link, const char *str) { strna(tclass_get_tca_kind(tclass))); } -TClass* tclass_drop(TClass *tclass) { +void tclass_mark_recursive(TClass *tclass) { QDisc *qdisc; - Link *link; assert(tclass); + assert(tclass->link); - link = ASSERT_PTR(tclass->link); + if (tclass_is_marked(tclass)) + return; - tclass_mark(tclass); /* To avoid stack overflow. */ - - /* Also drop all child qdiscs assigned to the class. */ - SET_FOREACH(qdisc, link->qdiscs) { - if (qdisc_is_marked(qdisc)) - continue; + tclass_mark(tclass); + /* Also mark all child qdiscs assigned to the class. */ + SET_FOREACH(qdisc, tclass->link->qdiscs) { if (qdisc->parent != tclass->classid) continue; - qdisc_drop(qdisc); + qdisc_mark_recursive(qdisc); } +} - tclass_unmark(tclass); - tclass_enter_removed(tclass); +void link_tclass_drop_marked(Link *link) { + TClass *tclass; - if (tclass->state == 0) { - log_tclass_debug(tclass, link, "Forgetting"); - tclass = tclass_free(tclass); - } else - log_tclass_debug(tclass, link, "Removed"); + assert(link); - return tclass; + SET_FOREACH(tclass, link->tclasses) { + if (!tclass_is_marked(tclass)) + continue; + + tclass_unmark(tclass); + tclass_enter_removed(tclass); + + if (tclass->state == 0) { + log_tclass_debug(tclass, link, "Forgetting"); + tclass_free(tclass); + } else + log_tclass_debug(tclass, link, "Removed"); + } +} + +TClass* tclass_drop(TClass *tclass) { + assert(tclass); + + tclass_mark_recursive(tclass); + + /* link_tclass_drop_marked() may invalidate tclass, so run link_qdisc_drop_marked() first. */ + link_qdisc_drop_marked(tclass->link); + link_tclass_drop_marked(tclass->link); + + return NULL; } static int tclass_handler(sd_netlink *rtnl, sd_netlink_message *m, Request *req, Link *link, TClass *tclass) { diff --git a/src/network/tc/tclass.h b/src/network/tc/tclass.h index e73e23c97f6..85df57d42c2 100644 --- a/src/network/tc/tclass.h +++ b/src/network/tc/tclass.h @@ -58,7 +58,9 @@ DEFINE_NETWORK_CONFIG_STATE_FUNCTIONS(TClass, tclass); TClass* tclass_free(TClass *tclass); int tclass_new_static(TClassKind kind, Network *network, const char *filename, unsigned section_line, TClass **ret); +void tclass_mark_recursive(TClass *tclass); TClass* tclass_drop(TClass *tclass); +void link_tclass_drop_marked(Link *link); int link_find_tclass(Link *link, uint32_t classid, TClass **ret);