From cffd775450f9cdafb31b9fe741bdd04182fb53c1 Mon Sep 17 00:00:00 2001 From: Keith Burzinski Date: Tue, 11 Aug 2026 23:21:37 -0500 Subject: [PATCH] [ethernet][network][wifi] Arbitrate the default route from the network priority list (#17797) --- .../components/ethernet/ethernet_component.h | 6 ++ .../ethernet/ethernet_component_esp32.cpp | 23 +++-- esphome/components/network/__init__.py | 41 ++++++++- .../components/network/network_component.cpp | 90 +++++++++++++++++++ .../components/network/network_component.h | 18 ++++ esphome/components/network/util.cpp | 27 ++++-- esphome/components/network/util.h | 3 +- esphome/components/wifi/wifi_component.h | 12 +++ .../wifi/wifi_component_esp_idf.cpp | 2 + esphome/core/defines.h | 1 + .../network/config/priority_arduino.yaml | 26 ++++++ .../network/config/priority_rp2040.yaml | 24 +++++ .../network/config/priority_single.yaml | 15 ++++ .../component_tests/network/test_priority.py | 71 ++++++++++++++- .../network/test-priority.esp32-ard.yaml | 23 +++++ 15 files changed, 366 insertions(+), 16 deletions(-) create mode 100644 tests/component_tests/network/config/priority_arduino.yaml create mode 100644 tests/component_tests/network/config/priority_rp2040.yaml create mode 100644 tests/component_tests/network/config/priority_single.yaml create mode 100644 tests/components/network/test-priority.esp32-ard.yaml diff --git a/esphome/components/ethernet/ethernet_component.h b/esphome/components/ethernet/ethernet_component.h index ad329f9b81..646e0af8e6 100644 --- a/esphome/components/ethernet/ethernet_component.h +++ b/esphome/components/ethernet/ethernet_component.h @@ -140,6 +140,12 @@ class EthernetComponent final : public Component { bool is_disabled() { return this->disabled_; } bool is_enabled() { return !this->disabled_; } +#ifdef USE_ESP32 + /// esp_netif handle, used by network for default-route arbitration. + /// nullptr until the driver/netif installation has run. + esp_netif_t *get_esp_netif() { return this->eth_netif_; } +#endif + void set_type(EthernetType type); #ifdef USE_ETHERNET_MANUAL_IP void set_manual_ip(const ManualIP &manual_ip); diff --git a/esphome/components/ethernet/ethernet_component_esp32.cpp b/esphome/components/ethernet/ethernet_component_esp32.cpp index dc623b6e5b..0220d6a19b 100644 --- a/esphome/components/ethernet/ethernet_component_esp32.cpp +++ b/esphome/components/ethernet/ethernet_component_esp32.cpp @@ -789,16 +789,25 @@ void EthernetComponent::start_connect_() { #ifdef USE_ETHERNET_MANUAL_IP if (this->manual_ip_.has_value()) { - LwIPLock lock; + // Set DNS through esp_netif so the servers are stored in the netif's own + // dns[] array; raw dns_setserver() would be lost when the default-route + // arbitration re-applies the default netif's DNS. + // Log-only on failure: the link still has a working IP/gateway, so degraded + // name resolution does not justify marking the whole component failed. + esp_netif_dns_info_t dns{}; if (this->manual_ip_->dns1.is_set()) { - ip_addr_t d; - d = this->manual_ip_->dns1; - dns_setserver(0, &d); + dns.ip = this->manual_ip_->dns1; + err = esp_netif_set_dns_info(this->eth_netif_, ESP_NETIF_DNS_MAIN, &dns); + if (err != ESP_OK) { + ESP_LOGE(TAG, "Set main DNS failed: %s", esp_err_to_name(err)); + } } if (this->manual_ip_->dns2.is_set()) { - ip_addr_t d; - d = this->manual_ip_->dns2; - dns_setserver(1, &d); + dns.ip = this->manual_ip_->dns2; + err = esp_netif_set_dns_info(this->eth_netif_, ESP_NETIF_DNS_BACKUP, &dns); + if (err != ESP_OK) { + ESP_LOGE(TAG, "Set backup DNS failed: %s", esp_err_to_name(err)); + } } } else #endif diff --git a/esphome/components/network/__init__.py b/esphome/components/network/__init__.py index 24e9aa45e1..3544fb2647 100644 --- a/esphome/components/network/__init__.py +++ b/esphome/components/network/__init__.py @@ -39,6 +39,12 @@ KEY_NETWORK_PRIORITY = "network_priority" # NETWORK_PLAN.md for the full multi-interface roadmap. VALID_NETWORK_TYPES = ["ethernet", "wifi"] +# Interfaces NetworkComponent::loop() knows how to arbitrate the default route +# for. Deliberately NOT derived from VALID_NETWORK_TYPES: extending that list +# without extending the C++ arbitration (and then this set) is caught in +# _final_validate() as a config error instead of a silently mis-routed interface. +ARBITRATED_NETWORK_TYPES = frozenset({"ethernet", "wifi"}) + # Setup priority base values — first in list gets the highest priority. # # The base equals the historical setup_priority::WIFI / ::ETHERNET default @@ -310,7 +316,8 @@ CONFIG_SCHEMA = cv.All( def _final_validate(config: ConfigType) -> None: """Check that every interface named in 'priority' has a corresponding component block.""" full = fv.full_config.get() - for entry in config.get(CONF_PRIORITY, []): + priority_list = config.get(CONF_PRIORITY, []) + for entry in priority_list: iface = entry["interface"] if iface not in full: raise cv.Invalid( @@ -319,6 +326,24 @@ def _final_validate(config: ConfigType) -> None: [CONF_PRIORITY], ) + # Tripwire for future interface types (openthread, modem): the C++ default-route + # arbitration pivots on USE_NETWORK_PRIMARY_INTERFACE_WIFI and only knows + # ethernet and wifi. Extend NetworkComponent::loop() before allowing another + # type here. Unreachable until VALID_NETWORK_TYPES grows. + if ( + len(priority_list) > 1 + and ( + unsupported := {e["interface"] for e in priority_list} + - ARBITRATED_NETWORK_TYPES + ) + and CORE.is_esp32 + ): + raise cv.Invalid( + "Default-route arbitration does not support: " + f"{', '.join(sorted(unsupported))}", + [CONF_PRIORITY], + ) + FINAL_VALIDATE_SCHEMA = _final_validate @@ -337,10 +362,22 @@ async def to_code(config): # network/util.cpp resolves the reported address (get_use_address_to, # get_ip_addresses) in a fixed ethernet-first order; a wifi-first priority # list is the only case that deviates from it, so it is the only case that - # needs a define. Runtime (active-interface) selection is a planned follow-up. + # needs a define. if priority_list[0]["interface"] == "wifi": cg.add_define("USE_NETWORK_PRIMARY_INTERFACE_WIFI") + # With more than one interface, NetworkComponent::loop() arbitrates the + # default route (ESP-IDF's fixed route_prio values would always favor + # WiFi). ESP32 only: the arbitration needs esp_netif, which both + # frameworks build from source. + # The ethernet/wifi-only assumption behind the arbitration is enforced in + # _final_validate() so a future unsupported type fails as a config error. + if len(priority_list) > 1 and CORE.is_esp32: + cg.add_define("USE_NETWORK_DEFAULT_ROUTE") + # Have lwIP switch to the DNS servers of the netif that owns the + # default route whenever the arbitration changes it. + add_idf_sdkconfig_option("CONFIG_ESP_NETIF_SET_DNS_PER_DEFAULT_NETIF", True) + _LOGGER.info( "Network interface priority: %s", " > ".join(entry["interface"] for entry in priority_list), diff --git a/esphome/components/network/network_component.cpp b/esphome/components/network/network_component.cpp index 40cf64906c..cf457bb661 100644 --- a/esphome/components/network/network_component.cpp +++ b/esphome/components/network/network_component.cpp @@ -6,6 +6,20 @@ #include "esp_err.h" #include "esp_netif.h" #include "esp_event.h" + +#ifdef USE_NETWORK_DEFAULT_ROUTE +#include "esphome/core/application.h" +#include "esphome/core/helpers.h" +#include "esp_netif_net_stack.h" +#include "lwip/netif.h" +#ifdef USE_ETHERNET +#include "esphome/components/ethernet/ethernet_component.h" +#endif +#ifdef USE_WIFI +#include "esphome/components/wifi/wifi_component.h" +#endif +#endif + namespace esphome::network { static const char *const TAG = "network"; @@ -29,5 +43,81 @@ void NetworkComponent::setup() { } } +#ifdef USE_NETWORK_DEFAULT_ROUTE +static esp_netif_t *connected_wifi_netif() { +#ifdef USE_WIFI + auto *wifi = wifi::global_wifi_component; + if (wifi != nullptr && wifi->is_connected()) + return wifi->get_esp_netif_sta(); +#endif + return nullptr; +} + +static esp_netif_t *connected_ethernet_netif() { +#ifdef USE_ETHERNET + auto *eth = ethernet::global_eth_component; + if (eth != nullptr && eth->is_connected()) + return eth->get_esp_netif(); +#endif + return nullptr; +} + +void NetworkComponent::loop() { + // Pin the default route to the first connected interface in the user's priority + // order; ESP-IDF's own route_prio selection would always favor WiFi. + // USE_NETWORK_PRIMARY_INTERFACE_WIFI is emitted for a wifi-first priority list; + // it selects the reported address in util.cpp and doubles as the route-order + // pivot here — the two uses must stay in sync. + esp_netif_t *best; +#ifdef USE_NETWORK_PRIMARY_INTERFACE_WIFI + best = connected_wifi_netif(); + if (best == nullptr) + best = connected_ethernet_netif(); +#else + best = connected_ethernet_netif(); + if (best == nullptr) + best = connected_wifi_netif(); +#endif + if (best == nullptr) { + // Forget the last winner: stopping its netif cleared lwIP's default route and + // IDF's manual override suppresses re-election, so reconnect must re-assert it. + this->default_netif_ = nullptr; + return; + } + if (best == this->default_netif_) { + // Same winner as the last assert. Still re-assert if lwIP's default route is + // not the winner's netif: a winner whose netif bounced down and up between two + // polls would otherwise stay routeless (stopping a netif nulls lwIP's + // netif_default). Checking lwIP directly keeps this independent of IDF's + // re-election bookkeeping (esp_netif_get_default_netif() cannot detect it). + // Throttled: LwIPLock is the global lwIP core mutex, and this branch runs on + // every pass once the route has settled. + const uint32_t now = App.get_loop_component_start_time(); + if (now - this->last_route_check_ < ROUTE_CHECK_INTERVAL_MS) + return; + this->last_route_check_ = now; + bool route_is_ours; + { + LwIPLock lock; + route_is_ours = static_cast(netif_default) == esp_netif_get_netif_impl(best); + } + if (route_is_ours) + return; + } + esp_err_t err = esp_netif_set_default_netif(best); + if (err != ESP_OK) { + ESP_LOGW(TAG, "Failed to set default interface: (%d) %s", err, esp_err_to_name(err)); + // Cache the intent anyway: subsequent passes take the same-winner branch + // above, so retries are throttled to ROUTE_CHECK_INTERVAL_MS and the lwIP + // verification keeps re-attempting until the route is actually ours. + this->default_netif_ = best; + this->last_route_check_ = App.get_loop_component_start_time(); + return; + } + this->default_netif_ = best; + ESP_LOGI(TAG, "Default interface: %s", esp_netif_get_desc(best)); +} +#endif // USE_NETWORK_DEFAULT_ROUTE + } // namespace esphome::network #endif diff --git a/esphome/components/network/network_component.h b/esphome/components/network/network_component.h index 2e76a95673..8d4866d4f0 100644 --- a/esphome/components/network/network_component.h +++ b/esphome/components/network/network_component.h @@ -3,12 +3,30 @@ #if defined(USE_NETWORK) && defined(USE_ESP32) #include "esphome/core/component.h" +#ifdef USE_NETWORK_DEFAULT_ROUTE +// Forward declaration matching esp_netif's own typedef; avoids pulling esp_netif.h +// into this header. +using esp_netif_t = struct esp_netif_obj; +#endif + namespace esphome::network { class NetworkComponent final : public Component { public: void setup() override; // AFTER_BLUETOOTH: BLE controller must initialize before esp_netif_init per IDF guidance. float get_setup_priority() const override { return setup_priority::AFTER_BLUETOOTH; } + +#ifdef USE_NETWORK_DEFAULT_ROUTE + void loop() override; + + protected: + // Verify-lwIP-route interval for the settled state; keeps the global lwIP core + // mutex off the hot loop path. + static constexpr uint32_t ROUTE_CHECK_INTERVAL_MS = 1000; + // Last netif this component made the default; avoids redundant esp_netif calls. + esp_netif_t *default_netif_{nullptr}; + uint32_t last_route_check_{0}; +#endif }; } // namespace esphome::network #endif diff --git a/esphome/components/network/util.cpp b/esphome/components/network/util.cpp index d90c28801e..11485fdcf0 100644 --- a/esphome/components/network/util.cpp +++ b/esphome/components/network/util.cpp @@ -10,16 +10,33 @@ namespace esphome::network { // an AP that uses a previous interface for NAT). bool is_disabled() { + // The network is disabled only when every configured interface with a + // disable() lifecycle is disabled; one enabled interface means traffic can flow. + bool disabled = false; #ifdef USE_MODEM - if (modem::global_modem_component != nullptr) - return modem::global_modem_component->is_disabled(); + if (modem::global_modem_component != nullptr) { + if (!modem::global_modem_component->is_disabled()) + return false; + disabled = true; + } #endif #ifdef USE_WIFI - if (wifi::global_wifi_component != nullptr) - return wifi::global_wifi_component->is_disabled(); + if (wifi::global_wifi_component != nullptr) { + if (!wifi::global_wifi_component->is_disabled()) + return false; + disabled = true; + } #endif - return false; + +#ifdef USE_ETHERNET + if (ethernet::global_eth_component != nullptr) { + if (!ethernet::global_eth_component->is_disabled()) + return false; + disabled = true; + } +#endif + return disabled; } const char *get_use_address_to(std::span buf) { diff --git a/esphome/components/network/util.h b/esphome/components/network/util.h index df7e164bda..65a578c22f 100644 --- a/esphome/components/network/util.h +++ b/esphome/components/network/util.h @@ -52,7 +52,8 @@ ESPHOME_ALWAYS_INLINE inline bool is_connected() { return false; } -/// Return whether the network is disabled (only wifi for now) +/// Return whether the network is disabled: every configured interface with a +/// disable() lifecycle (modem, wifi, ethernet) is disabled. bool is_disabled(); /// Buffer size for get_use_address_to(): 63-char DNS label + ".local" + null terminator static constexpr size_t USE_ADDRESS_BUFFER_SIZE = 70; diff --git a/esphome/components/wifi/wifi_component.h b/esphome/components/wifi/wifi_component.h index a851ea4015..ea043fd5c6 100644 --- a/esphome/components/wifi/wifi_component.h +++ b/esphome/components/wifi/wifi_component.h @@ -65,6 +65,12 @@ extern "C" { #include #endif +#ifdef USE_ESP32 +// Forward declaration matching esp_netif's own typedef; avoids pulling esp_netif.h +// into this widely-included header. +using esp_netif_t = struct esp_netif_obj; +#endif + namespace esphome::wifi { /// Sentinel value for RSSI when WiFi is not connected @@ -469,6 +475,12 @@ class WiFiComponent final : public Component { bool is_connected() const { return this->connected_; } +#ifdef USE_ESP32 + /// esp_netif handle of the station interface, used by network for default-route + /// arbitration. nullptr until wifi_lazy_init_() has run. + esp_netif_t *get_esp_netif_sta(); +#endif + void set_power_save_mode(WiFiPowerSaveMode power_save); void set_min_auth_mode(WifiMinAuthMode min_auth_mode) { min_auth_mode_ = min_auth_mode; } void set_output_power(float output_power) { output_power_ = output_power; } diff --git a/esphome/components/wifi/wifi_component_esp_idf.cpp b/esphome/components/wifi/wifi_component_esp_idf.cpp index 0198f899d5..245390b097 100644 --- a/esphome/components/wifi/wifi_component_esp_idf.cpp +++ b/esphome/components/wifi/wifi_component_esp_idf.cpp @@ -620,6 +620,8 @@ bool WiFiComponent::wifi_sta_ip_config_(const optional &manual_ip) { return true; } +esp_netif_t *WiFiComponent::get_esp_netif_sta() { return s_sta_netif; } + network::IPAddresses WiFiComponent::wifi_sta_ip_addresses() { if (!this->has_sta()) return {}; diff --git a/esphome/core/defines.h b/esphome/core/defines.h index 26b9025d96..bb4960aec7 100644 --- a/esphome/core/defines.h +++ b/esphome/core/defines.h @@ -138,6 +138,7 @@ #define USE_MEDIA_PLAYER #define USE_MEDIA_SOURCE #define USE_NETWORK +#define USE_NETWORK_DEFAULT_ROUTE #define USE_NETWORK_PRIMARY_INTERFACE_WIFI #define USE_NEXTION_COMMAND_SPACING #define USE_NEXTION_CONF_START_UP_PAGE diff --git a/tests/component_tests/network/config/priority_arduino.yaml b/tests/component_tests/network/config/priority_arduino.yaml new file mode 100644 index 0000000000..b66f676601 --- /dev/null +++ b/tests/component_tests/network/config/priority_arduino.yaml @@ -0,0 +1,26 @@ +esphome: + name: test + +esp32: + board: esp32dev + framework: + type: arduino + +wifi: + ssid: "test_ssid" + password: "test_password" + +ethernet: + type: W5500 + clk_pin: 19 + mosi_pin: 21 + miso_pin: 23 + cs_pin: 18 + interrupt_pin: 36 + reset_pin: 22 + clock_speed: 10Mhz + +network: + priority: + - ethernet + - wifi diff --git a/tests/component_tests/network/config/priority_rp2040.yaml b/tests/component_tests/network/config/priority_rp2040.yaml new file mode 100644 index 0000000000..984f2dcbb4 --- /dev/null +++ b/tests/component_tests/network/config/priority_rp2040.yaml @@ -0,0 +1,24 @@ +esphome: + name: test + +rp2: + board: rpipicow + +wifi: + ssid: "test_ssid" + password: "test_password" + +ethernet: + type: W5500 + clk_pin: 18 + mosi_pin: 19 + miso_pin: 16 + cs_pin: 17 + interrupt_pin: 21 + reset_pin: 20 + mac_address: "02:AA:BB:CC:DD:01" + +network: + priority: + - ethernet + - wifi diff --git a/tests/component_tests/network/config/priority_single.yaml b/tests/component_tests/network/config/priority_single.yaml new file mode 100644 index 0000000000..bd23697808 --- /dev/null +++ b/tests/component_tests/network/config/priority_single.yaml @@ -0,0 +1,15 @@ +esphome: + name: test + +esp32: + board: esp32dev + framework: + type: esp-idf + +wifi: + ssid: "test_ssid" + password: "test_password" + +network: + priority: + - wifi diff --git a/tests/component_tests/network/test_priority.py b/tests/component_tests/network/test_priority.py index da1c0a061d..017f0711a3 100644 --- a/tests/component_tests/network/test_priority.py +++ b/tests/component_tests/network/test_priority.py @@ -16,9 +16,10 @@ from esphome.components.network import ( _validate_priority_list, get_network_priority, ) -from esphome.const import CONF_PRIORITY +from esphome.const import CONF_PRIORITY, PlatformFramework from esphome.core import CORE import esphome.final_validate as fv +from tests.component_tests.types import SetCoreConfigCallable @pytest.fixture(autouse=True) @@ -138,6 +139,22 @@ def test_final_validate_noop_without_priority_list() -> None: _final_validate({}) # must not raise +def test_final_validate_rejects_unsupported_arbitration_interface( + set_core_config: SetCoreConfigCallable, +) -> None: + """The ethernet/wifi-only arbitration tripwire fails as a clean config error. + + Unreachable through the public schema today (VALID_NETWORK_TYPES gates the + list), so the config is hand-built to simulate a future interface type that + was added to the schema without extending NetworkComponent::loop(). + """ + set_core_config(PlatformFramework.ESP32_IDF) + fv.full_config.set({"openthread": {}, "wifi": {}}) + config = {CONF_PRIORITY: [{"interface": "openthread"}, {"interface": "wifi"}]} + with pytest.raises(Invalid, match="arbitration does not support: openthread"): + _final_validate(config) + + def _cpp_setup_priority(name: str) -> float: """Read a setup_priority constant straight from esphome/core/component.h.""" header = Path(__file__).parents[3] / "esphome" / "core" / "component.h" @@ -199,3 +216,55 @@ def test_no_primary_interface_define_without_priority( assert not any( d.name.startswith("USE_NETWORK_PRIMARY_INTERFACE_") for d in CORE.defines ) + + +def _dns_per_default_netif_option() -> bool | None: + from esphome.components.esp32.const import KEY_ESP32, KEY_SDKCONFIG_OPTIONS + + if KEY_ESP32 not in CORE.data: # non-ESP32 configs have no sdkconfig at all + return None + return CORE.data[KEY_ESP32][KEY_SDKCONFIG_OPTIONS].get( + "CONFIG_ESP_NETIF_SET_DNS_PER_DEFAULT_NETIF" + ) + + +@pytest.mark.parametrize( + "config_file", + [ + "priority_wifi_first.yaml", + "priority_ethernet_first.yaml", + "priority_arduino.yaml", + ], +) +def test_multi_interface_priority_enables_default_route_arbitration( + generate_main: Callable[[str | Path], str], + component_config_path: Callable[[str], Path], + config_file: str, +) -> None: + """More than one interface in 'priority' enables default-route arbitration.""" + generate_main(component_config_path(config_file)) + assert "USE_NETWORK_DEFAULT_ROUTE" in {d.name for d in CORE.defines} + assert _dns_per_default_netif_option() is True + + +@pytest.mark.parametrize( + "config_file", + [ + # Single-entry priority list / no list at all. + "priority_single.yaml", + "wifi_only.yaml", + # Dual-interface on rp2040: validates, but the arbitration is ESP32-only + # (NetworkComponent::loop() is compiled under USE_ESP32) — emitting the + # define here would be a hard build break. + "priority_rp2040.yaml", + ], +) +def test_single_interface_has_no_default_route_arbitration( + generate_main: Callable[[str | Path], str], + component_config_path: Callable[[str], Path], + config_file: str, +) -> None: + """Single-interface and non-ESP32 configs must not compile in the arbitration.""" + generate_main(component_config_path(config_file)) + assert "USE_NETWORK_DEFAULT_ROUTE" not in {d.name for d in CORE.defines} + assert _dns_per_default_netif_option() is None diff --git a/tests/components/network/test-priority.esp32-ard.yaml b/tests/components/network/test-priority.esp32-ard.yaml new file mode 100644 index 0000000000..a04246a128 --- /dev/null +++ b/tests/components/network/test-priority.esp32-ard.yaml @@ -0,0 +1,23 @@ +# Arduino dual-stack test: default-route arbitration must also compile under +# the Arduino framework, which builds the same esp_netif/ESP-IDF from source. +# Ethernet is listed first so this build exercises the ethernet-first side of +# the arbitration pivot in NetworkComponent::loop() (the IDF variant of this +# test covers the wifi-first side). +wifi: + ssid: MySSID + password: password1 + +ethernet: + type: W5500 + clk_pin: GPIO19 + mosi_pin: GPIO21 + miso_pin: GPIO23 + cs_pin: GPIO18 + interrupt_pin: GPIO36 + reset_pin: GPIO22 + clock_speed: 10Mhz + +network: + priority: + - ethernet + - wifi