Re: avoid flushing relayd pf tables during config reload

On Mon Apr 07, 2025 at 12:48:37PM -0400, Mark Johnston wrote:
> Hi,
> 
> One user of relayd on FreeBSD reported a problem where pf redirects
> installed by relayd would not apply when connections arrive in the
> middle of a relayd config reload.  The reload window is generally quite
> short, but connectivity problems were noticeable nonetheless.  A patch
> is below; I tested it manually against OpenBSD 7.6 and ran the
> regression tests.
> 
> More specifically, when handling a config reload, relayd's PFE  receives
> all of the new config structures (redirects, tables, etc.), and then
> flushes all of the corresponding pf tables prior to syncing them with
> pf.  This behaviour results in a small window where a redirect rule will
> fail to handle new connection requests.
> 
> I believe the intent behind flushing here is just to ensure that
> disabled redirects are removed from the ruleset.  pfe_sync() and
> sync_table() will handle the case where a table's contents change across
> a reload.  So, the patch stops relayd from flushing a table unless it
> was explicitly disabled.
> 
> diff --git a/usr.sbin/relayd/pfe_filter.c b/usr.sbin/relayd/pfe_filter.c
> index c1851260c62..7fe0c30cf4c 100644
> --- a/usr.sbin/relayd/pfe_filter.c
> +++ b/usr.sbin/relayd/pfe_filter.c
> @@ -91,10 +91,14 @@ init_tables(struct relayd *env)
>  		return;
>  
>  	/*
> -	 * clear all tables, since some already existed
> +	 * Clear disabled tables and tables belonging to disabled redirects.
> +	 * Don't touch enabled tables since that could disrupt traffic.
>  	 */
> -	TAILQ_FOREACH(rdr, env->sc_rdrs, entry)
> -		flush_table(env, rdr);
> +	TAILQ_FOREACH(rdr, env->sc_rdrs, entry) {
> +		if (rdr->conf.flags & F_DISABLE ||
> +		    rdr->table->conf.flags & F_DISABLE)
> +			flush_table(env, rdr);
> +	}
>  
>  	return;
>  
> 

Thanks Mark, I would like to backport this to fixes to our relayd. See
diff beflow.

I'm aware that this is just a workaround and that state tracking across
reloads should be fixed in general. However, this fix makes sense for
now.

OK?

commit 01d943b2f8caaa04284cc15c096d5f9a6628cc98
Author: Rafael Sadowski <[email protected]>
Date:   Fri Jul 10 15:23:49 2026 +0200

    relayd: Explicitly flush redirect rules after reloading
    
    If a config update disabled a redirect, the corresponding NAT rule was
    not getting flushed. This is because the F_ACTIVE_RULESET flag does not
    get preserved across reloads, so pfe_sync() doesn't know to disable the
    rule. Update init_tables() to handle this.
    
    Backport of FreeBSD relayd commit (Marius Halden <mhalden X entersekt X com>),
    Reported by Mark Johnston <markj X freebsd X org>

diff --git a/pfe_filter.c b/pfe_filter.c
index a3587dd..c071ea5 100644
--- a/pfe_filter.c
+++ b/pfe_filter.c
@@ -92,10 +92,16 @@ init_tables(struct relayd *env)
 		return;
 
 	/*
-	 * clear all tables, since some already existed
+	 * Clear disabled tables and tables belonging to disabled redirects.
+	 * Don't touch enabled tables since that could disrupt traffic.
 	 */
-	TAILQ_FOREACH(rdr, env->sc_rdrs, entry)
-		flush_table(env, rdr);
+	TAILQ_FOREACH(rdr, env->sc_rdrs, entry) {
+		if (rdr->conf.flags & F_DISABLE)
+			sync_ruleset(env, rdr, 0);
+		if (rdr->conf.flags & F_DISABLE ||
+		    rdr->table->conf.flags & F_DISABLE)
+			flush_table(env, rdr);
+	}
 
 	return;