diff --git a/doc/ChangeLog.md b/doc/ChangeLog.md index 95946eb718..ab375aa6f7 100644 --- a/doc/ChangeLog.md +++ b/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 diff --git a/src/confd/src/if-wifi.c b/src/confd/src/if-wifi.c index 8c39b5fcfd..c995d65af7 100644 --- a/src/confd/src/if-wifi.c +++ b/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) diff --git a/src/confd/src/interfaces.c b/src/confd/src/interfaces.c index ddb5088127..795edbc14b 100644 --- a/src/confd/src/interfaces.c +++ b/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: diff --git a/src/yangerd/internal/iwmonitor/iwmonitor.go b/src/yangerd/internal/iwmonitor/iwmonitor.go index ffbc9991b7..570d3223f5 100644 --- a/src/yangerd/internal/iwmonitor/iwmonitor.go +++ b/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) diff --git a/src/yangerd/internal/iwmonitor/wds.go b/src/yangerd/internal/iwmonitor/wds.go index a121aca7db..a722ffce37 100644 --- a/src/yangerd/internal/iwmonitor/wds.go +++ b/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) diff --git a/src/yangerd/internal/iwmonitor/wds_test.go b/src/yangerd/internal/iwmonitor/wds_test.go index 1dbba391dd..d067b0ea8b 100644 --- a/src/yangerd/internal/iwmonitor/wds_test.go +++ b/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) + } + } +} diff --git a/test/case/interfaces/wifi.yaml b/test/case/interfaces/wifi.yaml index 824e2fa697..7586609bf8 100644 --- a/test/case/interfaces/wifi.yaml +++ b/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 diff --git a/test/case/interfaces/wifi_ap_station_2dut/2_4ghz.adoc b/test/case/interfaces/wifi_ap_station_2dut/2_4ghz.adoc new file mode 100644 index 0000000000..db3b89d239 --- /dev/null +++ b/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 + + diff --git a/test/case/interfaces/wifi_ap_station_2dut/2_4ghz.py b/test/case/interfaces/wifi_ap_station_2dut/2_4ghz.py new file mode 120000 index 0000000000..9465664311 --- /dev/null +++ b/test/case/interfaces/wifi_ap_station_2dut/2_4ghz.py @@ -0,0 +1 @@ +test.py \ No newline at end of file diff --git a/test/case/interfaces/wifi_ap_station_2dut/test.adoc b/test/case/interfaces/wifi_ap_station_2dut/5ghz.adoc similarity index 85% rename from test/case/interfaces/wifi_ap_station_2dut/test.adoc rename to test/case/interfaces/wifi_ap_station_2dut/5ghz.adoc index f7b300e941..4ebbef8def 100644 --- a/test/case/interfaces/wifi_ap_station_2dut/test.adoc +++ b/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 diff --git a/test/case/interfaces/wifi_ap_station_2dut/5ghz.py b/test/case/interfaces/wifi_ap_station_2dut/5ghz.py new file mode 120000 index 0000000000..9465664311 --- /dev/null +++ b/test/case/interfaces/wifi_ap_station_2dut/5ghz.py @@ -0,0 +1 @@ +test.py \ No newline at end of file diff --git a/test/case/interfaces/wifi_ap_station_2dut/Readme.adoc b/test/case/interfaces/wifi_ap_station_2dut/Readme.adoc deleted file mode 120000 index ae32c84120..0000000000 --- a/test/case/interfaces/wifi_ap_station_2dut/Readme.adoc +++ /dev/null @@ -1 +0,0 @@ -test.adoc \ No newline at end of file diff --git a/test/case/interfaces/wifi_ap_station_2dut/Readme.adoc b/test/case/interfaces/wifi_ap_station_2dut/Readme.adoc new file mode 100644 index 0000000000..4ca5c26f3c --- /dev/null +++ b/test/case/interfaces/wifi_ap_station_2dut/Readme.adoc @@ -0,0 +1,6 @@ +include::2_4ghz.adoc[] + +<<< + +include::5ghz.adoc[] + diff --git a/test/case/interfaces/wifi_ap_station_2dut/test.py b/test/case/interfaces/wifi_ap_station_2dut/test.py index 307b65fc9a..b71ce4e5b1 100755 --- a/test/case/interfaces/wifi_ap_station_2dut/test.py +++ b/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,7 +42,11 @@ 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"), @@ -50,14 +54,14 @@ ) 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"}, diff --git a/test/case/interfaces/wifi_ap_station_2dut/test.yaml b/test/case/interfaces/wifi_ap_station_2dut/test.yaml new file mode 100644 index 0000000000..b0223d23a9 --- /dev/null +++ b/test/case/interfaces/wifi_ap_station_2dut/test.yaml @@ -0,0 +1,11 @@ +--- +- settings: + test-spec: .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"] diff --git a/test/case/interfaces/wifi_ap_station_2dut/topology.dot b/test/case/interfaces/wifi_ap_station_2dut/topology.dot index ff7c5327c2..7d0047f9c1 100644 --- a/test/case/interfaces/wifi_ap_station_2dut/topology.dot +++ b/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"] } diff --git a/test/infamy/wifi.py b/test/infamy/wifi.py index bdcb0fab08..43e729347e 100644 --- a/test/infamy/wifi.py +++ b/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. + """ + 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."""