yangerd: re-read wds-link ports on kernel station events

doc/ChangeLog.md

@@ -195,6 +195,9 @@ All notable changes to the project are documented in this file.
 
 ### Fixes
 
+- Moving a WiFi access point to another radio left it on the old radio,
+  and the access point did not start.  The radio of an access point can
+  now be changed
 - A bridge port recreated with a new address lost the VLANs it was tagged
   in, only its PVID VLAN came back
 - Removing the DHCPv6 client from an interface could leave its routes

src/confd/src/if-wifi.c

@@ -168,6 +168,11 @@ int wifi_mode_changed(struct lyd_node *wifi)
 	if (node && (op == LYDX_OP_CREATE || op == LYDX_OP_DELETE))
 		return 1;
 
+	/* Moved to another radio, dependency passes add the leaf without orig-value */
+	node = lydx_get_child(wifi, "radio");
+	if (node && lydx_get_mattr(node, "yang:orig-value"))
+		return 1;
+
 	/* No mode <-> station, and the 4-address flag is set at creation */
 	node = lydx_get_child(wifi, "station");
 	if (node)

src/confd/src/interfaces.c

@@ -493,7 +493,7 @@ static bool netdag_must_del(struct lyd_node *dif, struct lyd_node *cif)
 	case IFT_DUMMY:
 	case IFT_LO:
 	case IFT_WIFI:
-		return lydx_get_child(dif, "custom-phys-address") || lydx_get_descendant(dif, "wifi", "radio", NULL) || wifi_mode_changed(lydx_get_child(dif, "wifi"));
+		return lydx_get_child(dif, "custom-phys-address") || wifi_mode_changed(lydx_get_child(dif, "wifi"));
 		break;
 
 	case IFT_ETH:

src/yangerd/internal/iwmonitor/iwmonitor.go

@@ -153,6 +153,10 @@ func (m *IWMonitor) handleNL80211(ctx context.Context, msg genetlink.Message, fa
 			m.log.Debug("nl80211: station event", "cmd", cmd, "iface", ifname)
 			m.refreshInterface(ctx, ifname)
 		}
+		// A station leaving a wds-link port is moved back to its
+		// access point before the kernel deletes it, so the event
+		// names the access point, not the port.
+		m.refreshWDSPorts(ctx)
 	case unix.NL80211_CMD_CONNECT:
 		if ifname != "" {
 			m.log.Debug("nl80211: connect", "iface", ifname)

src/yangerd/internal/iwmonitor/wds.go

@@ -25,8 +25,10 @@ func buildWDSData(ms meshState) map[string]any {
 }
 
 // refreshWDSPorts re-reads every wds-link port.  hostapd binds a
-// station to its port without any nl80211 event for the port, the
-// AP-STA-CONNECTED on the access point is the cue.
+// station to its port without any nl80211 event for the port, and
+// unbinds it after it has announced the AP-STA-DISCONNECTED, so the
+// station events on the access point are the cue, from hostapd and
+// from the kernel.
 func (m *IWMonitor) refreshWDSPorts(ctx context.Context) {
 	for _, port := range m.wdsPorts() {
 		m.refreshInterface(ctx, port)

src/yangerd/internal/iwmonitor/wds_test.go

@@ -8,6 +8,9 @@ import (
 
 	"github.com/kernelkit/infix/src/yangerd/internal/nl80211"
 	"github.com/kernelkit/infix/src/yangerd/internal/wpactrl"
+	"github.com/mdlayher/genetlink"
+	"github.com/mdlayher/netlink"
+	"golang.org/x/sys/unix"
 )
 
 func TestBuildWDSData(t *testing.T) {
@@ -106,3 +109,35 @@ func TestAttachDetachRefreshWDSPorts(t *testing.T) {
 		t.Errorf("detached: wds0 published %q, want %s", published["wds0"], want)
 	}
 }
+
+// hostapd announces AP-STA-DISCONNECTED up to a second before it
+// unbinds a timed-out station from its port, and the kernel's delete
+// then names the access point, so that event re-reads the ports too.
+func TestNL80211StationEventsRefreshWDSPorts(t *testing.T) {
+	m := New(slog.Default())
+	m.meshState = func(ifname string) meshState {
+		if ifname == "wds0" {
+			return meshState{iftype: "AP_VLAN"}
+		}
+		return meshState{}
+	}
+	m.wdsPorts = func() []string { return []string{"wds0"} }
+
+	published := map[string]string{}
+	m.SetOnUpdate(func(ifname string, raw json.RawMessage) { published[ifname] = string(raw) })
+
+	ae := netlink.NewAttributeEncoder()
+	ae.String(unix.NL80211_ATTR_IFNAME, "wifi-test-none")
+	data, err := ae.Encode()
+	if err != nil {
+		t.Fatalf("encode: %v", err)
+	}
+	for _, cmd := range []uint8{unix.NL80211_CMD_NEW_STATION, unix.NL80211_CMD_DEL_STATION} {
+		delete(published, "wds0")
+		msg := genetlink.Message{Header: genetlink.Header{Command: cmd}, Data: data}
+		m.handleNL80211(context.Background(), msg, genetlink.Family{})
+		if want := `{"wds-link":{"connected":false}}`; published["wds0"] != want {
+			t.Errorf("cmd %d: wds0 published %q, want %s", cmd, published["wds0"], want)
+		}
+	}
+}

test/case/interfaces/wifi.yaml

@@ -1,6 +1,6 @@
 ---
 - name: WiFi AP and Station Association across two DUTs
-  case: wifi_ap_station_2dut/test.py
+  suite: wifi_ap_station_2dut/test.yaml
 
 - name: WiFi Access Point serving three Stations
   case: wifi_ap_multi_station/test.py

test/case/interfaces/wifi_ap_station_2dut/2_4ghz.adoc

@@ -0,0 +1,42 @@
+=== WiFi AP and Station Association across two DUTs on 2.4GHz
+
+ifdef::topdoc[:imagesdir: {topdoc}../../test/case/interfaces/wifi_ap_station_2dut]
+
+==== Description
+
+Two DUTs, each with its own radio: the ap runs a WPA2/WPA3-personal Access
+Point on 2.4GHz, the station associates to it.  The radios are in
+separate kernels, so the "air" between them is realised differently per
+environment:
+
+  * On real hardware the link is real RF -- the radios just associate.
+  * On QEMU the radios are mac80211_hwsim, and the `wifimedium` relay on
+    each DUT bridges hwsim frames over the Ethernet segment between the
+    guests (the topology's wifi-link).  See doc/wifi.md.
+
+The test asserts that the station associates with WPA2/WPA3-personal.  That
+is a strong end-to-end check: authentication, association and the WPA
+4-way handshake all require frames to cross the medium in *both*
+directions, so a successful association proves real bidirectional
+communication over the (relayed) radio link -- not just that an interface
+came up.  It then confirms data-plane reach: the ap serves DHCP and the
+station leases its address over the radio link.
+
+Topology:
+....
+    host ==(mgmt)== ap  )))  ~ wifi-link ~  ((( station ==(mgmt)== host
+....
+
+==== Topology
+
+image::topology.svg[WiFi AP and Station Association across two DUTs on 2.4GHz topology, align=center, scaledwidth=75%]
+
+==== Sequence
+
+. Set up topology and attach to the ap and the station
+. Configure the ap as an Access Point on 2.4GHz and the station
+. Verify the station associates to the ap over the wifi link
+. Verify the station's wifi0 operational status is up
+. Verify the station leases its address from the ap over wifi
+
+

test/case/interfaces/wifi_ap_station_2dut/2_4ghz.py

@@ -0,0 +1 @@
+test.py

test/case/interfaces/wifi_ap_station_2dut/test.adoc → test/case/interfaces/wifi_ap_station_2dut/5ghz.adoc

@@ -1,11 +1,11 @@
-=== WiFi AP and Station Association across two DUTs
+=== WiFi AP and Station Association across two DUTs on 5GHz
 
 ifdef::topdoc[:imagesdir: {topdoc}../../test/case/interfaces/wifi_ap_station_2dut]
 
 ==== Description
 
 Two DUTs, each with its own radio: the ap runs a WPA2/WPA3-personal Access
-Point, the station associates to it.  The radios are in
+Point on 5GHz, the station associates to it.  The radios are in
 separate kernels, so the "air" between them is realised differently per
 environment:
 
@@ -29,12 +29,12 @@ Topology:
 
 ==== Topology
 
-image::topology.svg[WiFi AP and Station Association across two DUTs topology, align=center, scaledwidth=75%]
+image::topology.svg[WiFi AP and Station Association across two DUTs on 5GHz topology, align=center, scaledwidth=75%]
 
 ==== Sequence
 
 . Set up topology and attach to the ap and the station
-. Configure the ap as an Access Point and the station on radio0
+. Configure the ap as an Access Point on 5GHz and the station
 . Verify the station associates to the ap over the wifi link
 . Verify the station's wifi0 operational status is up
 . Verify the station leases its address from the ap over wifi

test/case/interfaces/wifi_ap_station_2dut/5ghz.py

@@ -0,0 +1 @@
+test.py

test/case/interfaces/wifi_ap_station_2dut/Readme.adoc

@@ -0,0 +1,6 @@
+include::2_4ghz.adoc[]
+
+<<<
+
+include::5ghz.adoc[]
+

test/case/interfaces/wifi_ap_station_2dut/test.py

@@ -1,9 +1,9 @@
 #!/usr/bin/env python3
 r"""
-WiFi Access Point and Station association across two DUTs
+WiFi Access Point and Station association across two DUTs on {band}
 
 Two DUTs, each with its own radio: the ap runs a WPA2/WPA3-personal Access
-Point, the station associates to it.  The radios are in
+Point on {band}, the station associates to it.  The radios are in
 separate kernels, so the "air" between them is realised differently per
 environment:
 
@@ -42,22 +42,26 @@
 
 with infamy.Test() as test:
     with test.step("Set up topology and attach to the ap and the station"):
-        env = infamy.Env()
+        argp = wifi.BandArgumentParser()
+        env = infamy.Env(args=argp, edge_mappings=argp.edge_mappings)
+        band = env.args.band
+        ap_radio = env["ap"]["wifi"]
+        sta_radio = env["station"]["wifi"]
         # Connect to both nodes concurrently to cut setup time.
         ap, station = parallel(
             lambda: env.attach("ap", "mgmt"),
             lambda: env.attach("station", "mgmt"),
         )
         wifi.skip_unless_supported(test, ap, station)
 
-    with test.step("Configure the ap as an Access Point and the station on radio0"):
+    with test.step(f"Configure the ap as an Access Point on {band} and the station"):
         parallel(
             lambda: ap.put_config_dicts({
-                "ietf-hardware": wifi.hardware(wifi.radio("radio0", band="2.4GHz", channel=1)),
+                "ietf-hardware": wifi.hardware(wifi.radio(ap_radio, band=band, channel=wifi.CHANNEL[band])),
                 "ietf-keystore": wifi.keystore({"wifi": PSK}),
                 "ietf-interfaces": {"interfaces": {"interface": [
                     wifi.iface("wifi0", {
-                        "radio": "radio0",
+                        "radio": ap_radio,
                         "access-point": {
                             "ssid": SSID,
                             "security": {"mode": "wpa2-wpa3-personal", "secret": "wifi"},
@@ -70,11 +74,11 @@
                 }]}},
             }),
             lambda: station.put_config_dicts({
-                "ietf-hardware": wifi.hardware(wifi.radio("radio0")),
+                "ietf-hardware": wifi.hardware(wifi.radio(sta_radio)),
                 "ietf-keystore": wifi.keystore({"wifi": PSK}),
                 "ietf-interfaces": {"interfaces": {"interface": [
                     wifi.iface("wifi0", {
-                        "radio": "radio0",
+                        "radio": sta_radio,
                         "station": {
                             "ssid": SSID,
                             "security": {"mode": "auto", "secret": "wifi"},

test/case/interfaces/wifi_ap_station_2dut/test.yaml

@@ -0,0 +1,11 @@
+---
+- settings:
+    test-spec: <case>.adoc
+
+- name: WiFi AP and Station Association across two DUTs on 2.4GHz
+  case: 2_4ghz.py
+  opts: ["--band", "2.4GHz"]
+
+- name: WiFi AP and Station Association across two DUTs on 5GHz
+  case: 5ghz.py
+  opts: ["--band", "5GHz"]

test/case/interfaces/wifi_ap_station_2dut/topology.dot

@@ -39,6 +39,7 @@ graph "wifi-ap-station-2dut" {
 	host:mgmt1 -- ap:mgmt [requires="mgmt", color="lightgray"]
 	host:mgmt2 -- station:mgmt [requires="mgmt", color="lightgray"]
 
-	ap:wifi -- cell      [requires="wifi2.4GHz", style="dashed"]
-	station:wifi -- cell [requires="wifi2.4GHz", style="dashed"]
+	// wifi-band is the radio band the test runs on, see BandArgumentParser
+	ap:wifi -- cell      [requires="wifi-band", style="dashed"]
+	station:wifi -- cell [requires="wifi-band", style="dashed"]
 }

test/infamy/wifi.py

@@ -6,8 +6,36 @@
 import base64
 
 import infamy.iface as _iface
+from infamy.env import ArgumentParser as _ArgumentParser
+from infamy.topology import edge_mappings as _edge_mappings
 from infamy.util import until
 
+CHANNEL = {"2.4GHz": 1, "5GHz": 36}
+
+
+class BandArgumentParser(_ArgumentParser):
+    """Test arguments with a required --band, e.g. 2.4GHz or 5GHz.
+
+    Pass edge_mappings to infamy.Env() as well: a radio edge in the
+    logical topology that requires "wifi-band" then maps onto a radio
+    providing wifi<band>.
+    """
+    def __init__(self):
+        super().__init__()
+        self.add_argument("--band", choices=list(CHANNEL))
+        self.band = None
+
+    def parse_args(self, argv):
+        args = super().parse_args(argv)
+        self.band = args.band
+        return args
+
+    def edge_mappings(self, les, pes):
+        les = {k: dict(e, requires={f"wifi{self.band}" if r == "wifi-band" else r
+                                    for r in e["requires"]})
+               for k, e in les.items()}
+        return _edge_mappings(les, pes)
+
 
 def radio(name, band=None, channel=None):
     """ietf-hardware component for a WiFi radio."""