Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 11 May 2021 15:06:50 GMT
From:      Kristof Provost <kp@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: 09db9de2fea8 - stable/13 - pf: Error tracing SDTs
Message-ID:  <202105111506.14BF6oHn049670@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by kp:

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

commit 09db9de2fea847de2ebc35acd7333c97390dc8f8
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2021-04-20 09:18:26 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2021-05-11 15:04:45 +0000

    pf: Error tracing SDTs
    
    Add additional DTrace static trace points to facilitate debugging
    failing pf ioctl calls.
    
    MFC after:      1 week
    Sponsored by:   Rubicon Communications, LLC ("Netgate")
    
    (cherry picked from commit 6b146f3b9b665c9baf6ba2cb038bbee359cb738a)
---
 sys/netpfil/pf/pf_ioctl.c | 102 +++++++++++++++++++++++++---------------------
 sys/netpfil/pf/pf_nv.h    |   1 +
 2 files changed, 57 insertions(+), 46 deletions(-)

diff --git a/sys/netpfil/pf/pf_ioctl.c b/sys/netpfil/pf/pf_ioctl.c
index 36e3fb8ff332..d9e313f62683 100644
--- a/sys/netpfil/pf/pf_ioctl.c
+++ b/sys/netpfil/pf/pf_ioctl.c
@@ -63,6 +63,7 @@ __FBSDID("$FreeBSD$");
 #include <sys/module.h>
 #include <sys/nv.h>
 #include <sys/proc.h>
+#include <sys/sdt.h>
 #include <sys/smp.h>
 #include <sys/socket.h>
 #include <sys/sysctl.h>
@@ -93,6 +94,12 @@ __FBSDID("$FreeBSD$");
 #include <net/altq/altq.h>
 #endif
 
+SDT_PROVIDER_DECLARE(pf);
+SDT_PROBE_DEFINE3(pf, ioctl, ioctl, error, "int", "int", "int");
+SDT_PROBE_DEFINE3(pf, ioctl, function, error, "char *", "int", "int");
+SDT_PROBE_DEFINE2(pf, ioctl, addrule, error, "int", "int");
+SDT_PROBE_DEFINE2(pf, ioctl, nvchk, error, "int", "int");
+
 static struct pf_kpool	*pf_get_kpool(char *, u_int32_t, u_int8_t, u_int32_t,
 			    u_int8_t, u_int8_t, u_int8_t);
 
@@ -260,6 +267,14 @@ pflog_packet_t			*pflog_packet_ptr = NULL;
 
 extern u_long	pf_ioctl_maxcount;
 
+#define	ERROUT_FUNCTION(target, x)					\
+	do {								\
+		error = (x);						\
+		SDT_PROBE3(pf, ioctl, function, error, __func__, error,	\
+		    __LINE__);						\
+		goto target;						\
+	} while (0)
+
 static void
 pfattach_vnet(void)
 {
@@ -1962,23 +1977,23 @@ pf_nvrule_to_krule(const nvlist_t *nvl, struct pf_krule **prule)
 	struct pf_krule *rule;
 	int error = 0;
 
+#define	ERROUT(x)	ERROUT_FUNCTION(errout, x)
+
 	rule = malloc(sizeof(*rule), M_PFRULE, M_WAITOK | M_ZERO);
 
 	PFNV_CHK(pf_nvuint32(nvl, "nr", &rule->nr));
 
-	if (! nvlist_exists_nvlist(nvl, "src")) {
-		error = EINVAL;
-		goto errout;
-	}
+	if (! nvlist_exists_nvlist(nvl, "src"))
+		ERROUT(EINVAL);
+
 	error = pf_nvrule_addr_to_rule_addr(nvlist_get_nvlist(nvl, "src"),
 	    &rule->src);
 	if (error != 0)
-		goto errout;
+		ERROUT(error);
+
+	if (! nvlist_exists_nvlist(nvl, "dst"))
+		ERROUT(EINVAL);
 
-	if (! nvlist_exists_nvlist(nvl, "dst")) {
-		error = EINVAL;
-		goto errout;
-	}
 	PFNV_CHK(pf_nvrule_addr_to_rule_addr(nvlist_get_nvlist(nvl, "dst"),
 	    &rule->dst));
 
@@ -1991,18 +2006,14 @@ pf_nvrule_to_krule(const nvlist_t *nvl, struct pf_krule **prule)
 		int ret;
 
 		strs = nvlist_get_string_array(nvl, "labels", &items);
-		if (items > PF_RULE_MAX_LABEL_COUNT) {
-			error = E2BIG;
-			goto errout;
-		}
+		if (items > PF_RULE_MAX_LABEL_COUNT)
+			ERROUT(E2BIG);
 
 		for (size_t i = 0; i < items; i++) {
 			ret = strlcpy(rule->label[i], strs[i],
 			    sizeof(rule->label[0]));
-			if (ret >= sizeof(rule->label[0])) {
-				error = E2BIG;
-				goto errout;
-			}
+			if (ret >= sizeof(rule->label[0]))
+				ERROUT(E2BIG);
 		}
 	}
 
@@ -2018,10 +2029,8 @@ pf_nvrule_to_krule(const nvlist_t *nvl, struct pf_krule **prule)
 	PFNV_CHK(pf_nvstring(nvl, "overload_tblname", rule->overload_tblname,
 	    sizeof(rule->overload_tblname)));
 
-	if (! nvlist_exists_nvlist(nvl, "rpool")) {
-		error = EINVAL;
-		goto errout;
-	}
+	if (! nvlist_exists_nvlist(nvl, "rpool"))
+		ERROUT(EINVAL);
 	PFNV_CHK(pf_nvpool_to_pool(nvlist_get_nvlist(nvl, "rpool"),
 	    &rule->rpool));
 
@@ -2047,17 +2056,13 @@ pf_nvrule_to_krule(const nvlist_t *nvl, struct pf_krule **prule)
 	PFNV_CHK(pf_nvuint16(nvl, "max_mss", &rule->max_mss));
 	PFNV_CHK(pf_nvuint16(nvl, "scrub_flags", &rule->scrub_flags));
 
-	if (! nvlist_exists_nvlist(nvl, "uid")) {
-		error = EINVAL;
-		goto errout;
-	}
+	if (! nvlist_exists_nvlist(nvl, "uid"))
+		ERROUT(EINVAL);
 	PFNV_CHK(pf_nvrule_uid_to_rule_uid(nvlist_get_nvlist(nvl, "uid"),
 	    &rule->uid));
 
-	if (! nvlist_exists_nvlist(nvl, "gid")) {
-		error = EINVAL;
-		goto errout;
-	}
+	if (! nvlist_exists_nvlist(nvl, "gid"))
+		ERROUT(EINVAL);
 	PFNV_CHK(pf_nvrule_gid_to_rule_gid(nvlist_get_nvlist(nvl, "gid"),
 	    &rule->gid));
 
@@ -2095,10 +2100,8 @@ pf_nvrule_to_krule(const nvlist_t *nvl, struct pf_krule **prule)
 	if (nvlist_exists_nvlist(nvl, "divert")) {
 		const nvlist_t *nvldivert = nvlist_get_nvlist(nvl, "divert");
 
-		if (! nvlist_exists_nvlist(nvldivert, "addr")) {
-			error = EINVAL;
-			goto errout;
-		}
+		if (! nvlist_exists_nvlist(nvldivert, "addr"))
+			ERROUT(EINVAL);
 		PFNV_CHK(pf_nvaddr_to_addr(nvlist_get_nvlist(nvldivert, "addr"),
 		    &rule->divert.addr));
 		PFNV_CHK(pf_nvuint16(nvldivert, "port", &rule->divert.port));
@@ -2106,16 +2109,12 @@ pf_nvrule_to_krule(const nvlist_t *nvl, struct pf_krule **prule)
 
 	/* Validation */
 #ifndef INET
-	if (rule->af == AF_INET) {
-		error = EAFNOSUPPORT;
-		goto errout;
-	}
+	if (rule->af == AF_INET)
+		ERROUT(EAFNOSUPPORT);
 #endif /* INET */
 #ifndef INET6
-	if (rule->af == AF_INET6) {
-		error = EAFNOSUPPORT;
-		goto errout;
-	}
+	if (rule->af == AF_INET6)
+		ERROUT(EAFNOSUPPORT);
 #endif /* INET6 */
 
 	PFNV_CHK(pf_check_rule_addr(&rule->src));
@@ -2125,6 +2124,7 @@ pf_nvrule_to_krule(const nvlist_t *nvl, struct pf_krule **prule)
 
 	return (0);
 
+#undef ERROUT
 errout:
 	pf_krule_free(rule);
 	*prule = NULL;
@@ -2492,7 +2492,7 @@ pf_ioctl_addrule(struct pf_krule *rule, uint32_t ticket,
 		goto errout_unlocked;
 	}
 
-#define	ERROUT(x)	{ error = (x); goto errout; }
+#define	ERROUT(x)	ERROUT_FUNCTION(errout, x)
 
 	if (rule->ifname[0])
 		kif = pf_kkif_create(M_WAITOK);
@@ -2639,6 +2639,14 @@ pfioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flags, struct thread *td
 	int			 error = 0;
 	PF_RULES_RLOCK_TRACKER;
 
+#define	ERROUT_IOCTL(target, x)					\
+    do {								\
+	    error = (x);						\
+	    SDT_PROBE3(pf, ioctl, ioctl, error, cmd, error, __LINE__);	\
+	    goto target;						\
+    } while (0)
+
+
 	/* XXX keep in sync with switch() below */
 	if (securelevel_gt(td->td_ucred, 2))
 		switch (cmd) {
@@ -2793,7 +2801,7 @@ pfioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flags, struct thread *td
 		const char	*anchor = "", *anchor_call = "";
 		uint32_t	 ticket = 0, pool_ticket = 0;
 
-#define	ERROUT(x)	do { error = (x); goto DIOCADDRULENV_error; } while (0)
+#define	ERROUT(x)	ERROUT_IOCTL(DIOCADDRULENV_error, x)
 
 		if (nv->len > pf_ioctl_maxcount)
 			ERROUT(ENOMEM);
@@ -2962,7 +2970,7 @@ DIOCADDRULENV_error:
 		int			 rs_num, nr;
 		bool			 clear_counter = false;
 
-#define	ERROUT(x)	do { error = (x); goto DIOCGETRULENV_error; } while (0)
+#define	ERROUT(x)	ERROUT_IOCTL(DIOCGETRULENV_error, x)
 
 		if (nv->len > pf_ioctl_maxcount)
 			ERROUT(ENOMEM);
@@ -3997,7 +4005,7 @@ DIOCGETSTATES_full:
 				kif = pf_kkif_create(M_WAITOK);
 			newpa->kif = NULL;
 		}
-#define	ERROUT(x)	{ error = (x); goto DIOCCHANGEADDR_error; }
+#define	ERROUT(x)	ERROUT_IOCTL(DIOCCHANGEADDR_error, x)
 		PF_RULES_WLOCK();
 		ruleset = pf_find_kruleset(pca->anchor);
 		if (ruleset == NULL)
@@ -5129,6 +5137,8 @@ fail:
 		sx_xunlock(&pf_ioctl_lock);
 	CURVNET_RESTORE();
 
+#undef ERROUT_IOCTL
+
 	return (error);
 }
 
@@ -5337,7 +5347,7 @@ pf_keepcounters(struct pfioc_nv *nv)
 	void		*nvlpacked = NULL;
 	int		 error = 0;
 
-#define	ERROUT(x)	do { error = (x); goto on_error; } while (0)
+#define	ERROUT(x)	ERROUT_FUNCTION(on_error, x)
 
 	if (nv->len > pf_ioctl_maxcount)
 		ERROUT(ENOMEM);
diff --git a/sys/netpfil/pf/pf_nv.h b/sys/netpfil/pf/pf_nv.h
index 0a0f9beeef40..d50f46a1f5cd 100644
--- a/sys/netpfil/pf/pf_nv.h
+++ b/sys/netpfil/pf/pf_nv.h
@@ -53,6 +53,7 @@ int	pf_nvstring(const nvlist_t *, const char *, char *, size_t);
 
 #define	PFNV_CHK(x)	do {	\
 	error = (x);		\
+	SDT_PROBE2(pf, ioctl, nvchk, error, error, __LINE__);	\
 	if (error != 0)		\
 		goto errout;	\
 	} while (0)



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202105111506.14BF6oHn049670>