WireGuard Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] Don't delete peers if not needed on `setconf`
@ 2019-11-17 13:59 Grzegorz Nosek
  2019-11-17 13:59 ` Grzegorz Nosek
  0 siblings, 1 reply; 4+ messages in thread
From: Grzegorz Nosek @ 2019-11-17 13:59 UTC (permalink / raw)
  To: wireguard

Disclaimer: this patch has received only very light testing. Consider it
an invitation to discussion rather than battle-tested production code.

Disclaimer 2: I'm not subscribed, so please CC all replies.

Disclaimer 3: this is the first email ever I'm sending via git-send-email,
so please excuse any etiquette breaches :)

Without this patch, `wg setconf` causes a brief outage on the wg interface
since all peers are removed and have to be readded and redo the handshake.
I'm running WireGuard in a highly dynamic environment where peers come
and go and the reloads are frequent enough that the downtime is noticeable
(several seconds of downtime per minute isn't really acceptable for me).

Right now I'm working around by using `addconf` instead of `setconf`
and a bash one-liner in cron to clean the dead peers once in a while[*].

Still, I took a look at the driver code and it looks like it would be pretty
easy to not remove the peers unless they're really going away:
1. Mark all peers for potential deletion instead of removing them outright
2. Unmark any peer touched by set_peer (i.e. present in the netlink message)
3. Remove all marked peers

My patch basically does just that.

Please take a look and let me know what you think.

Thanks,
 Grzegorz Nosek

* Here's the one-liner if anyone cares :)

#!/bin/bash

INTERFACE=${1:-wg0}

diff -u <(wg showconf $INTERFACE | grep PublicKey | sort) <(grep PublicKey /etc/wireguard/$INTERFACE.conf | sort) | grep ^-PublicKey | awk '{ print $3 }' | xargs -iPEER wg set $INTERFACE peer PEER remove


_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] Don't delete peers if not needed on `setconf`
  2019-11-17 13:59 [PATCH] Don't delete peers if not needed on `setconf` Grzegorz Nosek
@ 2019-11-17 13:59 ` Grzegorz Nosek
  2019-11-27 10:22   ` Jason A. Donenfeld
  0 siblings, 1 reply; 4+ messages in thread
From: Grzegorz Nosek @ 2019-11-17 13:59 UTC (permalink / raw)
  To: wireguard; +Cc: Grzegorz Nosek

This prevents dropping sessions during `wg setconf` without manually
removing dead peers.

Signed-off-by: Grzegorz Nosek <root@localdomain.pl>
---
 src/netlink.c |  6 +++++-
 src/peer.c    | 31 +++++++++++++++++++++++++++++++
 src/peer.h    |  3 +++
 3 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/src/netlink.c b/src/netlink.c
index 190e405..a249380 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -434,6 +434,8 @@ static int set_peer(struct wg_device *wg, struct nlattr **attrs)
 		goto out;
 	}
 
+	peer->remove_me = false;
+
 	if (preshared_key) {
 		down_write(&peer->handshake.lock);
 		memcpy(&peer->handshake.preshared_key, preshared_key,
@@ -543,7 +545,7 @@ static int wg_set_device(struct sk_buff *skb, struct genl_info *info)
 	}
 
 	if (flags & WGDEVICE_F_REPLACE_PEERS)
-		wg_peer_remove_all(wg);
+		wg_peer_mark(wg);
 
 	if (info->attrs[WGDEVICE_A_PRIVATE_KEY] &&
 	    nla_len(info->attrs[WGDEVICE_A_PRIVATE_KEY]) ==
@@ -599,6 +601,8 @@ skip_set_private_key:
 	}
 	ret = 0;
 
+	if (flags & WGDEVICE_F_REPLACE_PEERS)
+		wg_peer_sweep(wg);
 out:
 	mutex_unlock(&wg->device_update_lock);
 	rtnl_unlock();
diff --git a/src/peer.c b/src/peer.c
index 071eedf..26e6df2 100644
--- a/src/peer.c
+++ b/src/peer.c
@@ -195,6 +195,37 @@ void wg_peer_remove_all(struct wg_device *wg)
 		peer_remove_after_dead(peer);
 }
 
+void wg_peer_mark(struct wg_device *wg)
+{
+	struct wg_peer *peer, *temp;
+
+	lockdep_assert_held(&wg->device_update_lock);
+	list_for_each_entry_safe(peer, temp, &wg->peer_list, peer_list) {
+		peer->remove_me = true;
+	}
+}
+
+void wg_peer_sweep(struct wg_device *wg)
+{
+	struct wg_peer *peer, *temp;
+	LIST_HEAD(dead_peers);
+
+	lockdep_assert_held(&wg->device_update_lock);
+
+	list_for_each_entry_safe(peer, temp, &wg->peer_list, peer_list) {
+		if (peer->remove_me) {
+			peer_make_dead(peer);
+			list_add_tail(&peer->peer_list, &dead_peers);
+		}
+	}
+	if (list_empty(&dead_peers))
+		return;
+
+	synchronize_rcu();
+	list_for_each_entry_safe(peer, temp, &dead_peers, peer_list)
+		peer_remove_after_dead(peer);
+}
+
 static void rcu_release(struct rcu_head *rcu)
 {
 	struct wg_peer *peer = container_of(rcu, struct wg_peer, rcu);
diff --git a/src/peer.h b/src/peer.h
index 23af409..b67dd73 100644
--- a/src/peer.h
+++ b/src/peer.h
@@ -64,6 +64,7 @@ struct wg_peer {
 	u64 internal_id;
 	struct napi_struct napi;
 	bool is_dead;
+	bool remove_me;
 };
 
 struct wg_peer *wg_peer_create(struct wg_device *wg,
@@ -79,5 +80,7 @@ static inline struct wg_peer *wg_peer_get(struct wg_peer *peer)
 void wg_peer_put(struct wg_peer *peer);
 void wg_peer_remove(struct wg_peer *peer);
 void wg_peer_remove_all(struct wg_device *wg);
+void wg_peer_mark(struct wg_device *wg);
+void wg_peer_sweep(struct wg_device *wg);
 
 #endif /* _WG_PEER_H */
-- 
2.17.1

_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Don't delete peers if not needed on `setconf`
  2019-11-17 13:59 ` Grzegorz Nosek
@ 2019-11-27 10:22   ` Jason A. Donenfeld
  2019-11-27 12:34     ` Lonnie Abelbeck
  0 siblings, 1 reply; 4+ messages in thread
From: Jason A. Donenfeld @ 2019-11-27 10:22 UTC (permalink / raw)
  To: Grzegorz Nosek; +Cc: WireGuard mailing list

Interesting implementation strategy. There's a pure-userspace version
of that too, here:

https://git.zx2c4.com/WireGuard/commit/?h=jd/syncconf
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Don't delete peers if not needed on `setconf`
  2019-11-27 10:22   ` Jason A. Donenfeld
@ 2019-11-27 12:34     ` Lonnie Abelbeck
  0 siblings, 0 replies; 4+ messages in thread
From: Lonnie Abelbeck @ 2019-11-27 12:34 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: WireGuard mailing list, Grzegorz Nosek


> On Nov 27, 2019, at 4:22 AM, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> 
> Interesting implementation strategy. There's a pure-userspace version
> of that too, here:
> 
> https://git.zx2c4.com/WireGuard/commit/?h=jd/syncconf

Our project has been using "syncconf" ever since Jason crafted it, works perfectly, solves the same issue as Grzegorz reported.

Lonnie

_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-17 13:59 [PATCH] Don't delete peers if not needed on `setconf` Grzegorz Nosek
2019-11-17 13:59 ` Grzegorz Nosek
2019-11-27 10:22   ` Jason A. Donenfeld
2019-11-27 12:34     ` Lonnie Abelbeck

WireGuard Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/wireguard/0 wireguard/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 wireguard wireguard/ https://lore.kernel.org/wireguard \
		wireguard@lists.zx2c4.com
	public-inbox-index wireguard

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/com.zx2c4.lists.wireguard


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git