arduino-pico icon indicating copy to clipboard operation
arduino-pico copied to clipboard

Initial work for enabling IPv6

Open oddstr13 opened this issue 1 year ago • 7 comments

This brings IPv6 up to the point where

  • Link local address is configured
  • Router Solicitation is sent
  • Router Advertizement is received and used to configure IP addresses
  • Neighbor Solicitation and Advertizement between the WiFi AP and the Pico happens
  • DHCPv6 is queried for DNS server
  • Multicast Listener Report is sent (mld6)
  • [x] Neighbor Solicitation requests are not received by the LWIP stack for some reason. This results in nothing working (IPv6 equivalent of arp request)
  • [x] Pinging the IPv6 addresses from a computer on the network does not work, due to the above issue

Not working:

  • [ ] Sending multicast packet to [ff18::554b:4841:536e:6574:1]:20750 results in :ust rc=-6 (Illegal value) Udp. beginPacketMulticast, write, endPacket

oddstr13 avatar Aug 01 '22 00:08 oddstr13

I suspect the cyw43 WiFi driver or firmware is discarding multicast MAC addresses. Looks like lwip is supposed to hook into the network interface to configure the filters, but that isn't configured here. I have not managed to find a way to do this in the cyw43 driver.

https://github.com/lwip-tcpip/lwip/blob/239918ccc173cb2c2a62f41a40fd893f57faf1d6/src/core/ipv6/mld6.c#L7-L13

https://github.com/lwip-tcpip/lwip/blob/159e31b689577dbf69cf0683bbaffbd71fa5ee10/src/include/lwip/netif.h#L369-L378

https://github.com/lwip-tcpip/lwip/blob/159e31b689577dbf69cf0683bbaffbd71fa5ee10/src/include/lwip/netif.h#L215-L224

It might be possible to work around this by enabling monitor mode, but that code is disabled in the driver for some reason

https://github.com/georgerobotics/cyw43-driver/blob/195dfcc10bb6f379e3dea45147590db2203d3c7b/src/cyw43_ll.c#L1848-L1865

https://github.com/georgerobotics/cyw43-driver/blob/195dfcc10bb6f379e3dea45147590db2203d3c7b/src/cyw43_ll.h#L57

In particular, cyw43_write_iovar_u32(self, "allmulti", value, WWD_STA_INTERFACE) looks interesting, as it looks like it might be enabling/disabling filtering of multicast packets.

oddstr13 avatar Aug 01 '22 22:08 oddstr13

I can confirm that calling cyw43_write_iovar_u32(self, "allmulti", true, WWD_STA_INTERFACE) enables multicast to work and thus ping and http over IPv6. I don't know how best to integrate that particular change into this repo, but here's a full diff of my current workdir (on top of this pr branch, includes a few non-relevant changes, mostly to get access to the stats)

File multicast.diff
diff --git a/.gitignore b/.gitignore
index 8d4200a..c6aec37 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,3 +1,4 @@
 .DS_Store
 system
 tools/dist
+tools/libpico/build
diff --git a/cores/rp2040/sdkoverride/cyw43_arch_threadsafe_background.c b/cores/rp2040/sdkoverride/cyw43_arch_threadsafe_background.c
index 66c7a3a..07ef28a 100644
--- a/cores/rp2040/sdkoverride/cyw43_arch_threadsafe_background.c
+++ b/cores/rp2040/sdkoverride/cyw43_arch_threadsafe_background.c
@@ -89,7 +89,7 @@ semaphore_t cyw43_irq_sem;
 
 // Called in low priority pendsv interrupt only to do lwip processing and check cyw43 sleep
 static void periodic_worker(void) {
-#if CYW43_USE_STATS
+#if CYW43_USE_STATS && LWIP_SYS_CHECK_MS
     static uint32_t counter;
     if (counter++ % (30000 / LWIP_SYS_CHECK_MS) == 0) {
         cyw43_dump_stats();
diff --git a/lib/libpico-ipv6.a b/lib/libpico-ipv6.a
index 4235bc9..6506547 100644
Binary files a/lib/libpico-ipv6.a and b/lib/libpico-ipv6.a differ
diff --git a/lib/libpico.a b/lib/libpico.a
index c4b8e83..a1276b9 100644
Binary files a/lib/libpico.a and b/lib/libpico.a differ
diff --git a/libraries/lwIP_CYW43/src/utility/CYW43shim.cpp b/libraries/lwIP_CYW43/src/utility/CYW43shim.cpp
index a216413..14a8425 100644
--- a/libraries/lwIP_CYW43/src/utility/CYW43shim.cpp
+++ b/libraries/lwIP_CYW43/src/utility/CYW43shim.cpp
@@ -49,6 +49,9 @@ bool CYW43::begin(const uint8_t* address, netif* netif) {
         if (_password == nullptr) {
             authmode = CYW43_AUTH_OPEN;
         }
+        
+        Serial.println("DBG: Enabling monitor mode!");
+        cyw43_set_monitor_mode(_self, true);
 
         if (cyw43_arch_wifi_connect_timeout_ms(_ssid, _password, authmode, _timeout)) {
             return false;
Submodule pico-sdk contains modified content
Submodule lib/cyw43-driver contains modified content
diff --git a/pico-sdk/lib/cyw43-driver/src/cyw43.h b/pico-sdk/lib/cyw43-driver/src/cyw43.h
index 8042417..f6cf076 100644
--- a/pico-sdk/lib/cyw43-driver/src/cyw43.h
+++ b/pico-sdk/lib/cyw43-driver/src/cyw43.h
@@ -176,6 +176,16 @@ int cyw43_ioctl(cyw43_t *self, uint32_t cmd, size_t len, uint8_t *buf, uint32_t
  */
 int cyw43_send_ethernet(cyw43_t *self, int itf, size_t len, const void *buf, bool is_pbuf);
 
+/*!
+ * \brief Enable/disable monitor mode
+ * 
+ * This method allows enabling monitoring mode.
+ * \param self the driver state object. This should always be \c &cyw43_state
+ * \param value true to enable monitor mode
+ * \return 0 on success
+ */
+int cyw43_set_monitor_mode(cyw43_t *self, bool value);
+
 /*!
  * \brief Set the wifi power management mode
  *
diff --git a/pico-sdk/lib/cyw43-driver/src/cyw43_ctrl.c b/pico-sdk/lib/cyw43-driver/src/cyw43_ctrl.c
index 7afb9f7..902fefe 100644
--- a/pico-sdk/lib/cyw43-driver/src/cyw43_ctrl.c
+++ b/pico-sdk/lib/cyw43-driver/src/cyw43_ctrl.c
@@ -442,6 +442,24 @@ int cyw43_send_ethernet(cyw43_t *self, int itf, size_t len, const void *buf, boo
     return ret;
 }
 
+/*******************************************************************************/
+// Monitor mode
+
+
+int cyw43_set_monitor_mode(cyw43_t *self, bool value) {
+    CYW43_THREAD_ENTER;
+    int ret = cyw43_ensure_up(self);
+    if (ret) {
+        CYW43_THREAD_EXIT;
+        return ret;
+    }
+
+    ret = cyw43_ll_set_monitor_mode(&self->cyw43_ll, value);
+    CYW43_THREAD_EXIT;
+
+    return ret;
+}
+
 /*******************************************************************************/
 // WiFi control
 
diff --git a/pico-sdk/lib/cyw43-driver/src/cyw43_ll.c b/pico-sdk/lib/cyw43-driver/src/cyw43_ll.c
index d46dd68..c3c025b 100644
--- a/pico-sdk/lib/cyw43-driver/src/cyw43_ll.c
+++ b/pico-sdk/lib/cyw43-driver/src/cyw43_ll.c
@@ -1844,22 +1844,14 @@ static uint32_t cyw43_read_iovar_u32(cyw43_int_t *self, const char *var, uint32_
 }
 #endif
 
-#if 0
+#if 1
 #define WLC_SET_MONITOR (108)
-int cyw43_set_monitor_mode(cyw43_ll_t *self, int value) {
-    CYW_THREAD_ENTER
-    int ret = cyw43_ensure_up(self);
-    if (ret) {
-        CYW_THREAD_EXIT
-        return ret;
-    }
+int cyw43_ll_set_monitor_mode(cyw43_ll_t *self_in, bool value) {
+    cyw43_int_t *self = CYW_INT_FROM_LL(self_in);
 
-    CYW_ENTER
-    self->is_monitor_mode = value;
+    //self->is_monitor_mode = value;
     cyw43_write_iovar_u32(self, "allmulti", value, WWD_STA_INTERFACE);
-    cyw43_do_ioctl_u32(self, SDPCM_SET, WLC_SET_MONITOR, value, WWD_STA_INTERFACE);
-    CYW_EXIT
-    CYW_THREAD_EXIT
+    //cyw43_do_ioctl_u32(self, SDPCM_SET, WLC_SET_MONITOR, value, WWD_STA_INTERFACE);
 
     return 0;
 }
diff --git a/pico-sdk/lib/cyw43-driver/src/cyw43_ll.h b/pico-sdk/lib/cyw43-driver/src/cyw43_ll.h
index 213b079..8fc2dd4 100644
--- a/pico-sdk/lib/cyw43-driver/src/cyw43_ll.h
+++ b/pico-sdk/lib/cyw43-driver/src/cyw43_ll.h
@@ -256,6 +256,8 @@ void cyw43_ll_process_packets(cyw43_ll_t *self);
 int cyw43_ll_ioctl(cyw43_ll_t *self, uint32_t cmd, size_t len, uint8_t *buf, uint32_t iface);
 int cyw43_ll_send_ethernet(cyw43_ll_t *self, int itf, size_t len, const void *buf, bool is_pbuf);
 
+int cyw43_ll_set_monitor_mode(cyw43_ll_t *self, bool value);
+
 int cyw43_ll_wifi_on(cyw43_ll_t *self, uint32_t country);
 int cyw43_ll_wifi_pm(cyw43_ll_t *self, uint32_t pm, uint32_t pm_sleep_ret, uint32_t li_bcn, uint32_t li_dtim, uint32_t li_assoc);
 int cyw43_ll_wifi_scan(cyw43_ll_t *self, cyw43_wifi_scan_options_t *opts);
diff --git a/pico-sdk/src/rp2_common/pico_cyw43_arch/cyw43_arch_freertos.c b/pico-sdk/src/rp2_common/pico_cyw43_arch/cyw43_arch_freertos.c
index 88dde6e..58a3cfb 100644
--- a/pico-sdk/src/rp2_common/pico_cyw43_arch/cyw43_arch_freertos.c
+++ b/pico-sdk/src/rp2_common/pico_cyw43_arch/cyw43_arch_freertos.c
@@ -53,7 +53,7 @@ static uint8_t cyw43_core_num;
 // Called in low priority pendsv interrupt only to do lwip processing and check cyw43 sleep
 static void periodic_worker(__unused TimerHandle_t handle)
 {
-#if CYW43_USE_STATS
+#if CYW43_USE_STATS && LWIP_SYS_CHECK_MS
     static uint32_t counter;
     if (counter++ % (30000 / LWIP_SYS_CHECK_MS) == 0) {
         cyw43_dump_stats();
diff --git a/pico-sdk/src/rp2_common/pico_cyw43_arch/cyw43_arch_threadsafe_background.c b/pico-sdk/src/rp2_common/pico_cyw43_arch/cyw43_arch_threadsafe_background.c
index a3dde74..6d19048 100644
--- a/pico-sdk/src/rp2_common/pico_cyw43_arch/cyw43_arch_threadsafe_background.c
+++ b/pico-sdk/src/rp2_common/pico_cyw43_arch/cyw43_arch_threadsafe_background.c
@@ -71,7 +71,7 @@ semaphore_t cyw43_irq_sem;
 // Called in low priority pendsv interrupt only to do lwip processing and check cyw43 sleep
 static void periodic_worker(void)
 {
-#if CYW43_USE_STATS
+#if CYW43_USE_STATS && LWIP_SYS_CHECK_MS
     static uint32_t counter;
     if (counter++ % (30000 / LWIP_SYS_CHECK_MS) == 0) {
         cyw43_dump_stats();
diff --git a/pico-sdk/src/rp2_common/pico_cyw43_arch/include/cyw43_configport.h b/pico-sdk/src/rp2_common/pico_cyw43_arch/include/cyw43_configport.h
index 7827c81..9c5754c 100644
--- a/pico-sdk/src/rp2_common/pico_cyw43_arch/include/cyw43_configport.h
+++ b/pico-sdk/src/rp2_common/pico_cyw43_arch/include/cyw43_configport.h
@@ -50,7 +50,7 @@
 #endif
 
 #ifndef CYW43_USE_STATS
-#define CYW43_USE_STATS 0
+#define CYW43_USE_STATS 1
 #endif
 
 // todo should this be user settable?

oddstr13 avatar Aug 02 '22 16:08 oddstr13

In general, multicast should possible. For UDP there should a :::beginMulticast(IPAddress ip, uint16_t port) class member like on the Ethernet and Ethernet3 library.

sstaub avatar Aug 02 '22 17:08 sstaub

@sstaub This is work on the underlying layers that the mentioned function utilizes, so that the standard Arduino methods can work properly.

Multicast is currently not possible without the mentioned wifi driver hack, as the wireless driver filters out all packets to ethernet MAC addresses other than the device and broadcast ones. Multicast (both IPv4 and IPv6) uses MAC addresses other than the device and broadcast ones.

Multicast is a requirement for basic IPv6 functionality, and part of the core specification. The Udp::beginMulticast() function seems to be IPv4 specific, but more importantly depends on igmp_mac_filter (mld_mac_filter for IPv6), which is currently not possible to implement with the current state of the wifi driver library (and possibly firmware). Disabling filtering by setting the allmulti flag in the firmware is a workaround, which I found in a piece of commented out code in the wifi driver.

Without multicast, the IPv6 stack doesn't know which MAC address to send the packets to, and as such doesn't even try to send them. I can send multicast packets without working reception of multicast, but other devices wouldn't be able to even send something as basic as a ping back to the pico.

oddstr13 avatar Aug 03 '22 21:08 oddstr13

@oddstr13 can you make a patchfile for the SDK with the change? I can have the applied as part of make-libpico.sh. Or do you think it would be possible to call the cyw43_xxx internal functions from the core? It seems like you're 99% of the way there except for this driver issue, and I'd hate to lose the good work you've done because upstream is slow to respond.

earlephilhower avatar Aug 06 '22 17:08 earlephilhower

@earlephilhower We could potentially access the low level driver functions, but I think that'd be a bit messy. Cleaner to apply a patch in my opinion.

I've cone ahead and committed a patch, as well as writing a function for applying it in make-libpico.sh

I'm seeing lwip errors attempting to send udp multicast packets (didn't spot that one before last night, haven't tracked down the reason yet), but I think that is best handled in a new PR, as this one right now brings up IPv6 unicast.

oddstr13 avatar Aug 06 '22 22:08 oddstr13

Well, that is a little problematic. The make-libpico.sh script isn't getting called by CI, neither during tests nor release.

oddstr13 avatar Aug 08 '22 11:08 oddstr13

The make-libpico.sh script isn't getting called by CI, neither during tests nor release.

That's a feature, not a bug. :) The script is normally run to generate a new libpico.a so as not to pollute the repo with tons of libpico.a blobs with no changes (library files are not generally reproducible since they include things like timestamps).

You can run ./make-libpico.sh and git add lib/libpico*.aas part of this PR, of course.

earlephilhower avatar Aug 08 '22 14:08 earlephilhower

@oddstr13 I think you can select "Edit" up on top of the page and select to merge this into the branch "hack1", and remove all the patching stuff. I manually updated the sub-submodule and updated the repo links accordingly. So your code can just call the new function since it is in the libpico.a and headers automatically.

Once that's merged into the hack1 branch, I can make a PR to merge that into master and we'll be all set.

earlephilhower avatar Aug 09 '22 22:08 earlephilhower