net/ipv6: Add anycast addresses to a global hashtable

icmp6_send() function is expensive on systems with a large number of interfaces. Every time it’s called, it has to verify that the source address does not correspond to an existing anycast address by looping through every device and every anycast address on the device. This can result in significant delays for a CPU when there are a large number of neighbors and ND timers are frequently timing out and calling neigh_invalidate().

Add anycast addresses to a global hashtable to allow quick searching for matching anycast addresses. This is based on inet6_addr_lst in addrconf.c.

Quoted 11 lines · Jeff Barnhill

icmp6_send() function is expensive on systems with a large number of interfaces. Every time it’s called, it has to verify that the source address does not correspond to an existing anycast address by looping through every device and every anycast address on the device. This can result in significant delays for a CPU when there are a large number of neighbors and ND timers are frequently timing out and calling neigh_invalidate().

Add anycast addresses to a global hashtable to allow quick searching for matching anycast addresses. This is based on inet6_addr_lst in addrconf.c.

I do not see this patch being netns aware ?

Also I believe you misunderstood what was stored in net->ifindex You can look at dev_new_index() for what I mean.

icmp6_send() function is expensive on systems with a large number of interfaces. Every time it’s called, it has to verify that the source address does not correspond to an existing anycast address by looping through every device and every anycast address on the device. This can result in significant delays for a CPU when there are a large number of neighbors and ND timers are frequently timing out and calling neigh_invalidate().

Add anycast addresses to a global hashtable to allow quick searching for matching anycast addresses. This is based on inet6_addr_lst in addrconf.c.

Signed-off-by Jeff Barnhill <[email protected]>

---
 include/net/addrconf.h |   2 +
 include/net/if_inet6.h |   8 +++
 net/ipv6/af_inet6.c    |   5 ++
 net/ipv6/anycast.c     | 132 ++++++++++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 145 insertions(+), 2 deletions(-)

include/net/addrconf.h +2 -0

index 6def0351bcc3..0cee3f99c41d 100644
@@ -312,6 +312,8 @@ bool ipv6_chk_acast_addr(struct net *net, struct net_device *dev,
 			 const struct in6_addr *addr);
 bool ipv6_chk_acast_addr_src(struct net *net, struct net_device *dev,
 			     const struct in6_addr *addr);
+int anycast_init(void);
+void anycast_cleanup(void);
 
 /* Device notifier */
 int register_inet6addr_notifier(struct notifier_block *nb);

include/net/if_inet6.h +0 -0

index d7578cf49c3a..55a4a1d8cebc 100644

include/net/if_inet6.h +4 -0

+	struct hlist_node	acal_lst; /* inet6_acaddr_lst */
+	struct rcu_head		rcu;
+};
+
 struct ifacaddr6 {
 	struct in6_addr		aca_addr;
 	struct fib6_info	*aca_rt;

net/ipv6/af_inet6.c +5 -0

index 9a4261e50272..971a05fdd3bd 100644
@@ -1001,6 +1001,9 @@ static int __init inet6_init(void)
 	err = ip6_flowlabel_init();
 	if (err)
 		goto ip6_flowlabel_fail;
+	err = anycast_init();
+	if (err)
+		goto anycast_fail;
 	err = addrconf_init();
 	if (err)
 		goto addrconf_fail;

@@ -1091,6 +1094,8 @@ static int __init inet6_init(void)
 ipv6_exthdrs_fail:
 	addrconf_cleanup();
 addrconf_fail:
+	anycast_cleanup();
+anycast_fail:
 	ip6_flowlabel_cleanup();
 ip6_flowlabel_fail:
 	ndisc_late_cleanup();

net/ipv6/anycast.c +15 -0

index 4e0ff7031edd..def1e156d857 100644
@@ -44,8 +44,22 @@
 
 #include <net/checksum.h>
 
+#define IN6_ADDR_HSIZE_SHIFT	8
+#define IN6_ADDR_HSIZE		BIT(IN6_ADDR_HSIZE_SHIFT)
+/*	anycast address hash table
+ */
+static struct hlist_head inet6_acaddr_lst[IN6_ADDR_HSIZE];
+static DEFINE_SPINLOCK(acaddr_hash_lock);
+
 static int ipv6_dev_ac_dec(struct net_device *dev, const struct in6_addr *addr);
 
+static u32 inet6_acaddr_hash(struct net *net, const struct in6_addr *addr)
+{
+	u32 val = ipv6_addr_hash(addr) ^ net_hash_mix(net);
+
+	return hash_32(val, IN6_ADDR_HSIZE_SHIFT);
+}
+
 /*
  *	socket join an anycast group
  */

@@ -204,6 +218,83 @@ void ipv6_sock_ac_close(struct sock *sk)
 	rtnl_unlock();
 }
 
+static struct ipv6_ac_addrlist *acal_alloc(struct net *net,

net/ipv6/anycast.c +52 -0

+}
+
+static int ipv6_add_acaddr_hash(struct net *net, const struct in6_addr *addr)
+{
+	unsigned int hash = inet6_acaddr_hash(net, addr);
+	struct ipv6_ac_addrlist *acal;
+	int err = 0;
+
+	spin_lock(&acaddr_hash_lock);
+	hlist_for_each_entry(acal, &inet6_acaddr_lst[hash], acal_lst) {
+		if (!net_eq(read_pnet(&acal->acal_pnet), net))
+			continue;
+		if (ipv6_addr_equal(&acal->acal_addr, addr)) {
+			acal->acal_users++;
+			goto out;
+		}
+	}
+
+	acal = acal_alloc(net, addr);
+	if (!acal) {
+		err = -ENOMEM;
+		goto out;
+	}
+
+	hlist_add_head_rcu(&acal->acal_lst, &inet6_acaddr_lst[hash]);
+
+out:
+	spin_unlock(&acaddr_hash_lock);
+	return err;
+}
+
+static void ipv6_del_acaddr_hash(struct net *net, const struct in6_addr *addr)
+{
+	unsigned int hash = inet6_acaddr_hash(net, addr);
+	struct ipv6_ac_addrlist *acal;
+
+	spin_lock(&acaddr_hash_lock);
+	hlist_for_each_entry(acal, &inet6_acaddr_lst[hash], acal_lst) {
+		if (!net_eq(read_pnet(&acal->acal_pnet), net))
+			continue;
+		if (ipv6_addr_equal(&acal->acal_addr, addr)) {
+			if (--acal->acal_users < 1) {
+				hlist_del_init_rcu(&acal->acal_lst);
+				call_rcu(&acal->rcu, acal_free_rcu);
+			}
+			spin_unlock(&acaddr_hash_lock);
+			return;
+		}
+	}
+	spin_unlock(&acaddr_hash_lock);
+}
+
 static void aca_get(struct ifacaddr6 *aca)
 {
 	refcount_inc(&aca->aca_refcnt);

net/ipv6/anycast.c +6 -0

+		kfree(aca);
+		goto out;
+	}
 
 	aca->aca_next = idev->ac_list;
 	idev->ac_list = aca;

@@ -324,6 +422,7 @@ int __ipv6_dev_ac_dec(struct inet6_dev *idev, const struct in6_addr *addr)
 		prev_aca->aca_next = aca->aca_next;
 	else
 		idev->ac_list = aca->aca_next;
+	ipv6_del_acaddr_hash(dev_net(idev->dev), &aca->aca_addr);
 	write_unlock_bh(&idev->lock);
 	addrconf_leave_solict(idev, &aca->aca_addr);
 

@@ -350,6 +449,8 @@ void ipv6_ac_destroy_dev(struct inet6_dev *idev)
 	write_lock_bh(&idev->lock);
 	while ((aca = idev->ac_list) != NULL) {
 		idev->ac_list = aca->aca_next;
+		ipv6_del_acaddr_hash(dev_net(idev->dev), &aca->aca_addr);
+
 		write_unlock_bh(&idev->lock);
 
 		addrconf_leave_solict(idev, &aca->aca_addr);

net/ipv6/anycast.c +27 -2

 	rcu_read_lock();
 	if (dev)
 		found = ipv6_chk_acast_dev(dev, addr);
 	else
-		for_each_netdev_rcu(net, dev)
-			if (ipv6_chk_acast_dev(dev, addr)) {
+		hlist_for_each_entry_rcu(acal, &inet6_acaddr_lst[hash],
+					 acal_lst) {
+			if (!net_eq(read_pnet(&acal->acal_pnet), net))
+				continue;
+			if (ipv6_addr_equal(&acal->acal_addr, addr)) {
 				found = true;
 				break;
 			}
+		}
 	rcu_read_unlock();
 	return found;
 }

@@ -539,4 +646,25 @@ void ac6_proc_exit(struct net *net)
 {
 	remove_proc_entry("anycast6", net->proc_net);
 }
+
+/*	Init / cleanup code
+ */
+int __init anycast_init(void)
+{
+	int i;
+
+	for (i = 0; i < IN6_ADDR_HSIZE; i++)
+		INIT_HLIST_HEAD(&inet6_acaddr_lst[i]);
+	return 0;
+}
+
+void anycast_cleanup(void)
+{
+	int i;
+
+	spin_lock(&acaddr_hash_lock);
+	for (i = 0; i < IN6_ADDR_HSIZE; i++)
+		WARN_ON(!hlist_empty(&inet6_acaddr_lst[i]));
+	spin_unlock(&acaddr_hash_lock);
+}
 #endif

Quoted 37 lines · Jeff Barnhill

icmp6_send() function is expensive on systems with a large number of interfaces. Every time it’s called, it has to verify that the source address does not correspond to an existing anycast address by looping through every device and every anycast address on the device. This can result in significant delays for a CPU when there are a large number of neighbors and ND timers are frequently timing out and calling neigh_invalidate().

Add anycast addresses to a global hashtable to allow quick searching for matching anycast addresses. This is based on inet6_addr_lst in addrconf.c.

Signed-off-by Jeff Barnhill <[email protected]>

---
 include/net/addrconf.h |   2 +
 include/net/if_inet6.h |   8 +++
 net/ipv6/af_inet6.c    |   5 ++
 net/ipv6/anycast.c     | 132 ++++++++++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 145 insertions(+), 2 deletions(-)

include/net/addrconf.h +2 -0

index 6def0351bcc3..0cee3f99c41d 100644
@@ -312,6 +312,8 @@ bool ipv6_chk_acast_addr(struct net *net, struct net_device *dev,
                       const struct in6_addr *addr);
 bool ipv6_chk_acast_addr_src(struct net *net, struct net_device *dev,
                           const struct in6_addr *addr);
+int anycast_init(void);
+void anycast_cleanup(void);
 
 /* Device notifier */
 int register_inet6addr_notifier(struct notifier_block *nb);

include/net/if_inet6.h +0 -0

index d7578cf49c3a..55a4a1d8cebc 100644

include/net/if_inet6.h struct ipv6_ac_socklist { · line 148

Quoted 7 lines

include/net/if_inet6.h +4 -0

@@ -142,6 +142,14 @@ struct ipv6_ac_socklist {
      struct ipv6_ac_socklist *acl_next;
 };
 
+struct ipv6_ac_addrlist {
+     struct in6_addr         acal_addr;
+     possible_net_t          acal_pnet;
+     int                     acal_users;

That would be a refcount_t acal_users; so that CONFIG_REFCOUNT_FULL brings debugging for free.

Quoted 62 lines

include/net/if_inet6.h +4 -0

+     struct hlist_node       acal_lst; /* inet6_acaddr_lst */
+     struct rcu_head         rcu;
+};
+
 struct ifacaddr6 {
      struct in6_addr         aca_addr;
      struct fib6_info        *aca_rt;

net/ipv6/af_inet6.c +5 -0

index 9a4261e50272..971a05fdd3bd 100644
@@ -1001,6 +1001,9 @@ static int __init inet6_init(void)
      err = ip6_flowlabel_init();
      if (err)
              goto ip6_flowlabel_fail;
+     err = anycast_init();
+     if (err)
+             goto anycast_fail;
      err = addrconf_init();
      if (err)
              goto addrconf_fail;

@@ -1091,6 +1094,8 @@ static int __init inet6_init(void)
 ipv6_exthdrs_fail:
      addrconf_cleanup();
 addrconf_fail:
+     anycast_cleanup();
+anycast_fail:
      ip6_flowlabel_cleanup();
 ip6_flowlabel_fail:
      ndisc_late_cleanup();

net/ipv6/anycast.c +15 -0

index 4e0ff7031edd..def1e156d857 100644
@@ -44,8 +44,22 @@
 
 #include <net/checksum.h>
 
+#define IN6_ADDR_HSIZE_SHIFT 8
+#define IN6_ADDR_HSIZE               BIT(IN6_ADDR_HSIZE_SHIFT)
+/*   anycast address hash table
+ */
+static struct hlist_head inet6_acaddr_lst[IN6_ADDR_HSIZE];
+static DEFINE_SPINLOCK(acaddr_hash_lock);
+
 static int ipv6_dev_ac_dec(struct net_device *dev, const struct in6_addr *addr);
 
+static u32 inet6_acaddr_hash(struct net *net, const struct in6_addr *addr)
+{
+     u32 val = ipv6_addr_hash(addr) ^ net_hash_mix(net);
+
+     return hash_32(val, IN6_ADDR_HSIZE_SHIFT);
+}
+
 /*
  *   socket join an anycast group
  */

@@ -204,6 +218,83 @@ void ipv6_sock_ac_close(struct sock *sk)
      rtnl_unlock();
 }
 
+static struct ipv6_ac_addrlist *acal_alloc(struct net *net,

net/ipv6/anycast.c void ipv6_sock_ac_close(struct sock *sk) · line 231

Quoted 10 lines

net/ipv6/anycast.c +10 -0

+                                        const struct in6_addr *addr)
+{
+     struct ipv6_ac_addrlist *acal;
+
+     acal = kzalloc(sizeof(*acal), GFP_ATOMIC);
+     if (!acal)
+             return NULL;
+
+     acal->acal_addr = *addr;
+     write_pnet(&acal->acal_pnet, get_net(net));

I am not sure why you grab a reference on the netns.

The ipv6 address will be freed at some point before the netns disappears. It would automatically remove the associated struct ipv6_ac_addrlist.

net/ipv6/anycast.c void ipv6_sock_ac_close(struct sock *sk) · line 243

Quoted 12 lines

net/ipv6/anycast.c +12 -0

+     acal->acal_users = 1;
+     INIT_HLIST_NODE(&acal->acal_lst);
+
+     return acal;
+}
+
+static void acal_free_rcu(struct rcu_head *h)
+{
+     struct ipv6_ac_addrlist *acal;
+
+     acal = container_of(h, struct ipv6_ac_addrlist, rcu);
+     WARN_ON(acal->acal_users);

Not needed with refcount_t debugging infra.

net/ipv6/anycast.c void ipv6_sock_ac_close(struct sock *sk) · line 245

Quoted 2 lines

net/ipv6/anycast.c +2 -0

+     put_net(read_pnet(&acal->acal_pnet));
+     kfree(acal);

So this could use kfree_rcu() in the caller, and get rid of acal_free_rcu() completely.

Quoted 56 lines

net/ipv6/anycast.c +52 -0

+}
+
+static int ipv6_add_acaddr_hash(struct net *net, const struct in6_addr *addr)
+{
+     unsigned int hash = inet6_acaddr_hash(net, addr);
+     struct ipv6_ac_addrlist *acal;
+     int err = 0;
+
+     spin_lock(&acaddr_hash_lock);
+     hlist_for_each_entry(acal, &inet6_acaddr_lst[hash], acal_lst) {
+             if (!net_eq(read_pnet(&acal->acal_pnet), net))
+                     continue;
+             if (ipv6_addr_equal(&acal->acal_addr, addr)) {
+                     acal->acal_users++;
+                     goto out;
+             }
+     }
+
+     acal = acal_alloc(net, addr);
+     if (!acal) {
+             err = -ENOMEM;
+             goto out;
+     }
+
+     hlist_add_head_rcu(&acal->acal_lst, &inet6_acaddr_lst[hash]);
+
+out:
+     spin_unlock(&acaddr_hash_lock);
+     return err;
+}
+
+static void ipv6_del_acaddr_hash(struct net *net, const struct in6_addr *addr)
+{
+     unsigned int hash = inet6_acaddr_hash(net, addr);
+     struct ipv6_ac_addrlist *acal;
+
+     spin_lock(&acaddr_hash_lock);
+     hlist_for_each_entry(acal, &inet6_acaddr_lst[hash], acal_lst) {
+             if (!net_eq(read_pnet(&acal->acal_pnet), net))
+                     continue;
+             if (ipv6_addr_equal(&acal->acal_addr, addr)) {
+                     if (--acal->acal_users < 1) {
+                             hlist_del_init_rcu(&acal->acal_lst);
+                             call_rcu(&acal->rcu, acal_free_rcu);
+                     }
+                     spin_unlock(&acaddr_hash_lock);
+                     return;
+             }
+     }
+     spin_unlock(&acaddr_hash_lock);
+}
+
 static void aca_get(struct ifacaddr6 *aca)
 {
      refcount_inc(&aca->aca_refcnt);

net/ipv6/anycast.c int __ipv6_dev_ac_inc(struct inet6_dev *idev, const struct in6_addr *addr) · line 372

Quoted 7 lines

net/ipv6/anycast.c +4 -0

@@ -275,6 +366,13 @@ int __ipv6_dev_ac_inc(struct inet6_dev *idev, const struct in6_addr *addr)
              err = -ENOMEM;
              goto out;
      }
+     err = ipv6_add_acaddr_hash(dev_net(idev->dev), addr);
+     if (err) {
+             fib6_info_release(f6i);
+             fib6_info_release(f6i);

Double call to fib6_info_release() ? Why ?

Quoted 24 lines

net/ipv6/anycast.c +6 -0

+             kfree(aca);
+             goto out;
+     }
 
      aca->aca_next = idev->ac_list;
      idev->ac_list = aca;

@@ -324,6 +422,7 @@ int __ipv6_dev_ac_dec(struct inet6_dev *idev, const struct in6_addr *addr)
              prev_aca->aca_next = aca->aca_next;
      else
              idev->ac_list = aca->aca_next;
+     ipv6_del_acaddr_hash(dev_net(idev->dev), &aca->aca_addr);
      write_unlock_bh(&idev->lock);
      addrconf_leave_solict(idev, &aca->aca_addr);
 

@@ -350,6 +449,8 @@ void ipv6_ac_destroy_dev(struct inet6_dev *idev)
      write_lock_bh(&idev->lock);
      while ((aca = idev->ac_list) != NULL) {
              idev->ac_list = aca->aca_next;
+             ipv6_del_acaddr_hash(dev_net(idev->dev), &aca->aca_addr);
+
              write_unlock_bh(&idev->lock);
 
              addrconf_leave_solict(idev, &aca->aca_addr);

net/ipv6/anycast.c bool ipv6_chk_acast_addr(struct net *net, struct net_device *dev, · line 496

Quoted 5 lines

net/ipv6/anycast.c +2 -0

@@ -391,16 +492,22 @@ bool ipv6_chk_acast_addr(struct net *net, struct net_device *dev,
                       const struct in6_addr *addr)
 {
      bool found = false;
+     unsigned int hash = inet6_acaddr_hash(net, addr);
+     struct ipv6_ac_addrlist *acal;

Reorder variable declaration in longest to shortest (reverse xmas tree), per David Miller request :)

Quoted 46 lines

net/ipv6/anycast.c +27 -2

      rcu_read_lock();
      if (dev)
              found = ipv6_chk_acast_dev(dev, addr);
      else
-             for_each_netdev_rcu(net, dev)
-                     if (ipv6_chk_acast_dev(dev, addr)) {
+             hlist_for_each_entry_rcu(acal, &inet6_acaddr_lst[hash],
+                                      acal_lst) {
+                     if (!net_eq(read_pnet(&acal->acal_pnet), net))
+                             continue;
+                     if (ipv6_addr_equal(&acal->acal_addr, addr)) {
                              found = true;
                              break;
                      }
+             }
      rcu_read_unlock();
      return found;
 }

@@ -539,4 +646,25 @@ void ac6_proc_exit(struct net *net)
 {
      remove_proc_entry("anycast6", net->proc_net);
 }
+
+/*   Init / cleanup code
+ */
+int __init anycast_init(void)
+{
+     int i;
+
+     for (i = 0; i < IN6_ADDR_HSIZE; i++)
+             INIT_HLIST_HEAD(&inet6_acaddr_lst[i]);
+     return 0;
+}
+
+void anycast_cleanup(void)
+{
+     int i;
+
+     spin_lock(&acaddr_hash_lock);
+     for (i = 0; i < IN6_ADDR_HSIZE; i++)
+             WARN_ON(!hlist_empty(&inet6_acaddr_lst[i]));
+     spin_unlock(&acaddr_hash_lock);
+}
 #endif

net/ipv6/anycast.c int __ipv6_dev_ac_inc(struct inet6_dev *idev, const struct in6_addr *addr) · line 366

Quoted 12 lines · Jeff Barnhill

net/ipv6/anycast.c +5 -0

@@ -275,6 +356,11 @@ int __ipv6_dev_ac_inc(struct inet6_dev *idev, const struct in6_addr *addr)
              err = -ENOMEM;
              goto out;
      }
+     err = ipv6_add_acaddr_hash(dev_net(idev->dev), addr);
+     if (err) {
+             aca_put(aca);
+             goto out;
+     }
 
      aca->aca_next = idev->ac_list;
      idev->ac_list = aca;

you misunderstood my comment. aca_put is instead of a double call to fib6_info_release(f6i). You still need one call to fib6_info_release(f6i) for the addrconf_f6i_alloc.

From: Jeff Barnhill <[email protected]>
Date: Sun, 28 Oct 2018 01:51:59 +0000

include/net/if_inet6.h struct ipv6_ac_socklist { · line 151

Quoted 7 lines

include/net/if_inet6.h +7 -0

+struct ipv6_ac_addrlist {
+     struct in6_addr         acal_addr;
+     possible_net_t          acal_pnet;
+     refcount_t              acal_users;
+     struct hlist_node       acal_lst; /* inet6_acaddr_lst */
+     struct rcu_head         rcu;
+};

Please just add the hlist to ifcaddr6 instead of duplicating so much information and reference counters here.

This seems to waste a lot of memory unnecessary and add lots of unnecessary object allocate/setup/destroy logic.

I originally started implementing it the way you suggested; however, it seemed to complicate management of that structure because it isn't currently using rcu. Also, assuming that can be worked out, where would I get the net from? Would I need to store a copy in ifcaddr6, or is there some way to access it during ipv6_chk_acast_addr()? It seems that if I don't add a copy of net, but instead access it through aca_rt(?), then freeing the ifcaddr6 memory becomes problematic (detaching it from idev, while read_rcu may still be accessing it). On Mon, Oct 29, 2018 at 11:32 PM David Miller <[email protected]> wrote:

From: Jeff Barnhill <[email protected]>
Date: Tue, 30 Oct 2018 07:10:58 -0400

Quoted 9 lines

I originally started implementing it the way you suggested; however, it seemed to complicate management of that structure because it isn't currently using rcu. Also, assuming that can be worked out, where would I get the net from? Would I need to store a copy in ifcaddr6, or is there some way to access it during ipv6_chk_acast_addr()? It seems that if I don't add a copy of net, but instead access it through aca_rt(?), then freeing the ifcaddr6 memory becomes problematic (detaching it from idev, while read_rcu may still be accessing it). On Mon, Oct 29, 2018 at 11:32 PM David Miller <[email protected]> wrote:

I don't think converting the structure over to RCU, especially because all of the read paths (everything leading to ipv6_chk_acast_dev()) are taking RCU locks already.

And I cannot understand how having _two_ structures to manage a piece of information can be less complicated than just one.

You can add a backpointer to the 'idev' in ifacaddr6 to get at the network namespace. You don't even need to do additional reference counting because the idev->ac_list is always purged before an idev is destroyed.

or make the table per namespace.

From: David Ahern <[email protected]>
Date: Tue, 30 Oct 2018 16:06:46 -0600

Quoted 1 line

or make the table per namespace.

This will increase namespace create/destroy cost, so I'd rather not for something like this.

icmp6_send() function is expensive on systems with a large number of interfaces. Every time it’s called, it has to verify that the source address does not correspond to an existing anycast address by looping through every device and every anycast address on the device. This can result in significant delays for a CPU when there are a large number of neighbors and ND timers are frequently timing out and calling neigh_invalidate().

Add anycast addresses to a global hashtable to allow quick searching for matching anycast addresses. This is based on inet6_addr_lst in addrconf.c.

Signed-off-by Jeff Barnhill <[email protected]>

I'll follow this email with a new patch using ifacaddr6 instead of creating a new struct. I ended up using fib6_nh.nh_dev to get the net, instead of adding a back pointer to idev. It seems that idev was recently removed in lieu of this, so if this is incorrect, please let me know. Hopefully, I got the locking correct.