I agree with this. Others to consider:
- 'tag': we could then replace the nasty 'tagged' keyword with 'tag' and
do proper 'tag != FOO' syntax.
- synproxy and modulate state - could go into 'scrub' also?
On Sat, Jul 07, 2012 at 07:24:23PM +0200, Henning Brauer wrote:
> so, we have some utter confusion in pf about filter criteria versus
> packet modifying options. I propose we move the ones that "write" into
> a set block, while the filter criteria remain as they are. for the
> moment this diff handles tos (I always disliked set-tos...) and prio.
> rdomain/rtable stuff should be done the same way (afterwards).
> no backwards compat for prio because i clearly stated it's not the
> final syntax all the time.
>
> no manpage bits yet.
>
> "match set { prio 6, tos lowdelay }"
> "match set prio 6"
>
> Index: sbin/pfctl/parse.y
> ===================================================================
> RCS file: /cvs/src/sbin/pfctl/parse.y,v
> retrieving revision 1.614
> diff -u -p -r1.614 parse.y
> --- sbin/pfctl/parse.y 7 Jul 2012 16:24:32 -0000 1.614
> +++ sbin/pfctl/parse.y 7 Jul 2012 17:09:19 -0000
> @@ -508,6 +508,7 @@ int parseport(char *, struct range *r, i
> %type <v.hfsc_opts> hfscopts_list hfscopts_item hfsc_opts
> %type <v.queue_bwspec> bandwidth
> %type <v.filter_opts> filter_opts filter_opt filter_opts_l
> +%type <v.filter_opts> filter_sets filter_set filter_sets_l
> %type <v.antispoof_opts> antispoof_opts antispoof_opt
> antispoof_opts_l
> %type <v.queue_opts> queue_opts queue_opt queue_opts_l
> %type <v.scrub_opts> scrub_opts scrub_opt scrub_opts_l
> @@ -979,7 +980,7 @@ scrub_opt : NODF {
> scrub_opts.marker |= FOM_MAXMSS;
> scrub_opts.maxmss = $2;
> }
> - | SETTOS tos {
> + | SETTOS tos { /* XXX remove in 5.4-current */
> if (scrub_opts.marker & FOM_SETTOS) {
> yyerror("set-tos cannot be respecified");
> YYERROR;
> @@ -2379,7 +2380,21 @@ filter_opt : USER uids {
> }
> filter_opts.rcv = $2;
> }
> - | prio {
> + | ONCE {
> + filter_opts.marker |= FOM_ONCE;
> + }
> + | filter_sets
> + ;
> +
> +filter_sets : SET '{' filter_sets_l '}' { $$ = filter_opts; }
> + | SET filter_set { $$ = filter_opts; }
> + ;
> +
> +filter_sets_l : filter_sets_l comma filter_set
> + | filter_set
> + ;
> +
> +filter_set : prio {
> if (filter_opts.marker & FOM_SETPRIO) {
> yyerror("prio cannot be redefined");
> YYERROR;
> @@ -2388,8 +2403,13 @@ filter_opt : USER uids {
> filter_opts.set_prio[0] = $1.b1;
> filter_opts.set_prio[1] = $1.b2;
> }
> - | ONCE {
> - filter_opts.marker |= FOM_ONCE;
> + | TOS tos {
> + if (filter_opts.marker & FOM_SETTOS) {
> + yyerror("tos cannot be respecified");
> + YYERROR;
> + }
> + filter_opts.marker |= FOM_SETTOS;
> + filter_opts.settos = $2;
> }
> ;
>
> Index: sbin/pfctl/pfctl_parser.c
> ===================================================================
> RCS file: /cvs/src/sbin/pfctl/pfctl_parser.c,v
> retrieving revision 1.285
> diff -u -p -r1.285 pfctl_parser.c
> --- sbin/pfctl/pfctl_parser.c 7 Jul 2012 16:24:32 -0000 1.285
> +++ sbin/pfctl/pfctl_parser.c 7 Jul 2012 17:08:31 -0000
> @@ -843,6 +843,25 @@ print_rule(struct pf_rule *r, const char
> if (r->tos)
> printf(" tos 0x%2.2x", r->tos);
>
> + if (r->set_prio[0] != PF_PRIO_NOTSET ||
> + r->scrub_flags & PFSTATE_SETTOS) {
> + char *comma = "";
> + printf(" set {");
> + if (r->set_prio[0] != PF_PRIO_NOTSET) {
> + if (r->set_prio[0] == r->set_prio[1])
> + printf("%s prio %u", comma, r->set_prio[0]);
> + else
> + printf("%s prio(%u, %u)", comma, r->set_prio[0],
> + r->set_prio[1]);
> + comma = ",";
> + }
> + if (r->scrub_flags & PFSTATE_SETTOS) {
> + printf("%s tos 0x%2.2x", comma, r->set_tos);
> + comma = ",";
> + }
> + printf(" }");
> + }
> +
> ropts = 0;
> if (r->max_states || r->max_src_nodes || r->max_src_states)
> ropts = 1;
> @@ -998,12 +1017,6 @@ print_rule(struct pf_rule *r, const char
> printf("min-ttl %d", r->min_ttl);
> ropts = 0;
> }
> - if (r->scrub_flags & PFSTATE_SETTOS) {
> - if (!ropts)
> - printf(" ");
> - printf("set-tos 0x%2.2x", r->set_tos);
> - ropts = 0;
> - }
> if (r->scrub_flags & PFSTATE_SCRUB_TCP) {
> if (!ropts)
> printf(" ");
> @@ -1088,12 +1101,6 @@ print_rule(struct pf_rule *r, const char
> printf(" dup-to");
> printf(" ");
> print_pool(&r->route, 0, 0, r->af, PF_POOL_ROUTE, verbose);
> - }
> - if (r->set_prio[0] != PF_PRIO_NOTSET) {
> - if (r->set_prio[0] == r->set_prio[1])
> - printf(" prio %u", r->set_prio[0]);
> - else
> - printf(" prio(%u, %u)", r->set_prio[0], r->set_prio[1]);
> }
> }
