From a4c2c1a843ff1492c1bfae2455a334b0d1c8389c Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 9 Mar 2020 11:19:47 +0100 Subject: [PATCH 1/3] ovs/ovsdb: support changing the MTU of an ovs interface Introduce a nm_ovsdb_set_interface_mtu() function to update the MTU of an ovs interface in the ovsdb. --- src/devices/ovs/nm-ovsdb.c | 79 ++++++++++++++++++++++++++++++-------- src/devices/ovs/nm-ovsdb.h | 3 ++ 2 files changed, 65 insertions(+), 17 deletions(-) diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c index 4265de647..18b22c246 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -103,6 +103,7 @@ typedef enum { OVSDB_MONITOR, OVSDB_ADD_INTERFACE, OVSDB_DEL_INTERFACE, + OVSDB_SET_INTERFACE_MTU, } OvsdbCommand; typedef struct { @@ -112,7 +113,10 @@ typedef struct { OvsdbMethodCallback callback; gpointer user_data; union { - char *ifname; + struct { + char *ifname; + guint32 mtu; + }; struct { NMConnection *bridge; NMConnection *port; @@ -154,6 +158,13 @@ _call_trace (const char *comment, OvsdbMethodCall *call, json_t *msg) msg ? ": " : "", msg ? str : ""); break; + case OVSDB_SET_INTERFACE_MTU: + _LOGT ("%s: set-iface-mtu interface=%s%s%s mtu=%u", + comment, call->ifname, + msg ? ": " : "", + msg ? str : "", + call->mtu); + break; } if (msg) @@ -172,7 +183,7 @@ ovsdb_call_method (NMOvsdb *self, OvsdbCommand command, const char *ifname, NMConnection *bridge, NMConnection *port, NMConnection *interface, NMDevice *bridge_device, NMDevice *interface_device, - OvsdbMethodCallback callback, gpointer user_data) + guint32 mtu, OvsdbMethodCallback callback, gpointer user_data) { NMOvsdbPrivate *priv = NM_OVSDB_GET_PRIVATE (self); OvsdbMethodCall *call; @@ -200,6 +211,10 @@ ovsdb_call_method (NMOvsdb *self, OvsdbCommand command, case OVSDB_DEL_INTERFACE: call->ifname = g_strdup (ifname); break; + case OVSDB_SET_INTERFACE_MTU: + call->ifname = g_strdup (ifname); + call->mtu = mtu; + break; } _call_trace ("enqueue", call, NULL); @@ -816,6 +831,22 @@ ovsdb_next_command (NMOvsdb *self) _delete_interface (self, params, call->ifname); + msg = json_pack ("{s:i, s:s, s:o}", + "id", call->id, + "method", "transact", "params", params); + break; + case OVSDB_SET_INTERFACE_MTU: + params = json_array (); + json_array_append_new (params, json_string ("Open_vSwitch")); + json_array_append_new (params, _inc_next_cfg (priv->db_uuid)); + + json_array_append_new (params, + json_pack ("{s:s, s:s, s:{s: i}, s:[[s, s, s]]}", + "op", "update", + "table", "Interface", + "row", "mtu_request", call->mtu, + "where", "name", "==", call->ifname)); + msg = json_pack ("{s:i, s:s, s:o}", "id", call->id, "method", "transact", "params", params); @@ -1461,7 +1492,7 @@ ovsdb_try_connect (NMOvsdb *self) /* Queue a monitor call before any other command, ensuring that we have an up * to date view of existing bridged that we need for add and remove ops. */ ovsdb_call_method (self, OVSDB_MONITOR, NULL, - NULL, NULL, NULL, NULL, NULL, _monitor_bridges_cb, NULL); + NULL, NULL, NULL, NULL, NULL, 0, _monitor_bridges_cb, NULL); } /*****************************************************************************/ @@ -1499,36 +1530,49 @@ out: g_slice_free (OvsdbCall, call); } +static OvsdbCall * +ovsdb_call_new (NMOvsdbCallback callback, gpointer user_data) +{ + OvsdbCall *call; + + call = g_slice_new (OvsdbCall); + call->callback = callback; + call->user_data = user_data; + + return call; +} + void nm_ovsdb_add_interface (NMOvsdb *self, NMConnection *bridge, NMConnection *port, NMConnection *interface, NMDevice *bridge_device, NMDevice *interface_device, NMOvsdbCallback callback, gpointer user_data) { - OvsdbCall *call; - - call = g_slice_new (OvsdbCall); - call->callback = callback; - call->user_data = user_data; - ovsdb_call_method (self, OVSDB_ADD_INTERFACE, NULL, bridge, port, interface, bridge_device, interface_device, - _transact_cb, call); + 0, + _transact_cb, + ovsdb_call_new (callback, user_data)); } void nm_ovsdb_del_interface (NMOvsdb *self, const char *ifname, NMOvsdbCallback callback, gpointer user_data) { - OvsdbCall *call; - - call = g_slice_new (OvsdbCall); - call->callback = callback; - call->user_data = user_data; - ovsdb_call_method (self, OVSDB_DEL_INTERFACE, ifname, - NULL, NULL, NULL, NULL, NULL, _transact_cb, call); + NULL, NULL, NULL, NULL, NULL, 0, + _transact_cb, + ovsdb_call_new (callback, user_data)); +} + +void nm_ovsdb_set_interface_mtu (NMOvsdb *self, const char *ifname, guint32 mtu, + NMOvsdbCallback callback, gpointer user_data) +{ + ovsdb_call_method (self, OVSDB_SET_INTERFACE_MTU, ifname, + NULL, NULL, NULL, NULL, NULL, mtu, + _transact_cb, + ovsdb_call_new (callback, user_data)); } /*****************************************************************************/ @@ -1549,6 +1593,7 @@ _clear_call (gpointer data) g_clear_object (&call->interface_device); break; case OVSDB_DEL_INTERFACE: + case OVSDB_SET_INTERFACE_MTU: nm_clear_g_free (&call->ifname); break; } diff --git a/src/devices/ovs/nm-ovsdb.h b/src/devices/ovs/nm-ovsdb.h index 59f462062..72a2dc732 100644 --- a/src/devices/ovs/nm-ovsdb.h +++ b/src/devices/ovs/nm-ovsdb.h @@ -34,4 +34,7 @@ void nm_ovsdb_add_interface (NMOvsdb *self, void nm_ovsdb_del_interface (NMOvsdb *self, const char *ifname, NMOvsdbCallback callback, gpointer user_data); +void nm_ovsdb_set_interface_mtu (NMOvsdb *self, const char *ifname, guint32 mtu, + NMOvsdbCallback callback, gpointer user_data); + #endif /* __NETWORKMANAGER_OVSDB_H__ */ From ad12f26312bb4ccc73076f39bd2a618094af6e7e Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Thu, 27 Feb 2020 13:54:43 +0100 Subject: [PATCH 2/3] ovs: set MTU from connection when creating an internal interface The ovs-vswitchd.conf.db(5) man page says about the the mtu_request column in the Interface table: "Requested MTU (Maximum Transmission Unit) for the interface. A client 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." Therefore, if the connection specifies a MTU, set it early when adding the interface to the ovsdb so that it will not be changed to the minimum of other interfaces. --- src/devices/ovs/nm-ovsdb.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c index 18b22c246..d78c489df 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -353,11 +353,20 @@ _insert_interface (json_t *params, NMConnection *interface, NMDevice *interface_ gs_free char *cloned_mac = NULL; gs_free_error GError *error = NULL; json_t *row; + guint32 mtu = 0; s_ovs_iface = nm_connection_get_setting_ovs_interface (interface); if (s_ovs_iface) type = nm_setting_ovs_interface_get_interface_type (s_ovs_iface); + if (nm_streq0 (type, "internal")) { + NMSettingWired *s_wired; + + s_wired = _nm_connection_get_setting (interface, NM_TYPE_SETTING_WIRED); + if (s_wired) + mtu = nm_setting_wired_get_mtu (s_wired); + } + if (!nm_device_hw_addr_get_cloned (interface_device, interface, FALSE, @@ -399,6 +408,9 @@ _insert_interface (json_t *params, NMConnection *interface, NMDevice *interface_ if (cloned_mac) json_object_set_new (row, "mac", json_string (cloned_mac)); + if (mtu != 0) + json_object_set_new (row, "mtu_request", json_integer (mtu)); + json_array_append_new (params, json_pack ("{s:s, s:s, s:o, s:s}", "op", "insert", From c2a97129453c49f86976b42c29b793a87d9d2e41 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 16 Mar 2020 10:53:06 +0100 Subject: [PATCH 3/3] ovs: set the MTU in ovsdb when changing platform MTU of ovs-interface If we change the the MTU of an ovs interface only through netlink, the change could be overridden by ovs-vswitchd at any time when other interfaces change. Set the MTU also in the ovsdb to prevent such changes. Note that if the MTU comes from the connection, we already set the ovsdb MTU at creation time and so this other update becomes useless. But it is needed when changing the MTU at runtime (reapply) or when the MTU comes from a different source (e.g. DHCP). --- src/devices/nm-device.c | 17 +++++++--- src/devices/nm-device.h | 2 ++ src/devices/ovs/nm-device-ovs-interface.c | 39 +++++++++++++++++++++++ 3 files changed, 54 insertions(+), 4 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 3a63af584..c3483e4db 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -9739,6 +9739,17 @@ _set_mtu (NMDevice *self, guint32 mtu) } } +static gboolean +set_platform_mtu (NMDevice *self, guint32 mtu) +{ + int r; + + r = nm_platform_link_set_mtu (nm_device_get_platform (self), + nm_device_get_ip_ifindex (self), + mtu); + return (r != -NME_PL_CANT_SET_MTU); +} + static void _commit_mtu (NMDevice *self, const NMIP4Config *config) { @@ -9898,10 +9909,7 @@ _commit_mtu (NMDevice *self, const NMIP4Config *config) } if (mtu_desired && mtu_desired != mtu_plat) { - int r; - - r = nm_platform_link_set_mtu (nm_device_get_platform (self), ifindex, mtu_desired); - if (r == -NME_PL_CANT_SET_MTU) { + if (!NM_DEVICE_GET_CLASS (self)->set_platform_mtu (self, mtu_desired)) { anticipated_failure = TRUE; success = FALSE; _LOGW (LOGD_DEVICE, "mtu: failure to set MTU. %s", @@ -17691,6 +17699,7 @@ nm_device_class_init (NMDeviceClass *klass) klass->parent_changed_notify = parent_changed_notify; klass->can_reapply_change = can_reapply_change; klass->reapply_connection = reapply_connection; + klass->set_platform_mtu = set_platform_mtu; obj_properties[PROP_UDI] = g_param_spec_string (NM_DEVICE_UDI, "", "", diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h index 28f86e825..c18301b27 100644 --- a/src/devices/nm-device.h +++ b/src/devices/nm-device.h @@ -444,6 +444,8 @@ typedef struct _NMDeviceClass { gboolean (* can_update_from_platform_link) (NMDevice *self, const NMPlatformLink *plink); + gboolean (* set_platform_mtu) (NMDevice *self, guint32 mtu); + /* Controls, whether to call act_stage2_config() callback also for assuming * a device or for external activations. In this case, act_stage2_config() must * take care not to touch the device's configuration. */ diff --git a/src/devices/ovs/nm-device-ovs-interface.c b/src/devices/ovs/nm-device-ovs-interface.c index 2868dee09..951b57889 100644 --- a/src/devices/ovs/nm-device-ovs-interface.c +++ b/src/devices/ovs/nm-device-ovs-interface.c @@ -121,6 +121,43 @@ _is_internal_interface (NMDevice *device) return nm_streq (nm_setting_ovs_interface_get_interface_type (s_ovs_iface), "internal"); } +static void +set_platform_mtu_cb (GError *error, gpointer user_data) +{ + NMDevice *device = user_data; + NMDeviceOvsInterface *self = NM_DEVICE_OVS_INTERFACE (device); + + if ( error + && !g_error_matches (error, NM_UTILS_ERROR, NM_UTILS_ERROR_CANCELLED_DISPOSING)) { + _LOGW (LOGD_DEVICE, "could not change mtu of '%s': %s", + nm_device_get_iface (device), error->message); + } + + g_object_unref (device); +} + +static gboolean +set_platform_mtu (NMDevice *device, guint32 mtu) +{ + /* + * If the MTU is not set in ovsdb, Open vSwitch will change + * the MTU of an internal interface to match the minimum of + * the other interfaces in the bridge. + */ + /* FIXME(shutdown): the function should become cancellable so + * that it doesn't need to hold a reference to the device, and + * it can be stopped during shutdown. + */ + if (_is_internal_interface (device)) { + nm_ovsdb_set_interface_mtu (nm_ovsdb_get (), + nm_device_get_ip_iface (device), + mtu, set_platform_mtu_cb, + g_object_ref (device)); + } + + return NM_DEVICE_CLASS (nm_device_ovs_interface_parent_class)->set_platform_mtu (device, mtu); +} + static NMActStageReturn act_stage3_ip_config_start (NMDevice *device, int addr_family, @@ -351,4 +388,6 @@ nm_device_ovs_interface_class_init (NMDeviceOvsInterfaceClass *klass) device_class->link_changed = link_changed; device_class->act_stage3_ip_config_start = act_stage3_ip_config_start; device_class->can_unmanaged_external_down = can_unmanaged_external_down; + device_class->set_platform_mtu = set_platform_mtu; + device_class->get_configured_mtu = nm_device_get_configured_mtu_for_wired; }