a4144785ab77 docs: include ./src in include path
efd6619234cd docs: use c-apidocs glob
git-subtree-dir: src/c-rbtree
git-subtree-split: a4144785ab77ecc0627898c7c60523b2368c6ecb
clang-3.4.2-9.el7 does not like nesting NM_MAX() macro inside nm_hash_update_vals() macro.
Workaround by using MAX() instead. NM_MAX() uses an expression statement and NM_UNIQ()
to evaluate the arguments only once. We don't need that here and glib's MAX() suffices.
CC src/libnm-platform/src_libnm_platform_libnm_platform_la-nm-platform.lo
../src/libnm-platform/nm-platform.c:8247:53: error: in-class initializer for static data member is not a constant expression
(guint8) NM_MAX(obj->weight, 1u));
^
../src/libnm-std-aux/nm-std-aux.h:399:40: note: expanded from macro 'NM_MAX'
#define NM_MAX(a, b) __NM_MAX(NM_UNIQ, a, NM_UNIQ, b)
^
../src/libnm-std-aux/nm-std-aux.h:402:39: note: expanded from macro '__NM_MAX'
typeof(a) NM_UNIQ_T(A, aq) = (a); \
^
../src/libnm-glib-aux/nm-hash-utils.h:124:36: note: expanded from macro 'nm_hash_update_vals'
NM_HASH_COMBINE_VALS(_val, __VA_ARGS__); \
^
Fixes: 8cc41d41fe ('platform: add NM_PLATFORM_IP_ROUTE_CMP_TYPE_ECMP_ID for comparing ECMP base route')
This is the version shipped in Fedora 37. As Fedora 37 is now out, the
core developers switch to it. Our gitlab-ci will also use that as base
image for the check-{patch.tree} tests and to generate the pages. There
is a need that everybody agrees on which clang-format version to use,
and that version should be the one of the currently used Fedora release.
Also update the used Fedora image in "contrib/scripts/nm-code-format-container.sh"
script.
The gitlab-ci still needs update in the following commit. The change
in isolation will break the "check-tree" test.
We sometimes have functions foo() and foo_full(), in which case
foo() has fewer arguments and just calls foo_full(). The "full"
function here is the more powerful one, and foo() is implemented
in terms of the former.
nm_platform_ip4_route_cmp_full() and m_platform_ip4_route_cmp() inverted
that pattern. The "_full" there stands for the full comparison, to not
allowing to select the comparison type.
That inconsistency is ugly. Also, these wrappers were used at only few
places. Let's drop them.
While at it, also drop nm_platform_qdisc_cmp() and rename
nm_platform_qdisc_cmp_full(). Here cmp()/cmp_full() followed the common
pattern foo()/foo_full(), but it's still hardly used and unnecessary.
When adding a new route we need to consider it contains extra nexthops
i.e it is a ECMP route. As we cannot modify the NMPObject once created,
we need to pass the extra nexthops as an argument.
We cannot use the original NMPObject because normalization is happening
during when adding the route.
When reading from netlink an ECMP IPv4 route, we need to parse the
multiple nexthops. In order to do that, we are introducing
NMPlatformIP4RtNextHop struct.
The first nexthop information will be kept at the original
NMPlatformIP4Route and the new property n_nexthops will indicate how
many nexthops we need to consider.
This test is inherently fragile, as it depends on starting processes,
wait for something and kill the process. There are timings involved
that are out of control of the test. Try to adjust the timing.
# NetworkManager-DEBUG: <debug> [1668755976.9741] kill child process test-s-4 (111487): sending SIGKILL...
# NetworkManager-DEBUG: <debug> [1668755976.9753] kill child process test-s-4 (111487): waiting for process to terminate after sending SIGTERM (15) and SIGKILL...
# NetworkManager-DEBUG: <debug> [1668755976.9758] kill child process test-s-4 (111487): after sending SIGTERM (15) and SIGKILL, process 111487 exited by signal 9 (5759 usec elapsed)
Bail out! GLib:ERROR:../src/core/tests/test-core-with-expect.c:154:test_nm_utils_kill_child_sync_do: Did not see expected message NetworkManager-DEBUG: *<debug> [*] kill child process test-s-4 (*): waiting up to 1 milliseconds for process to terminate normally after sending SIGTERM (15)...
Bail out! nm:ERROR:../src/core/tests/test-core-with-expect.c:457:test_nm_utils_kill_child: assertion failed (exit_status == 0): (6 == 0)
--- stderr ---
**
GLib:ERROR:../src/core/tests/test-core-with-expect.c:154:test_nm_utils_kill_child_sync_do: Did not see expected message NetworkManager-DEBUG: *<debug> [*] kill child process test-s-4 (*): waiting up to 1 milliseconds for process to terminate normally after sending SIGTERM (15)...
**
nm:ERROR:../src/core/tests/test-core-with-expect.c:457:test_nm_utils_kill_child: assertion failed (exit_status == 0): (6 == 0)
/builds/NetworkManager/NetworkManager/tools/run-nm-test.sh: line 337: 110662 Aborted "${NMTST_DBUS_RUN_SESSION[@]}" "${NMTST_LIBTOOL[@]}" "$NMTST_VALGRIND" --quiet --error-exitcode=$VALGRIND_ERROR --leak-check=full --gen-suppressions=all "${NMTST_SUPPRESSIONS[@]}" --num-callers=100 --log-file="$LOGFILE" "$TEST" "${TEST_ARGV[@]}"
Under normal circumstances, the timeout is not supposed to be hit.
I see it hit on gitlab-ci. Was that because the machine was very
busy? It's hard to say whether there was a legitimate problem here,
and more importantly, what that problem was.
Try to increase the timeout. If there is a real problem, we probably
will still hit the timeout.
We must consume the reference, like we would in the other case.
Interestingly, I am unable to reproduce a case where valgrind would
complain about the leak. But it is there nonetheless.
Fixes: 0a22f4e490 ('libnm: refactor tracking of NMSetting in NMConnection')
See wpa_supplicant commit [1]:
macsec: Make pre-shared CKN variable length
IEEE Std 802.1X-2010, 9.3.1 defines following restrictions for
CKN:
"MKA places no restriction on the format of the CKN, save that it
comprise an integral number of octets, between 1 and 32
(inclusive), and that all potential members of the CA use the same
CKN. No further constraints are placed on the CKNs used with PSKs,
..."
Hence do not require a 32 octet long CKN but instead allow a
shorter CKN to be configured.
This fixes interoperability with some Aruba switches, that do not
accept a 32 octet long CKN (only support shorter ones).
[1] https://w1.fi/cgit/hostap/commit/?id=b678ed1efc50e8da4638d962f8eac13312a4048f
When called with update_carrier=TRUE, nm_device_bring_up_full() checks
for carrier changes and it may queue a transition to DISCONNECTED
through the following call chain:
-> nm_device_bring_up_full()
-> nm_device_set_carrier_from_platform()
-> nm_device_set_carrier()
-> carrier_changed()
-> nm_device_queue_state()
In _set_state_full(state=UNAVAILABLE) after bringing the interface up
we also call nm_device_cleanup() which clears the enqueued state
change to DISCONNECTED. When this happens, the device remains in
UNAVAILABLE and never gets activated even if it was ready.
This was observed with macsec interfaces, but in theory can happen
with all those interfaces that get carrier immediately after being
brought up.
Avoid this issue by not checking the carrier synchronously from
_set_state_full(). The carrier change event will be processed in the
next asynchronous invocation of device_link_changed().
https://bugzilla.redhat.com/show_bug.cgi?id=2122564
In the next commit nm_device_bring_up() will be extended with a new
argument. Most callers just want to bring up the device synchronously
and don't care about the "no_firmware" argument. Introduce a
nm_device_bring_up_full() for callers that need special behavior.
Currently VPNs always apply the configuration sent by the server for
both address families. So, even if users set e.g. ipv6.method=disabled,
they might end up with IPv6 configured.
Change that and apply the automatic configuration only when the method
is "auto".
This is a change in behavior and as such it might be disruptive for
users that had a method different from "auto" and expected to have the
interface configured. However, that scenario seems unlikely and can be
easily fixed by setting the right method.
When creating NMPlatformRoutingRule from NMIPRouteRule object, the
protocol is being set to RTPROT_UNSPEC. According to linux kernel
documentation FRA_PROTOCOL indicates the originator of the rule.
In this case the route rule is coming from a connection and therefore
the originator of the rule is the user. The correct value is
RTPROT_STATIC which means the rule is installed by the administrator.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1457
Fixes: 3f9347745b ('core: add handling of IP routing rules to NMDevice')
This wasn't used. It also doesn't make much sense, because
g_dbus_connection_call() accepts a reply type argument. So if you want
to check the reply type, you can do so when making the call.
That differs from g_dbus_proxy_call(), where this might make some sense
and is actually used.
While at it, also drop _nm_dbus_typecheck_response() from the header
file. It seems not general purpose enough and is not necessarily a great
pattern. Because,
- for g_dbus_connection_call() you either specify the reply_type when making
the call, or you do some more elaborate handling of the result -- not merely
_nm_dbus_typecheck_response().
- for g_dbus_proxy_call(), you can use _nm_dbus_proxy_call_finish() to
get it, or again, you want some non-trivial type checking.
These are helpers for GDBusProxy. I think we should avoid GDBusProxy where possible,
and these functions too. Give the function a more specific name to show that this
is only for the proxy.
These are just general purpose D-Bus utils, based on glib and GDBus.
They fit perfectly to libnm-glib-aux. Move the code.
Also, there is already the file "src/core/nm-dbus-utils.c", having two
files with the same name on our source tree is just confusing.
Most users included this by accident, by including nm-connection.h. That
is not too great, becuase stuff it contains is by no means specific to
NMConnection.
Anyways, it's not like it would matter too that. I mainly care about it
being included in NetworkManager.h, so that there's one less special
case in a test that makes sure useful stuff from NetworkManager.h ends up
in gtk-doc (a separate commit).
libnm-core-impl/nm-setting-bond.c:1276: warning: Symbol name not found at the start of the comment block.
libnm-core-impl/nm-setting-vpn.c:1135: warning: Symbol name not found at the start of the comment block.
libnm-core-impl/nm-setting-vpn.c:1158: warning: Symbol name not found at the start of the comment block.
libnm-core-impl/nm-setting-wired.c:1560: warning: Symbol name not found at the start of the comment block.
libnm-client-impl/nm-dhcp-config.c:149: warning: Symbol name not found at the start of the comment block.
libnm-client-impl/nm-secret-agent-old.c:967: warning: Symbol name not found at the start of the comment block.
libnm-client-impl/nm-secret-agent-old.c:1010: warning: Symbol name not found at the start of the comment block.
libnm-client-impl/nm-secret-agent-old.c:1037: warning: Symbol name not found at the start of the comment block.
libnm-core-impl/nm-setting-bond.c:602: warning: Parameter description
for nm_setting_bond_validate_option::value (allow-none) is not used
from source code comment block.
Makes gtk-doc grumpy (but it likes getting grumpy too often for us to
actually pay attention, it seems):
libnm-core-impl/nm-utils.c:4342: warning: nm_utils_is_uuid is deprecated
in the inline comments, but no deprecation guards were found around
the declaration. (See the --deprecated-guards option for gtkdoc-scan.)
libnm-client-impl/nm-device-ovs-bridge.c:36: warning:
nm_device_ovs_bridge_get_slaves is deprecated in the inline comments,
...
libnm-client-impl/nm-device-ovs-port.c:36: warning:
nm_device_ovs_port_get_slaves is deprecated in the inline comments,
...
libnm-client-impl/nm-device-team.c:77: warning:
nm_device_team_get_slaves is deprecated in the inline comments,
...
gtk-doc gets confused by these, ignore them:
common.py:ParseFunctionDeclaration:541:WARNING:Cannot parse args for
function in "nm_device_get_hw_address) const char
*nm_device_dummy_get_hw_address(NMDeviceDummy *device
common.py:ParseFunctionDeclaration:541:WARNING:Cannot parse args for
function in "nm_device_get_ports) const GPtrArray
*nm_device_ovs_port_get_slaves(NMDeviceOvsPort *device
For some reason it's still confused if NM_AVAILABLE_* comes first.
I refuse to look into that, just reorder them.
We got a typedef of the same name, and that causes gtk-doc to generate
invalid docbook and complain aloud:
../xml/nm-client.xml:4118: element refsect2: validity error : ID NMDnsEntry already defined
<refsect2 id="NMDnsEntry" role="typedef" condition="since:1.6">
Warning: multiple "IDs" for constraint linkend: NMDnsEntry.
gtk-doc (and perhaps other tools) treat pound sign in comments
specially:
html/NMSetting8021x.html:1501: warning: no link for: "11" -> (<span class="type">11</span>).
In case the D-Bus interfaces start dropping off (typically all off them go
one by one when the object is being deleted), don't reset all the properties.
In particular, keep most properties around, only tear down "o" and "ao",
so that the object dependencies get torn down, but we still get enough
properties around to identify what the dead object was its heyday.
One example of where this is not good is when the device-removed signal
is emmitted, the device no longer has the ifname:
$ nmcli monitor
<quit NetworkManager>
(null): device removed
(null): device removed
...
Currently we assert that properties are reset on client teardown. That
is not the right thing to do and we're not going to do that in future.
However, what is important to test is that the properties are reset when
the daemon goes away. Test that.
The part where a device was created and its cleanup on client
description was only run randomly.
This is silly and gave me hard time. No reason not to be always running
it.
Previously we'd note if NM is stopped, but not if it's running.
I suppose it's nice for the user to know that the monitor started
running, but, it's also important for the monitor to be testable (so
that we know that we are ready to start adding mock objects, etc.)
This also gets rids of some duplication at expense of a little less
nuanced message.
NMSettingWired does not reject invalid flags. Filter them out in wake_on_lan_enable().
In practice, it makes no difference, the unknown flags were ignored anyway.
If there were any pause options and any non-pause options,
the created setting was invalid.
I don't think it's reasonably possible to parse the broken settings.
So there is no workaround trying to read the existing broken settings
from disk. Luckily, the broken setting was just silently ignored by
the parser, so you simply could not persist certain settings.
https://bugzilla.redhat.com/show_bug.cgi?id=2134569
Fixes: 652ddca04c ('ethtool: Introducing PAUSE support')
