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: <Zs3EB+p+Qq1nYObX@gmail.com>
Date: Tue, 27 Aug 2024 05:18:15 -0700
From: Breno Leitao <leitao@...ian.org>
To: Maksym Kutsevol <max@...sevol.com>
Cc: "David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] netcons: Add udp send fail statistics to netconsole

On Sat, Aug 24, 2024 at 02:50:24PM -0700, Maksym Kutsevol wrote:
> Enhance observability of netconsole. UDP sends can fail. Start tracking at
> least two failure possibilities: ENOMEM and NET_XMIT_DROP for every target.
> Stats are exposed via an additional attribute in CONFIGFS.
> 
> The exposed statistics allows easier debugging of cases when netconsole
> messages were not seen by receivers, eliminating the guesswork if the
> sender thinks that messages in question were sent out.
> 
> Stats are not reset on enable/disable/change remote ip/etc, they
> belong to the netcons target itself.
> 
> Signed-off-by: Maksym Kutsevol <max@...sevol.com>

Would you mind adding a "Reported-by" me in this case?

https://lore.kernel.org/all/ZsWoUzyK5du9Ffl+@gmail.com/

> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> index 9c09293b5258..45c07ec7842d 100644
> --- a/drivers/net/netconsole.c
> +++ b/drivers/net/netconsole.c
> @@ -82,6 +82,13 @@ static DEFINE_SPINLOCK(target_list_lock);
>   */
>  static struct console netconsole_ext;
>  
> +#ifdef CONFIG_NETCONSOLE_DYNAMIC
> +struct netconsole_target_stats  {
> +	size_t xmit_drop_count;
> +	size_t enomem_count;

I am looking at other drivers, and they use a specific type for these
counters, u64_stats_sync.

if it is possible to use this format, then you can leverage the
`__u64_stats_update` helpers, and not worry about locking/overflow!?

> @@ -1015,6 +1035,25 @@ static struct notifier_block netconsole_netdev_notifier = {
>  	.notifier_call  = netconsole_netdev_event,
>  };
>  
> +/**
> + * count_udp_send_stats - Classify netpoll_send_udp result and count errors.
> + * @nt: target that was sent to
> + * @result: result of netpoll_send_udp
> + *
> + * Takes the result of netpoll_send_udp and classifies the type of error that
> + * occurred. Increments statistics in nt->stats accordingly.
> + */
> +static void count_udp_send_stats(struct netconsole_target *nt, int result)
> +{
> +#ifdef CONFIG_NETCONSOLE_DYNAMIC
> +	if (result == NET_XMIT_DROP) {
> +		nt->stats.xmit_drop_count++;

If you change the type, you can use the `u64_stats_inc` helper here.

> @@ -1126,7 +1167,11 @@ static void send_ext_msg_udp(struct netconsole_target *nt, const char *msg,
>  			this_offset += this_chunk;
>  		}
>  
> -		netpoll_send_udp(&nt->np, buf, this_header + this_offset);
> +		count_udp_send_stats(nt,
> +				     netpoll_send_udp(&nt->np,
> +						      buf,
> +						      this_header + this_offset)
> +		);

as Jakub said, this is not a format that is common in the Linux kenrel.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ