> On March 5, 2015, 11:12 p.m., Jie Yu wrote:
> > I don't think adding `protocol` to `Filter` is a good idea. We should keep
> > `protocol` in `Classifier` because the `protocol` is used for classifying
> > packets, isn't it?
> >
> > Regarding the basic filter, you need to something the following:
> > ```
> > struct basic::Classifier
> > {
> > uint16_t protocol;
> > };
> >
> > Result<basic::Classifier> decode<basic::Classifier>(...)
> > {
> > if (rtnl_tc_get_kind(TC_CAST(cls.get())) == string("basic")) {
> > return basic::Classifier(rtnl_cls_get_protocol(cls.get()));
> > }
> >
> > return None();
> > }
> >
> > template <>
> > Try<Nothing> encode<basic::Classifier>(...)
> > {
> > rtnl_cls_set_protocol(cls.get(), classifier.protocol());
> > ...
> > }
> > ```
Our goal is not just to make basic filter work for my case, as we discussed,
arp filter will eventually (after this patchset) move on top of basic filter
too, because their only difference is protocol. Therefore, leaving protocol
inside basic::Classifier is not as good as exposing it in parameter, so that in
future arp::create() can simply call basic::create(..., ETH_P_ARP)? Also, my
way could save a little bit of code since protocol setting/getting is moved to
the generic layer.
- Cong
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31499/#review75408
-----------------------------------------------------------
On March 4, 2015, 8:10 p.m., Cong Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31499/
> -----------------------------------------------------------
>
> (Updated March 4, 2015, 8:10 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
> -------
>
> Protocol is a generic attribute for a filter, so it makes sense to move it
> into Filter. This is a preparation for the next patch.
>
>
> Diffs
> -----
>
> src/linux/routing/filter/arp.cpp bf19264
> src/linux/routing/filter/filter.hpp a603d73
> src/linux/routing/filter/icmp.cpp 86bd67b
> src/linux/routing/filter/internal.hpp 8a6c0c0
> src/linux/routing/filter/ip.cpp 922a732
>
> Diff: https://reviews.apache.org/r/31499/diff/
>
>
> Testing
> -------
>
> Run test cases.
>
>
> Thanks,
>
> Cong Wang
>
>
