[ovs-dev] [RFC PATCH 3/4] datapath: Minimize ovs_flow_cmd_new_or_set critical section.

Jarno Rajahalme jrajahalme at nicira.com
Wed Feb 5 18:16:53 UTC 2014


> On Feb 3, 2014, at 10:38 AM, Jesse Gross <jesse at nicira.com> wrote:
> 
>> On Fri, Jan 24, 2014 at 2:58 PM, Jarno Rajahalme <jrajahalme at nicira.com> wrote:
>> diff --git a/datapath/datapath.c b/datapath/datapath.c
>> index 92ae66a..5b12a5d 100644
>> --- a/datapath/datapath.c
>> +++ b/datapath/datapath.c
>> @@ -817,11 +840,27 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
>>                        OVS_NLERR("Flow actions may not be safe on all matching packets.\n");
>>                        goto err_kfree;
>>                }
>> -       } else if (info->genlhdr->cmd == OVS_FLOW_CMD_NEW) {
>> +       } else if (cmd == OVS_FLOW_CMD_NEW) {
>>                error = -EINVAL;
>>                goto error;
>>        }
>> +       /* XXX: Is it OK to SET actions to NULL? */
> 
> Yes, we already do this so it's OK as long as the length is also set to 0.

I don't immediately see how this is safe, as we seem to happily dereference the sf_acts pointer without ever checking if it is NULL, like in "acts->actions_len".

It seems to me that we should allocate a new sf_flow_actions in any case for SET, and make the actions_len zero if no actions are passed in by the userspace.

  Jarno


More information about the dev mailing list