Commit 4a6fd0e83e ("device: honor the connection.autoconnect-retries
for 802.1X") added a reset of the autoconnect retries when the device
changes state, because the retry logic for 802.1x is implemented in
NMDeviceEthernet. For other connections, we should not reset the
retries as NMPolicy handles them.
Fixes: 4a6fd0e83e
This argument is only relevant when the NMActStageReturn argument
indicates NM_ACT_STAGE_RETURN_FAILURE. In all other cases it is ignored.
Rename the argument to make the meaning clearer. The argument is passed
through several layers of code, it isn't obvious that this argument only
matters for the failure case. Also, the distinct name makes it easier
to distinguish from other uses of the "reason" name.
While at it, do some drive-by cleanup:
- use g_return_*() instead of g_assert() to have a more graceful
assertion.
- functions like dhcp4_start() don't need to return a failure reason.
Most callers don't care, and the caller who does can determine the
proper reason.
- allow omitting the out-argument via NM_SET_OUT().
NMDeviceEthernet and NMDeviceMacsec implement their own retry policy
for connection using 802.1X, and consider the credentials wrong when
the authentication fails for 3 times. In such case, they also disable
autoconnection for the device by setting the state reason NO_SECRETS.
This means that it's not possible at the moment to choose how many
times the authentication will be retried since they don't use the
standard reconnection logic.
Change NMDeviceEthernet and NMDeviceMacsec to use the number of
retries from connection.autoconnect-retries instead of a hardcoded
value to decide how many times the authentication must be restarted.
Use the per-connection authentication timeout for 802.1X Ethernet,
MACsec and Wi-Fi connections. In case the value is not defined, fall
back to the global one.
Instead of having a NM_SUPPLICANT_INTERFACE_CONNECTION_ERROR signal to notify
about failures during AddNetwork/SelectNetwork, accept a callback to report
success/failure.
Thereby, rename nm_supplicant_interface_set_config() to
nm_supplicant_interface_assoc().
The async callback is guaranteed to:
- be invoked exactly once, signalling success or failure
- always being invoked asyncronously.
The pending request can be (synchronously) cancelled via
nm_supplicant_interface_disconnect() or by disposing the
interface instance. In those cases the callback will be invoked
too, with error code cancelled/disposing.
Also change the signature of the NM_SUPPLICANT_INTERFACE_STATE signal,
to have three "int" type arguments. Thereby also fix the subscribers
to this signal that wrongly had type guint32, instead of guint
(which happens to be the same underlying type, so no real problem).
https://mail.gnome.org/archives/networkmanager-list/2017-February/msg00021.html
The -Wimplicit-fallthrough=3 warning is quite flexible of accepting
a fall-through warning.
Some comments were missing or not detected correctly.
Thereby, also change all other comments to follow the exact
same pattern.
Previously, we would have different functions like
- nm_match_spec_device_type()
- nm_match_spec_hwaddr()
- nm_match_spec_s390_subchannels()
- nm_match_spec_interface_name()
which all would handle one type of match-spec.
So, to get the overall result whether the arguments
match or not, nm_device_spec_match_list() had to stich
them together and iterate the list multiple times.
Refactor the code to have one nm_match_spec_device()
function that gets all relevant paramters.
The upside is:
- the logic how to evaluate the match-spec is all at one place
(match_device_eval()) instead of spread over multiple
functions.
- It requires iterating the list at most twice. Twice, because
we do a fast pre-search for "*".
One downside could be, that we have to pass all 4 arguments
for the evaluation, even if the might no be needed. That is,
because "nm-core-utils.c" shall be independend from NMDevice, it
cannot receive a device instance to get the parameters as needed.
As we would add new match-types, the argument list would grow.
However, all arguments are cached and fetching them from the
device's private data is very cheap.
(cherry picked from commit b957403efd)
Instead of overwriting ip4_config_pre_commit(), add a new function
get_mtu().
This also adds a default value in case there is no user-configuration.
This will allow us later to reset a default MTU based on the device
type.
Most implementations of realize_start_notify() do the same
for link_changed().
Let NMDevice's base implementation of realize_start_notify() call
link_changed() -- which by default does notthing. This allows subclasses
to only overwrite link_changed().
Instead of overwriting constructed(), update the s390 subchannels via
realize_start_notify(). This makes more sense and is also more similar
to what other device implementations do.
Previously, we would require a @self argument and the @call_id in
nm_act_request_cancel_secrets(), although the @call_id already has
a pointer to @self.
In principle that is not necessary, but it makes the API a bit
more robust as you need to care about the lifetime of the @req
as well.
However it is a bit inconvenient, because it requires that caller to
track both the activation request and the call-id.
Now, allow nm_act_request_get_secrets() to instruct the call-id to
take an additional reference to @self. Later on, we would allow to omit
the argument during cancelling. We only allow this, if the call-id
takes a reference to @self.
The ioctl APIs ethtool/mii require an interface ifname. That is inherrently
racy as interfaces can be renamed. This cannot be fixed, we can only
minimize the time between verifying the ifname and calling ioctl.
We already had problems with that when ethtool would access an interface
by name that didn't exists. See commit ab41c13b06 .
Checking for an existing interface only helps avoiding races when an interface
gets deleted. It does not help against renaming.
Go one step further, and instead of checking whether such an ifname
exists, try to get the ifname based on the ifindex immediately before
we need it.
This brings an additional overhead for each ethtool access.
Moving the PPP manager to a separate plugin that is loaded when needed
has the advantage of slightly reducing memory footprint and makes it
possible to install the PPP support only where needed.
https://bugzilla.gnome.org/show_bug.cgi?id=773482
This makes it easier to install the files with proper names.
Also, it makes the makefile rules slightly simpler.
Lastly, the documentation is now generated into docs/api, which makes it
possible to get rid of the awkward relative file names in docbook.
Added platform functions to retrieve device link mode status and to
switch from auto to manual link negotiation:
nm_platform_ethtool_get_link_settings
nm_platform_ethtool_set_link_settings
Keep the include paths clean and separate. We use directories to group source
files together. That makes sense (I guess), but then we should use this
grouping also when including files. Thus require to #include files with their
path relative to "src/".
Also, we build various artifacts from the "src/" tree. Instead of having
individual CFLAGS for each artifact in Makefile.am, the CFLAGS should be
unified. Previously, the CFLAGS for each artifact differ and are inconsistent
in which paths they add to the search path. Fix the inconsistency by just
don't add the paths at all.
The permanent MAC address of an NMDevice shall not change as
long as the device is realized. That is, we read it only once
and don't change it afterwards.
There are two issues that this commit tries to mitigate:
(1) users are advised to use UDEV to rename interfaces. As we lookup
the permenent MAC address using ethtool (which uses the interface
name), there is a race where we could read the permanent MAC
address using the wrong interface name. We should wait until
UDEV finished initializing the device and until the interface
name is stable (see rh#1388286).
This commit still cannot avoid the race of ethtool entirely. It only
tries to avoid ethtool until UDEV has done its work. That is, until we
expect the interface name no longer to change.
(2) some device types, don't have a permanent MAC address so we fall
back to use the currently set address (fake). Again, users are advised
to use UDEV to configure the MAC addresses on such software devices.
Thus, we should not get the fake MAC address until UDEV initialized
the device.
This patch actually doesn't solve the problem at all yet.
The reason is that a regular caller of nm_device_get_permanent_hw_address() can
not afford to wait until UDEV settled. Thus, any user who requests the
permanent MAC address before the link is initialized, runs into the
problems above.
In a next step, we shall revisit such calls to nm_device_get_permanent_hw_address()
and delay them until the link is initialized.
We only needed proper glib enum types for having properties
and signal arguments. These got all converted to plain int,
so no longer generate such an enum type.
Had to rename "nm-enum-types.h" because it works badly with
"libnm/nm-enum-types.h". Maybe I could fix that differently,
but duplicate names is anyway error prone.
Note that "nm-core-enum-types.h" is already taken too, so
"nm-src-enum-types.h" it is.
An interface would make sense to allow the actual device-factory to inherit
from another type.
However, glib interfaces make code much harder to follow and less
efficient. The device factory shall be a very simple type with meta data
about supported device types and the ability to create device instances.
There is no need to make this an interface implementation, instead just
let the factories inherit from NM_TYPE_DEVICE_FACTORY directly.
Some TTY drivers or devices appear to ignore port speed and always
report zero. Technically this means the port is hung up and control
lines should be disconnected, but with USB devices many of the serial
port attributes are meaningless and ignored by some devices.
pppd requires the port's speed to be greater than zero, and will
exit immediately when that is not the case, even though these
modems will work fine. Passing an explicit speed to pppd in this
case works around the issue, as pppd attempts to set that speed
on the port and doesn't actually care if that operation fails.
https://bugzilla.redhat.com/show_bug.cgi?id=1281731
- use _NM_GET_PRIVATE() and _NM_GET_PRIVATE_PTR() everywhere.
- reorder statements, to have GObject related functions (init, dispose,
constructed) at the bottom of each file and in a consistent order w.r.t.
each other.
- unify whitespaces in signal and properties declarations.
- use NM_GOBJECT_PROPERTIES_DEFINE() and _notify()
- drop unused signal slots in class structures
- drop unused header files for device factories
enables (back) matching against 802-3-ethernet.mac-address and
802-3-ethenet.mac-address-blacklist connection parameters
for MAC addresses belonging to virtual devices too.
No longer typedef NMDeviceEthernet to NMDevice. We don't do that
for most other classes, and I think it is not a good pattern
(yes, the casts are cumbersome, but what can you do).
Also, embed a pointer to the private data in NMDeviceEthernet
for fast lookup and ease of debugging.
