[lwip-devel] [bug #44565] PPPOS support in tcpip thread breaks SLIP

[Sylvain Rochet]

Follow-up Comment #13, bug #44565 (project lwip):

Hello Simon,


> And even if I would have preferred to keep the unescaping and
> packet deframing in interrupt context, without having to switch
> into tcpip_thread for every byte (it was valid to call
> pppos_input for each byte),

I didn't remove this feature, you can still do that. This is
what the PPP documentation is actually saying, which the
necessary addendum about the inherent problem when using
PPP_INPROC_MULTITHREADED.

------------8<------------8<------------8<------------
3 PPPoS input path (raw API, thread safe API, TCPIP API)
========================================================

PPPoS requires a serial I/O SIO port (see include/lwip/sio.h).
Received data on serial port should be sent to lwIP using the pppos_input()
or
pppos_input_sys() functions.

If PPP_INPROC_MULTITHREADED is 0 (the default), pppos_input() is not thread
safe
and then *MUST* only be called inside the lwIP context. You should use that
if
you are calling pppos_input() from your main loop context when NO_SYS=1.

If PPP_INPROC_MULTITHREADED is 1, pppos_input() is thread safe and can be
called
from a dedicated RX-thread or from interrupt context… *BUT* you should
NEVER
call pppos_connect(), pppos_listen() and ppp_free() if pppos_input() can
still
be running, doing this is NOT thread safe. You should also avoid calling
pppos_input() if PPPoS session is not started yet.

Using PPP_INPROC_MULTITHREADED is discouraged unless you really know what you
are doing, though it may greatly reduce your need of buffer if pppos_input()
is
called byte after byte in your rx serial interrupt, your move ;-)

Anyway, if you are using an OS (NO_SYS=0) and if PPP_INPROC_MULTITHREADED is
0,
you can use the pppos_input_tcpip() function to pass input data to the lwIP
core thread. This is thread safe in all cases but you should avoid passing
data byte after byte because it uses heavy locking (mailbox) and it allocates
pbuf, better fill them !

/*
 * Fonction to call for received data
 *
 * ppp, PPP control block
 * buffer, input buffer
 * buffer_len, buffer length in bytes
 */
void pppos_input(ppp, buffer, buffer_len);

or

void pppos_input_tcpip(ppp, buffer, buffer_len);
------------8<------------8<------------8<------------

I know it is in my broken english and may actually be hard
to understand even if it's crystal clear for me, I gladly
accept any help on the documentation subject, I am VERY bad
doing that :-)


> I accept that it's possibly easier this way to get it
> thread safe.

Yes, this is just another way for passing raw data PPPoS input
function, this is in fact the only way which is thread safe
in any case, for those who really care about thread safety
while having a very simple code on the user part (I am ! ;P).


> However, when asking how this was previously solved, I was
> rather thinking about how did the old version get into
> tcpip_thread. And looking at the old sources, it used
> 'tcpip_callback', not 'tcpip_input'. Now I'm just curious why
> this has been changed, even if the change interferes with
> other modules.

This is still there. And this is not thread safe.

pppos_input() in the old and new code might be running while
ppp_free_input_packet() is called.


Sylvain

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?44565>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/