1. 17 Sep, 2018 1 commit
  2. 22 Aug, 2018 1 commit
  3. 10 Aug, 2018 6 commits
    • Thomas Haller's avatar
      libnm, cli, ifcfg-rh: add NMSettingEthtool setting · df30651b
      Thomas Haller authored
      Note that in NetworkManager API (D-Bus, libnm, and nmcli),
      the features are called "feature-xyz". The "feature-" prefix
      is used, because NMSettingEthtool possibly will gain support
      for options that are not only -K|--offload|--features, for
      example -C|--coalesce.
      The "xzy" suffix is either how ethtool utility calls the feature
      ("tso", "rx"). Or, if ethtool utility specifies no alias for that
      feature, it's the name from kernel's ETH_SS_FEATURES ("tx-tcp6-segmentation").
      If possible, we prefer ethtool utility's naming.
      Also note, how the features "feature-sg", "feature-tso", and
      "feature-tx" actually refer to multiple underlying kernel features
      at once. This too follows what ethtool utility does.
      The functionality is not yet implemented server-side.
    • Thomas Haller's avatar
      libnm: add generic-data for implementing NMSetting · 4e0f1b16
      Thomas Haller authored
      Add a new way how NMSetting subclasses can be implemented.
      Currently, most NMSetting implementations realize all their properties
      via GObject properties. That has some downsides:
       - the biggest one, is the large effort to add new properties.
         Most of them are implemented on a one-by-one basis and they come
         with additional API (like native getter functions).
         It makes it cumbersome to add more properties.
       - for certain properties, it's hard to encode them entirely in
         a GObject property. That results in unusable API like
         NM_SETTING_USER_DATA. These complex valued properties only
         exist, because we currently always need GObject properties
         to even implement simple functionality. For example,
         nm_setting_duplicate() is entirely implemented via
         nm_setting_enumerate_values(), which can only iterate
         GObject properies. There is no reason why this is necessary.
         Note also how nmcli badly handles bond options and VPN
         data. That is only a shortcoming of nmcli and wouldn't
         need to be that way. But it happend, because we didn't
         keep an open mind that settings might be more than just
         accessing GObject properties.
       - a major point of NMSetting is to convert to/from a GVariant
         from the D-Bus API. As NMSetting needs to squeeze all values
         into the static GObject structure, there is no place to
         encode invalid or unknown properties. Optimally,
         _nm_setting_new_from_dbus() does not loose any information
         and a subsequent _nm_setting_to_dbus() can restore the original
         variant. That is interesting, because we want that an older
         libnm client can talk to a newer NetworkManager version. The
         client needs to handle unknown properties gracefully to stay
         forward compatible. However, it also should not just drop the
         properties on the floor.
         Note however, optimally we want that nm_setting_verify() still
         can reject settings that have such unknown/invalid values. So,
         it should be possible to create an NMSetting instance without
         error or loosing information. But verify() should be usable to
         identify such settings as invalid.
      They also have a few upsides.
       - libnm is heavily oriented around GObject. So, we generate
         our nm-settings manual based on the gtk-doc. Note however,
         how we fail to generate a useful manual for bond.options.
         Also note, that there is no reason we couldn't generate
         great documentation, even if the properties are not GObject
       - GObject properties do give some functionality like meta-data,
         data binding and notification. However, the meta-data is not
         sufficient on its own. Note how keyfile and nmcli need extensive
         descriptor tables on top of GObject properties, to make this
         useful. Note how GObject notifications for NMSetting instances
         are usually not useful, aside for data binding like nmtui does.
      Also note how NMSettingBond already follows a different paradigm
      than using GObject properties. Nowdays, NMSettingBond is considered
      a mistake (related bug rh#1032808). Many ideas of NMSettingBond
      are flawed, like exposing an inferiour API that reduces everything
      to a string hash. Also, it only implemented the options hash inside
      NMSettingBond. That means, if we would consider this a good style,
      we would have to duplicate this approach in each new setting
      Add a new style to track data for NMSetting subclasses. It keeps
      an internal hash table with all GVariant properies. Also, the
      functionality is hooked into NMSetting base class, so all future
      subclasses that follow this way, can benefit from this. This approach
      has a few similiarties with NMSettingBond, but avoids its flaws.
      With this, we also no longer need GObject properties (if we would
      also implement generating useful documentation based on non-gkt-doc).
      They may be added as accessors if they are useful, but there is no
      need for them.
      Also, handling the properties as a hash of variants invites for a
      more generic approach when handling them. While we still could add
      accessors that operate on a one-by-one bases, this leads to a more
      generic usage where we apply common functionality to a set of properties.
      Also, this is for the moment entirely internal and an implementation
      detail. It's entirely up to the NMSetting subclass to make use of this
      new style. Also, there are little hooks for the subclass available.
      If they turn out to be necessary, they might be added. However, for
      the moment, the functionality is restricted to what is useful and
    • Thomas Haller's avatar
      libnm: rework setting metadata for property handling · 37938043
      Thomas Haller authored
      NMSetting internally already tracked a list of all proper GObject properties
      and D-Bus-only properties.
      Rework the tracking of the list, so that:
      - instead of attaching the data to the GType of the setting via
        g_type_set_qdata(), it is tracked in a static array indexed by
        NMMetaSettingType. This allows to find the setting-data by simple
        pointer arithmetic, instead of taking a look and iterating (like
        g_type_set_qdata() does).
        Note, that this is still thread safe, because the static table entry is
        initialized in the class-init function with _nm_setting_class_commit().
        And it only accessed by following a NMSettingClass instance, thus
        the class constructor already ran (maybe not for all setting classes,
        but for the particular one that we look up).
        I think this makes initialization of the metadata simpler to
        Previously, in a first phase each class would attach the metadata
        to the GType as setting_property_overrides_quark(). Then during
        nm_setting_class_ensure_properties() it would merge them and
        set as setting_properties_quark(). Now, during the first phase,
        we only incrementally build a properties_override GArray, which
        we finally hand over during nm_setting_class_commit().
      - sort the property infos by name and do binary search.
      Also expose this meta data types as internal API in nm-setting-private.h.
      While not accessed yet, it can prove beneficial, to have direct (internal)
      access to these structures.
      Also, rename NMSettingProperty to NMSettInfoProperty to use a distinct
      naming scheme. We already have 40+ subclasses of NMSetting that are called
      NMSetting*. Likewise, NMMetaSetting* is heavily used already. So, choose a
      new, distinct name.
    • Thomas Haller's avatar
      libnm: use NMMetaSettingInfo for tracking setting priority · 9c47e2ce
      Thomas Haller authored
      Previously, each (non abstract) NMSetting class had to register
      its name and priority via _nm_register_setting().
      Note, that libnm-core.la already links against "nm-meta-setting.c",
      which also redundantly keeps track of the settings name and gtype
      as well.
      Re-use NMMetaSettingInfo also in libnm-core.la, to track this meta
      The goal is to get rid of private data structures that track
      meta data about NMSetting classes. In this case, "registered_settings"
      hash. Instead, we should have one place where all this meta data
      is tracked. This was, it is also accessible as internal API,
      which can be useful (for keyfile).
      Note that NMSettingClass has some overlap with NMMetaSettingInfo.
      One difference is, that NMMetaSettingInfo is const, while NMSettingClass
      is only initialized during the class_init() method. Appart from that,
      it's mostly a matter of taste, whether we attach meta data to
      NMSettingClass, to NMMetaSettingInfo, or to a static-array indexed
      by NMMetaSettingType.
      Note, that previously, _nm_register_setting() was private API. That
      means, no user could subclass a functioning NMSetting instance. The same
      is still true: NMMetaSettingInfo is internal API and users cannot access
      it to create their own NMSetting subclasses. But that is almost desired.
      libnm is not designed, to be extensible via subclassing, nor is it
      clear why that would be a useful thing to do. One day, we should remove
      the NMSetting and NMSettingClass definitions from public headers. Their
      only use is subclassing the types, which however does not work.
      While libnm-core was linking already against nm-meta-setting.c,
      nm_meta_setting_infos was unreferenced. So, this change increases
      the binary size of libnm and NetworkManager (1032 bytes). Note however
      that roughly the same information was previously allocated at runtime.
    • Thomas Haller's avatar
      libnm-core/trivial: move code · 19ef103e
      Thomas Haller authored
    • Thomas Haller's avatar
      libnm: minor rework checking property flags in _nm_setting_to_dbus() · a67a3439
      Thomas Haller authored
      Properties that are backed by a GObject property are fundamentally
      I think it's clearer to rework the check, to first check whether
      we have a param_spec, and then implement different checks.
  4. 03 Aug, 2018 1 commit
  5. 01 Aug, 2018 1 commit
  6. 26 Jul, 2018 2 commits
    • Lubomir Rintel's avatar
      nm-setting: add missing initializers · e12cb9ee
      Lubomir Rintel authored
      Fixes: f957ea2b
    • Lubomir Rintel's avatar
      core/setting: rework nm_connection_dump() · f957ea2b
      Lubomir Rintel authored
      Utilize _nm_setting_to_dbus() to serialize the setting. The main reason
      is that this way we can also print the more complicated values
      g_strdup_value_contents() can't grok, e.g. the GArrays and GHashTables.
      Some effort was spent on tidying up the results in a manner it was done
      previously, instead of reducing this to a plain g_variant_print(). It
      looks good that way:
          service-type : "org.freedesktop.NetworkManager.VPN.Novpn" (s)
          user-name : NULL (sd)
          persistent : FALSE (sd)
          data : ((GHashTable*) 0xc61060) (s)
          secrets : ((GHashTable*) 0xdda640) (s)
          timeout : 0 (sd)
          service-type : 'org.freedesktop.NetworkManager.VPN.Novpn'
          data : {'gateway': 'novpn.example.com', 'username': 'hello'}
          secrets : {'password': 'world'}
      Note that no effort was spent on printing the defaults. There are
      multiple ways that could be achieved, but I'm not sure it would be all
      that necessary given this is really just a quick'n'dirty debugging facilty.
  7. 11 Jul, 2018 2 commits
    • Beniamino Galvani's avatar
      libnm-core: add SR-IOV setting · a9b4532f
      Beniamino Galvani authored
      Add a setting containing SR-IOV parameters.
    • Thomas Haller's avatar
      all: don't use gchar/gshort/gint/glong but C types · e1c7a2b5
      Thomas Haller authored
      We commonly don't use the glib typedefs for char/short/int/long,
      but their C types directly.
          $ git grep '\<g\(char\|short\|int\|long\|float\|double\)\>' | wc -l
          $ git grep '\<\(char\|short\|int\|long\|float\|double\)\>' | wc -l
      One could argue that using the glib typedefs is preferable in
      public API (of our glib based libnm library) or where it clearly
      is related to glib, like during
        g_object_set (obj, PROPERTY, (gint) value, NULL);
      However, that argument does not seem strong, because in practice we don't
      follow that argument today, and seldomly use the glib typedefs.
      Also, the style guide for this would be hard to formalize, because
      "using them where clearly related to a glib" is a very loose suggestion.
      Also note that glib typedefs will always just be typedefs of the
      underlying C types. There is no danger of glib changing the meaning
      of these typedefs (because that would be a major API break of glib).
      A simple style guide is instead: don't use these typedefs.
      No manual actions, I only ran the bash script:
        FILES=($(git ls-files '*.[hc]'))
        sed -i \
            -e 's/\<g\(char\|short\|int\|long\|float\|double\)\>\( [^ ]\)/\1\2/g' \
            -e 's/\<g\(char\|short\|int\|long\|float\|double\)\>  /\1   /g' \
            -e 's/\<g\(char\|short\|int\|long\|float\|double\)\>/\1/g' \
  8. 01 Jul, 2018 3 commits
    • Thomas Haller's avatar
      libnm: avoid constructor function for registering NMSetting types · fa9fe466
      Thomas Haller authored
      constructor functions are ugly, because code is running before
      main() starts. Instead, as the registration code for NMSetting types
      is insid the GType constructor, we just need to ensure at the
      right place, that the GType was created.
      The right place here is _register_settings_ensure_inited(), because
      that is called before we need the registration information.
    • Thomas Haller's avatar
      libnm: make _nm_register_setting() thread safe · ecd53944
      Thomas Haller authored
      _nm_register_setting() and _nm_register_setting_impl() are called from within
      the GType constructor for the NMSetting subtype. As such, at that point it
      runs inside a g_once_init_enter() block. However, each implementation
      for initializing the GType has a separate g_once_init_enter() variable, hence,
      if two threads create GType instances for different NMSetting subclasses, there
      is a race.
      libnm is not thread safe. However, it should be at least thread safe
      with respect to constructing the GType instances.
    • Thomas Haller's avatar
      libnm: avoid constructor function to initialize setting registration for NMSetting · 8093c9d3
      Thomas Haller authored
      For NMSetting subtypes, we need the static dictionaries "registered_settings" and
      "registered_settings_by_type" to keep track of existing NMSetting types.
      Initialize these dictionaries inside NMSetting's type initialization code.
      This is guaranteed to run before any use of NMSetting type, and is also
      guarded by a mutex.
      Also, drop the __attribute__((constructor)) function to initialize the
      hash tables. They are not needed, and it's ugly to run code before
  9. 30 Apr, 2018 1 commit
  10. 18 Jan, 2018 1 commit
  11. 23 Nov, 2017 1 commit
  12. 16 Nov, 2017 1 commit
    • Thomas Haller's avatar
      all: use nm_str_hash() instead of g_str_hash() · a6be2f4a
      Thomas Haller authored
      We also do this for libnm and libnm-core, where it causes visible changes
      in behavior. But if somebody would rely on the hashing implementation
      for hash tables, it would be seriously flawed.
  13. 30 Oct, 2017 4 commits
  14. 26 Oct, 2017 2 commits
    • Thomas Haller's avatar
      libnm: fix the return value of nm_setting_diff() if a results hash was given · 975eeda6
      Thomas Haller authored
      Previously, nm_setting_diff() would return !(*results), that means,
      if the caller passed in a hash table (empty or not), the return value
      would always be FALSE, indicating a difference.
      That is not documented, and makes no sense.
      The return value, should solely indicate whether some difference was
      found. The only convenience is, if nm_setting_diff() created a hash
      table internally and no difference was found, it would destroy
      it again, without returning it to the caller.
    • Thomas Haller's avatar
      libnm: fix nm_connection_diff() for settings without properties · 6f94b165
      Thomas Haller authored
      NMSettingGeneric has no properties at all. Hence, nm_connection_diff() would report that
      a connection A with a generic setting and a connection B without a generic setting are
      They are not. For empty settings, let nm_setting_diff() return also empty difference
  15. 07 Jun, 2017 4 commits
    • Thomas Haller's avatar
      libnm: streamline functions in nm-connection.c · 4c094adf
      Thomas Haller authored
      Functions call each other, like
      Along the way, each function asserts that the input argument
      is of type NMConnection via
          g_return_val_if_fail (NM_IS_CONNECTION (connection), NULL);
      Avoid such duplicate assertions when we already verifyied the
      input argument.
      For example, in case of nm_connection_get_id(), don't check just call
      nm_connection_get_setting_connection() right away. It already
      The downside is, that the assertion no longer fails in the function
      that immediately called it. But these are assertions after all.
    • Thomas Haller's avatar
      libnm: downgrade assertions in _nm_register_setting_impl() to nm_assert() · 1d6b7174
      Thomas Haller authored
      This is entirely internal API. We have unit tests that execute these
      code paths. No need to have these assertions in production code.
    • Thomas Haller's avatar
      libnm: add enum for setting priorities · a973eacb
      Thomas Haller authored
      Using plain numbers make it cumbersome to grep for
      setting types by priority.
      The only downside is, that with the enum values it
      is no longer obvious which value has higher or lower
      Also, introduce NM_SETTING_PRIORITY_INVALID. This is what
      _nm_setting_type_get_base_type_priority() returns. For the moment
      it still has the same numerical value 0 as before. Later, that
      shall be distinct from NM_SETTING_PRIORITY_CONNECTION.
    • Thomas Haller's avatar
      libnm/trivial: rename _nm_register_setting function to _nm_register_setting_impl · c7e9f978
      Thomas Haller authored
      Avoid having the function _nm_register_setting() shadowed by a macro
      of the same name, but different behavior/arguments.
  16. 31 May, 2017 2 commits
    • Lubomir Rintel's avatar
      core: negotiate the best base setting · 2c1a178f
      Lubomir Rintel authored
      When the two base settings are present, use one of higher priority.
      This will pick the "bridge" setting when both "bridge" and "bluetooth" are
      present for a Bluetooth NAP connection.
    • Lubomir Rintel's avatar
      core: allow two priorities of base settings · 7b5712ac
      Lubomir Rintel authored
      We'll need two "base" settings for Bluetooth NAP connections: bridge to set up
      the actual link and bluetooth to identify the HCI to register the network
      server with.
      Let's use two priorities for base setting, with "1" marking one of higher
      priority and "2" of lower priority when both are present.
  17. 28 Mar, 2017 1 commit
  18. 10 Feb, 2017 1 commit
  19. 03 Oct, 2016 1 commit
  20. 12 May, 2016 1 commit
  21. 26 Mar, 2016 1 commit
    • Thomas Haller's avatar
      libnm-core: allow strict and relaxed error behavior for _nm_setting_new_from_dbus() · 737c8cc5
      Thomas Haller authored
      In some situations, we want strict checking of errors, for example when
      NetworkManager receives a new connection from a client, the connection
      must make sense as a whole (and since NetworkManager service is backward
      compatible to the clients and not the other way around, there is no
      excuse for sending invalid data to the server).
      In other situations, we want a best-effort behavior. Like when
      NetworkManager sends a connection to its clients, those clients
      want to extract as many properties as they understand, but in order
      to be forward compatible against newer server versions, invalid
      or unknown properties must be accepted.
      Previously, a mixture of both was done. Some issues caused a failure
      to create a new NMSetting, other invalid parts were just silently
      ignored or triggered a g_warning() in glib.
      Now allow for both. When doing strict-validation, be more strict and
      reject all unknown properties and catch when the user sets an invalid
      argument. On the other hand, allow for a best-effort mode that
      effectively cannot fail and will return a new NMSetting instance.
      For now, add NMSettingParseFlags so that the caller can choose the
      old behavior, strict parsing, or best effort.
      This patch doesn't have any externally visible change except that
      no more g_warnings will be emitted.
  22. 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
  23. 12 Feb, 2016 1 commit
    • Thomas Haller's avatar
      build: cleanup default includes · 2c2d9d2e
      Thomas Haller authored
      - "gsystem-local-alloc.h" and <gio/gio.h> are already included via
        "nm-default.h". No need to include them separately.
      - include "nm-macros-internal.h" via "nm-default.h" and drop all
        explict includes.
      - in the modified files, ensure that we always include "config.h"
        and "nm-default.h" first. As second, include the header file
        for the current source file (if applicable). Then follow external
        includes and finally internal nm includes.
      - include nm headers inside source code files with quotes
      - internal header files don't need to include default headers.
        They can savely assume that "nm-default.h" is already included
        and with it glib, nm-glib.h, nm-macros-internal.h, etc.