Merge pull request #29987 from yuwata/network-bridge-vlan

network: remove unnecessary bridge vlan IDs

man/systemd.network.xml

@@ -5866,8 +5866,9 @@ ServerAddress=192.168.0.1/24</programlisting>
         <varlistentry>
           <term><varname>VLAN=</varname></term>
           <listitem>
-            <para>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.</para>
+            <para>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.</para>
 
             <xi:include href="version-info.xml" xpointer="v231"/>
           </listitem>
@@ -5876,8 +5877,10 @@ ServerAddress=192.168.0.1/24</programlisting>
           <term><varname>EgressUntagged=</varname></term>
           <listitem>
             <para>The VLAN ID specified here will be used to untag frames on egress. Configuring
-            <varname>EgressUntagged=</varname> implicates the use of <varname>VLAN=</varname> above and will enable the
-            VLAN ID for ingress as well. This can be either a single ID or a range M-N.</para>
+            <varname>EgressUntagged=</varname> implicates the use of <varname>VLAN=</varname> 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.</para>
 
             <xi:include href="version-info.xml" xpointer="v231"/>
           </listitem>
@@ -5885,9 +5888,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>

src/network/networkd-bridge-vlan.h

@@ -6,26 +6,28 @@
 ***/
 
 #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_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, bool is_set);
 
-CONFIG_PARSER_PROTOTYPE(config_parse_brvlan_pvid);
-CONFIG_PARSER_PROTOTYPE(config_parse_brvlan_vlan);
-CONFIG_PARSER_PROTOTYPE(config_parse_brvlan_untagged);
+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);

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

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

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

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

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)

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,
 
+                .bridge_vlan_pvid = BRIDGE_VLAN_KEEP_PVID,
+
                 .lldp_mode = LLDP_MODE_ROUTERS_ONLY,
                 .lldp_multicast_mode = _SD_LLDP_MULTICAST_MODE_INVALID,
 

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;

src/network/networkd-queue.c

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

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

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,29 +339,14 @@ 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, /* is_set = */ true);
                 if (r < 0)
                         return r;
-
-                r = sd_netlink_message_open_container(req, IFLA_AF_SPEC);
+                break;
+        case REQUEST_TYPE_DEL_LINK_BRIDGE_VLAN:
+                r = bridge_vlan_set_message(link, req, /* is_set = */ false);
                 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);
@@ -430,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)
@@ -480,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. */
@@ -704,10 +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 (!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)) &&
+            link->network->bridge_vlan_pvid == BRIDGE_VLAN_KEEP_PVID)
                 return 0;
 
         if (!link->network->bridge && !streq_ptr(link->kind, "bridge")) {
@@ -723,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) {

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

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

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

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=

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

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

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