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


Some high level comments. Let's chat about that.


src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/31505/#comment122974>

    I am rethinking about the API about flows and here are my thoughts. I think 
this is pretty important for readability.
    
    Each `Flow` action needs to have a parent and an ID. So the interface is 
like the following:
    ```
    class Flow
    {
      // NOTE: 'id' must be great than zero.
      Flow(const Handle& parent, uint16_t id);
      
      // Construct a flow from 32 bit integer. This
      // constructor is only used by filter internals.
      Flow(uint32_t classid);
      
      uint16_t id();
    };
    ```
    
    So `parent` is the qdisc handle, and `id` is any 16 bit integer greater 
than 0.



src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/31505/#comment122976>

    I agree with Chi that the API is a little confusing and the code is hard to 
read and follow here.
    
    IMO, `flowId()` shouldn't be part of `ip:Classifier`'s interfaces because 
it has nothing to do with "classifying a packet".
    
    Here are my suggestions. We need to expose a new interface 
`vector<Filter<ip::Classifier>> filters(link, handle);` (it's already there in 
the internal namesapce, we just need to expose it).
    
    We can get the action attached to the filter and then we can get the flow 
id from the action.
    
    Let's chat about this.


- Jie Yu


On March 9, 2015, 6:57 p.m., Cong Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31505/
> -----------------------------------------------------------
> 
> (Updated March 9, 2015, 6:57 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 do nothing on the host egress side. By default, kernel uses its 
> own hash function to classify the packets to different TX queues (if the 
> hardware interface supports multiqueue). So packets coming out of different 
> containers could end up queueing in the same TX queue, in this case we saw 
> buffer bloat on some TX queue caused packet drops.
> 
> We need to isolation the egress traffic so that containers will not have 
> interference with each other. The number of hardware TX queues is limited by 
> hardware interface, usually not enough to map our container in 1:1 way, 
> therefore we need some software solution. We choose fq_codel and use tc 
> filters to classify packets coming out of different containers to different 
> fq_codel flows, and the codel algorithm on each flow could also help us to 
> reduce the buffer bloat. Note when the packets leave fq_codel, they still 
> share the physical TX queue(s), this is however (almost) beyond what we can 
> control, we have to rely on the kernel behavior.
> 
> TODO: get some performance numbers
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/isolators/network/port_mapping.hpp 0f9ad4a 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 7a38735 
> 
> Diff: https://reviews.apache.org/r/31505/diff/
> 
> 
> Testing
> -------
> 
> Manually start two mesos containers with netperf running side.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Reply via email to