Re: Review Request 33963: Move ARP filter implementation onto basic filter

2015-05-08 Thread Jie Yu 

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

Ship it!


Ship It!

- Jie Yu


On May 8, 2015, 7:29 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33963/
> ---
> 
> (Updated May 8, 2015, 7:29 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
> ---
> 
> After adding basic filter, ARP filter implementation now can move onto basic 
> filter, this also matches kernel implementation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 93c7c8a807a33ab639be6289535bbd32022aa85b 
>   src/linux/routing/filter/arp.hpp 819e8147b14193a50ce7f938676e86dbfd622ba0 
>   src/linux/routing/filter/arp.cpp 8c4976620ec296b44cefec375b5f21986cc886ae 
>   src/linux/routing/filter/basic.hpp PRE-CREATION 
>   src/linux/routing/filter/basic.cpp PRE-CREATION 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> fc0fa4c2086f0be84ab7ddc04b85dec4a3b5b5dc 
>   src/tests/routing_tests.cpp ce583b59bf9fb2ef855aa82ab6083ea11b138e55 
> 
> Diff: https://reviews.apache.org/r/33963/diff/
> 
> 
> Testing
> ---
> 
> Run ARP filter test cases
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 33963: Move ARP filter implementation onto basic filter

2015-05-08 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [31502, 31503, 31504, 33963]

All tests passed.

- Mesos ReviewBot


On May 8, 2015, 7:29 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33963/
> ---
> 
> (Updated May 8, 2015, 7:29 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
> ---
> 
> After adding basic filter, ARP filter implementation now can move onto basic 
> filter, this also matches kernel implementation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 93c7c8a807a33ab639be6289535bbd32022aa85b 
>   src/linux/routing/filter/arp.hpp 819e8147b14193a50ce7f938676e86dbfd622ba0 
>   src/linux/routing/filter/arp.cpp 8c4976620ec296b44cefec375b5f21986cc886ae 
>   src/linux/routing/filter/basic.hpp PRE-CREATION 
>   src/linux/routing/filter/basic.cpp PRE-CREATION 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> fc0fa4c2086f0be84ab7ddc04b85dec4a3b5b5dc 
>   src/tests/routing_tests.cpp ce583b59bf9fb2ef855aa82ab6083ea11b138e55 
> 
> Diff: https://reviews.apache.org/r/33963/diff/
> 
> 
> Testing
> ---
> 
> Run ARP filter test cases
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 33963: Move ARP filter implementation onto basic filter

2015-05-08 Thread Cong Wang 

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

(Updated May 8, 2015, 7:29 p.m.)


Review request for mesos, Chi Zhang, Ian Downes, and Jie Yu.


Changes
---

Remove old arp::xxx API's and implementations


Bugs: MESOS-2422
https://issues.apache.org/jira/browse/MESOS-2422


Repository: mesos


Description (updated)
---

After adding basic filter, ARP filter implementation now can move onto basic 
filter, this also matches kernel implementation.


Diffs (updated)
-

  src/Makefile.am 93c7c8a807a33ab639be6289535bbd32022aa85b 
  src/linux/routing/filter/arp.hpp 819e8147b14193a50ce7f938676e86dbfd622ba0 
  src/linux/routing/filter/arp.cpp 8c4976620ec296b44cefec375b5f21986cc886ae 
  src/linux/routing/filter/basic.hpp PRE-CREATION 
  src/linux/routing/filter/basic.cpp PRE-CREATION 
  src/slave/containerizer/isolators/network/port_mapping.cpp 
fc0fa4c2086f0be84ab7ddc04b85dec4a3b5b5dc 
  src/tests/routing_tests.cpp ce583b59bf9fb2ef855aa82ab6083ea11b138e55 

Diff: https://reviews.apache.org/r/33963/diff/


Testing
---

Run ARP filter test cases


Thanks,

Cong Wang

Re: Review Request 33963: Move ARP filter implementation onto basic filter

2015-05-08 Thread Cong Wang 


> On May 8, 2015, 5:37 p.m., Jie Yu wrote:
> > src/linux/routing/filter/arp.cpp, line 64
> > 
> >
> > Can you remove arp.hpp and arp.cpp completely and use the basic filter 
> > you just created?

Good point! We can move it into basic.{c,h}pp by keeping its namespace.


- Cong


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


On May 8, 2015, 12:14 a.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33963/
> ---
> 
> (Updated May 8, 2015, 12:14 a.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
> ---
> 
> After adding basic filter, ARP filter implementation now can move onto basic 
> filter, this also matches kernel implementation too.
> 
> 
> Diffs
> -
> 
>   src/linux/routing/filter/arp.cpp 8c4976620ec296b44cefec375b5f21986cc886ae 
>   src/linux/routing/filter/basic.hpp PRE-CREATION 
>   src/linux/routing/filter/basic.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/33963/diff/
> 
> 
> Testing
> ---
> 
> Run ARP filter test cases
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 33963: Move ARP filter implementation onto basic filter

2015-05-08 Thread Jie Yu 

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



src/linux/routing/filter/arp.cpp


Can you remove arp.hpp and arp.cpp completely and use the basic filter you 
just created?



src/linux/routing/filter/basic.hpp


I would rather not introduce this special version as it's semantics is 
rather implicit (one needs to read the document to figure out that it's for 
ETH_P_ALL).

Simply use exists(link, parent, ETH_P_ALL) at callsites.



src/linux/routing/filter/basic.hpp


Ditto. No need for this.



src/linux/routing/filter/basic.hpp


Ditto. No need for this interface.


- Jie Yu


On May 8, 2015, 12:14 a.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33963/
> ---
> 
> (Updated May 8, 2015, 12:14 a.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
> ---
> 
> After adding basic filter, ARP filter implementation now can move onto basic 
> filter, this also matches kernel implementation too.
> 
> 
> Diffs
> -
> 
>   src/linux/routing/filter/arp.cpp 8c4976620ec296b44cefec375b5f21986cc886ae 
>   src/linux/routing/filter/basic.hpp PRE-CREATION 
>   src/linux/routing/filter/basic.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/33963/diff/
> 
> 
> Testing
> ---
> 
> Run ARP filter test cases
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 33963: Move ARP filter implementation onto basic filter

2015-05-07 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [31502, 31503, 31504, 33963]

All tests passed.

- Mesos ReviewBot


On May 8, 2015, 12:14 a.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33963/
> ---
> 
> (Updated May 8, 2015, 12:14 a.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
> ---
> 
> After adding basic filter, ARP filter implementation now can move onto basic 
> filter, this also matches kernel implementation too.
> 
> 
> Diffs
> -
> 
>   src/linux/routing/filter/arp.cpp 8c4976620ec296b44cefec375b5f21986cc886ae 
>   src/linux/routing/filter/basic.hpp PRE-CREATION 
>   src/linux/routing/filter/basic.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/33963/diff/
> 
> 
> Testing
> ---
> 
> Run ARP filter test cases
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Review Request 33963: Move ARP filter implementation onto basic filter

2015-05-07 Thread Cong Wang 

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

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
---

After adding basic filter, ARP filter implementation now can move onto basic 
filter, this also matches kernel implementation too.


Diffs
-

  src/linux/routing/filter/arp.cpp 8c4976620ec296b44cefec375b5f21986cc886ae 
  src/linux/routing/filter/basic.hpp PRE-CREATION 
  src/linux/routing/filter/basic.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/33963/diff/


Testing
---

Run ARP filter test cases


Thanks,

Cong Wang