Fix DHCP server pbuf handling and allow to change netif address (#8602)

* Fix sending NACK, use helper function to fill pbuf As noticed in #8582 (comment) Plus, handle the case when `pbuf->len` is less than struct size * Make sure to call SDK functions to start and stop DHCP server As noticed in #8582 (comment) Can't really use `server.begin()` and `server.end()` directly, only default static IP is applied to the interface since DHCP server is deemed 'running' (see `wifi_softap_dhcps_status()` return value) * s Co-authored-by: david gauchard <[email protected]>
esp8266 · Jun 27, 2022 · 7e5520c · 7e5520c
1 parent 0bff52f
commit 7e5520c
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 34 deletions.
diff --git a/cores/esp8266/LwipDhcpServer.cpp b/cores/esp8266/LwipDhcpServer.cpp
@@ -414,7 +414,7 @@ DhcpServer::OptionsBuffer DhcpServer::create_msg(struct dhcps_msg* m)
 ///////////////////////////////////////////////////////////////////////////////////
 void DhcpServer::send_offer(struct dhcps_msg* m)
 {
-    struct pbuf *p, *q;
+    struct pbuf* p;
 
     auto options = create_msg(m);
     options.add(DHCP_OPTION_MSG_TYPE, DHCPOFFER);
@@ -438,12 +438,7 @@ void DhcpServer::send_offer(struct dhcps_msg* m)
         os_printf("dhcps: send_offer>>p->tot_len = %d\n", p->tot_len);
         os_printf("dhcps: send_offer>>p->len = %d\n", p->len);
 #endif
-        q = p;
-        while (q != nullptr)
-        {
-            std::memcpy((u8_t*)q->payload, reinterpret_cast<u8_t*>(&m), q->len);
-            q = q->next;
-        }
+        pbuf_take(p, m, sizeof(struct dhcps_msg));
     }
     else
     {
@@ -475,7 +470,7 @@ void DhcpServer::send_offer(struct dhcps_msg* m)
 ///////////////////////////////////////////////////////////////////////////////////
 void DhcpServer::send_nak(struct dhcps_msg* m)
 {
-    struct pbuf *p, *q;
+    struct pbuf* p;
 
     auto options = create_msg(m);
     options.add(DHCP_OPTION_MSG_TYPE, DHCPNAK);
@@ -492,12 +487,7 @@ void DhcpServer::send_nak(struct dhcps_msg* m)
         os_printf("dhcps: send_nak>>p->tot_len = %d\n", p->tot_len);
         os_printf("dhcps: send_nak>>p->len = %d\n", p->len);
 #endif
-        q = p;
-        while (q != nullptr)
-        {
-            std::memcpy((u8_t*)q->payload, (u8_t*)m, q->len);
-            q = q->next;
-        }
+        pbuf_take(p, m, sizeof(struct dhcps_msg));
     }
     else
     {
@@ -524,7 +514,7 @@ void DhcpServer::send_nak(struct dhcps_msg* m)
 ///////////////////////////////////////////////////////////////////////////////////
 void DhcpServer::send_ack(struct dhcps_msg* m)
 {
-    struct pbuf *p, *q;
+    struct pbuf* p;
 
     auto options = create_msg(m);
     options.add(DHCP_OPTION_MSG_TYPE, DHCPACK);
@@ -548,12 +538,7 @@ void DhcpServer::send_ack(struct dhcps_msg* m)
         os_printf("dhcps: send_ack>>p->tot_len = %d\n", p->tot_len);
         os_printf("dhcps: send_ack>>p->len = %d\n", p->len);
 #endif
-        q = p;
-        while (q != nullptr)
-        {
-            std::memcpy((u8_t*)q->payload, (u8_t*)m, q->len);
-            q = q->next;
-        }
+        pbuf_take(p, m, sizeof(struct dhcps_msg));
     }
     else
     {

diff --git a/libraries/ESP8266WiFi/src/ESP8266WiFiAP.cpp b/libraries/ESP8266WiFi/src/ESP8266WiFiAP.cpp
@@ -167,8 +167,7 @@ bool ESP8266WiFiAPClass::softAP(const char* ssid, const char* psk, int channel,
         DEBUG_WIFI("[AP] softap config unchanged\n");
     }
 
-    auto& server = softAPDhcpServer();
-    server.end();
+    wifi_softap_dhcps_stop();
 
     // check IP config
     struct ip_info ip;
@@ -179,17 +178,13 @@ bool ESP8266WiFiAPClass::softAP(const char* ssid, const char* psk, int channel,
                 IPAddress(192, 168, 4, 1),
                 IPAddress(192, 168, 4, 1),
                 IPAddress(255, 255, 255, 0));
-            if(!ret) {
-                DEBUG_WIFI("[AP] softAPConfig failed!\n");
-                ret = false;
-            }
         }
     } else {
         DEBUG_WIFI("[AP] wifi_get_ip_info failed!\n");
         ret = false;
     }
 
-    server.begin();
+    wifi_softap_dhcps_start();
 
     return ret;
 }
@@ -227,9 +222,10 @@ bool ESP8266WiFiAPClass::softAPConfig(IPAddress local_ip, IPAddress gateway, IPA
     info.gw.addr = gateway.v4();
     info.netmask.addr = subnet.v4();
 
-    auto& server = softAPDhcpServer();
-    server.end();
-
+    // use SDK function for dhcps, not just server.begin()
+    // setting info with static IPs will fail otherwise
+    // (TODO: dhcps_flag seems to store 'SDK' DHCPs status)
+    wifi_softap_dhcps_stop();
     if(!wifi_set_ip_info(SOFTAP_IF, &info)) {
         DEBUG_WIFI("[APConfig] wifi_set_ip_info failed!\n");
         ret = false;
@@ -246,14 +242,17 @@ bool ESP8266WiFiAPClass::softAPConfig(IPAddress local_ip, IPAddress gateway, IPA
     dhcp_lease.end_ip.addr = ip.v4();
     DEBUG_WIFI("[APConfig] DHCP IP end: %s\n", ip.toString().c_str());
 
+    auto& server = softAPDhcpServer();
     if(!server.set_dhcps_lease(&dhcp_lease))
     {
-        DEBUG_WIFI("[APConfig] wifi_set_ip_info failed!\n");
+        DEBUG_WIFI("[APConfig] server set_dhcps_lease failed!\n");
         ret = false;
     }
 
-    server.setRouter(true); // send ROUTER option with netif's gateway IP
-    server.begin();
+    // send ROUTER option with netif's gateway IP
+    server.setRouter(true);
+
+    wifi_softap_dhcps_start();
 
     // check config
     if(wifi_get_ip_info(SOFTAP_IF, &info)) {