ofproto: Remove double reporting from bundles.

Patch b0d38b2 unified flow mod reporting in ofproto for both stand-alone flow mods and bundle flow mods, but left bundle-specific reporting to the bundle removal code. This patch fixes this by removing the bundle-specific reporting of flow mods. Found by inspection. Fixes: b0d38b2 ("ofproto: Report flow mods also from bundles.") Signed-off-by: Jarno Rajahalme <[email protected]> Acked-by: Ben Pfaff <[email protected]>
grizax · Sep 15, 2016 · 0c78eeb · 0c78eeb
1 parent f069903
commit 0c78eeb
Show file tree

Hide file tree

Showing 5 changed files with 18 additions and 17 deletions.
diff --git a/ofproto/bundles.c b/ofproto/bundles.c
@@ -62,16 +62,11 @@ ofp_bundle_create(uint32_t id, uint16_t flags, const struct ofp_header *oh)
 }
 
 void
-ofp_bundle_remove__(struct ofconn *ofconn, struct ofp_bundle *bundle,
-                    bool success)
+ofp_bundle_remove__(struct ofconn *ofconn, struct ofp_bundle *bundle)
 {
     struct ofp_bundle_entry *msg;
 
     LIST_FOR_EACH_POP (msg, node, &bundle->msg_list) {
-        if (success && msg->type == OFPTYPE_FLOW_MOD) {
-            /* Tell connmgr about successful flow mods. */
-            ofconn_report_flow_mod(ofconn, msg->ofm.command);
-        }
         ofp_bundle_entry_free(msg);
     }
 
@@ -90,7 +85,7 @@ ofp_bundle_open(struct ofconn *ofconn, uint32_t id, uint16_t flags,
 
     if (bundle) {
         VLOG_INFO("Bundle %x already exists.", id);
-        ofp_bundle_remove__(ofconn, bundle, false);
+        ofp_bundle_remove__(ofconn, bundle);
 
         return OFPERR_OFPBFC_BAD_ID;
     }
@@ -116,12 +111,12 @@ ofp_bundle_close(struct ofconn *ofconn, uint32_t id, uint16_t flags)
     }
 
     if (bundle->state == BS_CLOSED) {
-        ofp_bundle_remove__(ofconn, bundle, false);
+        ofp_bundle_remove__(ofconn, bundle);
         return OFPERR_OFPBFC_BUNDLE_CLOSED;
     }
 
     if (bundle->flags != flags) {
-        ofp_bundle_remove__(ofconn, bundle, false);
+        ofp_bundle_remove__(ofconn, bundle);
         return OFPERR_OFPBFC_BAD_FLAGS;
     }
 
@@ -141,8 +136,7 @@ ofp_bundle_discard(struct ofconn *ofconn, uint32_t id)
         return OFPERR_OFPBFC_BAD_ID;
     }
 
-    ofp_bundle_remove__(ofconn, bundle, false);
-
+    ofp_bundle_remove__(ofconn, bundle);
     return 0;
 }
 
@@ -165,10 +159,10 @@ ofp_bundle_add_message(struct ofconn *ofconn, uint32_t id, uint16_t flags,
             return error;
         }
     } else if (bundle->state == BS_CLOSED) {
-        ofp_bundle_remove__(ofconn, bundle, false);
+        ofp_bundle_remove__(ofconn, bundle);
         return OFPERR_OFPBFC_BUNDLE_CLOSED;
     } else if (flags != bundle->flags) {
-        ofp_bundle_remove__(ofconn, bundle, false);
+        ofp_bundle_remove__(ofconn, bundle);
         return OFPERR_OFPBFC_BAD_FLAGS;
     }
 

diff --git a/ofproto/bundles.h b/ofproto/bundles.h
@@ -83,7 +83,7 @@ enum ofperr ofp_bundle_add_message(struct ofconn *, uint32_t id,
                                    uint16_t flags, struct ofp_bundle_entry *,
                                    const struct ofp_header *);
 
-void ofp_bundle_remove__(struct ofconn *, struct ofp_bundle *, bool success);
+void ofp_bundle_remove__(struct ofconn *, struct ofp_bundle *);
 
 static inline struct ofp_bundle_entry *
 ofp_bundle_entry_alloc(enum ofptype type, const struct ofp_header *oh)

diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
@@ -1234,7 +1234,7 @@ bundle_remove_all(struct ofconn *ofconn)
     struct ofp_bundle *b, *next;
 
     HMAP_FOR_EACH_SAFE (b, next, node, &ofconn->bundles) {
-        ofp_bundle_remove__(ofconn, b, false);
+        ofp_bundle_remove__(ofconn, b);
     }
 }
 
@@ -1247,7 +1247,7 @@ bundle_remove_expired(struct ofconn *ofconn, long long int now)
     HMAP_FOR_EACH_SAFE (b, next, node, &ofconn->bundles) {
         if (b->used <= limit) {
             ofconn_send_error(ofconn, &b->ofp_msg, OFPERR_OFPBFC_TIMEOUT);
-            ofp_bundle_remove__(ofconn, b, false);
+            ofp_bundle_remove__(ofconn, b);
         }
     }
 }

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
@@ -7590,7 +7590,7 @@ do_bundle_commit(struct ofconn *ofconn, uint32_t id, uint16_t flags)
     }
 
     /* The bundle is discarded regardless the outcome. */
-    ofp_bundle_remove__(ofconn, bundle, !error);
+    ofp_bundle_remove__(ofconn, bundle);
     return error;
 }
 

diff --git a/tests/ofproto.at b/tests/ofproto.at
@@ -5008,6 +5008,13 @@ vconn|DBG|unix: sent (Success): NXST_FLOW reply:
  in_port=2,dl_src=00:66:77:88:99:aa actions=drop
 ])
 
+AT_CHECK([grep " flow_mods in the last " ovs-vswitchd.log | sed -e 's/^.*connmgr|INFO|//'], [0], [dnl
+br0<->unix: 1 flow_mods in the last 0 s (1 deletes)
+br0<->unix: 9 flow_mods in the last 0 s (7 adds, 2 deletes)
+br0<->unix: 2 flow_mods in the last 0 s (2 modifications)
+br0<->unix: 3 flow_mods in the last 0 s (2 adds, 1 deletes)
+])
+
 OVS_VSWITCHD_STOP
 AT_CLEANUP