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@

diff --git a/sys/net/if_pfsync.c b/sys/net/if_pfsync.c
index 7d0d6b5..906afa0 100644 (file)
--- 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 30552d1..8de3737 100644 (file)
--- 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 d4893d7..e960f77 100644 (file)
--- 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 be0a3e3..3a136a8 100644 (file)
--- 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 d2f2464..1baee0f 100644 (file)
--- 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 *);