[crossbow-discuss] Design review of IP Instances part of Crossbow

[Erik Nordmark]

Darren.Reed at sun.com wrote:
> Erik,
> 
> Looking over the IPFilter changes, a couple of them don't appear to be
> what I would consider correct.
> 
> Firstly, for each proxy (ip_*pxy.c) .c file that has had variables brought
> into the ipf_stack_t, the correct architecture should be to have a stack
> instance structure for the proxy .c file and to allocate this when the
> proxy's _init() function is called.  Some of the fields currently in the
> per stack instance thing are initialised, are never changed and have
> no visibility (ifs_ftppxyfr, ifs_h323_fr, ifs_ipsecfr, ifs_netbiosfr,
> ifs_raudiofr, ifs_rcmdfr.)  Going with that, there are accompanying
> _init variables that may also not need to be per-instance.

I think you are suggesting that each proxy header file declare an 
instance structure, and then the proxy's apr_init gets to allocate and 
initialize a new instance of that structure.

Question is how ipf would then identify the instance and what it would 
pass to functions like apr_new.
One possibility is to have apr_init return a pointer to the instance 
structure it passed in, and have ip_proxy.c keep that as a void pointer 
for the calls to the other apr_functions.
Did you have something different in mind?

> Related to this are ifs_ap_proxylist and ifs_ap_proxies.  The array,
> ifs_ap_proxies is staticly configured data that is compiled in and is
> never changed.  The variable ifs_ap_proxylist is for proxies that are
> loaded externally using a dedicated LKM.

ifs_ap_proxies and ifs_ap_proxylist are almost static - apr_ref is 
modified. Are you suggesting we should keep apr_ref in a separate 
list/array?

> Looking at the IPFilter variables, ifs_fr_refcnt doesn't need this.
> Actually, looking at its presence today, it can probably be rm'd.

ifs_fr_refcnt doesn't seem to be needed in snv_49 any more.

We will audit all the foo_stack_t structures for fields that are no 
longer referenced at all; the structures have been built based on the 
global variables that existed whether or not they were used.


> There's one other area of concern - the radix.c file.
> 
> Given this, the design document is based on a pre-surya kernel?

Yes.
I've discussed with Sowmini how to handle radix.c (we didn't merge with 
a post-Surya kernel until last week.) Seems like at least mask_rnhead 
should be per instance.

    Erik