[ovs-dev] [PATCH] datapath: More strict vlan encap netlink check

[Jesse Gross]

On Thu, Aug 22, 2013 at 1:13 PM, Andy Zhou <azhou at nicira.com> wrote:
> On Thu, Aug 22, 2013 at 11:37 AM, Jesse Gross <jesse at nicira.com> wrote:
>>
>> On Thu, Aug 15, 2013 at 2:16 PM, Andy Zhou <azhou at 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?

No, I just applied the patch with this change. I just wanted to confirm first.