From ddb5e1d50e63e7c47cd143df3df77ac4180c34d2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 30 Jan 2023 21:52:13 +0100 Subject: [PATCH] platform/tests: disable check for sorted IPv4 routes by weak-id Due to a kernel bug, this assert can fail and I don't think it can be fixed in NetworkManager. Disable the check. See-also: https://bugzilla.redhat.com/show_bug.cgi?id=2165720 (cherry picked from commit 0347dc7ddc1c7f3b55d2273c11f93e8b58c71548) --- src/core/platform/tests/test-common.c | 23 ++++++++++++++++------- src/core/platform/tests/test-route.c | 12 +++++++++++- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/src/core/platform/tests/test-common.c b/src/core/platform/tests/test-common.c index 8064ea439..a1ac373fe 100644 --- a/src/core/platform/tests/test-common.c +++ b/src/core/platform/tests/test-common.c @@ -1329,14 +1329,23 @@ nmtstp_assert_platform(NMPlatform *platform, guint32 obj_type_flags) /* For IPv4, it also does not reliably always work. This may * be a bug we want to fix. For now, ignore the check. * - * This is probably caused by kernel bug - * https://bugzilla.redhat.com/show_bug.cgi?id=2162315 - * for which I think there is no workaround. + * a) Kernel can wrongly allow to configure the same route twice. + * That means, the same route is visible in `ip route` output, + * meaning, it would be added twice to the platform cache. + * At least due to that problem, may the weak-id not be properly sorted. + * See https://bugzilla.redhat.com/show_bug.cgi?id=2165720 which is + * a bug of kernel allowing to configure the exact same route twice. * - * Also, rhbz#2162315 means NMPlatform will merge two different - * routes together, if one of them were deleted, the RTM_DELROUTE - * message would wrongly delete single entry, leading to cache - * inconsistency. */ + * b) See https://bugzilla.redhat.com/show_bug.cgi?id=2162315 which is + * a bug where kernel does allow to configure single-hop routes that differ by + * their next-hop weight, but on the netlink API those routes look the same. + * + * Due to a) and b), the platform cache may contain only one instance + * of a route, which is visible more than once in `ip route` output. + * This merging of different routes causes problems, and it also means + * that the RTM_NEWROUTE events are wrongly interpreted and the weak-id + * is not properly sorted. + */ } else { /* Assert that also the original, not-sorted lists agree. */ _assert_platform_compare_arr(obj_type, diff --git a/src/core/platform/tests/test-route.c b/src/core/platform/tests/test-route.c index 569d371db..d52e6e61a 100644 --- a/src/core/platform/tests/test-route.c +++ b/src/core/platform/tests/test-route.c @@ -2312,7 +2312,17 @@ test_cache_consistency_routes(gconstpointer test_data) } extra_options[n_extra_options++] = "dev"; extra_options[n_extra_options++] = NMTSTP_ENV1_DEVICE_NAME[nmtst_get_rand_bool()]; - if (nmtst_get_rand_one_case_in(3)) { + if (IS_IPv4 && i == 0) { + /* For IPv4, there is a problem if we configure a route with + * only one next-hop and a weight. In that case, kernel allows + * to add duplicates (that only differ by weight), but on netlink + * the weight is not exposed, so the routes look identical and + * are deduplicated by the hash. + * See https://bugzilla.redhat.com/show_bug.cgi?id=2162315 + * + * This needs a kernel fix. Workaround that issue here, otherwise the test + * will randomly fail. */ + } else if (nmtst_get_rand_one_case_in(3)) { extra_options[n_extra_options++] = "weight"; extra_options[n_extra_options++] = "5"; }