On Mon, Mar 24, 2014 at 1:24 PM, Jarno Rajahalme <jrajaha...@nicira.com> wrote:
>
> On Mar 24, 2014, at 12:24 PM, Pravin Shelar <pshe...@nicira.com> wrote:
>
> On Mon, Mar 24, 2014 at 11:56 AM, Jarno Rajahalme <jrajaha...@nicira.com>
> 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 <jrajaha...@nicira.com>
>
> 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 <pshe...@nicira.com>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
