PERFORCE change 167260 for review

[Julian Elischer]

Marko Zec wrote:
> On Thursday 13 August 2009 02:22:22 Julian Elischer wrote:
>> Marko Zec wrote:
>>> On Thursday 13 August 2009 00:44:49 Julian Elischer wrote:
>>>> Marko Zec wrote:
>>>>> On Wednesday 12 August 2009 23:58:46 Julian Elischer wrote:
>>>>>> Marko Zec wrote:
>>>>> ...
>>>>>
>>>>>>> @@ -710,22 +715,36 @@
>>>>>>>  	.pr_input =		div_input,
>>>>>>>  	.pr_ctlinput =		div_ctlinput,
>>>>>>>  	.pr_ctloutput =		ip_ctloutput,
>>>>>>> -	.pr_init =		NULL,
>>>>>>> +	.pr_init =		div_init,
>>>>>>>  	.pr_usrreqs =		&div_usrreqs
>>>>>> If you are going to make pr_init() called for every vnet then
>>>>>> pr_destroy should be as well. But in fact that is not really safe.
>>>>>> (either of them)
>>>>>>
>>>>>> The trouble is that we can not guarantee that other protocols can
>>>>>> handle being called multiple times in their init and destroy methods.
>>>>>> Especially 3rd party protocols.
>>>>>>
>>>>>> We need to ensure only protocols that have been converted to run
>>>>>> with multiple vnets are ever called with multiple vnets.
>>>>>>
>>>>>> for this reason the only safe way to do this is via the VNET_SYSINIT
>>>>>> and VNET_SYSUNINIT calls.
>>>>> That would mean you would have to convert most if not all of the
>>>>> existing things that hang off of protosw-s in netinet, netinet6 etc. to
>>>>> use VNET_SYSINT / VNET_SYSUNIT instead of protosw->pr_init().  So the
>>>>> short answer is no.
>>>> robert has done just that.
>>> hmm:
>>>
>>> tpx32% pwd
>>> /u/marko/svn/head/sys
>>>
>>> tpx32% fgrep -R .pr_init netinet netinet6 netipsec|fgrep -v .svn
>>> netinet/ip_divert.c:    .pr_init =              div_init,
>>> netinet/in_proto.c:     .pr_init =              ip_init,
>>> netinet/in_proto.c:     .pr_init =              udp_init,
>>> netinet/in_proto.c:     .pr_init =              tcp_init,
>>> netinet/in_proto.c:        .pr_init =   sctp_init,
>>> netinet/in_proto.c:     .pr_init =              icmp_init,
>>> netinet/in_proto.c:     .pr_init =              encap_init,
>>> netinet/in_proto.c:     .pr_init =              encap_init,
>>> netinet/in_proto.c:     .pr_init =              encap_init,
>>> netinet/in_proto.c:     .pr_init =              encap_init,
>>> netinet/in_proto.c:     .pr_init =              encap_init,
>>> netinet/in_proto.c:     .pr_init =              rip_init,
>>> netinet6/in6_proto.c:   .pr_init =              ip6_init,
>>> netinet6/in6_proto.c:   .pr_init =              tcp_init,
>>> netinet6/in6_proto.c:   .pr_init =              icmp6_init,
>>> netinet6/in6_proto.c:   .pr_init =              encap_init,
>>> netinet6/in6_proto.c:   .pr_init =              encap_init,
>>> netinet6/ip6_mroute.c:  .pr_init =              pim6_init,
>>> netipsec/keysock.c:     .pr_init =              raw_init,
>> AND for example:
>> in ./netinet/in_proto.c
>> VNET_DOMAIN_SET(inet);
>> includes
>> VNET_SYSINIT   ##### --> called for every vnet as created ####
>>     calls
>> vnet_domain_init()
>>     calls
>> domain_init()
>>     calls
>> protosw_init()
>>     which includes
>>               if (pr->pr_init)
>>                  (*pr->pr_init)();
>>
>> so, robert is calling the init routine from each protocol
>> not the modevent.
> 
> Right.
> 
> But when we kldload ipdivert and register its protosw via pf_proto_register(), 
> VNET_DOMAIN_SET() will not call ipdivert's pr_init() routine for each 
> existing vnet, hence pf_proto_register() will have to do this.


yes I figured out that the distinguishing feature is whether you
are loading a domain or a protocol.


> 
> For subsequently created vnets, i.e. after ipdivert has been kldloaded, 
> pr_init() will indeed be called via VNET_DOMAIN_SET() mechanism on each vnet 
> instantiation.  But at that point in time pf_proto_register() will not be 
> called.  So, in both cases pr_init() will be called exactly once per vnet, 
> but via different mechanisms.

well it's right, but I'm glad we went over it..
and I still think what I did in divert was better :-)


> 
> Marko