You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Cong Wang <cw...@twopensource.com> on 2015/03/11 00:26:12 UTC
Re: Review Request 31503: Add classid to Filter
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31503/
-----------------------------------------------------------
(Updated March 10, 2015, 11:26 p.m.)
Review request for mesos, Chi Zhang, Ian Downes, and Jie Yu.
Summary (updated)
-----------------
Add classid to Filter
Bugs: MESOS-2422
https://issues.apache.org/jira/browse/MESOS-2422
Repository: mesos
Description (updated)
-------
Currently we don't support to read actions, so intead of using an action, we add the classid to the generic Filter class. Now it is much easier to read it for recovery.
Diffs (updated)
-----
src/linux/routing/filter/arp.hpp fa0ea6f93218fb3bee9f435f0268022d5edc5e11
src/linux/routing/filter/arp.cpp bf19264ccc83e8a8b8c2affe53779ac070472925
src/linux/routing/filter/filter.hpp d4ea099a2e35ffc0e6d560c5de9864dbbfe10038
src/linux/routing/filter/icmp.hpp 431bc19eb600b0568874dacfce123adef0f9ff0b
src/linux/routing/filter/icmp.cpp 706b5d1871d9e0b79d1b869d27d0e87112b75975
src/linux/routing/filter/internal.hpp 8a6c0c0390c4ed119a11f0f808e0a244f7734c45
src/linux/routing/filter/ip.hpp b5406024fdbfd1510bcf619f04a7369763396e72
src/linux/routing/filter/ip.cpp de6407119c6726e0f32b07fd6ff61fa29262638b
src/linux/routing/queueing/handle.hpp 2725d0794ca29ad5dc1b0148d0f68b90ce8b8369
src/tests/routing_tests.cpp 7cc3b57a3b71544874557d2b1cf88a241b7062ba
Diff: https://reviews.apache.org/r/31503/diff/
Testing
-------
Run the testcase.
Thanks,
Cong Wang
Re: Review Request 31503: Add classid to Filter
Posted by Ian Downes <ia...@gmail.com>.
> On March 11, 2015, 7:02 p.m., Jie Yu wrote:
> > Cong, I addressed the comments for you and try to commit. Looks like rtnl_u32_get_classid is not in libnl-3.2.25. That means we need a newer version of libnl for this to work!
>
> Cong Wang wrote:
> I have addressed your comments locally, just haven't uploaded a new version.
> Yes, we need this commit from upstream:
> https://github.com/thom311/libnl/commit/d8f080d94fa9cf5e977f8805446ac7ef39f82d31
>
> Cong Wang wrote:
> 3.2.26 will be released soon, according to upstream maintainer. So how about adding a check in configure.ac?
>
> + # Check for libnl-route (both headers and libraries).
> + AC_CHECK_LIB([nl-route-3], [rtnl_u32_get_classid], [],
> + [AC_MSG_ERROR([cannot find libnl-route-3
> +-------------------------------------------------------------------
> +We need libnl-route-3 for building network isolator!
> +
> +Please install libnl3 (version 3.2.26 or higher):
> +http://www.infradead.org/~tgr/libnl/
Do you have an estimated date for the release? Can we clean up some of the documentation if we add the check?
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31503/#review76195
-----------------------------------------------------------
On March 10, 2015, 4:26 p.m., Cong Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31503/
> -----------------------------------------------------------
>
> (Updated March 10, 2015, 4:26 p.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, and Jie Yu.
>
>
> Bugs: MESOS-2422
> https://issues.apache.org/jira/browse/MESOS-2422
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Currently we don't support to read actions, so intead of using an action, we add the classid to the generic Filter class. Now it is much easier to read it for recovery.
>
>
> Diffs
> -----
>
> src/linux/routing/filter/arp.hpp fa0ea6f93218fb3bee9f435f0268022d5edc5e11
> src/linux/routing/filter/arp.cpp bf19264ccc83e8a8b8c2affe53779ac070472925
> src/linux/routing/filter/filter.hpp d4ea099a2e35ffc0e6d560c5de9864dbbfe10038
> src/linux/routing/filter/icmp.hpp 431bc19eb600b0568874dacfce123adef0f9ff0b
> src/linux/routing/filter/icmp.cpp 706b5d1871d9e0b79d1b869d27d0e87112b75975
> src/linux/routing/filter/internal.hpp 8a6c0c0390c4ed119a11f0f808e0a244f7734c45
> src/linux/routing/filter/ip.hpp b5406024fdbfd1510bcf619f04a7369763396e72
> src/linux/routing/filter/ip.cpp de6407119c6726e0f32b07fd6ff61fa29262638b
> src/linux/routing/queueing/handle.hpp 2725d0794ca29ad5dc1b0148d0f68b90ce8b8369
> src/tests/routing_tests.cpp 7cc3b57a3b71544874557d2b1cf88a241b7062ba
>
> Diff: https://reviews.apache.org/r/31503/diff/
>
>
> Testing
> -------
>
> Run the testcase.
>
>
> Thanks,
>
> Cong Wang
>
>
Re: Review Request 31503: Add classid to Filter
Posted by Cong Wang <cw...@twopensource.com>.
> On March 12, 2015, 2:02 a.m., Jie Yu wrote:
> > Cong, I addressed the comments for you and try to commit. Looks like rtnl_u32_get_classid is not in libnl-3.2.25. That means we need a newer version of libnl for this to work!
>
> Cong Wang wrote:
> I have addressed your comments locally, just haven't uploaded a new version.
> Yes, we need this commit from upstream:
> https://github.com/thom311/libnl/commit/d8f080d94fa9cf5e977f8805446ac7ef39f82d31
>
> Cong Wang wrote:
> 3.2.26 will be released soon, according to upstream maintainer. So how about adding a check in configure.ac?
>
> + # Check for libnl-route (both headers and libraries).
> + AC_CHECK_LIB([nl-route-3], [rtnl_u32_get_classid], [],
> + [AC_MSG_ERROR([cannot find libnl-route-3
> +-------------------------------------------------------------------
> +We need libnl-route-3 for building network isolator!
> +
> +Please install libnl3 (version 3.2.26 or higher):
> +http://www.infradead.org/~tgr/libnl/
>
> Ian Downes wrote:
> Do you have an estimated date for the release? Can we clean up some of the documentation if we add the check?
3.2.26-rc1 was released on Mar 10, if there is no major bug reported 3.2.26 will be release ~2 weeks after that, that is about 24 Mar.
- Cong
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31503/#review76195
-----------------------------------------------------------
On March 10, 2015, 11:26 p.m., Cong Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31503/
> -----------------------------------------------------------
>
> (Updated March 10, 2015, 11:26 p.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, and Jie Yu.
>
>
> Bugs: MESOS-2422
> https://issues.apache.org/jira/browse/MESOS-2422
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Currently we don't support to read actions, so intead of using an action, we add the classid to the generic Filter class. Now it is much easier to read it for recovery.
>
>
> Diffs
> -----
>
> src/linux/routing/filter/arp.hpp fa0ea6f93218fb3bee9f435f0268022d5edc5e11
> src/linux/routing/filter/arp.cpp bf19264ccc83e8a8b8c2affe53779ac070472925
> src/linux/routing/filter/filter.hpp d4ea099a2e35ffc0e6d560c5de9864dbbfe10038
> src/linux/routing/filter/icmp.hpp 431bc19eb600b0568874dacfce123adef0f9ff0b
> src/linux/routing/filter/icmp.cpp 706b5d1871d9e0b79d1b869d27d0e87112b75975
> src/linux/routing/filter/internal.hpp 8a6c0c0390c4ed119a11f0f808e0a244f7734c45
> src/linux/routing/filter/ip.hpp b5406024fdbfd1510bcf619f04a7369763396e72
> src/linux/routing/filter/ip.cpp de6407119c6726e0f32b07fd6ff61fa29262638b
> src/linux/routing/queueing/handle.hpp 2725d0794ca29ad5dc1b0148d0f68b90ce8b8369
> src/tests/routing_tests.cpp 7cc3b57a3b71544874557d2b1cf88a241b7062ba
>
> Diff: https://reviews.apache.org/r/31503/diff/
>
>
> Testing
> -------
>
> Run the testcase.
>
>
> Thanks,
>
> Cong Wang
>
>
Re: Review Request 31503: Add classid to Filter
Posted by Cong Wang <cw...@twopensource.com>.
> On March 12, 2015, 2:02 a.m., Jie Yu wrote:
> > Cong, I addressed the comments for you and try to commit. Looks like rtnl_u32_get_classid is not in libnl-3.2.25. That means we need a newer version of libnl for this to work!
>
> Cong Wang wrote:
> I have addressed your comments locally, just haven't uploaded a new version.
> Yes, we need this commit from upstream:
> https://github.com/thom311/libnl/commit/d8f080d94fa9cf5e977f8805446ac7ef39f82d31
3.2.26 will be released soon, according to upstream maintainer. So how about adding a check in configure.ac?
+ # Check for libnl-route (both headers and libraries).
+ AC_CHECK_LIB([nl-route-3], [rtnl_u32_get_classid], [],
+ [AC_MSG_ERROR([cannot find libnl-route-3
+-------------------------------------------------------------------
+We need libnl-route-3 for building network isolator!
+
+Please install libnl3 (version 3.2.26 or higher):
+http://www.infradead.org/~tgr/libnl/
- Cong
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31503/#review76195
-----------------------------------------------------------
On March 10, 2015, 11:26 p.m., Cong Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31503/
> -----------------------------------------------------------
>
> (Updated March 10, 2015, 11:26 p.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, and Jie Yu.
>
>
> Bugs: MESOS-2422
> https://issues.apache.org/jira/browse/MESOS-2422
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Currently we don't support to read actions, so intead of using an action, we add the classid to the generic Filter class. Now it is much easier to read it for recovery.
>
>
> Diffs
> -----
>
> src/linux/routing/filter/arp.hpp fa0ea6f93218fb3bee9f435f0268022d5edc5e11
> src/linux/routing/filter/arp.cpp bf19264ccc83e8a8b8c2affe53779ac070472925
> src/linux/routing/filter/filter.hpp d4ea099a2e35ffc0e6d560c5de9864dbbfe10038
> src/linux/routing/filter/icmp.hpp 431bc19eb600b0568874dacfce123adef0f9ff0b
> src/linux/routing/filter/icmp.cpp 706b5d1871d9e0b79d1b869d27d0e87112b75975
> src/linux/routing/filter/internal.hpp 8a6c0c0390c4ed119a11f0f808e0a244f7734c45
> src/linux/routing/filter/ip.hpp b5406024fdbfd1510bcf619f04a7369763396e72
> src/linux/routing/filter/ip.cpp de6407119c6726e0f32b07fd6ff61fa29262638b
> src/linux/routing/queueing/handle.hpp 2725d0794ca29ad5dc1b0148d0f68b90ce8b8369
> src/tests/routing_tests.cpp 7cc3b57a3b71544874557d2b1cf88a241b7062ba
>
> Diff: https://reviews.apache.org/r/31503/diff/
>
>
> Testing
> -------
>
> Run the testcase.
>
>
> Thanks,
>
> Cong Wang
>
>
Re: Review Request 31503: Add classid to Filter
Posted by Cong Wang <cw...@twopensource.com>.
> On March 12, 2015, 2:02 a.m., Jie Yu wrote:
> > Cong, I addressed the comments for you and try to commit. Looks like rtnl_u32_get_classid is not in libnl-3.2.25. That means we need a newer version of libnl for this to work!
I have addressed your comments locally, just haven't uploaded a new version.
Yes, we need this commit from upstream:
https://github.com/thom311/libnl/commit/d8f080d94fa9cf5e977f8805446ac7ef39f82d31
- Cong
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31503/#review76195
-----------------------------------------------------------
On March 10, 2015, 11:26 p.m., Cong Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31503/
> -----------------------------------------------------------
>
> (Updated March 10, 2015, 11:26 p.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, and Jie Yu.
>
>
> Bugs: MESOS-2422
> https://issues.apache.org/jira/browse/MESOS-2422
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Currently we don't support to read actions, so intead of using an action, we add the classid to the generic Filter class. Now it is much easier to read it for recovery.
>
>
> Diffs
> -----
>
> src/linux/routing/filter/arp.hpp fa0ea6f93218fb3bee9f435f0268022d5edc5e11
> src/linux/routing/filter/arp.cpp bf19264ccc83e8a8b8c2affe53779ac070472925
> src/linux/routing/filter/filter.hpp d4ea099a2e35ffc0e6d560c5de9864dbbfe10038
> src/linux/routing/filter/icmp.hpp 431bc19eb600b0568874dacfce123adef0f9ff0b
> src/linux/routing/filter/icmp.cpp 706b5d1871d9e0b79d1b869d27d0e87112b75975
> src/linux/routing/filter/internal.hpp 8a6c0c0390c4ed119a11f0f808e0a244f7734c45
> src/linux/routing/filter/ip.hpp b5406024fdbfd1510bcf619f04a7369763396e72
> src/linux/routing/filter/ip.cpp de6407119c6726e0f32b07fd6ff61fa29262638b
> src/linux/routing/queueing/handle.hpp 2725d0794ca29ad5dc1b0148d0f68b90ce8b8369
> src/tests/routing_tests.cpp 7cc3b57a3b71544874557d2b1cf88a241b7062ba
>
> Diff: https://reviews.apache.org/r/31503/diff/
>
>
> Testing
> -------
>
> Run the testcase.
>
>
> Thanks,
>
> Cong Wang
>
>
Re: Review Request 31503: Add classid to Filter
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31503/#review76195
-----------------------------------------------------------
Cong, I addressed the comments for you and try to commit. Looks like rtnl_u32_get_classid is not in libnl-3.2.25. That means we need a newer version of libnl for this to work!
- Jie Yu
On March 10, 2015, 11:26 p.m., Cong Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31503/
> -----------------------------------------------------------
>
> (Updated March 10, 2015, 11:26 p.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, and Jie Yu.
>
>
> Bugs: MESOS-2422
> https://issues.apache.org/jira/browse/MESOS-2422
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Currently we don't support to read actions, so intead of using an action, we add the classid to the generic Filter class. Now it is much easier to read it for recovery.
>
>
> Diffs
> -----
>
> src/linux/routing/filter/arp.hpp fa0ea6f93218fb3bee9f435f0268022d5edc5e11
> src/linux/routing/filter/arp.cpp bf19264ccc83e8a8b8c2affe53779ac070472925
> src/linux/routing/filter/filter.hpp d4ea099a2e35ffc0e6d560c5de9864dbbfe10038
> src/linux/routing/filter/icmp.hpp 431bc19eb600b0568874dacfce123adef0f9ff0b
> src/linux/routing/filter/icmp.cpp 706b5d1871d9e0b79d1b869d27d0e87112b75975
> src/linux/routing/filter/internal.hpp 8a6c0c0390c4ed119a11f0f808e0a244f7734c45
> src/linux/routing/filter/ip.hpp b5406024fdbfd1510bcf619f04a7369763396e72
> src/linux/routing/filter/ip.cpp de6407119c6726e0f32b07fd6ff61fa29262638b
> src/linux/routing/queueing/handle.hpp 2725d0794ca29ad5dc1b0148d0f68b90ce8b8369
> src/tests/routing_tests.cpp 7cc3b57a3b71544874557d2b1cf88a241b7062ba
>
> Diff: https://reviews.apache.org/r/31503/diff/
>
>
> Testing
> -------
>
> Run the testcase.
>
>
> Thanks,
>
> Cong Wang
>
>
Re: Review Request 31503: Add classid to Filter
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31503/#review75999
-----------------------------------------------------------
Ship it!
src/linux/routing/filter/arp.hpp
<https://reviews.apache.org/r/31503/#comment123386>
s/flowid/classid/
src/linux/routing/filter/arp.hpp
<https://reviews.apache.org/r/31503/#comment123376>
Can you consistently use `classid` for Filter? The word 'flowid' is used by the isolator and should be the lower 16 bits.
src/linux/routing/filter/arp.cpp
<https://reviews.apache.org/r/31503/#comment123381>
Ditto. Please use 'classid' here.
src/linux/routing/filter/arp.cpp
<https://reviews.apache.org/r/31503/#comment123382>
Ditto.
src/linux/routing/filter/filter.hpp
<https://reviews.apache.org/r/31503/#comment123383>
Ha, you call it 'classid' here!
src/linux/routing/filter/filter.hpp
<https://reviews.apache.org/r/31503/#comment123385>
WE may wanna add classful qdisc in the future. So just say that we use 'classid' consistently in the code base.
src/linux/routing/filter/icmp.cpp
<https://reviews.apache.org/r/31503/#comment123387>
Ditto.
src/linux/routing/filter/ip.hpp
<https://reviews.apache.org/r/31503/#comment123389>
Ditto.
src/linux/routing/filter/ip.cpp
<https://reviews.apache.org/r/31503/#comment123390>
Ditto.
src/linux/routing/queueing/handle.hpp
<https://reviews.apache.org/r/31503/#comment123391>
Please add a comment about this function. It's not quite obvious what this constructor is doing.
- Jie Yu
On March 10, 2015, 11:26 p.m., Cong Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31503/
> -----------------------------------------------------------
>
> (Updated March 10, 2015, 11:26 p.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, and Jie Yu.
>
>
> Bugs: MESOS-2422
> https://issues.apache.org/jira/browse/MESOS-2422
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Currently we don't support to read actions, so intead of using an action, we add the classid to the generic Filter class. Now it is much easier to read it for recovery.
>
>
> Diffs
> -----
>
> src/linux/routing/filter/arp.hpp fa0ea6f93218fb3bee9f435f0268022d5edc5e11
> src/linux/routing/filter/arp.cpp bf19264ccc83e8a8b8c2affe53779ac070472925
> src/linux/routing/filter/filter.hpp d4ea099a2e35ffc0e6d560c5de9864dbbfe10038
> src/linux/routing/filter/icmp.hpp 431bc19eb600b0568874dacfce123adef0f9ff0b
> src/linux/routing/filter/icmp.cpp 706b5d1871d9e0b79d1b869d27d0e87112b75975
> src/linux/routing/filter/internal.hpp 8a6c0c0390c4ed119a11f0f808e0a244f7734c45
> src/linux/routing/filter/ip.hpp b5406024fdbfd1510bcf619f04a7369763396e72
> src/linux/routing/filter/ip.cpp de6407119c6726e0f32b07fd6ff61fa29262638b
> src/linux/routing/queueing/handle.hpp 2725d0794ca29ad5dc1b0148d0f68b90ce8b8369
> src/tests/routing_tests.cpp 7cc3b57a3b71544874557d2b1cf88a241b7062ba
>
> Diff: https://reviews.apache.org/r/31503/diff/
>
>
> Testing
> -------
>
> Run the testcase.
>
>
> Thanks,
>
> Cong Wang
>
>