[ovs-dev] [PATCH] datapath: Kernel flow metadata parsing should be less restrictive

Jesse Gross jesse at nicira.com
Wed Nov 9 17:26:01 UTC 2011


On Wed, Nov 9, 2011 at 9:18 AM, Ansis Atteka <aatteka at nicira.com> wrote:
> On Tue, Nov 8, 2011 at 4:02 PM, Jesse Gross <jesse at nicira.com> wrote:
>> On Tue, Nov 8, 2011 at 3:11 PM, Ansis Atteka <aatteka at nicira.com> wrote:
>> > -               switch (TRANSITION(prev_type, type)) {
>> > -               case TRANSITION(OVS_KEY_ATTR_UNSPEC,
>> > OVS_KEY_ATTR_PRIORITY):
>> > +               switch (type) {
>> > +               case OVS_KEY_ATTR_PRIORITY:
>> >                        *priority = nla_get_u32(nla);
>> >                        break;
>> >
>> > -               case TRANSITION(OVS_KEY_ATTR_UNSPEC,
>> > OVS_KEY_ATTR_TUN_ID):
>> > -               case TRANSITION(OVS_KEY_ATTR_PRIORITY,
>> > OVS_KEY_ATTR_TUN_ID):
>> > +               case OVS_KEY_ATTR_TUN_ID:
>> >                        *tun_id = nla_get_be64(nla);
>> >                        break;
>> >
>> > -               case TRANSITION(OVS_KEY_ATTR_UNSPEC,
>> > OVS_KEY_ATTR_IN_PORT):
>> > -               case TRANSITION(OVS_KEY_ATTR_PRIORITY,
>> > OVS_KEY_ATTR_IN_PORT):
>> > -               case TRANSITION(OVS_KEY_ATTR_TUN_ID,
>> > OVS_KEY_ATTR_IN_PORT):
>> > +               case OVS_KEY_ATTR_IN_PORT:
>> >                        if (nla_get_u32(nla) >= DP_MAX_PORTS)
>> >                                return -EINVAL;
>> >                        *in_port = nla_get_u32(nla);
>> >                        break;
>> >
>> >                default:
>> > -                       return 0;
>> > +                       break;
>>
>> If we don't hit one of the above cases then we should not break but
>> keep on parsing attributes.
>
> Not sure that I understood this one correctly. The only two places where we
> prematurely exit from nla_for_each_nested() loop are:
> 1. (type > OVS_KEY_ATTR_MAX || nla_len(nla) != ovs_key_lens[type]); and
> 2. (nla_get_u32(nla) >= DP_MAX_PORTS)
>
> As per your comment I will remove the first one.

There are other fields that aren't metadata fields that we understand
but are ignored by this function.  If we want to be ordering
independent then it's possible that those might be mixed in with the
metadata fields.  In this situation, we will hit the default condition
and prematurely break out of the loop.



More information about the dev mailing list