On Mar 24, 2014, at 12:24 PM, Pravin Shelar <[email protected]> wrote:
> On Mon, Mar 24, 2014 at 11:56 AM, Jarno Rajahalme <[email protected]> > wrote: >> Flow SET can accept an empty set of actions, with the intended >> semantics of leaving existing actions unmodified. This seems to have >> been brokin after OVS 1.7, as we have assigned the flow's actions >> pointer to NULL in this case, but we never check for the NULL pointer >> later on. This patch restores the intended behavior and documents it >> in the include/linux/openvswitch.h. >> >> Signed-off-by: Jarno Rajahalme <[email protected]> >> >> v4: Maintain OVS 1.7 semantics: Allow missing actions on OVS_FLOW_CMD_SET, >> which cause the existing actions to be retained. > This should not be part of commit msg. > So I left that on the wrong side of ‘—‘ line, sorry. >> --- >> datapath/datapath.c | 19 ++++++++++--------- >> include/linux/openvswitch.h | 4 +++- >> 2 files changed, 13 insertions(+), 10 deletions(-) >> >> diff --git a/datapath/datapath.c b/datapath/datapath.c >> index 1efbb00..918a990 100644 >> --- a/datapath/datapath.c >> +++ b/datapath/datapath.c >> @@ -811,6 +811,7 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, >> struct genl_info *info) >> goto err_kfree; >> } >> } else if (info->genlhdr->cmd == OVS_FLOW_CMD_NEW) { >> + /* OVS_FLOW_CMD_NEW must have actions. */ >> error = -EINVAL; >> goto error; >> } >> @@ -839,19 +840,16 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff >> *skb, struct genl_info *info) >> flow->key = masked_key; >> flow->unmasked_key = key; >> rcu_assign_pointer(flow->sf_acts, acts); >> + acts = NULL; >> >> /* Put flow in bucket. */ >> error = ovs_flow_tbl_insert(&dp->table, flow, &mask); >> - if (error) { >> - acts = NULL; >> + if (error) >> goto err_flow_free; >> - } > Why are you changing this? I found it easier to think and reason about freeing ‘acts’ by changing it to NULL as soon as the actions are assigned to the flow. I can easily revert this though. Jarno
_______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
