connectivity: fix portal detection with multiple devices (rh #1619873)

This commit is contained in:
Thomas Haller 2018-12-11 11:43:33 +01:00
parent ee9d7beef7
commit 162897002f
2 changed files with 192 additions and 1 deletions

View File

@ -0,0 +1,186 @@
From 3ce03f15d9c51f07f2811d0a387488d5d7a4ef3b Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Mon, 3 Dec 2018 10:27:06 +0100
Subject: [PATCH 1/2] libnm: add nm_connectivity_state_cmp() helper
(cherry picked from commit 487ee687d5bba82ee1054d74961afe122260811f)
(cherry picked from commit 51b7b10d3dc81d2837aba14ebf0e92f6ccd2db11)
(cherry picked from commit c155f776fd38eb8acfff3ac03d2e648fbb92930b)
---
src/nm-connectivity.h | 15 +++++++++++
src/tests/test-general.c | 58 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 73 insertions(+)
diff --git a/src/nm-connectivity.h b/src/nm-connectivity.h
index 178f27ad9..99333cede 100644
--- a/src/nm-connectivity.h
+++ b/src/nm-connectivity.h
@@ -24,6 +24,21 @@
#include "nm-dbus-interface.h"
+/*****************************************************************************/
+
+static inline int
+nm_connectivity_state_cmp (NMConnectivityState a, NMConnectivityState b)
+{
+ if (a == NM_CONNECTIVITY_PORTAL && b == NM_CONNECTIVITY_LIMITED)
+ return 1;
+ if (b == NM_CONNECTIVITY_PORTAL && a == NM_CONNECTIVITY_LIMITED)
+ return -1;
+ NM_CMP_DIRECT (a, b);
+ return 0;
+}
+
+/*****************************************************************************/
+
#define NM_CONNECTIVITY_ERROR ((NMConnectivityState) -1)
#define NM_CONNECTIVITY_FAKE ((NMConnectivityState) -2)
#define NM_CONNECTIVITY_CANCELLED ((NMConnectivityState) -3)
diff --git a/src/tests/test-general.c b/src/tests/test-general.c
index 4db05ac84..56e18571c 100644
--- a/src/tests/test-general.c
+++ b/src/tests/test-general.c
@@ -29,6 +29,8 @@
#include "NetworkManagerUtils.h"
#include "nm-core-internal.h"
+#include "nm-connectivity.h"
+
#include "nm-test-utils-core.h"
/* Reference implementation for nm_utils_ip6_address_clear_host_address.
@@ -1733,6 +1735,60 @@ test_nm_utils_exp10 (void)
/*****************************************************************************/
+static void
+test_connectivity_state_cmp (void)
+{
+ NMConnectivityState a;
+
+#define _cmp(a, b, cmp) \
+ G_STMT_START { \
+ const NMConnectivityState _a = (a); \
+ const NMConnectivityState _b = (b); \
+ const int _cmp = (cmp); \
+ \
+ g_assert (NM_IN_SET (_cmp, -1, 0, 1)); \
+ g_assert_cmpint (nm_connectivity_state_cmp (_a, _b), ==, _cmp); \
+ g_assert_cmpint (nm_connectivity_state_cmp (_b, _a), ==, -_cmp); \
+ } G_STMT_END
+
+ for (a = NM_CONNECTIVITY_UNKNOWN; a <= NM_CONNECTIVITY_FULL; a++)
+ _cmp (a, a, 0);
+
+ _cmp (NM_CONNECTIVITY_UNKNOWN, NM_CONNECTIVITY_UNKNOWN, 0);
+ _cmp (NM_CONNECTIVITY_UNKNOWN, NM_CONNECTIVITY_NONE, -1);
+ _cmp (NM_CONNECTIVITY_UNKNOWN, NM_CONNECTIVITY_LIMITED, -1);
+ _cmp (NM_CONNECTIVITY_UNKNOWN, NM_CONNECTIVITY_PORTAL, -1);
+ _cmp (NM_CONNECTIVITY_UNKNOWN, NM_CONNECTIVITY_FULL, -1);
+
+ _cmp (NM_CONNECTIVITY_NONE, NM_CONNECTIVITY_UNKNOWN, 1);
+ _cmp (NM_CONNECTIVITY_NONE, NM_CONNECTIVITY_NONE, 0);
+ _cmp (NM_CONNECTIVITY_NONE, NM_CONNECTIVITY_LIMITED, -1);
+ _cmp (NM_CONNECTIVITY_NONE, NM_CONNECTIVITY_PORTAL, -1);
+ _cmp (NM_CONNECTIVITY_NONE, NM_CONNECTIVITY_FULL, -1);
+
+ _cmp (NM_CONNECTIVITY_LIMITED, NM_CONNECTIVITY_UNKNOWN, 1);
+ _cmp (NM_CONNECTIVITY_LIMITED, NM_CONNECTIVITY_NONE, 1);
+ _cmp (NM_CONNECTIVITY_LIMITED, NM_CONNECTIVITY_LIMITED, 0);
+ _cmp (NM_CONNECTIVITY_LIMITED, NM_CONNECTIVITY_PORTAL, -1);
+ _cmp (NM_CONNECTIVITY_LIMITED, NM_CONNECTIVITY_FULL, -1);
+
+ _cmp (NM_CONNECTIVITY_PORTAL, NM_CONNECTIVITY_UNKNOWN, 1);
+ _cmp (NM_CONNECTIVITY_PORTAL, NM_CONNECTIVITY_NONE, 1);
+ _cmp (NM_CONNECTIVITY_PORTAL, NM_CONNECTIVITY_LIMITED, 1);
+ _cmp (NM_CONNECTIVITY_PORTAL, NM_CONNECTIVITY_PORTAL, 0);
+ _cmp (NM_CONNECTIVITY_PORTAL, NM_CONNECTIVITY_FULL, -1);
+
+ _cmp (NM_CONNECTIVITY_FULL, NM_CONNECTIVITY_UNKNOWN, 1);
+ _cmp (NM_CONNECTIVITY_FULL, NM_CONNECTIVITY_NONE, 1);
+ _cmp (NM_CONNECTIVITY_FULL, NM_CONNECTIVITY_LIMITED, 1);
+ _cmp (NM_CONNECTIVITY_FULL, NM_CONNECTIVITY_PORTAL, 1);
+ _cmp (NM_CONNECTIVITY_FULL, NM_CONNECTIVITY_FULL, 0);
+
+#undef _cmp
+}
+
+/*****************************************************************************/
+
NMTST_DEFINE ();
int
@@ -1777,6 +1833,8 @@ main (int argc, char **argv)
g_test_add_func ("/general/stable-id/parse", test_stable_id_parse);
g_test_add_func ("/general/stable-id/generated-complete", test_stable_id_generated_complete);
+ g_test_add_func ("/core/general/test_connectivity_state_cmp", test_connectivity_state_cmp);
+
return g_test_run ();
}
--
2.19.2
From b3b90479be4593c2d9e94b6805973c69ad9f9851 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Mon, 3 Dec 2018 10:31:51 +0100
Subject: [PATCH 2/2] connectivity: fix determining the global connectivity
state
Since we determine the connectivity state of each device individually,
the global connectivity state is an aggregate of all these states.
I am not sure about considering here devices that don't have the (best)
default route for their respective address family. But anyway.
When we aggregate the best connectivity, we chose the numerical largest
value. That is wrong, because PORTAL is numerically smaller than
LIMITED.
That means, if you have two devices, one with connectivity LIMITED and
one with connectivity PORTAL, then LIMITED wrongly wins.
Fixes: 6b7e9f9b225e81d365fd95901a88a7bc59c1eb39
https://bugzilla.redhat.com/show_bug.cgi?id=1619873
(cherry picked from commit ade753d06f4d8cac3a9c374fc1d9a409e2bce904)
(cherry picked from commit d1e98e334dd71b8fafa2512911b737adffddf569)
(cherry picked from commit 18103b00d8dd6dd99c9ff17d03cdf568a56d6720)
---
src/nm-manager.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/nm-manager.c b/src/nm-manager.c
index ad906169f..d5c849386 100644
--- a/src/nm-manager.c
+++ b/src/nm-manager.c
@@ -2741,18 +2741,21 @@ device_connectivity_changed (NMDevice *device,
best_state = nm_device_get_connectivity_state (device);
if (best_state < NM_CONNECTIVITY_FULL) {
+ /* FIXME: is this really correct, to considere devices that don't have
+ * (the best) default route for connectivity checking? */
c_list_for_each_entry (dev, &priv->devices_lst_head, devices_lst) {
state = nm_device_get_connectivity_state (dev);
- if (state <= best_state)
+ if (nm_connectivity_state_cmp (state, best_state) <= 0)
continue;
best_state = state;
- if (best_state >= NM_CONNECTIVITY_FULL) {
+ if (nm_connectivity_state_cmp (best_state, NM_CONNECTIVITY_FULL) >= 0) {
/* it doesn't get better than this. */
break;
}
}
}
nm_assert (best_state <= NM_CONNECTIVITY_FULL);
+ nm_assert (nm_connectivity_state_cmp (best_state, NM_CONNECTIVITY_FULL) <= 0);
if (best_state != priv->connectivity_state) {
priv->connectivity_state = best_state;
--
2.19.2

View File

@ -10,7 +10,7 @@
%global epoch_version 1
%global rpm_version 1.12.6
%global real_version 1.12.6
%global release_version 2
%global release_version 3
%global snapshot %{nil}
%global git_sha %{nil}
@ -112,6 +112,7 @@ Source4: 20-connectivity-redhat.conf
Patch1: 0001-fix-eth-generate-mac-address-mask-global-default.patch
Patch2: 0002-man-add-missing-connection-defaults.patch
Patch3: 0003-wifi-take-down-device-when-changing-mac.patch
Patch4: 0004-connectivity-check-rh1619873.patch
Requires(post): systemd
Requires(post): /usr/sbin/update-alternatives
@ -441,6 +442,7 @@ by nm-connection-editor and nm-applet in a non-graphical environment.
%patch1 -p1
%patch2 -p1
%patch3 -p1
%patch4 -p1
%build
%if %{with regen_docs}
@ -857,6 +859,9 @@ fi
%changelog
* Tue Dec 11 2018 Thomas Haller <thaller@redhat.com> - 1:1.12.6-3
- connectivity: fix portal detection with multiple devices (rh #1619873)
* Mon Dec 10 2018 Beniamino Galvani <bgalvani@redhat.com> - 1:1.12.6-2
- fix connection failure with some Wi-Fi adapters (rh #1656157)
- import other minor upstream fixes after 1.12.6