diff --git a/FAQ.md b/FAQ.md index 5a09d1344..01e2648c1 100644 --- a/FAQ.md +++ b/FAQ.md @@ -1065,6 +1065,33 @@ A: This is expected behavior on virtual switches. RFC2544 tests were ovs-vsctl --no-wait set Open_vSwitch . other_config:max-idle=50000 +### Q: How can I configure the bridge internal interface MTU? Why does Open + vSwitch keep changing internal ports MTU? + +A: By default Open vSwitch overrides the internal interfaces (e.g. br0) MTU. + If you have just an internal interface (e.g. br0) and a physical interface + (e.g. eth0), then every change in MTU to eth0 will be reflected to br0. + Any manual MTU configuration using `ip` or `ifconfig` on internal interfaces + is going to be overridden by Open vSwitch to match the current bridge + minimum. + + Sometimes this behavior is not desirable, for example with tunnels. The + MTU of an internal interface can be explicitly set using the following + command: + + ovs-vsctl set int br0 mtu_request=1450 + + After this, Open vSwitch will configure br0 MTU to 1450. Since this + setting is in the database it will be persistent (compared to what + happens with `ip` or `ifconfig`). + + The MTU configuration can be removed to restore the default behavior with + + ovs-vsctl set int br0 mtu_request=[] + + The mtu_request column can be used to configure MTU even for physical + interfaces (e.g. eth0). + ## QOS ### Q: Does OVS support Quality of Service (QoS)? diff --git a/NEWS b/NEWS index 143915136..8c78b3676 100644 --- a/NEWS +++ b/NEWS @@ -122,7 +122,7 @@ v2.6.0 - xx xxx xxxx SHA-1 is no longer secure and some operating systems have started to disable it in OpenSSL. - Add 'mtu_request' column to the Interface table. It can be used to - configure the MTU of non-internal ports. + configure the MTU of the ports. v2.5.0 - 26 Feb 2016 diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h index cd04ae9e6..c8507a56b 100644 --- a/lib/netdev-provider.h +++ b/lib/netdev-provider.h @@ -63,6 +63,10 @@ struct netdev { struct seq *reconfigure_seq; uint64_t last_reconfigure_seq; + /* If this is 'true', the user explicitly specified an MTU for this + * netdev. Otherwise, Open vSwitch is allowed to override it. */ + bool mtu_user_config; + /* The core netdev code initializes these at netdev construction and only * provide read-only access to its client. Netdev implementations may * modify them. */ diff --git a/lib/netdev.c b/lib/netdev.c index 10f2d0f03..ddf2a34ac 100644 --- a/lib/netdev.c +++ b/lib/netdev.c @@ -877,6 +877,27 @@ netdev_set_mtu(struct netdev *netdev, int mtu) return error; } +/* If 'user_config' is true, the user wants to control 'netdev''s MTU and we + * should not override it. If 'user_config' is false, we may adjust + * 'netdev''s MTU (e.g., if 'netdev' is internal). */ +void +netdev_mtu_user_config(struct netdev *netdev, bool user_config) +{ + if (netdev->mtu_user_config != user_config) { + netdev_change_seq_changed(netdev); + netdev->mtu_user_config = user_config; + } +} + +/* Returns 'true' if the user explicitly specified an MTU value for 'netdev'. + * Otherwise, returns 'false', in which case we are allowed to adjust the + * device MTU. */ +bool +netdev_mtu_is_user_config(struct netdev *netdev) +{ + return netdev->mtu_user_config; +} + /* Returns the ifindex of 'netdev', if successful, as a positive number. On * failure, returns a negative errno value. * diff --git a/lib/netdev.h b/lib/netdev.h index d8ec62798..634c665f3 100644 --- a/lib/netdev.h +++ b/lib/netdev.h @@ -133,6 +133,8 @@ const char *netdev_get_type(const struct netdev *); const char *netdev_get_type_from_name(const char *); int netdev_get_mtu(const struct netdev *, int *mtup); int netdev_set_mtu(struct netdev *, int mtu); +void netdev_mtu_user_config(struct netdev *, bool); +bool netdev_mtu_is_user_config(struct netdev *); int netdev_get_ifindex(const struct netdev *); int netdev_set_tx_multiq(struct netdev *, unsigned int n_txq); diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index da7372de0..f813c0b1a 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -175,8 +175,8 @@ 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 bool ofport_is_mtu_overridden(const struct ofproto *, + const struct ofport *); static int update_port(struct ofproto *, const char *devname); static int init_ports(struct ofproto *); @@ -2416,12 +2416,12 @@ static void ofport_remove(struct ofport *ofport) { struct ofproto *p = ofport->ofproto; - bool is_internal = ofport_is_internal(p, ofport); + bool is_mtu_overridden = ofport_is_mtu_overridden(p, ofport); connmgr_send_port_status(ofport->ofproto->connmgr, NULL, &ofport->pp, OFPPR_DELETE); ofport_destroy(ofport, true); - if (!is_internal) { + if (!is_mtu_overridden) { update_mtu_ofproto(p); } } @@ -2701,14 +2701,23 @@ init_ports(struct ofproto *p) return 0; } -static inline bool +static 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'. +/* If 'port' is internal and if the user didn't explicitly specify an mtu + * through the database, we have to override it. */ +static bool +ofport_is_mtu_overridden(const struct ofproto *p, const struct ofport *port) +{ + return ofport_is_internal(p, port) + && !netdev_mtu_is_user_config(port->netdev); +} + +/* Find the minimum MTU of all non-overridden devices attached to 'p'. * Returns ETH_PAYLOAD_MAX or the minimum of the ports. */ static int find_min_mtu(struct ofproto *p) @@ -2720,9 +2729,8 @@ find_min_mtu(struct ofproto *p) 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)) { + /* Skip any overridden port, since that's what we're trying to set. */ + if (ofport_is_mtu_overridden(p, ofport)) { continue; } @@ -2737,8 +2745,8 @@ find_min_mtu(struct ofproto *p) 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. */ +/* Update MTU of all overridden devices on 'p' to the minimum of the + * non-overridden ports in event of 'port' added or changed. */ static void update_mtu(struct ofproto *p, struct ofport *port) { @@ -2749,25 +2757,25 @@ update_mtu(struct ofproto *p, struct ofport *port) port->mtu = 0; return; } - if (ofport_is_internal(p, port)) { + if (ofport_is_mtu_overridden(p, port)) { if (dev_mtu > p->min_mtu) { - if (!netdev_set_mtu(port->netdev, p->min_mtu)) { - dev_mtu = p->min_mtu; - } + 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. */ + /* For non-overridden 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; @@ -2779,7 +2787,7 @@ update_mtu_ofproto(struct ofproto *p) HMAP_FOR_EACH (ofport, hmap_node, &p->ports) { struct netdev *netdev = ofport->netdev; - if (ofport_is_internal(p, ofport)) { + if (ofport_is_mtu_overridden(p, ofport)) { if (!netdev_set_mtu(netdev, p->min_mtu)) { ofport->mtu = p->min_mtu; } diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index d7705da87..0295a99ef 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -8892,5 +8892,20 @@ 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]) +# Explicitly set mtu_request on the internal interface. This should prevent +# the MTU from being overriden. +AT_CHECK([ovs-vsctl set int br0 mtu_request=1700]) +AT_CHECK([ovs-vsctl --timeout=10 wait-until Interface br0 mtu=1700]) + +# The new MTU on p2 should not affect br0. +AT_CHECK([ovs-vsctl set int p2 mtu_request=1400]) +AT_CHECK([ovs-vsctl --timeout=10 wait-until Interface p2 mtu=1400]) +AT_CHECK([ovs-vsctl --timeout=10 wait-until Interface br0 mtu=1700]) + +# Remove explicit mtu_request from br0. Now it should track the bridge +# minimum again. +AT_CHECK([ovs-vsctl set int br0 mtu_request=[[]]]) +AT_CHECK([ovs-vsctl --timeout=10 wait-until Interface br0 mtu=1400]) + OVS_VSWITCHD_STOP AT_CLEANUP diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index a7a12ecfe..61cb96633 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -722,20 +722,20 @@ add_ofp_port(ofp_port_t port, ofp_port_t *ports, size_t *n, size_t *allocated) } /* Configures the MTU of 'netdev' based on the "mtu_request" column - * in 'iface_cfg'. 'br_type' must be the datapath_type of the bridge - * which contains 'netdev'. */ + * in 'iface_cfg'. */ static int iface_set_netdev_mtu(const struct ovsrec_interface *iface_cfg, - const char *br_type, struct netdev *netdev) + struct netdev *netdev) { - if (iface_cfg->n_mtu_request == 1 - && strcmp(netdev_get_type(netdev), - ofproto_port_open_type(br_type, "internal"))) { - /* Try to set the MTU to the requested value. This is not done - * for internal interfaces, since their MTU is decided by the - * ofproto module, based on other ports in the bridge. */ + if (iface_cfg->n_mtu_request == 1) { + /* The user explicitly asked for this MTU. */ + netdev_mtu_user_config(netdev, true); + /* Try to set the MTU to the requested value. */ return netdev_set_mtu(netdev, *iface_cfg->mtu_request); } + + /* The user didn't explicitly asked for any MTU. */ + netdev_mtu_user_config(netdev, false); return 0; } @@ -793,7 +793,7 @@ bridge_delete_or_reconfigure_ports(struct bridge *br) goto delete; } - iface_set_netdev_mtu(iface->cfg, br->type, iface->netdev); + iface_set_netdev_mtu(iface->cfg, iface->netdev); /* If the requested OpenFlow port for 'iface' changed, and it's not * already the correct port, then we might want to temporarily delete @@ -1736,7 +1736,7 @@ iface_do_create(const struct bridge *br, goto error; } - iface_set_netdev_mtu(iface_cfg, br->type, netdev); + iface_set_netdev_mtu(iface_cfg, netdev); *ofp_portp = iface_pick_ofport(iface_cfg); error = ofproto_port_add(br->ofproto, netdev, ofp_portp); diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml index 69b559202..f64c18acb 100644 --- a/vswitchd/vswitch.xml +++ b/vswitchd/vswitch.xml @@ -2388,14 +2388,16 @@

- A client may change a non-internal interface MTU by filling in - . 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 - the currently configured value. + A client may change an interface MTU by filling in + . Open vSwitch then reports in + the currently configured value.

+

+ The currently configured MTU for the interface. +

+

This column will be empty for an interface that does not have an MTU as, for example, some kinds of tunnels do not. @@ -2411,7 +2413,13 @@ type='{"type": "integer", "minInteger": 1}'>

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. +

+ +

+ If this is not set and if the interface has internal + type, Open vSwitch will change the MTU to match the minimum of the + other interfaces in the bridge.