Re: [RFC PATCH v2 2/2] platform: add support for WireGuard links



Hi,

On Wed, 2018-04-04 at 15:45 +0100, Javier Arteaga wrote:

I wonder, what happens if the wireguard module is not loaded
initially.
I guess, caching the family-id is right, but you need to retry if
it's
not avilable. For example, in wireguard_get_link_properties()

Makes sense.

Should we attempt a refetch once per RTM_*LINK message then? Or is
there
some reliable way to get notified when a new genl family becomes
available?

I don't know, and I didn't think of this before. We probably could
cache the family-id inside the NMPObjectWireguard instance, because as
long as that instance is in the cache, we can assume that the module
stays loaded and the ID doesn't change.

But seems complicated to get this right. In the first attempt, let's
just re-fetch the family-id every time we need it.


+static void
+_vt_cmd_obj_hash_update_lnk_wireguard (const NMPObject *obj,
NMHashState *h)
+{
+ nm_assert (NMP_OBJECT_GET_TYPE (obj) ==
NMP_OBJECT_TYPE_LNK_WIREGUARD);
+
+ nm_platform_lnk_wireguard_hash_update (&obj-
lnk_wireguard,
h);
+ nm_hash_update_val (h, obj->_lnk_wireguard.peers);

You must hash the entire content, not just the peers-pointer.

This will become irrelevant when we have a simple array-of-structs,
but:
if copy_lnk_wireguard was implemented by simply grabbing a ref to the
same GPtrArray, would it still be wrong to take a shortcut here by
hashing/testing just the pointer?

I think so. 

We really want to compare the content, not the current pointer value.

If you have two identically configured wireguard interfaces, their
NMPObjectWireguard instances should compare equal.

Also, if you receive a RTM_NEWLINK message, you anyway once need to
compare the content of the GPtrArray instance, to figure out whether
the update actually involved any (relevant) changes.



best,
Thomas

Attachment: signature.asc
Description: This is a digitally signed message part



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