> 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());
> > ...
> > }
> > ```
>
> Cong Wang wrote:
> 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.
First of all, I am not convinced that merging arp and basic filters will be
hard in the way i just described. For example:
```
filter::basic::create(
...
basic::Classifier(ETH_P_ARP),
...);
```
Also, regarding your second argument. In mesos, we usually value code cleaness
and readability than performance and duplication.
- Jie
-----------------------------------------------------------
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
>
>
