<feed xmlns='http://www.w3.org/2005/Atom'>
<title>delta/NetworkManager.git/src, branch lr/unmanaged-err</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>core/device: demote "strictly unamanged" error</title>
<updated>2022-10-14T20:22:55+00:00</updated>
<author>
<name>Lubomir Rintel</name>
<email>lkundrak@v3.sk</email>
</author>
<published>2022-10-11T16:28:13+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/NetworkManager.git/commit/?id=ebc42cf28acae39d813a9c8bcc172099250be279'/>
<id>ebc42cf28acae39d813a9c8bcc172099250be279</id>
<content type='text'>
The "device ... not available because device is strictly unmanaged" is
almost certainly the least interesting of the reasons why connection
can't be activated on a device.

Invent a new error level for it and demote it.

Before:

  Error: Connection activation failed: No suitable device found
         for this connection (device lo not available because
         device is strictly unmanaged).

After

  Error: Connection activation failed: No suitable device found
         for this connection (device eth0 not available because
         profile is not compatible with device (...)).
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
The "device ... not available because device is strictly unmanaged" is
almost certainly the least interesting of the reasons why connection
can't be activated on a device.

Invent a new error level for it and demote it.

Before:

  Error: Connection activation failed: No suitable device found
         for this connection (device lo not available because
         device is strictly unmanaged).

After

  Error: Connection activation failed: No suitable device found
         for this connection (device eth0 not available because
         profile is not compatible with device (...)).
</pre>
</div>
</content>
</entry>
<entry>
<title>platform: use NM_STR_HAS_PREFIX() where appropriate</title>
<updated>2022-10-11T12:27:19+00:00</updated>
<author>
<name>Thomas Haller</name>
<email>thaller@redhat.com</email>
</author>
<published>2022-09-28T07:53:35+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/NetworkManager.git/commit/?id=93ec6abf16a5af54b96a9ee953c3cca8cb437449'/>
<id>93ec6abf16a5af54b96a9ee953c3cca8cb437449</id>
<content type='text'>
Prefer it over strncmp(), because it seems easier to understand (to me).

Prefer it over g_str_has_prefix(), because it can directly expand
to a plain strncmp() -- instead of first humping to glib, then calling
strlen() before calling strncmp().
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Prefer it over strncmp(), because it seems easier to understand (to me).

Prefer it over g_str_has_prefix(), because it can directly expand
to a plain strncmp() -- instead of first humping to glib, then calling
strlen() before calling strncmp().
</pre>
</div>
</content>
</entry>
<entry>
<title>platform: in _linktype_get_type() check for devtype before the interface name</title>
<updated>2022-10-11T12:27:18+00:00</updated>
<author>
<name>Thomas Haller</name>
<email>thaller@redhat.com</email>
</author>
<published>2022-10-07T15:09:25+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/NetworkManager.git/commit/?id=46fdf8a40e951766840b55f1d6d37cc5d373e278'/>
<id>46fdf8a40e951766840b55f1d6d37cc5d373e278</id>
<content type='text'>
I think the devtype should be checked first, before the interface name.
Checking by name seems really very hacky, move that last.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
I think the devtype should be checked first, before the interface name.
Checking by name seems really very hacky, move that last.
</pre>
</div>
</content>
</entry>
<entry>
<title>platform: also recognize rmnet/ccmni with ARPHRD_RAWIP type</title>
<updated>2022-10-11T12:27:18+00:00</updated>
<author>
<name>Ratchanan Srirattanamet</name>
<email>ratchanan@ubports.com</email>
</author>
<published>2022-09-21T19:23:17+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/NetworkManager.git/commit/?id=061cc60fda5a8f231178969af3b4c847fea997c6'/>
<id>061cc60fda5a8f231178969af3b4c847fea997c6</id>
<content type='text'>
Turns out, modern rmnet_* devices doesn't use ARPHRD_ETHER arptype, but
ARPHRD_RAWIP. Also complicating the fact is that ARPHRD_RAWIP is
actually added in v4.14, but devices using kernel before that version
define this value as "530" in an out-of-tree patch [1].

Recognize this case and check explicitly for 3 values of arptype.

[1] https://github.com/LineageOS/android_kernel_google_msm-4.9/commit/54948008c293fdf48552a5c39c91c09c3eb76ed2

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1392
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Turns out, modern rmnet_* devices doesn't use ARPHRD_ETHER arptype, but
ARPHRD_RAWIP. Also complicating the fact is that ARPHRD_RAWIP is
actually added in v4.14, but devices using kernel before that version
define this value as "530" in an out-of-tree patch [1].

Recognize this case and check explicitly for 3 values of arptype.

[1] https://github.com/LineageOS/android_kernel_google_msm-4.9/commit/54948008c293fdf48552a5c39c91c09c3eb76ed2

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1392
</pre>
</div>
</content>
</entry>
<entry>
<title>cli: fix translation string for error message in set_property()</title>
<updated>2022-10-11T07:43:26+00:00</updated>
<author>
<name>Thomas Haller</name>
<email>thaller@redhat.com</email>
</author>
<published>2022-10-11T07:43:06+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/NetworkManager.git/commit/?id=d5d68235581fbf3553ffa86fc635555cbc0a6451'/>
<id>d5d68235581fbf3553ffa86fc635555cbc0a6451</id>
<content type='text'>
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1112
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1112
</pre>
</div>
</content>
</entry>
<entry>
<title>glib-aux: extend nm_uuid_generate_from_strings() to honor NULL values</title>
<updated>2022-10-11T07:03:17+00:00</updated>
<author>
<name>Thomas Haller</name>
<email>thaller@redhat.com</email>
</author>
<published>2022-10-07T13:13:03+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/NetworkManager.git/commit/?id=2d8651ba91dc1fd30f08f7ebc2cb0f8bab4d958d'/>
<id>2d8651ba91dc1fd30f08f7ebc2cb0f8bab4d958d</id>
<content type='text'>
When you call

  nm_uuid_generate_from_strings_strv(uuid_type, type_arg, v1, v2);

you'd probably expect that both values are honored in some way.
However, if v1 happened to be NULL, then previously v2 would be ignored.

Extend nm_uuid_generate_from_strings() to accept also NULL values and
pass on the length. Currently, there are no users of nm_uuid_generate_from_strings(),
so nobody is affected by this change.

Also extend nm_uuid_generate_from_strings_strv() to take a length
argument. It still accepts "-1" to take the input strv as a NULL
terminated array.

If a positive length is provided to nm_uuid_generate_from_strings_strv(),
it hashes the same UUID as the respective NULL terminated array. But of
course only, if there is no NULL inside the array. If there are any
NULLs, a distinct UUID gets generated.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
When you call

  nm_uuid_generate_from_strings_strv(uuid_type, type_arg, v1, v2);

you'd probably expect that both values are honored in some way.
However, if v1 happened to be NULL, then previously v2 would be ignored.

Extend nm_uuid_generate_from_strings() to accept also NULL values and
pass on the length. Currently, there are no users of nm_uuid_generate_from_strings(),
so nobody is affected by this change.

Also extend nm_uuid_generate_from_strings_strv() to take a length
argument. It still accepts "-1" to take the input strv as a NULL
terminated array.

If a positive length is provided to nm_uuid_generate_from_strings_strv(),
it hashes the same UUID as the respective NULL terminated array. But of
course only, if there is no NULL inside the array. If there are any
NULLs, a distinct UUID gets generated.
</pre>
</div>
</content>
</entry>
<entry>
<title>glib-aux: add and use nm_uuid_generate_from_strings_old()</title>
<updated>2022-10-11T07:03:17+00:00</updated>
<author>
<name>Thomas Haller</name>
<email>thaller@redhat.com</email>
</author>
<published>2022-10-07T13:16:52+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/NetworkManager.git/commit/?id=b5e7e48bc185cb4e544efa8be9144e2bda63081c'/>
<id>b5e7e48bc185cb4e544efa8be9144e2bda63081c</id>
<content type='text'>
For a long time we have a function like nm_uuid_generate_from_strings().
This was recently reworked and renamed, but it preserved behavior. Preserving
behavior is important for this function, because for the existing users,
we need to keep generating the same UUIDs.

Originally, this function was a variadic function with NULL sentinel.
That means, when you write

  nm_uuid_generate_from_strings(uuid_type, type_arg, v1, v2, v3, NULL);

and v2 happens to be NULL, then v3 is ignored. That is most likely not
what the user intended. Maybe they had a bug and v2 should not be NULL.
But nm_uuid_generate_from_strings() should not require that all
arguments are non-NULL and it should not ignore arguments after the
first NULL.

For example, one user works around this via

    uuid = nm_uuid_generate_from_strings_old("ibft",
                                             s_hwaddr,
                                             s_vlanid ? "V" : "v",
                                             s_vlanid ? s_vlanid : "",
                                             s_ipaddr ? "A" : "DHCP",
                                             s_ipaddr ? s_ipaddr : "");

which is cumbersome and ugly.

That will be fixed next, by adding a function that doesn't suffer
from this problem. But "this problem" is part of the API of the
function, we cannot just change it. Instead, rename it and all
users, so they can keep doing the same.

New users of course should no longer use the "old" function.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
For a long time we have a function like nm_uuid_generate_from_strings().
This was recently reworked and renamed, but it preserved behavior. Preserving
behavior is important for this function, because for the existing users,
we need to keep generating the same UUIDs.

Originally, this function was a variadic function with NULL sentinel.
That means, when you write

  nm_uuid_generate_from_strings(uuid_type, type_arg, v1, v2, v3, NULL);

and v2 happens to be NULL, then v3 is ignored. That is most likely not
what the user intended. Maybe they had a bug and v2 should not be NULL.
But nm_uuid_generate_from_strings() should not require that all
arguments are non-NULL and it should not ignore arguments after the
first NULL.

For example, one user works around this via

    uuid = nm_uuid_generate_from_strings_old("ibft",
                                             s_hwaddr,
                                             s_vlanid ? "V" : "v",
                                             s_vlanid ? s_vlanid : "",
                                             s_ipaddr ? "A" : "DHCP",
                                             s_ipaddr ? s_ipaddr : "");

which is cumbersome and ugly.

That will be fixed next, by adding a function that doesn't suffer
from this problem. But "this problem" is part of the API of the
function, we cannot just change it. Instead, rename it and all
users, so they can keep doing the same.

New users of course should no longer use the "old" function.
</pre>
</div>
</content>
</entry>
<entry>
<title>libnm: use binary search for nm_meta_setting_infos_by_gtype() for libnmc</title>
<updated>2022-10-11T06:59:49+00:00</updated>
<author>
<name>Thomas Haller</name>
<email>thaller@redhat.com</email>
</author>
<published>2022-09-06T09:29:20+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/NetworkManager.git/commit/?id=611f2c3a60af1895575e0a0d2802952ff35f3c6c'/>
<id>611f2c3a60af1895575e0a0d2802952ff35f3c6c</id>
<content type='text'>
The file "nm-meta-setting-base-impl.c" is shared by "libnm-core-impl" and
"libnmc-setting". For "libnm-core-impl" it uses a efficient lookup from the
gtype. For "libnmc-setting", that class information is not available, so
it did a linear search. Instead, do a binary search.

Tested:

    diff --git a/src/libnm-core-impl/nm-meta-setting-base-impl.c b/src/libnm-core-impl/nm-meta-setting-base-impl.c
    index 3434c858391f..62c366d2ca42 100644
    --- a/src/libnm-core-impl/nm-meta-setting-base-impl.c
    +++ b/src/libnm-core-impl/nm-meta-setting-base-impl.c
    @@ -821,6 +821,11 @@ nm_meta_setting_infos_by_gtype(GType gtype)
     {
         const NMMetaSettingInfo *setting_info;

    +#if _NM_META_SETTING_BASE_IMPL_LIBNM
    +    return _infos_by_gtype_binary_search(gtype);
    +#endif
    +    nm_assert_not_reached();
    +
     #if _NM_META_SETTING_BASE_IMPL_LIBNM
         setting_info = _infos_by_gtype_from_class(gtype);
     #else
    diff --git a/src/libnm-core-impl/tests/test-setting.c b/src/libnm-core-impl/tests/test-setting.c
    index 85d549eb766c..65fcafd076c9 100644
    --- a/src/libnm-core-impl/tests/test-setting.c
    +++ b/src/libnm-core-impl/tests/test-setting.c
    @@ -5134,6 +5134,29 @@ main(int argc, char **argv)
     {
         nmtst_init(&amp;argc, &amp;argv, TRUE);

    +    {
    +        gs_unref_object NMConnection *con = NULL;
    +        guint8                        ctr = 0;
    +        guint                         i;
    +
    +        con = nmtst_create_minimal_connection("test", NULL, NM_SETTING_WIRED_SETTING_NAME, NULL);
    +
    +        nm_connection_add_setting(con, nm_setting_wired_new());
    +
    +        nmtst_connection_normalize(con);
    +        nmtst_assert_connection_verifies_without_normalization(con);
    +
    +        for (i = 0; i &lt; 10000000; i++) {
    +            ctr += GPOINTER_TO_UINT(nm_connection_get_setting(con, NM_TYPE_SETTING_WIRED));
    +            ctr += GPOINTER_TO_UINT(nm_connection_get_setting(con, NM_TYPE_SETTING_CONNECTION));
    +            ctr += GPOINTER_TO_UINT(nm_connection_get_setting(con, NM_TYPE_SETTING_PROXY));
    +            ctr += GPOINTER_TO_UINT(nm_connection_get_setting(con, NM_TYPE_SETTING_WIREGUARD));
    +            ctr += GPOINTER_TO_UINT(nm_connection_get_setting(con, NM_TYPE_SETTING_IP4_CONFIG));
    +        }
    +
    +        return !!ctr;
    +    }
    +
         g_test_add_func("/libnm/test_connection_uuid", test_connection_uuid);

         g_test_add_func("/libnm/settings/test_nm_meta_setting_types_by_priority",

Results of `make src/libnm-core-impl/tests/test-setting &amp;&amp; libtool --mode=execute perf stat -r 5 -B src/libnm-core-impl/tests/test-setting`:

 1) previous linear search: 3,182.81 msec
 2) bsearch not inlined:    1,611.19 msec
 3) bsearch open-coded:     1,214.45 msec
 4) bsearch inlined:        1,167.70 msec
 5) lookup from class:      1,147.64 msec

 1) previous implementation
 2) using nm_array_find_bsearch()
 3) manually implementing binary search
 4) using nm_array_find_bsearch_inline()
 5) only available to libnm-core as it uses internal meta data

Note that for "libnm-core-impl" the implementation was already fast (5), and
it didn't change. It only changed to binary search for "libnmc-setting",
which arguably is a less strong use-case.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
The file "nm-meta-setting-base-impl.c" is shared by "libnm-core-impl" and
"libnmc-setting". For "libnm-core-impl" it uses a efficient lookup from the
gtype. For "libnmc-setting", that class information is not available, so
it did a linear search. Instead, do a binary search.

Tested:

    diff --git a/src/libnm-core-impl/nm-meta-setting-base-impl.c b/src/libnm-core-impl/nm-meta-setting-base-impl.c
    index 3434c858391f..62c366d2ca42 100644
    --- a/src/libnm-core-impl/nm-meta-setting-base-impl.c
    +++ b/src/libnm-core-impl/nm-meta-setting-base-impl.c
    @@ -821,6 +821,11 @@ nm_meta_setting_infos_by_gtype(GType gtype)
     {
         const NMMetaSettingInfo *setting_info;

    +#if _NM_META_SETTING_BASE_IMPL_LIBNM
    +    return _infos_by_gtype_binary_search(gtype);
    +#endif
    +    nm_assert_not_reached();
    +
     #if _NM_META_SETTING_BASE_IMPL_LIBNM
         setting_info = _infos_by_gtype_from_class(gtype);
     #else
    diff --git a/src/libnm-core-impl/tests/test-setting.c b/src/libnm-core-impl/tests/test-setting.c
    index 85d549eb766c..65fcafd076c9 100644
    --- a/src/libnm-core-impl/tests/test-setting.c
    +++ b/src/libnm-core-impl/tests/test-setting.c
    @@ -5134,6 +5134,29 @@ main(int argc, char **argv)
     {
         nmtst_init(&amp;argc, &amp;argv, TRUE);

    +    {
    +        gs_unref_object NMConnection *con = NULL;
    +        guint8                        ctr = 0;
    +        guint                         i;
    +
    +        con = nmtst_create_minimal_connection("test", NULL, NM_SETTING_WIRED_SETTING_NAME, NULL);
    +
    +        nm_connection_add_setting(con, nm_setting_wired_new());
    +
    +        nmtst_connection_normalize(con);
    +        nmtst_assert_connection_verifies_without_normalization(con);
    +
    +        for (i = 0; i &lt; 10000000; i++) {
    +            ctr += GPOINTER_TO_UINT(nm_connection_get_setting(con, NM_TYPE_SETTING_WIRED));
    +            ctr += GPOINTER_TO_UINT(nm_connection_get_setting(con, NM_TYPE_SETTING_CONNECTION));
    +            ctr += GPOINTER_TO_UINT(nm_connection_get_setting(con, NM_TYPE_SETTING_PROXY));
    +            ctr += GPOINTER_TO_UINT(nm_connection_get_setting(con, NM_TYPE_SETTING_WIREGUARD));
    +            ctr += GPOINTER_TO_UINT(nm_connection_get_setting(con, NM_TYPE_SETTING_IP4_CONFIG));
    +        }
    +
    +        return !!ctr;
    +    }
    +
         g_test_add_func("/libnm/test_connection_uuid", test_connection_uuid);

         g_test_add_func("/libnm/settings/test_nm_meta_setting_types_by_priority",

Results of `make src/libnm-core-impl/tests/test-setting &amp;&amp; libtool --mode=execute perf stat -r 5 -B src/libnm-core-impl/tests/test-setting`:

 1) previous linear search: 3,182.81 msec
 2) bsearch not inlined:    1,611.19 msec
 3) bsearch open-coded:     1,214.45 msec
 4) bsearch inlined:        1,167.70 msec
 5) lookup from class:      1,147.64 msec

 1) previous implementation
 2) using nm_array_find_bsearch()
 3) manually implementing binary search
 4) using nm_array_find_bsearch_inline()
 5) only available to libnm-core as it uses internal meta data

Note that for "libnm-core-impl" the implementation was already fast (5), and
it didn't change. It only changed to binary search for "libnmc-setting",
which arguably is a less strong use-case.
</pre>
</div>
</content>
</entry>
<entry>
<title>libnm: cleanup implementations for nm_meta_setting_infos_by_gtype()</title>
<updated>2022-10-11T06:59:48+00:00</updated>
<author>
<name>Thomas Haller</name>
<email>thaller@redhat.com</email>
</author>
<published>2022-09-06T09:29:20+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/NetworkManager.git/commit/?id=5f4eb5eed53e86e8a84c163a19b8c7a84807f752'/>
<id>5f4eb5eed53e86e8a84c163a19b8c7a84807f752</id>
<content type='text'>
The file "nm-meta-setting-base-impl.c" is present twice in our source
tree and compiled twice. Once with internal headers of libnm-core and
once with only public headers.

Consequently, there are two implementations for
nm_meta_setting_infos_by_gtype(), one that can benefit from internal
access to the data structure, and the one for libnmc, which cannot.

Refactor the implementations, in the hope to have it clearer.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
The file "nm-meta-setting-base-impl.c" is present twice in our source
tree and compiled twice. Once with internal headers of libnm-core and
once with only public headers.

Consequently, there are two implementations for
nm_meta_setting_infos_by_gtype(), one that can benefit from internal
access to the data structure, and the one for libnmc, which cannot.

Refactor the implementations, in the hope to have it clearer.
</pre>
</div>
</content>
</entry>
<entry>
<title>glib-aux: add an inlinable version of nm_array_find_bsearch()</title>
<updated>2022-10-11T06:59:48+00:00</updated>
<author>
<name>Thomas Haller</name>
<email>thaller@redhat.com</email>
</author>
<published>2022-10-07T10:25:58+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/NetworkManager.git/commit/?id=28fa48aee44cae8349b3614ee2b9c1837fea0074'/>
<id>28fa48aee44cae8349b3614ee2b9c1837fea0074</id>
<content type='text'>
To implement binary search is not very hard. It's almost easy enough to
just open-code it, without using the existing nm_array_find_bsearch() function.
In particular, because nm_array_find_bsearch() won't be inlined,
and thus it is slower than implementing it by hand.

Add nm_array_find_bsearch_inline() as a variant that will be inlined.
This actually is as fast as reimplementing it by hand (I measured),
which takes away any reason to avoid the function.

However, our headers get huge. That may be a problem for complication
time. To counter that a bit, only define the function when the caller
requests it with a NM_WANT_NM_ARRAY_FIND_BSEARCH_INLINE define.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
To implement binary search is not very hard. It's almost easy enough to
just open-code it, without using the existing nm_array_find_bsearch() function.
In particular, because nm_array_find_bsearch() won't be inlined,
and thus it is slower than implementing it by hand.

Add nm_array_find_bsearch_inline() as a variant that will be inlined.
This actually is as fast as reimplementing it by hand (I measured),
which takes away any reason to avoid the function.

However, our headers get huge. That may be a problem for complication
time. To counter that a bit, only define the function when the caller
requests it with a NM_WANT_NM_ARRAY_FIND_BSEARCH_INLINE define.
</pre>
</div>
</content>
</entry>
</feed>
