On Tue, Oct 15, 2013 at 05:17:48PM +0900, Simon Horman wrote:
> This is a first step towards implementing the dpif side of groups.
>
> In order to be useful the action translation code needs
> to be taught about groups.
>
> Signed-off-by: Simon Horman <ho...@verge.net.au>
The code in group_get_stats() worries me. First, what's this? I
don't see why one would expect to have "struct bucket_counter"s
following the 'ogs' argument:
struct bucket_counter *bc = (struct bucket_counter *)(ogs + 1);
Second, if the following comment in group_get_stats() is correct, then
I don't see how push_all_stats__() helps. If the group that
group_get_stats() operates on can disappear then the right solution is
to take some lock or increment some refcount in the caller:
/* push_all_stats() can handle flow misses which, when using the learn
* action, can cause groups to be added and deleted. This can corrupt our
* caller's datastructures which assume that group_get_stats() doesn't have
* an impact on the flow table. To be safe, we disable miss handling. */
push_all_stats__(false);
Here are some tiny style fixes you can fold in:
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index b0c358a..470e455 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4827,7 +4827,7 @@ group_dealloc(struct ofgroup *group_)
static void
group_construct_stats(struct group_dpif *group)
- OVS_REQUIRES(&group->stats_mutex)
+ OVS_REQUIRES(group->stats_mutex)
{
group->packet_count = 0;
group->byte_count = 0;
@@ -4853,7 +4853,7 @@ group_construct(struct ofgroup *group_)
static void
group_destruct__(struct group_dpif *group)
- OVS_REQUIRES(&group->stats_mutex)
+ OVS_REQUIRES(group->stats_mutex)
{
free(group->bucket_stats);
group->bucket_stats = NULL;
@@ -4885,7 +4885,6 @@ group_modify(struct ofgroup *group_, struct ofgroup
*victim_)
return 0;
}
-
static enum ofperr
group_get_stats(const struct ofgroup *group_, struct ofputil_group_stats *ogs)
{
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
