On 28 October 2015 at 20:07, Jarno Rajahalme <jrajaha...@nicira.com> wrote:
> Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com>
Looks ok to me. I have some minor style/doc suggestions at the end of the mail.
In revalidate(), the way that 'recircs' is only initialised in some
paths, then in an overlapping set of paths passed to reval_op_init(),
seems prone to cause confusion or errors if the logic is further
modified. I think it's correct, but I wonder if there's a better way
we could compose the logic to make it more obvious (It's partly due to
the new function reval_op_init()).
Otherwise, up to you whether you'd like another pair of eyes on this code.
Acked-by: Joe Stringer <joestrin...@nicira.com>
diff --git a/ofproto/ofproto-dpif-rid.h b/ofproto/ofproto-dpif-rid.h
index b5cbc739fa02..135810d0b027 100644
--- a/ofproto/ofproto-dpif-rid.h
+++ b/ofproto/ofproto-dpif-rid.h
@@ -21,7 +21,6 @@
#include <stdint.h>
#include "cmap.h"
-#include "flow.h"
#include "list.h"
#include "ofp-actions.h"
#include "ofproto-dpif-mirror.h"
@@ -206,12 +205,13 @@ struct recirc_refs {
};
};
-#define RECIRC_REFS_EMPTY_INITIALIZER { 0, { { 0, 0 } } }
+#define RECIRC_REFS_EMPTY_INITIALIZER ((struct recirc_refs) \
+ { 0, { { 0, 0 } } })
/* Helpers to abstract the recirculation union away. */
static inline void
recirc_refs_init(struct recirc_refs *rr)
{
- memset(rr, 0, sizeof *rr);
+ *rr = RECIRC_REFS_EMPTY_INITIALIZER;
}
static inline void
@@ -1783,7 +1784,13 @@ should_revalidate(const struct udpif *udpif,
uint64_t packets,
* fine. Callers should change the actions to those found
* in the caller supplied 'odp_actions' buffer. The
* recirculation references can be found in 'recircs' and
- * must be handled by the caller. */
+ * must be handled by the caller.
+ *
+ * If the result is UKEY_MODIFY, then references to all recirc_ids used by the
+ * new flow will be held within 'recircs' (which may be none). The caller
+ * is responsible for ensuring that 'recircs' references are eventually freed.
+ * In all other cases, 'recircs' is initialised to have no references.
+ */
static enum reval_result
revalidate_ukey(struct udpif *udpif, struct udpif_key *ukey,
const struct dpif_flow_stats *stats,
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
