Trace logging from libnm is verbose. So, by default we print trace
messages to stderr. However, that means that messages printed to stdout
are not in sync with the trace logging.
That means, if the libnm application prints messages to stdout, and
you'd like to correlate them with trace messages, it is difficult.
Add an option to allow printing trace messages to stdout.
$ LIBNM_CLIENT_DEBUG=trace,stdout nmcli
Possibly redirecting stderr to stdout might also work around the
ordering issue. However, it's not entirely clear how buffering of
the file streams affects this.
When using VRF devices we must pre-generate dependent local
routes in the VRF's table otherwise they will be incorrectly added
to the local table instead.
https://bugzilla.redhat.com/show_bug.cgi?id=1857133
Fixes: a199cd2a7d ('core: add dependent local routes configured by kernel')
Kernel will reject setting "active_slave", if the interface is not enslaved or not
up. We already handle that by setting the option whenever we enslave an interface.
However, we also must not set it initially, otherwise we get an ugly error log message:
NetworkManager[939]: <debug> [1594709143.7459] platform-linux: sysctl: setting net:/sys/class/net/bond99/bonding/active_slave to eth1 (current value is )
NetworkManager[939]: <error> [1594709143.7459] platform-linux: sysctl: failed to set bonding/active_slave to eth1: (22) Invalid argument
NetworkManager[939]: <warn> [1594709143.7460] device (bond99): failed to set bonding attribute active_slave to eth1
...
kernel: bond99: (slave eth1): Device is not bonding slave
kernel: bond99: option active_slave: invalid value (eth1)
See-also: https://bugzilla.redhat.com/show_bug.cgi?id=1856640https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/577
Without this, `nmcli device modify "$DEVICE"` leads to a crash. At least
since commit c5d45848dd ('cli: mark argv argument for command line
parsing as const'), when this happens.
That is, because it passes a NULL strv array with argc being set to
zero. nmc_process_connection_properties() is not supposed to access
the array, if there are no elements there.
Fixes: c5d45848dd ('cli: mark argv argument for command line parsing as const')
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/492
(cherry picked from commit 09c94bc24f)
Without this, `nmcli device modify "$DEVICE"` leads to a crash. At least
since commit c5d45848dd ('cli: mark argv argument for command line
parsing as const'), when this happens.
That is, because it passes a NULL strv array with argc being set to
zero. nmc_process_connection_properties() is not supposed to access
the array, if there are no elements there.
Fixes: c5d45848dd ('cli: mark argv argument for command line parsing as const')
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/492
When configuring miimon settings, the updelay/downdelay fields with
value zero may not be stored in the setting.
For example:
- have a profile with "mode=balance-rr,arp_interval=11,arp_ip_target=10.10.10.1,miimon=10"
Switch the link monitoring mode to "MII" and press <OK>. Previously,
the change of the link monitoring did not update the settings, and
nothing was changed.
- when loading settings, initialize all fields with the values from the
settings, regardless whether they are currently visible or not.
Otherwise, if you edit a profile with
"mode=balance-rr,arp_interval=11,arp_ip_target=10.10.10.1,miimon=10"
and switch link monitoring mode to "MII", the miimon setting was not
initialized to 10.
- accept empty bond settings, for example for updelay. In that case,
initialize the text input to "0". Likewise, when the text entry is
empty, set the bond option to the respective default.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/488
(cherry picked from commit 61d4bc62e0)
Before 1.24, nm_setting_bond_add_option() would clear
miimon/arp_interval settings when the respective other was set.
That was no longer done, with the effect that enabling (for example)
miimon on a bond profile that has arp_interval enabled, sets both
conflicting options.
That is not a severe problem, because the profile still validates.
However, at runtime only one of the settings can be actually configured.
Fix that, by restoring the previous behavior for the client. But note
that this time it's implemented in the client, and not in libnm's
nm_setting_bond_add_option().
(cherry picked from commit b55578bf6e)
We use sysfs API for setting bond options. Note that the miimon and
arp_interval settings conflict each other, and whenever setting one
of these sysfs values, the other one gets reset. That means,
NetworkManager needs to mediate and handle a profile which has both
these options set.
Before 1.24, the libnm API nm_setting_bond_add_option() API would mangle
the content of the bond settings, to clear the respective other fields
when setting miimon/arp_interval. That also had the effect that the
settings plugins, weren't able to read such (conflicting) settings
back from disk (but they would write them to disk). If a keyfile
specified both miimon and arp_interval keys, then it would depend on
their order in the keyfile which wins.
It is wrong that a libnm property setter mangles the option in such a way,
especially, because you still could set the NM_SETTING_BOND_OPTIONS
property directly and bypass this. So, since 1.24, you can create
profiles that have conflicting options.
Also, we can now not start to reject such settings as invalid, because that
would be an API break. Instead, just make sure that when one of the
settings is set, that the other one consistently gets deactivated.
Also, before 1.24 already, NMDeviceBond would mediate whether to either set
miimon or arp_interval settings. Despite that the keyfile reader would
mangle the settings, it would also prefer miimon over arp_interval,
if both were set.
This mechanism was broken since we switch to _bond_get_option_normalized()
for that. As a consequence, NetworkManager would try to set both the
conflicting options. Fix that.
(cherry picked from commit 4aa46328ca)
_bond_get_option_normalized() gets called with code paths that don't
assume a valid options hash. That means, the bond mode might be invalid
and we should fail an assertion.
(cherry picked from commit 1543f8a1a1)
- the arp_ip_target option in the settings might not have normalized
IP addresses or duplicates. If there would be duplicates, setting
them twice would fail with EINVAL. Hence, first normalize them
and make them unique.
- if what we want to set is identical to what is already set, don't
do anything.
(cherry picked from commit 6a923a5d57)
We already have meta data for all bond options. For example,
"arp_ip_target" has type NM_BOND_OPTION_TYPE_IP.
Also, verify() already calls nm_setting_bond_validate_option() to validate
the option. Doing a second validation below is redundant (and done
inconsistently).
Validate the setting only once.
Also beef up the validation and use nm_utils_bond_option_arp_ip_targets_split()
to parse the IP addresses. This now strips extra whitespace and (as
before) removes empty entries.
(cherry picked from commit e96051d734)
Note yet used. The way how we split the option is relevant at various
places. The code should use the same helper function.
(cherry picked from commit 4ee0e8f075)
When configuring miimon settings, the updelay/downdelay fields with
value zero may not be stored in the setting.
For example:
- have a profile with "mode=balance-rr,arp_interval=11,arp_ip_target=10.10.10.1,miimon=10"
Switch the link monitoring mode to "MII" and press <OK>. Previously,
the change of the link monitoring did not update the settings, and
nothing was changed.
- when loading settings, initialize all fields with the values from the
settings, regardless whether they are currently visible or not.
Otherwise, if you edit a profile with
"mode=balance-rr,arp_interval=11,arp_ip_target=10.10.10.1,miimon=10"
and switch link monitoring mode to "MII", the miimon setting was not
initialized to 10.
- accept empty bond settings, for example for updelay. In that case,
initialize the text input to "0". Likewise, when the text entry is
empty, set the bond option to the respective default.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/488
Before 1.24, nm_setting_bond_add_option() would clear
miimon/arp_interval settings when the respective other was set.
That was no longer done, with the effect that enabling (for example)
miimon on a bond profile that has arp_interval enabled, sets both
conflicting options.
That is not a severe problem, because the profile still validates.
However, at runtime only one of the settings can be actually configured.
Fix that, by restoring the previous behavior for the client. But note
that this time it's implemented in the client, and not in libnm's
nm_setting_bond_add_option().
We use sysfs API for setting bond options. Note that the miimon and
arp_interval settings conflict each other, and whenever setting one
of these sysfs values, the other one gets reset. That means,
NetworkManager needs to mediate and handle a profile which has both
these options set.
Before 1.24, the libnm API nm_setting_bond_add_option() API would mangle
the content of the bond settings, to clear the respective other fields
when setting miimon/arp_interval. That also had the effect that the
settings plugins, weren't able to read such (conflicting) settings
back from disk (but they would write them to disk). If a keyfile
specified both miimon and arp_interval keys, then it would depend on
their order in the keyfile which wins.
It is wrong that a libnm property setter mangles the option in such a way,
especially, because you still could set the NM_SETTING_BOND_OPTIONS
property directly and bypass this. So, since 1.24, you can create
profiles that have conflicting options.
Also, we can now not start to reject such settings as invalid, because that
would be an API break. Instead, just make sure that when one of the
settings is set, that the other one consistently gets deactivated.
Also, before 1.24 already, NMDeviceBond would mediate whether to either set
miimon or arp_interval settings. Despite that the keyfile reader would
mangle the settings, it would also prefer miimon over arp_interval,
if both were set.
This mechanism was broken since we switch to _bond_get_option_normalized()
for that. As a consequence, NetworkManager would try to set both the
conflicting options. Fix that.
_bond_get_option_normalized() gets called with code paths that don't
assume a valid options hash. That means, the bond mode might be invalid
and we should fail an assertion.
- the arp_ip_target option in the settings might not have normalized
IP addresses or duplicates. If there would be duplicates, setting
them twice would fail with EINVAL. Hence, first normalize them
and make them unique.
- if what we want to set is identical to what is already set, don't
do anything.
We already have meta data for all bond options. For example,
"arp_ip_target" has type NM_BOND_OPTION_TYPE_IP.
Also, verify() already calls nm_setting_bond_validate_option() to validate
the option. Doing a second validation below is redundant (and done
inconsistently).
Validate the setting only once.
Also beef up the validation and use nm_utils_bond_option_arp_ip_targets_split()
to parse the IP addresses. This now strips extra whitespace and (as
before) removes empty entries.
