You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Cong Wang <cw...@twopensource.com> on 2015/02/27 00:15:17 UTC

Review Request 31503: (2/4) Add the Classifier action

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

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


Repository: mesos


Description
-------

Introduce a new action which saves the classid for a specified filter. Similar to the u32 terminal action, it is not a real action defined by TC, it is just for setting a filter attribute. The classid or flowid is used to match the fq_codel flows.


Diffs
-----

  src/linux/routing/filter/action.hpp 410c15ad20ffa694bdb4c4187f1544ef9617ebac 
  src/linux/routing/filter/arp.hpp fa0ea6f93218fb3bee9f435f0268022d5edc5e11 
  src/linux/routing/filter/arp.cpp bf19264ccc83e8a8b8c2affe53779ac070472925 
  src/linux/routing/filter/icmp.hpp 7b478c4e23a2fb0895021ce5fb30bdf78131b871 
  src/linux/routing/filter/icmp.cpp 86bd67b71a590b88344adbe10fd1b44ea1b5148d 
  src/linux/routing/filter/internal.hpp 8a6c0c0390c4ed119a11f0f808e0a244f7734c45 
  src/linux/routing/filter/ip.hpp 932ed4bbf57e60261ff3b48ae242a7dd5e1f4260 
  src/linux/routing/filter/ip.cpp 922a732c3543a072674208b123fdfadbef2b15f2 
  src/tests/routing_tests.cpp 3cda6ab8c1ad24e4b7d0b9aeda2abc595fc857a5 

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


Testing
-------

Run the testcase.


Thanks,

Cong Wang


Re: Review Request 31503: (2/4) Add the Classifier action

Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31503/#review74394
-----------------------------------------------------------


please add a jira reference and a dependency on the first part.

- Dominic Hamon


On Feb. 26, 2015, 3:15 p.m., Cong Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31503/
> -----------------------------------------------------------
> 
> (Updated Feb. 26, 2015, 3:15 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Introduce a new action which saves the classid for a specified filter. Similar to the u32 terminal action, it is not a real action defined by TC, it is just for setting a filter attribute. The classid or flowid is used to match the fq_codel flows.
> 
> 
> Diffs
> -----
> 
>   src/linux/routing/filter/action.hpp 410c15ad20ffa694bdb4c4187f1544ef9617ebac 
>   src/linux/routing/filter/arp.hpp fa0ea6f93218fb3bee9f435f0268022d5edc5e11 
>   src/linux/routing/filter/arp.cpp bf19264ccc83e8a8b8c2affe53779ac070472925 
>   src/linux/routing/filter/icmp.hpp 7b478c4e23a2fb0895021ce5fb30bdf78131b871 
>   src/linux/routing/filter/icmp.cpp 86bd67b71a590b88344adbe10fd1b44ea1b5148d 
>   src/linux/routing/filter/internal.hpp 8a6c0c0390c4ed119a11f0f808e0a244f7734c45 
>   src/linux/routing/filter/ip.hpp 932ed4bbf57e60261ff3b48ae242a7dd5e1f4260 
>   src/linux/routing/filter/ip.cpp 922a732c3543a072674208b123fdfadbef2b15f2 
>   src/tests/routing_tests.cpp 3cda6ab8c1ad24e4b7d0b9aeda2abc595fc857a5 
> 
> Diff: https://reviews.apache.org/r/31503/diff/
> 
> 
> Testing
> -------
> 
> Run the testcase.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>


Re: Review Request 31503: (2/4) Add the Classifier action

Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31503/#review74414
-----------------------------------------------------------


Bad patch!

Reviews applied: [31503]

Failed command: ./support/apply-review.sh -n -r 31503

Error:
 2015-02-27 00:45:37 URL:https://reviews.apache.org/r/31503/diff/raw/ [10551/10551] -> "31503.patch" [1]
error: patch failed: src/tests/routing_tests.cpp:425
error: src/tests/routing_tests.cpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On Feb. 26, 2015, 11:15 p.m., Cong Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31503/
> -----------------------------------------------------------
> 
> (Updated Feb. 26, 2015, 11:15 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Introduce a new action which saves the classid for a specified filter. Similar to the u32 terminal action, it is not a real action defined by TC, it is just for setting a filter attribute. The classid or flowid is used to match the fq_codel flows.
> 
> 
> Diffs
> -----
> 
>   src/linux/routing/filter/action.hpp 410c15ad20ffa694bdb4c4187f1544ef9617ebac 
>   src/linux/routing/filter/arp.hpp fa0ea6f93218fb3bee9f435f0268022d5edc5e11 
>   src/linux/routing/filter/arp.cpp bf19264ccc83e8a8b8c2affe53779ac070472925 
>   src/linux/routing/filter/icmp.hpp 7b478c4e23a2fb0895021ce5fb30bdf78131b871 
>   src/linux/routing/filter/icmp.cpp 86bd67b71a590b88344adbe10fd1b44ea1b5148d 
>   src/linux/routing/filter/internal.hpp 8a6c0c0390c4ed119a11f0f808e0a244f7734c45 
>   src/linux/routing/filter/ip.hpp 932ed4bbf57e60261ff3b48ae242a7dd5e1f4260 
>   src/linux/routing/filter/ip.cpp 922a732c3543a072674208b123fdfadbef2b15f2 
>   src/tests/routing_tests.cpp 3cda6ab8c1ad24e4b7d0b9aeda2abc595fc857a5 
> 
> Diff: https://reviews.apache.org/r/31503/diff/
> 
> 
> Testing
> -------
> 
> Run the testcase.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>


Re: Review Request 31503: (2/4) Add the Classifier action

Posted by Cong Wang <cw...@twopensource.com>.

> On March 2, 2015, 9:38 p.m., Jie Yu wrote:
> > src/linux/routing/filter/action.hpp, line 76
> > <https://reviews.apache.org/r/31503/diff/1/?file=879070#file879070line76>
> >
> >     The name `Classifier` here is a little confusing because we already have `ip::Classifier`, `icmp::Classifier` and `arc::Classifier`.
> >     
> >     How about renaming it to `Flow` since the tc command uses `flowid`?

I rename it to FlowId.


- Cong


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


On Feb. 27, 2015, 9:39 p.m., Cong Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31503/
> -----------------------------------------------------------
> 
> (Updated Feb. 27, 2015, 9:39 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
> -------
> 
> Introduce a new action which saves the classid for a specified filter. Similar to the u32 terminal action, it is not a real action defined by TC, it is just for setting a filter attribute. The classid or flowid is used to match the fq_codel flows.
> 
> 
> Diffs
> -----
> 
>   src/linux/routing/filter/action.hpp 410c15ad20ffa694bdb4c4187f1544ef9617ebac 
>   src/linux/routing/filter/arp.hpp fa0ea6f93218fb3bee9f435f0268022d5edc5e11 
>   src/linux/routing/filter/arp.cpp bf19264ccc83e8a8b8c2affe53779ac070472925 
>   src/linux/routing/filter/icmp.hpp 7b478c4e23a2fb0895021ce5fb30bdf78131b871 
>   src/linux/routing/filter/icmp.cpp 86bd67b71a590b88344adbe10fd1b44ea1b5148d 
>   src/linux/routing/filter/internal.hpp 8a6c0c0390c4ed119a11f0f808e0a244f7734c45 
>   src/linux/routing/filter/ip.hpp 932ed4bbf57e60261ff3b48ae242a7dd5e1f4260 
>   src/linux/routing/filter/ip.cpp 922a732c3543a072674208b123fdfadbef2b15f2 
>   src/tests/routing_tests.cpp 3cda6ab8c1ad24e4b7d0b9aeda2abc595fc857a5 
> 
> Diff: https://reviews.apache.org/r/31503/diff/
> 
> 
> Testing
> -------
> 
> Run the testcase.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>


Re: Review Request 31503: (2/4) Add the Classifier action

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31503/#review74787
-----------------------------------------------------------



src/linux/routing/filter/action.hpp
<https://reviews.apache.org/r/31503/#comment121546>

    Two lines apart between top level entities.



src/linux/routing/filter/action.hpp
<https://reviews.apache.org/r/31503/#comment121542>

    The name `Classifier` here is a little confusing because we already have `ip::Classifier`, `icmp::Classifier` and `arc::Classifier`.
    
    How about renaming it to `Flow` since the tc command uses `flowid`?



src/linux/routing/filter/action.hpp
<https://reviews.apache.org/r/31503/#comment121567>

    The constructors here are a little confusing. For example, what does `min` or `maj` mean.
    
    Looks like you just need to set the handle which contains a major and a minor (see code in src/linux/routing/queueing/handle.hpp).



src/linux/routing/filter/action.hpp
<https://reviews.apache.org/r/31503/#comment121544>

    One line apart between class methods.



src/linux/routing/filter/action.hpp
<https://reviews.apache.org/r/31503/#comment121545>

    Given the class name `Flow`, this method can be renamed to `id` or `handle`.



src/linux/routing/filter/action.hpp
<https://reviews.apache.org/r/31503/#comment121547>

    Two lines apart please.



src/linux/routing/filter/arp.hpp
<https://reviews.apache.org/r/31503/#comment121574>

    set the specified _flowid_.
    
    You need to adjust the comments here. What does that mean for "to a set of links"? :)



src/linux/routing/filter/internal.hpp
<https://reviews.apache.org/r/31503/#comment121600>

    "Failed to set the flow id: "



src/linux/routing/filter/internal.hpp
<https://reviews.apache.org/r/31503/#comment121598>

    Add a new line above, please.



src/linux/routing/filter/internal.hpp
<https://reviews.apache.org/r/31503/#comment121603>

    Please be consistent with other `attach` functions:
    ```
    return Error("Unsupported classifier kind: " + kind);
    ```



src/tests/routing_tests.cpp
<https://reviews.apache.org/r/31503/#comment121606>

    OK, I think you need to expose the `primiary` and `secondary` in `Handle`. So that here, you can do:
    
    ```
    action::Flow(Handle(fq_codel::HANDLE.primiary(), 1));
    ```
    
    Also, I thought minor=0 is only for the root qdisc?


- Jie Yu


On Feb. 27, 2015, 9:39 p.m., Cong Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31503/
> -----------------------------------------------------------
> 
> (Updated Feb. 27, 2015, 9:39 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
> -------
> 
> Introduce a new action which saves the classid for a specified filter. Similar to the u32 terminal action, it is not a real action defined by TC, it is just for setting a filter attribute. The classid or flowid is used to match the fq_codel flows.
> 
> 
> Diffs
> -----
> 
>   src/linux/routing/filter/action.hpp 410c15ad20ffa694bdb4c4187f1544ef9617ebac 
>   src/linux/routing/filter/arp.hpp fa0ea6f93218fb3bee9f435f0268022d5edc5e11 
>   src/linux/routing/filter/arp.cpp bf19264ccc83e8a8b8c2affe53779ac070472925 
>   src/linux/routing/filter/icmp.hpp 7b478c4e23a2fb0895021ce5fb30bdf78131b871 
>   src/linux/routing/filter/icmp.cpp 86bd67b71a590b88344adbe10fd1b44ea1b5148d 
>   src/linux/routing/filter/internal.hpp 8a6c0c0390c4ed119a11f0f808e0a244f7734c45 
>   src/linux/routing/filter/ip.hpp 932ed4bbf57e60261ff3b48ae242a7dd5e1f4260 
>   src/linux/routing/filter/ip.cpp 922a732c3543a072674208b123fdfadbef2b15f2 
>   src/tests/routing_tests.cpp 3cda6ab8c1ad24e4b7d0b9aeda2abc595fc857a5 
> 
> Diff: https://reviews.apache.org/r/31503/diff/
> 
> 
> Testing
> -------
> 
> Run the testcase.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>


Re: Review Request 31503: (2/6) Add the FlowId action

Posted by Cong Wang <cw...@twopensource.com>.

> On March 5, 2015, 9:57 p.m., Jie Yu wrote:
> > src/linux/routing/filter/internal.hpp, lines 247-252
> > <https://reviews.apache.org/r/31503/diff/2/?file=884502#file884502line247>
> >
> >     Adjust the comments accordingly (using action 'flow' consistently).
> >     
> >     What's `Classid` in your comment?

Yes I missed "basic" in comments. Classid is used exchangeable with flowid in kernel and iproute2, but I should use flowid here for consistency.


- Cong


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


On March 4, 2015, 8:06 p.m., Cong Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31503/
> -----------------------------------------------------------
> 
> (Updated March 4, 2015, 8:06 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
> -------
> 
> Introduce a new action which saves the classid for a specified filter. Similar to the u32 terminal action, it is not a real action defined by TC, it is just for setting a filter attribute. The classid or flowid is used to match the fq_codel flows.
> 
> 
> Diffs
> -----
> 
>   src/linux/routing/filter/action.hpp 410c15a 
>   src/linux/routing/filter/arp.hpp fa0ea6f 
>   src/linux/routing/filter/arp.cpp bf19264 
>   src/linux/routing/filter/icmp.hpp 7b478c4 
>   src/linux/routing/filter/icmp.cpp 86bd67b 
>   src/linux/routing/filter/internal.hpp 8a6c0c0 
>   src/linux/routing/filter/ip.hpp 932ed4b 
>   src/linux/routing/filter/ip.cpp 922a732 
>   src/linux/routing/queueing/handle.hpp 2725d07 
>   src/tests/routing_tests.cpp 3cda6ab 
> 
> Diff: https://reviews.apache.org/r/31503/diff/
> 
> 
> Testing
> -------
> 
> Run the testcase.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>


Re: Review Request 31503: (2/6) Add the FlowId action

Posted by Cong Wang <cw...@twopensource.com>.

> On March 5, 2015, 9:57 p.m., Jie Yu wrote:
> > src/linux/routing/filter/action.hpp, lines 83-84
> > <https://reviews.apache.org/r/31503/diff/2/?file=884497#file884497line83>
> >
> >     Why we need this constructor? IMO, it's not needed. We can always do the following at the callsites, right?
> >     ```
> >     Flow(parent.primary(), 0x1)
> >     ```

That will expose implementation details to the callers, what if people mistakenly call FlowId(parent.secondary(), 0x1)? There is no chance to mistake FlowId(parent, 0x1).


- Cong


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


On March 4, 2015, 8:06 p.m., Cong Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31503/
> -----------------------------------------------------------
> 
> (Updated March 4, 2015, 8:06 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
> -------
> 
> Introduce a new action which saves the classid for a specified filter. Similar to the u32 terminal action, it is not a real action defined by TC, it is just for setting a filter attribute. The classid or flowid is used to match the fq_codel flows.
> 
> 
> Diffs
> -----
> 
>   src/linux/routing/filter/action.hpp 410c15a 
>   src/linux/routing/filter/arp.hpp fa0ea6f 
>   src/linux/routing/filter/arp.cpp bf19264 
>   src/linux/routing/filter/icmp.hpp 7b478c4 
>   src/linux/routing/filter/icmp.cpp 86bd67b 
>   src/linux/routing/filter/internal.hpp 8a6c0c0 
>   src/linux/routing/filter/ip.hpp 932ed4b 
>   src/linux/routing/filter/ip.cpp 922a732 
>   src/linux/routing/queueing/handle.hpp 2725d07 
>   src/tests/routing_tests.cpp 3cda6ab 
> 
> Diff: https://reviews.apache.org/r/31503/diff/
> 
> 
> Testing
> -------
> 
> Run the testcase.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>


Re: Review Request 31503: (2/6) Add the FlowId action

Posted by Cong Wang <cw...@twopensource.com>.

> On March 5, 2015, 9:57 p.m., Jie Yu wrote:
> > src/linux/routing/filter/action.hpp, line 77
> > <https://reviews.apache.org/r/31503/diff/2/?file=884497#file884497line77>
> >
> >     Add a space before the colon. Also, how about calling it `action::Flow` instead of `action::FlowId`. It reads better since we have an `id()` member method:
> >     
> >     
> >     ```
> >     Try<bool> create(
> >         ...
> >         const action::Flow& flow)
> >     {
> >       xxx = flow.id();
> >     }
> >     ```

Since the action is supposed to set a flow id, "FlowId" is more meaningful than just "Flow". Also, we don't call .id() in create(), we only call it in attach(), which is hidden from its callers.


- Cong


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


On March 4, 2015, 8:06 p.m., Cong Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31503/
> -----------------------------------------------------------
> 
> (Updated March 4, 2015, 8:06 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
> -------
> 
> Introduce a new action which saves the classid for a specified filter. Similar to the u32 terminal action, it is not a real action defined by TC, it is just for setting a filter attribute. The classid or flowid is used to match the fq_codel flows.
> 
> 
> Diffs
> -----
> 
>   src/linux/routing/filter/action.hpp 410c15a 
>   src/linux/routing/filter/arp.hpp fa0ea6f 
>   src/linux/routing/filter/arp.cpp bf19264 
>   src/linux/routing/filter/icmp.hpp 7b478c4 
>   src/linux/routing/filter/icmp.cpp 86bd67b 
>   src/linux/routing/filter/internal.hpp 8a6c0c0 
>   src/linux/routing/filter/ip.hpp 932ed4b 
>   src/linux/routing/filter/ip.cpp 922a732 
>   src/linux/routing/queueing/handle.hpp 2725d07 
>   src/tests/routing_tests.cpp 3cda6ab 
> 
> Diff: https://reviews.apache.org/r/31503/diff/
> 
> 
> Testing
> -------
> 
> Run the testcase.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>


Re: Review Request 31503: (2/6) Add the FlowId action

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31503/#review75386
-----------------------------------------------------------



src/linux/routing/filter/action.hpp
<https://reviews.apache.org/r/31503/#comment122402>

    Add a space before the colon. Also, how about calling it `action::Flow` instead of `action::FlowId`. It reads better since we have an `id()` member method:
    
    ```
    Try<bool> create(
        ...
        const action::Flow& flow)
    {
      xxx = flow.id();
    }
    ```



src/linux/routing/filter/action.hpp
<https://reviews.apache.org/r/31503/#comment122409>

    Rename `_handle` to `_id`?



src/linux/routing/filter/action.hpp
<https://reviews.apache.org/r/31503/#comment122423>

    Why we need this constructor? IMO, it's not needed. We can always do the following at the callsites, right?
    ```
    Flow(parent.primary(), 0x1)
    ```



src/linux/routing/filter/action.hpp
<https://reviews.apache.org/r/31503/#comment122408>

    Rename it to id_?



src/linux/routing/filter/internal.hpp
<https://reviews.apache.org/r/31503/#comment122410>

    Adjust the comments accordingly (using action 'flow' consistently).
    
    What's `Classid` in your comment?



src/linux/routing/filter/internal.hpp
<https://reviews.apache.org/r/31503/#comment122412>

    `Failed to set u32 classid: "...`



src/linux/routing/filter/internal.hpp
<https://reviews.apache.org/r/31503/#comment122411>

    In the comments above, you said you only care about u32 filters. Why you set the target here? Maybe you should adjust your comments above.


- Jie Yu


On March 4, 2015, 8:06 p.m., Cong Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31503/
> -----------------------------------------------------------
> 
> (Updated March 4, 2015, 8:06 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
> -------
> 
> Introduce a new action which saves the classid for a specified filter. Similar to the u32 terminal action, it is not a real action defined by TC, it is just for setting a filter attribute. The classid or flowid is used to match the fq_codel flows.
> 
> 
> Diffs
> -----
> 
>   src/linux/routing/filter/action.hpp 410c15a 
>   src/linux/routing/filter/arp.hpp fa0ea6f 
>   src/linux/routing/filter/arp.cpp bf19264 
>   src/linux/routing/filter/icmp.hpp 7b478c4 
>   src/linux/routing/filter/icmp.cpp 86bd67b 
>   src/linux/routing/filter/internal.hpp 8a6c0c0 
>   src/linux/routing/filter/ip.hpp 932ed4b 
>   src/linux/routing/filter/ip.cpp 922a732 
>   src/linux/routing/queueing/handle.hpp 2725d07 
>   src/tests/routing_tests.cpp 3cda6ab 
> 
> Diff: https://reviews.apache.org/r/31503/diff/
> 
> 
> Testing
> -------
> 
> Run the testcase.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>


Re: Review Request 31503: (2/5) Add the FlowId action

Posted by Chi Zhang <ch...@gmail.com>.

> On March 4, 2015, 10:01 p.m., Chi Zhang wrote:
> > src/linux/routing/filter/action.hpp, line 87
> > <https://reviews.apache.org/r/31503/diff/1-2/?file=879070#file879070line87>
> >
> >     could be just parent and child directly.
> 
> Cong Wang wrote:
>     What is mesos preferred coding style here?

I think you do the _ if there is a conflict if you don't do it.


- Chi


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


On March 9, 2015, 6:29 p.m., Cong Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31503/
> -----------------------------------------------------------
> 
> (Updated March 9, 2015, 6: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
> -------
> 
> Introduce a new action which saves the classid for a specified filter. Similar to the u32 terminal action, it is not a real action defined by TC, it is just for setting a filter attribute. The classid or flowid is used to match the fq_codel flows.
> 
> 
> Diffs
> -----
> 
>   src/linux/routing/filter/action.hpp 410c15a 
>   src/linux/routing/filter/arp.hpp fa0ea6f 
>   src/linux/routing/filter/arp.cpp bf19264 
>   src/linux/routing/filter/icmp.hpp 0237f1a 
>   src/linux/routing/filter/icmp.cpp 9076af3 
>   src/linux/routing/filter/internal.hpp 8a6c0c0 
>   src/linux/routing/filter/ip.hpp 3954962 
>   src/linux/routing/filter/ip.cpp 059ad6e 
>   src/linux/routing/queueing/handle.hpp 2725d07 
>   src/tests/routing_tests.cpp 3cda6ab 
> 
> Diff: https://reviews.apache.org/r/31503/diff/
> 
> 
> Testing
> -------
> 
> Run the testcase.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>


Re: Review Request 31503: (2/6) Add the FlowId action

Posted by Cong Wang <cw...@twopensource.com>.

> On March 4, 2015, 10:01 p.m., Chi Zhang wrote:
> > src/linux/routing/filter/action.hpp, line 87
> > <https://reviews.apache.org/r/31503/diff/1-2/?file=879070#file879070line87>
> >
> >     could be just parent and child directly.

What is mesos preferred coding style here?


- Cong


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


On March 4, 2015, 8:06 p.m., Cong Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31503/
> -----------------------------------------------------------
> 
> (Updated March 4, 2015, 8:06 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
> -------
> 
> Introduce a new action which saves the classid for a specified filter. Similar to the u32 terminal action, it is not a real action defined by TC, it is just for setting a filter attribute. The classid or flowid is used to match the fq_codel flows.
> 
> 
> Diffs
> -----
> 
>   src/linux/routing/filter/action.hpp 410c15a 
>   src/linux/routing/filter/arp.hpp fa0ea6f 
>   src/linux/routing/filter/arp.cpp bf19264 
>   src/linux/routing/filter/icmp.hpp 7b478c4 
>   src/linux/routing/filter/icmp.cpp 86bd67b 
>   src/linux/routing/filter/internal.hpp 8a6c0c0 
>   src/linux/routing/filter/ip.hpp 932ed4b 
>   src/linux/routing/filter/ip.cpp 922a732 
>   src/linux/routing/queueing/handle.hpp 2725d07 
>   src/tests/routing_tests.cpp 3cda6ab 
> 
> Diff: https://reviews.apache.org/r/31503/diff/
> 
> 
> Testing
> -------
> 
> Run the testcase.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>


Re: Review Request 31503: (2/6) Add the FlowId action

Posted by Cong Wang <cw...@twopensource.com>.

> On March 4, 2015, 10:01 p.m., Chi Zhang wrote:
> > src/linux/routing/filter/internal.hpp, line 269
> > <https://reviews.apache.org/r/31503/diff/1-2/?file=879075#file879075line269>
> >
> >     Do you need to undo the previous step if this is an error?

There is no way to unset a classid once we set it.


- Cong


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


On March 4, 2015, 8:06 p.m., Cong Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31503/
> -----------------------------------------------------------
> 
> (Updated March 4, 2015, 8:06 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
> -------
> 
> Introduce a new action which saves the classid for a specified filter. Similar to the u32 terminal action, it is not a real action defined by TC, it is just for setting a filter attribute. The classid or flowid is used to match the fq_codel flows.
> 
> 
> Diffs
> -----
> 
>   src/linux/routing/filter/action.hpp 410c15a 
>   src/linux/routing/filter/arp.hpp fa0ea6f 
>   src/linux/routing/filter/arp.cpp bf19264 
>   src/linux/routing/filter/icmp.hpp 7b478c4 
>   src/linux/routing/filter/icmp.cpp 86bd67b 
>   src/linux/routing/filter/internal.hpp 8a6c0c0 
>   src/linux/routing/filter/ip.hpp 932ed4b 
>   src/linux/routing/filter/ip.cpp 922a732 
>   src/linux/routing/queueing/handle.hpp 2725d07 
>   src/tests/routing_tests.cpp 3cda6ab 
> 
> Diff: https://reviews.apache.org/r/31503/diff/
> 
> 
> Testing
> -------
> 
> Run the testcase.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>


Re: Review Request 31503: (2/6) Add the FlowId action

Posted by Chi Zhang <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31503/#review75243
-----------------------------------------------------------



src/linux/routing/filter/action.hpp
<https://reviews.apache.org/r/31503/#comment122210>

    is this handle here like a 'parent' as well (with a default 'child')?
    
    maybe document the constructors?



src/linux/routing/filter/action.hpp
<https://reviews.apache.org/r/31503/#comment122207>

    could be just parent and child directly.



src/linux/routing/filter/internal.hpp
<https://reviews.apache.org/r/31503/#comment122206>

    Do you need to undo the previous step if this is an error?


- Chi Zhang


On March 4, 2015, 8:06 p.m., Cong Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31503/
> -----------------------------------------------------------
> 
> (Updated March 4, 2015, 8:06 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
> -------
> 
> Introduce a new action which saves the classid for a specified filter. Similar to the u32 terminal action, it is not a real action defined by TC, it is just for setting a filter attribute. The classid or flowid is used to match the fq_codel flows.
> 
> 
> Diffs
> -----
> 
>   src/linux/routing/filter/action.hpp 410c15a 
>   src/linux/routing/filter/arp.hpp fa0ea6f 
>   src/linux/routing/filter/arp.cpp bf19264 
>   src/linux/routing/filter/icmp.hpp 7b478c4 
>   src/linux/routing/filter/icmp.cpp 86bd67b 
>   src/linux/routing/filter/internal.hpp 8a6c0c0 
>   src/linux/routing/filter/ip.hpp 932ed4b 
>   src/linux/routing/filter/ip.cpp 922a732 
>   src/linux/routing/queueing/handle.hpp 2725d07 
>   src/tests/routing_tests.cpp 3cda6ab 
> 
> Diff: https://reviews.apache.org/r/31503/diff/
> 
> 
> Testing
> -------
> 
> Run the testcase.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>


Re: Review Request 31503: Add classid to Filter

Posted by Ian Downes <ia...@gmail.com>.

> On March 11, 2015, 7:02 p.m., Jie Yu wrote:
> > Cong, I addressed the comments for you and try to commit. Looks like rtnl_u32_get_classid is not in libnl-3.2.25. That means we need a newer version of libnl for this to work!
> 
> Cong Wang wrote:
>     I have addressed your comments locally, just haven't uploaded a new version.
>     Yes, we need this commit from upstream:
>     https://github.com/thom311/libnl/commit/d8f080d94fa9cf5e977f8805446ac7ef39f82d31
> 
> Cong Wang wrote:
>     3.2.26 will be released soon, according to upstream maintainer. So how about adding a check in configure.ac?
>     
>     +  # Check for libnl-route (both headers and libraries).
>     +  AC_CHECK_LIB([nl-route-3], [rtnl_u32_get_classid], [],
>     +               [AC_MSG_ERROR([cannot find libnl-route-3
>     +-------------------------------------------------------------------
>     +We need libnl-route-3 for building network isolator!
>     +
>     +Please install libnl3 (version 3.2.26 or higher):
>     +http://www.infradead.org/~tgr/libnl/

Do you have an estimated date for the release? Can we clean up some of the documentation if we add the check?


- Ian


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


On March 10, 2015, 4:26 p.m., Cong Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31503/
> -----------------------------------------------------------
> 
> (Updated March 10, 2015, 4:26 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
> -------
> 
> Currently we don't support to read actions, so intead of using an action, we add the classid to the generic Filter class. Now it is much easier to read it for recovery.
> 
> 
> Diffs
> -----
> 
>   src/linux/routing/filter/arp.hpp fa0ea6f93218fb3bee9f435f0268022d5edc5e11 
>   src/linux/routing/filter/arp.cpp bf19264ccc83e8a8b8c2affe53779ac070472925 
>   src/linux/routing/filter/filter.hpp d4ea099a2e35ffc0e6d560c5de9864dbbfe10038 
>   src/linux/routing/filter/icmp.hpp 431bc19eb600b0568874dacfce123adef0f9ff0b 
>   src/linux/routing/filter/icmp.cpp 706b5d1871d9e0b79d1b869d27d0e87112b75975 
>   src/linux/routing/filter/internal.hpp 8a6c0c0390c4ed119a11f0f808e0a244f7734c45 
>   src/linux/routing/filter/ip.hpp b5406024fdbfd1510bcf619f04a7369763396e72 
>   src/linux/routing/filter/ip.cpp de6407119c6726e0f32b07fd6ff61fa29262638b 
>   src/linux/routing/queueing/handle.hpp 2725d0794ca29ad5dc1b0148d0f68b90ce8b8369 
>   src/tests/routing_tests.cpp 7cc3b57a3b71544874557d2b1cf88a241b7062ba 
> 
> Diff: https://reviews.apache.org/r/31503/diff/
> 
> 
> Testing
> -------
> 
> Run the testcase.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>


Re: Review Request 31503: Add classid to Filter

Posted by Cong Wang <cw...@twopensource.com>.

> On March 12, 2015, 2:02 a.m., Jie Yu wrote:
> > Cong, I addressed the comments for you and try to commit. Looks like rtnl_u32_get_classid is not in libnl-3.2.25. That means we need a newer version of libnl for this to work!
> 
> Cong Wang wrote:
>     I have addressed your comments locally, just haven't uploaded a new version.
>     Yes, we need this commit from upstream:
>     https://github.com/thom311/libnl/commit/d8f080d94fa9cf5e977f8805446ac7ef39f82d31
> 
> Cong Wang wrote:
>     3.2.26 will be released soon, according to upstream maintainer. So how about adding a check in configure.ac?
>     
>     +  # Check for libnl-route (both headers and libraries).
>     +  AC_CHECK_LIB([nl-route-3], [rtnl_u32_get_classid], [],
>     +               [AC_MSG_ERROR([cannot find libnl-route-3
>     +-------------------------------------------------------------------
>     +We need libnl-route-3 for building network isolator!
>     +
>     +Please install libnl3 (version 3.2.26 or higher):
>     +http://www.infradead.org/~tgr/libnl/
> 
> Ian Downes wrote:
>     Do you have an estimated date for the release? Can we clean up some of the documentation if we add the check?

3.2.26-rc1 was released on Mar 10, if there is no major bug reported 3.2.26 will be release ~2 weeks after that, that is about 24 Mar.


- Cong


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


On March 10, 2015, 11:26 p.m., Cong Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31503/
> -----------------------------------------------------------
> 
> (Updated March 10, 2015, 11:26 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
> -------
> 
> Currently we don't support to read actions, so intead of using an action, we add the classid to the generic Filter class. Now it is much easier to read it for recovery.
> 
> 
> Diffs
> -----
> 
>   src/linux/routing/filter/arp.hpp fa0ea6f93218fb3bee9f435f0268022d5edc5e11 
>   src/linux/routing/filter/arp.cpp bf19264ccc83e8a8b8c2affe53779ac070472925 
>   src/linux/routing/filter/filter.hpp d4ea099a2e35ffc0e6d560c5de9864dbbfe10038 
>   src/linux/routing/filter/icmp.hpp 431bc19eb600b0568874dacfce123adef0f9ff0b 
>   src/linux/routing/filter/icmp.cpp 706b5d1871d9e0b79d1b869d27d0e87112b75975 
>   src/linux/routing/filter/internal.hpp 8a6c0c0390c4ed119a11f0f808e0a244f7734c45 
>   src/linux/routing/filter/ip.hpp b5406024fdbfd1510bcf619f04a7369763396e72 
>   src/linux/routing/filter/ip.cpp de6407119c6726e0f32b07fd6ff61fa29262638b 
>   src/linux/routing/queueing/handle.hpp 2725d0794ca29ad5dc1b0148d0f68b90ce8b8369 
>   src/tests/routing_tests.cpp 7cc3b57a3b71544874557d2b1cf88a241b7062ba 
> 
> Diff: https://reviews.apache.org/r/31503/diff/
> 
> 
> Testing
> -------
> 
> Run the testcase.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>


Re: Review Request 31503: Add classid to Filter

Posted by Cong Wang <cw...@twopensource.com>.

> On March 12, 2015, 2:02 a.m., Jie Yu wrote:
> > Cong, I addressed the comments for you and try to commit. Looks like rtnl_u32_get_classid is not in libnl-3.2.25. That means we need a newer version of libnl for this to work!
> 
> Cong Wang wrote:
>     I have addressed your comments locally, just haven't uploaded a new version.
>     Yes, we need this commit from upstream:
>     https://github.com/thom311/libnl/commit/d8f080d94fa9cf5e977f8805446ac7ef39f82d31

3.2.26 will be released soon, according to upstream maintainer. So how about adding a check in configure.ac?

+  # Check for libnl-route (both headers and libraries).
+  AC_CHECK_LIB([nl-route-3], [rtnl_u32_get_classid], [],
+               [AC_MSG_ERROR([cannot find libnl-route-3
+-------------------------------------------------------------------
+We need libnl-route-3 for building network isolator!
+
+Please install libnl3 (version 3.2.26 or higher):
+http://www.infradead.org/~tgr/libnl/


- Cong


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


On March 10, 2015, 11:26 p.m., Cong Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31503/
> -----------------------------------------------------------
> 
> (Updated March 10, 2015, 11:26 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
> -------
> 
> Currently we don't support to read actions, so intead of using an action, we add the classid to the generic Filter class. Now it is much easier to read it for recovery.
> 
> 
> Diffs
> -----
> 
>   src/linux/routing/filter/arp.hpp fa0ea6f93218fb3bee9f435f0268022d5edc5e11 
>   src/linux/routing/filter/arp.cpp bf19264ccc83e8a8b8c2affe53779ac070472925 
>   src/linux/routing/filter/filter.hpp d4ea099a2e35ffc0e6d560c5de9864dbbfe10038 
>   src/linux/routing/filter/icmp.hpp 431bc19eb600b0568874dacfce123adef0f9ff0b 
>   src/linux/routing/filter/icmp.cpp 706b5d1871d9e0b79d1b869d27d0e87112b75975 
>   src/linux/routing/filter/internal.hpp 8a6c0c0390c4ed119a11f0f808e0a244f7734c45 
>   src/linux/routing/filter/ip.hpp b5406024fdbfd1510bcf619f04a7369763396e72 
>   src/linux/routing/filter/ip.cpp de6407119c6726e0f32b07fd6ff61fa29262638b 
>   src/linux/routing/queueing/handle.hpp 2725d0794ca29ad5dc1b0148d0f68b90ce8b8369 
>   src/tests/routing_tests.cpp 7cc3b57a3b71544874557d2b1cf88a241b7062ba 
> 
> Diff: https://reviews.apache.org/r/31503/diff/
> 
> 
> Testing
> -------
> 
> Run the testcase.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>


Re: Review Request 31503: Add classid to Filter

Posted by Cong Wang <cw...@twopensource.com>.

> On March 12, 2015, 2:02 a.m., Jie Yu wrote:
> > Cong, I addressed the comments for you and try to commit. Looks like rtnl_u32_get_classid is not in libnl-3.2.25. That means we need a newer version of libnl for this to work!

I have addressed your comments locally, just haven't uploaded a new version.
Yes, we need this commit from upstream:
https://github.com/thom311/libnl/commit/d8f080d94fa9cf5e977f8805446ac7ef39f82d31


- Cong


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


On March 10, 2015, 11:26 p.m., Cong Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31503/
> -----------------------------------------------------------
> 
> (Updated March 10, 2015, 11:26 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
> -------
> 
> Currently we don't support to read actions, so intead of using an action, we add the classid to the generic Filter class. Now it is much easier to read it for recovery.
> 
> 
> Diffs
> -----
> 
>   src/linux/routing/filter/arp.hpp fa0ea6f93218fb3bee9f435f0268022d5edc5e11 
>   src/linux/routing/filter/arp.cpp bf19264ccc83e8a8b8c2affe53779ac070472925 
>   src/linux/routing/filter/filter.hpp d4ea099a2e35ffc0e6d560c5de9864dbbfe10038 
>   src/linux/routing/filter/icmp.hpp 431bc19eb600b0568874dacfce123adef0f9ff0b 
>   src/linux/routing/filter/icmp.cpp 706b5d1871d9e0b79d1b869d27d0e87112b75975 
>   src/linux/routing/filter/internal.hpp 8a6c0c0390c4ed119a11f0f808e0a244f7734c45 
>   src/linux/routing/filter/ip.hpp b5406024fdbfd1510bcf619f04a7369763396e72 
>   src/linux/routing/filter/ip.cpp de6407119c6726e0f32b07fd6ff61fa29262638b 
>   src/linux/routing/queueing/handle.hpp 2725d0794ca29ad5dc1b0148d0f68b90ce8b8369 
>   src/tests/routing_tests.cpp 7cc3b57a3b71544874557d2b1cf88a241b7062ba 
> 
> Diff: https://reviews.apache.org/r/31503/diff/
> 
> 
> Testing
> -------
> 
> Run the testcase.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>


Re: Review Request 31503: Add classid to Filter

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31503/#review76195
-----------------------------------------------------------


Cong, I addressed the comments for you and try to commit. Looks like rtnl_u32_get_classid is not in libnl-3.2.25. That means we need a newer version of libnl for this to work!

- Jie Yu


On March 10, 2015, 11:26 p.m., Cong Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31503/
> -----------------------------------------------------------
> 
> (Updated March 10, 2015, 11:26 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
> -------
> 
> Currently we don't support to read actions, so intead of using an action, we add the classid to the generic Filter class. Now it is much easier to read it for recovery.
> 
> 
> Diffs
> -----
> 
>   src/linux/routing/filter/arp.hpp fa0ea6f93218fb3bee9f435f0268022d5edc5e11 
>   src/linux/routing/filter/arp.cpp bf19264ccc83e8a8b8c2affe53779ac070472925 
>   src/linux/routing/filter/filter.hpp d4ea099a2e35ffc0e6d560c5de9864dbbfe10038 
>   src/linux/routing/filter/icmp.hpp 431bc19eb600b0568874dacfce123adef0f9ff0b 
>   src/linux/routing/filter/icmp.cpp 706b5d1871d9e0b79d1b869d27d0e87112b75975 
>   src/linux/routing/filter/internal.hpp 8a6c0c0390c4ed119a11f0f808e0a244f7734c45 
>   src/linux/routing/filter/ip.hpp b5406024fdbfd1510bcf619f04a7369763396e72 
>   src/linux/routing/filter/ip.cpp de6407119c6726e0f32b07fd6ff61fa29262638b 
>   src/linux/routing/queueing/handle.hpp 2725d0794ca29ad5dc1b0148d0f68b90ce8b8369 
>   src/tests/routing_tests.cpp 7cc3b57a3b71544874557d2b1cf88a241b7062ba 
> 
> Diff: https://reviews.apache.org/r/31503/diff/
> 
> 
> Testing
> -------
> 
> Run the testcase.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>


Re: Review Request 31503: Add classid to Filter

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31503/#review75999
-----------------------------------------------------------

Ship it!



src/linux/routing/filter/arp.hpp
<https://reviews.apache.org/r/31503/#comment123386>

    s/flowid/classid/



src/linux/routing/filter/arp.hpp
<https://reviews.apache.org/r/31503/#comment123376>

    Can you consistently use `classid` for Filter? The word 'flowid' is used by the isolator and should be the lower 16 bits.



src/linux/routing/filter/arp.cpp
<https://reviews.apache.org/r/31503/#comment123381>

    Ditto. Please use 'classid' here.



src/linux/routing/filter/arp.cpp
<https://reviews.apache.org/r/31503/#comment123382>

    Ditto.



src/linux/routing/filter/filter.hpp
<https://reviews.apache.org/r/31503/#comment123383>

    Ha, you call it 'classid' here!



src/linux/routing/filter/filter.hpp
<https://reviews.apache.org/r/31503/#comment123385>

    WE may wanna add classful qdisc in the future. So just say that we use 'classid' consistently in the code base.



src/linux/routing/filter/icmp.cpp
<https://reviews.apache.org/r/31503/#comment123387>

    Ditto.



src/linux/routing/filter/ip.hpp
<https://reviews.apache.org/r/31503/#comment123389>

    Ditto.



src/linux/routing/filter/ip.cpp
<https://reviews.apache.org/r/31503/#comment123390>

    Ditto.



src/linux/routing/queueing/handle.hpp
<https://reviews.apache.org/r/31503/#comment123391>

    Please add a comment about this function. It's not quite obvious what this constructor is doing.


- Jie Yu


On March 10, 2015, 11:26 p.m., Cong Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31503/
> -----------------------------------------------------------
> 
> (Updated March 10, 2015, 11:26 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
> -------
> 
> Currently we don't support to read actions, so intead of using an action, we add the classid to the generic Filter class. Now it is much easier to read it for recovery.
> 
> 
> Diffs
> -----
> 
>   src/linux/routing/filter/arp.hpp fa0ea6f93218fb3bee9f435f0268022d5edc5e11 
>   src/linux/routing/filter/arp.cpp bf19264ccc83e8a8b8c2affe53779ac070472925 
>   src/linux/routing/filter/filter.hpp d4ea099a2e35ffc0e6d560c5de9864dbbfe10038 
>   src/linux/routing/filter/icmp.hpp 431bc19eb600b0568874dacfce123adef0f9ff0b 
>   src/linux/routing/filter/icmp.cpp 706b5d1871d9e0b79d1b869d27d0e87112b75975 
>   src/linux/routing/filter/internal.hpp 8a6c0c0390c4ed119a11f0f808e0a244f7734c45 
>   src/linux/routing/filter/ip.hpp b5406024fdbfd1510bcf619f04a7369763396e72 
>   src/linux/routing/filter/ip.cpp de6407119c6726e0f32b07fd6ff61fa29262638b 
>   src/linux/routing/queueing/handle.hpp 2725d0794ca29ad5dc1b0148d0f68b90ce8b8369 
>   src/tests/routing_tests.cpp 7cc3b57a3b71544874557d2b1cf88a241b7062ba 
> 
> Diff: https://reviews.apache.org/r/31503/diff/
> 
> 
> Testing
> -------
> 
> Run the testcase.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>


Re: Review Request 31503: Add classid to Filter

Posted by Cong Wang <cw...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31503/
-----------------------------------------------------------

(Updated March 10, 2015, 11:26 p.m.)


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


Summary (updated)
-----------------

Add classid to Filter


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


Repository: mesos


Description (updated)
-------

Currently we don't support to read actions, so intead of using an action, we add the classid to the generic Filter class. Now it is much easier to read it for recovery.


Diffs (updated)
-----

  src/linux/routing/filter/arp.hpp fa0ea6f93218fb3bee9f435f0268022d5edc5e11 
  src/linux/routing/filter/arp.cpp bf19264ccc83e8a8b8c2affe53779ac070472925 
  src/linux/routing/filter/filter.hpp d4ea099a2e35ffc0e6d560c5de9864dbbfe10038 
  src/linux/routing/filter/icmp.hpp 431bc19eb600b0568874dacfce123adef0f9ff0b 
  src/linux/routing/filter/icmp.cpp 706b5d1871d9e0b79d1b869d27d0e87112b75975 
  src/linux/routing/filter/internal.hpp 8a6c0c0390c4ed119a11f0f808e0a244f7734c45 
  src/linux/routing/filter/ip.hpp b5406024fdbfd1510bcf619f04a7369763396e72 
  src/linux/routing/filter/ip.cpp de6407119c6726e0f32b07fd6ff61fa29262638b 
  src/linux/routing/queueing/handle.hpp 2725d0794ca29ad5dc1b0148d0f68b90ce8b8369 
  src/tests/routing_tests.cpp 7cc3b57a3b71544874557d2b1cf88a241b7062ba 

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


Testing
-------

Run the testcase.


Thanks,

Cong Wang


Re: Review Request 31503: (2/5) Add the FlowId action

Posted by Cong Wang <cw...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31503/
-----------------------------------------------------------

(Updated March 9, 2015, 6:29 p.m.)


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


Changes
-------

Address review comment


Summary (updated)
-----------------

(2/5) Add the FlowId action


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


Repository: mesos


Description
-------

Introduce a new action which saves the classid for a specified filter. Similar to the u32 terminal action, it is not a real action defined by TC, it is just for setting a filter attribute. The classid or flowid is used to match the fq_codel flows.


Diffs (updated)
-----

  src/linux/routing/filter/action.hpp 410c15a 
  src/linux/routing/filter/arp.hpp fa0ea6f 
  src/linux/routing/filter/arp.cpp bf19264 
  src/linux/routing/filter/icmp.hpp 0237f1a 
  src/linux/routing/filter/icmp.cpp 9076af3 
  src/linux/routing/filter/internal.hpp 8a6c0c0 
  src/linux/routing/filter/ip.hpp 3954962 
  src/linux/routing/filter/ip.cpp 059ad6e 
  src/linux/routing/queueing/handle.hpp 2725d07 
  src/tests/routing_tests.cpp 3cda6ab 

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


Testing
-------

Run the testcase.


Thanks,

Cong Wang


Re: Review Request 31503: (2/6) Add the FlowId action

Posted by Cong Wang <cw...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31503/
-----------------------------------------------------------

(Updated March 4, 2015, 8:06 p.m.)


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


Changes
-------

Address review comments


Summary (updated)
-----------------

(2/6) Add the FlowId action


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


Repository: mesos


Description
-------

Introduce a new action which saves the classid for a specified filter. Similar to the u32 terminal action, it is not a real action defined by TC, it is just for setting a filter attribute. The classid or flowid is used to match the fq_codel flows.


Diffs (updated)
-----

  src/linux/routing/filter/action.hpp 410c15a 
  src/linux/routing/filter/arp.hpp fa0ea6f 
  src/linux/routing/filter/arp.cpp bf19264 
  src/linux/routing/filter/icmp.hpp 7b478c4 
  src/linux/routing/filter/icmp.cpp 86bd67b 
  src/linux/routing/filter/internal.hpp 8a6c0c0 
  src/linux/routing/filter/ip.hpp 932ed4b 
  src/linux/routing/filter/ip.cpp 922a732 
  src/linux/routing/queueing/handle.hpp 2725d07 
  src/tests/routing_tests.cpp 3cda6ab 

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


Testing
-------

Run the testcase.


Thanks,

Cong Wang


Re: Review Request 31503: (2/4) Add the Classifier action

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31503/#review74856
-----------------------------------------------------------



src/tests/routing_tests.cpp
<https://reviews.apache.org/r/31503/#comment121656>

    Please do not remove this test and adjust the comments here saying that if this check fails, then  there is a bug in the kernel that needs to be fixed.


- Jie Yu


On Feb. 27, 2015, 9:39 p.m., Cong Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31503/
> -----------------------------------------------------------
> 
> (Updated Feb. 27, 2015, 9:39 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
> -------
> 
> Introduce a new action which saves the classid for a specified filter. Similar to the u32 terminal action, it is not a real action defined by TC, it is just for setting a filter attribute. The classid or flowid is used to match the fq_codel flows.
> 
> 
> Diffs
> -----
> 
>   src/linux/routing/filter/action.hpp 410c15ad20ffa694bdb4c4187f1544ef9617ebac 
>   src/linux/routing/filter/arp.hpp fa0ea6f93218fb3bee9f435f0268022d5edc5e11 
>   src/linux/routing/filter/arp.cpp bf19264ccc83e8a8b8c2affe53779ac070472925 
>   src/linux/routing/filter/icmp.hpp 7b478c4e23a2fb0895021ce5fb30bdf78131b871 
>   src/linux/routing/filter/icmp.cpp 86bd67b71a590b88344adbe10fd1b44ea1b5148d 
>   src/linux/routing/filter/internal.hpp 8a6c0c0390c4ed119a11f0f808e0a244f7734c45 
>   src/linux/routing/filter/ip.hpp 932ed4bbf57e60261ff3b48ae242a7dd5e1f4260 
>   src/linux/routing/filter/ip.cpp 922a732c3543a072674208b123fdfadbef2b15f2 
>   src/tests/routing_tests.cpp 3cda6ab8c1ad24e4b7d0b9aeda2abc595fc857a5 
> 
> Diff: https://reviews.apache.org/r/31503/diff/
> 
> 
> Testing
> -------
> 
> Run the testcase.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>


Re: Review Request 31503: (2/4) Add the Classifier action

Posted by Chi Zhang <ch...@gmail.com>.

> On March 2, 2015, 9:36 p.m., Chi Zhang wrote:
> > src/tests/routing_tests.cpp, line 446
> > <https://reviews.apache.org/r/31503/diff/1/?file=879078#file879078line446>
> >
> >     If you can have a way to programmatically determine the existence of this condition, this test should be checked before enabling (environemnt.cpp has control for filtering tests; sometimes we also do it in the test fixture). 
> >     
> >     If that's impossible, you should print out error messages to explain this and give instructions to disable this test manually. src/tests has examples for that.
> >     
> >     EXPECT(condition) << errMsg;
> >     
> >     Also could you provide a link to the email thread or webpage where this bug is discussed / fixed in upstream for context (like how we document kernel issues in port_mapping.cpp)
> 
> Cong Wang wrote:
>     Hmm, it is possible, I will check how much effort we will need to detect this. At least I can just add a TODO saying we could detect this kernel bug.
>     
>     The kernel changelog of that commit already has these information I believe, or you mean I should add a web link to in the comment of the code?

Really just a nit: we did something like this in port_mapping.cpp:

// We disable 'rp_filter' and 'send_redirects' for host loopback                                                                               
// interface (lo) to work around a kernel bug, which was only                                                                                  
// recently addressed in upstream in the following 3 commits.                                                                                  
// https://git.kernel.org/cgit/linux/kernel/git/davem/net.git/:                                                                                
//   6a662719c9868b3d6c7d26b3a085f0cd3cc15e64                                                                                                  
//   0d5edc68739f1c1e0519acbea1d3f0c1882a15d7                                                                                                  
//   e374c618b1465f0292047a9f4c244bd71ab5f1f0


- Chi


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


On Feb. 27, 2015, 9:39 p.m., Cong Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31503/
> -----------------------------------------------------------
> 
> (Updated Feb. 27, 2015, 9:39 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
> -------
> 
> Introduce a new action which saves the classid for a specified filter. Similar to the u32 terminal action, it is not a real action defined by TC, it is just for setting a filter attribute. The classid or flowid is used to match the fq_codel flows.
> 
> 
> Diffs
> -----
> 
>   src/linux/routing/filter/action.hpp 410c15ad20ffa694bdb4c4187f1544ef9617ebac 
>   src/linux/routing/filter/arp.hpp fa0ea6f93218fb3bee9f435f0268022d5edc5e11 
>   src/linux/routing/filter/arp.cpp bf19264ccc83e8a8b8c2affe53779ac070472925 
>   src/linux/routing/filter/icmp.hpp 7b478c4e23a2fb0895021ce5fb30bdf78131b871 
>   src/linux/routing/filter/icmp.cpp 86bd67b71a590b88344adbe10fd1b44ea1b5148d 
>   src/linux/routing/filter/internal.hpp 8a6c0c0390c4ed119a11f0f808e0a244f7734c45 
>   src/linux/routing/filter/ip.hpp 932ed4bbf57e60261ff3b48ae242a7dd5e1f4260 
>   src/linux/routing/filter/ip.cpp 922a732c3543a072674208b123fdfadbef2b15f2 
>   src/tests/routing_tests.cpp 3cda6ab8c1ad24e4b7d0b9aeda2abc595fc857a5 
> 
> Diff: https://reviews.apache.org/r/31503/diff/
> 
> 
> Testing
> -------
> 
> Run the testcase.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>


Re: Review Request 31503: (2/4) Add the Classifier action

Posted by Cong Wang <cw...@twopensource.com>.

> On March 2, 2015, 9:36 p.m., Chi Zhang wrote:
> > src/linux/routing/filter/internal.hpp, line 274
> > <https://reviews.apache.org/r/31503/diff/1/?file=879075#file879075line274>
> >
> >     The function comment looks like you are not doing anything with any non-u32 filters but returning error. Maybe document what you are doing with basic filters as well?

Ah, comment is out of date, fixed now.


- Cong


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


On Feb. 27, 2015, 9:39 p.m., Cong Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31503/
> -----------------------------------------------------------
> 
> (Updated Feb. 27, 2015, 9:39 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
> -------
> 
> Introduce a new action which saves the classid for a specified filter. Similar to the u32 terminal action, it is not a real action defined by TC, it is just for setting a filter attribute. The classid or flowid is used to match the fq_codel flows.
> 
> 
> Diffs
> -----
> 
>   src/linux/routing/filter/action.hpp 410c15ad20ffa694bdb4c4187f1544ef9617ebac 
>   src/linux/routing/filter/arp.hpp fa0ea6f93218fb3bee9f435f0268022d5edc5e11 
>   src/linux/routing/filter/arp.cpp bf19264ccc83e8a8b8c2affe53779ac070472925 
>   src/linux/routing/filter/icmp.hpp 7b478c4e23a2fb0895021ce5fb30bdf78131b871 
>   src/linux/routing/filter/icmp.cpp 86bd67b71a590b88344adbe10fd1b44ea1b5148d 
>   src/linux/routing/filter/internal.hpp 8a6c0c0390c4ed119a11f0f808e0a244f7734c45 
>   src/linux/routing/filter/ip.hpp 932ed4bbf57e60261ff3b48ae242a7dd5e1f4260 
>   src/linux/routing/filter/ip.cpp 922a732c3543a072674208b123fdfadbef2b15f2 
>   src/tests/routing_tests.cpp 3cda6ab8c1ad24e4b7d0b9aeda2abc595fc857a5 
> 
> Diff: https://reviews.apache.org/r/31503/diff/
> 
> 
> Testing
> -------
> 
> Run the testcase.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>


Re: Review Request 31503: (2/4) Add the Classifier action

Posted by Cong Wang <cw...@twopensource.com>.

> On March 2, 2015, 9:36 p.m., Chi Zhang wrote:
> > src/tests/routing_tests.cpp, line 446
> > <https://reviews.apache.org/r/31503/diff/1/?file=879078#file879078line446>
> >
> >     If you can have a way to programmatically determine the existence of this condition, this test should be checked before enabling (environemnt.cpp has control for filtering tests; sometimes we also do it in the test fixture). 
> >     
> >     If that's impossible, you should print out error messages to explain this and give instructions to disable this test manually. src/tests has examples for that.
> >     
> >     EXPECT(condition) << errMsg;
> >     
> >     Also could you provide a link to the email thread or webpage where this bug is discussed / fixed in upstream for context (like how we document kernel issues in port_mapping.cpp)

Hmm, it is possible, I will check how much effort we will need to detect this. At least I can just add a TODO saying we could detect this kernel bug.

The kernel changelog of that commit already has these information I believe, or you mean I should add a web link to in the comment of the code?


- Cong


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


On Feb. 27, 2015, 9:39 p.m., Cong Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31503/
> -----------------------------------------------------------
> 
> (Updated Feb. 27, 2015, 9:39 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
> -------
> 
> Introduce a new action which saves the classid for a specified filter. Similar to the u32 terminal action, it is not a real action defined by TC, it is just for setting a filter attribute. The classid or flowid is used to match the fq_codel flows.
> 
> 
> Diffs
> -----
> 
>   src/linux/routing/filter/action.hpp 410c15ad20ffa694bdb4c4187f1544ef9617ebac 
>   src/linux/routing/filter/arp.hpp fa0ea6f93218fb3bee9f435f0268022d5edc5e11 
>   src/linux/routing/filter/arp.cpp bf19264ccc83e8a8b8c2affe53779ac070472925 
>   src/linux/routing/filter/icmp.hpp 7b478c4e23a2fb0895021ce5fb30bdf78131b871 
>   src/linux/routing/filter/icmp.cpp 86bd67b71a590b88344adbe10fd1b44ea1b5148d 
>   src/linux/routing/filter/internal.hpp 8a6c0c0390c4ed119a11f0f808e0a244f7734c45 
>   src/linux/routing/filter/ip.hpp 932ed4bbf57e60261ff3b48ae242a7dd5e1f4260 
>   src/linux/routing/filter/ip.cpp 922a732c3543a072674208b123fdfadbef2b15f2 
>   src/tests/routing_tests.cpp 3cda6ab8c1ad24e4b7d0b9aeda2abc595fc857a5 
> 
> Diff: https://reviews.apache.org/r/31503/diff/
> 
> 
> Testing
> -------
> 
> Run the testcase.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>


Re: Review Request 31503: (2/4) Add the Classifier action

Posted by Chi Zhang <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31503/#review74806
-----------------------------------------------------------



src/linux/routing/filter/action.hpp
<https://reviews.apache.org/r/31503/#comment121568>

    The name 'classifier' is used in filter.hpp; I feel like also using it for action might lead to confusion.



src/linux/routing/filter/internal.hpp
<https://reviews.apache.org/r/31503/#comment121573>

    "Failed to set the classifier _action_" (thats why suggested using a different name :)



src/linux/routing/filter/internal.hpp
<https://reviews.apache.org/r/31503/#comment121576>

    If the classifier action was set but setting terminal had an error. What would happen? is rewinding necessary?



src/linux/routing/filter/internal.hpp
<https://reviews.apache.org/r/31503/#comment121577>

    The function comment looks like you are not doing anything with any non-u32 filters but returning error. Maybe document what you are doing with basic filters as well?



src/linux/routing/filter/internal.hpp
<https://reviews.apache.org/r/31503/#comment121570>

    Be consistent with other err msgs? :)
    
    "Failed to do XXX: errMsg"



src/linux/routing/filter/ip.hpp
<https://reviews.apache.org/r/31503/#comment121579>

    Let's give this public interface some comments like other functions in this file?



src/tests/routing_tests.cpp
<https://reviews.apache.org/r/31503/#comment121601>

    If you can have a way to programmatically determine the existence of this condition, this test should be checked before enabling (environemnt.cpp has control for filtering tests; sometimes we also do it in the test fixture). 
    
    If that's impossible, you should print out error messages to explain this and give instructions to disable this test manually. src/tests has examples for that.
    
    EXPECT(condition) << errMsg;
    
    Also could you provide a link to the email thread or webpage where this bug is discussed / fixed in upstream for context (like how we document kernel issues in port_mapping.cpp)



src/tests/routing_tests.cpp
<https://reviews.apache.org/r/31503/#comment121602>

    kill this line



src/tests/routing_tests.cpp
<https://reviews.apache.org/r/31503/#comment121604>

    kill this line.


- Chi Zhang


On Feb. 27, 2015, 9:39 p.m., Cong Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31503/
> -----------------------------------------------------------
> 
> (Updated Feb. 27, 2015, 9:39 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
> -------
> 
> Introduce a new action which saves the classid for a specified filter. Similar to the u32 terminal action, it is not a real action defined by TC, it is just for setting a filter attribute. The classid or flowid is used to match the fq_codel flows.
> 
> 
> Diffs
> -----
> 
>   src/linux/routing/filter/action.hpp 410c15ad20ffa694bdb4c4187f1544ef9617ebac 
>   src/linux/routing/filter/arp.hpp fa0ea6f93218fb3bee9f435f0268022d5edc5e11 
>   src/linux/routing/filter/arp.cpp bf19264ccc83e8a8b8c2affe53779ac070472925 
>   src/linux/routing/filter/icmp.hpp 7b478c4e23a2fb0895021ce5fb30bdf78131b871 
>   src/linux/routing/filter/icmp.cpp 86bd67b71a590b88344adbe10fd1b44ea1b5148d 
>   src/linux/routing/filter/internal.hpp 8a6c0c0390c4ed119a11f0f808e0a244f7734c45 
>   src/linux/routing/filter/ip.hpp 932ed4bbf57e60261ff3b48ae242a7dd5e1f4260 
>   src/linux/routing/filter/ip.cpp 922a732c3543a072674208b123fdfadbef2b15f2 
>   src/tests/routing_tests.cpp 3cda6ab8c1ad24e4b7d0b9aeda2abc595fc857a5 
> 
> Diff: https://reviews.apache.org/r/31503/diff/
> 
> 
> Testing
> -------
> 
> Run the testcase.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>


Re: Review Request 31503: (2/4) Add the Classifier action

Posted by Cong Wang <cw...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31503/
-----------------------------------------------------------

(Updated Feb. 27, 2015, 9:39 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
-------

Introduce a new action which saves the classid for a specified filter. Similar to the u32 terminal action, it is not a real action defined by TC, it is just for setting a filter attribute. The classid or flowid is used to match the fq_codel flows.


Diffs
-----

  src/linux/routing/filter/action.hpp 410c15ad20ffa694bdb4c4187f1544ef9617ebac 
  src/linux/routing/filter/arp.hpp fa0ea6f93218fb3bee9f435f0268022d5edc5e11 
  src/linux/routing/filter/arp.cpp bf19264ccc83e8a8b8c2affe53779ac070472925 
  src/linux/routing/filter/icmp.hpp 7b478c4e23a2fb0895021ce5fb30bdf78131b871 
  src/linux/routing/filter/icmp.cpp 86bd67b71a590b88344adbe10fd1b44ea1b5148d 
  src/linux/routing/filter/internal.hpp 8a6c0c0390c4ed119a11f0f808e0a244f7734c45 
  src/linux/routing/filter/ip.hpp 932ed4bbf57e60261ff3b48ae242a7dd5e1f4260 
  src/linux/routing/filter/ip.cpp 922a732c3543a072674208b123fdfadbef2b15f2 
  src/tests/routing_tests.cpp 3cda6ab8c1ad24e4b7d0b9aeda2abc595fc857a5 

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


Testing
-------

Run the testcase.


Thanks,

Cong Wang