From 5b60e0b9256d0a19939da5fe88c58ecf1f765533 Mon Sep 17 00:00:00 2001
From: Ben Pfaff <blp@nicira.com>
Date: Tue, 17 Mar 2009 10:18:32 -0700
Subject: [PATCH] secchan: Centralize creation of rules in new function
 rule_create().

This is code cleanup that should make maintenance easier.
---
 secchan/ofproto.c | 73 +++++++++++++++++++++--------------------------
 1 file changed, 33 insertions(+), 40 deletions(-)

diff --git a/secchan/ofproto.c b/secchan/ofproto.c
index 682071e0..bc765e5f 100644
--- a/secchan/ofproto.c
+++ b/secchan/ofproto.c
@@ -128,6 +128,9 @@ struct rule {
     union ofp_action *actions;
 };
 
+static struct rule *rule_create(struct rule *super, const union ofp_action *,
+                                size_t n_actions, uint16_t idle_timeout,
+                                uint16_t hard_timeout);
 static void rule_free(struct rule *);
 static void rule_destroy(struct rule *);
 static struct rule *rule_from_cls_rule(const struct cls_rule *);
@@ -901,20 +904,9 @@ ofproto_add_flow(struct ofproto *p,
     struct odp_actions odp_actions;
     struct odp_flow odp_flow;
 
-    rule = xmalloc(sizeof *rule);
+    rule = rule_create(NULL, actions, n_actions,
+                       idle_timeout >= 0 ? idle_timeout : 5 /* XXX */, 0);
     cls_rule_from_flow(&rule->cr, flow, wildcards, priority);
-    rule->idle_timeout = idle_timeout >= 0 ? idle_timeout : 5; /* XXX */
-    rule->hard_timeout = 0;     /* XXX */
-    rule->used = rule->created = time_msec();
-    rule->packet_count = 0;
-    rule->byte_count = 0;
-    rule->tcp_flags = 0;
-    rule->ip_tos = 0;
-    rule->tags = 0;
-    rule->super = NULL;         /* XXX */
-    list_init(&rule->list);
-    rule->n_actions = n_actions;
-    rule->actions = xmemdup(actions, n_actions * sizeof *rule->actions);
 
     displaced_rule = rule_from_cls_rule(classifier_insert(&p->cls, &rule->cr));
     if (displaced_rule) {
@@ -1279,6 +1271,28 @@ ofconn_wait(struct ofconn *ofconn)
     rconn_recv_wait(ofconn->rconn);
 }
 
+/* Caller is responsible for initializing the 'cr' member of the returned
+ * rule. */
+static struct rule *
+rule_create(struct rule *super,
+            const union ofp_action *actions, size_t n_actions,
+            uint16_t idle_timeout, uint16_t hard_timeout)
+{
+    struct rule *rule = xcalloc(1, sizeof *rule);
+    rule->idle_timeout = idle_timeout;
+    rule->hard_timeout = hard_timeout;
+    rule->used = rule->created = time_msec();
+    rule->super = super;
+    if (super) {
+        list_push_back(&super->list, &rule->list);
+    } else {
+        list_init(&rule->list);
+    }
+    rule->n_actions = n_actions;
+    rule->actions = xmemdup(actions, n_actions * sizeof *actions);
+    return rule;
+}
+
 static struct rule *
 rule_from_cls_rule(const struct cls_rule *cls_rule)
 {
@@ -2259,20 +2273,10 @@ add_flow(struct ofproto *p, struct ofconn *ofconn,
     struct rule *rule, *displaced_rule;
     int error = 0;
 
-    rule = xmalloc(sizeof *rule);
+    rule = rule_create(NULL, (const union ofp_action *) ofm->actions,
+                       n_actions, ntohs(ofm->idle_timeout),
+                       ntohs(ofm->hard_timeout));
     cls_rule_from_match(&rule->cr, &ofm->match, ntohs(ofm->priority));
-    rule->idle_timeout = ntohs(ofm->idle_timeout);
-    rule->hard_timeout = ntohs(ofm->hard_timeout);
-    rule->used = rule->created = time_msec();
-    rule->packet_count = 0;
-    rule->byte_count = 0;
-    rule->tcp_flags = 0;
-    rule->ip_tos = 0;
-    rule->tags = 0;
-    rule->super = NULL;
-    list_init(&rule->list);
-    rule->n_actions = n_actions;
-    rule->actions = xmemdup(ofm->actions, n_actions * sizeof *rule->actions);
 
     if (ofm->buffer_id != htonl(UINT32_MAX)) {
         int byte_count = 0;
@@ -2702,25 +2706,16 @@ handle_odp_msg(struct ofproto *p, struct ofpbuf *packet)
         struct rule *old_sr;
         struct odp_flow odp_flow;
 
-        subrule = xmalloc(sizeof *subrule);
+        subrule = rule_create(rule, NULL, 0,
+                              rule->idle_timeout, rule->hard_timeout);
         cls_rule_from_flow(&subrule->cr, &flow, 0, 0);
-        subrule->idle_timeout = rule->idle_timeout;
-        subrule->hard_timeout = rule->hard_timeout;
-        subrule->used = subrule->created = time_msec();
-        subrule->packet_count = subrule->byte_count = 0;
-        subrule->tcp_flags = 0;
-        subrule->ip_tos = 0;
-        subrule->tags = 0;
-        subrule->super = rule;
-        subrule->n_actions = 0;
-        subrule->actions = NULL;
 
         old_sr = rule_from_cls_rule(classifier_insert(&p->cls, &subrule->cr));
         if (old_sr) {
             if (!old_sr->super) {
                 /* Put old_sr back. */
                 cls_rule_replace(&p->cls, &subrule->cr, &old_sr->cr);
-                free(subrule);
+                rule_destroy(subrule);
 
                 /* Execute old_sr on packet. */
                 rule_make_actions(p, old_sr, false, &actions);
@@ -2734,8 +2729,6 @@ handle_odp_msg(struct ofproto *p, struct ofpbuf *packet)
                 rule_destroy(old_sr);
             }
         }
-        list_push_back(&rule->list, &subrule->list);
-        rule->used = time_msec();
 
         /* Install flow entry into datapath. */
         rule_make_actions(p, subrule, false, &actions);
-- 
2.30.2