You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Cong Wang <cw...@twopensource.com> on 2015/10/18 02:29:19 UTC
Review Request 39417: Add --egress_flow_classifier_parent flag
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39417/
-----------------------------------------------------------
Review request for mesos, Ian Downes and Jie Yu.
Repository: mesos
Description
-------
When --egress_unique_flow_per_container is enabled, we need to install a flow classifier (fq_codel) qdisc on egress side. This flag specifies where to install it in the hierarchy. By default, we install it at root. But, for example, if you have already installed HTB qdisc at root, you may want this to be installed in other place than root, specify an HTB class ID as its parent here.
Diffs
-----
docs/configuration.md 9443d5fc2de0f46f9e117b08e29b09ff3a4579c6
src/slave/containerizer/isolators/network/port_mapping.cpp e6bb75e6f5ba48a0c4cf6dd8f353e5f5843d0eef
src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73
src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2
Diff: https://reviews.apache.org/r/39417/diff/
Testing
-------
Manual tests, with and without a pre-installed HTB qdisc and classes.
Thanks,
Cong Wang
Re: Review Request 39417: Add --egress_flow_classifier_parent flag
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39417/#review103048
-----------------------------------------------------------
Patch looks great!
Reviews applied: [39415, 39416, 39417]
All tests passed.
- Mesos ReviewBot
On Oct. 18, 2015, 12:29 a.m., Cong Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39417/
> -----------------------------------------------------------
>
> (Updated Oct. 18, 2015, 12:29 a.m.)
>
>
> Review request for mesos, Ian Downes and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When --egress_unique_flow_per_container is enabled, we need to install a flow classifier (fq_codel) qdisc on egress side. This flag specifies where to install it in the hierarchy. By default, we install it at root. But, for example, if you have already installed HTB qdisc at root, you may want this to be installed in other place than root, specify an HTB class ID as its parent here.
>
>
> Diffs
> -----
>
> docs/configuration.md 9443d5fc2de0f46f9e117b08e29b09ff3a4579c6
> src/slave/containerizer/isolators/network/port_mapping.cpp e6bb75e6f5ba48a0c4cf6dd8f353e5f5843d0eef
> src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73
> src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2
>
> Diff: https://reviews.apache.org/r/39417/diff/
>
>
> Testing
> -------
>
> Manual tests, with and without a pre-installed HTB qdisc and classes.
>
>
> Thanks,
>
> Cong Wang
>
>
Re: Review Request 39417: Add --egress_flow_classifier_parent flag
Posted by Cong Wang <cw...@twopensource.com>.
> On Oct. 27, 2015, 12:21 a.m., Ian Downes wrote:
> > Also, is there are way to test the different code paths?
I tested it manually: with and without this flag; with and without a pre-installed HTB qdisc and class.
- Cong
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39417/#review104120
-----------------------------------------------------------
On Oct. 18, 2015, 12:29 a.m., Cong Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39417/
> -----------------------------------------------------------
>
> (Updated Oct. 18, 2015, 12:29 a.m.)
>
>
> Review request for mesos, Ian Downes and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When --egress_unique_flow_per_container is enabled, we need to install a flow classifier (fq_codel) qdisc on egress side. This flag specifies where to install it in the hierarchy. By default, we install it at root. But, for example, if you have already installed HTB qdisc at root, you may want this to be installed in other place than root, specify an HTB class ID as its parent here.
>
>
> Diffs
> -----
>
> docs/configuration.md 9443d5fc2de0f46f9e117b08e29b09ff3a4579c6
> src/slave/containerizer/isolators/network/port_mapping.cpp e6bb75e6f5ba48a0c4cf6dd8f353e5f5843d0eef
> src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73
> src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2
>
> Diff: https://reviews.apache.org/r/39417/diff/
>
>
> Testing
> -------
>
> Manual tests, with and without a pre-installed HTB qdisc and classes.
>
>
> Thanks,
>
> Cong Wang
>
>
Re: Review Request 39417: Add --egress_flow_classifier_parent flag
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39417/#review104120
-----------------------------------------------------------
Also, is there are way to test the different code paths?
- Ian Downes
On Oct. 17, 2015, 5:29 p.m., Cong Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39417/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2015, 5:29 p.m.)
>
>
> Review request for mesos, Ian Downes and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When --egress_unique_flow_per_container is enabled, we need to install a flow classifier (fq_codel) qdisc on egress side. This flag specifies where to install it in the hierarchy. By default, we install it at root. But, for example, if you have already installed HTB qdisc at root, you may want this to be installed in other place than root, specify an HTB class ID as its parent here.
>
>
> Diffs
> -----
>
> docs/configuration.md 9443d5fc2de0f46f9e117b08e29b09ff3a4579c6
> src/slave/containerizer/isolators/network/port_mapping.cpp e6bb75e6f5ba48a0c4cf6dd8f353e5f5843d0eef
> src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73
> src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2
>
> Diff: https://reviews.apache.org/r/39417/diff/
>
>
> Testing
> -------
>
> Manual tests, with and without a pre-installed HTB qdisc and classes.
>
>
> Thanks,
>
> Cong Wang
>
>
Re: Review Request 39417: Add --egress_flow_classifier_parent flag
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39417/#review107690
-----------------------------------------------------------
Patch looks great!
Reviews applied: [40497, 40506, 39415, 39416, 39417]
Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh
- Mesos ReviewBot
On Nov. 23, 2015, 10:33 p.m., Cong Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39417/
> -----------------------------------------------------------
>
> (Updated Nov. 23, 2015, 10:33 p.m.)
>
>
> Review request for mesos, Ian Downes and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When --egress_unique_flow_per_container is enabled, we need to install a flow classifier (fq_codel) qdisc on egress side. This flag specifies where to install it in the hierarchy. By default, we install it at root. But, for example, if you have already installed HTB qdisc at root, you may want this to be installed in other place than root, specify an HTB class ID as its parent here.
>
>
> Diffs
> -----
>
> docs/configuration.md 126f4aa8e3de2a2346336991c9b9a2ea61a8cd0a
> src/Makefile.am a57e46d06c8c26a32a9444be4f0e1269d775f8b0
> src/linux/routing/handle.hpp 6deff85b52daa6b8771f127fea0d661cb5cd5e6a
> src/linux/routing/handle.cpp PRE-CREATION
> src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 0865a71b1d79adcf62ca0f12d7ea0a23ebd0e786
> src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 09f819685a96cb4785423a5b2303d147719e273e
> src/slave/flags.hpp 0858bdfa93f0c82c9654f9a11d1a251df2d41c36
> src/slave/flags.cpp 6e8872e435c9a5d895539e3cd47b41f66bc4eb89
>
> Diff: https://reviews.apache.org/r/39417/diff/
>
>
> Testing
> -------
>
> Manual tests, with and without a pre-installed HTB qdisc and classes.
>
>
> Thanks,
>
> Cong Wang
>
>
Re: Review Request 39417: Add --egress_flow_classifier_parent flag
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39417/#review108455
-----------------------------------------------------------
Patch looks great!
Reviews applied: [40497, 40506, 39415, 39416, 39417]
Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh
- Mesos ReviewBot
On Dec. 1, 2015, 12:18 a.m., Cong Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39417/
> -----------------------------------------------------------
>
> (Updated Dec. 1, 2015, 12:18 a.m.)
>
>
> Review request for mesos, Ian Downes and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When --egress_unique_flow_per_container is enabled, we need to install a flow classifier (fq_codel) qdisc on egress side. This flag specifies where to install it in the hierarchy. By default, we install it at root. But, for example, if you have already installed HTB qdisc at root, you may want this to be installed in other place than root, specify an HTB class ID as its parent here.
>
>
> Diffs
> -----
>
> docs/configuration.md addcc7262584c19f9e5afe4b796c5fc4ecdb53b7
> src/Makefile.am fd38cfa73d81a98c819378f99a766e2ddb7e1a04
> src/linux/routing/handle.hpp 6deff85b52daa6b8771f127fea0d661cb5cd5e6a
> src/linux/routing/handle.cpp PRE-CREATION
> src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 0865a71b1d79adcf62ca0f12d7ea0a23ebd0e786
> src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 09f819685a96cb4785423a5b2303d147719e273e
> src/slave/flags.hpp 0858bdfa93f0c82c9654f9a11d1a251df2d41c36
> src/slave/flags.cpp 31700b4f80cf346ab68f9e35d33d7cbb5ddc04cc
>
> Diff: https://reviews.apache.org/r/39417/diff/
>
>
> Testing
> -------
>
> Manual tests, with and without a pre-installed HTB qdisc and classes.
>
>
> Thanks,
>
> Cong Wang
>
>
Re: Review Request 39417: Add --egress_flow_classifier_parent flag
Posted by Cong Wang <cw...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39417/
-----------------------------------------------------------
(Updated Dec. 1, 2015, 12:18 a.m.)
Review request for mesos, Ian Downes and Jie Yu.
Changes
-------
Rebase
Repository: mesos
Description
-------
When --egress_unique_flow_per_container is enabled, we need to install a flow classifier (fq_codel) qdisc on egress side. This flag specifies where to install it in the hierarchy. By default, we install it at root. But, for example, if you have already installed HTB qdisc at root, you may want this to be installed in other place than root, specify an HTB class ID as its parent here.
Diffs (updated)
-----
docs/configuration.md addcc7262584c19f9e5afe4b796c5fc4ecdb53b7
src/Makefile.am fd38cfa73d81a98c819378f99a766e2ddb7e1a04
src/linux/routing/handle.hpp 6deff85b52daa6b8771f127fea0d661cb5cd5e6a
src/linux/routing/handle.cpp PRE-CREATION
src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 0865a71b1d79adcf62ca0f12d7ea0a23ebd0e786
src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 09f819685a96cb4785423a5b2303d147719e273e
src/slave/flags.hpp 0858bdfa93f0c82c9654f9a11d1a251df2d41c36
src/slave/flags.cpp 31700b4f80cf346ab68f9e35d33d7cbb5ddc04cc
Diff: https://reviews.apache.org/r/39417/diff/
Testing
-------
Manual tests, with and without a pre-installed HTB qdisc and classes.
Thanks,
Cong Wang
Re: Review Request 39417: Add --egress_flow_classifier_parent flag
Posted by Cong Wang <cw...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39417/
-----------------------------------------------------------
(Updated Nov. 30, 2015, 9:58 p.m.)
Review request for mesos, Ian Downes and Jie Yu.
Changes
-------
rebase
Repository: mesos
Description
-------
When --egress_unique_flow_per_container is enabled, we need to install a flow classifier (fq_codel) qdisc on egress side. This flag specifies where to install it in the hierarchy. By default, we install it at root. But, for example, if you have already installed HTB qdisc at root, you may want this to be installed in other place than root, specify an HTB class ID as its parent here.
Diffs (updated)
-----
docs/configuration.md addcc7262584c19f9e5afe4b796c5fc4ecdb53b7
src/Makefile.am fd38cfa73d81a98c819378f99a766e2ddb7e1a04
src/linux/routing/handle.hpp 6deff85b52daa6b8771f127fea0d661cb5cd5e6a
src/linux/routing/handle.cpp PRE-CREATION
src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 0865a71b1d79adcf62ca0f12d7ea0a23ebd0e786
src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 09f819685a96cb4785423a5b2303d147719e273e
src/slave/flags.hpp 0858bdfa93f0c82c9654f9a11d1a251df2d41c36
src/slave/flags.cpp 31700b4f80cf346ab68f9e35d33d7cbb5ddc04cc
Diff: https://reviews.apache.org/r/39417/diff/
Testing
-------
Manual tests, with and without a pre-installed HTB qdisc and classes.
Thanks,
Cong Wang
Re: Review Request 39417: Add --egress_flow_classifier_parent flag
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39417/#review108376
-----------------------------------------------------------
Ship it!
Ship It!
- Jie Yu
On Nov. 23, 2015, 10:33 p.m., Cong Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39417/
> -----------------------------------------------------------
>
> (Updated Nov. 23, 2015, 10:33 p.m.)
>
>
> Review request for mesos, Ian Downes and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When --egress_unique_flow_per_container is enabled, we need to install a flow classifier (fq_codel) qdisc on egress side. This flag specifies where to install it in the hierarchy. By default, we install it at root. But, for example, if you have already installed HTB qdisc at root, you may want this to be installed in other place than root, specify an HTB class ID as its parent here.
>
>
> Diffs
> -----
>
> docs/configuration.md 126f4aa8e3de2a2346336991c9b9a2ea61a8cd0a
> src/Makefile.am a57e46d06c8c26a32a9444be4f0e1269d775f8b0
> src/linux/routing/handle.hpp 6deff85b52daa6b8771f127fea0d661cb5cd5e6a
> src/linux/routing/handle.cpp PRE-CREATION
> src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 0865a71b1d79adcf62ca0f12d7ea0a23ebd0e786
> src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 09f819685a96cb4785423a5b2303d147719e273e
> src/slave/flags.hpp 0858bdfa93f0c82c9654f9a11d1a251df2d41c36
> src/slave/flags.cpp 6e8872e435c9a5d895539e3cd47b41f66bc4eb89
>
> Diff: https://reviews.apache.org/r/39417/diff/
>
>
> Testing
> -------
>
> Manual tests, with and without a pre-installed HTB qdisc and classes.
>
>
> Thanks,
>
> Cong Wang
>
>
Re: Review Request 39417: Add --egress_flow_classifier_parent flag
Posted by Cong Wang <cw...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39417/
-----------------------------------------------------------
(Updated Nov. 23, 2015, 10:33 p.m.)
Review request for mesos, Ian Downes and Jie Yu.
Changes
-------
Address review comments and rebase
Repository: mesos
Description
-------
When --egress_unique_flow_per_container is enabled, we need to install a flow classifier (fq_codel) qdisc on egress side. This flag specifies where to install it in the hierarchy. By default, we install it at root. But, for example, if you have already installed HTB qdisc at root, you may want this to be installed in other place than root, specify an HTB class ID as its parent here.
Diffs (updated)
-----
docs/configuration.md 126f4aa8e3de2a2346336991c9b9a2ea61a8cd0a
src/Makefile.am a57e46d06c8c26a32a9444be4f0e1269d775f8b0
src/linux/routing/handle.hpp 6deff85b52daa6b8771f127fea0d661cb5cd5e6a
src/linux/routing/handle.cpp PRE-CREATION
src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 0865a71b1d79adcf62ca0f12d7ea0a23ebd0e786
src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 09f819685a96cb4785423a5b2303d147719e273e
src/slave/flags.hpp 0858bdfa93f0c82c9654f9a11d1a251df2d41c36
src/slave/flags.cpp 6e8872e435c9a5d895539e3cd47b41f66bc4eb89
Diff: https://reviews.apache.org/r/39417/diff/
Testing
-------
Manual tests, with and without a pre-installed HTB qdisc and classes.
Thanks,
Cong Wang
Re: Review Request 39417: Add --egress_flow_classifier_parent flag
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39417/#review107485
-----------------------------------------------------------
Bad patch!
Reviews applied: [40497, 40506, 39415, 39416, 39417]
Failed command: ./support/apply-review.sh -n -r 39417
Error:
2015-11-21 09:49:48 URL:https://reviews.apache.org/r/39417/diff/raw/ [9414/9414] -> "39417.patch" [1]
error: patch failed: src/slave/containerizer/mesos/isolators/network/port_mapping.cpp:2388
error: src/slave/containerizer/mesos/isolators/network/port_mapping.cpp: patch does not apply
Failed to apply patch
- Mesos ReviewBot
On Nov. 20, 2015, 10:27 p.m., Cong Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39417/
> -----------------------------------------------------------
>
> (Updated Nov. 20, 2015, 10:27 p.m.)
>
>
> Review request for mesos, Ian Downes and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When --egress_unique_flow_per_container is enabled, we need to install a flow classifier (fq_codel) qdisc on egress side. This flag specifies where to install it in the hierarchy. By default, we install it at root. But, for example, if you have already installed HTB qdisc at root, you may want this to be installed in other place than root, specify an HTB class ID as its parent here.
>
>
> Diffs
> -----
>
> docs/configuration.md 126f4aa8e3de2a2346336991c9b9a2ea61a8cd0a
> src/slave/containerizer/mesos/isolators/network/port_mapping.cpp e50616fd609588c547c90bba6d7b3b9b3eb4c6a9
> src/slave/flags.hpp 6ae7c94d2e05d81c8b970e7dcaa82d8aa4de7936
> src/slave/flags.cpp 26f554e1a73f1f7f3d7baec7bfc1a7f456c5677c
>
> Diff: https://reviews.apache.org/r/39417/diff/
>
>
> Testing
> -------
>
> Manual tests, with and without a pre-installed HTB qdisc and classes.
>
>
> Thanks,
>
> Cong Wang
>
>
Re: Review Request 39417: Add --egress_flow_classifier_parent flag
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39417/#review107443
-----------------------------------------------------------
src/slave/containerizer/mesos/isolators/network/port_mapping.cpp (lines 390 - 414)
<https://reviews.apache.org/r/39417/#comment166548>
Can you introduce a static method in Handle (src/linux/routing/handle.hpp)
```
Try<Handle> Handle::parse(const string& value);
```
src/slave/containerizer/mesos/isolators/network/port_mapping.cpp (line 1416)
<https://reviews.apache.org/r/39417/#comment166549>
s/parent/egressParentHandle/
src/slave/containerizer/mesos/isolators/network/port_mapping.cpp (line 1418)
<https://reviews.apache.org/r/39417/#comment166550>
Failed to parse egress_flow_classifier_parent: ...
src/slave/containerizer/mesos/isolators/network/port_mapping.cpp (line 1421)
<https://reviews.apache.org/r/39417/#comment166551>
No need for a temp variable. Simply use egressParentHandle.get()
src/slave/containerizer/mesos/isolators/network/port_mapping.cpp (line 1423)
<https://reviews.apache.org/r/39417/#comment166553>
We typically do not update global variables. Can you make it an instance member of the isolator class and keep the constant HOST_TX_FQ_CODEL_HANDLE
src/slave/containerizer/mesos/isolators/network/port_mapping.cpp (lines 1425 - 1426)
<https://reviews.apache.org/r/39417/#comment166552>
Please add a TODO(cwang) here.
- Jie Yu
On Nov. 20, 2015, 10:27 p.m., Cong Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39417/
> -----------------------------------------------------------
>
> (Updated Nov. 20, 2015, 10:27 p.m.)
>
>
> Review request for mesos, Ian Downes and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When --egress_unique_flow_per_container is enabled, we need to install a flow classifier (fq_codel) qdisc on egress side. This flag specifies where to install it in the hierarchy. By default, we install it at root. But, for example, if you have already installed HTB qdisc at root, you may want this to be installed in other place than root, specify an HTB class ID as its parent here.
>
>
> Diffs
> -----
>
> docs/configuration.md 126f4aa8e3de2a2346336991c9b9a2ea61a8cd0a
> src/slave/containerizer/mesos/isolators/network/port_mapping.cpp e50616fd609588c547c90bba6d7b3b9b3eb4c6a9
> src/slave/flags.hpp 6ae7c94d2e05d81c8b970e7dcaa82d8aa4de7936
> src/slave/flags.cpp 26f554e1a73f1f7f3d7baec7bfc1a7f456c5677c
>
> Diff: https://reviews.apache.org/r/39417/diff/
>
>
> Testing
> -------
>
> Manual tests, with and without a pre-installed HTB qdisc and classes.
>
>
> Thanks,
>
> Cong Wang
>
>
Re: Review Request 39417: Add --egress_flow_classifier_parent flag
Posted by Cong Wang <cw...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39417/
-----------------------------------------------------------
(Updated Nov. 20, 2015, 10:27 p.m.)
Review request for mesos, Ian Downes and Jie Yu.
Changes
-------
Rebase
Repository: mesos
Description
-------
When --egress_unique_flow_per_container is enabled, we need to install a flow classifier (fq_codel) qdisc on egress side. This flag specifies where to install it in the hierarchy. By default, we install it at root. But, for example, if you have already installed HTB qdisc at root, you may want this to be installed in other place than root, specify an HTB class ID as its parent here.
Diffs (updated)
-----
docs/configuration.md 126f4aa8e3de2a2346336991c9b9a2ea61a8cd0a
src/slave/containerizer/mesos/isolators/network/port_mapping.cpp e50616fd609588c547c90bba6d7b3b9b3eb4c6a9
src/slave/flags.hpp 6ae7c94d2e05d81c8b970e7dcaa82d8aa4de7936
src/slave/flags.cpp 26f554e1a73f1f7f3d7baec7bfc1a7f456c5677c
Diff: https://reviews.apache.org/r/39417/diff/
Testing
-------
Manual tests, with and without a pre-installed HTB qdisc and classes.
Thanks,
Cong Wang
Re: Review Request 39417: Add --egress_flow_classifier_parent flag
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39417/#review107322
-----------------------------------------------------------
Bad patch!
Reviews applied: [39415, 39416, 40497, 40506, 39417]
Failed command: ./support/apply-review.sh -n -r 39417
Error:
2015-11-20 07:23:28 URL:https://reviews.apache.org/r/39417/diff/raw/ [9792/9792] -> "39417.patch" [1]
error: patch failed: src/slave/containerizer/mesos/isolators/network/port_mapping.cpp:1385
error: src/slave/containerizer/mesos/isolators/network/port_mapping.cpp: patch does not apply
Failed to apply patch
- Mesos ReviewBot
On Nov. 19, 2015, 10:59 p.m., Cong Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39417/
> -----------------------------------------------------------
>
> (Updated Nov. 19, 2015, 10:59 p.m.)
>
>
> Review request for mesos, Ian Downes and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When --egress_unique_flow_per_container is enabled, we need to install a flow classifier (fq_codel) qdisc on egress side. This flag specifies where to install it in the hierarchy. By default, we install it at root. But, for example, if you have already installed HTB qdisc at root, you may want this to be installed in other place than root, specify an HTB class ID as its parent here.
>
>
> Diffs
> -----
>
> docs/configuration.md 72847e5efe7008fdec8287cce100857f9e7c0fe0
> src/slave/containerizer/mesos/isolators/network/port_mapping.cpp cc0152ce44e9fa34210efda8a1c4a6374167eab1
> src/slave/flags.hpp 6ae7c94d2e05d81c8b970e7dcaa82d8aa4de7936
> src/slave/flags.cpp 26f554e1a73f1f7f3d7baec7bfc1a7f456c5677c
>
> Diff: https://reviews.apache.org/r/39417/diff/
>
>
> Testing
> -------
>
> Manual tests, with and without a pre-installed HTB qdisc and classes.
>
>
> Thanks,
>
> Cong Wang
>
>
Re: Review Request 39417: Add --egress_flow_classifier_parent flag
Posted by Cong Wang <cw...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39417/
-----------------------------------------------------------
(Updated Nov. 19, 2015, 10:59 p.m.)
Review request for mesos, Ian Downes and Jie Yu.
Changes
-------
Rebase
Repository: mesos
Description
-------
When --egress_unique_flow_per_container is enabled, we need to install a flow classifier (fq_codel) qdisc on egress side. This flag specifies where to install it in the hierarchy. By default, we install it at root. But, for example, if you have already installed HTB qdisc at root, you may want this to be installed in other place than root, specify an HTB class ID as its parent here.
Diffs (updated)
-----
docs/configuration.md 72847e5efe7008fdec8287cce100857f9e7c0fe0
src/slave/containerizer/mesos/isolators/network/port_mapping.cpp cc0152ce44e9fa34210efda8a1c4a6374167eab1
src/slave/flags.hpp 6ae7c94d2e05d81c8b970e7dcaa82d8aa4de7936
src/slave/flags.cpp 26f554e1a73f1f7f3d7baec7bfc1a7f456c5677c
Diff: https://reviews.apache.org/r/39417/diff/
Testing
-------
Manual tests, with and without a pre-installed HTB qdisc and classes.
Thanks,
Cong Wang
Re: Review Request 39417: Add --egress_flow_classifier_parent flag
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39417/#review107247
-----------------------------------------------------------
Bad patch!
Reviews applied: [40497]
Failed command: ./support/apply-review.sh -n -r 40497
Error:
2015-11-19 21:53:08 URL:https://reviews.apache.org/r/40497/diff/raw/ [4367/4367] -> "40497.patch" [1]
Successfully applied: Add hex number support to numify()
numify() in libprocess uses boost lexical_cast() to cast numbers, but it can not cast a hex number. This patch adds hex support to numify().
Review: https://reviews.apache.org/r/40497
Checking 2 files using filter --filter=-,+build/class,+build/deprecated,+build/endif_comment,+readability/todo,+readability/namespace,+runtime/vlog,+whitespace/blank_line,+whitespace/comma,+whitespace/end_of_line,+whitespace/ending_newline,+whitespace/forcolon,+whitespace/indent,+whitespace/line_length,+whitespace/operators,+whitespace/semicolon,+whitespace/tab,+whitespace/todo
Total errors found: 0
ERROR: Commit spanning multiple projects.
Please use separate commits for mesos, libprocess and stout.
Paths grouped by project:
stout:
3rdparty/libprocess/3rdparty/stout/Makefile.am
3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp
3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt
3rdparty/libprocess/3rdparty/stout/tests/numify_tests.cpp
libprocess:
3rdparty/libprocess/3rdparty/Makefile.am
Failed to commit patch
- Mesos ReviewBot
On Nov. 19, 2015, 8:15 p.m., Cong Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39417/
> -----------------------------------------------------------
>
> (Updated Nov. 19, 2015, 8:15 p.m.)
>
>
> Review request for mesos, Ian Downes and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When --egress_unique_flow_per_container is enabled, we need to install a flow classifier (fq_codel) qdisc on egress side. This flag specifies where to install it in the hierarchy. By default, we install it at root. But, for example, if you have already installed HTB qdisc at root, you may want this to be installed in other place than root, specify an HTB class ID as its parent here.
>
>
> Diffs
> -----
>
> docs/configuration.md 72847e5efe7008fdec8287cce100857f9e7c0fe0
> src/slave/containerizer/mesos/isolators/network/port_mapping.cpp cc0152ce44e9fa34210efda8a1c4a6374167eab1
> src/slave/flags.hpp 6ae7c94d2e05d81c8b970e7dcaa82d8aa4de7936
> src/slave/flags.cpp 26f554e1a73f1f7f3d7baec7bfc1a7f456c5677c
>
> Diff: https://reviews.apache.org/r/39417/diff/
>
>
> Testing
> -------
>
> Manual tests, with and without a pre-installed HTB qdisc and classes.
>
>
> Thanks,
>
> Cong Wang
>
>
Re: Review Request 39417: Add --egress_flow_classifier_parent flag
Posted by Cong Wang <cw...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39417/
-----------------------------------------------------------
(Updated Nov. 19, 2015, 8:15 p.m.)
Review request for mesos, Ian Downes and Jie Yu.
Changes
-------
Use numify()
Repository: mesos
Description
-------
When --egress_unique_flow_per_container is enabled, we need to install a flow classifier (fq_codel) qdisc on egress side. This flag specifies where to install it in the hierarchy. By default, we install it at root. But, for example, if you have already installed HTB qdisc at root, you may want this to be installed in other place than root, specify an HTB class ID as its parent here.
Diffs (updated)
-----
docs/configuration.md 72847e5efe7008fdec8287cce100857f9e7c0fe0
src/slave/containerizer/mesos/isolators/network/port_mapping.cpp cc0152ce44e9fa34210efda8a1c4a6374167eab1
src/slave/flags.hpp 6ae7c94d2e05d81c8b970e7dcaa82d8aa4de7936
src/slave/flags.cpp 26f554e1a73f1f7f3d7baec7bfc1a7f456c5677c
Diff: https://reviews.apache.org/r/39417/diff/
Testing
-------
Manual tests, with and without a pre-installed HTB qdisc and classes.
Thanks,
Cong Wang
Re: Review Request 39417: Add --egress_flow_classifier_parent flag
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39417/#review104383
-----------------------------------------------------------
Patch looks great!
Reviews applied: [39415, 39416, 39417]
All tests passed.
- Mesos ReviewBot
On Oct. 29, 2015, 12:12 a.m., Cong Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39417/
> -----------------------------------------------------------
>
> (Updated Oct. 29, 2015, 12:12 a.m.)
>
>
> Review request for mesos, Ian Downes and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When --egress_unique_flow_per_container is enabled, we need to install a flow classifier (fq_codel) qdisc on egress side. This flag specifies where to install it in the hierarchy. By default, we install it at root. But, for example, if you have already installed HTB qdisc at root, you may want this to be installed in other place than root, specify an HTB class ID as its parent here.
>
>
> Diffs
> -----
>
> docs/configuration.md ae9f2612b11447eff92ea85d4191e7011d71b2b2
> src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 1911ba6518190cbff6d72b56aaa477d508dbd391
> src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73
> src/slave/flags.cpp 1362dcebef799399739025171696230bded447dd
>
> Diff: https://reviews.apache.org/r/39417/diff/
>
>
> Testing
> -------
>
> Manual tests, with and without a pre-installed HTB qdisc and classes.
>
>
> Thanks,
>
> Cong Wang
>
>
Re: Review Request 39417: Add --egress_flow_classifier_parent flag
Posted by Cong Wang <cw...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39417/
-----------------------------------------------------------
(Updated Oct. 29, 2015, 12:12 a.m.)
Review request for mesos, Ian Downes and Jie Yu.
Changes
-------
Address review comments and rebase
Repository: mesos
Description
-------
When --egress_unique_flow_per_container is enabled, we need to install a flow classifier (fq_codel) qdisc on egress side. This flag specifies where to install it in the hierarchy. By default, we install it at root. But, for example, if you have already installed HTB qdisc at root, you may want this to be installed in other place than root, specify an HTB class ID as its parent here.
Diffs (updated)
-----
docs/configuration.md ae9f2612b11447eff92ea85d4191e7011d71b2b2
src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 1911ba6518190cbff6d72b56aaa477d508dbd391
src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73
src/slave/flags.cpp 1362dcebef799399739025171696230bded447dd
Diff: https://reviews.apache.org/r/39417/diff/
Testing
-------
Manual tests, with and without a pre-installed HTB qdisc and classes.
Thanks,
Cong Wang
Re: Review Request 39417: Add --egress_flow_classifier_parent flag
Posted by Cong Wang <cw...@twopensource.com>.
> On Oct. 27, 2015, 12:20 a.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, lines 1423-1424
> > <https://reviews.apache.org/r/39417/diff/1/?file=1100528#file1100528line1423>
> >
> > Can we verify this somehow? What's the failure mode?
Yeah, I can test it manually, in that case the following qdisc creation would just fail.
- Cong
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39417/#review104119
-----------------------------------------------------------
On Oct. 18, 2015, 12:29 a.m., Cong Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39417/
> -----------------------------------------------------------
>
> (Updated Oct. 18, 2015, 12:29 a.m.)
>
>
> Review request for mesos, Ian Downes and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When --egress_unique_flow_per_container is enabled, we need to install a flow classifier (fq_codel) qdisc on egress side. This flag specifies where to install it in the hierarchy. By default, we install it at root. But, for example, if you have already installed HTB qdisc at root, you may want this to be installed in other place than root, specify an HTB class ID as its parent here.
>
>
> Diffs
> -----
>
> docs/configuration.md 9443d5fc2de0f46f9e117b08e29b09ff3a4579c6
> src/slave/containerizer/isolators/network/port_mapping.cpp e6bb75e6f5ba48a0c4cf6dd8f353e5f5843d0eef
> src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73
> src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2
>
> Diff: https://reviews.apache.org/r/39417/diff/
>
>
> Testing
> -------
>
> Manual tests, with and without a pre-installed HTB qdisc and classes.
>
>
> Thanks,
>
> Cong Wang
>
>
Re: Review Request 39417: Add --egress_flow_classifier_parent flag
Posted by Cong Wang <cw...@twopensource.com>.
> On Oct. 27, 2015, 12:20 a.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, lines 401-405
> > <https://reviews.apache.org/r/39417/diff/1/?file=1100528#file1100528line401>
> >
> > Why not add a 0x prefix if it's not present so you can use numify?
> >
> > if (!strings::startsWith(tokens[0], "0x")) {
> > ...
> > }
>
> Cong Wang wrote:
> This should work too, but I don't feel it is better than my code.
>
> Ian Downes wrote:
> I really think it's necessary to use stout's conversion functions here.
Sure, I will change it, although I still don't have any preference.
- Cong
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39417/#review104119
-----------------------------------------------------------
On Oct. 29, 2015, 12:12 a.m., Cong Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39417/
> -----------------------------------------------------------
>
> (Updated Oct. 29, 2015, 12:12 a.m.)
>
>
> Review request for mesos, Ian Downes and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When --egress_unique_flow_per_container is enabled, we need to install a flow classifier (fq_codel) qdisc on egress side. This flag specifies where to install it in the hierarchy. By default, we install it at root. But, for example, if you have already installed HTB qdisc at root, you may want this to be installed in other place than root, specify an HTB class ID as its parent here.
>
>
> Diffs
> -----
>
> docs/configuration.md ae9f2612b11447eff92ea85d4191e7011d71b2b2
> src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 1911ba6518190cbff6d72b56aaa477d508dbd391
> src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73
> src/slave/flags.cpp 1362dcebef799399739025171696230bded447dd
>
> Diff: https://reviews.apache.org/r/39417/diff/
>
>
> Testing
> -------
>
> Manual tests, with and without a pre-installed HTB qdisc and classes.
>
>
> Thanks,
>
> Cong Wang
>
>
Re: Review Request 39417: Add --egress_flow_classifier_parent flag
Posted by Ian Downes <ia...@gmail.com>.
> On Oct. 26, 2015, 5:20 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, lines 401-405
> > <https://reviews.apache.org/r/39417/diff/1/?file=1100528#file1100528line401>
> >
> > Why not add a 0x prefix if it's not present so you can use numify?
> >
> > if (!strings::startsWith(tokens[0], "0x")) {
> > ...
> > }
>
> Cong Wang wrote:
> This should work too, but I don't feel it is better than my code.
I really think it's necessary to use stout's conversion functions here.
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39417/#review104119
-----------------------------------------------------------
On Oct. 28, 2015, 5:12 p.m., Cong Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39417/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2015, 5:12 p.m.)
>
>
> Review request for mesos, Ian Downes and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When --egress_unique_flow_per_container is enabled, we need to install a flow classifier (fq_codel) qdisc on egress side. This flag specifies where to install it in the hierarchy. By default, we install it at root. But, for example, if you have already installed HTB qdisc at root, you may want this to be installed in other place than root, specify an HTB class ID as its parent here.
>
>
> Diffs
> -----
>
> docs/configuration.md ae9f2612b11447eff92ea85d4191e7011d71b2b2
> src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 1911ba6518190cbff6d72b56aaa477d508dbd391
> src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73
> src/slave/flags.cpp 1362dcebef799399739025171696230bded447dd
>
> Diff: https://reviews.apache.org/r/39417/diff/
>
>
> Testing
> -------
>
> Manual tests, with and without a pre-installed HTB qdisc and classes.
>
>
> Thanks,
>
> Cong Wang
>
>
Re: Review Request 39417: Add --egress_flow_classifier_parent flag
Posted by Cong Wang <cw...@twopensource.com>.
> On Oct. 27, 2015, 12:20 a.m., Ian Downes wrote:
> > docs/configuration.md, lines 1545-1551
> > <https://reviews.apache.org/r/39417/diff/1/?file=1100527#file1100527line1545>
> >
> > Can we not infer where to put the classifier from any exisiting classifiers and only require the user to specify its parent if they want particular behavior?
The default is root if this flag is not set, just as before. So this flag is only required when people don't want root.
- Cong
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39417/#review104119
-----------------------------------------------------------
On Oct. 18, 2015, 12:29 a.m., Cong Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39417/
> -----------------------------------------------------------
>
> (Updated Oct. 18, 2015, 12:29 a.m.)
>
>
> Review request for mesos, Ian Downes and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When --egress_unique_flow_per_container is enabled, we need to install a flow classifier (fq_codel) qdisc on egress side. This flag specifies where to install it in the hierarchy. By default, we install it at root. But, for example, if you have already installed HTB qdisc at root, you may want this to be installed in other place than root, specify an HTB class ID as its parent here.
>
>
> Diffs
> -----
>
> docs/configuration.md 9443d5fc2de0f46f9e117b08e29b09ff3a4579c6
> src/slave/containerizer/isolators/network/port_mapping.cpp e6bb75e6f5ba48a0c4cf6dd8f353e5f5843d0eef
> src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73
> src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2
>
> Diff: https://reviews.apache.org/r/39417/diff/
>
>
> Testing
> -------
>
> Manual tests, with and without a pre-installed HTB qdisc and classes.
>
>
> Thanks,
>
> Cong Wang
>
>
Re: Review Request 39417: Add --egress_flow_classifier_parent flag
Posted by Cong Wang <cw...@twopensource.com>.
> On Oct. 27, 2015, 12:20 a.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, lines 401-405
> > <https://reviews.apache.org/r/39417/diff/1/?file=1100528#file1100528line401>
> >
> > Why not add a 0x prefix if it's not present so you can use numify?
> >
> > if (!strings::startsWith(tokens[0], "0x")) {
> > ...
> > }
This should work too, but I don't feel it is better than my code.
> On Oct. 27, 2015, 12:20 a.m., Ian Downes wrote:
> > src/slave/flags.cpp, lines 506-513
> > <https://reviews.apache.org/r/39417/diff/1/?file=1100530#file1100530line506>
> >
> > Why is this description different from the docs?
This is a simpler one, details are in docs.
- Cong
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39417/#review104119
-----------------------------------------------------------
On Oct. 18, 2015, 12:29 a.m., Cong Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39417/
> -----------------------------------------------------------
>
> (Updated Oct. 18, 2015, 12:29 a.m.)
>
>
> Review request for mesos, Ian Downes and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When --egress_unique_flow_per_container is enabled, we need to install a flow classifier (fq_codel) qdisc on egress side. This flag specifies where to install it in the hierarchy. By default, we install it at root. But, for example, if you have already installed HTB qdisc at root, you may want this to be installed in other place than root, specify an HTB class ID as its parent here.
>
>
> Diffs
> -----
>
> docs/configuration.md 9443d5fc2de0f46f9e117b08e29b09ff3a4579c6
> src/slave/containerizer/isolators/network/port_mapping.cpp e6bb75e6f5ba48a0c4cf6dd8f353e5f5843d0eef
> src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73
> src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2
>
> Diff: https://reviews.apache.org/r/39417/diff/
>
>
> Testing
> -------
>
> Manual tests, with and without a pre-installed HTB qdisc and classes.
>
>
> Thanks,
>
> Cong Wang
>
>
Re: Review Request 39417: Add --egress_flow_classifier_parent flag
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39417/#review104119
-----------------------------------------------------------
docs/configuration.md (lines 1545 - 1551)
<https://reviews.apache.org/r/39417/#comment162363>
Can we not infer where to put the classifier from any exisiting classifiers and only require the user to specify its parent if they want particular behavior?
src/slave/containerizer/isolators/network/port_mapping.cpp (line 398)
<https://reviews.apache.org/r/39417/#comment162359>
s/Failed tokenize/Failed to tokenize/
src/slave/containerizer/isolators/network/port_mapping.cpp (lines 401 - 405)
<https://reviews.apache.org/r/39417/#comment162360>
Why not add a 0x prefix if it's not present so you can use numify?
if (!strings::startsWith(tokens[0], "0x")) {
...
}
src/slave/containerizer/isolators/network/port_mapping.cpp (lines 407 - 409)
<https://reviews.apache.org/r/39417/#comment162361>
This will get refactored as above, but it's more helpful to specifically state which token couldn't be parsed.
src/slave/containerizer/isolators/network/port_mapping.cpp (line 1415)
<https://reviews.apache.org/r/39417/#comment162364>
kill this newline so the error checking is paired with the Try<>
src/slave/containerizer/isolators/network/port_mapping.cpp (line 1419)
<https://reviews.apache.org/r/39417/#comment162365>
add a newline in here
src/slave/containerizer/isolators/network/port_mapping.cpp (lines 1423 - 1424)
<https://reviews.apache.org/r/39417/#comment162366>
Can we verify this somehow? What's the failure mode?
src/slave/flags.cpp (lines 506 - 513)
<https://reviews.apache.org/r/39417/#comment162362>
Why is this description different from the docs?
- Ian Downes
On Oct. 17, 2015, 5:29 p.m., Cong Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39417/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2015, 5:29 p.m.)
>
>
> Review request for mesos, Ian Downes and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When --egress_unique_flow_per_container is enabled, we need to install a flow classifier (fq_codel) qdisc on egress side. This flag specifies where to install it in the hierarchy. By default, we install it at root. But, for example, if you have already installed HTB qdisc at root, you may want this to be installed in other place than root, specify an HTB class ID as its parent here.
>
>
> Diffs
> -----
>
> docs/configuration.md 9443d5fc2de0f46f9e117b08e29b09ff3a4579c6
> src/slave/containerizer/isolators/network/port_mapping.cpp e6bb75e6f5ba48a0c4cf6dd8f353e5f5843d0eef
> src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73
> src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2
>
> Diff: https://reviews.apache.org/r/39417/diff/
>
>
> Testing
> -------
>
> Manual tests, with and without a pre-installed HTB qdisc and classes.
>
>
> Thanks,
>
> Cong Wang
>
>