From: claudio <claudio@openbsd.org>
Date: Fri, 9 Dec 2022 17:32:53 +0000 (+0000)
Subject: Switch nd_opts from a union to just a struct.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=eaf13d837ca2e04b36ae3071804e300d22ccf55f;p=openbsd

Switch nd_opts from a union to just a struct.
The ND6 option handling in the kernel got a lot simpler since only
the tgt and src lladdr option are inspected by the kernel. The magic
of assigning options via one side of the union and accessing them
via the other is total overkill and actually quite error prone.
OK florian@
---

diff --git a/sys/netinet6/icmp6.c b/sys/netinet6/icmp6.c
index a9bf43e20b9..fb218628b85 100644
--- a/sys/netinet6/icmp6.c
+++ b/sys/netinet6/icmp6.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: icmp6.c,v 1.242 2022/05/05 13:57:40 claudio Exp $	*/
+/*	$OpenBSD: icmp6.c,v 1.243 2022/12/09 17:32:53 claudio Exp $	*/
 /*	$KAME: icmp6.c,v 1.217 2001/06/20 15:03:29 jinmei Exp $	*/
 
 /*
@@ -1234,7 +1234,7 @@ icmp6_redirect_input(struct mbuf *m, int off)
 	struct in6_addr src6 = ip6->ip6_src;
 	struct in6_addr redtgt6;
 	struct in6_addr reddst6;
-	union nd_opts ndopts;
+	struct nd_opts ndopts;
 	char addr[INET6_ADDRSTRLEN];
 
 	ifp = if_get(m->m_pkthdr.ph_ifidx);
diff --git a/sys/netinet6/nd6.c b/sys/netinet6/nd6.c
index 0647837b2ef..37c2f320af1 100644
--- a/sys/netinet6/nd6.c
+++ b/sys/netinet6/nd6.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: nd6.c,v 1.258 2022/12/07 17:34:20 kn Exp $	*/
+/*	$OpenBSD: nd6.c,v 1.259 2022/12/09 17:32:53 claudio Exp $	*/
 /*	$KAME: nd6.c,v 1.280 2002/06/08 19:52:07 itojun Exp $	*/
 
 /*
@@ -101,6 +101,8 @@ struct timeout nd6_slowtimo_ch;
 struct timeout nd6_expire_timeout;
 struct task nd6_expire_task;
 
+struct nd_opt_hdr *nd6_option(struct nd_opts *);
+
 void
 nd6_init(void)
 {
@@ -138,7 +140,7 @@ nd6_ifdetach(struct ifnet *ifp)
 }
 
 void
-nd6_option_init(void *opt, int icmp6len, union nd_opts *ndopts)
+nd6_option_init(void *opt, int icmp6len, struct nd_opts *ndopts)
 {
 	bzero(ndopts, sizeof(*ndopts));
 	ndopts->nd_opts_search = (struct nd_opt_hdr *)opt;
@@ -155,15 +157,11 @@ nd6_option_init(void *opt, int icmp6len, union nd_opts *ndopts)
  * Take one ND option.
  */
 struct nd_opt_hdr *
-nd6_option(union nd_opts *ndopts)
+nd6_option(struct nd_opts *ndopts)
 {
 	struct nd_opt_hdr *nd_opt;
 	int olen;
 
-	if (!ndopts)
-		panic("%s: ndopts == NULL", __func__);
-	if (!ndopts->nd_opts_last)
-		panic("%s: uninitialized ndopts", __func__);
 	if (!ndopts->nd_opts_search)
 		return NULL;
 	if (ndopts->nd_opts_done)
@@ -206,21 +204,18 @@ nd6_option(union nd_opts *ndopts)
  * multiple options of the same type.
  */
 int
-nd6_options(union nd_opts *ndopts)
+nd6_options(struct nd_opts *ndopts)
 {
 	struct nd_opt_hdr *nd_opt;
 	int i = 0;
 
-	if (!ndopts)
-		panic("%s: ndopts == NULL", __func__);
-	if (!ndopts->nd_opts_last)
-		panic("%s: uninitialized ndopts", __func__);
-	if (!ndopts->nd_opts_search)
+	KASSERT(ndopts->nd_opts_last != NULL);
+	if (ndopts->nd_opts_search == NULL)
 		return 0;
 
 	while (1) {
 		nd_opt = nd6_option(ndopts);
-		if (!nd_opt && !ndopts->nd_opts_last) {
+		if (nd_opt == NULL && ndopts->nd_opts_last == NULL) {
 			/*
 			 * Message validation requires that all included
 			 * options have a length that is greater than zero.
@@ -230,28 +225,30 @@ nd6_options(union nd_opts *ndopts)
 			return -1;
 		}
 
-		if (!nd_opt)
+		if (nd_opt == NULL)
 			goto skip1;
 
 		switch (nd_opt->nd_opt_type) {
 		case ND_OPT_SOURCE_LINKADDR:
+			if (ndopts->nd_opts_src_lladdr != NULL)
+				nd6log((LOG_INFO, "duplicated ND6 option found "
+				    "(type=%d)\n", nd_opt->nd_opt_type));
+			else
+				ndopts->nd_opts_src_lladdr = nd_opt;
+			break;
 		case ND_OPT_TARGET_LINKADDR:
+			if (ndopts->nd_opts_tgt_lladdr != NULL)
+				nd6log((LOG_INFO, "duplicated ND6 option found "
+				    "(type=%d)\n", nd_opt->nd_opt_type));
+			else
+				ndopts->nd_opts_tgt_lladdr = nd_opt;
+			break;
 		case ND_OPT_MTU:
 		case ND_OPT_REDIRECTED_HEADER:
-			if (ndopts->nd_opt_array[nd_opt->nd_opt_type]) {
-				nd6log((LOG_INFO,
-				    "duplicated ND6 option found (type=%d)\n",
-				    nd_opt->nd_opt_type));
-				/* XXX bark? */
-			} else {
-				ndopts->nd_opt_array[nd_opt->nd_opt_type]
-					= nd_opt;
-			}
-			break;
 		case ND_OPT_PREFIX_INFORMATION:
 		case ND_OPT_DNSSL:
 		case ND_OPT_RDNSS:
-			/* Don't warn */
+			/* Don't warn, not used by kernel */
 			break;
 		default:
 			/*
@@ -261,6 +258,7 @@ nd6_options(union nd_opts *ndopts)
 			nd6log((LOG_DEBUG,
 			    "nd6_options: unsupported option %d - "
 			    "option ignored\n", nd_opt->nd_opt_type));
+			break;
 		}
 
 skip1:
diff --git a/sys/netinet6/nd6.h b/sys/netinet6/nd6.h
index 43a4c6a3819..780434a2e82 100644
--- a/sys/netinet6/nd6.h
+++ b/sys/netinet6/nd6.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: nd6.h,v 1.92 2022/12/07 17:34:20 kn Exp $	*/
+/*	$OpenBSD: nd6.h,v 1.93 2022/12/09 17:32:53 claudio Exp $	*/
 /*	$KAME: nd6.h,v 1.95 2002/06/08 11:31:06 itojun Exp $	*/
 
 /*
@@ -108,34 +108,20 @@ extern int nd6_debug;
 
 #define nd6log(x)	do { if (nd6_debug) log x; } while (0)
 
-union nd_opts {
-	struct nd_opt_hdr *nd_opt_array[9];
-	struct {
-		struct nd_opt_hdr *zero;
-		struct nd_opt_hdr *src_lladdr;
-		struct nd_opt_hdr *tgt_lladdr;
-		struct nd_opt_prefix_info *pi_beg; /* multiple opts, start */
-		struct nd_opt_rd_hdr *rh;
-		struct nd_opt_mtu *mtu;
-		struct nd_opt_hdr *search;	/* multiple opts */
-		struct nd_opt_hdr *last;	/* multiple opts */
-		int done;
-		struct nd_opt_prefix_info *pi_end;/* multiple opts, end */
-	} nd_opt_each;
+struct nd_opts {
+	struct nd_opt_hdr *nd_opts_src_lladdr;
+	struct nd_opt_hdr *nd_opts_tgt_lladdr;
+	struct nd_opt_hdr *nd_opts_search;	/* multiple opts */
+	struct nd_opt_hdr *nd_opts_last;	/* multiple opts */
+	int nd_opts_done;
 };
-#define nd_opts_src_lladdr	nd_opt_each.src_lladdr
-#define nd_opts_tgt_lladdr	nd_opt_each.tgt_lladdr
-#define nd_opts_search		nd_opt_each.search
-#define nd_opts_last		nd_opt_each.last
-#define nd_opts_done		nd_opt_each.done
 
 void nd6_init(void);
 void nd6_ifattach(struct ifnet *);
 void nd6_ifdetach(struct ifnet *);
 int nd6_is_addr_neighbor(const struct sockaddr_in6 *, struct ifnet *);
-void nd6_option_init(void *, int, union nd_opts *);
-struct nd_opt_hdr *nd6_option(union nd_opts *);
-int nd6_options(union nd_opts *);
+void nd6_option_init(void *, int, struct nd_opts *);
+int nd6_options(struct nd_opts *);
 struct	rtentry *nd6_lookup(const struct in6_addr *, int, struct ifnet *,
     u_int);
 void nd6_llinfo_settimer(const struct llinfo_nd6 *, unsigned int);
diff --git a/sys/netinet6/nd6_nbr.c b/sys/netinet6/nd6_nbr.c
index 09a9e5e4e95..b7377f51639 100644
--- a/sys/netinet6/nd6_nbr.c
+++ b/sys/netinet6/nd6_nbr.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: nd6_nbr.c,v 1.138 2022/12/02 15:35:35 kn Exp $	*/
+/*	$OpenBSD: nd6_nbr.c,v 1.139 2022/12/09 17:32:53 claudio Exp $	*/
 /*	$KAME: nd6_nbr.c,v 1.61 2001/02/10 16:06:14 jinmei Exp $	*/
 
 /*
@@ -109,7 +109,7 @@ nd6_ns_input(struct mbuf *m, int off, int icmp6len)
 	int anycast = 0, proxy = 0, tentative = 0;
 	int router = ip6_forwarding;
 	int tlladdr;
-	union nd_opts ndopts;
+	struct nd_opts ndopts;
 	struct sockaddr_dl *proxydl = NULL;
 	char addr[INET6_ADDRSTRLEN], addr0[INET6_ADDRSTRLEN];
 
@@ -573,7 +573,7 @@ nd6_na_input(struct mbuf *m, int off, int icmp6len)
 	struct llinfo_nd6 *ln;
 	struct rtentry *rt = NULL;
 	struct sockaddr_dl *sdl;
-	union nd_opts ndopts;
+	struct nd_opts ndopts;
 	char addr[INET6_ADDRSTRLEN], addr0[INET6_ADDRSTRLEN];
 
 	NET_ASSERT_LOCKED();
diff --git a/sys/netinet6/nd6_rtr.c b/sys/netinet6/nd6_rtr.c
index 87adb7ce73f..c91aa4bfa78 100644
--- a/sys/netinet6/nd6_rtr.c
+++ b/sys/netinet6/nd6_rtr.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: nd6_rtr.c,v 1.167 2019/06/21 17:11:43 mpi Exp $	*/
+/*	$OpenBSD: nd6_rtr.c,v 1.168 2022/12/09 17:32:53 claudio Exp $	*/
 /*	$KAME: nd6_rtr.c,v 1.97 2001/02/07 11:09:13 itojun Exp $	*/
 
 /*
@@ -73,7 +73,7 @@ nd6_rtr_cache(struct mbuf *m, int off, int icmp6len, int icmp6_type)
 	struct in6_addr saddr6 = ip6->ip6_src;
 	char *lladdr = NULL;
 	int lladdrlen = 0;
-	union nd_opts ndopts;
+	struct nd_opts ndopts;
 	char src[INET6_ADDRSTRLEN], dst[INET6_ADDRSTRLEN];
 
 	KASSERT(icmp6_type == ND_ROUTER_SOLICIT || icmp6_type ==