[ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions/deletions

Aravind Prasad raja.avi at gmail.com
Sat Jun 16 12:44:38 UTC 2018


Currently, rule_insert() and rule_delete() ofproto provider APIs do not


have return values. There are some possible scenarios where rule insertions

and deletions can fail at run-time even though the static checks during

rule_construct() had passed previously.



Some possible scenarios for failure of rule insertions and deletions:



**) Rule insertions can fail dynamically in Hybrid mode (both Openflow and


Normal switch functioning coexist) where the CAM space could get suddenly


filled up by Normal switch functioning and Openflow gets devoid of


available space.



**) Some deployments could have separate independent layers for HW rule


insertions/deletions and application layer to interact with OVS. HW layer


could face any dynamic issue during rule handling which application could


not have predicted/captured in rule-construction phase.





This patch is the first step to introduce error reporting for rule


insertions/deletions from Client back to OVS.


Signed-off-by: Aravind Prasad S <raja.avi at gmail.com>

---

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c

index ca4582c..ca485b3 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4444,7 +4444,7 @@ rule_construct(struct rule *rule_)
     return 0;
 }

-static void
+static enum ofperr
 rule_insert(struct rule *rule_, struct rule *old_rule_, bool forward_counts)
     OVS_REQUIRES(ofproto_mutex)
 {
@@ -4474,6 +4474,8 @@ rule_insert(struct rule *rule_, struct rule
*old_rule_, bool forward_counts)
         ovs_mutex_unlock(&rule->stats_mutex);
         ovs_mutex_unlock(&old_rule->stats_mutex);
     }
+
+    return 0;
 }

 static void
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 2b77b89..8c7ed4e 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -1297,10 +1297,11 @@ struct ofproto_class {
     struct rule *(*rule_alloc)(void);
     enum ofperr (*rule_construct)(struct rule *rule)
         /* OVS_REQUIRES(ofproto_mutex) */;
-    void (*rule_insert)(struct rule *rule, struct rule *old_rule,
-                        bool forward_counts)
+    enum ofperr (*rule_insert)(struct rule *rule, struct rule *old_rule,
+                                                    bool forward_counts)
+        /* OVS_REQUIRES(ofproto_mutex) */;
+    enum ofperr (*rule_delete)(struct rule *rule)
         /* OVS_REQUIRES(ofproto_mutex) */;
-    void (*rule_delete)(struct rule *rule) /* OVS_REQUIRES(ofproto_mutex) */;
     void (*rule_destruct)(struct rule *rule);
     void (*rule_dealloc)(struct rule *rule);

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 829ccd8..f6cea33 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1522,6 +1522,8 @@ void
 ofproto_rule_delete(struct ofproto *ofproto, struct rule *rule)
     OVS_EXCLUDED(ofproto_mutex)
 {
+    int error = 0;
+
     /* This skips the ofmonitor and flow-removed notifications because the
      * switch is being deleted and any OpenFlow channels have been or soon will
      * be killed. */
@@ -1535,7 +1537,10 @@ ofproto_rule_delete(struct ofproto *ofproto,
struct rule *rule)
                                  &rule->cr);
         ofproto_rule_remove__(rule->ofproto, rule);
         if (ofproto->ofproto_class->rule_delete) {
-            ofproto->ofproto_class->rule_delete(rule);
+            error = ofproto->ofproto_class->rule_delete(rule);
+            if (error) {
+                VLOG_INFO("Rule deletion failed, error = %u", error);
+            }
         }

         /* This may not be the last reference to the rule. */
@@ -2882,11 +2887,15 @@ remove_rule_rcu__(struct rule *rule)
 {
     struct ofproto *ofproto = rule->ofproto;
     struct oftable *table = &ofproto->tables[rule->table_id];
+    int error = 0;

     ovs_assert(!cls_rule_visible_in_version(&rule->cr, OVS_VERSION_MAX));
     classifier_remove_assert(&table->cls, &rule->cr);
     if (ofproto->ofproto_class->rule_delete) {
-        ofproto->ofproto_class->rule_delete(rule);
+        error = ofproto->ofproto_class->rule_delete(rule);
+        if (error) {
+            VLOG_INFO("Rule deletion failed, error = %u", error);
+        }
     }
     ofproto_rule_unref(rule);
 }
@@ -5252,6 +5261,7 @@ replace_rule_finish(struct ofproto *ofproto,
struct ofproto_flow_mod *ofm,
     OVS_REQUIRES(ofproto_mutex)
 {
     struct rule *replaced_rule;
+    int error = 0;

     replaced_rule = (old_rule && old_rule->removed_reason != OFPRR_EVICTION)
         ? old_rule : NULL;
@@ -5261,8 +5271,12 @@ replace_rule_finish(struct ofproto *ofproto,
struct ofproto_flow_mod *ofm,
      * link the packet and byte counts from the old rule to the new one if
      * 'modify_keep_counts' is 'true'.  The 'replaced_rule' will be deleted
      * right after this call. */
-    ofproto->ofproto_class->rule_insert(new_rule, replaced_rule,
-                                        ofm->modify_keep_counts);
+    error = ofproto->ofproto_class->rule_insert(new_rule, replaced_rule,
+                                               ofm->modify_keep_counts);
+    if (error) {
+        VLOG_INFO("Rule insert failed, error=%u", error);
+    }
+
     learned_cookies_inc(ofproto, rule_get_actions(new_rule));

     if (old_rule) {


More information about the dev mailing list