Thanks for the reviews!
All six pushed to master,
Jarno
On Apr 1, 2014, at 3:20 PM, Pravin Shelar <pshe...@nicira.com> wrote:
> On Mon, Mar 31, 2014 at 11:18 AM, Jarno Rajahalme <jrajaha...@nicira.com>
> wrote:
>> Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com>
...
>> @@ -871,40 +885,45 @@ static int ovs_flow_cmd_new(struct sk_buff *skb,
>> struct genl_info *info)
>> * request. We also accept NLM_F_EXCL in case that bug ever
>> * gets fixed.
>> */
>> - error = -EEXIST;
>> - if (info->nlhdr->nlmsg_flags & (NLM_F_CREATE | NLM_F_EXCL))
>> + if (unlikely(info->nlhdr->nlmsg_flags & (NLM_F_CREATE
>> + | NLM_F_EXCL))) {
>> + error = -EEXIST;
>> goto err_unlock_ovs;
>> -
>> + }
>> /* The unmasked key has to be the same for flow updates. */
>> - if (!ovs_flow_cmp_unmasked_key(flow, &match))
>> + if (unlikely(!ovs_flow_cmp_unmasked_key(flow, &match))) {
>> + error = -EEXIST;
>> goto err_unlock_ovs;
>> -
>> + }
>> /* Update actions. */
>> old_acts = ovsl_dereference(flow->sf_acts);
>> rcu_assign_pointer(flow->sf_acts, acts);
>> - ovs_nla_free_flow_actions(old_acts);
>> - }
>>
>> - reply = ovs_flow_cmd_build_info(flow, ovs_header->dp_ifindex,
>> - info, OVS_FLOW_CMD_NEW, false);
>> - ovs_unlock();
>> + if (unlikely(reply)) {
>> + error = ovs_flow_cmd_fill_info(flow,
>> +
>> ovs_header->dp_ifindex,
>> + reply,
>> info->snd_portid,
>> + info->snd_seq, 0,
>> + OVS_FLOW_CMD_NEW);
>> + BUG_ON(error < 0);
>> + }
>> + ovs_unlock();
>>
>> - if (reply) {
>> - if (!IS_ERR(reply))
>> - ovs_notify(reply, info,
>> &ovs_dp_flow_multicast_group);
>> - else
>> - netlink_set_err(sock_net(skb->sk)->genl_sock, 0,
>> - ovs_dp_flow_multicast_group.id,
>> - PTR_ERR(reply));
>> + ovs_nla_free_flow_actions(old_acts);
>> + ovs_flow_free(new_flow, false);
>> }
> How much do we save by unlocking ovs-lock before free, I think this is
> rare case. If so then we can move ovs-unlock out of if and else block.
> It is just easy to read.
NEW on an existing flow can happen if a second miss for the same flow appears
before the new flow is set up. I do not know how often that happens, though.
Jarno_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
