Allocate struct rule_actions and the space for the actions at once.
This reduces one memory indirection and helps reduce cache misses
visible in perf annotations.
Fix some old comments referring to ref count, since we now use RCU for
this.
Enforce constness of the actions that are assigned from rule_actions
throughout the code.
Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
* Used for NXAST_OUTPUT_REG. */
struct ofpact_output_reg {
struct ofpact ofpact;
- struct mf_subfield src;
uint16_t max_len;
+ struct mf_subfield src;
};
/* OFPACT_BUNDLE.
size_t i;
for (i = 0; i < *n_fms; i++) {
- free((*fms)[i].ofpacts);
+ free(CONST_CAST(struct ofpact *, (*fms)[i].ofpacts));
}
free(*fms);
*fms = NULL;
ofp_port_t out_port;
uint32_t out_group;
enum ofputil_flow_mod_flags flags;
- struct ofpact *ofpacts; /* Series of "struct ofpact"s. */
- size_t ofpacts_len; /* Length of ofpacts, in bytes. */
+ struct ofpact *ofpacts; /* Series of "struct ofpact"s. */
+ size_t ofpacts_len; /* Length of ofpacts, in bytes. */
};
enum ofperr ofputil_decode_flow_mod(struct ofputil_flow_mod *,
int hard_age; /* Seconds since last change, -1 if unknown. */
uint64_t packet_count; /* Packet count, UINT64_MAX if unknown. */
uint64_t byte_count; /* Byte count, UINT64_MAX if unknown. */
- struct ofpact *ofpacts;
+ const struct ofpact *ofpacts;
size_t ofpacts_len;
enum ofputil_flow_mod_flags flags;
};
uint16_t priority;
ovs_be64 cookie;
struct match *match;
- struct ofpact *ofpacts;
+ const struct ofpact *ofpacts;
size_t ofpacts_len;
/* Used only for NXFME_ABBREV. */
ovs_mutex_unlock(&rule->mutex);
if (flags & NXFMF_ACTIONS) {
- struct rule_actions *actions = rule_get_actions(rule);
+ const struct rule_actions *actions = rule_get_actions(rule);
fu.ofpacts = actions->ofpacts;
fu.ofpacts_len = actions->ofpacts_len;
} else {
xlate_recursively(struct xlate_ctx *ctx, struct rule_dpif *rule)
{
struct rule_dpif *old_rule = ctx->rule;
- struct rule_actions *actions;
+ const struct rule_actions *actions;
if (ctx->xin->resubmit_stats) {
rule_dpif_credit_stats(rule, ctx->xin->resubmit_stats);
struct flow *flow = &xin->flow;
struct rule_dpif *rule = NULL;
- struct rule_actions *actions = NULL;
+ const struct rule_actions *actions = NULL;
enum slow_path_reason special;
const struct ofpact *ofpacts;
struct xport *in_port;
/* Returns 'rule''s actions. The caller owns a reference on the returned
* actions and must eventually release it (with rule_actions_unref()) to avoid
* a memory leak. */
-struct rule_actions *
+const struct rule_actions *
rule_dpif_get_actions(const struct rule_dpif *rule)
{
return rule_get_actions(&rule->up);
static void
trace_format_rule(struct ds *result, int level, const struct rule_dpif *rule)
{
- struct rule_actions *actions;
+ const struct rule_actions *actions;
ovs_be64 cookie;
ds_put_char_multiple(result, '\t', level);
bool rule_dpif_is_internal(const struct rule_dpif *);
uint8_t rule_dpif_get_table(const struct rule_dpif *);
-struct rule_actions *rule_dpif_get_actions(const struct rule_dpif *);
+const struct rule_actions *rule_dpif_get_actions(const struct rule_dpif *);
ovs_be64 rule_dpif_get_flow_cookie(const struct rule_dpif *rule);
#include "heap.h"
#include "hindex.h"
#include "list.h"
+#include "ofp-actions.h"
#include "ofp-errors.h"
#include "ofp-util.h"
#include "ofproto/ofproto.h"
#include "timeval.h"
struct match;
-struct ofpact;
struct ofputil_flow_mod;
struct bfd_cfg;
struct meter;
/* OpenFlow actions. See struct rule_actions for more thread-safety
* notes. */
- OVSRCU_TYPE(struct rule_actions *) actions;
+ OVSRCU_TYPE(const struct rule_actions *) actions;
/* In owning meter's 'rules' list. An empty list if there is no meter. */
struct list meter_list_node OVS_GUARDED_BY(ofproto_mutex);
void ofproto_rule_ref(struct rule *);
void ofproto_rule_unref(struct rule *);
-static inline struct rule_actions *
+static inline const struct rule_actions *
rule_get_actions(const struct rule *rule)
{
- return ovsrcu_get(struct rule_actions *, &rule->actions);
+ return ovsrcu_get(const struct rule_actions *, &rule->actions);
}
/* Returns true if 'rule' is an OpenFlow 1.3 "table-miss" rule, false
* Thread-safety
* =============
*
- * A struct rule_actions 'actions' may be accessed without a risk of being
+ * A struct rule_actions may be accessed without a risk of being
* freed by code that holds a read-lock or write-lock on 'rule->mutex' (where
- * 'rule' is the rule for which 'rule->actions == actions') or that owns a
- * reference to 'actions->ref_count' (or both). */
+ * 'rule' is the rule for which 'rule->actions == actions') or during the RCU
+ * active period. */
struct rule_actions {
/* These members are immutable: they do not change during the struct's
* lifetime. */
- struct ofpact *ofpacts; /* Sequence of "struct ofpacts". */
- unsigned int ofpacts_len; /* Size of 'ofpacts', in bytes. */
- uint32_t provider_meter_id; /* Datapath meter_id, or UINT32_MAX. */
+ uint32_t ofpacts_len; /* Size of 'ofpacts', in bytes. */
+ uint32_t provider_meter_id; /* Datapath meter_id, or UINT32_MAX. */
+ struct ofpact ofpacts[]; /* Sequence of "struct ofpacts". */
};
+BUILD_ASSERT_DECL(offsetof(struct rule_actions, ofpacts) % OFPACT_ALIGNTO == 0);
-struct rule_actions *rule_actions_create(const struct ofproto *,
- const struct ofpact *, size_t);
-void rule_actions_destroy(struct rule_actions *);
+const struct rule_actions *rule_actions_create(const struct ofproto *,
+ const struct ofpact *, size_t);
+void rule_actions_destroy(const struct rule_actions *);
/* A set of rules to which an OpenFlow operation applies. */
struct rule_collection {
/* OFOPERATION_MODIFY, OFOPERATION_REPLACE: The old actions, if the actions
* are changing. */
- struct rule_actions *actions;
+ const struct rule_actions *actions;
/* OFOPERATION_DELETE. */
enum ofp_flow_removed_reason reason; /* Reason flow was removed. */
rule = rule_from_cls_rule(classifier_find_match_exactly(
&ofproto->tables[0].cls, match, priority));
if (rule) {
- struct rule_actions *actions = rule_get_actions(rule);
+ const struct rule_actions *actions = rule_get_actions(rule);
must_add = !ofpacts_equal(actions->ofpacts, actions->ofpacts_len,
ofpacts, ofpacts_len);
} else {
static uint32_t get_provider_meter_id(const struct ofproto *,
uint32_t of_meter_id);
-/* Creates and returns a new 'struct rule_actions', with a ref_count of 1,
- * whose actions are a copy of from the 'ofpacts_len' bytes of 'ofpacts'. */
-struct rule_actions *
+/* Creates and returns a new 'struct rule_actions', whose actions are a copy
+ * of from the 'ofpacts_len' bytes of 'ofpacts'. */
+const struct rule_actions *
rule_actions_create(const struct ofproto *ofproto,
const struct ofpact *ofpacts, size_t ofpacts_len)
{
struct rule_actions *actions;
- actions = xmalloc(sizeof *actions);
- actions->ofpacts = xmemdup(ofpacts, ofpacts_len);
+ actions = xmalloc(sizeof *actions + ofpacts_len);
actions->ofpacts_len = ofpacts_len;
actions->provider_meter_id
= get_provider_meter_id(ofproto,
ofpacts_get_meter(ofpacts, ofpacts_len));
+ memcpy(actions->ofpacts, ofpacts, ofpacts_len);
return actions;
}
-static void
-rule_actions_destroy_cb(struct rule_actions *actions)
-{
- free(actions->ofpacts);
- free(actions);
-}
-
-/* Decrements 'actions''s ref_count and frees 'actions' if the ref_count
- * reaches 0. */
+/* Free the actions after the RCU quiescent period is reached. */
void
-rule_actions_destroy(struct rule_actions *actions)
+rule_actions_destroy(const struct rule_actions *actions)
{
if (actions) {
- ovsrcu_postpone(rule_actions_destroy_cb, actions);
+ ovsrcu_postpone(free, CONST_CAST(struct rule_actions *, actions));
}
}
long long int now = time_msec();
struct ofputil_flow_stats fs;
long long int created, used, modified;
- struct rule_actions *actions;
+ const struct rule_actions *actions;
enum ofputil_flow_mod_flags flags;
ovs_mutex_lock(&rule->mutex);
flow_stats_ds(struct rule *rule, struct ds *results)
{
uint64_t packet_count, byte_count;
- struct rule_actions *actions;
+ const struct rule_actions *actions;
long long int created, used;
rule->ofproto->ofproto_class->rule_get_stats(rule, &packet_count,
reset_counters = (fm->flags & OFPUTIL_FF_RESET_COUNTS) != 0;
if (actions_changed || reset_counters) {
- struct rule_actions *new_actions;
+ const struct rule_actions *new_actions;
op->actions = rule_get_actions(rule);
new_actions = rule_actions_create(ofproto,
rule->hard_timeout = op->hard_timeout;
ovs_mutex_unlock(&rule->mutex);
if (op->actions) {
- struct rule_actions *old_actions;
+ const struct rule_actions *old_actions;
ovs_mutex_lock(&rule->mutex);
old_actions = rule_get_actions(rule);
{
struct ofproto *ofproto = rule->ofproto;
struct oftable *table = &ofproto->tables[rule->table_id];
- struct rule_actions *actions;
+ const struct rule_actions *actions;
bool may_expire;
ovs_mutex_lock(&rule->mutex);
ds_destroy(&s);
for (i = 0; i < n_fses; i++) {
- free(fses[i].ofpacts);
+ free(CONST_CAST(struct ofpact *, fses[i].ofpacts));
}
free(fses);
struct ofputil_flow_mod *fm = &fms[i];
transact_noreply(vconn, ofputil_encode_flow_mod(fm, protocol));
- free(fm->ofpacts);
+ free(CONST_CAST(struct ofpact *, fm->ofpacts));
}
vconn_close(vconn);
}
fte_version_free(struct fte_version *version)
{
if (version) {
- free(version->ofpacts);
+ free(CONST_CAST(struct ofpact *, version->ofpacts));
free(version);
}
}
ofp_print(stdout, ofpbuf_data(msg), ofpbuf_size(msg), verbosity);
ofpbuf_delete(msg);
- free(fm->ofpacts);
+ free(CONST_CAST(struct ofpact *, fm->ofpacts));
}
}