> On April 15, 2014, 6:05 p.m., Ian Downes wrote:
> > src/linux/routing/filter/action.hpp, line 39
> > <https://reviews.apache.org/r/20295/diff/1/?file=555697#file555697line39>
> >
> >     const?

I need to put then into std containers. Declaring const will make the default 
assignment operator not working.


> On April 15, 2014, 6:05 p.m., Ian Downes wrote:
> > src/linux/routing/filter/icmp.hpp, line 38
> > <https://reviews.apache.org/r/20295/diff/1/?file=555698#file555698line38>
> >
> >     what about dropping the Ip in dstIp since it's clear from the type, 
> > i.e., just calling it 'destination'?
> >     
> >     Can you document what it means for the IP address to be optional?

Changed it to destinationIP. I wanna keep 'IP' here because we may wanna 
introduce destinationMAC:)


> On April 15, 2014, 6:05 p.m., Ian Downes wrote:
> > src/linux/routing/filter/internal.hpp, line 470
> > <https://reviews.apache.org/r/20295/diff/1/?file=555700#file555700line470>
> >
> >     s/oldCls/oldClassifier/?

I have the following convention in the code:

libnl related -> cls, act, etc.
our representation -> classifier, action, etc.


> On April 15, 2014, 6:05 p.m., Ian Downes wrote:
> > src/tests/routing_tests.cpp, line 383
> > <https://reviews.apache.org/r/20295/diff/1/?file=555702#file555702line383>
> >
> >     Can we write tests that confirm functionality, e.g., can we ping the 
> > interfaces? (see suggestion for IPs on 127/8)

Yeah, will follow up with more sophisticated tests!


> On April 15, 2014, 6:05 p.m., Ian Downes wrote:
> > src/tests/routing_tests.cpp, line 290
> > <https://reviews.apache.org/r/20295/diff/1/?file=555702#file555702line290>
> >
> >     we probably shouldn't be using routable IPs. What about 127.1.2.3 and 
> > 127.4.5.6 so we're on the local 127.0.0.0/8?

It's OK. I just want to check if filters are setup properly. Since all filters 
are attached to veth-test, no packet will be flowing around.


- Jie


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


On April 13, 2014, 11:45 p.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20295/
> -----------------------------------------------------------
> 
> (Updated April 13, 2014, 11:45 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Chi Zhang, Ian Downes, Vinod 
> Kone, and Cong Wang.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> This is Part 2 of the linux routing library.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 560b4c7 
>   src/linux/routing/filter/action.hpp PRE-CREATION 
>   src/linux/routing/filter/icmp.hpp PRE-CREATION 
>   src/linux/routing/filter/icmp.cpp PRE-CREATION 
>   src/linux/routing/filter/internal.hpp PRE-CREATION 
>   src/linux/routing/filter/priority.hpp PRE-CREATION 
>   src/tests/routing_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/20295/diff/
> 
> 
> Testing
> -------
> 
> make check
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Reply via email to