When the a revalidator thread is updating statistics for an XC_LEARN
xcache entry in xlate_push_stats_entry it uses ofproto_flow_mod_learn.
The revalidator will update stats for rules even if they are in a
removed state or marked as invisible. However, ofproto_flow_mod_learn
will detect if a flow has been removed and re-add it in that case. This
can result in an old learn action replacing the new learn action that
had replaced it in the first place.

This change adds a new parameter to ofproto_flow_mod_learn allowing the
caller to specify an action to take if temp_rule is removed. If it is
removed and the caller is xlate_push_stats_entry, then a new rule will
not be replaced.

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2213892
Signed-off-by: Mike Pattrick <m...@redhat.com>
---
 ofproto/ofproto-dpif-xlate-cache.c |  2 +-
 ofproto/ofproto-dpif-xlate.c       |  2 +-
 ofproto/ofproto-dpif.c             |  2 +-
 ofproto/ofproto-provider.h         |  6 ++++--
 ofproto/ofproto.c                  | 21 ++++++++++++++-------
 5 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/ofproto/ofproto-dpif-xlate-cache.c 
b/ofproto/ofproto-dpif-xlate-cache.c
index 9224ee2e6..843903614 100644
--- a/ofproto/ofproto-dpif-xlate-cache.c
+++ b/ofproto/ofproto-dpif-xlate-cache.c
@@ -125,7 +125,7 @@ xlate_push_stats_entry(struct xc_entry *entry,
     case XC_LEARN: {
         enum ofperr error;
         error = ofproto_flow_mod_learn(entry->learn.ofm, true,
-                                       entry->learn.limit, NULL);
+                                       entry->learn.limit, NULL, false);
         if (error) {
             static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
             VLOG_WARN_RL(&rl, "xcache LEARN action execution failed.");
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 4928ea99c..8aa2ed699 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -5701,7 +5701,7 @@ xlate_learn_action(struct xlate_ctx *ctx, const struct 
ofpact_learn *learn)
             bool success = true;
             if (ctx->xin->allow_side_effects) {
                 error = ofproto_flow_mod_learn(ofm, ctx->xin->xcache != NULL,
-                                               learn->limit, &success);
+                                               learn->limit, &success, true);
             } else if (learn->limit) {
                 if (!ofm->temp_rule
                     || ofm->temp_rule->state != RULE_INSERTED) {
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index fad7342b0..465f2a3f4 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4876,7 +4876,7 @@ packet_xlate(struct ofproto *ofproto_, struct 
ofproto_packet_out *opo)
             if (entry->type == XC_LEARN) {
                 struct ofproto_flow_mod *ofm = entry->learn.ofm;
 
-                error = ofproto_flow_mod_learn_refresh(ofm);
+                error = ofproto_flow_mod_learn_refresh(ofm, true);
                 if (error) {
                     goto error_out;
                 }
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 143ded690..de1823c89 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -2027,9 +2027,11 @@ enum ofperr ofproto_flow_mod_init_for_learn(struct 
ofproto *,
                                             struct ofproto_flow_mod *)
     OVS_EXCLUDED(ofproto_mutex);
 enum ofperr ofproto_flow_mod_learn(struct ofproto_flow_mod *, bool keep_ref,
-                                   unsigned limit, bool *below_limit)
+                                   unsigned limit, bool *below_limit,
+                                   bool recreate)
     OVS_EXCLUDED(ofproto_mutex);
-enum ofperr ofproto_flow_mod_learn_refresh(struct ofproto_flow_mod *ofm);
+enum ofperr ofproto_flow_mod_learn_refresh(struct ofproto_flow_mod *ofm,
+                                           bool recreate);
 enum ofperr ofproto_flow_mod_learn_start(struct ofproto_flow_mod *ofm)
     OVS_REQUIRES(ofproto_mutex);
 void ofproto_flow_mod_learn_revert(struct ofproto_flow_mod *ofm)
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index dbf4958bc..dd2562acb 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -5472,7 +5472,7 @@ ofproto_flow_mod_init_for_learn(struct ofproto *ofproto,
 }
 
 enum ofperr
-ofproto_flow_mod_learn_refresh(struct ofproto_flow_mod *ofm)
+ofproto_flow_mod_learn_refresh(struct ofproto_flow_mod *ofm, bool recreate)
 {
     enum ofperr error = 0;
 
@@ -5492,6 +5492,11 @@ ofproto_flow_mod_learn_refresh(struct ofproto_flow_mod 
*ofm)
      * after we have read the 'rule->state' below.  In this case the next time
      * this function is executed the rule will be reinstated. */
     if (rule->state == RULE_REMOVED) {
+        if (!recreate) {
+            /* If the rule is marked as removed and the caller doesn't want it
+             * recreated, just return an error here as if there was no rule. */
+            return OFPERR_OFPFMFC_UNKNOWN;
+        }
         struct cls_rule cr;
 
         cls_rule_clone(&cr, &rule->cr);
@@ -5565,10 +5570,12 @@ ofproto_flow_mod_learn_finish(struct ofproto_flow_mod 
*ofm,
 
 /* Refresh 'ofm->temp_rule', for which the caller holds a reference, if already
  * in the classifier, insert it otherwise.  If the rule has already been
- * removed from the classifier, a new rule is created using 'ofm->temp_rule' as
- * a template and the reference to the old 'ofm->temp_rule' is freed.  If
- * 'keep_ref' is true, then a reference to the current rule is held, otherwise
- * it is released and 'ofm->temp_rule' is set to NULL.
+ * removed from the classifier and 'recreate' is true, a new rule is created
+ * using 'ofm->temp_rule' as a template and the reference to the old
+ * 'ofm->temp_rule' is freed.  If recreate is false and the rule has been
+ * removed, do not reinsert a new copy of that rule. If 'keep_ref' is true,
+ * then a reference to the current rule is held, otherwise it is released and
+ * 'ofm->temp_rule' is set to NULL.
  *
  * If 'limit' != 0, insertion will fail if there are more than 'limit' rules
  * in the same table with the same cookie.  If insertion succeeds,
@@ -5579,10 +5586,10 @@ ofproto_flow_mod_learn_finish(struct ofproto_flow_mod 
*ofm,
  * during the call. */
 enum ofperr
 ofproto_flow_mod_learn(struct ofproto_flow_mod *ofm, bool keep_ref,
-                       unsigned limit, bool *below_limitp)
+                       unsigned limit, bool *below_limitp, bool recreate)
     OVS_EXCLUDED(ofproto_mutex)
 {
-    enum ofperr error = ofproto_flow_mod_learn_refresh(ofm);
+    enum ofperr error = ofproto_flow_mod_learn_refresh(ofm, recreate);
     struct rule *rule = ofm->temp_rule;
     bool below_limit = true;
 
-- 
2.39.3

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to