| Message ID | 1519652095-8588-1-git-send-email-sami.hartikainen@teleste.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | mongoose: Allow WEB API v1 and v2 both be enabled | expand |
Hi Sami, Am 26.02.2018 um 14:34 schrieb Sami Hartikainen: > To ease transition, or simply to enable the use of modern clients > while still supporting the use of the swuforwarder handler (which > only talks API v1), both the v1 and v2 API may be enabled. > > When both API versions are enabled, the v2 message broadcast thread > consumes the status messages from the installer. A message queue is > set up to make those messages available for the v1 message polling > handler for URI /getstatus.json. > > Signed-off-by: Sami Hartikainen <sami.hartikainen@teleste.com> > --- > mongoose/Config.in | 15 +-- > mongoose/mongoose_interface.c | 246 +++++++++++++++++++++++++++++++----------- > 2 files changed, 194 insertions(+), 67 deletions(-) > > diff --git a/mongoose/Config.in b/mongoose/Config.in > index a001247..cd52f2c 100644 > --- a/mongoose/Config.in > +++ b/mongoose/Config.in > @@ -14,27 +14,28 @@ choice > config MONGOOSE > bool "mongoose" > help > - Mongoose embeddded web server > + Mongoose embedded web server > > endchoice > > -choice > - prompt "Web Application Interface" > - default MONGOOSE_WEB_API_V1 > - help > - Choose the bootloader > +menu "Web Application Interface" > > config MONGOOSE_WEB_API_V1 > bool "Version 1 (deprecated)" > + default y > help > Support for version 1 > > config MONGOOSE_WEB_API_V2 > bool "Version 2" > + default n Maybe we should also enable V2 by default. > help > Support for version 2 > > -endchoice > +comment "Web API v1 or v2 or both required" > + depends on !MONGOOSE_WEB_API_V1 && !MONGOOSE_WEB_API_V2 > + > +endmenu > > config MONGOOSEIPV6 > bool "IPv6 support" > diff --git a/mongoose/mongoose_interface.c b/mongoose/mongoose_interface.c > index 89a51f3..0401a9a 100644 > --- a/mongoose/mongoose_interface.c > +++ b/mongoose/mongoose_interface.c > @@ -25,6 +25,7 @@ > #include <parselib.h> > #include <progress_ipc.h> > #include <swupdate_settings.h> > +#include <bsdqueue.h> > > #include "mongoose.h" > > @@ -32,6 +33,9 @@ > #define MG_PORT "8080" > #define MG_ROOT "." > > +#define WEB_API_v1 1 > +#define WEB_API_v2 2 > + > struct mongoose_options { > char *root; > char *listing; > @@ -49,6 +53,25 @@ struct file_upload_state { > > static struct mg_serve_http_opts s_http_server_opts; > > +#if !defined(CONFIG_MONGOOSE_WEB_API_V1) && !defined(CONFIG_MONGOOSE_WEB_API_V2) > +#error "WEB API v1 or v2 or both must be defined" > +#endif Maybe we should fall back to V1 to support older .config files. > + > +#if defined(CONFIG_MONGOOSE_WEB_API_V1) && defined(CONFIG_MONGOOSE_WEB_API_V2) > +#define NUM_CACHED_MESSAGES 100 > + > +struct msg_elem { > + ipc_message msg; > + SIMPLEQ_ENTRY(msg_elem) next; > +}; > + > +SIMPLEQ_HEAD(msglist, msg_elem); > +static struct msglist ipcmessages; > +static unsigned long nrmsgs = 0; > +static pthread_mutex_t msglock = PTHREAD_MUTEX_INITIALIZER; > +static pthread_cond_t msgcond = PTHREAD_COND_INITIALIZER; > +#endif > + > #if defined(CONFIG_MONGOOSE_WEB_API_V2) > #define enum_string(x) [x] = #x > static const char *get_status_string(unsigned int status) > @@ -112,20 +135,163 @@ static size_t snescape(char *dst, size_t n, const char *src) > } > #endif > > -static void upload_handler(struct mg_connection *nc, int ev, void *p) > +static int get_inst_status(ipc_message *msg, int __attribute__ ((__unused__)) api_vers) > +{ > +#if defined(CONFIG_MONGOOSE_WEB_API_V1) && defined(CONFIG_MONGOOSE_WEB_API_V2) > + struct msg_elem *newmsg; > + struct msg_elem *oldmsg; > + int ret = -1; > + > + switch (api_vers) { > + case WEB_API_v1: > + pthread_mutex_lock(&msglock); > + pthread_cond_wait(&msgcond, &msglock); > + oldmsg = SIMPLEQ_FIRST(&ipcmessages); > + if (oldmsg) { > + SIMPLEQ_REMOVE_HEAD(&ipcmessages, next); > + nrmsgs--; > + memcpy(msg, &oldmsg->msg, sizeof(*msg)); > + free(oldmsg); > + ret = 0; > + } > + pthread_mutex_unlock(&msglock); > + break; > + > + case WEB_API_v2: > + newmsg = (struct msg_elem *)calloc(1, sizeof(*newmsg)); > + if (!newmsg) > + return -1; > + > + ret = ipc_get_status(msg); > + > + /* Cache message for APIv1, but consider > + * changed messages only. */ > + if (!ret) { > + pthread_mutex_lock(&msglock); > + > + oldmsg = SIMPLEQ_LAST(&ipcmessages, msg_elem, next); > + if (!oldmsg || memcmp(&oldmsg->msg.data.status, > + &msg->data.status, > + sizeof(msg->data.status))) { > + > + nrmsgs++; > + if (nrmsgs > NUM_CACHED_MESSAGES) { > + oldmsg = SIMPLEQ_FIRST(&ipcmessages); > + SIMPLEQ_REMOVE_HEAD(&ipcmessages, next); > + free(oldmsg); > + nrmsgs--; > + } > + > + memcpy(&newmsg->msg, msg, sizeof(*msg)); > + SIMPLEQ_INSERT_TAIL(&ipcmessages, newmsg, next); > + newmsg = NULL; > + } > + > + pthread_cond_signal(&msgcond); > + pthread_mutex_unlock(&msglock); > + } > + > + if (newmsg) > + free(newmsg); > + break; > + } > + > + return ret; > +#else > + return ipc_get_status(msg); > +#endif > +} Why you bundle two functions into one function with a switch case? > + > +static void cleanup_msg_list(void) > +{ > +#if defined(CONFIG_MONGOOSE_WEB_API_V1) && defined(CONFIG_MONGOOSE_WEB_API_V2) > + struct msg_elem *elem; > + > + pthread_mutex_lock(&msglock); > + > + while (!SIMPLEQ_EMPTY(&ipcmessages)) { > + elem = SIMPLEQ_FIRST(&ipcmessages); > + SIMPLEQ_REMOVE_HEAD(&ipcmessages, next); > + free(elem); > + } > + > + nrmsgs = 0; > + pthread_mutex_unlock(&msglock); > +#endif > +} > + > +static void upload_handler_v2(struct mg_connection *nc, int ev, void *p) > { > struct mg_http_multipart_part *mp; > struct file_upload_state *fus; > + > + switch (ev) { > + case MG_EV_HTTP_PART_BEGIN: > + mp = (struct mg_http_multipart_part *) p; > + > + fus = (struct file_upload_state *) calloc(1, sizeof(*fus)); > + if (fus == NULL) { > + mg_http_send_error(nc, 500, "Out of memory"); > + break; > + } > + > + cleanup_msg_list(); > + > + fus->fd = ipc_inst_start_ext(SOURCE_WEBSERVER, strlen(mp->file_name), mp->file_name); > + if (fus->fd < 0) { > + mg_http_send_error(nc, 500, "Failed to queue command"); > + free(fus); > + break; > + } > + > + mp->user_data = fus; > + > + break; > + > + case MG_EV_HTTP_PART_DATA: > + mp = (struct mg_http_multipart_part *) p; > + fus = (struct file_upload_state *) mp->user_data; > + > + if (!fus) > + break; > + > + ipc_send_data(fus->fd, (char *) mp->data.p, mp->data.len); > + fus->len += mp->data.len; > + > + break; > + > + case MG_EV_HTTP_PART_END: > + mp = (struct mg_http_multipart_part *) p; > + fus = (struct file_upload_state *) mp->user_data; > + > + if (!fus) > + break; > + > + ipc_end(fus->fd); > + > + mg_send_response_line(nc, 200, > + "Content-Type: text/plain\r\n" > + "Connection: close"); > + mg_send(nc, "\r\n", 2); > + mg_printf(nc, "Ok, %s - %d bytes.\r\n", mp->file_name, (int) fus->len); > + nc->flags |= MG_F_SEND_AND_CLOSE; > + > + mp->user_data = NULL; > + free(fus); > + break; > + } > +} > + > #if defined(CONFIG_MONGOOSE_WEB_API_V1) > +static void upload_handler_v1(struct mg_connection *nc, int ev, void *p) > +{ > struct mg_str *filename, *data; > struct http_message *hm; > size_t length; > char buf[16]; > int fd; > -#endif > > switch (ev) { > -#if defined(CONFIG_MONGOOSE_WEB_API_V1) > case MG_EV_HTTP_REQUEST: > hm = (struct http_message *) p; > > @@ -149,6 +315,8 @@ static void upload_handler(struct mg_connection *nc, int ev, void *p) > return; > } > > + cleanup_msg_list(); > + > fd = ipc_inst_start_ext(SOURCE_WEBSERVER, filename->len, filename->p); > ipc_send_data(fd, (char *) hm->body.p, hm->body.len); > ipc_end(fd); > @@ -161,62 +329,13 @@ static void upload_handler(struct mg_connection *nc, int ev, void *p) > nc->flags |= MG_F_SEND_AND_CLOSE; > > break; > -#endif > - case MG_EV_HTTP_PART_BEGIN: > - mp = (struct mg_http_multipart_part *) p; > > - fus = (struct file_upload_state *) calloc(1, sizeof(*fus)); > - if (fus == NULL) { > - mg_http_send_error(nc, 500, "Out of memory"); > - break; > - } > - > - fus->fd = ipc_inst_start_ext(SOURCE_WEBSERVER, strlen(mp->file_name), mp->file_name); > - if (fus->fd < 0) { > - mg_http_send_error(nc, 500, "Failed to queue command"); > - free(fus); > - break; > - } > - > - mp->user_data = fus; > - > - break; > - > - case MG_EV_HTTP_PART_DATA: > - mp = (struct mg_http_multipart_part *) p; > - fus = (struct file_upload_state *) mp->user_data; > - > - if (!fus) > - break; > - > - ipc_send_data(fus->fd, (char *) mp->data.p, mp->data.len); > - fus->len += mp->data.len; > - > - break; > - > - case MG_EV_HTTP_PART_END: > - mp = (struct mg_http_multipart_part *) p; > - fus = (struct file_upload_state *) mp->user_data; > - > - if (!fus) > - break; > - > - ipc_end(fus->fd); > - > - mg_send_response_line(nc, 200, > - "Content-Type: text/plain\r\n" > - "Connection: close"); > - mg_send(nc, "\r\n", 2); > - mg_printf(nc, "Ok, %s - %d bytes.\r\n", mp->file_name, (int) fus->len); > - nc->flags |= MG_F_SEND_AND_CLOSE; > - > - mp->user_data = NULL; > - free(fus); > + default: > + upload_handler_v2(nc, ev, p); > break; > } > } > > -#if defined(CONFIG_MONGOOSE_WEB_API_V1) > static void recovery_status(struct mg_connection *nc, int ev, void *ev_data) > { > ipc_message ipc; > @@ -226,7 +345,7 @@ static void recovery_status(struct mg_connection *nc, int ev, void *ev_data) > (void)ev; > (void)ev_data; > > - ret = ipc_get_status(&ipc); > + ret = get_inst_status(&ipc, WEB_API_v1); > > if (ret) { > mg_http_send_error(nc, 500, NULL); > @@ -306,7 +425,9 @@ static void post_update_cmd(struct mg_connection *nc, int ev, void *ev_data) > > nc->flags |= MG_F_SEND_AND_CLOSE; > } > -#elif defined(CONFIG_MONGOOSE_WEB_API_V2) > +#endif > + > +#if defined(CONFIG_MONGOOSE_WEB_API_V2) > static void restart_handler(struct mg_connection *nc, int ev, void *ev_data) > { > struct http_message *hm = (struct http_message *) ev_data; > @@ -350,7 +471,7 @@ static void *broadcast_message_thread(void *data) > { > for (;;) { > ipc_message msg; > - int ret = ipc_get_status(&msg); > + int ret = get_inst_status(&msg, WEB_API_v2); > > if (!ret && strlen(msg.data.status.desc) != 0) { > struct mg_mgr *mgr = (struct mg_mgr *) data; > @@ -610,15 +731,20 @@ int start_mongoose(const char *cfgfname, int argc, char *argv[]) > exit(EXIT_FAILURE); > } > > +#if defined(CONFIG_MONGOOSE_WEB_API_V1) && defined(CONFIG_MONGOOSE_WEB_API_V2) > + SIMPLEQ_INIT(&ipcmessages); > +#endif > + > #if defined(CONFIG_MONGOOSE_WEB_API_V1) > - mg_register_http_endpoint(nc, "/handle_post_request", MG_CB(upload_handler, NULL)); > + mg_register_http_endpoint(nc, "/handle_post_request", MG_CB(upload_handler_v1, NULL)); > mg_register_http_endpoint(nc, "/getstatus.json", MG_CB(recovery_status, NULL)); > mg_register_http_endpoint(nc, "/rebootTarget", MG_CB(reboot_target, NULL)); > mg_register_http_endpoint(nc, "/postUpdateCommand", MG_CB(post_update_cmd, NULL)); > -#elif defined(CONFIG_MONGOOSE_WEB_API_V2) > +#endif > +#if defined(CONFIG_MONGOOSE_WEB_API_V2) > mg_register_http_endpoint(nc, "/restart", restart_handler); > #endif > - mg_register_http_endpoint(nc, "/upload", MG_CB(upload_handler, NULL)); > + mg_register_http_endpoint(nc, "/upload", MG_CB(upload_handler_v2, NULL)); Could you please move the upload_handler_v2 into to V2 guards. > mg_set_protocol_http_websocket(nc); > > #if defined(CONFIG_MONGOOSE_WEB_API_V2) The code use many ifdefs, maybe it is wise to split the code into three files. Best regards Stefan
Hi Stefan, > > To ease transition, or simply to enable the use of modern clients > > while still supporting the use of the swuforwarder handler (which only > > talks API v1), both the v1 and v2 API may be enabled. > > > > When both API versions are enabled, the v2 message broadcast thread > > consumes the status messages from the installer. A message queue is > > set up to make those messages available for the v1 message polling > > handler for URI /getstatus.json. > > > > Signed-off-by: Sami Hartikainen <sami.hartikainen@teleste.com> > > --- > > mongoose/Config.in | 15 +-- > > mongoose/mongoose_interface.c | 246 > +++++++++++++++++++++++++++++++----------- > > 2 files changed, 194 insertions(+), 67 deletions(-) > > > > diff --git a/mongoose/Config.in b/mongoose/Config.in index > > a001247..cd52f2c 100644 > > --- a/mongoose/Config.in > > +++ b/mongoose/Config.in > > @@ -14,27 +14,28 @@ choice > > config MONGOOSE > > bool "mongoose" > > help > > - Mongoose embeddded web server > > + Mongoose embedded web server > > > > endchoice > > > > -choice > > - prompt "Web Application Interface" > > - default MONGOOSE_WEB_API_V1 > > - help > > - Choose the bootloader > > +menu "Web Application Interface" > > > > config MONGOOSE_WEB_API_V1 > > bool "Version 1 (deprecated)" > > + default y > > help > > Support for version 1 > > > > config MONGOOSE_WEB_API_V2 > > bool "Version 2" > > + default n > > Maybe we should also enable V2 by default. Enabling both adds binary footprint size and code complexity, so I chose to be cautious with this. On the other hand, describing V1 as "deprecated" implies it should not be the only one by default enabled. > > help > > Support for version 2 > > > > -endchoice > > +comment "Web API v1 or v2 or both required" > > + depends on !MONGOOSE_WEB_API_V1 && > !MONGOOSE_WEB_API_V2 > > + > > +endmenu > > > > config MONGOOSEIPV6 > > bool "IPv6 support" > > diff --git a/mongoose/mongoose_interface.c > > b/mongoose/mongoose_interface.c index 89a51f3..0401a9a 100644 > > --- a/mongoose/mongoose_interface.c > > +++ b/mongoose/mongoose_interface.c > > @@ -25,6 +25,7 @@ > > #include <parselib.h> > > #include <progress_ipc.h> > > #include <swupdate_settings.h> > > +#include <bsdqueue.h> > > > > #include "mongoose.h" > > > > @@ -32,6 +33,9 @@ > > #define MG_PORT "8080" > > #define MG_ROOT "." > > > > +#define WEB_API_v1 1 > > +#define WEB_API_v2 2 > > + > > struct mongoose_options { > > char *root; > > char *listing; > > @@ -49,6 +53,25 @@ struct file_upload_state { > > > > static struct mg_serve_http_opts s_http_server_opts; > > > > +#if !defined(CONFIG_MONGOOSE_WEB_API_V1) && > > +!defined(CONFIG_MONGOOSE_WEB_API_V2) > > +#error "WEB API v1 or v2 or both must be defined" > > +#endif > > Maybe we should fall back to V1 to support older .config files. The _defconfig files with no mention of either CONFIG_MONGOOSE_WEB_API_V1 or CONFIG_MONGOOSE_WEB_API_V2 end up selecting the defaults, i.e. V1 currently. Old .config files will trigger Kconfig to query the setting for these two new options interactively. I believe it is best to require that these settings do exist in the generated autoconf headers, rather than falling back to defining them in a file-by-file basis; therefore it should be enough that during build we simply error out in the event the user has (intentionally) misconfigured these settings. > > + > > +#if defined(CONFIG_MONGOOSE_WEB_API_V1) && > > +defined(CONFIG_MONGOOSE_WEB_API_V2) > > +#define NUM_CACHED_MESSAGES 100 > > + > > +struct msg_elem { > > + ipc_message msg; > > + SIMPLEQ_ENTRY(msg_elem) next; > > +}; > > + > > +SIMPLEQ_HEAD(msglist, msg_elem); > > +static struct msglist ipcmessages; > > +static unsigned long nrmsgs = 0; > > +static pthread_mutex_t msglock = PTHREAD_MUTEX_INITIALIZER; static > > +pthread_cond_t msgcond = PTHREAD_COND_INITIALIZER; #endif > > + > > #if defined(CONFIG_MONGOOSE_WEB_API_V2) > > #define enum_string(x) [x] = #x > > static const char *get_status_string(unsigned int status) @@ -112,20 > > +135,163 @@ static size_t snescape(char *dst, size_t n, const char *src) > > } > > #endif > > > > -static void upload_handler(struct mg_connection *nc, int ev, void *p) > > +static int get_inst_status(ipc_message *msg, int __attribute__ > > +((__unused__)) api_vers) { #if > defined(CONFIG_MONGOOSE_WEB_API_V1) && > > +defined(CONFIG_MONGOOSE_WEB_API_V2) > > + struct msg_elem *newmsg; > > + struct msg_elem *oldmsg; > > + int ret = -1; > > + > > + switch (api_vers) { > > + case WEB_API_v1: > > + pthread_mutex_lock(&msglock); > > + pthread_cond_wait(&msgcond, &msglock); > > + oldmsg = SIMPLEQ_FIRST(&ipcmessages); > > + if (oldmsg) { > > + SIMPLEQ_REMOVE_HEAD(&ipcmessages, next); > > + nrmsgs--; > > + memcpy(msg, &oldmsg->msg, sizeof(*msg)); > > + free(oldmsg); > > + ret = 0; > > + } > > + pthread_mutex_unlock(&msglock); > > + break; > > + > > + case WEB_API_v2: > > + newmsg = (struct msg_elem *)calloc(1, sizeof(*newmsg)); > > + if (!newmsg) > > + return -1; > > + > > + ret = ipc_get_status(msg); > > + > > + /* Cache message for APIv1, but consider > > + * changed messages only. */ > > + if (!ret) { > > + pthread_mutex_lock(&msglock); > > + > > + oldmsg = SIMPLEQ_LAST(&ipcmessages, msg_elem, > next); > > + if (!oldmsg || memcmp(&oldmsg->msg.data.status, > > + &msg->data.status, > > + sizeof(msg->data.status))) { > > + > > + nrmsgs++; > > + if (nrmsgs > NUM_CACHED_MESSAGES) { > > + oldmsg = > SIMPLEQ_FIRST(&ipcmessages); > > + > SIMPLEQ_REMOVE_HEAD(&ipcmessages, next); > > + free(oldmsg); > > + nrmsgs--; > > + } > > + > > + memcpy(&newmsg->msg, msg, > sizeof(*msg)); > > + SIMPLEQ_INSERT_TAIL(&ipcmessages, > newmsg, next); > > + newmsg = NULL; > > + } > > + > > + pthread_cond_signal(&msgcond); > > + pthread_mutex_unlock(&msglock); > > + } > > + > > + if (newmsg) > > + free(newmsg); > > + break; > > + } > > + > > + return ret; > > +#else > > + return ipc_get_status(msg); > > +#endif > > +} > > Why you bundle two functions into one function with a switch case? To emphasize the fact that the function, from the caller's viewpoint, provides the same function(ality). Only if both the V1 and V2 APIs are enabled does the implementation differ. > > + > > +static void cleanup_msg_list(void) > > +{ > > +#if defined(CONFIG_MONGOOSE_WEB_API_V1) && > defined(CONFIG_MONGOOSE_WEB_API_V2) > > + struct msg_elem *elem; > > + > > + pthread_mutex_lock(&msglock); > > + > > + while (!SIMPLEQ_EMPTY(&ipcmessages)) { > > + elem = SIMPLEQ_FIRST(&ipcmessages); > > + SIMPLEQ_REMOVE_HEAD(&ipcmessages, next); > > + free(elem); > > + } > > + > > + nrmsgs = 0; > > + pthread_mutex_unlock(&msglock); > > +#endif > > +} > > + > > +static void upload_handler_v2(struct mg_connection *nc, int ev, void > > +*p) > > { > > struct mg_http_multipart_part *mp; > > struct file_upload_state *fus; > > + > > + switch (ev) { > > + case MG_EV_HTTP_PART_BEGIN: > > + mp = (struct mg_http_multipart_part *) p; > > + > > + fus = (struct file_upload_state *) calloc(1, sizeof(*fus)); > > + if (fus == NULL) { > > + mg_http_send_error(nc, 500, "Out of memory"); > > + break; > > + } > > + > > + cleanup_msg_list(); > > + > > + fus->fd = ipc_inst_start_ext(SOURCE_WEBSERVER, > strlen(mp->file_name), mp->file_name); > > + if (fus->fd < 0) { > > + mg_http_send_error(nc, 500, "Failed to queue > command"); > > + free(fus); > > + break; > > + } > > + > > + mp->user_data = fus; > > + > > + break; > > + > > + case MG_EV_HTTP_PART_DATA: > > + mp = (struct mg_http_multipart_part *) p; > > + fus = (struct file_upload_state *) mp->user_data; > > + > > + if (!fus) > > + break; > > + > > + ipc_send_data(fus->fd, (char *) mp->data.p, mp->data.len); > > + fus->len += mp->data.len; > > + > > + break; > > + > > + case MG_EV_HTTP_PART_END: > > + mp = (struct mg_http_multipart_part *) p; > > + fus = (struct file_upload_state *) mp->user_data; > > + > > + if (!fus) > > + break; > > + > > + ipc_end(fus->fd); > > + > > + mg_send_response_line(nc, 200, > > + "Content-Type: text/plain\r\n" > > + "Connection: close"); > > + mg_send(nc, "\r\n", 2); > > + mg_printf(nc, "Ok, %s - %d bytes.\r\n", mp->file_name, (int) > fus->len); > > + nc->flags |= MG_F_SEND_AND_CLOSE; > > + > > + mp->user_data = NULL; > > + free(fus); > > + break; > > + } > > +} > > + > > #if defined(CONFIG_MONGOOSE_WEB_API_V1) > > +static void upload_handler_v1(struct mg_connection *nc, int ev, void > > +*p) { > > struct mg_str *filename, *data; > > struct http_message *hm; > > size_t length; > > char buf[16]; > > int fd; > > -#endif > > > > switch (ev) { > > -#if defined(CONFIG_MONGOOSE_WEB_API_V1) > > case MG_EV_HTTP_REQUEST: > > hm = (struct http_message *) p; > > > > @@ -149,6 +315,8 @@ static void upload_handler(struct mg_connection > *nc, int ev, void *p) > > return; > > } > > > > + cleanup_msg_list(); > > + > > fd = ipc_inst_start_ext(SOURCE_WEBSERVER, filename->len, > filename->p); > > ipc_send_data(fd, (char *) hm->body.p, hm->body.len); > > ipc_end(fd); > > @@ -161,62 +329,13 @@ static void upload_handler(struct mg_connection > *nc, int ev, void *p) > > nc->flags |= MG_F_SEND_AND_CLOSE; > > > > break; > > -#endif > > - case MG_EV_HTTP_PART_BEGIN: > > - mp = (struct mg_http_multipart_part *) p; > > > > - fus = (struct file_upload_state *) calloc(1, sizeof(*fus)); > > - if (fus == NULL) { > > - mg_http_send_error(nc, 500, "Out of memory"); > > - break; > > - } > > - > > - fus->fd = ipc_inst_start_ext(SOURCE_WEBSERVER, > strlen(mp->file_name), mp->file_name); > > - if (fus->fd < 0) { > > - mg_http_send_error(nc, 500, "Failed to queue > command"); > > - free(fus); > > - break; > > - } > > - > > - mp->user_data = fus; > > - > > - break; > > - > > - case MG_EV_HTTP_PART_DATA: > > - mp = (struct mg_http_multipart_part *) p; > > - fus = (struct file_upload_state *) mp->user_data; > > - > > - if (!fus) > > - break; > > - > > - ipc_send_data(fus->fd, (char *) mp->data.p, mp->data.len); > > - fus->len += mp->data.len; > > - > > - break; > > - > > - case MG_EV_HTTP_PART_END: > > - mp = (struct mg_http_multipart_part *) p; > > - fus = (struct file_upload_state *) mp->user_data; > > - > > - if (!fus) > > - break; > > - > > - ipc_end(fus->fd); > > - > > - mg_send_response_line(nc, 200, > > - "Content-Type: text/plain\r\n" > > - "Connection: close"); > > - mg_send(nc, "\r\n", 2); > > - mg_printf(nc, "Ok, %s - %d bytes.\r\n", mp->file_name, (int) > fus->len); > > - nc->flags |= MG_F_SEND_AND_CLOSE; > > - > > - mp->user_data = NULL; > > - free(fus); > > + default: > > + upload_handler_v2(nc, ev, p); > > break; > > } > > } > > > > -#if defined(CONFIG_MONGOOSE_WEB_API_V1) > > static void recovery_status(struct mg_connection *nc, int ev, void > *ev_data) > > { > > ipc_message ipc; > > @@ -226,7 +345,7 @@ static void recovery_status(struct mg_connection > *nc, int ev, void *ev_data) > > (void)ev; > > (void)ev_data; > > > > - ret = ipc_get_status(&ipc); > > + ret = get_inst_status(&ipc, WEB_API_v1); > > > > if (ret) { > > mg_http_send_error(nc, 500, NULL); @@ -306,7 +425,9 @@ > static void > > post_update_cmd(struct mg_connection *nc, int ev, void *ev_data) > > > > nc->flags |= MG_F_SEND_AND_CLOSE; > > } > > -#elif defined(CONFIG_MONGOOSE_WEB_API_V2) > > +#endif > > + > > +#if defined(CONFIG_MONGOOSE_WEB_API_V2) > > static void restart_handler(struct mg_connection *nc, int ev, void > *ev_data) > > { > > struct http_message *hm = (struct http_message *) ev_data; @@ > > -350,7 +471,7 @@ static void *broadcast_message_thread(void *data) > > { > > for (;;) { > > ipc_message msg; > > - int ret = ipc_get_status(&msg); > > + int ret = get_inst_status(&msg, WEB_API_v2); > > > > if (!ret && strlen(msg.data.status.desc) != 0) { > > struct mg_mgr *mgr = (struct mg_mgr *) data; @@ - > 610,15 +731,20 > > @@ int start_mongoose(const char *cfgfname, int argc, char *argv[]) > > exit(EXIT_FAILURE); > > } > > > > +#if defined(CONFIG_MONGOOSE_WEB_API_V1) && > defined(CONFIG_MONGOOSE_WEB_API_V2) > > + SIMPLEQ_INIT(&ipcmessages); > > +#endif > > + > > #if defined(CONFIG_MONGOOSE_WEB_API_V1) > > - mg_register_http_endpoint(nc, "/handle_post_request", > MG_CB(upload_handler, NULL)); > > + mg_register_http_endpoint(nc, "/handle_post_request", > > +MG_CB(upload_handler_v1, NULL)); > > mg_register_http_endpoint(nc, "/getstatus.json", > MG_CB(recovery_status, NULL)); > > mg_register_http_endpoint(nc, "/rebootTarget", > MG_CB(reboot_target, NULL)); > > mg_register_http_endpoint(nc, "/postUpdateCommand", > > MG_CB(post_update_cmd, NULL)); -#elif > > defined(CONFIG_MONGOOSE_WEB_API_V2) > > +#endif > > +#if defined(CONFIG_MONGOOSE_WEB_API_V2) > > mg_register_http_endpoint(nc, "/restart", restart_handler); > > #endif > > - mg_register_http_endpoint(nc, "/upload", MG_CB(upload_handler, > NULL)); > > + mg_register_http_endpoint(nc, "/upload", > MG_CB(upload_handler_v2, > > +NULL)); > > Could you please move the upload_handler_v2 into to V2 guards. Please note how the upload_handler_v1 calls upload_handler_v2 in the default switch case. This was done to provide the same code paths for V1 (also when only V1 is enabled) as before the patch. Or do you mean the mg_register_http_endpoint() call for "/upload"? > > mg_set_protocol_http_websocket(nc); > > > > #if defined(CONFIG_MONGOOSE_WEB_API_V2) > > The code use many ifdefs, maybe it is wise to split the code into three files. Do you mean three files as in - mongoose_interface_v1.c - mongoose_interface_v2.c - mongoose_interface_v1and2.c Or just one additional file containing the bits that are enabled only if both V1 and V2 are enabled? Br, Sami
Hi Sami, Am 27.02.2018 um 11:08 schrieb Sami.Hartikainen@teleste.com: >>> To ease transition, or simply to enable the use of modern clients >>> while still supporting the use of the swuforwarder handler (which only >>> talks API v1), both the v1 and v2 API may be enabled. >>> >>> When both API versions are enabled, the v2 message broadcast thread >>> consumes the status messages from the installer. A message queue is >>> set up to make those messages available for the v1 message polling >>> handler for URI /getstatus.json. >>> >>> Signed-off-by: Sami Hartikainen <sami.hartikainen@teleste.com> >>> --- >>> mongoose/Config.in | 15 +-- >>> mongoose/mongoose_interface.c | 246 >> +++++++++++++++++++++++++++++++----------- >>> 2 files changed, 194 insertions(+), 67 deletions(-) >>> >>> diff --git a/mongoose/Config.in b/mongoose/Config.in index >>> a001247..cd52f2c 100644 >>> --- a/mongoose/Config.in >>> +++ b/mongoose/Config.in >>> @@ -14,27 +14,28 @@ choice >>> config MONGOOSE >>> bool "mongoose" >>> help >>> - Mongoose embeddded web server >>> + Mongoose embedded web server >>> >>> endchoice >>> >>> -choice >>> - prompt "Web Application Interface" >>> - default MONGOOSE_WEB_API_V1 >>> - help >>> - Choose the bootloader >>> +menu "Web Application Interface" >>> >>> config MONGOOSE_WEB_API_V1 >>> bool "Version 1 (deprecated)" >>> + default y >>> help >>> Support for version 1 >>> >>> config MONGOOSE_WEB_API_V2 >>> bool "Version 2" >>> + default n >> Maybe we should also enable V2 by default. > Enabling both adds binary footprint size and code complexity, so > I chose to be cautious with this. On the other hand, describing V1 > as "deprecated" implies it should not be the only one by default > enabled. The v1 was only the default to be backward compatible. @Stefano: What do you think? >>> help >>> Support for version 2 >>> >>> -endchoice >>> +comment "Web API v1 or v2 or both required" >>> + depends on !MONGOOSE_WEB_API_V1 && >> !MONGOOSE_WEB_API_V2 >>> + >>> +endmenu >>> >>> config MONGOOSEIPV6 >>> bool "IPv6 support" >>> diff --git a/mongoose/mongoose_interface.c >>> b/mongoose/mongoose_interface.c index 89a51f3..0401a9a 100644 >>> --- a/mongoose/mongoose_interface.c >>> +++ b/mongoose/mongoose_interface.c >>> @@ -25,6 +25,7 @@ >>> #include <parselib.h> >>> #include <progress_ipc.h> >>> #include <swupdate_settings.h> >>> +#include <bsdqueue.h> >>> >>> #include "mongoose.h" >>> >>> @@ -32,6 +33,9 @@ >>> #define MG_PORT "8080" >>> #define MG_ROOT "." >>> >>> +#define WEB_API_v1 1 >>> +#define WEB_API_v2 2 >>> + >>> struct mongoose_options { >>> char *root; >>> char *listing; >>> @@ -49,6 +53,25 @@ struct file_upload_state { >>> >>> static struct mg_serve_http_opts s_http_server_opts; >>> >>> +#if !defined(CONFIG_MONGOOSE_WEB_API_V1) && >>> +!defined(CONFIG_MONGOOSE_WEB_API_V2) >>> +#error "WEB API v1 or v2 or both must be defined" >>> +#endif >> Maybe we should fall back to V1 to support older .config files. > The _defconfig files with no mention of either CONFIG_MONGOOSE_WEB_API_V1 or > CONFIG_MONGOOSE_WEB_API_V2 end up selecting the defaults, i.e. V1 currently. > > Old .config files will trigger Kconfig to query the setting for these two new options > interactively. > > I believe it is best to require that these settings do exist in the generated autoconf headers, > rather than falling back to defining them in a file-by-file basis; therefore it should be enough > that during build we simply error out in the event the user has (intentionally) misconfigured > these settings. > >>> + >>> +#if defined(CONFIG_MONGOOSE_WEB_API_V1) && >>> +defined(CONFIG_MONGOOSE_WEB_API_V2) >>> +#define NUM_CACHED_MESSAGES 100 >>> + >>> +struct msg_elem { >>> + ipc_message msg; >>> + SIMPLEQ_ENTRY(msg_elem) next; >>> +}; >>> + >>> +SIMPLEQ_HEAD(msglist, msg_elem); >>> +static struct msglist ipcmessages; >>> +static unsigned long nrmsgs = 0; >>> +static pthread_mutex_t msglock = PTHREAD_MUTEX_INITIALIZER; static >>> +pthread_cond_t msgcond = PTHREAD_COND_INITIALIZER; #endif >>> + >>> #if defined(CONFIG_MONGOOSE_WEB_API_V2) >>> #define enum_string(x) [x] = #x >>> static const char *get_status_string(unsigned int status) @@ -112,20 >>> +135,163 @@ static size_t snescape(char *dst, size_t n, const char *src) >>> } >>> #endif >>> >>> -static void upload_handler(struct mg_connection *nc, int ev, void *p) >>> +static int get_inst_status(ipc_message *msg, int __attribute__ >>> +((__unused__)) api_vers) { #if >> defined(CONFIG_MONGOOSE_WEB_API_V1) && >>> +defined(CONFIG_MONGOOSE_WEB_API_V2) >>> + struct msg_elem *newmsg; >>> + struct msg_elem *oldmsg; >>> + int ret = -1; >>> + >>> + switch (api_vers) { >>> + case WEB_API_v1: >>> + pthread_mutex_lock(&msglock); >>> + pthread_cond_wait(&msgcond, &msglock); >>> + oldmsg = SIMPLEQ_FIRST(&ipcmessages); >>> + if (oldmsg) { >>> + SIMPLEQ_REMOVE_HEAD(&ipcmessages, next); >>> + nrmsgs--; >>> + memcpy(msg, &oldmsg->msg, sizeof(*msg)); >>> + free(oldmsg); >>> + ret = 0; >>> + } >>> + pthread_mutex_unlock(&msglock); >>> + break; >>> + >>> + case WEB_API_v2: >>> + newmsg = (struct msg_elem *)calloc(1, sizeof(*newmsg)); >>> + if (!newmsg) >>> + return -1; >>> + >>> + ret = ipc_get_status(msg); >>> + >>> + /* Cache message for APIv1, but consider >>> + * changed messages only. */ >>> + if (!ret) { >>> + pthread_mutex_lock(&msglock); >>> + >>> + oldmsg = SIMPLEQ_LAST(&ipcmessages, msg_elem, >> next); >>> + if (!oldmsg || memcmp(&oldmsg->msg.data.status, >>> + &msg->data.status, >>> + sizeof(msg->data.status))) { >>> + >>> + nrmsgs++; >>> + if (nrmsgs > NUM_CACHED_MESSAGES) { >>> + oldmsg = >> SIMPLEQ_FIRST(&ipcmessages); >>> + >> SIMPLEQ_REMOVE_HEAD(&ipcmessages, next); >>> + free(oldmsg); >>> + nrmsgs--; >>> + } >>> + >>> + memcpy(&newmsg->msg, msg, >> sizeof(*msg)); >>> + SIMPLEQ_INSERT_TAIL(&ipcmessages, >> newmsg, next); >>> + newmsg = NULL; >>> + } >>> + >>> + pthread_cond_signal(&msgcond); >>> + pthread_mutex_unlock(&msglock); >>> + } >>> + >>> + if (newmsg) >>> + free(newmsg); >>> + break; >>> + } >>> + >>> + return ret; >>> +#else >>> + return ipc_get_status(msg); >>> +#endif >>> +} >> Why you bundle two functions into one function with a switch case? > To emphasize the fact that the function, from the caller's viewpoint, provides the same > function(ality). Only if both the V1 and V2 APIs are enabled does the implementation > differ. In this case I would move the if def and ipc_get_status(msg) function to its old place and create two new meaningful functions. In the v1 and v2 API case you have one function with two totally different behaviors. Do you need the v1 and v2 API at the same time? Otherwise we could start the threads after the first web socket connected and stop them after the last web socket was closed. >>> + >>> +static void cleanup_msg_list(void) >>> +{ >>> +#if defined(CONFIG_MONGOOSE_WEB_API_V1) && >> defined(CONFIG_MONGOOSE_WEB_API_V2) >>> + struct msg_elem *elem; >>> + >>> + pthread_mutex_lock(&msglock); >>> + >>> + while (!SIMPLEQ_EMPTY(&ipcmessages)) { >>> + elem = SIMPLEQ_FIRST(&ipcmessages); >>> + SIMPLEQ_REMOVE_HEAD(&ipcmessages, next); >>> + free(elem); >>> + } >>> + >>> + nrmsgs = 0; >>> + pthread_mutex_unlock(&msglock); >>> +#endif >>> +} >>> + >>> +static void upload_handler_v2(struct mg_connection *nc, int ev, void >>> +*p) >>> { >>> struct mg_http_multipart_part *mp; >>> struct file_upload_state *fus; >>> + >>> + switch (ev) { >>> + case MG_EV_HTTP_PART_BEGIN: >>> + mp = (struct mg_http_multipart_part *) p; >>> + >>> + fus = (struct file_upload_state *) calloc(1, sizeof(*fus)); >>> + if (fus == NULL) { >>> + mg_http_send_error(nc, 500, "Out of memory"); >>> + break; >>> + } >>> + >>> + cleanup_msg_list(); >>> + >>> + fus->fd = ipc_inst_start_ext(SOURCE_WEBSERVER, >> strlen(mp->file_name), mp->file_name); >>> + if (fus->fd < 0) { >>> + mg_http_send_error(nc, 500, "Failed to queue >> command"); >>> + free(fus); >>> + break; >>> + } >>> + >>> + mp->user_data = fus; >>> + >>> + break; >>> + >>> + case MG_EV_HTTP_PART_DATA: >>> + mp = (struct mg_http_multipart_part *) p; >>> + fus = (struct file_upload_state *) mp->user_data; >>> + >>> + if (!fus) >>> + break; >>> + >>> + ipc_send_data(fus->fd, (char *) mp->data.p, mp->data.len); >>> + fus->len += mp->data.len; >>> + >>> + break; >>> + >>> + case MG_EV_HTTP_PART_END: >>> + mp = (struct mg_http_multipart_part *) p; >>> + fus = (struct file_upload_state *) mp->user_data; >>> + >>> + if (!fus) >>> + break; >>> + >>> + ipc_end(fus->fd); >>> + >>> + mg_send_response_line(nc, 200, >>> + "Content-Type: text/plain\r\n" >>> + "Connection: close"); >>> + mg_send(nc, "\r\n", 2); >>> + mg_printf(nc, "Ok, %s - %d bytes.\r\n", mp->file_name, (int) >> fus->len); >>> + nc->flags |= MG_F_SEND_AND_CLOSE; >>> + >>> + mp->user_data = NULL; >>> + free(fus); >>> + break; >>> + } >>> +} >>> + >>> #if defined(CONFIG_MONGOOSE_WEB_API_V1) >>> +static void upload_handler_v1(struct mg_connection *nc, int ev, void >>> +*p) { >>> struct mg_str *filename, *data; >>> struct http_message *hm; >>> size_t length; >>> char buf[16]; >>> int fd; >>> -#endif >>> >>> switch (ev) { >>> -#if defined(CONFIG_MONGOOSE_WEB_API_V1) >>> case MG_EV_HTTP_REQUEST: >>> hm = (struct http_message *) p; >>> >>> @@ -149,6 +315,8 @@ static void upload_handler(struct mg_connection >> *nc, int ev, void *p) >>> return; >>> } >>> >>> + cleanup_msg_list(); >>> + >>> fd = ipc_inst_start_ext(SOURCE_WEBSERVER, filename->len, >> filename->p); >>> ipc_send_data(fd, (char *) hm->body.p, hm->body.len); >>> ipc_end(fd); >>> @@ -161,62 +329,13 @@ static void upload_handler(struct mg_connection >> *nc, int ev, void *p) >>> nc->flags |= MG_F_SEND_AND_CLOSE; >>> >>> break; >>> -#endif >>> - case MG_EV_HTTP_PART_BEGIN: >>> - mp = (struct mg_http_multipart_part *) p; >>> >>> - fus = (struct file_upload_state *) calloc(1, sizeof(*fus)); >>> - if (fus == NULL) { >>> - mg_http_send_error(nc, 500, "Out of memory"); >>> - break; >>> - } >>> - >>> - fus->fd = ipc_inst_start_ext(SOURCE_WEBSERVER, >> strlen(mp->file_name), mp->file_name); >>> - if (fus->fd < 0) { >>> - mg_http_send_error(nc, 500, "Failed to queue >> command"); >>> - free(fus); >>> - break; >>> - } >>> - >>> - mp->user_data = fus; >>> - >>> - break; >>> - >>> - case MG_EV_HTTP_PART_DATA: >>> - mp = (struct mg_http_multipart_part *) p; >>> - fus = (struct file_upload_state *) mp->user_data; >>> - >>> - if (!fus) >>> - break; >>> - >>> - ipc_send_data(fus->fd, (char *) mp->data.p, mp->data.len); >>> - fus->len += mp->data.len; >>> - >>> - break; >>> - >>> - case MG_EV_HTTP_PART_END: >>> - mp = (struct mg_http_multipart_part *) p; >>> - fus = (struct file_upload_state *) mp->user_data; >>> - >>> - if (!fus) >>> - break; >>> - >>> - ipc_end(fus->fd); >>> - >>> - mg_send_response_line(nc, 200, >>> - "Content-Type: text/plain\r\n" >>> - "Connection: close"); >>> - mg_send(nc, "\r\n", 2); >>> - mg_printf(nc, "Ok, %s - %d bytes.\r\n", mp->file_name, (int) >> fus->len); >>> - nc->flags |= MG_F_SEND_AND_CLOSE; >>> - >>> - mp->user_data = NULL; >>> - free(fus); >>> + default: >>> + upload_handler_v2(nc, ev, p); >>> break; >>> } >>> } >>> >>> -#if defined(CONFIG_MONGOOSE_WEB_API_V1) >>> static void recovery_status(struct mg_connection *nc, int ev, void >> *ev_data) >>> { >>> ipc_message ipc; >>> @@ -226,7 +345,7 @@ static void recovery_status(struct mg_connection >> *nc, int ev, void *ev_data) >>> (void)ev; >>> (void)ev_data; >>> >>> - ret = ipc_get_status(&ipc); >>> + ret = get_inst_status(&ipc, WEB_API_v1); >>> >>> if (ret) { >>> mg_http_send_error(nc, 500, NULL); @@ -306,7 +425,9 @@ >> static void >>> post_update_cmd(struct mg_connection *nc, int ev, void *ev_data) >>> >>> nc->flags |= MG_F_SEND_AND_CLOSE; >>> } >>> -#elif defined(CONFIG_MONGOOSE_WEB_API_V2) >>> +#endif >>> + >>> +#if defined(CONFIG_MONGOOSE_WEB_API_V2) >>> static void restart_handler(struct mg_connection *nc, int ev, void >> *ev_data) >>> { >>> struct http_message *hm = (struct http_message *) ev_data; @@ >>> -350,7 +471,7 @@ static void *broadcast_message_thread(void *data) >>> { >>> for (;;) { >>> ipc_message msg; >>> - int ret = ipc_get_status(&msg); >>> + int ret = get_inst_status(&msg, WEB_API_v2); >>> >>> if (!ret && strlen(msg.data.status.desc) != 0) { >>> struct mg_mgr *mgr = (struct mg_mgr *) data; @@ - >> 610,15 +731,20 >>> @@ int start_mongoose(const char *cfgfname, int argc, char *argv[]) >>> exit(EXIT_FAILURE); >>> } >>> >>> +#if defined(CONFIG_MONGOOSE_WEB_API_V1) && >> defined(CONFIG_MONGOOSE_WEB_API_V2) >>> + SIMPLEQ_INIT(&ipcmessages); >>> +#endif >>> + >>> #if defined(CONFIG_MONGOOSE_WEB_API_V1) >>> - mg_register_http_endpoint(nc, "/handle_post_request", >> MG_CB(upload_handler, NULL)); >>> + mg_register_http_endpoint(nc, "/handle_post_request", >>> +MG_CB(upload_handler_v1, NULL)); >>> mg_register_http_endpoint(nc, "/getstatus.json", >> MG_CB(recovery_status, NULL)); >>> mg_register_http_endpoint(nc, "/rebootTarget", >> MG_CB(reboot_target, NULL)); >>> mg_register_http_endpoint(nc, "/postUpdateCommand", >>> MG_CB(post_update_cmd, NULL)); -#elif >>> defined(CONFIG_MONGOOSE_WEB_API_V2) >>> +#endif >>> +#if defined(CONFIG_MONGOOSE_WEB_API_V2) >>> mg_register_http_endpoint(nc, "/restart", restart_handler); >>> #endif >>> - mg_register_http_endpoint(nc, "/upload", MG_CB(upload_handler, >> NULL)); >>> + mg_register_http_endpoint(nc, "/upload", >> MG_CB(upload_handler_v2, >>> +NULL)); >> Could you please move the upload_handler_v2 into to V2 guards. > Please note how the upload_handler_v1 calls upload_handler_v2 in the default > switch case. This was done to provide the same code paths for V1 (also when only > V1 is enabled) as before the patch. > > Or do you mean the mg_register_http_endpoint() call for "/upload"? Yes I mean the mg_register_http_endpoint() call. I missed to move this into the if def. >>> mg_set_protocol_http_websocket(nc); >>> >>> #if defined(CONFIG_MONGOOSE_WEB_API_V2) >> The code use many ifdefs, maybe it is wise to split the code into three files. > Do you mean three files as in > - mongoose_interface_v1.c > - mongoose_interface_v2.c > - mongoose_interface_v1and2.c > > Or just one additional file containing the bits that are enabled only if both V1 and V2 > are enabled? I mean the three files as mentioned. @Stefano: What do you think? Best regards Stefan
Hi Stefan, > >>> diff --git a/mongoose/Config.in b/mongoose/Config.in index <snip> > >>> +menu "Web Application Interface" > >>> > >>> config MONGOOSE_WEB_API_V1 > >>> bool "Version 1 (deprecated)" > >>> + default y > >>> help > >>> Support for version 1 > >>> > >>> config MONGOOSE_WEB_API_V2 > >>> bool "Version 2" > >>> + default n > >> Maybe we should also enable V2 by default. > > Enabling both adds binary footprint size and code complexity, so I > > chose to be cautious with this. On the other hand, describing V1 as > > "deprecated" implies it should not be the only one by default enabled. > > The v1 was only the default to be backward compatible. > > @Stefano: What do you think? Ok. Enabling both by default unless someone objects. > >>> diff --git a/mongoose/mongoose_interface.c > >>> b/mongoose/mongoose_interface.c index 89a51f3..0401a9a 100644 > >>> --- a/mongoose/mongoose_interface.c > >>> +++ b/mongoose/mongoose_interface.c <snip> > > Do you need the v1 and v2 API at the same time? Otherwise we could start > the threads after the first web socket connected and stop them after the last > web socket was closed. No, I think they will not be needed at same time in any sane scenario. Starting the v2 broadcast threads only when needed sounds like a good idea. I'll look into that. > >> 610,15 +731,20 > >>> @@ int start_mongoose(const char *cfgfname, int argc, char *argv[]) > >>> exit(EXIT_FAILURE); > >>> } > >>> > >>> +#if defined(CONFIG_MONGOOSE_WEB_API_V1) && > >> defined(CONFIG_MONGOOSE_WEB_API_V2) > >>> + SIMPLEQ_INIT(&ipcmessages); > >>> +#endif > >>> + > >>> #if defined(CONFIG_MONGOOSE_WEB_API_V1) > >>> - mg_register_http_endpoint(nc, "/handle_post_request", > >> MG_CB(upload_handler, NULL)); > >>> + mg_register_http_endpoint(nc, "/handle_post_request", > >>> +MG_CB(upload_handler_v1, NULL)); > >>> mg_register_http_endpoint(nc, "/getstatus.json", > >> MG_CB(recovery_status, NULL)); > >>> mg_register_http_endpoint(nc, "/rebootTarget", > >> MG_CB(reboot_target, NULL)); > >>> mg_register_http_endpoint(nc, "/postUpdateCommand", > >>> MG_CB(post_update_cmd, NULL)); -#elif > >>> defined(CONFIG_MONGOOSE_WEB_API_V2) > >>> +#endif > >>> +#if defined(CONFIG_MONGOOSE_WEB_API_V2) > >>> mg_register_http_endpoint(nc, "/restart", restart_handler); > >>> #endif > >>> - mg_register_http_endpoint(nc, "/upload", MG_CB(upload_handler, > >> NULL)); > >>> + mg_register_http_endpoint(nc, "/upload", > >> MG_CB(upload_handler_v2, > >>> +NULL)); > >> Could you please move the upload_handler_v2 into to V2 guards. > > > > Or do you mean the mg_register_http_endpoint() call for "/upload"? > > Yes I mean the mg_register_http_endpoint() call. I missed to move this into > the if def. Ok. > >> The code use many ifdefs, maybe it is wise to split the code into three > files. > > Do you mean three files as in > > - mongoose_interface_v1.c > > - mongoose_interface_v2.c > > - mongoose_interface_v1and2.c > > > > Or just one additional file containing the bits that are enabled only > > if both V1 and V2 are enabled? > > I mean the three files as mentioned. > > @Stefano: What do you think? Let's revisit this after patch v2. Br, Sami
diff --git a/mongoose/Config.in b/mongoose/Config.in index a001247..cd52f2c 100644 --- a/mongoose/Config.in +++ b/mongoose/Config.in @@ -14,27 +14,28 @@ choice config MONGOOSE bool "mongoose" help - Mongoose embeddded web server + Mongoose embedded web server endchoice -choice - prompt "Web Application Interface" - default MONGOOSE_WEB_API_V1 - help - Choose the bootloader +menu "Web Application Interface" config MONGOOSE_WEB_API_V1 bool "Version 1 (deprecated)" + default y help Support for version 1 config MONGOOSE_WEB_API_V2 bool "Version 2" + default n help Support for version 2 -endchoice +comment "Web API v1 or v2 or both required" + depends on !MONGOOSE_WEB_API_V1 && !MONGOOSE_WEB_API_V2 + +endmenu config MONGOOSEIPV6 bool "IPv6 support" diff --git a/mongoose/mongoose_interface.c b/mongoose/mongoose_interface.c index 89a51f3..0401a9a 100644 --- a/mongoose/mongoose_interface.c +++ b/mongoose/mongoose_interface.c @@ -25,6 +25,7 @@ #include <parselib.h> #include <progress_ipc.h> #include <swupdate_settings.h> +#include <bsdqueue.h> #include "mongoose.h" @@ -32,6 +33,9 @@ #define MG_PORT "8080" #define MG_ROOT "." +#define WEB_API_v1 1 +#define WEB_API_v2 2 + struct mongoose_options { char *root; char *listing; @@ -49,6 +53,25 @@ struct file_upload_state { static struct mg_serve_http_opts s_http_server_opts; +#if !defined(CONFIG_MONGOOSE_WEB_API_V1) && !defined(CONFIG_MONGOOSE_WEB_API_V2) +#error "WEB API v1 or v2 or both must be defined" +#endif + +#if defined(CONFIG_MONGOOSE_WEB_API_V1) && defined(CONFIG_MONGOOSE_WEB_API_V2) +#define NUM_CACHED_MESSAGES 100 + +struct msg_elem { + ipc_message msg; + SIMPLEQ_ENTRY(msg_elem) next; +}; + +SIMPLEQ_HEAD(msglist, msg_elem); +static struct msglist ipcmessages; +static unsigned long nrmsgs = 0; +static pthread_mutex_t msglock = PTHREAD_MUTEX_INITIALIZER; +static pthread_cond_t msgcond = PTHREAD_COND_INITIALIZER; +#endif + #if defined(CONFIG_MONGOOSE_WEB_API_V2) #define enum_string(x) [x] = #x static const char *get_status_string(unsigned int status) @@ -112,20 +135,163 @@ static size_t snescape(char *dst, size_t n, const char *src) } #endif -static void upload_handler(struct mg_connection *nc, int ev, void *p) +static int get_inst_status(ipc_message *msg, int __attribute__ ((__unused__)) api_vers) +{ +#if defined(CONFIG_MONGOOSE_WEB_API_V1) && defined(CONFIG_MONGOOSE_WEB_API_V2) + struct msg_elem *newmsg; + struct msg_elem *oldmsg; + int ret = -1; + + switch (api_vers) { + case WEB_API_v1: + pthread_mutex_lock(&msglock); + pthread_cond_wait(&msgcond, &msglock); + oldmsg = SIMPLEQ_FIRST(&ipcmessages); + if (oldmsg) { + SIMPLEQ_REMOVE_HEAD(&ipcmessages, next); + nrmsgs--; + memcpy(msg, &oldmsg->msg, sizeof(*msg)); + free(oldmsg); + ret = 0; + } + pthread_mutex_unlock(&msglock); + break; + + case WEB_API_v2: + newmsg = (struct msg_elem *)calloc(1, sizeof(*newmsg)); + if (!newmsg) + return -1; + + ret = ipc_get_status(msg); + + /* Cache message for APIv1, but consider + * changed messages only. */ + if (!ret) { + pthread_mutex_lock(&msglock); + + oldmsg = SIMPLEQ_LAST(&ipcmessages, msg_elem, next); + if (!oldmsg || memcmp(&oldmsg->msg.data.status, + &msg->data.status, + sizeof(msg->data.status))) { + + nrmsgs++; + if (nrmsgs > NUM_CACHED_MESSAGES) { + oldmsg = SIMPLEQ_FIRST(&ipcmessages); + SIMPLEQ_REMOVE_HEAD(&ipcmessages, next); + free(oldmsg); + nrmsgs--; + } + + memcpy(&newmsg->msg, msg, sizeof(*msg)); + SIMPLEQ_INSERT_TAIL(&ipcmessages, newmsg, next); + newmsg = NULL; + } + + pthread_cond_signal(&msgcond); + pthread_mutex_unlock(&msglock); + } + + if (newmsg) + free(newmsg); + break; + } + + return ret; +#else + return ipc_get_status(msg); +#endif +} + +static void cleanup_msg_list(void) +{ +#if defined(CONFIG_MONGOOSE_WEB_API_V1) && defined(CONFIG_MONGOOSE_WEB_API_V2) + struct msg_elem *elem; + + pthread_mutex_lock(&msglock); + + while (!SIMPLEQ_EMPTY(&ipcmessages)) { + elem = SIMPLEQ_FIRST(&ipcmessages); + SIMPLEQ_REMOVE_HEAD(&ipcmessages, next); + free(elem); + } + + nrmsgs = 0; + pthread_mutex_unlock(&msglock); +#endif +} + +static void upload_handler_v2(struct mg_connection *nc, int ev, void *p) { struct mg_http_multipart_part *mp; struct file_upload_state *fus; + + switch (ev) { + case MG_EV_HTTP_PART_BEGIN: + mp = (struct mg_http_multipart_part *) p; + + fus = (struct file_upload_state *) calloc(1, sizeof(*fus)); + if (fus == NULL) { + mg_http_send_error(nc, 500, "Out of memory"); + break; + } + + cleanup_msg_list(); + + fus->fd = ipc_inst_start_ext(SOURCE_WEBSERVER, strlen(mp->file_name), mp->file_name); + if (fus->fd < 0) { + mg_http_send_error(nc, 500, "Failed to queue command"); + free(fus); + break; + } + + mp->user_data = fus; + + break; + + case MG_EV_HTTP_PART_DATA: + mp = (struct mg_http_multipart_part *) p; + fus = (struct file_upload_state *) mp->user_data; + + if (!fus) + break; + + ipc_send_data(fus->fd, (char *) mp->data.p, mp->data.len); + fus->len += mp->data.len; + + break; + + case MG_EV_HTTP_PART_END: + mp = (struct mg_http_multipart_part *) p; + fus = (struct file_upload_state *) mp->user_data; + + if (!fus) + break; + + ipc_end(fus->fd); + + mg_send_response_line(nc, 200, + "Content-Type: text/plain\r\n" + "Connection: close"); + mg_send(nc, "\r\n", 2); + mg_printf(nc, "Ok, %s - %d bytes.\r\n", mp->file_name, (int) fus->len); + nc->flags |= MG_F_SEND_AND_CLOSE; + + mp->user_data = NULL; + free(fus); + break; + } +} + #if defined(CONFIG_MONGOOSE_WEB_API_V1) +static void upload_handler_v1(struct mg_connection *nc, int ev, void *p) +{ struct mg_str *filename, *data; struct http_message *hm; size_t length; char buf[16]; int fd; -#endif switch (ev) { -#if defined(CONFIG_MONGOOSE_WEB_API_V1) case MG_EV_HTTP_REQUEST: hm = (struct http_message *) p; @@ -149,6 +315,8 @@ static void upload_handler(struct mg_connection *nc, int ev, void *p) return; } + cleanup_msg_list(); + fd = ipc_inst_start_ext(SOURCE_WEBSERVER, filename->len, filename->p); ipc_send_data(fd, (char *) hm->body.p, hm->body.len); ipc_end(fd); @@ -161,62 +329,13 @@ static void upload_handler(struct mg_connection *nc, int ev, void *p) nc->flags |= MG_F_SEND_AND_CLOSE; break; -#endif - case MG_EV_HTTP_PART_BEGIN: - mp = (struct mg_http_multipart_part *) p; - fus = (struct file_upload_state *) calloc(1, sizeof(*fus)); - if (fus == NULL) { - mg_http_send_error(nc, 500, "Out of memory"); - break; - } - - fus->fd = ipc_inst_start_ext(SOURCE_WEBSERVER, strlen(mp->file_name), mp->file_name); - if (fus->fd < 0) { - mg_http_send_error(nc, 500, "Failed to queue command"); - free(fus); - break; - } - - mp->user_data = fus; - - break; - - case MG_EV_HTTP_PART_DATA: - mp = (struct mg_http_multipart_part *) p; - fus = (struct file_upload_state *) mp->user_data; - - if (!fus) - break; - - ipc_send_data(fus->fd, (char *) mp->data.p, mp->data.len); - fus->len += mp->data.len; - - break; - - case MG_EV_HTTP_PART_END: - mp = (struct mg_http_multipart_part *) p; - fus = (struct file_upload_state *) mp->user_data; - - if (!fus) - break; - - ipc_end(fus->fd); - - mg_send_response_line(nc, 200, - "Content-Type: text/plain\r\n" - "Connection: close"); - mg_send(nc, "\r\n", 2); - mg_printf(nc, "Ok, %s - %d bytes.\r\n", mp->file_name, (int) fus->len); - nc->flags |= MG_F_SEND_AND_CLOSE; - - mp->user_data = NULL; - free(fus); + default: + upload_handler_v2(nc, ev, p); break; } } -#if defined(CONFIG_MONGOOSE_WEB_API_V1) static void recovery_status(struct mg_connection *nc, int ev, void *ev_data) { ipc_message ipc; @@ -226,7 +345,7 @@ static void recovery_status(struct mg_connection *nc, int ev, void *ev_data) (void)ev; (void)ev_data; - ret = ipc_get_status(&ipc); + ret = get_inst_status(&ipc, WEB_API_v1); if (ret) { mg_http_send_error(nc, 500, NULL); @@ -306,7 +425,9 @@ static void post_update_cmd(struct mg_connection *nc, int ev, void *ev_data) nc->flags |= MG_F_SEND_AND_CLOSE; } -#elif defined(CONFIG_MONGOOSE_WEB_API_V2) +#endif + +#if defined(CONFIG_MONGOOSE_WEB_API_V2) static void restart_handler(struct mg_connection *nc, int ev, void *ev_data) { struct http_message *hm = (struct http_message *) ev_data; @@ -350,7 +471,7 @@ static void *broadcast_message_thread(void *data) { for (;;) { ipc_message msg; - int ret = ipc_get_status(&msg); + int ret = get_inst_status(&msg, WEB_API_v2); if (!ret && strlen(msg.data.status.desc) != 0) { struct mg_mgr *mgr = (struct mg_mgr *) data; @@ -610,15 +731,20 @@ int start_mongoose(const char *cfgfname, int argc, char *argv[]) exit(EXIT_FAILURE); } +#if defined(CONFIG_MONGOOSE_WEB_API_V1) && defined(CONFIG_MONGOOSE_WEB_API_V2) + SIMPLEQ_INIT(&ipcmessages); +#endif + #if defined(CONFIG_MONGOOSE_WEB_API_V1) - mg_register_http_endpoint(nc, "/handle_post_request", MG_CB(upload_handler, NULL)); + mg_register_http_endpoint(nc, "/handle_post_request", MG_CB(upload_handler_v1, NULL)); mg_register_http_endpoint(nc, "/getstatus.json", MG_CB(recovery_status, NULL)); mg_register_http_endpoint(nc, "/rebootTarget", MG_CB(reboot_target, NULL)); mg_register_http_endpoint(nc, "/postUpdateCommand", MG_CB(post_update_cmd, NULL)); -#elif defined(CONFIG_MONGOOSE_WEB_API_V2) +#endif +#if defined(CONFIG_MONGOOSE_WEB_API_V2) mg_register_http_endpoint(nc, "/restart", restart_handler); #endif - mg_register_http_endpoint(nc, "/upload", MG_CB(upload_handler, NULL)); + mg_register_http_endpoint(nc, "/upload", MG_CB(upload_handler_v2, NULL)); mg_set_protocol_http_websocket(nc); #if defined(CONFIG_MONGOOSE_WEB_API_V2)
To ease transition, or simply to enable the use of modern clients while still supporting the use of the swuforwarder handler (which only talks API v1), both the v1 and v2 API may be enabled. When both API versions are enabled, the v2 message broadcast thread consumes the status messages from the installer. A message queue is set up to make those messages available for the v1 message polling handler for URI /getstatus.json. Signed-off-by: Sami Hartikainen <sami.hartikainen@teleste.com> --- mongoose/Config.in | 15 +-- mongoose/mongoose_interface.c | 246 +++++++++++++++++++++++++++++++----------- 2 files changed, 194 insertions(+), 67 deletions(-)