alfred: runtime configuration

alfred: runtime configuration

[Marek Lindner]

[-- Attachment #1: Type: text/plain, Size: 644 bytes --]

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 1/4] alfred: remove meaningless printf() call

[Marek Lindner]

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 {
-- 
2.32.0.rc0

[PATCH 2/4] alfred: Allow operating without any interface specified

[Marek Lindner]

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");
-- 
2.32.0.rc0

[PATCH 3/4] alfred: introduce 'change batman-adv interface' IPC call

[Marek Lindner]

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;
-- 
2.32.0.rc0

[PATCH 4/4] alfred: introduce 'server status' IPC call

[Marek Lindner]

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;
-- 
2.32.0.rc0

Re: [PATCH 2/4] alfred: Allow operating without any interface specified

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 1035 bytes --]

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 4/4] alfred: introduce 'server status' IPC call

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 1321 bytes --]

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/4] alfred: Allow operating without any interface specified

[Marek Lindner]

[-- Attachment #1: Type: text/plain, Size: 1405 bytes --]

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/4] alfred: Allow operating without any interface specified

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 2279 bytes --]

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 4/4] alfred: introduce 'server status' IPC call

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 575 bytes --]

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 4/4] alfred: introduce 'server status' IPC call

[Marek Lindner]

[-- Attachment #1: Type: text/plain, Size: 558 bytes --]

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 4/4] alfred: introduce 'server status' IPC call

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 747 bytes --]

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]