Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 10 Mar 2021 22:27:44 GMT
From:      "Alexander V. Chernikov" <melifaro@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: d9bcd8e7a2dd - stable/13 - Add ifa_try_ref() to simplify ifa handling inside epoch.
Message-ID:  <202103102227.12AMRimd032415@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by melifaro:

URL: https://cgit.FreeBSD.org/src/commit/?id=d9bcd8e7a2dd77969f8ac940fe8ec5e90588e3ea

commit d9bcd8e7a2dd77969f8ac940fe8ec5e90588e3ea
Author:     Alexander V. Chernikov <melifaro@FreeBSD.org>
AuthorDate: 2021-02-16 20:12:58 +0000
Commit:     Alexander V. Chernikov <melifaro@FreeBSD.org>
CommitDate: 2021-03-10 21:47:54 +0000

    Add ifa_try_ref() to simplify ifa handling inside epoch.
    
    More and more code migrates from lock-based protection to the NET_EPOCH
     umbrella. It requires some logic changes, including, notably, refcount
     handling.
    
    When we have an `ifa` pointer and we're running inside epoch we're
     guaranteed that this pointer will not be freed.
    However, the following case can still happen:
     * in thread 1 we drop to 0 refcount for ifa and schedule its deletion.
     * in thread 2 we use this ifa and reference it
     * destroy callout kicks in
     * unhappy user reports bug
    
    To address it, new `ifa_try_ref()` function is added, allowing to return
     failure when we try to reference `ifa` with 0 refcount.
    Additionally, existing `ifa_ref()` is enforced with `KASSERT` to provide
     cleaner error in such scenarious.
    
    Reviewed By: rstone, donner
    Differential Revision: https://reviews.freebsd.org/D28639
    
    (cherry picked from commit 600eade2fb4faacfcd408a01140ef15f85f0c817)
---
 sys/net/if.c     | 12 +++++++++++-
 sys/net/if_var.h |  1 +
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/sys/net/if.c b/sys/net/if.c
index c85cfab19bf6..948be6876b65 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -1857,8 +1857,18 @@ fail:
 void
 ifa_ref(struct ifaddr *ifa)
 {
+	u_int old;
 
-	refcount_acquire(&ifa->ifa_refcnt);
+	old = refcount_acquire(&ifa->ifa_refcnt);
+	KASSERT(old > 0, ("%s: ifa %p has 0 refs", __func__, ifa));
+}
+
+int
+ifa_try_ref(struct ifaddr *ifa)
+{
+
+	NET_EPOCH_ASSERT();
+	return (refcount_acquire_if_not_zero(&ifa->ifa_refcnt));
 }
 
 static void
diff --git a/sys/net/if_var.h b/sys/net/if_var.h
index beb9596895ee..bb364fd10974 100644
--- a/sys/net/if_var.h
+++ b/sys/net/if_var.h
@@ -575,6 +575,7 @@ struct ifaddr {
 struct ifaddr *	ifa_alloc(size_t size, int flags);
 void	ifa_free(struct ifaddr *ifa);
 void	ifa_ref(struct ifaddr *ifa);
+int	ifa_try_ref(struct ifaddr *ifa);
 
 /*
  * Multicast address structure.  This is analogous to the ifaddr



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202103102227.12AMRimd032415>