Userspace already sets NLM_F_ECHO when it wants a reply, make use of
it.
For set and del, OVS userspace only sets the NLM_F_ECHO if stats are
needed back. So, we minimize the data sent back by only responding
with key and stats for ovs_flow_cmd_new, ovs_flow_cmd_set, and
ovs_flow_cmd_del.
When not returning actions, we can allocate the reply (if requested)
before finding the flow, i.e., before locking anything. We make use
of this in following patches.
Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com>
---
datapath/datapath.c | 177 +++++++++++++++++++++++++++++----------------------
1 file changed, 102 insertions(+), 75 deletions(-)
diff --git a/datapath/datapath.c b/datapath/datapath.c
index f045fe4..9fdb447 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -639,21 +639,26 @@ static struct genl_multicast_group
ovs_dp_flow_multicast_group = {
.name = OVS_FLOW_MCGROUP
};
-static size_t ovs_flow_cmd_msg_size(const struct sw_flow_actions *acts)
+static size_t ovs_flow_cmd_msg_size(const struct sw_flow *flow, u64 what)
{
return NLMSG_ALIGN(sizeof(struct ovs_header))
+ nla_total_size(key_attr_size()) /* OVS_FLOW_ATTR_KEY */
- + nla_total_size(key_attr_size()) /* OVS_FLOW_ATTR_MASK */
- + 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 */
+ + (what & 1 << OVS_FLOW_ATTR_MASK
+ ? nla_total_size(key_attr_size()) : 0)
+ + (what & 1 << OVS_FLOW_ATTR_STATS
+ ? nla_total_size(sizeof(struct ovs_flow_stats))
+ + nla_total_size(1) /* OVS_FLOW_ATTR_TCP_FLAGS */
+ + nla_total_size(8) /* OVS_FLOW_ATTR_USED */
+ : 0)
+ + (what & 1 << OVS_FLOW_ATTR_ACTIONS
+ ?
nla_total_size(ovsl_dereference(flow->sf_acts)->actions_len)
+ : 0);
}
/* Called with ovs_mutex. */
static int ovs_flow_cmd_fill_info(struct sw_flow *flow, struct datapath *dp,
struct sk_buff *skb, u32 portid,
- u32 seq, u32 flags, u8 cmd)
+ u32 seq, u32 flags, u8 cmd, u64 what)
{
const int skb_orig_len = skb->len;
struct nlattr *start;
@@ -670,7 +675,7 @@ static int ovs_flow_cmd_fill_info(struct sw_flow *flow,
struct datapath *dp,
ovs_header->dp_ifindex = get_dpifindex(dp);
- /* Fill flow key. */
+ /* Fill flow key. Not optional. */
nla = nla_nest_start(skb, OVS_FLOW_ATTR_KEY);
if (!nla)
goto nla_put_failure;
@@ -680,28 +685,32 @@ static int ovs_flow_cmd_fill_info(struct sw_flow *flow,
struct datapath *dp,
goto error;
nla_nest_end(skb, nla);
- nla = nla_nest_start(skb, OVS_FLOW_ATTR_MASK);
- if (!nla)
- goto nla_put_failure;
+ if (what & 1 << OVS_FLOW_ATTR_MASK) {
+ nla = nla_nest_start(skb, OVS_FLOW_ATTR_MASK);
+ if (!nla)
+ goto nla_put_failure;
- err = ovs_nla_put_flow(&flow->key, &flow->mask->key, skb);
- if (err)
- goto error;
+ err = ovs_nla_put_flow(&flow->key, &flow->mask->key, skb);
+ if (err)
+ goto error;
- nla_nest_end(skb, nla);
+ nla_nest_end(skb, nla);
+ }
- ovs_flow_stats_get(flow, &stats, &used, &tcp_flags);
- if (used &&
- nla_put_u64(skb, OVS_FLOW_ATTR_USED, ovs_flow_used_time(used)))
- goto nla_put_failure;
+ if (what & 1 << OVS_FLOW_ATTR_STATS) {
+ ovs_flow_stats_get(flow, &stats, &used, &tcp_flags);
+ if (used &&
+ nla_put_u64(skb, OVS_FLOW_ATTR_USED,
ovs_flow_used_time(used)))
+ goto nla_put_failure;
- if (stats.n_packets &&
- nla_put(skb, OVS_FLOW_ATTR_STATS, sizeof(struct ovs_flow_stats),
&stats))
- goto nla_put_failure;
+ if (stats.n_packets &&
+ nla_put(skb, OVS_FLOW_ATTR_STATS, sizeof(struct
ovs_flow_stats), &stats))
+ goto nla_put_failure;
- if ((u8)ntohs(tcp_flags) &&
- nla_put_u8(skb, OVS_FLOW_ATTR_TCP_FLAGS, (u8)ntohs(tcp_flags)))
- goto nla_put_failure;
+ if ((u8)ntohs(tcp_flags) &&
+ nla_put_u8(skb, OVS_FLOW_ATTR_TCP_FLAGS,
(u8)ntohs(tcp_flags)))
+ goto nla_put_failure;
+ }
/* If OVS_FLOW_ATTR_ACTIONS doesn't fit, skip dumping the actions if
* this is the first flow to be dumped into 'skb'. This is unusual for
@@ -713,25 +722,26 @@ static int ovs_flow_cmd_fill_info(struct sw_flow *flow,
struct datapath *dp,
* 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 (what & 1 << OVS_FLOW_ATTR_ACTIONS) {
+ 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:
@@ -741,30 +751,29 @@ error:
return err;
}
+/* 'flow' will be dereferenced only if '(what & 1 << OVS_FLOW_ATTR_ACTIONS)'.
*/
static struct sk_buff *ovs_flow_cmd_alloc_info(struct sw_flow *flow,
- struct genl_info *info)
+ struct genl_info *info,
+ u64 what)
{
- 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(flow, what),
+ info, GFP_KERNEL);
}
static struct sk_buff *ovs_flow_cmd_build_info(struct sw_flow *flow,
struct datapath *dp,
struct genl_info *info,
- u8 cmd)
+ u8 cmd, u64 what)
{
struct sk_buff *skb;
int retval;
- skb = ovs_flow_cmd_alloc_info(flow, info);
+ skb = ovs_flow_cmd_alloc_info(flow, info, what);
if (!skb)
return ERR_PTR(-ENOMEM);
retval = ovs_flow_cmd_fill_info(flow, dp, skb, info->snd_portid,
- info->snd_seq, 0, cmd);
+ info->snd_seq, 0, cmd, what);
BUG_ON(retval < 0);
return skb;
}
@@ -776,7 +785,7 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb,
struct genl_info *info)
struct sw_flow_key key, masked_key;
struct sw_flow *flow = NULL;
struct sw_flow_mask mask;
- struct sk_buff *reply;
+ struct sk_buff *reply = NULL;
struct datapath *dp;
struct sw_flow_actions *acts = NULL;
struct sw_flow_match match;
@@ -845,7 +854,10 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb,
struct genl_info *info)
goto err_flow_free;
}
- reply = ovs_flow_cmd_build_info(flow, dp, info,
OVS_FLOW_CMD_NEW);
+ if (info->nlhdr->nlmsg_flags & NLM_F_ECHO)
+ reply = ovs_flow_cmd_build_info(flow, dp, info,
+ OVS_FLOW_CMD_NEW,
+ 1 <<
OVS_FLOW_ATTR_STATS);
} else {
/* We found a matching flow. */
struct sw_flow_actions *old_acts;
@@ -873,19 +885,23 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb,
struct genl_info *info)
rcu_assign_pointer(flow->sf_acts, acts);
ovs_nla_free_flow_actions(old_acts);
- reply = ovs_flow_cmd_build_info(flow, dp, info,
OVS_FLOW_CMD_NEW);
-
+ if (info->nlhdr->nlmsg_flags & NLM_F_ECHO)
+ reply = ovs_flow_cmd_build_info(flow, dp, info,
+ OVS_FLOW_CMD_NEW,
+ 1 <<
OVS_FLOW_ATTR_STATS);
/* Clear stats. */
if (a[OVS_FLOW_ATTR_CLEAR])
ovs_flow_stats_clear(flow);
}
ovs_unlock();
- 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) {
+ 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));
+ }
return 0;
err_flow_free:
@@ -932,7 +948,10 @@ static int ovs_flow_cmd_get(struct sk_buff *skb, struct
genl_info *info)
goto unlock;
}
- reply = ovs_flow_cmd_build_info(flow, dp, info, OVS_FLOW_CMD_NEW);
+ reply = ovs_flow_cmd_build_info(flow, dp, info, OVS_FLOW_CMD_NEW,
+ 1 << OVS_FLOW_ATTR_MASK |
+ 1 << OVS_FLOW_ATTR_STATS |
+ 1 << OVS_FLOW_ATTR_ACTIONS);
if (IS_ERR(reply)) {
err = PTR_ERR(reply);
goto unlock;
@@ -950,7 +969,7 @@ static int ovs_flow_cmd_del(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;
- struct sk_buff *reply;
+ struct sk_buff *reply = NULL;
struct sw_flow *flow;
struct datapath *dp;
struct sw_flow_match match;
@@ -979,22 +998,27 @@ static int ovs_flow_cmd_del(struct sk_buff *skb, struct
genl_info *info)
goto unlock;
}
- reply = ovs_flow_cmd_alloc_info(flow, info);
- if (!reply) {
- err = -ENOMEM;
- goto unlock;
+ if (info->nlhdr->nlmsg_flags & NLM_F_ECHO) {
+ reply = ovs_flow_cmd_alloc_info(flow, info,
+ 1 << OVS_FLOW_ATTR_STATS);
+ if (!reply) {
+ err = -ENOMEM;
+ goto unlock;
+ }
}
-
ovs_flow_tbl_remove(&dp->table, flow);
- err = ovs_flow_cmd_fill_info(flow, dp, reply, info->snd_portid,
- info->snd_seq, 0, OVS_FLOW_CMD_DEL);
- BUG_ON(err < 0);
-
+ if (reply) {
+ err = ovs_flow_cmd_fill_info(flow, dp, reply, info->snd_portid,
+ info->snd_seq, 0, OVS_FLOW_CMD_DEL,
+ 1 << OVS_FLOW_ATTR_STATS);
+ BUG_ON(err < 0);
+ }
ovs_flow_free(flow, true);
ovs_unlock();
- ovs_notify(reply, info, &ovs_dp_flow_multicast_group);
+ if (reply)
+ ovs_notify(reply, info, &ovs_dp_flow_multicast_group);
return 0;
unlock:
ovs_unlock();
@@ -1028,7 +1052,10 @@ static int ovs_flow_cmd_dump(struct sk_buff *skb, struct
netlink_callback *cb)
if (ovs_flow_cmd_fill_info(flow, dp, skb,
NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq, NLM_F_MULTI,
- OVS_FLOW_CMD_NEW) < 0)
+ OVS_FLOW_CMD_NEW,
+ 1 << OVS_FLOW_ATTR_MASK |
+ 1 << OVS_FLOW_ATTR_STATS |
+ 1 << OVS_FLOW_ATTR_ACTIONS) < 0)
break;
cb->args[0] = bucket;
--
1.7.10.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
