[<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