raspberrypi / pico-examples

BSD 3-Clause "New" or "Revised" License
2.61k stars 778 forks source link

reply through the same PCB packet came in #477

Closed mtz29 closed 4 months ago

mtz29 commented 4 months ago

d->udp should be used to set up callbacks and start up UDP server only. Replying via the same PCB instance is freezing Picow.

Raising again against the develop branch.

lurch commented 4 months ago

Perhaps the behaviour varies, depending on what other WiFi devices are involved? :shrug:

mtz29 commented 4 months ago

Perhaps the behaviour varies, depending on what other WiFi devices are involved? 🤷

It is just bare pico w.

Version: 7.95.49 (2271bb6 CY) CRC: b7a28ef3 Date: Mon 2021-11-29 22:50:27 PST Ucode Ver: 1043.2162 FWID 01-c51d9400                                        
cyw43 loaded ok, mac 28:cd:c1:0f:4a:6d                                                                                                                     
API: 12.2                                                                                                                                                  
Data: RaspberryPi.PicoW                                                                                                                                    
Compiler: 1.29.4                                                                                                                                           
ClmImport: 1.47.1                                                                                                                                          
Customization: v5 22/06/24                                                                                                                                 
Creation: 2022-06-24 06:55:08                                                                                                                              
lurch commented 4 months ago

It is just bare pico w.

But presumably some other device is connecting to the PicoW access point / dhcpserver?

mtz29 commented 4 months ago

It is just bare pico w.

But presumably some other device is connecting to the PicoW access point / dhcpserver?

And how is this relevant to the way UDP and lwip are behaving? I tried 2 different client devices (mobile phone and laptop) and using wrong PCB leads to picow getting frozen. There's a reason for passing pcb in callback function, isn't?

lurch commented 4 months ago

And how is this relevant to the way UDP and lwip are behaving?

Sorry, I was just trying to guess at reasons why you might be seeing the problem but Peter isn't? :man_shrugging:

mtz29 commented 4 months ago

And how is this relevant to the way UDP and lwip are behaving?

Sorry, I was just trying to guess at reasons why you might be seeing the problem but Peter isn't? 🤷‍♂️

Maybe he hasn't sent any valid DHCP message (REQUEST/DISCOVERY) so callback wasn't triggered? Debug mode not active so assert does not kick in?

@peterharperuk are you able to provide me with traffic dump?

peterharperuk commented 4 months ago

I connected to the access point with my phone and it opened the web site. Unless you can suggest how to reproduce your lockup I'll have to close this

mtz29 commented 4 months ago

I connected to the access point with my phone and it opened the web site. Unless you can suggest how to reproduce your lockup I'll have to close this

Let's try this:

  1. Connect laptop (ideally linux based) to pico AP
  2. send multiple DHCP requests (DHCPDISCOVER and DHCPREQUEST) - dhclient or dhcping should do the job
  3. reconnect with AP
  4. repeat point 2.
  5. is picow still responsive?
peterharperuk commented 4 months ago

I only have raspberry pi devices, still can't reproduce it. Are you using debug or release? Can you tell me the name of the binary - poll or threadsafe? I added extra logging...

IN DHCPREQUEST OUT DHCPACK DHCPS: client connected: MAC=b8:27:eb:28:ca:99 IP=192.168.4.17 About to call dhcp_socket_sendto d->udp=2000C204 upcb=2000C204 IN DHCPREQUEST OUT DHCPACK DHCPS: client connected: MAC=76:23:a6:1d:54:87 IP=192.168.4.16 About to call dhcp_socket_sendto d->udp=2000C204 upcb=2000C204 IN DHCPDISCOVER OUT DHCPOFFER About to call dhcp_socket_sendto d->udp=2000C204 upcb=2000C204 IN DHCPREQUEST OUT DHCPACK DHCPS: client connected: MAC=b8:27:eb:71:d6:b9 IP=192.168.4.18 About to call dhcp_socket_sendto d->udp=2000C204 upcb=2000C204

mtz29 commented 4 months ago

I only have raspberry pi devices, still can't reproduce it. Are you using debug or release? Can you tell me the name of the binary - poll or threadsafe? I added extra logging...

IN DHCPREQUEST OUT DHCPACK DHCPS: client connected: MAC=b8:27:eb:28:ca:99 IP=192.168.4.17 About to call dhcp_socket_sendto d->udp=2000C204 upcb=2000C204 IN DHCPREQUEST OUT DHCPACK DHCPS: client connected: MAC=76:23:a6:1d:54:87 IP=192.168.4.16 About to call dhcp_socket_sendto d->udp=2000C204 upcb=2000C204 IN DHCPDISCOVER OUT DHCPOFFER About to call dhcp_socket_sendto d->udp=2000C204 upcb=2000C204 IN DHCPREQUEST OUT DHCPACK DHCPS: client connected: MAC=b8:27:eb:71:d6:b9 IP=192.168.4.18 About to call dhcp_socket_sendto d->udp=2000C204 upcb=2000C204

threadsafe.

I spent the entire night on this and I think it might be something wrong with a compiler. I am importing C libs (i.e. dhcpserver) into C++ code and for instance this way passing dhcp_server_t object leads to malformed structure (fetched random data from memory?) when I get recv_arg in callback:

if (dhcp_socket_new_dgram(&upcb, d, dhcp_server_process) != 0) {
...

I if pass the data via buffer (memory allocation) then it is working as expected (getting dhcp lease):

uint8_t *buf = malloc(sizeof(dhcp_server_t));
memcpy(buf, d, sizeof(dhcp_server_t));
if (dhcp_socket_new_dgram(&upcb, buf, dhcp_server_process) != 0) {
...

So it must be the same case in terms of d->udp != upcb.

peterharperuk commented 4 months ago

If you're calling C code from C++ you/we might have to wrap any header files used in extern "C" { } or else it will use the wrong calling convention.

mtz29 commented 4 months ago

If you're calling C code from C++ you/we might have to wrap any header files used in extern "C" { } or else it will use the wrong calling convention.

I know that.

#include <stdio.h>
#include <thread>

extern "C" {
    #include "tusb.h"
    #include "dhcpserver.h"
    #include "dnsserver.h"

But it must be some kind of different incompatibility.

mtz29 commented 4 months ago

I figured it out. it is 100% incompatibility with the way c and c++ allocate memory.

This solves my issue and proves my PR was pointless.

dhcp_server_t *dhcp_server = new dhcp_server_t; // memory allocation here
dhcp_server->udp = new udp_pcb; // memory allocation here
dhcp_server_init(dhcp_server, &gw, &mask);

// EDIT

or simply (no headache if we're dealing with nested structures)

dhcp_server_t *dhcp_server = (dhcp_server_t*) malloc(sizeof(dhcp_server_t));
dhcp_server_init(dhcp_server, &gw, &mask);
peterharperuk commented 4 months ago

That's good. You can close this when you're ready.