libnm exposes simplified variants of hexstr2bin in its public API. I
think that was a mistake, because libnm should provide NetworkManager
specific utils. It should not provide such string functions.
However, nmcli used to need this, so it was added to libnm.
The better approach is to add it to our internally shared static
library, so that all interested components can make use of it.
That is slightly complex, because we need to (DNS) resolve the endpoints,
and we also have to retry periodically. For example, initially we may be
unable to resolve an endpoint, but later we may be.
What is also interesting is that during assume and reapply, we may not
have all information in the profile. Most notably, the private keys will
be missing. We need to cope with that and not reconfigure keys. However,
we still need to resolve names and update the endpoints.
Use the script to test how GObject introspection with libnm's WireGuard
support works.
Also, since support for WireGuard peers is not yet implemented in nmcli
(or other clients), this script is rather useful.
For now only add the core settings, no peers' data.
To support peers and the allowed-ips of the peers is more complicated
and will be done later. It's more complicated because these are nested
lists (allowed-ips) inside a list (peers). That is quite unusual and to
conveniently support that in D-Bus API, in keyfile format, in libnm,
and nmcli, is a effort.
Also, it's further complicated by the fact that each peer has a secret (the
preshared-key). Thus we probably need secret flags for each peer, which
is a novelty as well (until now we require a fixed set of secrets per
profile that is well known).
We already have NMPLookup to express a set of objects that can be looked
up via the multi-index.
Change the behavior of nmp_cache_dirty_set_all(), not to accept an
object-type. Instead, make it generally useful and accept a lookup
instance that is used to filter the elements that should be set as
dirty.
for-each macros can be nice to use. However, such macros are potentially
error prone, as they abstract C control statements and scoping blocks.
I find it ugly to expand the macro to:
for (...)
if (...)
Instead, move the additional "if" check inside the loop's condition
expression, so that the macro only expands to one "for(;;)" statement.
It does not matter too much, but the code optimizes for deletion events.
In that case, we don't require parsing the full netlink message.
Instead, it's enough to parse only the ID part so that we can find the
object in the cache that is to be removed removed.
Fix that for qdisc/tfilter objects.
I think the code before was correct. At the very least because
we only run the while-loop at most once because multipath routes
are not supported.
However, it seems odd that the while loop checks for
"tlen >= rtnh->rtnh_len"
but later we do
"tlen -= RTNH_ALIGN (rtnh->rtnh_len)"
Well, arguably, tlen itself is aligned to 4 bytes (as kernel sends
the netlink message that way). So, it was indeed fine.
Still, confusing. Try to check more explicitly for the buffer sizes.
nla_data_as() has a static assertion that casting to the pointer is
valid (regarding the alignment of the structure). It also contains
an nm_assert() that the data is in fact large enough.
It's safer, hence prefer it a some places where it makes sense.
- nlmsg_append_struct() determines the size based on the argument.
It avoids typing, but more importantly: it avoids typing redundant
information (which we might get wrong).
- also, declare the structs as const, where possible.
nla_get_u64() was unlike all other nla_get_u*() implementations, in that it
would allow for a missing/invalid nla argument, and return 0.
Don't do this. For one, don't behave different than other getters.
Also, there really is no space to report errors. Hence, the caller must
make sure that the attribute is present and suitable -- like for other
nla_get_*() functions.
None of the callers relied on being able to pass NULL attribute.
Also, inline the function and use unaligned_read_ne64(). That is our
preferred way for reading unaligned data, not memcpy().
These are only nm_assert(), meaning on non-DEBUG builds they
are not enabled.
Callers are supposed to first check that the netlink attribute
is suitable. Hence, we just assert.
The policy for strings must indicate a minlen of at least 1.
Everything else is a bug, because the policy contains invalid
data -- and is determined at compile-time.
- use size_t arguments for the memory sizes. While sizes from netlink
API currently are int typed and inherrently limited, use the more
appropriate data type.
- rename the arguments. The "count" is really the size of the
destination buffer.
- return how many bytes we wanted to write (like g_strlcpy()).
That makes more sense than how many bytes we actually wrote
because previously, we could not detect truncation.
Anyway, none of the callers cared about the return-value either
way.
- let nla_strlcpy() return how many bytes we would like to have
copied. That way, the caller could detect string truncation.
In practice, no caller cared about that.
- the code before would also fill the entire buffer with zeros first,
like strncpy(). We still do that. However, only copy the bytes up
to the first NUL byte. The previous version would have copied
"a\0b\0" (with srclen 4) as "a\0b". Strip all bytes after the
first NUL character from src. That seems more correct here.
- accept nla argument as %NULL.
- drop explicit MAX sizes like
static const struct nla_policy policy[IFLA_INET6_MAX+1] = {
The compiler will deduce that.
It saves redundant information (which is possibly wrong). Also,
the max define might be larger than we actually need it, so we
just waste a few bytes of static memory and do unnecesary steps
during validation.
Also, the compiler will catch bugs, if the array size of policy/tb
is too short for what we access later (-Warray-bounds).
- avoid redundant size specifiers like:
static const struct nla_policy policy[IFLA_INET6_MAX+1] = {
...
struct nlattr *tb[IFLA_INET6_MAX+1];
...
err = nla_parse_nested (tb, IFLA_INET6_MAX, attr, policy);
- use the nla_parse*_arr() macros that determine the maxtype
based on the argument types.
- move declaration of "static const struct nla_policy policy" variable
to the beginning, before auto variables.
- drop unneeded temporay error variables.
The common idiom is to stack allocate the tb array. Hence,
the maxtype is redundant. Add macros that autodetect the
maxtype based on the C type infomation.
Also, there is a static assertion that the size of the policy
(if provided) matches.
In practice, we don't fail to create the nlmsg, because in glib
malloc() cannot fail and we always create large enough buffers.
Anyway, handle the error correctly, and reduce the in-progress
counter again.
Will be used later. The point is to set an IP address from
unvalidated/untrusted input (that is, the data length might
not match the address-family).
Will be used later when parsing netlink attributes.
