git: 98fd94dc8331 - stable/12 - pfctl: cache getprotobynumber results

[Mateusz Guzik]

The branch stable/12 has been updated by mjg:

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

commit 98fd94dc83319db1b6bfc9f741245eefb0af7911
Author:     Mateusz Guzik <mjg at FreeBSD.org>
AuthorDate: 2021-07-01 19:25:43 +0000
Commit:     Mateusz Guzik <mjg at FreeBSD.org>
CommitDate: 2021-07-05 11:38:58 +0000

    pfctl: cache getprotobynumber results
    
    As for example pfctl -ss keeps calling it, it saves a lot of overhead
    from elided parsing of /etc/nsswitch.conf and /etc/protocols.
    
    Sample result when running a pre-nvlist binary with nfs root and dumping
    7 mln states:
    before: 24.817u 62.993s 1:28.52 99.1%
    after:  8.064u 1.117s 0:18.87 48.5%
    
    Idea by Jim Thompson
    
    Reviewed by:    kp
    Sponsored by:   Rubicon Communications, LLC ("Netgate")
    
    (cherry picked from commit 858937bea4599d254a97ee6321683f8629604e15)
---
 sbin/pfctl/parse.y          |  8 ++++----
 sbin/pfctl/pf_print_state.c |  6 +++---
 sbin/pfctl/pfctl.c          | 43 +++++++++++++++++++++++++++++++++++++++++++
 sbin/pfctl/pfctl.h          |  2 ++
 sbin/pfctl/pfctl_parser.c   |  6 +++---
 5 files changed, 55 insertions(+), 10 deletions(-)

diff --git a/sbin/pfctl/parse.y b/sbin/pfctl/parse.y
index 06d972556195..1083666e00ce 100644
--- a/sbin/pfctl/parse.y
+++ b/sbin/pfctl/parse.y
@@ -5009,13 +5009,13 @@ expand_label_port(const char *name, char *label, size_t len,
 void
 expand_label_proto(const char *name, char *label, size_t len, u_int8_t proto)
 {
-	struct protoent *pe;
+	const char *protoname;
 	char n[4];
 
 	if (strstr(label, name) != NULL) {
-		pe = getprotobynumber(proto);
-		if (pe != NULL)
-			expand_label_str(label, len, name, pe->p_name);
+		protoname = pfctl_proto2name(proto);
+		if (protoname != NULL)
+			expand_label_str(label, len, name, protoname);
 		else {
 			snprintf(n, sizeof(n), "%u", proto);
 			expand_label_str(label, len, name, n);
diff --git a/sbin/pfctl/pf_print_state.c b/sbin/pfctl/pf_print_state.c
index b1f0079154cf..b66a296d6080 100644
--- a/sbin/pfctl/pf_print_state.c
+++ b/sbin/pfctl/pf_print_state.c
@@ -211,7 +211,7 @@ print_state(struct pfctl_state *s, int opts)
 {
 	struct pfctl_state_peer *src, *dst;
 	struct pfctl_state_key *key, *sk, *nk;
-	struct protoent *p;
+	const char *protoname;
 	int min, sec;
 	sa_family_t af;
 	uint8_t proto;
@@ -243,8 +243,8 @@ print_state(struct pfctl_state *s, int opts)
 			sk->port[1] = nk->port[1];
 	}
 	printf("%s ", s->ifname);
-	if ((p = getprotobynumber(proto)) != NULL)
-		printf("%s ", p->p_name);
+	if ((protoname = pfctl_proto2name(proto)) != NULL)
+		printf("%s ", protoname);
 	else
 		printf("%u ", proto);
 
diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index f82d75198d61..14b7f3a01657 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -254,6 +254,49 @@ usage(void)
 	exit(1);
 }
 
+/*
+ * Cache protocol number to name translations.
+ *
+ * Translation is performed a lot e.g., when dumping states and
+ * getprotobynumber is incredibly expensive.
+ *
+ * Note from the getprotobynumber(3) manpage:
+ * <quote>
+ * These functions use a thread-specific data space; if the data is needed
+ * for future use, it should be copied before any subsequent calls overwrite
+ * it.  Only the Internet protocols are currently understood.
+ * </quote>
+ *
+ * Consequently we only cache the name and strdup it for safety.
+ *
+ * At the time of writing this comment the last entry in /etc/protocols is:
+ * divert  258     DIVERT          # Divert pseudo-protocol [non IANA]
+ */
+const char *
+pfctl_proto2name(int proto)
+{
+	static const char *pfctl_proto_cache[259];
+	struct protoent *p;
+
+	if (proto >= nitems(pfctl_proto_cache)) {
+		p = getprotobynumber(proto);
+		if (p == NULL) {
+			return (NULL);
+		}
+		return (p->p_name);
+	}
+
+	if (pfctl_proto_cache[proto] == NULL) {
+		p = getprotobynumber(proto);
+		if (p == NULL) {
+			return (NULL);
+		}
+		pfctl_proto_cache[proto] = strdup(p->p_name);
+	}
+
+	return (pfctl_proto_cache[proto]);
+}
+
 int
 pfctl_enable(int dev, int opts)
 {
diff --git a/sbin/pfctl/pfctl.h b/sbin/pfctl/pfctl.h
index f8ff5012e01b..80ef184fa90f 100644
--- a/sbin/pfctl/pfctl.h
+++ b/sbin/pfctl/pfctl.h
@@ -138,4 +138,6 @@ void	 pf_remove_if_empty_ruleset(struct pfctl_ruleset *);
 struct pfctl_ruleset	*pf_find_ruleset(const char *);
 struct pfctl_ruleset	*pf_find_or_create_ruleset(const char *);
 
+const char *pfctl_proto2name(int);
+
 #endif /* _PFCTL_H_ */
diff --git a/sbin/pfctl/pfctl_parser.c b/sbin/pfctl/pfctl_parser.c
index ee0f9f8318a9..c38411a145e7 100644
--- a/sbin/pfctl/pfctl_parser.c
+++ b/sbin/pfctl/pfctl_parser.c
@@ -812,10 +812,10 @@ print_rule(struct pfctl_rule *r, const char *anchor_call, int verbose, int numer
 			printf(" inet6");
 	}
 	if (r->proto) {
-		struct protoent	*p;
+		const char *protoname;
 
-		if ((p = getprotobynumber(r->proto)) != NULL)
-			printf(" proto %s", p->p_name);
+		if ((protoname = pfctl_proto2name(r->proto)) != NULL)
+			printf(" proto %s", protoname);
 		else
 			printf(" proto %u", r->proto);
 	}