fix(eppp): Fixup UART transport

Author: david-cermak

components/eppp_link/CMakeLists.txt

@@ -20,7 +20,11 @@ if(CONFIG_EPPP_LINK_DEVICE_SDIO)
     set(transport_src eppp_sdio.c eppp_sdio_slave.c eppp_sdio_host.c)
 endif()
 
-idf_component_register(SRCS eppp_link.c ${transport_src} eppp_slip.c
+if(NOT CONFIG_EPPP_LINK_USES_PPP)
+    set(netif_src eppp_netif_tun.c)
+endif()
+
+idf_component_register(SRCS eppp_link.c ${transport_src} ${netif_src}
                     INCLUDE_DIRS "include"
                     PRIV_REQUIRES esp_netif esp_timer esp_eth ${driver_deps})
 

components/eppp_link/Kconfig

@@ -1,10 +1,16 @@
 menu "eppp_link"
 
-    config EPPP_LINK_USES_LWIP
-        bool
-        default "y"
+    config EPPP_LINK_USES_PPP
+        bool "Use PPP network interface"
+        default "n"
         select LWIP_PPP_SUPPORT
         select LWIP_PPP_SERVER_SUPPORT
+        help
+            Enable this option to use PPP network interface.
+            This is useful when pairing with another PPP device,
+            e.g. pppd service on Linux.
+            By default EPPP_LINK uses plain TUN interface,
+            relying on transports to split on packet boundaries.
 
     choice EPPP_LINK_DEVICE
         prompt "Choose PPP device"

components/eppp_link/eppp_link.c

@@ -34,14 +34,7 @@ static const char *TAG = "eppp_link";
 static int s_retry_num = 0;
 static int s_eppp_netif_count = 0; // used as a suffix for the netif key
 
-#if CONFIG_EPPP_LINK_DEVICE_SDIO
-
-#elif CONFIG_EPPP_LINK_DEVICE_ETH
-
-#else
-#endif
-
-static void netif_deinit(esp_netif_t *netif)
+void eppp_netif_deinit(esp_netif_t *netif)
 {
     if (netif == NULL) {
         return;
@@ -52,47 +45,43 @@ static void netif_deinit(esp_netif_t *netif)
     }
 }
 
-extern esp_netif_netstack_config_t *netstack_default_slip;
+#ifdef CONFIG_EPPP_LINK_USES_PPP
+#define NETSTACK_CONFIG() ESP_NETIF_NETSTACK_DEFAULT_PPP
+#else
+#define NETSTACK_CONFIG() g_eppp_netif_config_tun
+extern esp_netif_netstack_config_t *g_eppp_netif_config_tun;
 #define ESP_NETIF_INHERENT_DEFAULT_SLIP() \
     {   \
         ESP_COMPILER_DESIGNATED_INIT_AGGREGATE_TYPE_EMPTY(mac) \
         ESP_COMPILER_DESIGNATED_INIT_AGGREGATE_TYPE_EMPTY(ip_info) \
         .flags = ESP_NETIF_FLAG_EVENT_IP_MODIFIED,              \
         .get_ip_event = IP_EVENT_PPP_GOT_IP,    \
         .lost_ip_event = IP_EVENT_PPP_LOST_IP,   \
-        .if_key = "SLP_DEF",  \
-        .if_desc = "slip",    \
+        .if_key = "EPPP_TUN",  \
+        .if_desc = "eppp",    \
         .route_prio = 1,     \
         .bridge_info = NULL \
 };
+#endif // CONFIG_EPPP_LINK_USES_PPP
 
-static esp_netif_t *netif_init(eppp_type_t role, eppp_transport_handle_t h, eppp_config_t *eppp_config)
+esp_netif_t *eppp_netif_init(eppp_type_t role, eppp_transport_handle_t h, eppp_config_t *eppp_config)
 {
     if (s_eppp_netif_count > 9) {   // Limit to max 10 netifs, since we use "EPPPx" as the unique key (where x is 0-9)
         ESP_LOGE(TAG, "Cannot create more than 10 instances");
         return NULL;
     }
 
     h->role = role;
-//    esp_netif_driver_ifconfig_t driver_cfg = {
-//        .handle = h,
-//#if CONFIG_EPPP_LINK_DEVICE_SDIO
-//        .transmit = role == EPPP_CLIENT ? eppp_sdio_host_tx : eppp_sdio_slave_tx,
-//#elif CONFIG_EPPP_LINK_DEVICE_ETH
-//        .transmit = eppp_transport_tx,
-//#else
-//        .transmit = transmit,
-//#endif
-//    };
-//    const esp_netif_driver_ifconfig_t *ppp_driver_cfg = &driver_cfg;
+#ifdef CONFIG_EPPP_LINK_USES_PPP
+    esp_netif_inherent_config_t base_netif_cfg = ESP_NETIF_INHERENT_DEFAULT_PPP();
+#else
+    esp_netif_inherent_config_t base_netif_cfg = ESP_NETIF_INHERENT_DEFAULT_SLIP();
     esp_netif_ip_info_t slip_ip4 = {};
     slip_ip4.ip.addr = eppp_config->ppp.our_ip4_addr.addr;
     slip_ip4.gw.addr = eppp_config->ppp.their_ip4_addr.addr;
     slip_ip4.netmask.addr = ESP_IP4TOADDR(255, 255, 255, 0);
-
-    esp_netif_inherent_config_t base_netif_cfg = ESP_NETIF_INHERENT_DEFAULT_SLIP();
     base_netif_cfg.ip_info = &slip_ip4;
-
+#endif
     char if_key[] = "EPPP0"; // netif key needs to be unique
     if_key[sizeof(if_key) - 2 /* 2 = two chars before the terminator */ ] += s_eppp_netif_count++;
     base_netif_cfg.if_key = if_key;
@@ -104,12 +93,27 @@ static esp_netif_t *netif_init(eppp_type_t role, eppp_transport_handle_t h, eppp
     if (eppp_config->ppp.netif_prio) {
         base_netif_cfg.route_prio = eppp_config->ppp.netif_prio;
     }
-    esp_netif_config_t netif_ppp_config = { .base = &base_netif_cfg,
-                                            .driver = NULL,
-                                            .stack = netstack_default_slip,
-                                          };
+    esp_netif_config_t netif_config = { .base = &base_netif_cfg,
+                                        .driver = NULL,
+                                        .stack = NETSTACK_CONFIG(),
+                                      };
 
-    return esp_netif_new(&netif_ppp_config);
+#ifdef CONFIG_EPPP_LINK_USES_PPP
+    __attribute__((unused)) esp_err_t ret = ESP_OK;
+    esp_netif_t *netif = esp_netif_new(&netif_config);
+    esp_netif_ppp_config_t netif_params;
+    ESP_GOTO_ON_ERROR(esp_netif_ppp_get_params(netif, &netif_params), err, TAG, "Failed to get PPP params");
+    netif_params.ppp_our_ip4_addr = eppp_config->ppp.our_ip4_addr;
+    netif_params.ppp_their_ip4_addr = eppp_config->ppp.their_ip4_addr;
+    netif_params.ppp_error_event_enabled = true;
+    ESP_GOTO_ON_ERROR(esp_netif_ppp_set_params(netif, &netif_params), err, TAG, "Failed to set PPP params");
+    return netif;
+err:
+    esp_netif_destroy(netif);
+    return NULL;
+#else
+    return esp_netif_new(&netif_config);
+#endif // CONFIG_EPPP_LINK_USES_PPP
 }
 
 esp_err_t eppp_netif_stop(esp_netif_t *netif, int stop_timeout_ms)
@@ -130,14 +134,16 @@ esp_err_t eppp_netif_stop(esp_netif_t *netif, int stop_timeout_ms)
     return ESP_OK;
 }
 
-void try_ping(esp_netif_t *netif);
-
 esp_err_t eppp_netif_start(esp_netif_t *netif)
 {
     esp_netif_action_start(netif, 0, 0, 0);
     esp_netif_action_connected(netif, 0, 0, 0);
-    try_ping(netif);
+#ifndef CONFIG_EPPP_LINK_USES_PPP
+    // PPP provides address negotiation, if not PPP, we need to check connection manually
+    return eppp_check_connection(netif);
+#else
     return ESP_OK;
+#endif
 }
 
 static int get_netif_num(esp_netif_t *netif)
@@ -160,7 +166,6 @@ static int get_netif_num(esp_netif_t *netif)
 static void on_ppp_event(void *arg, esp_event_base_t base, int32_t event_id, void *data)
 {
     esp_netif_t **netif = data;
-    ESP_LOGD(TAG, "PPP status event: %" PRId32, event_id);
     if (base == NETIF_PPP_STATUS && event_id == NETIF_PPP_ERRORUSER) {
         ESP_LOGI(TAG, "Disconnected %d", get_netif_num(*netif));
         struct eppp_handle *h = esp_netif_get_io_driver(*netif);
@@ -193,14 +198,6 @@ static void on_ip_event(void *arg, esp_event_base_t base, int32_t event_id, void
     }
 }
 
-//#if CONFIG_EPPP_LINK_DEVICE_SPI
-
-#if CONFIG_EPPP_LINK_DEVICE_SDIO
-
-
-
-#endif //  UART
-
 #if EPPP_NEEDS_TASK
 static void ppp_task(void *args)
 {
@@ -221,7 +218,7 @@ static void remove_handlers(void)
 {
     esp_netif_t *netif = esp_netif_find_if(have_some_eppp_netif, NULL);
     if (netif == NULL) {
-        // if EPPP netif in the system, we cleanup the statics
+        // if EPPP netif in the system, we clean up the statics
         vEventGroupDelete(s_event_group);
         s_event_group = NULL;
         esp_event_handler_unregister(IP_EVENT, ESP_EVENT_ANY_ID, on_ip_event);
@@ -235,23 +232,8 @@ void eppp_deinit(esp_netif_t *netif)
         return;
     }
     struct eppp_handle *h = esp_netif_get_io_driver(netif);
-#if CONFIG_EPPP_LINK_DEVICE_SPI
-    EPPP_TRANSPORT_DEINIT(h);
-//    if (h->role == EPPP_CLIENT) {
-//        deinit_master(netif);
-//    } else {
-//        deinit_slave(netif);
-//    }
-#elif CONFIG_EPPP_LINK_DEVICE_UART
-    EPPP_TRANSPORT_DEINIT(h);
-//    deinit_uart(esp_netif_get_io_driver(netif));
-#elif CONFIG_EPPP_LINK_DEVICE_SDIO
-//    struct eppp_handle *h = esp_netif_get_io_driver(netif);
     EPPP_TRANSPORT_DEINIT(h);
-#elif CONFIG_EPPP_LINK_DEVICE_ETH
-    EPPP_TRANSPORT_DEINIT(h);
-#endif
-    netif_deinit(netif);
+    eppp_netif_deinit(netif);
 }
 
 esp_netif_t *eppp_init(eppp_type_t role, eppp_config_t *config)
@@ -265,47 +247,15 @@ esp_netif_t *eppp_init(eppp_type_t role, eppp_config_t *config)
 
     eppp_transport_handle_t h = EPPP_TRANSPORT_INIT(config);
     ESP_GOTO_ON_FALSE(h, ESP_ERR_NO_MEM, err, TAG, "Failed to init EPPP transport");
-    ESP_GOTO_ON_FALSE(netif = netif_init(role, h, config), ESP_ERR_NO_MEM, err, TAG, "Failed to init EPPP netif");
-
-#ifdef CONFIG_EPPP_OVER_PPP
-    esp_netif_ppp_config_t netif_params;
-    ESP_ERROR_CHECK(esp_netif_ppp_get_params(netif, &netif_params));
-    netif_params.ppp_our_ip4_addr = config->ppp.our_ip4_addr;
-    netif_params.ppp_their_ip4_addr = config->ppp.their_ip4_addr;
-    netif_params.ppp_error_event_enabled = true;
-    ESP_ERROR_CHECK(esp_netif_ppp_set_params(netif, &netif_params));
-#endif
-
-#if CONFIG_EPPP_LINK_DEVICE_UART
-//    init_uart(esp_netif_get_io_driver(netif), config);
-#elif CONFIG_EPPP_LINK_DEVICE_SDIO
-//    esp_err_t ret;
-////    if (role == EPPP_SERVER) {
-////        ret = eppp_sdio_slave_init();
-////    } else {
-////        ret = eppp_sdio_host_init(&config->sdio);
-////    }
-//
-//    if (ret != ESP_OK) {
-//        ESP_LOGE(TAG, "Failed to initialize SDIO %d", ret);
-//        return NULL;
-//    }
-#elif CONFIG_EPPP_LINK_DEVICE_ETH
-//    ret = eppp_transport_init(config, netif);
-//    if (ret != ESP_OK) {
-//        ESP_LOGE(TAG, "Failed to initialize SDIO %d", ret);
-//        return NULL;
-//    }
-
-#endif
+    ESP_GOTO_ON_FALSE(netif = eppp_netif_init(role, h, config), ESP_ERR_NO_MEM, err, TAG, "Failed to init EPPP netif");
     ESP_GOTO_ON_ERROR(esp_netif_attach(netif, h), err, TAG, "Failed to attach netif to EPPP transport");
     return netif;
 err:
     if (h) {
         EPPP_TRANSPORT_DEINIT(h);
     }
     if (netif) {
-        netif_deinit(netif);
+        eppp_netif_deinit(netif);
     }
     return NULL;
 }