You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Jie Yu <yu...@gmail.com> on 2014/04/14 01:50:09 UTC
Review Request 20297: Added API for managing IP packet filters.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20297/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman, Chi Zhang, Ian Downes, Vinod Kone, and Cong Wang.
Repository: mesos-git
Description
-------
The is Part 4 of the linux routing library.
Diffs
-----
src/Makefile.am 560b4c7
src/linux/routing/filter/ip.hpp PRE-CREATION
src/linux/routing/filter/ip.cpp PRE-CREATION
src/tests/routing_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/20297/diff/
Testing
-------
make check
sudo make check
Thanks,
Jie Yu
Re: Review Request 20297: Added API for managing IP packet filters.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20297/
-----------------------------------------------------------
(Updated May 15, 2014, 12:45 a.m.)
Review request for mesos, Benjamin Hindman, Chi Zhang, Ian Downes, Vinod Kone, and Cong Wang.
Changes
-------
Rebased. Review comments.
Bugs: MESOS-1228
https://issues.apache.org/jira/browse/MESOS-1228
Repository: mesos-git
Description
-------
The is Part 5 of the linux routing library.
Diffs (updated)
-----
src/Makefile.am 12374c4
src/linux/routing/filter/ip.hpp PRE-CREATION
src/linux/routing/filter/ip.cpp PRE-CREATION
src/tests/routing_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/20297/diff/
Testing
-------
make check
sudo make check
Thanks,
Jie Yu
Re: Review Request 20297: Added API for managing IP packet filters.
Posted by Jie Yu <yu...@gmail.com>.
> On May 13, 2014, 1:24 a.m., Vinod Kone wrote:
> > src/linux/routing/filter/ip.cpp, line 184
> > <https://reviews.apache.org/r/20297/diff/4/?file=569537#file569537line184>
> >
> > s/destinationIP/destinationPorts/
Oops! Good catch! This is not captured by the unit test. I added another unit test for capturing the regression.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20297/#review42800
-----------------------------------------------------------
On April 28, 2014, 5:04 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20297/
> -----------------------------------------------------------
>
> (Updated April 28, 2014, 5:04 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Chi Zhang, Ian Downes, Vinod Kone, and Cong Wang.
>
>
> Bugs: MESOS-1228
> https://issues.apache.org/jira/browse/MESOS-1228
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The is Part 5 of the linux routing library.
>
>
> Diffs
> -----
>
> src/Makefile.am d2e006d
> src/linux/routing/filter/ip.hpp PRE-CREATION
> src/linux/routing/filter/ip.cpp PRE-CREATION
> src/tests/routing_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/20297/diff/
>
>
> Testing
> -------
>
> make check
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 20297: Added API for managing IP packet filters.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20297/#review42800
-----------------------------------------------------------
Ship it!
src/linux/routing/filter/ip.cpp
<https://reviews.apache.org/r/20297/#comment76716>
s/has/have/
src/linux/routing/filter/ip.cpp
<https://reviews.apache.org/r/20297/#comment76717>
s/offset/offset of/ ?
src/linux/routing/filter/ip.cpp
<https://reviews.apache.org/r/20297/#comment76718>
s/destinationIP/destinationPorts/
src/linux/routing/filter/ip.cpp
<https://reviews.apache.org/r/20297/#comment76719>
why not offset and mask on the same line...?
if (offset = 8 && mask = 0x00ff0000) {
}
src/linux/routing/filter/ip.cpp
<https://reviews.apache.org/r/20297/#comment76720>
can you mention which part?
src/linux/routing/filter/ip.cpp
<https://reviews.apache.org/r/20297/#comment76721>
ditto.
src/linux/routing/filter/ip.cpp
<https://reviews.apache.org/r/20297/#comment76722>
ditto.
src/linux/routing/filter/ip.cpp
<https://reviews.apache.org/r/20297/#comment76724>
include the value of size in the message.
- Vinod Kone
On April 28, 2014, 5:04 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20297/
> -----------------------------------------------------------
>
> (Updated April 28, 2014, 5:04 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Chi Zhang, Ian Downes, Vinod Kone, and Cong Wang.
>
>
> Bugs: MESOS-1228
> https://issues.apache.org/jira/browse/MESOS-1228
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The is Part 5 of the linux routing library.
>
>
> Diffs
> -----
>
> src/Makefile.am d2e006d
> src/linux/routing/filter/ip.hpp PRE-CREATION
> src/linux/routing/filter/ip.cpp PRE-CREATION
> src/tests/routing_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/20297/diff/
>
>
> Testing
> -------
>
> make check
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 20297: Added API for managing IP packet filters.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20297/
-----------------------------------------------------------
(Updated April 28, 2014, 5:04 p.m.)
Review request for mesos, Benjamin Hindman, Chi Zhang, Ian Downes, Vinod Kone, and Cong Wang.
Changes
-------
Adjusted to use the new internal APIs.
Bugs: MESOS-1228
https://issues.apache.org/jira/browse/MESOS-1228
Repository: mesos-git
Description (updated)
-------
The is Part 5 of the linux routing library.
Diffs (updated)
-----
src/Makefile.am d2e006d
src/linux/routing/filter/ip.hpp PRE-CREATION
src/linux/routing/filter/ip.cpp PRE-CREATION
src/tests/routing_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/20297/diff/
Testing
-------
make check
sudo make check
Thanks,
Jie Yu
Re: Review Request 20297: Added API for managing IP packet filters.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20297/
-----------------------------------------------------------
(Updated April 22, 2014, 6:05 a.m.)
Review request for mesos, Benjamin Hindman, Chi Zhang, Ian Downes, Vinod Kone, and Cong Wang.
Changes
-------
Updated "bugs"
Bugs: MESOS-1228
https://issues.apache.org/jira/browse/MESOS-1228
Repository: mesos-git
Description
-------
The is Part 4 of the linux routing library.
Diffs
-----
src/Makefile.am a44ea42
src/linux/routing/filter/ip.hpp PRE-CREATION
src/linux/routing/filter/ip.cpp PRE-CREATION
src/tests/routing_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/20297/diff/
Testing
-------
make check
sudo make check
Thanks,
Jie Yu
Re: Review Request 20297: Added API for managing IP packet filters.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20297/
-----------------------------------------------------------
(Updated April 22, 2014, 5:43 a.m.)
Review request for mesos, Benjamin Hindman, Chi Zhang, Ian Downes, Vinod Kone, and Cong Wang.
Changes
-------
Rebased. Added ingress/egress option.
Repository: mesos-git
Description
-------
The is Part 4 of the linux routing library.
Diffs (updated)
-----
src/Makefile.am a44ea42
src/linux/routing/filter/ip.hpp PRE-CREATION
src/linux/routing/filter/ip.cpp PRE-CREATION
src/tests/routing_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/20297/diff/
Testing
-------
make check
sudo make check
Thanks,
Jie Yu
Re: Review Request 20297: Added API for managing IP packet filters.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20297/
-----------------------------------------------------------
(Updated April 18, 2014, 10:19 p.m.)
Review request for mesos, Benjamin Hindman, Chi Zhang, Ian Downes, Vinod Kone, and Cong Wang.
Changes
-------
Review comments. Rebased.
Repository: mesos-git
Description
-------
The is Part 4 of the linux routing library.
Diffs (updated)
-----
src/Makefile.am 32f6338
src/linux/routing/filter/ip.hpp PRE-CREATION
src/linux/routing/filter/ip.cpp PRE-CREATION
src/tests/routing_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/20297/diff/
Testing
-------
make check
sudo make check
Thanks,
Jie Yu
Re: Review Request 20297: Added API for managing IP packet filters.
Posted by Jie Yu <yu...@gmail.com>.
> On April 14, 2014, 6:52 p.m., Dominic Hamon wrote:
> > src/linux/routing/filter/ip.hpp, line 80
> > <https://reviews.apache.org/r/20297/diff/1/?file=555708#file555708line80>
> >
> > any reason why this isn't in its own header?
Because each type of filter has its own defined Classifier.
> On April 14, 2014, 6:52 p.m., Dominic Hamon wrote:
> > src/linux/routing/filter/ip.hpp, line 107
> > <https://reviews.apache.org/r/20297/diff/1/?file=555708#file555708line107>
> >
> > these could be marked const and be public
I need to put it into the vector. So const field does not work.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20297/#review40284
-----------------------------------------------------------
On April 13, 2014, 11:50 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20297/
> -----------------------------------------------------------
>
> (Updated April 13, 2014, 11:50 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Chi Zhang, Ian Downes, Vinod Kone, and Cong Wang.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The is Part 4 of the linux routing library.
>
>
> Diffs
> -----
>
> src/Makefile.am 560b4c7
> src/linux/routing/filter/ip.hpp PRE-CREATION
> src/linux/routing/filter/ip.cpp PRE-CREATION
> src/tests/routing_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/20297/diff/
>
>
> Testing
> -------
>
> make check
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 20297: Added API for managing IP packet filters.
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20297/#review40284
-----------------------------------------------------------
src/linux/routing/filter/ip.hpp
<https://reviews.apache.org/r/20297/#comment73265>
any reason why this isn't in its own header?
src/linux/routing/filter/ip.hpp
<https://reviews.apache.org/r/20297/#comment73268>
these could be marked const and be public
- Dominic Hamon
On April 13, 2014, 4:50 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20297/
> -----------------------------------------------------------
>
> (Updated April 13, 2014, 4:50 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Chi Zhang, Ian Downes, Vinod Kone, and Cong Wang.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The is Part 4 of the linux routing library.
>
>
> Diffs
> -----
>
> src/Makefile.am 560b4c7
> src/linux/routing/filter/ip.hpp PRE-CREATION
> src/linux/routing/filter/ip.cpp PRE-CREATION
> src/tests/routing_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/20297/diff/
>
>
> Testing
> -------
>
> make check
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 20297: Added API for managing IP packet filters.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20297/#review40229
-----------------------------------------------------------
Patch looks great!
Reviews applied: [20291, 20292, 20295, 20296, 20297]
All tests passed.
- Mesos ReviewBot
On April 13, 2014, 11:50 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20297/
> -----------------------------------------------------------
>
> (Updated April 13, 2014, 11:50 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Chi Zhang, Ian Downes, Vinod Kone, and Cong Wang.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The is Part 4 of the linux routing library.
>
>
> Diffs
> -----
>
> src/Makefile.am 560b4c7
> src/linux/routing/filter/ip.hpp PRE-CREATION
> src/linux/routing/filter/ip.cpp PRE-CREATION
> src/tests/routing_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/20297/diff/
>
>
> Testing
> -------
>
> make check
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 20297: Added API for managing IP packet filters.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20297/#review40427
-----------------------------------------------------------
src/linux/routing/filter/ip.cpp
<https://reviews.apache.org/r/20297/#comment73424>
whose "production networks"? ;-)
src/tests/routing_tests.cpp
<https://reviews.apache.org/r/20297/#comment73426>
add another test for fromBeginMask where mask is not 0xffff to check begin and end are correct?
- Ian Downes
On April 13, 2014, 11:50 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20297/
> -----------------------------------------------------------
>
> (Updated April 13, 2014, 11:50 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Chi Zhang, Ian Downes, Vinod Kone, and Cong Wang.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The is Part 4 of the linux routing library.
>
>
> Diffs
> -----
>
> src/Makefile.am 560b4c7
> src/linux/routing/filter/ip.hpp PRE-CREATION
> src/linux/routing/filter/ip.cpp PRE-CREATION
> src/tests/routing_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/20297/diff/
>
>
> Testing
> -------
>
> make check
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 20297: Added API for managing IP packet filters.
Posted by Chi Zhang <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20297/#review40440
-----------------------------------------------------------
Ship it!
src/linux/routing/filter/ip.cpp
<https://reviews.apache.org/r/20297/#comment73484>
kill the extra offset?
- Chi Zhang
On April 13, 2014, 11:50 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20297/
> -----------------------------------------------------------
>
> (Updated April 13, 2014, 11:50 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Chi Zhang, Ian Downes, Vinod Kone, and Cong Wang.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The is Part 4 of the linux routing library.
>
>
> Diffs
> -----
>
> src/Makefile.am 560b4c7
> src/linux/routing/filter/ip.hpp PRE-CREATION
> src/linux/routing/filter/ip.cpp PRE-CREATION
> src/tests/routing_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/20297/diff/
>
>
> Testing
> -------
>
> make check
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 20297: Added API for managing IP packet filters.
Posted by Jie Yu <yu...@gmail.com>.
> On April 15, 2014, 7:05 p.m., Cong Wang wrote:
> > src/linux/routing/filter/ip.cpp, line 245
> > <https://reviews.apache.org/r/20297/diff/1/?file=555709#file555709line245>
> >
> > i <= 0xff is always true since i is an unsigned char. Picking a reasonable number here, for example 64, is okay. It's unlikely that anyone will use more than 64 keys in one filter.
It's OK. We won't iterate that number of time (given there is a 'break').
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20297/#review40435
-----------------------------------------------------------
On April 13, 2014, 11:50 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20297/
> -----------------------------------------------------------
>
> (Updated April 13, 2014, 11:50 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Chi Zhang, Ian Downes, Vinod Kone, and Cong Wang.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The is Part 4 of the linux routing library.
>
>
> Diffs
> -----
>
> src/Makefile.am 560b4c7
> src/linux/routing/filter/ip.hpp PRE-CREATION
> src/linux/routing/filter/ip.cpp PRE-CREATION
> src/tests/routing_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/20297/diff/
>
>
> Testing
> -------
>
> make check
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 20297: Added API for managing IP packet filters.
Posted by Cong Wang <cw...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20297/#review40435
-----------------------------------------------------------
src/linux/routing/filter/ip.cpp
<https://reviews.apache.org/r/20297/#comment73437>
i <= 0xff is always true since i is an unsigned char. Picking a reasonable number here, for example 64, is okay. It's unlikely that anyone will use more than 64 keys in one filter.
- Cong Wang
On April 13, 2014, 11:50 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20297/
> -----------------------------------------------------------
>
> (Updated April 13, 2014, 11:50 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Chi Zhang, Ian Downes, Vinod Kone, and Cong Wang.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The is Part 4 of the linux routing library.
>
>
> Diffs
> -----
>
> src/Makefile.am 560b4c7
> src/linux/routing/filter/ip.hpp PRE-CREATION
> src/linux/routing/filter/ip.cpp PRE-CREATION
> src/tests/routing_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/20297/diff/
>
>
> Testing
> -------
>
> make check
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>