From c97e88384b251efd05d99cc4990115bb68cfac49 Mon Sep 17 00:00:00 2001 From: sashan Date: Thu, 11 Nov 2021 12:35:01 +0000 Subject: [PATCH] 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@ --- sys/net/if_pfsync.c | 4 +-- sys/net/pf_if.c | 81 +++++++++++++++++++++++++++++++++------------ sys/net/pf_ioctl.c | 6 ++-- sys/net/pf_table.c | 4 +-- sys/net/pfvar.h | 6 ++-- 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 @@ -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 *); -- 2.20.1