From 1bd9af1e9d38d8b241f1d838e4af98e1b13b958b Mon Sep 17 00:00:00 2001 From: Raito Bezarius Date: Sun, 1 Sep 2024 15:42:26 +0200 Subject: [PATCH] fix(bridge): reorder initialization for bridge dependents Consider the scenario where you run DHCPv4 on the primary bridge interface. You have no real interface to "wait upon", so it's OK. Nonetheless, anything depending on successful completion of DHCPv4, e.g. adding a default route, will block `s6-rc -v2 up change default`. The way new interfaces are attached to the bridge is via `s6-rc -b -u change $attach-oneshot-service`, this introduce in turn a deadlock. At some point, DHCPv4 will timeout, unblocking the deadlock and attaching the members to the primary bridge interface, making it ready to send L2 broadcast packets for DHCP, unblocking DHCP in turn again. This is not satisfying because we really want to have a no-hiccups bring-up. To fix this, we proceed to multiple changes: - we remove `svc.ifwait.build` out of band `s6-rc -b -u $oneshot-attach` call, which is, by design, wrong here. - users can now depend on the members service to know when a bridge is fully operational (we could make it more granular and let them depend on the LAN member joining rather than WLAN, etc.) - users can also depend on the primary service being brought up rather than just being present, this is useful if you need to bring it up when it has AT LEAST one member to get link local address or MAC addresses (fixing DHCPv6 bring up as well because `ff02::1` is used there). One thing is not addressed yet, if you are running a WLAN service using RADIUS attached to the bridge, at bring up time, it will try to reach out the external RADIUS server and *fail*. To solve this, granular dependency on the DHCPv4 once LAN is joined. Then the hostapd can wait on defaultroute4 completion so that connectivity is available to reach RADIUS server. It can join the bridge later on without any hiccup as well. This is left as a TODO as hostapd can survive RADIUS authentication failure and retry later. Signed-off-by: Raito Bezarius --- modules/bridge/default.nix | 14 ++++++++++++++ modules/bridge/members.nix | 33 +++++++++++++-------------------- modules/bridge/primary.nix | 8 ++++++-- modules/bridge/ready.nix | 18 ++++++++++++++++++ 4 files changed, 51 insertions(+), 22 deletions(-) create mode 100644 modules/bridge/ready.nix diff --git a/modules/bridge/default.nix b/modules/bridge/default.nix index fd779f8..4c2911a 100644 --- a/modules/bridge/default.nix +++ b/modules/bridge/default.nix @@ -20,6 +20,7 @@ in system.service.bridge = { primary = mkOption { type = liminix.lib.types.serviceDefn; }; members = mkOption { type = liminix.lib.types.serviceDefn; }; + ready = mkOption { type = liminix.lib.types.serviceDefn; }; }; }; config.system.service.bridge = { @@ -46,6 +47,19 @@ in description = "interfaces to add to the bridge"; }; }; + + # TODO: generalize it outside + ready = config.system.callService ./ready.nix { + primary = mkOption { + type = liminix.lib.types.service; + description = "primary bridge interface"; + }; + + members = mkOption { + type = liminix.lib.types.service; + description = "members service"; + }; + }; }; config.kernel.config = { BRIDGE = "y"; diff --git a/modules/bridge/members.nix b/modules/bridge/members.nix index 114bc2e..2171eda 100644 --- a/modules/bridge/members.nix +++ b/modules/bridge/members.nix @@ -10,26 +10,19 @@ let inherit (liminix.networking) interface; inherit (liminix.services) bundle oneshot; inherit (lib) mkOption types; - addif = member : - # how do we get sight of services from here? maybe we need to - # implement ifwait as a regualr derivation instead of a - # servicedefinition - svc.ifwait.build { - state = "running"; - interface = member; - dependencies = [ primary member ]; - service = oneshot { - name = "${primary.name}.member.${member.name}"; - up = '' - echo "attaching $(output ${member} ifname) to $(output ${primary} ifname) bridge" - ip link set dev $(output ${member} ifname) master $(output ${primary} ifname) - ''; - down = '' - echo "detaching $(output ${member} ifname) from any bridge" - ip link set dev $(output ${member} ifname) nomaster - ''; - }; - }; + addif = member : oneshot { + name = "${primary.name}.member.${member.name}"; + up = '' + echo "attaching $(output ${member} ifname) to $(output ${primary} ifname) bridge" + ip link set dev $(output ${member} ifname) master $(output ${primary} ifname) + ''; + down = '' + echo "detaching $(output ${member} ifname) from any bridge" + ip link set dev $(output ${member} ifname) nomaster + ''; + + dependencies = [ primary member ]; + }; in bundle { name = "${primary.name}.members"; contents = map addif members; diff --git a/modules/bridge/primary.nix b/modules/bridge/primary.nix index aa7822c..f5e1219 100644 --- a/modules/bridge/primary.nix +++ b/modules/bridge/primary.nix @@ -14,9 +14,13 @@ in oneshot rec { "ip link add name ${ifname} type bridge" else "ip link add name ${ifname} address $(output ${macAddressFromInterface} ether) type bridge"} - ${liminix.networking.ifup name ifname} + + (in_outputs ${name} + echo ${ifname} > ifname + cat /sys/class/net/${ifname}/address > ether + ) ''; - down = "ip link set down dev ${ifname}"; + down = "ip link delete ${ifname}"; dependencies = optional (macAddressFromInterface != null) macAddressFromInterface; } diff --git a/modules/bridge/ready.nix b/modules/bridge/ready.nix new file mode 100644 index 0000000..f8c40b3 --- /dev/null +++ b/modules/bridge/ready.nix @@ -0,0 +1,18 @@ +{ + liminix +, ifwait +, lib +}: +{ primary, members } : +let + inherit (liminix.services) oneshot; +in oneshot { + name = "${primary.name}.oper"; + up = '' + ip link set up dev $(output ${primary} ifname) + ${ifwait}/bin/ifwait -v $(output ${primary} ifname) running + ''; + down = "ip link set down dev $(output ${primary} ifname)"; + + dependencies = [ members ]; +}