summaryrefslogtreecommitdiff
path: root/src/ppp
Commit message (Collapse)AuthorAgeFilesLines
* license: Add license using SPDX identifiers to meson build filesIñigo Martínez2020-02-171-0/+2
| | | | | License is missing in meson build files. This has been added using SPDX identifiers and licensed under LGPL-2.1+.
* all: drop explicit casts from _GET_PRIVATE() macro callsThomas Haller2020-02-141-3/+3
| | | | | | | | | The _GET_PRIVATE() macros are all implemented based on _NM_GET_PRIVATE(). That macro tries to be more type safe and uses _Generic() to do the right thing. Explicitly casting is not only unnecessary, it defeats these (static) type checks. Don't do that.
* shared: drop _STATIC variant of macros that define functionsThomas Haller2020-02-131-2/+4
| | | | | | | | | | | | | | | | | | Several macros are used to define function. They had a "_STATIC" variant, to define the function as static. I think those macros should not try to abstract entirely what they do. They should not accept the function scope as argument (or have two variants per scope). This also because it might make sense to add additional __attribute__(()) to the function. That only works, if the macro does not pretend to *not* define a plain function. Instead, embrace what the function does and let the users place the function scope as they see fit. This also follows what is already done with static NM_CACHED_QUARK_FCN ("autoconnect-root", autoconnect_root_quark)
* all: unify format of our Copyright source code commentsThomas Haller2019-10-021-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | ```bash readarray -d '' FILES < <( git ls-files -z \ ':(exclude)po' \ ':(exclude)shared/c-rbtree' \ ':(exclude)shared/c-list' \ ':(exclude)shared/c-siphash' \ ':(exclude)shared/c-stdaux' \ ':(exclude)shared/n-acd' \ ':(exclude)shared/n-dhcp4' \ ':(exclude)src/systemd/src' \ ':(exclude)shared/systemd/src' \ ':(exclude)m4' \ ':(exclude)COPYING*' ) sed \ -e 's/^\(--\|#\| \*\) *\(([cC]) *\)\?Copyright \+\(\(([cC])\) \+\)\?\(\(20\|19\)[0-9][0-9]\) *[-–] *\(\(20\|19\)[0-9][0-9]\) \+\([^ ].*\)$/\1 C1pyright#\5 - \7#\9/' \ -e 's/^\(--\|#\| \*\) *\(([cC]) *\)\?Copyright \+\(\(([cC])\) \+\)\?\(\(20\|19\)[0-9][0-9]\) *[,] *\(\(20\|19\)[0-9][0-9]\) \+\([^ ].*\)$/\1 C2pyright#\5, \7#\9/' \ -e 's/^\(--\|#\| \*\) *\(([cC]) *\)\?Copyright \+\(\(([cC])\) \+\)\?\(\(20\|19\)[0-9][0-9]\) \+\([^ ].*\)$/\1 C3pyright#\5#\7/' \ -e 's/^Copyright \(\(20\|19\)[0-9][0-9]\) \+\([^ ].*\)$/C4pyright#\1#\3/' \ -i \ "${FILES[@]}" echo ">>> untouched Copyright lines" git grep Copyright "${FILES[@]}" echo ">>> Copyright lines with unusual extra" git grep '\<C[0-9]pyright#' "${FILES[@]}" | grep -i reserved sed \ -e 's/\<C[0-9]pyright#\([^#]*\)#\(.*\)$/Copyright (C) \1 \2/' \ -i \ "${FILES[@]}" ``` https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/298
* meson: Improve ppp build fileIñigo Martínez2019-10-011-12/+6
| | | | | | The set of c_flags used when building `ppp` targets has been grouped together. Used dependencies have also been reviewed and removed the unnecessary one.
* meson: Improve the libnm-core build fileIñigo Martínez2019-10-011-6/+2
| | | | | | | | | | | | | | | | | | The `libnm-core` build file has been improved by applying a set of changes: - Indentation has been fixed to be consistent. - Library variable names have been changed to `lib{name}` pattern following their filename pattern. - `shared` prefix has been removed from all variables using it. - Dependencies have been reviewed to store the necessary data. - The use of the libraries and dependencies created in this file has been reviewed through the entire source code. This has required the addition or the removal of different libraries and dependencies in different targets. - Some files used directly with the `files` function have been moved to their nearest path build file because meson stores their full path seamessly and they can be used anywhere later.
* meson: Use dependency for nm-default headerIñigo Martínez2019-10-011-1/+2
| | | | | | | | | | | | | The `nm-default.h` header is used widely in the code by many targets. This header includes different headers and needs different libraries depending the compilation flags. A new set of `*nm_default_dep` dependencies have been created to ease the inclusion of different directorires and libraries. This allows cleaner build files and avoiding linking unnecessary libraries so this has been applied allowing the removal of some dependencies involving the linking of unnecessary libraries.
* all: manually drop code comments with file descriptionThomas Haller2019-10-018-16/+8
|
* core: extend nm_shutdown_wait_obj_*() to support notification via a GCancellableThomas Haller2019-09-221-2/+2
| | | | | | | | | | | | | | | | | | | | 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.
* all: SPDX header conversionLubomir Rintel2019-09-108-112/+8
| | | | | $ find * -type f |xargs perl contrib/scripts/spdx.pl $ git rm contrib/scripts/spdx.pl
* core/pppd-plugin: wait to recover port settings before notifying deathAlfonso Sánchez-Beato2019-06-141-3/+18
| | | | | | | | | | | | | | | | | | | | | | | | | | pppd restores the previous settings for the serial port it uses right before exiting. It is especially important to do so because otherwise ModemManager is not able to recover the port as it can receive a hangup event from the port due to CLOCAL not being restored. However, there is currently a race condition that produces this issue. This is because when PHASE_DEAD is notified, pppd still has not restored the port settings - it does that a bit later, in the die() function. This patch delays notifying PHASE_DEAD until when the exitnotify() hook is called by pppd: when this happens the port settings have already been restored. There were previously efforts to fix this in commit fe090c34b724, so PHASE_DEAD was used instead of PHASE_DISCONNECT to notify MM that the port was disconnected, but that still early to ensure that the port settings are restored. The MM traces seen when the bug is triggered are: ModemManager[2158]: <warn> (ttyACM1): could not re-acquire serial port lock: (5) Input/output error ModemManager[2158]: <warn> Couldn't load Operator Code: 'Cannot run sequence: 'Could not open serial device ttyACM1: it has been forced close' https://mail.gnome.org/archives/networkmanager-list/2019-June/msg00014.html
* all: drop emacs file variables from source filesThomas Haller2019-06-118-8/+0
| | | | | | | | | | | | | | | | | | | | | | We no longer add these. If you use Emacs, configure it yourself. Also, due to our "smart-tab" usage the editor anyway does a subpar job handling our tabs. However, on the upside every user can choose whatever tab-width he/she prefers. If "smart-tabs" are used properly (like we do), every tab-width will work. No manual changes, just ran commands: F=($(git grep -l -e '-\*-')) sed '1 { /\/\* *-\*- *[mM]ode.*\*\/$/d }' -i "${F[@]}" sed '1,4 { /^\(#\|--\|dnl\) *-\*- [mM]ode/d }' -i "${F[@]}" Check remaining lines with: git grep -e '-\*-' The ultimate purpose of this is to cleanup our files and eventually use SPDX license identifiers. For that, first get rid of the boilerplate lines.
* core/pppd-plugin: use GDBusConnection in "nm-pppd-plugin.c"Thomas Haller2019-05-131-97/+109
| | | | | | | | | | - use GDBusConnection instead of GDBusProxy. - namespace global variables with a "gl" struct. - don't log __func__. If a log line should have a certain topic/tag, the tag should be set manually, not based on the function name. It just looks odd. Also, it's unnecessary.
* build/meson: rename "nm_core_dep" to "libnm_core_dep"Thomas Haller2019-04-181-1/+1
| | | | | | | The library is called "libnm_core". So the dependency should be called "libnm_core_dep", like in all other cases. (cherry picked from commit c27ad37c278461fd783b6db56844683ab3088345)
* all: replace strerror() calls with nm_strerror_native()Thomas Haller2019-02-121-2/+2
|
* all: cache errno in local variable before using itThomas Haller2019-02-121-9/+15
|
* all: drop unnecessary includes of <errno.h> and <string.h>Thomas Haller2019-02-123-4/+0
| | | | | "nm-macros-interal.h" already includes <errno.h> and <string.h>. No need to include it everywhere else too.
* core: pass hints as strv to nm_act_request_get_secrets()Thomas Haller2019-02-051-5/+5
| | | | | Extend nm_act_request_get_secrets() API to allow for the underlying flexibility (of the API that it calls) to accept a strv list of hints.
* ppp-manager: fix a typo in a debugging statementLubomir Rintel2019-01-271-2/+2
| | | | | | | | | Discovered by GCC 9: src/ppp/nm-ppp-manager.c: In function ‘_ppp_manager_start’: ./src/nm-logging.h:59:9: error: ‘%s’ directive argument is null [-Werror=format-overflow=] Fixes: 35d9169c3c4a0361c363eca8a34ed6c575a620f1
* build: meson: Add trailing commasIñigo Martínez2018-12-201-4/+4
| | | | | | | Add missing trailing commas that avoids getting noise when another file/parameter is added and eases reviewing changes[0]. [0] https://gitlab.gnome.org/GNOME/dconf/merge_requests/11#note_291585
* core: use streq() instead of strcmp() for comparing ip-config methodsThomas Haller2018-12-131-2/+2
| | | | | | | | Refactor some code to use nm_streq() and NM_IN_STRSET() instead of strcmp(). Note that nm_utils_get_ip_config_method() never returns %NULL (not even with g_return*() assertion failures). nm_streq() is sufficent.
* core: use addr-family argument for nm_utils_get_ip_config_method()Thomas Haller2018-12-131-2/+2
| | | | | | | | | | Recently, more and more code was refactored to use an addr_family integer to distinguish between IPv4 and IPv6. Refactor nm_utils_get_ip_config_method() and nm_device_get_effective_ip_config_method() to do that too. If we use different identifiers, we need to translate from one to another and its inconsistent. Also, accessing a GType is an unnecessary function call, instead of a plain constant.
* ppp: replace NMCmdLine API with plain GPtrArray in create_pppd_cmd_line()Thomas Haller2018-12-031-146/+85
|
* ppp: check ppp status against correct typeSven Schwermer2018-10-221-1/+1
| | | | | | ppp_status is of type NMPPPStatus whereas PHASE_RUNNING is pppd's type. https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/28
* ppp: make ppp-manager cancellable via GCancellableThomas Haller2018-10-174-3/+41
| | | | | | | | | | | | | Previously nm_ppp_manager_stop() would return a handle which makes it easy to cancel the operation. However, sometimes, we may want to cancel an operation based on an GCancellable. So, extend nm_ppp_manager_stop() to hook it with a cancellable. Essentially, move the code from nm-modem.c to nm-ppp-manager-call.c, where it belongs and where the functionality gets available to every component.
* ppp: cleanup logging in impl_ppp_manager_set_ifindex()Thomas Haller2018-09-121-7/+6
| | | | | | | | It's enough that all code paths in impl_ppp_manager_set_ifindex() log exactly one message. Also, give all messages the same prefix, so that it's clear where they come from. (cherry picked from commit 2a45c32e8c5b25889dc10a8b954f79a3539d39b7)
* ppp: downgrade warning about repeated SetIfindex calls from ppp pluginThomas Haller2018-09-121-1/+4
| | | | | | | | | | | | | | | In src/ppp/nm-pppd-plugin.c, it seems that pppd can invoke phasechange(PHASE_RUNNING:) multiple times. Hence, the plugin calls SetIfindex multiple times too. In nm-ppp-manager.c, we want to make sure that the ifindex does not change after it was set once. However, calling SetIfindex with the same ifindex is not something worth warning. Just log a debug message and nothing. Maybe the plugin should remember that it already set the ifindex, and avoid multiple D-Bus calls. But it's unclear that that is desired. For now, just downgrade the warning. (cherry picked from commit 4a4439835dde96fc81f9e09889d8f82436b0331a)
* ppp: avoid strncpy() in ppp plugin nm_phasechange()Thomas Haller2018-09-121-1/+1
| | | | | | | strncpy() is deemed insecure, and it raises at least an eyebrow. While it's save in this case, just avoid it. (cherry picked from commit 4d11eba8c59b6dc00a0cc4b644104b19873699c9)
* ppp-manager: avoid crash with nonexisting link in impl_ppp_manager_set_ifindex()Thomas Haller2018-09-111-1/+5
| | | | | Fixes: dd98ada33f33820e0d0874d9aa97e0c2bfc7cdd0 (cherry picked from commit 30a469e0bba5ac052bc29914783e62ed24b2bd67)
* ppp-manager: fix pppd not exiting correctly on modem hangupFrederic Danis2018-09-111-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When unplugging an USB 3G modem device, pppd does not exit correctly and we have the following traces: Sep 10 07:58:24.616465 ModemManager[1158]: <info> (tty/ttyUSB0): released by device '/sys/devices/pci0000:00/0000:00:1c.0/0000:01:00.0/usb4/4-1' Sep 10 07:58:24.620314 pppd[2292]: Modem hangup Sep 10 07:58:24.621368 ModemManager[1158]: <info> (tty/ttyUSB1): released by device '/sys/devices/pci0000:00/0000:00:1c.0/0000:01:00.0/usb4/4-1' Sep 10 07:58:24.621835 ModemManager[1158]: <warn> (ttyUSB1): could not re-acquire serial port lock: (5) Input/output error Sep 10 07:58:24.621358 NetworkManager[1871]: <debug> ppp-manager: set-ifindex 4 Sep 10 07:58:24.621369 NetworkManager[1871]: <warn> ppp-manager: can't change the ifindex from 4 to 4 Sep 10 07:58:24.623982 NetworkManager[1871]: <info> device (ttyUSB0): state change: activated -> unmanaged (reason 'removed', sys-iface-state: 'removed') Sep 10 07:58:24.624411 NetworkManager[1871]: <debug> kill child process 'pppd' (2292): wait for process to terminate after sending SIGTERM (15) (send SIGKILL in 1500 milliseconds)... Sep 10 07:58:24.624440 NetworkManager[1871]: <debug> modem-broadband[ttyUSB0]: notifying ModemManager about the modem disconnection Sep 10 07:58:24.626591 NetworkManager[1871]: <debug> modem-broadband[ttyUSB0]: notifying ModemManager about the modem disconnection Sep 10 07:58:24.681016 NetworkManager[1871]: <warn> modem-broadband[ttyUSB0]: failed to disconnect modem: GDBus.Error:org.freedesktop.DBus.Error.UnknownMethod: No such interface 'org.freedesktop.ModemManager1.Modem.Simple' on object at path /org/freedesktop/ModemManager1/Modem/0 Sep 10 07:58:26.126817 NetworkManager[1871]: <debug> kill child process 'pppd' (2292): process not terminated after 1502368 usec. Sending SIGKILL signal Sep 10 07:58:26.128121 NetworkManager[1871]: <info> device (ppp0): state change: disconnected -> unmanaged (reason 'unmanaged', sys-iface-state: 'removed') Sep 10 07:58:26.135571 NetworkManager[1871]: <debug> kill child process 'pppd' (2292): terminated by signal 9 (1511158 usec elapsed) This is due to nm-ppp-plugin waiting on SetIfIndex call until timeout, which is longer than termination process timeout. Calling g_dbus_method_invocation_return_value() on error fixes this. Fixes: dd98ada33f33820e0d0874d9aa97e0c2bfc7cdd0 https://mail.gnome.org/archives/networkmanager-list/2018-September/msg00010.html (cherry picked from commit e66e4d0e718b0f9102160e98fb6a1bf059677d71)
* build: create "config-extra.h" header instead of passing directory variables ↵Thomas Haller2018-07-171-1/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | via CFLAGS 1) the command line gets shorter. I frequently run `make V=1` to see the command line arguments for the compiler, and there is a lot of noise. 2) define each of these variables at one place. This makes it easy to verify that for all compilation units, a particular define has the same value. Previously that was not obvious or even not the case (see commit e5d1a71396e107d1909744d26ad401f206c0c915 and commit d63cf1ef2faba57595112a82e962b9643cce4718). The point is to avoid redundancy. 3) not all compilation units need all defines. In fact, most modules would only need a few of these defines. We aimed to pass the necessary minium of defines to each compilation unit, but that was non-obvious to get right and often we set a define that wasn't used. See for example "src_settings_plugins_ibft_cppflags" which needlessly had "-DSYSCONFDIR". This question is now entirely avoided by just defining all variables in a header. We don't care to find the minimum, because every component gets anyway all defines from the header. 4) this also avoids the situation, where a module that previously did not use a particular define gets modified to require it. Previously, that would have required to identify the missing define, and add it to the CFLAGS of the complation unit. Since every compilation now includes "config-extra.h", all defines are available everywhere. 5) the fact that each define is now available in all compilation units could be perceived as a downside. But it isn't, because these defines should have a unique name and one specific value. Defining the same name with different values, or refer to the same value by different names is a bug, not a desirable feature. Since these defines should be unique accross the entire tree, there is no problem in providing them to every compilation unit. 6) the reason why we generate "config-extra.h" this way, instead of using AC_DEFINE() in configure.ac, is due to the particular handling of autoconf for directory variables. See [1]. With meson, it would be trivial to put them into "config.h.meson". While that is not easy with autoconf, the "config-extra.h" workaround seems still preferable to me. [1] https://www.gnu.org/software/autoconf/manual/autoconf-2.63/html_node/Installation-Directory-Variables.html
* all: don't use gchar/gshort/gint/glong but C typesThomas Haller2018-07-111-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 587 $ git grep '\<\(char\|short\|int\|long\|float\|double\)\>' | wc -l 21114 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' \ "${FILES[@]}"
* build/meson: fix meson build without pppdThomas Haller2018-07-091-0/+4
| | | | Fixes: 1cdb36b8de5ad942fed979c8838e5df63a1edcb0
* ppp-manager: use configured pppd pathJan Tojnar2018-07-091-1/+1
| | | | | | | | Path to pppd can be set via configure flag but the source code ignores it. Let's use PPPD_PATH like other calls of nm_utils_find_helper do. https://bugzilla.gnome.org/show_bug.cgi?id=796752
* core: add and use NM_SHUTDOWN_TIMEOUT_MS as duration that we plan for shutdownThomas Haller2018-05-251-1/+2
| | | | | | | | | | | | | | | | | | | | | | | 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.
* ppp-manager: rework stopping NMPPPManager by merging async/sync methodsThomas Haller2018-05-255-137/+136
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
* ppp-manager/trivial: rename variables holding self pointersThomas Haller2018-05-251-64/+64
| | | | | | 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.
* ppp-manager: refactor killing pppd process by using _ppp_kill() functionThomas Haller2018-05-251-24/+25
| | | | | | | | | | - 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.
* meson: distinguish arch specific and arch neutral lib dirLubomir Rintel2018-05-091-1/+1
| | | | | Plugins go to the arch specific place while conf.d/ and VPN/ are in lib/. Use the same naming as is used with autoconf.
* core/dbus: rework creating numbered D-Bus export path by putting counter ↵Thomas Haller2018-03-131-1/+1
| | | | | | | | | | | | | | | into class I dislike the static hash table to cache the integer counter for numbered paths. Let's instead cache the counter at the class instance itself -- since the class contains the information how the export path should be exported. However, we cannot use a plain integer field inside the class structure, because the class is copied between derived classes. For example, NMDeviceEthernet and NMDeviceBridge both get a copy of the NMDeviceClass instance. Hence, the class doesn't contain the counter directly, but a pointer to one counter that can be shared between sibling classes.
* core/dbus: rework D-Bus implementation to use lower layer GDBusConnection APIThomas Haller2018-03-122-53/+135
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, we used the generated GDBusInterfaceSkeleton types and glued them via the NMExportedObject base class to our NM types. We also used GDBusObjectManagerServer. Don't do that anymore. The resulting code was more complicated despite (or because?) using generated classes. It was hard to understand, complex, had ordering-issues, and had a runtime and memory overhead. This patch refactors this entirely and uses the lower layer API GDBusConnection directly. It replaces the generated code, GDBusInterfaceSkeleton, and GDBusObjectManagerServer. All this is now done by NMDbusObject and NMDBusManager and static descriptor instances of type GDBusInterfaceInfo. This adds a net plus of more then 1300 lines of hand written code. I claim that this implementation is easier to understand. Note that previously we also required extensive and complex glue code to bind our objects to the generated skeleton objects. Instead, now glue our objects directly to GDBusConnection. The result is more immediate and gets rid of layers of code in between. Now that the D-Bus glue us more under our control, we can address issus and bottlenecks better, instead of adding code to bend the generated skeletons to our needs. Note that the current implementation now only supports one D-Bus connection. That was effectively the case already, although there were places (and still are) where the code pretends it could also support connections from a private socket. We dropped private socket support mainly because it was unused, untested and buggy, but also because GDBusObjectManagerServer could not export the same objects on multiple connections. Now, it would be rather straight forward to fix that and re-introduce ObjectManager on each private connection. But this commit doesn't do that yet, and the new code intentionally supports only one D-Bus connection. Also, the D-Bus startup was simplified. There is no retry, either nm_dbus_manager_start() succeeds, or it detects the initrd case. In the initrd case, bus manager never tries to connect to D-Bus. Since the initrd scenario is not yet used/tested, this is good enough for the moment. It could be easily extended later, for example with polling whether the system bus appears (like was done previously). Also, restart of D-Bus daemon isn't supported either -- just like before. Note how NMDBusManager now implements the ObjectManager D-Bus interface directly. Also, this fixes race issues in the server, by no longer delaying PropertiesChanged signals. NMExportedObject would collect changed properties and send the signal out in idle_emit_properties_changed() on idle. This messes up the ordering of change events w.r.t. other signals and events on the bus. Note that not only NMExportedObject messed up the ordering. Also the generated code would hook into notify() and process change events in and idle handle, exhibiting the same ordering issue too. No longer do that. PropertiesChanged signals will be sent right away by hooking into dispatch_properties_changed(). This means, changing a property in quick succession will no longer be combined and is guaranteed to emit signals for each individual state. Quite possibly we emit now more PropertiesChanged signals then before. However, we are now able to group a set of changes by using standard g_object_freeze_notify()/g_object_thaw_notify(). We probably should make more use of that. Also, now that our signals are all handled in the right order, we might find places where we still emit them in the wrong order. But that is then due to the order in which our GObjects emit signals, not due to an ill behavior of the D-Bus glue. Possibly we need to identify such ordering issues and fix them. Numbers (for contrib/rpm --without debug on x86_64): - the patch changes the code size of NetworkManager by - 2809360 bytes + 2537528 bytes (-9.7%) - Runtime measurements are harder because there is a large variance during testing. In other words, the numbers are not reproducible. Currently, the implementation performs no caching of GVariants at all, but it would be rather simple to add it, if that turns out to be useful. Anyway, without strong claim, it seems that the new form tends to perform slightly better. That would be no surprise. $ time (for i in {1..1000}; do nmcli >/dev/null || break; echo -n .; done) - real 1m39.355s + real 1m37.432s $ time (for i in {1..2000}; do busctl call org.freedesktop.NetworkManager /org/freedesktop org.freedesktop.DBus.ObjectManager GetManagedObjects > /dev/null || break; echo -n .; done) - real 0m26.843s + real 0m25.281s - Regarding RSS size, just looking at the processes in similar conditions, doesn't give a large difference. On my system they consume about 19MB RSS. It seems that the new version has a slightly smaller RSS size. - 19356 RSS + 18660 RSS
* core: fix typo for parameter as "paramter"Thomas Haller2018-02-281-6/+6
|
* ppp/plugin: use g_strlcpy()Lubomir Rintel2018-02-121-15/+4
| | | | | It's nicer but also doesn't annoy gcc 8: "error: ‘strncpy’ specified bound depends on the length of the source argument [-Werror=stringop-overflow=]"
* all: require glib 2.40lr/glib-2-40Lubomir Rintel2018-01-181-2/+0
| | | | | | RHEL 7.1 and Ubuntu 14.04 LTS both have this. https://bugzilla.gnome.org/show_bug.cgi?id=792323
* build/meson: use variables for ldflags and linker-scriptThomas Haller2018-01-111-7/+5
|
* ppp: update interface name in the plugin after NM changes itBeniamino Galvani2018-01-101-6/+18
| | | | | | | | | | | | | | | | | | | | | | When NM knows of the ifindex/name of the new PPP interface (through the SetIfindex() call), it renames it. This can race with the pppd daemon, which issues ioctl() using the interface name cached in the global 'ifname' variable: ... NetworkManager[27213]: <debug> [1515427406.0036] ppp-manager: set-ifindex 71 pppd[27801]: sent [CCP ConfRej id=0x1 <deflate 15> <deflate(old#) 15> <bsd v1 15>] NetworkManager[27213]: <debug> [1515427406.0036] platform: link: setting 'ppp5' (71) name dsl-ppp pppd[27801]: sent [IPCP ConfAck id=0x2 <addr 3.1.1.1>] pppd[27801]: ioctl(SIOCSIFADDR): No such device (line 2473) pppd[27801]: Interface configuration failed pppd[27801]: Couldn't get PPP statistics: No such device ... Fortunately the variable is exposed to plugins and so we can turn the SetIfindex() D-Bus call into a synchronous one and then update the value of the 'ifname' global variable with the new interface name assigned by NM.
* ppp: introduce SetIfindex pppd plugin D-Bus methodBeniamino Galvani2018-01-103-44/+100
| | | | | | | | | | | | If IPV6CP terminates before IPCP, pppd enters the RUNNING phase and we start IP configuration without having an IP interface set, which triggers assertions. Instead, add a SetIfindex() D-Bus method that gets called by the plugin when pppd becomes RUNNING. The method sets the IP ifindex of the device and starts IP configuration. https://bugzilla.redhat.com/show_bug.cgi?id=1515829
* build/meson: unconditionally use linker version scriptsThomas Haller2018-01-101-4/+3
| | | | | | | We also unconditionally use them with autotools. Also, the detection for have_version_script does not seem correct to me. At least, it didn't work with clang.
* meson: Use string variables extensivelyIñigo Martínez2018-01-101-2/+2
| | | | | | | The strings holding the names used for libraries have also been moved to different variables. This way they would be less error as these variables can be reused easily and any typing error would be quickly detected.
* meson: Improve dependency systemIñigo Martínez2018-01-101-11/+6
| | | | | | | | | | | | | | | | | | | | Some targets are missing dependencies on some generated sources in the meson port. These makes the build to fail due to missing source files on a highly parallelized build. These dependencies have been resolved by taking advantage of meson's internal dependencies which can be used to pass source files, include directories, libraries and compiler flags. One of such internal dependencies called `core_dep` was already in use. However, in order to avoid any confusion with another new internal dependency called `nm_core_dep`, which is used to include directories and source files from the `libnm-core` directory, the `core_dep` dependency has been renamed to `nm_dep`. These changes have allowed minimizing the build details which are inherited by using those dependencies. The parallelized build has also been improved.
View Lorry Controller Status