Closed ljdevdev closed 1 week ago
One thing that strucks me is that the driver shouldn't really take control over the network interface admin state (I. .e it should not use net_if_up()
/net_if_down()
), but rather control CARRIER/DORMANT flags (net_if_carrier/dormant_on/off()
), see https://docs.zephyrproject.org/latest/connectivity/networking/api/net_if.html#network-interface-state-management.
Nonetheless, I'm not sure if it's correct that on net_if_down()
we skip LCP state transitions if PPP is in DEAD phase:
https://github.com/zephyrproject-rtos/zephyr/blob/main/subsys/net/l2/ppp/ppp_l2.c#L258
That's why LCP ends up stuck in STOPPED state.
@bjarki-andreasen @tomi-font You're more experienced with PPP interfaces, what's your view on this?
It seems you are cutting the carrier right after calling net_if_down()
which can cause the network interface to get stuck, as it tries to terminate the link properly. Your driver should be calling net_if_carrier_off()
when the data connection is closed, which immediately resets the internal state machine as attempting to close down properly is not possible :) net_if_up()
should be used by the application to signal that the interface should be up once it is possible, which is after your modem has registered to a network and the PPP link is up, at which point your driver should call net_if_carrier_on()
.
Thank you very much for your swift reply :) I'll check that that's working and then close the issue.
It seems I was a bit hasty with closing the issue. @bjarki-andreasen , I tried what you suggested (actually multiple combinations, since I wasn't exactly sure about the necessary order and timing between the calls).
My current setup is as follows:
net_if_up()
(otherwise the uart callback is not set and no PPP responses are received by the driver)net_if_carrier_on()
net_if_carrier_off()
This produces the exact same behavior as before (see log below)
Now I dug a bit into the code and found that net_if_carrier_on()
, as well as the functions for setting up the PPP uart are called from inside net_if_up
:
From ppp_start
(https://github.com/zephyrproject-rtos/zephyr/blob/main/drivers/net/ppp.c#L1098)
ppp_async_uart_rx_enable(context);
#else
uart_irq_rx_disable(context->dev);
uart_irq_tx_disable(context->dev);
ppp_uart_flush(context->dev);
uart_irq_callback_user_data_set(context->dev, ppp_uart_isr,
context);
uart_irq_rx_enable(context->dev);
#endif
}
#endif /* !CONFIG_NET_TEST */
net_if_carrier_on(context->iface);
return 0;
This is called in ppp_up
(https://github.com/zephyrproject-rtos/zephyr/blob/main/subsys/net/l2/ppp/ppp_l2.c#L211) which is called in ppp_enable
(https://github.com/zephyrproject-rtos/zephyr/blob/main/subsys/net/l2/ppp/ppp_l2.c#L293) which is called in net_if_up
.
So it is clear, that net_if_up
must be called after the mobile link was set up and an additional net_if_carrier_on
call seems redundant (in fact it works fine without an additional call to that function). This may not be the canonic way, but it seems to be the way the driver is built.
As for the call to net_if_carrier_off()
, it does not change the state of the LCP fsm and therefore I run into the same problem again.
[00:05:20.994,000]
[00:05:34.808,000] modem DEBUG: CEREG Command failed.
[00:05:35.819,000] modem DEBUG: --> AT+CEREG?
[00:05:35.838,000] modem DEBUG: <-- AT+CEREG? /+CEREG: 0,0 / /OK /
[00:05:36.878,000]
@ljdevdev This is a larger issue, where I am very much in opposition to the way ppp.c
works, IMO misusing the carrier... See
The modem subsystem, with modem_cellular.c
uses the interface and carrier appropriately, you may want to look into using it, or at least adapting your driver to behave similarly :)
@bjarki-andreasen I am fairly certain, that all of this has nothing to do with the way I set up the PPP conection (carrier vs net_if vs modem_cellular.c etc).
If I look at the actual LCP communication happening here I get the following:
The modem sends a terminate request (05)
[00:05:18.371,000] <dbg> net_l2_ppp: net_pkt_hexdump: 0x2027400c
c0 21 05 03 00 20 4e 6f 20 6e 65 74 77 6f 72 6b |.!... No network
20 70 72 6f 74 6f 63 6f 6c 73 20 72 75 6e 6e 69 | protoco ls runni
6e 67 |ng
[00:05:18.371,000] <dbg> net_l2_ppp: ppp_fsm_input: (rx_q[0]): [LCP/0x20207350] LCP Terminate-Req (5) id 3 payload len 28
[00:05:18.372,000] <dbg> net_l2_ppp: fsm_recv_terminate_req: (rx_q[0]): [LCP/0x20207350] Current state OPENED (9)
[00:05:18.372,000] <dbg> net_l2_ppp: fsm_recv_terminate_req: (rx_q[0]): [LCP/0x20207350] Terminated by peer (No network protocols running)
[00:05:18.372,000] <dbg> net_l2_ppp: ppp_change_state_debug: (rx_q[0]): [LCP/0x20207350] state OPENED (9) => STOPPING (5) (fsm_recv_terminate_req():877)
The ppp driver sends a terminate ack (06):
[00:05:18.373,000] <dbg> net_l2_ppp: net_pkt_hexdump: send L2
[00:05:18.373,000] <dbg> net_l2_ppp: net_pkt_hexdump: 0x2027345c
c0 21 06 03 00 04 |.!....
The lcp fsm then runs into a timeout, which puts it into mode STOPPED
[00:05:23.372,000] <dbg> net_l2_ppp: ppp_fsm_timeout: (sysworkq): [LCP/0x20207350] Current state STOPPING (5)
[00:05:23.373,000] <dbg> net_l2_ppp: ppp_change_state_debug: (sysworkq): [LCP/0x20207350] state STOPPING (5) => STOPPED (3) (ppp_fsm_timeout():136)
Now if I call net_if_carrier_on()
in this state, that may or may not be fine. This is what modem_cellular.c does, and if that works, it is because it uses cmux (I think). I don't use cmux (yet) and so I need to pass control over the UART back and forth.
In order to do that I can either call net_if_up()
and net_if_down()
, or ppp_start()
and ppp_stop()
. So I resorted to calling net_if_down()
after link termination and then net_if_up()
again. And while that is something a driver should not do, that seems to be something, that an application might want to do.
So if an application asynchronously calls net_if_down()
between a terminate request and reestablishment of the link, I think the driver would still freeze.
For the time being I found an ugly hack, to get this working for my driver without dragging a manual patch of zephyr along.
In the long run I think it would be great to have a PPP driver in zephyr that is usable with and without cmux and with and without any given cellular driver.
If there is any interest in pursuing this further I am happy to help, otherwise I think we can close this.
Thanks @ljdevdev closing this as you suggest.
I have a curious observation, and I am not quite sure, if I am using the driver wrong, or I encountered a bug. The LCP-statemachine gets stuck after calling
net_if_down
on the PPP interface. Do I need to reset the driver somehow?Obeserved behaviour I am running a custom AT-Command modem driver that hands over to the Zephyr-PPP driver after successfully establishing a mobile link.
When the modem terminates the PPP link (see log below) the PPP-driver switches to phase
PPP_DEAD
and the LCP statemachine switches toPPP_STOPPED
The AT-Command driver then takes over, calls
net_if_down
, reestablishes the mobile connection and callsnet_if_up
on the ppp-interface.The LCP-Statemachine then gets stuck in state
PPP_STOPPED
and never establishes a new link.Solution The problem goes away if I reset
lcp.fsm.state
toPPP_INITIAL
inppp_link_terminated
(https://github.com/zephyrproject-rtos/zephyr/blob/main/subsys/net/l2/ppp/link.c#L115):I am unsure though, if this is the proper place or way to reset the LCP statemachine.
To Reproduce Unfortuantely I have no easy way to reproduce this without a modem that terminates the connection. If someone points me to a testsuite where I could add this as a test, I would be happy to do so.
Expected behavior The statemachine should start from the same initial state each time ppp_start is called.
Impact This bug makes the PPP driver unusable in a commercial product
Logs and console output
Environment (please complete the following information):