[ovs-dev] [PATCH v5 1/4] classifier: Simplify versioning.
Ben Pfaff
blp at nicira.com
Fri Jun 12 22:21:45 UTC 2015
On Fri, Jun 12, 2015 at 02:36:21PM -0700, Jarno Rajahalme wrote:
> After all, there are some cases in which both the insertion version
> and removal version of a rule need to be considered. This makes the
> cls_match a bit bigger, but makes classifier versioning much simpler
> to understand.
>
> Also, avoid using type larger than int in an enum, as it is not
> portable C.
>
> Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
You could use this to avoid having to change CLS_MAX_VERSION and
CLS_NOT_REMOVED_VERSION if you change cls_version_t:
diff --git a/lib/classifier.h b/lib/classifier.h
index 9aec8b2..a25764e 100644
--- a/lib/classifier.h
+++ b/lib/classifier.h
@@ -308,6 +308,7 @@
#include "meta-flow.h"
#include "pvector.h"
#include "rculist.h"
+#include "type-props.h"
#ifdef __cplusplus
extern "C" {
@@ -330,8 +331,8 @@ typedef uint64_t cls_version_t;
#define CLS_NO_VERSION 0
#define CLS_MIN_VERSION 1 /* Default version number to use. */
-#define CLS_MAX_VERSION (UINT64_MAX - 1)
-#define CLS_NOT_REMOVED_VERSION UINT64_MAX
+#define CLS_MAX_VERSION (TYPE_MAXIMUM(cls_version_t) - 1)
+#define CLS_NOT_REMOVED_VERSION TYPE_MAXIMUM(cls_version_t)
enum {
CLS_MAX_INDICES = 3, /* Maximum number of lookup indices per subtable. */
cls_match has an *almost*-invariant that add_version <= remove_version.
If we did the following, it would be a full invariant, I believe, and we
could remove CLS_NO_VERSION. Using add_version == remove_version
implies to my mind that the rule was added and removed at the same time,
which seems a lot like it was never there. Is it correct codewise too
(it passes the unit tests at least)?
diff --git a/lib/classifier.c b/lib/classifier.c
index 6487426..d76b3fb 100644
--- a/lib/classifier.c
+++ b/lib/classifier.c
@@ -100,7 +100,7 @@ cls_match_alloc(const struct cls_rule *rule,
*CONST_CAST(const struct cls_rule **, &cls_match->cls_rule) = rule;
*CONST_CAST(int *, &cls_match->priority) = rule->priority;
*CONST_CAST(cls_version_t *, &cls_match->add_version) = rule->version;
- atomic_init(&cls_match->remove_version, CLS_NO_VERSION); /* Initially
+ atomic_init(&cls_match->remove_version, rule->version); /* Initially
invisible. */
miniflow_clone_inline(CONST_CAST(struct miniflow *, &cls_match->flow),
&rule->match.flow, count);
The 'version' member of cls_rule seems to have only marginal utility.
Maybe we can remove it. No need to do that now though.
Acked-by: Ben Pfaff <blp at nicira.com>
More information about the dev
mailing list