svn commit: r272515 - projects/ipfw/sys/netpfil/ipfw

[John Baldwin]

On Sunday, October 12, 2014 12:13:00 AM Alexander V. Chernikov wrote:
> On 10 Oct 2014, at 01:11, John Baldwin <jhb at freebsd.org> wrote:
> > On Thursday, October 09, 2014 12:17:50 pm Alexander V. Chernikov wrote:
> >> On 06.10.2014 19:45, John Baldwin wrote:
> >>> On Saturday, October 04, 2014 12:10:33 PM Alexander V. Chernikov wrote:
> >>>> Author: melifaro
> >>>> Date: Sat Oct  4 12:10:32 2014
> >>>> New Revision: 272515
> >>>> URL: https://svnweb.freebsd.org/changeset/base/272515
> >>>> 
> >>>> Log:
> >>>>   Add "ipfw_ctl3" FEATURE to indicate presence of new ipfw interface.
> >>>> 
> >>>> Modified:
> >>>>   projects/ipfw/sys/netpfil/ipfw/ip_fw2.c
> >>>> 
> >>>> Modified: projects/ipfw/sys/netpfil/ipfw/ip_fw2.c
> >>>> =======================================================================
> >>>> ===== == --- projects/ipfw/sys/netpfil/ipfw/ip_fw2.c	Sat Oct  4
> >>>> 11:40:35 2014	(r272514) +++
> >>>> projects/ipfw/sys/netpfil/ipfw/ip_fw2.c	Sat Oct  4 12:10:32
> >>>> 2014	(r272515) @@ -2874,6 +2874,7 @@ static moduledata_t ipfwmod = {
> >>>> 
> >>>>  #define	IPFW_VNET_ORDER		(IPFW_MODEVENT_ORDER + 2) /* Later still. 
*/
> >>>>  
> >>>>  DECLARE_MODULE(ipfw, ipfwmod, IPFW_SI_SUB_FIREWALL,
> >>>>  IPFW_MODEVENT_ORDER);
> >>>> 
> >>>> +FEATURE(ipfw_ctl3, "ipfw new sockopt calls");
> >>>> 
> >>>>  MODULE_VERSION(ipfw, 2);
> >>>>  /* should declare some dependencies here */
> >>> 
> >>> Would it be better to bump the module version to 3 instead?  Userland
> >>> programs can then use modfind() and modstat() to determine the version.
> >> 
> >> I've bumped ipfw module version in r272828. Actually, I've entirely
> >> forgotten about this possibility.
> >> However, it is a bit hard to determine module version inside
> >> (perl|python|sh|any) script.
> >> On the other case, FEATURE framework provides nice and easy way to
> >> determine any "feature" status
> >> both in C and interpreted programs.
> > 
> > I'll grant you that feature is convenient.  Perhaps create a SYSCTL node
> > though that holds the current version?  That is 'foo.ipfw.version' being
> > 2 or 3 is more future proof than 'feature.ipfw2/3/4’.
> 
> No, this is not about new _ipfw_ version. I’m unsure if all these changes
> are large enough to name ipfw as “ipfw3”. This is just an indication that
> all ipfw-related sockopts are available via single setsockopt called
> IP_FW3. Maybe naming is not the best - I’m open to any suggestion.

Hmm, it sure seems like a new version in that there is a different style of 
interface similar to how umtx changed from discrete system calls 
(umtx_lock/unlock) to a multiplexer (umtx_op).

> However, I’m not sure why should I invent additional sysctls instead of
> using standard interface.

Well, I think using FEATURE() to communicate version numbers is not really
its intended application.  That said, if you wanted, another option would be
to possibly rename the socket options to something like 'IP_FW_OP' and
'IP_DUMMYNET_OP' (to reflect that they take an operation as an argument
similar to umtx_op).  Also, if what you care about is 'xtable' support, you
could have 'FEATURE(ipfw_xtable)'.  That is more along the lines of how
FEATURE() is currently used rather than version numbers.

> > Alternatively, we could change the module code to export a dynamic sysctl
> > tree for all loaded modules that includes the versions, i.e.
> > 'module.<foo>.version', etc.

This is still another idea that would transparently export MODULE_VERSION()
info via sysctls without requiring API changes.

-- 
John Baldwin