On Wed, Sep 16, 2020 at 07:49:19PM +0900, YASUOKA Masahiko wrote:
> New diff is using -1 for ENOENT.
>
> Also domainid == 0 is a valid domain id, but previous diff cannot make
> a cache of it since 0 is the default value. So new diff is doing
>
> - static u_int found[RT_TABLEID_MAX+1];
> + static struct {
> + int found;
> + int domainid;
> + } rtables[RT_TABLEID_MAX+1];
>
> to distinguish the default 0 and domainid 0.
This looks more complicated than it needs to be, but I also don't want
to bikeshed it; given that the parser is happy with this and we plan to
remove this code alltogether anyway in the next release cycle: OK kn.
Alternatively, here's a much simpler diff resembling what I had in mind.
Feel free to commit this instead (with my OK), give me an OK for it or
go ahead with yours.
It uses the same function and reflects the fact that every rdomain is a
rtable but not every rtable is also a rdomain (your choice of `domainid'
seems inconsistent with that).
Index: parse.y
===================================================================
RCS file: /cvs/src/sbin/pfctl/parse.y,v
retrieving revision 1.701
diff -u -p -r1.701 parse.y
--- parse.y 28 Jan 2020 15:40:35 -0000 1.701
+++ parse.y 16 Sep 2020 13:58:23 -0000
@@ -392,7 +392,7 @@ 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 lookup_rtable(u_int);
int filteropts_to_rule(struct pf_rule *, struct filter_opts *);
TAILQ_HEAD(loadanchorshead, loadanchors)
@@ -1216,6 +1216,9 @@ antispoof_opt : LABEL label {
if ($2 < 0 || $2 > RT_TABLEID_MAX) {
yyerror("invalid rtable id");
YYERROR;
+ } else if (lookup_rtable($2) >= 1) {
+ yyerror("rtable %lld does not exist", $2);
+ YYERROR;
}
antispoof_opts.rtableid = $2;
}
@@ -2000,6 +2003,9 @@ filter_opt : USER uids {
if ($2 < 0 || $2 > RT_TABLEID_MAX) {
yyerror("invalid rtable id");
YYERROR;
+ } else if (lookup_rtable($2) >= 1) {
+ yyerror("rtable %lld does not exist", $2);
+ YYERROR;
}
filter_opts.rtableid = $2;
}
@@ -2475,7 +2481,7 @@ if_item : STRING {
| RDOMAIN NUMBER {
if ($2 < 0 || $2 > RT_TABLEID_MAX)
yyerror("rdomain %lld outside range", $2);
- else if (rdomain_exists($2) != 1)
+ else if (lookup_rtable($2) != 2)
yyerror("rdomain %lld does not exist", $2);
$$ = calloc(1, sizeof(struct node_if));
@@ -5868,37 +5874,38 @@ map_tos(char *s, int *val)
}
int
-rdomain_exists(u_int rdomain)
+lookup_rtable(u_int rtableid)
{
size_t len;
struct rt_tableinfo info;
int mib[6];
static u_int found[RT_TABLEID_MAX+1];
- if (found[rdomain] == 1)
- return 1;
+ if (found[rtableid])
+ return found[rtableid];
mib[0] = CTL_NET;
mib[1] = PF_ROUTE;
mib[2] = 0;
mib[3] = 0;
mib[4] = NET_RT_TABLE;
- mib[5] = rdomain;
+ mib[5] = rtableid;
len = sizeof(info);
if (sysctl(mib, 6, &info, &len, NULL, 0) == -1) {
if (errno == ENOENT) {
/* table nonexistent */
+ found[rtableid] = 0;
return 0;
}
err(1, "%s", __func__);
}
- if (info.rti_domainid == rdomain) {
- found[rdomain] = 1;
- return 1;
+ if (info.rti_domainid == rtableid) {
+ found[rtableid] = 2;
+ return 2;
}
- /* rdomain is a table, but not an rdomain */
- return 0;
+ found[rtableid] = 1;
+ return 1;
}
int
