Doc fixes and more

board/common/image/image-readme/README.md

@@ -103,11 +103,13 @@ Graphical Network Simulator 3 (GNS3)
 ------------------------------------
 
 GNS3 is a very powerful front-end to Qemu which takes care of creating
-virtual links between network devices running in Qemu.  This README is
-all you need to get going, alongisde it is the appliance file (.gns3a)
-that reference image files in this directory needed to load into GNS3.
+virtual links between network devices running in Qemu.  The appliance
+is available from the GNS3 Marketplace, search for Infix when adding a
+new template, and point it to the disk image in this directory.
 
-Necessary Ubuntu packages are available through the offical GNS3 PPA.
+ - https://gns3.com/marketplace/appliances/infix
+
+Necessary Ubuntu packages are available through the official GNS3 PPA.
 If you don't know what a PPA is, read up on that first:
 
  - https://launchpad.net/~gns3/+archive/ubuntu/ppa

doc/ChangeLog.md

@@ -21,6 +21,9 @@ All notable changes to the project are documented in this file.
   per port, and a Health card listing services that are not running, a
   pending reboot, and sensor readings.  Disk Usage no longer lists the
   read-only root filesystem
+- CLI: `check`, `commit`, and `leave` warn about bridge ports with a missing
+  or mismatched PVID, which drops untagged frames or puts them in the wrong
+  VLAN, issue #354
 
 ### Fixes
 
@@ -29,6 +32,8 @@ All notable changes to the project are documented in this file.
   never showed the boot partition
 - LLDP neighbors were listed without their system name, descriptions, and
   capabilities, in the CLI, the WebUI, and the operational datastore
+- The manufacturer from a VPD was not shown as `mfg-name` of its
+  `vpd-*` component in the operational datastore
 
 [v26.09.0][] - 2026-09-30
 -------------------------

doc/branding.md

@@ -24,7 +24,7 @@ Verify the result after a build by inspecting:
 
 - `output/images/*`: names, missing prefix, etc.
 - `output/target/etc/os-release`: this file is sourced by other build
-  scripts, e.g., `mkgns3a.sh`.  For reference, see [os-release(5)][]
+  scripts, e.g., `post-build.sh`.  For reference, see [os-release(5)][]
 
 > [!IMPORTANT]
 > To get a proper GIT revision (hash) from your OS spin, remember to set

doc/bridging.md

@@ -45,8 +45,8 @@ bridge should be used instead.
 By default bridges in Linux do not filter based on VLAN tags.  This can
 be enabled when creating a bridge by adding a port to a VLAN as a tagged
 or untagged member.  Use the port default VID (PVID) setting to control
-VLAN association for traffic ingressing a port untagged (default PVID:
-1).
+VLAN association for traffic ingressing a port untagged.  There is no
+default PVID, see [Ingress and Egress Rules](#ingress-and-egress-rules).
 
 <pre class="cli"><code>admin@example:/config/> <b>edit interface br0</b>
 admin@example:/config/interface/br0/> <b>up</b>
@@ -82,6 +82,42 @@ on this topic.
 > in VLAN 10, IP addresses can be set directly on the bridge without the
 > need for dedicated VLAN interfaces on top of the bridge.
 
+### Ingress and Egress Rules
+
+On a VLAN filtering bridge every frame belongs to a VLAN, decided when
+it enters a port.  Using `eth0` from the example above, with PVID 10
+and untagged member of VLAN 10:
+
+| Frame received on `eth0`  | Result                                         |
+|---------------------------|------------------------------------------------|
+| Untagged                  | Accepted, assigned to VLAN 10                  |
+| Priority tagged (VID 0)   | Accepted, assigned to VLAN 10, PCP is kept     |
+| Tagged with VID 10        | Accepted                                       |
+| Tagged with VID 20        | Dropped, `eth0` is not a member of VLAN 20     |
+
+The rules behind the table:
+
+- A tagged frame is accepted only if the port is a member of its VLAN.
+  Whether the membership is tagged or untagged only matters when frames
+  leave the port
+- Untagged and priority tagged frames are assigned to the port's PVID.
+  A port without a PVID drops them.  The PVID must be one of the port's
+  VLANs, otherwise it is ignored and a warning is logged
+- A frame is only forwarded to ports that are members of its VLAN.  It
+  leaves a tagged member with a VLAN tag, and an untagged member
+  without one, so the PCP of a priority tagged frame is lost on an
+  untagged port
+
+The same rules apply when the bridge is offloaded to a switch chip.
+
+The CLI commands `check`, `commit`, and `leave` warn about ports with a
+missing or mismatched PVID.  The configuration is applied anyway:
+
+<pre class="cli"><code>admin@example:/config/> <b>leave</b>
+Warning: eth0 is an untagged member of VLAN 10 on br0, but has no PVID.  Untagged frames received on eth0 are dropped.
+admin@example:/>
+</code></pre>
+
 
 ## Multicast Filtering and Snooping
 

doc/management.md

@@ -130,7 +130,7 @@ admin@example:/>
 
 The system provides a set of Web services:
 
-- a rudimentary Web server, currently limited to an information page
+- a Web management interface (WebUI) for configuration and monitoring
 - a RESTCONF server with equivalent management capabilities as NETCONF
 - a Web console service, where the shell/CLI can be accessed via
   HTTPS, similar to connecting via a console port or SSH
@@ -139,6 +139,11 @@ There is also a *Netbrowse* Web service presenting information about
 the unit's neighbors, collected via mDNS (see
 [Discovery](discovery.md) for more details).
 
+All of them are enabled in the factory configuration.  The WebUI, Web
+console, and Netbrowse are optional parts of the image.  Minimal builds
+have only a static information page and RESTCONF.  On such an image,
+the settings for the missing services are accepted but have no effect.
+
 <pre class="cli"><code>admin@example:/> <b>configure</b>
 admin@example:/config/> <b>edit web</b>
 admin@example:/config/web/> <b>help</b>
@@ -160,19 +165,20 @@ admin@example:/config/web/> <b>set enabled</b>
 admin@example:/config/web/>
 </code></pre>
 
-Enabling the Web service implies that a Web server is
-enabled. Currently this Web server provides generic Infix information,
-as well as a link to a Web console. The Web server uses HTTPS; any
-HTTP request is redirected to HTTPS.
+Enabling the Web service starts a Web server on port 443 (HTTPS), and
+any HTTP request on port 80 is redirected to HTTPS.  It serves the
+WebUI, where you log in with the same user accounts as for the CLI,
+SSH, and NETCONF.
 
 The _enabled_ setting for the Web service acts as a global
 enable/disable setting for the other Web services (Web console,
-RESTCONF and Netbrowse).
+RESTCONF and Netbrowse).  Disabling it stops all of them, regardless
+of their own settings.
 
 ### Enable/disable Web Console
 
 The Web console service provides a terminal service similar to Console
-or SSH. The Web console is secured via HTTPS on port 7861.
+or SSH. The Web console is secured via HTTPS on port 7681.
 
 The Web console has its own enable/disable setting, but will only be
 activated if the Web service is enabled. The example below shows how
@@ -185,19 +191,24 @@ admin@example:/config/web/console/>
 
 ### Enable/disable RESTCONF Service
 
-Alternatively, the system can be managed remotely using
-RESTCONF. Meaning you can `curl` it instead of using a dedicated
-NETCONF client.
+Alternatively, the system can be managed remotely using RESTCONF, at
+`https://<address>/restconf`.  Meaning you can `curl` it instead of
+using a dedicated NETCONF client, see [RESTCONF
+Scripting](scripting-restconf.md) for examples.
 
 The RESTCONF service has its own enable/disable setting, but will
-only be activated if the Web service is enabled. The example below
+only be activated if the Web service is enabled.  The example below
 shows how to disable the RESTCONF service.
 
 <pre class="cli"><code>admin@example:/config/web/> <b>edit restconf</b>
 admin@example:/config/web/restconf/> <b>no enabled</b>
 admin@example:/config/web/restconf/>
 </code></pre>
 
+> [!NOTE]
+> The WebUI uses RESTCONF internally, so disabling RESTCONF only blocks
+> requests from other hosts, and the WebUI keeps working.
+
 ### HTTPS Certificate
 
 The Web server uses a TLS certificate from the central

doc/vpd.md

@@ -15,6 +15,93 @@ each board making up a system, figure out if a system belongs to a
 particular production batch, etc.
 
 
+## Device Tree Bindings
+
+The system finds its VPDs through the `vpds` property of the
+`/chosen/infix` node in the device tree (DT), a list of phandles to EEPROMs with an ONIE TLV
+layout.  Each EEPROM is named with an `infix,board` property, and may
+be marked `infix,trusted`:
+
+```
+/ {
+	chosen {
+		infix {
+			vpds = <&vpd_cpu &vpd_product>;
+		};
+	};
+};
+
+&i2c0 {
+	vpd_product: eeprom@50 {
+		compatible = "atmel,24c02";
+		reg = <0x50>;
+
+		infix,board = "product";
+		infix,trusted;
+
+		nvmem-layout {
+			compatible = "onie,tlv-layout";
+
+			base_mac: mac-address {
+				#nvmem-cell-cells = <1>;
+			};
+		};
+	};
+};
+```
+
+Every VPD is listed in the _ietf-hardware_ model as a component named
+`vpd-<board>`, e.g., `vpd-product` for the example above.
+
+
+## Product VPD
+
+The VPD named `product` describes the device as a whole.  Its
+attributes are used for the following, falling back to other sources
+when an attribute is missing:
+
+| Key               | Used for                                    | Fallback                             |
+|-------------------|---------------------------------------------|--------------------------------------|
+| `"vendor"`        | `mfg-name` of the `mainboard` component     | Vendor of DT `compatible`            |
+| `"product-name"`  | `model-name` of the `mainboard` component   | DT `model` property                  |
+| `"part-number"`   | `hardware-rev` of the `mainboard` component | None                                 |
+| `"serial-number"` | `serial-num` of the `mainboard` component   | DT `serial-number` property          |
+| `"mac-address"`   | Base (chassis) MAC address, see below       | Lowest MAC address of all interfaces |
+
+The base MAC address is the `phys-address` of the `mainboard`
+component.  It is also what the `chassis` option of an interface's
+`custom-phys-address` refers to, see [Common Interface
+Settings](iface.md#chassis-mac), and the source of the `%m` format
+specifier in the hostname, which the factory configuration uses to
+give each device a unique name, e.g., `example-c0-ff-ee`.
+
+The kernel sets the MAC address of each Ethernet port from the
+`nvmem-cells` property of its device tree node, which can refer to the
+`mac-address` cell of the product VPD, plus an offset:
+
+```
+&eth0 {
+	nvmem-cells = <&base_mac 1>;
+	nvmem-cell-names = "mac-address";
+};
+```
+
+### Factory Password
+
+The factory default `admin` password hash, the `pwhash` [extension
+below](#infix-specific-extensions), is read from the first VPD marked
+`infix,trusted` that has one, which need not be the product VPD.  It takes precedence
+over a `factory-password-hash` property in the `/chosen/infix` node.
+Without either, the system reports a critical bootstrap error.
+
+### Virtual Machines
+
+When running in QEMU, the product VPD is read from the `opt/vpd`
+firmware configuration file, and is always trusted.  Without that file
+the password defaults to `admin`, and the base MAC address is the lowest
+port MAC address minus one.
+
+
 ## JSON Encoding
 
 To make EEPROM binary generation less cumbersome, Infix defines a JSON

src/klish-plugin-infix/src/infix.c

@@ -837,6 +837,103 @@ int infix_ssh_remove_known_host(kcontext_t *ctx)
 	return run_as_user(cd_home(ctx), argv);
 }
 
+#define IF_XPATH "/ietf-interfaces:interfaces/interface"
+
+static int is_member(struct lyd_node *vlan, const char *mode, const char *port)
+{
+	char path[64];
+
+	snprintf(path, sizeof(path), "%s[.='%s']", mode, port);
+	return !lyd_find_path(vlan, path, 0, NULL);
+}
+
+static void pvid_check_port(struct lyd_node *tree, struct lyd_node *iface)
+{
+	int pvid = 0, uvid = 0, num = 0, member = 0;
+	char untagged[64] = "", xpath[128];
+	struct lyd_node *node, *vlans, *vlan;
+	const char *port, *br;
+
+	port = lyd_get_value(lyd_child(iface));
+	if (!lyd_find_path(iface, "infix-interfaces:bridge", 0, NULL))
+		br = port;
+	else if (!lyd_find_path(iface, "infix-interfaces:bridge-port/bridge", 0, &node))
+		br = lyd_get_value(node);
+	else
+		return;
+
+	snprintf(xpath, sizeof(xpath), IF_XPATH "[name='%s']/infix-interfaces:bridge/vlans", br);
+	if (lyd_find_path(tree, xpath, 0, &vlans))
+		return;
+
+	if (!lyd_find_path(iface, "infix-interfaces:bridge-port/pvid", 0, &node))
+		pvid = atoi(lyd_get_value(node));
+
+	LY_LIST_FOR(lyd_child(vlans), vlan) {
+		int vid, untag;
+
+		if (strcmp(vlan->schema->name, "vlan"))
+			continue;
+
+		untag = is_member(vlan, "untagged", port);
+		if (!untag && !is_member(vlan, "tagged", port))
+			continue;
+
+		vid = atoi(lyd_get_value(lyd_child(vlan)));
+		if (vid == pvid)
+			member = 1;
+		if (untag) {
+			size_t len = strlen(untagged);
+
+			snprintf(&untagged[len], sizeof(untagged) - len, "%s%d", len ? ", " : "", vid);
+			uvid = vid;
+			num++;
+		}
+	}
+
+	if (pvid && !member)
+		printf("Warning: %s has PVID %d, but is not a member of VLAN %d on %s.  "
+		       "The PVID is ignored.\n", port, pvid, pvid, br);
+	else if (!pvid && num)
+		printf("Warning: %s is an untagged member of VLAN%s %s on %s, but has no PVID.  "
+		       "Untagged frames received on %s are dropped.\n",
+		       port, num > 1 ? "s" : "", untagged, br, port);
+	/* Untagged in several VLANs is an asymmetric VLAN setup, by design */
+	else if (pvid && num == 1 && uvid != pvid)
+		printf("Warning: %s is an untagged member of VLAN %d on %s, but has PVID %d.\n",
+		       port, uvid, br, pvid);
+}
+
+/*
+ * Warn about bridge port PVIDs that drop untagged traffic or put it in
+ * another VLAN than the one it leaves untagged in, issue #354.
+ */
+int infix_pvid_check(kcontext_t *ctx)
+{
+	const char *xpath = IF_XPATH "/infix-interfaces:bridge/vlans | "
+			    IF_XPATH "/infix-interfaces:bridge-port";
+	sr_session_ctx_t *sess = NULL;
+	sr_conn_ctx_t *conn = NULL;
+	sr_data_t *data = NULL;
+	struct lyd_node *iface;
+
+	(void)ctx;
+
+	if (sr_connect(SR_CONN_DEFAULT, &conn))
+		return 0;
+	if (sr_session_start(conn, SR_DS_CANDIDATE, &sess) ||
+	    sr_get_data(sess, xpath, 0, 0, 0, &data) || !data)
+		goto done;
+
+	LY_LIST_FOR(lyd_child(data->tree), iface)
+		pvid_check_port(data->tree, iface);
+done:
+	sr_release_data(data);
+	sr_disconnect(conn);
+
+	return 0;
+}
+
 int kplugin_infix_fini(kcontext_t *ctx)
 {
 	(void)ctx;
@@ -867,6 +964,7 @@ int kplugin_infix_init(kcontext_t *ctx)
 	kplugin_add_syms(plugin, ksym_new("firewall_addrsets", infix_firewall_addrsets));
 	kplugin_add_syms(plugin, ksym_new("firewall_addrset_action", infix_firewall_addrset_action));
 	kplugin_add_syms(plugin, ksym_new("set_boot_order", infix_set_boot_order));
+	kplugin_add_syms(plugin, ksym_new("pvid_check", infix_pvid_check));
 	kplugin_add_syms(plugin, ksym_new("shell", infix_shell));
 	kplugin_add_syms(plugin, ksym_new("ssh_connect",           infix_ssh_connect));
 	kplugin_add_syms(plugin, ksym_new("ssh_known_hosts",       infix_ssh_known_hosts));