On Thu, Aug 22, 2013 at 11:37 AM, Jesse Gross <je...@nicira.com> wrote:
> On Thu, Aug 15, 2013 at 2:16 PM, Andy Zhou <az...@nicira.com> wrote:
> > diff --git a/datapath/flow.c b/datapath/flow.c
> > index 4075350..a36a1ea 100644
> > --- a/datapath/flow.c
> > +++ b/datapath/flow.c
> > @@ -1585,26 +1585,37 @@ int ovs_match_from_nlattrs(struct sw_flow_match
> *match,
> > if (err)
> > return err;
> >
> > - if (key_attrs & 1ULL << OVS_KEY_ATTR_ENCAP) {
> > - encap = a[OVS_KEY_ATTR_ENCAP];
> > - key_attrs &= ~(1ULL << OVS_KEY_ATTR_ENCAP);
> > - if (nla_len(encap)) {
> > - __be16 eth_type = 0; /* ETH_P_8021Q */
> > + if ((key_attrs & (1ULL << OVS_KEY_ATTR_ETHERNET)) &&
> > + (key_attrs & (1ULL << OVS_KEY_ATTR_ETHERTYPE)) &&
> > + (nla_get_be16(a[OVS_KEY_ATTR_ETHERTYPE]) ==
> htons(ETH_P_8021Q))) {
> > + __be16 tci;
> >
> > - if (a[OVS_KEY_ATTR_ETHERTYPE])
> > - eth_type =
> nla_get_be16(a[OVS_KEY_ATTR_ETHERTYPE]);
> > + if (!((key_attrs & (1ULL << OVS_KEY_ATTR_VLAN)) &&
> > + (key_attrs & (1ULL << OVS_KEY_ATTR_ETHERTYPE)) &&
> > + (key_attrs & (1ULL << OVS_KEY_ATTR_ENCAP)))) {
>
> Thanks, I think this is a big improvement. One question: it seems like
> we check EtherType twice in the above two blocks. Is this just
> redundant or is there something the second check is supposed to catch?
>
Good catch, It is simply redundant & can be removed. Do you need a V2?
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
