[ovs-dev] [PATCH 1/3] ofproto-dpif: Hide struct rule_dpif internally.
Ethan Jackson
ethan at nicira.com
Wed Sep 4 19:37:34 UTC 2013
By hiding struct rule_dpif inside ofproto-dpif, it becomes very clear
which attributes are accessed by multiple threads and need to be
protected by locks.
Signed-off-by: Ethan Jackson <ethan at nicira.com>
---
ofproto/ofproto-dpif-upcall.c | 2 +-
ofproto/ofproto-dpif-xlate.c | 23 +++++++-------
ofproto/ofproto-dpif-xlate.h | 2 ++
ofproto/ofproto-dpif.c | 69 ++++++++++++++++++++++++++++++++++++-----
ofproto/ofproto-dpif.h | 57 ++++++++++++++--------------------
5 files changed, 100 insertions(+), 53 deletions(-)
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 22cdf1b..70bb2fe 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -661,7 +661,7 @@ execute_flow_miss(struct flow_miss *miss, struct dpif_op *ops, size_t *n_ops)
xlate_actions(&xin, &miss->xout);
flow_wildcards_or(&miss->xout.wc, &miss->xout.wc, &wc);
- if (rule->up.cr.priority == FAIL_OPEN_PRIORITY) {
+ if (rule_fail_open(rule)) {
LIST_FOR_EACH (packet, list_node, &miss->packets) {
struct ofputil_packet_in *pin;
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index ef254d9..ff1a259 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -1668,9 +1668,11 @@ compose_output_action(struct xlate_ctx *ctx, ofp_port_t ofp_port)
static void
xlate_recursively(struct xlate_ctx *ctx, struct rule_dpif *rule)
- OVS_RELEASES(rule->up.evict)
+ OVS_RELEASES(rule)
{
struct rule_dpif *old_rule = ctx->rule;
+ const struct ofpact *ofpacts;
+ size_t ofpacts_len;
if (ctx->xin->resubmit_stats) {
rule_credit_stats(rule, ctx->xin->resubmit_stats);
@@ -1678,7 +1680,8 @@ xlate_recursively(struct xlate_ctx *ctx, struct rule_dpif *rule)
ctx->recurse++;
ctx->rule = rule;
- do_xlate_actions(rule->up.ofpacts, rule->up.ofpacts_len, ctx);
+ rule_get_actions(rule, &ofpacts, &ofpacts_len);
+ do_xlate_actions(ofpacts, ofpacts_len, ctx);
ctx->rule = old_rule;
ctx->recurse--;
@@ -1722,10 +1725,9 @@ xlate_table_action(struct xlate_ctx *ctx,
* OFPTC_TABLE_MISS_DROP
* When OF1.0, OFPTC_TABLE_MISS_CONTINUE is used. What to do? */
xport = get_ofp_port(ctx->xbridge, ctx->xin->flow.in_port.ofp_port);
- rule = choose_miss_rule(xport ? xport->config : 0,
- ctx->xbridge->miss_rule,
- ctx->xbridge->no_packet_in_rule);
- ovs_rwlock_rdlock(&rule->up.evict);
+ choose_miss_rule(xport ? xport->config : 0,
+ ctx->xbridge->miss_rule,
+ ctx->xbridge->no_packet_in_rule, &rule);
xlate_recursively(ctx, rule);
}
@@ -1811,7 +1813,7 @@ execute_controller_action(struct xlate_ctx *ctx, int len,
pin->reason = reason;
pin->controller_id = controller_id;
pin->table_id = ctx->table_id;
- pin->cookie = ctx->rule ? ctx->rule->up.flow_cookie : 0;
+ pin->cookie = ctx->rule ? rule_get_flow_cookie(ctx->rule) : 0;
pin->send_len = len;
flow_get_metadata(&ctx->xin->flow, &pin->fmd);
@@ -2119,8 +2121,8 @@ xlate_fin_timeout(struct xlate_ctx *ctx,
const struct ofpact_fin_timeout *oft)
{
if (ctx->xin->tcp_flags & (TCP_FIN | TCP_RST) && ctx->rule) {
- ofproto_rule_reduce_timeouts(&ctx->rule->up, oft->fin_idle_timeout,
- oft->fin_hard_timeout);
+ rule_reduce_timeouts(ctx->rule, oft->fin_idle_timeout,
+ oft->fin_hard_timeout);
}
}
@@ -2602,8 +2604,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
ofpacts = xin->ofpacts;
ofpacts_len = xin->ofpacts_len;
} else if (xin->rule) {
- ofpacts = xin->rule->up.ofpacts;
- ofpacts_len = xin->rule->up.ofpacts_len;
+ rule_get_actions(xin->rule, &ofpacts, &ofpacts_len);
} else {
NOT_REACHED();
}
diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
index ba24e92..a54a9e4 100644
--- a/ofproto/ofproto-dpif-xlate.h
+++ b/ofproto/ofproto-dpif-xlate.h
@@ -21,6 +21,8 @@
#include "ofpbuf.h"
#include "ofproto-dpif-mirror.h"
#include "ofproto-dpif.h"
+#include "ofproto.h"
+#include "stp.h"
struct bfd;
struct bond;
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 70a226c..4aec12a 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -83,7 +83,29 @@ BUILD_ASSERT_DECL(N_TABLES >= 2 && N_TABLES <= 255);
struct flow_miss;
struct facet;
+struct rule_dpif {
+ struct rule up;
+
+ /* These statistics:
+ *
+ * - Do include packets and bytes from facets that have been deleted or
+ * whose own statistics have been folded into the rule.
+ *
+ * - Do include packets and bytes sent "by hand" that were accounted to
+ * the rule without any facet being involved (this is a rare corner
+ * case in rule_execute()).
+ *
+ * - Do not include packet or bytes that can be obtained from any facet's
+ * packet_count or byte_count member or that can be obtained from the
+ * datapath by, e.g., dpif_flow_get() for any subfacet.
+ */
+ struct ovs_mutex stats_mutex;
+ uint64_t packet_count OVS_GUARDED; /* Number of packets received. */
+ uint64_t byte_count OVS_GUARDED; /* Number of bytes received. */
+};
+
static void rule_get_stats(struct rule *, uint64_t *packets, uint64_t *bytes);
+static struct rule_dpif *rule_dpif_cast(const struct rule *);
struct ofbundle {
struct hmap_node hmap_node; /* In struct ofproto's "bundles" hmap. */
@@ -1359,7 +1381,7 @@ add_internal_flow(struct ofproto_dpif *ofproto, int id,
if (rule_dpif_lookup_in_table(ofproto, &fm.match.flow, NULL, TBL_INTERNAL,
rulep)) {
- ovs_rwlock_unlock(&(*rulep)->up.evict);
+ rule_release(*rulep);
} else {
NOT_REACHED();
}
@@ -4500,6 +4522,33 @@ rule_credit_stats(struct rule_dpif *rule, const struct dpif_flow_stats *stats)
ofproto_rule_update_used(&rule->up, stats->used);
ovs_mutex_unlock(&rule->stats_mutex);
}
+
+bool
+rule_fail_open(const struct rule_dpif *rule)
+{
+ return rule->up.cr.priority == FAIL_OPEN_PRIORITY;
+}
+
+ovs_be64
+rule_get_flow_cookie(const struct rule_dpif *rule)
+{
+ return rule->up.flow_cookie;
+}
+
+void
+rule_reduce_timeouts(struct rule_dpif *rule, uint16_t idle_timeout,
+ uint16_t hard_timeout)
+{
+ ofproto_rule_reduce_timeouts(&rule->up, idle_timeout, hard_timeout);
+}
+
+void
+rule_get_actions(const struct rule_dpif *rule, const struct ofpact **ofpacts,
+ size_t *ofpacts_len)
+{
+ *ofpacts = rule->up.ofpacts;
+ *ofpacts_len = rule->up.ofpacts_len;
+}
/* Subfacets. */
@@ -4768,9 +4817,8 @@ rule_dpif_lookup(struct ofproto_dpif *ofproto, const struct flow *flow,
flow->in_port.ofp_port);
}
- *rule = choose_miss_rule(port ? port->up.pp.config : 0, ofproto->miss_rule,
- ofproto->no_packet_in_rule);
- ovs_rwlock_rdlock(&(*rule)->up.evict);
+ choose_miss_rule(port ? port->up.pp.config : 0, ofproto->miss_rule,
+ ofproto->no_packet_in_rule, rule);
}
bool
@@ -4825,11 +4873,13 @@ rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto,
/* Given a port configuration (specified as zero if there's no port), chooses
* which of 'miss_rule' and 'no_packet_in_rule' should be used in case of a
* flow table miss. */
-struct rule_dpif *
+void
choose_miss_rule(enum ofputil_port_config config, struct rule_dpif *miss_rule,
- struct rule_dpif *no_packet_in_rule)
+ struct rule_dpif *no_packet_in_rule, struct rule_dpif **rule)
+ OVS_NO_THREAD_SAFETY_ANALYSIS
{
- return config & OFPUTIL_PC_NO_PACKET_IN ? no_packet_in_rule : miss_rule;
+ *rule = config & OFPUTIL_PC_NO_PACKET_IN ? no_packet_in_rule : miss_rule;
+ ovs_rwlock_rdlock(&(*rule)->up.evict);
}
void
@@ -4856,6 +4906,11 @@ complete_operation(struct rule_dpif *rule)
}
}
+static struct rule_dpif *rule_dpif_cast(const struct rule *rule)
+{
+ return rule ? CONTAINER_OF(rule, struct rule_dpif, up) : NULL;
+}
+
static struct rule *
rule_alloc(void)
{
diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index 15e58e9..03773f3 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -19,16 +19,18 @@
#include "hmapx.h"
#include "odp-util.h"
-#include "ofproto/ofproto-provider.h"
+#include "ofp-util.h"
#include "ovs-thread.h"
#include "timer.h"
#include "util.h"
#include "ovs-thread.h"
union user_action_cookie;
+struct dpif_flow_stats;
struct ofproto_dpif;
struct ofport_dpif;
struct dpif_backer;
+struct OVS_LOCKABLE rule_dpif;
/* Ofproto-dpif -- DPIF based ofproto implementation.
*
@@ -58,48 +60,35 @@ struct dpif_backer;
* Ofproto-dpif-xlate is responsible for translating translating OpenFlow
* actions into datapath actions. */
-struct rule_dpif {
- struct rule up;
-
- /* These statistics:
- *
- * - Do include packets and bytes from facets that have been deleted or
- * whose own statistics have been folded into the rule.
- *
- * - Do include packets and bytes sent "by hand" that were accounted to
- * the rule without any facet being involved (this is a rare corner
- * case in rule_execute()).
- *
- * - Do not include packet or bytes that can be obtained from any facet's
- * packet_count or byte_count member or that can be obtained from the
- * datapath by, e.g., dpif_flow_get() for any subfacet.
- */
- struct ovs_mutex stats_mutex;
- uint64_t packet_count OVS_GUARDED; /* Number of packets received. */
- uint64_t byte_count OVS_GUARDED; /* Number of bytes received. */
-};
-
-static inline struct rule_dpif *rule_dpif_cast(const struct rule *rule)
-{
- return rule ? CONTAINER_OF(rule, struct rule_dpif, up) : NULL;
-}
-
void rule_dpif_lookup(struct ofproto_dpif *, const struct flow *,
struct flow_wildcards *, struct rule_dpif **rule)
- OVS_ACQ_RDLOCK((*rule)->up.evict);
+ OVS_ACQ_RDLOCK(*rule);
bool rule_dpif_lookup_in_table(struct ofproto_dpif *, const struct flow *,
struct flow_wildcards *, uint8_t table_id,
struct rule_dpif **rule)
- OVS_TRY_RDLOCK(true, (*rule)->up.evict);
+ OVS_TRY_RDLOCK(true, *rule);
+
+void rule_release(struct rule_dpif *rule) OVS_RELEASES(rule);
+
+void rule_credit_stats(struct rule_dpif *rule ,
+ const struct dpif_flow_stats *);
+
+bool rule_fail_open(const struct rule_dpif *rule);
+
+void rule_get_actions(const struct rule_dpif *rule,
+ const struct ofpact **ofpacts, size_t *ofpacts_len);
-void rule_release(struct rule_dpif *rule) OVS_RELEASES(rule->up.evict);
+ovs_be64 rule_get_flow_cookie(const struct rule_dpif *rule);
-void rule_credit_stats(struct rule_dpif *, const struct dpif_flow_stats *);
+void rule_reduce_timeouts(struct rule_dpif *rule, uint16_t idle_timeout,
+ uint16_t hard_timeout);
-struct rule_dpif *choose_miss_rule(enum ofputil_port_config,
- struct rule_dpif *miss_rule,
- struct rule_dpif *no_packet_in_rule);
+void choose_miss_rule(enum ofputil_port_config,
+ struct rule_dpif *miss_rule,
+ struct rule_dpif *no_packet_in_rule,
+ struct rule_dpif **rule)
+ OVS_ACQ_RDLOCK(*rule);
bool ofproto_has_vlan_splinters(const struct ofproto_dpif *);
ofp_port_t vsp_realdev_to_vlandev(const struct ofproto_dpif *,
--
1.7.9.5
More information about the dev
mailing list