Date: Mon, 7 Jul 2025 15:07:49 GMT From: Kristof Provost <kp@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: f8a8bb7bade9 - main - pfctl: Improve duplicate table name warning Message-ID: <202507071507.567F7n8Q016403@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=f8a8bb7bade9575c549cbc94500ba706b712c650 commit f8a8bb7bade9575c549cbc94500ba706b712c650 Author: Kristof Provost <kp@FreeBSD.org> AuthorDate: 2025-07-01 10:02:03 +0000 Commit: Kristof Provost <kp@FreeBSD.org> CommitDate: 2025-07-07 15:06:48 +0000 pfctl: Improve duplicate table name warning When creating tables inside anchors, pfctl warned about namespace collisions with global tables, but only in certain cases and with limited information sometimes leaving users clueless. Deferring the check to process_tabledefs() where tables are eventually created, both anchor and table name are known which allows for checking all existing anchors. With this, warn on all duplicates even in dry-runs (`-n') and print quoted names so they can be copied to fix configurations right away. No functional change in parsing or ruleset production. Discussed with and OK sashan Obtained from: OpenBSD, kn <kn@openbsd.org>, 0de3a0c9ad Sponsored by: Rubicon Communications, LLC ("Netgate") --- sbin/pfctl/parse.y | 1 + sbin/pfctl/pfctl.c | 3 --- sbin/pfctl/pfctl.h | 2 +- sbin/pfctl/pfctl_table.c | 26 ++++++++------------------ 4 files changed, 10 insertions(+), 22 deletions(-) diff --git a/sbin/pfctl/parse.y b/sbin/pfctl/parse.y index c59204d3d5a4..0b98bd357a37 100644 --- a/sbin/pfctl/parse.y +++ b/sbin/pfctl/parse.y @@ -5424,6 +5424,7 @@ process_tabledef(char *name, struct table_opts *opts, int popts) if (pf->opts & PF_OPT_VERBOSE) print_tabledef(name, opts->flags, opts->init_addr, &opts->init_nodes); + warn_duplicate_tables(name, pf->anchor->path); if (!(pf->opts & PF_OPT_NOACTION) && pfctl_define_table(name, opts->flags, opts->init_addr, pf->anchor->path, &ab, pf->anchor->ruleset.tticket)) { diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c index 88123b17f1b3..b4732d178cbb 100644 --- a/sbin/pfctl/pfctl.c +++ b/sbin/pfctl/pfctl.c @@ -3435,9 +3435,6 @@ main(int argc, char *argv[]) if (pfctl_rules(dev, rulesopt, opts, optimize, anchorname, NULL)) error = 1; - else if (!(opts & PF_OPT_NOACTION) && - (loadopt & PFCTL_FLAG_TABLE)) - warn_namespace_collision(NULL); } if (opts & PF_OPT_ENABLE) diff --git a/sbin/pfctl/pfctl.h b/sbin/pfctl/pfctl.h index 08d48695709e..d8196c129187 100644 --- a/sbin/pfctl/pfctl.h +++ b/sbin/pfctl/pfctl.h @@ -86,7 +86,7 @@ void pfctl_show_tables(const char *, int); int pfctl_table(int, char *[], char *, const char *, char *, const char *, int); int pfctl_show_altq(int, const char *, int, int); -void warn_namespace_collision(const char *); +void warn_duplicate_tables(const char *, const char *); void pfctl_show_ifaces(const char *, int); void pfctl_show_creators(int); FILE *pfctl_fopen(const char *, const char *); diff --git a/sbin/pfctl/pfctl_table.c b/sbin/pfctl/pfctl_table.c index 53abea3e1ae1..d1f20761a4f4 100644 --- a/sbin/pfctl/pfctl_table.c +++ b/sbin/pfctl/pfctl_table.c @@ -94,7 +94,8 @@ static const char *istats_text[2][2][2] = { goto _error; \ } \ if (nadd) { \ - warn_namespace_collision(table.pfrt_name); \ + warn_duplicate_tables(table.pfrt_name, \ + table.pfrt_anchor); \ xprintf(opts, "%d table created", nadd); \ if (opts & PF_OPT_NOACTION) \ return (0); \ @@ -576,12 +577,10 @@ pfctl_define_table(char *name, int flags, int addrs, const char *anchor, } void -warn_namespace_collision(const char *filter) +warn_duplicate_tables(const char *tablename, const char *anchorname) { struct pfr_buffer b; struct pfr_table *t; - const char *name = NULL, *lastcoll; - int coll = 0; bzero(&b, sizeof(b)); b.pfrb_type = PFRB_TABLES; @@ -597,22 +596,13 @@ warn_namespace_collision(const char *filter) PFRB_FOREACH(t, &b) { if (!(t->pfrt_flags & PFR_TFLAG_ACTIVE)) continue; - if (filter != NULL && strcmp(filter, t->pfrt_name)) + if (!strcmp(anchorname, t->pfrt_anchor)) continue; - if (!t->pfrt_anchor[0]) - name = t->pfrt_name; - else if (name != NULL && !strcmp(name, t->pfrt_name)) { - coll++; - lastcoll = name; - name = NULL; - } + if (!strcmp(tablename, t->pfrt_name)) + warnx("warning: table <%s> already defined" + " in anchor \"%s\"", tablename, + t->pfrt_anchor[0] ? t->pfrt_anchor : "/"); } - if (coll == 1) - warnx("warning: namespace collision with <%s> global table.", - lastcoll); - else if (coll > 1) - warnx("warning: namespace collisions with %d global tables.", - coll); pfr_buf_clear(&b); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202507071507.567F7n8Q016403>