bridge: Avoid redundant dpif_flow_flush().
[openvswitch] / vswitchd / bridge.c
index 22af407b6af0cc3a8271f1fe265a38ebcb4ee8e6..44474966ead69747977ec55e387533767017c19f 100644 (file)
@@ -114,9 +114,6 @@ struct iface {
     bool up;                    /* Is the interface up? */
     const char *type;           /* Usually same as cfg->type. */
     const struct ovsrec_interface *cfg;
-
-    /* LACP information. */
-    uint16_t lacp_priority;     /* LACP port priority. */
 };
 
 #define BOND_MASK 0xff
@@ -184,9 +181,6 @@ struct port {
 
     /* LACP information. */
     struct lacp *lacp;          /* LACP object. NULL if LACP is disabled. */
-    bool lacp_active;           /* True if LACP is active */
-    bool lacp_fast;             /* True if LACP is in fast mode. */
-    uint16_t lacp_priority;     /* LACP system priority. */
 
     /* SLB specific bonding info. */
     struct bond_entry *bond_hash; /* An array of (BOND_MASK + 1) elements. */
@@ -374,7 +368,7 @@ static void
 bridge_configure_once(const struct ovsrec_open_vswitch *cfg)
 {
     static bool already_configured_once;
-    struct svec bridge_names;
+    struct sset bridge_names;
     struct sset dpif_names, dpif_types;
     const char *type;
     size_t i;
@@ -388,11 +382,10 @@ bridge_configure_once(const struct ovsrec_open_vswitch *cfg)
     stats_timer = time_msec() + STATS_INTERVAL;
 
     /* Get all the configured bridges' names from 'cfg' into 'bridge_names'. */
-    svec_init(&bridge_names);
+    sset_init(&bridge_names);
     for (i = 0; i < cfg->n_bridges; i++) {
-        svec_add(&bridge_names, cfg->bridges[i]->name);
+        sset_add(&bridge_names, cfg->bridges[i]->name);
     }
-    svec_sort(&bridge_names);
 
     /* Iterate over all system dpifs and delete any of them that do not appear
      * in 'cfg'. */
@@ -406,7 +399,7 @@ bridge_configure_once(const struct ovsrec_open_vswitch *cfg)
 
         /* Delete each dpif whose name is not in 'bridge_names'. */
         SSET_FOR_EACH (name, &dpif_names) {
-            if (!svec_contains(&bridge_names, name)) {
+            if (!sset_contains(&bridge_names, name)) {
                 struct dpif *dpif;
                 int retval;
 
@@ -418,7 +411,7 @@ bridge_configure_once(const struct ovsrec_open_vswitch *cfg)
             }
         }
     }
-    svec_destroy(&bridge_names);
+    sset_destroy(&bridge_names);
     sset_destroy(&dpif_names);
     sset_destroy(&dpif_types);
 }
@@ -820,12 +813,14 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
                 }
             }
 
-            opts.collectors.n = nf_cfg->n_targets;
-            opts.collectors.names = nf_cfg->targets;
+            sset_init(&opts.collectors);
+            sset_add_array(&opts.collectors,
+                           nf_cfg->targets, nf_cfg->n_targets);
             if (ofproto_set_netflow(br->ofproto, &opts)) {
                 VLOG_ERR("bridge %s: problem setting netflow collectors",
                          br->name);
             }
+            sset_destroy(&opts.collectors);
         } else {
             ofproto_set_netflow(br->ofproto, NULL);
         }
@@ -839,8 +834,9 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
 
             memset(&oso, 0, sizeof oso);
 
-            oso.targets.n = sflow_cfg->n_targets;
-            oso.targets.names = sflow_cfg->targets;
+            sset_init(&oso.targets);
+            sset_add_array(&oso.targets,
+                           sflow_cfg->targets, sflow_cfg->n_targets);
 
             oso.sampling_rate = SFL_DEFAULT_SAMPLING_RATE;
             if (sflow_cfg->sampling) {
@@ -870,7 +866,7 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
             }
             ofproto_set_sflow(br->ofproto, &oso);
 
-            /* Do not destroy oso.targets because it is owned by sflow_cfg. */
+            sset_destroy(&oso.targets);
         } else {
             ofproto_set_sflow(br->ofproto, NULL);
         }
@@ -1648,7 +1644,6 @@ bridge_create(const struct ovsrec_bridge *br_cfg)
         free(br);
         return NULL;
     }
-    dpif_flow_flush(br->dpif);
 
     error = ofproto_create(br_cfg->name, br_cfg->datapath_type, &bridge_ofhooks,
                            br, &br->ofproto);
@@ -1812,7 +1807,6 @@ static void
 bridge_reconfigure_one(struct bridge *br)
 {
     enum ofproto_fail_mode fail_mode;
-    struct svec snoops, old_snoops;
     struct port *port, *next;
     struct shash_node *node;
     struct shash new_ports;
@@ -1892,16 +1886,15 @@ bridge_reconfigure_one(struct bridge *br)
      * controller to another?) */
 
     /* Configure OpenFlow controller connection snooping. */
-    svec_init(&snoops);
-    svec_add_nocopy(&snoops, xasprintf("punix:%s/%s.snoop",
-                                       ovs_rundir(), br->name));
-    svec_init(&old_snoops);
-    ofproto_get_snoops(br->ofproto, &old_snoops);
-    if (!svec_equal(&snoops, &old_snoops)) {
+    if (!ofproto_has_snoops(br->ofproto)) {
+        struct sset snoops;
+
+        sset_init(&snoops);
+        sset_add_and_free(&snoops, xasprintf("punix:%s/%s.snoop",
+                                             ovs_rundir(), br->name));
         ofproto_set_snoops(br->ofproto, &snoops);
+        sset_destroy(&snoops);
     }
-    svec_destroy(&snoops);
-    svec_destroy(&old_snoops);
 
     mirror_reconfigure(br);
 }
@@ -3529,7 +3522,7 @@ bond_unixctl_show(struct unixctl_conn *conn,
 
     if (port->lacp) {
         ds_put_format(&ds, "lacp: %s\n",
-                      port->lacp_active ? "active" : "passive");
+                      lacp_is_active(port->lacp) ? "active" : "passive");
     } else {
         ds_put_cstr(&ds, "lacp: off\n");
     }
@@ -3833,8 +3826,8 @@ lacp_send_pdu_cb(void *aux, const struct lacp_pdu *pdu)
         struct lacp_pdu *packet_pdu;
 
         ofpbuf_init(&packet, 0);
-        packet_pdu = compose_packet(&packet, eth_addr_lacp, ea, ETH_TYPE_LACP,
-                                    sizeof *packet_pdu);
+        packet_pdu = eth_compose(&packet, eth_addr_lacp, ea, ETH_TYPE_LACP,
+                                 sizeof *packet_pdu);
         memcpy(packet_pdu, pdu, sizeof *packet_pdu);
         ofproto_send_packet(iface->port->bridge->ofproto,
                             iface->dp_ifidx, 0, &packet);
@@ -3989,7 +3982,7 @@ port_reconfigure(struct port *port, const struct ovsrec_port *cfg)
 {
     const char *detect_mode;
     struct sset new_ifaces;
-    long long int next_rebalance, miimon_next_update, lacp_priority;
+    long long int next_rebalance, miimon_next_update;
     bool need_flush = false;
     unsigned long *trunks;
     int vlan;
@@ -4087,57 +4080,9 @@ port_reconfigure(struct port *port, const struct ovsrec_port *cfg)
         iface->type = (!strcmp(if_cfg->name, port->bridge->name) ? "internal"
                        : if_cfg->type[0] ? if_cfg->type
                        : "system");
-
-        lacp_priority =
-            atoi(get_interface_other_config(if_cfg, "lacp-port-priority",
-                                            "0"));
-
-        if (lacp_priority <= 0 || lacp_priority > UINT16_MAX) {
-            iface->lacp_priority = UINT16_MAX;
-        } else {
-            iface->lacp_priority = lacp_priority;
-        }
     }
     sset_destroy(&new_ifaces);
 
-    port->lacp_fast = !strcmp(get_port_other_config(cfg, "lacp-time", "slow"),
-                             "fast");
-
-    lacp_priority =
-        atoi(get_port_other_config(cfg, "lacp-system-priority", "0"));
-
-    if (lacp_priority <= 0 || lacp_priority > UINT16_MAX) {
-        /* Prefer bondable links if unspecified. */
-        port->lacp_priority = port->n_ifaces > 1 ? UINT16_MAX - 1 : UINT16_MAX;
-    } else {
-        port->lacp_priority = lacp_priority;
-    }
-
-    if (!port->cfg->lacp) {
-        /* XXX when LACP implementation has been sufficiently tested, enable by
-         * default and make active on bonded ports. */
-        lacp_destroy(port->lacp);
-        port->lacp = NULL;
-    } else if (!strcmp(port->cfg->lacp, "off")) {
-        lacp_destroy(port->lacp);
-        port->lacp = NULL;
-    } else if (!strcmp(port->cfg->lacp, "active")) {
-        if (!port->lacp) {
-            port->lacp = lacp_create();
-        }
-        port->lacp_active = true;
-    } else if (!strcmp(port->cfg->lacp, "passive")) {
-        if (!port->lacp) {
-            port->lacp = lacp_create();
-        }
-        port->lacp_active = false;
-    } else {
-        VLOG_WARN("port %s: unknown LACP mode %s",
-                  port->name, port->cfg->lacp);
-        lacp_destroy(port->lacp);
-        port->lacp = NULL;
-    }
-
     /* Get VLAN tag. */
     vlan = -1;
     if (cfg->tag) {
@@ -4264,20 +4209,75 @@ port_lookup_iface(const struct port *port, const char *name)
     return iface && iface->port == port ? iface : NULL;
 }
 
+static bool
+enable_lacp(struct port *port, bool *activep)
+{
+    if (!port->cfg->lacp) {
+        /* XXX when LACP implementation has been sufficiently tested, enable by
+         * default and make active on bonded ports. */
+        return false;
+    } else if (!strcmp(port->cfg->lacp, "off")) {
+        return false;
+    } else if (!strcmp(port->cfg->lacp, "active")) {
+        *activep = true;
+        return true;
+    } else if (!strcmp(port->cfg->lacp, "passive")) {
+        *activep = false;
+        return true;
+    } else {
+        VLOG_WARN("port %s: unknown LACP mode %s",
+                  port->name, port->cfg->lacp);
+        return false;
+    }
+}
+
+static void
+iface_update_lacp(struct iface *iface)
+{
+    struct lacp_slave_settings s;
+    int priority;
+
+    s.name = iface->name;
+    s.id = iface->dp_ifidx;
+    priority = atoi(get_interface_other_config(
+                        iface->cfg, "lacp-port-priority", "0"));
+    s.priority = (priority >= 0 && priority <= UINT16_MAX ? priority
+                  : UINT16_MAX);
+
+    lacp_slave_register(iface->port->lacp, iface, &s);
+}
+
 static void
 port_update_lacp(struct port *port)
 {
-    if (port->lacp) {
-        struct iface *iface;
+    struct lacp_settings s;
+    struct iface *iface;
 
-        lacp_configure(port->lacp, port->name,
-                       port->bridge->ea, port->lacp_priority,
-                       port->lacp_active, port->lacp_fast);
+    if (!enable_lacp(port, &s.active)) {
+        lacp_destroy(port->lacp);
+        port->lacp = NULL;
+        return;
+    }
 
-        LIST_FOR_EACH (iface, port_elem, &port->ifaces) {
-            lacp_slave_register(port->lacp, iface, iface->name,
-                                iface->dp_ifidx, iface->lacp_priority);
-        }
+    if (!port->lacp) {
+        port->lacp = lacp_create();
+    }
+
+    s.name = port->name;
+    memcpy(s.id, port->bridge->ea, ETH_ADDR_LEN);
+    s.priority = atoi(get_port_other_config(port->cfg, "lacp-system-priority",
+                                          "0"));
+    s.fast = !strcmp(get_port_other_config(port->cfg, "lacp-time", "slow"),
+                     "fast");
+
+    if (s.priority <= 0 || s.priority > UINT16_MAX) {
+        /* Prefer bondable links if unspecified. */
+        s.priority = UINT16_MAX - (port->n_ifaces > 1);
+    }
+
+    lacp_configure(port->lacp, &s);
+    LIST_FOR_EACH (iface, port_elem, &port->ifaces) {
+        iface_update_lacp(iface);
     }
 }