Re: [PATCH 2/2] netcons: Add udp send fail statistics to netconsole
From: Jakub Kicinski <kuba@kernel.org>
Date: 2024-08-26 21:35:48
Also in:
lkml
On Sat, 24 Aug 2024 14:50:24 -0700 Maksym Kutsevol wrote:
Enhance observability of netconsole. UDP sends can fail. Start tracking at
nit: "UDP sends" sounds a bit too much like it's using sockets maybe "packet sends"?
least two failure possibilities: ENOMEM and NET_XMIT_DROP for every target. Stats are exposed via an additional attribute in CONFIGFS.
Please provide a reference to another configfs user in the kernel which exposes stats. To help reviewers validate it's a legit use case.
+#ifdef CONFIG_NETCONSOLE_DYNAMIC
+struct netconsole_target_stats {
+ size_t xmit_drop_count;
+ size_t enomem_count;
+};
+#endifDon't hide types under ifdefs In fact I'm not sure if hiding stats if DYNAMIC isn't enabled makes sense. They don't take up much space.
+static ssize_t stats_show(struct config_item *item, char *buf)
+{
+ struct netconsole_target *nt = to_target(item);
+
+ return sysfs_emit(buf, "xmit_drop: %lu enomem: %lu\n",
+ nt->stats.xmit_drop_count, nt->stats.enomem_count);does configfs require value per file like sysfs or this is okay?
quoted hunk ↗ jump to hunk
/** * send_ext_msg_udp - send extended log message to target * @nt: target to send message to@@ -1063,7 +1102,9 @@ static void send_ext_msg_udp(struct netconsole_target *nt, const char *msg, "%s", userdata); msg_ready = buf; - netpoll_send_udp(&nt->np, msg_ready, msg_len); + count_udp_send_stats(nt, netpoll_send_udp(&nt->np, + msg_ready, + msg_len));
Please add a wrapper which calls netpoll_send_udp() and counts the stats. This sort of nested function calls are unlikely to meet kernel coding requirements. -- pw-bot: cr