Handle WiFi drivers w/o separate P2P interfaces

Not all WiFi drivers expose a separate P2P control interface. This refactors the control file logic so that it tries both the normal control interface and the P2P interface when the driver is in AP mode. This fixes an issue with a USB WiFi module that doesn't have this interface. The Raspberry Pi, for example, has both the P2P and normal control interface, but only the P2P interface is used when in AP mode.
nerves-networking · Jul 22, 2019 · f7761b6 · f7761b6
1 parent 3927949
commit f7761b6
Show file tree

Hide file tree

Showing 4 changed files with 79 additions and 60 deletions.
diff --git a/lib/vintage_net/technology/wifi.ex b/lib/vintage_net/technology/wifi.ex
@@ -26,7 +26,7 @@ defmodule VintageNet.Technology.WiFi do
     network_interfaces_path = Path.join(tmpdir, "network_interfaces.#{ifname}")
     wpa_supplicant_conf_path = Path.join(tmpdir, "wpa_supplicant.conf.#{ifname}")
     control_interface_dir = Path.join(tmpdir, "wpa_supplicant")
-    control_interface_path = ctrl_interface_path(ifname, control_interface_dir, config)
+    control_interface_paths = ctrl_interface_paths(ifname, control_interface_dir, config)
     ap_mode = ap_mode?(config)
 
     {:ok, normalized_config} = normalize(config)
@@ -62,11 +62,11 @@ defmodule VintageNet.Technology.WiFi do
            type: __MODULE__,
            source_config: normalized_config,
            files: files ++ udhcpd_files,
-           cleanup_files: [control_interface_path],
+           cleanup_files: control_interface_paths,
            child_specs: [
              {VintageNet.Interface.ConnectivityChecker, ifname},
              {WPASupplicant,
-              ifname: ifname, control_path: control_interface_path, ap_mode: ap_mode}
+              ifname: ifname, control_path: control_interface_dir, ap_mode: ap_mode}
            ],
            up_cmds: up_cmds ++ udhcpd_up_cmds,
            up_cmd_millis: 60_000,
@@ -80,11 +80,11 @@ defmodule VintageNet.Technology.WiFi do
            type: __MODULE__,
            source_config: normalized_config,
            files: files,
-           cleanup_files: [control_interface_path],
+           cleanup_files: control_interface_paths,
            child_specs: [
              {VintageNet.Interface.ConnectivityChecker, ifname},
              {WPASupplicant,
-              ifname: ifname, control_path: control_interface_path, ap_mode: ap_mode}
+              ifname: ifname, control_path: control_interface_dir, ap_mode: ap_mode}
            ],
            up_cmds: up_cmds,
            up_cmd_millis: 60_000,
@@ -100,7 +100,7 @@ defmodule VintageNet.Technology.WiFi do
 
     wpa_supplicant_conf_path = Path.join(tmpdir, "wpa_supplicant.conf.#{ifname}")
     control_interface_dir = Path.join(tmpdir, "wpa_supplicant")
-    control_interface_path = ctrl_interface_path(ifname, control_interface_dir, config)
+    control_interface_paths = ctrl_interface_paths(ifname, control_interface_dir, config)
 
     files = [
       {wpa_supplicant_conf_path, "ctrl_interface=#{control_interface_dir}"}
@@ -122,11 +122,11 @@ defmodule VintageNet.Technology.WiFi do
        files: files,
        child_specs: [
          {VintageNet.Interface.ConnectivityChecker, ifname},
-         {WPASupplicant, ifname: ifname, control_path: control_interface_path, ap_mode: false}
+         {WPASupplicant, ifname: ifname, control_path: control_interface_dir, ap_mode: false}
        ],
        up_cmds: up_cmds,
        down_cmds: down_cmds,
-       cleanup_files: [control_interface_path]
+       cleanup_files: control_interface_paths
      }}
   end
 
@@ -448,9 +448,11 @@ defmodule VintageNet.Technology.WiFi do
   defp ap_mode?(%{wifi: %{mode: mode}}) when mode in [:host, 2], do: true
   defp ap_mode?(_config), do: false
 
-  defp ctrl_interface_path(ifname, dir, %{wifi: %{mode: mode}}) when mode in [:host, 2],
-    do: Path.join(dir, "p2p-dev-#{ifname}")
+  defp ctrl_interface_paths(ifname, dir, %{wifi: %{mode: mode}}) when mode in [:host, 2] do
+    # Some WiFi drivers expose P2P interfaces and those should be cleaned up too.
+    [Path.join(dir, "p2p-dev-#{ifname}"), Path.join(dir, ifname)]
+  end
 
-  defp ctrl_interface_path(ifname, dir, _),
-    do: Path.join(dir, ifname)
+  defp ctrl_interface_paths(ifname, dir, _),
+    do: [Path.join(dir, ifname)]
 end
diff --git a/lib/vintage_net/wifi/wpa_supplicant.ex b/lib/vintage_net/wifi/wpa_supplicant.ex
@@ -40,13 +40,13 @@ defmodule VintageNet.WiFi.WPASupplicant do
 
   @impl true
   def init(args) do
-    control_path = Keyword.fetch!(args, :control_path)
+    control_dir = Keyword.fetch!(args, :control_path)
     ifname = Keyword.fetch!(args, :ifname)
     keep_alive_interval = Keyword.get(args, :keep_alive_interval, 60000)
     ap_mode = Keyword.get(args, :ap_mode, false)
 
     state = %{
-      control_path: control_path,
+      control_dir: control_dir,
       keep_alive_interval: keep_alive_interval,
       ifname: ifname,
       ap_mode: ap_mode,
@@ -60,9 +60,15 @@ defmodule VintageNet.WiFi.WPASupplicant do
 
   @impl true
   def handle_continue(:continue, state) do
-    :ok = wait_for_file(state.control_path)
+    # The control file paths depend whether the config uses AP mode and whether
+    # the driver has a separate P2P interface. We find out based on which
+    # control files appear.
+    control_paths = get_control_paths(state)
 
-    {:ok, ll} = WPASupplicantLL.start_link(state.control_path)
+    # Wait for the wpa_supplicant to create its control files.
+    {:ok, control_path} = wait_for_control_file(control_paths)
+
+    {:ok, ll} = WPASupplicantLL.start_link(control_path)
     :ok = WPASupplicantLL.subscribe(ll)
 
     {:ok, "OK\n"} = WPASupplicantLL.control_request(ll, "ATTACH")
@@ -233,18 +239,28 @@ defmodule VintageNet.WiFi.WPASupplicant do
     )
   end
 
-  defp wait_for_file(path, timeleft \\ 3000)
+  defp get_control_paths(%{control_dir: dir, ap_mode: true, ifname: ifname} = _state) do
+    [Path.join(dir, "p2p-dev-#{ifname}"), Path.join(dir, ifname)]
+  end
 
-  defp wait_for_file(path, timeleft) when timeleft <= 0 do
-    {:error, "#{path} not found"}
+  defp get_control_paths(%{control_dir: dir, ifname: ifname}) do
+    [Path.join(dir, ifname)]
   end
 
-  defp wait_for_file(path, timeleft) do
-    if File.exists?(path) do
-      :ok
-    else
-      Process.sleep(250)
-      wait_for_file(path, timeleft - 250)
+  defp wait_for_control_file(paths, time_left \\ 3000)
+
+  defp wait_for_control_file(paths, time_left) when time_left <= 0 do
+    {:error, "#{inspect(paths)} not found"}
+  end
+
+  defp wait_for_control_file(paths, time_left) do
+    case Enum.find(paths, &File.exists?/1) do
+      nil ->
+        Process.sleep(250)
+        wait_for_control_file(paths, time_left - 250)
+
+      path ->
+        {:ok, path}
     end
   end
 end
diff --git a/test/vintage_net/technology/wifi_test.exs b/test/vintage_net/technology/wifi_test.exs
@@ -51,7 +51,7 @@ defmodule VintageNet.Technology.WiFiTest do
       child_specs: [
         {VintageNet.Interface.ConnectivityChecker, "wlan0"},
         {VintageNet.WiFi.WPASupplicant,
-         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant/wlan0", ap_mode: false]}
+         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant", ap_mode: false]}
       ],
       files: [
         {"/tmp/vintage_net/network_interfaces.wlan0", dhcp_interface("wlan0", "unit_test")},
@@ -104,7 +104,7 @@ defmodule VintageNet.Technology.WiFiTest do
       child_specs: [
         {VintageNet.Interface.ConnectivityChecker, "wlan0"},
         {VintageNet.WiFi.WPASupplicant,
-         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant/wlan0", ap_mode: false]}
+         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant", ap_mode: false]}
       ],
       files: [
         {"/tmp/vintage_net/network_interfaces.wlan0", dhcp_interface("wlan0", "unit_test")},
@@ -156,7 +156,7 @@ defmodule VintageNet.Technology.WiFiTest do
       child_specs: [
         {VintageNet.Interface.ConnectivityChecker, "wlan0"},
         {VintageNet.WiFi.WPASupplicant,
-         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant/wlan0", ap_mode: false]}
+         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant", ap_mode: false]}
       ],
       files: [
         {"/tmp/vintage_net/network_interfaces.wlan0", dhcp_interface("wlan0", "unit_test")},
@@ -213,7 +213,7 @@ defmodule VintageNet.Technology.WiFiTest do
       child_specs: [
         {VintageNet.Interface.ConnectivityChecker, "wlan0"},
         {VintageNet.WiFi.WPASupplicant,
-         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant/wlan0", ap_mode: false]}
+         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant", ap_mode: false]}
       ],
       files: [
         {"/tmp/vintage_net/network_interfaces.wlan0", dhcp_interface("wlan0", "unit_test")},
@@ -272,7 +272,7 @@ defmodule VintageNet.Technology.WiFiTest do
       child_specs: [
         {VintageNet.Interface.ConnectivityChecker, "wlan0"},
         {VintageNet.WiFi.WPASupplicant,
-         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant/wlan0", ap_mode: false]}
+         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant", ap_mode: false]}
       ],
       files: [
         {"/tmp/vintage_net/network_interfaces.wlan0", dhcp_interface("wlan0", "unit_test")},
@@ -333,7 +333,7 @@ defmodule VintageNet.Technology.WiFiTest do
       child_specs: [
         {VintageNet.Interface.ConnectivityChecker, "wlan0"},
         {VintageNet.WiFi.WPASupplicant,
-         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant/wlan0", ap_mode: false]}
+         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant", ap_mode: false]}
       ],
       files: [
         {"/tmp/vintage_net/network_interfaces.wlan0", dhcp_interface("wlan0", "unit_test")},
@@ -397,7 +397,7 @@ defmodule VintageNet.Technology.WiFiTest do
       child_specs: [
         {VintageNet.Interface.ConnectivityChecker, "wlan0"},
         {VintageNet.WiFi.WPASupplicant,
-         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant/wlan0", ap_mode: false]}
+         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant", ap_mode: false]}
       ],
       files: [
         {"/tmp/vintage_net/network_interfaces.wlan0", dhcp_interface("wlan0", "unit_test")},
@@ -461,7 +461,7 @@ defmodule VintageNet.Technology.WiFiTest do
       child_specs: [
         {VintageNet.Interface.ConnectivityChecker, "wlan0"},
         {VintageNet.WiFi.WPASupplicant,
-         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant/wlan0", ap_mode: false]}
+         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant", ap_mode: false]}
       ],
       files: [
         {"/tmp/vintage_net/network_interfaces.wlan0", dhcp_interface("wlan0", "unit_test")},
@@ -527,7 +527,7 @@ defmodule VintageNet.Technology.WiFiTest do
       child_specs: [
         {VintageNet.Interface.ConnectivityChecker, "wlan0"},
         {VintageNet.WiFi.WPASupplicant,
-         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant/wlan0", ap_mode: false]}
+         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant", ap_mode: false]}
       ],
       files: [
         {"/tmp/vintage_net/network_interfaces.wlan0", dhcp_interface("wlan0", "unit_test")},
@@ -591,7 +591,7 @@ defmodule VintageNet.Technology.WiFiTest do
       child_specs: [
         {VintageNet.Interface.ConnectivityChecker, "wlan0"},
         {VintageNet.WiFi.WPASupplicant,
-         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant/wlan0", ap_mode: false]}
+         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant", ap_mode: false]}
       ],
       files: [
         {"/tmp/vintage_net/network_interfaces.wlan0", dhcp_interface("wlan0", "unit_test")},
@@ -657,7 +657,7 @@ defmodule VintageNet.Technology.WiFiTest do
       child_specs: [
         {VintageNet.Interface.ConnectivityChecker, "wlan0"},
         {VintageNet.WiFi.WPASupplicant,
-         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant/wlan0", ap_mode: false]}
+         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant", ap_mode: false]}
       ],
       files: [
         {"/tmp/vintage_net/network_interfaces.wlan0", dhcp_interface("wlan0", "unit_test")},
@@ -720,7 +720,7 @@ defmodule VintageNet.Technology.WiFiTest do
       child_specs: [
         {VintageNet.Interface.ConnectivityChecker, "wlan0"},
         {VintageNet.WiFi.WPASupplicant,
-         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant/wlan0", ap_mode: false]}
+         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant", ap_mode: false]}
       ],
       files: [
         {"/tmp/vintage_net/network_interfaces.wlan0", dhcp_interface("wlan0", "unit_test")},
@@ -778,7 +778,7 @@ defmodule VintageNet.Technology.WiFiTest do
       child_specs: [
         {VintageNet.Interface.ConnectivityChecker, "wlan0"},
         {VintageNet.WiFi.WPASupplicant,
-         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant/wlan0", ap_mode: false]}
+         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant", ap_mode: false]}
       ],
       files: [
         {"/tmp/vintage_net/network_interfaces.wlan0", dhcp_interface("wlan0", "unit_test")},
@@ -840,7 +840,7 @@ defmodule VintageNet.Technology.WiFiTest do
       child_specs: [
         {VintageNet.Interface.ConnectivityChecker, "wlan0"},
         {VintageNet.WiFi.WPASupplicant,
-         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant/wlan0", ap_mode: false]}
+         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant", ap_mode: false]}
       ],
       files: [
         {"/tmp/vintage_net/network_interfaces.wlan0", dhcp_interface("wlan0", "unit_test")},
@@ -899,7 +899,7 @@ defmodule VintageNet.Technology.WiFiTest do
       child_specs: [
         {VintageNet.Interface.ConnectivityChecker, "wlan0"},
         {VintageNet.WiFi.WPASupplicant,
-         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant/wlan0", ap_mode: false]}
+         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant", ap_mode: false]}
       ],
       files: [
         {"/tmp/vintage_net/network_interfaces.wlan0", dhcp_interface("wlan0", "unit_test")},
@@ -952,7 +952,7 @@ defmodule VintageNet.Technology.WiFiTest do
       child_specs: [
         {VintageNet.Interface.ConnectivityChecker, "wlan0"},
         {VintageNet.WiFi.WPASupplicant,
-         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant/wlan0", ap_mode: false]}
+         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant", ap_mode: false]}
       ],
       files: [
         {"/tmp/vintage_net/network_interfaces.wlan0", dhcp_interface("wlan0", "unit_test")},
@@ -1006,11 +1006,7 @@ defmodule VintageNet.Technology.WiFiTest do
       child_specs: [
         {VintageNet.Interface.ConnectivityChecker, "wlan0"},
         {VintageNet.WiFi.WPASupplicant,
-         [
-           ifname: "wlan0",
-           control_path: "/tmp/vintage_net/wpa_supplicant/p2p-dev-wlan0",
-           ap_mode: true
-         ]}
+         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant", ap_mode: true]}
       ],
       files: [
         {"/tmp/vintage_net/network_interfaces.wlan0", dhcp_interface("wlan0", "unit_test")},
@@ -1039,7 +1035,10 @@ defmodule VintageNet.Technology.WiFiTest do
         {:run, "ifdown", ["-i", "/tmp/vintage_net/network_interfaces.wlan0", "wlan0"]},
         {:run, "killall", ["-q", "wpa_supplicant"]}
       ],
-      cleanup_files: ["/tmp/vintage_net/wpa_supplicant/p2p-dev-wlan0"]
+      cleanup_files: [
+        "/tmp/vintage_net/wpa_supplicant/p2p-dev-wlan0",
+        "/tmp/vintage_net/wpa_supplicant/wlan0"
+      ]
     }
 
     assert {:ok, output} == WiFi.to_raw_config("wlan0", input, default_opts())
@@ -1083,7 +1082,7 @@ defmodule VintageNet.Technology.WiFiTest do
       child_specs: [
         {VintageNet.Interface.ConnectivityChecker, "wlan0"},
         {VintageNet.WiFi.WPASupplicant,
-         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant/wlan0", ap_mode: false]}
+         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant", ap_mode: false]}
       ],
       files: [
         {"/tmp/vintage_net/network_interfaces.wlan0", dhcp_interface("wlan0", "unit_test")},
@@ -1160,7 +1159,7 @@ defmodule VintageNet.Technology.WiFiTest do
       child_specs: [
         {VintageNet.Interface.ConnectivityChecker, "wlan0"},
         {VintageNet.WiFi.WPASupplicant,
-         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant/wlan0", ap_mode: false]}
+         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant", ap_mode: false]}
       ],
       files: [
         {"/tmp/vintage_net/network_interfaces.wlan0",
@@ -1238,11 +1237,7 @@ defmodule VintageNet.Technology.WiFiTest do
       child_specs: [
         {VintageNet.Interface.ConnectivityChecker, "wlan0"},
         {VintageNet.WiFi.WPASupplicant,
-         [
-           ifname: "wlan0",
-           control_path: "/tmp/vintage_net/wpa_supplicant/p2p-dev-wlan0",
-           ap_mode: true
-         ]}
+         [ifname: "wlan0", control_path: "/tmp/vintage_net/wpa_supplicant", ap_mode: true]}
       ],
       files: [
         {"/tmp/vintage_net/network_interfaces.wlan0",
@@ -1293,7 +1288,10 @@ defmodule VintageNet.Technology.WiFiTest do
         {:run, "killall", ["-q", "wpa_supplicant"]},
         {:run, "killall", ["-q", "udhcpd"]}
       ],
-      cleanup_files: ["/tmp/vintage_net/wpa_supplicant/p2p-dev-wlan0"]
+      cleanup_files: [
+        "/tmp/vintage_net/wpa_supplicant/p2p-dev-wlan0",
+        "/tmp/vintage_net/wpa_supplicant/wlan0"
+      ]
     }
 
     assert {:ok, output} == WiFi.to_raw_config("wlan0", input, default_opts())

diff --git a/test/vintage_net/wifi/wpa_supplicant_test.exs b/test/vintage_net/wifi/wpa_supplicant_test.exs
@@ -6,19 +6,22 @@ defmodule VintageNet.WiFi.WPASupplicantTest do
 
   setup do
     socket_path = "test_tmp/tmp_wpa_supplicant_socket"
-    mock = start_supervised!({MockWPASupplicant, socket_path})
+    File.mkdir!(socket_path)
+
+    mock = start_supervised!({MockWPASupplicant, Path.join(socket_path, "test_wlan0")})
 
     on_exit(fn ->
-      _ = File.rm(socket_path)
-      _ = File.rm(socket_path <> ".ex")
+      _ = File.rm_rf(socket_path)
     end)
 
     {:ok, socket_path: socket_path, mock: mock}
   end
 
   test "attaches to wpa_supplicant", context do
     MockWPASupplicant.set_responses(context.mock, %{"ATTACH" => ["OK\n"]})
-    _ = start_supervised!({WPASupplicant, ifname: "wlan0", control_path: context.socket_path})
+
+    _ =
+      start_supervised!({WPASupplicant, ifname: "test_wlan0", control_path: context.socket_path})
 
     Process.sleep(100)
 
@@ -32,7 +35,7 @@ defmodule VintageNet.WiFi.WPASupplicantTest do
     _ =
       start_supervised!(
         {WPASupplicant,
-         ifname: "wlan0", control_path: context.socket_path, keep_alive_interval: 10}
+         ifname: "test_wlan0", control_path: context.socket_path, keep_alive_interval: 10}
       )
 
     Process.sleep(100)