1. 23 Sep, 2019 9 commits
    • Thomas Haller's avatar
      shared/trivial: rename _nm_g_slice_free_fcn1() macro to nm_g_slice_free_fcn1() · 32148417
      Thomas Haller authored
      The leading underscore has the notion that this would be a private function.
      It really isn't, and it would be fine for the user to call it directly.
      
      Just like we have g_slice_free() and g_slice_free1().
      32148417
    • Thomas Haller's avatar
      contrib/rpm: "Suggest" wpa_supplicant for NetworkManager-wifi package · 4b1cb404
      Thomas Haller authored
      "NetworkManager-wifi" package requires either wpa_supplicant or iwd.
      When installing the package without explicitly installing supplicant
      or iwd (and not having it installed yet), then we want to drag in
      wpa_supplicant by default. That is accomplished by suggesting wpa_supplicant
      package.
      
      Otherwise, the user installing NetworkManager-wifi might get iwd,
      which is only functioning if the user explicitly enables the backend
      in "NetworkManager.conf".
      
      https://bugzilla.redhat.com/show_bug.cgi?id=1743585
      4b1cb404
    • Thomas Haller's avatar
    • Thomas Haller's avatar
    • Thomas Haller's avatar
      settings: drop redundant check from have_connection_for_device() · 2a506d8a
      Thomas Haller authored
      have_connection_for_device() really should just call nm_device_check_connection_compatible().
      Note that nm_device_check_connection_compatible() of course checks the
      connection type already, so this is redundant.
      2a506d8a
    • Thomas Haller's avatar
      settings: shortcut check for whether to create auto-default wired connection · 3e39d2a5
      Thomas Haller authored
      This check is only useful for devices that implement new_default_connection.
      We can shortcut the possibly expensive checks like have_connection_for_device(),
      which need to iterate all profiles.
      3e39d2a5
    • Thomas Haller's avatar
      core: fix wrongly generating "Wired connection 1" (auto-default) for ethernet with MAC · 5aa50d7c
      Thomas Haller authored
      If a profile has only "ethernet.mac-address" set, but
      "connection.interface-name" not, then the previous check
      
          iface = nm_setting_connection_get_interface_name (s_con);
          if (!nm_streq0 (iface, nm_device_get_iface (device)))
               continue;
      
      would wrongly consider the profile not matching for the device.
      As a result, we would wrongly create a auto-default connection.
      
      Fix that. We already call nm_device_check_connection_compatible()
      above. That is fully suitable to compare the interface name and
      the MAC address. We don't need to duplicate this check (wrongly).
      
      See also commit 77d01c90 ('settings: ignore incompatible connections
      when looking for existing ones') for how this code changed.
      
      https://bugzilla.redhat.com/show_bug.cgi?id=1727909
      5aa50d7c
    • Thomas Haller's avatar
    • Thomas Haller's avatar
      bluetooth: refactor BlueZ handling and let NMBluezManager cache ObjectManager data · 4154d961
      Thomas Haller authored
      This is a complete refactoring of the bluetooth code.
      
      Now that BlueZ 4 support was dropped, the separation of NMBluezManager
      and NMBluez5Manager makes no sense. They should be merged.
      
      At that point, notice that BlueZ 5's D-Bus API is fully centered around
      D-Bus's ObjectManager interface. Using that interface, we basically only
      call GetManagedObjects() once and register to InterfacesAdded,
      InterfacesRemoved and PropertiesChanged signals. There is no need to
      fetch individual properties ever.
      
      Note how NMBluezDevice used to query the D-Bus properties itself by
      creating a GDBusProxy. This is redundant, because when using the ObjectManager
      interfaces, we have all information already.
      
      Instead, let NMBluezManager basically become the client-side cache of
      all of BlueZ's ObjectManager interface. NMBluezDevice was mostly concerned
      about caching the D-Bus interface's state, tracking suitable profiles
      (pan_connection), and moderate between bluez and NMDeviceBt.
      These tasks don't get simpler by moving them to a seprate file. Let them
      also be handled by NMBluezManager.
      
      I mean, just look how it was previously: NMBluez5Manager registers to
      ObjectManager interface and sees a device appearing. It creates a
      NMBluezDevice object and registers to its "initialized" and
      "notify:usable" signal. In the meantime, NMBluezDevice fetches the
      relevant information from D-Bus (although it was already present in the
      data provided by the ObjectManager) and eventually emits these usable
      and initialized signals.
      Then, NMBlue5Manager emits a "bdaddr-added" signal, for which NMBluezManager
      creates the NMDeviceBt instance. NMBluezManager, NMBluez5Manager and
      NMBluezDevice are strongly cooperating to the point that it is simpler
      to merge them.
      
      This is not mere refactoring. This patch aims to make everything
      asynchronously and always cancellable. Also, it aims to fix races
      and inconsistencies of the state.
      
      - Registering to a NAP server now waits for the response and delays
        activation of the NMDeviceBridge accordingly.
      
      - For NAP connections we now watch the bnep0 interface in platform, and tear
        down the device when it goes away. Bluez doesn't send us a notification
        on D-Bus in that case.
      
      - Rework establishing a DUN connection. It no longer uses blocking
        connect() and does not block until rfcomm device appears. It's
        all async now. It also watches the rfcomm file descriptor for
        POLLERR/POLLHUP to notice disconnect.
      
      - drop nm_device_factory_emit_component_added() and instead let
        NMDeviceBt directly register to the WWan factory's "added" signal.
      4154d961
  2. 22 Sep, 2019 13 commits
    • Thomas Haller's avatar
      bluetooth/tests: add "nm-bt-test helper" program for manual testing of bluetooth code · 878d4963
      Thomas Haller authored
      Just add a stub implementation and let it build. More will be added
      later.
      878d4963
    • Thomas Haller's avatar
    • Thomas Haller's avatar
      wwan: simplify API of nm_modem_act_stage2_config() to never fail · d6df0339
      Thomas Haller authored
      The previous function arguments of nm_modem_act_stage2_config() act as if the
      function could fail or even postpone the action. It never did.
      
      We cannot treat this generic. A caller needs to know whether nm_modem_act_stage2_config()
      can postpone the action, and when it does, which signal is emitted upon completion. That
      is, the caller needs to know how to proceed after postponing.
      
      In other words, since this function currently cannot fail or postpone
      the stage, so must all callers already rely on that. At this point it makes
      no sense to pretend that the function could be any different, if all callers
      assume it is not. Simplify the API.
      d6df0339
    • Thomas Haller's avatar
      wwan: add nm_modem_manager_get_modems() to iterate over modems · f796be3d
      Thomas Haller authored
      Currently, we cannot ask which modems exist. NMDeviceBt may claim it
      via nm_device_factory_emit_component_added(), and NMWWanFactory may
      take it by listening to NM_MODEM_MANAGER_MODEM_ADDED. But that's it.
      
      We will drop nm_device_factory_emit_component_added() because it's only
      used for passing modems to NMDeviceBt. Instead, NMDeviceBt can directly
      subscribe to NM_MODEM_MANAGER_MODEM_ADDED. It already has a reference
      to NMModemManager.
      
      Anyway, the NM_MODEM_MANAGER_MODEM_ADDED signal is no enough, because
      sometimes when the mode appears, NMDeviceBt might not yet know whether
      it should take it (because the DUN connect call is not yet complete).
      Currently that never happens because dun_connect() blocks waiting for
      the device. That must be fixed, by not waiting. But this opens up a
      race, and NMDeviceBt might after NM_MODEM_MANAGER_MODEM_ADDED need to
      search for the suitable modem: by iterating the list of all modems.
      f796be3d
    • Thomas Haller's avatar
      wwan: mark modems that are taken by a NMDevice as "claimed" · 6d644c66
      Thomas Haller authored
      NMModem-s are either used by NMDeviceModem or by NMDeviceBt.
      The mechanism how that is coordinated it odd:
      
        - the factory emits component-added, and then NMDeviceBt
          might take the device (and claim it). In that case, component-added
          would return TRUE to indicate that the modem should not be also
          used by NMDeviceModem.
      
        - next, if the modem has a driver that looks like bluetooth, NMDeviceModem
          ignores it too.
      
        - finally, NMDeviceModem claims the modem (which is now considered to
          be non-bluetooth).
      
      I think the first problem is that the device factory tries to have this
      generic mechanism of "component-added". It's literally only used to
      cover this special case. Note that NMDeviceBt is aware of modems. So,
      abstracting this just adds lots of code that could be solved better
      by handling the case (of giving the modem to either NMDeviceBt or
      NMDeviceModem) specifically.
      
      NMWWanFactory itself registers to the NM_MODEM_MANAGER_MODEM_ADDED
      signal and emits nm_device_factory_emit_component_added().
      We could just have NMWWanFactory and NMDeviceBt both register to
      that signal. Signals even support priorities, so we could have
      NMDeviceBt be called first to claim the device.
      
      Anyway, as the modem can only have one owner, the modem should have
      a flag that indicates whether it's claimed or not. That will allow
      multiple components all look at the same modem and moderate who is
      going to take ownership.
      6d644c66
    • Thomas Haller's avatar
      core: extend nm_shutdown_wait_obj_*() to support notification via a GCancellable · eae69e33
      Thomas Haller authored
      Now nm_shutdown_wait_obj_*() supports two styles:
      
       - NM_SHUTDOWN_WAIT_TYPE_OBJECT: this just registers a weak pointer
         on a source GObject. As long as the object is not destroyed
         (and the object is not unregistered), the shutdown gets blocked.
      
       - now new is NM_SHUTDOWN_WAIT_TYPE_CANCELLABLE: this source object
         is a GCancellable, and during shutdown, the system will cancel
         the instances to notify about the shutdown. That aside, the GCancellable
         is tracked exactly like a regular NM_SHUTDOWN_WAIT_TYPE_OBJECT (meaning:
         a weak pointer is registered and shutdown gets delayed as long as the instance
         lives).
      
      As the rest of the shutdown, it's not yet implemented on the shutdown-side.
      What is now possible is to register such cancellables, so that users can make
      use of this API before we fix shutdown. We cannot fix it all at the same time,
      so first users must be ready for this approach.
      eae69e33
    • Thomas Haller's avatar
      core: add and use NM_MANAGER_GET macro · 5131cc42
      Thomas Haller authored
      For our singleton getters we usually have such a macro. See NM_PLATFORM_GET
      and NM_SETTINGS_GET.
      
      Add such a macro for NMManager and use it.
      5131cc42
    • Thomas Haller's avatar
    • Thomas Haller's avatar
    • Thomas Haller's avatar
      e688e70b
    • Thomas Haller's avatar
      2667a468
    • Thomas Haller's avatar
      6a9ab7bb
    • Thomas Haller's avatar
  3. 21 Sep, 2019 8 commits
    • Thomas Haller's avatar
      libnm/test: add unit test with consistency checks about NMSetting type info · d19a403f
      Thomas Haller authored
      Add test for checking the meta data for expected consistency.
      
      This is also useful if you want to check something about the meta data
      programatically.
      
      For example, if you have the question which (if any) properties
      are GObject based but also implement a to_dbus_fcn() function. Then you
      can extend this code with some simple printf debugging to get a list of
      those.
      
      Or, if you want to find how many NMSettInfoProperty instances are in
      static data (e.g. to determine how much memory is used). You can easily
      modify this code to count them (and find 447 properties). Out of these,
      326 are plain GObject based properties. Meaning, we could refactor the
      code to create smaller NMSettInfoProperty instances for those, saving
      thus (326 * 4 * sizeof (gpointer)) bytes (10K).
      
      Such questions are interesting when refactoring the code.
      d19a403f
    • Thomas Haller's avatar
    • Thomas Haller's avatar
      libnm: drop unused property setter NM_DEVICE_MANAGED · 30ce0079
      Thomas Haller authored
      NM_DEVICE_MANAGED was intended to work like NM_DEVICE_AUTOCONNECT:
      namely it would call the D-Bus property setter synchronously.
      
      But such behavior is horrendous, we certainly don't want blocking calls
      during a property getter.
      
      Luckily this one instance was unused and never worked as the property
      was marked as G_PARAM_READABLE. Just drop the setter.
      30ce0079
    • Thomas Haller's avatar
      libnm: inline NMManager's get_permissions_sync() · 7db00be6
      Thomas Haller authored
      Synchrnous initialization is problmatic and needs cleanup.
      
      get_permissions_sync() is an internal function, that has only one
      caller. We need to keep track of functions that make synchronous D-Bus
      calls. Move the synchronous call into the caller, so that it's clearer
      who calls such API.
      7db00be6
    • Thomas Haller's avatar
      libnm: drop _nm_dbus_is_connection_private() · 0f9157f0
      Thomas Haller authored
      Currently, we don't use private sockets. We are always connected
      to D-Bus.
      0f9157f0
    • Thomas Haller's avatar
      libnm: drop nm_dbus_new_connection() helper API · be3712f6
      Thomas Haller authored
      We don't need a wrapper around g_bus_get*(). Just use
      it directly.
      
      I guess in the past this had some use when we were using
      a private socket too. Those days are gone. If we are going
      to re-introduce private socket support, then we probably should
      come up with a better solution.
      be3712f6
    • Thomas Haller's avatar
      shared: add NMRefString · 908fadec
      Thomas Haller authored
      I'd like to refactor libnm's caching. Note that cached D-Bus objects
      have repeated strings all over the place. For example every object will
      have a set of D-Bus interfaces (strings) and properties (strings) and an
      object path (which is referenced by other objects). We can save a lot of
      redundant strings by deduplicating/interning them. Also, by interning
      them, we can compare them using pointer equality.
      
      Add a NMRefString implementation for this.
      
      Maybe an alternative name would be NMInternedString or NMDedupString, because
      this string gets always interned. There is no way to create a NMRefString
      that is not interned. Still, NMRefString name sounds better. It is ref-counted
      after all.
      
      Notes:
      
       - glib has GQuark and g_intern_string(). However, such strings cannot
         be unrefered and are leaked indefinitely. It is thus unsuited for
         anything but a fixed set of well-known strings.
      
       - glib 2.58 adds GRefString, but we cannot use that because we
         currently still use glib 2.40.
         There are some differences:
      
           - GRefString is just a typedef to char. That means, the glib API
             exposes GRefString like regular character strings.
             NMRefString intentionally does that not. This makes it slightly
             less convenient to pass it to API that expects "const char *".
             But it makes it clear to the reader, that an instance is in fact
             a NMRefString, which means it indicates that the string is
             interned and can be referenced without additional copy.
      
           - GRefString can be optionally interned. That means you can
             only use pointer equality for comparing values if you know
             that the GRefString was created with g_ref_string_new_intern().
             So, GRefString looks like a "const char *" pointer and even if
             you know it's a GRefString, you might not know whether it is
             interned. NMRefString is always interned, and you can always
             compare it using pointer equality.
      
        - In the past I already proposed a different implementation for a
          ref-string. That made different choices. For example NMRefString
          then was a typedef to "const char *", it did not support interning
          but deduplication (without a global cache), ref/unref was not
          thread safe (but then there was no global cache so that two threads
          could still use the API independently).
      
      The point is, there are various choices to make. GRefString, the
      previous NMRefString implementation and the one here, all have pros and
      cons. I think for the purpose where I intend NMRefString (dedup and
      efficient comparison), it is a preferable implementation.
      
      Ah, and of course NMRefString is an immutable string, which is a nice
      property.
      908fadec
    • Thomas Haller's avatar
  4. 20 Sep, 2019 6 commits
  5. 18 Sep, 2019 4 commits