There really is no need for two(!) heap allocations while parsing
the netlink message. We already have it in the buffer. Just use it.
Note that netlink attributes need to be aligned to 4 bytes. But
nlmsg_next() already ensures that, so not even for alignment purpose we
need to clone the message.
Create a new "struct nl_msg_lite" that can hold pointers to everything
we need.
We almost always do the wrong thing in interactive add:
The software devices generally require an interactive name, but we don't
insist of asking for them; treating them as optional:
$ nmcli -a c add type dummy
There is 1 optional setting for General settings.
Do you want to provide it? (yes/no) [yes]
For some interface types (bridges, bonds, ...) we make up a name, presumably
for historical reasons. But we don't give the user an option to modify
them:
$ nmcli -a c add type bridge
<not asking for interface name at all>
There are 9 optional settings for Bridge device.
Do you want to provide them? (yes/no) [yes]
This fixes the above use cases -- still set the default, but be sure to
ask:
$ nmcli -a c add type dummy
Interface name:
$ nmcli -a c add type bridge
Interface name [nm-bridge1]:
Beautiful.
Do the same bookkeeping as would happen upon setting the "type" option
when the connection has a connection.type set upon its addition.
Otherwise the --ask mode is sad:
$ nmcli --ask c add connection.type team
** nm:ERROR:src/nmcli/connections.c:5648:connection_get_base_meta_setting_type: assertion failed: (base_setting)
Bail out! nm:ERROR:src/nmcli/connections.c:5648:connection_get_base_meta_setting_type: assertion failed: (base_setting)
Aborted (core dumped)
After the connection's type is set, some bookkeeping is necessary for
the interactive (--ask) mode: appropriate setting need to be added and
options enabled.
Currently it happens in an option setter; which runs when the "type"
options is present on the command line, or the value is set in a
response to interactive mode:
$ nmcli --ask c add type team
$ nmcli --ask c add
Connection type: team
But not when the property is set directly:
$ nmcli --ask c add connection.type team
** nm:ERROR:src/nmcli/connections.c:5648:connection_get_base_meta_setting_type: assertion failed: (base_setting)
Bail out! nm:ERROR:src/nmcli/connections.c:5648:connection_get_base_meta_setting_type: assertion failed: (base_setting)
Aborted (core dumped)
This doesn't fix the issue -- a followup commit (hopefully) will.
For new connections, this ensures the value in square brackets on
interactive add are always correct.
Apart from that, this allows us to initialize some non-default values
before asking (such as making up an interface name for some software
devices), and inform the user about what we picked:
Interface name [nm-bridge]:
This is slightly annoying:
$ nmcli -a c add type ethernet
There is 1 optional setting for General settings.
No point in asking if there's just one option. Just ask right away:
$ nmcli -a c add type ethernet
Interface name:
The interactive add is not too enthusiastic about not providing a value
in a list.
That is before on getting an empty line in ask_option() we take a
shortcut instead of dispatching to set_option(). That way we skip
setting the PROPERTY_INF_FLAG_DISABLED flag, causing the option to
be included in questionnaire_one_optional()'s info list.
There's no reason to avoid calling set_option() if we don't get a value;
set_option() handles NULL value just fine.
$ nmcli -a c add
Connection type: dummy
There is 1 optional setting for General settings.
Do you want to provide it? (yes/no) [yes]
Interface name [*]: lala
There are 2 optional settings for IPv4 protocol.
Do you want to provide them? (yes/no) [yes]
You can specify this option more than once. Press <Enter> when you're done.
IPv4 address (IP[/plen]) [none]:
You can specify this option more than once. Press <Enter> when you're done.
IPv4 address (IP[/plen]) [none]:
You can specify this option more than once. Press <Enter> when you're done.
IPv4 address (IP[/plen]) [none]:
...
Whether we use a socket blockingly or non-blocking is usually determined
upfront and does not change. Make it a parameter of nl_socket_new().
Also, it saves an additional syscall.
This is not good:
$ nmcli device delete nm-bond
Segmentation fault (core dumped)
Fixes: 5f9d2927ed ("nmcli/devices: use GPtrArray from get_device_list() directly")
Distinguish a OWE-TM enabled BSS (which itself is unencrypted) from the
OWE BSS actually employing encryption.
Signed-off-by: David Bauer <mail@david-bauer.net>
Prevent downgrade of Enhanced Open / OWE connection profiles
to unencrypted connections by forcing wpa_supplicant to use OWE.
Signed-off-by: David Bauer <mail@david-bauer.net>
- "priv->nlh" to "priv->sk_rtnl": as we also have an genl socket,
"nlh" is not a good name. The point is that this is rtnetlink.
Also, "h" sounds like a handle, that is, a file descriptor.
Make this clearer with a "sk_" prefix.
- "priv->genl" to "priv->sk_genl_sync": This socket is only used for synchronous
operations, that is, it is passed to various independent components, that use
it to send a request and wait for the response (while consuming all messages).
We will have a use for a second socket, hence the "_sync" part.
The "sk_" prefix is for consistency with "sk_rtnl".
- "priv->event_source" to "priv->rtnl_event_source". Just make it
clearer, that this is for the rtnetlink socket. In any case,
this field is hardly used at all, it can have a sturdy name.
Sockets are really a fundamental thing we require to operate.
We cannot meaningfully operate, if we fail to create them.
That is also why a too low file descriptor limit is fatal
and unsupported. This is similar with out of memory situations.
Just require that we always are able to create the generic
netlink socket.
There are only two callers of nl_socket_new(). One for NETLINK_GENERIC
and one for NETLINK_ROUTE.
We already were enabling ext-ack for the rtnetlink socket. Also enable
it for the genl socket.
Do that, but just moving this inside nl_socket_new(). I cannot imagine a
case where we don't want this.
Create and use new nl_socket_new().
nl_socket_alloc() really does nothing but allocating the struct and
initializing the fd to -1. In all cases, we want to call nl_connect()
right after.
Combine the two. Then we also cannot have a "struct nl_sock" without a
valid fd. This means several error checks can be dropped.
Note that former nl_connect() did several things at once. Maybe, for
more flexibility one would need to tweak what should be done there.
For now that is not necessary. In any case, if we need more flexibility,
then we would control what nl_connect() (now nl_socket_new()) does, and not
the split between nl_socket_alloc() and nl_connect().
Comments on the same line as field names are not rendered well by clang-format.
Even if manually edited, it seems not a preferable way to comment on a field.
Move the comment in the line before.
