GitLab

nmtui is a TUI (curses-based Text User Interface) for NetworkManager

This fixes automatic activation after changes in
commit ff7e47a4.

When a connection is deactivated impl_manager_deactivate_connection() is called
and the device goes to NM_DEVICE_STATE_DISCONNECTED. nm_device_state_changed()
then issues "state-changed" signal. The signal is connected to by various
listeners. The most interesting ones for this case are NMPolicy and
NMActiveConnection.
The problem is that NMPolicy's device_state_changed() is processed first and
thus in schedule_activate_check() we still have the old active connection
present (in ACTIVATED state).

This commit fixes the issue by connecting to "state-changed" signal using
g_signal_connect_after() in NMPolicy. This ensures NMPolicy's state-changed
handler is called after active connections are processed.

https://bugzilla.redhat.com/show_bug.cgi?id=1033187

The call is redundant, because the device will transition to DISCONNECTED
and schedule_activate_check() will be called of this state.

- refactor register_settings to allow lookup by GType and
  add the settings name to SettingInfo.

- setting NM_SETTING_NAME is deprecated and should not be set anymore.
  Indeed it has always be a bug, to reset the name to a different value.
  The only valid place to set the name was in the _init() function of
  the derived class itself.
  This is now no longer needed/possible. Instead the name get's
  detected based on the registered setting types. This makes use of
  the registered metadata that is available anyway since every
  usable setting has to register itself.
Signed-off-by: Thomas Haller <thaller@redhat.com>

If NetworkManager is the first thing to bring an interface up, the interface may not
know its carrier state for a few seconds.  We need to block startup completion until
we're reasonably sure that all initial devices know their carrier state, so that we
can start the initial connection activation on those devices before startup is
complete.

Second, there was a small race between when the Policy decided to auto-activate
a device, and when the device actually added the pending action that blocked
startup completion, during which startup could be declared complete but actually
wasn't.  Fix that.

The NMActiveConnection class tracks the full activation request, and internal
activation requests go through the same process as external ones, including
some authentication.  Sometimes that means activation is scheduled, control
returns to the mainloop, and then the activation proceeds from an idle
handler.

Unfortunately, that means that adding a pending "activation" action from
nm-device.c doesn't always work, since there is a short window between when
the activation is started in nm-manager.c (in nm_manager_activate_connection())
and when the device actually changes state.  Inside that window, the pending
actions may drop to zero, and startup will be declared complete before the
device actually starts activating.

Instead, ensure that the pending action is added when the internal activation
is actually started (eg, when NMActiveConnection receives the NMDevice object).

Carrier state is only valid if the network interface is IFF_UP, because drivers
are not required to do carrier detection if the device is not up.  Thus, if NM
is the first process to set the interface IFF_UP, there may be a short delay
while the driver performs carrier detection.  NetworkManager must suppress
"startup complete" during this delay to ensure that the carrier state is known
before making startup property decisions.

Previously, when NetworkManager set the interface IFF_UP, the interface would
not have a carrier for a few seconds until the driver's carrier detection was
done.  Since the interface had no carrier, NetworkManager could not begin
connection activation on the interface, and the interface would not suppress
the "startup complete" transition.  Thus, NetworkManager would declare that
startup was complete prematurely and anything depending on startup network
connectivity would fail as no interfaces were active.

https://bugzilla.redhat.com/show_bug.cgi?id=1034921
https://bugzilla.redhat.com/show_bug.cgi?id=1030583

This lets us do two things:

1) ensure that pending actions are unique and not doubly added/removed
2) we can (eventually) print out the pending action list for debugging

However, since we cannot have two pending actions with the same name at
the same time, we need to change the queued device state actions to
include the state name.  But that makes debugging even more descriptive
so it's a bonus.

https://bugzilla.gnome.org/show_bug.cgi?id=720326Signed-off-by: Thomas Haller <thaller@redhat.com>

This reworks and fixes the handling of current_ap in nm-device-wifi.

https://bugzilla.redhat.com/show_bug.cgi?id=1025371Signed-off-by: Thomas Haller <thaller@redhat.com>

Signed-off-by: Thomas Haller <thaller@redhat.com>

Signed-off-by: Thomas Haller <thaller@redhat.com>

Simplify check in nm_ap_set_ssid(). Note that previously there was
no bug here in case of self assignment, this just makes it more
explicit.
Signed-off-by: Thomas Haller <thaller@redhat.com>

We should use ap_scan=1 *except* for AP/IBSS/AdHoc, where ap_scan=2 is
required.  ap_scan for "infra" mode is all historical and was for old,
crappy, and proprietary drivers that we should really stop hacking stuff
for.  Those drivers did not support probe-scanning for hidden APs and
thus the supplicant just had to send all the config to the driver and
hope things worked.

All relevant and non-crappy drivers these days support at least one SSID
probe and thus is_broadcast affecting ap_scan should no longer be
something we support.  If you have an old, crappy
WEXT/proprietary/staging driver, and you use hidden APs, you're doing it
wrong.

So, in short, we must keep the ap_scan=2 logic for AP+AdHoc, but we can
remove the is_broadcast and has_scan_capa_ssid arguments and the code
where they change ap_scan.

https://bugzilla.redhat.com/show_bug.cgi?id=1025371#c18Signed-off-by: Thomas Haller <thaller@redhat.com>

rh #1025371 reports a crash in handle_ip_config_timeout() because
nm_device_wifi_get_activation_ap() did not return any access point
(line nm-device-wifi.c:3105).

In order to fix this, the whole handling of get_activation_ap vs.
current_ap was reworked and cleaned up.

This also fixes a memory leak in line nm-device-wifi.c:2111.

Also rename the functions get_active_ap (to find_active_ap) and
set_active_ap (to set_current_ap), because these two functions were not
getter/setter for an 'active_ap' property (as would be expected from the
previous name).

Also ensure, that a fake AP is never in the list of valid APs without
also being the current_ap. Whenever we reset a fake current_ap, the AP
gets removed.

https://bugzilla.redhat.com/show_bug.cgi?id=1025371Signed-off-by: Thomas Haller <thaller@redhat.com>

This reverts commit 788eed99.

Revert the previous workaround for the crash before cleanup the handling
of AP in nm-device-wifi.c
Signed-off-by: Thomas Haller <thaller@redhat.com>

It is an extension compared to initscripts (not in sysconfig.txt). But it is
necessary for preserving dhcp-send-hostname. Missing DHCP_SEND_HOSTNAME is
treated as "yes", which matches dhcp-send-hostname default value being TRUE.

https://bugzilla.redhat.com/show_bug.cgi?id=1001529

It is better to leave it to user whether he wants to enable sending hostname,
because he probably disabled it manually (dhcp-send-hostname is TRUE by default).
Also, this would not work for plugins that read and set dhcp-hostname after
dhcp-send-hostname.

https://bugzilla.redhat.com/show_bug.cgi?id=1001529

Workaround a serious issue, that a connection that failed to activate
might retry to autoconnect indefinitly.

In NMPolicy, device_state_changed() decrements the retry count for
autoconnect. But immediatly it calls nm_connection_clear_secrets(),
which in turn triggers an NM_SETTINGS_SIGNAL_CONNECTION_UPDATED signal.
The problem is, that connection_updated() resets the try count again to
the default, and thus, the counter was effictivly not decremented.

For now, do not reset the retry count in connection_updated(). This
works arount the issue, but means, that when a user changes the
connection, it is not immediatly retried to autoconnect (as the intent
originally was). This will be fixed later.

https://bugzilla.redhat.com/show_bug.cgi?id=1040528Signed-off-by: Thomas Haller <thaller@redhat.com>

Attempting an immediate reconnect if the peer terminates the connection
sometimes results in the peer not being ready to negotiate a new
connection, while a short delay allows the peer to correctly tear
down the old connection and get listen for a new one.  Introduce
a short delay when activating a PPPoE connection if a PPPoE
connection was recently deactivated.

https://bugzilla.redhat.com/show_bug.cgi?id=1023503
https://bugzilla.redhat.com/show_bug.cgi?id=602265

Rebased to master by jklimes.

`make check` '--with-valgrind=yes' failed due to memory leaks detected
by valgrind. These leaks originate from glib structures, and should be
ignored.

https://bugzilla.gnome.org/show_bug.cgi?id=705160Signed-off-by: Thomas Haller <thaller@redhat.com>

Keyfile plugin writer had a bug, when writing IP6 routes with gateway
"::". Instead of writing "net/plen,,metric" it wrote "net/plen,metric".

- fix this bug and add test cases. Also, add a workaround to reader, to
  accept such wrongly written IP6 routes as valid.

- change the writer for IP4 addresses, IP4 routes and IP6 routes to
  omit the gateway and the metric, if it is 0.0.0.0/::/0, respectively.
  Also change the reader, to accept such empty gateway as valid.
  It only omits the gateway, if the metric is not 0, this means it would
  write:
      route1=1.2.3.4/24,0.0.0.0,1
  instead of
      route1=1.2.3.4/24,,1
  Both representations are now supported by the reader, but older plugin
  versions could only read the former (thus, we keep writing that
  version).
  With a metric of zero, it would instead write:
      route1=1.2.3.4/24

- some refactoring and code cleanup. Fix a memory leak.

https://bugzilla.gnome.org/show_bug.cgi?id=719851Signed-off-by: Thomas Haller <thaller@redhat.com>

Signed-off-by: Thomas Haller <thaller@redhat.com>

Signed-off-by: Thomas Haller <thaller@redhat.com>

Signed-off-by: Thomas Haller <thaller@redhat.com>

ip6_addr_to_string did assume, that inet_ntop might write a scope id to
the result. But it does not (and cannot, because struct in6_addr does
not have any interface identifer).

Simplify and rework the function.

Also fix a memory leak.

https://bugzilla.gnome.org/show_bug.cgi?id=711684Signed-off-by: Thomas Haller <thaller@redhat.com>

https://bugzilla.gnome.org/show_bug.cgi?id=711684Signed-off-by: Thomas Haller <thaller@redhat.com>

https://bugzilla.gnome.org/show_bug.cgi?id=711684Signed-off-by: Thomas Haller <thaller@redhat.com>

Commit 6abc7b78 introduced a
bug in nm_connection_diff() by not reading the property value with
g_object_get_property().
Signed-off-by: Thomas Haller <thaller@redhat.com>

Because it's not trivial to generate a connection that exactly matches
one which was applied by NetworkManager before a restart, we need to
make matching somewhat fuzzier.  Mark any setting property that can be
read from the system or kernel as INFERRABLE, and match only on those
properties when trying to find the persistent connection (if any) which
is already active on that device.

https://bugzilla.redhat.com/show_bug.cgi?id=1029859

nm_connection_diff must also use the virtual functions like
nm_connection_compare. This way, settings can overwrite the default
comparison of individual properties.
Signed-off-by: Thomas Haller <thaller@redhat.com>