subsys/net/ppp: Make NET L2 PPP use net_if properly

Currently, the L2 PPP subsystem is not using the network
interface subsystem appropriately. Here are the issues:

1. net_if_up hidden away internally in net L2 PPP
2. net_if_down not used at all...
3. net_if_carrier_on / off is not used, a workaround is
   used instead, which results in duplicated code
4. L2 PPP does not listen for network events, instead
   it needs the workaround callbacks from drivers.
5. The carrier_on workaround is delegated to a complex
   and broken sys work queue item.

This commit fixes all above issues. net_if_up/down and
net_if_carrier_on/off now work as expected. workaround
for carrier_on/off has been removed.

Signed-off-by: Bjarki Arge Andreasen <baa@trackunit.com>
This commit is contained in:
Bjarki Arge Andreasen 2023-06-10 20:34:41 +02:00 committed by Anas Nashif
parent 29895d8275
commit df88664864
5 changed files with 60 additions and 203 deletions

View file

@ -384,14 +384,6 @@ struct ppp_context {
/** PPP startup worker. */
struct k_work_delayable startup;
/** Carrier ON/OFF handler worker. This is used to create
* network interface UP/DOWN event when PPP L2 driver
* notices carrier ON/OFF situation. We must not create another
* network management event from inside management handler thus
* we use worker thread to trigger the UP/DOWN event.
*/
struct k_work carrier_work;
struct {
/** Finite state machine for LCP */
struct ppp_fsm fsm;
@ -476,6 +468,9 @@ struct ppp_context {
/** Network interface related to this PPP connection */
struct net_if *iface;
/** Network management callback structure */
struct net_mgmt_event_callback mgmt_evt_cb;
/** Current phase of PPP link */
enum ppp_phase phase;
@ -494,12 +489,6 @@ struct ppp_context {
/** Is PPP ready to receive packets */
uint16_t is_ready_to_serve : 1;
/** Is PPP L2 enabled or not */
uint16_t is_enabled : 1;
/** PPP startup pending */
uint16_t is_startup_pending : 1;
/** PPP enable pending */
uint16_t is_enable_done : 1;
@ -522,22 +511,6 @@ struct ppp_context {
uint16_t is_pap_open : 1;
};
/**
* @brief Inform PPP L2 driver that carrier is detected.
* This happens when cable is connected etc.
*
* @param iface Network interface
*/
void net_ppp_carrier_on(struct net_if *iface);
/**
* @brief Inform PPP L2 driver that carrier was lost.
* This happens when cable is disconnected etc.
*
* @param iface Network interface
*/
void net_ppp_carrier_off(struct net_if *iface);
/**
* @brief Initialize PPP L2 stack for a given interface
*

View file

@ -4,21 +4,13 @@
menuconfig NET_L2_PPP
bool "Point-to-point (PPP) support [EXPERIMENTAL]"
select EXPERIMENTAL
select NET_MGMT
select NET_MGMT_EVENT
help
Add support for PPP.
if NET_L2_PPP
config NET_L2_PPP_DELAY_STARTUP_MS
int "PPP delay startup ms"
default 0
help
If the PPP starts too fast, it is possible to delay it
a bit. This is mostly useful in debugging if you want the
device be fully up before PPP handshake is started.
Wait amount of milliseconds before starting PPP. Value 0 disables
the wait.
config NET_L2_PPP_TIMEOUT
int "Maximum timeout in ms for Configure-Req"
default 3000

View file

@ -391,6 +391,10 @@ int ppp_send_pkt(struct ppp_fsm *fsm, struct net_if *iface,
iface = ppp_fsm_iface(fsm);
}
if (!net_if_is_carrier_ok(iface)) {
return -ENETDOWN;
}
if (fsm) {
protocol = fsm->protocol;
}

View file

@ -206,11 +206,6 @@ static void lcp_finished(struct ppp_fsm *fsm)
lcp.fsm);
ppp_link_terminated(ctx);
/* take the remainder down */
ppp_mgmt_raise_carrier_off_event(ctx->iface);
ppp_if_carrier_down(ctx->iface);
}
#if defined(CONFIG_NET_L2_PPP_OPTION_MRU)

View file

@ -195,62 +195,22 @@ static void ppp_close(struct ppp_context *ctx)
{
if (ppp_lcp) {
ppp_lcp->close(ctx, "Shutdown");
} else {
ppp_change_phase(ctx, PPP_DEAD);
}
}
static void ppp_lower_up(struct ppp_context *ctx)
{
if (ppp_lcp) {
ppp_lcp->lower_up(ctx);
}
}
static void start_ppp(struct ppp_context *ctx)
static void ppp_open(struct ppp_context *ctx)
{
ppp_change_phase(ctx, PPP_ESTABLISH);
ppp_lower_up(ctx);
if (ppp_lcp) {
NET_DBG("Starting LCP");
ppp_lcp->lower_up(ctx);
ppp_lcp->open(ctx);
}
}
static int ppp_enable(struct net_if *iface, bool state)
{
const struct ppp_api *ppp =
net_if_get_device(iface)->api;
struct ppp_context *ctx = net_if_l2_data(iface);
if (ctx->is_enabled == state) {
return 0;
}
ctx->is_enabled = state;
if (!state) {
ppp_close(ctx);
if (ppp->stop) {
ppp->stop(net_if_get_device(iface));
}
} else {
if (ppp->start) {
ppp->start(net_if_get_device(iface));
}
if (ctx->is_startup_pending) {
ctx->is_enable_done = true;
} else {
start_ppp(ctx);
}
}
return 0;
}
static enum net_l2_flags ppp_flags(struct net_if *iface)
{
struct ppp_context *ctx = net_if_l2_data(iface);
@ -258,70 +218,7 @@ static enum net_l2_flags ppp_flags(struct net_if *iface)
return ctx->ppp_l2_flags;
}
NET_L2_INIT(PPP_L2, ppp_recv, ppp_send, ppp_enable, ppp_flags);
/* A workaround for PPP L2 not yet supporting net_if_carrier_on/off(). */
void ppp_if_carrier_down(struct net_if *iface)
{
net_if_flag_clear(iface, NET_IF_UP);
net_mgmt_event_notify(NET_EVENT_IF_DOWN, iface);
net_ipv4_autoconf_reset(iface);
}
static void carrier_on_off(struct k_work *work)
{
struct ppp_context *ctx = CONTAINER_OF(work, struct ppp_context,
carrier_work);
bool ppp_carrier_up;
if (ctx->iface == NULL) {
return;
}
ppp_carrier_up = atomic_test_bit(&ctx->flags, PPP_CARRIER_UP);
if (ppp_carrier_up == (bool) ctx->is_net_carrier_up) {
return;
}
ctx->is_net_carrier_up = ppp_carrier_up;
NET_DBG("Carrier %s for interface %p", ppp_carrier_up ? "ON" : "OFF",
ctx->iface);
if (ppp_carrier_up) {
ppp_mgmt_raise_carrier_on_event(ctx->iface);
net_if_up(ctx->iface);
} else {
if (ppp_lcp) {
ppp_lcp->close(ctx, "Shutdown");
/* signaling for the carrier off event is done from the LCP callback */
} else {
ppp_change_phase(ctx, PPP_DEAD);
ppp_mgmt_raise_carrier_off_event(ctx->iface);
ppp_if_carrier_down(ctx->iface);
}
}
}
void net_ppp_carrier_on(struct net_if *iface)
{
struct ppp_context *ctx = net_if_l2_data(iface);
if (!atomic_test_and_set_bit(&ctx->flags, PPP_CARRIER_UP)) {
k_work_submit(&ctx->carrier_work);
}
}
void net_ppp_carrier_off(struct net_if *iface)
{
struct ppp_context *ctx = net_if_l2_data(iface);
if (atomic_test_and_clear_bit(&ctx->flags, PPP_CARRIER_UP)) {
k_work_submit(&ctx->carrier_work);
}
}
NET_L2_INIT(PPP_L2, ppp_recv, ppp_send, NULL, ppp_flags);
#if defined(CONFIG_NET_SHELL)
static int get_ppp_context(int idx, struct ppp_context **ctx,
@ -419,45 +316,6 @@ const struct ppp_protocol_handler *ppp_lcp_get(void)
return ppp_lcp;
}
static void ppp_startup(struct k_work *work)
{
struct k_work_delayable *dwork = k_work_delayable_from_work(work);
struct ppp_context *ctx = CONTAINER_OF(dwork, struct ppp_context,
startup);
int count = 0;
NET_DBG("PPP %p startup for interface %p", ctx, ctx->iface);
STRUCT_SECTION_FOREACH(ppp_protocol_handler, proto) {
if (proto->protocol == PPP_LCP) {
ppp_lcp = proto;
}
proto->init(ctx);
count++;
}
if (count == 0) {
NET_ERR("There are no PPP protocols configured!");
goto bail_out;
}
if (ppp_lcp == NULL) {
NET_ERR("No LCP found!");
goto bail_out;
}
ctx->is_ready_to_serve = true;
bail_out:
ctx->is_startup_pending = false;
if (ctx->is_enable_done) {
start_ppp(ctx);
ctx->is_enable_done = false;
}
}
void ppp_queue_pkt(struct net_pkt *pkt)
{
k_fifo_put(&tx_queue, pkt);
@ -485,9 +343,32 @@ static void tx_handler(void)
}
}
static void net_ppp_mgmt_evt_handler(struct net_mgmt_event_callback *cb, uint32_t mgmt_event,
struct net_if *iface)
{
struct ppp_context *ctx = net_if_l2_data(iface);
if (net_if_is_carrier_ok(iface)) {
ppp_mgmt_raise_carrier_on_event(iface);
} else {
ppp_mgmt_raise_carrier_off_event(iface);
}
if (mgmt_event == NET_EVENT_IF_UP) {
ppp_open(ctx);
return;
}
if (mgmt_event == NET_EVENT_IF_DOWN) {
ppp_close(ctx);
return;
}
}
void net_ppp_init(struct net_if *iface)
{
struct ppp_context *ctx = net_if_l2_data(iface);
uint8_t count = 0;
NET_DBG("Initializing PPP L2 %p for iface %p", ctx, iface);
@ -496,21 +377,33 @@ void net_ppp_init(struct net_if *iface)
ctx->ppp_l2_flags = NET_L2_MULTICAST | NET_L2_POINT_TO_POINT;
ctx->iface = iface;
k_work_init(&ctx->carrier_work, carrier_on_off);
#if defined(CONFIG_NET_SHELL)
k_sem_init(&ctx->shell.wait_echo_reply, 0, K_SEM_MAX_LIMIT);
#endif
/* TODO: Unify the startup worker code so that we can save
* some memory if there are more than one PPP context in the
* system. The issue is not very likely as typically there
* would be only one PPP network interface in the system.
*/
k_work_init_delayable(&ctx->startup, ppp_startup);
net_mgmt_init_event_callback(&ctx->mgmt_evt_cb, net_ppp_mgmt_evt_handler,
(NET_EVENT_IF_UP | NET_EVENT_IF_DOWN));
ctx->is_startup_pending = true;
net_mgmt_add_event_callback(&ctx->mgmt_evt_cb);
k_work_reschedule(&ctx->startup,
K_MSEC(CONFIG_NET_L2_PPP_DELAY_STARTUP_MS));
STRUCT_SECTION_FOREACH(ppp_protocol_handler, proto) {
if (proto->protocol == PPP_LCP) {
ppp_lcp = proto;
}
proto->init(ctx);
count++;
}
if (count == 0) {
NET_ERR("There are no PPP protocols configured!");
return;
}
if (ppp_lcp == NULL) {
NET_ERR("No LCP found!");
return;
}
ctx->is_ready_to_serve = true;
}