Hi,
the alfred daemon has built-in capabilities for accepting runtime configuration. Network interfaces and alfred 'mode' (primary/secondary) settings can be modified via instructions sent over unix socket.
To make all settings configurable, the unix socket commands are extended to also support setting the batman-adv interface name at runtime. Network interfaces should no longer be required at startup as those can be configured at runtime.
All these runtime settings make understanding the current alfred configuration tricky. Therefore, alfred should be able to export its operating configuration via unix socket.
Cheers, Marek
Signed-off-by: Marek Lindner mareklindner@neomailbox.ch --- server.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/server.c b/server.c index b4925e7..85bf453 100644 --- a/server.c +++ b/server.c @@ -442,7 +442,6 @@ int alfred_server(struct globals *globals) netsock_check_error(globals, &errfds);
if (FD_ISSET(globals->unix_sock, &fds)) { - printf("read unix socket\n"); unix_sock_read(globals); continue; } else {
The '-i' commandline parameter to specify interface names no longer is mandatory. Specifying interface 'none' or sending a 'none' interface string within the ALFRED_CHANGE_INTERFACE unix socket command disables all interfaces operations at runtime.
Signed-off-by: Marek Lindner mareklindner@neomailbox.ch --- README.rst | 6 +++++- alfred.h | 1 - man/alfred.8 | 6 +++++- netsock.c | 23 ++++------------------- server.c | 12 ------------ 5 files changed, 14 insertions(+), 34 deletions(-)
diff --git a/README.rst b/README.rst index 33200e4..7f44db6 100644 --- a/README.rst +++ b/README.rst @@ -82,7 +82,8 @@ documentation how to configure alfred in this case. In any event, you can still run alfred from the command line. The relevant options are (for a full list of options, run alfred -h):
- -i, --interface specify the interface to listen on + -i, --interface specify the interface to listen on. use 'none' + to disable interface operations -b specify the batman-adv interface configured on the system (default: bat0). use 'none' to disable the batman-adv based best server selection @@ -90,6 +91,9 @@ list of options, run alfred -h): accepts data from secondaries and syncs it with other primaries
+The interface option '-i' is optional. If interface 'none' is specified, the +alfred daemon will not communicate with other alfred instances on the +network unless the interface list is modified at runtime via the unix socket. The -b option is optional, and only needed if you run alfred on a batman-adv interface not called bat0, or if you don't use batman-adv at all (use '-b none'). In this case, alfred will still work but will not be able to diff --git a/alfred.h b/alfred.h index d844261..57d7daf 100644 --- a/alfred.h +++ b/alfred.h @@ -182,7 +182,6 @@ int unix_sock_req_data_finish(struct globals *globals, /* vis.c */ int vis_update_data(struct globals *globals); /* netsock.c */ -int netsock_open_all(struct globals *globals); size_t netsocket_count_interfaces(struct globals *globals); void netsock_close_all(struct globals *globals); int netsock_set_interfaces(struct globals *globals, char *interfaces); diff --git a/man/alfred.8 b/man/alfred.8 index 18e008e..ff9b315 100644 --- a/man/alfred.8 +++ b/man/alfred.8 @@ -95,12 +95,16 @@ Change the alfred server to use the new \fBinterface\fP(s) .SH SERVER OPTIONS .TP \fB-i\fP, \fB--interface\fP \fIiface\fP -Specify the interface (or comma separated list of interfaces) to listen on +Specify the interface (or comma separated list of interfaces) to listen on. +Use 'none' to disable interface operations. .TP \fB-b\fP \fIbatmanif\fP Specify the batman-adv interface configured on the system (default: bat0). Use 'none' to disable the batman-adv based best server selection.
+The interface option \fB-i\fP is optional. If interface 'none' is specified, the +alfred daemon will not communicate with other alfred instances on the +network unless the interface list is modified at runtime via the unix socket. The \fB-b\fP option is optional, and only needed if you run alfred on a batman-adv interface not called bat0, or if you don't use batman-adv at all (use '\fB-b\fP none'). In this case, alfred will still work but will not be diff --git a/netsock.c b/netsock.c index 84b0ec3..7bc49b4 100644 --- a/netsock.c +++ b/netsock.c @@ -116,6 +116,10 @@ int netsock_set_interfaces(struct globals *globals, char *interfaces)
netsock_close_all(globals);
+ /* interface 'none' disables all interface operations */ + if (strcmp(interfaces, "none") == 0) + return 0; + input = interfaces; while ((token = strtok_r(input, ",", &saveptr))) { input = NULL; @@ -452,25 +456,6 @@ err: return -1; }
-int netsock_open_all(struct globals *globals) -{ - int num_socks = 0; - int ret; - struct interface *interface; - - list_for_each_entry(interface, &globals->interfaces, list) { - if (globals->ipv4mode) - ret = netsock_open4(interface); - else - ret = netsock_open(interface); - - if (ret >= 0) - num_socks++; - } - - return num_socks; -} - size_t netsocket_count_interfaces(struct globals *globals) { struct interface *interface; diff --git a/server.c b/server.c index 85bf453..39206bf 100644 --- a/server.c +++ b/server.c @@ -372,7 +372,6 @@ int alfred_server(struct globals *globals) struct timespec last_check, now, tv; fd_set fds, errfds; size_t num_interfaces; - int num_socks;
if (create_hashes(globals)) return -1; @@ -380,11 +379,6 @@ int alfred_server(struct globals *globals) if (unix_sock_open_daemon(globals)) return -1;
- if (list_empty(&globals->interfaces)) { - fprintf(stderr, "Can't start server: interface missing\n"); - return -1; - } - if (strcmp(globals->mesh_iface, "none") != 0 && batadv_interface_check(globals->mesh_iface) < 0 && !globals->force) { @@ -393,12 +387,6 @@ int alfred_server(struct globals *globals) return -1; }
- num_socks = netsock_open_all(globals); - if (num_socks <= 0 && !globals->force) { - fprintf(stderr, "Failed to open interfaces\n"); - return -1; - } - num_interfaces = netsocket_count_interfaces(globals); if (num_interfaces > 1 && globals->opmode == OPMODE_SECONDARY) { fprintf(stderr, "More than one interface specified in secondary mode\n");
On Sunday, 2 January 2022 12:31:34 CET Marek Lindner wrote: [...]
diff --git a/README.rst b/README.rst
[...]
if (strcmp(globals->mesh_iface, "none") != 0 && batadv_interface_check(globals->mesh_iface) < 0 && !globals->force) { @@ -393,12 +387,6 @@ int alfred_server(struct globals *globals) return -1; }
- num_socks = netsock_open_all(globals);
- if (num_socks <= 0 && !globals->force) {
fprintf(stderr, "Failed to open interfaces\n");return -1;- }
- num_interfaces = netsocket_count_interfaces(globals); if (num_interfaces > 1 && globals->opmode == OPMODE_SECONDARY) { fprintf(stderr, "More than one interface specified in secondary mode\n");
This now causes the "--force" option (+its storage in the globals data structure) to be completely useless. I would prefer to have this handling still be there when !list_empty(&globals->interfaces).
And why is it necessary to not open the sockets on startup when interfaces are already given?
Kind regards, Sven
On Sunday, 2 January 2022 15:20:20 CET Sven Eckelmann wrote:
This now causes the "--force" option (+its storage in the globals data structure) to be completely useless.
Why would global->force be useless ? The alfred_server() function still uses the global->force state to determine if globals->mesh_iface is configured correctly.
I would prefer to have this handling still be there when !list_empty(&globals->interfaces).
To be honest, I hadn't fully understood what use case global->force is trying to address. What do you have in mind ? Checking for list_empty() will require alfred to be always started with an interface configured while alfred could be used without any interface at all and operate as local data storage between 2 processes on the same system or the interface could be configured at a later time (via unix socket).
And why is it necessary to not open the sockets on startup when interfaces are already given?
The main while loop calls netsock_reopen() in each round which will open all necessary sockets (unless I am mistaken). My guess is that this was added when the ALFRED_CHANGE_INTERFACE call was added. Therefore, the netsock_open() call is somewhat redundant unless alfred is meant to always require an interface at startup time and alfred is meant to bail out whenever that configured interface isn't available at startup time.
Cheers, Marek
On Sunday, 2 January 2022 20:01:47 CET Marek Lindner wrote:
On Sunday, 2 January 2022 15:20:20 CET Sven Eckelmann wrote:
This now causes the "--force" option (+its storage in the globals data structure) to be completely useless.
Why would global->force be useless ? The alfred_server() function still uses the global->force state to determine if globals->mesh_iface is configured correctly.
Ok, you are right about mesh_iface. But the patch missed to adjust the manpage to clarify this.
I would prefer to have this handling still be there when !list_empty(&globals->interfaces).
To be honest, I hadn't fully understood what use case global->force is trying to address. What do you have in mind ? Checking for list_empty() will require alfred to be always started with an interface configured while alfred could be used without any interface at all and operate as local data storage between 2 processes on the same system or the interface could be configured at a later time (via unix socket).
No, I wasn't talking about list_empty() but about !list_empty(). You removed the first block because you want to have have alfred started without any interface - fine with that.
But the default behavior of alfred in the past was to first check if the selected interfaces make sense and then return an error if there was a problem to open them. The force option basically ignored any error when there was an interface not ready yet. But the patch completely removed the chance to pre- check the interfaces on startup.
And why is it necessary to not open the sockets on startup when interfaces are already given?
The main while loop calls netsock_reopen() in each round which will open all necessary sockets (unless I am mistaken). My guess is that this was added when the ALFRED_CHANGE_INTERFACE call was added. Therefore, the netsock_open() call is somewhat redundant unless alfred is meant to always require an interface at startup time and alfred is meant to bail out whenever that configured interface isn't available at startup time.
See above. The situation I had in mind was:
* force not enabled * an interface given to the process * interface cannot be used by alfred
Kind regards, Sven
The batman-adv interface used by alfred can be changed at runtime by sending the CHANGE_BAT_IFACE command via unix socket.
Signed-off-by: Marek Lindner mareklindner@neomailbox.ch --- alfred.h | 4 +++- client.c | 37 +++++++++++++++++++++++++++++++++++++ main.c | 10 +++++++++- man/alfred.8 | 3 +++ packet.h | 14 ++++++++++++++ unix_sock.c | 27 ++++++++++++++++++++++++++- 6 files changed, 92 insertions(+), 3 deletions(-)
diff --git a/alfred.h b/alfred.h index 57d7daf..0fc6dc6 100644 --- a/alfred.h +++ b/alfred.h @@ -89,6 +89,7 @@ enum clientmode { CLIENT_SET_DATA, CLIENT_MODESWITCH, CLIENT_CHANGE_INTERFACE, + CLIENT_CHANGE_BAT_IFACE, };
struct interface { @@ -110,7 +111,7 @@ struct globals {
char *change_interface; struct server *best_server; /* NULL if we are a server ourselves */ - const char *mesh_iface; + char *mesh_iface; enum opmode opmode; enum clientmode clientmode; int clientmode_arg; @@ -150,6 +151,7 @@ int alfred_client_request_data(struct globals *globals); int alfred_client_set_data(struct globals *globals); int alfred_client_modeswitch(struct globals *globals); int alfred_client_change_interface(struct globals *globals); +int alfred_client_change_bat_iface(struct globals *globals); /* recv.c */ int recv_alfred_packet(struct globals *globals, struct interface *interface, int recv_sock); diff --git a/client.c b/client.c index dc643f3..e1107bf 100644 --- a/client.c +++ b/client.c @@ -296,3 +296,40 @@ int alfred_client_change_interface(struct globals *globals)
return 0; } + +int alfred_client_change_bat_iface(struct globals *globals) +{ + unsigned char buf[MAX_PAYLOAD]; + struct alfred_change_bat_iface_v0 *change_bat_iface; + int ret, len; + size_t interface_len; + + if (unix_sock_open_client(globals)) + return -1; + + interface_len = strlen(globals->mesh_iface); + if (interface_len > sizeof(change_bat_iface->bat_iface)) { + fprintf(stderr, "%s: batman-adv interface name list too long, not changing\n", + __func__); + return 0; + } + + change_bat_iface = (struct alfred_change_bat_iface_v0 *)buf; + len = sizeof(*change_bat_iface); + + change_bat_iface->header.type = ALFRED_CHANGE_BAT_IFACE; + change_bat_iface->header.version = ALFRED_VERSION; + change_bat_iface->header.length = htons(len - sizeof(change_bat_iface->header)); + strncpy(change_bat_iface->bat_iface, globals->mesh_iface, + sizeof(change_bat_iface->bat_iface)); + change_bat_iface->bat_iface[sizeof(change_bat_iface->bat_iface) - 1] = '\0'; + + ret = write(globals->unix_sock, buf, len); + if (ret != len) + fprintf(stderr, "%s: only wrote %d of %d bytes: %s\n", + __func__, ret, len, strerror(errno)); + + unix_sock_close(globals); + + return 0; +} diff --git a/main.c b/main.c index ad317cf..2cb6d44 100644 --- a/main.c +++ b/main.c @@ -37,6 +37,7 @@ static void alfred_usage(void) printf(" -M, --modeswitch primary switch daemon to mode primary\n"); printf(" secondary switch daemon to mode secondary\n"); printf(" -I, --change-interface [interface] change to the specified interface(s)\n"); + printf(" -B, --change-bat-iface [interface] change to the specified batman-adv interface\n"); printf("\n"); printf("server mode options:\n"); printf(" -i, --interface specify the interface (or comma separated list of interfaces) to listen on\n"); @@ -160,6 +161,7 @@ static struct globals *alfred_init(int argc, char *argv[]) {"req-version", required_argument, NULL, 'V'}, {"modeswitch", required_argument, NULL, 'M'}, {"change-interface", required_argument, NULL, 'I'}, + {"change-bat-iface", required_argument, NULL, 'B'}, {"unix-path", required_argument, NULL, 'u'}, {"update-command", required_argument, NULL, 'c'}, {"version", no_argument, NULL, 'v'}, @@ -194,7 +196,7 @@ static struct globals *alfred_init(int argc, char *argv[])
time_random_seed();
- while ((opt = getopt_long(argc, argv, "ms:r:hi:b:vV:M:I:u:dc:p:4:f", long_options, + while ((opt = getopt_long(argc, argv, "ms:r:hi:b:vV:M:I:B:u:dc:p:4:f", long_options, &opt_ind)) != -1) { switch (opt) { case 'r': @@ -252,6 +254,10 @@ static struct globals *alfred_init(int argc, char *argv[]) globals->clientmode = CLIENT_CHANGE_INTERFACE; globals->change_interface = strdup(optarg); break; + case 'B': + globals->clientmode = CLIENT_CHANGE_BAT_IFACE; + globals->mesh_iface = strdup(optarg); + break; case 'u': globals->unix_path = optarg; break; @@ -313,6 +319,8 @@ int main(int argc, char *argv[]) return alfred_client_modeswitch(globals); case CLIENT_CHANGE_INTERFACE: return alfred_client_change_interface(globals); + case CLIENT_CHANGE_BAT_IFACE: + return alfred_client_change_bat_iface(globals); }
return 0; diff --git a/man/alfred.8 b/man/alfred.8 index ff9b315..74814e0 100644 --- a/man/alfred.8 +++ b/man/alfred.8 @@ -91,6 +91,9 @@ to 0 ('\fB-V\fP 0'). .TP \fB-I\fP, \fB--change-interface\fP \fIinterface\fP Change the alfred server to use the new \fBinterface\fP(s) +.TP +\fB-B\fP, \fB--change-bat-iface\fP \fIinterface\fP +Change the alfred server to use the new \fBbatman-adv interface\fP . .SH SERVER OPTIONS .TP diff --git a/packet.h b/packet.h index 678f939..94c6a77 100644 --- a/packet.h +++ b/packet.h @@ -68,6 +68,7 @@ enum alfred_packet_type { ALFRED_STATUS_ERROR = 4, ALFRED_MODESWITCH = 5, ALFRED_CHANGE_INTERFACE = 6, + ALFRED_CHANGE_BAT_IFACE = 7, };
/* packets */ @@ -147,6 +148,19 @@ struct alfred_change_interface_v0 { char ifaces[IFNAMSIZ * 16]; } __packed;
+/** + * struct alfred_change_bat_iface_v0 - Request to change the + * batman-adv interface + * @header: TLV header describing the complete packet + * @bat_iface: interface to be changed to + * + * Sent to the daemon by client + */ +struct alfred_change_bat_iface_v0 { + struct alfred_tlv header; + char bat_iface[IFNAMSIZ]; +} __packed; + /** * struct alfred_status_v0 - Status info of a transaction * @header: TLV header describing the complete packet diff --git a/unix_sock.c b/unix_sock.c index d9ad07b..bc39199 100644 --- a/unix_sock.c +++ b/unix_sock.c @@ -345,6 +345,27 @@ err: return ret; }
+static int +unix_sock_change_bat_iface(struct globals *globals, + struct alfred_change_bat_iface_v0 *change_bat_iface, + int client_sock) +{ + int len, ret = -1; + + len = ntohs(change_bat_iface->header.length); + + if (len < (int)(sizeof(*change_bat_iface) - sizeof(change_bat_iface->header))) + goto err; + + free(globals->mesh_iface); + globals->mesh_iface = strdup(change_bat_iface->bat_iface); + + ret = 0; +err: + close(client_sock); + return ret; +} + int unix_sock_read(struct globals *globals) { int client_sock; @@ -402,7 +423,11 @@ int unix_sock_read(struct globals *globals) (struct alfred_change_interface_v0 *)packet, client_sock); break; - + case ALFRED_CHANGE_BAT_IFACE: + ret = unix_sock_change_bat_iface(globals, + (struct alfred_change_bat_iface_v0 *)packet, + client_sock); + break; default: /* unknown packet type */ ret = -1;
The 'server status' call exports the 'mode' as well as interface status via IPC. Both parameters can be modified at runtime via the IPC and as such, the current configuration is dynamic and not necessarily obvious.
The server status 'request' and 'reply' packet types are added to allow the IPC client to initiate the status retrieval. The server will respond with the 'reply' message.
Signed-off-by: Marek Lindner mareklindner@neomailbox.ch --- alfred.h | 2 ++ client.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++++ main.c | 9 +++++- man/alfred.8 | 3 ++ packet.h | 26 ++++++++++++++++ unix_sock.c | 50 ++++++++++++++++++++++++++++++ 6 files changed, 176 insertions(+), 1 deletion(-)
diff --git a/alfred.h b/alfred.h index 0fc6dc6..8ee7b21 100644 --- a/alfred.h +++ b/alfred.h @@ -90,6 +90,7 @@ enum clientmode { CLIENT_MODESWITCH, CLIENT_CHANGE_INTERFACE, CLIENT_CHANGE_BAT_IFACE, + CLIENT_SERVER_STATUS, };
struct interface { @@ -152,6 +153,7 @@ int alfred_client_set_data(struct globals *globals); int alfred_client_modeswitch(struct globals *globals); int alfred_client_change_interface(struct globals *globals); int alfred_client_change_bat_iface(struct globals *globals); +int alfred_client_server_status(struct globals *globals); /* recv.c */ int recv_alfred_packet(struct globals *globals, struct interface *interface, int recv_sock); diff --git a/client.c b/client.c index e1107bf..d540a9f 100644 --- a/client.c +++ b/client.c @@ -333,3 +333,90 @@ int alfred_client_change_bat_iface(struct globals *globals)
return 0; } + +int alfred_client_server_status(struct globals *globals) +{ + unsigned char buf[MAX_PAYLOAD]; + struct alfred_server_status_req_v0 *status_req; + struct alfred_server_status_rep_v0 *status_rep; + struct alfred_tlv *packet; + int ret, len, headsize, i; + + if (unix_sock_open_client(globals)) + return -1; + + status_req = (struct alfred_server_status_req_v0 *)buf; + len = sizeof(*status_req); + + status_req->header.type = ALFRED_SERVER_STATUS; + status_req->header.version = ALFRED_VERSION; + status_req->header.length = 0; + + ret = write(globals->unix_sock, buf, len); + if (ret != len) + fprintf(stderr, "%s: only wrote %d of %d bytes: %s\n", + __func__, ret, len, strerror(errno)); + + len = read(globals->unix_sock, buf, sizeof(buf)); + if (len <= 0) { + perror("read from unix socket failed"); + goto err; + } + + ret = -1; + + /* drop too small packets */ + headsize = sizeof(*packet); + if (len < headsize) { + perror("unexpected header size received from unix socket"); + goto err; + } + + packet = (struct alfred_tlv *)buf; + + if ((len - headsize) < ((int)ntohs(packet->length))) { + perror("unexpected packet size received from unix socket"); + goto err; + } + + if (packet->version != ALFRED_VERSION) { + perror("alfred version mismatch"); + goto err; + } + + status_rep = (struct alfred_server_status_rep_v0 *)buf; + headsize = ntohs(status_rep->header.length); + + if (headsize < (int)(sizeof(*status_rep) - sizeof(status_rep->header))) + goto err; + + fprintf(stdout, "alfred status:\n"); + + switch (status_rep->mode) { + case ALFRED_MODESWITCH_SECONDARY: + fprintf(stdout, "- mode: secondary\n"); + break; + case ALFRED_MODESWITCH_PRIMARY: + fprintf(stdout, "- mode: primary\n"); + break; + default: + fprintf(stderr, "- mode: unknown\n"); + break; + } + + fprintf(stderr, "- interfaces:\n"); + + for (i = 0; i < 16; i++) { + if (strlen(status_rep->ifaces[i].name) == 0) + continue; + + fprintf(stderr, "\t- name: %s\n", + status_rep->ifaces[i].name); + } + + fprintf(stdout, "- batman-adv interface: %s\n", status_rep->bat_iface); + +err: + unix_sock_close(globals); + return 0; +} diff --git a/main.c b/main.c index 2cb6d44..e2ac576 100644 --- a/main.c +++ b/main.c @@ -38,6 +38,7 @@ static void alfred_usage(void) printf(" secondary switch daemon to mode secondary\n"); printf(" -I, --change-interface [interface] change to the specified interface(s)\n"); printf(" -B, --change-bat-iface [interface] change to the specified batman-adv interface\n"); + printf(" -S, --server-status request server status info such as mode & interfaces\n"); printf("\n"); printf("server mode options:\n"); printf(" -i, --interface specify the interface (or comma separated list of interfaces) to listen on\n"); @@ -162,6 +163,7 @@ static struct globals *alfred_init(int argc, char *argv[]) {"modeswitch", required_argument, NULL, 'M'}, {"change-interface", required_argument, NULL, 'I'}, {"change-bat-iface", required_argument, NULL, 'B'}, + {"server-status", required_argument, NULL, 'S'}, {"unix-path", required_argument, NULL, 'u'}, {"update-command", required_argument, NULL, 'c'}, {"version", no_argument, NULL, 'v'}, @@ -196,7 +198,7 @@ static struct globals *alfred_init(int argc, char *argv[])
time_random_seed();
- while ((opt = getopt_long(argc, argv, "ms:r:hi:b:vV:M:I:B:u:dc:p:4:f", long_options, + while ((opt = getopt_long(argc, argv, "ms:r:hi:b:vV:M:I:B:Su:dc:p:4:f", long_options, &opt_ind)) != -1) { switch (opt) { case 'r': @@ -258,6 +260,9 @@ static struct globals *alfred_init(int argc, char *argv[]) globals->clientmode = CLIENT_CHANGE_BAT_IFACE; globals->mesh_iface = strdup(optarg); break; + case 'S': + globals->clientmode = CLIENT_SERVER_STATUS; + break; case 'u': globals->unix_path = optarg; break; @@ -321,6 +326,8 @@ int main(int argc, char *argv[]) return alfred_client_change_interface(globals); case CLIENT_CHANGE_BAT_IFACE: return alfred_client_change_bat_iface(globals); + case CLIENT_SERVER_STATUS: + return alfred_client_server_status(globals); }
return 0; diff --git a/man/alfred.8 b/man/alfred.8 index 74814e0..cf0eafc 100644 --- a/man/alfred.8 +++ b/man/alfred.8 @@ -94,6 +94,9 @@ Change the alfred server to use the new \fBinterface\fP(s) .TP \fB-B\fP, \fB--change-bat-iface\fP \fIinterface\fP Change the alfred server to use the new \fBbatman-adv interface\fP +.TP +\fB-S\fP, \fB--server-status\fP +Request server status information such as mode & interfaces\fP . .SH SERVER OPTIONS .TP diff --git a/packet.h b/packet.h index 94c6a77..b9e1f2e 100644 --- a/packet.h +++ b/packet.h @@ -69,6 +69,7 @@ enum alfred_packet_type { ALFRED_MODESWITCH = 5, ALFRED_CHANGE_INTERFACE = 6, ALFRED_CHANGE_BAT_IFACE = 7, + ALFRED_SERVER_STATUS = 8, };
/* packets */ @@ -159,6 +160,31 @@ struct alfred_change_interface_v0 { struct alfred_change_bat_iface_v0 { struct alfred_tlv header; char bat_iface[IFNAMSIZ]; +}; + +/** + * struct alfred_server_status_req_v0 - server status request + * @header: TLV header describing the complete packet + * + * Sent to the daemon by client + */ +struct alfred_server_status_req_v0 { + struct alfred_tlv header; +} __packed; + +/** + * struct alfred_server_status_req_v0 - server status reply + * @header: TLV header describing the complete packet + * + * Sent by the daemon to client in response to status request + */ +struct alfred_server_status_rep_v0 { + struct alfred_tlv header; + uint8_t mode; + struct { + char name[IFNAMSIZ]; + } ifaces[16]; + char bat_iface[IFNAMSIZ]; } __packed;
/** diff --git a/unix_sock.c b/unix_sock.c index bc39199..22a6805 100644 --- a/unix_sock.c +++ b/unix_sock.c @@ -366,6 +366,53 @@ err: return ret; }
+static int unix_sock_server_status(struct globals *globals, int client_sock) +{ + unsigned char buf[MAX_PAYLOAD]; + struct alfred_server_status_rep_v0 *status_rep; + struct interface *interface; + int ret, len, iface_count; + + status_rep = (struct alfred_server_status_rep_v0 *)buf; + len = sizeof(*status_rep); + + status_rep->header.type = ALFRED_SERVER_STATUS; + status_rep->header.version = ALFRED_VERSION; + status_rep->header.length = htons(len - sizeof(status_rep->header)); + + switch (globals->opmode) { + case OPMODE_SECONDARY: + status_rep->mode = ALFRED_MODESWITCH_SECONDARY; + break; + case OPMODE_PRIMARY: + status_rep->mode = ALFRED_MODESWITCH_PRIMARY; + break; + default: + break; + } + + iface_count = 0; + + list_for_each_entry(interface, &globals->interfaces, list) { + strncpy(status_rep->ifaces[iface_count].name, + interface->interface, + sizeof(status_rep->ifaces[iface_count].name)); + + iface_count++; + } + + strncpy(status_rep->bat_iface, globals->mesh_iface, + sizeof(status_rep->bat_iface)); + + ret = write(client_sock, buf, len); + if (ret == len) + return 0; + + fprintf(stderr, "%s: only wrote %d of %d bytes: %s\n", + __func__, ret, len, strerror(errno)); + return -1; +} + int unix_sock_read(struct globals *globals) { int client_sock; @@ -428,6 +475,9 @@ int unix_sock_read(struct globals *globals) (struct alfred_change_bat_iface_v0 *)packet, client_sock); break; + case ALFRED_SERVER_STATUS: + ret = unix_sock_server_status(globals, client_sock); + break; default: /* unknown packet type */ ret = -1;
On Sunday, 2 January 2022 12:31:36 CET Marek Lindner wrote:
list_for_each_entry(interface, &globals->interfaces, list) {strncpy(status_rep->ifaces[iface_count].name,interface->interface,sizeof(status_rep->ifaces[iface_count].name));iface_count++;}strncpy(status_rep->bat_iface, globals->mesh_iface,sizeof(status_rep->bat_iface));
strncpy doesn't guarantee that the copied string ends with a \0. You have to take care of that yourself.
This might not be a big problem here because the name buffers in the interfaces list have currently the same size as the name buffers in status_rep. Still feels better to have the line to manually set \0 at the end of the buffer.
Another thing I've just noticed: You don't take care of initialization all the bytes in the returned buffer. So you might leak data from the alfred process's stack to the client.
@Simon, would you prefer to have a global "status" message (which cannot be extended in the future) or separate "GET" queries for the existing commands:
* ALFRED_MODESWITCH -> ALFRED_GET_MODE * ALFRED_CHANGE_INTERFACE -> ALFRED_GET_INTERFACES * ALFRED_CHANGE_BAT_IFACE -> ALFRED_GET_BAT_IFACE
Kind regards, Sven
On Sunday, 2 January 2022 15:43:37 CET Sven Eckelmann wrote:
@Simon, would you prefer to have a global "status" message (which cannot be extended in the future) or separate "GET" queries for the existing commands:
- ALFRED_MODESWITCH -> ALFRED_GET_MODE
- ALFRED_CHANGE_INTERFACE -> ALFRED_GET_INTERFACES
- ALFRED_CHANGE_BAT_IFACE -> ALFRED_GET_BAT_IFACE
Another option would be to work with TLVs inside a single server status request. This would minimize the number of added packet definitions and retain future extensibility.
Cheers, Marek
On Wednesday, 12 January 2022 22:14:15 CET Marek Lindner wrote:
On Sunday, 2 January 2022 15:43:37 CET Sven Eckelmann wrote:
@Simon, would you prefer to have a global "status" message (which cannot be extended in the future) or separate "GET" queries for the existing commands:
- ALFRED_MODESWITCH -> ALFRED_GET_MODE
- ALFRED_CHANGE_INTERFACE -> ALFRED_GET_INTERFACES
- ALFRED_CHANGE_BAT_IFACE -> ALFRED_GET_BAT_IFACE
Another option would be to work with TLVs inside a single server status request. This would minimize the number of added packet definitions and retain future extensibility.
Just asked Simon directly and he seems to prefer the sub-TLV solution over the single requests.
Kind regards, Sven
On Sunday, 2 January 2022 12:31:36 CET Marek Lindner wrote:
+struct alfred_server_status_rep_v0 {
struct alfred_tlv header;uint8_t mode;struct {char name[IFNAMSIZ];} ifaces[16];char bat_iface[IFNAMSIZ];} __packed;
Another thing which came to my mind: maybe it could be interesting to have the status of an interface in the reply. So basically if netsock is >= 0 or not. Not sure how to deal with the IPv4 mcast code. Most likely just by using netsock_mcast and indicator.
Kind regards, Sven
b.a.t.m.a.n@lists.open-mesh.org
-
Marek Lindner -
Sven Eckelmann