Re: [PATCH net-next v3 14/24] ovpn: implement multi-peer support
From: Sabrina Dubroca <sd@queasysnail.net>
Date: 2024-05-28 14:44:36
Hi Antonio, I took a little break but I'm looking at your patches again now. 2024-05-06, 03:16:27 +0200, Antonio Quartulli wrote:
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ovpn/ovpnstruct.h b/drivers/net/ovpn/ovpnstruct.h index 7414c2459fb9..58166fdeac63 100644 --- a/drivers/net/ovpn/ovpnstruct.h +++ b/drivers/net/ovpn/ovpnstruct.h@@ -31,6 +35,12 @@ struct ovpn_struct { spinlock_t lock; /* protect writing to the ovpn_struct object */ struct workqueue_struct *crypto_wq; struct workqueue_struct *events_wq; + struct { + DECLARE_HASHTABLE(by_id, 12); + DECLARE_HASHTABLE(by_transp_addr, 12); + DECLARE_HASHTABLE(by_vpn_addr, 12);
Those are really big. I guess for large servers they make sense, but you're making clients hold 98kB in memory that they're not going to use. Maybe they could be dynamically sized, but I think struct peers should be allocated on demand (only for mode == MP) if you want this size.
quoted hunk ↗ jump to hunk
+ spinlock_t lock; /* protects writes to peers tables */ + } peers; struct ovpn_peer __rcu *peer; struct list_head dev_list; };diff --git a/drivers/net/ovpn/peer.c b/drivers/net/ovpn/peer.c index 99a2ae42a332..38a89595dade 100644 --- a/drivers/net/ovpn/peer.c +++ b/drivers/net/ovpn/peer.c@@ -361,6 +362,91 @@ struct ovpn_peer *ovpn_peer_get_by_src(struct ovpn_struct *ovpn, return peer; } +/** + * ovpn_peer_add_mp - add per to related tables in a MP instance
^
s/per/peer/
+ * @ovpn: the instance to add the peer to
+ * @peer: the peer to add
+ *
+ * Return: 0 on success or a negative error code otherwise
+ */
+static int ovpn_peer_add_mp(struct ovpn_struct *ovpn, struct ovpn_peer *peer)
+{[...]
+ index = ovpn_peer_index(ovpn->peers.by_id, &peer->id, sizeof(peer->id));
+ hlist_add_head_rcu(&peer->hash_entry_id, &ovpn->peers.by_id[index]);
+
+ if (peer->vpn_addrs.ipv4.s_addr != htonl(INADDR_ANY)) {
+ index = ovpn_peer_index(ovpn->peers.by_vpn_addr,
+ &peer->vpn_addrs.ipv4,
+ sizeof(peer->vpn_addrs.ipv4));
+ hlist_add_head_rcu(&peer->hash_entry_addr4,
+ &ovpn->peers.by_vpn_addr[index]);
+ }
+
+ hlist_del_init_rcu(&peer->hash_entry_addr6);Why are hash_entry_transp_addr and hash_entry_addr6 getting a hlist_del_init_rcu() call, but not hash_entry_id and hash_entry_addr4?
+ if (memcmp(&peer->vpn_addrs.ipv6, &in6addr_any,
+ sizeof(peer->vpn_addrs.ipv6))) {!ipv6_addr_any(&peer->vpn_addrs.ipv6)
+ index = ovpn_peer_index(ovpn->peers.by_vpn_addr, + &peer->vpn_addrs.ipv6, + sizeof(peer->vpn_addrs.ipv6)); + hlist_add_head_rcu(&peer->hash_entry_addr6, + &ovpn->peers.by_vpn_addr[index]); + } +
-- Sabrina