Note that this has only a little performance benefit when responses are not created (which is normal when there are no netlink multicast listeners around).
Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com> --- datapath/datapath.c | 123 +++++++++++++++++++++++++++++---------------------- 1 file changed, 71 insertions(+), 52 deletions(-) diff --git a/datapath/datapath.c b/datapath/datapath.c index 3d4037d..ba00824 100644 --- a/datapath/datapath.c +++ b/datapath/datapath.c @@ -754,15 +754,11 @@ error: return err; } -/* Must be called with ovs_mutex. */ -static struct sk_buff *ovs_flow_cmd_alloc_info(struct sw_flow *flow, +static struct sk_buff *ovs_flow_cmd_alloc_info(const struct sw_flow_actions *acts, struct genl_info *info) { - size_t len; - - len = ovs_flow_cmd_msg_size(ovsl_dereference(flow->sf_acts)); - - return genlmsg_new_unicast(len, info, GFP_KERNEL); + return genlmsg_new_unicast(ovs_flow_cmd_msg_size(acts), info, + GFP_KERNEL); } /* Must be called with ovs_mutex. */ @@ -773,7 +769,7 @@ static struct sk_buff *ovs_flow_cmd_build_info(struct sw_flow *flow, struct sk_buff *skb; int retval; - skb = ovs_flow_cmd_alloc_info(flow, info); + skb = ovs_flow_cmd_alloc_info(ovsl_dereference(flow->sf_acts), info); if (!skb) return ERR_PTR(-ENOMEM); @@ -789,11 +785,11 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, struct genl_info *info) struct nlattr **a = info->attrs; struct ovs_header *ovs_header = info->userhdr; struct sw_flow_key key, masked_key; - struct sw_flow *flow; + struct sw_flow *flow, *new_flow = NULL; struct sw_flow_mask mask; struct sk_buff *reply = NULL; struct datapath *dp; - struct sw_flow_actions *acts; + struct sw_flow_actions *acts, *old_acts = NULL; struct sw_flow_match match; int error; @@ -823,9 +819,28 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, struct genl_info *info) &masked_key, 0, &acts); if (error) { OVS_NLERR("Flow actions may not be safe on all matching packets.\n"); - goto err_kfree; + goto err_kfree_acts; + } + + if (ovs_must_build_reply(info, &ovs_dp_flow_multicast_group)) { + reply = ovs_flow_cmd_alloc_info(acts, info); + if (!reply) { + error = -ENOMEM; + goto err_kfree_acts; + } } + /* Most of the time we need to allocate a new flow, do it before + * locking. + */ + new_flow = ovs_flow_alloc(); + if (IS_ERR(new_flow)) { + error = PTR_ERR(new_flow); + goto err_kfree_reply; + } + new_flow->key = masked_key; + new_flow->unmasked_key = key; + ovs_lock(); dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex); error = -ENODEV; @@ -835,25 +850,17 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, struct genl_info *info) /* Check if this is a duplicate flow */ flow = ovs_flow_tbl_lookup(&dp->table, &key); if (!flow) { - /* Allocate flow. */ - flow = ovs_flow_alloc(); - if (IS_ERR(flow)) { - error = PTR_ERR(flow); - goto err_unlock_ovs; - } - - flow->key = masked_key; - flow->unmasked_key = key; + flow = new_flow; rcu_assign_pointer(flow->sf_acts, acts); acts = NULL; /* Put flow in bucket. */ error = ovs_flow_tbl_insert(&dp->table, flow, &mask); if (error) - goto err_flow_free; + goto err_unlock_ovs; + new_flow = NULL; } else { /* We found a matching flow. */ - struct sw_flow_actions *old_acts; /* Bail out if we're not allowed to modify an existing flow. * We accept NLM_F_CREATE in place of the intended NLM_F_EXCL @@ -872,32 +879,36 @@ 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); + if (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); + } + /* Clear stats. */ if (a[OVS_FLOW_ATTR_CLEAR]) ovs_flow_stats_clear(flow); 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)); - } + if (reply) + ovs_notify(reply, info, &ovs_dp_flow_multicast_group); + if (new_flow) + ovs_flow_free(new_flow, false); + if (old_acts) + ovs_nla_free_flow_actions(old_acts); return 0; -err_flow_free: - ovs_flow_free(flow, false); err_unlock_ovs: ovs_unlock(); -err_kfree: + if (new_flow) + ovs_flow_free(new_flow, false); +err_kfree_reply: + kfree_skb(reply); +err_kfree_acts: kfree(acts); error: return error; @@ -939,7 +950,7 @@ static int ovs_flow_cmd_set(struct sk_buff *skb, struct genl_info *info) &masked_key, 0, &acts); if (error) { OVS_NLERR("Flow actions may not be safe on all matching packets.\n"); - goto err_kfree; + goto err_kfree_acts; } } else { /* Need empty actions. */ @@ -949,13 +960,21 @@ static int ovs_flow_cmd_set(struct sk_buff *skb, struct genl_info *info) goto error; } + if (ovs_must_build_reply(info, &ovs_dp_flow_multicast_group)) { + reply = ovs_flow_cmd_alloc_info(acts, info); + if (!reply) { + error = -ENOMEM; + goto err_kfree_acts; + } + } + ovs_lock(); dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex); error = -ENODEV; if (!dp) goto err_unlock_ovs; - /* Check if this is a duplicate flow */ + /* Check if the flow exists. */ flow = ovs_flow_tbl_lookup(&dp->table, &key); error = -ENOENT; if (!flow) @@ -968,29 +987,29 @@ static int ovs_flow_cmd_set(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); + if (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); + } + /* Clear stats. */ if (a[OVS_FLOW_ATTR_CLEAR]) ovs_flow_stats_clear(flow); 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)); - } + if (reply) + ovs_notify(reply, info, &ovs_dp_flow_multicast_group); + ovs_nla_free_flow_actions(old_acts); return 0; err_unlock_ovs: ovs_unlock(); -err_kfree: + kfree_skb(reply); +err_kfree_acts: kfree(acts); error: return error; -- 1.7.10.4 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev