[ovs-discuss] [abstract-netdev v2 10/21] vswitchd: Factor out iteration over interfaces with deletion.

[Ben Pfaff]

Justin Pettit <jpettit at nicira.com> writes:

> On Jul 28, 2009, at 2:13 PM, Ben Pfaff wrote:
>
>> +static void
>> +for_each_iface(struct bridge *br,
>> +               bool (*cb)(struct bridge *, struct iface *, void
>> *aux),
>> +               void *aux)
>
> This function's name sounds like a nice useful interface iterator, so
> I was surprised to see that if "cb" returns false that the interface
> is destroyed.  I realize the function is static, but it might be nice
> to provide a comment describing this behavior at the top.

Fair enough.

I renamed the function to iterate_and_prune_ifaces and added a
comment:

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 0e6a340..419f918 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -377,10 +377,14 @@ check_iface_dp_ifidx(struct bridge *br, struct iface *iface,
     }
 }
 
+/* Calls 'cb' for each interfaces in 'br', passing along the 'aux' argument.
+ * Deletes from 'br' all the interfaces for which 'cb' returns false, and then
+ * deletes from 'br' any ports that no longer have any interfaces. */
 static void
-for_each_iface(struct bridge *br,
-               bool (*cb)(struct bridge *, struct iface *, void *aux),
-               void *aux)
+iterate_and_prune_ifaces(struct bridge *br,
+                         bool (*cb)(struct bridge *, struct iface *,
+                                    void *aux),
+                         void *aux)
 {
     size_t i, j;
 
@@ -522,7 +526,7 @@ bridge_reconfigure(void)
         bridge_fetch_dp_ifaces(br);
 
         local_iface = NULL;
-        for_each_iface(br, check_iface_dp_ifidx, &local_iface);
+        iterate_and_prune_ifaces(br, check_iface_dp_ifidx, &local_iface);
 
         /* Pick local port hardware address, datapath ID. */
         bridge_pick_local_hw_addr(br, ea, &devname);
@@ -1046,7 +1050,7 @@ bridge_reconfigure_one(struct bridge *br)
 
     /* Check and delete duplicate interfaces. */
     svec_init(&ifaces);
-    for_each_iface(br, check_duplicate_ifaces, &ifaces);
+    iterate_and_prune_ifaces(br, check_duplicate_ifaces, &ifaces);
     svec_destroy(&ifaces);
 
     /* Delete all flows if we're switching from connected to standalone or vice