Pushed to master, Jarno
On Mar 24, 2014, at 1:41 PM, Pravin Shelar <[email protected]> wrote: > On Mon, Mar 24, 2014 at 1:24 PM, Jarno Rajahalme <[email protected]> > wrote: >> >> 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. >> > > Lets drop it. This not related to this patch anyways. > > For the patch: > Acked-by: Pravin B Shelar <[email protected]>
_______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
