git: f218b851da04 - main - libpfctl: introduce state iterator

From: Kristof Provost <kp_at_FreeBSD.org>
Date: Tue, 10 Oct 2023 09:50:35 UTC
The branch main has been updated by kp:

URL: https://cgit.FreeBSD.org/src/commit/?id=f218b851da0480460195c1128d0c0b41d3b6a6d4

commit f218b851da0480460195c1128d0c0b41d3b6a6d4
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2023-10-02 13:48:18 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2023-10-10 09:48:21 +0000

    libpfctl: introduce state iterator
    
    Allow consumers to start processing states as the kernel supplies them,
    rather than having to build a full list and only then start processing.
    Especially for very large state tables this can significantly reduce
    memory use.
    
    Without this change when retrieving 1M states time -l reports:
    
        real 3.55
        user 1.95
        sys 1.05
            318832  maximum resident set size
               194  average shared memory size
                15  average unshared data size
               127  average unshared stack size
             79041  page reclaims
                 0  page faults
                 0  swaps
                 0  block input operations
                 0  block output operations
             15096  messages sent
            250001  messages received
                 0  signals received
                22  voluntary context switches
                34  involuntary context switches
    
    With it it reported:
    
        real 3.32
        user 1.88
        sys 0.86
              3220  maximum resident set size
               195  average shared memory size
                11  average unshared data size
               128  average unshared stack size
               260  page reclaims
                 0  page faults
                 0  swaps
                 0  block input operations
                 0  block output operations
             15096  messages sent
            250001  messages received
                 0  signals received
                21  voluntary context switches
                31  involuntary context switches
    
    Reviewed by:    mjg
    Sponsored by:   Rubicon Communications, LLC ("Netgate")
    Differential Revision:  https://reviews.freebsd.org/D42091
---
 lib/libpfctl/libpfctl.c | 62 ++++++++++++++++++++++++++++++++++++-------------
 lib/libpfctl/libpfctl.h |  2 ++
 sbin/pfctl/pfctl.c      | 45 +++++++++++++++++++++--------------
 3 files changed, 76 insertions(+), 33 deletions(-)

diff --git a/lib/libpfctl/libpfctl.c b/lib/libpfctl/libpfctl.c
index 8699d8132240..03f80baa2178 100644
--- a/lib/libpfctl/libpfctl.c
+++ b/lib/libpfctl/libpfctl.c
@@ -1203,10 +1203,11 @@ static const struct snl_hdr_parser *all_parsers[] = {
 };
 
 static int
-pfctl_get_states_nl(struct snl_state *ss, struct pfctl_states *states)
+pfctl_get_states_nl(struct snl_state *ss, pfctl_get_state_fn f, void *arg)
 {
 	SNL_VERIFY_PARSERS(all_parsers);
 	int family_id = snl_get_genl_family(ss, PFNL_FAMILY_NAME);
+	int ret;
 
 	struct nlmsghdr *hdr;
 	struct snl_writer nw;
@@ -1219,42 +1220,71 @@ pfctl_get_states_nl(struct snl_state *ss, struct pfctl_states *states)
 
 	snl_send_message(ss, hdr);
 
-	bzero(states, sizeof(*states));
-	TAILQ_INIT(&states->states);
-
 	struct snl_errmsg_data e = {};
 	while ((hdr = snl_read_reply_multi(ss, seq_id, &e)) != NULL) {
-		struct pfctl_state *s = malloc(sizeof(*s));
-		bzero(s, sizeof(*s));
-		if (s == NULL) {
-			pfctl_free_states(states);
-			return (ENOMEM);
-		}
-		if (!snl_parse_nlmsg(ss, hdr, &state_parser, s))
+		struct pfctl_state s;
+		bzero(&s, sizeof(s));
+		if (!snl_parse_nlmsg(ss, hdr, &state_parser, &s))
 			continue;
 
-		s->key[1].af = s->key[0].af;
-		s->key[1].proto = s->key[0].proto;
+		s.key[1].af = s.key[0].af;
+		s.key[1].proto = s.key[0].proto;
 
-		TAILQ_INSERT_TAIL(&states->states, s, entry);
+		ret = f(&s, arg);
+		if (ret != 0)
+			return (ret);
 	}
 
 	return (0);
 }
 
 int
-pfctl_get_states(int dev __unused, struct pfctl_states *states)
+pfctl_get_states_iter(pfctl_get_state_fn f, void *arg)
 {
 	struct snl_state ss = {};
 	int error;
 
 	snl_init(&ss, NETLINK_GENERIC);
-	error = pfctl_get_states_nl(&ss, states);
+	error = pfctl_get_states_nl(&ss, f, arg);
 	snl_free(&ss);
 
 	return (error);
 }
 
+static int
+pfctl_append_states(struct pfctl_state *s, void *arg)
+{
+	struct pfctl_state *new;
+	struct pfctl_states *states = (struct pfctl_states *)arg;
+
+	new = malloc(sizeof(*s));
+	if (new == NULL)
+		return (ENOMEM);
+
+	memcpy(new, s, sizeof(*s));
+
+	TAILQ_INSERT_TAIL(&states->states, s, entry);
+
+	return (0);
+}
+
+int
+pfctl_get_states(int dev __unused, struct pfctl_states *states)
+{
+	int ret;
+
+	bzero(states, sizeof(*states));
+	TAILQ_INIT(&states->states);
+
+	ret = pfctl_get_states_iter(pfctl_append_states, states);
+	if (ret != 0) {
+		pfctl_free_states(states);
+		return (ret);
+	}
+
+	return (0);
+}
+
 void
 pfctl_free_states(struct pfctl_states *states)
 {
diff --git a/lib/libpfctl/libpfctl.h b/lib/libpfctl/libpfctl.h
index 2559fc9c4843..4906ec3ccfce 100644
--- a/lib/libpfctl/libpfctl.h
+++ b/lib/libpfctl/libpfctl.h
@@ -413,6 +413,8 @@ int	pfctl_add_rule(int dev, const struct pfctl_rule *r,
 	    const char *anchor, const char *anchor_call, uint32_t ticket,
 	    uint32_t pool_ticket);
 int	pfctl_set_keepcounters(int dev, bool keep);
+typedef int (*pfctl_get_state_fn)(struct pfctl_state *, void *);
+int pfctl_get_states_iter(pfctl_get_state_fn f, void *arg);
 int	pfctl_get_states(int dev, struct pfctl_states *states);
 void	pfctl_free_states(struct pfctl_states *states);
 int	pfctl_clear_states(int dev, const struct pfctl_kill *kill,
diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index bfa76b299a02..9a75eb7d00b5 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -1514,30 +1514,41 @@ done:
 	return (0);
 }
 
+struct pfctl_show_state_arg {
+	int opts;
+	int dotitle;
+	const char *iface;
+};
+
+static int
+pfctl_show_state(struct pfctl_state *s, void *arg)
+{
+	struct pfctl_show_state_arg *a = (struct pfctl_show_state_arg *)arg;
+
+	if (a->iface != NULL && strcmp(s->ifname, a->iface))
+		return (0);
+
+	if (a->dotitle) {
+		pfctl_print_title("STATES:");
+		a->dotitle = 0;
+	}
+	print_state(s, a->opts);
+
+	return (0);
+}
+
 int
 pfctl_show_states(int dev, const char *iface, int opts)
 {
-	struct pfctl_states states;
-	struct pfctl_state *s;
-	int dotitle = (opts & PF_OPT_SHOWALL);
+	struct pfctl_show_state_arg arg;
 
-	memset(&states, 0, sizeof(states));
+	arg.opts = opts;
+	arg.dotitle = opts & PF_OPT_SHOWALL;
+	arg.iface = iface;
 
-	if (pfctl_get_states(dev, &states))
+	if (pfctl_get_states_iter(pfctl_show_state, &arg))
 		return (-1);
 
-	TAILQ_FOREACH(s, &states.states, entry) {
-		if (iface != NULL && strcmp(s->ifname, iface))
-			continue;
-		if (dotitle) {
-			pfctl_print_title("STATES:");
-			dotitle = 0;
-		}
-		print_state(s, opts);
-	}
-
-	pfctl_free_states(&states);
-
 	return (0);
 }