Date: Wed, 12 Aug 2009 17:22:22 -0700 From: Julian Elischer <julian@elischer.org> To: Marko Zec <zec@freebsd.org> Cc: Perforce Change Reviews <perforce@freebsd.org>, Robert Watson <rwatson@freebsd.org> Subject: Re: PERFORCE change 167260 for review Message-ID: <4A835CBE.4060903@elischer.org> In-Reply-To: <200908130052.11423.zec@freebsd.org> References: <200908122108.n7CL8uhJ058398@repoman.freebsd.org> <200908130034.57133.zec@freebsd.org> <4A8345E1.1070301@elischer.org> <200908130052.11423.zec@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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.
>
>>> 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
>
> Then we shouldn't attempt to do it.
>
> Marko
>
>
>>> 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...
>>>
>>> Marko
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4A835CBE.4060903>