diff mbox

[ovs-dev] bridge: open_type should be used for netdev_open

Message ID 1467656396-20760-1-git-send-email-cascardo@redhat.com
State Accepted
Headers show

Commit Message

Thadeu Lima de Souza Cascardo July 4, 2016, 6:19 p.m. UTC
ofproto_port_open_type should be used for netdev_open, but not for other tests.
For example, STP/RSTP check for interfaces of internal type, but that check will
fail when the netdev datapath is used.

The same thing goes for setting MAC address of internal Interfaces. That fails
for the netdev datapath because the interface type is set to "tap", but they are
still interfaces of type "internal", just their netdev implementation is
different.

Use a netdev_type for the type that needs to be used for netdev_open and
ofproto_port, while we still keep the type as the normalized configured type in
the database.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@redhat.com>
---
 tests/automake.mk    |  1 +
 tests/netdev-type.at | 24 ++++++++++++++++++++++++
 tests/testsuite.at   |  1 +
 vswitchd/bridge.c    | 16 ++++++++++++----
 4 files changed, 38 insertions(+), 4 deletions(-)
 create mode 100644 tests/netdev-type.at

Comments

Jesse Gross July 6, 2016, 4:08 p.m. UTC | #1
On Mon, Jul 4, 2016 at 11:19 AM, Thadeu Lima de Souza Cascardo
<cascardo@redhat.com> wrote:
> ofproto_port_open_type should be used for netdev_open, but not for other tests.
> For example, STP/RSTP check for interfaces of internal type, but that check will
> fail when the netdev datapath is used.
>
> The same thing goes for setting MAC address of internal Interfaces. That fails
> for the netdev datapath because the interface type is set to "tap", but they are
> still interfaces of type "internal", just their netdev implementation is
> different.
>
> Use a netdev_type for the type that needs to be used for netdev_open and
> ofproto_port, while we still keep the type as the normalized configured type in
> the database.
>
> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@redhat.com>

Applied to master (and backported to branch-2.5). Thank you -
especially for writing new tests.
diff mbox

Patch

diff --git a/tests/automake.mk b/tests/automake.mk
index 8b24221..bdf6828 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -60,6 +60,7 @@  TESTSUITE_AT = \
 	tests/ofproto-dpif.at \
 	tests/bridge.at \
 	tests/ofproto.at \
+	tests/netdev-type.at \
 	tests/ovsdb.at \
 	tests/ovsdb-log.at \
 	tests/ovsdb-types.at \
diff --git a/tests/netdev-type.at b/tests/netdev-type.at
new file mode 100644
index 0000000..184031b
--- /dev/null
+++ b/tests/netdev-type.at
@@ -0,0 +1,24 @@ 
+AT_BANNER([netdev-type])
+
+dnl Setting MAC address of netdev internal port fails
+AT_SETUP([bridge - set MAC address of internal port])
+OVS_VSWITCHD_START
+
+# Add an internal port and make sure that it shows up in the datapath.
+add_of_ports br0 1
+AT_CHECK([ovs-appctl dpif/show], [0], [dnl
+dummy@ovs-dummy: hit:0 missed:0
+	br0:
+		br0 65534/100: (dummy)
+		p1 1/1: (dummy)
+])
+#
+# Set MAC address of dummy device and check that it has been set
+AT_CHECK([ovs-vsctl set Interface p1 type=internal mac=\"aa:55:c0:ff:ee:00\"])
+AT_CHECK([ovs-vsctl get Interface p1 mac_in_use], [0], [dnl
+"aa:55:c0:ff:ee:00"
+])
+
+OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+AT_CLEANUP
diff --git a/tests/testsuite.at b/tests/testsuite.at
index 6b3fb25..f5f1253 100644
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -60,6 +60,7 @@  m4_include([tests/pmd.at])
 m4_include([tests/dpctl.at])
 m4_include([tests/ofproto-dpif.at])
 m4_include([tests/bridge.at])
+m4_include([tests/netdev-type.at])
 m4_include([tests/ovsdb.at])
 m4_include([tests/ovs-vsctl.at])
 m4_include([tests/ovs-monitor-ipsec.at])
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 52adc18..56ff7b4 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -89,6 +89,7 @@  struct iface {
 
     /* These members are valid only within bridge_reconfigure(). */
     const char *type;           /* Usually same as cfg->type. */
+    const char *netdev_type;    /* type that should be used for netdev_open. */
     const struct ovsrec_interface *cfg;
 };
 
@@ -765,7 +766,7 @@  bridge_delete_or_reconfigure_ports(struct bridge *br)
             goto delete;
         }
 
-        if (strcmp(ofproto_port.type, iface->type)
+        if (strcmp(ofproto_port.type, iface->netdev_type)
             || netdev_set_config(iface->netdev, &iface->cfg->options, NULL)) {
             /* The interface is the wrong type or can't be configured.
              * Delete it. */
@@ -1739,6 +1740,7 @@  iface_do_create(const struct bridge *br,
 {
     struct netdev *netdev = NULL;
     int error;
+    const char *type;
 
     if (netdev_is_reserved_name(iface_cfg->name)) {
         VLOG_WARN("could not create interface %s, name is reserved",
@@ -1747,8 +1749,9 @@  iface_do_create(const struct bridge *br,
         goto error;
     }
 
-    error = netdev_open(iface_cfg->name,
-                        iface_get_type(iface_cfg, br->cfg), &netdev);
+    type = ofproto_port_open_type(br->cfg->datapath_type,
+                                  iface_get_type(iface_cfg, br->cfg));
+    error = netdev_open(iface_cfg->name, type, &netdev);
     if (error) {
         VLOG_WARN_BUF(errp, "could not open network device %s (%s)",
                       iface_cfg->name, ovs_strerror(error));
@@ -1820,6 +1823,8 @@  iface_create(struct bridge *br, const struct ovsrec_interface *iface_cfg,
     iface->ofp_port = ofp_port;
     iface->netdev = netdev;
     iface->type = iface_get_type(iface_cfg, br->cfg);
+    iface->netdev_type = ofproto_port_open_type(br->cfg->datapath_type,
+                                                iface->type);
     iface->cfg = iface_cfg;
     hmap_insert(&br->ifaces, &iface->ofp_port_node,
                 hash_ofp_port(ofp_port));
@@ -3399,10 +3404,13 @@  bridge_del_ports(struct bridge *br, const struct shash *wanted_ports)
             const struct ovsrec_interface *cfg = port->interfaces[i];
             struct iface *iface = iface_lookup(br, cfg->name);
             const char *type = iface_get_type(cfg, br->cfg);
+            const char *dp_type = br->cfg->datapath_type;
+            const char *netdev_type = ofproto_port_open_type(dp_type, type);
 
             if (iface) {
                 iface->cfg = cfg;
                 iface->type = type;
+                iface->netdev_type = netdev_type;
             } else if (!strcmp(type, "null")) {
                 VLOG_WARN_ONCE("%s: The null interface type is deprecated and"
                                " may be removed in February 2013. Please email"
@@ -4282,7 +4290,7 @@  iface_get_type(const struct ovsrec_interface *iface,
         type = iface->type[0] ? iface->type : "system";
     }
 
-    return ofproto_port_open_type(br->datapath_type, type);
+    return type;
 }
 
 static void