Re: [RFC PATCH v3 3/3] platform: add support for WireGuard links

On Sun, Jun 17, 2018 at 12:34:58PM +0100, Javier Arteaga wrote:
Add support for a new wireguard link type to the platform code. For now
this only covers querying existing links via genetlink and parsing them
into platform objects.

    Changes in v2->v3:
    * Changed peers/allowedips from CList to arrays-of-structs
      (GArray wrappers are used during genl parsing stage to handle reallocs)
    * Wrote proper cmp/hash methods for peer objects
    * Split off two patches not really specific to WireGuard
    * Corrected WireGuard uapi identifier names
    * Addressed a few other comments on v2

 libnm-core/nm-core-types-internal.h |  25 +++
 src/nm-types.h                      |   2 +
 src/platform/nm-linux-platform.c    | 315 ++++++++++++++++++++++++++++
 src/platform/nm-platform.c          | 124 +++++++++++
 src/platform/nm-platform.h          |  15 ++
 src/platform/nmp-object.c           | 219 +++++++++++++++++++
 src/platform/nmp-object.h           |  10 +
 7 files changed, 710 insertions(+)

diff --git a/libnm-core/nm-core-types-internal.h b/libnm-core/nm-core-types-internal.h
index 4d43aaf45..f95652fa2 100644
--- a/libnm-core/nm-core-types-internal.h
+++ b/libnm-core/nm-core-types-internal.h
@@ -31,6 +31,31 @@ typedef struct {
      guint32 to;
 } NMVlanQosMapping;
+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'.

@@ -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

+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.

The rest LGTM, thanks.


Attachment: signature.asc
Description: PGP signature

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]