merge: branch 'reapply_default_pvid'

bridge: skip VLAN filtering resetting in reapply if no vlan change changed

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1855

src/core/devices/nm-device-bridge.c

@@ -619,7 +619,27 @@ master_update_slave_connection(NMDevice     *device,
 }
 
 static gboolean
-bridge_set_vlan_options(NMDevice *device, NMSettingBridge *s_bridge)
+is_bridge_pvid_changed(NMDevice *device, NMSettingBridge *s_bridge)
+{
+    int                        ifindex = nm_device_get_ifindex(device);
+    const NMPlatformLnkBridge *nmp_link_br;
+    NMPlatform                *platform = nm_device_get_platform(device);
+    bool    desired_vlan_filtering      = nm_setting_bridge_get_vlan_filtering(s_bridge);
+    guint16 desired_pvid                = nm_setting_bridge_get_vlan_default_pvid(s_bridge);
+
+    nm_platform_link_refresh(platform, ifindex);
+    nmp_link_br = nm_platform_link_get_lnk_bridge(platform, ifindex, NULL);
+
+    if (nmp_link_br) {
+        return desired_vlan_filtering != nmp_link_br->vlan_filtering
+               || desired_pvid != nmp_link_br->default_pvid;
+    } else {
+        return TRUE;
+    }
+}
+
+static gboolean
+bridge_set_vlan_options(NMDevice *device, NMSettingBridge *s_bridge, gboolean is_reapply)
 {
     NMDeviceBridge                      *self = NM_DEVICE_BRIDGE(device);
     gconstpointer                        hwaddr;
@@ -660,10 +680,15 @@ bridge_set_vlan_options(NMDevice *device, NMSettingBridge *s_bridge)
 
     self->vlan_configured = TRUE;
 
+    if (!is_reapply || is_bridge_pvid_changed(device, s_bridge)) {
         /* Filtering must be disabled to change the default PVID.
          * Clear the default PVID so that we later can force the re-creation of
          * default PVID VLANs by writing the option again. */
 
+        if (is_reapply) {
+            _LOGD(LOGD_BRIDGE, "default_pvid is changed, resetting bridge VLAN filtering");
+        }
+
         nm_platform_link_set_bridge_info(
             plat,
             ifindex,
@@ -686,6 +711,7 @@ bridge_set_vlan_options(NMDevice *device, NMSettingBridge *s_bridge)
                 &((NMPlatformLinkSetBridgeInfoData){.vlan_default_pvid_has = TRUE,
                                                     .vlan_default_pvid_val = pvid}));
         }
+    }
 
     /* Create VLANs only after setting the default PVID, so that
      * any PVID VLAN overrides the bridge's default PVID. */
@@ -743,7 +769,7 @@ _platform_lnk_bridge_init_from_setting(NMSettingBridge *s_bridge, NMPlatformLnkB
 }
 
 static gboolean
-link_config(NMDevice *device, NMConnection *connection)
+link_config(NMDevice *device, NMConnection *connection, gboolean is_reapply)
 {
     int                 ifindex = nm_device_get_ifindex(device);
     NMSettingBridge    *s_bridge;
@@ -757,7 +783,7 @@ link_config(NMDevice *device, NMConnection *connection)
     if (nm_platform_link_bridge_change(nm_device_get_platform(device), ifindex, &props) < 0)
         return FALSE;
 
-    return bridge_set_vlan_options(device, s_bridge);
+    return bridge_set_vlan_options(device, s_bridge, is_reapply);
 }
 
 static NMActStageReturn
@@ -768,7 +794,7 @@ act_stage1_prepare(NMDevice *device, NMDeviceStateReason *out_failure_reason)
     connection = nm_device_get_applied_connection(device);
     g_return_val_if_fail(connection, NM_ACT_STAGE_RETURN_FAILURE);
 
-    if (!link_config(device, connection)) {
+    if (!link_config(device, connection, FALSE)) {
         NM_SET_OUT(out_failure_reason, NM_DEVICE_STATE_REASON_CONFIG_FAILED);
         return NM_ACT_STAGE_RETURN_FAILURE;
     }
@@ -910,7 +936,7 @@ attach_port(NMDevice                  *device,
         s_port = nm_connection_get_setting_bridge_port(connection);
 
         if (!nm_device_sys_iface_state_is_external(device))
-            bridge_set_vlan_options(device, s_bridge);
+            bridge_set_vlan_options(device, s_bridge, FALSE);
 
         if (nm_setting_bridge_get_vlan_filtering(s_bridge)) {
             gs_free const NMPlatformBridgeVlan **plat_vlans = NULL;
@@ -1125,8 +1151,7 @@ reapply_connection(NMDevice *device, NMConnection *con_old, NMConnection *con_ne
     /* Make sure bridge_set_vlan_options() called by link_config()
      * sets vlan_filtering and default_pvid anew. */
     self->vlan_configured = FALSE;
-
+    link_config(device, con_new, TRUE);
-    link_config(device, con_new);
 }
 
 /*****************************************************************************/

src/core/platform/tests/test-link.c

@@ -1424,6 +1424,8 @@ test_software_detect(gconstpointer user_data)
         lnk_bridge.mcast_query_interval          = 12000;
         lnk_bridge.mcast_query_response_interval = 5200;
         lnk_bridge.mcast_startup_query_interval  = 3000;
+        lnk_bridge.vlan_filtering                = FALSE;
+        lnk_bridge.default_pvid                  = 1;
 
         if (!nmtstp_link_bridge_add(NULL, ext, DEVICE_NAME, &lnk_bridge))
             g_error("Failed adding Bridge interface");

src/libnm-platform/nm-linux-platform.c

@@ -1530,6 +1530,8 @@ _parse_lnk_bridge(const char *kind, struct nlattr *info_data)
         [IFLA_BR_MCAST_QUERY_INTVL]          = {.type = NLA_U64},
         [IFLA_BR_MCAST_QUERY_RESPONSE_INTVL] = {.type = NLA_U64},
         [IFLA_BR_MCAST_STARTUP_QUERY_INTVL]  = {.type = NLA_U64},
+        [IFLA_BR_VLAN_FILTERING]             = {.type = NLA_U8},
+        [IFLA_BR_VLAN_DEFAULT_PVID]          = {.type = NLA_U16},
     };
     NMPlatformLnkBridge *props;
     struct nlattr       *tb[G_N_ELEMENTS(policy)];
@@ -1600,6 +1602,10 @@ _parse_lnk_bridge(const char *kind, struct nlattr *info_data)
         props->mcast_query_response_interval = nla_get_u64(tb[IFLA_BR_MCAST_QUERY_RESPONSE_INTVL]);
     if (tb[IFLA_BR_MCAST_STARTUP_QUERY_INTVL])
         props->mcast_startup_query_interval = nla_get_u64(tb[IFLA_BR_MCAST_STARTUP_QUERY_INTVL]);
+    if (tb[IFLA_BR_VLAN_FILTERING])
+        props->vlan_filtering = !!nla_get_u8(tb[IFLA_BR_VLAN_FILTERING]);
+    if (tb[IFLA_BR_VLAN_DEFAULT_PVID])
+        props->default_pvid = nla_get_u16(tb[IFLA_BR_VLAN_DEFAULT_PVID]);
 
     return obj;
 }

src/libnm-platform/nm-platform.c

@@ -6221,7 +6221,8 @@ nm_platform_lnk_bridge_to_string(const NMPlatformLnkBridge *lnk, char *buf, gsiz
                " mcast_querier_interval %" G_GUINT64_FORMAT
                " mcast_query_interval %" G_GUINT64_FORMAT
                " mcast_query_response_interval %" G_GUINT64_FORMAT
-               " mcast_startup_query_interval %" G_GUINT64_FORMAT "",
+               " mcast_startup_query_interval %" G_GUINT64_FORMAT " vlan_filtering %d"
+               " default_pvid %" G_GUINT16_FORMAT "",
                lnk->forward_delay,
                lnk->hello_time,
                lnk->max_age,
@@ -6244,7 +6245,9 @@ nm_platform_lnk_bridge_to_string(const NMPlatformLnkBridge *lnk, char *buf, gsiz
                lnk->mcast_querier_interval,
                lnk->mcast_query_interval,
                lnk->mcast_query_response_interval,
-               lnk->mcast_startup_query_interval);
+               lnk->mcast_startup_query_interval,
+               lnk->vlan_filtering,
+               lnk->default_pvid);
     return buf;
 }
 
@@ -8146,12 +8149,14 @@ nm_platform_lnk_bridge_hash_update(const NMPlatformLnkBridge *obj, NMHashState *
                         obj->mcast_router,
                         obj->mcast_query_response_interval,
                         obj->mcast_startup_query_interval,
+                        obj->default_pvid,
                         NM_HASH_COMBINE_BOOLS(guint8,
                                               obj->stp_state,
                                               obj->mcast_querier,
                                               obj->mcast_query_use_ifaddr,
                                               obj->mcast_snooping,
-                                              obj->vlan_stats_enabled));
+                                              obj->vlan_stats_enabled,
+                                              obj->vlan_filtering));
 }
 
 void
@@ -8304,6 +8309,8 @@ nm_platform_lnk_bridge_cmp(const NMPlatformLnkBridge *a, const NMPlatformLnkBrid
     NM_CMP_FIELD(a, b, mcast_query_interval);
     NM_CMP_FIELD(a, b, mcast_query_response_interval);
     NM_CMP_FIELD(a, b, mcast_startup_query_interval);
+    NM_CMP_FIELD_BOOL(a, b, vlan_filtering);
+    NM_CMP_FIELD(a, b, default_pvid);
 
     return 0;
 }

src/libnm-platform/nm-platform.h

@@ -779,6 +779,8 @@ typedef struct {
     bool        mcast_snooping : 1;
     bool        stp_state : 1;
     bool        vlan_stats_enabled : 1;
+    bool        vlan_filtering;
+    guint16     default_pvid;
 } _nm_alignas(NMPlatformObject) NMPlatformLnkBridge;
 
 extern const NMPlatformLnkBridge nm_platform_lnk_bridge_default;