netdev - Re: [PATCH net-next v2 3/3] netconsole: Populate dynamic entry even if netpoll fails

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240820162725.6b9064f8@kernel.org>
Date: Tue, 20 Aug 2024 16:27:25 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Breno Leitao <leitao@...ian.org>
Cc: davem@...emloft.net, edumazet@...gle.com, pabeni@...hat.com,
 netdev@...r.kernel.org, linux-kernel@...r.kernel.org, Aijay Adams
 <aijay@...a.com>
Subject: Re: [PATCH net-next v2 3/3] netconsole: Populate dynamic entry even
 if netpoll fails

On Mon, 19 Aug 2024 03:36:13 -0700 Breno Leitao wrote:
> -	if (err)
> -		goto fail;
> +	if (!err) {
> +		nt->enabled = true;
> +	} else {
> +		pr_err("Not enabling netconsole. Netpoll setup failed\n");
> +		if (!IS_ENABLED(CONFIG_NETCONSOLE_DYNAMIC))
> +			/* only fail if dynamic reconfiguration is set,
> +			 * otherwise, keep the target in the list, but disabled.
> +			 */
> +			goto fail;
> +	}

This will be better written as:

	if (err) {
		/* handle err */
	}

	nt->enabled = true;

As for the message would it be more helpful to indicate target will be
disabled? Move the print after the check for dynamic and say "Netpoll
setup failed, netconsole target will be disabled" ?