On Mar 25, 2014, at 10:47 AM, Pravin Shelar <[email protected]> wrote:
> On Mon, Mar 24, 2014 at 11:56 AM, Jarno Rajahalme <[email protected]> > wrote: >> ... >> @@ -871,29 +876,32 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, >> struct genl_info *info) >> /* Update actions. */ >> old_acts = ovsl_dereference(flow->sf_acts); >> rcu_assign_pointer(flow->sf_acts, acts); >> - ovs_nla_free_flow_actions(old_acts); >> } >> >> - if (ovs_must_build_reply(info, &ovs_dp_flow_multicast_group)) >> - reply = ovs_flow_cmd_build_info(flow, ovs_header->dp_ifindex, >> - info, OVS_FLOW_CMD_NEW); >> - 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)); >> + 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) >> + ovs_notify(reply, info, &ovs_dp_flow_multicast_group); >> + if (new_flow) >> + ovs_flow_free(new_flow, false); > Most of time new_flow should be NULL, so we can move this check to error code. > >> + if (old_acts) >> + ovs_nla_free_flow_actions(old_acts); > If we move old_acts to its respective case we can avoid checking it in > common case. I addressed both of these by moving code after the two cases to the individual cases. This resulted in a nice cleanup, apart from the two identical calls to ovs_flow_cmd_fill_info(), but usually those are not called anyway, as the userspace is not expecting a response from OVS_FLOW_CMD_NEW. Jarno
_______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
