Not returning the actions from flow delete allows the reply (if any) be
allocated before taking the lock, and failing out properly if no memory
is available.
It seems that userspace has no plausable use for the actions in a flow
delete reply, so this should be OK.
Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com>
---
datapath/datapath.c | 64 ++++++++++++++++++++++++++++-----------------------
1 file changed, 35 insertions(+), 29 deletions(-)
diff --git a/datapath/datapath.c b/datapath/datapath.c
index 9e86ec8..da07f9d 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -664,7 +664,7 @@ static size_t ovs_flow_cmd_msg_size(const struct
sw_flow_actions *acts)
+ nla_total_size(sizeof(struct ovs_flow_stats)) /*
OVS_FLOW_ATTR_STATS */
+ nla_total_size(1) /* OVS_FLOW_ATTR_TCP_FLAGS */
+ nla_total_size(8) /* OVS_FLOW_ATTR_USED */
- + nla_total_size(acts->actions_len); /* OVS_FLOW_ATTR_ACTIONS */
+ + (acts ? nla_total_size(acts->actions_len) : 0); /*
OVS_FLOW_ATTR_ACTIONS */
}
/* Called with ovs_mutex or RCU read lock. */
@@ -730,25 +730,26 @@ static int ovs_flow_cmd_fill_info(struct sw_flow *flow,
int dp_ifindex,
* This can only fail for dump operations because the skb is always
* properly sized for single flows.
*/
- start = nla_nest_start(skb, OVS_FLOW_ATTR_ACTIONS);
- if (start) {
- const struct sw_flow_actions *sf_acts;
-
- sf_acts = rcu_dereference_ovsl(flow->sf_acts);
-
- err = ovs_nla_put_actions(sf_acts->actions,
- sf_acts->actions_len, skb);
- if (!err)
- nla_nest_end(skb, start);
- else {
- if (skb_orig_len)
- goto error;
-
- nla_nest_cancel(skb, start);
- }
- } else if (skb_orig_len)
- goto nla_put_failure;
-
+ if (cmd != OVS_FLOW_CMD_DEL) {
+ start = nla_nest_start(skb, OVS_FLOW_ATTR_ACTIONS);
+ if (start) {
+ const struct sw_flow_actions *sf_acts;
+
+ sf_acts = rcu_dereference_ovsl(flow->sf_acts);
+
+ err = ovs_nla_put_actions(sf_acts->actions,
+ sf_acts->actions_len, skb);
+ if (!err)
+ nla_nest_end(skb, start);
+ else {
+ if (skb_orig_len)
+ goto error;
+
+ nla_nest_cancel(skb, start);
+ }
+ } else if (skb_orig_len)
+ goto nla_put_failure;
+ }
return genlmsg_end(skb, ovs_header);
nla_put_failure:
@@ -992,6 +993,12 @@ static int ovs_flow_cmd_del(struct sk_buff *skb, struct
genl_info *info)
return err;
}
+ if (ovs_build_reply(info, &ovs_dp_flow_multicast_group)) {
+ reply = ovs_flow_cmd_alloc_info(NULL, info);
+ if (!reply)
+ return -ENOMEM;
+ }
+
ovs_lock();
dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex);
if (!dp) {
@@ -1011,20 +1018,19 @@ static int ovs_flow_cmd_del(struct sk_buff *skb, struct
genl_info *info)
ovs_flow_tbl_remove(&dp->table, flow);
ovs_unlock();
- if (ovs_build_reply(info, &ovs_dp_flow_multicast_group)) {
- /* XXX: We may skip sending a response if kernel is out of
- memory. We could do the allocation before locking if we did
- not need to return the actions (which are useless in this
- case anyway). */
- reply = ovs_flow_cmd_build_info(flow, ovs_header->dp_ifindex,
- info, OVS_FLOW_CMD_DEL);
- if (!IS_ERR(reply))
- ovs_notify(reply, info, &ovs_dp_flow_multicast_group);
+ if (reply) {
+ err = ovs_flow_cmd_fill_info(flow, ovs_header->dp_ifindex,
+ reply, info->snd_portid,
+ info->snd_seq, 0,
+ OVS_FLOW_CMD_DEL);
+ BUG_ON(err < 0);
+ ovs_notify(reply, info, &ovs_dp_flow_multicast_group);
}
ovs_flow_free(flow, true);
return 0;
ovs_unlock:
+ kfree_skb(reply);
ovs_unlock();
return err;
}
--
1.7.10.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
