-----------------------------------------------------------
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
> 
>

Reply via email to