Commit graph

2148 commits

Author SHA1 Message Date
Francesco Giudici
b6d2ad3312 device: enable DHCPv6 retries on lease renewal failure
https://bugzilla.gnome.org/show_bug.cgi?id=792745
(cherry picked from commit 1289450146)
2018-02-20 18:45:26 +01:00
Francesco Giudici
56353bfb82 device: never stop trying renewing the lease
Always reschedule a lease renewal attempt: just clear the scheduled
renewal if the connection is really deactivated.

(cherry picked from commit 1a20ff86d5)
2018-02-20 18:45:09 +01:00
Francesco Giudici
2d98ce9018 device: always consider both ip families when deciding to fail
Example: when dhcpv4 lease renewal fails, if ipv4.may-fail was "yes",
check also if we have a successful ipv6 conf: if not fail.
Previously we just ignored the other ip family status.

(cherry picked from commit da0fee4d9f)
2018-02-20 18:44:55 +01:00
Beniamino Galvani
258f4fc769 ppp: don't start IPv6 configuration on the device
If IPV6CP terminates before IPCP, pppd enters the RUNNING phase and we
start IP configuration without having an IP interface set, which
triggers assertions. Instead, reimplement stage3_ip6_config_start to
be a no-op. Note that IPv6 configuration on PPP devices has never been
supported by NM.

This is a simpler version of upstream commit dd98ada33f ("ppp:
introduce SetIfindex pppd plugin D-Bus method") that doesn't require
changing the internal plugin API.

https://bugzilla.redhat.com/show_bug.cgi?id=1515829
2018-02-08 09:49:26 +01:00
Thomas Haller
33cdfd8e0c device: gracefully handle unmanaged device during _device_activate()
(cherry picked from commit bbaa603a72)
2018-02-07 12:56:06 +01:00
Thomas Haller
26121eff14 device: don't return value from _device_activate()
It was only used at one place for an assertion. And it's not clear that the
assertion always holds.

(cherry picked from commit 9c094f93fb)
2018-02-07 12:54:53 +01:00
Thomas Haller
c7b1d4a2d3 device: clear priv->queued_act_request before setting state
Setting the state of NMActiveConnection results in invoking callbacks
in NMManager. Hence, it might be far-reaching. Clear
priv->queued_act_request before invoking the callbacks.

(cherry picked from commit ecf3677e57)
2018-02-07 12:54:53 +01:00
Thomas Haller
1be09bfbe3 device: minor cleanup unqueuing queued_act_request
Use gs_unref_object and g_steal_pointer() to move ownership around.

(cherry picked from commit edc4dd5167)
2018-02-07 12:54:53 +01:00
Thomas Haller
ff380c37bb core: transit to DISCONNECTING state for NMActiveConnection
Don't just directly switch to DISCONNECTED state. If we are ACTIVATING
or ACTIVATED, first transition to DISCONNECTING state.

(cherry picked from commit 6d623825f6)
2018-02-07 12:54:53 +01:00
Thomas Haller
5159c34ea8 ovs: fix compiler error for passing NMDevice pointer to NM_DEVICE_OVS_INTERFACE_GET_PRIVATE()
NM_DEVICE_OVS_INTERFACE_GET_PRIVATE() is implemented via the _NM_GET_PRIVATE()
macro. This macro uses C11's _Generic() to provide additional compiler checks
when casting from an incompatible pointer type.

As such,

  NMDevice *device = ...;
  NMDeviceOvsInterfacePrivate *priv;

  priv = NM_DEVICE_OVS_INTERFACE_GET_PRIVATE (device);

causes a compilation error:

    error: ‘_Generic’ selector of type ‘NMDevice * {aka struct _NMDevice *}’ is not compatible with any association

One workaround would be to cast the pointer first:

  priv = NM_DEVICE_OVS_INTERFACE_GET_PRIVATE ((NMDeviceOvsInterface *) device);

A better fix is to mark NMDevice as a compatible pointer in _NM_GET_PRIVATE(),
which this patch does.

Previously, this went unnoticed, because due to bug "a43bf3388 build: fix configure
check for CC support of _Generic() and __auto_type", we failed to detect support
for _Generic() when compiling with -Werror. That essentially disables this check,
and NM_DEVICE_OVS_INTERFACE_GET_PRIVATE() would do a direct cast.

A workaround for this build failure might be to build with -Werror, which accidentally
results in not using _Generic().

https://bugzilla.gnome.org/show_bug.cgi?id=793183

Fixes: 8ad310f8e3
(cherry picked from commit 782578122c)
2018-02-05 14:04:06 +01:00
Lubomir Rintel
60eb596b0d ovs-interface: avoid starting ip[46] configuration more than once
OvsInterface can postpone the stage3_ip[46]_config until the link
actually appears. It ought to restart the stage only when the link
appears, not upon further changes to it (which would trip an assertion
when starting the DHCP client while one already exists).

https://bugzilla.redhat.com/show_bug.cgi?id=1540063
(cherry picked from commit 8ad310f8e3)
2018-02-05 10:58:33 +01:00
Beniamino Galvani
a169247b7d device: skip IP configuration phase for external devices
We already avoid committing the IP configuration for external devices
(see commit 60334a2893). However, we still start DHCP/IPv6-autoconf
and, especially, we change sysctl values of the device.

To be sure that no action is taken on the device, return early from
the IP configuration phase, as in the method=disabled/ignore case.

https://bugzilla.redhat.com/show_bug.cgi?id=1530288
(cherry picked from commit 22f32a16f5)
2018-01-19 14:14:30 +01:00
Beniamino Galvani
3c60d63540 device: increase carrier wait time to 6 seconds
Some NICs need longer to establish the link, increase the timeout from
5 to 6 seconds.

https://bugzilla.redhat.com/show_bug.cgi?id=1520826
(cherry picked from commit 156344b8be)
2018-01-18 15:29:24 +01:00
Beniamino Galvani
207eb3266f all: add more meaningful error code for unsupported IP method
Add a new device state reason code for unsupported IP method. It is
returned, for example, when users select manual IP configuration for
WWAN connections:

 # nmcli connection mod Gsm ipv4.method manual ipv4.address 1.2.3.4/32
 # nmcli connection up Gsm
 Error: Connection activation failed: The selected IP method is not
 supported

compared to the old:

 Error: Connection activation failed: IP configuration could not be
 reserved (no available address, timeout, etc.)

Note that we could instead fail the connection validation if the
method is not supported by the connection type, but adding such
limitation now could make existing connections invalid.

https://bugzilla.redhat.com/show_bug.cgi?id=1459529
(cherry picked from commit aa820e9386)
2017-12-21 10:07:12 +01:00
Beniamino Galvani
8a570a41cf device: add a new state-reason for DAD failures
(cherry picked from commit 12a49cbdc7)
2017-12-21 10:07:07 +01:00
Beniamino Galvani
4ca7e3d0cf wwan: clear idle source id when the callback runs
Fixes: f0996d0eb8
(cherry picked from commit 5d372fd30e)
2017-12-21 09:45:01 +01:00
Beniamino Galvani
d9512bc807 wwan: add default route even if modem didn't return a gateway
If the modem didn't return a gateway, add a device route.

Fixes: 5c299454b4
(cherry picked from commit ec32edb21f)
2017-12-21 09:45:00 +01:00
Beniamino Galvani
f4dc5bd782 wwan: fix checks on IP configuration
Don't call nm_utils_parse_inaddr_bin() if the string returned by
mm_bearer_ip_config_get_address() and mm_bearer_ip_config_get_gateway()
is NULL, as the function requires a valid pointer. Throw an error if the
address is NULL, but allow an empty gateway.

Fixes: 7837afe87f
(cherry picked from commit 8ddc6caf98)
2017-12-21 09:44:59 +01:00
Thomas Haller
bd2d71754b device: generate unique default route-metrics per interface
In the past we had NMDefaultRouteManager which would coordinate adding
the default-route with identical metrics. That especially happened, when
activating two devices of the same type, without explicitly specifying
ipv4.route-metric. For example, with ethernet devices, the routes on
both interfaces would get a metric of 100.

Coordinating routes was especially necessary, because we added
routes with NLM_F_EXCL flag, akin to `ip route replace`. We not
only had to avoid that activating two devices in NetworkManager would
result in a fight over the default-route, but more importently
to preserve externally added default-routes on unmanaged interfaces.

NMDefaultRouteManager would ensure that in case of duplicate
metrics, that the device that activated first would keep the
best default-route. It would do so by bumping the metric
of the second device to find a unused metric. The bumping itself
was not very important -- MDefaultRouteManager could also just not
configure any default-routes that show up as second, the result
would be quite similar. More important was to keep the best
default-route on the first activating device until the device
deactivates or a device activates that really has a better
default-route..

Likewise, NMRouteManager would globally manage non-default-routes.
It would not do any bumping of metrics, but it would also ensure that the routes
of the device that activates first are not overwritten by a device activating
later.

However, the `ip route replace` approach has downsides, especially
that it messes with routes on other interfaces, interfaces that are
possibly not managed by NetworkManager. Another downside is, that
binding a socket to an interface might not result in correct
routes, because the route might just not be there (in case of
NMRouteManager, which wouldn't configure duplicate routes by bumping
their metric).

Since commit 77ec302714 we would no longer
use NLM_F_EXCL, but add routes akin to `ip route append`. When
activating for example two ethernet devices with no explict route
metric configuration, there are two routes like

   default via 10.16.122.254 dev eth0 proto dhcp metric 100
   default via 192.168.100.1 dev eth1 proto dhcp metric 100

This does not only affect default routes. In case of a multi-homing
setup you'd get

  192.168.100.0/24 dev eth0 proto kernel scope link src 192.168.100.1 metric 100
  192.168.100.0/24 dev eth1 proto kernel scope link src 192.168.100.1 metric 100

but it's visible the most for default-routes.

Note that we would append the routes that are activated later, as the order
of `ip route show` confirms. One might hence expect, that kernel selects
a route based on the order in the routing tables. However, that isn't
the case, and activating the second interface will non-deterministically
re-route traffic via the new interface. That will interfere badly with
with NAT, stateful firewalls, and existing connections (like TCP).

The solution is to have NMManager keep a global index of the default route-metrics
currently in use. So, instead of determining the default-route metric based solely
on the device-type, we now in addition generate default metrics that do not
overlap. For example, if you activate eth0 first, it gets route-metric 100,
and if you then activate eth1, it gets 101. Note that if you deactivate
and re-activate eth0, then it will get route-metric 102, because the
best route should stick on eth1 (which reserves the range 100 to 101).

Note that when a connection explititly selects a particular metric, then that
choice is honored (contrary to NMDefaultRouteManager which was more concerned
with avoiding conflicts, then keeping the exact metric).

https://bugzilla.redhat.com/show_bug.cgi?id=1505893
(cherry picked from commit 6a32c64d8f)
2017-12-15 11:44:52 +01:00
Thomas Haller
7b89933406 core: cache device state in NMConfig and load all at once
NMManager will need to know the state of all device at once.
Hence, load it once and cache it in NMConfig.

Note that this wastes a bit of memory in the order of
O(number-of-interfaces). But each device state entry is
rather small, and we always consume memory in the order
of O(number-of-interfaces).

(cherry picked from commit ea08df925f)
2017-12-15 11:44:52 +01:00
Thomas Haller
ea78f156f2 device: expose nm_device_get_route_metric_default()
(cherry picked from commit 989b5fabaa)
2017-12-15 11:44:52 +01:00
Francesco Giudici
2638d53ca8 devices/test: give more time to dad checking in test-arping
# random seed: R02Sc708af827453d4ace33cd27ffd3d7f0b
  1..2
  # Start of arping tests
  **
  NetworkManager:ERROR:src/devices/tests/test-arping.c:95:test_arping_common: assertion failed (nm_arping_manager_check_address (manager, info->addresses[i]) == info->expected_result[i]): (1 == 0)
  ok 1 /arping/1
  PASS: src/devices/tests/test-arping 1 /arping/1
  ./tools/run-nm-test.sh: line 193:  2836 Aborted                 "${NMTST_DBUS_RUN_SESSION[@]}" "$TEST" "$@"
  # NetworkManager:ERROR:src/devices/tests/test-arping.c:95:test_arping_common: assertion failed (nm_arping_manager_check_address (manager, info->addresses[i]) == info->expected_result[i]): (1 == 0)
  ERROR: src/devices/tests/test-arping - too few tests run (expected 2, got 1)
  ERROR: src/devices/tests/test-arping - exited with status 134 (terminated by signal 6?)

(cherry picked from commit 5c6a382d4d)
2017-12-13 10:27:43 +01:00
Lubomir Rintel
35e86a0cef device: ensure simple action sdata is a NUL-terminated bytestring
(cherry picked from commit 9639a176ff)
2017-12-11 19:53:09 +01:00
Lubomir Rintel
626bf76972 device: set traffic filters when device comes up
(cherry picked from commit 8bffb2c750)
2017-12-11 19:53:09 +01:00
Lubomir Rintel
97eeadb990 platform: add support for traffic filters
(cherry picked from commit b0fd3ecbaf)
2017-12-11 19:53:09 +01:00
Lubomir Rintel
f8da7febbc device: set qdiscs when device comes up
(cherry picked from commit e4bdb21909)
2017-12-11 19:34:31 +01:00
Lubomir Rintel
f7df4f0cde platform/trivial: s/ADDRROUTE/OBJECT/ for the cache lookup
It's going to be useful for other objects that have a type (of course)
and an ifindex.

(cherry picked from commit 93ac0e455b)
2017-12-11 18:56:41 +01:00
Thomas Haller
ab24d5356c settings: remove accessor functions to connection flags
The accessor functions just look whether a certain flag is set. As these
functions have a different name then the flags, this is more confusing
then helpful. For example, if you want to know where the NM_GENERATED
flag matters, you had to know to grep for nm_settings_connection_get_nm_generated()
in addition to NM_SETTINGS_CONNECTION_FLAGS_NM_GENERATED.

The accessor function hid that the property was implemented as
a connection flag. For example, it was not immediately obvious
that nm_settings_connection_get_nm_generated() is the same
as having the NM_SETTINGS_CONNECTION_FLAGS_NM_GENERATED flag
set.

Drop them.

(cherry picked from commit 545e3111c8)
2017-12-06 09:35:43 +01:00
Beniamino Galvani
f3b08bf114 all: replace 'inital' with 'initial'
sed -i -e 's/inital/initial/g' $(git grep -l inital)

(cherry picked from commit d74e1bef36)
2017-12-01 00:02:29 +01:00
Thomas Haller
bf7661189e c-list: re-import latest version of c-list.h from upstream
Most notably, it renames
  c_list_unlink_init() -> c_list_unlink()
  c_list_unlink() -> c_list_unlink_stale()

  $ sed -e 's/\<c_list_unlink\>/c_list_unlink_old/g' \
        -e 's/\<c_list_unlink_init\>/c_list_unlink/g' \
        -e 's/\<c_list_unlink_old\>/c_list_unlink_stale/g' \
        $(git grep -l c_list_unlink -- ':(exclude)shared/nm-utils/c-list.h') \
        -i

(cherry picked from commit b6efac9ec2)
2017-11-28 12:04:15 +01:00
Thomas Haller
27e38c77dd device: use same values for carrier-wait and carrier-defer time
Waiting for carrier on startup is probably the same times that carrier
needs, e.g. on an MTU change. Use the same timing.

Note, that as carrier-wait-timeout is no configurable, this
configuration applies to both -- as the manual page already
claims to do.

(cherry picked from commit 245590bacd)
2017-11-28 10:56:22 +01:00
Thomas Haller
d8b786570c device: extend carrier lost defer time after MTU change
Commit a17195b844 already tried to fix the
bug, but it did so wrongly. We must not increase the carrier-wait-time,
but the carrier-defer-time.

Keep the increase of carrier-wait-time too, and increase both timeouts
after a MTU change.

https://bugzilla.redhat.com/show_bug.cgi?id=1487702
https://bugzilla.gnome.org/show_bug.cgi?id=784854
(cherry picked from commit e4099f6764)
2017-11-28 10:56:22 +01:00
Thomas Haller
d67eeec5ef device: make carrier-wait-timeout configurable per device
As this depends on the particular host configuration, it's hard to find
a default that suits everybody. At least make it configurable per-device.

https://bugzilla.redhat.com/show_bug.cgi?id=1483343
https://bugzilla.redhat.com/show_bug.cgi?id=1515027
(cherry picked from commit b595a80977)
2017-11-28 10:56:22 +01:00
Thomas Haller
f988bbe613 core: merge IPv4 and IPv6 versions of nm_active_connection_get_default()
(cherry picked from commit 10a46c5ae2)
2017-11-27 16:00:52 +01:00
Thomas Haller
4b8afb752d core: merge nm_settings_get_connections_sorted() with nm_settings_get_connections_clone()
(cherry picked from commit 51531c9539)
2017-11-27 15:59:58 +01:00
Francesco Giudici
9c634e13c3 device: update device mtu from ip interface when required
If the tracked device is a control device only (has no network interface)
like in the case of a cdc-wdm device, get the mtu from the ip interface
(the exposed wwan network interface in this case).

https://bugzilla.redhat.com/show_bug.cgi?id=1460217
(cherry picked from commit efed5254cd)
2017-11-24 17:38:14 +01:00
Thomas Haller
a6b388d7d5 device: only set ip_forward sysctl if necessary
/proc/sys might be read-only but we want to set it for
enabling shared mode.

Check first if the sysctl already has the expected value,
and if so, do nothing.

https://bugzilla.gnome.org/show_bug.cgi?id=790726
(cherry picked from commit d841930d67)
2017-11-24 17:13:45 +01:00
Thomas Haller
26ca80ebf5 device: return and log failure reason for start_sharing()
Also downgrade a few intermediate error logging messages
for failures that happen while start_sharing(). A debug
message is enough in this case, because we propagate now
the error to the caller, which logs a warning anyway.

(cherry picked from commit 3369a2c0b0)
2017-11-24 17:13:45 +01:00
Thomas Haller
26905105bf core: refactor NMActRequestGetSecretsCallId typedef not to be a pointer to struct
Typedefs to structs are fine, but a typedef for a pointer seems confusing to
me. Let's avoid it.

(cherry picked from commit e5e291b65f)
2017-11-24 17:05:03 +01:00
Beniamino Galvani
de66655f0a device: check captured IPv6 configuration in check_and_add_ipv6ll_addr()
check_and_add_ipv6ll_addr() checks whether a link-local address is
already present in priv->ip6_config and if so, it returns with no
action.

priv->ip6_config is only updated after a merge-and-apply or (in an
idle source) when the external configuration changes and so there is
no guarantee that the addresses there are up-to-date.

priv->ext_ip6_config_captured should be checked instead, because it is
updated from platform right before starting the generation of a
link-local address. Note that also linklocal6_start() already checks
the captured external configuration rather than priv->ip6_config.

https://bugzilla.redhat.com/show_bug.cgi?id=1500350
(cherry picked from commit a7c97d58db)
2017-11-20 10:53:41 +01:00
Beniamino Galvani
a2e0c92901 device: don't touch external devices
If a device is 'external' (which means that NM generated an in-memory
connection to only to track the device state) we should not change its
IP configuration.

https://bugzilla.redhat.com/show_bug.cgi?id=1512316
(cherry picked from commit 60334a2893)
2017-11-17 18:22:37 +01:00
Beniamino Galvani
c516dca0d5 device: start managing external devices on reapply
In the next commit we will modify ipX_config_merge_and_apply to never
touch external devices. When a "reapply" call is issued on an external
device we are no longer simply tracking its state but we are actively
managing it and so its sys-iface-state must be promoted to managed.

https://bugzilla.redhat.com/show_bug.cgi?id=1512316
(cherry picked from commit 9e41ed4461)
2017-11-17 18:22:35 +01:00
Thomas Haller
fc2894508e all: use nm_close() instead of close()
(cherry picked from commit 5b29c2e5b9)
2017-11-14 15:17:02 +01:00
Beniamino Galvani
8db2120979 core: allow slaves to autoactivate when master is available
When a master connection is deactivated by user, we set the
autoconnect-blocked reason 'user-request' for the connection and we
propagate the same reason to slaves. Doing so prevents the
autoactivation of slaves when the master is manually activated again,
because the only way to override the 'user-request' blocked reason is
through manual activation of slaves.

Instead what should happen is that the manual deactivation of a master
marks slaves as blocked for failed dependencies. When the master
becomes available again, slaves can autoactivate if the profile allows
it.

https://bugzilla.redhat.com/show_bug.cgi?id=1437598
(cherry picked from commit b31118cfd2)
2017-11-13 20:34:23 +01:00
Beniamino Galvani
200e714885 device: make nm_device_state_reason_to_str() public
(cherry picked from commit dcd7760eae)
2017-11-13 20:34:22 +01:00
Thomas Haller
5282469098 shared: propagate constness in _NM_GET_PRIVATE_PTR()
The _NM_GET_PRIVATE() macro already preserved and propagated
the constness of @self to the resulting private pointer.

_NM_GET_PRIVATE_PTR() didn't do that. Extend the macro,
to make that possible.

(cherry picked from commit bdfdabea51)
2017-11-13 14:43:07 +01:00
Thomas Haller
384d557887 core: merge IPv4 and IPv6 implementations in NMDnsManager
(cherry picked from commit 54bcbb85d3)
2017-11-13 14:40:31 +01:00
Thomas Haller
dd02e4bfce shared: make NM_CONSTCAST() macro variadic
We need to pass more alias-types. Instead of having numbered
versions, use variadic number of macro arguments.

Also, fix build failure with old compiler:

  In file included from src/nm-ip6-config.c:24:
  ./src/nm-ip6-config.h:44:29: error: controlling expression type 'typeof (ipconf_iter->current->obj)' (aka 'const void *const') not compatible with any generic association type
                  *out_address = has_next ? NMP_OBJECT_CAST_IP6_ADDRESS (ipconf_iter->current->obj) : NULL;
                                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Fixes: b1810d7a68
(cherry picked from commit b339a2742a)
2017-11-13 14:37:30 +01:00
Thomas Haller
fda3458201 shared: rework _NM_GET_PRIVATE() to use _Generic()
_NM_GET_PRIVATE() used typeof() to propagate constness of the @self
pointer. However, that means, it could only be used with a self pointer
of the exact type. That means, you explicitly had to cast from (GObject *)
or from (void *).
The requirement is cumbersome, and often led us to either create @self
pointer we didn't need:

    NMDeviceVlan *self = NM_DEVICE_VLAN (device);
    NMDeviceVlanPrivate *priv = NM_DEVICE_VLAN_GET_PRIVATE (self);

or casting:

    NMDeviceVlanPrivate *priv = NM_DEVICE_VLAN_GET_PRIVATE ((NMDevice *) device);

In both cases we forcefully cast the source variable, loosing help from
the compiler to detect a bug.

For "nm-linux-platform.c", instead we commonly have a pointer of type
NMPlatform. Hence, we always forcefully cast the type via _NM_GET_PRIVATE_VOID().

Rework the macro to use _Generic(). If compiler supports _Generic(), then we
will get all compile time checks as desired. If the compiler doesn't support
_Generic(), it will still work. You don't get the compile-time checking of course,
but you'd notice that something is wrong once you build with a suitable
compiler.

(cherry picked from commit b1810d7a68)
2017-11-13 14:37:21 +01:00
Beniamino Galvani
6dc6b0f3d8 device: silent compiler warning
Fix the following warning:

 src/devices/nm-device.c: In function ‘activation_source_schedule’:
 src/devices/nm-device.c:4995:9: error: ‘source_func’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
   new_id = g_idle_add (source_func, self);
  ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

(cherry picked from commit bdfa7d882e)
2017-11-10 16:15:22 +01:00