fix(eppp_link): Per review comments

components/eppp_link/README.md

@@ -1,6 +1,14 @@
 # ESP PPP Link component (eppp_link)
 
-The component provides a general purpose connectivity engine between two micro-controllers, one acting as PPP server (slave), the other one as PPP client (host). Typical application is a WiFi connectivity provider for chips that do not have WiFi:
+The component provides a general purpose connectivity engine between two microcontrollers, one acting as PPP server (slave), the other one as PPP client (host).
+This component could be used for extending network using physical serial connection. Applications could vary from providing PRC engine for multiprocessor solutions to serial connection to POSIX machine. Typical application is a WiFi connectivity provider for chips that do not have WiFi
+
+## Typical application
+
+Using this component we can construct a WiFi connectivity gateway on PPP channel. The below diagram depicts an application where
+PPP server is running on a WiFi capable chip with NAPT module translating packets between WiFi and PPPoS interface.
+We usually call this node a SLAVE microcontroller. The "HOST" microcontroller runs PPP client and connects only to the serial line,
+brings in the WiFi connectivity from the "SLAVE" microcontroller.
 
 ```
              SLAVE micro                                  HOST micro
@@ -35,10 +43,10 @@ Tested with WiFi-NAPT example, no IRAM optimizations
 
 ### UART @ 3Mbauds
 
-* TCP - 2Mbits
-* UDP - 2Mbits
+* TCP - 2Mbits/s
+* UDP - 2Mbits/s
 
 ### SPI @ 20MHz
 
-* TCP - 6Mbits
-* UDP - 10Mbits
+* TCP - 6Mbits/s
+* UDP - 10Mbits/s

components/eppp_link/eppp_link.c

@@ -43,7 +43,7 @@ struct eppp_handle {
     uart_port_t uart_port;
 #endif
     esp_netif_t *netif;
-    enum eppp_type role;
+    eppp_type_t role;
     bool stop;
     bool exited;
     bool netif_stop;
@@ -63,7 +63,8 @@ static esp_err_t transmit(void *h, void *buffer, size_t len)
 
     uint8_t *current_buffer = buffer;
     size_t remaining = len;
-    do {
+    do {    // TODO: Refactor this loop to allocate only once and perform
+        //       fragmentation after receiving from the queue (applicable only if MSS > MAX_PAYLOAD)
         size_t batch = remaining > MAX_PAYLOAD ? MAX_PAYLOAD : remaining;
         buf.data = malloc(batch);
         if (buf.data == NULL) {
@@ -97,6 +98,12 @@ static void netif_deinit(esp_netif_t *netif)
         return;
     }
 #if CONFIG_EPPP_LINK_DEVICE_SPI
+    struct packet buf = { };
+    while (xQueueReceive(h->out_queue, &buf, 0) == pdTRUE) {
+        if (buf.len > 0) {
+            free(buf.data);
+        }
+    }
     vQueueDelete(h->out_queue);
     if (h->role == EPPP_CLIENT) {
         vSemaphoreDelete(h->ready_semaphore);
@@ -109,7 +116,7 @@ static void netif_deinit(esp_netif_t *netif)
     }
 }
 
-static esp_netif_t *netif_init(enum eppp_type role)
+static esp_netif_t *netif_init(eppp_type_t role)
 {
     if (s_eppp_netif_count > 9) {
         ESP_LOGE(TAG, "Cannot create more than 10 instances");
@@ -575,15 +582,6 @@ esp_err_t eppp_perform(esp_netif_t *netif)
     return ESP_OK;
 }
 
-static void ppp_task(void *args)
-{
-    esp_netif_t *netif = args;
-    while (eppp_perform(netif) != ESP_ERR_TIMEOUT) {}
-    struct eppp_handle *h = esp_netif_get_io_driver(netif);
-    h->exited = true;
-    vTaskDelete(NULL);
-}
-
 #elif CONFIG_EPPP_LINK_DEVICE_UART
 #define BUF_SIZE (1024)
 
@@ -636,17 +634,17 @@ esp_err_t eppp_perform(esp_netif_t *netif)
     return ESP_OK;
 }
 
+#endif // CONFIG_EPPP_LINK_DEVICE_SPI / UART
+
 static void ppp_task(void *args)
 {
     esp_netif_t *netif = args;
-    while (eppp_perform(netif) == ESP_OK) {}
+    while (eppp_perform(netif) != ESP_ERR_TIMEOUT) {}
     struct eppp_handle *h = esp_netif_get_io_driver(netif);
     h->exited = true;
     vTaskDelete(NULL);
 }
 
-#endif // CONFIG_EPPP_LINK_DEVICE_SPI / UART
-
 static bool have_some_eppp_netif(esp_netif_t *netif, void *ctx)
 {
     return get_netif_num(netif) > 0;
@@ -679,7 +677,7 @@ void eppp_deinit(esp_netif_t *netif)
     netif_deinit(netif);
 }
 
-esp_netif_t *eppp_init(enum eppp_type role, eppp_config_t *config)
+esp_netif_t *eppp_init(eppp_type_t role, eppp_config_t *config)
 {
     esp_netif_t *netif = netif_init(role);
     if (!netif) {
@@ -710,7 +708,7 @@ esp_netif_t *eppp_init(enum eppp_type role, eppp_config_t *config)
     return netif;
 }
 
-esp_netif_t *eppp_open(enum eppp_type role, eppp_config_t *config, TickType_t connect_timeout)
+esp_netif_t *eppp_open(eppp_type_t role, eppp_config_t *config, TickType_t connect_timeout)
 {
 #if CONFIG_EPPP_LINK_DEVICE_UART
     if (config->transport != EPPP_TRANSPORT_UART) {

components/eppp_link/examples/host/main/register_iperf.c

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2023 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2023-2024 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Unlicense OR CC0-1.0
  */
@@ -46,18 +46,14 @@ static struct {
 static int ppp_cmd_iperf(int argc, char **argv)
 {
     int nerrors = arg_parse(argc, argv, (void **)&iperf_args);
-    iperf_cfg_t cfg;
+    // ethernet iperf only support IPV4 address
+    iperf_cfg_t cfg = {.type = IPERF_IP_TYPE_IPV4};
 
     if (nerrors != 0) {
         arg_print_errors(stderr, iperf_args.end, argv[0]);
         return 0;
     }
 
-    memset(&cfg, 0, sizeof(cfg));
-
-    // ethernet iperf only support IPV4 address
-    cfg.type = IPERF_IP_TYPE_IPV4;
-
     /* iperf -a */
     if (iperf_args.abort->count != 0) {
         iperf_stop();

components/eppp_link/examples/host/sdkconfig.defaults

@@ -3,6 +3,6 @@
 #
 CONFIG_UART_ISR_IN_IRAM=y
 CONFIG_LWIP_PPP_SUPPORT=y
+CONFIG_LWIP_PPP_SERVER_SUPPORT=y
 CONFIG_LWIP_PPP_VJ_HEADER_COMPRESSION=n
 CONFIG_LWIP_PPP_DEBUG_ON=y
-CONFIG_EPPP_LINK_DEVICE_SPI=y

components/eppp_link/include/eppp_link.h

@@ -92,11 +92,11 @@ esp_netif_t *eppp_listen(eppp_config_t *config);
 
 void eppp_close(esp_netif_t *netif);
 
-esp_netif_t *eppp_init(enum eppp_type role, eppp_config_t *config);
+esp_netif_t *eppp_init(eppp_type_t role, eppp_config_t *config);
 
 void eppp_deinit(esp_netif_t *netif);
 
-esp_netif_t *eppp_open(enum eppp_type role, eppp_config_t *config, TickType_t connect_timeout);
+esp_netif_t *eppp_open(eppp_type_t role, eppp_config_t *config, TickType_t connect_timeout);
 
 esp_err_t eppp_netif_stop(esp_netif_t *netif, TickType_t stop_timeout);