[PATCH net-next 0/2] ipv6: addrlabel: avoid dirtying ip6addrlbl_entry

[PATCH net-next 0/2] ipv6: addrlabel: avoid dirtying ip6addrlbl_entry

[Eric Dumazet]

The refcount on ip6addrlbl_entry is only used to make sure ip6addrlbl_entry
does not disappear while ip6addrlbl_get() is allocating an skb.

We can instead allocate skb first, then use RCU, so that we no longer need
to refcount these structures.

Eric Dumazet (2):
  ipv6: addrlabel: rework ip6addrlbl_get()
  ipv6: addrlabel: remove refcounting

 net/ipv6/addrlabel.c | 69 +++++++++++++---------------------------------------
 1 file changed, 17 insertions(+), 52 deletions(-)

-- 
2.14.2.920.gcf0c67979c-goog

[PATCH net-next 1/2] ipv6: addrlabel: rework ip6addrlbl_get()

[Eric Dumazet]

If we allocate skb before the lookup, we can use RCU
without the need of ip6addrlbl_hold()

This means that the following patch can get rid of refcounting.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv6/addrlabel.c | 36 +++++++++++++-----------------------
 1 file changed, 13 insertions(+), 23 deletions(-)

diff --git a/net/ipv6/addrlabel.c b/net/ipv6/addrlabel.c
index c6311d7108f651c7385cd6316752ba4a86667dcc..f31489652c4ae1fc0c93d1756e13717f0689b0ab 100644
--- a/net/ipv6/addrlabel.c
+++ b/net/ipv6/addrlabel.c
@@ -546,38 +546,28 @@ static int ip6addrlbl_get(struct sk_buff *in_skb, struct nlmsghdr *nlh,
 		return -EINVAL;
 	addr = nla_data(tb[IFAL_ADDRESS]);
 
-	rcu_read_lock();
-	p = __ipv6_addr_label(net, addr, ipv6_addr_type(addr), ifal->ifal_index);
-	if (p && !ip6addrlbl_hold(p))
-		p = NULL;
-	lseq = net->ipv6.ip6addrlbl_table.seq;
-	rcu_read_unlock();
-
-	if (!p) {
-		err = -ESRCH;
-		goto out;
-	}
-
 	skb = nlmsg_new(ip6addrlbl_msgsize(), GFP_KERNEL);
-	if (!skb) {
-		ip6addrlbl_put(p);
+	if (!skb)
 		return -ENOBUFS;
-	}
 
-	err = ip6addrlbl_fill(skb, p, lseq,
-			      NETLINK_CB(in_skb).portid, nlh->nlmsg_seq,
-			      RTM_NEWADDRLABEL, 0);
+	err = -ESRCH;
 
-	ip6addrlbl_put(p);
+	rcu_read_lock();
+	p = __ipv6_addr_label(net, addr, ipv6_addr_type(addr), ifal->ifal_index);
+	lseq = net->ipv6.ip6addrlbl_table.seq;
+	if (p)
+		err = ip6addrlbl_fill(skb, p, lseq,
+				      NETLINK_CB(in_skb).portid,
+				      nlh->nlmsg_seq,
+				      RTM_NEWADDRLABEL, 0);
+	rcu_read_unlock();
 
 	if (err < 0) {
 		WARN_ON(err == -EMSGSIZE);
 		kfree_skb(skb);
-		goto out;
+	} else {
+		err = rtnl_unicast(skb, net, NETLINK_CB(in_skb).portid);
 	}
-
-	err = rtnl_unicast(skb, net, NETLINK_CB(in_skb).portid);
-out:
 	return err;
 }
 
-- 
2.14.2.920.gcf0c67979c-goog

[PATCH net-next 2/2] ipv6: addrlabel: remove refcounting

[Eric Dumazet]

After previous patch ("ipv6: addrlabel: rework ip6addrlbl_get()")
we can remove the refcount from struct ip6addrlbl_entry,
since it is no longer elevated in p6addrlbl_get()

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv6/addrlabel.c | 33 ++++-----------------------------
 1 file changed, 4 insertions(+), 29 deletions(-)

diff --git a/net/ipv6/addrlabel.c b/net/ipv6/addrlabel.c
index f31489652c4ae1fc0c93d1756e13717f0689b0ab..2606d2fa3ac4c8e7d13071d1895cf4e6e36a4116 100644
--- a/net/ipv6/addrlabel.c
+++ b/net/ipv6/addrlabel.c
@@ -18,7 +18,6 @@
 #include <linux/if_addrlabel.h>
 #include <linux/netlink.h>
 #include <linux/rtnetlink.h>
-#include <linux/refcount.h>
 
 #if 0
 #define ADDRLABEL(x...) printk(x)
@@ -36,7 +35,6 @@ struct ip6addrlbl_entry {
 	int addrtype;
 	u32 label;
 	struct hlist_node list;
-	refcount_t refcnt;
 	struct rcu_head rcu;
 };
 
@@ -111,28 +109,6 @@ static const __net_initconst struct ip6addrlbl_init_table
 	}
 };
 
-/* Object management */
-static inline void ip6addrlbl_free(struct ip6addrlbl_entry *p)
-{
-	kfree(p);
-}
-
-static void ip6addrlbl_free_rcu(struct rcu_head *h)
-{
-	ip6addrlbl_free(container_of(h, struct ip6addrlbl_entry, rcu));
-}
-
-static bool ip6addrlbl_hold(struct ip6addrlbl_entry *p)
-{
-	return refcount_inc_not_zero(&p->refcnt);
-}
-
-static inline void ip6addrlbl_put(struct ip6addrlbl_entry *p)
-{
-	if (refcount_dec_and_test(&p->refcnt))
-		call_rcu(&p->rcu, ip6addrlbl_free_rcu);
-}
-
 /* Find label */
 static bool __ip6addrlbl_match(const struct ip6addrlbl_entry *p,
 			       const struct in6_addr *addr,
@@ -219,7 +195,6 @@ static struct ip6addrlbl_entry *ip6addrlbl_alloc(const struct in6_addr *prefix,
 	newp->addrtype = addrtype;
 	newp->label = label;
 	INIT_HLIST_NODE(&newp->list);
-	refcount_set(&newp->refcnt, 1);
 	return newp;
 }
 
@@ -243,7 +218,7 @@ static int __ip6addrlbl_add(struct net *net, struct ip6addrlbl_entry *newp,
 				goto out;
 			}
 			hlist_replace_rcu(&p->list, &newp->list);
-			ip6addrlbl_put(p);
+			kfree_rcu(p, rcu);
 			goto out;
 		} else if ((p->prefixlen == newp->prefixlen && !p->ifindex) ||
 			   (p->prefixlen < newp->prefixlen)) {
@@ -281,7 +256,7 @@ static int ip6addrlbl_add(struct net *net,
 	ret = __ip6addrlbl_add(net, newp, replace);
 	spin_unlock(&net->ipv6.ip6addrlbl_table.lock);
 	if (ret)
-		ip6addrlbl_free(newp);
+		kfree(newp);
 	return ret;
 }
 
@@ -302,7 +277,7 @@ static int __ip6addrlbl_del(struct net *net,
 		    p->ifindex == ifindex &&
 		    ipv6_addr_equal(&p->prefix, prefix)) {
 			hlist_del_rcu(&p->list);
-			ip6addrlbl_put(p);
+			kfree_rcu(p, rcu);
 			ret = 0;
 			break;
 		}
@@ -360,7 +335,7 @@ static void __net_exit ip6addrlbl_net_exit(struct net *net)
 	spin_lock(&net->ipv6.ip6addrlbl_table.lock);
 	hlist_for_each_entry_safe(p, n, &net->ipv6.ip6addrlbl_table.head, list) {
 		hlist_del_rcu(&p->list);
-		ip6addrlbl_put(p);
+		kfree_rcu(p, rcu);
 	}
 	spin_unlock(&net->ipv6.ip6addrlbl_table.lock);
 }
-- 
2.14.2.920.gcf0c67979c-goog

Re: [PATCH net-next 0/2] ipv6: addrlabel: avoid dirtying ip6addrlbl_entry

[Martin KaFai Lau]

On Mon, Oct 09, 2017 at 04:52:23PM +0000, Eric Dumazet wrote:
> The refcount on ip6addrlbl_entry is only used to make sure ip6addrlbl_entry
> does not disappear while ip6addrlbl_get() is allocating an skb.
> 
> We can instead allocate skb first, then use RCU, so that we no longer need
> to refcount these structures.
Acked-by: Martin KaFai Lau <kafai@fb.com>

Re: [PATCH net-next 0/2] ipv6: addrlabel: avoid dirtying ip6addrlbl_entry

[David Miller]

From: Martin KaFai Lau <kafai@fb.com>
Date: Mon, 9 Oct 2017 10:44:17 -0700

> On Mon, Oct 09, 2017 at 04:52:23PM +0000, Eric Dumazet wrote:
>> The refcount on ip6addrlbl_entry is only used to make sure ip6addrlbl_entry
>> does not disappear while ip6addrlbl_get() is allocating an skb.
>> 
>> We can instead allocate skb first, then use RCU, so that we no longer need
>> to refcount these structures.
> Acked-by: Martin KaFai Lau <kafai@fb.com>

Series applied, thanks everyone.