Open vSwitch controls the MTU of internal ports and sets it to the
minimum of physical ports MTU on the same bridge.
Commit 47bf118665a3("ofproto: Always set MTU for new internal ports.")
made this more consistent. Now the MTU is always set, even when the
bridge doesn't have any physical ports (e.g. when it has only an
internal device and a tunnel).
This latest change broke the system testsuite. Some tests need to
lower internal devices MTU because they use tunnels and they want to
work with a 1500 bytes underlay.
There are other valid usecases where the user/controller needs to
configure the internal devices MTU (like mpls push actions, double vlans
or any overlay) and there's no way for Open vSwitch to know what the
appropriate MTU should be.
Since in the general case Open vSwitch is not able to configure a
reasonable MTU for internal devices, this commit removes the feature
entirely.
Now the user/controller is entirely responsible for configuring the MTU
of internal ports. Other hybrid solutions are possible (like not
touching the internal interfaces MTU, unless there's a physical device),
but they make the current MTU dependent on the previous state of the
system (if there was at some point a physical device the MTU would be
touched, but it wouldn't be possible to restore it).
This change breaks compatibility with previous versions on Open vSwitch.
Signed-off-by: Daniele Di Proietto <diproiet...@vmware.com>
---
I realize that it's not nice to introduce a backwards incompatible
change, especially so late in the release. I considered other possible
solutions, but they all introduce undesirable behavior. If it's not
acceptable to break compatibility, I can get away with alternative
approaches.
---
NEWS | 4 ++
debian/changelog | 4 ++
ofproto/ofproto-provider.h | 2 -
ofproto/ofproto.c | 101 ---------------------------------------------
tests/ofproto-dpif.at | 16 +------
vswitchd/vswitch.xml | 10 ++---
6 files changed, 13 insertions(+), 124 deletions(-)
diff --git a/NEWS b/NEWS
index 1439151..590a7b9 100644
--- a/NEWS
+++ b/NEWS
@@ -123,6 +123,10 @@ v2.6.0 - xx xxx xxxx
disable it in OpenSSL.
- Add 'mtu_request' column to the Interface table. It can be used to
configure the MTU of non-internal ports.
+ - Open vSwitch no longer automatically configures the internal interfaces
+ MTU to match the rest of the bridge. Please use external tools (or
+ better, the 'mtu_request' column) to appropriately configure the MTU on
+ internal ports.
v2.5.0 - 26 Feb 2016
diff --git a/debian/changelog b/debian/changelog
index d73e636..9958ef9 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -117,6 +117,10 @@ openvswitch (2.6.0-1) unstable; urgency=low
disable it in OpenSSL.
- Add 'mtu_request' column to the Interface table. It can be used to
configure the MTU of non-internal ports.
+ - Open vSwitch no longer automatically configures the internal interfaces
+ MTU to match the rest of the bridge. Please use external tools (or
+ better, the 'mtu_request' column) to appropriately configure the MTU on
+ internal ports.
-- Open vSwitch team <dev@openvswitch.org> Mon, 15 Aug 2016 19:53:13 -0700
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 7f7813e..9f2c408 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -115,8 +115,6 @@ struct ofproto {
/* OpenFlow connections. */
struct connmgr *connmgr;
- int min_mtu; /* Current MTU of non-internal ports. */
-
/* Groups. */
struct cmap groups; /* Contains "struct ofgroup"s. */
uint32_t n_groups[4] OVS_GUARDED; /* # of existing groups of each type. */
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 9d62b72..9fc87de 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -175,8 +175,6 @@ static void learned_cookies_flush(struct ofproto *, struct
ovs_list *dead_cookie
/* ofport. */
static void ofport_destroy__(struct ofport *) OVS_EXCLUDED(ofproto_mutex);
static void ofport_destroy(struct ofport *, bool del);
-static inline bool ofport_is_internal(const struct ofproto *,
- const struct ofport *);
static int update_port(struct ofproto *, const char *devname);
static int init_ports(struct ofproto *);
@@ -273,16 +271,12 @@ static void calc_duration(long long int start, long long
int now,
static uint64_t pick_datapath_id(const struct ofproto *);
static uint64_t pick_fallback_dpid(void);
static void ofproto_destroy__(struct ofproto *);
-static void update_mtu(struct ofproto *, struct ofport *);
-static void update_mtu_ofproto(struct ofproto *);
static void meter_delete(struct ofproto *, uint32_t first, uint32_t last);
static void meter_insert_rule(struct rule *);
/* unixctl. */
static void ofproto_unixctl_init(void);
-static int find_min_mtu(struct ofproto *p);
-
/* All registered ofproto classes, in probe order. */
static const struct ofproto_class **ofproto_classes;
static size_t n_ofproto_classes;
@@ -516,7 +510,6 @@ ofproto_create(const char *datapath_name, const char
*datapath_type,
hmap_init(&ofproto->learned_cookies);
ovs_list_init(&ofproto->expirable);
ofproto->connmgr = connmgr_create(ofproto, datapath_name, datapath_name);
- ofproto->min_mtu = find_min_mtu(ofproto);
cmap_init(&ofproto->groups);
ovs_mutex_unlock(&ofproto_mutex);
ofproto->ogf.types = 0xf;
@@ -2392,8 +2385,6 @@ ofport_install(struct ofproto *p,
hash_ofp_port(ofport->ofp_port));
shash_add(&p->port_by_name, netdev_name, ofport);
- update_mtu(p, ofport);
-
/* Let the ofproto_class initialize its private data. */
error = p->ofproto_class->port_construct(ofport);
if (error) {
@@ -2417,15 +2408,9 @@ error:
static void
ofport_remove(struct ofport *ofport)
{
- struct ofproto *p = ofport->ofproto;
- bool is_internal = ofport_is_internal(p, ofport);
-
connmgr_send_port_status(ofport->ofproto->connmgr, NULL, &ofport->pp,
OFPPR_DELETE);
ofport_destroy(ofport, true);
- if (!is_internal) {
- update_mtu_ofproto(p);
- }
}
/* If 'ofproto' contains an ofport named 'name', removes it from 'ofproto' and
@@ -2621,8 +2606,6 @@ update_port(struct ofproto *ofproto, const char *name)
ofport_modified(port, &pp);
}
- update_mtu(ofproto, port);
-
/* Install the newly opened netdev in case it has changed.
* Don't close the old netdev yet in case port_modified has to
* remove a retained reference to it.*/
@@ -2702,90 +2685,6 @@ init_ports(struct ofproto *p)
return 0;
}
-
-static inline bool
-ofport_is_internal(const struct ofproto *p, const struct ofport *port)
-{
- return !strcmp(netdev_get_type(port->netdev),
- ofproto_port_open_type(p->type, "internal"));
-}
-
-/* Find the minimum MTU of all non-datapath devices attached to 'p'.
- * Returns ETH_PAYLOAD_MAX or the minimum of the ports. */
-static int
-find_min_mtu(struct ofproto *p)
-{
- struct ofport *ofport;
- int mtu = 0;
-
- HMAP_FOR_EACH (ofport, hmap_node, &p->ports) {
- struct netdev *netdev = ofport->netdev;
- int dev_mtu;
-
- /* Skip any internal ports, since that's what we're trying to
- * set. */
- if (ofport_is_internal(p, ofport)) {
- continue;
- }
-
- if (netdev_get_mtu(netdev, &dev_mtu)) {
- continue;
- }
- if (!mtu || dev_mtu < mtu) {
- mtu = dev_mtu;
- }
- }
-
- return mtu ? mtu: ETH_PAYLOAD_MAX;
-}
-
-/* Update MTU of all datapath devices on 'p' to the minimum of the
- * non-datapath ports in event of 'port' added or changed. */
-static void
-update_mtu(struct ofproto *p, struct ofport *port)
-{
- struct netdev *netdev = port->netdev;
- int dev_mtu;
-
- if (netdev_get_mtu(netdev, &dev_mtu)) {
- port->mtu = 0;
- return;
- }
- if (ofport_is_internal(p, port)) {
- if (!netdev_set_mtu(port->netdev, p->min_mtu)) {
- dev_mtu = p->min_mtu;
- }
- port->mtu = dev_mtu;
- return;
- }
-
- port->mtu = dev_mtu;
- /* For non-internal port find new min mtu. */
- update_mtu_ofproto(p);
-}
-
-static void
-update_mtu_ofproto(struct ofproto *p)
-{
- /* For non-internal port find new min mtu. */
- struct ofport *ofport;
- int old_min = p->min_mtu;
-
- p->min_mtu = find_min_mtu(p);
- if (p->min_mtu == old_min) {
- return;
- }
-
- HMAP_FOR_EACH (ofport, hmap_node, &p->ports) {
- struct netdev *netdev = ofport->netdev;
-
- if (ofport_is_internal(p, ofport)) {
- if (!netdev_set_mtu(netdev, p->min_mtu)) {
- ofport->mtu = p->min_mtu;
- }
- }
- }
-}
static void
ofproto_rule_destroy__(struct rule *rule)
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 8e5fde6..250d99a 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -8870,10 +8870,7 @@ AT_CHECK([ovs-vsctl get Interface br0 mtu], [0], [dnl
add_of_ports br0 1
-# Check that initial MTU is 1500 for 'br0' and 'p1'.
-AT_CHECK([ovs-vsctl get Interface br0 mtu], [0], [dnl
-1500
-])
+# Check that initial MTU is 1500 for 'p1'.
AT_CHECK([ovs-vsctl get Interface p1 mtu], [0], [dnl
1500
])
@@ -8883,23 +8880,12 @@ AT_CHECK([ovs-vsctl set Interface p1 mtu_request=1600])
# Check that the new MTU is applied
AT_CHECK([ovs-vsctl --timeout=10 wait-until Interface p1 mtu=1600])
-# The internal port 'br0' should have the same MTU value as p1, becase it's
-# the new bridge minimum.
-AT_CHECK([ovs-vsctl --timeout=10 wait-until Interface br0 mtu=1600])
AT_CHECK([ovs-vsctl del-port br0 p1])
-# When 'p1' is deleted, the internal port should return to the default MTU
-AT_CHECK([ovs-vsctl --timeout=10 wait-until Interface br0 mtu=1500])
-
# New port with 'mtu_request' in the same transaction.
AT_CHECK([ovs-vsctl add-port br0 p2 -- set int p2 type=dummy mtu_request=1600])
AT_CHECK([ovs-vsctl --timeout=10 wait-until Interface p2 mtu=1600])
-AT_CHECK([ovs-vsctl --timeout=10 wait-until Interface br0 mtu=1600])
-
-# New internal port. The MTU should be updated even for a newly added port.
-AT_CHECK([ovs-vsctl add-port br0 int1 -- set int int1 type=internal])
-AT_CHECK([ovs-vsctl --timeout=10 wait-until Interface int1 mtu=1600])
OVS_VSWITCHD_STOP
AT_CLEANUP
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 69b5592..684df55 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -2388,11 +2388,9 @@
</p>
<p>
- A client may change a non-internal interface MTU by filling in
- <ref column="mtu_request"/>. Internal interfaces MTU, instead, is set
- by Open vSwitch to the minimum of non-internal interfaces MTU in the
- bridge. In any case, Open vSwitch then reports in <ref column="mtu"/>
- the currently configured value.
+ A client may change an interface MTU by filling in
+ <ref column="mtu_request"/>. Open vSwitch then reports in
+ <ref column="mtu"/> the currently configured value.
</p>
<column name="mtu">
@@ -2411,7 +2409,7 @@
type='{"type": "integer", "minInteger": 1}'>
<p>
Requested MTU (Maximum Transmission Unit) for the interface. A client
- can fill this column to change the MTU of a non-internal interface.
+ can fill this column to change the MTU of an interface.
</p>
</column>
--
2.9.3
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
