peer: get rid of peer_for_each magic

Since the peer list is protected by the device_update_lock, and since
items are removed from the peer list before putting their final
reference, we don't actually need to take a reference when iterating.
This allows us to simplify the macro considerably.

Suggested-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>

3 files changed, 17 insertions, 41 deletions

diff --git a/src/device.c b/src/device.c
index 433da3f..cf6e2b2 100644
--- a/src/device.c
+++ b/src/device.c
@@ -26,7 +26,7 @@ static LIST_HEAD(device_list);
 static int open(struct net_device *dev)
 {
 	int ret;
-	struct wireguard_peer *peer, *temp;
+	struct wireguard_peer *peer;
 	struct wireguard_device *wg = netdev_priv(dev);
 #ifndef COMPAT_CANNOT_USE_IN6_DEV_GET
 	struct inet6_dev *dev_v6 = __in6_dev_get(dev);
@@ -53,11 +53,13 @@ static int open(struct net_device *dev)
 	ret = socket_init(wg);
 	if (ret < 0)
 		return ret;
-	peer_for_each (wg, peer, temp, true) {
+	mutex_lock(&wg->device_update_lock);
+	list_for_each_entry (peer, &wg->peer_list, peer_list) {
 		packet_send_staged_packets(peer);
 		if (peer->persistent_keepalive_interval)
 			packet_send_keepalive(peer);
 	}
+	mutex_unlock(&wg->device_update_lock);
 	return 0;
 }
 
@@ -65,19 +67,21 @@ static int open(struct net_device *dev)
 static int suspending_clear_noise_peers(struct notifier_block *nb, unsigned long action, void *data)
 {
 	struct wireguard_device *wg;
-	struct wireguard_peer *peer, *temp;
+	struct wireguard_peer *peer;
 
 	if (action != PM_HIBERNATION_PREPARE && action != PM_SUSPEND_PREPARE)
 		return 0;
 
 	rtnl_lock();
 	list_for_each_entry (wg, &device_list, device_list) {
-		peer_for_each (wg, peer, temp, true) {
+		mutex_lock(&wg->device_update_lock);
+		list_for_each_entry (peer, &wg->peer_list, peer_list) {
 			noise_handshake_clear(&peer->handshake);
 			noise_keypairs_clear(&peer->keypairs);
 			if (peer->timers_enabled)
 				del_timer(&peer->timer_zero_key_material);
 		}
+		mutex_unlock(&wg->device_update_lock);
 	}
 	rtnl_unlock();
 	rcu_barrier_bh();
@@ -89,14 +93,16 @@ static struct notifier_block clear_peers_on_suspend = { .notifier_call = suspend
 static int stop(struct net_device *dev)
 {
 	struct wireguard_device *wg = netdev_priv(dev);
-	struct wireguard_peer *peer, *temp;
+	struct wireguard_peer *peer;
 
-	peer_for_each (wg, peer, temp, true) {
+	mutex_lock(&wg->device_update_lock);
+	list_for_each_entry (peer, &wg->peer_list, peer_list) {
 		skb_queue_purge(&peer->staged_packet_queue);
 		timers_stop(peer);
 		noise_handshake_clear(&peer->handshake);
 		noise_keypairs_clear(&peer->keypairs);
 	}
+	mutex_unlock(&wg->device_update_lock);
 	skb_queue_purge(&wg->incoming_handshakes);
 	socket_uninit(wg);
 	return 0;
diff --git a/src/netlink.c b/src/netlink.c
index 72dfa2b..fed16dc 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -254,13 +254,13 @@ static int get_device_done(struct netlink_callback *cb)
 
 static int set_port(struct wireguard_device *wg, u16 port)
 {
-	struct wireguard_peer *peer, *temp;
+	struct wireguard_peer *peer;
 
 	if (wg->incoming_port == port)
 		return 0;
 	socket_uninit(wg);
 	wg->incoming_port = port;
-	peer_for_each (wg, peer, temp, false)
+	list_for_each_entry (peer, &wg->peer_list, peer_list)
 		socket_clear_peer_endpoint_src(peer);
 	if (!netif_running(wg->dev))
 		return 0;
@@ -400,10 +400,10 @@ static int set_device(struct sk_buff *skb, struct genl_info *info)
 	++wg->device_update_gen;
 
 	if (info->attrs[WGDEVICE_A_FWMARK]) {
-		struct wireguard_peer *peer, *temp;
+		struct wireguard_peer *peer;
 
 		wg->fwmark = nla_get_u32(info->attrs[WGDEVICE_A_FWMARK]);
-		peer_for_each (wg, peer, temp, false)
+		list_for_each_entry (peer, &wg->peer_list, peer_list)
 			socket_clear_peer_endpoint_src(peer);
 	}
 
@@ -428,7 +428,7 @@ static int set_device(struct sk_buff *skb, struct genl_info *info)
 			peer_remove(peer);
 		}
 		noise_set_static_identity_private_key(&wg->static_identity, private_key);
-		peer_for_each (wg, peer, temp, false) {
+		list_for_each_entry_safe (peer, temp, &wg->peer_list, peer_list) {
 			if (!noise_precompute_static_static(peer))
 				peer_remove(peer);
 		}
diff --git a/src/peer.h b/src/peer.h
index 3581c73..994c8b6 100644
--- a/src/peer.h
+++ b/src/peer.h
@@ -69,34 +69,4 @@ struct wireguard_peer *peer_lookup_by_index(struct wireguard_device *wg, u32 ind
 
 unsigned int peer_total_count(struct wireguard_device *wg);
 
-/* This is a macro iterator of essentially as follows, with the
- * exception that typing `break` in the ITERATOR_BODY will still
- * actually put the peer reference:
- *
- * if (__should_lock)
- *	mutex_lock(&(__wg)->device_update_lock);
- * else
- *	lockdep_assert_held(&(__wg)->device_update_lock)
- * list_for_each_entry_safe (__peer, __temp, &(__wg)->peer_list, peer_list) {
- *	__peer = peer_rcu_get(__peer);
- *	if (!__peer)
- *		continue;
- *	ITERATOR_BODY
- *	peer_put(__peer);
- * }
- * if (__should_lock)
- *	mutex_unlock(&(__wg)->device_update_lock);
- *
- * While it's really ugly to look at, the code gcc produces from it is actually perfect.
- */
-#define pfe_label(n) __PASTE(__PASTE(pfe_label_, n ## _), __LINE__)
-#define peer_for_each(__wg, __peer, __temp, __should_lock) \
-	if (1) { if (__should_lock) mutex_lock(&(__wg)->device_update_lock); else lockdep_assert_held(&(__wg)->device_update_lock); goto pfe_label(1); } else pfe_label(1): \
-	if (1) goto pfe_label(2); else while (1) if (1) { if (__should_lock) mutex_unlock(&(__wg)->device_update_lock); break; } else pfe_label(2): \
-	list_for_each_entry_safe (__peer, __temp, &(__wg)->peer_list, peer_list) \
-	if (0) pfe_label(3): break; else \
-	if (0); else for (__peer = peer_rcu_get(peer); __peer;) if (1) { goto pfe_label(4); pfe_label(5): break; } else while (1) if (1) goto pfe_label(5); else pfe_label(4): \
-	if (1) { goto pfe_label(6); pfe_label(7):; } else while (1) if (1) goto pfe_label(3); else while (1) if (1) goto pfe_label(7); else pfe_label(6): \
-	if (1) { goto pfe_label(8); pfe_label(9): peer_put(__peer); break; pfe_label(10): peer_put(__peer); } else while (1) if (1) goto pfe_label(9); else while (1) if (1) goto pfe_label(10); else pfe_label(8):
-
 #endif /* _WG_PEER_H */