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.
Keep priv->sriov.pending set during the callback set so that it
becomes possible to insert a new operation from the callback itself.
(cherry picked from commit 74ccda8a71)
When dispose() is called, there can't be any pending operation because
they keep a reference to the device. Instead, there can be a a queued
operation not yet executed. Destroy it.
(cherry picked from commit 6fcb077a98)
The file doesn't exist for all interfaces that support SR-IOV. In
particular, netdevsim devices support SR-IOV but don't expose the
file.
(cherry picked from commit 63a932b851)