Hi, I'm probably too late, as it's already merged, but as explained in initial review [0], these checks are more about security than feature support:
ip netns uses mount namespaces since check-in (and mount namespaces are supported since 2.4.19 [1]) The reason for this check was more to make sure that this command isn't used outside a mount namespace to bind-mount arbitrary directory in the root filesystem, as neutron_netns_wrapper is allowed by CommandFilter. The only way i figured out to check that at time of writing was to use namespace inodes, that landed in 3.8 [2] There may be anyway better solutions to ensure that this bind-mount is done within a namespace... [0] https://review.openstack.org/#/c/33467/4/quantum/agent/linux/netns_wrapper.py [1] http://lwn.net/Articles/531114/ [2] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=98f842e675f96ffac96e6c50315790912b2812be francois. 2015-01-23 17:57 GMT+01:00 Paul Michali <p...@michali.net>: > Maybe I'm misunderstanding the issue? > > I thought the reason there is no version check currently, is because a check > is being made to see if the process is in the same namespace as root for the > net namespace (as a proxy to checking that the mount namespace is being > used). > > The comment indicates that one could check the mount namespace, but that > would require a kernel 3.8+. > > My question was whether we should check the mount namespace and therefore > require a kernel of 3.8+? > > Maybe it is just easier to stick with (C). > > Thoughts? Should this review move forward as-is? > > Regards, > > PCM > > > PCM (Paul Michali) > > IRC............ pc_m (irc.freenode.com) > Twitter....... @pmichali > > > On Fri, Jan 23, 2015 at 10:37 AM, Kyle Mestery <mest...@mestery.com> wrote: >> >> According to the patch author, the check isn't necessary at all. >> >> On Fri, Jan 23, 2015 at 7:12 AM, Paul Michali <p...@michali.net> wrote: >>> >>> To summarize, should we... >>> >>> A) Assume all kernels will be 3.8+ and use mount namespace (risky?) >>> B) Do a check to ensure kernel is 3.8+ and fall back to net namespace and >>> mount --bind if not (more work). >>> C) Just use net namespace as indication that namespace with mount --bind >>> done (simple) >>> >>> Maybe it is best to just do the simple thing for now. I wanted to double >>> check though, to see if the alternatives could/should be considered. >>> >>> Regards, >>> >>> PCM >>> >>> >>> PCM (Paul Michali) >>> >>> IRC............ pc_m (irc.freenode.com) >>> Twitter....... @pmichali >>> >>> >>> On Fri, Jan 23, 2015 at 1:35 AM, Joshua Zhang >>> <joshua.zh...@canonical.com> wrote: >>>> >>>> pls note that actually this patch doesn't have minumum kernel >>>> requirement because it only uses 'mount --bind' and 'net namespace', not >>>> use >>>> 'mount namespace'. ('mount --bind' is since linux 2.4, 'net namespace' is >>>> since Linux 3.0, 'mount namespace' is since Linux 3.8). >>>> >>>> so I think sanity checks for 3.8 is not need, any thoughts ? >>>> >>>> thanks. >>>> >>>> >>>> On Fri, Jan 23, 2015 at 2:12 PM, Kevin Benton <blak...@gmail.com> wrote: >>>>> >>>>> >If we can consolidate that and use a single tool from the master >>>>> > neutron repository, that would be my vote. >>>>> >>>>> +1 with a hook mechanism so the sanity checks stay in the *aas repos >>>>> and they are only run if installed. >>>>> >>>>> >>>>> On Thu, Jan 22, 2015 at 7:30 AM, Kyle Mestery <mest...@mestery.com> >>>>> wrote: >>>>>> >>>>>> On Wed, Jan 21, 2015 at 10:27 AM, Ihar Hrachyshka >>>>>> <ihrac...@redhat.com> wrote: >>>>>>> >>>>>>> On 01/20/2015 05:40 PM, Paul Michali wrote: >>>>>>> >>>>>>> Review https://review.openstack.org/#/c/146508/ is adding support for >>>>>>> StrongSwan VPN, which needs mount bind to be able to specify different >>>>>>> paths >>>>>>> for config files. >>>>>>> >>>>>>> The code, which used some older patch, does a test for >>>>>>> /proc/1/ns/net, instead of /proc/1/ns/mnt, because it stated that the >>>>>>> latter >>>>>>> is only supported in kernel 3.8+. That was a while ago, and I'm >>>>>>> wondering if >>>>>>> the condition is still true. If we know that for Kilo and on, we'll be >>>>>>> dealing with 3.8+ kernels, we could use the more accurate test. >>>>>>> >>>>>>> Can we require 3.8+ kernel for this? >>>>>>> >>>>>>> >>>>>>> I think we can but it's better to check with distributions. Red Hat >>>>>>> wise, we ship a kernel that is newer than 3.8. >>>>>>> >>>>>>> If so, how and where do we ensure that is true? >>>>>>> >>>>>>> >>>>>>> Ideally, you would implement a sanity check for the feature you need >>>>>>> from the kernel. Though it opens a question of whether we want to ship >>>>>>> multiple sanity check tools for each of repos (neutron + 3 *aas repos). >>>>>>> >>>>>> If we can consolidate that and use a single tool from the master >>>>>> neutron repository, that would be my vote. >>>>>>> >>>>>>> >>>>>>> Also, if you can kindly review the code here: >>>>>>> https://review.openstack.org/#/c/146508/5/neutron_vpnaas/services/vpn/common/netns_wrapper.py, >>>>>>> I'd really appreciate it, as I'm not versed in the Linux proc files at >>>>>>> all. >>>>>>> >>>>>>> Thanks! >>>>>>> >>>>>>> >>>>>>> PCM (Paul Michali) >>>>>>> >>>>>>> IRC............ pc_m (irc.freenode.com) >>>>>>> Twitter....... @pmichali >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> __________________________________________________________________________ >>>>>>> OpenStack Development Mailing List (not for usage questions) >>>>>>> Unsubscribe: >>>>>>> openstack-dev-requ...@lists.openstack.org?subject:unsubscribe >>>>>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> __________________________________________________________________________ >>>>>>> OpenStack Development Mailing List (not for usage questions) >>>>>>> Unsubscribe: >>>>>>> openstack-dev-requ...@lists.openstack.org?subject:unsubscribe >>>>>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >>>>>>> >>>>>> >>>>>> >>>>>> >>>>>> __________________________________________________________________________ >>>>>> OpenStack Development Mailing List (not for usage questions) >>>>>> Unsubscribe: >>>>>> openstack-dev-requ...@lists.openstack.org?subject:unsubscribe >>>>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >>>>>> >>>>> >>>>> >>>>> >>>>> -- >>>>> Kevin Benton >>>>> >>>>> >>>>> __________________________________________________________________________ >>>>> OpenStack Development Mailing List (not for usage questions) >>>>> Unsubscribe: >>>>> openstack-dev-requ...@lists.openstack.org?subject:unsubscribe >>>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >>>>> >>>> >>>> >>>> >>>> -- >>>> Best Regards >>>> Zhang Hua(张华) >>>> Software Engineer | Canonical >>>> IRC: zhhuabj >>>> >>>> >>>> __________________________________________________________________________ >>>> OpenStack Development Mailing List (not for usage questions) >>>> Unsubscribe: >>>> openstack-dev-requ...@lists.openstack.org?subject:unsubscribe >>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >>>> >>> >>> >>> >>> __________________________________________________________________________ >>> OpenStack Development Mailing List (not for usage questions) >>> Unsubscribe: >>> openstack-dev-requ...@lists.openstack.org?subject:unsubscribe >>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >>> >> >> >> __________________________________________________________________________ >> OpenStack Development Mailing List (not for usage questions) >> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe >> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >> > > > __________________________________________________________________________ > OpenStack Development Mailing List (not for usage questions) > Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > __________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev