diff --git a/board/common/image/image-readme/README.md b/board/common/image/image-readme/README.md index 3bca78387..ea55f2029 100644 --- a/board/common/image/image-readme/README.md +++ b/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 diff --git a/doc/ChangeLog.md b/doc/ChangeLog.md index 89de004c4..db6afc3d7 100644 --- a/doc/ChangeLog.md +++ b/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 ------------------------- diff --git a/doc/branding.md b/doc/branding.md index 01e381bea..2cdce2ca6 100644 --- a/doc/branding.md +++ b/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 diff --git a/doc/bridging.md b/doc/bridging.md index 3ba38052a..f9153111b 100644 --- a/doc/bridging.md +++ b/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). 
admin@example:/config/> edit interface br0
 admin@example:/config/interface/br0/> up
@@ -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:
+
+
admin@example:/config/> leave
+Warning: eth0 is an untagged member of VLAN 10 on br0, but has no PVID.  Untagged frames received on eth0 are dropped.
+admin@example:/>
+

+
 
 ## Multicast Filtering and Snooping
 
diff --git a/doc/management.md b/doc/management.md
index 411f6ce1a..be102122f 100644
--- a/doc/management.md
+++ b/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.
+
 
admin@example:/> configure
 admin@example:/config/> edit web
 admin@example:/config/web/> help
@@ -160,19 +165,20 @@ admin@example:/config/web/> set enabled
 admin@example:/config/web/>
 

 
-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,12 +191,13 @@ 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://
/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.
 
 
admin@example:/config/web/> edit restconf
@@ -198,6 +205,10 @@ admin@example:/config/web/restconf/> no enabled
 admin@example:/config/web/restconf/>
 

 
+> [!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
diff --git a/doc/vpd.md b/doc/vpd.md
index 10b58c82b..6adf21be8 100644
--- a/doc/vpd.md
+++ b/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-`, 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:
+
+```
+ð0 {
+	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
diff --git a/src/klish-plugin-infix/src/infix.c b/src/klish-plugin-infix/src/infix.c
index 57b8619aa..c301f3678 100644
--- a/src/klish-plugin-infix/src/infix.c
+++ b/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));
diff --git a/src/klish-plugin-infix/xml/infix.xml b/src/klish-plugin-infix/xml/infix.xml
index 1f7312a08..105700037 100644
--- a/src/klish-plugin-infix/xml/infix.xml
+++ b/src/klish-plugin-infix/xml/infix.xml
@@ -1145,6 +1145,7 @@ echo "Public:  $pub"
 
 
 
+  
   
   
   replace main
@@ -1192,6 +1193,7 @@ echo "Public:  $pub"
 
 
 
+  
   
 
 
@@ -1202,6 +1204,7 @@ echo "Public:  $pub"
 
 
 
+  
   
 
 
diff --git a/src/statd/python/yanger/ietf_hardware.py b/src/statd/python/yanger/ietf_hardware.py
index b52580924..9c25bc82c 100644
--- a/src/statd/python/yanger/ietf_hardware.py
+++ b/src/statd/python/yanger/ietf_hardware.py
@@ -31,7 +31,7 @@ def vpd_component(vpd):
             mfgdate = datetime.datetime.strptime(vpd["data"]["manufacture-date"],
                                                  "%m/%d/%Y %H:%M:%S")
             component["mfg-date"] = mfgdate.strftime("%Y-%m-%dT%H:%M:%SZ")
-        if vpd["data"].get("manufacter"):
+        if vpd["data"].get("manufacturer"):
             component["mfg-name"] = vpd["data"]["manufacturer"]
         if vpd["data"].get("product-name"):
             component["model-name"] = vpd["data"]["product-name"]
diff --git a/test/case/syslog/property_filter/test.py b/test/case/syslog/property_filter/test.py
index b2eb5184d..0b6d6c0c6 100755
--- a/test/case/syslog/property_filter/test.py
+++ b/test/case/syslog/property_filter/test.py
@@ -6,6 +6,8 @@
 
 """
 
+import uuid
+
 import infamy
 from infamy.util import parallel, until
 
@@ -19,6 +21,17 @@
     ("test", "Warning: lowercase"),
 ]
 
+# Old messages survive in /var/log on hardware, count only this run's
+TOKEN = uuid.uuid4().hex[:8]
+
+
+def count_lines(ssh, logfile, text):
+    """Count lines in /var/log/logfile from this run containing text"""
+    rc = ssh.runsh(f"cat /var/log/{logfile} 2>/dev/null")
+    return sum(1 for line in rc.stdout.splitlines()
+               if TOKEN in line and text in line)
+
+
 with infamy.Test() as test:
     with test.step("Set up topology and attach to target DUT"):
         env = infamy.Env()
@@ -26,7 +39,9 @@
                                   lambda: env.attach("target", "mgmt", "ssh"))
 
     with test.step("Clean up old log files"):
-        tgtssh.runsh("sudo rm -f /var/log/myapp /var/log/not-error /var/log/case-test /var/log/baseline")
+        rc = tgtssh.run_retry("sudo rm -f /var/log/myapp /var/log/not-error /var/log/case-test /var/log/baseline")
+        if rc.returncode:
+            test.fail("Failed removing old log files")
 
     with test.step("Configure syslog with property filters"):
         target.put_config_dicts({
@@ -94,41 +109,36 @@
 
     with test.step("Send test messages"):
         for tag, msg in TEST_MESSAGES:
-            target.log(msg, severity="info", app_name=tag)
-        until(lambda: "Application startup" in tgtssh.runsh("cat /var/log/baseline 2>/dev/null").stdout, attempts=10)
+            target.log(f"{msg} {TOKEN}", severity="info", app_name=tag)
+        until(lambda: count_lines(tgtssh, "baseline", TEST_MESSAGES[-1][1]), attempts=10)
 
     with test.step("Verify myapp log contains only myapp messages"):
-        rc = tgtssh.runsh("grep -c 'myapp' /var/log/myapp 2>/dev/null")
-        count = int(rc.stdout.strip()) if rc.returncode == 0 else 0
+        count = count_lines(tgtssh, "myapp", "myapp")
         if count != 2:
             test.fail(f"Expected 2 myapp messages in /var/log/myapp, got {count}")
 
-        rc = tgtssh.runsh("grep -c 'otherapp' /var/log/myapp 2>/dev/null")
-        count = int(rc.stdout.strip()) if rc.returncode == 0 else 0
+        count = count_lines(tgtssh, "myapp", "otherapp")
         if count != 0:
             test.fail(f"Expected 0 otherapp messages in /var/log/myapp, got {count}")
 
     with test.step("Verify not-error log excludes ERROR messages"):
-        rc = tgtssh.runsh("grep -c 'test' /var/log/not-error 2>/dev/null")
-        count = int(rc.stdout.strip()) if rc.returncode == 0 else 0
+        count = count_lines(tgtssh, "not-error", "test")
         if count != 3:
             test.fail(f"Expected 3 non-ERROR messages in /var/log/not-error, got {count}")
 
-        rc = tgtssh.runsh("grep -c 'ERROR' /var/log/not-error 2>/dev/null")
-        count = int(rc.stdout.strip()) if rc.returncode == 0 else 0
+        count = count_lines(tgtssh, "not-error", "ERROR")
         if count != 0:
             test.fail(f"Expected 0 ERROR messages in /var/log/not-error, got {count}")
 
     with test.step("Verify case-test log matches case-insensitive 'warning'"):
-        rc = tgtssh.runsh("grep -c 'WARNING\\|Warning' /var/log/case-test 2>/dev/null")
-        count = int(rc.stdout.strip()) if rc.returncode == 0 else 0
+        count = count_lines(tgtssh, "case-test", "WARNING") + \
+            count_lines(tgtssh, "case-test", "Warning")
         if count != 2:
             test.fail(f"Expected 2 warning messages in /var/log/case-test, got {count}")
 
     with test.step("Verify baseline log contains all messages"):
         for tag, msg in TEST_MESSAGES:
-            rc = tgtssh.runsh(f"grep -q '{msg}' /var/log/baseline 2>/dev/null")
-            if rc.returncode != 0:
+            if not count_lines(tgtssh, "baseline", msg):
                 test.fail(f"Expected message '{msg}' not found in /var/log/baseline")
 
     test.succeed()