The intermediate parsing step serves very little purpose.
The only use is to ensure that we always add the keys in a stable
order, but we can easily ensure that otherwise.
We only need to parse them to construct the GVariant. There is
no need to keep them around otherwise.
We still keep LldpAttrs array and don't construct the GVariant right
away. The benefit is that this way while parsing we set the array
fields, and afterwards, when we generate the string dictionary, the
keys are sorted.
Move the parsing of the LLDP attributes to a separate function.
In the next step, we will no longer keep all attribute around
and no longer parse them during lldp_neighbor_new().
One effect is that we can no longer (easily) declare the LLDP message as
invalid, if parsing the attributes fails. That makes IMO more sense,
because we should try to expose what little we could parse, and not
be forgiving to unexpected data. If we wanted, we still could hide such
neighbors entirely from being exposed, but that is not done, because
it seems better to expose the parts that were valid.
We actually only need to parse the attributes while constructing
the GVariant. In a first step decouple the tracking of the parsed
attributes from LldpNeighbor struct. More will follow.
Also, track sd_lldp_neighbor instance directly.
sd_lldp_neighbor is a perfectly reasonable container for keeping
track of the LLDP neighbor information. Just keep a reference to
it, and don't clone the data. Especially since the LLDP library
keeps a reference to this instance as well.
Also, to compare whether two neighbors are the same, it is sufficient
to only consider the raw data. Everything else depends on these fields
anyway.
This is only possible and useful becuase sd_lldp_neighbor is of course
immutable. It wouldn't make sense otherwise, but it also would be bad
design to mutate the sd_lldp_neighbor instances.
This couples our code slightly more to the systemd code, which we usually
try to avoid. But when we move away in the future from systemd LLDP library,
we anyway need to rework this heavily (and then too, we wouldn't want
to clone the data, when we could just share the reference).
Allocating and growing the buffer with realloc isn't really
complicated. Do that instead of using a CList.
Also, if there is only one element, then we can track it in-place.
NM_UTILS_LOOKUP_STR_DEFINE() is implemented via a switch statement.
You'd expect that the compiler could optimize that to plain lookup,
since all indexes are consecutive numbers. Anyway, my compiler doesn't,
so use the array ourself.
Note that NM_UTILS_LOOKUP_STR_DEFINE() is exactly intended to lookup
by enum/integer, if the enum values are not consecutive numbers. It may
not be best, when you can directly use the numbers as lookup index.
VARDICT sounds like it would be a variant of "a{sv}" type. But it
can be really any GVariant. Rename to make the type more generic.
This will be used to also hold a binary variant of type "ay".
Currently the LLDP parsing code uses GVariantBuild, which possibly does not
ensure a stable order of elements. Thus the test may not be stable.
However, that will be fixed very soon.
Yes, fewer lines of code is often better, if that means the code itself is
simpler. Code doesn't get simpler by cramping more in the same line.
Have every value on a separate line.
Also, vertically align the table.
Autoconnect-slaves currently forces an activation of all slaves, even
if there is already an active connection for them. This is bad because
at boot slaves first try to autoconnect, then the autoconnect-slaves
of the master kicks in and disconnects/reactivates them.
The only reason why the forceful reactivation was added was to fix
[1]; in that scenario, a slave connection is already active as
non-slave; then it is updated to be a slave; later, the master with
autoconnect-slaves is manually activated. NetworkManager should detect
that the slave connection must now be activated by autoconnect-slaves.
Add a specific check for such situation, instead of always
reactivating all slaves.
[1] https://bugzilla.redhat.com/show_bug.cgi?id=1845018
Fixes: 4985ca5ada ('manager: allow autoconnect-slaves to reconnect the same connection')
When a device is not marked as unmanaged, but also not actively managed
by NetworkManager, then NetworkManager will generate an in-memory
profile to represent the active state, if the device is up and
configured (with an IP address).
Such profiles are commonly named like "eth0", and they are utterly
confusing to users, because they look as if NetworkManager actually
manages the device, when it really just shows that somebody else configures
the device.
We should express this better in the UI, hence add flags to indicate
that.
In practice, such profiles are UNSAVED, NM_GENERATED, and VOLATILE. But
add an explicit flag to represent that.
https://bugzilla.redhat.com/show_bug.cgi?id=1816202
When there are two patch ports connected, each of them must reference
the other; however they can't be created in a single transaction
because they are part of different bridges (so, different
connections). Therefore, the first patch that gets activated will
always fail with "No usable peer $x exists in 'system' datapath" until
the second patch exists.
In theory we could also match the error message, however this doesn't
seem very robust as the message may slightly change in the future.
Psrse the 802.11 IEs after parsing the security information.
Previously the OWE transition mode flag was potentially removed
in case the security properties changed.
Signed-off-by: David Bauer <mail@david-bauer.net>
Commit 37e7fa38c2 ("nm-supplicant-interface: enable OWE security
when transition mode is available") adds the OWE security flag in
case a valid OWE transtition mode IE is present on the beacon.
It also removes the OWE security flag in case the Iinformation elements
of a beacon are updated and a OWE transition mode IE can't be found.
When a pure OWE AP updates it's Information Elements (e.g. BSS Load
Element), the OWE security flag is falsely removed.
Introduce a new NM_802_11_AP_SEC_KEY_MGMT_OWE_TM security flag and use
it exclusively for OWE transition mode. Don't use the
M_802_11_AP_SEC_KEY_MGMT_OWE security flag on transition-mode APs.
Signed-off-by: David Bauer <mail@david-bauer.net>
Systemd's LLDP client also internally tracks all neighbours, and it thus
already needs a maximum already. For systemd, that is currently 128.
We don't need to implement our own rate limiting on top of that,
because if we wouldn't trust the LLDP client to get this right,
it would be DoS-able already.
Also decrease the number of maximum neighbours from 4k to 128.
Note that already previously we wouldn't ever get more than 128
entries.
nm_keyfile_read() and nm_keyfile_write() will be public API.
As such, it must be flexible and extendible for future needs.
There is already the handler callback that fully solves this
(e.g. a future handler event could request whether a certain
behavior is enabled or not).
As additional possibility for future extension, add a flags
argument. Currently no flags are implemented.
From inside a callback 4 properties are potentially interesting
to all callbacks: the currenty group, key, setting and property-name.
Refactor the code to track these properties in NMKeyfileHandlerData
and distinguish between the property name and the keyfile key.
Setting the error on the callback does not work well from bindings.
Instead, let bindings call a (future) nm_keyfile_handler_data_fail_with_error()
function on the handler_data to indicate failure.
As the keyfile handler callback will become public API, it needs to be
usable via bindings. A plain void pointer is not usable. Instead, add
a new type that can be used via introspection.
This will become public API. The enum for read and write callback
serves very similar purposes. Merge them so that we have fewer
types in the public API.
6876b73ec113 all: rework logging to append logging messages to the event queue
65ca0e4bea32 client: check length of client id for n_dhcp4_client_config_set_client_id()
6a9ca10b7ad6 client: close timer file descriptor explicitly when EPOLL_CTL_ADD fails
e5f3b27f5301 connection: don't fail dispatch in case of receive errors
68790d5ec950 connection: keep trying after a failure in send()
788b03891cd0 n-dhcp4: fix logging macro
6cd0fd5e8b3f probe: request previous address after expiration
ef11c7079ad7 connection: avoid double free of NDhcp4Outgoing
922a9a2326e7 n-dhcp4: fix initialization of the 'secs' DHCP header field
7fff54117436 outgoing: fix uninitialized variable in n_dhcp4_outgoing_append()
27547faaf258 probe: move back to INIT after lease expires
d07cb4e32884 incoming: accept options that are longer than requested
34d365e2267f client: handle invalid return codes gracefully during n_dhcp4_client_dispatch()
2e8c6017f85b n-dhcp4: use packet socket in rebinding state
9a7c82143c06 n-dhcp4: support init-reboot state
e615e3f5b063 n-dhcp4/socket: use SO_REUSEADDR on UDP socket
8dbfa1ad2549 merge branch 'bengal:log'
6e50189fba86 n-dhcp4: log outgoing packets
aea0315d3862 n-dhcp4: log incoming packets
1ca64a82393d n-dhcp4: add logging API
72c16a0028ac client-probe: fix state transitions on timer dispatch
c023ae2ad829 merge branch 'leasetime'
780f94c333ca lease: add n_dhcp4_client_lease_get_basetime()
a94378274cf4 incoming: don't handle 0xFFFFFFFF timestamps special in n_dhcp4_incoming_query_u32()
6bb277aefe0b probe: unconditionally pass ownership of message in n_dhcp4_client_probe_dispatch_io()
1cf87c90c6a1 probe: fix leaking message during client probe
55239cef2d63 lease: remove unreachable code in n_dhcp4_incoming_get_timeouts()
fa5ee7903061 lease: expose the server IP address
0cfce4a4fe07 all: merge branch 'th/build-centos76'
cae4868e5f7e all: avoid {net,linux}/if.h clashes on old distros
05859a3e4d32 all: avoid c_min() macro to work with old GCC
0be7033dd940 probe: arm timers in bound state
8735cdb0f95f ci: switch to c-util automation
a0bb7c69a11a client: fallback to CLOCK_MONOTONIC for timerfd
308cb242caf7 ci: disable valgrind run
468c93195ad3 build: update submodules
9d0f690f9ee8 ci: switch to github actions
9ba143a037bb probe: allocate memory of right size in n_dhcp4_client_probe_option_new()
2546aa2c809d util/link: suppress gcc warning
e4a01f5870a6 ci: drop broken armv7hl
2e2fbfa18f2c build: update submodules
6277dfd20eca Merge pull request #2 from NetworkManager/th/declaration-after-statement
1b660ae2d12a outgoing: avoid "-Werror=declaration-after-statement" warning with static_assert
f34a54cd9719 client/probe: fix memory leak
21e68f3bba80 client/connection: fix memory leak
git-subtree-dir: shared/n-dhcp4
git-subtree-split: 6876b73ec113328712a5cdc2ffb9497fd774c0e2
../src/nm-manager.c: In function periodic_update_active_connection_timestamps:
../src/nm-manager.c:7358:43: error: t may be used uninitialized in this function [-Werror=maybe-uninitialized]
nm_settings_connection_update_timestamp (nm_active_connection_get_settings_connection (ac),
^
Fixes: 6f3ae8a563 ('core: in periodic_update_active_connection_timestamps() use same timestamp')
We don't need the mask argument. If the caller wants to check only for certain
flags, she can do that right away with
NM_FLAGS_ANY (nm_settings_connection_autoconnect_blocked_reason_get (sett_con), flags)
When the secrets for a connection are updated, unblock autoconnection
in case it had been blocked previously due to bad or no
secrets. Otherwise we would need to manually activate the connection
or restart NM to get another try with the new secrets.