@@ -2007,6 +2007,7 @@ struct ofproto_flow_mod {
bool modify_keep_counts;
enum nx_flow_update_event event;
uint8_t table_id;
+ bool keep_flow_mod; /* Preserve criteria beyond start. */
/* These are only used during commit execution.
* ofproto_flow_mod_uninit() does NOT clean these up. */
@@ -143,6 +143,10 @@ static void rule_criteria_init(struct rule_criteria *, uint8_t table_id,
static void rule_criteria_require_rw(struct rule_criteria *,
bool can_write_readonly);
static void rule_criteria_destroy(struct rule_criteria *);
+static void rule_criteria_clone(struct rule_criteria *,
+ const struct rule_criteria *);
+static void rule_criteria_replace(struct rule_criteria *,
+ const struct rule_criteria *);
static enum ofperr collect_rules_loose(struct ofproto *,
const struct rule_criteria *,
@@ -4600,6 +4604,31 @@ rules_mark_for_removal(struct ofproto *ofproto, struct rule_collection *rules)
rule_collection_destroy(rules);
}
+static void
+rule_criteria_clone(struct rule_criteria *dst,
+ const struct rule_criteria *src)
+{
+ cls_rule_clone(&dst->cr, &src->cr);
+ dst->table_id = src->table_id;
+ dst->version = src->version;
+ dst->cookie = src->cookie;
+ dst->cookie_mask = src->cookie_mask;
+ dst->out_port = src->out_port;
+ dst->out_group = src->out_group;
+ dst->include_hidden = src->include_hidden;
+ dst->include_readonly = src->include_readonly;
+}
+
+static void
+rule_criteria_replace(struct rule_criteria *dst,
+ const struct rule_criteria *src)
+{
+ if (dst->version != OVS_VERSION_NOT_REMOVED) {
+ rule_criteria_destroy(dst);
+ }
+ rule_criteria_clone(dst, src);
+}
+
/* Schedules postponed removal of rules, destroys 'rules'. */
static void
remove_rules_postponed(struct rule_collection *rules)
@@ -5605,6 +5634,14 @@ ofproto_flow_mod_learn_start(struct ofproto_flow_mod *ofm)
OVS_REQUIRES(ofproto_mutex)
{
struct rule *rule = ofm->temp_rule;
+ struct rule_criteria saved_criteria;
+ bool saved = false;
+
+ if (ofm->keep_flow_mod
+ && ofm->criteria.version != OVS_VERSION_NOT_REMOVED) {
+ rule_criteria_clone(&saved_criteria, &ofm->criteria);
+ saved = true;
+ }
/* ofproto_flow_mod_start() consumes the reference, so we
* take a new one. */
@@ -5612,6 +5649,11 @@ ofproto_flow_mod_learn_start(struct ofproto_flow_mod *ofm)
enum ofperr error = ofproto_flow_mod_start(rule->ofproto, ofm);
ofm->temp_rule = rule;
+ if (saved) {
+ rule_criteria_replace(&ofm->criteria, &saved_criteria);
+ rule_criteria_destroy(&saved_criteria);
+ }
+
return error;
}
@@ -5674,6 +5716,8 @@ ofproto_flow_mod_learn(struct ofproto_flow_mod *ofm, bool keep_ref,
struct rule *rule = ofm->temp_rule;
bool below_limit = true;
+ ofm->keep_flow_mod = keep_ref;
+
/* Do we need to insert the rule? */
if (!error && rule->state == RULE_INITIALIZED) {
ovs_mutex_lock(&ofproto_mutex);
@@ -8232,6 +8276,7 @@ ofproto_flow_mod_init(struct ofproto *ofproto, struct ofproto_flow_mod *ofm,
ofm->conjs = NULL;
ofm->n_conjs = 0;
ofm->table_id = fm->table_id;
+ ofm->keep_flow_mod = false;
/* Initialize flag used by ofproto_dpif_xcache_execute(). */
ofm->learn_adds_rule = false;
Learn actions cache struct ofproto_flow_mod instances, but ofproto_flow_mod_start() used to destroy their rule_criteria, leaving later replays with dangling minimask pointers that crashed in classifier_find_rule_exactly(). This change adds a keep_flow_mod flag, clones the criteria before the initial start, restores it afterward via new clone/replace helpers, and lets normal ofproto_flow_mod_uninit() free the clone. As a result, cached learn xcache entries can safely replay without touching freed memory. The call trace: *0 minimask_hash (basis=0, mask=0x7f05d7fd3f38) at lib/classifier-private.h:325 *1 find_subtable (cls=cls@entry=0x5598cf591588, mask=0x7f05d7fd3f38) at lib/classifier.c:1431 *2 0x00007f0770ab207d in classifier_find_rule_exactly (cls=cls@entry=0x5598cf591588, target=target@entry=0x7f064c696e30, version=18446744073709551615) at lib/classifier.c:1184 *3 0x00007f077110edb8 in collect_rules_strict (ofproto=ofproto@entry=0x5598cf4f45c0, criteria=criteria@entry=0x7f064c696e28, rules=rules@entry=0x7f064c696ec0) at ofproto/ofproto.c:4524 *4 0x00007f0771112c17 in modify_flow_start_strict (ofm=0x7f064c696e20, ofproto=0x5598cf4f45c0) at ofproto/ofproto.c:5789 *5 ofproto_flow_mod_start (ofproto=0x5598cf4f45c0, ofm=ofm@entry=0x7f064c696e20) at ofproto/ofproto.c:7943 *6 0x00007f0771112fd0 in ofproto_flow_mod_learn_start ( ofm=ofm@entry=0x7f064c696e20) at ofproto/ofproto.c:5331 *7 0x00007f0771114cad in ofproto_flow_mod_learn (ofm=0x7f064c696e20, keep_ref=<optimized out>, limit=<optimized out>, below_limitp=0x0) at ofproto/ofproto.c:5416 *8 0x00007f0771141f73 in xlate_push_stats_entry (entry=0x7f0651563d10, stats=0x0) at ofproto/ofproto-dpif-xlate-cache.c:127 *9 0x00007f077114206d in ofpbuf_try_pull (size=40, b=<synthetic pointer>) at include/openvswitch/ofpbuf.h:262 *10 xlate_push_stats (xcache=<optimized out>, stats=0x0) at ofproto/ofproto-dpif-xlate-cache.c:180 *11 0x00007f077112e080 in push_dp_ops (udpif=0x5598cf4caf90, ops=<optimized out>, n_ops=<optimized out>) at ofproto/ofproto-dpif-upcall.c:2409 *12 0x00007f077112e195 in push_ukey_ops (udpif=0x5598cf591588, umap=0x0, ops=0x7f0669b1ffd0, n_ops=860610658) at ofproto/ofproto-dpif-upcall.c:2439 *13 0x00005598cf4cb9a8 in ?? () *14 0x00007f077112ef7d in revalidator_sweep__ (revalidator=<optimized out>, purge=false) at ofproto/ofproto-dpif-upcall.c:2799 *15 0x00007f07711323c5 in udpif_revalidator (arg=0x5598cf4dcf80) at ofproto/ofproto-dpif-upcall.c:945 *16 0x00007f0770b6715f in ovsthread_wrapper (aux_=<optimized out>) at lib/ovs-thread.c:383 *17 0x00007f076f922e65 in start_thread (arg=0x0) at pthread_create.c:282 *18 0x00007f076ee4088d in __libc_ifunc_impl_list (name=<optimized out>, array=0x7f0669b22700, max=<optimized out>) at ../sysdeps/x86_64/multiarch/ifunc-impl-list.c:329 Signed-off-by: LIU Yulong <i@liuyulong.me> --- ofproto/ofproto-provider.h | 1 + ofproto/ofproto.c | 45 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+)