Skip to content

Adds DHCP Range Setup to APMode #6731

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 16, 2022
Merged

Adds DHCP Range Setup to APMode #6731

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3bd9d24
Adds DHCP Range Setup to APMode
SuGlider May 12, 2022
7d62667
Merge branch 'master' into dhcpRange
SuGlider May 16, 2022
6910c81
ready for supporting any netmask in DHCP server
SuGlider May 16, 2022
65f4166
Merge branch 'dhcpRange' of https://github.com/SuGlider/arduino-esp32…
SuGlider May 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions libraries/Ethernet/src/ETH.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ bool ETHClass::config(IPAddress local_ip, IPAddress gateway, IPAddress subnet, I
esp_err_t err = ESP_OK;
tcpip_adapter_ip_info_t info;

if(local_ip != (uint32_t)0x00000000 && local_ip != INADDR_NONE){
if(static_cast<uint32_t>(local_ip) != 0){
info.ip.addr = static_cast<uint32_t>(local_ip);
info.gw.addr = static_cast<uint32_t>(gateway);
info.netmask.addr = static_cast<uint32_t>(subnet);
Expand Down Expand Up @@ -443,13 +443,13 @@ bool ETHClass::config(IPAddress local_ip, IPAddress gateway, IPAddress subnet, I
ip_addr_t d;
d.type = IPADDR_TYPE_V4;

if(dns1 != (uint32_t)0x00000000 && dns1 != INADDR_NONE) {
if(static_cast<uint32_t>(dns1) != 0) {
// Set DNS1-Server
d.u_addr.ip4.addr = static_cast<uint32_t>(dns1);
dns_setserver(0, &d);
}

if(dns2 != (uint32_t)0x00000000 && dns2 != INADDR_NONE) {
if(static_cast<uint32_t>(dns2) != 0) {
// Set DNS2-Server
d.u_addr.ip4.addr = static_cast<uint32_t>(dns2);
dns_setserver(1, &d);
Expand Down
6 changes: 3 additions & 3 deletions libraries/WiFi/src/WiFiAP.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ extern "C" {
// -----------------------------------------------------------------------------------------------------------------------

esp_netif_t* get_esp_interface_netif(esp_interface_t interface);
esp_err_t set_esp_interface_ip(esp_interface_t interface, IPAddress local_ip=IPAddress(), IPAddress gateway=IPAddress(), IPAddress subnet=IPAddress());
esp_err_t set_esp_interface_ip(esp_interface_t interface, IPAddress local_ip=INADDR_NONE, IPAddress gateway=INADDR_NONE, IPAddress subnet=INADDR_NONE, IPAddress dhcp_lease_start=INADDR_NONE);
static bool softap_config_equal(const wifi_config_t& lhs, const wifi_config_t& rhs);

static size_t _wifi_strncpy(char * dst, const char * src, size_t dst_len){
Expand Down Expand Up @@ -195,7 +195,7 @@ String WiFiAPClass::softAPSSID() const
* @param gateway gateway IP
* @param subnet subnet mask
*/
bool WiFiAPClass::softAPConfig(IPAddress local_ip, IPAddress gateway, IPAddress subnet)
bool WiFiAPClass::softAPConfig(IPAddress local_ip, IPAddress gateway, IPAddress subnet, IPAddress dhcp_lease_start)
{
esp_err_t err = ESP_OK;

Expand All @@ -204,7 +204,7 @@ bool WiFiAPClass::softAPConfig(IPAddress local_ip, IPAddress gateway, IPAddress
return false;
}

err = set_esp_interface_ip(ESP_IF_WIFI_AP, local_ip, gateway, subnet);
err = set_esp_interface_ip(ESP_IF_WIFI_AP, local_ip, gateway, subnet, dhcp_lease_start);
return err == ESP_OK;
}

Expand Down
2 changes: 1 addition & 1 deletion libraries/WiFi/src/WiFiAP.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class WiFiAPClass
public:

bool softAP(const char* ssid, const char* passphrase = NULL, int channel = 1, int ssid_hidden = 0, int max_connection = 4, bool ftm_responder = false);
bool softAPConfig(IPAddress local_ip, IPAddress gateway, IPAddress subnet);
bool softAPConfig(IPAddress local_ip, IPAddress gateway, IPAddress subnet, IPAddress dhcp_lease_start = INADDR_NONE);
bool softAPdisconnect(bool wifioff = false);

uint8_t softAPgetStationNum();
Expand Down
72 changes: 58 additions & 14 deletions libraries/WiFi/src/WiFiGeneric.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ extern "C" {
#include <vector>
#include "sdkconfig.h"

#define _byte_swap32(num) (((num>>24)&0xff) | ((num<<8)&0xff0000) | ((num>>8)&0xff00) | ((num<<24)&0xff000000))
ESP_EVENT_DEFINE_BASE(ARDUINO_EVENTS);
/*
* Private (exposable) methods
Expand Down Expand Up @@ -82,7 +83,7 @@ esp_err_t set_esp_interface_hostname(esp_interface_t interface, const char * hos
return ESP_FAIL;
}

esp_err_t set_esp_interface_ip(esp_interface_t interface, IPAddress local_ip=IPAddress(), IPAddress gateway=IPAddress(), IPAddress subnet=IPAddress()){
esp_err_t set_esp_interface_ip(esp_interface_t interface, IPAddress local_ip=IPAddress(), IPAddress gateway=IPAddress(), IPAddress subnet=IPAddress(), IPAddress dhcp_lease_start=INADDR_NONE){
esp_netif_t *esp_netif = esp_netifs[interface];
esp_netif_dhcp_status_t status = ESP_NETIF_DHCP_INIT;
esp_netif_ip_info_t info;
Expand Down Expand Up @@ -138,20 +139,64 @@ esp_err_t set_esp_interface_ip(esp_interface_t interface, IPAddress local_ip=IPA

dhcps_lease_t lease;
lease.enable = true;
uint32_t dhcp_ipaddr = static_cast<uint32_t>(local_ip);
// prevents DHCP lease range to overflow subnet/24 range
// there will be 11 addresses for DHCP to lease
uint8_t leaseStart = (uint8_t)(~subnet[3] - 12);
if ((local_ip[3]) < leaseStart) {
lease.start_ip.addr = dhcp_ipaddr + (1 << 24);
lease.end_ip.addr = dhcp_ipaddr + (11 << 24);
} else {
// make range stay in the begining of the netmask range
dhcp_ipaddr = (dhcp_ipaddr & 0x00FFFFFF);
lease.start_ip.addr = dhcp_ipaddr + (1 << 24);
lease.end_ip.addr = dhcp_ipaddr + (11 << 24);
uint8_t CIDR = WiFiGenericClass::calculateSubnetCIDR(subnet);
log_v("SoftAP: %s | Gateway: %s | DHCP Start: %s | Netmask: %s", local_ip.toString().c_str(), gateway.toString().c_str(), dhcp_lease_start.toString().c_str(), subnet.toString().c_str());
// netmask must have room for at least 12 IP addresses (AP + GW + 10 DHCP Leasing addresses)
// netmask also must be limited to the last 8 bits of IPv4, otherwise this function won't work
// IDF NETIF checks netmask for the 3rd byte: https://github.com/espressif/esp-idf/blob/master/components/esp_netif/lwip/esp_netif_lwip.c#L1857-L1862
if (CIDR > 28 || CIDR < 24) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does the netmask have to be the last 8 bits? What is wrong with being 16 bits instead?

Copy link
Collaborator Author

@SuGlider SuGlider May 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to make it work correctly for netmask with more than 8 bits, it would be better to invert the byte order when IPAddress is cast to uint32_t.
I'm not sure that changing it would not break the code in other places or for other users.
Given that this issue has been already there since the very beginning, I just added code to prevent it from happening, limiting the netmask to 8 bits.

Example:
1.2.3.255 goes to 0xFF030201 in the current IPAddress Class casting method.
Adding 1, means to add (1 << 24) and it goes to 0x00030201 => IP 1.2.3.0 (it is worst when adding 11 << 24 to create a DHCP range).
If it were inverted, it would go to 0x010203FF, instead.
So adding 1 would result in 0x01020400 => IP 1.2.4.0, respectively, adding 11 would result in 0x01020410 => IP 1.2.4.10 (a lot better way to make the increment!)

It can be achieved by forcing this byte order inversion/reversion in the WiFiGenericClass code... is it worth it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inverting the bytes also makes it way easier to compare (uint32_t) IPAddress (greater, less than) in order to check if it belongs to an IP range or if it could break netmask limits and so on.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But inverting it will make it otherwise incompatible with everything else. We need to have the byte order that computers and IDF use. I do not see a reason why not add a method/methods to internally flip the order and do the operations agains another IP :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I'll add the code for it.

Copy link
Collaborator Author

@SuGlider SuGlider May 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@me-no-dev - I tested it with netmask 255.255.0.0 for these cases:
1- AP = 192.168.1.250 and DHCP Range = 192.168.1.251 to 192.168.2.4
2- AP = 192.168.1.1 and DHCP Range = 192.168.2.1 to 192.168.2.10

In both cases the IDF call tcpip_adapter_dhcps_option() fails and returns error 0x5001.
It only works if AP IP address and DHCP range are in the same network with netmask 255.255.255.0 to 255.255.255.240
So, it seems that IDF only works with 8 bits subnet.

Therefore, this PR is ready.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed by looking into the esp_netif_lwip.c code:
https://github.com/espressif/esp-idf/blob/master/components/esp_netif/lwip/esp_netif_lwip.c#L1857-L1862

it only uses the last 8 bits of the AP address, limiting netmask effect to also 8 bits.

                    /*config ip information must be in the same segment as the local ip*/
                    softap_ip >>= 8;
                    if ((start_ip >> 8 != softap_ip)
                        || (end_ip >> 8 != softap_ip)) {
                        return ESP_ERR_ESP_NETIF_INVALID_PARAMS;
                    }

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is ready for merging.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also found out an IDF limit for the size of DHCP range: DHCPS_MAX_LEASE, default is 0x64 = 100 addresses (lwip/include/apps/dhcpserver/dhcpserver.h)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commited changes that prepare the code for any netmask (for the future, when IDF supports it as well).

log_e("Bad netmask. It must be from /24 to /28 (255.255.255. 0<->240)");
return ESP_FAIL; // ESP_FAIL if initializing failed
}
// The code below is ready for any netmask, not limited to 255.255.255.0
uint32_t netmask = _byte_swap32(info.netmask.addr);
uint32_t ap_ipaddr = _byte_swap32(info.ip.addr);
uint32_t dhcp_ipaddr = _byte_swap32(static_cast<uint32_t>(dhcp_lease_start));
dhcp_ipaddr = dhcp_ipaddr == 0 ? ap_ipaddr + 1 : dhcp_ipaddr;
uint32_t leaseStartMax = ~netmask - 10;
// there will be 10 addresses for DHCP to lease
lease.start_ip.addr = dhcp_ipaddr;
lease.end_ip.addr = lease.start_ip.addr + 10;
// Check if local_ip is in the same subnet as the dhcp leasing range initial address
if ((ap_ipaddr & netmask) != (dhcp_ipaddr & netmask)) {
log_e("The AP IP address (%s) and the DHCP start address (%s) must be in the same subnet",
local_ip.toString().c_str(), IPAddress(_byte_swap32(dhcp_ipaddr)).toString().c_str());
return ESP_FAIL; // ESP_FAIL if initializing failed
}
// prevents DHCP lease range to overflow subnet range
if ((dhcp_ipaddr & ~netmask) >= leaseStartMax) {
// make first DHCP lease addr stay in the begining of the netmask range
lease.start_ip.addr = (dhcp_ipaddr & netmask) + 1;
lease.end_ip.addr = lease.start_ip.addr + 10;
log_w("DHCP Lease out of range - Changing DHCP leasing start to %s", IPAddress(_byte_swap32(lease.start_ip.addr)).toString().c_str());
}
// Check if local_ip is within DHCP range
if (ap_ipaddr >= lease.start_ip.addr && ap_ipaddr <= lease.end_ip.addr) {
log_e("The AP IP address (%s) can't be within the DHCP range (%s -- %s)",
local_ip.toString().c_str(), IPAddress(_byte_swap32(lease.start_ip.addr)).toString().c_str(), IPAddress(_byte_swap32(lease.end_ip.addr)).toString().c_str());
return ESP_FAIL; // ESP_FAIL if initializing failed
}
// Check if gateway is within DHCP range
uint32_t gw_ipaddr = _byte_swap32(info.gw.addr);
bool gw_in_same_subnet = (gw_ipaddr & netmask) == (ap_ipaddr & netmask);
if (gw_in_same_subnet && gw_ipaddr >= lease.start_ip.addr && gw_ipaddr <= lease.end_ip.addr) {
log_e("The GatewayP address (%s) can't be within the DHCP range (%s -- %s)",
gateway.toString().c_str(), IPAddress(_byte_swap32(lease.start_ip.addr)).toString().c_str(), IPAddress(_byte_swap32(lease.end_ip.addr)).toString().c_str());
return ESP_FAIL; // ESP_FAIL if initializing failed
}
// all done, just revert back byte order of DHCP lease range
lease.start_ip.addr = _byte_swap32(lease.start_ip.addr);
lease.end_ip.addr = _byte_swap32(lease.end_ip.addr);
log_v("DHCP Server Range: %s to %s", IPAddress(lease.start_ip.addr).toString().c_str(), IPAddress(lease.end_ip.addr).toString().c_str());
err = tcpip_adapter_dhcps_option(
(tcpip_adapter_dhcp_option_mode_t)TCPIP_ADAPTER_OP_SET,
(tcpip_adapter_dhcp_option_id_t)ESP_NETIF_SUBNET_MASK,
(void*)&info.netmask.addr, sizeof(info.netmask.addr)
);
if(err){
log_e("DHCPS Set Netmask Failed! 0x%04x", err);
return err;
}
err = tcpip_adapter_dhcps_option(
(tcpip_adapter_dhcp_option_mode_t)TCPIP_ADAPTER_OP_SET,
(tcpip_adapter_dhcp_option_id_t)REQUESTED_IP_ADDRESS,
Expand All @@ -161,7 +206,6 @@ esp_err_t set_esp_interface_ip(esp_interface_t interface, IPAddress local_ip=IPA
log_e("DHCPS Set Lease Failed! 0x%04x", err);
return err;
}

err = esp_netif_dhcps_start(esp_netif);
if(err){
log_e("DHCPS Start Failed! 0x%04x", err);
Expand Down
2 changes: 1 addition & 1 deletion libraries/WiFi/src/WiFiSTA.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ extern "C" {

esp_netif_t* get_esp_interface_netif(esp_interface_t interface);
esp_err_t set_esp_interface_dns(esp_interface_t interface, IPAddress main_dns=IPAddress(), IPAddress backup_dns=IPAddress(), IPAddress fallback_dns=IPAddress());
esp_err_t set_esp_interface_ip(esp_interface_t interface, IPAddress local_ip=IPAddress(), IPAddress gateway=IPAddress(), IPAddress subnet=IPAddress());
esp_err_t set_esp_interface_ip(esp_interface_t interface, IPAddress local_ip=INADDR_NONE, IPAddress gateway=INADDR_NONE, IPAddress subnet=INADDR_NONE, IPAddress dhcp_lease_start=INADDR_NONE);
static bool sta_config_equal(const wifi_config_t& lhs, const wifi_config_t& rhs);

static size_t _wifi_strncpy(char * dst, const char * src, size_t dst_len){
Expand Down