diff options
author | Thomas Haller <thaller@redhat.com> | 2022-11-18 18:00:33 +0100 |
---|---|---|
committer | Thomas Haller <thaller@redhat.com> | 2023-01-19 08:56:21 +0100 |
commit | 854f2cc1fcd63162029c8ae2907e560ced90bcab (patch) | |
tree | 9ecd0841dfe5b9bbd55eb83c855fedf174f8a94d /src | |
parent | c64053e6e66d9ea67721fe424eb852997056cbfb (diff) |
platform: better handle `ip route replace` for ignored routes
We don't cache certain routes, for example based on the protocol. This is
a performance optimization to ignore routes that we usually don't care
about.
Still, if the user does `ip route replace` with such a route, then we
need to pass it to nmp_cache_update_netlink_route(), so that we can
properly remove the replaced route.
Knowing which route was replaces might be impossible, as our cache does
not contain all routes. Likely all that nmp_cache_update_netlink_route()
can to is to set "resync_required" for NLM_F_REPLACE. But for that it
should see the object first.
This also means, if we ever write a BPF filter to filter out messages
that contain NLM_F_REPLACE, because that would lead to cache inconsistencies.
Diffstat (limited to 'src')
-rw-r--r-- | src/core/platform/nm-fake-platform.c | 1 | ||||
-rw-r--r-- | src/libnm-platform/nm-linux-platform.c | 34 | ||||
-rw-r--r-- | src/libnm-platform/nmp-object.c | 3 | ||||
-rw-r--r-- | src/libnm-platform/nmp-object.h | 1 |
4 files changed, 19 insertions, 20 deletions
diff --git a/src/core/platform/nm-fake-platform.c b/src/core/platform/nm-fake-platform.c index f85a02e159..c92d9aef56 100644 --- a/src/core/platform/nm-fake-platform.c +++ b/src/core/platform/nm-fake-platform.c @@ -1232,6 +1232,7 @@ ip_route_add(NMPlatform *platform, NMPNlmFlags flags, NMPObject *obj_stack) obj, FALSE, nlmsgflags, + TRUE, &obj_old, &obj_new, &obj_replace, diff --git a/src/libnm-platform/nm-linux-platform.c b/src/libnm-platform/nm-linux-platform.c index dd6dc54d01..aea08e54f7 100644 --- a/src/libnm-platform/nm-linux-platform.c +++ b/src/libnm-platform/nm-linux-platform.c @@ -5308,7 +5308,7 @@ ip_route_get_lock_flag(const NMPlatformIPRoute *route) } static gboolean -ip_route_ignored_protocol(const NMPlatformIPRoute *route) +ip_route_is_alive(const NMPlatformIPRoute *route) { guint8 prot; @@ -5320,12 +5320,18 @@ ip_route_ignored_protocol(const NMPlatformIPRoute *route) nm_assert(nmp_utils_ip_config_source_from_rtprot(prot) == route->rt_source); - /* We ignore all routes outside a certain subest of rtm_protocol. NetworkManager - * itself wouldn't configure those, so they are always configured by somebody - * external. We thus ignore them to avoid the overhead that processing them brings. - * For example, the BGP daemon "bird" might configure a huge number of RTPROT_BIRD routes. */ + if (prot > RTPROT_STATIC && !NM_IN_SET(prot, RTPROT_DHCP, RTPROT_RA)) { + /* We ignore certain rtm_protocol, because NetworkManager would only ever + * configure certain protocols. Other routes are not configured by NetworkManager + * and we don't track them in the platform cache. + * + * This is to help with the performance overhead of a huge number of + * routes, for example with the bird BGP software, that adds routes + * with RTPROT_BIRD protocol. */ + return FALSE; + } - return prot > RTPROT_STATIC && !NM_IN_SET(prot, RTPROT_DHCP, RTPROT_RA); + return TRUE; } /* Copied and modified from libnl3's build_route_msg() and rtnl_route_build_msg(). */ @@ -7829,6 +7835,7 @@ _rtnl_handle_msg(NMPlatform *platform, const struct nl_msg_lite *msg) gboolean resync_required = FALSE; gboolean only_dirty = FALSE; gboolean is_ipv6; + gboolean route_is_alive; /* IPv4 routes that are a response to RTM_GETROUTE must have * the cloned flag while IPv6 routes don't have to. */ @@ -7858,24 +7865,13 @@ _rtnl_handle_msg(NMPlatform *platform, const struct nl_msg_lite *msg) } } - if (ip_route_ignored_protocol(NMP_OBJECT_CAST_IP_ROUTE(obj))) { - /* We ignore certain rtm_protocol, because NetworkManager would only ever - * configure certain protocols. Other routes were not added by NetworkManager - * and we don't need to track them in the platform cache. - * - * This is to help with the performance overhead of a huge number of - * routes, for example with the bird BGP software, that adds routes - * with RTPROT_BIRD protocol. - * - * Even if this is a IPv6 multipath route, we abort (parse_nlmsg_iter). There - * is nothing for us to do. */ - return; - } + route_is_alive = ip_route_is_alive(NMP_OBJECT_CAST_IP_ROUTE(obj)); cache_op = nmp_cache_update_netlink_route(cache, obj, is_dump, msghdr->nlmsg_flags, + route_is_alive, &obj_old, &obj_new, &obj_replace, diff --git a/src/libnm-platform/nmp-object.c b/src/libnm-platform/nmp-object.c index ea1d12df0e..9bed736c7a 100644 --- a/src/libnm-platform/nmp-object.c +++ b/src/libnm-platform/nmp-object.c @@ -2959,6 +2959,7 @@ nmp_cache_update_netlink_route(NMPCache *cache, NMPObject *obj_hand_over, gboolean is_dump, guint16 nlmsgflags, + gboolean route_is_alive, const NMPObject **out_obj_old, const NMPObject **out_obj_new, const NMPObject **out_obj_replace, @@ -2990,7 +2991,7 @@ nmp_cache_update_netlink_route(NMPCache *cache, NM_SET_OUT(out_obj_old, nmp_object_ref(nm_dedup_multi_entry_get_obj(entry_old))); - is_alive = nmp_object_is_alive(obj_hand_over); + is_alive = route_is_alive && nmp_object_is_alive(obj_hand_over); if (!entry_old) { if (is_alive) { diff --git a/src/libnm-platform/nmp-object.h b/src/libnm-platform/nmp-object.h index 4999f43661..b23661378c 100644 --- a/src/libnm-platform/nmp-object.h +++ b/src/libnm-platform/nmp-object.h @@ -973,6 +973,7 @@ NMPCacheOpsType nmp_cache_update_netlink_route(NMPCache *cache, NMPObject *obj_hand_over, gboolean is_dump, guint16 nlmsgflags, + gboolean route_is_alive, const NMPObject **out_obj_old, const NMPObject **out_obj_new, const NMPObject **out_obj_replace, |