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