<feed xmlns='http://www.w3.org/2005/Atom'>
<title>delta/NetworkManager.git/src/platform/tests/test-common.c, branch th/cli-strsplit</title>
<subtitle>gitlab.freedesktop.org: NetworkManager/NetworkManager.git
</subtitle>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/NetworkManager.git/'/>
<entry>
<title>platform: merge nm_platform_*_delete() delete functions</title>
<updated>2017-12-11T09:30:26+00:00</updated>
<author>
<name>Thomas Haller</name>
<email>thaller@redhat.com</email>
</author>
<published>2017-11-29T12:10:39+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/NetworkManager.git/commit/?id=7573594a2182d141e85b5fc1cb376b7e78a40853'/>
<id>7573594a2182d141e85b5fc1cb376b7e78a40853</id>
<content type='text'>
It only makes sense to call delete() with NMPObjects that
we obtained from the platform cache. Otherwise, if we didn't
get it from the cache in the first place, we wouldn't know
what to delete.

Hence, the input argument is (almost) always an NMPObject
in the first place. That is different from add(), where
we might create a new specific NMPlatform* instance on the
stack. For add() it makes slightly more sense to have different
functions depending on the type. For delete(), it doesn't.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
It only makes sense to call delete() with NMPObjects that
we obtained from the platform cache. Otherwise, if we didn't
get it from the cache in the first place, we wouldn't know
what to delete.

Hence, the input argument is (almost) always an NMPObject
in the first place. That is different from add(), where
we might create a new specific NMPlatform* instance on the
stack. For add() it makes slightly more sense to have different
functions depending on the type. For delete(), it doesn't.
</pre>
</div>
</content>
</entry>
<entry>
<title>platform/trivial: s/ADDRROUTE/OBJECT/ for the cache lookup</title>
<updated>2017-12-11T09:30:26+00:00</updated>
<author>
<name>Lubomir Rintel</name>
<email>lkundrak@v3.sk</email>
</author>
<published>2017-11-23T14:41:57+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/NetworkManager.git/commit/?id=93ac0e455b35269df8a053a86ca2a43cafa4d3ee'/>
<id>93ac0e455b35269df8a053a86ca2a43cafa4d3ee</id>
<content type='text'>
It's going to be useful for other objects that have a type (of course)
and an ifindex.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
It's going to be useful for other objects that have a type (of course)
and an ifindex.
</pre>
</div>
</content>
</entry>
<entry>
<title>platform/tests: drop bad comment</title>
<updated>2017-12-11T09:30:25+00:00</updated>
<author>
<name>Lubomir Rintel</name>
<email>lkundrak@v3.sk</email>
</author>
<published>2017-12-09T10:06:43+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/NetworkManager.git/commit/?id=23b7f24d9e7d5d0a126226a576b5309856d71154'/>
<id>23b7f24d9e7d5d0a126226a576b5309856d71154</id>
<content type='text'>
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
</pre>
</div>
</content>
</entry>
<entry>
<title>all: use nm_close() instead of close()</title>
<updated>2017-11-14T14:10:42+00:00</updated>
<author>
<name>Thomas Haller</name>
<email>thaller@redhat.com</email>
</author>
<published>2017-11-14T13:22:21+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/NetworkManager.git/commit/?id=5b29c2e5b93c30347919b40e0885280fdb83c1a5'/>
<id>5b29c2e5b93c30347919b40e0885280fdb83c1a5</id>
<content type='text'>
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
</pre>
</div>
</content>
</entry>
<entry>
<title>platform: tests: silence compilation warning</title>
<updated>2017-08-28T08:41:13+00:00</updated>
<author>
<name>Beniamino Galvani</name>
<email>bgalvani@redhat.com</email>
</author>
<published>2017-08-28T08:39:23+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/NetworkManager.git/commit/?id=f42c7ba2b3d9f6a6191f5340727aef1efda308fd'/>
<id>f42c7ba2b3d9f6a6191f5340727aef1efda308fd</id>
<content type='text'>
src/platform/tests/test-common.c: In function ‘_ip4_route_get’:
./src/platform/nmp-object.h:336:18: error: ‘o’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  return obj ? obj-&gt;_class-&gt;obj_type : NMP_OBJECT_TYPE_UNKNOWN;
               ~~~^~~~~~~~
src/platform/tests/test-common.c:308:19: note: ‘o’ was declared here
  const NMPObject *o;
                   ^

Fixes: cdd8c6579919e542fb643a2070bb7ea49faef694
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
src/platform/tests/test-common.c: In function ‘_ip4_route_get’:
./src/platform/nmp-object.h:336:18: error: ‘o’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  return obj ? obj-&gt;_class-&gt;obj_type : NMP_OBJECT_TYPE_UNKNOWN;
               ~~~^~~~~~~~
src/platform/tests/test-common.c:308:19: note: ‘o’ was declared here
  const NMPObject *o;
                   ^

Fixes: cdd8c6579919e542fb643a2070bb7ea49faef694
</pre>
</div>
</content>
</entry>
<entry>
<title>platform: return failure reason from route-add and return only netlink response</title>
<updated>2017-08-24T08:48:04+00:00</updated>
<author>
<name>Thomas Haller</name>
<email>thaller@redhat.com</email>
</author>
<published>2017-08-21T13:33:57+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/NetworkManager.git/commit/?id=774c8a811e74097641b84b47e4e6c5fe90440113'/>
<id>774c8a811e74097641b84b47e4e6c5fe90440113</id>
<content type='text'>
Let nm_platform_ip_route_add() and friends return an NMPlatformError
failure reason.

Also, do_add_addrroute() did not return the response from kernel.
Instead, it determined success/failure based on the presence of the
object in the cache. That is racy and does not allow to give a failure
reason from kernel.

Instead, determine success solely based on the netlink reply from
kernel. The received errno shall be authorative, there is no need
to second guess the response.

There is a problem that netlink is not a reliable protocol. In case
of receive buffer overflow, the response is lost and we don't know
whether the command succeeded (it likely did). It's unclear how to fix
that, but for now just return "unspecified" error. We probably avoid
that already by having a huge buffer size.

Also, downgrade the error message to &lt;warn&gt; level. &lt;error&gt; is really
for bugs only.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Let nm_platform_ip_route_add() and friends return an NMPlatformError
failure reason.

Also, do_add_addrroute() did not return the response from kernel.
Instead, it determined success/failure based on the presence of the
object in the cache. That is racy and does not allow to give a failure
reason from kernel.

Instead, determine success solely based on the netlink reply from
kernel. The received errno shall be authorative, there is no need
to second guess the response.

There is a problem that netlink is not a reliable protocol. In case
of receive buffer overflow, the response is lost and we don't know
whether the command succeeded (it likely did). It's unclear how to fix
that, but for now just return "unspecified" error. We probably avoid
that already by having a huge buffer size.

Also, downgrade the error message to &lt;warn&gt; level. &lt;error&gt; is really
for bugs only.
</pre>
</div>
</content>
</entry>
<entry>
<title>platform: fix cache to use kernel's notion for equality of routes</title>
<updated>2017-08-12T14:04:28+00:00</updated>
<author>
<name>Thomas Haller</name>
<email>thaller@redhat.com</email>
</author>
<published>2017-08-02T05:55:05+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/NetworkManager.git/commit/?id=cdd8c6579919e542fb643a2070bb7ea49faef694'/>
<id>cdd8c6579919e542fb643a2070bb7ea49faef694</id>
<content type='text'>
Until now, NetworkManager's platform cache for routes used the quadruple
network/plen,metric,ifindex for equaliy. That is not kernel's
understanding of how routes behave. For example, with `ip route append`
you can add two IPv4 routes that only differ by their gateway. To
the previous form of platform cache, these two routes would wrongly
look identical, as the cache could not contain both routes. This also
easily leads to cache-inconsistencies.

Now that we have NM_PLATFORM_IP_ROUTE_CMP_TYPE_ID, fix the route's
compare operator to match kernel's.

Well, not entirely. Kernel understands more properties for routes then
NetworkManager. Some of these properties may also be part of the ID according
to kernel. To NetworkManager such routes would still look identical as
they only differ in a property that is not understood. This can still
cause cache-inconsistencies. The only fix here is to add support for
all these properties in NetworkManager as well. However, it's less serious,
because with this commit we support several of the more important properties.
See also the related bug rh#1337855 for kernel.

Another difficulty is that `ip route replace` and `ip route change`
changes an existing route. The replaced route has the same
NM_PLATFORM_IP_ROUTE_CMP_TYPE_WEAK_ID, but differ in the actual
NM_PLATFORM_IP_ROUTE_CMP_TYPE_ID:

    # ip -d -4 route show dev v
    # ip monitor route &amp;
    # ip route add 192.168.5.0/24 dev v
    192.168.5.0/24 dev v scope link
    # ip route change 192.168.5.0/24 dev v scope 10
    192.168.5.0/24 dev v scope 10
    # ip -d -4 route show dev v
    unicast 192.168.5.0/24 proto boot scope 10

Note that we only got one RTM_NEWROUTE message, although from NMPCache's
point of view, a new route (with a particular ID) was added and another
route (with a different ID) was deleted. The cumbersome workaround is,
to keep an ordered list of the routes, and figure out which route was
replaced in response to an RTM_NEWROUTE. In absence of bugs, this should
work fine. However, as we only rely on events, we might wrongly
introduce a cache-inconsistancy as well. See the related bug rh#1337860.

Also drop nm_platform_ip4_route_get() and the like. The ID of routes
is complex, so it makes little sense to look up a route directly.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Until now, NetworkManager's platform cache for routes used the quadruple
network/plen,metric,ifindex for equaliy. That is not kernel's
understanding of how routes behave. For example, with `ip route append`
you can add two IPv4 routes that only differ by their gateway. To
the previous form of platform cache, these two routes would wrongly
look identical, as the cache could not contain both routes. This also
easily leads to cache-inconsistencies.

Now that we have NM_PLATFORM_IP_ROUTE_CMP_TYPE_ID, fix the route's
compare operator to match kernel's.

Well, not entirely. Kernel understands more properties for routes then
NetworkManager. Some of these properties may also be part of the ID according
to kernel. To NetworkManager such routes would still look identical as
they only differ in a property that is not understood. This can still
cause cache-inconsistencies. The only fix here is to add support for
all these properties in NetworkManager as well. However, it's less serious,
because with this commit we support several of the more important properties.
See also the related bug rh#1337855 for kernel.

Another difficulty is that `ip route replace` and `ip route change`
changes an existing route. The replaced route has the same
NM_PLATFORM_IP_ROUTE_CMP_TYPE_WEAK_ID, but differ in the actual
NM_PLATFORM_IP_ROUTE_CMP_TYPE_ID:

    # ip -d -4 route show dev v
    # ip monitor route &amp;
    # ip route add 192.168.5.0/24 dev v
    192.168.5.0/24 dev v scope link
    # ip route change 192.168.5.0/24 dev v scope 10
    192.168.5.0/24 dev v scope 10
    # ip -d -4 route show dev v
    unicast 192.168.5.0/24 proto boot scope 10

Note that we only got one RTM_NEWROUTE message, although from NMPCache's
point of view, a new route (with a particular ID) was added and another
route (with a different ID) was deleted. The cumbersome workaround is,
to keep an ordered list of the routes, and figure out which route was
replaced in response to an RTM_NEWROUTE. In absence of bugs, this should
work fine. However, as we only rely on events, we might wrongly
introduce a cache-inconsistancy as well. See the related bug rh#1337860.

Also drop nm_platform_ip4_route_get() and the like. The ID of routes
is complex, so it makes little sense to look up a route directly.
</pre>
</div>
</content>
</entry>
<entry>
<title>platform: extend API for adding routes</title>
<updated>2017-08-03T16:51:57+00:00</updated>
<author>
<name>Thomas Haller</name>
<email>thaller@redhat.com</email>
</author>
<published>2017-08-02T08:27:32+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/NetworkManager.git/commit/?id=d373855e9820e3646c652c8530ef14d7cc25f758'/>
<id>d373855e9820e3646c652c8530ef14d7cc25f758</id>
<content type='text'>
Via the flags of the RTM_NEWROUTE netlink message, kernel and iproute2
support various variants to add a route.

 - ip route add
 - ip route change
 - ip route replace
 - ip route prepend
 - ip route append
 - ip route test

Previously, our nm_platform_ip4_route_add() function was basically
`ip route replace`. In the future, we should rather user `ip route
append` instead.

Anyway, expose the netlink message flags in the API. This allows to
use the various forms, and makes it also more apparent to the user that
they even exist.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Via the flags of the RTM_NEWROUTE netlink message, kernel and iproute2
support various variants to add a route.

 - ip route add
 - ip route change
 - ip route replace
 - ip route prepend
 - ip route append
 - ip route test

Previously, our nm_platform_ip4_route_add() function was basically
`ip route replace`. In the future, we should rather user `ip route
append` instead.

Anyway, expose the netlink message flags in the API. This allows to
use the various forms, and makes it also more apparent to the user that
they even exist.
</pre>
</div>
</content>
</entry>
<entry>
<title>platform: pass full route object to platform delete function</title>
<updated>2017-07-25T04:44:12+00:00</updated>
<author>
<name>Thomas Haller</name>
<email>thaller@redhat.com</email>
</author>
<published>2017-07-11T14:38:49+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/NetworkManager.git/commit/?id=2861c59116693598f89e549d8a14b38e8dae012b'/>
<id>2861c59116693598f89e549d8a14b38e8dae012b</id>
<content type='text'>
Contrary to addresses, routes have no ID. When deleting a route,
you cannot just specify certain properties like network/plen,metric.

Well, actually you can specify only certain properties, but then kernel
will treat unspecified properties as wildcard and delete the first matching
route. That is not something we want, because we need to be in control which
exact route shall be deleted.

Also, rtm_tos *must* match. Even if we like the wildcard behavior,
we would need to pass TOS to nm_platform_ip4_route_delete() to be
able to delete routes with non-zero TOS. So, while certain properties
may be omitted, some must not. See how test_ip4_route_options() was
broken.

For NetworkManager it only makes ever sense to call delete on a route,
if the route is already fully known. Which means, we only delete routes
that we have already in the platform cache (otherwise, how would we know
that there is something to delete). Because of that, no longer have separate
IPv4 and IPv6 functions. Instead, have nm_platform_ip_route_delete() which
accepts a full NMPObject from the platform cache.

The code in core doesn't jet make use of this new functionality. It will
in the future.

At least, it fixes deleting routes with differing TOS.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Contrary to addresses, routes have no ID. When deleting a route,
you cannot just specify certain properties like network/plen,metric.

Well, actually you can specify only certain properties, but then kernel
will treat unspecified properties as wildcard and delete the first matching
route. That is not something we want, because we need to be in control which
exact route shall be deleted.

Also, rtm_tos *must* match. Even if we like the wildcard behavior,
we would need to pass TOS to nm_platform_ip4_route_delete() to be
able to delete routes with non-zero TOS. So, while certain properties
may be omitted, some must not. See how test_ip4_route_options() was
broken.

For NetworkManager it only makes ever sense to call delete on a route,
if the route is already fully known. Which means, we only delete routes
that we have already in the platform cache (otherwise, how would we know
that there is something to delete). Because of that, no longer have separate
IPv4 and IPv6 functions. Instead, have nm_platform_ip_route_delete() which
accepts a full NMPObject from the platform cache.

The code in core doesn't jet make use of this new functionality. It will
in the future.

At least, it fixes deleting routes with differing TOS.
</pre>
</div>
</content>
</entry>
<entry>
<title>platform: refactor nm_platform_ip4_address_sync()</title>
<updated>2017-07-25T04:44:12+00:00</updated>
<author>
<name>Thomas Haller</name>
<email>thaller@redhat.com</email>
</author>
<published>2017-07-11T12:25:08+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/NetworkManager.git/commit/?id=5fcca9ba3e37152b9fbe645a8436cacbcf846dc5'/>
<id>5fcca9ba3e37152b9fbe645a8436cacbcf846dc5</id>
<content type='text'>
To reuse array of NMPObject instances instead of creating
a GArray clone.

Also get rid of the nm_platform_ipx_address_get_all() functions.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
To reuse array of NMPObject instances instead of creating
a GArray clone.

Also get rid of the nm_platform_ipx_address_get_all() functions.
</pre>
</div>
</content>
</entry>
</feed>
