From 31f0213b6fc66534125e27ebf082c133ff2cc62e Mon Sep 17 00:00:00 2001
From: Daniel Barlow <dan@telent.net>
Date: Sun, 27 Aug 2023 23:45:27 +0100
Subject: [PATCH] convert network link/address to module-based-service

... and make bridge use it.

We also had to convert bridge back into a pair of services.
Downstreams want to depend on the bridge it self being configured
even if not necessarily all the members are up. e.g. don't want
to break ssh on lan if there's a misconfigured wlan device
---
 devices/gl-ar750/default.nix | 27 ++++++++++--------
 examples/rotuer.nix          | 23 +++++++--------
 modules/bridge/default.nix   | 24 ++++++++++------
 modules/bridge/members.nix   | 25 +++++++++++++++++
 modules/bridge/primary.nix   | 18 ++++++++++++
 modules/bridge/service.nix   | 27 ------------------
 modules/network/address.nix  | 29 +++++++++++++++++++
 modules/network/default.nix  | 54 ++++++++++++++++++++++++++++++++++++
 modules/network/link.nix     | 16 +++++++++++
 9 files changed, 185 insertions(+), 58 deletions(-)
 create mode 100644 modules/bridge/members.nix
 create mode 100644 modules/bridge/primary.nix
 delete mode 100644 modules/bridge/service.nix
 create mode 100644 modules/network/address.nix
 create mode 100644 modules/network/default.nix
 create mode 100644 modules/network/link.nix

diff --git a/devices/gl-ar750/default.nix b/devices/gl-ar750/default.nix
index ee341392..51af49d7 100644
--- a/devices/gl-ar750/default.nix
+++ b/devices/gl-ar750/default.nix
@@ -80,6 +80,8 @@
       inherit (pkgs.pseudofile) dir symlink;
       inherit (pkgs.liminix.networking) interface;
     in {
+      imports = [ ../../modules/network];
+
       programs.busybox.options = {
         FEATURE_DD_IBS_OBS = "y"; # ath10k_cal_data needs skip_bytes,fullblock
       };
@@ -100,19 +102,20 @@
           ];
         };
 
-        networkInterfaces = {
-          lan = interface { device = "eth0"; };
-          wan = interface { device = "eth1"; };
-
-          wlan_24 = interface {
-            device = "wlan0";
-            dependencies = [ mac80211 ];
+        networkInterfaces =
+          let inherit (config.system.service.network) link;
+          in {
+            lan = link.build { ifname = "eth0"; };
+            wan = link.build { ifname = "eth1"; };
+            wlan_24 = link.build {
+              ifname = "wlan0";
+              dependencies = [ mac80211 ];
+            };
+            wlan_5 = link.build {
+              ifname = "wlan1";
+              dependencies = [ mac80211 ath10k_cal_data ];
+            };
           };
-          wlan_5 = interface {
-            device = "wlan1";
-            dependencies = [ mac80211 ath10k_cal_data ];
-          };
-        };
       };
       filesystem = dir {
         lib = dir {
diff --git a/examples/rotuer.nix b/examples/rotuer.nix
index 7e0cbfb7..6b3abbe9 100644
--- a/examples/rotuer.nix
+++ b/examples/rotuer.nix
@@ -44,6 +44,7 @@ in rec {
   imports = [
     ../modules/wlan.nix
     ../modules/standard.nix
+    ../modules/network
     ../modules/ppp
     ../modules/dnsmasq
     ../modules/firewall
@@ -78,16 +79,16 @@ in rec {
     } // wirelessConfig;
   };
 
-  services.int =
-    let iface = svc.bridge.build {
-          ifname = "int";
-          members = with config.hardware.networkInterfaces;  [
-            wlan_24 lan wlan_5
-          ];
-        };
-    in address iface {
-      family = "inet4"; address ="10.8.0.1"; prefixLength = 16;
-    };
+  services.int = svc.network.address.build {
+    interface = svc.bridge.primary.build { ifname = "int"; };# services.int;
+    family = "inet"; address ="10.8.0.1"; prefixLength = 16;
+  };
+
+  services.bridge =  svc.bridge.members.build {
+    primary = services.int;
+    members = with config.hardware.networkInterfaces;
+      [ wlan_24 wlan_5  lan ];
+  };
 
   services.ntp = svc.ntp.build {
     pools = { "pool.ntp.org" = ["iburst"]; };
@@ -204,8 +205,8 @@ in rec {
     name = "default";
     contents = with config.services; [
       config.hardware.networkInterfaces.lo
-      config.hardware.networkInterfaces.lan
       int
+      bridge
       hostap
       hostap5
       ntp
diff --git a/modules/bridge/default.nix b/modules/bridge/default.nix
index 79440cb6..1145ec7b 100644
--- a/modules/bridge/default.nix
+++ b/modules/bridge/default.nix
@@ -15,21 +15,29 @@ let
 in
 {
   options = {
-    system.service.bridge = mkOption {
-      type = liminix.lib.types.serviceDefn;
+    system.service.bridge = {
+      primary = mkOption { type = liminix.lib.types.serviceDefn; };
+      members = mkOption { type = liminix.lib.types.serviceDefn; };
     };
   };
-  config.system.service = {
-    bridge = liminix.callService ./service.nix {
-      members = mkOption {
-        type = types.listOf liminix.lib.types.service;
-        description = "interfaces to add to the bridge";
-      };
+  config.system.service.bridge = {
+    primary = liminix.callService ./primary.nix {
       ifname = mkOption {
         type = types.str;
         description = "bridge interface name to create";
       };
     };
+    members = liminix.callService ./members.nix {
+      primary = mkOption {
+        type = liminix.lib.types.interface;
+        description = "primary bridge interface";
+      };
+
+      members = mkOption {
+        type = types.listOf liminix.lib.types.interface;
+        description = "interfaces to add to the bridge";
+      };
+    };
   };
   config.kernel.config.BRIDGE = "y";
 }
diff --git a/modules/bridge/members.nix b/modules/bridge/members.nix
new file mode 100644
index 00000000..ca1989e3
--- /dev/null
+++ b/modules/bridge/members.nix
@@ -0,0 +1,25 @@
+{
+  liminix
+, ifwait
+, lib
+}:
+{ members, primary } :
+
+let
+  inherit (liminix.networking) interface;
+  inherit (liminix.services) bundle oneshot;
+  inherit (lib) mkOption types;
+  addif = member :
+    oneshot {
+      name = "${primary.name}.member.${member.name}";
+      up = ''
+        dev=$(output ${member} ifname)
+        ${ifwait}/bin/ifwait $dev running && ip link set dev $dev master $(output ${primary} ifname)
+      '';
+      down = "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
new file mode 100644
index 00000000..bd07d94b
--- /dev/null
+++ b/modules/bridge/primary.nix
@@ -0,0 +1,18 @@
+{
+  liminix
+, ifwait
+, lib
+}:
+{ ifname } :
+let
+  inherit (liminix.networking) interface;
+  inherit (liminix.services) bundle oneshot;
+  inherit (lib) mkOption types;
+in oneshot rec {
+  name = "${ifname}.link";
+  up = ''
+    ip link add name ${ifname} type bridge
+    ${liminix.networking.ifup name ifname}
+  '';
+  down = "ip link set down dev ${ifname}";
+}
diff --git a/modules/bridge/service.nix b/modules/bridge/service.nix
deleted file mode 100644
index 28194ae9..00000000
--- a/modules/bridge/service.nix
+++ /dev/null
@@ -1,27 +0,0 @@
-{
-  liminix
-, ifwait
-, lib
-}:
-{ members, ifname } :
-let
-  inherit (liminix.networking) interface;
-  inherit (liminix.services) bundle oneshot;
-  inherit (lib) mkOption types;
-  primary = interface {
-    device = ifname;
-    type = "bridge";
-  };
-  addif = member :
-    let ifname = "$(output ${member} ifname)";
-    in oneshot {
-      name = "add-${member.name}-to-br-${primary.name}";
-      up = "${ifwait}/bin/ifwait ${ifname} running && ip link set dev ${ifname} master $(output ${primary} ifname)";
-      down = "ip link set dev ${ifname} nomaster";
-      dependencies = [ primary member ];
-    };
-
-in bundle {
-  name = "bridge-${primary.name}-members";
-  contents = [ primary ] ++ map addif members;
-}
diff --git a/modules/network/address.nix b/modules/network/address.nix
new file mode 100644
index 00000000..0370f137
--- /dev/null
+++ b/modules/network/address.nix
@@ -0,0 +1,29 @@
+{
+  liminix
+, ifwait
+, serviceFns
+, lib
+}:
+{interface, family, address, prefixLength} :
+let
+  inherit (liminix.services) oneshot;
+  # rather depending on the assumption that nobody will
+  # ever add two addresses which are the same but with different
+  # prefixes, or the same but different protocols
+  name = "${interface.name}.a.${address}";
+  up = ''
+    . ${serviceFns}
+    dev=$(output ${interface} ifname)
+    ip address add ${address}/${toString prefixLength} dev $dev
+    (in_outputs ${name}
+     echo ${address} > address
+     echo ${toString prefixLength} > prefix-length
+     echo ${family} > family
+     echo $dev > ifname
+    )
+  '';
+in oneshot {
+  inherit name up;
+  down = "true";                # this has been broken for ~ ages
+  dependencies = [ interface ];
+}
diff --git a/modules/network/default.nix b/modules/network/default.nix
new file mode 100644
index 00000000..9045f1dd
--- /dev/null
+++ b/modules/network/default.nix
@@ -0,0 +1,54 @@
+## Network
+## =======
+##
+## Basic network services for creating hardware ethernet devices
+## and adding addresses
+
+
+{ lib, pkgs, config, ...}:
+let
+  inherit (lib) mkOption types;
+  inherit (pkgs) liminix;
+in {
+  options = {
+    system.service.network = {
+      link = mkOption {
+        description = "hardware network interface";
+        type = liminix.lib.types.serviceDefn;
+      };
+      address = mkOption {
+        description = "network interface address";
+        type = liminix.lib.types.serviceDefn;
+      };
+    };
+  };
+  config = {
+    system.service.network = {
+      link = liminix.callService ./link.nix {
+        ifname = mkOption {
+          type = types.str;
+          example = "eth0";
+        };
+        # other "ip link add" options could go here as well
+        mtu = mkOption {
+          type = types.nullOr types.int;
+          example = 1480;
+        };
+      };
+      address = liminix.callService ./address.nix {
+        interface = mkOption {
+          type = liminix.lib.types.service;
+        };
+        family = mkOption {
+          type = types.enum [ "inet" "inet6" ];
+        };
+        address = mkOption {
+          type = types.str;
+        };
+        prefixLength = mkOption {
+          type = types.ints.between 0 128;
+        };
+      };
+    };
+  };
+}
diff --git a/modules/network/link.nix b/modules/network/link.nix
new file mode 100644
index 00000000..d64a0d1f
--- /dev/null
+++ b/modules/network/link.nix
@@ -0,0 +1,16 @@
+{
+  liminix
+, ifwait
+, serviceFns
+, lib
+}:
+{ifname, mtu} :
+let
+  inherit (liminix.services) longrun oneshot;
+  inherit (lib) concatStringsSep;
+  name = "${ifname}.link";
+  up = liminix.networking.ifup name ifname;
+in oneshot {
+  inherit name up;
+  down = "ip link set down dev ${ifname}";
+}