On 02/08/2025 9:30, Vadim Fedorenko wrote:
diff --git a/Documentation/networking/ethtool-netlink.rst
b/Documentation/networking/ethtool-netlink.rst
index ab20c644af248..b270886c5f5d5 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -1541,6 +1541,11 @@ Drivers fill in the statistics in the following
structure:
.. kernel-doc:: include/linux/ethtool.h
:identifiers: ethtool_fec_stats
+Statistics may have FEC bins histogram attribute ``ETHTOOL_A_FEC_STAT_HIST``
+as defined in IEEE 802.3ck-2022 and 802.3df-2024. Nested attributes will have
+the range of FEC errors in the bin (inclusive) and the amount of error events
+in the bin.
+
Maybe worth mentioning per-lane histograms here.
FEC_SET
=======
diff --git a/drivers/net/netdevsim/ethtool.c b/drivers/net/netdevsim/ethtool.c
index f631d90c428ac..1dc9a6c126b24 100644
--- a/drivers/net/netdevsim/ethtool.c
+++ b/drivers/net/netdevsim/ethtool.c
@@ -164,12 +164,29 @@ nsim_set_fecparam(struct net_device *dev, struct
ethtool_fecparam *fecparam)
ns->ethtool.fec.active_fec = 1 << (fls(fec) - 1);
return 0;
}
+static const struct ethtool_fec_hist_range netdevsim_fec_ranges[] = {
+ { 0, 0},
+ { 1, 3},
+ { 4, 7},
+ { 0, 0}
+};
Following up on the discussion from v1, I agree with Gal's concern about
pushing array management into the driver. It adds complexity especially
when ranges depend on FEC mode.
The approach Andrew suggested makes sense to me. A simple helper to add
a bin would support both static and dynamic cases.
static void
-nsim_get_fec_stats(struct net_device *dev, struct ethtool_fec_stats *fec_stats)
+nsim_get_fec_stats(struct net_device *dev, struct ethtool_fec_stats *fec_stats,
+ const struct ethtool_fec_hist_range **ranges)
{
+ *ranges = netdevsim_fec_ranges;
+
fec_stats->corrected_blocks.total = 123;
fec_stats->uncorrectable_blocks.total = 4;
+
+ fec_stats->hist[0].bin_value = 345;
bin_value is 345 but the per-lane sum is 445.
+ fec_stats->hist[1].bin_value = 12;
+ fec_stats->hist[2].bin_value = 2;
+ fec_stats->hist[0].bin_value_per_lane[0] = 125;
+ fec_stats->hist[0].bin_value_per_lane[1] = 120;
+ fec_stats->hist[0].bin_value_per_lane[2] = 100;
+ fec_stats->hist[0].bin_value_per_lane[3] = 100;
}
+static int fec_put_hist(struct sk_buff *skb, const struct fec_stat_hist *hist,
+ const struct ethtool_fec_hist_range *ranges)
+{
+ struct nlattr *nest;
+ int i, j;
+
+ if (!ranges)
+ return 0;
+
+ for (i = 0; i < ETHTOOL_FEC_HIST_MAX; i++) {
+ if (i && !ranges[i].low && !ranges[i].high)
+ break;
+
+ nest = nla_nest_start(skb, ETHTOOL_A_FEC_STAT_HIST);
+ if (!nest)
+ return -EMSGSIZE;
+
+ if (nla_put_u32(skb, ETHTOOL_A_FEC_HIST_BIN_LOW,
+ ranges[i].low) ||
+ nla_put_u32(skb, ETHTOOL_A_FEC_HIST_BIN_HIGH,
+ ranges[i].high) ||
+ nla_put_uint(skb, ETHTOOL_A_FEC_HIST_BIN_VAL,
+ hist[i].bin_value))
Should skip bins where hist[i].bin_value isn’t set.
Thanks,
Carolina
