Graybox: Remove the error pool

Waw. I'm surprised this still exists. Aside from being a badly designed module (the jresponse_send()s should receive error messages directly rather than rely on an awkward external database), it stopped working at some point for Graybox specifically. It's been an appallingly-timed headache for joolif tests. I would like to purge this module entirely, but I don't have time right now. Instead, remove it from Graybox. Graybox needs to be simple, and doesn't need to be user-friendly. I can just dump error messages in dmesg.
NICMx · Feb 26, 2024 · 3b8f2f2 · 3b8f2f2
1 parent 5694c5e
commit 3b8f2f2
Show file tree

Hide file tree

Showing 7 changed files with 5 additions and 34 deletions.
diff --git a/src/mod/common/error_pool.h b/src/mod/common/error_pool.h
@@ -3,6 +3,8 @@
 
 #include <linux/types.h>
 
+/* TODO (fine) This is nonsense; find a better way to send messages to userspace. */
+
 void error_pool_setup(void);
 void error_pool_teardown(void);
 

diff --git a/test/graybox/common/graybox-types.h b/test/graybox/common/graybox-types.h
@@ -19,7 +19,6 @@ enum graybox_attribute {
 
 	/* Response fields */
 	ATTR_ERROR_CODE,
-	ATTR_ERROR_STRING,
 	ATTR_STATS,
 
 	__ATTR_MAX,

diff --git a/test/graybox/mod/Kbuild b/test/graybox/mod/Kbuild
@@ -9,5 +9,4 @@ graybox-objs += expecter.o
 graybox-objs += genetlink.o
 graybox-objs += sender.o
 graybox-objs += nl_handler.o
-graybox-objs += ../../../src/mod/common/error_pool.o
 graybox-objs += ../../../src/mod/common/ipv6_hdr_iterator.o
diff --git a/test/graybox/mod/genetlink.c b/test/graybox/mod/genetlink.c
@@ -1,7 +1,6 @@
 #include "genetlink.h"
 
 #include "common/graybox-types.h"
-#include "mod/common/error_pool.h"
 
 static struct genl_family *family;
 
@@ -64,22 +63,8 @@ static int respond(struct genl_info *info, int error_code,
 
 int genl_respond(struct genl_info *info, int error_code)
 {
-	char *error_msg;
-	size_t error_msg_size;
-	int error;
-
 	pr_debug("Responding error code %d.\n", error_code);
-
-	error = error_pool_get_message(&error_msg, &error_msg_size);
-	if (error)
-		return error; /* Error msg already printed. */
-
-	error = respond(info, error_code, ATTR_ERROR_STRING, error_msg,
-			error_msg_size);
-	if (error_msg)
-		kfree(error_msg);
-
-	return error;
+	return respond(info, error_code, 0, NULL, 0);
 }
 
 int genl_respond_attr(struct genl_info *info, int attr_id, void *attr,

diff --git a/test/graybox/mod/nf_hook.c b/test/graybox/mod/nf_hook.c
@@ -3,7 +3,6 @@
 
 #include "common/types.h"
 #include "common/xlat.h"
-#include "mod/common/error_pool.h"
 
 #include "expecter.h"
 #include "log.h"
@@ -28,10 +27,8 @@ static int graybox_init(void)
 	error = nlhandler_setup();
 	if (error)
 		return error;
-	error_pool_setup();
 	error = expecter_setup();
 	if (error) {
-		error_pool_teardown();
 		nlhandler_teardown();
 		return error;
 	}
@@ -43,7 +40,6 @@ static int graybox_init(void)
 static void graybox_exit(void)
 {
 	expecter_teardown();
-	error_pool_teardown();
 	nlhandler_teardown();
 
 	log_info("%s module removed.", xlat_get_name());

diff --git a/test/graybox/mod/nl_handler.c b/test/graybox/mod/nl_handler.c
@@ -5,7 +5,6 @@
 #include "log.h"
 #include "sender.h"
 #include "common/types.h"
-#include "mod/common/error_pool.h"
 #include "mod/common/linux_version.h"
 
 static DEFINE_MUTEX(config_mutex);
@@ -123,7 +122,6 @@ static int handle_userspace_msg(struct sk_buff *skb, struct genl_info *info)
 	int error;
 
 	mutex_lock(&config_mutex);
-	error_pool_activate();
 
 	switch (info->genlhdr->cmd) {
 	case COMMAND_EXPECT_ADD:
@@ -143,11 +141,9 @@ static int handle_userspace_msg(struct sk_buff *skb, struct genl_info *info)
 		break;
 	default:
 		log_err("Unknown command code: %d", info->genlhdr->cmd);
-		error_pool_deactivate();
 		return genl_respond(info, -EINVAL);
 	}
 
-	error_pool_deactivate();
 	mutex_unlock(&config_mutex);
 
 	return error;

diff --git a/test/graybox/usr/genetlink.c b/test/graybox/usr/genetlink.c
@@ -129,21 +129,15 @@ static int response_handler(struct nl_msg *msg, void *arg_void)
 	 */
 
 	if (!attrs[ATTR_ERROR_CODE]) {
-		pr_err("Jool's response lacks a success/error code.");
+		pr_err("Graybox's response lacks a success/error code.");
 		arg->jool_error = -EINVAL;
 		return 0;
 	}
 
 	error = nla_get_u16(attrs[ATTR_ERROR_CODE]);
 	arg->jool_error = error;
 	if (error) {
-		if (attrs[ATTR_ERROR_STRING]) {
-			pr_err("The module threw error %d: %s", error,
-					nla_get_string(attrs[ATTR_ERROR_STRING]));
-		} else {
-			pr_err("The module's response contains error %d.", error);
-			pr_err("(Sorry; the response lacks an error message.)");
-		}
+		pr_err("The module returned error %d. (See dmesg.)", error);
 		return 0;
 	}