Allow pfi_kif_get() callers to pre-allocate buffer for new kif. If kif

object exists already, then caller must free the pre-allocated buffer.
If caller does not pre-allocate buffer, the pfi_kif_get() will get
memory from pool using M_NOWAIT flag.

Commit is  also polishing pfi_initialize() a bit so it uses M_WAITOK
allocation for pfi_all.

there is no change in current behaviour.

feedback by bluhm@

OK bluhm@

5 files changed, 70 insertions, 31 deletions

diff --git a/sys/net/if_pfsync.c b/sys/net/if_pfsync.c
index 7d0d6b59ff9..906afa0e3b3 100644
--- a/sys/net/if_pfsync.c
+++ b/sys/net/if_pfsync.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_pfsync.c,v 1.297 2021/07/07 18:38:25 sashan Exp $	*/
+/*	$OpenBSD: if_pfsync.c,v 1.298 2021/11/11 12:35:01 sashan Exp $	*/
 
 /*
  * Copyright (c) 2002 Michael Shalayeff
@@ -547,7 +547,7 @@ pfsync_state_import(struct pfsync_state *sp, int flags)
 		return (EINVAL);
 	}
 
-	if ((kif = pfi_kif_get(sp->ifname)) == NULL) {
+	if ((kif = pfi_kif_get(sp->ifname, NULL)) == NULL) {
 		DPFPRINTF(LOG_NOTICE, "pfsync_state_import: "
 		    "unknown interface: %s", sp->ifname);
 		if (flags & PFSYNC_SI_IOCTL)
diff --git a/sys/net/pf_if.c b/sys/net/pf_if.c
index 30552d106f5..8de37375ab4 100644
--- a/sys/net/pf_if.c
+++ b/sys/net/pf_if.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pf_if.c,v 1.100 2020/06/24 22:03:42 cheloha Exp $ */
+/*	$OpenBSD: pf_if.c,v 1.101 2021/11/11 12:35:01 sashan Exp $ */
 
 /*
  * Copyright 2005 Henning Brauer <henning@openbsd.org>
@@ -82,9 +82,48 @@ RB_GENERATE(pfi_ifhead, pfi_kif, pfik_tree, pfi_if_compare);
 #define PFI_BUFFER_MAX		0x10000
 #define PFI_MTYPE		M_IFADDR
 
+struct pfi_kif *
+pfi_kif_alloc(const char *kif_name, int mflags)
+{
+	struct pfi_kif *kif;
+
+	kif = malloc(sizeof(*pfi_all), PFI_MTYPE, mflags|M_ZERO);
+	strlcpy(kif->pfik_name, kif_name, sizeof(kif->pfik_name));
+	kif->pfik_tzero = gettime();
+	TAILQ_INIT(&kif->pfik_dynaddrs);
+
+	if (!strcmp(kif->pfik_name, "any")) {
+		/* both so it works in the ioctl and the regular case */
+		kif->pfik_flags |= PFI_IFLAG_ANY;
+		kif->pfik_flags_new |= PFI_IFLAG_ANY;
+	}
+
+	return (kif);
+}
+
+void
+pfi_kif_free(struct pfi_kif *kif)
+{
+	if (kif == NULL)
+		return;
+
+	if ((kif->pfik_rules != 0) || (kif->pfik_states != 0) ||
+	    (kif->pfik_states != 0) || (kif->pfik_states != 0) ||
+	    (kif->pfik_srcnodes != 0))
+		panic("kif is still alive");
+
+	free(kif, PFI_MTYPE, sizeof(*kif));
+}
+
 void
 pfi_initialize(void)
 {
+	/*
+	 * The first time we arrive here is during kernel boot,
+	 * when if_attachsetup() for the first time. No locking
+	 * is needed in this case, because it's granted there
+	 * is a single thread, which sets pfi_all global var.
+	 */
 	if (pfi_all != NULL)	/* already initialized */
 		return;
 
@@ -94,8 +133,10 @@ pfi_initialize(void)
 	pfi_buffer = mallocarray(pfi_buffer_max, sizeof(*pfi_buffer),
 	    PFI_MTYPE, M_WAITOK);
 
-	if ((pfi_all = pfi_kif_get(IFG_ALL)) == NULL)
-		panic("pfi_kif_get for pfi_all failed");
+	pfi_all = pfi_kif_alloc(IFG_ALL, M_WAITOK);
+
+	if (RB_INSERT(pfi_ifhead, &pfi_ifs, pfi_all) != NULL)
+		panic("IFG_ALL kif found already");
 }
 
 struct pfi_kif *
@@ -109,7 +150,7 @@ pfi_kif_find(const char *kif_name)
 }
 
 struct pfi_kif *
-pfi_kif_get(const char *kif_name)
+pfi_kif_get(const char *kif_name, struct pfi_kif **prealloc)
 {
 	struct pfi_kif		*kif;
 
@@ -117,17 +158,13 @@ pfi_kif_get(const char *kif_name)
 		return (kif);
 
 	/* create new one */
-	if ((kif = malloc(sizeof(*kif), PFI_MTYPE, M_NOWAIT|M_ZERO)) == NULL)
-		return (NULL);
-
-	strlcpy(kif->pfik_name, kif_name, sizeof(kif->pfik_name));
-	kif->pfik_tzero = gettime();
-	TAILQ_INIT(&kif->pfik_dynaddrs);
-
-	if (!strcmp(kif->pfik_name, "any")) {
-		/* both so it works in the ioctl and the regular case */
-		kif->pfik_flags |= PFI_IFLAG_ANY;
-		kif->pfik_flags_new |= PFI_IFLAG_ANY;
+	if ((prealloc == NULL) || (*prealloc == NULL)) {
+		kif = pfi_kif_alloc(kif_name, M_NOWAIT);
+		if (kif == NULL)
+			return (NULL);
+	} else {
+		kif = *prealloc;
+		*prealloc = NULL;
 	}
 
 	RB_INSERT(pfi_ifhead, &pfi_ifs, kif);
@@ -239,7 +276,7 @@ pfi_attach_ifnet(struct ifnet *ifp)
 
 	pfi_initialize();
 	pfi_update++;
-	if ((kif = pfi_kif_get(ifp->if_xname)) == NULL)
+	if ((kif = pfi_kif_get(ifp->if_xname, NULL)) == NULL)
 		panic("%s: pfi_kif_get failed", __func__);
 
 	kif->pfik_ifp = ifp;
@@ -282,7 +319,7 @@ pfi_attach_ifgroup(struct ifg_group *ifg)
 
 	pfi_initialize();
 	pfi_update++;
-	if ((kif = pfi_kif_get(ifg->ifg_group)) == NULL)
+	if ((kif = pfi_kif_get(ifg->ifg_group, NULL)) == NULL)
 		panic("%s: pfi_kif_get failed", __func__);
 
 	kif->pfik_group = ifg;
@@ -310,7 +347,7 @@ pfi_group_change(const char *group)
 	struct pfi_kif		*kif;
 
 	pfi_update++;
-	if ((kif = pfi_kif_get(group)) == NULL)
+	if ((kif = pfi_kif_get(group, NULL)) == NULL)
 		panic("%s: pfi_kif_get failed", __func__);
 
 	pfi_kif_update(kif);
@@ -321,8 +358,8 @@ pfi_group_addmember(const char *group, struct ifnet *ifp)
 {
 	struct pfi_kif		*gkif, *ikif;
 
-	if ((gkif = pfi_kif_get(group)) == NULL ||
-	    (ikif = pfi_kif_get(ifp->if_xname)) == NULL)
+	if ((gkif = pfi_kif_get(group, NULL)) == NULL ||
+	    (ikif = pfi_kif_get(ifp->if_xname, NULL)) == NULL)
 		panic("%s: pfi_kif_get failed", __func__);
 	ikif->pfik_flags |= gkif->pfik_flags;
 
@@ -377,9 +414,9 @@ pfi_dynaddr_setup(struct pf_addr_wrap *aw, sa_family_t af)
 		return (1);
 
 	if (!strcmp(aw->v.ifname, "self"))
-		dyn->pfid_kif = pfi_kif_get(IFG_ALL);
+		dyn->pfid_kif = pfi_kif_get(IFG_ALL, NULL);
 	else
-		dyn->pfid_kif = pfi_kif_get(aw->v.ifname);
+		dyn->pfid_kif = pfi_kif_get(aw->v.ifname, NULL);
 	if (dyn->pfid_kif == NULL) {
 		rv = 1;
 		goto _bad;
diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c
index d4893d70d0f..e960f770a46 100644
--- a/sys/net/pf_ioctl.c
+++ b/sys/net/pf_ioctl.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pf_ioctl.c,v 1.365 2021/06/23 06:53:52 dlg Exp $ */
+/*	$OpenBSD: pf_ioctl.c,v 1.366 2021/11/11 12:35:01 sashan Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -884,7 +884,7 @@ int
 pf_kif_setup(char *ifname, struct pfi_kif **kif)
 {
 	if (ifname[0]) {
-		*kif = pfi_kif_get(ifname);
+		*kif = pfi_kif_get(ifname, NULL);
 		if (*kif == NULL)
 			return (EINVAL);
 
@@ -1289,7 +1289,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			pool_put(&pf_queue_pl, qs);
 			break;
 		}
-		qs->kif = pfi_kif_get(qs->ifname);
+		qs->kif = pfi_kif_get(qs->ifname, NULL);
 		if (qs->kif == NULL) {
 			error = ESRCH;
 			PF_UNLOCK();
diff --git a/sys/net/pf_table.c b/sys/net/pf_table.c
index be0a3e3220a..3a136a8da4f 100644
--- a/sys/net/pf_table.c
+++ b/sys/net/pf_table.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pf_table.c,v 1.136 2021/10/24 10:58:43 sashan Exp $	*/
+/*	$OpenBSD: pf_table.c,v 1.137 2021/11/11 12:35:01 sashan Exp $	*/
 
 /*
  * Copyright (c) 2002 Cedric Berger
@@ -848,7 +848,7 @@ pfr_create_kentry(struct pfr_addr *ad)
 		/* FALLTHROUGH */
 	case PFRKE_ROUTE:
 		if (ad->pfra_ifname[0])
-			ke->pfrke_rkif = pfi_kif_get(ad->pfra_ifname);
+			ke->pfrke_rkif = pfi_kif_get(ad->pfra_ifname, NULL);
 		if (ke->pfrke_rkif)
 			pfi_kif_ref(ke->pfrke_rkif, PFI_KIF_REF_ROUTE);
 		break;
diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index d2f2464bc25..1baee0f474a 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pfvar.h,v 1.502 2021/06/23 06:53:52 dlg Exp $ */
+/*	$OpenBSD: pfvar.h,v 1.503 2021/11/11 12:35:01 sashan Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -1849,8 +1849,10 @@ struct pfr_ktable
 extern struct pfi_kif		*pfi_all;
 
 void		 pfi_initialize(void);
+struct pfi_kif	*pfi_kif_alloc(const char *, int);
+void		 pfi_kif_free(struct pfi_kif *);
 struct pfi_kif	*pfi_kif_find(const char *);
-struct pfi_kif	*pfi_kif_get(const char *);
+struct pfi_kif	*pfi_kif_get(const char *, struct pfi_kif **);
 void		 pfi_kif_ref(struct pfi_kif *, enum pfi_kif_refs);
 void		 pfi_kif_unref(struct pfi_kif *, enum pfi_kif_refs);
 int		 pfi_kif_match(struct pfi_kif *, struct pfi_kif *);