> On 五月 20, 2016, 5:02 a.m., Cong Wang wrote:
> > src/slave/containerizer/mesos/isolators/network/port_mapping.cpp, line 3927
> > <https://reviews.apache.org/r/47633/diff/1/?file=1388820#file1388820line3927>
> >
> >     You need to move this check into the script rather than in the C++ 
> > code. Something like "test -f /proc/sys/net/ipv6/conf/all/disable_ipv6 && 
> > echo 1 > /proc/sys/net/ipv6/conf/all/disable_ipv6".
> 
> Zhengju Sha wrote:
>     Thanks for the reply.
>     I thought this way before, but I notice that we also use os::exists() to 
> check other file in this function, so I choose to keep in consistent. Both of 
> the manner is okay IMHO, is there other considerations to move it to bash 
> script?
> 
> haosdent huang wrote:
>     Hmm, keep consistent more reasonable. Because we have
>     
>     ```
>       // Enable route_localnet on lo because by default 127.0.0.1 traffic
>       // is dropped. This feature exists on 3.6 kernel or newer.
>       if (os::exists(path::join("/proc/sys/net/ipv4/conf", lo, 
> "route_localnet"))) {
>         script << "echo 1 > /proc/sys/net/ipv4/conf/" << lo << 
> "/route_localnet\n";
>       }
>     ```
> 
> Cong Wang wrote:
>     Nope, you don't understand it. Ipv6 module could be removed during 
> preparation and launch, although not likely, but still.
> 
> haosdent huang wrote:
>     Thanks @wangcong's nice explanation, it make sense. My bad to post that 
> before.

Yeah, sound reasonable, your consideration is more thoughtful. 

But speak more, as talking of this kind of risk window, "test -f 
/proc/sys/net/ipv6/conf/all/disable_ipv6 && echo 1 > 
/proc/sys/net/ipv6/conf/all/disable_ipv6" also doesn't avoid the problem in 
essence, but narrow the potential probability.

And I also find the following code in PortMappingIsolatorProcess::isolate() 
which has the some problem:

  2608   // Disable IPv6 for veth as IPv6 packets won't be forwarded anyway.
  2609   const string disableIPv6 =
  2610     path::join("/proc/sys/net/ipv6/conf", veth(pid), "disable_ipv6");
  2611 
  2612   if (os::exists(disableIPv6)) {
  2613     Try<Nothing> write = os::write(disableIPv6, "1");
  2614     if (write.isError()) {
  2615       return Failure(
  2616           "Failed to disable IPv6 for " + veth(pid) +
  2617           ": " + write.error());
  2618     }
  2619   }
  

As kernel doesn't export this kind of atomic test-set interface, these problems 
are very common. Maybe we can design very robust error handling for such cases, 
but I doubt whether it deserves high priority and extra energe in present 
circumstances of mesos community.

So if both of you have no objection, I'll choose Cong's proposal as a middle 
way in next version. :p


- Zhengju


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47633/#review134101
-----------------------------------------------------------


On 五月 20, 2016, 3:54 a.m., Zhengju Sha wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47633/
> -----------------------------------------------------------
> 
> (Updated 五月 20, 2016, 3:54 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Cong Wang.
> 
> 
> Bugs: MESOS-5381
>     https://issues.apache.org/jira/browse/MESOS-5381
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Isolation/networking: check if IPv6 is loaded before trying to disable it
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> ad792def2bb3a1614d21ca28d858e400d2e3ede1 
> 
> Diff: https://reviews.apache.org/r/47633/diff/
> 
> 
> Testing
> -------
> 
> Enniornment and steps:
> 1. Enable mesos-slave --isolation=network/port_mapping on CentOS7.2 with 
> kernel version: 3.10.0-327.10.1.el7.x86_64
> 2. Create application on marathon framework with commands such as "echo 
> hello" using MesosContainerizer
> 3. Load IPv6 module by removing "ipv6.disable=1" of GRUB_CMDLINE_LINUX in 
> /etc/default/grub
> 4. Disable IPv6 module by adding "ipv6.disable=1" of GRUB_CMDLINE_LINUX in 
> /etc/default/grub
> 
> Now mesos can run both of the testcases successfully.
> 
> 
> Thanks,
> 
> Zhengju Sha
> 
>

Reply via email to