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;
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
