Re: pfctl always prints warning when flushes ruleset

2017-09-26 Thread Alexandr Nedvedicky
Hello Mike,

> 
> Please make sure a pfctl regress doesn't run into issues with this
> diff.  Otherwise OK mikeb.
> 

the regress/src/sbin/pfctl did not spot any issues. will commit my
change later today.


thanks a lot
regards
sasha



Re: pfctl always prints warning when flushes ruleset

2017-09-26 Thread Mike Belopuhov
On Tue, Sep 26, 2017 at 11:15 +0200, Alexandr Nedvedicky wrote:
> Hello,
> 
> few users on Solaris don't like to read warning 'Anchor or Ruleset' does not
> exist:
> 
> # echo 'pass' |pfctl -a foo -f -
> # pfctl -a foo -Fa
> rules cleared
> pfctl: Anchor or Ruleset does not exist.
> # 
> 
> the commands above did work well, the 'pfctl: Anchor ...' warning message
> is kind of invalid. The code path, which ends up with warning, starts in
> pfct main() function:
> 
> 2518 case 'a':
> 2519 pfctl_clear_rules(dev, opts, anchorname);
> 2520 pfctl_clear_tables(anchorname, opts);
> 2521 if (ifaceopt && *ifaceopt) {
> 2522 warnx("don't specify an interface with 
> -Fall");
> 2523 usage();
> 2524 /* NOTREACHED */
> 2525 }
> 
> we call pfctl_clear_rules(), which flushes all rules from anchor. The anchor
> becomes empty. If there are no tables attached to anchor, then
> pf_remove_if_empty_ruleset() called on behalf of DIOCXCOMMIT also removes the
> anchor from table. No wonder the pfctl_clear_tables() invoked later at line 
> 2520
> can not find anchor, which it is searching table to be flushed.
> 
> The patch below just swaps line 2519 and 2520 to put the operations to correct
> order. With patch in place the warning is gone.
> 
> # echo 'pass' |pfctl -a foo -f -
> # pfctl -a foo -Fa
> 0 tables deleted.
> rules cleared
> # 
> 
> Also pfctl still prints warning in expected case:
> # pfctl -sA
> # pfctl -a foo -FT
> pfctl: Anchor or Ruleset does not exist.
> #
> 
> OK?
>

Please make sure a pfctl regress doesn't run into issues with this
diff.  Otherwise OK mikeb.

> thanks and
> regards
> sasha
> 
> 8<---8<---8<--8<
> diff -r 215db23c6b05 src/sbin/pfctl/pfctl.c
> --- src/sbin/pfctl/pfctl.c  Mon Sep 25 13:38:48 2017 +0200
> +++ src/sbin/pfctl/pfctl.c  Tue Sep 26 11:15:26 2017 +0200
> @@ -2516,8 +2516,8 @@ main(int argc, char *argv[])
> pfctl_clear_stats(dev, ifaceopt, opts);
> break;
> case 'a':
> +   pfctl_clear_tables(anchorname, opts);
> pfctl_clear_rules(dev, opts, anchorname);
> -   pfctl_clear_tables(anchorname, opts);
> if (ifaceopt && *ifaceopt) {
> warnx("don't specify an interface with 
> -Fall");
> usage();
> 8<---8<---8<--8<
> 
> 
> 
> 
> 



pfctl always prints warning when flushes ruleset

2017-09-26 Thread Alexandr Nedvedicky
Hello,

few users on Solaris don't like to read warning 'Anchor or Ruleset' does not
exist:

# echo 'pass' |pfctl -a foo -f -
# pfctl -a foo -Fa
rules cleared
pfctl: Anchor or Ruleset does not exist.
# 

the commands above did work well, the 'pfctl: Anchor ...' warning message
is kind of invalid. The code path, which ends up with warning, starts in
pfct main() function:

2518 case 'a':
2519 pfctl_clear_rules(dev, opts, anchorname);
2520 pfctl_clear_tables(anchorname, opts);
2521 if (ifaceopt && *ifaceopt) {
2522 warnx("don't specify an interface with 
-Fall");
2523 usage();
2524 /* NOTREACHED */
2525 }

we call pfctl_clear_rules(), which flushes all rules from anchor. The anchor
becomes empty. If there are no tables attached to anchor, then
pf_remove_if_empty_ruleset() called on behalf of DIOCXCOMMIT also removes the
anchor from table. No wonder the pfctl_clear_tables() invoked later at line 2520
can not find anchor, which it is searching table to be flushed.

The patch below just swaps line 2519 and 2520 to put the operations to correct
order. With patch in place the warning is gone.

# echo 'pass' |pfctl -a foo -f -
# pfctl -a foo -Fa
0 tables deleted.
rules cleared
# 

Also pfctl still prints warning in expected case:
# pfctl -sA
# pfctl -a foo -FT
pfctl: Anchor or Ruleset does not exist.
#

OK?

thanks and
regards
sasha

8<---8<---8<--8<
diff -r 215db23c6b05 src/sbin/pfctl/pfctl.c
--- src/sbin/pfctl/pfctl.c  Mon Sep 25 13:38:48 2017 +0200
+++ src/sbin/pfctl/pfctl.c  Tue Sep 26 11:15:26 2017 +0200
@@ -2516,8 +2516,8 @@ main(int argc, char *argv[])
pfctl_clear_stats(dev, ifaceopt, opts);
break;
case 'a':
+   pfctl_clear_tables(anchorname, opts);
pfctl_clear_rules(dev, opts, anchorname);
-   pfctl_clear_tables(anchorname, opts);
if (ifaceopt && *ifaceopt) {
warnx("don't specify an interface with -Fall");
usage();
8<---8<---8<--8<