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.
This commit is contained in:
Yu Watanabe 2023-11-12 16:04:19 +09:00
parent 11cee6efbf
commit 228693af47
8 changed files with 140 additions and 20 deletions

View file

@ -5880,9 +5880,11 @@ ServerAddress=192.168.0.1/24</programlisting>
<varlistentry>
<term><varname>PVID=</varname></term>
<listitem>
<para>The Port VLAN ID specified here is assigned to all untagged frames at ingress.
<varname>PVID=</varname> can be used only once. Configuring <varname>PVID=</varname> implicates the use of
<varname>VLAN=</varname> above and will enable the VLAN ID for ingress as well.</para>
<para>The port VLAN ID specified here is assigned to all untagged frames at ingress. Takes an
VLAN ID or negative boolean value (e.g. <literal>no</literal>). When false, the currently
assigned port VLAN ID will be dropped. Configuring <varname>PVID=</varname> implicates the use of
<varname>VLAN=</varname> 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.</para>
<xi:include href="version-info.xml" xpointer="v231"/>
</listitem>

View file

@ -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;
}

View file

@ -6,20 +6,26 @@
***/
#include <inttypes.h>
#include <stdbool.h>
#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);

View file

@ -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;

View file

@ -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,

View file

@ -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",

View file

@ -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. */

View file

@ -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) {