Commit a46bb010 authored by Andre Carvalho's avatar Andre Carvalho Committed by Jakub Kicinski
Browse files

netconsole: convert 'enabled' flag to enum for clearer state management



This patch refactors the netconsole driver's target enabled state from a
simple boolean to an explicit enum (`target_state`).

This allow the states to be expanded to a new state in the upcoming
change.

Co-developed-by: default avatarBreno Leitao <leitao@debian.org>
Signed-off-by: default avatarBreno Leitao <leitao@debian.org>
Reviewed-by: default avatarBreno Leitao <leitao@debian.org>
Signed-off-by: default avatarAndre Carvalho <asantostc@gmail.com>
Tested-by: default avatarBreno Leitao <leitao@debian.org>
Link: https://patch.msgid.link/20260118-netcons-retrigger-v11-2-4de36aebcf48@gmail.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent d793db4a
Loading
Loading
Loading
Loading
+28 −24
Original line number Diff line number Diff line
@@ -135,12 +135,12 @@ enum target_state {
 * @sysdata_fields:	Sysdata features enabled.
 * @msgcounter:	Message sent counter.
 * @stats:	Packet send stats for the target. Used for debugging.
 * @enabled:	On / off knob to enable / disable target.
 * @state:	State of the target.
 *		Visible from userspace (read-write).
 *		We maintain a strict 1:1 correspondence between this and
 *		whether the corresponding netpoll is active or inactive.
 *		Also, other parameters of a target may be modified at
 *		runtime only when it is disabled (enabled == 0).
 *		runtime only when it is disabled (state == STATE_DISABLED).
 * @extended:	Denotes whether console is extended or not.
 * @release:	Denotes whether kernel release version should be prepended
 *		to the message. Depends on extended console.
@@ -170,7 +170,7 @@ struct netconsole_target {
	u32			msgcounter;
#endif
	struct netconsole_target_stats stats;
	bool			enabled;
	enum target_state	state;
	bool			extended;
	bool			release;
	struct netpoll		np;
@@ -262,6 +262,7 @@ static struct netconsole_target *alloc_and_init(void)
	nt->np.local_port = 6665;
	nt->np.remote_port = 6666;
	eth_broadcast_addr(nt->np.remote_mac);
	nt->state = STATE_DISABLED;

	return nt;
}
@@ -280,7 +281,7 @@ static void netconsole_process_cleanups_core(void)
	mutex_lock(&target_cleanup_list_lock);
	list_for_each_entry_safe(nt, tmp, &target_cleanup_list, list) {
		/* all entries in the cleanup_list needs to be disabled */
		WARN_ON_ONCE(nt->enabled);
		WARN_ON_ONCE(nt->state == STATE_ENABLED);
		do_netpoll_cleanup(&nt->np);
		/* moved the cleaned target to target_list. Need to hold both
		 * locks
@@ -403,7 +404,7 @@ static void trim_newline(char *s, size_t maxlen)

static ssize_t enabled_show(struct config_item *item, char *buf)
{
	return sysfs_emit(buf, "%d\n", to_target(item)->enabled);
	return sysfs_emit(buf, "%d\n", to_target(item)->state == STATE_ENABLED);
}

static ssize_t extended_show(struct config_item *item, char *buf)
@@ -570,8 +571,8 @@ static ssize_t enabled_store(struct config_item *item,
		const char *buf, size_t count)
{
	struct netconsole_target *nt = to_target(item);
	bool enabled, current_enabled;
	unsigned long flags;
	bool enabled;
	ssize_t ret;

	mutex_lock(&dynamic_netconsole_mutex);
@@ -580,9 +581,10 @@ static ssize_t enabled_store(struct config_item *item,
		goto out_unlock;

	ret = -EINVAL;
	if (enabled == nt->enabled) {
	current_enabled = nt->state == STATE_ENABLED;
	if (enabled == current_enabled) {
		pr_info("network logging has already %s\n",
			nt->enabled ? "started" : "stopped");
			current_enabled ? "started" : "stopped");
		goto out_unlock;
	}

@@ -615,16 +617,16 @@ static ssize_t enabled_store(struct config_item *item,
		if (ret)
			goto out_unlock;

		nt->enabled = true;
		nt->state = STATE_ENABLED;
		pr_info("network logging started\n");
	} else {	/* false */
		/* We need to disable the netconsole before cleaning it up
		 * otherwise we might end up in write_msg() with
		 * nt->np.dev == NULL and nt->enabled == true
		 * nt->np.dev == NULL and nt->state == STATE_ENABLED
		 */
		mutex_lock(&target_cleanup_list_lock);
		spin_lock_irqsave(&target_list_lock, flags);
		nt->enabled = false;
		nt->state = STATE_DISABLED;
		/* Remove the target from the list, while holding
		 * target_list_lock
		 */
@@ -653,7 +655,7 @@ static ssize_t release_store(struct config_item *item, const char *buf,
	ssize_t ret;

	mutex_lock(&dynamic_netconsole_mutex);
	if (nt->enabled) {
	if (nt->state == STATE_ENABLED) {
		pr_err("target (%s) is enabled, disable to update parameters\n",
		       config_item_name(&nt->group.cg_item));
		ret = -EINVAL;
@@ -680,7 +682,7 @@ static ssize_t extended_store(struct config_item *item, const char *buf,
	ssize_t ret;

	mutex_lock(&dynamic_netconsole_mutex);
	if (nt->enabled) {
	if (nt->state == STATE_ENABLED)  {
		pr_err("target (%s) is enabled, disable to update parameters\n",
		       config_item_name(&nt->group.cg_item));
		ret = -EINVAL;
@@ -704,7 +706,7 @@ static ssize_t dev_name_store(struct config_item *item, const char *buf,
	struct netconsole_target *nt = to_target(item);

	mutex_lock(&dynamic_netconsole_mutex);
	if (nt->enabled) {
	if (nt->state == STATE_ENABLED) {
		pr_err("target (%s) is enabled, disable to update parameters\n",
		       config_item_name(&nt->group.cg_item));
		mutex_unlock(&dynamic_netconsole_mutex);
@@ -725,7 +727,7 @@ static ssize_t local_port_store(struct config_item *item, const char *buf,
	ssize_t ret = -EINVAL;

	mutex_lock(&dynamic_netconsole_mutex);
	if (nt->enabled) {
	if (nt->state == STATE_ENABLED) {
		pr_err("target (%s) is enabled, disable to update parameters\n",
		       config_item_name(&nt->group.cg_item));
		goto out_unlock;
@@ -747,7 +749,7 @@ static ssize_t remote_port_store(struct config_item *item,
	ssize_t ret = -EINVAL;

	mutex_lock(&dynamic_netconsole_mutex);
	if (nt->enabled) {
	if (nt->state == STATE_ENABLED) {
		pr_err("target (%s) is enabled, disable to update parameters\n",
		       config_item_name(&nt->group.cg_item));
		goto out_unlock;
@@ -770,7 +772,7 @@ static ssize_t local_ip_store(struct config_item *item, const char *buf,
	int ipv6;

	mutex_lock(&dynamic_netconsole_mutex);
	if (nt->enabled) {
	if (nt->state == STATE_ENABLED) {
		pr_err("target (%s) is enabled, disable to update parameters\n",
		       config_item_name(&nt->group.cg_item));
		goto out_unlock;
@@ -795,7 +797,7 @@ static ssize_t remote_ip_store(struct config_item *item, const char *buf,
	int ipv6;

	mutex_lock(&dynamic_netconsole_mutex);
	if (nt->enabled) {
	if (nt->state == STATE_ENABLED) {
		pr_err("target (%s) is enabled, disable to update parameters\n",
		       config_item_name(&nt->group.cg_item));
		goto out_unlock;
@@ -830,7 +832,7 @@ static ssize_t remote_mac_store(struct config_item *item, const char *buf,
	ssize_t ret = -EINVAL;

	mutex_lock(&dynamic_netconsole_mutex);
	if (nt->enabled) {
	if (nt->state == STATE_ENABLED) {
		pr_err("target (%s) is enabled, disable to update parameters\n",
		       config_item_name(&nt->group.cg_item));
		goto out_unlock;
@@ -1326,7 +1328,7 @@ static void drop_netconsole_target(struct config_group *group,
	 * The target may have never been enabled, or was manually disabled
	 * before being removed so netpoll may have already been cleaned up.
	 */
	if (nt->enabled)
	if (nt->state == STATE_ENABLED)
		netpoll_cleanup(&nt->np);

	config_item_put(&nt->group.cg_item);
@@ -1444,7 +1446,7 @@ static int netconsole_netdev_event(struct notifier_block *this,
			case NETDEV_RELEASE:
			case NETDEV_JOIN:
			case NETDEV_UNREGISTER:
				nt->enabled = false;
				nt->state = STATE_DISABLED;
				list_move(&nt->list, &target_cleanup_list);
				stopped = true;
			}
@@ -1725,7 +1727,8 @@ static void write_ext_msg(struct console *con, const char *msg,

	spin_lock_irqsave(&target_list_lock, flags);
	list_for_each_entry(nt, &target_list, list)
		if (nt->extended && nt->enabled && netif_running(nt->np.dev))
		if (nt->extended && nt->state == STATE_ENABLED &&
		    netif_running(nt->np.dev))
			send_ext_msg_udp(nt, msg, len);
	spin_unlock_irqrestore(&target_list_lock, flags);
}
@@ -1745,7 +1748,8 @@ static void write_msg(struct console *con, const char *msg, unsigned int len)

	spin_lock_irqsave(&target_list_lock, flags);
	list_for_each_entry(nt, &target_list, list) {
		if (!nt->extended && nt->enabled && netif_running(nt->np.dev)) {
		if (!nt->extended && nt->state == STATE_ENABLED &&
		    netif_running(nt->np.dev)) {
			/*
			 * We nest this inside the for-each-target loop above
			 * so that we're able to get as much logging out to
@@ -1901,7 +1905,7 @@ static struct netconsole_target *alloc_param_target(char *target_config,
			 */
			goto fail;
	} else {
		nt->enabled = true;
		nt->state = STATE_ENABLED;
	}
	populate_configfs_item(nt, cmdline_count);