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

When doing reapply on linux bridge interface, NetworkManager will reset
the VLAN filtering and default PVID which cause PVID been readded to all
bridge ports regardless they are managed by NetworkManager.

This is because Linux kernel will re-add PVID to bridge port upon the
changes of bridge default-pvid value.

To fix the issue, this patch introduce netlink parsing code for
`vlan_filtering` and `default_pvid` of NMPlatformLnkBridge, and use that
to compare desired VLAN filtering settings, skip the reset of VLAN
filter if `default_pvid` and `vlan_filtering` are unchanged.

Signed-off-by: Gris Ge <fge@redhat.com>
(cherry picked from commit 02c34d538c)
(cherry picked from commit f990f9b4e4)
(cherry picked from commit c448e22519)
This commit is contained in:
Gris Ge 2024-02-08 23:36:34 +08:00 committed by Fernando Fernandez Mancera
parent ffef5a4748
commit 348fd5a4bb
5 changed files with 69 additions and 31 deletions

View file

@ -726,7 +726,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;
@ -762,28 +782,30 @@ bridge_set_vlan_options(NMDevice *device, NMSettingBridge *s_bridge)
self->vlan_configured = TRUE;
/* Filtering must be disabled to change the default PVID */
if (!nm_platform_sysctl_master_set_option(plat, ifindex, "vlan_filtering", "0"))
return FALSE;
/* Clear the default PVID so that we later can force the re-creation of
* default PVID VLANs by writing the option again. */
if (!nm_platform_sysctl_master_set_option(plat, ifindex, "default_pvid", "0"))
return FALSE;
/* Clear all existing VLANs */
if (!nm_platform_link_set_bridge_vlans(plat, ifindex, FALSE, NULL))
return FALSE;
/* Now set the default PVID. After this point the kernel creates
* a PVID VLAN on each port, including the bridge itself. */
pvid = nm_setting_bridge_get_vlan_default_pvid(s_bridge);
if (pvid) {
char value[32];
nm_sprintf_buf(value, "%u", pvid);
if (!nm_platform_sysctl_master_set_option(plat, ifindex, "default_pvid", value))
if (!is_reapply || is_bridge_pvid_changed(device, s_bridge)) {
/* Filtering must be disabled to change the default PVID */
if (!nm_platform_sysctl_master_set_option(plat, ifindex, "vlan_filtering", "0"))
return FALSE;
/* Clear the default PVID so that we later can force the re-creation of
* default PVID VLANs by writing the option again. */
if (!nm_platform_sysctl_master_set_option(plat, ifindex, "default_pvid", "0"))
return FALSE;
/* Clear all existing VLANs */
if (!nm_platform_link_set_bridge_vlans(plat, ifindex, FALSE, NULL))
return FALSE;
/* Now set the default PVID. After this point the kernel creates
* a PVID VLAN on each port, including the bridge itself. */
pvid = nm_setting_bridge_get_vlan_default_pvid(s_bridge);
if (pvid) {
char value[32];
nm_sprintf_buf(value, "%u", pvid);
if (!nm_platform_sysctl_master_set_option(plat, ifindex, "default_pvid", value))
return FALSE;
}
}
/* Create VLANs only after setting the default PVID, so that
@ -838,7 +860,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;
@ -852,7 +874,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
@ -863,7 +885,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;
}
@ -1005,7 +1027,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;
@ -1213,8 +1235,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);
link_config(device, con_new, TRUE);
}
/*****************************************************************************/

View file

@ -1357,6 +1357,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");

View file

@ -1502,6 +1502,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)];
@ -1572,6 +1574,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;
}

View file

@ -6134,7 +6134,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,
@ -6157,7 +6158,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;
}
@ -8003,12 +8006,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
@ -8136,6 +8141,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;
}

View file

@ -748,6 +748,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;