Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 7 Sep 2021 21:12:48 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: 4151d8ccdc64 - stable/13 - [lltable] Restructure nd6 code.
Message-ID:  <202109072112.187LCmOv023930@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=4151d8ccdc647fb0cc2cee35eae24dd873812348

commit 4151d8ccdc647fb0cc2cee35eae24dd873812348
Author:     Alexander V. Chernikov <melifaro@FreeBSD.org>
AuthorDate: 2021-08-06 08:27:22 +0000
Commit:     Alexander V. Chernikov <melifaro@FreeBSD.org>
CommitDate: 2021-09-07 21:02:58 +0000

    [lltable] Restructure nd6 code.
    
    Factor out lltable locking logic from lltable_try_set_entry_addr()
     into a separate lltable_acquire_wlock(), so the latter can be used
     in other parts of the code w/o duplication.
    
    Create nd6_try_set_entry_addr() to avoid code duplication in nd6.c
     and nd6_nbr.c.
    
    Move lle creation logic from nd6_resolve_slow() into a separate
     nd6_get_llentry() to simplify the former.
    
    These changes serve as a pre-requisite for implementing
     RFC8950 (IPv4 prefixes with IPv6 nexthops).
    
    Differential Revision: https://reviews.freebsd.org/D31432
    
    (cherry picked from commit 0b79b007ebfc250a8a7b928df268ada6f1c988c4)
---
 sys/net/if_llatbl.c    |  40 +++++++++++------
 sys/net/if_llatbl.h    |   2 +
 sys/netinet6/nd6.c     | 114 ++++++++++++++++++++++++++++++++-----------------
 sys/netinet6/nd6.h     |   1 +
 sys/netinet6/nd6_nbr.c |  11 +----
 5 files changed, 105 insertions(+), 63 deletions(-)

diff --git a/sys/net/if_llatbl.c b/sys/net/if_llatbl.c
index 70baf58c2778..c656974c80ee 100644
--- a/sys/net/if_llatbl.c
+++ b/sys/net/if_llatbl.c
@@ -318,22 +318,18 @@ lltable_set_entry_addr(struct ifnet *ifp, struct llentry *lle,
 }
 
 /*
- * Tries to update @lle link-level address.
- * Since update requires AFDATA WLOCK, function
- * drops @lle lock, acquires AFDATA lock and then acquires
- * @lle lock to maintain lock order.
+ * Acquires lltable write lock.
  *
- * Returns 1 on success.
+ * Returns true on success, with both lltable and lle lock held.
+ * On failure, false is returned and lle wlock is still held.
  */
-int
-lltable_try_set_entry_addr(struct ifnet *ifp, struct llentry *lle,
-    const char *linkhdr, size_t linkhdrsize, int lladdr_off)
+bool
+lltable_acquire_wlock(struct ifnet *ifp, struct llentry *lle)
 {
+	NET_EPOCH_ASSERT();
 
 	/* Perform real LLE update */
 	/* use afdata WLOCK to update fields */
-	LLE_WLOCK_ASSERT(lle);
-	LLE_ADDREF(lle);
 	LLE_WUNLOCK(lle);
 	IF_AFDATA_WLOCK(ifp);
 	LLE_WLOCK(lle);
@@ -344,17 +340,33 @@ lltable_try_set_entry_addr(struct ifnet *ifp, struct llentry *lle,
 	 */
 	if ((lle->la_flags & LLE_DELETED) != 0) {
 		IF_AFDATA_WUNLOCK(ifp);
-		LLE_FREE_LOCKED(lle);
-		return (0);
+		return (false);
 	}
 
+	return (true);
+}
+
+/*
+ * Tries to update @lle link-level address.
+ * Since update requires AFDATA WLOCK, function
+ * drops @lle lock, acquires AFDATA lock and then acquires
+ * @lle lock to maintain lock order.
+ *
+ * Returns 1 on success.
+ */
+int
+lltable_try_set_entry_addr(struct ifnet *ifp, struct llentry *lle,
+    const char *linkhdr, size_t linkhdrsize, int lladdr_off)
+{
+
+	if (!lltable_acquire_wlock(ifp, lle))
+		return (0);
+
 	/* Update data */
 	lltable_set_entry_addr(ifp, lle, linkhdr, linkhdrsize, lladdr_off);
 
 	IF_AFDATA_WUNLOCK(ifp);
 
-	LLE_REMREF(lle);
-
 	return (1);
 }
 
diff --git a/sys/net/if_llatbl.h b/sys/net/if_llatbl.h
index 488f8b006315..ffbaa7a946bb 100644
--- a/sys/net/if_llatbl.h
+++ b/sys/net/if_llatbl.h
@@ -238,6 +238,8 @@ void lltable_fill_sa_entry(const struct llentry *lle, struct sockaddr *sa);
 struct ifnet *lltable_get_ifp(const struct lltable *llt);
 int lltable_get_af(const struct lltable *llt);
 
+bool lltable_acquire_wlock(struct ifnet *ifp, struct llentry *lle);
+
 int lltable_foreach_lle(struct lltable *llt, llt_foreach_cb_t *f,
     void *farg);
 /*
diff --git a/sys/netinet6/nd6.c b/sys/netinet6/nd6.c
index 143629d44fdb..ea64b3a6c14c 100644
--- a/sys/netinet6/nd6.c
+++ b/sys/netinet6/nd6.c
@@ -1382,6 +1382,35 @@ nd6_is_addr_neighbor(const struct sockaddr_in6 *addr, struct ifnet *ifp)
 	return (rc);
 }
 
+/*
+ * Tries to update @lle address/prepend data with new @lladdr.
+ *
+ * Returns true on success.
+ * In any case, @lle is returned wlocked.
+ */
+bool
+nd6_try_set_entry_addr(struct ifnet *ifp, struct llentry *lle, char *lladdr)
+{
+	u_char linkhdr[LLE_MAX_LINKHDR];
+	size_t linkhdrsize;
+	int lladdr_off;
+
+	LLE_WLOCK_ASSERT(lle);
+
+	linkhdrsize = sizeof(linkhdr);
+	if (lltable_calc_llheader(ifp, AF_INET6, lladdr,
+	    linkhdr, &linkhdrsize, &lladdr_off) != 0) {
+		return (false);
+	}
+
+	if (!lltable_acquire_wlock(ifp, lle))
+		return (false);
+	lltable_set_entry_addr(ifp, lle, linkhdr, linkhdrsize, lladdr_off);
+	IF_AFDATA_WUNLOCK(ifp);
+
+	return (true);
+}
+
 /*
  * Free an nd6 llinfo entry.
  * Since the function would cause significant changes in the kernel, DO NOT
@@ -2027,14 +2056,9 @@ nd6_cache_lladdr(struct ifnet *ifp, struct in6_addr *from, char *lladdr,
 		 * Record source link-layer address
 		 * XXX is it dependent to ifp->if_type?
 		 */
-		linkhdrsize = sizeof(linkhdr);
-		if (lltable_calc_llheader(ifp, AF_INET6, lladdr,
-		    linkhdr, &linkhdrsize, &lladdr_off) != 0)
-			return;
-
-		if (lltable_try_set_entry_addr(ifp, ln, linkhdr, linkhdrsize,
-		    lladdr_off) == 0) {
+		if (!nd6_try_set_entry_addr(ifp, ln, lladdr)) {
 			/* Entry was deleted */
+			LLE_WUNLOCK(ln);
 			return;
 		}
 
@@ -2257,6 +2281,39 @@ nd6_resolve(struct ifnet *ifp, int is_gw, struct mbuf *m,
 	return (nd6_resolve_slow(ifp, 0, m, dst6, desten, pflags, plle));
 }
 
+/*
+ * Finds or creates a new llentry for @addr.
+ * Returns wlocked llentry or NULL.
+ */
+static __noinline struct llentry *
+nd6_get_llentry(struct ifnet *ifp, const struct in6_addr *addr)
+{
+	struct llentry *lle, *lle_tmp;
+
+	lle = nd6_alloc(addr, 0, ifp);
+	if (lle == NULL) {
+		char ip6buf[INET6_ADDRSTRLEN];
+		log(LOG_DEBUG,
+		    "nd6_get_llentry: can't allocate llinfo for %s "
+		    "(ln=%p)\n",
+		    ip6_sprintf(ip6buf, addr), lle);
+		return (NULL);
+	}
+
+	IF_AFDATA_WLOCK(ifp);
+	LLE_WLOCK(lle);
+	/* Prefer any existing entry over newly-created one */
+	lle_tmp = nd6_lookup(addr, LLE_EXCLUSIVE, ifp);
+	if (lle_tmp == NULL)
+		lltable_link_entry(LLTABLE6(ifp), lle);
+	IF_AFDATA_WUNLOCK(ifp);
+	if (lle_tmp != NULL) {
+		lltable_free_entry(LLTABLE6(ifp), lle);
+		return (lle_tmp);
+	} else
+		return (lle);
+}
+
 /*
  * Do L2 address resolution for @sa_dst address. Stores found
  * address in @desten buffer. Copy of lle ln_flags can be also
@@ -2273,7 +2330,7 @@ nd6_resolve_slow(struct ifnet *ifp, int flags, struct mbuf *m,
     const struct sockaddr_in6 *dst, u_char *desten, uint32_t *pflags,
     struct llentry **plle)
 {
-	struct llentry *lle = NULL, *lle_tmp;
+	struct llentry *lle = NULL;
 	struct in6_addr *psrc, src;
 	int send_ns, ll_len;
 	char *lladdr;
@@ -2286,39 +2343,16 @@ nd6_resolve_slow(struct ifnet *ifp, int flags, struct mbuf *m,
 	 * At this point, the destination of the packet must be a unicast
 	 * or an anycast address(i.e. not a multicast).
 	 */
-	if (lle == NULL) {
-		lle = nd6_lookup(&dst->sin6_addr, LLE_EXCLUSIVE, ifp);
-		if ((lle == NULL) && nd6_is_addr_neighbor(dst, ifp))  {
-			/*
-			 * Since nd6_is_addr_neighbor() internally calls nd6_lookup(),
-			 * the condition below is not very efficient.  But we believe
-			 * it is tolerable, because this should be a rare case.
-			 */
-			lle = nd6_alloc(&dst->sin6_addr, 0, ifp);
-			if (lle == NULL) {
-				char ip6buf[INET6_ADDRSTRLEN];
-				log(LOG_DEBUG,
-				    "nd6_output: can't allocate llinfo for %s "
-				    "(ln=%p)\n",
-				    ip6_sprintf(ip6buf, &dst->sin6_addr), lle);
-				m_freem(m);
-				return (ENOBUFS);
-			}
+	lle = nd6_lookup(&dst->sin6_addr, LLE_EXCLUSIVE, ifp);
+	if ((lle == NULL) && nd6_is_addr_neighbor(dst, ifp))  {
+		/*
+		 * Since nd6_is_addr_neighbor() internally calls nd6_lookup(),
+		 * the condition below is not very efficient.  But we believe
+		 * it is tolerable, because this should be a rare case.
+		 */
+		lle = nd6_get_llentry(ifp, &dst->sin6_addr);
+	}
 
-			IF_AFDATA_WLOCK(ifp);
-			LLE_WLOCK(lle);
-			/* Prefer any existing entry over newly-created one */
-			lle_tmp = nd6_lookup(&dst->sin6_addr, LLE_EXCLUSIVE, ifp);
-			if (lle_tmp == NULL)
-				lltable_link_entry(LLTABLE6(ifp), lle);
-			IF_AFDATA_WUNLOCK(ifp);
-			if (lle_tmp != NULL) {
-				lltable_free_entry(LLTABLE6(ifp), lle);
-				lle = lle_tmp;
-				lle_tmp = NULL;
-			}
-		}
-	} 
 	if (lle == NULL) {
 		m_freem(m);
 		return (ENOBUFS);
diff --git a/sys/netinet6/nd6.h b/sys/netinet6/nd6.h
index ee53acce840a..fe0f2b22cc48 100644
--- a/sys/netinet6/nd6.h
+++ b/sys/netinet6/nd6.h
@@ -376,6 +376,7 @@ int nd6_resolve(struct ifnet *, int, struct mbuf *,
 int nd6_ioctl(u_long, caddr_t, struct ifnet *);
 void nd6_cache_lladdr(struct ifnet *, struct in6_addr *,
 	char *, int, int, int);
+bool nd6_try_set_entry_addr(struct ifnet *ifp, struct llentry *lle, char *lladdr);
 struct mbuf *nd6_grab_holdchain(struct llentry *);
 int nd6_flush_holdchain(struct ifnet *, struct llentry *, struct mbuf *);
 int nd6_add_ifa_lle(struct in6_ifaddr *);
diff --git a/sys/netinet6/nd6_nbr.c b/sys/netinet6/nd6_nbr.c
index 0f18a38c37a1..974c454e93a5 100644
--- a/sys/netinet6/nd6_nbr.c
+++ b/sys/netinet6/nd6_nbr.c
@@ -770,16 +770,9 @@ nd6_na_input(struct mbuf *m, int off, int icmp6len)
 		/*
 		 * Record link-layer address, and update the state.
 		 */
-		linkhdrsize = sizeof(linkhdr);
-		if (lltable_calc_llheader(ifp, AF_INET6, lladdr,
-		    linkhdr, &linkhdrsize, &lladdr_off) != 0)
-			return;
-
-		if (lltable_try_set_entry_addr(ifp, ln, linkhdr, linkhdrsize,
-		    lladdr_off) == 0) {
-			ln = NULL;
+		if (!nd6_try_set_entry_addr(ifp, ln, lladdr))
 			goto freeit;
-		}
+
 		EVENTHANDLER_INVOKE(lle_event, ln, LLENTRY_RESOLVED);
 		if (is_solicited)
 			nd6_llinfo_setstate(ln, ND6_LLINFO_REACHABLE);



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