Bug 218433 - Ipfilter ippool table handling source code or man page being incorrect.
Summary: Ipfilter ippool table handling source code or man page being incorrect.
Status: In Progress
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 11.0-RELEASE
Hardware: Any Any
: --- Affects Many People
Assignee: Cy Schubert
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-04-06 16:53 UTC by Ernie Luzar
Modified: 2019-10-04 02:12 UTC (History)
1 user (show)

See Also:


Attachments
Temporary fix for this PR. (1.27 KB, patch)
2017-04-07 01:39 UTC, Cy Schubert
no flags Details | Diff
Candidate ippool patch. (5.60 KB, patch)
2017-04-29 19:34 UTC, Cy Schubert
no flags Details | Diff
complete compile op (344.21 KB, text/plain)
2017-04-30 23:42 UTC, Ernie Luzar
no flags Details
text file listing problems (6.64 KB, text/plain)
2017-05-01 19:04 UTC, Ernie Luzar
no flags Details
ippool patch V2 (7.74 KB, patch)
2017-05-05 22:29 UTC, Cy Schubert
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ernie Luzar 2017-04-06 16:53:29 UTC
General to-do list
1. Determine whether the man page or the code is correct.
2. Verify that all arguments are parsed (and subsequently processes).
3. Verify that correct error messages are produced as appropriate.

Targeted to-do list
1. Man 5 ippool  talks about a evolving configuration syntax. The original syntax was more verbose. Remove all the source code dealing with the original verbose syntax and correct ippool(5) manual content to match. Or if in your opinion the original verbose syntax is easier to comprehend, keep it and get rid of the new syntax.   

2. ippool –R  -m table-name command has error causing core to dump. No documentation about where to find the core dump. Is it the in core table contents being dumped or the running code that is dumped? Found /etc/ippool.core file, maybe that’s the core dump file.

3. The command “ippool  -a –m table-name x.x.x.x used for adding a single node entry to the table works but “ippool –l –m table-name command does not show that just added ip address in the node list, but does show a ?(0)/32 node instead. If you try to add the node again you get message saying the node exists in the table already. Same thing occurs even if the ip address is suffixed with /32.

4. There is no documented way to dump the number of times a table ip address has been matched. The “man 8 ippool”  lists the –d  flag as a global option used for debugging the configuration file processing. Issuing “ippool -l -d –m table-name” displays all of the tables nodes with a hit count as a pair of 2 lines per node. This display needs to be condensed to a single line so it’s easier to parse through looking for the hits.

5. When the host system is shutdown or rebooted the ippool tables that were running are not restarted and the hit count is lost.

6. The “man 5 ippool” manual is mis-named. It should be named ippool.config.

7. In the  “man 5 ippool” manual it gives this example “table roll=all type=hash name=servers size=5”. What does “size=5” mean?

8. The  “man 5 ippool” manual doesn’t talk about the true syntax of the /etc/ippool.conf file.  
          pool ipf/tree (name test;) {
          1.121.136.228;
          1.186.172.218;
          1.34.169.204/32;
          101.109.155.81/16;
          104.121.89.129;
          };
	Notice the position of the left and right { }
	Notice the usage of :
	I’m thinking the ending }; is an error, should be just }

9. The maximum table size is not documented any where and/or if its possible to         increase it.

10. There is no documentation about the ip address being entered in a sorted order. Or about that ippool handles the placement of the entry in the in-core table allowing room for inserted new entry's while maintaining fast search performance. You don’t have to explain to people how this is accomplished, but you should tell them that it’s occurring automatically.

Possible enchantments. 
1. Would like to see an option that table entries get posted internally with some kind of auto expire date/time that automatically removes the entry when that timer elapses and the entry has no hits. If the entry gets a hit the timer resets and starts timer as of when the hit occurred. Maybe say 3 options, number of minutes, number of hours, and number of days. 

2. The usage of the “ippool –R  -m table-name” command is to remove the named table from running in core so it can be re-added in mass with updated content. I can all most do the same thing using this command sequence 
ippool -f /etc/ippool.conf -u 
this unloads all the entries but leaves the table name in place 
then this command reloads in mass 
ippool -f /etc/ippool.conf
Would like to see the –u unload option have option to write a file containing all the entries with their hit counts and auto expire value.
Comment 1 Cy Schubert freebsd_committer 2017-04-07 01:29:31 UTC
Removed my personal email and assigned to myself (my FreeBSD email).
Comment 2 Cy Schubert freebsd_committer 2017-04-07 01:39:08 UTC
Created attachment 181554 [details]
Temporary fix for this PR.

I've attached a temporary patch. There are other parser issues that need to be fixed in ippool.c but for now this should get you going.
Comment 3 Ernie Luzar 2017-04-13 00:22:53 UTC
Having trouble applying the patch you provided.

Did this to get the source

svnlite co svn://svn.freebsd.org/base/releng/11.0 /usr/src

Applied the patch to 
/usr/src/contrib/ipfilter/tools/ippool.c
I see ippool.c size change after the patch is run.

Then to just recompile ipfilter I 
cd /usr/src/sbin/ipf
make clean all install

Then ls -l /sbin/ippool shows the date and time of the 
module has changed but the size remains the same.

Issued "ippool -R -m table-name" and gets same results as 
reported previously

Looks like the patch is not being compiled. What am I doing wrong?
Comment 4 Cy Schubert freebsd_committer 2017-04-29 19:34:34 UTC
Created attachment 182179 [details]
Candidate ippool patch.

Sorry for the lateness of this. This patch should fix all the parsing issues in ippool.c and its man page.
Comment 5 Ernie Luzar 2017-04-29 20:31:22 UTC
I am still unable to recompile ippool. Please advise on the steps/commands to use.
Comment 6 Cy Schubert freebsd_committer 2017-04-29 21:06:22 UTC
Download and save the patch file. I'll use /tmp/ippool.diff in my example.

Below is a Readers Digest version of patch and install instructions.

cd /usr/src && patch -C -p0 < /tmp/ippool.diff && patch -p0 < /tmp/ippool.diff && cd sbin/ipf/ippool && make obj depend includes && make && make install
Comment 7 Ernie Luzar 2017-04-30 00:44:01 UTC
How do I download the patch from the PR?
Comment 8 Cy Schubert freebsd_committer 2017-04-30 02:58:18 UTC
Click on the  link called Candidate ippool patch at the top of this PR. The link is --> https://bugs.freebsd.org/bugzilla/attachment.cgi?id=182179.
Comment 9 Ernie Luzar 2017-04-30 12:26:56 UTC
Got compile error.

This is how I downloaded the source.

svnlite co svn://svn.freebsd.org/base/releng/11.0 /usr/src


/root >cd /usr/src/sbin/ipf/ippool
/usr/src/sbin/ipf/ippool >make obj depend includes
/usr/src/sbin/ipf/ippool >make

make: /usr/src/sbin/ipf/ippool/.depend, 1: ignoring stale .depend for /usr/src/sbin/ipf/libipf/libipf.a
cc -O2 -pipe -I. -I/usr/src/sbin/ipf/ippool/../../../contrib/ipfilter -I/usr/src/sbin/ipf/ippool/../../../contrib/ipfilter/tools -I/usr/src/sbin/ipf/ippool/../../../sys -I/usr/src/sbin/ipf/ippool/../../../sys/contrib/ipfilter -DSTATETOP -D__UIO_EXPOSE -DUSE_INET6 -g -std=gnu99 -fstack-protector-strong -Wsystem-headers -Werror -Wall -Wno-format-y2k -Wno-uninitialized -Wno-pointer-sign -Wno-format -Wno-empty-body -Wno-string-plus-int -Wno-unused-const-variable -Wno-tautological-compare -Wno-unused-value -Wno-parentheses-equality -Wno-unused-function -Wno-enum-conversion -Wno-unused-local-typedef -Wno-switch -Wno-switch-enum -Wno-knr-promoted-parameter -Wno-array-bounds -Qunused-arguments  -o ippool.full ippool_y.o ippool_l.o kmem.o ippool.o  -L/usr/obj/usr/src/sbin/ipf/libipf -lipf  -lkvm
/usr/bin/ld: cannot find -lipf
cc: error: linker command failed with exit code 1 (use -v to see invocation)
*** Error code 1

Stop.
make: stopped in /usr/src/sbin/ipf/ippool
/usr/src/sbin/ipf/ippool >
Comment 10 Cy Schubert freebsd_committer 2017-04-30 19:16:52 UTC
Follow the same instructions but from ippool's parent directory. Then cd to the ippool directory and make install from there.
Comment 11 Ernie Luzar 2017-04-30 23:42:39 UTC
Created attachment 182201 [details]
complete compile op

still having trouble with compile. Read my comments at end of the listing.
Comment 12 Cy Schubert freebsd_committer 2017-05-01 03:02:34 UTC
install(1) strips the binary of symbols, that's why it's smaller. You will see this by running the file command.

file /sbin/ippool* /usr/obj

See below:

slippy$ ls -l /sbin/ippool /usr/obj/opt/src/svn-current/sbin/ipf/ippool/ippool
-r-xr-xr-x  1 root  wheel  108040 Apr 29 20:29 /sbin/ippool
-rwxr-xr-x  1 root  wheel  119040 Apr 29 15:35 /usr/obj/opt/src/svn-current/sbin/ipf/ippool/ippool
slippy$ file /sbin/ippool /usr/obj/opt/src/svn-current/sbin/ipf/ippool/ippool
/sbin/ippool:                                        ELF 64-bit LSB executable, x86-64, version 1 (FreeBSD), dynamically linked, interpreter /libexec/ld-elf.so.1, for FreeBSD 12.0 (1200023), FreeBSD-style, stripped
/usr/obj/opt/src/svn-current/sbin/ipf/ippool/ippool: ELF 64-bit LSB executable, x86-64, version 1 (FreeBSD), dynamically linked, interpreter /libexec/ld-elf.so.1, for FreeBSD 12.0 (1200023), FreeBSD-style, not stripped
slippy$ 

The installed ippool is stripped. The one in /usr/obj is not.

What does stripping do? It makes the binary smaller at the expense of the loss of any symbols that may help with debugging. See below.

slippy$ nm /sbin/ippool
nm: /sbin/ippool: no symbols
slippy$ nm /usr/obj/opt/src/svn-current/sbin/ipf/ippool/ippool
000000000061ad40 B _CurrentRuneLocale@@FBSD_1.0
00000000006177b0 d _DYNAMIC
0000000000619c60 B _DefaultRuneLocale@@FBSD_1.0
0000000000617968 d _GLOBAL_OFFSET_TABLE_
                 w _Jv_RegisterClasses
                 U _ThreadRuneLocale@@FBSD_1.3
0000000000617790 d __CTOR_END__
0000000000617788 d __CTOR_LIST__
00000000006177a0 d __DTOR_END__
0000000000617798 d __DTOR_LIST__
0000000000417780 r __FRAME_END__
00000000006177a8 d __JCR_END__
00000000006177a8 d __JCR_LIST__
0000000000619c10 A __bss_start
000000000040d670 t __do_global_ctors_aux
0000000000401fd0 t __do_global_dtors_aux
0000000000617bc8 d __do_global_dtors_aux.completed
0000000000617bc0 d __do_global_dtors_aux.p
0000000000617bb8 D __dso_handle
                 U __error@@FBSD_1.0
0000000000617784 d __fini_array_end
0000000000617784 d __fini_array_start
                 U __inet_addr@@FBSD_1.0
                 U __inet_aton@@FBSD_1.0
                 U __inet_ntoa@@FBSD_1.0
                 U __inet_ntop@@FBSD_1.0
                 U __inet_pton@@FBSD_1.0
0000000000617784 d __init_array_end
0000000000617784 d __init_array_start
000000000061ace0 B __isthreaded@@FBSD_1.0
0000000000619c40 B __mb_sb_limit@@FBSD_1.0
0000000000617784 d __preinit_array_end
0000000000617784 d __preinit_array_start
0000000000617bb0 D __progname
                 U __stack_chk_fail@@FBSD_1.0
000000000061ad00 B __stack_chk_guard@@FBSD_1.0
000000000061ad48 B __stderrp@@FBSD_1.0
0000000000619c38 B __stdinp@@FBSD_1.0
0000000000619c20 B __stdoutp@@FBSD_1.0
                 U __swbuf@@FBSD_1.0
0000000000619c10 A _edata
0000000000625e90 A _end
000000000040d6a8 T _fini
0000000000401940 T _init
                 U _init_tls@@FBSD_1.0
0000000000401dc0 T _start
0000000000400218 r abitag
                 U abort@@FBSD_1.0
0000000000403cc0 t add_htablehosts
000000000040abd0 T alist_free
000000000040a8f0 T alist_new
000000000040a890 T assigndefined
                 U atexit@@FBSD_1.0
                 U atoi@@FBSD_1.0
                 U bcmp@@FBSD_1.0
                 U bcopy@@FBSD_1.0
000000000040c060 T bcopywrap
                 U calloc@@FBSD_1.0
                 U close@@FBSD_1.0
                 U connect@@FBSD_1.0
000000000040a790 T connecttcp
000000000040d5d0 T count4bits
000000000040d530 T count6bits
0000000000400230 r crt_noinit_tag
000000000061bbf0 B environ
                 U exit@@FBSD_1.0
000000000040ade0 t expand_string
000000000040d500 T familyname
                 U fclose@@FBSD_1.0
0000000000617db8 D fd
                 U feof@@FBSD_1.0
                 U fgetc@@FBSD_1.0
                 U fgets@@FBSD_1.0
000000000040cf90 T fill6bits
0000000000401f50 t finalizer
000000000040a740 T findword
                 U fopen@@FBSD_1.0
                 U fprintf@@FBSD_1.0
0000000000402010 t frame_dummy
                 U free@@FBSD_1.0
                 U freeaddrinfo@@FBSD_1.0
                 U fwrite@@FBSD_1.0
000000000040ac00 T get_variable
                 U getaddrinfo@@FBSD_1.0
                 U getenv@@FBSD_1.0
000000000040bf10 T gethost
                 U gethostbyname@@FBSD_1.0
                 U gethostname@@FBSD_1.0
                 U getnetbyname@@FBSD_1.0
                 U getopt@@FBSD_1.0
00000000004074b0 T getrole
00000000004074d0 T gettype
000000000040d650 T initparse
                 U ioctl@@FBSD_1.0
0000000000617dd0 d ipf_errors
000000000040bd80 T ipf_geterror
000000000061bb50 b ipf_geterror.text
000000000040bb60 T ipf_perror
000000000040bbb0 T ipf_perror_fd
000000000040be70 T ipf_strerror
000000000061bba0 b ipf_strerror.text
000000000040bc80 T ipferror
000000000061ade8 b ipht
000000000061b9b0 b iphte
000000000061ba48 b ipld
000000000061ad88 b iplo
0000000000402040 T ippool_parsefile
0000000000402120 T ippool_parsesome
000000000061baf4 B ippool_yybreakondot
000000000061bc2c B ippool_yychar
0000000000623c90 B ippool_yychars
000000000040eb00 r ippool_yycheck
000000000061bc64 B ippool_yydebug
000000000040d9b0 r ippool_yydefred
000000000040fd90 r ippool_yydgoto
000000000061baec B ippool_yydictfixed
000000000061bc60 B ippool_yyerrflag
0000000000405a90 T ippool_yyerror
000000000061baf0 B ippool_yyexpectaddr
00000000004058b0 t ippool_yygetc
000000000040fce0 r ippool_yygindex
000000000061bc70 B ippool_yyin
0000000000405a20 T ippool_yykeytostr
0000000000617db4 D ippool_yylast
000000000040f9c0 r ippool_yylen
0000000000403dd0 T ippool_yylex
000000000061bb0c b ippool_yylex.prior
000000000061bb10 b ippool_yylex.priornum
000000000040fb50 r ippool_yylhs
0000000000617db0 D ippool_yylineNum
000000000061bc30 B ippool_yylval
000000000040dd00 r ippool_yyname
000000000061bc28 B ippool_yynerrs
00000000004021d0 T ippool_yyparse
000000000061bae8 B ippool_yypos
00000000004059b0 T ippool_yyresetdict
000000000040f360 r ippool_yyrindex
000000000040f6b0 r ippool_yyrule
000000000061bb08 B ippool_yysavedepth
0000000000625ca0 B ippool_yysavewords
0000000000405e30 T ippool_yysetdict
0000000000405d90 T ippool_yysetfixeddict
0000000000405a70 T ippool_yysettab
000000000040e7b0 r ippool_yysindex
000000000061ad58 b ippool_yystack
000000000061bae0 B ippool_yystr
000000000040ef30 r ippool_yytable
000000000061bc80 B ippool_yytext
000000000061bafc B ippool_yytokentype
000000000061bbf8 B ippool_yyval
000000000061baf8 B ippool_yyvarnext
0000000000617bd0 d ippool_yywords
000000000061bb00 B ippool_yywordtab
0000000000405f00 T kmemcpy
000000000040a720 T kmemcpywrap
0000000000405fe0 T kstrncpy
000000000061bb18 b kvm_f
                 U kvm_open
                 U kvm_read
000000000040a520 T load_dstlist
000000000040b9c0 T load_dstlistnode
000000000040a1a0 T load_file
0000000000409ec0 T load_hash
000000000040b810 T load_hashnode
0000000000409940 T load_http
0000000000409750 T load_pool
000000000040b650 T load_poolnode
00000000004096f0 T load_url
0000000000406a00 T loadpoolfile
00000000004061c0 T main
                 U malloc@@FBSD_1.0
                 U memchr@@FBSD_1.0
                 U memcpy@@FBSD_1.0
                 U memmove@@FBSD_1.0
                 U memset@@FBSD_1.0
                 U nlist@@FBSD_1.0
000000000061bb30 B nohdrfields
000000000040cf20 T ntomask
                 U open@@FBSD_1.0
0000000000405ec0 T openkmem
000000000061acf0 B optarg@@FBSD_1.0
0000000000619c28 B optind@@FBSD_1.0
0000000000619c30 B optreset@@FBSD_1.0
000000000061bb20 B opts
0000000000409580 T parsefields
00000000004092d0 T parsewhoisline
                 U perror@@FBSD_1.0
000000000040cef0 T pool_close
000000000040cf10 T pool_fd
000000000061bb28 B pool_fields
000000000040ced0 T pool_ioctl
000000000040ce90 T pool_open
0000000000406680 T poolcommand
0000000000619b80 d poolfd
0000000000619b90 D poolfields
0000000000406b00 T poolflush
000000000061ad50 b poolioctl
0000000000406d50 T poollist
0000000000407db0 T poollist_dead
0000000000407560 T poollist_live
000000000061bad0 b poolname
00000000004062a0 T poolnodecommand
0000000000407000 T poolstats
000000000061bb40 B pos
0000000000409060 T printdstl_live
000000000040b4a0 T printdstlist
000000000040cd10 T printdstlistdata
000000000040ca70 T printdstlistnode
000000000040d4c0 T printdstlistpolicy
                 U printf@@FBSD_1.0
000000000040b300 T printhash
0000000000408db0 T printhash_live
000000000040c680 T printhashdata
000000000040c440 T printhashnode
000000000040d3d0 T printip
000000000040d370 T printmask
000000000040b0c0 T printpool
0000000000408b40 T printpool_live
000000000040c210 T printpooldata
000000000040d090 T printpoolfield
000000000040c070 T printpoolnode
000000000040d020 T printunit
                 U putc@@FBSD_1.0
                 U putchar@@FBSD_1.0
                 U puts@@FBSD_1.0
                 U read@@FBSD_1.0
                 U realloc@@FBSD_1.0
                 U reallocarray@@FBSD_1.4
0000000000408a20 T remove_hash
00000000004088a0 T remove_hashnode
0000000000408790 T remove_pool
0000000000408640 T remove_poolnode
0000000000408600 T resetlexer
000000000040afb0 T set_variable
0000000000407390 T setnodeaddr
00000000004084f0 T showdstls_live
00000000004083f0 T showhashs_live
00000000004082b0 T showpools_live
                 U snprintf@@FBSD_1.0
                 U socket@@FBSD_1.0
                 U sprintf@@FBSD_1.0
                 U sscanf@@FBSD_1.0
                 U strcasecmp@@FBSD_1.0
                 U strcat@@FBSD_1.0
                 U strchr@@FBSD_1.0
                 U strcmp@@FBSD_1.0
                 U strcpy@@FBSD_1.0
                 U strdup@@FBSD_1.0
                 U strerror@@FBSD_1.0
0000000000617dc8 D string_end
0000000000617dc0 D string_start
000000000061bb38 B string_val
                 U strlen@@FBSD_1.0
                 U strncmp@@FBSD_1.0
                 U strncpy@@FBSD_1.0
                 U strtok@@FBSD_1.0
0000000000625d90 B thishost
                 U ungetc@@FBSD_1.0
00000000004060c0 T usage
000000000061bb24 B use_inet6
000000000061bb48 b vtop
                 U warnx@@FBSD_1.0
                 U write@@FBSD_1.0
slippy$
Comment 13 Ernie Luzar 2017-05-01 19:04:58 UTC
Created attachment 182231 [details]
text file listing problems

Attachment file contains list of 9 outstanding problems that need addressing.
Comment 14 Cy Schubert freebsd_committer 2017-05-05 22:29:49 UTC
Created attachment 182323 [details]
ippool patch V2

Problem 1:

Can you post a dump, please. I'm not able to reproduce the dump on my -CURRENT systems here.

Yes, only addresses (for now) can be entered. However there is code to
use names (DNS or otherwise). I've reverted any changes that use
this code until it can be sorted out.


Problem 2:

-t <type> is not optional, it's part of the key. This has been fixed.


Problem 3:

ippool -l [ -t <type> ] [ -o <role> ] [ -m <name> ] is more correct.

However getopts() doesn't recognize options that the switch statement
below it does. (Frankly, the parser in ippool is quite a mess.)

For now use the above syntax with the patch I will attach. I won't
commit it yet because there's a lot of work yet to do to clean this up.


Problem 4:

Once again, ippool is quite a mess. I've fixed a lot and a lot more needs
fixing. There are a lot more problems you haven't identified yet.


Problem 5:

This has been fixed in my latest patch.


Problem 6:

ippool -a is totally broken. The problem is that it fails to recognize
IPv4/IPv6 addresses. In doing so it does not set the address family
(IPv4, IPv6) in in a structure. This is my current focus and will take
quite a bit of work. (Sorry this is taking so long. Day $JOB not
direclty related to FreeBSD at the moment.)


Problem 7:

This works in my latest patch now. -o & -t may be used. You can have
the same named pools in different roles and different tables. This
works now, though it doesn't work for entries added using the -a option
becuase -a fails to assign an address family to entries. (Working on
this.)


Problem 8:

I've yet to address this. Having seid that, I have replaced my
fwbuilder built rules with ippool-based rules. It's currently working.


Problem 9:

This will need to be a long-term project for now. There is an IPv6
checksum issue in ipfilter that needs to be resolved before I tackle
documentation. I can't recall the PR number but they have a patch
for now while I clean it up for commit. (I'm not happy with my patch
yet.)


Final comments:

Please try the attached patch. There are still a lot of problems and issues that I have yet to address and am still working on, not the least of which poolnodecommand() -- the -a option -- is totally useless because it fails to parse IP addresses correctly because it doesn't take into account IPv6 addresses. Furthermore I don't think anyone has ever tested ippool with IPv6 yet, not that I know of in FreeBSD, NetBSD, or Solaris.

My goal here is to get enough of ippool working to go back and commit the IPv6 checksum issue in the kernel (PR 203275) before 11.1 and 10.4 are released. This is pretty much a show stopper for anyone using IPv6.

Hope this helps.

Please send me that core dump and uname -a with it. I'm specifically interested in the SVN revision number.

Also, in the future, please reply in the PR instead of attaching a file. A person casually viewing this PR can kind-of lose context unless they clue into downloading a copy of attachment or viewing it in a separate browser tab. Thanks.

~cy
Comment 15 Ernie Luzar 2017-05-07 00:04:52 UTC
*************
Ernie 5/1/17;  
Problem #1. 
"man 5 ippool" says only ip addresses can be used as table content. 
Now its not worded like that but this meaning is implied. 
No where does it say fqdn can be used.

So reading "man 8 ippool" we see the "option" -R as in
ippool -l [-dvR] [-m <name>] [-t <type>]
Which is defined as "-R     Do not resolve IP addresses to hostnames.

This just doesn't make sense. I can not find am situation where any output
to the terminal defaults to showing the hostnames. So why have this option
at all? I would think this option can be removed from "man 8 ippool" 
all together.

Cy 5/5/17
Yes, only addresses (for now) can be entered. However there is code to
use names (DNS or otherwise). I've reverted any changes that use
this code until it can be sorted out.

Ernie 5/6/17
Verified the R option has been removed from the man 8 ippool.

*****************

Ernie 5/1/17;
Problem #2. 
  ippool -R [-dnvR] [-m <name>] [-o <role>] [-t <type>]
  -R     Remove an existing pool from within the kernel.

Issuing "ippool -R -m table-name" results in the help syntax
being shown. Issuing "ippool -R -m table-name -t tree" does remove the
named table. 

A table name has to be unique across all table types. 
So requiring the -t flag in identifying the table wanted to remove is 
unneeded. The ippool source needs to be changed to remove this requirement.
The "man 8 ippool" should show this as the syntax.
  ippool -R [-dnv] [-m <name>] 

Cy 5/5/17:
-t <type> is not optional, it's part of the key. This has been fixed.

Ernie 5/6/17
No this has not been fixed. -t <type> is still a requirement. It should not
be required based on the fact table names are unique. Look at code logic for
"ippool -l -m table-name" it finds the correct table just based on the table
name. That same code needs to be incorporated into the -R option so the
-t <type> is not required.
 
*********************

Ernie 5/1/17;
Problem #3.
  ippool -l [-dvR] [-m <name>] [-t <type>]
 
Issuing "ippool -l" lists the contents of all the in-core tables.
Issuing "ippool -l -m table-name" lists the contents of just the named table.
Issuing "ippool -l -t tree" lists the contents of all the in-core tables.
It looks like the [-t <type>] is not being used and logically makes no sense.
The "man 8 ippool" should show this as the syntax.
  ippool -l [-dv] [-m <name>]

Issuing this following command outputs a report containing the number 
of matches for each ip address in the named table.
ippool -l -d  -m test
poollist: opts = 0x2
Name: test Role: ipf    References: 2   Hits: 0
        Nodes Starting at 0xca312d00
        Address: 1.161.177.165/32
                Hits 0  Bytes 0 Name    Ref 2
        Address: 1.171.138.231/32
                Hits 0  Bytes 0 Name    Ref 2
        Address: 110.200.28.224/32
                Hits 0  Bytes 0 Name    Ref 2
        Address: 110.200.29.248/32
                Hits 0  Bytes 0 Name    Ref 2
        Address: 162.243.114.84/32

The -d flag is defined as 
" -d     Toggle debugging of processing the configuration file."
I don't see any debug info being generated here. What I do see is a valuable
report. This should not be happening under the pretence of debugging. 
This report should have its own flag, like -h and the report should be changed
to a single line containing the table ip address node plus number of hits.

Cy 5/5/17:
ippool -l [ -t <type> ] [ -o <role> ] [ -m <name> ] is more correct.

However getopts() doesn't recognize options that the switch statement
below it does. (Frankly, the parser in ippool is quite a mess.)

For now use the above syntax with the patch I attached. I won't
commit it yet because there's a lot of work yet to do to clean this up.

Ernie 5/6/17
man 8 ippool shows this
ippool -l [-dv] [-m <name>] [-t <type>]
it should show this
ippool -l [-dv] [-m <name>] 

ippool -l -t <type>  Does list content of all in-core tables that match
that table type. But since list is in wrap around mode its very hard to read.
There is no logical reason for this option. Looks like an left over option
for some future function that was never implemented.

Remove all code from the -l option that has anything to do with -t or -o
ippool -l  list content of all in-core tables
ippool -l [-m <name>] list content of named in-core table
This keeps things simple.

*********************

Ernie 5/1/17;
Problem #4.
  ippool -F [-dv] [-o <role>] [-t <type>]
  -F     Flush loaded pools from the kernel.

Why is [-dv] missing the n option which means
       -n     This flag (no-change) prevents ippool from actually making any
              ioctl calls or doing anything which would alter the currently
              running kernel.

Reading this I take it to mean that issuing "ippool -F -o or -t" is saying 
to flush ALL tables matching the -o or -t value. This doesn't make sense.
The -F option in the ippool source needs to be changed removing any logic for 
the -o & -t option processing and add -m logic to select the named table only.
The -n option logic also needs to be added.

The "man 8 ippool" should show this as the syntax.
  ippool -F [-dvn] [-m <name>]
and the " -F     Flush loaded pools from the kernel." changed to
" -F Flush loaded pool content from the kernel, leaving only empty pool name."

Cy 5/5/17:
Once again, ippool is quite a mess. I've fixed a lot and a lot more needs
fixing. There are a lot more problems that you haven't identified yet.

Ernie 5/6/17
Lets focus on the documented problems.
man 8 ippool shows this
ippool -F [-dv] [-o <role>] [-t <type>]
Issuing "ippool -F -m table-name" gets a option error msg and 
the same thing for
"ippool -F -n"
This change has not been done yet. The  removal of the -o & -t flags from
the source has not happened yet. The addition of the -m table-name flag
to the source has not been done yet. These changes have not been made to 
"man 8 ippool".

***************************

Ernie 5/1/17;
Problem #5.
  ippool -A [-dnvR] [-m <name>] [-o <role>] [-S <seed>] [-t <type>]
  -A     Add a new (empty) pool to the kernel.

Issuing "ippool -A -m test2 -o ipf -t tree"  results in
ippool: illegal option -- A

The -A option in the ippool source needs to be corrected. The -o and -t options
are required

Cy 5/5/17:
This has been fixed in my latest patch.

Ernie 5/6/17
Tested this issuing this command
"ippool -A -m test1 -t tree"  worked and
"ippool -f /etc/ippool.conf.test1" to load the contents also worked
"ippool -A -m test2 -t hash  worked but issued this msg
WARNING: empty hash table test3, recommend setting size to match expected use
This would seem to indicate that a size flag should be required.
"ippool -l -m test2"  shows
table role=ipf type=hash name=test2 size=1
        {; };
"ippool -f /etc/ippool.conf.test2" gets this error.
30017:add lookup hash table could not find table to remove node from table
This is defiantly bogus and mis-leading.

The "ippool -A" command still needs work.

*******************

Ernie 5/1/17;
Problem #6.
  ippool -a [-dnvR] [-m <name>] [-o <role>] [-t <type>] [-T ttl] -i
       <ipaddr>[/<netmask>]
  -a     Add a new data node to an existing pool in the kernel.

The -o & -t options are not needed to identify the target table to 
add the node to. 
Issuing "ippool -a -m test 99.58.98.79" gives no errors, but issuing
"ippool -l -m test" afterwards lists the content showing it containing
?(0)?/32; which is incorrect. 
This needs detail inspection of the ippool source to correct this.

The "man 8 ippool" should show this as the syntax.
  ippool -a [-dnv] [-m <name>] [-T ttl] -i <ipaddr>[/<netmask>]

Cy 5/5/17:
ippool -a is totally broken. The problem is that it fails to recognize
IPv4/IPv6 addresses. In doing so it does not set the address family
(IPv4, IPv6) in a structure. This is my current focus and will take
quite a bit of work. (Sorry this is taking so long. Day $JOB not
directly related to FreeBSD at the moment.)

Ernie 5/6/17
Lets focus on getting IPv4 working so the ?(0)?/32; problem can be addresses.
Trying to do IPv6 in ippool now is diluting the effort to much.
Lets make IPv6 a future ippool update, so we can get the basic flags to work
correctly.  
There must be IPv4 code in the "ippool -f" command that is handling this 
correctly. That code can be copied or performed for this -a flag. 

*************************

Ernie 5/1/17;
Problem #7.
  ippool -r [-dnvR] [-m <name>] [-o <role>] [-t <type>] -i <ipaddr>[/<netmask>]
  -r     Remove an existing data node from a pool in the kernel.

The -o & -t options are not needed to identify the target table to
remove the node from.
Issuing "ippool -r -m test 99.58.98.79" gives no errors, but issuing
"ippool -l -m test" afterwards lists the content showing the ?(0)?/32; 
is no longer there.
This needs detail inspection of the ippool source to correct this.

The "man 8 ippool" should show this as the syntax.
  ippool -r [-dnv] [-m <name>] -i <ipaddr>[/<netmask>]

Cy 5/5/17:
This works in my latest patch now. -o & -t may be used. You can have
the same named pools in different roles and different tables. This
works now, though it doesn't work for entries added using the -a option
because -a fails to assign an address family to entries. (Working on
this.)

Ernie 5/6/17
Having the same named pools in different roles and different tables should 
not be allowed. This breaks the unique table name idea we are working with.
The  -o & -t code needs to be removed. 

Tested by issuing "ippool -r -m test1 -i 106.75.81.110" worked, test1 is a 
tree type table and test2 was a hash type table and that worked also.

********************

Ernie 5/1/17;
Problem #8.
During the boot process these messages are issued

IP Filter: v5.1.2 initialized.  Default = pass all, Logging = enabled
Enabling ipfilter.
132:194:ioctl(add/insert rule) cannot find source lookup pool
Installing NAT rules.
0 entries flushed from NAT table
0 entries flushed from NAT list

In my ipfilter rule set I have this rule;
block in quick from pool/probing_ips to any

Rebooting IE: reboot, halt, shutdown, commands or power off button; 
causes the table hit count to be lost and no table content to be reloaded 
into core. This really is a major problem.

Have to add some kind of way to tell ipfilter to auto dump the in-core table 
with it's hit count information and auto restore that dumped information into
core when the system is booted.

Maybe adding an option flag to rc.conf ipfilter_flags="" statement defaulting
to internally issuing "ippool -f /etc/ippool.conf" command or playing with
the rc.d scripts is needed here.

Whatever solution gets employed to solve this problem needs to be 
documented in "man 5 ippool"

Cy 5/5/17:
I've yet to address this. Having said that, I have replaced my
fwbuilder built rules with ippool-based rules. It's currently working.

Ernie 5/6/17
This is not totally true. I see in the current patch-v2 you have added some
new defaults to etc/defaults/rc.conf file which are listed below;
+ippool_enable="NO"             # Set to YES to enable ip filter pools
+ippool_program="/sbin/ippool"  # where the ippool program lives
+ippool_rules="/etc/ippool.rules"       # pool definition file for ippool
+ippool_flags=""                        # additional flags for ippool

The statement ippool_rules="/etc/ippool.rules"  is not correct. It should
be ippool_rules="/etc/ippool.conf" because that is the default ippool 
configuration file documented in the man page.

You also added a completely new file etc/rc.d/ippool file that is executed 
at boot time to load the contents of /etc/ippool.conf into the system core. 
This is a good first step, but does not address existing in-core table hit
count being unloaded and saved to a file so it can be used to re-populate the
in-core table at boot time. 

When the shutdown command is issued there has to be a process to save the 
in-core table info. This 2nd part can come later after all the flags 
processing get corrected.

For whatever reason this patch has not propagated to /etc/rc.d/ippool and
/etc/defaults/rc.conf file, so I manually moved /usr/src/etc/rc.d/ippool to
/etc/rc.d/ippool and added 
ippool_enable="NO"
ippool_program="/sbin/ippool"  
ippool_rules="/etc/ippool.conf"  
ippool_flags=""     
to my system /etc/defaults/rc.conf file and then  added ippool_enable="YES" 
to my /etc/rc.conf file.

I did a "shutdown -r now" command and during the boot I got there messages
IP Filter: v5.1.2 initialized.  Default = pass all, Logging = enabled
Enabling ipfilter.
132:194:ioctl(add/insert rule) cannot find source lookup pool
Installing NAT rules.
0 entries flushed from NAT table
0 entries flushed from NAT list

After logging in I did "ippool -s" and there were no tables loaded in-core.
A "ippool -f /etc/ippool.conf" command worked and "ippool -s" showed the table
was loaded in-core. This is telling me that the /etc/ippool.conf file is good
and that most likely /etc/rc.d/ippool was never executed at boot time. 
There were no script errors in the boot messages.

This first part of Problem #8 still needs work.   

*********************

Ernie 5/6/17
Problem #9.
"man 5 ippool" needs to specify the usage of { } and ; in the ippool.conf file
content like shown here;

pool ipf/tree (name test;) {
1.161.177.165;
1.171.138.231;
110.200.28.224;
110.200.29.248;
162.243.114.84;
162.243.225.157;
88.211.91.195;
88.226.134.147;
99.58.98.78;
99.98.160.77;
};

Cy 5/5/17:
This will need to be a long-term project for now. There is an IPv6
checksum issue in ipfilter that needs to be resolved before I tackle
documentation. I can't recall the PR number but there is a patch
for it that I have to clean up for I can commit it. (I'm not happy with
my patch yet.)

Ernie 5/6/17
This is only documentation to be added to "man 5 ippool". There is nothing
wrong in /sbin/ippool that needs correction to solve this problem.

************* 

Ernie 5/6/17
Problem #10.
man 8 ippool shows this
ippool -s [-dtv] [-M <core>] [-N <namelist>]

The [-dtv] should be [-dv]
This is just a documentation change.
************

Your patch-v1 to ippool did correct the
"ippool -R -m table_name" core dump problem.

The patch-v2 includes this fix and continues to work.


I think both you and I can agree on the fact that /sbin/ippool has never worked
since day one. That it was written by some one else besides Darren Reed. 
That the coding style is a can of worms which looks more like a 
"concept proto-type" than a finished production product. 
I think the KISS programming method should by employed here. 
I can tell from the ippool.c patch that you are just adding new code around 
existing crap instead of deleting or at least commenting out crap logic code. 
It's time to take a wide brush to the source and remove all the 
"concept proto-type" code dealing with functions not implemented yet.

Everything should be based off of the following requirements.
1. All tables have to have unique table names.
2. All code references to -t or -o flags should be removed. 
3. All code that deals with converting fqdn to ip address should be removed
   from the source.
4. Any code that deals with processing input coming from the whois command 
   should also be removed from the source. whois as input is talked about 
   in "man 5.ippool" 
5. Limit table handling to IPv4 address for now so we can get a working 
   /sbin/ippool. Adding IPv6 can happen later after Ipv4 is a working.
6. Limit boot time table handling to just auto populating in-core tables 
   with definition content from the /etc/ippool.conf file. Adding support
   for capturing the in-core table with its hits count at "shutdown" and 
   the reloading that saved information back into the core at boot time can
   wait for phase 2 when IPv6 function is added.

Now you are the man with his finger in the source. What I have out lined 
above is just suggestions. I respect whatever direction you choose to take.

Take note, my use of ippool is not headed to production. I am using ippool 
to identify the offending IPv4 address that are probing my public ip address
over time. I have a working system based on the few ipool commands that do
function correctly. I am volunteering my time to identify and test the
ippool command giving you feedback on what works and what doesn't. So 
other words take your time and fix IPv6 in ipfilter, I am in no rush here.

As we get the basic set of command flags to function correctly using 
"tree & hash" table types, then I plan to test the other "role types" 
and table types until everything is working correctly.
Comment 16 commit-hook freebsd_committer 2017-05-11 04:39:42 UTC
A commit references this bug:

Author: cy
Date: Thu May 11 04:39:11 UTC 2017
New revision: 318173
URL: https://svnweb.freebsd.org/changeset/base/318173

Log:
  Implement outputting of IPv6 addresses in the ippool debug list of tree
  type pools (ippool -l -d -t tree). Currently IPv6 in ippool tree type
  pool handling is partially implemented (meaning it doesn't work).
  This is the first of a series of commits to remediate ippool.

  This will be MFCed with a yet to be committed series of fixes to ippool
  after it has been fully remediated.

  PR:		218433

Changes:
  head/contrib/ipfilter/lib/printpoolnode.c
Comment 17 Cy Schubert freebsd_committer 2017-05-13 06:31:45 UTC
Problem #2. 
The names are not unique. They are unique within a type.

slippy# ippool -l -m test
table role=ipf type=tree name=test
	{ 10.1.1.91/32; fc00:1:1:1::5b/128; 10.1.2.91/32; 10.1.1.1/32; 10.1.2.1/32; 10.1.1.7/32; 10.1.2.7/32; 10.1.1.254/32; 10.1.2.254/32; };
table role=ipf type=hash name=test size=31
	{ 10.1.1.91/32; fc00:1:1:1::5b/128; 10.1.2.91/32; fc00:1:1:2::5b/128; 10.1.1.1/32; fc00:1:1:1::1/128; 10.1.2.1/32; fc00:1:1:2::1/128; 10.1.1.7/32; fc00:1:1:1::7/128; 10.1.2.7/32; fc00:1:1:2::7/128; 10.1.1.254/32; fc00:1:1:1::fffe/128; 10.1.2.254/32; fc00:1:1:2::fffe/128; 192.168.56.0/24; 10.2.2.0/24; };
slippy# 

slippy# ippool -l -t tree
table role=ipf type=tree name=test3
	{ fc00:1:1:1::5b/128; };
table role=ipf type=tree name=test2
	{ 10.255.0.1/32; 10.255.0.2/32; };
table role=ipf type=tree name=test
	{ 10.1.1.91/32; fc00:1:1:1::5b/128; 10.1.2.91/32; 10.1.1.1/32; 10.1.2.1/32; 10.1.1.7/32; 10.1.2.7/32; 10.1.1.254/32; 10.1.2.254/32; };
table role=ipf type=tree name=vitp
	{ 10.255.0.1/32; 10.255.0.2/32; };
table role=ipf type=tree name=slippy
	{ 10.1.1.91/32; 10.1.2.91/32; 10.1.3.91/32; };
table role=ipf type=tree name=trusted
	{ 10.1.1.0/24; 10.1.2.0/24; 10.1.3.0/24; 10.2.2.1/32; 10.2.2.3/32; 10.2.2.4/32; 10.2.2.6/32; };
slippy# 

slippy# ippool -l -t hash
table role=ipf type=hash name=test size=31
	{ 10.1.1.91/32; fc00:1:1:1::5b/128; 10.1.2.91/32; fc00:1:1:2::5b/128; 10.1.1.1/32; fc00:1:1:1::1/128; 10.1.2.1/32; fc00:1:1:2::1/128; 10.1.1.7/32; fc00:1:1:1::7/128; 10.1.2.7/32; fc00:1:1:2::7/128; 10.1.1.254/32; fc00:1:1:1::fffe/128; 10.1.2.254/32; fc00:1:1:2::fffe/128; 192.168.56.0/24; 10.2.2.0/24; };
slippy# 


Problem #3.

-t can be either tree or hash.


Problem #4.

ippool -F -m is invalid. That is why it is not documented as such.
You should use ippool -R -m -t instead.


Problem #5.

Agreed. My latest patch, that I'm still testing here, has fixed this.
More testing is require before I can post it here.


Problem #8.

This is a long one. Hopefully I don't miss a point.

Why would you want to reload the counters? That doesn't make sense.

Saving the counters to a logfile or syslog is possible though that would be
inconsistent with ipfw and pf in FreeBSD. I prefer a global solution that
includes all three. (Which BTW, it is possible to run all three in the same
kernel simultaneously.)

ippool.rules is consistent with ipf.rules. This is a FreeBSD standard adopted
a good number of years ago. Darren Reed, the author of ipfilter preferred
ipf.conf, ipnat.conf and ippool.conf. When ipfilter was imported into FreeBSD
about 20 years ago it was decided to use .rules instead. To change the default
now would be a POLA violation. To change the man page might not be a good idea
either as many old-timers (like me) are used to it thus changing it is also
a POLA violation. The default names of the the files (there are three of them)
will not change.


Problem #9.

The man page is adaquate however describing the general syntax of the file
would help.


Problem #10.

1. I don't agree that all table names should have unique names. Users are
   free to name their pools as they desire. Additionally I would like to
   maintain as much compatibility with NetBSD as possible. (I'll send them
   patches when I'm done.)

2. I disagree.

3. There is value to being able to resolve names, just as other firewalls
   such as pf and ipfw do. It's also consistent with the ipf command.

4. I'm not sure if parsing whois output ever worked. It may have at one time.
   I need to look at it first before I decide whether to remove it.

5. Agreed. Going down this rabbit hole, IPv6 isn't supported by radix_ipf.

6. There is no reason to reload stats and hit counts. In the context of
   ipfs(8) maybe but then all stats, not just pool stats, would need to
   be reloaded.


I'll post my next patch when it's ready.
Comment 18 Ernie Luzar 2017-05-14 15:19:29 UTC
Problems 2, 3, and 4 are ALL caused by the same problem. That being the –t flag is being appended to the table name to make a unique identifier. You have now found and documented one of the major design flaws in ippool. This should be simple to remove the appending of the  –t flag value to make the table name alone to be unique.  This change is NOT a POLA violation because ippool has never worked from day one. 

Problem 5, 6 and 7. The –t flag also has to be removed.


About saving the table hits at shutdown and reloading them at boot time. I consider the hits info just as useful and valuable as the “state table” contents. There is a firewall subsystem to save the “state” table contents at shutdown and the auto restore of it at boot time. 

I have revised my data acquisition model to no longer use ippool tables.
Comment 19 commit-hook freebsd_committer 2017-05-15 03:40:34 UTC
A commit references this bug:

Author: cy
Date: Mon May 15 03:39:36 UTC 2017
New revision: 318284
URL: https://svnweb.freebsd.org/changeset/base/318284

Log:
  Just like r318173, which was for outputting IPv6 addresses in tree
  pools, implement outputting of IPv6 addresses in the ippool debug list
  of hash type pools (ippool -l -d -t hash). Currently IPv6 in ippool tree
  type pool handling is mostly implemented.
  This continues theseries of commits to remediate ippool.

  This will be MFCed with a yet to be committed series of fixes to ippool
  after it has been fully remediated.

  PR:		218433

Changes:
  head/contrib/ipfilter/lib/printhashnode.c
Comment 20 commit-hook freebsd_committer 2017-05-16 02:48:59 UTC
A commit references this bug:

Author: cy
Date: Tue May 16 02:48:46 UTC 2017
New revision: 318333
URL: https://svnweb.freebsd.org/changeset/base/318333

Log:
  Implement ippool command line IPv6 address parse support (for the -i
  option).

  PR:		218433

Changes:
  head/contrib/ipfilter/tools/ippool.c
Comment 21 Cy Schubert freebsd_committer 2018-05-28 19:41:39 UTC
Not fixed yet. IPv6 still broken. Partial fix in my local tree. radix_ipf needs a rewrite.
Comment 22 commit-hook freebsd_committer 2019-03-22 01:31:48 UTC
A commit references this bug:

Author: cy
Date: Fri Mar 22 01:30:52 UTC 2019
New revision: 345400
URL: https://svnweb.freebsd.org/changeset/base/345400

Log:
  Add rc.d support for ippool(8).

  I've been using ippool at my site for approximately two years. It's
  about time this was committed.

  PR:		218433
  MFC after:	2 weeks

Changes:
  head/libexec/rc/rc.conf
  head/libexec/rc/rc.d/ippool
Comment 23 commit-hook freebsd_committer 2019-03-22 01:42:59 UTC
A commit references this bug:

Author: cy
Date: Fri Mar 22 01:42:27 UTC 2019
New revision: 345401
URL: https://svnweb.freebsd.org/changeset/base/345401

Log:
  From r345400, connect ippool to the build/install.

  PR:		218433
  MFC after:	2 weeks
  X-MFC with:	r345400

Changes:
  head/libexec/rc/rc.d/Makefile
Comment 24 commit-hook freebsd_committer 2019-03-22 01:44:02 UTC
A commit references this bug:

Author: cy
Date: Fri Mar 22 01:43:55 UTC 2019
New revision: 345403
URL: https://svnweb.freebsd.org/changeset/base/345403

Log:
  From r345400, remove the ippool rc script when ipfilter is not wanted
  by the user.

  PR:		218433
  MFC after:	2 weeks
  X-MFC with:	r345400

Changes:
  head/tools/build/mk/OptionalObsoleteFiles.inc
Comment 25 commit-hook freebsd_committer 2019-04-05 01:22:38 UTC
A commit references this bug:

Author: cy
Date: Fri Apr  5 01:22:31 UTC 2019
New revision: 345899
URL: https://svnweb.freebsd.org/changeset/base/345899

Log:
  MFC r345400-345401,345403,345412,345437:

  Add rc.d support for ippool(8).

  I've been using ippool at my site for approximately two years. It's
  about time this was committed.

  PR:		218433

Changes:
_U  stable/12/
  stable/12/libexec/rc/rc.conf
  stable/12/libexec/rc/rc.d/Makefile
  stable/12/libexec/rc/rc.d/ippool
  stable/12/tools/build/mk/OptionalObsoleteFiles.inc
Comment 26 Cy Schubert freebsd_committer 2019-06-13 03:23:13 UTC
Still much more to be done. radix_ipf needs a fair bit of love and attention.
Comment 27 commit-hook freebsd_committer 2019-09-27 00:29:40 UTC
A commit references this bug:

Author: cy
Date: Fri Sep 27 00:29:06 UTC 2019
New revision: 352784
URL: https://svnweb.freebsd.org/changeset/base/352784

Log:
  Sync with source:

  Only a role of "ipf" is currentlysupported as the other documented
  (and undocumented) roles are #ifdef'd out.

  The plan is to complete ippool(8) as it is even in its current state
  a powerful feature/tool.

  PR:		218433
  MFC after:	1 month

Changes:
  head/contrib/ipfilter/man/ippool.8
Comment 28 commit-hook freebsd_committer 2019-09-27 00:30:17 UTC
A commit references this bug:

Author: cy
Date: Fri Sep 27 00:29:10 UTC 2019
New revision: 352785
URL: https://svnweb.freebsd.org/changeset/base/352785

Log:
  The no resolve (OPT_NORESOLVE) does nothing. Additionally, it (-R)
  conflicts with the command option of the same name (also -R).
  Remove the superfluous and confusing non-global non-command -R option.

  PR:		218433
  MFC after:	1 week

Changes:
  head/contrib/ipfilter/tools/ippool.c
Comment 29 commit-hook freebsd_committer 2019-09-27 00:30:20 UTC
A commit references this bug:

Author: cy
Date: Fri Sep 27 00:29:13 UTC 2019
New revision: 352786
URL: https://svnweb.freebsd.org/changeset/base/352786

Log:
  Implement the dynamic add (-A) and removal (-R) of ippool pools
  from the command line. Prior to this the functionality was mostly there
  however since the pool type (-t) was not recognized by the -A and -R
  command options -- not recognized by getopt(). Additionally the code to
  implement the dynamic add and removal of pools didn't work.

  When dynamically adding (-A) a pool a type (-t) to specify if the pool
  is a tree or hash pool must  be specified. When dynamically removing (-R)
  a pool, omitting -t will cause a search-and-destroy which will remove
  both types of pools matching the name given (-m).

  PR:		218433
  MFC after:	1 week

Changes:
  head/contrib/ipfilter/man/ippool.8
  head/contrib/ipfilter/tools/ippool.c
Comment 30 commit-hook freebsd_committer 2019-10-04 02:10:42 UTC
A commit references this bug:

Author: cy
Date: Fri Oct  4 02:09:52 UTC 2019
New revision: 353093
URL: https://svnweb.freebsd.org/changeset/base/353093

Log:
  MFC r352785:

  The no resolve (OPT_NORESOLVE) does nothing. Additionally, it (-R)
  conflicts with the command option of the same name (also -R).
  Remove the superfluous and confusing non-global non-command -R option.

  PR:		218433

Changes:
_U  stable/11/
  stable/11/contrib/ipfilter/tools/ippool.c
_U  stable/12/
  stable/12/contrib/ipfilter/tools/ippool.c
Comment 31 commit-hook freebsd_committer 2019-10-04 02:12:43 UTC
A commit references this bug:

Author: cy
Date: Fri Oct  4 02:11:46 UTC 2019
New revision: 353094
URL: https://svnweb.freebsd.org/changeset/base/353094

Log:
  MFC r352786:

  Implement the dynamic add (-A) and removal (-R) of ippool pools
  from the command line. Prior to this the functionality was mostly there
  however since the pool type (-t) was not recognized by the -A and -R
  command options -- not recognized by getopt(). Additionally the code to
  implement the dynamic add and removal of pools didn't work.

  When dynamically adding (-A) a pool a type (-t) to specify if the pool
  is a tree or hash pool must  be specified. When dynamically removing (-R)
  a pool, omitting -t will cause a search-and-destroy which will remove
  both types of pools matching the name given (-m).

  PR:		218433

Changes:
_U  stable/11/
  stable/11/contrib/ipfilter/man/ippool.8
  stable/11/contrib/ipfilter/tools/ippool.c
_U  stable/12/
  stable/12/contrib/ipfilter/man/ippool.8
  stable/12/contrib/ipfilter/tools/ippool.c