diff --git a/src/nm-default-route-manager.c b/src/nm-default-route-manager.c index 66a94a05a4..1f6342ccce 100644 --- a/src/nm-default-route-manager.c +++ b/src/nm-default-route-manager.c @@ -304,7 +304,7 @@ _platform_route_sync_add (const VTableIP *vtable, NMDefaultRouteManager *self, g rt.plen = 0; rt.metric = entry->effective_metric; - success = nm_platform_ip4_route_add (priv->platform, NMP_NLM_FLAG_REPLACE, &rt); + success = (nm_platform_ip4_route_add (priv->platform, NMP_NLM_FLAG_REPLACE, &rt) == NM_PLATFORM_ERROR_SUCCESS); } else { NMPlatformIP6Route rt = entry->route.r6; @@ -312,7 +312,7 @@ _platform_route_sync_add (const VTableIP *vtable, NMDefaultRouteManager *self, g rt.plen = 0; rt.metric = entry->effective_metric; - success = nm_platform_ip6_route_add (priv->platform, NMP_NLM_FLAG_REPLACE, &rt); + success = (nm_platform_ip6_route_add (priv->platform, NMP_NLM_FLAG_REPLACE, &rt) == NM_PLATFORM_ERROR_SUCCESS); } if (!success) { _LOGW (vtable->vt->addr_family, "failed to add default route %s with effective metric %u", diff --git a/src/platform/nm-fake-platform.c b/src/platform/nm-fake-platform.c index d51acce791..40877b321a 100644 --- a/src/platform/nm-fake-platform.c +++ b/src/platform/nm-fake-platform.c @@ -1199,7 +1199,7 @@ ip_route_delete (NMPlatform *platform, const NMPObject *obj) return ipx_route_delete (platform, AF_UNSPEC, -1, obj); } -static gboolean +static NMPlatformError ip_route_add (NMPlatform *platform, NMPNlmFlags flags, int addr_family, @@ -1284,7 +1284,7 @@ ip_route_add (NMPlatform *platform, nm_log_warn (LOGD_PLATFORM, "Fake platform: failure adding ip6-route '%d: %s/%d %d': Network Unreachable", r->ifindex, nm_utils_inet6_ntop (&r6->network, NULL), r->plen, r->metric); } - return FALSE; + return NM_PLATFORM_ERROR_UNSPECIFIED; } } @@ -1346,7 +1346,7 @@ ip_route_add (NMPlatform *platform, } } - return TRUE; + return NM_PLATFORM_ERROR_SUCCESS; } /*****************************************************************************/ diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 6c4b34a03c..7c874df23f 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -265,6 +265,16 @@ static gboolean event_handler_read_netlink (NMPlatform *platform, gboolean wait_ /*****************************************************************************/ +static NMPlatformError +wait_for_nl_response_to_plerr (WaitForNlResponseResult seq_result) +{ + if (seq_result == WAIT_FOR_NL_RESPONSE_RESULT_RESPONSE_OK) + return NM_PLATFORM_ERROR_SUCCESS; + if (seq_result < 0) + return (NMPlatformError) seq_result; + return NM_PLATFORM_ERROR_NETLINK; +} + static const char * wait_for_nl_response_to_string (WaitForNlResponseResult seq_result, char *buf, gsize buf_size) { @@ -4223,14 +4233,12 @@ do_add_link_with_lookup (NMPlatform *platform, return !!obj; } -static gboolean +static NMPlatformError do_add_addrroute (NMPlatform *platform, const NMPObject *obj_id, struct nl_msg *nlmsg) { WaitForNlResponseResult seq_result = WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN; int nle; char s_buf[256]; - const NMPObject *obj; - NMPCache *cache = nm_platform_get_cache (platform); nm_assert (NM_IN_SET (NMP_OBJECT_GET_TYPE (obj_id), NMP_OBJECT_TYPE_IP4_ADDRESS, NMP_OBJECT_TYPE_IP6_ADDRESS, @@ -4244,7 +4252,7 @@ do_add_addrroute (NMPlatform *platform, const NMPObject *obj_id, struct nl_msg * NMP_OBJECT_GET_CLASS (obj_id)->obj_type_name, nmp_object_to_string (obj_id, NMP_OBJECT_TO_STRING_ID, NULL, 0), nl_geterror (nle), -nle); - return FALSE; + return NM_PLATFORM_ERROR_NETLINK; } delayed_action_handle_all (platform, FALSE); @@ -4253,30 +4261,26 @@ do_add_addrroute (NMPlatform *platform, const NMPObject *obj_id, struct nl_msg * _NMLOG (seq_result == WAIT_FOR_NL_RESPONSE_RESULT_RESPONSE_OK ? LOGL_DEBUG - : LOGL_ERR, + : LOGL_WARN, "do-add-%s[%s]: %s", NMP_OBJECT_GET_CLASS (obj_id)->obj_type_name, nmp_object_to_string (obj_id, NMP_OBJECT_TO_STRING_ID, NULL, 0), wait_for_nl_response_to_string (seq_result, s_buf, sizeof (s_buf))); - /* In rare cases, the object is not yet ready as we received the ACK from - * kernel. Need to refetch. - * - * We want to safe the expensive refetch, thus we look first into the cache - * whether the object exists. - * - * FIXME: if the object already existed previously, we might not notice a - * missing update. It's not clear how to fix that reliably without refechting - * all the time. */ - obj = nmp_cache_lookup_obj (cache, obj_id); - if (!obj) { - do_request_one_type (platform, NMP_OBJECT_GET_TYPE (obj_id)); - obj = nmp_cache_lookup_obj (cache, obj_id); + if (NMP_OBJECT_GET_TYPE (obj_id) == NMP_OBJECT_TYPE_IP6_ADDRESS) { + /* In rare cases, the object is not yet ready as we received the ACK from + * kernel. Need to refetch. + * + * We want to safe the expensive refetch, thus we look first into the cache + * whether the object exists. + * + * rh#1484434 */ + if (!nmp_cache_lookup_obj (nm_platform_get_cache (platform), + obj_id)) + do_request_one_type (platform, NMP_OBJECT_GET_TYPE (obj_id)); } - /* Adding is only successful, if kernel reported success *and* we have the - * expected object in cache afterwards. */ - return obj && seq_result == WAIT_FOR_NL_RESPONSE_RESULT_RESPONSE_OK; + return wait_for_nl_response_to_plerr (seq_result); } static gboolean @@ -5910,7 +5914,7 @@ ip4_address_add (NMPlatform *platform, label); nmp_object_stackinit_id_ip4_address (&obj_id, ifindex, addr, plen, peer_addr); - return do_add_addrroute (platform, &obj_id, nlmsg); + return do_add_addrroute (platform, &obj_id, nlmsg) == NM_PLATFORM_ERROR_SUCCESS; } static gboolean @@ -5940,7 +5944,7 @@ ip6_address_add (NMPlatform *platform, NULL); nmp_object_stackinit_id_ip6_address (&obj_id, ifindex, &addr); - return do_add_addrroute (platform, &obj_id, nlmsg); + return do_add_addrroute (platform, &obj_id, nlmsg) == NM_PLATFORM_ERROR_SUCCESS; } static gboolean @@ -5995,7 +5999,7 @@ ip6_address_delete (NMPlatform *platform, int ifindex, struct in6_addr addr, gui /*****************************************************************************/ -static gboolean +static NMPlatformError ip_route_add (NMPlatform *platform, NMPNlmFlags flags, int addr_family, @@ -6019,7 +6023,7 @@ ip_route_add (NMPlatform *platform, nlmsg = _nl_msg_new_route (RTM_NEWROUTE, flags, &obj); if (!nlmsg) - g_return_val_if_reached (FALSE); + g_return_val_if_reached (NM_PLATFORM_ERROR_BUG); return do_add_addrroute (platform, &obj, nlmsg); } diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index 211a0abc9f..ae409b207b 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -248,6 +248,7 @@ NM_UTILS_LOOKUP_STR_DEFINE (_nm_platform_error_to_string, NMPlatformError, NM_UTILS_LOOKUP_STR_ITEM (NM_PLATFORM_ERROR_NOT_SLAVE, "not-slave"), NM_UTILS_LOOKUP_STR_ITEM (NM_PLATFORM_ERROR_NO_FIRMWARE, "no-firmware"), NM_UTILS_LOOKUP_STR_ITEM (NM_PLATFORM_ERROR_OPNOTSUPP, "not-supported"), + NM_UTILS_LOOKUP_STR_ITEM (NM_PLATFORM_ERROR_NETLINK, "netlink"), NM_UTILS_LOOKUP_ITEM_IGNORE (_NM_PLATFORM_ERROR_MININT), ); @@ -3597,6 +3598,8 @@ nm_platform_ip_route_sync (NMPlatform *self, for (i_type = 0; i_type < 2; i_type++) { for (i = 0; i < routes->len; i++) { + NMPlatformError plerr; + conf_o = routes->pdata[i]; #define VTABLE_IS_DEVICE_ROUTE(vt, o) (vt->is_ip4 \ @@ -3621,9 +3624,10 @@ nm_platform_ip_route_sync (NMPlatform *self, continue; } - if (!nm_platform_ip_route_add (self, - NMP_NLM_FLAG_APPEND, - conf_o)) { + plerr = nm_platform_ip_route_add (self, + NMP_NLM_FLAG_APPEND, + conf_o); + if (plerr != NM_PLATFORM_ERROR_SUCCESS) { /* ignore error adding route. */ } } @@ -3710,7 +3714,7 @@ nm_platform_ip_route_normalize (int addr_family, } } -static gboolean +static NMPlatformError _ip_route_add (NMPlatform *self, NMPNlmFlags flags, int addr_family, @@ -3733,7 +3737,7 @@ _ip_route_add (NMPlatform *self, return klass->ip_route_add (self, flags, addr_family, route); } -gboolean +NMPlatformError nm_platform_ip_route_add (NMPlatform *self, NMPNlmFlags flags, const NMPObject *route) @@ -3754,7 +3758,7 @@ nm_platform_ip_route_add (NMPlatform *self, return _ip_route_add (self, flags, addr_family, NMP_OBJECT_CAST_IP_ROUTE (route)); } -gboolean +NMPlatformError nm_platform_ip4_route_add (NMPlatform *self, NMPNlmFlags flags, const NMPlatformIP4Route *route) @@ -3762,7 +3766,7 @@ nm_platform_ip4_route_add (NMPlatform *self, return _ip_route_add (self, flags, AF_INET, route); } -gboolean +NMPlatformError nm_platform_ip6_route_add (NMPlatform *self, NMPNlmFlags flags, const NMPlatformIP6Route *route) diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h index 2bd6509c73..dbd55b4dd0 100644 --- a/src/platform/nm-platform.h +++ b/src/platform/nm-platform.h @@ -158,6 +158,7 @@ typedef enum { /*< skip >*/ NM_PLATFORM_ERROR_NOT_SLAVE, NM_PLATFORM_ERROR_NO_FIRMWARE, NM_PLATFORM_ERROR_OPNOTSUPP, + NM_PLATFORM_ERROR_NETLINK, } NMPlatformError; #define NM_PLATFORM_LINK_OTHER_NETNS (-1) @@ -797,10 +798,10 @@ typedef struct { gboolean (*ip4_address_delete) (NMPlatform *, int ifindex, in_addr_t address, guint8 plen, in_addr_t peer_address); gboolean (*ip6_address_delete) (NMPlatform *, int ifindex, struct in6_addr address, guint8 plen); - gboolean (*ip_route_add) (NMPlatform *, - NMPNlmFlags flags, - int addr_family, - const NMPlatformIPRoute *route); + NMPlatformError (*ip_route_add) (NMPlatform *, + NMPNlmFlags flags, + int addr_family, + const NMPlatformIPRoute *route); gboolean (*ip_route_delete) (NMPlatform *, const NMPObject *obj); NMPlatformError (*ip_route_get) (NMPlatform *self, @@ -1122,11 +1123,11 @@ gboolean nm_platform_ip_address_flush (NMPlatform *self, void nm_platform_ip_route_normalize (int addr_family, NMPlatformIPRoute *route); -gboolean nm_platform_ip_route_add (NMPlatform *self, - NMPNlmFlags flags, - const NMPObject *route); -gboolean nm_platform_ip4_route_add (NMPlatform *self, NMPNlmFlags flags, const NMPlatformIP4Route *route); -gboolean nm_platform_ip6_route_add (NMPlatform *self, NMPNlmFlags flags, const NMPlatformIP6Route *route); +NMPlatformError nm_platform_ip_route_add (NMPlatform *self, + NMPNlmFlags flags, + const NMPObject *route); +NMPlatformError nm_platform_ip4_route_add (NMPlatform *self, NMPNlmFlags flags, const NMPlatformIP4Route *route); +NMPlatformError nm_platform_ip6_route_add (NMPlatform *self, NMPNlmFlags flags, const NMPlatformIP6Route *route); gboolean nm_platform_ip_route_delete (NMPlatform *self, const NMPObject *route); diff --git a/src/platform/tests/test-common.c b/src/platform/tests/test-common.c index 9692c09050..c0e6cb96de 100644 --- a/src/platform/tests/test-common.c +++ b/src/platform/tests/test-common.c @@ -1009,7 +1009,7 @@ void nmtstp_ip4_route_add (NMPlatform *platform, route.metric = metric; route.mss = mss; - g_assert (nm_platform_ip4_route_add (platform, NMP_NLM_FLAG_REPLACE, &route)); + g_assert_cmpint (nm_platform_ip4_route_add (platform, NMP_NLM_FLAG_REPLACE, &route), ==, NM_PLATFORM_ERROR_SUCCESS); } void nmtstp_ip6_route_add (NMPlatform *platform, @@ -1033,7 +1033,7 @@ void nmtstp_ip6_route_add (NMPlatform *platform, route.metric = metric; route.mss = mss; - g_assert (nm_platform_ip6_route_add (platform, NMP_NLM_FLAG_REPLACE, &route)); + g_assert_cmpint (nm_platform_ip6_route_add (platform, NMP_NLM_FLAG_REPLACE, &route), ==, NM_PLATFORM_ERROR_SUCCESS); } /*****************************************************************************/ diff --git a/src/platform/tests/test-route.c b/src/platform/tests/test-route.c index a8ba384492..f749d7514c 100644 --- a/src/platform/tests/test-route.c +++ b/src/platform/tests/test-route.c @@ -469,7 +469,7 @@ test_ip4_route_options (void) route.mtu = 1350; route.lock_cwnd = TRUE; - g_assert (nm_platform_ip4_route_add (NM_PLATFORM_GET, NMP_NLM_FLAG_REPLACE, &route)); + g_assert (nm_platform_ip4_route_add (NM_PLATFORM_GET, NMP_NLM_FLAG_REPLACE, &route) == NM_PLATFORM_ERROR_SUCCESS); /* Test route listing */ routes = nmtstp_ip4_route_get_all (NM_PLATFORM_GET, ifindex); @@ -597,7 +597,7 @@ test_ip6_route_options (gconstpointer test_data) _wait_for_ipv6_addr_non_tentative (NM_PLATFORM_GET, 400, IFINDEX, addr_n, addr_in6); for (i = 0; i < rts_n; i++) - g_assert (nm_platform_ip6_route_add (NM_PLATFORM_GET, NMP_NLM_FLAG_REPLACE, &rts_add[i])); + g_assert (nm_platform_ip6_route_add (NM_PLATFORM_GET, NMP_NLM_FLAG_REPLACE, &rts_add[i]) == NM_PLATFORM_ERROR_SUCCESS); routes = nmtstp_ip6_route_get_all (NM_PLATFORM_GET, IFINDEX); switch (TEST_IDX) { @@ -703,7 +703,7 @@ again_find_idx: order_idx[order_len++] = idx; r->ifindex = iface_data[idx].ifindex; - g_assert (nm_platform_ip4_route_add (platform, NMP_NLM_FLAG_APPEND, r)); + g_assert (nm_platform_ip4_route_add (platform, NMP_NLM_FLAG_APPEND, r) == NM_PLATFORM_ERROR_SUCCESS); } else { i = nmtst_get_rand_int () % order_len; idx = order_idx[i];