Merge branch 'develop' into user/corbinphipps/add-update-config-apis

.gitignore

@@ -8,6 +8,7 @@ obj/
 # CMake
 build/
 out/
+Testing/
 
 # vcpkg
 vcpkg_installed*/

CMakePresets.json

@@ -203,7 +203,8 @@
             "name": "test-common",
             "hidden": true,
             "output": {
-                "outputOnFailure": true
+                "outputOnFailure": true,
+                "shortProgress": true
             }
         },
         {
@@ -214,24 +215,16 @@
             ]
         },
         {
-            "name": "local-base",
-            "hidden": true,
-            "description": "Run tests that don't require a remote connection",
+            "name": "non-root",
+            "configurePreset": "dev-linux",
             "inherits": [
                 "test-common"
             ],
             "filter": {
-                "include": {
-                    "label": "[local]"
+                "exclude": {
+                    "name": "(root)"
                 }
             }
-        },
-        {
-            "name": "local",
-            "configurePreset": "dev-linux",
-            "inherits": [
-                "local-base"
-            ]
         }
     ]
 }

src/common/server/NetRemoteServerConfiguration.cxx

@@ -26,6 +26,11 @@ ConfigureCliAppOptions(CLI::App& app, NetRemoteServerConfiguration& config)
         config.LogVerbosity,
         "The log verbosity level. Supply multiple times to increase verbosity (0=warnings, errors, and fatal messages, 1=info messages, 2=debug messages, 3=verbose messages)");
 
+    app.add_flag(
+        "--enable-file-logging",
+        config.EnableFileLogging,
+        "Enable logging to file (disabled by default)");
+
     return app;
 }
 

src/common/server/include/microsoft/net/remote/NetRemoteServerConfiguration.hxx

@@ -64,6 +64,11 @@ struct NetRemoteServerConfiguration
      * and a level of 3 or above will show all verbose messages.
      */
     uint32_t LogVerbosity{ 0 };
+
+    /**
+     * @brief Whether to enable logging to file or not.
+     */
+    bool EnableFileLogging{ false };
 
     /**
      * @brief Access point manager instance.

src/common/wifi/apmanager/AccessPointDiscoveryAgent.cxx

@@ -1,4 +1,7 @@
 
+#include <format>
+
+#include <magic_enum.hpp>
 #include <microsoft/net/wifi/AccessPointDiscoveryAgent.hxx>
 #include <microsoft/net/wifi/IAccessPoint.hxx>
 #include <notstd/Memory.hxx>
@@ -40,7 +43,7 @@ AccessPointDiscoveryAgent::DevicePresenceChanged(AccessPointPresenceEvent presen
 {
     std::shared_lock<std::shared_mutex> onDevicePresenceChangedLock{ m_onDevicePresenceChangedGate };
     if (m_onDevicePresenceChanged) {
-        LOGD << "Access point discovery agent detected a device presence change";
+        LOGI << std::format("Access Point Discovery Event: Interface {} {}", accessPoint->GetInterfaceName(), magic_enum::enum_name(presence));
         m_onDevicePresenceChanged(presence, std::move(accessPoint));
     }
 }
@@ -56,11 +59,13 @@ AccessPointDiscoveryAgent::Start()
 {
     bool expected = false;
     if (m_started.compare_exchange_weak(expected, true)) {
-        LOGD << "Access point discovery agent starting";
+        LOGI << "Access point discovery agent starting";
         m_operations->Start([weakThis = std::weak_ptr<AccessPointDiscoveryAgent>(GetInstance())](auto&& presence, auto&& accessPoint) {
             // Attempt to promote the weak pointer to a shared pointer to ensure this instance is still valid.
             if (auto strongThis = weakThis.lock(); strongThis) {
                 strongThis->DevicePresenceChanged(presence, std::move(accessPoint));
+            } else {
+                LOGW << "Access point discovery agent instance no longer valid; ignoring presence change event";
             }
         });
     }
@@ -71,7 +76,7 @@ AccessPointDiscoveryAgent::Stop()
 {
     bool expected = true;
     if (m_started.compare_exchange_weak(expected, false)) {
-        LOGD << "Access point discovery agent stopping";
+        LOGI << "Access point discovery agent stopping";
         m_operations->Stop();
     }
 }

src/linux/libnl-helpers/Netlink80211Interface.cxx

@@ -160,3 +160,9 @@ Nl80211Interface::GetWiphy() const
 {
     return Nl80211Wiphy::FromIndex(WiphyIndex);
 }
+
+bool
+Nl80211Interface::IsAccessPoint() const noexcept
+{
+    return (Type == nl80211_iftype::NL80211_IFTYPE_AP);
+}

src/linux/libnl-helpers/include/microsoft/net/netlink/nl80211/Netlink80211Interface.hxx

@@ -28,6 +28,9 @@ struct Nl80211Interface
 
     Nl80211Interface() = default;
 
+    auto
+    operator <=>(const Nl80211Interface&) const = default;
+
     /**
      * @brief Construct a new Nl80211Interface object with the specified attributes.
      *
@@ -73,8 +76,30 @@ struct Nl80211Interface
      */
     std::string
     ToString() const;
+
+    /**
+     * @brief Indicates if the interface is an access point.
+     * 
+     * @return true 
+     * @return false 
+     */
+    bool
+    IsAccessPoint() const noexcept;
 };
 
 } // namespace Microsoft::Net::Netlink::Nl80211
 
+namespace std
+{
+template <>
+struct hash<Microsoft::Net::Netlink::Nl80211::Nl80211Interface>
+{
+    size_t
+    operator()(const Microsoft::Net::Netlink::Nl80211::Nl80211Interface& interface) const noexcept
+    {
+        return std::hash<std::string_view>()(interface.Name);
+    }
+};
+} // namespace std
+
 #endif // NETLINK_82011_INTERFACE_HXX

src/linux/server/Main.cxx

@@ -41,10 +41,11 @@ main(int argc, char *argv[])
 
     // Configure logging, appending all loggers to the default instance.
     plog::init<notstd::to_underlying(LogInstanceId::Console)>(logSeverity, &colorConsoleAppender);
-    plog::init<notstd::to_underlying(LogInstanceId::File)>(logSeverity, &rollingFileAppender);
-    plog::init(logSeverity)
-        .addAppender(plog::get<notstd::to_underlying(LogInstanceId::Console)>())
-        .addAppender(plog::get<notstd::to_underlying(LogInstanceId::File)>());
+    plog::init(logSeverity).addAppender(plog::get<notstd::to_underlying(LogInstanceId::Console)>());
+    if (configuration.EnableFileLogging) {
+        plog::init<notstd::to_underlying(LogInstanceId::File)>(logSeverity, &rollingFileAppender);
+        plog::init(logSeverity).addAppender(plog::get<notstd::to_underlying(LogInstanceId::File)>());
+    }
 
     // Create an access point manager and discovery agent.
     {

src/linux/wifi/apmanager/AccessPointDiscoveryAgentOperationsNetlink.cxx

@@ -11,6 +11,7 @@
 #include <magic_enum.hpp>
 #include <microsoft/net/netlink/nl80211/Netlink80211.hxx>
 #include <microsoft/net/wifi/AccessPointDiscoveryAgentOperationsNetlink.hxx>
+#include <microsoft/net/wifi/AccessPointLinux.hxx>
 #include <microsoft/net/wifi/IAccessPoint.hxx>
 #include <netlink/genl/genl.h>
 #include <netlink/msg.h>
@@ -193,31 +194,36 @@ AccessPointDiscoveryAgentOperationsNetlink::ProcessNetlinkMessage(struct nl_msg
         return NL_SKIP;
     }
 
-    int interfaceIndex{ -1 };
-    const char *interfaceName{ nullptr };
-    nl80211_iftype interfaceType{ NL80211_IFTYPE_UNSPECIFIED };
-    AccessPointPresenceEvent accessPointPresenceEvent;
+    if (genlMessageHeader->cmd != NL80211_CMD_NEW_INTERFACE &&
+        genlMessageHeader->cmd != NL80211_CMD_DEL_INTERFACE &&
+        genlMessageHeader->cmd != NL80211_CMD_SET_INTERFACE) {
+        LOGV << std::format("Ignoring {} nl80211 command message", nl80211CommandName);
+    }
 
-    LOG_VERBOSE << std::format("Received {} nl80211 command message", nl80211CommandName);
+    LOGD << std::format("Received {} nl80211 command message", nl80211CommandName);
+
+    // Parse the message into an Nl80211Interface object.
+    auto nl80211InterfaceOpt = Nl80211Interface::Parse(netlinkMessage);
+    if (!nl80211InterfaceOpt.has_value()) {
+        LOGE << "Failed to parse nl80211 interface dump response";
+        return NL_OK;
+    }
+
+    AccessPointPresenceEvent accessPointPresenceEvent;
+    auto &nl80211Interface = nl80211InterfaceOpt.value();
 
     switch (genlMessageHeader->cmd) {
     case NL80211_CMD_NEW_INTERFACE:
     case NL80211_CMD_DEL_INTERFACE: {
-        interfaceIndex = static_cast<int32_t>(nla_get_u32(netlinkMessageAttributes[NL80211_ATTR_IFINDEX]));
-        interfaceName = static_cast<const char *>(nla_data(netlinkMessageAttributes[NL80211_ATTR_IFNAME]));
-        interfaceType = static_cast<nl80211_iftype>(nla_get_u32(netlinkMessageAttributes[NL80211_ATTR_IFTYPE]));
-        if (interfaceType != NL80211_IFTYPE_AP) {
-            LOG_VERBOSE << std::format("Ignoring interface presence change nl80211 message for non-ap wi-fi interface (type={})", Nl80211InterfaceTypeToString(interfaceType));
-            return NL_SKIP;
+        if (!nl80211Interface.IsAccessPoint()) {
+            LOGV << std::format("Ignoring interface presence change nl80211 message for non-ap wi-fi interface (type={})", Nl80211InterfaceTypeToString(nl80211Interface.Type));
+            return NL_OK;
         }
         accessPointPresenceEvent = (genlMessageHeader->cmd == NL80211_CMD_NEW_INTERFACE) ? AccessPointPresenceEvent::Arrived : AccessPointPresenceEvent::Departed;
         break;
     }
     case NL80211_CMD_SET_INTERFACE: {
-        interfaceIndex = static_cast<int32_t>(nla_get_u32(netlinkMessageAttributes[NL80211_ATTR_IFINDEX]));
-        interfaceName = static_cast<const char *>(nla_data(netlinkMessageAttributes[NL80211_ATTR_IFNAME]));
-        interfaceType = static_cast<nl80211_iftype>(nla_get_u32(netlinkMessageAttributes[NL80211_ATTR_IFTYPE]));
-        accessPointPresenceEvent = (interfaceType == NL80211_IFTYPE_AP) ? AccessPointPresenceEvent::Arrived : AccessPointPresenceEvent::Departed;
+        accessPointPresenceEvent = nl80211Interface.IsAccessPoint() ? AccessPointPresenceEvent::Arrived : AccessPointPresenceEvent::Departed;
         break;
     }
     default: {
@@ -226,24 +232,26 @@ AccessPointDiscoveryAgentOperationsNetlink::ProcessNetlinkMessage(struct nl_msg
     }
     }
 
-    // Update map tracking interface information. Handle the case where the
-    // interface is already present. This happens for NL80211_CMD_SET_INTERFACE
-    // when the interface type did not change.
-    auto interfaceInfo = m_interfaceInfo.find(interfaceIndex);
-    if (interfaceInfo == std::cend(m_interfaceInfo)) {
+    // Update interface seen cache, handling the case where the interface has already been seen. This happens for
+    // NL80211_CMD_SET_INTERFACE whern the interface type did not change.
+    auto interfaceSeenIterator = m_interfacesSeen.find(nl80211Interface);
+    if (interfaceSeenIterator == std::cend(m_interfacesSeen)) {
         if (accessPointPresenceEvent == AccessPointPresenceEvent::Arrived) {
-            m_interfaceInfo[interfaceIndex] = { interfaceName, interfaceType };
+            m_interfacesSeen.insert(nl80211Interface);
         }
     } else {
         if (accessPointPresenceEvent == AccessPointPresenceEvent::Departed) {
-            m_interfaceInfo.erase(interfaceInfo);
+            m_interfacesSeen.erase(interfaceSeenIterator);
         }
     }
 
     // Invoke presence event callback if present.
     if (accessPointPresenceEventCallback != nullptr) {
+        const auto interfaceName{ nl80211Interface.Name };
         LOGV << std::format("Invoking access point presence event callback with event args 'interface={}, presence={}'", interfaceName, magic_enum::enum_name(accessPointPresenceEvent));
-        auto accessPoint = m_accessPointFactory->Create(interfaceName);
+        auto accessPointCreateArgs = std::make_unique<AccessPointCreateArgsLinux>(std::move(nl80211Interface));
+        auto accessPoint = m_accessPointFactory->Create(interfaceName, std::move(accessPointCreateArgs));
+
         accessPointPresenceEventCallback(accessPointPresenceEvent, accessPoint);
     }
 

src/linux/wifi/apmanager/include/microsoft/net/wifi/AccessPointDiscoveryAgentOperationsNetlink.hxx

@@ -7,7 +7,7 @@
 #include <stop_token>
 #include <string>
 #include <thread>
-#include <unordered_map>
+#include <unordered_set>
 
 #include <linux/nl80211.h>
 #include <microsoft/net/netlink/NetlinkMessage.hxx>
@@ -120,13 +120,7 @@ private:
     int m_eventLoopStopFd{ -1 };
     std::jthread m_netlinkMessageProcessingThread;
 
-    struct WifiInterfaceInfo
-    {
-        std::string Name;
-        nl80211_iftype Type;
-    };
-
-    std::unordered_map<int, WifiInterfaceInfo> m_interfaceInfo;
+    std::unordered_set<Microsoft::Net::Netlink::Nl80211::Nl80211Interface> m_interfacesSeen;
     Microsoft::Net::Netlink::Nl80211::Nl80211ProtocolState &m_netlink80211ProtocolState;
 };
 } // namespace Microsoft::Net::Wifi

tests/unit/linux/wpa-controller/TestHostapd.cxx

@@ -8,7 +8,7 @@
 
 #include "detail/WpaDaemonManager.hxx"
 
-TEST_CASE("Create a Hostapd instance", "[wpa][hostapd][client][remote]")
+TEST_CASE("Create a Hostapd instance (root)", "[wpa][hostapd][client][remote]")
 {
     using namespace Wpa;
 
@@ -40,7 +40,7 @@ TEST_CASE("Create a Hostapd instance", "[wpa][hostapd][client][remote]")
     }
 }
 
-TEST_CASE("Send Ping() command", "[wpa][hostapd][client][remote]")
+TEST_CASE("Send Ping() command (root)", "[wpa][hostapd][client][remote]")
 {
     using namespace Wpa;
 
@@ -58,7 +58,7 @@ TEST_CASE("Send Ping() command", "[wpa][hostapd][client][remote]")
     }
 }
 
-TEST_CASE("Send command: GetStatus()", "[wpa][hostapd][client][remote]")
+TEST_CASE("Send command: GetStatus() (root)", "[wpa][hostapd][client][remote]")
 {
     using namespace Wpa;
 
@@ -96,7 +96,7 @@ TEST_CASE("Send command: GetStatus()", "[wpa][hostapd][client][remote]")
     }
 }
 
-TEST_CASE("Send GetProperty() command", "[wpa][hostapd][client][remote]")
+TEST_CASE("Send GetProperty() command (root)", "[wpa][hostapd][client][remote]")
 {
     using namespace Wpa;
 
@@ -135,7 +135,7 @@ TEST_CASE("Send GetProperty() command", "[wpa][hostapd][client][remote]")
     }
 }
 
-TEST_CASE("Send SetProperty() command", "[wpa][hostapd][client][remote]")
+TEST_CASE("Send SetProperty() command (root)", "[wpa][hostapd][client][remote]")
 {
     using namespace Wpa;
 
@@ -159,7 +159,7 @@ TEST_CASE("Send SetProperty() command", "[wpa][hostapd][client][remote]")
     // TODO: validate that the property was actually set. Need to find a property whose value is retrievable.
 }
 
-TEST_CASE("Send control commands: Enable(), Disable()", "[wpa][hostapd][client][remote]")
+TEST_CASE("Send control commands: Enable(), Disable() (root)", "[wpa][hostapd][client][remote]")
 {
     using namespace Wpa;
 
@@ -211,15 +211,15 @@ TEST_CASE("Send control commands: Enable(), Disable()", "[wpa][hostapd][client][
 // Also, keep Terminate() test cases at end-of-file since Catch2 will run
 // tests in declaration order by default, which minimizes the possibility
 // of a test case being run after the daemon has been terminated.
-TEST_CASE("Send command: Terminate() doesn't throw", "[wpa][hostapd][client][remote]")
+TEST_CASE("Send command: Terminate() doesn't throw (root)", "[wpa][hostapd][client][remote]")
 {
     using namespace Wpa;
 
     Hostapd hostapd(WpaDaemonManager::InterfaceNameDefault);
     REQUIRE_NOTHROW(hostapd.Terminate());
 }
 
-TEST_CASE("Send command: Terminate() ping failure", "[wpa][hostapd][client][remote]")
+TEST_CASE("Send command: Terminate() ping failure (root)", "[wpa][hostapd][client][remote]")
 {
     using namespace Wpa;
     using namespace std::chrono_literals;

tests/unit/linux/wpa-controller/TestWpaController.cxx

@@ -15,7 +15,7 @@ constexpr auto WpaTypesSupported = {
 };
 } // namespace TestDetail
 
-TEST_CASE("Send/receive WpaController request/response", "[wpa][hostapd][client][remote]")
+TEST_CASE("Send/receive WpaController request/response (root)", "[wpa][hostapd][client][remote]")
 {
     using namespace TestDetail;
     using namespace Wpa;