----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19702/#review39422 -----------------------------------------------------------
configure.ac <https://reviews.apache.org/r/19702/#comment71812> i think it's unnecessary to make this so bold. it's already an error so a simple message is better: Network isolator is only supported on Linux. src/linux/routing.hpp <https://reviews.apache.org/r/19702/#comment71813> __LINUX_ROUTING_HPP__ ? src/linux/routing.hpp <https://reviews.apache.org/r/19702/#comment71814> consider putting each namespace in a separate header in a routing/ folder instead. src/linux/routing.hpp <https://reviews.apache.org/r/19702/#comment71815> why are these extern? src/linux/routing.hpp <https://reviews.apache.org/r/19702/#comment71817> explicit src/linux/routing.hpp <https://reviews.apache.org/r/19702/#comment71816> const std::string& return src/linux/routing.hpp <https://reviews.apache.org/r/19702/#comment71818> explicit src/linux/routing.hpp <https://reviews.apache.org/r/19702/#comment71819> const std::set<std::string>& return src/linux/routing.cpp <https://reviews.apache.org/r/19702/#comment71820> consider an internal namespace for this if it's only used in this translation unit. maybe class ManagedNetlink ? in c++11 this should be a std::unique_ptr with a custom deleter. you might want a TODO. src/linux/routing.cpp <https://reviews.apache.org/r/19702/#comment71821> this isn't C - move these declarations down to where you use them first. src/linux/routing.cpp <https://reviews.apache.org/r/19702/#comment71822> else { return !link.isNone(); } src/linux/routing.cpp <https://reviews.apache.org/r/19702/#comment71824> these magic number offsets should be constants src/linux/routing.cpp <https://reviews.apache.org/r/19702/#comment71826> it seems we really need some support for this pattern as a macro somewhere. - Dominic Hamon On April 2, 2014, 7:15 p.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19702/ > ----------------------------------------------------------- > > (Updated April 2, 2014, 7:15 p.m.) > > > Review request for mesos, Benjamin Hindman, Chi Zhang, Ian Downes, Vinod > Kone, and Cong Wang. > > > Repository: mesos-git > > > Description > ------- > > UPDATE: > > Added impl. (including tests) for managing IP packet filter. > > ------ > > 1) adjusted a few interfaces per review comments. > 2) added impl. (including tests) for managing links. > > I'll be adding impl. for managing filters soon (currently, they return > Error("Unimplemented").) > > > ------ > > Hey guys, I send this review in order to get an idea about the interface > design. > > Feel free to jump in to express your thoughts, suggestions, concerns, etc. > > Thanks! > > > Diffs > ----- > > configure.ac 5404dc2 > src/Makefile.am 95f133d > src/linux/routing.hpp PRE-CREATION > src/linux/routing.cpp PRE-CREATION > src/tests/routing_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/19702/diff/ > > > Testing > ------- > > make check > > sudo make check > > > Thanks, > > Jie Yu > >
