----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33040/#review79776 -----------------------------------------------------------
configure.ac <https://reviews.apache.org/r/33040/#comment129351> See my comments. You don't need that anymore. src/linux/routing/queueing/internal.hpp <https://reviews.apache.org/r/33040/#comment129338> The build machine might have libnl3.2.26 while the production machine might not. So I would rather not use ifdef here. Simply use the first version and get rid of any dependency to rtnl_tc_stat2str. Adding a TODO here is sufficient. Also, this is header file, the compiler will create a copy of 'tcNames' in each cpp file that includes this header. Please avoid that. See my comments below. src/linux/routing/queueing/internal.hpp <https://reviews.apache.org/r/33040/#comment129346> Please be consistent with the link::get above: ``` if (qdisc.isError()) { ... } else if (qdisk.isNone()) { ... } ``` src/linux/routing/queueing/internal.hpp <https://reviews.apache.org/r/33040/#comment129350> You probably want to create a hashmap: ``` // TODO(..): This is a workaround... hashmap<int, std::string> names; names[(int) RTNL_TC_PACKETS] = "packets"; names[(int) RTNL_TC_BYTES] = "bytes"; ... names[(int) RTNL_TC_OVERLIMITS] = "overlimits"; hashmap<std::string, uint64_t> results; for(int i = 0; i < (int) RTNL_TC_STATS_MAX; i++) { results[names[i]] = rtnl_tc_get_stat( TC_CAST(qdisc.get().get(), (rtnl_tc_stat) i); } return results; ``` - Jie Yu On April 10, 2015, 11:17 p.m., Paul Brett wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33040/ > ----------------------------------------------------------- > > (Updated April 10, 2015, 11:17 p.m.) > > > Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. > > > Bugs: mesos-2332 > https://issues.apache.org/jira/browse/mesos-2332 > > > Repository: mesos > > > Description > ------- > > Expose qdisc statistics from libnl > > > Diffs > ----- > > configure.ac 868c041 > src/linux/routing/queueing/fq_codel.hpp 4f67ab7 > src/linux/routing/queueing/fq_codel.cpp 02ad8df > src/linux/routing/queueing/ingress.hpp b323a7f > src/linux/routing/queueing/ingress.cpp 47c7337 > src/linux/routing/queueing/internal.hpp 7c6c4d3 > src/tests/routing_tests.cpp 7cc3b57 > > Diff: https://reviews.apache.org/r/33040/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Paul Brett > >
