<feed xmlns='http://www.w3.org/2005/Atom'>
<title>delta/NetworkManager.git/Makefile.am, branch th/checkpoint</title>
<subtitle>gitlab.freedesktop.org: NetworkManager/NetworkManager.git
</subtitle>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/NetworkManager.git/'/>
<entry>
<title>checkpoint: embed CList in NMCheckpoint instance</title>
<updated>2018-03-29T09:24:32+00:00</updated>
<author>
<name>Thomas Haller</name>
<email>thaller@redhat.com</email>
</author>
<published>2018-03-27T10:45:23+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/NetworkManager.git/commit/?id=9d7dc20fbdb0dc0d8b0c57285963758055b31a26'/>
<id>9d7dc20fbdb0dc0d8b0c57285963758055b31a26</id>
<content type='text'>
We don't need an external CheckpointItem, just to wrap the
CList instance. Embed it directly in NMCheckpoint.

Sure, that exposes the checkpoints_lst field in the (internal)
header file, hiding the private member less.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
We don't need an external CheckpointItem, just to wrap the
CList instance. Embed it directly in NMCheckpoint.

Sure, that exposes the checkpoints_lst field in the (internal)
header file, hiding the private member less.
</pre>
</div>
</content>
</entry>
<entry>
<title>libnm: add nm-autoptr.h header</title>
<updated>2018-03-27T08:36:12+00:00</updated>
<author>
<name>Thomas Haller</name>
<email>thaller@redhat.com</email>
</author>
<published>2018-03-19T09:34:58+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/NetworkManager.git/commit/?id=ff8e56336574a168d427a993d666d9e038aacbaf'/>
<id>ff8e56336574a168d427a993d666d9e038aacbaf</id>
<content type='text'>
"nm-autoptr.h" is done in a way that allows you to copy the header
in your source tree to support older versions of libnm, that didn't
contain the header yet. For example, we might want to use it in
network-manager-applet, but we don't want to bump the libnm dependency
to 1.11.2+ only to get this functionality.

Note that G_DEFINE_AUTOPTR_CLEANUP_FUNC() was added in glib 2.43.4,
and requires compiler support for the cleanup attribute. The compiler
support is taken as given, because we rely on it already. However,
NetworkManager and network-manager-applet still don't depend on a glib
version recent enough to provide these macros. To actually use them
(*inside*) NetworkManager/network-manager-applet, we either would have
to bump the glib minimal dependency, or reimplement g_autoptr in
/shared/nm-utils/nm-glib.h compat header.

https://bugzilla.gnome.org/show_bug.cgi?id=794294
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
"nm-autoptr.h" is done in a way that allows you to copy the header
in your source tree to support older versions of libnm, that didn't
contain the header yet. For example, we might want to use it in
network-manager-applet, but we don't want to bump the libnm dependency
to 1.11.2+ only to get this functionality.

Note that G_DEFINE_AUTOPTR_CLEANUP_FUNC() was added in glib 2.43.4,
and requires compiler support for the cleanup attribute. The compiler
support is taken as given, because we rely on it already. However,
NetworkManager and network-manager-applet still don't depend on a glib
version recent enough to provide these macros. To actually use them
(*inside*) NetworkManager/network-manager-applet, we either would have
to bump the glib minimal dependency, or reimplement g_autoptr in
/shared/nm-utils/nm-glib.h compat header.

https://bugzilla.gnome.org/show_bug.cgi?id=794294
</pre>
</div>
</content>
</entry>
<entry>
<title>dhcp: remove unused nm_utils_resolve_conf_parse() function</title>
<updated>2018-03-20T20:03:20+00:00</updated>
<author>
<name>Thomas Haller</name>
<email>thaller@redhat.com</email>
</author>
<published>2018-03-09T17:56:17+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/NetworkManager.git/commit/?id=868c3cedfd859c9e496cda158a7252105eddf7d1'/>
<id>868c3cedfd859c9e496cda158a7252105eddf7d1</id>
<content type='text'>
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
</pre>
</div>
</content>
</entry>
<entry>
<title>dhcp: remove unused nm_dhcp_dhclient_read_lease_ip_configs() function</title>
<updated>2018-03-20T20:03:20+00:00</updated>
<author>
<name>Thomas Haller</name>
<email>thaller@redhat.com</email>
</author>
<published>2018-03-09T17:56:17+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/NetworkManager.git/commit/?id=c77784b5eac2a063609063c6b00b14082666302b'/>
<id>c77784b5eac2a063609063c6b00b14082666302b</id>
<content type='text'>
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
</pre>
</div>
</content>
</entry>
<entry>
<title>connectivity: always build nm-connectivity.c source</title>
<updated>2018-03-19T13:46:55+00:00</updated>
<author>
<name>Thomas Haller</name>
<email>thaller@redhat.com</email>
</author>
<published>2018-02-19T18:50:18+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/NetworkManager.git/commit/?id=c1054ec8ffdfeee74fca6155ef2fcd39ac7283a6'/>
<id>c1054ec8ffdfeee74fca6155ef2fcd39ac7283a6</id>
<content type='text'>
We already do conditional build with "#if WITH_CONCHECK".
Get rid of the conditional in the makefile and instead do
conditional compilating inside the source file "nm-connectivity.c".

The advantage is, now if you want to know which parts are build,
you only need to grep for the WITH_CONCHECK preprocessor define
instead of also caring about the conditional in Makefile.am and
meson.build.

It doesn't change the fact of conditional compilation. But it
consistently uses one mechanism to achieve it.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
We already do conditional build with "#if WITH_CONCHECK".
Get rid of the conditional in the makefile and instead do
conditional compilating inside the source file "nm-connectivity.c".

The advantage is, now if you want to know which parts are build,
you only need to grep for the WITH_CONCHECK preprocessor define
instead of also caring about the conditional in Makefile.am and
meson.build.

It doesn't change the fact of conditional compilation. But it
consistently uses one mechanism to achieve it.
</pre>
</div>
</content>
</entry>
<entry>
<title>core/dbus: rework D-Bus implementation to use lower layer GDBusConnection API</title>
<updated>2018-03-12T17:37:08+00:00</updated>
<author>
<name>Thomas Haller</name>
<email>thaller@redhat.com</email>
</author>
<published>2018-02-26T12:51:52+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/NetworkManager.git/commit/?id=297d4985abcc7b571b8c090ee90622357fc60e16'/>
<id>297d4985abcc7b571b8c090ee90622357fc60e16</id>
<content type='text'>
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 &gt;/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 &gt; /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
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
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 &gt;/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 &gt; /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
</pre>
</div>
</content>
</entry>
<entry>
<title>core: rename "nm-bus-manager.h" to "nm-dbus-manager.h"</title>
<updated>2018-03-12T17:03:07+00:00</updated>
<author>
<name>Thomas Haller</name>
<email>thaller@redhat.com</email>
</author>
<published>2018-03-02T04:55:21+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/NetworkManager.git/commit/?id=a1f37964f00a0dd66326d4f40564c223c9ecc88e'/>
<id>a1f37964f00a0dd66326d4f40564c223c9ecc88e</id>
<content type='text'>
The next commit will completely rework NMBusManager and replace
NMExportedObject by a new type NMDBusObject.

Originally, NMDBusObject was added along NMExportedObject to ease
the rework and have compilable, intermediate stages of refactoring. Now,
I think the new name is better, because NMDBusObject is very strongly related
to the bus manager and the old name NMExportedObject didn't make that
clear.

I also slighly prefer the name NMDBusObject over NMBusObject, hence
for consistancy, also rename NMBusManager to NMDBusManager.

This commit only renames the file for a nicer diff in the next commit.
It does not actually update the type name in sources. That will be done
later.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
The next commit will completely rework NMBusManager and replace
NMExportedObject by a new type NMDBusObject.

Originally, NMDBusObject was added along NMExportedObject to ease
the rework and have compilable, intermediate stages of refactoring. Now,
I think the new name is better, because NMDBusObject is very strongly related
to the bus manager and the old name NMExportedObject didn't make that
clear.

I also slighly prefer the name NMDBusObject over NMBusObject, hence
for consistancy, also rename NMBusManager to NMDBusManager.

This commit only renames the file for a nicer diff in the next commit.
It does not actually update the type name in sources. That will be done
later.
</pre>
</div>
</content>
</entry>
<entry>
<title>netlink: drop libnl3 dependency</title>
<updated>2018-02-21T11:08:46+00:00</updated>
<author>
<name>Thomas Haller</name>
<email>thaller@redhat.com</email>
</author>
<published>2018-02-15T15:54:30+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/NetworkManager.git/commit/?id=3fab322a203ae55adfb934b7fff246e99def2eb2'/>
<id>3fab322a203ae55adfb934b7fff246e99def2eb2</id>
<content type='text'>
From libnl3, we only used the helper function to parse/generate netlink
messages and the socket functions to send/receive messages. We don't
need an external dependency to do that, it is simple enough.

Drop the libnl3 dependency, and replace all missing code by directly
copying it from libnl3 sources. At this point, I mostly tried to
import the required bits to make it working with few modifications.

Note that this increases the binary size of NetworkManager by 4736 bytes
for contrib/rpm build on x86_64. In the future, we can simplify the code
further.

A few modifications from libnl3 are:

- netlink errors NLE_* are now in the domain or regular errno.
  The distinction of having to bother with two kinds of error
  number domains was annoying.

- parts of the callback handling is copied partially and unused parts
  are dropped. Especially, the verbose/debug handlers are not used.
  In following commits, the callback handling will be significantly
  simplified.

- the complex handling of seleting ports was simplified. We now always
  let kernel choose the right port automatically.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
From libnl3, we only used the helper function to parse/generate netlink
messages and the socket functions to send/receive messages. We don't
need an external dependency to do that, it is simple enough.

Drop the libnl3 dependency, and replace all missing code by directly
copying it from libnl3 sources. At this point, I mostly tried to
import the required bits to make it working with few modifications.

Note that this increases the binary size of NetworkManager by 4736 bytes
for contrib/rpm build on x86_64. In the future, we can simplify the code
further.

A few modifications from libnl3 are:

- netlink errors NLE_* are now in the domain or regular errno.
  The distinction of having to bother with two kinds of error
  number domains was annoying.

- parts of the callback handling is copied partially and unused parts
  are dropped. Especially, the verbose/debug handlers are not used.
  In following commits, the callback handling will be significantly
  simplified.

- the complex handling of seleting ports was simplified. We now always
  let kernel choose the right port automatically.
</pre>
</div>
</content>
</entry>
<entry>
<title>build: disable -Wgnu-variable-sized-type-not-at-end for systemd code</title>
<updated>2018-02-16T15:09:50+00:00</updated>
<author>
<name>Lubomir Rintel</name>
<email>lkundrak@v3.sk</email>
</author>
<published>2018-02-16T15:05:31+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/NetworkManager.git/commit/?id=9daa9346e2bfca4aa8ef6c459cf9ad8ccf81a8c9'/>
<id>9daa9346e2bfca4aa8ef6c459cf9ad8ccf81a8c9</id>
<content type='text'>
clang 5.0.1 complains:

  src/systemd/src/libsystemd-network/dhcp6-option.c:40:28: error:
      field 'option' with variable sized type 'struct DHCP6Option' not at
      the end of a struct or class is a GNU extension
      [-Werror,-Wgnu-variable-sized-type-not-at-end]
        struct DHCP6Option option;
                           ^

systemd disables this warning too.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
clang 5.0.1 complains:

  src/systemd/src/libsystemd-network/dhcp6-option.c:40:28: error:
      field 'option' with variable sized type 'struct DHCP6Option' not at
      the end of a struct or class is a GNU extension
      [-Werror,-Wgnu-variable-sized-type-not-at-end]
        struct DHCP6Option option;
                           ^

systemd disables this warning too.
</pre>
</div>
</content>
</entry>
<entry>
<title>build: fix glib dependency to require at least 2.40</title>
<updated>2018-02-16T12:26:07+00:00</updated>
<author>
<name>Thomas Haller</name>
<email>thaller@redhat.com</email>
</author>
<published>2018-02-16T12:01:12+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/NetworkManager.git/commit/?id=997cce7c90bf059f93fa9391bad591c56688e4ac'/>
<id>997cce7c90bf059f93fa9391bad591c56688e4ac</id>
<content type='text'>
Fixes: 8a46b25cfa8c33daa8af37bb8103ca02286001b1
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Fixes: 8a46b25cfa8c33daa8af37bb8103ca02286001b1
</pre>
</div>
</content>
</entry>
</feed>
