Store ofproto provider meter ID corresponding to the OpenFlow meter ID to
the meter action when installing flow's actions to avoid multi-threaded
access to ofproto's meters.
Changed the ofproto meter API to require the provider keep the provider
meter ID unchanged when modifying a meter.
Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com>
---
lib/ofp-actions.h | 3 +-
ofproto/ofproto-provider.h | 8 ++---
ofproto/ofproto.c | 79 ++++++++++++++++++++++++++++++--------------
ofproto/ofproto.h | 3 --
4 files changed, 61 insertions(+), 32 deletions(-)
diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h
index a3fb60f..0784384 100644
--- a/lib/ofp-actions.h
+++ b/lib/ofp-actions.h
@@ -381,7 +381,8 @@ struct ofpact_metadata {
* Used for OFPIT13_METER. */
struct ofpact_meter {
struct ofpact ofpact;
- uint32_t meter_id;
+ uint32_t meter_id; /* OpenFlow meter ID. */
+ uint32_t provider_meter_id; /* Used internally by ofproto providers. */
};
/* OFPACT_RESUBMIT.
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 0b8a5e5..4def111 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -85,6 +85,7 @@ struct ofproto {
* OpenFlow meters start at 1. To avoid confusion we leave the first
* pointer in the array un-used, and index directly with the OpenFlow
* meter_id. */
+ struct ovs_rwlock meters_rwlock;
struct ofputil_meter_features meter_features;
struct meter **meters; /* 'meter_features.max_meter' + 1 pointers. */
@@ -1480,10 +1481,9 @@ struct ofproto_class {
* implementation.
*
* If '*id' is a value other than UINT32_MAX, modifies the existing meter
- * with that meter provider ID to have configuration 'config'. On failure,
- * the existing meter configuration is left intact. Regardless of success,
- * any change to '*id' updates the provider meter id used for this
- * meter. */
+ * with that meter provider ID to have configuration 'config', while
+ * leaving '*id' unchanged. On failure, the existing meter configuration
+ * is left intact. */
enum ofperr (*meter_set)(struct ofproto *ofproto, ofproto_meter_id *id,
const struct ofputil_meter_config *config);
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index f413193..71db0b6 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2491,40 +2491,63 @@ reject_slave_controller(struct ofconn *ofconn)
}
/* Finds the OFPACT_METER action, if any, in the 'ofpacts_len' bytes of
- * 'ofpacts'. If found, returns its meter ID; if not, returns 0.
+ * 'ofpacts'. If found, returns pointer to the ofpact found, or NULL.
*
* This function relies on the order of 'ofpacts' being correct (as checked by
* ofpacts_verify()). */
-static uint32_t
-find_meter(const struct ofpact ofpacts[], size_t ofpacts_len)
+static struct ofpact *
+find_meter_ofpact(struct ofpact ofpacts[], size_t ofpacts_len)
{
- const struct ofpact *a;
+ struct ofpact *a;
OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) {
enum ovs_instruction_type inst;
inst = ovs_instruction_type_from_ofpact_type(a->type);
if (a->type == OFPACT_METER) {
- return ofpact_get_METER(a)->meter_id;
+ return a;
} else if (inst > OVSINST_OFPIT13_METER) {
break;
}
}
+ return NULL;
+
+}
+
+/* Finds the OFPACT_METER action, if any, in the 'ofpacts_len' bytes of
+ * 'ofpacts'. If found, returns its meter ID; if not, returns 0.
+ *
+ * This function relies on the order of 'ofpacts' being correct (as checked by
+ * ofpacts_verify()). */
+static uint32_t
+find_meter(struct ofpact ofpacts[], size_t ofpacts_len)
+{
+ const struct ofpact *a = find_meter_ofpact(ofpacts, ofpacts_len);
+
+ if (a) {
+ return ofpact_get_METER(a)->meter_id;
+ }
+
return 0;
}
+static uint32_t get_provider_meter_id(const struct ofproto * ofproto,
+ uint32_t of_meter_id);
+
/* Checks that the 'ofpacts_len' bytes of actions in 'ofpacts' are appropriate
* for a packet with the prerequisites satisfied by 'flow' in table 'table_id'.
* 'flow' may be temporarily modified, but is restored at return.
- */
+ * ofproto private fields in actions may be set as required for further action
+ * processing. */
static enum ofperr
-ofproto_check_ofpacts(struct ofproto *ofproto,
- const struct ofpact ofpacts[], size_t ofpacts_len,
- struct flow *flow, uint8_t table_id)
+ofproto_check_and_set_ofpacts(struct ofproto *ofproto,
+ struct ofpact ofpacts[],
+ size_t ofpacts_len,
+ struct flow *flow, uint8_t table_id)
{
enum ofperr error;
- uint32_t mid;
+ struct ofpact *a;
error = ofpacts_check(ofpacts, ofpacts_len, flow,
u16_to_ofp(ofproto->max_ports), table_id);
@@ -2532,9 +2555,16 @@ ofproto_check_ofpacts(struct ofproto *ofproto,
return error;
}
- mid = find_meter(ofpacts, ofpacts_len);
- if (mid && ofproto_get_provider_meter_id(ofproto, mid) == UINT32_MAX) {
- return OFPERR_OFPMMFC_INVALID_METER;
+ a = find_meter_ofpact(ofpacts, ofpacts_len);
+ if (a) {
+ uint32_t mid;
+ mid = get_provider_meter_id(ofproto, ofpact_get_METER(a)->meter_id);
+ if (mid == UINT32_MAX) {
+ return OFPERR_OFPMMFC_INVALID_METER;
+ } else {
+ /* Store the provider meter ID. */
+ ofpact_get_METER(a)->provider_meter_id = mid;
+ }
}
return 0;
}
@@ -2585,7 +2615,8 @@ handle_packet_out(struct ofconn *ofconn, const struct
ofp_header *oh)
/* Verify actions against packet, then send packet if successful. */
in_port_.ofp_port = po.in_port;
flow_extract(payload, 0, 0, NULL, &in_port_, &flow);
- error = ofproto_check_ofpacts(p, po.ofpacts, po.ofpacts_len, &flow, 0);
+ error = ofproto_check_and_set_ofpacts(p, po.ofpacts, po.ofpacts_len,
+ &flow, 0);
if (!error) {
error = p->ofproto_class->packet_out(p, payload, &flow,
po.ofpacts, po.ofpacts_len);
@@ -3512,8 +3543,9 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
}
/* Verify actions. */
- error = ofproto_check_ofpacts(ofproto, fm->ofpacts, fm->ofpacts_len,
- &fm->match.flow, table_id);
+ error = ofproto_check_and_set_ofpacts(ofproto, fm->ofpacts,
+ fm->ofpacts_len, &fm->match.flow,
+ table_id);
if (error) {
cls_rule_destroy(&cr);
return error;
@@ -4445,9 +4477,9 @@ handle_flow_monitor_cancel(struct ofconn *ofconn, const
struct ofp_header *oh)
* 'provider_meter_id' is for the provider's private use.
*/
struct meter {
+ ofproto_meter_id provider_meter_id;
long long int created; /* Time created. */
struct list rules; /* List of "struct rule_dpif"s. */
- ofproto_meter_id provider_meter_id;
uint16_t flags; /* Meter flags. */
uint16_t n_bands; /* Number of meter bands. */
struct ofputil_meter_band *bands;
@@ -4456,13 +4488,10 @@ struct meter {
/*
* This is used in instruction validation at flow set-up time,
* as flows may not use non-existing meters.
- * This is also used by ofproto-providers to translate OpenFlow meter_ids
- * in METER instructions to the corresponding provider meter IDs.
* Return value of UINT32_MAX signifies an invalid meter.
*/
-uint32_t
-ofproto_get_provider_meter_id(const struct ofproto * ofproto,
- uint32_t of_meter_id)
+static uint32_t
+get_provider_meter_id(const struct ofproto * ofproto, uint32_t of_meter_id)
{
if (of_meter_id && of_meter_id <= ofproto->meter_features.max_meters) {
const struct meter *meter = ofproto->meters[of_meter_id];
@@ -4533,7 +4562,7 @@ handle_add_meter(struct ofproto *ofproto, struct
ofputil_meter_mod *mm)
ovs_assert(provider_meter_id.uint32 != UINT32_MAX);
*meterp = meter_create(&mm->meter, provider_meter_id);
}
- return 0;
+ return error;
}
static enum ofperr
@@ -4541,15 +4570,17 @@ handle_modify_meter(struct ofproto *ofproto, struct
ofputil_meter_mod *mm)
{
struct meter *meter = ofproto->meters[mm->meter.meter_id];
enum ofperr error;
+ uint32_t provider_meter_id;
if (!meter) {
return OFPERR_OFPMMFC_UNKNOWN_METER;
}
+ provider_meter_id = meter->provider_meter_id.uint32;
error = ofproto->ofproto_class->meter_set(ofproto,
&meter->provider_meter_id,
&mm->meter);
- ovs_assert(meter->provider_meter_id.uint32 != UINT32_MAX);
+ ovs_assert(meter->provider_meter_id.uint32 == provider_meter_id);
if (!error) {
meter_update(meter, &mm->meter);
}
diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index 9adda2c..c42b068 100644
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -434,9 +434,6 @@ bool ofproto_has_vlan_usage_changed(const struct ofproto *);
int ofproto_port_set_realdev(struct ofproto *, ofp_port_t vlandev_ofp_port,
ofp_port_t realdev_ofp_port, int vid);
-uint32_t ofproto_get_provider_meter_id(const struct ofproto *,
- uint32_t of_meter_id);
-
#ifdef __cplusplus
}
#endif
--
1.7.10.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
