----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31502/#review74774 -----------------------------------------------------------
Ship it! Looking great! src/linux/routing/queueing/fq_codel.cpp <https://reviews.apache.org/r/31502/#comment121512> Please add a TODO here saying that we don't expose the fq_codel queueing discipline because we use all the default parameters currently. src/linux/routing/queueing/fq_codel.cpp <https://reviews.apache.org/r/31502/#comment121528> What if some one already installed an egress qdisc on the link? I think this is much more likely than ingress qdisc. Maybe use a handle that's more unlikely to be picked (e.g., Handle(0x1, 0x800)) for now and add a TODO saying that we may want to expose the handle in the interfaces and instead let the user choose the handle. src/linux/routing/queueing/fq_codel.cpp <https://reviews.apache.org/r/31502/#comment121531> This looks a little hacky right now. Ideally, this should be exposed to the user in fq_codel::Disipline and the user should be allowed to configure this parameter. The number 1024 is the default value used by the qdisc. How about add a TODO (basically saying the above) and rename it to `DEFAULT_FLOWS`. src/tests/routing_tests.cpp <https://reviews.apache.org/r/31502/#comment121532> Two lines apart between top level functions. src/tests/routing_tests.cpp <https://reviews.apache.org/r/31502/#comment121533> Ditto here. - Jie Yu On Feb. 27, 2015, 9:38 p.m., Cong Wang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31502/ > ----------------------------------------------------------- > > (Updated Feb. 27, 2015, 9:38 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 > ------- > > Fq_codel qdisc is needed for egress flow classification. Internally fq_codel > defines its own flows, and it allows us to specifiy the rule to classify the > flows with tc filters attached on it. By default it has 1024 flows, much more > than the number of containers we could create on a single host. > > > Diffs > ----- > > src/Makefile.am 17d0d7aa7361c3a373f6863d36b0a4767f5c05c4 > src/linux/routing/queueing/fq_codel.hpp PRE-CREATION > src/linux/routing/queueing/fq_codel.cpp PRE-CREATION > src/tests/routing_tests.cpp 3cda6ab8c1ad24e4b7d0b9aeda2abc595fc857a5 > > Diff: https://reviews.apache.org/r/31502/diff/ > > > Testing > ------- > > Run the testcase. > > > Thanks, > > Cong Wang > >
