From 4abb8608afb4000726adf9e3a09da26874880fe3 Mon Sep 17 00:00:00 2001
From: Ben Pfaff
Date: Wed, 11 Dec 2013 15:40:52 -0800
Subject: [PATCH] bridge: Support changing port numbers.
Feature #21293.
Requested-by: Anuprem Chalvadi
Signed-off-by: Ben Pfaff
---
NEWS | 3 ++
tests/ofproto.at | 55 ++++++++++++++++++++++++++
vswitchd/bridge.c | 91 +++++++++++++++++++++++++++++++++++++++-----
vswitchd/vswitch.xml | 21 ++++++----
4 files changed, 152 insertions(+), 18 deletions(-)
diff --git a/NEWS b/NEWS
index f654c4b0b..3b6958382 100644
--- a/NEWS
+++ b/NEWS
@@ -38,6 +38,9 @@ Post-v2.0.0
* OVS limits the OpenFlow port numbers it assigns to port 32767 and
below, leaving port numbers above that range free for assignment
by the controller.
+ * ovs-vswitchd now honors changes to the "ofport_request" column
+ in the Interface table by changing the port's OpenFlow port
+ number.
- ovs-vswitchd.conf.db.5 man page will contain graphviz/dot
diagram only if graphviz package was installed at the build time.
- Support for Linux kernels up to 3.11
diff --git a/tests/ofproto.at b/tests/ofproto.at
index be7387d33..f730465b2 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -2152,3 +2152,58 @@ OFPT_BARRIER_REPLY (OF1.3):
OVS_VSWITCHD_STOP
AT_CLEANUP
+
+AT_SETUP([ofproto - ofport_request])
+OVS_VSWITCHD_START
+ADD_OF_PORTS([br0], [1], [2], [3])
+
+set_and_check_specific_ofports () {
+ ovs-vsctl set Interface p1 ofport_request="$1" -- \
+ set Interface p2 ofport_request="$2" -- \
+ set Interface p3 ofport_request="$3"
+ ofports=`ovs-vsctl get Interface p1 ofport -- \
+ get Interface p2 ofport -- \
+ get Interface p3 ofport`
+ AT_CHECK_UNQUOTED([echo $ofports], [0], [$1 $2 $3
+])
+}
+for pre in '1 2 3' '1 3 2' '2 1 3' '2 3 1' '3 1 2' '3 2 1'; do
+ for post in '1 2 3' '1 3 2' '2 1 3' '2 3 1' '3 1 2' '3 2 1'; do
+ echo -----------------------------------------------------------
+ echo "Check changing port numbers from $pre to $post"
+ set_and_check_ofports $pre
+ set_and_check_ofports $post
+ done
+done
+
+ovs-vsctl del-port p3
+
+set_and_check_poorly_specified_ofports () {
+ ovs-vsctl set Interface p1 ofport_request="$1" -- \
+ set Interface p2 ofport_request="$2"
+ p1=`ovs-vsctl get Interface p1 ofport`
+ p2=`ovs-vsctl get Interface p2 ofport`
+ echo $p1 $p2
+
+ AT_CHECK([test "$p1" != "$p2"])
+ if test "$1" = "$2" && test "$1" != '[[]]'; then
+ # One port number must be the requested one.
+ AT_CHECK([test "$p1" = "$1" || test "$p2" = "$1"])
+ # The other port number must be different (already tested above).
+ else
+ AT_CHECK([test "$1" = '[[]]' || test "$p1" == "$1"])
+ AT_CHECK([test "$2" = '[[]]' || test "$p2" == "$2"])
+ fi
+}
+for pre in '1 2' '[[]] 2' '1 [[]]' '[[]] [[]]' '2 1' '[[]] 1' '2 [[]]' \
+ '1 1' '2 2'; do
+ for post in '1 2' '[[]] 2' '1 [[]]' '[[]] [[]]' '2 1' '[[]] 1' '2 [[]]' \
+ '1 1' '2 2'; do
+ echo -----------------------------------------------------------
+ echo "Check changing port numbers from $pre to $post"
+ set_and_check_poorly_specified_ofports $pre
+ set_and_check_poorly_specified_ofports $post
+ done
+done
+OVS_VSWITCHD_STOP
+AT_CLEANUP
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 4fe9d9654..f3b03af03 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -255,6 +255,8 @@ static void iface_refresh_cfm_stats(struct iface *);
static void iface_refresh_stats(struct iface *);
static void iface_refresh_status(struct iface *);
static bool iface_is_synthetic(const struct iface *);
+static ofp_port_t iface_get_requested_ofp_port(
+ const struct ovsrec_interface *);
static ofp_port_t iface_pick_ofport(const struct ovsrec_interface *);
/* Linux VLAN device support (e.g. "eth0.10" for VLAN 10.)
@@ -652,6 +654,7 @@ bridge_delete_or_reconfigure_ports(struct bridge *br)
n = allocated = 0;
OFPROTO_PORT_FOR_EACH (&ofproto_port, &dump, br->ofproto) {
+ ofp_port_t requested_ofp_port;
struct iface *iface;
iface = iface_lookup(br, ofproto_port.name);
@@ -675,6 +678,43 @@ bridge_delete_or_reconfigure_ports(struct bridge *br)
goto delete;
}
+ /* If the requested OpenFlow port for 'iface' changed, and it's not
+ * already the correct port, then we might want to temporarily delete
+ * this interface, so we can add it back again with the new OpenFlow
+ * port number. */
+ requested_ofp_port = iface_get_requested_ofp_port(iface->cfg);
+ if (iface->ofp_port != OFPP_LOCAL &&
+ requested_ofp_port != OFPP_NONE &&
+ requested_ofp_port != iface->ofp_port) {
+ ofp_port_t victim_request;
+ struct iface *victim;
+
+ /* Check for an existing OpenFlow port currently occupying
+ * 'iface''s requested port number. If there isn't one, then
+ * delete this port. Otherwise we need to consider further. */
+ victim = iface_from_ofp_port(br, requested_ofp_port);
+ if (!victim) {
+ goto delete;
+ }
+
+ /* 'victim' is a port currently using 'iface''s requested port
+ * number. Unless 'victim' specifically requested that port
+ * number, too, then we can delete both 'iface' and 'victim'
+ * temporarily. (We'll add both of them back again later with new
+ * OpenFlow port numbers.)
+ *
+ * If 'victim' did request port number 'requested_ofp_port', just
+ * like 'iface', then that's a configuration inconsistency that we
+ * can't resolve. We might as well let it keep its current port
+ * number. */
+ victim_request = iface_get_requested_ofp_port(victim->cfg);
+ if (victim_request != requested_ofp_port) {
+ del = add_ofp_port(victim->ofp_port, del, &n, &allocated);
+ iface_destroy(victim);
+ goto delete;
+ }
+ }
+
/* Keep it. */
continue;
@@ -690,7 +730,8 @@ bridge_delete_or_reconfigure_ports(struct bridge *br)
}
static void
-bridge_add_ports(struct bridge *br, const struct shash *wanted_ports)
+bridge_add_ports__(struct bridge *br, const struct shash *wanted_ports,
+ bool with_requested_port)
{
struct shash_node *port_node;
@@ -700,15 +741,32 @@ bridge_add_ports(struct bridge *br, const struct shash *wanted_ports)
for (i = 0; i < port_cfg->n_interfaces; i++) {
const struct ovsrec_interface *iface_cfg = port_cfg->interfaces[i];
- struct iface *iface = iface_lookup(br, iface_cfg->name);
+ ofp_port_t requested_ofp_port;
+
+ requested_ofp_port = iface_get_requested_ofp_port(iface_cfg);
+ if ((requested_ofp_port != OFPP_NONE) == with_requested_port) {
+ struct iface *iface = iface_lookup(br, iface_cfg->name);
- if (!iface) {
- iface_create(br, iface_cfg, port_cfg);
+ if (!iface) {
+ iface_create(br, iface_cfg, port_cfg);
+ }
}
}
}
}
+static void
+bridge_add_ports(struct bridge *br, const struct shash *wanted_ports)
+{
+ /* First add interfaces that request a particular port number. */
+ bridge_add_ports__(br, wanted_ports, true);
+
+ /* Then add interfaces that want automatic port number assignment.
+ * We add these afterward to avoid accidentally taking a specifically
+ * requested port number. */
+ bridge_add_ports__(br, wanted_ports, false);
+}
+
static void
port_configure(struct port *port)
{
@@ -1458,7 +1516,7 @@ iface_create(struct bridge *br, const struct ovsrec_interface *iface_cfg,
error = netdev_open(port->name, "internal", &netdev);
if (!error) {
- ofp_port_t fake_ofp_port = iface_pick_ofport(iface_cfg);
+ ofp_port_t fake_ofp_port = OFPP_NONE;
ofproto_port_add(br->ofproto, netdev, &fake_ofp_port);
netdev_close(netdev);
} else {
@@ -3636,14 +3694,27 @@ iface_is_synthetic(const struct iface *iface)
}
static ofp_port_t
-iface_pick_ofport(const struct ovsrec_interface *cfg)
+iface_validate_ofport__(size_t n, int64_t *ofport)
+{
+ return (n && *ofport >= 1 && *ofport < ofp_to_u16(OFPP_MAX)
+ ? u16_to_ofp(*ofport)
+ : OFPP_NONE);
+}
+
+static ofp_port_t
+iface_get_requested_ofp_port(const struct ovsrec_interface *cfg)
{
- ofp_port_t ofport = cfg->n_ofport ? u16_to_ofp(*cfg->ofport)
- : OFPP_NONE;
- return cfg->n_ofport_request ? u16_to_ofp(*cfg->ofport_request)
- : ofport;
+ return iface_validate_ofport__(cfg->n_ofport_request, cfg->ofport_request);
}
+static ofp_port_t
+iface_pick_ofport(const struct ovsrec_interface *cfg)
+{
+ ofp_port_t requested_ofport = iface_get_requested_ofp_port(cfg);
+ return (requested_ofport != OFPP_NONE
+ ? requested_ofport
+ : iface_validate_ofport__(cfg->n_ofport, cfg->ofport));
+}
/* Port mirroring. */
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index fe176ffa5..2889a4105 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -1306,14 +1306,19 @@
- Open vSwitch currently assigns the OpenFlow port number for an
- interface once, when the client first adds the interface. It does
- not change the port number later if the client sets or changes or
- clears . Therefore, to ensure that
- takes effect, the client should set
- it in the same database transaction that creates the interface.
- (Future versions of Open vSwitch might honor changes to .)
+ A client should ideally set this column's value in the same
+ database transaction that it uses to create the interface. Open
+ vSwitch version 2.1 and later will honor a later request for a
+ specific port number, althuogh it might confuse some controllers:
+ OpenFlow does not have a way to announce a port number change, so
+ Open vSwitch represents it over OpenFlow as a port deletion
+ followed immediately by a port addition.
+
+
+
+ If is set or changed to some other
+ port's automatically assigned port number, Open vSwitch chooses a
+ new port number for the latter port.
--
2.43.0