Run:
./contrib/scripts/nm-code-format.sh -i
./contrib/scripts/nm-code-format.sh -i
Yes, it needs to run twice because the first run doesn't yet produce the
final result.
Signed-off-by: Antonio Cardace <acardace@redhat.com>
NetworkManager can't control the name of the PPP interface name
created by pppd; so it has to wait for the interface to appear and
then rename it. This happens in nm_device_take_over_link() called by
nm-device-ppp.c:ppp_ifindex_set() when pppd tells NM the ifindex of
the interface that was created.
However, sometimes the initial interface name is already correct, for
example when the connection.interface-name is ppp0 and this is the
first PPP interface created.
When this happens, nm_device_update_from_platform_link() is called on
the NMDevicePPP and it sets the device ifindex. Later, when pppd
notifies NM, nm_device_take_over_link() fails because the ifindex is
already set:
nm_device_take_over_link: assertion 'priv->ifindex <= 0' failed
Make nm_device_take_over_link() more robust to cope with this
situation.
https://bugzilla.redhat.com/show_bug.cgi?id=1849386
(cherry picked from commit 75bc21c4cf)
GObject signals only complicate the code and are less efficient.
Also, NM_DEVICE_AUTH_REQUEST signal really invoked an asynchronous
request. Of course, fundamentally emitting a signal *is* the same as
calling a method. However, implementing this as signal is really not
nice nor best practice. For one, there is a (negligible) overhead emitting
a GObject signal. But what is worse, GObject signals are not as strongly
typed and make it harder to understand what happens.
The signal had the appearance of providing some special decoupling of
NMDevice and NMManager. Of course, in practice, they were not more
decoupled (both forms are the same in nature), but it was harder to
understand how they work together.
Add and call a method nm_manager_device_auth_request() instead. This
has the notion of invoking an asynchronous method. Also, never invoke
the callback synchronously and provide a cancellable. Like every asynchronous
operation, it *must* be cancellable, and callers should make sure to
provide a mechanism to abort.
(cherry picked from commit b50702775f)
NMDevice already has access to the NMSettings singleton. It is permissible that
NMDevice *knows* about NMManager. The current alternative is emitting GObject signals
like NM_DEVICE_AUTH_REQUEST, pretending that NMDevice and NMManager would be completely
independent, or that there could be anybody else handling the request aside NMManager.
No, NMManager and NMDevice may know each other and refer to each other. Just like
NMDevice also knows and refers to NMSettings.
(cherry picked from commit 800ac28cca)
We usually want to schedule stage2 when we just completed with the previous
stage (or, if we are currently in stage2, and want to re-enter it).
In those cases, the conditions are often right to just proceed right away.
No need to schedule the stage on an idle handler. Allow to invoke stage2
right away.
There was only API to schedule the stage on an idle handler.
Sometimes, we are just in the right situation to schedule the stage
right away. It should be possibly to avoid going through the extra hop.
For now, none of the caller makes use of this. So, there isn't any
actual change in behavior. But by adding this possibility, we may do
use in the future.
I think it's technically not correct to rely on the "sentinal" field
being immediately after the previous field, due to alignment. Implement
the macro differently.
We try to set only one time the MTU from the connection to not
interfere with manual user changes.
If at some point the parent interface changes temporarily MTU to a
lower value (for example, because the connection was reactivated), the
kernel will also lower the MTU on child interface and we will not
update it ever again.
Add a workaround to this. If we detect that the MTU we want to set
from connection is higher that the allowed one, go into a state where
we follow the parent MTU until it is possible to set again the desired
MTU. This is a bit ugly, but I can't think of any nicer way to do it.
https://bugzilla.redhat.com/show_bug.cgi?id=1751079
Introduce a generic function to set a MTU based on parent's one. Also
define a device-specific @mtu_parent_delta value that specifies the
difference from parent MTU that should be set by default. For VLAN it
is zero but other interface types (for example MACsec) require a
positive value due to encapsulation overhead.
Note that all subclasses of NMDevice that implement act_stage1_prepare(), call
the parents implementation as very first thing.
Previously, NMDevice's act_stage1_prepare() was setting up SR-IOV. But that is
problemantic. Note that it may have returned NM_ACT_STAGE_RETURN_POSTPONE, in which
case subclasses would just skip their action and return to the caller. Later,
sriov_params_cb() would directly call nm_device_activate_schedule_stage2_device_config(),
and thus act_stage1_prepare() was never executed for the subclass. That
is wrong.
First, I don't think it is good to let subclasses decide whether to call a
parents implementation (and when). It just causes ambiguity. In
the best case we do it always in the same order, in the worst case we
call the parent at the wrong time or never. Instead, we want to initialize
SR-IOV always and early in stage1, so we should just do it directly from
activate_stage1_device_prepare(). Now NMDevice's act_stage1_prepare() does
nothing.
The bigger problem is that when a device wants to resume a stage that
it previously postponed, that it would schedule the next stage!
Instead, it should schedule the same stage again instead. That allows
to postpone the completion of a stage for multiple reasons, and each
call to a certain stage merely notifies that something changed and
we need to check whether we can now complete the stage.
For that to work, stages must become re-entrant. That means we need to
remember whether an action that we care about needs to be started, is pending
or already completed.
Compare this to nm_device_activate_schedule_stage3_ip_config_start(),
which checks whether firewall is configured. That is likewise the wrong
approach. Callers that were in stage2 and postponed stage2, and later would
schedule stage3 when they are ready.
Then nm_device_activate_schedule_stage3_ip_config_start() would check whether
firewall is also ready, and do nothing if that's not the case (relying
that when the firewall code completes to call
nm_device_activate_schedule_stage3_ip_config_start().
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.
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.
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.
(cherry picked from commit 03b708f7f7)
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);
(cherry picked from commit 1585eaf473)
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.
It seems to me the NM_DEVICE_CLASS_DECLARE_TYPES() macro confuses more
than helping. Let's explicitly initialize the two fields, albeit with
another helper macro NM_DEVICE_DEFINE_LINK_TYPES() to get the list of
link-types right.
For consistency, also leave nop-lines like
device_class->connection_type_supported = NULL;
device_class->link_types = NM_DEVICE_DEFINE_LINK_TYPES ();
because all NMDevice class init methods should have this same
boiler plate code and to make it explicit that this is intended.
And there are only 3 occurences where this actually comes into play.
NMDeviceOvsPort and NMDeviceOvsInterface don't have an underlying link-type from platform.
Still use NM_DEVICE_CLASS_DECLARE_TYPES() macro, for consistancy reasons.
This requires to extend NM_DEVICE_CLASS_DECLARE_TYPES() macro, to support
a variadic argument list with zero link-types.
Instead of returning a boolean @is_user_config value from
get_configured_mtu(), return an mtu-source enum with possible values
NONE,CONNECTION. This enum will be expanded later; for now there is no
change in behavior.
(cherry picked from commit 9f8b0697de)
We commonly only allow tabs at the beginning of a line, not
afterwards. The reason for this style is so that the code
looks formated right with tabstop=4 and tabstop=8.
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, add a SetIfindex() D-Bus method that gets called by the
plugin when pppd becomes RUNNING. The method sets the IP ifindex of
the device and starts IP configuration.
https://bugzilla.redhat.com/show_bug.cgi?id=1515829
Kernel does not allow setting the MTU of a VLAN larger
then the MTU of the underlying device. Hence, we might
initially fail to set a large MTU of the VLAN, but we
have to retry when the MTU of the parent changes.
https://bugzilla.redhat.com/show_bug.cgi?id=1414901
- cleanup data type and use guint32 consistently. We might want to
introduce a new "infinity" value. But since libnm's
NM_SETTING_IP_CONFIG_DHCP_TIMEOUT asserts against the range
0 - G_MAXINT32, we cannot express it as -1 anyway. So, infinity
will have the numerical value G_MAXINT32, hence guint32 is just
fine.
- make use of existing ipv6.dhcp-timeout setting and add global
default configuration in NetworkManager.conf
- instead of having subclasses call nm_device_set_dhcp_timeout(),
add a virtual function get_dhcp_timeout().
Previously when the interface created by pppd was already the one we
expected, we would rename it to itself and remove the device from the
manager. Don't do it.
Fixes: 6c3195931e
Add code to NMPppDevice to activate new-style PPPoE connections. This
is a bit tricky because we can't create the link as usual in
create_and_realize(). Instead, we create a device without ifindex and
start pppd in stage2; when pppd reports a new configuration, we rename
the platform link to the correct name and set the ifindex into the
device.
This mechanism is inherently racy, but there is no way to tell pppd to
create an arbitrary interface name.
Since commit 2b51d3967 "device: merge branch 'th/device-mtu-bgo777251'",
we always set the MTU for certain device types during activation. Even
if the MTU is neither specified via the connection nor other means, like
DHCP.
Revert that change. On activation, if nothing explicitly configures the
MTU, leave it unchanged. This is like what we do with ethernet's
cloned-mac-address, which has a default value "preserve".
So, as last resort the default value for MTU is now 0 (don't change),
instead of depending on the device type.
Note that you also can override the default value in global
configuration via NetworkManager.conf.
This behavior makes sense, because whenever NM actively resets the MTU,
it remembers the previous value and restores it when deactivating
the connection. That wasn't implemented before 2b51d3967, and the
MTU would depend on which connection was previously active. That
is no longer an issue as the MTU gets reset when deactivating.
https://bugzilla.redhat.com/show_bug.cgi?id=1460760
