From 18e530b03f7e9a93a0b42655679f417676a4f263 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 12 Nov 2023 22:12:26 +0900 Subject: [PATCH 1/8] network/neighbor: suppress noisy debugging logs - AF_BRIDGE is not invalid. But we are not interested in it, currently. - Also, we are not interested in non-static neighbors, at least currently, - As already commented, the kernel sends message about neighbors after sending RTM_DELLINK. In that case, the corresponding Link object is already removed, and there is nothing we need to do in that case. --- src/network/networkd-neighbor.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/network/networkd-neighbor.c b/src/network/networkd-neighbor.c index c4e1c463c45..e9986567ac7 100644 --- a/src/network/networkd-neighbor.c +++ b/src/network/networkd-neighbor.c @@ -510,10 +510,9 @@ int manager_rtnl_process_neighbor(sd_netlink *rtnl, sd_netlink_message *message, if (r < 0) { log_warning_errno(r, "rtnl: received neighbor message with invalid state, ignoring: %m"); return 0; - } else if (!FLAGS_SET(state, NUD_PERMANENT)) { - log_debug("rtnl: received non-static neighbor, ignoring."); + } else if (!FLAGS_SET(state, NUD_PERMANENT)) + /* Currently, we are interested in only static neighbors. */ return 0; - } r = sd_rtnl_message_neigh_get_ifindex(message, &ifindex); if (r < 0) { @@ -525,12 +524,10 @@ int manager_rtnl_process_neighbor(sd_netlink *rtnl, sd_netlink_message *message, } r = link_get_by_index(m, ifindex, &link); - if (r < 0) { + if (r < 0) /* when enumerating we might be out of sync, but we will get the neighbor again. Also, * kernel sends messages about neighbors after a link is removed. So, just ignore it. */ - log_debug("rtnl: received neighbor for link '%d' we don't know about, ignoring.", ifindex); return 0; - } tmp = new0(Neighbor, 1); @@ -539,7 +536,10 @@ int manager_rtnl_process_neighbor(sd_netlink *rtnl, sd_netlink_message *message, if (r < 0) { log_link_warning(link, "rtnl: received neighbor message without family, ignoring."); return 0; - } else if (!IN_SET(tmp->family, AF_INET, AF_INET6)) { + } + if (tmp->family == AF_BRIDGE) /* Currently, we do not support it. */ + return 0; + if (!IN_SET(tmp->family, AF_INET, AF_INET6)) { log_link_debug(link, "rtnl: received neighbor message with invalid family '%i', ignoring.", tmp->family); return 0; } From 26ba67e5eca0fe7c924ab2b870544299313ad8df Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 11 Nov 2023 18:15:55 +0900 Subject: [PATCH 2/8] network/brvlan: rework bridge vlan ID assignment No functional change, just refactoring. --- src/network/networkd-bridge-vlan.c | 241 +++++++++++++++++------------ src/network/networkd-bridge-vlan.h | 7 +- src/network/networkd-setlink.c | 22 +-- 3 files changed, 144 insertions(+), 126 deletions(-) diff --git a/src/network/networkd-bridge-vlan.c b/src/network/networkd-bridge-vlan.c index 36e3610a8f9..bfc404f154d 100644 --- a/src/network/networkd-bridge-vlan.c +++ b/src/network/networkd-bridge-vlan.c @@ -17,9 +17,9 @@ #include "parse-util.h" #include "vlan-util.h" -static bool is_bit_set(unsigned bit, uint32_t scope) { - assert(bit < sizeof(scope)*8); - return scope & (UINT32_C(1) << bit); +static bool is_bit_set(unsigned nr, const uint32_t *addr) { + assert(nr < BRIDGE_VLAN_BITMAP_MAX); + return addr[nr / 32] & (UINT32_C(1) << (nr % 32)); } static void set_bit(unsigned nr, uint32_t *addr) { @@ -27,116 +27,159 @@ static void set_bit(unsigned nr, uint32_t *addr) { addr[nr / 32] |= (UINT32_C(1) << (nr % 32)); } -static int find_next_bit(int i, uint32_t x) { - int j; - - if (i >= 32) - return -1; - - /* find first bit */ - if (i < 0) - return BUILTIN_FFS_U32(x); - - /* mask off prior finds to get next */ - j = __builtin_ffs(x >> i); - return j ? j + i : 0; +static int add_single(sd_netlink_message *m, uint16_t id, bool untagged, bool is_pvid) { + assert(m); + assert(id < BRIDGE_VLAN_BITMAP_MAX); + return sd_netlink_message_append_data(m, IFLA_BRIDGE_VLAN_INFO, + &(struct bridge_vlan_info) { + .vid = id, + .flags = (untagged ? BRIDGE_VLAN_INFO_UNTAGGED : 0) | + (is_pvid ? BRIDGE_VLAN_INFO_PVID : 0), + }, + sizeof(struct bridge_vlan_info)); } -int bridge_vlan_append_info( - const Link *link, - sd_netlink_message *req, - uint16_t pvid, - const uint32_t *br_vid_bitmap, - const uint32_t *br_untagged_bitmap) { +static int add_range(sd_netlink_message *m, uint16_t begin, uint16_t end, bool untagged) { + int r; - struct bridge_vlan_info br_vlan; - bool done, untagged = false; - uint16_t begin, end; - int r, cnt; + assert(m); + assert(begin <= end); + assert(end < BRIDGE_VLAN_BITMAP_MAX); + + if (begin == end) + return add_single(m, begin, untagged, /* is_pvid = */ false); + + r = sd_netlink_message_append_data(m, IFLA_BRIDGE_VLAN_INFO, + &(struct bridge_vlan_info) { + .vid = begin, + .flags = (untagged ? BRIDGE_VLAN_INFO_UNTAGGED : 0) | + BRIDGE_VLAN_INFO_RANGE_BEGIN, + }, + sizeof(struct bridge_vlan_info)); + if (r < 0) + return r; + + r = sd_netlink_message_append_data(m, IFLA_BRIDGE_VLAN_INFO, + &(struct bridge_vlan_info) { + .vid = end, + .flags = (untagged ? BRIDGE_VLAN_INFO_UNTAGGED : 0) | + BRIDGE_VLAN_INFO_RANGE_END, + }, + sizeof(struct bridge_vlan_info)); + if (r < 0) + return r; + + return 0; +} + +static int bridge_vlan_append_set_info(Link *link, sd_netlink_message *m) { + uint16_t begin = UINT16_MAX; + bool untagged; + int r; assert(link); - assert(req); - assert(br_vid_bitmap); - assert(br_untagged_bitmap); + assert(link->network); + assert(m); - cnt = 0; - - begin = end = UINT16_MAX; - for (int k = 0; k < BRIDGE_VLAN_BITMAP_LEN; k++) { - uint32_t untagged_map = br_untagged_bitmap[k]; - uint32_t vid_map = br_vid_bitmap[k]; - unsigned base_bit = k * 32; - int i = -1; - - done = false; - do { - int j = find_next_bit(i, vid_map); - if (j > 0) { - /* first hit of any bit */ - if (begin == UINT16_MAX && end == UINT16_MAX) { - begin = end = j - 1 + base_bit; - untagged = is_bit_set(j - 1, untagged_map); - goto next; - } - - /* this bit is a continuation of prior bits */ - if (j - 2 + base_bit == end && untagged == is_bit_set(j - 1, untagged_map) && (uint16_t)j - 1 + base_bit != pvid && (uint16_t)begin != pvid) { - end++; - goto next; - } - } else - done = true; + for (uint16_t k = 0; k < BRIDGE_VLAN_BITMAP_MAX; k++) { + if (k > 0 && k == link->network->pvid) { + /* PVID needs to be sent alone. Finish previous bits. */ if (begin != UINT16_MAX) { - cnt++; - if (done && k < BRIDGE_VLAN_BITMAP_LEN - 1) - break; + assert(begin < k); - br_vlan.flags = 0; - if (untagged) - br_vlan.flags |= BRIDGE_VLAN_INFO_UNTAGGED; + r = add_range(m, begin, k - 1, untagged); + if (r < 0) + return r; - if (begin == end) { - br_vlan.vid = begin; - - if (begin == pvid) - br_vlan.flags |= BRIDGE_VLAN_INFO_PVID; - - r = sd_netlink_message_append_data(req, IFLA_BRIDGE_VLAN_INFO, &br_vlan, sizeof(br_vlan)); - if (r < 0) - return r; - } else { - br_vlan.vid = begin; - br_vlan.flags |= BRIDGE_VLAN_INFO_RANGE_BEGIN; - - r = sd_netlink_message_append_data(req, IFLA_BRIDGE_VLAN_INFO, &br_vlan, sizeof(br_vlan)); - if (r < 0) - return r; - - br_vlan.vid = end; - br_vlan.flags &= ~BRIDGE_VLAN_INFO_RANGE_BEGIN; - br_vlan.flags |= BRIDGE_VLAN_INFO_RANGE_END; - - r = sd_netlink_message_append_data(req, IFLA_BRIDGE_VLAN_INFO, &br_vlan, sizeof(br_vlan)); - if (r < 0) - return r; - } - - if (done) - break; - } - if (j > 0) { - begin = end = j - 1 + base_bit; - untagged = is_bit_set(j - 1, untagged_map); + begin = UINT16_MAX; } - next: - i = j; - } while (!done); + untagged = is_bit_set(k, link->network->br_untagged_bitmap); + r = add_single(m, k, untagged, /* is_pvid = */ true); + if (r < 0) + return r; + + continue; + } + + if (!is_bit_set(k, link->network->br_vid_bitmap)) { + /* This bit is not set. Finish previous bits. */ + if (begin != UINT16_MAX) { + assert(begin < k); + + r = add_range(m, begin, k - 1, untagged); + if (r < 0) + return r; + + begin = UINT16_MAX; + } + + continue; + } + + if (begin != UINT16_MAX) { + bool u; + + assert(begin < k); + + u = is_bit_set(k, link->network->br_untagged_bitmap); + if (untagged == u) + continue; + + /* Tagging flag is changed from the previous bits. Finish them. */ + r = add_range(m, begin, k - 1, untagged); + if (r < 0) + return r; + + begin = k; + untagged = u; + continue; + } + + /* This is the starting point of a new bit sequence. Save the position and the tagging flag. */ + begin = k; + untagged = is_bit_set(k, link->network->br_untagged_bitmap); } - assert(cnt > 0); - return cnt; + /* No pending bit sequence. + * Why? There is a trick. The conf parsers below only accepts vlan ID in the range 0…4094, but in + * the above loop, we run 0…4095. */ + assert_cc(BRIDGE_VLAN_BITMAP_MAX > VLANID_MAX); + assert(begin == UINT16_MAX); + return 0; +} + +int bridge_vlan_set_message(Link *link, sd_netlink_message *m) { + int r; + + assert(link); + assert(m); + + r = sd_rtnl_message_link_set_family(m, AF_BRIDGE); + if (r < 0) + return r; + + r = sd_netlink_message_open_container(m, IFLA_AF_SPEC); + if (r < 0) + return r; + + if (link->master_ifindex <= 0) { + /* master needs BRIDGE_FLAGS_SELF flag */ + r = sd_netlink_message_append_u16(m, IFLA_BRIDGE_FLAGS, BRIDGE_FLAGS_SELF); + if (r < 0) + return r; + } + + r = bridge_vlan_append_set_info(link, m); + if (r < 0) + return r; + + r = sd_netlink_message_close_container(m); + if (r < 0) + return r; + + return 0; } void network_adjust_bridge_vlan(Network *network) { diff --git a/src/network/networkd-bridge-vlan.h b/src/network/networkd-bridge-vlan.h index f44b8101a55..af159e06c0e 100644 --- a/src/network/networkd-bridge-vlan.h +++ b/src/network/networkd-bridge-vlan.h @@ -19,12 +19,7 @@ typedef struct Network Network; void network_adjust_bridge_vlan(Network *network); -int bridge_vlan_append_info( - const Link * link, - sd_netlink_message *req, - uint16_t pvid, - const uint32_t *br_vid_bitmap, - const uint32_t *br_untagged_bitmap); +int bridge_vlan_set_message(Link *link, sd_netlink_message *m); CONFIG_PARSER_PROTOTYPE(config_parse_brvlan_pvid); CONFIG_PARSER_PROTOTYPE(config_parse_brvlan_vlan); diff --git a/src/network/networkd-setlink.c b/src/network/networkd-setlink.c index 541c4b8a728..ae8db4d8c8a 100644 --- a/src/network/networkd-setlink.c +++ b/src/network/networkd-setlink.c @@ -326,29 +326,9 @@ static int link_configure_fill_message( return r; break; case REQUEST_TYPE_SET_LINK_BRIDGE_VLAN: - r = sd_rtnl_message_link_set_family(req, AF_BRIDGE); + r = bridge_vlan_set_message(link, req); if (r < 0) return r; - - r = sd_netlink_message_open_container(req, IFLA_AF_SPEC); - if (r < 0) - return r; - - if (link->master_ifindex <= 0) { - /* master needs BRIDGE_FLAGS_SELF flag */ - r = sd_netlink_message_append_u16(req, IFLA_BRIDGE_FLAGS, BRIDGE_FLAGS_SELF); - if (r < 0) - return r; - } - - r = bridge_vlan_append_info(link, req, link->network->pvid, link->network->br_vid_bitmap, link->network->br_untagged_bitmap); - if (r < 0) - return r; - - r = sd_netlink_message_close_container(req); - if (r < 0) - return r; - break; case REQUEST_TYPE_SET_LINK_CAN: r = can_set_netlink_message(link, req); From 0b8d501c2796717440b8339cb0df90916ff814d8 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 11 Nov 2023 19:28:24 +0900 Subject: [PATCH 3/8] network/brvlan: parse_vlanid() accepts zero, hence PVID may be zero So, the default value should not be zero. --- src/network/networkd-bridge-vlan.c | 4 ++-- src/network/networkd-network.c | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/network/networkd-bridge-vlan.c b/src/network/networkd-bridge-vlan.c index bfc404f154d..969a7da30b4 100644 --- a/src/network/networkd-bridge-vlan.c +++ b/src/network/networkd-bridge-vlan.c @@ -83,7 +83,7 @@ static int bridge_vlan_append_set_info(Link *link, sd_netlink_message *m) { for (uint16_t k = 0; k < BRIDGE_VLAN_BITMAP_MAX; k++) { - if (k > 0 && k == link->network->pvid) { + if (k == link->network->pvid) { /* PVID needs to be sent alone. Finish previous bits. */ if (begin != UINT16_MAX) { assert(begin < k); @@ -189,7 +189,7 @@ void network_adjust_bridge_vlan(Network *network) { return; /* pvid might not be in br_vid_bitmap yet */ - if (network->pvid) + if (vlanid_is_valid(network->pvid)) set_bit(network->pvid, network->br_vid_bitmap); } diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c index 72ed2abd957..e3735842e26 100644 --- a/src/network/networkd-network.c +++ b/src/network/networkd-network.c @@ -450,6 +450,8 @@ int network_load_one(Manager *manager, OrderedHashmap **networks, const char *fi .priority = LINK_BRIDGE_PORT_PRIORITY_INVALID, .multicast_router = _MULTICAST_ROUTER_INVALID, + .pvid = UINT16_MAX, + .lldp_mode = LLDP_MODE_ROUTERS_ONLY, .lldp_multicast_mode = _SD_LLDP_MULTICAST_MODE_INVALID, From f0f93d9ce4b6be49cea8ea80821282402a862c18 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 12 Nov 2023 15:15:18 +0900 Subject: [PATCH 4/8] network/brvlan: convert condtion to assertion The condition should be always true. --- src/network/networkd-bridge-vlan.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/network/networkd-bridge-vlan.c b/src/network/networkd-bridge-vlan.c index 969a7da30b4..a3d90651e5b 100644 --- a/src/network/networkd-bridge-vlan.c +++ b/src/network/networkd-bridge-vlan.c @@ -23,8 +23,8 @@ static bool is_bit_set(unsigned nr, const uint32_t *addr) { } static void set_bit(unsigned nr, uint32_t *addr) { - if (nr < BRIDGE_VLAN_BITMAP_MAX) - addr[nr / 32] |= (UINT32_C(1) << (nr % 32)); + assert(nr < BRIDGE_VLAN_BITMAP_MAX); + addr[nr / 32] |= (UINT32_C(1) << (nr % 32)); } static int add_single(sd_netlink_message *m, uint16_t id, bool untagged, bool is_pvid) { From f269016c3e447d5373000e9837a9821ae81a2d87 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 12 Nov 2023 19:26:06 +0900 Subject: [PATCH 5/8] network/brvlan: make [BridgeVLAN] settings support an empty string This also renames Network.pvid and friends. --- man/systemd.network.xml | 11 ++- src/network/networkd-bridge-vlan.c | 101 +++++++++-------------- src/network/networkd-bridge-vlan.h | 5 +- src/network/networkd-network-gperf.gperf | 6 +- src/network/networkd-network.c | 2 +- src/network/networkd-network.h | 7 +- src/network/networkd-setlink.c | 3 +- 7 files changed, 58 insertions(+), 77 deletions(-) diff --git a/man/systemd.network.xml b/man/systemd.network.xml index fe8c8a14c6a..886258fbd04 100644 --- a/man/systemd.network.xml +++ b/man/systemd.network.xml @@ -5858,8 +5858,9 @@ ServerAddress=192.168.0.1/24 VLAN= - The VLAN ID allowed on the port. This can be either a single ID or a range M-N. Takes - an integer in the range 1…4094. + The VLAN ID allowed on the port. This can be either a single ID or a range M-N. Takes an + integer in the range 1…4094. This setting can be specified multiple times. If an empty string is + assigned, then the all previous assignments are cleared. @@ -5868,8 +5869,10 @@ ServerAddress=192.168.0.1/24 EgressUntagged= The VLAN ID specified here will be used to untag frames on egress. Configuring - EgressUntagged= implicates the use of VLAN= above and will enable the - VLAN ID for ingress as well. This can be either a single ID or a range M-N. + EgressUntagged= implicates the use of VLAN= above and will + enable the VLAN ID for ingress as well. This can be either a single ID or a range M-N. This + setting can be specified multiple times. If an empty string is assigned, then the all previous + assignments are cleared. diff --git a/src/network/networkd-bridge-vlan.c b/src/network/networkd-bridge-vlan.c index a3d90651e5b..6c8ec286fc0 100644 --- a/src/network/networkd-bridge-vlan.c +++ b/src/network/networkd-bridge-vlan.c @@ -83,7 +83,7 @@ static int bridge_vlan_append_set_info(Link *link, sd_netlink_message *m) { for (uint16_t k = 0; k < BRIDGE_VLAN_BITMAP_MAX; k++) { - if (k == link->network->pvid) { + if (k == link->network->bridge_vlan_pvid) { /* PVID needs to be sent alone. Finish previous bits. */ if (begin != UINT16_MAX) { assert(begin < k); @@ -95,7 +95,7 @@ static int bridge_vlan_append_set_info(Link *link, sd_netlink_message *m) { begin = UINT16_MAX; } - untagged = is_bit_set(k, link->network->br_untagged_bitmap); + untagged = is_bit_set(k, link->network->bridge_vlan_untagged_bitmap); r = add_single(m, k, untagged, /* is_pvid = */ true); if (r < 0) return r; @@ -103,7 +103,7 @@ static int bridge_vlan_append_set_info(Link *link, sd_netlink_message *m) { continue; } - if (!is_bit_set(k, link->network->br_vid_bitmap)) { + if (!is_bit_set(k, link->network->bridge_vlan_bitmap)) { /* This bit is not set. Finish previous bits. */ if (begin != UINT16_MAX) { assert(begin < k); @@ -123,7 +123,7 @@ static int bridge_vlan_append_set_info(Link *link, sd_netlink_message *m) { assert(begin < k); - u = is_bit_set(k, link->network->br_untagged_bitmap); + u = is_bit_set(k, link->network->bridge_vlan_untagged_bitmap); if (untagged == u) continue; @@ -139,7 +139,7 @@ static int bridge_vlan_append_set_info(Link *link, sd_netlink_message *m) { /* This is the starting point of a new bit sequence. Save the position and the tagging flag. */ begin = k; - untagged = is_bit_set(k, link->network->br_untagged_bitmap); + untagged = is_bit_set(k, link->network->bridge_vlan_untagged_bitmap); } /* No pending bit sequence. @@ -185,15 +185,15 @@ int bridge_vlan_set_message(Link *link, sd_netlink_message *m) { void network_adjust_bridge_vlan(Network *network) { assert(network); - if (!network->use_br_vlan) - return; + for (uint16_t k = 0; k < BRIDGE_VLAN_BITMAP_MAX; k++) + if (is_bit_set(k, network->bridge_vlan_untagged_bitmap)) + set_bit(k, network->bridge_vlan_bitmap); - /* pvid might not be in br_vid_bitmap yet */ - if (vlanid_is_valid(network->pvid)) - set_bit(network->pvid, network->br_vid_bitmap); + if (vlanid_is_valid(network->bridge_vlan_pvid)) + set_bit(network->bridge_vlan_pvid, network->bridge_vlan_bitmap); } -int config_parse_brvlan_pvid( +int config_parse_bridge_vlan_id( const char *unit, const char *filename, unsigned line, @@ -205,21 +205,32 @@ int config_parse_brvlan_pvid( void *data, void *userdata) { - Network *network = userdata; - uint16_t pvid; + uint16_t v, *id = ASSERT_PTR(data); int r; - r = parse_vlanid(rvalue, &pvid); - if (r < 0) - return r; + assert(filename); + assert(section); + assert(lvalue); + assert(rvalue); - network->pvid = pvid; - network->use_br_vlan = true; + if (isempty(rvalue)) { + *id = UINT16_MAX; + return 0; + } + r = parse_vlanid(rvalue, &v); + if (r < 0) { + log_syntax(unit, LOG_WARNING, filename, line, r, + "Failed to parse %s=, ignoring: %s", + lvalue, rvalue); + return 0; + } + + *id = v; return 0; } -int config_parse_brvlan_vlan( +int config_parse_bridge_vlan_id_range( const char *unit, const char *filename, unsigned line, @@ -231,7 +242,7 @@ int config_parse_brvlan_vlan( void *data, void *userdata) { - Network *network = userdata; + uint32_t *bitmap = ASSERT_PTR(data); uint16_t vid, vid_end; int r; @@ -239,54 +250,22 @@ int config_parse_brvlan_vlan( assert(section); assert(lvalue); assert(rvalue); - assert(data); + + if (isempty(rvalue)) { + memzero(bitmap, BRIDGE_VLAN_BITMAP_LEN * sizeof(uint32_t)); + return 0; + } r = parse_vid_range(rvalue, &vid, &vid_end); if (r < 0) { - log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to parse VLAN, ignoring: %s", rvalue); + log_syntax(unit, LOG_WARNING, filename, line, r, + "Failed to parse %s=, ignoring: %s", + lvalue, rvalue); return 0; } for (; vid <= vid_end; vid++) - set_bit(vid, network->br_vid_bitmap); + set_bit(vid, bitmap); - network->use_br_vlan = true; - return 0; -} - -int config_parse_brvlan_untagged( - const char *unit, - const char *filename, - unsigned line, - const char *section, - unsigned section_line, - const char *lvalue, - int ltype, - const char *rvalue, - void *data, - void *userdata) { - - Network *network = userdata; - uint16_t vid, vid_end; - int r; - - assert(filename); - assert(section); - assert(lvalue); - assert(rvalue); - assert(data); - - r = parse_vid_range(rvalue, &vid, &vid_end); - if (r < 0) { - log_syntax(unit, LOG_WARNING, filename, line, r, "Could not parse VLAN: %s", rvalue); - return 0; - } - - for (; vid <= vid_end; vid++) { - set_bit(vid, network->br_vid_bitmap); - set_bit(vid, network->br_untagged_bitmap); - } - - network->use_br_vlan = true; return 0; } diff --git a/src/network/networkd-bridge-vlan.h b/src/network/networkd-bridge-vlan.h index af159e06c0e..f6a6807be09 100644 --- a/src/network/networkd-bridge-vlan.h +++ b/src/network/networkd-bridge-vlan.h @@ -21,6 +21,5 @@ void network_adjust_bridge_vlan(Network *network); int bridge_vlan_set_message(Link *link, sd_netlink_message *m); -CONFIG_PARSER_PROTOTYPE(config_parse_brvlan_pvid); -CONFIG_PARSER_PROTOTYPE(config_parse_brvlan_vlan); -CONFIG_PARSER_PROTOTYPE(config_parse_brvlan_untagged); +CONFIG_PARSER_PROTOTYPE(config_parse_bridge_vlan_id); +CONFIG_PARSER_PROTOTYPE(config_parse_bridge_vlan_id_range); diff --git a/src/network/networkd-network-gperf.gperf b/src/network/networkd-network-gperf.gperf index 628b1ad19f5..e101d2811b9 100644 --- a/src/network/networkd-network-gperf.gperf +++ b/src/network/networkd-network-gperf.gperf @@ -368,9 +368,9 @@ BridgeFDB.AssociatedWith, config_parse_fdb_ntf_flags, BridgeFDB.OutgoingInterface, config_parse_fdb_interface, 0, 0 BridgeMDB.MulticastGroupAddress, config_parse_mdb_group_address, 0, 0 BridgeMDB.VLANId, config_parse_mdb_vlan_id, 0, 0 -BridgeVLAN.PVID, config_parse_brvlan_pvid, 0, 0 -BridgeVLAN.VLAN, config_parse_brvlan_vlan, 0, 0 -BridgeVLAN.EgressUntagged, config_parse_brvlan_untagged, 0, 0 +BridgeVLAN.PVID, config_parse_bridge_vlan_id, 0, offsetof(Network, bridge_vlan_pvid) +BridgeVLAN.VLAN, config_parse_bridge_vlan_id_range, 0, offsetof(Network, bridge_vlan_bitmap) +BridgeVLAN.EgressUntagged, config_parse_bridge_vlan_id_range, 0, offsetof(Network, bridge_vlan_untagged_bitmap) DHCPPrefixDelegation.UplinkInterface, config_parse_uplink, 0, 0 DHCPPrefixDelegation.SubnetId, config_parse_dhcp_pd_subnet_id, 0, offsetof(Network, dhcp_pd_subnet_id) DHCPPrefixDelegation.Announce, config_parse_bool, 0, offsetof(Network, dhcp_pd_announce) diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c index e3735842e26..654fd7bb9ee 100644 --- a/src/network/networkd-network.c +++ b/src/network/networkd-network.c @@ -450,7 +450,7 @@ int network_load_one(Manager *manager, OrderedHashmap **networks, const char *fi .priority = LINK_BRIDGE_PORT_PRIORITY_INVALID, .multicast_router = _MULTICAST_ROUTER_INVALID, - .pvid = UINT16_MAX, + .bridge_vlan_pvid = UINT16_MAX, .lldp_mode = LLDP_MODE_ROUTERS_ONLY, .lldp_multicast_mode = _SD_LLDP_MULTICAST_MODE_INVALID, diff --git a/src/network/networkd-network.h b/src/network/networkd-network.h index 4995e55b531..80a46c488ec 100644 --- a/src/network/networkd-network.h +++ b/src/network/networkd-network.h @@ -289,10 +289,9 @@ struct Network { MulticastRouter multicast_router; /* Bridge VLAN */ - bool use_br_vlan; - uint16_t pvid; - uint32_t br_vid_bitmap[BRIDGE_VLAN_BITMAP_LEN]; - uint32_t br_untagged_bitmap[BRIDGE_VLAN_BITMAP_LEN]; + uint16_t bridge_vlan_pvid; + uint32_t bridge_vlan_bitmap[BRIDGE_VLAN_BITMAP_LEN]; + uint32_t bridge_vlan_untagged_bitmap[BRIDGE_VLAN_BITMAP_LEN]; /* CAN support */ uint32_t can_bitrate; diff --git a/src/network/networkd-setlink.c b/src/network/networkd-setlink.c index ae8db4d8c8a..3e1d904836f 100644 --- a/src/network/networkd-setlink.c +++ b/src/network/networkd-setlink.c @@ -687,7 +687,8 @@ int link_request_to_set_bridge_vlan(Link *link) { assert(link); assert(link->network); - if (!link->network->use_br_vlan) + /* If nothing configured, use the default vlan ID. */ + if (memeqzero(link->network->bridge_vlan_bitmap, BRIDGE_VLAN_BITMAP_LEN * sizeof(uint32_t))) return 0; if (!link->network->bridge && !streq_ptr(link->kind, "bridge")) { From 11cee6efbf54aeff25a6d106f6707ce822fa72d0 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 13 Nov 2023 02:13:03 +0900 Subject: [PATCH 6/8] network/brvlan: read bridge vlan IDs through netlink and save them In this commit, obtained vlan IDs are not used, but they will be used in the later commits. --- src/network/networkd-bridge-vlan.c | 61 ++++++++++++++++++++++++++++++ src/network/networkd-bridge-vlan.h | 2 + src/network/networkd-link.c | 7 ++++ src/network/networkd-link.h | 6 +++ src/network/networkd-manager.c | 14 +++++++ 5 files changed, 90 insertions(+) diff --git a/src/network/networkd-bridge-vlan.c b/src/network/networkd-bridge-vlan.c index 6c8ec286fc0..c38ae16a47d 100644 --- a/src/network/networkd-bridge-vlan.c +++ b/src/network/networkd-bridge-vlan.c @@ -182,6 +182,67 @@ int bridge_vlan_set_message(Link *link, sd_netlink_message *m) { return 0; } +#define RTA_TYPE(rta) ((rta)->rta_type & NLA_TYPE_MASK) + +int link_update_bridge_vlan(Link *link, sd_netlink_message *m) { + _cleanup_free_ void *data = NULL; + size_t len; + uint16_t begin = UINT16_MAX; + int r, family; + + assert(link); + assert(m); + + r = sd_rtnl_message_get_family(m, &family); + if (r < 0) + return r; + + if (family != AF_BRIDGE) + return 0; + + r = sd_netlink_message_read_data(m, IFLA_AF_SPEC, &len, &data); + if (r == -ENODATA) + return 0; + if (r < 0) + return r; + + memzero(link->bridge_vlan_bitmap, sizeof(link->bridge_vlan_bitmap)); + + for (struct rtattr *rta = data; RTA_OK(rta, len); rta = RTA_NEXT(rta, len)) { + struct bridge_vlan_info *p; + + if (RTA_TYPE(rta) != IFLA_BRIDGE_VLAN_INFO) + continue; + if (RTA_PAYLOAD(rta) != sizeof(struct bridge_vlan_info)) + continue; + + p = RTA_DATA(rta); + + if (FLAGS_SET(p->flags, BRIDGE_VLAN_INFO_RANGE_BEGIN)) { + begin = p->vid; + continue; + } + + if (FLAGS_SET(p->flags, BRIDGE_VLAN_INFO_RANGE_END)) { + for (uint16_t k = begin; k <= p->vid; k++) + set_bit(k, link->bridge_vlan_bitmap); + + begin = UINT16_MAX; + continue; + } + + if (FLAGS_SET(p->flags, BRIDGE_VLAN_INFO_PVID)) { + link->bridge_vlan_pvid = p->vid; + link->bridge_vlan_pvid_is_untagged = FLAGS_SET(p->flags, BRIDGE_VLAN_INFO_UNTAGGED); + } + + set_bit(p->vid, link->bridge_vlan_bitmap); + begin = UINT16_MAX; + } + + return 0; +} + void network_adjust_bridge_vlan(Network *network) { assert(network); diff --git a/src/network/networkd-bridge-vlan.h b/src/network/networkd-bridge-vlan.h index f6a6807be09..43d365e7457 100644 --- a/src/network/networkd-bridge-vlan.h +++ b/src/network/networkd-bridge-vlan.h @@ -21,5 +21,7 @@ void network_adjust_bridge_vlan(Network *network); int bridge_vlan_set_message(Link *link, sd_netlink_message *m); +int link_update_bridge_vlan(Link *link, sd_netlink_message *m); + CONFIG_PARSER_PROTOTYPE(config_parse_bridge_vlan_id); CONFIG_PARSER_PROTOTYPE(config_parse_bridge_vlan_id_range); diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 2caf4ff2495..aad86eed991 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -35,6 +35,7 @@ #include "networkd-address.h" #include "networkd-bridge-fdb.h" #include "networkd-bridge-mdb.h" +#include "networkd-bridge-vlan.h" #include "networkd-can.h" #include "networkd-dhcp-prefix-delegation.h" #include "networkd-dhcp-server.h" @@ -2435,6 +2436,10 @@ static int link_update(Link *link, sd_netlink_message *message) { if (r < 0) return r; + r = link_update_bridge_vlan(link, message); + if (r < 0) + return r; + return needs_reconfigure; } @@ -2508,6 +2513,8 @@ static int link_new(Manager *manager, sd_netlink_message *message, Link **ret) { .ifname = TAKE_PTR(ifname), .kind = TAKE_PTR(kind), + .bridge_vlan_pvid = UINT16_MAX, + .ipv6ll_address_gen_mode = _IPV6_LINK_LOCAL_ADDRESS_GEN_MODE_INVALID, .state_file = TAKE_PTR(state_file), diff --git a/src/network/networkd-link.h b/src/network/networkd-link.h index 938bbf482ec..a6c2f2d7202 100644 --- a/src/network/networkd-link.h +++ b/src/network/networkd-link.h @@ -21,6 +21,7 @@ #include "log-link.h" #include "netif-util.h" #include "network-util.h" +#include "networkd-bridge-vlan.h" #include "networkd-ipv6ll.h" #include "networkd-util.h" #include "ordered-set.h" @@ -72,6 +73,11 @@ typedef struct Link { sd_device *dev; char *driver; + /* bridge vlan */ + uint16_t bridge_vlan_pvid; + bool bridge_vlan_pvid_is_untagged; + uint32_t bridge_vlan_bitmap[BRIDGE_VLAN_BITMAP_LEN]; + /* to prevent multiple ethtool calls */ bool ethtool_driver_read; bool ethtool_permanent_hw_addr_read; diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index 1ca1d7abe5e..bdf6088f2f4 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -753,6 +753,20 @@ static int manager_enumerate_links(Manager *m) { if (r < 0) return r; + r = manager_enumerate_internal(m, m->rtnl, req, manager_rtnl_process_link); + if (r < 0) + return r; + + req = sd_netlink_message_unref(req); + + r = sd_rtnl_message_new_link(m->rtnl, &req, RTM_GETLINK, 0); + if (r < 0) + return r; + + r = sd_rtnl_message_link_set_family(req, AF_BRIDGE); + if (r < 0) + return r; + return manager_enumerate_internal(m, m->rtnl, req, manager_rtnl_process_link); } From 228693af47ffb43098b5cee41f5624b8351d9efa Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 12 Nov 2023 16:04:19 +0900 Subject: [PATCH 7/8] network/brvlan: remove unnecessary bridge vlan IDs When an interface is being reconfigured with different bridge vlan settings, this makes old vlan IDs on the interface removed. This also makes the PVID= setting support negative boolean value, e.g. "no", in which case, the currently assigned PVID (typically, assigned by the kernel when an interface is joined to a bridge) is dropped. This feature is requested by #15291. Note, if a .network file has no settings about bridge vlan, networkd keeps the currently assigned vlan IDs. That's intended, to make not break existing setups. When a .network file has only PVID=no line in [BridgeVLAN] section, then all assigned vlan IDs are removed. Fixes #29975. Closes #15291. --- man/systemd.network.xml | 8 ++- src/network/networkd-bridge-vlan.c | 88 +++++++++++++++++++++++++++--- src/network/networkd-bridge-vlan.h | 8 ++- src/network/networkd-link.h | 1 + src/network/networkd-network.c | 2 +- src/network/networkd-queue.c | 3 +- src/network/networkd-queue.h | 1 + src/network/networkd-setlink.c | 49 +++++++++++++++-- 8 files changed, 140 insertions(+), 20 deletions(-) diff --git a/man/systemd.network.xml b/man/systemd.network.xml index 886258fbd04..045ad271f8c 100644 --- a/man/systemd.network.xml +++ b/man/systemd.network.xml @@ -5880,9 +5880,11 @@ ServerAddress=192.168.0.1/24 PVID= - The Port VLAN ID specified here is assigned to all untagged frames at ingress. - PVID= can be used only once. Configuring PVID= implicates the use of - VLAN= above and will enable the VLAN ID for ingress as well. + The port VLAN ID specified here is assigned to all untagged frames at ingress. Takes an + VLAN ID or negative boolean value (e.g. no). When false, the currently + assigned port VLAN ID will be dropped. Configuring PVID= implicates the use of + VLAN= setting in the above and will enable the VLAN ID for ingress as well. + Defaults to unset, and will keep the assigned port VLAN ID if exists. diff --git a/src/network/networkd-bridge-vlan.c b/src/network/networkd-bridge-vlan.c index c38ae16a47d..d84b6b34f63 100644 --- a/src/network/networkd-bridge-vlan.c +++ b/src/network/networkd-bridge-vlan.c @@ -72,18 +72,42 @@ static int add_range(sd_netlink_message *m, uint16_t begin, uint16_t end, bool u return 0; } +static uint16_t link_get_pvid(Link *link, bool *ret_untagged) { + assert(link); + assert(link->network); + + if (vlanid_is_valid(link->network->bridge_vlan_pvid)) { + if (ret_untagged) + *ret_untagged = is_bit_set(link->network->bridge_vlan_pvid, + link->network->bridge_vlan_untagged_bitmap); + return link->network->bridge_vlan_pvid; + } + + if (link->network->bridge_vlan_pvid == BRIDGE_VLAN_KEEP_PVID) { + if (ret_untagged) + *ret_untagged = link->bridge_vlan_pvid_is_untagged; + return link->bridge_vlan_pvid; + } + + if (ret_untagged) + *ret_untagged = false; + return UINT16_MAX; +} + static int bridge_vlan_append_set_info(Link *link, sd_netlink_message *m) { - uint16_t begin = UINT16_MAX; - bool untagged; + uint16_t pvid, begin = UINT16_MAX; + bool untagged, pvid_is_untagged; int r; assert(link); assert(link->network); assert(m); + pvid = link_get_pvid(link, &pvid_is_untagged); + for (uint16_t k = 0; k < BRIDGE_VLAN_BITMAP_MAX; k++) { - if (k == link->network->bridge_vlan_pvid) { + if (k == pvid) { /* PVID needs to be sent alone. Finish previous bits. */ if (begin != UINT16_MAX) { assert(begin < k); @@ -95,8 +119,7 @@ static int bridge_vlan_append_set_info(Link *link, sd_netlink_message *m) { begin = UINT16_MAX; } - untagged = is_bit_set(k, link->network->bridge_vlan_untagged_bitmap); - r = add_single(m, k, untagged, /* is_pvid = */ true); + r = add_single(m, pvid, pvid_is_untagged, /* is_pvid = */ true); if (r < 0) return r; @@ -150,7 +173,48 @@ static int bridge_vlan_append_set_info(Link *link, sd_netlink_message *m) { return 0; } -int bridge_vlan_set_message(Link *link, sd_netlink_message *m) { +static int bridge_vlan_append_del_info(Link *link, sd_netlink_message *m) { + uint16_t pvid, begin = UINT16_MAX; + int r; + + assert(link); + assert(link->network); + assert(m); + + pvid = link_get_pvid(link, NULL); + + for (uint16_t k = 0; k < BRIDGE_VLAN_BITMAP_MAX; k++) { + + if (k == pvid || + !is_bit_set(k, link->bridge_vlan_bitmap) || + is_bit_set(k, link->network->bridge_vlan_bitmap)) { + /* This bit is not necessary to be removed. Finish previous bits. */ + if (begin != UINT16_MAX) { + assert(begin < k); + + r = add_range(m, begin, k - 1, /* untagged = */ false); + if (r < 0) + return r; + + begin = UINT16_MAX; + } + + continue; + } + + if (begin != UINT16_MAX) + continue; + + /* This is the starting point of a new bit sequence. Save the position. */ + begin = k; + } + + /* No pending bit sequence. */ + assert(begin == UINT16_MAX); + return 0; +} + +int bridge_vlan_set_message(Link *link, sd_netlink_message *m, bool is_set) { int r; assert(link); @@ -171,7 +235,10 @@ int bridge_vlan_set_message(Link *link, sd_netlink_message *m) { return r; } - r = bridge_vlan_append_set_info(link, m); + if (is_set) + r = bridge_vlan_append_set_info(link, m); + else + r = bridge_vlan_append_del_info(link, m); if (r < 0) return r; @@ -275,7 +342,12 @@ int config_parse_bridge_vlan_id( assert(rvalue); if (isempty(rvalue)) { - *id = UINT16_MAX; + *id = BRIDGE_VLAN_KEEP_PVID; + return 0; + } + + if (parse_boolean(rvalue) == 0) { + *id = BRIDGE_VLAN_REMOVE_PVID; return 0; } diff --git a/src/network/networkd-bridge-vlan.h b/src/network/networkd-bridge-vlan.h index 43d365e7457..0366cc6feea 100644 --- a/src/network/networkd-bridge-vlan.h +++ b/src/network/networkd-bridge-vlan.h @@ -6,20 +6,26 @@ ***/ #include +#include #include "sd-netlink.h" #include "conf-parser.h" +#include "vlan-util.h" #define BRIDGE_VLAN_BITMAP_MAX 4096 #define BRIDGE_VLAN_BITMAP_LEN (BRIDGE_VLAN_BITMAP_MAX / 32) +#define BRIDGE_VLAN_KEEP_PVID UINT16_MAX +#define BRIDGE_VLAN_REMOVE_PVID (UINT16_MAX - 1) +assert_cc(BRIDGE_VLAN_REMOVE_PVID > VLANID_MAX); + typedef struct Link Link; typedef struct Network Network; void network_adjust_bridge_vlan(Network *network); -int bridge_vlan_set_message(Link *link, sd_netlink_message *m); +int bridge_vlan_set_message(Link *link, sd_netlink_message *m, bool is_set); int link_update_bridge_vlan(Link *link, sd_netlink_message *m); diff --git a/src/network/networkd-link.h b/src/network/networkd-link.h index a6c2f2d7202..7458ea93bd0 100644 --- a/src/network/networkd-link.h +++ b/src/network/networkd-link.h @@ -155,6 +155,7 @@ typedef struct Link { bool activated:1; bool master_set:1; bool stacked_netdevs_created:1; + bool bridge_vlan_set:1; sd_dhcp_server *dhcp_server; diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c index 654fd7bb9ee..d4401757bd2 100644 --- a/src/network/networkd-network.c +++ b/src/network/networkd-network.c @@ -450,7 +450,7 @@ int network_load_one(Manager *manager, OrderedHashmap **networks, const char *fi .priority = LINK_BRIDGE_PORT_PRIORITY_INVALID, .multicast_router = _MULTICAST_ROUTER_INVALID, - .bridge_vlan_pvid = UINT16_MAX, + .bridge_vlan_pvid = BRIDGE_VLAN_KEEP_PVID, .lldp_mode = LLDP_MODE_ROUTERS_ONLY, .lldp_multicast_mode = _SD_LLDP_MULTICAST_MODE_INVALID, diff --git a/src/network/networkd-queue.c b/src/network/networkd-queue.c index 90caefbc72c..cc439a74a83 100644 --- a/src/network/networkd-queue.c +++ b/src/network/networkd-queue.c @@ -303,7 +303,8 @@ static const char *const request_type_table[_REQUEST_TYPE_MAX] = { [REQUEST_TYPE_SET_LINK_ADDRESS_GENERATION_MODE] = "IPv6LL address generation mode", [REQUEST_TYPE_SET_LINK_BOND] = "bond configurations", [REQUEST_TYPE_SET_LINK_BRIDGE] = "bridge configurations", - [REQUEST_TYPE_SET_LINK_BRIDGE_VLAN] = "bridge VLAN configurations", + [REQUEST_TYPE_SET_LINK_BRIDGE_VLAN] = "bridge VLAN configurations (step 1)", + [REQUEST_TYPE_DEL_LINK_BRIDGE_VLAN] = "bridge VLAN configurations (step 2)", [REQUEST_TYPE_SET_LINK_CAN] = "CAN interface configurations", [REQUEST_TYPE_SET_LINK_FLAGS] = "link flags", [REQUEST_TYPE_SET_LINK_GROUP] = "interface group", diff --git a/src/network/networkd-queue.h b/src/network/networkd-queue.h index e58d1be66f5..d6f5de421e2 100644 --- a/src/network/networkd-queue.h +++ b/src/network/networkd-queue.h @@ -37,6 +37,7 @@ typedef enum RequestType { REQUEST_TYPE_SET_LINK_BOND, /* Setting bond configs. */ REQUEST_TYPE_SET_LINK_BRIDGE, /* Setting bridge configs. */ REQUEST_TYPE_SET_LINK_BRIDGE_VLAN, /* Setting bridge VLAN configs. */ + REQUEST_TYPE_DEL_LINK_BRIDGE_VLAN, /* Removing bridge VLAN configs. */ REQUEST_TYPE_SET_LINK_CAN, /* Setting CAN interface configs. */ REQUEST_TYPE_SET_LINK_FLAGS, /* Setting IFF_NOARP or friends. */ REQUEST_TYPE_SET_LINK_GROUP, /* Setting interface group. */ diff --git a/src/network/networkd-setlink.c b/src/network/networkd-setlink.c index 3e1d904836f..2b37c86d235 100644 --- a/src/network/networkd-setlink.c +++ b/src/network/networkd-setlink.c @@ -103,6 +103,19 @@ static int link_set_bridge_handler(sd_netlink *rtnl, sd_netlink_message *m, Requ } static int link_set_bridge_vlan_handler(sd_netlink *rtnl, sd_netlink_message *m, Request *req, Link *link, void *userdata) { + int r; + + assert(link); + + r = set_link_handler_internal(rtnl, m, req, link, /* ignore = */ false, NULL); + if (r <= 0) + return r; + + link->bridge_vlan_set = true; + return 0; +} + +static int link_del_bridge_vlan_handler(sd_netlink *rtnl, sd_netlink_message *m, Request *req, Link *link, void *userdata) { return set_link_handler_internal(rtnl, m, req, link, /* ignore = */ false, NULL); } @@ -326,7 +339,12 @@ static int link_configure_fill_message( return r; break; case REQUEST_TYPE_SET_LINK_BRIDGE_VLAN: - r = bridge_vlan_set_message(link, req); + r = bridge_vlan_set_message(link, req, /* is_set = */ true); + if (r < 0) + return r; + break; + case REQUEST_TYPE_DEL_LINK_BRIDGE_VLAN: + r = bridge_vlan_set_message(link, req, /* is_set = */ false); if (r < 0) return r; break; @@ -410,6 +428,8 @@ static int link_configure(Link *link, Request *req) { r = sd_rtnl_message_new_link(link->manager->rtnl, &m, RTM_NEWLINK, link->master_ifindex); else if (IN_SET(req->type, REQUEST_TYPE_SET_LINK_CAN, REQUEST_TYPE_SET_LINK_IPOIB)) r = sd_rtnl_message_new_link(link->manager->rtnl, &m, RTM_NEWLINK, link->ifindex); + else if (req->type == REQUEST_TYPE_DEL_LINK_BRIDGE_VLAN) + r = sd_rtnl_message_new_link(link->manager->rtnl, &m, RTM_DELLINK, link->ifindex); else r = sd_rtnl_message_new_link(link->manager->rtnl, &m, RTM_SETLINK, link->ifindex); if (r < 0) @@ -460,9 +480,11 @@ static int link_is_ready_to_set_link(Link *link, Request *req) { if (link->network->keep_master && link->master_ifindex <= 0 && !streq_ptr(link->kind, "bridge")) return false; - break; + case REQUEST_TYPE_DEL_LINK_BRIDGE_VLAN: + return link->bridge_vlan_set; + case REQUEST_TYPE_SET_LINK_CAN: /* Do not check link->set_flgas_messages here, as it is ok even if link->flags * is outdated, and checking the counter causes a deadlock. */ @@ -684,11 +706,14 @@ int link_request_to_set_bridge(Link *link) { } int link_request_to_set_bridge_vlan(Link *link) { + int r; + assert(link); assert(link->network); /* If nothing configured, use the default vlan ID. */ - if (memeqzero(link->network->bridge_vlan_bitmap, BRIDGE_VLAN_BITMAP_LEN * sizeof(uint32_t))) + if (memeqzero(link->network->bridge_vlan_bitmap, BRIDGE_VLAN_BITMAP_LEN * sizeof(uint32_t)) && + link->network->bridge_vlan_pvid == BRIDGE_VLAN_KEEP_PVID) return 0; if (!link->network->bridge && !streq_ptr(link->kind, "bridge")) { @@ -704,9 +729,21 @@ int link_request_to_set_bridge_vlan(Link *link) { return 0; } - return link_request_set_link(link, REQUEST_TYPE_SET_LINK_BRIDGE_VLAN, - link_set_bridge_vlan_handler, - NULL); + link->bridge_vlan_set = false; + + r = link_request_set_link(link, REQUEST_TYPE_SET_LINK_BRIDGE_VLAN, + link_set_bridge_vlan_handler, + NULL); + if (r < 0) + return r; + + r = link_request_set_link(link, REQUEST_TYPE_DEL_LINK_BRIDGE_VLAN, + link_del_bridge_vlan_handler, + NULL); + if (r < 0) + return r; + + return 0; } int link_request_to_set_can(Link *link) { From 60f4b2c560ce930bdec0da615655b0d77d75d772 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 12 Nov 2023 20:06:39 +0900 Subject: [PATCH 8/8] test-network: extend tests for [BridgeVLAN] settings --- .../conf/26-bridge-vlan-master.network | 6 +- .../10-override.conf | 11 ++ .../20-override.conf | 9 ++ .../30-override.conf | 5 + .../conf/26-bridge-vlan-slave.network | 6 +- .../10-override.conf | 11 ++ .../20-override.conf | 9 ++ .../30-override.conf | 5 + test/test-network/systemd-networkd-tests.py | 111 ++++++++++++++++-- 9 files changed, 162 insertions(+), 11 deletions(-) create mode 100644 test/test-network/conf/26-bridge-vlan-master.network.d/10-override.conf create mode 100644 test/test-network/conf/26-bridge-vlan-master.network.d/20-override.conf create mode 100644 test/test-network/conf/26-bridge-vlan-master.network.d/30-override.conf create mode 100644 test/test-network/conf/26-bridge-vlan-slave.network.d/10-override.conf create mode 100644 test/test-network/conf/26-bridge-vlan-slave.network.d/20-override.conf create mode 100644 test/test-network/conf/26-bridge-vlan-slave.network.d/30-override.conf diff --git a/test/test-network/conf/26-bridge-vlan-master.network b/test/test-network/conf/26-bridge-vlan-master.network index 4bbbc5660ac..90ae0b23dfc 100644 --- a/test/test-network/conf/26-bridge-vlan-master.network +++ b/test/test-network/conf/26-bridge-vlan-master.network @@ -6,4 +6,8 @@ Name=bridge99 IPv6AcceptRA=false [BridgeVLAN] -VLAN=4060-4094 +PVID=1020 +VLAN=1018-1023 +VLAN=1200-1210 +EgressUntagged=1022-1025 +EgressUntagged=1203-1208 diff --git a/test/test-network/conf/26-bridge-vlan-master.network.d/10-override.conf b/test/test-network/conf/26-bridge-vlan-master.network.d/10-override.conf new file mode 100644 index 00000000000..25d213f8880 --- /dev/null +++ b/test/test-network/conf/26-bridge-vlan-master.network.d/10-override.conf @@ -0,0 +1,11 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[BridgeVLAN] +PVID= +VLAN= +EgressUntagged= + +PVID=2020 +VLAN=2018-2023 +VLAN=2200-2210 +EgressUntagged=2022-2025 +EgressUntagged=2203-2208 diff --git a/test/test-network/conf/26-bridge-vlan-master.network.d/20-override.conf b/test/test-network/conf/26-bridge-vlan-master.network.d/20-override.conf new file mode 100644 index 00000000000..0dcaee0af00 --- /dev/null +++ b/test/test-network/conf/26-bridge-vlan-master.network.d/20-override.conf @@ -0,0 +1,9 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[BridgeVLAN] +PVID= +VLAN= +EgressUntagged= + +PVID=2020 +VLAN=2018-2023 +EgressUntagged=2022-2025 diff --git a/test/test-network/conf/26-bridge-vlan-master.network.d/30-override.conf b/test/test-network/conf/26-bridge-vlan-master.network.d/30-override.conf new file mode 100644 index 00000000000..409296f42a7 --- /dev/null +++ b/test/test-network/conf/26-bridge-vlan-master.network.d/30-override.conf @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[BridgeVLAN] +PVID=no +VLAN= +EgressUntagged= diff --git a/test/test-network/conf/26-bridge-vlan-slave.network b/test/test-network/conf/26-bridge-vlan-slave.network index 9ac851030dc..d23d0fc7d0a 100644 --- a/test/test-network/conf/26-bridge-vlan-slave.network +++ b/test/test-network/conf/26-bridge-vlan-slave.network @@ -7,4 +7,8 @@ IPv6AcceptRA=no Bridge=bridge99 [BridgeVLAN] -VLAN=4064-4094 +PVID=1010 +VLAN=1008-1013 +VLAN=1100-1110 +EgressUntagged=1012-1015 +EgressUntagged=1103-1108 diff --git a/test/test-network/conf/26-bridge-vlan-slave.network.d/10-override.conf b/test/test-network/conf/26-bridge-vlan-slave.network.d/10-override.conf new file mode 100644 index 00000000000..0511b6419ee --- /dev/null +++ b/test/test-network/conf/26-bridge-vlan-slave.network.d/10-override.conf @@ -0,0 +1,11 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[BridgeVLAN] +PVID= +VLAN= +EgressUntagged= + +PVID=2010 +VLAN=2008-2013 +VLAN=2100-2110 +EgressUntagged=2012-2015 +EgressUntagged=2103-2108 diff --git a/test/test-network/conf/26-bridge-vlan-slave.network.d/20-override.conf b/test/test-network/conf/26-bridge-vlan-slave.network.d/20-override.conf new file mode 100644 index 00000000000..58df686a316 --- /dev/null +++ b/test/test-network/conf/26-bridge-vlan-slave.network.d/20-override.conf @@ -0,0 +1,9 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[BridgeVLAN] +PVID= +VLAN= +EgressUntagged= + +PVID=2010 +VLAN=2008-2013 +EgressUntagged=2012-2015 diff --git a/test/test-network/conf/26-bridge-vlan-slave.network.d/30-override.conf b/test/test-network/conf/26-bridge-vlan-slave.network.d/30-override.conf new file mode 100644 index 00000000000..409296f42a7 --- /dev/null +++ b/test/test-network/conf/26-bridge-vlan-slave.network.d/30-override.conf @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[BridgeVLAN] +PVID=no +VLAN= +EgressUntagged= diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 041dfd313b8..077a84ab756 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -4415,23 +4415,116 @@ class NetworkdBridgeTests(unittest.TestCase, Utilities): def test_bridge_vlan(self): copy_network_unit('11-dummy.netdev', '26-bridge-vlan-slave.network', - '26-bridge.netdev', '26-bridge-vlan-master.network') + '26-bridge.netdev', '26-bridge-vlan-master.network', + copy_dropins=False) start_networkd() self.wait_online(['test1:enslaved', 'bridge99:degraded']) output = check_output('bridge vlan show dev test1') print(output) - self.assertNotRegex(output, '4063') - for i in range(4064, 4095): - self.assertRegex(output, f'{i}') - self.assertNotRegex(output, '4095') + # check if the default VID is removed + self.assertNotIn('1 Egress Untagged', output) + for i in range(1000, 3000): + if i == 1010: + self.assertIn(f'{i} PVID', output) + elif i in range(1012, 1016) or i in range(1103, 1109): + self.assertIn(f'{i} Egress Untagged', output) + elif i in range(1008, 1014) or i in range(1100, 1111): + self.assertIn(f'{i}', output) + else: + self.assertNotIn(f'{i}', output) output = check_output('bridge vlan show dev bridge99') print(output) - self.assertNotRegex(output, '4059') - for i in range(4060, 4095): - self.assertRegex(output, f'{i}') - self.assertNotRegex(output, '4095') + # check if the default VID is removed + self.assertNotIn('1 Egress Untagged', output) + for i in range(1000, 3000): + if i == 1020: + self.assertIn(f'{i} PVID', output) + elif i in range(1022, 1026) or i in range(1203, 1209): + self.assertIn(f'{i} Egress Untagged', output) + elif i in range(1018, 1024) or i in range(1200, 1211): + self.assertIn(f'{i}', output) + else: + self.assertNotIn(f'{i}', output) + + # Change vlan IDs + copy_network_unit('26-bridge-vlan-slave.network.d/10-override.conf', + '26-bridge-vlan-master.network.d/10-override.conf') + networkctl_reload() + self.wait_online(['test1:enslaved', 'bridge99:degraded']) + + output = check_output('bridge vlan show dev test1') + print(output) + for i in range(1000, 3000): + if i == 2010: + self.assertIn(f'{i} PVID', output) + elif i in range(2012, 2016) or i in range(2103, 2109): + self.assertIn(f'{i} Egress Untagged', output) + elif i in range(2008, 2014) or i in range(2100, 2111): + self.assertIn(f'{i}', output) + else: + self.assertNotIn(f'{i}', output) + + output = check_output('bridge vlan show dev bridge99') + print(output) + for i in range(1000, 3000): + if i == 2020: + self.assertIn(f'{i} PVID', output) + elif i in range(2022, 2026) or i in range(2203, 2209): + self.assertIn(f'{i} Egress Untagged', output) + elif i in range(2018, 2024) or i in range(2200, 2211): + self.assertIn(f'{i}', output) + else: + self.assertNotIn(f'{i}', output) + + # Remove several vlan IDs + copy_network_unit('26-bridge-vlan-slave.network.d/20-override.conf', + '26-bridge-vlan-master.network.d/20-override.conf') + networkctl_reload() + self.wait_online(['test1:enslaved', 'bridge99:degraded']) + + output = check_output('bridge vlan show dev test1') + print(output) + for i in range(1000, 3000): + if i == 2010: + self.assertIn(f'{i} PVID', output) + elif i in range(2012, 2016): + self.assertIn(f'{i} Egress Untagged', output) + elif i in range(2008, 2014): + self.assertIn(f'{i}', output) + else: + self.assertNotIn(f'{i}', output) + + output = check_output('bridge vlan show dev bridge99') + print(output) + for i in range(1000, 3000): + if i == 2020: + self.assertIn(f'{i} PVID', output) + elif i in range(2022, 2026): + self.assertIn(f'{i} Egress Untagged', output) + elif i in range(2018, 2024): + self.assertIn(f'{i}', output) + else: + self.assertNotIn(f'{i}', output) + + # Remove all vlan IDs + copy_network_unit('26-bridge-vlan-slave.network.d/30-override.conf', + '26-bridge-vlan-master.network.d/30-override.conf') + networkctl_reload() + self.wait_online(['test1:enslaved', 'bridge99:degraded']) + + output = check_output('bridge vlan show dev test1') + print(output) + self.assertNotIn('PVID', output) + for i in range(1000, 3000): + self.assertNotIn(f'{i}', output) + + output = check_output('bridge vlan show dev bridge99') + print(output) + self.assertNotIn('PVID', output) + for i in range(1000, 3000): + self.assertNotIn(f'{i}', output) def test_bridge_vlan_issue_20373(self): copy_network_unit('11-dummy.netdev', '26-bridge-vlan-slave-issue-20373.network',