You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Chi Zhang <ch...@gmail.com> on 2014/10/23 21:09:32 UTC
Review Request 27105: routing: added a check to impose 3.6 and beyond
kernels. This could be removed once the bug in libnl3 is fixed.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27105/
-----------------------------------------------------------
Review request for mesos, Jie Yu and Cong Wang.
Bugs: mesos-1967
https://issues.apache.org/jira/browse/mesos-1967
Repository: mesos-git
Description
-------
see summary.
Diffs
-----
src/linux/routing/utils.cpp 80b62d0
src/tests/routing_tests.cpp bd4d071
Diff: https://reviews.apache.org/r/27105/diff/
Testing
-------
Tested on a 3.4 and 3.10, and made sure on the check works on 3.4.
Thanks,
Chi Zhang
Re: Review Request 27105: routing: added a check to impose 3.6 and
beyond kernels. This could be removed once the bug in libnl3 is fixed.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27105/#review58177
-----------------------------------------------------------
Patch looks great!
Reviews applied: [27105]
All tests passed.
- Mesos ReviewBot
On Oct. 23, 2014, 10:21 p.m., Chi Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27105/
> -----------------------------------------------------------
>
> (Updated Oct. 23, 2014, 10:21 p.m.)
>
>
> Review request for mesos, Jie Yu and Cong Wang.
>
>
> Bugs: mesos-1967
> https://issues.apache.org/jira/browse/mesos-1967
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> see summary.
>
>
> Diffs
> -----
>
> src/linux/routing/utils.cpp 80b62d0
> src/tests/routing_tests.cpp bd4d071
>
> Diff: https://reviews.apache.org/r/27105/diff/
>
>
> Testing
> -------
>
> Tested on a 3.4 and 3.10, and made sure on the check works on 3.4.
>
>
> Thanks,
>
> Chi Zhang
>
>
Re: Review Request 27105: routing: added a check to impose 3.6 and
beyond kernels. This could be removed once the bug in libnl3 is fixed.
Posted by Chi Zhang <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27105/
-----------------------------------------------------------
(Updated Oct. 23, 2014, 10:21 p.m.)
Review request for mesos, Jie Yu and Cong Wang.
Changes
-------
addressed comments.
Bugs: mesos-1967
https://issues.apache.org/jira/browse/mesos-1967
Repository: mesos-git
Description
-------
see summary.
Diffs (updated)
-----
src/linux/routing/utils.cpp 80b62d0
src/tests/routing_tests.cpp bd4d071
Diff: https://reviews.apache.org/r/27105/diff/
Testing
-------
Tested on a 3.4 and 3.10, and made sure on the check works on 3.4.
Thanks,
Chi Zhang
Re: Review Request 27105: routing: added a check to impose 3.6 and
beyond kernels. This could be removed once the bug in libnl3 is fixed.
Posted by Jie Yu <yu...@gmail.com>.
> On Oct. 23, 2014, 9:37 p.m., Ben Mahler wrote:
> > I'm curious if we can make the kernel + libnl version checking explicit at configure time.
> >
> > It would be nice to tell them when running ./configure that their kernel or libnl version are not valid for enabling network isolation, rather than failing make check, or failing at runtime.
>
> Jie Yu wrote:
> We delibrately choose to do dynamic check because the libnl that mesos links at runtime maybe different from the libnl during compilation. Same argument for kernel version.
>
> Ben Mahler wrote:
> Agreed, I'm not saying get rid of the dynamic check, that is definitely required.
>
> I'm just saying, it would be nice to tell the user at configure time that what they're trying to configure is not valid. Up to you guys.
Make sense. I'll create a ticket for tracking it.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27105/#review58114
-----------------------------------------------------------
On Oct. 23, 2014, 7:09 p.m., Chi Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27105/
> -----------------------------------------------------------
>
> (Updated Oct. 23, 2014, 7:09 p.m.)
>
>
> Review request for mesos, Jie Yu and Cong Wang.
>
>
> Bugs: mesos-1967
> https://issues.apache.org/jira/browse/mesos-1967
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> see summary.
>
>
> Diffs
> -----
>
> src/linux/routing/utils.cpp 80b62d0
> src/tests/routing_tests.cpp bd4d071
>
> Diff: https://reviews.apache.org/r/27105/diff/
>
>
> Testing
> -------
>
> Tested on a 3.4 and 3.10, and made sure on the check works on 3.4.
>
>
> Thanks,
>
> Chi Zhang
>
>
Re: Review Request 27105: routing: added a check to impose 3.6 and
beyond kernels. This could be removed once the bug in libnl3 is fixed.
Posted by Jie Yu <yu...@gmail.com>.
> On Oct. 23, 2014, 9:37 p.m., Ben Mahler wrote:
> > I'm curious if we can make the kernel + libnl version checking explicit at configure time.
> >
> > It would be nice to tell them when running ./configure that their kernel or libnl version are not valid for enabling network isolation, rather than failing make check, or failing at runtime.
We delibrately choose to do dynamic check because the libnl that mesos links at runtime maybe different from the libnl during compilation. Same argument for kernel version.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27105/#review58114
-----------------------------------------------------------
On Oct. 23, 2014, 7:09 p.m., Chi Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27105/
> -----------------------------------------------------------
>
> (Updated Oct. 23, 2014, 7:09 p.m.)
>
>
> Review request for mesos, Jie Yu and Cong Wang.
>
>
> Bugs: mesos-1967
> https://issues.apache.org/jira/browse/mesos-1967
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> see summary.
>
>
> Diffs
> -----
>
> src/linux/routing/utils.cpp 80b62d0
> src/tests/routing_tests.cpp bd4d071
>
> Diff: https://reviews.apache.org/r/27105/diff/
>
>
> Testing
> -------
>
> Tested on a 3.4 and 3.10, and made sure on the check works on 3.4.
>
>
> Thanks,
>
> Chi Zhang
>
>
Re: Review Request 27105: routing: added a check to impose 3.6 and
beyond kernels. This could be removed once the bug in libnl3 is fixed.
Posted by Chi Zhang <ch...@gmail.com>.
> On Oct. 23, 2014, 9:37 p.m., Ben Mahler wrote:
> > I'm curious if we can make the kernel + libnl version checking explicit at configure time.
> >
> > It would be nice to tell them when running ./configure that their kernel or libnl version are not valid for enabling network isolation, rather than failing make check, or failing at runtime.
>
> Jie Yu wrote:
> We delibrately choose to do dynamic check because the libnl that mesos links at runtime maybe different from the libnl during compilation. Same argument for kernel version.
>
> Ben Mahler wrote:
> Agreed, I'm not saying get rid of the dynamic check, that is definitely required.
>
> I'm just saying, it would be nice to tell the user at configure time that what they're trying to configure is not valid. Up to you guys.
>
> Jie Yu wrote:
> Make sense. I'll create a ticket for tracking it.
one throught is that the kernel version string is often times changed from its orginal value. 3.4 becomes 2.6.44, e.g. We avoided using the kernel version string for this dynamic check for that. In network isolator, there is also a handleful of checks that essentially are kernel version checks, nor could we use the actual version itself there. Not sure if there is an elegant way to do this in the config script.
- Chi
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27105/#review58114
-----------------------------------------------------------
On Oct. 23, 2014, 7:09 p.m., Chi Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27105/
> -----------------------------------------------------------
>
> (Updated Oct. 23, 2014, 7:09 p.m.)
>
>
> Review request for mesos, Jie Yu and Cong Wang.
>
>
> Bugs: mesos-1967
> https://issues.apache.org/jira/browse/mesos-1967
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> see summary.
>
>
> Diffs
> -----
>
> src/linux/routing/utils.cpp 80b62d0
> src/tests/routing_tests.cpp bd4d071
>
> Diff: https://reviews.apache.org/r/27105/diff/
>
>
> Testing
> -------
>
> Tested on a 3.4 and 3.10, and made sure on the check works on 3.4.
>
>
> Thanks,
>
> Chi Zhang
>
>
Re: Review Request 27105: routing: added a check to impose 3.6 and
beyond kernels. This could be removed once the bug in libnl3 is fixed.
Posted by Ben Mahler <be...@gmail.com>.
> On Oct. 23, 2014, 9:37 p.m., Ben Mahler wrote:
> > I'm curious if we can make the kernel + libnl version checking explicit at configure time.
> >
> > It would be nice to tell them when running ./configure that their kernel or libnl version are not valid for enabling network isolation, rather than failing make check, or failing at runtime.
>
> Jie Yu wrote:
> We delibrately choose to do dynamic check because the libnl that mesos links at runtime maybe different from the libnl during compilation. Same argument for kernel version.
Agreed, I'm not saying get rid of the dynamic check, that is definitely required.
I'm just saying, it would be nice to tell the user at configure time that what they're trying to configure is not valid. Up to you guys.
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27105/#review58114
-----------------------------------------------------------
On Oct. 23, 2014, 7:09 p.m., Chi Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27105/
> -----------------------------------------------------------
>
> (Updated Oct. 23, 2014, 7:09 p.m.)
>
>
> Review request for mesos, Jie Yu and Cong Wang.
>
>
> Bugs: mesos-1967
> https://issues.apache.org/jira/browse/mesos-1967
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> see summary.
>
>
> Diffs
> -----
>
> src/linux/routing/utils.cpp 80b62d0
> src/tests/routing_tests.cpp bd4d071
>
> Diff: https://reviews.apache.org/r/27105/diff/
>
>
> Testing
> -------
>
> Tested on a 3.4 and 3.10, and made sure on the check works on 3.4.
>
>
> Thanks,
>
> Chi Zhang
>
>
Re: Review Request 27105: routing: added a check to impose 3.6 and
beyond kernels. This could be removed once the bug in libnl3 is fixed.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27105/#review58114
-----------------------------------------------------------
I'm curious if we can make the kernel + libnl version checking explicit at configure time.
It would be nice to tell them when running ./configure that their kernel or libnl version are not valid for enabling network isolation, rather than failing make check, or failing at runtime.
- Ben Mahler
On Oct. 23, 2014, 7:09 p.m., Chi Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27105/
> -----------------------------------------------------------
>
> (Updated Oct. 23, 2014, 7:09 p.m.)
>
>
> Review request for mesos, Jie Yu and Cong Wang.
>
>
> Bugs: mesos-1967
> https://issues.apache.org/jira/browse/mesos-1967
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> see summary.
>
>
> Diffs
> -----
>
> src/linux/routing/utils.cpp 80b62d0
> src/tests/routing_tests.cpp bd4d071
>
> Diff: https://reviews.apache.org/r/27105/diff/
>
>
> Testing
> -------
>
> Tested on a 3.4 and 3.10, and made sure on the check works on 3.4.
>
>
> Thanks,
>
> Chi Zhang
>
>
Re: Review Request 27105: routing: added a check to impose 3.6 and
beyond kernels. This could be removed once the bug in libnl3 is fixed.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27105/#review58105
-----------------------------------------------------------
Ship it!
src/linux/routing/utils.cpp
<https://reviews.apache.org/r/27105/#comment99006>
Remove this.
- Jie Yu
On Oct. 23, 2014, 7:09 p.m., Chi Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27105/
> -----------------------------------------------------------
>
> (Updated Oct. 23, 2014, 7:09 p.m.)
>
>
> Review request for mesos, Jie Yu and Cong Wang.
>
>
> Bugs: mesos-1967
> https://issues.apache.org/jira/browse/mesos-1967
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> see summary.
>
>
> Diffs
> -----
>
> src/linux/routing/utils.cpp 80b62d0
> src/tests/routing_tests.cpp bd4d071
>
> Diff: https://reviews.apache.org/r/27105/diff/
>
>
> Testing
> -------
>
> Tested on a 3.4 and 3.10, and made sure on the check works on 3.4.
>
>
> Thanks,
>
> Chi Zhang
>
>
Re: Review Request 27105: routing: added a check to impose 3.6 and
beyond kernels. This could be removed once the bug in libnl3 is fixed.
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27105/#review58062
-----------------------------------------------------------
src/linux/routing/utils.cpp
<https://reviews.apache.org/r/27105/#comment98961>
s/therefor/therefore/
src/linux/routing/utils.cpp
<https://reviews.apache.org/r/27105/#comment98960>
s/upgrade/upgrade./
- Dominic Hamon
On Oct. 23, 2014, 12:09 p.m., Chi Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27105/
> -----------------------------------------------------------
>
> (Updated Oct. 23, 2014, 12:09 p.m.)
>
>
> Review request for mesos, Jie Yu and Cong Wang.
>
>
> Bugs: mesos-1967
> https://issues.apache.org/jira/browse/mesos-1967
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> see summary.
>
>
> Diffs
> -----
>
> src/linux/routing/utils.cpp 80b62d0
> src/tests/routing_tests.cpp bd4d071
>
> Diff: https://reviews.apache.org/r/27105/diff/
>
>
> Testing
> -------
>
> Tested on a 3.4 and 3.10, and made sure on the check works on 3.4.
>
>
> Thanks,
>
> Chi Zhang
>
>