The previous function arguments of nm_modem_act_stage2_config() act as if the
function could fail or even postpone the action. It never did.
We cannot treat this generic. A caller needs to know whether nm_modem_act_stage2_config()
can postpone the action, and when it does, which signal is emitted upon completion. That
is, the caller needs to know how to proceed after postponing.
In other words, since this function currently cannot fail or postpone
the stage, so must all callers already rely on that. At this point it makes
no sense to pretend that the function could be any different, if all callers
assume it is not. Simplify the API.
Currently, we cannot ask which modems exist. NMDeviceBt may claim it
via nm_device_factory_emit_component_added(), and NMWWanFactory may
take it by listening to NM_MODEM_MANAGER_MODEM_ADDED. But that's it.
We will drop nm_device_factory_emit_component_added() because it's only
used for passing modems to NMDeviceBt. Instead, NMDeviceBt can directly
subscribe to NM_MODEM_MANAGER_MODEM_ADDED. It already has a reference
to NMModemManager.
Anyway, the NM_MODEM_MANAGER_MODEM_ADDED signal is no enough, because
sometimes when the mode appears, NMDeviceBt might not yet know whether
it should take it (because the DUN connect call is not yet complete).
Currently that never happens because dun_connect() blocks waiting for
the device. That must be fixed, by not waiting. But this opens up a
race, and NMDeviceBt might after NM_MODEM_MANAGER_MODEM_ADDED need to
search for the suitable modem: by iterating the list of all modems.
NMModem-s are either used by NMDeviceModem or by NMDeviceBt.
The mechanism how that is coordinated it odd:
- the factory emits component-added, and then NMDeviceBt
might take the device (and claim it). In that case, component-added
would return TRUE to indicate that the modem should not be also
used by NMDeviceModem.
- next, if the modem has a driver that looks like bluetooth, NMDeviceModem
ignores it too.
- finally, NMDeviceModem claims the modem (which is now considered to
be non-bluetooth).
I think the first problem is that the device factory tries to have this
generic mechanism of "component-added". It's literally only used to
cover this special case. Note that NMDeviceBt is aware of modems. So,
abstracting this just adds lots of code that could be solved better
by handling the case (of giving the modem to either NMDeviceBt or
NMDeviceModem) specifically.
NMWWanFactory itself registers to the NM_MODEM_MANAGER_MODEM_ADDED
signal and emits nm_device_factory_emit_component_added().
We could just have NMWWanFactory and NMDeviceBt both register to
that signal. Signals even support priorities, so we could have
NMDeviceBt be called first to claim the device.
Anyway, as the modem can only have one owner, the modem should have
a flag that indicates whether it's claimed or not. That will allow
multiple components all look at the same modem and moderate who is
going to take ownership.
This adds capability to hand over the network configuration from
OpenFirmware (and potentially other boot loaders with openfirmware
support such as U-Boot) to NetworkManager.
It's done analogously to ACPI/iBFT. In fact, the same ip=ibft command
line option is used, adding a more general ip=fw alias. This probably
deserves some documentation, but I'm not adding any at this time.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/257
I am about to change the when stage1 gets postponed, then the way to
proceed it is to schedule stage1 again (instead of scheduling stage2).
The reason is that stage1 handling should be reentrant and we should
keep entering it until there is no more reason to postpone it. If
a subclass postpones stage1 and then later progresses it by directly
scheduling stage2, then only the subclass is in control over postponing
stage 2.
Instead, anybody should be able to delay stage2 independently. That can
only work if everybody signals readyness to proceed by scheduling stage1
again.
NMDevice's act_stage1_prepare() now does nothing. Calling it is not
useful and has no effect.
In general, when a subclass overwrites a virtual function, it must be
defined whether the subclass must, may or must-not call the parents
implementation. Likewise, it must be clear when the parents
implementation should be chained: first, as last, does it matter?
In any case, that very much depends on how the parent is implemented
and this can only be solved by documentation and common conventions.
It's a forgiving approach to have a parents implementation do nothing,
then the subclass may call it at any time (or not call it at all).
This is especially useful if classes don't know their parent class well.
But in NetworkManager code the relationship between classes are known
at compile time, so every of these classes knows it derives directly
from NMDevice.
This forgingin approach was what NMDevice's act_stage1_prepare() was doing.
However, it also adds lines of code resulting in a different kind of complexity.
So, it's not clear that this forgiving approach is really better. Note
that it also has a (tiny) runtime and code-size overhead.
Change the expectation of how NMDevice's act_stage1_prepare() should be
called: it is no longer implemented, and subclasses *MUST* not chain up.
NM_DEVICE_MODEM_GET_PRIVATE() is based on _NM_GET_PRIVATE(), which has
some smarts to check the pointer type, but is fine with well-known parent
pointer types like "NMDevice *".
Changing "ipv4.route-table" and "ipv6.route-table" was not allowed
during reapply.
The main difficulty for supporting that is changing the sync-mode.
With route-table 0, we don't sync all tables but only the main table.
So, when reapply changes from full-sync to no-full-sync, it's slightly
more complicated.
But it's probably not too complicated either. The change from
no-full-sync to full-sync is simple: we just start doing a full-sync.
The reverse change is slightly more complicated, because we need to
do one last full-sync, to get rid of routes that we configured on those
other tables.
We no longer add these. If you use Emacs, configure it yourself.
Also, due to our "smart-tab" usage the editor anyway does a subpar
job handling our tabs. However, on the upside every user can choose
whatever tab-width he/she prefers. If "smart-tabs" are used properly
(like we do), every tab-width will work.
No manual changes, just ran commands:
F=($(git grep -l -e '-\*-'))
sed '1 { /\/\* *-\*- *[mM]ode.*\*\/$/d }' -i "${F[@]}"
sed '1,4 { /^\(#\|--\|dnl\) *-\*- [mM]ode/d }' -i "${F[@]}"
Check remaining lines with:
git grep -e '-\*-'
The ultimate purpose of this is to cleanup our files and eventually use
SPDX license identifiers. For that, first get rid of the boilerplate lines.
NM_MODEM_OPERATOR_CODE property is construct-only. Add our common
code comment to the property setter.
Construct-only setters are pretty simple. They run before the object is
constructed, hence their scope is clearer. As such, there is no need to
emit property changed notifications (also because that is already taken
care by the GObject property setter). Don't call _nm_modem_set_operator_code(),
just directly set the property.
Usually we aim to have only one place where we set state (_nm_modem_set_operator_code()).
But a construct-only property setter is trivial enough that we can affort having two
places to modify the property. In particular, because the property setter does not "modify"
the property, it merely initializes it before the object is fully
constructed.
Connection defaults should correspond in range to the per-profile values.
"infiniband.mtu" is required to be not larger than 65520, so we also
need to honor that when parsing the connection default.
This is wrong -- we may want to start activating before device is
registered if it the SIM needs unlocking with a PIN code that's included
in the connection.
This reverts commit 2e8f43e379.
Based on Ubuntu's "Modify NMDeviceModem's available logic" patch by
Tony Espy <espy@canonical.com>. The original commit message:
This patch modifies NMDeviceModem's available logic such that the device
is only considered available if the modem_state is
>= NM_MODEM_STATE_REGISTERED. NMDevice defines 'available' as meaning the
device is in such a state that it can be activated. This change prevents
NM from trying to activate a modem which is not yet ready to be activated.
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1445080https://github.com/NetworkManager/NetworkManager/pull/312
dev2_ip_config (formerly wwan_ip_config) is only set by nm_device_set_dev2_ip_config()
(formerly nm_device_set_wwan_ip_config()), which is only called by NMDeviceModem.
For NMDeviceWireGuard we will also inject additional configuration
in the parent class. Rename and give it a wider purpose. The new name
merely indicates that this IP configuration is injected by a subclass
of NMDevice.
It is preferable to treat IPv4 and IPv6 in a similar manner.
This moves the places where we differ down the call-stack.
It also make it clearer how IPv6 behaves differently. I think this
is a bug, but leave it for now.
+ /* If IP had previously failed, move it back to IP_CONF since we
+ * clearly now have configuration.
+ */
+ if (priv->ip6_state == IP_FAIL)
+ _set_ip_state (self, AF_INET6, IP_CONF);
These functions call platform's sysctl getter and setters.
Note that the called platform functions are called nm_platform_sysctl_get()
and nm_platform_sysctl_set(). Also, in this case they use the ip-conf path
via nm_utils_sysctl_ip_conf_path().
Also, next we will add API nm_platform_sysctl_ip_conf_get() and
nm_platform_sysctl_ip_conf_set(), which will be wrappers around
nm_platform_sysctl_get() and nm_platform_sysctl_set(), using the ip-conf
paths as well.
Rename the device functions, to be more similar to the existing and future
naming in platform.
For one, next we will drop setting rp_filter, hence there are no
more users of an IPv4 variant and nm_device_ipv4_sysctl_set() would
have to be dropped anyway.
However, instead of doing that, merge the IPv4 and IPv6 variant.
With this, the fallback to the default is now also supported for IPv6
(though unused).
Also, don't access nm_device_get_ip_iface(). The interface name might
not be right, we should only rely on the ifindex. Load the interface
name from platform cache instead.
The 'number' property in GSM settings is a legacy thing that comes
from when ModemManager used user-provided numbers, if any, to connect
3GPP modems.
Since ModemManager 1.0, this property is completely unused for 3GPP
modems, and so it doesn't make sense to use it in the NetworkManager
settings. Ofono does not use it either.
For AT+PPP-based 3GPP modems, the 'number' to call to establish the
data connection is decided by ModemManager itself, e.g. for standard
GSM/UMTS/LTE modems it will connect a given predefined PDP context,
and for other modems like Iridium it will have the number to call
hardcoded in the plugin itself.
https://github.com/NetworkManager/NetworkManager/pull/261
The reasons to block autoconnection at settings level are not the same
as the ones to block autoconnection at device level.
E.g. if the SIM-PIN is wrong, you may want to block autoconnection
both at settings level (as the PIN configured in settings is wrong)
and at device level (so that no other setting is tried automatically).
For some other reasons, you may want to block autoconnection only at
setting level (e.g. wrong APN).
And for some other reasons you may want to block autoconnection at
device level only (e.g. SIM missing), so that the autoconnection
blocking is removed when the device goes away. This is especially
important with SIM hotplug events processed by ModemManager, as a
device without SIM will be removed from MM when a new SIM is
inserted, so that a completely new object is exposed in MM with the
newly detected SIM.
https://github.com/NetworkManager/NetworkManager/pull/259
Refactor some code to use nm_streq() and NM_IN_STRSET() instead of
strcmp().
Note that nm_utils_get_ip_config_method() never returns %NULL (not even
with g_return*() assertion failures). nm_streq() is sufficent.
Recently, more and more code was refactored to use an addr_family
integer to distinguish between IPv4 and IPv6.
Refactor nm_utils_get_ip_config_method() and nm_device_get_effective_ip_config_method()
to do that too. If we use different identifiers, we need to translate from one to
another and its inconsistent. Also, accessing a GType is an unnecessary function call,
instead of a plain constant.
