summaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAgeFilesLines
* platform: support IPv6 mulitpath routes and fix cache inconsistencyth/platform-ip6-multipath-routesThomas Haller2022-02-153-106/+134
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add support for IPv6 multipath routes, by treating them as single-hop routes. Otherwise, we can easily end up with an inconsistent platform cache. Background: ----------- Routes are hard. We have NMPlatform which is a cache of netlink objects. That means, we have a hash table and we cache objects based on some identity (nmp_object_id_equal()). So those objects must have some immutable, indistinguishable properties that determine whether an object is the same or a different one. For routes and routing rules, this identifying property is basically a subset of the attributes (but not all!). That makes it very hard, because tomorrow kernel could add an attribute that becomes part of the identity, and NetworkManager wouldn't recognize it, resulting in cache inconsistency by wrongly thinking two different routes are one and the same. Anyway. The other point is that we rely on netlink events to maintain the cache. So when we receive a RTM_NEWROUTE we add the object to the cache, and delete it upon RTM_DELROUTE. When you do `ip route replace`, kernel might replace a (different!) route, but only send one RTM_NEWROUTE message. We handle that by somehow finding the route that was replaced/deleted. It's ugly. Did I say, that routes are hard? Also, for IPv4 routes, multipath attributes are just a part of the routes identity. That is, you add two different routes that only differ by their multipath list, and then kernel does as you would expect. NetworkManager does not support IPv4 multihop routes and just ignores them. Also, a multipath route can have next hops on different interfaces, which goes against our current assumption, that an NMPlatformIP4Route has an interface (or no interface, in case of blackhole routes). That makes it hard to meaningfully support IPv4 routes. But we probably don't have to, because we can just pretend that such routes don't exist and our cache stays consistent (at least, until somebody calls `ip route replace` *sigh*). Not so for IPv6. When you add (`ip route append`) an IPv6 route that is identical to an existing route -- except their multipath attribute -- then it behaves as if the existing route was modified and the result is the merged route with more next-hops. Note that in this case kernel will only send a RTM_NEWROUTE message with the full multipath list. If we would treat the multipath list as part of the route's identity, this would be as if kernel deleted one routes and created a different one (the merged one), but only sending one notification. That's a bit similar to what happens during `ip route replace`, but it would be nightmare to find out which route was thereby replaced. Likewise, when you delete a route, then kernel will "subtract" the next-hop and sent a RTM_DELROUTE notification only about the next-hop that was deleted. To handle that, you would have to find the full multihop route, and replace it with the remainder after the subtraction. NetworkManager so far ignored IPv6 routes with more than one next-hop, this means you can start with one single-hop route (that NetworkManger sees and has in the platform cache). Then you create a similar route (only differing by the next-hop). Kernel will merge the routes, but not notify NetworkManager that the single-hop route is not longer a single-hop route. This can easily cause a cache inconsistency and subtle bugs. For IPv6 we MUST handle multihop routes. Kernels behavior makes little sense, if you expect that routes have an immutable identity and want to get notifications about addition/removal. We can however make sense by it by pretending that all IPv6 routes are single-hop! With only the twist that a single RTM_NEWROUTE notification might notify about multiple routes at the same time. This is what the patch does. The Patch --------- Now one RTM_NEWROUTE message can contain multiple IPv6 routes (NMPObject). That would mean that nmp_object_new_from_nl() needs to return a list of objects. But it's not implemented that way. Instead, we still call nmp_object_new_from_nl(), and the parsing code can indicate that there is something more, indicating the caller to call nmp_object_new_from_nl() again in a loop to fetch more objects. In practice, I think all RTM_DELROUTE messages for IPv6 routes are single-hop. Still, we implement it to handle also multi-hop messages the same way. Note that we just parse the netlink message again from scratch. The alternative would be to parse the first object once, and then clone the object and only update the next-hop. That would be more efficient, but probably harder to understand/implement. https://bugzilla.redhat.com/show_bug.cgi?id=1837254#c20
* platform: fix parsing RTA_MULTIHOP netlink attribute to use no policyThomas Haller2022-02-151-2/+2
| | | | | | | | | | | To parse the RTA_MULTIHOP message, "policy" is not right (which is used to parse the overall message). Instead, we don't really have a special policy that we should use. This was not a severe issue, because the allocated buffer (with G_N_ELEMENTS(policy) elements) was larger than need be. And apparently, using the wrong policy also didn't cause us to reject important messages.
* po: remove ar.po translationsJavier Jardón2022-02-152-12839/+0
| | | | | | | | | | | | There is actually not translation output from this at the moment This, while is not generally a problem is triggering an issue with gettext, see https://lists.gnu.org/archive/html/bug-gettext/2019-05/msg00125.html While the fix is already in gettext upstream, I think is ok to remove this translation file for now as it doesn't produce any valid translation at the moment anyway https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1094#note_1258564
* gitlab-ci: disable CentOS 8 Linux containersThomas Haller2022-02-142-98/+5
| | | | | | | | | | | | | | | | | | CentOS 8 Linux is end of life. That wouldn't bother us, but when you start such a container $ podman run -ti --privileged quay.io/centos/centos:8.3.2011 then `dnf upgrade` will fail, because the mirror list returns nothing. To work around that, we need to adjust ci-templates ([1]). The work around might be to patch /etc/yum.repos.d when creating the container image ([2]). For now (or maybe indefinitely) disable these build targets. [1] https://gitlab.freedesktop.org/freedesktop/ci-templates/-/merge_requests/131 [2] https://stackoverflow.com/questions/70926799/centos-through-vm-no-urls-in-mirrorlist
* gitlab-ci: regenerate ci-templates's containersThomas Haller2022-02-142-6/+6
|
* examples/python: avoid Python2 "print" statementThomas Haller2022-02-143-22/+28
| | | | | Recent python-black (22.0) dropped support for Python 2 and thus fail for those files. Make the examples Python3 compatible.
* libnmc: fix typo in passwd-file exampleSigurd Rønningen Jenssen2022-02-141-1/+1
| | | | https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1096
* Merge branch 'ac/wip-empty-properties'Ana Cabral2022-02-114-74/+98
|\ | | | | https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1088
| * keyfile: do not write empty string list propertiesAna Cabral2022-02-112-70/+63
| | | | | | | | https://bugzilla.redhat.com/show_bug.cgi?id=2022623
| * keyfile: write ethernet group always on the top of the fileAna Cabral2022-02-113-4/+35
|/
* libnm: merge branch 'th/libnm-direct-strv-property-1'Thomas Haller2022-02-109-225/+353
|\ | | | | | | https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1082
| * libnm: rework strv properties of NMSetting as "direct" propertiesThomas Haller2022-02-103-160/+115
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Make use of direct strv property in some cases. It doesn't work for other cases yet, because they are implemented differently, and porting them is more effort and needs to be done one by one. The goal is to have a unified, standard implementation for our properties. One that requires a minimal amount of property-specific code. For strv properties, that is a bit more cumbersome, because usually there are multiple C accessor functions. Still, make an effort to have a "direct" strv property. What this also gives, is that we no longer need to clone the strv array for various operations. We know how to access the data, and can do it directly without g_object_get()/g_object_set().
| * libnm: add direct strv type for NMSetting and use it for "match.interface-name"Thomas Haller2022-02-106-70/+216
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | G_TYPE_STRV is the last property type in NMSetting that is implemented by directly accessing the GObect property. Note that we have lots of override, non-default implementations that still use GObject properties, but I am talking here about properties that don't have a special implementation and use a G_TYPE_STRV GObject property. Add a "direct" implementation also for strv arrays. The advantage is that we no longer call g_value_get() for various operations, which requires a deep-copy of the strv array. The other advantage is that we will get a unified approach for implementing strv properties. In particular strv arrays need a lot of code to implement, and most settings do it differently. By adding a general mechanism, this code (and behavior) can be unified. Showcase it on "match.interface-name".
| * glib-aux: add nm_strvarray_*() helpers for cmp/equal functionsThomas Haller2022-02-101-0/+27
|/
* core/tests: make test_machine_id_read() more robust against the test systemThomas Haller2022-02-101-4/+25
| | | | | | | | | | | | | test_machine_id_read() is a flawed unit test, as it reads the machine-id of the machine where it's running. That means the test depends on the test machine, which is obviously a problem. If you had no /etc/machine-id but a /var/lib/dbus/machine-id, then previously the test would fail. If the file exists, assume we are able to read a valid machine-id. On test systems that have a bogus /etc/machine-id or /var/lib/dbus/machine-id, the test would still fail. Just don't do that.
* core/trivial: add code comment to _set_hostname()Thomas Haller2022-02-101-1/+4
|
* NEWS: update for 1.36-rc2Lubomir Rintel2022-02-101-2/+9
|
* nm-l3cfg: fix check on timestamp for assuming probing is goodFernando Fernandez Mancera2022-02-101-3/+3
| | | | | | https://bugzilla.redhat.com/show_bug.cgi?id=2028751 Fixes: db0d84f13a (“l3cfg: fix handling "instance-reset" ACD event”)
* libnm,core: merge branch 'th/route-blackhole'Thomas Haller2022-02-1030-1362/+1939
|\ | | | | | | | | | | | | https://bugzilla.redhat.com/show_bug.cgi?id=1937823 https://bugzilla.redhat.com/show_bug.cgi?id=2013587 https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1083
| * libnm/doc: describe routing-rules in `man nm-settings-nmcli`Thomas Haller2022-02-093-0/+36
| |
| * libnm/doc: list route attributes in `man nm-settings-nmcli`Thomas Haller2022-02-092-3/+132
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | IPv4: routes A list of IPv4 destination addresses, prefix length, optional IPv4 next hop addresses, optional route metric, optional attribute. The valid syntax is: "ip[/prefix] [next-hop] [metric] [attribute=val]...[,ip[/prefix]...]". For example "192.0.2.0/24 10.1.1.1 77, 198.51.100.0/24". Various attributes are supported: • "cwnd" - an unsigned 32 bit integer. • "initcwnd" - an unsigned 32 bit integer. • "initrwnd" - an unsigned 32 bit integer. • "lock-cwnd" - a boolean value. • "lock-initcwnd" - a boolean value. • "lock-initrwnd" - a boolean value. • "lock-mtu" - a boolean value. • "lock-window" - a boolean value. • "mtu" - an unsigned 32 bit integer. • "onlink" - a boolean value. • "scope" - an unsigned 8 bit integer. IPv4 only. • "src" - an IPv4 address. • "table" - an unsigned 32 bit integer. The default depends on ipv4.route-table. • "tos" - an unsigned 8 bit integer. IPv4 only. • "type" - one of unicast, local, blackhole, unavailable, prohibit. The default is unicast. • "window" - an unsigned 32 bit integer. For details see also `man ip-route`. Format: a comma separated list of routes IPv6: routes A list of IPv6 destination addresses, prefix length, optional IPv6 next hop addresses, optional route metric, optional attribute. The valid syntax is: "ip[/prefix] [next-hop] [metric] [attribute=val]...[,ip[/prefix]...]". Various attributes are supported: • "cwnd" - an unsigned 32 bit integer. • "from" - an IPv6 address with optional prefix. IPv6 only. • "initcwnd" - an unsigned 32 bit integer. • "initrwnd" - an unsigned 32 bit integer. • "lock-cwnd" - a boolean value. • "lock-initcwnd" - a boolean value. • "lock-initrwnd" - a boolean value. • "lock-mtu" - a boolean value. • "lock-window" - a boolean value. • "mtu" - an unsigned 32 bit integer. • "onlink" - a boolean value. • "src" - an IPv6 address. • "table" - an unsigned 32 bit integer. The default depends on ipv6.route-table. • "type" - one of unicast, local, blackhole, unavailable, prohibit. The default is unicast. • "window" - an unsigned 32 bit integer. For details see also `man ip-route`. Format: a comma separated list of routes
| * tools: fix constructing XML by dropping broken pretty_xml()Thomas Haller2022-02-091-14/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | I don't understand the code, but it mangles the XML. There is no difference in the markup we have so far. But if you have nested XML (like for description-docbook tag) there are cases where this is wrong. There is also no need to prettify anything. If you want pretty-formatted XML, do it yourself, for example with $ tidy --indent yes --indent-spaces 4 --indent-attributes yes --wrap-attributes yes --input-xml yes --output-xml yes src/libnm-client-impl/nm-property-infos-nmcli.xml I think this was initially done, because we had the tool in perl, and when migrating, we wanted to generate the exactly same output. And it was the same output, and it was fine for the input we have. But with different input, it's wrong. Drop it now.
| * tools: re-use regular expression in process_data()Thomas Haller2022-02-091-21/+24
| | | | | | | | | | Yes, they get cached by the library already. Still, no need for doing this repeatedly.
| * libnm: allow configuring blackhole/unreachable/prohibit routesThomas Haller2022-02-091-1/+18
| |
| * core/l3cfg: let NML3Cfg handle nodev (blackhole) routesThomas Haller2022-02-092-15/+106
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Certain route types (blackhole, unreachable, prohibit) are not tied to an interface. They are thus global and we need to track them system wide (or better: per network namespace). That is done by NMPRouteManager. For the routing rules, it's NMDevice itself to track/untrack the rules. That is done for historical reasons, at the time, NML3Cfg did not exit. Now with NML3Cfg, it seems that also NML3Cfg should be the part that handles nodev routes. One reason is that we want to move IP functionality out of NMDevice. So callers (NMDevice) would just add blackhole routes to the NML3ConfigData and let NML3Cfg handle them. Still, to handle these routes is rather different from regular routes. Normally, NML3Cfg tracks an object state (ObjStateData) for each address/route, and it hooks into platform signals to update the os_plobj field. Those signals are dispatched by NMNetns and are only per-ifindex. Hence, NML3Cfg wouldn't be notified about those nodev routes. Consequently, there os_plobj could not be (efficiently) maintained and there is no ObjStateData for such routes. Instead, all that NML3Cfg does is have the routes in the NML3ConfigData and tell NMPRouteManager about them. Seems simple enough. The only question is when should NMPRouteManager sync? For now, we sync when the track/untracking brings any changes and during reapply. Which is probably fine.
| * core: handle blackhole/unreachable/prohibit route types in coreThomas Haller2022-02-092-2/+17
| | | | | | | | | | | | | | Specifically, in nm_utils_ip_route_attribute_to_platform() and in _l3_config_data_add_obj() handle such new route type. For the moment, they cannot be stored in a valid NMSettingIPConfig, but later this will be necessary.
| * core/l3cfg: rework generating list of routes in _l3_commit_one()Thomas Haller2022-02-091-31/+70
| | | | | | | | | | This will be required next, when we will have also routes without a device. Split the generation of the route list out.
| * platform: improve way to prune dirty route-manager entriesThomas Haller2022-02-093-7/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The general idea is that when we have entries tracked by the route-manager, that we can mark them all as dirty. Then, calling the "track" function will reset the dirty flag. Finally, there is a method to delete all dirty entries. As we can lookup an entry with O(1) (using dictionaries), we can sync the list of tracked objects with O(n). We just need to track all the ones we care about, and then delete those that were not touched (that is, are still dirty). Previously, we had to explicitly mark all entries as dirty. We can do better. Just let nmp_route_manager_untrack_all() mark the survivors as dirty right away. This way, we can save iterating the list once. It also makes sense because the only purpose of the dirty flag is to aid this prune mechanism with track/untrack-all. So, untrack-all can just help out, and leave the remaining entries dirty, so that the next track does the right thing.
| * platform: return boolean changed value from nmp_route_manager_track()Thomas Haller2022-02-092-42/+61
| |
| * platform: return self from nmp_route_manager_ref()Thomas Haller2022-02-092-4/+5
| | | | | | | | It's just more convenient.
| * platform: track linked list of objects in NMPRouteManager by typeThomas Haller2022-02-091-12/+39
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We now track up to three kinds of object types in NMPRouteManager. There is only one place, where we need to iterate over all objects of the same type (e.g. all ipv4-routes), and that is nmp_route_manager_sync(). Previously, we only had one GHashTable with all the object, and when iterating we had to skip over them after checking the type. That has some overhead, but OK. The ugliness with iterating over a GHashTable is that the order is non deterministic. We should have a defined order in which things happen. To achieve that, track three different CList, one for each object type. Also, I expect that to be slightly faster, as you only have to iterate over the list you care about.
| * platform: extend NMPRouteManager to work for routesThomas Haller2022-02-094-65/+126
| |
| * platform: use nm_pdirect_{hash,equal}() in "nmp-route-manager.c"Thomas Haller2022-02-091-19/+4
| | | | | | | | | | No need for a dedicated implementation just to compare two indirect pointers.
| * platform: use nm_g_slice_free() in "nmp-route-manager.c"Thomas Haller2022-02-091-4/+4
| |
| * platform: use NM_HASH_OBFUSCATE_PTR() in "nmp-route-manager.c"Thomas Haller2022-02-091-4/+2
| | | | | | | | | | | | | | | | | | NM_HASH_OBFUSCATE_PTR() is some snake-oil to not log raw pointer values. It obviously makes debugging harder. But we don't need to generate differently obfuscated pointer values. At least, let most users use the same obfuscation, so that the values are comparable.
| * platform: use __NMLOG_DEFAULT() in "nmp-route-manager.c"Thomas Haller2022-02-091-16/+1
| |
| * platform: rename internals in "nmp-route-manager.c"Thomas Haller2022-02-091-147/+149
| | | | | | | | We will not only track (routing) rules, but also routes. Rename.
| * platform: drop lazy initialization _rules_init() of NMPRouteManagerThomas Haller2022-02-091-38/+11
| | | | | | | | | | Let's just always allocate the hash tables. We will likely need them, and three hash tables are relatively cheap.
| * platform: rename "nmp-route-manager.h" to "nmp-rules-manager.h"Thomas Haller2022-02-098-8/+8
| |
| * platform: rename NMPRulesManager API to NMPRouteManagerThomas Haller2022-02-096-207/+208
| | | | | | | | | | | | | | | | | | | | | | | | | | Routes of type blackhole, unreachable, prohibit don't have an ifindex/device. They are thus in many ways similar to routing rules, as they are global. We need a mediator to keep track which routes to configure. This will be very similar to what NMPRulesManager already does for routing rules. Rename the API, so that it also can be used for routes. Renaming the file will be done next, so that git's rename detection doesn't get too confused.
| * platform: add support for blackhole,unreachable,prohibit route typeThomas Haller2022-02-093-8/+108
| |
| * platform: add nm_platform_route_type_is_nodev() helperThomas Haller2022-02-092-0/+32
| |
| * platform: don't treat ifindex zero special in nmp_lookup_init_object()Thomas Haller2022-02-093-8/+19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | So far, certain NMObject types could not have an ifindex of zero. Hence, nmp_lookup_init_object() took such an ifindex to mean lookup all objects of that type. Soon, we will support blackhole/unreachable/prohibit route types, which have their ifindex set to zero. It is still useful to lookup those routes types via nmp_lookup_init_object(). Change behaviour how to interpret the ifindex. Note that this also affects various callers of nmp_lookup_init_object(). If somebody was relying on the previous behavior, it would need fixing.
| * platform: don't check for valid ifindex in _vt_cmd_obj_is_alive_ipx_route()Thomas Haller2022-02-091-2/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | _vt_cmd_obj_is_alive_ipx_route() is called by nmp_object_is_alive(). Non-alive objects are not put into the cache. That certainly makes sense for RTM_F_CLONED routes, because they are generated ad-hoc during the `ip route get` request. Checking for the ifindex is not necessary. For one, some route types (blackhole, unreachable, prohibit) don't have an ifindex. Also, the purpose of _vt_cmd_obj_is_alive_ipx_route() is not to validate the object. Just don't create objects without an ifindex, if you think the route needs an ifindex. Checking here is not useful. We also don't check that other fields like rt_source are valid, so there is no need to do it for the ifindex either.
| * platform: don't print NUL gateway in nm_platform_ip[46]_route_to_string()Thomas Haller2022-02-091-32/+52
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently, for NMPlatformIP[46]Route always has a gateway, even if it's possibly set to 0.0.0.0/::. Not sure whether kernel has a further distinction between no-gateway and all-zero gateway. Anyway. For us, a gateway of 0.0.0.0/:: means the same as having no gateway. We cannot differentiate the two (nor do we need to). Don't print that in nm_platform_ip[46]_route_to_string(). Also, because we are going to add blackhole route types, which cannot have a next-hop. But we do this change for all routes types, because it makes sense in general (and also what `ip route show` prints).
| * core: use IS_IPv4 variable in nm_utils_ip_route_attribute_to_platform()Thomas Haller2022-02-091-5/+5
| | | | | | | | It's what we do at many other places. Consistency.
| * platform: rename variable "IS_IPv4" in platform codeThomas Haller2022-02-091-39/+44
| | | | | | | | | | | | | | | | | | | | | | | | The variable with this purpose is usually called "IS_IPv4". It's upper case, because usually this is a const variable, and because it reminds of the NM_IS_IPv4(addr_family) macro. That letter case is unusual, but it makes sense to me for the special purpose that this variable has. Anyway. The naming of this variable is a different point. Let's use the variable name that is consistent and widely used.
| * libnm: rework validating route attributes to avoid duplicate checkThomas Haller2022-02-091-53/+81
| | | | | | | | | | | | | | | | | | | | | | | | | | _nm_ip_route_attribute_validate_all() validates all attributes together. As such, it calls to nm_ip_route_attribute_validate(), which in turn validates one attribute at a time. Such full validation needs to check that (potentially conflicting) attributes are valid together. Hence, _nm_ip_route_attribute_validate_all() needs again peek into the attributes. Refactor the code, so that we can extract the pieces that we need and not need to parse them twice.
| * libnm: change NMVariantAttributeSpec.str_type to work for attributes of any typeThomas Haller2022-02-093-81/+91
| | | | | | | | | | | | | | | | | | | | | | | | | | First of all, all of NMVariantAttributeSpec is internal API. We only expose the typedef itself as public API, but not its fields nor their meaning. So we can change things. Change "str_type" to "type_detail", so that it can work for any kind of attribute, not only for strings. Usually, we want to avoid special cases and treat all attributes the same, based on their GVariant type. But sometimes, it is necessary to do something special with an attribute. This is what the "type_detail" encodes, but it's not only relevant for strings.
| * libnm: avoid parsing IP addresses twice in NMIPAddress/NMIPRoute APIThomas Haller2022-02-091-111/+110
| | | | | | | | | | | | | | Usually the normalization (canonicalize) and validation of the IP address string both requires to parse the string. As we always do validation first, we can use the parsed address and don't need to parse it a second time.
View Lorry Controller Status