1. 29 May, 2018 4 commits
    • Beniamino Galvani's avatar
      core: vlan: avoid unneeded casts · b6b158e3
      Beniamino Galvani authored
      b6b158e3
    • Beniamino Galvani's avatar
      n-acd: slightly improve logging · 7f6a19b1
      Beniamino Galvani authored
      If timeout is 0 we don't really do a probe. Also, log the timeout.
      7f6a19b1
    • Beniamino Galvani's avatar
      n-acd: better handle interfaces going temporarily down · d082af6b
      Beniamino Galvani authored
      NM sometimes brings an interface temporarily down (for example to
      change a VLAN MAC to align it to the parent interface's one). When
      this happens, any recv() or send() in n-acd fails, the n-acd instance
      is reset to the initial state and a DOWN event is reported to the
      manager, which currently does not handle it. The result is an
      inconsistent state.
      
      There is no simple way of dealing with the DOWN event in the
      manager. What we can do instead is to:
      
       - ignore errors during recv() because there is really nothing we can
         do, except for waiting timeouts to expire;
      
       - during probe, ignore errors during send() so that we don't exceed
         the probe timeout;
      
       - during announcement, retry after a send() error to ensure we send
         all 3 announcements.
      
      https://bugzilla.redhat.com/show_bug.cgi?id=1578675
      d082af6b
    • Beniamino Galvani's avatar
      n-acd: use RFC 5227 timeout for announcements · 2f4b3392
      Beniamino Galvani authored
      When doing announcements, use the the timeout specified by RFC
      5227. Note that timeout_multiplier might be 0.
      
      This aligns behavior to upstream version of n-acd.
      2f4b3392
  2. 28 May, 2018 14 commits
    • Aleksander Morgado's avatar
      policy: don't block connection if device is gone · d97eab6c
      Aleksander Morgado authored
      If the active connection is deactivated because the device is gone,
      don't block autoconnection. Otherwise, whenever the device comes
      back (e.g. maybe it was reset in the middle of a connection attempt),
      the autoconnection logic won't be triggered, as the settings are still
      blocked.
      
      I'm able to reproduce this by performing a WWAN modem reset in the
      middle of a connection attempt.
      
      https://github.com/NetworkManager/NetworkManager/pull/121
      d97eab6c
    • Thomas Haller's avatar
      clients/tests: run nmcli commands in parallel · baaab522
      Thomas Haller authored
      Most nmcli calls from clients/tests don't change the server's state.
      Hence, they can easily run in parallel.
      
      Run tests in parallel. No longer handle one nmcli invocation after the other.
      Instead, spawn groups of processes in parallel, and track the pending jobs.
      
      Only at certain synchronization points we call self.async_wait() to
      wait for all previous jobs to complete.
      
      This reduces the test time on my machine from 7 to 3 seconds. Arguably,
      that matters less during a full `make check -j 8`, because the entire
      set of tests anyway takes longer than 7 seconds. So when running the
      entire test suite, the machine is kept busy anyway. It matters however
      for manual invocations.
      baaab522
    • Thomas Haller's avatar
    • Thomas Haller's avatar
      all: add stable-id specifier "${DEVICE}" · eb821ead
      Thomas Haller authored
      Add new stable-id specifier "${DEVICE}" to explicitly declare that the
      connection's identity differs per-device.
      
      Note that for settings like "ipv6.addr-gen-mode=stable" we already hash
      the interface's name. So, in combination with addr-gen-mode, using this
      specifier has no real use. But for example, we don't do that for
      "ipv4.dhcp-client-id=stable".
      Point being, in various context we possibly already include a per-device
      token into the generation algorithm. But that is not the case for all
      contexts and uses.
      
      Especially the DHCPv4 client identifier is supposed to differ between interfaces
      (according to RFC). We don't do that by default with "ipv4.dhcp-client-id=stable",
      but with "${DEVICE}" can can now be configured by the user.
      Note that the fact that the client-id is the same accross interfaces, is not a
      common problem, because profiles are usually restricted to one device via
      connection.interface-name.
      eb821ead
    • Thomas Haller's avatar
      device: hash a per-host key for ipv4.dhcp-client-id=stable · d1a94a85
      Thomas Haller authored
      Otherwise, the generated client-id depends purely on the profile's
      stable-id. It means, the same profile (that is, either the same UUID
      or same stable-id) on different hosts will result in identical client-ids.
      
      That is clearly not desired. Hash a per-host secret-key as well.
      
      Note, that we don't hash the interface name. So, activating the
      profile on different interfaces, will still yield the same client-id.
      But also note, that commonly a profile is restricted to one device,
      via "connection.interface-name".
      
      Note that this is a change in behavior. However, "ipv4.dhcp-client-id=stable"
      was only added recently and not yet released.
      
      Fixes: 62a78639
      d1a94a85
    • Thomas Haller's avatar
      core: let nm_utils_secret_key_read() handle failures internally · dbcb1d6d
      Thomas Haller authored
      and add nm_utils_secret_key_get() to cache the secret-key, to only
      obtain it once.
      
      nm_utils_secret_key_read() is not expected to fail. However, in case
      of an unexpected error, don't propagate the error to the caller,
      but instead handle it internally.
      
      That means, in case of error:
        - log a warning within nm_utils_secret_key_read() itself.
        - always return a generated secret-key. In case of error, the
          key won't be persisted (obviously). But the caller can ignore
          the error and just proceed with an in-memory key.
      
      Hence, also add nm_utils_secret_key_get() to cache the key. This way,
      we only try to actually generate/read the secret-key once.  Since that
      might fail and return an in-memory key, we must for future invocations
      return the same key, without generating a new one.
      dbcb1d6d
    • Thomas Haller's avatar
      core: ensure NUL terminated secret_key buffer · d0f1dc65
      Thomas Haller authored
      The secret_key is binary random data, so one shouldn't ever use it as a
      NUL terminated string directly.
      
      Still, just ensure that the entire buffer is always NUL terminated.
      d0f1dc65
    • Thomas Haller's avatar
      5f5f75ce
    • Thomas Haller's avatar
      libnm: allow nm_utils_hwaddr_valid() of length zero · 4e463df1
      Thomas Haller authored
      nm_utils_hwaddr_valid() is used for validating strings. It should
      not assert against calling it with an empty string "". That is just
      an invalid hwaddr.
      4e463df1
    • Lubomir Rintel's avatar
      rpm/build.sh: suggest dnf builddep after BUILDTYPE=SRPM · c28b334f
      Lubomir Rintel authored
      Convenient for copy & paste.
      c28b334f
    • Lubomir Rintel's avatar
      rpm: prefer python3 · de326a4e
      Lubomir Rintel authored
      From Fedora 28 on we can build without Python 2. That is good,
      because it's eventually going to be removed.
      
      Based on a change in Fedora dist-git by Iryna Shcherbina.
      de326a4e
    • Beniamino Galvani's avatar
      cli: fix property matching · 1f7780cb
      Beniamino Galvani authored
      @ret was not initialized when there was only one partial match.
      
      Also, refactor the code to return all matching values.
      
      Fixes: 3fd9bf9d
      
      https://github.com/NetworkManager/NetworkManager/pull/123
      1f7780cb
    • Beniamino Galvani's avatar
      af946871
    • Thomas Haller's avatar
      clients/tests: refactor by moving replace-text helper function · 78e877ee
      Thomas Haller authored
      Have less logic in TestNmcli.
      78e877ee
  3. 27 May, 2018 1 commit
    • Thomas Haller's avatar
      clients/tests: generate Makefile.am for expected files · ee85151a
      Thomas Haller authored
      The developer can re-generate .expected files with
      
       $ NM_TEST_REGENERATE=1 ./clients/tests/test-client.py
      
      Note that these files are also dist-ed, so that the tests also work
      from a source-tarball. For that, we need to add them to EXTRA_DIST.
      
      Previously, this was done manually in the base Makefile.am file. This
      was cumbersome, because when adding a new test, the developer would need
      to manually add the files.
      
      Now, let the test (with NM_TEST_REGENERATE=1) also generate a makefile
      part.
      ee85151a
  4. 26 May, 2018 9 commits
  5. 25 May, 2018 8 commits
    • Thomas Haller's avatar
      cli: avoid bogus line seprator in pretty-output · e1bbc2e1
      Thomas Haller authored
      This bug resulted in spurious lines with "--pretty --mode tabular",
      whenever nmc_print() was called with multiple rows.
      
      Currently, the only case where this was visible was with:
      
        $ nmcli --pretty general permissions
      
      (note that "--mode tabular" is the default).
      
      Fixes: 16299e5a
      e1bbc2e1
    • Thomas Haller's avatar
    • Thomas Haller's avatar
    • Thomas Haller's avatar
      core: add and use NM_SHUTDOWN_TIMEOUT_MS as duration that we plan for shutdown · eaf36db6
      Thomas Haller authored
      nm_ppp_manager_stop() wants to ensure that the pppd process is really
      gone. For that it uses nm_utils_kill_child_async() to first send
      SIGTERM, and sending SIGKILL after a timeout.
      
      Later, we want to fix shutdown of NetworkManager to iterate the mainloop
      during shutdown, so that such operations are still handled. However, we
      can only delay shutdown for a certain time. After a timeout (NM_SHUTDOWN_TIMEOUT_MS
      plus NM_SHUTDOWN_TIMEOUT_MS_GRACE) we really have to give up and
      terminate.
      
      That means, the right amount of time between sending SIGTERM and SIGKILL
      is exactly NM_SHUTDOWN_TIMEOUT_MS. Hopefully that is of course
      sufficient in the first place. If not, send SIGKILL afterwards, and give
      a bit more time (NM_SHUTDOWN_TIMEOUT_MS_GRACE) to reap the child.
      And if all this time is still not enough, something is really odd and we
      abort waiting, with a warning in the logfile.
      
      Since we don't properly handle shutdown yet, the description above is
      not really true. But with this patch, we fix it from point of view of
      NMPPPManager.
      eaf36db6
    • Thomas Haller's avatar
      ppp-manager: rework stopping NMPPPManager by merging async/sync methods · 43f67b42
      Thomas Haller authored
      Previously, there were two functions nm_ppp_manager_stop_sync() and
      nm_ppp_manager_stop_async().
      
      However, stop-sync() would still kill the process asynchronously (with a
      2 seconds timeout before sending SIGKILL).
      
      On the other hand, stop-async() did pretty much the same thing as
      sync-code, except also using the GAsyncResult.
      
      Merge the two functions. Stopping the instance for the most part can be
      done entirely synchrnous. The only thing that is asynchronous, is
      to wait for the process to terminate. For that, add a new callback
      argument to nm_ppp_manager_stop(). This replaces the GAsyncResult
      pattern.
      
      Also, always ensure that NetworkManager runs the mainloop at least as
      long until the process really terminated. Currently we don't get that
      right, and during shutdown we just stop iterating the mainloop. However,
      fix this from point of view of NMPPPManager and register a wait-object,
      that later will correctly delay shutdown.
      
      Also, NMDeviceWwan cared to wait (asynchronously) until pppd really
      terminated. Keep that functionality. nm_ppp_manager_stop() returns
      a handle that can be used to cancel the asynchrounous request and invoke
      the callback right away. However note, that even when cancelling the
      request, the wait-object that prevents shutdown of NetworkManager is
      kept around, so that we can be sure to properly clean up.
      43f67b42
    • Thomas Haller's avatar
      ppp-manager/trivial: rename variables holding self pointers · 53d04a1d
      Thomas Haller authored
      We usually structure our code in a (pseudo) object oriented way.
      It makes sense to call the variable for the target object "self",
      it is more familiar and usually done.
      53d04a1d
    • Thomas Haller's avatar
      ppp-manager: refactor killing pppd process by using _ppp_kill() function · 51566351
      Thomas Haller authored
      - add callback arguments to _ppp_kill(). Although most callers don't
        care, it makes it more obvious that this kills the process
        asynchronously.
      
      - the call to nm_utils_kill_child_async() is complicated, with many
        arguments. Only call it from one place, and re-use the simpler wrapper
        function _ppp_kill() everywhere.
      51566351
    • Thomas Haller's avatar
      core: add nm_shutdown_register_watchdog() for marking object to wait for shutdown · f09e7797
      Thomas Haller authored
      Eventually we should do a coordinated shutdown when NetworkManager exits.
      That is a large work, ensuring that all asynchronous actions are cancellable
      (in time), and that we wait for all pending operations to complete.
      
      Add a function nm_shutdown_register_watchdog(), so that objects can register
      themselves, to block shutdown until they are destroyed. It's not yet used,
      because actually iterating the mainloop during shutdown can only be done,
      once the code is prepared to be ready for that. But we already need the
      function, so that we can refactor individual parts of the code, in preparation
      of using it in the future.
      f09e7797
  6. 24 May, 2018 4 commits