| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This solves a bug exposed by the following cmds:
$ nmcli c add type bond ifname bond0 con-name bond0
$ nmcli c modify bond0 +bond.options miimon=100
$ nmcli -f bond.options c show bond0
bond.options: mode=balance-rr
Here we just added the option 'miimon=100', but it doesn't get saved in
because nm_settings_connection_set_connection() which is responsible for
actually updating the connection compares the new connection with old
one and if and only if the 2 are different the update is carried out.
The bug is triggered because when comparing, if default values are taken into
account, then having 'miimon=100' or not having it it's essentially the
same for compare(). While this doesn't cause a bond to have a wrong
setting when activated it's wrong from a user experience point of view
and thus must be fixed.
When this patch is applied, the above
commands will give the following results:
$ nmcli c add type bond ifname bond0 con-name bond0
$ nmcli c modify bond0 +bond.options miimon=100
$ nmcli -f bond.options c show bond0
bond.options: mode=balance-rr,miimon=100
Fix unit tests and also add a new case covering this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=1806549
|
|
|
|
|
|
|
|
| |
Since ifcfg-rh doesn't write out to file the 'connection.timestamp' property
let's add it before comparing an updated connection with the plugin's reread
one otherwise the comparison operation would always fail.
The fix is not necessary for the keyfile plugin, because the reader/writer
correctly reads/writes the connection timestamp.
|
|
|
|
|
|
|
|
|
|
| |
When a device gets a prefix delegation, we call
nm_device_use_ip6_subnet() for all other devices that have IPv6
sharing enabled, which changes the current IPv6 configuration and
notifies NMPolicy. When updating the DNS configuration in NMPolicy, we
should notify all devices except the one that triggered the change.
https://bugzilla.redhat.com/show_bug.cgi?id=1488030
|
|
|
|
| |
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/397#note_438764
|
|
|
|
|
|
| |
The email address should match the commit's authors and
what we track in .mailmap file. Adjust the email address
for Dan.
|
|
|
|
| |
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/397#note_438590
|
|
|
|
| |
https://bugzilla.redhat.com/show_bug.cgi?id=1706646
|
|
|
|
| |
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/397#note_438497
|
|
|
|
| |
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/397#note_438328
|
|
|
|
| |
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/397#note_438327
|
|
|
|
| |
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/397#note_438290
|
|
|
|
| |
https://mail.gnome.org/archives/networkmanager-list/2020-March/msg00023.html
|
|\
| |
| |
| | |
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/427
|
| | |
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Configuration stages like act_stage2_config() can postpone progressing
to the next stage. Currently, when the condition that we wait for gets
satisfied, the code schedules the next stage from there.
I think that is wrong, because when we postpone from act_stage2_config(),
follow up steps of stage2 get skipped. Thus, when we are ready to progress,
the class should enter stage 2 again.
This requires that stage2 becomes reentrant and that the code reenters the
same stage.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
right away
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.
|
| | |
|
| |
| |
| |
| | |
Mostly just cleanups, there should be no significant change in behavior.
|
| | |
|
|/ |
|
|\
| |
| |
| | |
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/435
|
| |
| |
| |
| |
| |
| |
| |
| | |
cols_len might be larger than header_row->len. That is when
the cols has entries that are not leaf entries (which currently
I think is never the case).
Fix it to use the right variable for the length of the row.
|
| | |
|
| |
| |
| |
| |
| |
| | |
It makes debugging and understanding the code slightly simpler, if we
have a pointer of correct type, instead of returning a GArray. We don't
need the GArray at this point anymore.
|
| |
| |
| |
| | |
Will be used later.
|
| |
| |
| |
| | |
initializers for data
|
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Refactor the selection of the Wi-Fi device by name. Avoid
find_wifi_device_by_iface() to lookup by name. We already
have a sorted list of candidate devices. The ifname is just
an additional filter to exclude devices. So, we shouldn't
use find_wifi_device_by_iface(), but instead check our prepared
list of devices, whether it contains matching candidates.
|
| |
| |
| |
| |
| |
| |
| |
| | |
It's not really necessary, but it feels slightly more correct. The only
reason not to take a reference is to safe the overhead of increasing and
decreasing the reference counter. But that doesn't matter for nmcli
at this point (and is tiny anyway). Let the API make sure that the instances
are kept alive.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
The compare pattern seems simple, but seems error prone and subtle.
NM_CMP*() avoids that.
For example, nm_access_point_get_strength() returns an uint8_t.
C will promote those values to "int" before doing the subtraction.
Likewise, nm_access_point_get_frequency() returns a uint32_t. This
gets promoted to unsigned int when doing the subtraction. Afterwards,
that is converted to a signed int.
So both cases were in fact correct. But such things are not obvious.
Also, as fallback sort by D-Bus path. While that is not semantically
useful, we should use a defined sort order.
|
|/
|
|
|
|
| |
We already have an implementation for converting a binary
array to hex. And, it doesn't require a GString for constructing
the output that has an known length.
|
|
|
|
|
|
|
| |
Of course, having no list does not mean we cannot resolve the service-type.
That is, because we also have a hard-coded list of known VPNs.
Fixes: 67c00353d367 ('libnm: reuse _list_find_by_service() for searching NMVpnPluginInfo')
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The NMVpnPluginInfo class is not intended to be subclassed. An API that
allows to be subclassed needs to be designed in a certain manner for
that to be useful. NMVpnPluginInfo does not want to support that.
Only because a user technically could do that (as the structs were
in the public headers), it does not make it supported. Not everything
that is possible in C is guaranteed to work.
Also, of course there exist no users in practice that would rely on this.
So, hide the structs.
Also, this allows to embed the private data in the GObject struct
itself, which is useful for debugging and for performance.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
list-load does some special handling, for example, it will avoid adding
duplicates. As such, two plugin infos cannot have the same name or
same service type.
nm_vpn_plugin_info_new_search_file() did not implement this, it merely
loaded each directory after the other, sort the plugin infos, and
returned the first match.
That might mean, with unusual (duplicate) name files,
nm_vpn_plugin_info_new_search_file() might return a value
that would not otherwise be returned by
nm_vpn_plugin_info_list_load().
Let nm_vpn_plugin_info_new_search_file() call list-load, so that
the search result is always consistent.
The downside of this is that previously, if the searched plugin was
already found in /usr/lib, we would skip loading /etc. But
that is a minor optimization, in any case nm_vpn_plugin_info_new_search_file()
scales with the number of .name files on disk, which is expected to be small.
|
|\
| |
| |
| | |
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/430
|
| |
| |
| |
| |
| |
| |
| | |
Drop device-specific 'hw-address' GObject properties which are now
redundant.
https://bugzilla.redhat.com/show_bug.cgi?id=1786937
|
|/ |
|
|
|
|
| |
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/436
|
|\
| |
| |
| | |
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/426
|
| |
| |
| |
| |
| | |
Let's not do linear search. Use a hash table to find the AP by D-Bus
path.
|
| |
| |
| |
| |
| | |
We internally track the string as NMRefString. Expose it, so that
users can directly use the reference counted string.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Avoid GDBusProxy, instead use GDBusConnection directly. I very much
prefer this because that way we have explicit control over what happens
on D-Bus. With GDBusProxy this is hidden under another layer of complex
code. The hardest part when using a D-Bus interface is to manage the
state via an asynchronous medium. GDBusProxy contains state about the
D-Bus interface and duplicate the state that we track. This makes it hard
to reason about things.
Rework creation of NMSupplicantInterface. Previously, a NMSupplicantInterface
had multiple initialization states. In particular, the first state would not
yet tie the interface to a certain D-Bus object path. Instead, NMSupplicantInterface
would try and retry to create the D-Bus object.
Now, NMSupplicantManager has an asynchronous method to create interface
instances. The manager only creates an interface instance after the D-Bus
path is known. That means, a NMSupplicantInterface instance is now
strongly tied to a name-owner and D-Bus path.
It follows that the state of NMSupplicantInterface can only go from STARTING,
via the supplicant states, to DOWN. Never back. That was already previously
the case that the state from DOWN was final and once the 3 initial
states were passed, the interface's state would never go back to the initial
state. Now this is more strict and more formalized. The 3 initialization states
are combined.
I think the tighter state handling simplifies users of NMSupplicantInterface.
See for example "nm-device-ethernet.c". It's still complicated, because handling
state is fundamentally difficult.
NMSupplicantManager will take care to D-Bus activate wpa_supplicant only
when necessary (poke). Previously, creating the manager instance
would always start suppliant service. Now, it's started on demand.
|
|/
|
|
|
| |
We should first ref the new instance and emit the notify signal,
before unref the old value. It feels better in this order.
|
|
|
|
| |
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/434
|
|\
| |
| |
| | |
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/424
|
| |
| |
| |
| |
| |
| |
| |
| |
| | |
When creating a new connection before the user gets the chance to modify
the bond options let's just initialize 'mode' and 'miimon' (with related
'updelay' and 'downdelay').
Initializing also 'arp_interval', 'arp_ip_target' and 'primary' doesn't
make much sense as by default they're disabled or contain empty values.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Add 'nm_setting_bond_get_option_normalized()', the purpose of this API
is to retrieve a bond option normalized value which is the option that
NetworkManager will actually apply to the bond when activating the
connection, this takes into account default values for some options that
NM assumes.
For example, if you create a connection:
$ nmcli c add type bond con-name nm-bond ifname bond0 bond.options mode=0
Calling 'nm_setting_bond_get_option_normalized(s_bond, "miimon")' would
return "100" as even if not specified NetworkManager enables miimon for
bond connections.
Another example:
$ nmcli c add type bond con-name nm-bond ifname bond0 bond.options mode=0,arp_interval=100
Calling 'nm_setting_bond_get_option_normalized(s_bond, "miimon")' would
return NULL in this case because NetworkManager disables miimon if
'arp_interval' is set explicitly but 'miimon' is not.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Add '_nm_setting_bond_get_option_or_default()' and move all the custom
policies applied by NM for bond options in there.
One such example of a custom policy is to set 'miimon' to 0 (instead of its
default value of 100) if 'arp_interval' is explicitly enabled
and 'miimon' is not.
This means removing every piece of logic from
nm_setting_bond_add_option() which used to clear out 'arp_interval' and
'arp_ip_target' if 'miimon' was set or clear out 'miimon' along with
'downdelay', 'updelay' and 'miimon' if 'arp_interval' was set.
This behaviour is a bug since the kernel allow setting any combination
of this options for bonds and NetworkManager should not limit the user
to do so.
Also use 'set_bond_attr_or_default()' instead of 'set_bond_attr()' as
the former calls '_nm_setting_bond_get_option_or_default()' to implement
the right logic to retrieve bond options according to current bond
configuration.
|
| |
| |
| |
| |
| |
| |
| |
| | |
Fix 'miimon' and 'arp_interval' validation, they can both be set indeed,
the kernel does not impose this limitation, nevertheless is sensible to
keep the defaults as previously (miimon=100, arp_interval=0).
Also add unit test.
|