----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31505/#review75271 -----------------------------------------------------------
This is a pretty signifant feature. I am pretty sure we need a slave flag to control it. This file has examples for that. src/slave/containerizer/isolators/network/port_mapping.hpp <https://reviews.apache.org/r/31505/#comment122239> I think we probably want to rename other counter names to account for both ingress and egree qdiscs, but that's probably another patch. src/slave/containerizer/isolators/network/port_mapping.cpp <https://reviews.apache.org/r/31505/#comment122241> looks like the library code uses uint32_t here but your set is uint16_t. assuming you have a reason to do that in the library, maybe just uint32_t in the client code as well? are there other caveats? it's weired for the library to support a 32bit type but the client takes the liberty to use a 16bit type. src/slave/containerizer/isolators/network/port_mapping.cpp <https://reviews.apache.org/r/31505/#comment122243> maybe just nextFlowId? I think this file has a section where all the isolatorprocess helper functions go into? mind putting it here? Just that this file is already quite long, i think a bit organization will help in the long run. also 2 blank lines before and after the function. ditto to other helper functions you created. src/slave/containerizer/isolators/network/port_mapping.cpp <https://reviews.apache.org/r/31505/#comment122245> Maybe we don't need to give a new flow at all here (update should be a noop for this feature?) This feature is all internal. update is about updating resources other components know about. To PortMappingIsolator it means a container's public listening ports. No one except us knows about the flowId story? src/slave/containerizer/isolators/network/port_mapping.cpp <https://reviews.apache.org/r/31505/#comment122246> I think you need a default value for the info->flowId, or better, make it an Option, then here you have to check it before setting? If for a new container, say, 'prepare' fails which happens before isolate, where you first initialize flowId, cleanup will be called to destroy it, in which case, an undefined value will be inserted to your set?? - Chi Zhang On March 4, 2015, 8:18 p.m., Cong Wang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31505/ > ----------------------------------------------------------- > > (Updated March 4, 2015, 8:18 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 8443097 > src/slave/containerizer/isolators/network/port_mapping.cpp 5227987 > > Diff: https://reviews.apache.org/r/31505/diff/ > > > Testing > ------- > > Manually start two mesos containers with netperf running side. > > > Thanks, > > Cong Wang > >
