On Mon, Sep 14, 2020 at 02:09:27PM +0900, YASUOKA Masahiko wrote:
> When pf rule with a "on rdomain n" with nonexisting rdomain n causes
>
> /etc/pf.conf:XXX: rdomain n does not exist
Actually, that should just work regardless of whether the rounting
domain exists at ruleset creation time; just like it is the case with
interface names/groups which may come and go at runtime without
requiring changes to the ruleset.
Rules on nonexistent interfaces won't match, routing domains (and
ultimately routing tables) should behave the same, I think.
Here's a diff that does this for routing domains allowing me to always
use `on rdomain 5' - I've tested it with a few examplatory rulesets and
behaviour is as expected.
It will need more eye balling and I am not pushing such changes before
release, but if that is a general direction we agree, your proposed
`rtable' fix could move along and become just as flexible instead.
Discussed with claudio at k2k20.
Index: /sys/net/pf_ioctl.c
===================================================================
RCS file: /cvs/src/sys/net/pf_ioctl.c,v
retrieving revision 1.356
diff -u -p -r1.356 pf_ioctl.c
--- /sys/net/pf_ioctl.c 24 Aug 2020 15:41:15 -0000 1.356
+++ /sys/net/pf_ioctl.c 14 Sep 2020 22:27:55 -0000
@@ -2820,10 +2820,8 @@ pf_rule_copyin(struct pf_rule *from, str
if (to->rtableid >= 0 && !rtable_exists(to->rtableid))
return (EBUSY);
to->onrdomain = from->onrdomain;
- if (to->onrdomain >= 0 && !rtable_exists(to->onrdomain))
- return (EBUSY);
- if (to->onrdomain >= 0) /* make sure it is a real rdomain */
- to->onrdomain = rtable_l2(to->onrdomain);
+ if (to->rtableid < 0 || to->rtableid > RT_TABLEID_MAX)
+ return (EINVAL);
for (i = 0; i < PFTM_MAX; i++)
to->timeout[i] = from->timeout[i];
Index: sbin/pfctl/parse.y
===================================================================
RCS file: /cvs/src/sbin/pfctl/parse.y,v
retrieving revision 1.701
diff -u -p -r1.701 parse.y
--- sbin/pfctl/parse.y 28 Jan 2020 15:40:35 -0000 1.701
+++ sbin/pfctl/parse.y 14 Sep 2020 21:52:54 -0000
@@ -392,7 +392,6 @@ int invalid_redirect(struct node_host *
u_int16_t parseicmpspec(char *, sa_family_t);
int kw_casecmp(const void *, const void *);
int map_tos(char *string, int *);
-int rdomain_exists(u_int);
int filteropts_to_rule(struct pf_rule *, struct filter_opts *);
TAILQ_HEAD(loadanchorshead, loadanchors)
@@ -2475,8 +2474,6 @@ if_item : STRING {
| RDOMAIN NUMBER {
if ($2 < 0 || $2 > RT_TABLEID_MAX)
yyerror("rdomain %lld outside range", $2);
- else if (rdomain_exists($2) != 1)
- yyerror("rdomain %lld does not exist", $2);
$$ = calloc(1, sizeof(struct node_if));
if ($$ == NULL)
@@ -5865,40 +5862,6 @@ map_tos(char *s, int *val)
return (1);
}
return (0);
-}
-
-int
-rdomain_exists(u_int rdomain)
-{
- size_t len;
- struct rt_tableinfo info;
- int mib[6];
- static u_int found[RT_TABLEID_MAX+1];
-
- if (found[rdomain] == 1)
- return 1;
-
- mib[0] = CTL_NET;
- mib[1] = PF_ROUTE;
- mib[2] = 0;
- mib[3] = 0;
- mib[4] = NET_RT_TABLE;
- mib[5] = rdomain;
-
- len = sizeof(info);
- if (sysctl(mib, 6, &info, &len, NULL, 0) == -1) {
- if (errno == ENOENT) {
- /* table nonexistent */
- return 0;
- }
- err(1, "%s", __func__);
- }
- if (info.rti_domainid == rdomain) {
- found[rdomain] = 1;
- return 1;
- }
- /* rdomain is a table, but not an rdomain */
- return 0;
}
int
