Re: [RFC PATCH v3 3/3] platform: add support for WireGuard links
- From: Javier Arteaga <jarteaga jbeta is>
- To: Beniamino Galvani <bgalvani redhat com>
- Cc: Thomas Haller <thaller redhat com>, "Jason A. Donenfeld" <Jason zx2c4 com>, networkmanager-list gnome org
- Subject: Re: [RFC PATCH v3 3/3] platform: add support for WireGuard links
- Date: Wed, 20 Jun 2018 10:16:07 +0100
Hi Beniamino,
Thanks for your review!
On Wed, Jun 20, 2018 at 10:39:00AM +0200, Beniamino Galvani wrote:
+typedef struct {
+ NMIPAddr ip;
+ guint8 family;
+ guint8 cidr;
Just a nitpick, feel free to ignore, but perhaps 'mask' would be a
better name than 'cidr'.
Sure, if it parses better I'm all for it :)
[...]
@@ -7103,6 +7416,8 @@ constructed (GObject *_object)
g_assert (!nle);
_LOGD ("Generic netlink socket established: port=%u, fd=%d", nl_socket_get_local_port (priv->genlh),
nl_socket_get_fd (priv->genlh));
+ priv->wireguard_family_id = _support_genl_family (priv->genlh, "wireguard");
+
Since the genl family id is already determined when needed (i.e. in
wireguard_get_link_properties()), I don't think we need to do it here
too.
Yeah you're right, I'll drop it.
+void
+nm_platform_lnk_wireguard_hash_update (const NMPlatformLnkWireguard *obj, NMHashState *h)
+{
+ nm_hash_update_vals (h,
+ obj->private_key,
+ obj->public_key,
+ obj->listen_port,
+ obj->fwmark);
+}
This gives the following compile error here:
CC src/platform/src_libNetworkManagerBase_la-nm-platform.lo
In file included from ./shared/nm-default.h:292,
from src/platform/nm-platform.c:21:
src/platform/nm-platform.c: In function ‘nm_platform_lnk_wireguard_hash_update’:
./shared/nm-utils/nm-hash-utils.h:121:57: error: initialization of ‘unsigned char’ from ‘const guint8 *’
{aka ‘const unsigned char *’} makes integer from pointer without a cast [-Werror=int-conversion]
#define _NM_HASH_COMBINE_VALS_val_x_4( y, ...) ._v4 = (y), _NM_HASH_COMBINE_VALS_val_x_3 (__VA_ARGS__)
because private and public keys are guint8[]; I think you should use
nm_hash_update() for them.
Oof. These were correctly using nm_hash_update in v2 too :/
Will fix.
(Reviewing my build settings now, it seems I lost -Werror in the process
of trying out meson, and ended up missing this warning.)
The rest LGTM, thanks.
Beniamino
Thank you!
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]