Summary

The ESP32 PPP implementation predates the generic implementation in extmod. The new extmod implementation has a few advantages such as a better deinit procedure (the ESP32 implementation would not clean up properly and cause crashes if recreated) and using the UART IRQ functionality instead of running a task to read data from the UART.

This change restructures the ESP implementation to be much closer to the new extmod version, while also bringing a few tiny improvements from the ESP32 version to the extmod version. The diff between extmod/network_ppp_lwip.c and ports/esp32/network_ppp.c is now a small set of easy to review ESP32 port-specific changes.

Testing

I've been running these changes for months on a few custom boards (original ESP32 and ESP32S3) with three different modem chips to connect to the Internet via PPP. Those firmware versions do contain a bunch of other changes as well (for example #17135, which I highly recommend also gets considered if this PR gets merged since it fixes a broken behaviour in the UART that can now get triggered by this updated PPP version) so it's not a completely clean test. I'm fairly confident in the changes, and the approach makes sense, but I'd be happy to see other people giving it a go as well and letting me know if they spot anything.

Note for reviewers: Instead of reviewing the changes to the ESP32 version directly, it might be easier to review the differences to the extmod version first (they should make sense on their own) and then compare the diff between the extmod and ESP32 versions:
<details><summary><code>git diff --no-index extmod/network_ppp_lwip.c ports/esp32/network_ppp.c</code></summary>

diff --git a/extmod/network_ppp_lwip.c b/ports/esp32/network_ppp.c
index 2c3dac9201..8b700c98ef 100644
--- a/extmod/network_ppp_lwip.c
+++ b/ports/esp32/network_ppp.c
@@ -3,8 +3,11 @@
  *
  * The MIT License (MIT)
  *
+ * Copyright (c) 2018 "Eric Poulsen" <eric@zyxod.com>
  * Copyright (c) 2024 Damien P. George
  *
+ * Based on the ESP IDF example code which is Public Domain / CC0
+ *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
  * in the Software without restriction, including without limitation the rights
@@ -24,22 +27,25 @@
  * THE SOFTWARE.
  */
 
-// This file is intended to closely match ports/esp32/network_ppp.c. Changes can
+// This file is intended to closely match extmod/network_ppp_lwip.c. Changes can
 // and should probably be applied to both files. Compare them directly by using:
 // git diff --no-index extmod/network_ppp_lwip.c ports/esp32/network_ppp.c
 
 #include "py/runtime.h"
 #include "py/mphal.h"
 #include "py/stream.h"
-#include "extmod/modnetwork.h"
 
-#if MICROPY_PY_NETWORK_PPP_LWIP
+#if defined(CONFIG_ESP_NETIF_TCPIP_LWIP) && defined(CONFIG_LWIP_PPP_SUPPORT)
 
 #include "lwip/dns.h"
 #include "netif/ppp/ppp.h"
 #include "netif/ppp/pppapi.h"
 #include "netif/ppp/pppos.h"
 
+// Includes for port-specific changes compared to network_ppp_lwip.c
+#include "shared/netutils/netutils.h"
+#include "ppp_set_auth.h"
+
 // Enable this to see the serial data going between the PPP layer.
 #define PPP_TRACE_IN_OUT (0)
 
@@ -60,7 +66,7 @@ typedef struct _network_ppp_obj_t {
     struct netif netif;
 } network_ppp_obj_t;
 
-const mp_obj_type_t mp_network_ppp_lwip_type;
+const mp_obj_type_t esp_network_ppp_lwip_type;
 
 static mp_obj_t network_ppp___del__(mp_obj_t self_in);
 
@@ -122,12 +128,12 @@ static mp_obj_t network_ppp___del__(mp_obj_t self_in) {
             // Still connected over the stream.
             // Force the connection to close, with nocarrier=1.
             self->state = STATE_INACTIVE;
-            ppp_close(self->pcb, 1);
+            pppapi_close(self->pcb, 1);
         }
         network_ppp_stream_uart_irq_disable(self);
         // Free PPP PCB and reset state.
         self->state = STATE_INACTIVE;
-        ppp_free(self->pcb);
+        pppapi_free(self->pcb);
         self->pcb = NULL;
     }
     return mp_const_none;
@@ -220,6 +226,17 @@ static mp_obj_t network_ppp_config(size_t n_args, const mp_obj_t *args, mp_map_t
             val = self->stream;
             break;
         }
+        case MP_QSTR_ifname: {
+            if (self->pcb != NULL) {
+                struct netif *pppif = ppp_netif(self->pcb);
+                char ifname[NETIF_NAMESIZE + 1] = {0};
+                netif_index_to_name(netif_get_index(pppif), ifname);
+                if (ifname[0] != 0) {
+                    val = mp_obj_new_str_from_cstr((char *)ifname);
+                }
+            }
+            break;
+        }
         default:
             mp_raise_ValueError(MP_ERROR_TEXT("unknown config param"));
     }
@@ -238,7 +255,12 @@ static mp_obj_t network_ppp_status(mp_obj_t self_in) {
 }
 static MP_DEFINE_CONST_FUN_OBJ_1(network_ppp_status_obj, network_ppp_status);
 
-static u32_t network_ppp_output_callback(ppp_pcb *pcb, const void *data, u32_t len, void *ctx) {
+#if ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 4, 0)
+static u32_t network_ppp_output_callback(ppp_pcb *pcb, const void *data, u32_t len, void *ctx)
+#else
+static u32_t network_ppp_output_callback(ppp_pcb *pcb, u8_t *data, u32_t len, void *ctx)
+#endif
+{
     network_ppp_obj_t *self = ctx;
     #if PPP_TRACE_IN_OUT
     mp_printf(&mp_plat_print, "ppp_out(n=%u,data=", len);
@@ -258,20 +280,35 @@ static u32_t network_ppp_output_callback(ppp_pcb *pcb, const void *data, u32_t l
 }
 
 static mp_obj_t network_ppp_connect(size_t n_args, const mp_obj_t *args, mp_map_t *kw_args) {
-    enum { ARG_security, ARG_user, ARG_key };
+    enum { ARG_security, ARG_user, ARG_key, ARG_authmode, ARG_username, ARG_password };
     static const mp_arg_t allowed_args[] = {
-        { MP_QSTR_security, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = PPPAUTHTYPE_NONE} },
+        { MP_QSTR_security, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = -1} },
         { MP_QSTR_user, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_rom_obj = MP_ROM_NONE} },
         { MP_QSTR_key, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_rom_obj = MP_ROM_NONE} },
+        // Deprecated arguments for backwards compatibility
+        { MP_QSTR_authmode, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = PPPAUTHTYPE_NONE} },
+        { MP_QSTR_username, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_rom_obj = MP_ROM_NONE} },
+        { MP_QSTR_password, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_rom_obj = MP_ROM_NONE} },
     };
 
     mp_arg_val_t parsed_args[MP_ARRAY_SIZE(allowed_args)];
     mp_arg_parse_all(n_args - 1, args + 1, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, parsed_args);
 
+    // Use deprecated arguments as defaults
+    if (parsed_args[ARG_security].u_int == -1) {
+        parsed_args[ARG_security].u_int = parsed_args[ARG_authmode].u_int;
+    }
+    if (parsed_args[ARG_user].u_obj == mp_const_none) {
+        parsed_args[ARG_user].u_obj = parsed_args[ARG_username].u_obj;
+    }
+    if (parsed_args[ARG_key].u_obj == mp_const_none) {
+        parsed_args[ARG_key].u_obj = parsed_args[ARG_password].u_obj;
+    }
+
     network_ppp_obj_t *self = MP_OBJ_TO_PTR(args[0]);
 
     if (self->state == STATE_INACTIVE) {
-        self->pcb = pppos_create(&self->netif, network_ppp_output_callback, network_ppp_status_cb, self);
+        self->pcb = pppapi_pppos_create(&self->netif, network_ppp_output_callback, network_ppp_status_cb, self);
         if (self->pcb == NULL) {
             mp_raise_msg(&mp_type_OSError, MP_ERROR_TEXT("pppos_create failed"));
         }
@@ -296,14 +333,16 @@ static mp_obj_t network_ppp_connect(size_t n_args, const mp_obj_t *args, mp_map_
     if (parsed_args[ARG_security].u_int != PPPAUTHTYPE_NONE) {
         const char *user_str = mp_obj_str_get_str(parsed_args[ARG_user].u_obj);
         const char *key_str = mp_obj_str_get_str(parsed_args[ARG_key].u_obj);
-        ppp_set_auth(self->pcb, parsed_args[ARG_security].u_int, user_str, key_str);
+        pppapi_set_auth(self->pcb, parsed_args[ARG_security].u_int, user_str, key_str);
     }
 
-    ppp_set_default(self->pcb);
+    if (pppapi_set_default(self->pcb) != ERR_OK) {
+        mp_raise_msg(&mp_type_OSError, MP_ERROR_TEXT("ppp_set_default failed"));
+    }
 
     ppp_set_usepeerdns(self->pcb, true);
 
-    if (ppp_connect(self->pcb, 0) != ERR_OK) {
+    if (pppapi_connect(self->pcb, 0) != ERR_OK) {
         mp_raise_msg(&mp_type_OSError, MP_ERROR_TEXT("ppp_connect failed"));
     }
 
@@ -320,7 +359,7 @@ static mp_obj_t network_ppp_disconnect(mp_obj_t self_in) {
     network_ppp_obj_t *self = MP_OBJ_TO_PTR(self_in);
     if (self->state == STATE_CONNECTING || self->state == STATE_CONNECTED) {
         // Initiate close and wait for PPPERR_USER callback.
-        ppp_close(self->pcb, 0);
+        pppapi_close(self->pcb, 0);
     }
     return mp_const_none;
 }
@@ -334,16 +373,101 @@ static MP_DEFINE_CONST_FUN_OBJ_1(network_ppp_isconnected_obj, network_ppp_isconn
 
 static mp_obj_t network_ppp_ifconfig(size_t n_args, const mp_obj_t *args) {
     network_ppp_obj_t *self = MP_OBJ_TO_PTR(args[0]);
-    return mod_network_nic_ifconfig(&self->netif, n_args - 1, args + 1);
+    if (n_args == 1) {
+        // get
+        const ip_addr_t *dns;
+        if (self->pcb != NULL) {
+            dns = dns_getserver(0);
+            struct netif *pppif = ppp_netif(self->pcb);
+            mp_obj_t tuple[4] = {
+                netutils_format_ipv4_addr((uint8_t *)&pppif->ip_addr, NETUTILS_BIG),
+                netutils_format_ipv4_addr((uint8_t *)&pppif->gw, NETUTILS_BIG),
+                netutils_format_ipv4_addr((uint8_t *)&pppif->netmask, NETUTILS_BIG),
+                netutils_format_ipv4_addr((uint8_t *)dns, NETUTILS_BIG),
+            };
+            return mp_obj_new_tuple(4, tuple);
+        } else {
+            mp_obj_t tuple[4] = { mp_const_none, mp_const_none, mp_const_none, mp_const_none };
+            return mp_obj_new_tuple(4, tuple);
+        }
+    } else {
+        ip_addr_t dns;
+        mp_obj_t *items;
+        mp_obj_get_array_fixed_n(args[1], 4, &items);
+        #if CONFIG_LWIP_IPV6
+        netutils_parse_ipv4_addr(items[3], (uint8_t *)&dns.u_addr.ip4, NETUTILS_BIG);
+        #else
+        netutils_parse_ipv4_addr(items[3], (uint8_t *)&dns, NETUTILS_BIG);
+        #endif // CONFIG_LWIP_IPV6
+        dns_setserver(0, &dns);
+        return mp_const_none;
+    }
 }
 static MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(network_ppp_ifconfig_obj, 1, 2, network_ppp_ifconfig);
 
 static mp_obj_t network_ppp_ipconfig(size_t n_args, const mp_obj_t *args, mp_map_t *kwargs) {
     network_ppp_obj_t *self = MP_OBJ_TO_PTR(args[0]);
-    return mod_network_nic_ipconfig(&self->netif, n_args - 1, args + 1, kwargs);
+    if (kwargs->used == 0) {
+        if (self->pcb == NULL) {
+            mp_raise_ValueError(MP_ERROR_TEXT("PPP not active"));
+        }
+        struct netif *netif = ppp_netif(self->pcb);
+        // Get config value
+        if (n_args != 2) {
+            mp_raise_TypeError(MP_ERROR_TEXT("must query one param"));
+        }
+
+        switch (mp_obj_str_get_qstr(args[1])) {
+            case MP_QSTR_addr4: {
+                mp_obj_t tuple[2] = {
+                    netutils_format_ipv4_addr((uint8_t *)&netif->ip_addr, NETUTILS_BIG),
+                    netutils_format_ipv4_addr((uint8_t *)&netif->netmask, NETUTILS_BIG),
+                };
+                return mp_obj_new_tuple(2, tuple);
+            }
+            case MP_QSTR_gw4: {
+                return netutils_format_ipv4_addr((uint8_t *)&netif->gw, NETUTILS_BIG);
+            }
+            default: {
+                mp_raise_ValueError(MP_ERROR_TEXT("unexpected key"));
+                break;
+            }
+        }
+        return mp_const_none;
+    } else {
+        mp_raise_TypeError(MP_ERROR_TEXT("setting properties not supported"));
+    }
+    return mp_const_none;
 }
 static MP_DEFINE_CONST_FUN_OBJ_KW(network_ppp_ipconfig_obj, 1, network_ppp_ipconfig);
 
+static mp_obj_t network_ppp_active(size_t n_args, const mp_obj_t *args) {
+    network_ppp_obj_t *self = MP_OBJ_TO_PTR(args[0]);
+    if (n_args > 1) {
+        if (mp_obj_is_true(args[1])) {
+            if (self->state >= STATE_ACTIVE) {
+                return mp_const_true;
+            }
+
+            self->pcb = pppapi_pppos_create(&self->netif, network_ppp_output_callback, network_ppp_status_cb, self);
+            if (self->pcb == NULL) {
+                mp_raise_msg(&mp_type_OSError, MP_ERROR_TEXT("pppos_create failed"));
+            }
+            self->state = STATE_ACTIVE;
+
+            network_ppp_stream_uart_irq_enable(self);
+        } else {
+            if (self->state < STATE_ACTIVE) {
+                return mp_const_false;
+            }
+
+            network_ppp___del__(MP_OBJ_FROM_PTR(self));
+        }
+    }
+    return mp_obj_new_bool(self->state >= STATE_ACTIVE);
+}
+static MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(network_ppp_active_obj, 1, 2, network_ppp_active);
+
 static const mp_rom_map_elem_t network_ppp_locals_dict_table[] = {
     { MP_ROM_QSTR(MP_QSTR___del__), MP_ROM_PTR(&network_ppp___del___obj) },
     { MP_ROM_QSTR(MP_QSTR_config), MP_ROM_PTR(&network_ppp_config_obj) },
@@ -358,11 +482,17 @@ static const mp_rom_map_elem_t network_ppp_locals_dict_table[] = {
     { MP_ROM_QSTR(MP_QSTR_SEC_NONE), MP_ROM_INT(PPPAUTHTYPE_NONE) },
     { MP_ROM_QSTR(MP_QSTR_SEC_PAP), MP_ROM_INT(PPPAUTHTYPE_PAP) },
     { MP_ROM_QSTR(MP_QSTR_SEC_CHAP), MP_ROM_INT(PPPAUTHTYPE_CHAP) },
+
+    // Deprecated interface for backwards compatibility
+    { MP_ROM_QSTR(MP_QSTR_active), MP_ROM_PTR(&network_ppp_active_obj) },
+    { MP_ROM_QSTR(MP_QSTR_AUTH_NONE), MP_ROM_INT(PPPAUTHTYPE_NONE) },
+    { MP_ROM_QSTR(MP_QSTR_AUTH_PAP), MP_ROM_INT(PPPAUTHTYPE_PAP) },
+    { MP_ROM_QSTR(MP_QSTR_AUTH_CHAP), MP_ROM_INT(PPPAUTHTYPE_CHAP) },
 };
 static MP_DEFINE_CONST_DICT(network_ppp_locals_dict, network_ppp_locals_dict_table);
 
 MP_DEFINE_CONST_OBJ_TYPE(
-    mp_network_ppp_lwip_type,
+    esp_network_ppp_lwip_type,
     MP_QSTR_PPP,
     MP_TYPE_FLAG_NONE,
     make_new, network_ppp_make_new,

</details>

Trade-offs and Alternatives

I didn't fully replace the ESP32 version with the extmod version since there are distinct differences, and the ESP32 port does not use the lwIP version shipped by us but the modified version shipped as part of the ESP-IDF.

I've also aimed to keep the ESP32 version backwards compatible, thus it implements a few extra arguments and settings compared to the extmod version. Removing these deprecated arguments and porting the missing features over to the extmod version was out of scope for this PR, to keep it a bit easier to review.

We might want to add a message for future contributors to be aware that these two files are now very similar and that many changes could probably be applied to both.