From b55412a0c447d24adaae9c929e5098b381adf177 Mon Sep 17 00:00:00 2001 From: Sylvain Rochet Date: Sun, 13 Sep 2015 17:53:16 +0200 Subject: [PATCH] PPP, PPPoS, replaced static sio_write() calls to a user defined callback The overall lwIP design on data flows (netif,udp,tcp) is to use a user defined callback to get data from stack and a static function to send data to stack, which makes perfect sense. The SIO port was an exception, the PPP stack never really used the SIO port by only using the sio_send() function (and the ignominious sio_read_abort() function a while back). The way the SIO port is currently designed adds a tight coupling between the lwIP port and the user code if the user need to do specific user code if the current uart used is the PPPoS uart, which is not nice, especially because all the lwIP stack is quite clean at this subject. While we are at stabilizing the PPP API, change this behavior before it's too late by replacing the static sio_write() calls to a user defined callback. --- doc/ppp.txt | 22 ++++++++++++++++++---- src/api/pppapi.c | 8 ++++---- src/include/lwip/pppapi.h | 7 +++++-- src/include/netif/ppp/pppos.h | 7 +++++-- src/netif/ppp/pppos.c | 28 +++++++--------------------- 5 files changed, 39 insertions(+), 33 deletions(-) diff --git a/doc/ppp.txt b/doc/ppp.txt index 68da58e3..8b0acdf0 100644 --- a/doc/ppp.txt +++ b/doc/ppp.txt @@ -183,17 +183,30 @@ static void status_cb(ppp_pcb *pcb, int err_code, void *ctx) { #include "netif/ppp/pppos.h" +/* + * PPPoS serial output callback + * + * ppp_pcb, PPP control block + * data, buffer to write to serial port + * len, length of the data buffer + * ctx, optional user-provided callback context pointer + * + * Return value: len if write succeed + */ +static u32_t output_cb(ppp_pcb *pcb, u8_t *data, u32_t len, void *ctx) { + return uart_write(UART, data, len); +} + /* * Create a new PPPoS interface * * ppp_netif, netif to use for this PPP link, i.e. PPP IP interface - * ppp_sio, handler to your SIO port (see include/lwip/sio.h) + * output_cb, PPPoS serial output callback * status_cb, PPP status callback, called on PPP status change (up, down, …) * ctx_cb, optional user-provided callback context pointer */ ppp = pppos_create(&ppp_netif, - ppp_sio, - status_cb, ctx_cb); + output_cb, status_cb, ctx_cb); /* @@ -304,7 +317,6 @@ ppp_free(ppp); 3 PPPoS input path (raw API, IRQ 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() function or the pppos_input_sys() function. @@ -405,3 +417,5 @@ from previous lwIP version is pretty easy: unconditionally registering them, which is probably the wanted behavior in almost all cases. If you need it conditional contact us and we will made it conditional. + +* PPPoS now requires a serial output callback diff --git a/src/api/pppapi.c b/src/api/pppapi.c index 5b7940d1..e1122d77 100644 --- a/src/api/pppapi.c +++ b/src/api/pppapi.c @@ -127,7 +127,7 @@ pppapi_set_notify_phase_callback(ppp_pcb *pcb, ppp_notify_phase_cb_fn notify_pha static void pppapi_do_pppos_create(struct pppapi_msg_msg *msg) { - msg->ppp = pppos_create(msg->msg.serialcreate.pppif, msg->msg.serialcreate.fd, + msg->ppp = pppos_create(msg->msg.serialcreate.pppif, msg->msg.serialcreate.output_cb, msg->msg.serialcreate.link_status_cb, msg->msg.serialcreate.ctx_cb); TCPIP_PPPAPI_ACK(msg); } @@ -137,13 +137,13 @@ pppapi_do_pppos_create(struct pppapi_msg_msg *msg) * tcpip_thread context. */ ppp_pcb* -pppapi_pppos_create(struct netif *pppif, sio_fd_t fd, ppp_link_status_cb_fn link_status_cb, - void *ctx_cb) +pppapi_pppos_create(struct netif *pppif, pppos_output_cb_fn output_cb, + ppp_link_status_cb_fn link_status_cb, void *ctx_cb) { struct pppapi_msg msg; msg.function = pppapi_do_pppos_create; msg.msg.msg.serialcreate.pppif = pppif; - msg.msg.msg.serialcreate.fd = fd; + msg.msg.msg.serialcreate.output_cb = output_cb; msg.msg.msg.serialcreate.link_status_cb = link_status_cb; msg.msg.msg.serialcreate.ctx_cb = ctx_cb; TCPIP_PPPAPI(&msg); diff --git a/src/include/lwip/pppapi.h b/src/include/lwip/pppapi.h index 270c5fc7..28c8a5fd 100644 --- a/src/include/lwip/pppapi.h +++ b/src/include/lwip/pppapi.h @@ -35,6 +35,9 @@ #include "lwip/sys.h" #include "lwip/netif.h" #include "netif/ppp/ppp.h" +#if PPPOS_SUPPORT +#include "netif/ppp/pppos.h" +#endif /* PPPOS_SUPPORT */ #ifdef __cplusplus extern "C" { @@ -60,7 +63,7 @@ struct pppapi_msg_msg { #if PPPOS_SUPPORT struct { struct netif *pppif; - sio_fd_t fd; + pppos_output_cb_fn output_cb; ppp_link_status_cb_fn link_status_cb; void *ctx_cb; } serialcreate; @@ -119,7 +122,7 @@ void pppapi_set_auth(ppp_pcb *pcb, u8_t authtype, const char *user, const char * void pppapi_set_notify_phase_callback(ppp_pcb *pcb, ppp_notify_phase_cb_fn notify_phase_cb); #endif /* PPP_NOTIFY_PHASE */ #if PPPOS_SUPPORT -ppp_pcb *pppapi_pppos_create(struct netif *pppif, sio_fd_t fd, ppp_link_status_cb_fn link_status_cb, void *ctx_cb); +ppp_pcb *pppapi_pppos_create(struct netif *pppif, pppos_output_cb_fn output_cb, ppp_link_status_cb_fn link_status_cb, void *ctx_cb); #endif /* PPPOS_SUPPORT */ #if PPPOE_SUPPORT ppp_pcb *pppapi_pppoe_create(struct netif *pppif, struct netif *ethif, const char *service_name, diff --git a/src/include/netif/ppp/pppos.h b/src/include/netif/ppp/pppos.h index e1721395..39b2b7d0 100644 --- a/src/include/netif/ppp/pppos.h +++ b/src/include/netif/ppp/pppos.h @@ -55,6 +55,9 @@ enum { PDDATA /* Process data byte. */ }; +/* PPPoS serial output callback function prototype */ +typedef u32_t (*pppos_output_cb_fn)(ppp_pcb *pcb, u8_t *data, u32_t len, void *ctx); + /* * Extended asyncmap - allows any character to be escaped. */ @@ -67,7 +70,7 @@ typedef struct pppos_pcb_s pppos_pcb; struct pppos_pcb_s { /* -- below are data that will NOT be cleared between two sessions */ ppp_pcb *ppp; /* PPP PCB */ - sio_fd_t fd; /* File device ID of port. */ + pppos_output_cb_fn output_cb; /* PPP serial output callback */ /* -- below are data that will be cleared between two sessions * @@ -92,7 +95,7 @@ struct pppos_pcb_s { }; /* Create a new PPPoS session. */ -ppp_pcb *pppos_create(struct netif *pppif, sio_fd_t fd, +ppp_pcb *pppos_create(struct netif *pppif, pppos_output_cb_fn output_cb, ppp_link_status_cb_fn link_status_cb, void *ctx_cb); #if !NO_SYS && !PPP_INPROC_IRQ_SAFE diff --git a/src/netif/ppp/pppos.c b/src/netif/ppp/pppos.c index 07a2dade..4e78e5f9 100644 --- a/src/netif/ppp/pppos.c +++ b/src/netif/ppp/pppos.c @@ -44,7 +44,6 @@ #include "lwip/snmp_mib2.h" #include "lwip/tcpip.h" #include "lwip/api.h" -#include "lwip/sio.h" #include "lwip/ip4.h" /* for ip4_input() */ #include "netif/ppp/ppp_impl.h" @@ -170,7 +169,7 @@ ppp_get_fcs(u8_t byte) * * Return 0 on success, an error code on failure. */ -ppp_pcb *pppos_create(struct netif *pppif, sio_fd_t fd, +ppp_pcb *pppos_create(struct netif *pppif, pppos_output_cb_fn output_cb, ppp_link_status_cb_fn link_status_cb, void *ctx_cb) { pppos_pcb *pppos; @@ -188,7 +187,7 @@ ppp_pcb *pppos_create(struct netif *pppif, sio_fd_t fd, } pppos->ppp = ppp; - pppos->fd = fd; + pppos->output_cb = output_cb; ppp_link_set_callbacks(ppp, &pppos_callbacks, pppos); return ppp; } @@ -786,21 +785,10 @@ pppos_recv_config(ppp_pcb *ppp, void *ctx, u32_t accm, int pcomp, int accomp) static err_t pppos_ioctl(ppp_pcb *pcb, void *ctx, int cmd, void *arg) { - pppos_pcb *pppos = (pppos_pcb *)ctx; LWIP_UNUSED_ARG(pcb); - - switch(cmd) { - case PPPCTLG_FD: /* Get the fd associated with the ppp */ - if (!arg) { - goto fail; - } - *(sio_fd_t *)arg = pppos->fd; - return ERR_OK; - - default: ; - } - -fail: + LWIP_UNUSED_ARG(ctx); + LWIP_UNUSED_ARG(cmd); + LWIP_UNUSED_ARG(arg); return ERR_VAL; } @@ -858,7 +846,7 @@ pppos_output_append(pppos_pcb *pppos, err_t err, struct pbuf *nb, u8_t c, u8_t a * Sure we don't quite fill the buffer if the character doesn't * get escaped but is one character worth complicating this? */ if ((PBUF_POOL_BUFSIZE - nb->len) < 2) { - u32_t l = sio_write(pppos->fd, (u8_t*)nb->payload, nb->len); + u32_t l = pppos->output_cb(pppos->ppp, (u8_t*)nb->payload, nb->len, pppos->ppp->ctx_cb); if (l != nb->len) { return ERR_IF; } @@ -884,9 +872,7 @@ pppos_output_append(pppos_pcb *pppos, err_t err, struct pbuf *nb, u8_t c, u8_t a static err_t pppos_output_last(pppos_pcb *pppos, err_t err, struct pbuf *nb, u16_t *fcs) { -#if MIB2_STATS ppp_pcb *ppp = pppos->ppp; -#endif /* MIB2_STATS */ /* Add FCS and trailing flag. */ err = pppos_output_append(pppos, err, nb, ~(*fcs) & 0xFF, 1, NULL); @@ -899,7 +885,7 @@ pppos_output_last(pppos_pcb *pppos, err_t err, struct pbuf *nb, u16_t *fcs) /* Send remaining buffer if not empty */ if (nb->len > 0) { - u32_t l = sio_write(pppos->fd, (u8_t*)nb->payload, nb->len); + u32_t l = pppos->output_cb(ppp, (u8_t*)nb->payload, nb->len, ppp->ctx_cb); if (l != nb->len) { err = ERR_IF; goto failed;