[ovs-dev] [VLAN splinters 16/16] Implement new "VLAN splinters" feature.

Ben Pfaff blp at nicira.com
Wed Nov 23 23:07:36 UTC 2011


Here's an incremental I applied.  It includes the changes you
suggested plus makes sure that VLAN splinters update as flow tables
match on more VLANs.

I don't really expect a review on this (I'm going to work on unit
tests now) but certainly one would be welcome if you have time.

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 6ae9b87..96bd764 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -5780,6 +5780,7 @@ ofproto_dpif_unixctl_init(void)
 static int
 set_realdev(struct ofport *ofport_, uint16_t realdev_ofp_port, int vid)
 {
+    struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofport_->ofproto);
     struct ofport_dpif *ofport = ofport_dpif_cast(ofport_);
 
     if (realdev_ofp_port == ofport->realdev_ofp_port
@@ -5787,10 +5788,14 @@ set_realdev(struct ofport *ofport_, uint16_t realdev_ofp_port, int vid)
         return 0;
     }
 
+    ofproto->need_revalidate = true;
+
     if (ofport->realdev_ofp_port) {
         vsp_remove(ofport);
     }
-    if (ofport->bundle) {
+    if (realdev_ofp_port && ofport->bundle) {
+        /* vlandevs are enslaved to their realdevs, so they are not allowed to
+         * themselves be part of a bundle. */
         bundle_set(ofport->up.ofproto, ofport->bundle, NULL);
     }
 
@@ -5812,7 +5817,7 @@ hash_realdev_vid(uint16_t realdev_ofp_port, int vid)
 
 static uint32_t
 vsp_realdev_to_vlandev(const struct ofproto_dpif *ofproto,
-                   uint32_t realdev_odp_port, ovs_be16 vlan_tci)
+                       uint32_t realdev_odp_port, ovs_be16 vlan_tci)
 {
     if (!hmap_is_empty(&ofproto->realdev_vid_map)) {
         uint16_t realdev_ofp_port = odp_port_to_ofp_port(realdev_odp_port);
@@ -5834,12 +5839,12 @@ vsp_realdev_to_vlandev(const struct ofproto_dpif *ofproto,
 static struct vlan_splinter *
 vlandev_find(const struct ofproto_dpif *ofproto, uint16_t vlandev_ofp_port)
 {
-    const struct vlan_splinter *vsp;
+    struct vlan_splinter *vsp;
 
     HMAP_FOR_EACH_WITH_HASH (vsp, vlandev_node, hash_int(vlandev_ofp_port, 0),
                              &ofproto->vlandev_map) {
         if (vsp->vlandev_ofp_port == vlandev_ofp_port) {
-            return (struct vlan_splinter *) vsp;
+            return vsp;
         }
     }
 
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 46b2af3..558b871 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -69,6 +69,15 @@ struct ofproto {
     struct list pending;        /* List of "struct ofopgroup"s. */
     unsigned int n_pending;     /* list_size(&pending). */
     struct hmap deletions;      /* All OFOPERATION_DELETE "ofoperation"s. */
+
+    /* Linux VLAN device support (e.g. "eth0.10" for VLAN 10.)
+     *
+     * This is deprecated.  It is only for compatibility with broken device
+     * drivers in old versions of Linux that do not properly support VLANs when
+     * VLAN devices are not used.  When broken device drivers are no longer in
+     * widespread use, we will delete these interfaces. */
+    unsigned long int *vlan_bitmap; /* 4096-bit bitmap of in-use VLANs. */
+    bool vlans_changed;             /* True if new VLANs are in use. */
 };
 
 struct ofproto *ofproto_lookup(const char *name);
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 8bdcb46..1a20097 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -338,6 +338,8 @@ ofproto_create(const char *datapath_name, const char *datapath_type,
     list_init(&ofproto->pending);
     ofproto->n_pending = 0;
     hmap_init(&ofproto->deletions);
+    ofproto->vlan_bitmap = NULL;
+    ofproto->vlans_changed = false;
 
     error = ofproto->ofproto_class->construct(ofproto, &n_tables);
     if (error) {
@@ -835,6 +837,8 @@ ofproto_destroy__(struct ofproto *ofproto)
 
     hmap_destroy(&ofproto->deletions);
 
+    free(ofproto->vlan_bitmap);
+
     ofproto->ofproto_class->dealloc(ofproto);
 }
 
@@ -3102,7 +3106,8 @@ ofoperation_complete(struct ofoperation *op, int error)
 {
     struct ofopgroup *group = op->group;
     struct rule *rule = op->rule;
-    struct classifier *table = &rule->ofproto->tables[rule->table_id];
+    struct ofproto *ofproto = rule->ofproto;
+    struct classifier *table = &ofproto->tables[rule->table_id];
 
     assert(rule->pending == op);
     assert(op->status < 0);
@@ -3134,6 +3139,15 @@ ofoperation_complete(struct ofoperation *op, int error)
             if (op->victim) {
                 ofproto_rule_destroy__(op->victim);
             }
+            if (!(rule->cr.wc.vlan_tci_mask & htons(VLAN_VID_MASK))
+                && ofproto->vlan_bitmap) {
+                uint16_t vid = vlan_tci_to_vid(rule->cr.flow.vlan_tci);
+
+                if (!bitmap_is_set(ofproto->vlan_bitmap, vid)) {
+                    bitmap_set1(ofproto->vlan_bitmap, vid);
+                    ofproto->vlans_changed = true;
+                }
+            }
         } else {
             if (op->victim) {
                 classifier_replace(table, &op->victim->cr);
@@ -3254,12 +3268,17 @@ ofproto_unixctl_init(void)
  * devices are not used.  When broken device drivers are no longer in
  * widespread use, we will delete these interfaces. */
 
+/* Sets a 1-bit in the 4096-bit 'vlan_bitmap' for each VLAN ID that is matched
+ * (exactly) by an OpenFlow rule in 'ofproto'. */
 void
-ofproto_get_vlan_usage(const struct ofproto *ofproto,
-                       unsigned long int *vlan_bitmap)
+ofproto_get_vlan_usage(struct ofproto *ofproto, unsigned long int *vlan_bitmap)
 {
     const struct classifier *cls;
 
+    free(ofproto->vlan_bitmap);
+    ofproto->vlan_bitmap = bitmap_allocate(4096);
+    ofproto->vlans_changed = false;
+
     OFPROTO_FOR_EACH_TABLE (cls, ofproto) {
         const struct cls_table *table;
 
@@ -3270,12 +3289,28 @@ ofproto_get_vlan_usage(const struct ofproto *ofproto,
                 HMAP_FOR_EACH (rule, hmap_node, &table->rules) {
                     uint16_t vid = vlan_tci_to_vid(rule->flow.vlan_tci);
                     bitmap_set1(vlan_bitmap, vid);
+                    bitmap_set1(ofproto->vlan_bitmap, vid);
                 }
             }
         }
     }
 }
 
+/* Returns true if new VLANs have come into use by the flow table since the
+ * last call to ofproto_get_vlan_usage().
+ *
+ * We don't track when old VLANs stop being used. */
+bool
+ofproto_has_vlan_usage_changed(const struct ofproto *ofproto)
+{
+    return ofproto->vlans_changed;
+}
+
+/* Configures a VLAN splinter binding between the ports identified by OpenFlow
+ * port numbers 'vlandev_ofp_port' and 'realdev_ofp_port'.  If
+ * 'realdev_ofp_port' is nonzero, then the VLAN device is enslaved to the real
+ * device as a VLAN splinter for VLAN ID 'vid'.  If 'realdev_ofp_port' is zero,
+ * then the VLAN device is un-enslaved. */
 int
 ofproto_port_set_realdev(struct ofproto *ofproto, uint16_t vlandev_ofp_port,
                          uint16_t realdev_ofp_port, int vid)
@@ -3283,6 +3318,8 @@ ofproto_port_set_realdev(struct ofproto *ofproto, uint16_t vlandev_ofp_port,
     struct ofport *ofport;
     int error;
 
+    assert(vlandev_ofp_port != realdev_ofp_port);
+
     ofport = ofproto_get_port(ofproto, vlandev_ofp_port);
     if (!ofport) {
         VLOG_WARN("%s: cannot set realdev on nonexistent port %"PRIu16,
diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index 05164a5..4999c82 100644
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -329,8 +329,8 @@ void ofproto_free_ofproto_controller_info(struct shash *);
  * devices are not used.  When broken device drivers are no longer in
  * widespread use, we will delete these interfaces. */
 
-void ofproto_get_vlan_usage(const struct ofproto *,
-                            unsigned long int *vlan_bitmap);
+void ofproto_get_vlan_usage(struct ofproto *, unsigned long int *vlan_bitmap);
+bool ofproto_has_vlan_usage_changed(const struct ofproto *);
 int ofproto_port_set_realdev(struct ofproto *, uint16_t vlandev_ofp_port,
                              uint16_t realdev_ofp_port, int vid);
 
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index ceab835..32885eb 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -226,7 +226,11 @@ static void shash_to_ovs_idl_map(struct shash *,
  * in old versions of Linux that do not properly support VLANs when VLAN
  * devices are not used.  When broken device drivers are no longer in
  * widespread use, we will delete these interfaces. */
-static bool enable_vlan_splinters(const struct ovsrec_interface *);
+
+/* True if VLAN splinters are enabled on any interface, false otherwise.*/
+static bool vlan_splinters_enabled_anywhere;
+
+static bool vlan_splinters_is_enabled(const struct ovsrec_interface *);
 static unsigned long int *collect_splinter_vlans(
     const struct ovsrec_open_vswitch *);
 static void configure_splinter_port(struct port *);
@@ -1780,6 +1784,7 @@ bridge_run(void)
 {
     const struct ovsrec_open_vswitch *cfg;
 
+    bool vlan_splinters_changed;
     bool datapath_destroyed;
     bool database_changed;
     struct bridge *br;
@@ -1827,7 +1832,19 @@ bridge_run(void)
         stream_ssl_set_ca_cert_file(ssl->ca_cert, ssl->bootstrap_ca_cert);
     }
 
-    if (database_changed || datapath_destroyed) {
+    /* If VLAN splinters are in use, then we need to reconfigure if VLAN usage
+     * has changed. */
+    vlan_splinters_changed = false;
+    if (vlan_splinters_enabled_anywhere) {
+        HMAP_FOR_EACH (br, node, &all_bridges) {
+            if (ofproto_has_vlan_usage_changed(br->ofproto)) {
+                vlan_splinters_changed = true;
+                break;
+            }
+        }
+    }
+
+    if (database_changed || datapath_destroyed || vlan_splinters_changed) {
         if (cfg) {
             struct ovsdb_idl_txn *txn = ovsdb_idl_txn_create(idl);
 
@@ -3308,7 +3325,7 @@ free_registered_blocks(void)
 /* Returns true if VLAN splinters are enabled on 'iface_cfg', false
  * otherwise. */
 static bool
-enable_vlan_splinters(const struct ovsrec_interface *iface_cfg)
+vlan_splinters_is_enabled(const struct ovsrec_interface *iface_cfg)
 {
     const char *value;
 
@@ -3325,7 +3342,9 @@ enable_vlan_splinters(const struct ovsrec_interface *iface_cfg)
  * with free().
  *
  * If VLANs splinters are not enabled on any interface or if no VLANs are in
- * use, returns NULL. */
+ * use, returns NULL.
+ *
+ * Updates 'vlan_splinters_enabled_anywhere'. */
 static unsigned long int *
 collect_splinter_vlans(const struct ovsrec_open_vswitch *ovs_cfg)
 {
@@ -3350,7 +3369,7 @@ collect_splinter_vlans(const struct ovsrec_open_vswitch *ovs_cfg)
             for (k = 0; k < port_cfg->n_interfaces; k++) {
                 struct ovsrec_interface *iface_cfg = port_cfg->interfaces[k];
 
-                if (enable_vlan_splinters(iface_cfg)) {
+                if (vlan_splinters_is_enabled(iface_cfg)) {
                     sset_add(&splinter_ifaces, iface_cfg->name);
 
                     if (!splinter_vlans) {
@@ -3363,6 +3382,8 @@ collect_splinter_vlans(const struct ovsrec_open_vswitch *ovs_cfg)
             }
         }
     }
+
+    vlan_splinters_enabled_anywhere = splinter_vlans != NULL;
     if (!splinter_vlans) {
         sset_destroy(&splinter_ifaces);
         return NULL;
@@ -3374,6 +3395,9 @@ collect_splinter_vlans(const struct ovsrec_open_vswitch *ovs_cfg)
         }
     }
 
+    /* Don't allow VLANs 0 or 4095 to be splintered.  VLAN 0 should appear on
+     * the real device.  VLAN 4095 is reserved and Linux doesn't allow a VLAN
+     * device to be created for it. */
     bitmap_set0(splinter_vlans, 0);
     bitmap_set0(splinter_vlans, 4095);
 
@@ -3393,7 +3417,8 @@ collect_splinter_vlans(const struct ovsrec_open_vswitch *ovs_cfg)
                 struct netdev *netdev;
 
                 if (!netdev_open(vlan_dev->name, "system", &netdev)) {
-                    if (!netdev_get_in4(netdev, NULL, NULL)) {
+                    if (!netdev_get_in4(netdev, NULL, NULL) ||
+                        !netdev_get_in6(netdev, NULL)) {
                         vlandev_del(vlan_dev->name);
                     } else {
                         /* It has an IP address configured, so we don't own
@@ -3510,7 +3535,7 @@ add_vlan_splinter_ports(struct bridge *br,
         for (j = 0; j < port_cfg->n_interfaces; j++) {
             struct ovsrec_interface *iface_cfg = port_cfg->interfaces[j];
 
-            if (enable_vlan_splinters(iface_cfg)) {
+            if (vlan_splinters_is_enabled(iface_cfg)) {
                 const char *real_dev_name;
                 uint16_t vid;
 



More information about the dev mailing list