From 021141481e3cb7f747111dad67323f5d1a608bf6 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. (cherry picked from commit a4c2c1a843ff1492c1bfae2455a334b0d1c8389c) (cherry picked from commit c1be15a66edf1ef1e7de1cc7a0f6e134b8dbdcb7) (cherry picked from commit 990f46505d6d9bd4de40db458978b6c94664de13) --- 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 c85115201e..9ed8e06f52 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -118,6 +118,7 @@ typedef enum { OVSDB_MONITOR, OVSDB_ADD_INTERFACE, OVSDB_DEL_INTERFACE, + OVSDB_SET_INTERFACE_MTU, } OvsdbCommand; typedef struct { @@ -127,7 +128,10 @@ typedef struct { OvsdbMethodCallback callback; gpointer user_data; union { - char *ifname; + struct { + char *ifname; + guint32 mtu; + }; struct { NMConnection *bridge; NMConnection *port; @@ -169,6 +173,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) @@ -187,7 +198,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; @@ -215,6 +226,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); @@ -817,6 +832,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); @@ -1462,7 +1493,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); } /*****************************************************************************/ @@ -1500,36 +1531,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)); } /*****************************************************************************/ @@ -1550,6 +1594,7 @@ _clear_call (gpointer data) g_clear_object (&call->interface_device); break; case OVSDB_DEL_INTERFACE: + case OVSDB_SET_INTERFACE_MTU: g_clear_pointer (&call->ifname, g_free); break; } diff --git a/src/devices/ovs/nm-ovsdb.h b/src/devices/ovs/nm-ovsdb.h index e7f9d71984..9552dcca32 100644 --- a/src/devices/ovs/nm-ovsdb.h +++ b/src/devices/ovs/nm-ovsdb.h @@ -48,4 +48,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 59787018c983e433da6172944e60246961c38d71 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. (cherry picked from commit ad12f26312bb4ccc73076f39bd2a618094af6e7e) (cherry picked from commit 7311d5e2943931a70fb427096ea934072aed63b3) (cherry picked from commit b81370f70b538688a62ad4a29482dbf9b34c69bc) --- 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 9ed8e06f52..753f5f172a 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -367,11 +367,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, @@ -404,6 +413,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 04264be151f323a78dea23e58311c90d9928bd9e 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). (cherry picked from commit c2a97129453c49f86976b42c29b793a87d9d2e41) (cherry picked from commit e27a59c69e956961efe99f83068b760380b77066) (cherry picked from commit 99ef891db6a561d6b3dcaec9ead2b2158b43ae8d) --- 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 f966ae6f4c..5b7015779c 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -9152,6 +9152,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) { @@ -9310,10 +9321,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", @@ -16995,6 +17003,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 0eab3aca19..5a9339d03d 100644 --- a/src/devices/nm-device.h +++ b/src/devices/nm-device.h @@ -462,6 +462,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 9c74ffd1c2..f2919b4881 100644 --- a/src/devices/ovs/nm-device-ovs-interface.c +++ b/src/devices/ovs/nm-device-ovs-interface.c @@ -134,6 +134,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, @@ -364,4 +401,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; }