Hi,
I just committed yours.
Thanks,
On Wed, 16 Sep 2020 16:07:40 +0200
Klemens Nanni <k...@openbsd.org> wrote:
> 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
