PERFORCE change 167260 for review

[Julian Elischer]

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.
> 
>>
>> I cannot recall that we ever discussed or planned to be able to mix 
>> virtualized with non-virtualized protocols in the same kernel.  That 
>> would be a horrible mess, and I cannot even imagine having say a 
>> multi-instance INET with a single-instance INET6 kernel, shared among 
>> all the vnets.  To start with, how would you decide that you're not 
>> allowed to process an IPv6 packet received on the wire in a 
>> non-default vnet in such an environment?  Do we have the 
>> infrastructure in place necessary for preventing doing say a ifconfig 
>> lo0 ::1 in a non-default vnet in such an hypotetical setup?  The 
>> answer is no.
>>
> 
> 
> I agree that it is horrible and we have not said that it will all work
> 
>> VNET_SYSINIT is nice, but proper special-casing changes required to 
>> support single-instance protocols to work only with vnet0 and not with 
>> the other protocols are simply not there, and I hope will never be, 
>> because I fear they would be highly intrusive, difficult to verify and 
>> maintain, and probably also have an impact on performance.
>>
>> A proper solution for the issue you are raising could be something 
>> that would prevent modules assuming our stack is compiled as 
>> single-instance to be kldloaded if the kernel was actually built with 
>> multi-instance stack support.  I think Robert (cc-ed) had some ideas 
>> on how to accomplish this by having such modules depend on a magic 
>> global variable (say __no_vnet_support) to be available.
>>
>> All the current "base" protocols are already using pr_init() in 
>> multi-instance mode in options VIMAGE case.  So I see no reason for 
>> ip_divert not being allowed to leverage on the same mechanism.
>>
>> Re. pr_destroy(), you're right, patch already submitted to p4...

But pr_destroy is not called from pf_proto_unregister()
(a bug I think.) so I notice that you call it yourself,
  but only on one vnet.

with the code I had you could load and unload divert when there were 
jails present or not.

it would do the right thing.

robert's code was specifically set up to avoid calling the proto_init
function on each as far as I could see.



>>
>> Marko