Hi Nikolas,
On Tue, Feb 19, 2013 at 02:55:27PM +1100, Nikolas Karakotas wrote: > Hi Sylvain, > > Ok I think I found the problem. I use PPP_INPROC_OWNTHREAD and once > I added ppp_delete() to the end it seems to fix the problem. > I also removed the ppp_delete from the linkStatusCB. Somehow a clean > up wasn't completed and a pbuf was always allocated but not freed as > you can here: Well, this is one of the reason why I consider to remove the PPP_INPROC_OWNTHREAD crap in ppp-new, as said in bugs #37278 and #37353. 1. It requires to be modified to match user system, like you did adding the vTaskDelete(NULL); FreeRTOS call at the end of the function. This is a tiny-tiny fonction that should be, in my opinion, on the user port, like the Ethernet input thread we see in many Ethernet port. 2. It is actually not thread safe. 2.1. pcb->phase IS modified by the lwIP core thread so it should at least be set to volatile, otherwise the pcb->phase copy may live indefinitely in CPU register. It works because of the sio_read() function call which without doubt flush pcb->phase copy from CPU register. 2.2. This function assume PCB still exists whatever is happening, which is not the case after you called ppp_delete() function outside of this thread. If sio_read() is blocking waiting data and pcb destroyed, it is going to read a deallocated pcb which luckily should still have pcb->phase set to 0 (=PHASE_DEAD) due to preallocated "control block" structures of lwIP. 3. I dislike the sys_msleep(1), it means that systems should have at least a 11 chr buffer at 115200/10 byte/s, and bigger with higher serial speed, for example with 3G/HSDPA modems accessed through SPI, at 20 Mbits/s this is a ~2000 bytes buffer required to keep incoming data during this sleep, I don't see why we require systems to do so, sio_read() should obviously be a blocking call. Of course, I do not want to call ppp_delete() at the end of this thread, keeping the PPP PCB live over (re-)connections is necessary to keep the netif interface that might be used with udp_sendto(), ppp_over_l2tp_open() (for L2TP-VPN over PPPoE/oS), ... and this is why the ppp-new branch creates the netif early during ppp_new(), so that you can start to use this netif elsewhere in the code whatever the PPP session state is. Deleting a PPP PCB is as hard as removing an Ethernet netif. If I were you I should rewrite the ppp_input_thread() on your side with your FreeRTOS-related change and with the necessary signaling to kill this thread -before- you call ppp_delete() from another thread, this will fix the threading issue you might have (but unlikely to happen, luckily) and this will help me removing this one so that you can upgrade to the updated branch without PPP_INPROC_OWNTHREAD ;-) But, honestly, deleting a PPP session control block using ppp_delete() (or pppapi_delete()) requires you to ensure that are not using anymore the pcb ptr nor the PPP netif, which requires in my opinion a quite difficult signaling all over your threads to tell them to stop using those objects. I am going to check if adding the ppp_free_current_input_packet() in ppp_delete() is necessary, it depends if I already do so at the end of the connection or not which I don't remember. Sylvain
signature.asc
Description: Digital signature
_______________________________________________ lwip-users mailing list [email protected] https://lists.nongnu.org/mailman/listinfo/lwip-users
