1. 11 Apr, 2016 2 commits
  2. 01 Mar, 2016 2 commits
    • Thomas Haller's avatar
      platform: add flags argument to nm_platform_ip4_address_add() · 684e80b5
      Thomas Haller authored
      The argument is still always unset. We will need it later to set
      IFA_F_NOPREFIXROUTE.
      684e80b5
    • Thomas Haller's avatar
      core: split "nm-core-utils.h" out of "NetworkManagerUtils.h" · adb56d13
      Thomas Haller authored
      "NetworkManagerUtils.h" contains a bunch of helper tools for core
      daemon ("src/").
      
      Unfortunately, it has dependencies to other parts of core,
      such as "nm-device.h" and "nm-platform.h". Split out a part
      of tools that are independent so that they can be used without
      dragging in other dependencies.
      
      "nm-core-utils.h" should only use libnm-core, "nm-logging.h"
      and shared.
      
      "NetworkManagerUtils.h" should provide all "nm-core-utils.h" and
      possibly other utilities that have larger dependencies.
      adb56d13
  3. 29 Feb, 2016 3 commits
  4. 19 Feb, 2016 1 commit
    • Thomas Haller's avatar
      all: cleanup includes and let "nm-default.h" include "config.h" · 8bace23b
      Thomas Haller authored
      - All internal source files (except "examples", which are not internal)
        should include "config.h" first. As also all internal source
        files should include "nm-default.h", let "config.h" be included
        by "nm-default.h" and include "nm-default.h" as first in every
        source file.
        We already wanted to include "nm-default.h" before other headers
        because it might contains some fixes (like "nm-glib.h" compatibility)
        that is required first.
      
      - After including "nm-default.h", we optinally allow for including the
        corresponding header file for the source file at hand. The idea
        is to ensure that each header file is self contained.
      
      - Don't include "config.h" or "nm-default.h" in any header file
        (except "nm-sd-adapt.h"). Public headers anyway must not include
        these headers, and internal headers are never included after
        "nm-default.h", as of the first previous point.
      
      - Include all internal headers with quotes instead of angle brackets.
        In practice it doesn't matter, because in our public headers we must
        include other headers with angle brackets. As we use our public
        headers also to compile our interal source files, effectively the
        result must be the same. Still do it for consistency.
      
      - Except for <config.h> itself. Include it with angle brackets as suggested by
        https://www.gnu.org/software/autoconf/manual/autoconf.html#Configuration-Headers
      8bace23b
  5. 29 Jan, 2016 1 commit
    • Lubomir Rintel's avatar
      fake-platform: check link_get return · 9b851798
      Lubomir Rintel authored
      Can not fail no fake platform, but makes Coverity worried:
      
      CID 59381 (#1 of 1): Dereference null return value (NULL_RETURNS)
      6.  dereference: Dereferencing a null pointer device.
      9b851798
  6. 11 Dec, 2015 1 commit
  7. 10 Dec, 2015 2 commits
    • Thomas Haller's avatar
      platform: move reading sysctl-options for master/slave to NMPlatform · 1a7b19ea
      Thomas Haller authored
      These functions are only helpers for accessing sysctl and independent
      from NMLinuxPlatform. Move their implementation to the base class.
      1a7b19ea
    • Thomas Haller's avatar
      platform: return pointer to NMPlatformLink object for add functions · a4de9187
      Thomas Haller authored
      Let the link-add functions return the internal pointer to the platform
      link object. Similar to link-get, which doesn't copy the link either.
      
      Also adjust the sole users of the add-functions (create-and-realize)
      to take the pointer.
      
      Eventually we still copy the returned data, because accessing platform can
      invalidate the returned pointer. Thus we don't actually safe any copying
      by this (at least every use of the function currently leads to the data
      being copied).
      Still change it, because I think the API of NMPlatform should look like that.
      a4de9187
  8. 09 Dec, 2015 1 commit
  9. 27 Nov, 2015 1 commit
    • Thomas Haller's avatar
      platform: remove NMPlatformReason enum · 510e53ca
      Thomas Haller authored
      This enum was unused and meaningless because the platform signals
      are emitted as a consequence of netlink messages. It is not clear
      whether a netlink message was received due to an external event
      or an internal action.
      510e53ca
  10. 02 Nov, 2015 6 commits
    • Thomas Haller's avatar
      platform: use ifi_change field for setting link flags · 7ae20d70
      Thomas Haller authored
      Previously, we would not set the ifi_change field, so that all
      flags in ifi_flags were considered. That required us to lookup
      the currently set flags from the cache.
      
      Change that, to set only the flags in the netlink message that
      we want to change. This saves us a cache-lookup, but more importantly,
      the cache might be out of date.
      7ae20d70
    • Thomas Haller's avatar
      platform/vlan: add support for ingress/egress-qos-mappings and changing flags · a5ea1419
      Thomas Haller authored
      Previously, we could only set the ingress-qos-mappings/egress-qos-mappings.
      Now also cache the mappings and expose them from the platform cache.
      
      Also, support changing the vlan flags not only when creating the vlan
      interface.
      a5ea1419
    • Thomas Haller's avatar
      platform: promise that the link lnk is an immutable NMPObject and expose it · 4b1e1f8a
      Thomas Haller authored
      Expose internal lnk object and promise in the API that the object will
      not be modified (which allows the user to ref it).
      4b1e1f8a
    • Thomas Haller's avatar
      99f97853
    • Thomas Haller's avatar
      platform: properly handle IPv4 peer-addresses · 6c8aa669
      Thomas Haller authored
      The peer-address (IFA_ADDRESS) can also be all-zero (0.0.0.0).
      That is distinct from an usual address without explicit peer-address,
      which implicitly has the same peer and local address.
      
      Previously, we treated an all-zero peer_address as having peer and
      local address equal. This is especially grave, because the peer is part
      of the primary key for an IPv4 address. So we not only get a property of
      the address wrong, but we wrongly consider two different addresses as
      one and the same.
      
      To properly handle these addresses, we always must explicitly set the peer.
      6c8aa669
    • Thomas Haller's avatar
      platform: refactor nm_platform_veth_get_properties() · 7cdbc393
      Thomas Haller authored
      For recent kernels, the peer-ifindex of veths is reported as
      parent (IFA_LINK). Prefer that over the ethtool lookup.
      
      For one, this avoids the extra ethtool call which has the
      downside of sidestepping the platform cache. Also, looking
      up the peer-ifindex in ethtool does not report whether the
      peer lifes in another netns (NM_PLATFORM_LINK_OTHER_NETNS).
      
      Only use ethtool as fallback for older kernels.
      7cdbc393
  11. 01 Nov, 2015 7 commits
  12. 14 Oct, 2015 3 commits
    • Thomas Haller's avatar
      platform/test: add test adding IPv4 addresses that only differ by their peer-address · 06aafabf
      Thomas Haller authored
      Also do a major cleanup of the tests:
      
      - Have utility functions in "test-common.h" with a new prefix "nmtstp_".
        The prefix indicates that these are test functions for platform.
      
      - Add functions to add/remove IP addresses that either use external
        iproute2 command or platform function itself. These commands also
        assert whether the command had the expected result.
      
      - Randomize, whether we use the external command for adding
        ip-addresses. Both approaches should yield the same result
        for linux-platform.
        I did this now for address-tests, but effectively this doubled
        all our previous tests to use both internal and external ways
        to configure the address.
      
      - Enable all address tests for fake-platform. They now
        automatically don't call external iproute2 but fallback
        to fake-platform implementation. This adds more coverage
        to the fake-platform, which we want to behave identical
        to linux-platform.
      
      - Setup a clean test device before every address-test.
      06aafabf
    • Thomas Haller's avatar
      platform: properly handle peer-address for IPv4 addresses · 8968e15e
      Thomas Haller authored
      Kernel allows to add the same IPv4 address that only differs by
      peer-address (IFL_ADDRESS):
      
          $ ip link add dummy type dummy
          $ ip address add 1.1.1.1 peer 1.1.1.3/24 dev dummy
          $ ip address add 1.1.1.1 peer 1.1.1.4/24 dev dummy
          RTNETLINK answers: File exists
          $ ip address add 1.1.1.1 peer 1.1.2.3/24 dev dummy
          $ ip address show dev dummy
          2: dummy@NONE: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN group default
              link/ether 52:58:a7:1e:e8:93 brd ff:ff:ff:ff:ff:ff
              inet 1.1.1.1 peer 1.1.1.3/24 scope global dummy
                 valid_lft forever preferred_lft forever
              inet 1.1.1.1 peer 1.1.2.3/24 scope global dummy
                 valid_lft forever preferred_lft forever
      
      We must also consider peer-address, otherwise platform will treat
      two different addresses as one and the same.
      
      https://bugzilla.gnome.org/show_bug.cgi?id=756356
      8968e15e
    • Thomas Haller's avatar
      platform: refactor order of peer-address argument in ip_address_add() function · f193d98c
      Thomas Haller authored
      The peer-address seems less important then the prefix-length.
      Also, nm_platform_ip4_address_delete() has the peer-address
      argument as last.
      
      Soon ip4_address_get() also receives a peer-address argument,
      so get the order right first.
      f193d98c
  13. 04 Sep, 2015 1 commit
  14. 05 Aug, 2015 1 commit
  15. 14 Jul, 2015 1 commit
  16. 01 Jul, 2015 2 commits
    • Thomas Haller's avatar
      route-manager: manage IPv4 device-routes with NMRouteManager · 5f54a323
      Thomas Haller authored
      When adding an IPv4 address, kernel will also add a device-route.
      We don't want that route because it has the wrong metric. Instead,
      we add our own route (with a different metric) and remove the
      kernel-added one.
      
      This could be avoided if kernel would support an IPv4 address flag
      IFA_F_NOPREFIXROUTE like it does for IPv6 (see related bug rh#1221311).
      
      One important thing is, that we want don't want to manage the
      device-route on assumed devices. Note that this is correct behavior
      if "assumed" means "do-not-touch".
      If "assumed" means "seamlessly-takeover", then this is wrong.
      Imagine we get a new DHCP address. In this case, we would not manage
      the device-route on the assumed device. This cannot be fixed without
      splitting unmanaged/assumed with related bug bgo 746440.
      This is no regression as we would also not manage device-routes
      for assumed devices previously.
      
      We also don't want to remove the device-route if the user added
      it externally. Note that here we behave wrongly too, because we
      don't record externally added kernel routes in update_ip_config().
      This still needs fixing.
      
      Let IPv4 device-routes also be managed by NMRouteManager. NMRouteManager
      has a list of all routes and can properly add, remove, and restore
      the device route as needed.
      
      One problem is, that the device-route does not get added immediately
      with the address. It only appears some time later. This is solved
      by NMRouteManager watching platform and if a matchin device-route shows up
      within a short time after configuring  addresses, remove it.
      If the route appears after the short timeout, assume they were added for
      other reasons (e.g. by the user) and don't remove them.
      
      https://bugzilla.gnome.org/show_bug.cgi?id=751264
      https://bugzilla.redhat.com/show_bug.cgi?id=1211287
      5f54a323
    • Thomas Haller's avatar
      platform: change NMPlatformGetRouteMode enum to NMPlatformGetRouteFlags flags · d9dba6b6
      Thomas Haller authored
      By having flags instead of an enum/mode, we can encode more combinations
      of filtering the result.
      d9dba6b6
  17. 24 Jun, 2015 1 commit
  18. 21 Jun, 2015 1 commit
    • Thomas Haller's avatar
      platform: refactor virtual methods for link objects in NMPlatform · e8e45581
      Thomas Haller authored
      Change nm_platform_link_get() to return the cached NMPlatformLink
      instance. Now what all our implementations (fake and linux) have such a
      cache internal object, let's just expose it directly.
      Note that the lifetime of the exposed link object is possibly quite
      short. A caller must copy the returned value if he intends to preserve
      it for later.
      Also add nm_platform_link_get_by_ifname() and modify nm_platform_link_get_by_address()
      to return the instance.
      
      Certain functions, such as nm_platform_link_get_name(),
      nm_platform_link_get_ifindex(), etc. are solely implemented based
      on looking at the returned NMPlatformLink object. No longer implement
      them as virtual functions but instead implement them in the base class
      (nm-platform.c).
      This removes code and eliminates the redundancy of the exposed
      NMPlatformLink instance and the nm_platform_link_get_*() accessors.
      Thereby also fix a bug in NMFakePlatform that tracked the link address
      in a separate "address" field, instead of using "link.addr". That was
      a case where the redundancy actually led to a bug in fake platform.
      
      Also remove some stub implementations in NMFakePlatform that just
      bail out. Instead allow for a missing virtual functions and perform
      the "default" action in the accessor.
      An example for that is nm_platform_link_get_permanent_address().
      e8e45581
  19. 17 Jun, 2015 3 commits
    • Thomas Haller's avatar
      platform: drop nm_platform_get_error() · 68a4ffb4
      Thomas Haller authored
      For NMPlatform instances we had an error reporting mechanism
      which stores the last error reason in a private field. Later we
      would check it via nm_platform_get_error().
      
      Remove this. It was not used much, and it is not a great way
      to report errors.
      
      One problem is that at the point where the error happens, you don't
      know whether anybody cares about an error code. So, you add code to set
      the error reason because somebody *might* need it (but in realitiy, almost
      no caller cares).
      Also, we tested this functionality which is hardly used in non-testing code.
      While this was a burden to maintain in the tests, it was likely still buggy
      because there were no real use-cases, beside the tests.
      
      Then, sometimes platform functions call each other which might overwrite the
      error reason. So, every function must be cautious to preserve/set
      the error reason according to it's own meaning. This can involve storing
      the error code, calling another function, and restoring it afterwards.
      This is harder to get right compared to a "return-error-code" pattern, where
      every function manages its error code independently.
      
      It is better to return the error reason whenever due. For that we already
      have our common glib patterns
      
          (1) gboolean fcn (...);
          (2) gboolean fcn (..., GError **error);
      
      In few cases, we need more details then a #gboolean, but don't want
      to bother constructing a #GError. Then we should do instead:
      
          (3) NMPlatformError fcn (...);
      68a4ffb4
    • Thomas Haller's avatar
      platform: signal missing firmware in nm_platform_set_up() · c1a945b9
      Thomas Haller authored
      Don't use nm_platform_get_error() anymore.
      c1a945b9
    • Thomas Haller's avatar
      platform: no longer expose udi field in NMPlatformLink · 1b2b988e
      Thomas Haller authored
      The @udi field is not a static string, so any user of a NMPlatformLink
      instance must make sure not to use the field beyond the lifetime of the
      NMPlatformLink instance.
      As we pass on the platform link instance during platform changed events,
      this is hard to ensure for the subscriber of the signal -- because a
      call back into platform could invalidate/modify the object.
      
      Just not expose this field as part of the link instance. The few callers
      who actually needed it should instead call nm_platform_get_uid(). With
      that, the lifetime of the returned 'const char *' pointer is clearly
      defined.
      1b2b988e