Created attachment 205343 [details] ipfilter-ipmon-usage.diff The current online usage message from ipmon(8) (ipmon -h) is really far away from the actual options that it accept. This issue can also be seen by comparing this 'usage' string and the SYNOPSIS part of its man page: > fprintf(stderr, "%s: [-NFhstvxX] [-f <logfile>]\n", prog); vs. > ipmon [ -abBDFhnpstvxX ] [ -N <device> ] [ -L <facility> ] [ -o [NSI] ] [ -O [NSI] ] [ -P <pidfile> ] [ -S <device> ] [ -f <device> ] [ <filename> ] This patch updates this usage message.
Thank you for the report and patch
Your git repo is not synced up with FreeBSD's repo. Hmm... Looks like a unified diff to me... The text leading up to this was: -------------------------- |--- old/contrib/ipfilter/tools/ipmon.c 2019-06-14 08:02:04.000000000 +0800 |+++ new/contrib/ipfilter/tools/ipmon.c 2019-06-26 13:09:51.682796000 +0800 -------------------------- Patching file contrib/ipfilter/tools/ipmon.c using Plan A... Hunk #1 failed at 1438. 1 out of 1 hunks failed while patching contrib/ipfilter/tools/ipmon.c done You can use this as a basis instead: https://github.com/cschuber/ipfilter Please rework this patch to apply or I will do it myself, but you will not get credit for the patch in the commit log message.
Created attachment 205354 [details] ipfilter-ipmon-usage.diff Well, I think may be there is a bug in either this Bugzilla or my browser. I used 'Paste the text to be added as an attachment' to post the last attachment; but the file somehow miserably been converted to CRLF line ending; I had to use dos2unix(1) and reupload it.
-S is not state input file. It can only be a device the kernel writes to, e.g. /dev/ipstate. Similarly -f, it is a device, /dev/ipl. -N, /dev/ipnat. To say these are files is incorrect. output-log-file is tautological. Why not simply log file? -O <letters> and -o <letters> is incorrect.
(In reply to Cy Schubert from comment #4) According the man page and actual source code, those input files are not need to be devices, they could be regular files. And supporting those options to changes those input files instead of the defaults (/dev/ip*) is really mean to let the user to replay a previously saved binary log, from regular files. Please see https://svnweb.freebsd.org/base/head/contrib/ipfilter/man/ipmon.8?revision=288683&view=markup#l100 and https://svnweb.freebsd.org/base/head/contrib/ipfilter/tools/ipmon.c?revision=343701&view=markup#l1769
'output-log-file' is distinguish from input log files, describes above. And, how is '-O <letters> and -o <letters>' incorrect?
(In reply to WHR from comment #5) I suppose a person could but to maintain consistency with the man page let's stay with device.
(In reply to WHR from comment #6) Saying <letters> is too generic. That's like saying -B <letters>. Technically correct because it's all alphanumeric but non-descriptive. I will use what Darren has done in other parts of ipfilter. I would also like to maintain consistency with other projects: NetBSD and illumos. Let's not change the style from the man pages. It looks inconsistent. I see you still support, in your repo, platforms and architectures that are no longer being produced, and Linux which has no interest in ipfilter. It is recommended you use what I have done in FreeBSD as a base for your work. This will make this process go smoother.
Spotted a type-o in first paragraph of DESCRIPTION: - human readable for, however, IP#'s are not mapped back to hostnames, nor are + human readable form, however, IP#'s are not mapped back to hostnames, nor are ...while you're here.
And we will add the ipmon.5 man page, currently building in my git repo and prod svn repo. We can deal with all the ipmon doc issues here, except for modernizing the nroff man page to mdoc, that's a larger project.
Created attachment 205364 [details] Untested ipmon doc updates Pending testing, this will be committed.
Passed installworld test.
A commit references this bug: Author: cy Date: Thu Jun 27 02:42:57 UTC 2019 New revision: 349448 URL: https://svnweb.freebsd.org/changeset/base/349448 Log: Fix a typo. PR/238816 initially addressed updates to usage() however it has now become a shopping list of fixes to ipmon man pages and usage(). PR: 238816 MFC after: 3 days Changes: head/contrib/ipfilter/man/ipmon.8
A commit references this bug: Author: cy Date: Thu Jun 27 02:43:27 UTC 2019 New revision: 349449 URL: https://svnweb.freebsd.org/changeset/base/349449 Log: Add the ipmon.5 man page. PR/238816 initially addressed updates to usage() however the PR has morphed into a shopping list of updates to usage() and man pages. PR: 238816 (I added to the list during discussion) MFC after: 1 week Changes: head/sbin/ipf/ipmon/Makefile head/tools/build/mk/OptionalObsoleteFiles.inc
A commit references this bug: Author: cy Date: Thu Jun 27 02:43:30 UTC 2019 New revision: 349450 URL: https://svnweb.freebsd.org/changeset/base/349450 Log: Update usage() to refect the current state of ipmon. PR: 238816 MFC after: 1 week Changes: head/contrib/ipfilter/tools/ipmon.c
(In reply to Cy Schubert from comment #10) Yes, the man page ipmon(8) needs to be updated too; for example it also leaks '-B' argumant in 'SYNOPSIS', and left '-C' completely undocumented. Keeping the online usage message sync with SYNOPSIS part in man page is a good idea; however I think using more descriptive texts like '<nat-input-file>', '<state-input-file>' and '<output-log-file>' for online usage will be more useful to end users, as I also agree that: > Saying <letters> is too generic. And of course updating the man page is also need to keep it consistent. My own fork of ipfilter is a v4 branch, because I failed to make v5 working on Solaris; the Linux support is a tryout, I don't plan to use it with Linux seriously (in fact in won't compile with recent Linux versions). '-O <letters>' is in fact copied from Solaris man page ipmon(1M): > -O letter > Specify which log files you do not wish to read from. > This is most commonly used in conjunction with the -a. > Letters available as parameters are the same as for -o. It is also in illumos: https://illumos.org/man/1m/ipmon
A commit references this bug: Author: cy Date: Thu Jun 27 03:50:13 UTC 2019 New revision: 349451 URL: https://svnweb.freebsd.org/changeset/base/349451 Log: Return a return code scripts might expect. I missed this while reviewing and rewriting a patch in PR/238816. PR: 238816 Reported by: rgrimes@ Pointy hat to: cy@ MFC after: 1 week X-MFC with: r349450 Changes: head/contrib/ipfilter/tools/ipmon.c
I should have been more careful and noticed the exit(-1) error. exit(-1) is incorrect.
(In reply to WHR from comment #16) We can't use ipfilter v4. It is a regression. It does not fully support IPv6. The illumos folks have an expectation that I port ipfilter 5.1.2 to illumos. This work will commence when my work implementing vnet0 control over vnet jail ipfilter is complete. It should work as nicely with illumos zones as it does with FreeBSD vnet.
(In reply to Cy Schubert from comment #18) Sorry I didn't notice that FreeBSD user program uses exit status 1 after printing usage message, when porting this patch from my Solaris environment. After browsing the FreeBSD source tree, I found many program simply uses 'exit(1);' in their 'void usage(void);'; but some uses 'exit(EX_USAGE);', while 'EX_USAGE' is defined in 'sysexits.h'.
A commit references this bug: Author: cy Date: Thu Jun 27 12:37:45 UTC 2019 New revision: 349452 URL: https://svnweb.freebsd.org/changeset/base/349452 Log: Create a link to the ipmon.conf.5 man page as documented in ipmon.5. Add its corresponding optional removal entry. PR: 238816 MFC after: 1 week Changes: head/sbin/ipf/ipmon/Makefile head/tools/build/mk/OptionalObsoleteFiles.inc
A commit references this bug: Author: cy Date: Fri Jun 28 04:28:33 UTC 2019 New revision: 349503 URL: https://svnweb.freebsd.org/changeset/base/349503 Log: Document the -B, binary logfile, and the -C config file options. Reference the ipmon.5 man page and ipmon.conf. PR: 238816 MFC after: 1 week Changes: head/contrib/ipfilter/man/ipmon.8
It should be .B "\-C <configfile>" not .B "\-B <configfile>" in base r349503.
A commit references this bug: Author: cy Date: Fri Jun 28 04:52:24 UTC 2019 New revision: 349504 URL: https://svnweb.freebsd.org/changeset/base/349504 Log: Fix a typo. PR: 238816 MFC after: 1 week X-MFC with: r349503 Changes: head/contrib/ipfilter/man/ipmon.8
A commit references this bug: Author: cy Date: Wed Jul 3 18:11:23 UTC 2019 New revision: 349659 URL: https://svnweb.freebsd.org/changeset/base/349659 Log: MFC r349448: Fix a typo. PR/238816 initially addressed updates to usage() however it has now become a shopping list of fixes to ipmon man pages and usage(). PR: 238816 Changes: _U stable/10/ stable/10/contrib/ipfilter/man/ipmon.8 _U stable/11/ stable/11/contrib/ipfilter/man/ipmon.8 _U stable/12/ stable/12/contrib/ipfilter/man/ipmon.8
A commit references this bug: Author: cy Date: Thu Jul 4 03:04:43 UTC 2019 New revision: 349715 URL: https://svnweb.freebsd.org/changeset/base/349715 Log: MFC r349449, r349452: Add the ipmon.5 man page and link ipmon.conf.5 to it. PR/238816 initially addressed updates to usage() however the PR has morphed into a shopping list of updates to usage() and man pages. PR: 238816 Changes: _U stable/11/ stable/11/sbin/ipf/ipmon/Makefile stable/11/tools/build/mk/OptionalObsoleteFiles.inc _U stable/12/ stable/12/sbin/ipf/ipmon/Makefile stable/12/tools/build/mk/OptionalObsoleteFiles.inc
A commit references this bug: Author: cy Date: Thu Jul 4 03:08:15 UTC 2019 New revision: 349716 URL: https://svnweb.freebsd.org/changeset/base/349716 Log: MFC r349450-349451: Update usage() to refect the current state of ipmon. PR: 238816 Changes: _U stable/11/ stable/11/contrib/ipfilter/tools/ipmon.c _U stable/12/ stable/12/contrib/ipfilter/tools/ipmon.c
A commit references this bug: Author: cy Date: Fri Jul 5 04:24:11 UTC 2019 New revision: 349759 URL: https://svnweb.freebsd.org/changeset/base/349759 Log: MFC r349503-349504: Document the -B, binary logfile, and the -C config file options. Reference the ipmon.5 man page and ipmon.conf. PR: 238816 Changes: _U stable/10/ stable/10/contrib/ipfilter/man/ipmon.8 _U stable/11/ stable/11/contrib/ipfilter/man/ipmon.8 _U stable/12/ stable/12/contrib/ipfilter/man/ipmon.8